Skip to content

Commit 1d6c102

Browse files
alexmarkovcommit-bot@chromium.org
alexmarkov
authored and
[email protected]
committed
[VM] Fix OOM handling when allocating buffers in dart:io
* Test standalone_2/file_error_test is updated for Dart 2.0 fixed-size integers. * This update revealed that certain dart:io native methods do not handle out-of-memory properly when I/O buffers are allocated. This CL fixes this bug. Change-Id: I6a9018ab86da7b163d9797d745544835dfb0f15c Reviewed-on: https://dart-review.googlesource.com/20582 Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Zach Anderson <[email protected]>
1 parent f669d19 commit 1d6c102

File tree

10 files changed

+81
-38
lines changed

10 files changed

+81
-38
lines changed

‎runtime/bin/dartutils.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -945,7 +945,9 @@ Dart_CObject* CObject::NewIOBuffer(int64_t length) {
945945
return NULL;
946946
}
947947
uint8_t* data = IOBuffer::Allocate(static_cast<intptr_t>(length));
948-
ASSERT(data != NULL);
948+
if (data == NULL) {
949+
return NULL;
950+
}
949951
return NewExternalUint8Array(static_cast<intptr_t>(length), data, data,
950952
IOBuffer::Finalizer);
951953
}

‎runtime/bin/file.cc

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -176,36 +176,39 @@ void FUNCTION_NAME(File_Read)(Dart_NativeArguments args) {
176176
ASSERT(file != NULL);
177177
Dart_Handle length_object = Dart_GetNativeArgument(args, 1);
178178
int64_t length = 0;
179-
if (DartUtils::GetInt64Value(length_object, &length)) {
180-
uint8_t* buffer = NULL;
181-
Dart_Handle external_array = IOBuffer::Allocate(length, &buffer);
182-
int64_t bytes_read = file->Read(reinterpret_cast<void*>(buffer), length);
183-
if (bytes_read < 0) {
184-
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
185-
} else {
186-
if (bytes_read < length) {
187-
const int kNumArgs = 3;
188-
Dart_Handle dart_args[kNumArgs];
189-
dart_args[0] = external_array;
190-
dart_args[1] = Dart_NewInteger(0);
191-
dart_args[2] = Dart_NewInteger(bytes_read);
192-
// TODO(sgjesse): Cache the _makeUint8ListView function somewhere.
193-
Dart_Handle io_lib =
194-
Dart_LookupLibrary(DartUtils::NewString("dart:io"));
195-
if (Dart_IsError(io_lib)) {
196-
Dart_PropagateError(io_lib);
197-
}
198-
Dart_Handle array_view =
199-
Dart_Invoke(io_lib, DartUtils::NewString("_makeUint8ListView"),
200-
kNumArgs, dart_args);
201-
Dart_SetReturnValue(args, array_view);
202-
} else {
203-
Dart_SetReturnValue(args, external_array);
204-
}
205-
}
206-
} else {
179+
if (!DartUtils::GetInt64Value(length_object, &length) || (length < 0)) {
207180
OSError os_error(-1, "Invalid argument", OSError::kUnknown);
208181
Dart_SetReturnValue(args, DartUtils::NewDartOSError(&os_error));
182+
return;
183+
}
184+
uint8_t* buffer = NULL;
185+
Dart_Handle external_array = IOBuffer::Allocate(length, &buffer);
186+
if (Dart_IsNull(external_array)) {
187+
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
188+
return;
189+
}
190+
int64_t bytes_read = file->Read(reinterpret_cast<void*>(buffer), length);
191+
if (bytes_read < 0) {
192+
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
193+
return;
194+
}
195+
if (bytes_read < length) {
196+
const int kNumArgs = 3;
197+
Dart_Handle dart_args[kNumArgs];
198+
dart_args[0] = external_array;
199+
dart_args[1] = Dart_NewInteger(0);
200+
dart_args[2] = Dart_NewInteger(bytes_read);
201+
// TODO(sgjesse): Cache the _makeUint8ListView function somewhere.
202+
Dart_Handle io_lib = Dart_LookupLibrary(DartUtils::NewString("dart:io"));
203+
if (Dart_IsError(io_lib)) {
204+
Dart_PropagateError(io_lib);
205+
}
206+
Dart_Handle array_view =
207+
Dart_Invoke(io_lib, DartUtils::NewString("_makeUint8ListView"),
208+
kNumArgs, dart_args);
209+
Dart_SetReturnValue(args, array_view);
210+
} else {
211+
Dart_SetReturnValue(args, external_array);
209212
}
210213
}
211214

@@ -1105,7 +1108,9 @@ CObject* File::ReadRequest(const CObjectArray& request) {
11051108
}
11061109
const int64_t length = CObjectInt32OrInt64ToInt64(request[1]);
11071110
Dart_CObject* io_buffer = CObject::NewIOBuffer(length);
1108-
ASSERT(io_buffer != NULL);
1111+
if (io_buffer == NULL) {
1112+
return CObject::NewOSError();
1113+
}
11091114
uint8_t* data = io_buffer->value.as_external_typed_data.data;
11101115
const int64_t bytes_read = file->Read(data, length);
11111116
if (bytes_read < 0) {
@@ -1135,7 +1140,9 @@ CObject* File::ReadIntoRequest(const CObjectArray& request) {
11351140
}
11361141
const int64_t length = CObjectInt32OrInt64ToInt64(request[1]);
11371142
Dart_CObject* io_buffer = CObject::NewIOBuffer(length);
1138-
ASSERT(io_buffer != NULL);
1143+
if (io_buffer == NULL) {
1144+
return CObject::NewOSError();
1145+
}
11391146
uint8_t* data = io_buffer->value.as_external_typed_data.data;
11401147
const int64_t bytes_read = file->Read(data, length);
11411148
if (bytes_read < 0) {

‎runtime/bin/filter.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,10 @@ void FUNCTION_NAME(Filter_Processed)(Dart_NativeArguments args) {
246246
} else {
247247
uint8_t* io_buffer;
248248
Dart_Handle result = IOBuffer::Allocate(read, &io_buffer);
249+
if (Dart_IsNull(result)) {
250+
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
251+
return;
252+
}
249253
memmove(io_buffer, filter->processed_buffer(), read);
250254
Dart_SetReturnValue(args, result);
251255
}

‎runtime/bin/io_buffer.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ namespace bin {
99

1010
Dart_Handle IOBuffer::Allocate(intptr_t size, uint8_t** buffer) {
1111
uint8_t* data = Allocate(size);
12+
if (data == NULL) {
13+
return Dart_Null();
14+
}
1215
Dart_Handle result =
1316
Dart_NewExternalTypedData(Dart_TypedData_kUint8, data, size);
1417
Dart_NewWeakPersistentHandle(result, data, size, IOBuffer::Finalizer);
@@ -24,7 +27,7 @@ Dart_Handle IOBuffer::Allocate(intptr_t size, uint8_t** buffer) {
2427
}
2528

2629
uint8_t* IOBuffer::Allocate(intptr_t size) {
27-
return new uint8_t[size];
30+
return reinterpret_cast<uint8_t*>(malloc(size));
2831
}
2932

3033
} // namespace bin

‎runtime/bin/io_buffer.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,7 @@ class IOBuffer {
2222

2323
// Function for disposing of IO buffer storage. All backing storage
2424
// for IO buffers must be freed using this function.
25-
static void Free(void* buffer) {
26-
delete[] reinterpret_cast<uint8_t*>(buffer);
27-
}
25+
static void Free(void* buffer) { free(buffer); }
2826

2927
// Function for finalizing external byte arrays used as IO buffers.
3028
static void Finalizer(void* isolate_callback_data,

‎runtime/bin/process.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,10 @@ void FUNCTION_NAME(StringToSystemEncoding)(Dart_NativeArguments args) {
346346
}
347347
uint8_t* buffer = NULL;
348348
Dart_Handle external_array = IOBuffer::Allocate(system_len, &buffer);
349+
if (Dart_IsNull(external_array)) {
350+
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
351+
return;
352+
}
349353
if (!Dart_IsError(external_array)) {
350354
memmove(buffer, system_string, system_len);
351355
}

‎runtime/bin/process.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,9 @@ class BufferListBase {
247247
uint8_t* buffer;
248248
intptr_t buffer_position = 0;
249249
Dart_Handle result = IOBuffer::Allocate(data_size_, &buffer);
250+
if (Dart_IsNull(result)) {
251+
return DartUtils::NewDartOSError();
252+
}
250253
if (Dart_IsError(result)) {
251254
Free();
252255
return result;

‎runtime/bin/socket.cc

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,12 +323,17 @@ void FUNCTION_NAME(Socket_Read)(Dart_NativeArguments args) {
323323
Socket* socket =
324324
Socket::GetSocketIdNativeField(Dart_GetNativeArgument(args, 0));
325325
int64_t length = 0;
326-
if (DartUtils::GetInt64Value(Dart_GetNativeArgument(args, 1), &length)) {
326+
if (DartUtils::GetInt64Value(Dart_GetNativeArgument(args, 1), &length) &&
327+
(length >= 0)) {
327328
if (Socket::short_socket_read()) {
328329
length = (length + 1) / 2;
329330
}
330331
uint8_t* buffer = NULL;
331332
Dart_Handle result = IOBuffer::Allocate(length, &buffer);
333+
if (Dart_IsNull(result)) {
334+
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
335+
return;
336+
}
332337
if (Dart_IsError(result)) {
333338
Dart_PropagateError(result);
334339
}
@@ -340,6 +345,10 @@ void FUNCTION_NAME(Socket_Read)(Dart_NativeArguments args) {
340345
} else if (bytes_read > 0) {
341346
uint8_t* new_buffer = NULL;
342347
Dart_Handle new_result = IOBuffer::Allocate(bytes_read, &new_buffer);
348+
if (Dart_IsNull(new_result)) {
349+
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
350+
return;
351+
}
343352
if (Dart_IsError(new_result)) {
344353
Dart_PropagateError(new_result);
345354
}
@@ -393,6 +402,10 @@ void FUNCTION_NAME(Socket_RecvFrom)(Dart_NativeArguments args) {
393402
ASSERT(bytes_read > 0);
394403
uint8_t* data_buffer = NULL;
395404
Dart_Handle data = IOBuffer::Allocate(bytes_read, &data_buffer);
405+
if (Dart_IsNull(data)) {
406+
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
407+
return;
408+
}
396409
if (Dart_IsError(data)) {
397410
Dart_PropagateError(data);
398411
}

‎runtime/bin/sync_socket.cc

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,20 +213,29 @@ void FUNCTION_NAME(SynchronousSocket_Read)(Dart_NativeArguments args) {
213213
DART_CHECK_ERROR(result);
214214

215215
int64_t length = 0;
216-
if (!DartUtils::GetInt64Value(Dart_GetNativeArgument(args, 1), &length)) {
216+
if (!DartUtils::GetInt64Value(Dart_GetNativeArgument(args, 1), &length) ||
217+
(length < 0)) {
217218
Dart_SetReturnValue(args, DartUtils::NewDartArgumentError(
218219
"First parameter must be an integer."));
219220
return;
220221
}
221222
uint8_t* buffer = NULL;
222223
result = IOBuffer::Allocate(length, &buffer);
224+
if (Dart_IsNull(result)) {
225+
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
226+
return;
227+
}
223228
ASSERT(buffer != NULL);
224229
intptr_t bytes_read = SynchronousSocket::Read(socket->fd(), buffer, length);
225230
if (bytes_read == length) {
226231
Dart_SetReturnValue(args, result);
227232
} else if (bytes_read > 0) {
228233
uint8_t* new_buffer = NULL;
229234
Dart_Handle new_result = IOBuffer::Allocate(bytes_read, &new_buffer);
235+
if (Dart_IsNull(new_result)) {
236+
Dart_SetReturnValue(args, DartUtils::NewDartOSError());
237+
return;
238+
}
230239
ASSERT(new_buffer != NULL);
231240
memmove(new_buffer, buffer, bytes_read);
232241
Dart_SetReturnValue(args, new_result);

‎tests/standalone_2/io/file_error_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ testRepeatedlyCloseFileSync() {
400400

401401
testReadSyncBigInt() {
402402
createTestFile((file, done) {
403-
var bigint = 100000000000000000000000000000000000000000;
403+
var bigint = 9223372036854775807;
404404
var openedFile = file.openSync();
405405
Expect.throws(
406406
() => openedFile.readSync(bigint), (e) => e is FileSystemException);

0 commit comments

Comments
 (0)