llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.05k stars 11.98k forks source link

stack overflow in debug info for union debug info #34662

Open andrewrk opened 6 years ago

andrewrk commented 6 years ago
Bugzilla Link 35314
Version 5.0
OS All
Attachments test.ll
CC @adrian-prantl,@dwblaikie,@pogo59,@pogo59

Extended Description

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

adrian-prantl 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...

pogo59 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.

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...

andrewrk 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;

}

adrian-prantl 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.

dwblaikie commented 6 years ago

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

dwblaikie 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.