Drop explicitly passed -lc unless -nodefaultlibs or similar is passed #22765
Conversation
There have been a lot of bugs when the caller passes `-lc` over the years. For example it crashes if we do: ``` emcc -lc -sDISABLE_EXCEPTION_CATCHING=0 -sMIN_SAFARI_VERSION=150000 ``` Rust likes to pass `-lc`. Let's drop it and stop causing trouble for Rust. Resolves emscripten-core#22758, emscripten-core#22742 and would have resolved emscripten-core#16680 if it hadn't disappeared first.
|
@sbc100 does this need a rebaseline code size or something like that? |
| # similar. It's important to link libc after our other injected system | ||
| # libraries like libbulkmemory, but user linked libraries go ahead of | ||
| # system libraries, so if the user passes `-lc` then we can get crashes. | ||
| continue |
There was a problem hiding this comment.
While I agree this fixed the program at hand, it also seems a little to magical. What if user explictly wants to do something like this:
-lmylib_prelibc -lc -lmylib_postlibc
In this case removing the explicit -lc provided by the user seems wrong/non-intuitive.
Perhaps it would be better to fix rust not to pass the explicit -lc, do you know why rust insists on doing this?
There was a problem hiding this comment.
Perhaps we should at least warn the user when we do this?
There was a problem hiding this comment.
Perhaps we should at least warn the user when we do this?
I suppose so. But passing -lc at all is also very likely to be wrong if it's likely to be depending on other libraries that are automatically injected at the very end to work correctly. I guess the warning could say to pass -nolibc if they need to link libc in a particular order though -nolibc seems to also disable linking of libmalloc and libnoexit if they should be added.
What about the option of injecting the system libs ahead of the user object files? Do you think that would have any negative consequences?
There was a problem hiding this comment.
Perhaps it would be better to fix rust not to pass the explicit
-lc, do you know why rust insists on doing this?
Well it seems to work for them with all of the other linker they use. I'm trying to fix it from rust's side too, but it seems to me like it's the linker's responsibility to correctly handle this and not the consumer's responsibility not to pass it.
There was a problem hiding this comment.
Is there any downside to remove it that you know of? Why is it even there?
There was a problem hiding this comment.
No, the workaround on the rust side is pretty short nad has no downsides I know of. It was rejected two years ago when I tried to merge the fix to rust, but I opened a new PR with the same fix and maybe it will be accepted this time. I think their response was roughly "if it breaks to pass -lc maybe it breaks when you pass all sorts of things, we need to figure out what the actual cause is and implement a real fix".
There was a problem hiding this comment.
I suppose so. But passing
-lcat all is also very likely to be wrong if it's likely to be depending on other libraries that are automatically injected at the very end to work correctly. I guess the warning could say to pass-nolibcif they need to link libc in a particular order though-nolibcseems to also disable linking oflibmallocandlibnoexitif they should be added.
Yes, anyone of passes -lc in emscripten is likely going to have a hard time because they would need to also add -lmalloc and -lnoexit and maybe -lbulkmemory. i.e. in emscripten libc is split up into multiple libraryies which are order dependent.
So I think that removing -lc is likely the right thing to do but I also worry that doing it silently could break some advanced users who are passing all of those libraries. i.e. it will fix rust but perhaps break some other use case.
Another solution would be to merge all out libc libraries into a single one but then we would end up with even more variants, e.g. libc-noexit-bulkmemory-debug-mt.a. This complexity would be largely hidden, but it would mean would have to build probably over 100 different libc versions :-/ Having separate libraries allows us to avoid that.
What about the option of injecting the system libs ahead of the user object files? Do you think that would have any negative consequences?
There was a problem hiding this comment.
Reading the Rust codebase, I am not at all sure where it gets decided that it should pass -lc in the first place, but it's easy to filter it out as it's constructing the linker invocation.
There was a problem hiding this comment.
"if it breaks to pass
-lcmaybe it breaks when you pass all sorts of things, we need to figure out what the actual cause is and implement a real fix".
I guess now that we know more about the problem we can make a better argument for removing it. We can say "supplying -lc on its own just doesn't work with emscripten and will break the build". Also, since the compiler always includes libc by default this flag is redundant and pointless. Removing it is simply better for everyone.
There was a problem hiding this comment.
Right, I'd just really like either Emscripten or Rust to merge a fix for this, I'll see what comments I get on the Rust PR.
There have been a lot of bugs when the caller passes
-lcover the years. For example it crashes if we do:Rust likes to pass
-lc. Let's drop it and stop causing trouble for Rust.Resolves #22758, resolves #22742 and would have resolved #16680 if it hadn't disappeared first.