Allow to load the full stacktrace

[ignatov]

For better visual debuggers will be cool to have an ability to load the whole stack trace without setting the depth. Right now we should pass it through the API deep to proc.stacktrace function.

@derekparker What do you think?

Original request – https://youtrack.jetbrains.com/issue/GO-3995

[derekparker]

@ignatov sorry for the delay in responding to this.

I think that's a fine idea, I'm fine with supporting it. We could potentially have -1 be the sentinal value for a "full" stack trace.

Thoughts @aarzilli?

[aarzilli]

Sure, we can just add this to debugger.(*Debugger).Stacktrace:

if depth < 0 {
        depth = int(^uint(0)/(2*d.target.BinInfo().Arch.PtrSize()))
}

(it's times 2 because a stack frame is at least a return address and a frame pointer)

I wouldn't do it for the same reason I wouldn't add a "load entire string" option, memory could be corrupted and we can't handle the theoretical maximum of 1152921504606846975 frames.

[ignatov]

I'll be glad to see that feature.

[slp]

I find the default behavior of only showing 10 frames by default somewhat unexpected. In fact, a colleague drew my attention to this issue when he thought there was a problem in Delve while working on a coredump, and TBH I was confused for some time too (so there's at least two of us ;)

I've just tried this and works here (append automatically extends the slice if there are more than 10 frames):

diff --git a/pkg/proc/stack.go b/pkg/proc/stack.go
index 77bcf86..792e0ce 100644
--- a/pkg/proc/stack.go
+++ b/pkg/proc/stack.go
@@ -393,10 +393,10 @@ func (it *stackIterator) stacktrace(depth int) ([]Stackframe, error) {
        if depth < 0 {
                return nil, errors.New("negative maximum stack depth")
        }
-       frames := make([]Stackframe, 0, depth+1)
+       frames := make([]Stackframe, 0, 10)
        for it.Next() {
                frames = it.appendInlineCalls(frames, it.Frame())
-               if len(frames) >= depth+1 {
+               if depth != 0 && len(frames) >= depth+1 {
                        break
                }
        }
diff --git a/pkg/terminal/command.go b/pkg/terminal/command.go
index 3334dbb..c17d3a2 100644
--- a/pkg/terminal/command.go
+++ b/pkg/terminal/command.go
@@ -1288,7 +1288,7 @@ type stackArgs struct {
 
 func parseStackArgs(argstr string) (stackArgs, error) {
        r := stackArgs{
-               depth: 10,
+               depth: 0,
                full:  false,
        }
        if argstr != "" {

@derekparker @aarzilli If you think it's worth it, I can create a proper PR for this.

[aarzilli]

I stand by my previous opinion that this shouldn't be done. The default stack depth in parseStackArgs could however be increased from 10 to 100.

[zbdba]

I also catch this problem,hope this feature is released soon.

[zbdba]

@slp I temporarily used your code.but not working.

[slp]

@aarzilli

I stand by my previous opinion that this shouldn't be done. The default stack depth in parseStackArgs could however be increased from 10 to 100.

That would work for me too. Should I send a PR for this?

[aarzilli]

Sure, you don't need permission for that.

[dlsniper]

Wouldn't increasing the default limit load a lot more data for the clients that do not set the value themselves but rely on the default? I think that it would be better if clients could be allowed to retrieve portions of the stack themselves, using a pagination mechanism.

[aarzilli]

@dlsniper this only applies to the command line client.