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 wrote: > Hi Di, > > Thanks for the patch. > > On Tue, Dec 19, 2023 at 9:56 PM Di Chen wrote: > > > > Hey Aaron, > > > > I have added the wildcard handling for the PR29997. > > > > From 0c95ce19272d6e13abf6f41c4ae29b6aa925ca20 Mon Sep 17 00:00:00 2001 > > From: Di Chen > > 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 > > --- > > 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(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(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(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(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 > "@" 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 > ~ > >