Skip to content

Commit

Permalink
pkg/compiler: fix struct layout bug
Browse files Browse the repository at this point in the history
Currently we have a bug in struct layout that affects
some corner cases that involve recursive structs.
The result of this bug is that we use wrong alignment 1
(not yet calculated) for some structs when calculating
layout of other structs.
The root cause of this bug is that we calculate struct
alignment too early in typeStruct.Gen when structs
are not yet laid out.
For this reason we moved struct size calculation to the
later phase (after compiler.layoutStruct).
Move alignment calculation from typeStruct.Gen to
compiler.layoutStruct to fix this.
  • Loading branch information
dvyukov committed Jan 20, 2025
1 parent cb1c9c3 commit 12b75fa
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 20 deletions.
13 changes: 13 additions & 0 deletions pkg/compiler/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,13 @@ func (comp *compiler) layoutArray(t *prog.ArrayType) {
if t.Kind == prog.ArrayRangeLen && t.RangeBegin == t.RangeEnd && !t.Elem.Varlen() {
t.TypeSize = t.RangeBegin * t.Elem.Size()
}
t.TypeAlign = t.Elem.Alignment()
}

func (comp *compiler) layoutUnion(t *prog.UnionType) {
for _, fld := range t.Fields {
t.TypeAlign = max(t.TypeAlign, fld.Alignment())
}
t.TypeSize = 0
structNode := comp.structs[t.TypeName]
if structNode == conditionalFieldWrapper {
Expand Down Expand Up @@ -303,6 +307,15 @@ func (comp *compiler) layoutStruct(t *prog.StructType) {
attrs := comp.parseIntAttrs(structAttrs, structNode, structNode.Attrs)
t.AlignAttr = attrs[attrAlign]
comp.layoutStructFields(t, varlen, attrs[attrPacked] != 0)
if align := attrs[attrAlign]; align != 0 {
t.TypeAlign = align
} else if attrs[attrPacked] != 0 {
t.TypeAlign = 1
} else {
for _, f := range t.Fields {
t.TypeAlign = max(t.TypeAlign, f.Alignment())
}
}
t.TypeSize = 0
if !varlen {
var size uint64
Expand Down
17 changes: 2 additions & 15 deletions pkg/compiler/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,8 +358,7 @@ var typeArray = &typeDesc{
NoZ: true,
}
}
// TypeSize is assigned later in layoutArray.
base.TypeAlign = elemType.Alignment()
// TypeSize/TypeAlign are assigned later in layoutArray.
return &prog.ArrayType{
TypeCommon: base.TypeCommon,
Elem: elemType,
Expand Down Expand Up @@ -1006,9 +1005,6 @@ func init() {
switch typ1 := typ.(type) {
case *prog.UnionType:
typ1.Fields = fields
for _, f := range fields {
typ1.TypeAlign = max(typ1.TypeAlign, f.Type.Alignment())
}
case *prog.StructType:
typ1.Fields = fields
for i, field := range fields {
Expand All @@ -1019,17 +1015,8 @@ func init() {
if overlayField >= 0 {
typ1.OverlayField = overlayField
}
attrs := comp.parseIntAttrs(structAttrs, s, s.Attrs)
if align := attrs[attrAlign]; align != 0 {
typ1.TypeAlign = align
} else if attrs[attrPacked] != 0 {
typ1.TypeAlign = 1
} else {
for _, f := range fields {
typ1.TypeAlign = max(typ1.TypeAlign, f.Type.Alignment())
}
}
}
// TypeSize/TypeAlign are assigned later in layoutStruct.
return typ
}
}
Expand Down
2 changes: 1 addition & 1 deletion sys/test/test/align3
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# 32 has 4-byte alignment for int64 and everything goes havoc.
# requires: -arch=32 -bigendian

syz_compare(&AUTO="00000000000000000199999999999999990300000000000000009999999999999999000000000000", AUTO, &AUTO=@align3={{0x0}, 0x1, {{0x2}}, 0x3, [{{0x4}}, {{0x5}}]}, AUTO)
syz_compare(&AUTO="000000000000000001000000000000009999999999999999030000000000000000000000000000009999999999999999", AUTO, &AUTO=@align3={{0x0}, 0x1, {{0x2}}, 0x3, [{{0x4}}, {{0x5}}]}, AUTO)
3 changes: 1 addition & 2 deletions tools/syz-declextract/testdata/types.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ struct various {
};

struct recursive {
// This is not handled properly yet.
// struct various various;
struct various various;
};

SYSCALL_DEFINE1(types_syscall, struct anon_struct* p, struct empty_struct* y,
Expand Down
12 changes: 11 additions & 1 deletion tools/syz-declextract/testdata/types.c.json
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,17 @@
},
{
"name": "recursive",
"align": 1
"byte_size": 64,
"align": 32,
"fields": [
{
"name": "various",
"counted_by": -1,
"type": {
"struct": "various"
}
}
]
},
{
"name": "various",
Expand Down
6 changes: 5 additions & 1 deletion tools/syz-declextract/testdata/types.c.txt
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,13 @@ packed_t$auto {
y int32
} [packed, align[32]]

recursive$auto {
various various$auto
}

various$auto {
recursive ptr[inout, various$auto, opt]
next ptr[inout, auto_aligner[1], opt]
next ptr[inout, recursive$auto, opt]
packed packed_t$auto
}

Expand Down

0 comments on commit 12b75fa

Please sign in to comment.