llvm / llvm-project

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

lldb can't find enumerators in parent scope #54602

Open slackito opened 2 years ago

slackito commented 2 years ago

Consider the following C++ program, built with clang++ -g and no other special flags.

namespace ns {
enum MyEnum { VALUE = 1 };
enum { ANON_ENUM_VALUE = 2 };
}

int main() {
  ns::MyEnum v = ns::VALUE;
  int i = ns::ANON_ENUM_VALUE;
  return v;
}

When loading this program into lldb, it will fail to evaluate ns::VALUE and ns::ANON_ENUM_VALUE:

(lldb) p ns::VALUE
error: expression failed to parse:
error: <user expression 0>:1:5: no member named 'VALUE' in namespace 'ns'
ns::VALUE
~~~~^
(lldb) p ns::ANON_ENUM_VALUE
error: expression failed to parse:
error: <user expression 1>:1:5: no member named 'ANON_ENUM_VALUE' in namespace 'ns'
ns::ANON_ENUM_VALUE
~~~~^

For the named enum this can be worked around by qualifying the value name:

(lldb) p ns::MyEnum::VALUE
(ns::MyEnum) $0 = VALUE

but not for the anonymous enum.

gdb behaves correctly (MyEnum is not an enum class, so ns::MyEnum::VALUE is not a valid expression):

(gdb) p ns::VALUE
$1 = ns::VALUE
(gdb) p ns::ANON_ENUM_VALUE
$2 = ns::ANON_ENUM_VALUE
(gdb) p ns::MyEnum::VALUE
`ns::MyEnum' is not defined as an aggregate type.
llvmbot commented 2 years ago

@llvm/issue-subscribers-lldb

Michael137 commented 2 years ago

Tried on Darwin. This is what the ast looks like:

(lldb) target module dump ast                                                        
Dumping clang ast for 43 modules.                                                    
TranslationUnitDecl 0x103821808 <<invalid sloc>> <invalid sloc>                      
|-FunctionDecl 0x1038226b0 <<invalid sloc>> <invalid sloc> main 'int ()' extern      
`-NamespaceDecl 0x103822758 <<invalid sloc>> <invalid sloc> ns                       
  `-EnumDecl 0x12d235608 <<invalid sloc>> <invalid sloc> MyEnum                      
    `-EnumConstantDecl 0x12d2356e8 <<invalid sloc>> <invalid sloc> VALUE 'ns::MyEnum'

Which explains why lookup is failing. When compiling on MacOS, it looks like dsymutil strips out debug info for anonymous enums so LLDB can't know about ns::ANON_ENUM_VALUE. If we skip the dsymutil step, we do get ANON_ENUM_VALUE in the debug info but the lookup still fails.

Michael137 commented 2 years ago

Interestingly we never call DWARFASTParserClang::ParseEnum on the anonymous enum. And the only reason we do call it for MyEnum is because we have a local variable of that type in the current frame and we resolve the type from DWARF in ClangExpressionDeclMap::LookupLocalVariable.

dwblaikie commented 8 months ago

Tried on Darwin. This is what the ast looks like:

(lldb) target module dump ast                                                        
Dumping clang ast for 43 modules.                                                    
TranslationUnitDecl 0x103821808 <<invalid sloc>> <invalid sloc>                      
|-FunctionDecl 0x1038226b0 <<invalid sloc>> <invalid sloc> main 'int ()' extern      
`-NamespaceDecl 0x103822758 <<invalid sloc>> <invalid sloc> ns                       
  `-EnumDecl 0x12d235608 <<invalid sloc>> <invalid sloc> MyEnum                      
    `-EnumConstantDecl 0x12d2356e8 <<invalid sloc>> <invalid sloc> VALUE 'ns::MyEnum'

Which explains why lookup is failing. When compiling on MacOS, it looks like dsymutil strips out debug info for anonymous enums so LLDB can't know about ns::ANON_ENUM_VALUE. If we skip the dsymutil step, we do get ANON_ENUM_VALUE in the debug info but the lookup still fails.

Hmm, this is confusing to me - if the AST is right (which it looks at least plausible/similar to Clang's AST with similar code) why doesn't it manage to do the ns::VALUE lookup in expression evaluation?

Michael137 commented 8 months ago

Tried on Darwin. This is what the ast looks like:

(lldb) target module dump ast                                                        
Dumping clang ast for 43 modules.                                                    
TranslationUnitDecl 0x103821808 <<invalid sloc>> <invalid sloc>                      
|-FunctionDecl 0x1038226b0 <<invalid sloc>> <invalid sloc> main 'int ()' extern      
`-NamespaceDecl 0x103822758 <<invalid sloc>> <invalid sloc> ns                       
  `-EnumDecl 0x12d235608 <<invalid sloc>> <invalid sloc> MyEnum                      
    `-EnumConstantDecl 0x12d2356e8 <<invalid sloc>> <invalid sloc> VALUE 'ns::MyEnum'

Which explains why lookup is failing. When compiling on MacOS, it looks like dsymutil strips out debug info for anonymous enums so LLDB can't know about ns::ANON_ENUM_VALUE. If we skip the dsymutil step, we do get ANON_ENUM_VALUE in the debug info but the lookup still fails.

