[PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Alan Hayward]

On aarch64, the (int) casting in the following causes a gdb segfault:
$ ./gdb ./gdb
(gdb) b dwarf2_physname if (int)strcmp (name, "another_thread_local") == 0
(gdb) run a.out         // use any a.out

This is due to getting a null pointer from TYPE_TARGET_TYPE, and then
using it for language_pass_by_reference().

Fixed by adding a null check, similar to other occurrences in gdb.

Tested on aarch64 with make check using unix, native_gdbserver.

Alan.


2018-03-01  Alan Hayward  <alan.hayward@arm.com>

	* aarch64-tdep.c (aarch64_push_dummy_call): Check for null
	return_type.

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index f08945ea07101e1cd7906ca640c023ac7d189dd9..ef982c78fe64ceef3c7c378fd22d76604bf81c31 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1382,7 +1382,7 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   struct aarch64_call_info info;
   struct type *func_type;
   struct type *return_type;
-  int lang_struct_return;
+  int lang_struct_return = 0;

   memset (&info, 0, sizeof (info));

@@ -1424,7 +1424,8 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
      given an additional initial argument, a hidden pointer to the
      return slot in memory.  */
   return_type = TYPE_TARGET_TYPE (func_type);
-  lang_struct_return = language_pass_by_reference (return_type);
+  if (return_type != nullptr)
+    lang_struct_return = language_pass_by_reference (return_type);

   /* Set the return address.  For the AArch64, the return breakpoint
      is always at BP_ADDR.  */

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Joel Brobecker]

On Thu, Mar 01, 2018 at 05:03:44PM +0000, Alan Hayward wrote:
> On aarch64, the (int) casting in the following causes a gdb segfault:
> $ ./gdb ./gdb
> (gdb) b dwarf2_physname if (int)strcmp (name, "another_thread_local") == 0
> (gdb) run a.out         // use any a.out
> 
> This is due to getting a null pointer from TYPE_TARGET_TYPE, and then
> using it for language_pass_by_reference().
> 
> Fixed by adding a null check, similar to other occurrences in gdb.
> 
> Tested on aarch64 with make check using unix, native_gdbserver.
> 
> Alan.
> 
> 
> 2018-03-01  Alan Hayward  <alan.hayward@arm.com>
> 
> 	* aarch64-tdep.c (aarch64_push_dummy_call): Check for null
> 	return_type.

The patch looks good to me, but do you think you could add a test
for it? Intuitively, I think this should be fairly easily doable,
but can you confirm?

> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index f08945ea07101e1cd7906ca640c023ac7d189dd9..ef982c78fe64ceef3c7c378fd22d76604bf81c31 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -1382,7 +1382,7 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>    struct aarch64_call_info info;
>    struct type *func_type;
>    struct type *return_type;
> -  int lang_struct_return;
> +  int lang_struct_return = 0;
> 
>    memset (&info, 0, sizeof (info));
> 
> @@ -1424,7 +1424,8 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>       given an additional initial argument, a hidden pointer to the
>       return slot in memory.  */
>    return_type = TYPE_TARGET_TYPE (func_type);
> -  lang_struct_return = language_pass_by_reference (return_type);
> +  if (return_type != nullptr)
> +    lang_struct_return = language_pass_by_reference (return_type);
> 
>    /* Set the return address.  For the AArch64, the return breakpoint
>       is always at BP_ADDR.  */
> 

-- 
Joel

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Alan Hayward]

> On 2 Mar 2018, at 03:32, Joel Brobecker <brobecker@adacore.com> wrote:
> 
> On Thu, Mar 01, 2018 at 05:03:44PM +0000, Alan Hayward wrote:
>> On aarch64, the (int) casting in the following causes a gdb segfault:
>> $ ./gdb ./gdb
>> (gdb) b dwarf2_physname if (int)strcmp (name, "another_thread_local") == 0
>> (gdb) run a.out         // use any a.out
>> 
>> This is due to getting a null pointer from TYPE_TARGET_TYPE, and then
>> using it for language_pass_by_reference().
>> 
>> Fixed by adding a null check, similar to other occurrences in gdb.
>> 
>> Tested on aarch64 with make check using unix, native_gdbserver.
>> 
>> Alan.
>> 
>> 
>> 2018-03-01  Alan Hayward  <alan.hayward@arm.com>
>> 
>> 	* aarch64-tdep.c (aarch64_push_dummy_call): Check for null
>> 	return_type.
> 
> The patch looks good to me, but do you think you could add a test
> for it? Intuitively, I think this should be fairly easily doable,
> but can you confirm?

Agreed, should be easy enough.
I’ve not added anything to the .exp files yet, so this is a good excuse for me to
look into them a bit more :)

Thanks for the review.


> On 2 Mar 2018, at 10:07, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> On Thu, Mar 1, 2018 at 5:03 PM, Alan Hayward <Alan.Hayward@arm.com> wrote:
>> 2018-03-01  Alan Hayward  <alan.hayward@arm.com>
>> 
>>        * aarch64-tdep.c (aarch64_push_dummy_call): Check for null
>>        return_type.
> 
> Add "PR gdb/22736" in ChangeLog entry.
> 

Will add.

> Any idea on why TYPE_TARGET_TYPE (func_type) is NULL? (because
> there is no strcmp debug info?)
> 

The cast to (int) is causing this - remove the cast and it finds the type.
I’m assuming that’s causing it to drop the debug info.


Also, thanks for the review.


Alan.

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Alan Hayward]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 810 bytes --]



