memory overflow reported in tst_h_dimscales

[edhartnett]

I built the HEAD with gcc address/memory checking turned on. Turns out there are some memory leaks/overflows.

See #504 for build info.

./tst_h_dimscales

*** Checking HDF5 dimension scales.
*** Creating simple dimension scales file...ok.
*** Checking that simple dimscale file can be read...ok.
*** Creating simple dimension scales file with lots of datasets...ok.
*** Creating a file with an unlimited dimension scale...ok.
=================================================================
==22535==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe37d95610 at pc 0x557a8a8165d8 bp 0x7ffe37d94fa0 sp 0x7ffe37d94f90
WRITE of size 8 at 0x7ffe37d95610 thread T0
    #0 0x557a8a8165d7 in alien_visitor /home/ed/tmp/netcdf-c/h5_test/tst_h_dimscales.c:25
    #1 0x7fbc424f0ed7 in H5DSiterate_scales /home/ed/Downloads/hdf5-1.10.1/hl/src/H5DS.c:1357
    #2 0x557a8a81b104 in main /home/ed/tmp/netcdf-c/h5_test/tst_h_dimscales.c:437
    #3 0x7fbc4195c3f0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x203f0)
    #4 0x557a8a816309 in _start (/home/ed/tmp/netcdf-c/h5_test/tst_h_dimscales+0x2309)

Address 0x7ffe37d95610 is located in stack of thread T0 at offset 32 in frame
    #0 0x557a8a817697 in main /home/ed/tmp/netcdf-c/h5_test/tst_h_dimscales.c:118

  This frame has 12 object(s):
    [32, 36) 'visitor_data' <== Memory access at offset 32 partially overflows this variable
    [96, 104) 'num_obj'
    [160, 168) 'i'
    [224, 232) 'num_obj'
    [288, 296) 'dims'
    [352, 360) 'maxdims'
    [416, 432) 'dims'
    [480, 504) 'dims'
    [544, 568) 'max_dims'
    [608, 4608) 'var1_datasetid'
    [4640, 4896) 'obj_name'
    [4928, 5184) 'label'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /home/ed/tmp/netcdf-c/h5_test/tst_h_dimscales.c:25 in alien_visitor
Shadow bytes around the buggy address:
  0x100046faaa70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100046faaa80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100046faaa90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100046faaaa0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100046faaab0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1
=>0x100046faaac0: f1 f1[04]f4 f4 f4 f2 f2 f2 f2 00 f4 f4 f4 f2 f2
  0x100046faaad0: f2 f2 00 f4 f4 f4 f2 f2 f2 f2 00 f4 f4 f4 f2 f2
  0x100046faaae0: f2 f2 00 f4 f4 f4 f2 f2 f2 f2 00 f4 f4 f4 f2 f2
  0x100046faaaf0: f2 f2 00 00 f4 f4 f2 f2 f2 f2 00 00 00 f4 f2 f2
  0x100046faab00: f2 f2 00 00 00 f4 f2 f2 f2 f2 00 00 00 00 00 00
  0x100046faab10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==22535==ABORTING> 

[WardF]

The tests generally don’t seem to clean up after themselves, and since they’re tests I haven’t prioritized cleaning them up at this point. Any memory issues in the main library or utilities are of course a much different matter. I may or may not close these out after tagging them to revisit in the future tho. And I’ll takr a look at all the other ones you’ve opened, but for the time being they’re lower priority :). Thanks!

[edhartnett]

Keep them open and I will take care of them.

I've got a fix for tst_h_dimscales, I will prepare a PR.

I disagree about your priorities. Getting the tests to run without memory errors is very important, because otherwise you cannot be running these memory checks on all code changes.

I need a clean memory build for PIO to work well. In HPC memory leaks are much to be feared. A small leak becomes a big overall leak when it is running on 500K cores. And the long runs and complex IO of climate and Earth system models will allow memory leaks to accumulate.

[WardF]

I agree with their importance, but running static analysis with tests turns on shows an abundance of issues (many related to memory allocated but not explicitly freed); addressing all of those are important, but we don't have the resources to chase them and address other more explicit/obvious bugs. I appreciate your PR, and opening these issues. I'll leave them open, and now that I've scanned them I see some are not specific to the tests, so I will work on those as well. Have you seen our coverity dashboard? If not, I'd be happy to add you as a collaborator there.

[edhartnett]

By all means add me as a collaborator. I will take a look at some of the other memory issues. The tst_h_dimscales.c one was actually my own code so fair enough that I should be the one to fix it. ;-)

…

On Mon, Oct 23, 2017 at 9:47 AM, Ward Fisher ***@***.***> wrote: I agree with their importance, but running static analysis with tests turns on shows an abundance of issues (many related to memory allocated but not explicitly freed); addressing all of those are important, but we don't have the resources to chase them and address other more explicit/obvious bugs. I appreciate your PR, and opening these issues. I'll leave them open, and now that I've scanned them I see some are not specific to the tests, so I will work on those as well. Have you seen our coverity dashboard? If not, I'd be happy to add you as a collaborator there. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#505 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEUr3JX81SPUWCkts0uOf6b2pZsvAYi0ks5svLUdgaJpZM4QC955> .