Skip to content

tests: Don't assume presence of __builtin_mul_overflow(…) in MultiplicationOverflow(…) fuzzing harness #18393

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.

Sign up for GitHub

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

Jump to bottom
Merged
maflcko merged 1 commit into from
Mar 23, 2020
Merged

tests: Don't assume presence of __builtin_mul_overflow(…) in MultiplicationOverflow(…) fuzzing harness #18393

maflcko merged 1 commit into from
Mar 23, 2020

Conversation

@practicalswift
Copy link
Contributor

@practicalswift practicalswift commented Mar 20, 2020

Don't assume presence of __builtin_mul_overflow(…) in MultiplicationOverflow(…) fuzzing harness.

Fixes #18389.

@practicalswift practicalswift mentioned this pull request Mar 20, 2020
Closed
@practicalswift practicalswift force-pushed the multiplication-overflow-fixup branch from 405e883 to 3748e1d Compare March 20, 2020 16:25
@naumenkogs
Copy link
Contributor

naumenkogs commented Mar 20, 2020 •
edited
Loading

Using 3748e1daec3f3b597e642d5eb86adb21fade6e36, got this with gcc:

 #if defined(__has_builtin) && __has_builtin(__builtin_mul_overflow)
                                            ^
In file included from ./util/system.h:22:0,
                 from ./addrman.h:15,
                 from test/fuzz/deserialize.cpp:6:```

@DrahtBot DrahtBot added the Tests label Mar 20, 2020
@practicalswift practicalswift force-pushed the multiplication-overflow-fixup branch from 3748e1d to 5ff2ee1 Compare March 21, 2020 08:24
@practicalswift
Copy link
Contributor Author

practicalswift commented Mar 21, 2020

@naumenkogs Oh, thanks! Clang-isms now removed :) Please re-review!

@naumenkogs
Copy link
Contributor

naumenkogs commented Mar 21, 2020 •
edited
Loading 

test/fuzz/multiplication_overflow.cpp: In instantiation of ‘void {anonymous}::TestMultiplicationOverflow(FuzzedDataProvider&) [with T = bool]’:
test/fuzz/multiplication_overflow.cpp:55:58:   required from here
test/fuzz/multiplication_overflow.cpp:30:75: error: argument 3 in call to function ‘__builtin_mul_overflow’ does not have pointer to integer type
     const bool is_multiplication_overflow_builtin = __builtin_mul_overflow(i, j, &result_builtin);
                                                                           ^
afl-as 2.52b by <lcamtuf@google.com>

I also noticed that I have gcc 5.4.0. Perhaps I should just update my compilers? I dunno what are the expectations.

@naumenkogs
Copy link
Contributor

naumenkogs commented Mar 21, 2020 •
edited
Loading

Updated both compilers, didn't help with this particular issue I mention above.

Update:
Alright, got gcc-9.2.1.
Seeing this instead:

[+] Instrumented 2039 locations (64-bit, hardened mode, ratio 100%).
test/fuzz/multiplication_overflow.cpp: In instantiation of ‘void {anonymous}::TestMultiplicationOverflow(FuzzedDataProvider&) [with T = bool]’:
test/fuzz/multiplication_overflow.cpp:55:58:   required from here
test/fuzz/multiplication_overflow.cpp:30:83: error: argument 3 in call to function ‘__builtin_mul_overflow’ has pointer to boolean type
   30 |     const bool is_multiplication_overflow_builtin = __builtin_mul_overflow(i, j, &result_builtin);
      |                                                                                   ^~~~~~~~~~~~~~
afl-as 2.52b by <lcamtuf@google.com>

@practicalswift practicalswift force-pushed the multiplication-overflow-fixup branch from 5ff2ee1 to 7c1ac70 Compare March 22, 2020 13:29
@practicalswift
Copy link
Contributor Author

practicalswift commented Mar 22, 2020

@naumenkogs Thanks for testing! Hopefully fixed now. Would you mind testing the latest version? :)

@naumenkogs
Copy link
Contributor

naumenkogs commented Mar 22, 2020

ACK 7c1ac70

@maflcko maflcko merged commit 97b0687 into bitcoin:master Mar 23, 2020
@MarkLTZ MarkLTZ mentioned this pull request Apr 4, 2020
Closed
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jan 20, 2021
@practicalswift @Fabcien
tests: Don't assume presence of __builtin_mul_overflow in Multiplicat…
2fb7c38 
…ionOverflow(...) fuzzing harness

Summary:
Backport of core [[bitcoin/bitcoin#18393 | PR18393]].

Depends on D8967.

Test Plan:
I cannot reproduce the issue myself, but I can check it doesn't break
the fuzzer:
  ninja bitcoin-fuzzers
  ./test/fuzz/test_runner.py <path_to_corpus> multiplication_overflow

Reviewers: #bitcoin_abc, PiRK

Reviewed By: #bitcoin_abc, PiRK

Differential Revision: https://reviews.bitcoinabc.org/D8969
@practicalswift practicalswift deleted the multiplication-overflow-fixup branch April 10, 2021 19:40
kwvg added a commit to kwvg/dash that referenced this pull request Jul 15, 2022
@kwvg
merge bitcoin#18393: Don't assume presence of __builtin_mul_overflow …
bf04694 
…in MultiplicationOverflow(...) fuzzing harness
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Jul 17, 2022
@PastaPastaPasta
Merge pull request #4886 from kittywhiskers/fuzz3
30d6584 
backport: bitcoin#18867, bitcoin#19247, bitcoin#19222, bitcoin#18363, bitcoin#18190, bitcoin#18393, partial bitcoin#18047, bitcoin#18314, bitcoin#19143 (fuzzing harness backports: part 3)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fuzzing: compilation error ‘__builtin_mul_overflow’ does not have pointer to integer type

4 participants

@practicalswift @naumenkogs @maflcko @DrahtBot