Skip to content

Commit

Permalink
proc/*: only load floating point registers when needed (#1981)
Browse files Browse the repository at this point in the history
Changes implementations of proc.Registers interface and the
op.DwarfRegisters struct so that floating point registers can be loaded
only when they are needed.
Removes the floatingPoint parameter from proc.Thread.Registers.
This accomplishes three things:

1. it simplifies the proc.Thread.Registers interface
2. it makes it impossible to accidentally create a broken set of saved
   registers or of op.DwarfRegisters by accidentally calling
   Registers(false)
3. it improves general performance of Delve by avoiding to load
   floating point registers as much as possible

Floating point registers are loaded under two circumstances:

1. When the Slice method is called with floatingPoint == true
2. When the Copy method is called

Benchmark before:

BenchmarkConditionalBreakpoints-4   	       1	4327350142 ns/op

Benchmark after:

BenchmarkConditionalBreakpoints-4   	       1	3852642917 ns/op

Updates #1549
  • Loading branch information
aarzilli authored May 13, 2020
1 parent f96663a commit 200994b
Show file tree
Hide file tree
Showing 37 changed files with 360 additions and 246 deletions.
68 changes: 59 additions & 9 deletions pkg/dwarf/op/regs.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,55 @@ type DwarfRegisters struct {
CFA int64
FrameBase int64
ObjBase int64
Regs []*DwarfRegister
regs []*DwarfRegister

ByteOrder binary.ByteOrder
PCRegNum uint64
SPRegNum uint64
BPRegNum uint64
LRRegNum uint64

FloatLoadError error // error produced when loading floating point registers
loadMoreCallback func()
}

type DwarfRegister struct {
Uint64Val uint64
Bytes []byte
}

// NewDwarfRegisters returns a new DwarfRegisters object.
func NewDwarfRegisters(staticBase uint64, regs []*DwarfRegister, byteOrder binary.ByteOrder, pcRegNum, spRegNum, bpRegNum, lrRegNum uint64) *DwarfRegisters {
return &DwarfRegisters{
StaticBase: staticBase,
regs: regs,
ByteOrder: byteOrder,
PCRegNum: pcRegNum,
SPRegNum: spRegNum,
BPRegNum: bpRegNum,
LRRegNum: lrRegNum,
}
}

// SetLoadMoreCallback sets a callback function that will be called the
// first time the user of regs tries to access an undefined register.
func (regs *DwarfRegisters) SetLoadMoreCallback(fn func()) {
regs.loadMoreCallback = fn
}

// CurrentSize returns the current number of known registers. This number might be
// wrong if loadMoreCallback has been set.
func (regs *DwarfRegisters) CurrentSize() int {
return len(regs.regs)
}

// Uint64Val returns the uint64 value of register idx.
func (regs *DwarfRegisters) Uint64Val(idx uint64) uint64 {
reg := regs.Reg(idx)
if reg == nil {
return 0
}
return regs.Regs[idx].Uint64Val
return regs.regs[idx].Uint64Val
}

// Bytes returns the bytes value of register idx, nil if the register is not
Expand All @@ -50,12 +78,26 @@ func (regs *DwarfRegisters) Bytes(idx uint64) []byte {
return reg.Bytes
}

func (regs *DwarfRegisters) loadMore() {
if regs.loadMoreCallback == nil {
return
}
regs.loadMoreCallback()
regs.loadMoreCallback = nil
}

// Reg returns register idx or nil if the register is not defined.
func (regs *DwarfRegisters) Reg(idx uint64) *DwarfRegister {
if idx >= uint64(len(regs.Regs)) {
return nil
if idx >= uint64(len(regs.regs)) {
regs.loadMore()
if idx >= uint64(len(regs.regs)) {
return nil
}
}
return regs.Regs[idx]
if regs.regs[idx] == nil {
regs.loadMore()
}
return regs.regs[idx]
}

func (regs *DwarfRegisters) PC() uint64 {
Expand All @@ -72,12 +114,20 @@ func (regs *DwarfRegisters) BP() uint64 {

// AddReg adds register idx to regs.
func (regs *DwarfRegisters) AddReg(idx uint64, reg *DwarfRegister) {
if idx >= uint64(len(regs.Regs)) {
if idx >= uint64(len(regs.regs)) {
newRegs := make([]*DwarfRegister, idx+1)
copy(newRegs, regs.Regs)
regs.Regs = newRegs
copy(newRegs, regs.regs)
regs.regs = newRegs
}
regs.regs[idx] = reg
}

// ClearRegisters clears all registers.
func (regs *DwarfRegisters) ClearRegisters() {
regs.loadMoreCallback = nil
for regnum := range regs.regs {
regs.regs[regnum] = nil
}
regs.Regs[idx] = reg
}

func DwarfRegisterFromUint64(v uint64) *DwarfRegister {
Expand Down
40 changes: 22 additions & 18 deletions pkg/proc/amd64_arch.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,21 +345,32 @@ func maxAmd64DwarfRegister() int {
}

func amd64RegistersToDwarfRegisters(staticBase uint64, regs Registers) op.DwarfRegisters {
dregs := make([]*op.DwarfRegister, maxAmd64DwarfRegister()+1)
dregs := initDwarfRegistersFromSlice(maxAmd64DwarfRegister(), regs, amd64NameToDwarf)
dr := op.NewDwarfRegisters(staticBase, dregs, binary.LittleEndian, amd64DwarfIPRegNum, amd64DwarfSPRegNum, amd64DwarfBPRegNum, 0)
dr.SetLoadMoreCallback(loadMoreDwarfRegistersFromSliceFunc(dr, regs, amd64NameToDwarf))
return *dr
}

for _, reg := range regs.Slice(true) {
if dwarfReg, ok := amd64NameToDwarf[strings.ToLower(reg.Name)]; ok {
func initDwarfRegistersFromSlice(maxRegs int, regs Registers, nameToDwarf map[string]int) []*op.DwarfRegister {
dregs := make([]*op.DwarfRegister, maxRegs+1)
regslice, _ := regs.Slice(false)
for _, reg := range regslice {
if dwarfReg, ok := nameToDwarf[strings.ToLower(reg.Name)]; ok {
dregs[dwarfReg] = reg.Reg
}
}
return dregs
}

return op.DwarfRegisters{
StaticBase: staticBase,
Regs: dregs,
ByteOrder: binary.LittleEndian,
PCRegNum: amd64DwarfIPRegNum,
SPRegNum: amd64DwarfSPRegNum,
BPRegNum: amd64DwarfBPRegNum,
func loadMoreDwarfRegistersFromSliceFunc(dr *op.DwarfRegisters, regs Registers, nameToDwarf map[string]int) func() {
return func() {
regslice, err := regs.Slice(true)
dr.FloatLoadError = err
for _, reg := range regslice {
if dwarfReg, ok := nameToDwarf[strings.ToLower(reg.Name)]; ok {
dr.AddReg(uint64(dwarfReg), reg.Reg)
}
}
}
}

Expand All @@ -369,14 +380,7 @@ func amd64AddrAndStackRegsToDwarfRegisters(staticBase, pc, sp, bp, lr uint64) op
dregs[amd64DwarfSPRegNum] = op.DwarfRegisterFromUint64(sp)
dregs[amd64DwarfBPRegNum] = op.DwarfRegisterFromUint64(bp)

return op.DwarfRegisters{
StaticBase: staticBase,
Regs: dregs,
ByteOrder: binary.LittleEndian,
PCRegNum: amd64DwarfIPRegNum,
SPRegNum: amd64DwarfSPRegNum,
BPRegNum: amd64DwarfBPRegNum,
}
return *op.NewDwarfRegisters(staticBase, dregs, binary.LittleEndian, amd64DwarfIPRegNum, amd64DwarfSPRegNum, amd64DwarfBPRegNum, 0)
}

func amd64DwarfRegisterToString(i int, reg *op.DwarfRegister) (name string, floatingPoint bool, repr string) {
Expand Down
36 changes: 18 additions & 18 deletions pkg/proc/arm64_arch.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,20 @@ var arm64DwarfToHardware = map[int]arm64asm.Reg{
95: arm64asm.V31,
}

var arm64NameToDwarf = func() map[string]int {
r := make(map[string]int)
for i := 0; i <= 30; i++ {
r[fmt.Sprintf("x%d", i)] = i
}
r["pc"] = int(arm64DwarfIPRegNum)
r["lr"] = int(arm64DwarfLRRegNum)
r["sp"] = 31
for i := 0; i <= 31; i++ {
r[fmt.Sprintf("v%d", i)] = i + 64
}
return r
}()

func maxArm64DwarfRegister() int {
max := int(arm64DwarfIPRegNum)
for i := range arm64DwarfToHardware {
Expand Down Expand Up @@ -339,15 +353,9 @@ func arm64RegistersToDwarfRegisters(staticBase uint64, regs Registers) op.DwarfR
}
}

return op.DwarfRegisters{
StaticBase: staticBase,
Regs: dregs,
ByteOrder: binary.LittleEndian,
PCRegNum: arm64DwarfIPRegNum,
SPRegNum: arm64DwarfSPRegNum,
BPRegNum: arm64DwarfBPRegNum,
LRRegNum: arm64DwarfLRRegNum,
}
dr := op.NewDwarfRegisters(staticBase, dregs, binary.LittleEndian, arm64DwarfIPRegNum, arm64DwarfSPRegNum, arm64DwarfBPRegNum, arm64DwarfLRRegNum)
dr.SetLoadMoreCallback(loadMoreDwarfRegistersFromSliceFunc(dr, regs, arm64NameToDwarf))
return *dr
}

func arm64AddrAndStackRegsToDwarfRegisters(staticBase, pc, sp, bp, lr uint64) op.DwarfRegisters {
Expand All @@ -357,15 +365,7 @@ func arm64AddrAndStackRegsToDwarfRegisters(staticBase, pc, sp, bp, lr uint64) op
dregs[arm64DwarfBPRegNum] = op.DwarfRegisterFromUint64(bp)
dregs[arm64DwarfLRRegNum] = op.DwarfRegisterFromUint64(lr)

return op.DwarfRegisters{
StaticBase: staticBase,
Regs: dregs,
ByteOrder: binary.LittleEndian,
PCRegNum: arm64DwarfIPRegNum,
SPRegNum: arm64DwarfSPRegNum,
BPRegNum: arm64DwarfBPRegNum,
LRRegNum: arm64DwarfLRRegNum,
}
return *op.NewDwarfRegisters(staticBase, dregs, binary.LittleEndian, arm64DwarfIPRegNum, arm64DwarfSPRegNum, arm64DwarfBPRegNum, arm64DwarfLRRegNum)
}

func arm64DwarfRegisterToString(i int, reg *op.DwarfRegister) (name string, floatingPoint bool, repr string) {
Expand Down
8 changes: 4 additions & 4 deletions pkg/proc/core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ type thread struct {
}

type osThread interface {
registers(floatingPoint bool) (proc.Registers, error)
registers() (proc.Registers, error)
pid() int
}

Expand Down Expand Up @@ -281,7 +281,7 @@ func (t *thread) WriteMemory(addr uintptr, data []byte) (int, error) {
// Location returns the location of this thread based on
// the value of the instruction pointer register.
func (t *thread) Location() (*proc.Location, error) {
regs, err := t.th.registers(false)
regs, err := t.th.registers()
if err != nil {
return nil, err
}
Expand All @@ -303,8 +303,8 @@ func (t *thread) ThreadID() int {
}

// Registers returns the current value of the registers for this thread.
func (t *thread) Registers(floatingPoint bool) (proc.Registers, error) {
return t.th.registers(floatingPoint)
func (t *thread) Registers() (proc.Registers, error) {
return t.th.registers()
}

// RestoreRegisters will only return an error for core files,
Expand Down
12 changes: 8 additions & 4 deletions pkg/proc/core/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,9 @@ func withCoreFile(t *testing.T, name, args string) *proc.Target {

func logRegisters(t *testing.T, regs proc.Registers, arch *proc.Arch) {
dregs := arch.RegistersToDwarfRegisters(0, regs)
for i, reg := range dregs.Regs {
dregs.Reg(^uint64(0))
for i := 0; i < dregs.CurrentSize(); i++ {
reg := dregs.Reg(uint64(i))
if reg == nil {
continue
}
Expand Down Expand Up @@ -250,7 +252,7 @@ func TestCore(t *testing.T) {
t.Errorf("main.msg = %q, want %q", msg.Value, "BOOM!")
}

regs, err := p.CurrentThread().Registers(true)
regs, err := p.CurrentThread().Registers()
if err != nil {
t.Fatalf("Couldn't get current thread registers: %v", err)
}
Expand Down Expand Up @@ -286,7 +288,7 @@ func TestCoreFpRegisters(t *testing.T) {
continue
}
if frames[i].Current.Fn.Name == "main.main" {
regs, err = thread.Registers(true)
regs, err = thread.Registers()
if err != nil {
t.Fatalf("Could not get registers for thread %x, %v", thread.ThreadID(), err)
}
Expand Down Expand Up @@ -326,7 +328,9 @@ func TestCoreFpRegisters(t *testing.T) {

for _, regtest := range regtests {
found := false
for i, reg := range dregs.Regs {
dregs.Reg(^uint64(0))
for i := 0; i < dregs.CurrentSize(); i++ {
reg := dregs.Reg(uint64(i))
regname, _, regval := arch.DwarfRegisterToString(i, reg)
if reg != nil && regname == regtest.name {
found = true
Expand Down
12 changes: 4 additions & 8 deletions pkg/proc/core/linux_core.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,21 +154,17 @@ type linuxARM64Thread struct {
t *linuxPrStatusARM64
}

func (t *linuxAMD64Thread) registers(floatingPoint bool) (proc.Registers, error) {
func (t *linuxAMD64Thread) registers() (proc.Registers, error) {
var r linutil.AMD64Registers
r.Regs = t.regs.Regs
if floatingPoint {
r.Fpregs = t.regs.Fpregs
}
r.Fpregs = t.regs.Fpregs
return &r, nil
}

func (t *linuxARM64Thread) registers(floatingPoint bool) (proc.Registers, error) {
func (t *linuxARM64Thread) registers() (proc.Registers, error) {
var r linutil.ARM64Registers
r.Regs = t.regs.Regs
if floatingPoint {
r.Fpregs = t.regs.Fpregs
}
r.Fpregs = t.regs.Fpregs
return &r, nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/proc/core/windows_amd64_minidump.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,6 @@ func (th *windowsAMD64Thread) pid() int {
return int(th.th.ID)
}

func (th *windowsAMD64Thread) registers(floatingPoint bool) (proc.Registers, error) {
return winutil.NewAMD64Registers(&th.th.Context, th.th.TEB, floatingPoint), nil
func (th *windowsAMD64Thread) registers() (proc.Registers, error) {
return winutil.NewAMD64Registers(&th.th.Context, th.th.TEB), nil
}
2 changes: 1 addition & 1 deletion pkg/proc/dwarf_expr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func TestDwarfExprLoclist(t *testing.T) {

uintExprCheck(t, scope, "a", before)
scope.PC = 0x40800
scope.Regs.Regs[scope.Regs.PCRegNum].Uint64Val = scope.PC
scope.Regs.Reg(scope.Regs.PCRegNum).Uint64Val = scope.PC
uintExprCheck(t, scope, "a", after)
}

Expand Down
Loading

0 comments on commit 200994b

Please sign in to comment.