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

[Iago Toral]

On Tue, 2018-03-06 at 13:05 +0000, Emil Velikov wrote:
> On 6 March 2018 at 12:26, Iago Toral <itoral at igalia.com> wrote:
> > 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.o
> > > > > rg>
> > > > > wrote:
> > > > > > 
> > > > > > 
> > > > > > On Tue, Mar 6, 2018 at 8:02 AM, Iago Toral <itoral at igalia.c
> > > > > > om>
> > > > > > 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.
> 
> Such an application will crash _regardless_ of the patch, as
> mentioned earlier.
> That is as per Lenny's analysis - haven't checked it personally.

I am not sure why he says that, the point of using vkGet*Addr is to get
a driver function pointer so you can skip the loader completely. If you
get a valid pointer and you pass valid parameters to it, I think it
should work be just fine, but have't tested it myself, I can try to do
that and see what happens.

Iago

> -Emil
>