[LLVMdev] Addressing const reference in ArrayRef

[David Blaikie]

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
>
>