radeonsi: fix out-of-bounds indexing of shader images
authorNicolai Hähnle <nicolai.haehnle@amd.com>
Mon, 21 Mar 2016 19:50:50 +0000 (14:50 -0500)
committerNicolai Hähnle <nicolai.haehnle@amd.com>
Wed, 23 Mar 2016 16:49:53 +0000 (11:49 -0500)
Results are undefined but may not crash. Without this change, out-of-bounds
indexing can lead to VM faults and GPU hangs.

Constant buffers, samplers, and possibly others will eventually need similar
treatment to support GL_ARB_robust_buffer_access_behavior.

Reviewed-by: Edward O'Callaghan <eocallaghan@alterapraxis.com>
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Reviewed-and-Tested-by: Michel Dänzer <michel.daenzer@amd.com>
src/gallium/drivers/radeonsi/si_shader.c

index 9ad2290fd4fe6ddba0e06c1201c7571de18f21b3..1e4bf828ae425a679a1fc290bd5d6d717b83a28b 100644 (file)
@@ -531,6 +531,37 @@ static LLVMValueRef get_indirect_index(struct si_shader_context *ctx,
        return result;
 }
 
+/**
+ * Like get_indirect_index, but restricts the return value to a (possibly
+ * undefined) value inside [0..num).
+ */
+static LLVMValueRef get_bounded_indirect_index(struct si_shader_context *ctx,
+                                              const struct tgsi_ind_register *ind,
+                                              int rel_index, unsigned num)
+{
+       struct gallivm_state *gallivm = &ctx->radeon_bld.gallivm;
+       LLVMBuilderRef builder = gallivm->builder;
+       LLVMValueRef result = get_indirect_index(ctx, ind, rel_index);
+       LLVMValueRef c_max = LLVMConstInt(ctx->i32, num - 1, 0);
+       LLVMValueRef cc;
+
+       if (util_is_power_of_two(num)) {
+               result = LLVMBuildAnd(builder, result, c_max, "");
+       } else {
+               /* In theory, this MAX pattern should result in code that is
+                * as good as the bit-wise AND above.
+                *
+                * In practice, LLVM generates worse code (at the time of
+                * writing), because its value tracking is not strong enough.
+                */
+               cc = LLVMBuildICmp(builder, LLVMIntULE, result, c_max, "");
+               result = LLVMBuildSelect(builder, cc, result, c_max, "");
+       }
+
+       return result;
+}
+
+
 /**
  * Calculate a dword address given an input or output register and a stride.
  */
@@ -2814,7 +2845,18 @@ image_fetch_rsrc(
                LLVMValueRef rsrc_ptr;
                LLVMValueRef tmp;
 
-               ind_index = get_indirect_index(ctx, &image->Indirect, image->Register.Index);
+               /* From the GL_ARB_shader_image_load_store extension spec:
+                *
+                *    If a shader performs an image load, store, or atomic
+                *    operation using an image variable declared as an array,
+                *    and if the index used to select an individual element is
+                *    negative or greater than or equal to the size of the
+                *    array, the results of the operation are undefined but may
+                *    not lead to termination.
+                */
+               ind_index = get_bounded_indirect_index(ctx, &image->Indirect,
+                                                      image->Register.Index,
+                                                      SI_NUM_IMAGES);
 
                rsrc_ptr = LLVMGetParam(ctx->radeon_bld.main_fn, SI_PARAM_IMAGES);
                tmp = build_indexed_load_const(ctx, rsrc_ptr, ind_index);