go icon indicating copy to clipboard operation
go copied to clipboard

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

Open dev747368 opened this issue 2 years ago • 9 comments

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.

dev747368 avatar May 04 '23 17:05 dev747368

CC @thanm

ianlancetaylor avatar May 04 '23 21:05 ianlancetaylor

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

mknyszek avatar May 10 '23 19:05 mknyszek

(And as usual feel free to unassign.)

mknyszek avatar May 10 '23 19:05 mknyszek

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.

thanm avatar Jun 13 '24 12:06 thanm

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.

dev747368 avatar Jun 14 '24 04:06 dev747368

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

thanm avatar Jun 14 '24 11:06 thanm

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.

dev747368 avatar Jun 24 '24 18:06 dev747368

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

gopherbot avatar Jun 28 '24 11:06 gopherbot

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 avatar Jun 28 '24 11:06 thanm