Skip to content

Commit f5bd750

Browse files
jakebaileydeadprogram
authored andcommitted
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.
1 parent 0a5875a commit f5bd750

4 files changed

Lines changed: 66 additions & 7 deletions

File tree

interp/interp_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ func TestInterp(t *testing.T) {
1818
"copy",
1919
"interface",
2020
"revert",
21+
"store",
2122
"alloc",
2223
} {
2324
name := name // make local to this closure

interp/memory.go

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -330,26 +330,41 @@ func (mv *memoryView) store(v value, p pointerValue) bool {
330330
if checks && mv.hasExternalLoadOrStore(p) {
331331
panic("interp: store to object with external load/store")
332332
}
333-
obj := mv.get(p.index())
333+
index := p.index()
334+
var obj object
335+
writable := false
336+
if mv.objects != nil {
337+
obj, writable = mv.objects[index]
338+
}
339+
if !writable {
340+
obj = mv.get(index)
341+
}
334342
if obj.buffer == nil {
335343
// External global, return false (for a failure).
336344
return false
337345
}
338-
if checks && p.offset()+v.len(mv.r) > obj.size {
346+
valueLen := v.len(mv.r)
347+
if checks && p.offset()+valueLen > obj.size {
339348
panic("interp: store out of bounds")
340349
}
341-
if p.offset() == 0 && v.len(mv.r) == obj.buffer.len(mv.r) {
342-
obj.buffer = v
350+
if p.offset() == 0 && valueLen == obj.buffer.len(mv.r) {
351+
obj.buffer = v.clone()
343352
} else {
344-
obj = obj.clone()
353+
if !writable {
354+
obj = obj.clone()
355+
}
345356
buffer := obj.buffer.asRawValue(mv.r)
346357
obj.buffer = buffer
347358
v := v.asRawValue(mv.r)
348-
for i := uint32(0); i < v.len(mv.r); i++ {
359+
if writable {
360+
// A partial load from this object may share the destination buffer.
361+
v.buf = append([]uint64(nil), v.buf...)
362+
}
363+
for i := uint32(0); i < valueLen; i++ {
349364
buffer.buf[p.offset()+i] = v.buf[i]
350365
}
351366
}
352-
mv.put(p.index(), obj)
367+
mv.put(index, obj)
353368
return true // success
354369
}
355370

interp/testdata/store.ll

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
2+
target triple = "x86_64--linux"
3+
4+
@overlap.buf = global [4 x i8] c"\01\02\03\04"
5+
@alias.src = global [4 x i8] c"\05\06\07\08"
6+
@alias.dst = global [2 x i8] zeroinitializer
7+
8+
define void @runtime.initAll() unnamed_addr {
9+
entry:
10+
call void @overlap.init(ptr undef)
11+
call void @alias.init(ptr undef)
12+
ret void
13+
}
14+
15+
define internal void @overlap.init(ptr %context) unnamed_addr {
16+
entry:
17+
%tail = getelementptr [4 x i8], ptr @overlap.buf, i32 0, i32 3
18+
store i8 9, ptr %tail
19+
%val = load i16, ptr @overlap.buf
20+
%dst = getelementptr [4 x i8], ptr @overlap.buf, i32 0, i32 1
21+
store i16 %val, ptr %dst
22+
ret void
23+
}
24+
25+
define internal void @alias.init(ptr %context) unnamed_addr {
26+
entry:
27+
%src = getelementptr [4 x i8], ptr @alias.src, i32 0, i32 1
28+
%val = load i16, ptr %src
29+
store i16 %val, ptr @alias.dst
30+
store i8 9, ptr @alias.dst
31+
ret void
32+
}

interp/testdata/store.out.ll

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
2+
target triple = "x86_64--linux"
3+
4+
@overlap.buf = local_unnamed_addr global [4 x i8] c"\01\01\02\09"
5+
@alias.src = local_unnamed_addr global [4 x i8] c"\05\06\07\08"
6+
@alias.dst = local_unnamed_addr global [2 x i8] c"\09\07"
7+
8+
define void @runtime.initAll() unnamed_addr {
9+
entry:
10+
ret void
11+
}

0 commit comments

Comments
 (0)