loader: LoadOne/LoadMany with NoPreempt left as false produces empty PcUnsafePoint table, crashes runtime async preempt

[Maki-Daisuke]

I understand sonic itself always sets NoPreempt: true in its in-tree callers (e.g. loader/wrapper.go), so this bug is invisible from sonic's own usage and probably very low priority for the team. I'm filing it because I'm building a small JIT on top of sonic/loader in my own project (the loader linkname-shim is exactly the abstraction I need for hosting ABI0 functions in moduledata), and I tripped on this when leaving NoPreempt at its zero value. I have a working user-side workaround, so feel free to deprioritize — but I wanted to leave a clear record in case anyone else hits the same wall, and I'm happy to send a PR if maintainers agree on the direction.

Describe the bug

loader.Loader.LoadOne / LoadMany, when NoPreempt is left at its zero value (false), produce a PcUnsafePoint table that encodes to zero entries (just the terminator byte). The Go runtime's async-preempt path then crashes with invalid pc-encoded table the first time sysmon tries to preempt a goroutine running inside a loaded function.

Root cause — interaction of two pieces of code:

1. loader/loader_latest.go:48-56 (buildLoadFunc, noPreempt == false branch) sets:

   fn.PcUnsafePoint = &Pcdata{
       {PC: textSize, Val: PCDATA_UnsafePointSafe}, // -1
   }

2. loader/pcdata.go:73-94 (Pcdata.MarshalBinary) initializes the running state as sv := int32(_PCDATA_START_VAL) /* -1 */; sp := uint32(0), then for each entry computes dp := v.PC - sp, dv := v.Val - sv, and:

   if dv == 0 || dp == 0 {
       continue
   }

For the single entry above: dp = textSize - 0 = textSize (non-zero), but dv = (-1) - (-1) = 0 → the entry is skipped. The function then appends only the trailing 0 terminator, yielding a 1-byte pcdata table with no real entries.

When runtime.sysmon calls preemptone → preemptM → isAsyncSafePoint → pcdatavalue2 → pcvalue on a PC inside the loaded function, pcvalue walks the empty table, fails to find an entry covering the target PC, and panics:

runtime: invalid pc-encoded table f=<funcname> pc=... targetpc=... tab=[0/0]0x0
fatal error: invalid runtime symbol table

The same shape applies to any single-entry Pcdata whose Val == _PCDATA_START_VAL. The async-preempt path is just the most visible victim because the runtime hits the table on every preempt attempt.

Why this isn't caught today:

• loader/loader_go117_test.go builds PcUnsafePoint = {{size, PCDATA_UnsafePointUnsafe (-2)}}, where dv = -2 - (-1) = -1 is non-zero, so the table encodes correctly.
• All in-tree callers in loader/wrapper.go hard-code {{size, PCDATA_UnsafePointUnsafe}}, the same shape, and always set NoPreempt: true.
• Reaching this bug requires (a) using the public Loader API without setting NoPreempt: true, and (b) the loaded code being long enough that sysmon actually attempts to preempt it. Sonic's own JSON code paths never take this branch, so the bug is invisible from sonic's own tests and benchmarks.

To Reproduce

Steps to reproduce the behavior:

1. Create a fresh module and add github.com/bytedance/sonic/loader v0.5.1.

