cmd/go: TestNoteReading fails on Solaris with linkmode=external

[rsc]

See http://build.golang.org/log/c7559674bb66a323958ebc8f8b51aaa0908c46f9

--- FAIL: TestNoteReading (0.36s)
    go_test.go:251: running testgo [build -ldflags -buildid=TestNoteReading-Build-ID -o /tmp/gotest473966932/hello.exe /tmp/gotest473966932/hello.go]
    go_test.go:251: running testgo [build -ldflags -buildid=TestNoteReading-Build-ID -linkmode=external -o /tmp/gotest473966932/hello.exe /tmp/gotest47396693   2/hello.go]
    note_test.go:39: buildID in hello binary = "", want "TestNoteReading-Build-ID" (linkmode=external)
FAIL

Disabling test for Go 1.5.

[gopherbot]

CL https://golang.org/cl/13679 mentions this issue.

[binarycrusader]

Debugging this now; short version is .note section is likely in first ~1MB when using external linking on Solaris instead of first 16K due to how Go is linking things in external linking mode. I'll submit a CL once I feel like I have the right fix.

The naive fix at the moment is to read the first ~1MB of the file instead of the first ~16K in ReadBuildIDFromBinary, but that's obviously undesirable.

[binarycrusader]

So the root cause of this is a subtlety in the System V ABI; the ABI specification defines .note sections as "special" sections that are not allocable (SHF_ALLOC):

Name    Type    Attributes
...
.note   SHT_NOTE    0
...

http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/specialsections.html

However, Go relies on the fact that it manually marked the section as allocable, and GNU ld will apply any specified flags to the sections. As a result, on Linux or other platforms where the allocable flag is honoured, the .note.go.buildid section will be placed roughly in the first 16K or so of the file.

Solaris ld(1), however, follows the System V ABI specification strictly and ignores any section attributes that the developer specified by "cleaning" the section flags by default to match System V although this can be "overridden" via a mapfile.

As such, it seems like there are two possible solutions:

• use a .data or other "expected" allocable section entry instead of a .note section to store the buildid
• use a mapfile on Solaris only to force the .note section to be allocable:

$ cat /tmp/note.mapfile 
$mapfile_version 2

LOAD_SEGMENT text {
        ASSIGN_SECTION {
                TYPE = SHT_NOTE;
        };
};
$ LD_OPTIONS=-M/tmp/note.mapfile go test -run TestNoteReading
PASS
ok      cmd/go  3.289s

Comments?

[ianlancetaylor]

On Thu, Aug 20, 2015 at 3:18 PM, Shawn notifications@github.com wrote:

So the root cause of this is a subtlety in the System V ABI; the ABI
specification defines .note sections as "special" sections that are not
allocable (SHF_ALLOC):

Name Type Attributes
...
.note SHT_NOTE 0
...

http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/specialsections.html

Although it's true that the ABI says that sections named ".note"
should not have any flags, the Go linker is not creating a section
named ".note". It's creating a section named ".note.go.buildid". The
ABI doesn't say anything about such a section.

Also, the ABI defines a PT_NOTE segment. Both the GNU linker and gold
will take a SHF_ALLOC SHT_NOTE section and put it into a PT_NOTE
segment. Is there any way to tell the Solaris linker to create a
PT_NOTE segment?

use a .data or other "expected" allocable section entry instead of a .note
section to store the buildid

This seems like the simplest solution if there is no way to create a
PT_NOTE segment. I think we would probably want a .text section so
that it goes very early in the binary.

Ian

[binarycrusader]

Although it's true that the ABI says that sections named ".note" should not have any flags, the Go
linker is not creating a section named ".note". It's creating a section named ".note.go.buildid". The
ABI doesn't say anything about such a section.

I think this is another one of those "matter of interpretation due to vague or non-explicit wording in standard" cases. For example, taking other statements in the standard into consideration:

"sh_type
This member categorizes the section’s contents and semantics.
Section types and their descriptions appear below."
...
"Section names with a dot (.) prefix are reserved for the system, although applica-
tions may use these sections if their existing meanings are satisfactory. Applica-
tions may use names without the prefix to avoid conflicts with system sections."

...one could interpret those statements to imply that sections generated by the application, if marked with SHT_NOTE, such as a .note.go.buildid are system sections since they start with a '.' prefix and while the name is not exactly ".note", I could understand why someone would interpret that to mean that any section with type SHT_NOTE should have the attributes listed in the ABI specification.

Regardless, our tentative plan for future versions of Solaris is to mimic the GNU linker's behaviour that you describe below and that I also found in an older blog post: http://www.airs.com/blog/archives/45 ...in that we'll change it to honour SHF_ALLOC for those sections.

