golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.84k stars 17.65k forks source link

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

Closed thanm closed 2 years ago

thanm commented 2 years ago

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) symbolstest.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]} symbolstest.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.

gopherbot commented 2 years ago

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

alexbrainman commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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

gopherbot commented 2 years ago

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