impala-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jruss...@apache.org
Subject [1/4] incubator-impala git commit: IMPALA-5912: fix crash in trunc(..., "WW") in release build
Date Tue, 12 Sep 2017 07:16:12 GMT
Repository: incubator-impala
Updated Branches:
  refs/heads/master 2fbdc8e37 -> 03571788b


IMPALA-5912: fix crash in trunc(..., "WW") in release build

The bug is with the pattern below:

  const date& d = TruncMonth(orig_date).date();

The problem is that 'd' is a reference into the temporary returned from
TruncMonth. But the temporary is only guaranteed to live until the
expression has been evaluated. Thus if the compiler re-uses the register
or stack slot holding the temporary, 'd' may end up pointing to a bogus
value, causing a crash or incorrect results.

The fix is to simply create a local date value with the required date,
which avoids use of references to expression temporary and makes the
logic more obviously correct.

Also remove other uses of references to temporary that were correct but
unnecessary given that the function returned a value and C++11
return value optimisation should kick in.

Testing:
Ran expr-test to completion with a release build. Before this fix it
reliably crashed for me.

Change-Id: Ic5017518188f5025daa5040ca1943581a0675726
Reviewed-on: http://gerrit.cloudera.org:8080/8015
Reviewed-by: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Tested-by: Impala Public 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/e1ae9884
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/e1ae9884
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/e1ae9884

Branch: refs/heads/master
Commit: e1ae9884168b0c455fa147bb10dc57d37989f9e8
Parents: 2fbdc8e
Author: Tim Armstrong <tarmstrong@cloudera.com>
Authored: Fri Sep 8 15:14:00 2017 -0700
Committer: Impala Public Jenkins <impala-public-jenkins@gerrit.cloudera.org>
Committed: Tue Sep 12 01:53:20 2017 +0000

----------------------------------------------------------------------
 be/src/exprs/udf-builtins.cc | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e1ae9884/be/src/exprs/udf-builtins.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/udf-builtins.cc b/be/src/exprs/udf-builtins.cc
index 01178e3..ebfca35 100644
--- a/be/src/exprs/udf-builtins.cc
+++ b/be/src/exprs/udf-builtins.cc
@@ -193,7 +193,7 @@ TimestampValue TruncWeek(const date& orig_date) {
 
 // Same day of the week as the first day of the year
 TimestampValue TruncWW(const date& orig_date) {
-  const date& first_day_of_year = TruncYear(orig_date).date();
+  date first_day_of_year(orig_date.year(), 1, 1);
   int target_week_day = first_day_of_year.day_of_week();
   date new_date = GoBackToWeekday(orig_date, target_week_day);
   time_duration new_time(0, 0, 0, 0);
@@ -202,8 +202,8 @@ TimestampValue TruncWW(const date& orig_date) {
 
 // Same day of the week as the first day of the month
 TimestampValue TruncW(const date& orig_date) {
-  const date& first_day_of_mon = TruncMonth(orig_date).date();
-  const date& new_date = GoBackToWeekday(orig_date, first_day_of_mon.day_of_week());
+  date first_day_of_mon(orig_date.year(), orig_date.month(), 1);
+  date new_date = GoBackToWeekday(orig_date, first_day_of_mon.day_of_week());
   time_duration new_time(0, 0, 0, 0);
   return TimestampValue(new_date, new_time);
 }
@@ -216,7 +216,7 @@ TimestampValue TruncDay(const date& orig_date) {
 
 // Date of the previous Monday
 TimestampValue TruncDayOfWeek(const date& orig_date) {
-  const date& new_date = GoBackToWeekday(orig_date, 1);
+  date new_date = GoBackToWeekday(orig_date, 1);
   time_duration new_time(0, 0, 0, 0);
   return TimestampValue(new_date, new_time);
 }
@@ -364,7 +364,7 @@ TimestampVal DoTrunc(
 TimestampVal UdfBuiltins::TruncImpl(
     FunctionContext* context, const TimestampVal& tv, const StringVal& unit_str)
{
   if (tv.is_null) return TimestampVal::null();
-  const TimestampValue& ts = TimestampValue::FromTimestampVal(tv);
+  TimestampValue ts = TimestampValue::FromTimestampVal(tv);
 
   // resolve trunc_unit using the prepared state if possible, o.w. parse now
   // TruncPrepare() can only parse trunc_unit if user passes it as a string literal
@@ -413,7 +413,7 @@ void UdfBuiltins::TruncClose(FunctionContext* ctx,
 TimestampVal UdfBuiltins::DateTruncImpl(
     FunctionContext* context, const TimestampVal& tv, const StringVal& unit_str)
{
   if (tv.is_null) return TimestampVal::null();
-  const TimestampValue& ts = TimestampValue::FromTimestampVal(tv);
+  TimestampValue ts = TimestampValue::FromTimestampVal(tv);
 
   // resolve date_trunc_unit using the prepared state if possible, o.w. parse now
   // DateTruncPrepare() can only parse trunc_unit if user passes it as a string literal


Mime
View raw message