Skip to content

Cast IMPs to the correct signature before calling #418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
HendrikHuebner wants to merge 1 commit into
base: master
Choose a base branch
from
+25 −14
Open

Cast IMPs to the correct signature before calling #418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions arc.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,12 @@ static TLS_CALLBACK(cleanupPools)(struct arc_tls* tls)


static Class AutoreleasePool;
static IMP NewAutoreleasePool;
static IMP DeleteAutoreleasePool;
static IMP AutoreleaseAdd;
typedef id (*NewAutoreleasePoolIMP)(id, SEL);
typedef void (*DeleteAutoreleasePoolIMP)(id, SEL);
typedef void (*AutoreleaseAddIMP)(id, SEL, id);
static NewAutoreleasePoolIMP NewAutoreleasePool;
static DeleteAutoreleasePoolIMP DeleteAutoreleasePool;
static AutoreleaseAddIMP AutoreleaseAdd;
Comment on lines +253 to +258

@davidchisnall davidchisnall Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about something like this:

Suggested change
typedef id (*NewAutoreleasePoolIMP)(id, SEL);
typedef void (*DeleteAutoreleasePoolIMP)(id, SEL);
typedef void (*AutoreleaseAddIMP)(id, SEL, id);
static NewAutoreleasePoolIMP NewAutoreleasePool;
static DeleteAutoreleasePoolIMP DeleteAutoreleasePool;
static AutoreleaseAddIMP AutoreleaseAdd;
template<typename Return, typename... Arguments>
using Selector = Return(*)(id, SEL, Arguments...);
static Selector<id> NewAutoreleasePool;
static Selector<void> DeleteAutoreleasePoolIMP DeleteAutoreleasePool;
static Selector<void, id> AutoreleaseAdd;


static BOOL useARCAutoreleasePool;

Expand Down Expand Up @@ -566,12 +569,15 @@ static inline void initAutorelease(void)
if (!useARCAutoreleasePool)
{
[AutoreleasePool class];
NewAutoreleasePool = class_getMethodImplementation(object_getClass(AutoreleasePool),
SELECTOR(new));
DeleteAutoreleasePool = class_getMethodImplementation(AutoreleasePool,
SELECTOR(release));
AutoreleaseAdd = class_getMethodImplementation(object_getClass(AutoreleasePool),
SELECTOR(addObject:));
NewAutoreleasePool = reinterpret_cast<NewAutoreleasePoolIMP>(
class_getMethodImplementation(object_getClass(AutoreleasePool),
SELECTOR(new)));
Comment on lines +572 to +574

@davidchisnall davidchisnall Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we consolidate the casts with something like:

auto storeSelector = [](auto &target, IMP imp)
    {
         target = reinterpret_cast<std::remove_reference_t<decltype(target)>>(imp);
    };

DeleteAutoreleasePool = reinterpret_cast<DeleteAutoreleasePoolIMP>(
class_getMethodImplementation(AutoreleasePool,
SELECTOR(release)));
AutoreleaseAdd = reinterpret_cast<AutoreleaseAddIMP>(
class_getMethodImplementation(object_getClass(AutoreleasePool),
SELECTOR(addObject:)));
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions dtable.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,7 @@ LEGACY void update_dispatch_table_for_class(Class cls)
}

BOOL objc_resolve_class(Class);
typedef void (*InitializeIMP)(id, SEL);

__attribute__((unused)) static void objc_release_object_lock(id *x)
{
Expand Down Expand Up @@ -851,6 +852,6 @@ OBJC_PUBLIC void objc_send_initialize(id object)
// Store the buffer in the temporary dtables list. Note that it is safe to
// insert it into a global list, even though it's a temporary variable,
// because we will clean it up after this function.
initializeSlot->imp((id)class, initializeSel);
InitializeIMP initialize = (InitializeIMP)initializeSlot->imp;
initialize((id)class, initializeSel);
}

10 changes: 7 additions & 3 deletions runtime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@

#define CHECK_ARG(arg) if (0 == arg) { return 0; }

typedef void (*CXXDestructIMP)(id, SEL);
typedef id (*CXXConstructIMP)(id, SEL);

static inline void safe_remove_from_subclass_list(Class cls);
PRIVATE BOOL objc_resolve_class(Class);
void objc_send_initialize(id object);
Expand All @@ -46,7 +49,8 @@ PRIVATE void call_cxx_destruct(id obj)
cls = cls->super_class;
if (currentClass->cxx_destruct)

@davidchisnall davidchisnall Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably change the type of these from IMP and do the cast when we set them.

{
currentClass->cxx_destruct(obj, cxx_destruct);
CXXDestructIMP destruct = (CXXDestructIMP)currentClass->cxx_destruct;
destruct(obj, cxx_destruct);
}
}
}
Expand All @@ -65,7 +69,8 @@ static void call_cxx_construct_for_class(Class cls, id obj)
}
if (cls->cxx_construct)
{
cls->cxx_construct(obj, cxx_construct);
CXXConstructIMP construct = (CXXConstructIMP)cls->cxx_construct;
construct(obj, cxx_construct);
}
}

Expand Down Expand Up @@ -846,4 +851,3 @@ void objc_registerClassPair(Class cls)
class_table_insert(cls);
objc_resolve_class(cls);
}