pkg/proc/core: Clean up some repetitive code (#3834)

Cleans up repetitive code in pkg/proc/core and also adds coredumpctl
support to our test suite.

pkg/proc/core/core.go

@@ -168,14 +168,14 @@ type process struct {
 
 // thread represents a thread in the core file being debugged.
 type thread struct {
-	th     osThread
+	osThread
 	p      *process
 	common proc.CommonThread
 }
 
 type osThread interface {
-	registers() (proc.Registers, error)
+	Registers() (proc.Registers, error)
-	pid() int
+	ThreadID() int
 }
 
 var (
@@ -322,7 +322,7 @@ func (t *thread) ProcessMemory() proc.MemoryReadWriter {
 // 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()
+	regs, err := t.Registers()
 	if err != nil {
 		return nil, err
 	}
@@ -338,16 +338,6 @@ func (t *thread) Breakpoint() *proc.BreakpointState {
 	return &proc.BreakpointState{}
 }
 
-// ThreadID returns the ID for this thread.
-func (t *thread) ThreadID() int {
-	return t.th.pid()
-}
-
-// Registers returns the current value of the registers for this thread.
-func (t *thread) Registers() (proc.Registers, error) {
-	return t.th.registers()
-}
-
 // RestoreRegisters will only return an error for core files,
 // you cannot change register values for core files.
 func (t *thread) RestoreRegisters(proc.Registers) error {

pkg/proc/core/core_test.go

@@ -213,8 +213,13 @@ func withCoreFile(t *testing.T, name, args string) *proc.TargetGroup {
 	case err != nil || len(cores) > 1:
 		t.Fatalf("Got %v, wanted one file named core* in %v", cores, tempDir)
 	case len(cores) == 0:
-		t.Skipf("core file was not produced, could not run test")
+		cores = []string{fix.Path + ".dump"}
-		return nil
+		err := exec.Command("coredumpctl", "--output="+cores[0], "dump", fix.Path).Run()
+		if err != nil {
+			t.Skipf("core file was not produced, could not run test, coredumpctl error: %v", err)
+			return nil
+		}
+		test.PathsToRemove = append(test.PathsToRemove, cores[0])
 	}
 	corePath := cores[0]
 

pkg/proc/core/delve_core.go

@@ -130,11 +130,11 @@ type delveThread struct {
 	regs *delveRegisters
 }
 
-func (th *delveThread) pid() int {
+func (th *delveThread) ThreadID() int {
 	return int(th.id)
 }
 
-func (th *delveThread) registers() (proc.Registers, error) {
+func (th *delveThread) Registers() (proc.Registers, error) {
 	return th.regs, nil
 }
 

pkg/proc/core/linux_core.go

@@ -48,50 +48,38 @@ const elfErrorBadMagicNumber = "bad magic number"
 
 func linuxThreadsFromNotes(p *process, notes []*note, machineType elf.Machine) proc.Thread {
 	var currentThread proc.Thread
-	var lastThreadAMD *linuxAMD64Thread
+	var lastThread osThread
-	var lastThreadARM *linuxARM64Thread
-	var lastThreadRISCV *linuxRISCV64Thread
 
 	for _, note := range notes {
 		switch note.Type {
 		case elf.NT_PRSTATUS:
-			if machineType == _EM_X86_64 {
+			switch machineType {
+			case _EM_X86_64:
 				t := note.Desc.(*linuxPrStatusAMD64)
-				lastThreadAMD = &linuxAMD64Thread{linutil.AMD64Registers{Regs: &t.Reg}, t}
+				lastThread = &linuxAMD64Thread{linutil.AMD64Registers{Regs: &t.Reg}, t}
-				p.Threads[int(t.Pid)] = &thread{lastThreadAMD, p, proc.CommonThread{}}
+			case _EM_AARCH64:
-				if currentThread == nil {
-					currentThread = p.Threads[int(t.Pid)]
-				}
-			} else if machineType == _EM_AARCH64 {
 				t := note.Desc.(*linuxPrStatusARM64)
-				lastThreadARM = &linuxARM64Thread{linutil.ARM64Registers{Regs: &t.Reg}, t}
+				lastThread = &linuxARM64Thread{linutil.ARM64Registers{Regs: &t.Reg}, t}
-				p.Threads[int(t.Pid)] = &thread{lastThreadARM, p, proc.CommonThread{}}
+			case _EM_RISCV:
-				if currentThread == nil {
-					currentThread = p.Threads[int(t.Pid)]
-				}
-			} else if machineType == _EM_RISCV {
 				t := note.Desc.(*linuxPrStatusRISCV64)
-				lastThreadRISCV = &linuxRISCV64Thread{linutil.RISCV64Registers{Regs: &t.Reg}, t}
+				lastThread = &linuxRISCV64Thread{linutil.RISCV64Registers{Regs: &t.Reg}, t}
-				p.Threads[int(t.Pid)] = &thread{lastThreadRISCV, p, proc.CommonThread{}}
+			default:
-				if currentThread == nil {
+				continue
-					currentThread = p.Threads[int(t.Pid)]
+			}
-				}
+			p.Threads[lastThread.ThreadID()] = &thread{lastThread, p, proc.CommonThread{}}
+			if currentThread == nil {
+				currentThread = p.Threads[lastThread.ThreadID()]
 			}
 		case _NT_FPREGSET:
-			if machineType == _EM_AARCH64 {
+			switch th := lastThread.(type) {
-				if lastThreadARM != nil {
+			case *linuxARM64Thread:
-					lastThreadARM.regs.Fpregs = note.Desc.(*linutil.ARM64PtraceFpRegs).Decode()
+				th.regs.Fpregs = note.Desc.(*linutil.ARM64PtraceFpRegs).Decode()
-				}
+			case *linuxRISCV64Thread:
-			} else if machineType == _EM_RISCV {
+				th.regs.Fpregs = note.Desc.(*linutil.RISCV64PtraceFpRegs).Decode()
-				if lastThreadRISCV != nil {
-					lastThreadRISCV.regs.Fpregs = note.Desc.(*linutil.RISCV64PtraceFpRegs).Decode()
-				}
 			}
 		case _NT_X86_XSTATE:
-			if machineType == _EM_X86_64 {
+			if lastThread != nil {
-				if lastThreadAMD != nil {
+				lastThread.(*linuxAMD64Thread).regs.Fpregs = note.Desc.(*amd64util.AMD64Xstate).Decode()
-					lastThreadAMD.regs.Fpregs = note.Desc.(*amd64util.AMD64Xstate).Decode()
-				}
 			}
 		case elf.NT_PRPSINFO:
 			p.pid = int(note.Desc.(*linuxPrPsInfo).Pid)
@@ -100,6 +88,12 @@ func linuxThreadsFromNotes(p *process, notes []*note, machineType elf.Machine) p
 	return currentThread
 }
 
+var supportedLinuxMachines = map[elf.Machine]string{
+	_EM_X86_64:  "amd64",
+	_EM_AARCH64: "arm64",
+	_EM_RISCV:   "riscv64",
+}
+
 // readLinuxOrPlatformIndependentCore reads a core file from corePath
 // corresponding to the executable at exePath. For details on the Linux ELF
 // core format, see:
@@ -155,17 +149,10 @@ func readLinuxOrPlatformIndependentCore(corePath, exePath string) (*process, pro
 			return nil, nil, err
 		}
 		bi = proc.NewBinaryInfo(goos, goarch)
+	} else if goarch, ok := supportedLinuxMachines[machineType]; ok {
+		bi = proc.NewBinaryInfo("linux", goarch)
 	} else {
-		switch machineType {
+		return nil, nil, errors.New("unsupported machine type")
-		case _EM_X86_64:
-			bi = proc.NewBinaryInfo("linux", "amd64")
-		case _EM_AARCH64:
-			bi = proc.NewBinaryInfo("linux", "arm64")
-		case _EM_RISCV:
-			bi = proc.NewBinaryInfo("linux", "riscv64")
-		default:
-			return nil, nil, errors.New("unsupported machine type")
-		}
 	}
 
 	entryPoint := findEntryPoint(notes, bi.Arch.PtrSize())
@@ -202,36 +189,36 @@ type linuxRISCV64Thread struct {
 	t    *linuxPrStatusRISCV64
 }
 
-func (t *linuxAMD64Thread) registers() (proc.Registers, error) {
+func (t *linuxAMD64Thread) Registers() (proc.Registers, error) {
 	var r linutil.AMD64Registers
 	r.Regs = t.regs.Regs
 	r.Fpregs = t.regs.Fpregs
 	return &r, nil
 }
 
-func (t *linuxARM64Thread) registers() (proc.Registers, error) {
+func (t *linuxARM64Thread) Registers() (proc.Registers, error) {
 	var r linutil.ARM64Registers
 	r.Regs = t.regs.Regs
 	r.Fpregs = t.regs.Fpregs
 	return &r, nil
 }
 
-func (t *linuxRISCV64Thread) registers() (proc.Registers, error) {
+func (t *linuxRISCV64Thread) Registers() (proc.Registers, error) {
 	var r linutil.RISCV64Registers
 	r.Regs = t.regs.Regs
 	r.Fpregs = t.regs.Fpregs
 	return &r, nil
 }
 
-func (t *linuxAMD64Thread) pid() int {
+func (t *linuxAMD64Thread) ThreadID() int {
 	return int(t.t.Pid)
 }
 
-func (t *linuxARM64Thread) pid() int {
+func (t *linuxARM64Thread) ThreadID() int {
 	return int(t.t.Pid)
 }
 
-func (t *linuxRISCV64Thread) pid() int {
+func (t *linuxRISCV64Thread) ThreadID() int {
 	return int(t.t.Pid)
 }
 
@@ -360,19 +347,12 @@ func readNote(r io.ReadSeeker, machineType elf.Machine) (*note, error) {
 		note.Desc = desc
 	case _NT_FPREGSET:
 		if machineType == _EM_AARCH64 {
-			fpregs := &linutil.ARM64PtraceFpRegs{}
+			err = readFpregsetNote(note, &linutil.ARM64PtraceFpRegs{}, desc[:_ARM_FP_HEADER_START])
-			rdr := bytes.NewReader(desc[:_ARM_FP_HEADER_START])
-			if err := binary.Read(rdr, binary.LittleEndian, fpregs.Byte()); err != nil {
-				return nil, err
-			}
-			note.Desc = fpregs
 		} else if machineType == _EM_RISCV {
-			fpregs := &linutil.RISCV64PtraceFpRegs{}
+			err = readFpregsetNote(note, &linutil.RISCV64PtraceFpRegs{}, desc)
-			rdr := bytes.NewReader(desc)
+		}
-			if err := binary.Read(rdr, binary.LittleEndian, fpregs.Byte()); err != nil {
+		if err != nil {
-				return nil, err
+			return nil, err
-			}
-			note.Desc = fpregs
 		}
 	}
 	if err := skipPadding(r, 4); err != nil {
@@ -381,6 +361,15 @@ func readNote(r io.ReadSeeker, machineType elf.Machine) (*note, error) {
 	return note, nil
 }
 
+func readFpregsetNote(note *note, fpregs interface{ Byte() []byte }, desc []byte) error {
+	rdr := bytes.NewReader(desc)
+	if err := binary.Read(rdr, binary.LittleEndian, fpregs.Byte()); err != nil {
+		return err
+	}
+	note.Desc = fpregs
+	return nil
+}
+
 // skipPadding moves r to the next multiple of pad.
 func skipPadding(r io.ReadSeeker, pad int64) error {
 	pos, err := r.Seek(0, io.SeekCurrent)

pkg/proc/core/windows_amd64_minidump.go

@@ -57,10 +57,10 @@ type windowsAMD64Thread struct {
 	th *minidump.Thread
 }
 
-func (th *windowsAMD64Thread) pid() int {
+func (th *windowsAMD64Thread) ThreadID() int {
 	return int(th.th.ID)
 }
 
-func (th *windowsAMD64Thread) registers() (proc.Registers, error) {
+func (th *windowsAMD64Thread) Registers() (proc.Registers, error) {
 	return winutil.NewAMD64Registers(&th.th.Context, th.th.TEB), nil
 }