[PATCH v3 4/4] add Linux kernel symbol namespace support
Giuliano Procida
gprocida@google.com
Thu Mar 17 16:38:58 GMT 2022
Bug 28954 - add Linux Kernel symbol namespace support
Each Linux kernel symbol can be exported to a specified named
namespace or left in the global (nameless) namespace.
One complexity is that the symbol values which identify a string in
the __ksymtab_strings section must be interpretated differently for
vmlinux and .ko loadable modules as the former has a fixed load
address but the latter are relocatable. For vmlinux, the section base
address needs to be subtracted to obtain a section-relative offset.
The global namespace is explicitly represented as the empty string, at
least when it comes to the value of __kstrtabns_FOO symbols, but the
common interpretation is that such symbols lack an export namespace.
I would rather not have to make use of "empty implies missing" in many
places, so the code here represents namespace as optional<string> and
only the symtab reader cares about empty strings in __ksymtab_strings.
* include/abg-ir.h (elf_symbol::elf_symbol): Add ns argument.
(elf_symbol::create): Add ns argument.
(elf_symbol::get_namespace): Declare new function.
(elf_symbol::set_namespace): Declare new function.
and set_namespace.
* src/abg-comp-filter.cc (namespace_changed): Define new
helper functions.
(categorize_harmful_diff_node): Also call namespace_changed().
* src/abg-ir.cc (elf_symbol::priv): Add namespace_ member.
(elf_symbol::priv::priv): Add namespace_ to initialisers.
(elf_symbol::elf_symbol): Take new ns argument and pass it to
priv constructor.
(elf_symbol::create): Take new ns argument and pass it to
elf_symbol constructor.
(elf_symbol::get_namespace): Define new function.
(elf_symbol::set_namespace): Define new function.
* src/abg-reader.cc (build_elf_symbol): If namespace
attribute is present, set symbol namespace.
* src/abg-reporter-priv.cc (maybe_report_diff_for_symbol): If
symbol namespaces differ, report this.
* src/abg-symtab-reader.cc (symtab::load): Get ELF header to
distinguish vmlinux from .ko. Try to get __ksymtab_strings
metadata and data. Use these to look up __kstrtabns_FOO
namespace entries. Set symbol namespace where found.
* src/abg-writer.cc (write_elf_symbol): Emit namespace
attribute, if symbol has a namespace.
* tests/data/Makefile.am: Add new test files.
* tests/data/test-abidiff/test-namespace-0.xml: New test file.
* tests/data/test-abidiff/test-namespace-1.xml: Likewise
* tests/data/test-abidiff/test-namespace-report.txt: Likewise.
* tests/test-abidiff.cc: Add new test case.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
include/abg-ir.h | 8 +++
src/abg-comp-filter.cc | 40 ++++++++++-
src/abg-ir.cc | 30 ++++++++-
src/abg-reader.cc | 7 ++
src/abg-reporter-priv.cc | 18 +++++
src/abg-symtab-reader.cc | 66 +++++++++++++++++++
src/abg-writer.cc | 3 +
tests/data/Makefile.am | 3 +
tests/data/test-abidiff/test-namespace-0.xml | 15 +++++
tests/data/test-abidiff/test-namespace-1.xml | 15 +++++
.../test-abidiff/test-namespace-report.txt | 17 +++++
tests/test-abidiff.cc | 6 ++
12 files changed, 225 insertions(+), 3 deletions(-)
create mode 100644 tests/data/test-abidiff/test-namespace-0.xml
create mode 100644 tests/data/test-abidiff/test-namespace-1.xml
create mode 100644 tests/data/test-abidiff/test-namespace-report.txt
diff --git a/include/abg-ir.h b/include/abg-ir.h
index b05a8c6f..7c558828 100644
--- a/include/abg-ir.h
+++ b/include/abg-ir.h
@@ -922,6 +922,7 @@ private:
visibility vi,
bool is_in_ksymtab = false,
const abg_compat::optional<uint64_t>& crc = {},
+ const abg_compat::optional<std::string>& ns = {},
bool is_suppressed = false);
elf_symbol(const elf_symbol&);
@@ -947,6 +948,7 @@ public:
visibility vi,
bool is_in_ksymtab = false,
const abg_compat::optional<uint64_t>& crc = {},
+ const abg_compat::optional<std::string>& ns = {},
bool is_suppressed = false);
const environment*
@@ -1024,6 +1026,12 @@ public:
void
set_crc(const abg_compat::optional<uint64_t>& crc);
+ const abg_compat::optional<std::string>&
+ get_namespace() const;
+
+ void
+ set_namespace(const abg_compat::optional<std::string>& ns);
+
bool
is_suppressed() const;
diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc
index 31590284..22da5244 100644
--- a/src/abg-comp-filter.cc
+++ b/src/abg-comp-filter.cc
@@ -254,6 +254,43 @@ crc_changed(const diff* diff)
return false;
}
+/// Test if there was a function or variable namespace change.
+///
+/// @param f the first function or variable to consider.
+///
+/// @param s the second function or variable to consider.
+///
+/// @return true if the test is positive, false otherwise.
+template <typename function_or_var_decl_sptr>
+static bool
+namespace_changed(const function_or_var_decl_sptr& f,
+ const function_or_var_decl_sptr& s)
+{
+ const auto& symbol_f = f->get_symbol();
+ const auto& symbol_s = s->get_symbol();
+ if (!symbol_f || !symbol_s)
+ return false;
+ return symbol_f->get_namespace() != symbol_s->get_namespace();
+}
+
+/// Test if the current diff tree node carries a namespace change in
+/// either a function or a variable.
+///
+/// @param diff the diff tree node to consider.
+///
+/// @return true if the test is positive, false otherwise.
+static bool
+namespace_changed(const diff* diff)
+{
+ if (const function_decl_diff* d =
+ dynamic_cast<const function_decl_diff*>(diff))
+ return namespace_changed(d->first_function_decl(),
+ d->second_function_decl());
+ if (const var_diff* d = dynamic_cast<const var_diff*>(diff))
+ return namespace_changed(d->first_var(), d->second_var());
+ return false;
+}
+
/// Test if there was a function name change, but there there was no
/// change in name of the underlying symbol. IOW, if the name of a
/// function changed, but the symbol of the new function is equal to
@@ -1781,7 +1818,8 @@ categorize_harmful_diff_node(diff *d, bool pre)
|| non_static_data_member_added_or_removed(d)
|| base_classes_added_or_removed(d)
|| has_harmful_enum_change(d)
- || crc_changed(d)))
+ || crc_changed(d)
+ || namespace_changed(d)))
category |= SIZE_OR_OFFSET_CHANGE_CATEGORY;
if (has_virtual_mem_fn_change(d))
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 853b01ee..f90be2e4 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -1728,6 +1728,7 @@ struct elf_symbol::priv
bool is_common_;
bool is_in_ksymtab_;
abg_compat::optional<uint64_t> crc_;
+ abg_compat::optional<std::string> namespace_;
bool is_suppressed_;
elf_symbol_wptr main_symbol_;
elf_symbol_wptr next_alias_;
@@ -1745,6 +1746,7 @@ struct elf_symbol::priv
is_common_(false),
is_in_ksymtab_(false),
crc_(),
+ namespace_(),
is_suppressed_(false)
{}
@@ -1760,6 +1762,7 @@ struct elf_symbol::priv
elf_symbol::visibility vi,
bool is_in_ksymtab,
const abg_compat::optional<uint64_t>& crc,
+ const abg_compat::optional<std::string>& ns,
bool is_suppressed)
: env_(e),
index_(i),
@@ -1773,6 +1776,7 @@ struct elf_symbol::priv
is_common_(c),
is_in_ksymtab_(is_in_ksymtab),
crc_(crc),
+ namespace_(ns),
is_suppressed_(is_suppressed)
{
if (!is_common_)
@@ -1818,6 +1822,8 @@ elf_symbol::elf_symbol()
/// @param vi the visibility of the symbol.
///
/// @param crc the CRC (modversions) value of Linux Kernel symbols
+///
+/// @param ns the namespace of Linux Kernel symbols, if any
elf_symbol::elf_symbol(const environment* e,
size_t i,
size_t s,
@@ -1830,6 +1836,7 @@ elf_symbol::elf_symbol(const environment* e,
visibility vi,
bool is_in_ksymtab,
const abg_compat::optional<uint64_t>& crc,
+ const abg_compat::optional<std::string>& ns,
bool is_suppressed)
: priv_(new priv(e,
i,
@@ -1843,6 +1850,7 @@ elf_symbol::elf_symbol(const environment* e,
vi,
is_in_ksymtab,
crc,
+ ns,
is_suppressed))
{}
@@ -1886,6 +1894,8 @@ elf_symbol::create()
///
/// @param crc the CRC (modversions) value of Linux Kernel symbols
///
+/// @param ns the namespace of Linux Kernel symbols, if any
+///
/// @return a (smart) pointer to a newly created instance of @ref
/// elf_symbol.
elf_symbol_sptr
@@ -1901,10 +1911,11 @@ elf_symbol::create(const environment* e,
visibility vi,
bool is_in_ksymtab,
const abg_compat::optional<uint64_t>& crc,
+ const abg_compat::optional<std::string>& ns,
bool is_suppressed)
{
elf_symbol_sptr sym(new elf_symbol(e, i, s, n, t, b, d, c, ve, vi,
- is_in_ksymtab, crc, is_suppressed));
+ is_in_ksymtab, crc, ns, is_suppressed));
sym->priv_->main_symbol_ = sym;
return sym;
}
@@ -1926,7 +1937,8 @@ textually_equals(const elf_symbol&l,
&& l.is_defined() == r.is_defined()
&& l.is_common_symbol() == r.is_common_symbol()
&& l.get_version() == r.get_version()
- && l.get_crc() == r.get_crc());
+ && l.get_crc() == r.get_crc()
+ && l.get_namespace() == r.get_namespace());
if (equals && l.is_variable())
// These are variable symbols. Let's compare their symbol size.
@@ -2146,6 +2158,20 @@ void
elf_symbol::set_crc(const abg_compat::optional<uint64_t>& crc)
{priv_->crc_ = crc;}
+/// Getter of the 'namespace' property.
+///
+/// @return the namespace for Linux Kernel symbols, if any
+const abg_compat::optional<std::string>&
+elf_symbol::get_namespace() const
+{return priv_->namespace_;}
+
+/// Setter of the 'namespace' property.
+///
+/// @param ns the new namespace for Linux Kernel symbols, if any
+void
+elf_symbol::set_namespace(const abg_compat::optional<std::string>& ns)
+{priv_->namespace_ = ns;}
+
/// Getter for the 'is-suppressed' property.
///
/// @return true iff the current symbol has been suppressed by a
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 7a9ad1f9..f9e420f1 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -3265,6 +3265,13 @@ build_elf_symbol(read_context& ctxt, const xmlNodePtr node,
if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "crc"))
e->set_crc(strtoull(CHAR_STR(s), NULL, 0));
+ if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "namespace"))
+ {
+ std::string ns;
+ xml::xml_char_sptr_to_string(s, ns);
+ e->set_namespace(ns);
+ }
+
return e;
}
diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
index f9dd928b..9ad733f4 100644
--- a/src/abg-reporter-priv.cc
+++ b/src/abg-reporter-priv.cc
@@ -1168,6 +1168,24 @@ maybe_report_diff_for_symbol(const elf_symbol_sptr& symbol1,
out << std::noshowbase << std::dec
<< "\n";
}
+
+ const abg_compat::optional<std::string>& ns1 = symbol1->get_namespace();
+ const abg_compat::optional<std::string>& ns2 = symbol2->get_namespace();
+ if (ns1 != ns2)
+ {
+ const std::string none = "(none)";
+ out << indent << "namespace changed from ";
+ if (ns1.has_value())
+ out << "'" << ns1.value() << "'";
+ else
+ out << none;
+ out << " to ";
+ if (ns2.has_value())
+ out << "'" << ns2.value() << "'";
+ else
+ out << none;
+ out << "\n";
+ }
}
/// For a given symbol, emit a string made of its name and version.
diff --git a/src/abg-symtab-reader.cc b/src/abg-symtab-reader.cc
index b42ce87d..123ef84a 100644
--- a/src/abg-symtab-reader.cc
+++ b/src/abg-symtab-reader.cc
@@ -204,6 +204,13 @@ symtab::load_(Elf* elf_handle,
ir::environment* env,
symbol_predicate is_suppressed)
{
+ GElf_Ehdr ehdr_mem;
+ GElf_Ehdr* header = gelf_getehdr(elf_handle, &ehdr_mem);
+ if (!header)
+ {
+ std::cerr << "Could not load get ELF header: Skipping symtab load.\n";
+ return false;
+ }
Elf_Scn* symtab_section = elf_helpers::find_symbol_table_section(elf_handle);
if (!symtab_section)
@@ -232,9 +239,34 @@ symtab::load_(Elf* elf_handle,
return false;
}
+ // The __kstrtab_strings sections is basically an ELF strtab but does not
+ // support elf_strptr lookups. A single call to elf_getdata gives a handle to
+ // section data.
+ //
+ // The value of a __kstrtabns_FOO (or other similar) symbol is an address
+ // within the __kstrtab_strings section. To look up the string value, we need
+ // to translate from vmlinux load address to section offset by subtracting the
+ // base address of the section. This adjustment is not needed for loadable
+ // modules which are relocatable.
+ Elf_Scn* strings_section = elf_helpers::find_ksymtab_strings_section(elf_handle);
+ size_t strings_offset = 0;
+ const char* strings_data = nullptr;
+ size_t strings_size = 0;
+ if (strings_section)
+ {
+ GElf_Shdr strings_sheader;
+ gelf_getshdr(strings_section, &strings_sheader);
+ strings_offset = header->e_type == ET_REL ? 0 : strings_sheader.sh_addr;
+ Elf_Data* data = elf_getdata(strings_section, nullptr);
+ ABG_ASSERT(data->d_off == 0);
+ strings_data = reinterpret_cast<const char *>(data->d_buf);
+ strings_size = data->d_size;
+ }
+
const bool is_kernel = elf_helpers::is_linux_kernel(elf_handle);
std::unordered_set<std::string> exported_kernel_symbols;
std::unordered_map<std::string, uint64_t> crc_values;
+ std::unordered_map<std::string, std::string> namespaces;
const bool is_arm32 = elf_helpers::architecture_is_arm32(elf_handle);
const bool is_ppc64 = elf_helpers::architecture_is_ppc64(elf_handle);
@@ -288,6 +320,29 @@ symtab::load_(Elf* elf_handle,
ABG_ASSERT(crc_values.emplace(name.substr(6), sym->st_value).second);
continue;
}
+ if (strings_section && is_kernel && name.rfind("__kstrtabns_", 0) == 0)
+ {
+ // This symbol lives in the __ksymtab_strings section but st_value may
+ // be a vmlinux load address so we need to subtract the offset before
+ // looking it up in that section.
+ const size_t value = sym->st_value;
+ const size_t offset = value - strings_offset;
+ // check offset
+ ABG_ASSERT(offset < strings_size);
+ // find the terminating NULL
+ const char* first = strings_data + offset;
+ const char* last = strings_data + strings_size;
+ const char* limit = std::find(first, last, 0);
+ // check NULL found
+ ABG_ASSERT(limit < last);
+ // interpret the empty namespace name as no namespace name
+ if (first < limit)
+ {
+ const std::string ns(first, limit - first);
+ ABG_ASSERT(namespaces.emplace(name.substr(12), ns).second);
+ }
+ continue;
+ }
// filter out uninteresting entries and only keep functions/variables for
// now. The rest might be interesting in the future though.
@@ -402,6 +457,17 @@ symtab::load_(Elf* elf_handle,
symbol->set_crc(crc_entry.second);
}
+ // Now add the namespaces
+ for (const auto& namespace_entry : namespaces)
+ {
+ const auto r = name_symbol_map_.find(namespace_entry.first);
+ if (r == name_symbol_map_.end())
+ continue;
+
+ for (const auto& symbol : r->second)
+ symbol->set_namespace(namespace_entry.second);
+ }
+
// sort the symbols for deterministic output
std::sort(symbols_.begin(), symbols_.end(), symbol_sort);
diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index 0a1e7b66..03fb658b 100644
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -3136,6 +3136,9 @@ write_elf_symbol(const elf_symbol_sptr& sym,
<< std::hex << std::showbase << sym->get_crc().value()
<< std::dec << std::noshowbase << "'";
+ if (sym->get_namespace().has_value())
+ o << " namespace='" << sym->get_namespace().value() << "'";
+
o << "/>\n";
return true;
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 90bd1934..7388697b 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -93,6 +93,9 @@ test-abidiff/test-crc-2.xml \
test-abidiff/test-crc-report-0-1.txt \
test-abidiff/test-crc-report-1-0.txt \
test-abidiff/test-crc-report-1-2.txt \
+test-abidiff/test-namespace-0.xml \
+test-abidiff/test-namespace-1.xml \
+test-abidiff/test-namespace-report.txt \
test-abidiff/test-PR27985-report.txt \
test-abidiff/test-PR27985-v0.c \
test-abidiff/test-PR27985-v0.o \
diff --git a/tests/data/test-abidiff/test-namespace-0.xml b/tests/data/test-abidiff/test-namespace-0.xml
new file mode 100644
index 00000000..5a9f5cd2
--- /dev/null
+++ b/tests/data/test-abidiff/test-namespace-0.xml
@@ -0,0 +1,15 @@
+<abi-corpus path='test.o' architecture='elf-amd-x86_64'>
+ <elf-variable-symbols>
+ <elf-symbol name='v1' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+ <elf-symbol name='v2' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes' namespace='this'/>
+ <elf-symbol name='v3' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes' namespace='that'/>
+ <elf-symbol name='v4' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes' namespace=''/>
+ </elf-variable-symbols>
+ <abi-instr version='1.0' address-size='64' path='test.c' comp-dir-path='/tmp' language='LANG_C89'>
+ <type-decl name='int' size-in-bits='32' id='type-id-1'/>
+ <var-decl name='v1' type-id='type-id-1' mangled-name='v1' visibility='default' filepath='test.c' line='1' column='1' elf-symbol-id='v1'/>
+ <var-decl name='v2' type-id='type-id-1' mangled-name='v2' visibility='default' filepath='test.c' line='2' column='1' elf-symbol-id='v2'/>
+ <var-decl name='v3' type-id='type-id-1' mangled-name='v3' visibility='default' filepath='test.c' line='3' column='1' elf-symbol-id='v3'/>
+ <var-decl name='v4' type-id='type-id-1' mangled-name='v4' visibility='default' filepath='test.c' line='4' column='1' elf-symbol-id='v4'/>
+ </abi-instr>
+</abi-corpus>
diff --git a/tests/data/test-abidiff/test-namespace-1.xml b/tests/data/test-abidiff/test-namespace-1.xml
new file mode 100644
index 00000000..9814844b
--- /dev/null
+++ b/tests/data/test-abidiff/test-namespace-1.xml
@@ -0,0 +1,15 @@
+<abi-corpus path='test.o' architecture='elf-amd-x86_64'>
+ <elf-variable-symbols>
+ <elf-symbol name='v1' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes' namespace='this'/>
+ <elf-symbol name='v2' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes' namespace='that'/>
+ <elf-symbol name='v3' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes' namespace=''/>
+ <elf-symbol name='v4' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+ </elf-variable-symbols>
+ <abi-instr version='1.0' address-size='64' path='test.c' comp-dir-path='/tmp' language='LANG_C89'>
+ <type-decl name='int' size-in-bits='32' id='type-id-1'/>
+ <var-decl name='v1' type-id='type-id-1' mangled-name='v1' visibility='default' filepath='test.c' line='1' column='1' elf-symbol-id='v1'/>
+ <var-decl name='v2' type-id='type-id-1' mangled-name='v2' visibility='default' filepath='test.c' line='2' column='1' elf-symbol-id='v2'/>
+ <var-decl name='v3' type-id='type-id-1' mangled-name='v3' visibility='default' filepath='test.c' line='3' column='1' elf-symbol-id='v3'/>
+ <var-decl name='v4' type-id='type-id-1' mangled-name='v4' visibility='default' filepath='test.c' line='4' column='1' elf-symbol-id='v4'/>
+ </abi-instr>
+</abi-corpus>
diff --git a/tests/data/test-abidiff/test-namespace-report.txt b/tests/data/test-abidiff/test-namespace-report.txt
new file mode 100644
index 00000000..d2c421ed
--- /dev/null
+++ b/tests/data/test-abidiff/test-namespace-report.txt
@@ -0,0 +1,17 @@
+Functions changes summary: 0 Removed, 0 Changed, 0 Added function
+Variables changes summary: 0 Removed, 4 Changed, 0 Added variables
+
+4 Changed variables:
+
+ [C] 'int v1' was changed:
+ namespace changed from (none) to 'this'
+
+ [C] 'int v2' was changed:
+ namespace changed from 'this' to 'that'
+
+ [C] 'int v3' was changed:
+ namespace changed from 'that' to ''
+
+ [C] 'int v4' was changed:
+ namespace changed from '' to (none)
+
diff --git a/tests/test-abidiff.cc b/tests/test-abidiff.cc
index a02bbe3d..93e44d6a 100644
--- a/tests/test-abidiff.cc
+++ b/tests/test-abidiff.cc
@@ -128,6 +128,12 @@ static InOutSpec specs[] =
"data/test-abidiff/test-crc-report-1-2.txt",
"output/test-abidiff/test-crc-report-1-2.txt"
},
+ {
+ "data/test-abidiff/test-namespace-0.xml",
+ "data/test-abidiff/test-namespace-1.xml",
+ "data/test-abidiff/test-namespace-report.txt",
+ "output/test-abidiff/test-namespace-report-0-1.txt"
+ },
{
"data/test-abidiff/test-PR27616-v0.xml",
"data/test-abidiff/test-PR27616-v1.xml",
--
2.35.1.894.gb6a874cedc-goog
More information about the Libabigail
mailing list