Skip to content

Hopefully, fix the long-standing problem with h2root #15915

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

couet
Copy link
Member

@couet couet commented Jun 24, 2024

Using the C option (meaning C I.O) in hropen seems to fix the issue on Mac.

@couet couet requested a review from dpiparo June 24, 2024 13:06
@couet couet self-assigned this Jun 24, 2024
@couet couet requested a review from pcanal as a code owner June 24, 2024 13:06
Copy link

github-actions bot commented Jun 24, 2024

Test Results

    18 files      18 suites   4d 11h 9m 38s ⏱️
 2 682 tests  2 680 ✅ 0 💤 2 ❌
46 556 runs  46 553 ✅ 0 💤 3 ❌

For more details on these failures, see this check.

Results for commit e052758.

♻️ This comment has been updated with latest results.

@hahnjo
Copy link
Member

hahnjo commented Jun 25, 2024

As far as I can tell, this shouldn't make a difference because HROPEN defaults to to C unless F is specified:

IF (INDEX(CHOPT,'F').EQ.0) THEN
IC = MIN(LENOCC(CHOPT)+1,8)
CHOPT(IC:IC) = 'C'
ENDIF

@couet
Copy link
Member Author

couet commented Jun 27, 2024

@hahnjo it made on on MacOS. (long debug)

@dpiparo dpiparo closed this Sep 9, 2024
@dpiparo dpiparo reopened this Sep 9, 2024
@hahnjo
Copy link
Member

hahnjo commented Sep 9, 2024

my comment from #15915 (comment) still stands: based on the code, there should be no change in behavior because C is already the default.

@couet
Copy link
Member Author

couet commented Dec 19, 2024

I know "c" is supposed to be the default, but this change made a difference on Mac when I debugged it a while ago. I guess it is worth merging.

@@ -312,7 +312,7 @@ int main(int argc, char **argv)

int lun = 10;
#ifndef WIN32
hropen(lun,PASSCHAR("example"),PASSCHAR(file_in),PASSCHAR("px"),record_size,ier,7,strlen(file_in),2);
hropen(lun,PASSCHAR("example"),PASSCHAR(file_in),PASSCHAR("pxc"),record_size,ier,7,strlen(file_in),2);
Copy link
Contributor

@ferdymercury ferdymercury Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
hropen(lun,PASSCHAR("example"),PASSCHAR(file_in),PASSCHAR("pxc"),record_size,ier,7,strlen(file_in),2);
hropen(lun,PASSCHAR("example"),PASSCHAR(file_in),PASSCHAR("px "),record_size,ier,7,strlen(file_in),2);

@couet Could you try to see what happens if you change to this, if it also fixes the problem on macos?
(Assuming the problem is the malloc issue when appending the 'C' after the 'PX' on the Fortran side CHOPT(LEN+1:LEN+1) = 'C', since lenocc will ignore spaces.). (Just to check if memory is reserved in pads of two bytes or so.)

Copy link
Contributor

@ferdymercury ferdymercury Apr 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this can get us any closer: https://its.cern.ch/jira/browse/ROOT-9179

But with ASAN enabled I found here #18234 a couple of issues.

  ==84992==ERROR: AddressSanitizer: memcpy-param-overlap: memory ranges [0xffffd9c19d70,0xfffed9c19d72) and [0x0000004d2160, 0xffffffff004d2162) overlap
      #0 0xffff95246d08 in __interceptor_memcpy (/lib64/libasan.so.6+0x3ad08)
      #1 0x43ff88 in hropen_ /github/home/ROOT-CI/src/misc/minicern/src/hbook.f:251
      #2 0x405ae8 in main /github/home/ROOT-CI/src/main/src/h2root.cxx:315
      #3 0xffff9367927c in __libc_start_call_main (/lib64/libc.so.6+0x2727c)
      #4 0xffff93679354 in __libc_start_main_alias_2 (/lib64/libc.so.6+0x27354)
      #5 0x405e6c in _start (/github/home/ROOT-CI/build/bin/h2root+0x405e6c)
  
  Address 0xffffd9c19d70 is located in stack of thread T0 at offset 48 in frame
      #0 0x43f81c in hropen_ /github/home/ROOT-CI/src/misc/minicern/src/hbook.f:239
  
    This frame has 5 object(s):
      [48, 56) 'chopt' (line 250) <== Memory access at offset 48 partially overflows this variable
      [80, 144) 'close_parm.62' (line 281) <== Memory access at offset 48 partially underflows this variable
      [176, 704) 'dt_parm.57' (line 255) <== Memory access at offset 48 partially underflows this variable
      [832, 1360) 'dt_parm.59' (line 266) <== Memory access at offset 48 partially underflows this variable
      [1488, 2016) 'dt_parm.61' (line 279) <== Memory access at offset 48 partially underflows this variable
  HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
        (longjmp and C++ exceptions *are* supported)
  Address 0x0000004d2160 is a wild pointer.
  SUMMARY: AddressSanitizer: memcpy-param-overlap (/lib64/libasan.so.6+0x3ad08) in __interceptor_memcpy
  ==84992==ABORTING