> On 2 Mar 2018, at 12:21, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> On Fri, Mar 2, 2018 at 12:09 PM, Alan Hayward <Alan.Hayward@arm.com> wrote:
>>> Any idea on why TYPE_TARGET_TYPE (func_type) is NULL? (because
>>> there is no strcmp debug info?)
>>> 
>> 
>> The cast to (int) is causing this - remove the cast and it finds the type.
>> I’m assuming that’s causing it to drop the debug info.
> 
> It sounds like a bug to me.  If this bug "TYPE_TARGET_TYPE (func_type) becomes
> NULL caused by cast" is fixed, the GDB segfault will go away accordingly.
> 
> To be clear, your patch here is fine to me.  My suggestion is that we'd better
> dig it deeper.
> 

Agreed. I’ll raise a new bug, and have a look into it too whilst I’m in the area.


Alan.\x16º&Öéj×!zÊÞ¶êç׎{ßYb²Ö«r\x18\x1dn–­r\x17¬

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Joel Brobecker]

> >> The cast to (int) is causing this - remove the cast and it finds the type.
> >> I’m assuming that’s causing it to drop the debug info.
> > 
> > It sounds like a bug to me.  If this bug "TYPE_TARGET_TYPE
> > (func_type) becomes NULL caused by cast" is fixed, the GDB segfault
> > will go away accordingly.
> > 
> > To be clear, your patch here is fine to me.  My suggestion is that
> > we'd better dig it deeper.
> 
> Agreed. I’ll raise a new bug, and have a look into it too whilst I’m
> in the area.

Having read what Yao said, and looking at the example you gave,
I'm now thinking that one could very well be the cause of the other;
it seems like the cast might indeed be returning a value with
a struct type that's missing the return type. Even a subprogram
which returns nothing has a TYPE_TARGET_TYPE set to a TYPE_CODE_VOID,
right?

-- 
Joel

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Alan Hayward]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2961 bytes --]



> On 2 Mar 2018, at 15:18, Joel Brobecker <brobecker@adacore.com> wrote:
> 
>>>> The cast to (int) is causing this - remove the cast and it finds the type.
>>>> I’m assuming that’s causing it to drop the debug info.
>>> 
>>> It sounds like a bug to me.  If this bug "TYPE_TARGET_TYPE
>>> (func_type) becomes NULL caused by cast" is fixed, the GDB segfault
>>> will go away accordingly.
>>> 
>>> To be clear, your patch here is fine to me.  My suggestion is that
>>> we'd better dig it deeper.
>> 
>> Agreed. I’ll raise a new bug, and have a look into it too whilst I’m
>> in the area.
> 
> Having read what Yao said, and looking at the example you gave,
> I'm now thinking that one could very well be the cause of the other;
> it seems like the cast might indeed be returning a value with
> a struct type that's missing the return type. Even a subprogram
> which returns nothing has a TYPE_TARGET_TYPE set to a TYPE_CODE_VOID,
> right?

Been debugging this a little more (and learnt quite a few things about gdb along the way!)
Not sure if this reply is the best place to discuss, or if it should go onto the bug.
But….

On x86, for strcmp the type and it's target types, as received in amd64_push_dummy_call are:
TYPE_CODE_FUNC -> TYPE_CODE_INT -> 0x0
Whereas on aarch64 in aarch64_push_dummy_call we get:
TYPE_CODE_FUNC -> 0x0

It turns out this occurs because strcmp has IFUNC vs FUNC differences:

X86:
$ readelf -s /lib/x86_64-linux-gnu/libc-2.19.so | grep strcmp
  2085: 0000000000087380    60 IFUNC   GLOBAL DEFAULT   12 strcmp@@GLIBC_2.2.5

Aarch64:
$ readelf -s /lib/aarch64-linux-gnu/libc-2.23.so | grep strcmp
  2043: 0000000000076380   164 FUNC    GLOBAL DEFAULT   12 strcmp@@GLIBC_2.17


I decided to test this on X86 using a FUNC….

$ readelf -s /lib/x86_64-linux-gnu/libc-2.19.so | grep strlen
   769: 0000000000088dd0   436 FUNC    GLOBAL DEFAULT   12 strlen@@GLIBC_2.2.5

Changed my test to do:
(gdb) b foo if (int)strlen(name) == 3

...Now the type received by the target code is TYPE_CODE_FUNC -> 0, the same as aarch64.

However, x86 does not segfault. This is because amd64_push_dummy_call doesn’t bother to
check the function parameter.


Looking into the code, when reading a FUNC, readlf.c : elf_symtab_read() hits:

		  if (sym->flags & BSF_GNU_INDIRECT_FUNCTION)
		    ms_type = mst_text_gnu_ifunc;
		  else
		    ms_type = mst_text;

Setting the type to mst_text, which eventually causes find_minsym_type_and_address() to
set the type to objfile_type (objfile)->nodebug_text_symbol
Whereas an IFUNC gets set to objfile_type (objfile)->nodebug_text_gnu_ifunc_symbol.

So, I think probably either:
* Everything is correct
* mst_text needs handling differently
* FUNCs need a new minimal symbol type (mst_text_gnu_func?)
(I’m not familiar enough with this part of the code to give a definitive answer).


Alan.\x16º&Öéj×!zÊÞ¶êç׎{÷Ib²Ö«r\x18\x1dn–­r\x17¬

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Pedro Alves]

On 03/05/2018 03:57 PM, Alan Hayward wrote:
> 
> 
>> On 2 Mar 2018, at 15:18, Joel Brobecker <brobecker@adacore.com> wrote:
>>
>>>>> The cast to (int) is causing this - remove the cast and it finds the type.
>>>>> I’m assuming that’s causing it to drop the debug info.
>>>>
>>>> It sounds like a bug to me.  If this bug "TYPE_TARGET_TYPE
>>>> (func_type) becomes NULL caused by cast" is fixed, the GDB segfault
>>>> will go away accordingly.
>>>>
>>>> To be clear, your patch here is fine to me.  My suggestion is that
>>>> we'd better dig it deeper.
>>>
>>> Agreed. I’ll raise a new bug, and have a look into it too whilst I’m
>>> in the area.
>>
>> Having read what Yao said, and looking at the example you gave,
>> I'm now thinking that one could very well be the cause of the other;
>> it seems like the cast might indeed be returning a value with
>> a struct type that's missing the return type. Even a subprogram
>> which returns nothing has a TYPE_TARGET_TYPE set to a TYPE_CODE_VOID,
>> right?
> 
> Been debugging this a little more (and learnt quite a few things about gdb along the way!)
> Not sure if this reply is the best place to discuss, or if it should go onto the bug.
> But….
> 
> On x86, for strcmp the type and it's target types, as received in amd64_push_dummy_call are:
> TYPE_CODE_FUNC -> TYPE_CODE_INT -> 0x0
> Whereas on aarch64 in aarch64_push_dummy_call we get:
> TYPE_CODE_FUNC -> 0x0
> 
> It turns out this occurs because strcmp has IFUNC vs FUNC differences:
> 
> X86:
> $ readelf -s /lib/x86_64-linux-gnu/libc-2.19.so | grep strcmp
>   2085: 0000000000087380    60 IFUNC   GLOBAL DEFAULT   12 strcmp@@GLIBC_2.2.5
> 
> Aarch64:
> $ readelf -s /lib/aarch64-linux-gnu/libc-2.23.so | grep strcmp
>   2043: 0000000000076380   164 FUNC    GLOBAL DEFAULT   12 strcmp@@GLIBC_2.17
> 
> 
> I decided to test this on X86 using a FUNC….
> 
> $ readelf -s /lib/x86_64-linux-gnu/libc-2.19.so | grep strlen
>    769: 0000000000088dd0   436 FUNC    GLOBAL DEFAULT   12 strlen@@GLIBC_2.2.5
> 
> Changed my test to do:
> (gdb) b foo if (int)strlen(name) == 3
> 
> ...Now the type received by the target code is TYPE_CODE_FUNC -> 0, the same as aarch64.
> 
> However, x86 does not segfault. This is because amd64_push_dummy_call doesn’t bother to
> check the function parameter.
> 
> 
> Looking into the code, when reading a FUNC, readlf.c : elf_symtab_read() hits:
> 
> 		  if (sym->flags & BSF_GNU_INDIRECT_FUNCTION)
> 		    ms_type = mst_text_gnu_ifunc;
> 		  else
> 		    ms_type = mst_text;
> 
> Setting the type to mst_text, which eventually causes find_minsym_type_and_address() to
> set the type to objfile_type (objfile)->nodebug_text_symbol
> Whereas an IFUNC gets set to objfile_type (objfile)->nodebug_text_gnu_ifunc_symbol.
> 
> So, I think probably either:
> * Everything is correct
> * mst_text needs handling differently
> * FUNCs need a new minimal symbol type (mst_text_gnu_func?)
> (I’m not familiar enough with this part of the code to give a definitive answer).

Funny enough, I started working on fixing ifunc-related problems early
last week, but then got sidetracked into other high priority things.  I've not
run into a GDB crash, but just in case, maybe this branch helps:

 https://github.com/palves/gdb/commits/palves/ifunc

The gnu-ifunc.exp test doesn't pass cleanly on that branch
because I'm midway through augmenting that testcase to cover
different scenarios and evaluating whether it's the test that
needs fixing, or gdb.

Thanks,
Pedro Alves

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Alan Hayward]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 13590 bytes --]



> On 5 Mar 2018, at 16:45, Pedro Alves <palves@redhat.com> wrote:
> 
> On 03/05/2018 03:57 PM, Alan Hayward wrote:
>> 
>> 
>>> On 2 Mar 2018, at 15:18, Joel Brobecker <brobecker@adacore.com> wrote:
>>> 
>>>>>> The cast to (int) is causing this - remove the cast and it finds the type.
>>>>>> I’m assuming that’s causing it to drop the debug info.
>>>>> 
>>>>> It sounds like a bug to me.  If this bug "TYPE_TARGET_TYPE
>>>>> (func_type) becomes NULL caused by cast" is fixed, the GDB segfault
>>>>> will go away accordingly.
>>>>> 
>>>>> To be clear, your patch here is fine to me.  My suggestion is that
>>>>> we'd better dig it deeper.
>>>> 
>>>> Agreed. I’ll raise a new bug, and have a look into it too whilst I’m
>>>> in the area.
>>> 
>>> Having read what Yao said, and looking at the example you gave,
>>> I'm now thinking that one could very well be the cause of the other;
>>> it seems like the cast might indeed be returning a value with
>>> a struct type that's missing the return type. Even a subprogram
>>> which returns nothing has a TYPE_TARGET_TYPE set to a TYPE_CODE_VOID,
>>> right?
>> 
>> Been debugging this a little more (and learnt quite a few things about gdb along the way!)
>> Not sure if this reply is the best place to discuss, or if it should go onto the bug.
>> But….
>> 
>> On x86, for strcmp the type and it's target types, as received in amd64_push_dummy_call are:
>> TYPE_CODE_FUNC -> TYPE_CODE_INT -> 0x0
>> Whereas on aarch64 in aarch64_push_dummy_call we get:
>> TYPE_CODE_FUNC -> 0x0
>> 
>> It turns out this occurs because strcmp has IFUNC vs FUNC differences:
>> 
>> X86:
>> $ readelf -s /lib/x86_64-linux-gnu/libc-2.19.so | grep strcmp
>>  2085: 0000000000087380    60 IFUNC   GLOBAL DEFAULT   12 strcmp@@GLIBC_2.2.5
>> 
>> Aarch64:
>> $ readelf -s /lib/aarch64-linux-gnu/libc-2.23.so | grep strcmp
>>  2043: 0000000000076380   164 FUNC    GLOBAL DEFAULT   12 strcmp@@GLIBC_2.17
>> 
>> 
>> I decided to test this on X86 using a FUNC….
>> 
>> $ readelf -s /lib/x86_64-linux-gnu/libc-2.19.so | grep strlen
>>   769: 0000000000088dd0   436 FUNC    GLOBAL DEFAULT   12 strlen@@GLIBC_2.2.5
>> 
>> Changed my test to do:
>> (gdb) b foo if (int)strlen(name) == 3
>> 
>> ...Now the type received by the target code is TYPE_CODE_FUNC -> 0, the same as aarch64.
>> 
>> However, x86 does not segfault. This is because amd64_push_dummy_call doesn’t bother to
>> check the function parameter.
>> 
>> 
>> Looking into the code, when reading a FUNC, readlf.c : elf_symtab_read() hits:
>> 
>> 		  if (sym->flags & BSF_GNU_INDIRECT_FUNCTION)
>> 		    ms_type = mst_text_gnu_ifunc;
>> 		  else
>> 		    ms_type = mst_text;
>> 
>> Setting the type to mst_text, which eventually causes find_minsym_type_and_address() to
>> set the type to objfile_type (objfile)->nodebug_text_symbol
>> Whereas an IFUNC gets set to objfile_type (objfile)->nodebug_text_gnu_ifunc_symbol.
>> 
>> So, I think probably either:
>> * Everything is correct
>> * mst_text needs handling differently
>> * FUNCs need a new minimal symbol type (mst_text_gnu_func?)
>> (I’m not familiar enough with this part of the code to give a definitive answer).
> 
> Funny enough, I started working on fixing ifunc-related problems early
> last week, but then got sidetracked into other high priority things. I've not
> run into a GDB crash, but just in case, maybe this branch helps:
> 
> https://github.com/palves/gdb/commits/palves/ifunc
> 
> The gnu-ifunc.exp test doesn't pass cleanly on that branch
> because I'm midway through augmenting that testcase to cover
> different scenarios and evaluating whether it's the test that
> needs fixing, or gdb.
> 

Thanks for the branch, gave that it a try, but it has the same error.

Digging a little more into this….

The target type for the *function is set to the type of the function pointer, not the return type.
So, for IFUNC, TYPE_CODE_FUNC target type is TYPE_CODE_INT which gives the type
of the function pointer. For the FUNC, there is no pointer, so the target type of TYPE_CODE_FUNC
is set to null. That makes sense to me, but isn’t immediately obvious.

In call_function_by_hand_dummy() there already exists code to extract the type of the function
return, which is set as target_values_type. Patch below simply passes this through to the
gdbarch_push_dummy_call  method. Updated aarch64 and arm targets to use this.

If this patch is ok for amd64/aarch64/arm, then I will post again with _push_dummy_call()
fixed on every target. (Not done that yet because it’s a lot of tedious copy pasting). Most targets
don’t even look at the return types, so I’m not expecting many issues when I look at them.

Tested on aarch64 only with make check unitest and native_gdbserver.


2018-03-06  Alan Hayward  <alan.hayward@arm.com>

	PR gdb/22736
	* aarch64-tdep.c (aarch64_push_dummy_call): Use passed in return type.
	* amd64-tdep.c (amd64_push_dummy_call): Add new arg.
	* arm-tdep.c (arm_push_dummy_call): Use passed in return type.
	* gdbarch.sh (gdbarch_push_dummy_call): Add return type.
	* gdbarch.h: Regenerate.
	* gdbarch.sh: Likewise.
	* infcall.c (call_function_by_hand_dummy): Pass down return type.


diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index f08945ea07101e1cd7906ca640c023ac7d189dd9..bf9ae4475f80a8200eeb56a8edf4a9f7ee3daa37 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1376,13 +1376,12 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 			 struct regcache *regcache, CORE_ADDR bp_addr,
 			 int nargs,
 			 struct value **args, CORE_ADDR sp, int struct_return,
-			 CORE_ADDR struct_addr)
+			 CORE_ADDR struct_addr, struct type *return_type)
 {
   int argnum;
   struct aarch64_call_info info;
   struct type *func_type;
-  struct type *return_type;
-  int lang_struct_return;
+  int lang_struct_return = 0;

   memset (&info, 0, sizeof (info));

@@ -1411,20 +1410,11 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
      Rather that change the target interface we call the language code
      directly ourselves.  */

-  func_type = check_typedef (value_type (function));
-
-  /* Dereference function pointer types.  */
-  if (TYPE_CODE (func_type) == TYPE_CODE_PTR)
-    func_type = TYPE_TARGET_TYPE (func_type);
-
-  gdb_assert (TYPE_CODE (func_type) == TYPE_CODE_FUNC
-	      || TYPE_CODE (func_type) == TYPE_CODE_METHOD);
-
   /* If language_pass_by_reference () returned true we will have been
      given an additional initial argument, a hidden pointer to the
      return slot in memory.  */
-  return_type = TYPE_TARGET_TYPE (func_type);
-  lang_struct_return = language_pass_by_reference (return_type);
+  if (return_type != nullptr)
+    lang_struct_return = language_pass_by_reference (return_type);

   /* Set the return address.  For the AArch64, the return breakpoint
      is always at BP_ADDR.  */
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 6b92c9244c627af5fea78fdfd97b41a887fb679a..c50f611846c32364f31c33e0b4092af881fa6248 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -990,7 +990,8 @@ static CORE_ADDR
 amd64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		       struct regcache *regcache, CORE_ADDR bp_addr,
 		       int nargs, struct value **args,	CORE_ADDR sp,
-		       int struct_return, CORE_ADDR struct_addr)
+		       int struct_return, CORE_ADDR struct_addr,
+		       struct type *return_type)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   gdb_byte buf[8];
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index ef7e66b36afe34aed3880b86d16b466984481131..78240d21ef1082bfdaaafa6be6c2f5a73ac819a6 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -3689,7 +3689,7 @@ static CORE_ADDR
 arm_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		     struct regcache *regcache, CORE_ADDR bp_addr, int nargs,
 		     struct value **args, CORE_ADDR sp, int struct_return,
-		     CORE_ADDR struct_addr)
+		     CORE_ADDR struct_addr, struct type *return_type)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int argnum;
@@ -3700,12 +3700,8 @@ arm_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   struct type *ftype;
   unsigned vfp_regs_free = (1 << 16) - 1;

-  /* Determine the type of this function and whether the VFP ABI
-     applies.  */
-  ftype = check_typedef (value_type (function));
-  if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
-    ftype = check_typedef (TYPE_TARGET_TYPE (ftype));
-  use_vfp_abi = arm_vfp_abi_for_function (gdbarch, ftype);
+  /* Determine whether the VFP ABI applies.  */
+  use_vfp_abi = arm_vfp_abi_for_function (gdbarch, return_type);

   /* Set the return address.  For the ARM, the return breakpoint is
      always at BP_ADDR.  */
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 5cb131de1d27209107b5b83773ea7560ef0da6ac..086a230ffe6d295e1d1d1c3b5e3391ec979444eb 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -397,8 +397,8 @@ extern void set_gdbarch_deprecated_fp_regnum (struct gdbarch *gdbarch, int depre

 extern int gdbarch_push_dummy_call_p (struct gdbarch *gdbarch);

-typedef CORE_ADDR (gdbarch_push_dummy_call_ftype) (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr);
-extern CORE_ADDR gdbarch_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr);
+typedef CORE_ADDR (gdbarch_push_dummy_call_ftype) (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr, struct type *return_type);
+extern CORE_ADDR gdbarch_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr, struct type *return_type);
 extern void set_gdbarch_push_dummy_call (struct gdbarch *gdbarch, gdbarch_push_dummy_call_ftype *push_dummy_call);

 extern int gdbarch_call_dummy_location (struct gdbarch *gdbarch);
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index b8703e5a556e310e023cadf57037918d1bdd2850..50dd72db5d5a06a841391290c1afac345af49eb9 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -2368,13 +2368,13 @@ gdbarch_push_dummy_call_p (struct gdbarch *gdbarch)
 }

 CORE_ADDR
-gdbarch_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr)
+gdbarch_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr, struct type *return_type)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->push_dummy_call != NULL);
   if (gdbarch_debug >= 2)
     fprintf_unfiltered (gdb_stdlog, "gdbarch_push_dummy_call called\n");
-  return gdbarch->push_dummy_call (gdbarch, function, regcache, bp_addr, nargs, args, sp, struct_return, struct_addr);
+  return gdbarch->push_dummy_call (gdbarch, function, regcache, bp_addr, nargs, args, sp, struct_return, struct_addr, return_type);
 }

 void
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 33dfa6b349dee778f1a82a511a6cf57960d48f89..a1472069aa3d3a3bd355a772a4f9527f413330ca 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -488,7 +488,7 @@ M;struct frame_id;dummy_id;struct frame_info *this_frame;this_frame
 # deprecated_fp_regnum.
 v;int;deprecated_fp_regnum;;;-1;-1;;0

-M;CORE_ADDR;push_dummy_call;struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr;function, regcache, bp_addr, nargs, args, sp, struct_return, struct_addr
+M;CORE_ADDR;push_dummy_call;struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr, struct type *return_type;function, regcache, bp_addr, nargs, args, sp, struct_return, struct_addr, return_type
 v;int;call_dummy_location;;;;AT_ENTRY_POINT;;0
 M;CORE_ADDR;push_dummy_code;CORE_ADDR sp, CORE_ADDR funaddr, struct value **args, int nargs, struct type *value_type, CORE_ADDR *real_pc, CORE_ADDR *bp_addr, struct regcache *regcache;sp, funaddr, args, nargs, value_type, real_pc, bp_addr, regcache

diff --git a/gdb/infcall.c b/gdb/infcall.c
index b7f4a176db581c15c4fdd8c5299aab35d0fe4a68..773f43e884b3d831e0bd8254998cbb7ecb8daf33 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -1070,7 +1070,8 @@ call_function_by_hand_dummy (struct value *function,
      return address should be pointed.  */
   sp = gdbarch_push_dummy_call (gdbarch, function, get_current_regcache (),
 				bp_addr, nargs, args,
-				sp, struct_return, struct_addr);
+				sp, struct_return, struct_addr,
+				target_values_type);

   /* Set up a frame ID for the dummy frame so we can pass it to
      set_momentary_breakpoint.  We need to give the breakpoint a frame




\x16º&Öéj×!zÊÞ¶êç׎|ÛÙb²Ö«r\x18\x1dn–­r\x17¬

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Joel Brobecker]

Hello Alan,

> The target type for the *function is set to the type of the function
> pointer, not the return type.  So, for IFUNC, TYPE_CODE_FUNC target type
> is TYPE_CODE_INT which gives the type of the function pointer. For the
> FUNC, there is no pointer, so the target type of TYPE_CODE_FUNC is set
> to null. That makes sense to me, but isn’t immediately obvious.

I don't really understand what you mean by that -- maybe it is related
to the '*' before 'function' above? If we're talking about the
target_type for the function, it should be the type of the return value.
At least according to the documentation for that field in gdb_types.h.
But maybe you're talking about something else?

> In call_function_by_hand_dummy() there already exists code to extract
> the type of the function return, which is set as target_values_type.
> Patch below simply passes this through to the gdbarch_push_dummy_call
> method. Updated aarch64 and arm targets to use this.
>
> If this patch is ok for amd64/aarch64/arm, then I will post again with
> _push_dummy_call() fixed on every target. (Not done that yet because
> it’s a lot of tedious copy pasting). Most targets don’t even look at
> the return types, so I’m not expecting many issues when I look at
> them.

Sounds like a good change even if we didn't have this issue, as
it avoids a gdbarch method having to re-compute something that was
already previously computed.

> 2018-03-06  Alan Hayward  <alan.hayward@arm.com>
> 
> 	PR gdb/22736
> 	* aarch64-tdep.c (aarch64_push_dummy_call): Use passed in return type.
> 	* amd64-tdep.c (amd64_push_dummy_call): Add new arg.
> 	* arm-tdep.c (arm_push_dummy_call): Use passed in return type.
> 	* gdbarch.sh (gdbarch_push_dummy_call): Add return type.
> 	* gdbarch.h: Regenerate.
> 	* gdbarch.sh: Likewise.
> 	* infcall.c (call_function_by_hand_dummy): Pass down return type.

Just a minor comment below (besides what you already remarked on about
updating the remaining targets). Please give Pedro some time to comment;
my reviews have not be exactly top notch, lately, and I don't want you
to do useless work because I missed something.

> @@ -1411,20 +1410,11 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>       Rather that change the target interface we call the language code
>       directly ourselves.  */
> 
> -  func_type = check_typedef (value_type (function));
> -
> -  /* Dereference function pointer types.  */
> -  if (TYPE_CODE (func_type) == TYPE_CODE_PTR)
> -    func_type = TYPE_TARGET_TYPE (func_type);
> -
> -  gdb_assert (TYPE_CODE (func_type) == TYPE_CODE_FUNC
> -	      || TYPE_CODE (func_type) == TYPE_CODE_METHOD);
> -
>    /* If language_pass_by_reference () returned true we will have been
>       given an additional initial argument, a hidden pointer to the
>       return slot in memory.  */
> -  return_type = TYPE_TARGET_TYPE (func_type);
> -  lang_struct_return = language_pass_by_reference (return_type);
> +  if (return_type != nullptr)

I think return_type can never be nullptr (from what I understand of
the code in call_function_by_hand_dummy).

-- 
Joel

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Pedro Alves]

On 03/09/2018 08:51 AM, Joel Brobecker wrote:

> Just a minor comment below (besides what you already remarked on about
> updating the remaining targets). Please give Pedro some time to comment;
> my reviews have not be exactly top notch, lately, and I don't want you
> to do useless work because I missed something.

Sorry for the constant delays the past couple weeks.  I've been getting
distracted by other things more than usual.  Today I'm trying to finish/post
the ifunc-fixing series I pointed at earlier, and hopefully that will give
me enough background to understand/review this patch (I'm afraid I haven't
really looked at it in any detail).

Thanks,
Pedro Alves

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> Today I'm trying to finish/post
> the ifunc-fixing series I pointed at earlier, and hopefully that will give
> me enough background to understand/review this patch (I'm afraid I haven't
> really looked at it in any detail).

FWIW, this issue is *not* related to ifunc.  As Alan described in
previous email, ifunc symbol is OK, but normal function symbol's target
type is NULL, because without debug information, GDB doesn't know the
symbol is a function or not.  I thought about it, but I am not confident
that we can set symbol's target type (for example, set it void or int)
in absent of debug information.

Anyway, it is great if you can take a look.

-- 
Yao (齐尧)

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Alan Hayward]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3036 bytes --]



> On 9 Mar 2018, at 08:51, Joel Brobecker <brobecker@adacore.com> wrote:
> 
> Hello Alan,
> 
>> The target type for the *function is set to the type of the function
>> pointer, not the return type.  So, for IFUNC, TYPE_CODE_FUNC target type
>> is TYPE_CODE_INT which gives the type of the function pointer. For the
>> FUNC, there is no pointer, so the target type of TYPE_CODE_FUNC is set
>> to null. That makes sense to me, but isn’t immediately obvious.
> 
> I don't really understand what you mean by that -- maybe it is related
> to the '*' before 'function' above? If we're talking about the
> target_type for the function, it should be the type of the return value.
> At least according to the documentation for that field in gdb_types.h.
> But maybe you're talking about something else?

Agreed that the documentation says “type” should contain the type of the
function return.

I’m not quite sure on the terminology here, but, what the gdb code is picking
up and placing into type is the type of the function entry in the elf file.
The code in find_minsym_type_and_address() ensures that a FUNC will always
have the type nodebug_text_gnu_ifunc_symbol and an IFUNC will always have
the type nodebug_text_symbol.  This is completely unrelated to the return type
of the function, which is only resolved much later in call_function_by_hand_dummy().

In an ideal world, maybe the common code needs a bit of a rewrite:
The function structure needs to contain both the type of the elf entry and the
type of the return value. But, the existing value* and function* structures don’t
really support adding extra fields. I wasn’t really comfortable enough will this part
of the code to meddle with that.

Happy with the other comments too. Thanks!


