mirror of
https://github.com/brl/mutter.git
synced 2024-11-28 19:10:43 -05:00
Don't dereference an unitialised pointer in _cogl_container_of
The previous implementation was dereferencing the sample pointer in order to get the offset to subtract from the member pointer. The resulting value is then only used to get a pointer to the member in order to calculate the offset so it doesn't actually read from the memory location and shouldn't cause any problems. However this is probably technically invalid and could have undefined behaviour. It looks like clang takes advantage of this undefined behaviour and doesn't actually offset the pointer. It also generates a warning when it does this. This patch splits the _cogl_container_of macro into two implementations. Previously the macro was always used in the list iterator macros like this: SomeType *sample = _cogl_container_of(list_node, sample, link) Instead of doing that there is now a new macro called _cogl_list_set_iterator which explicitly assigns to the sample pointer with an initial value before assigning to it again with the real offset. This redundant initialisation gets optimised out by compiler. The second macro is still called _cogl_container_of but instead of taking a sample pointer it just directly takes the type name. That way it can use the standard offsetof macro. https://bugzilla.gnome.org/show_bug.cgi?id=723530 Reviewed-by: Robert Bragg <robert@linux.intel.com> (cherry picked from commit 1efed1e0a2bce706eb4901979ed4e717bb13e4e2)
This commit is contained in:
parent
356f1725f3
commit
7bf0fe9df8
@ -215,7 +215,9 @@ _cogl_fence_cancel_fences_for_framebuffer (CoglFramebuffer *framebuffer)
|
|||||||
|
|
||||||
while (!_cogl_list_empty (&journal->pending_fences))
|
while (!_cogl_list_empty (&journal->pending_fences))
|
||||||
{
|
{
|
||||||
fence = _cogl_container_of (journal->pending_fences.next, fence, link);
|
fence = _cogl_container_of (journal->pending_fences.next,
|
||||||
|
CoglFenceClosure,
|
||||||
|
link);
|
||||||
cogl_framebuffer_cancel_fence_callback (framebuffer, fence);
|
cogl_framebuffer_cancel_fence_callback (framebuffer, fence);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1520,7 +1520,7 @@ _cogl_gles2_context_free (CoglGLES2Context *gles2_context)
|
|||||||
{
|
{
|
||||||
CoglGLES2Offscreen *gles2_offscreen =
|
CoglGLES2Offscreen *gles2_offscreen =
|
||||||
_cogl_container_of (gles2_context->foreign_offscreens.next,
|
_cogl_container_of (gles2_context->foreign_offscreens.next,
|
||||||
gles2_offscreen,
|
CoglGLES2Offscreen,
|
||||||
link);
|
link);
|
||||||
|
|
||||||
/* Note: this will also indirectly free the gles2_offscreen by
|
/* Note: this will also indirectly free the gles2_offscreen by
|
||||||
|
@ -26,6 +26,8 @@
|
|||||||
#ifndef COGL_LIST_H
|
#ifndef COGL_LIST_H
|
||||||
#define COGL_LIST_H
|
#define COGL_LIST_H
|
||||||
|
|
||||||
|
#include <stddef.h>
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* CoglList - linked list
|
* CoglList - linked list
|
||||||
*
|
*
|
||||||
@ -84,40 +86,44 @@ void
|
|||||||
_cogl_list_insert_list (CoglList *list,
|
_cogl_list_insert_list (CoglList *list,
|
||||||
CoglList *other);
|
CoglList *other);
|
||||||
|
|
||||||
#ifdef __GNUC__
|
/* This assigns to iterator first so that taking a reference to it
|
||||||
#define _cogl_container_of(ptr, sample, member) \
|
* later in the second step won't be an undefined operation. It
|
||||||
(__typeof__(sample))((char *)(ptr) - \
|
* assigns the value of list_node rather than 0 so that it is possible
|
||||||
((char *)&(sample)->member - (char *)(sample)))
|
* have list_node be based on the previous value of iterator. In that
|
||||||
#else
|
* respect iterator is just used as a convenient temporary variable.
|
||||||
#define _cogl_container_of(ptr, sample, member) \
|
* The compiler optimises all of this down to a single subtraction by
|
||||||
(void *)((char *)(ptr) - \
|
* a constant */
|
||||||
((char *)&(sample)->member - (char *)(sample)))
|
#define _cogl_list_set_iterator(list_node, iterator, member) \
|
||||||
#endif
|
((iterator) = (void *) (list_node), \
|
||||||
|
(iterator) = (void *) ((char *) (iterator) - \
|
||||||
|
(((char *) &(iterator)->member) - \
|
||||||
|
(char *) (iterator))))
|
||||||
|
|
||||||
|
#define _cogl_container_of(ptr, type, member) \
|
||||||
|
(type *) ((char *) (ptr) - offsetof (type, member))
|
||||||
|
|
||||||
#define _cogl_list_for_each(pos, head, member) \
|
#define _cogl_list_for_each(pos, head, member) \
|
||||||
for (pos = 0, pos = _cogl_container_of((head)->next, pos, member); \
|
for (_cogl_list_set_iterator ((head)->next, pos, member); \
|
||||||
&pos->member != (head); \
|
&pos->member != (head); \
|
||||||
pos = _cogl_container_of(pos->member.next, pos, member))
|
_cogl_list_set_iterator (pos->member.next, pos, member))
|
||||||
|
|
||||||
#define _cogl_list_for_each_safe(pos, tmp, head, member) \
|
#define _cogl_list_for_each_safe(pos, tmp, head, member) \
|
||||||
for (pos = 0, tmp = 0, \
|
for (_cogl_list_set_iterator ((head)->next, pos, member), \
|
||||||
pos = _cogl_container_of((head)->next, pos, member), \
|
_cogl_list_set_iterator ((pos)->member.next, tmp, member); \
|
||||||
tmp = _cogl_container_of((pos)->member.next, tmp, member); \
|
|
||||||
&pos->member != (head); \
|
&pos->member != (head); \
|
||||||
pos = tmp, \
|
pos = tmp, \
|
||||||
tmp = _cogl_container_of(pos->member.next, tmp, member))
|
_cogl_list_set_iterator (pos->member.next, tmp, member))
|
||||||
|
|
||||||
#define _cogl_list_for_each_reverse(pos, head, member) \
|
#define _cogl_list_for_each_reverse(pos, head, member) \
|
||||||
for (pos = 0, pos = _cogl_container_of((head)->prev, pos, member); \
|
for (_cogl_list_set_iterator ((head)->prev, pos, member); \
|
||||||
&pos->member != (head); \
|
&pos->member != (head); \
|
||||||
pos = _cogl_container_of(pos->member.prev, pos, member))
|
_cogl_list_set_iterator (pos->member.prev, pos, member))
|
||||||
|
|
||||||
#define _cogl_list_for_each_reverse_safe(pos, tmp, head, member) \
|
#define _cogl_list_for_each_reverse_safe(pos, tmp, head, member) \
|
||||||
for (pos = 0, tmp = 0, \
|
for (_cogl_list_set_iterator ((head)->prev, pos, member), \
|
||||||
pos = _cogl_container_of((head)->prev, pos, member), \
|
_cogl_list_set_iterator ((pos)->member.prev, tmp, member); \
|
||||||
tmp = _cogl_container_of((pos)->member.prev, tmp, member); \
|
|
||||||
&pos->member != (head); \
|
&pos->member != (head); \
|
||||||
pos = tmp, \
|
pos = tmp, \
|
||||||
tmp = _cogl_container_of(pos->member.prev, tmp, member))
|
_cogl_list_set_iterator (pos->member.prev, tmp, member))
|
||||||
|
|
||||||
#endif /* COGL_LIST_H */
|
#endif /* COGL_LIST_H */
|
||||||
|
@ -124,9 +124,9 @@ _cogl_memory_stack_alloc (CoglMemoryStack *stack, size_t bytes)
|
|||||||
* is made then we may need to skip over one or more of the
|
* is made then we may need to skip over one or more of the
|
||||||
* sub-stacks that are too small for the requested allocation
|
* sub-stacks that are too small for the requested allocation
|
||||||
* size... */
|
* size... */
|
||||||
for (sub_stack = _cogl_container_of (sub_stack->link.next, sub_stack, link);
|
for (_cogl_list_set_iterator (sub_stack->link.next, sub_stack, link);
|
||||||
&sub_stack->link != &stack->sub_stacks;
|
&sub_stack->link != &stack->sub_stacks;
|
||||||
sub_stack = _cogl_container_of (sub_stack->link.next, sub_stack, link))
|
_cogl_list_set_iterator (sub_stack->link.next, sub_stack, link))
|
||||||
{
|
{
|
||||||
if (sub_stack->bytes >= bytes)
|
if (sub_stack->bytes >= bytes)
|
||||||
{
|
{
|
||||||
@ -143,11 +143,15 @@ _cogl_memory_stack_alloc (CoglMemoryStack *stack, size_t bytes)
|
|||||||
* requested allocation if that's bigger.
|
* requested allocation if that's bigger.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
sub_stack = _cogl_container_of (stack->sub_stacks.prev, sub_stack, link);
|
sub_stack = _cogl_container_of (stack->sub_stacks.prev,
|
||||||
|
CoglMemorySubStack,
|
||||||
|
link);
|
||||||
|
|
||||||
_cogl_memory_stack_add_sub_stack (stack, MAX (sub_stack->bytes, bytes) * 2);
|
_cogl_memory_stack_add_sub_stack (stack, MAX (sub_stack->bytes, bytes) * 2);
|
||||||
|
|
||||||
sub_stack = _cogl_container_of (stack->sub_stacks.prev, sub_stack, link);
|
sub_stack = _cogl_container_of (stack->sub_stacks.prev,
|
||||||
|
CoglMemorySubStack,
|
||||||
|
link);
|
||||||
|
|
||||||
stack->sub_stack_offset += bytes;
|
stack->sub_stack_offset += bytes;
|
||||||
|
|
||||||
@ -158,7 +162,7 @@ void
|
|||||||
_cogl_memory_stack_rewind (CoglMemoryStack *stack)
|
_cogl_memory_stack_rewind (CoglMemoryStack *stack)
|
||||||
{
|
{
|
||||||
stack->sub_stack = _cogl_container_of (stack->sub_stacks.next,
|
stack->sub_stack = _cogl_container_of (stack->sub_stacks.next,
|
||||||
stack->sub_stack,
|
CoglMemorySubStack,
|
||||||
link);
|
link);
|
||||||
stack->sub_stack_offset = 0;
|
stack->sub_stack_offset = 0;
|
||||||
}
|
}
|
||||||
@ -177,7 +181,7 @@ _cogl_memory_stack_free (CoglMemoryStack *stack)
|
|||||||
while (!_cogl_list_empty (&stack->sub_stacks))
|
while (!_cogl_list_empty (&stack->sub_stacks))
|
||||||
{
|
{
|
||||||
CoglMemorySubStack *sub_stack =
|
CoglMemorySubStack *sub_stack =
|
||||||
_cogl_container_of (stack->sub_stacks.next, sub_stack, link);
|
_cogl_container_of (stack->sub_stacks.next, CoglMemorySubStack, link);
|
||||||
_cogl_list_remove (&sub_stack->link);
|
_cogl_list_remove (&sub_stack->link);
|
||||||
_cogl_memory_sub_stack_free (sub_stack);
|
_cogl_memory_sub_stack_free (sub_stack);
|
||||||
}
|
}
|
||||||
|
@ -181,7 +181,9 @@ _cogl_dispatch_onscreen_cb (CoglContext *context)
|
|||||||
while (!_cogl_list_empty (&context->onscreen_dirty_queue))
|
while (!_cogl_list_empty (&context->onscreen_dirty_queue))
|
||||||
{
|
{
|
||||||
CoglOnscreenQueuedDirty *qe =
|
CoglOnscreenQueuedDirty *qe =
|
||||||
_cogl_container_of (context->onscreen_dirty_queue.next, qe, link);
|
_cogl_container_of (context->onscreen_dirty_queue.next,
|
||||||
|
CoglOnscreenQueuedDirty,
|
||||||
|
link);
|
||||||
|
|
||||||
_cogl_list_remove (&qe->link);
|
_cogl_list_remove (&qe->link);
|
||||||
|
|
||||||
|
@ -909,7 +909,7 @@ _cogl_pipeline_fragend_glsl_add_layer (CoglPipeline *pipeline,
|
|||||||
else
|
else
|
||||||
{
|
{
|
||||||
LayerData *first =
|
LayerData *first =
|
||||||
_cogl_container_of (shader_state->layers.next, first, link);
|
_cogl_container_of (shader_state->layers.next, LayerData, link);
|
||||||
layer_data->previous_layer_index = first->layer->index;
|
layer_data->previous_layer_index = first->layer->index;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1018,7 +1018,7 @@ _cogl_pipeline_fragend_glsl_end (CoglPipeline *pipeline,
|
|||||||
LayerData *layer_data, *tmp;
|
LayerData *layer_data, *tmp;
|
||||||
|
|
||||||
layer_data = _cogl_container_of (shader_state->layers.next,
|
layer_data = _cogl_container_of (shader_state->layers.next,
|
||||||
layer_data,
|
LayerData,
|
||||||
link);
|
link);
|
||||||
last_layer = layer_data->layer;
|
last_layer = layer_data->layer;
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user