Skip to content

Commit 5ed6fe0

Browse files
gliderCommit bot
authored andcommitted
Revert of Enable ASan default options on Mac. (patchset #4 id:60001 of https://codereview.chromium.org/581983003/)
Reason for revert: This CL broke ui_unittests on both Mac ASan and Mac ASan 64 bots: /Volumes/data/bool/build/slave/Mac_ASan_Tests__2_/build/src/out/Release/ui_unittests --brave-new-test-launcher --test-launcher-bot-mode --verbose --test-launcher-print-test-stdio=always --gtest_print_time --test-launcher-summary-output=/tmp/tmpnPVxX8 dyld: Library not loaded: @executable_path/../Versions/1.0.0.0/ui_unittests Framework.framework/ui_unittests Framework Referenced from: /Volumes/data/bool/build/slave/Mac_ASan_Tests__2_/build/src/out/Release/ui_unittests Reason: image not found http://build.chromium.org/p/chromium.memory/builders/Mac%20ASan%20Tests%20%282%29/builds/5393/steps/ui_unittests/logs/stdio Original issue's description: > Enable ASan default options on Mac. > > This CL links libsanitizer_options into every executable built with ASan on OSX. > The existing implementation of __asan_default_options for Chromium.app is merged with that in sanitizer_options.cc > > Also now use_sanitizer_options is only set when building with sanitizers so that there isn't an unconditional dependency on an empty object file in every executable in non-sanitizer builds. > > BUG=302040 > [email protected] > > Committed: https://crrev.com/0640a5d19ef72aec62787423e8a7c78c4f62b955 > Cr-Commit-Position: refs/heads/master@{#295958} [email protected],[email protected] NOTREECHECKS=true NOTRY=true BUG=302040 Review URL: https://codereview.chromium.org/593683004 Cr-Commit-Position: refs/heads/master@{#295987}
1 parent 685ccea commit 5ed6fe0

File tree

5 files changed

+46
-41
lines changed

5 files changed

+46
-41
lines changed

build/common.gypi

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@
389389

390390
# Enable Chromium overrides of the default configurations for various
391391
# dynamic tools (like ASan).
392-
'use_sanitizer_options%': 0,
392+
'use_sanitizer_options%': 1,
393393

394394
# Enable building with SyzyAsan.
395395
# See https://code.google.com/p/sawbuck/wiki/SyzyASanHowTo
@@ -2140,7 +2140,6 @@
21402140
['asan==1 or msan==1 or lsan==1 or tsan==1', {
21412141
'clang%': 1,
21422142
'use_allocator%': 'none',
2143-
'use_sanitizer_options%': 1,
21442143
}],
21452144
['asan==1 and OS=="linux" and chromeos==0', {
21462145
'use_custom_libcxx%': 1,
@@ -3481,14 +3480,6 @@
34813480
'-Wl,-z,now',
34823481
'-Wl,-z,relro',
34833482
],
3484-
# TODO(glider): enable the default options on other systems.
3485-
'conditions': [
3486-
['use_sanitizer_options==1 and ((OS=="linux" and (chromeos==0 or target_arch!="ia32")) or OS=="mac")', {
3487-
'dependencies': [
3488-
'<(DEPTH)/build/sanitizers/sanitizers.gyp:sanitizer_options',
3489-
],
3490-
}],
3491-
],
34923483
},
34933484
}],
34943485
# TODO(jochen): Enable this on chromeos on arm. http://crbug.com/356580
@@ -4105,6 +4096,14 @@
41054096
],
41064097
}],
41074098
],
4099+
# TODO(glider): enable the default options on other systems.
4100+
'conditions': [
4101+
['use_sanitizer_options==1 and OS=="linux" and (chromeos==0 or target_arch!="ia32")', {
4102+
'dependencies': [
4103+
'<(DEPTH)/build/sanitizers/sanitizers.gyp:sanitizer_options',
4104+
],
4105+
}],
4106+
],
41084107
}],
41094108
['asan==1', {
41104109
'target_conditions': [

build/sanitizers/sanitizer_options.cc

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,6 @@
77

88
#include "build/build_config.h"
99

10-
#if defined(ADDRESS_SANITIZER) && defined(OS_MACOSX)
11-
#include <crt_externs.h> // for _NSGetArgc, _NSGetArgv
12-
#include <string.h>
13-
#endif // ADDRESS_SANITIZER && OS_MACOSX
14-
1510
// Functions returning default options are declared weak in the tools' runtime
1611
// libraries. To make the linker pick the strong replacements for those
1712
// functions from this module, we explicitly force its inclusion by passing
@@ -66,8 +61,6 @@ const char *kAsanDefaultOptions =
6661
const char *kAsanDefaultOptions =
6762
"strict_memcmp=0 replace_intrin=0 check_printf=1 use_sigaltstack=1 "
6863
"strip_path_prefix=Release/../../ ";
69-
static const char kNaClDefaultOptions[] = "handle_segv=0";
70-
static const char kNaClFlag[] = "--type=nacl-loader";
7164
#endif // OS_LINUX
7265

7366
#if defined(OS_LINUX) || defined(OS_MACOSX)
@@ -78,18 +71,6 @@ __attribute__((visibility("default")))
7871
// stripped by the linker.
7972
__attribute__((used))
8073
const char *__asan_default_options() {
81-
#if defined(OS_MACOSX)
82-
char*** argvp = _NSGetArgv();
83-
int* argcp = _NSGetArgc();
84-
if (!argvp || !argcp) return kAsanDefaultOptions;
85-
char** argv = *argvp;
86-
int argc = *argcp;
87-
for (int i = 0; i < argc; ++i) {
88-
if (strcmp(argv[i], kNaClFlag) == 0) {
89-
return kNaClDefaultOptions;
90-
}
91-
}
92-
#endif
9374
return kAsanDefaultOptions;
9475
}
9576
#endif // OS_LINUX || OS_MACOSX

build/sanitizers/sanitizers.gyp

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,6 @@
4545
'ldflags': [
4646
'-Wl,-u_sanitizer_options_link_helper',
4747
],
48-
'target_conditions': [
49-
['_type=="executable"', {
50-
'xcode_settings': {
51-
'OTHER_LDFLAGS': [
52-
'-Wl,-u,__sanitizer_options_link_helper',
53-
],
54-
},
55-
}],
56-
],
5748
},
5849
},
5950
],

