Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(410)

Issue 238123003: Set the default ASan options for executables built with ASan on Linux. (Closed)

Created:
6 years, 8 months ago by Alexander Potapenko
Modified:
6 years, 8 months ago
Reviewers:
Mark Seaborn, Nico
CC:
chromium-reviews, erikwright+watch_chromium.org
Visibility:
Public.

Description

Set the default ASan options for executables built with ASan on Linux. This is a copy of https://codereview.chromium.org/201153007 and https://codereview.chromium.org/213113002 updated to the current trunk with more descriptive comments in sanitizer_options.cc and the additional legacy_pthread_cond=1 option for the Linux Official builds (legacy_pthread_cond is to be deprecated, thus we don't enable it on the bots). This CL introduces a module, base/debug/sanitizer_options.cc, which will override the defaults for various dynamic tools (only ASan at this moment). For every executable built with a dynamic tool this module will be linked into that executable, providing weak functions to be called by the tool. The existing declaration of __asan_default_options() in chrome/app/chrome_exe_main_gtk.cc has been moved into sanitizer_options.cc (now every binary built with GOOGLE_CHROME_BUILD=1 will have the same options as google-chrome-asan. The existing declaration of __asan_default_options() in chrome/nacl/nacl_helper_linux.cc has been kept as is, but we had to remove -Wl,-u_sanitizer_options_link_helper to avoid picking sanitizer_options.cc. The default options target is deliberately disabled on 32-bit Chromium OS builds, where one of the host binaries (mksnapshot.ia32) is 32-bit despite host_arch==x86_64. GYP changes for OSX and iOS will be committed separately. TBR=thakis@chromium.org,mseaborn@chromium.org BUG=302040 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=263941

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+102 lines, -0 lines) Patch
M base/base.gyp View 1 chunk +26 lines, -0 lines 0 comments Download
A base/debug/sanitizer_options.cc View 1 chunk +62 lines, -0 lines 0 comments Download
M build/common.gypi View 1 chunk +8 lines, -0 lines 0 comments Download
M components/nacl.gyp View 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Alexander Potapenko
TBR Re-landing this again. Last time the problem was in legacy_pthread_cond=1 flag, which led to ...
6 years, 8 months ago (2014-04-15 15:23:50 UTC) #1
Alexander Potapenko
The CQ bit was checked by glider@chromium.org
6 years, 8 months ago (2014-04-15 15:23:58 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/glider@chromium.org/238123003/1
6 years, 8 months ago (2014-04-15 15:24:13 UTC) #3
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 8 months ago (2014-04-15 15:24:14 UTC) #4
commit-bot: I haz the power
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an ...
6 years, 8 months ago (2014-04-15 15:24:14 UTC) #5
Alexander Potapenko
The CQ bit was checked by glider@chromium.org
6 years, 8 months ago (2014-04-15 15:26:06 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/glider@chromium.org/238123003/1
6 years, 8 months ago (2014-04-15 15:26:48 UTC) #7
commit-bot: I haz the power
6 years, 8 months ago (2014-04-15 18:39:24 UTC) #8
Message was sent while issue was closed.
Change committed as 263941

Powered by Google App Engine
This is Rietveld 408576698