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: [PATCH v2 1/3] Add min size to regset section iterations


On 2018-08-07 07:01, Alan Hayward wrote:
It’s probably clearer if I explain the SVE specific case:

With SVE when we read the section from the core file it will give us
one of two things:
1) SVE header followed by a full SVE register dump (size dependant on
register size)
Or
2) SVE header followed by a neon register dump (I usually refer to
this as a fpsimd dump).

The second is a shortcut used by the kernel if the process hadn't run
any SVE code, and
all the SVE state is null. This structure is significantly smaller
than the SVE dump.

In the common gdb supply code, it will assert if the size of the
section read from the core
file is smaller than the given size. So for SVE I need to set the size
to the fpsimd size
to prevent this.

Ok, I could peak ahead into the core file to see what type of dump it
is. But, at the
point of the _iterate_over_regset_sections() we don’t have any access
to the core file.

In the common collect code, it uses the size to allocate memory for
writing the dump into.
For this we always want to write out a full SVE dump, so need the size
of the first dump.


If that makes sense now, I can rework it into the comments.

Ok, it kind of make sense but I still find the naming (min_size and size) confusing. I can't quickly tell what is used when.

From what I understand, when supplying, you only care about min_size. If REGSET_VARIABLE_SIZE is not set, min_size is actually not a min_size, but the expected size (anything else than this size will be rejected). When collecting, you only care about "size", to allocate the memory for the dump.

Therefore, I am starting to think the semantic is more straightforward (to me at least) if we named them supply_size and collect_size (which you mentioned in the original patch message). This would make it somewhat clear that if you are in a supply scenario, collect_size is meaningless (and vice versa). It becomes a bit simpler to explain:

- When supplying fixed-size regsets (!REGSET_VARIABLE_SIZE), supply_size is the exact expected size. - When supplying variable-size regsets (REGSET_VARIABLE_SIZE) supply_size is actually just a minumum, because we don't know what we will actually find in the section yet. - When collecting, we know the size in advance (since we know what we will dump), so collect_size is always the exact size that will be dumped.

In core_target::get_core_register_section (or maybe somewhere else), we can assert that

  if (!variable_size_section)
    gdb_assert (supply_size == collect_size);


On a different track, did you consider keeping a single "size" parameter to gdbarch_iterate_over_regset_sections, but add one to indicate whether the caller intends to supply or collect registers? And then, in aarch64's implementation, pass different sizes in the supply/collect cases? Most other arch implementations would simply ignore this parameter and always pass the same size, as they do today.

Simon


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