Page MenuHomePhabricator

[WebAssembly] Fix imported function symbol names that differ from their import names in the .o format
ClosedPublic

Authored by sunfish on Feb 1 2019, 5:12 PM.

Details

Summary

It appears that in the current .o file format, symbol names for undefined function symbols are not included, which means that we can't have symbols where the symbol name differs from the import name.

The following patch fixes it. (It includes both llvm and lld changes, as they're closely related, but I'll split them when committing.) With this patch, the following testcase:

__attribute__((import_module("bar"), import_name("qux"))) void foo(void);

void _start(void) {
    foo();
}

compiles with `clang -nostdlib test.c --target=wasm32-unknown-unknown -Wl,--allow-undefined-file=undefined.txt

where undefined.txt contains:

foo

into a wasm that contains this import:

(import "bar" "qux" (func $foo (type 0)))

Since this requires a change to the binary form, this patch also bumps the metadata version number.

Diff Detail

Repository
rL LLVM

Event Timeline

sunfish created this revision.Feb 1 2019, 5:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2019, 5:12 PM
sunfish updated this revision to Diff 185170.Feb 4 2019, 3:13 PM
  • Use a symbol flag to indicate when a custom symbol name is present.
  • Revert the version number bump.
  • Allow globals and events to have custom symbol names too, for consistency.
sbc100 added a comment.Feb 4 2019, 3:27 PM

Looking good.

Can we can a test in the MC/WebAssembly and lld/test/wasm?

Any not do the same for data symbols?

We probably want to fail in lld if two different object import the same symbol with the different import name or import module, but that was already on my radar and I'm OK to do that as a followup.

sunfish updated this revision to Diff 185835.Feb 7 2019, 11:30 AM
  • Add MC/WebAssembly and lld tests
  • Set the WASM_SYMBOL_NAME_EXPLICIT flag when a symbols name differs from its import name.

Looking good.

Can we can a test in the MC/WebAssembly and lld/test/wasm?

Done.

Any not do the same for data symbols?

Data symbols don't have a corresponding import, and already include a symbol string unconditionally.

We probably want to fail in lld if two different object import the same symbol with the different import name or import module, but that was already on my radar and I'm OK to do that as a followup.

Yes, that sounds good to do.

sbc100 accepted this revision.Feb 7 2019, 12:16 PM

lgtm % comment about tests

Remember to update the tool-convensions/Linking.md with this new symbol flag

tools/lld/test/wasm/import-names.ll
93 ↗(On Diff #185835)

I'd be tempted to only include IMPORT and CUSTOM sections, and to use CHECK-NEXT.

We only care about the name of function 0 too.

e.g.

; CHECK:   - Type:            CUSTOM
​; CHECK-NEXT:     Name:            name
; CHECK-NEXT:     FunctionNames:
​; CHECK-NEXT:       - Index:           0
​; CHECK-NEXT:         Name:            f0

Same for the MC test

This revision is now accepted and ready to land.Feb 7 2019, 12:16 PM
This revision was automatically updated to reflect the committed changes.

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-ubsan/builds/10518

FAIL: LLVM :: tools/llvm-readobj/symbols.test (29567 of 29761)
******************** TEST 'LLVM :: tools/llvm-readobj/symbols.test' FAILED ********************
Script:
--
: 'RUN: at line 1';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readobj --symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.coff-i386    | /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/symbols.test -check-prefix COFF
: 'RUN: at line 3';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readobj --symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.elf-i386    | /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/symbols.test -check-prefix ELF
: 'RUN: at line 5';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readobj --symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.wasm    | /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/symbols.test -check-prefix WASM
: 'RUN: at line 9';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readobj --symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.elf-i386 > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.symbols
: 'RUN: at line 10';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readobj --syms /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.elf-i386 > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.syms
: 'RUN: at line 11';   cmp /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.syms
: 'RUN: at line 12';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readobj -t /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.elf-i386 > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.t
: 'RUN: at line 13';   cmp /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.t
: 'RUN: at line 14';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readelf -s -elf-output-style LLVM /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.elf-i386 > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.lowers
: 'RUN: at line 15';   cmp /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.lowers
--
Exit Code: 1

Command Output (stderr):
--
/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/symbols.test:107:12: error: WASM-NEXT: is not on the line after the previous match
WASM-NEXT: Module: env
           ^
<stdin>:31:8: note: 'next' match was here
 ImportModule: env
       ^
<stdin>:29:3: note: previous match ended here
 ]
  ^
<stdin>:30:1: note: non-matching line after previous match is here
 ImportName: puts
^