[Lldb-commits] [Diffusion] rL234178: We have an issue where if you use a C function right now that has no prototype…

[Ilia K]

@clayborg wrote:

> This probably fails after my recent change to ClangExpressionDeclMap::GetFunctionAddress() from revision 234178.

> 

> The problem was that you might have an expression like:

> 

> (lldb) p (int)printf("%i\n", 123)

> 

> And we would mangle printf as a C++ function and get "_Z...printf..." which of course doesn't exist as a mangled name. Prior to my fix, this function would always do:

> 

>   Mangled mangled(name, is_mangled);

>   

>   CPPLanguageRuntime::MethodName method_name(mangled.GetDemangledName());

>   

>   llvm::StringRef basename = method_name.GetBasename();

>   

>   if (!basename.empty())

>   {

>       FindCodeSymbolInContext(ConstString(basename), m_parser_vars->m_sym_ctx, sc_list);

>       sc_list_size = sc_list.GetSize();

>   }

> 

> This is just plain wrong. It needs to be:

> 

>   Mangled mangled(name, is_mangled);

>   

>   CPPLanguageRuntime::MethodName method_name(mangled.GetDemangledName());

>   

>   // the C++ context must be empty before we can think of searching for symbol by a simple basename

>   if (method_name.GetContext().empty())

>   {

>       llvm::StringRef basename = method_name.GetBasename();

>   

>       if (!basename.empty())

>       {

>           FindCodeSymbolInContext(ConstString(basename), m_parser_vars->m_sym_ctx, sc_list);

>           sc_list_size = sc_list.GetSize();

>       }

>   }

> 

> Note that we now check for the method's context (namespace and containing classes) to be empty. So this works for "_Z...printf..." because the context is empty and the basename is "printf".

> 

> The problem with ignoring if the context is empty is you can end up getting some mangled name like:

> 

> _ZNSt3__16vectorIcNS_9allocatorIcEEEixEm

> 

> which demangles to:

> 

> std::__1::vector<char, std::__1::allocator<char> >::operator[](unsigned long)

> 

> The "method_name.GetContext()" returns "std::__1::vector<char, std::__1::allocator<char> >"

>  and "method_name.GetBasename()" returns "operator[]"

> 

> Now if we don't check for the context being empty, we are happy to accept _any_ function whose basename is "operator[]". So if the exact mangled name is not in your program because you never used that function so the template was never instantiated or used, then we will search for any function whose basename is "operator[]" which will match std::basic_string<char>::operator[] since all C++ standard libraries explicitly instantiate std::string. So now your function would happily run using the completely wrong function.

> 

> In your first case "myInt::myInt(int)" will give a context of "myInt" and we won't try to lookup "myInt" by basename. So the bug here is that you have no symbol for _ZN5myIntC1Ei in your executable's symbol table and you probably should.

> 

> Same goes for "_ZN3fooC1Ei" which demangles to "foo::foo(int)".

> 

> If you have a class like:

> 

> template <class T>

>  namespace my_stuff {

> 

>   class my_class {

>       static void foo();

>   }

> 

> }

> 

> Any you don't instantiate this template and yet you still have "foo::foo(int) like the second example, you really don't want to have a call to "my_stuff::my_class::foo()" actually fall "foo::foo()" because we grab the basename from "my_stuff::my_class::foo()" and have it just call any function whose basename if "foo".

> 

> So my fix needs to stay in and we need to figure out why there is no symbol for constructors that are supposed to exist, or we need to change the code so that those symbols do exist. I know constructors have "in charge" and "not in charge" constructors and maybe the test case code only generates on of them and then the expression tries to use the other?

> 

> Dump the symbol table of the "a.out" file and see what symbols it has. That should tell us more. We will need to see the mangled names, so use the following command:

> 

> (lldb) image dump symtab --show-mangled-names a.out


The DataFormatterSynthValueTestCase.test_with_dwarf_and_run_command test declares a new struct type in run-time. It can't be in the object file :)

  self.expect("expression struct S { myInt theInt{12}; }; S()", substrs = ['(theInt = 12)'])


USERS
  tberghammer (Auditor)
  ki.stfu (Auditor)

http://reviews.llvm.org/rL234178

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/