You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2019/06/03 16:35:53 UTC
[impala] 01/06: IMPALA-5031: signed overflow is undefined behavior
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit e10d56762d26a6364e915d8693f936944f43d3ab
Author: Jim Apple <jb...@apache.org>
AuthorDate: Fri May 24 17:50:56 2019 -0700
IMPALA-5031: signed overflow is undefined behavior
This undefined behavior was caught with UBSAN in the end-to-end
tests. The interesting part of the backtrace is:
gutil/atomicops-internals-x86.h:283:15: runtime error: signed
integer overflow: -9223370395229620599 + -9223371946660462582
cannot be represented in type 'long'
#0 base::subtle::Barrier_AtomicIncrement(long volatile*, long)
gutil/atomicops-internals-x86.h:283:15
#1 internal::AtomicInt<long>::Add(long) common/atomic.h:93:12
#2 RuntimeProfile::Counter::Add(long) util/runtime-profile.h:93
#3 HdfsOrcScanner::AssembleRows(RowBatch*)
exec/hdfs-orc-scanner.cc:636:50
#4 HdfsOrcScanner::GetNextInternal(RowBatch*)
exec/hdfs-orc-scanner.cc:507:19
#5 HdfsOrcScanner::ProcessSplit() exec/hdfs-orc-scanner.cc:426:21
#6 HdfsScanNode::ProcessSplit(vector<FilterContext> const&,
MemPool*, io::ScanRange*, long*) exec/hdfs-scan-node.cc:514:21
#7 HdfsScanNode::ScannerThread(bool, long)
exec/hdfs-scan-node.cc:415:7
#8 HdfsScanNode::ThreadTokenAvailableCb(ThreadResourcePool*)::
$_0::operator()() const exec/hdfs-scan-node.cc:337:13
Change-Id: Ic638ff4959eaaffc79caa3453dbccaaabcbe95c9
Reviewed-on: http://gerrit.cloudera.org:8080/13433
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/gutil/atomicops-internals-x86.h | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/be/src/gutil/atomicops-internals-x86.h b/be/src/gutil/atomicops-internals-x86.h
index 90ae25a..4d26bff 100644
--- a/be/src/gutil/atomicops-internals-x86.h
+++ b/be/src/gutil/atomicops-internals-x86.h
@@ -31,6 +31,8 @@
#include <common/logging.h>
+#include "util/arithmetic-util.h"
+
#define BASE_HAS_ATOMIC64 1 // Use only in tests and base/atomic*
@@ -116,7 +118,7 @@ inline Atomic32 NoBarrier_AtomicIncrement(volatile Atomic32* ptr,
: "+r" (temp), "+m" (*ptr)
: : "memory");
// temp now holds the old value of *ptr
- return temp + increment;
+ return impala::ArithmeticUtil::AsUnsigned<std::plus>(temp, increment);
}
inline Atomic32 Barrier_AtomicIncrement(volatile Atomic32* ptr,
@@ -127,7 +129,7 @@ inline Atomic32 Barrier_AtomicIncrement(volatile Atomic32* ptr,
: "+r" (temp), "+m" (*ptr)
: : "memory");
// temp now holds the old value of *ptr
- return temp + increment;
+ return impala::ArithmeticUtil::AsUnsigned<std::plus>(temp, increment);
}
// On x86, the NoBarrier_CompareAndSwap() uses a locked instruction and so also
@@ -269,7 +271,7 @@ inline Atomic64 NoBarrier_AtomicIncrement(volatile Atomic64* ptr,
: "+r" (temp), "+m" (*ptr)
: : "memory");
// temp now contains the previous value of *ptr
- return temp + increment;
+ return impala::ArithmeticUtil::AsUnsigned<std::plus>(temp, increment);
}
inline Atomic64 Barrier_AtomicIncrement(volatile Atomic64* ptr,
@@ -280,7 +282,7 @@ inline Atomic64 Barrier_AtomicIncrement(volatile Atomic64* ptr,
: "+r" (temp), "+m" (*ptr)
: : "memory");
// temp now contains the previous value of *ptr
- return temp + increment;
+ return impala::ArithmeticUtil::AsUnsigned<std::plus>(temp, increment);
}
inline void NoBarrier_Store(volatile Atomic64* ptr, Atomic64 value) {
@@ -408,10 +410,10 @@ inline Atomic64 NoBarrier_AtomicIncrement(volatile Atomic64* ptr,
do {
old_val = *ptr;
- new_val = old_val + increment;
+ new_val = return impala::ArithmeticUtil::AsUnsigned<std::plus>(old_val, increment);
} while (__sync_val_compare_and_swap(ptr, old_val, new_val) != old_val);
- return old_val + increment;
+ return impala::ArithmeticUtil::AsUnsigned<std::plus>(old_val, increment);
}
inline Atomic64 Barrier_AtomicIncrement(volatile Atomic64* ptr,