Skip to content

FIX Turn off settings.LTO if -fno-lto is passed #20309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 22, 2023

Conversation

hoodmane
Copy link
Collaborator

@hoodmane hoodmane commented Sep 21, 2023

Resolves #20308.

  • Add a test

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work coming up with test case. Does this test indeed fail without this patch?

I made a PR too but was going to pass on the trying to write a test. (#20310) :) I'll close mine.

@hoodmane
Copy link
Collaborator Author

Yes I checked that the test fails without the patch.

@@ -11148,6 +11148,23 @@ def test_setjmp_emulated_casts(self):
''')
self.do_runf('src.c', 'ok\ndone\n', emcc_args=['-sEMULATE_FUNCTION_POINTER_CASTS'])

def test_setjmp_no_lto(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps just call this test_no_lto.. and add a comment with a link to the bug, since this seems like a very specific regression test.

@sbc100
Copy link
Collaborator

sbc100 commented Sep 21, 2023

Thanks for looking into this and fixed it!

@sbc100 sbc100 enabled auto-merge (squash) September 21, 2023 22:45
hoodmane added a commit to hoodmane/pyodide that referenced this pull request Sep 21, 2023
@hoodmane
Copy link
Collaborator Author

test-browser-chrome seems to have some flakes...

@sbc100 sbc100 merged commit f9ce05b into emscripten-core:main Sep 22, 2023
@hoodmane hoodmane deleted the no-lto branch September 22, 2023 00:59
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.

setjmp/longjmp + -fexceptions does not work with -flto -fno-lto
2 participants