From b22221a2b2691ff1d586bcd1bec48135ac16178e Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Tue, 24 Mar 2020 16:58:06 +0100 Subject: [PATCH 1/2] interp: allow loading a value from a bitcast --- interp/values.go | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/interp/values.go b/interp/values.go index ae29bf99d2..6c38f99942 100644 --- a/interp/values.go +++ b/interp/values.go @@ -52,7 +52,8 @@ func (v *LocalValue) Load() (llvm.Value, error) { case llvm.GetElementPtr: indices := v.getConstGEPIndices() if indices[0] != 0 { - return llvm.Value{}, errors.New("invalid GEP") + v.MarkDirty() + return v.Eval.builder.CreateLoad(v.Underlying, ""), nil } global := v.Eval.getValue(v.Underlying.Operand(0)) agg, err := global.Load() @@ -61,9 +62,35 @@ func (v *LocalValue) Load() (llvm.Value, error) { } return llvm.ConstExtractValue(agg, indices[1:]), nil case llvm.BitCast: - return llvm.Value{}, errors.New("interp: load from a bitcast") + // Assuming a pointer bitcast, such as i64* to double*. + // By loading the pre-bitcast value and then bitcasting the loaded value + // we work around the problem. + sourceType := v.Underlying.Operand(0).Type().ElementType() + destType := v.Underlying.Type().ElementType() + if v.Eval.TargetData.TypeAllocSize(sourceType) != v.Eval.TargetData.TypeAllocSize(destType) { + v.MarkDirty() + return v.Eval.builder.CreateLoad(v.Underlying, ""), nil + } + if !isScalar(sourceType) || !isScalar(destType) { + // Actually bitcasts are a bit more flexible than this: any + // non-aggregate first-class type can be casted. + v.MarkDirty() + return v.Eval.builder.CreateLoad(v.Underlying, ""), nil + } + valueBeforeCast := LocalValue{v.Eval, v.Underlying.Operand(0)} + llvmValueBeforeCast, err := valueBeforeCast.Load() + if err != nil { + return llvm.Value{}, err + } + if !llvmValueBeforeCast.IsConstant() { + // Unlikely but check for it anyway. + v.MarkDirty() + return v.Eval.builder.CreateLoad(v.Underlying, ""), nil + } + return llvm.ConstBitCast(llvmValueBeforeCast, destType), nil default: - return llvm.Value{}, errors.New("interp: load from a constant") + v.MarkDirty() + return v.Eval.builder.CreateLoad(v.Underlying, ""), nil } } From b446ca5c9c4130a204c5defaae62a536d7bd4c62 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Tue, 24 Mar 2020 17:10:08 +0100 Subject: [PATCH 2/2] interp: improve soundness of loads and stores Previously, whether a value could be safely loaded from or stored to was based on some heuristics. This commit fixes that by making sure the underlying global is really not part of the dirty set (which must be operated on at runtime instead of compile time). Together with the previous commit, this commit gets a number of standard library packages to compile: * crypto/dsa * crypto/ed25519 * crypto/elliptic * crypto/rand * go/constant * math/big --- interp/frame.go | 6 ++++-- interp/interp.go | 39 ++++++++++++++++++++------------------- interp/utils.go | 31 ++++++++++++++++--------------- interp/values.go | 16 +++++++--------- 4 files changed, 47 insertions(+), 45 deletions(-) diff --git a/interp/frame.go b/interp/frame.go index 16984f93f4..90cee4a194 100644 --- a/interp/frame.go +++ b/interp/frame.go @@ -92,7 +92,8 @@ func (fr *frame) evalBasicBlock(bb, incoming llvm.BasicBlock, indent string) (re case !inst.IsALoadInst().IsNil(): operand := fr.getLocal(inst.Operand(0)).(*LocalValue) var value llvm.Value - if !operand.IsConstant() || inst.IsVolatile() || (!operand.Underlying.IsAConstantExpr().IsNil() && operand.Underlying.Opcode() == llvm.BitCast) { + if inst.IsVolatile() || fr.Eval.isDirty(operand.Value()) { + fr.Eval.markDirty(operand.Value()) value = fr.builder.CreateLoad(operand.Value(), inst.Name()) } else { var err error @@ -108,7 +109,8 @@ func (fr *frame) evalBasicBlock(bb, incoming llvm.BasicBlock, indent string) (re case !inst.IsAStoreInst().IsNil(): value := fr.getLocal(inst.Operand(0)) ptr := fr.getLocal(inst.Operand(1)) - if inst.IsVolatile() { + if inst.IsVolatile() || fr.Eval.isDirty(ptr.Value()) { + fr.Eval.markDirty(ptr.Value()) fr.builder.CreateStore(value.Value(), ptr.Value()) } else { err := ptr.Store(value.Value()) diff --git a/interp/interp.go b/interp/interp.go index d5ff396568..fe09d21b7d 100644 --- a/interp/interp.go +++ b/interp/interp.go @@ -136,26 +136,27 @@ func (e *Eval) getValue(v llvm.Value) Value { // markDirty marks the passed-in LLVM value dirty, recursively. For example, // when it encounters a constant GEP on a global, it marks the global dirty. func (e *Eval) markDirty(v llvm.Value) { - if !v.IsAGlobalVariable().IsNil() { - if v.IsGlobalConstant() { - return - } - if _, ok := e.dirtyGlobals[v]; !ok { - e.dirtyGlobals[v] = struct{}{} - e.sideEffectFuncs = nil // re-calculate all side effects - } - } else if v.IsConstant() { - if v.OperandsCount() >= 2 && !v.Operand(0).IsAGlobalVariable().IsNil() { - // looks like a constant getelementptr of a global. - // TODO: find a way to make sure it really is: v.Opcode() returns 0. - e.markDirty(v.Operand(0)) + if v.Type().TypeKind() != llvm.PointerTypeKind { + return + } + v = unwrap(v) + if v.IsAGlobalVariable().IsNil() { + if !v.IsAUndefValue().IsNil() || !v.IsAConstantPointerNull().IsNil() { + // Nothing to mark here: these definitely don't point to globals. return } - return // nothing to mark - } else if !v.IsAGetElementPtrInst().IsNil() { - panic("interp: todo: GEP") - } else { - // Not constant and not a global or GEP so doesn't have to be marked - // non-constant. + panic("interp: trying to mark a non-global as dirty") + } + e.dirtyGlobals[v] = struct{}{} +} + +// isDirty returns whether the value is tracked as part of the set of dirty +// globals or not. If it's not dirty, it means loads/stores can be done at +// compile time. +func (e *Eval) isDirty(v llvm.Value) bool { + v = unwrap(v) + if _, ok := e.dirtyGlobals[v]; ok { + return true } + return false } diff --git a/interp/utils.go b/interp/utils.go index c115477133..4903e30205 100644 --- a/interp/utils.go +++ b/interp/utils.go @@ -106,21 +106,22 @@ func isZero(v llvm.Value) (result bool, ok bool) { return false, false // not valid } -// unwrap returns the underlying value, with GEPs removed. This can be useful to -// get the underlying global of a GEP pointer. +// unwrap returns the underlying global value, with GEPs and bitcasts removed. +// This can be useful to get the underlying global of a GEP pointer. func unwrap(value llvm.Value) llvm.Value { - for { - if !value.IsAConstantExpr().IsNil() { - switch value.Opcode() { - case llvm.GetElementPtr: - value = value.Operand(0) - continue - } - } else if !value.IsAGetElementPtrInst().IsNil() { - value = value.Operand(0) - continue - } - break + var opcode llvm.Opcode + if !value.IsAConstantExpr().IsNil() { + opcode = value.Opcode() + } else if !value.IsAInstruction().IsNil() { + opcode = value.InstructionOpcode() + } else { + return value + } + switch opcode { + case llvm.BitCast, llvm.GetElementPtr, llvm.PtrToInt: + // Make sure dirty globals are caught through pointer casts. + return unwrap(value.Operand(0)) + default: + return value } - return value } diff --git a/interp/values.go b/interp/values.go index 6c38f99942..493d35361a 100644 --- a/interp/values.go +++ b/interp/values.go @@ -37,7 +37,7 @@ func (v *LocalValue) Type() llvm.Type { } func (v *LocalValue) IsConstant() bool { - if _, ok := v.Eval.dirtyGlobals[unwrap(v.Underlying)]; ok { + if v.Eval.isDirty(v.Underlying) { return false } return v.Underlying.IsConstant() @@ -45,6 +45,11 @@ func (v *LocalValue) IsConstant() bool { // Load loads a constant value if this is a constant pointer. func (v *LocalValue) Load() (llvm.Value, error) { + if v.Eval.isDirty(v.Underlying) { + // This may indicate a bug elsewhere. + v.MarkDirty() + return v.Eval.builder.CreateLoad(v.Underlying, ""), nil + } if !v.Underlying.IsAGlobalVariable().IsNil() { return v.Underlying.Initializer(), nil } @@ -188,14 +193,7 @@ func (v *LocalValue) getConstGEPIndices() []uint32 { // MarkDirty marks this global as dirty, meaning that every load from and store // to this global (from now on) must be performed at runtime. func (v *LocalValue) MarkDirty() { - underlying := unwrap(v.Underlying) - if underlying.IsAGlobalVariable().IsNil() { - panic("trying to mark a non-global as dirty") - } - if !v.IsConstant() { - return // already dirty - } - v.Eval.dirtyGlobals[underlying] = struct{}{} + v.Eval.markDirty(v.Underlying) } // MapValue implements a Go map which is created at compile time and stored as a