[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2
labath added a comment. In https://reviews.llvm.org/D41962#973827, @craig.topper wrote: > I don't know what platforms this needs to support. But __builtin_cpu_support > only works when compiled with clang or gcc. And it requires compiler-rt or > libgcc. I don't know if that's guaranteed to exist on Windows. I doubt this test was ever passing on windows, as our RegisterContextWindows does not even acknowledge the existence of sse registers. If we wanted to be fancy, we could do some manual cpuid parsing here (the test contains inline assembly anyway), but that's probably not necessary. Comment at: packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c:21 + static volatile unsigned haveAVX2; + haveAVX2 = __builtin_cpu_supports("avx2"); unsigned int ymmallones = 0x; lebedev.ri wrote: > Note that you need to call `__builtin_cpu_init()` before calling > `__builtin_cpu_supports()`. > Or maybe it is already called before this? Gcc manual says: ```This built-in (__builtin_cpu_init) function needs to be invoked ..., only when used in a function that is executed before any constructors are called. ``` So calling it here should not be necessary. However, I am still unable to get gcc (6.3) to return 1 here. Clang (since at least 3.8) seems to be doing fine however, so that's probably enough for this test. Repository: rL LLVM https://reviews.llvm.org/D41962 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2
jasonmolenda added a comment. fwiw I'm working on upstreaming on zmm (avx512) patches that we have locally (there's one testsuite fail I still need to find time to fix) and the TestZMMRegister.py test that ChrisB wrote to test this is written as skip-unless-darwin, and there's a new skipUnlessFeature() method added to decorators.py which runs sysctl to detect hardware features (in this case, hw.optional.avx512f) which, I suspect, is an even more mac-specific way of doing this. While Adrian's approach would be gcc/clang specific, it would def be better than depending on a sysctl. Repository: rL LLVM https://reviews.llvm.org/D41962 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2
jasonmolenda added a comment. I suppose a possible alternative would be to figure out the avx2 / avx512 features manually based on the cpuid instead of letting the compiler do it for us. e.g. https://stackoverflow.com/questions/1666093/cpuid-implementations-in-c and then checking the bits as e.g. described in https://en.wikipedia.org/wiki/CPUID . Bummer to do it so low level if we can delegate this to the compiler though. Repository: rL LLVM https://reviews.llvm.org/D41962 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2
aprantl added a comment. In https://reviews.llvm.org/D41962#974656, @jasonmolenda wrote: > I suppose a possible alternative would be to figure out the avx2 / avx512 > features manually based on the cpuid instead of letting the compiler do it > for us. e.g. > https://stackoverflow.com/questions/1666093/cpuid-implementations-in-c and > then checking the bits as e.g. described in > https://en.wikipedia.org/wiki/CPUID . Bummer to do it so low level if we can > delegate this to the compiler though. I don't know MSVC well enough and don't have access to one to test it but: This would also only work if there were a compiler-independent way of writing inline assembler. Is that possible? Other fun facts: Clang doesn't even define `__builtin_cpu_init()`. Repository: rL LLVM https://reviews.llvm.org/D41962 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2
aprantl updated this revision to Diff 129639. aprantl added a comment. Uploaded a mildly better version that is NFC on MSVC. https://reviews.llvm.org/D41962 Files: packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c Index: packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c === --- packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c +++ packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c @@ -6,7 +6,6 @@ // License. See LICENSE.TXT for details. // //===--===// - void func() { unsigned int ymmvalues[16]; unsigned char val; @@ -17,6 +16,14 @@ ymmvalues[i] = (val << 24) | (val << 16) | (val << 8) | val; } + // Detect AVX2. + static volatile unsigned haveAVX2; +#ifdef _MSC_VER + haveAVX2 = 1; // TODO: Implement this for MSVC. +#else + haveAVX2 = __builtin_cpu_supports("avx2"); +#endif + unsigned int ymmallones = 0x; __asm__("int3;" "vbroadcastss %1, %%ymm0;" Index: packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py === --- packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py +++ packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py @@ -51,6 +51,10 @@ break self.assertTrue(matched, STOPPED_DUE_TO_SIGNAL) +# Detect AVX2 support and early exit otherwise. +if self.frame().FindVariable("haveAVX2").GetValue() == "0": + return False + if self.getArchitecture() == 'x86_64': register_range = 16 else: Index: packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c === --- packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c +++ packages/Python/lldbsuite/test/functionalities/register/intel_avx/main.c @@ -6,7 +6,6 @@ // License. See LICENSE.TXT for details. // //===--===// - void func() { unsigned int ymmvalues[16]; unsigned char val; @@ -17,6 +16,14 @@ ymmvalues[i] = (val << 24) | (val << 16) | (val << 8) | val; } + // Detect AVX2. + static volatile unsigned haveAVX2; +#ifdef _MSC_VER + haveAVX2 = 1; // TODO: Implement this for MSVC. +#else + haveAVX2 = __builtin_cpu_supports("avx2"); +#endif + unsigned int ymmallones = 0x; __asm__("int3;" "vbroadcastss %1, %%ymm0;" Index: packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py === --- packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py +++ packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py @@ -51,6 +51,10 @@ break self.assertTrue(matched, STOPPED_DUE_TO_SIGNAL) +# Detect AVX2 support and early exit otherwise. +if self.frame().FindVariable("haveAVX2").GetValue() == "0": + return False + if self.getArchitecture() == 'x86_64': register_range = 16 else: ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2
craig.topper added a comment. __builtin_cpu_init was added to clang between 5.0 and 6.0 https://reviews.llvm.org/D41962 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF
lhames created this revision. lhames added a reviewer: davide. lhames added a project: LLDB. Herald added subscribers: llvm-commits, JDevlieghere. Failure to build the method override tables results in expression failures on the following trivial test case: class Base { public: virtual ~Base() {} virtual void foo() {} }; class Derived : public Base { public: virtual void foo() {} }; int main() { Derived d; Base *b = &d; return 0; // "expr b->foo()" ok. "expr d.foo()" crashes. } The reason is that without an overrides table, the definition of foo in derived is treated as a new method definition (rather than an override) and allocated its own vtable entry which does not exist in the vtable of Derived in the compiled program. Repository: rL LLVM https://reviews.llvm.org/D41997 Files: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Python/lldbsuite/test/expression_command/call-overridden-method/Makefile Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp Index: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp === --- Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -40,6 +40,7 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/StreamString.h" +#include "clang/AST/CXXInheritance.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" @@ -2099,6 +2100,84 @@ return template_param_infos.args.size() == template_param_infos.names.size(); } +static bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) { + // FIXME: This should detect covariant return types, but currently doesn't. + assert(&m1->getASTContext() == &m2->getASTContext() && + "Methods should have the same AST context"); + clang::ASTContext &context = m1->getASTContext(); + + const auto *m1Type = +llvm::cast( + context.getCanonicalType(m1->getType())); + + const auto *m2Type = +llvm::cast( + context.getCanonicalType(m2->getType())); + + auto compareArgTypes = +[&context](const clang::QualType &m1p, const clang::QualType &m2p) { + return context.hasSameType(m1p.getUnqualifiedType(), + m2p.getUnqualifiedType()); +}; + + return !std::equal(m1Type->param_type_begin(), m1Type->param_type_end(), + m2Type->param_type_begin(), compareArgTypes); +} + +static void addOverridesForMethod(clang::CXXMethodDecl *decl) { + if (!decl->isVirtual()) +return; + + clang::CXXBasePaths paths; + + auto find_overridden_methods = +[decl](const clang::CXXBaseSpecifier *specifier, clang::CXXBasePath &path) { + if (auto *base_record = + llvm::dyn_cast( +specifier->getType()->getAs()->getDecl())) { + +clang::DeclarationName name = decl->getDeclName(); + +// If this is a destructor, check whether the base class destructor is +// virtual. +if (name.getNameKind() == clang::DeclarationName::CXXDestructorName) + if (auto *baseDtorDecl = base_record->getDestructor()) { +if (baseDtorDecl->isVirtual()) { + path.Decls = baseDtorDecl; + return true; +} else + return false; + } + +// Otherwise, search for name in the base class. +for (path.Decls = base_record->lookup(name); !path.Decls.empty(); + path.Decls = path.Decls.slice(1)) { + if (auto *method_decl = +llvm::dyn_cast(path.Decls.front())) +if (method_decl->isVirtual() && !isOverload(decl, method_decl)) { + path.Decls = method_decl; + return true; +} +} + } + + return false; +}; + + if (decl->getParent()->lookupInBases(find_overridden_methods, paths)) { +for (auto *overridden_decl : paths.found_decls()) + decl->addOverriddenMethod( +llvm::cast(overridden_decl)); + } +} + +static void addMethodOverrides(ClangASTContext &ast, CompilerType &clang_type) { + if (auto *record = + ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType())) +for (auto *method : record->methods()) + addOverridesForMethod(method); +} + bool DWARFASTParserClang::CompleteTypeFromDWARF(const DWARFDIE &die, lldb_private::Type *type, CompilerType &clang_type) { @@ -2311,6 +2390,7 @@ } } +addMethodOverrides(m_ast, clang_type); ClangASTContext::BuildIndirectFields(clang_type); ClangASTContext::CompleteTagDeclarationDefinition(clang_type); Index: Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp === --- /dev/null +++ Python/lldbs
[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF
aprantl added a comment. Awesome! Comment at: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2103 +static bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) { + // FIXME: This should detect covariant return types, but currently doesn't. Could you add some doxygen comments explaining what the new function do and why doing this is necessary? Comment at: Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py:14 + +from __future__ import print_function + where is this used? Comment at: Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp:15 + Base *b = &d; + return 0; // Please test these expressions while stopped at this line: +} the expressions are missing :-) Perhaps convert this into an inline testcase? Repository: rL LLVM https://reviews.llvm.org/D41997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF
jingham added inline comments. Comment at: Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp:15 + Base *b = &d; + return 0; // Please test these expressions while stopped at this line: +} aprantl wrote: > the expressions are missing :-) > Perhaps convert this into an inline testcase? The expressions are in the test .py file. It isn't necessary to put them here, I'd just fix the comment. Please don't convert regular test cases into inline ones, however. The benefit of inline test cases is mostly that they are easier to write, but they are harder to debug when they go wrong. So if the are already in regular form I'd rather not convert them. Repository: rL LLVM https://reviews.llvm.org/D41997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2
Why not just look for the AVX registers by name that are only available if they are correctly detected by the native lldb-server or debugserver? Then we can avoid all of this. If we don't execute any instructions that crash the program, we can stop before any specialized AVX instructions are executed and kill the program is we don't see a register by name? > On Jan 12, 2018, at 8:44 AM, Jason Molenda via Phabricator via lldb-commits > wrote: > > jasonmolenda added a comment. > > I suppose a possible alternative would be to figure out the avx2 / avx512 > features manually based on the cpuid instead of letting the compiler do it > for us. e.g. > https://stackoverflow.com/questions/1666093/cpuid-implementations-in-c and > then checking the bits as e.g. described in > https://en.wikipedia.org/wiki/CPUID . Bummer to do it so low level if we can > delegate this to the compiler though. > > > Repository: > rL LLVM > > https://reviews.llvm.org/D41962 > > > > ___ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Very cool and close. It would be nice to function correctly without asserts, see inlined comment. Comment at: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2105-2106 + // FIXME: This should detect covariant return types, but currently doesn't. + assert(&m1->getASTContext() == &m2->getASTContext() && + "Methods should have the same AST context"); + clang::ASTContext &context = m1->getASTContext(); Use lldb_assert and possibly return false afterwards in case the asserts are compiled out Repository: rL LLVM https://reviews.llvm.org/D41997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF
davide added a comment. In https://reviews.llvm.org/D41997#974957, @clayborg wrote: > Very cool and close. It would be nice to function correctly without asserts, > see inlined comment. Out of curiosity, why does `lldb` roll its own assertion() mechanism instead of using the standard one? Repository: rL LLVM https://reviews.llvm.org/D41997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2
aprantl added a comment. > Why not just look for the AVX registers by name that are only available if > they are correctly detected by the native lldb-server or debugserver? Then we > can avoid all of this. If we don't execute any instructions that crash the > program, we can stop before any specialized AVX instructions are executed and > kill the program is we don't see a register by name? I considered doing something like this, but I want to avoid relying on the AVX2 support in LLDB to work in order to detect AVX2. If I use an LLDB mechanism for this then (exaggerating here!) someone could remove AVX support from LLDB and this test would still pass. https://reviews.llvm.org/D41962 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2
aprantl added a comment. > there's a new skipUnlessFeature() method added to decorators.py which runs > sysctl to detect hardware features (in this case, hw.optional.avx512f) How does one execute a program like `sysctl` on the remote? I have seen code in TestLldbGdbServer.py that uses `platform get-file /proc/cpuinfo` to achieve something similar for Linux, but that works without executing a new process. https://reviews.llvm.org/D41962 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2
jasonmolenda added a comment. In https://reviews.llvm.org/D41962#975168, @aprantl wrote: > > there's a new skipUnlessFeature() method added to decorators.py which runs > > sysctl to detect hardware features (in this case, hw.optional.avx512f) > > How does one execute a program like `sysctl` on the remote? I have seen code > in TestLldbGdbServer.py that uses `platform get-file /proc/cpuinfo` to > achieve something similar for Linux, but that works without executing a new > process. this skipUnlessFeature sysctl check was all performed on the system running the testsuite. Checking whether the feature exists in the program (the approach you're taking) is more correct. We usually do host != target testsuite runs for arm devices, but there's no reason why someone couldn't do a macos x freebsd testsuite run and the sysctl check would be invalid in that case. https://reviews.llvm.org/D41962 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r322418 - We have two sources for path remapping information that we get out
Author: jmolenda Date: Fri Jan 12 14:42:45 2018 New Revision: 322418 URL: http://llvm.org/viewvc/llvm-project?rev=322418&view=rev Log: We have two sources for path remapping information that we get out of a dSYM per-uuid plist that may be present (dsymutil does not create this plist, it is only added after the fact by additional tools) -- either the DBGBuildSourcePath + DBGSourcePath pair of k-v entries which give us the build-time and debug-time remapping, or the newer DBGSourcePathRemapping dictionary which may give us multiple remappings. I'm changing the order that we process these & add them to the list of source remappings. If the DBGSourcePathRemapping dict is present, it should be the first entries we will try. Modified: lldb/trunk/source/Host/macosx/Symbols.cpp lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp Modified: lldb/trunk/source/Host/macosx/Symbols.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/Symbols.cpp?rev=322418&r1=322417&r2=322418&view=diff == --- lldb/trunk/source/Host/macosx/Symbols.cpp (original) +++ lldb/trunk/source/Host/macosx/Symbols.cpp Fri Jan 12 14:42:45 2018 @@ -333,28 +333,9 @@ static bool GetModuleSpecInfoFromUUIDDic std::string DBGBuildSourcePath; std::string DBGSourcePath; -cf_str = (CFStringRef)CFDictionaryGetValue((CFDictionaryRef)uuid_dict, - CFSTR("DBGBuildSourcePath")); -if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) { - CFCString::FileSystemRepresentation(cf_str, DBGBuildSourcePath); -} - -cf_str = (CFStringRef)CFDictionaryGetValue((CFDictionaryRef)uuid_dict, - CFSTR("DBGSourcePath")); -if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) { - CFCString::FileSystemRepresentation(cf_str, DBGSourcePath); -} - -if (!DBGBuildSourcePath.empty() && !DBGSourcePath.empty()) { - if (DBGSourcePath[0] == '~') { -FileSpec resolved_source_path(DBGSourcePath.c_str(), true); -DBGSourcePath = resolved_source_path.GetPath(); - } - module_spec.GetSourceMappingList().Append( - ConstString(DBGBuildSourcePath.c_str()), - ConstString(DBGSourcePath.c_str()), true); -} - +// If DBGVersion value 2 or higher, look for +// DBGSourcePathRemapping dictionary and append the key-value pairs +// to our remappings. cf_dict = (CFDictionaryRef)CFDictionaryGetValue( (CFDictionaryRef)uuid_dict, CFSTR("DBGSourcePathRemapping")); if (cf_dict && CFGetTypeID(cf_dict) == CFDictionaryGetTypeID()) { @@ -439,6 +420,32 @@ static bool GetModuleSpecInfoFromUUIDDic free(values); } } + + +// If we have a DBGBuildSourcePath + DBGSourcePath pair, +// append them to the source remappings list. + +cf_str = (CFStringRef)CFDictionaryGetValue((CFDictionaryRef)uuid_dict, + CFSTR("DBGBuildSourcePath")); +if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) { + CFCString::FileSystemRepresentation(cf_str, DBGBuildSourcePath); +} + +cf_str = (CFStringRef)CFDictionaryGetValue((CFDictionaryRef)uuid_dict, + CFSTR("DBGSourcePath")); +if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) { + CFCString::FileSystemRepresentation(cf_str, DBGSourcePath); +} + +if (!DBGBuildSourcePath.empty() && !DBGSourcePath.empty()) { + if (DBGSourcePath[0] == '~') { +FileSpec resolved_source_path(DBGSourcePath.c_str(), true); +DBGSourcePath = resolved_source_path.GetPath(); + } + module_spec.GetSourceMappingList().Append( + ConstString(DBGBuildSourcePath.c_str()), + ConstString(DBGSourcePath.c_str()), true); +} } return success; } Modified: lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp?rev=322418&r1=322417&r2=322418&view=diff == --- lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp (original) +++ lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp Fri Jan 12 14:42:45 2018 @@ -179,21 +179,6 @@ SymbolVendorMacOSX::CreateInstance(const std::string DBGBuildSourcePath; std::string DBGSourcePath; - plist.GetValueAsString("DBGBuildSourcePath", - DBGBuildSourcePath); - plist.GetValueAsString("DBGSourcePath", DBGSourcePath); - if (!DBGBuildSourcePath.empty() && - !DBGSourcePath.empty()) { -if (DBGSourcePath[0]
[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF
lhames added inline comments. Comment at: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2103 +static bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) { + // FIXME: This should detect covariant return types, but currently doesn't. aprantl wrote: > Could you add some doxygen comments explaining what the new function do and > why doing this is necessary? Will do. Comment at: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2105-2106 + // FIXME: This should detect covariant return types, but currently doesn't. + assert(&m1->getASTContext() == &m2->getASTContext() && + "Methods should have the same AST context"); + clang::ASTContext &context = m1->getASTContext(); clayborg wrote: > Use lldb_assert and possibly return false afterwards in case the asserts are > compiled out We can switch to lldb_assert to give us more control (at compile time) about when we turn it on or off, but I don't think we should bail out: this is an invariant, rather than a potential error case. Comment at: Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp:15 + Base *b = &d; + return 0; // Please test these expressions while stopped at this line: +} jingham wrote: > aprantl wrote: > > the expressions are missing :-) > > Perhaps convert this into an inline testcase? > The expressions are in the test .py file. It isn't necessary to put them > here, I'd just fix the comment. > > Please don't convert regular test cases into inline ones, however. The > benefit of inline test cases is mostly that they are easier to write, but > they are harder to debug when they go wrong. So if the are already in > regular form I'd rather not convert them. The text of the comment was just cribbed from one of the other expression tests. Is there a preferred phraseology? return 0; // run expressions here ? Repository: rL LLVM https://reviews.llvm.org/D41997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41428: [lldb] This commit adds support to cache a PDB's global scope and fixes a bug in getting the source file name for a compiland
asmith updated this revision to Diff 129747. asmith added a comment. Herald added a subscriber: llvm-commits. Added a lit unit test Repository: rL LLVM https://reviews.llvm.org/D41428 Files: lit/SymbolFile/PDB/Inputs/CompilandsTest.cpp lit/SymbolFile/PDB/compilands.test lit/SymbolFile/PDB/lit.local.cfg source/Plugins/SymbolFile/PDB/PDBASTParser.cpp source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp source/Plugins/SymbolFile/PDB/SymbolFilePDB.h Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h === --- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h +++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h @@ -16,6 +16,7 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/DebugInfo/PDB/IPDBSession.h" #include "llvm/DebugInfo/PDB/PDB.h" +#include "llvm/DebugInfo/PDB/PDBSymbolExe.h" class SymbolFilePDB : public lldb_private::SymbolFile { public: @@ -167,23 +168,34 @@ const llvm::pdb::IPDBSession &GetPDBSession() const; private: - lldb::CompUnitSP ParseCompileUnitForSymIndex(uint32_t id); + lldb::CompUnitSP + ParseCompileUnitForUID(uint32_t id, uint32_t index = UINT32_MAX); bool ParseCompileUnitLineTable(const lldb_private::SymbolContext &sc, uint32_t match_line); void BuildSupportFileIdToSupportFileIndexMap( - const llvm::pdb::PDBSymbolCompiland &cu, + const llvm::pdb::PDBSymbolCompiland &pdb_compiland, llvm::DenseMap &index_map) const; void FindTypesByName(const std::string &name, uint32_t max_matches, lldb_private::TypeMap &types); + void GetCompileUnitIndex(const llvm::pdb::PDBSymbolCompiland *pdb_compiland, + uint32_t &index); + + std::string GetSourceFileNameForPDBCompiland( + const llvm::pdb::PDBSymbolCompiland *pdb_compiland); + + std::unique_ptr + GetPDBCompilandByUID(uint32_t uid); + llvm::DenseMap m_comp_units; llvm::DenseMap m_types; std::vector m_builtin_types; std::unique_ptr m_session_up; + std::unique_ptr m_global_scope_up; uint32_t m_cached_compile_unit_count; std::unique_ptr m_tu_decl_ctx_up; }; Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp === --- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -18,6 +18,7 @@ #include "lldb/Symbol/LineTable.h" #include "lldb/Symbol/ObjectFile.h" #include "lldb/Symbol/SymbolContext.h" +#include "lldb/Symbol/SymbolVendor.h" #include "lldb/Symbol/TypeMap.h" #include "lldb/Utility/RegularExpression.h" @@ -43,6 +44,7 @@ #include +using namespace lldb; using namespace lldb_private; using namespace llvm::pdb; @@ -92,7 +94,8 @@ } SymbolFilePDB::SymbolFilePDB(lldb_private::ObjectFile *object_file) -: SymbolFile(object_file), m_cached_compile_unit_count(0) {} +: SymbolFile(object_file), m_session_up(), m_global_scope_up(), + m_cached_compile_unit_count(0), m_tu_decl_ctx_up() {} SymbolFilePDB::~SymbolFilePDB() {} @@ -152,41 +155,95 @@ void SymbolFilePDB::InitializeObject() { lldb::addr_t obj_load_address = m_obj_file->GetFileOffset(); + lldbassert(obj_load_address && + obj_load_address != LLDB_INVALID_ADDRESS); m_session_up->setLoadAddress(obj_load_address); + if (!m_global_scope_up) +m_global_scope_up = m_session_up->getGlobalScope(); + lldbassert(m_global_scope_up.get()); TypeSystem *type_system = GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus); ClangASTContext *clang_type_system = llvm::dyn_cast_or_null(type_system); + lldbassert(clang_type_system); m_tu_decl_ctx_up = llvm::make_unique( type_system, clang_type_system->GetTranslationUnitDecl()); } uint32_t SymbolFilePDB::GetNumCompileUnits() { if (m_cached_compile_unit_count == 0) { -auto global = m_session_up->getGlobalScope(); -auto compilands = global->findAllChildren(); +auto compilands = m_global_scope_up->findAllChildren(); +if (!compilands) + return 0; + +// The linker could link *.dll (compiland language = LINK), or import +// *.dll. For example, a compiland with name `Import:KERNEL32.dll` +// could be found as a child of the global scope (PDB executable). +// Usually, such compilands contain `thunk` symbols in which we are not +// interested for now. However we still count them in the compiland list +// so making access to them by index could be more easier. +// The other reason is that if we perform any compiland related activity, +// like finding symbols through llvm::pdb::IPDBSession methods, +// such compilands will be all searched automatically no matter whether +// we count them in or not. m_cached_compile_unit_count = compilands->getChildCount(); // The linker can inject an additional "dummy" compilation unit into the // PDB. Ignore this special compile u
[Lldb-commits] [PATCH] D41428: [lldb] This commit adds support to cache a PDB's global scope and fixes a bug in getting the source file name for a compiland
zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. It's nice that this turned out so easy. Didn't even require modifying `lldb-test`. Comment at: lit/SymbolFile/PDB/compilands.test:9 +CHECK: {{^[0-9A-F]+}}: SymbolVendor ([[CU]]) +CHECK-DAG: {{^[0-9A-F]+}}: CompileUnit{{[{]0x[0-9a-f]+[}]}}, language = "c++", file = '{{.*}}\CompilandsTest.cpp' If there's only one line then `CHECK-DAG` is equivalent to `CHECK`. Repository: rL LLVM https://reviews.llvm.org/D41428 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41428: [lldb] Add support to cache a PDB's global scope and fix a bug in getting the source file name for a compiland
asmith updated this revision to Diff 129750. asmith retitled this revision from "[lldb] This commit adds support to cache a PDB's global scope and fixes a bug in getting the source file name for a compiland" to "[lldb] Add support to cache a PDB's global scope and fix a bug in getting the source file name for a compiland". asmith edited the summary of this revision. asmith added a comment. Cleaned up some comments before commit Repository: rL LLVM https://reviews.llvm.org/D41428 Files: lit/SymbolFile/PDB/Inputs/CompilandsTest.cpp lit/SymbolFile/PDB/compilands.test lit/SymbolFile/PDB/lit.local.cfg source/Plugins/SymbolFile/PDB/PDBASTParser.cpp source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp source/Plugins/SymbolFile/PDB/SymbolFilePDB.h Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h === --- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h +++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h @@ -16,6 +16,7 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/DebugInfo/PDB/IPDBSession.h" #include "llvm/DebugInfo/PDB/PDB.h" +#include "llvm/DebugInfo/PDB/PDBSymbolExe.h" class SymbolFilePDB : public lldb_private::SymbolFile { public: @@ -167,23 +168,34 @@ const llvm::pdb::IPDBSession &GetPDBSession() const; private: - lldb::CompUnitSP ParseCompileUnitForSymIndex(uint32_t id); + lldb::CompUnitSP + ParseCompileUnitForUID(uint32_t id, uint32_t index = UINT32_MAX); bool ParseCompileUnitLineTable(const lldb_private::SymbolContext &sc, uint32_t match_line); void BuildSupportFileIdToSupportFileIndexMap( - const llvm::pdb::PDBSymbolCompiland &cu, + const llvm::pdb::PDBSymbolCompiland &pdb_compiland, llvm::DenseMap &index_map) const; void FindTypesByName(const std::string &name, uint32_t max_matches, lldb_private::TypeMap &types); + void GetCompileUnitIndex(const llvm::pdb::PDBSymbolCompiland *pdb_compiland, + uint32_t &index); + + std::string GetSourceFileNameForPDBCompiland( + const llvm::pdb::PDBSymbolCompiland *pdb_compiland); + + std::unique_ptr + GetPDBCompilandByUID(uint32_t uid); + llvm::DenseMap m_comp_units; llvm::DenseMap m_types; std::vector m_builtin_types; std::unique_ptr m_session_up; + std::unique_ptr m_global_scope_up; uint32_t m_cached_compile_unit_count; std::unique_ptr m_tu_decl_ctx_up; }; Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp === --- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -18,6 +18,7 @@ #include "lldb/Symbol/LineTable.h" #include "lldb/Symbol/ObjectFile.h" #include "lldb/Symbol/SymbolContext.h" +#include "lldb/Symbol/SymbolVendor.h" #include "lldb/Symbol/TypeMap.h" #include "lldb/Utility/RegularExpression.h" @@ -43,6 +44,7 @@ #include +using namespace lldb; using namespace lldb_private; using namespace llvm::pdb; @@ -92,7 +94,8 @@ } SymbolFilePDB::SymbolFilePDB(lldb_private::ObjectFile *object_file) -: SymbolFile(object_file), m_cached_compile_unit_count(0) {} +: SymbolFile(object_file), m_session_up(), m_global_scope_up(), + m_cached_compile_unit_count(0), m_tu_decl_ctx_up() {} SymbolFilePDB::~SymbolFilePDB() {} @@ -152,41 +155,93 @@ void SymbolFilePDB::InitializeObject() { lldb::addr_t obj_load_address = m_obj_file->GetFileOffset(); + lldbassert(obj_load_address && + obj_load_address != LLDB_INVALID_ADDRESS); m_session_up->setLoadAddress(obj_load_address); + if (!m_global_scope_up) +m_global_scope_up = m_session_up->getGlobalScope(); + lldbassert(m_global_scope_up.get()); TypeSystem *type_system = GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus); ClangASTContext *clang_type_system = llvm::dyn_cast_or_null(type_system); + lldbassert(clang_type_system); m_tu_decl_ctx_up = llvm::make_unique( type_system, clang_type_system->GetTranslationUnitDecl()); } uint32_t SymbolFilePDB::GetNumCompileUnits() { if (m_cached_compile_unit_count == 0) { -auto global = m_session_up->getGlobalScope(); -auto compilands = global->findAllChildren(); +auto compilands = m_global_scope_up->findAllChildren(); +if (!compilands) + return 0; + +// The linker could link *.dll (compiland language = LINK), or import +// *.dll. For example, a compiland with name `Import:KERNEL32.dll` +// could be found as a child of the global scope (PDB executable). +// Usually, such compilands contain `thunk` symbols in which we are not +// interested for now. However we still count them in the compiland list. +// If we perform any compiland related activity, like finding symbols +// through llvm::pdb::IPDBSession methods, such compilands will all be +// searched automatically no matter whether we i