interp: avoid repeated object clones on partial stores

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

interp/memory.go

@@ -316,10 +316,14 @@ func (mv *memoryView) load(p pointerValue, size uint32) value {
 		panic("interp: load out of bounds")
 	}
 	v := obj.buffer.asRawValue(mv.r)
-	loadedValue := rawValue{
-		buf: v.buf[p.offset() : p.offset()+size],
+	loadedBuf := v.buf[p.offset() : p.offset()+size]
+	if _, writable := mv.objects[p.index()]; writable {
+		// This object's buffer is owned by this view, which means a later
+		// store may mutate it in place (see store below). Copy the loaded
+		// slice so the returned value is not aliased with the live buffer.
+		loadedBuf = append([]uint64(nil), loadedBuf...)
 	}
-	return loadedValue
+	return rawValue{buf: loadedBuf}
 }
 
 // Store to the value behind the given pointer. This overwrites the value in the
@@ -330,26 +334,37 @@ 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++ {
+		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
 }
 

interp/testdata/store.ll

@@ -0,0 +1,49 @@
+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
+@reload.buf = global [4 x i8] c"\01\02\03\04"
+@reload.out = global [2 x i8] zeroinitializer
+
+define void @runtime.initAll() unnamed_addr {
+entry:
+  call void @overlap.init(ptr undef)
+  call void @alias.init(ptr undef)
+  call void @reload.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
+}
+
+define internal void @reload.init(ptr %context) unnamed_addr {
+entry:
+  ; First store makes reload.buf writable in the current memory view.
+  %tail = getelementptr [4 x i8], ptr @reload.buf, i32 0, i32 3
+  store i8 9, ptr %tail
+  ; Partial load whose result may share the underlying buffer.
+  %val = load i16, ptr @reload.buf
+  ; Subsequent in-place partial store; this must not corrupt %val.
+  store i8 99, ptr @reload.buf
+  ; Write the originally-loaded value to a separate global.
+  store i16 %val, ptr @reload.out
+  ret void
+}

interp/testdata/store.out.ll

@@ -0,0 +1,13 @@
+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"
+@reload.buf = local_unnamed_addr global [4 x i8] c"c\02\03\09"
+@reload.out = local_unnamed_addr global [2 x i8] c"\01\02"
+
+define void @runtime.initAll() unnamed_addr {
+entry:
+  ret void
+}