S390 gdb patches

Andrew Cagney ac131313@cygnus.com
Tue Nov 21 16:46:00 GMT 2000


[JimB, note the assignment issue.  Feel free to correct :-)]

DJBARROW@de.ibm.com wrote:

> What is the logic behind not using TRUE or FALSE definitions ?

Logic?  Er, you think codeing standards are actually logical? :-)

There are two schools of thought.  This code just happened to end up in
one of them.  In this school, boolean expressions are written as:

	valid_p = 1;
	if (valid_p)
	while (!valid_p)

rather than:

	if (valid_P == TRUE)

The main rationale comes from two observations: C doesn't have an
especially strong type system; and in C there is no boolean type, any
non-zero value is considered true.

Opinion suggests that it is better to not try to lie about this aspect
of the C language. Instead of trying to paper over the nastness (and
make it look like Pascal) this problem should be exposed in its full
garish glory.  The position is also consistent with the ``lowest common
demonator'' rule - you can't be sure that everyone is using TRUE/FALSE
but you can be sure that C's boolean rules still apply.

The standard example is to observe that ``valid_p == TRUE'' may not
actually do as you expect :-)

Disclaimer - this is my memory of one of N different variants on the
debate.  My personal opinion is that the decision was a good one.  It is
better to not try to paint over such problems - eventually the paint
starts to peel and you end up with an even bigger mess.

> >s390-tdep.c includes <netinet/in.h>
> >
> >    I can't see any reason for this (well except
> >    to get at the htonl() macros and you shouldn't
> >    be using them).
> 
> This probably can be worked around, have you any better ideas ?

(Er, I was actually guessing).

GDB has a suite of functions that can convert between target
byte-ordered memory and host values. Check the extract_*() functions
such as extract_unsigned_integer().  These functions work with any
combination of host/target byte ordering.

> >tm-s390.h
> >
> >    If I'm reading the code right, much of this
> >    file is conditional on WANT_S390_TGT_DEFS.
> >    That code should be deleted.
> 
> We already did talk about this I said I wanted to keep the
> old definitions e.g. FRAME_CHAIN etc.
> As I believe if the multiarch was mature it could generate this
> code from the macros & this is the only stuff that is currently
> documented.

Um, yes.  Sorry.  Can you make it conditional on ``GDB_MULTI_ARCH''
rather than adding your own macro?  You should be able to enable/disable
multi-arch by configuring with ``--enable-gdb-multi-arch={yes,no}''.

> To be honest it will not fit for s390, I've only implemented
> this stuff to get it accepted e.g. when running 31 bit programs
> on a 64 bit machine we will produce 31 bit core dumps.
> I also presume sparc has this problem whatever the claims are
> unless they came up with a generic core dump format ( unlikely ).
> This would also require multiple regset definitions.
> You mentioned on a web page the idea behind multi arch
> was to run on multiple different processors within the same
> box & gave playstation 2 as an example, it is  short of this goal
> & I cannot see why we have this interface rammed down our throat if
> it simply isn't mature.

I believe the issues have (or are) being solved.  I know that KevenB
recently made significant changes to the shared library code so that it
handed 32/64 bit problems at run time.

If in the first instance you need two targets then that is ok.  However,
you need to avoid doing anything that locks the code down making it
impossible to multi-arch.  If that happens then the code will quickly
suffer bit rot and be left behind.

> > I'm not clear on the copyright status of the files:
> >         s390-gdbregs.
> >         s390-regs-common.h
> >         sigcontext.h
> >    which were lifted from the Linux kernel.
> >
> >    Does IBM currently control the copyright on those
> >    files?
> I'm no legal genius,
> Well they are under GPL so I think Richard Stallman
> & Linus can fight about ownership & may the best loyar win.
> The files are required to.
> 1) To keep the kernels register formats compatible with gdb.
> 2) Remote debugging.
> 3) Backchaining out of signal handlers.

I'm not a lawyer either.  However, I do know (and I'm probably going to
get flamed for suggesting this) that the Linux kernel group are no where
near as strict as the FSF is when it comes to copyright assignment. 
Because something is in the Linux kernel, it isn't safe to assume that
the FSF own the code and it can be included in the GDB sources.

If the heratage of these files isn't clear then I can't accept them :-(

Other targets have created local definitions that mirror the kernel
structures and this has worked.  This might run against the grain. 
However, remember, the kernel can't (ok shouldn't) change fundamental
interfaces such as this without also changing the system call interface.

With regard local/remote there is a general problem that hasn't been
solved.

> >    __u8 et.al. are used liberally.  Within GDB
> >    I believe that ``bfd_byte'' is used.
> 
> I like these definitions what happens if I want
> a signed or unsigned bfd_byte. I'll change this ..... grudgingly.

It depends.  The file src/sim/common/sim-types.h provides a full type
system.  Within GDB, I think it is proving to be better to just stick
with the more generic bfd_byte, LONGEST, ULONGEST and CORE_ADDR.

There may be an argument for a full type system so that some target
dependant structures can be better described.  I suspect, however, that
given host/target dependant struct packing rules, sticking to the above
is probably more prudent.  The shared library code did this.

> >gdbarch.sh:
> >    I'll figure out what macros you needed and multi
> >    arch them my self (avoid assignment :-)
> 
> I cannot see any point in rewriting my gdbarch.sh macros unless ,
> 1) You find something wrong with them or they require improvment, fair
> enough.
> 2)  Don't rewrite them just to claim "ownership" of them.

I'm just trying to make both of our lives easier :-)  The actual changes
made to gdbarch.sh are cosmetic so really don't need an assignment.  If
I ignore your patches beyond noting which macro's need converting we can
at least get that code in now.

	Andrew


More information about the Gdb-patches mailing list