Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(490)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1197263002: Fix crash in modPow (issue 23693); various bigint knowledge missing. (Closed)

Created:
5 years, 6 months ago by srdjan
Modified:
5 years, 6 months ago
Reviewers:
regis, Ivan Posva
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Base URL:
https://github.com/dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Fix crash in modPow (issue 23693); various bigint knowledge missing. BUG= R=regis@google.com Committed: https://github.com/dart-lang/sdk/commit/e91b99b66d7cce90ec0784e01128834d861b9d82

Patch Set 1 #

Patch Set 2 : v #

Total comments: 2

Patch Set 3 : c #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -5 lines) Patch
M runtime/vm/constant_propagator.cc View 1 2 1 chunk +5 lines, -1 line 0 comments Download
M runtime/vm/intermediate_language.cc View 1 chunk +1 line, -1 line 0 comments Download
M tests/corelib/corelib.status View 1 chunk +0 lines, -3 lines 2 comments Download

Messages

Total messages: 8 (2 generated)
srdjan
5 years, 6 months ago (2015-06-22 21:53:47 UTC) #2
regis
lgtm https://codereview.chromium.org/1197263002/diff/20001/runtime/vm/constant_propagator.cc File runtime/vm/constant_propagator.cc (right): https://codereview.chromium.org/1197263002/diff/20001/runtime/vm/constant_propagator.cc#newcode520 runtime/vm/constant_propagator.cc:520: if ((left.IsInteger() && right.IsInteger()) && Parenthesis around (left.IsInteger() ...
5 years, 6 months ago (2015-06-22 21:57:18 UTC) #3
srdjan
https://codereview.chromium.org/1197263002/diff/20001/runtime/vm/constant_propagator.cc File runtime/vm/constant_propagator.cc (right): https://codereview.chromium.org/1197263002/diff/20001/runtime/vm/constant_propagator.cc#newcode520 runtime/vm/constant_propagator.cc:520: if ((left.IsInteger() && right.IsInteger()) && On 2015/06/22 21:57:18, regis ...
5 years, 6 months ago (2015-06-22 22:00:41 UTC) #4
srdjan
Committed patchset #3 (id:40001) manually as e91b99b66d7cce90ec0784e01128834d861b9d82 (presubmit successful).
5 years, 6 months ago (2015-06-22 22:12:46 UTC) #5
Ivan Posva
Thanks! -Ivan https://codereview.chromium.org/1197263002/diff/40001/tests/corelib/corelib.status File tests/corelib/corelib.status (left): https://codereview.chromium.org/1197263002/diff/40001/tests/corelib/corelib.status#oldcode209 tests/corelib/corelib.status:209: big_integer_arith_vm_test/gcd: Pass, Crash # Issue 23693 Just ...
5 years, 6 months ago (2015-06-23 00:48:27 UTC) #7
srdjan
5 years, 6 months ago (2015-06-23 04:24:46 UTC) #8
Message was sent while issue was closed. 
https://codereview.chromium.org/1197263002/diff/40001/tests/corelib/corelib.s...
File tests/corelib/corelib.status (left):

https://codereview.chromium.org/1197263002/diff/40001/tests/corelib/corelib.s...
tests/corelib/corelib.status:209: big_integer_arith_vm_test/gcd: Pass, Crash #
Issue 23693
On 2015/06/23 00:48:26, Ivan Posva wrote:
> Just confirming that gcd and modInv are not fixed by this CL, correct?

Unfortunately, they are not fixed by that. It seems an issue in the range
analysis.

Powered by Google App Engine
This is Rietveld 408576698