[5/5] impala git commit: IMPALA-5031: Fix undefined behavior: ptr overflow

[ta...@apache.org]

IMPALA-5031: Fix undefined behavior: ptr overflow

In expr.add, the standard says:

    When an expression that has integral type is added to or
    subtracted from a pointer, the result has the type of the pointer
    operand. ... If both the pointer operand and the result point to
    elements of the same array object, or one past the last element of
    the array object, the evaluation shall not produce an overflow;
    otherwise, the behavior is undefined.

In the end-to-end tests this is triggered, and the interesting part of
the backtrace is:

include/c++/4.9.2/bits/stl_iterator.h:782:45: runtime error: pointer index expression with base 0x000000000000 overflowed to 0xffffffffffffffe8
    #0 __normal_iterator<MemPool::ChunkInfo*, vector<MemPool::ChunkInfo>>::operator+(long) const stl_iterator.h:782:45
    #1 MemPool::AcquireData(MemPool*, bool) runtime/mem-pool.cc:190:62
    #2 RowBatch::TransferResourceOwnership(RowBatch*) runtime/row-batch.cc:444:26
    #3 RowBatch::AcquireState(RowBatch*) runtime/row-batch.cc:505:8
    #4 HdfsScanNode::GetNextInternal(RuntimeState*, RowBatch*, bool*) exec/hdfs-scan-node.cc:105:16
    #5 HdfsScanNode::GetNext(RuntimeState*, RowBatch*, bool*) exec/hdfs-scan-node.cc:81:19
    #6 StreamingAggregationNode::GetRowsStreaming(RuntimeState*, RowBatch*) exec/streaming-aggregation-node.cc:116:51
    #7 StreamingAggregationNode::GetNext(RuntimeState*, RowBatch*, bool*) exec/streaming-aggregation-node.cc:92:41

Change-Id: I3d28a80763adb62572b3dd81ea732d18d957d248
Reviewed-on: http://gerrit.cloudera.org:8080/11118
Reviewed-by: Jim Apple <jb...@apache.org>
Tested-by: Jim Apple <jb...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/b8222003
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/b8222003
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/b8222003

Branch: refs/heads/master
Commit: b8222003f9a0a39ea9b22eefe24f7b70102a5a8b
Parents: 75a11d4
Author: Jim Apple <jb...@apache.org>
Authored: Fri Aug 3 15:31:39 2018 -0700
Committer: Jim Apple <jb...@apache.org>
Committed: Tue Aug 7 16:14:36 2018 +0000

----------------------------------------------------------------------
 be/src/runtime/mem-pool.cc | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/b8222003/be/src/runtime/mem-pool.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/mem-pool.cc b/be/src/runtime/mem-pool.cc
index 8b6a4bf..a489630 100644
--- a/be/src/runtime/mem-pool.cc
+++ b/be/src/runtime/mem-pool.cc
@@ -186,8 +186,10 @@ void MemPool::AcquireData(MemPool* src, bool keep_current) {
 
   src->mem_tracker_->TransferTo(mem_tracker_, total_transfered_bytes);
 
-  // insert new chunks after current_chunk_idx_
-  vector<ChunkInfo>::iterator insert_chunk = chunks_.begin() + current_chunk_idx_ + 1;
+  // insert new chunks after current_chunk_idx_. We must calculate current_chunk_idx_ + 1
+  // before finding the offset from chunks_.begin() because current_chunk_idx_ can be -1
+  // and adding a negative number to chunks_.begin() is undefined behavior in C++.
+  vector<ChunkInfo>::iterator insert_chunk = chunks_.begin() + (current_chunk_idx_ + 1);
   chunks_.insert(insert_chunk, src->chunks_.begin(), end_chunk);
   src->chunks_.erase(src->chunks_.begin(), end_chunk);
   current_chunk_idx_ += num_acquired_chunks;