From bc802a427a6e59f53f06686b6cbbb0d08e6ab514 Mon Sep 17 00:00:00 2001 From: Benjamin Brittain Date: Fri, 6 Mar 2020 19:47:48 +0000 Subject: [PATCH 01/12] [rust] Add zerocopy tests to GN to run locally: $ fx run-host-tests zerocopy_derive_test Change-Id: Ibe3bb0d209ab5c8d3c3de3811d882bc4d717e952 --- BUILD.gn | 5 ++++- zerocopy-derive/BUILD.gn | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/BUILD.gn b/BUILD.gn index ce7fb4e523..1790b7017a 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -33,5 +33,8 @@ unittest_package("zerocopy_tests") { group("tests") { testonly = true - deps = [ ":zerocopy_tests" ] + deps = [ + ":zerocopy_tests", + "zerocopy-derive:tests", + ] } diff --git a/zerocopy-derive/BUILD.gn b/zerocopy-derive/BUILD.gn index a2723bac18..3bab982d1c 100644 --- a/zerocopy-derive/BUILD.gn +++ b/zerocopy-derive/BUILD.gn @@ -3,6 +3,7 @@ # found in the LICENSE file. import("//build/rust/rustc_macro.gni") +import("//build/rust/rustc_test.gni") rustc_macro("zerocopy-derive") { version = "0.1.0" @@ -14,3 +15,21 @@ rustc_macro("zerocopy-derive") { "//third_party/rust_crates:synstructure", ] } + +group("tests") { + testonly = true + deps = [ ":zerocopy-derive-test($host_toolchain)" ] +} + +if (is_host) { + rustc_test("zerocopy-derive-test") { + version = "0.1.0" + edition = "2018" + + deps = [ + "//third_party/rust_crates:proc-macro2", + "//third_party/rust_crates:syn", + "//third_party/rust_crates:synstructure", + ] + } +} From 80b650be0d630250230a4d5a11938e21cbc6aa7d Mon Sep 17 00:00:00 2001 From: Rabi Guha Date: Tue, 10 Mar 2020 04:24:06 +0000 Subject: [PATCH 02/12] [rust][zerocopy] Fix zerocopy-dervice tests - Bump compiletest version to work with latest stable rust without requiring experimental features - Also remove ![feature=repr_align_enum] since it has been stabilized. Allows `cargo test` without requiring nightly toolchains Change-Id: Id1b6c856b70279d840d276687957dcd26afe0573 --- zerocopy-derive/Cargo.toml.crates-io | 2 +- zerocopy-derive/tests/enum_as_bytes.rs | 1 - zerocopy-derive/tests/enum_from_bytes.rs | 1 - zerocopy-derive/tests/enum_unaligned.rs | 1 - zerocopy-derive/tests/ui/enum.rs | 2 - zerocopy-derive/tests/ui/enum.stderr | 220 +++++++++++------------ 6 files changed, 111 insertions(+), 116 deletions(-) diff --git a/zerocopy-derive/Cargo.toml.crates-io b/zerocopy-derive/Cargo.toml.crates-io index db7d54d166..3970b716da 100644 --- a/zerocopy-derive/Cargo.toml.crates-io +++ b/zerocopy-derive/Cargo.toml.crates-io @@ -25,4 +25,4 @@ synstructure = "0.12.1" [dev-dependencies] zerocopy = { path = "../" } -compiletest_rs = "=0.3.22" +compiletest_rs = "0.3" diff --git a/zerocopy-derive/tests/enum_as_bytes.rs b/zerocopy-derive/tests/enum_as_bytes.rs index 02c8f7adb4..2859aa3efb 100644 --- a/zerocopy-derive/tests/enum_as_bytes.rs +++ b/zerocopy-derive/tests/enum_as_bytes.rs @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#![feature(repr_align_enum)] #![allow(warnings)] use zerocopy::AsBytes; diff --git a/zerocopy-derive/tests/enum_from_bytes.rs b/zerocopy-derive/tests/enum_from_bytes.rs index 6a4f39d07c..a687cdaf89 100644 --- a/zerocopy-derive/tests/enum_from_bytes.rs +++ b/zerocopy-derive/tests/enum_from_bytes.rs @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#![feature(repr_align_enum)] #![allow(warnings)] use zerocopy::FromBytes; diff --git a/zerocopy-derive/tests/enum_unaligned.rs b/zerocopy-derive/tests/enum_unaligned.rs index a50eeb82cb..192dab8a4b 100644 --- a/zerocopy-derive/tests/enum_unaligned.rs +++ b/zerocopy-derive/tests/enum_unaligned.rs @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#![feature(repr_align_enum)] #![allow(warnings)] use zerocopy::Unaligned; diff --git a/zerocopy-derive/tests/ui/enum.rs b/zerocopy-derive/tests/ui/enum.rs index dd5250fa3f..c340a07364 100644 --- a/zerocopy-derive/tests/ui/enum.rs +++ b/zerocopy-derive/tests/ui/enum.rs @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#![feature(repr_align_enum)] - #[macro_use] extern crate zerocopy; diff --git a/zerocopy-derive/tests/ui/enum.stderr b/zerocopy-derive/tests/ui/enum.stderr index ff02dd7c9f..b7679e47ad 100644 --- a/zerocopy-derive/tests/ui/enum.stderr +++ b/zerocopy-derive/tests/ui/enum.stderr @@ -1,217 +1,217 @@ error: unrecognized representation hint - --> $DIR/enum.rs:17:8 + --> $DIR/enum.rs:15:8 | -17 | #[repr("foo")] +15 | #[repr("foo")] | ^^^^^ error: unrecognized representation hint - --> $DIR/enum.rs:23:8 + --> $DIR/enum.rs:21:8 | -23 | #[repr(foo)] +21 | #[repr(foo)] | ^^^ error: unsupported representation for deriving FromBytes, AsBytes, or Unaligned on an enum - --> $DIR/enum.rs:29:8 + --> $DIR/enum.rs:27:8 | -29 | #[repr(transparent)] +27 | #[repr(transparent)] | ^^^^^^^^^^^ error: conflicting representation hints - --> $DIR/enum.rs:35:1 + --> $DIR/enum.rs:33:1 | -35 | / #[repr(u8, u16)] -36 | | enum Generic4 { -37 | | A, -38 | | } +33 | / #[repr(u8, u16)] +34 | | enum Generic4 { +35 | | A, +36 | | } | |_^ error: must have a non-align #[repr(...)] attribute in order to guarantee this type's memory layout - --> $DIR/enum.rs:40:10 + --> $DIR/enum.rs:38:10 | -40 | #[derive(FromBytes)] +38 | #[derive(FromBytes)] | ^^^^^^^^^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:50:1 + --> $DIR/enum.rs:48:1 | -50 | / #[repr(C)] -51 | | enum FromBytes1 { -52 | | A, -53 | | } +48 | / #[repr(C)] +49 | | enum FromBytes1 { +50 | | A, +51 | | } | |_^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:56:1 + --> $DIR/enum.rs:54:1 | -56 | / #[repr(usize)] -57 | | enum FromBytes2 { -58 | | A, -59 | | } +54 | / #[repr(usize)] +55 | | enum FromBytes2 { +56 | | A, +57 | | } | |_^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:62:1 + --> $DIR/enum.rs:60:1 | -62 | / #[repr(isize)] -63 | | enum FromBytes3 { -64 | | A, -65 | | } +60 | / #[repr(isize)] +61 | | enum FromBytes3 { +62 | | A, +63 | | } | |_^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:68:1 + --> $DIR/enum.rs:66:1 | -68 | / #[repr(u32)] -69 | | enum FromBytes4 { -70 | | A, -71 | | } +66 | / #[repr(u32)] +67 | | enum FromBytes4 { +68 | | A, +69 | | } | |_^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:74:1 + --> $DIR/enum.rs:72:1 | -74 | / #[repr(i32)] -75 | | enum FromBytes5 { -76 | | A, -77 | | } +72 | / #[repr(i32)] +73 | | enum FromBytes5 { +74 | | A, +75 | | } | |_^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:80:1 + --> $DIR/enum.rs:78:1 | -80 | / #[repr(u64)] -81 | | enum FromBytes6 { -82 | | A, -83 | | } +78 | / #[repr(u64)] +79 | | enum FromBytes6 { +80 | | A, +81 | | } | |_^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:86:1 + --> $DIR/enum.rs:84:1 | -86 | / #[repr(i64)] -87 | | enum FromBytes7 { -88 | | A, -89 | | } +84 | / #[repr(i64)] +85 | | enum FromBytes7 { +86 | | A, +87 | | } | |_^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:96:1 + --> $DIR/enum.rs:94:1 | -96 | / #[repr(C)] -97 | | enum Unaligned1 { -98 | | A, -99 | | } +94 | / #[repr(C)] +95 | | enum Unaligned1 { +96 | | A, +97 | | } | |_^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:102:1 + --> $DIR/enum.rs:100:1 | -102 | / #[repr(u16)] -103 | | enum Unaligned2 { -104 | | A, -105 | | } +100 | / #[repr(u16)] +101 | | enum Unaligned2 { +102 | | A, +103 | | } | |_^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:108:1 + --> $DIR/enum.rs:106:1 | -108 | / #[repr(i16)] -109 | | enum Unaligned3 { -110 | | A, -111 | | } +106 | / #[repr(i16)] +107 | | enum Unaligned3 { +108 | | A, +109 | | } | |_^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:114:1 + --> $DIR/enum.rs:112:1 | -114 | / #[repr(u32)] -115 | | enum Unaligned4 { -116 | | A, -117 | | } +112 | / #[repr(u32)] +113 | | enum Unaligned4 { +114 | | A, +115 | | } | |_^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:120:1 + --> $DIR/enum.rs:118:1 | -120 | / #[repr(i32)] -121 | | enum Unaligned5 { -122 | | A, -123 | | } +118 | / #[repr(i32)] +119 | | enum Unaligned5 { +120 | | A, +121 | | } | |_^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:126:1 + --> $DIR/enum.rs:124:1 | -126 | / #[repr(u64)] -127 | | enum Unaligned6 { -128 | | A, -129 | | } +124 | / #[repr(u64)] +125 | | enum Unaligned6 { +126 | | A, +127 | | } | |_^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:132:1 + --> $DIR/enum.rs:130:1 | -132 | / #[repr(i64)] -133 | | enum Unaligned7 { -134 | | A, -135 | | } +130 | / #[repr(i64)] +131 | | enum Unaligned7 { +132 | | A, +133 | | } | |_^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:138:1 + --> $DIR/enum.rs:136:1 | -138 | / #[repr(usize)] -139 | | enum Unaligned8 { -140 | | A, -141 | | } +136 | / #[repr(usize)] +137 | | enum Unaligned8 { +138 | | A, +139 | | } | |_^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:144:1 + --> $DIR/enum.rs:142:1 | -144 | / #[repr(isize)] -145 | | enum Unaligned9 { -146 | | A, -147 | | } +142 | / #[repr(isize)] +143 | | enum Unaligned9 { +144 | | A, +145 | | } | |_^ error: cannot derive Unaligned with repr(align(N > 1)) - --> $DIR/enum.rs:150:1 + --> $DIR/enum.rs:148:1 | -150 | / #[repr(u8, align(2))] -151 | | enum Unaligned10 { -152 | | A, -153 | | } +148 | / #[repr(u8, align(2))] +149 | | enum Unaligned10 { +150 | | A, +151 | | } | |_^ error: cannot derive Unaligned with repr(align(N > 1)) - --> $DIR/enum.rs:156:1 + --> $DIR/enum.rs:154:1 | -156 | / #[repr(i8, align(2))] -157 | | enum Unaligned11 { -158 | | A, -159 | | } +154 | / #[repr(i8, align(2))] +155 | | enum Unaligned11 { +156 | | A, +157 | | } | |_^ error[E0565]: meta item in `repr` must be an identifier - --> $DIR/enum.rs:17:8 + --> $DIR/enum.rs:15:8 | -17 | #[repr("foo")] +15 | #[repr("foo")] | ^^^^^ error[E0552]: unrecognized representation hint - --> $DIR/enum.rs:23:8 + --> $DIR/enum.rs:21:8 | -23 | #[repr(foo)] +21 | #[repr(foo)] | ^^^ warning[E0566]: conflicting representation hints - --> $DIR/enum.rs:35:8 + --> $DIR/enum.rs:33:8 | -35 | #[repr(u8, u16)] +33 | #[repr(u8, u16)] | ^^ ^^^ error: aborting due to 25 previous errors -Some errors have detailed explanations: E0552, E0565. +Some errors have detailed explanations: E0552, E0565, E0566. For more information about an error, try `rustc --explain E0552`. From 11c47ee0c3697754054bdd2ba680fec0eb413a9b Mon Sep 17 00:00:00 2001 From: Rabi Guha Date: Thu, 12 Mar 2020 05:04:04 +0000 Subject: [PATCH 03/12] [rust][zerocopy] Make zerocopy-derive errors point to the specific attr Instead of pointing to the entire input (enum or struct) on which the derive attribute is applied, pinpoint the error to the exact attribute meta-item which caused the error. Change-Id: Ia97e92851f76c304a5ce2aea3d6ca8411474280d --- zerocopy-derive/src/repr.rs | 59 ++++-- zerocopy-derive/tests/ui/enum.rs | 12 ++ zerocopy-derive/tests/ui/enum.stderr | 191 +++++++----------- .../tests/ui/late_compile_pass.stderr | 36 ++-- zerocopy-derive/tests/ui/struct.rs | 8 + zerocopy-derive/tests/ui/struct.stderr | 40 ++-- 6 files changed, 170 insertions(+), 176 deletions(-) diff --git a/zerocopy-derive/src/repr.rs b/zerocopy-derive/src/repr.rs index 9dff0523ab..0868e3e4a3 100644 --- a/zerocopy-derive/src/repr.rs +++ b/zerocopy-derive/src/repr.rs @@ -5,6 +5,7 @@ use core::fmt::{self, Display, Formatter}; use proc_macro2::Span; +use syn::spanned::Spanned; use syn::{Attribute, DeriveInput, Error, Lit, Meta, NestedMeta}; pub struct Config { @@ -39,33 +40,51 @@ impl Config { /// whether `align` attributes are considered during validation, they are /// stripped out of the returned value since no callers care about them. pub fn validate_reprs(&self, input: &DeriveInput) -> Result, Vec> { - let mut reprs = reprs(&input.attrs)?; - reprs.sort(); + let mut metas_reprs = reprs(&input.attrs)?; + metas_reprs.sort_by(|a: &(NestedMeta, R), b| a.1.partial_cmp(&b.1).unwrap()); - if self.derive_unaligned && reprs.iter().any(KindRepr::is_align_gt_one) { - // TODO(joshlf): Have the span correspond just to the attributes - // instead of the entire input. - return Err(vec![Error::new_spanned( - input, - "cannot derive Unaligned with repr(align(N > 1))", - )]); + if self.derive_unaligned { + match metas_reprs.iter().find(|&repr: &&(NestedMeta, R)| repr.1.is_align_gt_one()) { + Some((meta, _)) => { + return Err(vec![Error::new_spanned( + meta, + "cannot derive Unaligned with repr(align(N > 1))", + )]) + } + None => (), + } } - reprs.retain(|repr: &R| !repr.is_align()); + + let mut metas = Vec::new(); + let mut reprs = Vec::new(); + metas_reprs.into_iter().filter(|(_, repr)| !repr.is_align()).for_each(|(meta, repr)| { + metas.push(meta); + reprs.push(repr) + }); if reprs.is_empty() { // Use Span::call_site to report this error on the #[derive(...)] // itself. - Err(vec![Error::new(Span::call_site(), "must have a non-align #[repr(...)] attribute in order to guarantee this type's memory layout")]) - } else if self.allowed_combinations.contains(&reprs.as_slice()) { + return Err(vec![Error::new(Span::call_site(), "must have a non-align #[repr(...)] attribute in order to guarantee this type's memory layout")]); + } + + let initial_sp = metas[0].span(); + let err_span = metas.iter().skip(1).fold(Some(initial_sp), |sp_option, meta| { + sp_option.and_then(|sp| sp.join(meta.span())) + }); + + if self.allowed_combinations.contains(&reprs.as_slice()) { Ok(reprs) } else if self.disallowed_but_legal_combinations.contains(&reprs.as_slice()) { - // TODO(joshlf): Have the span correspond just to the attributes - // instead of the entire input. - Err(vec![Error::new_spanned(input, self.allowed_combinations_message)]) + Err(vec![Error::new( + err_span.unwrap_or(input.span()), + self.allowed_combinations_message, + )]) } else { - // TODO(joshlf): Have the span correspond just to the attributes - // instead of the entire input. - Err(vec![Error::new_spanned(input, "conflicting representation hints")]) + Err(vec![Error::new( + err_span.unwrap_or(input.span()), + "conflicting representation hints", + )]) } } } @@ -227,7 +246,7 @@ impl Display for Repr { } } -fn reprs(attrs: &[Attribute]) -> Result, Vec> { +fn reprs(attrs: &[Attribute]) -> Result, Vec> { let mut reprs = Vec::new(); let mut errors = Vec::new(); for attr in attrs { @@ -240,7 +259,7 @@ fn reprs(attrs: &[Attribute]) -> Result, Vec> { if meta_list.path.is_ident("repr") { for nested_meta in &meta_list.nested { match R::parse(nested_meta) { - Ok(repr) => reprs.push(repr), + Ok(repr) => reprs.push((nested_meta.clone(), repr)), Err(err) => errors.push(err), } } diff --git a/zerocopy-derive/tests/ui/enum.rs b/zerocopy-derive/tests/ui/enum.rs index c340a07364..188b0a68d2 100644 --- a/zerocopy-derive/tests/ui/enum.rs +++ b/zerocopy-derive/tests/ui/enum.rs @@ -155,3 +155,15 @@ enum Unaligned10 { enum Unaligned11 { A, } + +#[derive(Unaligned)] +#[repr(align(1), align(2))] +enum Unaligned12 { + A, +} + +#[derive(Unaligned)] +#[repr(align(2), align(4))] +enum Unaligned13 { + A, +} diff --git a/zerocopy-derive/tests/ui/enum.stderr b/zerocopy-derive/tests/ui/enum.stderr index b7679e47ad..7ee50c35ff 100644 --- a/zerocopy-derive/tests/ui/enum.stderr +++ b/zerocopy-derive/tests/ui/enum.stderr @@ -17,13 +17,10 @@ error: unsupported representation for deriving FromBytes, AsBytes, or Unaligned | ^^^^^^^^^^^ error: conflicting representation hints - --> $DIR/enum.rs:33:1 + --> $DIR/enum.rs:33:8 | -33 | / #[repr(u8, u16)] -34 | | enum Generic4 { -35 | | A, -36 | | } - | |_^ +33 | #[repr(u8, u16)] + | ^^^^^^^ error: must have a non-align #[repr(...)] attribute in order to guarantee this type's memory layout --> $DIR/enum.rs:38:10 @@ -32,166 +29,124 @@ error: must have a non-align #[repr(...)] attribute in order to guarantee this t | ^^^^^^^^^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:48:1 + --> $DIR/enum.rs:48:8 | -48 | / #[repr(C)] -49 | | enum FromBytes1 { -50 | | A, -51 | | } - | |_^ +48 | #[repr(C)] + | ^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:54:1 + --> $DIR/enum.rs:54:8 | -54 | / #[repr(usize)] -55 | | enum FromBytes2 { -56 | | A, -57 | | } - | |_^ +54 | #[repr(usize)] + | ^^^^^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:60:1 + --> $DIR/enum.rs:60:8 | -60 | / #[repr(isize)] -61 | | enum FromBytes3 { -62 | | A, -63 | | } - | |_^ +60 | #[repr(isize)] + | ^^^^^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:66:1 + --> $DIR/enum.rs:66:8 | -66 | / #[repr(u32)] -67 | | enum FromBytes4 { -68 | | A, -69 | | } - | |_^ +66 | #[repr(u32)] + | ^^^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:72:1 + --> $DIR/enum.rs:72:8 | -72 | / #[repr(i32)] -73 | | enum FromBytes5 { -74 | | A, -75 | | } - | |_^ +72 | #[repr(i32)] + | ^^^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:78:1 + --> $DIR/enum.rs:78:8 | -78 | / #[repr(u64)] -79 | | enum FromBytes6 { -80 | | A, -81 | | } - | |_^ +78 | #[repr(u64)] + | ^^^ error: FromBytes requires repr of "u8", "u16", "i8", or "i16" - --> $DIR/enum.rs:84:1 + --> $DIR/enum.rs:84:8 | -84 | / #[repr(i64)] -85 | | enum FromBytes7 { -86 | | A, -87 | | } - | |_^ +84 | #[repr(i64)] + | ^^^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:94:1 + --> $DIR/enum.rs:94:8 | -94 | / #[repr(C)] -95 | | enum Unaligned1 { -96 | | A, -97 | | } - | |_^ +94 | #[repr(C)] + | ^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:100:1 + --> $DIR/enum.rs:100:8 | -100 | / #[repr(u16)] -101 | | enum Unaligned2 { -102 | | A, -103 | | } - | |_^ +100 | #[repr(u16)] + | ^^^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:106:1 + --> $DIR/enum.rs:106:8 | -106 | / #[repr(i16)] -107 | | enum Unaligned3 { -108 | | A, -109 | | } - | |_^ +106 | #[repr(i16)] + | ^^^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:112:1 + --> $DIR/enum.rs:112:8 | -112 | / #[repr(u32)] -113 | | enum Unaligned4 { -114 | | A, -115 | | } - | |_^ +112 | #[repr(u32)] + | ^^^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:118:1 + --> $DIR/enum.rs:118:8 | -118 | / #[repr(i32)] -119 | | enum Unaligned5 { -120 | | A, -121 | | } - | |_^ +118 | #[repr(i32)] + | ^^^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:124:1 + --> $DIR/enum.rs:124:8 | -124 | / #[repr(u64)] -125 | | enum Unaligned6 { -126 | | A, -127 | | } - | |_^ +124 | #[repr(u64)] + | ^^^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:130:1 + --> $DIR/enum.rs:130:8 | -130 | / #[repr(i64)] -131 | | enum Unaligned7 { -132 | | A, -133 | | } - | |_^ +130 | #[repr(i64)] + | ^^^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:136:1 + --> $DIR/enum.rs:136:8 | -136 | / #[repr(usize)] -137 | | enum Unaligned8 { -138 | | A, -139 | | } - | |_^ +136 | #[repr(usize)] + | ^^^^^ error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1))) - --> $DIR/enum.rs:142:1 + --> $DIR/enum.rs:142:8 | -142 | / #[repr(isize)] -143 | | enum Unaligned9 { -144 | | A, -145 | | } - | |_^ +142 | #[repr(isize)] + | ^^^^^ error: cannot derive Unaligned with repr(align(N > 1)) - --> $DIR/enum.rs:148:1 + --> $DIR/enum.rs:148:12 | -148 | / #[repr(u8, align(2))] -149 | | enum Unaligned10 { -150 | | A, -151 | | } - | |_^ +148 | #[repr(u8, align(2))] + | ^^^^^^^^ error: cannot derive Unaligned with repr(align(N > 1)) - --> $DIR/enum.rs:154:1 + --> $DIR/enum.rs:154:12 | -154 | / #[repr(i8, align(2))] -155 | | enum Unaligned11 { -156 | | A, -157 | | } - | |_^ +154 | #[repr(i8, align(2))] + | ^^^^^^^^ + +error: cannot derive Unaligned with repr(align(N > 1)) + --> $DIR/enum.rs:160:18 + | +160 | #[repr(align(1), align(2))] + | ^^^^^^^^ + +error: cannot derive Unaligned with repr(align(N > 1)) + --> $DIR/enum.rs:166:8 + | +166 | #[repr(align(2), align(4))] + | ^^^^^^^^ error[E0565]: meta item in `repr` must be an identifier --> $DIR/enum.rs:15:8 @@ -205,13 +160,17 @@ error[E0552]: unrecognized representation hint 21 | #[repr(foo)] | ^^^ -warning[E0566]: conflicting representation hints +error[E0566]: conflicting representation hints --> $DIR/enum.rs:33:8 | 33 | #[repr(u8, u16)] | ^^ ^^^ + | + = note: `#[deny(conflicting_repr_hints)]` on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #68585 -error: aborting due to 25 previous errors +error: aborting due to 28 previous errors Some errors have detailed explanations: E0552, E0565, E0566. For more information about an error, try `rustc --explain E0552`. diff --git a/zerocopy-derive/tests/ui/late_compile_pass.stderr b/zerocopy-derive/tests/ui/late_compile_pass.stderr index ee4ae4583e..7531579210 100644 --- a/zerocopy-derive/tests/ui/late_compile_pass.stderr +++ b/zerocopy-derive/tests/ui/late_compile_pass.stderr @@ -1,14 +1,11 @@ error[E0277]: the trait bound `&'static str: zerocopy::FromBytes` is not satisfied --> $DIR/late_compile_pass.rs:18:10 | -18 | #[derive(FromBytes)] - | ^^^^^^^^^ the trait `zerocopy::FromBytes` is not implemented for `&'static str` - | -note: required by `::only_derive_is_allowed_to_implement_this_trait::ImplementsFromBytes` - --> $DIR/late_compile_pass.rs:18:10 - | 18 | #[derive(FromBytes)] | ^^^^^^^^^ + | | + | the trait `zerocopy::FromBytes` is not implemented for `&'static str` + | required by `::only_derive_is_allowed_to_implement_this_trait::ImplementsFromBytes` error[E0080]: evaluation of constant value failed --> $DIR/late_compile_pass.rs:27:10 @@ -19,38 +16,29 @@ error[E0080]: evaluation of constant value failed error[E0277]: the trait bound `u16: zerocopy::Unaligned` is not satisfied --> $DIR/late_compile_pass.rs:39:10 | -39 | #[derive(Unaligned)] - | ^^^^^^^^^ the trait `zerocopy::Unaligned` is not implemented for `u16` - | -note: required by `::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned` - --> $DIR/late_compile_pass.rs:39:10 - | 39 | #[derive(Unaligned)] | ^^^^^^^^^ + | | + | the trait `zerocopy::Unaligned` is not implemented for `u16` + | required by `::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned` error[E0277]: the trait bound `u16: zerocopy::Unaligned` is not satisfied --> $DIR/late_compile_pass.rs:47:10 | -47 | #[derive(Unaligned)] - | ^^^^^^^^^ the trait `zerocopy::Unaligned` is not implemented for `u16` - | -note: required by `::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned` - --> $DIR/late_compile_pass.rs:47:10 - | 47 | #[derive(Unaligned)] | ^^^^^^^^^ + | | + | the trait `zerocopy::Unaligned` is not implemented for `u16` + | required by `::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned` error[E0277]: the trait bound `u16: zerocopy::Unaligned` is not satisfied --> $DIR/late_compile_pass.rs:54:10 | -54 | #[derive(Unaligned)] - | ^^^^^^^^^ the trait `zerocopy::Unaligned` is not implemented for `u16` - | -note: required by `::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned` - --> $DIR/late_compile_pass.rs:54:10 - | 54 | #[derive(Unaligned)] | ^^^^^^^^^ + | | + | the trait `zerocopy::Unaligned` is not implemented for `u16` + | required by `::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned` error: aborting due to 5 previous errors diff --git a/zerocopy-derive/tests/ui/struct.rs b/zerocopy-derive/tests/ui/struct.rs index 331eacdeb6..ee8fd0637d 100644 --- a/zerocopy-derive/tests/ui/struct.rs +++ b/zerocopy-derive/tests/ui/struct.rs @@ -32,3 +32,11 @@ struct Unaligned2 { #[derive(Unaligned)] #[repr(packed, align(2))] struct Unaligned3; + +#[derive(Unaligned)] +#[repr(align(1), align(2))] +struct Unaligned4; + +#[derive(Unaligned)] +#[repr(align(2), align(4))] +struct Unaligned5; diff --git a/zerocopy-derive/tests/ui/struct.stderr b/zerocopy-derive/tests/ui/struct.stderr index 117ea3c986..8cdb703e46 100644 --- a/zerocopy-derive/tests/ui/struct.stderr +++ b/zerocopy-derive/tests/ui/struct.stderr @@ -5,27 +5,34 @@ error: unsupported on types with type parameters | ^^^^^^^ error: cannot derive Unaligned with repr(align(N > 1)) - --> $DIR/struct.rs:23:1 + --> $DIR/struct.rs:23:11 | -23 | / #[repr(C, align(2))] -24 | | struct Unaligned1; - | |__________________^ +23 | #[repr(C, align(2))] + | ^^^^^^^^ error: cannot derive Unaligned with repr(align(N > 1)) - --> $DIR/struct.rs:27:1 + --> $DIR/struct.rs:27:21 | -27 | / #[repr(transparent, align(2))] -28 | | struct Unaligned2 { -29 | | foo: u8, -30 | | } - | |_^ +27 | #[repr(transparent, align(2))] + | ^^^^^^^^ + +error: cannot derive Unaligned with repr(align(N > 1)) + --> $DIR/struct.rs:33:16 + | +33 | #[repr(packed, align(2))] + | ^^^^^^^^ + +error: cannot derive Unaligned with repr(align(N > 1)) + --> $DIR/struct.rs:37:18 + | +37 | #[repr(align(1), align(2))] + | ^^^^^^^^ error: cannot derive Unaligned with repr(align(N > 1)) - --> $DIR/struct.rs:33:1 + --> $DIR/struct.rs:41:8 | -33 | / #[repr(packed, align(2))] -34 | | struct Unaligned3; - | |__________________^ +41 | #[repr(align(2), align(4))] + | ^^^^^^^^ error[E0692]: transparent struct cannot have other repr hints --> $DIR/struct.rs:27:8 @@ -39,6 +46,7 @@ error[E0587]: type has conflicting packed and align representation hints 34 | struct Unaligned3; | ^^^^^^^^^^^^^^^^^^ -error: aborting due to 6 previous errors +error: aborting due to 8 previous errors -For more information about this error, try `rustc --explain E0692`. +Some errors have detailed explanations: E0587, E0692. +For more information about an error, try `rustc --explain E0587`. From ccaef63011468d7ddf5c094286ea725f5975ae90 Mon Sep 17 00:00:00 2001 From: Marc Khouri Date: Thu, 7 May 2020 23:15:11 +0000 Subject: [PATCH 04/12] [zerocopy] Update clippy lint name Getting rid of a warning I noticed in my editor: ``` lint `clippy::cyclomatic_complexity` has been renamed to `clippy::cognitive_complexity` note: `#[warn(renamed_and_removed_lints)]` on by default help: use the new name: `clippy::cognitive_complexity` ``` Test: no behavior change Change-Id: Ic1738deed93f8c7e59a86247a1c335bcf2430b4f Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/387256 Reviewed-by: Joshua Liebow-Feeser Testability-Review: Joshua Liebow-Feeser Commit-Queue: Marc Khouri --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index f14a27be72..ba7d518e65 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1447,7 +1447,7 @@ mod tests { } #[test] - #[allow(clippy::cyclomatic_complexity)] + #[allow(clippy::cognitive_complexity)] fn test_new_error() { // fail because the buffer is too large From 19b613d8ed9f28ae29a4726038038773d4c6b7d5 Mon Sep 17 00:00:00 2001 From: Miguel Young de la Sota Date: Mon, 6 Jul 2020 17:16:46 +0000 Subject: [PATCH 05/12] [zerocopy] Add LayoutVerified::new_slice_from_prefix and friends Add variants of `new_slice` for creating a LayoutVerified<[T]> out of the first N or last N putative `T`s in a byte slice. Change-Id: If827981377b80e2d755a0023e48b63bc8f75e2c9 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/394715 Commit-Queue: Miguel Young de la Sota Reviewed-by: Joshua Liebow-Feeser Testability-Review: Joshua Liebow-Feeser --- src/lib.rs | 460 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 403 insertions(+), 57 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ba7d518e65..6681110b17 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -457,6 +457,56 @@ where } Some(LayoutVerified(bytes, PhantomData)) } + + /// Construct a new `LayoutVerified` of a slice type from the prefix of a byte slice. + /// + /// `new_slice_from_prefix` verifies that `bytes.len() >= size_of::() * count` + /// and that `bytes` is aligned to `align_of::()`. It consumes the first + /// `size_of::() * count` bytes from `bytes` to construct a `LayoutVerified`, and + /// returns the remaining bytes to the caller. It also ensures that + /// `sizeof::() * count` does not overflow a `usize`. If any of the length, + /// alignment, or overflow checks fail, it returns `None`. + /// + /// # Panics + /// + /// `new_slice_from_prefix` panics if `T` is a zero-sized type. + #[inline] + pub fn new_slice_from_prefix(bytes: B, count: usize) -> Option<(LayoutVerified, B)> { + let expected_len = match mem::size_of::().checked_mul(count) { + Some(len) => len, + None => return None, + }; + if bytes.len() < expected_len { + return None; + } + let (prefix, bytes) = bytes.split_at(expected_len); + Self::new_slice(prefix).map(move |l| (l, bytes)) + } + + /// Construct a new `LayoutVerified` of a slice type from the suffix of a byte slice. + /// + /// `new_slice_from_suffix` verifies that `bytes.len() >= size_of::() * count` + /// and that `bytes` is aligned to `align_of::()`. It consumes the last + /// `size_of::() * count` bytes from `bytes` to construct a `LayoutVerified`, and + /// returns the preceding bytes to the caller. It also ensures that + /// `sizeof::() * count` does not overflow a `usize`. If any of the length, + /// alignment, or overflow checks fail, it returns `None`. + /// + /// # Panics + /// + /// `new_slice_from_suffix` panics if `T` is a zero-sized type. + #[inline] + pub fn new_slice_from_suffix(bytes: B, count: usize) -> Option<(B, LayoutVerified)> { + let expected_len = match mem::size_of::().checked_mul(count) { + Some(len) => len, + None => return None, + }; + if bytes.len() < expected_len { + return None; + } + let (bytes, suffix) = bytes.split_at(expected_len); + Self::new_slice(suffix).map(move |l| (bytes, l)) + } } fn map_zeroed( @@ -570,6 +620,56 @@ where pub fn new_slice_zeroed(bytes: B) -> Option> { map_zeroed(Self::new_slice(bytes)) } + + /// Construct a new `LayoutVerified` of a slice type from the prefix of a byte slice, + /// after zeroing the bytes. + /// + /// `new_slice_from_prefix` verifies that `bytes.len() >= size_of::() * count` + /// and that `bytes` is aligned to `align_of::()`. It consumes the first + /// `size_of::() * count` bytes from `bytes` to construct a `LayoutVerified`, and + /// returns the remaining bytes to the caller. It also ensures that + /// `sizeof::() * count` does not overflow a `usize`. If any of the length, + /// alignment, or overflow checks fail, it returns `None`. + /// + /// If the checks succeed, then the suffix which is consumed will be + /// initialized to zero. This can be useful when re-using buffers to ensure + /// that sensitive data previously stored in the buffer is not leaked. + /// + /// # Panics + /// + /// `new_slice_from_prefix_zeroed` panics if `T` is a zero-sized type. + #[inline] + pub fn new_slice_from_prefix_zeroed( + bytes: B, + count: usize, + ) -> Option<(LayoutVerified, B)> { + map_prefix_tuple_zeroed(Self::new_slice_from_prefix(bytes, count)) + } + + /// Construct a new `LayoutVerified` of a slice type from the prefix of a byte slice, + /// after zeroing the bytes. + /// + /// `new_slice_from_suffix` verifies that `bytes.len() >= size_of::() * count` + /// and that `bytes` is aligned to `align_of::()`. It consumes the last + /// `size_of::() * count` bytes from `bytes` to construct a `LayoutVerified`, and + /// returns the preceding bytes to the caller. It also ensures that + /// `sizeof::() * count` does not overflow a `usize`. If any of the length, + /// alignment, or overflow checks fail, it returns `None`. + /// + /// If the checks succeed, then the consumed suffix will be initialized to zero. This + /// can be useful when re-using buffers to ensure that sensitive data + /// previously stored in the buffer is not leaked. + /// + /// # Panics + /// + /// `new_slice_from_suffix_zeroed` panics if `T` is a zero-sized type. + #[inline] + pub fn new_slice_from_suffix_zeroed( + bytes: B, + count: usize, + ) -> Option<(B, LayoutVerified)> { + map_suffix_tuple_zeroed(Self::new_slice_from_suffix(bytes, count)) + } } impl LayoutVerified @@ -648,6 +748,62 @@ where } Some(LayoutVerified(bytes, PhantomData)) } + + /// Construct a new `LayoutVerified` of a slice type with no alignment requirement + /// from the prefix of a byte slice. + /// + /// `new_slice_from_prefix` verifies that `bytes.len() >= size_of::() * count`. + /// It consumes the first `size_of::() * count` bytes from `bytes` to construct + /// a `LayoutVerified`, and returns the remaining bytes to the caller. It also + /// ensures that `sizeof::() * count` does not overflow a `usize`. If either the + /// length, or overflow checks fail, it returns `None`. + /// + /// # Panics + /// + /// `new_slice_unaligned_from_prefix` panics if `T` is a zero-sized type. + #[inline] + pub fn new_slice_unaligned_from_prefix( + bytes: B, + count: usize, + ) -> Option<(LayoutVerified, B)> { + let expected_len = match mem::size_of::().checked_mul(count) { + Some(len) => len, + None => return None, + }; + if bytes.len() < expected_len { + return None; + } + let (prefix, bytes) = bytes.split_at(expected_len); + Self::new_slice_unaligned(prefix).map(move |l| (l, bytes)) + } + + /// Construct a new `LayoutVerified` of a slice type with no alignment requirement + /// from the suffix of a byte slice. + /// + /// `new_slice_from_suffix` verifies that `bytes.len() >= size_of::() * count`. + /// It consumes the last `size_of::() * count` bytes from `bytes` to construct + /// a `LayoutVerified`, and returns the remaining bytes to the caller. It also + /// ensures that `sizeof::() * count` does not overflow a `usize`. If either the + /// length, or overflow checks fail, it returns `None`. + /// + /// # Panics + /// + /// `new_slice_unaligned_from_suffix` panics if `T` is a zero-sized type. + #[inline] + pub fn new_slice_unaligned_from_suffix( + bytes: B, + count: usize, + ) -> Option<(B, LayoutVerified)> { + let expected_len = match mem::size_of::().checked_mul(count) { + Some(len) => len, + None => return None, + }; + if bytes.len() < expected_len { + return None; + } + let (bytes, suffix) = bytes.split_at(expected_len); + Self::new_slice_unaligned(suffix).map(move |l| (bytes, l)) + } } impl LayoutVerified @@ -726,6 +882,54 @@ where pub fn new_slice_unaligned_zeroed(bytes: B) -> Option> { map_zeroed(Self::new_slice_unaligned(bytes)) } + + /// Construct a new `LayoutVerified` of a slice type with no alignment requirement + /// from the prefix of a byte slice, after zeroing the bytes. + /// + /// `new_slice_from_prefix` verifies that `bytes.len() >= size_of::() * count`. + /// It consumes the first `size_of::() * count` bytes from `bytes` to construct + /// a `LayoutVerified`, and returns the remaining bytes to the caller. It also + /// ensures that `sizeof::() * count` does not overflow a `usize`. If either the + /// length, or overflow checks fail, it returns `None`. + /// + /// If the checks succeed, then the prefix will be initialized to zero. This + /// can be useful when re-using buffers to ensure that sensitive data + /// previously stored in the buffer is not leaked. + /// + /// # Panics + /// + /// `new_slice_unaligned_from_prefix_zeroed` panics if `T` is a zero-sized type. + #[inline] + pub fn new_slice_unaligned_from_prefix_zeroed( + bytes: B, + count: usize, + ) -> Option<(LayoutVerified, B)> { + map_prefix_tuple_zeroed(Self::new_slice_unaligned_from_prefix(bytes, count)) + } + + /// Construct a new `LayoutVerified` of a slice type with no alignment requirement + /// from the suffix of a byte slice, after zeroing the bytes. + /// + /// `new_slice_from_suffix` verifies that `bytes.len() >= size_of::() * count`. + /// It consumes the last `size_of::() * count` bytes from `bytes` to construct + /// a `LayoutVerified`, and returns the remaining bytes to the caller. It also + /// ensures that `sizeof::() * count` does not overflow a `usize`. If either the + /// length, or overflow checks fail, it returns `None`. + /// + /// If the checks succeed, then the suffix will be initialized to zero. This + /// can be useful when re-using buffers to ensure that sensitive data + /// previously stored in the buffer is not leaked. + /// + /// # Panics + /// + /// `new_slice_unaligned_from_suffix_zeroed` panics if `T` is a zero-sized type. + #[inline] + pub fn new_slice_unaligned_from_suffix_zeroed( + bytes: B, + count: usize, + ) -> Option<(B, LayoutVerified)> { + map_suffix_tuple_zeroed(Self::new_slice_unaligned_from_suffix(bytes, count)) + } } impl<'a, B, T> LayoutVerified @@ -1136,11 +1340,6 @@ mod tests { unsafe { ptr::read(&u as *const u64 as *const [u8; 8]) } } - // convert a u128 to bytes using this platform's endianness - fn u128_to_bytes(u: u128) -> [u8; 16] { - unsafe { ptr::read(&u as *const u128 as *const [u8; 16]) } - } - #[test] fn test_address() { // test that the Deref and DerefMut implementations return a reference which @@ -1180,25 +1379,29 @@ mod tests { // verify that values written to a LayoutVerified are properly shared // between the typed and untyped representations; pass a value with - // byte length 16/typed length 2 - fn test_new_helper_slice<'a>(mut lv: LayoutVerified<&'a mut [u8], [u64]>) { - // assert that the value starts at [0, 0] - assert_eq!(*lv, [0, 0]); + // `typed_len` `u64`s backed by an array of `typed_len * 8` bytes. + fn test_new_helper_slice<'a>(mut lv: LayoutVerified<&'a mut [u8], [u64]>, typed_len: usize) { + // assert that the value starts out zeroed + assert_eq!(&*lv, vec![0; typed_len].as_slice()); + + // check the backing storage is the exact same slice + let untyped_len = typed_len * 8; + assert_eq!(lv.bytes().len(), untyped_len); + assert_eq!(lv.bytes().as_ptr(), lv.as_ptr() as *const u8); // assert that values written to the typed value are reflected in the // byte slice const VAL1: u64 = 0xFF00FF00FF00FF00; - const VAL1_DOUBLED: u128 = 0xFF00FF00FF00FF00FF00FF00FF00FF00; - lv[0] = VAL1; - lv[1] = VAL1; - assert_eq!(lv.bytes(), &u128_to_bytes(VAL1_DOUBLED)); + for typed in &mut *lv { + *typed = VAL1; + } + assert_eq!(lv.bytes(), VAL1.to_ne_bytes().repeat(typed_len).as_slice()); // assert that values written to the byte slice are reflected in the // typed value const VAL2: u64 = !VAL1; // different from VAL1 - const VAL2_DOUBLED: u128 = !VAL1_DOUBLED; - lv.bytes_mut().copy_from_slice(&u128_to_bytes(VAL2_DOUBLED)[..]); - assert_eq!(*lv, [VAL2, VAL2]); + lv.bytes_mut().copy_from_slice(&VAL2.to_ne_bytes().repeat(typed_len)); + assert!(lv.iter().copied().all(|x| x == VAL2)); } // verify that values written to a LayoutVerified are properly shared @@ -1222,28 +1425,28 @@ mod tests { // verify that values written to a LayoutVerified are properly shared // between the typed and untyped representations; pass a value with - // length 16 - fn test_new_helper_slice_unaligned<'a>(mut lv: LayoutVerified<&'a mut [u8], [u8]>) { - // assert that the value starts at [0; 16] - assert_eq!(*lv, [0u8; 16][..]); + // `len` `u8`s backed by an array of `len` bytes. + fn test_new_helper_slice_unaligned<'a>(mut lv: LayoutVerified<&'a mut [u8], [u8]>, len: usize) { + // assert that the value starts out zeroed + assert_eq!(&*lv, vec![0u8; len].as_slice()); + + // check the backing storage is the exact same slice + assert_eq!(lv.bytes().len(), len); + assert_eq!(lv.bytes().as_ptr(), lv.as_ptr()); // assert that values written to the typed value are reflected in the // byte slice - const VAL1: [u8; 16] = [ - 0xFF, 0x00, 0xFF, 0x00, 0xFF, 0x00, 0xFF, 0x00, 0xFF, 0x00, 0xFF, 0x00, 0xFF, 0x00, - 0xFF, 0x00, - ]; - lv.copy_from_slice(&VAL1[..]); - assert_eq!(lv.bytes(), &VAL1); + let mut expected_bytes = [0xFF, 0x00].iter().copied().cycle().take(len).collect::>(); + lv.copy_from_slice(&expected_bytes); + assert_eq!(lv.bytes(), expected_bytes.as_slice()); // assert that values written to the byte slice are reflected in the // typed value - const VAL2: [u8; 16] = [ - 0x00, 0xFF, 0x00, 0xFF, 0x00, 0xFF, 0x00, 0xFF, 0x00, 0xFF, 0x00, 0xFF, 0x00, 0xFF, - 0x00, 0xFF, - ]; - lv.bytes_mut().copy_from_slice(&VAL2[..]); - assert_eq!(*lv, VAL2); + for byte in &mut expected_bytes { + *byte = !*byte; // different from expected_len + } + lv.bytes_mut().copy_from_slice(&expected_bytes); + assert_eq!(&*lv, expected_bytes.as_slice()); } #[test] @@ -1293,11 +1496,43 @@ mod tests { let mut buf = AlignedBuffer::::default(); // buf.buf should be aligned to 8 and have a length which is a multiple // of size_of::(), so this should always succeed - test_new_helper_slice(LayoutVerified::<_, [u64]>::new_slice(&mut buf.buf[..]).unwrap()); + test_new_helper_slice(LayoutVerified::<_, [u64]>::new_slice(&mut buf.buf[..]).unwrap(), 2); buf.buf = [0xFFu8; 16]; test_new_helper_slice( LayoutVerified::<_, [u64]>::new_slice_zeroed(&mut buf.buf[..]).unwrap(), + 2, ); + + { + buf.clear_buf(); + let (lv, suffix) = + LayoutVerified::<_, [u64]>::new_slice_from_prefix(&mut buf.buf[..], 1).unwrap(); + assert_eq!(suffix, [0; 8]); + test_new_helper_slice(lv, 1); + } + { + buf.buf = [0xFFu8; 16]; + let (lv, suffix) = + LayoutVerified::<_, [u64]>::new_slice_from_prefix_zeroed(&mut buf.buf[..], 1) + .unwrap(); + assert_eq!(suffix, [0xFF; 8]); + test_new_helper_slice(lv, 1); + } + { + buf.clear_buf(); + let (prefix, lv) = + LayoutVerified::<_, [u64]>::new_slice_from_suffix(&mut buf.buf[..], 1).unwrap(); + assert_eq!(prefix, [0; 8]); + test_new_helper_slice(lv, 1); + } + { + buf.buf = [0xFFu8; 16]; + let (prefix, lv) = + LayoutVerified::<_, [u64]>::new_slice_from_suffix_zeroed(&mut buf.buf[..], 1) + .unwrap(); + assert_eq!(prefix, [0xFF; 8]); + test_new_helper_slice(lv, 1); + } } #[test] @@ -1354,12 +1589,47 @@ mod tests { // buf.buf should be aligned to 8 and have a length which is a multiple // of size_of::(), so this should always succeed test_new_helper_slice_unaligned( - LayoutVerified::<_, [u8]>::new_slice(&mut buf[..]).unwrap(), + LayoutVerified::<_, [u8]>::new_slice_unaligned(&mut buf[..]).unwrap(), + 16, ); buf = [0xFFu8; 16]; test_new_helper_slice_unaligned( - LayoutVerified::<_, [u8]>::new_slice_zeroed(&mut buf[..]).unwrap(), + LayoutVerified::<_, [u8]>::new_slice_unaligned_zeroed(&mut buf[..]).unwrap(), + 16, ); + + { + buf = [0u8; 16]; + let (lv, suffix) = + LayoutVerified::<_, [u8]>::new_slice_unaligned_from_prefix(&mut buf[..], 8) + .unwrap(); + assert_eq!(suffix, [0; 8]); + test_new_helper_slice_unaligned(lv, 8); + } + { + buf = [0xFFu8; 16]; + let (lv, suffix) = + LayoutVerified::<_, [u8]>::new_slice_unaligned_from_prefix_zeroed(&mut buf[..], 8) + .unwrap(); + assert_eq!(suffix, [0xFF; 8]); + test_new_helper_slice_unaligned(lv, 8); + } + { + buf = [0u8; 16]; + let (prefix, lv) = + LayoutVerified::<_, [u8]>::new_slice_unaligned_from_suffix(&mut buf[..], 8) + .unwrap(); + assert_eq!(prefix, [0; 8]); + test_new_helper_slice_unaligned(lv, 8); + } + { + buf = [0xFFu8; 16]; + let (prefix, lv) = + LayoutVerified::<_, [u8]>::new_slice_unaligned_from_suffix_zeroed(&mut buf[..], 8) + .unwrap(); + assert_eq!(prefix, [0xFF; 8]); + test_new_helper_slice_unaligned(lv, 8); + } } #[test] @@ -1490,6 +1760,32 @@ mod tests { LayoutVerified::<_, [[u8; 8]]>::new_slice_unaligned_zeroed(&mut buf.buf[..]).is_none() ); + // fail beacuse the buffer is too short. + let mut buf = AlignedBuffer::::default(); + // buf.buf has length 12, but the element size is 8 (and we're expecting two of them). + assert!(LayoutVerified::<_, [u64]>::new_slice_from_prefix(&buf.buf[..], 2).is_none()); + assert!( + LayoutVerified::<_, [u64]>::new_slice_from_prefix_zeroed(&mut buf.buf[..], 2).is_none() + ); + assert!(LayoutVerified::<_, [u64]>::new_slice_from_suffix(&buf.buf[..], 2).is_none()); + assert!( + LayoutVerified::<_, [u64]>::new_slice_from_suffix_zeroed(&mut buf.buf[..], 2).is_none() + ); + assert!(LayoutVerified::<_, [[u8; 8]]>::new_slice_unaligned_from_prefix(&buf.buf[..], 2) + .is_none()); + assert!(LayoutVerified::<_, [[u8; 8]]>::new_slice_unaligned_from_prefix_zeroed( + &mut buf.buf[..], + 2 + ) + .is_none()); + assert!(LayoutVerified::<_, [[u8; 8]]>::new_slice_unaligned_from_suffix(&buf.buf[..], 2) + .is_none()); + assert!(LayoutVerified::<_, [[u8; 8]]>::new_slice_unaligned_from_suffix_zeroed( + &mut buf.buf[..], + 2 + ) + .is_none()); + // fail because the alignment is insufficient // a buffer with an alignment of 8 @@ -1502,34 +1798,84 @@ mod tests { assert!(LayoutVerified::<_, u64>::new_from_prefix_zeroed(&mut buf.buf[4..]).is_none()); assert!(LayoutVerified::<_, [u64]>::new_slice(&buf.buf[4..]).is_none()); assert!(LayoutVerified::<_, [u64]>::new_slice_zeroed(&mut buf.buf[4..]).is_none()); + assert!(LayoutVerified::<_, [u64]>::new_slice_from_prefix(&buf.buf[4..], 1).is_none()); + assert!(LayoutVerified::<_, [u64]>::new_slice_from_prefix_zeroed(&mut buf.buf[4..], 1) + .is_none()); + assert!(LayoutVerified::<_, [u64]>::new_slice_from_suffix(&buf.buf[4..], 1).is_none()); + assert!(LayoutVerified::<_, [u64]>::new_slice_from_suffix_zeroed(&mut buf.buf[4..], 1) + .is_none()); // slicing from 4 should be unnecessary because new_from_suffix[_zeroed] // use the suffix of the slice assert!(LayoutVerified::<_, u64>::new_from_suffix(&buf.buf[..]).is_none()); assert!(LayoutVerified::<_, u64>::new_from_suffix_zeroed(&mut buf.buf[..]).is_none()); - } - #[test] - #[should_panic] - fn test_new_slice_zst_panics() { - LayoutVerified::<_, [()]>::new_slice(&[0u8][..]); - } - - #[test] - #[should_panic] - fn test_new_slice_zeroed_zst_panics() { - LayoutVerified::<_, [()]>::new_slice_zeroed(&mut [0u8][..]); - } + // fail due to arithmetic overflow - #[test] - #[should_panic] - fn test_new_slice_unaligned_zst_panics() { - LayoutVerified::<_, [()]>::new_slice_unaligned(&[0u8][..]); - } - - #[test] - #[should_panic] - fn test_new_slice_unaligned_zeroed_zst_panics() { - LayoutVerified::<_, [()]>::new_slice_unaligned_zeroed(&mut [0u8][..]); + let mut buf = AlignedBuffer::::default(); + let unreasonable_len = std::usize::MAX / mem::size_of::() + 1; + assert!(LayoutVerified::<_, [u64]>::new_slice_from_prefix(&buf.buf[..], unreasonable_len) + .is_none()); + assert!(LayoutVerified::<_, [u64]>::new_slice_from_prefix_zeroed( + &mut buf.buf[..], + unreasonable_len + ) + .is_none()); + assert!(LayoutVerified::<_, [u64]>::new_slice_from_suffix(&buf.buf[..], unreasonable_len) + .is_none()); + assert!(LayoutVerified::<_, [u64]>::new_slice_from_suffix_zeroed( + &mut buf.buf[..], + unreasonable_len + ) + .is_none()); + assert!(LayoutVerified::<_, [[u8; 8]]>::new_slice_unaligned_from_prefix( + &buf.buf[..], + unreasonable_len + ) + .is_none()); + assert!(LayoutVerified::<_, [[u8; 8]]>::new_slice_unaligned_from_prefix_zeroed( + &mut buf.buf[..], + unreasonable_len + ) + .is_none()); + assert!(LayoutVerified::<_, [[u8; 8]]>::new_slice_unaligned_from_suffix( + &buf.buf[..], + unreasonable_len + ) + .is_none()); + assert!(LayoutVerified::<_, [[u8; 8]]>::new_slice_unaligned_from_suffix_zeroed( + &mut buf.buf[..], + unreasonable_len + ) + .is_none()); + } + + // Tests for ensuring that, if a ZST is passed into a slice-like function, we always + // panic. Since these tests need to be separate per-function, and they tend to take + // up a lot of space, we genrate them using a macro in a submodule instead. The + // submodule ensures that we can just re-use the name of the function under test for + // the name of the test itself. + mod test_zst_panics { + macro_rules! zst_test { + ($name:ident($($tt:tt)*)) => { + #[test] + #[should_panic] + fn $name() { + $crate::LayoutVerified::<_, [()]>::$name(&mut [0u8][..], $($tt)*); + } + } + } + zst_test!(new_slice()); + zst_test!(new_slice_zeroed()); + zst_test!(new_slice_from_prefix(1)); + zst_test!(new_slice_from_prefix_zeroed(1)); + zst_test!(new_slice_from_suffix(1)); + zst_test!(new_slice_from_suffix_zeroed(1)); + zst_test!(new_slice_unaligned()); + zst_test!(new_slice_unaligned_zeroed()); + zst_test!(new_slice_unaligned_from_prefix(1)); + zst_test!(new_slice_unaligned_from_prefix_zeroed(1)); + zst_test!(new_slice_unaligned_from_suffix(1)); + zst_test!(new_slice_unaligned_from_suffix_zeroed(1)); } #[test] From 277438a38bc39e107c232d3c62a0a5a4381c9daa Mon Sep 17 00:00:00 2001 From: Joshua Liebow-Feeser Date: Tue, 14 Jul 2020 03:33:51 +0000 Subject: [PATCH 06/12] [zerocopy] Document why Vec: !ByteSlice Test: None Change-Id: I4ca64b81df5e0c1d83cd3b2b32cccfed50c8f677 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/406497 Reviewed-by: Tyler Mandry Testability-Review: Joshua Liebow-Feeser Commit-Queue: Joshua Liebow-Feeser --- src/lib.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 6681110b17..62b5d18bdf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1254,6 +1254,14 @@ mod sealed { /// `ByteSlice` abstracts over the mutability of a byte slice reference, and is /// implemented for various special reference types such as `Ref<[u8]>` and /// `RefMut<[u8]>`. +/// +/// Note that, while it would be technically possible, `ByteSlice` is not +/// implemented for [`Vec`], as the only way to implement the [`split_at`] +/// method would involve reallocation, and `split_at` must be a very cheap +/// operation in order for the utilities in this crate to perform as designed. +/// +/// [`Vec`]: std::vec::Vec +/// [`split_at`]: crate::ByteSlice::split_at pub unsafe trait ByteSlice: Deref + Sized + self::sealed::Sealed { fn as_ptr(&self) -> *const u8; fn split_at(self, mid: usize) -> (Self, Self); From 2e0c9475baa65779d9703e3eeda14473f7792010 Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Wed, 22 Jul 2020 21:17:36 +0000 Subject: [PATCH 07/12] [lsc][rust][gn] List sources for Rust targets Bug: 55669 Change-Id: I534ef63a9bb8209027796787577e7a2842e0f8f5 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/409525 Reviewed-by: Kevin Lindkvist Commit-Queue: Adam Lesinski --- BUILD.gn | 7 +++++++ zerocopy-derive/BUILD.gn | 16 ++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/BUILD.gn b/BUILD.gn index 1790b7017a..3fa4116434 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -17,6 +17,13 @@ rustc_library("zerocopy") { "//third_party/rust_crates:byteorder", "//third_party/rust_crates:rand", ] + + enforce_source_listing = true + + sources = [ + "src/byteorder.rs", + "src/lib.rs", + ] } unittest_package("zerocopy_tests") { diff --git a/zerocopy-derive/BUILD.gn b/zerocopy-derive/BUILD.gn index 3bab982d1c..b235509c5f 100644 --- a/zerocopy-derive/BUILD.gn +++ b/zerocopy-derive/BUILD.gn @@ -14,6 +14,14 @@ rustc_macro("zerocopy-derive") { "//third_party/rust_crates:syn", "//third_party/rust_crates:synstructure", ] + + enforce_source_listing = true + + sources = [ + "src/ext.rs", + "src/lib.rs", + "src/repr.rs", + ] } group("tests") { @@ -31,5 +39,13 @@ if (is_host) { "//third_party/rust_crates:syn", "//third_party/rust_crates:synstructure", ] + + enforce_source_listing = true + + sources = [ + "src/ext.rs", + "src/lib.rs", + "src/repr.rs", + ] } } From e63098190d56eb70c9fde62edf138477f0f1089b Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Sat, 8 Aug 2020 00:42:51 +0000 Subject: [PATCH 08/12] [gn][rust] Cleanup soft-migration source enforcement opt-in Change-Id: I25b441fa70a64bb35a31cc94cf7f5aefc1e880be Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/414748 Commit-Queue: Adam Lesinski Reviewed-by: Kevin Lindkvist --- BUILD.gn | 2 -- zerocopy-derive/BUILD.gn | 4 ---- 2 files changed, 6 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index 3fa4116434..c45c589c78 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -18,8 +18,6 @@ rustc_library("zerocopy") { "//third_party/rust_crates:rand", ] - enforce_source_listing = true - sources = [ "src/byteorder.rs", "src/lib.rs", diff --git a/zerocopy-derive/BUILD.gn b/zerocopy-derive/BUILD.gn index b235509c5f..636c1859b6 100644 --- a/zerocopy-derive/BUILD.gn +++ b/zerocopy-derive/BUILD.gn @@ -15,8 +15,6 @@ rustc_macro("zerocopy-derive") { "//third_party/rust_crates:synstructure", ] - enforce_source_listing = true - sources = [ "src/ext.rs", "src/lib.rs", @@ -40,8 +38,6 @@ if (is_host) { "//third_party/rust_crates:synstructure", ] - enforce_source_listing = true - sources = [ "src/ext.rs", "src/lib.rs", From 2599f31bc3946fd50b75cfaf533e94d5280fb7e5 Mon Sep 17 00:00:00 2001 From: Nathan Mulcahey Date: Mon, 26 Oct 2020 18:51:00 +0000 Subject: [PATCH 09/12] [cleanup] Use inclusive terms in git links. Bug: 53982 Change-Id: Ifb7c0f8055fb09520410dbf9dec79be697aa33ba Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/442877 Reviewed-by: Ian McKellar API-Review: Ian McKellar Testability-Review: Ian McKellar Commit-Queue: Nathan Mulcahey --- Cargo.toml.crates-io | 2 +- zerocopy-derive/Cargo.toml.crates-io | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml.crates-io b/Cargo.toml.crates-io index 9e91c1b144..a929502d0c 100644 --- a/Cargo.toml.crates-io +++ b/Cargo.toml.crates-io @@ -11,7 +11,7 @@ version = "0.3.0" authors = ["Joshua Liebow-Feeser "] description = "Utilities for zero-copy parsing and serialization" license = "BSD-3-Clause" -repository = "https://fuchsia.googlesource.com/fuchsia/+/master/src/lib/zerocopy" +repository = "https://fuchsia.googlesource.com/fuchsia/+/HEAD/src/lib/zerocopy" include = ["src/*", "Cargo.toml", "LICENSE"] diff --git a/zerocopy-derive/Cargo.toml.crates-io b/zerocopy-derive/Cargo.toml.crates-io index 3970b716da..1c1c2721bc 100644 --- a/zerocopy-derive/Cargo.toml.crates-io +++ b/zerocopy-derive/Cargo.toml.crates-io @@ -11,7 +11,7 @@ version = "0.2.0" authors = ["Joshua Liebow-Feeser "] description = "Custom derive for traits from the zerocopy crate" license = "BSD-3-Clause" -repository = "https://fuchsia.googlesource.com/fuchsia/+/master/src/lib/zerocopy/zerocopy-derive" +repository = "https://fuchsia.googlesource.com/fuchsia/+/HEAD/src/lib/zerocopy/zerocopy-derive" include = ["src/*", "tests/*", "Cargo.toml", "LICENSE"] From 3c3f66c8a7f65f8a7c60545b4031fb874ede81f8 Mon Sep 17 00:00:00 2001 From: Joshua Liebow-Feeser Date: Wed, 10 Feb 2021 01:17:44 +0000 Subject: [PATCH 10/12] [zerocopy] Implement traits for all array lengths Now that minimal const generics are stable, implement zerocopy traits for all array lengths rather than a specific list of array lengths. Change-Id: I4f03d2fd65641cecbfc44345e7803de64d592cff Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/483581 Reviewed-by: Tyler Mandry Reviewed-by: Adam Barth Commit-Queue: Joshua Liebow-Feeser --- src/lib.rs | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 62b5d18bdf..c1dbb3bb18 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -68,7 +68,13 @@ macro_rules! impl_for_composite_types { { } } - impl_for_array_sizes!($trait); + unsafe impl $trait for [T; N] { + fn only_derive_is_allowed_to_implement_this_trait() + where + Self: Sized, + { + } + } }; } @@ -90,26 +96,6 @@ macro_rules! impl_for_primitives { ); } -// implement an unsafe trait for all array lengths up to 64, plus several -// useful powers-of-two beyond that, plus lengths needed by Fuchsia with -// an element type that implements the trait -macro_rules! impl_for_array_sizes { - ($trait:ident) => ( - impl_for_array_sizes!(@inner $trait, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 98, 126, 128, 236, 255, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768, 65536); - ); - (@inner $trait:ident, $n:expr) => ( - unsafe impl $trait for [T; $n] { - fn only_derive_is_allowed_to_implement_this_trait() where Self: Sized {} - } - ); - (@inner $trait:ident, $n:expr, $($ns:expr),*) => ( - unsafe impl $trait for [T; $n] { - fn only_derive_is_allowed_to_implement_this_trait() where Self: Sized {} - } - impl_for_array_sizes!(@inner $trait, $($ns),*); - ); -} - /// Types for which any byte pattern is valid. /// /// WARNING: Do not implement this trait yourself! Instead, use From 1abe610a80978d8a454488b8d826470494395e75 Mon Sep 17 00:00:00 2001 From: Victor Chang Date: Thu, 25 Feb 2021 17:43:06 +0000 Subject: [PATCH 11/12] [package] Migrate more `src/lib/*` to use the new package templates. Change-Id: Ib950efa4945de7fcc8fea29f299f2ad5dfdb69fb Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/490583 Fuchsia-Auto-Submit: Victor Chang Reviewed-by: Shai Barack Commit-Queue: Victor Chang --- BUILD.gn | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index c45c589c78..443903011d 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -3,7 +3,7 @@ # found in the LICENSE file. import("//build/rust/rustc_library.gni") -import("//build/test/test_package.gni") +import("//src/sys/build/components.gni") rustc_library("zerocopy") { name = "zerocopy" @@ -24,15 +24,8 @@ rustc_library("zerocopy") { ] } -unittest_package("zerocopy_tests") { +fuchsia_unittest_package("zerocopy_tests") { deps = [ ":zerocopy_test" ] - - tests = [ - { - name = "zerocopy_lib_test" - environments = basic_envs - }, - ] } group("tests") { From 2c37bbe515f39b466842e8e8b803a81f65c9495d Mon Sep 17 00:00:00 2001 From: Joshua Liebow-Feeser Date: Fri, 5 Mar 2021 02:16:31 +0000 Subject: [PATCH 12/12] [zerocopy] Publish 0.3.1 Change-Id: I65825dadf73c8e20c858b9e724486f667c7e4166 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/496020 Commit-Queue: Tyler Mandry Commit-Queue: Joshua Liebow-Feeser Reviewed-by: Tyler Mandry --- Cargo.toml.crates-io | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml.crates-io b/Cargo.toml.crates-io index a929502d0c..93f634ccb7 100644 --- a/Cargo.toml.crates-io +++ b/Cargo.toml.crates-io @@ -7,7 +7,7 @@ [package] edition = "2018" name = "zerocopy" -version = "0.3.0" +version = "0.3.1" authors = ["Joshua Liebow-Feeser "] description = "Utilities for zero-copy parsing and serialization" license = "BSD-3-Clause"