Repository navigation
Bindgen can't translate #pragma pack(n) into #[repr(packed = "n")] #537
Description
Activity
Thanks for the bug report! I'm not super familiar with
#pragma pack-- is it standard? Or at least widely used? If not, we might want to consider whether implementing support is worth the pay off, and what ongoing maintenance costs might be.If we did support this, I think we would want to use
#[repr(C, packed)], not just#[repr(C)].is it standard?
#pragma packis understood by GCC and Visual Studio, and I think Borland too. I don't know about clang.Or at least widely used?
I hope not! Abusing structs for serialisation purposes doesn't seem to be a good idea, and can cause problems. Unfortunately, I know of least one library which does this extensively.
whether implementing support is worth the pay off
bindgen is currently generating structs which have the wrong layout; this could also lead to some fairly strange behaviour. I think there are a number of ways you could go here:
- When bindgen sees
#pragma pack(*)it aborts. - When bindgen sees
#pragma pack(*)it spits out a warning and stops generating bindings until it sees#pragma pack()(which resets things to normal). - bindgen generates compatible structs. In this case, I think a warning is still in order, since
repr(packed)can lead to undefined behaviour in rust.
#[repr(C, packed)], not just #[repr(C)]
Ah yes, you're right.
Reacted by Pete Gadomski- When bindgen sees
I don't think we can even see
#pragma packedfrom libclang?I guess we can try to guess if we see the alignment is 1 and the relevant fields are packed when looking at their offsets, but...
Hello,
I'm having a similar issue on macOS 10.12.4. When generating the bindings for a header which includes "fcntl.h", the following struct is encountered:
#pragma pack(4) struct log2phys { unsigned int l2p_flags; /* unused so far */ off_t l2p_contigbytes; /* F_LOG2PHYS: unused so far */ /* F_LOG2PHYS_EXT: IN: number of bytes to be queried */ /* OUT: number of contiguous bytes at this position */ off_t l2p_devoffset; /* F_LOG2PHYS: OUT: bytes into device */ /* F_LOG2PHYS_EXT: IN: bytes into file */ /* OUT: bytes into device */ }; #pragma pack()
Rust-bindgen generates the following code:
#[repr(C)] #[derive(Debug, Copy)] pub struct log2phys { pub l2p_flags: ::std::os::raw::c_uint, pub l2p_contigbytes: off_t, pub l2p_devoffset: off_t, } #[test] fn bindgen_test_layout_log2phys() { assert_eq!(::std::mem::size_of::<log2phys>() , 20usize , concat ! ( "Size of: " , stringify ! ( log2phys ) )); assert_eq! (::std::mem::align_of::<log2phys>() , 4usize , concat ! ( "Alignment of " , stringify ! ( log2phys ) )); assert_eq! (unsafe { & ( * ( 0 as * const log2phys ) ) . l2p_flags as * const _ as usize } , 0usize , concat ! ( "Alignment of field: " , stringify ! ( log2phys ) , "::" , stringify ! ( l2p_flags ) )); assert_eq! (unsafe { & ( * ( 0 as * const log2phys ) ) . l2p_contigbytes as * const _ as usize } , 4usize , concat ! ( "Alignment of field: " , stringify ! ( log2phys ) , "::" , stringify ! ( l2p_contigbytes ) )); assert_eq! (unsafe { & ( * ( 0 as * const log2phys ) ) . l2p_devoffset as * const _ as usize } , 12usize , concat ! ( "Alignment of field: " , stringify ! ( log2phys ) , "::" , stringify ! ( l2p_devoffset ) )); }
Some of these assertions are failing:
::std::mem::size_of::<log2phys>()is 24 bytes, not 20.::std::mem::align_of::<log2phys>()is 8 bytes, not 4.unsafe { & ( * ( 0 as * const log2phys ) ) . l2p_contigbytes as * const _ as usize }is 8 bytes not 4.unsafe { & ( * ( 0 as * const log2phys ) ) . l2p_devoffset as * const _ as usize }is 16 bytes not 12.
I don't know how helpful this is, but please let me know if I can supply more information.
Thanks for making bindgen!
I'm running into this same issue for osx/ios.
Some of the system provided headers use
packquite heavily.Running:
cd /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/ rg -c "^\s*#pragma\s*pack\(.*\d+\)" . | rg -v -N ":1$"
Gives the count of
#pragma packper header (with more than 1 occurrence):usr/include/mach/clock.h:6 usr/include/mach/clock_priv.h:4 usr/include/mach/clock_reply.h:2 usr/include/mach/exc.h:6 usr/include/mach/host_security.h:4 usr/include/mach/host_priv.h:50 usr/include/mach/lock_set.h:12 usr/include/mach/mach_host.h:54 usr/include/mach/mach_port.h:72 usr/include/mach/mach_vm.h:40 usr/include/mach/mach_voucher.h:10 usr/include/mach/processor.h:12 usr/include/mach/processor_set.h:20 usr/include/mach/task.h:102 usr/include/mach/thread_act.h:56 usr/include/mach/vm_map.h:54 usr/include/net/if.h:4 usr/include/servers/netname.h:8 System/Library/Frameworks/AudioToolbox.framework/Versions/A/Headers/AudioUnitCarbonView.h:2 System/Library/Frameworks/IOKit.framework/Versions/A/Headers/OSMessageNotification.h:2 System/Library/Frameworks/IOKit.framework/Versions/A/Headers/iokitmig.h:160 System/Library/Frameworks/PCSC.framework/Versions/A/Headers/pcsclite.h:2 System/Library/Frameworks/IOKit.framework/Versions/A/Headers/scsi/SCSICmds_INQUIRY_Definitions.h:3 System/Library/Frameworks/IOKit.framework/Versions/A/Headers/usb/USB.h:5 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/gssd/gssd_mach.h:18 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/kextd/kextd_mach.h:2 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/IOKit/OSMessageNotification.h:2 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/lockd/lockd_mach.h:6 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/audit_triggers_server.h:2 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/clock.h:6 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/clock_priv.h:4 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/clock_reply_server.h:2 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/coalition_notification_server.h:2 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/exc_server.h:6 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/host_security.h:4 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/host_priv.h:50 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/ktrace_background.h:2 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/lock_set.h:12 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/mach_exc_server.h:6 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/mach_host.h:50 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/mach_port.h:72 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/mach_voucher.h:10 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/mach_voucher_attr_control.h:4 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/mach_vm.h:40 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/memory_object_default_server.h:2 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/memory_object_control.h:24 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/net/if.h:2 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/notify_server.h:10 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/processor.h:12 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/processor_set.h:20 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/sysdiagnose_notification_server.h:2 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/task_access.h:4 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/task_access_server.h:4 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/task.h:102 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/telemetry_notification_server.h:2 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/thread_act.h:56 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/upl.h:8 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/mach/vm_map.h:54 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/sys/fcntl.h:2 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/sys/mount.h:2 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/IOKit/scsi/SCSICmds_INQUIRY_Definitions.h:3 System/Library/Frameworks/Kernel.framework/Versions/A/Headers/IOKit/usb/USB.h:5 System/Library/Frameworks/ApplicationServices.framework/Versions/A/Frameworks/QD.framework/Versions/A/Headers/ColorSyncDeprecated.h:2And there are many more headers with a single
#pragma packoccurrence (not listed here to avoid excessive noise).AFAICT, @emilo is right, and
libclangdoesn't even give us access to#pragma pack.For this header
#pragma pack(1) struct foo { int x; };
we get this
libclangAST (note theUnexposedAttr):( kind = StructDecl spelling = "foo" location = /home/fitzgen/scratch/packed.h:2:8 is-definition? true is-declaration? true is-inlined-function? false usr = "c:@S@foo" semantic-parent.kind = TranslationUnit semantic-parent.spelling = "/home/fitzgen/scratch/packed.h" semantic-parent.location = builtin definitions semantic-parent.is-definition? false semantic-parent.is-declaration? false semantic-parent.is-inlined-function? false type.kind = Record type.cconv = 100 type.spelling = "struct foo" type.is-variadic? false ( kind = UnexposedAttr spelling = "" location = builtin definitions is-definition? false is-declaration? false is-inlined-function? false type.kind = Invalid ) ( kind = FieldDecl spelling = "x" location = /home/fitzgen/scratch/packed.h:2:18 is-definition? true is-declaration? true is-inlined-function? false usr = "c:@S@foo@FI@x" semantic-parent.kind = StructDecl semantic-parent.spelling = "foo" semantic-parent.location = /home/fitzgen/scratch/packed.h:2:8 semantic-parent.is-definition? true semantic-parent.is-declaration? true semantic-parent.is-inlined-function? false semantic-parent.usr = "c:@S@foo" semantic-parent.semantic-parent.kind = TranslationUnit semantic-parent.semantic-parent.spelling = "/home/fitzgen/scratch/packed.h" semantic-parent.semantic-parent.location = builtin definitions semantic-parent.semantic-parent.is-definition? false semantic-parent.semantic-parent.is-declaration? false semantic-parent.semantic-parent.is-inlined-function? false type.kind = Int type.cconv = 100 type.spelling = "int" type.is-variadic? false ) )Another version of this issue:
struct AlignedToLess { int i; } __attribute__ ((packed,aligned(2)));
Hmmm, reading http://llvm.org/viewvc/llvm-project?view=revision&revision=191345 (and the patch in http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20130923/089367.html) I would assume that
packedshould indeed be exposed by libclang?It appears
clang-sysat least has the constant: https://github.com/KyleMayes/clang-sys/blob/da6280de7ef5f5e7dde7e63ea00fb6df26eda601/src/lib.rs#L446- On 10/19/2017 08:25 PM, Christian Legnitto wrote:> Hmmm, readinghttp://llvm.org/viewvc/llvm-project?view=revision&revision=191345 I would assume that packed should indeed be exposed by libclang?Yup, __attribute__(packed) is exposed by libclang, but not pragma pack, unfortunately.
- added a commit that references this issue
on Nov 1, 2017 I have a band-aid in #1136
It should make
#pragma packed(1)work.For
#pragma pack(n)wheren > 1, it detects that something is off and makes the type opaque. This is the best that we can do since Rust doesn't have#[repr(packed = "n")]yet.- changed the title
[-]Bindgen doesn't know about #pragma pack(1)[/-][+]Bindgen doesn't know about `#pragma pack(...)`[/+]on Nov 1, 2017 - changed the title
[-]Bindgen doesn't know about `#pragma pack(...)`[/-][+]Bindgen can't translate `#pragma pack(n)` into `#[repr(packed = "n")]`[/+]on Nov 1, 2017 - added a commit that references this issue
on Nov 2, 2017 - added a commit that references this issue
on Nov 2, 2017 The PR for
packedis there rust-lang/rust#48528It would be great if
bindgencould start mapping#pragma pack(N)and thepackedC attributes to#[repr(packed(N))]behind some sort of opt-in feature - the produced bindings are necessarily going to require nightly Rust untilrepr(packed)is stabilized.Given that getting struct packing wrong is essentially undefined behavior, I'd wish that once the PR is merged it can be quickly stabilized.
@fitzgen
repr(packed(N))just landed this week, would it be possible for bindgen to use it instead of making the struct opaque?We can start doing that when the rust target = nightly now. I don't have time to write a PR myself, but I could review one.
Reacted by gnzlbg- added 3 commits that reference this issue
on Jan 8, 2019
Given the header
bindgen generates
which uses
repr(C)instead ofrepr(packed), and therefore has the wrong alignment:In fact, bindgen emits a warning to this effect:
Details
Steps to reproduce: