[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D58368#1406490 , @NoQ wrote:

> Address comments.
>
> @Charusso: I agreed not to rush for D58367  
> and implemented an old-style visitor here instead :)


Thanks you! I wanted to accept it when you remove it from the review-chain and 
after rewriting the summary. The latter is not that big deal, just wanted to 
let you know it remained the same.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58368/new/

https://reviews.llvm.org/D58368



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL354641: [analyzer] MIGChecker: Improve intermediate 
diagnostic notes. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58368?vs=187873=187883#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58368/new/

https://reviews.llvm.org/D58368

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  cfe/trunk/test/Analysis/mig.mm

Index: cfe/trunk/test/Analysis/mig.mm
===
--- cfe/trunk/test/Analysis/mig.mm
+++ cfe/trunk/test/Analysis/mig.mm
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.osx.MIG\
-// RUN:-fblocks -verify %s
+// RUN:   -analyzer-output=text -fblocks -verify %s
 
 // XNU APIs.
 
@@ -20,9 +20,11 @@
 
 MIG_SERVER_ROUTINE
 kern_return_t basic_test(mach_port_name_t port, vm_address_t address, vm_size_t size) {
-  vm_deallocate(port, address, size);
-  if (size > 10) {
+  vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}}
+  if (size > 10) { // expected-note{{Assuming 'size' is > 10}}
+   // expected-note@-1{{Taking true branch}}
 return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+			 // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
   }
   return KERN_SUCCESS;
 }
@@ -42,6 +44,18 @@
   vm_deallocate(port, address, size);
 }
 
+// When releasing two parameters, add a note for both of them.
+// Also when returning a variable, explain why do we think that it contains
+// a non-success code.
+MIG_SERVER_ROUTINE
+kern_return_t release_twice(mach_port_name_t port, vm_address_t addr1, vm_address_t addr2, vm_size_t size) {
+  kern_return_t ret = KERN_ERROR; // expected-note{{'ret' initialized to 1}}
+  vm_deallocate(port, addr1, size); // expected-note{{Value passed through parameter 'addr1' is deallocated}}
+  vm_deallocate(port, addr2, size); // expected-note{{Value passed through parameter 'addr2' is deallocated}}
+  return ret; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+ // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+}
+
 // Check that we work on Objective-C messages and blocks.
 @interface I
 - (kern_return_t)fooAtPort:(mach_port_name_t)port withAddress:(vm_address_t)address ofSize:(vm_size_t)size;
@@ -51,8 +65,9 @@
 - (kern_return_t)fooAtPort:(mach_port_name_t)port
withAddress:(vm_address_t)address
 ofSize:(vm_size_t)size MIG_SERVER_ROUTINE {
-  vm_deallocate(port, address, size);
+  vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}}
   return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+ // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
 }
 @end
 
@@ -60,8 +75,9 @@
   kern_return_t (^block)(mach_port_name_t, vm_address_t, vm_size_t) =
   ^MIG_SERVER_ROUTINE (mach_port_name_t port,
vm_address_t address, vm_size_t size) {
-vm_deallocate(port, address, size);
+vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}}
     return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+   // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
   };
 }
 
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -41,19 +41,56 @@
 public:
   void 

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 187873.
NoQ marked an inline comment as done.
NoQ added a comment.

Address comments.

@Charusso: I agreed not to rush for D58367  
and implemented an old-style visitor here instead :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58368/new/

https://reviews.llvm.org/D58368

Files:
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/test/Analysis/mig.mm

Index: clang/test/Analysis/mig.mm
===
--- clang/test/Analysis/mig.mm
+++ clang/test/Analysis/mig.mm
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.osx.MIG\
-// RUN:-fblocks -verify %s
+// RUN:   -analyzer-output=text -fblocks -verify %s
 
 // XNU APIs.
 
