xlab / c-for-go

Automatic C-Go Bindings Generator for Go Programming Language
https://c.for-go.com
MIT License
1.5k stars 119 forks source link

Bitfield assignment is broken #26

Open pwaller opened 7 years ago

pwaller commented 7 years ago

bitfield.yml

---
GENERATOR:
  PackageName: bitfield
  PackageDescription: "n.a."
  PackageLicense: "n.a."
  Includes: [bitfield.h]
  FlagGroups:
  # Because I am lazy and trying to get this working quickly.
  - {name: "LDFLAGS", flags: ["-Wl,--allow-multiple-definition"]}

PARSER:
  IncludePaths: [.]
  SourcesPaths: [bitfield/bitfield.h]

TRANSLATOR:
  Rules:
    global:
      - {action: accept}
      - {action: ignore, from: "__GO__"}

bitfield/bitfield.h

#pragma once

typedef struct {
        int x : 8;
        int y : 8;
        int z : 16;
        unsigned int zz : 16;
        signed int a : 32;
        long int b : 64;
} Foo;

void dofoo(Foo *foo) {}

Go compiler output:

c-for-go bitfield.yml && go build -v ./bitfield
bitfield/cgo_helpers.go:123: cannot use C.int(x.x) (type C.int) as type uint8 in assignment
bitfield/cgo_helpers.go:127: cannot use C.int(x.y) (type C.int) as type uint8 in assignment
bitfield/cgo_helpers.go:131: cannot use C.int(x.z) (type C.int) as type uint16 in assignment
bitfield/cgo_helpers.go:135: cannot use C.uint(x.zz) (type C.uint) as type uint16 in assignment
bitfield/cgo_helpers.go:139: cannot use C.int(x.a) (type C.int) as type uint32 in assignment
bitfield/cgo_helpers.go:143: cannot use C.long(x.b) (type C.long) as type uint64 in assignment

Comment

Sorry, I ran out of time to describe things in detail. Hopefully the problem is clear from the above. It seems like a quirk of the Go compiler that it expects different types than used in the type definition in this circumstance.

I have a work in progress patch which seems to fix the problem:

https://github.com/pwaller/cgogen/commit/6c435cecf788aba6513db2ebf766680689259477

If it's a valid approach, I would like to submit it as a PR, once it's tidied up.

cznic commented 7 years ago

@pwaller & @xlab

I haven't really checked but it's quite possible the bitfield handling is broken in cc. I've run into several such issues and somehow bypassed them in ccir, for example. Once I will get [more] confident about the correctness of the bypass, I gues it's due to backport ccir's hack to cc.

However, please feel free to report any cc issue directly at its issue tracker. No promises about when the fixes will come, but I'm currently becoming more and more dependent on [at least some] correctness of cc, so bug reports are definitely welcome.

pwaller commented 7 years ago

@cznic as far as I can tell, the interpretation of the bitfield is correct from cc's side. The problem is a difference between what c-for-go and cgo expect. From a careful inspection of the error output (above); you can see that for some reason, even though the bitfield's type is an int, if the bitfield with is 8 - even though the type is an int, cgo expects you to assign a uint8 to it! Not a C.uint8_t, but a go uint8. I think this is a surprising feature of cgo and I can't find it documented anywhere.


Separate thought:

I didn't think to check non-multiples of 8, but in that case I get:

typedef struct {
        int i : 1;
        int j : 7;
        int k : 15;
        int l : 31;
} Foo;
bitfield/cgo_helpers.go:159: x.refb43e23c1.i undefined (type *C.struct___0 has no field or method i)
bitfield/cgo_helpers.go:160: x.refb43e23c1.j undefined (type *C.struct___0 has no field or method j)
bitfield/cgo_helpers.go:161: x.refb43e23c1.k undefined (type *C.struct___0 has no field or method k)
bitfield/cgo_helpers.go:162: x.refb43e23c1.l undefined (type *C.struct___0 has no field or method l)

... which seems to suggest that CGo doesn't support odd-sized bitfields? Am I missing something here - is the above struct definition valid C?

cznic commented 7 years ago

Am I missing something here - is the above struct definition valid C?

Seems valid. I'm not aware of any requirement of the bitfileds to be of width a multiply of something.

CtrlZvi commented 7 years ago

I don't think it's just odd bitfield sizes. I think it might be any bitfields that are not powers of 2? From the cgo command documentation: "C struct fields that cannot be expressed in Go, such as bit fields or misaligned data, are omitted in the Go struct, replaced by appropriate padding to reach the next field or the end of the struct."