From f5bd750a4d8a8f27e8371d1caa5d7d5ee9a8dcc9 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 4 May 2026 10:53:41 -0700 Subject: [PATCH] interp: avoid repeated object clones on partial stores memoryView.store cloned the entire destination object on every partial store. Compiling a program that pulls in golang.org/x/text/collate hit this hard: its generated tables are initialized one element at a time, and each element store copied the whole global, making interp quadratic in the table size. Switch to copy-on-first-write per memory view: clone the object the first time this view writes to it, then mutate that private copy in place on later partial stores. Rollback is unaffected because revert still discards the view's objects wholesale. Two extra safety tweaks fall out of this: - Full overwrites now clone the incoming value, so the stored buffer can never alias state held by the caller. - Partial in-place stores snapshot the source buffer, so a store whose source is a load from the same object (overlapping or not) still sees the pre-store bytes. Add an interp golden test exercising both the overlapping load/store case and a cross-object aliased load/store case. --- interp/interp_test.go | 1 + interp/memory.go | 29 ++++++++++++++++++++++------- interp/testdata/store.ll | 32 ++++++++++++++++++++++++++++++++ interp/testdata/store.out.ll | 11 +++++++++++ 4 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 interp/testdata/store.ll create mode 100644 interp/testdata/store.out.ll diff --git a/interp/interp_test.go b/interp/interp_test.go index 52ffb95a2..e7e37a76d 100644 --- a/interp/interp_test.go +++ b/interp/interp_test.go @@ -18,6 +18,7 @@ func TestInterp(t *testing.T) { "copy", "interface", "revert", + "store", "alloc", } { name := name // make local to this closure diff --git a/interp/memory.go b/interp/memory.go index 147bc5f2a..8b7834787 100644 --- a/interp/memory.go +++ b/interp/memory.go @@ -330,26 +330,41 @@ func (mv *memoryView) store(v value, p pointerValue) bool { if checks && mv.hasExternalLoadOrStore(p) { panic("interp: store to object with external load/store") } - obj := mv.get(p.index()) + index := p.index() + var obj object + writable := false + if mv.objects != nil { + obj, writable = mv.objects[index] + } + if !writable { + obj = mv.get(index) + } if obj.buffer == nil { // External global, return false (for a failure). return false } - if checks && p.offset()+v.len(mv.r) > obj.size { + valueLen := v.len(mv.r) + if checks && p.offset()+valueLen > obj.size { panic("interp: store out of bounds") } - if p.offset() == 0 && v.len(mv.r) == obj.buffer.len(mv.r) { - obj.buffer = v + if p.offset() == 0 && valueLen == obj.buffer.len(mv.r) { + obj.buffer = v.clone() } else { - obj = obj.clone() + if !writable { + obj = obj.clone() + } buffer := obj.buffer.asRawValue(mv.r) obj.buffer = buffer v := v.asRawValue(mv.r) - for i := uint32(0); i < v.len(mv.r); i++ { + if writable { + // A partial load from this object may share the destination buffer. + v.buf = append([]uint64(nil), v.buf...) + } + for i := uint32(0); i < valueLen; i++ { buffer.buf[p.offset()+i] = v.buf[i] } } - mv.put(p.index(), obj) + mv.put(index, obj) return true // success } diff --git a/interp/testdata/store.ll b/interp/testdata/store.ll new file mode 100644 index 000000000..6d532f77f --- /dev/null +++ b/interp/testdata/store.ll @@ -0,0 +1,32 @@ +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64--linux" + +@overlap.buf = global [4 x i8] c"\01\02\03\04" +@alias.src = global [4 x i8] c"\05\06\07\08" +@alias.dst = global [2 x i8] zeroinitializer + +define void @runtime.initAll() unnamed_addr { +entry: + call void @overlap.init(ptr undef) + call void @alias.init(ptr undef) + ret void +} + +define internal void @overlap.init(ptr %context) unnamed_addr { +entry: + %tail = getelementptr [4 x i8], ptr @overlap.buf, i32 0, i32 3 + store i8 9, ptr %tail + %val = load i16, ptr @overlap.buf + %dst = getelementptr [4 x i8], ptr @overlap.buf, i32 0, i32 1 + store i16 %val, ptr %dst + ret void +} + +define internal void @alias.init(ptr %context) unnamed_addr { +entry: + %src = getelementptr [4 x i8], ptr @alias.src, i32 0, i32 1 + %val = load i16, ptr %src + store i16 %val, ptr @alias.dst + store i8 9, ptr @alias.dst + ret void +} diff --git a/interp/testdata/store.out.ll b/interp/testdata/store.out.ll new file mode 100644 index 000000000..1de806aa1 --- /dev/null +++ b/interp/testdata/store.out.ll @@ -0,0 +1,11 @@ +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64--linux" + +@overlap.buf = local_unnamed_addr global [4 x i8] c"\01\01\02\09" +@alias.src = local_unnamed_addr global [4 x i8] c"\05\06\07\08" +@alias.dst = local_unnamed_addr global [2 x i8] c"\09\07" + +define void @runtime.initAll() unnamed_addr { +entry: + ret void +}