[Analyzer] Fix bug report source locations in minimal output.

[balazske]

Summary

Fix of the following problem:
If the bug report equivalence class contains multiple
reports and no (minimal) analyzer output was requested
it can happen that the wrong location is used for the warning.

[balazske]

Lot of tests are failing because changed warning locations.
These will be updated if the code change looks good.

[NoQ]

These will be updated if the code change looks good.

Hard to tell; this entire code is too convoluted, i'd rather look at the changes. Can you update at least some tests?

[balazske]

Fixed some source location changes in tests.
Re-added check for empty path.

[balazske]

Big part of the test failures is with the osx.cocoa.RetainCount checker. Only a small part of the errors is fixed now.

clang/lib/Analysis/PathDiagnostic.cpp

369

This change is needed to eliminate crash in tests at assert(b.hasValue());.

[Szelethus]

I find the summary here a bit lacking. We detected this issue in D83120, where a lot of discussion can be found, so its worth linking in. On its own, I'm not immediately sold on whether this is the correct solution, and even if it is, how does it solve the problem. I bet you had to struggle a bit to understand the related machinery to fix this, it'd be invaluable to share the knowledge you gained here as well.

I took a look myself, and the issue fixed here seems to be that PathDiagnostic's constructor doesn't set the associated PathDiagnosticLocation itself, and generateEmptyDiagnosticForReport pretty much resolves to that. The thing I'm still struggling with, is that I'm not terribly sure whether this the same issue raised in the previous patch.

Fix of the following problem:
If the bug report equivalence class contains multiple
reports and no (minimal) analyzer output was requested
it can happen that the wrong location is used for the warning.

I have two burning questions about this:

• Are we sure that we used the correct (with the shortest bug path) bug report, but associated it with the wrong location? Mind that everything you touched here, as I understand it, affects sorting, not uniqueing.
• If so, some (even if incorrect) location must've been used, where did that come from?

In D83961#2158128, @balazske wrote:

Big part of the test failures is with the osx.cocoa.RetainCount checker.

Every time :^)

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

3007

Is this the *only* instance when that happens? How about the text-minimal output?

[balazske]

The problem in D83120 is fixed by this patch.
What I figured out from the code is that BugReporter::FlushReport calls findReportInEquivalenceClass that returns a report that has not necessary the shortest path. That report is get from the passed bug report class and a different instance is returned if the uniqueing is turned on or off. This report is used to fill the last location in the bug path, if the bug path is empty. So the location in the bug path changes dependent on the setting of uniqueing. The bug path here is empty if the analyzer-output is set to minimal (or not specified). Even in the minimal output case, function PathDiagnosticBuilder::generate is called if it is a path sensitive case. In that function the shortest path is used. So instead of returning an empty path from that function (what is filled later by FlushReport) the last part of the path can be filled here (and use the correct source location from the shortest path).
The problem can happen if not the report with shortest path is returned from findReportInEquivalenceClass, and analyzer-output is set to minimal or default. If the output is not minimal, the bug path is filled by the PathDiagnosticBuilder::generate with correct locations. So changing the output type can result in change of the warning location.
Other way to get the problem is change only the uniqueing setting in bug type and use minimal (or default) output. Then the example report can change and because this example report is used to get the bug location the location can change too (if the equivalence class contains paths with different end locations).

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

3007

I am not totally sure on this but inserted an assert and all the tests passed without triggering it.
Is the text-minimal different from the default (when to --analyzer-output is given at all)?
I think it works like this: Function PathSensitiveBugReporter::generateDiagnosticForConsumerMap is called from here, then if it is not a BasicBugReport the function PathDiagnosticBuilder::generate will be called that is fixed now to return always with non-empty path.

[NoQ]

clang/test/Analysis/CGColorSpace.c

8–11

