[LLVMdev] Addressing const reference in ArrayRef

[Reid Kleckner]

Is there some way we can get lifetime extension of temporaries to kick in
here?


On Thu, Aug 21, 2014 at 8:05 AM, David Blaikie <dblaikie at gmail.com> wrote:

> Yeah - I suspect there are just too many cases where we use this ctor
> correctly: where the ArrayRef is only a temporary to a function call.
>
> Perhaps this is a problem for which the best solution is a clang-tidy
> checker - though I'm not sure if we have good integration there yet.
>
> (& yes, Andreas, that looks like the previous thread - thanks!)
>
> On Thu, Aug 21, 2014 at 5:09 AM, Andreas Weber
> <andreas.c.weber at gmail.com> wrote:
> > 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
> >
> >
> _______________________________________________
> 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/7f94e7fc/attachment.html>