IMPALA-4446: expr-test fails under ASAN Various places in the LikePredicate code assumed StringVal is null-terminated. There is no such guarantee. By coincidence string literals were sometimes backed by std::string storage that was null-terminated, so this bug was latent until recently.
Testing: Was able to reproduce the failure locally under ASAN, now the test passes. Running the full ASAN tests to verify, but putting this up for review first to unbreak the build sooner. Change-Id: I0ac10d34dd6463ab52e41de1002ef065cfe63a20 Reviewed-on: http://gerrit.cloudera.org:8080/5000 Reviewed-by: Tim Armstrong <[email protected]> Tested-by: Internal Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/27e9f0ae Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/27e9f0ae Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/27e9f0ae Branch: refs/heads/master Commit: 27e9f0aea1c898b2892b8129c042114d2e67bd6f Parents: f3d23be Author: Tim Armstrong <[email protected]> Authored: Tue Nov 8 10:44:38 2016 -0800 Committer: Internal Jenkins <[email protected]> Committed: Wed Nov 9 02:03:24 2016 +0000 ---------------------------------------------------------------------- be/src/exprs/like-predicate.cc | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/27e9f0ae/be/src/exprs/like-predicate.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/like-predicate.cc b/be/src/exprs/like-predicate.cc index 2e56370..18e0f9a 100644 --- a/be/src/exprs/like-predicate.cc +++ b/be/src/exprs/like-predicate.cc @@ -101,8 +101,7 @@ void LikePredicate::LikePrepareInternal(FunctionContext* context, opts.set_case_sensitive(case_sensitive); state->regex_.reset(new RE2(re_pattern, opts)); if (!state->regex_->ok()) { - context->SetError( - strings::Substitute("Invalid regex: $0", pattern_val.ptr).c_str()); + context->SetError(Substitute("Invalid regex: $0", pattern_str).c_str()); } } } @@ -164,9 +163,8 @@ void LikePredicate::RegexPrepareInternal(FunctionContext* context, opts.set_case_sensitive(case_sensitive); state->regex_.reset(new RE2(pattern_str, opts)); if (!state->regex_->ok()) { - stringstream error; - error << "Invalid regex expression" << pattern->ptr; - context->SetError(error.str().c_str()); + context->SetError( + Substitute("Invalid regex expression: '$0'", pattern_str).c_str()); } state->function_ = ConstantRegexFnPartial; } @@ -202,8 +200,8 @@ void LikePredicate::RegexpLikePrepare(FunctionContext* context, string pattern_str(reinterpret_cast<const char*>(pattern->ptr), pattern->len); state->regex_.reset(new RE2(pattern_str, opts)); if (!state->regex_->ok()) { - error << "Invalid regex expression" << pattern->ptr; - context->SetError(error.str().c_str()); + context->SetError( + Substitute("Invalid regex expression: '$0'", pattern_str).c_str()); } } } @@ -226,11 +224,10 @@ BooleanVal LikePredicate::RegexpLikeInternal(FunctionContext* context, string re_pattern(reinterpret_cast<const char*>(pattern.ptr), pattern.len); re2::RE2 re(re_pattern, opts); if (re.ok()) { - return RE2::PartialMatch(re2::StringPiece( - reinterpret_cast<const char*>(val.ptr), val.len), re); + return RE2::PartialMatch( + re2::StringPiece(reinterpret_cast<const char*>(val.ptr), val.len), re); } else { - context->SetError( - strings::Substitute("Invalid regex: $0", pattern.ptr).c_str()); + context->SetError(Substitute("Invalid regex: $0", re_pattern).c_str()); return BooleanVal(false); } } @@ -353,12 +350,15 @@ BooleanVal LikePredicate::RegexMatch(FunctionContext* context, return RE2::FullMatch(re2::StringPiece( reinterpret_cast<const char*>(operand_value.ptr), operand_value.len), re); } else { - return RE2::PartialMatch(re2::StringPiece( - reinterpret_cast<const char*>(operand_value.ptr), operand_value.len), re); + return RE2::PartialMatch( + re2::StringPiece( + reinterpret_cast<const char*>(operand_value.ptr), operand_value.len), + re); } } else { - context->SetError( - strings::Substitute("Invalid regex: $0", pattern_value.ptr).c_str()); + string pattern_str( + reinterpret_cast<const char*>(pattern_value.ptr), pattern_value.len); + context->SetError(Substitute("Invalid regex: $0", pattern_str).c_str()); return BooleanVal(false); } }
