Print source location for panics when using -monitor
#3673
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aykevl
force-pushed
the
print-panic-address
branch
from
8c4a57d
to
68fcb8c
Compare
|
@aykevl looks like something failing the various CI tests, maybe a file that was not added? |
aykevl
force-pushed
the
print-panic-address
branch
2 times, most recently
from
7cfceb5
to
02eed71
Compare
|
Fixed, but I can't really test the change because of #3674. At least not on my Circuit Playground Bluefruit. |
|
Looks like CI still failing. |
aykevl
force-pushed
the
print-panic-address
branch
from
02eed71
to
b7ec4a8
Compare
|
Fixed, MachO is problematic again and I'm not really interested in fixing that. It's ELF that's important (used on Linux and baremetal). |
aykevl
force-pushed
the
print-panic-address
branch
from
b7ec4a8
to
a97f292
Compare
|
Breaks on Windows too, so I'll limit testing on Linux only. Which is all we care about, really (we care about the ELF format which isn't used on MacOS or Windows). |
|
@aykevl can you please resolve merge conflict? |
This is not very useful in itself, but makes it possible to detect this address in the output. See the next commit. This adds around 50 bytes to each binary (except for AVR and wasm). This is unfortunate, but I think this feature is quite useful still. A future enhancement might be to create a build tag for extended panic information that's not set by default.
The previous commit started printing the instruction address for runtime
panics. This commit starts using this address to print the source
location.
Here is an example where this feature is very useful. There is a heap
allocation in the Bluetooth package, but we don't know where exactly.
Printing the instruction address of the panic is already useful, but
what is even more useful is looking up this address in the DWARF debug
information that's part of the binary:
$ tinygo flash -target=circuitplay-bluefruit -monitor ./examples/heartrate
Connected to /dev/ttyACM0. Press Ctrl-C to exit.
tick 00:00.810
tick 00:01.587
tick 00:02.387
tick 00:03.244
panic: runtime error at 0x00027c4d: alloc in interrupt
[tinygo: panic at /home/ayke/src/tinygo/bluetooth/adapter_sd.go:74:4]
To be clear, this path isn't stored on the microcontroller. It's stored
as part of the build, and `-monitor` just looks up the path from the
panic message.
Possible enhancements:
- Print such an address for regular panics as well. I'm not sure
that's so useful, as it's usually a lot easier to look up panics
just by their message.
- Use runtimePanicAt (instead of runtimePanic) in other locations, if
that proves to be beneficial.
- Print the TinyGo-generated output in some other color, to
distinguish it from the regular console output.
- Print more details when panicking (registers, stack values), and
print an actual backtrace.
aykevl
force-pushed
the
print-panic-address
branch
from
a97f292
to
9fbc1e0
Compare
|
Rebased. |
It never seems to fail? |
|
IIRC it only fails when you actually connect to it. You can try connecting using examples/heartrate-monitor for example. |
Thanks. It works (fails?) as expected. |
deadprogram
approved these changes
Apr 26, 2023
|
Now merging, thanks @aykevl |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the ability to print source locations for runtime panics. Here is an example:
To be clear, this path isn't stored on the microcontroller. It's stored as part of the build, and
-monitorjust looks up the path from the panic message.Possible enhancements: