The attached patch generates warnings of cases where an ObjC message is sent to a nil object and the size of return type of that message is larger than the size of void pointer. This may result in undefined return values as described in PR 2718 [1]. The patch also includes test cases.
Warnings are currently generated only in those cases where we know for sure that an object pointer is nil. Enabling this warning in all possible nil cases would probably produce too many false positives.
clang-nil-checker.patch (7.82 KB)
Sorry, I had the wrong From-account selected in Mail.app when sending the patch, that's why the wrong From-name.
- Nikita Zhuk
Yes, that's the correct one.
I have a question related to the nil receiver checks. Would it be possible to take the nil receiver assumption (ObjC messages with size of return value <= sizeof(void*) sent to nil always return zero) into account in GRExprEngine, so that the number of false positives could be reduced even further? For example, the following cases generate the nil receiver warning, although the conditions of 'if', 'for' and 'while' statements are always false when the 'obj' is nil (because any message sent to nil returns zero).
for(int i = 0; i < [obj intM]; i++)
{
long long j = [obj longlongM]; // no-warning (currently generates the 'nil receiver' warning)
}
while([obj intM] != 0)
{
long long j = [obj longlongM]; // no-warning (currently generates the 'nil receiver' warning)
}
if([obj intM] != 0)
{
long long j = [obj longlongM]; // no-warning (currently generates the 'nil receiver' warning)
}
Hi Nikita,
I see; [obj intM] should evaluate to 0 if obj is nil. Sure that would be easy to do!
Incidentally, this example touches on same dangerous territory as far as reasoning about sending messages to nil, as some of these semantics are architecture specific and could change with the whim of any updates to the runtime.
As you know, for portability the only thing we can assume is (from the referenced doc):
"The Objective-C runtime assumes that the return value of a message sent to a nil object is nil, as long as the message returns an object or any integer scalar of size less than or equal to sizeof(void*)."
While this seems pretty clear for 'int', pointers, etc., long long doesn't fall into that category. In fact that's only kosher on Intel macs, and the documentation even says that is undefined for PowerPC. I'm also not certain what the results would be for ARM, but I imagine that the result would be undefined as well. This means this code will break when used to build an iPhone app.
I think there are two strategies we can take here. We either issue target-specific warnings or target-independent warnings. Target-independent warnings would flag cases that are potentially non-portable. I realize that you weren't arguing that the analyzer specially handle the [ob longlongM] case, but I thought I would bring these issues up while we are here.
Ted
Hi Nikita,
I see; [obj intM] should evaluate to 0 if obj is nil. Sure that would be easy to do!
Great!
Incidentally, this example touches on same dangerous territory as far as reasoning about sending messages to nil, as some of these semantics are architecture specific and could change with the whim of any updates to the runtime.
As you know, for portability the only thing we can assume is (from the referenced doc):
"The Objective-C runtime assumes that the return value of a message sent to a nil object is nil, as long as the message returns an object or any integer scalar of size less than or equal to sizeof(void*)."
While this seems pretty clear for 'int', pointers, etc., long long doesn't fall into that category. In fact that's only kosher on Intel macs, and the documentation even says that is undefined for PowerPC. I'm also not certain what the results would be for ARM, but I imagine that the result would be undefined as well. This means this code will break when used to build an iPhone app.
Yes, that's why I used the "[obj intM]" method in those examples (underlying assumption being that the "intM" method returns 'int', which is a safe return type for messages being sent to nil). If condition of an 'if' statement would have been "if([obj longlongM] != 0)", then static analyzer should issue the nil receiver warning and not to assume anything about the value of the condition (as it does now).
I have a feeling (based on communication with other developers, no hard data to back it up) that this particular issue is not very well-known in Obj-C developer community, so getting this check into Clang is a very good thing (of course, static analysis is not enough to find all possible cases of incorrect use of the nil receiver, but it's a start).
I think there are two strategies we can take here. We either issue target-specific warnings or target-independent warnings. Target-independent warnings would flag cases that are potentially non-portable. I realize that you weren't arguing that the analyzer specially handle the [ob longlongM] case, but I thought I would bring these issues up while we are here.
Ted
I would personally vote for target-independent warnings and portable code, but there're certainly developers with different requirements.