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 <[email protected]> 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 > <[email protected]> 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 <[email protected]> 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> _______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
