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

give a clear error when TOTAL_MEMORY is not big enough #8369

Merged
merged 4 commits into from
Apr 2, 2019
Merged

Conversation

kripken
Copy link
Member

@kripken kripken commented Mar 29, 2019

(for all the static allocation + stack etc.)

Not catching this early leads to late validation errors in asm2wasm.

fixes WebAssembly/binaryen#1968

…atic allocation + stack etc. not catching this early leads to late validation errors in asm2wasm. fixes WebAssembly/binaryen#1968
emscripten.py Outdated
@@ -754,6 +754,9 @@ def __init__(self):
# * then dynamic memory begins
self.dynamic_base = align_memory(self.stack_high)

if self.dynamic_base >= shared.Settings.TOTAL_MEMORY:
exit_with_error('Memory is not large enough for static initialization (%d) plus the stack (%d), please increase TOTAL_MEMORY (%d) to at least %d' % (self.static_bump, shared.Settings.TOTAL_STACK, shared.Settings.TOTAL_MEMORY, self.dynamic_base))
Copy link
Collaborator

Choose a reason for hiding this comment

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

static initialization -> static data?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixing.

double muchData[] = {
''')
for i in range(50 * 1000):
f.write('1.2, 2.3, 3.4, 4.5, 5.6, 6.7, 7.8, 8.9, 9.9,\n')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why bother with this huge test? Why not just create little static data and set TOTAL_MEMORY to a small value?

Copy link
Collaborator

Choose a reason for hiding this comment

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

"Tests that we should a" -> "Tests that we give a"

ret += muchData[i];
}
return ret & 255;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also why doesn't this program just return &muchData. There should be no need to actually do anything there since the program won't link..

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, could be much simpler, improved it now.

@kripken
Copy link
Member Author

kripken commented Apr 1, 2019

Some minor test fixes pushed. Should be green now except for wasm0.test_the_bullet - @sbc100, is that a known current breakage on the wasm backend? (PIC stuff maybe?)

@sbc100
Copy link
Collaborator

sbc100 commented Apr 1, 2019

wasm0.test_the_bullet looks like a llvm crash. We are not seeing it on the wasm waterfall because we only test wasm2 there. I'm looking into it.

@kripken
Copy link
Member Author

kripken commented Apr 1, 2019

@sbc100 cool, thanks. meanwhile i bisected it to the PIC changes ("Initial implementation of PIC code generation").

@kripken kripken merged commit ff5c8cb into incoming Apr 2, 2019
@kripken kripken deleted the err branch April 2, 2019 16:15
VirtualTim pushed a commit to VirtualTim/emscripten that referenced this pull request May 21, 2019
…re#8369)

(for all the static allocation + stack etc.)

Not catching this early leads to late validation errors in asm2wasm.

fixes WebAssembly/binaryen#1968
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
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.

asm2wasm throws a fatal error during output validation
2 participants