Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions interp/interp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ func TestInterp(t *testing.T) {
"slicedata",
"aggregate",
"fastrand",
"ptrtoint",
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
Expand Down
27 changes: 27 additions & 0 deletions interp/interpreter.go
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,29 @@ func (r *runner) run(fn *function, params []value, parentMem *memoryView, indent
// while changing the LLVM type.
// Because interp doesn't preserve the type, these operations are
// identity operations.
if inst.opcode != llvm.BitCast {
bits := int(r.pointerSize * 8)
if inst.opcode == llvm.PtrToInt {
bits = inst.llvmInst.Type().IntTypeWidth()
}
Comment on lines +671 to +675

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Constant expressions now truncate or zero-extend too. If a real pointer would need resizing, the value falls back to runtime, and a global with such an initializer is treated as external. Added test cases for both.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dkegel-fastly the requested changes have been made.

if int(operands[0].len(r)*8) != bits {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still misses same-width casts of real pointers. On a 64-bit target, converting a global pointer to uintptr and comparing it with a number fails in pointerValue.Uint, because the cast stays in the interpreter instead of running at runtime.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jakebailey. I tried this with ptrtoint ptr @global to i64 followed by icmp ugt against a constant on x86_64. The result is the same on dev and on this PR. pointerValue.Uint sets a recoverable error, so interp gives up on that package init and runs it at runtime. It does not crash, but the whole init loses compile time evaluation.

The same width case is left as an identity on purpose. Keeping the pointer lets interp handle unsafe.Pointer(uintptr(p) + off), the strings.noescape xor trick, and pointer tagging with &. Sending every same width ptrtoint of a real pointer to runtime would push those to runtime too.

A narrower fix is to have icmp run at runtime when an ordered compare gets a pointer operand. That is not related to the size bug, so how about doing that in a separate PR?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine, however you want it; glad I was wrong

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #5748 for the icmp fix.

// ptrtoint and inttoptr truncate or zero-extend when sizes differ.
// https://llvm.org/docs/LangRef.html#ptrtoint-to-instruction
_, isPtr := operands[0].(pointerValue)
if raw, ok := operands[0].(rawValue); ok && raw.hasPointer() {
isPtr = true
}
if isPtr || (bits != 8 && bits != 16 && bits != 32 && bits != 64) {
err := r.runAtRuntime(fn, inst, locals, &mem, indent)
if err != nil {
return nil, mem, err
}
continue
}
locals[inst.localIndex] = makeLiteralInt(operands[0].Uint(r), bits)
continue
}
}
if r.debug {
fmt.Fprintln(os.Stderr, indent+instructionNameMap[inst.opcode]+":", operands[0])
}
Expand Down Expand Up @@ -976,6 +999,10 @@ func (r *runner) runAtRuntime(fn *function, inst instruction, locals []value, me
}
case llvm.BitCast:
result = r.builder.CreateBitCast(operands[0], inst.llvmInst.Type(), inst.name)
case llvm.PtrToInt:
result = r.builder.CreatePtrToInt(operands[0], inst.llvmInst.Type(), inst.name)
case llvm.IntToPtr:
result = r.builder.CreateIntToPtr(operands[0], inst.llvmInst.Type(), inst.name)
case llvm.ExtractValue:
indices := inst.llvmInst.Indices()
// Note: the Go LLVM API doesn't support multiple indices, so simulate
Expand Down
55 changes: 41 additions & 14 deletions interp/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -1004,10 +1004,11 @@ func (v rawValue) toLLVMValue(llvmType llvm.Type, mem *memoryView) (llvm.Value,
}
}

func (v *rawValue) set(llvmValue llvm.Value, r *runner) {
// set returns false if the constant can only be computed at runtime.
func (v *rawValue) set(llvmValue llvm.Value, r *runner) bool {
if llvmValue.IsNull() {
// A zero value is common so check that first.
return
return true
}
if !llvmValue.IsAGlobalValue().IsNil() {
ptrSize := r.pointerSize
Expand All @@ -1020,11 +1021,25 @@ func (v *rawValue) set(llvmValue llvm.Value, r *runner) {
}
} else if !llvmValue.IsAConstantExpr().IsNil() {
switch llvmValue.Opcode() {
case llvm.IntToPtr, llvm.PtrToInt, llvm.BitCast:
// All these instructions effectively just reinterprets the bits
// (like a bitcast) while no bits change and keeping the same
// length, so just read its contents.
v.set(llvmValue.Operand(0), r)
case llvm.BitCast:
return v.set(llvmValue.Operand(0), r)
case llvm.IntToPtr, llvm.PtrToInt:
// These truncate or zero-extend when sizes differ.
// https://llvm.org/docs/LangRef.html#ptrtoint-to-instruction
operand := llvmValue.Operand(0)
src := newRawValue(uint32(r.targetData.TypeAllocSize(operand.Type())))
if !src.set(operand, r) {
return false
}
size := uint32(r.targetData.TypeAllocSize(llvmValue.Type()))
if src.len(r) != size && src.hasPointer() {
return false
}
if llvmValue.Type().TypeKind() == llvm.IntegerTypeKind && uint32(llvmValue.Type().IntTypeWidth()) != size*8 {
// Widths like i1 don't fill their bytes, so leave them for runtime.
return false
}
copy(v.buf[:size], src.buf)
case llvm.GetElementPtr:
ptr := llvmValue.Operand(0)
index := llvmValue.Operand(1)
Expand Down Expand Up @@ -1065,8 +1080,9 @@ func (v *rawValue) set(llvmValue llvm.Value, r *runner) {
size := r.targetData.TypeAllocSize(llvmValue.Operand(0).Type())
lhs := newRawValue(uint32(size))
rhs := newRawValue(uint32(size))
lhs.set(llvmValue.Operand(0), r)
rhs.set(llvmValue.Operand(1), r)
if !lhs.set(llvmValue.Operand(0), r) || !rhs.set(llvmValue.Operand(1), r) {
return false
}
if r.interpretICmp(lhs, rhs, llvmValue.IntPredicate()) {
v.buf[0] = 1 // result is true
} else {
Expand Down Expand Up @@ -1118,7 +1134,9 @@ func (v *rawValue) set(llvmValue llvm.Value, r *runner) {
field := rawValue{
buf: v.buf[offset:],
}
field.set(r.builder.CreateExtractValue(llvmValue, i, ""), r)
if !field.set(r.builder.CreateExtractValue(llvmValue, i, ""), r) {
return false
}
}
case llvm.ArrayTypeKind:
numElements := llvmType.ArrayLength()
Expand All @@ -1129,7 +1147,9 @@ func (v *rawValue) set(llvmValue llvm.Value, r *runner) {
field := rawValue{
buf: v.buf[offset:],
}
field.set(r.builder.CreateExtractValue(llvmValue, i, ""), r)
if !field.set(r.builder.CreateExtractValue(llvmValue, i, ""), r) {
return false
}
}
case llvm.DoubleTypeKind:
f, _ := llvmValue.DoubleValue()
Expand All @@ -1151,6 +1171,7 @@ func (v *rawValue) set(llvmValue llvm.Value, r *runner) {
panic("unknown constant")
}
}
return true
}

// hasPointer returns true if this raw value contains a pointer somewhere in the
Expand Down Expand Up @@ -1219,8 +1240,12 @@ func (r *runner) getValue(llvmValue llvm.Value) value {
if !llvmValue.IsAGlobalVariable().IsNil() {
obj.size = uint32(r.targetData.TypeAllocSize(llvmValue.GlobalValueType()))
if initializer := llvmValue.Initializer(); !initializer.IsNil() {
obj.buffer = r.getValue(initializer)
obj.constant = llvmValue.IsGlobalConstant()
// Treat the global as external if its initializer is only known at runtime.
buf := r.getValue(initializer)
if _, ok := buf.(localValue); !ok {
obj.buffer = buf
obj.constant = llvmValue.IsGlobalConstant()
}
}
} else if !llvmValue.IsAFunction().IsNil() {
// OK
Expand Down Expand Up @@ -1251,7 +1276,9 @@ func (r *runner) getValue(llvmValue llvm.Value) value {
}
size := r.targetData.TypeAllocSize(llvmValue.Type())
v := newRawValue(uint32(size))
v.set(llvmValue, r)
if !v.set(llvmValue, r) {
return localValue{llvmValue}
}
return v
} else if !llvmValue.IsAInstruction().IsNil() || !llvmValue.IsAArgument().IsNil() {
return localValue{llvmValue}
Expand Down
65 changes: 65 additions & 0 deletions interp/testdata/ptrtoint.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64--linux"

@main.v1 = global i32 0
@main.v2 = global i64 0
@main.v3 = global i8 0
@main.global = global i8 0
@main.v4 = global i8 0
@main.v5 = global i8 0
@main.v6 = global { i8, i8 } { i8 ptrtoint (ptr @main.global to i8), i8 7 }
@main.v7 = global i8 0
@main.v8 = global i1 0

define void @runtime.initAll() unnamed_addr {
entry:
call void @main.init()
ret void
}

define internal void @main.init() unnamed_addr {
entry:
; Narrow the receiver from a pointer, like an interface method thunk.
%v1 = call i32 @main.narrow.id.unpack(ptr inttoptr (i8 1 to ptr))
store i32 %v1, ptr @main.v1

; Widen a narrow integer through a pointer.
%ptr = inttoptr i16 -1 to ptr
%v2 = ptrtoint ptr %ptr to i64
store i64 %v2, ptr @main.v2

; A real pointer can't be truncated at compile time.
%v3 = ptrtoint ptr @main.global to i8
store i8 %v3, ptr @main.v3

; The same conversions as constant expressions.
store i8 ptrtoint (ptr inttoptr (i64 258 to ptr) to i8), ptr @main.v4
store i8 ptrtoint (ptr @main.global to i8), ptr @main.v5

; A global whose initializer can't be computed at compile time.
%v7 = load i8, ptr getelementptr inbounds (i8, ptr @main.v6, i64 1)
store i8 %v7, ptr @main.v7

; A result narrower than its allocation size is left for runtime.
store i1 ptrtoint (ptr inttoptr (i64 2 to ptr) to i1), ptr @main.v8
ret void
}

define internal i32 @main.narrow.id.unpack(ptr %receiver) unnamed_addr {
entry:
%unpack.int = ptrtoint ptr %receiver to i8
%ret = call i32 @main.narrow.id(i8 %unpack.int)
ret i32 %ret
}

define internal i32 @main.narrow.id(i8 %h) unnamed_addr {
entry:
%cmp = icmp eq i8 %h, 1
br i1 %cmp, label %if.then, label %if.done

if.then:
ret i32 10

if.done:
ret i32 0
}
22 changes: 22 additions & 0 deletions interp/testdata/ptrtoint.out.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64--linux"

@main.v1 = local_unnamed_addr global i32 10
@main.v2 = local_unnamed_addr global i64 65535
@main.v3 = local_unnamed_addr global i8 0
@main.global = global i8 0
@main.v4 = local_unnamed_addr global i8 2
@main.v5 = local_unnamed_addr global i8 0
@main.v6 = local_unnamed_addr global { i8, i8 } { i8 ptrtoint (ptr @main.global to i8), i8 7 }
@main.v7 = local_unnamed_addr global i8 0
@main.v8 = local_unnamed_addr global i1 false

define void @runtime.initAll() unnamed_addr {
entry:
store i8 ptrtoint (ptr @main.global to i8), ptr @main.v3, align 1
store i8 ptrtoint (ptr @main.global to i8), ptr @main.v5, align 1
%v7 = load i8, ptr getelementptr inbounds (i8, ptr @main.v6, i64 1), align 1
store i8 %v7, ptr @main.v7, align 1
store i1 ptrtoint (ptr inttoptr (i64 2 to ptr) to i1), ptr @main.v8, align 1
ret void
}
Loading