Fix compilation errors in support/lockedpool.cpp
AbandonedPublic
Actions

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 8292
Build 14606: Default Diff Build & Tests
Build 14605: 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

Build Bitcoin-ABC / Diffs / Diff Testing started.

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

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8292: Diff 14424.Nov 26 2019, 02:57

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.

In D4548#109898, @Fabien wrote:

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.

Build Bitcoin-ABC / Diffs / Diff Testing started.

nakihito edited the summary of this revision. (Show Details)Dec 3 2019, 19:13

nakihito edited the test plan for this revision. (Show Details)

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8389: Diff 14595.Dec 3 2019, 19:32

Build Bitcoin-ABC / Diffs (Staging) / Diff Testing started.

Build Bitcoin-ABC / Diffs (Staging) / Diff Testing passed.

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?