[10/20] dwarf-reader: read_context: use new symtab in *_symbols_is_exported

Message ID 20210127125853.886677-11-maennich@google.com
State Committed
Headers
Series Refactor (k)symtab reader |

Commit Message

Matthias Männich Jan. 27, 2021, 12:58 p.m. UTC
  Testing whether a symbol is exported can be simplified using the new
symtab implementation. The same holds true for whether a symbol is
exported via ksymtab in case of linux kernel binaries. So, do that.

	* src/abg-dwarf-reader.cc (function_symbol_is_exported): Use new
	  symtab implementation.
	  (variable_symbol_is_exported): Likewise.

Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
---
 src/abg-dwarf-reader.cc | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)
  

Comments

Dodji Seketeli March 15, 2021, 6:13 p.m. UTC | #1
Matthias Maennich <maennich@google.com> a écrit:

> Testing whether a symbol is exported can be simplified using the new
> symtab implementation. The same holds true for whether a symbol is
> exported via ksymtab in case of linux kernel binaries. So, do that.
>
> 	* src/abg-dwarf-reader.cc (function_symbol_is_exported): Use new
> 	  symtab implementation.
> 	  (variable_symbol_is_exported): Likewise.

Note that just applying this patch regressed tests/runtestreaddwarf,
especially while reading the binary
data/test-read-dwarf/test-libandroid.so, which is an ARM32 binary.

This is because for this patch to work, it needs to adaptation of this
patch:

    commit 32c7829e4156666e1975329fb1b3201c43b5f84f
    Author: Giuliano Procida <gprocida@google.com>
    Date:   Tue Mar 9 22:57:54 2021 +0000

        DWARF reader: Interpret ARM32 ELF addresses correctly

        Bug 27552 - libabigail needs to interpret ARM32 symbol addresses specially

        The ARM32 ELF specification specifies that bit 0 of an ELF function
        address is a flag specifying whether the instructions are Thumb or
        ARM. So clear this bit before using the addresses for symbol mapping.

                * src/abg-dwarf-reader.cc
                (read_context::load_symbol_maps_from_symtab_section): Clear
                bit zero of ARM32 function addresses.
                * src/abg-elf-helpers.cc (architecture_is_arm32): Add new
                function.
                * src/abg-elf-helpers.h (architecture_is_arm32): Likewise.
                * tests/data/test-read-dwarf/test-libandroid.so.abi: Update.

By adaptation, I mean that the the bit 0 of the ELF function address
must be cleared in symtab::load in the abg-symtab-reader.cc file as well.

> Reviewed-by: Giuliano Procida <gprocida@google.com>
> Signed-off-by: Matthias Maennich <maennich@google.com>

OK to apply to master once the previous patches, including a patch doing
the above, are applied.

Thanks!

[...]

Cheers,
  

Patch

diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 9f533e64959c..3a93fe0df5d0 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -5807,11 +5807,11 @@  public:
   elf_symbol_sptr
   function_symbol_is_exported(GElf_Addr symbol_address) const
   {
-    elf_symbol_sptr symbol = lookup_elf_fn_symbol_from_address(symbol_address);
+    elf_symbol_sptr symbol = symtab()->lookup_symbol(symbol_address);
     if (!symbol)
       return symbol;
 
-    if (!symbol->is_public())
+    if (!symbol->is_function() || !symbol->is_public())
       return elf_symbol_sptr();
 
     address_set_sptr set;
@@ -5820,16 +5820,8 @@  public:
 
     if (looking_at_linux_kernel_binary)
       {
-	if ((set = linux_exported_fn_syms()))
-	  {
-	    if (set->find(symbol_address) != set->end())
-	      return symbol;
-	  }
-	if ((set = linux_exported_gpl_fn_syms()))
-	  {
-	    if (set->find(symbol_address) != set->end())
-	      return symbol;
-	  }
+	if (symbol->is_in_ksymtab())
+	  return symbol;
 	return elf_symbol_sptr();
       }
 
@@ -5847,11 +5839,11 @@  public:
   elf_symbol_sptr
   variable_symbol_is_exported(GElf_Addr symbol_address) const
   {
-    elf_symbol_sptr symbol = lookup_elf_var_symbol_from_address(symbol_address);
+    elf_symbol_sptr symbol = symtab()->lookup_symbol(symbol_address);
     if (!symbol)
       return symbol;
 
-    if (!symbol->is_public())
+    if (!symbol->is_variable() || !symbol->is_public())
       return elf_symbol_sptr();
 
     address_set_sptr set;
@@ -5860,16 +5852,8 @@  public:
 
     if (looking_at_linux_kernel_binary)
       {
-	if ((set = linux_exported_var_syms()))
-	  {
-	    if (set->find(symbol_address) != set->end())
-	      return symbol;
-	  }
-	if ((set = linux_exported_gpl_var_syms()))
-	  {
-	    if (set->find(symbol_address) != set->end())
-	      return symbol;
-	  }
+	if (symbol->is_in_ksymtab())
+	  return symbol;
 	return elf_symbol_sptr();
       }