Bug 146297 - Address Sanitizer does not play well with memcpy in JSC::MachineThreads::tryCopyOtherThreadStack.
Summary: Address Sanitizer does not play well with memcpy in JSC::MachineThreads::tryC...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-06-24 16:48 PDT by Keith Miller
Modified: 2017-09-14 10:48 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.71 KB, patch)
2015-06-24 17:04 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (3.93 KB, patch)
2015-06-25 10:30 PDT, Keith Miller
msaboff: review+
Details | Formatted Diff | Diff
Patch (3.94 KB, patch)
2015-06-25 11:31 PDT, Keith Miller
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2015-06-24 16:48:10 PDT
WebViews inside apps built with Address sanitizer will sometimes crash in JSC::MachineThreads::tryCopyOtherThreadStack.
Comment 1 Keith Miller 2015-06-24 17:04:58 PDT
Created attachment 255530 [details]
Patch
Comment 2 Alexey Proskuryakov 2015-06-24 22:07:51 PDT
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.
Comment 3 Keith Miller 2015-06-25 10:30:50 PDT
Created attachment 255563 [details]
Patch

Updated comments do note that blacklisting was related to ASan.
Comment 4 Keith Miller 2015-06-25 10:50:04 PDT
rdar://problem/21330174
Comment 5 Michael Saboff 2015-06-25 11:00:00 PDT
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.
Comment 6 Keith Miller 2015-06-25 11:31:02 PDT
Created attachment 255565 [details]
Patch

Removed the declaration for copyMemory changed the patch name to be more descriptive.
Comment 7 WebKit Commit Bot 2015-06-25 12:23:13 PDT
Comment on attachment 255565 [details]
Patch

Clearing flags on attachment: 255565

Committed r185957: <http://trac.webkit.org/changeset/185957>
Comment 8 WebKit Commit Bot 2015-06-25 12:23:17 PDT
All reviewed patches have been landed.  Closing bug.