[LLVMdev] Addressing const reference in ArrayRef

[Andreas Weber]

I think David is referring this thread:
http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-July/074909.html

I also tried building with the suggested patch (replacing/modifying the
existing ctor (to take non-const ref) ) and this indeed results in plenty
of build errors. I tried fixing them for a few hours but haven't seen light
at the end of the tunnel.

Best regards,
Andy


2014-08-21 9:38 GMT+02:00 Justin Bogner <mail at justinbogner.com>:

> David Blaikie <dblaikie at gmail.com> writes:
> > I seem to recall discussing this before - is there an existing llvm
> > bug filed, another email thread or something (or perhaps it was just
> > an IRC conversation)? It would be good to keep all the discussion
> > together or at least reference the prior (llvm community) discussion.
>
> I'm not sure if it's been discussed before, but it's led to issues as
> recently as a couple of weeks ago:
>
>
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140804/229557.html
>
> I certainly think it's worthwhile to make this API safer, or at least to
> document the caveats in how it can safely be used.
>
> > Have you tried applying your suggested patch? I assume you meant to
> suggest
> > replacing/modifying the existing ctor (to take non-const ref) rather than
> > adding another?
> >
> > I'd assume that causes a lot of build failures as we probably rely on
> binding
> > temporaries to ArrayRef's in many places correctly (most ArrayRef's are
> > temporaries, not local variables).
> >
> > I think in the previous discussion I suggested we might just want to
> make a
> > practice of treating named/local (not parameter) ArrayRef's with greater
> > suspicion, the same way we do for twine, but I'm not sure.
> >
> > We could move this ctor into a factory function (and/or just make the
> CTor
> > private and friend the makeArrayRef helper for this case) to make it more
> > obvious/easier to find bad call sites. But it would be helpful to have
> the
> > context of the prior discussion to continue from there.
> >
> > On Aug 19, 2014 11:18 PM, "Joey Ye" <joey.ye.cc at gmail.com> wrote:
> >
> >     Analyzing why GCC failed to build LLVM recently, one root cause lies
> >     in definition of ArrayRef:
> >     // ArrayRef.h:
> >     ArrayRef(const T &OneElt) : Data(&OneElt), Length(1) {}
> >
> >     Here address of const reference is taken and stored to an object. It
> >     is believed that live range of const reference is only at the
> function
> >     call site, escaping of its address to an object with a longer live
> >     range is invalid. Referring to the case and discussion here:
> >     https://gcc.gnu.org/ml/gcc/2014-08/msg00173.html
> >
> >     So I would suggest to fix ArrayRef. Adding a non-const version of
> >     constructor should work, but it still leaves the vulnerability in
> >     const version, which I'd expect on people in the community to work
> out
> >     a solution.
> >     ArrayRef(T &OneElt) : Data(&OneElt), Length(1) {}
> >
> >     Thanks,
> >     Joey
> >     _______________________________________________
> >     LLVM Developers mailing list
> >     LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> >     http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> >
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.eduhttp://
> lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140821/5f56cec7/attachment.html>