chrome/app/chrome_exe_main_mac.cc

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,45 @@
55
// The entry point for all Mac Chromium processes, including the outer app
66
// bundle (browser) and helper app (renderer, plugin, and friends).
77

8+
#if defined(ADDRESS_SANITIZER)
9+
#include <crt_externs.h> // for _NSGetArgc, _NSGetArgv
10+
#include <string.h>
11+
#endif // ADDRESS_SANITIZER
812
#include <stdlib.h>
913

14+
#if defined(ADDRESS_SANITIZER)
15+
// NaCl requires its own SEGV handler, so we need to add handle_segv=0 to
16+
// ASAN_OPTIONS. This is done by injecting __asan_default_options into the
17+
// executable.
18+
// Because there's no distinct NaCl executable on OSX, we have to look at the
19+
// command line arguments to understand whether the process is a NaCl loader.
20+
21+
static const char kNaClDefaultOptions[] = "handle_segv=0";
22+
static const char kNaClFlag[] = "--type=nacl-loader";
23+
24+
extern "C"
25+
// __asan_default_options() is called at ASan initialization, so it must
26+
// not be instrumented with ASan -- thus the "no_sanitize_address" attribute.
27+
__attribute__((no_sanitize_address))
28+
// The function isn't referenced from the executable itself. Make sure it isn't
29+
// stripped by the linker.
30+
__attribute__((used))
31+
__attribute__((visibility("default")))
32+
const char* __asan_default_options() {
33+
char*** argvp = _NSGetArgv();
34+
int* argcp = _NSGetArgc();
35+
if (!argvp || !argcp) return NULL;
36+
char** argv = *argvp;
37+
int argc = *argcp;
38+
for (int i = 0; i < argc; ++i) {
39+
if (strcmp(argv[i], kNaClFlag) == 0) {
40+
return kNaClDefaultOptions;
41+
}
42+
}
43+
return NULL;
44+
}
45+
#endif // ADDRESS_SANITIZER
46+
1047
extern "C" {
1148
int ChromeMain(int argc, char** argv);
1249
} // extern "C"

chrome/app/framework.order

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,6 @@ ___gcov_flush
4646
___gcov_merge_add
4747
___gcov_fork
4848

49-
# Provided by build/sanitizers/sanitizer_options.cc in ASan builds.
50-
___asan_default_options
51-
5249
# Written in asm as a .globl. (Is that necessary?)
5350
_NaClSwitch
5451
_NaClSyscallSeg

0 commit comments

Comments
 (0)