[webkit-reviews] review granted: [Bug 186765] [Armv7] Linkbuffer: executableOffsetFor() fails for location 2 : [Attachment 342936] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jun 18 10:50:19 PDT 2018
Michael Saboff <msaboff at apple.com> has granted review:
Bug 186765: [Armv7] Linkbuffer: executableOffsetFor() fails for location 2
https://bugs.webkit.org/show_bug.cgi?id=186765
Attachment 342936: Patch
https://bugs.webkit.org/attachment.cgi?id=342936&action=review
--- Comment #7 from Michael Saboff <msaboff at apple.com> ---
Comment on attachment 342936
--> https://bugs.webkit.org/attachment.cgi?id=342936
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=342936&action=review
r=me
>> Source/JavaScriptCore/assembler/LinkBuffer.h:300
>> + if (location < sizeof(int32_t))
>
> Why is it ever valid to pass a location of 2 and expect it to map to an
offset of 0? It looks to me like this change is trying to mask the bug instead
of fixing it. Please provide justification as why this is valid, or if this
isn't the real issue, fix the real issue instead.
I'm fine with this change. Since the buffer records the offsets prior to the
current location and the offsets are stored as int32_t values, this works. You
might want to add a comment that no compaction can happen before this point.
More information about the webkit-reviews
mailing list