WebFreak001 / code-debug

Native debugging for VSCode
The Unlicense
400 stars 114 forks source link

GDB 12.1: -var-create: unable to create variable object #437

Open kmARC opened 1 month ago

kmARC commented 1 month ago

Symptom

When using a recent version of gdb (anything after 2022 March 08), the thread context is not selected by the extension anymore, and therefore (multi-threaded?) binaries cannot be properly debugged. Our team encountered two issues:

Screenshot 2024-08-14 at 11 17 11

Cause

I git-bisected the offending commit in gdb: https://github.com/bminor/binutils-gdb/commit/a9c82bc13cf8dd5d9076e746f744ee711eb55507 changed how the thread / stack frame context is selected. From the commit message:

With this change, there are only two GDB/MI commands that can change user selected context: -thread-select and -stack-select-frame. This allows us to remove all and rather complicated logic of notifying about user selected context change from mi_execute_command (), leaving it to these two commands themselves to notify.

Workaround

Note that I'm not sure if this is the right spot to invoke -stack-select-frame. It seems to be performant and does its job.

System

Other mentions

GitMensch commented 3 weeks ago

As that command is available in GDB since < 7.1 I think using it in some places may be the way to go - but that shouldn't be this place; checking the notes on the GDB change, this mostly means that no command should rely on a stack frame or thread to be set (it should be always explicit requested), as it previously was in this extension in some places.

@kmARC Starting from the message you see.. Can you please test the following change in the same file (backend/mi2/mi2.ts) instead (which, I guess, should fix the var-create issue):

    async varCreate(threadId: number, frameLevel: number, expression: string, name: string = "-", frame: string = "@"): Promise<VariableObject> {
        if (trace)
            this.log("stderr", "varCreate");
-       let miCommand = "var-create ";
-       if (threadId != 0) {
-           miCommand += `--thread ${threadId} --frame ${frameLevel}`;
-       }
-       const res = await this.sendCommand(`${miCommand} ${this.quote(name)} ${frame} "${expression}"`);
+       cconst miCommand = `var-create --thread ${threadId} --frame ${frameLevel} ${this.quote(name)} ${frame} "${expression}"`;
+       const res = await this.sendCommand(`${miCommand}`);
        return new VariableObject(res.result(""));
    }

and do the the same change to evalExpression() and sendCliCommand()?

If this works out, then these changes, along with a Changelog entry, can be part of a PR.

oltolm commented 3 weeks ago

I have already fixed it in https://github.com/WebFreak001/code-debug/commit/be19c277870e04469347bc2f7032af9dd2daf580.

GitMensch commented 3 weeks ago

I don't think so as you only pass the frame if there's a thread-id?!? And even if the thread id is zero, I think we should always pass it, as it can be different in GDB (for example via console and commands executing at breakpoints) than in the ui, no?

oltolm commented 3 weeks ago

I don't know if the thread id can even be zero in practice. The OP should just test the git version, but I don't know how to build the extension from source.

kmARC commented 2 weeks ago

Thanks for looking into it!

Last week I couldn't build the master branch, therefore I was trying a fix on v0.27.0 (the latest tag that time).

I'm happy to report that today master succeeded to build and the issue is gone. I also tried removing the threadId check and it works with it too (the check for thread Id was changed by https://github.com/WebFreak001/code-debug/commit/be19c277870e04469347bc2f7032af9dd2daf580, so wasn't part of the last release yet, I think.)

GitMensch commented 2 weeks ago

So this PR should be closed?

kmARC commented 2 weeks ago

So this PR should be closed?

As of now, this report is still valid; is there a new release scheduled any time soon?