On 05/09/2011 21:22, Blue Swirl wrote:
> On Mon, Sep 5, 2011 at 9:33 AM, Fabien Chouteau <chout...@adacore.com> wrote:
>> On 03/09/2011 11:25, Blue Swirl wrote:
>>> On Thu, Sep 1, 2011 at 2:17 PM, Fabien Chouteau <chout...@adacore.com>
>>> wrote:
>>>> Gdb expects all registers windows to be flushed in ram, which is not the
>>>> case
>>>> in Qemu. Therefore the back-trace generation doesn't work. This patch adds
>>>> a
>>>> function to handle reads/writes in stack frames as if windows were flushed.
>>>>
>>>> Signed-off-by: Fabien Chouteau <chout...@adacore.com>
>>>> ---
>>>> gdbstub.c | 10 ++++--
>>>> target-sparc/cpu.h | 7 ++++
>>>> target-sparc/helper.c | 85
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 99 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/gdbstub.c b/gdbstub.c
>>>> index 3b87c27..85d5ad7 100644
>>>> --- a/gdbstub.c
>>>> +++ b/gdbstub.c
>>>> @@ -41,6 +41,9 @@
>>>> #include "qemu_socket.h"
>>>> #include "kvm.h"
>>>>
>>>> +#ifndef TARGET_CPU_MEMORY_RW_DEBUG
>>>> +#define TARGET_CPU_MEMORY_RW_DEBUG cpu_memory_rw_debug
>>>
>>> These days, inline functions are preferred over macros.
>>>
>>
>> This is to allow target-specific implementation of the function.
>
> That can be done with inline functions too.
OK, how do you do that?
>>>> +#endif
>>>>
>>>> enum {
>>>> GDB_SIGNAL_0 = 0,
>>>> @@ -2013,7 +2016,7 @@ static int gdb_handle_packet(GDBState *s, const char
>>>> *line_buf)
>>>> if (*p == ',')
>>>> p++;
>>>> len = strtoull(p, NULL, 16);
>>>> - if (cpu_memory_rw_debug(s->g_cpu, addr, mem_buf, len, 0) != 0) {
>>>> + if (TARGET_CPU_MEMORY_RW_DEBUG(s->g_cpu, addr, mem_buf, len, 0)
>>>> != 0) {
>>>
>>> cpu_memory_rw_debug() could remain unwrapped with a generic function
>>> like cpu_gdb_sync_memory() which gdbstub should explicitly call.
>>>
>>> Maybe the lazy condition codes etc. could be handled in similar way,
>>> cpu_gdb_sync_registers().
>>>
>>
>> Excuse me, I don't understand here.
>
> cpu_gdb_{read,write}_register needs to force calculation of lazy
> condition codes. On Sparc this is handled by cpu_get_psr(), so it is
> not explicit.
I still don't understand you point. Do you suggest a cpu_gdb_sync_memory() that
will flush register windows?
>>>> +
>>>> +/* Gdb expects all registers windows to be flushed in ram. This function
>>>> handles
>>>> + * reads/writes in stack frames as if windows were flushed. We assume
>>>> that the
>>>> + * sparc ABI is followed.
>>>> + */
>>>
>>> We can't assume that, it depends on what we are executing (BIOS, OS,
>>> even application).
>>
>> Well, maybe the statement is too strong. The ABI is required to get a valid
>> result. Gdb cannot build back-traces if the ABI is not followed anyway.
>
> But if the ABI assumption happens to be wrong (for example registers
> contain random values), memory may be corrupted because this would
> happily use whatever the registers contain.
This cannot corrupt memory, the point is to read/write in registers instead of
memory.
> Another way to fix this would be that GDB would tell QEMU what ABI to
> use for flushing. But how would one tell GDB about a non-standard ABI?
>
> For user emulators we can make ABI assumptions, there similar patch
> could make sense. But system emulators can't assume anything about the
> guest OS, it could be Linux, *BSD, a commercial OS or even a toy OS.
I think all of these kernels follow the SPARC32 ABI, and if they don't Gdb
cannot handle them anyway.
This solution covers 99% of the problem.
>
>>> On Sparc64 there are two ABIs (32 bit and 64 bit
>>> with offset of -2047), though calling flushw instruction could handle
>>> that.
>>
>> This solution is for SPARC32 only.
>>
>>> If the flush happens to trigger a fault, we're in big trouble.
>>>
>>
>> That's why it's safer/easier to use this "hackish" read/write in the
>> registers.
>
> No, if the fault happens here, handling it may be tricky. See for
> example what paranoia Linux has to do for user window flushing, it
> involves the no-fault mode in MMU.
There's no possible fault, as we do not read or write in memory, that's the
point of this implementation.
>
>>> Overall, I think this is too hackish. Maybe this is a bug in GDB
>>> instead, information from backtrace is not reliable if ABI is not
>>> known.
>>>
>>
>> It's not a bug in Gdb. To build back-traces you have to read stack frames. To
>> read stack frames, register windows must be flushed.
>
> Yes, but the flusher should be GDB, assuming that flushing is even a
> good idea which I doubt.
>
> Back traces are not reliable in any case. The code could be compiled
> to omit the frame pointer.
This is a corner case. And again, something not supported by Gdb.
>> In Qemu we can avoid
>> flushing with this little trick.
>
> This doesn't avoid flushing but performs it magically during GDB memory
> access.
>
...instead of writing and reading all register windows each time Gdb stops Qemu.
That's what I call "avoid flushing".
Regards,
--
Fabien Chouteau
