diff --git a/Test/CMakeLists.txt b/Test/CMakeLists.txt index e6df343..8a530b1 100644 --- a/Test/CMakeLists.txt +++ b/Test/CMakeLists.txt @@ -41,6 +41,7 @@ set(TESTS WeakImportClass.m ivar_arc.m IVarOverlap.m + IVarSuperclassOverlap.m objc_msgSend.m msgInterpose.m NilException.m diff --git a/Test/IVarSuperclassOverlap.m b/Test/IVarSuperclassOverlap.m new file mode 100644 index 0000000..21beb2f --- /dev/null +++ b/Test/IVarSuperclassOverlap.m @@ -0,0 +1,97 @@ +#import "../objc/runtime.h" +#include "Test.h" + +@interface Space : Test +{ + uint16_t baseSmall; +} +@end + +@interface StartsWithChar : Space +{ + char c1; + char c2; + char c3; +} +@end +@interface StartsWithBitfield : Space +{ + @public + uint16_t b1:4; + uint16_t b2:4; + uint16_t b3:4; + uint16_t b4:4; + uint16_t notBitfield; +} +@end + +@implementation Space @end +@implementation StartsWithChar @end +@implementation StartsWithBitfield @end + +int main(int argc, char *argv[]) +{ + Class s = objc_getClass("Space"); + assert(s); + Class swc = objc_getClass("StartsWithChar"); + assert(swc); + Class swb = objc_getClass("StartsWithBitfield"); + assert(swb); + Ivar baseSmall = class_getInstanceVariable(s, "baseSmall"); + Ivar c1 = class_getInstanceVariable(swc, "c1"); + Ivar c2 = class_getInstanceVariable(swc, "c2"); + Ivar c3 = class_getInstanceVariable(swc, "c3"); + Ivar b1 = class_getInstanceVariable(swb, "b1"); + Ivar b2 = class_getInstanceVariable(swb, "b2"); + Ivar b3 = class_getInstanceVariable(swb, "b3"); + Ivar b4 = class_getInstanceVariable(swb, "b3"); + Ivar notBitfield = class_getInstanceVariable(swb, "notBitfield"); + assert(baseSmall); + assert(c1); + assert(c2); + assert(c3); + assert(b1); + assert(b2); + assert(b3); + StartsWithBitfield *swbi = [StartsWithBitfield new]; + + // Alternating 01 bit pattern, should catch small overwrites. + swbi->notBitfield = 0x5555; + swbi->b1 = 5; + swbi->b2 = 11; + swbi->b3 = 11; + swbi->b4 = 5; + assert(swbi->b1 == 5); + assert(swbi->b2 == 11); + assert(swbi->b3 == 11); + assert(swbi->b4 == 5); + assert(swbi->notBitfield == 0x5555); + swbi->notBitfield = 0xaaaa; + swbi->b1 = 5; + swbi->b2 = 11; + swbi->b3 = 5; + swbi->b4 = 11; + assert(swbi->notBitfield == 0xaaaa); + assert(swbi->b1 == 5); + assert(swbi->b2 == 11); + assert(swbi->b3 == 5); + assert(swbi->b4 == 11); + + ptrdiff_t baseSmallOffset = ivar_getOffset(baseSmall); +#ifdef NEW_ABI + // These should pass with the old ABI, but they don't at the moment. The + // way that they don't is not very harmful though: we just get a bit of + // redundant padding, so I don't consider a fix a very high priority. + assert(ivar_getOffset(c1) == baseSmallOffset + 2); + assert(ivar_getOffset(c2) == baseSmallOffset + 3); + assert(ivar_getOffset(c3) == baseSmallOffset + 4); + assert(ivar_getOffset(b1) == baseSmallOffset + 2); + assert(ivar_getOffset(b2) == baseSmallOffset + 2); + assert(ivar_getOffset(b3) == baseSmallOffset + 3); + assert(ivar_getOffset(b4) == baseSmallOffset + 3); + assert(ivar_getOffset(notBitfield) == baseSmallOffset + 4); +#endif + + + return 0; +} diff --git a/Test/zeroSizedIVar.m b/Test/zeroSizedIVar.m index 7b80caa..80690b4 100644 --- a/Test/zeroSizedIVar.m +++ b/Test/zeroSizedIVar.m @@ -56,7 +56,7 @@ int main() Ivar flag3 = class_getInstanceVariable(bitfield, "flag3"); assert(flag3); assert(ivar_getOffset(flag3) == sizeof(id)); - assert(ivar_getOffset(flag3) + sizeof(int) <= class_getInstanceSize(bitfield)); + assert(ivar_getOffset(flag3) + sizeof(BOOL) <= class_getInstanceSize(bitfield)); bitfield = objc_getClass("BitfieldTest2"); flag1 = class_getInstanceVariable(bitfield, "flag1"); diff --git a/ivar.c b/ivar.c index a1311b0..92af800 100644 --- a/ivar.c +++ b/ivar.c @@ -65,22 +65,13 @@ PRIVATE void objc_compute_ivar_offsets(Class class) // If the first instance variable had any alignment padding, then we need // to discard it. We will recompute padding ourself later. long next_ivar = ivar_start; - long last_offset = -1; + long last_offset = LONG_MIN; + long last_size = 0; long last_computed_offset = -1; size_t refcount_size = isGCEnabled ? 0 : sizeof(uintptr_t); for (i = 0 ; i < class->ivars->count ; i++) { struct objc_ivar *ivar = ivar_at_index(class->ivars, i); - // The compiler may generate negative offsets for the first - // instance variable, because the last instance variable falls - // within the space allocated for the last instance variable. - // We are completely recomputing the instance size, so we - // simply discard this and recompute the padding ourselves. - if (*ivar->offset < 0) - { - assert(i == 0); - *ivar->offset = ivar_start; - } // Clang 7 and 8 have a bug where the size of _Bool is encoded // as 0, not 1. Silently fix this up when we see it. if (ivar->size == 0 && ivar->type[0] == 'B') @@ -93,18 +84,20 @@ PRIVATE void objc_compute_ivar_offsets(Class class) // then we will need to ensure that we are properly aligned again. long ivar_size = ivar->size; // Bitfields have the same offset - the base of the variable - // that contains them. If we are in a bitfield, - if (*ivar->offset == last_offset) - { - *ivar->offset = last_computed_offset; - } - else + // that contains them. If we are in a bitfield, then we need + // to make sure that we don't add any displacement from the + // previous value. + if (*ivar->offset < last_offset + last_size) { - last_offset = *ivar->offset; - *ivar->offset = next_ivar; - last_computed_offset = *ivar->offset; - next_ivar += ivar_size; + *ivar->offset = last_computed_offset + (*ivar->offset - last_offset); + ivar_size = 0; + continue; } + last_offset = *ivar->offset; + *ivar->offset = next_ivar; + last_computed_offset = *ivar->offset; + next_ivar += ivar_size; + last_size = ivar->size; size_t align = ivarGetAlign(ivar); if ((*ivar->offset + refcount_size) % align != 0) {