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 <[email protected]> Reviewed-by: Dan Hecht <[email protected]> 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 <[email protected]> Authored: Fri Sep 8 15:14:00 2017 -0700 Committer: Impala Public Jenkins <[email protected]> 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
