typo

In D99887#2669582, @MaskRay wrote:

I don't know wasm enough... In ELF, --export-dynamic-symbol only affects defined symbols. --export-dynamic affects all defined symbols. They don't have effect on other symbols.

In D99887#2669570, @dschuff wrote:

Pre-existing, but what does --export plus --allow-undefined do?

I dont love the name of this option.. but I failed to come up with any thing nicer.

rebase

cleanup

Man, these kind of bug are so common, and so hard to test and occur across so many different tools.. I wonder if we would have made a different choice if we have known how many issues LEB128 would cause.

Nice!

Great!

feedback

feedback

In D81803#2592409, @nikic wrote:

Just FTR, this change had to be reverted in Rust's LLVM fork because it breaks compilation on Windows 7, see https://github.com/rust-lang/rust/issues/81051.

add tests

feedback

Rebase against mono-repo

lgtm!

lgtm % comments / nits

I don't have any more objections myself. @sunfish , WDYT of this addition? My feeling is that if it helps swift then its probably worth adding.

cleanup

cleanup

Mostly lgtm. Just a few nits at this point.

lgtm with a test for the new error(s).

Emscripten test suite passes. lgtm.

I'm trying to think how this could effect either wasi-sdk or emscripten and I can't think of any ways.. but I will run the emscripten test suite with this change just in case.

Assuming all the tests pass this lgtm!

No thats fine. I was just making sure we wouldn't break an existing test..

Is there no test that includes this encoding?

Does this mean the --relocatable always produces output with an entry in the symbol table for __indirect_function_table? i.e. does the linker always produce non-MVP object files?

Is the "pc-rel" in the title a little misleading? How about just "relative" or "location relative"?

In D95914#2559003, @tlively wrote:

@sbc100 This revision is responsible for the sanitizer errors on the emscripten autorollers: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket.appspot.com/8855537012484712752/+/steps/Emscripten_testsuite__upstream__wasm3_/0/stdout

Would you be able to take a look?

Nice! I like this much better now. Thanks for making all those changes.

we when we comes to assigning indexes we do something like this:

Ok so we can see this as an interim thing. I think I'm OK with that. Could you add a comment about that, or at least say why we want to support both the old crt1 and the new crt1-command at the same time (i.e. so that wasi-libc can easily suppot both old and new llvm right?)

In D89274#2555511, @sunfish wrote:

It's to ensure that older LLVM works with newer WASI libc, and newer clang works with older WASI libc. New-style commands require [lld support]. We can assume that if clang is updated, lld has the requisite support.

That said, I'm open to other ideas here.

[lld support]: https://github.com/llvm/llvm-project/commit/6cd8511e5932e4a53b2bb7780f69489355fc7783

Why not just use crt1.o in both cases? If you are going to prefer crt1-command.o in all cases then a toolchain would have no reason to ever ship crt1.o would it? (since it would always be ignored?)

rebase