Skip to content

Commit

Permalink
[vm] Ensure 'print' is atomic between printing the string and '\n'
Browse files Browse the repository at this point in the history
Should fix #53471

TEST=ci

Change-Id: I76da5776bb95f8297ef63aa4c8754c940e1d4e71
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330999
Commit-Queue: Siva Annamalai <asiva@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
  • Loading branch information
a-siva authored and Commit Queue committed Nov 3, 2023
1 parent 258863c commit 1d3d819
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 34 deletions.
17 changes: 4 additions & 13 deletions pkg/vm_service/test/capture_stdio_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,14 @@ var tests = <IsolateTest>[
hasStoppedAtBreakpoint,
(VmService service, IsolateRef isolateRef) async {
final completer = Completer<void>();
int eventNumber = 1;
late StreamSubscription stdoutSub;
stdoutSub = service.onStdoutEvent.listen((event) async {
expect(event.kind, EventKind.kWriteEvent);
final decoded = utf8.decode(base64Decode(event.bytes!));

if (eventNumber == 1) {
expect(decoded, 'print');
} else if (eventNumber == 2) {
expect(decoded, '\n');
await service.streamCancel(EventStreams.kStdout);
await stdoutSub.cancel();
completer.complete();
} else {
fail('Unreachable');
}
eventNumber++;
expect(decoded, 'print');
await service.streamCancel(EventStreams.kStdout);
await stdoutSub.cancel();
completer.complete();
});
await service.streamListen(EventStreams.kStdout);
await service.resume(isolateRef.id!);
Expand Down
19 changes: 10 additions & 9 deletions runtime/bin/builtin_natives.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,27 +84,28 @@ const uint8_t* Builtin::NativeSymbol(Dart_NativeFunction nf) {
// test/debug functionality in standalone dart mode.
void FUNCTION_NAME(Builtin_PrintString)(Dart_NativeArguments args) {
intptr_t length = 0;
uint8_t* chars = nullptr;
Dart_Handle str = Dart_GetNativeArgument(args, 0);
Dart_Handle result = Dart_StringToUTF8(str, &chars, &length);
Dart_Handle result = Dart_StringUTF8Length(str, &length);
if (Dart_IsError(result)) {
Dart_PropagateError(result);
}
uint8_t* chars = Dart_ScopeAllocate(length + 1);
ASSERT(chars != nullptr);
result = Dart_CopyUTF8EncodingOfString(str, chars, length);
if (Dart_IsError(result)) {
Dart_PropagateError(result);
}
chars[length] = '\n';

// Uses fwrite to support printing NUL bytes.
intptr_t res = fwrite(chars, 1, length, stdout);
ASSERT(res == length);
fputs("\n", stdout);
intptr_t res = fwrite(chars, 1, length + 1, stdout);
ASSERT(res == (length + 1));
fflush(stdout);
if (ShouldCaptureStdout()) {
// For now we report print output on the Stdout stream.
uint8_t newline[] = {'\n'};
const char* res =
Dart_ServiceSendDataEvent("Stdout", "WriteEvent", chars, length);
ASSERT(res == nullptr);
res = Dart_ServiceSendDataEvent("Stdout", "WriteEvent", newline,
sizeof(newline));
ASSERT(res == nullptr);
}
}

Expand Down
29 changes: 29 additions & 0 deletions runtime/include/dart_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -2241,6 +2241,17 @@ DART_EXPORT Dart_Handle Dart_BooleanValue(Dart_Handle boolean_obj, bool* value);
*/
DART_EXPORT Dart_Handle Dart_StringLength(Dart_Handle str, intptr_t* length);

/**
* Gets the length of UTF-8 encoded representation for a string.
*
* \param str A String.
* \param length Returns the length of UTF-8 encoded representation for string.
*
* \return A valid handle if no error occurs during the operation.
*/
DART_EXPORT Dart_Handle Dart_StringUTF8Length(Dart_Handle str,
intptr_t* length);

/**
* Returns a String built from the provided C string
* (There is an implicit assumption that the C string passed in contains
Expand Down Expand Up @@ -2368,6 +2379,24 @@ DART_EXPORT Dart_Handle Dart_StringToUTF8(Dart_Handle str,
uint8_t** utf8_array,
intptr_t* length);

/**
* Copies the UTF-8 encoded representation of a String into specified buffer.
*
* Any unpaired surrogate code points in the string will be converted as
* replacement characters (U+FFFD, 0xEF 0xBF 0xBD in UTF-8).
*
* \param str A string.
* \param utf8_array Buffer into which the UTF-8 encoded representation of
* the string is copied into.
* The buffer is allocated and managed by the caller.
* \param length Specifies the length of the buffer passed in.
*
* \return A valid handle if no error occurs during the operation.
*/
DART_EXPORT Dart_Handle Dart_CopyUTF8EncodingOfString(Dart_Handle str,
uint8_t* utf8_array,
intptr_t length);

/**
* Gets the data corresponding to the string object. This function returns
* the data only for Latin-1 (ISO-8859-1) string objects. For all other
Expand Down
16 changes: 4 additions & 12 deletions runtime/observatory/tests/service/capture_stdio_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,14 @@ var tests = <IsolateTest>[
(Isolate isolate) async {
Completer completer = new Completer();
var stdoutSub;
int eventNumber = 1;
stdoutSub = await isolate.vm.listenEventStream(VM.kStdoutStream,
(ServiceEvent event) {
expect(event.isolate != null, isTrue);
expect(event.kind, equals('WriteEvent'));
if (eventNumber == 1) {
expect(event.bytesAsString, equals('print'));
} else if (eventNumber == 2) {
expect(event.bytesAsString, equals('\n'));
stdoutSub.cancel().then((_) {
completer.complete();
});
} else {
expect(true, false);
}
eventNumber++;
expect(event.bytesAsString, equals('print'));
stdoutSub.cancel().then((_) {
completer.complete();
});
});
await isolate.resume();
await completer.future;
Expand Down
2 changes: 2 additions & 0 deletions runtime/tests/vm/dart/exported_symbols_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -324,11 +324,13 @@ main() {
"Dart_StopProfiling",
"Dart_StringGetProperties",
"Dart_StringLength",
"Dart_StringUTF8Length",
"Dart_StringStorageSize",
"Dart_StringToCString",
"Dart_StringToLatin1",
"Dart_StringToUTF16",
"Dart_StringToUTF8",
"Dart_CopyUTF8EncodingOfString",
"Dart_ThreadDisableProfiling",
"Dart_ThreadEnableProfiling",
"Dart_ThrowException",
Expand Down
36 changes: 36 additions & 0 deletions runtime/vm/dart_api_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2918,6 +2918,20 @@ DART_EXPORT Dart_Handle Dart_StringLength(Dart_Handle str, intptr_t* len) {
RETURN_TYPE_ERROR(thread->zone(), str, String);
}

DART_EXPORT Dart_Handle Dart_StringUTF8Length(Dart_Handle str, intptr_t* len) {
Thread* thread = Thread::Current();
DARTSCOPE(thread);
{
ReusableObjectHandleScope reused_obj_handle(thread);
const String& str_obj = Api::UnwrapStringHandle(reused_obj_handle, str);
if (!str_obj.IsNull()) {
*len = Utf8::Length(str_obj);
return Api::Success();
}
}
RETURN_TYPE_ERROR(thread->zone(), str, String);
}

DART_EXPORT Dart_Handle Dart_NewStringFromCString(const char* str) {
DARTSCOPE(Thread::Current());
API_TIMELINE_DURATION(T);
Expand Down Expand Up @@ -3059,6 +3073,28 @@ DART_EXPORT Dart_Handle Dart_StringToUTF8(Dart_Handle str,
return Api::Success();
}

DART_EXPORT Dart_Handle Dart_CopyUTF8EncodingOfString(Dart_Handle str,
uint8_t* utf8_array,
intptr_t length) {
DARTSCOPE(Thread::Current());
API_TIMELINE_DURATION(T);
if (utf8_array == nullptr) {
RETURN_NULL_ERROR(utf8_array);
}
const String& str_obj = Api::UnwrapStringHandle(Z, str);
if (str_obj.IsNull()) {
RETURN_TYPE_ERROR(Z, str, String);
}
intptr_t str_len = Utf8::Length(str_obj);
if (length < str_len) {
return Api::NewError(
"Provided buffer is not large enough to hold "
"the UTF-8 representation of the string");
}
str_obj.ToUTF8(utf8_array, str_len);
return Api::Success();
}

DART_EXPORT Dart_Handle Dart_StringToLatin1(Dart_Handle str,
uint8_t* latin1_array,
intptr_t* length) {
Expand Down
25 changes: 25 additions & 0 deletions runtime/vm/dart_api_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1710,6 +1710,31 @@ TEST_CASE(DartAPI_MalformedStringToUTF8) {
}
}

TEST_CASE(DartAPI_CopyUTF8EncodingOfString) {
const char* kScriptChars =
"String lowSurrogate() {"
" return '\\u{1D11E}'[1];"
"}";

Dart_Handle lib = TestCase::LoadTestScript(kScriptChars, nullptr);
Dart_Handle str1 = Dart_Invoke(lib, NewString("lowSurrogate"), 0, nullptr);
EXPECT_VALID(str1);

uint8_t* utf8_encoded = nullptr;
intptr_t utf8_length = 0;
Dart_Handle result = Dart_StringToUTF8(str1, &utf8_encoded, &utf8_length);
EXPECT_VALID(result);
EXPECT_EQ(3, utf8_length);

intptr_t utf8_copy_length = 0;
result = Dart_StringUTF8Length(str1, &utf8_copy_length);
uint8_t* utf8_encoded_copy = Dart_ScopeAllocate(utf8_copy_length);
result =
Dart_CopyUTF8EncodingOfString(str1, utf8_encoded_copy, utf8_copy_length);
EXPECT_VALID(result);
EXPECT_EQ(0, memcmp(utf8_encoded, utf8_encoded_copy, utf8_length));
}

static void ExternalStringCallbackFinalizer(void* isolate_callback_data,
void* peer) {
*static_cast<int*>(peer) *= 2;
Expand Down

0 comments on commit 1d3d819

Please sign in to comment.