Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
7541045:
Adding GYP_DEFINES=ASAN to enable ASAN build (Closed)
|
Created:
9 years, 4 months ago by glotov Modified:
9 years, 4 months ago Reviewers:
Nico CC:
chromium-reviews, davemoore+watch_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionAdding GYP_DEFINES=ASAN to enable ASAN build
BUG=chromium-os:16717
TEST=manual yet
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=96772
Patch Set 1 #Patch Set 2 : sync #
Total comments: 4
Patch Set 3 : review comments fixed #
Total comments: 4
Patch Set 4 : fixes #
Total comments: 2
Patch Set 5 : comment added #Messages
Total messages: 17 (0 generated)
Hi! Please have a look.
http://codereview.chromium.org/7541045/diff/1001/build/common.gypi File build/common.gypi (right): http://codereview.chromium.org/7541045/diff/1001/build/common.gypi#newcode407 build/common.gypi:407: 'asan%': 0, Should setting this set 'clang' to 1 automatically? http://codereview.chromium.org/7541045/diff/1001/build/common.gypi#newcode1519 build/common.gypi:1519: 'target_conditions': [ I think the 'target_conditions'/ '_toolset' part should no longer be necessary when people build with CC.host set as now described on http://code.google.com/p/chromium/wiki/Clang
Hi Nico! What are CC.host and CXX.host variables? I don't know about them. On Fri, Aug 5, 2011 at 10:37 PM, <thakis@chromium.org> wrote: > > http://codereview.chromium.**org/7541045/diff/1001/build/**common.gypi<http:/... > File build/common.gypi (right): > > http://codereview.chromium.**org/7541045/diff/1001/build/** > common.gypi#newcode407<http://codereview.chromium.org/7541045/diff/1001/build/common.gypi#newcode407> > build/common.gypi:407: 'asan%': 0, > Should setting this set 'clang' to 1 automatically? > > http://codereview.chromium.**org/7541045/diff/1001/build/** > common.gypi#newcode1519<http://codereview.chromium.org/7541045/diff/1001/build/common.gypi#newcode1519> > build/common.gypi:1519: 'target_conditions': [ > I think the 'target_conditions'/ '_toolset' part should no longer be > necessary when people build with CC.host set as now described on > http://code.google.com/p/**chromium/wiki/Clang<http://code.google.com/p/chrom... > > > http://codereview.chromium.**org/7541045/<http://codereview.chromium.org/7541... > -- Thank you, Denis
See http://code.google.com/p/chromium/wiki/Clang On Fri, Aug 5, 2011 at 11:37 AM, <thakis@chromium.org> wrote: > > http://codereview.chromium.org/7541045/diff/1001/build/common.gypi > File build/common.gypi (right): > > http://codereview.chromium.org/7541045/diff/1001/build/common.gypi#newcode407 > build/common.gypi:407: 'asan%': 0, > Should setting this set 'clang' to 1 automatically? > > http://codereview.chromium.org/7541045/diff/1001/build/common.gypi#newcode1519 > build/common.gypi:1519: 'target_conditions': [ > I think the 'target_conditions'/ '_toolset' part should no longer be > necessary when people build with CC.host set as now described on > http://code.google.com/p/chromium/wiki/Clang > > http://codereview.chromium.org/7541045/ >
They're used for cross-compiling. Do you use clang with cross-compiled CrOS builds (say, arm)? On Fri, Aug 5, 2011 at 12:03 PM, Nico Weber <thakis@chromium.org> wrote: > See http://code.google.com/p/chromium/wiki/Clang > > On Fri, Aug 5, 2011 at 11:37 AM, <thakis@chromium.org> wrote: >> >> http://codereview.chromium.org/7541045/diff/1001/build/common.gypi >> File build/common.gypi (right): >> >> http://codereview.chromium.org/7541045/diff/1001/build/common.gypi#newcode407 >> build/common.gypi:407: 'asan%': 0, >> Should setting this set 'clang' to 1 automatically? >> >> http://codereview.chromium.org/7541045/diff/1001/build/common.gypi#newcode1519 >> build/common.gypi:1519: 'target_conditions': [ >> I think the 'target_conditions'/ '_toolset' part should no longer be >> necessary when people build with CC.host set as now described on >> http://code.google.com/p/chromium/wiki/Clang >> >> http://codereview.chromium.org/7541045/ >> >
Hi Nico! Please have another look. http://codereview.chromium.org/7541045/diff/1001/build/common.gypi File build/common.gypi (right): http://codereview.chromium.org/7541045/diff/1001/build/common.gypi#newcode407 build/common.gypi:407: 'asan%': 0, On 2011/08/05 18:37:19, Nico wrote: > Should setting this set 'clang' to 1 automatically? Done. http://codereview.chromium.org/7541045/diff/1001/build/common.gypi#newcode1519 build/common.gypi:1519: 'target_conditions': [ On 2011/08/05 18:37:19, Nico wrote: > I think the 'target_conditions'/ '_toolset' part should no longer be necessary > when people build with CC.host set as now described on > http://code.google.com/p/chromium/wiki/Clang Done.
LGTM http://codereview.chromium.org/7541045/diff/5001/build/common.gypi File build/common.gypi (right): http://codereview.chromium.org/7541045/diff/5001/build/common.gypi#newcode1525 build/common.gypi:1525: '-fasan', Add a comment above this line "-fasan only works with clang, but asan=1 implies clang=1"
http://codereview.chromium.org/7541045/diff/5001/build/common.gypi File build/common.gypi (right): http://codereview.chromium.org/7541045/diff/5001/build/common.gypi#newcode1524 build/common.gypi:1524: 'cflags': [ Is this required for ldflags as well? aarya was seeing issues building genversion without this in ldflag
-ldl is generated automatically as soon as one uses ASAN-patched clang with -fasan option. Or did I miss what you said? On Wed, Aug 10, 2011 at 10:48 PM, <thakis@chromium.org> wrote: > > http://codereview.chromium.org/7541045/diff/5001/build/common.gypi > File build/common.gypi (right): > > http://codereview.chromium.org/7541045/diff/5001/build/common.gypi#newcode1524 > build/common.gypi:1524: 'cflags': [ > Is this required for ldflags as well? aarya was seeing issues building > genversion without this in ldflag > > http://codereview.chromium.org/7541045/ > -- Thank you, Denis
Here's the error aarya was seeing (note: You need to set LINK.host=$CXX before seeing this): /usr/local/google/chrome/src/third_party/asan/asan_clang_Linux/bin/clang++ -pthread -Wl,-z,noexecstack -o llvm/Debug/genversion -Wl,--start-group llvm/Debug/obj.host/genversion/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.o -Wl,--end-group /usr/bin/ld: llvm/Debug/obj.host/genversion/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.o: in function asan.module_ctor:/usr/local/google/chrome/src/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.c:70: error: undefined reference to '__asan_init' On Wed, Aug 10, 2011 at 11:53 AM, Denis Glotov <glotov@chromium.org> wrote: > -ldl is generated automatically as soon as one uses ASAN-patched clang > with -fasan option. Or did I miss what you said? > > On Wed, Aug 10, 2011 at 10:48 PM, <thakis@chromium.org> wrote: >> >> http://codereview.chromium.org/7541045/diff/5001/build/common.gypi >> File build/common.gypi (right): >> >> http://codereview.chromium.org/7541045/diff/5001/build/common.gypi#newcode1524 >> build/common.gypi:1524: 'cflags': [ >> Is this required for ldflags as well? aarya was seeing issues building >> genversion without this in ldflag >> >> http://codereview.chromium.org/7541045/ >> > > > > -- > Thank you, > Denis >
Yes, I just noticed the ASAN change. Will amend and test it tomorrow. Thanks! One question. What made you switch from CC/CXX/LINK to CC.host/CXX.host/LINK.host? Why did you do it? On Wed, Aug 10, 2011 at 11:44 PM, Nico Weber <thakis@chromium.org> wrote: > Here's the error aarya was seeing (note: You need to set > LINK.host=$CXX before seeing this): > > /usr/local/google/chrome/src/third_party/asan/asan_clang_Linux/bin/clang++ > -pthread -Wl,-z,noexecstack -o llvm/Debug/genversion > -Wl,--start-group > llvm/Debug/obj.host/genversion/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.o > -Wl,--end-group > /usr/bin/ld: llvm/Debug/obj.host/genversion/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.o: > in function asan.module_ctor:/usr/local/google/chrome/src/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.c:70: > error: undefined reference to '__asan_init' > > > On Wed, Aug 10, 2011 at 11:53 AM, Denis Glotov <glotov@chromium.org> wrote: >> -ldl is generated automatically as soon as one uses ASAN-patched clang >> with -fasan option. Or did I miss what you said? >> >> On Wed, Aug 10, 2011 at 10:48 PM, <thakis@chromium.org> wrote: >>> >>> http://codereview.chromium.org/7541045/diff/5001/build/common.gypi >>> File build/common.gypi (right): >>> >>> http://codereview.chromium.org/7541045/diff/5001/build/common.gypi#newcode1524 >>> build/common.gypi:1524: 'cflags': [ >>> Is this required for ldflags as well? aarya was seeing issues building >>> genversion without this in ldflag >>> >>> http://codereview.chromium.org/7541045/ >>> >> >> >> >> -- >> Thank you, >> Denis >> > -- Thank you, Denis
On Thu, Aug 11, 2011 at 1:13 PM, Denis Glotov <glotov@chromium.org> wrote: > Yes, I just noticed the ASAN change. Will amend and test it tomorrow. Thanks! > One question. What made you switch from CC/CXX/LINK to > CC.host/CXX.host/LINK.host? Why did you do it? There's no switch, CC.host etc are set in addition to CC etc. This was done so that clang is used for both host and target files (we're not cross-compiling here after all), so that everything's built with clang (previously a few things were still built with gcc) > > On Wed, Aug 10, 2011 at 11:44 PM, Nico Weber <thakis@chromium.org> wrote: >> Here's the error aarya was seeing (note: You need to set >> LINK.host=$CXX before seeing this): >> >> /usr/local/google/chrome/src/third_party/asan/asan_clang_Linux/bin/clang++ >> -pthread -Wl,-z,noexecstack -o llvm/Debug/genversion >> -Wl,--start-group >> llvm/Debug/obj.host/genversion/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.o >> -Wl,--end-group >> /usr/bin/ld: llvm/Debug/obj.host/genversion/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.o: >> in function asan.module_ctor:/usr/local/google/chrome/src/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.c:70: >> error: undefined reference to '__asan_init' >> >> >> On Wed, Aug 10, 2011 at 11:53 AM, Denis Glotov <glotov@chromium.org> wrote: >>> -ldl is generated automatically as soon as one uses ASAN-patched clang >>> with -fasan option. Or did I miss what you said? >>> >>> On Wed, Aug 10, 2011 at 10:48 PM, <thakis@chromium.org> wrote: >>>> >>>> http://codereview.chromium.org/7541045/diff/5001/build/common.gypi >>>> File build/common.gypi (right): >>>> >>>> http://codereview.chromium.org/7541045/diff/5001/build/common.gypi#newcode1524 >>>> build/common.gypi:1524: 'cflags': [ >>>> Is this required for ldflags as well? aarya was seeing issues building >>>> genversion without this in ldflag >>>> >>>> http://codereview.chromium.org/7541045/ >>>> >>> >>> >>> >>> -- >>> Thank you, >>> Denis >>> >> > > > > -- > Thank you, > Denis >
Yes, I've just seen this on latest sources. Seems that this is because libasan32.a library is not included in the linker arguments, while it should. -ldl and -lpthread are added (but maybe by someone else). Is the problem fixed or should I work with ASAN team on it? And regarding "CC.host=$CC CXX.host=$CXX LINK.host=$CXX". Do you think this could be hidden to some GYP file? This seem not to be what user should deal with herself. On Wed, Aug 10, 2011 at 11:44 PM, Nico Weber <thakis@chromium.org> wrote: > Here's the error aarya was seeing (note: You need to set > LINK.host=$CXX before seeing this): > > /usr/local/google/chrome/src/third_party/asan/asan_clang_Linux/bin/clang++ > -pthread -Wl,-z,noexecstack -o llvm/Debug/genversion > -Wl,--start-group > > llvm/Debug/obj.host/genversion/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.o > -Wl,--end-group > /usr/bin/ld: > llvm/Debug/obj.host/genversion/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.o: > in function > asan.module_ctor:/usr/local/google/chrome/src/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.c:70: > error: undefined reference to '__asan_init' > > > On Wed, Aug 10, 2011 at 11:53 AM, Denis Glotov <glotov@chromium.org> > wrote: > > -ldl is generated automatically as soon as one uses ASAN-patched clang > > with -fasan option. Or did I miss what you said? > > > > On Wed, Aug 10, 2011 at 10:48 PM, <thakis@chromium.org> wrote: > >> > >> http://codereview.chromium.org/7541045/diff/5001/build/common.gypi > >> File build/common.gypi (right): > >> > >> > http://codereview.chromium.org/7541045/diff/5001/build/common.gypi#newcode1524 > >> build/common.gypi:1524: 'cflags': [ > >> Is this required for ldflags as well? aarya was seeing issues building > >> genversion without this in ldflag > >> > >> http://codereview.chromium.org/7541045/ > >> > > > > > > > > -- > > Thank you, > > Denis > > > -- Thank you, Denis
On Fri, Aug 12, 2011 at 10:15 AM, Denis Glotov <glotov@chromium.org> wrote: > Yes, I've just seen this on latest sources. Seems that this is > because libasan32.a library is not included in the linker arguments, while > it should. -ldl and -lpthread are added (but maybe by someone else). Is the > problem fixed or should I work with ASAN team on it? Talk to the ASAN guys. I guess the fix is to add asan to the gyp 'libraries' (not sure if that's the right key) if 'asan=1'. > And regarding "CC.host=$CC CXX.host=$CXX LINK.host=$CXX". Do you think this > could be hidden to some GYP file? This seem not to be what user should deal > with herself. It might be possible, I have 50% of a plan for that at the moment :-) > > > On Wed, Aug 10, 2011 at 11:44 PM, Nico Weber <thakis@chromium.org> wrote: >> >> Here's the error aarya was seeing (note: You need to set >> LINK.host=$CXX before seeing this): >> >> >> /usr/local/google/chrome/src/third_party/asan/asan_clang_Linux/bin/clang++ >> -pthread -Wl,-z,noexecstack -o llvm/Debug/genversion >> -Wl,--start-group >> >> llvm/Debug/obj.host/genversion/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.o >> -Wl,--end-group >> /usr/bin/ld: >> llvm/Debug/obj.host/genversion/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.o: >> in function >> asan.module_ctor:/usr/local/google/chrome/src/third_party/yasm/source/patched-yasm/modules/preprocs/nasm/genversion.c:70: >> error: undefined reference to '__asan_init' >> >> >> On Wed, Aug 10, 2011 at 11:53 AM, Denis Glotov <glotov@chromium.org> >> wrote: >> > -ldl is generated automatically as soon as one uses ASAN-patched clang >> > with -fasan option. Or did I miss what you said? >> > >> > On Wed, Aug 10, 2011 at 10:48 PM, <thakis@chromium.org> wrote: >> >> >> >> http://codereview.chromium.org/7541045/diff/5001/build/common.gypi >> >> File build/common.gypi (right): >> >> >> >> >> >> http://codereview.chromium.org/7541045/diff/5001/build/common.gypi#newcode1524 >> >> build/common.gypi:1524: 'cflags': [ >> >> Is this required for ldflags as well? aarya was seeing issues building >> >> genversion without this in ldflag >> >> >> >> http://codereview.chromium.org/7541045/ >> >> >> > >> > >> > >> > -- >> > Thank you, >> > Denis >> > > > > > -- > Thank you, > Denis >
Successfully built and run with BUILDTYPE=Release. In Debug mode I see errors in asan_rtl.cc, tracking it. http://codereview.chromium.org/7541045/diff/5001/build/common.gypi File build/common.gypi (right): http://codereview.chromium.org/7541045/diff/5001/build/common.gypi#newcode1524 build/common.gypi:1524: 'cflags': [ Yes, ldflags were added. ldl flag is passed by ASAN-patched Clang to linker when it fases -fasan. http://codereview.chromium.org/7541045/diff/5001/build/common.gypi#newcode1525 build/common.gypi:1525: '-fasan', On 2011/08/10 18:15:20, Nico wrote: > Add a comment above this line "-fasan only works with clang, but asan=1 implies > clang=1" Done.
Do you want to figure out what's up with release builds before checking in or after? If after, this lgtm. http://codereview.chromium.org/7541045/diff/14001/build/common.gypi File build/common.gypi (right): http://codereview.chromium.org/7541045/diff/14001/build/common.gypi#newcode1556 build/common.gypi:1556: ['asan==1', { Add a comment "# Only in the linux section for now, since ASAN doesn't work on Mac yet."
Release build works fine, Debug build errors are tracked here: https://groups.google.com/a/google.com/group/asan-users/browse_thread/thread/... http://codereview.chromium.org/7541045/diff/14001/build/common.gypi File build/common.gypi (right): http://codereview.chromium.org/7541045/diff/14001/build/common.gypi#newcode1556 build/common.gypi:1556: ['asan==1', { On 2011/08/12 20:17:27, Nico wrote: > Add a comment "# Only in the linux section for now, since ASAN doesn't work on > Mac yet." Done. |
