Bug 139916

Summary: Simplify building with ASan
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Tools / TestsAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dbates, dburkart, ddkilzer, mrowe
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch mrowe: review+

Description Alexey Proskuryakov 2014-12-23 14:31:19 PST
Make the following work:

set-webkit-configuration --[no]-asan

Building will just work after this, provided that the installed toolchain supports ASan.

make ASAN=(YES|NO)

This will be equivalent to running set-webkit-configuration, and then make.

Running the binaries in all normal ways will work too, with some caveats (notably, no symbolication, and somewhat broken run-webkit-tests integration).
Comment 1 Alexey Proskuryakov 2014-12-23 14:39:56 PST
Created attachment 243695 [details]
proposed patch
Comment 2 Mark Rowe (bdash) 2014-12-23 15:48:22 PST
Comment on attachment 243695 [details]
proposed patch

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

> Makefile.shared:60
> +ifneq (,$(ASAN))
> +	$(SCRIPTS_PATH)/set-webkit-configuration $(ASAN_OPTION)
> +endif

This may run set-webkit-configuration when ASAN_OPTIONS hasn't been set, such as if ASAN=foo is passed. Should we guard this on whether ASAN_OPTION is set instead?

> Tools/Scripts/webkitdirs.pm:366
> +    if (open ASAN, "$baseProductDir/ASan") {
> +        $asanConfigurationValue = <ASAN>;
> +        close ASAN;
> +    }
> +    if ($asanConfigurationValue) {
> +        chomp $asanConfigurationValue;
> +        $asanIsEnabled = 1 if $asanConfigurationValue eq "YES";
> +    }

Is there a reason to use two if blocks here? Isn't it safe to assume $asanConfigurationValue is defined if we were able to open the file?

> Tools/asan/asan.xcconfig:12
> +ASAN_OTHER_LDFLAGS = -fsanitize=address
> +
> +GCC_ENABLE_OBJC_GC = NO

Semicolons at end of line.
Comment 3 Alexey Proskuryakov 2014-12-23 16:04:13 PST
> Should we guard this on whether ASAN_OPTION is set instead?

Yes, this makes the behavior consistent with "make debug ASAN=foo".

> Is there a reason to use two if blocks here? Isn't it safe to assume $asanConfigurationValue is defined if we were able to open the file?

Probably not, was just following what we do for configuration. I'll merge these.
Comment 4 Alexey Proskuryakov 2014-12-23 16:13:39 PST
Committed <http://trac.webkit.org/r177703>.