This change doesn't look expected to me. It's not "we've found a report on a different path" (there's only one path here), it's "we're reporting the same report in a different location" (previously it was the uniqueing location, now it's the end-of-path location).

That said, RetainCountChecker is special because it has its own PathSensitiveBugReport sub-class with custom behavior. I think we should get regular checkers like MallocChecker right first. Are there more changes on tests on regular checkers? Or is the one in malloc-plist.c the only one?

clang/test/Analysis/malloc-plist.c

137–140

This sounds like an expected change: we're now displaying the same report on a different path. Except it's the longer path rather than the shorter path, so it still looks suspicious.

[balazske]

clang/test/Analysis/malloc-plist.c

137–140

This location may be wrong but it is then another problem. The important thing is that after this patch the same location is used for a warning in text-minimal mode as it is in text mode. Without the patch in text-minimal mode a different location is used for this warning, the one at the end of the function. But still in text mode (without the patch) the location at y++ is shown. (The warning in function function_with_leak4 in the same test is already at a similar location, not at the end of function.)

[balazske]

Every other test failure comes from RetainCount checker except malloc-plist.c.

[balazske]

Change column in malloc-plist test because code was reformatted.

[NoQ]

In D83961#2166903, @balazske wrote:

Every other test failure comes from RetainCount checker except malloc-plist.c.

Aha, ok. So, anyway, for retain count checker we ultimately only care about plist and html reports, not about text reports. It's also probably easier to write more precise tests after your patch: test functions are unlikely to have multiple possible uniqueing locations, and even if there are they can be discriminated between by warning message text.

But i'd still rather have it explained why does your patch affect the location of RefLeakReports in order to make sure we understand what we're doing. This consequence of the patch wasn't intended - what other unintended consequences does it have? Are we even sure that plist/html reports don't change? Is RefLeakReport even implemented correctly from our current point of view? Or, regardless of correctness, do we want its current implementation to have the old behavior or the new behavior?

I should probably investigate this myself from the fairness/justice point of view but you'll probably land your patch faster if you don't wait on me to get un-busy with other stuff :/ As a bonus, you might be able to get away without updating all the tests if you find out that this change is accidental and not really intended :) Note that you don't need to know Objective-C or know much about the checker to investigate these things; say, CGColorSpace.c is a pure C test with a straightforward resource leak bug. The customizations in RefLeakReport aren't really checker-specific either - we simply didn't ever make up our mind on whether they should apply to all leak reports or to none; they have visual consequences on plist reports though.

clang/test/Analysis/malloc-plist.c

137–140

Oh, wait, the other path isn't in fact shorter. In both cases it leaks immediately after the if-statement, and the path with y++ is in fact a bit shorter (it immediately hits PreStmtPurgeDeadSymbols for the DeclRefExpr whereas the other path hits CallExitBegin first, because the call is inlined).

So it's a good and expected change!

[balazske]

From the beginning on in this patch I assumed that the location of the bug report should be the end of the bug path. But this is probably a wrong approach, specially if the bug report has uniqueing location. In that case the uniqueing location may be a better place for the bug report. Specially for resource leak it is not bad if the report is located at the allocation site (still there is a bug path that shows the full path and place of leak). So another approach can be to allow only bug equivalence classes where the reports have same locations. If this is not true the checker should be fixed that generates the reports.

The behavior with RefLeakReport is correct if the indicated location in minimal output mode should be the end of the bug path. Otherwise it is not correct, the report has a different location (allocation site) than the end of the path (return point) and we want to see the allocation site in minimal mode (or not?). Still there is problem, in text output mode the summary line indicates not the correct place but the end of the bug path:
before the patch:

$ clang -cc1 -internal-isystem ~/clang/llvm1/build/Debug/lib/clang/11.0.0/include -nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-store=region ~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c
~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c:9:23: warning: Potential leak of an object stored into 'X' [osx.cocoa.RetainCount]
  CGColorSpaceRef X = CGColorSpaceCreateDeviceRGB(); // expected-warning{{leak}}
                      ^
1 warning generated.

$ clang -cc1 -internal-isystem ~/clang/llvm1/build/Debug/lib/clang/11.0.0/include -nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-store=region ~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c -analyzer-output text
~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c:11:1: warning: Potential leak of an object stored into 'X' [osx.cocoa.RetainCount]
}
^
~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c:9:23: note: Call to function 'CGColorSpaceCreateDeviceRGB' returns a Core Foundation object of type 'CGColorSpaceRef' with a +1 retain count
  CGColorSpaceRef X = CGColorSpaceCreateDeviceRGB(); // expected-warning{{leak}}
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c:10:3: note: Reference count incremented. The object now has a +2 retain count
  CGColorSpaceRetain(X);
  ^~~~~~~~~~~~~~~~~~~~~
