r338321 - Fix use of uninitialized variable in r338299
Eric Christopher via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 31 16:20:19 PDT 2018
On Tue, Jul 31, 2018 at 3:59 PM <scott at scottlinder.com> wrote:
> 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?
>
>
I'd either forgotten or blocked that the code does that. Also, why on earth.
And probably.
Anyhow, if you don't mind commenting the OpenCL part of the code when you
get a chance to do the rest of the cleanup I'd appreciate it.
-eric
> 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&view=rev
> > [5]
> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=338321&r1=338320&r2=338321&view=diff
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180731/74b5d9dc/attachment.html>
More information about the cfe-commits
mailing list