From: Mark Kettenis <kettenis@science.uva.nl>
To: Jiri Smid <smid@suse.cz>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA]: x86_64 target - multiarch
Date: Wed, 05 Sep 2001 15:31:00 -0000 [thread overview]
Message-ID: <s3ielplqnkb.fsf@soliton.wins.uva.nl> (raw)
In-Reply-To: <s8vr8tlenxi.fsf@naga.suse.cz>
Hi Jiri,
This starts looking good. A few notes though.
* i386-tdep.h:
Is it really necessary to include all those FPU register numbers in
`struct gdbarch_tdep'? At the moment you use at least the same order
for most things as the standard i386 register numbering scheme, so I
think you can remove a few variables from `struct gdbarch_tdep' and
define the register numbers relative to the remaining variables. For
example you could remove fctrl_regnum, fstat_regnum, etc. and do
#define FCTRL_REGNUM FIRST_FPU_CTRL_REGNUM
#define FSTAT_REGNUM FIRST_FPU_CTRL_REGNUM + 1
etc.
* x86_64-linux-nat.c:
Including <asm/*.h> is considered bad practice. You can probably use
<sys/debugreg.h> instead of <asm/debugreg.h> and <sys/syscall.h>
instead of <asm/unistd.h>.
There is a while load of backwards compatibility stuff in this file
that's probably unecessary on x86_64. You probably copied it from
i386-linux-nat.c, where this stuff is necessary to be able to run GDB
on Linux 2.0, and with older versions of glibc. I assume that all
Linux kernels that run on the x86_64 have the PTRACE_GETREGS,
PTRACE_GETFPREGS, PTRACE_GETFPXREGS, etc. requests, so you can
probably get rid of all the HAVE_PTRACE_XXX hackery. The code to
fetch registers directly from the U area (fetch_register,
old_fetch_inferior_registers) can probably be removed too.
child_resume() and child_xfer_memory appear to have some formatting
problems. Might be related to using tabs instead of spaces.
Eli already pointed out that the you should try to use the generic
hardware watchpoint support in i386-nat.c instead of rolling your own
here. Check the current i386-linux-nat.c to see how you can use that
stuff.
* x86_64-nat.c:
If you get rid of the U area register reading code above, the stuff in
this file shouldn't be necessary anymore (at least in theory).
* x86_64-tdep.c:
The implementation of x86_64_register_convert_to_virtual() is almost
certainly wrong. However, the floating-point stuff is a bit in flux
right now, and you might want to fix this later, when things have
settled a bit.
Please fix merge_classes() to use an ISO C function definition.
Again some formatting problems.
Mark
next prev parent reply other threads:[~2001-09-05 15:31 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2001-09-05 7:02 Jiri Smid
2001-09-05 7:38 ` Eli Zaretskii
2001-09-05 15:31 ` Mark Kettenis [this message]
2001-09-05 18:05 ` Andrew Cagney
2001-09-06 0:30 ` Eli Zaretskii
2001-09-11 5:13 ` Jiri Smid
2001-09-11 5:43 ` Eli Zaretskii
2001-09-12 2:31 ` Jiri Smid
2001-09-05 17:55 ` Andrew Cagney
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=s3ielplqnkb.fsf@soliton.wins.uva.nl \
--to=kettenis@science.uva.nl \
--cc=gdb-patches@sources.redhat.com \
--cc=smid@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox