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

Thu Apr 9 11:16:06 PDT 2015

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/
>>
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang.patch
Type: text/x-patch
Size: 1392 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150409/a7381ffd/attachment.bin>