[PATCH] D42539: [WebAssembly] Handle relocations where provisional value != index

Wed Jan 31 02:31:05 PST 2018

ncw added a comment.

In https://reviews.llvm.org/D42539#992827, @sbc100 wrote:

> I'm a little sad to loose this extra level to checking.
>
> When you say "we have relocations where the provisional value is not the same as the symbol index", where are you seeing this?  Not in any of our tests presumably?  Can we write a test case for this?

I don't feel too strongly about needing to check the provisional values here: they don't actually do anything - and we aren't checking them for TABLE/MEMORY relocations anyway.

Yes, I'm seeing the problem in our tests. As it says in the description above, the issue is that as soon as https://reviews.llvm.org/D42095 lands, LLC will start to output relocations that cause this check to fail, so this review needs to land before https://reviews.llvm.org/D42095 in order for LLD to keep on working. This chunk was originally part of "LLD patch #3" (and https://reviews.llvm.org/D42095 was originally titled "LLVM patch #3"), so they are a matched pair of changes. This is only coming in its own review because somehow when https://reviews.llvm.org/rLLD323168 was landed this little chunk was dropped.

The code as it stands is just dodgy code full stop, it's reading a relocation "value" and comparing it against the relocation "index". If it was safe to assert that the "value" and "index" are equal, then we wouldn't need both fields on the relocation object! We're only getting away with it here because WasmObjectWriter (until https://reviews.llvm.org/D42095 lands) is currently writing out the "index" as the provisional value.

(Long-term we simply can't write out the index as the value. This is because in general there are more symbols than functions/globals, so the indexes will be larger than the Wasm values. Hence writing out an index as a value is going to cause the Wasm module to fail validation. Your symbol index might be "4", and there might be only three functions, so the Wasm module wouldn't validate if you tried to put a "4" as the operand of the call instruction.)

Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D42539