[PATCH 1/2] Suppress printing of superfluous BFD error messages
Lancelot SIX
lsix@lancelotsix.com
Tue Sep 6 10:09:24 GMT 2022
Hi Kevin
On Fri, Sep 02, 2022 at 05:47:58PM -0700, Kevin Buettner via Gdb-patches wrote:
> This commit adds a hook to the BFD error handler for suppressing
> identical messages which have been output once already.
>
> It's motivated by this Fedora bug...
>
> https://bugzilla.redhat.com/show_bug.cgi?id=2083315
>
> ...in which over 900,000 BFD error messages are output when attaching
> to firefox. From the bug report, the messages all say:
>
> BFD: /usr/lib/debug/usr/lib64/firefox/libxul.so-100.0-2.fc35.x86_64.debug: attempt to load strings from a non-string section (number 38)
>
> Since there's no (additional) context which might assist the user in
> determining what's wrong, there's really no point in outputting more
> than one message. Of course, if BFD should output some
> other/different message, it should be output too, but all future
> messages identical to those already output should be suppressed.
>
> For the firefox problem, it turned out that there were only 37
> sections, but something was referring to section #38. I haven't
> investigated further to find out how this came to be.
>
> Despite this problem, useful debugging might still be done, especially
> if the user doesn't care about debugging the problematic library.
>
> If it turns out that knowing the quantity of messages might be useful,
> I've implemented the suppression mechanism by keeping a count of each
> identical message. A new GDB command, perhaps a 'maintenance'
> command, could be added to print out each message along with the
> count. I haven't implemented this though because I'm not convinced of
> its utility. Also, the BFD message printer has support for BFD-
> specific format specifiers. The BFD message strings that GDB stores
> in a its map are sufficient for distinguishing messages from each
> other, but are not identical to those output by BFD's default error
> handler. So, that problem would need to be solved too.
> ---
> gdb/gdb_bfd.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 71 insertions(+)
>
> diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
> index 6c03ae5ef05..e74d649ea16 100644
> --- a/gdb/gdb_bfd.c
> +++ b/gdb/gdb_bfd.c
> @@ -33,6 +33,7 @@
> #include "gdb/fileio.h"
> #include "inferior.h"
> #include "cli/cli-style.h"
> +#include <map>
>
> /* An object of this type is stored in the section's user data when
> mapping a section. */
> @@ -1125,6 +1126,73 @@ maintenance_info_bfds (const char *arg, int from_tty)
> htab_traverse (all_bfds, print_one_bfd, uiout);
> }
>
> +/* BFD related per-inferior data. */
> +
> +struct bfd_inferior_data
> +{
> + std::map<std::string, unsigned long> bfd_error_string_counts;
> +};
> +
> +/* Per-inferior data key. */
> +
> +static const registry<inferior>::key<bfd_inferior_data> bfd_inferior_data_key;
> +
> +/* Fetch per-inferior BFD data. It always returns a valid pointer to
> + a bfd_inferior_data struct. */
> +
> +static struct bfd_inferior_data *
> +get_bfd_inferior_data (struct inferior *inf)
> +{
> + struct bfd_inferior_data *data;
> +
> + data = bfd_inferior_data_key.get (inf);
> + if (data == nullptr)
> + data = bfd_inferior_data_key.emplace (inf);
> +
> + return data;
> +}
> +
> +/* Increment the bfd error count for STR and return the updated
> + count. */
> +
> +static unsigned long
> +increment_bfd_error_count (std::string str)
> +{
> + struct bfd_inferior_data *bid = get_bfd_inferior_data (current_inferior ());
> +
> + auto &map = bid->bfd_error_string_counts;
> + if (map.find (str) == map.end ())
> + {
> + map[str] = 0;
> + }
> + return ++map[str];
I think this can all be simplified.
If `str` is not a key in `map`, `map[str]` will default initialize a
value in the map (in this case 0-initialize as it is an unsigned long)
before returning the reference to it.
https://en.cppreference.com/w/cpp/container/map/operator_at says:
If an insertion is performed, the mapped value is value-initialized
(default-constructed for class types, zero-initialized otherwise) and
a reference to it is returned.
As a consequence, this block can all be summed up as:
return ++map[str];
| |
| - 1) Fetch existing value, or 0-initialize one and return
| it
|
- 2) Do the pre-increment.
If you prefer a more explicit option, std::map::insert inserts an
element in a map if not already present, and return a pair containing:
- An iterator to the newly inserted element if an insertion took
place, to the element which was already in the map otherwise
- A bool indicating if an insertion took place.
The above could be re-written:
return ++map.insert ({str, 0ul}).first->second;
This would avoid having to do the lookup twice to increment a counter
(the initial map::find and then map::operator[]), or three times to
initialize if the counter needs to be initialized (map::find and then
map::operator[] twice).
Best,
Lancelot.
> +}
> +
> +static bfd_error_handler_type default_bfd_error_handler;
> +
> +/* Define a BFD error handler which will suppress the printing of
> + messages which have been printed once already. This is done on a
> + per-inferior basis. */
> +
> +static void
> +gdb_bfd_error_handler (const char *fmt, va_list ap)
> +{
> + va_list ap_copy;
> +
> + va_copy(ap_copy, ap);
> + const std::string str = string_vprintf (fmt, ap_copy);
> + va_end (ap_copy);
> +
> + if (increment_bfd_error_count (str) > 1)
> + return;
> +
> + /* We must call the BFD mechanism for printing format strings since
> + it supports additional format specifiers that GDB's vwarning() doesn't
> + recognize. It also outputs additional text, i.e. "BFD: ", which
> + makes it clear that it's a BFD warning/error. */
> + (*default_bfd_error_handler) (fmt, ap);
> +}
> +
> void _initialize_gdb_bfd ();
> void
> _initialize_gdb_bfd ()
> @@ -1157,4 +1225,7 @@ When non-zero, bfd cache specific debugging is enabled."),
> NULL,
> &show_bfd_cache_debug,
> &setdebuglist, &showdebuglist);
> +
> + /* Hook the bfd error/warning handler to limit amount of output. */
> + default_bfd_error_handler = bfd_set_error_handler (gdb_bfd_error_handler);
> }
> --
> 2.37.2
>
More information about the Gdb-patches
mailing list