[lldb-dev] Issue with expression parser finding the correct types

[Greg Clayton via lldb-dev]

I have identified an issue in the expression parse. If you compile this code:

struct Foo { typedef unsigned node; };
struct Bar { typedef int node; };
int main(int argc, const char **argv) {
    Foo::node foo_carp = 22;
    Bar::node bar_node = 33;
    return 0;
}


Then then do:

(lldb) file a.out
(lldb) b main
(lldb) run
(lldb) p (node *)&bar_node

This should fail because there is no "node" type in the global namespace. But we don't fail, we pick the first type whose base name matches "node" and return it and start using it regardless of where the type is located decl context wise... 

The issue happens when we call:

void ClangASTSource::FindExternalVisibleDecls(NameSearchContext &context, lldb::ModuleSP module_sp, CompilerDeclContext &namespace_decl, unsigned int current_id);

In this code we end up doing:

    TypeList types;
    SymbolContext null_sc;
    const bool exact_match = false;
    llvm::DenseSet<lldb_private::SymbolFile *> searched_symbol_files;
    if (module_sp && namespace_decl)
      module_sp->FindTypesInNamespace(null_sc, name, &namespace_decl, 1, types);
    else
      m_target->GetImages().FindTypes(null_sc, name, exact_match, 1,
                                      searched_symbol_files, types);

    if (size_t num_types = types.GetSize()) {
      for (size_t ti = 0; ti < num_types; ++ti) {

So if we don't have a valid namespace_decl, which will be the case when we are searching at the translation unit level, we end up searching the target for the first match for the type whose _basename_ matches "name" regardless if of the module we are currently running the expression in, and regardless of the translation unit decl context being a requirement. So we find any type whose basename matches "name" from _any_ decl context. That is really bad!

A few things wrong here:
1 - we seem to assume if we have a namespace (which is a parameter whose type is CompilerDeclContext &), that it must go with the module_sp. Why? Because a CompilerDeclContext has a "TypeSystem *" inside of it, and each module has a their own "TypeSystem *" for a give language. We then call module_sp->FindTypesInNamespace(...) with the CompilerDeclContext * to namespace_decl. This eventually makes its way down to SymbolFileDWARF::FindTypes(...) which will do the check:

  if (!DeclContextMatchesThisSymbolFile(parent_decl_ctx))
    return 0;

So if the CompilerDeclContext's type system doesn't match the type system of the DWARF file that is makes it into, it will ignore the search entirely. This means if we have a namespace from an expression like "p std::foo", and we find the namespace "foo" in one module, we will only search a specific module for "foo" within "std", even though another module could contain "std::foo". So if the first module that has debug info for namespace "std" doesn't contain "foo", we don't find the type?

2 - If we don't have a namespace, we want things only at the translation unit level, yet we search all modules in the target in the target image list order. It seems we should be checking by starting with the current module, then expanding to any module except the current module if the type lookup fails in the current module. Then we need to search for more than 1 type so we can weed any results out that don't have a decl context of a translation unit level or anonymous namespace from the current translation unit. 

So to fix this I might propose the following:

Modify all "FindTypes()" functions to take a new version of something like the DWARFDeclContext from DWARFDeclContext.h. This is an object that contains a enumeration for the decl context type plus an optional name . This think things like "translation unit with name 'main.c'", class with name "Foo", namespace with  name "std", function with name "erase". This new item could be something like:

class DeclContextMatch {
  enum class Kind {
    TranslationUnit,
    Namespace,
    Class,
    Struct,
    Union,
    Function
  };
  struct Entry {
    Kind kind;
    ConstString name;
  };
  std::vector<Entry> m_entries;
  void Append(DeclContextMatch::Kind k, const char *name = nullptr);
};

We would stop passing the "const CompilerDeclContext *parent_decl_ctx" to the ::FindTypes(...) calls and instead pass a "DeclContextMatch *" so that clients can easily weed out things that don't match in an abstract way that doesn't tie the containing decl context to the current module. So if I was doing to search for "node" at the translation unit level, I would fill in:

DeclContextMatch dcm;
dcm.Append(DeclContextMatch::Kind::TranslationUnit);
target->GetImages.FindTypes(null_sc, name, &dcm, ...)


Since the DeclContextMatch isn't specific to any module (unlike CompilerDeclContext instances which are), the search can happen and each SymbolFile subclass can search how ever they need to by only returning matches that match the DeclContextMatch if one is given. If no DeclContextMatch is supplied, then we search for any type whose basename matches the name requested. If DeclContextMatch is given, only matching types are returned.

This has to be causing all sorts of problems in our expressions, so we really need to fix this ASAP. Comments and suggestions are welcome.

Greg

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20180425/9b348b52/attachment.html>