From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Buettner To: Orjan Friberg , gdb-patches@sourceware.cygnus.com Subject: Re: Patch for DejaGnu test case "pointers" Date: Thu, 07 Sep 2000 12:56:00 -0000 Message-id: <1000907195608.ZM26383@ocotillo.lan> References: <39B7A02B.EEB3F889@axis.com> X-SW-Source: 2000-09/msg00065.html On Sep 7, 4:03pm, Orjan Friberg wrote: > [...] This is my first patch submission > (more patches are on the way), so I'd appreciate feedback if any changes > are needed. In addition to providing ChangeLog entries (as noted by Hans-Peter Nilsson), you should keep the diff headers intact. I.e, you should preserve the lines that appeared in your diff output immediately before the following lines: > *************** int more_code() > *** 194,200 **** Kevin >From jtc@redback.com Thu Sep 07 13:15:00 2000 From: jtc@redback.com (J.T. Conklin) To: gdb-patches@sourceware.cygnus.com Subject: [PATCH]: move i386nbsd_ Date: Thu, 07 Sep 2000 13:15:00 -0000 Message-id: <5m1yywq7rk.fsf@jtc.redback.com> X-SW-Source: 2000-09/msg00066.html Content-length: 1743 Someone reported a problem building a NetBSD/i386 cross-debugger because i386nbsd_use_struct_convention() was in a native-only file instead of a target file. I applied the enclosed patch to fix this. --jtc 2000-09-07 J.T. Conklin * config/i386/nbsd.mt (TDEPFILES): Add i386nbsd-tdep.o. * i386nbsd-nat.c (i386nbsd_use_struct_convention): Moved from here. * i386nbsd-tdep.c (i386nbsd_use_struct_convention): To here. * i386nbsd-tdep.c: New file. Index: i386nbsd-nat.c =================================================================== RCS file: /cvs/src/src/gdb/i386nbsd-nat.c,v retrieving revision 1.4 diff -c -r1.4 i386nbsd-nat.c *** i386nbsd-nat.c 2000/07/30 01:48:25 1.4 --- i386nbsd-nat.c 2000/09/07 18:22:15 *************** *** 147,161 **** (PTRACE_ARG3_TYPE) &inferior_fpregisters, 0); } - int - i386nbsd_use_struct_convention (int gcc_p, struct type *type) - { - return !(TYPE_LENGTH (type) == 1 - || TYPE_LENGTH (type) == 2 - || TYPE_LENGTH (type) == 4 - || TYPE_LENGTH (type) == 8); - } - struct md_core { struct reg intreg; --- 147,152 ---- Index: config/i386/nbsd.mt =================================================================== RCS file: /cvs/src/src/gdb/config/i386/nbsd.mt,v retrieving revision 1.3 diff -c -r1.3 nbsd.mt *** nbsd.mt 2000/05/24 04:16:27 1.3 --- nbsd.mt 2000/09/07 18:22:24 *************** *** 1,5 **** # Target: Intel 386 running NetBSD ! TDEPFILES= i386-tdep.o i387-tdep.o TM_FILE= tm-nbsd.h GDBSERVER_DEPFILES= low-nbsd.o --- 1,5 ---- # Target: Intel 386 running NetBSD ! TDEPFILES= i386-tdep.o i387-tdep.o i386nbsd-tdep.o TM_FILE= tm-nbsd.h GDBSERVER_DEPFILES= low-nbsd.o -- J.T. Conklin RedBack Networks >From kettenis@wins.uva.nl Thu Sep 07 13:19:00 2000 From: Mark Kettenis To: gdb-patches@sourceware.cygnus.com Subject: [PATCH] lin-lwp.c fix Date: Thu, 07 Sep 2000 13:19:00 -0000 Message-id: <200009072019.e87KJMm00944@delius.kettenis.local> X-SW-Source: 2000-09/msg00067.html Content-length: 6408 The attached patch (checked in) fixes a fairly critical bug in the new Linux threads code (debugging threaded code might cause GDB to terminate with one of the real-time signals). I reorganized the signal handling stuff a bit so that now the SIGCHLD signal is no longer automatically blocked in the inferior. This makes the failures in gdb.base/sigall.exp disappear :-). Mark 2000-09-06 Mark Kettenis * lin-lwp.c (normal_mask, blocked_mask): New variables. (lin_lwp_wait): Block SIGCHLD here if it isn't already blocked. (lin_lwp_mourn_inferior): Restore the origional signal mask, and reset the mask of blocked signals. (_initialize_lin_lwp): Don't block SIGCHLD here, but do initialize suspend_mask and blocked_mask. This makes us pass gdb.base/sigall.exp for Linux/x86 now. (lin_thread_get_thread_signals): Treat the LinuxThreads "cancel" signal similarly to SIGCHLD in the generic code. Avoids GDB being terminated by a Real-time signal. Index: lin-lwp.c =================================================================== RCS file: /cvs/src/src/gdb/lin-lwp.c,v retrieving revision 1.1 diff -u -p -r1.1 lin-lwp.c --- lin-lwp.c 2000/09/03 18:41:28 1.1 +++ lin-lwp.c 2000/09/07 20:05:28 @@ -55,9 +55,11 @@ extern const char *strsignal (int sig); code: - In general one should specify the __WCLONE flag to waitpid in - order to make it report events for any of the cloned processes. - However, if a cloned process has exited the exit status is only - reported if the __WCLONE flag is absent. + order to make it report events for any of the cloned processes + (and leave it out for the initial process). However, if a cloned + process has exited the exit status is only reported if the + __WCLONE flag is absent. Linux 2.4 has a __WALL flag, but we + cannot use it since GDB must work on older systems too. - When a traced, cloned process exits and is waited for by the debugger, the kernel reassigns it to the origional parent and @@ -126,9 +128,26 @@ static struct target_ops lin_lwp_ops; /* The standard child operations. */ extern struct target_ops child_ops; +/* Since we cannot wait (in lin_lwp_wait) for the initial process and + any cloned processes with a single call to waitpid, we have to use + use the WNOHANG flag and call waitpid in a loop. To optimize + things a bit we use `sigsuspend' to wake us up when a process has + something to report (it will send us a SIGCHLD if it has). To make + this work we have to juggle with the signal mask. We save the + origional signal mask such that we can restore it before creating a + new process in order to avoid blocking certain signals in the + inferior. We then block SIGCHLD during the waitpid/sigsuspend + loop. */ + +/* Origional signal mask. */ +static sigset_t normal_mask; + /* Signal mask for use with sigsuspend in lin_lwp_wait, initialized in _initialize_lin_lwp. */ static sigset_t suspend_mask; + +/* Signals to block to make that sigsuspend work. */ +static sigset_t blocked_mask; /* Prototypes for local functions. */ @@ -479,7 +498,7 @@ stop_wait_callback (struct lwp_info *lp, is_cloned (lp->pid) ? __WCLONE : 0); if (pid == -1 && errno == ECHILD) /* OK, the proccess has disappeared. We'll catch the actual - exit event in lin_lwp_wait. */ + exit event in lin_lwp_wait. */ return 0; gdb_assert (pid == GET_LWP (lp->pid)); @@ -575,6 +594,13 @@ lin_lwp_wait (int pid, struct target_wai int options = 0; int status = 0; + /* Make sure SIGCHLD is blocked. */ + if (! sigismember (&blocked_mask, SIGCHLD)) + { + sigaddset (&blocked_mask, SIGCHLD); + sigprocmask (SIG_BLOCK, &blocked_mask, NULL); + } + retry: /* First check if there is a LWP with a wait status pending. */ @@ -659,7 +685,8 @@ lin_lwp_wait (int pid, struct target_wai lp = add_lwp (BUILD_LWP (lwpid, inferior_pid)); if (threaded) { - gdb_assert (WIFSTOPPED (status) && WSTOPSIG (status) == SIGSTOP); + gdb_assert (WIFSTOPPED (status) + && WSTOPSIG (status) == SIGSTOP); lp->signalled = 1; if (! in_thread_list (inferior_pid)) @@ -863,6 +890,10 @@ lin_lwp_mourn_inferior (void) trap_pid = 0; + /* Restore the origional signal mask. */ + sigprocmask (SIG_SETMASK, &normal_mask, NULL); + sigemptyset (&blocked_mask); + #if 0 target_beneath = find_target_beneath (&lin_lwp_ops); #else @@ -978,7 +1009,6 @@ void _initialize_lin_lwp (void) { struct sigaction action; - sigset_t mask; extern void thread_db_init (struct target_ops *); @@ -986,18 +1016,19 @@ _initialize_lin_lwp (void) add_target (&lin_lwp_ops); thread_db_init (&lin_lwp_ops); + /* Save the origional signal mask. */ + sigprocmask (SIG_SETMASK, NULL, &normal_mask); + action.sa_handler = sigchld_handler; sigemptyset (&action.sa_mask); action.sa_flags = 0; sigaction (SIGCHLD, &action, NULL); - /* We block SIGCHLD throughout this code ... */ - sigemptyset (&mask); - sigaddset (&mask, SIGCHLD); - sigprocmask (SIG_BLOCK, &mask, &suspend_mask); - - /* ... except during a sigsuspend. */ + /* Make sure we don't block SIGCHLD during a sigsuspend. */ + sigprocmask (SIG_SETMASK, NULL, &suspend_mask); sigdelset (&suspend_mask, SIGCHLD); + + sigemptyset (&blocked_mask); } @@ -1030,8 +1061,8 @@ get_signo (const char *name) void lin_thread_get_thread_signals (sigset_t *set) { - int restart; - int cancel; + struct sigaction action; + int restart, cancel; sigemptyset (set); @@ -1045,4 +1076,21 @@ lin_thread_get_thread_signals (sigset_t sigaddset (set, restart); sigaddset (set, cancel); + + /* The LinuxThreads library makes terminating threads send a special + "cancel" signal instead of SIGCHLD. Make sure we catch those (to + prevent them from terminating GDB itself, which is likely to be + their default action) and treat them the same way as SIGCHLD. */ + + action.sa_handler = sigchld_handler; + sigemptyset (&action.sa_mask); + action.sa_flags = 0; + sigaction (cancel, &action, NULL); + + /* We block the "cancel" signal throughout this code ... */ + sigaddset (&blocked_mask, cancel); + sigprocmask (SIG_BLOCK, &blocked_mask, NULL); + + /* ... except during a sigsuspend. */ + sigdelset (&suspend_mask, cancel); } >From ac131313@cygnus.com Thu Sep 07 23:13:00 2000 From: Andrew Cagney To: GDB Patches Subject: [patch] Re-generate aclocal.m4 Date: Thu, 07 Sep 2000 23:13:00 -0000 Message-id: <39B882E9.1F2444B1@cygnus.com> X-SW-Source: 2000-09/msg00068.html Content-length: 285 FYI, I've re-generated aclocal.m4 and along with everything else. Recent ../bfd changes didn't get correctly propogated into GDB. Andrew Thu Sep 7 21:59:23 2000 Andrew Cagney * aclocal.m4: Regenerate. * config.in, configure: Regenerate. >From ac131313@cygnus.com Thu Sep 07 23:43:00 2000 From: Andrew Cagney To: Kevin Buettner , Daniel Berlin Cc: gdb-patches@sourceware.cygnus.com Subject: Re: [PATCH] C++ patches Date: Thu, 07 Sep 2000 23:43:00 -0000 Message-id: <39B88998.3DE3966@cygnus.com> References: <1000907045542.ZM25066@ocotillo.lan> X-SW-Source: 2000-09/msg00069.html Content-length: 2039 Kevin Buettner wrote: > > On Sep 6, 9:35pm, Daniel Berlin wrote: > > > I've rerun indent on the files this affects, because the formatting has > > gotten way out of whack with the GNU standards. > > I hope nobody minds, if somebody does, i'll undo the whitespace changes in > > my patch > > (diff -c3pwBb, remove the unbreaking of lines it did). > > I think the generally agreed upon practice is to do two separate > commits; the first being the actual changes you made and the second > being the reformatting. This way it's easier to review the first > patch. Either order. The main thing is to keep indentation and real codeing changes separate. A maintainer shouldn't be presented with a patch that contains a combination of both. Reviewing it is impossible :-(. Don't forget to clean the diff up a little (strip out the ChangeLog etc) next time :-) > > Please note we now have one memory leak in lookup_symbol. The demangler > > gives us a buffer we never free. I'd need to free it at every exit point > > from the function, of which we have 12 right now, all of which end almost > > the exact same way (most return fixup_symbol_section(sym), a few just > > return sym. Is this incorrect?). > > > > I'm thinking that i'll change it so we just have a label at the end, and > > 12 gotos, rather than 12 exit points, and do the cleanup of the > > buffer there. > > It would make it about 50x easier for someone who needed to do something > > that requires cleanup when we exit. > > I think gotos in this case are preferable. Gotos are bad. M'kay? We've so far spent three years trying to eliminate all the goto's in WFI, adding more will just make me depressed :-) Why not push the ALL_SYMTABS() loop into a sub function parameterized by ``name'' - I'm assuming that it is the loop that has the N different exit points? That way you get your goto without actually having a goto... Hmm, I suspect that ``name'' will need a cleanup. Throw an error and, regardless of the gotos, it will leak memory. Keep it up. Andrew >From dberlin@redhat.com Fri Sep 08 06:36:00 2000 From: Daniel Berlin To: Andrew Cagney Cc: Kevin Buettner , Daniel Berlin , gdb-patches@sourceware.cygnus.com Subject: Re: [PATCH] C++ patches Date: Fri, 08 Sep 2000 06:36:00 -0000 Message-id: References: <1000907045542.ZM25066@ocotillo.lan> <39B88998.3DE3966@cygnus.com> X-SW-Source: 2000-09/msg00070.html Content-length: 2610 Andrew Cagney writes: > Kevin Buettner wrote: > > > > On Sep 6, 9:35pm, Daniel Berlin wrote: > > > > > I've rerun indent on the files this affects, because the formatting has > > > gotten way out of whack with the GNU standards. > > > I hope nobody minds, if somebody does, i'll undo the whitespace changes in > > > my patch > > > (diff -c3pwBb, remove the unbreaking of lines it did). > > > > I think the generally agreed upon practice is to do two separate > > commits; the first being the actual changes you made and the second > > being the reformatting. This way it's easier to review the first > > patch. > > Either order. The main thing is to keep indentation and real codeing > changes separate. A maintainer shouldn't be presented with a patch that > contains a combination of both. Reviewing it is impossible :-(. Yup. Dunno what i was thinking. Brain slipped. > > Don't forget to clean the diff up a little (strip out the ChangeLog etc) > next time :-) > > > > Please note we now have one memory leak in lookup_symbol. The demangler > > > gives us a buffer we never free. I'd need to free it at every exit point > > > from the function, of which we have 12 right now, all of which end almost > > > the exact same way (most return fixup_symbol_section(sym), a few just > > > return sym. Is this incorrect?). > > > > > > I'm thinking that i'll change it so we just have a label at the end, and > > > 12 gotos, rather than 12 exit points, and do the cleanup of the > > > buffer there. > > > It would make it about 50x easier for someone who needed to do something > > > that requires cleanup when we exit. > > > > I think gotos in this case are preferable. > > Gotos are bad. M'kay? We've so far spent three years trying to > eliminate all the goto's in WFI, adding more will just make me depressed > :-) > > Why not push the ALL_SYMTABS() loop into a sub function parameterized by > ``name'' - I'm assuming that it is the loop that has the N different > exit points? That way you get your goto without actually having a > goto... Hmm, I suspect that ``name'' will need a cleanup. Throw an > error and, regardless of the gotos, it will leak memory. I'm not staring at the code right now, but IIRC, part of the problem was that they aren't all inside one loop. I have IMHO a better solution, like the above. I'll split lookup_symbol into lookup_symbol and lookup_symbol_aux, do the case sensitivity/demangling in lookup_symbol, then call lookup_symbol_aux, do the cleanup, and retunr whatever aux gave us. Is this acceptable? > > Keep it up. > > Andrew