ssa: describe audited runtime function contracts - #2510
Draft
zhouguangyuan0718 wants to merge 2 commits into
Draft
zhouguangyuan0718 wants to merge 2 commits into
zhouguangyuan0718 wants to merge 2 commits into
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review: runtime function attributes
This PR attaches LLVM function attributes (memory effects, capture info, range, noreturn/cold, nonnull/returned) to a fixed set of runtime symbols so the optimizer can reason about them across go:linkname boundaries. The design is careful and well-tested (multi-target IR checks, O2/LTO fold verification, linkname-aliasing coverage).
Verified sound:
- Parameter-index → LLVM-index mapping lines up with Go param positions (no sret/byval expansion for these signatures).
- Linkname resolution happens in
rtFuncbeforeNewFunc, so the contract map is correctly keyed by resolved symbol names; package-qualified keys prevent cross-package collisions. - The bit constants (
runtimeMemoryRead=0x555,runtimeArgRead/ReadWrite,runtimeCaptureReturn=0xf) correctly encode LLVM'sMemoryEffects/CaptureInfolayouts, and all accompanying comments are accurate. - The
rangecgo bridge is ABI-sound; thebits!=32&&bits!=64guard prevents shift UB. - No security or performance concerns (the per-function cost is a single map miss with an early return before any closure allocation).
The findings below are hardening/maintainability suggestions, not active defects in the current tested configuration.
| runtimeCaptureReturn = 0xf | ||
| ) | ||
|
|
||
| func (p Program) addRuntimeAttributes(fn llvm.Value, name string) { |
Contributor
There was a problem hiding this comment.
Hardening: no signature/arity validation before applying per-parameter pointer attributes.
addRuntimeAttributes keys purely on the resolved name and then blindly applies pointer(2, "writeonly"), pointer(3, "readonly"), etc. The reflect.typedmemmove entry is a bare pkg.name reachable by the real reflect package; today its signature matches the move contract, but nothing enforces that. If a contract-named symbol ever has a different shape (signature drift, or a future name collision), pointer attributes would be attached to a non-pointer parameter or an out-of-range index, producing a silent miscompile that is very hard to trace.
Consider asserting parameter count and that the targeted indices are pointer-typed before adding pointer attributes (mirroring the width guard already in addNonNegativeReturnRange). Cheap, and it turns a latent silent-miscompile into a loud panic.
|
|
||
| func addNonNegativeReturnRange(ctx llvm.Context, fn llvm.Value, bits int) { | ||
| if bits != 32 && bits != 64 { | ||
| panic("ssa: unsupported runtime integer width") |
Contributor
There was a problem hiding this comment.
Minor: the panic message "ssa: unsupported runtime integer width" doesn't identify the offending function. If a mismatched symbol reaches the length contract (see the related note in runtime_attrs.go), the failure surfaces here without pointing at which function caused it. Including the function name would make the failure actionable.
| if !ok { | ||
| return | ||
| } | ||
| add := func(index int, name string, value uint64) { |
Contributor
There was a problem hiding this comment.
Minor readability: the closure parameter name string shadows the enclosing function's name argument (the symbol name). Harmless since the outer name isn't used after the map lookup, but shadowing an attribute-kind string over a symbol-name string is easy to misread. Renaming the closure param to attr/kind would remove the ambiguity.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
LLGo WebAssembly build benchmarks
WebAssembly output sizes
LLGo WebAssembly build measurements
Compared with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Runtime declarations currently omit contracts that their implementations already guarantee, so callers in separate modules cannot eliminate redundant comparisons, reuse read-only results, or exploit nonnegative lengths. Attach a first batch of audited LLVM 22 return, parameter, and function attributes when creating the resolved runtime symbol, covering both definitions and imported/linkname declarations.
AssertNilDerefPtr:returnedinput andnonnullresult. The input remains nullable and the panic-producing call remains observable.CStrCopy: returned destination, write-only access, return-only capture, and read/write memory effects that include the aggregate string source.memequal, string comparisons, typed move/clear,MapLen,ChanCap, and hash helpers: precise pointer access/capture and memory effects, plusnofree,nosync,nounwind, andwillreturn. Length/capacity results receive a target-width nonnegative range. String aggregates and global hash seeds require effects beyond argument memory.coldandnoreturn, without purity ornounwindpromises.Rethrow, conditional checks,throw, and lockingChanLenare excluded.Preserve overlap and zero-length semantics by leaving copy/comparison pointer arguments nullable and without
noalias. Matchreflect.typedmemmoveafter linkname resolution. Allocation contracts and constructor changes are outside this PR.Use
Context.CreateConstantRangeAttributefrom xgo-dev/llvm#52. LLGo contains no private cgo bridge for this API. Until the binding change is released,go.modtemporarily replacesgithub.com/xgo-dev/llvmwith the personal branch pinned at4de18f5844ba28c8f608312e7a1f0606ade54d41(v0.0.0-20260905230337-4de18f5844ba); both module checksums are recorded ingo.sum. Replace this temporary pin with the upstream binding release when available.Validation on macOS arm64 with Go 1.27.0 and LLVM/Clang 22.1.8:
go test ./ssa ./cl -count=1passed against the pinned binding commit.llgo test -v ./internal/testandllgo test -tags=nogc -v ./internal/testpassed fromruntime/, covering recoverable nil/panic behavior, overlapping moves through both runtime and reflect entries, zero-byte equality, typed clearing, C strings, and map/channel queries.