This is the mail archive of the gdb-patches@sourceware.org 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]
Other format: [Raw text]

Re: [unavailable values part 1, 03/17] expose list of available ranges to common code


On Mon, 07 Feb 2011 15:29:18 +0100, Pedro Alves wrote:
> For the record, I tried making all this range handling be
> done with addrmaps, but, I found out it was much more
> trouble than it's worth.

I wanted to suggest it first, even if the code may be larger those are just
stubs while the expensive bug-prone core could exist in a single instance.

Moreover coreaddr is still more effective, at least algorithmically.
(not a concern here as you state.)


> +/* qsort comparison function, that compares mem_ranges.  */

"and sorts them in ascending order according to their START."


> +void
> +normalize_mem_ranges (VEC(mem_range_s) *ranges)

/* This function must not use any VEC operation on RANGES which may reallocate
   the memory block as the callers keep the original memory location.  */


> +{
> +  if (!VEC_empty (mem_range_s, ranges))
> +    {
> +      struct mem_range *ra, *rb;
> +      int a, b;
> +
> +      qsort (VEC_address (mem_range_s, ranges),
> +	     VEC_length (mem_range_s, ranges),
> +	     sizeof (mem_range_s),
> +	     compare_mem_ranges);
> +
> +      a = 0;
> +      ra = VEC_index (mem_range_s, ranges, a);
> +      for (b = 1; VEC_iterate (mem_range_s, ranges, b, rb); b++)
> +	{
> +	  /* If mem_range B overlaps or is adjacent to mem_range A,
> +	     merge them.  */
> +	  if (rb->start <= ra->start + ra->length)
> +	    {
> +	      ra->length = (rb->start + rb->length) - ra->start;
> +	      continue;		/* next b, same a */
> +	    }

Here if `ra->start == rb->start && ra->length > rb->length' this normalization
will lose the `ra->length - rb->length' part.

Not sure if gdbserver can generate such data but at least this function looks
general enough so it should behave general enough.


> +struct mem_range
> +{
> +  /* Lowest address in the range.  */
> +  CORE_ADDR start;
> +
> +  /* Length of the range.  */
> +  int length;
> +};

Why couldn't GDB become 64bit clean - that is CORE_ADDR length.  It is not
supported by the internal gdbserver representation but this structure tries to
be more general.


> +/* Return in RESULT, the set of collected memory in the current
> +   traceframe, found within the LEN bytes range starting at MEMADDR.
> +   Returns true if the target supports the query, otherwise returns
> +   false.  */
            (and RESULTS remains always NULL).
> +
> +int
> +traceframe_available_memory (VEC(mem_range_s) **result,
> +			     CORE_ADDR memaddr, ULONGEST len)

ULONGEST?  It is `int' in `struct mem_range', it should just match the type of
`memaddr'.


> +{
> +  struct traceframe_info *info = get_traceframe_info ();
> +
> +  if (info != NULL)
> +    {
> +      struct mem_range *r;
> +      int i;
> +
> +      *result = NULL;

I would make this line unconditional.  Or the function comment should be
different.


> +      for (i = 0; VEC_iterate (mem_range_s, info->memory, i, r); i++)
> +	if (mem_ranges_overlap (r->start, r->length, memaddr, len))
> +	  {
> +	    ULONGEST lo1, hi1, lo2, hi2;
> +	    struct mem_range *nr;
> +
> +	    lo1 = memaddr;
> +	    hi1 = memaddr + len;
> +
> +	    lo2 = r->start;
> +	    hi2 = r->start + r->length;
> +
> +	    nr = VEC_safe_push (mem_range_s, *result, NULL);
> +
> +	    nr->start = max (lo1, lo2);
> +	    nr->length = min (hi1, hi2) - nr->start;
> +	  }
> +
> +      normalize_mem_ranges (*result);
> +      return 1;
> +    }
> +
> +  return 0;
> +}
> +


Thanks,
Jan


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