> On 9 Mar 2018, at 16:04, Pedro Alves <palves@redhat.com> wrote:
> 
> Sorry for the constant delays the past couple weeks.  I've been getting
> distracted by other things more than usual.  Today I'm trying to finish/post
> the ifunc-fixing series I pointed at earlier, and hopefully that will give
> me enough background to understand/review this patch (I'm afraid I haven't
> really looked at it in any detail).
> 

Ok, happy to wait until you have some time. I’m looking at pr/22943 which also
might help with my understanding of this one a little more.

> 
> On 9 Mar 2018, at 16:44, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> FWIW, this issue is *not* related to ifunc.  As Alan described in
> previous email, ifunc symbol is OK, but normal function symbol's target
> type is NULL, because without debug information, GDB doesn't know the
> symbol is a function or not.  I thought about it, but I am not confident
> that we can set symbol's target type (for example, set it void or int)
> in absent of debug information.
> 

I’d just add that even with IFUNC the type will always be set to a pointer to an
Int, regardless of the return type of the function.




Alan.

\x16º&Öéj×!zÊÞ¶êç׎|ï™b²Ö«r\x18\x1dn–­r\x17¬

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Yao Qi]

On Thu, Mar 1, 2018 at 5:03 PM, Alan Hayward <Alan.Hayward@arm.com> wrote:
> 2018-03-01  Alan Hayward  <alan.hayward@arm.com>
>
>         * aarch64-tdep.c (aarch64_push_dummy_call): Check for null
>         return_type.

Add "PR gdb/22736" in ChangeLog entry.

Any idea on why TYPE_TARGET_TYPE (func_type) is NULL? (because
there is no strcmp debug info?)

-- 
Yao (齐尧)

[PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Weimin Pan]

Don't call language_pass_by_reference() with function that has no return type.

Only call language_pass_by_reference(), which returns whether or not an 
additional initial argument has been given, when return_type is not NULL
in function aarch64_push_dummy_call().

Tested on aarch64-linux-gnu. No regressions.
---
 gdb/ChangeLog      | 6 ++++++
 gdb/aarch64-tdep.c | 4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6f4153b..4c5691f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2018-05-23  Weimin Pan  <weimin.pan@oracle.com>
+
+	PR gdb/22736:
+	* aarch64-tdep.c (aarch64_push_dummy_call): Do not call
+	language_pass_by_reference if return_type is NULL.
+
 2018-05-14  Weimin Pan  <weimin.pan@oracle.com>
 
 	* minsyms.h (lookup_minimal_symbol_and_objfile): Remove declaration.
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 01566b4..b4633ff 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1426,7 +1426,9 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
      given an additional initial argument, a hidden pointer to the
      return slot in memory.  */
   return_type = TYPE_TARGET_TYPE (func_type);
-  lang_struct_return = language_pass_by_reference (return_type);
+  lang_struct_return = (return_type != NULL
+			 ? language_pass_by_reference (return_type) 
+			 : 0);
 
   /* Set the return address.  For the AArch64, the return breakpoint
      is always at BP_ADDR.  */
-- 
1.8.3.1

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Simon Marchi]

On 2018-05-25 19:20, Weimin Pan wrote:
> Don't call language_pass_by_reference() with function that has no 
> return type.
> 
> Only call language_pass_by_reference(), which returns whether or not an
> additional initial argument has been given, when return_type is not 
> NULL
> in function aarch64_push_dummy_call().

Hi Weimin,

Since Pedro's patch that makes GDB not assume that the return type of 
functions without debug info is int:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=7022349d5c86bae74b49225515f42d2e221bd368

I think we will always know the return type of the function.  Either 
it's in the debug info or it's provided by the user.  In 
call_function_by_hand_dummy, if the debug info doesn't provide the 
return type of the function, we use the type of the user-provided cast:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/infcall.c;h=cd3eedfeeb712b27234a68cf8af394558ce4f57d;hb=cd3eedfeeb712b27234a68cf8af394558ce4f57d#l870

I think the default_return_type could be passed down to 
gdbarch_push_dummy_call and used the same way, so that we always have a 
return type.

Also, could you add a test case for this?  I was able to create a simple 
C++ (not C) program made from an object file built with no debug info:

int returns_two ()
{
   return 2;
}

and one built with debug info:

int returns_two();

void func()
{
}

int main()
{
   func();
   return 0;
}


Putting this breakpoint and running crashes GDB:

(gdb) b func if (int)returns_two() == 2"

Thanks,

Simon

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Wei-min Pan]

On 5/25/2018 6:14 PM, Simon Marchi wrote:
> On 2018-05-25 19:20, Weimin Pan wrote:
>> Don't call language_pass_by_reference() with function that has no 
>> return type.
>>
>> Only call language_pass_by_reference(), which returns whether or not an
>> additional initial argument has been given, when return_type is not NULL
>> in function aarch64_push_dummy_call().
>
> Hi Weimin,
>
> Since Pedro's patch that makes GDB not assume that the return type of 
> functions without debug info is int:
>
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=7022349d5c86bae74b49225515f42d2e221bd368 
>
>
> I think we will always know the return type of the function. Either 
> it's in the debug info or it's provided by the user.  In 
> call_function_by_hand_dummy, if the debug info doesn't provide the 
> return type of the function, we use the type of the user-provided cast:
>
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/infcall.c;h=cd3eedfeeb712b27234a68cf8af394558ce4f57d;hb=cd3eedfeeb712b27234a68cf8af394558ce4f57d#l870 
>
>
> I think the default_return_type could be passed down to 
> gdbarch_push_dummy_call and used the same way, so that we always have 
> a return type.

Hi Simon,

Since call_function_by_hand_dummy () already calls 
gdbarch_return_in_first_hidden_param_p() and sets
hidden_first_param_p accordingly. Instead of passing the 
deault_return_type and having the target make
the same call again , I think we should just pass hidden_first_param_p 
to gdbarch_push_dummy_call()?

>
> Also, could you add a test case for this?  I was able to create a 
> simple C++ (not C) program made from an object file built with no 
> debug info:
>
> int returns_two ()
> {
>   return 2;
> }
>
> and one built with debug info:
>
> int returns_two();
>
> void func()
> {
> }
>
> int main()
> {
>   func();
>   return 0;
> }
>
>
> Putting this breakpoint and running crashes GDB:
>
> (gdb) b func if (int)returns_two() == 2"

Yes, will do and maybe use the one you provided here.

Thanks for your comments.

Weimin

>
> Thanks,
>
> Simon

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Simon Marchi]

On 2018-05-29 01:11 PM, Wei-min Pan wrote:
> Since call_function_by_hand_dummy () already calls gdbarch_return_in_first_hidden_param_p() and sets
> hidden_first_param_p accordingly. Instead of passing the deault_return_type and having the target make
> the same call again , I think we should just pass hidden_first_param_p to gdbarch_push_dummy_call()?

I can't really tell, I am a bit confused by gdbarch_return_in_first_hidden_param_p vs
using_struct_return, and the fact that the AArch64 code also checks
language_pass_by_reference on the function's return value type.  You are suggesting
replacing the call to language_pass_by_reference in aarch64_push_dummy_call by
the result of gdbarch_return_in_first_hidden_param_p coming from call_function_by_hand_dummy?
Is it really equivalent?

Simon

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Weimin Pan]

On 5/29/2018 1:40 PM, Simon Marchi wrote:
> On 2018-05-29 01:11 PM, Wei-min Pan wrote:
>> Since call_function_by_hand_dummy () already calls gdbarch_return_in_first_hidden_param_p() and sets
>> hidden_first_param_p accordingly. Instead of passing the deault_return_type and having the target make
>> the same call again , I think we should just pass hidden_first_param_p to gdbarch_push_dummy_call()?
> I can't really tell, I am a bit confused by gdbarch_return_in_first_hidden_param_p vs
> using_struct_return, and the fact that the AArch64 code also checks
> language_pass_by_reference on the function's return value type.  You are suggesting
> replacing the call to language_pass_by_reference in aarch64_push_dummy_call by
> the result of gdbarch_return_in_first_hidden_param_p coming from call_function_by_hand_dummy?
> Is it really equivalent?
>
> Simon

The traceback below shows that gdbarch_return_in_first_hidden_param_p() 
does
call language_pass_by_reference():


#0  gnuv3_pass_by_reference (type=0xbe0760) at gnu-v3-abi.c:1255
#1  0x000000000052cdc0 in cp_pass_by_reference (type=<optimized out>)
     at cp-abi.c:229
#2  0x00000000005d7428 in language_pass_by_reference (type=<optimized out>)
     at language.c:662
#3  0x00000000005a35c4 in gdbarch_return_in_first_hidden_param_p (
     gdbarch=gdbarch@entry=0xbd6100, type=0xbe0760) at gdbarch.c:2740
#4  0x00000000005bbe7c in call_function_by_hand_dummy (function=0xc319e0,
     default_return_type=default_return_type@entry=0xbe0760, nargs=0,
     args=0xffffffffe118, dummy_dtor=dummy_dtor@entry=0x0,
     dummy_dtor_data=dummy_dtor_data@entry=0x0) at infcall.c:894
  .....

Since I just learned this morning that Alan has been working on this PR, 
maybe it's
best for me  to withdraw the patch that I submitted. Sorry about that.

Weimin

Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type

[Pedro Alves]

On 05/26/2018 02:14 AM, Simon Marchi wrote:
> On 2018-05-25 19:20, Weimin Pan wrote:
>> Don't call language_pass_by_reference() with function that has no return type.
>>
>> Only call language_pass_by_reference(), which returns whether or not an
>> additional initial argument has been given, when return_type is not NULL
>> in function aarch64_push_dummy_call().
> 
> Hi Weimin,
> 
> Since Pedro's patch that makes GDB not assume that the return type of functions without debug info is int:
> 
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=7022349d5c86bae74b49225515f42d2e221bd368
> 
> I think we will always know the return type of the function.  Either it's in the debug info or it's provided by the user.  In call_function_by_hand_dummy, if the debug info doesn't provide the return type of the function, we use the type of the user-provided cast:
> 
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/infcall.c;h=cd3eedfeeb712b27234a68cf8af394558ce4f57d;hb=cd3eedfeeb712b27234a68cf8af394558ce4f57d#l870
> 
> I think the default_return_type could be passed down to gdbarch_push_dummy_call and used the same way, so that we always have a return type.

Agreed.  

Note this bug discussed earlier, and Alan had a patch too:

 https://sourceware.org/ml/gdb-patches/2018-03/msg00157.html

That was discussed just before the recent ifunc revamp, and I
wasn't exactly sure whether master still had the issue.  Also I
forgot about it.  :-P

Alan, do you recall the status of that from your end?

The issue of using the cast-to type was discussed then too:

 https://sourceware.org/ml/gdb-patches/2018-03/msg00204.html

So I wonder whether you already had a patch for that somewhere.

> 
> Also, could you add a test case for this?  I was able to create a simple C++ (not C) program made from an object file built with no debug info:
> 
> int returns_two ()
> {
>   return 2;
> }
> 
> and one built with debug info:
> 
> int returns_two();
> 
> void func()
> {
> }
> 
> int main()
> {
>   func();
>   return 0;
> }
> 
> 
> Putting this breakpoint and running crashes GDB:
> 
> (gdb) b func if (int)returns_two() == 2"
Thanks,
Pedro Alves