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

[Pedro Alves <palves at redhat dot com>]

On 10/29/2018 11:58 AM, Alan Hayward wrote:
> 
> 
>> On 24 Oct 2018, at 16:14, Pedro Alves <palves@redhat.com> wrote:
>>
>>>
>>>>
>>>> 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>
>>
> 
> Yes, I get the same.
> 
> Sounds to me like there might still be an issue in gdb? Or at least
> a difference in the way the type is calculated.
> This on it’s own is still a good fix, so I’ll send a new version.

I think we should learn the answer to the "where is that pointer
coming from" question.  I'm really not understanding why the
shared library makes a difference.

> 
> 
>> 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.
> 
> 
> With the nodebug and debug shared library version:
> (I need to run to main before I can set breakpoint on cmp3, otherwise
> "Function "cmp3" not defined.”)
> 
> But with all versions, when stopped at cmp3, I get:
> (gdb) bt
> #0  0x00000000004005d4 in cmp3(char const*) ()
> #1  <function called from gdb>
> #2  0x00000000004005a4 in foo() ()
> #3  0x00000000004005bc in main ()


That's a backtrace in the inferior.  I meant instead for you to set
a breakpoint on gdb's aarch64_push_dummy_call, figure out where the
TYPE_CODE_PTR->TYPE_CODE_INT pointer comes from.  
With "b foo if (int)cmp3("abc") == 1", gdb will do two
infcalls, one malloc call to allocate space for "abc"
string, and then the call to cmp3.

Thanks,
Pedro Alves

> 
>>
>> 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.
> 
> Will do.
> Using this still causes the bug in HEAD, so that’s ok.
> 
> 
>>
>> 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.
> 
> 
> Switching to C causes the bug to vanish.
> This is because C++ uses gnuv3_pass_by_reference(), and the
> segfault happens inside there, whereas C uses
> default_pass_by_reference(), which just returns 0.
> 
> I’ll expand the test to cover both C and C++.