Re: [PATCH] Add _GLIBCXX_DEBUG backtrace generation

[Jonathan Wakely <jwakely@redhat.com>]

On Wed, 31 Aug 2022 at 06:05, François Dumont <frs.dumont@gmail.com> wrote:
>
> After a second thought here is an even cleaner version. No more function
> rename, current pretty_print is fine.
>
>      libstdc++: [_GLIBCXX_DEBUG] Add backtrace generation on demand
>
>        Add _GLIBCXX_DEBUG_BACKTRACE macro to activate backtrace
> generation on
>      _GLIBCXX_DEBUG assertions. Prerequisite is to have configure the
> lib with:
>
>      --enable-libstdcxx-backtrace=yes
>
>      libstdc++-v3/ChangeLog:
>
>              * include/debug/formatter.h
>              [_GLIBCXX_HAVE_STACKTRACE](__glibcxx_backtrace_state): Declare.
> [_GLIBCXX_HAVE_STACKTRACE](__glibcxx_backtrace_create_state): Declare.
> [_GLIBCXX_HAVE_STACKTRACE](__glibcxx_backtrace_full_callback): Define.
> [_GLIBCXX_HAVE_STACKTRACE](__glibcxx_backtrace_error_callback): Define.
> [_GLIBCXX_HAVE_STACKTRACE](__glibcxx_backtrace_full_func): Define.
>              [_GLIBCXX_HAVE_STACKTRACE](__glibcxx_backtrace_full): Declare.
> [_GLIBCXX_HAVE_STACKTRACE](_Error_formatter::_M_backtrace_state): New.
> [_GLIBCXX_HAVE_STACKTRACE](_Error_formatter::_M_backtrace_full): New.
>              * src/c++11/debug.cc
> [_GLIBCXX_HAVE_STACKTRACE](print_backtrace): New.
>              (_Error_formatter::_M_error()): Adapt.
>              * src/libbacktrace/Makefile.am: Add backtrace.c.
>              * src/libbacktrace/Makefile.in: Regenerate.
>              * src/libbacktrace/backtrace-rename.h (backtrace_full): New.
>              *
> testsuite/23_containers/vector/debug/assign4_backtrace_neg.cc: New test.
>              * doc/xml/manual/debug_mode.xml: Document
> _GLIBCXX_DEBUG_BACKTRACE.
>              * doc/xml/manual/using.xml: Likewise.
> Ok to commit ?

OK for trunk, thanks.

The small change to print_raw in this patch makes me wonder whether
that function is actually useful.

It supports two modes, print with max precision, and print without.
The only time we use it to print with max precision we pass a string
of exactly the right length, so the precision is not needed (but the
caller has to get the string length correct: if we increase _S_indent
and do not increase the "    " literal passed to print_raw, the
effects would be wrong).

Wouldn't it be better to just use fprintf directly when we want to
print without precision, and use a minimum field width instead of
precision for indenting? i.e. ...

--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -608,15 +608,6 @@ namespace
    print_literal(PrintContext& ctx, const char(&word)[Length])
    { print_word(ctx, word, Length - 1); }

-  void
-  print_raw(PrintContext& ctx, const char* str, ptrdiff_t nbc = -1)
-  {
-    if (nbc >= 0)
-      ctx._M_column += fprintf(stderr, "%.*s", (int)nbc, str);
-    else
-      ctx._M_column += fprintf(stderr, "%s", str);
-  }
-
  void
  print_word(PrintContext& ctx, const char* word, ptrdiff_t nbc = -1)
  {
@@ -643,12 +634,9 @@ namespace
       || (ctx._M_column + visual_length < ctx._M_max_length)
       || (visual_length >= ctx._M_max_length && ctx._M_column == 1))
      {
-       // If this isn't the first line, indent
+       // If this isn't the first line, indent.
       if (ctx._M_column == 1 && !ctx._M_first_line)
-         {
-           const char spacing[PrintContext::_S_indent + 1] = "    ";
-           print_raw(ctx, spacing, PrintContext::_S_indent);
-         }
+         ctx._M_column += fprintf(stderr, "%*c", PrintContext::_S_indent, ' ');

       int written = fprintf(stderr, "%.*s", (int)length, word);

@@ -1112,7 +1100,7 @@ namespace __gnu_debug
    PrintContext ctx;
    if (_M_file)
      {
-       print_raw(ctx, _M_file);
+       ctx._M_column += fprintf(stderr, "%s", _M_file);
       print_literal(ctx, ":");
       go_to_next_line = true;
      }