Also, the ABI defines a PT_NOTE segment. Both the GNU linker and gold will take a SHF_ALLOC
SHT_NOTE section and put it into a PT_NOTE segment. Is there any way to tell the Solaris linker to
create a PT_NOTE segment?

The Solaris linker will create a PT_NOTE segment regardless of whether SHF_ALLOC is set, it just ignores the SHF_ALLOC flags per the table in the ABI; from the Solaris documentation:

"The note segment captures all sections of type SHT_NOTE. The link-editor provides a PT_NOTE
program header entry to reference the note segment."

The GNU linker appears to only create a PT_NOTE section though if SHF_ALLOC is set on a SHT_NOTE section which seems potentially at odds again with some readings of the standard.

As an example of what happens today with Go's -linkmode=external:

$ go build -ldflags -linkmode=external hello.go
$ elfdump hello
...
Program Header[7]:
    p_vaddr:      0                   p_flags:    0
    p_paddr:      0                   p_type:     [ PT_NOTE ]
    p_filesz:     0x38                p_memsz:    0
    p_offset:     0xbfa80             p_align:    0
...
Section Header[34]:  sh_name: .note.go.buildid
    sh_addr:      0                   sh_flags:   0
    sh_size:      0x38                sh_type:    [ SHT_NOTE ]
    sh_offset:    0xbfa80             sh_entsize: 0
    sh_link:      0                   sh_info:    0
    sh_addralign: 0x20              
...

Note the SH_OFFSET of the .note.go.buildid section. That's about ~786K into the file, which is because the SHF_ALLOC on the original section is ignored.

If we apply the mapfile I showed originally with -linkmode=external:

Section Header[23]:  sh_name: .note.go.buildid
    sh_addr:      0x4be700            sh_flags:   [ SHF_ALLOC ]
    sh_size:      0x38                sh_type:    [ SHT_NOTE ]
    sh_offset:    0xbe700             sh_entsize: 0
    sh_link:      0                   sh_info:    0
    sh_addralign: 0x20              

...but we don't get a PT_NOTE segment because of our override. In talking with the local linker aliens, there doesn't seem to be any way to get the desired result (SHT_NOTE section with SHF_ALLOC set and a corresponding PT_NOTE section). The Solaris linker makes the explicit assumption that note sections were never meant to be loaded into memory; only used for on-disk purposes.

[binarycrusader]

For the record, the test does not actually pass with mapfile example I showed earlier; I failed to remove the "skip" for LinkExternal on Solaris from note_test.go after doing a "git sync". While the mapfile gets us halfway there, the previous mapfile doesn't ensure that a PT_NOTE segment is created.

Anyway, short of changing Go to use something other than a SHT_NOTE section to record the buildid, the only other option I can see is to use a mapfile whenever linking the executables to leave space for a PT_NOTE header, ensure that the .note.go.buildid section stays within the first 16K of the binary, and ensure that the .note.go.buildid section is allocable:

$ cat /tmp/note.mapfile
$mapfile_version 2

PHDR_ADD_NULL = 1;

LOAD_SEGMENT text {
        ASSIGN_SECTION {
                IS_NAME = .note.go.buildid;
        };
        OS_ORDER = .note.go.buildid;
};

$ elfdump /tmp/gotest963298988/hello.exe
...
Program Header[4]:
    p_vaddr:      0                   p_flags:    0
    p_paddr:      0                   p_type:     [ PT_NULL ]
    p_filesz:     0                   p_memsz:    0
    p_offset:     0                   p_align:    0
...
Section Header[1]:  sh_name: .note.go.buildid
    sh_addr:      0x400200            sh_flags:   [ SHF_ALLOC ]
    sh_size:      0x28                sh_type:    [ SHT_NOTE ]
    sh_offset:    0x200               sh_entsize: 0
    sh_link:      0                   sh_info:    0
    sh_addralign: 0x20              
...

Then, to leverage the above, Go would have to either transform the PT_NULL header into a PT_NOTE header itself or use an 'elfedit' script to do the same.

[binarycrusader]

So it isn't pretty, but if Go wants to continue using the SHT_NOTE section for .note.go.buildid, the following appears to work:

