Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Nov 27, 2024

These two options are not compatible, and GLOBAL_BASE is currently just ignored in this mode.

@sbc100 sbc100 enabled auto-merge (squash) November 27, 2024 22:19
def test_dylink_global_base(self):
err = self.expect_fail([EMCC, test_file('hello_world.c'), '-sGLOBAL_BASE=1024', '-sMAIN_MODULE'])
self.assertContained('emcc: error: GLOBAL_BASE is not compatible with relocatable output', err)

Copy link
Member

Choose a reason for hiding this comment

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

Testing a side module instead would be faster, and also be future compatible if we ever make main modules non-relocatable. But just a suggestion, lgtm either way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The error happens before we do any actual linking work so its just as fast either way.

@sbc100 sbc100 changed the title Error if GLOBAL_BASE is used with relocatable output Add test for GLOBAL_BASE and SIDE_MODULE error message. NFC Nov 28, 2024
@sbc100
Copy link
Collaborator Author

sbc100 commented Nov 28, 2024

Oops it turns out the GLOBAL_BASE does work with MAIN_MODULE since it gets embedded in the JS file.

Updated this RP to just a test for the existing error for SIDE_MODULE.

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Oh, interesting.

@sbc100 sbc100 disabled auto-merge December 2, 2024 20:30
@sbc100 sbc100 merged commit c85a097 into emscripten-core:main Dec 2, 2024
26 of 28 checks passed
@sbc100 sbc100 deleted the global_base branch December 2, 2024 20:30
hedwigz pushed a commit to hedwigz/emscripten that referenced this pull request Dec 18, 2024
…ripten-core#23029)

These two options are not compatible, and `GLOBAL_BASE` is currently
just ignored in this mode.
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