Bug 136765 – Move ASan flag settings from DebugRelease.xcconfig to Base.xcconfig

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

[David Farler]

Production-style builds aren't picking up ASan flags even though the ASan xcconfig is passed at the command line. We need to move those settings up to Base.xcconfig wherever it occurs in DebugRelease.xcconfig.

[Alexey Proskuryakov]

Dana, isn't this why ASan roots build incorrectly?

[Dana Burkart]

(In reply to comment #1)
> Dana, isn't this why ASan roots build incorrectly?

Could be. I will test this theory out.

[Radar WebKit Bug Importer]

<rdar://problem/19610008>

[Dana Burkart]

Created attachment 245411 [details]
This patch moves all of the ASan environment variables

[WebKit Commit Bot]

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

[WebKit Commit Bot]

Attachment 245411 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dana Burkart]

Created attachment 245412 [details]
Fix style issue

[Darin Adler]

Comment on attachment 245412 [details]
Fix style issue

Why do some of these also include $(inherited) but others don’t?

[Alexey Proskuryakov]

Comment on attachment 245412 [details]
Fix style issue

View in context: https://bugs.webkit.org/attachment.cgi?id=245412&action=review

This breaks the build: "recursive template instantiation exceeded maximum depth".

That's caused by losing this option from DebugRelease.xcconfig in TestWebKitAPI:

OTHER_CPLUSPLUSFLAGS = -ftemplate-depth=256 $(ASAN_OTHER_CPLUSPLUSFLAGS);

Grepping for OTHER_ in WebKit, I noticed that DumpRenderTree.xcodeproj overrides some of these flags to empty in its "All" target. Is this just accidental noise? Can it be deleted?

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:30365
> -				OTHER_LDFLAGS = "";
> +				OTHER_LDFLAGS = "$(ASAN_OTHER_LDFLAGS)";

The WebCoreExportFileGenerator target configuration looks super weird. We should probably give it its own xcconfig file, rather than use WebCore.xcconfig and then override its values.

[Dana Burkart]

(In reply to comment #8)
> Comment on attachment 245412 [details]
> Fix style issue
> 
> Why do some of these also include $(inherited) but others don’t?

I'm not sure. All of the projects with DebugRelease.xcconfigs which had $(inherited) don't set the environment variables anywhere else, so I guess they were there just to be safe?

[Dana Burkart]

(In reply to comment #9)
> Comment on attachment 245412 [details]
> Fix style issue
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=245412&action=review
> 
> This breaks the build: "recursive template instantiation exceeded maximum
> depth".
> 
> That's caused by losing this option from DebugRelease.xcconfig in
> TestWebKitAPI:
> 
> OTHER_CPLUSPLUSFLAGS = -ftemplate-depth=256 $(ASAN_OTHER_CPLUSPLUSFLAGS);
> 

Okay, I'll put that back in.

> Grepping for OTHER_ in WebKit, I noticed that DumpRenderTree.xcodeproj
> overrides some of these flags to empty in its "All" target. Is this just
> accidental noise? Can it be deleted?

I think so. I'll give it a shot.

> 
> > Source/WebCore/WebCore.xcodeproj/project.pbxproj:30365
> > -				OTHER_LDFLAGS = "";
> > +				OTHER_LDFLAGS = "$(ASAN_OTHER_LDFLAGS)";
> 
> The WebCoreExportFileGenerator target configuration looks super weird. We
> should probably give it its own xcconfig file, rather than use
> WebCore.xcconfig and then override its values.

The other targets in WebCore set it to $(ASAN_OTHER_LDFLAGS) instead of being empty, so I simply changed it to match...

[Alexey Proskuryakov]

> The other targets in WebCore set it to $(ASAN_OTHER_LDFLAGS) instead of being empty, so I simply changed it to match...

Right, I don't think that factoring out an xcconfig for this target should be part of this patch.

[Dana Burkart]

Created attachment 245493 [details]
Address concerns

[Dana Burkart]

Committed revision 179269.