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 <impala-public-jenk...@cloudera.com> Reviewed-by: Lars Volker <l...@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 <l...@cloudera.com> Authored: Tue Jul 3 17:05:37 2018 -0700 Committer: Lars Volker <l...@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_; }