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

ZST DST PME when calling FromBytes::ref_from_bytes in client code #1867

Closed
clundin25 opened this issue Oct 10, 2024 · 19 comments
Closed

ZST DST PME when calling FromBytes::ref_from_bytes in client code #1867

clundin25 opened this issue Oct 10, 2024 · 19 comments

Comments

@clundin25
Copy link
Contributor

clundin25 commented Oct 10, 2024

Hello,

I am migrating from v0.6.6 to v0.8.3 but I am facing a compilation error in zerocode. The library code happily compiles on it's own but when I try to build a fuzz target it fails to compile zerocopy.

This makes me think that I have made a configuration or environmental mistake for the fuzz harness.

However I am having issues tracing the root cause, as the error occurs when compiling the zerocopy crate. It looks like a static assert is being hit, but I'm not entirely sure what it's purpose is and how to trace it back to my code change.

Can you help me identify how to trouble shoot further?

Here is the error I am seeing:

error[E0080]: evaluation of `<[()] as FromBytes::ref_from_bytes::StaticAssert>::ASSERT` failed
    --> /usr/local/google/home/clundin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.3/src/util/macros.rs:664:9
     |
664  |         assert!($e);
     |         ^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: {
    let dst_is_zst =
        match Self::LAYOUT.size_info {
            crate::SizeInfo::Sized { .. } => false,
            crate::SizeInfo::SliceDst(TrailingSliceLayout { elem_size, .. })
                => {
                elem_size == 0
            }
        };
    !dst_is_zst
}', /usr/local/google/home/clundin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.3/src/lib.rs:3508:9
     |
    ::: /usr/local/google/home/clundin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.3/src/lib.rs:3508:9
     |
3508 |         static_assert_dst_is_not_zst!(Self);
     |         ----------------------------------- in this macro invocation
     |
     = note: this error originates in the macro `assert` which comes from the expansion of the macro `static_assert_dst_is_not_zst` (in Nightly builds, run with -Z macro-backtrace for more info)

note: erroneous constant encountered
    --> /usr/local/google/home/clundin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.3/src/util/macros.rs:724:23
     |
724  |         const_assert!(<Self as StaticAssert>::ASSERT);
     |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
    ::: /usr/local/google/home/clundin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.3/src/lib.rs:3508:9
     |
3508 |         static_assert_dst_is_not_zst!(Self);
     |         ----------------------------------- in this macro invocation
     |
     = note: this note originates in the macro `static_assert` which comes from the expansion of the macro `static_assert_dst_is_not_zst` (in Nightly builds, run with -Z macro-backtrace for more info)

note: the above error was encountered while instantiating `fn <[()] as FromBytes>::ref_from_bytes`
    --> /usr/local/google/home/clundin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.3/src/lib.rs:4555:9
     |
4555 |         <[Self]>::ref_from_bytes(source).ok()
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0080]: evaluation of `<[()] as FromBytes::mut_from_bytes::StaticAssert>::ASSERT` failed
    --> /usr/local/google/home/clundin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.3/src/util/macros.rs:664:9
     |
664  |         assert!($e);
     |         ^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: {
    let dst_is_zst =
        match Self::LAYOUT.size_info {
            crate::SizeInfo::Sized { .. } => false,
            crate::SizeInfo::SliceDst(TrailingSliceLayout { elem_size, .. })
                => {
                elem_size == 0
            }
        };
    !dst_is_zst
}', /usr/local/google/home/clundin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.3/src/lib.rs:3744:9
     |
    ::: /usr/local/google/home/clundin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.3/src/lib.rs:3744:9
     |
3744 |         static_assert_dst_is_not_zst!(Self);
     |         ----------------------------------- in this macro invocation
     |
     = note: this error originates in the macro `assert` which comes from the expansion of the macro `static_assert_dst_is_not_zst` (in Nightly builds, run with -Z macro-backtrace for more info)

note: erroneous constant encountered
    --> /usr/local/google/home/clundin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.3/src/util/macros.rs:724:23
     |
724  |         const_assert!(<Self as StaticAssert>::ASSERT);
     |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
    ::: /usr/local/google/home/clundin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.3/src/lib.rs:3744:9
     |
