Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

stack overflow in debug info for union debug info #34287

Open Quuxplusone opened 6 years ago

Quuxplusone commented 6 years ago
Bugzilla Link PR35314
Status NEW
Importance P normal
Reported by Andrew Kelley (andrew@ziglang.org)
Reported on 2017-11-14 20:42:57 -0800
Last modified on 2017-11-15 13:42:03 -0800
Version 5.0
Hardware All All
CC aprantl@apple.com, dblaikie@gmail.com, llvm-bugs@lists.llvm.org, paul.robinson@am.sony.com, paul_robinson@playstation.sony.com
Fixed by commit(s)
Attachments test.ll (4345 bytes, text/plain)
Blocks
Blocked by
See also

Created attachment 19424 test.ll

With llvm 5.0.0, clang -c test.ll causes a stack overflow and then segfault.

Quuxplusone commented 6 years ago

Attached test.ll (4345 bytes, text/plain): test.ll

Quuxplusone commented 6 years ago

As pointed out in a discussion thread - this is likely caused by an enum (or any other type, likely) being its own scope.

Quuxplusone commented 6 years ago

So this shouldn't crash, of course - probably should be caught by the verifier (that scope chains are never cyclic)

Quuxplusone commented 6 years ago

Agreed, the Verifier should reject this. Checking that not entity's scope is self-referential would be easy to check, checking for cycles in general might be a little expensive.

Quuxplusone commented 6 years ago
It might not be so bad if we're willing to add a flag to the data structure.

For example here's pseudocode for how I do it in my frontend:

void check(node) {

    if (node.self_dependency_flag) {
        report_error("depends on itself");
        return;
    }

    node.self_dependency_flag = true;

    // do the code that has the potentially recursive call in it
    // ...

    node.self_dependency_flag = false;

}
Quuxplusone commented 6 years ago
(In reply to Adrian Prantl from comment #3)
> Agreed, the Verifier should reject this. Checking that not entity's scope is
> self-referential would be easy to check, checking for cycles in general
> might be a little expensive.

How expensive is it to walk up the parent scopes to the unit?  If you
find yourself during that walk, boom.  Seems like it shouldn't be too bad,
as it's not a generic cycle detection problem.

You only need to do this for entities that are themselves scopes.
And as long as each scope checks itself before walking its children,
it should be fine.

I am probably missing something...
Quuxplusone commented 6 years ago
> How expensive is it to walk up the parent scopes to the unit?
> If you find yourself during that walk, boom.

!1 = DICompositeType(scope: !1)
!2 = DICompositeType(scope: !1)

If we start this check at !2, we would run into an infinite recursion in the
Verifier. Well, I guess it could verify the parent before recursing...