[go] cmd/internal/link: merge .pdata and .xdata sections from host object files
13 views
Skip to first unread message
Quim Muntal (Gerrit)
Oct 17, 2023, 5:54:09 AM10/17/23
to goph...@pubsubhelper.golang.org, Than McIntosh, Gopher Robot, Go LUCI, golang-co...@googlegroups.com
Patch set 2:Run-TryBot +1Commit-Queue +1
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Oct 17, 2023, 6:14:05 AM10/17/23
to goph...@pubsubhelper.golang.org, Gopher Robot, Go LUCI, Than McIntosh, golang-co...@googlegroups.com
Attention is currently required from: Quim Muntal, Than McIntosh.
Patch set 2:Commit-Queue +1
1 comment:
File src/cmd/link/internal/loadpe/ldpe.go:
Patch Set #1, Line 457:
// .pdata and .xdata sections can contain recordsWhat happens with COMDAT just out of curiosity? Similar setup?
.pdata/xdata symbols are special because they are not referenced from anywhere else, and a xdata symbol can contain a relocation to a function that is not reachable using a normal reachability graph, as it is dynamically called by the Windows runtime when an exception happens.
This makes it different to COMDAT sections/symbols, as these can't reference unreachable symbols.
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Oct 17, 2023, 6:30:14 AM10/17/23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Quim Muntal, Than McIntosh.
Quim Muntal uploaded patch set #3 to this change.
The following approvals got outdated and were removed: LUCI-TryBot-Result-1 by Go LUCI, Run-TryBot+1 by Quim Muntal, TryBot-Result-1 by Gopher Robot
cmd/internal/link: merge .pdata and .xdata sections from host object files
The Go linker doesn't currently merge .pdata/.xdata sections from the
host object files generated by the C compiler when using internal
linking. This means that the stack can't be unwind in C -> Go.
This CL fixes that and adds a test to ensure that the stack can be
unwind in C -> Go and Go -> C transitions, which was not well tested.
Updates #57302
Change-Id: Ie86a5e6e30b80978277e66ccc2c48550e51263c8
---
A src/cmd/cgo/internal/test/callback_windows.go
A src/cmd/cgo/internal/test/cgo_windows_test.go
M src/cmd/link/internal/ld/data.go
M src/cmd/link/internal/ld/lib.go
M src/cmd/link/internal/ld/pe.go
M src/cmd/link/internal/ld/seh.go
M src/cmd/link/internal/loadpe/ldpe.go
A src/cmd/link/internal/loadpe/seh.go
8 files changed, 369 insertions(+), 53 deletions(-)
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Oct 17, 2023, 6:30:24 AM10/17/23
to goph...@pubsubhelper.golang.org, Gopher Robot, Go LUCI, Than McIntosh, golang-co...@googlegroups.com
Attention is currently required from: Than McIntosh.
Patch set 3:Run-TryBot +1Commit-Queue +1
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Oct 17, 2023, 9:13:10 AM10/17/23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Quim Muntal, Than McIntosh.
Quim Muntal uploaded patch set #4 to this change.
The following approvals got outdated and were removed: LUCI-TryBot-Result-1 by Go LUCI, Run-TryBot+1 by Quim Muntal, TryBot-Result-1 by Gopher Robot
cmd/internal/link: merge .pdata and .xdata sections from host object files
The Go linker doesn't currently merge .pdata/.xdata sections from the
host object files generated by the C compiler when using internal
linking. This means that the stack can't be unwind in C -> Go.
This CL fixes that and adds a test to ensure that the stack can be
unwind in C -> Go and Go -> C transitions, which was not well tested.
Updates #57302
Change-Id: Ie86a5e6e30b80978277e66ccc2c48550e51263c8
---
A src/cmd/cgo/internal/test/callback_windows.go
A src/cmd/cgo/internal/test/cgo_windows_test.go
M src/cmd/link/internal/ld/data.go
M src/cmd/link/internal/ld/lib.go
M src/cmd/link/internal/ld/pe.go
M src/cmd/link/internal/ld/seh.go
M src/cmd/link/internal/loadpe/ldpe.go
A src/cmd/link/internal/loadpe/seh.go
8 files changed, 357 insertions(+), 53 deletions(-)
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Oct 17, 2023, 9:13:22 AM10/17/23
to goph...@pubsubhelper.golang.org, Gopher Robot, Go LUCI, Than McIntosh, golang-co...@googlegroups.com
Attention is currently required from: Than McIntosh.
Patch set 4:Run-TryBot +1Commit-Queue +1
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Oct 17, 2023, 9:39:58 AM10/17/23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Quim Muntal, Than McIntosh.
Quim Muntal uploaded patch set #5 to this change.
The following approvals got outdated and were removed: LUCI-TryBot-Result-1 by Go LUCI, Run-TryBot+1 by Quim Muntal, TryBot-Result-1 by Gopher Robot
cmd/internal/link: merge .pdata and .xdata sections from host object files
The Go linker doesn't currently merge .pdata/.xdata sections from the
host object files generated by the C compiler when using internal
linking. This means that the stack can't be unwind in C -> Go.
This CL fixes that and adds a test to ensure that the stack can be
unwind in C -> Go and Go -> C transitions, which was not well tested.
Updates #57302
Change-Id: Ie86a5e6e30b80978277e66ccc2c48550e51263c8
---
A src/cmd/cgo/internal/test/callback_windows.go
A src/cmd/cgo/internal/test/cgo_windows_test.go
M src/cmd/link/internal/ld/data.go
M src/cmd/link/internal/ld/lib.go
M src/cmd/link/internal/ld/pe.go
M src/cmd/link/internal/ld/seh.go
M src/cmd/link/internal/loadpe/ldpe.go
A src/cmd/link/internal/loadpe/seh.go
8 files changed, 368 insertions(+), 53 deletions(-)
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Oct 17, 2023, 9:40:11 AM10/17/23
to goph...@pubsubhelper.golang.org, Gopher Robot, Go LUCI, Than McIntosh, golang-co...@googlegroups.com
Attention is currently required from: Than McIntosh.
Patch set 5:Run-TryBot +1Commit-Queue +1
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Mauri de Souza Meneguzzo (Gerrit)
Oct 17, 2023, 10:52:09 AM10/17/23
to Quim Muntal, goph...@pubsubhelper.golang.org, Than McIntosh, Cherry Mui, Gopher Robot, Go LUCI, golang-co...@googlegroups.com
Attention is currently required from: Cherry Mui, Quim Muntal, Than McIntosh.
1 comment:
File src/cmd/cgo/internal/test/callback_windows.go:
Patch Set #5, Line 10:
#ifdef _AMD64_should this test be guarded with a go:build amd64?
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Oct 17, 2023, 10:58:48 AM10/17/23
to goph...@pubsubhelper.golang.org, Mauri de Souza Meneguzzo, Than McIntosh, Cherry Mui, Gopher Robot, Go LUCI, golang-co...@googlegroups.com
Attention is currently required from: Cherry Mui, Mauri de Souza Meneguzzo, Than McIntosh.
1 comment:
File src/cmd/cgo/internal/test/callback_windows.go:
Patch Set #5, Line 10:
#ifdef _AMD64_should this test be guarded with a go:build amd64?
I prefer that it appears as a skipped test so it is easier to see that some work is missing here.
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Nov 4, 2023, 3:48:19 PM11/4/23
to goph...@pubsubhelper.golang.org, Mauri de Souza Meneguzzo, Than McIntosh, Cherry Mui, Gopher Robot, Go LUCI, golang-co...@googlegroups.com
Attention is currently required from: Cherry Mui, Mauri de Souza Meneguzzo, Than McIntosh.
1 comment:
Patchset:
Hi! Can I use some reviews? I'd like to merge this in this release cycle. Thanks.
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Nov 6, 2023, 4:49:05 PM11/6/23
to Quim Muntal, goph...@pubsubhelper.golang.org, Than McIntosh, Mauri de Souza Meneguzzo, Cherry Mui, Gopher Robot, Go LUCI, golang-co...@googlegroups.com
Attention is currently required from: Cherry Mui, Mauri de Souza Meneguzzo, Quim Muntal.
Patch set 5:Code-Review +2
2 comments:
Patchset:
LGTM
File src/cmd/link/internal/loadpe/ldpe.go:
Patch Set #1, Line 457:
// .pdata and .xdata sections can contain records
> What happens with COMDAT just out of curiosity? Similar setup? […]
I guess my question was more along the lines of whether a .pdata/.xdata symbol would ever be inside a COMDAT group. Sounds like the answer is no, this won't happen?
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Heschi Kreinick (Gerrit)
Nov 8, 2023, 11:58:00 AM11/8/23
to Quim Muntal, goph...@pubsubhelper.golang.org, Than McIntosh, Mauri de Souza Meneguzzo, Cherry Mui, Gopher Robot, Go LUCI, golang-co...@googlegroups.com
Attention is currently required from: Cherry Mui, Mauri de Souza Meneguzzo, Quim Muntal.
Patch set 5:Code-Review +1
Quim Muntal (Gerrit)
Nov 8, 2023, 4:24:40 PM11/8/23
to goph...@pubsubhelper.golang.org, Heschi Kreinick, Than McIntosh, Mauri de Souza Meneguzzo, Cherry Mui, Gopher Robot, Go LUCI, golang-co...@googlegroups.com
Attention is currently required from: Cherry Mui, Mauri de Souza Meneguzzo.
1 comment:
File src/cmd/link/internal/loadpe/ldpe.go:
Patch Set #1, Line 457:
// .pdata and .xdata sections can contain records
I guess my question was more along the lines of whether a .pdata/. […]
Hmm, after googling a bit, it seems that COMDAT can contain a pdata/xdata symbol.I'll have to investigate how this fact affect my changes.
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Cherry Mui (Gerrit)
Nov 9, 2023, 11:26:39 AM11/9/23
to Quim Muntal, goph...@pubsubhelper.golang.org, Heschi Kreinick, Than McIntosh, Mauri de Souza Meneguzzo, Gopher Robot, Go LUCI, golang-co...@googlegroups.com
Attention is currently required from: Mauri de Souza Meneguzzo, Quim Muntal.
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Nov 9, 2023, 11:44:06 AM11/9/23
to Quim Muntal, goph...@pubsubhelper.golang.org, Cherry Mui, Heschi Kreinick, Than McIntosh, Mauri de Souza Meneguzzo, Gopher Robot, Go LUCI, golang-co...@googlegroups.com
Attention is currently required from: Mauri de Souza Meneguzzo, Quim Muntal.
Patch Set #1, Line 457:
// .pdata and .xdata sections can contain records
Hmm, after googling a bit, it seems that COMDAT can contain a pdata/xdata symbol. […]
OK, that's interesting to note. I wouldn't worry about it too much IMHO, I think the existing COMDAT machinery should handle it, unless I am missing something.
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Nov 9, 2023, 12:36:43 PM11/9/23
to goph...@pubsubhelper.golang.org, Cherry Mui, Heschi Kreinick, Than McIntosh, Mauri de Souza Meneguzzo, Gopher Robot, Go LUCI, golang-co...@googlegroups.com
Attention is currently required from: Mauri de Souza Meneguzzo.
Patch set 6:Run-TryBot +1Commit-Queue +1
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Nov 9, 2023, 1:42:37 PM11/9/23
to goph...@pubsubhelper.golang.org, Gopher Robot, Go LUCI, Cherry Mui, Heschi Kreinick, Than McIntosh, Mauri de Souza Meneguzzo, golang-co...@googlegroups.com
Attention is currently required from: Mauri de Souza Meneguzzo, Quim Muntal.
Quim Muntal removed a vote from this change.
Removed TryBot-Result-1 by Gopher Robot <go...@golang.org>
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Nov 9, 2023, 1:42:47 PM11/9/23
to goph...@pubsubhelper.golang.org, Gopher Robot, Go LUCI, Cherry Mui, Heschi Kreinick, Than McIntosh, Mauri de Souza Meneguzzo, golang-co...@googlegroups.com
Attention is currently required from: Mauri de Souza Meneguzzo.
1 comment:
Patchset:
1 of 48 SlowBots failed. […]
arm64 fails on tip, this failure is not related with this cl.
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Nov 9, 2023, 2:00:58 PM11/9/23
to goph...@pubsubhelper.golang.org, Gopher Robot, Go LUCI, Cherry Mui, Heschi Kreinick, Than McIntosh, golang-co...@googlegroups.com
Attention is currently required from: Quim Muntal.
Quim Muntal removed a vote from this change.
Removed TryBot-Result-1 by Gopher Robot <go...@golang.org>
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Nov 9, 2023, 2:01:15 PM11/9/23
to goph...@pubsubhelper.golang.org, Gopher Robot, Go LUCI, Cherry Mui, Heschi Kreinick, Than McIntosh, golang-co...@googlegroups.com
1 of 48 SlowBots failed. […]
Acknowledged
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Nov 9, 2023, 2:01:35 PM11/9/23
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Gopher Robot, Go LUCI, Cherry Mui, Heschi Kreinick, Than McIntosh, golang-co...@googlegroups.com
Quim Muntal submitted this change.
5 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Approvals:
Heschi Kreinick: Looks good to me, but someone else must approve
Cherry Mui: Looks good to me, but someone else must approve
Than McIntosh: Looks good to me, approved
Go LUCI: TryBots succeeded
cmd/internal/link: merge .pdata and .xdata sections from host object files
The Go linker doesn't currently merge .pdata/.xdata sections from the
host object files generated by the C compiler when using internal
linking. This means that the stack can't be unwind in C -> Go.
This CL fixes that and adds a test to ensure that the stack can be
unwind in C -> Go and Go -> C transitions, which was not well tested.
Updates #57302
Change-Id: Ie86a5e6e30b80978277e66ccc2c48550e51263c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/534555
Reviewed-by: Heschi Kreinick <hes...@google.com>
Reviewed-by: Cherry Mui <cher...@google.com>
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <th...@google.com>
---
A src/cmd/cgo/internal/test/callback_windows.go
A src/cmd/cgo/internal/test/cgo_windows_test.go
M src/cmd/link/internal/ld/data.go
M src/cmd/link/internal/ld/lib.go
M src/cmd/link/internal/ld/pe.go
M src/cmd/link/internal/ld/seh.go
M src/cmd/link/internal/loadpe/ldpe.go
A src/cmd/link/internal/loadpe/seh.go
8 files changed, 368 insertions(+), 53 deletions(-)
diff --git a/src/cmd/cgo/internal/test/callback_windows.go b/src/cmd/cgo/internal/test/callback_windows.go
new file mode 100644
index 0000000..95e97c9
--- /dev/null
+++ b/src/cmd/cgo/internal/test/callback_windows.go
@@ -0,0 +1,133 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package cgotest
+
+/*
+#include <windows.h>
+USHORT backtrace(ULONG FramesToCapture, PVOID *BackTrace) {
+#ifdef _AMD64_
+ CONTEXT context;
+ RtlCaptureContext(&context);
+ ULONG64 ControlPc;
+ ControlPc = context.Rip;
+ int i;
+ for (i = 0; i < FramesToCapture; i++) {
+ PRUNTIME_FUNCTION FunctionEntry;
+ ULONG64 ImageBase;
+ VOID *HandlerData;
+ ULONG64 EstablisherFrame;
+
+ FunctionEntry = RtlLookupFunctionEntry(ControlPc, &ImageBase, NULL);
+
+ if (!FunctionEntry) {
+ // For simplicity, don't unwind leaf entries, which are not used in this test.
+ break;
+ } else {
+ RtlVirtualUnwind(0, ImageBase, ControlPc, FunctionEntry, &context, &HandlerData, &EstablisherFrame, NULL);
+ }
+
+ ControlPc = context.Rip;
+ // Check if we left the user range.
+ if (ControlPc < 0x10000) {
+ break;
+ }
+
+ BackTrace[i] = (PVOID)(ControlPc);
+ }
+ return i;
+#else
+ return 0;
+#endif
+}
+*/
+import "C"
+
+import (
+ "internal/testenv"
+ "reflect"
+ "runtime"
+ "strings"
+ "testing"
+ "unsafe"
+)
+
+// Test that the stack can be unwound through a call out and call back
+// into Go.
+func testCallbackCallersSEH(t *testing.T) {
+ testenv.SkipIfOptimizationOff(t) // This test requires inlining.
+ if runtime.Compiler != "gc" {
+ // The exact function names are not going to be the same.
+ t.Skip("skipping for non-gc toolchain")
+ }
+ if runtime.GOARCH != "amd64" {
+ // TODO: support SEH on other architectures.
+ t.Skip("skipping on non-amd64")
+ }
+ const cgoexpPrefix = "_cgoexp_"
+ want := []string{
+ "runtime.asmcgocall_landingpad",
+ "runtime.asmcgocall",
+ "runtime.cgocall",
+ "test._Cfunc_backtrace",
+ "test.testCallbackCallersSEH.func1.1",
+ "test.testCallbackCallersSEH.func1",
+ "test.goCallback",
+ cgoexpPrefix + "goCallback",
+ "runtime.cgocallbackg1",
+ "runtime.cgocallbackg",
+ "runtime.cgocallbackg",
+ "runtime.cgocallback",
+ "crosscall2",
+ "runtime.asmcgocall_landingpad",
+ "runtime.asmcgocall",
+ "runtime.cgocall",
+ "test._Cfunc_callback",
+ "test.nestedCall.func1",
+ "test.nestedCall",
+ "test.testCallbackCallersSEH",
+ "test.TestCallbackCallersSEH",
+ "testing.tRunner",
+ "testing.(*T).Run.gowrap1",
+ "runtime.goexit",
+ }
+ pc := make([]uintptr, 100)
+ n := 0
+ nestedCall(func() {
+ n = int(C.backtrace(C.DWORD(len(pc)), (*C.PVOID)(unsafe.Pointer(&pc[0]))))
+ })
+ got := make([]string, 0, n)
+ for i := 0; i < n; i++ {
+ f := runtime.FuncForPC(pc[i] - 1)
+ if f == nil {
+ continue
+ }
+ fname := f.Name()
+ switch fname {
+ case "goCallback", "callback":
+ // TODO(qmuntal): investigate why these functions don't appear
+ // when using the external linker.
+ continue
+ }
+ // Skip cgo-generated functions, the runtime might not know about them,
+ // depending on the link mode.
+ if strings.HasPrefix(fname, "_cgo_") {
+ continue
+ }
+ // Remove the cgo-generated random prefix.
+ if strings.HasPrefix(fname, cgoexpPrefix) {
+ idx := strings.Index(fname[len(cgoexpPrefix):], "_")
+ if idx >= 0 {
+ fname = cgoexpPrefix + fname[len(cgoexpPrefix)+idx+1:]
+ }
+ }
+ // In module mode, this package has a fully-qualified import path.
+ // Remove it if present.
+ fname = strings.TrimPrefix(fname, "cmd/cgo/internal/")
+ got = append(got, fname)
+ }
+ if !reflect.DeepEqual(want, got) {
+ t.Errorf("incorrect backtrace:\nwant:\t%v\ngot:\t%v", want, got)
+ }
+}
diff --git a/src/cmd/cgo/internal/test/cgo_windows_test.go b/src/cmd/cgo/internal/test/cgo_windows_test.go
new file mode 100644
index 0000000..7bbed5b
--- /dev/null
+++ b/src/cmd/cgo/internal/test/cgo_windows_test.go
@@ -0,0 +1,11 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build cgo && windows
+
+package cgotest
+
+import "testing"
+
+func TestCallbackCallersSEH(t *testing.T) { testCallbackCallersSEH(t) }
diff --git a/src/cmd/link/internal/ld/data.go b/src/cmd/link/internal/ld/data.go
index 8534a55..2d761c7 100644
--- a/src/cmd/link/internal/ld/data.go
+++ b/src/cmd/link/internal/ld/data.go
@@ -1161,11 +1161,11 @@
}
func pdatablk(ctxt *Link, out *OutBuf, addr int64, size int64) {
- writeBlocks(ctxt, out, ctxt.outSem, ctxt.loader, []loader.Sym{sehp.pdata}, addr, size, zeros[:])
+ writeBlocks(ctxt, out, ctxt.outSem, ctxt.loader, sehp.pdata, addr, size, zeros[:])
}
func xdatablk(ctxt *Link, out *OutBuf, addr int64, size int64) {
- writeBlocks(ctxt, out, ctxt.outSem, ctxt.loader, []loader.Sym{sehp.xdata}, addr, size, zeros[:])
+ writeBlocks(ctxt, out, ctxt.outSem, ctxt.loader, sehp.xdata, addr, size, zeros[:])
}
var covCounterDataStartOff, covCounterDataLen uint64
@@ -2199,14 +2199,14 @@
// allocateSEHSections allocate a sym.Section object for SEH
// symbols, and assigns symbols to sections.
func (state *dodataState) allocateSEHSections(ctxt *Link) {
- if sehp.pdata > 0 {
- sect := state.allocateDataSectionForSym(&Segpdata, sehp.pdata, 04)
- state.assignDsymsToSection(sect, []loader.Sym{sehp.pdata}, sym.SRODATA, aligndatsize)
+ if len(sehp.pdata) > 0 {
+ sect := state.allocateNamedDataSection(&Segpdata, ".pdata", []sym.SymKind{}, 04)
+ state.assignDsymsToSection(sect, sehp.pdata, sym.SRODATA, aligndatsize)
state.checkdatsize(sym.SSEHSECT)
}
- if sehp.xdata > 0 {
+ if len(sehp.xdata) > 0 {
sect := state.allocateNamedDataSection(&Segxdata, ".xdata", []sym.SymKind{}, 04)
- state.assignDsymsToSection(sect, []loader.Sym{sehp.xdata}, sym.SRODATA, aligndatsize)
+ state.assignDsymsToSection(sect, sehp.xdata, sym.SRODATA, aligndatsize)
state.checkdatsize(sym.SSEHSECT)
}
}
@@ -2872,7 +2872,12 @@
}
}
- for _, s := range []loader.Sym{sehp.pdata, sehp.xdata} {
+ for _, s := range sehp.pdata {
+ if sect := ldr.SymSect(s); sect != nil {
+ ldr.AddToSymValue(s, int64(sect.Vaddr))
+ }
+ }
+ for _, s := range sehp.xdata {
if sect := ldr.SymSect(s); sect != nil {
ldr.AddToSymValue(s, int64(sect.Vaddr))
}
diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go
index a6f7173..b603fba 100644
--- a/src/cmd/link/internal/ld/lib.go
+++ b/src/cmd/link/internal/ld/lib.go
@@ -2220,15 +2220,21 @@
0xc401, // arm
0x64aa: // arm64
ldpe := func(ctxt *Link, f *bio.Reader, pkg string, length int64, pn string) {
- textp, rsrc, err := loadpe.Load(ctxt.loader, ctxt.Arch, ctxt.IncVersion(), f, pkg, length, pn)
+ ls, err := loadpe.Load(ctxt.loader, ctxt.Arch, ctxt.IncVersion(), f, pkg, length, pn)
if err != nil {
Errorf(nil, "%v", err)
return
}
- if len(rsrc) != 0 {
- setpersrc(ctxt, rsrc)
+ if len(ls.Resources) != 0 {
+ setpersrc(ctxt, ls.Resources)
}
- ctxt.Textp = append(ctxt.Textp, textp...)
+ if ls.PData != 0 {
+ sehp.pdata = append(sehp.pdata, ls.PData)
+ }
+ if ls.XData != 0 {
+ sehp.xdata = append(sehp.xdata, ls.XData)
+ }
+ ctxt.Textp = append(ctxt.Textp, ls.Textp...)
}
return ldhostobj(ldpe, ctxt.HeadType, f, pkg, length, pn, file)
}
diff --git a/src/cmd/link/internal/ld/pe.go b/src/cmd/link/internal/ld/pe.go
index 7c585b3..8cfecaf 100644
--- a/src/cmd/link/internal/ld/pe.go
+++ b/src/cmd/link/internal/ld/pe.go
@@ -635,11 +635,11 @@
{f.rdataSect, &Segrodata, ctxt.datap},
{f.dataSect, &Segdata, ctxt.datap},
}
- if sehp.pdata != 0 {
- sects = append(sects, relsect{f.pdataSect, &Segpdata, []loader.Sym{sehp.pdata}})
+ if len(sehp.pdata) != 0 {
+ sects = append(sects, relsect{f.pdataSect, &Segpdata, sehp.pdata})
}
- if sehp.xdata != 0 {
- sects = append(sects, relsect{f.xdataSect, &Segxdata, []loader.Sym{sehp.xdata}})
+ if len(sehp.xdata) != 0 {
+ sects = append(sects, relsect{f.xdataSect, &Segxdata, sehp.xdata})
}
for _, s := range sects {
s.peSect.emitRelocations(ctxt.Out, func() int {
diff --git a/src/cmd/link/internal/ld/seh.go b/src/cmd/link/internal/ld/seh.go
index 43b5176..9f0f747 100644
--- a/src/cmd/link/internal/ld/seh.go
+++ b/src/cmd/link/internal/ld/seh.go
@@ -11,8 +11,8 @@
)
var sehp struct {
- pdata loader.Sym
- xdata loader.Sym
+ pdata []sym.LoaderSym
+ xdata []sym.LoaderSym
}
func writeSEH(ctxt *Link) {
@@ -72,6 +72,6 @@
pdata.AddPEImageRelativeAddrPlus(ctxt.Arch, s, ldr.SymSize(s))
pdata.AddPEImageRelativeAddrPlus(ctxt.Arch, xdata.Sym(), off)
}
- sehp.pdata = pdata.Sym()
- sehp.xdata = xdata.Sym()
+ sehp.pdata = append(sehp.pdata, pdata.Sym())
+ sehp.xdata = append(sehp.xdata, xdata.Sym())
}
diff --git a/src/cmd/link/internal/loadpe/ldpe.go b/src/cmd/link/internal/loadpe/ldpe.go
index 2e9880b..d1b7ae2 100644
--- a/src/cmd/link/internal/loadpe/ldpe.go
+++ b/src/cmd/link/internal/loadpe/ldpe.go
@@ -221,12 +221,17 @@
// is symbol size (or -1 if we're using the "any" strategy).
var comdatDefinitions map[string]int64
+// Symbols contains the symbols that can be loaded from a PE file.
+type Symbols struct {
+ Textp []loader.Sym // text symbols
+ Resources []loader.Sym // .rsrc section or set of .rsrc$xx sections
+ PData loader.Sym
+ XData loader.Sym
+}
+
// Load loads the PE file pn from input.
-// Symbols from the object file are created via the loader 'l',
-// and a slice of the text symbols is returned.
-// If an .rsrc section or set of .rsrc$xx sections is found, its symbols are
-// returned as rsrc.
-func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Reader, pkg string, length int64, pn string) (textp []loader.Sym, rsrc []loader.Sym, err error) {
+// Symbols from the object file are created via the loader 'l'.
+func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Reader, pkg string, length int64, pn string) (*Symbols, error) {
state := &peLoaderState{
l: l,
arch: arch,
@@ -249,11 +254,13 @@
// TODO: replace pe.NewFile with pe.Load (grep for "add Load function" in debug/pe for details)
f, err := pe.NewFile(sr)
if err != nil {
- return nil, nil, err
+ return nil, err
}
defer f.Close()
state.f = f
+ var ls Symbols
+
// TODO return error if found .cormeta
// create symbols for mapped sections
@@ -274,7 +281,12 @@
switch sect.Characteristics & (pe.IMAGE_SCN_CNT_UNINITIALIZED_DATA | pe.IMAGE_SCN_CNT_INITIALIZED_DATA | pe.IMAGE_SCN_MEM_READ | pe.IMAGE_SCN_MEM_WRITE | pe.IMAGE_SCN_CNT_CODE | pe.IMAGE_SCN_MEM_EXECUTE) {
case pe.IMAGE_SCN_CNT_INITIALIZED_DATA | pe.IMAGE_SCN_MEM_READ: //.rdata
- bld.SetType(sym.SRODATA)
+ if issehsect(arch, sect) {
+ bld.SetType(sym.SSEHSECT)
+ bld.SetAlign(4)
+ } else {
+ bld.SetType(sym.SRODATA)
+ }
case pe.IMAGE_SCN_CNT_UNINITIALIZED_DATA | pe.IMAGE_SCN_MEM_READ | pe.IMAGE_SCN_MEM_WRITE: //.bss
bld.SetType(sym.SNOPTRBSS)
@@ -286,13 +298,13 @@
bld.SetType(sym.STEXT)
default:
- return nil, nil, fmt.Errorf("unexpected flags %#06x for PE section %s", sect.Characteristics, sect.Name)
+ return nil, fmt.Errorf("unexpected flags %#06x for PE section %s", sect.Characteristics, sect.Name)
}
if bld.Type() != sym.SNOPTRBSS {
data, err := sect.Data()
if err != nil {
- return nil, nil, err
+ return nil, err
}
state.sectdata[sect] = data
bld.SetData(data)
@@ -300,13 +312,19 @@
bld.SetSize(int64(sect.Size))
state.sectsyms[sect] = s
if sect.Name == ".rsrc" || strings.HasPrefix(sect.Name, ".rsrc$") {
- rsrc = append(rsrc, s)
+ ls.Resources = append(ls.Resources, s)
+ } else if bld.Type() == sym.SSEHSECT {
+ if sect.Name == ".pdata" {
+ ls.PData = s
+ } else if sect.Name == ".xdata" {
+ ls.XData = s
+ }
}
}
// Make a prepass over the symbols to collect info about COMDAT symbols.
if err := state.preprocessSymbols(); err != nil {
- return nil, nil, err
+ return nil, err
}
// load relocations
@@ -327,22 +345,23 @@
}
splitResources := strings.HasPrefix(rsect.Name, ".rsrc$")
+ issehsect := issehsect(arch, rsect)
sb := l.MakeSymbolUpdater(state.sectsyms[rsect])
for j, r := range rsect.Relocs {
if int(r.SymbolTableIndex) >= len(f.COFFSymbols) {
- return nil, nil, fmt.Errorf("relocation number %d symbol index idx=%d cannot be large then number of symbols %d", j, r.SymbolTableIndex, len(f.COFFSymbols))
+ return nil, fmt.Errorf("relocation number %d symbol index idx=%d cannot be large then number of symbols %d", j, r.SymbolTableIndex, len(f.COFFSymbols))
}
pesym := &f.COFFSymbols[r.SymbolTableIndex]
_, gosym, err := state.readpesym(pesym)
if err != nil {
- return nil, nil, err
+ return nil, err
}
if gosym == 0 {
name, err := pesym.FullName(f.StringTable)
if err != nil {
name = string(pesym.Name[:])
}
- return nil, nil, fmt.Errorf("reloc of invalid sym %s idx=%d type=%d", name, r.SymbolTableIndex, pesym.Type)
+ return nil, fmt.Errorf("reloc of invalid sym %s idx=%d type=%d", name, r.SymbolTableIndex, pesym.Type)
}
rSym := gosym
@@ -352,21 +371,29 @@
var rType objabi.RelocType
switch arch.Family {
default:
- return nil, nil, fmt.Errorf("%s: unsupported arch %v", pn, arch.Family)
+ return nil, fmt.Errorf("%s: unsupported arch %v", pn, arch.Family)
case sys.I386, sys.AMD64:
switch r.Type {
default:
- return nil, nil, fmt.Errorf("%s: %v: unknown relocation type %v", pn, state.sectsyms[rsect], r.Type)
+ return nil, fmt.Errorf("%s: %v: unknown relocation type %v", pn, state.sectsyms[rsect], r.Type)
case IMAGE_REL_I386_REL32, IMAGE_REL_AMD64_REL32,
IMAGE_REL_AMD64_ADDR32, // R_X86_64_PC32
IMAGE_REL_AMD64_ADDR32NB:
- rType = objabi.R_PCREL
+ if r.Type == IMAGE_REL_AMD64_ADDR32NB {
+ rType = objabi.R_PEIMAGEOFF
+ } else {
+ rType = objabi.R_PCREL
+ }
rAdd = int64(int32(binary.LittleEndian.Uint32(state.sectdata[rsect][rOff:])))
case IMAGE_REL_I386_DIR32NB, IMAGE_REL_I386_DIR32:
- rType = objabi.R_ADDR
+ if r.Type == IMAGE_REL_I386_DIR32NB {
+ rType = objabi.R_PEIMAGEOFF
+ } else {
+ rType = objabi.R_ADDR
+ }
// load addend from image
rAdd = int64(int32(binary.LittleEndian.Uint32(state.sectdata[rsect][rOff:])))
@@ -383,7 +410,7 @@
case sys.ARM:
switch r.Type {
default:
- return nil, nil, fmt.Errorf("%s: %v: unknown ARM relocation type %v", pn, state.sectsyms[rsect], r.Type)
+ return nil, fmt.Errorf("%s: %v: unknown ARM relocation type %v", pn, state.sectsyms[rsect], r.Type)
case IMAGE_REL_ARM_SECREL:
rType = objabi.R_PCREL
@@ -391,7 +418,11 @@
rAdd = int64(int32(binary.LittleEndian.Uint32(state.sectdata[rsect][rOff:])))
case IMAGE_REL_ARM_ADDR32, IMAGE_REL_ARM_ADDR32NB:
- rType = objabi.R_ADDR
+ if r.Type == IMAGE_REL_ARM_ADDR32NB {
+ rType = objabi.R_PEIMAGEOFF
+ } else {
+ rType = objabi.R_ADDR
+ }
rAdd = int64(int32(binary.LittleEndian.Uint32(state.sectdata[rsect][rOff:])))
@@ -404,10 +435,14 @@
case sys.ARM64:
switch r.Type {
default:
- return nil, nil, fmt.Errorf("%s: %v: unknown ARM64 relocation type %v", pn, state.sectsyms[rsect], r.Type)
+ return nil, fmt.Errorf("%s: %v: unknown ARM64 relocation type %v", pn, state.sectsyms[rsect], r.Type)
case IMAGE_REL_ARM64_ADDR32, IMAGE_REL_ARM64_ADDR32NB:
- rType = objabi.R_ADDR
+ if r.Type == IMAGE_REL_ARM64_ADDR32NB {
+ rType = objabi.R_PEIMAGEOFF
+ } else {
+ rType = objabi.R_ADDR
+ }
rAdd = int64(int32(binary.LittleEndian.Uint32(state.sectdata[rsect][rOff:])))
}
@@ -420,12 +455,20 @@
if issect(pesym) || splitResources {
rAdd += int64(pesym.Value)
}
+ if issehsect {
+ // .pdata and .xdata sections can contain records
+ // associated to functions that won't be used in
+ // the final binary, in which case the relocation
+ // target symbol won't be reachable.
+ rType |= objabi.R_WEAK
+ }
rel, _ := sb.AddRel(rType)
rel.SetOff(rOff)
rel.SetSiz(rSize)
rel.SetSym(rSym)
rel.SetAdd(rAdd)
+
}
sb.SortRelocs()
@@ -439,7 +482,7 @@
name, err := pesym.FullName(f.StringTable)
if err != nil {
- return nil, nil, err
+ return nil, err
}
if name == "" {
continue
@@ -477,7 +520,7 @@
bld, s, err := state.readpesym(pesym)
if err != nil {
- return nil, nil, err
+ return nil, err
}
if pesym.SectionNumber == 0 { // extern
@@ -491,14 +534,14 @@
} else if pesym.SectionNumber > 0 && int(pesym.SectionNumber) <= len(f.Sections) {
sect = f.Sections[pesym.SectionNumber-1]
if _, found := state.sectsyms[sect]; !found {
- return nil, nil, fmt.Errorf("%s: %v: missing sect.sym", pn, s)
+ return nil, fmt.Errorf("%s: %v: missing sect.sym", pn, s)
}
} else {
- return nil, nil, fmt.Errorf("%s: %v: sectnum < 0!", pn, s)
+ return nil, fmt.Errorf("%s: %v: sectnum < 0!", pn, s)
}
if sect == nil {
- return nil, nil, nil
+ return nil, nil
}
// Check for COMDAT symbol.
@@ -517,7 +560,7 @@
}
outerName := l.SymName(l.OuterSym(s))
sectName := l.SymName(state.sectsyms[sect])
- return nil, nil, fmt.Errorf("%s: duplicate symbol reference: %s in both %s and %s", pn, l.SymName(s), outerName, sectName)
+ return nil, fmt.Errorf("%s: duplicate symbol reference: %s in both %s and %s", pn, l.SymName(s), outerName, sectName)
}
bld = makeUpdater(l, bld, s)
@@ -528,7 +571,7 @@
bld.SetSize(4)
if l.SymType(sectsym) == sym.STEXT {
if bld.External() && !bld.DuplicateOK() {
- return nil, nil, fmt.Errorf("%s: duplicate symbol definition", l.SymName(s))
+ return nil, fmt.Errorf("%s: duplicate symbol definition", l.SymName(s))
}
bld.SetExternal(true)
}
@@ -536,7 +579,7 @@
// This is a COMDAT definition. Record that we're picking
// this instance so that we can ignore future defs.
if _, ok := comdatDefinitions[l.SymName(s)]; ok {
- return nil, nil, fmt.Errorf("internal error: preexisting COMDAT definition for %q", name)
+ return nil, fmt.Errorf("internal error: preexisting COMDAT definition for %q", name)
}
comdatDefinitions[l.SymName(s)] = sz
}
@@ -554,15 +597,19 @@
if l.SymType(s) == sym.STEXT {
for ; s != 0; s = l.SubSym(s) {
if l.AttrOnList(s) {
- return nil, nil, fmt.Errorf("symbol %s listed multiple times", l.SymName(s))
+ return nil, fmt.Errorf("symbol %s listed multiple times", l.SymName(s))
}
l.SetAttrOnList(s, true)
- textp = append(textp, s)
+ ls.Textp = append(ls.Textp, s)
}
}
}
- return textp, rsrc, nil
+ if ls.PData != 0 {
+ processSEH(l, arch, ls.PData, ls.XData)
+ }
+
+ return &ls, nil
}
// PostProcessImports works to resolve inconsistencies with DLL import
@@ -643,6 +690,10 @@
return nil
}
+func issehsect(arch *sys.Arch, s *pe.Section) bool {
+ return arch.Family == sys.AMD64 && (s.Name == ".pdata" || s.Name == ".xdata")
+}
+
func issect(s *pe.COFFSymbol) bool {
return s.StorageClass == IMAGE_SYM_CLASS_STATIC && s.Type == 0 && s.Name[0] == '.'
}
diff --git a/src/cmd/link/internal/loadpe/seh.go b/src/cmd/link/internal/loadpe/seh.go
new file mode 100644
index 0000000..a97595c
--- /dev/null
+++ b/src/cmd/link/internal/loadpe/seh.go
@@ -0,0 +1,109 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package loadpe
+
+import (
+ "cmd/internal/objabi"
+ "cmd/internal/sys"
+ "cmd/link/internal/loader"
+ "cmd/link/internal/sym"
+ "fmt"
+ "sort"
+)
+
+const (
+ UNW_FLAG_EHANDLER = 1 << 3
+ UNW_FLAG_UHANDLER = 2 << 3
+ UNW_FLAG_CHAININFO = 3 << 3
+ unwStaticDataSize = 8
+)
+
+// processSEH walks all pdata relocations looking for exception handler function symbols.
+// We want to mark these as reachable if the function that they protect is reachable
+// in the final binary.
+func processSEH(ldr *loader.Loader, arch *sys.Arch, pdata sym.LoaderSym, xdata sym.LoaderSym) error {
+ switch arch.Family {
+ case sys.AMD64:
+ ldr.SetAttrReachable(pdata, true)
+ if xdata != 0 {
+ ldr.SetAttrReachable(xdata, true)
+ }
+ return processSEHAMD64(ldr, pdata)
+ default:
+ // TODO: support SEH on other architectures.
+ return fmt.Errorf("unsupported architecture for SEH: %v", arch.Family)
+ }
+}
+
+func processSEHAMD64(ldr *loader.Loader, pdata sym.LoaderSym) error {
+ // The following loop traverses a list of pdata entries,
+ // each entry being 3 relocations long. The first relocation
+ // is a pointer to the function symbol to which the pdata entry
+ // corresponds. The third relocation is a pointer to the
+ // corresponding .xdata entry.
+ // Reference:
+ // https://learn.microsoft.com/en-us/cpp/build/exception-handling-x64#struct-runtime_function
+ rels := ldr.Relocs(pdata)
+ if rels.Count()%3 != 0 {
+ return fmt.Errorf(".pdata symbol %q has invalid relocation count", ldr.SymName(pdata))
+ }
+ for i := 0; i < rels.Count(); i += 3 {
+ xrel := rels.At(i + 2)
+ handler := findHandlerInXDataAMD64(ldr, xrel.Sym(), xrel.Add())
+ if handler != 0 {
+ sb := ldr.MakeSymbolUpdater(rels.At(i).Sym())
+ r, _ := sb.AddRel(objabi.R_KEEP)
+ r.SetSym(handler)
+ }
+ }
+ return nil
+}
+
+// findHandlerInXDataAMD64 finds the symbol in the .xdata section that
+// corresponds to the exception handler.
+// Reference:
+// https://learn.microsoft.com/en-us/cpp/build/exception-handling-x64#struct-unwind_info
+func findHandlerInXDataAMD64(ldr *loader.Loader, xsym sym.LoaderSym, add int64) loader.Sym {
+ data := ldr.Data(xsym)
+ if add < 0 || add+unwStaticDataSize > int64(len(data)) {
+ return 0
+ }
+ data = data[add:]
+ var isChained bool
+ switch flag := data[0]; {
+ case flag&UNW_FLAG_EHANDLER != 0 || flag&UNW_FLAG_UHANDLER != 0:
+ // Exception handler.
+ case flag&UNW_FLAG_CHAININFO != 0:
+ isChained = true
+ default:
+ // Nothing to do.
+ return 0
+ }
+ codes := data[3]
+ if codes%2 != 0 {
+ // There are always an even number of unwind codes, even if the last one is unused.
+ codes += 1
+ }
+ // The exception handler relocation is the first relocation after the unwind codes,
+ // unless it is chained, but we will handle this case later.
+ targetOff := add + unwStaticDataSize*(1+int64(codes))
+ xrels := ldr.Relocs(xsym)
+ idx := sort.Search(xrels.Count(), func(i int) bool {
+ return int64(xrels.At(i).Off()) >= targetOff
+ })
+ if idx == 0 {
+ return 0
+ }
+ if isChained {
+ // The third relocations references the next .xdata entry in the chain, recurse.
+ idx += 2
+ if idx >= xrels.Count() {
+ return 0
+ }
+ r := xrels.At(idx)
+ return findHandlerInXDataAMD64(ldr, r.Sym(), r.Add())
+ }
+ return xrels.At(idx).Sym()
+}
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Nov 21, 2023, 5:28:58 PM11/21/23
to Quim Muntal, goph...@pubsubhelper.golang.org, Bryan Mills, Gopher Robot, Go LUCI, Cherry Mui, Heschi Kreinick, Than McIntosh, golang-co...@googlegroups.com
Attention is currently required from: Quim Muntal.
1 comment:
File src/cmd/cgo/internal/test/callback_windows.go:
"runtime.asmcgocall_landingpad",
"runtime.asmcgocall",
"runtime.cgocall",
"test._Cfunc_backtrace",
"test.testCallbackCallersSEH.func1.1",
"test.testCallbackCallersSEH.func1",
"test.goCallback",
cgoexpPrefix + "goCallback",
"runtime.cgocallbackg1",
"runtime.cgocallbackg",
"runtime.cgocallbackg",
"runtime.cgocallback",
"crosscall2",
"runtime.asmcgocall_landingpad",
"runtime.asmcgocall",
"runtime.cgocall",
"test._Cfunc_callback",
"test.nestedCall.func1",
"test.nestedCall",
"test.testCallbackCallersSEH",
"test.TestCallbackCallersSEH",
"testing.tRunner",
"testing.(*T).Run.gowrap1",
"runtime.goexit"This test is way too fragile — it will break if anything in this call chain is refactored to have a different number of function calls (such as in CL 506755).
Can we instead check for specific frames at various points in the call chain, or use some other mechanism to get the ground truth for what the call chain ought to be?
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Nov 21, 2023, 5:57:13 PM11/21/23
to goph...@pubsubhelper.golang.org, Bryan Mills, Gopher Robot, Go LUCI, Cherry Mui, Heschi Kreinick, Than McIntosh, golang-co...@googlegroups.com
This test is way too fragile — it will break if anything in this call chain is refactored to have a […]
We could just check for frames from the tested package (`test`), which are the most relevant here. This will allow standard library packaged to change the call chain without breaking this test.
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Nov 29, 2023, 5:02:51 AM11/29/23
to goph...@pubsubhelper.golang.org, Bryan Mills, Gopher Robot, Go LUCI, Cherry Mui, Heschi Kreinick, Than McIntosh, golang-co...@googlegroups.com
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal (Gerrit)
Nov 29, 2023, 5:19:14 AM11/29/23
to goph...@pubsubhelper.golang.org, Bryan Mills, Gopher Robot, Go LUCI, Cherry Mui, Heschi Kreinick, Than McIntosh, golang-co...@googlegroups.com
We could just check for frames from the tested package (`test`), which are the most relevant here. […]
To view, visit change 534555. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages