Skip to content
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

Remove unused JS globals for stdin/stdout/stderr #8410

Merged
merged 2 commits into from
Apr 9, 2019

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Apr 8, 2019

These globals are defined in musl C code so as far as I can tell
are never used.

@sbc100 sbc100 requested a review from kripken April 8, 2019 16:44
@sbc100
Copy link
Collaborator Author

sbc100 commented Apr 8, 2019

Am I missing something?

@sbc100
Copy link
Collaborator Author

sbc100 commented Apr 8, 2019

Looks like these variable go back to 16f60de, but I don't see them used today

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.

I think these might be leftovers from when we did more of that in JS, yes.

(However, that one other test failure makes me worry we are missing something?)

These globals are defined in musl C code so as far as I can tell
are never used.
@sbc100
Copy link
Collaborator Author

sbc100 commented Apr 9, 2019

Looks like we have a test there is hyper sensitve to size of static data:

FAIL: test_clear_error_on_massive_static_data (test_other.other)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.5/unittest/case.py", line 58, in testPartExecutor
    yield
  File "/usr/lib/python3.5/unittest/case.py", line 600, in run
    testMethod()
  File "/root/emscripten/tests/test_other.py", line 8232, in test_clear_error_on_massive_static_data
    self.assertContained('Memory is not large enough for static data (134032) plus the stack (1024), please increase TOTAL_MEMORY (65536)', err)
  File "/root/emscripten/tests/runner.py", line 712, in assertContained
    additional_info
  File "/usr/lib/python3.5/unittest/case.py", line 665, in fail
    raise self.failureException(msg)
AssertionError: Expected to find 'Memory is not large enough for static data (134032) plus the stack (1024), please increase TOTAL_MEMORY (65536)' in 'shared:ERROR: Memory is not large enough for static data (133984) plus the stack (1024), please increase TOTAL_MEMORY (65536) to at least 136032
', diff:

Will fix.

@sbc100
Copy link
Collaborator Author

sbc100 commented Apr 9, 2019

Also, makeStaticAlloc default to 16 byte alignment so this saves 48 bytes (in the debug build only I guess)

@sbc100 sbc100 merged commit 4ce1300 into incoming Apr 9, 2019
@sbc100 sbc100 deleted the remove_js_stdc_globals branch April 9, 2019 19:45
VirtualTim pushed a commit to VirtualTim/emscripten that referenced this pull request May 21, 2019
These globals are defined in musl C code so as far as I can tell
are never used.
VirtualTim added a commit to VirtualTim/emscripten that referenced this pull request May 23, 2019
VirtualTim added a commit to VirtualTim/emscripten that referenced this pull request May 23, 2019
belraquib pushed a commit to belraquib/emscripten that referenced this pull request Dec 23, 2020
These globals are defined in musl C code so as far as I can tell
are never used.
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