Skip to content

Commit c20c6fa

Browse files
committed
Better fix: use offsetof
1 parent 876e4c5 commit c20c6fa

3 files changed

Lines changed: 19 additions & 37 deletions

File tree

mypyc/codegen/emitfunc.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -809,9 +809,12 @@ def visit_get_element_ptr(self, op: GetElementPtr) -> None:
809809
# TODO: support tuple type
810810
assert isinstance(op.src_type, RStruct), op.src_type
811811
assert op.field in op.src_type.names, "Invalid field name."
812+
# Use offsetof to avoid undefined behavior when src is NULL
813+
# (e.g., vec buf pointer for empty vecs). The &((T*)p)->field
814+
# pattern is UB when p is NULL, which GCC -O3 can exploit.
812815
self.emit_line(
813-
"{} = ({})&(({} *){})->{};".format(
814-
dest, op.type._ctype, op.src_type.name, src, op.field
816+
"{} = ({})((CPyPtr){} + offsetof({}, {}));".format(
817+
dest, op.type._ctype, src, op.src_type.name, op.field
815818
)
816819
)
817820

mypyc/irbuild/vec.py

Lines changed: 8 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -145,16 +145,6 @@ def vec_create_initialized(
145145
init = builder.coerce(init, item_type, line)
146146
vec = vec_create(builder, vtype, length, line)
147147

148-
# Guard against computing &buf->items when buf is NULL (length == 0).
149-
# vec_create returns buf=NULL for empty vecs, and accessing members
150-
# through a NULL pointer is undefined behavior that GCC -O3 can exploit.
151-
fill_body = BasicBlock()
152-
fill_end = BasicBlock()
153-
zero = Integer(0, c_pyssize_t_rprimitive)
154-
comp = builder.add(ComparisonOp(length, zero, ComparisonOp.SGT, line=line))
155-
builder.add(Branch(comp, fill_body, fill_end, Branch.BOOL))
156-
builder.activate_block(fill_body)
157-
158148
items_start = vec_items(builder, vec)
159149
step = step_size(item_type)
160150
items_end = builder.int_add(items_start, builder.int_mul(length, step))
@@ -165,9 +155,6 @@ def vec_create_initialized(
165155
builder.set_mem(for_loop.index, item_type, init)
166156
for_loop.finish()
167157

168-
builder.goto(fill_end)
169-
builder.activate_block(fill_end)
170-
171158
builder.keep_alive([vec], line)
172159
return vec
173160

@@ -176,13 +163,12 @@ def vec_create_from_values(
176163
builder: LowLevelIRBuilder, vtype: RVec, values: list[Value], line: int
177164
) -> Value:
178165
vec = vec_create(builder, vtype, len(values), line)
179-
if values:
180-
ptr = vec_items(builder, vec)
181-
item_type = vtype.item_type
182-
step = step_size(item_type)
183-
for value in values:
184-
builder.set_mem(ptr, item_type, value)
185-
ptr = builder.int_add(ptr, step)
166+
ptr = vec_items(builder, vec)
167+
item_type = vtype.item_type
168+
step = step_size(item_type)
169+
for value in values:
170+
builder.set_mem(ptr, item_type, value)
171+
ptr = builder.int_add(ptr, step)
186172
builder.keep_alive([vec], line)
187173
return vec
188174

@@ -238,9 +224,8 @@ def vec_len_native(builder: LowLevelIRBuilder, val: Value) -> Value:
238224
def vec_items(builder: LowLevelIRBuilder, vecobj: Value) -> Value:
239225
"""Return pointer to first item in vec's buf.
240226
241-
The caller must ensure buf is not NULL (i.e., vec is non-empty).
242-
Empty vecs have buf=NULL, and computing &NULL->items is undefined
243-
behavior that GCC -O3 can exploit to miscompile surrounding code.
227+
Safe to call even when buf is NULL (empty vec), since GetElementPtr
228+
uses offsetof-based arithmetic instead of &((T*)p)->field.
244229
"""
245230
vtype = cast(RVec, vecobj.type)
246231
buf = builder.get_element(vecobj, "buf")
@@ -507,14 +492,6 @@ def vec_contains(builder: LowLevelIRBuilder, vec: Value, target: Value, line: in
507492

508493
true, end = BasicBlock(), BasicBlock()
509494

510-
# Guard against computing &buf->items when buf is NULL (empty vec).
511-
search_body = BasicBlock()
512-
search_done = BasicBlock()
513-
zero = Integer(0, c_pyssize_t_rprimitive)
514-
len_check = builder.add(ComparisonOp(len_val, zero, ComparisonOp.SGT, line=line))
515-
builder.add(Branch(len_check, search_body, search_done, Branch.BOOL))
516-
builder.activate_block(search_body)
517-
518495
items_start = vec_items(builder, vec)
519496
items_end = builder.int_add(items_start, builder.int_mul(len_val, step))
520497

@@ -529,7 +506,6 @@ def vec_contains(builder: LowLevelIRBuilder, vec: Value, target: Value, line: in
529506
for_loop.finish()
530507

531508
builder.keep_alive([vec], line)
532-
builder.goto_and_activate(search_done)
533509

534510
res = Register(bool_rprimitive)
535511
builder.assign(res, Integer(0, bool_rprimitive))

mypyc/test/test_emitfunc.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -760,13 +760,16 @@ def test_get_element_ptr(self) -> None:
760760
"Foo", ["b", "i32", "i64"], [bool_rprimitive, int32_rprimitive, int64_rprimitive]
761761
)
762762
self.assert_emit(
763-
GetElementPtr(self.o, r, "b"), """cpy_r_r0 = (CPyPtr)&((Foo *)cpy_r_o)->b;"""
763+
GetElementPtr(self.o, r, "b"),
764+
"""cpy_r_r0 = (CPyPtr)((CPyPtr)cpy_r_o + offsetof(Foo, b));""",
764765
)
765766
self.assert_emit(
766-
GetElementPtr(self.o, r, "i32"), """cpy_r_r0 = (CPyPtr)&((Foo *)cpy_r_o)->i32;"""
767+
GetElementPtr(self.o, r, "i32"),
768+
"""cpy_r_r0 = (CPyPtr)((CPyPtr)cpy_r_o + offsetof(Foo, i32));""",
767769
)
768770
self.assert_emit(
769-
GetElementPtr(self.o, r, "i64"), """cpy_r_r0 = (CPyPtr)&((Foo *)cpy_r_o)->i64;"""
771+
GetElementPtr(self.o, r, "i64"),
772+
"""cpy_r_r0 = (CPyPtr)((CPyPtr)cpy_r_o + offsetof(Foo, i64));""",
770773
)
771774

772775
def test_set_element(self) -> None:

0 commit comments

Comments
 (0)