elf_symbol: remove "is Linux string constant" property
Commit Message
This boolean property was obsoleted by the new symtab reader
implementation. It has no users.
Following this change, the find_ksymtab_strings_section function joins
find_ksymtab_section and find_ksymtab_gpl_section in having no users.
* include/abg-ir.h (elf_symbol::elf_symbol): Drop
is_linux_string_cst argument.
(elf_symbol::create): Likewise.
(elf_symbol::get_is_linux_string_cst): Drop method.
* src/abg-dwarf-reader.cc (lookup_symbol_from_sysv_hash_tab):
Remove code that gets the index of the __ksymtab_strings
section. Drop corresponding elf_symbol::create argument.
(lookup_symbol_from_gnu_hash_tab): Likewise.
(lookup_symbol_from_symtab): Likewise.
(create_default_fn_sym): Drop false is_linux_string_cst
argument to elf_symbol::create.
* src/abg-ir.cc (elf_symbol::priv): Drop is_linux_string_cst_
member variable.
(elf_symbol::priv default ctor): Drop initialisation of
is_linux_string_cst_.
(elf_symbol::priv normal ctor): Drop is_linux_string_cst
argument and corresponding is_linux_string_cst_
initialisation.
(elf_symbol::elf_symbol ctor): Drop is_linux_string_cst
argument and corresponding forwarding to priv ctor.
(elf_symbol::create): Drop is_linux_string_cst argument and
corresponding forwarding to ctor.
(elf_symbol::get_is_linux_string_cst): Drop method.
* src/abg-reader.cc (build_elf_symbol): Drop false
is_linux_string_cst argument to elf_symbol::create.
* src/abg-symtab-reader.cc (symtab::load): Likewise.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
include/abg-ir.h | 5 -----
src/abg-dwarf-reader.cc | 24 ++++--------------------
src/abg-ir.cc | 23 -----------------------
src/abg-reader.cc | 3 +--
src/abg-symtab-reader.cc | 5 +----
5 files changed, 6 insertions(+), 54 deletions(-)
Comments
On Tue, Jul 06, 2021 at 05:36:02PM +0100, Giuliano Procida wrote:
>This boolean property was obsoleted by the new symtab reader
>implementation. It has no users.
>
>Following this change, the find_ksymtab_strings_section function joins
>find_ksymtab_section and find_ksymtab_gpl_section in having no users.
>
> * include/abg-ir.h (elf_symbol::elf_symbol): Drop
> is_linux_string_cst argument.
> (elf_symbol::create): Likewise.
> (elf_symbol::get_is_linux_string_cst): Drop method.
> * src/abg-dwarf-reader.cc (lookup_symbol_from_sysv_hash_tab):
> Remove code that gets the index of the __ksymtab_strings
> section. Drop corresponding elf_symbol::create argument.
> (lookup_symbol_from_gnu_hash_tab): Likewise.
> (lookup_symbol_from_symtab): Likewise.
> (create_default_fn_sym): Drop false is_linux_string_cst
> argument to elf_symbol::create.
> * src/abg-ir.cc (elf_symbol::priv): Drop is_linux_string_cst_
> member variable.
> (elf_symbol::priv default ctor): Drop initialisation of
> is_linux_string_cst_.
> (elf_symbol::priv normal ctor): Drop is_linux_string_cst
> argument and corresponding is_linux_string_cst_
> initialisation.
> (elf_symbol::elf_symbol ctor): Drop is_linux_string_cst
> argument and corresponding forwarding to priv ctor.
> (elf_symbol::create): Drop is_linux_string_cst argument and
> corresponding forwarding to ctor.
> (elf_symbol::get_is_linux_string_cst): Drop method.
> * src/abg-reader.cc (build_elf_symbol): Drop false
> is_linux_string_cst argument to elf_symbol::create.
> * src/abg-symtab-reader.cc (symtab::load): Likewise.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>
Thanks for clearing up this leftover TODO!
Reviewed-by: Matthias Maennich <maennich@google.com>
Cheers,
Matthias
>---
> include/abg-ir.h | 5 -----
> src/abg-dwarf-reader.cc | 24 ++++--------------------
> src/abg-ir.cc | 23 -----------------------
> src/abg-reader.cc | 3 +--
> src/abg-symtab-reader.cc | 5 +----
> 5 files changed, 6 insertions(+), 54 deletions(-)
>
>diff --git a/include/abg-ir.h b/include/abg-ir.h
>index da43727f..7ab3b2b7 100644
>--- a/include/abg-ir.h
>+++ b/include/abg-ir.h
>@@ -922,7 +922,6 @@ private:
> bool c,
> const version& ve,
> visibility vi,
>- bool is_linux_string_cst = false,
> bool is_in_ksymtab = false,
> uint64_t crc = 0,
> bool is_suppressed = false);
>@@ -948,7 +947,6 @@ public:
> bool c,
> const version& ve,
> visibility vi,
>- bool is_linux_string_cst = false,
> bool is_in_ksymtab = false,
> uint64_t crc = 0,
> bool is_suppressed = false);
>@@ -965,9 +963,6 @@ public:
> void
> set_index(size_t);
>
>- bool
>- get_is_linux_string_cst() const;
>-
> const string&
> get_name() const;
>
>diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
>index c8cd5170..bcb3883a 100644
>--- a/src/abg-dwarf-reader.cc
>+++ b/src/abg-dwarf-reader.cc
>@@ -824,10 +824,6 @@ lookup_symbol_from_sysv_hash_tab(const environment* env,
> elf_symbol::binding sym_binding;
> elf_symbol::visibility sym_visibility;
> bool found = false;
>- Elf_Scn *strings_section = find_ksymtab_strings_section(elf_handle);
>- size_t strings_ndx = strings_section
>- ? elf_ndxscn(strings_section)
>- : 0;
>
> do
> {
>@@ -856,8 +852,7 @@ lookup_symbol_from_sysv_hash_tab(const environment* env,
> sym_binding,
> symbol.st_shndx != SHN_UNDEF,
> symbol.st_shndx == SHN_COMMON,
>- ver, sym_visibility,
>- symbol.st_shndx == strings_ndx);
>+ ver, sym_visibility);
> syms_found.push_back(symbol_found);
> found = true;
> }
>@@ -1103,10 +1098,6 @@ lookup_symbol_from_gnu_hash_tab(const environment* env,
> elf_symbol::type sym_type;
> elf_symbol::binding sym_binding;
> elf_symbol::visibility sym_visibility;
>- Elf_Scn *strings_section = find_ksymtab_strings_section(elf_handle);
>- size_t strings_ndx = strings_section
>- ? elf_ndxscn(strings_section)
>- : 0;
>
> // Let's walk the hash table and record the versions of all the
> // symbols which name equal sym_name.
>@@ -1151,8 +1142,7 @@ lookup_symbol_from_gnu_hash_tab(const environment* env,
> sym_type, sym_binding,
> symbol.st_shndx != SHN_UNDEF,
> symbol.st_shndx == SHN_COMMON,
>- ver, sym_visibility,
>- symbol.st_shndx == strings_ndx);
>+ ver, sym_visibility);
> syms_found.push_back(symbol_found);
> found = true;
> }
>@@ -1275,10 +1265,6 @@ lookup_symbol_from_symtab(const environment* env,
> char* name_str = 0;
> elf_symbol::version ver;
> bool found = false;
>- Elf_Scn *strings_section = find_ksymtab_strings_section(elf_handle);
>- size_t strings_ndx = strings_section
>- ? elf_ndxscn(strings_section)
>- : 0;
>
> for (size_t i = 0; i < symcount; ++i)
> {
>@@ -1307,8 +1293,7 @@ lookup_symbol_from_symtab(const environment* env,
> elf_symbol::create(env, i, sym->st_size,
> name_str, sym_type,
> sym_binding, sym_is_defined,
>- sym_is_common, ver, sym_visibility,
>- sym->st_shndx == strings_ndx);
>+ sym_is_common, ver, sym_visibility);
> syms_found.push_back(symbol_found);
> found = true;
> }
>@@ -14118,8 +14103,7 @@ create_default_fn_sym(const string& sym_name, const environment *env)
> /*symbol is defined=*/ true,
> /*symbol is common=*/ false,
> /*symbol version=*/ ver,
>- /*symbol visibility=*/elf_symbol::DEFAULT_VISIBILITY,
>- /*symbol is linux string cst=*/false);
>+ /*symbol visibility=*/elf_symbol::DEFAULT_VISIBILITY);
> return result;
> }
>
>diff --git a/src/abg-ir.cc b/src/abg-ir.cc
>index f8664566..d3134744 100644
>--- a/src/abg-ir.cc
>+++ b/src/abg-ir.cc
>@@ -1395,7 +1395,6 @@ struct elf_symbol::priv
> // STT_COMMON. If no such symbol is found, it looks for the
> // STT_COMMON definition of that name that has the largest size.
> bool is_common_;
>- bool is_linux_string_cst_;
> bool is_in_ksymtab_;
> uint64_t crc_;
> bool is_suppressed_;
>@@ -1413,7 +1412,6 @@ struct elf_symbol::priv
> visibility_(elf_symbol::DEFAULT_VISIBILITY),
> is_defined_(false),
> is_common_(false),
>- is_linux_string_cst_(false),
> is_in_ksymtab_(false),
> crc_(0),
> is_suppressed_(false)
>@@ -1429,7 +1427,6 @@ struct elf_symbol::priv
> bool c,
> const elf_symbol::version& ve,
> elf_symbol::visibility vi,
>- bool is_linux_string_cst,
> bool is_in_ksymtab,
> uint64_t crc,
> bool is_suppressed)
>@@ -1443,7 +1440,6 @@ struct elf_symbol::priv
> visibility_(vi),
> is_defined_(d),
> is_common_(c),
>- is_linux_string_cst_(is_linux_string_cst),
> is_in_ksymtab_(is_in_ksymtab),
> crc_(crc),
> is_suppressed_(is_suppressed)
>@@ -1490,9 +1486,6 @@ elf_symbol::elf_symbol()
> ///
> /// @param vi the visibility of the symbol.
> ///
>-/// @param is_linux_string_cst true if the symbol is a Linux Kernel
>-/// string constant defined in the __ksymtab_strings section.
>-///
> /// @param crc the CRC (modversions) value of Linux Kernel symbols
> elf_symbol::elf_symbol(const environment* e,
> size_t i,
>@@ -1504,7 +1497,6 @@ elf_symbol::elf_symbol(const environment* e,
> bool c,
> const version& ve,
> visibility vi,
>- bool is_linux_string_cst,
> bool is_in_ksymtab,
> uint64_t crc,
> bool is_suppressed)
>@@ -1518,7 +1510,6 @@ elf_symbol::elf_symbol(const environment* e,
> c,
> ve,
> vi,
>- is_linux_string_cst,
> is_in_ksymtab,
> crc,
> is_suppressed))
>@@ -1562,9 +1553,6 @@ elf_symbol::create()
> ///
> /// @param vi the visibility of the symbol.
> ///
>-/// @param is_linux_string_cst if true, it means the symbol represents
>-/// a string constant from a linux kernel binary.
>-///
> /// @param crc the CRC (modversions) value of Linux Kernel symbols
> ///
> /// @return a (smart) pointer to a newly created instance of @ref
>@@ -1580,13 +1568,11 @@ elf_symbol::create(const environment* e,
> bool c,
> const version& ve,
> visibility vi,
>- bool is_linux_string_cst,
> bool is_in_ksymtab,
> uint64_t crc,
> bool is_suppressed)
> {
> elf_symbol_sptr sym(new elf_symbol(e, i, s, n, t, b, d, c, ve, vi,
>- is_linux_string_cst,
> is_in_ksymtab, crc, is_suppressed));
> sym->priv_->main_symbol_ = sym;
> return sym;
>@@ -1653,15 +1639,6 @@ void
> elf_symbol::set_index(size_t s)
> {priv_->index_ = s;}
>
>-/// Test if the ELF symbol is for a string constant of a Linux binary
>-/// defined in the __ksymtab_strings symbol table.
>-///
>-/// @return true iff ELF symbol is for a string constant of a Linux
>-/// binary defined in the __ksymtab_strings symbol table.
>-bool
>-elf_symbol::get_is_linux_string_cst() const
>-{return priv_->is_linux_string_cst_;}
>-
> /// Getter for the name of the @ref elf_symbol.
> ///
> /// @return a reference to the name of the @ref symbol.
>diff --git a/src/abg-reader.cc b/src/abg-reader.cc
>index 0b8149f6..6af4d404 100644
>--- a/src/abg-reader.cc
>+++ b/src/abg-reader.cc
>@@ -2985,8 +2985,7 @@ build_elf_symbol(read_context& ctxt, const xmlNodePtr node,
> elf_symbol_sptr e = elf_symbol::create(env, /*index=*/0,
> size, name, type, binding,
> is_defined, is_common,
>- version, visibility,
>- /*is_linux_string_cst=*/false);
>+ version, visibility);
>
> e->set_is_suppressed(is_suppressed);
>
>diff --git a/src/abg-symtab-reader.cc b/src/abg-symtab-reader.cc
>index edc9d825..4cc40d28 100644
>--- a/src/abg-symtab-reader.cc
>+++ b/src/abg-symtab-reader.cc
>@@ -315,10 +315,7 @@ symtab::load_(Elf* elf_handle,
> elf_helpers::stb_to_elf_symbol_binding(GELF_ST_BIND(sym->st_info)),
> sym_is_defined, sym_is_common, ver,
> elf_helpers::stv_to_elf_symbol_visibility
>- (GELF_ST_VISIBILITY(sym->st_other)),
>- /*is_linux_strings_cstr=*/false); // TODO: remove
>- // is_linux_strings_cstr
>- // as it is obsolete
>+ (GELF_ST_VISIBILITY(sym->st_other)));
>
> // We do not take suppressed symbols into our symbol vector to avoid
> // accidental leakage. But we ensure supressed symbols are otherwise set
>--
>2.32.0.93.g670b81a890-goog
>
Giuliano Procida <gprocida@google.com> a écrit:
> This boolean property was obsoleted by the new symtab reader
> implementation. It has no users.
>
> Following this change, the find_ksymtab_strings_section function joins
> find_ksymtab_section and find_ksymtab_gpl_section in having no users.
[...]
Matthias Maennich <maennich@google.com> a écrit:
> Thanks for clearing up this leftover TODO!
Indeed, thanks a lot for looking into this!
>
> * include/abg-ir.h (elf_symbol::elf_symbol): Drop
> is_linux_string_cst argument.
> (elf_symbol::create): Likewise.
> (elf_symbol::get_is_linux_string_cst): Drop method.
> * src/abg-dwarf-reader.cc (lookup_symbol_from_sysv_hash_tab):
> Remove code that gets the index of the __ksymtab_strings
> section. Drop corresponding elf_symbol::create argument.
> (lookup_symbol_from_gnu_hash_tab): Likewise.
> (lookup_symbol_from_symtab): Likewise.
> (create_default_fn_sym): Drop false is_linux_string_cst
> argument to elf_symbol::create.
> * src/abg-ir.cc (elf_symbol::priv): Drop is_linux_string_cst_
> member variable.
> (elf_symbol::priv default ctor): Drop initialisation of
> is_linux_string_cst_.
> (elf_symbol::priv normal ctor): Drop is_linux_string_cst
> argument and corresponding is_linux_string_cst_
> initialisation.
> (elf_symbol::elf_symbol ctor): Drop is_linux_string_cst
> argument and corresponding forwarding to priv ctor.
> (elf_symbol::create): Drop is_linux_string_cst argument and
> corresponding forwarding to ctor.
> (elf_symbol::get_is_linux_string_cst): Drop method.
> * src/abg-reader.cc (build_elf_symbol): Drop false
> is_linux_string_cst argument to elf_symbol::create.
> * src/abg-symtab-reader.cc (symtab::load): Likewise.
>
> Signed-off-by: Giuliano Procida <gprocida@google.com>
> Reviewed-by: Matthias Maennich <maennich@google.com>
Applied to master. Thanks!
[...]
Cheers,
@@ -922,7 +922,6 @@ private:
bool c,
const version& ve,
visibility vi,
- bool is_linux_string_cst = false,
bool is_in_ksymtab = false,
uint64_t crc = 0,
bool is_suppressed = false);
@@ -948,7 +947,6 @@ public:
bool c,
const version& ve,
visibility vi,
- bool is_linux_string_cst = false,
bool is_in_ksymtab = false,
uint64_t crc = 0,
bool is_suppressed = false);
@@ -965,9 +963,6 @@ public:
void
set_index(size_t);
- bool
- get_is_linux_string_cst() const;
-
const string&
get_name() const;
@@ -824,10 +824,6 @@ lookup_symbol_from_sysv_hash_tab(const environment* env,
elf_symbol::binding sym_binding;
elf_symbol::visibility sym_visibility;
bool found = false;
- Elf_Scn *strings_section = find_ksymtab_strings_section(elf_handle);
- size_t strings_ndx = strings_section
- ? elf_ndxscn(strings_section)
- : 0;
do
{
@@ -856,8 +852,7 @@ lookup_symbol_from_sysv_hash_tab(const environment* env,
sym_binding,
symbol.st_shndx != SHN_UNDEF,
symbol.st_shndx == SHN_COMMON,
- ver, sym_visibility,
- symbol.st_shndx == strings_ndx);
+ ver, sym_visibility);
syms_found.push_back(symbol_found);
found = true;
}
@@ -1103,10 +1098,6 @@ lookup_symbol_from_gnu_hash_tab(const environment* env,
elf_symbol::type sym_type;
elf_symbol::binding sym_binding;
elf_symbol::visibility sym_visibility;
- Elf_Scn *strings_section = find_ksymtab_strings_section(elf_handle);
- size_t strings_ndx = strings_section
- ? elf_ndxscn(strings_section)
- : 0;
// Let's walk the hash table and record the versions of all the
// symbols which name equal sym_name.
@@ -1151,8 +1142,7 @@ lookup_symbol_from_gnu_hash_tab(const environment* env,
sym_type, sym_binding,
symbol.st_shndx != SHN_UNDEF,
symbol.st_shndx == SHN_COMMON,
- ver, sym_visibility,
- symbol.st_shndx == strings_ndx);
+ ver, sym_visibility);
syms_found.push_back(symbol_found);
found = true;
}
@@ -1275,10 +1265,6 @@ lookup_symbol_from_symtab(const environment* env,
char* name_str = 0;
elf_symbol::version ver;
bool found = false;
- Elf_Scn *strings_section = find_ksymtab_strings_section(elf_handle);
- size_t strings_ndx = strings_section
- ? elf_ndxscn(strings_section)
- : 0;
for (size_t i = 0; i < symcount; ++i)
{
@@ -1307,8 +1293,7 @@ lookup_symbol_from_symtab(const environment* env,
elf_symbol::create(env, i, sym->st_size,
name_str, sym_type,
sym_binding, sym_is_defined,
- sym_is_common, ver, sym_visibility,
- sym->st_shndx == strings_ndx);
+ sym_is_common, ver, sym_visibility);
syms_found.push_back(symbol_found);
found = true;
}
@@ -14118,8 +14103,7 @@ create_default_fn_sym(const string& sym_name, const environment *env)
/*symbol is defined=*/ true,
/*symbol is common=*/ false,
/*symbol version=*/ ver,
- /*symbol visibility=*/elf_symbol::DEFAULT_VISIBILITY,
- /*symbol is linux string cst=*/false);
+ /*symbol visibility=*/elf_symbol::DEFAULT_VISIBILITY);
return result;
}
@@ -1395,7 +1395,6 @@ struct elf_symbol::priv
// STT_COMMON. If no such symbol is found, it looks for the
// STT_COMMON definition of that name that has the largest size.
bool is_common_;
- bool is_linux_string_cst_;
bool is_in_ksymtab_;
uint64_t crc_;
bool is_suppressed_;
@@ -1413,7 +1412,6 @@ struct elf_symbol::priv
visibility_(elf_symbol::DEFAULT_VISIBILITY),
is_defined_(false),
is_common_(false),
- is_linux_string_cst_(false),
is_in_ksymtab_(false),
crc_(0),
is_suppressed_(false)
@@ -1429,7 +1427,6 @@ struct elf_symbol::priv
bool c,
const elf_symbol::version& ve,
elf_symbol::visibility vi,
- bool is_linux_string_cst,
bool is_in_ksymtab,
uint64_t crc,
bool is_suppressed)
@@ -1443,7 +1440,6 @@ struct elf_symbol::priv
visibility_(vi),
is_defined_(d),
is_common_(c),
- is_linux_string_cst_(is_linux_string_cst),
is_in_ksymtab_(is_in_ksymtab),
crc_(crc),
is_suppressed_(is_suppressed)
@@ -1490,9 +1486,6 @@ elf_symbol::elf_symbol()
///
/// @param vi the visibility of the symbol.
///
-/// @param is_linux_string_cst true if the symbol is a Linux Kernel
-/// string constant defined in the __ksymtab_strings section.
-///
/// @param crc the CRC (modversions) value of Linux Kernel symbols
elf_symbol::elf_symbol(const environment* e,
size_t i,
@@ -1504,7 +1497,6 @@ elf_symbol::elf_symbol(const environment* e,
bool c,
const version& ve,
visibility vi,
- bool is_linux_string_cst,
bool is_in_ksymtab,
uint64_t crc,
bool is_suppressed)
@@ -1518,7 +1510,6 @@ elf_symbol::elf_symbol(const environment* e,
c,
ve,
vi,
- is_linux_string_cst,
is_in_ksymtab,
crc,
is_suppressed))
@@ -1562,9 +1553,6 @@ elf_symbol::create()
///
/// @param vi the visibility of the symbol.
///
-/// @param is_linux_string_cst if true, it means the symbol represents
-/// a string constant from a linux kernel binary.
-///
/// @param crc the CRC (modversions) value of Linux Kernel symbols
///
/// @return a (smart) pointer to a newly created instance of @ref
@@ -1580,13 +1568,11 @@ elf_symbol::create(const environment* e,
bool c,
const version& ve,
visibility vi,
- bool is_linux_string_cst,
bool is_in_ksymtab,
uint64_t crc,
bool is_suppressed)
{
elf_symbol_sptr sym(new elf_symbol(e, i, s, n, t, b, d, c, ve, vi,
- is_linux_string_cst,
is_in_ksymtab, crc, is_suppressed));
sym->priv_->main_symbol_ = sym;
return sym;
@@ -1653,15 +1639,6 @@ void
elf_symbol::set_index(size_t s)
{priv_->index_ = s;}
-/// Test if the ELF symbol is for a string constant of a Linux binary
-/// defined in the __ksymtab_strings symbol table.
-///
-/// @return true iff ELF symbol is for a string constant of a Linux
-/// binary defined in the __ksymtab_strings symbol table.
-bool
-elf_symbol::get_is_linux_string_cst() const
-{return priv_->is_linux_string_cst_;}
-
/// Getter for the name of the @ref elf_symbol.
///
/// @return a reference to the name of the @ref symbol.
@@ -2985,8 +2985,7 @@ build_elf_symbol(read_context& ctxt, const xmlNodePtr node,
elf_symbol_sptr e = elf_symbol::create(env, /*index=*/0,
size, name, type, binding,
is_defined, is_common,
- version, visibility,
- /*is_linux_string_cst=*/false);
+ version, visibility);
e->set_is_suppressed(is_suppressed);
@@ -315,10 +315,7 @@ symtab::load_(Elf* elf_handle,
elf_helpers::stb_to_elf_symbol_binding(GELF_ST_BIND(sym->st_info)),
sym_is_defined, sym_is_common, ver,
elf_helpers::stv_to_elf_symbol_visibility
- (GELF_ST_VISIBILITY(sym->st_other)),
- /*is_linux_strings_cstr=*/false); // TODO: remove
- // is_linux_strings_cstr
- // as it is obsolete
+ (GELF_ST_VISIBILITY(sym->st_other)));
// We do not take suppressed symbols into our symbol vector to avoid
// accidental leakage. But we ensure supressed symbols are otherwise set