Re: [RFA] 64-bit range types in GDB

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Paul" == Paul Hilfinger <Hilfinger@adacore.com> writes:

Paul> It required doing some violence to struct main_type,
Paul> unfortunately, but as a side effect, the representation of range
Paul> types is a bit less of an abuse of abstraction (IMHO, anyway).

I agree, I think this is better.

Overall this patch looks great.  I have a couple nits to pick.

Paul> I apologize to those of you who will have to modify your
Paul> type-manipulation routines in .gdbinit files.

I'm interested in seeing this sort of thing, so if you have gdb-specific
gdbinit hacks, please email them to me :-)

Paul> +/* Fix the RANGE_TYPE flags if we think they are incorrect.
Paul> +   This is a temporary (?) hack to work around problems with handling 
Paul> +   of >32bit range types on older compilers. On pure 32-bit hosts, 
Paul> +   the compiler does not always emit the bounds as expected. 
Paul> +   FIXME: pnh/2009-08-05. */
Paul> + 
Paul> +static void
Paul> +fixup_range_type_hack (struct type *range_type, struct die_info *die,
Paul> +                       struct dwarf2_cu *cu)

On irc, Joel said that he thought that this was perhaps no longer
needed.  Could you comment?

Paul> +         _("Suspicious DW_AT_byte_size value treated as zero instead of 0x%llx"),
Paul> +         (long long) DW_UNSND (attr));

Are %ll and `long long' really portable?  I think you need something
else here.  There are a few cases.

[check_typedef]
Paul> -	  const int low_bound = TYPE_LOW_BOUND (range_type);
Paul> -	  const int high_bound = TYPE_HIGH_BOUND (range_type);
Paul> +	  const LONGEST low_bound = TYPE_LOW_BOUND (range_type);
Paul> +	  const LONGEST high_bound = TYPE_HIGH_BOUND (range_type);
Paul>  	  int nb_elements;
 	
Paul>  	  if (high_bound < low_bound)
Paul>  	    nb_elements = 0;
Paul>  	  else
Paul> -	    nb_elements = high_bound - low_bound + 1;
Paul> +	    nb_elements = (int) (high_bound - low_bound + 1);

Can't this overflow?  There's a subsequent multiplication, too...

I don't know what would be best to do here.  Changing TYPE_LENGTH to a
LONGEST seems tough to swallow, but so does throwing an error.

Also, what does this code mean if either the high or low bound is
undefined?  Maybe those cases should choose the min or max of the
underlying integral type?

Paul> +    struct range_bounds {

Brace on its own line.

Paul> +      TYPE_RANGE_DATA (tp) = ((struct range_bounds *)
Paul> +			  TYPE_ALLOC (tp, sizeof (struct range_bounds)));
Paul> +      memset (TYPE_RANGE_DATA (tp), 0, sizeof (struct range_bounds));

Use TYPE_ZALLOC instead.

thanks,
Tom