dlv/dap: do not override backend process' own environment variables with those meant for debuggee

[aktau]

NOTE: The title implies what I would want for my use case, but I don't know if that's desired. See below.

1. What version of Delve are you using (dlv version)? Current HEAD (477e46e), v1.32.2-dev
2. What version of Go are you using? (go version)? Go 1.24-dev
3. What operating system and processor architecture are you using? Linux/AMD64
4. What did you do? Try to combine dlv dap with a shim for LLDB (so that Delve contacts a proprietary debugging backend instead of LLDB). This works normally, but not with dlv dap.
5. What did you expect to see? I expect the Delve DAP backend (which spawns the debuggee when the "launch" RPC is sent) to pass the Env to child without modifying its own environment.
6. What did you see instead? This:

   delve/service/dap/server.go

   Lines 919 to 931 in 477e46e

   	for k, v := range args.Env {
   	if v != nil {
   	if err := os.Setenv(k, *v); err != nil {
   	s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", fmt.Sprintf("failed to setenv(%v) - %v", k, err))
   	return
   	}
   	} else {
   	if err := os.Unsetenv(k); err != nil {
   	s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch", fmt.Sprintf("failed to unsetenv(%v) - %v", k, err))
   	return
   	}
   	}
   	}

I'm not sure if this is a bug, or what the purpose of this is, given the documentation of LaunchConfig.Env implying that Env really should apply to the Delve backend/server:

delve/service/dap/types.go

Lines 150 to 157 in 477e46e

	// Env specifies optional environment variables for Delve server
	// in addition to the environment variables Delve initially
	// started with.
	// Variables with 'nil' values can be used to unset the named
	// environment variables.
	// Values are interpreted verbatim. Variable substitution or
	// reference to other environment variables is not supported.
	Env map[string]*string `json:"env,omitempty"`

If someone knows what Chesterton's fence I'm looking at, I'd be happy to find out. Thanks.

[aarzilli]

This is how this feature is implemented, delve's backends do not have the capability to manipulate the environment of the child process. The other frontends of delve do not even have the ability to change the environment at all.

[aktau]

This is how this feature is implemented, delve's backends do not have the capability to manipulate the environment of the child process.

There's two cases of manipulating the environment:

• Before spawning (this case): that should be easy, either with exec.Command.Env or os.StartProcess -> os.ProcAttr.Env (used by Windows AFAIK).
• After spawning: this is tricky I agree. I don't think we need it to replace the way this feature is implmeneted, as changing the parent env doesn't change the environment of a child anyway. If it was somehow necessary, one could use the property that the inferior is ptrace'd already to make syscalls there.

I made a dummy change for Linux only and it doesn't appear impossible. It:

• Adds an Env variable to debugger.Config, sourced from args.Env + the backend processes current environment.
• Passes that to native.Launch.

I'm curious as to what I'm missing.

[aarzilli]

I'm curious why this would be a problem for a lldb shim.

[aktau]

The problem goes like this:

1. Put lldb shim (lldb-server) somewhere unique, then prepend that to $PATH. E.g.: export PATH=/path/to/lldb/shim:$PATH.
2. Start a Delve DAP backend: dlv dap --listen
3. Client connects to DAP backend and issues a launch request, including an Env, which includes a PATH.
4. args.Env.PATH overrides the Delve DAP backends' path, therefore it no longer finds the lldb shim in the path.

We have a workaround: pass /path/to/lldb/shim inside the path from the DAP launch request. It feels unclean though. Why would Delve need to modify its own environment this way, I wondered.

[aarzilli]

You can set the path of debugserver using DELVE_DEBUGSERVER_PATH, the way it's called is slightly different than lldb-server but if you are writing a shim anyway...

[aktau]

I thought I had tried this path and concluded it was somehow macOS-only and I'd need to PATH-shim. Perhaps I was mistaken, we'll try it out.

Even if this works though, changing the environment of the backend instead of the debuggee feels wrong. While I can't readily think of any variable that would have an adverse effect on a running program (GOMAXPROCS, GOGC, GOMEMLIMIT, GODEBUG, ...), it's not out of the question that this could happen in the future. Also changing the environment would change it for a spawned lldb-server or debugserver as well (any children).

[aarzilli]

The env field of launch.json was added because vscode doesn't let you easily specify the environment to be used for the debugger (like you would be able to do on a shell with ENV1=blah ENV2=blah dlv debug ...). If we were to distinguish between the environment of delve itself and the environment of the child we would really need 4 environment fields:

• environment variables for delve
• environment variables for the go command, if we need to call it
• environment variables for debugserver/lldb-server/rr, if we need to use them
• environment variables for the child process

It's adding a bunch of complexity for something few people need.

[aktau]

I agree that's too much. But does the current option need to apply to the backend process, or could it (for the current use cases) also be made to apply just to the debuggee?

[aarzilli]

But the debugger is spawned by your shim. We would have to apply the env for delve, unapply it when calling debugserver and tell debugserver to reapply it (how?) Il mer 11 dic 2024, 10:59 Nicolas Hillegeer ***@***.***> ha scritto:

…

I agree that's too much. But does the current option *need* to apply to the backend process, or could it (for the current use cases) also be made to apply just to the debuggee? — Reply to this email directly, view it on GitHub <#3876 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACKEBD2ITTMIXFKRFH7EJ32FAEHZAVCNFSM6AAAAABTJFY2SOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMZVGM3DMMJUGQ> . You are receiving this because you commented.Message ID: ***@***.***>

[labath]

[Disclaimer: I've worked with @aktau on this project, but I don't speak for him. I'm a long time LLDB dev, but have very little experience with go/delve]

I think there's too much focus here on our shim use case. With my upstream hat, I'd say that this is a niche use case that's not worth it. I think we could agree on that. However, I don't think that's why this bug was filed. We have a workaround for this that works adequately.

I believe we filed the bug because we found it odd that the "environment" field in the launch request affects the environment of the debugger. I certainly found it very surprising. The debuggers I'm familiar with (gdb, lldb, including the lldb vscode plugin) all support the ability to run the debuggee with a different environment, and they all do it by applying the environment to the debuggee alone. For gdb and lldb this is particularly important, as they support debugging more than one process simultaneously, but even for one process, it's possible to come up scenarios where one would want to run the debuggee with an environment var (TMP? HOME? GOGC? LD_PRELOAD? TCMALLOC_***?) that one would not want to be applied to the debugger (or any other process spawned by the debugger).

Is that likely scenario? Probably not. I don't remember if I've ever needed to set an debuggee environment variable, that couldn't also be set on the debugger as well. However, it's common enough that both lldb and gdb (and probably other debuggers as well) have the ability to do that, and I, as a debugger developer am very happy that the I don't have to worry about the user breaking the debugger by setting some env variable meant for the debuggee.

That said, it seems delve is not one of those "other debuggers" -- even the command line client is not capable of setting debuggee-only variables. The only way to do it is to set it for both, as you pointed out in #3876 (comment), so I can see this issue (feature request?) being rejected on those grounds. However:

• although I obviously have no influence over this, I'd hope that if at some point someone proposes an option for the CLI client to set a debuggee-only variable, that this proposal would not get rejected (the CLI already supports starting the debuggee with a different working directory, and stdin/out/err from the debugger, so I think that a different environment would be a natural extension of that)
• I think that the current implementation doesn't emulate the CLI behavior particularly well anyway. As @aktau pointed out in dlv/dap: do not override backend process' own environment variables with those meant for debuggee #3876 (comment), a lot of the environment variables are only read on startup, so if I did something like GOGC=off dlv exec my/binary, the variable would affect delve as well. This would not happen via vscode, because the variable will be read before we get a chance to set it. The achieve the same behavior it would have to be set before launching the dap client (somewhere here maybe?)

---

If we were to distinguish between the environment of delve itself and the environment of the child we would really need 4 environment fields:

• environment variables for delve
• environment variables for the go command, if we need to call it
• environment variables for debugserver/lldb-server/rr, if we need to use them
• environment variables for the child process

I agree that's too much, but do we need the ability to set the variables for the first three? From what you've said, I surmise that's true. I can certainly believe that, but do you have a specific use case in mind? Can you share it?

If it is necessary, what would you say to having just two fields? One which applies to everything -- that would be the equivalent of FOO=BAR dlv exec; and one which applies to just the debuggee (and maybe also the debugserver, see next paragraph). Then, if one doesn't want to pass some variables from the first field to the debuggee, he can explicitly unset them in the second field.

tell debugserver to reapply it (how?)

This is possible, but you may have to change the way you communicate with it. The mode that works right now in lldb-server and debugserver (the question is moot for rr -- in the replay mode at least -- because the environment variables have already been set) is where you start them without a debuggee, and then launch it later by sending a packet (vRun/A). In this setup you can configure the environment for the child alone using the QEnvironment family of packets. (As it happens, this method of connection is not currently supported by our debugging backend, but I don't think that should matter here.) That's a relatively large undertaking, so you probably don't want to take that on. I'd say it would be fine to just start the debug server with the environment meant for the debuggee. at least for now.

As an alternative, debugserver also supports a way to change the environment for the debuggee alone using command line arguments. lldb-server doesn't support that right now, but if there's a demand for it (and you're willing to wait for a new release -- I'm not sure what's your compatibility story) I think I could add an equivalent feature to it.