Page MenuHomePhabricator

Fix compilation errors in support/lockedpool.cpp
AbandonedPublic

Authored by nakihito on Nov 26 2019, 02:42.

Details

Reviewers
deadalnix
Fabien
jasonbcox
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Summary

Changes in #12048 cause a compilation error in Arena::walk() when
ARENA_DEBUG is defined. Specifically, Arena's chunks_free map was
changed to have a different value type.

Additionally, missing includes cause other compilation errors when
ARENA_DEBUG is defined.

Second commit fixes segfault in allocator_tests/arena_tests
The test uses reinterpret_cast<void*> on unallocated memory. Using this
memory in printchunk as char* causes a segfault, so have printchunk take
void* instead.

Reproduced with:

make CPPFLAGS=-DARENA_DEBUG

Backport of Core PR16161
https://github.com/bitcoin/bitcoin/pull/16161/commits/

Depends on D3898

Reviewer note: the last commit in this PR is TRAVIS related and was therefore modified to our code base.

Test Plan
../configure CPPFLAGS=-DARENA_DEBUG
make check
test_runner.py
ABC_BUILD_NAME=build-asan build-configurations.sh

Diff Detail

Repository
rABC Bitcoin ABC
Branch
PR16161
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8389
Build 14796: Bitcoin ABC Buildbot
Build 14795: arc lint + arc unit

Event Timeline

nakihito created this revision.Nov 26 2019, 02:42
Owners added a reviewer: Restricted Owners Package.Nov 26 2019, 02:42
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 26 2019, 02:42
nakihito planned changes to this revision.Nov 26 2019, 02:42
nakihito edited the summary of this revision. (Show Details)Nov 26 2019, 02:45
nakihito requested review of this revision.Nov 26 2019, 02:49
Fabien added a comment.Nov 26 2019, 10:49

The Travis changes are actually interesting to ensure that no such regression occurs in the future, I don't think they should be skipped.
I'd suggest to add the flag to the ASAN CI build which seems the most appropriated place to me, since this is memory allocation related.

nakihito planned changes to this revision.Nov 26 2019, 17:34

The Travis changes are actually interesting to ensure that no such regression occurs in the future, I don't think they should be skipped.
I'd suggest to add the flag to the ASAN CI build which seems the most appropriated place to me, since this is memory allocation related.

Will be added.

nakihito updated this revision to Diff 14595.Dec 3 2019, 19:12

Rebase and added CPPFLAGS=-DARENA_DEBUG to asan build.

nakihito edited the summary of this revision. (Show Details)Dec 3 2019, 19:13
nakihito edited the test plan for this revision. (Show Details)
Fabien accepted this revision.Dec 4 2019, 09:51
This revision is now accepted and ready to land.Dec 4 2019, 09:51

Is there a reason why the original patch is backported as this, even though it is known to be bogus, and then this?

nakihito abandoned this revision.Dec 12 2019, 22:29

Squashed into D3898.