Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix fuzztarget symbols #191

Merged
merged 2 commits into from Jan 9, 2020
Merged

Conversation

@elichai
Copy link
Contributor

elichai commented Jan 9, 2020

This is why rust-bitcoin/rust-bitcoin#380 tests fail.

I fixed the linking but changed 1 logic thing.
I replaced secp256k1_context_no_precomp from being linked to the actual secp256k1_context to just be a pointer to a local instance of our Context.
I did it for 2 reasons:

  1. Maybe we can skip even compiling libsecp for the fuzztarget one day.
  2. Avoid a theoretical UB, i.e. our Context struct doesn't match secp256k1_context. it's meant to be used just as an Opaque pointer. but the fuzztarget actually dereferences that pointer and access the c_uint parameter in there. which might correspond to some padding in secp256k1_context -> access uninitialized values.

I wanted to also replace the 2 function pointers with null but apparently rust doesn't allow null function pointers https://rust-lang.github.io/unsafe-code-guidelines/layout/function-pointers.html

@elichai elichai force-pushed the elichai:2020-01-fuzztarget branch from 984ae9f to 088a89d Jan 9, 2020
@elichai

This comment has been minimized.

Copy link
Contributor Author

elichai commented Jan 9, 2020

I replaced with cargo test --no-run --features=fuzztarget so that a binary will be created, pass the linker. because the tests will fail.
checked that this fails without fixing the links.

@elichai elichai force-pushed the elichai:2020-01-fuzztarget branch 2 times, most recently from 2e3fa77 to 417941f Jan 9, 2020
@elichai elichai force-pushed the elichai:2020-01-fuzztarget branch from 417941f to 89271c9 Jan 9, 2020
Copy link
Contributor

stevenroose left a comment

ACK!

@apoelstra apoelstra merged commit 5c82bb4 into rust-bitcoin:master Jan 9, 2020
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@apoelstra

This comment has been minimized.

Copy link
Member

apoelstra commented Jan 9, 2020

Merged despite emscripten travis failure because we believe this is a bug upstream (in rustc)

@elichai elichai deleted the elichai:2020-01-fuzztarget branch Jan 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.