This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [unavailable values part 1, 03/17] expose list of available ranges to common code
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Pedro Alves <pedro at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 14 Feb 2011 12:59:39 +0100
- Subject: Re: [unavailable values part 1, 03/17] expose list of available ranges to common code
- References: <201102071429.19096.pedro@codesourcery.com>
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