Also:

 /github/home/ROOT-CI/src/misc/minicern/src/cernlib.c:82:18: runtime error: left shift of negative value -2012982016
  /github/home/ROOT-CI/src/misc/minicern/src/cernlib.c:83:18: runtime error: left shift of negative value -2012982016
  /github/home/ROOT-CI/src/misc/minicern/src/cernlib.c:100:12: runtime error: left shift of 1212370737 by 8 places cannot be represented in type 'int'
  /github/home/ROOT-CI/src/misc/minicern/src/cernlib.c:100:39: runtime error: left shift of 1212370737 by 24 places cannot be represented in type 'int'
  /github/home/ROOT-CI/src/misc/minicern/src/zebra.f:6746:72: runtime error: load of address 0x56353fb115cc with insufficient space for an object of type 'integer(kind=4)'
  0x56353fb115cc: note: pointer points here
    24 02 00 00 18 2f 0f 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
                ^ 
  /github/home/ROOT-CI/src/misc/minicern/src/zebra.f:6747:72: runtime error: load of address 0x56353fb115cc with insufficient space for an object of type 'integer(kind=4)'
  0x56353fb115cc: note: pointer points here
    24 02 00 00 18 2f 0f 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
                ^ 
  /github/home/ROOT-CI/src/misc/minicern/src/zebra.f:6753:72: runtime error: store to address 0x56353fb115cc with insufficient space for an object of type 'integer(kind=4)'
  0x56353fb115cc: note: pointer points here
    24 02 00 00 18 2f 0f 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
                ^ 

Without ASAN I got:

  Ubuntu 22 imt=off Debug build, sometimes:
  
  cd /github/home/ROOT-CI/build/roottest/root/hist
  /usr/bin/timeout -s USR2 270s h2root mb4i1.hbook
  -- BEGIN TEST OUTPUT --
  
   *** Break *** segmentation violation
   Generating stack trace...
   0x000055aeff2de9c0 in mzlink_ + 0x1ac from h2root
   0x000055aeff2ceda9 in hlimit_ + 0xbd from h2root
   0x000055aeff2bd7ab in main + 0x2d7 from h2root
   0x00007f255ac2ed90 in <unknown> from /lib/x86_64-linux-gnu/libc.so.6
   0x00007f255ac2ee40 in __libc_start_main + 0x80 from /lib/x86_64-linux-gnu/libc.so.6
   0x000055aeff2bd405 in _start + 0x25 from h2root
  
  -- END TEST OUTPUT --
  CMake Error at /github/home/ROOT-CI/build/RootTestDriver.cmake:186 (message):
    got exit code 139 but expected 0
 
 alma9 march=native RelWithDebInfo build   
(sometimes)
    
  -- TEST COMMAND -- 
  cd /github/home/ROOT-CI/build/roottest/root/hist
  /usr/bin/timeout -s USR2 270s h2root mb4i1.hbook
  -- BEGIN TEST OUTPUT --
   RZIODO. Error at record =    2 LUN =    10 IOSTAT =  5001
   >>>>>> CALL RZEND(CHDIR)
   Cannot open fileHROPEN           0
   Error on hropen was 101 
  Error cannot open input file: mb4i1.hbook
  
  -- END TEST OUTPUT --
  CMake Error at /github/home/ROOT-CI/build/RootTestDriver.cmake:186 (message):
    got exit code 1 but expected 0
    

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants