[webkit-reviews] review denied: [Bug 176874] AddressSanitizer: stack-buffer-underflow in JSC::Probe::Page::Page : [Attachment 320715] proposed patch.

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied	review:
Bug 176874: AddressSanitizer: stack-buffer-underflow in JSC::Probe::Page::Page
https://bugs.webkit.org/show_bug.cgi?id=176874

Attachment 320715: proposed patch.

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




--- Comment #6 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 320715
  --> https://bugs.webkit.org/attachment.cgi?id=320715
proposed patch.

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

r-, as this doesn't fix ASanified builds.

> Source/JavaScriptCore/ChangeLog:16
> +	   I've also added the SUPPRESS_ASAN attribute to tell ASan to ignore
this
> +	   constructor function.  Unfortunately, Clang does not seem to honor
that attribute
> +	   (see <rdar://problem/34422508>).  Regardless, this fix is correct
and should be
> +	   landed.

The attribute only applies to the function it's on, and the violation occurs in
a function called by it. It is expected behavior that SUPPRESS_ASAN doesn't
help here, and we shouldn't be adding it.

We've seen this before, the solution was to avoid using memcpy on poisoned
memory (see for example copyMemory in MachineStackMarker.cpp).