Skip to content

Commit

Permalink
interp: don't copy unknown values in runtime.sliceCopy
Browse files Browse the repository at this point in the history
This was bug #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.
  • Loading branch information
aykevl committed Sep 9, 2023
1 parent e9d8a9b commit a1e0bea
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 0 deletions.
14 changes: 14 additions & 0 deletions interp/interpreter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
23 changes: 23 additions & 0 deletions interp/testdata/slice-copy.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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()
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
12 changes: 12 additions & 0 deletions interp/testdata/slice-copy.out.ll
Original file line number Diff line number Diff line change
@@ -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
}

Expand All @@ -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
}

0 comments on commit a1e0bea

Please sign in to comment.