Re: [PATCH 3/3] Fix control-c handling on Windows

[Hannes Domani <ssbssa@yahoo.de>]

 Am Freitag, 9. Dezember 2022, 18:21:27 MEZ hat Tom Tromey <tromey@adacore.com> Folgendes geschrieben:

> >> If that's the issue then I can write a patch to change sigint_ours to be
> >> a gdb::optional and check it that way.
>
> Hannes> I should have been more clear about this.
> Hannes> I started with 'new-console on', so sharing_input_terminal() returned false,
> Hannes> and that's why sigint_ours was not set.
>
> Hannes> So yes, gdb::optional would probably fix this.
> Hannes> I just wonder if this is never an issue on Linux, e.g. if you attach,
> Hannes> of does signal() maybe ignore NULL-pointer functions?
>
> Normally SIG_DFL is NULL, but also on Linux I couldn't get this to
> trigger inappropriately.  Maybe my theory about what's going on here is
> incorrect.
>
> Hannes> As far as I could tell, signal() calls SetConsoleCtrlHandler(), probably
> Hannes> similar to how you handled this.
>
> Yeah, that was my guess as well, but really we'd want more details.
> Like, does calling signal reinstall the SetConsoleCtrlHandler?  If so
> then why didn't that work for gdb?  But if not then why did we need to
> call SetConsoleCtrlHandler again to tweak the ordering of callbacks?
>
> Maybe I should go back and try to figure out what else is calling signal
> and/or SetConsoleCtrlHandler.  I somewhat suspect Python but I don't
> really know.

I tried just now to debug signal a bit more, but now I can't reproduce
it calling SetConsoleCtrlHandler any more.

The more I look, the more confused I get again.


> >> I did try 'new-console on' with my (x86-64) build, and that worked fine.
> >> I can try an x86 build and see if that works any better.
>
> Hannes> Again, I wasn't clear enough here.
> Hannes> The difference is not because of i686 and x86_64, but that the x86_64 build
> Hannes> has TUI+python enabled, but my i686 build has not.
>
> Aha, I see, thanks.
>
> Hannes> Some TUI startup code would later call install_sigint_handler() again,
> Hannes> overriding the SIGINT handler again, and everything is fine.
>
> Do you know where this happens?

Yes, it's because of my extra python TUI windows, which call
gdbpy_register_tui_window:

#0  install_sigint_handler (fn=fn@entry=0x13fde3a50 <handle_sigint(int)>) at C:/src/repos/binutils-gdb.git/gdb/mingw-hdep.c:426
#1  0x000000013fde9cdf in install_gdb_sigint_handler (previous=0x27b88a8) at C:/src/repos/binutils-gdb.git/gdb/extension.c:679
#2  set_active_ext_lang (now_active=now_active@entry=0x1407aee00 <extension_language_python>) at C:/src/repos/binutils-gdb.git/gdb/extension.c:736
#3  0x000000013ff4503e in gdbpy_enter::gdbpy_enter (this=this@entry=0x24c6c0, gdbarch=gdbarch@entry=0x0, language=language@entry=0x0) at C:/src/repos/binutils-gdb.git/gdb/python/python.c:212
#4  0x000000013ff35ac0 in gdbpy_tui_window_maker::gdbpy_tui_window_maker (other=..., this=0x24c6b8) at C:/src/repos/binutils-gdb.git/gdb/python/py-tui.c:301
#5  gdbpy_register_tui_window (self=<optimized out>, args=<optimized out>, kw=<optimized out>) at C:/src/repos/binutils-gdb.git/gdb/python/py-tui.c:380
...


And also there is this call later on:

#0  install_sigint_handler (fn=fn@entry=0x13fde3a50 <handle_sigint(int)>) at C:/src/repos/binutils-gdb.git/gdb/mingw-hdep.c:426
#1  0x000000013fde9cdf in install_gdb_sigint_handler (previous=0x28ade48) at C:/src/repos/binutils-gdb.git/gdb/extension.c:679
#2  set_active_ext_lang (now_active=now_active@entry=0x1407aee00 <extension_language_python>) at C:/src/repos/binutils-gdb.git/gdb/extension.c:736
#3  0x000000013ff4503e in gdbpy_enter::gdbpy_enter (this=0x24f7b0, gdbarch=0x0, language=0x0) at C:/src/repos/binutils-gdb.git/gdb/python/python.c:212
#4  0x000000013ff4522a in gdbpy_before_prompt_hook (extlang=<optimized out>, current_gdb_prompt=0x140be96b0 <top_prompt+16> "(gdb) ") at C:/src/repos/binutils-gdb.git/gdb/python/python.c:1114
#5  0x000000013fde8fc3 in ext_lang_before_prompt (current_gdb_prompt=0x140be96b0 <top_prompt+16> "(gdb) ") at C:/src/repos/binutils-gdb.git/gdb/extension.c:963
#6  0x000000013fde4431 in std::function<void (char const*)>::operator()(char const*) const (__args#0=0x140be96b0 <top_prompt+16> "(gdb) ", this=0x4dee28) at c:/msys64/mingw64/x86_64-w64-mingw32/include/c++/11.2.0/bits/std_function.h:560
#7  gdb::observers::observable<char const*>::notify (args#0=0x140be96b0 <top_prompt+16> "(gdb) ", this=<optimized out>) at c:/src/repos/binutils-gdb.git/gdbsupport/observable.h:166
#8  top_level_prompt () at C:/src/repos/binutils-gdb.git/gdb/event-top.c:461
#9  display_gdb_prompt (new_prompt=<optimized out>) at C:/src/repos/binutils-gdb.git/gdb/event-top.c:428
#10 0x000000013fea4115 in captured_command_loop () at C:/src/repos/binutils-gdb.git/gdb/main.c:472
#11 0x000000013fea5de5 in captured_main (data=0x24fa70) at C:/src/repos/binutils-gdb.git/gdb/main.c:1341
#12 gdb_main (args=args@entry=0x24fad0) at C:/src/repos/binutils-gdb.git/gdb/main.c:1356
#13 0x0000000140692e27 in main (argc=2, argv=0x5c4cc0) at C:/src/repos/binutils-gdb.git/gdb/gdb.c:32


> Anyway I am wondering if we can have gdb_rl_deprep_term_function call
> rl_clear_signals and then reinstall the gdb signal handlers.  This idea
> makes me wonder if we even need SetConsoleCtrlHandler at all -- maybe gdb
> could just use signal after all.

Good question, maybe it doesn't handle C-break as well?


Hannes