From 6e87cfa299c98d36e79e8b8718a4126899a3ba2f Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Fri, 17 Jan 2025 10:39:49 +0100 Subject: pkg/compiler: fix struct layout bug 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. --- pkg/compiler/gen.go | 13 +++++++++++++ pkg/compiler/types.go | 17 ++--------------- sys/test/test/align3 | 2 +- tools/syz-declextract/testdata/types.c | 3 +-- tools/syz-declextract/testdata/types.c.json | 12 +++++++++++- tools/syz-declextract/testdata/types.c.txt | 6 +++++- 6 files changed, 33 insertions(+), 20 deletions(-) diff --git a/pkg/compiler/gen.go b/pkg/compiler/gen.go index 81e8f83bc..688dd7013 100644 --- a/pkg/compiler/gen.go +++ b/pkg/compiler/gen.go @@ -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 { @@ -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 diff --git a/pkg/compiler/types.go b/pkg/compiler/types.go index ae0864725..e022efafe 100644 --- a/pkg/compiler/types.go +++ b/pkg/compiler/types.go @@ -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, @@ -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 { @@ -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 } } diff --git a/sys/test/test/align3 b/sys/test/test/align3 index 7a6331c99..00ba25576 100644 --- a/sys/test/test/align3 +++ b/sys/test/test/align3 @@ -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) diff --git a/tools/syz-declextract/testdata/types.c b/tools/syz-declextract/testdata/types.c index 014318d42..7a98394c1 100644 --- a/tools/syz-declextract/testdata/types.c +++ b/tools/syz-declextract/testdata/types.c @@ -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, diff --git a/tools/syz-declextract/testdata/types.c.json b/tools/syz-declextract/testdata/types.c.json index 99d0adc2b..6c479dffb 100644 --- a/tools/syz-declextract/testdata/types.c.json +++ b/tools/syz-declextract/testdata/types.c.json @@ -758,7 +758,17 @@ }, { "name": "recursive", - "align": 1 + "byte_size": 64, + "align": 32, + "fields": [ + { + "name": "various", + "counted_by": -1, + "type": { + "struct": "various" + } + } + ] }, { "name": "various", diff --git a/tools/syz-declextract/testdata/types.c.txt b/tools/syz-declextract/testdata/types.c.txt index ea95b4bb5..085aee017 100644 --- a/tools/syz-declextract/testdata/types.c.txt +++ b/tools/syz-declextract/testdata/types.c.txt @@ -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 } -- cgit mrf-deployment