[PATCH] D120781: [IRLinker] materialize Functions before moving any

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D120781#3355054 <https://reviews.llvm.org/D120781#3355054>, @nickdesaulniers wrote:

> In D120781#3353252 <https://reviews.llvm.org/D120781#3353252>, @dexonsmith wrote:
>
>> Another potential concern: what happens with weak linkage? I.e., what if you have:
>>
>>   declare void @sink(i8*)
>>   define linkonce_odr void @haslabel() {
>>     br label %label
>>   label:
>>     ret void
>>   }
>>   define void @usesblockaddress() {
>>     call void @sink(i8* blockaddress(@haslabel, %label))
>>     ret void
>>   }
>>
>> What if this TU's `@haslabel` is not selected by the linker? (Maybe this is just a failed link, since there's no guarantee the other `@haslabel` still has a basic block, or that it means the same thing, since it could be optimized away if its TU didn't have a `blockaddress` pointing at it.)
>
> The above test case currently runs through `llvm-link` just fine, regardless of my patch.

Sorry, I guess I wasn't clear. That's not a complete test case. Complete testcase needs two files so that `@haslabel` gets replaced by another TU. For example:

  % cat a.ll
  define weak_odr void @haslabel() {
    ret void
  }
  % cat b.ll
  declare void @sink(i8*)
  define linkonce_odr void @haslabel() {
    br label %label
  label:
    ret void
  }
  
  define void @usesblockaddress() {
    call void @sink(i8* blockaddress(@haslabel, %label))
    ret void
  }

Result of linking a.ll and b.ll looks to me like it produces a bogus `blockaddress`:

  % ./staging/build/bin/llvm-link a.ll b.ll -S
  ; ModuleID = 'llvm-link'
  source_filename = "llvm-link"
  
  define weak_odr void @haslabel() {
    ret void
  }
  
  define void @usesblockaddress() {
    call void @sink(i8* inttoptr (i32 1 to i8*))
    ret void
  }
  
  declare void @sink(i8*)

Maybe it should error instead... or maybe taking a `blockaddress` to a weak function shouldn't be allowed at all?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120781/new/

https://reviews.llvm.org/D120781