From 304900e765a99e3034855900703c8e6c3b4bb9e2 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Thu, 14 Apr 2022 13:33:42 +0300 Subject: [PATCH] compiler: improve debugging experience Currently one instruction can correspond to multiple sequence points because of inlining. This leads to a bad user experience as only the last one is used. In this commit we create a sequence point for each inlined call and also make sure that each time a new sequence point is created the corresponding opcode can easily be seen in code. The NOPs increase contract size, but not to a large degree. Other solutions considered: 1. Discard NOPs if a special flag is provided. Still leads to bad debugging experience if deployed contract differs from the debugged one. 2. Create an issue for a debugger. When multiple sequence points are provided for a single instruction they can be used to emulate non-inline behaviour with pseudo-NOPs. I believe this is what windows debugger does (the last paragraph https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugging-optimized-code-and-inline-functions-external ) 3. Emit debug info for inlined functions even without creating a function in the NEF itself. This should be done for each called instance and would also create overlapping opcode ranges for the enclosing function. However this approach can also ensure consistent values view for inlined function parameters. Signed-off-by: Evgeniy Stratonikov --- pkg/compiler/codegen.go | 3 +-- pkg/compiler/inline.go | 5 +++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/compiler/codegen.go b/pkg/compiler/codegen.go index 88655b188..252f31e22 100644 --- a/pkg/compiler/codegen.go +++ b/pkg/compiler/codegen.go @@ -593,7 +593,6 @@ func (c *codegen) Visit(node ast.Node) ast.Visitor { case *ast.AssignStmt: multiRet := len(n.Rhs) != len(n.Lhs) - c.saveSequencePoint(n) // Assign operations are grouped https://github.com/golang/go/blob/master/src/go/types/stmt.go#L160 isAssignOp := token.ADD_ASSIGN <= n.Tok && n.Tok <= token.AND_NOT_ASSIGN if isAssignOp { @@ -708,8 +707,8 @@ func (c *codegen) Visit(node ast.Node) ast.Visitor { c.processDefers() - c.saveSequencePoint(n) if len(c.pkgInfoInline) == 0 { + c.saveSequencePoint(n) emit.Opcodes(c.prog.BinWriter, opcode.RET) } return nil diff --git a/pkg/compiler/inline.go b/pkg/compiler/inline.go index 74e33c190..3f0881c00 100644 --- a/pkg/compiler/inline.go +++ b/pkg/compiler/inline.go @@ -21,6 +21,11 @@ import ( // // } func (c *codegen) inlineCall(f *funcScope, n *ast.CallExpr) { + // Save sequence point for the debugger. Not having NOP can result in + // one instruction being used by multiple sequence points. + c.saveSequencePoint(n) + emit.Opcodes(c.prog.BinWriter, opcode.NOP) + labelSz := len(c.labelList) offSz := len(c.inlineLabelOffsets) c.inlineLabelOffsets = append(c.inlineLabelOffsets, labelSz)