[Analyzer] Support note tags for smart ptr checker

[vrnithinkumar]

Summary

• Added support for note tags for null smart_ptr reporting
• Updated the warning message with variable and smart_ptr type
• Added tests for note tags

[NoQ]

clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp

100

Please getNameAsString() because getName() crashes on anonymous declarations (eg., lambda captures, anonymous nested structs/unions, etc.).

109–110

Aha, this time you checked for anonymous declarations! Good.

That said, i'm not sure type is actually useful for this warning, because they're roughly all the same.

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

149

I feel this is a bit over-engineered. There's no need to create an enum and later convert it into a string when you can capture a string directly. Like, this entire "details" structure of your, it should be just captures instead, and your strings would make it immediately obvious what kind of note is emitted:

C.addTransition(State, getNoteTag([R](PathSensitiveBugReport &BR) {
  if (&BR.getBugType() != &NullDereferenceBugType || !R->isInteresting())
    return "";
 
  return Twine("Default constructed smart pointer '") + getSmartPtrName(R) + "' is null";
}));

Hmm, maybe we should also provide an overload with lambdas of signature`void(PathSensitiveBugReport &BR, llvm::raw_ostream OS)` so that to make the same one-liners possible with streams? Something like this:

class CheckerContext {
// ...
  NoteTag *getNoteTag(std::function<void(PathSensitiveBugReport &BR, llvm::raw_ostream OS)> f) {
    return getNoteTag([](PathSensitiveBugReport &BR) -> std::string {
      llvm::SmallString<128> Str;
      llvm::raw_svector_ostream OS(Str);
      f(BR, OS);
      return OS.str();
    });
  }
// ...
};

Then you could do:

C.addTransition(State, getNoteTag([R](PathSensitiveBugReport &BR) {
  if (&BR.getBugType() != &NullDereferenceBugType || !R->isInteresting())
    return;

  OS << "Default constructed smart pointer '" << getSmartPtrName(R) << "' is null";
}));

344–348

Ok, note that note tags are attached to nodes independently of bug reports; when the report is thrown, only then we know what are the smart pointers that should be explained.

So there are two checks that you should do here:

1. Check that the bug report is emitted by your checker (eg., by comparing bug types). If not, don't add notes.

