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
commit 227da84c3757eb857008e7b82aad622ed959eb84 Author: Joe McDonnell <joemcdonn...@cloudera.com> AuthorDate: Wed May 27 16:21:36 2020 -0700 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 memset / memcpy with local variables to avoid the unaligned stores. Testing: - Ran exhaustive tests with a release binary built by GCC 7.5.0 - Ran exhaustive tests Change-Id: I67320790789d5b57aeaf2dff0eae7352a1cbf81e Reviewed-on: http://gerrit.cloudera.org:8080/15993 Reviewed-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com> --- be/src/exprs/slot-ref.cc | 5 ++++- be/src/util/decimal-util.h | 3 ++- be/src/util/dict-encoding.h | 5 +++-- 3 files changed, 9 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/util/decimal-util.h b/be/src/util/decimal-util.h index f505ecc..4ddfe23 100644 --- a/be/src/util/decimal-util.h +++ b/be/src/util/decimal-util.h @@ -128,7 +128,8 @@ class DecimalUtil { const uint8_t* buffer, int fixed_len_size, T* v) { DCHECK_GT(fixed_len_size, 0); DCHECK_LE(fixed_len_size, sizeof(T)); - *v = 0; + // Avoid an unaligned store by using memset + memset(v, 0, sizeof(T)); // We need to sign extend val. For example, if the original value was // -1, the original bytes were -1,-1,-1,-1. If we only wrote out 1 byte, after // the encode step above, val would contain (-1, 0, 0, 0). We need to sign 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.