~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c:11:1: note: Object leaked: object allocated and stored into 'X' is not referenced later in this execution path and has a retain count of +2
}
^
1 warning generated.

after the patch:

$ clang -cc1 -internal-isystem ~/clang/llvm1/build/Debug/lib/clang/11.0.0/include -nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-store=region ~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c
~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c:11:1: warning: Potential leak of an object stored into 'X' [osx.cocoa.RetainCount]
}
^
1 warning generated.

$ clang -cc1 -internal-isystem ~/clang/llvm1/build/Debug/lib/clang/11.0.0/include -nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-store=region ~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c -analyzer-output text
~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c:11:1: warning: Potential leak of an object stored into 'X' [osx.cocoa.RetainCount]
}
^
~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c:9:23: note: Call to function 'CGColorSpaceCreateDeviceRGB' returns a Core Foundation object of type 'CGColorSpaceRef' with a +1 retain count
  CGColorSpaceRef X = CGColorSpaceCreateDeviceRGB(); // expected-warning{{leak}}
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c:10:3: note: Reference count incremented. The object now has a +2 retain count
  CGColorSpaceRetain(X);
  ^~~~~~~~~~~~~~~~~~~~~
~/clang/llvm1/llvm-project/clang/test/Analysis/CGColorSpace.c:11:1: note: Object leaked: object allocated and stored into 'X' is not referenced later in this execution path and has a retain count of +2
}
^
1 warning generated.

So it should be clarified how this should work, have the end of the bug path or the "location" of the bug report as display location of the bug report.

[NoQ]

I believe i clarified above how this checker should work. For testing purposes (which is the only reason to ever use the minimal output) i prefer to have it at the end of path, because that tells more about the bug path and therefore more important to test.

The question i'm asking is more about API contracts of the PathSensitiveBugReport class over its virtual methods. Right now i don't care whether the entire system behaves correctly or not. I want you to explain how the change in one component caused a change in another component, so that to avoid introducing pairs of mutually cancelling bugs. In other words, this is a unit-test question, not an integration-test question.

[balazske]

Then the current solution is good, print always end of the bug path.
A change to the bug reporting component was made that caused reported position of bugs to change. New is the end of the path, old is the location set by the checker (BugReport::getLocation value). The location is often same as end of the path. The RefLeakReport has a special getLocation function and there is a difference between this location (that is the allocation site) and the end of the path.

clang/lib/Analysis/PathDiagnostic.cpp

 //===- PathDiagnostic.cpp - Path-Specific Diagnostic Handling -------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
