diff options
| -rw-r--r-- | CONTRIBUTING.md | 104 | ||||
| -rwxr-xr-x | csmith-fuzzing/driver.py | 57 | ||||
| -rwxr-xr-x | csmith-fuzzing/predicate.py | 248 | ||||
| -rw-r--r-- | src/codegen/mod.rs | 22 | ||||
| -rw-r--r-- | src/features.rs | 3 | ||||
| -rw-r--r-- | src/ir/function.rs | 4 | ||||
| -rw-r--r-- | tests/expectations/tests/win32-thiscall_1_0.rs | 29 | ||||
| -rw-r--r-- | tests/expectations/tests/win32-thiscall_nightly.rs | 48 | ||||
| -rw-r--r-- | tests/headers/win32-thiscall_1_0.hpp | 7 | ||||
| -rw-r--r-- | tests/headers/win32-thiscall_nightly.hpp | 7 |
10 files changed, 447 insertions, 82 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a3562c8b..0a06709a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,6 +25,7 @@ out to us in a GitHub issue, or stop by - [Generating Graphviz Dot Files](#generating-graphviz-dot-files) - [Debug Logging](#debug-logging) - [Using `creduce` to Minimize Test Cases](#using-creduce-to-minimize-test-cases) + - [Getting `creduce`](#getting-creduce) - [Isolating Your Test Case](#isolating-your-test-case) - [Writing a Predicate Script](#writing-a-predicate-script) @@ -337,11 +338,25 @@ $ RUST_LOG=bindgen cargo test ## Using `creduce` to Minimize Test Cases -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. +If you find a test case that triggers an unexpected panic in `bindgen`, causes +`bindgen` to emit bindings that won't compile, define structs with the wrong +size/alignment, or results in any other kind of incorrectness, then using +`creduce` can help reduce the test case to a minimal one that still exhibits +that same bad behavior. -[Follow these instructions for building and/or installing `creduce`.](https://github.com/csmith-project/creduce/blob/master/INSTALL) +***Reduced test cases are SUPER helpful when filing bug reports!*** + +### Getting `creduce` + +Often, you can install `creduce` from your OS's package manager: + +``` +$ sudo apt install creduce +$ brew install creduce +$ # Etc... +``` + +[Otherwise, follow these instructions for building and/or installing `creduce`.](https://github.com/csmith-project/creduce/blob/master/INSTALL) Running `creduce` requires two things: @@ -352,7 +367,7 @@ Running `creduce` requires two things: With those two things in hand, running `creduce` looks like this: - $ creduce ./predicate.sh ./isolated_test_case.h + $ creduce ./predicate.sh ./isolated-test-case.h ### Isolating Your Test Case @@ -369,12 +384,9 @@ standalone test case. ### 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: +Writing a `predicate.sh` script for a `bindgen` test case is straightforward. We +already have a general purpose predicate script that you can use, you just have +to wrap and configure it. ```bash #!/usr/bin/env bash @@ -384,39 +396,61 @@ Here is a skeleton predicate script: # * we access any undefined variable. set -eu -# Print out Rust backtraces on panic. Useful for minimizing a particular panic. -export RUST_BACKTRACE=1 +# Invoke the general purpose predicate script that comes in the +# `bindgen` repository. +# +# You'll need to replace `--whatever-flags` with things that are specific to the +# incorrectness you're trying to pin down. See below for details. +path/to/rust-bindgen/csmith-fuzzing/predicate.py \ + --whatever-flags \ + ./isolated-test-case.h +``` + +When hunting down a particular panic emanating from inside `bindgen`, you can +invoke `predicate.py` like this: -# 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 +```bash +path/to/rust-bindgen/csmith-fuzzing/predicate.py \ + --expect-bindgen-fail \ + --bindgen-grep "thread main panicked at '<insert panic message here>'" \ + ./isolated-test-case.h +``` -# 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 +Alternatively, when hunting down a bad `#[derive(Eq)]` that is causing `rustc` +to fail to compile `bindgen`'s emitted bindings, you can invoke `predicate.py` +like this: -# 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 ...>" +```bash +path/to/rust-bindgen/csmith-fuzzing/predicate.py \ + --bindings-grep NameOfTheStructThatIsErroneouslyDerivingEq \ + --expect-compile-fail \ + --rustc-grep 'error[E0277]: the trait bound `f64: std::cmp::Eq` is not satisfied' \ + ./isolated-test-case.h ``` -When hunting down a panic, I `grep`ed like this: +Or, when minimizing a failing layout test in the compiled bindings, you can +invoke `predicate.py` like this: - ... | grep "thread main panicked at '<panic error message here>'" +```bash +path/to/rust-bindgen/csmith-fuzzing/predicate.py \ + --bindings-grep MyStruct \ + --expect-layout-tests-fail \ + --layout-tests-grep "thread 'bindgen_test_layout_MyStruct' panicked" \ + ./isolated-test-case.h +``` + +For details on all the flags that you can pass to `predicate.py`, run: -When hunting down bad codegen for a base member, I `grep`ed like this: +``` +$ path/to/rust-bindgen/csmith-fuzzing/predicate.py --help +``` - ... | grep "pub _base: MyInvalidBaseTypeThatShouldntBeHere" +And you can always write your own, arbitrary predicate script if you prefer. +(Although, maybe we should add extra functionality to `predicate.py` -- file an +issue if you think so!) -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`! +`creduce` is *really* helpful and can cut hundreds of thousands of lines of test +case down to 5 lines. Happy bug hunting and test case reducing! diff --git a/csmith-fuzzing/driver.py b/csmith-fuzzing/driver.py index 38ae9ff3..7dc6086c 100755 --- a/csmith-fuzzing/driver.py +++ b/csmith-fuzzing/driver.py @@ -28,29 +28,12 @@ def run_logged(cmd): cat(stdout.name, title="stderr") return result -def run_bindgen(input, output): - return run_logged([ - "bindgen", - "--with-derive-partialeq", - "--with-derive-eq", - "--with-derive-partialord", - "--with-derive-ord", - "--with-derive-hash", - "--with-derive-default", - "-o", output.name, - input.name, - "--", - "-I", os.path.abspath(os.path.dirname(sys.argv[0])), - ]) - -def run_rustc(output, test): - return run_logged([ - "rustc", - "--crate-type", "lib", - "--test", - output.name, - "-o", test.name, - ]) +BINDGEN_ARGS = "--with-derive-partialeq \ +--with-derive-eq \ +--with-derive-partialord \ +--with-derive-ord \ +--with-derive-hash \ +--with-derive-default" def main(): print("Fuzzing `bindgen` with C-Smith...\n") @@ -65,36 +48,22 @@ def main(): if result.returncode != 0: exit(1) - output = NamedTemporaryFile(delete=False, prefix="output-", suffix=".rs") - output.close() - result = run_bindgen(input, output) - if result.returncode != 0: - cat(input.name) - cat(output.name) - exit(1) - - test = NamedTemporaryFile(delete=False, prefix="test-") - test.close() - result = run_rustc(output, test) - if result.returncode != 0: - cat(input.name) - cat(output.name) - exit(1) - - result = run_logged([test.name]) + result = run_logged([ + "./predicate.py", + "--bindgen-args", + "{} -- -I{}".format(BINDGEN_ARGS, os.path.abspath(os.path.dirname(sys.argv[0]))), + input.name + ]) if result.returncode != 0: cat(input.name) - cat(output.name) exit(1) os.remove(input.name) - os.remove(output.name) - os.remove(test.name) - iterations += 1 if __name__ == "__main__": try: + os.chdir(os.path.abspath(os.path.dirname(sys.argv[0]))) main() except KeyboardInterrupt: exit() diff --git a/csmith-fuzzing/predicate.py b/csmith-fuzzing/predicate.py new file mode 100755 index 00000000..8896cd96 --- /dev/null +++ b/csmith-fuzzing/predicate.py @@ -0,0 +1,248 @@ +#!/usr/bin/env python3 + +import argparse +import os +import re +import shlex +import subprocess +import sys +from tempfile import NamedTemporaryFile + +DESC = """ + +Determine whether `bindgen` can successfully process a C or C++ input header. + +First, `bindgen` is run on the input header. Then the emitted bindings are +compiled with `rustc`. Finally, the compiled bindings' layout tests are run. + +By default, this script will exit zero if all of the above steps are successful, +and non-zero if any of them fail. This is appropriate for determining if some +test case (perhaps generated with `csmith` or another fuzzer) uncovers any bugs +in `bindgen`. + +However, this script can also be used when reducing (perhaps with `creduce`) a +known-bad test case into a new, smaller test case that exhibits the same bad +behavior. In this mode, you might expect that the emitted bindings fail to +compile with `rustc`, and want to exit non-zero early if that is not the +case. See the "reducing arguments" for details and what knobs are available. + +""" + +parser = argparse.ArgumentParser( + formatter_class=argparse.RawDescriptionHelpFormatter, + description=DESC.strip()) + +BINDGEN_ARGS = "--with-derive-partialeq \ +--with-derive-eq \ +--with-derive-partialord \ +--with-derive-ord \ +--with-derive-hash \ +--with-derive-default" + +parser.add_argument( + "--bindgen-args", + type=str, + default=BINDGEN_ARGS, + help="An argument string that `bindgen` should be invoked with. By default, all traits are derived. Note that the input header and output bindings file will automatically be provided by this script, and you should not manually specify them.") + +parser.add_argument( + "input", + type=str, + default="input.h", + help="The input header file. Defaults to 'input.h'.") + +REDUCING_DESC = """ + +Arguments that are useful when reducing known-bad test cases into +equivalent-but-smaller test cases that exhibit the same bug with `creduce`. + +""" + +reducing = parser.add_argument_group("reducing arguments", REDUCING_DESC.strip()) + +reducing.add_argument( + "--expect-bindgen-fail", + action="store_true", + help="Exit non-zero if `bindgen` successfully emits bindings.") +reducing.add_argument( + "--bindgen-grep", + type=str, + help="Exit non-zero if the given regexp pattern is not found in `bindgen`'s output.") +reducing.add_argument( + "--bindings-grep", + type=str, + help="Exit non-zero if the given regexp pattern is not found in the emitted bindings.") + +reducing.add_argument( + "--no-compile-bindings", + action="store_false", + dest="rustc", + help="Do not attempt to compile the emitted bindings with `rustc`.") +reducing.add_argument( + "--expect-compile-fail", + action="store_true", + help="Exit non-zero if `rustc` successfully compiles the emitted bindings.") +reducing.add_argument( + "--rustc-grep", + type=str, + help="Exit non-zero if the output from compiling the bindings with `rustc` does not contain the given regexp pattern") + +reducing.add_argument( + "--no-layout-tests", + action="store_false", + dest="layout_tests", + help="Do not run the compiled bindings' layout tests.") +reducing.add_argument( + "--expect-layout-tests-fail", + action="store_true", + help="Exit non-zero if the compiled bindings' layout tests pass.") +reducing.add_argument( + "--layout-tests-grep", + type=str, + help="Exit non-zero if the output of running the compiled bindings' layout tests does not contain the given regexp pattern.") + +################################################################################ + +class ExitOne(Exception): + pass + +def exit_1(msg, child=None): + print(msg) + + if child: + print("---------- stdout ----------------------------------------------") + print(decode(child.stdout)) + print("---------- stderr ----------------------------------------------") + print(decode(child.stderr)) + + raise ExitOne() + +def main(): + args = parser.parse_args() + os.environ["RUST_BACKTRACE"] = "full" + + exit_code = 0 + try: + bindings = new_temp_file(prefix="bindings-", suffix=".rs") + run_bindgen(args, bindings) + + if args.rustc and not args.expect_bindgen_fail: + test_exe = new_temp_file(prefix="layout-tests-") + run_rustc(args, bindings, test_exe) + + if args.layout_tests and not args.expect_compile_fail: + run_layout_tests(args, test_exe) + except ExitOne: + exit_code = 1 + except Exception as e: + exit_code = 2 + print("Unexpected exception:", e) + + for p in TEMP_FILES: + try: + os.remove(path) + except: + pass + + sys.exit(exit_code) + +def run(cmd, **kwargs): + print("Running:", cmd) + return subprocess.run(cmd, **kwargs) + +def decode(f): + return f.decode(encoding="utf-8", errors="ignore") + +TEMP_FILES = [] + +def new_temp_file(prefix, suffix=None): + temp = NamedTemporaryFile(delete=False, prefix=prefix, suffix=suffix) + temp.close() + TEMP_FILES.append(temp.name) + return temp.name + +def contains(pattern, lines): + for line in lines: + if re.match(pattern, line): + return True + return False + +def regexp(pattern): + if not pattern.startswith("^"): + pattern = ".*" + pattern + if not pattern.endswith("$"): + pattern = pattern + ".*" + return re.compile(pattern) + +def run_bindgen(args, bindings): + manifest_path = os.path.abspath(os.path.join(os.path.dirname(sys.argv[0]), + "..", + "Cargo.toml")) + child = run( + ["cargo", "run", + "--manifest-path", manifest_path, + "--", + args.input, "-o", bindings] + shlex.split(args.bindgen_args), + stderr=subprocess.PIPE, + stdout=subprocess.PIPE) + + if args.bindgen_grep: + pattern = regexp(args.bindgen_grep) + if not (contains(pattern, decode(child.stdout).splitlines()) or + contains(pattern, decode(child.stderr).splitlines())): + exit_1("Error: did not find '{}' in `bindgen`'s output".format(args.bindgen_grep), child) + + if args.expect_bindgen_fail and child.returncode == 0: + exit_1("Error: expected running `bindgen` to fail, but it didn't", child) + + if not args.expect_bindgen_fail and child.returncode != 0: + exit_1("Error: running `bindgen` failed", child) + + if args.bindings_grep: + pattern = regexp(args.bindings_grep) + with open(bindings, mode="r") as f: + if not contains(pattern, f): + print("Error: expected the emitted bindings to contain '{}', but they didn't".format(args.bindings_grep)) + print("---------- {} ----------------------------------------------".format(bindings)) + f.seek(0) + print(f.read()) + raise ExitOne() + +def run_rustc(args, bindings, test_exe): + child = run( + ["rustc", "--crate-type", "lib", "--test", "-o", test_exe, bindings], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + + if args.rustc_grep: + pattern = regexp(args.rustc_grep) + if not (contains(pattern, decode(child.stdout).splitlines()) or + contains(pattern, decode(child.stderr).splitlines())): + exit_1("Error: did not find '{}' in `rustc`'s output".format(args.rustc_grep), child) + + if args.expect_compile_fail and child.returncode == 0: + exit_1("Error: expected running `rustc` on the emitted bindings to fail, but it didn't", child) + + if not args.expect_compile_fail and child.returncode != 0: + exit_1("Error: running `rustc` on the emitted bindings failed", child) + +def run_layout_tests(args, test_exe): + child = run( + [test_exe], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + + if args.layout_tests_grep: + pattern = regexp(args.layout_tests_grep) + if not (contains(pattern, decode(child.stdout).splitlines()) or + contains(pattern, decode(child.stderr).splitlines())): + exit_1("Error: did not find '{}' in the compiled bindings' layout tests' output".format(args.layout_tests_grep), child) + + if args.expect_layout_tests_fail and child.returncode == 0: + exit_1("Error: expected running the compiled bindings' layout tests to fail, but it didn't", child) + + if not args.expect_layout_tests_fail and child.returncode != 0: + exit_1("Error: running the compiled bindings' layout tests failed", child) + +if __name__ == "__main__": + main() diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index baffae17..694157d0 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -1959,6 +1959,10 @@ impl MethodCodegen for Method { _ => panic!("How in the world?"), }; + if let (Abi::ThisCall, false) = (signature.abi(), ctx.options().rust_features().thiscall_abi()) { + return; + } + // Do not generate variadic methods, since rust does not allow // implementing them, and we don't do a good job at it anyway. if signature.is_variadic() { @@ -3072,9 +3076,17 @@ impl TryToRustTy for FunctionSig { let arguments = utils::fnsig_arguments(ctx, &self); let abi = self.abi(); - Ok(quote! { - unsafe extern #abi fn ( #( #arguments ),* ) #ret - }) + match abi { + Abi::ThisCall if !ctx.options().rust_features().thiscall_abi() => { + warn!("Skipping function with thiscall ABI that isn't supported by the configured Rust target"); + Ok(quote::Tokens::new()) + } + _ => { + Ok(quote! { + unsafe extern #abi fn ( #( #arguments ),* ) #ret + }) + } + } } } @@ -3146,6 +3158,10 @@ impl CodeGenerator for Function { } let abi = match signature.abi() { + Abi::ThisCall if !ctx.options().rust_features().thiscall_abi() => { + warn!("Skipping function with thiscall ABI that isn't supported by the configured Rust target"); + return; + } Abi::Unknown(unknown_abi) => { panic!( "Invalid or unknown abi {:?} for function {:?} ({:?})", diff --git a/src/features.rs b/src/features.rs index 29e60ab7..b89185fd 100644 --- a/src/features.rs +++ b/src/features.rs @@ -140,6 +140,8 @@ rust_feature_def!( => untagged_union; /// Constant function ([RFC 911](https://github.com/rust-lang/rfcs/blob/master/text/0911-const-fn.md)) => const_fn; + /// `thiscall` calling convention ([Tracking issue](https://github.com/rust-lang/rust/issues/42202)) + => thiscall_abi; ); impl From<RustTarget> for RustFeatures { @@ -152,6 +154,7 @@ impl From<RustTarget> for RustFeatures { if rust_target >= RustTarget::Nightly { features.const_fn = true; + features.thiscall_abi = true; } features diff --git a/src/ir/function.rs b/src/ir/function.rs index b637f9f1..2eab6638 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -142,6 +142,8 @@ pub enum Abi { Stdcall, /// The "fastcall" ABI. Fastcall, + /// The "thiscall" ABI. + ThisCall, /// The "aapcs" ABI. Aapcs, /// The "win64" ABI. @@ -166,6 +168,7 @@ impl quote::ToTokens for Abi { Abi::C => quote! { "C" }, Abi::Stdcall => quote! { "stdcall" }, Abi::Fastcall => quote! { "fastcall" }, + Abi::ThisCall => quote! { "thiscall" }, Abi::Aapcs => quote! { "aapcs" }, Abi::Win64 => quote! { "win64" }, Abi::Unknown(cc) => panic!( @@ -200,6 +203,7 @@ fn get_abi(cc: CXCallingConv) -> Abi { CXCallingConv_C => Abi::C, CXCallingConv_X86StdCall => Abi::Stdcall, CXCallingConv_X86FastCall => Abi::Fastcall, + CXCallingConv_X86ThisCall => Abi::ThisCall, CXCallingConv_AAPCS => Abi::Aapcs, CXCallingConv_X86_64Win64 => Abi::Win64, other => Abi::Unknown(other), diff --git a/tests/expectations/tests/win32-thiscall_1_0.rs b/tests/expectations/tests/win32-thiscall_1_0.rs new file mode 100644 index 00000000..f84a7005 --- /dev/null +++ b/tests/expectations/tests/win32-thiscall_1_0.rs @@ -0,0 +1,29 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] + + +#[repr(C)] +#[derive(Debug, Default, Copy)] +pub struct Foo { + pub _address: u8, +} +#[test] +fn bindgen_test_layout_Foo() { + assert_eq!( + ::std::mem::size_of::<Foo>(), + 1usize, + concat!("Size of: ", stringify!(Foo)) + ); + assert_eq!( + ::std::mem::align_of::<Foo>(), + 1usize, + concat!("Alignment of ", stringify!(Foo)) + ); +} +impl Clone for Foo { + fn clone(&self) -> Self { + *self + } +} diff --git a/tests/expectations/tests/win32-thiscall_nightly.rs b/tests/expectations/tests/win32-thiscall_nightly.rs new file mode 100644 index 00000000..0fd1c00d --- /dev/null +++ b/tests/expectations/tests/win32-thiscall_nightly.rs @@ -0,0 +1,48 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] +#![cfg(feature = "nightly")] +#![feature(abi_thiscall)] + +#[repr(C)] +#[derive(Debug, Default, Copy)] +pub struct Foo { + pub _address: u8, +} +#[test] +fn bindgen_test_layout_Foo() { + assert_eq!( + ::std::mem::size_of::<Foo>(), + 1usize, + concat!("Size of: ", stringify!(Foo)) + ); + assert_eq!( + ::std::mem::align_of::<Foo>(), + 1usize, + concat!("Alignment of ", stringify!(Foo)) + ); +} +extern "thiscall" { + #[link_name = "\u{1}?test@Foo@@QAEXXZ"] + pub fn Foo_test(this: *mut Foo); +} +extern "thiscall" { + #[link_name = "\u{1}?test2@Foo@@QAEHH@Z"] + pub fn Foo_test2(this: *mut Foo, var: ::std::os::raw::c_int) -> ::std::os::raw::c_int; +} +impl Clone for Foo { + fn clone(&self) -> Self { + *self + } +} +impl Foo { + #[inline] + pub unsafe fn test(&mut self) { + Foo_test(self) + } + #[inline] + pub unsafe fn test2(&mut self, var: ::std::os::raw::c_int) -> ::std::os::raw::c_int { + Foo_test2(self, var) + } +} diff --git a/tests/headers/win32-thiscall_1_0.hpp b/tests/headers/win32-thiscall_1_0.hpp new file mode 100644 index 00000000..5907c76e --- /dev/null +++ b/tests/headers/win32-thiscall_1_0.hpp @@ -0,0 +1,7 @@ +// bindgen-flags: --rust-target 1.0 -- --target=i686-pc-windows-msvc + +class Foo { + public: + void test(); + int test2(int var); +}; diff --git a/tests/headers/win32-thiscall_nightly.hpp b/tests/headers/win32-thiscall_nightly.hpp new file mode 100644 index 00000000..2c9f2f17 --- /dev/null +++ b/tests/headers/win32-thiscall_nightly.hpp @@ -0,0 +1,7 @@ +// bindgen-flags: --rust-target nightly --raw-line '#![cfg(feature = "nightly")]' --raw-line '#![feature(abi_thiscall)]' -- --target=i686-pc-windows-msvc + +class Foo { + public: + void test(); + int test2(int var); +}; |