[Mesa-dev] [PATCH 03/20] glsl: add missing null check in tfeedback_decl::init()

[Ian Romanick]

On 05/14/2014 10:55 AM, Juha-Pekka Heikkila wrote:
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
>  src/glsl/link_varyings.cpp | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index 71998df..7fdf337 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -318,6 +318,11 @@ tfeedback_decl::init(struct gl_context *ctx, const void *mem_ctx,
>     const char *base_name_end;
>     long subscript = parse_program_resource_name(input, &base_name_end);
>     this->var_name = ralloc_strndup(mem_ctx, input, base_name_end - input);
> +   if (this->var_name == NULL) {
> +      _mesa_error_no_memory(__func__);
> +      return;
> +   }
> +

This doesn't make anything better.  For giggles, I replaced this with

   if ((rand() & 0x07) == 0) {
      ralloc_free((void *)this->var_name);
      this->var_name = NULL;
      _mesa_error_no_memory(__func__);
      return;
   }

Randomly, pretend that the allocation failed.  Then I ran piglit
transform feedback tests:

    ./piglit-run.py -p glx -t feed -t xfb tests/all.py results

And got a GIANT pile of crashes.

    [370/370] crash: 68, fail: 1, pass: 264, skip: 36, warn: 1

One of the crashes is:

[idr at mumford-wire piglit]$ gdb --args bin/ext_transform_feedback-output-type gl_NextBuffer-2 -fbo -auto
GNU gdb (GDB) Fedora 7.5.1-42.fc18
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /home/idr/devel/graphics/piglit/bin/ext_transform_feedback-output-type...done.
(gdb) r
Starting program: /home/idr/devel/graphics/piglit/bin/ext_transform_feedback-output-type gl_NextBuffer-2 -fbo -auto
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Testing type: gl_NextBuffer-2

Program received signal SIGSEGV, Segmentation fault.
0x000000375052fa96 in __strcmp_sse42 () from /lib64/libc.so.6
Missing separate debuginfos, use: debuginfo-install expat-2.1.0-4.fc18.x86_64 glibc-2.16-34.fc18.x86_64 libgcc-4.7.2-8.fc18.x86_64 libstdc++-4.7.2-8.fc18.x86_64 mesa-libGLU-9.0.0-1.fc18.x86_64 systemd-libs-201-2.fc18.8.x86_64
(gdb) bt
#0  0x000000375052fa96 in __strcmp_sse42 () from /lib64/libc.so.6
#1  0x00007ffff3e5d164 in tfeedback_decl::is_same (x=..., y=...)
    at ../../src/glsl/link_varyings.cpp:357
#2  0x00007ffff3e5d9b7 in parse_tfeedback_decls (ctx=0x7ffff3ac6040, 
    prog=0xaf0710, mem_ctx=0xaeec50, num_names=11, 
    varying_names=0xaee5f0, decls=0xe437f0)
    at ../../src/glsl/link_varyings.cpp:581
#3  0x00007ffff3e64100 in link_shaders (ctx=0x7ffff3ac6040, prog=0xaf0710)
    at ../../src/glsl/linker.cpp:2489
#4  0x00007ffff3d90c44 in _mesa_glsl_link_shader (ctx=0x7ffff3ac6040, 
    prog=0xaf0710) at program/ir_to_mesa.cpp:3088
#5  0x00007ffff3c35694 in link_program (ctx=0x7ffff3ac6040, program=2)
    at main/shaderapi.c:915
#6  0x00007ffff3c364dc in _mesa_LinkProgram (programObj=2)
    at main/shaderapi.c:1383
#7  0x00007ffff7d2fcca in stub_glLinkProgram (program=2)
    at tests/util/generated_dispatch.c:17777
#8  0x0000000000401aef in piglit_init (argc=2, argv=0x7fffffffdeb8)
    at tests/spec/ext_transform_feedback/output-type.c:1500
#9  0x00007ffff7d07897 in run_test (gl_fw=0x69f010, argc=2, 
    argv=0x7fffffffdeb8)
    at tests/util/piglit-framework-gl/piglit_fbo_framework.c:50
#10 0x00007ffff7d05703 in piglit_gl_test_run (argc=2, 
    argv=0x7fffffffdeb8, config=0x7fffffffdd80)
    at tests/util/piglit-framework-gl.c:151
#11 0x00000000004018dd in main (argc=2, argv=0x7fffffffdeb8)
    at tests/spec/ext_transform_feedback/output-type.c:41
(gdb) 

It's still crashing in transform feedback handling code in Mesa.

If instead I change the code to

   if ((rand() & 0x07) == 0) {
      ralloc_free((void *)this->var_name);
      this->var_name = NULL;
   }

Randomly pretend that the allocation failed, but carry on as if nothing
happended.  Running the piglit transform feedback tests then gives:

    [370/370] crash: 67, fail: 1, pass: 263, skip: 36, warn: 3

In this mode the crash in "ext_transform_feedback-output-type
gl_NextBuffer-2" was at the next strcmp a few lines later in the same
function.

Since both tests use rand, there are a different number of crashes on
each run.  If the issue Klocwork found is actually fixed, there will be
zero crashes... just more failures.

For reference, without any change, a normal piglit run (with no
allocation failures) give:

    [370/370] fail: 1, pass: 330, skip: 36, warn: 3

This is a big part of the reason that nobody checks allocation
failures.  Physical and virtual memory is big enough that you almost
never fail an allocation before encountering some other failure.  The
failure paths are never tested, so they're wrong almost 100% of the
time. :(  A better way for us to test this would be to add an option to
ralloc (via an environment variable) to make it randomly fail
allocations.

>     if (subscript >= 0) {
>        this->array_subscript = subscript;
>        this->is_subscripted = true;