build: Do not define `PROVIDE_FUZZ_MAIN_FUNCTION` macro unconditionally #24337

hebasto · 2022-02-14T12:01:08Z

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

See #24336 (review).

hebasto · 2022-02-17T17:44:51Z

@MarcoFalke

Do you think that the fuzz task timeout is caused by this PR change?

maflcko · 2022-02-17T17:47:53Z

No

fanquake · 2022-02-18T11:54:48Z

This should be combined with the CHECK_RUNTIME_LIB check rather than another enable_fuzz_binary test.

maflcko · 2022-02-18T12:00:15Z

A third refacotring alternative might be to remove it. Then, call AX_CHECK_LINK_FLAG unconditionally?

hebasto · 2022-02-19T20:51:59Z

Updated 4a1e159 -> 7e7ae42 (pr24337.01 -> pr24337.02).

This should be combined with the CHECK_RUNTIME_LIB check rather than another enable_fuzz_binary test.

A third refacotring alternative might be to remove it. Then, call AX_CHECK_LINK_FLAG unconditionally?

Chosen the former suggestion as it assumes minimal diff.

hebasto · 2022-02-20T09:26:54Z

Updated 7e7ae42 -> 8d0683d (pr24337.02 -> pr24337.03): reverted back and rebased.

This should be combined with the CHECK_RUNTIME_LIB check rather than another enable_fuzz_binary test.

Doesn't work. Fails with "multiple definition of `main'" error.

A third refacotring alternative might be to remove it. Then, call AX_CHECK_LINK_FLAG unconditionally?

Doesn't work. The -DPROVIDE_FUZZ_MAIN_FUNCTION macro still defined even configured with --disable-fuzz-binary.

maflcko · 2022-02-20T12:21:18Z

I mean:

if enable_fuzz_binary:
  AX_CHECK_LINK_FLAG
  CHECK_RUNTIME_LIB
fi

hebasto · 2022-02-20T13:15:30Z

I mean:

if enable_fuzz_binary:
  AX_CHECK_LINK_FLAG
  CHECK_RUNTIME_LIB
fi

Thanks! Done: 8d0683d -> 121c4ba (pr24337.03 -> pr24337.04).

hebasto · 2022-02-20T15:20:03Z

I mean:
if enable_fuzz_binary:
  AX_CHECK_LINK_FLAG
  CHECK_RUNTIME_LIB
fi
Thanks! Done: 8d0683d -> 121c4ba (pr24337.03 -> pr24337.04).

Reverted back due to the CI failures.

maflcko · 2022-02-20T16:20:30Z

failures

That's because it incorrectly uses a previously cached, but unrelated result. You can work around that with:

