Skip to content
This repository was archived by the owner on Sep 4, 2024. It is now read-only.

Conversation

@tcharding
Copy link
Collaborator

@tcharding tcharding commented May 27, 2023

Fix fuzzing introduced in #94. Patch one is cleanup, patch 2 is the fix.

Hey @apoelstra, you could consider just merging #100 since it includes this.

tcharding added 2 commits May 27, 2023 12:31
No manual changes, just run the script so we are in a "clean" state.
The `generate-files.sh` script was copied from `rust-bitcoin`, looks
like who ever copied it forgot to read it (*cough* Tobin) ;)

Use yaml pipe syntax to write multiline shell code; use the correct cfg
flags, echoing them before running the fuzz script.
@tcharding tcharding mentioned this pull request May 27, 2023
@tcharding
Copy link
Collaborator Author

CI is failing, @apoelstra:

  • Why do we fuzz with Rust 1.58?
  • The build fail does not happen with later versions, stable 1.69 in particular
  • Any reason to not use stable so I can be lazy and not investigate why this started failing right now when the code is not new code (the mutex stuff in simple_http anyways)?

@apoelstra
Copy link
Owner

Why do we fuzz with Rust 1.58?

Because this is the version that works with honggfuzz. Stable stopped working (at least for a while) and earlier versions don't work.

The build fail does not happen with later versions, stable 1.69 in particular

Yeah, we can switch to 1.69 for this crate. It looks like I'm using 1.64 locally, which also works.

Any reason to not use stable so I can be lazy and not investigate why this started failing right now when the code is not new code (the mutex stuff in simple_http anyways)?

It started failing now because this is the first time we're running the fuzz tests in CI, and the offending code is gated on the fuzz cfg flag. I encountered this a few weeks ago after merging #98, and forgot about it, sorry.

But no, let's not use stable, that'll break. Let's just pick a version that works.

Rust 1.58 doesn't like usage of `Mutex`, switching to Rust 1.69 for
fuzzing resolves the issue.

Do not use `stable` because it has broken in the past with `hongfuzz`.
@tcharding
Copy link
Collaborator Author

All green - party.

@tcharding tcharding mentioned this pull request May 28, 2023
Copy link
Owner

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 87338f3

@apoelstra
Copy link
Owner

Will just review/merge #100.

@apoelstra apoelstra closed this May 28, 2023
apoelstra added a commit that referenced this pull request May 28, 2023
bdda5e8 bump version to 0.15.0 (Tobin C. Harding)
87338f3 Use Rust 1.69 to run the fuzzer (Tobin C. Harding)
89930cc fuzz: Use correct cfg flags (Tobin C. Harding)
d50b5a8 Run fuzz/generate-files.sh (Tobin C. Harding)

Pull request description:

  Release 0.15.0 - like its our job!

  On top of #99, needs rebase after that merges.

ACKs for top commit:
  apoelstra:
    ACK bdda5e8

Tree-SHA512: df2fdaf6f2b53466c20f1d3dccca485da5f7baeed03d1369ad6c1a6c941b7a1842cabf7e3fa93b99591265ded017fb8247602414213d4cce84f6a3cf36452fc2
@tcharding tcharding deleted the 05-26-fuzz-yaml-bug branch June 1, 2023 06:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants