From: Giuliano Procida <gprocida@google.com>
To: Matthias Maennich <maennich@google.com>
Cc: libabigail@sourceware.org, dodji@seketeli.org, kernel-team@android.com
Subject: Re: [PATCH v3 4/4] add Linux kernel symbol namespace support
Date: Mon, 21 Mar 2022 15:52:49 +0000 [thread overview]
Message-ID: <CAGvU0Hk5BvbNDHkssVmzNL1mJVXiwLyYMuvHJgbLLL8iEm7UUw@mail.gmail.com> (raw)
In-Reply-To: <Yjh1ZEqnIhWEl8tP@google.com>
Hi.
On Mon, 21 Mar 2022 at 12:53, Matthias Maennich <maennich@google.com> wrote:
>
> On Thu, Mar 17, 2022 at 04:38:58PM +0000, Giuliano Procida wrote:
> >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";
>
> nit: "Could not load ELF header: ..."
>
> >+ 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.
>
> raw section data
Documentation actually refers to "washed" data, so "raw" doesn't sound right.
I'll insert "washed".
>
> >+ //
> >+ // 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.
>
> ... identifiable by ELF type ET_REL.
>
Done.
> >+ 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;
>
> Since you later assume strings_data and strings_size being valid based
> on the existence of strings_section, perhaps assert that the extraction
> worked?
> ABG_ASSERT(strings_data);
> ABG_ASSERT(strings_size > 0); // otherwise, why would there be a section?
>
If the section is empty, null and 0 are expected. Later offset bounds
checking will assert if any lookups are actually attempted.
Any checking here is at best redundant and at worst (very
theoretically) premature.
If strings_data is outside mapped memory and strings_size is non-zero
that will SEGV. I don't think it's worth doing special checks for the
various strings_size == 0 possibilities.
> >+ }
> >+
> > 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;
>
> odd indentation
>
Done (tabification).
> >+ 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);
>
> ABG_ASSERT(namespaces.emplace(name.substr(12), std::move(ns)).second);
>
Done (inlined).
>
> With the few nits addressed, feel free to add
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
>
> Cheers,
> Matthias
>
Thanks!
Giuliano.
> >+ }
> >+ 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
> >
next prev parent reply other threads:[~2022-03-21 15:53 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-14 18:13 [PATCH 0/2] Bug 28954 - add Linux Kernel " Giuliano Procida
2022-03-14 18:13 ` [PATCH 1/2] optional: add operator== and operator!= Giuliano Procida
2022-03-15 11:02 ` Matthias Maennich
2022-03-16 9:31 ` Giuliano Procida
2022-03-14 18:13 ` [PATCH 2/2] add Linux kernel symbol namespace support Giuliano Procida
2022-03-15 11:25 ` Matthias Maennich
2022-03-16 16:30 ` [PATCH v2 0/4] add symbol namespace support, update symbol CRC support Giuliano Procida
2022-03-16 16:30 ` [PATCH v2 1/4] optional: minor improvements Giuliano Procida
2022-03-17 10:56 ` Matthias Maennich
2022-03-16 16:30 ` [PATCH v2 2/4] crc_changed: eliminate copying of shared_ptr values Giuliano Procida
2022-03-17 11:01 ` Matthias Maennich
2022-03-16 16:30 ` [PATCH v2 3/4] add Linux kernel symbol namespace support Giuliano Procida
2022-03-17 11:57 ` Matthias Maennich
2022-03-16 16:30 ` [PATCH v2 4/4] Linux symbol CRCs: support 0 and report presence changes Giuliano Procida
2022-03-17 12:01 ` Matthias Maennich
2022-03-17 16:38 ` [PATCH v3 0/4] add symbol namespace support, update symbol CRC support Giuliano Procida
2022-03-17 16:38 ` [PATCH v3 1/4] crc_changed: eliminate copying of shared_ptr values Giuliano Procida
2022-03-17 16:38 ` [PATCH v3 2/4] optional: minor improvements Giuliano Procida
2022-03-17 16:38 ` [PATCH v3 3/4] Linux symbol CRCs: support 0 and report presence changes Giuliano Procida
2022-03-17 16:38 ` [PATCH v3 4/4] add Linux kernel symbol namespace support Giuliano Procida
2022-03-21 12:53 ` Matthias Maennich
2022-03-21 15:52 ` Giuliano Procida [this message]
2022-03-21 16:02 ` [PATCH v4 0/4] add symbol namespace support, update symbol CRC support Giuliano Procida
2022-03-21 16:02 ` [PATCH v4 1/4] crc_changed: eliminate copying of shared_ptr values Giuliano Procida
2022-03-21 16:02 ` [PATCH v4 2/4] optional: minor improvements Giuliano Procida
2022-03-21 16:02 ` [PATCH v4 3/4] Linux symbol CRCs: support 0 and report presence changes Giuliano Procida
2022-03-21 16:02 ` [PATCH v4 4/4] add Linux kernel symbol namespace support Giuliano Procida
2022-06-13 14:25 ` [PATCH v5 0/4] add symbol namespace support, update symbol CRC support Giuliano Procida
2022-06-30 16:39 ` Dodji Seketeli
2022-06-13 14:25 ` [PATCH v5 1/4] crc_changed: eliminate copying of shared_ptr values Giuliano Procida
2022-06-30 16:40 ` Dodji Seketeli
2022-06-13 14:25 ` [PATCH v5 2/4] optional: minor improvements Giuliano Procida
2022-06-30 16:40 ` Dodji Seketeli
2022-06-13 14:25 ` [PATCH v5 3/4] Linux symbol CRCs: support 0 and report presence changes Giuliano Procida
2022-06-30 16:41 ` Dodji Seketeli
2022-06-13 14:25 ` [PATCH v5 4/4] add Linux kernel symbol namespace support Giuliano Procida
2022-07-01 12:54 ` Dodji Seketeli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAGvU0Hk5BvbNDHkssVmzNL1mJVXiwLyYMuvHJgbLLL8iEm7UUw@mail.gmail.com \
--to=gprocida@google.com \
--cc=dodji@seketeli.org \
--cc=kernel-team@android.com \
--cc=libabigail@sourceware.org \
--cc=maennich@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).