diff --git a/configure.ac b/configure.ac
index b7852ae3b5..efdeb186c3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1300,7 +1300,7 @@ fi
 if test "$enable_fuzz_binary" = "yes"; then
   AC_MSG_CHECKING([whether main function is needed for fuzz binary])
   AX_CHECK_LINK_FLAG(
-    [-fsanitize=$use_sanitizers],
+    [-fsanitize= -fsanitize=$use_sanitizers],
     [AC_MSG_RESULT([no])],
     [AC_MSG_RESULT([yes]); CPPFLAGS="$CPPFLAGS -DPROVIDE_FUZZ_MAIN_FUNCTION"],
     [],

maflcko · 2022-02-20T16:27:42Z

See https://cirrus-ci.com/task/6160465456791552?logs=ci#L2772

checking whether main function is needed for fuzz binary... checking whether the linker accepts -fsanitize=address,integer,undefined... (cached) yes
no

hebasto · 2022-02-20T17:45:44Z

fsanitize=address,integer,undefined... (cached) yes

Yeap, AX_CHECK_LINK_FLAG creates and uses a cached variable ax_cv_check_ldflags_$4_$1.

hebasto · 2022-02-20T19:38:49Z

Updated 8d0683d -> c0a089b (pr24337.03 -> pr24337.05):

addressed @MarcoFalke's comment

DrahtBot · 2022-02-21T00:14:34Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#23969 (build: remove use of TARGET_OS and BUILD_OS by fanquake)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

hebasto · 2022-04-05T17:51:55Z

Rebased c0a089b -> 3ec7dc7 (pr24337.05 -> pr24337.06).

configure.ac

hebasto · 2022-04-06T07:18:28Z

Updated 3ec7dc7 -> c9c4e6c (pr24337.06 -> pr24337.07, diff):

addressed @fanquake's comment

DrahtBot · 2022-04-10T10:43:13Z

Guix builds

File	commit `38d3d0b` (master)	commit `7b2fdd9` (master and this pull)
SHA256SUMS.part	`5b338da8888ed74e...`	`2e10517f4f3fcf8e...`
*-aarch64-linux-gnu-debug.tar.gz	`169181d915ec8a1b...`	`a943369a6fbd0320...`
*-aarch64-linux-gnu.tar.gz	`7ceb686ca03481e2...`	`4110ac2ed8cb2b1b...`
*-arm-linux-gnueabihf-debug.tar.gz	`322bc373e08936dc...`	`6fa23007a6127157...`
*-arm-linux-gnueabihf.tar.gz	`1312ea4bb32a2e95...`	`f8d9f25ffc7afe69...`
*-arm64-apple-darwin-unsigned.dmg	`1f14f9b49309b65a...`	`38418789670b459b...`
*-arm64-apple-darwin-unsigned.tar.gz	`09abc3c6b900ccd2...`	`52fe888221dc5019...`
*-arm64-apple-darwin.tar.gz	`6bbdb6f1b9470a8e...`	`382a64f73a31b6f2...`
*-powerpc64-linux-gnu-debug.tar.gz	`8eb5fdd4a09e3848...`	`da2ce5ce8b752d8c...`
*-powerpc64-linux-gnu.tar.gz	`22926c107bc7180f...`	`92e8c7d1f73a00c1...`
*-powerpc64le-linux-gnu-debug.tar.gz	`2ce49961659115f8...`	`2521db9010aabd42...`
*-powerpc64le-linux-gnu.tar.gz	`a4e7ad11fe61f2d3...`	`3166a101fe257919...`
*-riscv64-linux-gnu-debug.tar.gz	`22f977cf97993240...`	`b7ed43b8cd7a4cde...`
*-riscv64-linux-gnu.tar.gz	`42f28fe61dee331f...`	`268a612a528635b6...`
*-win64-debug.zip	`cc3ddc0b1b213e13...`	`089f46fb944a4c74...`
*-win64-setup-unsigned.exe	`9434f90b613e9ff6...`	`101bc0ab4983a989...`
*-win64-unsigned.tar.gz	`0d3362db53cd0e0c...`	`fd184459eb9e63aa...`
*-win64.zip	`81e85958fe5ef86b...`	`5c3a579ec9ef27ff...`
*-x86_64-apple-darwin-unsigned.dmg	`74b78a90e14e8026...`	`ab58b53d274fbc45...`
*-x86_64-apple-darwin-unsigned.tar.gz	`6be471a6518fc661...`	`3b4f5d89aeed9184...`
*-x86_64-apple-darwin.tar.gz	`13253355fb41e82a...`	`dbffa5a6738b4ea4...`
*-x86_64-linux-gnu-debug.tar.gz	`9df8f207b22ad2d6...`	`e9c30b48a0573a78...`
*-x86_64-linux-gnu.tar.gz	`79347964dbbc0281...`	`9d539b8652bd3e66...`
*.tar.gz	`1ff1ea0b99723d31...`	`29ddf1f0a991e217...`
guix_build.log	`3c7b9e6a0ca97995...`	`deee81a07ee46bf4...`
guix_build.log.diff		`ee7f2c1b8a55de45...`

maflcko · 2022-04-11T09:38:46Z

Approach ACK c9c4e6c did not review or test 🐤

Show signature

Signature:

fanquake

ACK c9c4e6c Checked that PROVIDE_FUZZ_MAIN_FUNCTION isn't defined when configuring with --disable-fuzz-binary.

…` 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

hebasto mentioned this pull request Feb 14, 2022

Print enable_fuzz_binary in configure #24336

Merged

DrahtBot added the Build system label Feb 14, 2022

hebasto force-pushed the 220214-fuzz branch from 4a1e159 to 7e7ae42 Compare February 19, 2022 20:49

hebasto force-pushed the 220214-fuzz branch from 7e7ae42 to 8d0683d Compare February 20, 2022 09:21

hebasto force-pushed the 220214-fuzz branch from 8d0683d to 121c4ba Compare February 20, 2022 13:13

hebasto changed the title ~~build: Define PROVIDE_FUZZ_MAIN_FUNCTION macro only when needed~~ build: Do not define PROVIDE_FUZZ_MAIN_FUNCTION macro unconditionally Feb 20, 2022

hebasto marked this pull request as draft February 20, 2022 14:46

hebasto marked this pull request as ready for review February 20, 2022 15:19

hebasto force-pushed the 220214-fuzz branch from 121c4ba to 8d0683d Compare February 20, 2022 15:19

hebasto force-pushed the 220214-fuzz branch from 8d0683d to c0a089b Compare February 20, 2022 17:50

DrahtBot mentioned this pull request Feb 20, 2022

build: stop overriding user autoconf flags #24391

Merged

This was referenced Feb 21, 2022

build: Bump minimum Qt version to 5.11.3 #24132

Merged

build: remove use of TARGET_OS and BUILD_OS #23969

Closed

DrahtBot added the Needs rebase label Mar 7, 2022

hebasto force-pushed the 220214-fuzz branch from c0a089b to 3ec7dc7 Compare April 5, 2022 17:50

maflcko reviewed Apr 5, 2022

View reviewed changes

configure.ac Outdated Show resolved Hide resolved

fanquake reviewed Apr 5, 2022

View reviewed changes

configure.ac Outdated Show resolved Hide resolved

DrahtBot removed the Needs rebase label Apr 5, 2022

hebasto force-pushed the 220214-fuzz branch from 3ec7dc7 to 1a3c63c Compare April 6, 2022 06:44

build: Do not define PROVIDE_FUZZ_MAIN_FUNCTION macro unconditionally

c9c4e6c

hebasto force-pushed the 220214-fuzz branch from 1a3c63c to c9c4e6c Compare April 6, 2022 06:55

maflcko added the DrahtBot Guix build requested label Apr 6, 2022

DrahtBot removed the DrahtBot Guix build requested label Apr 10, 2022

maflcko requested a review from fanquake April 11, 2022 09:38

fanquake approved these changes Apr 11, 2022

View reviewed changes

fanquake merged commit 7626e54 into bitcoin:master Apr 11, 2022

hebasto deleted the 220214-fuzz branch April 11, 2022 15:52

bitcoin locked and limited conversation to collaborators Apr 11, 2023

build: Do not define PROVIDE_FUZZ_MAIN_FUNCTION macro unconditionally #24337

build: Do not define PROVIDE_FUZZ_MAIN_FUNCTION macro unconditionally #24337

Uh oh!

Conversation

hebasto commented Feb 14, 2022

Uh oh!

hebasto commented Feb 17, 2022

Uh oh!

maflcko commented Feb 17, 2022

Uh oh!

fanquake commented Feb 18, 2022

Uh oh!

maflcko commented Feb 18, 2022

Uh oh!

hebasto commented Feb 19, 2022

Uh oh!

hebasto commented Feb 20, 2022

Uh oh!

maflcko commented Feb 20, 2022

Uh oh!

hebasto commented Feb 20, 2022

Uh oh!

hebasto commented Feb 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Feb 20, 2022

Uh oh!

maflcko commented Feb 20, 2022

Uh oh!

hebasto commented Feb 20, 2022

Uh oh!

hebasto commented Feb 20, 2022

Uh oh!

DrahtBot commented Feb 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

hebasto commented Apr 5, 2022

Uh oh!

Uh oh!

Uh oh!

hebasto commented Apr 6, 2022

Uh oh!

DrahtBot commented Apr 10, 2022

Guix builds

Uh oh!

maflcko commented Apr 11, 2022

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

build: Do not define `PROVIDE_FUZZ_MAIN_FUNCTION` macro unconditionally #24337

build: Do not define `PROVIDE_FUZZ_MAIN_FUNCTION` macro unconditionally #24337

hebasto commented Feb 20, 2022 •

edited

Loading

DrahtBot commented Feb 21, 2022 •

edited

Loading