This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] MIPS: Avoid breakpoints in branch delay slots
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Tom Tromey <tromey at redhat dot com>
- Cc: Joel Brobecker <brobecker at adacore dot com>, <gdb-patches at sourceware dot org>, Chris Dearman <chris at mips dot com>
- Date: Mon, 27 Feb 2012 23:05:47 +0000
- Subject: Re: [PATCH] MIPS: Avoid breakpoints in branch delay slots
- References: <alpine.DEB.1.10.1111221734400.4191@tp.orcam.me.uk> <20111205105852.GD2777@adacore.com> <alpine.DEB.1.10.1112051335240.6710@tp.orcam.me.uk> <20111209091531.GM21915@adacore.com> <m3pqfxfyi9.fsf@fleche.redhat.com>
On Fri, 9 Dec 2011, Tom Tromey wrote:
> Joel> I am having doubts again, but I think we do require C90 to compile GDB.
>
> We do.
>
> Joel> It's C99 that is not mandatory yet (hence the use of gnulib for some of
> Joel> the C99 headers). Based on that, can we consider that we're always going
> Joel> to have a 64bit "long long"?
>
> I think we can't.
>
> If debugging a particular target needs a 64 bit type on the host, and
> such a type isn't available, then there is just no way to do that
> debugging. But, this situation is already accounted for, see
> ALL_64_TARGET_OBS in gdb/Makefile.in.
>
> I think this setup is increasingly rare, though.
>
> Joel> The code looks good to me. I will let you decide which version you
> Joel> think is going to be better to maintain. I was just afraid that you
> Joel> needed to know what the *target* address size was, in which case the
> Joel> define would have been the wrong solution.
>
> I didn't really read the thread, but I would much prefer following the
> existing approach over adding more BFD64 checks, if possible.
The problem here is a piece of code has to handle target addresses using
conditionals against values both inside and outside the range possible to
express with a 32-bit host data type. It is possible to do so if
CORE_ADDR is 64-bit data type which can be qualified with BFD64 at the
preprocessor level. Otherwise addresses outside the 32-bit range will
never be considered and the respective conditionals will not be relevant
anymore, but these within that range still need to work.
All of this can be done by checking the width of CORE_ADDR at the
compiler level instead which I am actually all in favour to, but that
requires some care because when CORE_ADDR is a 32-bit data type, the
compiler will produce warnings about a shift amount exceeding the width of
the data type operated on even though it only happens in dead code
(perhaps dumber compilers will only evaluate expressions whose value is
known at the compilation time, such as ones involving sizeof, at the run
time and then actually emit conditional code that is supposed to be dead,
hmm?).
Hence this strange:
bpaddr >> ((sizeof (CORE_ADDR) << 3) - 2)
calculation that makes sure the right amount is used. Frankly, on second
thoughts, I could have used an expression like this:
bpaddr >> (sizeof (CORE_ADDR) == 8 ? 62 : 0)
But in the context of the:
if (sizeof (CORE_ADDR) == 8)
conditional this expression is calculated in it wouldn't be any less
obscure, so I have only updated the comment to clarify what is going on
here to say:
/* Get the topmost two bits of bpaddr in a 32-bit safe manner (avoid
a compiler warning produced where CORE_ADDR is a 32-bit type even
though in that case this is dead code). */
Committed now with this change, thanks for your review.
Maciej