2. Save the following as main.go:

   //go:build amd64 && (windows || linux)
   
   package main
   
   import (
       "runtime"
       "sync"
       "time"
       "unsafe"
   
       "github.com/bytedance/sonic/loader"
   )
   
   // 20 bytes of x86-64:
   //   mov  rax, 42         ; 48 C7 C0 2A 00 00 00
   //   mov  rcx, 100000000  ; 48 C7 C1 00 E1 F5 05
   //   dec  rcx             ; 48 FF C9
   //   jnz  -5              ; 75 FB
   //   ret                  ; C3
   var spinReturnCode = []byte{
       0x48, 0xC7, 0xC0, 0x2A, 0x00, 0x00, 0x00,
       0x48, 0xC7, 0xC1, 0x00, 0xE1, 0xF5, 0x05,
       0x48, 0xFF, 0xC9,
       0x75, 0xFB,
       0xC3,
   }
   
   func main() {
       ld := loader.Loader{
           Name: "repro.spin",
           File: "repro.go",
           // NoPreempt left at zero value (false) — this is what triggers the bug.
       }
       out := ld.LoadMany([]loader.LoadOneItem{{
           Text:      spinReturnCode,
           FuncName:  "spin",
           FrameSize: 0,
           ArgSize:   0,
           Pcdata:    loader.Pcdata{{PC: uint32(len(spinReturnCode)), Val: 0}},
       }})
       spin := *(*func() int64)(unsafe.Pointer(&out[0]))
   
       var wg sync.WaitGroup
       stop := time.After(3 * time.Second)
       for i := 0; i < runtime.NumCPU(); i++ {
           wg.Add(1)
           go func() {
               defer wg.Done()
               for {
                   select {
                   case <-stop:
                       return
                   default:
                       spin()
                   }
               }
           }()
       }
       go func() {
           for {
               select {
               case <-stop:
                   return
               default:
                   runtime.GC()
                   time.Sleep(10 * time.Millisecond)
               }
           }
       }()
       wg.Wait()
   }

3. Run go run main.go.

4. Within ~100 ms, observe fatal error: invalid runtime symbol table from runtime.sysmon → preemptM → isAsyncSafePoint.

Expected behavior

The program runs for 3 seconds and exits cleanly. The PcUnsafePoint table for a function loaded with NoPreempt: false should encode to a non-empty pcdata table covering [0, textSize) with value PCDATA_UnsafePointSafe, so that the runtime's async-preempt machinery can read it without crashing.

Screenshots

Sonic version

github.com/bytedance/sonic/loader v0.5.1 — corresponds to the tag loader/v0.5.1 (the latest loader release at the time of filing).

The relevant source paths referenced below (loader/pcdata.go, loader/loader_latest.go, loader/wrapper.go, loader/loader_go117_test.go) are all under the loader/ subdirectory module, not the main sonic module.

Environment

$ go env
AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/yanother/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/yanother/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3445309661=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/dev/null'
GOMODCACHE='/home/yanother/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/yanother/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/yanother/golang'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/yanother/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/yanother/golang/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.26.3'
GOWORK=''
PKG_CONFIG='pkg-config'

Also reproducible on windows/amd64 with the same Go version.

Additional context

Workaround in user code — bypass Loader.LoadOne/LoadMany and call low-level loader.Load with a hand-crafted PcUnsafePoint:

PcUnsafePoint: &Pcdata{
    {PC: 1,        Val: PCDATA_UnsafePointUnsafe}, // -2
    {PC: textSize, Val: PCDATA_UnsafePointSafe},   // -1
},

This forces the first entry's dv to be non-zero (-2 - (-1) = -1) so it encodes correctly. The 1-byte unsafe window at function entry is observationally equivalent to "preempt at entry".

Proposed fixes (pick one)

• Option A — fix in loader/pcdata.go (preferred, minimal blast radius). The dv == 0 short-circuit only saves bytes when an entry is genuinely redundant (same value as previous), but it also drops the first entry whenever its value happens to equal _PCDATA_START_VAL. Either keep only the dp == 0 skip:

  // Only skip duplicate-of-previous entries.
  // Skipping dv==0 alone is unsafe: it drops the first entry when its Val
  // happens to equal _PCDATA_START_VAL.
  if dp == 0 {
      continue
  }

  or keep the dv == 0 skip but exempt the first iteration via a first := true flag.

• Option B — fix in loader/loader_latest.go's buildLoadFunc so the NoPreempt: false branch emits a 2-entry table similar to the workaround above. Fixes the visible crash but leaves the latent encoder bug for any other future caller.

I'd recommend Option A; Option B can ship alongside as defense-in-depth.

Test that would have caught this

func TestPcdataMarshalBinary_SingleSafeEntry(t *testing.T) {
    pcd := Pcdata{{PC: 32, Val: PCDATA_UnsafePointSafe}}
    data, err := pcd.MarshalBinary()
    if err != nil {
        t.Fatal(err)
    }
    // Expect a real entry, not just the terminator.
    if len(data) <= 1 {
        t.Fatalf("encoded table has no entries: %x", data)
    }
}

If maintainers agree on Option A, I'd be happy to send a PR against develop (branch bugfix/pcdata-skips-first-entry) per CONTRIBUTING.md.