[impala] 01/02: IMPALA-5031: signed overflow in TimestampValue

[ar...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

arodoni pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 436a70e604d92ba17d11680dfdccc95988257a98
Author: Jim Apple <jb...@apache.org>
AuthorDate: Sat Nov 10 17:39:23 2018 -0800

    IMPALA-5031: signed overflow in TimestampValue
    
    The standard says that overflow for signed arithmetic operations is
    undefined behavior; see [expr]:
    
        If during the evaluation of an expression, the result is not
        mathematically defined or not in the range of representable values
        for its type, the behavior is undefined.
    
    This patch fixes a signed overflow with the folowing backtrace
    (uninteresting parts elided):
    
    runtime/timestamp-value.inline.h:67:13: runtime error: signed integer overflow: -9223372036854775808 + -9223372037 cannot be represented in type 'long'
        #0 TimestampValue::FromUnixTimeNanos(long, long, cctz::time_zone const&) runtime/timestamp-value.inline.h:67:13
        #1 TimestampValue::FromSubsecondUnixTime(double, cctz::time_zone const&) runtime/timestamp-value.inline.h:62:10
        #2 CastFunctions::CastToTimestampVal(impala_udf::FunctionContext*, impala_udf::FloatVal const&) exprs/cast-functions-ir.cc:248:172
        #3 impala_udf::TimestampVal ScalarFnCall::InterpretEval<impala_udf::TimestampVal>(ScalarExprEvaluator*, TupleRow const*) const exprs/scalar-fn-call.cc:485:208
        #4 ScalarFnCall::GetTimestampVal(ScalarExprEvaluator*, TupleRow const*) const exprs/scalar-fn-call.cc:608:44
        #5 ScalarExprEvaluator::GetValue(ScalarExpr const&, TupleRow const*) exprs/scalar-expr-evaluator.cc:314:41
        #6 ScalarExprEvaluator::GetValue(TupleRow const*) exprs/scalar-expr-evaluator.cc:250:10
        #7 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, ScalarExprEvaluator* const*, MemPool*, StringValue**, int*, int*) runtime/tuple.cc:222:27
        #8 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, vector<ScalarExprEvaluator*> const&, MemPool*, vector<StringValue*>*, int*) runtime/tuple.h:174:5
        #9 UnionNode::MaterializeExprs(vector<ScalarExprEvaluator*> const&, TupleRow*, unsigned char*, RowBatch*) exec/union-node-ir.cc:29:14
        #10 UnionNode::GetNextConst(RuntimeState*, RowBatch*) exec/union-node.cc:263:5
        #11 UnionNode::GetNext(RuntimeState*, RowBatch*, bool*) exec/union-node.cc:296:45
    
    This was seen in the backend test ExprTest.CastExprs.
    
    Change-Id: Iaad158e6634314a5690a43a0cc04426c1aba8f41
    Reviewed-on: http://gerrit.cloudera.org:8080/11919
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/runtime/timestamp-value.inline.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/be/src/runtime/timestamp-value.inline.h b/be/src/runtime/timestamp-value.inline.h
index 2e2adac..1aebd78 100644
--- a/be/src/runtime/timestamp-value.inline.h
+++ b/be/src/runtime/timestamp-value.inline.h
@@ -28,6 +28,7 @@
 #include "exprs/timezone_db.h"
 #include "kudu/util/int128.h"
 #include "gutil/walltime.h"
+#include "util/arithmetic-util.h"
 
 namespace impala {
 
@@ -70,7 +71,8 @@ inline TimestampValue TimestampValue::UtcFromUnixTimeLimitedRangeNanos(
 
 inline TimestampValue TimestampValue::FromUnixTimeNanos(time_t unix_time, int64_t nanos,
     const Timezone& local_tz) {
-  unix_time += SplitTime<NANOS_PER_SEC>(&nanos);
+  unix_time =
+      ArithmeticUtil::AsUnsigned<std::plus>(unix_time, SplitTime<NANOS_PER_SEC>(&nanos));
   TimestampValue result = FromUnixTime(unix_time, local_tz);
   // 'nanos' is guaranteed to be between [0,NANOS_PER_SEC) at this point, so the
   // next addition cannot change the day or step to a different timezone.