[PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Bernd Edlinger]

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

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Tom Tromey]

>>>>> "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

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Christian Biesinger]

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

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Bernd Edlinger]

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.

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Tom Tromey]

>>>>> "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

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Bernd Edlinger]

[-- 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

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Andrew Burgess]

* 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
> 

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Bernd Edlinger]

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.

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Tom Tromey]

>>>>> "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

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Rainer Orth]

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

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Bernd Edlinger]

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.

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Rainer Orth]

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

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Bernd Edlinger]

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.

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Rainer Orth]

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

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Tom Tromey]

>>>>> "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

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Tom Tromey]

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

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Pedro Alves]

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

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Bernd Edlinger]

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
> 

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Rainer Orth]

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

Re: [PATCH] Fix gdb crash due to SIGPIPE when the compile command fails

[Pedro Alves]

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