Page MenuHomePhabricator

[CGExprScalar] In EmitCompare trunc the result if it has different type as E->getType()
ClosedPublic

Authored by Carrot on Oct 6 2017, 5:48 PM.

Details

Summary

Usually compare expression should return i1 type, so EmitScalarConversion is called before return

return EmitScalarConversion(Result, CGF.getContext().BoolTy, E->getType(), E->getExprLoc());

But when ppc intrinsic is called to compare vectors, the ppc intrinsic can return i32 even E->getType() is BoolTy, in this case EmitScalarConversion does nothing, an i32 type result is returned and causes crash later.

This patch detects this case and truncates the result before return.

Diff Detail

Repository
rL LLVM

Event Timeline

Carrot created this revision.Oct 6 2017, 5:48 PM
Carrot added a comment.Oct 9 2017, 9:14 AM

I worked on a similar bug as 31161, and then found this one, it should be same as in comment7.
What is the current status of the work on that bug?

majnemer added inline comments.
lib/CodeGen/CGExprScalar.cpp
3223–3224 ↗(On Diff #118114)

Is this clang-format'd ?

3224 ↗(On Diff #118114)

You are unconditionally dereferencing the result of a dyn_cast. You are either missing a null-check or this should be a cast<>

3225–3226 ↗(On Diff #118114)

Extra parens.

Carrot updated this revision to Diff 118236.Oct 9 2017, 10:52 AM
Carrot marked 3 inline comments as done.

I worked on a similar bug as 31161, and then found this one, it should be same as in comment7.
What is the current status of the work on that bug?

No one has had time to finalize a fix to it. Please go ahead with this patch. If this patch indeed fixes the bug, please close it.

I worked on a similar bug as 31161, and then found this one, it should be same as in comment7.
What is the current status of the work on that bug?

No one has had time to finalize a fix to it. Please go ahead with this patch. If this patch indeed fixes the bug, please close it.

This patch fixes the second part of bug 31161 described in comment7. I will check it in soon.
For the first part of the bug, I will retest your patch in comment1, plus incorrect handling of type long in our case, and then send it out.

This revision was automatically updated to reflect the committed changes.