Re: [PATCH v2 3/4] Make sure that sorting does not change section order

Simon Marchi
2018-06-26T13:26:28Z

On 2018-06-26 01:09, Petr Tesarik wrote:
>> It took me a while to acknowledge that this was correct, because
>> other_sections::sectindex usually refers to the section index in the
>> BFD. After digging I understood that this field was actually unused
>> until filled by addr_info_make_relative, and that you kind of
>> re-purposed it. It sounds like there should be some comment at
>> other_sections::sectindex and probably in add_symbol_file_command to
>> explain how it's used.
>
> Agreed. As a matter of fact, it also took me some while to understand
> why add_symbol_file_command could get away with setting the index to
> zero for all sections...
>
>> Another option would be to use std::stable_sort instead of std::sort.
>> But it's more resource-hungry and not needed for all paths that lead
>> to
>> addrs_section_sort, so it would be a bit wasteful.
>
> Yes, I tried to avoid that solution. OTOH it's unlikely that there are
> any object files with more than a few dozen sections, and to my best
> knowledge this code is never in the GDB hot path, so if you prefer
> std::stable_sort for clarity, I'm not against. Please, advise.
>
> Petr T

I think you solution is fine, it just needs to be documented that it's
intentional that we use some other value than the section index in
sectindex.

Simon