[Mesa-dev] [PATCH 2/2] freedreno: a2xx: fix crash when freeing context

[Erik Faye-Lund]

On Tue, Jun 26, 2018, 14:00 Rob Clark <robdclark at gmail.com> wrote:

> On Tue, Jun 26, 2018 at 3:32 AM, Erik Faye-Lund <kusmabite at gmail.com>
> wrote:
> > On Wed, Jun 20, 2018 at 3:03 AM Jonathan Marek <jonathan at marek.ca>
> wrote:
> >>
> >> Signed-off-by: Jonathan Marek <jonathan at marek.ca>
> >> ---
> >>  src/gallium/drivers/freedreno/a2xx/fd2_program.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/src/gallium/drivers/freedreno/a2xx/fd2_program.c
> b/src/gallium/drivers/freedreno/a2xx/fd2_program.c
> >> index 9a77457251..834a7c7fcd 100644
> >> --- a/src/gallium/drivers/freedreno/a2xx/fd2_program.c
> >> +++ b/src/gallium/drivers/freedreno/a2xx/fd2_program.c
> >> @@ -54,6 +54,8 @@ create_shader(enum shader_t type)
> >>  static void
> >>  delete_shader(struct fd2_shader_stateobj *so)
> >>  {
> >> +       if (!so)
> >> +               return;
> >>         ir2_shader_destroy(so->ir);
> >>         free(so->tokens);
> >>         free(so->bin);
> >
> > This seems to just put a band-aid on top of some other bug... Wouldn't
> > it be better to lift this out to the call-site instead?
>
> Mmm, then you just end up w/ the null check in two or three different
> spots, instead of one..
>

I meant in the affected call site, not all. Basically, I was trying to hint
that this shouldn't have happened in the first place, and should be fixed
where this unexpectedly becomes null...

>
> I would be a bit curious to see the call-stack of the crash (since I
> haven't seen similar issues with a3xx/a4xx/a5xx), but I think putting
> the null check here is reasonable.
>
> BR,
> -R
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180626/f4919aaa/attachment-0001.html>