@@ -20,9 +20,11 @@
 
 MIG_SERVER_ROUTINE
 kern_return_t basic_test(mach_port_name_t port, vm_address_t address, vm_size_t size) {
-  vm_deallocate(port, address, size);
-  if (size > 10) {
+  vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}}
+  if (size > 10) { // expected-note{{Assuming 'size' is > 10}}
+   // expected-note@-1{{Taking true branch}}
 return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+			 // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
   }
   return KERN_SUCCESS;
 }
@@ -42,6 +44,18 @@
   vm_deallocate(port, address, size);
 }
 
+// When releasing two parameters, add a note for both of them.
+// Also when returning a variable, explain why do we think that it contains
+// a non-success code.
+MIG_SERVER_ROUTINE
+kern_return_t release_twice(mach_port_name_t port, vm_address_t addr1, vm_address_t addr2, vm_size_t size) {
+  kern_return_t ret = KERN_ERROR; // expected-note{{'ret' initialized to 1}}
+  vm_deallocate(port, addr1, size); // expected-note{{Value passed through parameter 'addr1' is deallocated}}
+  vm_deallocate(port, addr2, size); // expected-note{{Value passed through parameter 'addr2' is deallocated}}
+  return ret; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+ // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+}
+
 // Check that we work on Objective-C messages and blocks.
 @interface I
 - (kern_return_t)fooAtPort:(mach_port_name_t)port withAddress:(vm_address_t)address ofSize:(vm_size_t)size;
@@ -51,8 +65,9 @@
 - (kern_return_t)fooAtPort:(mach_port_name_t)port
withAddress:(vm_address_t)address
 ofSize:(vm_size_t)size MIG_SERVER_ROUTINE {
-  vm_deallocate(port, address, size);
+  vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}}
   return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+ // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
 }
 @end
 
@@ -60,8 +75,9 @@
   kern_return_t (^block)(mach_port_name_t, vm_address_t, vm_size_t) =
   ^MIG_SERVER_ROUTINE (mach_port_name_t port,
vm_address_t address, vm_size_t size) {
-vm_deallocate(port, address, size);
+vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}}
     return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+   // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
   };
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -41,19 +41,56 @@
 public:
   void checkPostCall(const CallEvent , CheckerContext ) const;
   void checkPreStmt(const ReturnStmt *RS, CheckerContext ) const;
+
+  class Visitor : public BugReporterVisitor {
+  public:
+void Profile(llvm::FoldingSetNodeID ) const {
+  static int X = 0;

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked 3 inline comments as done.
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:47
 
 REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, bool);
 

Charusso wrote:
> `;` is not necessary.
Addressed in the earlier patch, D57558.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58368/new/

https://reviews.llvm.org/D58368



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked 3 inline comments as done.
NoQ added a comment.

Thx!




Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:109
+llvm::raw_svector_ostream OS(Str);
+OS << "Deallocating object passed through parameter '" << PVD->getName()
+   << '\'';

dcoughlin wrote:
> Could we just have the note say "'name' is deallocated"?
> 
> Or "Value passed through parameter 'name' is deallocated"
> 
> The ".. is ... " construction matches our other checkers. (Like "Memory is 
> released" from the Malloc Checker.)
Great point! I'd pick the latter because it's important to point out that the 
value is loaded from the parameter.

Hmm, btw, we should probably add more "visitors" in order to explain that the 
value is indeed copied from the parameter.



Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:113
+  });
+  C.addTransition(C.getState()->set(true), T);
 }

Charusso wrote:
> This is a cool approach, but it is difficult to use this API in other 
> checkers. If you do not out-chain D58367 I would like to see something like 
> that here:
> 
> ```
>   SmallString<64> Str;
>   llvm::raw_svector_ostream OS(Str);
>   OS << "Deallocating object passed through parameter '" << PVD->getName()
>  << '\'';
> 
>   C.addNote(C.getState()->set(true), OS.str());
> ```
I'll reply in D58367 because it seems to be universally relevant :)



