Skip to content

Accept new #cgo directives introduced by go 1.24 #23439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
skirino wants to merge 2 commits into
base: main
Choose a base branch
from
+90 −1
Open

Accept new #cgo directives introduced by go 1.24 #23439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b09fe9
Accept new `#cgo` directives introduced by go 1.24
skirino Jun 24, 2026
24f5353
Port cgo test into pytest; confirm `nocallback` directive actually ta…
skirino Jun 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/python/pants/backend/go/go_sources/analyze_package/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@ func saveCgo(filename string, pkg *Package, cg *ast.CommentGroup, buildContext *
continue
}

// #cgo (nocallback|noescape) <function name>
if fields := strings.Fields(line); len(fields) == 3 && (fields[1] == "nocallback" || fields[1] == "noescape") {

@tdyas tdyas Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code ignores these #cgo comments instead of recording them in the comment before import "C" like other #cgo comments. That seems wrong since ignoring them here will result in go not seeing the directives at a later compilation stage.

Also, the call to strings.Fields duplicates the call on line 161 to strings.Fields.

@skirino skirino Jun 26, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tdyas Thank you for reviewing this PR!

IIUC nocallback and noescape directives are somewhat different in the sense that they affect the contents of cgo-generated source files, whereas other #cgo directives affect how the generated sources are handled. There's nothing we need to explicitly tell pants about the directive; the directive in .go should suffice.

To back up the idea: the following code (intentionally violate "nocallback" by calling back into Go) results in a panic when pants run. Without the #cgo nocallback line it runs successfully. (I'm trying to turn this into a python test...)

package main

// #include <stdio.h>
// void goCallback(void);
// static inline void triggerViolation(void) {
//     printf("[C] Executing triggerViolation...\n");
//     goCallback(); 
//     printf("[C] This line will never execute.\n");
// }
// #cgo nocallback triggerViolation
import "C"
import "fmt"

//export goCallback
func goCallback() {
	fmt.Println("[Go] Inside goCallback!")
}

func main() {
	fmt.Println("[Go] Calling triggerViolation...")
	C.triggerViolation()
}

As for the duplicated calls of strings.Fields: I think they are not pretty, but saveCgo() function is taken from go compiler code and it's good to keep diffs from the upstream code minimal. What do you think?

continue
}

// Split at colon.
line, argstr, ok := stringsCut(strings.TrimSpace(line[4:]), ":")
if !ok {
Expand Down
86 changes: 85 additions & 1 deletion src/python/pants/backend/go/util_rules/cgo_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@
from pants.core.goals.package import BuiltPackage
from pants.core.target_types import ResourceTarget
from pants.core.util_rules import source_files
from pants.engine.environment import EnvironmentName
from pants.engine.internals.native_engine import EMPTY_DIGEST
from pants.engine.process import Process, ProcessResult
from pants.engine.process import FallibleProcessResult, Process, ProcessResult
from pants.testutil.rule_runner import QueryRule, RuleRunner
from pants.testutil.skip_utils import skip_if_linux_arm64

Expand Down Expand Up @@ -68,6 +69,7 @@ def rule_runner() -> RuleRunner:
QueryRule(FallibleFirstPartyPkgDigest, [FirstPartyPkgDigestRequest]),
QueryRule(CGoCompileResult, [CGoCompileRequest]),
QueryRule(ProcessResult, (Process,)),
QueryRule(FallibleProcessResult, (EnvironmentName, Process,)),
],
target_types=[
GoModTarget,
Expand Down Expand Up @@ -879,3 +881,85 @@ def test_cgo_with_embedded_static_library(rule_runner: RuleRunner) -> None:

tgt = rule_runner.get_target(Address("", target_name="bin"))
rule_runner.request(BuiltPackage, [GoBinaryFieldSet.create(tgt)])


def test_cgo_nocallback_noescape_directives(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"BUILD": dedent(
"""\
go_mod(name="mod")
go_package(name="pkg")
go_binary(name="bin")
"""
),
"go.mod": "module example.pantsbuild.org/cgo_nocallback_noescape_test\n",
"main.go": dedent(
"""\
package main

// void goCallback();
// static inline void triggerViolation() {
// goCallback();
// }
// #cgo nocallback triggerViolation
import "C"
import "fmt"

//export goCallback
func goCallback() {
fmt.Println("[Go] Inside goCallback!")
}

func main() {
fmt.Println("[Go] Calling triggerViolation...")
C.triggerViolation()
fmt.Println("[Go] Should panic before this line due to violation of nocallback")
}
"""
),
}
)

tgt = rule_runner.get_target(Address("", target_name="pkg"))
maybe_analysis = rule_runner.request(
FallibleFirstPartyPkgAnalysis,
[FirstPartyPkgAnalysisRequest(tgt.address, build_opts=GoBuildOptions())],
)
assert maybe_analysis.analysis is not None
analysis = maybe_analysis.analysis
assert analysis.cgo_files == ("main.go",)

maybe_digest = rule_runner.request(
FallibleFirstPartyPkgDigest,
[FirstPartyPkgDigestRequest(tgt.address, build_opts=GoBuildOptions())],
)
assert maybe_digest.pkg_digest is not None
pkg_digest = maybe_digest.pkg_digest

cgo_request = CGoCompileRequest(
import_path=analysis.import_path,
pkg_name=analysis.name,
digest=pkg_digest.digest,
build_opts=GoBuildOptions(),
dir_path=analysis.dir_path,
cgo_files=analysis.cgo_files,
cgo_flags=analysis.cgo_flags,
)
cgo_compile_result = rule_runner.request(CGoCompileResult, [cgo_request])
assert cgo_compile_result.digest != EMPTY_DIGEST

tgt = rule_runner.get_target(Address("", target_name="bin"))
pkg = rule_runner.request(BuiltPackage, [GoBinaryFieldSet.create(tgt)])
result = rule_runner.request(
FallibleProcessResult,
[
Process(
argv=["./bin"],
input_digest=pkg.digest,
description="Run cgo binary",
)
],
)
assert result.exit_code != 0
assert "panic: runtime: function marked with #cgo nocallback called back into Go" in result.stderr.decode()