* [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails @ 2021-06-02 17:31 Bernd Edlinger 2021-06-02 18:21 ` Tom Tromey 0 siblings, 1 reply; 20+ messages in thread From: Bernd Edlinger @ 2021-06-02 17:31 UTC (permalink / raw) To: gdb-patches Due to the SIGPIPE the gdb process is killed here, which is not helpful. 2021-06-02 Bernd Edlinger <bernd.edlinger@hotmail.de> * compile/compile.c (compile_to_object): Ignore SIGPIPE before calling the plugin. --- gdb/compile/compile.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c index 8481d14..134a077 100644 --- a/gdb/compile/compile.c +++ b/gdb/compile/compile.c @@ -755,6 +755,12 @@ struct compile_options fprintf_unfiltered (gdb_stdlog, "source file produced: %s\n\n", fnames.source_file ()); +#ifdef SIGPIPE + /* If we don't do this, then GDB simply exits + when the remote side dies. */ + signal (SIGPIPE, SIG_IGN); +#endif + /* Call the compiler and start the compilation process. */ compiler->set_source_file (fnames.source_file ()); ok = compiler->compile (fnames.object_file (), compile_debug); -- 1.9.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-02 17:31 [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails Bernd Edlinger @ 2021-06-02 18:21 ` Tom Tromey 2021-06-02 22:19 ` Christian Biesinger 0 siblings, 1 reply; 20+ messages in thread From: Tom Tromey @ 2021-06-02 18:21 UTC (permalink / raw) To: Bernd Edlinger; +Cc: gdb-patches >>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes: Bernd> Due to the SIGPIPE the gdb process is killed here, which is Bernd> not helpful. Bernd> 2021-06-02 Bernd Edlinger <bernd.edlinger@hotmail.de> Bernd> * compile/compile.c (compile_to_object): Ignore SIGPIPE before calling Bernd> the plugin. I don't understand why gdb gets a SIGPIPE here. Could you explain it a bit more? Tom ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-02 18:21 ` Tom Tromey @ 2021-06-02 22:19 ` Christian Biesinger 2021-06-03 5:45 ` Bernd Edlinger 0 siblings, 1 reply; 20+ messages in thread From: Christian Biesinger @ 2021-06-02 22:19 UTC (permalink / raw) To: Tom Tromey; +Cc: Bernd Edlinger, gdb-patches On Wed, Jun 2, 2021 at 1:22 PM Tom Tromey <tom@tromey.com> wrote: > > >>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes: > > Bernd> Due to the SIGPIPE the gdb process is killed here, which is > Bernd> not helpful. > > Bernd> 2021-06-02 Bernd Edlinger <bernd.edlinger@hotmail.de> > > Bernd> * compile/compile.c (compile_to_object): Ignore SIGPIPE before calling > Bernd> the plugin. > > I don't understand why gdb gets a SIGPIPE here. > Could you explain it a bit more? Also, if we determine that this is necessary, this does not seem like right way to do this. If gdb wants to globally ignore SIGPIPE, it should probably do that in general startup code, maybe in async_init_signals. If it should only be done for this specific call, it should reset it afterwards. Christian ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-02 22:19 ` Christian Biesinger @ 2021-06-03 5:45 ` Bernd Edlinger 2021-06-04 13:39 ` Tom Tromey 0 siblings, 1 reply; 20+ messages in thread From: Bernd Edlinger @ 2021-06-03 5:45 UTC (permalink / raw) To: Christian Biesinger, Tom Tromey; +Cc: gdb-patches On 6/3/21 12:19 AM, Christian Biesinger wrote: > On Wed, Jun 2, 2021 at 1:22 PM Tom Tromey <tom@tromey.com> wrote: >> >>>>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes: >> >> Bernd> Due to the SIGPIPE the gdb process is killed here, which is >> Bernd> not helpful. >> >> Bernd> 2021-06-02 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> Bernd> * compile/compile.c (compile_to_object): Ignore SIGPIPE before calling >> Bernd> the plugin. >> >> I don't understand why gdb gets a SIGPIPE here. >> Could you explain it a bit more? > > Also, if we determine that this is necessary, this does not seem like > right way to do this. If gdb wants to globally ignore SIGPIPE, it > should probably do that in general startup code, maybe in > async_init_signals. If it should only be done for this specific call, > it should reset it afterwards. > Sorry, I apparently sent the reply to the wrong message. I believe this is necessary, since this works around a bug in a plugin that is not in the binutils-gdb source tree. what I did is this: gdb --args gdb ./compile-cplus-anonymous GNU gdb (GDB) 11.0.50.20210601-git ... (gdb) r Starting program: /home/ed/gnu/gdb-install-1/bin/gdb ./compile-cplus-anonymous ... (gdb) b main Haltepunkt 1 at 0x401165: file /home/ed/gnu/gdb-build-1/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.compile/compile-cplus-anonymous.cc, line 71. (gdb) r Starting program: /home/ed/gnu/gdb-build-1/gdb/testsuite/outputs/gdb.compile/compile-cplus-anonymous/compile-cplus-anonymous (gdb) compile code var = anon_e [Detaching after fork from child process 4880] *** WARNUNG *** es gibt aktive Plugins – bitte Fehler nur dann melden, wenn diese auch ohne alle Plugins reproduziert werden können. Ereignis | Plugins PLUGIN_PRE_GENERICIZE | libcp1plugin PLUGIN_GGC_MARKING | libcp1plugin PLUGIN_PRAGMAS | libcp1plugin gdb command line: In Funktion »void _gdb_expr(__gdb_regs*)«: gdb command line:1:1: interner Compiler-Fehler: in set_decl_context_in_fn, bei cp/name-lookup.c:3359 0x6f03a1 set_decl_context_in_fn ../../gcc-trunk/gcc/cp/name-lookup.c:3359 0x6f03a1 do_pushdecl ../../gcc-trunk/gcc/cp/name-lookup.c:3633 0xa99c42 do_pushdecl ../../gcc-trunk/gcc/cp/name-lookup.c:3852 0xa99c42 pushdecl(tree_node*, bool) ../../gcc-trunk/gcc/cp/name-lookup.c:3852 0x7ffff7fa044c safe_pushdecl ../../gcc-trunk/libcc1/libcp1plugin.cc:654 ... 0x7ffff7fa6aa1 cc1_plugin::connection::do_wait(bool) ../../gcc-trunk/libcc1/connection.cc:135 0x7ffff7f98f1d cc1_plugin::connection::wait_for_result() ../../gcc-trunk/libcc1/connection.hh:75 0x7ffff7f98f1d cc1_plugin::status cc1_plugin::call<int, gcc_cp_oracle_request, char const*>(cc1_plugin::connection*, char const*, int*, gcc_cp_oracle_request, char const*) ../../gcc-trunk/libcc1/rpc.hh:116 0x7ffff7f98f1d plugin_binding_oracle ../../gcc-trunk/libcc1/libcp1plugin.cc:101 0xa965e7 query_oracle ../../gcc-trunk/gcc/cp/name-lookup.c:2387 Thread 1 "gdb" received signal SIGPIPE, Broken pipe. 0x00007ffff704b34d in write () from /lib/x86_64-linux-gnu/libpthread.so.0 (gdb) bt #0 0x00007ffff704b34d in write () from /lib/x86_64-linux-gnu/libpthread.so.0 #1 0x00007ffff7e1816f in cc1_plugin::connection::send (this=<optimized out>, c=<optimized out>) at ../../gcc-trunk/libcc1/connection.cc:33 #2 0x00007ffff7e128c4 in cc1_plugin::invoker<int, gcc_cp_oracle_request, char const*>::invoke<(anonymous namespace)::cp_call_binding_oracle> (conn=0xee03b0) at ../../gcc-trunk/libcc1/libcp1.cc:81 #3 0x00007ffff7e183c2 in cc1_plugin::connection::do_wait (this=0xee03b0, want_result=<optimized out>) at ../../gcc-trunk/libcc1/connection.cc:135 #4 0x00007ffff7e1708c in cc1_plugin::connection::wait_for_query (this=<optimized out>) at ../../gcc-trunk/libcc1/connection.hh:82 #5 cc1_plugin::base_gdb_plugin<gcc_cp_context>::fork_exec (stderr_fds=0x7fffffffd778, spair_fds=0x7fffffffd770, argv=0x12fe060, this=<optimized out>) at ../../gcc-trunk/libcc1/gdbctx.hh:248 #6 cc1_plugin::base_gdb_plugin<gcc_cp_context>::do_compile (s=<optimized out>, filename=<optimized out>) at ../../gcc-trunk/libcc1/gdbctx.hh:315 #7 0x00000000005058e4 in compile_instance::compile (this=<optimized out>, filename=<optimized out>, verbose_level=<optimized out>) at ../../binutils-gdb/gdb/compile/compile.c:914 #8 0x0000000000506387 in compile_to_object (scope=<optimized out>, cmd_string=<optimized out>, cmd=<optimized out>) at /home/ed/gnu/install/include/c++/12.0.0/bits/basic_string.h:193 #9 eval_compile_command (cmd=0x0, cmd_string=0x124bb4d "var = anon_e", scope=COMPILE_I_SIMPLE_SCOPE, scope_data=0x0) at ../../binutils-gdb/gdb/compile/compile.c:789 #10 0x00000000005070dc in compile_code_command (args=<optimized out>, from_tty=<optimized out>) at ../../binutils-gdb/gdb/compile/compile.c:341 #11 0x00000000004ddbb2 in cmd_func (cmd=<optimized out>, args=<optimized out>, from_tty=<optimized out>) at ../../binutils-gdb/gdb/cli/cli-decode.c:2176 #12 0x0000000000776c82 in execute_command (p=<optimized out>, p@entry=0x124bb40 "compile code var = anon_e", from_tty=1) at ../../binutils-gdb/gdb/top.c:674 #13 0x000000000059e8ed in command_handler (command=0x124bb40 "compile code var = anon_e") at ../../binutils-gdb/gdb/event-top.c:588 #14 0x000000000059ec0b in command_line_handler (rl=...) at ../../binutils-gdb/gdb/event-top.c:773 #15 0x000000000059f1d0 in gdb_rl_callback_handler (rl=0xcce3c0 "compile code var = anon_e") at ../../binutils-gdb/gdb/event-top.c:218 #16 0x00000000007f8e08 in rl_callback_read_char () at ../../../binutils-gdb/readline/readline/callback.c:281 #17 0x000000000059db2e in gdb_rl_callback_read_char_wrapper_noexcept () at ../../binutils-gdb/gdb/event-top.c:176 #18 0x000000000059f0be in gdb_rl_callback_read_char_wrapper (client_data=<optimized out>) at ../../binutils-gdb/gdb/event-top.c:193 #19 0x000000000059d940 in stdin_event_handler (error=<optimized out>, client_data=0xcccd10) at ../../binutils-gdb/gdb/event-top.c:515 #20 0x00000000008ca836 in gdb_wait_for_event (block=block@entry=1) at ../../binutils-gdb/gdbsupport/event-loop.cc:701 #21 0x00000000008caa7d in gdb_wait_for_event (block=1) at ../../binutils-gdb/gdbsupport/event-loop.cc:597 #22 gdb_do_one_event () at ../../binutils-gdb/gdbsupport/event-loop.cc:237 #23 0x0000000000654f05 in start_event_loop () at ../../binutils-gdb/gdb/main.c:421 #24 captured_command_loop () at ../../binutils-gdb/gdb/main.c:481 #25 0x00000000006568d5 in captured_main (data=data@entry=0x7fffffffdd80) at ../../binutils-gdb/gdb/main.c:1353 #26 gdb_main (args=args@entry=0x7fffffffddb0) at ../../binutils-gdb/gdb/main.c:1368 #27 0x0000000000427b65 in main (argc=<optimized out>, argv=<optimized out>) at ../../binutils-gdb/gdb/gdb.c:32 So the plugin forks, executes gcc, and does something that causes gcc to crash. Now the plugin uses a pipe to send more commands to the already died gcc, and since the write is sending data to a closed pipe, the SIGPIPE is sent to the gdb process which dies immediately. I believe it is right to avoid the SIGPIPE before calling the plugin, instead of doing that in gcc-trunk, since we don't know which version we will be calling, and all versions I tried have failed like this. Bernd. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-03 5:45 ` Bernd Edlinger @ 2021-06-04 13:39 ` Tom Tromey 2021-06-05 11:44 ` Bernd Edlinger 2021-06-14 23:35 ` Pedro Alves 0 siblings, 2 replies; 20+ messages in thread From: Tom Tromey @ 2021-06-04 13:39 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Christian Biesinger, Tom Tromey, gdb-patches >>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes: Bernd> I believe it is right to avoid the SIGPIPE before calling the plugin, Bernd> instead of doing that in gcc-trunk, since we don't know which version Bernd> we will be calling, and all versions I tried have failed like this. That seems fine, but I think it would be better to install the handler just when working with the plugin, and then uninstall it afterward, sort of like what class scoped_ignore_sigttou does. Tom ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-04 13:39 ` Tom Tromey @ 2021-06-05 11:44 ` Bernd Edlinger 2021-06-05 12:04 ` Andrew Burgess 2021-06-14 11:41 ` Rainer Orth 2021-06-14 23:35 ` Pedro Alves 1 sibling, 2 replies; 20+ messages in thread From: Bernd Edlinger @ 2021-06-05 11:44 UTC (permalink / raw) To: Tom Tromey; +Cc: Christian Biesinger, gdb-patches [-- Attachment #1: Type: text/plain, Size: 609 bytes --] On 6/4/21 3:39 PM, Tom Tromey wrote: >>>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes: > > Bernd> I believe it is right to avoid the SIGPIPE before calling the plugin, > Bernd> instead of doing that in gcc-trunk, since we don't know which version > Bernd> we will be calling, and all versions I tried have failed like this. > > That seems fine, but I think it would be better to install the handler > just when working with the plugin, and then uninstall it afterward, sort > of like what class scoped_ignore_sigttou does. > Okay, done, that works for me. Is this OK? Thanks Bernd. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Fix-gdb-crash-due-to-SIGPIPE-when-the-compile-comman.patch --] [-- Type: text/x-patch; name="0001-Fix-gdb-crash-due-to-SIGPIPE-when-the-compile-comman.patch", Size: 1833 bytes --] From 3bb3966601a89c18ea2700c3eae4aa4f4e195269 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger <bernd.edlinger@hotmail.de> Date: Wed, 2 Jun 2021 19:21:15 +0200 Subject: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails Due to the SIGPIPE the gdb process is killed here, which is not helpful. 2021-06-02 Bernd Edlinger <bernd.edlinger@hotmail.de> * compile/compile.c (compile_to_object): Ignore SIGPIPE before calling the plugin. --- gdb/compile/compile.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c index 8481d14..3431d4c 100644 --- a/gdb/compile/compile.c +++ b/gdb/compile/compile.c @@ -633,6 +633,33 @@ struct compile_options fputs_filtered (message, gdb_stderr); } +/* RAII class used to ignore SIGPIPE in a scope. */ + +class scoped_ignore_sigpipe +{ +public: + scoped_ignore_sigpipe () + { +#ifdef SIGPIPE + m_osigpipe = signal (SIGPIPE, SIG_IGN); +#endif + } + + ~scoped_ignore_sigpipe () + { +#ifdef SIGTTOU + signal (SIGPIPE, m_osigpipe); +#endif + } + + DISABLE_COPY_AND_ASSIGN (scoped_ignore_sigpipe); + +private: +#ifdef SIGPIPE + sighandler_t m_osigpipe = NULL; +#endif +}; + /* Process the compilation request. On success it returns the object and source file names. On an error condition, error () is called. */ @@ -755,6 +782,10 @@ struct compile_options fprintf_unfiltered (gdb_stdlog, "source file produced: %s\n\n", fnames.source_file ()); + /* If we don't do this, then GDB simply exits + when the compiler dies. */ + scoped_ignore_sigpipe ignore_sigpipe; + /* Call the compiler and start the compilation process. */ compiler->set_source_file (fnames.source_file ()); ok = compiler->compile (fnames.object_file (), compile_debug); -- 1.9.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-05 11:44 ` Bernd Edlinger @ 2021-06-05 12:04 ` Andrew Burgess 2021-06-05 12:27 ` Bernd Edlinger 2021-06-14 11:41 ` Rainer Orth 1 sibling, 1 reply; 20+ messages in thread From: Andrew Burgess @ 2021-06-05 12:04 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Tom Tromey, gdb-patches * Bernd Edlinger <bernd.edlinger@hotmail.de> [2021-06-05 13:44:03 +0200]: > On 6/4/21 3:39 PM, Tom Tromey wrote: > >>>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes: > > > > Bernd> I believe it is right to avoid the SIGPIPE before calling the plugin, > > Bernd> instead of doing that in gcc-trunk, since we don't know which version > > Bernd> we will be calling, and all versions I tried have failed like this. > > > > That seems fine, but I think it would be better to install the handler > > just when working with the plugin, and then uninstall it afterward, sort > > of like what class scoped_ignore_sigttou does. > > > > Okay, done, that works for me. > > Is this OK? > > > Thanks > Bernd. > > From 3bb3966601a89c18ea2700c3eae4aa4f4e195269 Mon Sep 17 00:00:00 2001 > From: Bernd Edlinger <bernd.edlinger@hotmail.de> > Date: Wed, 2 Jun 2021 19:21:15 +0200 > Subject: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails > > Due to the SIGPIPE the gdb process is killed here, which is > not helpful. > > 2021-06-02 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * compile/compile.c (compile_to_object): Ignore SIGPIPE before calling > the plugin. > --- > gdb/compile/compile.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c > index 8481d14..3431d4c 100644 > --- a/gdb/compile/compile.c > +++ b/gdb/compile/compile.c > @@ -633,6 +633,33 @@ struct compile_options > fputs_filtered (message, gdb_stderr); > } > > +/* RAII class used to ignore SIGPIPE in a scope. */ > + > +class scoped_ignore_sigpipe > +{ > +public: > + scoped_ignore_sigpipe () > + { > +#ifdef SIGPIPE > + m_osigpipe = signal (SIGPIPE, SIG_IGN); > +#endif > + } > + > + ~scoped_ignore_sigpipe () > + { > +#ifdef SIGTTOU SIGPIPE? Thanks, Andrew > + signal (SIGPIPE, m_osigpipe); > +#endif > + } > + > + DISABLE_COPY_AND_ASSIGN (scoped_ignore_sigpipe); > + > +private: > +#ifdef SIGPIPE > + sighandler_t m_osigpipe = NULL; > +#endif > +}; > + > /* Process the compilation request. On success it returns the object > and source file names. On an error condition, error () is > called. */ > @@ -755,6 +782,10 @@ struct compile_options > fprintf_unfiltered (gdb_stdlog, "source file produced: %s\n\n", > fnames.source_file ()); > > + /* If we don't do this, then GDB simply exits > + when the compiler dies. */ > + scoped_ignore_sigpipe ignore_sigpipe; > + > /* Call the compiler and start the compilation process. */ > compiler->set_source_file (fnames.source_file ()); > ok = compiler->compile (fnames.object_file (), compile_debug); > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-05 12:04 ` Andrew Burgess @ 2021-06-05 12:27 ` Bernd Edlinger 2021-06-05 14:04 ` Tom Tromey 0 siblings, 1 reply; 20+ messages in thread From: Bernd Edlinger @ 2021-06-05 12:27 UTC (permalink / raw) To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches On 6/5/21 2:04 PM, Andrew Burgess wrote: > * Bernd Edlinger <bernd.edlinger@hotmail.de> [2021-06-05 13:44:03 +0200]: > >> On 6/4/21 3:39 PM, Tom Tromey wrote: >>>>>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes: >>> Bernd> I believe it is right to avoid the SIGPIPE before calling the plugin, >>> Bernd> instead of doing that in gcc-trunk, since we don't know which version >>> Bernd> we will be calling, and all versions I tried have failed like this. >>> >>> That seems fine, but I think it would be better to install the handler >>> just when working with the plugin, and then uninstall it afterward, sort >>> of like what class scoped_ignore_sigttou does. >>> >> Okay, done, that works for me. >> >> Is this OK? >> >> >> Thanks >> Bernd. >> >> From 3bb3966601a89c18ea2700c3eae4aa4f4e195269 Mon Sep 17 00:00:00 2001 >> From: Bernd Edlinger <bernd.edlinger@hotmail.de> >> Date: Wed, 2 Jun 2021 19:21:15 +0200 >> Subject: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails >> >> Due to the SIGPIPE the gdb process is killed here, which is >> not helpful. >> >> 2021-06-02 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> * compile/compile.c (compile_to_object): Ignore SIGPIPE before calling >> the plugin. >> --- >> gdb/compile/compile.c | 31 +++++++++++++++++++++++++++++++ >> 1 file changed, 31 insertions(+) >> >> diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c >> index 8481d14..3431d4c 100644 >> --- a/gdb/compile/compile.c >> +++ b/gdb/compile/compile.c >> @@ -633,6 +633,33 @@ struct compile_options >> fputs_filtered (message, gdb_stderr); >> } >> >> +/* RAII class used to ignore SIGPIPE in a scope. */ >> + >> +class scoped_ignore_sigpipe >> +{ >> +public: >> + scoped_ignore_sigpipe () >> + { >> +#ifdef SIGPIPE >> + m_osigpipe = signal (SIGPIPE, SIG_IGN); >> +#endif >> + } >> + >> + ~scoped_ignore_sigpipe () >> + { >> +#ifdef SIGTTOU > SIGPIPE? > Ah, yes of course! Consider this fixed. Thanks, Bernd. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-05 12:27 ` Bernd Edlinger @ 2021-06-05 14:04 ` Tom Tromey 0 siblings, 0 replies; 20+ messages in thread From: Tom Tromey @ 2021-06-05 14:04 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Andrew Burgess, Tom Tromey, gdb-patches >>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes: >>> +#ifdef SIGTTOU >> SIGPIPE? >> Bernd> Ah, yes of course! Bernd> Consider this fixed. Thanks, this is ok with this fix. I thought at first that '#ifdef SIGPIPE' was overkill, but I see that the other uses of SIGPIPE in the tree are guarded, so we might as well do it here. Also I see that gdb already randomly disables SIGPIPE globally, e.g. in ser-tcp.c. So maybe your original patch should have gone in. But, I suppose I'd still prefer that the new one land, since it's clearly cleaner. thank you, Tom ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-05 11:44 ` Bernd Edlinger 2021-06-05 12:04 ` Andrew Burgess @ 2021-06-14 11:41 ` Rainer Orth 2021-06-14 12:57 ` Bernd Edlinger 1 sibling, 1 reply; 20+ messages in thread From: Rainer Orth @ 2021-06-14 11:41 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Tom Tromey, gdb-patches Hi Bernd, > On 6/4/21 3:39 PM, Tom Tromey wrote: >>>>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes: >> >> Bernd> I believe it is right to avoid the SIGPIPE before calling the plugin, >> Bernd> instead of doing that in gcc-trunk, since we don't know which version >> Bernd> we will be calling, and all versions I tried have failed like this. >> >> That seems fine, but I think it would be better to install the handler >> just when working with the plugin, and then uninstall it afterward, sort >> of like what class scoped_ignore_sigttou does. >> > > Okay, done, that works for me. > > Is this OK? this patch broke the Solaris build: /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c:659:3: error: ‘sighandler_t’ does not name a type; did you mean ‘sa_handler’? 659 | sighandler_t m_osigpipe = NULL; | ^~~~~~~~~~~~ | sa_handler /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c: In constructor ‘scoped_ignore_sigpipe::scoped_ignore_sigpipe()’: /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c:644:5: error: ‘m_osigpipe’ was not declared in this scope 644 | m_osigpipe = signal (SIGPIPE, SIG_IGN); | ^~~~~~~~~~ /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c:644:18: error: ‘signal’ was not declared in this scope; did you mean ‘sigval’? 644 | m_osigpipe = signal (SIGPIPE, SIG_IGN); | ^~~~~~ | sigval /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c: In destructor ‘scoped_ignore_sigpipe::~scoped_ignore_sigpipe()’: /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c:651:22: error: ‘m_osigpipe’ was not declared in this scope 651 | signal (SIGPIPE, m_osigpipe); | ^~~~~~~~~~ /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c:651:5: error: ‘signal’ was not declared in this scope; did you mean ‘sigval’? 651 | signal (SIGPIPE, m_osigpipe); | ^~~~~~ | sigval and I have a very hard time deciding where <signal.h> needs to be included given the enormous range of headers included by compile.c. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-14 11:41 ` Rainer Orth @ 2021-06-14 12:57 ` Bernd Edlinger 2021-06-14 12:59 ` Rainer Orth 2021-06-14 15:04 ` Tom Tromey 0 siblings, 2 replies; 20+ messages in thread From: Bernd Edlinger @ 2021-06-14 12:57 UTC (permalink / raw) To: Rainer Orth; +Cc: Tom Tromey, gdb-patches Hi Rainer, On 6/14/21 1:41 PM, Rainer Orth wrote: > Hi Bernd, > >> On 6/4/21 3:39 PM, Tom Tromey wrote: >>>>>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes: >>> >>> Bernd> I believe it is right to avoid the SIGPIPE before calling the plugin, >>> Bernd> instead of doing that in gcc-trunk, since we don't know which version >>> Bernd> we will be calling, and all versions I tried have failed like this. >>> >>> That seems fine, but I think it would be better to install the handler >>> just when working with the plugin, and then uninstall it afterward, sort >>> of like what class scoped_ignore_sigttou does. >>> >> >> Okay, done, that works for me. >> >> Is this OK? > > this patch broke the Solaris build: > > /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c:659:3: error: ‘sighandler_t’ does not name a type; did you mean ‘sa_handler’? > 659 | sighandler_t m_osigpipe = NULL; > | ^~~~~~~~~~~~ > | sa_handler > /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c: In constructor ‘scoped_ignore_sigpipe::scoped_ignore_sigpipe()’: > /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c:644:5: error: ‘m_osigpipe’ was not declared in this scope > 644 | m_osigpipe = signal (SIGPIPE, SIG_IGN); > | ^~~~~~~~~~ > /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c:644:18: error: ‘signal’ was not declared in this scope; did you mean ‘sigval’? > 644 | m_osigpipe = signal (SIGPIPE, SIG_IGN); > | ^~~~~~ > | sigval > /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c: In destructor ‘scoped_ignore_sigpipe::~scoped_ignore_sigpipe()’: > /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c:651:22: error: ‘m_osigpipe’ was not declared in this scope > 651 | signal (SIGPIPE, m_osigpipe); > | ^~~~~~~~~~ > /vol/src/gnu/gdb/hg/master/dist/gdb/compile/compile.c:651:5: error: ‘signal’ was not declared in this scope; did you mean ‘sigval’? > 651 | signal (SIGPIPE, m_osigpipe); > | ^~~~~~ > | sigval > > and I have a very hard time deciding where <signal.h> needs to be > included given the enormous range of headers included by compile.c. > Oh, I see, looks like it did only work by chance. Maybe just include it after all other headers: From 6c6d25339460df2ad6eade17cc47a5e472208cc0 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger <bernd.edlinger@hotmail.de> Date: Mon, 14 Jun 2021 14:49:21 +0200 Subject: [PATCH] Include missing header signal.h 2021-06-14 Bernd Edlinger <bernd.edlinger@hotmail.de> * compile/compile.c: Include missing header signal.h. --- gdb/compile/compile.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c index 8247fc4..abbb72a 100644 --- a/gdb/compile/compile.c +++ b/gdb/compile/compile.c @@ -43,6 +43,7 @@ #include "gdbsupport/gdb_optional.h" #include "gdbsupport/gdb_unlinker.h" #include "gdbsupport/pathstuff.h" +#include <signal.h> ^L -- 1.9.1 I guess, I'll commit it as obvious, unless someone objects. Thanks Bernd. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-14 12:57 ` Bernd Edlinger @ 2021-06-14 12:59 ` Rainer Orth 2021-06-14 14:36 ` Bernd Edlinger 2021-06-14 15:07 ` Tom Tromey 2021-06-14 15:04 ` Tom Tromey 1 sibling, 2 replies; 20+ messages in thread From: Rainer Orth @ 2021-06-14 12:59 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Tom Tromey, gdb-patches Hi Bernd, >> and I have a very hard time deciding where <signal.h> needs to be >> included given the enormous range of headers included by compile.c. >> > > Oh, I see, looks like it did only work by chance. > Maybe just include it after all other headers: that's what I've been using myself to fix the build locally. However, as I said I'm completely uncertain if that's the right fix, especially given that gdb/compile files don't include any system headers directly. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-14 12:59 ` Rainer Orth @ 2021-06-14 14:36 ` Bernd Edlinger 2021-06-14 14:39 ` Rainer Orth 2021-06-14 15:07 ` Tom Tromey 1 sibling, 1 reply; 20+ messages in thread From: Bernd Edlinger @ 2021-06-14 14:36 UTC (permalink / raw) To: Rainer Orth; +Cc: Tom Tromey, gdb-patches On 6/14/21 2:59 PM, Rainer Orth wrote: > Hi Bernd, > >>> and I have a very hard time deciding where <signal.h> needs to be >>> included given the enormous range of headers included by compile.c. >>> >> >> Oh, I see, looks like it did only work by chance. >> Maybe just include it after all other headers: > > that's what I've been using myself to fix the build locally. However, > as I said I'm completely uncertain if that's the right fix, especially > given that gdb/compile files don't include any system headers directly. > Are you concerned, that it might be unavailable on some platfroms? I seems to be included where needed in many gdb source files. It's not the plain system header anyways, actually it seems to be a gnulib-enhanced version which tries to improve the portability. Bernd. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-14 14:36 ` Bernd Edlinger @ 2021-06-14 14:39 ` Rainer Orth 0 siblings, 0 replies; 20+ messages in thread From: Rainer Orth @ 2021-06-14 14:39 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Tom Tromey, gdb-patches Hi Bernd, > On 6/14/21 2:59 PM, Rainer Orth wrote: >> Hi Bernd, >> >>>> and I have a very hard time deciding where <signal.h> needs to be >>>> included given the enormous range of headers included by compile.c. >>>> >>> >>> Oh, I see, looks like it did only work by chance. >>> Maybe just include it after all other headers: >> >> that's what I've been using myself to fix the build locally. However, >> as I said I'm completely uncertain if that's the right fix, especially >> given that gdb/compile files don't include any system headers directly. >> > > Are you concerned, that it might be unavailable on some platfroms? > > I seems to be included where needed in many gdb source files. > It's not the plain system header anyways, actually it seems to be > a gnulib-enhanced version which tries to improve the portability. no, I'm concerned that including the system header directly doesn't match gdb conventions. Many are only included indirectly via some gdb-private header, but I don't see a clear pattern there. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-14 12:59 ` Rainer Orth 2021-06-14 14:36 ` Bernd Edlinger @ 2021-06-14 15:07 ` Tom Tromey 2021-06-15 11:10 ` Rainer Orth 1 sibling, 1 reply; 20+ messages in thread From: Tom Tromey @ 2021-06-14 15:07 UTC (permalink / raw) To: Rainer Orth; +Cc: Bernd Edlinger, Tom Tromey, gdb-patches Rainer> that's what I've been using myself to fix the build locally. However, Rainer> as I said I'm completely uncertain if that's the right fix, especially Rainer> given that gdb/compile files don't include any system headers directly. That must just be happenstance. Normally, a universally-available system header can be included anywhere. Sometimes there is a wrapper header, and so in these cases one ought to use the wrapper instead. There's no particular ordering of headers, except that defs.h (gdbsupport and gdbserver have similar, but differently-named headers) must come first. (We looked into a script to reorder headers but I never finished ironing out the bugs.) Tom ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-14 15:07 ` Tom Tromey @ 2021-06-15 11:10 ` Rainer Orth 0 siblings, 0 replies; 20+ messages in thread From: Rainer Orth @ 2021-06-15 11:10 UTC (permalink / raw) To: Tom Tromey; +Cc: Bernd Edlinger, gdb-patches Hi Tom, > Rainer> that's what I've been using myself to fix the build locally. However, > Rainer> as I said I'm completely uncertain if that's the right fix, especially > Rainer> given that gdb/compile files don't include any system headers directly. > > That must just be happenstance. > > Normally, a universally-available system header can be included > anywhere. Sometimes there is a wrapper header, and so in these cases > one ought to use the wrapper instead. There's no particular ordering of > headers, except that defs.h (gdbsupport and gdbserver have similar, but > differently-named headers) must come first. (We looked into a script to > reorder headers but I never finished ironing out the bugs.) ah, thanks for the clarification. I guess I must have been thinking of gcc's system.h which centralizes the inclusion of many system headers, dealing with platform-specific quirks along the way. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-14 12:57 ` Bernd Edlinger 2021-06-14 12:59 ` Rainer Orth @ 2021-06-14 15:04 ` Tom Tromey 1 sibling, 0 replies; 20+ messages in thread From: Tom Tromey @ 2021-06-14 15:04 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Rainer Orth, Tom Tromey, gdb-patches >>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes: Bernd> 2021-06-14 Bernd Edlinger <bernd.edlinger@hotmail.de> Bernd> * compile/compile.c: Include missing header signal.h. Thanks, this is ok. Tom ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-04 13:39 ` Tom Tromey 2021-06-05 11:44 ` Bernd Edlinger @ 2021-06-14 23:35 ` Pedro Alves 2021-06-15 5:14 ` Bernd Edlinger 1 sibling, 1 reply; 20+ messages in thread From: Pedro Alves @ 2021-06-14 23:35 UTC (permalink / raw) To: Tom Tromey, Bernd Edlinger; +Cc: gdb-patches On 2021-06-04 2:39 p.m., Tom Tromey wrote: >>>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes: > > Bernd> I believe it is right to avoid the SIGPIPE before calling the plugin, > Bernd> instead of doing that in gcc-trunk, since we don't know which version > Bernd> we will be calling, and all versions I tried have failed like this. > > That seems fine, but I think it would be better to install the handler > just when working with the plugin, and then uninstall it afterward, sort > of like what class scoped_ignore_sigttou does. One thing I dislike here (and also about scoped_ignore_sigttou), is that the signal disposition is process-wide. If we happen to have multiple threads that need to ignore the signal changing the disposition like that, then they may race. We can fix that by making the ignoring be per-thread instead by using sigprocmask + sigtimedwait. We can also introduce a generic scoped_ignore_signal so that both scoped_ignore_sigttou and scoped_ignore_pipe (and whatever other similar signal we may need to ignore) share the same code. I gave it a quick try. See the 4 patches at the top of the users/palves/scoped_ignore_signal branch. This was something that I've thought before about doing for SIGTTOU to be honest. Seeing it being copied over to SIGPIPE was simply the trigger that made me actually give it a try. WDYT? Pedro Alves ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-14 23:35 ` Pedro Alves @ 2021-06-15 5:14 ` Bernd Edlinger 2021-06-15 11:16 ` Pedro Alves 0 siblings, 1 reply; 20+ messages in thread From: Bernd Edlinger @ 2021-06-15 5:14 UTC (permalink / raw) To: Pedro Alves, Tom Tromey; +Cc: gdb-patches On 6/15/21 1:35 AM, Pedro Alves wrote: > On 2021-06-04 2:39 p.m., Tom Tromey wrote: >>>>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes: >> >> Bernd> I believe it is right to avoid the SIGPIPE before calling the plugin, >> Bernd> instead of doing that in gcc-trunk, since we don't know which version >> Bernd> we will be calling, and all versions I tried have failed like this. >> >> That seems fine, but I think it would be better to install the handler >> just when working with the plugin, and then uninstall it afterward, sort >> of like what class scoped_ignore_sigttou does. > > One thing I dislike here (and also about scoped_ignore_sigttou), is that the > signal disposition is process-wide. > > If we happen to have multiple threads that need to ignore the signal changing the > disposition like that, then they may race. > > We can fix that by making the ignoring be per-thread instead by using > sigprocmask + sigtimedwait. > > We can also introduce a generic scoped_ignore_signal so that both scoped_ignore_sigttou > and scoped_ignore_pipe (and whatever other similar signal we may need to ignore) share > the same code. > > I gave it a quick try. See the 4 patches at the top of the > users/palves/scoped_ignore_signal branch. > > This was something that I've thought before about doing for SIGTTOU to be honest. > Seeing it being copied over to SIGPIPE was simply the trigger that made me > actually give it a try. > > WDYT? > +1 Thanks Bernd. > Pedro Alves > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails 2021-06-15 5:14 ` Bernd Edlinger @ 2021-06-15 11:16 ` Pedro Alves 0 siblings, 0 replies; 20+ messages in thread From: Pedro Alves @ 2021-06-15 11:16 UTC (permalink / raw) To: Bernd Edlinger, Tom Tromey; +Cc: gdb-patches On 2021-06-15 6:14 a.m., Bernd Edlinger wrote: > On 6/15/21 1:35 AM, Pedro Alves wrote: >> One thing I dislike here (and also about scoped_ignore_sigttou), is that the >> signal disposition is process-wide. >> >> If we happen to have multiple threads that need to ignore the signal changing the >> disposition like that, then they may race. >> >> We can fix that by making the ignoring be per-thread instead by using >> sigprocmask + sigtimedwait. >> >> We can also introduce a generic scoped_ignore_signal so that both scoped_ignore_sigttou >> and scoped_ignore_pipe (and whatever other similar signal we may need to ignore) share >> the same code. >> >> I gave it a quick try. See the 4 patches at the top of the >> users/palves/scoped_ignore_signal branch. >> >> This was something that I've thought before about doing for SIGTTOU to be honest. >> Seeing it being copied over to SIGPIPE was simply the trigger that made me >> actually give it a try. >> >> WDYT? >> > > +1 Alright, I posted it here, now with a unit test: https://sourceware.org/pipermail/gdb-patches/2021-June/179982.html Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2021-06-15 11:16 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-02 17:31 [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails Bernd Edlinger 2021-06-02 18:21 ` Tom Tromey 2021-06-02 22:19 ` Christian Biesinger 2021-06-03 5:45 ` Bernd Edlinger 2021-06-04 13:39 ` Tom Tromey 2021-06-05 11:44 ` Bernd Edlinger 2021-06-05 12:04 ` Andrew Burgess 2021-06-05 12:27 ` Bernd Edlinger 2021-06-05 14:04 ` Tom Tromey 2021-06-14 11:41 ` Rainer Orth 2021-06-14 12:57 ` Bernd Edlinger 2021-06-14 12:59 ` Rainer Orth 2021-06-14 14:36 ` Bernd Edlinger 2021-06-14 14:39 ` Rainer Orth 2021-06-14 15:07 ` Tom Tromey 2021-06-15 11:10 ` Rainer Orth 2021-06-14 15:04 ` Tom Tromey 2021-06-14 23:35 ` Pedro Alves 2021-06-15 5:14 ` Bernd Edlinger 2021-06-15 11:16 ` Pedro Alves
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).