Skip to content

Commit a13d085

Browse files
committed
cmd/cgo: don't hardcode section name in TestNumberOfExportedFunctions
TestNumberOfExportedFunctions checks the number of exported functions announced in the PE export table, getting it from the .edata section. If the section is not found, the test is skipped. However, the PE spec doesn't mandate that the export table be in a section named .edata, making this test prone to being skipped unnecessarily. While here, remove a check in cmd/go.testBuildmodePIE that was testing the same thing in order to verify that the binary had a relocation table . Not only the test is duplicated, but also it in unnecessary because it already testing that the PE characteristics doesn't contain the IMAGE_FILE_RELOCS_STRIPPED flag. Closes golang#46719 Cq-Include-Trybots: luci.golang.try:gotip-windows-arm64 Change-Id: I28d1e261b38388868dd3c19ef6ddddad7bf105ef Reviewed-on: https://go-review.googlesource.com/c/go/+/705755 Reviewed-by: Junyang Shao <[email protected]> Reviewed-by: Cherry Mui <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 61bf26a commit a13d085

File tree

2 files changed

+75
-74
lines changed

2 files changed

+75
-74
lines changed

src/cmd/cgo/internal/testcshared/cshared_test.go

Lines changed: 75 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -375,26 +375,7 @@ func TestExportedSymbols(t *testing.T) {
375375
}
376376
}
377377

378-
func checkNumberOfExportedFunctionsWindows(t *testing.T, exportAllSymbols bool) {
379-
const prog = `
380-
package main
381-
382-
import "C"
383-
384-
//export GoFunc
385-
func GoFunc() {
386-
println(42)
387-
}
388-
389-
//export GoFunc2
390-
func GoFunc2() {
391-
println(24)
392-
}
393-
394-
func main() {
395-
}
396-
`
397-
378+
func checkNumberOfExportedFunctionsWindows(t *testing.T, prog string, exportedFunctions int, wantAll bool) {
398379
tmpdir := t.TempDir()
399380

400381
srcfile := filepath.Join(tmpdir, "test.go")
@@ -403,7 +384,7 @@ func main() {
403384
t.Fatal(err)
404385
}
405386
argv := []string{"build", "-buildmode=c-shared"}
406-
if exportAllSymbols {
387+
if wantAll {
407388
argv = append(argv, "-ldflags", "-extldflags=-Wl,--export-all-symbols")
408389
}
409390
argv = append(argv, "-o", objfile, srcfile)
@@ -417,10 +398,36 @@ func main() {
417398
t.Fatalf("pe.Open failed: %v", err)
418399
}
419400
defer f.Close()
420-
section := f.Section(".edata")
401+
402+
_, pe64 := f.OptionalHeader.(*pe.OptionalHeader64)
403+
// grab the export data directory entry
404+
var idd pe.DataDirectory
405+
if pe64 {
406+
idd = f.OptionalHeader.(*pe.OptionalHeader64).DataDirectory[pe.IMAGE_DIRECTORY_ENTRY_EXPORT]
407+
} else {
408+
idd = f.OptionalHeader.(*pe.OptionalHeader32).DataDirectory[pe.IMAGE_DIRECTORY_ENTRY_EXPORT]
409+
}
410+
411+
// figure out which section contains the import directory table
412+
var section *pe.Section
413+
for _, s := range f.Sections {
414+
if s.Offset == 0 {
415+
continue
416+
}
417+
if s.VirtualAddress <= idd.VirtualAddress && idd.VirtualAddress-s.VirtualAddress < s.VirtualSize {
418+
section = s
419+
break
420+
}
421+
}
421422
if section == nil {
422-
t.Skip(".edata section is not present")
423+
t.Fatal("no section contains export directory")
423424
}
425+
d, err := section.Data()
426+
if err != nil {
427+
t.Fatal(err)
428+
}
429+
// seek to the virtual address specified in the export data directory
430+
d = d[idd.VirtualAddress-section.VirtualAddress:]
424431

425432
// TODO: deduplicate this struct from cmd/link/internal/ld/pe.go
426433
type IMAGE_EXPORT_DIRECTORY struct {
@@ -432,26 +439,22 @@ func main() {
432439
_ [3]uint32
433440
}
434441
var e IMAGE_EXPORT_DIRECTORY
435-
if err := binary.Read(section.Open(), binary.LittleEndian, &e); err != nil {
442+
if err := binary.Read(bytes.NewReader(d), binary.LittleEndian, &e); err != nil {
436443
t.Fatalf("binary.Read failed: %v", err)
437444
}
438445

439-
// Only the two exported functions and _cgo_dummy_export should be exported
440-
expectedNumber := uint32(3)
441-
442-
if exportAllSymbols {
443-
if e.NumberOfFunctions <= expectedNumber {
444-
t.Fatalf("missing exported functions: %v", e.NumberOfFunctions)
445-
}
446-
if e.NumberOfNames <= expectedNumber {
447-
t.Fatalf("missing exported names: %v", e.NumberOfNames)
446+
// Only the two exported functions and _cgo_dummy_export should be exported.
447+
// NumberOfNames is the number of functions exported with a unique name.
448+
// NumberOfFunctions can be higher than that because it also counts
449+
// functions exported only by ordinal, a unique number asigned by the linker,
450+
// and linkers might add an unknown number of their own ordinal-only functions.
451+
if wantAll {
452+
if e.NumberOfNames <= uint32(exportedFunctions) {
453+
t.Errorf("got %d exported names, want > %d", e.NumberOfNames, exportedFunctions)
448454
}
449455
} else {
450-
if e.NumberOfFunctions != expectedNumber {
451-
t.Fatalf("got %d exported functions; want %d", e.NumberOfFunctions, expectedNumber)
452-
}
453-
if e.NumberOfNames != expectedNumber {
454-
t.Fatalf("got %d exported names; want %d", e.NumberOfNames, expectedNumber)
456+
if e.NumberOfNames > uint32(exportedFunctions) {
457+
t.Errorf("got %d exported names, want <= %d", e.NumberOfNames, exportedFunctions)
455458
}
456459
}
457460
}
@@ -467,11 +470,42 @@ func TestNumberOfExportedFunctions(t *testing.T) {
467470

468471
t.Parallel()
469472

470-
t.Run("OnlyExported", func(t *testing.T) {
471-
checkNumberOfExportedFunctionsWindows(t, false)
473+
const prog0 = `
474+
package main
475+
476+
import "C"
477+
478+
func main() {
479+
}
480+
`
481+
482+
const prog2 = `
483+
package main
484+
485+
import "C"
486+
487+
//export GoFunc
488+
func GoFunc() {
489+
println(42)
490+
}
491+
492+
//export GoFunc2
493+
func GoFunc2() {
494+
println(24)
495+
}
496+
497+
func main() {
498+
}
499+
`
500+
// All programs export _cgo_dummy_export, so add 1 to the expected counts.
501+
t.Run("OnlyExported/0", func(t *testing.T) {
502+
checkNumberOfExportedFunctionsWindows(t, prog0, 0+1, false)
503+
})
504+
t.Run("OnlyExported/2", func(t *testing.T) {
505+
checkNumberOfExportedFunctionsWindows(t, prog2, 2+1, false)
472506
})
473507
t.Run("All", func(t *testing.T) {
474-
checkNumberOfExportedFunctionsWindows(t, true)
508+
checkNumberOfExportedFunctionsWindows(t, prog2, 2+1, true)
475509
})
476510
}
477511

src/cmd/go/go_test.go

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"debug/elf"
1010
"debug/macho"
1111
"debug/pe"
12-
"encoding/binary"
1312
"flag"
1413
"fmt"
1514
"go/format"
@@ -2131,38 +2130,6 @@ func testBuildmodePIE(t *testing.T, useCgo, setBuildmodeToPIE bool) {
21312130
if (dc & pe.IMAGE_DLLCHARACTERISTICS_DYNAMIC_BASE) == 0 {
21322131
t.Error("IMAGE_DLLCHARACTERISTICS_DYNAMIC_BASE flag is not set")
21332132
}
2134-
if useCgo {
2135-
// Test that only one symbol is exported (#40795).
2136-
// PIE binaries don´t require .edata section but unfortunately
2137-
// binutils doesn´t generate a .reloc section unless there is
2138-
// at least one symbol exported.
2139-
// See https://sourceware.org/bugzilla/show_bug.cgi?id=19011
2140-
section := f.Section(".edata")
2141-
if section == nil {
2142-
t.Skip(".edata section is not present")
2143-
}
2144-
// TODO: deduplicate this struct from cmd/link/internal/ld/pe.go
2145-
type IMAGE_EXPORT_DIRECTORY struct {
2146-
_ [2]uint32
2147-
_ [2]uint16
2148-
_ [2]uint32
2149-
NumberOfFunctions uint32
2150-
NumberOfNames uint32
2151-
_ [3]uint32
2152-
}
2153-
var e IMAGE_EXPORT_DIRECTORY
2154-
if err := binary.Read(section.Open(), binary.LittleEndian, &e); err != nil {
2155-
t.Fatalf("binary.Read failed: %v", err)
2156-
}
2157-
2158-
// Only _cgo_dummy_export should be exported
2159-
if e.NumberOfFunctions != 1 {
2160-
t.Fatalf("got %d exported functions; want 1", e.NumberOfFunctions)
2161-
}
2162-
if e.NumberOfNames != 1 {
2163-
t.Fatalf("got %d exported names; want 1", e.NumberOfNames)
2164-
}
2165-
}
21662133
default:
21672134
// testBuildmodePIE opens object files, so it needs to understand the object
21682135
// file format.

0 commit comments

Comments
 (0)