Use HAS_FEATURE() macro instead of __has_feature() Clang ext... [crashpad/crashpad : master]

1,000 views

Skip to first unread message

Robert Sesek (Gerrit)

unread,

Nov 22, 2016, 12:27:47 PM11/22/16

to Mark Mentovai, Robert Sesek, crashp...@chromium.org

Robert Sesek posted comments on this change.

Patch Set 3: Code-Review+1

To view, visit this change. To unsubscribe, visit settings.

Mark Mentovai (Gerrit)

unread,

Nov 22, 2016, 2:28:08 PM11/22/16

to Robert Sesek, crashp...@chromium.org

Mark Mentovai merged this change.

View Change

Approvals:
  Robert Sesek: Looks good to me

Use ADDRESS_SANITIZER instead of __has_feature(address_sanitizer)

__has_feature() is a Clang-ism not implemented by GCC.
base/compiler_specific.h provides a HAS_FEATURE() macro that always
returns 0 when __has_feature() is not implemented. Use this macro for
compatibility with GCC and other compilers that do not implement this
Clang extension.

http://clang.llvm.org/docs/LanguageExtensions.html#has-feature-and-has-extension

For GCC’s Address Sanitizer implementation, test the
__SANITIZE_ADDRESS__ macro that it provides as an alternative to
__has_feature(address_sanitizer).

Note that in Chrome builds, ADDRESS_SANITIZER is pushed in by the build
system. The definition of ADDRESS_SANITIZER provides another way for
that macro to be set. It’s supplementary, not exclusive.

https://chromium.googlesource.com/chromium/src/+/cb33b243721b0e7dbdfa8c4301702e319360e88d/build/config/BUILD.gn#118

BUG=crashpad:30

Change-Id: I5c3145d29bbc966925369c03a37b1ecb5622a004
Reviewed-on: https://chromium-review.googlesource.com/413109
Reviewed-by: Robert Sesek <rse...@chromium.org>
---
M client/capture_context_mac_test.cc
M client/crashpad_info.cc
A util/misc/address_sanitizer.h
M util/util.gyp
4 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/client/capture_context_mac_test.cc b/client/capture_context_mac_test.cc
index 436ac5a..1904b2b 100644
--- a/client/capture_context_mac_test.cc
+++ b/client/capture_context_mac_test.cc
@@ -21,6 +21,7 @@
 
 #include "build/build_config.h"
 #include "gtest/gtest.h"
+#include "util/misc/address_sanitizer.h"
 #include "util/misc/implicit_cast.h"
 
 namespace crashpad {
@@ -103,13 +104,14 @@
   // captured program counter should be slightly greater than or equal to the
   // reference program counter.
   uintptr_t pc = ProgramCounterFromContext(context_1);
-#if !__has_feature(address_sanitizer)
+
+#if !defined(ADDRESS_SANITIZER)
   // AddressSanitizer can cause enough code bloat that the “nearby” check would
   // likely fail.
   const uintptr_t kReferencePC =
       reinterpret_cast<uintptr_t>(TestCaptureContext);
   EXPECT_LT(pc - kReferencePC, 64u);
-#endif
+#endif  // !defined(ADDRESS_SANITIZER)
 
   // Declare sp and context_2 here because all local variables need to be
   // declared before computing the stack pointer reference value, so that the
diff --git a/client/crashpad_info.cc b/client/crashpad_info.cc
index e8a6a9e..e517f7b 100644
--- a/client/crashpad_info.cc
+++ b/client/crashpad_info.cc
@@ -14,6 +14,7 @@
 
 #include "client/crashpad_info.h"
 
+#include "util/misc/address_sanitizer.h"
 #include "util/stdlib/cxx.h"
 
 #if defined(OS_MACOSX)
@@ -72,14 +73,14 @@
 #error Port
 #endif  // !defined(OS_MACOSX) && !defined(OS_LINUX) && !defined(OS_ANDROID)
 
-#if __has_feature(address_sanitizer)
+#if defined(ADDRESS_SANITIZER)
     // AddressSanitizer would add a trailing red zone of at least 32 bytes,
     // which would be reflected in the size of the custom section. This confuses
     // MachOImageReader::GetCrashpadInfo(), which finds that the section’s size
     // disagrees with the structure’s size_ field. By specifying an alignment
     // greater than the red zone size, the red zone will be suppressed.
     aligned(64),
-#endif  // __has_feature(address_sanitizer)
+#endif  // defined(ADDRESS_SANITIZER)
 
     // The “used” attribute prevents the structure from being dead-stripped.
     used,
diff --git a/util/misc/address_sanitizer.h b/util/misc/address_sanitizer.h
new file mode 100644
index 0000000..75c7239
--- /dev/null
+++ b/util/misc/address_sanitizer.h
@@ -0,0 +1,28 @@
+// Copyright 2016 The Crashpad Authors. All rights reserved.
+//
+// Licensed 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 CRASHPAD_UTIL_MISC_ADDRESS_SANITIZER_H_
+#define CRASHPAD_UTIL_MISC_ADDRESS_SANITIZER_H_
+
+#include "base/compiler_specific.h"
+#include "build/build_config.h"
+
+#if !defined(ADDRESS_SANITIZER)
+#if HAS_FEATURE(address_sanitizer) || \
+    (defined(COMPILER_GCC) && defined(__SANITIZE_ADDRESS__))
+#define ADDRESS_SANITIZER 1
+#endif
+#endif  // !defined(ADDRESS_SANITIZER)
+
+#endif  // CRASHPAD_UTIL_MISC_ADDRESS_SANITIZER_H_
diff --git a/util/util.gyp b/util/util.gyp
index 5768788..418256d 100644
--- a/util/util.gyp
+++ b/util/util.gyp
@@ -84,6 +84,7 @@
         'mach/task_for_pid.h',
         'mach/task_memory.cc',
         'mach/task_memory.h',
+        'misc/address_sanitizer.h',
         'misc/arraysize_unsafe.h',
         'misc/clock.h',
         'misc/clock_mac.cc',