This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen][ObjC] Fix broken IR generated when there is a nil receiver check
ClosedPublic

Authored by ahatanak on Jan 21 2021, 3:05 PM.

Details

Summary

This patch fixes a bug in emitARCOperationAfterCall where it inserts the fall-back call after a bitcast instruction and then replaces the bitcast's operand with the result of the fall-back call. The generated IR without this patch looks like this:

msgSend.call:                                     ; preds = %entry
  %call = call i8* bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to i8* (i8*, i8*, i8*)*)(i8* %6, i8* %5, i8* %4) #4, !clang.arc.no_objc_arc_exceptions !11
  br label %msgSend.cont

msgSend.null-receiver:                            ; preds = %entry
  call void @llvm.objc.release(i8* %4) #1, !clang.imprecise_release !11
  br label %msgSend.cont

msgSend.cont:                                     ; preds = %msgSend.null-receiver, %msgSend.call
  %8 = phi i8* [ %call, %msgSend.call ], [ null, %msgSend.null-receiver ]
  %9 = bitcast i8* %10 to %0*
  %10 = call i8* @llvm.objc.retain(i8* %8) #1

Notice that %9 = bitcast i8* %10 to %0* is taking operand %10 which is defined after it.

To fix the bug, this patch modifies the insert point to point to the bitcast instruction so that the fall-back call is inserted before the bitcast. In addition, it teaches the function to look at phi instructions that are generated when there is a check for a null receiver and insert the retainRV/claimRV instruction right after the call instead of inserting a fall-back call right after the phi instruction.

rdar://73360225

Diff Detail

Unit TestsFailed

Event Timeline

ahatanak created this revision.Jan 21 2021, 3:05 PM
ahatanak requested review of this revision.Jan 21 2021, 3:05 PM
rjmccall accepted this revision.Jan 21 2021, 3:16 PM

This looks good, but in the long term it would be great if we could eliminate the need for emitARCOperationAfterCall and just emit the call properly in the first place.

Please add a test where we have a null check around an invoke.

This revision is now accepted and ready to land.Jan 21 2021, 3:16 PM
ahatanak updated this revision to Diff 318366.Jan 21 2021, 5:36 PM

Add a test for invoke and a FIXME for eliminating the need for emitARCOperationAfterCall.

This revision was landed with ongoing or failed builds.Jan 21 2021, 5:39 PM
This revision was automatically updated to reflect the committed changes.