[analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

[rnkovacs]

Summary

A pointer referring to the elements of a basic_string may be invalidated by calling a non-const member function, except operator[], at, front, back, begin, rbegin, end, and rend. The checker now warns if the pointer is used after such operations.

[NoQ]

Cool! I don't have a strong preference with respect to whitelist vs. blacklist; your approach is safer but listing functions that don't immediately invalidate the buffer would allow us to avoid hard-to-detect false negatives while pretending that our users would notice and report easy-to-fix false positives for us. Also we rarely commit to adding a test for every single supported API function; bonus points for that, but usually 2-3 functions from a series of similar functions is enough :)

lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp

112–124

That quote from the Standard would look great here.

[rnkovacs]

Added standard quote, marking the section about non-member functions that may also invalidate the buffer as a TODO.
Also changed the note message to that suggested by @NoQ (thanks!). All tests pass now.

[rnkovacs]

In D49360#1163113, @NoQ wrote:

Also we rarely commit to adding a test for every single supported API function; bonus points for that, but usually 2-3 functions from a series of similar functions is enough :)

Um, okay, noted for next time :)

[NoQ]

Hmm, the destructor-specific message was pretty good, can we keep it? It should be possible to print a different message depending on the program point within N.

[dcoughlin]

It is really nice to see this checker take shape! Some drive by diagnostic comments in line.

test/Analysis/dangling-internal-buffer.cpp

175

In other parts of clang we use the term "inner pointer" to mean a pointer that will be invalidated if its containing object is destroyed https://clang.llvm.org/docs/AutomaticReferenceCounting.html#interior-pointers. There are existing attributes that use this term to specify that a method returns an inner pointer.

I think it would be good to use the same terminology here. So the diagnostic could be something like "Dangling inner pointer obtained here".

176

What do you think about explicitly mentioning the name of the method here when we have it? This will make it more clear when there are multiple methods on the same line.

I also think that instead of saying "is allowed to" (which raises the question "by whom?") you could make it more direct.

How about:
"Inner pointer invalidated by call to 'clear'"

or, for the destructor "Inner pointer invalidated by call to destructor"?

What do you think?

If you're worried about this wording being to strong, you could weaken it with a "may be" to:

"Inner pointer may be invalidated by call to 'clear'"

[rnkovacs]

Note messages updated.

test/Analysis/dangling-internal-buffer.cpp

175

I feel like I should also retitle the checker to DanglingInnerBuffer to remain consistent. What do you think?

176

I like these, thanks! I went with the stronger versions now, as they seem to fit better with the warnings themselves.

[NoQ]

I showed the bug mentioned in D49058 to a friend who didn't do much C++ recently, for a fresh look, and he provided a bunch of interesting feedback by explaining the way he didn't understand what the analyzer was trying to say.

1. When we call c_str(), the pointer is not dangling yet, not until the destructor or realloc is called. He didn't understand the report because he was trying to figure out why do we think the pointer is already dangling.
2. A generic "use after free" warning on the return site is confusing because the user would expect to see an actual "use" instead of just passing it around. We should be more specific, i.e. "Deallocated pointer returned to the caller".
3. We mention that there's a destructor, but the destructor is hard to see. Knowing the type of the destroyed object would help. Knowing that it's a temporary object would help.
4. The whole idea of "string has a buffer that would be destroyed when the string is destroyed and we shouldn't pass the pointer around" needs to be explained all together, rather than separated into different diagnostic pieces. The user needs to be somehow informed that this is how std::string operates because he doesn't necessarily know that.

test/Analysis/dangling-internal-buffer.cpp

176

"Inner pointer invalidated by call to 'clear'"

I think the word "invalidated" may be confusing, how about "reallocated"? And "deallocated" in case of destructors.

[NoQ]

Let's commit this patch and make another round later, as we gather ideas for better names and messages.

[NoQ]

test/Analysis/dangling-internal-buffer.cpp

175

