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 2018/07/13 06:03:59 UTC
[47/51] [abbrv] impala git commit: IMPALA-7006: Pick parts of recent
Kudu gutil changes
IMPALA-7006: Pick parts of recent Kudu gutil changes
- Include some ASAN macros from gutil (Kudu commit c8724c61)
- Pick parts of KUDU-2427 (Kudu commit b7cf3b2e)
- Rename constants (Kudu commit e719b5ef)
These changes will be subsumed by a proper rebase of GUTIL.
Change-Id: Id2dc8c70425e3ac030427ebeb1ec18a44d14d5cb
Reviewed-on: http://gerrit.cloudera.org:8080/10769
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Lars Volker <lv...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/0459721c
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/0459721c
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/0459721c
Branch: refs/heads/master
Commit: 0459721ccd59deae34548d44c684def4e04b31a6
Parents: 1ac9a3f
Author: Lars Volker <lv...@cloudera.com>
Authored: Tue Jul 3 17:05:37 2018 -0700
Committer: Lars Volker <lv...@cloudera.com>
Committed: Thu Jul 12 21:35:42 2018 +0000
----------------------------------------------------------------------
be/src/gutil/macros.h | 22 +++++++++++++++++
be/src/gutil/port.h | 26 ++++++++++++++++++++
be/src/gutil/strings/substitute.cc | 4 ++--
be/src/gutil/strings/substitute.h | 42 ++++++++++++++++-----------------
be/src/gutil/sysinfo.cc | 26 ++++++++++++++------
be/src/util/error-util.h | 20 ++++++++--------
6 files changed, 100 insertions(+), 40 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/0459721c/be/src/gutil/macros.h
----------------------------------------------------------------------
diff --git a/be/src/gutil/macros.h b/be/src/gutil/macros.h
index 0318008..da7ea13 100644
--- a/be/src/gutil/macros.h
+++ b/be/src/gutil/macros.h
@@ -262,4 +262,26 @@ enum LinkerInitialized { LINKER_INITIALIZED };
#define FALLTHROUGH_INTENDED do { } while (0)
#endif
+// Retry on EINTR for functions like read() that return -1 on error.
+#define RETRY_ON_EINTR(err, expr) do { \
+ static_assert(std::is_signed<decltype(err)>::value, \
+ #err " must be a signed integer"); \
+ (err) = (expr); \
+} while ((err) == -1 && errno == EINTR)
+
+// Same as above but for stream API calls like fread() and fwrite().
+#define STREAM_RETRY_ON_EINTR(nread, stream, expr) do { \
+ static_assert(std::is_unsigned<decltype(nread)>::value == true, \
+ #nread " must be an unsigned integer"); \
+ (nread) = (expr); \
+} while ((nread) == 0 && ferror(stream) == EINTR)
+
+// Same as above but for functions that return pointer types (like
+// fopen() and freopen()).
+#define POINTER_RETRY_ON_EINTR(ptr, expr) do { \
+ static_assert(std::is_pointer<decltype(ptr)>::value == true, \
+ #ptr " must be a pointer"); \
+ (ptr) = (expr); \
+} while ((ptr) == nullptr && errno == EINTR)
+
#endif // BASE_MACROS_H_
http://git-wip-us.apache.org/repos/asf/impala/blob/0459721c/be/src/gutil/port.h
----------------------------------------------------------------------
diff --git a/be/src/gutil/port.h b/be/src/gutil/port.h
index e204de7..e258dba 100644
--- a/be/src/gutil/port.h
+++ b/be/src/gutil/port.h
@@ -423,6 +423,32 @@ inline void* memrchr(const void* bytes, int find_char, size_t len) {
#define ATTRIBUTE_NO_SANITIZE_THREAD
#endif
+// Tell UBSAN to ignore a given function completely. There is no
+// __has_feature(undefined_sanitizer) or equivalent, so ASAN support is used as
+// a proxy.
+#if defined(__has_feature)
+# if __has_feature(address_sanitizer)
+# define ATTRIBUTE_NO_SANITIZE_UNDEFINED \
+ __attribute__((no_sanitize("undefined")))
+# endif
+#endif
+#ifndef ATTRIBUTE_NO_SANITIZE_UNDEFINED
+#define ATTRIBUTE_NO_SANITIZE_UNDEFINED
+#endif
+
+// Tell UBSAN to ignore integer overflows in a given function. There is no
+// __has_feature(undefined_sanitizer) or equivalent, so ASAN support is used as
+// a proxy.
+#if defined(__has_feature)
+# if __has_feature(address_sanitizer)
+# define ATTRIBUTE_NO_SANITIZE_INTEGER \
+ __attribute__((no_sanitize("integer")))
+# endif
+#endif
+#ifndef ATTRIBUTE_NO_SANITIZE_INTEGER
+#define ATTRIBUTE_NO_SANITIZE_INTEGER
+#endif
+
#ifndef HAVE_ATTRIBUTE_SECTION // may have been pre-set to 0, e.g. for Darwin
#define HAVE_ATTRIBUTE_SECTION 1
#endif
http://git-wip-us.apache.org/repos/asf/impala/blob/0459721c/be/src/gutil/strings/substitute.cc
----------------------------------------------------------------------
diff --git a/be/src/gutil/strings/substitute.cc b/be/src/gutil/strings/substitute.cc
index 76ca151..5c69f04 100644
--- a/be/src/gutil/strings/substitute.cc
+++ b/be/src/gutil/strings/substitute.cc
@@ -13,13 +13,13 @@ namespace strings {
using internal::SubstituteArg;
-const SubstituteArg SubstituteArg::NoArg;
+const SubstituteArg SubstituteArg::kNoArg;
// Returns the number of args in arg_array which were passed explicitly
// to Substitute().
static int CountSubstituteArgs(const SubstituteArg* const* args_array) {
int count = 0;
- while (args_array[count] != &SubstituteArg::NoArg) {
+ while (args_array[count] != &SubstituteArg::kNoArg) {
++count;
}
return count;
http://git-wip-us.apache.org/repos/asf/impala/blob/0459721c/be/src/gutil/strings/substitute.h
----------------------------------------------------------------------
diff --git a/be/src/gutil/strings/substitute.h b/be/src/gutil/strings/substitute.h
index 84d362a..902a073 100644
--- a/be/src/gutil/strings/substitute.h
+++ b/be/src/gutil/strings/substitute.h
@@ -131,7 +131,7 @@ class SubstituteArg {
inline int size() const { return size_; }
// Indicates that no argument was given.
- static const SubstituteArg NoArg;
+ static const SubstituteArg kNoArg;
private:
inline SubstituteArg() : text_(NULL), size_(-1) {}
@@ -158,29 +158,29 @@ char* SubstituteToBuffer(StringPiece format,
void SubstituteAndAppend(
string* output, StringPiece format,
- const internal::SubstituteArg& arg0 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg1 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg2 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg3 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg4 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg5 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg6 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg7 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg8 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg9 = internal::SubstituteArg::NoArg);
+ const internal::SubstituteArg& arg0 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg1 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg2 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg3 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg4 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg5 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg6 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg7 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg8 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg9 = internal::SubstituteArg::kNoArg);
inline string Substitute(
StringPiece format,
- const internal::SubstituteArg& arg0 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg1 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg2 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg3 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg4 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg5 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg6 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg7 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg8 = internal::SubstituteArg::NoArg,
- const internal::SubstituteArg& arg9 = internal::SubstituteArg::NoArg) {
+ const internal::SubstituteArg& arg0 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg1 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg2 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg3 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg4 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg5 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg6 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg7 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg8 = internal::SubstituteArg::kNoArg,
+ const internal::SubstituteArg& arg9 = internal::SubstituteArg::kNoArg) {
string result;
SubstituteAndAppend(&result, format, arg0, arg1, arg2, arg3, arg4,
arg5, arg6, arg7, arg8, arg9);
http://git-wip-us.apache.org/repos/asf/impala/blob/0459721c/be/src/gutil/sysinfo.cc
----------------------------------------------------------------------
diff --git a/be/src/gutil/sysinfo.cc b/be/src/gutil/sysinfo.cc
index a2b6077..dddfa74 100644
--- a/be/src/gutil/sysinfo.cc
+++ b/be/src/gutil/sysinfo.cc
@@ -42,8 +42,8 @@
#include <unistd.h> // for read()
#endif
#if defined __MACH__ // Mac OS X, almost certainly
-#include <sys/types.h>
#include <sys/sysctl.h> // how we figure out numcpu's on OS X
+#include <sys/types.h>
#elif defined __FreeBSD__
#include <sys/sysctl.h>
#elif defined __sun__ // Solaris
@@ -114,18 +114,25 @@ static int64 EstimateCyclesPerSecond(const int estimate_time_ms) {
// issue a FATAL error.
static bool SlurpSmallTextFile(const char* file, char* buf, int buflen) {
bool ret = false;
- int fd = open(file, O_RDONLY);
+ int fd;
+ RETRY_ON_EINTR(fd, open(file, O_RDONLY));
if (fd == -1) return ret;
memset(buf, '\0', buflen);
- int n = read(fd, buf, buflen - 1);
+ int n;
+ RETRY_ON_EINTR(n, read(fd, buf, buflen - 1));
CHECK_NE(n, buflen - 1) << "buffer of len " << buflen << " not large enough to store "
<< "contents of " << file;
if (n > 0) {
ret = true;
}
- close(fd);
+ int close_ret;
+ RETRY_ON_EINTR(close_ret, close(fd));
+ if (PREDICT_FALSE(close_ret != 0)) {
+ PLOG(WARNING) << "Failed to close fd " << fd;
+ }
+
return ret;
}
@@ -220,7 +227,8 @@ static void InitializeSystemInfo() {
// Read /proc/cpuinfo for other values, and if there is no cpuinfo_max_freq.
const char* pname = "/proc/cpuinfo";
- int fd = open(pname, O_RDONLY);
+ int fd;
+ RETRY_ON_EINTR(fd, open(pname, O_RDONLY));
if (fd == -1) {
PLOG(FATAL) << "Unable to read CPU info from /proc. procfs must be mounted.";
}
@@ -243,7 +251,7 @@ static void InitializeSystemInfo() {
const int linelen = strlen(line);
const int bytes_to_read = sizeof(line)-1 - linelen;
CHECK(bytes_to_read > 0); // because the memmove recovered >=1 bytes
- chars_read = read(fd, line + linelen, bytes_to_read);
+ RETRY_ON_EINTR(chars_read, read(fd, line + linelen, bytes_to_read));
line[linelen + chars_read] = '\0';
newline = strchr(line, '\n');
}
@@ -288,7 +296,11 @@ static void InitializeSystemInfo() {
num_cpus++; // count up every time we see an "processor :" entry
}
} while (chars_read > 0);
- close(fd);
+ int ret;
+ RETRY_ON_EINTR(ret, close(fd));
+ if (PREDICT_FALSE(ret != 0)) {
+ PLOG(WARNING) << "Failed to close fd " << fd;
+ }
if (!saw_mhz) {
if (saw_bogo) {
http://git-wip-us.apache.org/repos/asf/impala/blob/0459721c/be/src/util/error-util.h
----------------------------------------------------------------------
diff --git a/be/src/util/error-util.h b/be/src/util/error-util.h
index 44dcb26..3975548 100644
--- a/be/src/util/error-util.h
+++ b/be/src/util/error-util.h
@@ -93,16 +93,16 @@ class ErrorMsg {
/// the cost of this method is proportional to the number of entries in the global error
/// message list.
/// WARNING: DO NOT CALL THIS METHOD IN A NON STATIC CONTEXT
- static ErrorMsg Init(TErrorCode::type error, const ArgType& arg0 = ArgType::NoArg,
- const ArgType& arg1 = ArgType::NoArg,
- const ArgType& arg2 = ArgType::NoArg,
- const ArgType& arg3 = ArgType::NoArg,
- const ArgType& arg4 = ArgType::NoArg,
- const ArgType& arg5 = ArgType::NoArg,
- const ArgType& arg6 = ArgType::NoArg,
- const ArgType& arg7 = ArgType::NoArg,
- const ArgType& arg8 = ArgType::NoArg,
- const ArgType& arg9 = ArgType::NoArg);
+ static ErrorMsg Init(TErrorCode::type error, const ArgType& arg0 = ArgType::kNoArg,
+ const ArgType& arg1 = ArgType::kNoArg,
+ const ArgType& arg2 = ArgType::kNoArg,
+ const ArgType& arg3 = ArgType::kNoArg,
+ const ArgType& arg4 = ArgType::kNoArg,
+ const ArgType& arg5 = ArgType::kNoArg,
+ const ArgType& arg6 = ArgType::kNoArg,
+ const ArgType& arg7 = ArgType::kNoArg,
+ const ArgType& arg8 = ArgType::kNoArg,
+ const ArgType& arg9 = ArgType::kNoArg);
TErrorCode::type error() const { return error_; }