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

[Paul Hilfinger <Hilfinger@adacore.com>]

Tom,

Thanks for the (blindingly) fast response.  Is Joel blackmailing you or 
something?  I could have a word with him if so.

> 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?

In fact, at one point I HAD removed this function, but then discovered to
my annoyance that it was still needed somewhere.  I'll have to go back and
reconstruct (from some months back) just what the problem was and whether it
has become OBE.

> 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.

Yeah, it seems that long long either appears in system-specific code (like
linux-nat) where one can probably assume its existence, or conditionalized
as in printcmd.c.

> 
> [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.
> 

Hmm.  Sticky.  There's this comment on 'length' in struct type:

  /* Length of storage for a value of this type.  This is what
     sizeof(type) would return; use it for address arithmetic,
     memory reads and writes, etc. ....

But then the code goes on to define 'unsigned length'.  However, according 
to the C standard, the type should really be size_t---and technically
that should be the TARGET's size_t.  I suspect this part of the representation 
hasn't been updated since the days when people actually used 16-bit address
spaces.  What do you suppose people would say to using size_t here at least?

In any case, you are correct that there ought to be some provision for 
overflow here.

> 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?

That also will require some thought.  Some undefined bounds seem to be 
carelessness on the part of the compiler, and I suspect that returning a
null range might be safer.

> 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.

OK and OK.  

Thanks for the input.  I should be submitting a revision soon.

Paul