3744 |         static_assert_dst_is_not_zst!(Self);
     |         ----------------------------------- in this macro invocation
     |
     = note: this note originates in the macro `static_assert` which comes from the expansion of the macro `static_assert_dst_is_not_zst` (in Nightly builds, run with -Z macro-backtrace for more info)

note: the above error was encountered while instantiating `fn <[()] as FromBytes>::mut_from_bytes`
    --> /usr/local/google/home/clundin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.3/src/lib.rs:4588:9
     |
4588 |         <[Self]>::mut_from_bytes(source).ok()
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

For context, here is my change: chipsalliance/caliptra-dpe#360

And finally here are some steps to repro locally:

$ DATE="2023-11-16"
$ git clone https://github.com/clundin25/caliptra-dpe --branch zero-copy-update 
$ cd caliptra-dpe/dpe/fuzz 
$ rustup install "nightly-${DATE}" 
$ cargo +"nightly-$DATE" install cargo-fuzz cargo-afl 
$ cargo +"nightly-$DATE" fuzz build --features libfuzzer-sys

I tweaked the $DATE variable to try and match the pinned zerocopy nightly toolchain, but the error remained unchanged.

@joshlf
Copy link
Member

joshlf commented Oct 11, 2024

I'm going to see if I can make a few tweaks to make this easier to debug, but likely what's happening is that you're calling an API that doesn't support DSTs whose trailing slice element type is a zero-sized type. The reason we don't support these is that it's ambiguous how many trailing slice elements should be constructed since, no matter what number is chosen, the resulting value will have the same number of bytes. For example, see these docs.

@joshlf
Copy link
Member

joshlf commented Oct 11, 2024

Okay, I looked more closely at your compiler output, and it seems like the problem is coming from FromBytes::slice_from on this line.

I tried locally to add #[track_caller] to FromBytes::slice_from in an attempt to get Rust to attribute the failure to slice_from's caller, but that didn't work (which makes sense - this isn't a panic, but is instead a post-monomorphization error). IMO your best bet is to update all T::slice_from calls to <[T]>::ref_from_bytes (slice_from is deprecated in favor of ref_from_bytes anyway), at which point the compiler should tell you which call is the problem.

@clundin25
Copy link
Contributor Author

From what I can tell there is no usage of T::slice_from in the code, I verified with rg "slice_from" in case I've made a mistake.

My suspicion is that the feature flags for the fuzz target are causing the issue described in the docs, since the other targets will build but it's hard to trace where the issue is.

@clundin25
Copy link
Contributor Author

Sorry @joshlf I forgot to add, do you have any more debugging advice? Thank you for all your help so far, it's been very helpful!

@joshlf
Copy link
Member

joshlf commented Oct 11, 2024

Oh hahaha I totally missed the crucial piece of information in your original message:

error[E0080]: evaluation of `<[()] as FromBytes::ref_from_bytes::StaticAssert>::ASSERT` failed

Unless I'm missing something, the call in question is to <[()] as FromBytes>::ref_from_bytes.

joshlf added a commit that referenced this issue Oct 11, 2024
joshlf added a commit that referenced this issue Oct 11, 2024
joshlf added a commit that referenced this issue Oct 11, 2024
github-merge-queue bot pushed a commit that referenced this issue Oct 11, 2024
@clundin25
Copy link
Contributor Author

clundin25 commented Oct 11, 2024

@joshlf no usage of ref_from_bytes either!

I tried upgrading to v0.8.3 without adding KnownLayout to any types (ref) and the issue persisted, which makes me believe my code changes are not the root cause, since I haven't accidentally decorated a DST with a zero-sized tail.

At this point all I can guess is there is some strange interaction with the libfuzzer setup. All other targets can compile, including the alternative "AFL" fuzzer build.

Feel free to close this, I don't have any more leads to chase but I might try to pick this up again later. Once again, thank you for your help! 😄

@joshlf
Copy link
Member

joshlf commented Oct 11, 2024

I tried upgrading to v0.8.3 without adding KnownLayout to any types (ref) and the issue persisted, which makes me believe my code changes are not the root cause, since I haven't accidentally decorated a DST with a zero-sized tail.

You wouldn't need to explicitly add KnownLayout - it's already implemented for some built-in types (such as [()]). I believe that this error message is not about a type which contains a [()], but rather about [()] itself as the top-level type being passed to FromBytes::ref_from_bytes:

error[E0080]: evaluation of `<[()] as FromBytes::ref_from_bytes::StaticAssert>::ASSERT` failed

@clundin25
Copy link
Contributor Author

@joshlf the compiler issue I am seeing has been resolved with 49749b7.

I didn't see any calls to ref_from_bytes or mut_from_bytes in the code so I unfortunately do not know the root cause.

@joshlf
Copy link
Member

joshlf commented Oct 15, 2024

@joshlf the compiler issue I am seeing has been resolved with 49749b7.

I didn't see any calls to ref_from_bytes or mut_from_bytes in the code so I unfortunately do not know the root cause.

Well that's extremely confusing! 😆

Are you guys just vendoring from our main branch? That commit isn't published on 0.8, so I assume so.

@clundin25
Copy link
Contributor Author

I am migrating us from 0.6.6 to 0.8.5! On a whim I pointed to main to see if aa9db4f would help me debug.

I was happily surprised to see things start to work!

So far this has only impacted our fuzzing build, but I am glad to see we can move to a future release to turn it back on.

@joshlf
Copy link
Member

joshlf commented Oct 15, 2024

Awesome! Unfortunately we probably won't backport 49749b7 to 0.8. Does having it help to debug what's going on? If not backporting it to 0.8 causes serious problems for you, let me know. Maybe we can figure out something else that will unblock you.

@clundin25
Copy link
Contributor Author

Does having it help to debug what's going on?

I don't think so. Since the error get's emitted when compiling the zerocopy crate it's hard to trace the offender.

If not backporting it to 0.8 causes serious problems for you, let me know. Maybe we can figure out something else that will unblock you.

I will check with my team and follow up with.

@joshlf joshlf changed the title zerocopy StaticAssert Failure in FromBytes ZST DST PME when calling FromBytes::ref_from_bytes in client code Oct 16, 2024
@joshlf
Copy link
Member

joshlf commented Oct 16, 2024

@joshlf the compiler issue I am seeing has been resolved with 49749b7.

I didn't see any calls to ref_from_bytes or mut_from_bytes in the code so I unfortunately do not know the root cause.

Reflecting on this more, this sounds impossible. Consider this error:

note: the above error was encountered while instantiating `fn <[()] as FromBytes>::ref_from_bytes`
    --> /usr/local/google/home/clundin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.3/src/lib.rs:4555:9
     |
4555 |         <[Self]>::ref_from_bytes(source).ok()
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

In 0.8.3, line 4555 is in FromBytes::slice_from, which is a deprecated method which we remove in 49749b7.

If your code was previously failing because of this error, that means that FromBytes::slice_from was being called. Presumably removing FromBytes::slice_from would cause the same code to compile because the slice_from method wouldn't resolve anymore. Off the top of my head, I can think of three possible explanations:

  • Your code actually changed in the interim and 49749b7 is a red herring
  • Your fuzzing framework somehow eats some kinds of errors like the "slice_from not found" compilation error I'm theorizing you should see
  • The type in question has another slice_from method that is "unshadowed" now that FromBytes::slice_from doesn't exist. I'm not sure if this is possible, but I know method resolution in Rust is complex. I'm pretty sure the type is () based on my previous comment, and I'm not aware of it having any method called slice_from, but maybe there's another trait in scope or something

This is very confusing 🤔

@Catamantaloedis
Copy link

I've encountered what appears to be the same issue starting yesterday using cargo-tarpaulin on Linux. By running cargo tarpaulin --debug on Linux, I've found that cargo-tarpaulin builds zerocopy using -Clink-dead-code. Meanwhile, on Windows, where I don't encounter this compile error, cargo-tarpaulin doesn't use that option.

When I build my local project on Windows using cargo rustc -- -Clink-dead-code, I get a large number of failures originating from static_assert_dst_is_not_zst!. Not sure if this is the exact cause, but hope it helps diagnose.

