On Wed, Nov 22, 2017 at 10:15 PM, Ian Romanick <i...@freedesktop.org> wrote:
> On 11/21/2017 10:01 AM, Marek Olšák wrote:
>> From: Marek Olšák <marek.ol...@amd.com>
>>
>> ---
>> src/mesa/drivers/common/meta.c | 18 +++---
>> src/mesa/drivers/dri/i915/i830_texblend.c | 3 +-
>> src/mesa/drivers/dri/nouveau/nouveau_util.h | 2 +-
>> src/mesa/drivers/dri/nouveau/nv04_context.c | 14 +++--
>> src/mesa/drivers/dri/nouveau/nv04_state_frag.c | 6 +-
>> src/mesa/drivers/dri/nouveau/nv10_state_frag.c | 4 +-
>> src/mesa/drivers/dri/nouveau/nv10_state_tex.c | 5 +-
>> src/mesa/drivers/dri/nouveau/nv20_state_tex.c | 3 +-
>> src/mesa/drivers/dri/r200/r200_tex.c | 3 +-
>> src/mesa/drivers/dri/r200/r200_texstate.c | 31 +++++----
>> src/mesa/drivers/dri/radeon/radeon_maos_verts.c | 2 +-
>> src/mesa/drivers/dri/radeon/radeon_state.c | 2 +-
>> src/mesa/drivers/dri/radeon/radeon_tex.c | 3 +-
>> src/mesa/drivers/dri/radeon/radeon_texstate.c | 13 ++--
>> src/mesa/main/attrib.c | 17 ++---
>> src/mesa/main/context.c | 6 +-
>> src/mesa/main/enable.c | 23 ++++---
>> src/mesa/main/ff_fragment_shader.cpp | 3 +-
>> src/mesa/main/ffvertex_prog.c | 5 +-
>> src/mesa/main/get.c | 7 ++-
>> src/mesa/main/get_hash_params.py | 10 +--
>> src/mesa/main/mtypes.h | 44 +++++++------
>> src/mesa/main/rastpos.c | 5 +-
>> src/mesa/main/texenv.c | 40 +++++++-----
>> src/mesa/main/texgen.c | 18 +++---
>> src/mesa/main/texstate.c | 83
>> ++++++++++++++-----------
>> src/mesa/main/texstate.h | 15 +++++
>> src/mesa/program/prog_statevars.c | 20 +++---
>> src/mesa/swrast/s_context.c | 2 +-
>> src/mesa/swrast/s_texcombine.c | 3 +-
>> src/mesa/swrast/s_triangle.c | 10 +--
>> src/mesa/tnl/t_vb_texgen.c | 10 +--
>> 32 files changed, 253 insertions(+), 177 deletions(-)
>>
>
> [snip]
>
>> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
>> index 83838d8..cc18ac1 100644
>> --- a/src/mesa/main/get.c
>> +++ b/src/mesa/main/get.c
>> @@ -1357,21 +1357,20 @@ static const struct value_desc error_value =
>> *
>> * \return the struct value_desc corresponding to the enum or a struct
>> * value_desc of TYPE_INVALID if not found. This lets the calling
>> * glGet*v() function jump right into a switch statement and
>> * handle errors there instead of having to check for NULL.
>> */
>> static const struct value_desc *
>> find_value(const char *func, GLenum pname, void **p, union value *v)
>> {
>> GET_CURRENT_CONTEXT(ctx);
>> - struct gl_texture_unit *unit;
>> int mask, hash;
>> const struct value_desc *d;
>> int api;
>>
>> api = ctx->API;
>> /* We index into the table_set[] list of per-API hash tables using the
>> API's
>> * value in the gl_api enum. Since GLES 3 doesn't have an API_OPENGL*
>> enum
>> * value since it's compatible with GLES2 its entry in table_set[] is at
>> the
>> * end.
>> */
>> @@ -1412,22 +1411,24 @@ find_value(const char *func, GLenum pname, void **p,
>> union value *v)
>> case LOC_BUFFER:
>> *p = ((char *) ctx->DrawBuffer + d->offset);
>> return d;
>> case LOC_CONTEXT:
>> *p = ((char *) ctx + d->offset);
>> return d;
>> case LOC_ARRAY:
>> *p = ((char *) ctx->Array.VAO + d->offset);
>> return d;
>> case LOC_TEXUNIT:
>> - unit = &ctx->Texture.Unit[ctx->Texture.CurrentUnit];
>> - *p = ((char *) unit + d->offset);
>> + if (ctx->Texture.CurrentUnit <
>> ARRAY_SIZE(ctx->Texture.FixedFuncUnit)) {
>> + unsigned index = ctx->Texture.CurrentUnit;
>> + *p = ((char *)&ctx->Texture.FixedFuncUnit[index] + d->offset);
>> + }
>
> Presumably this is an error? Looking at the surrounding code, it's not
> obvious that the error is signaled. If we hit this patch, nothing ever
> initializes *p, and _mesa_GetIntegerv, for example, would dereference
> random junk.
>
> If the error was previously signaled, then execution shouldn't even get
> here in the ctx->Texture.CurrentUnit >=
> ARRAY_SIZE(ctx->Texture.FixedFuncUnit) case, right?
This is only used by GL_TEXTURE_GEN_* queries. It seems to be a
discrepancy in the GL spec. It's allowed to set and get the gen enables
for 192 combined texture units, but only 8 are usable in practice.
Reporting an error would be out-of-spec, returning garbage (like here)
is also out-of-spec, but it's an unusable state and therefore wasted memory.
If ARRAY_SIZE(ctx->Texture.FixedFuncUnit) ==
MAX_COMBINED_TEXTURE_IMAGE_UNITS, the condition is always true.
If ARRAY_SIZE(ctx->Texture.FixedFuncUnit) == MAX_TEXTURE_COORD_UNITS,
GetIntegerv returns garbage if active texture >= 8;
Marek
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
