From 7cf06fff7f28f3b729bf8ce0854f4e720af4c031 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 8 Apr 2021 12:49:43 -0700 Subject: [PATCH 1/3] HDF5: Fix Segfault with libSplash Files Fix a segfault (on some systems) with legacy libSplash HDF5 files. From the docs: ``` Name: H5Tget_member_name ... The HDF5 Library allocates a buffer to receive the name of the field. The caller must subsequently free the buffer with H5free_memory. ``` Refs.: - https://support.hdfgroup.org/HDF5/doc/RM/RM_H5T.html#Datatype-GetMemberName - https://support.hdfgroup.org/HDF5/doc/RM/RM_H5.html#Library-FreeMemory --- src/IO/HDF5/HDF5IOHandler.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index 3aa32453b8..e7eba13d3c 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/src/IO/HDF5/HDF5IOHandler.cpp @@ -1465,9 +1465,9 @@ HDF5IOHandlerImpl::readAttribute(Writable* writable, char* m2 = H5Tget_member_name(attr_type, 2); if(strcmp("x", m0) != 0 || strcmp("y", m1) != 0 || strcmp("z", m2) != 0) isLegacyLibSplashAttr = false; - free(m2); - free(m1); - free(m0); + H5free_memory(m2); + H5free_memory(m1); + H5free_memory(m0); } if( isLegacyLibSplashAttr ) { From 63db1f5d161daca344ebd3d660eb651e5ffb80c7 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 8 Apr 2021 13:57:41 -0700 Subject: [PATCH 2/3] avoid undefined behavior in str(n)cmp Passing potential `nullptr`s is undefined behavior. --- src/IO/HDF5/HDF5IOHandler.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index e7eba13d3c..8435c17361 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/src/IO/HDF5/HDF5IOHandler.cpp @@ -1424,8 +1424,9 @@ HDF5IOHandlerImpl::readAttribute(Writable* writable, { char* m0 = H5Tget_member_name(attr_type, 0); char* m1 = H5Tget_member_name(attr_type, 1); - if( (strncmp("TRUE" , m0, 4) == 0) && (strncmp("FALSE", m1, 5) == 0) ) - attrIsBool = true; + if( m0 != nullptr && m1 != nullptr ) + if( (strncmp("TRUE" , m0, 4) == 0) && (strncmp("FALSE", m1, 5) == 0) ) + attrIsBool = true; H5free_memory(m1); H5free_memory(m0); } @@ -1446,8 +1447,9 @@ HDF5IOHandlerImpl::readAttribute(Writable* writable, { char* m0 = H5Tget_member_name(attr_type, 0); char* m1 = H5Tget_member_name(attr_type, 1); - if( (strncmp("r" , m0, 4) == 0) && (strncmp("i", m1, 5) == 0) ) - isComplexType = true; + if( m0 != nullptr && m1 != nullptr ) + if( (strncmp("r" , m0, 1) == 0) && (strncmp("i", m1, 1) == 0) ) + isComplexType = true; H5free_memory(m1); H5free_memory(m0); } @@ -1463,7 +1465,9 @@ HDF5IOHandlerImpl::readAttribute(Writable* writable, char* m0 = H5Tget_member_name(attr_type, 0); char* m1 = H5Tget_member_name(attr_type, 1); char* m2 = H5Tget_member_name(attr_type, 2); - if(strcmp("x", m0) != 0 || strcmp("y", m1) != 0 || strcmp("z", m2) != 0) + if( m0 == nullptr || m1 == nullptr || m2 == nullptr ) + isLegacyLibSplashAttr = false; + else if(strcmp("x", m0) != 0 || strcmp("y", m1) != 0 || strcmp("z", m2) != 0) isLegacyLibSplashAttr = false; H5free_memory(m2); H5free_memory(m1); @@ -1498,7 +1502,7 @@ HDF5IOHandlerImpl::readAttribute(Writable* writable, a = Attribute(cld); } else - throw unsupported_data_error("[HDF5] Unknow complex type representation"); + throw unsupported_data_error("[HDF5] Unknown complex type representation"); } else throw unsupported_data_error("[HDF5] Compound attribute type not supported"); From e5f4ceb2ba17b63fcf17ebeb411b7ffd039e0485 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 8 Apr 2021 15:06:41 -0700 Subject: [PATCH 3/3] Clang-Tidy: Intentional Branch Clone --- src/IO/HDF5/HDF5IOHandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index 8435c17361..9056648078 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/src/IO/HDF5/HDF5IOHandler.cpp @@ -1466,7 +1466,7 @@ HDF5IOHandlerImpl::readAttribute(Writable* writable, char* m1 = H5Tget_member_name(attr_type, 1); char* m2 = H5Tget_member_name(attr_type, 2); if( m0 == nullptr || m1 == nullptr || m2 == nullptr ) - isLegacyLibSplashAttr = false; + isLegacyLibSplashAttr = false; // NOLINT(bugprone-branch-clone) else if(strcmp("x", m0) != 0 || strcmp("y", m1) != 0 || strcmp("z", m2) != 0) isLegacyLibSplashAttr = false; H5free_memory(m2);