Skip to content

Commit 04c984c

Browse files
dcharkescommit-bot@chromium.org
authored andcommitted
[vm/ffi] Make overflow checks consistent
This CL changes the semantics of Pointer.allocate() to not do any range or overflow checks. This is consistent with the truncating behavior that the rest of the FFI has. Fixes: #37250 Change-Id: Icc2b53e229cd6a2faae99c833ea5df372eb35b74 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/107503 Commit-Queue: Daco Harkes <dacoharkes@google.com> Reviewed-by: Samir Jindel <sjindel@google.com>
1 parent b4e99fb commit 04c984c

2 files changed

Lines changed: 22 additions & 17 deletions

File tree

runtime/lib/ffi.cc

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -169,18 +169,6 @@ static void CheckDartAndCTypeCorrespond(const AbstractType& native_type,
169169

170170
// The following functions are runtime checks on arguments.
171171

172-
// Note that expected_from and expected_to are inclusive.
173-
static void CheckRange(const Integer& argument_value,
174-
intptr_t expected_from,
175-
intptr_t expected_to,
176-
const char* argument_name) {
177-
int64_t value = argument_value.AsInt64Value();
178-
if (value < expected_from || expected_to < value) {
179-
Exceptions::ThrowRangeError(argument_name, argument_value, expected_from,
180-
expected_to);
181-
}
182-
}
183-
184172
static const Pointer& AsPointer(const Instance& instance) {
185173
if (!instance.IsPointer()) {
186174
const String& error = String::Handle(String::NewFormatted(
@@ -221,11 +209,10 @@ DEFINE_NATIVE_ENTRY(Ffi_allocate, 1, 1) {
221209
GET_NON_NULL_NATIVE_ARGUMENT(Integer, argCount, arguments->NativeArgAt(0));
222210
int64_t count = argCount.AsInt64Value();
223211
classid_t type_cid = type_arg.type_class_id();
224-
int64_t max_count = INTPTR_MAX / compiler::ffi::ElementSizeInBytes(type_cid);
225-
CheckRange(argCount, 1, max_count, "count");
226212

227-
size_t size = compiler::ffi::ElementSizeInBytes(type_cid) * count;
228-
uint64_t memory = reinterpret_cast<uint64_t>(malloc(size));
213+
size_t size = compiler::ffi::ElementSizeInBytes(type_cid) *
214+
count; // Truncates overflow.
215+
size_t memory = reinterpret_cast<size_t>(malloc(size));
229216
if (memory == 0) {
230217
const String& error = String::Handle(String::NewFormatted(
231218
"allocating (%" Pd ") bytes of memory failed", size));

tests/ffi/data_test.dart

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,25 @@ void testPointerPointerArithmeticSizes() {
9393
}
9494

9595
void testPointerAllocateNonPositive() {
96-
Expect.throws(() => ffi.allocate<ffi.Int8>(count: 0));
96+
// > If size is 0, either a null pointer or a unique pointer that can be
97+
// > successfully passed to free() shall be returned.
98+
// http://pubs.opengroup.org/onlinepubs/009695399/functions/malloc.html
99+
//
100+
// Null pointer throws a Dart exception.
101+
bool returnedNullPointer = false;
102+
ffi.Pointer<ffi.Int8> p;
103+
try {
104+
p = ffi.allocate<ffi.Int8>(count: 0);
105+
} on Exception {
106+
returnedNullPointer = true;
107+
}
108+
if (!returnedNullPointer) {
109+
p.free();
110+
}
111+
112+
// Passing in -1 will be converted into an unsigned integer. So, it will try
113+
// to allocate SIZE_MAX - 1 + 1 bytes. This will fail as it is the max amount
114+
// of addressable memory on the system.
97115
Expect.throws(() => ffi.allocate<ffi.Int8>(count: -1));
98116
}
99117

0 commit comments

Comments
 (0)