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

Implement debug compilation mode for runtime sanity checking and assertions #1895

Closed
osa1 opened this issue Sep 4, 2020 · 2 comments · Fixed by #1916
Closed

Implement debug compilation mode for runtime sanity checking and assertions #1895

osa1 opened this issue Sep 4, 2020 · 2 comments · Fixed by #1916
Assignees

Comments

@osa1
Copy link
Contributor

osa1 commented Sep 4, 2020

There are lots of sanity checks and assertions that will help testing and debugging the runtime system but are also too expensive (in terms of runtime and binary size) to enable always. One example is resetting the scratch space in the current copying garbage collector: it cost us significant gas when it was enabled in #1750 so we had to remove it.

For these checks I think we'll need a compilation mode (let's call it "debug", maybe enable with -debug passed to moc) that will generate the final .wasm file with the assertions and sanity checks. These binaries will be used when testing and debugging. In normal operation we'll build without -debug.

Initially debug compilation mode could simply link moc-generated code with the Rust parts of the RTS built in debug mode (instead of release mode). In Rust, debug mode does overflow and underflow checking in arithmetic (unless methods like wrapping_add used, which allow overflows and underflows) which is already quite useful (having debugged underflows in a GC in the past ...). We could then add more checks manually using if cfg!(debug_assertions) { ... } or debug_assert_eq! etc.

@nomeata @ggreif what do you think?

@nomeata
Copy link
Collaborator

nomeata commented Sep 4, 2020

Yes, sounds useful, absolutely in favor.

We probably want to run the test suite in both modes then? Would be a shame to not see a bug that’s hidden by resetting the scratch space. The output should be identical, so we could use the same ok/… files, right?

Implementation-wise, I’d say we build both mo-rts.wasm and mo-rts.wasm using rts/Makefile; embed both into the moc binary, and then use the right one based on -debug or not.

The IR typechecker is a similar thing that we would probably not want to to run by default.

@osa1
Copy link
Contributor Author

osa1 commented Sep 4, 2020

We probably want to run the test suite in both modes then? Would be a shame to not see a bug that’s hidden by resetting the scratch space. The output should be identical, so we could use the same ok/… files, right?

Yes to all.

Implementation-wise, I’d say we build both mo-rts.wasm and mo-rts.wasm using rts/Makefile; embed both into the moc binary, and then use the right one based on -debug or not.

👍

The IR typechecker is a similar thing that we would probably not want to to run by default.

Yeah once we have the flag we can also do more compile-time checking as well. Or maybe generate different code that does sanity checking. The latter would be difficult if we had separate compilation (as debug code would have to be compatible with non-debug code generated for libraries), but with whole-program compilation it's easier.

@ghost ghost assigned osa1 Sep 8, 2020
@mergify mergify bot closed this as completed in #1916 Sep 10, 2020
mergify bot pushed a commit that referenced this issue Sep 10, 2020
…bg (#1916)

This adds a RTS variant for debugging purposes. For C code it passes `-DDEBUG`
to the compiler. For Rust it builds the code in debug mode. Currently we don't
have any new assertion checking code, though debug builds in Rust does more
error checking my default (e.g. underflow and overflow checking).

This allows implementing expensive checks in the generated code or RTS when
testing without bloating release binaries.

Fixes #1895
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 a pull request may close this issue.

2 participants