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.

Sign up for GitHub

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/pe: TestReadCOFFSymbolAuxInfo test fails on big-endian systems #52079

Closed
thanm opened this issue Mar 31, 2022 · 5 comments
Closed

debug/pe: TestReadCOFFSymbolAuxInfo test fails on big-endian systems #52079

thanm opened this issue Mar 31, 2022 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@thanm
Copy link
Contributor

thanm commented Mar 31, 2022

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

go tip

Does this issue reproduce with the latest release?

no

What operating system and processor architecture are you using (go env)?

Big endian systems, e.g. s390, PPC BE.

What did you do?

On a big-endian machine, run "go test debug/pe"

What did you expect to see?

clean run

What did you see instead?

--- FAIL: TestReadCOFFSymbolAuxInfo (0.00s)
symbols_test.go:76: COFFSymbolReadSectionDefAux on 39 bad return, got:
{Size:134217728 NumRelocs:256 NumLineNumbers:0 Checksum:0 SecNum:16 Selection:0 _:[2 0 0]}
want:
{Size:8 NumRelocs:1 NumLineNumbers:0 Checksum:0 SecNum:16 Selection:2 _:[0 0 0]}
symbols_test.go:76: COFFSymbolReadSectionDefAux on 81 bad return, got:
{Size:3791847424 NumRelocs:256 NumLineNumbers:0 Checksum:1624223678 SecNum:32 Selection:0 _:[0 0 0]}
want:
{Size:994 NumRelocs:1 NumLineNumbers:0 Checksum:1624223678 SecNum:32 Selection:0 _:[0 0 0]}
FAIL
FAIL debug/pe 0.003s
FAIL

Basically it looks as though the new COFFSymbolReadSectionDefAux is not working properly in a big-endian environment.
@thanm thanm added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 31, 2022
@thanm thanm self-assigned this Mar 31, 2022
@gopherbot
Copy link

Change https://go.dev/cl/397294 mentions this issue: debug/pe: skip TestReadCOFFSymbolAuxInfo on big-endian systems

gopherbot pushed a commit that referenced this issue Mar 31, 2022
@thanm
debug/pe: skip TestReadCOFFSymbolAuxInfo on big-endian systems
c5c66d7 
Disable the new TestReadCOFFSymbolAuxInfo testpoint on big endian
systems, pending resolution of issue 52079. The newly added interfaces
for reading symbol definition aux info is not working properly when
reading PE objects obj big-endian systems.

Updates #52079.

Change-Id: I8d55c7e4c03fc6444ef06a6a8154cb50596ca58a
Reviewed-on: https://go-review.googlesource.com/c/go/+/397294
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
@alexbrainman
Copy link
Member

@thanm

I am pretty sure you should be using encoding/binary.LittleEndian instead of unsafe, and that will fix this issue. See, for example

$ git grep -ne binary.LittleEndian | grep debug/pe
debug/pe/file.go:74:            signoff := int64(binary.LittleEndian.Uint32(dosheader[0x3c:]))
debug/pe/file.go:85:    if err := binary.Read(sr, binary.LittleEndian, &f.FileHeader); err != nil {
debug/pe/file.go:133:           if err := binary.Read(sr, binary.LittleEndian, sh); err != nil {
debug/pe/file.go:376:           dt.OriginalFirstThunk = binary.LittleEndian.Uint32(d[0:4])
debug/pe/file.go:377:           dt.TimeDateStamp = binary.LittleEndian.Uint32(d[4:8])
debug/pe/file.go:378:           dt.ForwarderChain = binary.LittleEndian.Uint32(d[8:12])
debug/pe/file.go:379:           dt.Name = binary.LittleEndian.Uint32(d[12:16])
debug/pe/file.go:380:           dt.FirstThunk = binary.LittleEndian.Uint32(d[16:20])
debug/pe/file.go:402:                           va := binary.LittleEndian.Uint64(d[0:8])
debug/pe/file.go:414:                           va := binary.LittleEndian.Uint32(d[0:4])
debug/pe/file.go:477:           err = binary.Read(r, binary.LittleEndian, data)
debug/pe/file.go:609:   if err := binary.Read(r, binary.LittleEndian, dd); err != nil {
debug/pe/section.go:61: err = binary.Read(r, binary.LittleEndian, relocs)
debug/pe/string.go:38:  err = binary.Read(r, binary.LittleEndian, &l)
debug/pe/symbol.go:37:  err = binary.Read(r, binary.LittleEndian, syms)
debug/pe/symbol.go:47:          return true, binary.LittleEndian.Uint32(name[4:])
$

I should have picked this up during original review.

Alex

@alexbrainman
Copy link
Member

I should have picked this up during original review.

I will read

https://commandcenter.blogspot.com/2012/04/byte-order-fallacy.html

as a penance for my sins.

Alex

@thanm
Copy link
Contributor Author

thanm commented Apr 1, 2022

No worries, I should have caught it as well. I sent a CL.

@gopherbot
Copy link

Change https://go.dev/cl/397485 mentions this issue: debug/pe: rework reading of aux symbols to fix endianity problems

@rsc rsc unassigned thanm Jun 22, 2022
@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@gopherbot @alexbrainman @thanm