2. Check that the region about which the note speaks is related to your report (i.e., it's not a completely unrelated smart pointer). You can do that by marking the smart pointer as "interesting" (i.e., PathSensitiveBugReport::markIntersting()) when you emit the report, and then in the lambda you check whether the smart pointer is interesting before you emit a note. Additionally, you can carry over interestingness when smart pointers are copied.

This is what i was trying to accomplish with this code snippet that i included in the examples in the other comment:

if (&BR.getBugType() != &NullDereferenceBugType || !R->isInteresting())
  return "";

[NoQ]

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

149

(forgot , llvm::raw_ostream OS in the last snippet; probably forgot a bunch of other stuff because i didn't try to actually compile these snippets)

344–348

(i strongly recommend having test cases for both of these issues)

[vrnithinkumar]

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

344–348

I was stuck on how to check the 2 cases from SmartPtrModeling.

1. I was not able to figure out how to access NullDereferenceBugType defined in the SmartPtrChecker in SmartPtrModeling to check &BR.getBugType() != &NullDereferenceBugType. Since NullDereferenceBugType is part of the SmartPtrChecker I could not access it from PathSensitiveBugReport. One way I figured out is to use getCheckerName() on BugType and compare the string. I feel this one as little hacky.

2. I got stuck on how will we implement the !R->isInteresting() in case of the bug report is added by some other checker on some other region. For below test case, bug report is added on a raw pointer by CallAndMessageChecker and the !R->isInteresting() will not satisfy and we will not be adding note tags where unique_ptr is released. I tried getting the LHS region for A *AP = P.release(); assignment operation and check if the region is interesting but not sure whether its gonna work for some complex assignment cases.

void derefOnReleasedNullRawPtr() {
  std::unique_ptr<A> P;
  A *AP = P.release(); // expected-note {{'AP' initialized to a null pointer value}}
  // expected-note@-1 {{Smart pointer 'P' is released and set to null}}
  AP->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
  // expected-note@-1{{Called C++ object pointer is null}}
}

[NoQ]

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

344–348

Since NullDereferenceBugType is part of the SmartPtrChecker I could not access it from PathSensitiveBugReport.

You shouldn't be accessing it from the bug report, you should be accessing it from the lambda. See the example code snippets in D84600#inline-779418

For below test case, bug report is added on a raw pointer by CallAndMessageChecker and the !R->isInteresting() will not satisfy and we will not be adding note tags where unique_ptr is released.

That's an interesting question (no pun intended). The way i imagine this working is: the note tag for .release() should try to figure out whether the raw pointer is tracked and mark the smart pointer as interesting based on that. If the raw pointer was a symbol that would have been easy (either null dereference checker or trackExpressionValue() could mark it as interesting). But for concrete null pointer this won't work.

Maybe we should consider introducing interesting expressions. I.e., when trackExpressionValue() reaches the call-expression P.release(), it has to stop there. But if it also marked the call-expression as interesting, the note tag provided by the checker could read that interestingness information and act upon it by marking the smart pointer region as interesting.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

344–348

That's an interesting question

I'd rather make a separate commit for this endeavor because it sounds pretty nasty.

[vrnithinkumar]

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

344–348

You shouldn't be accessing it from the bug report, you should be accessing it from the lambda. See the example code snippets in D84600#inline-779418

Sorry, I am still confused how will the lambda defined in the SmartPtrModeling can capture the NullDereferenceBugType from SmartPtrChecker?

[NoQ]

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

344–348

Lambda can capture anything that's available in its lexical context. For capturing a field of the surrounding class, i believe you should capture this. See how other checkers do that.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

344–348

Wait, i see what you mean. We're in the wrong checker!

Let's turn the null dereference bug type into an inter-checker API then? Make a header, like Move.h but say NullDereference.h, and introduce a const BugType *getNullDereferenceBugType() that we could invoke in the lambda.

Because the bug type is created with checker registration, it might make sense to maintain a static global pointer to the bug type that'll initially be null but initialized during registration.

[vrnithinkumar]

• Addressing review comments

clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp

100

Changed to use getNameAsString()

109–110

Okay. I was trying to be consistent with MoveChecker.
Just removed smart pointer type.

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

149

• Added overload for getNoteTag with lambda accepting a llvm::raw_ostream
• Used the overload instead of SmartPtrTagDetails structure to add tags.
• Removed the SmartPtrTagDetails and changed related code.

344–348

(i strongly recommend having test cases for both of these issues)

Added the tests

344–348

That's an interesting question

I'd rather make a separate commit for this endeavor because it sounds pretty nasty.

Will make a separate commit

344–348

• Created NullDereference.h and added const BugType *getNullDereferenceBugType()
• Added a static global pointer to the bug type.
• Added the check if the region is interesting and bug type is matching.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/NullDereference.h

21 ↗

(On Diff #282771)

Namespaces are traditionally snake_case rather than camelCase.

clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp

102

Wait, i don't understand again. You're taking the bug type from that checker and using it in that checker. Why did you need to make it global? I thought your problem was about capturing the bug type from the raw null dereference checker?

109–110

These were important in MoveChecker because use-after-move rules for smart pointers are different from use-after-move rules of any other class in the standard library and the checker has to behave differently (https://wiki.sei.cmu.edu/confluence/x/O3s-BQ).

[vrnithinkumar]

clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp

102

Wait, i don't understand again. You're taking the bug type from that checker and using it in that checker. Why did you need to make it global? I thought your problem was about capturing the bug type from the raw null dereference checker?

The check for bug type is in SmartPtrModeling but the bug type is defined in SmartPtrChecker

109–110

Oh
That makes sense.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp

102

Aha, ok, i misunderstood again. So i guess we didn't need a new header then. That said, it's an interesting layering violation that we encounter in this design: it used to be up to the checker to attach a visitor and so should be activating a note tag, but note tags are part of modeling, not checking.

I guess that's just how it is and it's the responsibility of the checkers to inform the modeling about bug types. I guess the ultimate API could look like BugReport->activateNoteTag(NoteTagTag TT) (where TT is a crying smiley that cries about "tag tags" T_T). But that's a story for another time.

[NoQ]

Generally i think this is shaping up nicely, with the couple of minor nits addressed i'm happy to land this. Like, there are a few cornercases that we could address but we could do that in follow-up patches. Folks, do you see anything else here?

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

129

You never ever check for this case. Therefore this function entirely boils down to Call.getArgExpr(0) which is shorter than getFirstArgExpr(Call) anyway.

[xazax.hun]

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

312

The callback is taken is an rvalue reference in other getNoteTag APIs. I think these overloads should be consistent.
Also, I wonder if the caller should be able to manipulate the buffer size of the small string (as a template parameter), but I do not have strong feelings about this.

As a side note, since Clang is using C++14, maybe the lambda captures in the getNoteTag overloads above should utilize it and capture the callback by move. This is more of a note to ourselves independent of this patch.

Side note 2: maybe a modernize tidy check would be useful to discover where rvalue references are captured by value in lambdas?

clang/lib/StaticAnalyzer/Checkers/NullDereference.h

23 ↗

(On Diff #282771)

I think this comment does not really add much value.

clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp

102

I hope we will be able to figure out a nicer solution at some point. But I do not have a better alternative now, so I am ok with committing as is.

The API Artem is suggesting looks good to me (although it is out of scope for the GSoC). But I think that might not solve the problem of how multiple checkers should share the information about those tags. (Or at least I do not see how.)

Note that if we had both checks in the same file we might be able to work this problem around using CheckerManager::getChecker. I am not suggesting merging them, only wanted to point out.

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

132

I wonder whether MemRegion::printPretty is what we want here.

181

I am a bit confused. As far as I understand we ALWAYS add the note tag below, regardless of the first argument being null.
I think I do understand that we only trigger this note tag when there is a null dereference but consider the code below:

void f(int *p) {
  std::unique_ptr<int> u(p); // p is not always null here.
  if (!p) {
    *u; // p is always null here
  }
}

Do we ant to output the message that the smart pointer is constructed using a null value? While this is technically true, there is a later event in the path that uncovers the fact that p is null.

@NoQ what do you think? I do not have any strong feelings about this, I only wonder how users would react to a bug report like that.

334

I am a bit unhappy with this function.
I feel like it does not really have a purpose. It returns the null constant for default constructor calls, for every other case it just returns the first argument. The branch we have in this function would completely go away if we would inline this to all the call sites and the result would be a single line each place. I'd rather just remove this function.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

181

Indeed, nice!! We can only proclaim the pointer to be null if it's known to be null in the current state. I.e., if previous steps of the report indicated that it's null. If it's discovered to be null later, we cannot call it null yet. I think we should check the current state and say "is constructed using a null value" if it's already null or simply "is constructed" if it's not yet known to be null.

[vrnithinkumar]

clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp

102

So i guess we didn't need a new header then.

So we should remove the NullDereference.h and add the inter checker api to get the BugType to SmartPtr.h?

I guess the ultimate API could look like BugReport->activateNoteTag(NoteTagTag TT)

Do we have to make these api changes on this review? Or some follow up changes?

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

334

Also we have to assert that we are adding a pointer value to TrackedRegionMap.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp

102

So we should remove the NullDereference.h and add the inter checker api to get the BugType to SmartPtr.h?

Yes.

Do we have to make these api changes on this review? Or some follow up changes?

You don't have to do this at all; i think you have enough stuff on your plate. But if you like you can put up another patch later.

[vrnithinkumar]

• Changes from review comments

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

312

Changed to rvalue reference.

clang/lib/StaticAnalyzer/Checkers/NullDereference.h

21 ↗

(On Diff #282771)

removed the new namespace

23 ↗

(On Diff #282771)

Yeah.
removed it.

clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp

102

Removed the new header file and added to SmartPtr.h

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

129

removed it and inlined.

132

Yeah Thanks.
This is what we exactly needed.

181

Added a check if the value is null or not and add message based on that.

334

Removed the function and inlined it

[xazax.hun]

LGTM, thanks!

I think all the problems that left should be solved in a separate patch or even out of the scope for the GSoC.

clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp

33

I find this comment redundant as well. It just repeats what is already evident from the code.

[NoQ]

clang/test/Analysis/smart-ptr-text-output.cpp

66

Such note is unnecessary. We don't care what happens to P after it's released; we only care about its old value.

80

Instead of commenting out tests, i recommend testing the incorrect behavior (with a FIXME comment telling us why it's incorrect). This way we'll be notified when the test is fixed, accidentally or intentionally, and also generally that's more testing for everybody.

[NoQ]

clang/test/Analysis/smart-ptr-text-output.cpp

64

Unlike the next line, this line deserves a note that P holds a null value.

[Szelethus]

Layering violations are a running theme in the analyzer -- CheckerRegistry and the entire MallocChecker fiasco are two glaring examples. Luckily, this isn't a severe case so I wouldn't worry about it much.

I've been following your patches, but them mentors answer way ahead of me and are very thorough :^) Best of luck with the finale!

[vrnithinkumar]

• Review comment changes

clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp

33

removed

clang/test/Analysis/smart-ptr-text-output.cpp

64

Added a FIXEM to add note "Default constructed smart pointer 'P' is null"

66

Removed this.

80

I have commented out this since right now std::swap is using unique_ptr.swap.
So note tag for std::swap is added in header file system-header-simulator-cxx.h
eg:
system-header-simulator-cxx.h Line 978: Swapped null smart pointer 'PNull' with smart pointer 'P'

[vrnithinkumar]

In D84600#2203604, @Szelethus wrote:

Layering violations are a running theme in the analyzer -- CheckerRegistry and the entire MallocChecker fiasco are two glaring examples. Luckily, this isn't a severe case so I wouldn't worry about it much.

I've been following your patches, but them mentors answer way ahead of me and are very thorough :^) Best of luck with the finale!

Thanks for sharing checker dependency talk.
Also fo suggesting to separate checker and modeling from beginning. :)

[vrnithinkumar]

clang/test/Analysis/smart-ptr-text-output.cpp

80

• Is it okay to add a expected-note on system-header-simulator-cxx.h?

[NoQ]

clang/test/Analysis/smart-ptr-text-output.cpp

80

Aha, i see.

It's obviously not ok because the header is included by multiple tests but only some tests will have the note show up.

The usual solution is to put the expected-note comment in the current test file but make it refer to the header. Here's doc from https://clang.llvm.org/doxygen/classclang_1_1VerifyDiagnosticConsumer.html:

If the diagnostic is generated in a separate file, for example in a shared header file, it may be beneficial to be able to declare the file in which the diagnostic will appear, rather than placing the expected-* directive in the actual file itself. This can be done using the following syntax:

// expected-error@path/include.h:15 {{error message}}

It still is a bit of a problem that you have to hardcode the line number (so everybody who modifies the header above your note will have to update your test) but that's not a big deal.

[vrnithinkumar]

• Updating test with tags from header file

clang/test/Analysis/smart-ptr-text-output.cpp

80

Thanks!
Added the tags for the line from header file.

[NoQ]

Let's land this! Damn, we've learned a lot from this patch.

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

public:
   ///        bug path significantly shorter.
   const NoteTag *getNoteTag(StringRef Note, bool IsPrunable = false) {
     return getNoteTag(
         [Note](BugReporterContext &,
                PathSensitiveBugReport &) { return std::string(Note); },
         IsPrunable);
   }
 
+  /// A shorthand version of getNoteTag that accepts a lambda with stream for
+  /// note.
+  ///
+  /// @param Cb Callback with 'BugReport &' and 'llvm::raw_ostream &'.
+  /// @param IsPrunable Whether the note is prunable. It allows BugReporter
+  ///        to omit the note from the report if it would make the displayed
+  ///        bug path significantly shorter.
+  const NoteTag *getNoteTag(
+      std::function<void(PathSensitiveBugReport &BR, llvm::raw_ostream &OS)> &&Cb,
+      bool IsPrunable = false) {
+    return getNoteTag(
+        [Cb](PathSensitiveBugReport &BR) -> std::string {
+          llvm::SmallString<128> Str;
+          llvm::raw_svector_ostream OS(Str);
+          Cb(BR, OS);
+          return std::string(OS.str());
+        },
+        IsPrunable);
+  }
+
   /// Returns the word that should be used to refer to the declaration
   /// in the report.
   StringRef getDeclDescription(const Decl *D);
 
   /// Get the declaration of the called function (path-sensitive).
   const FunctionDecl *getCalleeDecl(const CallExpr *CE) const;
 
   /// Get the name of the called function (path-sensitive).

clang/lib/StaticAnalyzer/Checkers/SmartPtr.h

 namespace smartptr {
 
 /// Returns true if the event call is on smart pointer.
 bool isStdSmartPtrCall(const CallEvent &Call);
 
 /// Returns whether the smart pointer is null or not.
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion);
 
+const BugType *getNullDereferenceBugType();
+
 } // namespace smartptr
 } // namespace ento
 } // namespace clang
 
 #endif // LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_SMARTPTR_H

clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp

 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
+#include "llvm/ADT/StringRef.h"
 
 using namespace clang;
 using namespace ento;
 
 namespace {
-class SmartPtrChecker : public Checker<check::PreCall> {
-  BugType NullDereferenceBugType{this, "Null SmartPtr dereference",
-                                 "C++ Smart Pointer"};
 
+static const BugType *NullDereferenceBugTypePtr;
+
+class SmartPtrChecker : public Checker<check::PreCall> {
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+  BugType NullDereferenceBugType{this, "Null SmartPtr dereference",
+                                 "C++ Smart Pointer"};
 
 private:
-  void reportBug(CheckerContext &C, const CallEvent &Call) const;
+  void reportBug(CheckerContext &C, const MemRegion *DerefRegion,
+                 const CallEvent &Call) const;
+  void explainDereference(llvm::raw_ostream &OS, const MemRegion *DerefRegion,
+                          const CallEvent &Call) const;
 };
 } // end of anonymous namespace
 
+// Define the inter-checker API.
+namespace clang {
+namespace ento {
+namespace smartptr {
+
+const BugType *getNullDereferenceBugType() { return NullDereferenceBugTypePtr; }
+
+} // namespace smartptr
+} // namespace ento
+} // namespace clang
+
 void SmartPtrChecker::checkPreCall(const CallEvent &Call,
                                    CheckerContext &C) const {
   if (!smartptr::isStdSmartPtrCall(Call))
     return;
   ProgramStateRef State = C.getState();
   const auto *OC = dyn_cast<CXXMemberOperatorCall>(&Call);
   if (!OC)
     return;
   const MemRegion *ThisRegion = OC->getCXXThisVal().getAsRegion();
   if (!ThisRegion)
     return;
 
   OverloadedOperatorKind OOK = OC->getOverloadedOperator();
   if (OOK == OO_Star || OOK == OO_Arrow) {
     if (smartptr::isNullSmartPtr(State, ThisRegion))
-      reportBug(C, Call);
+      reportBug(C, ThisRegion, Call);
   }
 }
 
-void SmartPtrChecker::reportBug(CheckerContext &C,
+void SmartPtrChecker::reportBug(CheckerContext &C, const MemRegion *DerefRegion,
                                 const CallEvent &Call) const {
   ExplodedNode *ErrNode = C.generateErrorNode();
   if (!ErrNode)
     return;
-
-  auto R = std::make_unique<PathSensitiveBugReport>(
-      NullDereferenceBugType, "Dereference of null smart pointer", ErrNode);
+  llvm::SmallString<128> Str;
+  llvm::raw_svector_ostream OS(Str);
+  explainDereference(OS, DerefRegion, Call);
+  auto R = std::make_unique<PathSensitiveBugReport>(NullDereferenceBugType,
+                                                    OS.str(), ErrNode);
+  R->markInteresting(DerefRegion);
   C.emitReport(std::move(R));
 }
 
+void SmartPtrChecker::explainDereference(llvm::raw_ostream &OS,
+                                         const MemRegion *DerefRegion,
+                                         const CallEvent &Call) const {
+  OS << "Dereference of null smart pointer ";
+  DerefRegion->printPretty(OS);
+}
+
 void ento::registerSmartPtrChecker(CheckerManager &Mgr) {
-  Mgr.registerChecker<SmartPtrChecker>();
+  SmartPtrChecker *Checker = Mgr.registerChecker<SmartPtrChecker>();
+  NullDereferenceBugTypePtr = &Checker->NullDereferenceBugType;
 }
 
 bool ento::shouldRegisterSmartPtrChecker(const CheckerManager &mgr) {
   const LangOptions &LO = mgr.getLangOpts();
   return LO.CPlusPlus;
 }

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

 //===----------------------------------------------------------------------===//
 
 #include "Move.h"
 #include "SmartPtr.h"
 
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/Type.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include <string>
 
 using namespace clang;
 using namespace ento;
 
 namespace {
 class SmartPtrModeling
     : public Checker<eval::Call, check::DeadSymbols, check::RegionChanges> {
 
   bool isNullAfterMoveMethod(const CallEvent &Call) const;
 
 public:
   // Whether the checker should model for null dereferences of smart pointers.
   DefaultBool ModelSmartPtrDereference;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
   ProgramStateRef
   checkRegionChanges(ProgramStateRef State,
                      const InvalidatedSymbols *Invalidated,
                      ArrayRef<const MemRegion *> ExplicitRegions,
                      ArrayRef<const MemRegion *> Regions,
                      const LocationContext *LCtx, const CallEvent *Call) const;
 
 private:
-  ProgramStateRef updateTrackedRegion(const CallEvent &Call, CheckerContext &C,
-                                      const MemRegion *ThisValRegion) const;
   void handleReset(const CallEvent &Call, CheckerContext &C) const;
   void handleRelease(const CallEvent &Call, CheckerContext &C) const;
   void handleSwap(const CallEvent &Call, CheckerContext &C) const;
 
   using SmartPtrMethodHandlerFn =
       void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
   CallDescriptionMap<SmartPtrMethodHandlerFn> SmartPtrMethodHandlers{
       {{"reset"}, &SmartPtrModeling::handleReset},
static ProgramStateRef updateSwappedRegion(ProgramStateRef State,
   return State;
 }
 
 bool SmartPtrModeling::isNullAfterMoveMethod(const CallEvent &Call) const {
   // TODO: Update CallDescription to support anonymous calls?
   // TODO: Handle other methods, such as .get() or .release().
   // But once we do, we'd need a visitor to explain null dereferences
   // that are found via such modeling.
   const auto *CD = dyn_cast_or_null<CXXConversionDecl>(Call.getDecl());
   return CD && CD->getConversionType()->isBooleanType();
 }
 
 bool SmartPtrModeling::evalCall(const CallEvent &Call,
                                 CheckerContext &C) const {
 
+  ProgramStateRef State = C.getState();
   if (!smartptr::isStdSmartPtrCall(Call))
     return false;
 
   if (isNullAfterMoveMethod(Call)) {
-    ProgramStateRef State = C.getState();
     const MemRegion *ThisR =
         cast<CXXInstanceCall>(&Call)->getCXXThisVal().getAsRegion();
 
     if (!move::isMovedFrom(State, ThisR)) {
       // TODO: Model this case as well. At least, avoid invalidation of
       // globals.
       return false;
     }
 
     // TODO: Add a note to bug reports describing this decision.
     C.addTransition(
         State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
                         C.getSValBuilder().makeZeroVal(Call.getResultType())));
     return true;
   }
 
   if (!ModelSmartPtrDereference)
     return false;
 
   if (const auto *CC = dyn_cast<CXXConstructorCall>(&Call)) {
     if (CC->getDecl()->isCopyOrMoveConstructor())
       return false;
 
-    const MemRegion *ThisValRegion = CC->getCXXThisVal().getAsRegion();
-    if (!ThisValRegion)
+    const MemRegion *ThisRegion = CC->getCXXThisVal().getAsRegion();
+    if (!ThisRegion)
       return false;
 
-    auto State = updateTrackedRegion(Call, C, ThisValRegion);
-    C.addTransition(State);
+    if (Call.getNumArgs() == 0) {
+      auto NullVal = C.getSValBuilder().makeNull();
+      State = State->set<TrackedRegionMap>(ThisRegion, NullVal);
+
+      C.addTransition(
+          State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
+                                           llvm::raw_ostream &OS) {
+            if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
+                !BR.isInteresting(ThisRegion))
+              return;
+            OS << "Default constructed smart pointer ";
+            ThisRegion->printPretty(OS);
+            OS << " is null";
+          }));
+    } else {
+      const auto *TrackingExpr = Call.getArgExpr(0);
+      assert(TrackingExpr->getType()->isPointerType() &&
+             "Adding a non pointer value to TrackedRegionMap");
+      auto ArgVal = Call.getArgSVal(0);
+      State = State->set<TrackedRegionMap>(ThisRegion, ArgVal);
+
+      C.addTransition(State, C.getNoteTag([ThisRegion, TrackingExpr,
+                                           ArgVal](PathSensitiveBugReport &BR,
+                                                   llvm::raw_ostream &OS) {
+        if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
+            !BR.isInteresting(ThisRegion))
+          return;
+        bugreporter::trackExpressionValue(BR.getErrorNode(), TrackingExpr, BR);
+        OS << "Smart pointer ";
+        ThisRegion->printPretty(OS);
+        if (ArgVal.isZeroConstant())
+          OS << " is constructed using a null value";
+        else
+          OS << " is constructed";
+      }));
+    }
     return true;
   }
 
   const SmartPtrMethodHandlerFn *Handler = SmartPtrMethodHandlers.lookup(Call);
   if (!Handler)
     return false;
   (this->**Handler)(Call, C);
 
ProgramStateRef SmartPtrModeling::checkRegionChanges(
   for (const auto *Region : Regions)
     RegionMap = removeTrackedSubregions(RegionMap, RegionMapFactory,
                                         Region->getBaseRegion());
   return State->set<TrackedRegionMap>(RegionMap);
 }
 
 void SmartPtrModeling::handleReset(const CallEvent &Call,
                                    CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
   const auto *IC = dyn_cast<CXXInstanceCall>(&Call);
   if (!IC)
     return;
 
-  const MemRegion *ThisValRegion = IC->getCXXThisVal().getAsRegion();
-  if (!ThisValRegion)
+  const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
+  if (!ThisRegion)
     return;
-  auto State = updateTrackedRegion(Call, C, ThisValRegion);
-  C.addTransition(State);
+
+  assert(Call.getArgExpr(0)->getType()->isPointerType() &&
+         "Adding a non pointer value to TrackedRegionMap");
+  State = State->set<TrackedRegionMap>(ThisRegion, Call.getArgSVal(0));
+  const auto *TrackingExpr = Call.getArgExpr(0);
+  C.addTransition(
+      State, C.getNoteTag([ThisRegion, TrackingExpr](PathSensitiveBugReport &BR,
+                                                     llvm::raw_ostream &OS) {
+        if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
+            !BR.isInteresting(ThisRegion))
+          return;
+        bugreporter::trackExpressionValue(BR.getErrorNode(), TrackingExpr, BR);
+        OS << "Smart pointer ";
+        ThisRegion->printPretty(OS);
+        OS << " reset using a null value";
+      }));
   // TODO: Make sure to ivalidate the region in the Store if we don't have
   // time to model all methods.
 }
 
 void SmartPtrModeling::handleRelease(const CallEvent &Call,
                                      CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
   const auto *IC = dyn_cast<CXXInstanceCall>(&Call);
   if (!IC)
     return;
 
-  const MemRegion *ThisValRegion = IC->getCXXThisVal().getAsRegion();
-  if (!ThisValRegion)
+  const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
+  if (!ThisRegion)
     return;
 
-  auto State = updateTrackedRegion(Call, C, ThisValRegion);
+  const auto *InnerPointVal = State->get<TrackedRegionMap>(ThisRegion);
 
-  const auto *InnerPointVal = State->get<TrackedRegionMap>(ThisValRegion);
   if (InnerPointVal) {
     State = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
                             *InnerPointVal);
   }
-  C.addTransition(State);
+
+  auto ValueToUpdate = C.getSValBuilder().makeNull();
+  State = State->set<TrackedRegionMap>(ThisRegion, ValueToUpdate);
+
+  C.addTransition(State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
+                                                   llvm::raw_ostream &OS) {
+    if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
+        !BR.isInteresting(ThisRegion))
+      return;
+
+    OS << "Smart pointer ";
+    ThisRegion->printPretty(OS);
+    OS << " is released and set to null";
+  }));
   // TODO: Add support to enable MallocChecker to start tracking the raw
   // pointer.
 }
 
 void SmartPtrModeling::handleSwap(const CallEvent &Call,
                                   CheckerContext &C) const {
   // To model unique_ptr::swap() method.
   const auto *IC = dyn_cast<CXXInstanceCall>(&Call);
const auto *ThisRegionInnerPointerVal =
       State->get<TrackedRegionMap>(ThisRegion);
   const auto *ArgRegionInnerPointerVal =
       State->get<TrackedRegionMap>(ArgRegion);
 
   // Swap the tracked region values.
   State = updateSwappedRegion(State, ThisRegion, ArgRegionInnerPointerVal);
   State = updateSwappedRegion(State, ArgRegion, ThisRegionInnerPointerVal);
 
-  C.addTransition(State);
-}
-
-ProgramStateRef
-SmartPtrModeling::updateTrackedRegion(const CallEvent &Call, CheckerContext &C,
-                                      const MemRegion *ThisValRegion) const {
-  // TODO: Refactor and clean up handling too many things.
-  ProgramStateRef State = C.getState();
-  auto NumArgs = Call.getNumArgs();
-
-  if (NumArgs == 0) {
-    auto NullSVal = C.getSValBuilder().makeNull();
-    State = State->set<TrackedRegionMap>(ThisValRegion, NullSVal);
-  } else if (NumArgs == 1) {
-    auto ArgVal = Call.getArgSVal(0);
-    assert(Call.getArgExpr(0)->getType()->isPointerType() &&
-           "Adding a non pointer value to TrackedRegionMap");
-    State = State->set<TrackedRegionMap>(ThisValRegion, ArgVal);
-  }
-
-  return State;
+  C.addTransition(
+      State, C.getNoteTag([ThisRegion, ArgRegion](PathSensitiveBugReport &BR,
+                                                  llvm::raw_ostream &OS) {
+        if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
+            !BR.isInteresting(ThisRegion))
+          return;
+        BR.markInteresting(ArgRegion);
+        OS << "Swapped null smart pointer ";
+        ArgRegion->printPretty(OS);
+        OS << " with smart pointer ";
+        ThisRegion->printPretty(OS);
+      }));
 }
 
 void ento::registerSmartPtrModeling(CheckerManager &Mgr) {
   auto *Checker = Mgr.registerChecker<SmartPtrModeling>();
   Checker->ModelSmartPtrDereference =
       Mgr.getAnalyzerOptions().getCheckerBooleanOption(
           Checker, "ModelSmartPtrDereference");
 }
 
 bool ento::shouldRegisterSmartPtrModeling(const CheckerManager &mgr) {
   const LangOptions &LO = mgr.getLangOpts();
   return LO.CPlusPlus;
 }

clang/test/Analysis/smart-ptr-text-output.cpp

+// RUN: %clang_analyze_cc1\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr\
+// RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
+// RUN:  -analyzer-output=text -std=c++11 %s -verify=expected
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+class A {
+public:
+  A(){};
+  void foo();
+};
+
+A *return_null() {
+  return nullptr;
+}
+
+void derefAfterDefaultCtr() {
+  std::unique_ptr<A> P; // expected-note {{Default constructed smart pointer 'P' is null}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+void derefAfterCtrWithNull() {
+  A *NullInnerPtr = nullptr; // expected-note {{'NullInnerPtr' initialized to a null pointer value}}
+  std::unique_ptr<A> P(NullInnerPtr); // expected-note {{Smart pointer 'P' is constructed using a null value}}
+  *P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+void derefAfterCtrWithNullVariable() {
+  A *NullInnerPtr = nullptr; // expected-note {{'NullInnerPtr' initialized to a null pointer value}}
+  std::unique_ptr<A> P(NullInnerPtr); // expected-note {{Smart pointer 'P' is constructed using a null value}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+void derefAfterRelease() {
+  std::unique_ptr<A> P(new A());
+  P.release(); // expected-note {{Smart pointer 'P' is released and set to null}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+void derefAfterReset() {
+  std::unique_ptr<A> P(new A());
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+void derefAfterResetWithNull() {
+  A *NullInnerPtr = nullptr; // expected-note {{'NullInnerPtr' initialized to a null pointer value}}
+  std::unique_ptr<A> P(new A());
+  P.reset(NullInnerPtr); // expected-note {{Smart pointer 'P' reset using a null value}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+// FIXME: Fix this test when support is added for tracking raw pointer
+// and mark the smart pointer as interesting based on that and add tags.
+void derefOnReleasedNullRawPtr() {
+  std::unique_ptr<A> P; // FIXME: add note "Default constructed smart pointer 'P' is null"
+  A *AP = P.release(); // expected-note {{'AP' initialized to a null pointer value}}
+  AP->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
+  // expected-note@-1{{Called C++ object pointer is null}}
+}
+
+void derefOnSwappedNullPtr() {
+  std::unique_ptr<A> P(new A());
+  std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+  P.swap(PNull); // expected-note {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
+  PNull->foo(); // No warning.
+  (*P).foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+// FIXME: Fix this test when "std::swap" is modeled seperately.
+void derefOnStdSwappedNullPtr() {
+  std::unique_ptr<A> P;
+  std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+  std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:978 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
+  // expected-note@-1 {{Calling 'swap<A>'}}
+  // expected-note@-2 {{Returning from 'swap<A>'}}
+  PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'PNull'}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+struct StructWithSmartPtr { // expected-note {{Default constructed smart pointer 'S.P' is null}}
+  std::unique_ptr<A> P;
+};
+
+void derefAfterDefaultCtrInsideStruct() {
+  StructWithSmartPtr S; // expected-note {{Calling implicit default constructor for 'StructWithSmartPtr'}}
+  // expected-note@-1 {{Returning from default constructor for 'StructWithSmartPtr'}}
+  S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'S.P'}}
+}
+
+void noNoteTagsForNonInterestingRegion() {
+  std::unique_ptr<A> P; // expected-note {{Default constructed smart pointer 'P' is null}}
+  std::unique_ptr<A> P1; // No note.
+  std::unique_ptr<A> P2; // No note.
+  P1.release(); // No note.
+  P1.reset(); // No note.
+  P1.swap(P2); // No note.
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+void noNoteTagsForNonMatchingBugType() {
+  std::unique_ptr<A> P; // No note.
+  std::unique_ptr<A> P1; // No note.
+  P1 = std::move(P); // expected-note {{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' of type 'std::unique_ptr' [cplusplus.Move]}}
+  // expected-note@-1 {{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+}

clang/test/Analysis/smart-ptr.cpp

 // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection\
 // RUN:   -analyzer-checker cplusplus.Move,alpha.cplusplus.SmartPtr\
 // RUN:   -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
 // RUN:   -std=c++11 -verify %s
 
 #include "Inputs/system-header-simulator-cxx.h"
 
 void clang_analyzer_warnIfReached();
 void clang_analyzer_numTimesReached();
 
 void derefAfterMove(std::unique_ptr<int> P) {
   std::unique_ptr<int> Q = std::move(P);
   if (Q)
     clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
-  *Q.get() = 1;                     // no-warning
+  *Q.get() = 1; // no-warning
   if (P)
     clang_analyzer_warnIfReached(); // no-warning
   // TODO: Report a null dereference (instead).
   *P.get() = 1; // expected-warning {{Method called on moved-from object 'P'}}
 }
 
 // Don't crash when attempting to model a call with unknown callee.
 namespace testUnknownCallee {

 }
 
 void derefOfUnknown(std::unique_ptr<A> P) {
   P->foo(); // No warning.
 }
 
 void derefAfterDefaultCtr() {
   std::unique_ptr<A> P;
-  P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterCtrWithNull() {
   std::unique_ptr<A> P(nullptr);
-  *P; // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  *P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
-void derefAfterCtrWithNullReturnMethod() {
-  std::unique_ptr<A> P(return_null());
-  P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+void derefAfterCtrWithNullVariable() {
+  A *InnerPtr = nullptr;
+  std::unique_ptr<A> P(InnerPtr);
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterRelease() {
   std::unique_ptr<A> P(new A());
   P.release();
   clang_analyzer_numTimesReached(); // expected-warning {{1}}
-  P->foo();                         // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterReset() {
   std::unique_ptr<A> P(new A());
   P.reset();
   clang_analyzer_numTimesReached(); // expected-warning {{1}}
-  P->foo();                         // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterResetWithNull() {
   std::unique_ptr<A> P(new A());
   P.reset(nullptr);
-  P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterResetWithNonNull() {
   std::unique_ptr<A> P;
   P.reset(new A());
   P->foo(); // No warning.
 }
 
 void derefAfterReleaseAndResetWithNonNull() {
   std::unique_ptr<A> P(new A());
   P.release();
   P.reset(new A());
   P->foo(); // No warning.
 }
 
 void derefOnReleasedNullRawPtr() {
   std::unique_ptr<A> P;
   A *AP = P.release();
   AP->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
 }
 
+void derefOnReleasedValidRawPtr() {
+  std::unique_ptr<A> P(new A());
+  A *AP = P.release();
+  AP->foo(); // No warning.
+}
+
 void pass_smart_ptr_by_ref(std::unique_ptr<A> &a);
 void pass_smart_ptr_by_const_ref(const std::unique_ptr<A> &a);
 void pass_smart_ptr_by_rvalue_ref(std::unique_ptr<A> &&a);
 void pass_smart_ptr_by_const_rvalue_ref(const std::unique_ptr<A> &&a);
 void pass_smart_ptr_by_ptr(std::unique_ptr<A> *a);
 void pass_smart_ptr_by_const_ptr(const std::unique_ptr<A> *a);
 
 void regioninvalidationTest() {
   {
     std::unique_ptr<A> P;
     pass_smart_ptr_by_ref(P);
     P->foo(); // no-warning
   }
   {
     std::unique_ptr<A> P;
     pass_smart_ptr_by_const_ref(P);
-    P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+    P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   }
   {
     std::unique_ptr<A> P;
     pass_smart_ptr_by_rvalue_ref(std::move(P));
     P->foo(); // no-warning
   }
   {
     std::unique_ptr<A> P;
     pass_smart_ptr_by_const_rvalue_ref(std::move(P));
-    P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+    P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   }
   {
     std::unique_ptr<A> P;
     pass_smart_ptr_by_ptr(&P);
     P->foo();
   }
   {
     std::unique_ptr<A> P;
     pass_smart_ptr_by_const_ptr(&P);
-    P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+    P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   }
 }
 
 struct StructWithSmartPtr {
   std::unique_ptr<A> P;
 };
 
 void pass_struct_with_smart_ptr_by_ref(StructWithSmartPtr &a);
 void pass_struct_with_smart_ptr_by_const_ref(const StructWithSmartPtr &a);
 void pass_struct_with_smart_ptr_by_rvalue_ref(StructWithSmartPtr &&a);
 void pass_struct_with_smart_ptr_by_const_rvalue_ref(const StructWithSmartPtr &&a);
 void pass_struct_with_smart_ptr_by_ptr(StructWithSmartPtr *a);
 void pass_struct_with_smart_ptr_by_const_ptr(const StructWithSmartPtr *a);
 
 void regioninvalidationTestWithinStruct() {
   {
     StructWithSmartPtr S;
     pass_struct_with_smart_ptr_by_ref(S);
     S.P->foo(); // no-warning
   }
   {
     StructWithSmartPtr S;
     pass_struct_with_smart_ptr_by_const_ref(S);
-    S.P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+    S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
   }
   {
     StructWithSmartPtr S;
     pass_struct_with_smart_ptr_by_rvalue_ref(std::move(S));
     S.P->foo(); // no-warning
   }
   {
     StructWithSmartPtr S;
     pass_struct_with_smart_ptr_by_const_rvalue_ref(std::move(S));
-    S.P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+    S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
   }
   {
     StructWithSmartPtr S;
     pass_struct_with_smart_ptr_by_ptr(&S);
     S.P->foo();
   }
   {
     StructWithSmartPtr S;
     pass_struct_with_smart_ptr_by_const_ptr(&S);
-    S.P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+    S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
   }
 }
 
 void derefAfterAssignment() {
   {
     std::unique_ptr<A> P(new A());
     std::unique_ptr<A> Q;
     Q = std::move(P);
     Q->foo(); // no-warning
   }
   {
     std::unique_ptr<A> P;
     std::unique_ptr<A> Q;
     Q = std::move(P);
     // TODO: Fix test with expecting warning after '=' operator overloading modeling.
     Q->foo(); // no-warning
   }
 }
 
 void derefOnSwappedNullPtr() {
   std::unique_ptr<A> P(new A());
   std::unique_ptr<A> PNull;
   P.swap(PNull);
   PNull->foo(); // No warning.
-  (*P).foo();   // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  (*P).foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefOnStdSwappedNullPtr() {
   std::unique_ptr<A> P;
   std::unique_ptr<A> PNull;
   std::swap(P, PNull);
-  PNull->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
-  P->foo();     // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefOnSwappedValidPtr() {
   std::unique_ptr<A> P(new A());
   std::unique_ptr<A> PValid(new A());
   P.swap(PValid);
-  (*P).foo();    // No warning.
+  (*P).foo(); // No warning.
   PValid->foo(); // No warning.
   std::swap(P, PValid);
-  P->foo();      // No warning.
+  P->foo(); // No warning.
   PValid->foo(); // No warning.
 }