llvmpipe: Don't use u_ringbuffer for lp_scene_queue

Inline the ring buffer and signal logic into lp_scene_queue instead of
using a u_ringbuffer.  The code ends up simpler since there's no need
to handle serializing data from / to packets.

This fixes a crash when compiling Mesa with LTO, that happened because
of util_ringbuffer_dequeue() was writing data after the "header
packet", as shown below

    struct scene_packet {
       struct util_packet header;
       struct lp_scene *scene;
    };

    /* Snippet of old lp_scene_deque(). */
    packet.scene = NULL;
    ret = util_ringbuffer_dequeue(queue->ring,
                                  &packet.header,
                                  sizeof packet / 4,
    return packet.scene;

but due to the way aliasing analysis work the compiler didn't
considered the "&packet->header" to alias with "packet->scene".  With
the aggressive inlining done by LTO, this would end up always
returning NULL instead of the content read by
util_ringbuffer_dequeue().

Issue found by Marco Simental and iThiago Macieira.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110884
Reviewed-by: Roland Scheidegger <sroland@vmware.com>
This commit is contained in:
Caio Marcelo de Oliveira Filho 2019-06-12 15:32:30 -07:00
parent 390126e70a
commit 397d1a18ef
1 changed files with 48 additions and 36 deletions

View File

@ -32,25 +32,33 @@
* which are produced by the "rast" code when it finishes rendering a scene. * which are produced by the "rast" code when it finishes rendering a scene.
*/ */
#include "util/u_ringbuffer.h" #include "os/os_thread.h"
#include "util/u_memory.h" #include "util/u_memory.h"
#include "lp_scene_queue.h" #include "lp_scene_queue.h"
#include "util/u_math.h"
#define MAX_SCENE_QUEUE 4 #define SCENE_QUEUE_SIZE 4
struct scene_packet {
struct util_packet header;
struct lp_scene *scene;
};
/** /**
* A queue of scenes * A queue of scenes
*/ */
struct lp_scene_queue struct lp_scene_queue
{ {
struct util_ringbuffer *ring; struct lp_scene *scenes[SCENE_QUEUE_SIZE];
mtx_t mutex;
cnd_t change;
/* These values wrap around, so that head == tail means empty. When used
* to index the array, we use them modulo the queue size. This scheme
* works because the queue size is a power of two.
*/
unsigned head;
unsigned tail;
}; };
@ -59,20 +67,19 @@ struct lp_scene_queue
struct lp_scene_queue * struct lp_scene_queue *
lp_scene_queue_create(void) lp_scene_queue_create(void)
{ {
/* Circular queue behavior depends on size being a power of two. */
STATIC_ASSERT(SCENE_QUEUE_SIZE > 0);
STATIC_ASSERT((SCENE_QUEUE_SIZE & (SCENE_QUEUE_SIZE - 1)) == 0);
struct lp_scene_queue *queue = CALLOC_STRUCT(lp_scene_queue); struct lp_scene_queue *queue = CALLOC_STRUCT(lp_scene_queue);
if (!queue) if (!queue)
return NULL; return NULL;
queue->ring = util_ringbuffer_create( MAX_SCENE_QUEUE * (void) mtx_init(&queue->mutex, mtx_plain);
sizeof( struct scene_packet ) / 4); cnd_init(&queue->change);
if (queue->ring == NULL)
goto fail;
return queue; return queue;
fail:
FREE(queue);
return NULL;
} }
@ -80,7 +87,8 @@ fail:
void void
lp_scene_queue_destroy(struct lp_scene_queue *queue) lp_scene_queue_destroy(struct lp_scene_queue *queue)
{ {
util_ringbuffer_destroy(queue->ring); cnd_destroy(&queue->change);
mtx_destroy(&queue->mutex);
FREE(queue); FREE(queue);
} }
@ -89,19 +97,25 @@ lp_scene_queue_destroy(struct lp_scene_queue *queue)
struct lp_scene * struct lp_scene *
lp_scene_dequeue(struct lp_scene_queue *queue, boolean wait) lp_scene_dequeue(struct lp_scene_queue *queue, boolean wait)
{ {
struct scene_packet packet; mtx_lock(&queue->mutex);
enum pipe_error ret;
packet.scene = NULL; if (wait) {
/* Wait for queue to be not empty. */
while (queue->head == queue->tail)
cnd_wait(&queue->change, &queue->mutex);
} else {
if (queue->head == queue->tail) {
mtx_unlock(&queue->mutex);
return NULL;
}
}
ret = util_ringbuffer_dequeue(queue->ring, struct lp_scene *scene = queue->scenes[queue->head++ % SCENE_QUEUE_SIZE];
&packet.header,
sizeof packet / 4,
wait );
if (ret != PIPE_OK)
return NULL;
return packet.scene; cnd_signal(&queue->change);
mtx_unlock(&queue->mutex);
return scene;
} }
@ -109,16 +123,14 @@ lp_scene_dequeue(struct lp_scene_queue *queue, boolean wait)
void void
lp_scene_enqueue(struct lp_scene_queue *queue, struct lp_scene *scene) lp_scene_enqueue(struct lp_scene_queue *queue, struct lp_scene *scene)
{ {
struct scene_packet packet; mtx_lock(&queue->mutex);
packet.header.dwords = sizeof packet / 4; /* Wait for free space. */
packet.header.data24 = 0; while (queue->tail - queue->head >= SCENE_QUEUE_SIZE)
packet.scene = scene; cnd_wait(&queue->change, &queue->mutex);
util_ringbuffer_enqueue(queue->ring, &packet.header); queue->scenes[queue->tail++ % SCENE_QUEUE_SIZE] = scene;
cnd_signal(&queue->change);
mtx_unlock(&queue->mutex);
} }