feat(#1812): add Stacktrace Mechanism to Machine for Improved Exception Handling

[omarsy]

Closes #1812

Summary

This pull request introduces a new Stack mechanism to the Machine class, enhancing its exception handling capabilities by generating and appending detailed stacktraces during panic situations. This approach provides a more efficient and useful way to debug issues.

Solution

The implementation of a Stack mechanism allows for the tracking of execution frames and statements and more. During a panic, this stack is copied and included in the exception details, providing a structured and comprehensive view of the call sequence and state leading up to the panic.

Sequence Diagram

sequenceDiagram
    participant Runner
    participant Machine
    participant Stack

    Runner->>Machine: Run
    alt When we pop a Frame
        Machine->>Stack: OnFramePopped(frame)
        alt Frame.Func!=nil
            Stack->>Stack: Decrease Execution Stack
            Stack->>Stack: CurrentExecution = Execution[LenExecution-1]
        end
    else When we push a Frame
        Machine->>Stack: OnFramePushed(frame)
        alt Frame.Func!=nil
            Stack->>Stack: Increase Execution Stack
            Stack->>Stack: CurrentExecution = Execution[LenExecution-1]
        end
    else When we pop a Stmt
        Machine->>Stack: OnStmtPopped(stmt)
        alt Stmt
            Stack->>Stack: CurrentExecution.Stmt = stmt
        end
    end
    break panic
        Machine->>Stack: Copy
        Stack-->>Machine: StackCopy
        Machine->>Machine: append Exception {Value, LastCallFrame, StackCopy}
    end

Implementation Details

1. New Stack Mechanism:

   • Frame Operations:
     • OnFramePopped: Decreases the execution stack and updates the current execution state.
     • OnFramePushed: Increases the execution stack and updates the current execution state.
   • Statement Operations:
     • OnStmtPopped: Updates the current statement in the execution stack.

2. Panic Handling:

   • When a panic occurs, the current stack state is copied and appended to the exception details.
   • This includes the value of the panic, the last call frame, and the copied stack trace.

3. Code Example:

   package main
   
   func main() {
       f()
   }
   
   func f() {
       defer func() {
           panic("third")
       }()
       defer func() {
           panic("second")
       }()
       panic("first")
   }
   

4. Sample Output:

   Stacktrace:
       Exception 0: 
           panic((const ("first" <untyped> string)))
               main/files/panic0b.gno:14
           f<VPBlock(3,1)>()
               main/files/panic0b.gno:4
       Exception 1: 
           panic((const ("second" <untyped> string)))
               main/files/panic0b.gno:12
           f<VPBlock(3,1)>()
               main/files/panic0b.gno:4
       Exception 2: 
           panic((const ("third" <untyped> string)))
               main/files/panic0b.gno:9
           f<VPBlock(3,1)>()
               main/files/panic0b.gno:4
   

Contributors' checklist...

• [ ] Added new tests, or not needed, or not feasible
• [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
• [ ] Updated the official documentation or not needed
• [ ] No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
• [ ] Added references to related issues and PRs
• [ ] Provided any useful hints for running manual tests
• [ ] Added new benchmarks to generated graphs, if any. More info here.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 82.92683% with 35 lines in your changes missing coverage. Please review.

Project coverage is 60.09%. Comparing base (5f28803) to head (778965e). Report is 1 commits behind head on master.

Files	Patch %	Lines
gnovm/tests/file.go	55.26%	15 Missing and 2 partials :warning:
gnovm/pkg/gnolang/machine.go	82.25%	11 Missing :warning:
gnovm/pkg/gnolang/frame.go	95.50%	3 Missing and 1 partial :warning:
gnovm/cmd/gno/run.go	57.14%	3 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2145      +/-   ##
==========================================
- Coverage   60.14%   60.09%   -0.05%     
==========================================
  Files         560      560              
  Lines       74738    75035     +297     
==========================================
+ Hits        44950    45093     +143     
- Misses      26400    26556     +156     
+ Partials     3388     3386       -2     

Flag	Coverage Δ
contribs/gnodev	60.58% <ø> (ø)
contribs/gnofaucet	15.31% <ø> (+0.85%)	:arrow_up:
gno.land	64.18% <100.00%> (+0.02%)	:arrow_up:
gnovm	64.13% <82.41%> (-0.15%)	:arrow_down:
misc/genstd	80.54% <ø> (ø)
misc/logos	19.88% <ø> (ø)
tm2	62.04% <ø> (+0.03%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[leohhhn]

@omarsy

Thank you so much for this PR - this is super useful and is something that has been sitting on the backlog for way too long. I will make sure the team hears about this PR and takes a look as soon as possible.

[omarsy]

1. The stacktrace should be in the same format as Go. Or, better -- the format can change, but I'd like the resulting stacktraces to be parseable by panicparse. Some things can change due to different information, but the overall "shape" and format should be similar.

I will try to do a new proposal for this.

2. Please don't add a new "Stack" to the machine (unless you have a very compelling reason to do so), we have enough of them. See injectLocOnPanic for how we currently generate line numbers when creating panic messges.

I thought about introducing a new stack since injectLocOnPanic loses context when frames, statements, or expressions are removed from the machine once executed. I'll message you on Signal for more guidance.

3. Please make sure to decouple the logic from anything specific to panics. To this effect, please add stdlib debug (in tests/stdlibs/), and add function PrintStack. See Add "debug" stdlib #1266. This way, in tests, we can add recover() followed by debug.PrintStack(). (Note -- this should not go directly in the "normal" stdlibs, as it's better if this feature is experimental for now).

Yes I will do it ;)

4. Please don't test this only using txtars. I suggest you add them as part of the error messages for the tests we have in tests/files; so we can see and check how the stacktraces become on a large variety of existing tests. Please also make sure to add any weird cases you can think of (panic within a closure, panic in a very recursive function, etc.). I suggest you remove panic2 and only keep 1 txtar test checking the stacktraces; the rest should be at the VM level.

Right I will do it ;)

[deelawn]

After discussion with Jae, the place where the trace is built is fine -- it will make the code easier to read. Here are two additional requests:

1. Define a stack trace struct type which contains the frame and statement slices needed to build the trace; these can be copied from the machine state. The item at index zero should be from the most recent frame / statement. The newly define struct can have a String() method for building the trace string to be printed.
2. When building the stack trace struct, limit the number of frames that are processed; this can be something like 64 or 128. The reason for this is to prevent massive traces from being generated in cases such as infinite recursion.

[omarsy]

@deelawn I do this on my previous commit https://github.com/gnolang/gno/pull/2145/commits/7cfe142855ea7693bd4f8bab04862c45def83f80#diff-f2491da2772a90a3a7f424bc5e193d5b0eb538a3ebc719089d13b5c4e49d2f4d . WDYT ?

[omarsy]

Define a stack trace struct type which contains the frame and statement slices needed to build the trace; these can be copied from the machine state. The item at index zero should be from the most recent frame / statement. The newly define struct can have a String() method for building the trace string to be printed.

I did it here https://github.com/gnolang/gno/commit/c6a7a0315f0dc3a138a892028cf7ba17b30eb722. If it is good for you then I can add the Limit guys

[deelawn]

Thanks for the quick responses @omarsy; I'm working my way through the rest of the open items. One more question for you -- is it necessary for the tests to be txtar tests? It seems like this is a gno language thing and is not dependent on the chain. If that's the case, then it may be better to make these gno file tests isntead.

[omarsy]

@deelawn Thank you for your reviews too ! I had a few issues running the tests in unit test. I'm going to add unit tests. And keep a single test where it will be done on txtar. Is it ok for you guys ?

[omarsy]

@deelawn I have added some files test. You can find them here: https://github.com/gnolang/gno/pull/2145/commits/004be131107dbcdc791a0f7c2295306a149ed620

[deelawn]

Should this line also call m.ExceptionsStacktrace() rather than m.String()? It seems like it should, for consistency. https://github.com/gnolang/gno/blob/98098784d14fa912b1343184da0595641d7422df/gnovm/cmd/gno/run.go#L192