Skip to content

Conversation

juj
Copy link
Collaborator

@juj juj commented Aug 28, 2025

The GooTimer.cc file had a #warning, and STRICT test mode sets -Werror.

GooTimer.cc:91:2: error: "no support for GooTimer" [-Werror,-W#warnings]
   91 | #warning "no support for GooTimer"
      |  ^
1 error generated.

@juj juj enabled auto-merge (squash) August 28, 2025 21:30
@sbc100
Copy link
Collaborator

sbc100 commented Aug 28, 2025

Hmm.. the strict mode doesn't do -Werror as far as I can tell.

We do inject -Werror in all tests by default though:

self.cflags = ['-Wclosure', '-Werror', '-Wno-limited-postlink-optimizations']

@sbc100
Copy link
Collaborator

sbc100 commented Aug 28, 2025

Looks like that is happening here is that -Werror is always being passed by the #warning is only being triggered in -sSTRICT mode because HAVE_GETTIMEOFDAY is not being detected in strict mode.

I'm not sure why ... the gettimeofday function itself is always available regardless of -sSTRICT.

@sbc100
Copy link
Collaborator

sbc100 commented Aug 28, 2025

I figured what is going on.

-sSTRICT causes --fatal-warnings to be passed to the linker, which then makes many of the autoconf tests fail due to errors like this:

 wasm-ld: error: function signature mismatch: gettimeofday                        
 >>> defined as () -> i32 in /tmp/emscripten_temp_k6d489qt/conftest_0.o           
 >>> defined as (i32, i32) -> i32 in /usr/local/google/home/sbc/dev/wasm/emscripten/cache/sysroot/lib/wasm32-emscripten/libc.a(emscripten_time.o)

This is because of the way autoconf tries to declare functions without any signature at all. It relies on the fact that on most plaforms this is find, but not under wasm.

I guess we should probably instead decorate this test with @no_strict('autoconf is incompatible with strict mode')

@juj
Copy link
Collaborator Author

juj commented Aug 28, 2025

I guess we should probably instead decorate this test with @no_strict('autoconf is incompatible with strict mode')

emconfigure ./configure and -sSTRICT are both user facing features, so it wouldn't help to fix this only in test suite then?

Though the failure #warning is only specific to this test code. I think it's still cleaner to add that -Wno- warning rather than add more @no_x directives that would be easy to resolve?

@sbc100
Copy link
Collaborator

sbc100 commented Aug 28, 2025

I guess we should probably instead decorate this test with @no_strict('autoconf is incompatible with strict mode')

emconfigure ./configure and -sSTRICT are both user facing features, so it wouldn't help to fix this only in test suite then?

Though the failure #warning is only specific to this test code. I think it's still cleaner to add that -Wno- warning rather than add more @no_x directives that would be easy to resolve?

But the larger problem is that emconfigure with -sSTRICT will detect completely incorrect results.. having this test ignore the incorrect results doesn't really solve it, but sure, if you want to add -Wno-warning please add a comment explaining the roundabout reason why its needed.

@sbc100
Copy link
Collaborator

sbc100 commented Aug 28, 2025

Since emcc itself has some awareness of when a configure test is being built (see autoconf = .. in phase_linker_setup) we could also inject -Wl,--no-fatal-warnings magically there?

@juj
Copy link
Collaborator Author

juj commented Aug 29, 2025

Ok, now I see what you mean about the configuring.

To get autoconf work with STRCICT, I tried with

diff --git a/tools/link.py b/tools/link.py
index 021513da8..1106c7a36 100644
--- a/tools/link.py
+++ b/tools/link.py
@@ -724,6 +724,9 @@ def phase_linker_setup(options, linker_args):  # noqa: C901, PLR0912, PLR0915
     settings.NODERAWFS = 1
     # Add `#!` line to output JS and make it executable.
     options.executable = True
+    # Do not run in strict mode when configuring, since autoconf declares functions without their proper signatures, and STRICT
+    # causes that to trip up by passing --fatal-warnings to the linker.
+    settings.STRICT = 0
 
   if settings.OPT_LEVEL >= 1:
     default_setting('ASSERTIONS', 0)

but that resulted in strict.test_poppler failing with a surprising Emscripten internal error:

Traceback (most recent call last):
  File "/mnt/ssd1000/emsdk/emscripten/main/emcc.py", line 629, in <module>
    sys.exit(main(sys.argv))
             ^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/contextlib.py", line 81, in inner
    return func(*args, **kwds)
           ^^^^^^^^^^^^^^^^^^^
  File "/mnt/ssd1000/emsdk/emscripten/main/emcc.py", line 622, in main
    ret = run(args)
          ^^^^^^^^^
  File "/mnt/ssd1000/emsdk/emscripten/main/emcc.py", line 354, in run
    return link.run(options, linker_args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/mnt/ssd1000/emsdk/emscripten/main/tools/link.py", line 3099, in run
    js_info = get_js_sym_info()
              ^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/contextlib.py", line 81, in inner
    return func(*args, **kwds)
           ^^^^^^^^^^^^^^^^^^^
  File "/mnt/ssd1000/emsdk/emscripten/main/tools/link.py", line 258, in get_js_sym_info
    input_files = [json.dumps(settings.external_dict(skip_keys=skip_settings), sort_keys=True, indent=2)]
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/mnt/ssd1000/emsdk/emscripten/main/tools/settings.py", line 227, in external_dict
    external_settings[key] = self.attrs[key]
                             ~~~~~~~~~~^^^^^
KeyError: 'BINARYEN'

Adding a further

diff --git a/tools/settings.py b/tools/settings.py
index 8e640c6a5..fde44486c 100644
--- a/tools/settings.py
+++ b/tools/settings.py
@@ -223,7 +223,8 @@ class SettingsManager:
       # When not running in strict mode we also externalize all legacy settings
       # (Since the external tools do process LEGACY_SETTINGS themselves)
       for key in self.legacy_settings:
-        external_settings[key] = self.attrs[key]
+        if key in self.attrs:
+          external_settings[key] = self.attrs[key]
     return external_settings
 
   def keys(self):

does then resolve that and then the test strict.test_poppler works again.

Though it looks like the root cause for that KeyError is something doesn't stay consistent. It seems as if part of the toolchain run operated in STRICT mode up until phase_linker_setup() point, and then force-setting settings.STRICT = 0 at that point is no longer compatible. So I'll not go there.

Updated the PR to disable strict.test_poppler and autoconf+STRICT.

@juj juj changed the title Fix strict.test_poppler Disable strict.test_poppler Aug 29, 2025
@juj juj merged commit 60cee73 into emscripten-core:main Aug 29, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants