[FN] RetainCount: Use-after-free through @autorelease.

Quuxplusone commented 16 years ago


Bugzilla Link	PR2505
Status	CONFIRMED
Importance	P enhancement
Reported by	Nikita Zhuk (nikita@zhuk.fi)
Reported on	2008-06-29 14:22:07 -0700
Last modified on	2018-12-23 06:15:44 -0800
Version	unspecified
Hardware	Macintosh MacOS X
CC	kremenek@apple.com, llvm-bugs@lists.llvm.org, noqnoqneo@gmail.com
Fixed by commit(s)
Attachments
Blocks	PR2665
Blocked by
See also

In a non-GC enrivonment, clang static analyzer stops tracking reference counts
as soon as it sees an "autorelease" message sent to an object (tracking is
stopped in CFRefCount.cpp:879).

This causes clang to miss possible memory leaks and uses after release. Some
concrete examples:

- (void)useAfterRelease
{
  NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
  NSString *str = [[NSString alloc] initWithString:@"some string data"]; // rc(str)=1

  [str autorelease]; // rc(str)=1, added to local autorelease pool 'pool'
  [pool release]; // rc(str)=0, since local autorelease pool 'pool' is released

  NSLog(@"%@", str); // 'str' is used after release. Not reported by clang checker.
}

- (void)memoryLeak
{
  NSString *str = [[NSString alloc] initWithString:@"some string data"]; // rc(str)=1

  [str retain]; // rc(str)=2
  [str autorelease]; // rc(str)=2, and will be 1 after the current event.
   // 'str' is leaked, since it goes out of scope. Not reported by clang checker.
}

Clang should probably track local autorelease pools (which may be nested) and
handle 'autorelease' messages just as it handles 'release' messages, but taking
into account the knowledge about autorelease pools.

Clang revision used: 52881

Quuxplusone commented 16 years ago

Having the retain/release checker maintain a local stack of autorelease pools is not that difficult, and I agree would make an excellent enhancement to the tool. These are great test cases that could go directly into the Clang test suite.

Quuxplusone commented 15 years ago

While there isn't complete support yet for tracking the lifetime of autorelease pools, the analyzer can now flag many errors related to not using autorelease pools correctly (e.g., sending -autorelease to objects you don't own, over-releasing an object, etc.).

Nikita: Can you give this a try using the latest checker release? I'd like to shake out any obvious bugs.

Quuxplusone commented 15 years ago

Sounds great. I'll give it a try today and attach the results to this PR.

Quuxplusone commented 15 years ago

I tried the CFRef checker in clang rev #71635, and it seems to catch all those
incorrect uses of autorelease nicely. It still doesn't catch those cases when
objects are released because the autorelease pool there're in is released, but
this is expected.

I did find one weird crash, though. I'm not sure if I somehow compiled clang
incorrectly or if this is a real bug. Maybe you could check if you can
reproduce it?

Clang crashes when I try to analyze the following class:

@interface TheClass : NSObject @end
@implementation TheClass

- (void)m1
{
  NSString *s = [[NSString alloc] init];

  [s retain];
  [s autorelease];
}

- (void)m2
{
  NSString *s = [[[NSString alloc] init] autorelease];

  [s retain];
}

- (void)m3
{
  NSString *s = [[[NSString alloc] init] autorelease];

  [s retain];
  [s autorelease];
}

- (void)m4
{
  NSString *s = [[NSString alloc] init];

  [s retain];
}

- (void)m5
{
  NSString *s = [[NSString alloc] init];

  [s autorelease];
}
@end

The output is included below. Note that only methods m1 and m2 cause the crash.
There's no crashes if only methods m3, m4 and m5 are included in the class.

Output:

