amd64: allow gdb(4) to write to most registers
ClosedPublic
Actions

Authored by mhorne on Dec 16 2020, 8:35 PM.

Details

Reviewers

markj
cem
jhb

Commits

rS368764: amd64: allow gdb(4) to write to most registers

Summary

Similar to the recent patch to arm's gdb stub in rS368414.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

mhorne created this revision.Dec 16 2020, 8:35 PM

Herald added subscribers: emaste, imp. · View Herald TranscriptDec 16 2020, 8:35 PM

mhorne requested review of this revision.Dec 16 2020, 8:35 PM

Harbormaster completed remote builds in B35449: Diff 80785.Dec 16 2020, 8:35 PM

mhorne mentioned this in D27644: gdb(4): allow bulk write and single read of registers.Dec 16 2020, 8:49 PM

Can we lift the condition out and just have two switches? Not sure if that’s any better but it seems sort of ugly as is.

In D27642#617763, @cem wrote:

Can we lift the condition out and just have two switches? Not sure if that’s any better but it seems sort of ugly as is.

I'm not sure which is worse/better. I'm fine with either approach.

jhb accepted this revision.Dec 17 2020, 5:33 PM

This revision is now accepted and ready to land.Dec 17 2020, 5:33 PM

markj accepted this revision.Dec 17 2020, 5:41 PM

Update to use two switch statements. This version fits vertically on a single screen, and more closely matches the formatting of gdb_cpu_setreg().

This revision now requires review to proceed.Dec 17 2020, 8:10 PM

Harbormaster completed remote builds in B35490: Diff 80854.Dec 17 2020, 8:10 PM

Fix typo, GDB_REG_RIP -> GDB_REG_PC.

Harbormaster completed remote builds in B35491: Diff 80855.Dec 17 2020, 8:14 PM

jhb added inline comments.Dec 17 2020, 8:24 PM

sys/amd64/amd64/gdb_machdep.c
103 ↗	(On Diff #80855)	I think the two switch statements is fine. I think the style of the case bodies is perhaps atypical for FreeBSD though. Ah, I see it matches getreg. Oh well. Changing getreg to use the new constants might be a nice followup as well.

cem accepted this revision.Dec 17 2020, 9:01 PM

This revision is now accepted and ready to land.Dec 17 2020, 9:01 PM

mhorne added inline comments.Dec 17 2020, 9:43 PM

sys/amd64/amd64/gdb_machdep.c
103 ↗	(On Diff #80855)	Yeah, this is a bit of a weird construct with no ideal way to format it. With expanded case bodies the function becomes unreasonably long, so this is the best option, to my eye. I am planning on that follow-up to getreg().