daltenty accepted this revision.
daltenty added a comment.
This revision is now accepted and ready to land.
LGTM as a workaround.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2389
if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
- if (F->getPointerAlignment(getDataLayout()) < 2 && isa<CXXMethodDecl>(D))
+ if (isa<CXXMethodDecl>(D) && F->getPointerAlignment(getDataLayout()) < 2)
F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));
----------------
v.g.vassilev wrote:
> daltenty wrote:
> > Thanks for looking into this.
> >
> > It's not clear to me how this re-ordering ends up fixing things. Can you
> > clarify what the uninitialized value was in this expression?
> >
> >
> The issue happens only in Release builds (RelWithDebInfo, too). From what I
> was able to see it is somewhere in `F->getPointerAlignment`. My assumption
> was that we cannot rely on the full properties of `F` to be set unless it is
> the declaration kind we expected (similar to checking if a something is a
> nullptr and then probing its members). Secondly the `isa` check is likely to
> be the less expensive check anyway.
>
> I saw the issue yesterday and dug into it for a while. However, I decided to
> insert a "fix" before the release which is in few days since the `isa` seems
> to the faster check anyway.
Thanks, yeah thats kind of what I expected. `F->getPointerAlignment()` is
likely getting inlined into in to this callsite and we are inspecting
uninitialized properties of the DataLayout. The weird part is I don't see why
those properties of the DataLayout ever should be uninitialized, so I think
there might be something more broken underneath this.
That said, this is definitely better than before as you say, so let's go ahead
with this for the release and maybe I'll do some more digging in
`getPointerAlignment`.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D159339/new/
https://reviews.llvm.org/D159339
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
