Patch for cp-valprint.c

Patch for cp-valprint.c

[Chris Moller]

[-- Attachment #1: Type: text/plain, Size: 400 bytes --]

While working on something else, I found the following:

    /* FIXME (alloca): unsafe if baseclass is really really large. */
    gdb_byte *buf = alloca (TYPE_LENGTH (baseclass));

The attached patch replaces that with a xmalloc/xfree pair.

There are some differences I don't understand between the before- and 
after- gdb.sum files from make check--the diff -u of those files is also 
attached.



[-- Attachment #2: cp-valprint.c.patch --]
[-- Type: text/plain, Size: 870 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.11285
diff -r1.11285 ChangeLog
0a1,5
> Fri Jan 22 09:52:28 2010  Chris Moller  <cmoller@redhat.com>
> 
> 	* cp-valprint.c (cp_print_value): Took out an alloca and replaced
> 	it with a xmalloc/xfree pair.
> 
Index: cp-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/cp-valprint.c,v
retrieving revision 1.62
diff -r1.62 cp-valprint.c
356a357,359
>       gdb_byte *buf;
> 
>       buf = NULL;
389,390c392
< 	      /* FIXME (alloca): unsafe if baseclass is really really large. */
< 	      gdb_byte *buf = alloca (TYPE_LENGTH (baseclass));
---
> 	      buf = xmalloc (TYPE_LENGTH (baseclass));
446a449,453
> 
>       if (buf) {
> 	xfree (buf);
> 	buf = NULL;
>       }

[-- Attachment #3: gdb-sum.diff --]
[-- Type: text/plain, Size: 3168 bytes --]

--- /tmp/virgin-gdb.sum	2010-01-22 11:57:29.542165115 -0500
+++ /tmp/patched-gdb.sum	2010-01-22 12:15:24.255165387 -0500
@@ -1,4 +1,4 @@
-Test Run By moller on Fri Jan 22 11:47:04 2010
+Test Run By moller on Fri Jan 22 11:57:53 2010
 Native configuration is i686-pc-linux-gnu
 
 		=== gdb tests ===
@@ -8167,15 +8167,10 @@
 PASS: gdb.base/solib-display.exp: display a_global
 PASS: gdb.base/solib-display.exp: display b_global
 PASS: gdb.base/solib-display.exp: display c_global
-FAIL: gdb.base/solib-display.exp: after rerun
-FAIL: gdb.base/solib-display.exp: after rerun (GDB internal error)
-PASS: gdb.base/solib-display.exp: break 25
-FAIL: gdb.base/solib-display.exp: continue (GDB internal error)
-PASS: gdb.base/solib-display.exp: display main_global
-PASS: gdb.base/solib-display.exp: display a_local
-PASS: gdb.base/solib-display.exp: display a_static
-PASS: gdb.base/solib-display.exp: break 25
-FAIL: gdb.base/solib-display.exp: continue
+ERROR: Process no longer exists
+UNRESOLVED: gdb.base/solib-display.exp: after rerun
+WARNING: remote_expect statement without a default case?!
+FAIL: gdb.base/solib-display.exp: Can't run to main (3)
 Running ../../../src/gdb/testsuite/gdb.base/solib-overlap.exp ...
 PASS: gdb.base/solib-overlap.exp: 0x40000000: attach
 PASS: gdb.base/solib-overlap.exp: 0x40000000: detach
@@ -12363,7 +12358,7 @@
 PASS: gdb.mi/mi-break.exp: mi runto main
 PASS: gdb.mi/mi-break.exp: breakpoint commands: insert breakpoint at basics.c:callee2, again
 PASS: gdb.mi/mi-break.exp: breakpoint commands: set commands
-PASS: gdb.mi/mi-break.exp: test hitting breakpoint with commands
+FAIL: gdb.mi/mi-break.exp: test hitting breakpoint with commands
 Running ../../../src/gdb/testsuite/gdb.mi/mi-cli.exp ...
 PASS: gdb.mi/mi-cli.exp: -interpreter-exec with no arguments
 PASS: gdb.mi/mi-cli.exp: -interpreter-exec with one argument
@@ -12526,7 +12521,7 @@
 PASS: gdb.mi/mi-nsmoribund.exp: resume all, thread specific breakpoint
 PASS: gdb.mi/mi-nsmoribund.exp: hit thread specific breakpoint
 PASS: gdb.mi/mi-nsmoribund.exp: thread state: all running except the breakpoint thread
-PASS: gdb.mi/mi-nsmoribund.exp: resume all, program exited normally
+FAIL: gdb.mi/mi-nsmoribund.exp: unexpected stop
 Running ../../../src/gdb/testsuite/gdb.mi/mi-nsthrexec.exp ...
 PASS: gdb.mi/mi-nsthrexec.exp: successfully compiled posix threads test case
 PASS: gdb.mi/mi-nsthrexec.exp: breakpoint at main
@@ -12592,6 +12587,7 @@
 PASS: gdb.mi/mi-simplerun.exp: step at main
 PASS: gdb.mi/mi-simplerun.exp: step to callee4
 PASS: gdb.mi/mi-simplerun.exp: exec-finish
+FAIL: gdb.mi/mi-simplerun.exp: continue to end (failed to resume)
 PASS: gdb.mi/mi-simplerun.exp: continue to end
 Running ../../../src/gdb/testsuite/gdb.mi/mi-stack.exp ...
 PASS: gdb.mi/mi-stack.exp: breakpoint at callee4
@@ -15636,10 +15632,11 @@
 
 		=== gdb Summary ===
 
-# of expected passes		14843
+# of expected passes		14836
 # of unexpected failures	30
 # of expected failures		41
 # of untested testcases		3
+# of unresolved testcases	1
 # of unsupported tests		61
 /home/moller/tinkering/fsfgdb-ck/build/gdb/testsuite/../../gdb/gdb version  7.0.50.20100122-cvs -nw -nx 
 

Re: Patch for cp-valprint.c

[Tom Tromey]

>>>>> "Chris" == Chris Moller <cmoller@redhat.com> writes:

Chris> While working on something else, I found the following:
Chris>    /* FIXME (alloca): unsafe if baseclass is really really large. */
Chris>    gdb_byte *buf = alloca (TYPE_LENGTH (baseclass));

Chris> The attached patch replaces that with a xmalloc/xfree pair.

Please send unidiff or context diffs.

I just put "diff -u -N" into my ~/.cvsrc.  I've never regretted it.

>> buf = xmalloc (TYPE_LENGTH (baseclass));

>> if (buf) {
>> xfree (buf);
>> buf = NULL;

In gdb you can't generally do pairings like this, due to exceptions.
Instead, you must make a cleanup.

Also, you don't need to check the argument to xfree; xfree handles NULL
pointers fine.

Chris> -FAIL: gdb.base/solib-display.exp: after rerun
Chris> -FAIL: gdb.base/solib-display.exp: after rerun (GDB internal error)
Chris> -PASS: gdb.base/solib-display.exp: break 25
Chris> -FAIL: gdb.base/solib-display.exp: continue (GDB internal error)
Chris> -PASS: gdb.base/solib-display.exp: display main_global
Chris> -PASS: gdb.base/solib-display.exp: display a_local
Chris> -PASS: gdb.base/solib-display.exp: display a_static
Chris> -PASS: gdb.base/solib-display.exp: break 25
Chris> -FAIL: gdb.base/solib-display.exp: continue
Chris> +ERROR: Process no longer exists
Chris> +UNRESOLVED: gdb.base/solib-display.exp: after rerun
Chris> +WARNING: remote_expect statement without a default case?!
Chris> +FAIL: gdb.base/solib-display.exp: Can't run to main (3)

Jan has a patch pending for this.

I don't know about the other changes offhand.  Sometimes I re-run the
test suite to see if problems just go away... though I think Jan has put
some work into making this happen less.

Tom

Re: Patch for cp-valprint.c

[Jan Kratochvil]

On Fri, 22 Jan 2010 23:07:01 +0100, Tom Tromey wrote:
> I don't know about the other changes offhand.  Sometimes I re-run the
> test suite to see if problems just go away... though I think Jan has put
> some work into making this happen less.

-PASS: gdb.mi/mi-break.exp: test hitting breakpoint with commands
+FAIL: gdb.mi/mi-break.exp: test hitting breakpoint with commands

From: Daniel Jacobowitz: Bug in mi-break.exp
http://sourceware.org/ml/gdb-patches/2009-11/msg00510.html

IIRC found it as not completely trivial to fix it.


-PASS: gdb.mi/mi-nsmoribund.exp: resume all, program exited normally
+FAIL: gdb.mi/mi-nsmoribund.exp: unexpected stop

In my ignore list as racy.


+FAIL: gdb.mi/mi-simplerun.exp: continue to end (failed to resume)

Unaware of.


Regards,
Jan