You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2016/11/17 16:09:34 UTC
[23/50] [abbrv] incubator-impala git commit: IMPALA-4446: expr-test
fails under ASAN
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 <ta...@cloudera.com>
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/hadoop-next
Commit: 27e9f0aea1c898b2892b8129c042114d2e67bd6f
Parents: f3d23be
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Tue Nov 8 10:44:38 2016 -0800
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
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);
}
}