Skip to content
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.

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

build: Run functional tests and benchmarks under the undefined behaviour sanitizer (UBSan) #14252

Merged
merged 1 commit into from Nov 5, 2018

Conversation

Projects
None yet
5 participants
@practicalswift
Copy link
Member

commented Sep 18, 2018

Run functional tests and benchmarks under the undefined behaviour sanitizer (UBSan).

This will make Travis automatically detect issues such as:

  • #14242: Avoid triggering undefined behaviour (std::memset(nullptr, 0, 0)) if an invalid string is passed to DecodeSecret(...)
  • #14239: Avoid dividing by zero (undefined behaviour) in EstimateMedianVal (policy)/ConnectTip (validation)/CreateTransaction (wallet)
  • #13546: wallet: Avoid potential use of uninitialized value bnb_used in CWallet::CreateTransaction(...)

Addresses issue #14059.

@practicalswift practicalswift force-pushed the practicalswift:ubsan branch from 3cdb4bd Sep 18, 2018

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Sep 18, 2018

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #14528 (travis: Compile once on xenial by MarcoFalke)
  • #14489 (refactor: Drop boost::thread and boost::chrono by ken2812221)
  • #14092 (tests: Dry run bench_bitcoin as part "make check" to allow for quick identification of assertion/sanitizer failures in benchmarking code by practicalswift)
  • #12134 (Build previous releases and run functional tests by Sjors)

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.

NO_DEPENDS=1
GOAL="install"
BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --enable-glibc-back-compat --enable-reduce-exports --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER"
# x86_64 Linux (sanitizers)

This comment has been minimized.

Copy link
@ken2812221

ken2812221 Sep 19, 2018

Member

Why do you remove this job?

This comment has been minimized.

Copy link
@practicalswift

practicalswift Sep 19, 2018

Author Member

It is not removed - the two jobs are merged in accordance to this comment:

Disabled for now, can be combined with the other x86_64 linux NO_DEPENDS job
when functional tests pass the sanitizers

Let me know if you see any technical reasons for not doing that job merge :-)

@practicalswift

This comment has been minimized.

Copy link
Member Author

commented Sep 27, 2018

@MarcoFalke Would you mind reviewing? :-) This would help solve the issue #14059 which you posted :-)

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Sep 28, 2018

Coverage Change (pull 14252) Reference (master)
Lines +0.0022 % 87.0361 %
Functions +0.0463 % 84.1130 %
Branches -0.0076 % 51.5451 %
@practicalswift

This comment has been minimized.

Copy link
Member Author

commented Nov 5, 2018

Rebased!

@DrahtBot DrahtBot added Needs rebase and removed Needs rebase labels Nov 5, 2018

@practicalswift practicalswift force-pushed the practicalswift:ubsan branch 2 times, most recently Nov 5, 2018

Show resolved Hide resolved .travis.yml Outdated

@practicalswift practicalswift force-pushed the practicalswift:ubsan branch Nov 5, 2018

@DrahtBot DrahtBot removed the Needs rebase label Nov 5, 2018

Show resolved Hide resolved .travis.yml Outdated
Enable functional tests in UBSAN job. Enable -fsanitize=integer (part…
… of UBSAN). Merge UBSAN Travis job with no depends.

@practicalswift practicalswift force-pushed the practicalswift:ubsan branch to 9f49db7 Nov 5, 2018

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Nov 5, 2018

utACK 9f49db7

@MarcoFalke MarcoFalke merged commit 9f49db7 into bitcoin:master Nov 5, 2018

1 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Nov 5, 2018

Merge #14252: build: Run functional tests and benchmarks under the un…
…defined behaviour sanitizer (UBSan)

9f49db7 Enable functional tests in UBSAN job. Enable -fsanitize=integer (part of UBSAN). Merge UBSAN Travis job with no depends. (practicalswift)

Pull request description:

  Run functional tests and benchmarks under the undefined behaviour sanitizer (UBSan).

  This will make Travis automatically detect issues such as:
  * #14242: Avoid triggering undefined behaviour (`std::memset(nullptr, 0, 0)`) if an invalid string is passed to `DecodeSecret(...)`
  * #14239: Avoid dividing by zero (undefined behaviour) in `EstimateMedianVal` (policy)/`ConnectTip` (validation)/`CreateTransaction` (wallet)
  * #13546: wallet: Avoid potential use of uninitialized value `bnb_used` in `CWallet::CreateTransaction(...)`

  Addresses issue #14059.

Tree-SHA512: 285e1542b36c582516c47938ce8d999fd89ba6c867bc0976e7306e7c949b8b84ffbfa43dbc679dd97ae639b086092e7d799d8e1c903c66a37d529ce61d5c64b4

MarcoFalke added a commit that referenced this pull request Nov 6, 2018

Merge #14673: travis: Fail the UBSan Travis build in case of newly in…
…troduced UBSan errors

4773fa8 Add llvm-symbolizer directory to PATH. Needed to get symbolized stack traces from the sanitizers. (practicalswift)
5c292da Add UBSan suppressions needed to pass test suite (practicalswift)
fced6b5 Add UBSan options: print_stacktrace + halt_on_error (practicalswift)

Pull request description:

  Fail the UBSan Travis build in case of newly introduced [UBSan (UndefinedBehaviorSanitizer)](https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html) errors.

  Prior to this commit new UBSan errors were printed but didn't fail the UBSan Travis build.

  Changes:
  * Travis: Add UBSan options: `print_stacktrace` + `halt_on_error`
  * Travis: Add UBSan suppressions needed to pass test suite
  * Travis: Add `llvm-symbolizer` directory to PATH. Needed to get symbolized stack traces from the sanitizers.

  `halt_on_error` should have been part of #14252 really :-)

Tree-SHA512: 30e960659196873d4f636f3a61267b8b4441a0e8773e3f3ae4660a9341d028c363636f0cb919ef9d6662ceb484e3d58054adfb6dc76ff8a355a1c9f927c328d1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.