From: Aaron Merey <amerey@redhat.com>
To: Di Chen <dichen@redhat.com>
Cc: systemtap@sourceware.org
Subject: Re: [PATCH] PR29997: Fix the symbol aliases search failure when symbol version is missing
Date: Wed, 17 Jan 2024 19:15:01 -0500 [thread overview]
Message-ID: <CAJDtP-Sq8Pp8CPd=FdM_gyVndzmZc=1awze4YooNRk_nbPOj2A@mail.gmail.com> (raw)
In-Reply-To: <CAN-Pu7QsExY_i3f+mdFZn5T3YWxiPWaqgujfzOPMR8C2mmZzNw@mail.gmail.com>
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
~
next prev parent reply other threads:[~2024-01-18 0:15 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 [this message]
2024-01-31 2:45 ` Di Chen
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='CAJDtP-Sq8Pp8CPd=FdM_gyVndzmZc=1awze4YooNRk_nbPOj2A@mail.gmail.com' \
--to=amerey@redhat.com \
--cc=dichen@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).