Store hardware/software breakpoints in same struct

proc/arch.go

@@ -7,15 +7,16 @@ type Arch interface {
 	BreakpointInstruction() []byte
 	BreakpointSize() int
 	CurgInstructions() []byte
-	HardwareBreakpoints() []*Breakpoint
+	HardwareBreakpointUsage() []bool
+	SetHardwareBreakpointUsage(int, bool)
 }
 
 type AMD64 struct {
-	ptrSize             int
-	breakInstruction    []byte
-	breakInstructionLen int
-	curgInstructions    []byte
-	hardwareBreakpoints []*Breakpoint // Slice of hardware breakpoints
+	ptrSize                 int
+	breakInstruction        []byte
+	breakInstructionLen     int
+	curgInstructions        []byte
+	hardwareBreakpointUsage []bool
 }
 
 func AMD64Arch() *AMD64 {
@@ -38,11 +39,11 @@ func AMD64Arch() *AMD64 {
 	curg = append(curg, breakInstr[0])
 
 	return &AMD64{
-		ptrSize:             8,
-		breakInstruction:    breakInstr,
-		breakInstructionLen: 1,
-		curgInstructions:    curg,
-		hardwareBreakpoints: make([]*Breakpoint, 4),
+		ptrSize:                 8,
+		breakInstruction:        breakInstr,
+		breakInstructionLen:     1,
+		curgInstructions:        curg,
+		hardwareBreakpointUsage: make([]bool, 4),
 	}
 }
 
@@ -62,6 +63,10 @@ func (a *AMD64) CurgInstructions() []byte {
 	return a.curgInstructions
 }
 
-func (a *AMD64) HardwareBreakpoints() []*Breakpoint {
-	return a.hardwareBreakpoints
+func (a *AMD64) HardwareBreakpointUsage() []bool {
+	return a.hardwareBreakpointUsage
+}
+
+func (a *AMD64) SetHardwareBreakpointUsage(reg int, set bool) {
+	a.hardwareBreakpointUsage[reg] = set
 }

proc/breakpoints.go

@@ -63,22 +63,16 @@ func (iae InvalidAddressError) Error() string {
 	return fmt.Sprintf("Invalid address %#v\n", iae.address)
 }
 
-// Returns whether or not a breakpoint has been set for the given address.
-func (dbp *DebuggedProcess) BreakpointExists(addr uint64) bool {
-	for _, bp := range dbp.arch.HardwareBreakpoints() {
-		// TODO(darwin)
-		if runtime.GOOS == "darwin" {
-			break
-		}
-		if bp != nil && bp.Addr == addr {
-			return true
-		}
+func (dbp *DebuggedProcess) setBreakpoint(tid int, addr uint64, temp bool) (*Breakpoint, error) {
+	if bp, ok := dbp.FindBreakpoint(addr); ok {
+		return nil, BreakpointExistsError{bp.File, bp.Line, bp.Addr}
+	}
+
+	f, l, fn := dbp.goSymTable.PCToLine(uint64(addr))
+	if fn == nil {
+		return nil, InvalidAddressError{address: addr}
 	}
-	_, ok := dbp.Breakpoints[addr]
-	return ok
-}
 
-func (dbp *DebuggedProcess) newBreakpoint(fn, f string, l int, addr uint64, data []byte, temp bool) *Breakpoint {
 	var id int
 	if temp {
 		dbp.tempBreakpointIDCounter++
@@ -87,48 +81,35 @@ func (dbp *DebuggedProcess) newBreakpoint(fn, f string, l int, addr uint64, data
 		dbp.breakpointIDCounter++
 		id = dbp.breakpointIDCounter
 	}
-	return &Breakpoint{
-		FunctionName: fn,
-		File:         f,
-		Line:         l,
-		Addr:         addr,
-		OriginalData: data,
-		ID:           id,
-		Temp:         temp,
-	}
-}
 
-func (dbp *DebuggedProcess) newHardwareBreakpoint(fn, f string, l int, addr uint64, data []byte, temp bool, reg int) *Breakpoint {
-	bp := dbp.newBreakpoint(fn, f, l, addr, data, temp)
-	bp.hardware = true
-	bp.reg = reg
-	return bp
-}
-
-func (dbp *DebuggedProcess) setBreakpoint(tid int, addr uint64, temp bool) (*Breakpoint, error) {
-	var f, l, fn = dbp.goSymTable.PCToLine(uint64(addr))
-	if fn == nil {
-		return nil, InvalidAddressError{address: addr}
-	}
-	if dbp.BreakpointExists(addr) {
-		return nil, BreakpointExistsError{f, l, addr}
-	}
 	// Try and set a hardware breakpoint.
-	for i, v := range dbp.arch.HardwareBreakpoints() {
-		// TODO(darwin)
-		if runtime.GOOS == "darwin" {
+	for i, used := range dbp.arch.HardwareBreakpointUsage() {
+		if runtime.GOOS == "darwin" { // TODO(dp): Implement hardware breakpoints on OSX.
 			break
 		}
-		if v == nil {
-			for t, _ := range dbp.Threads {
-				if err := dbp.setHardwareBreakpoint(i, t, addr); err != nil {
-					return nil, fmt.Errorf("could not set hardware breakpoint on thread %d: %s", t, err)
-				}
+		if used {
+			continue
+		}
+		for t, _ := range dbp.Threads {
+			if err := dbp.setHardwareBreakpoint(i, t, addr); err != nil {
+				return nil, fmt.Errorf("could not set hardware breakpoint on thread %d: %s", t, err)
 			}
-			dbp.arch.HardwareBreakpoints()[i] = dbp.newHardwareBreakpoint(fn.Name, f, l, addr, nil, temp, i)
-			return dbp.arch.HardwareBreakpoints()[i], nil
 		}
+		dbp.arch.SetHardwareBreakpointUsage(i, true)
+		dbp.Breakpoints[addr] = &Breakpoint{
+			FunctionName: fn.Name,
+			File:         f,
+			Line:         l,
+			Addr:         addr,
+			ID:           id,
+			Temp:         temp,
+			hardware:     true,
+			reg:          i,
+		}
+
+		return dbp.Breakpoints[addr], nil
 	}
+
 	// Fall back to software breakpoint. 0xCC is INT 3 trap interrupt.
 	thread := dbp.Threads[tid]
 	originalData := make([]byte, dbp.arch.BreakpointSize())
@@ -138,7 +119,16 @@ func (dbp *DebuggedProcess) setBreakpoint(tid int, addr uint64, temp bool) (*Bre
 	if _, err := writeMemory(thread, uintptr(addr), dbp.arch.BreakpointInstruction()); err != nil {
 		return nil, err
 	}
-	dbp.Breakpoints[addr] = dbp.newBreakpoint(fn.Name, f, l, addr, originalData, temp)
+	dbp.Breakpoints[addr] = &Breakpoint{
+		FunctionName: fn.Name,
+		File:         f,
+		Line:         l,
+		Addr:         addr,
+		OriginalData: originalData,
+		ID:           id,
+		Temp:         temp,
+	}
+
 	return dbp.Breakpoints[addr], nil
 }
 
@@ -153,25 +143,13 @@ func (nbp NoBreakpointError) Error() string {
 
 func (dbp *DebuggedProcess) clearBreakpoint(tid int, addr uint64) (*Breakpoint, error) {
 	thread := dbp.Threads[tid]
-	// Check for hardware breakpoint
-	for i, bp := range dbp.arch.HardwareBreakpoints() {
-		if bp == nil {
-			continue
-		}
-		if bp.Addr == addr {
-			_, err := bp.Clear(thread)
-			if err != nil {
-				return nil, err
-			}
-			dbp.arch.HardwareBreakpoints()[i] = nil
-			return bp, nil
-		}
-	}
-	// Check for software breakpoint
 	if bp, ok := dbp.Breakpoints[addr]; ok {
 		if _, err := bp.Clear(thread); err != nil {
 			return nil, err
 		}
+		if bp.hardware {
+			dbp.arch.SetHardwareBreakpointUsage(bp.reg, false)
+		}
 		delete(dbp.Breakpoints, addr)
 		return bp, nil
 	}

proc/proc.go

@@ -26,7 +26,7 @@ type DebuggedProcess struct {
 	Pid     int         // Process Pid
 	Process *os.Process // Pointer to process struct for the actual process we are debugging
 
-	// Software breakpoint table. Hardware breakpoints are stored in proc/arch.go, as they are architecture dependant.
+	// Breakpoint table. Hardware breakpoints are stored in proc/arch.go, as they are architecture dependant.
 	Breakpoints map[uint64]*Breakpoint
 
 	// List of threads mapped as such: pid -> *Thread
@@ -98,11 +98,6 @@ func Attach(pid int) (*DebuggedProcess, error) {
 
 func (dbp *DebuggedProcess) Detach(kill bool) (err error) {
 	// Clean up any breakpoints we've set.
-	for _, bp := range dbp.arch.HardwareBreakpoints() {
-		if bp != nil {
-			dbp.Clear(bp.Addr)
-		}
-	}
 	for _, bp := range dbp.Breakpoints {
 		if bp != nil {
 			dbp.Clear(bp.Addr)
@@ -187,15 +182,6 @@ func (dbp *DebuggedProcess) FindLocation(str string) (uint64, error) {
 		return 0, fmt.Errorf("unable to find location for %s", str)
 	}
 
-	// Use as breakpoint id
-	for _, bp := range dbp.arch.HardwareBreakpoints() {
-		if bp == nil {
-			continue
-		}
-		if uint64(bp.ID) == id {
-			return bp.Addr, nil
-		}
-	}
 	for _, bp := range dbp.Breakpoints {
 		if uint64(bp.ID) == id {
 			return bp.Addr, nil
@@ -249,10 +235,6 @@ func (dbp *DebuggedProcess) BreakByLocation(loc string) (*Breakpoint, error) {
 	return dbp.Break(addr)
 }
 
-func (dbp *DebuggedProcess) HardwareBreakpoints() []*Breakpoint {
-	return dbp.arch.HardwareBreakpoints()
-}
-
 // Clears a breakpoint in the current thread.
 func (dbp *DebuggedProcess) Clear(addr uint64) (*Breakpoint, error) {
 	return dbp.clearBreakpoint(dbp.CurrentThread.Id, addr)
@@ -527,11 +509,6 @@ func (dbp *DebuggedProcess) PCToLine(pc uint64) (string, int, *gosym.Func) {
 
 // Finds the breakpoint for the given ID.
 func (dbp *DebuggedProcess) FindBreakpointByID(id int) (*Breakpoint, bool) {
-	for _, bp := range dbp.arch.HardwareBreakpoints() {
-		if bp != nil && bp.ID == id {
-			return bp, true
-		}
-	}
 	for _, bp := range dbp.Breakpoints {
 		if bp.ID == id {
 			return bp, true
@@ -542,11 +519,15 @@ func (dbp *DebuggedProcess) FindBreakpointByID(id int) (*Breakpoint, bool) {
 
 // Finds the breakpoint for the given pc.
 func (dbp *DebuggedProcess) FindBreakpoint(pc uint64) (*Breakpoint, bool) {
-	for _, bp := range dbp.arch.HardwareBreakpoints() {
-		if bp != nil && bp.Addr == pc {
-			return bp, true
-		}
+	// Check for software breakpoint. PC will be at
+	// breakpoint instruction + size of breakpoint.
+	if bp, ok := dbp.Breakpoints[pc-uint64(dbp.arch.BreakpointSize())]; ok {
+		return bp, true
 	}
+	// Check for hardware breakpoint. PC will equal
+	// the breakpoint address since the CPU will stop
+	// the process without executing the instruction at
+	// this address.
 	if bp, ok := dbp.Breakpoints[pc]; ok {
 		return bp, true
 	}
@@ -591,13 +572,6 @@ func initializeDebugProcess(dbp *DebuggedProcess, path string, attach bool) (*De
 }
 
 func (dbp *DebuggedProcess) clearTempBreakpoints() error {
-	for _, bp := range dbp.arch.HardwareBreakpoints() {
-		if bp != nil && bp.Temp {
-			if _, err := dbp.Clear(bp.Addr); err != nil {
-				return err
-			}
-		}
-	}
 	for _, bp := range dbp.Breakpoints {
 		if !bp.Temp {
 			continue
@@ -618,15 +592,8 @@ func (dbp *DebuggedProcess) handleBreakpointOnThread(id int) (*Thread, error) {
 	if err != nil {
 		return nil, err
 	}
-	// Check for hardware breakpoint
-	for _, bp := range dbp.arch.HardwareBreakpoints() {
-		if bp != nil && bp.Addr == pc {
-			thread.CurrentBreakpoint = bp
-			return thread, nil
-		}
-	}
-	// Check to see if we have hit a software breakpoint.
-	if bp, ok := dbp.Breakpoints[pc-1]; ok {
+	// Check to see if we have hit a breakpoint.
+	if bp, ok := dbp.FindBreakpoint(pc); ok {
 		thread.CurrentBreakpoint = bp
 		if err = thread.SetPC(bp.Addr); err != nil {
 			return nil, err

proc/proc_linux.go

@@ -253,11 +253,12 @@ func (dbp *DebuggedProcess) trapWait(pid int) (*Thread, error) {
 			if err != nil {
 				return nil, err
 			}
-			for reg, bp := range dbp.HardwareBreakpoints() {
-				if bp == nil {
+			// Set all hardware breakpoints on the new thread.
+			for _, bp := range dbp.Breakpoints {
+				if !bp.hardware {
 					continue
 				}
-				if err = dbp.setHardwareBreakpoint(reg, th.Id, bp.Addr); err != nil {
+				if err = dbp.setHardwareBreakpoint(bp.reg, th.Id, bp.Addr); err != nil {
 					return nil, err
 				}
 			}

proc/proc_test.go

@@ -254,11 +254,6 @@ func testnext(program string, testcases []nextTest, initialLocation string, t *t
 		if len(p.Breakpoints) != 0 {
 			t.Fatal("Not all breakpoints were cleaned up", len(p.Breakpoints))
 		}
-		for _, bp := range p.arch.HardwareBreakpoints() {
-			if bp != nil {
-				t.Fatal("Not all breakpoints were cleaned up", bp.Addr)
-			}
-		}
 	})
 }
 

service/debugger/debugger.go

@@ -117,13 +117,6 @@ func (d *Debugger) ClearBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoint
 
 func (d *Debugger) Breakpoints() []*api.Breakpoint {
 	bps := []*api.Breakpoint{}
-	for _, bp := range d.process.HardwareBreakpoints() {
-		if bp == nil {
-			continue
-		}
-		bps = append(bps, api.ConvertBreakpoint(bp))
-	}
-
 	for _, bp := range d.process.Breakpoints {
 		if bp.Temp {
 			continue