diff --git a/src/cmd/go/note_test.go b/src/cmd/go/note_test.go
index 3d64451..9b7e246 100644
--- a/src/cmd/go/note_test.go
+++ b/src/cmd/go/note_test.go
@@ -33,9 +33,6 @@ func TestNoteReading(t *testing.T) {
        // no external linking
        t.Logf("no external linking - skipping linkmode=external test")

-   case "solaris":
-       t.Logf("skipping - golang.org/issue/12178")
-
    default:
        tg.run("build", "-ldflags", "-buildid="+buildID+" -linkmode=external", "-o", tg.path("hello.exe"), tg.path("hello.go"))
        id, err := main.ReadBuildIDFromBinary(tg.path("hello.exe"))
diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go
index 8ccbec9..79e1b96 100644
--- a/src/cmd/link/internal/ld/lib.go
+++ b/src/cmd/link/internal/ld/lib.go
@@ -1071,6 +1071,35 @@ func hostlink() {
        argv = append(argv, peimporteddlls()...)
    }

+   if HEADTYPE == obj.Hsolaris {
+       // Before linking, create a mapfile that directs the Solaris
+       // linker to reassign the .note.go.buildid section to the text
+       // segment so that it remains allocable.
+       p := fmt.Sprintf("%s/mapfile", tmpdir)
+       var err error
+       f, err := os.OpenFile(p, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
+       if err != nil {
+           Exitf("cannot create %s: %v", p, err)
+       }
+
+       b := bufio.NewWriter(f)
+       _, err = b.WriteString(`$mapfile_version 2
+PHDR_ADD_NULL = 1;
+LOAD_SEGMENT text {
+   ASSIGN_SECTION {
+       IS_NAME = .note.go.buildid;
+   };
+   OS_ORDER = .note.go.buildid;
+};
+`)
+       if err != nil {
+           Exitf("cannot write to %s: %v", p, err)
+       }
+       b.Flush()
+
+       argv = append(argv, "-Wl,-M," + p)
+   }
+
    if Debug['v'] != 0 {
        fmt.Fprintf(&Bso, "host link:")
        for _, v := range argv {
@@ -1087,6 +1116,30 @@ func hostlink() {
        Bso.Flush()
    }

+   if HEADTYPE == obj.Hsolaris {
+       // After linking, transform the PT_NULL entry created by
+       // Solaris linker using a mapfile into a PT_NOTE for the
+       // .note.go.buildid section.
+       f, err := elf.Open(outfile)
+       defer f.Close()
+       if err != nil {
+           return
+       }
+
+       sect := f.Section(".note.go.buildid")
+       f.Close()
+
+       argv := []string{"elfedit", "-e", fmt.Sprintf("phdr:p_vaddr PT_NULL %d", sect.Addr), "-e", fmt.Sprintf("phdr:p_paddr PT_NULL %d", sect.Addr), "-e", fmt.Sprintf("phdr:p_offset PT_NULL %d", sect.Offset), "-e", fmt.Sprintf("phdr:p_memsz PT_NULL %d", sect.Size), "-e", fmt.Sprintf("phdr:p_filesz PT_NULL %d", sect.Size), "-e", fmt.Sprintf("phdr:p_align PT_NULL %d", sect.Addralign), "-e", "phdr:p_type PT_NULL PT_NOTE", outfile}
+       if out, err := exec.Command(argv[0], argv[1:]...).CombinedOutput(); err != nil {
+           Exitf("running %s failed: %v\n%s", argv[0], err, out)
+       }
+
+       if Debug['v'] != 0 {
+           fmt.Fprintf(&Bso, "elfedit: %s\n", strings.Join(argv, " "))
+           Bso.Flush()
+       }
+   }
+
    if Debug['s'] == 0 && debug_s == 0 && HEADTYPE == obj.Hdarwin {
        // Skip combining dwarf on arm.
        if Thearch.Thechar != '5' && Thearch.Thechar != '7' {

It accomplishes this by a combination of what I outlined earlier:

• ensure .note.go.buildid is placed in text segment via mapfile
• ensure a stub program header is created via mapfile
• after linking, invoke elfedit to transform stub program header into a PT_NOTE and link to .note.go.buildid

$ elfdump hello
Program Header[4]:
    p_vaddr:      0x400200            p_flags:    0
    p_paddr:      0x400200            p_type:     [ PT_NOTE ]
    p_filesz:     0x38                p_memsz:    0x38
    p_offset:     0x200               p_align:    0x20
...
Section Header[1]:  sh_name: .note.go.buildid
    sh_addr:      0x400200            sh_flags:   [ SHF_ALLOC ]
    sh_size:      0x38                sh_type:    [ SHT_NOTE ]
    sh_offset:    0x200               sh_entsize: 0
    sh_link:      0                   sh_info:    0
    sh_addralign: 0x20              
...
/builds/srwalker/golang/go/pkg/tool/solaris_amd64/link -o $WORK/cmd/go/_test/go.test -L $WORK/cmd/go/_test -L $WORK -w -extld=gcc -buildmode=exe $WORK/cmd/go/_test/main.a
$WORK/cmd/go/_test/go.test -test.v=true -test.run=TestNoteReading
=== RUN   TestNoteReading
--- PASS: TestNoteReading (0.35s)
    go_test.go:251: running testgo [build -ldflags -buildid=TestNoteReading-Build-ID -o /tmp/gotest494547271/hello.exe /tmp/gotest494547271/hello.go]
    go_test.go:251: running testgo [build -ldflags -buildid=TestNoteReading-Build-ID -linkmode=external -o /tmp/gotest494547271/hello.exe /tmp/gotest494547271/hello.go]
PASS
ok      cmd/go  2.886s
...

The test now passes as expected. However, I feel like this solution is pretty ugly given the amount of additional platform-specific code I had to add to the linking step.

[ianlancetaylor]

The Solaris linker will create a PT_NOTE segment regardless of whether SHF_ALLOC is set, it just ignores the SHF_ALLOC flags per the table in the ABI:

Huh. It seems very weird to me to have a segment that refers to non-allocated sections. The whole point of segments is so that the dynamic linker can find things in memory at run time. That fails if the segment only points back to the file.

Does the Solaris linker set the p_vaddr, p_paddr, and p_memsz fields to zero?

[ianlancetaylor]

I think that on Solaris we just shouldn't use a SHT_NOTE section. We really don't care what the type of section is. On GNU/Linux SHT_NOTE is appropriate--it's convenient for readelf and objdump, for example--but if Solaris needs something else, we should use something else.

[binarycrusader]

Huh. It seems very weird to me to have a segment that refers to non-allocated sections. The whole point of segments is so that the dynamic linker can find things in memory at run time. That fails if the segment only points back to the file.

The specification seems to strongly imply that non-allocable sections are expected in the segments by calling some segments "loadable" and others not. For example, in the "Program Header" section of the specification (5-1), it states: "Some entries describe process segments; others give supplementary information and do not contribute to the process image." That seems to pretty clearly imply that some process segments found in the program header will not be loaded into memory. Again, another one of the dark corners of the standard I think.

Does the Solaris linker set the p_vaddr, p_paddr, and p_memsz fields to zero?

Yes, as I showed earlier:

$ go build -ldflags -linkmode=external hello.go
$ elfdump hello
...
Program Header[7]:
    p_vaddr:      0                   p_flags:    0
    p_paddr:      0                   p_type:     [ PT_NOTE ]
    p_filesz:     0x38                p_memsz:    0
    p_offset:     0xbfa80             p_align:    0
...
Section Header[34]:  sh_name: .note.go.buildid
    sh_addr:      0                   sh_flags:   0
    sh_size:      0x38                sh_type:    [ SHT_NOTE ]
    sh_offset:    0xbfa80             sh_entsize: 0
    sh_link:      0                   sh_info:    0
    sh_addralign: 0x20              
...

Which is logical; the specification states that "[p_memsz] gives the number of bytes in the memory image of the segment; it may be zero." That implies that p_vaddr may also be zero since there is no memory-resident data. And p_paddr is obsolete, so doesn't matter: " Because System V ignores physical addressing for application programs, this member [p_paddr] has unspecified contents for executable files and shared objects.".

[binarycrusader]

I think that on Solaris we just shouldn't use a SHT_NOTE section. We really don't care what the type of section is. On GNU/Linux SHT_NOTE is appropriate--it's convenient for readelf and objdump, for example--but if Solaris needs something else, we should use something else.

That's fine by me; as I said, future versions of Solaris will attempt to more closely mimic the GNU linker's behaviour, but for the foreseeable future, we'll have to pursue another solution for Go.

I'll see what I can come up with using a different section type instead of SHT_NOTE. Thanks for your patience.

[ianlancetaylor]

The specification seems to strongly imply that non-allocable sections are expected in the segments by calling some segments "loadable" and others not. For example, in the "Program Header" section of the specification (5-1), it states: "Some entries describe process segments; others give supplementary information and do not contribute to the process image." That seems to pretty clearly imply that some process segments found in the program header will not be loaded into memory. Again, another one of the dark corners of the standard I think.

OK, fair enough. On GNU/Linux systems, the sense has always been that you can have a PT_LOAD segment, you can have a segment like PT_INTERP or PT_TLS that refers to in-process memory, you can have a segment ilke PT_GNU_STACK that conveys information but does not refer to any contents of the file, but you will never have a program header that refers to file contents not loaded into memory, because such a segment is not useful. The kernel loads the PT_LOAD segments into memory, and passes them to the dynamic linker. We never want the dynamic linker to look at anything other than what the kernel already loaded into memory, so a segment that refers to unloaded memory is useless.

But you seem to be right that the ELF ABI permits it.