Skip to content

fuzz: Apply fuzz env (suppressions, etc.) when fetching harness list#29583

Merged
fanquake merged 1 commit intobitcoin:masterfrom
dergoegge:2024-03-ubsan-sup
Mar 8, 2024
Merged

fuzz: Apply fuzz env (suppressions, etc.) when fetching harness list#29583
fanquake merged 1 commit intobitcoin:masterfrom
dergoegge:2024-03-ubsan-sup

Conversation

@dergoegge
Copy link
Member

@dergoegge dergoegge commented Mar 7, 2024

The fuzz test runner does not add the UBSan suppressions when fetching the harness list. We can observe this in CI as lots of UBSan errors prior to the harnesses actually executing: https://api.cirrus-ci.com/v1/task/5678606140047360/logs/ci.log

+ test/fuzz/test_runner.py -j10 -l DEBUG /ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/ --empty_min_time=60
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:38: runtime error: unsigned integer overflow: 12 - 23 cannot be represented in type 'size_type' (aka 'unsigned long')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:38 in 
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:33: runtime error: implicit conversion from type 'size_type' (aka 'unsigned long') of value 18446744073709551605 (64-bit, unsigned) to type 'const difference_type' (aka 'const long') changed the value to -11 (64-bit, signed)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:33 in 
crypto/sha256.cpp:75:57: runtime error: left shift of 1359893119 by 26 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:75:57 in 
crypto/sha256.cpp:75:79: runtime error: left shift of 1359893119 by 21 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:75:79 in 
crypto/sha256.cpp:75:101: runtime error: left shift of 1359893119 by 7 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:75:101 in 
crypto/sha256.cpp:82:47: runtime error: unsigned integer overflow: 2968370640 + 2483695512 cannot be represented in type 'uint32_t' (aka 'unsigned int')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:82:47 in 
crypto/sha256.cpp:74:57: runtime error: left shift of 1779033703 by 30 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:74:57 in 
crypto/sha256.cpp:74:79: runtime error: left shift of 1779033703 by 19 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:74:79 in 
crypto/sha256.cpp:74:101: runtime error: left shift of 1779033703 by 10 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:74:101 in 
crypto/sha256.cpp:83:29: runtime error: unsigned integer overflow: 3458249854 + 980412007 cannot be represented in type 'uint32_t' (aka 'unsigned int')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:83:29 in 
crypto/sha256.cpp:82:21: runtime error: unsigned integer overflow: 528734635 + 4228187651 cannot be represented in type 'uint32_t' (aka 'unsigned int')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:82:21 in 
crypto/sha256.cpp:84:7: runtime error: unsigned integer overflow: 1013904242 + 3720769133 cannot be represented in type 'uint32_t' (aka 'unsigned int')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:84:7 in 
crypto/sha256.cpp:85:12: runtime error: unsigned integer overflow: 3720769133 + 2654153126 cannot be represented in type 'uint32_t' (aka 'unsigned int')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:85:12 in 
crypto/sha256.cpp:82:33: runtime error: unsigned integer overflow: 4165002546 + 1259303586 cannot be represented in type 'uint32_t' (aka 'unsigned int')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:82:33 in 
crypto/sha256.cpp:125:50: runtime error: unsigned integer overflow: 3835390401 + 1367343104 cannot be represented in type 'unsigned int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:125:50 in 
crypto/sha256.cpp:77:58: runtime error: left shift of 1367343104 by 15 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
...

To fix this we simply apply the usual fuzz env variables (that apply the suppressions) when fetching the harness list as well.

@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 7, 2024 •
edited
Loading

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK ismaelsadeeq, fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@DrahtBot DrahtBot added the Tests label Mar 7, 2024
@dergoegge
Copy link
Member Author

dergoegge commented Mar 7, 2024

No more ubsan errors with this PR: https://cirrus-ci.com/task/4545546522722304?logs=ci#L1995

ismaelsadeeq
ismaelsadeeq reviewed Mar 7, 2024
View reviewed changes
Copy link
Member

@ismaelsadeeq ismaelsadeeq left a comment

Choose a reason for hiding this comment

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

Tested ACK 738a537

I was running the fuzz tests locally and noticed same behavior, I can confirm that 738a537 suppress the warnings.

fanquake
fanquake approved these changes Mar 8, 2024
View reviewed changes
Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK 738a537

@fanquake fanquake merged commit c49450d into bitcoin:master Mar 8, 2024
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 24, 2024
@fanquake @PastaPastaPasta
Merge bitcoin#29583: fuzz: Apply fuzz env (suppressions, etc.) when f…
8d6e5e7 
…etching harness list

738a537 [fuzz] Apply fuzz env (suppressions, etc.) when fetching harness list (dergoegge)

Pull request description:

  The fuzz test runner does not add the UBSan suppressions when fetching the harness list. We can observe this in CI as lots of UBSan errors prior to the harnesses actually executing: https://api.cirrus-ci.com/v1/task/5678606140047360/logs/ci.log

  ```
  + test/fuzz/test_runner.py -j10 -l DEBUG /ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/ --empty_min_time=60
  /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:38: runtime error: unsigned integer overflow: 12 - 23 cannot be represented in type 'size_type' (aka 'unsigned long')
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:38 in
  /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:33: runtime error: implicit conversion from type 'size_type' (aka 'unsigned long') of value 18446744073709551605 (64-bit, unsigned) to type 'const difference_type' (aka 'const long') changed the value to -11 (64-bit, signed)
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:33 in
  crypto/sha256.cpp:75:57: runtime error: left shift of 1359893119 by 26 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:75:57 in
  crypto/sha256.cpp:75:79: runtime error: left shift of 1359893119 by 21 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:75:79 in
  crypto/sha256.cpp:75:101: runtime error: left shift of 1359893119 by 7 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:75:101 in
  crypto/sha256.cpp:82:47: runtime error: unsigned integer overflow: 2968370640 + 2483695512 cannot be represented in type 'uint32_t' (aka 'unsigned int')
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:82:47 in
  crypto/sha256.cpp:74:57: runtime error: left shift of 1779033703 by 30 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:74:57 in
  crypto/sha256.cpp:74:79: runtime error: left shift of 1779033703 by 19 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:74:79 in
  crypto/sha256.cpp:74:101: runtime error: left shift of 1779033703 by 10 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:74:101 in
  crypto/sha256.cpp:83:29: runtime error: unsigned integer overflow: 3458249854 + 980412007 cannot be represented in type 'uint32_t' (aka 'unsigned int')
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:83:29 in
  crypto/sha256.cpp:82:21: runtime error: unsigned integer overflow: 528734635 + 4228187651 cannot be represented in type 'uint32_t' (aka 'unsigned int')
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:82:21 in
  crypto/sha256.cpp:84:7: runtime error: unsigned integer overflow: 1013904242 + 3720769133 cannot be represented in type 'uint32_t' (aka 'unsigned int')
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:84:7 in
  crypto/sha256.cpp:85:12: runtime error: unsigned integer overflow: 3720769133 + 2654153126 cannot be represented in type 'uint32_t' (aka 'unsigned int')
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:85:12 in
  crypto/sha256.cpp:82:33: runtime error: unsigned integer overflow: 4165002546 + 1259303586 cannot be represented in type 'uint32_t' (aka 'unsigned int')
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:82:33 in
  crypto/sha256.cpp:125:50: runtime error: unsigned integer overflow: 3835390401 + 1367343104 cannot be represented in type 'unsigned int'
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:125:50 in
  crypto/sha256.cpp:77:58: runtime error: left shift of 1367343104 by 15 places cannot be represented in type 'uint32_t' (aka 'unsigned int')
  ...
  ```

  To fix this we simply apply the usual fuzz env variables (that apply the suppressions) when fetching the harness list as well.

ACKs for top commit:
  ismaelsadeeq:
    Tested ACK 738a537
  fanquake:
    ACK 738a537

Tree-SHA512: befebaeb4ee5f2eddca67fc6dc69e997c6a250ea54844e5e6e93d1f6a13be49364a3ace31eaa942b02dcf73612af29ec4ace86c9eb7567b92f6f5dc3ea14dc11
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Oct 24, 2024
@PastaPastaPasta
Merge #6345: backport: trivial 2024 10 23 pr1
aaccc9e 
b70e091 Merge bitcoin#29667: fuzz: actually test garbage >64b in p2p transport test (fanquake)
6d7aa3d Merge bitcoin#29497: test: simplify test_runner.py (fanquake)
d0e15d5 Merge bitcoin#29606: refactor: Reserve memory for ToLower/ToUpper conversions (Ava Chow)
045fa5f Merge bitcoin#29514: tests: Provide more helpful assert_equal errors (Ava Chow)
bd607f0 Merge bitcoin#29393: i2p: log connection was refused due to arbitrary port (Ava Chow)
c961755 Merge bitcoin#29595: doc: Wrap flags with code in developer-notes.md (fanquake)
8d6e5e7 Merge bitcoin#29583: fuzz: Apply fuzz env (suppressions, etc.) when fetching harness list (fanquake)
4dce690 Merge bitcoin#29576: Update functional test runner to return error code when no tests are found to run (fanquake)
910a7d6 Merge bitcoin#29529: fuzz: restrict fopencookie usage to Linux & FreeBSD (fanquake)
fdac2b3 Merge bitcoin#29493: subtree: update crc32c subtree (fanquake)
a23b342 Merge bitcoin#29475: doc: Fix Broken Links (fanquake)
92bad90 Merge bitcoin#28178: fuzz: Generate with random libFuzzer settings (fanquake)
9b6a05d Merge bitcoin#29443: depends: fix BDB compilation on OpenBSD (fanquake)
9963e6b Merge bitcoin#29413: fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse` (fanquake)
3914745 Merge bitcoin#29425: test: fix intermittent failure in wallet_reorgrestore.py (fanquake)
b719883 Merge bitcoin#29399: test: Fix utxo set hash serialisation signedness (fanquake)
f096880 Merge bitcoin#29377: test: Add makefile target for running unit tests (Ava Chow)
03e0bd3 Merge bitcoin#27319: addrman, refactor: improve stochastic test in `AddSingle` (Ava Chow)

Pull request description:

  ## Issue being fixed or feature implemented
  Batch of trivial backports

  ## What was done?
  See commits

  ## How Has This Been Tested?
  built locally; large combined merge passed tests locally

  ## Breaking Changes
  Should be none

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK b70e091
  knst:
    utACK b70e091

Tree-SHA512: 659a931f812c8a92cf3854abb873d92885219a392d6aa8e49ac4b27fe41e8e163ef9a135050e7c2e1bd33cecd2f7dae215e05a9c29f62e837e0057d3c16746d6
@bitcoin bitcoin locked and limited conversation to collaborators Mar 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@fanquake fanquake fanquake approved these changes

+1 more reviewer

@ismaelsadeeq ismaelsadeeq ismaelsadeeq left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dergoegge @DrahtBot @fanquake @ismaelsadeeq