[analyzer] InnerPointerChecker: Fix a segfault.

[NoQ]

Summary

Return value of dyn_cast_or_null should be checked before use. Otherwise we may put a null pointer into the map as a key and eventually crash in checkDeadSymbols.

Reka: Why did we restrict ourselves to TypedValueRegions here? While we are mostly interested in local string variables and temporaries, which would of course be typed, i guess there's nothing that prevents us from checking that we don't delete or mutate a string in a SymbolicRegion somewhere between obtaining and using its inner pointer.

[rnkovacs]

Return value of dyn_cast_or_null should be checked before use. Otherwise we may put a null pointer into the map as a key and eventually crash in checkDeadSymbols.

Hm, so with the last CallDescription patch we removed some code here that essentially checked if the same region was null before this cast, which means two things: a) in the previous version it probably should have been a dyn_cast instead of dyn_cast_or_null, but now that makes it accidentally fine, and b) I should have thought about this when that code was removed.

Reka: Why did we restrict ourselves to TypedValueRegions here? While we are mostly interested in local string variables and temporaries, which would of course be typed, i guess there's nothing that prevents us from checking that we don't delete or mutate a string in a SymbolicRegion somewhere between obtaining and using its inner pointer.

I think the reason is that previously CallDescriptions didn't match fully qualified function names and the type was needed to see if the object was a string.

[NoQ]

Ok, i'll get to the rest of the stuff a bit later (unless you pick it up).

lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp

 // -- Calling non-const member functions, except operator[], at, front, back,
 // begin, rbegin, end, and rend."
 
 void InnerPointerChecker::checkPostCall(const CallEvent &Call,
                                         CheckerContext &C) const {
   ProgramStateRef State = C.getState();
 
   if (const auto *ICall = dyn_cast<CXXInstanceCall>(&Call)) {
+    // TODO: Do we need these to be typed?
     const auto *ObjRegion = dyn_cast_or_null<TypedValueRegion>(
         ICall->getCXXThisVal().getAsRegion());
+    if (!ObjRegion)
+      return;
 
     if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
       SVal RawPtr = Call.getReturnValue();
       if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) {
         // Start tracking this raw pointer by adding it to the set of symbols
         // associated with this container object in the program state map.
 
         PtrSet::Factory &F = State->getStateManager().get_context<PtrSet>();

test/Analysis/inner-pointer.cpp

 char *c();
 class A {};
 
 void no_CXXRecordDecl() {
   A a, *b;
   *(void **)&b = c() + 1;
   *b = a; // no-crash
 }
+
+void checkReference(std::string &s) {
+  const char *c = s.c_str();
+}