diff --git a/cl/_testrt/cvar/in.go b/cl/_testrt/cvar/in.go index 408e4942eb..75db1c72b2 100644 --- a/cl/_testrt/cvar/in.go +++ b/cl/_testrt/cvar/in.go @@ -18,9 +18,6 @@ var barY struct { Arr [16]int8 } -// CHECK-LABEL: define void @main.main(){{.*}} { -// CHECK: load { [16 x i8], [2 x ptr] }, ptr @_bar_x -// CHECK: load { [16 x i8] }, ptr @_bar_y func main() { _ = barX _ = barY diff --git a/cl/compile.go b/cl/compile.go index 6ea1569324..14903e83f1 100644 --- a/cl/compile.go +++ b/cl/compile.go @@ -1325,34 +1325,28 @@ func (p *context) compileInstrOrValue(b llssa.Builder, iv instrOrValue, asValue if _, ok := p.methodNilDerefChecks[v]; ok { return p.compileCheckedDeref(b, v) } - if isEffectfulArrayPointerDeref(v) { - x := p.compileValue(b, v.X) - p.recordPanicLocation(b, v.Pos()) - b.AssertNilDeref(x) - } + effectfulArrayDeref := isEffectfulArrayPointerDeref(v) if refs, ok := nonDebugReferrers(v); ok && len(refs) == 0 { - if t := p.type_(v.Type(), llssa.InGo); t.RawType() != nil { - if p.isLargeNonPointerValue(t) { - x := p.compileValue(b, v.X) + if skipUnusedArrayDeref(v) { + x := p.compileValue(b, v.X) + if effectfulArrayDeref { p.recordPanicLocation(b, v.Pos()) - p.assertNilDerefBase(b, v.X) b.AssertNilDeref(x) - return } - } - if skipUnusedArrayDeref(v) { - p.compileValue(b, v.X) - return - } - if _, ok := types.Unalias(v.Type()).Underlying().(*types.Slice); ok { - // Zero-length slice-to-array conversions can leave only - // an unused slice deref; preserve its required nil check. - x := p.compileValue(b, v.X) - p.recordPanicLocation(b, v.Pos()) - p.assertNilDerefBase(b, v.X) - b.AssertNilDeref(x) return } + // Elide the unused load, but keep an explicit nil check so the + // Go dereference still panics instead of relying on a trapping load. + x := p.compileValue(b, v.X) + p.recordPanicLocation(b, v.Pos()) + p.assertNilDerefBase(b, v.X) + b.AssertNilDeref(x) + return + } + if effectfulArrayDeref { + x := p.compileValue(b, v.X) + p.recordPanicLocation(b, v.Pos()) + b.AssertNilDeref(x) } if refs, ok := nonDebugReferrers(v); ok && len(refs) == 1 { if _, ok := refs[0].(*ssa.MakeInterface); ok { diff --git a/cl/range_array_compile_test.go b/cl/range_array_compile_test.go index f004f839da..791ec4a3ea 100644 --- a/cl/range_array_compile_test.go +++ b/cl/range_array_compile_test.go @@ -143,8 +143,8 @@ func rangeArrayReceive(ch <-chan *[3]int) { for _, name := range []string{"rangeArrayCall", "rangeArrayReceive"} { ir := mustNamedFunction(t, m, "foo."+name).String() - if !strings.Contains(ir, "AssertNilDeref") { - t.Fatalf("%s should preserve its required array pointer nil check:\n%s", name, ir) + if got := strings.Count(ir, "AssertNilDeref"); got != 1 { + t.Fatalf("%s nil-check count = %d, want 1:\n%s", name, got, ir) } } } @@ -373,6 +373,41 @@ func copyArray(p *[5]int) [5]int { } } +func TestEffectfulArrayDerefWithBuiltinRefKeepsNilCheck(t *testing.T) { + ssaPkg, _, files := buildGoSSAPkg(t, ` +package foo + +func nextArray() *[3]int { return nil } + +func discard() { _ = *nextArray() } + +func lenSlice(s []int) int { return len(s) } +`) + + load := findUnOp(t, ssaPkg.Func("discard"), token.MUL, true) + refs := load.Referrers() + if refs == nil { + t.Fatal("array deref has no referrer list") + } + oldRefs := *refs + // Current x/tools folds len(*nextArray()) to a static constant and leaves + // the deref unused. Model the builtin ref shape retained by other supported + // SSA versions so the compiler compatibility path remains covered. + builtin := findBuiltinCall(t, ssaPkg.Func("lenSlice"), "len") + *refs = []ssa.Instruction{&ssa.Call{Call: ssa.CallCommon{Value: builtin}}} + defer func() { *refs = oldRefs }() + + prog := newLLSSAProg(t) + pkg, err := NewPackage(prog, ssaPkg, files) + if err != nil { + t.Fatal(err) + } + ir := mustNamedFunction(t, pkg.Module(), "foo.discard").String() + if !strings.Contains(ir, "foo.nextArray") || !strings.Contains(ir, "AssertNilDeref") { + t.Fatalf("builtin-referenced array deref should retain its call and nil check:\n%s", ir) + } +} + func findUnOp(t *testing.T, fn *ssa.Function, op token.Token, wantArray bool) *ssa.UnOp { t.Helper() for _, block := range fn.Blocks { diff --git a/cl/zero_size_deref_test.go b/cl/zero_size_deref_test.go index 48de2b8d67..a84cccc06f 100644 --- a/cl/zero_size_deref_test.go +++ b/cl/zero_size_deref_test.go @@ -71,3 +71,24 @@ func keepPointer(pointer *struct{}) func() bool { }) } } + +func TestUnusedDerefEmitsNilGuard(t *testing.T) { + const src = `package unusedderef +func LoadArrayElement() { + var values [2]*int + _ = *values[1] +} +func LoadPointer(p *int) { + _ = *p +} +` + ir := compileWithRewrites(t, src, nil) + arrayLoad := llvmFunction(t, ir, "unusedderef.LoadArrayElement") + if !strings.Contains(arrayLoad, "AssertNilDeref") { + t.Fatalf("unused array-element dereference should retain a nil guard:\n%s", arrayLoad) + } + directLoad := llvmFunction(t, ir, "unusedderef.LoadPointer") + if !strings.Contains(directLoad, "AssertNilDeref") { + t.Fatalf("unused direct dereference should retain a nil guard:\n%s", directLoad) + } +} diff --git a/test/go/nil_deref_address_test.go b/test/go/nil_deref_address_test.go index e2a9506b3d..c02f966f53 100644 --- a/test/go/nil_deref_address_test.go +++ b/test/go/nil_deref_address_test.go @@ -47,6 +47,33 @@ func TestNilDerefAddressOperationsPanic(t *testing.T) { } } +func TestUnusedNilDerefOperationsPanic(t *testing.T) { + tests := []struct { + name string + f func() + }{ + { + name: "direct pointer", + f: func() { + var p *int + _ = *p + }, + }, + { + name: "pointer loaded from array", + f: func() { + var values [2]*int + _ = *values[1] + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + expectNilDerefAddressPanic(t, tt.f) + }) + } +} + func TestNilDerefPrintedCompositeLoadsPanic(t *testing.T) { tests := []struct { name string diff --git a/test/goroot/xfail.yaml b/test/goroot/xfail.yaml index dae23f6fb2..3ae5c8c3ba 100644 --- a/test/goroot/xfail.yaml +++ b/test/goroot/xfail.yaml @@ -1391,14 +1391,6 @@ xfails: directive: run case: fixedbugs/issue29504.go reason: panic/fault statement-line granularity in untracked functions (P4b prebuilt pcline) - - platform: darwin/arm64 - directive: run - case: fixedbugs/issue38496.go - reason: current main goroot run failure on darwin/arm64 - - platform: linux/amd64 - directive: run - case: fixedbugs/issue38496.go - reason: current main goroot run failure on linux/amd64 - version: go1.26 platform: darwin/arm64 directive: rundir