[Bug,106928] When starting a match Rocket League crashes on "Go"

Commit Message

[e-mail] June 29, 2018, 7:16 a.m. UTC

https://bugs.freedesktop.org/show_bug.cgi?id=106928

--- Comment #11 from [e-mail] ---
(In reply to Roland Scheidegger from comment #9)
> (In reply to ubizjak from comment #7)
> > Please configure the build with:
> > 
> > CXXFLAGS="-Wp,-D_GLIBCXX_ASSERTIONS" ./autogen.sh
> 
> That didn't do anything neither. However I figured out the problem more or
> less in the code, and some googling said that using -D_GLIBCXX_DEBUG should
> make it trigger reliably, and indeed it does...

Great ;)

> The issue is that (you already showed that actually)
> src = std::vector of length 2, capacity 3 = {0x7f94d905d110, 0x7f94d905cf70}}
> And trying to access element src[2].
> There's an early exit in the function if src.size() is < 3. Since this
> didn't hit, apparently fold_assoc() resized the vector. And indeed it can do
> that (there's an explicit n->src.resize(2) somewhere, and it would still
> return false in this case).
> 
> I think something like this should do:
> diff --git a/src/gallium/drivers/r600/sb/sb_expr.cpp
> b/src/gallium/drivers/r600/sb/sb_expr.cpp
> index 1df78da660..c77b9f2d7d 100644
> --- a/src/gallium/drivers/r600/sb/sb_expr.cpp
> +++ b/src/gallium/drivers/r600/sb/sb_expr.cpp
> @@ -945,6 +945,8 @@ bool expr_handler::fold_alu_op3(alu_node& n) {
>         if (!sh.safe_math && (n.bc.op_ptr->flags & AF_M_ASSOC)) {
>                 if (fold_assoc(&n))
>                         return true;
> +               else if (n.src.size() < 3)
> +                       return fold_alu_op2(n);
>         }
>  
>         value* v0 = n.src[0]->gvalue();

I wonder if we should fix expr_handler::fold_assoc instead. Digging through the
code, fold_assoc is called only from expr_handler::fold_alu_op2 and
expr_handler::fold_alu_op3. When true is returned, it triggers an early exit
from expr_handler::fold_alu_op{2,3} functions.

The part we are looking for in expr_handler::fold_assoc is:

                } else { // MULADD => ADD
                        n->src[0] = n->src[2];
                        n->bc.src[0] = n->bc.src[2];
                        n->src[1] = sh.get_const_value(cr);
                        memset(&n->bc.src[1], 0, sizeof(bc_alu_src));

                        n->src.resize(2);
                        n->bc.set_op(ALU_OP2_ADD);
                }

So, let's call fold_alu_op2 here and return true to trigger early exit in
expr_handler::fold_alu_op3. This is what the code a couple of lines above the
presented code does when an operand degenerates to mov.

The (effectively the same patch as yours) proposed patch would be:


WDYT?

On a side note, maybe -D_GLIBCXX_ASSERTIONS should be added to mesa testsuite.
This is the flag that Fedora 28 builds use by default now, so it would be
beneficial to catch these bugs early in the development cycle, before they
reach users.

Patch

diff --git a/src/gallium/drivers/r600/sb/sb_expr.cpp
b/src/gallium/drivers/r600/sb/sb_expr.cpp
index 7a5d62c8e8..a609d1377f 100644
--- a/src/gallium/drivers/r600/sb/sb_expr.cpp
+++ b/src/gallium/drivers/r600/sb/sb_expr.cpp
@@ -714,6 +714,8 @@  bool expr_handler::fold_assoc(alu_node *n) {

                        n->src.resize(2);
                        n->bc.set_op(ALU_OP2_ADD);
+                       fold_alu_op2(*n);
+                       return true;
                }
        } else if (last_arg >= 0) {
                n->src[0] = a->src[last_arg];