Repository navigation
feat(c/signal): restore native APIs and signal layouts - #48
MeteorsLiu wants to merge 8 commits into
Conversation
|
Merge order: #45 → #51 → #48 → #49 → #44 → #47 → #46 → #50 → #52. This draft changes only All retained native signal functions have link-address and native-prototype coverage; public types, constants, globals and generated union accessors also have complete source-inventory coverage. Executed tests pass on macOS arm64, Linux amd64 and Linux arm64 with GC and Private C entry points and their private macOS wrapper are removed. Linux Standalone build CI currently stops at |
There was a problem hiding this comment.
Review — c/signal native types
Ran code-quality, performance, security, and documentation passes over the four generated files.
Verified correct — no action needed
- Struct layouts match the glibc amd64 ABI, confirmed with a size/offset probe:
SiginfoT= 128 bytes (__SI_MAX_SIZE) with_sifieldsat offset 16,Sigval= 8 bytes,SigsetT= 128 bytes ([16]c.Ulong). DarwinSigsetTresolves to a 4-byteuint32, matching__darwin_sigset_t. - Every
XGof_ref_*accessor casts strictly within its backing union — no out-of-bounds reads — and they are zero-costunsafe.Pointerreinterpretations (no copies/allocations). - Package builds for
linux/amd64anddarwin/arm64; all files are gofmt-clean.
Findings (non-blocking)
These are llcppg-generated files, so fixes most likely belong in the generator/config rather than hand edits.
- Platform-coverage gap on the published type surface — see inline on
llcppg.pub. [???]doc placeholder — see inline ontypes_darwin_arm64.go.
Minor nits
_llcppg_anon_34(_sigsys,libc_linux_amd64.go:113) is the only_sifieldsunion member without the category doc comment its siblings carry (glibc labels it/* SIGSYS. */).- Darwin maps
__uint32_ttoc.Uint(types_darwin_arm64.go:9) while the linux file usesc.Uint32Tfor the analogous 32-bit field — same width today, purely a cross-file consistency nit.
| @@ -0,0 +1,5 @@ | |||
| D github.com/goplus/lib/c | |||
| T __sigval_t X__SigvalT | |||
| T siginfo_t SiginfoT | |||
There was a problem hiding this comment.
llcppg.pub publishes SiginfoT, Sigval, and X__SigvalT as package types, but they are defined only for linux && amd64 (libc_linux_amd64.go). On darwin/arm64 only SigsetT exists, and on other platforms none of these do — so any consumer that references signal.SiginfoT or signal.Sigval will fail to build outside linux/amd64. If this is an intended incremental first step, no change needed; otherwise consider generating the darwin (and other-platform) definitions so the published surface matches what is actually available.
|
|
||
| type SigsetT = X__DarwinSigsetT | ||
|
|
||
| // [???] signal set |
There was a problem hiding this comment.
The doc comment // [???] signal set carries a [???] placeholder artifact that surfaces in godoc. __darwin_sigset_t is simply __uint32_t, so this should read something like // signal set. Worth fixing at the generator so the placeholder is not emitted.
931115b to
45790ae
Compare
Verify every retained native signal symbol and exercise real handler delivery while preserving the existing
Signalcontract.The implementation includes:
Signalreturning theSigactioninstallation status andNativeSignalreturning Csignal's previous handler; use generated records and union accessors without changing existing public function signatures.SysSiglistwith explicit approval because current glibc provides no defaultsys_siglistexport._xgo_unionstorage as a C union while retaining all size/alignment assertions.sigactionoutput, signal-set operations, handler installation/restoration, realRaisedelivery,Kill(self, 0), and the complete Linux non-notifying timer lifecycle.TimerCreatekeeps its link check and uses an approved static signature/ABI check to avoid the dependency's reflection-wrapper limitation.Executed LLGo v1.0.8 (
c171abfa130108bc6451e01fcf15a8a3474eb0a7) GC andnogctests pass on all three targets. Validation includes #45's rootcfiles at99ddac92c7335a53a1c996174e1753bcd72c5050and #51'sc/timefiles at855f0c294499b32a0d3380d1d608592255155e89; merge #45, then #51, before this draft. Standalone build CI currently reportsundefined: lltime.TimerTbecause #51 is not merged. The existingc/timefull-signature reflection limitation remains documented and unchanged. This PR changes onlyc/signaland makes missing symbol coverage and native layout regressions visible.