Skip to content

Commit 5ad0863

Browse files
committed
[treeplayer] Fix memory management of C-style array branches in test
The test was dynamically allocating the array data members of the `Data` struct, but never deallocating them. This commit polishes the `Data` struct definition and ensures proper management of the data members. The previous way of writing data to the TTree was leading to a bad memory access in the ReadBasicPointer inlined function in TStreamerInfoReadBuffer.cxx while reading the `double*` array. In particular, the issue arises when accessing and then deallocating the array at the current index provided by the `TCompInfo` object. ``` Target 0: (repro.out) stopped. (lldb) Process 13498 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = step in frame #0: 0x00000001044cf140 libRIO.so`int TStreamerInfo::ReadBuffer<char**>(this=<unavailable>, b=<unavailable>, arr=<unavailable>, compinfo=<unavailable>, first=<unavailable>, last=<unavailable>, narr=<unavailable>, eoffset=<unavailable>, arrayMode=0) at TStreamerInfoReadBuffer.cxx:923:65 [opt] 920 case TStreamerInfo::kOffsetP + TStreamerInfo::kLong: ReadBasicPointer(Long_t); continue; 921 case TStreamerInfo::kOffsetP + TStreamerInfo::kLong64: ReadBasicPointer(Long64_t); continue; 922 case TStreamerInfo::kOffsetP + TStreamerInfo::kFloat: ReadBasicPointer(Float_t); continue; -> 923 case TStreamerInfo::kOffsetP + TStreamerInfo::kDouble: ReadBasicPointer(Double_t); continue; 924 case TStreamerInfo::kOffsetP + TStreamerInfo::kUChar: ReadBasicPointer(UChar_t); continue; 925 case TStreamerInfo::kOffsetP + TStreamerInfo::kUShort: ReadBasicPointer(UShort_t); continue; 926 case TStreamerInfo::kOffsetP + TStreamerInfo::kUInt: ReadBasicPointer(UInt_t); continue; Target 0: (repro.out) stopped. (lldb) Process 13498 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = step in frame #0: 0x00000001044cf184 libRIO.so`int TStreamerInfo::ReadBuffer<char**>(TBuffer&, char** const&, TStreamerInfo::TCompInfo* const*, int, int, int, int, int) [inlined] TBuffer::BufferSize(this=0x000060e00010ef00) const at TBuffer.h:98:41 [opt] 95 TObject *GetParent() const; 96 char *Buffer() const { return fBuffer; } 97 char *GetCurrent() const { return fBufCur; } -> 98 Int_t BufferSize() const { return fBufSize; } 99 void DetachBuffer() { fBuffer = nullptr; } 100 Int_t Length() const { return (Int_t)(fBufCur - fBuffer); } 101 void Expand(Int_t newsize, Bool_t copy = kTRUE); // expand buffer to newsize Target 0: (repro.out) stopped. (lldb) p fBufSize (Int_t) 32008 (lldb) s Process 13498 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = step in frame #0: 0x00000001044cf194 libRIO.so`int TStreamerInfo::ReadBuffer<char**>(this=<unavailable>, b=<unavailable>, arr=<unavailable>, compinfo=<unavailable>, first=<unavailable>, last=<unavailable>, narr=<unavailable>, eoffset=<unavailable>, arrayMode=0) at TStreamerInfoReadBuffer.cxx:923:65 [opt] 920 case TStreamerInfo::kOffsetP + TStreamerInfo::kLong: ReadBasicPointer(Long_t); continue; 921 case TStreamerInfo::kOffsetP + TStreamerInfo::kLong64: ReadBasicPointer(Long64_t); continue; 922 case TStreamerInfo::kOffsetP + TStreamerInfo::kFloat: ReadBasicPointer(Float_t); continue; -> 923 case TStreamerInfo::kOffsetP + TStreamerInfo::kDouble: ReadBasicPointer(Double_t); continue; 924 case TStreamerInfo::kOffsetP + TStreamerInfo::kUChar: ReadBasicPointer(UChar_t); continue; 925 case TStreamerInfo::kOffsetP + TStreamerInfo::kUShort: ReadBasicPointer(UShort_t); continue; 926 case TStreamerInfo::kOffsetP + TStreamerInfo::kUInt: ReadBasicPointer(UInt_t); continue; Target 0: (repro.out) stopped. (lldb) s Process 13498 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbebebebebebebeae) frame #0: 0x0000000107bac674 libclang_rt.asan_osx_dynamic.dylib`__asan::Allocator::Deallocate(void*, unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType) + 76 libclang_rt.asan_osx_dynamic.dylib`__asan::Allocator::Deallocate: -> 0x107bac674 <+76>: casalb w8, w9, [x22] 0x107bac678 <+80>: cmp w8, #0x2 0x107bac67c <+84>: b.ne 0x107bac6f4 ; <+204> 0x107bac680 <+88>: mov x8, #-0x100000000 ; =-4294967296 Target 0: (repro.out) stopped. (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbebebebebebebeae) * frame #0: 0x0000000107bac674 libclang_rt.asan_osx_dynamic.dylib`__asan::Allocator::Deallocate(void*, unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType) + 76 frame #1: 0x0000000107c0c444 libclang_rt.asan_osx_dynamic.dylib`wrap__ZdaPv + 232 frame #2: 0x00000001044d4a60 libRIO.so`int TStreamerInfo::ReadBuffer<char**>(this=<unavailable>, b=<unavailable>, arr=<unavailable>, compinfo=<unavailable>, first=<unavailable>, last=<unavailable>, narr=<unavailable>, eoffset=<unavailable>, arrayMode=0) at TStreamerInfoReadBuffer.cxx:923:65 [opt] frame #3: 0x0000000103ffc888 libRIO.so`TStreamerInfoActions::GenericReadAction(buf=0x000060e00010ef00, addr=0x0000602000056bd0, config=0x0000604000149910) at TStreamerInfoActions.cxx:195:45 frame #4: 0x0000000103caa5ec libRIO.so`TStreamerInfoActions::TConfiguredAction::operator()(this=0x00006030001693f0, buffer=0x000060e00010ef00, object=0x0000602000056bd0) const at TStreamerInfoActions.h:123:17 frame #5: 0x0000000103ca9ef8 libRIO.so`TBufferFile::ApplySequence(this=0x000060e00010ef00, sequence=0x000060600011ac20, obj=0x0000602000056bd0) at TBufferFile.cxx:3702:10 frame #6: 0x00000001064bc570 libTree.so`TBranchElement::ReadLeavesMemberBranchCount(this=0x0000619000566380, b=0x000060e00010ef00) at TBranchElement.cxx:4603:6 frame #7: 0x0000000106455ce4 libTree.so`TBranch::GetEntry(this=0x0000619000566380, entry=0, getall=0) at TBranch.cxx:1753:4 frame #8: 0x00000001064a1764 libTree.so`TBranchElement::GetEntry(this=0x0000619000566380, entry=0, getall=0) at TBranchElement.cxx:2783:27 frame #9: 0x000000010739915c libTreePlayer.so`ROOT::Detail::TBranchProxy::Read(this=0x00006110000c9580) at TBranchProxy.h:163:42 frame #10: 0x0000000107649ba8 libTreePlayer.so`(anonymous namespace)::TObjectArrayReader::At(this=0x0000603000169900, proxy=0x00006110000c9580, idx=1) at TTreeReaderArray.cxx:176:22 frame #11: 0x000000010000c2e4 repro.out`ROOT::Internal::TTreeReaderArrayBase::UntypedAt(this=0x000000016fdfe740, idx=1) const at TTreeReaderArray.h:41:62 frame #12: 0x000000010000c200 repro.out`TTreeReaderArray<double>::At(this=0x000000016fdfe740, idx=1) at TTreeReaderArray.h:205:54 frame #13: 0x00000001000065e0 repro.out`TTreeReaderArray<double>::operator[](this=0x000000016fdfe740, idx=1) at TTreeReaderArray.h:207:44 frame #14: 0x0000000100007b48 repro.out`simpleTest() at repro.cpp:123:26 frame #15: 0x0000000100007e10 repro.out`main at repro.cpp:128:5 frame #16: 0x000000018c718274 dyld`start + 2840 ```
1 parent 0550ae7 commit 5ad0863

