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

[bugzilla-daemon at webkit.org]

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

--- Comment #14 from Yusuke Suzuki <ysuzuki at apple.com> ---
(In reply to Filip Pizlo from comment #13)
> Consider the case of signed integer overflow.  UBSAN will only find the
> cases where either by folding constants or by the way that the result is
> used, you have an obvious case of UB.  So it's not like running UBSAN and
> fixing the bugs it reports will mean that we are avoiding UBs.  We still
> have them.
> 
> On the other hand, -fwrapv completely eliminates that class of UB.
> 
> Therefore, if we are concerned about leaving UBs, we should use compiler
> flags to disable them.

I think that we use these sanitizers with Debug build with the hope of disabling such optimizations.
Then, we can find such a dangerous constant foldings performed in Release build.
For example, B3 issue was only caused in Release build[1]. I think that issue can be caught by Debug+UBSAN effectively.

But, I strongly agree that removing UBs with the flag is more effective to remove such bugs than removing random bugs found at runtime.

[1]: https://bugs.webkit.org/show_bug.cgi?id=153647#c3

> > > If that is successful, maybe we can just ask for an option in clang to mask
> > > shift amounts.  I could write an llvm pass to do that in an afternoon.
> > 
> > I think this pass should not cause any performance problem in x86.
> > I searched and learned that UBs in shift amount is because of some CPU
> > behavior (PowerPC), and having a flag to align the behavior to x86 behavior
> > does not cause any additional code emission in x86 I think. Maybe, it could
> > emit masking ops for PowerPC, but we don't care the performance on PowerPC.
> 
> Exactly.
> 
> > 
> > Anyway, I think we need a flag for this to explicitly make this type of UBs
> > well-defined.
> > But..., we need to add this flag to GCC and VC++ too to remove UBs.
> 
> If we only added it to llvm then we would use it there.  Most folks who use
> this code will use a version compiled by llvm.
> 
> Since we're not talking about something that would cause top crashes, I'm
> not sure we have to worry too much about minority compilers doing the right
> thing.

If the UB-related issue occurs in GCC and VC++ due to missing flags, folks using these compilers will add some annotation / cast / masking or something like that to avoid the issues.
If it does not matter, I think it is OK.

But now, I'm a bit optimistic than before. Fortunately, all the supported compilers already have a option to remove UBs of signed integer overflow (I did not know /d2UndefIntOverflow on VC++).
I saw many UB-related issues are caused by this UB, and I believe that UB-related issues can decrease a lot if we enable this option.

-- 
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/61919d95/attachment-0001.html>