[LLVMdev] Addressing const reference in ArrayRef

[Joey Ye]

Just applied and it didn't work. I'm fresh to LLVM so not sure if it
is built correct. Anyway, if the issue is known I'll leave people from
this community to solve it.

Thanks,
Joey

On Wed, Aug 20, 2014 at 11:28 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 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.
>
> 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