Page MenuHomePhabricator
Differential D3074

Fix undefined behavior in avalanche.cpp
ClosedPublic
Actions

Authored by Fabien on May 20 2019, 11:39.

Details

Reviewers
deadalnix
markblundeberg
Group Reviewers
Restricted Project
Commits
rSTAGINGd478be9e220e: Fix undefined behavior in avalanche.cpp
rABCd478be9e220e: Fix undefined behavior in avalanche.cpp
Summary

In the MMIX LCG calculation, the right side calculation underlying type
is signed. This causes the undefined behavior sanitizer to detect a
signed integer overflow.
This diff sets the constants to unsigned long long to enforce the
underlying type to be unsigned and get rid of this error.

Test Plan

With the UBSAN activated, run

test_bitcoin -t avalanche_tests

Diff Detail

Repository
rABC Bitcoin ABC
Branch
undefined_avalanche
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5900
Build 9860: Bitcoin ABC Buildbot (legacy)
Build 9859: arc lint + arc unit

Event Timeline

Fabien created this revision.May 20 2019, 11:39
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 20 2019, 11:39
Herald added a reviewer: deadalnix. · View Herald Transcript
teamcity added a comment.May 20 2019, 11:40

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Harbormaster completed remote builds in B5900: Diff 8740.May 20 2019, 11:54
teamcity added a comment.May 20 2019, 11:54

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

markblundeberg added a subscriber: markblundeberg.May 20 2019, 14:09

In src/tests/lcg.h it should pick up the same problem, does it?

markblundeberg accepted this revision.May 20 2019, 15:10
This revision is now accepted and ready to land.May 20 2019, 15:10
deadalnix requested changes to this revision.May 20 2019, 16:37

If the problem is that nodeid is signed, then this is what needs fixing, not all the constants.

This revision now requires changes to proceed.May 20 2019, 16:37
Fabien updated this revision to Diff 8747.May 20 2019, 17:24

Cast nodeid instead of the constants.

teamcity added a comment.May 20 2019, 17:26

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

markblundeberg added a comment.May 20 2019, 17:28

still approve :D

markblundeberg accepted this revision.May 20 2019, 17:28
Harbormaster completed remote builds in B5905: Diff 8747.May 20 2019, 17:39
teamcity added a comment.May 20 2019, 17:39

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

deadalnix accepted this revision.May 20 2019, 20:29
This revision is now accepted and ready to land.May 20 2019, 20:29
Closed by commit rABCd478be9e220e: Fix undefined behavior in avalanche.cpp (authored by Fabien). · Explain WhyMay 21 2019, 09:25
This revision was automatically updated to reflect the committed changes.
Fabien added a commit: rSTAGINGd478be9e220e: Fix undefined behavior in avalanche.cpp.Sep 18 2019, 23:38

Revision Contents
Changeset List

PathSize
src/
3 lines

Diff 8740

View Options

src/avalanche.cpp

Loading...