File tree

2 files changed

+41
-22
lines changed

2 files changed

+41
-22
lines changed

tree/treeplayer/test/data.h

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,39 @@
22
#define TTREEREADER_LEAF_TEST_DATA
33

44
#include <vector>
5-
struct Data {
6-
unsigned int fUSize;
7-
int fSize;
8-
double* fArray; //[fSize]
9-
float* fUArray; //[fUSize]
10-
std::vector<double> fVec;
11-
Double32_t fDouble32;
12-
Float16_t fFloat16;
5+
class Data {
6+
unsigned int fUSize{};
7+
int fSize{};
8+
double *fArray{nullptr}; //[fSize]
9+
float *fUArray{nullptr}; //[fUSize]
10+
std::vector<double> fVec{};
11+
Double32_t fDouble32{};
12+
Float16_t fFloat16{};
13+
14+
public:
15+
Data() = default;
16+
Data(const Data &) = delete;
17+
Data &operator=(const Data &) = delete;
18+
Data(Data &&) = delete;
19+
Data &operator=(Data &&) = delete;
20+
~Data()
21+
{
22+
delete[] fArray;
23+
fArray = nullptr;
24+
delete[] fUArray;
25+
fUArray = nullptr;
26+
}
27+
void Init()
28+
{
29+
fUSize = 2;
30+
fSize = 4;
31+
fArray = new double[4]{12., 13., 14., 15.};
32+
fUArray = new float[2]{42., 43.};
33+
fVec = {17., 18., 19., 20., 21., 22.};
34+
fDouble32 = 17.;
35+
fFloat16 = 44.;
36+
}
37+
std::vector<double> &GetVecMember() { return fVec; }
1338
};
1439

1540
struct V {

tree/treeplayer/test/leafs.cxx

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -84,32 +84,26 @@ TEST(TTreeReaderLeafs, LeafListCaseA) {
8484
EXPECT_EQ(Bool, *trBool);
8585
}
8686

87-
88-
89-
std::unique_ptr<TTree> CreateTree() {
87+
std::unique_ptr<TTree> CreateTree()
88+
{
9089
TInterpreter::EErrorCode error = TInterpreter::kNoError;
9190
gInterpreter->ProcessLine("#include \"data.h\"", &error);
9291
if (error != TInterpreter::kNoError)
9392
return {};
9493

9594
Data data;
95+
data.Init();
96+
auto &vecDataMember = data.GetVecMember();
9697
auto tree = std::make_unique<TTree>("T", "test tree");
9798
tree->Branch("Data", &data);
98-
data.fArray = new double[4]{12., 13., 14., 15.};
99-
data.fSize = 4;
100-
data.fUArray = new float[2]{42., 43.};
101-
data.fUSize = 2;
102-
data.fVec = { 17., 18., 19., 20., 21., 22.};
103-
data.fDouble32 = 17.;
104-
data.fFloat16 = 44.;
10599
tree->Fill();
106100

107-
data.fVec.clear();
108-
data.fVec.resize(3210, 1001.f); // ROOT-8747
101+
vecDataMember.clear();
102+
vecDataMember.resize(3210, 1001.f); // ROOT-8747
109103
tree->Fill();
110104

111-
data.fVec.clear();
112-
data.fVec.resize(2, 42.f); // ROOT-8747
105+
vecDataMember.clear();
106+
vecDataMember.resize(2, 42.f); // ROOT-8747
113107
tree->Fill();
114108

115109
tree->ResetBranchAddresses();

0 commit comments

Comments
 (0)