This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new b188340 Reapply IMPALA-9781: Fix GCC 7 unaligned 128-bit loads /
stores
b188340 is described below
commit b1883405cd59988b92df2799656666ff2f733c0e
Author: Joe McDonnell <joemcdonn...@cloudera.com>
AuthorDate: Wed May 27 16:21:36 2020 -0700
Reapply IMPALA-9781: Fix GCC 7 unaligned 128-bit loads / stores
When running a release binary built with GCC 7.5.0, it crashes
with an unaligned memory error in multiple pieces of code.
In these locations, we are doing stores to 128-bit values, but we
cannot guarantee alignment. GCC 7 must be optimizing the code to
use instructions that require a higher level of alignment than
we can provide.
This switches the code locations to use memcpy to avoid the
unaligned stores (with local variables as necessary).
Testing:
- Ran exhaustive tests with a release binary built by GCC 7.5.0
- Ran UBSAN core tests
- Add unaligned test case in decimal-test
Change-Id: I7edd8beeb15e4fbb69126a9f97a1476a4b8f12a9
Reviewed-on: http://gerrit.cloudera.org:8080/16009
Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Reviewed-by: Tim Armstrong <tarmstr...@cloudera.com>
---
be/src/exprs/slot-ref.cc | 5 ++++-
be/src/runtime/decimal-test.cc | 4 ++++
be/src/runtime/decimal-value.h | 3 ++-
be/src/util/dict-encoding.h | 5 +++--
4 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/be/src/exprs/slot-ref.cc b/be/src/exprs/slot-ref.cc
index 634a989..661c7ef 100644
--- a/be/src/exprs/slot-ref.cc
+++ b/be/src/exprs/slot-ref.cc
@@ -422,7 +422,10 @@ DecimalVal SlotRef::GetDecimalValInterpreted(
case 8:
return DecimalVal(*reinterpret_cast<int64_t*>(t->GetSlot(slot_offset_)));
case 16:
- return
DecimalVal(*reinterpret_cast<int128_t*>(t->GetSlot(slot_offset_)));
+ // Avoid an unaligned load by using memcpy
+ __int128_t val;
+ memcpy(&val, t->GetSlot(slot_offset_), sizeof(val));
+ return DecimalVal(val);
default:
DCHECK(false);
return DecimalVal::null();
diff --git a/be/src/runtime/decimal-test.cc b/be/src/runtime/decimal-test.cc
index 9a2f7a8..c5aed53 100644
--- a/be/src/runtime/decimal-test.cc
+++ b/be/src/runtime/decimal-test.cc
@@ -726,6 +726,10 @@ TEST(DecimalTest, UnalignedValues) {
stringstream ss;
RawValue::PrintValue(unaligned, ColumnType::CreateDecimalType(28, 2), 0,
&ss);
EXPECT_EQ("123.45", ss.str());
+ // Regression test for IMPALA-9781: Verify that operator=() works
+ *unaligned = 0;
+ __int128_t val = unaligned->value();
+ EXPECT_EQ(val, 0);
free(unaligned_mem);
}
diff --git a/be/src/runtime/decimal-value.h b/be/src/runtime/decimal-value.h
index 761d474..e329476 100644
--- a/be/src/runtime/decimal-value.h
+++ b/be/src/runtime/decimal-value.h
@@ -49,7 +49,8 @@ class DecimalValue {
DecimalValue(const T& s) : value_(s) { }
DecimalValue& operator=(const T& s) {
- value_ = s;
+ // 'value_' may be unaligned. Use memcpy to avoid an unaligned store.
+ memcpy(&value_, &s, sizeof(T));
return *this;
}
diff --git a/be/src/util/dict-encoding.h b/be/src/util/dict-encoding.h
index f440332..e6e01bc 100644
--- a/be/src/util/dict-encoding.h
+++ b/be/src/util/dict-encoding.h
@@ -346,10 +346,11 @@ class DictDecoder : public DictDecoderBase {
virtual int num_entries() const { return dict_.size(); }
virtual void GetValue(int index, void* buffer) {
- T* val_ptr = reinterpret_cast<T*>(buffer);
DCHECK_GE(index, 0);
DCHECK_LT(index, dict_.size());
- *val_ptr = dict_[index];
+ // Avoid an unaligned store by using memcpy
+ T val = dict_[index];
+ memcpy(buffer, reinterpret_cast<const void*>(&val), sizeof(T));
}
/// Returns the next value. Returns false if the data is invalid.
