cmd/link: emit MSVC-compatible object files
We need to ensure that the .pdata section we emit has RUNTIME_FUNCTION
entries ordered by function start address. Otherwise the MSVC linker is
going to fail with an error like this:
fatal error LNK1223: invalid or corrupt file: file contains invalid
.pdata contributions
diff --git a/src/cmd/link/internal/ld/main.go b/src/cmd/link/internal/ld/main.go
index 6a68489..bd3266e 100644
--- a/src/cmd/link/internal/ld/main.go
+++ b/src/cmd/link/internal/ld/main.go
@@ -443,6 +443,11 @@
bench.Start("layout")
filesize := ctxt.layout(order)
+ if ctxt.IsWindows() {
+ bench.Start("dopePost")
+ ctxt.dopePost()
+ }
+
// Write out the output file.
// It is split into two parts (Asmb and Asmb2). The first
// part writes most of the content (sections and segments),
diff --git a/src/cmd/link/internal/ld/pe.go b/src/cmd/link/internal/ld/pe.go
index fbfd928..6fd6556 100644
--- a/src/cmd/link/internal/ld/pe.go
+++ b/src/cmd/link/internal/ld/pe.go
@@ -1640,6 +1640,10 @@
writeSEH(ctxt)
}
+func (ctxt *Link) dopePost() {
+ commitSEH(ctxt)
+}
+
func setpersrc(ctxt *Link, syms []loader.Sym) {
if len(rsrcsyms) != 0 {
Errorf("too many .rsrc sections")
diff --git a/src/cmd/link/internal/ld/seh.go b/src/cmd/link/internal/ld/seh.go
index 9f0f747..96842e3 100644
--- a/src/cmd/link/internal/ld/seh.go
+++ b/src/cmd/link/internal/ld/seh.go
@@ -8,11 +8,18 @@
"cmd/internal/sys"
"cmd/link/internal/loader"
"cmd/link/internal/sym"
+ "sort"
)
var sehp struct {
- pdata []sym.LoaderSym
- xdata []sym.LoaderSym
+ pdata []sym.LoaderSym
+ pdataEntries [][]PdataEntry
+ xdata []sym.LoaderSym
+}
+
+type PdataEntry struct {
+ sym loader.Sym
+ xdataOffset int64
}
func writeSEH(ctxt *Link) {
@@ -22,6 +29,13 @@
}
}
+func commitSEH(ctxt *Link) {
+ switch ctxt.Arch.Family {
+ case sys.AMD64:
+ commitSEHAMD64(ctxt)
+ }
+}
+
func writeSEHAMD64(ctxt *Link) {
ldr := ctxt.loader
mkSecSym := func(name string, kind sym.SymKind) *loader.SymbolBuilder {
@@ -31,6 +45,7 @@
return s
}
pdata := mkSecSym(".pdata", sym.SSEHSECT)
+ pdataEntries := make([]PdataEntry, 0, len(ctxt.Textp))
xdata := mkSecSym(".xdata", sym.SSEHSECT)
// The .xdata entries have very low cardinality
// as it only contains frame pointer operations,
@@ -66,12 +81,39 @@
}
}
- // Reference:
- // https://learn.microsoft.com/en-us/cpp/build/exception-handling-x64#struct-runtime_function
- pdata.AddPEImageRelativeAddrPlus(ctxt.Arch, s, 0)
- pdata.AddPEImageRelativeAddrPlus(ctxt.Arch, s, ldr.SymSize(s))
- pdata.AddPEImageRelativeAddrPlus(ctxt.Arch, xdata.Sym(), off)
+ // The MSVC linker requires that entries are sorted by function start address,
+ // but at this point symbol addresses are not yet known. This means we cannot
+ // emit the .pdata just yet.
+ pdataEntries = append(pdataEntries, PdataEntry{
+ sym: s,
+ xdataOffset: off,
+ })
}
+ pdata.SetSize((int64)(len(pdataEntries) * 12))
sehp.pdata = append(sehp.pdata, pdata.Sym())
+ sehp.pdataEntries = append(sehp.pdataEntries, pdataEntries)
sehp.xdata = append(sehp.xdata, xdata.Sym())
}
+
+func commitSEHAMD64(ctxt *Link) {
+ ldr := ctxt.loader
+
+ for i, pdataSym := range sehp.pdata {
+ pdataEntries := sehp.pdataEntries[i]
+ sort.Slice(pdataEntries, func(i, j int) bool {
+ return ldr.SymAddr(pdataEntries[i].sym) < ldr.SymAddr(pdataEntries[j].sym)
+ })
+
+ pdata := ldr.MakeSymbolUpdater(pdataSym)
+ pdata.SetSize(0)
+ xdataSym := sehp.xdata[i]
+ for _, e := range pdataEntries {
+ s := e.sym
+ // Reference:
+ // https://learn.microsoft.com/en-us/cpp/build/exception-handling-x64#struct-runtime_function
+ pdata.AddPEImageRelativeAddrPlus(ctxt.Arch, s, 0)
+ pdata.AddPEImageRelativeAddrPlus(ctxt.Arch, s, ldr.SymSize(s))
+ pdata.AddPEImageRelativeAddrPlus(ctxt.Arch, xdataSym, e.xdataOffset)
+ }
+ }
+}
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Congratulations on opening your first change. Thank you for your contribution!
Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thanks for the CL. I'm not sure we support linking with the MSVC linker. #20982 is about it, but I don't think it is fully implemented, or we have decided to do so. Perhaps Quim can clarify. Thanks.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thanks for the CL. I'm not sure we support linking with the MSVC linker. #20982 is about it, but I don't think it is fully implemented, or we have decided to do so. Perhaps Quim can clarify. Thanks.
Hi Cherry,
Just to add some missing context for this CL:
We currently post-process `go.o` inside the archive to sort the `.pdata` section’s array of `RUNTIME_FUNCTION` structures by function start address, which MSVC expects. This CL moves that fix upstream so future Go releases emit COFF objects MSVC already accepts.
Hope that clarifies the intent—thanks for taking a look!
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ole André Vadla RavnåsThanks for the CL. I'm not sure we support linking with the MSVC linker. #20982 is about it, but I don't think it is fully implemented, or we have decided to do so. Perhaps Quim can clarify. Thanks.
Hi Cherry,
Just to add some missing context for this CL:
- Goal: `-buildmode=c-archive` should emit an `.a` archive whose objects can be linked by MSVC. The only manual step is calling `_rt0_amd64_windows_lib()` explicitly, because MSVC ignores GCC-style constructor metadata.
- Status quo: Building the Go code still needs MinGW; the CL simply lets MSVC consume the produced `.a` directly, so we don’t need to wrap the Go code in a DLL or EXE.
- Why it matters: In Frida (https://frida.re) we support multiple toolchains, including MSVC. Since we recently embedded Go and aim to ship as a single self-contained binary, this tweak prevents a regression.
We currently post-process `go.o` inside the archive to sort the `.pdata` section’s array of `RUNTIME_FUNCTION` structures by function start address, which MSVC expects. This CL moves that fix upstream so future Go releases emit COFF objects MSVC already accepts.
Hope that clarifies the intent—thanks for taking a look!
Hi Ole, Thanks for the information. It would probably be helpful to discuss the goal and the steps on an issue, probably #20982.
Besides the code changes, if we want this be a supported feature, we'd also need mechanisms for testing. Is it easy to set up such tests?
Also, do you plan to support it on all Windows platforms that Go supports, i.e. AMD64, ARM64, and 32-bit x86?
Let's continue the discussion on an issue. Then we can go from there. Thanks!
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ole André Vadla RavnåsThanks for the CL. I'm not sure we support linking with the MSVC linker. #20982 is about it, but I don't think it is fully implemented, or we have decided to do so. Perhaps Quim can clarify. Thanks.
Cherry MuiHi Cherry,
Just to add some missing context for this CL:
- Goal: `-buildmode=c-archive` should emit an `.a` archive whose objects can be linked by MSVC. The only manual step is calling `_rt0_amd64_windows_lib()` explicitly, because MSVC ignores GCC-style constructor metadata.
- Status quo: Building the Go code still needs MinGW; the CL simply lets MSVC consume the produced `.a` directly, so we don’t need to wrap the Go code in a DLL or EXE.
- Why it matters: In Frida (https://frida.re) we support multiple toolchains, including MSVC. Since we recently embedded Go and aim to ship as a single self-contained binary, this tweak prevents a regression.
We currently post-process `go.o` inside the archive to sort the `.pdata` section’s array of `RUNTIME_FUNCTION` structures by function start address, which MSVC expects. This CL moves that fix upstream so future Go releases emit COFF objects MSVC already accepts.
Hope that clarifies the intent—thanks for taking a look!
Hi Ole, Thanks for the information. It would probably be helpful to discuss the goal and the steps on an issue, probably #20982.
Besides the code changes, if we want this be a supported feature, we'd also need mechanisms for testing. Is it easy to set up such tests?
Also, do you plan to support it on all Windows platforms that Go supports, i.e. AMD64, ARM64, and 32-bit x86?
Let's continue the discussion on an issue. Then we can go from there. Thanks!
This is great Ole! Agree that MSVC support should be tackled holistically in #20982. On the other hand, this CL also fixes #1093, so we do have a test that can be unskipped if this lands (see CL 556635).
Ole, can you change the CL title and description so that it is focused on the fact that it is fixing the .pdata section layout, rather than being MSVC-centric?
Also, as part of this same CL you can undo what I did in CL 556635
type PdataEntry struct {PdataEntry doesn't need to be exported.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thanks Cherry and Quim,
I've updated the CL title/description to focus on the .pdata ordering
fix and re-enabled the test skipped in CL 556635. I can also confirm
that things already work on x86 and arm64; no changes needed there.
Happy to continue the broader MSVC discussion in #20982—let me know if
anything else is needed.
PdataEntry doesn't need to be exported.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ole André Vadla RavnåsThanks for the CL. I'm not sure we support linking with the MSVC linker. #20982 is about it, but I don't think it is fully implemented, or we have decided to do so. Perhaps Quim can clarify. Thanks.
Cherry MuiHi Cherry,
Just to add some missing context for this CL:
- Goal: `-buildmode=c-archive` should emit an `.a` archive whose objects can be linked by MSVC. The only manual step is calling `_rt0_amd64_windows_lib()` explicitly, because MSVC ignores GCC-style constructor metadata.
- Status quo: Building the Go code still needs MinGW; the CL simply lets MSVC consume the produced `.a` directly, so we don’t need to wrap the Go code in a DLL or EXE.
- Why it matters: In Frida (https://frida.re) we support multiple toolchains, including MSVC. Since we recently embedded Go and aim to ship as a single self-contained binary, this tweak prevents a regression.
We currently post-process `go.o` inside the archive to sort the `.pdata` section’s array of `RUNTIME_FUNCTION` structures by function start address, which MSVC expects. This CL moves that fix upstream so future Go releases emit COFF objects MSVC already accepts.
Hope that clarifies the intent—thanks for taking a look!
Quim MuntalHi Ole, Thanks for the information. It would probably be helpful to discuss the goal and the steps on an issue, probably #20982.
Besides the code changes, if we want this be a supported feature, we'd also need mechanisms for testing. Is it easy to set up such tests?
Also, do you plan to support it on all Windows platforms that Go supports, i.e. AMD64, ARM64, and 32-bit x86?
Let's continue the discussion on an issue. Then we can go from there. Thanks!
This is great Ole! Agree that MSVC support should be tackled holistically in #20982. On the other hand, this CL also fixes #1093, so we do have a test that can be unskipped if this lands (see CL 556635).
Ole, can you change the CL title and description so that it is focused on the fact that it is fixing the .pdata section layout, rather than being MSVC-centric?
Also, as part of this same CL you can undo what I did in CL 556635
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
This also reverts commit adead1a93f472affa97c494ef19f2f492ee6f34a, as```suggestion
This also reverts commit CL 556635, as
```
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thanks Quim!
This also reverts commit adead1a93f472affa97c494ef19f2f492ee6f34a, as```suggestion
This also reverts commit CL 556635, as
```
Done
Done
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +0 |
pdataEntries := sehp.pdataEntries[i]Some sehp.pdata items may come from ld.ldobj (aka C host objects), which don't have `pdataEntries` entries. They may need to be reordered too. Regardless, that can be tackled in a follow up PR.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
pdataEntries := sehp.pdataEntries[i]Some sehp.pdata items may come from ld.ldobj (aka C host objects), which don't have `pdataEntries` entries. They may need to be reordered too. Regardless, that can be tackled in a follow up PR.
But `sehp.pdataEntries[i]` is causing test to fail, as some `sehp.pdata` do comes from a C host object. You should at least add an empty `sehp.pdataEntries` in `ld.ldobj` to keep this loop happy.
if ctxt.IsWindows() {
bench.Start("dopePost")
ctxt.dopePost()
}Probably no need to introduce a top-level path. Can it be part of, say, asmbPe?
sort.Slice(pdataEntries, func(i, j int) bool {
return ldr.SymAddr(pdataEntries[i].sym) < ldr.SymAddr(pdataEntries[j].sym)
})Consider using `slices.SortFunc`, which is the new idiom.
// Reference:
// https://learn.microsoft.com/en-us/cpp/build/exception-handling-x64#struct-runtime_function
pdata.AddPEImageRelativeAddrPlus(ctxt.Arch, s, 0)
pdata.AddPEImageRelativeAddrPlus(ctxt.Arch, s, ldr.SymSize(s))
pdata.AddPEImageRelativeAddrPlus(ctxt.Arch, xdataSym, e.xdataOffset)
}While here, it may make it easier to read if we also include inlined comments, "start address", "end address", "xdata".
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ole André Vadla RavnåsThanks for the CL. I'm not sure we support linking with the MSVC linker. #20982 is about it, but I don't think it is fully implemented, or we have decided to do so. Perhaps Quim can clarify. Thanks.
Cherry MuiHi Cherry,
Just to add some missing context for this CL:
- Goal: `-buildmode=c-archive` should emit an `.a` archive whose objects can be linked by MSVC. The only manual step is calling `_rt0_amd64_windows_lib()` explicitly, because MSVC ignores GCC-style constructor metadata.
- Status quo: Building the Go code still needs MinGW; the CL simply lets MSVC consume the produced `.a` directly, so we don’t need to wrap the Go code in a DLL or EXE.
- Why it matters: In Frida (https://frida.re) we support multiple toolchains, including MSVC. Since we recently embedded Go and aim to ship as a single self-contained binary, this tweak prevents a regression.
We currently post-process `go.o` inside the archive to sort the `.pdata` section’s array of `RUNTIME_FUNCTION` structures by function start address, which MSVC expects. This CL moves that fix upstream so future Go releases emit COFF objects MSVC already accepts.
Hope that clarifies the intent—thanks for taking a look!
Quim MuntalHi Ole, Thanks for the information. It would probably be helpful to discuss the goal and the steps on an issue, probably #20982.
Besides the code changes, if we want this be a supported feature, we'd also need mechanisms for testing. Is it easy to set up such tests?
Also, do you plan to support it on all Windows platforms that Go supports, i.e. AMD64, ARM64, and 32-bit x86?
Let's continue the discussion on an issue. Then we can go from there. Thanks!
This is great Ole! Agree that MSVC support should be tackled holistically in #20982. On the other hand, this CL also fixes #1093, so we do have a test that can be unskipped if this lands (see CL 556635).
Ole, can you change the CL title and description so that it is focused on the fact that it is fixing the .pdata section layout, rather than being MSVC-centric?
Also, as part of this same CL you can undo what I did in CL 556635
Than McIntoshDone
>Besides the code changes, if we want this be a supported feature, we'd also need >mechanisms for testing. Is it easy to set up such tests?
I am also interested in the answer to Cherry's question. What is our regression testing story? If someone accidentally makes a change that breaks the Go + MSVC linker path, how will such problems get detected? Thanks.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ole I'm interested that this CL lands sooner than latter. Did you have a change to see the reviews?
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Friendly ping.
Friendly ping.
>Friendly ping
A reminder about the previous thread relating to testing strategy. Have there been any thoughts on this? Thanks.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
This CL reenable some tests which are failing with Mingw-w64, so the CL has value regardless of the MSVC support.
You are risisng a good point, though. The Windows builders do have MSVC installed, so we could add some tests that load a Go dll from C code compiled with MSVC.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |