Summary: | Simplify building with ASan | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||
Component: | Tools / Tests | Assignee: | 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
Alexey Proskuryakov
2014-12-23 14:31:19 PST
Created attachment 243695 [details]
proposed patch
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. > 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. Committed <http://trac.webkit.org/r177703>. |