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/cgo: does not correctly read DWARF data for unsigned types #39136

Open
elagergren-spideroak opened this issue May 18, 2020 · 7 comments
Open

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

elagergren-spideroak opened this issue May 18, 2020 · 7 comments
Milestone

Comments

@elagergren-spideroak
Copy link

@elagergren-spideroak elagergren-spideroak commented May 18, 2020

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

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
Copy link
Author

@elagergren-spideroak elagergren-spideroak commented May 18, 2020

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 18, 2020

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.

@ianlancetaylor ianlancetaylor added this to the Go1.16 milestone May 18, 2020
@elagergren-spideroak
Copy link
Author

@elagergren-spideroak elagergren-spideroak commented May 18, 2020

@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
Copy link
Author

@elagergren-spideroak elagergren-spideroak commented May 18, 2020

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
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 18, 2020

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

@elagergren-spideroak
Copy link
Author

@elagergren-spideroak elagergren-spideroak commented May 18, 2020

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
Copy link
Member

@mdempsky mdempsky commented Aug 11, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.