From: Di Chen <dichen@redhat.com>
To: Aaron Merey <amerey@redhat.com>
Cc: systemtap@sourceware.org
Subject: Re: [PATCH] PR29997: Fix the symbol aliases search failure when symbol version is missing
Date: Wed, 31 Jan 2024 10:45:31 +0800 [thread overview]
Message-ID: <CAN-Pu7Q_Rz12Qq7xjR+EB-QFMNpzOpLBDcOY93pdaDufMqcsJQ@mail.gmail.com> (raw)
In-Reply-To: <CAJDtP-Sq8Pp8CPd=FdM_gyVndzmZc=1awze4YooNRk_nbPOj2A@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 7046 bytes --]
Thanks Aaron,
Re-submitted with:
1) Indentation improvement.
2) Remove the wildcard handling in "iterate_single_function".
3) Add examples in the commit message.
On Thu, Jan 18, 2024 at 8:15 AM Aaron Merey <amerey@redhat.com> wrote:
> Hi Di,
>
> Thanks for the patch.
>
> On Tue, Dec 19, 2023 at 9:56 PM Di Chen <dichen@redhat.com> wrote:
> >
> > Hey Aaron,
> >
> > I have added the wildcard handling for the PR29997.
> >
> > From 0c95ce19272d6e13abf6f41c4ae29b6aa925ca20 Mon Sep 17 00:00:00 2001
> > From: Di Chen <dichen@redhat.com>
> > Date: Sun, 5 Nov 2023 11:23:50 +0800
> > Subject: [PATCH] PR29997: Fix the symbol aliases search failure when
> symbol
> > version is missing
> >
> > After calling module_info::update_symtab, function aliases will be
> > populated. Then the updated symtab will be used for symbol searching.
> >
> > For the _IO_new_fopen familty with the aliases:
> >
> > $ eu-readelf -s /lib64/libc.so.6 | grep 0000000000077440
> > 247: 0000000000077440 14 FUNC WEAK DEFAULT 16 fopen64@
> @GLIBC_2.2.5
> > 1014: 0000000000077440 14 FUNC GLOBAL DEFAULT 16 fopen@
> @GLIBC_2.2.5
> > 1028: 0000000000077440 14 FUNC GLOBAL DEFAULT 16
> _IO_fopen@@GLIBC_2.2.5
> > 1556: 0000000000077440 14 FUNC LOCAL DEFAULT 16
> _IO_fopen64
> > 3471: 0000000000077440 14 FUNC LOCAL DEFAULT 16
> __new_fopen
> > 4765: 0000000000077440 14 FUNC LOCAL DEFAULT 16
> _IO_new_fopen
> > 5110: 0000000000077440 14 FUNC WEAK DEFAULT 16 fopen64
> > 7198: 0000000000077440 14 FUNC GLOBAL DEFAULT 16 fopen@
> @GLIBC_2.2.5
> > 7433: 0000000000077440 14 FUNC GLOBAL DEFAULT 16
> _IO_fopen@@GLIBC_2.2.5
> >
> > a) fopen@@GLIBC_2.2.5 exists in the updated symtab
> > b) fopen does not exist in the updated symtab
> >
> > This PR is to add a version info padding when symbol cannot be found in
> > the updated symtab.
> >
> > Signed-off-by: Di Chen <dichen@redhat.com>
> > ---
> > dwflpp.cxx | 44 +++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/dwflpp.cxx b/dwflpp.cxx
> > index 9fccca0a9..044b0ca97 100644
> > --- a/dwflpp.cxx
> > +++ b/dwflpp.cxx
> > @@ -1055,6 +1055,15 @@ dwflpp::iterate_over_functions<void>(int
> (*callback)(Dwarf_Die*, void*),
> > }
> >
> > auto range = v->equal_range(function);
> > + // version padding if the symbol is not found
> > + if (range.first == range.second)
> > + {
> > + std::string function_with_ver = function + "@";
> > + for (auto it = v->begin(); it != v->end(); ++it)
> > + if (it->first.find(function_with_ver) == 0)
> > + function_with_ver = it->first;
> > + range = v->equal_range(function_with_ver);
> > + }
>
> Just a nit but there should be another 2 spaces of indentation for the
> lines between these braces.
>
> > if (range.first != range.second)
> > {
> > for (auto it = range.first; it != range.second; ++it)
> > @@ -1098,7 +1107,10 @@ dwflpp::iterate_over_functions<void>(int
> (*callback)(Dwarf_Die*, void*),
> > if (pending_interrupts) return DWARF_CB_ABORT;
> > const string& func_name = it->first;
> > Dwarf_Die& die = it->second;
> > - if (function_name_matches_pattern (func_name, function))
> > +
> > + // version padding if the pattern is not matched
> > + if ((function_name_matches_pattern (func_name, function)) ||
> > + (function_name_matches_pattern (func_name, function +
> "@*")))
>
> Nice, this fixes the issue where names containing wildcards and version
> info aren't found.
>
> > {
> > if (sess.verbose > 4)
> > clog << _F("function cache %s:%s match %s vs %s",
> module_name.c_str(),
> > @@ -1141,6 +1153,15 @@ dwflpp::iterate_single_function<void>(int
> (*callback)(Dwarf_Die*, void*),
> > }
> >
> > auto range = v->equal_range(function);
> > + // version padding if the symbol is not found
> > + if (range.first == range.second)
> > + {
> > + std::string function_with_ver = function + "@";
> > + for (auto it = v->begin(); it != v->end(); ++it)
> > + if (it->first.find(function_with_ver) == 0)
> > + function_with_ver = it->first;
> > + range = v->equal_range(function_with_ver);
> > + }
> > if (range.first != range.second)
> > {
> > for (auto it = range.first; it != range.second; ++it)
> > @@ -1158,6 +1179,27 @@ dwflpp::iterate_single_function<void>(int
> (*callback)(Dwarf_Die*, void*),
> > if (rc != DWARF_CB_OK) break;
> > }
> > }
> > + else if (name_has_wildcard (function))
> > + {
> > + for (auto it = v->begin(); it != v->end(); ++it)
> > + {
> > + if (pending_interrupts) return DWARF_CB_ABORT;
> > + const string& func_name = it->first;
> > + Dwarf_Die& die = it->second;
> > +
> > + // version padding if the pattern is not matched
> > + if ((function_name_matches_pattern (func_name, function)) ||
> > + (function_name_matches_pattern (func_name, function +
> "@*")))
> > + {
> > + if (sess.verbose > 4)
> > + clog << _F("function cache %s:%s match %s vs %s",
> module_name.c_str(),
> > + cu_name().c_str(), func_name.c_str(),
> function.c_str()) << endl;
> > +
> > + rc = (*callback)(& die, data);
> > + if (rc != DWARF_CB_OK) break;
> > + }
> > + }
> > + }
>
> Do we need to include wildcard handling here? Since this function didn't
> originally include any wildcard handling, I'd assume that searches for
> names with wildcards would always go through dwflpp::iterate_over_functions
> instead.
>
> >
> > // undo the focus_on_cu
> > this->cu = NULL;
> > --
> > 2.41.0
>
> I was also thinking about whether we should include "@" handling for
> mangled C++ names as well in dwflpp::iterate_over_functions. I
> experimented with trying to list probe points with mangled function names
> that include version info. eu-readelf shows some mangled names with
> "@<VERSION>" in the stap binary. However it looks like these names
> never appear among the linkage_names from dwarf_linkage_name that are
> searched for a match.
>
> For example, "_ZSt24__throw_out_of_range_fmtPKcz@GLIBCXX_3.4.20" is
> list by eu-readelf in my locally built stap binary but it doesn't
> appear in a dump of linkage_names search during `stap -L
>
> 'process("/usr/local/bin/stap").function("_ZSt24__throw_out_of_range_fmtPKcz")'`.
>
> This might be a bug or possibly a limitation of dwarf_linkage_name.
> However this goes beyond the scope of your patch so it's ok to not
> address this in your patch.
>
> Aaron
> ~
>
>
[-- Attachment #2: 0001-PR29997-Fix-the-symbol-aliases-search-failure-when-s.patch --]
[-- Type: text/x-patch, Size: 3761 bytes --]
From c1462a5b904d1aff044d76fc8c9717a243bb182e Mon Sep 17 00:00:00 2001
From: Di Chen <dichen@redhat.com>
Date: Wed, 31 Jan 2024 10:25:35 +0800
Subject: [PATCH] PR29997: Fix the symbol aliases search failure when symbol
version is missing
After calling module_info::update_symtab, function aliases will be
populated. Then the updated symtab will be used for symbol searching.
For the _IO_new_fopen family with the aliases:
$ eu-readelf -s /lib64/libc.so.6 | grep 0000000000077440
247: 0000000000077440 14 FUNC WEAK DEFAULT 16 fopen64@@GLIBC_2.2.5
1014: 0000000000077440 14 FUNC GLOBAL DEFAULT 16 fopen@@GLIBC_2.2.5
1028: 0000000000077440 14 FUNC GLOBAL DEFAULT 16 _IO_fopen@@GLIBC_2.2.5
1556: 0000000000077440 14 FUNC LOCAL DEFAULT 16 _IO_fopen64
3471: 0000000000077440 14 FUNC LOCAL DEFAULT 16 __new_fopen
4765: 0000000000077440 14 FUNC LOCAL DEFAULT 16 _IO_new_fopen
5110: 0000000000077440 14 FUNC WEAK DEFAULT 16 fopen64
7198: 0000000000077440 14 FUNC GLOBAL DEFAULT 16 fopen@@GLIBC_2.2.5
7433: 0000000000077440 14 FUNC GLOBAL DEFAULT 16 _IO_fopen@@GLIBC_2.2.5
a) fopen@@GLIBC_2.2.5 exists in the updated symtab
b) fopen does not exist in the updated symtab
This PR is to add a version info padding when symbol cannot be found in
the updated symtab, so systemap can support searching symbol aliases like
this:
$ stap -L 'process("/lib64/libc.so.6").function("fopen")'
# And with wildcard, like this
$ stap -L 'process("/lib64/libc.so.6").function("fo*en")'
Signed-off-by: Di Chen <dichen@redhat.com>
---
dwflpp.cxx | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/dwflpp.cxx b/dwflpp.cxx
index 9fccca0a9..26e9144cd 100644
--- a/dwflpp.cxx
+++ b/dwflpp.cxx
@@ -1055,6 +1055,15 @@ dwflpp::iterate_over_functions<void>(int (*callback)(Dwarf_Die*, void*),
}
auto range = v->equal_range(function);
+ // version padding if the symbol is not found
+ if (range.first == range.second)
+ {
+ std::string function_with_ver = function + "@";
+ for (auto it = v->begin(); it != v->end(); ++it)
+ if (it->first.find(function_with_ver) == 0)
+ function_with_ver = it->first;
+ range = v->equal_range(function_with_ver);
+ }
if (range.first != range.second)
{
for (auto it = range.first; it != range.second; ++it)
@@ -1098,7 +1107,10 @@ dwflpp::iterate_over_functions<void>(int (*callback)(Dwarf_Die*, void*),
if (pending_interrupts) return DWARF_CB_ABORT;
const string& func_name = it->first;
Dwarf_Die& die = it->second;
- if (function_name_matches_pattern (func_name, function))
+
+ // version padding if the pattern is not matched
+ if ((function_name_matches_pattern (func_name, function)) ||
+ (function_name_matches_pattern (func_name, function + "@*")))
{
if (sess.verbose > 4)
clog << _F("function cache %s:%s match %s vs %s", module_name.c_str(),
@@ -1141,6 +1153,15 @@ dwflpp::iterate_single_function<void>(int (*callback)(Dwarf_Die*, void*),
}
auto range = v->equal_range(function);
+ // version padding if the symbol is not found
+ if (range.first == range.second)
+ {
+ std::string function_with_ver = function + "@";
+ for (auto it = v->begin(); it != v->end(); ++it)
+ if (it->first.find(function_with_ver) == 0)
+ function_with_ver = it->first;
+ range = v->equal_range(function_with_ver);
+ }
if (range.first != range.second)
{
for (auto it = range.first; it != range.second; ++it)
--
2.41.0
next prev parent reply other threads:[~2024-01-31 2:45 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-05 3:59 Di Chen
2023-11-20 16:05 ` Aaron Merey
2023-12-20 2:56 ` Di Chen
2024-01-18 0:15 ` Aaron Merey
2024-01-31 2:45 ` Di Chen [this message]
2024-01-31 22:38 ` Aaron Merey
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=CAN-Pu7Q_Rz12Qq7xjR+EB-QFMNpzOpLBDcOY93pdaDufMqcsJQ@mail.gmail.com \
--to=dichen@redhat.com \
--cc=amerey@redhat.com \
--cc=systemtap@sourceware.org \
/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).