[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

Vedant Kumar via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 3 13:57:32 PST 2017

vsk marked an inline comment as done.
vsk added a comment.

In https://reviews.llvm.org/D29369#665878, @filcab wrote:

> Why the switch to `if` instead of a fully-covered switch/case?

It lets me avoid repeating two function calls:

  switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
  case LangOptions::SOB_Defined:
    return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul");
  case LangOptions::SOB_Undefined:
    if (CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow) &&
        !CanElideOverflowCheck(CGF.getContext(), Ops))
      return EmitOverflowCheckedBinOp(Ops);
    return Builder.CreateNSWMul(Ops.LHS, Ops.RHS, "mul");
  case LangOptions::SOB_Trapping:
    if (CanElideOverflowCheck(CGF.getContext(), Ops))
      return Builder.CreateNSWMul(Ops.LHS, Ops.RHS, "mul");
    return EmitOverflowCheckedBinOp(Ops);

Comment at: test/CodeGen/ubsan-promoted-arith.cpp:56
+// Note: -SHRT_MIN * -SHRT_MIN can overflow.
filcab wrote:
> Nit: Maybe `USHRT_MAX * USHRT_MAX` is more understandable?
Yes, I'll fix this.

Comment at: test/CodeGen/ubsan-promoted-arith.cpp:99
+// CHECK-LABEL: define signext i8 @_Z4rem3
+// rdar30301609: ubsan_handle_divrem_overflow
+char rem3(int i, char c) { return i % c; }
filcab wrote:
> Maybe put the rdar ID next to the FIXME? Like this it looks like you might have written that instead of `CHECK` by mistake.
I placed the rdar ID here to communicate that the line should become a CHECK line once the bug is fixed. It will go away with D29437.


More information about the cfe-commits mailing list