Bug 165651

Summary: REGRESSION(r208791): Assertion in testb3
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: JavaScriptCoreAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Critical CC: cgarcia, commit-queue, fpizlo, gyuyoung.kim, keith_miller, mark.lam, msaboff, ossy, saam, ysuzuki
Priority: P1    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch saam: review+

Csaba Osztrogonác
Reported 2016-12-09 03:34:07 PST
ASSERTION FAILED: compileAndRun<uint32_t>(proc, a, b) == a / b (compileAndRun<uint32_t>(proc, a, b) == 1, a / b == 0) ../../Source/JavaScriptCore/b3/testb3.cpp(1775) : void {anonymous}::testUDivArgsInt32(uint32_t, uint32_t) I don't know if it is a regression or not, I haven't run testb3 in the last months. I don't know if it is Linux specific or not, I don't have any chance to test on non Linux environment.
Attachments
Patch (1.53 KB, patch)
2016-12-10 01:02 PST, Yusuke Suzuki 		saam: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1 2016-12-09 03:40:59 PST
ASSERTION FAILED: compileAndRun<uint64_t>(proc, a, b) == a / b (compileAndRun<uint64_t>(proc, a, b) == 1, a / b == 0) ../../Source/JavaScriptCore/b3/testb3.cpp(1791) : void {anonymous}::testUDivArgsInt64(uint64_t, uint64_t)
Csaba Osztrogonác
Comment 2 2016-12-09 04:00:31 PST
Just a note, this test was added by https://trac.webkit.org/changeset/208781
Csaba Osztrogonác
Comment 3 2016-12-09 04:01:58 PST
(In reply to comment #2) > Just a note, this test was added by https://trac.webkit.org/changeset/208781 It must be a regression, because it passed when it was added.
Csaba Osztrogonác
Comment 4 2016-12-09 04:47:25 PST
I bisected, it is definitely a regression caused by https://trac.webkit.org/changeset/208791 .
Keith Miller
Comment 5 2016-12-09 09:01:49 PST
Hmm, interesting. I definitely ran testb3 before landing maybe I messed up while rebasing or something.
Yusuke Suzuki
Comment 6 2016-12-10 00:54:36 PST
OK, I've found the issue.
Yusuke Suzuki
Comment 7 2016-12-10 01:02:22 PST
Created attachment 296789 [details] Patch
Saam Barati
Comment 8 2016-12-10 01:22:57 PST
Comment on attachment 296789 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296789&action=review > Source/JavaScriptCore/ChangeLog:9 > + Accidentally we always use edx/rdx for the result of UDiv/UMod. > + But it is incorrect. We should use eax/rax for the result of UDiv. Is there documentation on this?
Yusuke Suzuki
Comment 9 2016-12-10 01:28:16 PST
Comment on attachment 296789 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296789&action=review >> Source/JavaScriptCore/ChangeLog:9 >> + But it is incorrect. We should use eax/rax for the result of UDiv. > > Is there documentation on this? It's x86's div convension. It should be handled in B3/Air, so that the caller side should not consider about it.
Yusuke Suzuki
Comment 10 2016-12-10 01:29:42 PST
Committed r209658: <http://trac.webkit.org/changeset/209658>
Note You need to log in before you can comment on or make changes to this bug.