[Mesa-dev] [PATCH] anv/entrypoints: VkGetDeviceProcAddr returns NULL for core instance commands

[Iago Toral]

On Tue, 2018-03-06 at 12:16 +0000, Emil Velikov wrote:
> On 6 March 2018 at 12:09, Iago Toral <itoral at igalia.com> wrote:
> > On Tue, 2018-03-06 at 11:21 +0000, Emil Velikov wrote:
> > > On 6 March 2018 at 09:57, Bas Nieuwenhuizen <basni at chromium.org>
> > > wrote:
> > > > 
> > > > 
> > > > On Tue, Mar 6, 2018 at 8:02 AM, Iago Toral <itoral at igalia.com>
> > > > wrote:
> > > > > 
> > > > > On Mon, 2018-03-05 at 12:11 +0000, Emil Velikov wrote:
> > > > > > Hi Iago,
> > > > > > 
> > > > > > Top level questions:
> > > > > > 
> > > > > > I think this and the original commit should go to stable
> > > > > > right?
> > > > > 
> > > > > I am not sure if this qualifies for stable: these patches
> > > > > don't
> > > > > fix any
> > > > > user-visible bugs. If an application was calling
> > > > > vkGetDeviceProcAddr to
> > > > > get pointers to non-device functions (which is incorrect by
> > > > > the
> > > > > spec)
> > > > > the previous behavior would allow it to get away with it
> > > > > without
> > > > > issues, bit with these patches it will start to crash since
> > > > > it
> > > > > will
> > > > > receive NULL pointers.
> > > > > 
> > > 
> > > According to Lenny's comment in the github issue there's nothing
> > > to
> > > be
> > > concerned. Namely:
> > >  - "The pointers being returned are invalid. ... trying to use
> > > them
> > > will result in a crash."
> > >  - "Wolfenstein was acquiring, but not using the pointers."
> > 
> > Because it is not using the pointers :), if some other app is using
> > them it will start to crash.
> > 
> > But that was not my point, my point was that this doesn't fix
> > anything
> > for users, so I was questioning whether it was material for stable
> > based on that.
> > 
> 
> Surely we don't want to apps be written against the current incorrect
> behaviour?
> Which may go unnoticed since there are no validation/loader checks
> for
> this (based on the github issue).

If there is such an application, and this patch makes it into the
stable release, that application will probably start to crash. That's
the _only_ user-visible change that users can possibly get from
including these patches. The question is: is this okay for a _stable_
release? I figured that in stable releases we only cared about fixing
bugs that affected end-user experience (as in those who use these
applications and can't use them properly because of bugs), and to my
understanding, this doesn't fit in that category, but my vision of this
might be too simplistic...  if you get my point but still believe this
is helpful in stable I don't have any reason to object, just wanted to
make sure that we understand the scenario.

Iago

> But if you feel so strongly about it, fair enough.
> 
> -Emil
>