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

[Greg Clayton]

Siva: You will need to forward this to the clang lists to see if this is a correct fix.

> On Apr 9, 2015, at 11:16 AM, Siva Chandra <sivachandra at google.com> wrote:
> 
> Hi all,
> 
> I do not have anything concrete. However, the attached patch for clang
> fixes both the failing tests. I do not know if my fix is correct. Even
> if it is, we should find a way to "workaround" in LLDB so that older
> versions of clang are OK. I have some ideas for this, will share when
> I am able to put them into concrete words.
> 
> Thanks,
> Siva Chandra
> 
> On Thu, Apr 9, 2015 at 11:09 AM, Tamas Berghammer
> <tberghammer at google.com> wrote:
>> +sivachandra
>> 
>> Siva is looking into this issue. The problem is that lldb looks for a
>> mangled name _ZN3fooC1Ei while the object file (compiled by clang) contains
>> it with _ZN3fooC2Ei. The difference is that the first one belongs to the
>> complete constructor and the second one belongs to the base constructor.
>> 
>> On Thu, Apr 9, 2015 at 7:05 PM, Ilia K <ki.stfu at gmail.com> wrote:
>>> 
>>> @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/
>>> 
>>> 
>> 
> <clang.patch>