Re: [PATCH] Fix print of value type in a corner case of finish

[Pedro Alves <palves@redhat.com>]

On 02/26/2015 02:11 PM, Antoine Tremblay wrote:
> On 02/26/2015 08:35 AM, Pedro Alves wrote:
>> It there a way to add a test?
> I would need to add a test that's specific to an architecture that does 
> not support the struct return value...

Hmm, AFAICS, it's not that the architecture doesn't support
the struct return value, or some kind of memory read error or
some such.  It's that the ABI returns the object using
struct return convention / RETURN_VALUE_STRUCT_CONVENTION:

/* Return the value of the result of a function at the end of a 'finish'
   command/BP.  */

struct value *
get_return_value (struct value *function, struct type *value_type)
{

...
  switch (gdbarch_return_value (gdbarch, function, value_type,
  				NULL, NULL, NULL))
    {
...
      break;
    case RETURN_VALUE_STRUCT_CONVENTION:
      value = NULL;
      break;

I think that we make the test return a big enough
structure, we'll hit RETURN_VALUE_STRUCT_CONVENTION on most
architectures (but not x86...)?

For archs that don't do RETURN_VALUE_STRUCT_CONVENTION, the
test could cope, by also passing if GDB manages to extract
the real value.  In sum, PASS if real value, or at least type.
FAIL otherwise.

But, ... I'm actually very much surprised we don't test this
already.  So I did

$ grep -rn "\"finish\"" testsuite/

and noticed:
...
gdb.base/structs.exp:259:# "finish") and correctly extract/store any corresponding
gdb.base/structs.exp:276:# is in three parts: test "return"; test "finish"; check that the two
gdb.base/structs.exp:299:    # consistency between this and the "finish" case.
gdb.base/structs.exp:404:    # Check that a "finish" works.
gdb.base/structs.exp:427:    gdb_test_multiple "finish" "${test}" {
gdb.base/structs.exp:467:    # Since "finish" works in more cases than "return" (see
gdb.base/structs.exp:470:    # known implies that the "finish" value is known (but not the
...

And lo:

    set test "finish foo<n>; ${tests}"
    set finish_value_known 1
    gdb_test_multiple "finish" "${test}" {
	-re "Value returned is .*${gdb_prompt} $" {
	    pass "${test}"
	}
	-re "Cannot determine contents.*${gdb_prompt} $" {
	    # Expected bad value.  For the moment this is ok.
	    set finish_value_known 0
	    pass "${test}"
	}
    }


So the regex here is too lax and missed this bug.  Could you
tweak it to make sure some type was output?

> 
>> Also please use "const char *" for type_name, so C++
>> doesn't complain.  :-)
> Humm not sure I understand here since type_to_string returns a char* 
> that needs to be freed ?
> 
> Converting to const char* would make make_cleanup complain...
> 
> How is C++ complaining ?

Oh, nevermind, that was a thinko on my part, sorry.


Thanks,
Pedro Alves