[llvm] r208502 - DwarfUnit: Make explicit a limitation/bug in enumeration constant emission.

[David Blaikie]

On Mon, May 12, 2014 at 10:28 AM, Adrian Prantl <aprantl at apple.com> wrote:
>
>> On May 11, 2014, at 10:04 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> Author: dblaikie
>> Date: Sun May 11 12:04:05 2014
>> New Revision: 208502
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=208502&view=rev
>> Log:
>> DwarfUnit: Make explicit a limitation/bug in enumeration constant emission.
>>
>> Filed as PR19712, LLVM fails to detect the right type of an enum
>> constant when a frontend does not provide an underlying type for the
>> enumeration type.
>>
>> Modified:
>>    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=208502&r1=208501&r2=208502&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Sun May 11 12:04:05 2014
>> @@ -748,12 +748,17 @@ void DwarfUnit::addBlockByrefAddress(con
>> /// Return true if type encoding is unsigned.
>> static bool isUnsignedDIType(DwarfDebug *DD, DIType Ty) {
>>   DIDerivedType DTy(Ty);
>> -  if (DTy.isDerivedType())
>> -    return isUnsignedDIType(DD, DD->resolve(DTy.getTypeDerivedFrom()));
>> +  if (DTy.isDerivedType()) {
>> +    if (DIType Deriv = DD->resolve(DTy.getTypeDerivedFrom()))
>> +      return isUnsignedDIType(DD, Deriv);
>> +    // FIXME: Enums without a fixed underlying type have unknown signedness
>> +    // here, leading to incorrectly emitted constants.
>> +    assert(DTy.getTag() == dwarf::DW_TAG_enumeration_type);
>> +    return false;
>> +  }
>
> This should be relaxed to allow other DIDerivedTypes, including DIComposite types.

What relaxation did you have in mind? "isDerivedType" returns true for
DICompositeTypes too, I believe - so perhaps it's already as relaxed
as you were suggesting it should be?

> I’m imagining a world where SROA does not elide debug info, and a function like this:
>
> struct S { int c; };
> int foo() {
>   struct S s = { 42 };
>   return s.c;
> }
>
> Walking the struct members to find their signedness will be fun!

Yeah... :)

- David

>
> cheers,
> adrian
>>
>>   DIBasicType BTy(Ty);
>> -  if (!BTy.isBasicType())
>> -    return false;
>> +  assert(BTy.isBasicType());
>>   unsigned Encoding = BTy.getEncoding();
>>   assert(Encoding == dwarf::DW_ATE_unsigned ||
>>          Encoding == dwarf::DW_ATE_unsigned_char ||
>
>
>
>