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

Stack trace order in nargo debug is broken #7195

Closed
smanilov opened this issue Jan 27, 2025 · 4 comments
Closed

Stack trace order in nargo debug is broken #7195

smanilov opened this issue Jan 27, 2025 · 4 comments
Labels
bug Something isn't working

Comments

@smanilov
Copy link

smanilov commented Jan 27, 2025

Aim

Use nargo debug to inspect the stack trace of a simple program.

Expected Behavior

Before #7163, nargo debug used to work in the following way.

Consider the program:

fn bar(y: Field) {
    assert(y != 0);
}

fn foo(x: Field) {
    bar(x)
}

fn main(x: Field, y: pub Field) {
    foo(x);
    foo(y);
    assert(x != y);
}

When running nargo debug on it, and stepping until the assert(y != 0); line, the printed trace is the following:

> next
At opcode 0:0.25 :: Const { destination: Relative(8), bit_size: Integer(U32), value: 0 }
At /home/aristotelis/code/repos/clone_fast/tmp_sync/test_programs/plonky2_trace/2_function_calls/src/main.nr:10:5
  5    ...
  6        bar(x)
  7    }
  8    
  9    fn main(x: Field, y: pub Field) {
 10 ->     foo(x);
 11        foo(y);
 12        assert(x != y);
 13    }
At /home/aristotelis/code/repos/clone_fast/tmp_sync/test_programs/plonky2_trace/2_function_calls/src/main.nr:6:5
  1    fn bar(y: Field) {
  2        assert(y != 0);
  3    }
  4    
  5    fn foo(x: Field) {
  6 ->     bar(x)
  7    }
  8    
  9    fn main(x: Field, y: pub Field) {
 10        foo(x);
 11    ...
At /home/aristotelis/code/repos/clone_fast/tmp_sync/test_programs/plonky2_trace/2_function_calls/src/main.nr:1:18
  1 -> fn bar(y: Field) {
  2        assert(y != 0);
  3    }
  4    
  5    fn foo(x: Field) {
  6        bar(x)
  7    }
  8    ...
> q

Note that the order of the stack frames here is correct. First is main, then foo, then bar. This is chronological. This happens after about 6 steps into the program (six times calling next in the debugger CLI).

Bug

#7163 seems to break nargo debug in the following way.

With the same program, nargo debug prints the following stack trace after a few next commands:

> next
At opcode 0:0.25 :: Const { destination: Relative(8), bit_size: Integer(U32), value: 0 }
At /home/aristotelis/code/repos/clone_fast/tmp_sync/test_programs/plonky2_trace/2_function_calls/src/main.nr:10:5
  5    ...
  6        bar(x)
  7    }
  8    
  9    fn main(x: Field, y: pub Field) {
 10 ->     foo(x);
 11        foo(y);
 12        assert(x != y);
 13    }
At /home/aristotelis/code/repos/clone_fast/tmp_sync/test_programs/plonky2_trace/2_function_calls/src/main.nr:1:18
  1 -> fn bar(y: Field) {
  2        assert(y != 0);
  3    }
  4    
  5    fn foo(x: Field) {
  6        bar(x)
  7    }
  8    ...
At /home/aristotelis/code/repos/clone_fast/tmp_sync/test_programs/plonky2_trace/2_function_calls/src/main.nr:6:5
  1    fn bar(y: Field) {
  2        assert(y != 0);
  3    }
  4    
  5    fn foo(x: Field) {
  6 ->     bar(x)
  7    }
  8    
  9    fn main(x: Field, y: pub Field) {
 10        foo(x);
 11    ...
> 
CTRL-C

Note that the order in which the stack frames are printed is not the order in which they are created: main is first, bar is next, foo is last. It should be main -> foo -> bar, because that's the chronological order in which they appear.

To Reproduce

  1. Create a project using nargo new;
  2. Replace the source in src/main.nr with the sample program;
  3. For Prover.toml, use x = 4 and y = 5;
  4. Run nargo debug and step until the assert line.

Workaround

Yes

Workaround Description

Reverting the changes in preprocess_fns.rs from #7163 fixes the issue.

But, we suspect the issue is in nargo debug itself.

Additional Context

No response

Project Impact

Blocker

Blocker Context

This is blocking other work we're doing in our fork of Noir in https://github.com/blocksense-network/noir , so we have a temporary patch, but we'd love it if this is fixed upstream.

Nargo Version

nargo version = 1.0.0-beta.1 noirc version = 1.0.0-beta.1+c44b62615f1c8ee657eedd82f2b80e2ec76c9078 (git version hash: c44b626, is dirty: false)

NoirJS Version

No response

Proving Backend Tooling & Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@smanilov smanilov added the bug Something isn't working label Jan 27, 2025
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Jan 27, 2025
@TomAFrench
Copy link
Member

I think this is a duplicate issue of #7093 with them both sharing the same root cause.

@smanilov
Copy link
Author

Thanks for the reply! Might be the same root cause, although we experienced the issue only yesterday. nargo debug was working fine on Friday. #7093 is from two weeks ago.

michaeljklein added a commit that referenced this issue Jan 29, 2025
… test of expected trace when stepping through test that was failing in #7195
@asterite
Copy link
Collaborator

asterite commented Feb 5, 2025

I'm trying it right now in master and seeing correct behavior. I can also confirm that it was broken in c44b626

Using git bisect it seems this was fixed by #7212

@asterite asterite closed this as completed Feb 5, 2025
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Feb 5, 2025
@michaeljklein
Copy link
Contributor

@asterite yes, it appears so: I'm regression testing this in #7233

github-merge-queue bot pushed a commit that referenced this issue Feb 21, 2025
TomAFrench added a commit that referenced this issue Feb 25, 2025
* master: (74 commits)
  feat: optimize out range checks on limiting cases (#7510)
  chore: clippy fixes (#7505)
  chore(docs): Supplement docs on `modexp` as a required precompile for Barretenberg's Solidity verifier (#7508)
  feat(debugger): REPL add breakpoint by sourcecode line (#5204)
  fix: issue duplicate error on impl function without self (#7490)
  feat(experimental): Support struct constructors in match patterns (#7489)
  feat: use resolved type instead of needing Constructor.struct_type (#7500)
  feat: better error message when keyword is found instead of type in p… (#7501)
  chore: bump external pinned commits (#7497)
  feat(experimental): Add invalid pattern syntax error (#7487)
  fix(performance): Accurately mark safe constant indices for arrays of complex types (#7491)
  fix(experimental): Allow shadowing in match patterns (#7484)
  chore: regression test #7195 (#7233)
  chore(docs): Section on `noir-profiler execution-opcodes` (#7480)
  chore: improve proptesting of 128bit values in `noirc_abi` (#7485)
  chore(profiler): Use brillig names for outputted flamegraphs  (#7470)
  chore(docs): Profiler images reference (#7481)
  fix: don't use dummy location when inserting debug code (#7482)
  feat(cli)!: Add `--unstable-features` to gate unstable features (#7449)
  feat: Sync from aztec-packages (#7474)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

4 participants