[Mesa-dev] [PATCH] i965: Do null pointer check before dereferencing vue_prog_data

[Kenneth Graunke]

On Tuesday, February 6, 2018 4:38:00 PM PST Nanley Chery wrote:
> On Mon, Feb 05, 2018 at 04:08:40PM -0800, Anuj Phogat wrote:
> > Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> > ---
> >  src/mesa/drivers/dri/i965/genX_state_upload.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > index aa4d64d08e..67fb328dbc 100644
> > --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> > +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > @@ -3966,7 +3966,8 @@ genX(upload_ds_state)(struct brw_context *brw)
> >             tes_prog_data->domain == BRW_TESS_DOMAIN_TRI;
> >  
> >  #if GEN_GEN >= 8
> > -        if (vue_prog_data->dispatch_mode == DISPATCH_MODE_SIMD8)
> > +        if (vue_prog_data &&
> > +            vue_prog_data->dispatch_mode == DISPATCH_MODE_SIMD8)
> 
> In this else case (where tes_prog_data != NULL) this variable is also
> guaranteed to be non-NULL. They point to the same location in memory as
> far as I can tell.

NAK on this patch.  The if (!tes_prog_data) {...} else { surrounding
this code guarantees that tes_prog_data != NULL, which also means that
vue_prog_data != NULL.

> Though, what I find confusing is that we can simultaneously access
> fields of struct brw_tes_prog_data as well as struct brw_vue_prog_data
> even though neither is a subclass of the other.

The class hierarchy is:

  brw_stage_prog_data -> brw_vue_prog_data -> brw_vs_prog_data
                                           -> brw_tcs_prog_data
                                           -> brw_tes_prog_data
                                           -> brw_gs_prog_data
                      -> brw_wm_prog_data
                      -> brw_cs_prog_data

This is done in the traditional C style of embedding the base object
as the first member in the struct.  So, you can safely cast between
brw_stage_prog_data, brw_vue_prog_data, and brw_tes_prog_data for a
tessellation evaluation shader.

> >             ds.DispatchMode = DISPATCH_MODE_SIMD8_SINGLE_PATCH;
> >          ds.UserClipDistanceCullTestEnableBitmask =
> >              vue_prog_data->cull_distance_mask;
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180207/d1d35784/attachment-0001.sig>