[Webkit-unassigned] [Bug 196533] [META] Undefined behavior bugs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=196533

--- Comment #9 from Yusuke Suzuki <ysuzuki at apple.com> ---
(In reply to Filip Pizlo from comment #7)
> (In reply to Yusuke Suzuki from comment #6)
> > (In reply to Yusuke Suzuki from comment #5)
> > > (In reply to Filip Pizlo from comment #3)
> > > > The first of those is just not a bug. CPUs we target ignore the high bits of
> > > > a shift amount. This code would only be recompiled if the shift amount ended
> > > > up being a constant.
> > > 
> > > I think the problem of UB is not CPU related thing. CPU behavior is really
> > > nice, and meets our expectation.
> > > Rather, I think the typical UB-related problem is caused because of the C
> > > compiler's assumption "dev never does UB" (clearly, it is wrong).
> > > This assumption introduces restriction on some value's range (like, "you are
> > > doing "v << x", so, x should be [0, 64), and let's use this information to
> > > do further optimizations"), it leads to "aggressively" optimized code, which
> > > does not meet our expected behavior.
> > > One of the issue I remember is that
> > > https://trac.webkit.org/changeset/195906/webkit, GCC leverages our UB
> > > behavior and does "optimizations" which makes B3 broken.
> > > 
> > > My thought on UB is,
> > > 
> > > 1. If we can easily avoid UB, we should do that.
> > 
> > In particular, signed integer overflow, 
> 
> Possible with -fwrapv and no code changes.
> 
> > too large shift amount,
> 
> Sounds like llvm should just fix that.  All CPUs we care about agree on the
> behavior of large shift amounts.  It's dumb that llvm might make the code
> wrong if you do something that the CPU thinks is right.
> 
> > and use of
> > uninitialized value.
> 
> Sounds like llvm should just fix that, too.

Even if we avoid such an optimization in LLVM, we need to do the same things for GCC and VC++, or leave them broken.
If we leave them broken, then, anyway, folks using GCC need UB fix with COMPILER(GCC) / COMPILER(MSVC), and the situation is not changed.

I think the ideal way is fixing C spec.
But, I think fixing the spec takes toooooooooooooooooooooooooooooooooo long time since we need to convince folks who believe the optimizations introduced by these UB are actually useful.
Until the spec is fixed, what we can do is limited. Fix UBs or leave them broken.
Given that UB actually broke B3 previously, I think the relatively safe path for now is fixing UBs.


As we see, UB codes found by UBSAN are not so many. We already avoid many UBs by the other warnings / sanitizers etc.
And for the particularly important 3 cases, signed integer overflow, too large shift amount, uninitialized value, we have very easy fixes: (1) cast to unsigned, (2) mask the shift amount if it can be too large, (3) initialize it.
I don't think this mess the code so much.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190411/7e4ad5a3/attachment-0001.html>