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.

Sign up for GitHub

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

Jump to bottom

debug/dwarf: cgo use causing build panic #13039

Closed
tlby opened this issue Oct 23, 2015 · 4 comments
Closed

debug/dwarf: cgo use causing build panic #13039

tlby opened this issue Oct 23, 2015 · 4 comments
Labels
FrozenDueToAge
Milestone
Go1.6

Comments

@tlby
Copy link

tlby commented Oct 23, 2015

I have a go code example that is causing "go build" to panic using go version go1.5.1 linux/amd64. The C struct declaration in that sample is awkward, but is a simplification of a pattern I found in upstream library headers.

package main;
/*
typedef struct aaa *AAA;
typedef AAA BBB;
struct aaa { BBB val; };

static inline AAA new_thingy(void) {
    return (AAA)0;
}

static inline void thingy_thing(AAA t) {
    (void)t;
}

static inline void doit() {
    thingy_thing(new_thingy());
}
*/
import "C"
import "fmt"

func main() {
    fmt.Println("ENTER")
    C.doit()
    C.thingy_thing(C.new_thingy())
    fmt.Println("LEAVE")
}

reproduces the problem. I found that a small patch to the go sources will allow this example to build and run cleanly, but it's not a good solution and it's not portable code.

diff --git a/src/debug/dwarf/type.go b/src/debug/dwarf/type.go
index a5daa1d..8fb1375 100644
--- a/src/debug/dwarf/type.go
+++ b/src/debug/dwarf/type.go
@@ -259,7 +259,14 @@ type TypedefType struct {

 func (t *TypedefType) String() string { return t.Name }

-func (t *TypedefType) Size() int64 { return t.Type.Size() }
+func (t *TypedefType) Size() int64 {
+    if t.Type == nil {
+        println("unresolved typedef, pretending \"" + t.String() +
+            "\" is some kind of pointer for now")
+        return 8
+    }
+    return t.Type.Size()
+}

 // typeReader is used to read from either the info section or the
 // types section.

which assumes that any time t.Type is not defined it's always an opaque struct pointer, and futher assumes that pointers are always 8 bytes. I'm certain a better solution is possible.
@ianlancetaylor ianlancetaylor changed the title cgo use causing build panic debug/dwarf: cgo use causing build panic Oct 24, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Oct 24, 2015
@rsc
Copy link
Contributor

rsc commented Nov 5, 2015

/cc @aclements

@aclements aclements self-assigned this Jan 8, 2016
@aclements
Copy link
Member

Awesome.

Here's the relevant readelf output for the cgo-generated source:

  Compilation Unit @ offset 0x0:
   Length:        0x14c (32-bit)
   Version:       2
   Abbrev Offset: 0x0
   Pointer Size:  8
 <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <c>   DW_AT_producer    : (indirect string, offset: 0x7e): GNU C 4.8.4 -m64 -mtune=generic -march=x86-64 -gdwarf-2 -fstack-protector    
    <10>   DW_AT_language    : 1    (ANSI C)
    <11>   DW_AT_comp_dir    : (indirect string, offset: 0x5d): /home/austin/go.dev/austin/13039    
    <15>   DW_AT_stmt_list   : 0x0  
 <1><19>: Abbrev Number: 2 (DW_TAG_typedef)
    <1a>   DW_AT_name        : AAA  
    <1e>   DW_AT_decl_file   : 1    
    <1f>   DW_AT_decl_line   : 4    
    <20>   DW_AT_type        : <0x24>   
 <1><24>: Abbrev Number: 3 (DW_TAG_pointer_type)
    <25>   DW_AT_byte_size   : 8    
    <26>   DW_AT_type        : <0x2a>   
 <1><2a>: Abbrev Number: 4 (DW_TAG_structure_type)
    <2b>   DW_AT_name        : aaa  
    <2f>   DW_AT_byte_size   : 8    
    <30>   DW_AT_decl_file   : 1    
    <31>   DW_AT_decl_line   : 6    
    <32>   DW_AT_sibling     : <0x45>   
 <2><36>: Abbrev Number: 5 (DW_TAG_member)
    <37>   DW_AT_name        : val  
    <3b>   DW_AT_decl_file   : 1    
    <3c>   DW_AT_decl_line   : 6    
    <3d>   DW_AT_type        : <0x45>   
    <41>   DW_AT_data_member_location: 2 byte block: 23 0   (DW_OP_plus_uconst: 0)
 <2><44>: Abbrev Number: 0
 <1><45>: Abbrev Number: 2 (DW_TAG_typedef)
    <46>   DW_AT_name        : BBB  
    <4a>   DW_AT_decl_file   : 1    
    <4b>   DW_AT_decl_line   : 5    
    <4c>   DW_AT_type        : <0x19>

The cause of the problem is that the two typedefs at 0x19 and 0x45 participate in a cycle. We start reading at the 0x19 typedef, put it in the type cache, then start constructing its underlying type. At this point, its "Type" field is still nil. This reads the pointer type at 0x24, which reads the struct type at 0x2a, which reads the typedef at 0x45, which reads the typedef at 0x19. This is in the cache, so we return it immediately, but then the readType of the 0x45 typedef tries to get its size and crashes because the 0x19 Type field is still nil.

I think we simply can't combine the steps of constructing the type graph and computing type sizes because these two steps may need to ground out at different points in cycles. Reading always terminates the cycle where it entered it, which is the 0x19 typedef here, but computing the size needs to terminate the cycle in a basic or pointer type, which is the 0x24 pointer here.

@gopherbot
Copy link

CL https://golang.org/cl/18459 mentions this issue.

@tlby
Copy link
Author

tlby commented Mar 2, 2016

Lovely, thanks for the assistance! The typedef lies I used to sidestep this problem were a bit brutal. It was a joy to remove them.

tlby added a commit to tlby/plgo that referenced this issue Mar 2, 2016
@tlby
hide the tTHX and SV* from Go
68888e1 
This is a brute force workaround for
golang/go#13039
tlby added a commit to tlby/plgo that referenced this issue Mar 2, 2016
@tlby
undo bugfix workaround
304a08e 
golang/go#13039 has been fixed for go1.6 so we
no longer need to hide SV* and tTHX from go.  Hopefully some trampoline
functions can be eliminated as well, but that'll be another pass.
@golang golang locked and limited conversation to collaborators Mar 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.6
Development

No branches or pull requests
5 participants
@rsc @aclements @tlby @ianlancetaylor @gopherbot