On Sat, 17 May 2025 15:00:53 +0100
Jon Turney wrote:
> This fixes constantly replaying the exception if we have a segfault
> while a debugger is already attached, e.g. stracing a segv, see:
>
> https://cygwin.com/pipermail/cygwin/2025-May/258144.html
>
> (I'm tempted to remove the 'debugging' static from exception::handle()
> and everything associated with it, since replaying the exception the
> next half a million times it's hit seems really weird)
>
> (This would seem to make try_to_debug() less useful, as it then does
> nothing if you're just run under gdb, but it's what the code used to
> do...)
I don't understand what the sentences in ()s mean.
> Fixes: 91457377d6c9 ("Cygwin: Make 'ulimit -c' control writing a coredump")
Please add "Signed-off-by:". I also think it is better to add
"Reported-by:".
> ---
> winsup/cygwin/exceptions.cc | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
> index 9763a1b04..3a0315fd0 100644
> --- a/winsup/cygwin/exceptions.cc
> +++ b/winsup/cygwin/exceptions.cc
> @@ -591,13 +591,16 @@ int exec_prepared_command (PWCHAR command)
> extern "C" int
> try_to_debug ()
> {
> + if (!debugger_command)
> + return 0;
> +
> /* If already being debugged, break into the debugger (Note that this
> function
> can be called from places other than an exception) */
> if (being_debugged ())
> {
> extern void break_here ();
> break_here ();
> - return 1;
> + return 0;
> }
>
> /* Otherwise, invoke the JIT debugger, if set */
> @@ -812,6 +815,8 @@ exception::handle (EXCEPTION_RECORD *e, exception_list
> *frame, CONTEXT *in,
> else if (try_to_debug ())
> {
> debugging = 1;
> + /* If a JIT debugger just attached, replay the exception for the
> benefit
> + of that */
> return ExceptionContinueExecution;
> }
>
> --
> 2.45.1
Otherwise, LGTM. Please push.
--
Takashi Yano <[email protected]>
