IMPALA-3308: Get expr-test passing on PPC64LE When using gcc 5+ (which introduced a new library ABI that includes new implementations of std::string) to build Impala, the copy of the class ExprValue(std::string) would be unsafe as string_val.ptr will not be updated to point to the relocated string_data.data().
In order to solve this issue, we need to change how we initialize value_ so that it is initialized in-place, rather than created as a temporary on the stack and then copied. Change-Id: I4504ee6a52a085f530aadfcfa009bacb83c64787 Reviewed-on: http://gerrit.cloudera.org:8080/4186 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/9cee2b5f Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/9cee2b5f Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/9cee2b5f Branch: refs/heads/master Commit: 9cee2b5f1376ad6286ed65edffe6d152a0012cf1 Parents: 1ccd83b Author: segelyang <[email protected]> Authored: Wed Aug 31 18:13:17 2016 +0800 Committer: Internal Jenkins <[email protected]> Committed: Wed Sep 28 23:09:28 2016 +0000 ---------------------------------------------------------------------- be/src/exprs/expr-value.h | 9 +++++---- be/src/exprs/literal.cc | 14 ++++++-------- 2 files changed, 11 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/9cee2b5f/be/src/exprs/expr-value.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/expr-value.h b/be/src/exprs/expr-value.h index cbe1361..a2336d3 100644 --- a/be/src/exprs/expr-value.h +++ b/be/src/exprs/expr-value.h @@ -67,10 +67,9 @@ struct ExprValue { ExprValue(double v) : double_val(v) {} ExprValue(int64_t t, int64_t n) : timestamp_val(t, n) {} - /// c'tor for string values - ExprValue(const std::string& str) - : string_data(str) { - string_val.ptr = const_cast<char*>(string_data.data()); + void Init(const std::string& str) { + string_data = str; + string_val.ptr = &string_data[0]; string_val.len = string_data.size(); } @@ -198,6 +197,8 @@ struct ExprValue { private: std::string string_data; // Stores the data for string_val if necessary. + + DISALLOW_COPY_AND_ASSIGN(ExprValue); }; } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/9cee2b5f/be/src/exprs/literal.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/literal.cc b/be/src/exprs/literal.cc index cc089da..1d816c3 100644 --- a/be/src/exprs/literal.cc +++ b/be/src/exprs/literal.cc @@ -74,7 +74,7 @@ Literal::Literal(const TExprNode& node) case TYPE_VARCHAR: { DCHECK_EQ(node.node_type, TExprNodeType::STRING_LITERAL); DCHECK(node.__isset.string_literal); - value_ = ExprValue(node.string_literal.value); + value_.Init(node.string_literal.value); if (type_.type == TYPE_VARCHAR) { value_.string_val.len = min(type_.len, value_.string_val.len); } @@ -91,7 +91,7 @@ Literal::Literal(const TExprNode& node) // Pad out literal with spaces. str.replace(str_len, type_.len - str_len, type_.len - str_len, ' '); } - value_ = ExprValue(str); + value_.Init(str); break; } case TYPE_DECIMAL: { @@ -186,16 +186,14 @@ Literal::Literal(ColumnType type, double v) } } -Literal::Literal(ColumnType type, const string& v) - : Expr(type), - value_(v) { +Literal::Literal(ColumnType type, const string& v) : Expr(type) { + value_.Init(v); DCHECK(type.type == TYPE_STRING || type.type == TYPE_CHAR || type.type == TYPE_VARCHAR) << type; } -Literal::Literal(ColumnType type, const StringValue& v) - : Expr(type), - value_(v.DebugString()) { +Literal::Literal(ColumnType type, const StringValue& v) : Expr(type) { + value_.Init(v.DebugString()); DCHECK(type.type == TYPE_STRING || type.type == TYPE_CHAR) << type; }
