✨ Add interactive stack trace #233
Conversation
Stack trace now comes with an indicator of which line crashes the program. This is formatted as "cold.package : hot.package : monolith". This trace is also saved to a .txt file
This is where I previously had the code for ease of testing. Deleting it now that I've move the code to terminal.py.
|
One thing I noticed when looking through the code is that the paths in the code use |
Co-authored-by: BennyBot <48661356+BennyBot@users.noreply.github.com>
Co-authored-by: BennyBot <48661356+BennyBot@users.noreply.github.com>
…uesigbots/pros-cli into add-interactive-stack-trace
This reverts commit 059b077.
The previous algorithm does not work with how terminal.py prints the stack trace. This algorithm takes it into account, and has been verified to work with a VEX brain.
pros/serial/terminal/terminal.py
Outdated
| import sys | ||
| import threading | ||
|
|
||
| import colorama | ||
| from sqlalchemy import null |
There was a problem hiding this comment.
| from sqlalchemy import null |
pros/serial/terminal/terminal.py
Outdated
| class Terminal(object): | ||
| """This class is loosely based off of the pyserial miniterm""" | ||
| beginStackTrace = False | ||
| stackTraceFile = null |
There was a problem hiding this comment.
| stackTraceFile = null | |
| stackTraceFile = None |
pros/serial/terminal/terminal.py
Outdated
|
|
||
| if "END OF TRACE" in text: | ||
| file.close() | ||
| file = null |
There was a problem hiding this comment.
| file = null | |
| file = None |
pros/cli/terminal.py
Outdated
| @@ -29,6 +30,9 @@ def terminal_cli(): | |||
| help='Specify 2 ports for the "share" backend. The default option deterministically selects ports ' | |||
| 'based on the serial port name') | |||
| @click.option('--banner/--no-banner', 'request_banner', default=True) | |||
| @click.option('--auto_stack_trace', default=True) | |||
There was a problem hiding this comment.
| @click.option('--auto_stack_trace', default=True) | |
| @click.option('--raw_stack_trace', is_flag=True, default=False) |
pros/cli/terminal.py
Outdated
| @@ -80,11 +84,36 @@ def terminal(port: str, backend: str, **kwargs): | |||
| device = devices.RawStreamDevice(ser) | |||
| else: | |||
| device = devices.vex.V5UserDevice(ser) | |||
| term = Terminal(device, request_banner=kwargs.pop('request_banner', True)) | |||
| term = Terminal(device, request_banner=kwargs.pop('request_banner', True), auto_stack_trace=kwargs.pop('auto_stack_trace', True)) | |||
There was a problem hiding this comment.
| term = Terminal(device, request_banner=kwargs.pop('request_banner', True), auto_stack_trace=kwargs.pop('auto_stack_trace', True)) | |
| term = Terminal(device, request_banner=kwargs.pop('request_banner', True), auto_stack_trace=(not kwargs.pop('auto_stack_trace', False))) |
| class Terminal(object): | ||
| """This class is loosely based off of the pyserial miniterm""" | ||
| beginStackTrace = False | ||
| stackTraceFile = None | ||
|
|
There was a problem hiding this comment.
| file = None |
Did we ever decide if we needed to do this @ryan-doan ?
|
Ignore all of the suggestions that say "Outdated". Not sure what Github just did. |
ayushuk
added
feature
Fixes a fileNotFound error
|
Progress: |
Wonder how this would look on a small window (like the VSCode integrated terminal). One other interesting thought, get rid of the "Registers at Exception?" section? We might want to consider shaving some of the fat that your average user may not need. Most important part is the stack trace section. (I'm fully aware addr2line provides all this data, not feasible how this is since it's been so long 💀 ) |
|
I think it looks really good. One thing that needs to be done is identifying the deepest level of user code where the data abort comes from (if the user is using hot/cold package, if we have to use monolith to fill in the information we can just ignore this part). However, it would be cool if we could identify this deepest level of user code, somehow get that to VSCode, and then VSCodecan direct the user to that line of code automatically. It may even be possible to guide them through the whole trace in VSCode, perhaps a new "Data Abort Debugger" button on the sidebar. |
| def getTrace(s, path): | ||
| if not os.path.exists(path): | ||
| return '' | ||
| temp = subprocess.check_output([addr2line_path, '-faps', '-e', path, s]).decode('utf-8') |
There was a problem hiding this comment.
| temp = subprocess.check_output([addr2line_path, '-faps', '-e', path, s]).decode('utf-8') | |
| temp = subprocess.check_output([addr2line_path, '-faps', '-e', '-C', path, s]).decode('utf-8') |
Right now C++ function names look like letter soup, since they're not demangled.
Summary:
Stack trace now comes with detailed description of where the program fails. This is also saved in a text file in pros-cli/pros/serial/stack_trace.txt
Motivation:
This change help ease the debug of seg faults.
References (optional):
Issue #76
Test Plan:
The code should be tested with stack traces of varied address count, and compared with output from addr2line with hot package, cold package, and monolith to make sure it doesn't omit important information (the algorithm displays addr2line output that contains the character '?' as "??", since these output are most of the time gibberish that doesn't aid debugging, but I'm not sure if this is the case ONLY for gibberish output, which can lead to the algorithm omitting useful debug information).