Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Feb 14, 2022

A disabled enable_fuzz on current master does not mean the the fuzz binary is not compiled. This is confusion, so fix it.

  • enable_fuzz toggles compilation flags for fuzzing and disables all other target. There is no need to print this in the configure result, because the compilation flags are already printed. Also, all other targets are already printed as no.
  • enable_fuzz_binary does what it says it does and is currently not printed. So print it.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

Concept ACK.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK faef344, tested on Linux Mint 20.2 (x86_64):

$ ./configure 2>&1 | grep -i fuzz
  with fuzz binary = yes
  CPPFLAGS        =  -fmacro-prefix-map=$(abs_top_srcdir)=.  -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2  -DHAVE_BUILD_INFO -DPROVIDE_FUZZ_MAIN_FUNCTION
$ ./configure --disable-fuzz-binary 2>&1 | grep -i fuzz
configure: running /bin/bash ./configure --disable-option-checking '--prefix=/usr/local'  '--disable-fuzz-binary' '--disable-shared' '--with-pic' '--enable-benchmark=no' '--enable-module-recovery' '--enable-module-schnorrsig' '--enable-experimental' --cache-file=/dev/null --srcdir=.
  with fuzz binary = no
  CPPFLAGS        =  -fmacro-prefix-map=$(abs_top_srcdir)=.  -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2  -DHAVE_BUILD_INFO -DPROVIDE_FUZZ_MAIN_FUNCTION

Btw, it looks like the setting of the -DPROVIDE_FUZZ_MAIN_FUNCTION flag should be revisited.

@maflcko
Copy link
Member Author

maflcko commented Feb 14, 2022

Btw, it looks like the setting of the -DPROVIDE_FUZZ_MAIN_FUNCTION flag should be revisited.

Why? A binary needs a main function, otherwise it can't be linked. Maybe create a separate issue if there is one?

@hebasto
Copy link
Member

hebasto commented Feb 14, 2022

Btw, it looks like the setting of the -DPROVIDE_FUZZ_MAIN_FUNCTION flag should be revisited.

Why? A binary needs a main function, otherwise it can't be linked. Maybe create a separate issue if there is one?

See #24337.

@maflcko maflcko merged commit 566df80 into bitcoin:master Feb 15, 2022
@maflcko maflcko deleted the 2202-buildFuzz branch February 15, 2022 08:33
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 15, 2022
fanquake added a commit to bitcoin-core/gui that referenced this pull request Apr 11, 2022
…FUNCTION` macro unconditionally

c9c4e6c build: Do not define `PROVIDE_FUZZ_MAIN_FUNCTION` macro unconditionally (Hennadii Stepanov)

Pull request description:

  No need to define the `PROVIDE_FUZZ_MAIN_FUNCTION` macro when the build system has been configured with the `--disable-fuzz-binary` option.

  See bitcoin/bitcoin#24336 (review).

ACKs for top commit:
  MarcoFalke:
    Approach ACK c9c4e6c did not review or test 🐤
  fanquake:
    ACK c9c4e6c Checked that `PROVIDE_FUZZ_MAIN_FUNCTION` isn't defined when configuring with `--disable-fuzz-binary`.

Tree-SHA512: 54fbf02ba9f5ecc61b176b8ea7d05e308788d4de3f97ed40913e731300d9dc0edfdfcbf8e0a6e74cf1b2e2ae63f6208a34e03b9c8d203d070c457c4a7d9b5f2c
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 11, 2022
…` macro unconditionally

c9c4e6c build: Do not define `PROVIDE_FUZZ_MAIN_FUNCTION` macro unconditionally (Hennadii Stepanov)

Pull request description:

  No need to define the `PROVIDE_FUZZ_MAIN_FUNCTION` macro when the build system has been configured with the `--disable-fuzz-binary` option.

  See bitcoin#24336 (review).

ACKs for top commit:
  MarcoFalke:
    Approach ACK c9c4e6c did not review or test 🐤
  fanquake:
    ACK c9c4e6c Checked that `PROVIDE_FUZZ_MAIN_FUNCTION` isn't defined when configuring with `--disable-fuzz-binary`.

Tree-SHA512: 54fbf02ba9f5ecc61b176b8ea7d05e308788d4de3f97ed40913e731300d9dc0edfdfcbf8e0a6e74cf1b2e2ae63f6208a34e03b9c8d203d070c457c4a7d9b5f2c
@bitcoin bitcoin locked and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants