Differential D9110

build: warn on potentially uninitialized reads
ClosedPublic
Actions

Authored by PiRK on Jan 29 2021, 09:27.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC9c7bed4ea4b8: build: warn on potentially uninitialized reads

Summary

Enable -Wconditional-uninitialized to warn on potentially uninitialized reads.

Fix the sole such warning in Bitcoin Core in GetRdRand(): r1 would be
set to 0 on rdrand failure, so initializing it to 0 is a non-functional
change.

From "Intel 64 and IA-32 ArchitecturesSoftware Developer's Manual" [1],
page 1711: "CF=1 indicates that the data in the destination is valid.
Otherwise CF=0 and the data in the destination operand will be returned
as zeros for the specified width."

[1] https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf

This is a backport of Core PR18843

Test Plan

cmake .. -GNinja \
    -DCMAKE_C_COMPILER=clang  \
    -DCMAKE_CXX_COMPILER=clang++ \
    -DENABLE_CLANG_TIDY=ON \
    -DCMAKE_C_FLAGS="-Werror"
ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.Jan 29 2021, 09:27

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 29 2021, 09:27

PiRK requested review of this revision.Jan 29 2021, 09:27

teamcity edited the summary of this revision. (Show Details)Jan 29 2021, 09:28

Harbormaster completed remote builds in B15051: Diff 27424.Jan 29 2021, 09:33

Build Bitcoin ABC Diffs / Diff Testing (build-clang) failed.

Tail of the build log:

[378/437] Running utility command for check-bitcoin-merkleblock_tests
[379/437] bitcoin: testing sync_tests
[380/437] Running utility command for check-bitcoin-sighashtype_tests
[381/437] Running utility command for check-bitcoin-script_commitment_tests
[382/437] Running utility command for check-bitcoin-bip32_tests
[383/437] Running utility command for check-bitcoin-sync_tests
[384/437] bitcoin: testing torcontrol_tests
[385/437] Running utility command for check-bitcoin-torcontrol_tests
[386/437] bitcoin: testing settings_tests
[387/437] Running utility command for check-bitcoin-settings_tests
[388/437] bitcoin: testing streams_tests
[389/437] bitcoin: testing wallet_tests
[390/437] Running utility command for check-bitcoin-streams_tests
[391/437] Running utility command for check-bitcoin-wallet_tests
[392/437] bitcoin: testing uint256_tests
[393/437] bitcoin: testing undo_tests
[394/437] Running utility command for check-bitcoin-uint256_tests
[395/437] Running utility command for check-bitcoin-undo_tests
[396/437] bitcoin: testing walletdb_tests
[397/437] Running utility command for check-bitcoin-walletdb_tests
[398/437] bitcoin: testing util_threadnames_tests
[399/437] bitcoin: testing validation_flush_tests
[400/437] Running utility command for check-bitcoin-util_threadnames_tests
[401/437] Running utility command for check-bitcoin-validation_flush_tests
[402/437] bitcoin: testing serialize_tests
[403/437] Running utility command for check-bitcoin-serialize_tests
[404/437] bitcoin: testing compilerbug_tests
[405/437] bitcoin: testing radix_tests
[406/437] bitcoin: testing schnorr_tests
[407/437] Running utility command for check-bitcoin-compilerbug_tests
[408/437] Running utility command for check-bitcoin-radix_tests
[409/437] Running utility command for check-bitcoin-schnorr_tests
[410/437] bitcoin: testing getarg_tests
[411/437] Running utility command for check-bitcoin-getarg_tests
[412/437] bitcoin: testing coinselector_tests
[413/437] bitcoin: testing ref_tests
[414/437] Running utility command for check-bitcoin-coinselector_tests
[415/437] bitcoin: testing crypto_tests
[416/437] bitcoin: testing blockcheck_tests
[417/437] Running utility command for check-bitcoin-ref_tests
[418/437] Running utility command for check-bitcoin-crypto_tests
[419/437] Running utility command for check-bitcoin-blockcheck_tests
[420/437] bitcoin: testing script_tests
[421/437] bitcoin: testing validation_tests
[422/437] Running utility command for check-bitcoin-script_tests
[423/437] Running utility command for check-bitcoin-validation_tests
[424/437] bitcoin: testing monolith_opcodes_tests
[425/437] Running utility command for check-bitcoin-monolith_opcodes_tests
[426/437] bitcoin: testing skiplist_tests
[427/437] Running utility command for check-bitcoin-skiplist_tests
[428/437] bitcoin: testing op_reversebytes_tests
[429/437] Running utility command for check-bitcoin-op_reversebytes_tests
[430/437] bitcoin: testing transaction_tests
[431/437] Running utility command for check-bitcoin-transaction_tests
[432/437] bitcoin: testing coins_tests
[433/437] Running utility command for check-bitcoin-coins_tests
[434/437] Running bitcoin test suite
PASSED: bitcoin test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1

Working on submitting a patch upstream (secp256k1)

PiRK planned changes to this revision.Jan 29 2021, 12:00

PiRK edited the test plan for this revision. (Show Details)Feb 1 2021, 07:06

PiRK mentioned this in D9372: [secp256k1] initialize variable in tests.Apr 8 2021, 06:44

rebase onto D9372

Fabien accepted this revision.Apr 8 2021, 06:58

This revision is now accepted and ready to land.Apr 8 2021, 06:58

Harbormaster completed remote builds in B15455: Diff 28085.Apr 8 2021, 07:10

Closed by commit rABC9c7bed4ea4b8: build: warn on potentially uninitialized reads (authored by Vasil Dimov <vd@FreeBSD.org>, committed by PiRK). · Explain WhyApr 8 2021, 08:28

This revision was automatically updated to reflect the committed changes.

PiRK mentioned this in rABCb0d8c607a6ca: [secp256k1] initialize variable in tests.

PiRK added a commit: rABC9c7bed4ea4b8: build: warn on potentially uninitialized reads.