Changeset 221723 in webkit for trunk/Source/JavaScriptCore/API

Timestamp:

Sep 7, 2017, 1:14:30 AM (8 years ago)

Author:

Yusuke Suzuki

Message:

[JSC] Remove "malloc" and "free" from JSC/API
​https://bugs.webkit.org/show_bug.cgi?id=176331

Reviewed by Keith Miller.

Remove "malloc" and "free" manual calls in JSC/API.

• API/JSValue.mm:

(createStructHandlerMap):

• API/JSWrapperMap.mm:

(parsePropertyAttributes):
(makeSetterName):
(copyPrototypeProperties):
Use RetainPtr<NSString> to keep NSString. We avoid repeated "char*" to "NSString" conversion.

• API/ObjcRuntimeExtras.h:

(adoptSystem):
Add adoptSystem to automate calling system free().

(protocolImplementsProtocol):
(forEachProtocolImplementingProtocol):
(forEachMethodInClass):
(forEachMethodInProtocol):
(forEachPropertyInProtocol):
(StringRange::StringRange):
(StringRange::operator const char* const):
(StringRange::get const):
Use CString for backend.

(StructBuffer::StructBuffer):
(StructBuffer::~StructBuffer):
(StringRange::~StringRange): Deleted.
Use fastAlignedMalloc/astAlignedFree to get aligned memory.

Location:

trunk/Source/JavaScriptCore/API

Files:

• JSValue.mm (modified) (2 diffs)
• JSWrapperMap.mm (modified) (6 diffs)
• ObjcRuntimeExtras.h (modified) (8 diffs)

trunk/Source/JavaScriptCore/API/JSValue.mm

@@ -1055 +1055 @@
         char idType[3];
         // Check 2nd argument type is "@"
-        char* secondType = method_copyArgumentType(method, 3);
-        if (strcmp(secondType, "@") != 0) {
-            free(secondType);
-            return;
+        {
+            auto secondType = adoptSystem<char[]>(method_copyArgumentType(method, 3));
+            if (strcmp(secondType.get(), "@") != 0)
+                return;
         }
-        free(secondType);
         // Check result type is also "@"
         method_getReturnType(method, idType, 3);
         if (strcmp(idType, "@") != 0)
             return;
-        char* type = method_copyArgumentType(method, 2);
-        structHandlers->add(StringImpl::create(type), (StructTagHandler){ selector, 0 });
-        free(type);
+        {
+            auto type = adoptSystem<char[]>(method_copyArgumentType(method, 2));
+            structHandlers->add(StringImpl::create(type.get()), (StructTagHandler) { selector, 0 });
+        }
     });
 
@@ -1082 +1082 @@
             return;
         // Try to find a matching valueWith<Foo>:context: method.
-        char* type = method_copyReturnType(method);
-
-        StructHandlers::iterator iter = structHandlers->find(type);
-        free(type);
+        auto type = adoptSystem<char[]>(method_copyReturnType(method));
+        StructHandlers::iterator iter = structHandlers->find(type.get());
         if (iter == structHandlers->end())
             return;

trunk/Source/JavaScriptCore/API/JSWrapperMap.mm

@@ -92 +92 @@
         // Copy the character, converting to upper case if necessary.
         // If the character we copy is '\0', then we're done!
-        if (!(*(output++) = toupper(c)))
+        if (!(*(output++) = toASCIIUpper(c)))
             goto done;
         // Loop over characters other than ':'.
@@ -261 +261 @@
 }
 
-static bool parsePropertyAttributes(objc_property_t property, char*& getterName, char*& setterName)
+struct Property {
+    const char* name;
+    RetainPtr<NSString> getterName;
+    RetainPtr<NSString> setterName;
+};
+
+static bool parsePropertyAttributes(objc_property_t objcProperty, Property& property)
 {
     bool readonly = false;
     unsigned attributeCount;
-    objc_property_attribute_t* attributes = property_copyAttributeList(property, &attributeCount);
+    auto attributes = adoptSystem<objc_property_attribute_t[]>(property_copyAttributeList(objcProperty, &attributeCount));
     if (attributeCount) {
         for (unsigned i = 0; i < attributeCount; ++i) {
             switch (*(attributes[i].name)) {
             case 'G':
-                getterName = strdup(attributes[i].value);
+                property.getterName = @(attributes[i].value);
                 break;
             case 'S':
-                setterName = strdup(attributes[i].value);
+                property.setterName = @(attributes[i].value);
                 break;
             case 'R':
@@ -282 +288 @@
             }
         }
-        free(attributes);
     }
     return readonly;
 }
 
-static char* makeSetterName(const char* name)
+static RetainPtr<NSString> makeSetterName(const char* name)
 {
     size_t nameLength = strlen(name);
-    char* setterName = (char*)malloc(nameLength + 5); // "set" Name ":\0"
-    setterName[0] = 's';
-    setterName[1] = 'e';
-    setterName[2] = 't';
-    setterName[3] = toupper(*name);
-    memcpy(setterName + 4, name + 1, nameLength - 1);
-    setterName[nameLength + 3] = ':';
-    setterName[nameLength + 4] = '\0';
-    return setterName;
+    // "set" Name ":\0"  => nameLength + 5.
+    Vector<char, 128> buffer(nameLength + 5);
+    buffer[0] = 's';
+    buffer[1] = 'e';
+    buffer[2] = 't';
+    buffer[3] = toASCIIUpper(*name);
+    memcpy(buffer.data() + 4, name + 1, nameLength - 1);
+    buffer[nameLength + 3] = ':';
+    buffer[nameLength + 4] = '\0';
+    return @(buffer.data());
 }
 
@@ -304 +310 @@
 {
     // First gather propreties into this list, then handle the methods (capturing the accessor methods).
-    struct Property {
-        const char* name;
-        char* getterName;
-        char* setterName;
-    };
     __block Vector<Property> propertyList;
 
@@ -317 +318 @@
     JSValue *undefined = [JSValue valueWithUndefinedInContext:context];
 
-    forEachPropertyInProtocol(protocol, ^(objc_property_t property){
-        char* getterName = 0;
-        char* setterName = 0;
-        bool readonly = parsePropertyAttributes(property, getterName, setterName);
-        const char* name = property_getName(property);
-
-        // Add the names of the getter & setter methods to 
-        if (!getterName)
-            getterName = strdup(name);
-        accessorMethods[@(getterName)] = undefined;
+    forEachPropertyInProtocol(protocol, ^(objc_property_t objcProperty) {
+        const char* name = property_getName(objcProperty);
+        Property property { name, nullptr, nullptr };
+        bool readonly = parsePropertyAttributes(objcProperty, property);
+
+        // Add the names of the getter & setter methods to
+        if (!property.getterName)
+            property.getterName = @(name);
+        accessorMethods[property.getterName.get()] = undefined;
         if (!readonly) {
-            if (!setterName)
-                setterName = makeSetterName(name);
-            accessorMethods[@(setterName)] = undefined;
+            if (!property.setterName)
+                property.setterName = makeSetterName(name);
+            accessorMethods[property.setterName.get()] = undefined;
         }
 
         // Add the properties to a list.
-        propertyList.append((Property){ name, getterName, setterName });
+        propertyList.append(WTFMove(property));
     });
 
@@ -341 +341 @@
 
     // Iterate the propertyList & generate accessor properties.
-    for (size_t i = 0; i < propertyList.size(); ++i) {
-        Property& property = propertyList[i];
-
-        JSValue *getter = accessorMethods[@(property.getterName)];
-        free(property.getterName);
+    for (auto& property : propertyList) {
+        JSValue* getter = accessorMethods[property.getterName.get()];
         ASSERT(![getter isUndefined]);
 
-        JSValue *setter = undefined;
+        JSValue* setter = undefined;
         if (property.setterName) {
-            setter = accessorMethods[@(property.setterName)];
-            free(property.setterName);
+            setter = accessorMethods[property.setterName.get()];
             ASSERT(![setter isUndefined]);
         }

trunk/Source/JavaScriptCore/API/ObjcRuntimeExtras.h

@@ -24 +24 @@
  */
 
+#import <memory>
 #import <objc/Protocol.h>
 #import <objc/runtime.h>
 #import <wtf/HashSet.h>
+#import <wtf/SystemFree.h>
 #import <wtf/Vector.h>
+#import <wtf/text/CString.h>
+
+template<typename T, typename U>
+inline std::unique_ptr<T, WTF::SystemFree<T>> adoptSystem(U value)
+{
+    return std::unique_ptr<T, WTF::SystemFree<T>>(value);
+}
 
 inline bool protocolImplementsProtocol(Protocol *candidate, Protocol *target)
 {
     unsigned protocolProtocolsCount;
-    Protocol ** protocolProtocols = protocol_copyProtocolList(candidate, &protocolProtocolsCount);
+    auto protocolProtocols = adoptSystem<Protocol*[]>(protocol_copyProtocolList(candidate, &protocolProtocolsCount));
     for (unsigned i = 0; i < protocolProtocolsCount; ++i) {
-        if (protocol_isEqual(protocolProtocols[i], target)) {
-            free(protocolProtocols);
+        if (protocol_isEqual(protocolProtocols[i], target))
             return true;
-        }
-    }
-    free(protocolProtocols);
+    }
     return false;
 }
@@ -48 +54 @@
     ASSERT(target);
 
-    Vector<Protocol *> worklist;
+    Vector<Protocol*> worklist;
     HashSet<void*> visited;
 
     // Initially fill the worklist with the Class's protocols.
-    unsigned protocolsCount;
-    Protocol ** protocols = class_copyProtocolList(cls, &protocolsCount);
-    worklist.append(protocols, protocolsCount);
-    free(protocols);
+    {
+        unsigned protocolsCount;
+        auto protocols = adoptSystem<Protocol*[]>(class_copyProtocolList(cls, &protocolsCount));
+        worklist.append(protocols.get(), protocolsCount);
+    }
 
     bool stop = false;
@@ -74 +81 @@
 
         // Add incorporated protocols to the worklist.
-        protocols = protocol_copyProtocolList(protocol, &protocolsCount);
-        worklist.append(protocols, protocolsCount);
-        free(protocols);
+        {
+            unsigned protocolsCount;
+            auto protocols = adoptSystem<Protocol*[]>(protocol_copyProtocolList(protocol, &protocolsCount));
+            worklist.append(protocols.get(), protocolsCount);
+        }
     }
 }
@@ -83 +92 @@
 {
     unsigned count;
-    Method* methods = class_copyMethodList(cls, &count);
+    auto methods = adoptSystem<Method[]>(class_copyMethodList(cls, &count));
     for (unsigned i = 0; i < count; ++i)
         callback(methods[i]);
-    free(methods);
 }
 
@@ -92 +100 @@
 {
     unsigned count;
-    struct objc_method_description* methods = protocol_copyMethodDescriptionList(protocol, isRequiredMethod, isInstanceMethod, &count);
+    auto methods = adoptSystem<objc_method_description[]>(protocol_copyMethodDescriptionList(protocol, isRequiredMethod, isInstanceMethod, &count));
     for (unsigned i = 0; i < count; ++i)
         callback(methods[i].name, methods[i].types);
-    free(methods);
 }
 
@@ -101 +108 @@
 {
     unsigned count;
-    objc_property_t* properties = protocol_copyPropertyList(protocol, &count);
+    auto properties = adoptSystem<objc_property_t[]>(protocol_copyPropertyList(protocol, &count));
     for (unsigned i = 0; i < count; ++i)
         callback(properties[i]);
-    free(properties);
 }
 
@@ -125 +131 @@
     WTF_MAKE_NONCOPYABLE(StringRange);
 public:
-    StringRange(const char* begin, const char* end) : m_ptr(strndup(begin, end - begin)) { }
-    ~StringRange() { free(m_ptr); }
-    operator const char*() const { return m_ptr; }
-    const char* get() const { return m_ptr; }
+    StringRange(const char* begin, const char* end)
+        : m_string(begin, end - begin)
+    { }
+    operator const char*() const { return m_string.data(); }
+    const char* get() const { return m_string.data(); }
 
 private:
-    char* m_ptr;
+    CString m_string;
 };
 
@@ -141 +148 @@
         NSUInteger size, alignment;
         NSGetSizeAndAlignment(encodedType, &size, &alignment);
-        --alignment;
-        m_allocation = static_cast<char*>(malloc(size + alignment));
-        m_buffer = reinterpret_cast<char*>((reinterpret_cast<intptr_t>(m_allocation) + alignment) & ~alignment);
-    }
-
-    ~StructBuffer() { free(m_allocation); }
+        m_buffer = fastAlignedMalloc(alignment, size);
+    }
+
+    ~StructBuffer() { fastAlignedFree(m_buffer); }
     operator void*() const { return m_buffer; }
 
 private:
-    void* m_allocation;
     void* m_buffer;
 };