Comment at: clang/test/Analysis/mig.mm:52
+kern_return_t release_twice(mach_port_name_t port, vm_address_t addr1, 
vm_address_t addr2, vm_size_t size) {
+  kern_return_t ret = KERN_ERROR; // expected-note{{'ret' initialized to 1}}
+  vm_deallocate(port, addr1, size); // expected-note{{Deallocating object 
passed through parameter 'addr1'}}

dcoughlin wrote:
> A nice QoI improvement here (for a later patch, perhaps) would be to have 
> this note use the macro name: "'ret initialized to KERN_ERROR'".
> 
> Users probably won't know that KERN_ERROR is 1.
Yup, but that's a separate story, because this message is produced by a 
generic, checker-inspecific visitor. We'll have to teach 
~~`trackNullOrUndefValue()`~~ `trackExpressionValue()` to be aware of macros, 
and it might turn out that we'd also want to mention macro names in messages 
that correspond to the subsequent copies of the same value (which, in turn, is 
tricky as it causes time paradoxes due to visiting order).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58368/new/

https://reviews.llvm.org/D58368



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:47
 
 REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, bool);
 

`;` is not necessary.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58368/new/

https://reviews.llvm.org/D58368



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a reviewer: Charusso.
Charusso requested changes to this revision.
Charusso added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:113
+  });
+  C.addTransition(C.getState()->set(true), T);
 }

This is a cool approach, but it is difficult to use this API in other checkers. 
If you do not out-chain D58367 I would like to see something like that here:

```
  SmallString<64> Str;
  llvm::raw_svector_ostream OS(Str);
  OS << "Deallocating object passed through parameter '" << PVD->getName()
 << '\'';

  C.addNote(C.getState()->set(true), OS.str());
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58368/new/

https://reviews.llvm.org/D58368



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

Looks good to me. I have some a minor diagnostic wording suggestion in line.




Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:109
+llvm::raw_svector_ostream OS(Str);
+OS << "Deallocating object passed through parameter '" << PVD->getName()
+   << '\'';

Could we just have the note say "'name' is deallocated"?

Or "Value passed through parameter 'name' is deallocated"

The ".. is ... " construction matches our other checkers. (Like "Memory is 
released" from the Malloc Checker.)



Comment at: clang/test/Analysis/mig.mm:52
+kern_return_t release_twice(mach_port_name_t port, vm_address_t addr1, 
vm_address_t addr2, vm_size_t size) {
+  kern_return_t ret = KERN_ERROR; // expected-note{{'ret' initialized to 1}}
+  vm_deallocate(port, addr1, size); // expected-note{{Deallocating object 
passed through parameter 'addr1'}}

A nice QoI improvement here (for a later patch, perhaps) would be to have this 
note use the macro name: "'ret initialized to KERN_ERROR'".

Users probably won't know that KERN_ERROR is 1.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58368/new/

https://reviews.llvm.org/D58368



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 187502.
NoQ added a comment.

Rebase.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58368/new/

https://reviews.llvm.org/D58368

Files:
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/test/Analysis/mig.mm

Index: clang/test/Analysis/mig.mm
===
--- clang/test/Analysis/mig.mm
+++ clang/test/Analysis/mig.mm
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.osx.MIG\
-// RUN:-fblocks -verify %s
+// RUN:   -analyzer-output=text -fblocks -verify %s
 
 // XNU APIs.
 
@@ -20,9 +20,11 @@
 
 MIG_SERVER_ROUTINE
 kern_return_t basic_test(mach_port_name_t port, vm_address_t address, vm_size_t size) {
-  vm_deallocate(port, address, size);
-  if (size > 10) {
+  vm_deallocate(port, address, size); // expected-note{{Deallocating object passed through parameter 'address'}}
+  if (size > 10) { // expected-note{{Assuming 'size' is > 10}}
+   // expected-note@-1{{Taking true branch}}
 return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is use-after-free vulnerability because caller will try to deallocate it again}}
+			 // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is use-after-free vulnerability because caller will try to deallocate it again}}
   }
   return KERN_SUCCESS;
 }
@@ -42,6 +44,18 @@
   vm_deallocate(port, address, size);
 }
 
+// When releasing two parameters, add a note for both of them.
+// Also when returning a variable, explain why do we think that it contains
+// a non-success code.
+MIG_SERVER_ROUTINE
+kern_return_t release_twice(mach_port_name_t port, vm_address_t addr1, vm_address_t addr2, vm_size_t size) {
+  kern_return_t ret = KERN_ERROR; // expected-note{{'ret' initialized to 1}}
+  vm_deallocate(port, addr1, size); // expected-note{{Deallocating object passed through parameter 'addr1'}}
+  vm_deallocate(port, addr2, size); // expected-note{{Deallocating object passed through parameter 'addr2'}}
+  return ret; // expected-warning{{MIG callback fails with error after deallocating argument value. This is use-after-free vulnerability because caller will try to deallocate it again}}
+ // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is use-after-free vulnerability because caller will try to deallocate it again}}
+}
+
 // Check that we work on Objective-C messages and blocks.
 @interface I
 - (kern_return_t)fooAtPort:(mach_port_name_t)port withAddress:(vm_address_t)address ofSize:(vm_size_t)size;
@@ -51,8 +65,9 @@
 - (kern_return_t)fooAtPort:(mach_port_name_t)port
withAddress:(vm_address_t)address
 ofSize:(vm_size_t)size MIG_SERVER_ROUTINE {
-  vm_deallocate(port, address, size);
+  vm_deallocate(port, address, size); // expected-note{{Deallocating object passed through parameter 'address'}}
   return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is use-after-free vulnerability because caller will try to deallocate it again}}
+ // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is use-after-free vulnerability because caller will try to deallocate it again}}
 }
 @end
 
@@ -60,8 +75,9 @@
   kern_return_t (^block)(mach_port_name_t, vm_address_t, vm_size_t) =
   ^MIG_SERVER_ROUTINE (mach_port_name_t port,
vm_address_t address, vm_size_t size) {
-vm_deallocate(port, address, size);
+vm_deallocate(port, address, size); // expected-note{{Deallocating object passed through parameter 'address'}}
     return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is use-after-free vulnerability because caller will try to deallocate it again}}
+   // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is use-after-free vulnerability because caller will try to deallocate it again}}
   };
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -46,14 +46,17 @@
 
 REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, bool);
 
-static bool isCurrentArgSVal(SVal V, CheckerContext ) {
+static const ParmVarDecl *getOriginParam(SVal V, CheckerContext ) {
   SymbolRef Sym = V.getAsSymbol();
   if (!Sym)
-return false;
+return nullptr;
 
   const auto *VR = dyn_cast_or_null(Sym->getOriginRegion());
-  return VR && VR->hasStackParametersStorage() &&
- VR->getStackFrame()->inTopFrame();
+  if (VR && VR->hasStackParametersStorage() &&
+ 

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

This adds two visitors to the checker:

- `trackExpressionValue()` in order to highlight where does the return value 
come from when it's not a literal.
- A tag-based visitor (as in D58367 ) that 
explains where parameters are deallocated.


Repository:
  rC Clang

https://reviews.llvm.org/D58368

Files:
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/test/Analysis/mig.cpp


Index: clang/test/Analysis/mig.cpp
===
--- clang/test/Analysis/mig.cpp
+++ clang/test/Analysis/mig.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.osx.MIG -verify %s
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.osx.MIG\
+// RUN:   -analyzer-output=text -verify %s
+
 
 // XNU APIs.
 
@@ -16,9 +18,11 @@
 
 MIG_SERVER_ROUTINE
 kern_return_t basic_test(mach_port_name_t port, vm_address_t address, 
vm_size_t size) {
-  vm_deallocate(port, address, size);
-  if (size > 10) {
+  vm_deallocate(port, address, size); // expected-note{{Deallocating object 
passed through parameter 'address'}}
+  if (size > 10) { // expected-note{{Assuming 'size' is > 10}}
+   // expected-note@-1{{Taking true branch}}
 return KERN_ERROR; // expected-warning{{MIG callback fails with error 
after deallocating argument value. This is use-after-free vulnerability because 
caller will try to deallocate it again}}
+   // expected-note@-1{{MIG callback fails with error 
after deallocating argument value. This is use-after-free vulnerability because 
caller will try to deallocate it again}}
   }
   return KERN_SUCCESS;
 }
@@ -28,3 +32,15 @@
 kern_return_t no_crash(mach_port_name_t port, vm_address_t address, vm_size_t 
size) {
   vm_deallocate(port, address, size);
 }
+
+// When releasing two parameters, add a note for both of them.
+// Also when returning a variable, explain why do we think that it contains
+// a non-success code.
+MIG_SERVER_ROUTINE
+kern_return_t release_twice(mach_port_name_t port, vm_address_t addr1, 
vm_address_t addr2, vm_size_t size) {
+  kern_return_t ret = KERN_ERROR; // expected-note{{'ret' initialized to 1}}
+  vm_deallocate(port, addr1, size); // expected-note{{Deallocating object 
passed through parameter 'addr1'}}
+  vm_deallocate(port, addr2, size); // expected-note{{Deallocating object 
passed through parameter 'addr2'}}
+  return ret; // expected-warning{{MIG callback fails with error after 
deallocating argument value. This is use-after-free vulnerability because 
caller will try to deallocate it again}}
+ // expected-note@-1{{MIG callback fails with error after 
deallocating argument value. This is use-after-free vulnerability because 
caller will try to deallocate it again}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -45,14 +45,17 @@
 
 REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, bool);
 
-static bool isCurrentArgSVal(SVal V, CheckerContext ) {
+static const ParmVarDecl *getOriginParam(SVal V, CheckerContext ) {
   SymbolRef Sym = V.getAsSymbol();
   if (!Sym)
-return false;
+return nullptr;
 
   const auto *VR = dyn_cast_or_null(Sym->getOriginRegion());
-  return VR && VR->hasStackParametersStorage() &&
- VR->getStackFrame()->inTopFrame();
+  if (VR && VR->hasStackParametersStorage() &&
+ VR->getStackFrame()->inTopFrame())
+return cast(VR->getDecl());
+
+  return nullptr;
 }
 
 static bool isInMIGCall(const LocationContext *LC) {
@@ -86,8 +89,18 @@
 
   // TODO: Unhardcode "1".
   SVal Arg = Call.getArgSVal(1);
-  if (isCurrentArgSVal(Arg, C))
-C.addTransition(C.getState()->set(true));
+  const ParmVarDecl *PVD = getOriginParam(Arg, C);
+  if (!PVD)
+return;
+
+  const NoteTag *T = C.getNoteTag([PVD]() -> std::string {
+SmallString<64> Str;
+llvm::raw_svector_ostream OS(Str);
+OS << "Deallocating object passed through parameter '" << PVD->getName()
+   << '\'';
+return OS.str();
+  });
+  C.addTransition(C.getState()->set(true), T);
 }
 
 void MIGChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext ) const {
@@ -129,6 +142,8 @@
   "deallocate it again",
   N);
 
+  R->addRange(RS->getSourceRange());
+  bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, false);
   C.emitReport(std::move(R));
 }
 


Index: clang/test/Analysis/mig.cpp
===
--- clang/test/Analysis/mig.cpp
+++ clang/test/Analysis/mig.cpp
@@ -1,4 +1,6 @@
-// RUN: