Skip to content
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.

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

fix: types with protocol qualified are not built-ins #451

Merged
merged 6 commits into from
Apr 29, 2024

Conversation

t0rr3sp3dr0
Copy link
Contributor

I made a mistake in #443 and the code detects things like id <OS_dispatch_queue> as built-ins. While id is a built-in, OS_dispatch_queue is not.

This patch addresses the problem by looking for built-ins only when we have an unqualified type. When we have a qualified type, we split the type in two and check both parts separately:

before, after, _ := strings.Cut(typ, "<")
o.fillImportsForType(before, className, classNames, protoNames, imp)
o.fillImportsForType("<"+after, className, classNames, protoNames, imp)

@blacktop
Copy link
Owner

Not sure this is a good test, because it's Foundation, but: (check out)

  • Foundation/NSAKDeserializerStream.h
  • Foundation/NSKeyedUnarchiver.h
  • Foundation/NSOperation.h
  • Foundation/NSOperationQueue.h
  • Foundation/NSXPCConnection.h
  • Foundation/NSXPCListener.h

Might be time to start using regep vs. strings.Contains

@t0rr3sp3dr0
Copy link
Contributor Author

Okay, we have two problems:

  1. We have a mix of Objective-C encoded types like @"NSObject<OS_dispatch_group>" and regular types like NSString *. Trying to parse both at the same time is not pretty straightforward.
  2. There is no exhaustive grammar for Objective-C type encoding besides the compiler implementation itself. I couldn't find a place that documents A or %, for example. The only thing we can 100% rely on is 14 thousand lines of C++.

@t0rr3sp3dr0
Copy link
Contributor Author

I believe to have addressed the problems you listed.

But, unrelated to this PR, I found that we are not representing some more obscure types properly.

  • ipsw Header:
@interface NSDecimalNumber : NSNumber {
    /* instance variables */
    unsigned int x :8 _exponent;
    unsigned int x :4 _length;
    unsigned int x :1 _isNegative;
    unsigned int x :1 _isCompact;
    unsigned int x :1 _reserved;
    unsigned int x :1 _hasExternalRefCount;
    unsigned int x :16 _refs;
    unsigned short * _mantissa;
}
  • Official Apple SDK Header:
@interface NSDecimalNumber : NSNumber {
@private
    signed   int _exponent:8;
    unsigned int _length:4;
    unsigned int _isNegative:1;
    unsigned int _isCompact:1;
    unsigned int _reserved:1;
    unsigned int _hasExternalRefCount:1;
    unsigned int _refs:16;
    unsigned short _mantissa[];
}

@t0rr3sp3dr0
Copy link
Contributor Author

Another example.

ipsw:

@interface _NSObserverList : NSObject {
    /* instance variables */
    id _owner;
    id _observers;
    atomic unsigned int _observerCount;
    struct os_unfair_lock_s { unsigned int _os_unfair_lock_opaque; } _observersLock;
}

Expected:

@interface _NSObserverList : NSObject {
    /* instance variables */
    id _owner;
    id _observers;
    _Atomic unsigned int _observerCount;
    struct os_unfair_lock_s { unsigned int _os_unfair_lock_opaque; } _observersLock;
}

@blacktop
Copy link
Owner

Another example.

ipsw:

@interface _NSObserverList : NSObject {
    /* instance variables */
    id _owner;
    id _observers;
    atomic unsigned int _observerCount;
    struct os_unfair_lock_s { unsigned int _os_unfair_lock_opaque; } _observersLock;
}

Expected:

@interface _NSObserverList : NSObject {
    /* instance variables */
    id _owner;
    id _observers;
    _Atomic unsigned int _observerCount;
    struct os_unfair_lock_s { unsigned int _os_unfair_lock_opaque; } _observersLock;
}

Is atomic not a valid alias for _Atomic ?

@blacktop
Copy link
Owner

I believe to have addressed the problems you listed.

But, unrelated to this PR, I found that we are not representing some more obscure types properly.

  • ipsw Header:
@interface NSDecimalNumber : NSNumber {
    /* instance variables */
    unsigned int x :8 _exponent;
    unsigned int x :4 _length;
    unsigned int x :1 _isNegative;
    unsigned int x :1 _isCompact;
    unsigned int x :1 _reserved;
    unsigned int x :1 _hasExternalRefCount;
    unsigned int x :16 _refs;
    unsigned short * _mantissa;
}
  • Official Apple SDK Header:
@interface NSDecimalNumber : NSNumber {
@private
    signed   int _exponent:8;
    unsigned int _length:4;
    unsigned int _isNegative:1;
    unsigned int _isCompact:1;
    unsigned int _reserved:1;
    unsigned int _hasExternalRefCount:1;
    unsigned int _refs:16;
    unsigned short _mantissa[];
}

I believed this is fixed in go-macho

@blacktop
Copy link
Owner

Another example.

ipsw:

