From 386dd6d3be780106d43c6c9195042db86e316e03 Mon Sep 17 00:00:00 2001
From: David Chisnall <github@theravensnest.org>
Date: Sat, 29 Nov 2025 12:02:43 +0000
Subject: [PATCH] Fix offset calculation for ivar bitfields.

In some cases, we were allowing ivars declared as bitfields to overlap
other ivars, which caused memory corruption.

This showed up in OOlite.
---
 Test/CMakeLists.txt          |  1 +
 Test/IVarSuperclassOverlap.m | 15 ++++++++--
 Test/bitfield.m              | 57 ++++++++++++++++++++++++++++++++++++
 ivar.c                       |  8 ++---
 4 files changed, 74 insertions(+), 7 deletions(-)
 create mode 100644 Test/bitfield.m

diff --git a/Test/CMakeLists.txt b/Test/CMakeLists.txt
index 3aa27b73..1f66e325 100644
--- a/Test/CMakeLists.txt
+++ b/Test/CMakeLists.txt
@@ -13,6 +13,7 @@ endif ()
 set(TESTS
 	alias.m
 	alignTest.m
+	bitfield.m
 	AllocatePair.m
 	AssociatedObject.m
 	AssociatedObject2.m
diff --git a/Test/IVarSuperclassOverlap.m b/Test/IVarSuperclassOverlap.m
index 3a10f3f8..7e14b19f 100644
--- a/Test/IVarSuperclassOverlap.m
+++ b/Test/IVarSuperclassOverlap.m
@@ -89,9 +89,18 @@ int main(int argc, char *argv[])
 	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);
+	// These could be tighter, but the way that clang currently emits ivars for
+	// bitfields is a bit odd.  Unfortunately, it sometimes adds a small
+	// displacement so that it can load individual words, but the metadata
+	// doesn't tell us anything about them.  Fixing this would require using an
+	// extra metadata flag or similar to indicate that the size of the ivar is
+	// not the storage size.
+	assert((ivar_getOffset(b3) == baseSmallOffset + 3) ||
+	       (ivar_getOffset(b3) == baseSmallOffset + 4));
+	assert((ivar_getOffset(b4) == baseSmallOffset + 3) ||
+	       (ivar_getOffset(b4) == baseSmallOffset + 4));
+	assert((ivar_getOffset(notBitfield) == baseSmallOffset + 4) ||
+	       (ivar_getOffset(notBitfield) == baseSmallOffset + 6));
 #endif
 
 
diff --git a/Test/bitfield.m b/Test/bitfield.m
new file mode 100644
index 00000000..c9bb02be
--- /dev/null
+++ b/Test/bitfield.m
@@ -0,0 +1,57 @@
+#include "stdio.h"
+#include "Test.h"
+#include "objc/runtime.h"
+#include "objc/encoding.h"
+
+@interface Bitfield : Test
+{
+	unsigned short first;
+	unsigned  isShip: 1,
+	          isStation: 1;
+	unsigned y;
+}
+
+@end
+@implementation Bitfield
+- (BOOL)isShip { return isShip; }
+- (BOOL)isStation { return isStation; }
+- (void)setShip: (BOOL)aValue
+{
+	isShip = aValue;
+}
+- (void)setStation: (BOOL)aValue
+{
+	isStation = aValue;
+}
+- (void)setY: (int)anInt
+{
+	y = anInt;
+}
+@end
+
+static size_t offset(const char *ivar)
+{
+	return ivar_getOffset(class_getInstanceVariable([Bitfield class], ivar));
+}
+
+static size_t size(const char *ivar)
+{
+	return objc_sizeof_type(ivar_getTypeEncoding(class_getInstanceVariable([Bitfield class], ivar)));
+}
+
+int main(void)
+{
+	Bitfield *bf = [Bitfield new];
+	assert(![bf isShip]);
+	assert(![bf isStation]);
+	[bf setShip: YES];
+	assert([bf isShip]);
+	assert(![bf isStation]);
+	[bf setStation: YES];
+	[bf setY: 0];
+	assert([bf isShip]);
+	assert([bf isStation]);
+	assert(offset("isShip") >= offset("first") + size("first"));
+	assert(offset("isShip") == offset("isStation"));
+	assert(offset("y") >= offset("isStation") + size("isStation"));
+}
diff --git a/ivar.c b/ivar.c
index 5391a921..ac172fe0 100644
--- a/ivar.c
+++ b/ivar.c
@@ -85,18 +85,17 @@ PRIVATE void objc_compute_ivar_offsets(Class class)
 				// 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)
+				if ((i != 0) && (*ivar->offset == last_offset))
 				{
-					*ivar->offset = last_computed_offset + (*ivar->offset - last_offset);
-					ivar_size = 0;
+					*ivar->offset = last_computed_offset;
 					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 the alignment is insufficient, round it up.
 				if ((*ivar->offset + refcount_size) % align != 0)
 				{
 					long padding = align - ((*ivar->offset + refcount_size) % align); 
@@ -104,6 +103,7 @@ PRIVATE void objc_compute_ivar_offsets(Class class)
 					class->instance_size += padding;
 					next_ivar += padding;
 				}
+				last_computed_offset = *ivar->offset;
 				assert((*ivar->offset + sizeof(uintptr_t)) % ivarGetAlign(ivar) == 0);
 				class->instance_size += ivar_size;
 			}