[PATCH] D41922: [WebAssembly] When loading libraries look for companion `.imports` file

Sam Clegg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 29 12:50:27 PST 2018


sbc100 added a comment.

In https://reviews.llvm.org/D41922#991056, @ncw wrote:

> In https://reviews.llvm.org/D41922#990851, @sbc100 wrote:
>
> > I'm not sure about that.  Isn't it libc that knows exactly what symbols it expects to come from the embeder?  Sure with emscripten we could imagine a complex compiler driver that is aware of all the JS functions and construct a long list of possible imports to be passed to linker, but I don't think that a very nice approach and requires complexity in the driver that it would nice to avoid.   I like Dan's approach to being able to mark such function in the source language: https://reviews.llvm.org/D42520.
>
>
> Not from my point of view! That would be an "inversion of control" in the Dependency Injection jargon. We often write software that has an external dependency on a higher layer (for example Musl does this when it invokes the "kernel"/embedder for syscalls, or a library that requires the developer to define/resolve some symbols that the library doesn't provide). The rule is: if you implement it, you get to say where it comes from. So anything that Musl doesn't provide, Musl shouldn't care where it comes from: whoever provides the syscalls has business of getting them through to Musl. Since Musl doesn't provide the syscalls, it feels wrong for me have libc.imports file shipped along with Musl, since that imports file is effectively a file whose whole purpose is to declare the implementation language (JS vs C) for certain routines. Only the person who provides those routines should care which language they're written in (ie which "side" of the JS-Wasm interface provides them).
>
> You mentioned a "complex" linker driver that is "aware of all the JS functions". But what I'm envisaging is fairly simple: Emscripten (and my "minscripten" toolchain) currently construct a JS file that loads the Wasm module and builds some JS shims around it. Those tools are fully aware of which linkable symbols are available on the JS side - it's not a hardcoded list, it's in fact part of the working set of the Emscripten toolchain; they've got a variable already in their Python code that knows about the those symbols, because it's an unavoidable part of building the JS module. There's no practical reason why LLD couldn't be told an exact list of which functions are available on the JS side.
>
> And indeed, the nice thing about that is it's fully automatic - who wants to build a `.imports` file manually? If you write some functions in C, and some functions in JS, you want Emscripten to link them together for you, rather than having to manually write a second set of declarations mirroring your JS files for the benefit of LLD. Anything that can be derived automatically from the developer's code, should be. Hence the build toolchain ought to be passing the list of JS-defined symbols down to LLD, since those are precisely the symbols that it's OK for LLD to be importing.
>
> > In any case it seems like we can support both methods: import defined by the libraries you link as well as explicitly allowed imports via the linker command line.
> > 
> > On this topic I was considering removing --allow-undefined-file since we are no longer using it.. and perhaps renaming --allow-undefined to --import (to match the existing --export).  Are you using these flags currently in your toolchain?
>
> I am using `--allow-undefined-file`, and I think that it is useful. I'm not using `--allow-undefined` or `--export`, but I suspect they must be useful to someone.


I agree that providing the imports file is a hack and needs to go.  That is way we want to allow for source level attributes.    Once that lands we can remove support for the imports file.  If you prefer the idea of the linker driver controlling this stuff we don't need to remove the support for `--allow-undefined/-file`.  I don't think there is a reason these two approaches can't both exist (Unless you are opposed to Dan added the new source attribute?).    I think the main thing is that we agree this imports hack should be removed.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D41922





More information about the llvm-commits mailing list