@interface _NSObserverList : NSObject {
    /* instance variables */
    id _owner;
    id _observers;
    atomic unsigned int _observerCount;
    struct os_unfair_lock_s { unsigned int _os_unfair_lock_opaque; } _observersLock;
}

Expected:

@interface _NSObserverList : NSObject {
    /* instance variables */
    id _owner;
    id _observers;
    _Atomic unsigned int _observerCount;
    struct os_unfair_lock_s { unsigned int _os_unfair_lock_opaque; } _observersLock;
}

replaced atomic w/ _Atomic in go-macho

@t0rr3sp3dr0
Copy link
Contributor Author

_Atomic and atomic are actually different. The former is a C type qualifier and it is used with C types, while the latter is a Objective-C property attribute and cannot be used outside property definitions.

@t0rr3sp3dr0
Copy link
Contributor Author

Just resolved the conflicts, PTAL

@blacktop
Copy link
Owner

Just resolved the conflicts, PTAL

Can you also rebase from master to pull in the go-macho fixes? I'm still seeing a lot of weird things in those Foundation headers

@blacktop
Copy link
Owner

Foundation/NSConcreteFileHandle.h

@class out unsigned long long;

@blacktop
Copy link
Owner

Foundation/NSOperationQueue.h

@class __backingQueue"@"NSObject;

@blacktop
Copy link
Owner

I forgot to ask about this but in Foundation/NSProgressRegistrar-Protocol.h this PR removes: @protocol OS_dispatch_queue;

Maybe it doesn't make sense to have forward declarations of protocols in protocol headers?

@blacktop
Copy link
Owner

blacktop commented Apr 23, 2024

Foundation/NSXPCConnection.h

@class remote"@"OS_xpc_remote_connection");

and Foundation/NSXPCListener.h is similar

@t0rr3sp3dr0
Copy link
Contributor Author

I’ll try to address these problems this weekend.

@t0rr3sp3dr0
Copy link
Contributor Author

I'll have to fix some stuff in go-macho to make this work properly. I'll send the PR shortly.

@blacktop
Copy link
Owner

I'll have to fix some stuff in go-macho to make this work properly. I'll send the PR shortly.

merged your go-macho PR and cut a release 👍

@t0rr3sp3dr0
Copy link
Contributor Author

t0rr3sp3dr0 commented Apr 29, 2024

Okay, just tested against Foundation and the only thing weird I found was @class by copy id; in NSSpellServer and @class undefined; in multiple places. But that's because of go-macho as well.

I'm reviewing these maps and will submit a PR there. Beyond spaces in bycopy, byref, and oneway, there are other problems like int128 instead of __int128 and BOOL instead of _Bool. But I'm having a hard time validating these types:

	"z": "size_t",
	"Z": "int32",
	"w": "wchar_t",
	"%": "NXAtom",
	"!":  "vector",
	"Vv": "void",

I don't know if zZw%!V are legacy types but, with Xcode 15.3, I get unsupported type encoding spec when using them with [[NSMethodSignature signatureWithObjCTypes:s] debugDescription]. Do you know anything about them or have an example where they are used?

@blacktop
Copy link
Owner

Okay, just tested against Foundation and the only thing weird I found was @class by copy id; in NSSpellServer and @class undefined; in multiple places. But that's because of go-macho as well.

I'm reviewing these maps and will submit a PR there. Beyond spaces in bycopy, byref, and oneway, there are other problems like int128 instead of __int128 and BOOL instead of _Bool. But I'm having a hard time validating these types:

	"z": "size_t",
	"Z": "int32",
	"w": "wchar_t",
	"%": "NXAtom",
	"!":  "vector",
	"Vv": "void",

I don't know if zZw%!V are legacy types but, with Xcode 15.3, I get unsupported type encoding spec when using them with [[NSMethodSignature signatureWithObjCTypes:s] debugDescription]. Do you know anything about them or have an example where they are used?

I can't remember where I saw some of those originally, they were added 3-4yrs ago.

Do you know how 'gnu register' should be represented?

@blacktop
Copy link
Owner

@t0rr3sp3dr0
Copy link
Contributor Author

Take a look at blacktop/go-macho#52. I've removed Vv from there (because it should be oneway void) and left the other types untouched.

@blacktop
Copy link
Owner

I pushed some fixes including some that you mentioned to go-macho and the output is looking pretty good to me now 👍

@t0rr3sp3dr0
Copy link
Contributor Author

@t0rr3sp3dr0
Copy link
Contributor Author

Let me rebase my PR in go-macho, there are some changes worth picking there.

@blacktop
Copy link
Owner

Let me rebase my PR in go-macho, there are some changes worth picking there.

agreed

@blacktop
Copy link
Owner

@blacktop
Copy link
Owner

Tested a few of my fav dylibs and it looks good 👍

@blacktop blacktop merged commit 5ad4bc9 into blacktop:master Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants