Fix length computation in Dart_StringToUTF8.

bin/builtin_natives.cc

Index: bin/builtin_natives.cc
===================================================================
--- bin/builtin_natives.cc	(revision 14418)
+++ bin/builtin_natives.cc	(working copy)
@@ -103,9 +103,17 @@
 // test/debug functionality in standalone dart mode.
 
 void Builtin::PrintString(FILE* out, Dart_Handle str) {
-  const char* chars = NULL;
-
-  Dart_Handle result = Dart_StringToCString(str, &chars);
+  intptr_t length = 0;
+  Dart_Handle result = Dart_StringLength(str, &length);
+  DART_CHECK_VALID(result);
+  if (Dart_IsAsciiString(str)) {
+    length = (length * sizeof(uint8_t));
+  } else {
+    length = (length * sizeof(uint16_t));

[Bill Hesse, 2012/11/01 20:38:01]

This is not enough.  Some 16-bit code points are encoded in 3 bytes in UTF8.

I am assuming that StringLength is reporting a surrogate pair as two characters,
and the UTF8 encoder is actually encoding CESU-8:

Main article: CESU-8
Many programs added UTF-8 conversions for UCS-2 data and did not alter this
UTF-8 conversion when UCS-2 was replaced with the surrogate-pair using UTF-16.
In such programs each half of a UTF-16 surrogate pair is encoded as its own
3-byte UTF-8 encoding, resulting in 6-byte sequences rather than 4 bytes for
characters outside the Basic Multilingual Plane. Oracle and MySQL databases use
this, as well as Java and Tcl as described below, and probably many Windows
programs where the programmers were unaware of the complexities of UTF-16.
Although this non-optimal encoding is generally not deliberate, a supposed
benefit is that it preserves UTF-16 binary sorting order when CESU-8 is binary
sorted.

[cshapiro, 2012/11/02 17:01:56]

We should be emitting UTF-8, not CESU-8.

However, the UTF-8 encoder is still assuming that supplementary characters will
appear as a single element in the input stream.  That is no longer true.

I will open a separate bug.

+  }
+  uint8_t* chars = reinterpret_cast<uint8_t*>(malloc(length));
+  ASSERT(chars != NULL);
+  result = Dart_StringToUTF8(str, chars, &length);

[Bill Hesse, 2012/11/01 20:38:01]

I don't see what the problem is with using Dart_StringToCString is here - it
does exactly what we need, and the lifetime of the C string is long enough for
it to be printed - the fwrite doesn't need the buffer after it returns.  It
seems to do exactly the needed job, since PrintString is only called from
Logger_PrintString, which contains its EnterScope and ExitScope.

[siva, 2012/11/01 21:23:03]

Because that would mean we are assuming that the string
object does not have any embedded NUL characters in it.

We do a strlen(chars) on the returned string which might
return a truncated length.

I have changed the API to do a scope allocation of the buffer instead of
requiring the buffer to be allocated by the caller.


On 2012/11/01 20:38:01, Bill Hesse wrote:

   if (Dart_IsError(result)) {
     // TODO(turnidge): Consider propagating some errors here.  What if
     // an isolate gets interrupted by the embedder in the middle of
@@ -113,11 +121,11 @@
     // interrupt.
     fputs(Dart_GetError(result), out);
   } else {
-    intptr_t length = strlen(chars);
     fwrite(chars, sizeof(*chars), length, out);
   }
   fputc('\n', out);
   fflush(out);
+  free(chars);
 }