[analyzer] PR41239: Fix a crash on invalid source location in NoStoreFuncVisitor.

[NoQ]

Summary

It turns out that SourceManager::isInSystemHeader() crashes when an invalid source location is passed into it. Invalid source locations are relatively common: not only they come from body farms, but also, say, any function in C that didn't come with a forward declaration would have an implicit forward declaration with invalid source locations.

Not sure if this deserves to be fixed in SourceManager, but there's anyway a more comfy API for us to use in the Static Analyzer: CallEvent::isInSystemHeader(), so i just used that.

[Charusso]

Nice solution.

[Szelethus]

An assert would be real nice in SourceManager, we should definitely add that, since some callbacks don't even receive a CallEvent object. My checker runs on checkPostCall, and also checks where fields lie in a system header.

[alexfh]

I've posted another test case to https://bugs.llvm.org/show_bug.cgi?id=41239. It may already be covered by the fix. Could you check?

[NoQ]

This other example seems to work and looks relatively similar, just with a different checker causing the visitor to be attached.

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
 
     // No diagnostic if region was modified inside the frame.
     if (!CallExitLoc || isRegionOfInterestModifiedInFrame(N))
       return nullptr;
 
     CallEventRef<> Call =
         BR.getStateManager().getCallEventManager().getCaller(SCtx, State);
 
-    if (SM.isInSystemHeader(Call->getDecl()->getSourceRange().getBegin()))
+    if (Call->isInSystemHeader())
       return nullptr;
 
     // Region of interest corresponds to an IVar, exiting a method
     // which could have written into that IVar, but did not.
     if (const auto *MC = dyn_cast<ObjCMethodCall>(Call)) {
       if (const auto *IvarR = dyn_cast<ObjCIvarRegion>(RegionOfInterest)) {
         const MemRegion *SelfRegion = MC->getReceiverSVal().getAsRegion();
         if (RegionOfInterest->isSubRegionOf(SelfRegion) &&

clang/test/Analysis/diagnostics/no-store-func-path-notes.c

-// RUN: %clang_analyze_cc1 -x c -analyzer-checker=core -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -w -x c -analyzer-checker=core -analyzer-output=text\
+// RUN:     -verify %s
 
 typedef __typeof(sizeof(int)) size_t;
 void *memset(void *__s, int __c, size_t __n);
 
 int initializer1(int *p, int x) {
   if (x) { // expected-note{{Taking false branch}}
     *p = 1;
     return 0;
int useInitializeMaybeInStruct() {
   int z; // expected-note{{'z' declared without an initial value}}
   D d;
   d.x = &z;
   initializeMaybeInStruct(&d); // expected-note{{Calling 'initializeMaybeInStruct'}}
                                // expected-note@-1{{Returning from 'initializeMaybeInStruct'}}
   return z; // expected-warning{{Undefined or garbage value returned to caller}}
             // expected-note@-1{{Undefined or garbage value returned to caller}}
 }
+
+void test_implicit_function_decl(int *x) {
+  if (x) {} // expected-note{{Assuming 'x' is null}}
+            // expected-note@-1{{Taking false branch}}
+  implicit_function(x);
+  *x = 4; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+          // expected-note@-1{{Dereference of null pointer (loaded from variable 'x')}}
+}
+int implicit_function(int *y) {}