static bool compareCrossTUSourceLocs(FullSourceLoc XL, FullSourceLoc YL) {
   return XL.getFileID() < YL.getFileID();
 }
 
 static bool compare(const PathDiagnostic &X, const PathDiagnostic &Y) {
   FullSourceLoc XL = X.getLocation().asLocation();
   FullSourceLoc YL = Y.getLocation().asLocation();
   if (XL != YL)
     return compareCrossTUSourceLocs(XL, YL);
+  PathDiagnosticRange XR = X.getLocation().asRange();
+  PathDiagnosticRange YR = Y.getLocation().asRange();
+  if (XR.isValid() && !YR.isValid())
+    return true;
+  if (!XR.isValid() && YR.isValid())
+    return false;
+  if (XR.isValid() && YR.isValid()) {
+    if (XR.isPoint && !YR.isPoint)
+      return true;
+    if (!XR.isPoint && YR.isPoint)
+      return false;
+    if (!XR.isPoint && !YR.isPoint) {
+      FullSourceLoc XRE{XR.getEnd(), XL.getManager()};
+      FullSourceLoc YRE{YR.getEnd(), YL.getManager()};
+      if (XRE != YRE)
+        return compareCrossTUSourceLocs(XRE, YRE);
+    }
+  }
   if (X.getBugType() != Y.getBugType())
     return X.getBugType() < Y.getBugType();
   if (X.getCategory() != Y.getCategory())
     return X.getCategory() < Y.getCategory();
   if (X.getVerboseDescription() != Y.getVerboseDescription())
     return X.getVerboseDescription() < Y.getVerboseDescription();
   if (X.getShortDescription() != Y.getShortDescription())
     return X.getShortDescription() < Y.getShortDescription();

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

 //===- BugReporter.cpp - Generate PathDiagnostics for bugs ----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
PathDiagnosticBuilder::generate(const PathDiagnosticConsumer *PDC) const {
 
   // See whether we need to silence the checker/package.
   // FIXME: This will not work if the report was emitted with an incorrect tag.
   for (const std::string &CheckerOrPackage : Opts.SilencedCheckersAndPackages) {
     if (ErrorTag.startswith(CheckerOrPackage))
       return nullptr;
   }
 
-  if (!PDC->shouldGenerateDiagnostics())
-    return generateEmptyDiagnosticForReport(R, getSourceManager());
-
   // Construct the final (warning) event for the bug report.
   auto EndNotes = VisitorsDiagnostics->find(ErrorNode);
   PathDiagnosticPieceRef LastPiece;
   if (EndNotes != VisitorsDiagnostics->end()) {
     assert(!EndNotes->second.empty());
     LastPiece = EndNotes->second[0];
   } else {
     LastPiece = BugReporterVisitor::getDefaultEndPath(*this, ErrorNode,
                                                       *getBugReport());
   }
   Construct.PD->setEndOfPath(LastPiece);
 
+  if (!PDC->shouldGenerateDiagnostics())
+    return std::move(Construct.PD);
+
   PathDiagnosticLocation PrevLoc = Construct.PD->getLocation();
   // From the error node to the root, ascend the bug path and construct the bug
   // report.
   while (Construct.ascendToPrevNode()) {
     generatePathDiagnosticsForNode(Construct, PrevLoc);
 
     auto VisitorNotes = VisitorsDiagnostics->find(Construct.getCurrentNode());
     if (VisitorNotes == VisitorsDiagnostics->end())
std::unique_ptr<DiagnosticForConsumerMapTy> Diagnostics =
       generateDiagnosticForConsumerMap(report, Consumers, bugReports);
 
   for (auto &P : *Diagnostics) {
     PathDiagnosticConsumer *Consumer = P.first;
     std::unique_ptr<PathDiagnostic> &PD = P.second;
 
     // If the path is empty, generate a single step path with the location
     // of the issue.
+    // This can happen if report is BasicBugReport.
     if (PD->path.empty()) {
       PathDiagnosticLocation L = report->getLocation();
       auto piece = std::make_unique<PathDiagnosticEventPiece>(
           L, report->getDescription());
       for (SourceRange Range : report->getRanges())
         piece->addRange(Range);
       PD->setEndOfPath(std::move(piece));
     }
 
     PathPieces &Pieces = PD->getMutablePieces();
     if (getAnalyzerOptions().ShouldDisplayNotesAsEvents) {
       // For path diagnostic consumers that don't support extra notes,

clang/test/Analysis/CGColorSpace.c

 // RUN: %clang_analyze_cc1 -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-store=region -verify %s
 
 typedef struct CGColorSpace *CGColorSpaceRef;
 extern CGColorSpaceRef CGColorSpaceCreateDeviceRGB(void);
 extern CGColorSpaceRef CGColorSpaceRetain(CGColorSpaceRef space);
 extern void CGColorSpaceRelease(CGColorSpaceRef space);
 
 void f() {
-  CGColorSpaceRef X = CGColorSpaceCreateDeviceRGB(); // expected-warning{{leak}}
+  CGColorSpaceRef X = CGColorSpaceCreateDeviceRGB();
   CGColorSpaceRetain(X);
-}
+} // expected-warning{{leak}}
 
 void fb() {
   CGColorSpaceRef X = CGColorSpaceCreateDeviceRGB();
   CGColorSpaceRetain(X);
   CGColorSpaceRelease(X);
   CGColorSpaceRelease(X);  // no-warning
 }

clang/test/Analysis/Inputs/expected-plists/malloc-plist.c.plist

<key>edges</key>
            <key>col</key><integer>9</integer>
            <key>file</key><integer>0</integer>
           </dict>
          </array>
         <key>end</key>
          <array>
           <dict>
            <key>line</key><integer>138</integer>
-           <key>col</key><integer>9</integer>
+           <key>col</key><integer>7</integer>
            <key>file</key><integer>0</integer>
           </dict>
           <dict>
            <key>line</key><integer>138</integer>
-           <key>col</key><integer>9</integer>
+           <key>col</key><integer>7</integer>
            <key>file</key><integer>0</integer>
           </dict>
          </array>
        </dict>
       </array>
     </dict>
     <dict>
      <key>kind</key><string>event</string>
      <key>location</key>
      <dict>
       <key>line</key><integer>138</integer>
-      <key>col</key><integer>9</integer>
+      <key>col</key><integer>7</integer>
       <key>file</key><integer>0</integer>
      </dict>
      <key>depth</key><integer>1</integer>
      <key>extended_message</key>
      <string>Potential leak of memory pointed to by &apos;x&apos;</string>
      <key>message</key>
      <string>Potential leak of memory pointed to by &apos;x&apos;</string>
     </dict>
    </array>
    <key>description</key><string>Potential leak of memory pointed to by &apos;x&apos;</string>
    <key>category</key><string>Memory error</string>
    <key>type</key><string>Memory leak</string>
    <key>check_name</key><string>unix.Malloc</string>
    <!-- This hash is experimental and going to change! -->
    <key>issue_hash_content_of_line_in_context</key><string>0ddc87e8a7e7104428af3905f3057611</string>
   <key>issue_context_kind</key><string>function</string>
   <key>issue_context</key><string>function_with_leak3</string>
   <key>issue_hash_function_offset</key><string>1</string>
   <key>location</key>
   <dict>
    <key>line</key><integer>138</integer>
-   <key>col</key><integer>9</integer>
+   <key>col</key><integer>7</integer>
    <key>file</key><integer>0</integer>
   </dict>
   <key>ExecutedLines</key>
   <dict>
    <key>0</key>
    <array>
     <integer>135</integer>
     <integer>136</integer>

clang/test/Analysis/NSString.m

 }
 
 NSArray *f6(NSString* s) {
   return [s componentsSeparatedByCharactersInSet:0]; // expected-warning {{Argument to 'NSString' method 'componentsSeparatedByCharactersInSet:' cannot be nil}}
 }
 
 NSString* f7(NSString* s1, NSString* s2, NSString* s3) {
 
-  NSString* s4 = (NSString*)
-    CFStringCreateWithFormat(kCFAllocatorDefault, 0,  // expected-warning{{leak}}
-                             (CFStringRef) __builtin___CFStringMakeConstantString("%@ %@ (%@)"), 
+  NSString *s4 = (NSString *)
+      CFStringCreateWithFormat(kCFAllocatorDefault, 0,
+                               (CFStringRef)__builtin___CFStringMakeConstantString("%@ %@ (%@)"),
                                s1, s2, s3);
 
   CFRetain(s4);
-  return s4;
+  return s4; // expected-warning{{leak}}
 }
 
 NSMutableArray* f8() {
   
   NSString* s = [[NSString alloc] init];
   NSMutableArray* a = [[NSMutableArray alloc] initWithCapacity:2];
   [a addObject:s];
   [s release]; // no-warning

 @end
 @implementation TestAutorelease
 -(NSString*) getString {
   NSString *str = [[NSString alloc] init];
   return [str autorelease]; // no-warning
 }
 - (void)m1
 {
- NSString *s = [[NSString alloc] init]; // expected-warning{{leak}}
+  NSString *s = [[NSString alloc] init];
   [s retain];
   [s autorelease];
-}
+} // expected-warning{{leak}}
 - (void)m2
 {
- NSString *s = [[[NSString alloc] init] autorelease]; // expected-warning{{leak}}
+  NSString *s = [[[NSString alloc] init] autorelease];
   [s retain];
-}
+} // expected-warning{{leak}}
 - (void)m3
 {
  NSString *s = [[[NSString alloc] init] autorelease];
  [s retain];
  [s autorelease];
 }
 - (void)m4
 {
- NSString *s = [[NSString alloc] init]; // expected-warning{{leak}}
+  NSString *s = [[NSString alloc] init];
   [s retain];
-}
+} // expected-warning{{leak}}
 - (void)m5
 {
  NSString *s = [[NSString alloc] init];
  [s autorelease];
 }
 @end
 
 @interface C1 : NSObject {}
void test_objc_atomicCompareAndSwap_parameter(NSString **old) {
   NSString *s = [[NSString alloc] init]; // no-warning
   if (!objc_atomicCompareAndSwapPtr(0, s, old))
     [s release];
   else    
     [*old release];
 }
 
 void test_objc_atomicCompareAndSwap_parameter_no_direct_release(NSString **old) {
-  NSString *s = [[NSString alloc] init]; // expected-warning{{leak}}
+  NSString *s = [[NSString alloc] init];
   if (!objc_atomicCompareAndSwapPtr(0, s, old))
-    return;
+    return; // expected-warning{{leak}}
   else    
     [*old release];
 }
 
 
 // Test stringWithFormat (<rdar://problem/6815234>)
 void test_stringWithFormat() {  
   NSString *string = [[NSString stringWithFormat:@"%ld", (long) 100] retain];
   [string release];
   [string release]; // expected-warning{{Incorrect decrement of the reference count}}
 }
 
 // Test isTrackedObjectType().
 typedef NSString* WonkyTypedef;
 @interface TestIsTracked
 + (WonkyTypedef)newString;
 @end
 
 void test_isTrackedObjectType(void) {
-  NSString *str = [TestIsTracked newString]; // expected-warning{{Potential leak}}
-}
+  NSString *str = [TestIsTracked newString];
+} // expected-warning{{Potential leak}}
 
 // Test isTrackedCFObjectType().
 @interface TestIsCFTracked
 + (CFStringRef) badNewCFString;
 + (CFStringRef) newCFString;
 @end
 
 @implementation TestIsCFTracked
 + (CFStringRef) newCFString {
   return CFStringCreateWithFormat(kCFAllocatorDefault, ((void*)0), ((CFStringRef) __builtin___CFStringMakeConstantString ("" "%d" "")), 100); // no-warning
 }
 + (CFStringRef) badNewCFString {
   return CFStringCreateWithFormat(kCFAllocatorDefault, ((void*)0), ((CFStringRef) __builtin___CFStringMakeConstantString ("" "%d" "")), 100); // expected-warning{{leak}}
 }
 
 // Test @synchronized
 void test_synchronized(id x) {
   @synchronized(x) {
-    NSString *string = [[NSString stringWithFormat:@"%ld", (long) 100] retain]; // expected-warning {{leak}}
-  }
+    NSString *string = [[NSString stringWithFormat:@"%ld", (long)100] retain];
   }
+} // expected-warning {{leak}}
 @end
 
 void testOSCompareAndSwapXXBarrier_parameter(NSString **old) {
   NSString *s = [[NSString alloc] init]; // no-warning
   if (!COMPARE_SWAP_BARRIER((intptr_t) 0, (intptr_t) s, (intptr_t*) old))
     [s release];
   else    
     [*old release];

clang/test/Analysis/malloc-plist.c

static void function_with_leak2() {
     int m = 0; //expected-warning{{Potential leak}}
 }
 void use_function_with_leak2() {
     function_with_leak2();
 }
 
 static void function_with_leak3(int y) {
     char *x = (char*)malloc(12);
     if (y)
-        y++;
-}//expected-warning{{Potential leak}}
+      y++; //expected-warning{{Potential leak}}
+}
 void use_function_with_leak3(int y) {
     function_with_leak3(y);
 }
 
 static void function_with_leak4(int y) {
     char *x = (char*)malloc(12);
     if (y)
         y++;
     else