rnkovacs updated this revision to Diff 152604.
rnkovacs marked an inline comment as done.
rnkovacs added a comment.

Um, sorry, I totally forgot about that. Added your case to the tests.


https://reviews.llvm.org/D48522

Files:
  lib/StaticAnalyzer/Checkers/AllocationState.h
  lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/dangling-internal-buffer.cpp

Index: test/Analysis/dangling-internal-buffer.cpp
===================================================================
--- test/Analysis/dangling-internal-buffer.cpp
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -25,17 +25,29 @@
   const char *c;
   {
     std::string s;
-    c = s.c_str();
+    c = s.c_str(); // expected-note {{Dangling buffer was obtained here}}
+  } // expected-note {{Internal buffer is released because the object was destroyed}}
+  consume(c); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char2() {
+  const char *c;
+  {
+    std::string s;
+    c = s.c_str(); // expected-note {{Dangling buffer was obtained here}}
   } // expected-note {{Internal buffer is released because the object was destroyed}}
+  std::string s;
+  const char *c2 = s.c_str();
   consume(c); // expected-warning {{Use of memory after it is freed}}
   // expected-note@-1 {{Use of memory after it is freed}}
 }
 
 void deref_after_scope_wchar_t() {
   const wchar_t *w;
   {
     std::wstring ws;
-    w = ws.c_str();
+    w = ws.c_str(); // expected-note {{Dangling buffer was obtained here}}
   } // expected-note {{Internal buffer is released because the object was destroyed}}
   consume(w); // expected-warning {{Use of memory after it is freed}}
   // expected-note@-1 {{Use of memory after it is freed}}
@@ -45,7 +57,7 @@
   const char16_t *c16;
   {
     std::u16string s16;
-    c16 = s16.c_str();
+    c16 = s16.c_str(); // expected-note {{Dangling buffer was obtained here}}
   } // expected-note {{Internal buffer is released because the object was destroyed}}
   consume(c16); // expected-warning {{Use of memory after it is freed}}
   // expected-note@-1 {{Use of memory after it is freed}}
@@ -55,7 +67,7 @@
   const char32_t *c32;
   {
     std::u32string s32;
-    c32 = s32.c_str();
+    c32 = s32.c_str(); // expected-note {{Dangling buffer was obtained here}}
   } // expected-note {{Internal buffer is released because the object was destroyed}}
   consume(c32); // expected-warning {{Use of memory after it is freed}}
   // expected-note@-1 {{Use of memory after it is freed}}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1988,6 +1988,11 @@
     R->markInteresting(Sym);
     R->addRange(Range);
     R->addVisitor(llvm::make_unique<MallocBugVisitor>(Sym));
+
+    const RefState *RS = C.getState()->get<RegionState>(Sym);
+    if (RS->getAllocationFamily() == AF_InternalBuffer)
+      R->addVisitor(allocation_state::getDanglingBufferBRVisitor(Sym));
+
     C.emitReport(std::move(R));
   }
 }
Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -24,13 +24,51 @@
 using namespace clang;
 using namespace ento;
 
+// FIXME: c_str() may be called on a string object many times, so it should
+// have a list of symbols associated with it.
+REGISTER_MAP_WITH_PROGRAMSTATE(RawPtrMap, const MemRegion *, SymbolRef)
+
 namespace {
 
 class DanglingInternalBufferChecker : public Checker<check::DeadSymbols,
                                                      check::PostCall> {
   CallDescription CStrFn;
 
 public:
+  class DanglingBufferBRVisitor
+      : public BugReporterVisitorImpl<DanglingBufferBRVisitor> {
+    // Tracked pointer to a buffer.
+    SymbolRef Sym;
+
+  public:
+    DanglingBufferBRVisitor(SymbolRef Sym) : Sym(Sym) {}
+
+    static void *getTag() {
+      static int Tag = 0;
+      return &Tag;
+    }
+
+    void Profile(llvm::FoldingSetNodeID &ID) const override {
+      ID.AddPointer(getTag());
+    }
+
+    std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
+                                                   const ExplodedNode *PrevN,
+                                                   BugReporterContext &BRC,
+                                                   BugReport &BR) override;
+
+    bool isSymbolTracked(ProgramStateRef State, SymbolRef Sym) {
+      bool Found = false;
+      RawPtrMapTy Map = State->get<RawPtrMap>();
+      for (const auto Entry : Map) {
+        if (Entry.second == Sym)
+          Found = true;
+      }
+      return Found;
+    }
+
+  };
+
   DanglingInternalBufferChecker() : CStrFn("c_str") {}
 
   /// Record the connection between the symbol returned by c_str() and the
@@ -44,10 +82,6 @@
 
 } // end anonymous namespace
 
-// FIXME: c_str() may be called on a string object many times, so it should
-// have a list of symbols associated with it.
-REGISTER_MAP_WITH_PROGRAMSTATE(RawPtrMap, const MemRegion *, SymbolRef)
-
 void DanglingInternalBufferChecker::checkPostCall(const CallEvent &Call,
                                                   CheckerContext &C) const {
   const auto *ICall = dyn_cast<CXXInstanceCall>(&Call);
@@ -103,6 +137,41 @@
   C.addTransition(State);
 }
 
+std::shared_ptr<PathDiagnosticPiece>
+DanglingInternalBufferChecker::DanglingBufferBRVisitor::VisitNode(
+    const ExplodedNode *N, const ExplodedNode *PrevN,
+    BugReporterContext &BRC, BugReport &BR) {
+
+  if (!isSymbolTracked(N->getState(), Sym) ||
+      isSymbolTracked(PrevN->getState(), Sym))
+    return nullptr;
+
+  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  if (!S)
+    return nullptr;
+
+  SmallString<256> Buf;
+  llvm::raw_svector_ostream OS(Buf);
+  OS << "Dangling buffer was obtained here";
+  PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
+                             N->getLocationContext());
+  return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(),
+                                                    true, nullptr);
+}
+
+namespace clang {
+namespace ento {
+namespace allocation_state {
+
+std::unique_ptr<BugReporterVisitor>
+getDanglingBufferBRVisitor(SymbolRef Sym) {
+  return llvm::make_unique<DanglingInternalBufferChecker::DanglingBufferBRVisitor>(Sym);
+}
+
+} // end namespace allocation_state
+} // end namespace ento
+} // end namespace clang
+
 void ento::registerDanglingInternalBufferChecker(CheckerManager &Mgr) {
   registerNewDeleteChecker(Mgr);
   Mgr.registerChecker<DanglingInternalBufferChecker>();
Index: lib/StaticAnalyzer/Checkers/AllocationState.h
===================================================================
--- lib/StaticAnalyzer/Checkers/AllocationState.h
+++ lib/StaticAnalyzer/Checkers/AllocationState.h
@@ -20,6 +20,9 @@
 ProgramStateRef markReleased(ProgramStateRef State, SymbolRef Sym,
                              const Expr *Origin);
 
+std::unique_ptr<BugReporterVisitor>
+getDanglingBufferBRVisitor(SymbolRef Sym);
+
 } // end namespace allocation_state
 
 } // end namespace ento
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to