[analyzer][StackAddressEscape] Tie warnings to the diagnostic checkers rather then core.StackAddrEscapeBase

[Szelethus]

Summary

Exactly what it says on the tin!

[baloghadamsoftware]

This seems to be an obvious fix. Thank you for working on this!

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h

public:
   BuiltinBug(class CheckerNameRef checker, const char *name,
              const char *description)
       : BugType(checker, name, categories::LogicError), desc(description) {}
 
   BuiltinBug(const CheckerBase *checker, const char *name,
              const char *description)
       : BugType(checker, name, categories::LogicError), desc(description) {}
 
+  BuiltinBug(class CheckerNameRef checker, const char *name)
+      : BugType(checker, name, categories::LogicError), desc(name) {}
+
   BuiltinBug(const CheckerBase *checker, const char *name)
       : BugType(checker, name, categories::LogicError), desc(name) {}
 
   StringRef getDescription() const { return desc; }
 };
 
-} // end ento namespace
+} // namespace ento
 
 } // end clang namespace
 #endif

clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp

 public:
   enum CheckKind {
     CK_StackAddrEscapeChecker,
     CK_StackAddrAsyncEscapeChecker,
     CK_NumCheckKinds
   };
 
   DefaultBool ChecksEnabled[CK_NumCheckKinds];
+  CheckerNameRef CheckNames[CK_NumCheckKinds];
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const;
   void checkEndFunction(const ReturnStmt *RS, CheckerContext &Ctx) const;
 
 private:
   void checkReturnedBlockCaptures(const BlockDataRegion &B,
                                   CheckerContext &C) const;

 void StackAddrEscapeChecker::EmitStackError(CheckerContext &C,
                                             const MemRegion *R,
                                             const Expr *RetE) const {
   ExplodedNode *N = C.generateNonFatalErrorNode();
   if (!N)
     return;
   if (!BT_returnstack)
     BT_returnstack = std::make_unique<BuiltinBug>(
-        this, "Return of address to stack-allocated memory");
+        CheckNames[CK_StackAddrEscapeChecker],
+        "Return of address to stack-allocated memory");
   // Generate a report for this bug.
   SmallString<128> buf;
   llvm::raw_svector_ostream os(buf);
   SourceRange range = genName(os, R, C.getASTContext());
   os << " returned to caller";
   auto report =
       std::make_unique<PathSensitiveBugReport>(*BT_returnstack, os.str(), N);
   report->addRange(RetE->getSourceRange());
