Skip to content

Commit b0b0ca1

Browse files
committed
fix(runtime): zero the whole return slot for empty callback results
ArgConverter::SetValue wrote a single ffi_arg when a JS callback returned null, undefined or a revoked Proxy, so struct-valued returns handed native code stale bytes past the first word. Callers now pass cif->rtype->size and the empty branch clears the full slot.
1 parent 7206d01 commit b0b0ca1

5 files changed

Lines changed: 29 additions & 9 deletions

File tree

‎NativeScript/runtime/ArgConverter.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,10 @@ class ArgConverter {
7171
static const ProtocolMeta* FindProtocolMeta(Protocol* protocol);
7272
static void MethodCallback(ffi_cif* cif, void* retValue, void** argValues,
7373
void* userData);
74+
// returnSize is the ABI size of the return slot (cif->rtype->size); an
75+
// empty, null or undefined value zeroes all of it.
7476
static void SetValue(v8::Local<v8::Context> context, void* retValue,
75-
v8::Local<v8::Value> value,
77+
size_t returnSize, v8::Local<v8::Value> value,
7678
const TypeEncoding* typeEncoding);
7779
// Returns (lazily creating) the per-isolate interop.escapeException brand
7880
// private stored in Caches. Empty handle if the isolate cache is invalid.

‎NativeScript/runtime/ArgConverter.mm‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "ArgConverter.h"
22
#include <Foundation/Foundation.h>
3+
#include <algorithm>
34
#include <sstream>
45
#include "DictionaryAdapter.h"
56
#include "Helpers.h"
@@ -438,7 +439,7 @@
438439
if (!success) {
439440
memset(retValue, 0, cif->rtype->size);
440441
} else {
441-
ArgConverter::SetValue(context, retValue, result, data->typeEncoding_);
442+
ArgConverter::SetValue(context, retValue, cif->rtype->size, result, data->typeEncoding_);
442443
}
443444
} else {
444445
memset(retValue, 0, cif->rtype->size);
@@ -450,18 +451,18 @@
450451
}
451452
}
452453

453-
void ArgConverter::SetValue(Local<Context> context, void* retValue, Local<Value> value,
454-
const TypeEncoding* typeEncoding) {
454+
void ArgConverter::SetValue(Local<Context> context, void* retValue, size_t returnSize,
455+
Local<Value> value, const TypeEncoding* typeEncoding) {
455456
if (typeEncoding->type == BinaryTypeEncodingType::VoidEncoding) {
456457
return;
457458
}
458459

459460
// Runs inside an ffi closure, where a C++ throw cannot propagate; a revoked
460-
// proxy returns nil.
461+
// proxy returns nil. libffi sizes the return slot to at least ffi_arg even
462+
// for narrower types, and the full slot must be written.
461463
value = tns::UnwrapProxy(value);
462464
if (value.IsEmpty() || value->IsNullOrUndefined()) {
463-
void* nullPtr = nullptr;
464-
*(ffi_arg*)retValue = (unsigned long)nullPtr;
465+
memset(retValue, 0, std::max(returnSize, sizeof(ffi_arg)));
465466
return;
466467
}
467468

‎NativeScript/runtime/ClassBuilder.mm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -931,7 +931,7 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) {
931931
memset(retValue, 0, cif->rtype->size);
932932
} else {
933933
const TypeEncoding* typeEncoding = context->meta_->getter()->encodings()->first();
934-
ArgConverter::SetValue(v8Context, retValue, res, typeEncoding);
934+
ArgConverter::SetValue(v8Context, retValue, cif->rtype->size, res, typeEncoding);
935935
}
936936
}
937937
if (pendingThrow != nil) {

‎NativeScript/runtime/MetadataBuilder.mm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1280,7 +1280,7 @@ CMethodCall methodCall(context, item->userData_, typeEncoding, args,
12801280
memset(retValue, 0, cif->rtype->size);
12811281
} else {
12821282
const TypeEncoding* typeEncoding = context->meta_->getter()->encodings()->first();
1283-
ArgConverter::SetValue(v8Context, retValue, res, typeEncoding);
1283+
ArgConverter::SetValue(v8Context, retValue, cif->rtype->size, res, typeEncoding);
12841284
}
12851285
}
12861286
if (pendingThrow != nil) {

‎TestRunner/app/tests/ProxyReceiverTests.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,23 @@ describe(module.id, function () {
145145
}).toThrowError(TypeError, /not a native object/);
146146
});
147147

148+
it("zeroes a struct return when a callback returns a revoked proxy", function () {
149+
var revocable = Proxy.revocable(CGRectMake(1, 2, 3, 4), {});
150+
revocable.revoke();
151+
152+
[revocable.proxy, null].forEach(function (returned) {
153+
var rect = getStructFromCallback(new interop.FunctionReference(function () {
154+
return returned;
155+
}));
156+
157+
expect(rect instanceof CGRect).toBe(true);
158+
expect(rect.origin.x).toBe(0);
159+
expect(rect.origin.y).toBe(0);
160+
expect(rect.size.width).toBe(0);
161+
expect(rect.size.height).toBe(0);
162+
});
163+
});
164+
148165
it("releases the native counterpart of the proxy target", function () {
149166
var target = NSMutableString.alloc().init();
150167
var proxy = new Proxy(target, {});

0 commit comments

Comments
 (0)