Re: bitpos expansion patches summary

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Fri, 10 Aug 2012 03:43:38 +0200, Siddhesh Poyarekar wrote:
> On Thu, 9 Aug 2012 22:03:56 +0200, Jan wrote:
> >  +  if (!TYPE_LENGTH_FITS_SIZET (type))
> >  +    error (_("Target object too large for host GDB memory."));
> > +
> > +Please print the sizes.  Also this message is present at multiple
> > places so +maybe rather make a function for unconditionally printing
> > the error? +
> 
> Could you please give an example of this? I didn't think that there are
> any such checks in the source yet.

      if (accumulate_size < 0)
        error (_("Total size of arguments too large for host GDB memory."));

It also should print the sizes, in some cases GDB would print the first error
above, in other cases the second error, I do not think there is a difference.


> > +Make it TYPE_LENGTH_FITS_SIZE_T, please.  And I think this macro is
> > not needed, +inline it.  (It does not access internal fields of the
> > type structures.) +
> 
> I had kept it for possible future need if someone wants to only check
> if a type fits in and not throw an error. I will inline it.

OK, I understand now the goal.  But GDB does not accept additional code which
is useful only with future extension.


> > +And (a) check it already for ssize_t.  Because the code is not safe
> > enough to +properly handle unsigned sizes everywhere without
> > overflows.  (b) Make there +some reserve, anything close to ssize_t
> > will never get successfully xmalloc-ed +anyway.  Some maximum size
> > could be: ((size_t) -1) / 4.  Depending on SSIZE_MAX +may not be
> > compatible enough I guess. +
> 
> I had thought of that, but I figured that instead of guessing a value,
> I would be better off only doing the size_t check (i.e. code sanity) and
> leave the question of whether a type gets successfully malloc'd or not
> to the OS.

The problem is if you futher add something to the value it overflows, xmalloc
will then get few bytes, which succeeds, but GDB behavior is wrong then.
You have correctly put in alpha-tdep.c the new check for such case:
      /* Check for overflow.  */
      if (accumulate_size < 0)
        error (_("Total size of arguments too large for host GDB memory."));

But I have doubts one can catch such overflows across the whole GDB codebase,
so I see some 'safe enough margin' for overflows to be more universal check
for it.


> For ssize_t, I could add an extra boolean argument to
> type_fits_size_t_or_error that indicates whether the type is signed or
> not.

We discussed before that the larger size of unsigned types vs. signed types is
not useful.  GDB will never successfully allocate more than 2GB on 32-bit host
(size_t max larger than ssize_t).  And it does not make sense to consider
anyhow sizes above 9 quintillion (ULONGEST max larger than LONGEST on any host
or size_t larger than ssize_t on 64-bit hosts)


Thanks,
Jan