function signature mismatch with thinLTO and iostreams since 3.0.0

[manxorist]

We (libopenmpt) are seeing the following warnings in our CI builds since updating emscripten to 3.0.0:

wasm-ld: warning: function signature mismatch: _ZNSt3__215basic_streambufIcNS_11char_traitsIcEEE7seekposENS_4fposI11__mbstate_tEEj
>>> defined as () -> void in /home/openmpt/opt/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/thinlto/libc++.a(ios.instantiations.o)
>>> defined as (i32, i32, i32, i32) -> void in lto.tmp

wasm-ld: warning: function signature mismatch: _ZNSt3__215basic_streambufIcNS_11char_traitsIcEEE7seekoffExNS_8ios_base7seekdirEj
>>> defined as () -> void in /home/openmpt/opt/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/thinlto/libc++.a(ios.instantiations.o)
>>> defined as (i32, i32, i64, i32, i32) -> void in lto.tmp

This is reproducible with Emscripten SDK 3.0.0 in PATH by running make CONFIG=emscripten ONLY_TEST=1 in the root directory of the repository on Linux (maybe also macOS, but we do not test our emscripten build system from macOS).

I have not investigated any further about possible causes yet.

[kripken]

This may be an LLVM LTO bug perhaps, but that's just a guess.

I'd try to rebuild everything from scratch, including emcc --clear-cache to make sure that's not the issue. If it's not that, bisection is probably the easiest way to figure things out, unless you have a fast way to make a small standalone testcase,

https://emscripten.org/docs/contributing/developers_guide.html#bisecting

[manxorist]

Our builds are run from newly generated docker containers for each emscripten SDK release. Thus, there should be no emscripten cache leftovers from earlier versions at all. We do however have ccache in PATH for other build targets in these containers and keep its cache across container rebuilds, but that does not affect emscripten at all (I just verified that).

I also tested emcc --clear-cache locally, does also not change anything.

I'll try building a minimal testcase next.

[manxorist]

minimal testcase:

manx@appendix:~/projects/openmpt/trunk-1/testcase$ cat testcase.cpp
#include <iostream>

int main( int /*argc*/ , char * /*argv*/ [] ) {
        std::cout << std::endl;
        return 0;
}
manx@appendix:~/projects/openmpt/trunk-1/testcase$ em++ -std=c++17 -flto=thin -o testcase.js testcase.cpp
wasm-ld: warning: function signature mismatch: _ZNSt3__215basic_streambufIcNS_11char_traitsIcEEE7seekposENS_4fposI11__mbstate_tEEj
>>> defined as () -> void in /home/manx/opt/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/thinlto/libc++-noexcept.a(ios.instantiations.o)
>>> defined as (i32, i32, i32, i32) -> void in lto.tmp

wasm-ld: warning: function signature mismatch: _ZNSt3__215basic_streambufIcNS_11char_traitsIcEEE7seekoffExNS_8ios_base7seekdirEj
>>> defined as () -> void in /home/manx/opt/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/thinlto/libc++-noexcept.a(ios.instantiations.o)
>>> defined as (i32, i32, i64, i32, i32) -> void in lto.tmp

wasm-ld: warning: function signature mismatch: _ZNSt3__215basic_streambufIwNS_11char_traitsIwEEE7seekposENS_4fposI11__mbstate_tEEj
>>> defined as () -> void in /home/manx/opt/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/thinlto/libc++-noexcept.a(ios.instantiations.o)
>>> defined as (i32, i32, i32, i32) -> void in lto.tmp

wasm-ld: warning: function signature mismatch: _ZNSt3__215basic_streambufIwNS_11char_traitsIwEEE7seekoffExNS_8ios_base7seekdirEj
>>> defined as () -> void in /home/manx/opt/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten/thinlto/libc++-noexcept.a(ios.instantiations.o)
>>> defined as (i32, i32, i64, i32, i32) -> void in lto.tmp
manx@appendix:~/projects/openmpt/trunk-1/testcase$ em++ --version
emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.0.0 (3fd52e107187b8a169bb04a02b9f982c8a075205)
Copyright (C) 2014 the Emscripten authors (see AUTHORS.txt)
This is free and open source software under the MIT license.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

manx@appendix:~/projects/openmpt/trunk-1/testcase$

The problem does not occur without -flto=thin.

[manxorist]

git bisect resulted in:

24ee2bd7e55d2d3c99d04f7c5c800836138a8913 is the first bad commit
commit 24ee2bd7e55d2d3c99d04f7c5c800836138a8913
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date:   Wed Nov 17 17:16:34 2021 +0000

    Roll llvm-project from bdabf3c38a2e to e1ef14069efb (16 revisions)

    https://chromium.googlesource.com/external/github.com/llvm/llvm-project.git/+log/bdabf3c38a2e..e1ef14069efb

    2021-11-17 aeubanks@google.com [gn build] Add missed comma
    2021-11-17 aeubanks@google.com [NewPM] Add option to prevent rerunning function pipeline on functions in CGSCC adaptor
    2021-11-17 a.bataev@outlook.com [SLP][NFC]Add a test for multiple alternate nodes with cost estimation,
    2021-11-17 kazu@google.com [Format, Sema] Use range-based for loops with llvm::reverse (NFC)
    2021-11-17 martin@martin.st [OpenMP] Silence build warnings when built with MinGW
    2021-11-17 ldionne.2@gmail.com [libc++] Remove _LIBCPP_HAS_NO_SPACESHIP_OPERATOR
    2021-11-17 ldionne.2@gmail.com [libc++abi] Don't re-define _LIBCPP_HAS_NO_THREADS in single-threaded mode
    2021-11-17 Quinn.Pham@ibm.com [NFC][gn build] Inclusive language: replace master with main in sync_source_lists_from_cmake.py
    2021-11-17 gchatelet@google.com [libc] Use more consistent if defined syntax
    2021-11-17 gchatelet@google.com [libc] Fix missing restricts
    2021-11-17 gchatelet@google.com [libc] Fix documentation typo
    2021-11-17 michalt@google.com [mlir][Vector] First step for 0D vector type
    2021-11-17 balazs.benics@sigmatechnology.se [analyzer][NFC] Make the API of CallDescription safer slightly
    2021-11-17 zarko@ca.ibm.com [NFC][clang] Inclusive terms: replace uses of blacklist in clang/test/
    2021-11-17 kuhnel@google.com [NFC][clangd] cleanup llvm-else-after-return findings
    2021-11-17 hokein.wu@gmail.com [clangd] Fix assertion crashes on unmatched NOLINTBEGIN comments.

    If this roll has caused a breakage, revert this CL and stop the roller
    using the controls here:
    https://autoroll.skia.org/r/llvm-project-emscripten-releases
    Please CC wasm-waterfall@grotations.appspotmail.com on the revert to ensure that a human
    is aware of the problem.

    To report a problem with the AutoRoller itself, please file a bug:
    https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

    Documentation for the AutoRoller is here:
    https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

    Tbr: wasm-waterfall@grotations.appspotmail.com
    Change-Id: I6d3e5ee602ee2d8acdccfd45cb61c49de9bc39ff
    Reviewed-on: https://chromium-review.googlesource.com/c/emscripten-releases/+/3289720
    Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
    Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>

 DEPS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

[manxorist]

Using -flto instead of -flto=thin does not cause the warnings. Does emscripten recommend using lto instead of thinlto?

Also happens with tot build.

[sbc100]

I'm afraid that we don't yet have much testing of thinglto with either in wasm-ld or in emscripten. I'm trying to increase the coverage so this kind of regression does not occur. For the time being is full LTO an acceptable solution for you?

[manxorist]

For the time being is full LTO an acceptable solution for you?

Sure. We choose to enable thinLTO because as I understand it, it is generally recommended by clang. The Emscripten documentation also lists it at https://emscripten.org/docs/compiling/WebAssembly.html. Maybe a comment could be added to the documentation that thinLTO is not as well tested as full LTO with Emscripten?

I'll switch libopenmpt to full LTO for now.

[kripken]

I opened #15664 to add a note on thin LTO not being tested much yet, and that full LTO is recommended atm.

Btw, it is odd that bisection showed that range, though, as none of those commits look relevant, or at least I can't see a likely connection... that none of them mention LTO at all, or thin LTO, worries me.

Please update us when you know if full LTO works @manxorist

[manxorist]

I opened #15664 to add a note on thin LTO not being tested much yet, and that full LTO is recommended atm.

Thanks.

Btw, it is odd that bisection showed that range, though, as none of those commits look relevant, or at least I can't see a likely connection... that none of them mention LTO at all, or thin LTO, worries me.

I found that confusing as well, and thus I did double-check that commit and the previous one.

Please update us when you know if full LTO works @manxorist

For what it's worth, our unit tests (which run in node.js) also ran successfully with the thinLTO build. However we only make moderate use of iostreams, and I have not checked whether the offending functions would actually get called by our unit tests at all. It's basically only the warnings at link time that indicate any problem at all. They do sound kind of severe to me though.

[manxorist]

Please update us when you know if full LTO works @manxorist

libopenmpt builds without any warnings with full LTO.

[kripken]

Thanks @manxorist , good to know!

Yes, those warnings are definitely worrying. I'd stick to full LTO for now.

Side question, how much of a benefit do you see from LTO (vs non-LTO/normal), if you have those numbers handly?