Bug 216746 – Enable AddressSanitizer in C++ std library templates

• Component: Tools / Tests
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

Enable AddressSanitizer in C++ std library templates.

This amounts to undefining the _LIBCPP_HAS_NO_ASAN macro when building.

Although most of WebKit doesn't used C++ std library types like std::vector or std::hash, ANGLE and libwebrtc do use these types, so it's beneficial to enable AddressSanitizer in these cases when compiling with Asan enabled for the rest of WebKit.

[David Kilzer (:ddkilzer)]

Created attachment 409239 [details]
Patch v1

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #0)
> Although most of WebKit doesn't used C++ std library types like std::vector
> or std::hash, ANGLE and libwebrtc do use these types, so it's beneficial to
> enable AddressSanitizer in these cases when compiling with Asan enabled for
> the rest of WebKit.

Actually, I think this is mostly affects std::vector and related classes, not std::hash.

[David Kilzer (:ddkilzer)]

I CCed folks that work on ANGLE and libwebrtc since there is a small chance that enabling this might find some latent issues in those libraries when running layout tests.

[Alexey Proskuryakov]

Comment on attachment 409239 [details]
Patch v1

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

> Tools/sanitizer/asan.xcconfig:9
> +WK_SANITIZER_OTHER_CPLUSPLUSFLAGS_YES = $(inherited) -U_LIBCPP_HAS_NO_ASAN;

Nice. Did you run tests locally to confirm that nothing terrible was going to happen? EWS doesn't have ASan.

> Tools/sanitizer/sanitizer.xcconfig:7
>  WK_SANITIZER_OTHER_CFLAGS_YES = -fno-omit-frame-pointer -g;
> +WK_SANITIZER_OTHER_CPLUSPLUSFLAGS_YES = ;

Does "-fno-omit-frame-pointer -g" get added for C++ in some other way, or do we not need it?

[David Kilzer (:ddkilzer)]

Comment on attachment 409239 [details]
Patch v1

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

>> Tools/sanitizer/asan.xcconfig:9
>> +WK_SANITIZER_OTHER_CPLUSPLUSFLAGS_YES = $(inherited) -U_LIBCPP_HAS_NO_ASAN;
> 
> Nice. Did you run tests locally to confirm that nothing terrible was going to happen? EWS doesn't have ASan.

Yes.  No new ASan-related crashes in any std C++ objects.

>> Tools/sanitizer/sanitizer.xcconfig:7
>> +WK_SANITIZER_OTHER_CPLUSPLUSFLAGS_YES = ;
> 
> Does "-fno-omit-frame-pointer -g" get added for C++ in some other way, or do we not need it?

As noted in the ChangeLog, OTHER_CFLAGS applies to C++ sources, so it doesn't need to be listed twice.  I verified this by reviewing build log output.

[Alexey Proskuryakov]

> As noted in the ChangeLog, OTHER_CFLAGS applies to C++ sources, so it doesn't need to be listed twice.

My understanding was that OTHER_CFLAGS was the default value for OTHER_CPLUSPLUSFLAGS, but it had no effect when OTHER_CPLUSPLUSFLAGS was defined.

> I verified this by reviewing build log output.

Surprising.

[Radar WebKit Bug Importer]

<rdar://problem/69319755>

[EWS]

Committed r267358: <https://trac.webkit.org/changeset/267358>

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