diff options
| -rw-r--r-- | CONTRIBUTING.md | 86 | ||||
| -rw-r--r-- | src/ir/comp.rs | 3 | ||||
| -rw-r--r-- | src/ir/item.rs | 3 | ||||
| -rw-r--r-- | src/ir/ty.rs | 50 | ||||
| -rw-r--r-- | tests/expectations/tests/forward-inherit-struct-with-fields.rs | 17 | ||||
| -rw-r--r-- | tests/expectations/tests/forward-inherit-struct.rs | 18 | ||||
| -rw-r--r-- | tests/expectations/tests/inherit-namespaced.rs | 18 | ||||
| -rw-r--r-- | tests/expectations/tests/multiple-inherit-empty-correct-layout.rs | 45 | ||||
| -rw-r--r-- | tests/headers/forward-inherit-struct-with-fields.hpp | 8 | ||||
| -rw-r--r-- | tests/headers/forward-inherit-struct.hpp | 5 | ||||
| -rw-r--r-- | tests/headers/inherit-namespaced.hpp | 4 | ||||
| -rw-r--r-- | tests/headers/multiple-inherit-empty-correct-layout.hpp | 3 |
12 files changed, 256 insertions, 4 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fb3208ff..ce95e21c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,6 +14,7 @@ yourself. * [Authoring New Tests](#tests-new) * [Automatic Code Formatting](#formatting) * [Debug Logging](#logs) +* [Using `creduce` to Minimize Test Cases](#creduce) ## Code of Conduct <span id="coc"/> @@ -138,3 +139,88 @@ This logging can also be used when debugging failing tests under ``` $ RUST_LOG=bindgen ./tests/tools/run-bindgen.py ./target/debug/bindgen tests/headers/whatever.h ``` + +## Using `creduce` to Minimize Test Cases <span id="creduce"/> + +If you are hacking on `bindgen` and find a test case that causes an unexpected +panic, results in bad Rust bindings, or some other incorrectness in `bindgen`, +then using `creduce` can help reduce the test case to a minimal one. + +[Follow these instructions for building and/or installing `creduce`.](https://github.com/csmith-project/creduce/blob/master/INSTALL) + +Running `creduce` requires two things: + +1. Your isolated test case, and + +2. A script to act as a predicate script describing whether the behavior you're + trying to isolate occurred. + +With those two things in hand, running `creduce` looks like this: + + $ creduce ./predicate.sh ./isolated_test_case.h + +### Isolating Your Test Case + +Use the `-save-temps` flag to make Clang spit out its intermediate +representations when compiling the test case into an object file. + + $ clang[++ -x c++ --std=c++14] -save-temps -c my_test_case.h + +There should now be a `my_test_case.ii` file, which is the results after the C +pre-processor has processed all the `#include`s, `#define`s, and `#ifdef`s. This +is generally what we're looking for. + +### Writing a Predicate Script + +Writing a `predicate.sh` script for a `bindgen` test case is fairly +straightforward. One potential gotcha is that `creduce` can and will attempt to +reduce test cases into invalid C/C++ code. That might be useful for C/C++ +compilers, but we generally only care about valid C/C++ input headers. + +Here is a skeleton predicate script: + +```bash +#!/usr/bin/env bash + +# Exit the script with a nonzero exit code if: +# * any individual command finishes with a nonzero exit code, or +# * we access any undefined variable. +set -eu + +# Print out Rust backtraces on panic. Useful for minimizing a particular panic. +export RUST_BACKTRACE=1 + +# If the `libclang.so` you're using for `bindgen` isn't the system +# `libclang.so`, let the linker find it. +export LD_LIBRARY_PATH=~/path/to/your/directory/containing/libclang + +# Make sure that the reduced test case is valid C/C++ by compiling it. If it +# isn't valid C/C++, this command will exit with a nonzero exit code and cause +# the whole script to do the same. +clang[++ --std=c++14] -c ./pre_processed_header.hpp + +# Run `bindgen` and `grep` for the thing your hunting down! Make sure to include +# `2>&1` to get at stderr if you're hunting down a panic. +~/src/rust-bindgen/target/debug/bindgen \ + ./pre_processed_header.hpp \ + [ <extra flags> ] \ + 2>&1 \ + | grep "<pattern in generated bindings or a panic string or ...>" +``` + +When hunting down a panic, I `grep`ed like this: + + ... | grep "thread main panicked at '<panic error message here>'" + +When hunting down bad codegen for a base member, I `grep`ed like this: + + ... | grep "pub _base: MyInvalidBaseTypeThatShouldntBeHere" + +That's pretty much it! I want to impress upon you that `creduce` is *really* +helpful and has enabled me to reduce 30k lines of test case into 5 lines. And it +works pretty quickly too. Super valuable tool to have in your belt when hacking +on `bindgen`! + +Happy bug hunting and test case reducing! + +[More information on using `creduce`.](https://embed.cs.utah.edu/creduce/using/) diff --git a/src/ir/comp.rs b/src/ir/comp.rs index 41e5c3d3..bd794a98 100644 --- a/src/ir/comp.rs +++ b/src/ir/comp.rs @@ -635,7 +635,7 @@ impl CompInfo { ci.has_vtable = cur.is_virtual_base(); } let type_id = - Item::from_ty(&cur.cur_type(), None, None, ctx) + Item::from_ty(&cur.cur_type(), Some(cur), None, ctx) .expect("BaseSpecifier"); ci.base_members.push(type_id); } @@ -763,6 +763,7 @@ impl CompInfo { CXCursor_UnionDecl => CompKind::Union, CXCursor_ClassDecl | CXCursor_StructDecl => CompKind::Struct, + CXCursor_CXXBaseSpecifier | CXCursor_ClassTemplatePartialSpecialization | CXCursor_ClassTemplate => { match cursor.template_kind() { diff --git a/src/ir/item.rs b/src/ir/item.rs index c6d80a08..691cfec2 100644 --- a/src/ir/item.rs +++ b/src/ir/item.rs @@ -1043,8 +1043,7 @@ impl ClangItemParser for Item { } } // If we have recursed into the AST all we know, and we still - // haven't found what we've got, let's - // just make a named type. + // haven't found what we've got, let's just make a named type. // // This is what happens with some template members, for example. // diff --git a/src/ir/ty.rs b/src/ir/ty.rs index 77dc61be..57490cc2 100644 --- a/src/ir/ty.rs +++ b/src/ir/ty.rs @@ -540,8 +540,56 @@ impl Type { } else if let Some(location) = location { match location.kind() { CXCursor_ClassTemplatePartialSpecialization | + CXCursor_CXXBaseSpecifier | CXCursor_ClassTemplate => { - name = location.spelling(); + if location.kind() == CXCursor_CXXBaseSpecifier { + // In the case we're parsing a base specifier + // inside an unexposed or invalid type, it means + // that we're parsing one of two things: + // + // * A template parameter. + // * A complex class that isn't exposed. + // + // This means, unfortunately, that there's no + // good way to differentiate between them. + // + // Probably we could try to look at the + // declaration and complicate more this logic, + // but we'll keep it simple... if it's a valid + // C++ identifier, we'll consider it as a + // template parameter. + // + // This is because: + // + // * We expect every other base that is a + // proper identifier (that is, a simple + // struct/union declaration), to be exposed, + // so this path can't be reached in that + // case. + // + // * Quite conveniently, complex base + // specifiers preserve their full names (that + // is: Foo<T> instead of Foo). We can take + // advantage of this. + // + // If we find some edge case where this doesn't + // work (which I guess is unlikely, see the + // different test cases[1][2][3][4]), we'd need + // to find more creative ways of differentiating + // these two cases. + // + // [1]: inherit_named.hpp + // [2]: forward-inherit-struct-with-fields.hpp + // [3]: forward-inherit-struct.hpp + // [4]: inherit-namespaced.hpp + if location.spelling() + .chars() + .all(|c| c.is_alphanumeric() || c == '_') { + return Err(ParseError::Recurse); + } + } else { + name = location.spelling(); + } let complex = CompInfo::from_ty(potential_id, ty, Some(location), diff --git a/tests/expectations/tests/forward-inherit-struct-with-fields.rs b/tests/expectations/tests/forward-inherit-struct-with-fields.rs new file mode 100644 index 00000000..84104971 --- /dev/null +++ b/tests/expectations/tests/forward-inherit-struct-with-fields.rs @@ -0,0 +1,17 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct Rooted<T> { + pub _base: RootedBase<T>, +} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct RootedBase<T> { + pub foo: *mut T, + pub next: *mut Rooted<T>, +} diff --git a/tests/expectations/tests/forward-inherit-struct.rs b/tests/expectations/tests/forward-inherit-struct.rs new file mode 100644 index 00000000..e053adcd --- /dev/null +++ b/tests/expectations/tests/forward-inherit-struct.rs @@ -0,0 +1,18 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct Rooted<T> { + pub _address: u8, + pub _phantom_0: ::std::marker::PhantomData<T>, +} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct RootedBase<T> { + pub _address: u8, + pub _phantom_0: ::std::marker::PhantomData<T>, +} diff --git a/tests/expectations/tests/inherit-namespaced.rs b/tests/expectations/tests/inherit-namespaced.rs new file mode 100644 index 00000000..a58058b0 --- /dev/null +++ b/tests/expectations/tests/inherit-namespaced.rs @@ -0,0 +1,18 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct RootedBase<T> { + pub _address: u8, + pub _phantom_0: ::std::marker::PhantomData<T>, +} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct Rooted<T> { + pub _address: u8, + pub _phantom_0: ::std::marker::PhantomData<T>, +} diff --git a/tests/expectations/tests/multiple-inherit-empty-correct-layout.rs b/tests/expectations/tests/multiple-inherit-empty-correct-layout.rs new file mode 100644 index 00000000..5e9cf522 --- /dev/null +++ b/tests/expectations/tests/multiple-inherit-empty-correct-layout.rs @@ -0,0 +1,45 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +#[repr(C)] +#[derive(Debug, Copy)] +pub struct Foo { + pub _address: u8, +} +#[test] +fn bindgen_test_layout_Foo() { + assert_eq!(::std::mem::size_of::<Foo>() , 1usize); + assert_eq!(::std::mem::align_of::<Foo>() , 1usize); +} +impl Clone for Foo { + fn clone(&self) -> Self { *self } +} +#[repr(C)] +#[derive(Debug, Copy)] +pub struct Bar { + pub _address: u8, +} +#[test] +fn bindgen_test_layout_Bar() { + assert_eq!(::std::mem::size_of::<Bar>() , 1usize); + assert_eq!(::std::mem::align_of::<Bar>() , 1usize); +} +impl Clone for Bar { + fn clone(&self) -> Self { *self } +} +#[repr(C)] +#[derive(Debug, Copy)] +pub struct Baz { + pub _address: u8, +} +#[test] +fn bindgen_test_layout_Baz() { + assert_eq!(::std::mem::size_of::<Baz>() , 1usize); + assert_eq!(::std::mem::align_of::<Baz>() , 1usize); +} +impl Clone for Baz { + fn clone(&self) -> Self { *self } +} diff --git a/tests/headers/forward-inherit-struct-with-fields.hpp b/tests/headers/forward-inherit-struct-with-fields.hpp new file mode 100644 index 00000000..437fff5d --- /dev/null +++ b/tests/headers/forward-inherit-struct-with-fields.hpp @@ -0,0 +1,8 @@ +template <typename> class Rooted; +namespace js { + template <typename T> class RootedBase { + T* foo; + Rooted<T>* next; + }; +} +template <typename T> class Rooted : js::RootedBase<T> {}; diff --git a/tests/headers/forward-inherit-struct.hpp b/tests/headers/forward-inherit-struct.hpp new file mode 100644 index 00000000..ac7aef5e --- /dev/null +++ b/tests/headers/forward-inherit-struct.hpp @@ -0,0 +1,5 @@ +template <typename> class Rooted; +namespace js { + template <typename T> class RootedBase {}; +} +template <typename T> class Rooted : js::RootedBase<T> {}; diff --git a/tests/headers/inherit-namespaced.hpp b/tests/headers/inherit-namespaced.hpp new file mode 100644 index 00000000..61eafd5a --- /dev/null +++ b/tests/headers/inherit-namespaced.hpp @@ -0,0 +1,4 @@ +namespace js { + template <typename T> class RootedBase {}; +} +template <typename T> class Rooted : js::RootedBase<T> {}; diff --git a/tests/headers/multiple-inherit-empty-correct-layout.hpp b/tests/headers/multiple-inherit-empty-correct-layout.hpp new file mode 100644 index 00000000..1e2b133a --- /dev/null +++ b/tests/headers/multiple-inherit-empty-correct-layout.hpp @@ -0,0 +1,3 @@ +struct Foo {}; +struct Bar {}; +struct Baz : public Foo, public Bar {}; |