Rework some of the ivar offset calculations.

We now correctly handle ivars that overlap with the end of what the
compiler thinks is the start of the superclass and bitfields at the
start of a class.

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

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;
+}

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");

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, 
+				// that contains them.  If we are in a bitfield, then we need
-				if (*ivar->offset == last_offset)
+				// to make sure that we don't add any displacement from the
-				{
+				// previous value.
-					*ivar->offset = last_computed_offset;
+				if (*ivar->offset < last_offset + last_size)
-				}
-				else
 				{
-					last_offset = *ivar->offset;
+					*ivar->offset = last_computed_offset + (*ivar->offset - last_offset);
-					*ivar->offset = next_ivar;
+					ivar_size = 0;
-					last_computed_offset = *ivar->offset;
+					continue;
-					next_ivar += ivar_size;
 				}
+				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)
 				{