- Fix regression of gdb-7.0 (from 6.8) crashing on typedefed bitfields.

- Fix related_breakpoint stale ref crash.
This commit is contained in:
Jan Kratochvil 2010-01-02 21:26:54 +00:00
parent 2e0773b161
commit d7bd836d26
4 changed files with 265 additions and 82 deletions

View File

@ -0,0 +1,101 @@
http://sourceware.org/ml/gdb-patches/2010-01/msg00030.html
Subject: [patch] Re: Regression: field type preservation: 7.0 -> 7.0.1+HEAD
On Friday 01 January 2010 21:45:05 Jan Kratochvil wrote:
> -PASS: gdb.mi/mi-var-child.exp: get children of struct_declarations.s2.u2.u1s1
> +FAIL: gdb.mi/mi-var-child.exp: get children of struct_declarations.s2.u2.u1s1
> -PASS: gdb.mi/mi2-var-child.exp: get children of struct_declarations.s2.u2.u1s1
> +FAIL: gdb.mi/mi2-var-child.exp: get children of struct_declarations.s2.u2.u1s1
> -PASS: gdb.python/py-mi.exp: examine container children=0, no pretty-printing
> +FAIL: gdb.python/py-mi.exp: examine container children=0, no pretty-printing
>
> due to:
> Re: RFA: unbreak typedefed bitfield
> http://sourceware.org/ml/gdb-patches/2009-12/msg00295.html
> commit fc85da4ee2a7c32afc53b1b334a4f84e2e9bd84e
> http://sourceware.org/ml/gdb-cvs/2009-12/msg00100.html
attached a fix on top of existing HEAD.
Original PR gdb/10884 was a regression 6.8 -> 7.0 due to:
RFC: Lazy bitfields
http://sourceware.org/ml/gdb-patches/2009-07/msg00437.html
http://sourceware.org/ml/gdb-cvs/2009-07/msg00143.html
07491b3409f6ace0b7a9a707775a56ce10fece1c
No regressions for HEAD on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu. Plus:
-FAIL: gdb.mi/mi-var-child.exp: get children of struct_declarations.s2.u2.u1s1
+PASS: gdb.mi/mi-var-child.exp: get children of struct_declarations.s2.u2.u1s1
-FAIL: gdb.mi/mi2-var-child.exp: get children of struct_declarations.s2.u2.u1s1
+PASS: gdb.mi/mi2-var-child.exp: get children of struct_declarations.s2.u2.u1s1
-FAIL: gdb.python/py-mi.exp: examine container children=0, no pretty-printing
+PASS: gdb.python/py-mi.exp: examine container children=0, no pretty-printing
Going to check it in also for gdb_7_0-branch after an approval.
Thanks,
Jan
gdb/
2010-01-02 Jan Kratochvil <jan.kratochvil@redhat.com>
* value.c (value_primitive_field): Remove one check_typedef call.
Move bitpos and container_bitsize initialization after
allocate_value_lazy. New comment before accessing TYPE_LENGTH.
gdb/testsuite/
2010-01-02 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.mi/var-cmd.c (do_bitfield_tests): Change "V.sharable" type to
"uint_for_mi_testing".
--- a/gdb/testsuite/gdb.mi/var-cmd.c
+++ b/gdb/testsuite/gdb.mi/var-cmd.c
@@ -494,7 +494,7 @@ void do_bitfield_tests ()
mi_create_varobj V d "create varobj for Data"
mi_list_varobj_children "V" {
{"V.alloc" "alloc" "0" "int"}
- {"V.sharable" "sharable" "0" "unsigned int"}
+ {"V.sharable" "sharable" "0" "uint_for_mi_testing"}
} "list children of Data"
mi_check_varobj_value V.sharable 3 "access bitfield"
:*/
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1873,7 +1873,6 @@ value_primitive_field (struct value *arg1, int offset,
CHECK_TYPEDEF (arg_type);
type = TYPE_FIELD_TYPE (arg_type, fieldno);
- type = check_typedef (type);
/* Handle packed fields */
@@ -1885,10 +1884,14 @@ value_primitive_field (struct value *arg1, int offset,
Otherwise, adjust offset to the byte containing the first
bit. Assume that the address, offset, and embedded offset
are sufficiently aligned. */
- int bitpos = TYPE_FIELD_BITPOS (arg_type, fieldno);
- int container_bitsize = TYPE_LENGTH (type) * 8;
+ int bitpos, container_bitsize;
v = allocate_value_lazy (type);
+
+ /* TYPE_LENGTH of TYPE gets initialized by allocate_value_lazy. */
+ bitpos = TYPE_FIELD_BITPOS (arg_type, fieldno);
+ container_bitsize = TYPE_LENGTH (type) * 8;
+
v->bitsize = TYPE_FIELD_BITSIZE (arg_type, fieldno);
if ((bitpos % container_bitsize) + v->bitsize <= container_bitsize
&& TYPE_LENGTH (type) <= (int) sizeof (LONGEST))
@@ -1939,6 +1942,8 @@ value_primitive_field (struct value *arg1, int offset,
else
{
v = allocate_value (type);
+
+ /* TYPE_LENGTH of TYPE gets initialized by allocate_value. */
memcpy (value_contents_raw (v),
value_contents_raw (arg1) + offset,
TYPE_LENGTH (type));

View File

@ -1,80 +0,0 @@
http://sourceware.org/ml/gdb/2010-01/msg00017.html
revert:
http://sourceware.org/ml/gdb-cvs/2009-12/msg00104.html
http://sourceware.org/ml/gdb-cvs/2009-12/msg00103.html
Leftover testcase would severely hang the testsuite on timeouts.
--- ./gdb/testsuite/gdb.mi/mi-var-cmd.exp 2009-12-21 14:21:43.000000000 +0100
+++ ./gdb/testsuite/gdb.mi/mi-var-cmd.exp 2010-01-01 19:47:13.000000000 +0100
@@ -577,8 +577,6 @@ proc set_frozen {varobjs flag} {
mi_prepare_inline_tests $srcfile
mi_run_inline_test frozen
-mi_run_inline_test bitfield
-
# Since the inline test framework does not really work with
# function calls, first to inline tests and then do the reminder
# manually.
--- ./gdb/testsuite/gdb.mi/var-cmd.c 2009-12-21 14:21:43.000000000 +0100
+++ ./gdb/testsuite/gdb.mi/var-cmd.c 2010-01-01 19:47:13.000000000 +0100
@@ -468,40 +468,6 @@ void do_at_tests ()
/*: END: floating :*/
}
-/* Some header appear to define uint already, so apply some
- uglification. Note that without uglification, the compile
- does not fail, rather, we don't test what we want because
- something else calls check_typedef on 'uint' already. */
-typedef unsigned int uint_for_mi_testing;
-
-struct Data {
- int alloc;
- uint_for_mi_testing sharable : 4;
-};
-
-/* Accessing a value of a bitfield whose type is a typed used to
- result in division by zero. See:
-
- http://sourceware.org/bugzilla/show_bug.cgi?id=10884
-
- This tests for this bug. */
-
-void do_bitfield_tests ()
-{
- /*: BEGIN: bitfield :*/
- struct Data d = {0, 3};
- /*:
- mi_create_varobj V d "create varobj for Data"
- mi_list_varobj_children "V" {
- {"V.alloc" "alloc" "0" "int"}
- {"V.sharable" "sharable" "0" "unsigned int"}
- } "list children of Data"
- mi_check_varobj_value V.sharable 3 "access bitfield"
- :*/
- return;
- /*: END: bitfield :*/
-}
-
int
main (int argc, char *argv [])
{
@@ -511,7 +477,6 @@ main (int argc, char *argv [])
do_special_tests ();
do_frozen_tests ();
do_at_tests ();
- do_bitfield_tests ();
exit (0);
}
--- ./gdb/value.c 2010-01-01 19:46:58.000000000 +0100
+++ ./gdb/value.c 2010-01-01 19:47:13.000000000 +0100
@@ -1960,7 +1960,6 @@ value_primitive_field (struct value *arg
CHECK_TYPEDEF (arg_type);
type = TYPE_FIELD_TYPE (arg_type, fieldno);
- type = check_typedef (type);
/* Handle packed fields */

View File

@ -0,0 +1,154 @@
http://sourceware.org/ml/gdb-patches/2009-12/msg00364.html
Subject: [patch] related_breakpoint stale ref crash fix
Hi,
getting occasional random:
PASS: gdb.threads/local-watch-wrong-thread.exp: local watchpoint still triggers
PASS: gdb.threads/local-watch-wrong-thread.exp: let thread_function0 return
PASS: gdb.threads/local-watch-wrong-thread.exp: breakpoint on thread_function0's caller
-PASS: gdb.threads/local-watch-wrong-thread.exp: local watchpoint automatically deleted
+ERROR: Process no longer exists
+UNRESOLVED: gdb.threads/local-watch-wrong-thread.exp: local watchpoint automatically deleted
It is even reproducible on HEAD using "input" file below and:
valgrind ../gdb -nx <input
(gdb) (gdb) Breakpoint 6 at 0x400685: file ./gdb.threads/local-watch-wrong-thread.c, line 47.
(gdb) ==31759== Invalid write of size 4
==31759== at 0x601A11: bpstat_check_breakpoint_conditions (breakpoint.c:3482)
==31759== by 0x601C70: bpstat_stop_status (breakpoint.c:3596)
==31759== by 0x65D228: handle_inferior_event (infrun.c:3589)
==31759== by 0x65A563: wait_for_inferior (infrun.c:2281)
==31759== by 0x659AA0: proceed (infrun.c:1883)
==31759== by 0x65300B: continue_1 (infcmd.c:668)
==31759== by 0x653282: continue_command (infcmd.c:760)
==31759== by 0x5C51D8: do_cfunc (cli-decode.c:67)
==31759== by 0x5C824F: cmd_func (cli-decode.c:1738)
==31759== by 0x48335A: execute_command (top.c:450)
==31759== by 0x67273E: command_handler (event-top.c:511)
==31759== by 0x672E53: command_line_handler (event-top.c:736)
==31759== Address 0xbbdc950 is 240 bytes inside a block of size 336 free'd
==31759== at 0x4A04D72: free (vg_replace_malloc.c:325)
==31759== by 0x486E4B: xfree (utils.c:1286)
==31759== by 0x60BC35: delete_breakpoint (breakpoint.c:8708)
==31759== by 0x60BDAF: delete_command (breakpoint.c:8765)
==31759== by 0x5C51D8: do_cfunc (cli-decode.c:67)
==31759== by 0x5C824F: cmd_func (cli-decode.c:1738)
==31759== by 0x48335A: execute_command (top.c:450)
==31759== by 0x67273E: command_handler (event-top.c:511)
==31759== by 0x672E53: command_line_handler (event-top.c:736)
==31759== by 0x672FCF: gdb_readline2 (event-top.c:817)
==31759== by 0x6725F7: stdin_event_handler (event-top.c:433)
==31759== by 0x670CDE: handle_file_event (event-loop.c:812)
==31759==
Watchpoint 4 deleted because the program has left the block in
which its expression is valid.
There is already automatic deletion of RELATED_BREAKPOINT in
map_breakpoint_numbers but "delete breakpoints" (for all the breakpoints)
calls delete_breakpoint from delete_command directly without calling
map_breakpoint_numbers and it does not delete the associated
bp_watchpoint_scope.
I find the attached patch is right for delete_breakpoint itself as such
function should not leave stale references in the leftover data structures.
How well could be other code cleaned up with this patch in place I have not
targeted by this patch.
The existing code expects accessibility of freed memory and discusses the
current stale references problem:
void
delete_breakpoint (struct breakpoint *bpt)
[...]
/* Has this bp already been deleted? This can happen because multiple
lists can hold pointers to bp's. bpstat lists are especial culprits.
One example of this happening is a watchpoint's scope bp. When the
scope bp triggers, we notice that the watchpoint is out of scope, and
delete it. We also delete its scope bp. But the scope bp is marked
"auto-deleting", and is already on a bpstat. That bpstat is then
checked for auto-deleting bp's, which are deleted.
A real solution to this problem might involve reference counts in bp's,
and/or giving them pointers back to their referencing bpstat's, and
teaching delete_breakpoint to only free a bp's storage when no more
references were extent. A cheaper bandaid was chosen. */
if (bpt->type == bp_none)
return;
[...]
bpt->type = bp_none;
xfree (bpt);
}
While fixing this part may be difficult I find the attached patch easy enough
fixing the IMO currently most common crash due to it.
No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.
Thanks,
Jan
"input":
set height 0
set width 0
set confirm no
file ../testsuite/gdb.threads/local-watch-wrong-thread
set can-use-hw-watchpoints 1
break main
run
break local-watch-wrong-thread.c:36
continue
delete breakpoints
watch *myp
continue
delete breakpoints
echo MAKE watch\n
watch *myp if trigger != 0
echo MAKE break\n
break local-watch-wrong-thread.c:60
info break
continue
echo DELETE five\n
delete 5
set trigger=1
continue
set *myp=0
break local-watch-wrong-thread.c:47
continue
2009-12-23 Jan Kratochvil <jan.kratochvil@redhat.com>
* breakpoint.c (delete_breakpoint <bpt->related_breakpoint != NULL>):
New.
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8649,6 +8649,16 @@ delete_breakpoint (struct breakpoint *bpt)
if (bpt->type == bp_none)
return;
+ /* At least avoid this stale reference until the reference counting of
+ breakpoints gets resolved. */
+ if (bpt->related_breakpoint != NULL)
+ {
+ gdb_assert (bpt->related_breakpoint->related_breakpoint == bpt);
+ bpt->related_breakpoint->disposition = disp_del_at_next_stop;
+ bpt->related_breakpoint->related_breakpoint = NULL;
+ bpt->related_breakpoint = NULL;
+ }
+
observer_notify_breakpoint_deleted (bpt->number);
if (breakpoint_chain == bpt)

View File

@ -36,7 +36,7 @@ Version: 7.0.1
# The release always contains a leading reserved number, start it at 1.
# `upstream' is not a part of `name' to stay fully rpm dependencies compatible for the testing.
Release: 16%{?_with_upstream:.upstream}%{dist}
Release: 18%{?_with_upstream:.upstream}%{dist}
License: GPLv3+
Group: Development/Debuggers
@ -429,7 +429,10 @@ Patch397: gdb-follow-child-stale-parent.patch
Patch398: gdb-testsuite-unknown-output.patch
# Fix regression of gdb-7.0.1 not preserving typedef of a field.
Patch399: gdb-field-check_typedef-revert.patch
Patch399: gdb-bitfield-check_typedef.patch
# Fix related_breakpoint stale ref crash.
Patch400: gdb-stale-related_breakpoint.patch
BuildRequires: ncurses-devel texinfo gettext flex bison expat-devel
Requires: readline
@ -681,6 +684,7 @@ rm -f gdb/jv-exp.c gdb/m2-exp.c gdb/objc-exp.c gdb/p-exp.c
%patch397 -p1
%patch398 -p1
%patch399 -p1
%patch400 -p1
find -name "*.orig" | xargs rm -f
! find -name "*.rej" # Should not happen.
@ -998,6 +1002,10 @@ fi
%endif
%changelog
* Sat Jan 2 2010 Jan Kratochvil <jan.kratochvil@redhat.com> - 7.0.1-18.fc12
- Fix regression of gdb-7.0 (from 6.8) crashing on typedefed bitfields.
- Fix related_breakpoint stale ref crash.
* Fri Jan 1 2010 Jan Kratochvil <jan.kratochvil@redhat.com> - 7.0.1-17.fc12
- Formal upgrade to the FSF GDB release gdb-7.0.1.
- Fix regression of gdb-7.0.1 not preserving typedef of a field.