interp: improve error handling of markExternal* functions

Author: aykevl

interp/interp.go

@@ -121,7 +121,10 @@ func Run(mod llvm.Module, debug bool) error {
 				r.builder.CreateCall(fn, []llvm.Value{i8undef}, "")
 				// Make sure that any globals touched by the package
 				// initializer, won't be accessed by later package initializers.
-				r.markExternalLoad(fn)
+				err := r.markExternalLoad(fn)
+				if err != nil {
+					return fmt.Errorf("failed to interpret package %s: %w", r.pkgName, err)
+				}
 				continue
 			}
 			return callErr
@@ -288,12 +291,16 @@ func (r *runner) getFunction(llvmFn llvm.Value) *function {
 // variable. Another package initializer might read from the same global
 // variable. By marking this function as being run at runtime, that load
 // instruction will need to be run at runtime instead of at compile time.
-func (r *runner) markExternalLoad(llvmValue llvm.Value) {
+func (r *runner) markExternalLoad(llvmValue llvm.Value) error {
 	mem := memoryView{r: r}
-	mem.markExternalLoad(llvmValue)
+	err := mem.markExternalLoad(llvmValue)
+	if err != nil {
+		return err
+	}
 	for index, obj := range mem.objects {
 		if obj.marked > r.objects[index].marked {
 			r.objects[index].marked = obj.marked
 		}
 	}
+	return nil
 }

interp/interpreter.go

@@ -945,12 +945,18 @@ func (r *runner) runAtRuntime(fn *function, inst instruction, locals []value, me
 		args := operands[:len(operands)-1]
 		for _, arg := range args {
 			if arg.Type().TypeKind() == llvm.PointerTypeKind {
-				mem.markExternalStore(arg)
+				err := mem.markExternalStore(arg)
+				if err != nil {
+					return r.errorAt(inst, err)
+				}
 			}
 		}
 		result = r.builder.CreateCall(llvmFn, args, inst.name)
 	case llvm.Load:
-		mem.markExternalLoad(operands[0])
+		err := mem.markExternalLoad(operands[0])
+		if err != nil {
+			return r.errorAt(inst, err)
+		}
 		result = r.builder.CreateLoad(operands[0], inst.name)
 		if inst.llvmInst.IsVolatile() {
 			result.SetVolatile(true)
@@ -959,7 +965,10 @@ func (r *runner) runAtRuntime(fn *function, inst instruction, locals []value, me
 			result.SetOrdering(ordering)
 		}
 	case llvm.Store:
-		mem.markExternalStore(operands[1])
+		err := mem.markExternalStore(operands[1])
+		if err != nil {
+			return r.errorAt(inst, err)
+		}
 		result = r.builder.CreateStore(operands[0], operands[1])
 		if inst.llvmInst.IsVolatile() {
 			result.SetVolatile(true)

interp/memory.go

@@ -17,6 +17,7 @@ package interp
 import (
 	"encoding/binary"
 	"errors"
+	"fmt"
 	"math"
 	"math/big"
 	"strconv"
@@ -104,28 +105,28 @@ func (mv *memoryView) revert() {
 // means that the interpreter can still read from it, but cannot write to it as
 // that would mean the external read (done at runtime) reads from a state that
 // would not exist had the whole initialization been done at runtime.
-func (mv *memoryView) markExternalLoad(llvmValue llvm.Value) {
-	mv.markExternal(llvmValue, 1)
+func (mv *memoryView) markExternalLoad(llvmValue llvm.Value) error {
+	return mv.markExternal(llvmValue, 1)
 }
 
 // markExternalStore marks the given LLVM value as having an external write.
 // This means that the interpreter can no longer read from it or write to it, as
 // that would happen in a different order than if all initialization were
 // happening at runtime.
-func (mv *memoryView) markExternalStore(llvmValue llvm.Value) {
-	mv.markExternal(llvmValue, 2)
+func (mv *memoryView) markExternalStore(llvmValue llvm.Value) error {
+	return mv.markExternal(llvmValue, 2)
 }
 
 // markExternal is a helper for markExternalLoad and markExternalStore, and
 // should not be called directly.
-func (mv *memoryView) markExternal(llvmValue llvm.Value, mark uint8) {
+func (mv *memoryView) markExternal(llvmValue llvm.Value, mark uint8) error {
 	if llvmValue.IsUndef() || llvmValue.IsNull() {
 		// Null and undef definitely don't contain (valid) pointers.
-		return
+		return nil
 	}
 	if !llvmValue.IsAInstruction().IsNil() || !llvmValue.IsAArgument().IsNil() {
 		// These are considered external by default, there is nothing to mark.
-		return
+		return nil
 	}
 
 	if !llvmValue.IsAGlobalValue().IsNil() {
@@ -144,7 +145,10 @@ func (mv *memoryView) markExternal(llvmValue llvm.Value, mark uint8) {
 					// Using mark '2' (which means read/write access) because
 					// even from an object that is only read from, the resulting
 					// loaded pointer can be written to.
-					mv.markExternal(initializer, 2)
+					err := mv.markExternal(initializer, 2)
+					if err != nil {
+						return err
+					}
 				}
 			} else {
 				// This is a function. Go through all instructions and mark all
@@ -170,7 +174,10 @@ func (mv *memoryView) markExternal(llvmValue llvm.Value, mark uint8) {
 						for i := 0; i < numOperands; i++ {
 							// Using mark '2' (which means read/write access)
 							// because this might be a store instruction.
-							mv.markExternal(inst.Operand(i), 2)
+							err := mv.markExternal(inst.Operand(i), 2)
+							if err != nil {
+								return err
+							}
 						}
 					}
 				}
@@ -179,9 +186,12 @@ func (mv *memoryView) markExternal(llvmValue llvm.Value, mark uint8) {
 	} else if !llvmValue.IsAConstantExpr().IsNil() {
 		switch llvmValue.Opcode() {
 		case llvm.IntToPtr, llvm.PtrToInt, llvm.BitCast, llvm.GetElementPtr:
-			mv.markExternal(llvmValue.Operand(0), mark)
+			err := mv.markExternal(llvmValue.Operand(0), mark)
+			if err != nil {
+				return err
+			}
 		default:
-			panic("interp: unknown constant expression")
+			return fmt.Errorf("interp: unknown constant expression '%s'", instructionNameMap[llvmValue.Opcode()])
 		}
 	} else if !llvmValue.IsAInlineAsm().IsNil() {
 		// Inline assembly can modify globals but only exported globals. Let's
@@ -196,18 +206,25 @@ func (mv *memoryView) markExternal(llvmValue llvm.Value, mark uint8) {
 			numElements := llvmType.StructElementTypesCount()
 			for i := 0; i < numElements; i++ {
 				element := llvm.ConstExtractValue(llvmValue, []uint32{uint32(i)})
-				mv.markExternal(element, mark)
+				err := mv.markExternal(element, mark)
+				if err != nil {
+					return err
+				}
 			}
 		case llvm.ArrayTypeKind:
 			numElements := llvmType.ArrayLength()
 			for i := 0; i < numElements; i++ {
 				element := llvm.ConstExtractValue(llvmValue, []uint32{uint32(i)})
-				mv.markExternal(element, mark)
+				err := mv.markExternal(element, mark)
+				if err != nil {
+					return err
+				}
 			}
 		default:
-			panic("interp: unknown type kind in markExternalValue")
+			return errors.New("interp: unknown type kind in markExternalValue")
 		}
 	}
+	return nil
 }
 
 // hasExternalLoadOrStore returns true if this object has an external load or