for (const MemRegion *Region : getCapturedStackRegions(B, C)) {
     // the "inner" block will be copied as well.
     if (isa<BlockDataRegion>(Region))
       continue;
     ExplodedNode *N = C.generateNonFatalErrorNode();
     if (!N)
       continue;
     if (!BT_capturedstackasync)
       BT_capturedstackasync = std::make_unique<BuiltinBug>(
-          this, "Address of stack-allocated memory is captured");
+          CheckNames[CK_StackAddrAsyncEscapeChecker],
+          "Address of stack-allocated memory is captured");
     SmallString<128> Buf;
     llvm::raw_svector_ostream Out(Buf);
     SourceRange Range = genName(Out, Region, C.getASTContext());
     Out << " is captured by an asynchronously-executed block";
     auto Report = std::make_unique<PathSensitiveBugReport>(
         *BT_capturedstackasync, Out.str(), N);
     if (Range.isValid())
       Report->addRange(Range);
     C.emitReport(std::move(Report));
   }
 }
 
 void StackAddrEscapeChecker::checkReturnedBlockCaptures(
     const BlockDataRegion &B, CheckerContext &C) const {
   for (const MemRegion *Region : getCapturedStackRegions(B, C)) {
     if (isArcManagedBlock(Region, C) || isNotInCurrentFrame(Region, C))
       continue;
     ExplodedNode *N = C.generateNonFatalErrorNode();
     if (!N)
       continue;
     if (!BT_capturedstackret)
       BT_capturedstackret = std::make_unique<BuiltinBug>(
-          this, "Address of stack-allocated memory is captured");
+          CheckNames[CK_StackAddrEscapeChecker],
+          "Address of stack-allocated memory is captured");
     SmallString<128> Buf;
     llvm::raw_svector_ostream Out(Buf);
     SourceRange Range = genName(Out, Region, C.getASTContext());
     Out << " is captured by a returned block";
     auto Report = std::make_unique<PathSensitiveBugReport>(*BT_capturedstackret,
                                                            Out.str(), N);
     if (Range.isValid())
       Report->addRange(Range);
void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
   // ExprWithCleanups node.)
   if (const ExprWithCleanups *Cleanup = dyn_cast<ExprWithCleanups>(RetE))
     RetE = Cleanup->getSubExpr();
   if (isa<CXXConstructExpr>(RetE) && RetE->getType()->isRecordType())
     return;
 
   // The CK_CopyAndAutoreleaseBlockObject cast causes the block to be copied
   // so the stack address is not escaping here.
-  if (auto *ICE = dyn_cast<ImplicitCastExpr>(RetE)) {
+  if (const auto *ICE = dyn_cast<ImplicitCastExpr>(RetE)) {
     if (isa<BlockDataRegion>(R) &&
         ICE->getCastKind() == CK_CopyAndAutoreleaseBlockObject) {
       return;
     }
   }
 
   EmitStackError(C, R, RetE);
 }
void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
 
   // Generate an error node.
   ExplodedNode *N = Ctx.generateNonFatalErrorNode(State);
   if (!N)
     return;
 
   if (!BT_stackleak)
     BT_stackleak = std::make_unique<BuiltinBug>(
-        this, "Stack address stored into global variable",
+        CheckNames[CK_StackAddrEscapeChecker],
+        "Stack address stored into global variable",
         "Stack address was saved into a global variable. "
         "This is dangerous because the address will become "
         "invalid after returning from the function");
 
   for (const auto &P : Cb.V) {
     // Generate a report for this bug.
     SmallString<128> Buf;
     llvm::raw_svector_ostream Out(Buf);

 }
 
 bool ento::shouldRegisterStackAddrEscapeBase(const CheckerManager &mgr) {
   return true;
 }
 
 #define REGISTER_CHECKER(name)                                                 \
   void ento::register##name(CheckerManager &Mgr) {                             \
-    StackAddrEscapeChecker *Chk =                                              \
-        Mgr.getChecker<StackAddrEscapeChecker>();                              \
+    StackAddrEscapeChecker *Chk = Mgr.getChecker<StackAddrEscapeChecker>();    \
     Chk->ChecksEnabled[StackAddrEscapeChecker::CK_##name] = true;              \
+    Chk->CheckNames[StackAddrEscapeChecker::CK_##name] =                       \
+        Mgr.getCurrentCheckerName();                                           \
   }                                                                            \
                                                                                \
-  bool ento::shouldRegister##name(const CheckerManager &mgr) {                     \
-    return true;                                                               \
-  }
+  bool ento::shouldRegister##name(const CheckerManager &mgr) { return true; }
 
 REGISTER_CHECKER(StackAddrEscapeChecker)
 REGISTER_CHECKER(StackAddrAsyncEscapeChecker)

clang/test/Analysis/incorrect-checker-names.cpp

 // RUN: %clang_analyze_cc1 -verify %s -fblocks \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-output=text
 
 int* stack_addr_escape_base() {
   int x = 0;
   // FIXME: This shouldn't be tied to a modeling checker.
-  return &x; // expected-warning{{Address of stack memory associated with local variable 'x' returned to caller [core.StackAddrEscapeBase]}}
+  return &x; // expected-warning{{Address of stack memory associated with local variable 'x' returned to caller [core.StackAddressEscape]}}
   // expected-note-re@-1{{{{^Address of stack memory associated with local variable 'x' returned to caller$}}}}
   // Just a regular compiler warning.
   // expected-warning@-3{{address of stack memory associated with local variable 'x' returned}}
 }
 
+char const *p;
+
+void f0() {
+  char const str[] = "This will change";
+  p = str;
+} // expected-warning{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller.  This will be a dangling reference [core.StackAddressEscape]}}
+// expected-note@-1{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller.  This will be a dangling reference}}

clang/test/Analysis/incorrect-checker-names.mm

-// RUN: %clang_analyze_cc1 -fblocks -verify %s -Wno-objc-root-class \
+// RUN: %clang_analyze_cc1 -fblocks -fobjc-arc -verify %s -Wno-objc-root-class \
 // RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.core.StackAddressAsyncEscape \
 // RUN:   -analyzer-checker=nullability \
 // RUN:   -analyzer-checker=osx
 
 #include "Inputs/system-header-simulator-for-nullability.h"
 #include "os_object_base.h"
 
 struct OSIterator : public OSObject {
   static const OSMetaClass * const metaClass;

 
 bool write_into_out_param_on_success(OS_RETURNS_RETAINED OSObject **obj);
 
 void use_out_param_leak() {
   OSObject *obj;
   // FIXME: This shouldn't be tied to a modeling checker.
   write_into_out_param_on_success(&obj); // expected-warning{{Potential leak of an object stored into 'obj' [osx.cocoa.RetainCountBase]}}
 }
+
+typedef struct dispatch_queue_s *dispatch_queue_t;
+typedef void (^dispatch_block_t)(void);
+void dispatch_async(dispatch_queue_t queue, dispatch_block_t block);
+typedef long dispatch_once_t;
+void dispatch_once(dispatch_once_t *predicate, dispatch_block_t block);
+typedef long dispatch_time_t;
+void dispatch_after(dispatch_time_t when, dispatch_queue_t queue, dispatch_block_t block);
+void dispatch_barrier_sync(dispatch_queue_t queue, dispatch_block_t block);
+
+extern dispatch_queue_t queue;
+extern dispatch_once_t *predicate;
+extern dispatch_time_t when;
+
+dispatch_block_t get_leaking_block() {
+  int leaked_x = 791;
+  int *p = &leaked_x;
+  return ^void(void) {
+    *p = 1;
+  };
+  // expected-warning@-3 {{Address of stack memory associated with local variable 'leaked_x' \
+is captured by a returned block [core.StackAddressEscape]}}
+}
+
+void test_returned_from_func_block_async() {
+  dispatch_async(queue, get_leaking_block());
+  // expected-warning@-1 {{Address of stack memory associated with local variable 'leaked_x' \
+is captured by an asynchronously-executed block [alpha.core.StackAddressAsyncEscape]}}
+}