Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Charusso added a comment.
It also reverts https://reviews.llvm.org/rL362632 which is not a fix, but a problem instead. A condition could be a multi-line expression where we create the highlight in separated chunks. PathDiagnosticPopUpPiece is not made for that purpose, it cannot be added to multiple lines because we have only one ending part which contains all the notes. So that it cannot have multiple endings and therefore this patch narrows down the ranges of the highlight to the given interesting variable of the condition. It prevents HTML-breaking injections. Repository: rC Clang https://reviews.llvm.org/D65663 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp clang/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist clang/test/Analysis/Inputs/expected-plists/inline-plist.c.plist clang/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
Index: clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist =================================================================== --- clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist +++ clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist @@ -2513,7 +2513,7 @@ </dict> <dict> <key>line</key><integer>96</integer> - <key>col</key><integer>13</integer> + <key>col</key><integer>8</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -2735,7 +2735,7 @@ </dict> <dict> <key>line</key><integer>96</integer> - <key>col</key><integer>13</integer> + <key>col</key><integer>8</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -3554,7 +3554,7 @@ </dict> <dict> <key>line</key><integer>127</integer> - <key>col</key><integer>14</integer> + <key>col</key><integer>9</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -3776,7 +3776,7 @@ </dict> <dict> <key>line</key><integer>127</integer> - <key>col</key><integer>14</integer> + <key>col</key><integer>9</integer> <key>file</key><integer>0</integer> </dict> </array> Index: clang/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist =================================================================== --- clang/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist +++ clang/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist @@ -836,7 +836,7 @@ </dict> <dict> <key>line</key><integer>38</integer> - <key>col</key><integer>37</integer> + <key>col</key><integer>20</integer> <key>file</key><integer>0</integer> </dict> </array> Index: clang/test/Analysis/Inputs/expected-plists/inline-plist.c.plist =================================================================== --- clang/test/Analysis/Inputs/expected-plists/inline-plist.c.plist +++ clang/test/Analysis/Inputs/expected-plists/inline-plist.c.plist @@ -548,7 +548,7 @@ </dict> <dict> <key>line</key><integer>45</integer> - <key>col</key><integer>12</integer> + <key>col</key><integer>7</integer> <key>file</key><integer>0</integer> </dict> </array> Index: clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist =================================================================== --- clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist +++ clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist @@ -2727,7 +2727,7 @@ </dict> <dict> <key>line</key><integer>146</integer> - <key>col</key><integer>13</integer> + <key>col</key><integer>8</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -2949,7 +2949,7 @@ </dict> <dict> <key>line</key><integer>146</integer> - <key>col</key><integer>13</integer> + <key>col</key><integer>8</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -3929,7 +3929,7 @@ </dict> <dict> <key>line</key><integer>178</integer> - <key>col</key><integer>14</integer> + <key>col</key><integer>9</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -4185,7 +4185,7 @@ </dict> <dict> <key>line</key><integer>178</integer> - <key>col</key><integer>14</integer> + <key>col</key><integer>9</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -4281,7 +4281,7 @@ </dict> <dict> <key>line</key><integer>181</integer> - <key>col</key><integer>14</integer> + <key>col</key><integer>9</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -8087,7 +8087,7 @@ <key>location</key> <dict> <key>line</key><integer>267</integer> - <key>col</key><integer>18</integer> + <key>col</key><integer>19</integer> <key>file</key><integer>0</integer> </dict> <key>ranges</key> @@ -8095,7 +8095,7 @@ <array> <dict> <key>line</key><integer>267</integer> - <key>col</key><integer>18</integer> + <key>col</key><integer>19</integer> <key>file</key><integer>0</integer> </dict> <dict> @@ -8119,12 +8119,12 @@ <array> <dict> <key>line</key><integer>267</integer> - <key>col</key><integer>18</integer> + <key>col</key><integer>19</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>267</integer> - <key>col</key><integer>18</integer> + <key>col</key><integer>22</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -11983,12 +11983,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -12000,7 +12000,7 @@ <key>location</key> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <key>ranges</key> @@ -12008,7 +12008,7 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> @@ -12032,12 +12032,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -12244,12 +12244,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -12261,7 +12261,7 @@ <key>location</key> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <key>ranges</key> @@ -12269,7 +12269,7 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> @@ -12293,12 +12293,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -12571,12 +12571,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -12588,7 +12588,7 @@ <key>location</key> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <key>ranges</key> @@ -12596,7 +12596,7 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> @@ -12620,12 +12620,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -13128,12 +13128,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -13145,7 +13145,7 @@ <key>location</key> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <key>ranges</key> @@ -13153,7 +13153,7 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> @@ -13177,12 +13177,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -13752,12 +13752,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -13769,7 +13769,7 @@ <key>location</key> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <key>ranges</key> @@ -13777,7 +13777,7 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> @@ -13801,12 +13801,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -15295,12 +15295,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -15312,7 +15312,7 @@ <key>location</key> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <key>ranges</key> @@ -15320,7 +15320,7 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> @@ -15344,12 +15344,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -16965,12 +16965,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -16982,7 +16982,7 @@ <key>location</key> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <key>ranges</key> @@ -16990,7 +16990,7 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> @@ -17014,12 +17014,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -18860,12 +18860,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -18877,7 +18877,7 @@ <key>location</key> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <key>ranges</key> @@ -18885,7 +18885,7 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> @@ -18909,12 +18909,12 @@ <array> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> <dict> <key>line</key><integer>457</integer> - <key>col</key><integer>9</integer> + <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -22261,7 +22261,7 @@ </dict> <dict> <key>line</key><integer>587</integer> - <key>col</key><integer>16</integer> + <key>col</key><integer>11</integer> <key>file</key><integer>0</integer> </dict> </array> Index: clang/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist =================================================================== --- clang/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist +++ clang/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist @@ -191,7 +191,7 @@ </dict> <dict> <key>line</key><integer>11</integer> - <key>col</key><integer>14</integer> + <key>col</key><integer>9</integer> <key>file</key><integer>0</integer> </dict> </array> @@ -515,7 +515,7 @@ </dict> <dict> <key>line</key><integer>11</integer> - <key>col</key><integer>14</integer> + <key>col</key><integer>9</integer> <key>file</key><integer>0</integer> </dict> </array> Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -612,7 +612,7 @@ for (const auto &Range : PopUpRanges) { html::HighlightRange(R, Range.getBegin(), Range.getEnd(), "", "<table class='variable_popup'><tbody>", - /*IsTokenRange=*/false); + /*IsTokenRange=*/true); } } @@ -644,12 +644,11 @@ Out << "</tbody></table></span>"; html::HighlightRange(R, Range.getBegin(), Range.getEnd(), "<span class='variable'>", Buf.c_str(), - /*IsTokenRange=*/false); - - // Otherwise inject just the new row at the end of the range. + /*IsTokenRange=*/true); } else { + // Otherwise inject just the new row at the end of the range. html::HighlightRange(R, Range.getBegin(), Range.getEnd(), "", Buf.c_str(), - /*IsTokenRange=*/false); + /*IsTokenRange=*/true); } } Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -2410,16 +2410,18 @@ Out << (shouldInvert ? LhsString : RhsString); const LocationContext *LCtx = N->getLocationContext(); - PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx); // Convert 'field ...' to 'Field ...' if it is a MemberExpr. std::string Message = Out.str(); Message[0] = toupper(Message[0]); - // If we know the value create a pop-up note. - if (!IsAssuming) + // If we know the value create a pop-up note to the left part of 'BExpr'. + if (!IsAssuming) { + PathDiagnosticLocation Loc(BExpr->getLHS(), BRC.getSourceManager(), LCtx); return std::make_shared<PathDiagnosticPopUpPiece>(Loc, Message); + } + PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx); auto event = std::make_shared<PathDiagnosticEventPiece>(Loc, Message); if (shouldPrune.hasValue()) event->setPrunable(shouldPrune.getValue()); @@ -2472,12 +2474,14 @@ return nullptr; const LocationContext *LCtx = N->getLocationContext(); - PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx); - // If we know the value create a pop-up note. - if (!IsAssuming) + // If we know the value create a pop-up note to the 'DRE'. + if (!IsAssuming) { + PathDiagnosticLocation Loc(DRE, BRC.getSourceManager(), LCtx); return std::make_shared<PathDiagnosticPopUpPiece>(Loc, Out.str()); + } + PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx); auto event = std::make_shared<PathDiagnosticEventPiece>(Loc, Out.str()); const ProgramState *state = N->getState().get(); if (const MemRegion *R = state->getLValue(VD, LCtx).getAsRegion()) { @@ -2505,7 +2509,7 @@ return nullptr; const LocationContext *LCtx = N->getLocationContext(); - PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx); + PathDiagnosticLocation Loc(ME, BRC.getSourceManager(), LCtx); if (!Loc.isValid() || !Loc.asLocation().isValid()) return nullptr;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits