[PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type
Alan Hayward
Alan.Hayward@arm.com
Wed Mar 7 11:10:00 GMT 2018
> 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
More information about the Gdb-patches
mailing list