r338321 - Fix use of uninitialized variable in r338299

[via cfe-commits]

What might be missing is the impl of CreateMemberType:

llvm::DIType *CGDebugInfo::CreateMemberType(llvm::DIFile *Unit, QualType 
FType,
                                             StringRef Name, uint64_t 
*Offset) {
   llvm::DIType *FieldTy = CGDebugInfo::getOrCreateType(FType, Unit);
   uint64_t FieldSize = CGM.getContext().getTypeSize(FType);
   auto FieldAlign = getTypeAlignIfRequired(FType, CGM.getContext());
   llvm::DIType *Ty =
       DBuilder.createMemberType(Unit, Name, Unit, 0, FieldSize, 
FieldAlign,
                                 *Offset, llvm::DINode::FlagZero, 
FieldTy);
   *Offset += FieldSize;
   return Ty;
}

The FieldOffset is advanced twice in the OpenCL case, each time by
getTypeSize(IntTy). I'm not certain why CreateMemberType is not used
for __descriptor in the non-OpenCL case; it looks like it is
because it needs to handle FieldTy in a special way?

Scott

On 2018-07-31 18:03, Eric Christopher wrote:
> I'm probably missing something, from looking here:
> 
>   FieldOffset = 0;
>   if (CGM.getLangOpts().OpenCL) {
>     FType = CGM.getContext().IntTy;
>     EltTys.push_back(CreateMemberType(Unit, FType, "__size",
> &FieldOffset));
>     EltTys.push_back(CreateMemberType(Unit, FType, "__align",
> &FieldOffset));
>   } else {
>     FType = CGM.getContext().getPointerType(CGM.getContext().VoidTy);
>     EltTys.push_back(CreateMemberType(Unit, FType, "__isa",
> &FieldOffset));
>     FType = CGM.getContext().IntTy;
>     EltTys.push_back(CreateMemberType(Unit, FType, "__flags",
> &FieldOffset));
>     EltTys.push_back(CreateMemberType(Unit, FType, "__reserved",
> &FieldOffset));
>     FType = CGM.getContext().getPointerType(Ty->getPointeeType());
>     EltTys.push_back(CreateMemberType(Unit, FType, "__FuncPtr",
> &FieldOffset));
>     FType = CGM.getContext().getPointerType(CGM.getContext().VoidTy);
>     FieldSize = CGM.getContext().getTypeSize(Ty);
>     FieldAlign = CGM.getContext().getTypeAlign(Ty);
>     EltTys.push_back(DBuilder.createMemberType(
>         Unit, "__descriptor", nullptr, LineNo, FieldSize, FieldAlign,
> FieldOffset,
>         llvm::DINode::FlagZero, DescTy));
>     FieldOffset += FieldSize;
>   }
> 
> FieldOffset is only advanced for non opencl blocks. And then just used
> in the type.
> 
> I'll be honest none of that makes any particular sense, but in
> particular these two definitely don't match for the type.
> 
> Thoughts? At any rate if you could document what the intended code is
> here I'd appreciate it.
> 
> -eric
> 
> On Tue, Jul 31, 2018 at 9:18 AM <scott at scottlinder.com> wrote:
> 
>> I think this version is right; the FieldOffset for OpenCL here will
>> be 2
>> * getTypeSize(IntTy). The final `FieldOffset += FieldSize` that was
>> moved only applies to the non-OpenCL "__descriptor" field.
>> 
>> Scott
>> 
>> On 2018-07-30 19:22, Eric Christopher wrote:
>>> Is 0 right for FieldOffset for OpenCL here? Seems a little odd.
>>> 
>>> -eric
>>> 
>>> On Mon, Jul 30, 2018 at 3:56 PM Scott Linder via cfe-commits
>>> <cfe-commits at lists.llvm.org> wrote:
>>> 
>>>> Author: scott.linder
>>>> Date: Mon Jul 30 15:52:07 2018
>>>> New Revision: 338321
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=338321&view=rev [1]
>> [1]
>>>> Log:
>>>> Fix use of uninitialized variable in r338299
>>>> 
>>>> Modified:
>>>> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>>> 
>>>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>>> URL:
>>>> 
>>> 
>> 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=338321&r1=338320&r2=338321&view=diff
>> [2]
>>>> [2]
>>>> 
>>> 
>> 
> ==============================================================================
>>>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>>>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Jul 30 15:52:07
>> 2018
>>>> @@ -989,9 +989,9 @@ llvm::DIType *CGDebugInfo::CreateType(co
>>>> EltTys.push_back(DBuilder.createMemberType(
>>>> Unit, "__descriptor", nullptr, LineNo, FieldSize,
>>>> FieldAlign, FieldOffset,
>>>> llvm::DINode::FlagZero, DescTy));
>>>> + FieldOffset += FieldSize;
>>>> }
>>>> 
>>>> - FieldOffset += FieldSize;
>>>> Elements = DBuilder.getOrCreateArray(EltTys);
>>>> 
>>>> // The __block_literal_generic structs are marked with a special
>>>> 
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [3]
>> [3]
>>> 
>>> 
>>> Links:
>>> ------
>>> [1] http://llvm.org/viewvc/llvm-project?rev=338321&view=rev
>> [4]
>>> [2]
>>> 
>> 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=338321&r1=338320&r2=338321&view=diff
>> [5]
>>> [3] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [3]
> 
> 
> Links:
> ------
> [1] http://llvm.org/viewvc/llvm-project?rev=338321&view=rev
> [2]
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=338321&r1=338320&r2=338321&view=diff
> [3] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> [4] http://llvm.org/viewvc/llvm-project?rev=338321&amp;view=rev
> [5]
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=338321&amp;r1=338320&amp;r2=338321&amp;view=diff