Fix use of uninit memory in h5dumpgentest

[mattjala]

Important

Fix uninitialized memory usage in h5dumpgentest.c by initializing arrays and structures, using calloc, and adding memory deallocation.

• Memory Initialization:
  • Initialize arrays s1, buf3, buf32, buf33, buf2, fillval2, buf5, buf52, buf53, buf42, buf43, buf4, buf, buf2, buf3, buf4, buf5 to zero in h5dumpgentest.c.
  • Use memset to initialize compound structures buf3, buf32, buf2, fillval2.
• Memory Allocation:
  • Replace malloc with calloc for Cmpd1, Cmpd2, and buf to ensure zero-initialized memory.
• Memory Deallocation:
  • Add loop to destroy references in gent_test_reference_external() to free allocated memory.

^{This description was created by for 69df16f. You can customize this summary. It will automatically update as commits are pushed.}

[jhendersonHDF]

Something like this seems more like fixing things in the wrong place. The initializer for buf32 was already initializing it in the same way and any uninitialized bits in the structure shouldn't really matter for this purpose. It's reasonable to do, but making sure the structures are completely zeroed out should not generally be a requirement, so it seems like this is more of a problem to be fixed with the Cache VOL.

[mattjala]

Actually, it's the Native VOL that has uninitialized memory access during write_attr_in() and gent_nestcomp(). Though this could still indicate that the library isn't taking the proper care in handling buffers.

The other invalid accesses (aside from the reference one) are exclusive to the Cache VOL, and I'll investigate what's going on there.

For posterity's sake, here's the exact memory trace for the Native VOL uninitialized accesses:

==2064844== Syscall param pwrite64(buf) points to uninitialised byte(s)
==2064844==    at 0x55E27FA: pwrite (pwrite64.c:25)
==2064844==    by 0x4C12546: H5FD__sec2_write (H5FDsec2.c:754)
==2064844==    by 0x4BDF483: H5FD_write (H5FDint.c:338)
==2064844==    by 0x4B6C35C: H5F__accum_write (H5Faccum.c:821)
==2064844==    by 0x4EA714F: H5PB_write (H5PB.c:1003)
==2064844==    by 0x4B89E01: H5F_shared_block_write (H5Fio.c:177)
==2064844==    by 0x4AD2EF9: H5D__flush_sieve_buf (H5Dint.c:3265)
==2064844==    by 0x4AA5448: H5D__contig_flush (H5Dcontig.c:1597)
==2064844==    by 0x4AD3288: H5D__flush_real (H5Dint.c:3301)
==2064844==    by 0x4ACC33A: H5D_close (H5Dint.c:1963)
==2064844==    by 0x51D912F: H5VL__native_dataset_close (H5VLnative_dataset.c:826)
==2064844==    by 0x519B265: H5VL__dataset_close (H5VLcallback.c:2780)
==2064844==  Address 0x5a83989 is 25 bytes inside a block of size 328 alloc'd
==2064844==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2064844==    by 0x4C2252C: H5FL__malloc (H5FL.c:211)
==2064844==    by 0x4C24C25: H5FL_blk_malloc (H5FL.c:773)
==2064844==    by 0x4C24F8A: H5FL_blk_calloc (H5FL.c:819)
==2064844==    by 0x4AA41B8: H5D__contig_writevv_sieve_cb (H5Dcontig.c:1319)
==2064844==    by 0x51EF711: H5VM_opvv (H5VM.c:1261)
==2064844==    by 0x4AA508D: H5D__contig_writevv (H5Dcontig.c:1554)
==2064844==    by 0x4AEF027: H5D__select_io (H5Dselect.c:223)
==2064844==    by 0x4AF0721: H5D__select_write (H5Dselect.c:482)
==2064844==    by 0x4AA2800: H5D__contig_write (H5Dcontig.c:967)
==2064844==    by 0x4ADC646: H5D__write (H5Dio.c:829)
==2064844==    by 0x51D7189: H5VL__native_dataset_write (H5VLnative_dataset.c:420)
==2064844==  Uninitialised value was created by a stack allocation
==2064844==    at 0x13AC0C: gent_nestcomp (h5dumpgentest.c:2537)
==2064844== 
==2064844== Syscall param pwrite64(buf) points to uninitialised byte(s)
==2064844==    at 0x55E27FA: pwrite (pwrite64.c:25)
==2064844==    by 0x4C12546: H5FD__sec2_write (H5FDsec2.c:754)
==2064844==    by 0x4BDF483: H5FD_write (H5FDint.c:338)
==2064844==    by 0x4B6B8BB: H5F__accum_write (H5Faccum.c:631)
==2064844==    by 0x4EA714F: H5PB_write (H5PB.c:1003)
==2064844==    by 0x4B8A2F4: H5F_block_write (H5Fio.c:218)
==2064844==    by 0x49F32EC: H5C__flush_single_entry (H5Centry.c:600)
==2064844==    by 0x4A289B4: H5C__flush_ring (H5Cint.c:1748)
==2064844==    by 0x49E1B85: H5C_flush_cache (H5C.c:710)
==2064844==    by 0x49962BA: H5AC_flush (H5AC.c:645)
==2064844==    by 0x4B7F073: H5F__flush_phase2 (H5Fint.c:2346)
==2064844==    by 0x4B7B8D8: H5F__dest (H5Fint.c:1441)
==2064844==  Address 0x58346e1 is 353 bytes inside a block of size 4,104 alloc'd
==2064844==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2064844==    by 0x4C2252C: H5FL__malloc (H5FL.c:211)
==2064844==    by 0x4C24C25: H5FL_blk_malloc (H5FL.c:773)
==2064844==    by 0x4C2577A: H5FL_blk_realloc (H5FL.c:936)
==2064844==    by 0x4B6A899: H5F__accum_adjust (H5Faccum.c:376)
==2064844==    by 0x4B6AF24: H5F__accum_write (H5Faccum.c:459)
==2064844==    by 0x4EA714F: H5PB_write (H5PB.c:1003)
==2064844==    by 0x4B8A2F4: H5F_block_write (H5Fio.c:218)
==2064844==    by 0x49F32EC: H5C__flush_single_entry (H5Centry.c:600)
==2064844==    by 0x4A289B4: H5C__flush_ring (H5Cint.c:1748)
==2064844==    by 0x49E1B85: H5C_flush_cache (H5C.c:710)
==2064844==    by 0x49962BA: H5AC_flush (H5AC.c:645)
==2064844==  Uninitialised value was created by a stack allocation
==2064844==    at 0x141855: write_attr_in (h5dumpgentest.c:4100)

[jhendersonHDF]

I see; I had mistakenly assumed these were fixes for the Cache VOL issue. These traces are fairly normal occurrences though and are not likely pointing at actual issues. There are many cases where you may end up writing uninitialized data using pwrite, such as in this case with uninitialized bits of a struct. No real harm zeroing them out though.

[jhendersonHDF]

However, I could see this missing destroy call being a source of problems. Have you verified whether this change alone fixes the crash seen?

[mattjala]

This isn't (directly) tied to any of the crashes in CI, I just discovered the memory loss and uninit access while looking into that. The CI crash should be fixed by HDFGroup/vol-cache#40

[jhendersonHDF]

Note that most of these arrays probably don't really need to be initialized though, except maybe buf33 and buf53. And since hobj_ref_t is aliased to haddr_t, HADDR_UNDEF is probably a more appropriate initializer than 0.

[jhendersonHDF]

This will just initialize the first element to HADDR_UNDEF and the rest to 0. Unless these arrays cause an informational message about uninitialized bytes that you want to avoid, it's probably simpler to avoid initializing them explicitly and just let them be default-initialized to zeroes. Otherwise, you'd need to loop through all the elements in buf43 which is probably overkill for this program.

[mattjala]

I've gone through and trimmed the initializations to only be applied to the buffers that valgrind was picking up, and to zero out the entire buffer instead of just the first element. I think this is a worthwhile change since we want it to be easy to detect any real memory errors that get introduced later on.