From ccb803e35d8377264831f4be7b11e34cf855a741 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Fri, 24 Jul 2020 00:30:02 +0200 Subject: [PATCH] interp: replace some panics with error messages A number of functions now return errors instead of panicking, which should help greatly when investigating interp errors. It at least shows the package responsible for it. --- interp/frame.go | 38 ++++++++++++++++++++------ interp/utils.go | 15 ++++++---- interp/values.go | 71 ++++++++++++++++++++++++++++++++---------------- 3 files changed, 87 insertions(+), 37 deletions(-) diff --git a/interp/frame.go b/interp/frame.go index ed7c3f98..28e61095 100644 --- a/interp/frame.go +++ b/interp/frame.go @@ -95,7 +95,11 @@ func (fr *frame) evalBasicBlock(bb, incoming llvm.BasicBlock, indent string) (re if !operand.IsConstant() || inst.IsVolatile() || (!operand.Underlying.IsAConstantExpr().IsNil() && operand.Underlying.Opcode() == llvm.BitCast) { value = fr.builder.CreateLoad(operand.Value(), inst.Name()) } else { - value = operand.Load() + var err error + value, err = operand.Load() + if err != nil { + return nil, nil, fr.errorAt(inst, err) + } } if value.Type() != inst.Type() { return nil, nil, fr.errorAt(inst, errors.New("interp: load: type does not match")) @@ -308,7 +312,10 @@ func (fr *frame) evalBasicBlock(bb, incoming llvm.BasicBlock, indent string) (re } // "key" is a Go string value, which in the TinyGo calling convention is split up // into separate pointer and length parameters. - m.PutString(keyBuf, keyLen, valPtr) + err := m.PutString(keyBuf, keyLen, valPtr) + if err != nil { + return nil, nil, fr.errorAt(inst, err) + } case callee.Name() == "runtime.hashmapBinarySet": // set a binary (int etc.) key in the map keyBuf := fr.getLocal(inst.Operand(1)).(*LocalValue) @@ -329,15 +336,24 @@ func (fr *frame) evalBasicBlock(bb, incoming llvm.BasicBlock, indent string) (re fr.builder.CreateCall(callee, llvmParams, "") continue } - m.PutBinary(keyBuf, valPtr) + err := m.PutBinary(keyBuf, valPtr) + if err != nil { + return nil, nil, fr.errorAt(inst, err) + } case callee.Name() == "runtime.stringConcat": // adding two strings together buf1Ptr := fr.getLocal(inst.Operand(0)) buf1Len := fr.getLocal(inst.Operand(1)) buf2Ptr := fr.getLocal(inst.Operand(2)) buf2Len := fr.getLocal(inst.Operand(3)) - buf1 := getStringBytes(buf1Ptr, buf1Len.Value()) - buf2 := getStringBytes(buf2Ptr, buf2Len.Value()) + buf1, err := getStringBytes(buf1Ptr, buf1Len.Value()) + if err != nil { + return nil, nil, fr.errorAt(inst, err) + } + buf2, err := getStringBytes(buf2Ptr, buf2Len.Value()) + if err != nil { + return nil, nil, fr.errorAt(inst, err) + } result := []byte(string(buf1) + string(buf2)) vals := make([]llvm.Value, len(result)) for i := range vals { @@ -401,9 +417,12 @@ func (fr *frame) evalBasicBlock(bb, incoming llvm.BasicBlock, indent string) (re return nil, nil, fr.errorAt(inst, errors.New("interp: trying to copy a slice with negative length?")) } for i := int64(0); i < length; i++ { - var err error // *dst = *src - dstArray.Store(srcArray.Load()) + val, err := srcArray.Load() + if err != nil { + return nil, nil, fr.errorAt(inst, err) + } + dstArray.Store(val) // dst++ dstArrayValue, err := dstArray.GetElementPtr([]uint32{1}) if err != nil { @@ -421,7 +440,10 @@ func (fr *frame) evalBasicBlock(bb, incoming llvm.BasicBlock, indent string) (re // convert a string to a []byte bufPtr := fr.getLocal(inst.Operand(0)) bufLen := fr.getLocal(inst.Operand(1)) - result := getStringBytes(bufPtr, bufLen.Value()) + result, err := getStringBytes(bufPtr, bufLen.Value()) + if err != nil { + return nil, nil, fr.errorAt(inst, err) + } vals := make([]llvm.Value, len(result)) for i := range vals { vals[i] = llvm.ConstInt(fr.Mod.Context().Int8Type(), uint64(result[i]), false) diff --git a/interp/utils.go b/interp/utils.go index 018da8f1..c1154771 100644 --- a/interp/utils.go +++ b/interp/utils.go @@ -1,6 +1,8 @@ package interp import ( + "errors" + "tinygo.org/x/go-llvm" ) @@ -18,20 +20,23 @@ func getUses(value llvm.Value) []llvm.Value { // getStringBytes loads the byte slice of a Go string represented as a // {ptr, len} pair. -func getStringBytes(strPtr Value, strLen llvm.Value) []byte { +func getStringBytes(strPtr Value, strLen llvm.Value) ([]byte, error) { if !strLen.IsConstant() { - panic("getStringBytes with a non-constant length") + return nil, errors.New("getStringBytes with a non-constant length") } buf := make([]byte, strLen.ZExtValue()) for i := range buf { gep, err := strPtr.GetElementPtr([]uint32{uint32(i)}) if err != nil { - panic(err) // TODO + return nil, err + } + c, err := gep.Load() + if err != nil { + return nil, err } - c := gep.Load() buf[i] = byte(c.ZExtValue()) } - return buf + return buf, nil } // getLLVMIndices converts an []uint32 into an []llvm.Value, for use in diff --git a/interp/values.go b/interp/values.go index a6e48c35..dba81fce 100644 --- a/interp/values.go +++ b/interp/values.go @@ -15,7 +15,7 @@ type Value interface { Value() llvm.Value // returns a LLVM value Type() llvm.Type // equal to Value().Type() IsConstant() bool // returns true if this value is a constant value - Load() llvm.Value // dereference a pointer + Load() (llvm.Value, error) // dereference a pointer Store(llvm.Value) // store to a pointer GetElementPtr([]uint32) (Value, error) // returns an interior pointer String() string // string representation, for debugging @@ -44,23 +44,26 @@ func (v *LocalValue) IsConstant() bool { } // Load loads a constant value if this is a constant pointer. -func (v *LocalValue) Load() llvm.Value { +func (v *LocalValue) Load() (llvm.Value, error) { if !v.Underlying.IsAGlobalVariable().IsNil() { - return v.Underlying.Initializer() + return v.Underlying.Initializer(), nil } switch v.Underlying.Opcode() { case llvm.GetElementPtr: indices := v.getConstGEPIndices() if indices[0] != 0 { - panic("invalid GEP") + return llvm.Value{}, errors.New("invalid GEP") } global := v.Eval.getValue(v.Underlying.Operand(0)) - agg := global.Load() - return llvm.ConstExtractValue(agg, indices[1:]) + agg, err := global.Load() + if err != nil { + return llvm.Value{}, err + } + return llvm.ConstExtractValue(agg, indices[1:]), nil case llvm.BitCast: - panic("interp: load from a bitcast") + return llvm.Value{}, errors.New("interp: load from a bitcast") default: - panic("interp: load from a constant") + return llvm.Value{}, errors.New("interp: load from a constant") } } @@ -88,7 +91,10 @@ func (v *LocalValue) Store(value llvm.Value) { panic("invalid GEP") } global := &LocalValue{v.Eval, v.Underlying.Operand(0)} - agg := global.Load() + agg, err := global.Load() + if err != nil { + panic(err) // TODO + } agg = llvm.ConstInsertValue(agg, value, indices[1:]) global.Store(agg) return @@ -225,7 +231,11 @@ func (v *MapValue) Value() llvm.Value { keyPtr := llvm.ConstExtractValue(llvmKey, []uint32{0}) keyLen := llvm.ConstExtractValue(llvmKey, []uint32{1}) keyPtrVal := v.Eval.getValue(keyPtr) - keyBuf = getStringBytes(keyPtrVal, keyLen) + var err error + keyBuf, err = getStringBytes(keyPtrVal, keyLen) + if err != nil { + panic(err) // TODO + } } else if key.Type().TypeKind() == llvm.IntegerTypeKind { keyBuf = make([]byte, v.Eval.TargetData.TypeAllocSize(key.Type())) n := key.Value().ZExtValue() @@ -299,7 +309,7 @@ func (v *MapValue) IsConstant() bool { } // Load panics: maps are of reference type so cannot be dereferenced. -func (v *MapValue) Load() llvm.Value { +func (v *MapValue) Load() (llvm.Value, error) { panic("interp: load from a map") } @@ -316,23 +326,26 @@ func (v *MapValue) GetElementPtr(indices []uint32) (Value, error) { // PutString does a map assign operation, assuming that the map is of type // map[string]T. -func (v *MapValue) PutString(keyBuf, keyLen, valPtr *LocalValue) { +func (v *MapValue) PutString(keyBuf, keyLen, valPtr *LocalValue) error { if !v.Underlying.IsNil() { - panic("map already created") + return errors.New("map already created") } if valPtr.Underlying.Opcode() == llvm.BitCast { valPtr = &LocalValue{v.Eval, valPtr.Underlying.Operand(0)} } - value := valPtr.Load() + value, err := valPtr.Load() + if err != nil { + return err + } if v.ValueType.IsNil() { v.ValueType = value.Type() if int(v.Eval.TargetData.TypeAllocSize(v.ValueType)) != v.ValueSize { - panic("interp: map store value type has the wrong size") + return errors.New("interp: map store value type has the wrong size") } } else { if value.Type() != v.ValueType { - panic("interp: map store value type is inconsistent") + return errors.New("interp: map store value type is inconsistent") } } @@ -345,26 +358,31 @@ func (v *MapValue) PutString(keyBuf, keyLen, valPtr *LocalValue) { // TODO: avoid duplicate keys v.Keys = append(v.Keys, &LocalValue{v.Eval, key}) v.Values = append(v.Values, &LocalValue{v.Eval, value}) + + return nil } // PutBinary does a map assign operation. -func (v *MapValue) PutBinary(keyPtr, valPtr *LocalValue) { +func (v *MapValue) PutBinary(keyPtr, valPtr *LocalValue) error { if !v.Underlying.IsNil() { - panic("map already created") + return errors.New("map already created") } if valPtr.Underlying.Opcode() == llvm.BitCast { valPtr = &LocalValue{v.Eval, valPtr.Underlying.Operand(0)} } - value := valPtr.Load() + value, err := valPtr.Load() + if err != nil { + return err + } if v.ValueType.IsNil() { v.ValueType = value.Type() if int(v.Eval.TargetData.TypeAllocSize(v.ValueType)) != v.ValueSize { - panic("interp: map store value type has the wrong size") + return errors.New("interp: map store value type has the wrong size") } } else { if value.Type() != v.ValueType { - panic("interp: map store value type is inconsistent") + return errors.New("interp: map store value type is inconsistent") } } @@ -375,21 +393,26 @@ func (v *MapValue) PutBinary(keyPtr, valPtr *LocalValue) { keyPtr = &LocalValue{v.Eval, keyPtr.Underlying.Operand(0)} } } - key := keyPtr.Load() + key, err := keyPtr.Load() + if err != nil { + return err + } if v.KeyType.IsNil() { v.KeyType = key.Type() if int(v.Eval.TargetData.TypeAllocSize(v.KeyType)) != v.KeySize { - panic("interp: map store key type has the wrong size") + return errors.New("interp: map store key type has the wrong size") } } else { if key.Type() != v.KeyType { - panic("interp: map store key type is inconsistent") + return errors.New("interp: map store key type is inconsistent") } } // TODO: avoid duplicate keys v.Keys = append(v.Keys, &LocalValue{v.Eval, key}) v.Values = append(v.Values, &LocalValue{v.Eval, value}) + + return nil } // Get FNV-1a hash of this string.