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: -godefs makes negative values from unsigned constants on openbsd/386 #2470

Closed
mikioh opened this issue Nov 16, 2011 · 16 comments

Comments

@mikioh
Copy link
Contributor

commented Nov 16, 2011

Before filing a bug, please check whether it has been fixed since
the latest release: run "hg pull", "hg update default", rebuild, and
retry
what you did to
reproduce the problem.  Thanks.

What steps will reproduce the problem?
1. run cgo -godefs test.go on openbsd/386
2.
3.

What is the expected output?
const (
        BIOCGDLTLIST = 0xc008427b
)

What do you see instead?
const (
        BIOCGDLTLIST = -0x3ff7bd85
)

Which compiler are you using (5g, 6g, 8g, gccgo)?
8g

Which operating system are you using?
openbsd/386

Which revision are you using?  (hg identify)
44246cae3737 tip

Please provide any additional information below.

Attachments:

  1. test.go (1458 bytes)
@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2011

Comment 1:

What is the C definition of BIOCGDLTLIST?
Is it an enum?  A #define?  What is the precise value?
On OpenBSD the headers should be simple enough
that this is not a hard question to answer.
I would not even ask if you were running Linux.
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2011

Comment 2:

Looks like it is
#define BIOCGDLTLIST    _IOWR('B',123, struct bpf_dltlist)
#define _IOWR(g,n,t)    _IOC(IOC_INOUT, (g), (n), sizeof(t))
#define _IOC(inout,group,num,len) \
  (inout | ((len & IOCPARM_MASK) << 16) | ((group) << 8) | (num))
#define IOC_OUT         (unsigned long)0x40000000
#define IOC_IN          (unsigned long)0x80000000
#define IOC_INOUT       (IOC_IN|IOC_OUT)
#define IOCPARM_MASK    0x1fff
@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2011

Comment 3:

On my Mac, running cgo -godefs on this program produces
the correct (unsigned) output:
package main
/*
struct bpf_dltlist { int x[4]; };
#define BIOCGDLTLIST    _IOWR('B',123, struct bpf_dltlist)
#define _IOWR(g,n,t)    _IOC(IOC_INOUT, (g), (n), sizeof(t))
#define _IOC(inout,group,num,len) (inout | ((len & IOCPARM_MASK) << 16) | ((group)
<< 8) | (num))
#define IOC_OUT         (unsigned long)0x40000000
#define IOC_IN          (unsigned long)0x80000000
#define IOC_INOUT       (IOC_IN|IOC_OUT)
#define IOCPARM_MASK    0x1fff  
*/
import "C"
const (
    BIOCGDLTLIST = C.BIOCGDLTLIST
)

Owner changed to @rsc.

Status changed to Accepted.

@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented Nov 17, 2011

Comment 4:

Just tried comment #3 snippet on openbsd/386:
vm8:work {151} cgo -godefs test2.go 
// Created by cgo -godefs - DO NOT EDIT
// cgo -godefs test2.go
package main
const (
        BIOCGDLTLIST = -0x3fefbd85
)
@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 17, 2011

Comment 5:

What does gcc -version print on your OpenBSD/386 machine?
Thanks.
@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented Nov 17, 2011

Comment 6:

Ah, EnumValues returns 64-bit integer,
// A Type collects information about a type in both the C and Go worlds.
type Type struct {
        Size       int64
        Align      int64
        C          *TypeRepr
        Go         ast.Expr
        EnumValues map[string]int64
}
then,
gcc.go:L604: n.Const = fmt.Sprintf("%#x", n.Type.EnumValues[k])
hmm...
@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented Nov 17, 2011

Comment 7:

Reading specs from /usr/lib/gcc-lib/i386-unknown-openbsd5.0/4.2.1/specs
Target: i386-unknown-openbsd5.0
Configured with: OpenBSD/i386 system compiler
Thread model: posix
gcc version 4.2.1 20070719
@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 17, 2011

Comment 8:

Can we make it a map[string]interface{} and store
an unsigned number when we know it's unsigned?
@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 17, 2011

Comment 9:

Actually, that doesn't matter in this case.  The positive
number is easily in the range of an int64.
Russ
@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented Nov 17, 2011

Comment 10:

> when we know it's unsigned?
First question: how?
Second question: is "-0x01" valid in Go spec?
If not, I guess package format might be an appropriate guy
for this case, by assuming the verb "%#x" means its value 
is unsigned.
@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented Nov 17, 2011

Comment 11:

Oops, I was wrong.
kqueue stuff for BSD variants require negative 
integer values, e.g., EVFILT_AIO = -0x3.
@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 17, 2011

Comment 12:

The problem is that we extract the value of complicated #define values
by sticking them in enum { X = theDefine }; and then reading the value
out of the debug info.  The C spec is super vague about the concrete
type of an enum type, but often the compiler picks int, and in this case,
on OpenBSD/386, the defined value is negative if interpreted as an int.
There are many things we could do but the easiest is probably to
stop using the enum values entirely.  Cgo already has code to
generate a long long array with the values and then read the
values out of the object file directly for Mach-O.  This was necessary
for Apple's latest LLVM-based gcc implementations, which have
taken an axe to the amount of useful DWARF information they
leave in the object.  If we make the ELF systems read that array
too, then it might all just work.
Want to give it a try, Mikio?
@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 17, 2011

Comment 13:

To find the relevant code in cgo grep for __cgodebug_data.
@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented Nov 18, 2011

Comment 14:

> Want to give it a try, Mikio?
Sounds interesting but it's too much for me for now.
@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2011

Comment 15:

Labels changed: added priority-later, removed priority-medium.

@4a6f656c

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2012

Comment 17:

This issue was closed by revision 4cfcb4a.

Status changed to Fixed.

@mikioh mikioh added fixed labels Sep 7, 2012

@golang golang locked and limited conversation to collaborators Jun 24, 2016

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.