From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eli Zaretskii To: jimb@cygnus.com Cc: hjl@lucon.org, gdb@sourceware.cygnus.com Subject: Re: Hardware watchpoints Date: Wed, 27 Oct 1999 13:07:00 -0000 Message-id: <199910272007.QAA04553@mescaline.gnu.org> References: <19991019235249.917DC1B494@ocean.lucon.org> <199910201401.KAA28719@mescaline.gnu.org> <199910221200.IAA24556@mescaline.gnu.org> <199910231048.GAA31392@mescaline.gnu.org> X-SW-Source: 1999-q4/msg00174.html As promised, diffs for all the changes that avoid inserting/removing watchpoints for lazy values are attached below. Now for the next problem: hardware watchpoints still don't work, even after these changes, for struct members that are bitfields. As far as I could see, the reason for this is that value_primitive_field needs to fetch the value of the field in order to unpack it into an int, and that causes the lazy flag of the struct itself to be reset. It seems that relying on the lazy flag is too fragile: any code that needs to fetch the value of the struct, for some purpose, will break the ability to watch members of that struct. Perhaps we need a special flag for this, which will be set for structs and arrays only if they are not the watched expression itself. * breakpoint.c (insert_breakpoints): Fetch the value of the expression we need to watch, to make sure its LAZY flag is off. Don't insert watchpoints for values whose LAZY flag is set. (remove_breakpoint): Don't remove a watchpoint if the value's LAZY flag is set (since we didn't insert them in the first place). (bpstat_stop_status): Don't check values whose LAZY flag is set, since we don't watch them. *** gdb/breakpoi.c~4 Sun Aug 22 19:03:14 1999 --- gdb/breakpoi.c Wed Oct 27 19:37:04 1999 *************** insert_breakpoints () *** 779,784 **** --- 779,786 ---- /* Evaluate the expression and cut the chain of values produced off from the value chain. */ v = evaluate_expression (b->exp); + if (VALUE_LAZY (v)) + value_fetch_lazy (v); value_release_to_mark (mark); b->val_chain = v; *************** insert_breakpoints () *** 788,794 **** for ( ; v; v=v->next) { /* If it's a memory location, then we must watch it. */ ! if (v->lval == lval_memory) { int addr, len, type; --- 790,796 ---- for ( ; v; v=v->next) { /* If it's a memory location, then we must watch it. */ ! if (v->lval == lval_memory && !v->lazy) { int addr, len, type; *************** remove_breakpoint (b, is) *** 1127,1133 **** { /* For each memory reference remove the watchpoint at that address. */ ! if (v->lval == lval_memory) { int addr, len, type; --- 1129,1135 ---- { /* For each memory reference remove the watchpoint at that address. */ ! if (v->lval == lval_memory && !v->lazy) { int addr, len, type; *************** bpstat_stop_status (pc, not_a_breakpoint *** 2199,2205 **** } for (v = b->val_chain; v; v = v->next) { ! if (v->lval == lval_memory) { CORE_ADDR vaddr; --- 2201,2207 ---- } for (v = b->val_chain; v; v = v->next) { ! if (v->lval == lval_memory && !v->lazy) { CORE_ADDR vaddr; *************** can_use_hardware_watchpoint (v) *** 4486,4492 **** hardware watchpoint for the constant expression. */ for ( ; v; v = v->next) { ! if (v->lval == lval_memory) { int vaddr = VALUE_ADDRESS (v) + VALUE_OFFSET (v); int len = TYPE_LENGTH (VALUE_TYPE (v)); --- 4488,4494 ---- hardware watchpoint for the constant expression. */ for ( ; v; v = v->next) { ! if (v->lval == lval_memory && !v->lazy) { int vaddr = VALUE_ADDRESS (v) + VALUE_OFFSET (v); int len = TYPE_LENGTH (VALUE_TYPE (v)); >From hjl@valinux.com Wed Oct 27 13:43:00 1999 From: hjl@valinux.com (H.J. Lu) To: gdb-patches@sourceware.cygnus.com Cc: slouken@devolution.com, gdb@sourceware.cygnus.com (GDB) Subject: A patch for shared library support Date: Wed, 27 Oct 1999 13:43:00 -0000 Message-id: <19991027204300.7816E3FC1@valinux.com> X-SW-Source: 1999-q4/msg00175.html Content-length: 3401 Hi, I finally get annoyed enough to port Sam's patch to gdb 4.18 in CVS. When you set a break point in shared library, gdb will crash when you restart the program. This patch seems to work for me. -- H.J. Lu (hjl@gnu.org) -- Wed Mar 17 19:49:22 1999 Sam Lantinga (slouken@devolution.com) Added function check_solib_consistency() to reload list of shared objects when they are added or deleted. This fixed crashing when the program being debugged unloaded a dynamic library and added a new library afterwards. * solib.h (CHECK_SOLIB_CONSISTENCY): New. (check_solib_consistency): New prototype. * solib.c (check_solib_consistency): Defined. * infrun.c (handle_inferior_event): Before calling SOLIB_ADD (), call CHECK_SOLIB_CONSISTENCY () if defined. Index: solib.h =================================================================== RCS file: /work/cvs/gnu/gdb/gdb/solib.h,v retrieving revision 1.1.1.1 diff -u -p -r1.1.1.1 solib.h --- solib.h 1999/09/09 00:38:38 1.1.1.1 +++ solib.h 1999/10/27 20:35:54 @@ -186,6 +186,11 @@ solib_create_inferior_hook PARAMS ((void extern char * solib_address PARAMS ((CORE_ADDR)); /* solib.c */ +/* Check shared library consistency */ + +#define CHECK_SOLIB_CONSISTENCY() check_solib_consistency() +extern void check_solib_consistency PARAMS ((void)); + /* If ADDR lies in a shared library, return its name. */ #define PC_SOLIB(addr) solib_address (addr) Index: solib.c =================================================================== RCS file: /work/cvs/gnu/gdb/gdb/solib.c,v retrieving revision 1.1.1.2 diff -u -p -r1.1.1.2 solib.c --- solib.c 1999/10/19 16:16:27 1.1.1.2 +++ solib.c 1999/10/27 20:29:03 @@ -950,6 +954,38 @@ open_symbol_file_object (arg) return 1; } #endif /* SVR4_SHARED_LIBS */ + +/* + +GLOBAL FUNCTION + + check_solib_consistency -- check solib list consistency + +SYNOPSIS + + void check_solib_consistency (void) + +DESCRIPTION + + This module is called whenever we hit a dynamic linker breakpoint + and allows us to check the consistency of our shared object list. + Without this, dynamic unlinking of objects could crash us. + */ + +void +check_solib_consistency (void) +{ +#ifdef SVR4_SHARED_LIBS + + if ( debug_base ) { + read_memory (debug_base, (char *) &debug_copy, sizeof (struct r_debug)); + /* If the shared object state is consistent, we can reload our list */ + if ( debug_copy.r_state == RT_CONSISTENT ) + clear_solib(); + } + +#endif /* SVR4_SHARED_LIBS */ +} /* Index: infrun.c =================================================================== RCS file: /work/cvs/gnu/gdb/gdb/infrun.c,v retrieving revision 1.1.1.2 diff -u -p -r1.1.1.2 infrun.c --- infrun.c 1999/10/19 16:16:22 1.1.1.2 +++ infrun.c 1999/10/27 20:30:20 @@ -1479,6 +1479,9 @@ handle_inferior_event (struct execution_ /* Switch terminal for any messages produced by breakpoint_re_set. */ target_terminal_ours_for_output (); +#ifdef CHECK_SOLIB_CONSISTENCY + CHECK_SOLIB_CONSISTENCY(); +#endif SOLIB_ADD (NULL, 0, NULL); target_terminal_inferior (); } @@ -2409,6 +2412,9 @@ handle_inferior_event (struct execution_ /* Switch terminal for any messages produced by breakpoint_re_set. */ target_terminal_ours_for_output (); +#ifdef CHECK_SOLIB_CONSISTENCY + CHECK_SOLIB_CONSISTENCY(); +#endif SOLIB_ADD (NULL, 0, NULL); target_terminal_inferior (); } >From jimb@cygnus.com Wed Oct 27 23:44:00 1999 From: Jim Blandy To: Eli Zaretskii Cc: gdb@sourceware.cygnus.com Subject: Re: Hardware watchpoints Date: Wed, 27 Oct 1999 23:44:00 -0000 Message-id: References: <19991019235249.917DC1B494@ocean.lucon.org> <199910201401.KAA28719@mescaline.gnu.org> <199910221200.IAA24556@mescaline.gnu.org> <199910231048.GAA31392@mescaline.gnu.org> <199910272007.QAA04553@mescaline.gnu.org> X-SW-Source: 1999-q4/msg00176.html Content-length: 1309 > Now for the next problem: hardware watchpoints still don't work, even > after these changes, for struct members that are bitfields. As far as > I could see, the reason for this is that value_primitive_field needs > to fetch the value of the field in order to unpack it into an int, and > that causes the lazy flag of the struct itself to be reset. > > It seems that relying on the lazy flag is too fragile: any code that > needs to fetch the value of the struct, for some purpose, will break > the ability to watch members of that struct. > > Perhaps we need a special flag for this, which will be set for structs > and arrays only if they are not the watched expression itself. I agree that using the value chain is fragile. It's my impression that it was never intended for anything more than freeing old values. Nobody ever promised that it would have any specific semantics. Instead of adding yet another special flag, we could change value_primitive_field, or whoever extracts bitfields, to create a new value object, also on the chain, containing just the words from the original value that hold the bitfield, and then let that smaller value become unlazy. That should yield exactly the words we need to set hardware watchpoints on. I'm not sure what type that intermediate value should have. >From aj@suse.de Thu Oct 28 01:31:00 1999 From: Andreas Jaeger To: gdb@sourceware.cygnus.com Subject: Patch for warning in gdb/linux-thread.c Date: Thu, 28 Oct 1999 01:31:00 -0000 Message-id: X-SW-Source: 1999-q4/msg00177.html Content-length: 5354 The current cvs version of gdb produces a number of harmless warnings in linuxthreads. I've fixed the following warnings with the appended patch. ../../cvs/gdb/gdb/linux-thread.c:165: warning: missing initializer ../../cvs/gdb/gdb/linux-thread.c:165: warning: (near initialization for `linuxthreads_sig_restart.print') ../../cvs/gdb/gdb/linux-thread.c:168: warning: missing initializer ../../cvs/gdb/gdb/linux-thread.c:168: warning: (near initialization for `linuxthreads_sig_cancel.print') ../../cvs/gdb/gdb/linux-thread.c:171: warning: missing initializer ../../cvs/gdb/gdb/linux-thread.c:171: warning: (near initialization for `linuxthreads_sig_debug.print') ../../cvs/gdb/gdb/linux-thread.c: In function `linuxthreads_find_trap': ../../cvs/gdb/gdb/linux-thread.c:338: warning: suggest explicit braces to avoid ambiguous `else' ../../cvs/gdb/gdb/linux-thread.c: In function `resume_thread': ../../cvs/gdb/gdb/linux-thread.c:653: warning: suggest explicit braces to avoid ambiguous `else' ../../cvs/gdb/gdb/linux-thread.c: In function `stop_thread': ../../cvs/gdb/gdb/linux-thread.c:681: warning: suggest explicit braces to avoid ambiguous `else' ../../cvs/gdb/gdb/linux-thread.c: In function `linuxthreads_wait': ../../cvs/gdb/gdb/linux-thread.c:1286: warning: suggest explicit braces to avoid ambiguous `else' ../../cvs/gdb/gdb/linux-thread.c:1366: warning: suggest explicit braces to avoid ambiguous `else' Andreas 1999-10-28 Andreas Jaeger * linux-thread.c: Add missing initializers to avoid gcc warnings. (resume_thread): Add braces as recommended by gcc -Wparentheses. (stop_thread): Likewise. (linuxthreads_wait): Likewise. (linuxthreads_find_trap): Likewise. --- gdb/linux-thread.c.~1~ Thu Sep 9 01:59:18 1999 +++ gdb/linux-thread.c Thu Oct 28 10:28:53 1999 @@ -161,13 +161,13 @@ struct linuxthreads_signal { }; struct linuxthreads_signal linuxthreads_sig_restart = { - "__pthread_sig_restart", 1, 0, 0, 0 + "__pthread_sig_restart", 1, 0, 0, 0, 0 }; struct linuxthreads_signal linuxthreads_sig_cancel = { - "__pthread_sig_cancel", 1, 0, 0, 0 + "__pthread_sig_cancel", 1, 0, 0, 0, 0 }; struct linuxthreads_signal linuxthreads_sig_debug = { - "__pthread_sig_debug", 0, 0, 0, 0 + "__pthread_sig_debug", 0, 0, 0, 0, 0 }; /* A table of breakpoint locations, one per PID. */ @@ -336,10 +336,12 @@ linuxthreads_find_trap (pid, stop) else if (WSTOPSIG(status) != SIGSTOP) wstatus[last++] = status; else if (stop) - if (found_trap) - break; - else - found_stop = 1; + { + if (found_trap) + break; + else + found_stop = 1; + } } /* Resend any other signals we noticed to the thread, to be received @@ -651,10 +653,12 @@ resume_thread (pid) if (pid != inferior_pid && in_thread_list (pid) && linuxthreads_thread_alive (pid)) - if (pid == linuxthreads_step_pid) - child_resume (pid, 1, linuxthreads_step_signo); - else - child_resume (pid, 0, TARGET_SIGNAL_0); + { + if (pid == linuxthreads_step_pid) + child_resume (pid, 1, linuxthreads_step_signo); + else + child_resume (pid, 0, TARGET_SIGNAL_0); + } } /* Detach a thread */ @@ -679,21 +683,23 @@ stop_thread (pid) int pid; { if (pid != inferior_pid) - if (in_thread_list (pid)) - kill (pid, SIGSTOP); - else if (ptrace (PT_ATTACH, pid, (PTRACE_ARG3_TYPE) 0, 0) == 0) - { - if (!linuxthreads_attach_pending) - printf_unfiltered ("[New %s]\n", target_pid_to_str (pid)); - add_thread (pid); - if (linuxthreads_sig_debug.signal) - /* After a new thread in glibc 2.1 signals gdb its existence, - it suspends itself and wait for linuxthreads_sig_restart, - now we can wake up it. */ - kill (pid, linuxthreads_sig_restart.signal); - } - else - perror_with_name ("ptrace in stop_thread"); + { + if (in_thread_list (pid)) + kill (pid, SIGSTOP); + else if (ptrace (PT_ATTACH, pid, (PTRACE_ARG3_TYPE) 0, 0) == 0) + { + if (!linuxthreads_attach_pending) + printf_unfiltered ("[New %s]\n", target_pid_to_str (pid)); + add_thread (pid); + if (linuxthreads_sig_debug.signal) + /* After a new thread in glibc 2.1 signals gdb its existence, + it suspends itself and wait for linuxthreads_sig_restart, + now we can wake up it. */ + kill (pid, linuxthreads_sig_restart.signal); + } + else + perror_with_name ("ptrace in stop_thread"); + } } /* Wait for a thread */ @@ -1284,10 +1290,12 @@ linuxthreads_wait (pid, ourstatus) if (rpid > 0) break; if (rpid < 0) - if (errno == EINTR) - continue; - else if (save_errno != 0) - break; + { + if (errno == EINTR) + continue; + else if (save_errno != 0) + break; + } sigsuspend(&omask); } @@ -1364,10 +1372,12 @@ linuxthreads_wait (pid, ourstatus) { /* Skip SIGSTOP signals. */ if (!linuxthreads_pending_status (rpid)) - if (linuxthreads_step_pid == rpid) - child_resume (rpid, 1, linuxthreads_step_signo); - else - child_resume (rpid, 0, TARGET_SIGNAL_0); + { + if (linuxthreads_step_pid == rpid) + child_resume (rpid, 1, linuxthreads_step_signo); + else + child_resume (rpid, 0, TARGET_SIGNAL_0); + } continue; } -- Andreas Jaeger SuSE Labs aj@suse.de private aj@arthur.rhein-neckar.de >From bryce@albatross.co.nz Thu Oct 28 03:05:00 1999 From: Bryce McKinlay To: "H.J. Lu" Cc: slouken@devolution.com, GDB Subject: Re: A patch for shared library support Date: Thu, 28 Oct 1999 03:05:00 -0000 Message-id: <3818203B.28D094A5@albatross.co.nz> References: <19991027204300.7816E3FC1@valinux.com> X-SW-Source: 1999-q4/msg00178.html Content-length: 393 Great! Thanks - this bug has been annoying me too, and the patch seems to have fixed it. I've noticed that Redhat 6.1's gdb suffers from this as well. regards [ bryce ] "H.J. Lu" wrote: > I finally get annoyed enough to port Sam's patch to gdb 4.18 in CVS. > When you set a break point in shared library, gdb will crash when you > restart the program. This patch seems to work for me.