[go] cmd/link: emit MSVC-compatible object files

49 views
Skip to first unread message

Ole André Vadla Ravnås (Gerrit)

unread,
Jun 4, 2025, 10:58:46 AM6/4/25
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Ole André Vadla Ravnås has uploaded the change for review

Commit message

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
Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634

Change diff

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)
+ }
+ }
+}

Change information

Files:
  • M src/cmd/link/internal/ld/main.go
  • M src/cmd/link/internal/ld/pe.go
  • M src/cmd/link/internal/ld/seh.go
Change size: M
Delta: 3 files changed, 58 insertions(+), 7 deletions(-)
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newchange
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
Gerrit-Change-Number: 678795
Gerrit-PatchSet: 1
Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
unsatisfied_requirement
satisfied_requirement
open
diffy

Gopher Robot (Gerrit)

unread,
Jun 4, 2025, 11:00:38 AM6/4/25
to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Message from Gopher Robot

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.

Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
Gerrit-Change-Number: 678795
Gerrit-PatchSet: 1
Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-Comment-Date: Wed, 04 Jun 2025 15:00:30 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
unsatisfied_requirement
satisfied_requirement
open
diffy

Cherry Mui (Gerrit)

unread,
Jun 4, 2025, 11:14:40 AM6/4/25
to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, Quim Muntal, Ian Lance Taylor, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Ian Lance Taylor, Ole André Vadla Ravnås, Quim Muntal and Russ Cox

Cherry Mui added 1 comment

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Cherry Mui . unresolved

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.

Open in Gerrit

Related details

