llvm dev - Feb 2016 - Possible soundness issue with available_externally (split from "RFC: Add guard intrinsics")

On Mon, Feb 29, 2016 at 10:42 AM, Mehdi Amini <mehdi.amini at apple.com>
wrote:
>
> On Feb 29, 2016, at 10:38 AM, Xinliang David Li via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>
> ok thanks.  A more reduced test case can show different behavior between O2
> and O0.
>
> Say we have
>
> unsigned maybe_divide (unsigned *ptr) {
>    int flag = false;
>    unsigned val = 500/ptr[0];
>    if (flag)
>        return val;
>    return (unsigned)(intptr_t)ptr);
> }
>
> int main() {
>    unsigned g = 0;
>     return maybe_divide(&g);
> }
>
>
> At O2, it runs fine, but at O0 it core dumps.
>
>
> I believe this program has a divide by zero and is not correct.
> By luck the optimization removes the faulty instruction, which does not
mean
> the program is well formed.
>
> IMO this is different from Sanjoy's example, where a wrong optimization
> introduces the error.
Yup; in my example there actually is no place where the program
divides by zero.  Both the call sites that call maybe_divide store a
non-zero value to ptr[0] before calling maybe_divide.

-- Sanjoy

On Mon, Feb 29, 2016 at 10:50 AM, Sanjoy Das <sanjoy at
playingwithpointers.com
> wrote:
> On Mon, Feb 29, 2016 at 10:42 AM, Mehdi Amini <mehdi.amini at
apple.com>
> wrote:
> >
> > On Feb 29, 2016, at 10:38 AM, Xinliang David Li via llvm-dev
> > <llvm-dev at lists.llvm.org> wrote:
> >
> > ok thanks.  A more reduced test case can show different behavior
between
> O2
> > and O0.
> >
> > Say we have
> >
> > unsigned maybe_divide (unsigned *ptr) {
> >    int flag = false;
> >    unsigned val = 500/ptr[0];
> >    if (flag)
> >        return val;
> >    return (unsigned)(intptr_t)ptr);
> > }
> >
> > int main() {
> >    unsigned g = 0;
> >     return maybe_divide(&g);
> > }
> >
> >
> > At O2, it runs fine, but at O0 it core dumps.
> >
> >
> > I believe this program has a divide by zero and is not correct.
> > By luck the optimization removes the faulty instruction, which does
not
> mean
> > the program is well formed.
> >
> > IMO this is different from Sanjoy's example, where a wrong
optimization
> > introduces the error.
>
> Yup; in my example there actually is no place where the program
> divides by zero.  Both the call sites that call maybe_divide store a
> non-zero value to ptr[0] before calling maybe_divide.
>
yes -- it is UB if user writes code like this. What if the g=0 is
speculatively moved above the call of maybe_divide at O2?  My point is that
such runtime behavior difference may not specific to multi-TU definition
scenario.

David
>
> -- Sanjoy
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160229/7d7b5dbf/attachment.html>

I may be misunderstanding you here, but I've tried to reply inline
below:

On Mon, Feb 29, 2016 at 11:00 AM, Xinliang David Li
<xinliangli at gmail.com> wrote:
> yes -- it is UB if user writes code like this.
So we agree that given that the user wrote exactly what you have
above, there is no guarantee from the compiler to generate anything
meaningful?
> What if the g=0 is
> speculatively moved above the call of maybe_divide at O2?  My point is that
You mean, the program was initially something like

g = 42
maybe_divide(&g)
g = 0

which LLVM optimized to

g = 0
maybe_divide(&g)

?

That code motion is legal only if `maybe_divide` is `readnone` -- if
LLVM speculates the store without first inferring `readnone` on
`maybe_divide`, that's buggy for a different reason.
> such runtime behavior difference may not specific to multi-TU definition
> scenario.
The multi-TU constraint is relevant here because the linker may
legally replace a `readnone` version of `maybe_divide` with version
that reads memory; retroactively invalidating the optimization above.  If
there is only one TU, then the `maybe_divide` the optimizer sees is
what the final executable gets; and all is well.

-- Sanjoy