We are going to change libc++abi instead: https://reviews.llvm.org/D64961

Maybe add a test for this?

In D61452#1488789, @sunfish wrote:

If "$sysroot/lib" ends up coming to mean "wasm32" because people come to depend on that, then wasm64 may end up needing to be different in a gratuitous way, which I'd like to avoid.

I'd like to keep our sysroots tidy when we can. If some libraries are installed in lib/wasm32-wasi and others lib for no reason other than build script inertia, that's untidy.

It's not an absolute for me, but I am inclined to see if we can understand the need better first. Single-arch sysroots are possible either way, for example.

In D64537#1586614, @quantum wrote:

In D64537#1586577, @dschuff wrote:

Another high-level question (based just on reading the CL description): The TLS-size intrinsic is per-function, does that mean that the tls-init function is called for every function? are there just multiple TLS sections per object file?

The TLS-size intrinsic returns the total size of TLS for the module it's called from. The initialization function initializes the TLS for the entire module.

In D64537#1586556, @dschuff wrote:

The offset field of a segment can be a constant expression which can be a global.get of an imported global. So we could have an imported global __tls_base which is different for each thread, and have an active segment with that as its segment offset?

I didn't know that it could have been a constant expression. I don't think this would have worked very well on the main thread though, since we need to run malloc before we can compute __tls_base. I think this requires the global to be mutable, if only because we need to be able to initialize it on the main thread.

I think the problem here is that there is no such thing as a first class "type" symbol, there are only functions symbols that can be referenced with @TYPE_INDEX to find where they live in the type index space.

Its really great to see this change BTW! Thanks.

I wonder if __tls_base should be allocated by the embedder (or by the parent/creator thread). Then it could be an *immutable* global import that is allocated up front. I guess __stack_pointer should be treated in the same way (except mutable).

Can we get a test for this?

• rebase

• fix typo

• cleanup

I tried adding a dependency on TransformUtils to Analysis/LLVMBuild.txt but that generated:

This broke my build too. Could be related to the fact that I build with -DBUILD_SHARED_LIBS=ON?

• feedback

This looks like we we doing the exact same thing that x86 is. So it would make sense to me that whatever we end up doing we should continue to match x86. Perhaps landing this as-is and then opening a bug to turn this into an unreachable (for both x86 and wasm) is one option?

I expanded this change a little. @ruiu PTAL.

• cleanup test

• Report undefined symbols when scanning relocations

Address feedback

Nice! I wonder if its worth adding some kind of static assert or unittest to avoid accidental increases in future? Either way lgtm.

Nice!

Can you take a closer look? I'm pretty sure this change is correct. At the very least an improvement, and not a regression.

• revert part

I'm sad to see more code duplication but OK with adding along with the TODO to find a better way.

The comment also says exactly that does't it?

I'm pretty certain yes. The old code only loads archives one object at a time if all objects are bitcode. I didn't change that part.

Remember to remove "A symbol __global_base is added so that code may know where the shadow
memory ends and real memory begins." from the CL description.

As I said in D63742, I don't really like that fact that __global_base and __data_end are use for the start and end of the static data but use completely different naming conventions, but since we already expose the --global-base command line arg I guess its hard to make the align without changing the command line arg and/or change the name of __data_end. So I think I'm ok with this for now, but I'd like to find a way to make these consistent in the future.

The previous behaviour was also to ignore archives that contain only ELF files but no index.

This change only effects archives without an index. If you previously created an archive with both bitcode *and* ELF files *and* you didn't create and index, the previous behaviour was to completely ignore the archive. This seems bad to me. The new behaviour is to give an error. We could downgrade it to a warning but both GNU ld and wasm-ld give a hard error in this case.

Oh cool, I totally didn't think of that. I find it a bit strange to have the test script within the invalid binary wasm file, but hey if it works its kinda cool.

lgtm with comment

Ideally I'd rather see the invalid file be generated on the fly, but this is also ok since it should never really change.

I wonder if we should use the linux/unix convention or edata etext and end? Terrible names obviously but there is precedent. I can't remember why I didn't do that for data_end and heap_base.

I guess this means we are also missing a test case for --emit-relocs + -fPIC?

Thanks!