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

ic-ref tests should show trapping instruction locations on trap #1964

Closed
osa1 opened this issue Sep 22, 2020 · 1 comment
Closed

ic-ref tests should show trapping instruction locations on trap #1964

osa1 opened this issue Sep 22, 2020 · 1 comment
Assignees
Labels
P2 medium priority, resolve within a couple of milestones testing Related to test suite

Comments

@osa1
Copy link
Contributor

osa1 commented Sep 22, 2020

Currently it's incredibly difficult to debug RTS or code generator issues because the error messages do not help, they simply say "canister trapped". Here's an actual example:

ingress Err: IC0502: Canister 75hes-oqbaa-aaaaa-aaaaa-aaaaa-aaaaa-aaaaa-q trapped: heap out of bounds

I previously spent days trying to debug these errors in #1750, and I'm doing the same now in #1931.

If ic-ref showed the trapping instruction's location, as an offset in the Wasm module, that'd help immensely. Some of the bugs that I wasted hours debugging would take a few minutes.

Since we'll be doing more work on the RTS (for garbage collection and probably other stuff too) and code generator I think it makes sense to invest some time into making ic-ref show trapping instruction locations.

I already started hacking on winter for this. This is an issue to keep track of the progress.

Note that the fix for this will be in winter and maybe ic-ref, not in Motoko. I'm creating the issue here as the end goal is to help with Motoko development, but let me know if I should move this to winter's or ic-ref's Github repo.

(We previously briefly discussed this on slack)

@osa1 osa1 self-assigned this Sep 22, 2020
@rossberg rossberg added P1 high priority, resolve before the next milestone P2 medium priority, resolve within a couple of milestones testing Related to test suite and removed P1 high priority, resolve before the next milestone labels Oct 6, 2020
@nomeata
Copy link
Collaborator

nomeata commented Nov 23, 2020

Fixed in 81962fc

@nomeata nomeata closed this as completed Nov 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 medium priority, resolve within a couple of milestones testing Related to test suite
Projects
None yet
Development

No branches or pull requests

3 participants