llvm dev - Dec 2012 - [LLVMdev] Value of structure passed byval to a recurse function not initialized when accessed through GDB

Hi David,

I think it might not be exactly PR13303 which might be causing the
corruption of struct when accessed through GDB.
This seems to be an ABI problem in clang.
The problem seems to be that when we have pass by value of struct
(having indirect arguments) stack is not aligned properly.

I tried realigning the stack for indirect arguments in(TargetInfo.cpp) -

ABIArgInfo X86_32ABIInfo::getIndirectResult(QualType Ty, bool ByVal)

.....
 if (StackAlign == 0)
        return ABIArgInfo::getIndirect(4, /*ByVal=*/true,
                                   /*Realign=*/true);      // Do a
realign of stack.

...


This seems to have fixed the issue. Also in case we have a large
structure - e.g. -

typedef struct s
{
  long s;
  long i;
  long l;
  long s1;
  long i1;
  long l1;
} SVAL;	

in the above mentioned code the same issue(corruption of member
variables when accessed through GDB) was observed which has got fixed
after this change.

Need input if this change is correct.

Thanks

On Wed, Dec 5, 2012 at 1:45 AM, David Blaikie <dblaikie at gmail.com>
wrote:
> This seems to be another case of PR13303 - since GDB can't figure out
> where to break for this function based on the debug info (you'll
> notice when you "break recurse" that it's not breaking on a
line or
> source file, just an address) it's breaking at the very start, before
> the prologue
>
> I'm about to commit a fix to this.
>
> On Tue, Dec 4, 2012 at 5:34 AM, Karthik Bhat <karthikthecool at
gmail.com> wrote:
>> Hi All,
>>
>> I was debugging a clang binary when i found this problem. The
>> following code is complied with clang.
>>
>> typedef struct s
>> {
>>   short s;
>> } SVAL;
>>
>>
>> void recurse (SVAL a, int depth)
>> {
>>   a.s = --depth;
>>   if (depth == 0)
>>     return;
>>   else
>>    recurse(a,depth);
>> }
>>
>> int main ()
>> {
>>   SVAL s; s.s = 5;
>>   recurse (s, 5);
>>   return 0;
>> }
>>
>> When i try to access value of a.s in function recurse through gdb(i.e
>> gdb > p a.s) it gives me an uninitialized value.
>> The problem occurs only when we have a function call within function
>> to which we have passed a structure.
>>
>> Could someone guide me were can i look to fix this issue.
>>
>> I have started with LowerFormalArguments in X86ISelLowering.cpp file.
>>
>> Thanks
>> Karthik
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

On Thu, Dec 6, 2012 at 12:33 AM, Karthik Bhat <karthikthecool at
gmail.com> wrote:
> Hi David,
>
> I think it might not be exactly PR13303 which might be causing the
> corruption of struct when accessed through GDB.
> This seems to be an ABI problem in clang.
> The problem seems to be that when we have pass by value of struct
> (having indirect arguments) stack is not aligned properly.
>
> I tried realigning the stack for indirect arguments in(TargetInfo.cpp) -
>
> ABIArgInfo X86_32ABIInfo::getIndirectResult(QualType Ty, bool ByVal)
>
> .....
>  if (StackAlign == 0)
>         return ABIArgInfo::getIndirect(4, /*ByVal=*/true,
>                                    /*Realign=*/true);      // Do a
> realign of stack.
>
> ...
>
>
> This seems to have fixed the issue. Also in case we have a large
> structure - e.g. -
>
> typedef struct s
> {
>   long s;
>   long i;
>   long l;
>   long s1;
>   long i1;
>   long l1;
> } SVAL;
>
> in the above mentioned code the same issue(corruption of member
> variables when accessed through GDB) was observed which has got fixed
> after this change.
>
> Need input if this change is correct.
I haven't looked at this carefully yet (& I'm not the authority on
ABI
issues - not sure who pays most attention to this in the backend, John
McCall deals with it mostly in Clang proper but I've CC'd him here in
case things rings any bells for him) but a simple way you could
provide strong motivation for this change is if you can demonstrate
that this is also a correctness issue: If clang/llvm are really
incorrectly implementing the ABI this should cause interoperability
issues if clang is used to compile a caller and gcc a callee to the
same function (or the other way around).

If you can come up with a simple example that demonstrates that it
should be a fairly unquestionable change. (test cases along with the
change all in a patch file help too)

Thanks,
- David
>
> Thanks
>
> On Wed, Dec 5, 2012 at 1:45 AM, David Blaikie <dblaikie at gmail.com>
wrote:
>> This seems to be another case of PR13303 - since GDB can't figure
out
>> where to break for this function based on the debug info (you'll
>> notice when you "break recurse" that it's not breaking on
a line or
>> source file, just an address) it's breaking at the very start,
before
>> the prologue
>>
>> I'm about to commit a fix to this.
>>
>> On Tue, Dec 4, 2012 at 5:34 AM, Karthik Bhat <karthikthecool at
gmail.com> wrote:
>>> Hi All,
>>>
>>> I was debugging a clang binary when i found this problem. The
>>> following code is complied with clang.
>>>
>>> typedef struct s
>>> {
>>>   short s;
>>> } SVAL;
>>>
>>>
>>> void recurse (SVAL a, int depth)
>>> {
>>>   a.s = --depth;
>>>   if (depth == 0)
>>>     return;
>>>   else
>>>    recurse(a,depth);
>>> }
>>>
>>> int main ()
>>> {
>>>   SVAL s; s.s = 5;
>>>   recurse (s, 5);
>>>   return 0;
>>> }
>>>
>>> When i try to access value of a.s in function recurse through
gdb(i.e
>>> gdb > p a.s) it gives me an uninitialized value.
>>> The problem occurs only when we have a function call within
function
>>> to which we have passed a structure.
>>>
>>> Could someone guide me were can i look to fix this issue.
>>>
>>> I have started with LowerFormalArguments in X86ISelLowering.cpp
file.
>>>
>>> Thanks
>>> Karthik
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

Hi,
There are few clang test cases which address this issue but are not
fixed. E.g. clang/test/CodeGen/x86_32-arguments-linux.c ,
tools/clang/test/CodeGen/x86_32-arguments-darwin.c etc have mentioned
them with a FIXME: note.

Will see if i can come up with some other examples and data.

Thanks

On Thu, Dec 6, 2012 at 10:55 PM, David Blaikie <dblaikie at gmail.com>
wrote:
> On Thu, Dec 6, 2012 at 12:33 AM, Karthik Bhat <karthikthecool at
gmail.com> wrote:
>> Hi David,
>>
>> I think it might not be exactly PR13303 which might be causing the
>> corruption of struct when accessed through GDB.
>> This seems to be an ABI problem in clang.
>> The problem seems to be that when we have pass by value of struct
>> (having indirect arguments) stack is not aligned properly.
>>
>> I tried realigning the stack for indirect arguments in(TargetInfo.cpp)
-
>>
>> ABIArgInfo X86_32ABIInfo::getIndirectResult(QualType Ty, bool ByVal)
>>
>> .....
>>  if (StackAlign == 0)
>>         return ABIArgInfo::getIndirect(4, /*ByVal=*/true,
>>                                    /*Realign=*/true);      // Do a
>> realign of stack.
>>
>> ...
>>
>>
>> This seems to have fixed the issue. Also in case we have a large
>> structure - e.g. -
>>
>> typedef struct s
>> {
>>   long s;
>>   long i;
>>   long l;
>>   long s1;
>>   long i1;
>>   long l1;
>> } SVAL;
>>
>> in the above mentioned code the same issue(corruption of member
>> variables when accessed through GDB) was observed which has got fixed
>> after this change.
>>
>> Need input if this change is correct.
>
> I haven't looked at this carefully yet (& I'm not the authority
on ABI
> issues - not sure who pays most attention to this in the backend, John
> McCall deals with it mostly in Clang proper but I've CC'd him here
in
> case things rings any bells for him) but a simple way you could
> provide strong motivation for this change is if you can demonstrate
> that this is also a correctness issue: If clang/llvm are really
> incorrectly implementing the ABI this should cause interoperability
> issues if clang is used to compile a caller and gcc a callee to the
> same function (or the other way around).
>
> If you can come up with a simple example that demonstrates that it
> should be a fairly unquestionable change. (test cases along with the
> change all in a patch file help too)
>
> Thanks,
> - David
>
>>
>> Thanks
>>
>> On Wed, Dec 5, 2012 at 1:45 AM, David Blaikie <dblaikie at
gmail.com> wrote:
>>> This seems to be another case of PR13303 - since GDB can't
figure out
>>> where to break for this function based on the debug info
(you'll
>>> notice when you "break recurse" that it's not
breaking on a line or
>>> source file, just an address) it's breaking at the very start,
before
>>> the prologue
>>>
>>> I'm about to commit a fix to this.
>>>
>>> On Tue, Dec 4, 2012 at 5:34 AM, Karthik Bhat <karthikthecool at
gmail.com> wrote:
>>>> Hi All,
>>>>
>>>> I was debugging a clang binary when i found this problem. The
>>>> following code is complied with clang.
>>>>
>>>> typedef struct s
>>>> {
>>>>   short s;
>>>> } SVAL;
>>>>
>>>>
>>>> void recurse (SVAL a, int depth)
>>>> {
>>>>   a.s = --depth;
>>>>   if (depth == 0)
>>>>     return;
>>>>   else
>>>>    recurse(a,depth);
>>>> }
>>>>
>>>> int main ()
>>>> {
>>>>   SVAL s; s.s = 5;
>>>>   recurse (s, 5);
>>>>   return 0;
>>>> }
>>>>
>>>> When i try to access value of a.s in function recurse through
gdb(i.e
>>>> gdb > p a.s) it gives me an uninitialized value.
>>>> The problem occurs only when we have a function call within
function
>>>> to which we have passed a structure.
>>>>
>>>> Could someone guide me were can i look to fix this issue.
>>>>
>>>> I have started with LowerFormalArguments in X86ISelLowering.cpp
file.
>>>>
>>>> Thanks
>>>> Karthik
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev