incorrectly converting C types to smaller sizes

[nergdron]

ok, this is a weird one. using the latest c-for-go, and go 1.18.1, I'm getting types converted to smaller, inaccurate types from what is in the C headers. for instance, in /usr/include/vulkan/vulkan_core.h, I have:

typedef struct VkImageDrmFormatModifierPropertiesEXT {
    VkStructureType    sType;
    void*              pNext;
    uint64_t           drmFormatModifier;
} VkImageDrmFormatModifierPropertiesEXT;

but what c-for-go generates is:

type ImageDrmFormatModifierProperties struct {
	SType             StructureType
	PNext             unsafe.Pointer
	DrmFormatModifier uint32
	ref86a0f149       *C.VkImageDrmFormatModifierPropertiesEXT
	allocs86a0f149    interface{}
}

and the diff between this and the last generated version in go-vulkan repo shows it being changed from uint64 to uint32, so this is definitely something that used to work. pretty much every instance I can find in the C code of 64-bit numbers are now converted to 32-bit versions, which obviously completely breaks. Any ideas why this is happening?

[rcoreilly]

You should try editing your /usr/local/include/stddef.h file or wherever that uint64_t type is defined -- see the notes in UPDATING.md in my PR on vulkan-go -- I made a separate directory with that file edited so the sizes were correct (ish -- couldn't get a plain uint). If you look at parser/predefined.go it only knows about basic int long etc types (the C basic types are horrible compared to go!), and relies on parsing .h typedefs to get the corresponding Go types, so somewhere it is getting the wrong definition.

I did NOT have this issue in my conversion using my edited files, and it still crashed in #121 so there is something else going on..

[TomTheBear]

Hi. I'm having a similar problem and while c-for-go recognizes that it is an uint64_t after adding the right header, it still uses uint32 on the Golang side:

func rsmi_init(Init_flags uint32) RSMI_status {
	cInit_flags, cInit_flagsAllocMap := (C.uint64_t)(Init_flags), cgoAllocsUnknown
	__ret := C.rsmi_init(cInit_flags)
	runtime.KeepAlive(cInit_flagsAllocMap)
	__v := (RSMI_status)(__ret)
	return __v
}

I tried to resolve uint64_t to its fundamental type with this small C snippet:

#include <stdlib.h>
#include <stdio.h>
#include <stdint.h>

int main() {
    uint64_t test = 0;
    if (__builtin_types_compatible_p(typeof(test), unsigned)) {
        printf("unsigned\n");
    } else if (__builtin_types_compatible_p(typeof(test), unsigned long)) {
        printf("unsigned long\n");
    } else if (__builtin_types_compatible_p(typeof(test), unsigned long long)) {
        printf("unsigned long long\n");
    } else if (__builtin_types_compatible_p(typeof(test), unsigned long int)) {
        printf("unsigned long int\n");
    }
    return 0;
}

I replaced all occurrences of uint64_t in the header with the printed unsigned long on my ubuntu box, I still get only uint32 on the Golang side. When looking at the mappings in parser/predefined.go, unsigned long (aka cc.ULong) should resolve to uint64. I explicitly set Arch: x86_64 to (hopefully) use the model64 table. If I replace all occurrences with unsigned long long it works but I don't want to hardcode the mapping.

Any helpful advice?

As a remark: I cannot edit system headers due to lack of privileges. Adding a separate header with just the typedef uint64_t ... causes warnings and does not change anything.

[rcoreilly]

Yep I had to use unsigned long long to get a uint64. I think that is the most unambiguous C way to specify a uint64. Probably the best solution would be to predefine these standard types instead of relying on parsing header files: uint64 has a fixed correspondence with the corresponding Go type. @xlab are you able to do something like that easily?

[rcoreilly]

Also, looking at those model tables, it would seem that it is just not finding the right model somehow..

and @TomTheBear instead of editing system headers, you can add a -I include path to your own header that is only used in the c-for-go build process -- that is what I did..

[TomTheBear]

My remark is related to this sentence part:

You should try editing your /usr/local/include/stddef.h file or wherever that uint64_t type is defined [...]

[nergdron]

I'm seeing very different, and mostly better, results since the v4 merge a couple days ago. I suspect this ticket is fine to close. I've got new issues with duplicate declarations of types, so I can file another ticket on that if it's not already being worked on, but I think this is ok now.

[bearsh]

@nergdron I don't think this issue is solved...

[bearsh]

I've created #138 for the data type size problem also mentioned here