From: Csaba Dabis Date: Fri, 9 Aug 2019 02:20:44 +0000 (+0000) Subject: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f26add7ee19ae25f2f7c67e3706b0a0f865a29c0;p=clang [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces Summary: 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. Reviewed By: NoQ Differential Revision: https://reviews.llvm.org/D65663 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@368382 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 250793c4ba..731545022c 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -2335,12 +2335,12 @@ std::shared_ptr ConditionBRVisitor::VisitTrueTest( // Check if the field name of the MemberExprs is ambiguous. Example: // " 'a.d' is equal to 'h.d' " in 'test/Analysis/null-deref-path-notes.cpp'. bool IsSameFieldName = false; - if (const auto *LhsME = - dyn_cast(BExpr->getLHS()->IgnoreParenCasts())) - if (const auto *RhsME = - dyn_cast(BExpr->getRHS()->IgnoreParenCasts())) - IsSameFieldName = LhsME->getMemberDecl()->getName() == - RhsME->getMemberDecl()->getName(); + const auto *LhsME = dyn_cast(BExpr->getLHS()->IgnoreParenCasts()); + const auto *RhsME = dyn_cast(BExpr->getRHS()->IgnoreParenCasts()); + + if (LhsME && RhsME) + IsSameFieldName = + LhsME->getMemberDecl()->getName() == RhsME->getMemberDecl()->getName(); SmallString<128> LhsString, RhsString; { @@ -2410,16 +2410,31 @@ std::shared_ptr ConditionBRVisitor::VisitTrueTest( Out << (shouldInvert ? LhsString : RhsString); const LocationContext *LCtx = N->getLocationContext(); - PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx); + const SourceManager &SM = BRC.getSourceManager(); // 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 value part of 'BExpr'. + if (!IsAssuming) { + PathDiagnosticLocation Loc; + if (!shouldInvert) { + if (LhsME && LhsME->getMemberLoc().isValid()) + Loc = PathDiagnosticLocation(LhsME->getMemberLoc(), SM); + else + Loc = PathDiagnosticLocation(BExpr->getLHS(), SM, LCtx); + } else { + if (RhsME && RhsME->getMemberLoc().isValid()) + Loc = PathDiagnosticLocation(RhsME->getMemberLoc(), SM); + else + Loc = PathDiagnosticLocation(BExpr->getRHS(), SM, LCtx); + } + return std::make_shared(Loc, Message); + } + PathDiagnosticLocation Loc(Cond, SM, LCtx); auto event = std::make_shared(Loc, Message); if (shouldPrune.hasValue()) event->setPrunable(shouldPrune.getValue()); @@ -2472,12 +2487,14 @@ std::shared_ptr ConditionBRVisitor::VisitTrueTest( 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(Loc, Out.str()); + } + PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx); auto event = std::make_shared(Loc, Out.str()); const ProgramState *state = N->getState().get(); if (const MemRegion *R = state->getLValue(VD, LCtx).getAsRegion()) { @@ -2505,11 +2522,17 @@ std::shared_ptr ConditionBRVisitor::VisitTrueTest( return nullptr; const LocationContext *LCtx = N->getLocationContext(); - PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx); + PathDiagnosticLocation Loc; + + // If we know the value create a pop-up note to the member of the MemberExpr. + if (!IsAssuming && ME->getMemberLoc().isValid()) + Loc = PathDiagnosticLocation(ME->getMemberLoc(), BRC.getSourceManager()); + else + Loc = PathDiagnosticLocation(Cond, BRC.getSourceManager(), LCtx); + if (!Loc.isValid() || !Loc.asLocation().isValid()) return nullptr; - // If we know the value create a pop-up note. if (!IsAssuming) return std::make_shared(Loc, Out.str()); diff --git a/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp b/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp index f781c949e4..a641695722 100644 --- a/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ b/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -612,7 +612,7 @@ HandlePopUpPieceStartTag(Rewriter &R, for (const auto &Range : PopUpRanges) { html::HighlightRange(R, Range.getBegin(), Range.getEnd(), "", "", - /*IsTokenRange=*/false); + /*IsTokenRange=*/true); } } @@ -644,12 +644,11 @@ static void HandlePopUpPieceEndTag(Rewriter &R, Out << "
"; html::HighlightRange(R, Range.getBegin(), Range.getEnd(), "", 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); } } diff --git a/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist b/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist index e7f6dba2fa..edcaaf2cd4 100644 --- a/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist +++ b/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist @@ -191,7 +191,7 @@ line11 - col14 + col9 file0 @@ -515,7 +515,7 @@ line11 - col14 + col9 file0 diff --git a/test/Analysis/Inputs/expected-plists/edges-new.mm.plist b/test/Analysis/Inputs/expected-plists/edges-new.mm.plist index b4c79018c6..ab04dadbf4 100644 --- a/test/Analysis/Inputs/expected-plists/edges-new.mm.plist +++ b/test/Analysis/Inputs/expected-plists/edges-new.mm.plist @@ -2727,7 +2727,7 @@ line146 - col13 + col8 file0 @@ -2949,7 +2949,7 @@ line146 - col13 + col8 file0 @@ -3929,7 +3929,7 @@ line178 - col14 + col9 file0 @@ -4185,7 +4185,7 @@ line178 - col14 + col9 file0 @@ -4281,7 +4281,7 @@ line181 - col14 + col9 file0 @@ -8087,7 +8087,7 @@ location line267 - col18 + col19 file0 ranges @@ -8095,7 +8095,7 @@ line267 - col18 + col19 file0 @@ -8119,12 +8119,12 @@ line267 - col18 + col19 file0 line267 - col18 + col22 file0 @@ -11983,12 +11983,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -12000,7 +12000,7 @@ location line457 - col9 + col10 file0 ranges @@ -12008,7 +12008,7 @@ line457 - col9 + col10 file0 @@ -12032,12 +12032,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -12244,12 +12244,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -12261,7 +12261,7 @@ location line457 - col9 + col10 file0 ranges @@ -12269,7 +12269,7 @@ line457 - col9 + col10 file0 @@ -12293,12 +12293,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -12571,12 +12571,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -12588,7 +12588,7 @@ location line457 - col9 + col10 file0 ranges @@ -12596,7 +12596,7 @@ line457 - col9 + col10 file0 @@ -12620,12 +12620,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -13128,12 +13128,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -13145,7 +13145,7 @@ location line457 - col9 + col10 file0 ranges @@ -13153,7 +13153,7 @@ line457 - col9 + col10 file0 @@ -13177,12 +13177,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -13752,12 +13752,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -13769,7 +13769,7 @@ location line457 - col9 + col10 file0 ranges @@ -13777,7 +13777,7 @@ line457 - col9 + col10 file0 @@ -13801,12 +13801,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -15295,12 +15295,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -15312,7 +15312,7 @@ location line457 - col9 + col10 file0 ranges @@ -15320,7 +15320,7 @@ line457 - col9 + col10 file0 @@ -15344,12 +15344,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -16965,12 +16965,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -16982,7 +16982,7 @@ location line457 - col9 + col10 file0 ranges @@ -16990,7 +16990,7 @@ line457 - col9 + col10 file0 @@ -17014,12 +17014,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -18860,12 +18860,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -18877,7 +18877,7 @@ location line457 - col9 + col10 file0 ranges @@ -18885,7 +18885,7 @@ line457 - col9 + col10 file0 @@ -18909,12 +18909,12 @@ line457 - col9 + col10 file0 line457 - col9 + col14 file0 @@ -22243,6 +22243,40 @@ + + kindcontrol + edges + + + start + + + line587 + col11 + file0 + + + line587 + col11 + file0 + + + end + + + line587 + col11 + file0 + + + line587 + col11 + file0 + + + + + kindpop-up location @@ -22251,21 +22285,6 @@ col11 file0 - ranges - - - - line587 - col11 - file0 - - - line587 - col16 - file0 - - - extended_message Field 'b' is equal to 2 message diff --git a/test/Analysis/Inputs/expected-plists/inline-plist.c.plist b/test/Analysis/Inputs/expected-plists/inline-plist.c.plist index db6b5af0d5..1ee17de4b5 100644 --- a/test/Analysis/Inputs/expected-plists/inline-plist.c.plist +++ b/test/Analysis/Inputs/expected-plists/inline-plist.c.plist @@ -548,7 +548,7 @@ line45 - col12 + col7 file0 diff --git a/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist b/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist index 926f827426..3c87e3909b 100644 --- a/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist +++ b/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist @@ -836,7 +836,7 @@ line38 - col37 + col20 file0 diff --git a/test/Analysis/Inputs/expected-plists/plist-output.m.plist b/test/Analysis/Inputs/expected-plists/plist-output.m.plist index 5b1de9121f..ee8bf06e59 100644 --- a/test/Analysis/Inputs/expected-plists/plist-output.m.plist +++ b/test/Analysis/Inputs/expected-plists/plist-output.m.plist @@ -2513,7 +2513,7 @@ line96 - col13 + col8 file0 @@ -2735,7 +2735,7 @@ line96 - col13 + col8 file0 @@ -3554,7 +3554,7 @@ line127 - col14 + col9 file0 @@ -3776,7 +3776,7 @@ line127 - col14 + col9 file0