DART STANDALONE VM CRASH – 2.0.0-dev.33 #32465
Assignees
Labels
Milestone
Comments
kevmoo
added
area-vm
|
Repo seems to be missing a step? |
|
@rmacnak-google – is this running on your local machine? I need to give you secret magic to get things going. Will follow-up offline... |
|
With symbols: |
|
Here's a repro that doesn't require complex config https://travis-ci.org/dart-lang/googleapis_auth/jobs/350953311#L525 |
|
@crelier could this be related to the recent bigint intrinsification change? |
|
@crelier is on a flight today can somebody else please take a look.... |
|
I'll take a look. |
|
Reverting the bigint intrinsics removes the crash: https://dart-review.googlesource.com/c/sdk/+/45800 |
|
Ok we can go with reverting it for now and assign the bug to @crelier to take a look at it when he is back online. |
|
It should be easy to comment out the _BigIntImpl and _Montgomery intrinsics
in method_recognizer.h for now.
If this code runs on ia32, but not x64, I suspect that a digit array is too
short to be processed accessing digits pair-wise.
If disabling intrinsics is not sufficient, I'll look at it from my vacation
in Switzerland.
Sorry for the trouble and thank you Zach for jumping in!
-- Regis (waiting to board a plane in Billund)
…On Fri, Mar 9, 2018, 09:23 Siva ***@***.***> wrote:
Ok we can go with reverting it for now and assign the bug to @crelier
<https://github.com/crelier> to take a look at it when he is back online.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#32465 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AIOIO-Q-7PWTQGxDNSz4hIq2a_kNwUSqks5tcjvvgaJpZM4SjeVM>
.
|
dart-bot
pushed a commit
that referenced
this issue
Mar 9, 2018
… implementation."" This reverts commit be62fff. Reason for revert: See #32465 Original change's description: > Reland "[VM runtime] Switch intrinsics from old to new Bigint implementation." > > The change was reverted (thanks Slava) because gen_snapshot was failing to > find the core lib class _BigIntImpl on flutter. > The PR flutter/engine#4735 fixed that issue on flutter. > > Change-Id: Id6863d79f7fdb17f0076b63b0168394d5e95ec8d > Reviewed-on: https://dart-review.googlesource.com/44660 > Reviewed-by: Alexander Markov <alexmarkov@google.com> TBR=alexmarkov@google.com,regis@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Change-Id: I412f5143442ca6ad43784b037c01fcf9bb42c348 Reviewed-on: https://dart-review.googlesource.com/45800 Reviewed-by: Zach Anderson <zra@google.com> Reviewed-by: Siva Annamalai <asiva@google.com> Commit-Queue: Zach Anderson <zra@google.com>
|
One thing I noticed: Old version did: var r_used = _used + ds + 1;
var r_digits = new Uint32List(r_used + 2 - (r_used & 1)); // for 64-bit.New version does: /// Allocate a new digits list of even length.
Uint32List _newDigits(int length) => new Uint32List(length + (length & 1));
var resultUsed = _used + digitShift + 1;
var resultDigits = _newDigits(resultUsed);Note that |
|
Okay I read the code a little bit more and I see the bug (might be mistaken - because I did not verify it by debugging).
Now if we look at how _lsh(_digits, _used, shiftAmount, resultDigits);If we do __ movq(RDI, Address(RSP, 4 * kWordSize)); // x_digits
__ movq(R8, Address(RSP, 3 * kWordSize)); // x_used is Smi
__ subq(R8, Immediate(2)); // x_used > 0, Smi. R8 = x_used - 1, round up.
__ sarq(R8, Immediate(2)); // R8 + 1 = number of digit pairs to read.
// ...
Label last;
__ cmpq(R8, Immediate(0));
__ j(EQUAL, &last, Assembler::kNearJump);I assumes that Previously shifting |
|
The revert of the CL causing this issue is included in dev version 2.0.0-dev.35.0. Both dev versions -dev.33 and -dev.34 have the crash noted above in the vm. |
|
With the revert, can we move this out of the " IO Beta 2" milestone? |
|
I've confirmed that this is now fixed. |
dart-bot
pushed a commit
that referenced
this issue
Mar 21, 2018
…ns (fixes #32465). Fix VM, dart2js, and dcc Bigint implementations. Add shift tests. Re-enable Bigint intrinsics on VM. Change-Id: Iec19eac8069cf17783a5346289ea2745ffcc7c26 Reviewed-on: https://dart-review.googlesource.com/46570 Reviewed-by: Florian Loitsch <floitsch@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reproduces on linux (Travis-CI) https://travis-ci.org/firebase/firebase-dart/jobs/351022568#L534
And my mac
Repro steps:
The text was updated successfully, but these errors were encountered: