Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com>
On Tue, Apr 16, 2013 at 1:37 PM, Paul Berry <stereotype...@gmail.com> wrote: > Fixes issue identified by Klocwork analysis: > > 'attribute_map' array elements might be used uninitialized in this > function (vec4_visitor::lower_attributes_to_hw_regs). > > The attribute_map array contains the mapping from shader input > attributes to the hardware registers they are stored in. > vec4_vs_visitor::setup_attributes() only populates elements of this > array which, according to core Mesa, are actually used by the shader. > Therefore, when vec4_visitor::lower_attributes_to_hw_regs() accesses > the array to lower a register access in the shader, it should in > principle only access elements of attribute_map that contain valid > data. However, if a bug ever caused the driver back-end to access an > input that was not flagged as used by core Mesa, then > lower_attributes_to_hw_regs() would access uninitialized memory, which > could cause illegal instructions to get generated, resulting in a > possible GPU hang. > > This patch makes the situation more robust by using memset() to > pre-initialize the attribute_map array to zero, so that if such a bug > ever occurred, lower_attributes_to_hw_regs() would generate a (mostly) > harmless access to r0. In addition, it adds assertions to > lower_attributes_to_hw_regs() so that if we do have such a bug, we're > likely to discover it quickly. > --- > src/mesa/drivers/dri/i965/brw_vec4.cpp | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp > b/src/mesa/drivers/dri/i965/brw_vec4.cpp > index b0527c7..0afff6f 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp > @@ -1197,6 +1197,11 @@ vec4_visitor::lower_attributes_to_hw_regs(const int > *attribute_map) > if (inst->dst.file == ATTR) { > int grf = attribute_map[inst->dst.reg + inst->dst.reg_offset]; > > + /* All attributes used in the shader need to have been assigned a > + * hardware register by the caller > + */ > + assert(grf != 0); > + > struct brw_reg reg = brw_vec8_grf(grf, 0); > reg.type = inst->dst.type; > reg.dw1.bits.writemask = inst->dst.writemask; > @@ -1211,6 +1216,11 @@ vec4_visitor::lower_attributes_to_hw_regs(const int > *attribute_map) > > int grf = attribute_map[inst->src[i].reg + inst->src[i].reg_offset]; > > + /* All attributes used in the shader need to have been assigned a > + * hardware register by the caller > + */ > + assert(grf != 0); > + > struct brw_reg reg = brw_vec8_grf(grf, 0); > reg.dw1.bits.swizzle = inst->src[i].swizzle; > reg.type = inst->src[i].type; > @@ -1230,6 +1240,7 @@ vec4_vs_visitor::setup_attributes(int payload_reg) > { > int nr_attributes; > int attribute_map[VERT_ATTRIB_MAX + 1]; > + memset(attribute_map, 0, sizeof(attribute_map)); > > nr_attributes = 0; > for (int i = 0; i < VERT_ATTRIB_MAX; i++) { > -- > 1.8.2.1 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev