From f45dd6d31b2ff46a082931386ccd0bf043cfad59 Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Sun, 10 Feb 2019 22:49:20 -0800 Subject: st/mesa: Limit GL_MAX_[NATIVE_]PROGRAM_PARAMETERS_ARB to 2048 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Piglit's vp-max-array test creates a vertex program containing a uniform array sized to the value of GL_MAX_NATIVE_PROGRAM_PARAMETERS_ARB. Mesa will then add additional state-var parameters for things like the MVP matrix. radeonsi currently exposes a value of 4096, derived from constant buffer upload size. This means the array will have 4096 elements, and the extra MVP state-vars would get a prog_src_register::Index of over 4096. Unfortunately, prog_src_register::Index is a signed 13-bit integer, so values beyond 4096 end up turning into negative numbers. Negative source indexes are only valid for relative addressing, so this ends up generating illegal IR. In prog_to_nir, this would cause an out of bounds array access. st_mesa_to_tgsi checks for a negative value, assumes it's bogus, and remaps it to parameter 0 in order to get something in-range. This isn't right - instead of reading the MVP matrix, it would read the first element of the vertex program's large array. But the test only checks that the program compiles, so we never noticed that it was broken. This patch limits the size of the program limits, with the understanding that we may need to generate additional state-vars internally. i965 has exposed 1024 for this limit for years, so I don't expect lowering it to 2048 will cause any practical problems for radeonsi or other drivers. Fixes vp-max-array with prog_to_nir.c. Cc: "19.0" Reviewed-by: Marek Olšák Reviewed-by: Eric Anholt --- src/mesa/state_tracker/st_extensions.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/mesa/state_tracker/st_extensions.c b/src/mesa/state_tracker/st_extensions.c index 2dbda83ca10..d2660099fc1 100644 --- a/src/mesa/state_tracker/st_extensions.c +++ b/src/mesa/state_tracker/st_extensions.c @@ -223,8 +223,13 @@ void st_init_limits(struct pipe_screen *screen, pc->MaxUniformComponents = MIN2(pc->MaxUniformComponents, MAX_UNIFORMS * 4); + /* For ARB programs, prog_src_register::Index is a signed 13-bit number. + * This gives us a limit of 4096 values - but we may need to generate + * internal values in addition to what the source program uses. So, we + * drop the limit one step lower, to 2048, to be safe. + */ pc->MaxParameters = - pc->MaxNativeParameters = pc->MaxUniformComponents / 4; + pc->MaxNativeParameters = MIN2(pc->MaxUniformComponents / 4, 2048); pc->MaxInputComponents = screen->get_shader_param(screen, sh, PIPE_SHADER_CAP_MAX_INPUTS) * 4; pc->MaxOutputComponents = -- cgit v1.2.3