Page MenuHomePhabricator

Do not call replaceAllUsesWith to upgrade calls to ARC runtime functions to intrinsic calls

Authored by ahatanak on Aug 9 2019, 6:58 PM.



This fixes a bug in r368311.

It turns out that the ARC runtime functions in the IR can have pointer parameter types that are not i8* or i8**. Instead of calling replaceAllUsesWith, manually bitcast the arguments and return values before and after calling the intrinsic functions.


Diff Detail


Event Timeline

ahatanak created this revision.Aug 9 2019, 6:58 PM
rjmccall accepted this revision.Aug 12 2019, 10:26 AM

LGTM, although I'm not generally familiar with the upgrader code.


Intrinsic functions can't be used in any way except calling them, right?

This revision is now accepted and ready to land.Aug 12 2019, 10:26 AM
steven_wu added inline comments.Aug 12 2019, 10:32 AM

I probably missed this from previous commit, shouldn't this function return bool?

ahatanak marked 2 inline comments as done.Aug 12 2019, 11:21 AM
ahatanak added inline comments.

I changed the return type to void in a follow-up commit since nothing seemed to be using the return value. Do these functions have to return anything?


Intrinsic functions can only be called, but it's possible to use ObjC ARC functions without calling it. For example, if the code below was compiled by clang and the auto-upgrader was reading the bitcode, the check would prevent the auto-upgrader from replacing the reference to @objc_autorelease:

id objc_autorelease(id);

void foo2(void *);

void foo1(void) {
  //   call void @foo2(i8* bitcast (i8* (i8*)* @objc_autorelease to i8*))

I'm not sure why anyone would do something like this, but currently clang doesn't reject it and I don't think we can rule out the possibility that someone is generating an IR like this.

Ah, yes, if this isn't just working on intrinsic functions then that makes total sense.

steven_wu accepted this revision.Aug 12 2019, 12:26 PM
steven_wu added inline comments.

I guess it is just the convention and the return value is not used anymore. This is fine.

ahatanak closed this revision.Aug 12 2019, 4:55 PM

Thanks, committed in r368634.