Skip to content
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

cmd/compile: DWARF DW_TAG_subroutine_type doesn't mark return value's formal parameters as DW_AT_variable_parameter #59977

Closed
dev747368 opened this issue May 4, 2023 · 9 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@dev747368
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.20.4 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOOS="linux"

What did you do?

When trying to decode the type info for a pointer to a function (DW_TAG_subroutine_type), the return values of the function type DIE aren't encoded in a way that lets you determine that they are return values instead of normal parameters.

package main

type myfunctype func(int) bool

//go:noinline
func getfoo() myfunctype {
	return func(p1 int) bool {
		return false
	}
}

func main() {

	foo := getfoo();

	println("foo: ", foo(1))
}

What did you expect to see?

I expected that a function type DIE's parameters & return values to be encoded in a similar way as a function DIE.

For example, the function that is returned from getfoo() is defined this way, with the return value being the correct type and being marked with a DW_AT_variable_parameter flag:

$ readelf -wi funcdef | grep -B1 -A18 main.getfoo.func1
 <1><4959b>: Abbrev Number: 3 (DW_TAG_subprogram)
    <4959c>   DW_AT_name        : main.getfoo.func1
    <495ae>   DW_AT_low_pc      : 0x458180
    <495b6>   DW_AT_high_pc     : 0x458183
    <495be>   DW_AT_frame_base  : 1 byte block: 9c 	(DW_OP_call_frame_cfa)
    <495c0>   DW_AT_decl_file   : 0x2
    <495c4>   DW_AT_external    : 1
 <2><495c5>: Abbrev Number: 18 (DW_TAG_formal_parameter)
    <495c6>   DW_AT_name        : p1
    <495c9>   DW_AT_variable_parameter: 0
    <495ca>   DW_AT_decl_line   : 7
    <495cb>   DW_AT_type        : <0x4a079>
    <495cf>   DW_AT_location    : 0x6ec83 (location list)
 <2><495d3>: Abbrev Number: 17 (DW_TAG_formal_parameter)
    <495d4>   DW_AT_name        : ~r0
    <495d8>   DW_AT_variable_parameter: 1      <----- flag to indicate that this is a return value
    <495d9>   DW_AT_decl_line   : 7
    <495da>   DW_AT_type        : <0x499c9>   <---- normal bool type
    <495de>   DW_AT_location    : 0 byte block: 	()
 <2><495df>: Abbrev Number: 0

What did you see instead?

Looking at the DWARF for the function type definition, I get a DIE that defines 2 parameters with no way to tell that the last parameter is a return value:

$ readelf -wi funcdef | grep -A10 myfunctype
    <4a03f>   DW_AT_name        : main.myfunctype
    <4a04f>   DW_AT_byte_size   : 8
    <4a050>   Unknown AT value: 2900: 19
    <4a051>   Unknown AT value: 2904: 0x0
 <2><4a059>: Abbrev Number: 25 (DW_TAG_formal_parameter)
    <4a05a>   DW_AT_type        : <0x4a079>
 <2><4a05e>: Abbrev Number: 25 (DW_TAG_formal_parameter)
    <4a05f>   DW_AT_type        : <0x499da>   <--- bool* type instead of bool
<2><4a063>: Abbrev Number: 0                   <--------- this is where I would expect a DW_AT_variable_parameter flag
...

$ readelf -wi funcdef | grep -A10 "<499da"
 <1><499da>: Abbrev Number: 35 (DW_TAG_pointer_type)
    <499db>   DW_AT_name        : *bool
    <499e1>   DW_AT_type        : <0x499c9>
    <499e5>   Unknown AT value: 2900: 0
    <499e6>   Unknown AT value: 2904: 0x2760
...

Additionally, the return types are converted to a pointer-to-real-return-type.

@ianlancetaylor ianlancetaylor changed the title DWARF DW_TAG_subroutine_type doesn't mark return value's formal parameters as DW_AT_variable_parameter cmd/compile: DWARF DW_TAG_subroutine_type doesn't mark return value's formal parameters as DW_AT_variable_parameter May 4, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 4, 2023
@ianlancetaylor
Copy link
Contributor

CC @thanm

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 5, 2023
@cagedmantis cagedmantis added this to the Backlog milestone May 5, 2023
@mknyszek
Copy link
Contributor

@thanm In triage we assigned it to you so please take a look whenever you get the chance! Thanks.

@mknyszek
Copy link
Contributor

(And as usual feel free to unassign.)

@thanm thanm added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 13, 2024
@thanm
Copy link
Contributor

thanm commented Jun 13, 2024

Circling back to look at this issue. I can confirm that we are indeed leaving out the variable parameter attribute in this case, however I think it would be useful to understand a bit more about why this additional bit of info is needed, e.g. what your use case is. There are many places in Go's generated DWARF where things are not as complete/exhaustive as they could be. Adding this attr would result in object file and binary bloat (admittedly a small amount).

What exactly is the use case? Our main debugger (Delve) doesn't seem to need this info.

@dev747368
Copy link
Author

In my case, I am using the DWARF information to model a golang binary in Ghidra. When the DWARF information is incorrect, it leads to misleading / substandard info. Happy to go into more detail if needed.

@thanm
Copy link
Contributor

thanm commented Jun 14, 2024

Sure, I'm curious to learn more. Maybe a pointer to the GHIDRA code that reads the DWARF?

@dev747368
Copy link
Author

Sure, I'm curious to learn more. Maybe a pointer to the GHIDRA code that reads the DWARF?

Ghidra is hosted here on github

The specific file that is dealing with reading DWARF data type info and translating it into ghidra-ese is DWARFDataTypeImporter.java

As an example, when Ghidra translates the runtime._type struct, as defined in DWARF, we wind up with a definition that looks like this, which is ghidra's c-ish way of displaying struct types:

struct runtime._type  
   Length: 48 (0x30)  Alignment: 8
{ 
     uintptr  size            
     uintptr ptrdata      
     uint32  hash          
     runtime.tflag tflag           
     uint8    align          
     uint8    fieldAlign    
     uint8    kind           
     func(unsafe.Pointer,_unsafe.Pointer)_bool * *  equal         
     uint8 * gcdata       
     runtime.nameOff str              
     runtime.typeOff   ptrToThis   
} pack()

Looking at the type used by the equal field, we get a func def that is slightly mangled in the way I described:

void func(unsafe.Pointer,_unsafe.Pointer)_bool( 
    void * , 
    void * , 
    bool * )

The symbol name, which we don't(can't) parse has the correct return info, but the DWARF info led us to create it with the bogus pointer-fied return value as a parameter.

This specific struct & type probably won't cause many problems for users because they won't be exploring go's built-in runtime code where this field would be used, but this issue will be problematic in other places.

Ghidra also has code that tries to directly parse go's rtti embedded in stripped binaries, however DWARF is still used when available (and it provides some info not available in the rtti), and also for for creating bootstrap information on how to parse go's rtti structs.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/595715 mentions this issue: cmd/link: add variable_parameter attr to functype outparams

@thanm
Copy link
Contributor

thanm commented Jun 28, 2024

I have created a patch https://go-review.googlesource.com/c/go/+/595715 that should address this issue. Given the nature of the change I think it would be better to not make the change for 1.23, but rather wait and check it into 1.24 early in the development cycle. I also need to spend some time benchmarking it to understand what the compile time and binary size implications are. Thanks.

@thanm thanm removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 28, 2024
@thanm thanm modified the milestones: Backlog, Go1.24 Jun 28, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Go Compiler / Runtime Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Development

No branches or pull requests

6 participants