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

[Brian Paul]

On 11/09/2016 12:39 PM, Francisco Jerez wrote:
> 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.

Jose pointed me at the patch from George Kyriazis on Monday.  His patch 
seems to fix things for me so I'll drop this patch.

I'd like to get that patch in ASAP.

-Brian