This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] MIPS: Avoid breakpoints in branch delay slots


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]