[llvm] 89c4b29 - [GuardWidening] Fix a nasty cast bug in c2eccc6

[Craig Topper via llvm-commits]

I think the flags for nuw/nsw are stored in the subclass data in the
Instruction base class so it shouldn't write any arbitrary data. It could
corrupt the subclass data if it is used for another purpose, but the
sanitizers wound't be able to catch that.

~Craig


On Tue, Jun 7, 2022 at 1:31 PM Philip Reames via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

>
> Author: Philip Reames
> Date: 2022-06-07T13:27:13-07:00
> New Revision: 89c4b29e8d35ec352019d828e546bea3850403df
>
> URL:
> https://github.com/llvm/llvm-project/commit/89c4b29e8d35ec352019d828e546bea3850403df
> DIFF:
> https://github.com/llvm/llvm-project/commit/89c4b29e8d35ec352019d828e546bea3850403df.diff
>
> LOG: [GuardWidening] Fix a nasty cast bug in c2eccc6
>
> c2eccc6 introduced a call to etHasNoUnsignedWrap which implicitly assumes
> that Inst is a OverflowingBinaryOperator.  This is frequently untrue, but
> was not caught because cast<Ty>(X) has been broken, see
> https://discourse.llvm.org/t/cast-x-is-broken-implications-and-proposal-to-address/63033
> for context.
>
> I considered reverting this, but since doing so re-introduces a nasty
> miscompile of its own, I decided to fix forward instead.
>
> I'll note that this is a particularly nasty form of the cast<Ty>(X)
> issue.  Because the cast was succeeding unexpected, we were writing data to
> instructions which weren't OBOs.  This could result in near arbitrary data
> or memory corruption.  I'm a bit shocked that the sanitizers didn't find
> this TBH.
>
> Added:
>
>
> Modified:
>     llvm/lib/Transforms/Scalar/GuardWidening.cpp
>
> Removed:
>
>
>
>
> ################################################################################
> diff  --git a/llvm/lib/Transforms/Scalar/GuardWidening.cpp
> b/llvm/lib/Transforms/Scalar/GuardWidening.cpp
> index 5032f3106d50c..af6062d142f07 100644
> --- a/llvm/lib/Transforms/Scalar/GuardWidening.cpp
> +++ b/llvm/lib/Transforms/Scalar/GuardWidening.cpp
> @@ -495,9 +495,8 @@ void GuardWideningImpl::makeAvailableAt(Value *V,
> Instruction *Loc) const {
>      makeAvailableAt(Op, Loc);
>
>    Inst->moveBefore(Loc);
> -  // If we moved instruction before guard we must clean nuw, nsw flags.
> -  Inst->setHasNoUnsignedWrap(false);
> -  Inst->setHasNoSignedWrap(false);
> +  // If we moved instruction before guard we must clean poison generating
> flags.
> +  Inst->dropPoisonGeneratingFlags();
>  }
>
>  bool GuardWideningImpl::widenCondCommon(Value *Cond0, Value *Cond1,
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220607/d93a0cec/attachment.html>