This is the mail archive of the gdb-patches@sourceware.cygnus.com 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]

Re: [eliz@is.elta.co.il: GDB: fine print in 387 info]



> > Could you please send me the patch that you are going to apply, or
> > tell me where can I look at it?  I could then see if the small changes
> > I suggested are still needed, and if so, create diffs that would apply
> > cleanly after Bill's.
> 
> Sure.  They're under:
> 
> http://www.linuxsupportline.com/~billm/index.html

The new code indeed solves the problems with printing FPU info, except
for one small gripe: it prints the description of the status word that
ends with a semi-colon when the lower 8 bits of the status word are
zero (i.e., no exception flags are set).  If some exception bits _are_
set, the line does not end with a semi-colon.  I think the semi-colon
should either always be printed or never be printed.

But beyond the above, these patches present several problems, at least
as far as the DJGPP version of GDB is concerned:

  - tm-linux.h #define's FPDATA_REGNUM which isn't defined by any
    other platform (at least as far as I could see in gdb 4.18
    sources).  It then goes ahead and uses FPDATA_REGNUM in
    i387-tdep.c, in function i387_print_register, which isn't
    Linux-specific.  This will cause link errors on other x86-based
    platforms.

    In addition, even if FPDATA_REGNUM is added to other platforms, it
    will cause trouble because its use in i387_print_register assumes
    that the FP data registers are last in the list of the CPU
    registers.  This isn't true for DJGPP and for other platforms.

    i387_print_register also uses intimate knowledge about how FP
    registers are layed out.  Here's an excerpt from the patches that
    illustrates this (there are a few others as well):

	       if ( regnum == FP0_REGNUM )
	 	 print_387_control_bits (val);
	       else if ( regnum == FP0_REGNUM + 1)
	 	 print_387_status_bits (val);

    The assumptions used in such fragments are only true for the
    layout of FP registers as defined by tm-linux.h.

  - tm-linux.h #define's FPREG_RAW_SIZE.  This isn't defined by other
    platforms, so the #error directive in i387-tdep.c will fire and
    prevent it from being compiled.  There's also an if clause that
    tests at run time that REGISTER_RAW_SIZE == FPREG_RAW_SIZE for the
    8 FP data registers.

  - tm-linux.h #define's HEX_FLOAT_INPUT and a function that goes with
    it is defined on i387-tdep.c.  However, tm-go32.h defines a
    similar macro HEX_LONG_DOUBLE_INPUT which calls a different
    function, and the new function i387_hex_float_input (that is
    called by HEX_FLOAT_INPUT), defined on i387-tdep.c, isn't
    conditioned on HEX_FLOAT_INPUT, which makes it unused code for
    non-Linux x86-based platforms.

  - tm-linux.h #define's FLOAT_INFO to call the new function
    i387_float_info, which is also defined unconditionally on
    i387-tdep.c--again an unused code.  For example, DJGPP defines a
    similar function called i386_go32_float_info (on go32-nat.c) and
    nm-go32.h #define's FLOAT_INFO to call that function.

To summarize: these patches seem to convert i387-tdep.c, which until
now was largely platform-independent and could be used by any
x87-based target, into a Linux-specific code.  Is that really the
intent here?  I would expect the new code to unify different platforms
that use x87, not to segregate them.

Assuming that the new i387_float_info is indeed meant to serve all
x87-based targets, and the above problems will be solved to allow
platforms other than Linux to use it, I have a few general remarks on
the format used by i387_float_info and its subroutines:

  - I'm not sure it is a good idea to print the 8 FP data registers in
    the top-relative order.  This produces a display where the stack
    top seems to be always in the same place, which might confuse
    users.  In particular, top-relative order makes the printed top
    pointer confusing, since e.g. "top: 5" doesn't mean that st5 is
    the top of stack.

    In contrast, all platform-specific source files in GDB 4.18 print
    the x87 registers in their physical order, from R7 down to R0, and
    mark the top of stack with "=>", which produces stack-like
    behavior whereby the stack top moves down and up as operands are
    pushed and popped.  I think this way of presenting the registers
    is better.

  - I would also suggest to separate the code segment from the opcode
    byte in the printed representation, and maybe even print the
    mnemonic of the opcode.

  - Perhaps the 4 condition code flags should be also printed in the
    same style like exception flags, especially since C3 is disjoint.

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