[Mesa-dev] Zero allocation of C++ classes.

Francisco Jerez currojerez at riseup.net
Fri Sep 20 21:51:38 PDT 2013


This is a follow-up patch series on the issue we were discussing in
the ARB_shader_atomic_counters thread, it's based on master with Ken's
recent patch series that defines macros for implementing ralloc-based
new and delete operators.

I've done a closer analysis on the classes we used to allocate with
rzalloc, and I think I've found all cases where a class constructor
leaves member variables uninitialized.  This is a real problem because
classes that rely on the allocator to initialize members to zero for
them cannot be allocated in the stack, they cannot be aggregated into
other objects safely, and they cannot be allocated using array new or
the normal variant of new -- And there's no clear indication or
comment that it doesn't work until the program starts reading
uninitialized memory.

In the current situation, the fact that a few classes use rzalloc
instead of ralloc doesn't save anyone extending a class with new
member variables from the responsibility of making sure whether the
class that's being extended uses the ralloc or the rzalloc convention,
as it's been quite inconsistent until now which uses which.

I think it's worrying to get used to the latter, because you're twice
as likely to make a mistake when you come across the former
convention, because it makes your classes less flexible on how they
can be allocated, and because it also makes it more likely for the
users of your class to make a mistake.

If anyone feels strongly against any of these patches, let's change
that particular case to use memset(0) from the class constructor, or
let's make its constructor private and use a factory method that
handles allocation and construction together, so there's no
possibility for them to be allocated incorrectly.

I've been running piglit on valgrind for a few hours already and I
haven't found any regression from this series.  I'll let it run
overnight anyway, just in case I've missed something.

I hope we can reach some sort of consensus on this.  [The last patch
could probably be squashed into Ken's series in that case.]

[1] http://lists.freedesktop.org/archives/mesa-dev/2013-September/044900.html

[PATCH 01/11] glsl: Initialize all member variables of _mesa_glsl_parse_state on construction.
[PATCH 02/11] i965: Initialize all member variables of vec4_instruction on construction.
[PATCH 03/11] glsl: Switch ast_node to the non-zeroing allocator.
[PATCH 04/11] glsl: Switch ast_type_qualifier to the non-zeroing allocator.
[PATCH 05/11] i965: Initialize all member variables of bblock_t on construction.
[PATCH 06/11] i965: Initialize all member variables of cfg_t on construction.
[PATCH 07/11] i965: Switch ip_record to the non-zeroing allocator.
[PATCH 08/11] i965: Switch fs_inst to the non-zeroing allocator.
[PATCH 09/11] i965: Switch fs_live_variables to the non-zeroing allocator.
[PATCH 10/11] i965: Switch vec4_live_variables to the non-zeroing allocator.
[PATCH 11/11] ralloc: Remove the rzalloc-based new/delete operator definition macro.


More information about the mesa-dev mailing list