| Message ID | 4C9D415F.6090909@cs.ucla.edu |
|---|---|
| State | New |
| Headers | show |
Am 25.09.2010 02:25, Eddie Kohler wrote: > Hi, > > QEMU has a bug that complicates GDB debugging of i386 targets when the > current code or data segment has a nonzero base. A fix is attached. > > If the current code segment has a nonzero base, breakpoints don't work > as expected, because the breakpoint detector does not consider segment > bases. > > If the current data segment has a nonzero base, memory inspection doesn't > work, because cpu_get_phys_page_debug does not consider segment bases. > > A tiny 'operating system' demonstrating the problem is here: > > http://read.cs.ucla.edu/~kohler/qemu-gdbseg-demo.tgz > > The README enclosed in that tarball gives steps on how to replicate the > breakpoint problem. The 'kernel' runs with segment base 0x10000000, so > that linear address 0xF0001000 is translated into physical address > 0x00001000. But breakpoints (which should use virtual addresses) at > a linear address (e.g. 0xF0100000) are ignored. You can stop execution > using a physical address, but all the addresses reported > back to GDB are linear addresses, so this isn't consistent. > > This is a real problem that prevents us from using unpatched QEMU in > classwork. Any comments on the fix?? (A version was initially posted > several years ago.) This is just a workaround for the core issue: lacking segment support in gdb and its remote protocol. Assuming that an address passed via the current protocol refers to CS and DS is broken. Your patch e.g. requires that CS.base == DS.base, which is not always the case with truly segmented OSes. And it prevents setting breakpoints in inactive segments. We really need to fix gdb instead, i.e. transfer the segment states so that the frontend can linearize as required. And gdb needs to be extended to support segment:offset addressing syntax. For the time being, you should be able to workaround the gdb limitation by setting two breakpoints: one on the linear address and another one on the CS offset. Not nice, but used to work for us. Jan
Thanks for the response. I agree the patch is a workaround, but it is a useful workaround, and I'd still argue for including it. The patch doesn't *require* that CS.base == DS.base. Breakpoints correctly and exclusively use CS.base. However, any memory examination uses DS.base, and you're right that the user might "want" to examine some other segment. A GDB fix would involve changing the gdb remote protocol as well as GDB itself and the GDB user interface. Google says you've been thinking about that for a while now -- is it going well? > For the time being, you should be able to workaround the gdb limitation > by setting two breakpoints: one on the linear address and another one on > the CS offset. Not nice, but used to work for us. I don't mind the double-breakpoint as much, but memory examination would still be broken, yes? I don't understand the comment about "prevents setting breakpoints on inactive segments." The code for setting breakpoints has not changed. Do you think the patch would actually make debugging WORSE on any OS? Or have any other undesirable effects, or make it harder to DTRT when GDB is ready? It seems safe & useful to me; & it's 2 LOC! Eddie
Am 25.09.2010 10:35, Eddie Kohler wrote: > Thanks for the response. I agree the patch is a workaround, but it is a > useful workaround, and I'd still argue for including it. Nope, sorry, I have to vote against this. > > The patch doesn't *require* that CS.base == DS.base. Breakpoints It does. There are several parts in QEMU that use cpu_memory_rw_debug for reading or writing code segment: the disassembler, KVM when manipulating soft breakpoints, and also the TB flushing on breakpoint changes relies on cpu_get_phys_page_debug and would break when using DS as base. > correctly and exclusively use CS.base. However, any memory examination > uses DS.base, and you're right that the user might "want" to examine > some other segment. A GDB fix would involve changing the gdb remote > protocol as well as GDB itself and the GDB user interface. Google says > you've been thinking about that for a while now -- is it going well? It's on a long list of things that would be nice to work on... > >> For the time being, you should be able to workaround the gdb limitation >> by setting two breakpoints: one on the linear address and another one on >> the CS offset. Not nice, but used to work for us. > > I don't mind the double-breakpoint as much, but memory examination would > still be broken, yes? Issue "monitor info registers", extract the segment base, add it to the variable address you are interested in, and issue a print request. It is definitely not impossible, just "a bit" unhandy. > > I don't understand the comment about "prevents setting breakpoints on > inactive segments." The code for setting breakpoints has not changed. It has because you unconditionally subtract the CS base from the passed address. If you want to set a breakpoint on some other CS, you would have to account for their base offsets and pass a weirdly "corrected" address from gdb. That's really no sane interface, specifically long-term. > > Do you think the patch would actually make debugging WORSE on any OS? Or > have any other undesirable effects, or make it harder to DTRT when GDB > is ready? It seems safe & useful to me; & it's 2 LOC! The pathes change the interface to gdb by re-defining the semantics of the passed addresses in way that is not future-proof, and they are buggy. Jan
OK, thanks. I understand how you're relying on the current behavior. I'd rather not change all of QEMU and GDB in one step, but I'd like to address this. QEMU documentation implies, and new users expect, that debugging uses virtual addresses, not the segmentation-specific "linear addresses" that are actually used now. - How about a maintenance packet type that changed behavior to what I would prefer (breakpoints and memory access use virtual addresses, not linear addresses)? - We could add a "segment identifier" parameter to cpu_get_phys_page_debug, ignored on all targets but i386 at first. Then we could pass information through to cpu_get_phys_page_debug about what kind of address is being translated. This change could be propagated to cpu_memory_rw_debug (now or later). Would you object? Eddie
Am 26.09.2010 19:19, Eddie Kohler wrote: > OK, thanks. I understand how you're relying on the current behavior. > > I'd rather not change all of QEMU and GDB in one step, The first step is changing gdb anyway. > but I'd like to > address this. QEMU documentation implies, and new users expect, that > debugging uses virtual addresses, not the segmentation-specific "linear > addresses" that are actually used now. > > - How about a maintenance packet type that changed behavior to what I > would prefer (breakpoints and memory access use virtual addresses, not > linear addresses)? > > - We could add a "segment identifier" parameter to > cpu_get_phys_page_debug, ignored on all targets but i386 at first. Then > we could pass information through to cpu_get_phys_page_debug about what > kind of address is being translated. This change could be propagated to > cpu_memory_rw_debug (now or later). Would you object? These changes would establish a temporary interface for an incomplete workaround, and that even with impact on non-x86 code. I would prefer if you could invest your time on the gdb side instead. Anything improved there is not lost - in contrast to the modifications of qemu. I would try to support any effort in this direction. BTW, gdb has nice Python binding these days, so some extensions may also start their life as a helper script. Jan
diff --git a/target-i386/helper.c b/target-i386/helper.c index e134340..0bfd4a9 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -831,6 +831,7 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUState *env, target_ulong addr) target_phys_addr_t paddr; uint32_t page_offset; int page_size; + addr += env->segs[R_DS].base; if (env->cr[4] & CR4_PAE_MASK) { target_ulong pdpe_addr; diff --git a/target-i386/translate.c b/target-i386/translate.c index 7b6e3c2..d9e5b79 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -7816,7 +7816,7 @@ static inline void gen_intermediate_code_internal(CPUState *env, for(;;) { if (unlikely(!QTAILQ_EMPTY(&env->breakpoints))) { QTAILQ_FOREACH(bp, &env->breakpoints, entry) { - if (bp->pc == pc_ptr && + if (bp->pc == pc_ptr - dc->cs_base && !((bp->flags & BP_CPU) && (tb->flags & HF_RF_MASK))) { gen_debug(dc, pc_ptr - dc->cs_base); break;
Hi, QEMU has a bug that complicates GDB debugging of i386 targets when the current code or data segment has a nonzero base. A fix is attached. If the current code segment has a nonzero base, breakpoints don't work as expected, because the breakpoint detector does not consider segment bases. If the current data segment has a nonzero base, memory inspection doesn't work, because cpu_get_phys_page_debug does not consider segment bases. A tiny 'operating system' demonstrating the problem is here: http://read.cs.ucla.edu/~kohler/qemu-gdbseg-demo.tgz The README enclosed in that tarball gives steps on how to replicate the breakpoint problem. The 'kernel' runs with segment base 0x10000000, so that linear address 0xF0001000 is translated into physical address 0x00001000. But breakpoints (which should use virtual addresses) at a linear address (e.g. 0xF0100000) are ignored. You can stop execution using a physical address, but all the addresses reported back to GDB are linear addresses, so this isn't consistent. This is a real problem that prevents us from using unpatched QEMU in classwork. Any comments on the fix?? (A version was initially posted several years ago.) Thanks, Eddie Kohler From 6784824c7576514456a989192e07e63352bdb4ae Mon Sep 17 00:00:00 2001 From: Eddie Kohler <ekohler@gmail.com> Date: Fri, 24 Sep 2010 16:42:27 -0700 Subject: [PATCH] i386 debugging stubs: Consider segment bases - Access dumpable memory relative to the current data segment base. - Detect breakpoints relative to the current code segment base. --- target-i386/helper.c | 1 + target-i386/translate.c | 2 +- 2 files changed, 2 insertions(+), 1 deletions(-)