Unnecessary -var-create and -var-delete commands during a stack trace can cause noticeable pause each time the debuggee stops

[gareth-rees]

Summary

Each time the debuggee stops, Visual Studio Code requests a stack trace of the threads that are expanded in the CALL STACK window. Turning on engineLogging shows that MIEngine implements a stack trace for a thread by issuing a -stack-list-arguments for the thread, and then issuing a -var-create and -var-delete command for every argument in every frame in the stack.

When there are many stack frames with many arguments, or if the debugger is behind a sluggish network connection, or if the user installs a hook that runs on every GDB prompt, the time taken to execute these commands can amount to a significant delay.

These -var-create and -var-delete commands ought to be unnecessary as explained below.

Reproduction

1. Put this program in a suitable file, say recurse.c:

   static void
   r(int a, int b, int c, int d, int e, int f, int g, int h, int i, int j)
   {
       if (a)
       {
           r(b, c, d, e, f, g, h, i, j, 0);
       }
   }
   
   int
   main(void)
   {
       r(1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
       return 0;
   }
   

2. Compile it with debugging information, for example gcc -g -o recurse recurse.c.

3. Open Visual Studio Code and create a new launch configuration using the "(gdb) launch" template, supplying recurse as the value for the "program" key, and turning on engineLogging:

   "logging": {
       "engineLogging": true
   }
   

4. Select View ⟶ Run to switch to the run panel, select the new launch configuration in the RUN AND DEBUG dropdown, and press F5 to start debugging.

5. Click "Step Into" a few times until there are multiple frames on the stack.

6. Look at the DEBUG CONSOLE to see the GDB/MI commands sent by MIEngine. I took a copy of the commands issued by a single Step Over operation with ten frames on the stack and attached them here: stack-list-arguments.gz. Here's a summary of the commands issued by MIEngine to GDB:

   Command	Count
   -stack-list-arguments	2
   -stack-list-frames	1
   -stack-list-variables	1
   -stack-select-frame	20
   -var-create	210
   -var-delete	210

   If each of the -var-create and -var-delete commands takes only a millisecond due to network lag or a GDB before-prompt hook, then there's a 0.4-second pause each time the debuggee stops.

Analysis

You can see from the log output that each -var-create is followed immediately by a -var-delete. This means that MIEngine is only issuing these commands to collect information about the variables (not to get a variable reference that it can later use). Why is this? The culprit is DebuggedProcess.GetParameterInfoOnly() which has a comment explaining what's happening:

MIEngine/src/MIDebugEngine/Engine.Impl/DebuggedProcess.cs

Lines 1975 to 1976 in c594534

	// If values are requested, request simple values, otherwise we'll use -var-create to get the type of argument it is.
	var frames = await MICommandFactory.StackListArguments(values ? PrintValue.SimpleValues : PrintValue.NoValues, thread.Id, low, high);

The problem is that GetParameterInfoOnly() was called with values = false and so it passed PrintValue.NoValues to the -stack-list-arguments call and then followed up with -var-create and -var-delete commands to get the types of the arguments.

The reason why values = false here is that AD7DebugSession.HandleStackTraceRequestAsync() did not set the FIF_FUNCNAME_ARGS_VALUES flag when calling AD7Thread.EnumFrameInfo(). And the reason why this flag is unset is that VSCode did not pass a format argument to the StackTrace request, and so we are in the "default format" case:

MIEngine/src/OpenDebugAD7/AD7DebugSession.cs

Lines 1706 to 1710 in c594534

	// No formatting flags provided in the request - use the default format, which includes the module name and argument names / types
	flags |= enum_FRAMEINFO_FLAGS.FIF_FUNCNAME_MODULE |
	enum_FRAMEINFO_FLAGS.FIF_FUNCNAME_ARGS |
	enum_FRAMEINFO_FLAGS.FIF_FUNCNAME_ARGS_TYPES |
	enum_FRAMEINFO_FLAGS.FIF_FUNCNAME_ARGS_NAMES;

Solution ideas

1. The default format could include FIF_FUNCNAME_ARGS_VALUES: then GetParameterInfoOnly() would pass PrintValue.SimpleValues to the the -stack-list-arguments call, which would return the types immediately and there would be no need for the subsequent -var-create and -var-delete calls.

2. GetParameterInfoOnly() could pass PrintValue.SimpleValues to the the -stack-list-arguments call if either types or values were required, discarding the values information if it is not needed.

Software versions

Cpptools extension: 1.12.4
VSCode: 1.71.0
Commit: 784b0177c56c607789f9638da7b6bf3230d47a8c
Date: 2022-09-01T07:25:10.472Z
Electron: 19.0.12
Chromium: 102.0.5005.167
Node.js: 16.14.2
V8: 10.2.154.15-electron.0
OS: Linux x64 5.4.0-124-generic
Sandboxed: No

[gregg-miskelly]

It feels like the best solution here is to have a launch.json knob to control the default stack trace format (and ideally context menu controls in the call stack window to change it during a debug session).

[gareth-rees]

Maybe—but first I think it is worth trying to see if GDB can be fixed. The underlying problem is that the GDB/MI command -stack-list-arguments --simple-values is supposed to print simple values like integers and not print compound values like structures, so that the whole stack can be printed in reasonable time. The problem noted in #673 is that --simple-values does not work as expected in C++: in particular, references to arbitrarily large arrays, structures, and unions are printed.

I think this is a bug, or at least an important oversight, in GDB, so I have filed bug 29554 together with a patch to GDB that updates --simple-values so that it does not print references to arrays, structures, and unions. If I can get this merged, then MIEngine can be updated to take advantage of the new behaviour.

(It might be that the GDB maintainers won't agree that this is a bug, but if so I can try again, this time leaving --simple-values unchanged and adding a new print-values option that has the desired behaviour.)

[gregg-miskelly]

Even with that, I think we would still need a knob to turn off values for folks using older versions of GDB (unless we want to do version sniffing and turn it off by default).

[gareth-rees]

GDB/MI has built-in feature-detection via the -list-features command—you'll see that in my patch on bug 29554 I added a feature for the new behaviour of --simple-values which MIEngine could consult.

[gregg-miskelly]

Sounds reasonable. BTW: Due to GDB's licensing, I am not allowed to look at GDB source code.