Bug 159744 - YARR uses mixture of int and unsigned values to index into subject string
Summary: YARR uses mixture of int and unsigned values to index into subject string
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar
: 107898 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-07-13 15:39 PDT by Michael Saboff
Modified: 2022-02-11 14:13 PST (History)
5 users (show)

See Also:


Attachments
Patch (42.40 KB, patch)
2016-07-13 16:52 PDT, Michael Saboff
benjamin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2016-07-13 15:39:53 PDT
Both the YARR interpreter and the JIT use a mixture of unsigned and int offsets when referencing or generating code to reference subject strings.  Usually this works out due to 2's complement math, however there have been bugs reported where we underflow / overflow counts, reference out of bounds memory, or trying to generate code to do the same.

Instead we should make the YARR code "unsigned" clean for all references to subject strings.

This bug is in response to two radars:
    <rdar://problem/27084358> ASSERTION FAILED: (&term - term.atom.parenthesesWidth)->inputPosition == term.inputPosition
    <rdar://problem/27171689> REGRESSION (r197869): CrashOnOverflow in JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::generateCharacterClassFixed
The first is an issue in the YARR interpreter and the second in the YARR JIT.
Comment 1 Michael Saboff 2016-07-13 16:52:06 PDT
Created attachment 283587 [details]
Patch
Comment 2 Benjamin Poulain 2016-07-13 17:19:00 PDT
Comment on attachment 283587 [details]
Patch

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

rs=me. Let's hope we our test coverage will catch any missing case.

> Source/JavaScriptCore/yarr/YarrJIT.cpp:2660
> +        , m_checkedOffset(0)

You don't need to initialized Checked.
If you want to do it anyway to make the initial value explicit, it would be better to have in at declaration with the C++11 syntax.
Comment 3 Michael Saboff 2016-07-13 18:21:00 PDT
Committed r203206: <http://trac.webkit.org/changeset/203206>
Comment 4 Brent Fulgham 2022-02-11 14:13:20 PST
*** Bug 107898 has been marked as a duplicate of this bug. ***