You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by tm...@apache.org on 2018/08/14 01:31:40 UTC

[2/6] impala git commit: IMPALA-5031: Remove some undefined behavior of NULL pointers

IMPALA-5031: Remove some undefined behavior of NULL pointers

This was found in ExprTest.LiteralExprs. The rules vioalted are:

1. "reference binding to null pointer of type 'long'". This violates
the standard's [dcl.ref]:

    a null reference cannot exist in a well-defined program, because
    the only way to create such a reference would be to bind it to the
    "object" obtained by indirection through a null pointer, which
    causes undefined behavior.

The interesting part of the backtrace is:

    #1 RuntimeProfile::TimeSeriesCounter::ToThrift(TTimeSeriesCounter*) util/runtime-profile.cc:1117:11
    #2 RuntimeProfile::ToThrift(std::vector<TRuntimeProfileNode>*) const util/runtime-profile.cc:905:21
    #3 RuntimeProfile::ToThrift(TRuntimeProfileTree*) const util/runtime-profile.cc:847:3
    #4 QueryState::ReportExecStatusAux(bool, Status const&, FragmentInstanceState*, bool) runtime/query-state.cc:281:21
    #5 QueryState::ReportExecStatus(bool, Status const&, FragmentInstanceState*) runtime/query-state.cc:250:3
    #6 FragmentInstanceState::SendReport(bool, Status const&) runtime/fragment-instance-state.cc:406:17
    #7 FragmentInstanceState::Finalize(Status const&) runtime/fragment-instance-state.cc:496:3

2. The use of a null pointer when calling memcpy. According to "7.1.4
Use of library functions" in the C99 standard (which is included in
C++14 in section [intro.refs]:

    If an argument to a function has an invalid value (such as a value
    outside the domain of the function, or a pointer outside the
    address space of the program, or a null pointer, or a pointer to
    non-modifiable storage when the corresponding parameter is not
    const-qualified) or a type (after promotion) not expected by a
    function with variable number of arguments, the behavior is
    undefined.

The interesting part of the backtrace is the same as above.

Change-Id: I3c8a6624918389396789a83b32dbf068b9327f76
Reviewed-on: http://gerrit.cloudera.org:8080/11195
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/13c6116f
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/13c6116f
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/13c6116f

Branch: refs/heads/master
Commit: 13c6116f76bf2fa568710038d3897a1e248cf243
Parents: 0f6d051
Author: Jim Apple <jb...@apache.org>
Authored: Sun Aug 12 09:39:18 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Mon Aug 13 21:02:13 2018 +0000

----------------------------------------------------------------------
 be/src/util/runtime-profile.cc | 3 ++-
 be/src/util/ubsan.h            | 7 +++++++
 2 files changed, 9 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/13c6116f/be/src/util/runtime-profile.cc
----------------------------------------------------------------------
diff --git a/be/src/util/runtime-profile.cc b/be/src/util/runtime-profile.cc
index b43bbbe..51ab2fc 100644
--- a/be/src/util/runtime-profile.cc
+++ b/be/src/util/runtime-profile.cc
@@ -36,6 +36,7 @@
 #include "util/pretty-printer.h"
 #include "util/redactor.h"
 #include "util/scope-exit-trigger.h"
+#include "util/ubsan.h"
 
 #include "common/names.h"
 
@@ -1114,7 +1115,7 @@ void RuntimeProfile::TimeSeriesCounter::ToThrift(TTimeSeriesCounter* counter) {
   SpinLock* lock;
   const int64_t* samples = samples_.GetSamples(&num, &period, &lock);
   counter->values.resize(num);
-  memcpy(&counter->values[0], samples, num * sizeof(int64_t));
+  Ubsan::MemCpy(counter->values.data(), samples, num * sizeof(int64_t));
   lock->unlock();
   counter->period_ms = period;
 }

http://git-wip-us.apache.org/repos/asf/impala/blob/13c6116f/be/src/util/ubsan.h
----------------------------------------------------------------------
diff --git a/be/src/util/ubsan.h b/be/src/util/ubsan.h
index 78f6bc1..da8b843 100644
--- a/be/src/util/ubsan.h
+++ b/be/src/util/ubsan.h
@@ -32,6 +32,13 @@ class Ubsan {
     }
     return std::memset(s, c, n);
   }
+  static void* MemCpy(void* dest, const void* src, size_t n) {
+    if (dest == nullptr || src == nullptr) {
+      DCHECK_EQ(n, 0);
+      return dest;
+    }
+    return std::memcpy(dest, src, n);
+  }
 };
 
 #endif // UTIL_UBSAN_H_