Bug 146297 – Address Sanitizer does not play well with memcpy in JSC::MachineThreads::tryCopyOtherThreadStack.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Keith Miller]

WebViews inside apps built with Address sanitizer will sometimes crash in JSC::MachineThreads::tryCopyOtherThreadStack.

[Keith Miller]

Created attachment 255530 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 255530 [details]
Patch

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

> Source/JavaScriptCore/heap/MachineStackMarker.cpp:543
> +// Since we cannot blacklist the system memcpy we must use our own naive implementation,

There is no context here to understand that blackisting is about ASan.  Please update the comment to make it clear that this is needed to use ASan with both instrumented and non-instrumented builds.

[Keith Miller]

Created attachment 255563 [details]
Patch

Updated comments do note that blacklisting was related to ASan.

[Keith Miller]

rdar://problem/21330174

[Michael Saboff]

Comment on attachment 255563 [details]
Patch

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

r=me with a couple of comments.

> Source/JavaScriptCore/ChangeLog:3
> +        Address Sanitizer finds a problem in JavaScriptCore.

Change the bug title to reflect the issue.

> Source/JavaScriptCore/heap/MachineStackMarker.cpp:522
> +void copyMemory(void*, const void*, size_t);

Don't think this declaration serves much purpose with the definition below.

[Keith Miller]

Created attachment 255565 [details]
Patch

Removed the declaration for copyMemory changed the patch name to be more descriptive.

[WebKit Commit Bot]

Comment on attachment 255565 [details]
Patch

Clearing flags on attachment: 255565

Committed r185957: <http://trac.webkit.org/changeset/185957>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.