Re: [PATCH v3 3/3] Aarch64: Fix segfault when casting dummy calls

Pedro Alves
2018-10-24T15:15:49Z

On 10/23/2018 05:08 PM, Alan Hayward wrote:
>
>
>> On 19 Oct 2018, at 12:35, Pedro Alves <palves@redhat.com> wrote:
>>
>
> <snip>
>
>>> to calculate lang_struct_return. This information is available in the
>>> return_method enum. The fix is to simply use this instead.
>>>
>>> Add common test case using a shared library to ensure FUNC resolving.
>>
>> What does "common" mean here?
>
> By common I just meant not-aarch64-specific. I guess what I really meant
> was any target that supported shared libraries.

I see. (Suggest clarifying it in the commit log then.)

>
>>
>> And what does "to ensure FUNC resolving" mean too, btw?
>> AFAICT, the only reason to use a shared library is to
>> compile it with or without debug information, right?
>> Come to think of it, you could instead eliminate the
>> shared library and compile a separate .o file instead, right?
>> That would simplify the testcase a bit and expose it to more
>> targets.
>>
>
> I could only get the bug to expose itself when using a .so
>
> If I do the following using current HEAD then the bug is not present:
>
> g++ -c condbreak-solib-main.cc -o condbreak-solib-main.o -fno-inline
> g++ -c condbreak-solib-lib.cc -o condbreak-solib-lib.o -fno-inline
> g++ condbreak-solib-main.o condbreak-solib-lib.o
>
> It causes the type of the return value to be detected as
> TYPE_CODE_PTR->TYPE_CODE_INT.

Huh. That's really strange. Where is that pointer coming from?

What does "ptype cmp3" say for you?

(gdb) b foo if (int)cmp3("abc") == 1
Breakpoint 1 at 0x40048b
(gdb) ptype cmp3
type = <unknown return type> ()
(gdb) whatis cmp3
type = <text variable, no debug info>

I wonder if what you're looking at is actually the malloc call?
GDB will call malloc to allocate space for the "abc" string in
the inferior. Look at the backtrace, see where the call is coming
from.

Actually, because of that, I think it would be better (more focused)
to pass in a variable instead of "abc". You already have the
unused variable "word" there:

const char *word = "stuff";

So:

(gdb) b foo if (int)cmp3(word) == 1

but please rename it to something else, because I tried that
locally and there's another symbol called "word"
in glibc's localeinfo.h, and gdb picks that one up.

Also, is there a reason for the test to be a C++ program?
If there's none, it'd be better to make it a C program, again
to expose it to more targets.

Thanks,
Pedro Alves