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

The x86_64 extern "C" fn ABIs is wrong when packed structs are involved #115667

Closed
heiher opened this issue Sep 8, 2023 · 3 comments
Closed

The x86_64 extern "C" fn ABIs is wrong when packed structs are involved #115667

heiher opened this issue Sep 8, 2023 · 3 comments
Labels
C-bug Category: This is a bug.

Comments

@heiher
Copy link
Contributor

heiher commented Sep 8, 2023

I tried this code:

t.rs:

#[repr(packed, C)]
struct P {
    i: i8,
    f: f32,
}

#[no_mangle]
extern "C" fn test(p: P) {
    let ip = std::ptr::addr_of!(p.i);
    let fp = std::ptr::addr_of!(p.f);
    let i = unsafe { std::ptr::read_unaligned(ip) };
    let f = unsafe { std::ptr::read_unaligned(fp) };
    println!("{:p} {:p} {} {}", ip, fp, i, f);
}

t.c:

struct P
{
    char i;
    float f;
} __packed__;

extern void test(struct P p);

int
main (int argc, char *argv[])
{
    struct P p;
    
    p.i = 16;
    p.f = 128.0;

    test(p);

    return 0;
}

run:

rustc --crate-type=cdylib -o libt.so t.rs; gcc -o t t.c -L . -l t; LD_LIBRARY_PATH=`pwd` ./t

I expected to see this happen:

0x7ffc5a60eaa0 0x7ffc5a60eaa4 16 128

Instead, this happened:

0x7ffc31f02520 0x7ffc31f02521 88 -3695629300000000000000000000000000000

Meta

rustc --version --verbose:

rustc 1.74.0-nightly (1e746d774 2023-09-07)
binary: rustc
commit-hash: 1e746d7741d44551e9378daf13b8797322aa0b74
commit-date: 2023-09-07
host: x86_64-unknown-linux-gnu
release: 1.74.0-nightly
LLVM version: 17.0.0

Link: #115609

@heiher heiher added the C-bug Category: This is a bug. label Sep 8, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 8, 2023
@heiher
Copy link
Contributor Author

heiher commented Sep 8, 2023

rustc think x86_64 uses indirect pass mode for packed structs, but actual not.

Pass mode cast:

           args: [
               ArgAbi {
                   layout: TyAndLayout {
                       ty: P,
                       layout: Layout {
                           size: Size(5 bytes),
                           align: AbiAndPrefAlign {
                               abi: Align(1 bytes),
                               pref: Align(1 bytes),
                           },
                           abi: Aggregate {
                               sized: true,
                           },
                           fields: Arbitrary {
                               offsets: [
                                   Size(0 bytes),
                                   Size(1 bytes),
                               ],
                               memory_index: [
                                   0,
                                   1,
                               ],
                           },
                           largest_niche: None,
                           variants: Single {
                               index: 0,
                           },
                           max_repr_align: None,
                           unadjusted_abi_align: Align(1 bytes),
                       },
                   },
                   mode: Indirect {
                       attrs: ArgAttributes {
                           regular: NoAlias | NoCapture | NonNull | NoUndef,
                           arg_ext: None,
                           pointee_size: Size(5 bytes),
                           pointee_align: Some(
                               Align(1 bytes),
                           ),
                       },
                       extra_attrs: None,
                       on_stack: true,
                   },
               },
           ],

Passed by integer register with packed layout:

Breakpoint 1, 0x00007ffff7f694b0 in test () from /home/hev/ws/libt.so
#
# Breakpoint at the first instruction of test function (before executing any instructions)
#
(gdb) i r rdi
rdi            0x43000000f7ffda10  4827858804701911568 { 0x43000000, <0xf7ffda>, 0x10 }

@lukas-code
Copy link
Member

struct P
{
    char i;
    float f;
} __packed__;

That's defining a global variable named __packed__ of type struct P. The correct syntax is

struct P
{
    char i;
    float f;
} __attribute__((__packed__));

The x86-64 psABI says

  • If the size of an object is larger than eight eightbytes, or it contains unaligned fields,
    it has class MEMORY.
  • If the class is MEMORY, pass the argument on the stack at an address respecting the
    arguments alignment (which might be more than its natural alignement).

Since the field f is unaligned, passing the value indirectly / on the stack is correct.

@heiher
Copy link
Contributor Author

heiher commented Sep 8, 2023

@lukas-code Oops, my mistake. After seeing this, I mistakenly thought that __packed__ was a short alias for __attribute__((__packed__)).

@heiher heiher closed this as completed Sep 8, 2023
@fmease fmease removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

4 participants