Quickchecking crate CLI
Prior to this commit the quickchecking crate used for generating proprty tests for bindgen was a [lib] target and had configurations that required commenting/uncommenting code to enable/disable. This meant it was inconvienent/prohibitive to configure the property tests on a per-run basis. This commit reorganizes the `quickchecking` crate to provide both [lib] and [[bin]] targets in order to expose those configurations through a CLI.
The configurations that are exposed through the [[bin]] target's CLI are:
* Count/number of tests to run.
* Directory to provide fuzzed headers
* Generation range corresponding to the range quickcheck uses to
* generate arbitrary.
__Usage from the__ `tests/quickchecking` __directory__
```bash
quickchecking 0.2.0
Bindgen property tests with quickcheck. Generate random valid C code and pass it to the csmith/predicate.py script
USAGE:
quickchecking [OPTIONS]
FLAGS:
-h, --help Prints help information
-V, --version Prints version information
OPTIONS:
-c, --count <COUNT> Count / number of tests to run. Running a fuzzed header through the predicate.py script can
take a long time, especially if the generation range is large. Increase this number if you're
willing to wait a while. [default: 2]
-p, --path <PATH> Optional. Preserve generated headers for inspection, provide directory path for header
output. [default: None]
-r, --range <RANGE> Sets the range quickcheck uses during generation. Corresponds to things like arbitrary usize
and arbitrary vector length. This number doesn't have to grow much for that execution time to
increase significantly. [default: 32]
```
Because the actual work of running the property tests moved to the [[bin]]
target, rather than duplicate that code in the `quickchecking` crate's tests
directory, some actual (very basic) tests for the `quickchecking` crate were
added.
*Note: I'm not attached to any of the option flags, if there are better characters/words for any of the options I've exposed I'll be happy to revise!
Also, I'm not sure how palatable the "global singleton" is for managing context (output path) across tests in the `lib.rs` file. Very open to suggestions on how to manage that if it's not an acceptable approach.
Thanks for taking a look, looking forward to feedback!
Closes#1168
r? @fitzgen
The changes reflected in this PR include the logic to test the
`quickcheking` crate itself. Rather that just validate that the
`quickchecking` crate builds in CI with `cargo check`, we can run
now `cargo test`.
Prior to this commit the quickchecking crate used for generating proprty
tests
for bindgen was a [lib] target and had configurations that required
commenting/uncommenting code to enable/disable. This meant it was
inconvienent/prohibitive to configure the property tests on a per-run
basis.
This commit reorganizes the `quickchecking` crate to provide both [lib]
and
[[bin]] targets in order to expose those configurations through a CLI.
The configurations that are exposed through the [[bin]] target's CLI
are:
* Count/number of tests to run.
* Directory to provide fuzzed headers
* Generation range corresponding to the range quickcheck uses to
* generate arbitrary.
__Usage from the__ `tests/quickchecking` __directory__
```bash
quickchecking 0.2.0
Bindgen property tests with quickcheck. Generate random valid C code and
pass it to the csmith/predicate.py script
USAGE:
quickchecking [OPTIONS]
FLAGS:
-h, --help Prints help information
-V, --version Prints version information
OPTIONS:
-c, --count <COUNT> Count / number of tests to run. Running a
fuzzed header through the predicate.py script can
take a long time, especially if the
generation range is large. Increase this number if you're
willing to wait a while. [default: 2]
-p, --path <PATH> Optional. Preserve generated headers for
inspection, provide directory path for header
output. [default: None]
-r, --range <RANGE> Sets the range quickcheck uses during
generation. Corresponds to things like arbitrary usize
and arbitrary vector length. This number
doesn't have to grow much for that execution time to
increase significantly. [default: 32]
```
Because the actual work of running the property tests moved to the
[[bin]]
target, rather than duplicate that code in the `quickchecking` crate's
tests
directory, some actual (very basic) tests for the `quickchecking` crate
were
added.
*Note: I'm not attached to any of the option flags, if there are better
characters/words for any of the options I've exposed I'll be happy to
revise!
Thanks for taking a look, looking forward to feedback!
Closes#1168
r? @fitzgen
Property testing with quickcheck
This PR represents an attempt to address issue #970. It also represents a portion of the meta issue for fuzzing #972.
The code base reflected here uses quickcheck to generate C headers that
include a variety of types including basic types, structs, unions,
function prototypes and function pointers. The headers generated by quickcheck
are passed to the `csmith-fuzzing/predicate.py` script. Examples of headers
generated by this iteration of the tooling can be viewed
[here](https://gist.github.com/snewt/03ce934f35c5b085807d2d5cf11d1d5c).
At the top of each header are two simple struct definitions,
`whitelistable` and `blacklistable`. Those types are present in the vector that
represents otherwise primitive types used to generate. They represent a naive
approach to exposing custom types without having to intuit generated type names like
`struct_21_8` though _any actual whitelisting logic isn't implemented
here_.
Test success is measured by the success of the
`csmith-fuzzing/predicate.py`
script. This means that for a test to pass the following must be true:
- bindgen doesn't panic
- the resulting bindings compile
- the resulting bindings layout tests pass
#### Usage
```bash
cd tests/property_test
cargo test
```
Some things I'm unsure of:
#### Where should this feature live?
At the moment it lives in `tests/property_test` but isn't run when
`cargo test` is invoked from bindgen's cargo manifest directory.
#### What's an acceptable ammount of time for these tests to take?
At this point, the source is genereated in ~1 second but the files are
large enough that it takes the `predicate.py` script ~30 seconds to run
through each one. In order for the tests to run in under a minute only 2 are
generated by quickcheck by default. This can be changed in the `test_bindgen`
function of the `tests/property_test/tests/fuzzed-c-headers.rs` file.
#### How do we expose the generated code for easy inspection?
For now the `run_predicate_script` function in the
`tests/property_test/tests/fuzzed-c-headers.rs` file contains a
commented block that will copy generated source in the `tests/property_test/tests`
directory. Should it be easier?
#### Special casing
There is some logic in the fuzzer that disallows 0 sized arrays because
tests will regulary fail due to issues documented in #684 and #1153. Should
this be special casing?
#### Does the fuzzer warrant its own crate?
After any iterations the reviewers are interested in required to make
this a functional testing tool, should/could the fuzzing library be made into
its own crate? I didn't move in that direction yet because having it all in one
place seemed like the best way to figure out what works an doesn't but I'm
interested in whether it might be useful as a standalone library.
#### What does it look like to expose more useful functionality?
I'm looking forward to feedback on how to make this a more useful tool
and one that provides the right configurability.
Thanks!
r? @fitzgen
- Remove `whitelistable` and `blacklistable` types.
- Rename test crate directory from `property_test` to `quickchecking`.
- Add new CI job that checks that this crate continues to build.
- Revise matching logic to be more idomatic.
- Phase out modular arithmetic in favor of `gen_range`.
- Incorporate `unreachable!` into match statements.
- Revise logic for accessing random element of vector, favor `choose`
over `nth`.
- Proper punctuation and capitalization in comments.
- Using actual structures rather than converting everything to strings
in order to leverage type system.
- Add `#![deny(missing_docs)]` and filled in documentation required for
the project to build again.
- Add special case logic so we don't generate structs with `long double`
fields as it will cause tests to fail unitl issue \#550 is resolved
Note on making sure we don't lose test cases we're interested in
preserving:
We're copying the directories `TempDir` makes so we get things like
this:
```
├── bindgen_prop.1WYe3F5HZU1c
│ └── prop_test.h
├── bindgen_prop.H4SLI1JX0jd8
│ └── prop_test.h
```
I'm not sure that `TempDir` makes any claims about uniqueness, so
collisions
probably aren't impossible. I'm up for any suggestions on a more
bulletproof
solution.
_Tasks not addressed by this PR:_
* TODO: Add `cargo features` logic to allow generating problematic code.
* TODO: Make a [bin] target with CLI to manage test settings.
* TODO: Whitelisting and opaque types.
* TODO: Generate bitfields, C++, I-bogus-codegen cases.
Figured this would be a good point to update the PR but if any of the
above TODO
items should be incorporated before moving forward I'm up for it!
Thanks for taking another look!
r? @fitzgen
Remove unnecessary ci jobs
And then in the process I noticed that we weren't running the layout tests for any of the libclang version-specific expectation files, so I also fixed that.
r? @emilio or @pepyakin
This adds a build.rs to generate
#[path="$FILE"]
mod $FILE_AS_IDENT;
for each $FILE in our libclang version-specific directories. We need to do this
to get those files' layout tests to run because cargo doesn't automatically pick
up tests in subdirectories.
Only generating bindings depends on which libclang version we are dealing
with. Running the test expectations' layout tests does not change between
libclang versions, so these were redundant tests.
Note that we have not been testing the expected bindings that differ across
libclang versions (the nested sub-directories aren't automatically picked up by
cargo) and this commit does not change that.
Support bitfield allocation units larger than 64 bits
Individual bitfields are still limited to at most 64 bits, but this restriction can be weakened when Rust supports `u128`.
This implements issue #816.
Usage notes:
* Since common code is added to each generated binding, a program which uses
more than one binding may need to work around the duplication by including
each binding in its own module.
* The values created by bitfield allocation unit constructors can be assigned
directly to the corresponding struct fields with no need for transmutation.
Implementation notes:
`__BindgenBitfieldUnit` represents a bitfield allocation unit using a `Storage`
type accessible as a slice of `u8`. The alignment of the unit is inherited from
an `Align` type by virtue of the field:
```
align: [Align; 0],
```
The position of this field in the struct is irrelevant.
It is assumed that the alignment of the `Storage` type is no larger than the
alignment of the `Align` type, which will be true if the `Storage` type is, for
example, an array of `u8`. This assumption is checked in a debug assertion.
Although the double underscore (__) prefix is reserved for implementations of
C++, there are precedents for this convention elsewhere in bindgen and so the
convention is adopted here too.
Acknowledgement:
Thanks to @fitzgen for an initial implementation of `__BindgenBitfieldUnit` and
code to integrate it into bindgen.
r? @emilio
a portion of the meta issue for fuzzing #972.
The code base reflected here uses quickcheck to generate C headers that
include a variety of types including basic types, structs, unions,
function
prototypes and function pointers. The headers generated by quickcheck
are
passed to the `csmith-fuzzing/predicate.py` script. Examples of headers
generated by this iteration of the tooling can be viewed
[here](https://gist.github.com/snewt/03ce934f35c5b085807d2d5cf11d1d5c).
At the top of each header are two simple struct definitions,
`whitelistable`
and `blacklistable`. Those types are present in the vector that
represents
otherwise primitive types used to generate. They represent a naive
approach to
exposing custom types without having to intuit generated type names like
`struct_21_8` though _any actual whitelisting logic isn't implemented
here_.
Test success is measured by the success of the
`csmith-fuzzing/predicate.py`
script. This means that for a test to pass the following must be true:
- bindgen doesn't panic
- the resulting bindings compile
- the resulting bindings layout tests pass
```bash
cd tests/property_test
cargo test
```
Some things I'm unsure of:
At the moment it lives in `tests/property_test` but isn't run when
`cargo test`
is invoked from bindgen's cargo manifest directory.
At this point, the source is genereated in ~1 second but the files are
large
enough that it takes the `predicate.py` script ~30 seconds to run
through each
one. In order for the tests to run in under a minute only 2 are
generated by
quickcheck by default. This can be changed in the `test_bindgen`
function of the
`tests/property_test/tests/fuzzed-c-headers.rs` file.
For now the `run_predicate_script` function in the
`tests/property_test/tests/fuzzed-c-headers.rs` file contains a
commented block
that will copy generated source in the `tests/property_test/tests`
directory.
Should it be easier?
There is some logic in the fuzzer that disallows 0 sized arrays because
tests
will regulary fail due to issues documented in #684 and #1153. Should
this be
special casing?
After any iterations the reviewers are interested in required to make
this
a functional testing tool, should/could the fuzzing library be made into
its own
crate? I didn't move in that direction yet because having it all in one
place
seemed like the best way to figure out what works an doesn't but I'm
interested
in whether it might be useful as a standalone library.
I'm looking forward to feedback on how to make this a more useful tool
and one
that provides the right configurability.
Thanks!
r? @fitzgen
Individual bitfields are still limited to at most 64 bits, but this
restriction can be weakened when Rust supports u128.
This implements issue #816.
Usage notes:
* Since common code is added to each generated binding, a program which uses
more than one binding may need to work around the duplication by including
each binding in its own module.
* The values created by bitfield allocation unit constructors can be assigned
directly to the corresponding struct fields with no need for transmutation.
Implementation notes:
__BindgenBitfieldUnit represents a bitfield allocation unit using a Storage
type accessible as a slice of u8. The alignment of the unit is inherited from
an Align type by virtue of the field:
align: [Align; 0],
The position of this field in the struct is irrelevant.
The alignment of the Storage type is intended to be no larger than the
alignment of the Align type, which will be true if the Storage type is, for
example, an array of u8.
Although the double underscore (__) prefix is reserved for implementations of
C++, there are precedents for this convention elsewhere in bindgen and so the
convention is adopted here too.
Acknowledgement:
Thanks to @fitzgen for an initial implementation of __BindgenBitfieldUnit and
code to integrate it into bindgen.
Also:
* disable rustfmt in the integration test, to avoid it causing problems, since
it is not needed.
* note the need to rebuild bindgen when running a single test.
Remove unnecessary flag from rustfmt invocation
From rustfmt docs, the --write-mode flag is unecessary when passing the flag to stdin. If we leave it there, rustfmt-nightly complains that it cannot find the file named --write-mode, so let's remove the flag.
here's the logs when I leave the flag :
```
Error: file `--write-mode=display` does not exist
Error { repr: Custom(Custom { kind: Other, error: StringError("Internal rustfmt error") }) }
```
<details>
<summary>The build.rs : </summary>
```rust
extern crate bindgen;
use std::env;
use std::path::PathBuf;
fn main() {
println!("cargo:rustc-link-lib=static=transistor.nro");
println!("cargo:rustc-link-search=native=libtransistor/build/lib");
let bindings = bindgen::Builder::default()
.header("libtransistor/include/libtransistor/nx.h")
// Don't use host headers, to make sure we're building against newlib
.clang_arg("-nostdinc")
// Include the newlib/transistor headers, and the clang builtin headers
.clang_args(&["-isystem", "/usr/lib/clang/5.0.0/include"])
.clang_args(&["-isystem", "libtransistor/newlib/newlib/libc/include"])
.clang_args(&["-isystem", "libtransistor/newlib/newlib/libc/sys/switch/include"])
.clang_arg("-Ilibtransistor/include")
// We don't need to define those types, rust has them already anyways.
// Blacklisting avoids a bug in bindgen where it creates cyclic references
// (pub type u8 = u8)
.blacklist_type("u(8|16|32|64)")
.rustfmt_bindings(true)
.rustfmt_configuration_file(None)
.generate()
.expect("Unable to generate bindings");
let out_path = PathBuf::from(env::var("OUT_DIR").unwrap());
bindings
.write_to_file(out_path.join("bindings.rs"))
.expect("Couldn't write bindings!");
}
```
</details>
From rustfmt docs, the --write-mode flag is unecessary when passing the
flag to stdin. If we leave it there, rustfmt-nightly complains that it
cannot find the file named --write-mode, so let's remove the flag.
Make bitfield unit allocation fallible
Instead of panicking when we see a bitfield that does not have a layout, return an error up the stack. If we get an error when allocating bitfields into units, then make the whole struct opaque.
Fixes#1140
r? @pepyakin or @emilio
Instead of panicking when we see a bitfield that does not have a layout, return
an error up the stack. If we get an error when allocating bitfields into units,
then make the whole struct opaque.
Fixes#1140
Detect `#pragma pack(...)` and make `pack(n)` where `n > 1` opaque
This is a bandaid for #537. It does *not* fix the underlying issue, which requires `#[repr(packed = "N")]` support in Rust. However, it does make sure that we don't generate type definitions with the wrong layout, or fail our generated layout tests.
r? @emilio or @pepyakin
This is a bandaid for #537. It does *not* fix the underlying issue, which
requires `#[repr(packed = "N")]` support in Rust. However, it does make sure
that we don't generate type definitions with the wrong layout, or fail our
generated layout tests.
Add a changelog
It is initially populated with information from our release announcements.
It has an "unreleased" section for accumulating notable changes that haven't been released yet, and to make it easier to summarize releases when we do publish them.
Fixes#1131
r? @emilio or @pepyakin
It is initially populated with information from our release announcements.
It has an "unreleased" section for accumulating notable changes that haven't
been released yet, and to make it easier to summarize releases when we do
publish them.
Fixes#1131
Actually cleanup temp files in predicate
Embarrassingly, we were ignoring and swallowing a dumb reference error that prevented the temp files from ever getting cleaned up.
The good news: with ignoring bitfields and packed structs, I got to `driver.py` iteration 27145 without triggering any `bindgen` bugs, and then the disk got full! :-p
r? @pepyakin
According to the x86[-64] ABI spec: "Unnamed bit-fields’ types do not affect the
alignment of a structure or union". This makes sense: such bit-fields are only
used for padding, and we can't perform an un-aligned read of something we can't
read because we can't even name it.
Fixes#1076
ir: We can't guarantee the type to be in the item map while propagating AlreadyResolved
The item may come from a recursive check down the stack, and as we say there:
> Unchecked because we haven't finished this type yet.
Fixes#1127
