[Mesa-dev] [PATCH] ralloc: don't use ralloc_set_destructor() for linear allocations

[Francisco Jerez]

Francisco Jerez <currojerez at riseup.net> writes:

> Brian Paul <brianp at vmware.com> writes:
>
>> With older gcc versions and MSVC we were using _ralloc_destructor() in
>> with the linear allocator.  That led to a failed canary assertion.
>>
>> This patch prevents _ralloc_destructor() from being used in those cases.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98595
>> ---
>>  src/util/ralloc.h | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/util/ralloc.h b/src/util/ralloc.h
>> index 3e2d342..f28d9c4 100644
>> --- a/src/util/ralloc.h
>> +++ b/src/util/ralloc.h
>> @@ -417,7 +417,7 @@ bool ralloc_vasprintf_append(char **str, const char *fmt, va_list args);
>>   *
>>   * which is more idiomatic in C++ than calling ralloc.
>>   */
>> -#define DECLARE_ALLOC_CXX_OPERATORS_TEMPLATE(TYPE, ALLOC_FUNC)           \
>> +#define DECLARE_ALLOC_CXX_OPERATORS_TEMPLATE(TYPE, ALLOC_FUNC, USE_DESTRUCTOR) \
>>  private:                                                                 \
>>     static void _ralloc_destructor(void *p)                               \
>>     {                                                                     \
>> @@ -428,7 +428,7 @@ public:                                                                  \
>>     {                                                                     \
>>        void *p = ALLOC_FUNC(mem_ctx, size);                               \
>>        assert(p != NULL);                                                 \
>> -      if (!HAS_TRIVIAL_DESTRUCTOR(TYPE))                                 \
>> +      if (USE_DESTRUCTOR && !HAS_TRIVIAL_DESTRUCTOR(TYPE))               \
>>           ralloc_set_destructor(p, _ralloc_destructor);                   \
>
> This change will lead to UB if the object doesn't have a trivial
> destructor but USE_DESTRUCTOR is false.  I think this should be
> something along the lines of "static_assert(HAS_TRIVIAL_DESTRUCTOR(TYPE) ||
> USE_DESTRUCTOR)" instead of changing the if condition.
>

Although I guess that wouldn't help you at all with MSVC because
HAS_TRIVIAL_DESTRUCTOR always returns false -- I guess you need to
either fix ralloc_set_destructor to work on linear allocations, or keep
the workaround you applied which means the linear allocator cannot be
used with non-POD types.  If you do the latter a static_assert would be
extremely useful (even if you only enable it on non-MSVC compilers) to
make sure the extra condition doesn't conceal changes with undefined
behavior.

>>        return p;                                                          \
>>     }                                                                     \
>> @@ -445,16 +445,16 @@ public:                                                                  \
>>     }
>>  
>>  #define DECLARE_RALLOC_CXX_OPERATORS(type) \
>> -   DECLARE_ALLOC_CXX_OPERATORS_TEMPLATE(type, ralloc_size)
>> +   DECLARE_ALLOC_CXX_OPERATORS_TEMPLATE(type, ralloc_size, true)
>>  
>>  #define DECLARE_RZALLOC_CXX_OPERATORS(type) \
>> -   DECLARE_ALLOC_CXX_OPERATORS_TEMPLATE(type, rzalloc_size)
>> +   DECLARE_ALLOC_CXX_OPERATORS_TEMPLATE(type, rzalloc_size, true)
>>  
>>  #define DECLARE_LINEAR_ALLOC_CXX_OPERATORS(type) \
>> -   DECLARE_ALLOC_CXX_OPERATORS_TEMPLATE(type, linear_alloc_child)
>> +   DECLARE_ALLOC_CXX_OPERATORS_TEMPLATE(type, linear_alloc_child, false)
>>  
>>  #define DECLARE_LINEAR_ZALLOC_CXX_OPERATORS(type) \
>> -   DECLARE_ALLOC_CXX_OPERATORS_TEMPLATE(type, linear_zalloc_child)
>> +   DECLARE_ALLOC_CXX_OPERATORS_TEMPLATE(type, linear_zalloc_child, false)
>>  
>>  
>>  /**
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161109/8939a3b1/attachment.sig>