Attention is currently required from:
  • Ian Lance Taylor
  • Ole André Vadla Ravnås
  • Quim Muntal
  • Russ Cox
Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
    Gerrit-Change-Number: 678795
    Gerrit-PatchSet: 1
    Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
    Gerrit-Reviewer: Cherry Mui <cher...@google.com>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
    Gerrit-Reviewer: Russ Cox <r...@golang.org>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Attention: Ole André Vadla Ravnås <ole...@gmail.com>
    Gerrit-Attention: Russ Cox <r...@golang.org>
    Gerrit-Attention: Quim Muntal <quimm...@gmail.com>
    Gerrit-Comment-Date: Wed, 04 Jun 2025 15:14:32 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    unsatisfied_requirement
    open
    diffy

    Ole André Vadla Ravnås (Gerrit)

    unread,
    Jun 4, 2025, 12:33:25 PM6/4/25
    to goph...@pubsubhelper.golang.org, Quim Muntal, Cherry Mui, Ian Lance Taylor, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Cherry Mui, Ian Lance Taylor, Quim Muntal and Russ Cox

    Ole André Vadla Ravnås added 1 comment

    Patchset-level comments
    Cherry Mui . unresolved

    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.

    Ole André Vadla Ravnås

    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!

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Cherry Mui
    • Ian Lance Taylor
    • Quim Muntal
    • Russ Cox
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
    Gerrit-Change-Number: 678795
    Gerrit-PatchSet: 1
    Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
    Gerrit-Reviewer: Cherry Mui <cher...@google.com>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
    Gerrit-Reviewer: Russ Cox <r...@golang.org>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Attention: Russ Cox <r...@golang.org>
    Gerrit-Attention: Quim Muntal <quimm...@gmail.com>
    Gerrit-Attention: Cherry Mui <cher...@google.com>
    Gerrit-Comment-Date: Wed, 04 Jun 2025 16:33:17 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Cherry Mui <cher...@google.com>
    unsatisfied_requirement
    open
    diffy

    Cherry Mui (Gerrit)

    unread,
    Jun 4, 2025, 12:53:00 PM6/4/25
    to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, Quim Muntal, Ian Lance Taylor, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Ian Lance Taylor, Ole André Vadla Ravnås, Quim Muntal and Russ Cox

    Cherry Mui added 1 comment

    Patchset-level comments
    Cherry Mui . unresolved

    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.

    Ole André Vadla Ravnås

    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!

    Cherry Mui

    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!

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Ian Lance Taylor
    • Ole André Vadla Ravnås
    • Quim Muntal
    • Russ Cox
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
    Gerrit-Change-Number: 678795
    Gerrit-PatchSet: 1
    Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
    Gerrit-Reviewer: Cherry Mui <cher...@google.com>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
    Gerrit-Reviewer: Russ Cox <r...@golang.org>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Attention: Ole André Vadla Ravnås <ole...@gmail.com>
    Gerrit-Attention: Russ Cox <r...@golang.org>
    Gerrit-Attention: Quim Muntal <quimm...@gmail.com>
    Gerrit-Comment-Date: Wed, 04 Jun 2025 16:52:54 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Ole André Vadla Ravnås <ole...@gmail.com>
    Comment-In-Reply-To: Cherry Mui <cher...@google.com>
    unsatisfied_requirement
    open
    diffy

    Quim Muntal (Gerrit)

    unread,
    Jun 5, 2025, 4:36:34 AM6/5/25
    to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, Cherry Mui, Ian Lance Taylor, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Ian Lance Taylor, Ole André Vadla Ravnås and Russ Cox

    Quim Muntal added 2 comments

    Patchset-level comments
    Cherry Mui . unresolved

    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.

    Ole André Vadla Ravnås

    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!

    Cherry Mui

    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!

    Quim Muntal

    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

    File src/cmd/link/internal/ld/seh.go
    Line 20, Patchset 1 (Latest):type PdataEntry struct {
    Quim Muntal . unresolved

    PdataEntry doesn't need to be exported.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Ian Lance Taylor
    • Ole André Vadla Ravnås
    • Russ Cox
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
    Gerrit-Change-Number: 678795
    Gerrit-PatchSet: 1
    Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
    Gerrit-Reviewer: Cherry Mui <cher...@google.com>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
    Gerrit-Reviewer: Russ Cox <r...@golang.org>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Attention: Ole André Vadla Ravnås <ole...@gmail.com>
    Gerrit-Attention: Russ Cox <r...@golang.org>
    Gerrit-Comment-Date: Thu, 05 Jun 2025 08:36:26 +0000
    unsatisfied_requirement
    open
    diffy

    Ole André Vadla Ravnås (Gerrit)

    unread,
    Jun 5, 2025, 4:33:09 PM6/5/25
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
    Attention needed from Ian Lance Taylor, Ole André Vadla Ravnås and Russ Cox

    Ole André Vadla Ravnås uploaded new patchset

    Ole André Vadla Ravnås uploaded patch set #2 to this change.
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Ian Lance Taylor
    • Ole André Vadla Ravnås
    • Russ Cox
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: newpatchset
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
    Gerrit-Change-Number: 678795
    Gerrit-PatchSet: 2
    unsatisfied_requirement
    open
    diffy

    Ole André Vadla Ravnås (Gerrit)

    unread,
    Jun 5, 2025, 4:36:54 PM6/5/25
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
    Attention needed from Ian Lance Taylor, Ole André Vadla Ravnås and Russ Cox

    Ole André Vadla Ravnås uploaded new patchset

    Ole André Vadla Ravnås uploaded patch set #3 to this change.
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Ian Lance Taylor
    • Ole André Vadla Ravnås
    • Russ Cox
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: newpatchset
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
    Gerrit-Change-Number: 678795
    Gerrit-PatchSet: 3
    unsatisfied_requirement
    open
    diffy

    Ole André Vadla Ravnås (Gerrit)

    unread,
    Jun 5, 2025, 4:48:34 PM6/5/25
    to goph...@pubsubhelper.golang.org, Quim Muntal, Cherry Mui, Ian Lance Taylor, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Ian Lance Taylor, Quim Muntal and Russ Cox

    Ole André Vadla Ravnås added 2 comments

    Patchset-level comments
    File-level comment, Patchset 3 (Latest):
    Ole André Vadla Ravnås . resolved

    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.

    File src/cmd/link/internal/ld/seh.go
    Line 20, Patchset 1:type PdataEntry struct {
    Quim Muntal . resolved

    PdataEntry doesn't need to be exported.

    Ole André Vadla Ravnås

    Done

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Ian Lance Taylor
    • Quim Muntal
    • Russ Cox
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
    Gerrit-Change-Number: 678795
    Gerrit-PatchSet: 3
    Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
    Gerrit-Reviewer: Cherry Mui <cher...@google.com>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
    Gerrit-Reviewer: Russ Cox <r...@golang.org>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Attention: Russ Cox <r...@golang.org>
    Gerrit-Attention: Quim Muntal <quimm...@gmail.com>
    Gerrit-Comment-Date: Thu, 05 Jun 2025 20:48:27 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Quim Muntal <quimm...@gmail.com>
    unsatisfied_requirement
    open
    diffy

    Ole André Vadla Ravnås (Gerrit)

    unread,
    Jun 5, 2025, 4:50:07 PM6/5/25
    to goph...@pubsubhelper.golang.org, Quim Muntal, Cherry Mui, Ian Lance Taylor, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Cherry Mui, Ian Lance Taylor, Quim Muntal and Russ Cox

    Ole André Vadla Ravnås added 1 comment

    Patchset-level comments
    File-level comment, Patchset 1:
    Cherry Mui . resolved

    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.

    Ole André Vadla Ravnås

    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!

    Cherry Mui

    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!

    Quim Muntal

    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

    Ole André Vadla Ravnås

    Done

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Cherry Mui
    • Ian Lance Taylor
    • Quim Muntal
    • Russ Cox
      Submit Requirements:
        • requirement is not satisfiedCode-Review
        • requirement satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        • requirement is not satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
        Gerrit-Change-Number: 678795
        Gerrit-PatchSet: 3
        Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
        Gerrit-Reviewer: Cherry Mui <cher...@google.com>
        Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
        Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
        Gerrit-Reviewer: Russ Cox <r...@golang.org>
        Gerrit-CC: Gopher Robot <go...@golang.org>
        Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
        Gerrit-Attention: Russ Cox <r...@golang.org>
        Gerrit-Attention: Quim Muntal <quimm...@gmail.com>
        Gerrit-Attention: Cherry Mui <cher...@google.com>
        Gerrit-Comment-Date: Thu, 05 Jun 2025 20:50:00 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Comment-In-Reply-To: Ole André Vadla Ravnås <ole...@gmail.com>
        Comment-In-Reply-To: Quim Muntal <quimm...@gmail.com>
        Comment-In-Reply-To: Cherry Mui <cher...@google.com>
        unsatisfied_requirement
        satisfied_requirement
        open
        diffy

        Quim Muntal (Gerrit)

        unread,
        Jun 5, 2025, 4:53:43 PM6/5/25
        to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, Cherry Mui, Ian Lance Taylor, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
        Attention needed from Cherry Mui, Ian Lance Taylor, Ole André Vadla Ravnås and Russ Cox

        Quim Muntal added 2 comments

        Commit Message
        Line 23, Patchset 3 (Latest):This also reverts commit adead1a93f472affa97c494ef19f2f492ee6f34a, as
        Quim Muntal . unresolved

        ```suggestion
        This also reverts commit CL 556635, as
        ```

        Line 25, Patchset 3 (Latest):
        Quim Muntal . unresolved

        Add: Fixes #65116.

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Cherry Mui
        • Ian Lance Taylor
        • Ole André Vadla Ravnås
        • Russ Cox
        Submit Requirements:
          • requirement is not satisfiedCode-Review
          • requirement is not satisfiedNo-Unresolved-Comments
          • requirement is not satisfiedReview-Enforcement
          • requirement is not satisfiedTryBots-Pass
          Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
          Gerrit-MessageType: comment
          Gerrit-Project: go
          Gerrit-Branch: master
          Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
          Gerrit-Change-Number: 678795
          Gerrit-PatchSet: 3
          Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
          Gerrit-Reviewer: Cherry Mui <cher...@google.com>
          Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
          Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
          Gerrit-Reviewer: Russ Cox <r...@golang.org>
          Gerrit-CC: Gopher Robot <go...@golang.org>
          Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
          Gerrit-Attention: Ole André Vadla Ravnås <ole...@gmail.com>
          Gerrit-Attention: Russ Cox <r...@golang.org>
          Gerrit-Attention: Cherry Mui <cher...@google.com>
          Gerrit-Comment-Date: Thu, 05 Jun 2025 20:53:36 +0000
          Gerrit-HasComments: Yes
          Gerrit-Has-Labels: No
          unsatisfied_requirement
          open
          diffy

          Ole André Vadla Ravnås (Gerrit)

          unread,
          Jun 5, 2025, 5:44:24 PM6/5/25
          to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
          Attention needed from Cherry Mui, Ian Lance Taylor, Ole André Vadla Ravnås and Russ Cox

          Ole André Vadla Ravnås uploaded new patchset

          Ole André Vadla Ravnås uploaded patch set #4 to this change.
          Open in Gerrit

          Related details

          Attention is currently required from:
          • Cherry Mui
          • Ian Lance Taylor
          • Ole André Vadla Ravnås
          • Russ Cox
          Submit Requirements:
          • requirement is not satisfiedCode-Review
          • requirement is not satisfiedNo-Unresolved-Comments
          • requirement is not satisfiedReview-Enforcement
          • requirement is not satisfiedTryBots-Pass
          Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
          Gerrit-MessageType: newpatchset
          Gerrit-Project: go
          Gerrit-Branch: master
          Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
          Gerrit-Change-Number: 678795
          Gerrit-PatchSet: 4
          unsatisfied_requirement
          open
          diffy

          Ole André Vadla Ravnås (Gerrit)

          unread,
          Jun 5, 2025, 5:45:25 PM6/5/25
          to goph...@pubsubhelper.golang.org, Quim Muntal, Cherry Mui, Ian Lance Taylor, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
          Attention needed from Cherry Mui, Ian Lance Taylor, Quim Muntal and Russ Cox

          Ole André Vadla Ravnås added 3 comments

          Ole André Vadla Ravnås . resolved

          Thanks Quim!

          Commit Message
          Line 23, Patchset 3:This also reverts commit adead1a93f472affa97c494ef19f2f492ee6f34a, as
          Quim Muntal . resolved

          ```suggestion
          This also reverts commit CL 556635, as
          ```

          Ole André Vadla Ravnås

          Done

          Line 25, Patchset 3:
          Quim Muntal . resolved

          Add: Fixes #65116.

          Ole André Vadla Ravnås

          Done

          Open in Gerrit

          Related details

          Attention is currently required from:
          • Cherry Mui
          • Ian Lance Taylor
          • Quim Muntal
          • Russ Cox
          Submit Requirements:
            • requirement is not satisfiedCode-Review
            • requirement satisfiedNo-Unresolved-Comments
            • requirement is not satisfiedReview-Enforcement
            • requirement is not satisfiedTryBots-Pass
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
            Gerrit-Change-Number: 678795
            Gerrit-PatchSet: 4
            Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Reviewer: Cherry Mui <cher...@google.com>
            Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
            Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
            Gerrit-Reviewer: Russ Cox <r...@golang.org>
            Gerrit-CC: Gopher Robot <go...@golang.org>
            Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
            Gerrit-Attention: Russ Cox <r...@golang.org>
            Gerrit-Attention: Quim Muntal <quimm...@gmail.com>
            Gerrit-Attention: Cherry Mui <cher...@google.com>
            Gerrit-Comment-Date: Thu, 05 Jun 2025 21:45:18 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            Comment-In-Reply-To: Quim Muntal <quimm...@gmail.com>
            unsatisfied_requirement
            satisfied_requirement
            open
            diffy

            Quim Muntal (Gerrit)

            unread,
            Jun 6, 2025, 5:07:32 AM6/6/25
            to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, Cherry Mui, Gopher Robot, golang-co...@googlegroups.com
            Attention needed from Cherry Mui and Ole André Vadla Ravnås

            Quim Muntal voted

            Code-Review+2
            Commit-Queue+1
            Open in Gerrit

            Related details

            Attention is currently required from:
            • Cherry Mui
            • Ole André Vadla Ravnås
            Submit Requirements:
            • requirement satisfiedCode-Review
            • requirement satisfiedNo-Unresolved-Comments
            • requirement is not satisfiedReview-Enforcement
            • requirement is not satisfiedTryBots-Pass
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
            Gerrit-Change-Number: 678795
            Gerrit-PatchSet: 4
            Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Reviewer: Cherry Mui <cher...@google.com>
            Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
            Gerrit-CC: Gopher Robot <go...@golang.org>
            Gerrit-Attention: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Attention: Cherry Mui <cher...@google.com>
            Gerrit-Comment-Date: Fri, 06 Jun 2025 09:07:24 +0000
            Gerrit-HasComments: No
            Gerrit-Has-Labels: Yes
            satisfied_requirement
            unsatisfied_requirement
            open
            diffy

            Quim Muntal (Gerrit)

            unread,
            Jun 6, 2025, 5:41:50 AM6/6/25
            to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, Go LUCI, Cherry Mui, Gopher Robot, golang-co...@googlegroups.com
            Attention needed from Cherry Mui and Ole André Vadla Ravnås

            Quim Muntal voted and added 1 comment

            Votes added by Quim Muntal

            Code-Review+0

            1 comment

            File src/cmd/link/internal/ld/seh.go
            Line 102, Patchset 4 (Latest): pdataEntries := sehp.pdataEntries[i]
            Quim Muntal . unresolved

            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.

            Open in Gerrit

            Related details

            Attention is currently required from:
            • Cherry Mui
            • Ole André Vadla Ravnås
            Submit Requirements:
            • requirement is not satisfiedCode-Review
            • requirement is not satisfiedNo-Unresolved-Comments
            • requirement is not satisfiedReview-Enforcement
            • requirement is not satisfiedTryBots-Pass
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
            Gerrit-Change-Number: 678795
            Gerrit-PatchSet: 4
            Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Reviewer: Cherry Mui <cher...@google.com>
            Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
            Gerrit-CC: Gopher Robot <go...@golang.org>
            Gerrit-Attention: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Attention: Cherry Mui <cher...@google.com>
            Gerrit-Comment-Date: Fri, 06 Jun 2025 09:41:43 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: Yes
            unsatisfied_requirement
            open
            diffy

            Quim Muntal (Gerrit)

            unread,
            Jun 27, 2025, 4:23:43 AM6/27/25
            to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, Go LUCI, Cherry Mui, Gopher Robot, golang-co...@googlegroups.com
            Attention needed from Cherry Mui and Ole André Vadla Ravnås

            Quim Muntal added 1 comment

            File src/cmd/link/internal/ld/seh.go
            Line 102, Patchset 4 (Latest): pdataEntries := sehp.pdataEntries[i]
            Quim Muntal . unresolved

            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.

            Quim Muntal

            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.

            Gerrit-Comment-Date: Fri, 27 Jun 2025 08:23:34 +0000
            unsatisfied_requirement
            open
            diffy

            Cherry Mui (Gerrit)

            unread,
            Jun 30, 2025, 11:50:37 AM6/30/25
            to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, Go LUCI, Quim Muntal, Gopher Robot, golang-co...@googlegroups.com
            Attention needed from Ole André Vadla Ravnås

            Cherry Mui added 3 comments

            File src/cmd/link/internal/ld/main.go
            Line 446, Patchset 4 (Latest): if ctxt.IsWindows() {
            bench.Start("dopePost")
            ctxt.dopePost()
            }
            Cherry Mui . unresolved

            Probably no need to introduce a top-level path. Can it be part of, say, asmbPe?

            File src/cmd/link/internal/ld/seh.go
            Line 103, Patchset 4 (Latest): sort.Slice(pdataEntries, func(i, j int) bool {

            return ldr.SymAddr(pdataEntries[i].sym) < ldr.SymAddr(pdataEntries[j].sym)
            })
            Cherry Mui . unresolved

            Consider using `slices.SortFunc`, which is the new idiom.

            Line 112, Patchset 4 (Latest): // 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)
            }
            Cherry Mui . unresolved

            While here, it may make it easier to read if we also include inlined comments, "start address", "end address", "xdata".

            Open in Gerrit

            Related details

            Attention is currently required from:
            • Ole André Vadla Ravnås
            Submit Requirements:
            • requirement is not satisfiedCode-Review
            • requirement is not satisfiedNo-Unresolved-Comments
            • requirement is not satisfiedReview-Enforcement
            • requirement is not satisfiedTryBots-Pass
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
            Gerrit-Change-Number: 678795
            Gerrit-PatchSet: 4
            Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Reviewer: Cherry Mui <cher...@google.com>
            Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
            Gerrit-CC: Gopher Robot <go...@golang.org>
            Gerrit-Attention: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Comment-Date: Mon, 30 Jun 2025 15:50:33 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            unsatisfied_requirement
            open
            diffy

            Than McIntosh (Gerrit)

            unread,
            Jul 7, 2025, 10:07:51 AM7/7/25
            to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, Than McIntosh, Go LUCI, Quim Muntal, Cherry Mui, Gopher Robot, golang-co...@googlegroups.com
            Attention needed from Cherry Mui, Ole André Vadla Ravnås and Quim Muntal

            Than McIntosh added 1 comment

            Patchset-level comments
            Cherry Mui . resolved

            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.

            Ole André Vadla Ravnås

            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!

            Cherry Mui

            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!

            Quim Muntal

            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

            Ole André Vadla Ravnås

            Done

            Than McIntosh

            >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.

            Open in Gerrit

            Related details

            Attention is currently required from:
            • Cherry Mui
            • Ole André Vadla Ravnås
            • Quim Muntal
            Submit Requirements:
            • requirement is not satisfiedCode-Review
            • requirement is not satisfiedNo-Unresolved-Comments
            • requirement is not satisfiedReview-Enforcement
            • requirement is not satisfiedTryBots-Pass
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
            Gerrit-Change-Number: 678795
            Gerrit-PatchSet: 4
            Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Reviewer: Cherry Mui <cher...@google.com>
            Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
            Gerrit-CC: Gopher Robot <go...@golang.org>
            Gerrit-CC: Than McIntosh <th...@golang.org>
            Gerrit-Attention: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Attention: Quim Muntal <quimm...@gmail.com>
            Gerrit-Attention: Cherry Mui <cher...@google.com>
            Gerrit-Comment-Date: Mon, 07 Jul 2025 14:07:47 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            unsatisfied_requirement
            open
            diffy

            Quim Muntal (Gerrit)

            unread,
            Jul 28, 2025, 8:35:56 AM7/28/25
            to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, Than McIntosh, Go LUCI, Cherry Mui, Gopher Robot, golang-co...@googlegroups.com
            Attention needed from Cherry Mui and Ole André Vadla Ravnås

            Quim Muntal added 1 comment

            Patchset-level comments
            File-level comment, Patchset 4 (Latest):
            Quim Muntal . resolved

            Ole I'm interested that this CL lands sooner than latter. Did you have a change to see the reviews?

            Open in Gerrit

            Related details

            Attention is currently required from:
            • Cherry Mui
            • Ole André Vadla Ravnås
            Submit Requirements:
            • requirement is not satisfiedCode-Review
            • requirement is not satisfiedNo-Unresolved-Comments
            • requirement is not satisfiedReview-Enforcement
            • requirement is not satisfiedTryBots-Pass
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
            Gerrit-Change-Number: 678795
            Gerrit-PatchSet: 4
            Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Reviewer: Cherry Mui <cher...@google.com>
            Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
            Gerrit-CC: Gopher Robot <go...@golang.org>
            Gerrit-CC: Than McIntosh <th...@golang.org>
            Gerrit-Attention: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Attention: Cherry Mui <cher...@google.com>
            Gerrit-Comment-Date: Mon, 28 Jul 2025 12:33:50 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            unsatisfied_requirement
            open
            diffy

            Quim Muntal (Gerrit)

            unread,
            Aug 18, 2025, 4:43:33 AM8/18/25
            to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, Than McIntosh, Go LUCI, Cherry Mui, Gopher Robot, golang-co...@googlegroups.com
            Attention needed from Cherry Mui and Ole André Vadla Ravnås

            Quim Muntal added 1 comment

            Patchset-level comments
            Quim Muntal . resolved

            Friendly ping.

            Gerrit-Comment-Date: Mon, 18 Aug 2025 08:43:25 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            unsatisfied_requirement
            open
            diffy

            Than McIntosh (Gerrit)

            unread,
            Aug 18, 2025, 4:08:40 PM8/18/25
            to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, Than McIntosh, Go LUCI, Quim Muntal, Cherry Mui, Gopher Robot, golang-co...@googlegroups.com
            Attention needed from Cherry Mui, Ole André Vadla Ravnås and Quim Muntal

            Than McIntosh added 1 comment

            Patchset-level comments
            Quim Muntal . resolved

            Friendly ping.

            Than McIntosh

            >Friendly ping

            A reminder about the previous thread relating to testing strategy. Have there been any thoughts on this? Thanks.

            Open in Gerrit

            Related details

            Attention is currently required from:
            • Cherry Mui
            • Ole André Vadla Ravnås
            • Quim Muntal
            Submit Requirements:
            • requirement is not satisfiedCode-Review
            • requirement is not satisfiedNo-Unresolved-Comments
            • requirement is not satisfiedReview-Enforcement
            • requirement is not satisfiedTryBots-Pass
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
            Gerrit-Change-Number: 678795
            Gerrit-PatchSet: 4
            Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Reviewer: Cherry Mui <cher...@google.com>
            Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
            Gerrit-CC: Gopher Robot <go...@golang.org>
            Gerrit-CC: Than McIntosh <th...@golang.org>
            Gerrit-Attention: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Attention: Quim Muntal <quimm...@gmail.com>
            Gerrit-Attention: Cherry Mui <cher...@google.com>
            Gerrit-Comment-Date: Mon, 18 Aug 2025 20:08:36 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            Comment-In-Reply-To: Quim Muntal <quimm...@gmail.com>
            unsatisfied_requirement
            open
            diffy

            Quim Muntal (Gerrit)

            unread,
            Aug 18, 2025, 4:17:25 PM8/18/25
            to Ole André Vadla Ravnås, goph...@pubsubhelper.golang.org, Than McIntosh, Go LUCI, Cherry Mui, Gopher Robot, golang-co...@googlegroups.com
            Attention needed from Cherry Mui, Ole André Vadla Ravnås and Than McIntosh

            Quim Muntal added 1 comment

            Patchset-level comments
            Quim Muntal

            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.

            Open in Gerrit

            Related details

            Attention is currently required from:
            • Cherry Mui
            • Ole André Vadla Ravnås
            • Than McIntosh
            Submit Requirements:
            • requirement is not satisfiedCode-Review
            • requirement is not satisfiedNo-Unresolved-Comments
            • requirement is not satisfiedReview-Enforcement
            • requirement is not satisfiedTryBots-Pass
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: Ife8841105522b64f35c10947cbfa4631b7401634
            Gerrit-Change-Number: 678795
            Gerrit-PatchSet: 4
            Gerrit-Owner: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Reviewer: Cherry Mui <cher...@google.com>
            Gerrit-Reviewer: Quim Muntal <quimm...@gmail.com>
            Gerrit-CC: Gopher Robot <go...@golang.org>
            Gerrit-CC: Than McIntosh <th...@golang.org>
            Gerrit-Attention: Than McIntosh <th...@golang.org>
            Gerrit-Attention: Ole André Vadla Ravnås <ole...@gmail.com>
            Gerrit-Attention: Cherry Mui <cher...@google.com>
            Gerrit-Comment-Date: Mon, 18 Aug 2025 20:17:17 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            Comment-In-Reply-To: Than McIntosh <th...@golang.org>
            unsatisfied_requirement
            open
            diffy
            Reply all
            Reply to author
            Forward
            0 new messages