[PATCH] D87407: [WebAssembly][MC] Fix computation of relative symbol offset

Mon Sep 21 20:47:17 PDT 2020

ddcc added a comment.

> Is this essentially a fix for the work that was started in https://reviews.llvm.org/D79462 ?

Yeah, and enabling CFI will enable the LowerTypeTests pass which can generate relative addressing.

> If we I wonder if the existing test for aliases at offset could be extended to include a relocation: `llvm/test/MC/WebAssembly/offset.s`?  Rather than adding a new test?
> I'd be happy to rename the existing test to `alias-relative.ll` or `alias-offset.ll` which seem more meaningful.
> We are currently trying to move away from `.ll` tests in the MC directory to `.s` where possible so even if we do need to add a new test `.s` would probably be prefered.

Ok, this is pretty small so I can just merge it into the existing test.

================
Comment at: llvm/lib/MC/WasmObjectWriter.cpp:1233
+                           ": absolute addressing not supported!");
+      registerFunctionType(*cast<MCSymbolWasm>(BS));
     }
----------------
sbc100 wrote:
> Is this a separate issue?  It looks like a possible crash.  Did you hit this case during testing?   I'm curious what conditions would cause this.
Currently, if you enable CFI for WebAssembly (`-fsanitize=cfi -fvisibility=hidden`), the LowerTypeTests pass will generate a null base pointer as an absolute address, which eventually triggers here. It was a workaround from back when I originally implemented CFI support; D87258 reworks the WAsm portion of that pass to avoid it.

================
Comment at: llvm/test/MC/WebAssembly/alias-relative.ll:31
+; CHECK-NEXT:        Locals:          []
+; CHECK-NEXT:        Body:            41004100360284808080000B
+
----------------
sbc100 wrote:
> ddcc wrote:
> > I haven't been able to get obj2yaml or llvm-readobj to print out the full relocation information. Is there a more appropriate tool to use?
> The relocation is shown above.. what more information are you looking for?
> 
> I would recommend `objdump -d -r` but I'm not sure its works well enough with WebAssembly at this point.
Never mind, I misunderstood the relocation behavior.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87407