[2/2] impala git commit: IMPALA-5031: Fix undefined behavior: memset NULL

[jb...@apache.org]

IMPALA-5031: Fix undefined behavior: memset NULL

memset has undefined behavior when its first argument is NULL. The
instance fixed here was found by Clang's undefined behavior
sanitizer.

It was found in the end-to-end tests. The interesting part of the
stack trace is:

/exec/data-source-scan-node.cc:152:10: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:62:79: note: nonnull attribute specified here
    #0 0x482fd8e in DataSourceScanNode::GetNextInputBatch() /exec/data-source-scan-node.cc:152:3
    #1 0x482fb40 in DataSourceScanNode::Open(RuntimeState*) /exec/data-source-scan-node.cc:124:10
    #2 0x47ef854 in AggregationNode::Open(RuntimeState*) /exec/aggregation-node.cc:71:49
    #3 0x23506a4 in FragmentInstanceState::Open() /runtime/fragment-instance-state.cc:266:53
    #4 0x234b6a8 in FragmentInstanceState::Exec() /runtime/fragment-instance-state.cc:81:12
    #5 0x236ee52 in QueryState::ExecFInstance(FragmentInstanceState*) /runtime/query-state.cc:401:24
    #6 0x237093e in QueryState::StartFInstances()::$_0::operator()() const /runtime/query-state.cc:341:44

Change-Id: I18fa02dc887a42a94c6f81e4923d17568f2184f2
Reviewed-on: http://gerrit.cloudera.org:8080/10948
Reviewed-by: Jim Apple <jb...@apache.org>
Tested-by: Impala Public Jenkins <im...@cloudera.com>


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

Branch: refs/heads/master
Commit: f7efba23607974a9c5d3da02999db531a92628c2
Parents: cb0f8a0
Author: Jim Apple <jb...@apache.org>
Authored: Sun Jul 15 18:32:52 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Mon Jul 23 22:37:16 2018 +0000

----------------------------------------------------------------------
 be/src/exec/data-source-scan-node.cc |  3 ++-
 be/src/util/ubsan.h                  | 37 +++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/f7efba23/be/src/exec/data-source-scan-node.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/data-source-scan-node.cc b/be/src/exec/data-source-scan-node.cc
index 88cf11b..fa11d05 100644
--- a/be/src/exec/data-source-scan-node.cc
+++ b/be/src/exec/data-source-scan-node.cc
@@ -33,6 +33,7 @@
 #include "runtime/tuple-row.h"
 #include "util/jni-util.h"
 #include "util/periodic-counter-updater.h"
+#include "util/ubsan.h"
 #include "util/runtime-profile-counters.h"
 
 #include "common/names.h"
@@ -149,7 +150,7 @@ Status DataSourceScanNode::GetNextInputBatch() {
   input_batch_.reset(new TGetNextResult());
   next_row_idx_ = 0;
   // Reset all the indexes into the column value arrays to 0
-  memset(cols_next_val_idx_.data(), 0, sizeof(int) * cols_next_val_idx_.size());
+  Ubsan::MemSet(cols_next_val_idx_.data(), 0, sizeof(int) * cols_next_val_idx_.size());
   TGetNextParams params;
   params.__set_scan_handle(scan_handle_);
   RETURN_IF_ERROR(data_source_executor_->GetNext(params, input_batch_.get()));

http://git-wip-us.apache.org/repos/asf/impala/blob/f7efba23/be/src/util/ubsan.h
----------------------------------------------------------------------
diff --git a/be/src/util/ubsan.h b/be/src/util/ubsan.h
new file mode 100644
index 0000000..78f6bc1
--- /dev/null
+++ b/be/src/util/ubsan.h
@@ -0,0 +1,37 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#ifndef UTIL_UBSAN_H_
+#define UTIL_UBSAN_H_
+
+// Utilities mimicking parts of the standard prone to accidentally using in a way causeing
+// undefined behavior.
+
+#include <cstring>
+
+class Ubsan {
+ public:
+  static void* MemSet(void* s, int c, size_t n) {
+    if (s == nullptr) {
+      DCHECK_EQ(n, 0);
+      return s;
+    }
+    return std::memset(s, c, n);
+  }
+};
+
+#endif // UTIL_UBSAN_H_