@joshlf
Copy link
Member

joshlf commented Oct 16, 2024

When I build my local project on Windows using cargo rustc -- -Clink-dead-code, I get a large number of failures originating from static_assert_dst_is_not_zst!. Not sure if this is the exact cause, but hope it helps diagnose.

Is your code open source so we could try to reproduce?

@Catamantaloedis
Copy link

Catamantaloedis commented Oct 16, 2024

Unfortunately it's closed-source, but I've found what seems to be a minimal reproduceable example.

Cargo.lock:

[package]
name = "zc-dead-code"
version = "0.1.0"
edition = "2021"

[dependencies]
zerocopy = { version="0.8.5", features=["derive"] }

lib.rs:

use zerocopy::{FromBytes, IntoBytes};

#[derive(IntoBytes, FromBytes)]
pub struct Foo;

On Windows (I've tested using both cargo/rustc versions 1.78 and 1.81), cargo rustc successfully builds, whereas cargo rustc -- -Clink-dead-code returns an error:

error[E0080]: evaluation of `<[Foo] as zerocopy::FromBytes::mut_from_bytes::StaticAssert>::ASSERT` failed
    --> [path_redacted]\zerocopy-0.8.5\src\lib.rs:3744:9
     |
3744 |         static_assert_dst_is_not_zst!(Self);
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: {
    let dst_is_zst =
        match Self::LAYOUT.size_info {
            crate::SizeInfo::Sized { .. } => false,
            crate::SizeInfo::SliceDst(TrailingSliceLayout { elem_size, .. })
                => {
                elem_size == 0
            }
        };
    !dst_is_zst
}', [path_redacted]\zerocopy-0.8.5\src\lib.rs:3744:9
     |
     = note: this error originates in the macro `assert` which comes from the expansion of the macro `static_assert_dst_is_not_zst` (in Nightly builds, run with -Z macro-backtrace for more info)

note: erroneous constant encountered
    --> [path_redacted]\zerocopy-0.8.5\src\lib.rs:3744:9
     |
3744 |         static_assert_dst_is_not_zst!(Self);
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: this note originates in the macro `static_assert` which comes from the expansion of the macro `static_assert_dst_is_not_zst` (in Nightly builds, run with -Z macro-backtrace for more info)   

note: the above error was encountered while instantiating `fn <[Foo] as zerocopy::FromBytes>::mut_from_bytes`
    --> [path_redacted]\zerocopy-0.8.5\src\lib.rs:4588:9
     |
4588 |         <[Self]>::mut_from_bytes(source).ok()
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0080`.

@spacejam
Copy link

spacejam commented Oct 16, 2024

This happens for me when building zerocopy in a cargo-fuzz test, without any actual usage of version 0.8.5.

cargo +nightly --version       
# prints cargo 1.84.0-nightly (15fbd2f60 2024-10-08)
uname -a 
# prints Darwin Mac.fritz.box 24.0.0 Darwin Kernel Version 24.0.0: Tue Sep 24 23:37:36 PDT 2024; root:xnu-11215.1.12~1/RELEASE_ARM64_T6020 arm64

# repro:
git clone [email protected]:komora-io/art.git
cd art
cargo +nightly fuzz build ops_0
# succeeds
cargo add zerocopy
# zerocopy 0.8.5 added
cargo +nightly fuzz build ops_0

   Compiling art v0.1.3 (/Users/tylerneely/src/art)                                                                                                                                                                                                       
error[E0080]: evaluation of `<[()] as FromBytes::ref_from_bytes::StaticAssert>::ASSERT` failed                                                                                                                                                            
    --> /Users/tylerneely/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.5/src/util/macros.rs:664:9                                                                                                                                    
     |                                                                                                                                                                                                                                                    
664  |         assert!($e);                                                                                                                                                                                                                               
     |         ^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: {                                                                                                                                                                         
    let dst_is_zst =                                                                                                                                                                                                                                      
        match Self::LAYOUT.size_info {                                                                                                                                                                                                                    
            crate::SizeInfo::Sized { .. } => false,                                                                                                                                                                                                       
            crate::SizeInfo::SliceDst(TrailingSliceLayout { elem_size, .. })                                                                                                                                                                              
                => {                                                                                                                                                                                                                                      
                elem_size == 0                                                                                                                                                                                                                            
            }                                                                                                                                                                                                                                             
        };                                                                                                                                                                                                                                                
    !dst_is_zst                                                                                                                                                                                                                                           
}', /Users/tylerneely/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.5/src/lib.rs:3508:9                                                                                                                                               
     |                                                                                                                                                                                                                                                    
    ::: /Users/tylerneely/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.5/src/lib.rs:3508:9                                                                                                                                           
     |                                                                                                                                                                                                                                                    
3508 |         static_assert_dst_is_not_zst!(Self);                                                                                                                                                                                                       
     |         ----------------------------------- in this macro invocation                                                                                                                                                                               
     |                                                                                                                                                                                                                                                    
     = note: this error originates in the macro `assert` which comes from the expansion of the macro `static_assert_dst_is_not_zst` (in Nightly builds, run with -Z macro-backtrace for more info)                                                        
                                                                                                                                                                                                                                                          
note: erroneous constant encountered                                                                                                                                                                                                                      
    --> /Users/tylerneely/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.5/src/util/macros.rs:724:23                                                                                                                                   
     |                                                                                                                                                                                                                                                    
724  |         const_assert!(<Self as StaticAssert>::ASSERT);                                                                                                                                                                                             
     |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                                                                                                                                                               
     |                                                                                                                                                                                                                                                    
    ::: /Users/tylerneely/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.5/src/lib.rs:3508:9                                                                                                                                           
     |                                                                                                                                                                                                                                                    
3508 |         static_assert_dst_is_not_zst!(Self);                                                                                                                                                                                                       
     |         ----------------------------------- in this macro invocation                                                                                                                                                                               
     |                                                                                                                                                                                                                                                    
     = note: this note originates in the macro `static_assert` which comes from the expansion of the macro `static_assert_dst_is_not_zst` (in Nightly builds, run with -Z macro-backtrace for more info)                                                  
                                                                                                                                                                                                                                                          
note: the above error was encountered while instantiating `fn <[()] as FromBytes>::ref_from_bytes`                                                                                                                                                        
    --> /Users/tylerneely/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.5/src/lib.rs:4555:9                                                                                                                                           
     |                                                                                                                                                                                                                                                    
4555 |         <[Self]>::ref_from_bytes(source).ok()                                                                                                                                                                                                      

@joshlf
Copy link
Member

joshlf commented Oct 16, 2024

Okay, thanks for everyone's help on this! We've root-caused it. We'll start thinking about how to work around this issue.

jswrenn added a commit that referenced this issue Oct 16, 2024
These are causing PMEs in the presence of `-C link-dead-code`,
which casues them to be monomorphized for `Self = ()`, thus
producing the problematic ZSTy DST `[()]`.

Ref rust-lang/rust#131793
Fixes #1867
jswrenn added a commit that referenced this issue Oct 16, 2024
These are causing PMEs in the presence of `-C link-dead-code`,
which casues them to be monomorphized for `Self = ()`, thus
producing the problematic ZSTy DST `[()]`.

Also bumps version to 0.8.6.

Ref rust-lang/rust#131793
Fixes #1867
jswrenn added a commit that referenced this issue Oct 16, 2024
These are causing PMEs in the presence of `-C link-dead-code`,
which casues them to be monomorphized for `Self = ()`, thus
producing the problematic ZSTy DST `[()]`.

Also bumps version to 0.8.6.

Ref rust-lang/rust#131793
Fixes #1867
joshlf pushed a commit that referenced this issue Oct 16, 2024
These are causing PMEs in the presence of `-C link-dead-code`,
which casues them to be monomorphized for `Self = ()`, thus
producing the problematic ZSTy DST `[()]`.

Also bumps version to 0.8.6.

Ref rust-lang/rust#131793
Fixes #1867
@joshlf
Copy link
Member

joshlf commented Oct 16, 2024

Closed by #1923 and published in 0.8.6.

Let me know if you encounter any more issues!

@joshlf joshlf closed this as completed Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants