Skip to content

Commit bd87f5d

Browse files
geoffreygarengeoffreygaren
committed
Fixed unretained local variable warnings in JavaScriptCore/API
https://bugs.webkit.org/show_bug.cgi?id=305424 rdar://168095490 Reviewed by Ryosuke Niwa. Mechanical fixes dictated by the SaferCPP bot. Canonical link: https://commits.webkit.org/305626@main
1 parent ace5138 commit bd87f5d

8 files changed

Lines changed: 103 additions & 100 deletions

File tree

‎Source/JavaScriptCore/API/JSAPIGlobalObject.mm‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,13 +209,13 @@
209209
if (![script isKindOfClass:[JSScript class]]) [[unlikely]]
210210
return rejectPromise("First argument of resolution callback is not a JSScript"_s);
211211

212-
JSScript* jsScript = static_cast<JSScript *>(script);
212+
RetainPtr jsScript = static_cast<JSScript *>(script);
213213

214-
JSSourceCode* source = [jsScript jsSourceCode];
215-
if ([jsScript type] != kJSScriptTypeModule) [[unlikely]]
214+
JSSourceCode* source = [jsScript.get() jsSourceCode];
215+
if ([jsScript.get() type] != kJSScriptTypeModule) [[unlikely]]
216216
return rejectPromise("The JSScript that was provided did not have expected type of kJSScriptTypeModule."_s);
217217

218-
NSURL *sourceURL = [jsScript sourceURL];
218+
NSURL *sourceURL = [jsScript.get() sourceURL];
219219
String oldModuleKey { [sourceURL absoluteString] };
220220
if (Identifier::fromString(vm, oldModuleKey) != moduleKey) [[unlikely]]
221221
return rejectPromise(makeString("The same JSScript was provided for two different identifiers, previously: "_s, oldModuleKey, " and now: "_s, moduleKey.string()));

‎Source/JavaScriptCore/API/JSValue.mm‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -938,18 +938,18 @@ static id containerValueToObject(JSGlobalContextRef context, JSContainerConverto
938938

939939
if (current.type == ContainerArray) {
940940
ASSERT([current.objc isKindOfClass:[NSMutableArray class]]);
941-
NSMutableArray *array = (NSMutableArray *)current.objc;
942-
941+
RetainPtr array = dynamic_objc_cast<NSMutableArray>(current.objc);
942+
943943
auto lengthString = OpaqueJSString::tryCreate("length"_s);
944944
unsigned length = JSC::toUInt32(JSValueToNumber(context, JSObjectGetProperty(context, js, lengthString.get(), 0), 0));
945945

946946
for (unsigned i = 0; i < length; ++i) {
947947
id objc = convertor.convert(JSObjectGetPropertyAtIndex(context, js, i, 0));
948-
[array addObject:objc ? objc : [NSNull null]];
948+
[array.get() addObject:objc ? objc : [NSNull null]];
949949
}
950950
} else {
951951
ASSERT([current.objc isKindOfClass:[NSMutableDictionary class]]);
952-
NSMutableDictionary *dictionary = (NSMutableDictionary *)current.objc;
952+
RetainPtr dictionary = dynamic_objc_cast<NSMutableDictionary>(current.objc);
953953

954954
JSC::JSLockHolder locker(toJS(context));
955955

@@ -959,7 +959,7 @@ static id containerValueToObject(JSGlobalContextRef context, JSContainerConverto
959959
for (size_t i = 0; i < length; ++i) {
960960
JSStringRef propertyName = JSPropertyNameArrayGetNameAtIndex(propertyNameArray, i);
961961
if (id objc = convertor.convert(JSObjectGetProperty(context, js, propertyName, 0)))
962-
dictionary[(__bridge NSString *)adoptCF(JSStringCopyCFString(kCFAllocatorDefault, propertyName)).get()] = objc;
962+
dictionary.get()[(__bridge NSString *)adoptCF(JSStringCopyCFString(kCFAllocatorDefault, propertyName)).get()] = objc;
963963
}
964964

965965
JSPropertyNameArrayRelease(propertyNameArray);
@@ -1197,18 +1197,18 @@ JSValueRef objectToValue(JSContext *context, id object)
11971197

11981198
if (current.type == ContainerArray) {
11991199
ASSERT([current.objc isKindOfClass:[NSArray class]]);
1200-
NSArray *array = (NSArray *)current.objc;
1201-
NSUInteger count = [array count];
1200+
RetainPtr array = dynamic_objc_cast<NSArray>(current.objc);
1201+
NSUInteger count = [array.get() count];
12021202
for (NSUInteger index = 0; index < count; ++index)
1203-
JSObjectSetPropertyAtIndex(contextRef, js, index, convertor.convert([array objectAtIndex:index]), 0);
1203+
JSObjectSetPropertyAtIndex(contextRef, js, index, convertor.convert([array.get() objectAtIndex:index]), 0);
12041204
} else {
12051205
ASSERT(current.type == ContainerDictionary);
12061206
ASSERT([current.objc isKindOfClass:[NSDictionary class]]);
1207-
NSDictionary *dictionary = (NSDictionary *)current.objc;
1208-
for (id key in [dictionary keyEnumerator]) {
1209-
if (auto *keyString = dynamic_objc_cast<NSString>(key)) {
1210-
auto propertyName = OpaqueJSString::tryCreate(keyString);
1211-
JSObjectSetProperty(contextRef, js, propertyName.get(), convertor.convert([dictionary objectForKey:key]), 0, 0);
1207+
RetainPtr dictionary = dynamic_objc_cast<NSDictionary>(current.objc);
1208+
for (id key in [dictionary.get() keyEnumerator]) {
1209+
if (RetainPtr keyString = dynamic_objc_cast<NSString>(key)) {
1210+
auto propertyName = OpaqueJSString::tryCreate(keyString.get());
1211+
JSObjectSetProperty(contextRef, js, propertyName.get(), convertor.convert([dictionary.get() objectForKey:key]), 0, 0);
12121212
}
12131213
}
12141214
}

‎Source/JavaScriptCore/API/JSVirtualMachine.mm‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -128,24 +128,24 @@ - (void)dealloc
128128
[super dealloc];
129129
}
130130

131-
static id getInternalObjcObject(id object)
131+
static id getInternalObjcObject(id objectArg)
132132
{
133-
if ([object isKindOfClass:[JSManagedValue class]]) {
134-
JSValue* value = [static_cast<JSManagedValue *>(object) value];
133+
if (auto object = dynamic_objc_cast<JSManagedValue>(objectArg)) {
134+
JSValue* value = [object value];
135135
if (!value)
136136
return nil;
137137
id temp = tryUnwrapObjcObject([value.context JSGlobalContextRef], [value JSValueRef]);
138138
if (temp)
139139
return temp;
140-
return object;
140+
return objectArg;
141141
}
142-
143-
if ([object isKindOfClass:[JSValue class]]) {
144-
JSValue *value = static_cast<JSValue *>(object);
145-
object = tryUnwrapObjcObject([value.context JSGlobalContextRef], [value JSValueRef]);
142+
143+
if (auto value = dynamic_objc_cast<JSValue>(objectArg)) {
144+
RetainPtr object = tryUnwrapObjcObject([value.context JSGlobalContextRef], [value JSValueRef]);
145+
return object.autorelease();
146146
}
147147

148-
return object;
148+
return objectArg;
149149
}
150150

151151
- (bool)isOldExternalObject:(id)object
@@ -161,68 +161,68 @@ - (void)addExternalRememberedObject:(id)object
161161
[m_externalRememberedSet setObject:@YES forKey:object];
162162
}
163163

164-
- (void)addManagedReference:(id)object withOwner:(id)owner
164+
- (void)addManagedReference:(id)objectArg withOwner:(id)ownerArg
165165
{
166166
@autoreleasepool {
167-
if ([object isKindOfClass:[JSManagedValue class]])
168-
[object didAddOwner:owner];
167+
if ([objectArg isKindOfClass:[JSManagedValue class]])
168+
[objectArg didAddOwner:ownerArg];
169169

170-
object = getInternalObjcObject(object);
171-
owner = getInternalObjcObject(owner);
170+
RetainPtr object = getInternalObjcObject(objectArg);
171+
RetainPtr owner = getInternalObjcObject(ownerArg);
172172

173173
if (!object || !owner)
174174
return;
175175

176176
JSC::JSLockHolder locker(toJS(m_group));
177-
if ([self isOldExternalObject:owner] && ![self isOldExternalObject:object])
178-
[self addExternalRememberedObject:owner];
177+
if ([self isOldExternalObject:owner.get()] && ![self isOldExternalObject:object.get()])
178+
[self addExternalRememberedObject:owner.get()];
179179

180180
Locker externalDataMutexLocker { m_externalDataMutex };
181-
RetainPtr<NSMapTable> ownedObjects = [m_externalObjectGraph objectForKey:owner];
181+
RetainPtr<NSMapTable> ownedObjects = [m_externalObjectGraph objectForKey:owner.get()];
182182
if (!ownedObjects) {
183183
NSPointerFunctionsOptions weakIDOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
184184
NSPointerFunctionsOptions integerOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsIntegerPersonality;
185185
ownedObjects = adoptNS([[NSMapTable alloc] initWithKeyOptions:weakIDOptions valueOptions:integerOptions capacity:1]);
186186

187-
[m_externalObjectGraph setObject:ownedObjects.get() forKey:owner];
187+
[m_externalObjectGraph setObject:ownedObjects.get() forKey:owner.get()];
188188
}
189189

190-
size_t count = reinterpret_cast<size_t>(NSMapGet(ownedObjects.get(), (__bridge void*)object));
191-
NSMapInsert(ownedObjects.get(), (__bridge void*)object, reinterpret_cast<void*>(count + 1));
190+
size_t count = reinterpret_cast<size_t>(NSMapGet(ownedObjects.get(), (__bridge void*)object.get()));
191+
NSMapInsert(ownedObjects.get(), (__bridge void*)object.get(), reinterpret_cast<void*>(count + 1));
192192
}
193193
}
194194

195-
- (void)removeManagedReference:(id)object withOwner:(id)owner
195+
- (void)removeManagedReference:(id)objectArg withOwner:(id)ownerArg
196196
{
197197
@autoreleasepool {
198-
if ([object isKindOfClass:[JSManagedValue class]])
199-
[object didRemoveOwner:owner];
198+
if ([objectArg isKindOfClass:[JSManagedValue class]])
199+
[objectArg didRemoveOwner:ownerArg];
200200

201-
object = getInternalObjcObject(object);
202-
owner = getInternalObjcObject(owner);
201+
RetainPtr object = getInternalObjcObject(objectArg);
202+
RetainPtr owner = getInternalObjcObject(ownerArg);
203203

204204
if (!object || !owner)
205205
return;
206206

207207
JSC::JSLockHolder locker(toJS(m_group));
208208

209209
Locker externalDataMutexLocker { m_externalDataMutex };
210-
NSMapTable *ownedObjects = [m_externalObjectGraph objectForKey:owner];
210+
NSMapTable *ownedObjects = [m_externalObjectGraph objectForKey:owner.get()];
211211
if (!ownedObjects)
212212
return;
213213

214-
size_t count = reinterpret_cast<size_t>(NSMapGet(ownedObjects, (__bridge void*)object));
214+
size_t count = reinterpret_cast<size_t>(NSMapGet(ownedObjects, (__bridge void*)object.get()));
215215
if (count > 1) {
216-
NSMapInsert(ownedObjects, (__bridge void*)object, reinterpret_cast<void*>(count - 1));
216+
NSMapInsert(ownedObjects, (__bridge void*)object.get(), reinterpret_cast<void*>(count - 1));
217217
return;
218218
}
219219

220220
if (count == 1)
221-
NSMapRemove(ownedObjects, (__bridge void*)object);
221+
NSMapRemove(ownedObjects, (__bridge void*)object.get());
222222

223223
if (![ownedObjects count]) {
224-
[m_externalObjectGraph removeObjectForKey:owner];
225-
[m_externalRememberedSet removeObjectForKey:owner];
224+
[m_externalObjectGraph removeObjectForKey:owner.get()];
225+
[m_externalRememberedSet removeObjectForKey:owner.get()];
226226
}
227227
}
228228
}

‎Source/JavaScriptCore/API/JSWrapperMap.mm‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ static bool constructorHasInstance(JSContextRef ctx, JSObjectRef constructorRef,
166166
auto renameMap = adoptNS([[NSMutableDictionary alloc] init]);
167167

168168
forEachMethodInProtocol(protocol, NO, isInstanceMethod, ^(SEL sel, const char*){
169-
NSString *rename = @(sel_getName(sel));
169+
RetainPtr rename = @(sel_getName(sel));
170170
NSRange range = [rename rangeOfString:@"__JS_EXPORT_AS__"];
171171
if (range.location == NSNotFound)
172172
return;
@@ -255,22 +255,22 @@ static void copyMethodsToObject(JSContext *context, Class objcClass, Protocol *p
255255

256256
forEachMethodInProtocol(protocol, YES, isInstanceMethod, ^(SEL sel, const char* types){
257257
const char* nameCStr = sel_getName(sel);
258-
NSString *name = @(nameCStr);
258+
RetainPtr name = @(nameCStr);
259259

260-
if (shouldSkipMethodWithName(name))
260+
if (shouldSkipMethodWithName(name.get()))
261261
return;
262262

263-
if (accessorMethods && accessorMethods[name]) {
263+
if (accessorMethods && accessorMethods[name.get()]) {
264264
JSObjectRef method = objCCallbackFunctionForMethod(context, objcClass, protocol, isInstanceMethod, sel, types);
265265
if (!method)
266266
return;
267-
accessorMethods[name] = [JSValue valueWithJSValueRef:method inContext:context];
267+
accessorMethods[name.get()] = [JSValue valueWithJSValueRef:method inContext:context];
268268
} else {
269-
name = renameMap.get()[name];
269+
name = renameMap.get()[name.get()];
270270
if (!name)
271271
name = selectorToPropertyName(nameCStr);
272272
JSC::JSGlobalObject* globalObject = toJS([context JSGlobalContextRef]);
273-
JSValue *existingMethod = object[name];
273+
JSValue *existingMethod = object[name.get()];
274274
// ObjCCallbackFunction does a dynamic lookup for the
275275
// selector before calling the method. In order to save
276276
// memory we only put one callback object in any givin
@@ -282,7 +282,7 @@ static void copyMethodsToObject(JSContext *context, Class objcClass, Protocol *p
282282
return;
283283
JSObjectRef method = objCCallbackFunctionForMethod(context, objcClass, protocol, isInstanceMethod, sel, types);
284284
if (method)
285-
putNonEnumerable(context, object, name, [JSValue valueWithJSValueRef:method inContext:context]);
285+
putNonEnumerable(context, object, name.get(), [JSValue valueWithJSValueRef:method inContext:context]);
286286
}
287287
});
288288
}
@@ -368,15 +368,15 @@ static void copyPrototypeProperties(JSContext *context, Class objcClass, Protoco
368368
JSValue* getter = accessorMethods[property.getterName.get()];
369369
ASSERT(![getter isUndefined]);
370370

371-
JSValue* setter = undefined;
371+
RetainPtr setter = undefined;
372372
if (property.setterName) {
373373
setter = accessorMethods[property.setterName.get()];
374374
ASSERT(![setter isUndefined]);
375375
}
376-
376+
377377
[prototypeValue defineProperty:@(property.name) descriptor:@{
378378
JSPropertyDescriptorGetKey: getter,
379-
JSPropertyDescriptorSetKey: setter,
379+
JSPropertyDescriptorSetKey: setter.get(),
380380
JSPropertyDescriptorEnumerableKey: @NO,
381381
JSPropertyDescriptorConfigurableKey: @YES
382382
}];
@@ -431,9 +431,9 @@ - (void)dealloc
431431

432432
// For each protocol that the class implements, gather all of the init family methods into a hash table.
433433
__block UncheckedKeyHashMap<String, CFTypeRef> initTable;
434-
Protocol *exportProtocol = getJSExportProtocol();
435-
for (Class currentClass = cls; currentClass; currentClass = class_getSuperclass(currentClass)) {
436-
forEachProtocolImplementingProtocol(currentClass, exportProtocol, ^(Protocol *protocol, bool&) {
434+
RetainPtr exportProtocol = getJSExportProtocol();
435+
for (RetainPtr currentClass = cls; currentClass; currentClass = class_getSuperclass(currentClass.get())) {
436+
forEachProtocolImplementingProtocol(currentClass.get(), exportProtocol.get(), ^(Protocol *protocol, bool&) {
437437
forEachMethodInProtocol(protocol, YES, YES, ^(SEL selector, const char*) {
438438
const char* name = sel_getName(selector);
439439
if (!isInitFamilyMethod(@(name)))
@@ -443,12 +443,12 @@ - (void)dealloc
443443
});
444444
}
445445

446-
for (Class currentClass = cls; currentClass; currentClass = class_getSuperclass(currentClass)) {
446+
for (RetainPtr currentClass = cls; currentClass; currentClass = class_getSuperclass(currentClass.get())) {
447447
__block unsigned numberOfInitsFound = 0;
448448
__block SEL initMethod = 0;
449-
__block Protocol *initProtocol = 0;
449+
__block RetainPtr<Protocol> initProtocol;
450450
__block const char* types = 0;
451-
forEachMethodInClass(currentClass, ^(Method method) {
451+
forEachMethodInClass(currentClass.get(), ^(Method method) {
452452
SEL selector = method_getName(method);
453453
const char* name = sel_getName(selector);
454454
auto iter = initTable.find(String::fromLatin1(name));
@@ -470,7 +470,7 @@ - (void)dealloc
470470
break;
471471
}
472472

473-
JSObjectRef method = objCCallbackFunctionForInit(context, cls, initProtocol, initMethod, types);
473+
JSObjectRef method = objCCallbackFunctionForInit(context, cls, initProtocol.get(), initMethod, types);
474474
return toJS(method);
475475
}
476476
return constructorWithCustomBrand(context, [NSString stringWithFormat:@"%sConstructor", className], cls);
@@ -511,8 +511,8 @@ - (ConstructorPrototypePair)ensureConstructorAndPrototypeInContext:(JSContext *)
511511
putNonEnumerable(context, prototype, @"constructor", constructor);
512512
putNonEnumerable(context, constructor, @"prototype", prototype);
513513

514-
Protocol *exportProtocol = getJSExportProtocol();
515-
forEachProtocolImplementingProtocol(m_class, exportProtocol, ^(Protocol *protocol, bool&){
514+
RetainPtr exportProtocol = getJSExportProtocol();
515+
forEachProtocolImplementingProtocol(m_class, exportProtocol.get(), ^(Protocol *protocol, bool&) {
516516
copyPrototypeProperties(context, m_class, protocol, prototype);
517517
copyMethodsToObject(context, m_class, protocol, NO, constructor);
518518
});
@@ -714,14 +714,14 @@ bool supportsInitMethodConstructors()
714714

715715
Protocol *getJSExportProtocol()
716716
{
717-
static Protocol *protocol = objc_getProtocol("JSExport");
718-
return protocol;
717+
static NeverDestroyed<RetainPtr<Protocol>> protocol = objc_getProtocol("JSExport");
718+
return protocol->get();
719719
}
720720

721721
Class getNSBlockClass()
722722
{
723-
static Class cls = objc_getClass("NSBlock");
724-
return cls;
723+
static NeverDestroyed<RetainPtr<Class>> cls = objc_getClass("NSBlock");
724+
return cls->get();
725725
}
726726

727727
WTF_ALLOW_UNSAFE_BUFFER_USAGE_END

0 commit comments

Comments
 (0)