]> Cypherpunks repositories - gostls13.git/commitdiff
debug/dwarf: fix problems with handling of bit offsets for bitfields
authorThan McIntosh <thanm@google.com>
Tue, 25 Jan 2022 14:34:35 +0000 (09:34 -0500)
committerThan McIntosh <thanm@google.com>
Fri, 28 Jan 2022 20:07:54 +0000 (20:07 +0000)
This patch reworks the handling of the DWARF DW_AT_bit_offset and
DW_AT_data_bit_offset attributes to resolve problems arising from
a previous related change (CL 328709).

In CL 328709 the DWARF type reader was updated to look for and use
the DW_AT_data_bit_offset attribute for structure fields, handling
the value of the attribute in the same way as for DW_AT_bit_offset.
This caused problems for clients, since the two attributes have very
different semantics.

This CL effectively reverts CL 328709 and moves to a scheme in which
we detect and report the two attributes separately/independently.

This patch also corrects a problem in the DWARF type reader in the
code that detects and fixes up the type of struct fields corresponding
to zero-length arrays; the code in question was testing the
DW_AT_bit_offset attribute value but assuming DW_AT_data_bit_offset
semantics, meaning that it would fail to fix up cases such as

  typedef struct another_struct {
    unsigned short quix;
    int xyz[0];
    unsigned  x:1;
    long long array[40];
  } t;

The code in question has been changed to avoid using BitOffset and
instead consider only ByteOffset and BitSize.

Fixes #50685.
Updates #46784.

Change-Id: Ic15ce01c851af38ebd81af827973ec49badcab6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/380714
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
api/go1.18.txt
src/debug/dwarf/testdata/bitfields.c [new file with mode: 0644]
src/debug/dwarf/testdata/bitfields.elf4 [new file with mode: 0644]
src/debug/dwarf/testdata/typedef.elf5 [new file with mode: 0644]
src/debug/dwarf/type.go
src/debug/dwarf/type_test.go

index afcb31c63806901706cd51cc18c68ef682fee32a..7805d29eb79428d20551cc3e05ed73d93c94c8ea 100644 (file)
@@ -13,6 +13,8 @@ pkg debug/buildinfo, func ReadFile(string) (*debug.BuildInfo, error)
 pkg debug/buildinfo, type BuildInfo = debug.BuildInfo
 pkg debug/elf, const R_PPC64_RELATIVE = 22
 pkg debug/elf, const R_PPC64_RELATIVE R_PPC64
+pkg debug/dwarf, type BasicType struct, DataBitOffset int64
+pkg debug/dwarf, type StructField struct, DataBitOffset int64
 pkg debug/plan9obj, var ErrNoSymbols error
 pkg go/ast, method (*IndexListExpr) End() token.Pos
 pkg go/ast, method (*IndexListExpr) Pos() token.Pos
diff --git a/src/debug/dwarf/testdata/bitfields.c b/src/debug/dwarf/testdata/bitfields.c
new file mode 100644 (file)
index 0000000..0583333
--- /dev/null
@@ -0,0 +1,17 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+/*
+Linux ELF:
+gcc -gdwarf-4 -m64 -c bitfields.c -o bitfields.elf4
+*/
+
+typedef struct another_struct {
+  unsigned short quix;
+  int xyz[0];
+  unsigned  x:1;
+  long long array[40];
+} t_another_struct;
+t_another_struct q2;
+
diff --git a/src/debug/dwarf/testdata/bitfields.elf4 b/src/debug/dwarf/testdata/bitfields.elf4
new file mode 100644 (file)
index 0000000..2e06e68
Binary files /dev/null and b/src/debug/dwarf/testdata/bitfields.elf4 differ
diff --git a/src/debug/dwarf/testdata/typedef.elf5 b/src/debug/dwarf/testdata/typedef.elf5
new file mode 100644 (file)
index 0000000..aec48f6
Binary files /dev/null and b/src/debug/dwarf/testdata/typedef.elf5 differ
index 2e5a605174c378f29ab89acad9a28240555f133a..9c15cfb920612ed49ca570d548c4ef81914baf25 100644 (file)
@@ -33,10 +33,14 @@ func (c *CommonType) Size() int64 { return c.ByteSize }
 // Basic types
 
 // A BasicType holds fields common to all basic types.
+//
+// See the documentation for StructField for more info on the interpretation of
+// the BitSize/BitOffset/DataBitOffset fields.
 type BasicType struct {
        CommonType
-       BitSize   int64
-       BitOffset int64
+       BitSize       int64
+       BitOffset     int64
+       DataBitOffset int64
 }
 
 func (b *BasicType) Basic() *BasicType { return b }
@@ -150,13 +154,87 @@ type StructType struct {
 }
 
 // A StructField represents a field in a struct, union, or C++ class type.
+//
+// Bit Fields
+//
+// The BitSize, BitOffset, and DataBitOffset fields describe the bit
+// size and offset of data members declared as bit fields in C/C++
+// struct/union/class types.
+//
+// BitSize is the number of bits in the bit field.
+//
+// DataBitOffset, if non-zero, is the number of bits from the start of
+// the enclosing entity (e.g. containing struct/class/union) to the
+// start of the bit field. This corresponds to the DW_AT_data_bit_offset
+// DWARF attribute that was introduced in DWARF 4.
+//
+// BitOffset, if non-zero, is the number of bits between the most
+// significant bit of the storage unit holding the bit field to the
+// most significant bit of the bit field. Here "storage unit" is the
+// type name before the bit field (for a field "unsigned x:17", the
+// storage unit is "unsigned"). BitOffset values can vary depending on
+// the endianness of the system. BitOffset corresponds to the
+// DW_AT_bit_offset DWARF attribute that was deprecated in DWARF 4 and
+// removed in DWARF 5.
+//
+// At most one of DataBitOffset and BitOffset will be non-zero;
+// DataBitOffset/BitOffset will only be non-zero if BitSize is
+// non-zero. Whether a C compiler uses one or the other
+// will depend on compiler vintage and command line options.
+//
+// Here is an example of C/C++ bit field use, along with what to
+// expect in terms of DWARF bit offset info. Consider this code:
+//
+// struct S {
+//   int q;
+//   int j:5;
+//   int k:6;
+//   int m:5;
+//   int n:8;
+// } s;
+//
+// For the code above, one would expect to see the following for
+// DW_AT_bit_offset values (using GCC 8):
+//
+//          Little   |     Big
+//          Endian   |    Endian
+//                   |
+//   "j":     27     |     0
+//   "k":     21     |     5
+//   "m":     16     |     11
+//   "n":     8      |     16
+//
+// Note that in the above the offsets are purely with respect to the
+// containing storage unit for j/k/m/n -- these values won't vary based
+// on the size of prior data members in the containing struct.
+//
+// If the compiler emits DW_AT_data_bit_offset, the expected values
+// would be:
+//
+//   "j":     32
+//   "k":     37
+//   "m":     43
+//   "n":     48
+//
+// Here the value 32 for "j" reflects the fact that the bit field is
+// preceded by other data members (recall that DW_AT_data_bit_offset
+// values are relative to the start of the containing struct). Hence
+// DW_AT_data_bit_offset values can be quite large for structs with
+// many fields.
+//
+// DWARF also allow for the possibility of base types that have
+// non-zero bit size and bit offset, so this information is also
+// captured for base types, but it is worth noting that it is not
+// possible to trigger this behavior using mainstream languages.
+//
 type StructField struct {
-       Name       string
-       Type       Type
-       ByteOffset int64
-       ByteSize   int64 // usually zero; use Type.Size() for normal fields
-       BitOffset  int64 // within the ByteSize bytes at ByteOffset
-       BitSize    int64 // zero if not a bit field
+       Name          string
+       Type          Type
+       ByteOffset    int64
+       ByteSize      int64 // usually zero; use Type.Size() for normal fields
+       BitOffset     int64
+       DataBitOffset int64
+       BitSize       int64 // zero if not a bit field
 }
 
 func (t *StructType) String() string {
@@ -166,6 +244,13 @@ func (t *StructType) String() string {
        return t.Defn()
 }
 
+func (f *StructField) bitOffset() int64 {
+       if f.BitOffset != 0 {
+               return f.BitOffset
+       }
+       return f.DataBitOffset
+}
+
 func (t *StructType) Defn() string {
        s := t.Kind
        if t.StructName != "" {
@@ -184,7 +269,7 @@ func (t *StructType) Defn() string {
                s += "@" + strconv.FormatInt(f.ByteOffset, 10)
                if f.BitSize > 0 {
                        s += " : " + strconv.FormatInt(f.BitSize, 10)
-                       s += "@" + strconv.FormatInt(f.BitOffset, 10)
+                       s += "@" + strconv.FormatInt(f.bitOffset(), 10)
                }
        }
        s += "}"
@@ -469,8 +554,12 @@ func (d *Data) readType(name string, r typeReader, off Offset, typeCache map[Off
                //      AttrName: name of base type in programming language of the compilation unit [required]
                //      AttrEncoding: encoding value for type (encFloat etc) [required]
                //      AttrByteSize: size of type in bytes [required]
-               //      AttrBitOffset: for sub-byte types, size in bits
-               //      AttrBitSize: for sub-byte types, bit offset of high order bit in the AttrByteSize bytes
+               //      AttrBitOffset: bit offset of value within containing storage unit
+               //      AttrDataBitOffset: bit offset of value within containing storage unit
+               //      AttrBitSize: size in bits
+               //
+               // For most languages BitOffset/DataBitOffset/BitSize will not be present
+               // for base types.
                name, _ := e.Val(AttrName).(string)
                enc, ok := e.Val(AttrEncoding).(int64)
                if !ok {
@@ -517,8 +606,12 @@ func (d *Data) readType(name string, r typeReader, off Offset, typeCache map[Off
                t.Name = name
                t.BitSize, _ = e.Val(AttrBitSize).(int64)
                haveBitOffset := false
-               if t.BitOffset, haveBitOffset = e.Val(AttrBitOffset).(int64); !haveBitOffset {
-                       t.BitOffset, _ = e.Val(AttrDataBitOffset).(int64)
+               haveDataBitOffset := false
+               t.BitOffset, haveBitOffset = e.Val(AttrBitOffset).(int64)
+               t.DataBitOffset, haveDataBitOffset = e.Val(AttrDataBitOffset).(int64)
+               if haveBitOffset && haveDataBitOffset {
+                       err = DecodeError{name, e.Offset, "duplicate bit offset attributes"}
+                       goto Error
                }
 
        case TagClassType, TagStructType, TagUnionType:
@@ -533,6 +626,7 @@ func (d *Data) readType(name string, r typeReader, off Offset, typeCache map[Off
                //              AttrType: type of member [required]
                //              AttrByteSize: size in bytes
                //              AttrBitOffset: bit offset within bytes for bit fields
+               //              AttrDataBitOffset: field bit offset relative to struct start
                //              AttrBitSize: bit size for bit fields
                //              AttrDataMemberLoc: location within struct [required for struct, class]
                // There is much more to handle C++, all ignored for now.
@@ -551,7 +645,8 @@ func (d *Data) readType(name string, r typeReader, off Offset, typeCache map[Off
                t.Incomplete = e.Val(AttrDeclaration) != nil
                t.Field = make([]*StructField, 0, 8)
                var lastFieldType *Type
-               var lastFieldBitOffset int64
+               var lastFieldBitSize int64
+               var lastFieldByteOffset int64
                for kid := next(); kid != nil; kid = next() {
                        if kid.Tag != TagMember {
                                continue
@@ -578,30 +673,31 @@ func (d *Data) readType(name string, r typeReader, off Offset, typeCache map[Off
                                f.ByteOffset = loc
                        }
 
-                       haveBitOffset := false
                        f.Name, _ = kid.Val(AttrName).(string)
                        f.ByteSize, _ = kid.Val(AttrByteSize).(int64)
-                       if f.BitOffset, haveBitOffset = kid.Val(AttrBitOffset).(int64); !haveBitOffset {
-                               f.BitOffset, haveBitOffset = kid.Val(AttrDataBitOffset).(int64)
+                       haveBitOffset := false
+                       haveDataBitOffset := false
+                       f.BitOffset, haveBitOffset = kid.Val(AttrBitOffset).(int64)
+                       f.DataBitOffset, haveDataBitOffset = kid.Val(AttrDataBitOffset).(int64)
+                       if haveBitOffset && haveDataBitOffset {
+                               err = DecodeError{name, e.Offset, "duplicate bit offset attributes"}
+                               goto Error
                        }
                        f.BitSize, _ = kid.Val(AttrBitSize).(int64)
                        t.Field = append(t.Field, f)
 
-                       bito := f.BitOffset
-                       if !haveBitOffset {
-                               bito = f.ByteOffset * 8
-                       }
-                       if bito == lastFieldBitOffset && t.Kind != "union" {
+                       if lastFieldBitSize == 0 && lastFieldByteOffset == f.ByteOffset && t.Kind != "union" {
                                // Last field was zero width. Fix array length.
                                // (DWARF writes out 0-length arrays as if they were 1-length arrays.)
                                fixups.recordArrayType(lastFieldType)
                        }
                        lastFieldType = &f.Type
-                       lastFieldBitOffset = bito
+                       lastFieldByteOffset = f.ByteOffset
+                       lastFieldBitSize = f.BitSize
                }
                if t.Kind != "union" {
                        b, ok := e.Val(AttrByteSize).(int64)
-                       if ok && b*8 == lastFieldBitOffset {
+                       if ok && b == lastFieldByteOffset {
                                // Final field must be zero width. Fix array length.
                                fixups.recordArrayType(lastFieldType)
                        }
index 431d0853e0546d0361dc627d6920e25065a37bcd..0acc606df7ff8f872543a0ed8bb1253502332ee3 100644 (file)
@@ -83,15 +83,19 @@ func peData(t *testing.T, name string) *Data {
        return d
 }
 
-func TestTypedefsELF(t *testing.T) { testTypedefs(t, elfData(t, "testdata/typedef.elf"), "elf") }
+func TestTypedefsELF(t *testing.T) {
+       testTypedefs(t, elfData(t, "testdata/typedef.elf"), "elf", typedefTests)
+}
 
 func TestTypedefsMachO(t *testing.T) {
-       testTypedefs(t, machoData(t, "testdata/typedef.macho"), "macho")
+       testTypedefs(t, machoData(t, "testdata/typedef.macho"), "macho", typedefTests)
 }
 
-func TestTypedefsELFDwarf4(t *testing.T) { testTypedefs(t, elfData(t, "testdata/typedef.elf4"), "elf") }
+func TestTypedefsELFDwarf4(t *testing.T) {
+       testTypedefs(t, elfData(t, "testdata/typedef.elf4"), "elf", typedefTests)
+}
 
-func testTypedefs(t *testing.T, d *Data, kind string) {
+func testTypedefs(t *testing.T, d *Data, kind string, testcases map[string]string) {
        r := d.Reader()
        seen := make(map[string]bool)
        for {
@@ -115,7 +119,7 @@ func testTypedefs(t *testing.T, d *Data, kind string) {
                                typstr = t1.Type.String()
                        }
 
-                       if want, ok := typedefTests[t1.Name]; ok {
+                       if want, ok := testcases[t1.Name]; ok {
                                if seen[t1.Name] {
                                        t.Errorf("multiple definitions for %s", t1.Name)
                                }
@@ -130,7 +134,7 @@ func testTypedefs(t *testing.T, d *Data, kind string) {
                }
        }
 
-       for k := range typedefTests {
+       for k := range testcases {
                if !seen[k] {
                        t.Errorf("missing %s", k)
                }
@@ -229,21 +233,42 @@ func TestUnsupportedTypes(t *testing.T) {
        }
 }
 
-func TestBitOffsetsELF(t *testing.T) { testBitOffsets(t, elfData(t, "testdata/typedef.elf")) }
+var expectedBitOffsets1 = map[string]string{
+       "x": "S:1 DBO:32",
+       "y": "S:4 DBO:33",
+}
+
+var expectedBitOffsets2 = map[string]string{
+       "x": "S:1 BO:7",
+       "y": "S:4 BO:27",
+}
+
+func TestBitOffsetsELF(t *testing.T) {
+       f := "testdata/typedef.elf"
+       testBitOffsets(t, elfData(t, f), f, expectedBitOffsets2)
+}
 
 func TestBitOffsetsMachO(t *testing.T) {
-       testBitOffsets(t, machoData(t, "testdata/typedef.macho"))
+       f := "testdata/typedef.macho"
+       testBitOffsets(t, machoData(t, f), f, expectedBitOffsets2)
 }
 
 func TestBitOffsetsMachO4(t *testing.T) {
-       testBitOffsets(t, machoData(t, "testdata/typedef.macho4"))
+       f := "testdata/typedef.macho4"
+       testBitOffsets(t, machoData(t, f), f, expectedBitOffsets1)
 }
 
 func TestBitOffsetsELFDwarf4(t *testing.T) {
-       testBitOffsets(t, elfData(t, "testdata/typedef.elf4"))
+       f := "testdata/typedef.elf4"
+       testBitOffsets(t, elfData(t, f), f, expectedBitOffsets1)
+}
+
+func TestBitOffsetsELFDwarf5(t *testing.T) {
+       f := "testdata/typedef.elf5"
+       testBitOffsets(t, elfData(t, f), f, expectedBitOffsets1)
 }
 
-func testBitOffsets(t *testing.T, d *Data) {
+func testBitOffsets(t *testing.T, d *Data, tag string, expectedBitOffsets map[string]string) {
        r := d.Reader()
        for {
                e, err := r.Next()
@@ -262,15 +287,26 @@ func testBitOffsets(t *testing.T, d *Data) {
 
                        t1 := typ.(*StructType)
 
+                       bitInfoDump := func(f *StructField) string {
+                               res := fmt.Sprintf("S:%d", f.BitSize)
+                               if f.BitOffset != 0 {
+                                       res += fmt.Sprintf(" BO:%d", f.BitOffset)
+                               }
+                               if f.DataBitOffset != 0 {
+                                       res += fmt.Sprintf(" DBO:%d", f.DataBitOffset)
+                               }
+                               return res
+                       }
+
                        for _, field := range t1.Field {
                                // We're only testing for bitfields
                                if field.BitSize == 0 {
                                        continue
                                }
-
-                               // Ensure BitOffset is not zero
-                               if field.BitOffset == 0 {
-                                       t.Errorf("bit offset of field %s in %s %s is not set", field.Name, t1.Kind, t1.StructName)
+                               got := bitInfoDump(field)
+                               want := expectedBitOffsets[field.Name]
+                               if got != want {
+                                       t.Errorf("%s: field %s in %s: got info %q want %q", tag, field.Name, t1.StructName, got, want)
                                }
                        }
                }
@@ -279,3 +315,22 @@ func testBitOffsets(t *testing.T, d *Data) {
                }
        }
 }
+
+var bitfieldTests = map[string]string{
+       "t_another_struct": "struct another_struct {quix short unsigned int@0; xyz [0]int@4; x unsigned int@4 : 1@31; array [40]long long int@8}",
+}
+
+// TestBitFieldZeroArrayIssue50685 checks to make sure that the DWARF
+// type reading code doesn't get confused by the presence of a
+// specifically-sized bitfield member immediately following a field
+// whose type is a zero-length array. Prior to the fix for issue
+// 50685, we would get this type for the case in testdata/bitfields.c:
+//
+// another_struct {quix short unsigned int@0; xyz [-1]int@4; x unsigned int@4 : 1@31; array [40]long long int@8}
+//
+// Note the "-1" for the xyz field, which should be zero.
+//
+func TestBitFieldZeroArrayIssue50685(t *testing.T) {
+       f := "testdata/bitfields.elf4"
+       testTypedefs(t, elfData(t, f), "elf", bitfieldTests)
+}