Assertion failed: (isa<X>(Val) && "cast<Ty>() argument of incompatible type!"),
function cast, file
/Volumes/DocsHD/Ohjelmointi/llvm/trunk/include/llvm/Support/Casting.h, line 199.
0   clang-cc          0x00e94938 main + 14851320
1   clang-cc          0x00e94eba main + 14852730
2   libSystem.B.dylib 0x96f742bb _sigtramp + 43
3   libSystem.B.dylib 0xffffffff _sigtramp + 1762180463
4   libSystem.B.dylib 0x96fe823a raise + 26
5   libSystem.B.dylib 0x96ff4679 abort + 73
6   libSystem.B.dylib 0x96fe93db __assert_rtn + 101
7   clang-cc          0x00171647 main + 1075719
8   clang-cc          0x0016127e main + 1009214
9   clang-cc          0x00164e7e main + 1024574
10  clang-cc          0x00158f86 main + 975686
11  clang-cc          0x0015a16f main + 980271
12  clang-cc          0x0000e3f9 _mh_execute_header + 54265
13  clang-cc          0x0000c9b8 _mh_execute_header + 47544
14  clang-cc          0x0000cf82 _mh_execute_header + 49026
15  clang-cc          0x000137f4 _mh_execute_header + 75764
16  clang-cc          0x0022d00b main + 1844171
17  clang-cc          0x0006754f _mh_execute_header + 419151
18  clang-cc          0x0006b49d main + 2141
19  clang-cc          0x00002236 _mh_execute_header + 4662
20  clang-cc          0x0000002a _mh_execute_header + 18446744073709547562
Stack dump:
0.  Program arguments: clang-cc -DIBOutlet=__attribute__((iboutlet)) -triple
i386-apple-darwin9 -fsyntax-only -disable-free -main-file-name TheClass.m --
relocation-model pic -pic-level=1 --disable-fp-elim --unwind-tables=0 --
mcpu=yonah --fmath-errno=0 -mmacosx-version-min=10.5 -isysroot /iPhone-SDK-
3.0/SDKs/MacOSX10.5.sdk -iquote
/Users/nzhuk/Desktop/AutoreleasePoolTest/build/AutoreleasePoolTest.build/Debug/AutoreleasePoolTest.build/AutoreleasePoolTest-
generated-files.hmap -iquote
/Users/nzhuk/Desktop/AutoreleasePoolTest/build/AutoreleasePoolTest.build/Debug/AutoreleasePoolTest.build/AutoreleasePoolTest-
project-headers.hmap -include
/Users/nzhuk/Desktop/AutoreleasePoolTest/AutoreleasePoolTest_Prefix.pch -
I/Users/nzhuk/Desktop/AutoreleasePoolTest/build/AutoreleasePoolTest.build/Debug/AutoreleasePoolTest.build/AutoreleasePoolTest-
own-target-headers.hmap -
I/Users/nzhuk/Desktop/AutoreleasePoolTest/build/AutoreleasePoolTest.build/Debug/AutoreleasePoolTest.build/AutoreleasePoolTest-
all-target-headers.hmap -F/Users/nzhuk/Desktop/AutoreleasePoolTest/build/Debug -
I/Users/nzhuk/Desktop/AutoreleasePoolTest/build/Debug/include -
I/Users/nzhuk/Desktop/AutoreleasePoolTest/build/AutoreleasePoolTest.build/Debug/AutoreleasePoolTest.build/DerivedSources/i386
-
I/Users/nzhuk/Desktop/AutoreleasePoolTest/build/AutoreleasePoolTest.build/Debug/AutoreleasePoolTest.build/DerivedSources
-std=c99 -fpascal-strings -fdiagnostics-show-option -x objective-c
/Users/nzhuk/Desktop/AutoreleasePoolTest/TheClass.m -analyze -analyzer-display-
progress -analyzer-eagerly-assume -checker-cfref -o
/var/folders/EN/ENJ+o01TFS03ZBOHBOZpdk+++TI/-Tmp-/scan-build-2009-05-13-30
1.  /Users/nzhuk/Desktop/AutoreleasePoolTest/TheClass.m:23:1: current parser
token '-'

The line 23 of TheClass.m looks like this:
- (void)m2

Quuxplusone commented 15 years ago

Thanks Nikita.

The crashes are now fixed:

http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20090511/017194.html

Quuxplusone commented 5 years ago

We currently already find the leak:

  test.m:27:1: warning: Potential leak of an object stored into 'str'
  }
  ^
  test.m:22:19: note: Method returns an instance of NSString with a +1 retain count
    NSString *str = [[NSString alloc] initWithString:@"some string data"]; // rc(str)=1
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  test.m:24:3: note: Reference count incremented. The object now has a +2 retain count
    [str retain]; // rc(str)=2
    ^~~~~~~~~~~~
  test.m:25:3: note: Object autoreleased
    [str autorelease]; // rc(str)=2, and will be 1 after the current event.
    ^~~~~~~~~~~~~~~~~
  test.m:27:1: note: Object leaked: object allocated and stored into 'str' is not
        referenced later in this execution path and has a retain count of +1
  }
  ^

We still don't find the use-after-free bug, so i'll keep this report to track the use-after-free false negative. We also don't find it with the modern @autorelease syntax, even though it does make the use-after-free harder to trigger, eg.:

(void)useAfterReleaseModernized { NSString escape; @autoreleasepool { NSString str = [[NSString alloc] initWithString:@"some string data"]; // rc(str)=1 escape = str; [str autorelease]; // rc(str)=1, added to local autorelease pool 'pool' }

NSLog(@"%@", escape); // 'str' is used after release. }

Tracked internally as rdar://problem/46924233

Quuxplusone / LLVMBugzillaTest

[FN] RetainCount: Use-after-free through @autorelease. #2729