silent miscompilation of code sending objc messages to pointers and ints

[llvmbot]

Bugzilla Link	3766
Resolution	FIXED
Resolved on	Feb 22, 2010 12:54
Version	unspecified
OS	MacOS X
Reporter	LLVM Bugzilla Contributor
CC	@tkremenek

Extended Description

clang version: 0.169

The following example generates a 'Branch condition evaluates to an uninitialized value' false positive:

[johne@MacBook] checker-0.169% cat ../bug2.m #import <Foundation/NSAutoreleasePool.h>
#import <Foundation/NSGarbageCollector.h>
#import <Foundation/NSObject.h>
#import <Foundation/NSString.h>
#include <stdio.h>
//#include <objc/runtime.h> // <-- Uncomment to make the false positive go away.

BOOL collectingEnabled (void) {
BOOL gcEnabled = ([objc_getClass("NSGarbageCollector") defaultCollector] != NULL) ? YES : NO;
if(gcEnabled == YES) { // <-- False positive here.
printf("GC is enabled.\n");
}
return(gcEnabled);
}

int main(int argc, char *argv[]) {
NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
BOOL gcEnabled = collectingEnabled();

printf("GC enabled: %d\n", gcEnabled);

[pool release];
pool = NULL;

return(0);
}

[johne@MacBook] checker-0.169% ./scan-build gcc-4.2 -std=gnu99 -c -g -pedantic -Wall -Wextra -Wmissing-prototypes -Wredundant-decls ../bug2.m
../bug2.m:1:2: warning: #import is a GCC extension
../bug2.m:2:2: warning: #import is a GCC extension
../bug2.m:3:2: warning: #import is a GCC extension
../bug2.m:4:2: warning: #import is a GCC extension
../bug2.m:8: warning: no previous prototype for 'collectingEnabled'
../bug2.m:16: warning: unused parameter 'argc'
../bug2.m:16: warning: unused parameter 'argv'
../bug2.m:9:21: warning: bad receiver type 'int'
BOOL gcEnabled = ([objc_getClass("NSGarbageCollector") defaultCollector] != NULL) ? YES : NO;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ANALYZE: ../bug2.m collectingEnabled
../bug2.m:10:3: warning: Branch condition evaluates to an uninitialized value.
if(gcEnabled == YES) { // <-- False positive here.
^ ~~~~~~~~~
ANALYZE: ../bug2.m main
2 diagnostics generated.
scan-build: 1 bugs found.
scan-build: Run 'scan-view /var/folders/Ee/Eeuv+JUYEF4h5NFY82FFvE+++TI/-Tmp-/scan-build-2009-03-09-15' to examine bug reports.

Rerun with #include <objc/runtime.h> uncommented:

[johne@MacBook] checker-0.169% ./scan-build gcc-4.2 -std=gnu99 -c -g -pedantic -Wall -Wextra -Wmissing-prototypes -Wredundant-decls ../bug2.m
../bug2.m:1:2: warning: #import is a GCC extension
../bug2.m:2:2: warning: #import is a GCC extension
../bug2.m:3:2: warning: #import is a GCC extension
../bug2.m:4:2: warning: #import is a GCC extension
../bug2.m:8: warning: no previous prototype for 'collectingEnabled'
../bug2.m:16: warning: unused parameter 'argc'
../bug2.m:16: warning: unused parameter 'argv'
ANALYZE: ../bug2.m collectingEnabled
ANALYZE: ../bug2.m main
scan-build: Removing directory '/var/folders/Ee/Eeuv+JUYEF4h5NFY82FFvE+++TI/-Tmp-/scan-build-2009-03-09-15' because it contains no reports.

If I had to guess, this is probably because the compiler/analyzer is not agnostic about objc_getClass(). That is to say that it is not relying on the headers to properly declare objc_getClass(), but instead has a hidden and internal prototype built directly in to the compiler.

[llvmbot]

assigned to @tkremenek

[lattner]

BTW, ToT clang should not emit "warning: #import is a GCC extension" with -pedantic on .m files anymore.

[tkremenek]

If I had to guess, this is probably because the compiler/analyzer is not
agnostic about objc_getClass(). That is to say that it is not relying on the
headers to properly declare objc_getClass(), but instead has a hidden and
internal prototype built directly in to the compiler.

Your diagnosis is almost correct. The analyzer isn't even reasoning about objc_getClass(), although it has nothing to do with the headers. It also isn't reasoning about the 'defaultCollector' message.

[lattner]

Why does that matter? The condition is of the form:

x = (somestuff) ? 1 : 0;

You don't need to know what somestuff does to know that x is properly initialized, no?

[tkremenek]

Why does that matter? The condition is of the form:

x = (somestuff) ? 1 : 0;

You don't need to know what somestuff does to know that x is properly
initialized, no?

That's an excellent point. We shouldn't be flagging a false positive here. I'll dig into it further.

[tkremenek]

Why does that matter? The condition is of the form:

x = (somestuff) ? 1 : 0;

You don't need to know what somestuff does to know that x is properly
initialized, no?

That's an excellent point. We shouldn't be flagging a false positive here.
I'll dig into it further.

Chris: It looks like a broken AST. The DeclStmt for 'gcEnabled' isn't even in the AST. Notice that the first statement in the CompoundStmt is the IfStmt:

(CompoundStmt 0x1e2a250 <test2.m:8:31, line:15:1>
(IfStmt 0x1e2a1e0 <line:11:3, line:13:3>
(BinaryOperator 0x1e2a0a0 <line:11:6, /usr/include/objc/objc.h:49:31> 'int' '=='
(ImplicitCastExpr 0x1e2a060 test2.m:11:6 'int'
(DeclRefExpr 0x1e24110 col:6 'BOOL':'signed char' Var='gcEnabled' 0x1e044e0))
(ImplicitCastExpr 0x1e2a080 </usr/include/objc/objc.h:49:25, col:31> 'int'
(CStyleCastExpr 0x1e282c0 <col:25, col:31> 'BOOL':'signed char'
(IntegerLiteral 0x1e282a0 col:31 'int' 1))))
(CompoundStmt 0x1e2a1b0 <test2.m:11:24, line:13:3>
(CallExpr 0x1e2a160 <line:12:5, col:30> 'int'
(ImplicitCastExpr 0x1e2a140 col:5 'int (*)(char const *restrict, ...)'
(DeclRefExpr 0x1e2a0c0 col:5 'int (char const *restrict, ...)' FunctionDecl='printf' 0x1e25250))
(ImplicitCastExpr 0x1e2a190 col:12 'char const *restrict'
(StringLiteral 0x1e2a0e0 col:12 'char [16]' "GC is enabled.\n"))))
<<>>)
(ReturnStmt 0x1e2a240 <line:14:3, col:19>
(ParenExpr 0x1e2a220 <col:9, col:19> 'BOOL':'signed char'
(DeclRefExpr 0x1e2a200 col:10 'BOOL':'signed char' Var='gcEnabled' 0x1e044e0))))
(CompoundStmt 0x1e2a450 <test2.m:17:34, line:27:1>
(DeclStmt 0x1e2a520 line:18:3
0x1e2a390 "NSAutoreleasePool *pool =
(ImplicitCastExpr 0x1e2a500 <col:29, col:60> 'NSAutoreleasePool *'
(ObjCMessageExpr 0x1e2a4d0 <col:29, col:60> 'id':'struct objc_object ' selector=init
(ObjCMessageExpr 0x1e2a410 <col:30, col:54> 'id':'struct objc_object ' selector=alloc class=NSAutoreleasePool)))"
(DeclStmt 0x1e2a610 line:19:3
0x1e2a540 "BOOL gcEnabled =
(CallExpr 0x1e2a5f0 <col:20, col:38> 'BOOL':'signed char'
(ImplicitCastExpr 0x1e2a5d0 col:20 'BOOL ()(void)'
(DeclRefExpr 0x1e2a570 col:20 'BOOL (void)' FunctionDecl='collectingEnabled' 0x1e29e80)))"
(CallExpr 0x1e2a6d0 <line:21:3, col:39> 'int'
(ImplicitCastExpr 0x1e2a6b0 col:3 'int ()(char const *restrict, ...)'
(DeclRefExpr 0x1e2a630 col:3 'int (char const *restrict, ...)' FunctionDecl='printf' 0x1e25250))
(ImplicitCastExpr 0x1e2a700 col:10 'char const *restrict'
(StringLiteral 0x1e2a650 col:10 'char [16]' "GC enabled: %d\n"))
(ImplicitCastExpr 0x1e2a720 col:30 'int'
(DeclRefExpr 0x1e2a690 col:30 'BOOL':'signed char' Var='gcEnabled' 0x1e2a540)))
(ObjCMessageExpr 0x1e2a790 <line:23:3, col:16> 'void' selector=release
(DeclRefExpr 0x1e2a740 col:4 'NSAutoreleasePool *' Var='pool' 0x1e2a390))
(BinaryOperator 0x1e2a860 <line:24:3, /usr/include/sys/_types.h:91:33> 'NSAutoreleasePool *' '='
(DeclRefExpr 0x1e2a7c0 test2.m:24:3 'NSAutoreleasePool *' Var='pool' 0x1e2a390)
(ImplicitCastExpr 0x1e2a840 </usr/include/sys/_types.h:91:23, col:33> 'NSAutoreleasePool *'
(ParenExpr 0x1e2a820 <col:23, col:33> 'void *'
(CStyleCastExpr 0x1e2a800 <col:24, col:32> 'void *'
(IntegerLiteral 0x1e2a7e0 col:32 'int' 0)))))
(ReturnStmt 0x1e2a770 <test2.m:26:3, col:11>
(ParenExpr 0x1e2a8a0 <col:9, col:11> 'int'
(IntegerLiteral 0x1e2a880 col:10 'int' 0))))

[lattner]

You're right, here's a reduced testcase, I'll fix.

int collectingEnabled (void) {
int X;
int gcEnabled = [X defaultCollector];
}

[lattner]

Fixed here:
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20090309/013668.html

This was a really really bad bug, thank you for reporting it!