[PATCH] abg-dwarf-reader: resolve relocation sections by index

[Matthias Maennich via libabigail]

Hi!

On Tue, Jan 07, 2020 at 09:54:47AM +0100, Dodji Seketeli wrote:
>Hello Jessica, Matthias,
>
>Matthias Maennich <maennich@google.com> a écrit:
>
>> Looking up relocation sections by name introduces a dependency to the
>> linker in use. Relocation sections might be named differently. For
>> instance, linking kernel modules with the bfd linker leads to a
>> .rela__ksymtab section corresponding to the __ksymtab section. Using lld
>> as a linker leads to .rela___ksymtab as section name. Both are valid.
>> When the kernel loads these, it simply applies all relocations from all
>> sections it finds. Tools should not depend on the concrete name (even
>> though I would prefer consistency among them). Libabigail hit an
>> assertion when trying to extract the ABI from a kernel module linked
>> with lld.
>>
>> Hence, resolve the relocation sections for __ksymtab and __ksymtab_gpl
>> by iterating over the ELF sections, searching for relocation sections
>> and identifying the one that points to the respective ksymtab.
>>
>> 	* src/abg-dwarf-reader.cc (find_relocation_section): New function.
>> 	(find_ksymtab_reloc_section): Use find_relocation_section to
>> 	resolve the ksymtab's relocation section.
>> 	(find_ksymtab_gpl_reloc_section): Likewise.
>
>Thank you for working on this.  I definitely agree with the idea of this
>patch.
>
>[...]
>
>
>Jessica Yu <jeyu@kernel.org> a écrit:
>
>[...]
>
>>>diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
>>>index 6f8f5beb3b30..4065b82512ba 100644
>>>--- a/src/abg-dwarf-reader.cc
>>>+++ b/src/abg-dwarf-reader.cc
>>>@@ -6167,6 +6167,35 @@ public:
>>>     return ksymtab_section_;
>>>   }
>>>
>>>+  /// Return the .rel{a,} section corresponding to a given section.
>>>+  ///
>>>+  /// @return the .rel{a,} section if found, null otherwise.
>>
>> Perhaps we should document @param target_section as well?
>
>I agree with Jessica's comment here, FWIW.
>
>> Other than that, I agree that using sh_info is a much more robust way
>> of finding the correct relocation section, compared to just guessing
>> the section name. Thanks for the improvement :-)
>
>Well, what else can I say ? :-)
>
>I think this patch should go into master.  So feel free to merge it in
>with the small comment added.

Thanks for the review! Pushed to master.

Cheers,
Matthias

>
>Thank a lot for the improvement.
>
>Cheers,
>
>-- 
>		Dodji
>
>