<div dir="ltr">Thanks very much for the example!<div>I committed it together with the design correction in r338918.</div></div><br><div class="gmail_quote"><div dir="ltr">Alexander Kornienko <<a href="mailto:alexfh@google.com">alexfh@google.com</a>> ezt írta (időpont: 2018. aug. 3., P, 5:36):<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Thanks for the prompt fix, it solved the problem.</div><div><br></div><div>In case you want to add a regression test, this is what I came up with:</div><div>$ cat test-isCalledOnStringObject.cc</div><div>char *c();</div><div>class B {};</div><div>void d() {</div><div> B g, *h;</div><div> *(void **)&h = c() + 1;</div><div> *h = g;</div><div>}</div><div>$ ./clang-tidy -checks=-*,clang-analyzer* test-isCalledOnStringObject.cc -- -std=c++11</div><div><div> @ 0x5640f3e56b85 32 clang::DeclarationName::isIdentifier()</div><div> @ 0x5640f3ebaca3 80 clang::NamedDecl::getName()</div><div> @ 0x5640f9f7e56b 336 (anonymous namespace)::InnerPointerChecker::isCalledOnStringObject()</div><div> @ 0x5640f9f7e0ef 288 (anonymous namespace)::InnerPointerChecker::checkPostCall()</div><div> @ 0x5640f9f7e080 48 clang::ento::check::PostCall::_checkCall<>()</div><div> @ 0x5640fa2d5c12 64 clang::ento::CheckerFn<>::operator()()</div><div> @ 0x5640fa2c82d8 208 (anonymous namespace)::CheckCallContext::runChecker()</div><div> @ 0x5640fa2c4d6c 384 expandGraphWithCheckers<>()</div><div> @ 0x5640fa2c4acb 208 clang::ento::CheckerManager::runCheckersForCallEvent()</div><div> @ 0x5640fa32fdc8 80 clang::ento::CheckerManager::runCheckersForPostCall()</div><div> @ 0x5640fa332ff3 336 clang::ento::ExprEngine::evalCall()</div><div> @ 0x5640fa332e89 400 clang::ento::ExprEngine::VisitCallExpr()</div><div> @ 0x5640fa2ef8cb 4304 clang::ento::ExprEngine::Visit()</div><div> @ 0x5640fa2ec38c 464 clang::ento::ExprEngine::ProcessStmt()</div><div> @ 0x5640fa2ec079 208 clang::ento::ExprEngine::processCFGElement()</div><div> @ 0x5640fa31d8ff 128 clang::ento::CoreEngine::HandlePostStmt()</div><div> @ 0x5640fa31d208 496 clang::ento::CoreEngine::dispatchWorkItem()</div><div> @ 0x5640fa31cdbb 544 clang::ento::CoreEngine::ExecuteWorkList()</div><div> @ 0x5640f9c466b4 80 clang::ento::ExprEngine::ExecuteWorkList()</div></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Aug 3, 2018 at 12:28 AM Réka Nikolett Kovács <<a href="mailto:rekanikolett@gmail.com" target="_blank">rekanikolett@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Thanks for the notice. Have you managed to get the repro? I haven't succeeded in constructing one yet, but I've committed a check for the CXXRecordDecl. I hope that fixes the crashes.</div><br><div class="gmail_quote"><div dir="ltr">Alexander Kornienko <<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>> ezt írta (időpont: 2018. aug. 2., Cs, 11:07):<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Jul 30, 2018 at 5:44 PM Reka Kovacs via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: rkovacs<br>
Date: Mon Jul 30 08:43:45 2018<br>
New Revision: 338259<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=338259&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=338259&view=rev</a><br>
Log:<br>
[analyzer] Add support for more invalidating functions in InnerPointerChecker.<br>
<br>
According to the standard, pointers referring to the elements of a<br>
`basic_string` may be invalidated if they are used as an argument to<br>
any standard library function taking a reference to non-const<br>
`basic_string` as an argument. This patch makes InnerPointerChecker warn<br>
for these cases.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D49656" rel="noreferrer" target="_blank">https://reviews.llvm.org/D49656</a><br>
<br>
Modified:<br>
cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp<br>
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp<br>
cfe/trunk/test/Analysis/inner-pointer.cpp<br>
<br>
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp?rev=338259&r1=338258&r2=338259&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp?rev=338259&r1=338258&r2=338259&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp (original)<br>
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp Mon Jul 30 08:43:45 2018<br>
@@ -91,37 +91,53 @@ public:<br>
ReserveFn("reserve"), ResizeFn("resize"),<br>
ShrinkToFitFn("shrink_to_fit"), SwapFn("swap") {}<br>
<br>
- /// Check whether the function called on the container object is a<br>
- /// member function that potentially invalidates pointers referring<br>
- /// to the objects's internal buffer.<br>
- bool mayInvalidateBuffer(const CallEvent &Call) const;<br>
-<br>
- /// Record the connection between the symbol returned by c_str() and the<br>
- /// corresponding string object region in the ProgramState. Mark the symbol<br>
- /// released if the string object is destroyed.<br>
+ /// Check if the object of this member function call is a `basic_string`.<br>
+ bool isCalledOnStringObject(const CXXInstanceCall *ICall) const;<br>
+<br>
+ /// Check whether the called member function potentially invalidates<br>
+ /// pointers referring to the container object's inner buffer.<br>
+ bool isInvalidatingMemberFunction(const CallEvent &Call) const;<br>
+<br>
+ /// Mark pointer symbols associated with the given memory region released<br>
+ /// in the program state.<br>
+ void markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State,<br>
+ const MemRegion *ObjRegion,<br>
+ CheckerContext &C) const;<br>
+<br>
+ /// Standard library functions that take a non-const `basic_string` argument by<br>
+ /// reference may invalidate its inner pointers. Check for these cases and<br>
+ /// mark the pointers released.<br>
+ void checkFunctionArguments(const CallEvent &Call, ProgramStateRef State,<br>
+ CheckerContext &C) const;<br>
+<br>
+ /// Record the connection between raw pointers referring to a container<br>
+ /// object's inner buffer and the object's memory region in the program state.<br>
+ /// Mark potentially invalidated pointers released.<br>
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;<br>
<br>
- /// Clean up the ProgramState map.<br>
+ /// Clean up the program state map.<br>
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;<br>
};<br>
<br>
} // end anonymous namespace<br>
<br>
-// [string.require]<br>
-//<br>
-// "References, pointers, and iterators referring to the elements of a<br>
-// basic_string sequence may be invalidated by the following uses of that<br>
-// basic_string object:<br>
-//<br>
-// -- TODO: As an argument to any standard library function taking a reference<br>
-// to non-const basic_string as an argument. For example, as an argument to<br>
-// non-member functions swap(), operator>>(), and getline(), or as an argument<br>
-// to basic_string::swap().<br>
-//<br>
-// -- Calling non-const member functions, except operator[], at, front, back,<br>
-// begin, rbegin, end, and rend."<br>
-//<br>
-bool InnerPointerChecker::mayInvalidateBuffer(const CallEvent &Call) const {<br>
+bool InnerPointerChecker::isCalledOnStringObject(<br>
+ const CXXInstanceCall *ICall) const {<br>
+ const auto *ObjRegion =<br>
+ dyn_cast_or_null<TypedValueRegion>(ICall->getCXXThisVal().getAsRegion());<br>
+ if (!ObjRegion)<br>
+ return false;<br>
+<br>
+ QualType ObjTy = ObjRegion->getValueType();<br>
+ if (ObjTy.isNull() ||<br>
+ ObjTy->getAsCXXRecordDecl()->getName() != "basic_string")<br></blockquote><div><br></div><div>We observe crashes on this line. I'm working on an isolated repro, but you could probably try to construct one yourself (I suppose, getAsCXXRecordDecl() can return null here).</div><div> </div></div></div>
</blockquote></div>
</blockquote></div>
</blockquote></div>