-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/compile: invalid DWARF location list for local var - erroneously extends to function epilogue #60493
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
Comments
Before this patch, the visibility of function arguments for purposes like the `locals` command was considered to start on the line on which they are declared. I believe this was incorrect; instead their visibility should start at the beginning of the function. This makes a difference for function declarations like: 1: func foo( 2: a int, 3: b int, 4: ) Here, foo() starts on line 1 (e.g. a breakpoint on "foo" will break on :1), but "a" has a decl_line of :2 and "b" has :3. This was causing a and b to not be readable for a few instructions at the beginning of the function, although they should have been. This patch fixes it by ignoring decl_line for function arguments, essentially making them visible throughout the function. Note that function arguments are not subject to the bug in location lists described in golang/go#60493. That issues describes how the loclists for some variables erroneously extends to the end of the function's code, covering epilogue code related to stack growth that should not be covered because the respective variables have yet to be initialized when that code runs. The decl_line check helps Delve avoid making the variables visible during that epilogue, and thus prevents reading garbage. However, function arguments are initialized when the stack growth runs, and they can be read just fine during the epilogue (so their loclist is fine) - so this patch does not introduce a regression (additionally, function args declared on the same line as the function were already unprotected by the decl_line check).
Before this patch, the visibility of function arguments for purposes like the `locals` command was considered to start on the line on which they are declared. I believe this was incorrect; instead their visibility should start at the beginning of the function. This makes a difference for function declarations like: 1: func foo( 2: a int, 3: b int, 4: ) Here, foo() starts on line 1 (e.g. a breakpoint on "foo" will break on :1), but "a" has a decl_line of :2 and "b" has :3. This was causing a and b to not be readable for a few instructions at the beginning of the function, although they should have been. This patch fixes it by ignoring decl_line for function arguments, essentially making them visible throughout the function. Note that function arguments are not subject to the bug in location lists described in golang/go#60493. That issues describes how the loclists for some variables erroneously extends to the end of the function's code, covering trailing code related to stack growth that should not be covered because the respective variables have yet to be initialized when that code runs. The decl_line check helps Delve avoid making the variables visible during that trailing code, and thus prevents reading garbage. However, function arguments are already initialized when the stack growth runs, and they can be read just fine from the respective PCs (so their loclist is fine). So, this patch does not introduce a regression (additionally, function args declared on the same line as the function were already unprotected by the decl_line check).
Before this patch, the visibility of function arguments for purposes like the `locals` command was considered to start on the line on which they are declared. I believe this was incorrect; instead their visibility should start at the beginning of the function. This makes a difference for function declarations like: 1: func foo( 2: a int, 3: b int, 4: ) Here, foo() starts on line 1 (e.g. a breakpoint on "foo" will break on :1), but "a" has a decl_line of :2 and "b" has :3. This was causing a and b to not be readable for a few instructions at the beginning of the function, although they should have been. This patch fixes it by ignoring decl_line for function arguments, essentially making them visible throughout the function. Note that function arguments are not subject to the bug in location lists described in golang/go#60493. That issues describes how the loclists for some variables erroneously extends to the end of the function's code, covering trailing code related to stack growth that should not be covered because the respective variables have yet to be initialized when that code runs. The decl_line check helps Delve avoid making the variables visible during that trailing code, and thus prevents reading garbage. However, function arguments are already initialized when the stack growth runs, and they can be read just fine from the respective PCs (so their loclist is fine). So, this patch does not introduce a regression (additionally, function args declared on the same line as the function were already unprotected by the decl_line check).
Kind of a novice in this area, but the compiler makes the location lists, and the wrapper for stack overflow is not added until the compiler (or at least the SSA part, which builds the location lists) is done. Maybe the compiler is using some sort of "all-the-way-to-the-end" marker? |
Thank you for the hints; they helped.
Yeah, something like this was going on. I believe I have a patch that makes the "the end" marker be interpreted correctly. I will work on sending it over. |
This patch fixes some incorrect DWARF location lists. Before this patch, a location list for a variable that was available until the "end of its function" erroneously extended over trailing code that dealt with the stack growth code. The code in question is placed at the end of the function, but it is morally part of the function's prologue (the prologue jumps to it if stack growth is needed, and this trailer then jumps back to the prologue). Thus, for the purposes of the location lists, it should be treated more like the beginning of the function than the end of the function; instructions for reading variables at the end of the function do not apply in this code region. The patch makes the loclists for regular variable not expand over this trailing code, as they do not yet exist when this code runs. Function arguments are special though - they do exist when the stack growth code runs, and generally can be read. For function arguments, this patch "fixes up" the location lists generated by the compiler after the linker has inserted the stack growth code, as described below: The layout of a function's code is as follows: <function prologue jumps to the stack growth trailer> <regular function code> ... ret movq %rax, 0x38(%rsp) <- FuncLogicalEnd movq %rbx, 0x40(%rsp) ... callq <runtime.morestack_noctxt.abi0> <- StackGrowthCall movq 0x38(%rsp), %rax movq 0x40(%rsp), %rbx ... jmp <beginning of function> int3 <- FuncPhysicalEndA The stack growth trailer consists of register spilling (starting at FuncLogicalEnd), a call to the runtime stack growth function (marked by StackGrowthCall), and then register unspilling (up until FuncPhysicalEnd). A variable that's available at the beginning of the function (i.e. a function argument) is available in between [FuncLogicalEnd,StackGrowthCall) using the same DWARF instructions as the ones used at the beginning of the function. The variable is then available in between [StackGrowthCall,FuncPhysicalEnd) with new instructions referencing the stack location where the variable was spilled to. The patch only performs this fixup for function arguments on amd64; the tracking that linker needs to do for identifying FuncLoficalEnd and StackGrowthCall is architecture-specific and I didn't go through the trouble of doing it outside amd64. Fixes golang#60493
This patch fixes some incorrect DWARF location lists. Before this patch, a location list for a variable that was available until the "end of its function" erroneously extended over trailing code that dealt with the stack growth code. The code in question is placed at the end of the function, but it is morally part of the function's prologue (the prologue jumps to it if stack growth is needed, and this trailer then jumps back to the prologue). Thus, for the purposes of the location lists, it should be treated more like the beginning of the function than the end of the function; instructions for reading variables at the end of the function do not apply in this code region. The patch makes the loclists for regular variable not expand over this trailing code, as they do not yet exist when this code runs. Function arguments are special though - they do exist when the stack growth code runs, and generally can be read. For function arguments, this patch "fixes up" the location lists generated by the compiler after the linker has inserted the stack growth code, as described below: The layout of a function's code is as follows: <function prologue jumps to the stack growth trailer> <regular function code> ... ret movq %rax, 0x38(%rsp) <- FuncLogicalEnd movq %rbx, 0x40(%rsp) ... callq <runtime.morestack_noctxt.abi0> <- StackGrowthCall movq 0x38(%rsp), %rax movq 0x40(%rsp), %rbx ... jmp <beginning of function> int3 <- FuncPhysicalEndA The stack growth trailer consists of register spilling (starting at FuncLogicalEnd), a call to the runtime stack growth function (marked by StackGrowthCall), and then register unspilling (up until FuncPhysicalEnd). A variable that's available at the beginning of the function (i.e. a function argument) is available in between [FuncLogicalEnd,StackGrowthCall) using the same DWARF instructions as the ones used at the beginning of the function. The variable is then available in between [StackGrowthCall,FuncPhysicalEnd) with new instructions referencing the stack location where the variable was spilled to. The patch only performs this fixup for function arguments on amd64; the tracking that linker needs to do for identifying FuncLoficalEnd and StackGrowthCall is architecture-specific and I didn't go through the trouble of doing it outside amd64. Fixes golang#60493 Change-Id: I8ccc9ae802bddc9427ecc4ffc5a26f172a8d6b83
Change https://go.dev/cl/502117 mentions this issue: |
Add a test with loose assertions about location lists, checking that 60493 was fixed. The bug is fixed by https://go.dev/cl/502117 on arm64 and amd64, the same architectures as the ones where these tests run. For golang/go#60493 Change-Id: I55a356cbe41004a956b5dfb863ed1a403551c1cb
Change https://go.dev/cl/503878 mentions this issue: |
This patch fixes some incorrect DWARF location lists. Before this patch, a location list for a variable that was available until the "end of its function" erroneously extended over trailing code that dealt with the stack growth code. The code in question is placed at the end of the function, but it is morally part of the function's prologue (the prologue jumps to it if stack growth is needed, and this trailer then jumps back to the prologue). Thus, for the purposes of the location lists, it should be treated more like the beginning of the function than the end of the function; instructions for reading variables at the end of the function do not apply in this code region. The patch makes the loclists for regular variable not expand over this trailing code, as they do not yet exist when this code runs. Function arguments are special though - they do exist when the stack growth code runs, and generally can be read. For function arguments, this patch "fixes up" the location lists generated by the compiler after the linker has inserted the stack growth code, as described below: The layout of a function's code is as follows: <function prologue jumps to the stack growth trailer> <regular function code> ... ret movq %rax, 0x38(%rsp) <- FuncLogicalEnd movq %rbx, 0x40(%rsp) ... callq <runtime.morestack_noctxt.abi0> <- StackGrowthCall movq 0x38(%rsp), %rax movq 0x40(%rsp), %rbx ... jmp <beginning of function> int3 <- FuncPhysicalEndA The stack growth trailer consists of register spilling (starting at FuncLogicalEnd), a call to the runtime stack growth function (marked by StackGrowthCall), and then register unspilling (up until FuncPhysicalEnd). A variable that's available at the beginning of the function (i.e. a function argument) is available in between [FuncLogicalEnd,StackGrowthCall) using the same DWARF instructions as the ones used at the beginning of the function. The variable is then available in between [StackGrowthCall,FuncPhysicalEnd) with new instructions referencing the stack location where the variable was spilled to. The patch only performs this fixup for function arguments on amd64 and arm64; the tracking that linker needs to do for identifying FuncLoficalEnd and StackGrowthCall is architecture-specific and I didn't go through the trouble of doing it outside these two archs. Fixes golang#60493 Change-Id: I8ccc9ae802bddc9427ecc4ffc5a26f172a8d6b83
Change https://go.dev/cl/504836 mentions this issue: |
The test harness for the variable location tests in "dwtest" was using an out-of-date version of Delve; update to something more recent. Updates golang/go#60493. Change-Id: I4e72fd12fa1c6e07094067402b2c8693e37eea39 Reviewed-on: https://go-review.googlesource.com/c/debug/+/504836 Reviewed-by: David Chase <drchase@google.com> Run-TryBot: Than McIntosh <thanm@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
Add a test with loose assertions about location lists, checking that 60493 was fixed. The bug is fixed by https://go.dev/cl/502117 on arm64 and amd64, the same architectures as the ones where these tests run. For golang/go#60493 Change-Id: I55a356cbe41004a956b5dfb863ed1a403551c1cb
Add a test with loose assertions about location lists, checking that 60493 was fixed. The bug is fixed by https://go.dev/cl/502117 on arm64 and amd64, the same architectures as the ones where these tests run. For golang/go#60493 Change-Id: I55a356cbe41004a956b5dfb863ed1a403551c1cb
Add a test with loose assertions about location lists, checking that 60493 was fixed. The bug is fixed by https://go.dev/cl/502117 on arm64 and amd64, the same architectures as the ones where these tests run. For golang/go#60493 Change-Id: I55a356cbe41004a956b5dfb863ed1a403551c1cb
Add a test with loose assertions about location lists, checking that 60493 was fixed. The bug is fixed by https://go.dev/cl/502117 on arm64 and amd64, the same architectures as the ones where these tests run. For golang/go#60493 Change-Id: I55a356cbe41004a956b5dfb863ed1a403551c1cb
This patch fixes some incorrect DWARF location lists. Before this patch, a location list for a variable that was available until the "end of its function" erroneously extended over trailing code that dealt with the stack growth code. The code in question is placed at the end of the function, but it is morally part of the function's prologue (the prologue jumps to it if stack growth is needed, and this trailer then jumps back to the prologue). Thus, for the purposes of the location lists, it should be treated more like the beginning of the function than the end of the function; instructions for reading variables at the end of the function do not apply in this code region. The patch makes the loclists for regular variable not expand over this trailing code, as they do not yet exist when this code runs. Function arguments are special though - they do exist when the stack growth code runs, and generally can be read. For function arguments, this patch "fixes up" the location lists generated by the compiler after the linker has inserted the stack growth code, as described below: The layout of a function's code is as follows: <function prologue jumps to the stack growth trailer> <regular function code> ... ret movq %rax, 0x38(%rsp) <- FuncLogicalEnd movq %rbx, 0x40(%rsp) ... callq <runtime.morestack_noctxt.abi0> <- StackGrowthCall movq 0x38(%rsp), %rax movq 0x40(%rsp), %rbx ... jmp <beginning of function> int3 <- FuncPhysicalEndA The stack growth trailer consists of register spilling (starting at FuncLogicalEnd), a call to the runtime stack growth function (marked by StackGrowthCall), and then register unspilling (up until FuncPhysicalEnd). A variable that's available at the beginning of the function (i.e. a function argument) is available in between [FuncLogicalEnd,StackGrowthCall) using the same DWARF instructions as the ones used at the beginning of the function. The variable is then available in between [StackGrowthCall,FuncPhysicalEnd) with new instructions referencing the stack location where the variable was spilled to. The patch only performs this fixup for function arguments on amd64 and arm64; the tracking that linker needs to do for identifying FuncLoficalEnd and StackGrowthCall is architecture-specific and I didn't go through the trouble of doing it outside these two archs. Fixes golang#60493 Change-Id: I8ccc9ae802bddc9427ecc4ffc5a26f172a8d6b83
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
The DWARF location list for some local variables (vars declared on the first line within a function?) appears to be incorrect: it extends to include code placed in the object file at the end of the function that has to do with growing the stack (i.e. calling
runtime.morestack_noctxt
). Even though it's placed at the end of the function, this code executes when the function start if it executes at all (i.e. when the stack actually has to grow). By including the program counters corresponding to this code in the variable's location list, the compiler is declaring that the variable is live at those locations. In fact, the variable is not live; when the respective code executes, the variable has yet to be initialized. The respective location list instructions are thus invalid - where they do be followed to read the variable, we'd read random heap memory.Let's study an example.
Consider the variable
localVar
in following program:https://go.dev/play/p/P4_7_JfCbzo
This is the debug information produced for
localVar
, with my arrow markings:Notice that the variable has a loclist extending to the end of the function's code -
0x00000000004572d1
. The info says that the variable can be read from the stack.By disassembling, we can see that this is incorrect. The last instructions before
0x4572d1
are about the stack growth. When that code executes, the variable is not alive, and attempting to read it according to the generated loclist would result in reading essentially random heap memory (outside of the current function's stack frame).Disassembly
Notice the
retq
at pc4572c6
. I believe the loclist forlocalVar
should end around there.Also notice how the function prelude jumps to
0x4572c7
is the stack needs growth.I have noticed (but I'm not sure) that the loclist extends to the end of the epilogue when the variable is declared at the very beginning of the function and lives until the end of the function. I think function arguments might be susceptible to the same problem.
As a random tidbit of trivia, Delve takes a variable's
DW_AT_decl_line
into consideration, in addition to its location list, when deciding whether a particular variable can be read at a given pc, for better or worse. This way, Delve avoids using the erroneous instructions in the pc range where they are invalid (because the respective epilogue code is marked as corresponding to the line on which the function is declared, which is below the variable's decl_line). Function arguments are treated differently by Delve; I think those might be susceptible to invalid reads (but I'm not sure).What did you expect to see?
I expected the location list for
localVar
to not extend past the instruction returning from the function.What did you see instead?
The location list for
localVar
extends too much.If my involvement would be useful, I'd be happy to work on addressing this bug myself - if someone knows the solution and is willing to guide me.
cc @dr2chase @thanm @heschi as people active on go's debug symbol generation
cc @aarzilli for the Delve connection
The text was updated successfully, but these errors were encountered: