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

Mon Sep 21 16:52:41 PDT 2020

sbc100 added a comment.

Thanks for fixing this!  Sorry it took me so long to get back to you.  This lgtm % some comments.

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

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.

================
Comment at: llvm/lib/MC/WasmObjectWriter.cpp:1233
+                           ": absolute addressing not supported!");
+      registerFunctionType(*cast<MCSymbolWasm>(BS));
     }
----------------
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.

================
Comment at: llvm/test/MC/WebAssembly/alias-relative.ll:31
+; CHECK-NEXT:        Locals:          []
+; CHECK-NEXT:        Body:            41004100360284808080000B
+
----------------
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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87407