debug/elf: (*Section).Data() returns non-zero data for SHT_NOBITS section

[minux]

(*Section).Data fails to take section.Type == Type:elf.SHT_NOBITS into account, and will happily return anything in the file at the section file offset.

// x.go
package main

import (
	"debug/elf"
	"fmt"
	"os"
)

func main() {
	f, err := elf.Open(os.Args[1])
	if err != nil {
		panic(err)
	}
	sect := f.Section(".bss")
	fmt.Printf("%#v\n", *sect)
	data, err := sect.Data()
	if err != nil {
		panic(err)
	}
	fmt.Printf("data = %d bytes\n%q\n", len(data), data)
}

On certain ELF files, debug/elf will output non-zero data for .bss section.
For example, compile the following assembly file as a shared library:

.bss
.comm d, 10

$ cc -o a.so a.c -shared
$ go run x.go a.so
elf.Section{SectionHeader:elf.SectionHeader{Name:".bss", Type:elf.SHT_NOBITS, Flags:elf.SHF_WRITE+elf.SHF_ALLOC, Addr:0x201030, Offset:0x1030, Size:0x20, Link:0x0, Info:0x0, Addralign:0x10, Entsize:0x0, FileSize:0x20}, ReaderAt:(*io.SectionReader)(0xc420016870), sr:(*io.SectionReader)(0xc420016870), compressionType:0, compressionOffset:0}
data = 32 bytes
"GCC: .....\x00\x00.s"

This is because the .comment section happens to be after the .bss section.

Perhaps this is a user error, but I'm not entirely sure. And if it is indeed a bug of debug/elf, I'm not sure whether we should make it return make([]byte, section.Size) or just an error indicating that the section doesn't have file data. I'm leaning towards returning a zero byte slice.

/cc @ianlancetaylor

[ianlancetaylor]

I would be inclined to return an error of the Data method. A SHT_NOBITS section has no data. I don't think that programs that look at ELF data should be encouraged to disregard the section flags.

That said, I would reconsider if that choice breaks real programs that seem otherwise correct.

[minux]

Return an error from both (*Section).Open and (*Section).Data is also fine with me. The current behavior of silently returning wrong data (and nil error) should be fixed.

[bradfitz]

Bumping to Go 1.10 for lack of activity.

[rsc]

If we don't return an error we have to decide between returning []byte{} and make([]byte, Size). Better to return an error and make the user understand what's going on.