Skip to content

Fix use of uninit memory in h5dumpgentest #5947

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.

Sign up for GitHub

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

Merged
lrknox merged 10 commits into from
Oct 27, 2025
Merged

Fix use of uninit memory in h5dumpgentest #5947

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
50becac
Fix use of unit memory in h5dumpgentest
mattjala Oct 24, 2025
190a22d
Fix write_dset_in/write_attr_in memory issues
mattjala Oct 24, 2025
c434944
Fix uninit values in gent_fvalues
mattjala Oct 24, 2025
4c989dc
Fix uninit memory in gent_compound_complex2
mattjala Oct 24, 2025
2f40515
Fix uninit memory use in gent_compound_ints
mattjala Oct 24, 2025
f916c9d
Fix uninit memory use in gent_bitnopaquefields
mattjala Oct 24, 2025
477c6f9
Release reference memory
mattjala Oct 24, 2025
69df16f
Clang format
mattjala Oct 24, 2025
217c2c7
Correct init type for obj ref buffer
mattjala Oct 24, 2025
29ccde7
Remove unneeded initialization
mattjala Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 85 additions & 43 deletions tools/test/h5dump/h5dumpgentest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2567,6 +2567,7 @@ gent_nestcomp(void)
/*
* Initialize the data
*/
memset(s1, 0, sizeof(s1));
for (i = 0; i < 10; i++) {
s1[i].a = i;
s1[i].b = (float)(i * i);
Expand Down Expand Up @@ -4115,29 +4116,41 @@ write_attr_in(hid_t loc_id, const char *dset_name, /* for saving reference to da

/* create 1D attributes with dimension [2], 2 elements */
hsize_t dims[1] = {2};
char buf1[2][2] = {"ab", "de"}; /* string, NO NUL fixed length */
char buf2[2] = {1, 2}; /* bitfield, opaque */
s_t buf3[2] = {{1, 2}, {3, 4}}; /* compound */
hobj_ref_t buf4[2]; /* reference */
hvl_t buf5[2]; /* vlen */
hsize_t dimarray[1] = {3}; /* array dimension */
int buf6[2][3] = {{1, 2, 3}, {4, 5, 6}}; /* array */
int buf7[2] = {1, 2}; /* integer */
float buf8[2] = {1, 2}; /* float */
float buf9[4] = {1, 2, 3, 4}; /* complex */
char buf1[2][2] = {"ab", "de"}; /* string, NO NUL fixed length */
char buf2[2] = {1, 2}; /* bitfield, opaque */
s_t buf3[2]; /* compound */
hobj_ref_t buf4[2]; /* reference */
hvl_t buf5[2]; /* vlen */

memset(buf3, 0, sizeof(buf3));
buf3[0].a = 1;
buf3[0].b = 2;
buf3[1].a = 3;
buf3[1].b = 4;
hsize_t dimarray[1] = {3}; /* array dimension */
int buf6[2][3] = {{1, 2, 3}, {4, 5, 6}}; /* array */
int buf7[2] = {1, 2}; /* integer */
float buf8[2] = {1, 2}; /* float */
float buf9[4] = {1, 2, 3, 4}; /* complex */

/* create 2D attributes with dimension [3][2], 6 elements */
hsize_t dims2[2] = {3, 2};
char buf12[6][2] = {"ab", "cd", "ef", "gh", "ij", "kl"}; /* string, NO NUL fixed length */
char buf22[3][2] = {{1, 2}, {3, 4}, {5, 6}}; /* bitfield, opaque */
s_t buf32[6] = {{1, 2}, {3, 4}, {5, 6}, {7, 8}, {9, 10}, {11, 12}}; /* compound */
hobj_ref_t buf42[3][2]; /* reference */
hvl_t buf52[3][2]; /* vlen */
s_t buf32[6]; /* compound */
hobj_ref_t buf42[3][2]; /* reference */
hvl_t buf52[3][2]; /* vlen */
int buf62[6][3] = {{1, 2, 3}, {4, 5, 6}, {7, 8, 9}, {10, 11, 12}, {13, 14, 15}, {16, 17, 18}}; /* array */
int buf72[3][2] = {{1, 2}, {3, 4}, {5, 6}}; /* integer */
float buf82[3][2] = {{1, 2}, {3, 4}, {5, 6}}; /* float */
float buf92[6][2] = {{1, 2}, {3, 4}, {5, 6}, {7, 8}, {9, 10}, {11, 12}}; /* complex */

memset(buf32, 0, sizeof(buf32));

@jhendersonHDF jhendersonHDF Oct 24, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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 mattjala Oct 24, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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 jhendersonHDF Oct 24, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

for (i = 0; i < 6; i++) {
buf32[i].a = (2 * i) + 1;
buf32[i].b = (2 * i) + 2;
}

/* create 3D attributes with dimension [4][3][2], 24 elements */
hsize_t dims3[3] = {4, 3, 2};
char buf13[24][2] = {
Expand All @@ -4152,6 +4165,7 @@ write_attr_in(hid_t loc_id, const char *dset_name, /* for saving reference to da
float buf83[4][3][2]; /* float */
float buf93[24][2]; /* complex */

memset(buf33, 0, sizeof(buf33));
/*-------------------------------------------------------------------------
* 1D attributes
*-------------------------------------------------------------------------
Expand Down Expand Up @@ -4573,29 +4587,41 @@ write_dset_in(hid_t loc_id, const char *dset_name, /* for saving reference to da

/* create 1D attributes with dimension [2], 2 elements */
hsize_t dims[1] = {2};
char buf1[2][2] = {"ab", "de"}; /* string, NO NUL fixed length */
char buf2[2] = {1, 2}; /* bitfield, opaque */
s_t buf3[2] = {{1, 2}, {3, 4}}; /* compound */
hobj_ref_t buf4[2]; /* reference */
hvl_t buf5[2]; /* vlen */
hsize_t dimarray[1] = {3}; /* array dimension */
int buf6[2][3] = {{1, 2, 3}, {4, 5, 6}}; /* array */
int buf7[2] = {1, 2}; /* integer */
float buf8[2] = {1, 2}; /* float */
float buf9[4] = {1, 2, 3, 4}; /* complex */
char buf1[2][2] = {"ab", "de"}; /* string, NO NUL fixed length */
char buf2[2] = {1, 2}; /* bitfield, opaque */
s_t buf3[2]; /* compound */
hobj_ref_t buf4[2]; /* reference */
hvl_t buf5[2]; /* vlen */

memset(buf3, 0, sizeof(buf3));
buf3[0].a = 1;
buf3[0].b = 2;
buf3[1].a = 3;
buf3[1].b = 4;
hsize_t dimarray[1] = {3}; /* array dimension */
int buf6[2][3] = {{1, 2, 3}, {4, 5, 6}}; /* array */
int buf7[2] = {1, 2}; /* integer */
float buf8[2] = {1, 2}; /* float */
float buf9[4] = {1, 2, 3, 4}; /* complex */

/* create 2D attributes with dimension [3][2], 6 elements */
hsize_t dims2[2] = {3, 2};
char buf12[6][2] = {"ab", "cd", "ef", "gh", "ij", "kl"}; /* string, NO NUL fixed length */
char buf22[3][2] = {{1, 2}, {3, 4}, {5, 6}}; /* bitfield, opaque */
s_t buf32[6] = {{1, 2}, {3, 4}, {5, 6}, {7, 8}, {9, 10}, {11, 12}}; /* compound */
hobj_ref_t buf42[3][2]; /* reference */
hvl_t buf52[3][2]; /* vlen */
s_t buf32[6]; /* compound */
hobj_ref_t buf42[3][2]; /* reference */
hvl_t buf52[3][2]; /* vlen */
int buf62[6][3] = {{1, 2, 3}, {4, 5, 6}, {7, 8, 9}, {10, 11, 12}, {13, 14, 15}, {16, 17, 18}}; /* array */
int buf72[3][2] = {{1, 2}, {3, 4}, {5, 6}}; /* integer */
float buf82[3][2] = {{1, 2}, {3, 4}, {5, 6}}; /* float */
float buf92[6][2] = {{1, 2}, {3, 4}, {5, 6}, {7, 8}, {9, 10}, {11, 12}}; /* complex */

memset(buf32, 0, sizeof(buf32));
for (int i = 0; i < 6; i++) {
buf32[i].a = (2 * i) + 1;
buf32[i].b = (2 * i) + 2;
}

/* create 3D attributes with dimension [4][3][2], 24 elements */
hsize_t dims3[3] = {4, 3, 2};
char buf13[24][2] = {
Expand All @@ -4610,6 +4636,7 @@ write_dset_in(hid_t loc_id, const char *dset_name, /* for saving reference to da
float buf83[4][3][2]; /* float */
float buf93[24][2]; /* complex */

memset(buf33, 0, sizeof(buf33));
/*-------------------------------------------------------------------------
* 1D
*-------------------------------------------------------------------------
Expand Down Expand Up @@ -6080,20 +6107,29 @@ gent_fvalues(void)
double b;
} c_t;

hid_t fid; /* file id */
hid_t dcpl; /* dataset creation property list */
hid_t sid; /* dataspace ID */
hid_t tid; /* datatype ID */
hid_t did; /* datasetID */
hsize_t dims[1] = {2};
int buf[2] = {1, 2}; /* integer */
int fillval1 = -99; /* integer fill value */
c_t buf2[2] = {{1, 2}, {3, 4}}; /* compound */
c_t fillval2[1] = {{1, 2}}; /* compound fill value */
hvl_t buf3[2]; /* vlen */
hvl_t fillval3; /* vlen fill value */
hsize_t dimarray[1] = {3}; /* array dimension */
int buf4[2][3] = {{1, 2, 3}, {4, 5, 6}}; /* array */
hid_t fid; /* file id */
hid_t dcpl; /* dataset creation property list */
hid_t sid; /* dataspace ID */
hid_t tid; /* datatype ID */
hid_t did; /* datasetID */
hsize_t dims[1] = {2};
int buf[2] = {1, 2}; /* integer */
int fillval1 = -99; /* integer fill value */
c_t buf2[2]; /* compound */
c_t fillval2[1]; /* compound fill value */
hvl_t buf3[2]; /* vlen */
hvl_t fillval3; /* vlen fill value */
hsize_t dimarray[1] = {3}; /* array dimension */

memset(buf2, 0, sizeof(buf2));
buf2[0].a = 1;
buf2[0].b = 2;
buf2[1].a = 3;
buf2[1].b = 4;
memset(fillval2, 0, sizeof(fillval2));
fillval2[0].a = 1;
fillval2[0].b = 2;
int buf4[2][3] = {{1, 2, 3}, {4, 5, 6}}; /* array */
int H5_ATTR_NDEBUG_UNUSED ret;

/* create a file */
Expand Down Expand Up @@ -10195,9 +10231,9 @@ gent_compound_ints(void)
int m; /* Array init loop vars */

/* Allocate buffers */
Cmpd1 = (Cmpd1Struct *)malloc(sizeof(Cmpd1Struct) * F77_LENGTH);
Cmpd1 = (Cmpd1Struct *)calloc(F77_LENGTH, sizeof(Cmpd1Struct));
assert(Cmpd1);
Cmpd2 = (Cmpd2Struct *)malloc(sizeof(Cmpd2Struct) * F77_LENGTH);
Cmpd2 = (Cmpd2Struct *)calloc(F77_LENGTH, sizeof(Cmpd2Struct));
assert(Cmpd2);

/* Initialize the data in the arrays/datastructure */
Expand Down Expand Up @@ -11217,6 +11253,8 @@ gent_bitnopaquefields(void)
uint64_t buf4[F80_DIM32]; /* bitfield, opaque */
s_t buf5[F80_DIM32]; /* compound */

memset(buf5, 0, sizeof(buf5));

file_id = H5Fcreate(FILE80, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT);

if ((grp = H5Gcreate2(file_id, "bittypetests", H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) >= 0) {
Expand Down Expand Up @@ -11468,7 +11506,7 @@ gent_compound_complex2(void)
hsize_t nelmts = F82_DIM32;

/* Allocate buffer */
buf = (compound *)malloc(sizeof(compound) * F82_DIM32);
buf = (compound *)calloc(F82_DIM32, sizeof(compound));
assert(buf);

file = H5Fcreate(FILE82, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT);
Expand Down Expand Up @@ -14289,6 +14327,10 @@ gent_test_reference_external(void)
if (H5Dwrite(dataset, H5T_STD_REF, H5S_ALL, H5S_ALL, H5P_DEFAULT, ref_wbuf) < 0)
return 1;

/* Destroy references to free allocated memory */
for (i = 0; i < SPACE1_DIM1; i++)
H5Rdestroy(&ref_wbuf[i]);

@jhendersonHDF jhendersonHDF Oct 24, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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 mattjala Oct 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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


/* Close disk dataspace */
if (H5Sclose(sid) < 0)
return 1;
Expand Down
Loading