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. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
