From a1e0beac22985c3e214f403aceac75317ac73997 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Sat, 9 Sep 2023 16:38:30 +0200 Subject: [PATCH] interp: don't copy unknown values in runtime.sliceCopy This was bug https://github.com/tinygo-org/tinygo/issues/3890. See the diff for details. Essentially, it means that `copy` in the interpreter was copying data that wasn't known yet, or copying data into a slice that could be read externally after the interp pass. --- interp/interpreter.go | 14 ++++++++++++++ interp/testdata/slice-copy.ll | 23 +++++++++++++++++++++++ interp/testdata/slice-copy.out.ll | 12 ++++++++++++ 3 files changed, 49 insertions(+) diff --git a/interp/interpreter.go b/interp/interpreter.go index c48b9b5fef..8e5faf642d 100644 --- a/interp/interpreter.go +++ b/interp/interpreter.go @@ -338,6 +338,20 @@ func (r *runner) run(fn *function, params []value, parentMem *memoryView, indent if err != nil { return nil, mem, r.errorAt(inst, err) } + if mem.hasExternalStore(src) || mem.hasExternalLoadOrStore(dst) { + // These are the same checks as there are on llvm.Load + // and llvm.Store in the interpreter. Copying is + // essentially loading from the source array and storing + // to the destination array, hence why we need to do the + // same checks here. + // This fixes the following bug: + // https://github.com/tinygo-org/tinygo/issues/3890 + err := r.runAtRuntime(fn, inst, locals, &mem, indent) + if err != nil { + return nil, mem, err + } + continue + } nBytes := uint32(n * elemSize) dstObj := mem.getWritable(dst.index()) dstBuf := dstObj.buffer.asRawValue(r) diff --git a/interp/testdata/slice-copy.ll b/interp/testdata/slice-copy.ll index 6776d2d6fd..5d741358a2 100644 --- a/interp/testdata/slice-copy.ll +++ b/interp/testdata/slice-copy.ll @@ -7,6 +7,10 @@ target triple = "x86_64--linux" @main.int16SliceSrc.buf = internal global [3 x i16] [i16 5, i16 123, i16 1024] @main.int16SliceSrc = internal unnamed_addr global { ptr, i64, i64 } { ptr @main.int16SliceSrc.buf, i64 3, i64 3 } @main.int16SliceDst = internal unnamed_addr global { ptr, i64, i64 } zeroinitializer +@main.sliceSrcUntaint.buf = internal global [2 x i8] c"ab" +@main.sliceDstUntaint.buf = internal global [2 x i8] zeroinitializer +@main.sliceSrcTaint.buf = internal global [2 x i8] c"cd" +@main.sliceDstTaint.buf = internal global [2 x i8] zeroinitializer declare i64 @runtime.sliceCopy(ptr %dst, ptr %src, i64 %dstLen, i64 %srcLen, i64 %elemSize) unnamed_addr @@ -16,6 +20,8 @@ declare void @runtime.printuint8(i8) declare void @runtime.printint16(i16) +declare void @use(ptr) + define void @runtime.initAll() unnamed_addr { entry: call void @main.init() @@ -43,6 +49,15 @@ entry: %int16SliceDst.buf = load ptr, ptr @main.int16SliceDst %int16SliceDst.val = load i16, ptr %int16SliceDst.buf call void @runtime.printint16(i16 %int16SliceDst.val) + + ; print(sliceDstUntaint[0]) + %sliceDstUntaint.val = load i8, ptr getelementptr inbounds (i8, ptr @main.sliceDstUntaint.buf, i32 0) + call void @runtime.printuint8(i8 %sliceDstUntaint.val) + + ; print(sliceDstTaint[0]) + %sliceDstTaint.val = load i8, ptr getelementptr inbounds (i8, ptr @main.sliceDstTaint.buf, i32 0) + call void @runtime.printuint8(i8 %sliceDstTaint.val) + ret void } @@ -79,5 +94,13 @@ entry: %int16SliceSrc.buf = extractvalue { ptr, i64, i64 } %int16SliceSrc, 0 %copy.n2 = call i64 @runtime.sliceCopy(ptr %int16SliceDst.buf, ptr %int16SliceSrc.buf, i64 %int16SliceSrc.len, i64 %int16SliceSrc.len, i64 2) + ; Copy slice that has a known value. + %copy.n3 = call i64 @runtime.sliceCopy(ptr @main.sliceDstUntaint.buf, ptr @main.sliceSrcUntaint.buf, i64 2, i64 2, i64 1) + + ; Copy slice that might have been modified by the external @use call. + ; This is a fix for https://github.com/tinygo-org/tinygo/issues/3890. + call void @use(ptr @main.sliceSrcTaint.buf) + %copy.n4 = call i64 @runtime.sliceCopy(ptr @main.sliceDstTaint.buf, ptr @main.sliceSrcTaint.buf, i64 2, i64 2, i64 1) + ret void } diff --git a/interp/testdata/slice-copy.out.ll b/interp/testdata/slice-copy.out.ll index 2817564333..d61619e13f 100644 --- a/interp/testdata/slice-copy.out.ll +++ b/interp/testdata/slice-copy.out.ll @@ -1,12 +1,21 @@ target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64--linux" +@main.sliceSrcTaint.buf = internal global [2 x i8] c"cd" +@main.sliceDstTaint.buf = internal global [2 x i8] zeroinitializer + +declare i64 @runtime.sliceCopy(ptr, ptr, i64, i64, i64) unnamed_addr + declare void @runtime.printuint8(i8) local_unnamed_addr declare void @runtime.printint16(i16) local_unnamed_addr +declare void @use(ptr) local_unnamed_addr + define void @runtime.initAll() unnamed_addr { entry: + call void @use(ptr @main.sliceSrcTaint.buf) + %copy.n4 = call i64 @runtime.sliceCopy(ptr @main.sliceDstTaint.buf, ptr @main.sliceSrcTaint.buf, i64 2, i64 2, i64 1) ret void } @@ -16,5 +25,8 @@ entry: call void @runtime.printuint8(i8 3) call void @runtime.printint16(i16 5) call void @runtime.printint16(i16 5) + call void @runtime.printuint8(i8 97) + %sliceDstTaint.val = load i8, ptr @main.sliceDstTaint.buf, align 1 + call void @runtime.printuint8(i8 %sliceDstTaint.val) ret void }