RFC: Relax checking of garbage struct return values

RFC: Relax checking of garbage struct return values

[Jim Blandy]

The comments are supposed to have the story.  Tested on
x86_64-pc-linux-gnu.

2005-10-05  Jim Blandy  <jimb@redhat.com>

	* gdb.base/structs.exp (any): New function.
	(test_struct_returns): Don't make any assumptions at all about
	what value the function returns when GDB can't set the return
	value.
	
Index: gdb/testsuite/gdb.base/structs.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/structs.exp,v
retrieving revision 1.21
diff -c -p -r1.21 structs.exp
*** gdb/testsuite/gdb.base/structs.exp	10 Jun 2004 21:38:03 -0000	1.21
--- gdb/testsuite/gdb.base/structs.exp	6 Oct 2005 17:41:49 -0000
*************** proc zed { n } {
*** 162,167 ****
--- 162,190 ----
      } $n]
  }
  
+ proc any { n } {
+     return [lindex {
+ 	"{}"
+ 	"{a = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*, e = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*, e = \[^,\}\]*, f = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*, e = \[^,\}\]*, f = \[^,\}\]*, g = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*, e = \[^,\}\]*, f = \[^,\}\]*, g = \[^,\}\]*, h = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*, e = \[^,\}\]*, f = \[^,\}\]*, g = \[^,\}\]*, h = \[^,\}\]*, i = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*, e = \[^,\}\]*, f = \[^,\}\]*, g = \[^,\}\]*, h = \[^,\}\]*, i = \[^,\}\]*, j = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*, e = \[^,\}\]*, f = \[^,\}\]*, g = \[^,\}\]*, h = \[^,\}\]*, i = \[^,\}\]*, j = \[^,\}\]*, k = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*, e = \[^,\}\]*, f = \[^,\}\]*, g = \[^,\}\]*, h = \[^,\}\]*, i = \[^,\}\]*, j = \[^,\}\]*, k = \[^,\}\]*, l = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*, e = \[^,\}\]*, f = \[^,\}\]*, g = \[^,\}\]*, h = \[^,\}\]*, i = \[^,\}\]*, j = \[^,\}\]*, k = \[^,\}\]*, l = \[^,\}\]*, m = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*, e = \[^,\}\]*, f = \[^,\}\]*, g = \[^,\}\]*, h = \[^,\}\]*, i = \[^,\}\]*, j = \[^,\}\]*, k = \[^,\}\]*, l = \[^,\}\]*, m = \[^,\}\]*, n = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*, e = \[^,\}\]*, f = \[^,\}\]*, g = \[^,\}\]*, h = \[^,\}\]*, i = \[^,\}\]*, j = \[^,\}\]*, k = \[^,\}\]*, l = \[^,\}\]*, m = \[^,\}\]*, n = \[^,\}\]*, o = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*, e = \[^,\}\]*, f = \[^,\}\]*, g = \[^,\}\]*, h = \[^,\}\]*, i = \[^,\}\]*, j = \[^,\}\]*, k = \[^,\}\]*, l = \[^,\}\]*, m = \[^,\}\]*, n = \[^,\}\]*, o = \[^,\}\]*, p = \[^,\}\]*}"
+ 	"{a = \[^,\}\]*, b = \[^,\}\]*, c = \[^,\}\]*, d = \[^,\}\]*, e = \[^,\}\]*, f = \[^,\}\]*, g = \[^,\}\]*, h = \[^,\}\]*, i = \[^,\}\]*, j = \[^,\}\]*, k = \[^,\}\]*, l = \[^,\}\]*, m = \[^,\}\]*, n = \[^,\}\]*, o = \[^,\}\]*, p = \[^,\}\]*, q = \[^,\}\]*}"
+     } $n]
+ }
+ 
  # Given N (0..25), return the corresponding alphabetic letter in lower
  # or upper case.  This is ment to be i18n proof.
  
*************** proc test_struct_returns { n } {
*** 330,357 ****
      # Check that the return-value is as expected.  At this stage we're
      # just checking that GDB has returned a value consistent with
      # "return_value_known" set above.
  
      set test "value foo<n> returned; ${tests}"
      gdb_test_multiple "p/c L${n}" "${test}" {
  	-re " = [foo ${n}].*${gdb_prompt} $" {
! 	    if $return_value_known {
! 		pass "${test}"
! 		# This contradicts the above claim that GDB didn't
! 		# know the location of the return-value.
! 	    } else {
! 		fail "${test}"
! 	    }
  	}
! 	-re " = [zed ${n}].*${gdb_prompt} $" {
  	    if $return_value_known {
  		# This contradicts the above claim that GDB knew
! 		# the location of the return-value.
  		fail "${test}"
  	    } else {
! 		# The struct return case.  Since any modification
! 		# would be by reference, and that can't happen, the
! 		# value should be unmodified and hence Z is expected.
! 		# Is this a reasonable assumption?
  		pass "${test}"
  	    }
  	}
--- 353,395 ----
      # Check that the return-value is as expected.  At this stage we're
      # just checking that GDB has returned a value consistent with
      # "return_value_known" set above.
+     #
+     # Note that, when return_value_known is false, we can't make any
+     # assumptions at all about the value L<n>:
+     #
+     # - If the caller passed the address of L<n> directly as fun<n>'s
+     #   return value buffer, then L<n> will be unchanged, because we
+     #   forced fun<n> to return before it could store anything in it.
+     #
+     # - If the caller passed the address of some temporary buffer to
+     #   fun<n>, and then copied the buffer into L<n>, then L<n> will
+     #   have been overwritten with whatever garbage was in the
+     #   uninitialized buffer.
+     #
+     # - However, if the temporary buffer just happened to have the
+     #   "right" value of foo<n> in it, then L<n> will, in fact, have
+     #   the value you'd expect to see if the 'return' had worked!
+     #   This has actually been observed to happen on the Renesas M32C.
+     #
+     # So, really, anything is acceptable unless return_value_known is
+     # true.
  
      set test "value foo<n> returned; ${tests}"
      gdb_test_multiple "p/c L${n}" "${test}" {
  	-re " = [foo ${n}].*${gdb_prompt} $" {
!             # This answer is okay regardless of whether GDB claims to
!             # have set the return value: if it did, then this is what
!             # we expected; and if it didn't, then any answer is okay.
!             pass "${test}"
  	}
! 	-re " = [any $n].*${gdb_prompt} $" {
  	    if $return_value_known {
  		# This contradicts the above claim that GDB knew
! 		# the location of the return value.
  		fail "${test}"
  	    } else {
!                 # We expected L${n} to be set to garbage, so any
!                 # answer is acceptable.
  		pass "${test}"
  	    }
  	}

Re: RFC: Relax checking of garbage struct return values

[Daniel Jacobowitz]

On Thu, Oct 06, 2005 at 10:44:29AM -0700, Jim Blandy wrote:
> 
> The comments are supposed to have the story.  Tested on
> x86_64-pc-linux-gnu.
> 
> 2005-10-05  Jim Blandy  <jimb@redhat.com>
> 
> 	* gdb.base/structs.exp (any): New function.
> 	(test_struct_returns): Don't make any assumptions at all about
> 	what value the function returns when GDB can't set the return
> 	value.

Yeah, I agree with your comments.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: RFC: Relax checking of garbage struct return values

[Mark Kettenis]

> From: Jim Blandy <jimb@redhat.com>
> Date: Thu, 06 Oct 2005 10:44:29 -0700
> 
> The comments are supposed to have the story.  Tested on
> x86_64-pc-linux-gnu.
> 
> 2005-10-05  Jim Blandy  <jimb@redhat.com>
> 
> 	* gdb.base/structs.exp (any): New function.
> 	(test_struct_returns): Don't make any assumptions at all about
> 	what value the function returns when GDB can't set the return
> 	value.

I remember having a discussion about this with Andrew some time ago.
I believe Andrew argued that if the compiler doesn't pass L<n> as the
buffer, this should be considered a compiler bug.

Anyway, there is another case where things fail.  Some compilers use a
"static" buffer to store function values.  On those systems these
tests fail too.

I think your patch is ok: gdb is not a compiler testsuite.

Mark

Re: RFC: Relax checking of garbage struct return values

[Jim Blandy]

Mark Kettenis <mark.kettenis@xs4all.nl> writes:
>> From: Jim Blandy <jimb@redhat.com>
>> Date: Thu, 06 Oct 2005 10:44:29 -0700
>> 
>> The comments are supposed to have the story.  Tested on
>> x86_64-pc-linux-gnu.
>> 
>> 2005-10-05  Jim Blandy  <jimb@redhat.com>
>> 
>> 	* gdb.base/structs.exp (any): New function.
>> 	(test_struct_returns): Don't make any assumptions at all about
>> 	what value the function returns when GDB can't set the return
>> 	value.
>
> I remember having a discussion about this with Andrew some time ago.
> I believe Andrew argued that if the compiler doesn't pass L<n> as the
> buffer, this should be considered a compiler bug.

I don't agree with that at all.

The compiler is free to generate any code it pleases, as long as the
machine code behaves the way the semantics of C require it to.  An
ordinary assignment of one int to another could go through seven
temporaries, get xor'd with 55 twice, pushed on the stack, and then
popped into its destination, and that's correct.  It's just lousy
code.

> Anyway, there is another case where things fail.  Some compilers use a
> "static" buffer to store function values.  On those systems these
> tests fail too.

It's a pretty losing convention, because it isn't reentrant.  But I
know of such a compiler, too.

> I think your patch is ok: gdb is not a compiler testsuite.

Okay --- thanks for the review.