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

[Francisco Jerez]

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.

>        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/16acfcd5/attachment.sig>