[webkit-reviews] review granted: [Bug 230100] Fix ASan+UBSan builds: part two : [Attachment 437749] Patch v1

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted David Kilzer (:ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 230100: Fix ASan+UBSan builds: part two
https://bugs.webkit.org/show_bug.cgi?id=230100

Attachment 437749: Patch v1

https://bugs.webkit.org/attachment.cgi?id=437749&action=review




--- Comment #3 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 437749
  --> https://bugs.webkit.org/attachment.cgi?id=437749
Patch v1

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

> Tools/ChangeLog:16
> +	     That mechanism doesn't work at the time when Xcode resolves
> +	     variables from an xcconfig file specified on the command-line.

This is surprising, why would those be different? It seems like it cannot be by
accident, someone would have to explicitly implement this behavior.

> Tools/sanitizer/sanitizer.xcconfig:7
> +OTHER_CFLAGS = $(inherited) -fno-omit-frame-pointer -g
$(WK_UNDEFINED_BEHAVIOR_SANITIZER_OTHER_CFLAGS_$(WK_ENABLE_UNDEFINED_BEHAVIOR_S
ANITIZER));
> +OTHER_CPLUSPLUSFLAGS = $(inherited)
$(WK_ADDRESS_SANITIZER_OTHER_CPLUSPLUSFLAGS_$(WK_ENABLE_ADDRESS_SANITIZER));

I think that OTHER_CFLAGS apply to C++ files when OTHER_CPLUSPLUSFLAGS isn't
defined. But it is defined, so we need UBSan flags in the second line too.

> Tools/sanitizer/tsan.xcconfig:7
> +WK_ENABLE_THREAD_SANITIZER = $(ENABLE_THREAD_SANITIZER);

This looks like dead code, not used anywhere.