In case of ARM_TRADITIONAL, crash occurs like below when loading http://v.youku.com/v_playlist/f16850225o1p0.html #0 0xfffffffe in ?? () No symbol table info available. #1 0x419d160c in execute (jitObject=<value optimized out>, input=<value optimized out>, start=<value optimized out>, length=<value optimized out>, output=0x0) at /home/SOURCE/webkit-efl/Source/JavaScriptCore/yarr/YarrJIT.h:74 No locals. #2 JSC::Yarr::execute (jitObject=<value optimized out>, input=<value optimized out>, start=<value optimized out>, length=<value optimized out>, output=0x0) at /home/SOURCEwebkit-efl/Source/JavaScriptCore/yarr/YarrJIT.cpp:2505 The problem is caused by the instruction which loads the invalid branch target(0xffffffff). That instruction is append to the assembler buffer while generating greedy pattern character about a 16 bit pattern character and an 8 bit string. Updating the branch target for that instruction is necessary but it's not in any where.
Created attachment 121366 [details] Patch
Created attachment 121368 [details] Patch
Comment on attachment 121368 [details] Patch Looks right to me. Good catch!
Comment on attachment 121368 [details] Patch Clearing flags on attachment: 121368 Committed r108456: <http://trac.webkit.org/changeset/108456>
All reviewed patches have been landed. Closing bug.
(In reply to comment #5) > All reviewed patches have been landed. Closing bug. Looks like this broke fast/regex/pcre-test-4.html : --- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/fast/regex/pcre-test-4-expected.txt +++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/fast/regex/pcre-test-4-actual.txt @@ -70,11 +70,11 @@ PASS regex34.exec(input0); is results PASS regex35.exec(input0); is results PASS regex36.exec(input0); is results -PASS regex37.exec(input0); is results +FAIL regex37.exec(input0); should be . Was null. PASS regex37.exec(input1); is results PASS regex38.exec(input0); is results PASS regex38.exec(input1); is results -PASS regex39.exec(input0); is results +FAIL regex39.exec(input0); should be . Was null. PASS regex39.exec(input1); is results PASS regex40.exec(input0); is results PASS regex41.exec(input0); is results @@ -215,7 +215,7 @@ PASS regex104.exec(input0); is results PASS regex105.exec(input0); is results PASS regex105.exec(input1); is results -PASS regex106.exec(input0); is results +FAIL regex106.exec(input0); should be a. Was null. PASS regex107.exec(input0); is results PASS input0.match(regexGlobal0); is results PASS input1.match(regexGlobal0); is results
Reopen, because it fails on SL and Qt too.
(In reply to comment #7) > Reopen, because it fails on SL and Qt too. That test didn't even run in my environment, arm & linux, before the patch was applied. Now I'm finding out the reason why the returned value is null, not string "".
Rollout landed in http://trac.webkit.org/changeset/108484
Created attachment 128382 [details] Patch
(In reply to comment #9) > Rollout landed in http://trac.webkit.org/changeset/108484 I'm really sorry for rollout despite your grateful review and comments. In the previous patch, I had focused on the short circuit necessary to be linked. But there's one thing missed is that short circuit is not necessary for the pattern character greedy although it's helpful for non-greedy. I ran regular expression tests. Broken results are fine with new patch.
Created attachment 128449 [details] Patch
Comment on attachment 128449 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=128449&action=review r+ > Source/JavaScriptCore/yarr/YarrJIT.cpp:819 > + if (!((ch > 0xff) && (m_charSize == Char8))) { Please add a comment similar to what was removed. > Source/JavaScriptCore/yarr/YarrJIT.cpp:879 > + JumpList nonGreedyFailures; Add a comment here as well.
Created attachment 128617 [details] Patch
(In reply to comment #13) > (From update of attachment 128449 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=128449&action=review > > r+ > > > Source/JavaScriptCore/yarr/YarrJIT.cpp:819 > > + if (!((ch > 0xff) && (m_charSize == Char8))) { > > Please add a comment similar to what was removed. > > > Source/JavaScriptCore/yarr/YarrJIT.cpp:879 > > + JumpList nonGreedyFailures; > > Add a comment here as well. Thank you for your review. I added comments, and uploaded new patch that has more comments on what you reviewed. Didn't I need to upload the patch again?
Comment on attachment 128617 [details] Patch Clearing flags on attachment: 128617 Committed r108753: <http://trac.webkit.org/changeset/108753>