Bug 187222

Summary: [CMake] build-webkit should be able to enable address sanitizer
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: Tools / TestsAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, calvaris, dbates, ews-watchlist, lforschler, mcatanzaro, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch dbates: review+

Description Michael Catanzaro 2018-06-30 11:58:02 PDT
If I have run 'set-webkit-configuration --asan' then build-webkit should pass -DENABLE_ADDRESS_SANITIZER=ON to CMake, so it actually does something.
Comment 1 Michael Catanzaro 2018-06-30 11:58:37 PDT
Created attachment 344018 [details]
Patch
Comment 2 Daniel Bates 2018-06-30 12:48:50 PDT
Comment on attachment 344018 [details]
Patch

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

Do we already support ASan building with CMake using script build-jsc? If not, we should fix this up.

> Tools/ChangeLog:7
> +

Please add a description for this change. I liked what you wrote in comment 0.
Comment 3 Daniel Bates 2018-06-30 12:50:20 PDT
Comment on attachment 344018 [details]
Patch

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

> Tools/Scripts/webkitdirs.pm:2255
> +    determineASanIsEnabled();

It is unnecessary to call this. We should call asanIsEnabled() on the line below (notice it is a function call).
Comment 4 Michael Catanzaro 2018-06-30 13:57:56 PDT
(In reply to Daniel Bates from comment #2)
> Comment on attachment 344018 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=344018&action=review
> 
> Do we already support ASan building with CMake using script build-jsc? If
> not, we should fix this up.

We did not, but build-jsc uses buildCMakeProjectOrExit(), which calls generateBuildSystemFromCMakeProject(), which is the function I'm modifying here, so now it will.

> > Tools/ChangeLog:7
> > +
> 
> Please add a description for this change. I liked what you wrote in comment
> 0.

OK.


(In reply to Daniel Bates from comment #3)
> It is unnecessary to call this. We should call asanIsEnabled() on the line
> below (notice it is a function call).

Yes.
Comment 5 Michael Catanzaro 2018-06-30 14:07:54 PDT
Committed r233402: <https://trac.webkit.org/changeset/233402>
Comment 6 Radar WebKit Bug Importer 2018-06-30 14:08:23 PDT
<rdar://problem/41682664>