Treat negative utf8-units the same way as other broken encodings.

Treat negative utf8-units the same way as other broken encodings.

R=lrn@google.com

Committed: https://code.google.com/p/dart/source/detail?r=26528

[Lasse Reichstein Nielsen, 2013-08-22 06:10:42]

lgtm

https://codereview.chromium.org/22929022/diff/1/sdk/lib/convert/utf.dart
File sdk/lib/convert/utf.dart (right):

https://codereview.chromium.org/22929022/diff/1/sdk/lib/convert/utf.dart#newc...
sdk/lib/convert/utf.dart:458: if (unit < 0) {
Would it be cheaper/faster to do:
  if ((unit & ~0x7f) == 0) {
    // one-byte
  } else if ((unit & ~0xdf) == 0) {
    // two-byte
  } else if ((unit & ~0xef) == 0) {
    // three-byte
  } else if ((unit & ~0xf7) == 0) {
    // four-byte
  } else {
    // malformed
  }

This puts the most common case first, instead of wasting time checking for
malformedness first. Each check should be a single bit-and + branch-if-not-zero
in optimized smi code, so it's hard to make cheaper. Negative numbers are tested
the same way as any other numbers - they have bits set outside the first octet.

https://codereview.chromium.org/22929022/diff/1/sdk/lib/convert/utf.dart#newc...
sdk/lib/convert/utf.dart:459: // TODO(floitsch): should this be unit <= 0 ?
No, zero is a valid UTF-8 code unit encoding the U+0000 code point.

https://codereview.chromium.org/22929022/diff/1/tests/lib/convert/utf82_test....
File tests/lib/convert/utf82_test.dart (right):

https://codereview.chromium.org/22929022/diff/1/tests/lib/convert/utf82_test....
tests/lib/convert/utf82_test.dart:56: [ -0xFF ]];
Try also:
-0x80000000  (min int32)
-0x40000000  (min smi)
-0x80000000000000000 (some bigint)

[floitsch, 2013-08-22 15:40:25]

I think we also allow values that are too large (if they satisfy the
bit-pattern).
We need to revisit this loop at some point...

https://codereview.chromium.org/22929022/diff/1/sdk/lib/convert/utf.dart
File sdk/lib/convert/utf.dart (right):

https://codereview.chromium.org/22929022/diff/1/sdk/lib/convert/utf.dart#newc...
sdk/lib/convert/utf.dart:458: if (unit < 0) {
On 2013/08/22 06:10:42, Lasse Reichstein Nielsen wrote:
> Would it be cheaper/faster to do:
>   if ((unit & ~0x7f) == 0) {
>     // one-byte
>   } else if ((unit & ~0xdf) == 0) {
>     // two-byte
>   } else if ((unit & ~0xef) == 0) {
>     // three-byte
>   } else if ((unit & ~0xf7) == 0) {
>     // four-byte
>   } else {
>     // malformed
>   }
> 
> This puts the most common case first, instead of wasting time checking for
> malformedness first. Each check should be a single bit-and +
branch-if-not-zero
> in optimized smi code, so it's hard to make cheaper. Negative numbers are
tested
> the same way as any other numbers - they have bits set outside the first
octet.

Rapidly tried it, but somehow introduced a bug. Now added a TODO.

https://codereview.chromium.org/22929022/diff/1/sdk/lib/convert/utf.dart#newc...
sdk/lib/convert/utf.dart:459: // TODO(floitsch): should this be unit <= 0 ?
On 2013/08/22 06:10:42, Lasse Reichstein Nielsen wrote:
> No, zero is a valid UTF-8 code unit encoding the U+0000 code point.

Done.

https://codereview.chromium.org/22929022/diff/1/tests/lib/convert/utf82_test....
File tests/lib/convert/utf82_test.dart (right):

https://codereview.chromium.org/22929022/diff/1/tests/lib/convert/utf82_test....
tests/lib/convert/utf82_test.dart:56: [ -0xFF ]];
On 2013/08/22 06:10:42, Lasse Reichstein Nielsen wrote:
> Try also:
> -0x80000000  (min int32)
> -0x40000000  (min smi)
> -0x80000000000000000 (some bigint)

Done.

[floitsch, 2013-08-22 15:40:51]

Committed patchset #2 manually as r26528 (presubmit successful).