Hmm, this is confusing to me - if the AST is right (which it looks at least plausible/similar to Clang's AST with similar code) why doesn't it manage to do the ns::VALUE lookup in expression evaluation?

I think what I meant is that we just don't have debug-info for ns::ANON_ENUM_VALUE because it's stripped by dsymutil. So ns::ANON_ENUM_VALUE has no chance of working with the expression evaluator (in the dsym case). Even if we do get debug-info for it, the lookup still fails for the reason below.

Regarding, ns::VALUE, it looks like what's happening is that LLDB is trying to do a FindTypesInNamespace lookup to find VALUE inside of the namespace. This won't work since VALUE is not a type. I guess we could try to make LLDB look "through" enums, by fetching VALUE first and realising that its parent is an enum DIE. Then if the parent context chains match we allow evaluating VALUE. But since VALUE is not indexed, I don't see a good way of doing this kind of lookup short of scanning all the children of ns, which doesn't seem feasible?

dwblaikie commented 8 months ago

Right ,the fact that enumerators aren't in the index is a problem (I've got an issue filed on the DWARF committee, but I'm meant to go measure the impact of adding enumerators to the index - and I expect the answer is "this is too much of an index size increase - though it'd be technically the right thing to do").

But what I wondered was - you showed the dumped AST was correct. So /then/ I don't understand why the expression can't be evaluated, if we managed to get the AST right eventually. Because I guess some name lookup is done by lldb through the index and if that fails, even if the expression would succeed if it was given straight to Clang's name lookup... which seems sort of unfortunate? Yeah, probably makes the expression evaluation more deterministic (less likely to have "this expression failed before, and worked now - even though I just did some other apparently unrelated action in between (because it caused some more DWARF to be loaded into the AST)") though less functional - maybe this is the cause of several situations I've seen where I'm like "but the AST looks totally fine for this expression being evaluated, so why didn't it parse?" - because lldb's index/lookup stuff before passing off to the AST failed.

Michael137 commented 8 months ago

But what I wondered was - you showed the dumped AST was correct. So /then/ I don't understand why the expression can't be evaluated, if we managed to get the AST right eventually.

To clarify, the AST happens to be correct because we have a local variable of type MyEnum. In the LLDB lookup machinery we do a LookupLocalVariable unconditionally, which creates AST nodes for all nodes in the frame. If we changed the example to:

namespace ns {
enum MyEnum { VALUE = 1 };
}  // namespace ns

class S {
  ns::MyEnum v;
};

int main() {
  S s;
  return 0;
}

...the AST after expr ns::VALUE would look as follows:

TranslationUnitDecl 0x13d1e8808 <<invalid sloc>> <invalid sloc>
|-FunctionDecl 0x13d1e9718 <<invalid sloc>> <invalid sloc> main 'int ()' extern
|-CXXRecordDecl 0x13dcd5208 <<invalid sloc>> <invalid sloc> <undeserialized declarations> class S
`-NamespaceDecl 0x13dcd53b0 <<invalid sloc>> <invalid sloc> ns

LLDB has no idea that it just created the decl for the type we were looking for. From its perspective, clang is asking for a type (or global variable) called VALUE, which exists nowhere that LLDB knows of (short of doing a search through all enum children of the ns namespace).

So to answer your question, we can't just hand the correct-looking AST to clang because clang is looking for a specific decl but LLDB doesn't know which of the decls in the AST clang actually wants. Only when one of the LLDB lookups into DWARF succeeds can we do this, because then LLDB has a handle to the correct AST node.

To make this work consistently (i.e., without relying on the AST coincidentally containing the correct EnumDecl), LLDB needs to be able to know that when clang asks us what VALUE is, we can tell that it's actually the name of an enum constant. Two options come to mind:

  1. Add enum constants into the index (but as you say this will probably balloon section size considerably)
  2. OR, during LLDB's type lookups, if we don't find a type by the name VALUE, we could be looking for a constant. So enumerate all enums in the current DeclContext and check whether a constant with such name exists

Hope that sheds some more light on the situation

adrian-prantl commented 8 months ago

(but as you say this will probably balloon section size considerably)

Would be a nice experiment to reapply some form of this commit and see what the numbers actually are:

commit ad64aeac44841df272456c65e40ece9476ae3b1e
Author: Adrian Prantl <aprantl@apple.com>
Date:   Mon Dec 23 23:50:20 2013 +0000

    Debug info: Add enumerators to the __apple_names accelerator table.
    rdar://problem/11516681.

    llvm-svn: 197927
dwblaikie commented 8 months ago

ad64aeac44841df272456c65e40ece9476ae3b1e

Oh, right ,I was going to write this myself - thanks for the pointer/reminder that you'd already done that...

dwblaikie commented 8 months ago

ad64aea

Oh, right ,I was going to write this myself - thanks for the pointer/reminder that you'd already done that...

Hmm, wonder if there's any more history/discussion we can dredge up on it being reverted...

adrian-prantl commented 8 months ago

I looked through my inbox when this came up on the DWARF meetings recently, but I think it boiled down to that I landed this without proper review at first, then reverted it after Eric pointed that out, and then never got around to revive the patch.