cmd/cgo: does not correctly read DWARF data for unsigned types

[elagergren-spideroak]

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

$ go version
go version go1.14.2 darwin/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
GO111MODULE=""
GOARCH="amd64"
GOBIN="/Users/elagergren/gopath/bin"
GOCACHE="/Users/elagergren/Library/Caches/go-build"
GOENV="/Users/elagergren/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/elagergren/gopath"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/2b/74fz3jhd4wz4vnbf4z7ywzww0000gp/T/go-build455376315=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

https://play.golang.org/p/73rgY0pPok8

What did you expect to see?

The values for all types be 0xa4a4a4... (i.e., unsigned integers).

What did you see instead?

Them converted to signed integers.

This is because

go/src/cmd/cgo/gcc.go

Line 618 in afd477f

if _, ok := types[i].(*dwarf.UintType); ok {

only checks for *dwarf.UintType when some types are, e.g., a *QualType wrapping a *TypedefType, which then contains a *UintType.

This patch fixes it, at least on macOS.

615c615
< 					if isUnsigned(types[i]) {
---
> 					if _, ok := types[i].(*dwarf.UintType); ok {
653,672d652
< func isUnsigned(t dwarf.Type) bool {
< 	unwrap := func(t dwarf.Type) dwarf.Type {
< 		switch t := t.(type) {
< 		case *dwarf.QualType:
< 			return t.Type
< 		case *dwarf.TypedefType:
< 			return t.Type
< 		default:
< 			return nil
< 		}
< 	}
< 	for t != nil {
< 		if _, ok := t.(*dwarf.UintType); ok {
< 			return true
< 		}
< 		t = unwrap(t)
< 	}
< 	return false
< }
<

[elagergren-spideroak]

/cc: @ianlancetaylor

[ianlancetaylor]

I can't run your test case on GNU/Linux.

> go run foo.go
# command-line-arguments
/usr/bin/ld: $WORK/b001/_cgo_main.o:/tmp/go-build/cgo-generated-wrappers:2: undefined reference to `R'
collect2: error: ld returned 1 exit status

> CC=clang go run foo.go
# command-line-arguments
./foo.go:25:2: const initializer *_Cvar_T is not a constant
./foo.go:26:2: const initializer *_Cvar_U is not a constant
./foo.go:33:13: constant 11863788345444574372 overflows int

Can you shrink this down to a smaller more portable test case? Thanks.

[elagergren-spideroak]

@ianlancetaylor yeah, this works with CC=clang (tested on a Debian Buster Docker container). If CC=gcc there's an undefined reference to Q, just like when you ran it above. I'm not immediately sure why.

package main

/*
#include <stdint.h>

static const uint64_t Q = UINT64_C(0xa4a4a4a4a4a4a4a4);
*/
import "C"
import "fmt"

const Q = C.Q

func main() {
	const q uint64 = 0xa4a4a4a4a4a4a4a4
	fmt.Println("want=", q)
	fmt.Println("got=", C.Q, Q)
}

[elagergren-spideroak]

Although perhaps tangential, I'd posit that this is also a bug:

# cat x.go
package main

/*
#include <stdint.h>

// Static causes link errors with gcc.
/* static */ const uint64_t Q = UINT64_C(0xa4a4a4a4a4a4a4a4);

uint64_t use_Q(uint64_t* x) {
	return *x;
}
*/
import "C"
import "fmt"

const Q = C.Q

func main() {
	const q uint64 = 0xa4a4a4a4a4a4a4a4
	fmt.Println("want=", q)
	fmt.Println("got=", C.Q, Q)
}

root@a1cd0c7fa7b0:/go# go tool cgo -godefs x.go
// Code generated by cmd/cgo -godefs; DO NOT EDIT.
// cgo -godefs x.go

package main

import "fmt"

const Q = *_Cvar_Q

func main() {
	const q uint64 = 0xa4a4a4a4a4a4a4a4
	fmt.Println("want=", q)
	fmt.Println("got=", *_Cvar_Q, Q)
}

[ianlancetaylor]

Thanks. If it's a bug, it's a different bug.

[elagergren-spideroak]

Ok. For more context, the original bug only occurs when the (top level) const (and/or static?) qualifier is added:

# cat x.go
package main

/*
#include <stdint.h>

const uint64_t Q_const = UINT64_C(0xa4a4a4a4a4a4a4a4);
uint64_t Q_noconst = UINT64_C(0xa4a4a4a4a4a4a4a4);
*/
import "C"
import "fmt"

var Q_const = C.Q_const
var Q_noconst = C.Q_noconst

func main() {
	const q uint64 = 0xa4a4a4a4a4a4a4a4
	fmt.Println("want=", q)
	fmt.Println("got (const)=", C.Q_const, Q_const)
	fmt.Println("got (non-const)=", C.Q_noconst, Q_noconst)
}

root@a1cd0c7fa7b0:/go# CC=clang go run x.go
want= 11863788345444574372
got (const)= -6582955728264977244 -6582955728264977244
got (non-const)= 11863788345444574372 11863788345444574372

[mdempsky]

Minimal reproducer:

package p

// const unsigned long long X = 1ULL << 63;
import "C"

var x = C.X

CC=gcc go tool cgo p.go emits this in _cgo_gotypes.go:

//go:linkname __cgo_X X
//go:cgo_import_static X
var __cgo_X byte
var _Cvar_X *_Ctype_ulonglong = (*_Ctype_ulonglong)(unsafe.Pointer(&__cgo_X))

but CC=clang go tool cgo p.go emits this:

const _Ciconst_X = -0x8000000000000000

There are two related issues here:

1. cgo is emitting (untyped) constant declarations here for clang, whereas it's emitting variable references for gcc.
2. The constant handling code doesn't check for QualType, as @elagergren-spideroak points out.

The second issue is addressed by changing types[i].(*dwarf.UintType) to base(types[i]).(*dwarf.UintType).

The first issue seems to be because gcc rejects this code, but clang accepts it:

const int x = 0;
enum { X = x };

I'm not sure how best to deal with this. One possibility might be for cgo to emit code using &(x) to disambiguate whether x is addressable, and only represent non-addressable C declarations as Go constants.

--

Finally, I'll note while looking into this, I also observed that gcc and clang differently handle

// enum { X = 1ULL << 63 };
import "C"

In particular, gcc results in C.X being declared as a negative constant in Go, while clang leads to a positive constant. Casually looking at the -debug-gcc output, it seems like it's related to how they represent this in DWARF.

[odeke-em]

Thanks for the investigation, reproducers, and debugging @mdempsky @ianlancetaylor, and thanks for reporting the @elagergren-spideroak! I am punting this issue to Go1.17 as it is quite late in the Go1.16 cycle, but perhaps for Go1.17. Kindly also cc-ing @hajimehoshi, if you might be interested.