Bug 209963 – [Xcode] Replace ASAN_OTHER_CFLAGS and ASAN_OTHER_CPLUSPLUSFLAGS with $(inherited)

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[David Kilzer (:ddkilzer)]

Replace ASAN_OTHER_CFLAGS and ASAN_OTHER_CPLUSPLUSFLAGS with the use of $(inherited) in Tools/asan/asan.xcconfig.

[Radar WebKit Bug Importer]

<rdar://problem/61257504>

[David Kilzer (:ddkilzer)]

BTW, TestWebKitAPI on macOS may not have been built with ASan enabled due to this:

OTHER_CFLAGS = $(ASAN_OTHER_CFLAGS) --system-header-prefix=WebKit/;
OTHER_CFLAGS[sdk=macosx*] = $(inherited) -iframework $(SDKROOT)$(SYSTEM_LIBRARY_DIR)/PrivateFrameworks;
OTHER_CPLUSPLUSFLAGS = $(ASAN_OTHER_CPLUSPLUSFLAGS);
OTHER_LDFLAGS = $(ASAN_OTHER_LDFLAGS);

So this change might surface some issues in macOS-only TestWebKitAPI tests.

[David Kilzer (:ddkilzer)]

Created attachment 395372 [details]
Patch v1

[EWS Watchlist]

Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE

[Jonathan Bedard]

(In reply to David Kilzer (:ddkilzer) from comment #0)
> Replace ASAN_OTHER_CFLAGS and ASAN_OTHER_CPLUSPLUSFLAGS with the use of
> $(inherited) in Tools/asan/asan.xcconfig.

The change looks correct, how did you test this?

[David Kilzer (:ddkilzer)]

(In reply to Jonathan Bedard from comment #5)
> (In reply to David Kilzer (:ddkilzer) from comment #0)
> > Replace ASAN_OTHER_CFLAGS and ASAN_OTHER_CPLUSPLUSFLAGS with the use of
> > $(inherited) in Tools/asan/asan.xcconfig.
> 
> The change looks correct, how did you test this?

Yep.  Built with ASan enabled on my 10.14.6 MacBook Pro:

$ make release SDKROOT=macosx.internal ARCHS=x86_64 ONLY_ACTIVE_ARCH=YES 2>&1 | tee ./build-release-macOS-asan.txt | ./Tools/Scripts/filter-build-webkit

$ grep -A2 '^CompileC' ./build-release-macOS-asan.txt | grep 'clang -x' | grep -v -- '-fsanitize=address' | grep -v 'Source/third_party/yasm/'
$ 

Everything except yasm (see Bug 190327 and r236898) was built with ASan.

[Jonathan Bedard]

(In reply to David Kilzer (:ddkilzer) from comment #6)
> (In reply to Jonathan Bedard from comment #5)
> > (In reply to David Kilzer (:ddkilzer) from comment #0)
> > > Replace ASAN_OTHER_CFLAGS and ASAN_OTHER_CPLUSPLUSFLAGS with the use of
> > > $(inherited) in Tools/asan/asan.xcconfig.
> > 
> > The change looks correct, how did you test this?
> 
> Yep.  Built with ASan enabled on my 10.14.6 MacBook Pro:
> 
> $ make release SDKROOT=macosx.internal ARCHS=x86_64 ONLY_ACTIVE_ARCH=YES
> 2>&1 | tee ./build-release-macOS-asan.txt |
> ./Tools/Scripts/filter-build-webkit
> 
> $ grep -A2 '^CompileC' ./build-release-macOS-asan.txt | grep 'clang -x' |
> grep -v -- '-fsanitize=address' | grep -v 'Source/third_party/yasm/'
> $ 
> 
> Everything except yasm (see Bug 190327 and r236898) was built with ASan.

Probably should apply the r236898's logic to ImageDiff, but that's a different discussion.

[EWS]

Committed r259466: <https://trac.webkit.org/changeset/259466>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395372 [details].

[David Kilzer (:ddkilzer)]

(In reply to Jonathan Bedard from comment #7)
> (In reply to David Kilzer (:ddkilzer) from comment #6)
> > (In reply to Jonathan Bedard from comment #5)
> > > (In reply to David Kilzer (:ddkilzer) from comment #0)
> > > > Replace ASAN_OTHER_CFLAGS and ASAN_OTHER_CPLUSPLUSFLAGS with the use of
> > > > $(inherited) in Tools/asan/asan.xcconfig.
> > > 
> > > The change looks correct, how did you test this?
> > 
> > Yep.  Built with ASan enabled on my 10.14.6 MacBook Pro:
> > 
> > $ make release SDKROOT=macosx.internal ARCHS=x86_64 ONLY_ACTIVE_ARCH=YES
> > 2>&1 | tee ./build-release-macOS-asan.txt |
> > ./Tools/Scripts/filter-build-webkit
> > 
> > $ grep -A2 '^CompileC' ./build-release-macOS-asan.txt | grep 'clang -x' |
> > grep -v -- '-fsanitize=address' | grep -v 'Source/third_party/yasm/'
> > $ 
> > 
> > Everything except yasm (see Bug 190327 and r236898) was built with ASan.
> 
> Probably should apply the r236898's logic to ImageDiff, but that's a
> different discussion.

Why?  Does running ImageDiff with ASan slow down test runs significantly?

[Jonathan Bedard]

(In reply to David Kilzer (:ddkilzer) from comment #9)
> (In reply to Jonathan Bedard from comment #7)
> > (In reply to David Kilzer (:ddkilzer) from comment #6)
> > > (In reply to Jonathan Bedard from comment #5)
> > > > (In reply to David Kilzer (:ddkilzer) from comment #0)
> > > > > Replace ASAN_OTHER_CFLAGS and ASAN_OTHER_CPLUSPLUSFLAGS with the use of
> > > > > $(inherited) in Tools/asan/asan.xcconfig.
> > > > 
> > > > The change looks correct, how did you test this?
> > > 
> > > Yep.  Built with ASan enabled on my 10.14.6 MacBook Pro:
> > > 
> > > $ make release SDKROOT=macosx.internal ARCHS=x86_64 ONLY_ACTIVE_ARCH=YES
> > > 2>&1 | tee ./build-release-macOS-asan.txt |
> > > ./Tools/Scripts/filter-build-webkit
> > > 
> > > $ grep -A2 '^CompileC' ./build-release-macOS-asan.txt | grep 'clang -x' |
> > > grep -v -- '-fsanitize=address' | grep -v 'Source/third_party/yasm/'
> > > $ 
> > > 
> > > Everything except yasm (see Bug 190327 and r236898) was built with ASan.
> > 
> > Probably should apply the r236898's logic to ImageDiff, but that's a
> > different discussion.
> 
> Why?  Does running ImageDiff with ASan slow down test runs significantly?

Not that I'm aware of, but like yasm, it's a tool built with something other than the SDK being tested.