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

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

Open
jtsylve opened this issue Jun 16, 2021 · 5 comments · May be fixed by #46790
Open

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

jtsylve opened this issue Jun 16, 2021 · 5 comments · May be fixed by #46790
Labels
Milestone

Comments

@jtsylve
Copy link
Contributor

@jtsylve 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 thanm commented Jun 16, 2021

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

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 16, 2021

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

@jtsylve
Copy link
Contributor Author

@jtsylve 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

@ianlancetaylor ianlancetaylor commented Jun 17, 2021

@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 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants