* [PATCH v4] Fix signal unsafe call inside a signal
@ 2024-10-28 4:47 Bernd Edlinger
2024-10-29 10:27 ` Andrew Burgess
0 siblings, 1 reply; 3+ messages in thread
From: Bernd Edlinger @ 2024-10-28 4:47 UTC (permalink / raw)
To: gdb-patches, Andrew Burgess
It can easily happen that the signal handler function
`handle_fatal_signal` uses various signal unsafe functions.
The problematic functions are `_` and `strsignal` which
can be pre-computed after the `setlocale` call is done.
Unfortunately when compiled with --disable-libbacktrace a
different code path is used, that calls the glibc function
`backtrace` which calls `malloc` and `free` and is therefore
also signal unsafe, that is probably unfixable, so there
is no attempt to fix anything in this code path.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31713#c9
---
gdb/bt-utils.c | 19 +++++++++--
gdb/bt-utils.h | 2 ++
gdb/event-top.c | 84 ++++++++++++++++++++++++++++++++++++++++++++-----
gdb/main.c | 2 ++
4 files changed, 98 insertions(+), 9 deletions(-)
v2: moved initalization of language specific string to an init function
v3: addressed review comments
v4: improved commit message, addressed review commments
diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c
index f658ce0d4bc..fb006c29178 100644
--- a/gdb/bt-utils.c
+++ b/gdb/bt-utils.c
@@ -145,8 +145,23 @@ gdb_internal_backtrace_1 ()
#else
#error "unexpected internal backtrace policy"
#endif
+
+static const char *str_backtrace = "----- Backtrace -----\n";
+static const char *str_backtrace_unavailable = "Backtrace unavailable\n";
+
#endif /* GDB_PRINT_INTERNAL_BACKTRACE */
+/* Initialize language specific strings. */
+
+void
+gdb_internal_backtrace_init_str ()
+{
+#ifdef GDB_PRINT_INTERNAL_BACKTRACE
+ str_backtrace = _("----- Backtrace -----\n");
+ str_backtrace_unavailable = _("Backtrace unavailable\n");
+#endif
+}
+
/* See bt-utils.h. */
void
@@ -161,12 +176,12 @@ gdb_internal_backtrace ()
gdb_stderr->write_async_safe (msg, strlen (msg));
};
- sig_write (_("----- Backtrace -----\n"));
+ sig_write (str_backtrace);
if (gdb_stderr->fd () > -1)
gdb_internal_backtrace_1 ();
else
- sig_write (_("Backtrace unavailable\n"));
+ sig_write (str_backtrace_unavailable);
sig_write ("---------------------\n");
#endif
diff --git a/gdb/bt-utils.h b/gdb/bt-utils.h
index ec2d14a5484..6fe7c8c4f3b 100644
--- a/gdb/bt-utils.h
+++ b/gdb/bt-utils.h
@@ -71,4 +71,6 @@ extern void gdb_internal_backtrace ();
extern void gdb_internal_backtrace_set_cmd (const char *args, int from_tty,
cmd_list_element *c);
+extern void gdb_internal_backtrace_init_str ();
+
#endif /* BT_UTILS_H */
diff --git a/gdb/event-top.c b/gdb/event-top.c
index d3cf144958a..e7556f71453 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -908,6 +908,51 @@ unblock_signal (int sig)
return false;
}
+/* Signal safe language specific strings. */
+
+#ifdef GDB_PRINT_INTERNAL_BACKTRACE
+static const char *str_fatal_signal;
+static const char *str_sigsegv;
+#ifdef SIGFPE
+static const char *str_sigfpe;
+#endif
+#ifdef SIGBUS
+static const char *str_sigbus;
+#endif
+#ifdef SIGABRT
+static const char *str_sigabrt;
+#endif
+static const char *str_unknown_signal;
+static const char *str_fatal_error_detected_gdb_will_now_terminate;
+static const char *str_this_is_a_bug;
+static const char *str_for_instructions_see;
+
+/* Initialize language specific strings. */
+
+static void
+init_str_handle_fatal_signal ()
+{
+ str_fatal_signal = _("Fatal signal: ");
+ str_sigsegv = strsignal (SIGSEGV);
+#ifdef SIGFPE
+ str_sigfpe = strsignal (SIGFPE);
+#endif
+#ifdef SIGBUS
+ str_sigbus = strsignal (SIGBUS);
+#endif
+#ifdef SIGABRT
+ str_sigabrt = strsignal (SIGABRT);
+#endif
+ str_unknown_signal = _("Unknown signal");
+ str_fatal_error_detected_gdb_will_now_terminate =
+ _("A fatal error internal to GDB has been detected, "
+ "further\ndebugging is not possible. GDB will now "
+ "terminate.\n\n");
+ str_this_is_a_bug = _("This is a bug, please report it.");
+ str_for_instructions_see = _(" For instructions, see:\n");
+}
+#endif
+
/* Called to handle fatal signals. SIG is the signal number. */
[[noreturn]] static void
@@ -926,19 +971,40 @@ handle_fatal_signal (int sig)
if (bt_on_fatal_signal)
{
sig_write ("\n\n");
- sig_write (_("Fatal signal: "));
- sig_write (strsignal (sig));
+ sig_write (str_fatal_signal);
+ switch (sig)
+ {
+ case SIGSEGV:
+ sig_write (str_sigsegv);
+ break;
+#ifdef SIGFPE
+ case SIGFPE:
+ sig_write (str_sigfpe);
+ break;
+#endif
+#ifdef SIGBUS
+ case SIGBUS:
+ sig_write (str_sigbus);
+ break;
+#endif
+#ifdef SIGABRT
+ case SIGABRT:
+ sig_write (str_sigabrt);
+ break;
+#endif
+ default:
+ sig_write (str_unknown_signal);
+ break;
+ }
sig_write ("\n");
gdb_internal_backtrace ();
- sig_write (_("A fatal error internal to GDB has been detected, "
- "further\ndebugging is not possible. GDB will now "
- "terminate.\n\n"));
- sig_write (_("This is a bug, please report it."));
+ sig_write (str_fatal_error_detected_gdb_will_now_terminate);
+ sig_write (str_this_is_a_bug);
if (REPORT_BUGS_TO[0] != '\0')
{
- sig_write (_(" For instructions, see:\n"));
+ sig_write (str_for_instructions_see);
sig_write (REPORT_BUGS_TO);
sig_write (".");
}
@@ -1066,6 +1132,10 @@ gdb_init_signals (void)
create_async_signal_handler (async_sigtstp_handler, NULL, "sigtstp");
#endif
+#ifdef GDB_PRINT_INTERNAL_BACKTRACE
+ init_str_handle_fatal_signal ();
+#endif
+
#ifdef SIGFPE
signal (SIGFPE, handle_fatal_signal);
#endif
diff --git a/gdb/main.c b/gdb/main.c
index e4a40c51023..4370e95ada4 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -58,6 +58,7 @@
#include "observable.h"
#include "serial.h"
#include "cli-out.h"
+#include "bt-utils.h"
/* The selected interpreter. */
std::string interpreter_p;
@@ -676,6 +677,7 @@ captured_main_1 (struct captured_main_args *context)
/* Note: `error' cannot be called before this point, because the
caller will crash when trying to print the exception. */
main_ui = new ui (stdin, stdout, stderr);
+ gdb_internal_backtrace_init_str ();
current_ui = main_ui;
gdb_stdtarg = gdb_stderr;
--
2.39.2
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v4] Fix signal unsafe call inside a signal
2024-10-28 4:47 [PATCH v4] Fix signal unsafe call inside a signal Bernd Edlinger
@ 2024-10-29 10:27 ` Andrew Burgess
2024-10-29 15:05 ` Bernd Edlinger
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Burgess @ 2024-10-29 10:27 UTC (permalink / raw)
To: Bernd Edlinger, gdb-patches
Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
> It can easily happen that the signal handler function
> `handle_fatal_signal` uses various signal unsafe functions.
> The problematic functions are `_` and `strsignal` which
> can be pre-computed after the `setlocale` call is done.
>
> Unfortunately when compiled with --disable-libbacktrace a
> different code path is used, that calls the glibc function
> `backtrace` which calls `malloc` and `free` and is therefore
> also signal unsafe, that is probably unfixable, so there
> is no attempt to fix anything in this code path.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31713#c9
> ---
> gdb/bt-utils.c | 19 +++++++++--
> gdb/bt-utils.h | 2 ++
> gdb/event-top.c | 84 ++++++++++++++++++++++++++++++++++++++++++++-----
> gdb/main.c | 2 ++
> 4 files changed, 98 insertions(+), 9 deletions(-)
>
> v2: moved initalization of language specific string to an init function
> v3: addressed review comments
> v4: improved commit message, addressed review commments
>
> diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c
> index f658ce0d4bc..fb006c29178 100644
> --- a/gdb/bt-utils.c
> +++ b/gdb/bt-utils.c
> @@ -145,8 +145,23 @@ gdb_internal_backtrace_1 ()
> #else
> #error "unexpected internal backtrace policy"
> #endif
> +
> +static const char *str_backtrace = "----- Backtrace -----\n";
> +static const char *str_backtrace_unavailable = "Backtrace unavailable\n";
> +
> #endif /* GDB_PRINT_INTERNAL_BACKTRACE */
>
> +/* Initialize language specific strings. */
Move this comment into the header file please. Then place a comment
here that just says:
/* See bt-utils.h. */
With that change made:
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
> +
> +void
> +gdb_internal_backtrace_init_str ()
> +{
> +#ifdef GDB_PRINT_INTERNAL_BACKTRACE
> + str_backtrace = _("----- Backtrace -----\n");
> + str_backtrace_unavailable = _("Backtrace unavailable\n");
> +#endif
> +}
> +
> /* See bt-utils.h. */
>
> void
> @@ -161,12 +176,12 @@ gdb_internal_backtrace ()
> gdb_stderr->write_async_safe (msg, strlen (msg));
> };
>
> - sig_write (_("----- Backtrace -----\n"));
> + sig_write (str_backtrace);
>
> if (gdb_stderr->fd () > -1)
> gdb_internal_backtrace_1 ();
> else
> - sig_write (_("Backtrace unavailable\n"));
> + sig_write (str_backtrace_unavailable);
>
> sig_write ("---------------------\n");
> #endif
> diff --git a/gdb/bt-utils.h b/gdb/bt-utils.h
> index ec2d14a5484..6fe7c8c4f3b 100644
> --- a/gdb/bt-utils.h
> +++ b/gdb/bt-utils.h
> @@ -71,4 +71,6 @@ extern void gdb_internal_backtrace ();
> extern void gdb_internal_backtrace_set_cmd (const char *args, int from_tty,
> cmd_list_element *c);
>
> +extern void gdb_internal_backtrace_init_str ();
> +
> #endif /* BT_UTILS_H */
> diff --git a/gdb/event-top.c b/gdb/event-top.c
> index d3cf144958a..e7556f71453 100644
> --- a/gdb/event-top.c
> +++ b/gdb/event-top.c
> @@ -908,6 +908,51 @@ unblock_signal (int sig)
> return false;
> }
>
> +/* Signal safe language specific strings. */
> +
> +#ifdef GDB_PRINT_INTERNAL_BACKTRACE
> +static const char *str_fatal_signal;
> +static const char *str_sigsegv;
> +#ifdef SIGFPE
> +static const char *str_sigfpe;
> +#endif
> +#ifdef SIGBUS
> +static const char *str_sigbus;
> +#endif
> +#ifdef SIGABRT
> +static const char *str_sigabrt;
> +#endif
> +static const char *str_unknown_signal;
> +static const char *str_fatal_error_detected_gdb_will_now_terminate;
> +static const char *str_this_is_a_bug;
> +static const char *str_for_instructions_see;
> +
> +/* Initialize language specific strings. */
> +
> +static void
> +init_str_handle_fatal_signal ()
> +{
> + str_fatal_signal = _("Fatal signal: ");
> + str_sigsegv = strsignal (SIGSEGV);
> +#ifdef SIGFPE
> + str_sigfpe = strsignal (SIGFPE);
> +#endif
> +#ifdef SIGBUS
> + str_sigbus = strsignal (SIGBUS);
> +#endif
> +#ifdef SIGABRT
> + str_sigabrt = strsignal (SIGABRT);
> +#endif
> + str_unknown_signal = _("Unknown signal");
> + str_fatal_error_detected_gdb_will_now_terminate =
> + _("A fatal error internal to GDB has been detected, "
> + "further\ndebugging is not possible. GDB will now "
> + "terminate.\n\n");
> + str_this_is_a_bug = _("This is a bug, please report it.");
> + str_for_instructions_see = _(" For instructions, see:\n");
> +}
> +#endif
> +
> /* Called to handle fatal signals. SIG is the signal number. */
>
> [[noreturn]] static void
> @@ -926,19 +971,40 @@ handle_fatal_signal (int sig)
> if (bt_on_fatal_signal)
> {
> sig_write ("\n\n");
> - sig_write (_("Fatal signal: "));
> - sig_write (strsignal (sig));
> + sig_write (str_fatal_signal);
> + switch (sig)
> + {
> + case SIGSEGV:
> + sig_write (str_sigsegv);
> + break;
> +#ifdef SIGFPE
> + case SIGFPE:
> + sig_write (str_sigfpe);
> + break;
> +#endif
> +#ifdef SIGBUS
> + case SIGBUS:
> + sig_write (str_sigbus);
> + break;
> +#endif
> +#ifdef SIGABRT
> + case SIGABRT:
> + sig_write (str_sigabrt);
> + break;
> +#endif
> + default:
> + sig_write (str_unknown_signal);
> + break;
> + }
> sig_write ("\n");
>
> gdb_internal_backtrace ();
>
> - sig_write (_("A fatal error internal to GDB has been detected, "
> - "further\ndebugging is not possible. GDB will now "
> - "terminate.\n\n"));
> - sig_write (_("This is a bug, please report it."));
> + sig_write (str_fatal_error_detected_gdb_will_now_terminate);
> + sig_write (str_this_is_a_bug);
> if (REPORT_BUGS_TO[0] != '\0')
> {
> - sig_write (_(" For instructions, see:\n"));
> + sig_write (str_for_instructions_see);
> sig_write (REPORT_BUGS_TO);
> sig_write (".");
> }
> @@ -1066,6 +1132,10 @@ gdb_init_signals (void)
> create_async_signal_handler (async_sigtstp_handler, NULL, "sigtstp");
> #endif
>
> +#ifdef GDB_PRINT_INTERNAL_BACKTRACE
> + init_str_handle_fatal_signal ();
> +#endif
> +
> #ifdef SIGFPE
> signal (SIGFPE, handle_fatal_signal);
> #endif
> diff --git a/gdb/main.c b/gdb/main.c
> index e4a40c51023..4370e95ada4 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -58,6 +58,7 @@
> #include "observable.h"
> #include "serial.h"
> #include "cli-out.h"
> +#include "bt-utils.h"
>
> /* The selected interpreter. */
> std::string interpreter_p;
> @@ -676,6 +677,7 @@ captured_main_1 (struct captured_main_args *context)
> /* Note: `error' cannot be called before this point, because the
> caller will crash when trying to print the exception. */
> main_ui = new ui (stdin, stdout, stderr);
> + gdb_internal_backtrace_init_str ();
> current_ui = main_ui;
>
> gdb_stdtarg = gdb_stderr;
> --
> 2.39.2
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v4] Fix signal unsafe call inside a signal
2024-10-29 10:27 ` Andrew Burgess
@ 2024-10-29 15:05 ` Bernd Edlinger
0 siblings, 0 replies; 3+ messages in thread
From: Bernd Edlinger @ 2024-10-29 15:05 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 10/29/24 11:27, Andrew Burgess wrote:
> Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
>
>> It can easily happen that the signal handler function
>> `handle_fatal_signal` uses various signal unsafe functions.
>> The problematic functions are `_` and `strsignal` which
>> can be pre-computed after the `setlocale` call is done.
>>
>> Unfortunately when compiled with --disable-libbacktrace a
>> different code path is used, that calls the glibc function
>> `backtrace` which calls `malloc` and `free` and is therefore
>> also signal unsafe, that is probably unfixable, so there
>> is no attempt to fix anything in this code path.
>>
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31713#c9
>> ---
>> gdb/bt-utils.c | 19 +++++++++--
>> gdb/bt-utils.h | 2 ++
>> gdb/event-top.c | 84 ++++++++++++++++++++++++++++++++++++++++++++-----
>> gdb/main.c | 2 ++
>> 4 files changed, 98 insertions(+), 9 deletions(-)
>>
>> v2: moved initalization of language specific string to an init function
>> v3: addressed review comments
>> v4: improved commit message, addressed review commments
>>
>> diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c
>> index f658ce0d4bc..fb006c29178 100644
>> --- a/gdb/bt-utils.c
>> +++ b/gdb/bt-utils.c
>> @@ -145,8 +145,23 @@ gdb_internal_backtrace_1 ()
>> #else
>> #error "unexpected internal backtrace policy"
>> #endif
>> +
>> +static const char *str_backtrace = "----- Backtrace -----\n";
>> +static const char *str_backtrace_unavailable = "Backtrace unavailable\n";
>> +
>> #endif /* GDB_PRINT_INTERNAL_BACKTRACE */
>>
>> +/* Initialize language specific strings. */
>
> Move this comment into the header file please. Then place a comment
> here that just says:
>
> /* See bt-utils.h. */
>
> With that change made:
>
> Approved-By: Andrew Burgess <aburgess@redhat.com>
>
> Thanks,
> Andrew
>
Ok, pushed as 7ffd4868a7e22dd2f8126822501bfa18479d715c.
Thanks
Bernd.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-10-29 15:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-28 4:47 [PATCH v4] Fix signal unsafe call inside a signal Bernd Edlinger
2024-10-29 10:27 ` Andrew Burgess
2024-10-29 15:05 ` Bernd Edlinger
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).