[RFC] breakpoint.c: Fix nasty problem with msvcrt DLL on Windows

["Pierre Muller" <pierre dot muller at ics-cnrs dot unistra dot fr>]

  I ran testsuite on Cygwin and found out that
all pascal tests were failing. Further inverstigation showed
that the same problem happens more than 300 times,
so that it is not pascal specific.

  It seems to be related to the fact that the Google toolbar 
is installed on that machine: 

  On startup of executables, a Google specific DLL gets loaded,
which itself load msvcrt.dll, a DLL that contains a 'longjmp' function.
This 'longjmp' functions triggers the creation of a bp_longjmp_master
breakpoint type.
  But the Google toolbar also unloads itself during initialization
and this triggers unloading of msvcrt.dll.
  Here a GDB bug appears: the unloading of msvcrt DLL
does not trigger the removal of the corresponding longjmp breakpoint,
which then leads to the error when trying to step
that executable.
  I wrote a little test C code that allows to check
for the bug on any Windows system with msvcrt.dll installed.
The bug is probably not windows specific, and would probably 
show on each dynamically loaded library that contains a 'longjmp' function
and gets unloaded.

  Does anyone know if this is a known problem?
I was able to 'fix' it by setting  loc->shlib_disabled to 1
for the longjmp_master breakpoint.
  But maybe we should completely remove the bp_longjmp_master
breakpoint as it is an internal breakpoint, but I
am not familiar enough with all the subtleties of breakpoint code
to be sure that this is not dangerous.
  I am also not sure about the code in set_longjmp_breakpoint 
is it safe to assume that b->loc is always non NULL
for a bp_longjmp_master, or should that be modified?


  Comments most welcome,

Pierre Muller


>>>>>>>>>>>>>>>>>>>>Start of transcript
(gdb) start
Temporary breakpoint 1 at 0x401056: file
../../../src/gdb/testsuite/gdb.pascal/f
loats.pas, line 25.
Starting program:
/usr/local/src/gdbcvs/build-norm/gdb/testsuite/gdb.pascal/floa
ts.exe
gdb: windows_init_thread_list
gdb: kernel event for pid=1544 tid=d7c code=CREATE_PROCESS_DEBUG_EVENT)
[New Thread 1544.0xd7c]
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=LOAD_DLL_DEBUG_EVENT)
gdb: Loading dll "/cygdrive/c/WINDOWS/system32/ntdll.dll" at 0x7c910000.
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=LOAD_DLL_DEBUG_EVENT)
gdb: Loading dll "/cygdrive/c/WINDOWS/system32/kernel32.dll" at 0x7c800000.
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=LOAD_DLL_DEBUG_EVENT)
gdb: Loading dll "/cygdrive/c/cygwin/bin/cygwin1.dll" at 0x61000000.
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=LOAD_DLL_DEBUG_EVENT)
gdb: Loading dll "/cygdrive/c/WINDOWS/system32/ADVAPI32.DLL" at 0x77da0000.
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=LOAD_DLL_DEBUG_EVENT)
gdb: Loading dll "/cygdrive/c/WINDOWS/system32/RPCRT4.dll" at 0x77e50000.
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=LOAD_DLL_DEBUG_EVENT)
gdb: Loading dll "/cygdrive/c/WINDOWS/system32/Secur32.dll" at 0x77fc0000.
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=EXCEPTION_DEBUG_EVENT)
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=350 code=CREATE_THREAD_DEBUG_EVENT)
[New Thread 1544.0x350]
ContinueDebugEvent (cpid=1544, ctid=350, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=OUTPUT_DEBUG_STRING_EVENT)
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=OUTPUT_DEBUG_STRING_EVENT)
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=LOAD_DLL_DEBUG_EVENT)
gdb: Loading dll "/cygdrive/c/WINDOWS/system32/user32.dll" at 0x7e390000.
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=LOAD_DLL_DEBUG_EVENT)
gdb: Loading dll "/cygdrive/c/WINDOWS/system32/GDI32.dll" at 0x77ef0000.
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=LOAD_DLL_DEBUG_EVENT)
gdb: Loading dll "/cygdrive/c/WINDOWS/system32/IMM32.DLL" at 0x76320000.
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=LOAD_DLL_DEBUG_EVENT)
gdb: Loading dll "/cygdrive/c/PROGRA~1/Google/GOOGLE~2/GOEC62~1.DLL" at
0x480000
00.
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=LOAD_DLL_DEBUG_EVENT)
gdb: Loading dll "/cygdrive/c/WINDOWS/system32/WS2_32.dll" at 0x719f0000.
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=LOAD_DLL_DEBUG_EVENT)
gdb: Loading dll "/cygdrive/c/WINDOWS/system32/msvcrt.dll" at 0x77be0000.
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=LOAD_DLL_DEBUG_EVENT)
gdb: Loading dll "/cygdrive/c/WINDOWS/system32/WS2HELP.dll" at 0x719e0000.
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=UNLOAD_DLL_DEBUG_EVENT)
gdb: Unloading dll "/cygdrive/c/PROGRA~1/Google/GOOGLE~2/GOEC62~1.DLL".
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=UNLOAD_DLL_DEBUG_EVENT)
gdb: Unloading dll "/cygdrive/c/WINDOWS/system32/WS2_32.dll".
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=UNLOAD_DLL_DEBUG_EVENT)
gdb: Unloading dll "/cygdrive/c/WINDOWS/system32/WS2HELP.dll".
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=UNLOAD_DLL_DEBUG_EVENT)
gdb: Unloading dll "/cygdrive/c/WINDOWS/system32/msvcrt.dll".
ContinueDebugEvent (cpid=1544, ctid=d7c, DBG_CONTINUE);
gdb: kernel event for pid=1544 tid=d7c code=EXCEPTION_DEBUG_EVENT)

Temporary breakpoint 1, _p__M0_main_program ()
    at ../../../src/gdb/testsuite/gdb.pascal/floats.pas:25
25      begin
(gdb) maint inf b
Num     Type           Disp Enb Address    What
-10     longjmp master keep n   0x61093868 <longjmp> inf 1
-11     longjmp master keep n   0x77c06d74  inf 1

(gdb) n
Warning:
Cannot insert breakpoint -11.
Error accessing memory address 0x77c06d74: Input/Output error.

(gdb)

>>>>>>>>>>>>>>>>>>>>End of transcript

This -11 longjmp breakpoint should have been removed when msvcrtl.dll was
unloaded, but this does not seem to work correctly.

ChangeLog entry:
2010-08-05  Pierre Muller  <muller@ics.u-strasbg.fr>

        * breakpoint.c (set_longjmp_breakpoint): Only insert breakpoints
        if the dynamic library was not unloaded.
        (disable_breakpoints_in_unloaded_shlib): Disable also 'longjmp'
        breakpoints.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.506
diff -u -p -r1.506 breakpoint.c
--- breakpoint.c        3 Aug 2010 22:35:41 -0000       1.506
+++ breakpoint.c        5 Aug 2010 10:03:14 -0000
@@ -5573,10 +5573,12 @@ set_longjmp_breakpoint (int thread)
   /* To avoid having to rescan all objfile symbols at every step,
      we maintain a list of continually-inserted but always disabled
      longjmp "master" breakpoints.  Here, we simply create momentary
-     clones of those and enable them for the requested thread.  */
+     clones of those and enable them for the requested thread.
+     Do not try to insert 'longjmp' breakpoints of unloaded libraries.  */
   ALL_BREAKPOINTS_SAFE (b, temp)
     if (b->pspace == current_program_space
-       && b->type == bp_longjmp_master)
+       && b->type == bp_longjmp_master
+        && b->loc && !b->loc->shlib_disabled)
       {
        struct breakpoint *clone = clone_momentary_breakpoint (b);

@@ -5793,6 +5795,8 @@ disable_breakpoints_in_unloaded_shlib (s
        && !loc->shlib_disabled
        && (b->type == bp_breakpoint
            || b->type == bp_jit_event
+           || b->type == bp_longjmp
+           || b->type == bp_longjmp_master
            || b->type == bp_hardware_breakpoint)
        && solib_contains_address_p (solib, loc->address))
       {

PS: Here is a simple test code to reproduce the problem:
1> #include <windows.h>
2> 
3> int
4> main ()
5> 
6> {
7>   HMODULE h = LoadLibraryA ("msvcrt.dll");
8>   void * func = GetProcAddress (h, "longjmp");
9>   FreeLibrary (h);
10>  printf ("Address of longjmp is 0x%x\n", (long) func);
11> }

  You can compile this on Windows and use 
(gdb) start
(gdb) set debugevents on
(gdb) next
until you reach line 10,
from there 'next' will fail with the 'Cannot insert breakpoint' error
on current head GDB, 
'cont' is still working, because in that case the longjmp internal
breakpoints
are not inserted. 
  With my patch you will just get a warning about 'Temporarily disabling
breakpoints for unloaded
share library ...msvcrt.dll". This warning should probably be removed
also for an internal breakpoint.