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 Passed
Unit
No Test Coverage
Build Status
Buildable 8389
Build 14796: Default Diff Build & Tests
Build 14795: arc lint + arc unit

Event Timeline

Owners added a reviewer: Restricted Owners Package.Nov 26 2019, 02:42

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.

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.

Rebase and added CPPFLAGS=-DARENA_DEBUG to asan build.

nakihito edited the test plan for this revision. (Show Details)
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?