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

When erroring on inadequate alignment, consider reminding reader that packed can lower alignment #110199

Closed
pnkfelix opened this issue Apr 11, 2023 · 5 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Apr 11, 2023

Code

#[repr(packed)]
pub struct Foo {
    a: usize,
    b: [u16; 1],
}

pub fn foo(f: &Foo) {
    bar(f.b.as_ptr());
}

pub fn bar(_ptr: *const u16) {}

#[repr(C, align(8))]
pub struct ByteThenFoo { a: u8, b: Foo }

fn main() {
    let mut v = vec![];
    v.push(ByteThenFoo {
        a: 1,
        b: Foo { a: 123, b: [2] },
    });
    
    let x = v.last().unwrap();
    foo(&x.b);
}

Current output

error[E0793]: reference to packed field is unaligned
 --> src/main.rs:8:9
  |
8 |     bar(f.b.as_ptr());
  |         ^^^^^^^^^^^^
  |
  = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
  = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

For more information about this error, try `rustc --explain E0793`.
error: could not compile `playground` (bin "playground") due to previous error

Desired output

much like the above, but point out that #[packed] makes the struct itself have lower alignment

Rationale and extra context

Spawned off of #109745 (comment)

Its not intuitive to many that #[packed] will imply a lower #[align]ment for the whole struct.

Other cases

No response

Anything else?

No response

@pnkfelix pnkfelix added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 11, 2023
@Noratrieb Noratrieb added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Apr 11, 2023
@Noratrieb
Copy link
Member

Mentor instructions: find where the error is emitted (using grep or -Ztreat-err-as-bug). Then, add the note to the diagnostics struct.
See https://rustc-dev-guide.rust-lang.org/diagnostics.html for more information.

@bindsdev
Copy link
Contributor

@rustbot claim

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 29, 2023
…tic-note, r=compiler-errors

improve error notes for packed struct reference diagnostic

Addresses rust-lang#110199
@gernot-ohner
Copy link

Is this issue solved by PR #110973?

@bindsdev
Copy link
Contributor

Is this issue solved by PR #110973?

It seems like it was part of the 1.71 milestone. I'm not sure why this issue wasn't closed.

@Dylan-DPC
Copy link
Member

Closing as this is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants
@pnkfelix @gernot-ohner @Noratrieb @bindsdev @Dylan-DPC and others