Bug 33810 - Endless loop in ralloc code
Summary: Endless loop in ralloc code
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: glsl-compiler (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Linux (All)
: medium blocker
Assignee: Ian Romanick
QA Contact:
URL:
Whiteboard:
Keywords:
: 33809 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-02-01 07:52 UTC by s3734770
Modified: 2011-02-02 13:28 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch: fixes the issue (975 bytes, patch)
2011-02-02 03:23 UTC, s3734770
Details | Splinter Review

Description s3734770 2011-02-01 07:52:25 UTC
The ralloc code generates a deadlock when calling strlen().

It's a blocker because an error message is printed out using ralloc_asprintf_append.
So before you wait any longer please apply this patch to fix compiling the shader compiler:

diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
index 1fa209c..a06108f 100644
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -104,10 +104,11 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *ctx,
 	 ? ""
 	 : ((ver == highest_version) ? ", and " : ", ");
 
-      ralloc_asprintf_append(& supported, "%s%d.%02d%s",
+     // TODO: fix the endless loop in ralloc_asprintf_append (strlen exceeds)
+      /*ralloc_asprintf_append(& supported, "%s%d.%02d%s",
 			     prefix,
 			     ver / 100, ver % 100,
-			     (ver == 100) ? " ES" : "");
+			     (ver == 100) ? " ES" : "");*/
    }
 
    this->supported_version_string = supported;



The endless loop is caught at the following backtrace:

#0  __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:40
#1  0x00007fffea864071 in ralloc_vasprintf_append (str=0x7ffffffe68b8, 
    fmt=0x7fffea99e667 "%s%d.%02d%s", args=0x7ffffffe67d0) at ralloc.c:446
#2  0x00007fffea8641a8 in ralloc_asprintf_append (str=0x13dcc30, 
    fmt=0xffffffff <Address 0xffffffff out of bounds>) at ralloc.c:427
#3  0x00007fffea798d2d in _mesa_glsl_parse_state::_mesa_glsl_parse_state (
    this=0x1365610, ctx=<value optimized out>, target=<value optimized out>, 
    mem_ctx=<value optimized out>) at glsl_parser_extras.cpp:111
#4  0x00007fffea7a4c40 in read_builtins (target=35633, 
    protos=0x7fffea9bfb00 "(\n(function radians\n  (signature float\n    (parameters\n      (declare (in) float degrees))\n    ())\n  (signature vec2\n    (parameters\n      (declare (in) vec2 degrees))\n    ())\n  (signature vec3\n    (p"..., functions=0x7fffeace0180, count=74) at builtin_function.cpp:42
#5  0x00007fffea7a4e18 in _mesa_read_profile (state=0x12f5710, 
    profile_index=5, prototypes=<value optimized out>, functions=0x0, 
    count=20372624) at builtin_function.cpp:14696
#6  0x00007fffea7a50c1 in _mesa_glsl_initialize_functions (state=0x12f5710)
    at builtin_function.cpp:14754
#7  0x00007fffea8664b7 in _mesa_ast_to_hir (instructions=0x1305240, 
    state=0x12f5710) at ast_to_hir.cpp:63
#8  0x00007fffea85d880 in _mesa_glsl_compile_shader (ctx=0xf18ba0, 
    shader=0x127a6f0) at program/ir_to_mesa.cpp:3133
#9  0x00000000005329eb in LOAD (this=0x7ffff7f6cd00, 


Thanks for fixing
Comment 1 Ian Romanick 2011-02-01 16:05:20 UTC
I think this bug was fixed by the following commit.  Please reopen the bug if that doesn't do it.

commit a7d350790b4d0416117bc785aa77de52e9298a01
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Tue Feb 1 00:20:01 2011 -0800

    glsl: Fix memory error when creating the supported version string.
    
    Passing ralloc_vasprintf_append a 0-byte allocation doesn't work.  If
    passed a non-NULL argument, ralloc calls strlen to find the end of the
    string.  Since there's no terminating '\0', it runs off the end.
    
    Fixes a crash introduced in 14880a510a1a288df0778395097d5a52806abfb0.
Comment 2 Ian Romanick 2011-02-01 18:37:00 UTC
*** Bug 33809 has been marked as a duplicate of this bug. ***
Comment 3 s3734770 2011-02-01 23:40:15 UTC
a7d350790b4d0416117bc785aa77de52e9298a01 does not fix the issue, I already tested after a7d350790b4d0416117bc785aa77de52e9298a01.

char *supported = ralloc_strdup(this, ""); is optimized to NULL by GCC. I'm not able to write a workaround because GCC optimizes out all pre-checks.

I think the bug is in strlen itself (the trivial '\0'), but it's only a presumption.
Comment 4 s3734770 2011-02-01 23:57:24 UTC
 - It also occurs when I use an other string value for the strdup call.
 - Alternative strlen implementations are also transformed into SSE2 "optimized" strlen.
 - setting existing_length to 0 leads to compile errors with builtin_function
Comment 5 s3734770 2011-02-02 03:23:56 UTC
Created attachment 42849 [details] [review]
Patch: fixes the issue

ralloc_asprintf_append is not working correctly, so i replaced it by ralloc_asprintf and ralloc_strcat.
Comment 6 Kenneth Graunke 2011-02-02 11:53:35 UTC
This is insane.  If your GCC is "optimizing" ralloc_strdup(this, "") to NULL, it is fundamentally broken.  That call is supposed to allocate a chunk of memory containing a ralloc header followed by 1-byte of '\0'.  Replacing a call to allocate 49 bytes or so with NULL is just completely wrong.

Furthermore, ralloc_vasprintf_append checks for the supplied pointer being NULL and will fall back to ralloc_vasprintf.  So even if it -were- NULL, it should still work.

Finally, if supported is NULL (as you say), your patch should make things worse, not better.  ralloc_strcat should assertion fail in that case.  Though, since I assume you have assertions turned off, it would proceed to call strlen(NULL), which seems even less likely to work.

Try a clean build.  Maybe re-autogen.  It's probably insane transitory breakage.

Failing that, post your compiler version and optimization flags.
Comment 7 s3734770 2011-02-02 13:15:53 UTC
gcc version 4.4.5

No matter what my compiler version and opt. flags are, you cannot expect from devs to set up a specific flag setting that this one optimization is not done that the code works.

My patch does not leak memory, it keeps the semantic and follows the code style. It fixes the bug and Ivans commit is no longer a blocker.

Are there any reasons left why not apply this?

And don't try to reclose this bug till it's not fixed.
Comment 8 s3734770 2011-02-02 13:28:57 UTC
oh sorry, you're right.
After a reboot with an updated package and a rebuild it works fine.

thanks for being patient


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.