From 40ab1782e627bbc544418b9524a8439831a91f7e Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Mon, 2 Nov 2026 07:21:49 +0000 Subject: [PATCH] [hbook] Pass the full 64 bit buffer address to HBNAM Since 21cb0e620b2 (6.40.04), LOCF returns the full 64 bit word address instead of the low 32 bits. HNMADR computes the word offset of a column-wise ntuple variable as IADDW - LOCF(IQ(1)), but h2root and THbookFile still passed the buffer address to HBNAM as a 32 bit int, so IADDW lost bits 32 and 33 of the address while LOCF kept them. The stored offset was off by a multiple of 2^30 words whenever those bits were set, which is the case for most PIE placements, and HGNT2 then wrote the ntuple columns to a wild address. Pass the address as INTEGER*8 through HBNAM and compute the offset from the full addresses. THbookFile::SetBranchAddress also passed the value stored at the user pointer instead of the pointer; it now passes the address like the other call sites. Add a roottest that converts a small column-wise ntuple with h2root. The input file is written by the included mkcwn.f with CERNLIB. Fixes #23628 Assisted-by: claude-code:claude-fable-5-1 --- builtins/cernlib/src/hbook.f | 15 +++++++++---- hist/hbook/src/THbookFile.cxx | 15 +++++++------ main/src/h2root.cxx | 9 ++++---- roottest/root/hist/CMakeLists.txt | 7 ++++++ roottest/root/hist/cwn.hbook | Bin 0 -> 32768 bytes roottest/root/hist/h2root_cwn.ref | 31 +++++++++++++++++++++++++++ roottest/root/hist/mkcwn.f | 34 ++++++++++++++++++++++++++++++ 7 files changed, 96 insertions(+), 15 deletions(-) create mode 100644 roottest/root/hist/cwn.hbook create mode 100644 roottest/root/hist/h2root_cwn.ref create mode 100644 roottest/root/hist/mkcwn.f diff --git a/builtins/cernlib/src/hbook.f b/builtins/cernlib/src/hbook.f index b893c0085c799..cdf87e54a775a 100644 --- a/builtins/cernlib/src/hbook.f +++ b/builtins/cernlib/src/hbook.f @@ -1220,7 +1220,11 @@ SUBROUTINE HBNAM(IDD, BLKNA1, ADDRES, FORM1, ISCHAR) INCLUDE 'hcform.inc' INCLUDE 'zbits.inc' INCLUDE 'hcflag.inc' - INTEGER IDD, ADDRES, HNBPTR + INTEGER IDD, HNBPTR +* The address is passed in as the full 64 bit value. It must not be +* truncated to 32 bits, since LOCF (used to compute the offset +* relative to the PAWC store) returns the full address too. + INTEGER*8 ADDRES, IADD CHARACTER*(*) BLKNA1, FORM1 PARAMETER (MAXTOK = 50) CHARACTER*8 BLKNAM @@ -1713,7 +1717,7 @@ SUBROUTINE HNMADR(VAR1, IADD, ISCHAR) INCLUDE 'zbits.inc' CHARACTER*(*) VAR1 CHARACTER*32 NAME, VAR - INTEGER IADD + INTEGER*8 IADD, IADDW, LOCF LOGICAL ISCHAR, ALL, LDUM VAR = VAR1 CALL CLTOU(VAR) @@ -1744,9 +1748,12 @@ SUBROUTINE HNMADR(VAR1, IADD, ISCHAR) IELEM = IELEM*IE 10 CONTINUE IADDW = ISHFT(IADD, -2) - IBYOF = IAND(IADD, IBYPW-1) + IBYOF = IAND(INT(IADD,4), IBYPW-1) IF (IBYOF .NE. 0) GOTO 40 - IQ(LNAME+IOFF+ZNADDR) = IADDW - LOCF(IQ(1)) +* Word offset of the user variable relative to IQ(1), computed +* from the full 64 bit addresses so that it does not depend on +* where the buffer and the PAWC common happen to be mapped. + IQ(LNAME+IOFF+ZNADDR) = INT(IADDW - LOCF(IQ(1)), 4) IADD = IADD + IELEM*ISIZE 20 IOFF = IOFF + ZNADDR 30 CONTINUE diff --git a/hist/hbook/src/THbookFile.cxx b/hist/hbook/src/THbookFile.cxx index 613a7c8432547..3c2429eaad7b6 100644 --- a/hist/hbook/src/THbookFile.cxx +++ b/hist/hbook/src/THbookFile.cxx @@ -46,6 +46,7 @@ #include "TMath.h" #include "strlcpy.h" +#include #include #include #include @@ -220,9 +221,9 @@ extern "C" void type_of_call hntvar3(const int&,const int&,DEFCHAR); #endif #ifndef WIN32 -extern "C" void type_of_call hbnam(const int&,DEFCHAR,const int&,DEFCHAR,const int&,fortran_charlen_t, fortran_charlen_t); +extern "C" void type_of_call hbnam(const int&,DEFCHAR,const std::int64_t&,DEFCHAR,const int&,fortran_charlen_t, fortran_charlen_t); #else -extern "C" void type_of_call hbnam(const int&,DEFCHAR,const int&,DEFCHAR,const int&); +extern "C" void type_of_call hbnam(const int&,DEFCHAR,const std::int64_t&,DEFCHAR,const int&); #endif extern "C" void type_of_call hgnpar(const int&,const char *,const int); @@ -590,8 +591,7 @@ Bool_t THbookFile::IsOpen() const void THbookFile::SetBranchAddress(Int_t id, const char *bname, void *add) { - Int_t *iadd = (Int_t*)add; - Int_t &aadd = *iadd; + std::int64_t aadd = reinterpret_cast(add); #ifndef WIN32 hbnam(id,PASSCHAR(bname),aadd,PASSCHAR("$SET"),0,strlen(bname),4); #else @@ -698,10 +698,11 @@ TObject *THbookFile::ConvertCWN(Int_t id) char *bigbuf = tree->MakeX(500000); + std::int64_t add = reinterpret_cast(&bigbuf[0]); #ifndef WIN32 - hbnam(id,PASSCHAR(" "),bigbuf[0],PASSCHAR("$CLEAR"),0,1,6); + hbnam(id,PASSCHAR(" "),add,PASSCHAR("$CLEAR"),0,1,6); #else - hbnam(id,PASSCHAR(" "),bigbuf[0],PASSCHAR("$CLEAR"),0); + hbnam(id,PASSCHAR(" "),add,PASSCHAR("$CLEAR"),0); #endif UInt_t varNumber = 0; @@ -755,7 +756,7 @@ TObject *THbookFile::ConvertCWN(Int_t id) varNumber = 0; strlcpy(oldblock,block,32); oldischar = ischar; - Long_t add= (Long_t)&bigbuf[bufpos]; + add = reinterpret_cast(&bigbuf[bufpos]); Int_t lblock = strlen(block); #ifndef WIN32 hbnam(id,PASSCHAR(block),add,PASSCHAR("$SET"),ischar,lblock,4); diff --git a/main/src/h2root.cxx b/main/src/h2root.cxx index 3b41cbc88f474..07031a7817c5f 100644 --- a/main/src/h2root.cxx +++ b/main/src/h2root.cxx @@ -31,6 +31,7 @@ #include "TMath.h" #include "strlcpy.h" +#include #include #include #include @@ -205,9 +206,9 @@ extern "C" void type_of_call hntvar2(const int&,const int&,DEFCHAR,DEFCHAR,DEFC #endif #ifndef WIN32 -extern "C" void type_of_call hbnam(const int&,DEFCHAR,const int&,DEFCHAR,const int&,fortran_charlen_t,fortran_charlen_t); +extern "C" void type_of_call hbnam(const int&,DEFCHAR,const std::int64_t&,DEFCHAR,const int&,fortran_charlen_t,fortran_charlen_t); #else -extern "C" void type_of_call hbnam(const int&,DEFCHAR,const int&,DEFCHAR,const int&); +extern "C" void type_of_call hbnam(const int&,DEFCHAR,const std::int64_t&,DEFCHAR,const int&); #endif extern "C" void type_of_call hprntu(const int&); @@ -715,7 +716,7 @@ void convert_cwn(Int_t id) #else hgiven(id,chtitl,80,nvar,chtag_out,kNchar,rmin[0],rmax[0]); #endif - Long_t add= (Long_t)&bigbuf[0]; + std::int64_t add = reinterpret_cast(&bigbuf[0]); #ifndef WIN32 hbnam(id,PASSCHAR(" "),add,PASSCHAR("$CLEAR"),0,1,6); #else @@ -825,7 +826,7 @@ void convert_cwn(Int_t id) strlcpy(oldblock,block,512); oldischar = ischar; Int_t lblock = strlen(block); - add= (Long_t)&bigbuf[bufpos]; + add = reinterpret_cast(&bigbuf[bufpos]); #ifndef WIN32 hbnam(id,PASSCHAR(block),add,PASSCHAR("$SET"),ischar,lblock,4); #else diff --git a/roottest/root/hist/CMakeLists.txt b/roottest/root/hist/CMakeLists.txt index adcf2726d598b..0e68e6c6bbd50 100644 --- a/roottest/root/hist/CMakeLists.txt +++ b/roottest/root/hist/CMakeLists.txt @@ -11,6 +11,13 @@ if(fortran AND CMAKE_Fortran_COMPILER AND NOT CMAKE_Fortran_COMPILER_ID STREQUAL COMMAND h2root mb4i1.hbook COPY_TO_BUILDDIR mb4i1.hbook OUTREF h2root.ref) + # Column-wise ntuples: the user buffer addresses are handed to HBOOK via + # HBNAM, which used to crash depending on where the executable was mapped. + # See https://github.com/root-project/root/issues/23628 + ROOTTEST_ADD_TEST(h2root-cwn + COMMAND h2root cwn.hbook + COPY_TO_BUILDDIR cwn.hbook + OUTREF h2root_cwn.ref) ROOT_ADD_GTEST(THbookFile THbookFile.cxx LIBRARIES Hbook) set_tests_properties(roottest-root-hist-h2root PROPERTIES FIXTURES_SETUP hbookFile) set_tests_properties(gtest-roottest-root-hist-THbookFile PROPERTIES FIXTURES_REQUIRED hbookFile) diff --git a/roottest/root/hist/cwn.hbook b/roottest/root/hist/cwn.hbook new file mode 100644 index 0000000000000000000000000000000000000000..cb2f170903938886be2ae8090a38d861a460040c GIT binary patch literal 32768 zcmeI5U29ZV6o%KHGntGv*r*9qdUH_xs-_Zy=tV+iOcN$Fj&vqON^{Uygi%<|dtwQi%DU1eUo z*~inq(Ap*s6B>P&Bt^f3tUU<#n`OI#BHq62i<;$-mw|4x{%1c}=Zzn2cRXQTcdv(Z z=0*5C!FR*)?KRRQA|L`HAOa#F0wN#+A|L`HAOa#F0_#ek{S^-Hl2d5^2jP_2-mU2K z{}t?Qu%8EeJM5>y#`plo1z27p0wN#+A|L`HAOa#F0wN#+A|L{-Nx;v&=WW0`ql27f zM?8;Nc*L{lHivjF#4)Sx^V>qaEyS}S4upPxS@hF8tsha&vmZ4#dj)z5`g89s`_Kw` zrmsnoe;Em`G<32TA@iwSn4YJ^P@GM38C)Z1R|9R~wBM|`+5CIVo0TB=Z5fA|p5CIVo0TFnA1VaCR zy3gG7;baV;uK#}kr;=&b{~tsq#}x+ehH;D-VF5a65)lvq5fA|p5CIVo0TB=Z5fA|p z5P^+DAoTyIj+vXnx(AK{)b;;A!Kp^g`v1t|{6FXZ>A!%=dH+q&WDNlfi3o^*2#A0P zh=2%)fCz|y2#A0Ph`_oN2>t)b0dtf6$rwOg{~!JeE!(XBKlsxh_X@eQXA2z&M-Yx7 ztoHR4Zb6rzzas4+!bb>~Z`@q>tJMjJfCz|y2#A0Ph=2%)fCz|y2#7#y62P2&v@7u` zaQaVkXa2@-{+FQl&0YK!>8C6M{|6MuF#@a;@b=8_DDxXrOXcDTgj@qaS>Psb$72Lc zqjHTvx=w&=1JXE5vz3*VCzjb6!(`dSdqPr%_7-Su`3U8gktW&51Xk+|e^&zE5)?Pv z0o_ciQJt}*&hVb|epAx>-l)zzGV^tY}3>Rz4 zF4WjETa`ejuAb}zhNv`J=6?gFvXB}G(>AUqNoGqBXsfFy`*F?qtz_wq=gBC1Ez#dn zd3Y?wvnXx}HoC}F0?pgDGZ*k4jdy9^9&xR$YtXi(?>7zGw(3xEtXQeWxkdYE^?9Yuy2V7yfXs-0?-P$%>V!Z literal 0 HcmV?d00001 diff --git a/roottest/root/hist/h2root_cwn.ref b/roottest/root/hist/h2root_cwn.ref new file mode 100644 index 0000000000000..61bad11465cff --- /dev/null +++ b/roottest/root/hist/h2root_cwn.ref @@ -0,0 +1,31 @@ + Converting directory //example + Converting CWN with ID= 100, nentries = 7 +Creating branch:Ivar, block:SCALARS, fullname:Ivar/I, nsub=0, itype=2, isize=4, ielem=1 +Creating branch:Fvar, block:SCALARS, fullname:Fvar/F, nsub=0, itype=1, isize=4, ielem=1 +Creating branch:Nel, block:ARRAYS, fullname:Nel/I, nsub=0, itype=2, isize=4, ielem=1 +Creating branch:Arr, block:ARRAYS, fullname:Arr[Nel]/F, nsub=1, itype=1, isize=4, ielem=3 +****************************************************************************** +*Tree :h100 : cwn * +*Entries : 7 : Total = 3010 bytes File Size = 0 * +* : : Tree compression factor = 1.00 * +****************************************************************************** +*Br 0 :Ivar : Ivar/I * +*Entries : 7 : Total Size= 667 bytes One basket in memory * +*Baskets : 0 : Basket Size= 64000 bytes Compression= 1.00 * +*............................................................................* +*Br 1 :Fvar : Fvar/F * +*Entries : 7 : Total Size= 667 bytes One basket in memory * +*Baskets : 0 : Basket Size= 64000 bytes Compression= 1.00 * +*............................................................................* +*Br 2 :Nel : Nel/I * +*Entries : 7 : Total Size= 661 bytes One basket in memory * +*Baskets : 0 : Basket Size= 64000 bytes Compression= 1.00 * +*............................................................................* +*Br 3 :Arr : Arr[Nel]/F * +*Entries : 7 : Total Size= 796 bytes One basket in memory * +*Baskets : 0 : Basket Size= 64000 bytes Compression= 1.00 * +*............................................................................* +TFile** cwn.root HBOOK file: cwn.hbook converted to ROOT + TFile* cwn.root HBOOK file: cwn.hbook converted to ROOT + KEY: TH1F h1;1 FVAR + KEY: TTree h100;1 cwn diff --git a/roottest/root/hist/mkcwn.f b/roottest/root/hist/mkcwn.f new file mode 100644 index 0000000000000..9ef0646313495 --- /dev/null +++ b/roottest/root/hist/mkcwn.f @@ -0,0 +1,34 @@ +* Generator for cwn.hbook, the input of the h2root-cwn test. +* Build against CERNLIB, which must be linked without PIE so that the +* commons live in the low 32 bit address space: +* gfortran -fno-pie -no-pie -fno-automatic -o mkcwn mkcwn.f \ +* -lpacklib -lmathlib -lkernlib + PROGRAM MKCWN + INTEGER NWPAWC + PARAMETER (NWPAWC = 100000) + COMMON /PAWC/ H(NWPAWC) + INTEGER IVAR, NEL + REAL FVAR, ARR(3) + COMMON /CWNBLK/ IVAR, FVAR, NEL, ARR + INTEGER ISTAT, ICYCLE, I, J + CALL HLIMIT(NWPAWC) + CALL HROPEN(10, 'TEST', 'cwn.hbook', 'N', 1024, ISTAT) + IF (ISTAT .NE. 0) STOP 'HROPEN failed' + CALL HBOOK1(1, 'FVAR', 10, 0., 10., 0.) + CALL HBNT(100, 'cwn test', ' ') + CALL HBNAME(100, 'SCALARS', IVAR, 'IVAR:I*4,FVAR:R*4') + CALL HBNAME(100, 'ARRAYS', NEL, 'NEL[0,3]:I*4,ARR(NEL):R*4') + DO 20 I = 1, 7 + IVAR = I + FVAR = 0.5*I + NEL = MOD(I, 4) + DO 10 J = 1, NEL + ARR(J) = I + 0.1*J + 10 CONTINUE + CALL HFILL(1, FVAR, 0., 1.) + CALL HFNT(100) + 20 CONTINUE + CALL HROUT(0, ICYCLE, ' ') + CALL HREND('TEST') + CLOSE(10) + END