[PATCH] D30174: [Sema][ObjC] Warn about 'performSelector' calls with selectors that return record types
Alex Lorenz via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 21 09:21:55 PST 2017
arphaman added a comment.
In https://reviews.llvm.org/D30174#681890, @ahatanak wrote:
> In https://reviews.llvm.org/D30174#681843, @arphaman wrote:
> > Yes, we do. Primarily for the following reason: even if some target may return a struct in a register, another target isn't guaranteed to do the same thing. It's better to always warn about this rather than accept some small structs. Furthermore, the official documentation states that "For methods that return anything other than an object, use NSInvocation." , so methods that return records are completely prohibited by the docs.
> OK, I think you are right. It doesn't seem like a good idea to warn when compiling for one target and not warn when compiling for another target.
> If the official documentation says the method should return an object, why not prohibit all methods that don't return a pointer to an object (methods like returnsInt in the test case)? Doing so will also take care of methods that don't return struct but still return via sret (for example, I think some targets return vectors via sret).
I think it would catch too many "false positives" if we prohibit all primitive types. This might make the warning less effective as well, since users might get annoyed and completely disable the warning if they think that it's too strict. Maybe we can have a stricter version of the warning that's not on by default? I agree that we can warn on vectors as well though.
> Also, the documentation says that methods passed to performSelectorInBackground and performSelectorOnMainThread should not have a significant return value. Does it mean that the methods can have any return type but the return value will be ignored (I noticed that those two methods return "void" unlike performSelector which returns "id")?
No, they still use objc_msgSend, so the memory corruption problem still applies to them as they can write to the object thinking that it's the pointer to the return value. The fact that `performSelectorInBackground` and `performSelectorOnMainThread` don't return anything doesn't really make a difference.
More information about the cfe-commits