From 5c286101c8d96d94eaf58c5a8ae0dd3fa7f467bf Mon Sep 17 00:00:00 2001 From: David Sisson Date: Fri, 11 Aug 2023 16:29:33 -0700 Subject: [PATCH 1/4] Add address and leak sanitization to the debug builds. --- CMakeLists.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index bd7fe7e6..d4f78ef9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,6 +11,12 @@ set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED True) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) +add_compile_options($<$:-fsanitize=undefined>) +add_link_options($<$:-fsanitize=undefined>) + +add_compile_options($<$:-fsanitize=address>) +add_link_options($<$:-fsanitize=address>) + option( SUBSTRAIT_CPP_BUILD_TESTING "Enable substrait-cpp tests. This will enable all other build options automatically." From c69442869a281ba4de4616f05f9786ed19687d8e Mon Sep 17 00:00:00 2001 From: David Sisson Date: Fri, 11 Aug 2023 17:03:27 -0700 Subject: [PATCH 2/4] Fixed an issue discovered by the address sanitizer. --- src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp b/src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp index 403caa22..a26c964a 100644 --- a/src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp +++ b/src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp @@ -109,7 +109,7 @@ ::substrait::proto::Type SubstraitPlanTypeVisitor::typeToProto( } case TypeKind::kVarchar: { auto varChar = - reinterpret_cast(&decodedType); + reinterpret_cast(&decodedType); if (varChar == nullptr) { break; } From 340ec9ce9a914cf18e14b45b3a86b8dd1671559e Mon Sep 17 00:00:00 2001 From: David Sisson Date: Mon, 14 Aug 2023 18:16:55 -0700 Subject: [PATCH 3/4] Fix an issues processing varchar types. --- src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp | 5 +++++ src/substrait/type/Type.cpp | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp b/src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp index a26c964a..3d47206d 100644 --- a/src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp +++ b/src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp @@ -114,6 +114,11 @@ ::substrait::proto::Type SubstraitPlanTypeVisitor::typeToProto( break; } try { + if (!varChar->length()->isInteger()) { + errorListener_->addError( + ctx->getStart(), "Missing varchar length."); + break; + } int32_t length = std::stoi(varChar->length()->value()); type.mutable_varchar()->set_length(length); } catch (...) { diff --git a/src/substrait/type/Type.cpp b/src/substrait/type/Type.cpp index 1fa21115..5a0e3cff 100644 --- a/src/substrait/type/Type.cpp +++ b/src/substrait/type/Type.cpp @@ -197,7 +197,12 @@ ParameterizedTypePtr ParameterizedType::decode( const auto& leftAngleBracketPos = matchingType.find('<'); if (leftAngleBracketPos == std::string::npos) { - bool nullable = matchingType.back() == '?'; + bool nullable; + if (matchingType.empty()) { + nullable = false; + } else { + nullable = matchingType.back() == '?'; + } // deal with type and with a question mask like "i32?". const auto& baseType = nullable ? matchingType = matchingType.substr(0, questionMaskPos) From 1873beb41b11bdda3b697afe47cc22f115ee520b Mon Sep 17 00:00:00 2001 From: David Sisson Date: Tue, 15 Aug 2023 00:54:06 -0700 Subject: [PATCH 4/4] Ran clang format. --- src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp b/src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp index 3d47206d..5b42d817 100644 --- a/src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp +++ b/src/substrait/textplan/parser/SubstraitPlanTypeVisitor.cpp @@ -115,8 +115,7 @@ ::substrait::proto::Type SubstraitPlanTypeVisitor::typeToProto( } try { if (!varChar->length()->isInteger()) { - errorListener_->addError( - ctx->getStart(), "Missing varchar length."); + errorListener_->addError(ctx->getStart(), "Missing varchar length."); break; } int32_t length = std::stoi(varChar->length()->value());