Rebased to master e96bd93d436e464a532a7e1161e1d201c9fc50c7


On 13-04-03 03:32 PM, Jan Kratochvil wrote:
> On Tue, 02 Apr 2013 18:22:48 +0200, Aleksandar Ristovski wrote:
>>>> +static int
>>>> +find_phdr_p (const void *const phdr, const int is_elf64,
>>>> +		const void *const data)
>>>
>>> But I do not understand why this function exists - it could be integrated in
>>> find_phdr as very every caller of find_phdr passse as find_phdr_p parameter
>>> this find_phdr_p implementation.
>>
>>
>> [AR] Yes, but I am eyeing solib-svr4.c loops over pheaders
>> generalization - find_phdr could be moved to 'common' and possibly
>> called 'iterate_phdrs' with the ability to pass in any function, not
>> necessarily a predicate only. E.g. svr4_exec_displacement, etc...)
>
> It is not relevant for this patch what do you plan.  In the form as is it is
> needlessly overcomplicated.  That more thorough generalization would be
> possibly good but it is not here and it may never happen.  GDB will just
> remain with needlessly complicated code.
>
>
>
>>> You need to check here also the name content, it is "GNU\x00" (n_namesz == 4).
>>
>> [AR], can it be any other name if type is NT_GNU_BUILD_ID? Added the
>> check but seems like an overkill to me.
>
> NT_GNU_BUILD_ID is just number 3.  I find very realistic for example
> "SUNW Solaris" would use at least 3 note types (does not use it already?).
>
>
>>>> +/* Linear reverse find starting from RBEGIN towards REND looking for
>>>> +   the lowest vaddr mapping of the same inode and zero offset.  */
>>>> +
>>>> +static mapping_entry_s *
>>>> +lrfind_mapping_entry (mapping_entry_s *const rbegin,
>>>> +		      const mapping_entry_s *const rend)
>>>> +{
>>>> +  mapping_entry_s *p;
>>>> +
>>>> +  for (p = rbegin - 1; p >= rend && p->inode == rbegin->inode; --p)
>>>> +    if (p->offset == 0)
>>>> +      return p;
>>>
>>> I had here this layout:
>>> 7ffff7ddc000-7ffff7dfd000 r-xp 00000000 fd:01 51415762                   /usr/lib64/ld-2.17.so
>>> 7ffff7ff9000-7ffff7ffa000 rw-p 00000000 00:00 0
>>> 7ffff7ffa000-7ffff7ffc000 r-xp 00000000 00:00 0                          [vdso]
>>> 7ffff7ffc000-7ffff7ffe000 rw-p 00020000 fd:01 51415762                   /usr/lib64/ld-2.17.so
>>>
>>> so it should rather be:
>>>    for (p = rbegin - 1; p >= rend; --p)
>>>      if (p->offset == 0 && p->inode == rbegin->inode)
>>>        return p;
>>>
>>> Fortunately it should not have any real performance impact.
>>
>> [AR] Ok, though we are not adding mappings with inode == 0. See
>> 'find_memory_region_callback'.
>
> Hmm, that's true, I do not see now why it failed for me.  But that could be
> even non-zero-inode mapping so a change was appropriate.
>
>
> This is not yet a full review.
>
>
> Thanks,
> Jan
>