Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dart2js uses unsigned shift for '>>' operator while VM uses signed #3109

Closed
DartBot opened this issue May 17, 2012 · 6 comments
Closed

dart2js uses unsigned shift for '>>' operator while VM uses signed #3109

DartBot opened this issue May 17, 2012 · 6 comments
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures web-dart2js

Comments

@DartBot
Copy link

DartBot commented May 17, 2012

This issue was originally filed by [email protected]


  Consider the following code:

void main() {
  int negativeOne = -1;
  if (1 + 1 != 2) negativeOne = 999; // defeat constant-folding in the compiler
  print("${negativeOne >> 8}");
}

In the VM, it prints -1, while dart2js prints 16777215. dart2js emits this following code:

$.main = function() {
  var negativeOne = -1;
  $.print('' + $.stringToString($.shr(negativeOne, 8)));
};

$.shr = function(a, b) {
  if ($.checkNumbers(a, b) === true) {
    if ($.ltB(b, 0)) {
      throw $.captureStackTrace($.IllegalArgumentException$1(b));
    }
    if ($.gtB(b, 31)) {
      return 0;
    }
    return a >>> b;
  }
  return a.operator$shr$1(b);
};

The upshot is that dart2js compiles the dart expression '-1 >> 8' to the js expression '-1 >>> 8', which does not match the VM behavior.

This makes it difficult to implement bit operations reliably in dart2js, which is gating other library work.

@iposva-google
Copy link
Contributor

Shift-right (">>") is a signed shift.


Added Triaged label.

@iposva-google
Copy link
Contributor

cc @floitschG.

@rakudrama
Copy link
Member

See Issue #2725
JavaScript supports only 32 bit values for bitwise and shift operations.
There will always be some discrepancy between the VM and JavaScript so long as JavaScript native numbers are used.
Two possible policies:
(1) all results are in the range [-2^31, 2^31)
(2) all results are in the range [0, 2^32)

I believe dart2js is consciously implementing (2) because that makes crypto libraries work.
frog mostly implements (1).

What we need is a clear description of how (and why) dart2js is dealing with these issues so we can quickly triage this endless list of edge cases as examples of the underlying problem.

@floitschG
Copy link
Contributor

Stephen is right that we always return something in the range [0, 2^32], but the behavior described in this issue is a bug.
The idea was to transform "x bitOp y" into (in Dart semantics) (x bitOp y) & 0x32bit
I wrongly assumed that (x >> y) & 32bit would be equivalent to JS "x >>> y". I'm going to fix this on Monday.


Added Accepted label.

@kasperl
Copy link

kasperl commented May 24, 2012

Was this fixed in r7899?

@floitschG
Copy link
Contributor

Yes. fixed in r7899.


Added Fixed label.

@DartBot DartBot added Type-Defect P1 A high priority bug; for example, a single project is unusable or has many test failures web-dart2js labels May 24, 2012
copybara-service bot pushed a commit that referenced this issue Aug 12, 2022
…3 revisions)

https://dart.googlesource.com/dartdoc/+log/ed09195c559c..446e0fd1f26b

2022-08-12 [email protected] Tidy potentially applicable extensions (#3117)
2022-08-12 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.1.17 to 2.1.18 (#3109)
2022-08-12 [email protected] Accept theirs in merge conflicts of generated files (#3118)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/dart-doc-dart-sdk
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Dart Documentation Generator: https://github.com/dart-lang/dartdoc/issues
To file a bug in Dart SDK: https://github.com/dart-lang/sdk/issues

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Tbr: [email protected]
Change-Id: I5ba63e0e8eaa9a2e400d5777f359f164ffb5b184
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255002
Reviewed-by: Devon Carew <[email protected]>
Commit-Queue: DEPS Autoroller <[email protected]>
Commit-Queue: Devon Carew <[email protected]>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures web-dart2js
Projects
None yet
Development

No branches or pull requests

5 participants