My intuition suggests that we should remove the word "Dangling" from the checker name, because our checker names are usually indicating what they check, not what bugs they find. Eg., MallocChecker doesn't find all mallocs, it checks that mallocs are used correctly. This checker checks that pointers to inner buffers are used correctly, so we may call it InnerPointerChecker or something like that.

lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp

 using PtrSet = llvm::ImmutableSet<SymbolRef>;
 
 // Associate container objects with a set of raw pointer symbols.
 REGISTER_MAP_WITH_PROGRAMSTATE(RawPtrMap, const MemRegion *, PtrSet)
 
 // This is a trick to gain access to PtrSet's Factory.
 namespace clang {
 namespace ento {
-template<> struct ProgramStateTrait<PtrSet>
-  : public ProgramStatePartialTrait<PtrSet> {
+template <>
+struct ProgramStateTrait<PtrSet> : public ProgramStatePartialTrait<PtrSet> {
   static void *GDMIndex() {
     static int Index = 0;
     return &Index;
   }
 };
 } // end namespace ento
 } // end namespace clang
 
 namespace {
 
 class DanglingInternalBufferChecker
     : public Checker<check::DeadSymbols, check::PostCall> {
-  CallDescription CStrFn, DataFn;
+
+  CallDescription AppendFn, AssignFn, ClearFn, CStrFn, DataFn, EraseFn,
+      InsertFn, PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn,
+      ShrinkToFitFn, SwapFn;
 
 public:
   class DanglingBufferBRVisitor : public BugReporterVisitor {
     SymbolRef PtrToBuf;
 
   public:
     DanglingBufferBRVisitor(SymbolRef Sym) : PtrToBuf(Sym) {}
 
bool isSymbolTracked(ProgramStateRef State, SymbolRef Sym) {
       for (const auto Entry : Map) {
         if (Entry.second.contains(Sym))
           return true;
       }
       return false;
     }
   };
 
-  DanglingInternalBufferChecker() : CStrFn("c_str"), DataFn("data") {}
+  DanglingInternalBufferChecker()
+      : AppendFn("append"), AssignFn("assign"), ClearFn("clear"),
+        CStrFn("c_str"), DataFn("data"), EraseFn("erase"), InsertFn("insert"),
+        PopBackFn("pop_back"), PushBackFn("push_back"), ReplaceFn("replace"),
+        ReserveFn("reserve"), ResizeFn("resize"),
+        ShrinkToFitFn("shrink_to_fit"), SwapFn("swap") {}
+
+  /// Check whether the function called on the container object is a
+  /// member function that potentially invalidates pointers referring
+  /// to the objects's internal buffer.
+  bool mayInvalidateBuffer(const CallEvent &Call) const;
 
   /// Record the connection between the symbol returned by c_str() and the
   /// corresponding string object region in the ProgramState. Mark the symbol
   /// released if the string object is destroyed.
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
 
   /// Clean up the ProgramState map.
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
 };
 
 } // end anonymous namespace
 
+// [string.require]
+//
+// "References, pointers, and iterators referring to the elements of a
+// basic_string sequence may be invalidated by the following uses of that
+// basic_string object:
+//
+// -- TODO: As an argument to any standard library function taking a reference
+// to non-const basic_string as an argument. For example, as an argument to
+// non-member functions swap(), operator>>(), and getline(), or as an argument
+// to basic_string::swap().
+//
+// -- Calling non-const member functions, except operator[], at, front, back,
+// begin, rbegin, end, and rend."
+//
+bool DanglingInternalBufferChecker::mayInvalidateBuffer(
+    const CallEvent &Call) const {
+  if (const auto *MemOpCall = dyn_cast<CXXMemberOperatorCall>(&Call)) {
+    OverloadedOperatorKind Opc = MemOpCall->getOriginExpr()->getOperator();
+    if (Opc == OO_Equal || Opc == OO_PlusEqual)
+      return true;
+    return false;
+  }
+  return (isa<CXXDestructorCall>(Call) || Call.isCalled(AppendFn) ||
+          Call.isCalled(AssignFn) || Call.isCalled(ClearFn) ||
+          Call.isCalled(EraseFn) || Call.isCalled(InsertFn) ||
+          Call.isCalled(PopBackFn) || Call.isCalled(PushBackFn) ||
+          Call.isCalled(ReplaceFn) || Call.isCalled(ReserveFn) ||
+          Call.isCalled(ResizeFn) || Call.isCalled(ShrinkToFitFn) ||
+          Call.isCalled(SwapFn));
+}
+
 void DanglingInternalBufferChecker::checkPostCall(const CallEvent &Call,
                                                   CheckerContext &C) const {
   const auto *ICall = dyn_cast<CXXInstanceCall>(&Call);
   if (!ICall)
     return;
 
   SVal Obj = ICall->getCXXThisVal();
   const auto *ObjRegion = dyn_cast_or_null<TypedValueRegion>(Obj.getAsRegion());
if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) {
       assert(C.wasInlined || !Set.contains(Sym));
       Set = F.add(Set, Sym);
       State = State->set<RawPtrMap>(ObjRegion, Set);
       C.addTransition(State);
     }
     return;
   }
 
-  if (isa<CXXDestructorCall>(ICall)) {
+  if (mayInvalidateBuffer(Call)) {
     if (const PtrSet *PS = State->get<RawPtrMap>(ObjRegion)) {
       // Mark all pointer symbols associated with the deleted object released.
       const Expr *Origin = Call.getOriginExpr();
       for (const auto Symbol : *PS) {
         // NOTE: `Origin` may be null, and will be stored so in the symbol's
         // `RefState` in MallocChecker's `RegionState` program state map.
         State = allocation_state::markReleased(State, Symbol, Origin);
       }
for (const auto Entry : RPM) {
     }
     if (const PtrSet *OldSet = State->get<RawPtrMap>(Entry.first)) {
       PtrSet CleanedUpSet = *OldSet;
       for (const auto Symbol : Entry.second) {
         if (!SymReaper.isLive(Symbol))
           CleanedUpSet = F.remove(CleanedUpSet, Symbol);
       }
       State = CleanedUpSet.isEmpty()
                   ? State->remove<RawPtrMap>(Entry.first)
                   : State->set<RawPtrMap>(Entry.first, CleanedUpSet);
     }
   }
   C.addTransition(State);
 }
 
 std::shared_ptr<PathDiagnosticPiece>
 DanglingInternalBufferChecker::DanglingBufferBRVisitor::VisitNode(
     const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC,

lib/StaticAnalyzer/Checkers/MallocChecker.cpp

if (isAllocated(RS, RSPrev, S)) {
         case AF_Alloca:
         case AF_Malloc:
         case AF_CXXNew:
         case AF_CXXNewArray:
         case AF_IfNameIndex:
           Msg = "Memory is released";
           break;
         case AF_InternalBuffer:
-          Msg = "Internal buffer is released because the object was destroyed";
+          Msg = "Method call is allowed to invalidate the internal buffer";
           break;
         case AF_None:
           llvm_unreachable("Unhandled allocation family!");
       }
       StackHint = new StackHintGeneratorForSymbol(Sym,
                                              "Returning; memory was released");
 
       // See if we're releasing memory while inlining a destructor

test/Analysis/dangling-internal-buffer.cpp

 //RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.DanglingInternalBuffer %s -analyzer-output=text -verify
 
 namespace std {
 
-template< typename CharT >
+typedef int size_type;
+
+template <typename CharT>
 class basic_string {
 public:
+  basic_string();
+  basic_string(const CharT *s);
+
   ~basic_string();
+  void clear();
+
+  basic_string &operator=(const basic_string &str);
+  basic_string &operator+=(const basic_string &str);
+
   const CharT *c_str() const;
   const CharT *data() const;
   CharT *data();
+
+  basic_string &append(size_type count, CharT ch);
+  basic_string &assign(size_type count, CharT ch);
+  basic_string &erase(size_type index, size_type count);
+  basic_string &insert(size_type index, size_type count, CharT ch);
+  basic_string &replace(size_type pos, size_type count, const basic_string &str);
+  void pop_back();
+  void push_back(CharT ch);
+  void reserve(size_type new_cap);
+  void resize(size_type count);
+  void shrink_to_fit();
+  void swap(basic_string &other);
 };
 
 typedef basic_string<char> string;
 typedef basic_string<wchar_t> wstring;
 typedef basic_string<char16_t> u16string;
 typedef basic_string<char32_t> u32string;
 
 } // end namespace std
 
 void consume(const char *) {}
 void consume(const wchar_t *) {}
 void consume(const char16_t *) {}
 void consume(const char32_t *) {}
 
-void deref_after_scope_char_cstr() {
-  const char *c;
+void deref_after_scope_char(bool cond) {
+  const char *c, *d;
   {
     std::string s;
     c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
+    d = s.data();  // expected-note {{Pointer to dangling buffer was obtained here}}
+  }                // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  // expected-note@-1 {{Method call is allowed to invalidate the internal buffer}}
   std::string s;
   const char *c2 = s.c_str();
+  if (cond) {
+    // expected-note@-1 {{Assuming 'cond' is not equal to 0}}
+    // expected-note@-2 {{Taking true branch}}
+    // expected-note@-3 {{Assuming 'cond' is 0}}
+    // expected-note@-4 {{Taking false branch}}
     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_char_data() {
-  const char *c;
-  {
-    std::string s;
-    c = s.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
-  std::string s;
-  const char *c2 = s.data();
-  consume(c); // expected-warning {{Use of memory after it is freed}}
+  } else {
+    consume(d); // expected-warning {{Use of memory after it is freed}}
     // expected-note@-1 {{Use of memory after it is freed}}
   }
+}
 
 void deref_after_scope_char_data_non_const() {
   char *c;
   {
     std::string s;
     c = s.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
+  }               // expected-note {{Method call is allowed to invalidate the internal buffer}}
   std::string s;
   char *c2 = s.data();
   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_cstr() {
-  const wchar_t *w;
+void deref_after_scope_wchar_t(bool cond) {
+  const wchar_t *c, *d;
   {
-    std::wstring ws;
-    w = ws.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
-  std::wstring ws;
-  const wchar_t *w2 = ws.c_str();
-  consume(w); // expected-warning {{Use of memory after it is freed}}
+    std::wstring s;
+    c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
+    d = s.data();  // expected-note {{Pointer to dangling buffer was obtained here}}
+  }                // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  // expected-note@-1 {{Method call is allowed to invalidate the internal buffer}}
+  std::wstring s;
+  const wchar_t *c2 = s.c_str();
+  if (cond) {
+    // expected-note@-1 {{Assuming 'cond' is not equal to 0}}
+    // expected-note@-2 {{Taking true branch}}
+    // expected-note@-3 {{Assuming 'cond' is 0}}
+    // expected-note@-4 {{Taking false branch}}
+    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_data() {
-  const wchar_t *w;
-  {
-    std::wstring ws;
-    w = ws.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
-  std::wstring ws;
-  const wchar_t *w2 = ws.data();
-  consume(w); // expected-warning {{Use of memory after it is freed}}
+  } else {
+    consume(d); // expected-warning {{Use of memory after it is freed}}
     // expected-note@-1 {{Use of memory after it is freed}}
   }
+}
 
 void deref_after_scope_char16_t_cstr() {
   const char16_t *c16;
   {
     std::u16string s16;
     c16 = s16.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
+  }                    // expected-note {{Method call is allowed to invalidate the internal buffer}}
   std::u16string s16;
   const char16_t *c16_2 = s16.c_str();
   consume(c16); // expected-warning {{Use of memory after it is freed}}
   // expected-note@-1 {{Use of memory after it is freed}}
 }
 
 void deref_after_scope_char32_t_data() {
   const char32_t *c32;
   {
     std::u32string s32;
     c32 = s32.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
+  }                   // expected-note {{Method call is allowed to invalidate the internal buffer}}
   std::u32string s32;
   const char32_t *c32_2 = s32.data();
   consume(c32); // expected-warning {{Use of memory after it is freed}}
   // expected-note@-1 {{Use of memory after it is freed}}
 }
 
 void multiple_symbols(bool cond) {
   const char *c1, *d1;
   {
     std::string s1;
     c1 = s1.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
-    d1 = s1.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
+    d1 = s1.data();  // expected-note {{Pointer to dangling buffer was obtained here}}
     const char *local = s1.c_str();
     consume(local); // no-warning
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
-  // expected-note@-1 {{Internal buffer is released because the object was destroyed}}
+  }                 // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  // expected-note@-1 {{Method call is allowed to invalidate the internal buffer}}
   std::string s2;
   const char *c2 = s2.c_str();
   if (cond) {
     // expected-note@-1 {{Assuming 'cond' is not equal to 0}}
     // expected-note@-2 {{Taking true branch}}
     // expected-note@-3 {{Assuming 'cond' is 0}}
     // expected-note@-4 {{Taking false branch}}
     consume(c1); // expected-warning {{Use of memory after it is freed}}
     // expected-note@-1 {{Use of memory after it is freed}}
   } else {
     consume(d1); // expected-warning {{Use of memory after it is freed}}
-  } // expected-note@-1 {{Use of memory after it is freed}}
+  }              // expected-note@-1 {{Use of memory after it is freed}}
 }
 
-void deref_after_scope_cstr_ok() {
+void deref_after_equals() {
+  const char *c;
+  std::string s = "hello";
+  c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
+  s = "world";   // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  consume(c);    // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_plus_equals() {
+  const char *c;
+  std::string s = "hello";
+  c = s.data();  // expected-note {{Pointer to dangling buffer was obtained here}}
+  s += " world"; // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  consume(c);    // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_clear() {
   const char *c;
   std::string s;
-  {
-    c = s.c_str();
-  }
-  consume(c); // no-warning
+  c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
+  s.clear();     // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  consume(c);    // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+
+void deref_after_append() {
+  const char *c;
+  std::string s = "hello";
+  c = s.c_str();    // expected-note {{Pointer to dangling buffer was obtained here}}
+  s.append(2, 'x'); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  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_data_ok() {
+void deref_after_assign() {
   const char *c;
   std::string s;
+  c = s.data();     // expected-note {{Pointer to dangling buffer was obtained here}}
+  s.assign(4, 'a'); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  consume(c);       // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_erase() {
+  const char *c;
+  std::string s = "hello";
+  c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
+  s.erase(0, 2); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  consume(c);    // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_insert() {
+  const char *c;
+  std::string s = "ello";
+  c = s.c_str();       // expected-note {{Pointer to dangling buffer was obtained here}}
+  s.insert(0, 1, 'h'); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  consume(c);          // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_replace() {
+  const char *c;
+  std::string s = "hello world";
+  c = s.c_str();             // expected-note {{Pointer to dangling buffer was obtained here}}
+  s.replace(6, 5, "string"); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  consume(c);                // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_pop_back() {
+  const char *c;
+  std::string s;
+  c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
+  s.pop_back();  // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  consume(c);    // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_push_back() {
+  const char *c;
+  std::string s;
+  c = s.data();     // expected-note {{Pointer to dangling buffer was obtained here}}
+  s.push_back('c'); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  consume(c);       // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_reserve() {
+  const char *c;
+  std::string s;
+  c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
+  s.reserve(5);  // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  consume(c);    // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_resize() {
+  const char *c;
+  std::string s;
+  c = s.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
+  s.resize(5);  // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  consume(c);   // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_shrink_to_fit() {
+  const char *c;
+  std::string s;
+  c = s.data();      // expected-note {{Pointer to dangling buffer was obtained here}}
+  s.shrink_to_fit(); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  consume(c);        // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_swap() {
+  const char *c;
+  std::string s1, s2;
+  c = s1.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
+  s1.swap(s2);   // expected-note {{Method call is allowed to invalidate the internal buffer}}
+  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_ok(bool cond) {
+  const char *c, *d;
+  std::string s;
   {
-    c = s.data();
+    c = s.c_str();
+    d = s.data();
   }
+  if (cond)
     consume(c); // no-warning
+  else
+    consume(d); // no-warning
 }