diff --git a/BUILD.gn b/BUILD.gn index 25f4b6a5c3b..6f33af71f1a 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -51,8 +51,8 @@ group("runtime") { "samples/embedder:kernel", "samples/ffi/http:fake_http", "samples/ffi/httpIG:fake_httpIG", - "utils/dartdev:dartdev", - "utils/kernel-service:kernel-service", + "utils/dartdev", + "utils/kernel-service", ] # The following dependencies allow dartdev to start the resident frontend @@ -68,8 +68,8 @@ group("runtime") { ] } else { deps += [ - "utils/dds:dds", - "utils/dtd:dtd", + "utils/dds", + "utils/dtd", "utils/kernel-service:frontend_server", ] } @@ -104,6 +104,8 @@ group("runtime") { deps += [ "runtime/bin:abstract_socket_test" ] } else if (is_fuchsia) { deps += [ ":fuchsia_test_package" ] + } else if (is_win) { + deps += [ "runtime/bin:create_process_test_helper" ] } } diff --git a/runtime/bin/BUILD.gn b/runtime/bin/BUILD.gn index df60370a61a..6785943045d 100644 --- a/runtime/bin/BUILD.gn +++ b/runtime/bin/BUILD.gn @@ -1128,6 +1128,11 @@ executable("abstract_socket_test") { include_dirs = [ ".." ] } +executable("create_process_test_helper") { + sources = [ "create_process_test_helper.cc" ] + include_dirs = [ ".." ] +} + source_set("run_vm_tests_set") { if (target_os == "fuchsia") { testonly = true diff --git a/runtime/bin/create_process_test_helper.cc b/runtime/bin/create_process_test_helper.cc new file mode 100644 index 00000000000..5e37e03e2b0 --- /dev/null +++ b/runtime/bin/create_process_test_helper.cc @@ -0,0 +1,138 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// This is a utility program for testing that Dart binary correctly handles +// situations when stdout and stderr handles are the same. +// +// This can happen in certain terminal emulators, e.g. one used by GitBash +// see https://github.com/dart-lang/sdk/issues/61981 for an example. + +#include "platform/globals.h" +#if defined(DART_HOST_OS_WINDOWS) + +#include +#include +#include +#include + +struct StdioHandles { + HANDLE in; + HANDLE out; + HANDLE err; +}; + +static void ReportErrorAndAbort( + std::source_location location = std::source_location::current()) { + const auto code = GetLastError(); + + wchar_t buffer[512]; + auto message_size = + FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, + nullptr, code, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + buffer, ARRAY_SIZE(buffer), nullptr); + if (message_size == 0) { + _snwprintf(buffer, ARRAY_SIZE(buffer), L"OS Error %d", code); + } + fprintf(stderr, "error at %s:%d: %ls", location.file_name(), location.line(), + buffer); + abort(); +} + +static void LaunchProcessWith(wchar_t* command_line, + const StdioHandles& stdio_handles) { + fprintf(stderr, "LAUNCHING %ls\n", command_line); + + // Setup info + STARTUPINFOEXW startup_info; + ZeroMemory(&startup_info, sizeof(startup_info)); + startup_info.StartupInfo.cb = sizeof(startup_info); + + // Setup the handles to inherit. We only want to inherit the three + // handles for stdin, stdout and stderr. + startup_info.StartupInfo.hStdInput = stdio_handles.in; + startup_info.StartupInfo.hStdOutput = stdio_handles.out; + startup_info.StartupInfo.hStdError = stdio_handles.err; + startup_info.StartupInfo.dwFlags = STARTF_USESTDHANDLES; + SIZE_T size = 0; + // The call to determine the size of an attribute list always fails with + // ERROR_INSUFFICIENT_BUFFER and that error should be ignored. + if (!InitializeProcThreadAttributeList(nullptr, 1, 0, &size) && + (GetLastError() != ERROR_INSUFFICIENT_BUFFER)) { + return ReportErrorAndAbort(); + } + auto attribute_list = + reinterpret_cast(malloc(size)); + ZeroMemory(attribute_list, size); + if (!InitializeProcThreadAttributeList(attribute_list, 1, 0, &size)) { + return ReportErrorAndAbort(); + } + std::vector inherited_handles = {stdio_handles.in}; + if (stdio_handles.out != stdio_handles.in) { + inherited_handles.push_back(stdio_handles.out); + } + if (stdio_handles.err != stdio_handles.out && + stdio_handles.err != stdio_handles.in) { + inherited_handles.push_back(stdio_handles.err); + } + if (!UpdateProcThreadAttribute( + attribute_list, 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, + inherited_handles.data(), inherited_handles.size() * sizeof(HANDLE), + nullptr, nullptr)) { + return ReportErrorAndAbort(); + } + startup_info.lpAttributeList = attribute_list; + + PROCESS_INFORMATION process_info; + ZeroMemory(&process_info, sizeof(process_info)); + + // Create process. + BOOL result = CreateProcessW( + /*lpApplicationName=*/nullptr, command_line, + /*lpProcessAttributes=*/nullptr, + /*lpThreadAttributes=*/nullptr, + /*bInheritHandles=*/TRUE, + /*dwCreationFlags=*/EXTENDED_STARTUPINFO_PRESENT, + /*lpEnvironment=*/nullptr, + /*lpCurrentDirectory=*/nullptr, + reinterpret_cast(&startup_info), &process_info); + + if (result == 0) { + return ReportErrorAndAbort(); + } + + WaitForSingleObject(process_info.hProcess, INFINITE); + CloseHandle(process_info.hProcess); + CloseHandle(process_info.hThread); +} + +int main(int argc, char* argv[]) { + if (argc <= 1) { + fprintf(stderr, "Usage: %s ... \n", argv[0]); + return -1; + } + + // Generate command line. Assume that it does not contain any white-space + // in the arguments. + std::wstringstream wstr; + for (int i = 1; i < argc; i++) { + wstr << (i > 1 ? " " : "") << argv[i]; + } + + HANDLE stdin_handle = GetStdHandle(STD_INPUT_HANDLE); + HANDLE stdout_handle = GetStdHandle(STD_OUTPUT_HANDLE); + LaunchProcessWith( + wstr.str().data(), + {.in = stdin_handle, .out = stdout_handle, .err = stdout_handle}); + CloseHandle(stdin_handle); + CloseHandle(stdout_handle); + return 0; +} + +#else + +int main() { + return -1; +} + +#endif // defined(DART_HOST_OS_WINDOWS) diff --git a/runtime/bin/process.cc b/runtime/bin/process.cc index 027734d06fa..46918237363 100644 --- a/runtime/bin/process.cc +++ b/runtime/bin/process.cc @@ -155,7 +155,7 @@ void FUNCTION_NAME(Process_Start)(Dart_NativeArguments args) { Dart_Handle stderr_handle = Dart_GetNativeArgument(args, 9); Dart_Handle exit_handle = Dart_GetNativeArgument(args, 10); intptr_t pid = -1; - char* os_error_message = nullptr; // Scope allocated by Process::Start. + char* os_error_message = nullptr; int error_code = Process::Start( namespc, path, string_args, args_length, working_directory, diff --git a/runtime/bin/process_win.cc b/runtime/bin/process_win.cc index 902797b79e5..01368f7cb85 100644 --- a/runtime/bin/process_win.cc +++ b/runtime/bin/process_win.cc @@ -9,6 +9,7 @@ #include // NOLINT #include // NOLINT +#include #include #include "bin/builtin.h" @@ -294,12 +295,15 @@ static void CloseProcessPipes(HANDLE handles1[2], CloseProcessPipe(handles4); } -static int SetOsErrorMessage(char** os_error_message) { +static int SetOsErrorMessage( + CStringUniquePtr* os_error_message, + std::source_location location = std::source_location::current()) { int error_code = GetLastError(); const int kMaxMessageLength = 256; wchar_t message[kMaxMessageLength]; FormatMessageIntoBuffer(error_code, message, kMaxMessageLength); - *os_error_message = StringUtilsWin::WideToUtf8(message); + os_error_message->reset(Utils::SCreate( + "%ls (at %s:%d)", message, location.file_name(), location.line())); return error_code; } @@ -356,7 +360,7 @@ class ProcessStarter { intptr_t* err, intptr_t* id, intptr_t* exit_handler, - char** os_error_message) + CStringUniquePtr* os_error_message) : path_(path), working_directory_(working_directory), mode_(mode), @@ -617,7 +621,13 @@ class ProcessStarter { if (!InitializeProcThreadAttributeList(attribute_list_, 1, 0, &size)) { return CleanupAndReturnError(); } - inherited_handles_ = {stdin_handle, stdout_handle, stderr_handle}; + inherited_handles_ = {stdin_handle}; + if (stdout_handle != stdin_handle) { + inherited_handles_.push_back(stdout_handle); + } + if (stderr_handle != stdout_handle && stderr_handle != stdin_handle) { + inherited_handles_.push_back(stderr_handle); + } if (!UpdateProcThreadAttribute( attribute_list_, 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, inherited_handles_.data(), @@ -707,8 +717,9 @@ class ProcessStarter { return 0; } - int CleanupAndReturnError() { - int error_code = SetOsErrorMessage(os_error_message_); + int CleanupAndReturnError( + std::source_location location = std::source_location::current()) { + int error_code = SetOsErrorMessage(os_error_message_, location); CloseProcessPipes(stdin_handles_, stdout_handles_, stderr_handles_, exit_handles_); return error_code; @@ -734,7 +745,7 @@ class ProcessStarter { intptr_t* err_; intptr_t* id_; intptr_t* exit_handler_; - char** os_error_message_; + CStringUniquePtr* os_error_message_; private: DISALLOW_ALLOCATION(); @@ -755,10 +766,15 @@ int Process::Start(Namespace* namespc, intptr_t* id, intptr_t* exit_handler, char** os_error_message) { + CStringUniquePtr error; ProcessStarter starter(path, arguments, arguments_length, working_directory, environment, environment_length, mode, in, out, err, - id, exit_handler, os_error_message); - return starter.Start(); + id, exit_handler, &error); + const auto result = starter.Start(); + if (result != 0 && error != nullptr) { + *os_error_message = DartUtils::ScopedCopyCString(error.get()); + } + return result; } class BufferList : public BufferListBase { @@ -981,7 +997,7 @@ int Process::Exec(Namespace* namespc, HANDLE hjob = CreateJobObject(nullptr, nullptr); if (hjob == nullptr) { BufferFormatter f(errmsg, errmsg_len); - f.Printf("Process::Exec - CreateJobObject failed %d\n", GetLastError()); + f.Printf("Process::Exec - CreateJobObject failed %d", GetLastError()); return -1; } JOBOBJECT_EXTENDED_LIMIT_INFORMATION info; @@ -991,7 +1007,7 @@ int Process::Exec(Namespace* namespc, sizeof(JOBOBJECT_EXTENDED_LIMIT_INFORMATION), &qresult)) { BufferFormatter f(errmsg, errmsg_len); - f.Printf("Process::Exec - QueryInformationJobObject failed %d\n", + f.Printf("Process::Exec - QueryInformationJobObject failed %d", GetLastError()); return -1; } @@ -1005,7 +1021,7 @@ int Process::Exec(Namespace* namespc, if (!SetInformationJobObject(hjob, JobObjectExtendedLimitInformation, &info, sizeof(JOBOBJECT_EXTENDED_LIMIT_INFORMATION))) { BufferFormatter f(errmsg, errmsg_len); - f.Printf("Process::Exec - SetInformationJobObject failed %d\n", + f.Printf("Process::Exec - SetInformationJobObject failed %d", GetLastError()); return -1; } @@ -1015,7 +1031,7 @@ int Process::Exec(Namespace* namespc, // we haven't spawned any children yet this race is harmless) if (!AssignProcessToJobObject(hjob, GetCurrentProcess())) { BufferFormatter f(errmsg, errmsg_len); - f.Printf("Process::Exec - AssignProcessToJobObject failed %d\n", + f.Printf("Process::Exec - AssignProcessToJobObject failed %d", GetLastError()); return -1; } @@ -1028,14 +1044,14 @@ int Process::Exec(Namespace* namespc, // as the value passed in 'path', we strip that off when starting the // process. intptr_t pid = -1; - char* os_error_message = nullptr; // Scope allocated by Process::Start. + CStringUniquePtr os_error_message; ProcessStarter starter(path, &(arguments[1]), (arguments_length - 1), working_directory, nullptr, 0, kInheritStdio, nullptr, nullptr, nullptr, &pid, nullptr, &os_error_message); int result = starter.StartForExec(hjob); if (result != 0) { BufferFormatter f(errmsg, errmsg_len); - f.Printf("Process::Exec - %s\n", os_error_message); + f.Printf("ProcessStarter::StartForExec failed: %s", os_error_message.get()); return -1; } @@ -1047,14 +1063,14 @@ int Process::Exec(Namespace* namespc, DWORD wait_result = WaitForSingleObject(child_process, INFINITE); if (wait_result != WAIT_OBJECT_0) { BufferFormatter f(errmsg, errmsg_len); - f.Printf("Process::Exec - WaitForSingleObject failed %d\n", GetLastError()); + f.Printf("Process::Exec - WaitForSingleObject failed %d", GetLastError()); CloseHandle(child_process); return -1; } int retval; if (!GetExitCodeProcess(child_process, reinterpret_cast(&retval))) { BufferFormatter f(errmsg, errmsg_len); - f.Printf("Process::Exec - GetExitCodeProcess failed %d\n", GetLastError()); + f.Printf("Process::Exec - GetExitCodeProcess failed %d", GetLastError()); CloseHandle(child_process); return -1; } diff --git a/runtime/bin/utils_win.cc b/runtime/bin/utils_win.cc index c73d97299bb..7a0217e0a5c 100644 --- a/runtime/bin/utils_win.cc +++ b/runtime/bin/utils_win.cc @@ -46,9 +46,17 @@ void FormatMessageIntoBuffer(DWORD code, wchar_t* buffer, int buffer_length) { code, GetLastError()); } _snwprintf(buffer, buffer_length, L"OS Error %d", code); + return; } - // Ensure string termination. - buffer[buffer_length - 1] = 0; + + // Strip trailing whitespace and a dot. + while (message_size > 0 && iswspace(buffer[message_size - 1])) { + message_size--; + } + if (message_size > 0 && buffer[message_size - 1] == L'.') { + message_size--; + } + buffer[Utils::Minimum(message_size, buffer_length - 1)] = 0; } FILETIME GetFiletimeFromMillis(int64_t millis) { diff --git a/runtime/tests/vm/dart/regress_61981_test.dart b/runtime/tests/vm/dart/regress_61981_test.dart new file mode 100644 index 00000000000..2fa8b7668fe --- /dev/null +++ b/runtime/tests/vm/dart/regress_61981_test.dart @@ -0,0 +1,44 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// Test that Dart binary correctly handles situations when stdout and stderr +// handles are the same. +// +// This can happen in certain terminal emulators, e.g. one used by GitBash +// see https://github.com/dart-lang/sdk/issues/61981 for an example. + +import 'dart:io'; + +import 'package:expect/expect.dart'; +import 'package:path/path.dart' as p; + +void main(List args) { + if (args case ['child-process']) { + print('OK'); + return; + } + + if (!Platform.isWindows || + p.basenameWithoutExtension(Platform.executable) != 'dart') { + return; + } + + final createProcessHelper = p.join( + p.dirname(Platform.executable), + 'create_process_test_helper.exe', + ); + + final result = Process.runSync(createProcessHelper, [ + Platform.executable, + 'run', + Platform.script.toFilePath(), + 'child-process', + ]); + if (result.exitCode != 0) { + print(result.stdout); + print(result.stderr); + Expect.fail('process exited with ${result.exitCode}'); + } + Expect.equals('OK', (result.stdout as String).trim()); +}