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

Can we make memory management bugs more debuggable? #1038

Closed
liamappelbe opened this issue Mar 22, 2024 · 3 comments · Fixed by #1111
Closed

Can we make memory management bugs more debuggable? #1038

liamappelbe opened this issue Mar 22, 2024 · 3 comments · Fixed by #1111
Labels
lang-objective_c Related to Objective C support package:ffigen type-enhancement A request for a change that isn't a bug

Comments

@liamappelbe
Copy link
Contributor

liamappelbe commented Mar 22, 2024

For example, if we try to take a reference to an ObjC object pointer after it has been freed, we get a crash with no stack trace. We can write some native code that will try to detect if an object is valid, then assert it's valid before calling retain. This is definitely possible for blocks (we have tests that do this already), so we just need to investigate if it's possible for objects. There will always be false negatives with a check like this, but an assertion that catches some/most of these mistakes would already be a huge improvement.

Ideally we'd add an assert for this to objectRetain, objectRelease, blockCopy, and blockRelease. The assert would check that the object is still alive and has a non-zero ref count.

See #1017

@liamappelbe liamappelbe added type-enhancement A request for a change that isn't a bug package:ffigen lang-objective_c Related to Objective C support labels Mar 22, 2024
@liamappelbe
Copy link
Contributor Author

Poked at the object represenation a bit today. It looks like the ObjC object header is 8 bytes:

Obj1, 1 ref: 011D800107A6B341  // Header of a test object
Obj2, 1 ref: 011D800107A6B341  // Same type as Obj1
Obj3, 1 ref: 011DFFF85303A0F1  // NSObject

Obj1, 2 refs: 021D800107A6B341
Obj2, 2 refs: 021D800107A6B341
Obj3, 2 refs: 021DFFF85303A0F1

Obj3, 254 refs: FE1DFFF85303A0F1
Obj3, 255 refs: FF1DFFF85303A0F1
Obj3, 256 refs: 809DFFF85303A0F1
Obj3, 257 refs: 819DFFF85303A0F1
Obj3, 1000 refs: E89DFFF85303A0F1
Obj3, 2000 refs: D09DFFF85303A0F1
Obj3, 3000 refs: B89DFFF85303A0F1
Obj3, 100000 refs:	A09DFFF85303A0F1

Obj1, dead: 00006DF707378C60
Obj2, dead: 00006DF70735E910
Obj3, dead: 00006DF707F5DE00

I think the first 9 bits encode the refcount. When the refcount is below 256, it's just stored in the first byte, and the second byte is 0x1D. When it hits 256 or higher, the 0x80 bit of the first two bytes is always on, and the exact count is stored in some way I haven't figured out. Since the ref count works correctly above 2^9, part of it must be stored elsewhere.

The latter 6 bytes of the header doesn't change with the refcount, and are specific to the type of the object. I'll call these the type ID. The test object's type ID was different each run, but NSObject's was always the same.

Once the object is deleted, the first 2 bytes is zeroed out, and the other bytes are randomized (or possibly some of them are set to a sentinel value, but that value varied between runs).

So I think the best I can do to check object validity at the moment is to check the first 2 bytes of the object, and verify they match either [0b0xxxxxxx, 0x1D] or [0b1xxxxxxx, 0x9D]. There's a 1/256 chance that random bytes will pass this test, but occasional false passes are ok.

If we wanted to be stricter, we could store a table of known type IDs. At runtime, the ffigen'd ObjC bindings would add the type ID of every class it has bindings for to the table. 2 downsides to this approach are:

  1. It's expensive. As well as storing the table, the only way I can think of to get the type ID is to allocate an object of that type and look at its header.
  2. ObjC could give us objects with type IDs we don't know about, and we'd mark them as invalid. This could legitimately happen in well formed bindings if the object we're given is a subclass of a class we do have bindings for. While false passes are acceptable, false fails are not.

@liamappelbe
Copy link
Contributor Author

This approach doesn't work for all objects. I tried the same thing with NSString, and in some cases the first two bytes were 0 for valid objects (eg the empty string). I think certain strings are being canonicalized. For this to work we would have to special case certain types, which is pretty brittle.

I think a better approach would be to dig through the ObjC runtime to look for functions that might help. _objc_getFreedObjectClass looked promising, but seems to just always return null on my mac. Maybe there's a function that can get the type ID/ISA/Class of an object that doesn't abort if the object is invalid (eg by just masking the object header).

@liamappelbe
Copy link
Contributor Author

The main issue with using these internal functions is that they're not well documented. I found the headers on my mac though, so for future reference, the directory is /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/objc/.

There's some functions in there that should work. object_getClass seems to extract the class by masking the header, like I wanted. Seems to be the latter 6 bytes of the header, minus a couple of bits:

NSObject header:  011DFFF85303A0F1
NSObject class:     0x7FF85303A0F0

It works for any dereferencable pointer or null, without aborting, including pointers to deleted objects. So then we just have to check whether the class is valid.

There's a function that looks like it might be able to verify the class, object_isClass, but it crashes if you pass it junk. The solution I found that works without crashing is to use objc_getClassList or objc_copyClassList to get all the classes, and then stick them in Set. This is similar to the alterative solution I suggested above, but cheaper and less brittle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-objective_c Related to Objective C support package:ffigen type-enhancement A request for a change that isn't a bug
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant