Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

debug/dwarf: BitOffset values of bitfields is not being set in newer version of DWARF #46784

Closed
jtsylve opened this issue Jun 16, 2021 · 9 comments
Closed

debug/dwarf: BitOffset values of bitfields is not being set in newer version of DWARF #46784

jtsylve opened this issue Jun 16, 2021 · 9 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.18

Comments

@jtsylve
Copy link
Contributor

jtsylve commented Jun 16, 2021

What version of Go are you using (go version)?

 go version go1.16.5 darwin/amd64

Does this issue reproduce with the latest release?

Yes (including 1.17 beta 1)

Background Information

The DWARF v4 Standard depreciated the DW_AT_bit_offset attribute in favor of a new DW_AT_data_bit_offset attribute. The value of DW_AT_bit_offset was reserved for backwards compatibility.

See the following snippet from the v4 DWARF Standard [1]:

Attribute DW_AT_data_bit_offset is new in DWARF Version 4 and is also used for bit field
members (see Section 5.5.6). It replaces the attribute DW_AT_bit_offset when used for base
types as defined in DWARF V3 and earlier. The earlier attribute is defined in a manner suitable
for bit field members on big-endian architectures but which is wasteful for use on little-endian
architectures.

The attribute DW_AT_bit_offset is deprecated in DWARF Version 4 for use in base types, but
implementations may continue to support its use for compatibility.

The Issue

The debug/dwarf package only checks against AttrBitOffset and ignores the AttrDataBitOffset attribute, causing the BitOffset fields to be set to zero in newer versions of DWARF debug info.

Possible Solution

AttrDataBitOffset should be checked if there is no AttrBitOffset attribute.

diff --git a/src/debug/dwarf/type.go b/src/debug/dwarf/type.go
index eb5a666ed3..2e5a605174 100644
--- a/src/debug/dwarf/type.go
+++ b/src/debug/dwarf/type.go
@@ -516,7 +516,10 @@ func (d *Data) readType(name string, r typeReader, off Offset, typeCache map[Off
 		}).Basic()
 		t.Name = name
 		t.BitSize, _ = e.Val(AttrBitSize).(int64)
-		t.BitOffset, _ = e.Val(AttrBitOffset).(int64)
+		haveBitOffset := false
+		if t.BitOffset, haveBitOffset = e.Val(AttrBitOffset).(int64); !haveBitOffset {
+			t.BitOffset, _ = e.Val(AttrDataBitOffset).(int64)
+		}
 
 	case TagClassType, TagStructType, TagUnionType:
 		// Structure, union, or class type.  (DWARF v2 §5.5)
@@ -578,7 +581,9 @@ func (d *Data) readType(name string, r typeReader, off Offset, typeCache map[Off
 			haveBitOffset := false
 			f.Name, _ = kid.Val(AttrName).(string)
 			f.ByteSize, _ = kid.Val(AttrByteSize).(int64)
-			f.BitOffset, haveBitOffset = kid.Val(AttrBitOffset).(int64)
+			if f.BitOffset, haveBitOffset = kid.Val(AttrBitOffset).(int64); !haveBitOffset {
+				f.BitOffset, haveBitOffset = kid.Val(AttrDataBitOffset).(int64)
+			}
 			f.BitSize, _ = kid.Val(AttrBitSize).(int64)
 			t.Field = append(t.Field, f)

References

[1] DWARF Debugging Information Format, Version 4

@thanm
Copy link
Contributor

thanm commented Jun 16, 2021

This seems reasonable to me. Would you like to send a patch? Thanks.

jtsylve pushed a commit to jtsylve/go that referenced this issue Jun 16, 2021
Add logic to check the new AttrDataBitOffset introduced in DWARFv4. F…
2508d5d 
…ixes golang#46784
@jtsylve jtsylve mentioned this issue Jun 16, 2021
Closed
jtsylve added a commit to jtsylve/go that referenced this issue Jun 16, 2021
@jtsylve
Add logic to check the new AttrDataBitOffset introduced in DWARFv4. F…
077a173 
…ixes golang#46784
@gopherbot
Copy link

Change https://golang.org/cl/328709 mentions this issue: debug/dwarf: add logic to check the new AttrDataBitOffset introduced in DWARFv4

@seankhliao seankhliao added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 16, 2021
@jtsylve
Copy link
Contributor Author

jtsylve commented Jun 17, 2021

@seankhliao What is the fix needed? I have already updated the PR to add the requested unit test.

@ianlancetaylor
Copy link
Contributor

@jtsylve The "NeedsFix" label just means that closing this issue requires a fix to be applied to the repository. The change you already sent can be that fix. Thanks.

@jtsylve
Copy link
Contributor Author

jtsylve commented Jun 17, 2021

@ianlancetaylor My apologies. I thought I was responding to the thread label on the PR and not the issue. That makes sense.

@ianlancetaylor ianlancetaylor added this to the Go1.18 milestone Jul 11, 2021
jtsylve added a commit to jtsylve/go that referenced this issue Sep 2, 2021
@jtsylve
Add logic to check the new AttrDataBitOffset introduced in DWARFv4. F…
c101b2e 
…ixes golang#46784
@dvyukov
Copy link
Member

dvyukov commented Jan 13, 2022

For anybody who found this issue: currently there is a subtle difference in bitfield representation with dward4+, see:
https://go-review.googlesource.com/c/go/+/328709/comments/edf0619d_daec236f

@thanm thanm mentioned this issue Jan 19, 2022
Closed
@thanm
Copy link
Contributor

thanm commented Jan 19, 2022

I filed a new issue to track the problem Dmitry raised: #50685

@gopherbot
Copy link

Change https://golang.org/cl/380714 mentions this issue: debug/dwarf: fix problems with handling of bit offsets for bitfields

gopherbot pushed a commit that referenced this issue Jan 28, 2022
@thanm
debug/dwarf: fix problems with handling of bit offsets for bitfields
8314544 
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>
@gopherbot
Copy link

Change https://golang.org/cl/381778 mentions this issue: doc/go1.18: mention new debug/dwarf DataBitOffset fields

pull bot pushed a commit to lambdafunc/go that referenced this issue Jan 29, 2022
@ianlancetaylor
doc/go1.18: mention new debug/dwarf DataBitOffset fields
1226547 
For golang#46784
For golang#47694
For golang#50685

Change-Id: I5351b56722d036a520d1a598ef7af95c5eb44c35
Reviewed-on: https://go-review.googlesource.com/c/go/+/381778
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
@ianlancetaylor ianlancetaylor mentioned this issue Feb 24, 2022
Closed
@golang golang locked and limited conversation to collaborators Jan 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.18
Development

Successfully merging a pull request may close this issue.

debug/dwarf: check for DWARFv4 AttrDataBitOffset value jtsylve/go
6 participants
@jtsylve @dvyukov @ianlancetaylor @gopherbot @seankhliao @thanm