r213790 - PR20228: don't retain a pointer to a vector element after the container has been resized.

[David Blaikie]

On Mon, Jul 28, 2014 at 8:25 AM, David Blaikie <dblaikie at gmail.com> wrote:
> On Mon, Jul 28, 2014 at 12:51 AM, Kostya Serebryany <kcc at google.com> wrote:
>>
>>
>>
>> On Thu, Jul 24, 2014 at 2:25 AM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>>>
>>> On Wed, Jul 23, 2014 at 1:32 PM, Alexey Samsonov <vonosmas at gmail.com>
>>> wrote:
>>>>
>>>> +kcc
>>>>
>>>> I wonder if we could detect it with container-overflow feature in ASan
>>>> and annotated libcxx vector.
>>>
>>>
>>> FWIW, the existing ASan diagnostic was great here, and let me find the bug
>>> with essentially no work (but only triggers in the cases where the vector
>>> actually gets reallocated).
>>
>>
>> The container overflow annotations will not help here -- there is not use of
>> data between begin()+size() and begin()+capacity() here.
>
> *nod* you'd have to pretend that the container had been reallocated
> /every/ time - is there any way to do that? Probably not, as I assume
> ASan depends on pointer identity (not on how a pointer was derived) so
> there's no way to invalidate existing pointers into the buffer and
> only allow ones newly derived from begin()?

Hmm - std::vector couldn't do that anyway, since a user might've
deliberately reserved enough space, so if an operation doesn't cause a
reallocation it /might/ be guaranteed not to reallocate, or it might
not, depending on whether the previous reallocation was explicit or
implicit.

>
>>
>>>
>>>
>>>>
>>>> On Wed, Jul 23, 2014 at 1:07 PM, Richard Smith
>>>> <richard-llvm at metafoo.co.uk> wrote:
>>>>>
>>>>> Author: rsmith
>>>>> Date: Wed Jul 23 15:07:08 2014
>>>>> New Revision: 213790
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=213790&view=rev
>>>>> Log:
>>>>> PR20228: don't retain a pointer to a vector element after the container
>>>>> has been resized.
>>>>>
>>>>> Modified:
>>>>>     cfe/trunk/lib/Sema/SemaExprCXX.cpp
>>>>>     cfe/trunk/test/SemaCXX/type-traits.cpp
>>>>>
>>>>> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=213790&r1=213789&r2=213790&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
>>>>> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Jul 23 15:07:08 2014
>>>>> @@ -3651,12 +3651,13 @@ static bool evaluateTypeTrait(Sema &S, T
>>>>>        if (T->isObjectType() || T->isFunctionType())
>>>>>          T = S.Context.getRValueReferenceType(T);
>>>>>        OpaqueArgExprs.push_back(
>>>>> -        OpaqueValueExpr(Args[I]->getTypeLoc().getLocStart(),
>>>>> +        OpaqueValueExpr(Args[I]->getTypeLoc().getLocStart(),
>>>>>                          T.getNonLValueExprType(S.Context),
>>>>>                          Expr::getValueKindForType(T)));
>>>>> -      ArgExprs.push_back(&OpaqueArgExprs.back());
>>>>>      }
>>>>> -
>>>>> +    for (Expr &E : OpaqueArgExprs)
>>>>> +      ArgExprs.push_back(&E);
>>>>> +
>>>>>      // Perform the initialization in an unevaluated context within a
>>>>> SFINAE
>>>>>      // trap at translation unit scope.
>>>>>      EnterExpressionEvaluationContext Unevaluated(S, Sema::Unevaluated);
>>>>>
>>>>> Modified: cfe/trunk/test/SemaCXX/type-traits.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/type-traits.cpp?rev=213790&r1=213789&r2=213790&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- cfe/trunk/test/SemaCXX/type-traits.cpp (original)
>>>>> +++ cfe/trunk/test/SemaCXX/type-traits.cpp Wed Jul 23 15:07:08 2014
>>>>> @@ -146,6 +146,10 @@ struct ThreeArgCtor {
>>>>>    ThreeArgCtor(int*, char*, int);
>>>>>  };
>>>>>
>>>>> +struct VariadicCtor {
>>>>> +  template<typename...T> VariadicCtor(T...);
>>>>> +};
>>>>> +
>>>>>  void is_pod()
>>>>>  {
>>>>>    { int arr[T(__is_pod(int))]; }
>>>>> @@ -1968,6 +1972,10 @@ void constructible_checks() {
>>>>>    // PR19178
>>>>>    { int arr[F(__is_constructible(Abstract))]; }
>>>>>    { int arr[F(__is_nothrow_constructible(Abstract))]; }
>>>>> +
>>>>> +  // PR20228
>>>>> +  { int arr[T(__is_constructible(VariadicCtor,
>>>>> +                                 int, int, int, int, int, int, int,
>>>>> int, int))]; }
>>>>>  }
>>>>>
>>>>>  // Instantiation of __is_trivially_constructible
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> cfe-commits mailing list
>>>>> cfe-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Alexey Samsonov
>>>> vonosmas at gmail.com
>>>
>>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>