build(deps): bump golang.org/x/tools to v0.49.0 and github.com/goplus/mod to v0.22.0 - #2377
Conversation
There was a problem hiding this comment.
Review summary
Reviewed the fixed diff 0314fae...63ede18 across code quality, performance, security, and documentation.
Dependency bumps (go.mod/go.sum): golang.org/x/tools v0.38.0 -> v0.49.0, plus golang.org/x/mod, x/sys, x/sync (indirect), and goplus/mod v0.22.0. All checksums verify against the Go checksum database; direct/indirect classification is correct.
ssa/decl.go — the moduleZeroName sentinel guard in Init/InitNil is correct: a Go identifier can't contain $, so no source-level global can collide with __llgo.moduleZeroSizedAlloc$. No miscompilation path.
cl/static_init.go — the new alloc-tracing logic is careful: referrer counts are validated, recursion is cycle-guarded via visited, and appendStaticInitPath allocates fresh slices to avoid aliasing. No correctness-blocking issues found. Two maintainability notes inline.
benchmark/baseline/main.go — buildArgs is built from a fresh literal per iteration, so no append-aliasing bug; README matches the six workloads. Good.
Performance — appendStaticInitPath is O(depth^2) over an address chain, but this mirrors the pre-existing staticInitStorePath and runs only over init-function stores on shallow chains at compile time. Negligible; not blocking.
Additional note (no reliable inline anchor)
.github/workflows/benchmark.yml(merge-base step):github.event.pull_request.*values are interpolated directly into arun:shell block (base.repo.full_name,base.ref,head.sha). This is the classic Actions script-injection shape. Exploitability is low here — the trigger ispull_request(notpull_request_target), permissions arecontents: read, forked runs get no secrets, andhead.shais a validated hex SHA. As defense-in-depth, consider moving those expressions into a step-levelenv:block and referencing quoted shell vars ("$BASE_REPO","$BASE_REF","$HEAD_SHA"), so the pattern stays safe if the trigger/permissions ever change.
Overall: solid change, no blocking issues.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Compared with |
63ede18 to
9e4982b
Compare
d394f8d to
b085b5c
Compare
Note
#2376 is now merged. This branch is rebased onto current
main; the static-initializer and zero-sized-global fixes from #2376 are required by the x/tools v0.49 upgrade.Summary
github.com/goplus/modfrom v0.21.2 to v0.22.0.golang.org/x/toolsfrom v0.38.0 to v0.49.0, with the correspondingx/mod,x/sync, andx/sysversions.Why the #2376 fixes are required
x/tools v0.49 includes
go/ssa: avoid in-place initialization of CompositeLit(golang/tools@59ef372), which fixes golang/go#79414. Composite values are now built in a temporary aggregate and copied to their destination only after every right-hand-side expression has been evaluated.That shape bypasses LLGo's old direct-global static initializer path. Without #2376, a real
fmt.Printfworkload on v0.49 also fails LLVM verification when a zero-sized global aliases the shared sentinel. #2376 fixes that correctness issue and folds only side-effect-free constant global aggregates; dynamic and local assignments retain the new x/tools ordering.Validation
go mod tidy,go mod verify, repository-wide gofmt, baseline unit tests, inherited static-init tests, and the fullssasuite pass locally.