Re: [Lldb-commits] [PATCH] D49309: No longer pass a StringRef to the Python API
If the implementation of a function requires a string - it should probably accept string (not a StringRef) as a parameter - to avoid back-and-forth conversions in cases where the caller already has a string to use. On Fri, Jul 13, 2018 at 12:43 PM Stella Stamenova via Phabricator via llvm-commits wrote: > stella.stamenova added a comment. > > All better now! Tests are passing. > > > Repository: > rL LLVM > > https://reviews.llvm.org/D49309 > > > > ___ > llvm-commits mailing list > llvm-comm...@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D51208: [DWARF] Fix dwarf5-index-is-used.cpp
On Tue, Aug 28, 2018 at 7:33 AM Greg Clayton via Phabricator < revi...@reviews.llvm.org> wrote: > clayborg added a comment. > > In https://reviews.llvm.org/D51208#1214743, @dblaikie wrote: > > > >> But if LLDB has different performance characteristics, or the default > should be different for other reasons - I'm fine with that. I think I left > it on for Apple so as not to mess with their stuff because of the > MachO/dsym sort of thing that's a bit different from the environments I'm > looking at. > > > > > > These are the numbers from my llvm-dev email in June: > > > > > >> setting a breakpoint on a non-existing function without the use of > > >> accelerator tables: > > >> real0m5.554s > > >> user0m43.764s > > >> sys 0m6.748s > > >> > > >> setting a breakpoint on a non-existing function with accelerator > tables: > > >> real0m3.517s > > >> user0m3.136s > > >> sys 0m0.376s > > > > > > This is an extreme case, > > > > What was being tested here? Is it a realistic case (ie: not a > pathalogical case with an absurd number of symbols, etc)? Using ELF? > Fission or not? > > > > How's it compare to GDB performance, I wonder? Perhaps LLDB hasn't been > optimized for the non-indexed case and could be - though that'd still > potentially motivate turning on indexes by default for LLDB until someone > has a motivation to do any non-indexed performance tuning/improvements. > > > The performance is huge for large binaries. LLDB doesn't need to manually > index all DWARF if the accelerator tables are there, it does if they are > not. Many people complain about the time it takes to index the DWARF, so > avoiding will be a huge improvement. We have some server binaries that > statically link in libc, libstdc++ and many other binaries and they are > currently 11GB, with over 10GB of that being DWARF. It makes debugging with > GDB and LLDB unusable when manual indexing of all that DWARF is needed. How long's the GDB startup/indexing time? (I take it this is without split DWARF?) & LLDB? Because I've observed some pretty large binaries without indexes & GDB startup times in the minute or so. > They have .debug_types enabled where they can on these binaries, but LTO > tends to be the culprit that can't take advantage of the .debug_types so > the binaries are still huge. > Not sure I follow - LTO should allow for even more compact debug info than debug_types (because LTO will merge the types before generating DWARF which means more compact debug info because there's no need to make the isolated/slicable chunks that debug_types requires), I think.. > GDB performance is hard to compare to due to the way they import types. > They import all types in a compile unit at the same time and have 3 layers > of resolution as they parse. Their indexes, if created by a linker, just > say "foo" exists in this compile unit somewhere, not the exact DIE offset, > so their indexes are useful, but they don't help LLDB out as much as the > DWARF 5 versions do. > Oh, yeah, I'm not suggesting LLDB should use GDB's indexes - but that if GDB can get reasonable performance without precomputed indexes then that might be an indication that LLDB is leaving some performance improvements on the table that might be worth investigating at some point. >> because practically the only thing we are doing is building the symbol > index, but it's nice for demonstrating the amount of work that lldb needs > to do without it. In practice, the ratio will not be this huge most of the > time, because we will usually find some matches, and then will have to do > some extra work, which will add a constant overhead to both sides. However, > this means that the no-accel case will take even longer. I am not sure how > this compares to gdb numbers, but I think the difference here is > significant. > >> > >> Also, I am pretty sure the Apple folks, who afaik are in the process of > switching to debug_names, will want to have them on by default for their > targets (ping @aprantl, @JDevlieghere). I think the cleanest way (and one > that best reflects the reality) to achieve that would be to have `-glldb` > imply `-gpubnames`. As for whether we should emit debug_names for DWARF 5 > by default (-gdwarf-5 => -gpubnames) is a more tricky question, and I don't > have a clear opinion on that (however, @probinson might). > >> > >> (In either case, I agree that there are circumstances in which having > debug_names is not beneficial, so having a flag to control them is a good > idea). > > > > *nod* Happy if you want to (or I can) have clang set pubnames on by > default when tuning for LLDB, while allowing -gno-pubnames to turn them > off. (& maybe we should have another alias for that flag, since debug_names > isn't "pubnames", but that's pretty low-priority) > > .debug_pubnames and .debug_pubtypes are not useful for LLDB or any > debugger in general so please don't enable for LLDB. Oh, sure - I didn't mean to suggest that, it's just the name of
Re: [Lldb-commits] [PATCH] D42386: Fix memory leak in TestClangASTContext.TestRecordHasFields
Any chance of using unique_ptr or other RAII/etc ownership to make this API safer by default? On Mon, Jan 22, 2018 at 10:58 AM Raphael Isemann via Phabricator via llvm-commits wrote: > This revision was not accepted when it landed; it landed in state "Needs > Review". > This revision was automatically updated to reflect the committed changes. > Closed by commit rL323138: Fix memory leak in > TestClangASTContext.TestRecordHasFields (authored by teemperor, committed > by ). > Herald added a subscriber: llvm-commits. > > Changed prior to commit: > https://reviews.llvm.org/D42386?vs=130926&id=130928#toc > > Repository: > rL LLVM > > https://reviews.llvm.org/D42386 > > Files: > lldb/trunk/unittests/Symbol/TestClangASTContext.cpp > > > Index: lldb/trunk/unittests/Symbol/TestClangASTContext.cpp > === > --- lldb/trunk/unittests/Symbol/TestClangASTContext.cpp > +++ lldb/trunk/unittests/Symbol/TestClangASTContext.cpp > @@ -11,6 +11,8 @@ > > #include "gtest/gtest.h" > > +#include "clang/AST/DeclCXX.h" > + > #include "lldb/Host/HostInfo.h" > #include "lldb/Symbol/ClangASTContext.h" > #include "lldb/Symbol/ClangUtil.h" > @@ -375,6 +377,9 @@ > empty_derived_non_empty_vbase_cxx_decl, false)); >EXPECT_TRUE( > > ClangASTContext::RecordHasFields(empty_derived_non_empty_vbase_decl)); > + > + delete non_empty_base_spec; > + delete non_empty_vbase_spec; > } > > TEST_F(TestClangASTContext, TemplateArguments) { > > > ___ > llvm-commits mailing list > llvm-comm...@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D35740: Fix PR33875 by distinguishing between DWO and clang modules
On Fri, Jul 21, 2017 at 4:05 PM Greg Clayton via Phabricator < revi...@reviews.llvm.org> wrote: > clayborg accepted this revision. > clayborg added a comment. > > Looks like there already is a test case that was failing as Jim mentioned. > Accepting based on that. > Ah, I was thinking more a test that would've failed when LLDB regressed (regardless of whether Clang was still producing this DWARF or not) - does LLDB have tests like that? (either binary, asm, or some other terse way of writing DWARF directly to test "does LLDB do the right thing with this DWARF" sort of tests?) > > > https://reviews.llvm.org/D35740 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D35740: Fix PR33875 by distinguishing between DWO and clang modules
On Fri, Jul 21, 2017 at 6:14 PM Jim Ingham wrote: > Not at present, but you presumably know more about this than I do. Part > of the point of Greg's extracting the DWARF parser from lldb and making it > into it's own library in llvm was precisely so that somebody could then > write a simple wrapper tool that would poke it with not necessarily > complete but interesting canned bits of DWARF and see that it does the > right thing. I thought you were involved with the reviews for that work? Yep yep - though not necessarily clear on the bigger picture goals in terms of which components were going where in the long term. > I was not paying attention to the details of that effort as DWARF > parsing's not really my thing. > > Anyway, the extraction of the DWARF parser was Greg's last act before > leaving Apple, and the project stalled at that point. I don't imagine he > could have gotten that code into llvm without some testing, so the kind of > test you are thinking of should be done using whatever mechanism you guys > devised for the new llvm dwarf parser. Adrian - any chance something like the DwarfGenerator stuff in LLVM could be used to test this code? > Of course, it's less interesting to test the llvm version of the DWARF > parser if lldb's not using it, so for that to be directly relevant here > that piece of work would need to be done. > Perhaps - or reusing the same testing approach without that. Though I think this particular failure/fix was in a higher/lower different layer than the pure parsing stuff in LLVM, but I could be wrong - there's sufficient divergence it's not obvious from a few class names, etc, to tell how much overlap (& where) there is. > > Jim > > > > > On Jul 21, 2017, at 5:51 PM, David Blaikie wrote: > > > > > > > > On Fri, Jul 21, 2017 at 4:05 PM Greg Clayton via Phabricator < > revi...@reviews.llvm.org> wrote: > > clayborg accepted this revision. > > clayborg added a comment. > > > > Looks like there already is a test case that was failing as Jim > mentioned. Accepting based on that. > > > > Ah, I was thinking more a test that would've failed when LLDB regressed > (regardless of whether Clang was still producing this DWARF or not) - does > LLDB have tests like that? (either binary, asm, or some other terse way of > writing DWARF directly to test "does LLDB do the right thing with this > DWARF" sort of tests?) > > > > > > > > https://reviews.llvm.org/D35740 > > > > > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D35740: Fix PR33875 by distinguishing between DWO and clang modules
On Sun, Jul 23, 2017 at 10:54 AM Adrian Prantl wrote: > On Jul 22, 2017, at 2:26 PM, David Blaikie wrote: > > > > On Fri, Jul 21, 2017 at 6:14 PM Jim Ingham wrote: > >> Not at present, but you presumably know more about this than I do. Part >> of the point of Greg's extracting the DWARF parser from lldb and making it >> into it's own library in llvm was precisely so that somebody could then >> write a simple wrapper tool that would poke it with not necessarily >> complete but interesting canned bits of DWARF and see that it does the >> right thing. I thought you were involved with the reviews for that work? > > > Yep yep - though not necessarily clear on the bigger picture goals in > terms of which components were going where in the long term. > > >> I was not paying attention to the details of that effort as DWARF >> parsing's not really my thing. >> >> Anyway, the extraction of the DWARF parser was Greg's last act before >> leaving Apple, and the project stalled at that point. I don't imagine he >> could have gotten that code into llvm without some testing, so the kind of >> test you are thinking of should be done using whatever mechanism you guys >> devised for the new llvm dwarf parser. > > > Adrian - any chance something like the DwarfGenerator stuff in LLVM could > be used to test this code? > > >> Of course, it's less interesting to test the llvm version of the DWARF >> parser if lldb's not using it, so for that to be directly relevant here >> that piece of work would need to be done. >> > > Perhaps - or reusing the same testing approach without that. Though I > think this particular failure/fix was in a higher/lower different layer > than the pure parsing stuff in LLVM, but I could be wrong - there's > sufficient divergence it's not obvious from a few class names, etc, to tell > how much overlap (& where) there is. > > > Yes, I would also say that this is one level above the pure parsing. This > is how LLDB interprets the data. Once the LLVM DWARF parser (which is > architected more for testability) is complete enough to be used inside > LLDB, there is no reason to not also implement this level > (cross-referencing dwarf+dwo) inside LLVM and properly test with a unit > test or a yaml object description. > Any reason this would need to wait until it's sunk into LLVM? It seems like this kind of testing would be useful to do in LLDB no matter how many libraries are sunk into LLVM - how far off/much work will it be to have this sort of testing available in LLDB? > Inside LLDB an end-to-end test like the existing one is as good as it gets > now. > > -- adrian > > > >> >> Jim >> >> >> >> > On Jul 21, 2017, at 5:51 PM, David Blaikie wrote: >> > >> > >> > >> > On Fri, Jul 21, 2017 at 4:05 PM Greg Clayton via Phabricator < >> revi...@reviews.llvm.org> wrote: >> > clayborg accepted this revision. >> > clayborg added a comment. >> > >> > Looks like there already is a test case that was failing as Jim >> mentioned. Accepting based on that. >> > >> > Ah, I was thinking more a test that would've failed when LLDB regressed >> (regardless of whether Clang was still producing this DWARF or not) - does >> LLDB have tests like that? (either binary, asm, or some other terse way of >> writing DWARF directly to test "does LLDB do the right thing with this >> DWARF" sort of tests?) >> > >> > >> > >> > https://reviews.llvm.org/D35740 >> > >> > >> > >> >> ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D56458: Fix unused private field warning.
Might be better to only define the variable if HAVE_LIBCOMPRESSION is defined? On Wed, Jan 9, 2019 at 8:58 AM Raphael Isemann via Phabricator via llvm-commits wrote: > This revision was not accepted when it landed; it landed in state "Needs > Review". > This revision was automatically updated to reflect the committed changes. > Closed by commit rL350675: Fix unused private field warning. (authored by > teemperor, committed by ). > Herald added a subscriber: llvm-commits. > > Changed prior to commit: > https://reviews.llvm.org/D56458?vs=180748&id=180750#toc > > Repository: > rL LLVM > > CHANGES SINCE LAST ACTION > https://reviews.llvm.org/D56458/new/ > > https://reviews.llvm.org/D56458 > > Files: > lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp > > > Index: > lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp > === > --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp > +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp > @@ -69,8 +69,9 @@ >m_echo_number(0), m_supports_qEcho(eLazyBoolCalculate), > m_history(512), >m_send_acks(true), m_compression_type(CompressionType::None), >m_listen_url(), m_decompression_scratch_type(CompressionType::None), > - m_decompression_scratch(nullptr) > -{ > + m_decompression_scratch(nullptr) { > + // Unused unless HAVE_LIBCOMPRESSION is defined. > + (void)m_decompression_scratch_type; > } > > //-- > > > ___ > llvm-commits mailing list > llvm-comm...@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r291200 - Fixes for Clang API changes to use std::shared_ptr
Author: dblaikie Date: Thu Jan 5 18:38:12 2017 New Revision: 291200 URL: http://llvm.org/viewvc/llvm-project?rev=291200&view=rev Log: Fixes for Clang API changes to use std::shared_ptr Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp?rev=291200&r1=291199&r2=291200&view=diff == --- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp (original) +++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp Thu Jan 5 18:38:12 2017 @@ -64,10 +64,10 @@ private: class ClangModulesDeclVendorImpl : public ClangModulesDeclVendor { public: ClangModulesDeclVendorImpl( - llvm::IntrusiveRefCntPtr &diagnostics_engine, - llvm::IntrusiveRefCntPtr &compiler_invocation, - std::unique_ptr &&compiler_instance, - std::unique_ptr &&parser); + llvm::IntrusiveRefCntPtr diagnostics_engine, + std::shared_ptr compiler_invocation, + std::unique_ptr compiler_instance, + std::unique_ptr parser); ~ClangModulesDeclVendorImpl() override = default; @@ -96,7 +96,7 @@ private: bool m_enabled = false; llvm::IntrusiveRefCntPtr m_diagnostics_engine; - llvm::IntrusiveRefCntPtr m_compiler_invocation; + std::shared_ptr m_compiler_invocation; std::unique_ptr m_compiler_instance; std::unique_ptr m_parser; size_t m_source_location_index = @@ -157,14 +157,14 @@ ClangModulesDeclVendor::ClangModulesDecl ClangModulesDeclVendor::~ClangModulesDeclVendor() {} ClangModulesDeclVendorImpl::ClangModulesDeclVendorImpl( -llvm::IntrusiveRefCntPtr &diagnostics_engine, -llvm::IntrusiveRefCntPtr &compiler_invocation, -std::unique_ptr &&compiler_instance, -std::unique_ptr &&parser) -: ClangModulesDeclVendor(), m_diagnostics_engine(diagnostics_engine), - m_compiler_invocation(compiler_invocation), +llvm::IntrusiveRefCntPtr diagnostics_engine, +std::shared_ptr compiler_invocation, +std::unique_ptr compiler_instance, +std::unique_ptr parser) +: m_diagnostics_engine(std::move(diagnostics_engine)), + m_compiler_invocation(std::move(compiler_invocation)), m_compiler_instance(std::move(compiler_instance)), - m_parser(std::move(parser)), m_imported_modules() {} + m_parser(std::move(parser)) {} void ClangModulesDeclVendorImpl::ReportModuleExportsHelper( std::set &exports, @@ -621,9 +621,9 @@ ClangModulesDeclVendor::Create(Target &t compiler_invocation_argument_cstrs.push_back(arg.c_str()); } - llvm::IntrusiveRefCntPtr invocation( + std::shared_ptr invocation = clang::createInvocationFromCommandLine(compiler_invocation_argument_cstrs, - diagnostics_engine)); + diagnostics_engine); if (!invocation) return nullptr; @@ -640,7 +640,7 @@ ClangModulesDeclVendor::Create(Target &t new clang::CompilerInstance); instance->setDiagnostics(diagnostics_engine.get()); - instance->setInvocation(invocation.get()); + instance->setInvocation(invocation); std::unique_ptr action(new clang::SyntaxOnlyAction); @@ -674,6 +674,7 @@ ClangModulesDeclVendor::Create(Target &t while (!parser->ParseTopLevelDecl(parsed)) ; - return new ClangModulesDeclVendorImpl(diagnostics_engine, invocation, + return new ClangModulesDeclVendorImpl(std::move(diagnostics_engine), +std::move(invocation), std::move(instance), std::move(parser)); } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r291198 - Make lldb -Werror clean for -Wstring-conversion
Author: dblaikie Date: Thu Jan 5 18:38:06 2017 New Revision: 291198 URL: http://llvm.org/viewvc/llvm-project?rev=291198&view=rev Log: Make lldb -Werror clean for -Wstring-conversion Also found/fixed one bug identified by this warning in RenderScriptx86ABIFixups.cpp where a string literal was being used in an effort to provide a name for an instruction/register, but was instead being passed as the bool 'isVolatile' parameter. Modified: lldb/trunk/include/lldb/Core/MappedHash.h lldb/trunk/source/Core/DataEncoder.cpp lldb/trunk/source/Core/ValueObjectMemory.cpp lldb/trunk/source/Expression/IRInterpreter.cpp lldb/trunk/source/Host/windows/EditLineWin.cpp lldb/trunk/source/Interpreter/OptionValueProperties.cpp lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ThreadKDP.cpp lldb/trunk/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/trunk/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp lldb/trunk/source/Symbol/ClangASTContext.cpp lldb/trunk/source/Symbol/Type.cpp lldb/trunk/source/Target/ABI.cpp lldb/trunk/source/Target/Platform.cpp lldb/trunk/source/Target/StackFrameList.cpp lldb/trunk/tools/debugserver/source/DNBDataRef.cpp lldb/trunk/tools/driver/Platform.cpp lldb/trunk/tools/lldb-perf/lib/Results.cpp Modified: lldb/trunk/include/lldb/Core/MappedHash.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/MappedHash.h?rev=291198&r1=291197&r2=291198&view=diff == --- lldb/trunk/include/lldb/Core/MappedHash.h (original) +++ lldb/trunk/include/lldb/Core/MappedHash.h Thu Jan 5 18:38:06 2017 @@ -52,8 +52,7 @@ public: default: break; } -assert(!"Invalid hash function index"); -return 0; +llvm_unreachable("Invalid hash function index"); } static const uint32_t HASH_MAGIC = 0x48415348u; Modified: lldb/trunk/source/Core/DataEncoder.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/DataEncoder.cpp?rev=291198&r1=291197&r2=291198&view=diff == --- lldb/trunk/source/Core/DataEncoder.cpp (original) +++ lldb/trunk/source/Core/DataEncoder.cpp Thu Jan 5 18:38:06 2017 @@ -258,8 +258,7 @@ uint32_t DataEncoder::PutMaxU64(uint32_t case 8: return PutU64(offset, value); default: -assert(!"GetMax64 unhandled case!"); -break; +llvm_unreachable("GetMax64 unhandled case!"); } return UINT32_MAX; } Modified: lldb/trunk/source/Core/ValueObjectMemory.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObjectMemory.cpp?rev=291198&r1=291197&r2=291198&view=diff == --- lldb/trunk/source/Core/ValueObjectMemory.cpp (original) +++ lldb/trunk/source/Core/ValueObjectMemory.cpp Thu Jan 5 18:38:06 2017 @@ -165,8 +165,7 @@ bool ValueObjectMemory::UpdateValue() { switch (value_type) { default: - assert(!"Unhandled expression result value kind..."); - break; + llvm_unreachable("Unhandled expression result value kind..."); case Value::eValueTypeScalar: // The variable value is in the Scalar value inside the m_value. Modified: lldb/trunk/source/Expression/IRInterpreter.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/IRInterpreter.cpp?rev=291198&r1=291197&r2=291198&view=diff == --- lldb/trunk/source/Expression/IRInterpreter.cpp (original) +++ lldb/trunk/source/Expression/IRInterpreter.cpp Thu Jan 5 18:38:06 2017 @@ -1602,25 +1602,23 @@ bool IRInterpreter::Interpret(llvm::Modu lldb::addr_t addr = tmp_op.ULongLong(); size_t dataSize = 0; - if (execution_unit.GetAllocSize(addr, dataSize)) { -// Create the required buffer -rawArgs[i].size = dataSize; -rawArgs[i].data_ap.reset(new uint8_t[dataSize + 1]); + bool Success = execution_unit.GetAllocSize(addr, dataSize); + (void)Success; + assert(Success && + "unable to locate host data for transfer to device"); + // Crea
[Lldb-commits] [lldb] r291199 - Fix -Wunused-function warning by preprocessor conditionalizing the function the same way as the caller
Author: dblaikie Date: Thu Jan 5 18:38:10 2017 New Revision: 291199 URL: http://llvm.org/viewvc/llvm-project?rev=291199&view=rev Log: Fix -Wunused-function warning by preprocessor conditionalizing the function the same way as the caller Modified: lldb/trunk/source/Plugins/Process/POSIX/CrashReason.cpp Modified: lldb/trunk/source/Plugins/Process/POSIX/CrashReason.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/POSIX/CrashReason.cpp?rev=291199&r1=291198&r2=291199&view=diff == --- lldb/trunk/source/Plugins/Process/POSIX/CrashReason.cpp (original) +++ lldb/trunk/source/Plugins/Process/POSIX/CrashReason.cpp Thu Jan 5 18:38:10 2017 @@ -21,6 +21,7 @@ void AppendFaultAddr(std::string &str, l str += ss.str(); } +#if defined(si_lower) && defined(si_upper) void AppendBounds(std::string &str, lldb::addr_t lower_bound, lldb::addr_t upper_bound, lldb::addr_t addr) { llvm::raw_string_ostream stream(str); @@ -37,6 +38,7 @@ void AppendBounds(std::string &str, lldb stream << ")"; stream.flush(); } +#endif CrashReason GetCrashReasonForSIGSEGV(const siginfo_t &info) { assert(info.si_signo == SIGSEGV); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r291204 - Revert part of cleanup to fix a build break
Author: dblaikie Date: Thu Jan 5 19:42:56 2017 New Revision: 291204 URL: http://llvm.org/viewvc/llvm-project?rev=291204&view=rev Log: Revert part of cleanup to fix a build break Wasn't sure I could include ErrorHandling.h here, and evidently I wasn't building this part (must've made the change using sed after getting tired of fixing each compilation error individually). Modified: lldb/trunk/tools/debugserver/source/DNBDataRef.cpp Modified: lldb/trunk/tools/debugserver/source/DNBDataRef.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/DNBDataRef.cpp?rev=291204&r1=291203&r2=291204&view=diff == --- lldb/trunk/tools/debugserver/source/DNBDataRef.cpp (original) +++ lldb/trunk/tools/debugserver/source/DNBDataRef.cpp Thu Jan 5 19:42:56 2017 @@ -115,13 +115,18 @@ uint32_t DNBDataRef::GetMax32(offset_t * switch (byte_size) { case 1: return Get8(offset_ptr); +break; case 2: return Get16(offset_ptr); +break; case 4: return Get32(offset_ptr); +break; default: -llvm_unreachable("GetMax32 unhandled case!"); +assert(!"GetMax32 unhandled case!"); +break; } + return 0; } //-- @@ -134,15 +139,21 @@ uint64_t DNBDataRef::GetMax64(offset_t * switch (size) { case 1: return Get8(offset_ptr); +break; case 2: return Get16(offset_ptr); +break; case 4: return Get32(offset_ptr); +break; case 8: return Get64(offset_ptr); +break; default: -llvm_unreachable("GetMax64 unhandled case!"); +assert(!"GetMax64 unhandled case!"); +break; } + return 0; } //-- ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r291198 - Make lldb -Werror clean for -Wstring-conversion
Ah, thanks - r291204 should hopefully do it, otherwise I might need some help. I'm assuming I'm not actually building this code - think I just ended up touching it after doing a bunch of manual cleanup then resorting to sed - so possibly ended up touching code that isn't building in my configuration. On Thu, Jan 5, 2017 at 5:02 PM Tim Hammerquist wrote: > Green Dragon build < > http://lab.llvm.org:8080/green/job/lldb_build_test/23854/> failed with > this commit with the error: > > CompileC > build/debugserver.build/Release/debugserver.build/Objects-normal/x86_64/DNBDataRef.o > source/DNBDataRef.cpp normal x86_64 c++ > com.apple.compilers.llvm.clang.1_0.compiler cd > "/Users/buildslave/jenkins/sharedspace/lldb@2/lldb/tools/debugserver" > export LANG=en_US.US-ASCII > /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang > -x c++ -arch x86_64 -fmessage-length=0 > -fdiagnostics-show-note-include-stack -fmacro-backtrace-limit=0 -std=c++11 > -stdlib=libc++ -Wno-trigraphs -fpascal-strings -Os -fno-common > -Wno-missing-field-initializers -Wno-missing-prototypes -Wunreachable-code > -Wno-non-virtual-dtor -Wno-overloaded-virtual -Wno-exit-time-destructors > -Wno-missing-braces -Wparentheses -Wswitch -Wunused-function > -Wno-unused-label -Wno-unused-parameter -Wunused-variable -Wunused-value > -Wempty-body -Wuninitialized -Wno-unknown-pragmas -Wno-shadow > -Wno-four-char-constants -Wno-conversion -Wconstant-conversion > -Wint-conversion -Wbool-conversion -Wenum-conversion -Wshorten-64-to-32 > -Wno-newline-eof -Wno-c++11-extensions -DLLDB_DEBUGSERVER_RELEASE -isysroot > /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk > -fasm-blocks -fstrict-aliasing -Wdeprecated-declarations -Winvalid-offsetof > -mmacosx-version-min=10.9 -g -fvisibility=hidden > -fvisibility-inlines-hidden -Wno-sign-conversion -Wno-infinite-recursion > -Wno-move > -I/Users/buildslave/jenkins/sharedspace/lldb@2/lldb/tools/debugserver/build/debugserver.build/Release/debugserver.build/debugserver.hmap > -Isource -I../../source > -I/Users/buildslave/jenkins/sharedspace/lldb@2/lldb/tools/debugserver/build/debugserver.build/Release/debugserver.build/DerivedSources > -I../../include > -I/Users/buildslave/jenkins/sharedspace/lldb@2/lldb/tools/debugserver/build/Release/include > -I/Users/buildslave/jenkins/sharedspace/lldb@2/lldb/tools/debugserver/build/debugserver.build/Release/debugserver.build/DerivedSources/x86_64 > -I/Users/buildslave/jenkins/sharedspace/lldb@2/lldb/tools/debugserver/build/debugserver.build/Release/debugserver.build/DerivedSources > -F/Users/buildslave/jenkins/sharedspace/lldb@2/lldb/tools/debugserver/build/Release > -F/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/PrivateFrameworks > -Wparentheses -DDT_VARIANT_ -DHAVE_LIBZ=1 -DLLDB_USE_OS_LOG=0 -MMD -MT > dependencies -MF > /Users/buildslave/jenkins/sharedspace/lldb@2/lldb/tools/debugserver/build/debugserver.build/Release/debugserver.build/Objects-normal/x86_64/DNBDataRef.d > --serialize-diagnostics > /Users/buildslave/jenkins/sharedspace/lldb@2/lldb/tools/debugserver/build/debugserver.build/Release/debugserver.build/Objects-normal/x86_64/DNBDataRef.dia > -c > /Users/buildslave/jenkins/sharedspace/lldb@2/lldb/tools/debugserver/source/DNBDataRef.cpp > -o > /Users/buildslave/jenkins/sharedspace/lldb@2/lldb/tools/debugserver/build/debugserver.build/Release/debugserver.build/Objects-normal/x86_64/DNBDataRef.o > /Users/buildslave/jenkins/sharedspace/lldb@2/lldb/tools/debugserver/source/DNBDataRef.cpp:123:5: > error: use of undeclared identifier 'llvm_unreachable' > llvm_unreachable("GetMax32 > unhandled case!"); ^ > /Users/buildslave/jenkins/sharedspace/lldb@2/lldb/tools/debugserver/source/DNBDataRef.cpp:144:5: > error: use of undeclared identifier 'llvm_unreachable' > llvm_unreachable("GetMax64 unhandled case!"); ^ 2 errors generated. > > On Thu, Jan 5, 2017 at 4:38 PM, David Blaikie via lldb-commits < > lldb-commits@lists.llvm.org> wrote: > > Author: dblaikie > Date: Thu Jan 5 18:38:06 2017 > New Revision: 291198 > > URL: http://llvm.org/viewvc/llvm-project?rev=291198&view=rev > Log: > Make lldb -Werror clean for -Wstring-conversion > > Also found/fixed one bug identified by this warning in > RenderScriptx86ABIFixups.cpp where a string literal was being used in an > effort to provide a name for an instruction/register, but was instead > being passed as the bool 'isVolatile' parameter. > > Modified: > lldb/trunk/include/lldb/Core/MappedHash.h > lldb/trunk/source/Core/DataEncoder.cpp > lldb/trunk
[Lldb-commits] [lldb] r291250 - Revert "Fixes for Clang API changes to use std::shared_ptr"
Author: dblaikie Date: Fri Jan 6 11:47:15 2017 New Revision: 291250 URL: http://llvm.org/viewvc/llvm-project?rev=291250&view=rev Log: Revert "Fixes for Clang API changes to use std::shared_ptr" The original Clang change caused a memory leak detected by asan. Reverting while I investigate. This reverts commit r291200. Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp?rev=291250&r1=291249&r2=291250&view=diff == --- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp (original) +++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp Fri Jan 6 11:47:15 2017 @@ -64,10 +64,10 @@ private: class ClangModulesDeclVendorImpl : public ClangModulesDeclVendor { public: ClangModulesDeclVendorImpl( - llvm::IntrusiveRefCntPtr diagnostics_engine, - std::shared_ptr compiler_invocation, - std::unique_ptr compiler_instance, - std::unique_ptr parser); + llvm::IntrusiveRefCntPtr &diagnostics_engine, + llvm::IntrusiveRefCntPtr &compiler_invocation, + std::unique_ptr &&compiler_instance, + std::unique_ptr &&parser); ~ClangModulesDeclVendorImpl() override = default; @@ -96,7 +96,7 @@ private: bool m_enabled = false; llvm::IntrusiveRefCntPtr m_diagnostics_engine; - std::shared_ptr m_compiler_invocation; + llvm::IntrusiveRefCntPtr m_compiler_invocation; std::unique_ptr m_compiler_instance; std::unique_ptr m_parser; size_t m_source_location_index = @@ -157,14 +157,14 @@ ClangModulesDeclVendor::ClangModulesDecl ClangModulesDeclVendor::~ClangModulesDeclVendor() {} ClangModulesDeclVendorImpl::ClangModulesDeclVendorImpl( -llvm::IntrusiveRefCntPtr diagnostics_engine, -std::shared_ptr compiler_invocation, -std::unique_ptr compiler_instance, -std::unique_ptr parser) -: m_diagnostics_engine(std::move(diagnostics_engine)), - m_compiler_invocation(std::move(compiler_invocation)), +llvm::IntrusiveRefCntPtr &diagnostics_engine, +llvm::IntrusiveRefCntPtr &compiler_invocation, +std::unique_ptr &&compiler_instance, +std::unique_ptr &&parser) +: ClangModulesDeclVendor(), m_diagnostics_engine(diagnostics_engine), + m_compiler_invocation(compiler_invocation), m_compiler_instance(std::move(compiler_instance)), - m_parser(std::move(parser)) {} + m_parser(std::move(parser)), m_imported_modules() {} void ClangModulesDeclVendorImpl::ReportModuleExportsHelper( std::set &exports, @@ -621,9 +621,9 @@ ClangModulesDeclVendor::Create(Target &t compiler_invocation_argument_cstrs.push_back(arg.c_str()); } - std::shared_ptr invocation = + llvm::IntrusiveRefCntPtr invocation( clang::createInvocationFromCommandLine(compiler_invocation_argument_cstrs, - diagnostics_engine); + diagnostics_engine)); if (!invocation) return nullptr; @@ -640,7 +640,7 @@ ClangModulesDeclVendor::Create(Target &t new clang::CompilerInstance); instance->setDiagnostics(diagnostics_engine.get()); - instance->setInvocation(invocation); + instance->setInvocation(invocation.get()); std::unique_ptr action(new clang::SyntaxOnlyAction); @@ -674,7 +674,6 @@ ClangModulesDeclVendor::Create(Target &t while (!parser->ParseTopLevelDecl(parsed)) ; - return new ClangModulesDeclVendorImpl(std::move(diagnostics_engine), -std::move(invocation), + return new ClangModulesDeclVendorImpl(diagnostics_engine, invocation, std::move(instance), std::move(parser)); } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r291271 - Reapply "Fixes for Clang API changes to use std::shared_ptr"
Author: dblaikie Date: Fri Jan 6 13:49:05 2017 New Revision: 291271 URL: http://llvm.org/viewvc/llvm-project?rev=291271&view=rev Log: Reapply "Fixes for Clang API changes to use std::shared_ptr" Aleksey Shlyapnikov found the memory leak I introduced, recommitted the Clang change with a fix for this. This reapplies r291200 reverted in r291250 Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp?rev=291271&r1=291270&r2=291271&view=diff == --- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp (original) +++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp Fri Jan 6 13:49:05 2017 @@ -64,10 +64,10 @@ private: class ClangModulesDeclVendorImpl : public ClangModulesDeclVendor { public: ClangModulesDeclVendorImpl( - llvm::IntrusiveRefCntPtr &diagnostics_engine, - llvm::IntrusiveRefCntPtr &compiler_invocation, - std::unique_ptr &&compiler_instance, - std::unique_ptr &&parser); + llvm::IntrusiveRefCntPtr diagnostics_engine, + std::shared_ptr compiler_invocation, + std::unique_ptr compiler_instance, + std::unique_ptr parser); ~ClangModulesDeclVendorImpl() override = default; @@ -96,7 +96,7 @@ private: bool m_enabled = false; llvm::IntrusiveRefCntPtr m_diagnostics_engine; - llvm::IntrusiveRefCntPtr m_compiler_invocation; + std::shared_ptr m_compiler_invocation; std::unique_ptr m_compiler_instance; std::unique_ptr m_parser; size_t m_source_location_index = @@ -157,14 +157,14 @@ ClangModulesDeclVendor::ClangModulesDecl ClangModulesDeclVendor::~ClangModulesDeclVendor() {} ClangModulesDeclVendorImpl::ClangModulesDeclVendorImpl( -llvm::IntrusiveRefCntPtr &diagnostics_engine, -llvm::IntrusiveRefCntPtr &compiler_invocation, -std::unique_ptr &&compiler_instance, -std::unique_ptr &&parser) -: ClangModulesDeclVendor(), m_diagnostics_engine(diagnostics_engine), - m_compiler_invocation(compiler_invocation), +llvm::IntrusiveRefCntPtr diagnostics_engine, +std::shared_ptr compiler_invocation, +std::unique_ptr compiler_instance, +std::unique_ptr parser) +: m_diagnostics_engine(std::move(diagnostics_engine)), + m_compiler_invocation(std::move(compiler_invocation)), m_compiler_instance(std::move(compiler_instance)), - m_parser(std::move(parser)), m_imported_modules() {} + m_parser(std::move(parser)) {} void ClangModulesDeclVendorImpl::ReportModuleExportsHelper( std::set &exports, @@ -621,9 +621,9 @@ ClangModulesDeclVendor::Create(Target &t compiler_invocation_argument_cstrs.push_back(arg.c_str()); } - llvm::IntrusiveRefCntPtr invocation( + std::shared_ptr invocation = clang::createInvocationFromCommandLine(compiler_invocation_argument_cstrs, - diagnostics_engine)); + diagnostics_engine); if (!invocation) return nullptr; @@ -640,7 +640,7 @@ ClangModulesDeclVendor::Create(Target &t new clang::CompilerInstance); instance->setDiagnostics(diagnostics_engine.get()); - instance->setInvocation(invocation.get()); + instance->setInvocation(invocation); std::unique_ptr action(new clang::SyntaxOnlyAction); @@ -674,6 +674,7 @@ ClangModulesDeclVendor::Create(Target &t while (!parser->ParseTopLevelDecl(parsed)) ; - return new ClangModulesDeclVendorImpl(diagnostics_engine, invocation, + return new ClangModulesDeclVendorImpl(std::move(diagnostics_engine), +std::move(invocation), std::move(instance), std::move(parser)); } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] Buildbot numbers for week of 11/22/2015 - 11/28/2015
Thanks for tracking these results, Galina I think it might be better to send these to the -dev lists, rather than the -commits lists, but I'm not sure what other people think on that. On Mon, Nov 30, 2015 at 4:17 PM, Galina Kistanova via cfe-commits < cfe-comm...@lists.llvm.org> wrote: > Hello everyone, > > Below are some buildbot numbers for the last week of 11/22/2015 - > 11/28/2015. > > Thanks > > Galina > > > > Top 10 fastest builders(not docs): > > lldb-amd64-ninja-freebsd11 > clang-bpf-build > llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast > libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03 > sanitizer-windows > libcxx-libcxxabi-x86_64-linux-ubuntu-cxx11 > libcxx-libcxxabi-x86_64-linux-ubuntu-cxx1z > libcxx-libcxxabi-x86_64-linux-ubuntu-cxx14 > libcxx-libcxxabi-x86_64-linux-ubuntu-unstable-abi > llvm-hexagon-elf > > > > 10 most slow builders: > > clang-native-arm-lnt-perf > clang-native-aarch64-full > clang-atom-d525-fedora > clang-cmake-thumbv7-a15-full-sh > perf-x86_64-penryn-O3 > perf-x86_64-penryn-O3-polly > perf-x86_64-penryn-O3-polly-before-vectorizer-detect-only > clang-cmake-armv7-a15-selfhost-neon > llvm-mips-linux > sanitizer-x86_64-linux-bootstrap > > > > Number of commits by project: > > project | commits > --- > llvm | 218 > cfe |67 > compiler-rt |33 > lld |30 > lldb |23 > polly | 8 > clang-tools-extra | 7 > libcxx| 4 > llgo | 1 > --- >391 > > > Number of completed builds, failed builds and average build time for > successful builds per active builder: > Maybe we could sort this \/ list by some ratio of failed/completed? I guess the entries with no entry in the failed column have zero failures? > > buildername | completed | > failed | time > > - > clang-aarch64-lnt | 56 > | 3 | 02:31:36 > clang-atom-d525-fedora| 21 > | 5 | 08:06:27 > clang-atom-d525-fedora-rel| 80 > | 12 | 01:31:08 > clang-bpf-build |239 > | 47 | 00:02:59 > clang-cmake-aarch64-42vma |214 > | 80 | 00:16:33 > clang-cmake-aarch64-full | 1 > || 03:17:48 > clang-cmake-aarch64-quick |178 > | 64 | 00:22:56 > clang-cmake-armv7-a15 |170 > | 9 | 00:27:14 > clang-cmake-armv7-a15-full|135 > | 31 | 00:47:54 > clang-cmake-armv7-a15-selfhost| 41 > | 17 | 04:13:48 > clang-cmake-armv7-a15-selfhost-neon | 38 > | 18 | 05:04:39 > clang-cmake-mips |158 > | 50 | 00:27:47 > clang-cmake-thumbv7-a15 |157 > | 22 | 00:27:57 > clang-cmake-thumbv7-a15-full-sh | 28 > | 12 | 06:13:18 > clang-hexagon-elf |139 > | 12 | 00:16:00 > clang-native-aarch64-full | 16 > | 5 | 08:14:29 > clang-native-arm-lnt |107 > | 17 | 01:08:19 > clang-native-arm-lnt-perf | 25 > | 11 | 08:40:48 > clang-ppc64-elf-linux |110 > | 3 | 01:01:45 > clang-ppc64-elf-linux2| 83 > | 14 | 01:31:11 > clang-sphinx-docs |111 > || 00:00:20 > clang-x64-ninja-win7 |158 > | 28 | 00:34:54 > clang-x86-win2008-selfhost|110 > | 17 | 01:02:17 > clang-x86_64-darwin13-cross-arm |197 > | 5 | 00:19:48 > clang-x86_64-darwin13-cross-mingw32 |184 > | 5 | 00:23:16 > clang-x86_64-debian-fast |137 > | 24 | 00:10:32 > clang-x86_64-linux-abi-test |243 > | 3 | 00:18:45 > clang-x86_64-linux-selfhost-modules
Re: [Lldb-commits] Buildbot numbers for week of 11/22/2015 - 11/28/2015
On Mon, Nov 30, 2015 at 5:00 PM, Galina Kistanova wrote: > Hi David, > > Thank you for the useful suggestions, I will work on these. > > >I guess the entries with no entry in the failed column have zero failures? > Yes. > > perf-x86_64-penryn-O3-polly-before-vectorizer | > 31 | 31 | > perf-x86_64-penryn-O3-polly-before-vectorizer-detect-only | > 31 | 27 | 05:28:12 > > These /\ two builders failed most of the time, but they also only ran a > handful of times. I guess they're triggering off polly changes only? I > wonder if they should trigger off LLVM changes too, but I'm not sure. > > They build on llvm changes also, but they both build for quite a long > time. I think it's the reason why they do not build more. > Ah, OK - so the other suggestions might help demonstrate/understand that behavior better. Tobias - any idea why these builders are /quite/ so slow & whether they could be improved? With large blame lists that come from a small number of slaves on a slow builder task it makes them hard to action. Are they useful to you? - Dave > > Thanks > > Galina > > > > > On Mon, Nov 30, 2015 at 4:28 PM, David Blaikie wrote: > >> Thanks for tracking these results, Galina >> >> I think it might be better to send these to the -dev lists, rather than >> the -commits lists, but I'm not sure what other people think on that. >> >> On Mon, Nov 30, 2015 at 4:17 PM, Galina Kistanova via cfe-commits < >> cfe-comm...@lists.llvm.org> wrote: >> >>> Hello everyone, >>> >>> Below are some buildbot numbers for the last week of 11/22/2015 - >>> 11/28/2015. >>> >>> Thanks >>> >>> Galina >>> >>> >>> >>> Top 10 fastest builders(not docs): >>> >>> lldb-amd64-ninja-freebsd11 >>> clang-bpf-build >>> llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast >>> libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03 >>> sanitizer-windows >>> libcxx-libcxxabi-x86_64-linux-ubuntu-cxx11 >>> libcxx-libcxxabi-x86_64-linux-ubuntu-cxx1z >>> libcxx-libcxxabi-x86_64-linux-ubuntu-cxx14 >>> libcxx-libcxxabi-x86_64-linux-ubuntu-unstable-abi >>> llvm-hexagon-elf >>> >>> >>> >>> 10 most slow builders: >>> >>> clang-native-arm-lnt-perf >>> clang-native-aarch64-full >>> clang-atom-d525-fedora >>> clang-cmake-thumbv7-a15-full-sh >>> perf-x86_64-penryn-O3 >>> perf-x86_64-penryn-O3-polly >>> perf-x86_64-penryn-O3-polly-before-vectorizer-detect-only >>> clang-cmake-armv7-a15-selfhost-neon >>> llvm-mips-linux >>> sanitizer-x86_64-linux-bootstrap >>> >>> >>> >>> Number of commits by project: >>> >>> project | commits >>> --- >>> llvm | 218 >>> cfe |67 >>> compiler-rt |33 >>> lld |30 >>> lldb |23 >>> polly | 8 >>> clang-tools-extra | 7 >>> libcxx| 4 >>> llgo | 1 >>> --- >>>391 >>> >>> >>> Number of completed builds, failed builds and average build time for >>> successful builds per active builder: >>> >> >> Maybe we could sort this \/ list by some ratio of failed/completed? >> >> I guess the entries with no entry in the failed column have zero failures? >> >> >>> >>> buildername | completed >>> | failed | time >>> >>> - >>> clang-aarch64-lnt | 56 >>> | 3 | 02:31:36 >>> clang-atom-d525-fedora| 21 >>> | 5 | 08:06:27 >>> clang-atom-d525-fedora-rel| 80 >>> | 12 | 01:31:08 >>> clang-bpf-build |239 >>> | 47 | 00:02:59 >>> clang-cmake-aarch64-42vma |214 >>> | 80 | 00:16:33 >>> clang-cmake-aarch64-full | 1 >>> || 03:17:48 >>> clang-cmake-aarch64-quick |178 >>> | 64 | 00:22:56 >>> clang-cmake-armv7-a15 |170 >>> | 9 | 00:27:14 >>> clang-cmake-armv7-a15-full|135 >>> | 31 | 00:47:54 >>> clang-cmake-armv7-a15-selfhost| 41 >>> | 17 | 04:13:48 >>> clang-cmake-armv7-a15-selfhost-neon | 38 >>> | 18 | 05:04:39 >>> clang-cmake-mips |158 >>> | 50 | 00:27:47 >>> clang-cmake-thumbv7-a15 |157 >>> | 22 | 00:27:57 >>> clang-cmake-thumbv7-a15-full-sh
Re: [Lldb-commits] [PATCH] D24369: [support] copying JSON parser/writer from lldb
On Fri, Sep 9, 2016 at 7:31 PM Mike Aizatsky wrote: > aizatsky abandoned this revision. > > > Comment at: include/llvm/Support/JSON.h:207 > @@ +206,3 @@ > +private: > + typedef std::map Map; > + typedef Map::iterator Iterator; > > zturner wrote: > > `DenseMap` seems like a better choice here. > DenseMap doesn't seem to be defined for std::string keys. > > > Comment at: include/llvm/Support/JSON.h:238 > @@ +237,3 @@ > + > + JSONValue::SP getObject(Index I); > + > > zturner wrote: > > How about providing an overloaded `operator[]` and providing `begin()` > and `end()` functions so that it can be used in a range based for syntax? > done for operator[]. > > begin() and end() will be tricky, because underlying storage has > std::unique_ptr<> > see llvm::pointee_iterator in include/llvm/ADT/iterator.h > > > Comment at: include/llvm/Support/JSON.h:282-283 > @@ +281,4 @@ > + > + std::string Buffer; > + size_t Index; > +}; > > zturner wrote: > > Instead of having a `std::string` here, which will copy the input JSON, > I would make this a `StringRef`. Then you can delete the `Index` field as > well, because the internal `StringRef` itself could always point to the > beginning of the next char by using a `Buffer = Buffer.drop_front(N)` like > pattern. This won't work if you ever have to do back-referencing across > functions, but I don't think that is needed. Also if `Buffer` is a > `StringRef`, a lot of the methods like `peekChar()` and `skipSpaces` become > trivial one-liners that you wouldn't even need to provide a separate > function for. > `std::string => StringRef` - done. > > As for removing `Index` - I'm not sure. It is currently used in error > reporting and is the only feedback we give when we encounter an error. > > > Comment at: lib/Support/JSON.cpp:53 > @@ +52,3 @@ > + case DataType::Signed: > +return (uint64_t)Data.Signed; > + case DataType::Double: > > zturner wrote: > > Can you use C++ style casting operators here and in other similar > functions? > Removed all 3 them and added get method. > > > https://reviews.llvm.org/D24369 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D13066: DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child
dblaikie added a subscriber: dblaikie. Comment at: test/lang/cpp/incomplete-types/Makefile:8 @@ +7,3 @@ + +ifneq (,$(findstring clang,$(CC))) + CFLAGS_LIMIT += -flimit-debug-info In case it's interesting, you can get similarly problematic DWARF by using a dynamic class (one with virtual functions) with a key function that is not defined in the current translation unit. GCC implements this behavior as well (whereas GCC doesn't implement the behavior that is triggering on basic_string involving explicit instantiation declarations/definitions) and also has a flag for it: -femit-class-debug-always (I think last I recall, Eric Christopher mentioned he'd looked at the GCC implementation of this flag and it differed in some ways from Clang's, so he was reticent to add a compatibility flag for this in Clang... but we could discuss/revisit that, perhaps (though I suppose it wouldn't allow you to /enable/ this optimization, only disable it - not sure if there's a way to opt-in in GCC)) Though it's easy to "disable" the feature by simply not triggering it, rather than using a flag to turn it off - eg: providing no key function for a type (if you're triggering the dynamic class case, not the template case), or removing an explicit instantiation declaration/definition (if you're triggering the template case) Comment at: test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py:5 @@ +4,3 @@ + +class TestCppIncompleteTypes(TestBase): + You don't seem to have a test case for the shared library case in the bug report? (decl in one obj, def in the other, shared library/object/whatever boundary between the two) (& not sure what the behavior is in the case where the def is provided in one obj and they are statically linked together - bug report mentions the failure doesn't occur, but I don't know if it does the right thing (actually finds the definition) - I know lldb used to not do the right thing there, and had some error message about the compiler being wrong) Comment at: test/lang/cpp/incomplete-types/length.h:4 @@ +3,3 @@ + +#include + Having a test case depend on the entire header of seems a bit brittle - and means this test may or may not test the behavior you're interested in, depending on how the standard library is defined (depending on whether it actually has an explicit instantiation decl/def for basic_string or not) http://reviews.llvm.org/D13066 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Unique enums parsed from declarations (PR #96751)
@@ -1798,6 +1805,13 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, } if (def_die) { +if (auto [it, inserted] = dwarf->GetDIEToType().try_emplace( dwblaikie wrote: any chance of reducing/avoiding this duplication? Looks like the same code here as above? (refactor into a common function?) https://github.com/llvm/llvm-project/pull/96751 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
dwblaikie wrote: > It looks like the test is flaky: > https://lab.llvm.org/buildbot/#/builders/59/builds/535 > > It looks like the order of the types is nondeterministic. I think you should > be able to use CHECK-DAG along with [this trick to match > newlines](https://llvm.org/docs/CommandGuide/FileCheck.html#matching-newline-characters) > to make the test accept both orderings. What are lldb's guarantees in this regard? Clang/LLVM/etc require nondeterministic output - presumably the same would be valuable to lldb, but I don't know what lldb's precedents are in this regard. https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
dwblaikie wrote: Ah, I think this right solution to /this/ case is that the compiler should be emitting the alignment for the structure? Like there's no way for the debugger to know that this structure is underaligned at the moment: ``` struct __attribute__((packed)) t1 { int i; }; ``` Which means the debugger might generate code that assumes `t1` is naturally aligned, and break (ARM crashes on underaligned operations, doesn't it? so if you got a pointer to t1 back from some API that happened to have put it in an underaligned location - then in your expression you tried to read `i`, this could generate crashing code?) Essentially the `packed` attribute on the above structure is providing the same value/effect as `aligned(1)` and should produce the same DWARF, but it doesn't. https://github.com/llvm/llvm-project/pull/93809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
dwblaikie wrote: oh, slight misquote above - `aligned(1)` is a no-op, as that attribute can't reduce the alignment... But I think my argument still stands, roughly as - if increases in alignment are explicitly specified, decreases in alignment should be too. https://github.com/llvm/llvm-project/pull/93809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
dwblaikie wrote: The other effects of `packed` are encoded (the changes to the member offsets) but that's not the only effect, and we shouldn't use that effect (which isn't guaranteed to be observable - as in the case of "packed struct with a single member/that just happens to not have padding anyway") to try to divine the alignment. https://github.com/llvm/llvm-project/pull/93809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
dwblaikie wrote: Here's the smallest patch that would put explicit alignment on any packed structure: ``` diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index a072475ba770..bbb13ddd593b 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -64,7 +64,7 @@ static uint32_t getTypeAlignIfRequired(const Type *Ty, const ASTContext &Ctx) { // MaxFieldAlignmentAttr is the attribute added to types // declared after #pragma pack(n). if (auto *Decl = Ty->getAsRecordDecl()) -if (Decl->hasAttr()) +if (Decl->hasAttr() || Decl->hasAttr()) return TI.Align; return 0; ``` But I don't think that's the right approach - I think what we should do is compute the natural alignment of the structure, then compare that to the actual alignment - and if they differ, we should put an explicit alignment on the structure. This avoids the risk that other alignment-influencing effects might be missed (and avoids the case of putting alignment on a structure that, when packed, just has the same alignment anyway - which is a minor issue, but nice to get right (eg: packed struct of a single char probably shouldn't have an explicit alignment - since it's the same as the implicit alignment anyway)) https://github.com/llvm/llvm-project/pull/93809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
dwblaikie wrote: Oh, this code was touched really recently in 66df6141659375e738d9b9b74bf79b2317576042 (@augusto2112 ) (see notes in previous comments about how this code should be generalized) https://github.com/llvm/llvm-project/pull/93809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Add tests for alignof on class with overlapping bases (PR #97068)
@@ -0,0 +1,30 @@ +// XFAIL: * + +// RUN: %clangxx_host -gdwarf -o %t %s +// RUN: %lldb %t \ +// RUN: -o "expr alignof(OverlappingDerived)" \ +// RUN: -o "expr sizeof(OverlappingDerived)" \ +// RUN: -o exit | FileCheck %s + +// CHECK: (lldb) expr alignof(OverlappingDerived) +// CHECK-NEXT: ${{.*}} = 4 dwblaikie wrote: Might be nice to put the checks next to the static_asserts? https://github.com/llvm/llvm-project/pull/97068 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DataFormatter][NFC] Factor out MapIterator logic into separate helper (PR #97544)
https://github.com/dwblaikie approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/97544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
dwblaikie wrote: What's an actual test case for this issue? The example given above doesn't look like it'd produce entries for the declaration of ios_base? Like here's something that looks equivalent, but is complete, and doesn't have a DW_IDX_parent on the nested typedef entry in the index: https://godbolt.org/z/efjbze3x1 it'd be good to have a reproducer for this to motivate the discussion... https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
dwblaikie wrote: & FWIW, I think it is valid to include these declarations as entries, though not as named/index entries, per the spec: > It is possible that an indexed debugging information entry has a parent that > is not indexed (for example, if its parent does not have a name attribute). > In such a case, a parent attribute may point to a nameless index entry (that > is, one that cannot be reached from any entry in the name table), or it may > point to the nearest ancestor that does have an index entry So I think you could have a parent that's a declaration, represented as a nameless index entry. That'd allow faster comparisons when examining the entry for the named child that had such a parent - because you could potentially check that named entry's fully qualified name directly from the named entry and the parents - without needing to parse all the DIEs in the CU to walk and find the start of the parents. But I don't believe clang is producing such DWARF today. https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
dwblaikie wrote: Sorry I can't quite figure out who/where to reply to, so perhaps by summarizing the situation (though it's going to be verbose, and I'm going to add some other examples/complications) I can get a grip on all this/clarify where we're at. So, history. * minimum spec-compliant .debug_names (DWARFv5, 6.1.1 Lookup by Name) implementation contains a name entry for each unqualified name which contains index entries for each definition DIE with that unqualified name (I'm glossing over all the pedantic language in the spec - can read that if anyone's interested) * the original [Apple names](https://llvm.org/docs/SourceLevelDebugging.html#name-accelerator-tables) also included a hash of the qualified name to check that the exact name is found, but that relies on textually identical names, which isn't portable/wouldn't work for humans writing names in a variety of ways - so the standardized version allowed producers to include a DW_IDX_parent link up to the (hmm, I can't find any mention of the "hash of the fully qualified name" part in the llvm.org docs, so perhaps they're out of date - yeah, it's in the code, DW_ATOM_qual_name_hash but now I just have further question (it seems this data's only generated for dsyms, not tables in .o files)) * having only unqualified names in the index with no further context requires a fair bit of DWARF deserialization to do fully or partially qualified name lookup - got to walk the whole DIE tree of the CU to find the unqualified DIEs parents to check them against the qualified name * so @felipepiovezan and others worked on adding DW_IDX_parent support to LLVM's .debug_names emission code, so that qualified names could be compared using the parent chain - deserializing just the specifically targeted DIEs/basically allowing quick access to the parent chain of the unqualified DIE * One complication of DW_IDX_parent implementation is how to detect the difference between an index name entry that describes an entity in the global namespace (ie: truly has no qualifying name needed to reach/identify it) and another entry that a producer didn't choose to emit DW_IDX_parent for. * The way this was addressed was to use `DW_IDX_parent` with `DW_FORM_flag_present` to indicate that this DIE's qualified and unqualified name are identical, it has no qualifying parent * The DWARF spec does allow unnamed entries in the index - not in the name index (because that's the list of non-empty names), but in the index entry list * The DWARF spec does say that if a named entity's parent isn't indexed, perhaps because it's unnamed (unnamed entities don't go in the /name/ index) then a producer might reference an unnamed index entry, or might skip the entity and reference its named parent. * The /intent/ of this wording I think is clear, to allow consumers to still do qualified name lookup directly from an index-with-parents even if there are some unnamed scopes present - but the language is too loose because there can be named entities that aren't present in the index (because they aren't definitions) that, if skipped, would make the index basically unusable - the consumer wouldn't know they were skipped and would treat those name components as not-present, so "foo::bar::baz" would appear to a consumer reading the index as "foo::baz" and a query for "foo::bar::baz" would be rejected. I think the cases where these missing parents come up are most likely not present for Apple, the two places I can think of are the one we're currently discussing, type units - and `-fno-standalone-debug` situations. The latter might be more informative as an example to consider? ``` struct t1 { virtual void f1(); struct t2 { }; }; t1::t2 v1; ``` ``` $ llvm-dwarfdump-tot missing_parent.o | grep "DW_TAG\|DW_AT_name\|DW_AT_declaration" | sed -e "s/^.{12}//" 0x000c: DW_TAG_compile_unit DW_AT_name("missing_parent.cpp") 0x001e: DW_TAG_variable DW_AT_name ("v1") 0x0029: DW_TAG_structure_type DW_AT_name ("t1") DW_AT_declaration (true) 0x002b: DW_TAG_structure_type DW_AT_name("t2") ``` ``` $ llvm-dwarfdump-tot missing_parent.o -debug-names "v1" Tag: DW_TAG_variable DW_IDX_die_offset: 0x001e DW_IDX_parent: "t2" Tag: DW_TAG_structure_type DW_IDX_die_offset: 0x002b ``` In this case I think we currently can't do qualified lookup of `t1::t2` using just the index. We'd have to go and at least quick-parse the whole CU to find `t2`'s parent(s) and check them. In the case where `t1` is defined in the same translation unit/CU (and we put `DW_IDX_parent` on the `t2` entry) we still /probably/ have to parse the `t1` DIE, because the entry offset we'd emit wouldn't tell us the name of `t1`. Unless we reverse the name entry->index entry mapping to find the name? (@felipepiovezan can you co
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
dwblaikie wrote: > "std::ios_base" is forward declared and it contains typedefs whose entries in > the .debug_names table will point to the DIE at offset 0x0090cdd5. These > entries cause our type lookups to try and parse a TON of forward declarations > and waste time and resources. > > This fix makes sure when/if we find an entry in the .debug_names table, we > don't process it if it has a DW_AT_declaration(true) attribute. To come back to this - the table consists of buckets, buckets (a list of indexes into a hash table, one hash per string), then a string table (string offsets into .debug_str and entry offsets). The entry offsets point into the entry pool which is where the tag and DW_IDX values are. Each string entry points to a sequence of index entries, terminated by a zero abbrev code. But there can be index entries that aren't referenced by string entries - they could appear anywhere in the entry pool, just not in a sequence pointed to by a name entry (because if they were there, they'd be included in that string's entries). https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
dwblaikie wrote: As for the original issue this patch is meant to address - I'd still like to see an example of the problem, as I'm not sure what the issue is. (but, equally, I'm not a decider in lldb - so don't have to wait on my for approval if other lldb devs reckon this is the right direction) > "std::ios_base" is forward declared and it contains typedefs whose entries in > the .debug_names table will point to the DIE at offset 0x0090cdd5. These > entries cause our type lookups to try and parse a TON of forward declarations > and waste time and resources. Yeah, so far as I know if the parent of the typedef isn't indexed (because it's a declaration that isn't a definition) we don't put a DW_IDX_parent on the typedef. It wouldn't be totally wrong for a producer to do this, though they shouldn't put the declaration of the enclosing/outer type in a /named/ entry, just in an unnamed index entry. But if they did, why would that cause bad performance? The name lookup should check the referenced parent DIE to see what its name is - why would there be a lookup of everything with that name? Or you mean all these declarations end up in the table, and any lookup for the name is inefficient because it finds lots of declarations so there's just loads more results than we want? Yeah - if some producer did mistakenly put declarations in named entries in the debug_names table, that'd be bad - filtering them out in the debugger's not the worst thing, or just erroring out & saying the table's bogus (maybe ignoring the table entirely). Not sure. https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
dwblaikie wrote: But seems like this isn't a DW_IDX_parent issue, then (other than as /maybe/ an artifact of the producer) - so the description seems misleading. "ignore invalid named entries that refer to declarations" A test case/demonstration this is a real problem would be nice, though... https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
dwblaikie wrote: Could probably adjust the assertions to be `assert (debug || whatever)` rather than `if (!debug) assert(whatever)`. My understanding/expectation was that these assertions would be removed entirely - that whatever generated the AST should just be trusted, whether it's the debugger or the compiler frontend... but I'll certainly leave it up to @AaronBallman as to where he thinks the right tradeoff/sweetspot is. https://github.com/llvm/llvm-project/pull/93809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve performance of .debug_names lookups when DW_IDX_parent attributes are used (PR #91808)
dwblaikie wrote: One easy question would be: do you/your users use -fdebug-types-section? If so, that'd probably explain what you were seeing & you could add some test coverage for that wherever you like (in lldb, presumably, maybe in bolt too). But if you/they don't, then it's unclear where this bad index came from - and it'd be good to know more about the DWARF so we could figure out where else there might be problems like this... https://github.com/llvm/llvm-project/pull/91808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
dwblaikie wrote: > > > You can repro this by running ./bin/lldb-dotest -p > > > TestConstStaticIntegralMember.py --dwarf-version 5 > > > Could you take a look @ZequanWu ? > > > > > > It should be fixed by the following diff. We just need to skip the > > declaration DIEs that are struct/class/union. This failure you see is > > caused by skipping a declaration DIE `DW_TAG_variable`. > > ``` > > diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp > > b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp > > index 56717bab1ecd..6330470b970e 100644 > > --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp > > +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp > > @@ -87,7 +87,8 @@ bool DebugNamesDWARFIndex::ProcessEntry( > > return true; > >// Clang erroneously emits index entries for declaration DIEs in case > > when the > >// definition is in a type unit (llvm.org/pr77696). Weed those out. > > - if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0)) > > + if (die.IsStructUnionOrClass() && > > + die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0)) > > return true; > >return callback(die); > > } > > ``` > > > > > > > > > > > > > > > > > > > > > > > > Can you (or anyone with commit access) commit above fix for me? I somehow > > cannot pull/push from github. > > Ah right, I remember encountering these `DW_AT_declaration`s in the index for > DWARFv5 static member variables. Technically this isn't standard conforming > right? I thought anything `DW_AT_declaration` shouldn't be indexed (though in > the case of these static variables it's very useful that they do, because we > don't emit definitions for these constants that the debugger could look for > instead). @dwblaikie @felipepiovezan Should we allow such entries in the > index? And does this warrant rephrasing in the DWARF spec? At least we aren't producing that on x86 from the compiler: https://godbolt.org/z/ereKsasWf ``` ... 0x0029: DW_TAG_variable DW_AT_name("i") DW_AT_type(0x0033 "const int") DW_AT_decl_file ("/app/example.cpp") DW_AT_decl_line (2) DW_AT_external(true) DW_AT_declaration (true) DW_AT_const_value (3) ... ``` and the `.debug_names` only includes `t1`, `main`, and `int`, nothing for `i`. Ah, right - some of the previous context on this is https://github.com/llvm/llvm-project/pull/70639 (which got reverted in https://github.com/llvm/llvm-project/pull/74580 )- though I guess we probably had some discourse and dwarf workgroup discussions about how to do this that aren't likned from the PR... maybe it was all private chat, not sure. https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb] Return an llvm::Expected from DWARFExpression::Evaluate (NFCI) (PR #94420)
@@ -2341,9 +2198,7 @@ bool DWARFExpression::Evaluate( // the stack by the called expression may be used as return values by prior // agreement between the calling and called expressions. case DW_OP_call2: - if (error_ptr) -error_ptr->SetErrorString("Unimplemented opcode DW_OP_call2."); - return false; + return llvm::createStringError("Unimplemented opcode DW_OP_call2."); dwblaikie wrote: and probably drop the uppercase at the start too? https://github.com/llvm/llvm-project/pull/94420 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [WIP][lldb] Use forward decls with redeclared definitions instead of minimal import for records (PR #95100)
dwblaikie wrote: Yeah, putting this behind a very-explicitly-temporary flag seems reasonable to me, FWIW. & yeah, the analogy to modules re: member function expansion seems relevant as far as I understand things too. https://github.com/llvm/llvm-project/pull/95100 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [WIP][lldb] Use forward decls with redeclared definitions instead of minimal import for records (PR #95100)
dwblaikie wrote: > Good question, I haven't dug into the modules import mechanism yet, but if > this is true, then that could give us an opportunity to introduce a "lazy > method loading" mechanism on the ExternalASTSource that would fit both LLDB > and modules? Will try to confirm this (unless someone else knows the answer > to this already, e.g., @dwblaikie). Yeah, I don't have any great info here - only the vague assumption/understanding that this path was more modules-aligned, and so perf improvements for lldb could also be perf improvements (& test coverage) for modules... - but I could be wrong on that conception. https://github.com/llvm/llvm-project/pull/95100 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
dwblaikie wrote: > Oh, in this particular case, the issue isn't the computed datasize, it's that > FieldDecl::isZeroSize() returns the wrong result. If that's the case, maybe > we can just change FieldDecl::isZeroSize() to say the field is zero size? So > essentially, we pretend all empty fields are no_unique_address. Nothing in > codegen should care if we treat an non-zero-size empty field as if it's > zero-size. (sorry if I'm jumping in without enough context... ) - couldn't the inverse be true, then - that codegen should ignore if something `isZeroSize` or not? So lldb doesn't have to put any particular value here? https://github.com/llvm/llvm-project/pull/93809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)
dwblaikie wrote: > That would mean if someone wrote `struct Empty {}; struct Z { Empty a,b,c; > }`, we'd lower it to `{ [3 x i8] }` instead of `{%Empty, %Empty, %Empty}`, > which is a bit ugly. Other than that, sure, I guess we could do that. Ah, fair enough. Glad to understand and I don't feel /super/ strongly either way. Though it might help with confidence if codegen didn't depend on this property at all (that it depends on the property a bit may make it harder to detect if later codegen depends on the property in a real/ABI-breaking way). The struct layout validation stuff that @Michael137 found may be adequate to provide confidence (especially combined with fuzzing, maybe) without the need for the codegen-is-zero-length-independent invariant. I don't feel too strongly - mostly happy with whatever Clang owners are happy with. https://github.com/llvm/llvm-project/pull/93809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Fix type definition search with simple template names (PR #95905)
@@ -3073,14 +3073,43 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) { // See comments below about -gsimple-template-names for why we attempt to // compute missing template parameter names. -ConstString template_params; -if (type_system) { - DWARFASTParser *dwarf_ast = type_system->GetDWARFParser(); - if (dwarf_ast) -template_params = dwarf_ast->GetDIEClassTemplateParams(die); +std::vector template_params; +DWARFDeclContext die_dwarf_decl_ctx; +DWARFASTParser *dwarf_ast = type_system ? type_system->GetDWARFParser() : nullptr; +for (DWARFDIE ctx_die = die; ctx_die && !isUnitType(ctx_die.Tag()); + ctx_die = ctx_die.GetParentDeclContextDIE()) { + die_dwarf_decl_ctx.AppendDeclContext(ctx_die.Tag(), ctx_die.GetName()); + template_params.push_back( + (ctx_die.IsStructUnionOrClass() && dwarf_ast) + ? dwarf_ast->GetDIEClassTemplateParams(ctx_die) + : ""); } +const bool any_template_params = llvm::any_of( +template_params, [](llvm::StringRef p) { return !p.empty(); }); -const DWARFDeclContext die_dwarf_decl_ctx = die.GetDWARFDeclContext(); +auto die_matches = [&](DWARFDIE type_die) { + // Resolve the type if both have the same tag or {class, struct} tags. + const bool tag_matches = + type_die.Tag() == tag || + (IsStructOrClassTag(type_die.Tag()) && IsStructOrClassTag(tag)); dwblaikie wrote: FWIW, I'm pretty sure that the only difference between class and struct is the access control. Though C++ technically rejects things like forward declaring a thing as a struct, then actually defining it as a class - not sure if that'd ever come up for lldb (for instance when interacting with clang header modules? If the DWARF contained a declaration of a thing and always created it as a struct, but then tried to load a module with that name as a class definition) I guess it'd also maybe break technically valid user code in expression evaluation - if the name was used for a varibale and a type, the user should be able to disambiguate in favor of the type by saying, eg: "sizeof(class MyType)" but if lldb made everything a struct ... hmm, nope, that'd be fine: ``` $ cat test.cpp struct MyType { }; int MyType = 3; static_assert(sizeof(class MyType) == 1); $ clang++-tot test.cpp -fsyntax-only ``` I guess printing types in lldb would be technically wrong, since it'd print as "struct MyType {" instead of "class MyType {"? (but it might mean not having to have the access modifier hack? If everything was made as a struct, and none of the members were given other access modifiers - everything would be accessible by default anyway) ``` https://github.com/llvm/llvm-project/pull/95905 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)
dwblaikie wrote: > @ZequanWu, @labath, since this PR got reverted due to crash for > `--gsimple-template-names`, do you guys have a timeline to revise a new > version without crashing? I ask this because our internal customers have many > forward declarations that are suffering from the slow definition lookup. cc > @clayborg It's certainly stuff that @ZequanWu, @labath, and myself are looking into. We could do a better job of communicating that upstream though :) I think we're meeting next week to chat a bit more about it, and can perhaps formalize that into some tracking bugs, etc. https://github.com/llvm/llvm-project/pull/92328 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Remove parsing recursion when searching for definition DIEs (PR #96484)
dwblaikie wrote: This patch as-is is NFC? (no tests) but without this patch, the other delay-definition-die patch would have had a problem? Is it possible to add a test in this patch that would exercise the thing that would become buggy if the delay-definition-die patch were to be recommitted? https://github.com/llvm/llvm-project/pull/96484 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Remove parsing recursion when searching for definition DIEs (PR #96484)
dwblaikie wrote: > > This patch as-is is NFC? > > NFC_**I**_, I would say :) I don't think this should change the behavior in > any way, but it's pretty hard to guarantee that. Sure enough - I take any claim as a statement of intent/belief, not of something mathematically proved, etc. > > (no tests) but without this patch, the other delay-definition-die patch > > would have had a problem? > > Is it possible to add a test in this patch that would exercise the thing > > that would become buggy if the delay-definition-die patch were to be > > recommitted? > > Sort of. The situation is a bit complicated, because this touches pretty much > the same code as the other patch, so the other patch will not apply cleanly > or become magically correct. It's more like a building block that enables us > to rewrite the other patch in a more robust manner -- which brings us to the > second way this is complicated: It's not that the other patch was wrong on > its own. It was just very sensitive to the other bugs. Previously, if we > failed to unique the types correctly, we would "just" get the wrong type (or > maybe no type). With the patch, that situation would trigger a hard assert. > On its own, that might even be considered a good thing (easier to find bugs), > we're it not for the fact that this made the logic of the patch very hard to > follow. So, this is my attempt to make it more straight-forward. > > As for tests, it is possible to write a test which would reproduce a crash > with the original patch, but that test is contingent on the existence of > another bug. When I reverted that patch, I added a test (in > [de3f1b6](https://github.com/llvm/llvm-project/commit/de3f1b6d68ab8a0e827db84b328803857a4f60df)) > which triggered the crash. Having a bit of a hard time following this - is the test you added the same as the test you speculated is possible to write in the prior sentence? > However, now that that bug is fixed (#95905), it does not crash anymore. Now, > I happen to know of another bug (which happens to be triggered by the same > code, only compiled with type units), but the same thing will happen once > that bug is fixed. OK - I think I'm following. > Given all of that, I don't think another test case is needed with this > particular patch. It might be interesting for the final delay patch, if we > don't fix the type unit thing by then, but I think of this patch mostly as a > cleanup. Perhaps a separate commit could add another RUN line to the existing test you added to demonstrate the reason for the revert? Rather than worrying about which comes first (the type unit patch or the delay patch) But in any case, I /think/ I understand why this patch doesn't need a test (because this patch avoids the delay patch causing a crash (yeah, more complex than that because the patch doesn't apply cleanly over this one) and that crash already has a test committed) - thanks for the explanation. https://github.com/llvm/llvm-project/pull/96484 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Construct SmallVector with ArrayRef (NFC) (PR #102793)
https://github.com/dwblaikie approved this pull request. https://github.com/llvm/llvm-project/pull/102793 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [clang] [clang][DebugInfo] Revert "emit definitions for constant-initialized static data-members" (PR #74580)
dwblaikie wrote: > To support access to such constants from LLDB we'll most likely have to have > to make LLDB find the constants by looking at the containing class first. Tangentially related to: https://discourse.llvm.org/t/rfc-improve-dwarf-5-debug-names-type-lookup-parsing-speed/74151/31?u=dblaikie Clang/LLVM do know the difference between type-invariant members and type variant ones (type invariant members are in the `members` list of the `DICompositeType` and variant members have a `scope` that refers to the type but don't appear in the `members` list) - would it be reasonable to not include the invariant members in the accelerator table, but only include the variant ones? Invariant ones can be found by finding any instance of the type in the index, then looking at its members - and for variant members it'd be useful to have them in the index. (though, honestly, I'm not sure how lldb and gdb handle that situation - last time I tested it with gdb, it just saw two different copies of the type and didn't try to unify/aggregate all the variant members... but lldb only creates one copy of the type, so don't know what it does if you've got, say, two member function template instantiations for different template parameters in two different translation units/compile units) https://github.com/llvm/llvm-project/pull/74580 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 2e19760 - lldb: Cache string hash during ConstString pool queries/insertions
Author: David Blaikie Date: 2023-12-12T00:07:08Z New Revision: 2e197602305be18b963928e6ae024a004a95af6d URL: https://github.com/llvm/llvm-project/commit/2e197602305be18b963928e6ae024a004a95af6d DIFF: https://github.com/llvm/llvm-project/commit/2e197602305be18b963928e6ae024a004a95af6d.diff LOG: lldb: Cache string hash during ConstString pool queries/insertions lldb was rehashing the string 3 times (once to determine which StringMap to use, once to query the StringMap, once to insert) on insertion (twice on successful lookup). This patch allows the lldb to benefit from hash improvements in LLVM (from djbHash to xxh3). Though further changes would be needed to cache this value to disk - we shouldn't rely on the StringMap::hash remaining the same in the future/this value should not be serialized to disk. If we want cache this value StringMap should take a hashing template parameter to allow for a fixed hash to be requested. Added: Modified: lldb/source/Utility/ConstString.cpp Removed: diff --git a/lldb/source/Utility/ConstString.cpp b/lldb/source/Utility/ConstString.cpp index 4535771adfb735..ea897dc611cc94 100644 --- a/lldb/source/Utility/ConstString.cpp +++ b/lldb/source/Utility/ConstString.cpp @@ -77,10 +77,10 @@ class Pool { return 0; } - StringPoolValueType GetMangledCounterpart(const char *ccstr) const { + StringPoolValueType GetMangledCounterpart(const char *ccstr) { if (ccstr != nullptr) { - const uint8_t h = hash(llvm::StringRef(ccstr)); - llvm::sys::SmartScopedReader rlock(m_string_pools[h].m_mutex); + const PoolEntry &pool = selectPool(llvm::StringRef(ccstr)); + llvm::sys::SmartScopedReader rlock(pool.m_mutex); return GetStringMapEntryFromKeyData(ccstr).getValue(); } return nullptr; @@ -100,19 +100,20 @@ class Pool { const char *GetConstCStringWithStringRef(llvm::StringRef string_ref) { if (string_ref.data()) { - const uint8_t h = hash(string_ref); + const uint32_t string_hash = StringPool::hash(string_ref); + PoolEntry &pool = selectPool(string_hash); { -llvm::sys::SmartScopedReader rlock(m_string_pools[h].m_mutex); -auto it = m_string_pools[h].m_string_map.find(string_ref); -if (it != m_string_pools[h].m_string_map.end()) +llvm::sys::SmartScopedReader rlock(pool.m_mutex); +auto it = pool.m_string_map.find(string_ref, string_hash); +if (it != pool.m_string_map.end()) return it->getKeyData(); } - llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex); + llvm::sys::SmartScopedWriter wlock(pool.m_mutex); StringPoolEntryType &entry = - *m_string_pools[h] - .m_string_map.insert(std::make_pair(string_ref, nullptr)) + *pool.m_string_map + .insert(std::make_pair(string_ref, nullptr), string_hash) .first; return entry.getKeyData(); } @@ -125,12 +126,14 @@ class Pool { const char *demangled_ccstr = nullptr; { - const uint8_t h = hash(demangled); - llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex); + const uint32_t demangled_hash = StringPool::hash(demangled); + PoolEntry &pool = selectPool(demangled_hash); + llvm::sys::SmartScopedWriter wlock(pool.m_mutex); // Make or update string pool entry with the mangled counterpart - StringPool &map = m_string_pools[h].m_string_map; - StringPoolEntryType &entry = *map.try_emplace(demangled).first; + StringPool &map = pool.m_string_map; + StringPoolEntryType &entry = + *map.try_emplace_with_hash(demangled, demangled_hash).first; entry.second = mangled_ccstr; @@ -141,8 +144,8 @@ class Pool { { // Now assign the demangled const string as the counterpart of the // mangled const string... - const uint8_t h = hash(llvm::StringRef(mangled_ccstr)); - llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex); + PoolEntry &pool = selectPool(llvm::StringRef(mangled_ccstr)); + llvm::sys::SmartScopedWriter wlock(pool.m_mutex); GetStringMapEntryFromKeyData(mangled_ccstr).setValue(demangled_ccstr); } @@ -171,17 +174,20 @@ class Pool { } protected: - uint8_t hash(llvm::StringRef s) const { -uint32_t h = llvm::djbHash(s); -return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff; - } - struct PoolEntry { mutable llvm::sys::SmartRWMutex m_mutex; StringPool m_string_map; }; std::array m_string_pools; + + PoolEntry &selectPool(const llvm::StringRef &s) { +return selectPool(StringPool::hash(s)); + } + + PoolEntry &selectPool(uint32_t h) { +return m_string_pools[((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff]; + } }; // Frameworks and dylibs aren't supposed to have global C++ initializers so we @@ -197,7 +203,7 @@ static Pool &Strin
[Lldb-commits] [lldb] 5bc1adf - Revert "lldb: Cache string hash during ConstString pool queries/insertions"
Author: David Blaikie Date: 2023-12-14T17:44:18Z New Revision: 5bc1adff69315dcef670e9fcbe04067b5d5963fb URL: https://github.com/llvm/llvm-project/commit/5bc1adff69315dcef670e9fcbe04067b5d5963fb DIFF: https://github.com/llvm/llvm-project/commit/5bc1adff69315dcef670e9fcbe04067b5d5963fb.diff LOG: Revert "lldb: Cache string hash during ConstString pool queries/insertions" Underlying StringMap API for providing a hash has caused some problems (observed a crash in lld) - so reverting this until I can figure out/fix what's going on there. This reverts commit 52ba075571958e2fec8d871ddfa1ef56486b86d3. This reverts commit 2e197602305be18b963928e6ae024a004a95af6d. Added: Modified: lldb/source/Utility/ConstString.cpp llvm/lib/Support/StringMap.cpp Removed: diff --git a/lldb/source/Utility/ConstString.cpp b/lldb/source/Utility/ConstString.cpp index ea897dc611cc94..4535771adfb735 100644 --- a/lldb/source/Utility/ConstString.cpp +++ b/lldb/source/Utility/ConstString.cpp @@ -77,10 +77,10 @@ class Pool { return 0; } - StringPoolValueType GetMangledCounterpart(const char *ccstr) { + StringPoolValueType GetMangledCounterpart(const char *ccstr) const { if (ccstr != nullptr) { - const PoolEntry &pool = selectPool(llvm::StringRef(ccstr)); - llvm::sys::SmartScopedReader rlock(pool.m_mutex); + const uint8_t h = hash(llvm::StringRef(ccstr)); + llvm::sys::SmartScopedReader rlock(m_string_pools[h].m_mutex); return GetStringMapEntryFromKeyData(ccstr).getValue(); } return nullptr; @@ -100,20 +100,19 @@ class Pool { const char *GetConstCStringWithStringRef(llvm::StringRef string_ref) { if (string_ref.data()) { - const uint32_t string_hash = StringPool::hash(string_ref); - PoolEntry &pool = selectPool(string_hash); + const uint8_t h = hash(string_ref); { -llvm::sys::SmartScopedReader rlock(pool.m_mutex); -auto it = pool.m_string_map.find(string_ref, string_hash); -if (it != pool.m_string_map.end()) +llvm::sys::SmartScopedReader rlock(m_string_pools[h].m_mutex); +auto it = m_string_pools[h].m_string_map.find(string_ref); +if (it != m_string_pools[h].m_string_map.end()) return it->getKeyData(); } - llvm::sys::SmartScopedWriter wlock(pool.m_mutex); + llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex); StringPoolEntryType &entry = - *pool.m_string_map - .insert(std::make_pair(string_ref, nullptr), string_hash) + *m_string_pools[h] + .m_string_map.insert(std::make_pair(string_ref, nullptr)) .first; return entry.getKeyData(); } @@ -126,14 +125,12 @@ class Pool { const char *demangled_ccstr = nullptr; { - const uint32_t demangled_hash = StringPool::hash(demangled); - PoolEntry &pool = selectPool(demangled_hash); - llvm::sys::SmartScopedWriter wlock(pool.m_mutex); + const uint8_t h = hash(demangled); + llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex); // Make or update string pool entry with the mangled counterpart - StringPool &map = pool.m_string_map; - StringPoolEntryType &entry = - *map.try_emplace_with_hash(demangled, demangled_hash).first; + StringPool &map = m_string_pools[h].m_string_map; + StringPoolEntryType &entry = *map.try_emplace(demangled).first; entry.second = mangled_ccstr; @@ -144,8 +141,8 @@ class Pool { { // Now assign the demangled const string as the counterpart of the // mangled const string... - PoolEntry &pool = selectPool(llvm::StringRef(mangled_ccstr)); - llvm::sys::SmartScopedWriter wlock(pool.m_mutex); + const uint8_t h = hash(llvm::StringRef(mangled_ccstr)); + llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex); GetStringMapEntryFromKeyData(mangled_ccstr).setValue(demangled_ccstr); } @@ -174,20 +171,17 @@ class Pool { } protected: + uint8_t hash(llvm::StringRef s) const { +uint32_t h = llvm::djbHash(s); +return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff; + } + struct PoolEntry { mutable llvm::sys::SmartRWMutex m_mutex; StringPool m_string_map; }; std::array m_string_pools; - - PoolEntry &selectPool(const llvm::StringRef &s) { -return selectPool(StringPool::hash(s)); - } - - PoolEntry &selectPool(uint32_t h) { -return m_string_pools[((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff]; - } }; // Frameworks and dylibs aren't supposed to have global C++ initializers so we @@ -203,7 +197,7 @@ static Pool &StringPool() { static Pool *g_string_pool = nullptr; llvm::call_once(g_pool_initialization_flag, - []() { g_string_pool = new Pool(); }); + []() { g_string_pool = new Pool(); }); return *g_string_p
[Lldb-commits] [lldb] Make only one function that needs to be implemented when searching for types (PR #74786)
dwblaikie wrote: FWIW, we're seeing this breaking a pretty printer in google - previously a lookup was able to find `absl::cord_internal::RefcountAndFlags::Flags::kNumFlags` ( https://github.com/abseil/abseil-cpp/blob/8184f16e898fcb13b310b40430e13ba40679cf0e/absl/strings/internal/cord_internal.h#L202 ) (the name components are, in order: namespace, namespace, class, unscoped enumeration, enumerator) The code we have (working via [gala](https://github.com/sivachandra/gala)) has been doing this query twice to workaround this bug: https://github.com/llvm/llvm-project/issues/53904 - but it looks like since this change, that workaround has even broken and the name is no longer found? I don't have a fully isolated repro yet, but figured I'd start with raising the flag in case anyone recognizes this. https://github.com/llvm/llvm-project/pull/74786 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Make only one function that needs to be implemented when searching for types (PR #74786)
dwblaikie wrote: @clayborg any thoughts on this ^ ? https://github.com/llvm/llvm-project/pull/74786 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix expressions that involve nested structs/classes/unions. (PR #77029)
dwblaikie wrote: Thanks for the fix! I did test this patch with both our regression issue, and also allowing us to remove the double-lookup working around #53904 and can confirm it addressed both issues. Thanks again! https://github.com/llvm/llvm-project/pull/77029 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] GetClangDeclForDIE: don't create VarDecl for static data members (PR #77155)
@@ -0,0 +1,23 @@ +# UNSUPPORTED: system-darwin, system-windows + +# In DWARFv5, C++ static data members are represented +# as DW_TAG_variable. We make sure LLDB's expression +# evaluator doesn't crash when trying to parse such +# a DW_TAG_variable DIE, whose parent DIE is only +# a forward declaration. + +# RUN: %clangxx_host %S/Inputs/dwo-static-data-member.cpp \ +# RUN: -g -gdwarf-5 -gsplit-dwarf -o %t +# RUN: %lldb %t -s %s -o exit 2>&1 | FileCheck %s + +breakpoint set -n main +process launch + +# CHECK: Process {{.*}} stopped + +# There is no definition for NoCtor anywhere +# in the debug-info, so LLDB can't evaluate +# this expression. +expression NoCtor::i +# CHECK-LABEL: expression NoCtor::i +# CHECK: use of undeclared identifier 'NoCtor' dwblaikie wrote: (totally side note: Pity about how this fails (a) gdb can evaluate this, so it's a bug/limitation in lldb (though it sounds like lldb-eval maybe can handle this, hopefully, and possibly without the need for a running process either) (b) `NoCtor` isn't undeclared - it's declared but not defined, but even saying that wouldn't be quite accurate/wouldn't explain why it can't be evaluated by the debugger, since the info for the `i` member is present But that's just me rambling/not relevant to this bug/regression/immediate issue) https://github.com/llvm/llvm-project/pull/77155 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] GetClangDeclForDIE: don't create VarDecl for static data members (PR #77155)
@@ -0,0 +1,23 @@ +# UNSUPPORTED: system-darwin, system-windows dwblaikie wrote: That's actually a great question, but I think for now the answer with lldb is "no, expression evaluation doesn't work without a running process" - but I was just commenting on an internal bug where folks were having trouble investigating core dumps with some of our pretty printers that evaluate expressions that happen to be constants that gdb can evaluate on a core/without a running process, but it seems lldb can't do that... So while that sounds like a good thing, my understanding is that lldb can't do that at the moment. https://github.com/llvm/llvm-project/pull/77155 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 4e429fd - Few linter fixes
Author: David Blaikie Date: 2023-07-31T18:52:57Z New Revision: 4e429fd2a72511bc3c0a20e1b12f735863e615df URL: https://github.com/llvm/llvm-project/commit/4e429fd2a72511bc3c0a20e1b12f735863e615df DIFF: https://github.com/llvm/llvm-project/commit/4e429fd2a72511bc3c0a20e1b12f735863e615df.diff LOG: Few linter fixes size() > 0 -> !empty indentation mismatched names on parameters in decls/defs const on value return types Added: Modified: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Removed: diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h index d298af92ba5e62..920a5eba20abd9 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h @@ -77,7 +77,7 @@ class ClassDescriptorV2 : public ObjCLanguageRuntime::ClassDescriptor { void GetIVarInformation(); private: - static const uint32_t RW_REALIZED = (1 << 31); + static const uint32_t RW_REALIZED = (1u << 31); struct objc_class_t { ObjCLanguageRuntime::ObjCISA m_isa = 0; // The class's metaclass. @@ -173,7 +173,8 @@ class ClassDescriptorV2 : public ObjCLanguageRuntime::ClassDescriptor { } bool Read(Process *process, lldb::addr_t addr, - lldb::addr_t relative_method_lists_base_addr, bool, bool); + lldb::addr_t relative_selector_base_addr, bool is_small, + bool has_direct_sel); }; struct ivar_list_t { diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp index d5357b94d68edc..a50cdc88cd0124 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp @@ -1399,7 +1399,7 @@ class RemoteNXMapTable { return *this; } -const element operator*() const { +element operator*() const { if (m_index == -1) { // TODO find a way to make this an error, but not an assert return element(); @@ -2739,7 +2739,7 @@ lldb::addr_t AppleObjCRuntimeV2::LookupRuntimeSymbol(ConstString name) { std::pair class_and_ivar = ivar_skipped_prefix.split('.'); - if (class_and_ivar.first.size() && class_and_ivar.second.size()) { + if (!class_and_ivar.first.empty() && !class_and_ivar.second.empty()) { const ConstString class_name_cs(class_and_ivar.first); ClassDescriptorSP descriptor = ObjCLanguageRuntime::GetClassDescriptorFromClassName(class_name_cs); diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h index 678865ecd9186f..c9d0b3a907b54b 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h @@ -65,12 +65,12 @@ class AppleObjCRuntimeV2 : public AppleObjCRuntime { return ObjCRuntimeVersions::eAppleObjC_V2; } - size_t GetByteOffsetForIvar(CompilerType &parent_qual_type, + size_t GetByteOffsetForIvar(CompilerType &parent_ast_type, const char *ivar_name) override; void UpdateISAToDescriptorMapIfNeeded() override; - ClassDescriptorSP GetClassDescriptor(ValueObject &in_value) override; + ClassDescriptorSP GetClassDescriptor(ValueObject &valobj) override; ClassDescriptorSP GetClassDescriptorFromISA(ObjCISA isa) override; diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp index 8f9d9173af9fbf..d81b634be87b1e 100644 --- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp +++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp @@ -2867,7 +2867,7 @@ bool IRTranslator::translateVAArg(const User &U, MachineIRBuilder &MIRBuilder) { } bool IRTranslator::translateUnreachable(const User &U, MachineIRBuilder &MIRBuilder) { -if (!MF->getTarget().Options.TrapUnreachable) + if (!MF->getTarget().Options.TrapUnreachable) return true; auto &UI = cast(U); diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 87ba56137728b3..7191e89d36071b 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAG
[Lldb-commits] [lldb] [lldb-vscode] Display a more descriptive summary for containers and pointers (PR #65514)
@@ -132,6 +132,84 @@ std::vector GetStrings(const llvm::json::Object *obj, return strs; } +/// Create a short summary for a container that contains the summary of its +/// first children, so that the user can get a glimpse of its contents at a +/// glance. +static std::optional +GetSyntheticSummaryForContainer(lldb::SBValue &v) { + if (v.TypeIsPointerType() || !v.MightHaveChildren()) +return std::nullopt; + /// As this operation can be potentially slow, we limit the total time spent + /// fetching children to a few ms. + const auto max_evaluation_time = std::chrono::milliseconds(10); + /// We don't want to generate a extremely long summary string, so we limit its + /// length. + const size_t max_length = 32; + + auto start = std::chrono::steady_clock::now(); + std::string summary; + llvm::raw_string_ostream os(summary); + os << "{"; + + llvm::StringRef separator = ""; + + for (size_t i = 0, e = v.GetNumChildren(); i < e; ++i) { dwblaikie wrote: What do other debuggers do? (like Visual Studio itself) & not sure that `GetNumChildren` should cause indefinite recursive expansion - could limit this feature to just going one level deep, for instance. But certainly worth comparing to other debuggers to see what sort of heuristics/rules/guidelines are good in terms of the amount of data expanded by default V unexpanded. (& yeah, if everything's expanded all the way down, that's probably not good) https://github.com/llvm/llvm-project/pull/65514 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add `target modules dump separate-debug-info` (PR #66035)
dwblaikie wrote: (aside: isn't the SBAPI meant to be the thing to use if you want to script stuff, rather than having formatted output/scraping that? - but sounds like the output has converged on a table rather than JSON anyway? So maybe a moot point) Re: errors, OSO (what does that stand for anyway) - yeah, maybe separate tables for different data? (one table for OSO, one for DWO, one for errors? (or two for errors, one for OSO errors and one for DWO errors?)) https://github.com/llvm/llvm-project/pull/66035 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add `target modules dump separate-debug-info` (PR #66035)
dwblaikie wrote: > N_SO is the stab moniker for a source file, and N_OSO is the object file > associated with that source file (N_SO was in Sun's implementation, we made > up N_OSO). Most nm''s leave off the N_ when they print stabs, so then this > became just OSO... > We don't do DWO on Darwin, and nobody but Darwin does OSO's, so for now I > don't think you need to worry about mixing of the entities in these reports. Ah, thanks for the context - makes sense. https://github.com/llvm/llvm-project/pull/66035 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-vscode] Display a more descriptive summary for containers and pointers (PR #65514)
@@ -132,6 +132,84 @@ std::vector GetStrings(const llvm::json::Object *obj, return strs; } +/// Create a short summary for a container that contains the summary of its +/// first children, so that the user can get a glimpse of its contents at a +/// glance. +static std::optional +GetSyntheticSummaryForContainer(lldb::SBValue &v) { + if (v.TypeIsPointerType() || !v.MightHaveChildren()) +return std::nullopt; + /// As this operation can be potentially slow, we limit the total time spent + /// fetching children to a few ms. + const auto max_evaluation_time = std::chrono::milliseconds(10); + /// We don't want to generate a extremely long summary string, so we limit its + /// length. + const size_t max_length = 32; + + auto start = std::chrono::steady_clock::now(); + std::string summary; + llvm::raw_string_ostream os(summary); + os << "{"; + + llvm::StringRef separator = ""; + + for (size_t i = 0, e = v.GetNumChildren(); i < e; ++i) { dwblaikie wrote: I'm not sure I understand. `v.GetNumChildren()` wouldn't need to do any more work for a case with "10 class pointer variables", right? You can determine how many children this type has (10) without completing the types those pointers point to? Or is it later code (like line 168, etc) that try to get the summary of those pointer member variables that are an issue? (though getting the summary of a pointer shouldn't automatically dereference the pointer/complete its pointed-to type, should it?) https://github.com/llvm/llvm-project/pull/65514 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-vscode] Display a more descriptive summary for containers and pointers (PR #65514)
@@ -132,6 +132,84 @@ std::vector GetStrings(const llvm::json::Object *obj, return strs; } +/// Create a short summary for a container that contains the summary of its +/// first children, so that the user can get a glimpse of its contents at a +/// glance. +static std::optional +GetSyntheticSummaryForContainer(lldb::SBValue &v) { + if (v.TypeIsPointerType() || !v.MightHaveChildren()) +return std::nullopt; + /// As this operation can be potentially slow, we limit the total time spent + /// fetching children to a few ms. + const auto max_evaluation_time = std::chrono::milliseconds(10); + /// We don't want to generate a extremely long summary string, so we limit its + /// length. + const size_t max_length = 32; + + auto start = std::chrono::steady_clock::now(); + std::string summary; + llvm::raw_string_ostream os(summary); + os << "{"; + + llvm::StringRef separator = ""; + + for (size_t i = 0, e = v.GetNumChildren(); i < e; ++i) { dwblaikie wrote: Thanks for the context - how's all this compare to other debuggers? (like Visual Studio, for example - what's it show by default for the types?) https://github.com/llvm/llvm-project/pull/65514 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)
dwblaikie wrote: The dev policy says "Avoid committing formatting- or whitespace-only changes outside of code you plan to make subsequent changes to." - I think it's reasonable to consider changing this, but probably under the "clang-format everything" or a similar discussion (broad discussion before we worry about making pull requests https://github.com/llvm/llvm-project/pull/82838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lld] [lldb] [llvm] [mlir] Rename ThreadPool->DefaultThreadPool and ThreadPoolInterface->ThreadPool (NFC) (PR #83702)
dwblaikie wrote: I don't have really firm feelings/justification for this, but I'd have guessed that doing this as two separate (separated by a few days, maybe a week) renamings would be better for downstream consumers - they'd get a clear break without any ambiguity/name reuse. No strong feels if other folks reckon doing it in one go is better/OK, though. https://github.com/llvm/llvm-project/pull/83702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lld] [lldb] [llvm] [mlir] Rename ThreadPool->DefaultThreadPool and ThreadPoolInterface->ThreadPool (NFC) (PR #83702)
dwblaikie wrote: > @dwblaikie : how would you split it? I didn't quite get the two renamings you > have in mind? One patch `ThreadPool->DefaultThreadPool` (people get a build error about `ThreadPool` not being the name of anything, find this patch as the root cause, and rename all their ThreadPool->DefaultThreadPool) Follow-up patch, `ThreadPoolInterface->ThreadPool` (similarly clear/separate errors) Changing both in one patch risks folks getting confusing error messages because their existing ThreadPool usage will now instantly start being interpreted as the interface type - resulting in different/confusing errors about inability to instantiate abstract types, etc, presumably. Rather than just that the name is no longer present at all. Ultimately they can root cause and figure out both renamings - probably not a huge deal either way, but my understanding was that separating them might be marginally better for downstrteamers. https://github.com/llvm/llvm-project/pull/83702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lld] [lldb] [llvm] [mlir] Rename llvm::ThreadPool -> llvm::DefaultThreadPool (NFC) (PR #83702)
dwblaikie wrote: > I did the first part of the renaming @dwblaikie : looks good? Hmm - this patch still seems to have both renamings in it, if I'm reading the PR correctly? I take it from the subject that isn't the intent/the intent is that his patch only does the ThreadPool->DefaultThreadPool change? https://github.com/llvm/llvm-project/pull/83702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lld] [lldb] [llvm] [mlir] Rename llvm::ThreadPool -> llvm::DefaultThreadPool (NFC) (PR #83702)
dwblaikie wrote: Oh, maybe issues related to layered patches - this is intended to be submitted after the introduction of the ThreadPoolInterface? But includes those changes in this review at the moment (I still haven't figured out what we're doing for dependent changes - and I thought the ThreadPoolInterface-introducing patch was already in - so not sure why it appears in this patch too) https://github.com/llvm/llvm-project/pull/83702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Store SupportFile in FileEntry (NFC) (PR #85468)
dwblaikie wrote: (because we don't have a good sense of where feedback should be provided... crosslinking here from some feedback on the commit: https://github.com/llvm/llvm-project/commit/d5a277d309e92b1d3e493da6036cffdf815105b1#commitcomment-139991120 ) https://github.com/llvm/llvm-project/pull/85468 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
dwblaikie wrote: > For those files in the repository that do need CRLF endings, I've adopted a > policy of eol=crlf which means that git will store them in history with LF, > but regardless of user config, they'll be checked out in tree with CRLF. This ^ seems a bit problematic to me, though (where the contents of the repo isn't representative of the bits we want - but perhaps it's schrodinger's line endings & it doesn't matter if it's always checked out as crlf - though if we one day converted to another source control system, would that be a problem? are there some things that examine the underlying/"real" repo contents?) - what would be the cost to using `eol=input` as you've done for the mixed line ending case? below \= > There also appears to be a single test, clang/test/Frontend/rewrite-includes-mixed-eol-crlf.[ch] which needs mixed line endings. This one uses eol=input to preserve it as-is. https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 9df19ce - Add uncovered enums in switches caused by 9434c083475e42f47383f3067fe2a155db5c6a30
Author: David Blaikie Date: 2024-04-01T23:07:01Z New Revision: 9df19ce40281551bd348b262a131085cf98dadf5 URL: https://github.com/llvm/llvm-project/commit/9df19ce40281551bd348b262a131085cf98dadf5 DIFF: https://github.com/llvm/llvm-project/commit/9df19ce40281551bd348b262a131085cf98dadf5.diff LOG: Add uncovered enums in switches caused by 9434c083475e42f47383f3067fe2a155db5c6a30 These are probably actually unreachable - perhaps an lldb developer would be interested in rephrasing this change to move the new cases into some unreachable/unsupported bucket, rather than my half-hearted guess at what the desired behavior would be (completely untested, because they're probably untestable/unreachable - maybe debugging from modules?) Added: Modified: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp Removed: diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index ebcc3bc99a801f..4a1c8d57655215 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -4097,6 +4097,8 @@ TypeSystemClang::GetTypeClass(lldb::opaque_compiler_type_t type) { return lldb::eTypeClassArray; case clang::Type::DependentSizedArray: return lldb::eTypeClassArray; + case clang::Type::ArrayParameter: +return lldb::eTypeClassArray; case clang::Type::DependentSizedExtVector: return lldb::eTypeClassVector; case clang::Type::DependentVector: @@ -4776,6 +4778,7 @@ lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type, case clang::Type::IncompleteArray: case clang::Type::VariableArray: + case clang::Type::ArrayParameter: break; case clang::Type::ConstantArray: @@ -5109,6 +5112,7 @@ lldb::Format TypeSystemClang::GetFormat(lldb::opaque_compiler_type_t type) { case clang::Type::IncompleteArray: case clang::Type::VariableArray: + case clang::Type::ArrayParameter: break; case clang::Type::ConstantArray: ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -48,15 +60,31 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) { return result; } +DWARFTypeUnit * +DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry &entry) const { + std::optional type_sig = entry.getForeignTUTypeSignature(); + if (type_sig) +if (auto dwp_sp = m_debug_info.GetDwpSymbolFile()) dwblaikie wrote: this `auto` should probably be const ref, to avoid inc/dec on the ref counted smart pointer's ref count https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -1726,44 +1725,59 @@ lldb::ModuleSP SymbolFileDWARF::GetExternalModule(ConstString name) { return pos->second; } -DWARFDIE -SymbolFileDWARF::GetDIE(const DIERef &die_ref) { - // This method can be called without going through the symbol vendor so we - // need to lock the module. - std::lock_guard guard(GetModuleMutex()); - - SymbolFileDWARF *symbol_file = nullptr; - +SymbolFileDWARF *SymbolFileDWARF::GetDIERefSymbolFile(const DIERef &die_ref) { // Anytime we get a "lldb::user_id_t" from an lldb_private::SymbolFile API we // must make sure we use the correct DWARF file when resolving things. On // MacOSX, when using SymbolFileDWARFDebugMap, we will use multiple // SymbolFileDWARF classes, one for each .o file. We can often end up with // references to other DWARF objects and we must be ready to receive a // "lldb::user_id_t" that specifies a DIE from another SymbolFileDWARF // instance. + std::optional file_index = die_ref.file_index(); + + // If the file index matches, then we have the right SymbolFileDWARF already. + // This will work for both .dwo file and DWARF in .o files for mac. Also if + // both the file indexes are invalid, then we have a match. + if (GetFileIndex() == file_index) +return this; + + // If we are currently in a .dwo file and our file index doesn't match we need + // to let the base symbol file handle this. + SymbolFileDWARFDwo *dwo = llvm::dyn_cast_or_null(this); + if (dwo) +return dwo->GetBaseSymbolFile().GetDIERefSymbolFile(die_ref); + if (file_index) { -if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) { - symbol_file = debug_map->GetSymbolFileByOSOIndex(*file_index); // OSO case - if (symbol_file) -return symbol_file->DebugInfo().GetDIE(die_ref); - return DWARFDIE(); -} +SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile(); +if (debug_map) { +// We have a SymbolFileDWARFDebugMap, so let it find the right file + return debug_map->GetSymbolFileByOSOIndex(*file_index); +} else { + // Handle the .dwp file case correctly + if (*file_index == DIERef::k_file_index_mask) +return GetDwpSymbolFile().get(); // DWP case -if (*file_index == DIERef::k_file_index_mask) - symbol_file = GetDwpSymbolFile().get(); // DWP case -else - symbol_file = this->DebugInfo() -.GetUnitAtIndex(*die_ref.file_index()) + // Handle the .dwo file case correctly + return DebugInfo().GetUnitAtIndex(*die_ref.file_index()) ->GetDwoSymbolFile(); // DWO case - } else if (die_ref.die_offset() == DW_INVALID_OFFSET) { -return DWARFDIE(); +} } + return this; +} - if (symbol_file) -return symbol_file->GetDIE(die_ref); +DWARFDIE +SymbolFileDWARF::GetDIE(const DIERef &die_ref) { + if (die_ref.die_offset() == DW_INVALID_OFFSET) +return DWARFDIE(); - return DebugInfo().GetDIE(die_ref); + // This method can be called without going through the symbol vendor so we + // need to lock the module. + std::lock_guard guard(GetModuleMutex()); + SymbolFileDWARF *symbol_file = GetDIERefSymbolFile(die_ref); + if (symbol_file) dwblaikie wrote: Similar feedback others have given elsewhere, roll the variable declaration into the `if` condition https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -58,6 +58,8 @@ class DWARFDebugInfo { const DWARFDebugAranges &GetCompileUnitAranges(); + const std::shared_ptr GetDwpSymbolFile(); dwblaikie wrote: Remove const from by-value return. (it messes with move semantics and some other things) - or was this meant to return by reference? - yeah, I guess this latter, the underlying `m_dwarf.GetDwpSymbolFile()` seems to return by const reference, so this function probably should too? https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -0,0 +1,91 @@ +// REQUIRES: lld + +// This test will make a type that will be compiled differently into two +// different .dwo files in a type unit with the same type hash, but with +// differing contents. I have discovered that the hash for the type unit is +// simply based off of the typename and doesn't seem to differ when the contents +// differ, so that will help us test foreign type units in the .debug_names dwblaikie wrote: Rather than the personal statement, could maybe just be explicit: "Clang's type unit signature is based only on the mangled name of the type, regardless of the contents of the type" (I can tell you that's how it works - that's how I implemented it - it is a violation of the DWARF spec (the spec says to use a content hash - though that content hash is of the semantic contents, not the syntactic content (eg: if the TU contained a type with a single int member, the spec-compliant hash would be the same whether it was the type DIE followed by the int DIE, or the other way around) - so it would still be the same despite some variations in layout, so the index entries still wouldn't be portable to all matched-hash definitions)) https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -273,6 +301,44 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType( if (!isType(entry.tag())) continue; + +DWARFTypeUnit *foreign_tu = GetForeignTypeUnit(entry); +if (foreign_tu) { + // If this entry represents a foreign type unit, we need to verify that + // the type unit that ended up in the final .dwp file is the right type + // unit. Type units have signatures which are the same across multiple + // .dwo files, but only one of those type units will end up in the .dwp + // file. The contents of type units for the same type can be different + // in different .dwo file, which means the DIE offsets might not be the + // same between two different type units. So we need to determine if this + // accelerator table matches the type unit in the .dwp file. If it doesn't + // match, then we need to ignore this accelerator table entry as the type + // unit that is in the .dwp file will have its own index. + const llvm::DWARFDebugNames::NameIndex *name_index = entry.getNameIndex(); + if (name_index == nullptr) +continue; + // In order to determine if the type unit that ended up in a .dwp file + // is valid, we need to grab the type unit and check the attribute on the + // type unit matches the .dwo file. For this to happen we rely on each + // .dwo file having its own .debug_names table with a single compile unit + // and multiple type units. This is the only way we can tell if a type + // unit came from a specific .dwo file. dwblaikie wrote: Sounds like this wouldn't work for a merged `.debug_names` table? Could you leave a FIXME/do you plan to fix this? Oh, it also wouldn't work for any kind of LTO which could have multiple CUs in a single object file/dwo file. (FYI @cmtice ) https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -273,6 +301,44 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType( if (!isType(entry.tag())) continue; + +DWARFTypeUnit *foreign_tu = GetForeignTypeUnit(entry); +if (foreign_tu) { + // If this entry represents a foreign type unit, we need to verify that + // the type unit that ended up in the final .dwp file is the right type + // unit. Type units have signatures which are the same across multiple + // .dwo files, but only one of those type units will end up in the .dwp + // file. The contents of type units for the same type can be different + // in different .dwo file, which means the DIE offsets might not be the + // same between two different type units. So we need to determine if this + // accelerator table matches the type unit in the .dwp file. If it doesn't + // match, then we need to ignore this accelerator table entry as the type + // unit that is in the .dwp file will have its own index. + const llvm::DWARFDebugNames::NameIndex *name_index = entry.getNameIndex(); + if (name_index == nullptr) +continue; + // In order to determine if the type unit that ended up in a .dwp file + // is valid, we need to grab the type unit and check the attribute on the + // type unit matches the .dwo file. For this to happen we rely on each + // .dwo file having its own .debug_names table with a single compile unit + // and multiple type units. This is the only way we can tell if a type + // unit came from a specific .dwo file. dwblaikie wrote: I think our conclusion from previous discussions was to put `DW_IDX_compile_unit`, in addition to the `DW_IDX_type_unit` on the entries in the .debug_names table - and add the CU column to the .debug_tu_index in the dwp file, to match these things up? https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -0,0 +1,91 @@ +// REQUIRES: lld + +// This test will make a type that will be compiled differently into two +// different .dwo files in a type unit with the same type hash, but with +// differing contents. I have discovered that the hash for the type unit is +// simply based off of the typename and doesn't seem to differ when the contents +// differ, so that will help us test foreign type units in the .debug_names +// section of the main executable. When a DWP file is made, only one type unit +// will be kept and the type unit that is kept has the .dwo file name that it +// came from. When LLDB loads the foreign type units, it needs to verify that +// any entries from foreign type units come from the right .dwo file. We test +// this since the contents of type units are not always the same even though +// they have the same type hash. We don't want invalid accelerator table entries +// to come from one .dwo file and be used on a type unit from another since this +// could cause invalid lookups to happen. LLDB knows how to track down which +// .dwo file a type unit comes from by looking at the DW_AT_dwo_name attribute +// in the DW_TAG_type_unit. + +// Now test with DWARF5 +// RUN: %clang -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf \ +// RUN: -fdebug-types-section -gpubnames -c %s -o %t.main.o +// RUN: %clang -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -DVARIANT \ +// RUN: -fdebug-types-section -gpubnames -c %s -o %t.foo.o +// RUN: ld.lld %t.main.o %t.foo.o -o %t + +// First we check when we make the .dwp file with %t.main.dwo first so it will +// pick the type unit from %t.main.dwo. Verify we find only the types from +// %t.main.dwo's type unit. +// RUN: llvm-dwp %t.main.dwo %t.foo.dwo -o %t.dwp +// RUN: %lldb \ +// RUN: -o "type lookup IntegerType" \ +// RUN: -o "type lookup FloatType" \ +// RUN: -o "type lookup IntegerType" \ +// RUN: -b %t | FileCheck %s +// CHECK: (lldb) type lookup IntegerType +// CHECK-NEXT: int +// CHECK-NEXT: (lldb) type lookup FloatType +// CHECK-NEXT: double +// CHECK-NEXT: (lldb) type lookup IntegerType +// CHECK-NEXT: int + +// Next we check when we make the .dwp file with %t.foo.dwo first so it will +// pick the type unit from %t.main.dwo. Verify we find only the types from +// %t.main.dwo's type unit. +// RUN: llvm-dwp %t.foo.dwo %t.main.dwo -o %t.dwp +// RUN: %lldb \ +// RUN: -o "type lookup IntegerType" \ +// RUN: -o "type lookup FloatType" \ +// RUN: -o "type lookup IntegerType" \ +// RUN: -b %t | FileCheck %s --check-prefix=VARIANT + +// VARIANT: (lldb) type lookup IntegerType +// VARIANT-NEXT: unsigned int +// VARIANT-NEXT: (lldb) type lookup FloatType +// VARIANT-NEXT: float +// VARIANT-NEXT: (lldb) type lookup IntegerType +// VARIANT-NEXT: unsigned int + + +// We need to do this so we end with a type unit in each .dwo file and that has +// the same signature but different contents. When we make the .dwp file, then +// one of the type units will end up in the .dwp file and we will have +// .debug_names accelerator tables for both type units and we need to ignore +// the type units .debug_names entries that don't match the .dwo file whose +// copy of the type unit ends up in the final .dwp file. To do this, LLDB will +// look at the type unit and take the DWO name attribute and make sure it +// matches, and if it doesn't, it will ignore the accelerator table entry. +struct CustomType { + // We switch the order of "FloatType" and "IntegerType" so that if we do + // end up reading the wrong accelerator table entry, that we would end up + // getting an invalid offset and not find anything, or the offset would have + // matched and we would find the wrong thing. dwblaikie wrote: Couldn't we have a difference between these type variants that's simpler - like one version of the struct with a single `int` member, and one version of the type with no members? Then do lookup of the `CustomType` and make sure the type we get back is the one with the `int` member? (or not, depending on the test) https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
@@ -273,6 +301,44 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType( if (!isType(entry.tag())) continue; + +DWARFTypeUnit *foreign_tu = GetForeignTypeUnit(entry); +if (foreign_tu) { + // If this entry represents a foreign type unit, we need to verify that + // the type unit that ended up in the final .dwp file is the right type + // unit. Type units have signatures which are the same across multiple + // .dwo files, but only one of those type units will end up in the .dwp + // file. The contents of type units for the same type can be different + // in different .dwo file, which means the DIE offsets might not be the + // same between two different type units. So we need to determine if this + // accelerator table matches the type unit in the .dwp file. If it doesn't + // match, then we need to ignore this accelerator table entry as the type + // unit that is in the .dwp file will have its own index. + const llvm::DWARFDebugNames::NameIndex *name_index = entry.getNameIndex(); + if (name_index == nullptr) +continue; + // In order to determine if the type unit that ended up in a .dwp file + // is valid, we need to grab the type unit and check the attribute on the + // type unit matches the .dwo file. For this to happen we rely on each + // .dwo file having its own .debug_names table with a single compile unit + // and multiple type units. This is the only way we can tell if a type + // unit came from a specific .dwo file. dwblaikie wrote: Ah, indeed - no, currently there's no well defined way to do LTO (LTO is basically the only time you get multiple CUs in a single .o straight out of the compiler) and Split DWARF (there's that long-standing issue that Cary and I should discuss how that should work & propose something to the DWARF committee, but we haven't as-yet). I guess even if we did make that work, all the skeleton CUs in the dwo file would have the same DW_AT_dwo_name https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/test] Add basic ld.lld --debug-names tests (PR #88335)
dwblaikie wrote: looks approximately right to me, but wouldn't' mind a set of eyes more familiar with lldb to take a look https://github.com/llvm/llvm-project/pull/88335 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] lldb simplified template names rebuild without clang ast (PR #90008)
https://github.com/dwblaikie created https://github.com/llvm/llvm-project/pull/90008 - DO NOT SUBMIT: lldb DWARF-Clang AST parsing tracing - Fix scope operator ordering - DO NOT SUBMIT: Really dodgy demonstration of DWARFTypePrinter reuse in lldb >From 863343317c47602163d75c13b2687601740e8410 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Fri, 19 Apr 2024 03:34:27 + Subject: [PATCH 1/3] DO NOT SUBMIT: lldb DWARF-Clang AST parsing tracing --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp| 13 + 1 file changed, 13 insertions(+) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 54d06b1115a229..17445e0c3ad17d 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -7,6 +7,7 @@ //===--===// #include +#include #include "DWARFASTParser.h" #include "DWARFASTParserClang.h" @@ -1556,6 +1557,15 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) { const char *name = die.GetName(); if (!name) return ""; + static int indent = 0; + std::cerr << std::string(indent, ' ') << "starting qualified name for: " << name << '\n'; + auto &FS = die.GetCU()->GetSymbolFileDWARF().GetObjectFile()->GetFileSpec(); + std::string Directory = FS.GetDirectory().AsCString(""); + std::cerr << std::string(indent, ' ') +<< Directory.substr(std::min(59ul, Directory.size())) << '/' +<< FS.GetFilename().AsCString("") << ':' << std::hex +<< die.GetDIE()->GetOffset() << '\n'; + ++indent; std::string qualified_name; DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE(); // TODO: change this to get the correct decl context parent @@ -1601,6 +1611,9 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) { qualified_name.append(name); qualified_name.append(GetDIEClassTemplateParams(die).AsCString("")); + --indent; + std::cerr << std::string(indent, ' ') << "computed qualified name: " << qualified_name << '\n'; + return qualified_name; } >From b2926499710b7bf673111aabc5be51597a46493c Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Thu, 25 Apr 2024 00:28:57 + Subject: [PATCH 2/3] Fix scope operator ordering --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 17445e0c3ad17d..ab181b482e4fbc 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -1590,9 +1590,9 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) { case DW_TAG_structure_type: case DW_TAG_union_type: { if (const char *class_union_struct_name = parent_decl_ctx_die.GetName()) { +qualified_name.insert(0, "::"); qualified_name.insert( 0, GetDIEClassTemplateParams(parent_decl_ctx_die).AsCString("")); -qualified_name.insert(0, "::"); qualified_name.insert(0, class_union_struct_name); } parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE(); >From 9a654b056d9c05c0aa4856db161c1f1b08b9dfe9 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Thu, 25 Apr 2024 00:46:48 + Subject: [PATCH 3/3] DO NOT SUBMIT: Really dodgy demonstration of DWARFTypePrinter reuse in lldb The hacks necessary to make lldb's DWARFDIE APIs sufficiently compatible with LLVM's DWARFDie API aren't shippable, but maybe somewhere to start the conversation. With all these changes, an internal example that would crash expanding too many types (computing the fully qualified name for 414671 types before crashing due to running out of stack) - but with these patches applied, it comes down to 856 expansions (compared to 848 for non-simplified template names inputs) --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 11 + .../Plugins/SymbolFile/DWARF/DWARFBaseDIE.h | 10 + .../Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 21 +- .../Plugins/SymbolFile/DWARF/DWARFDIE.h | 13 + .../Plugins/SymbolFile/DWARF/DWARFFormValue.h | 37 + .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 24 +- llvm/include/llvm-c/Error.h | 8 + llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h | 2 + .../llvm/DebugInfo/DWARF/DWARFFormValue.h | 7 +- .../llvm/DebugInfo/DWARF/DWARFTypePrinter.h | 734 +- llvm/lib/DebugInfo/DWARF/DWARFDie.cpp | 4 +- llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp | 674 12 files changed, 830 insertions(+), 715 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
dwblaikie wrote: does this cause multiple (an open ended amount?) of declarations for a type to be created if the type declarations from multiple CUs are encountered separately? Or still only one due to the extra map? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
dwblaikie wrote: Hmm - but the byte size wouldn't be known from only a declaration, right? so how'd that key work between a declaration and a definition? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 41574f5 - Add new BuiltinType introduced in 7a484d3a1f630ba9ce7b22e744818be974971470
Author: David Blaikie Date: 2024-05-05T17:59:54Z New Revision: 41574f5a6e2d961f398d3c671c34ac3c8e417464 URL: https://github.com/llvm/llvm-project/commit/41574f5a6e2d961f398d3c671c34ac3c8e417464 DIFF: https://github.com/llvm/llvm-project/commit/41574f5a6e2d961f398d3c671c34ac3c8e417464.diff LOG: Add new BuiltinType introduced in 7a484d3a1f630ba9ce7b22e744818be974971470 I don't think this is one lldb would encounter when building ASTs from DWARF. Added: Modified: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp Removed: diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 3bdb288e97dd6a..a771016039e851 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -4996,6 +4996,9 @@ lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type, case clang::BuiltinType::IncompleteMatrixIdx: break; + +case clang::BuiltinType::UnresolvedTemplate: + break; } break; // All pointer types are represented as unsigned integer encodings. We may ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -16,61 +16,66 @@ using namespace lldb_private::plugin::dwarf; bool UniqueDWARFASTTypeList::Find(const DWARFDIE &die, const lldb_private::Declaration &decl, const int32_t byte_size, + bool is_forward_declaration, UniqueDWARFASTType &entry) const { for (const UniqueDWARFASTType &udt : m_collection) { // Make sure the tags match if (udt.m_die.Tag() == die.Tag()) { - // Validate byte sizes of both types only if both are valid. - if (udt.m_byte_size < 0 || byte_size < 0 || - udt.m_byte_size == byte_size) { -// Make sure the file and line match -if (udt.m_declaration == decl) { - // The type has the same name, and was defined on the same file and - // line. Now verify all of the parent DIEs match. - DWARFDIE parent_arg_die = die.GetParent(); - DWARFDIE parent_pos_die = udt.m_die.GetParent(); - bool match = true; - bool done = false; - while (!done && match && parent_arg_die && parent_pos_die) { -const dw_tag_t parent_arg_tag = parent_arg_die.Tag(); -const dw_tag_t parent_pos_tag = parent_pos_die.Tag(); -if (parent_arg_tag == parent_pos_tag) { - switch (parent_arg_tag) { - case DW_TAG_class_type: - case DW_TAG_structure_type: - case DW_TAG_union_type: - case DW_TAG_namespace: { -const char *parent_arg_die_name = parent_arg_die.GetName(); -if (parent_arg_die_name == -nullptr) // Anonymous (i.e. no-name) struct -{ - match = false; -} else { - const char *parent_pos_die_name = parent_pos_die.GetName(); - if (parent_pos_die_name == nullptr || - ((parent_arg_die_name != parent_pos_die_name) && - strcmp(parent_arg_die_name, parent_pos_die_name))) -match = false; -} - } break; - - case DW_TAG_compile_unit: - case DW_TAG_partial_unit: -done = true; -break; - default: -break; - } + // If they are not both definition DIEs or both declaration DIEs, then + // don't check for byte size and declaration location, because declaration + // DIEs usually don't have those info. + bool matching_size_declaration = + udt.m_is_forward_declaration != is_forward_declaration + ? true + : (udt.m_byte_size < 0 || byte_size < 0 || + udt.m_byte_size == byte_size) && +udt.m_declaration == decl; + if (!matching_size_declaration) +continue; + // The type has the same name, and was defined on the same file and + // line. Now verify all of the parent DIEs match. + DWARFDIE parent_arg_die = die.GetParent(); + DWARFDIE parent_pos_die = udt.m_die.GetParent(); + bool match = true; + bool done = false; + while (!done && match && parent_arg_die && parent_pos_die) { +const dw_tag_t parent_arg_tag = parent_arg_die.Tag(); +const dw_tag_t parent_pos_tag = parent_pos_die.Tag(); +if (parent_arg_tag == parent_pos_tag) { + switch (parent_arg_tag) { + case DW_TAG_class_type: + case DW_TAG_structure_type: + case DW_TAG_union_type: + case DW_TAG_namespace: { +const char *parent_arg_die_name = parent_arg_die.GetName(); +if (parent_arg_die_name == +nullptr) // Anonymous (i.e. no-name) struct +{ dwblaikie wrote: The comment between the `)` and `{` creates some weird line wrapping, maybe put the comment inside the `{` block? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -0,0 +1,40 @@ +# Test definition DIE searching is delayed until complete type is required. + +# RUN: split-file %s %t +# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -gsimple-template-names -o %t.out +# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s + +# CHECK: (lldb) p v1 +# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type (DW_TAG_structure_type) name = 't2' +# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type (DW_TAG_structure_type) name = 't1' +# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type (DW_TAG_base_type) name = 'int' +# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't2' resolving forward declaration... +# CHECK: (t2 >) {} +# CHECK: (lldb) p v2 +# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type (DW_TAG_structure_type) name = 't1' +# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type (DW_TAG_base_type) name = 'int' +# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't1' resolving forward declaration... +# CHECK: (t1) {} + +#--- lldb.cmd dwblaikie wrote: Might be better to test this without `-gsimple-template-names`, because it's a more generally valuable feature than that? Oh, but that "look for the child count through a pointer" issue comes up and it's hard to reach this situation without `-gsimple-template-names`? (Maybe indirectly - if you print a struct with a pointer - perhaps that doesn't do the child count searching through the pointer?) https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -24,13 +24,16 @@ class UniqueDWARFASTType { UniqueDWARFASTType() : m_type_sp(), m_die(), m_declaration() {} UniqueDWARFASTType(lldb::TypeSP &type_sp, const DWARFDIE &die, - const Declaration &decl, int32_t byte_size) + const Declaration &decl, int32_t byte_size, + bool is_forward_declaration) : m_type_sp(type_sp), m_die(die), m_declaration(decl), -m_byte_size(byte_size) {} +m_byte_size(byte_size), +m_is_forward_declaration(is_forward_declaration) {} dwblaikie wrote: Where is this ctor used? I can't find any calls to it. And if it is never used, how is `m_is_forward_declaration` ever `false`, if it's initialized to true and this ctor is never called, I don't see any other place that assigns to it? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1632,27 +1632,34 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) { return true; } - DWARFDIE dwarf_die = GetDIE(die_it->getSecond()); - if (dwarf_die) { -// Once we start resolving this type, remove it from the forward -// declaration map in case anyone child members or other types require this -// type to get resolved. The type will get resolved when all of the calls -// to SymbolFileDWARF::ResolveClangOpaqueTypeDefinition are done. -GetForwardDeclCompilerTypeToDIE().erase(die_it); - -Type *type = GetDIEToType().lookup(dwarf_die.GetDIE()); + // Once we start resolving this type, remove it from the forward + // declaration map in case anyone's child members or other types require this + // type to get resolved. + DWARFDIE dwarf_die = GetDIE(die_it->second); + GetForwardDeclCompilerTypeToDIE().erase(die_it); + Type *type = nullptr; + if (DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU())) +type = dwarf_ast->FindDefinitionTypeForDIE(dwarf_die); + if (!type) +return false; -Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion); -if (log) - GetObjectFile()->GetModule()->LogMessageVerboseBacktrace( - log, "{0:x8}: {1} ({2}) '{3}' resolving forward declaration...", - dwarf_die.GetID(), DW_TAG_value_to_name(dwarf_die.Tag()), - dwarf_die.Tag(), type->GetName().AsCString()); -assert(compiler_type); -if (DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU())) - return dwarf_ast->CompleteTypeFromDWARF(dwarf_die, type, compiler_type); + die_it = GetForwardDeclCompilerTypeToDIE().find( + compiler_type_no_qualifiers.GetOpaqueQualType()); + if (die_it != GetForwardDeclCompilerTypeToDIE().end()) { +dwarf_die = GetDIE(die_it->getSecond()); +GetForwardDeclCompilerTypeToDIE().erase(die_it); } - return false; + + Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion); + if (log) dwblaikie wrote: ``` if (Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion)) ``` https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -0,0 +1,40 @@ +# Test definition DIE searching is delayed until complete type is required. + +# RUN: split-file %s %t +# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -gsimple-template-names -o %t.out +# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s + +# CHECK: (lldb) p v1 +# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type (DW_TAG_structure_type) name = 't2' +# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type (DW_TAG_structure_type) name = 't1' +# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type (DW_TAG_base_type) name = 'int' +# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't2' resolving forward declaration... +# CHECK: (t2 >) {} +# CHECK: (lldb) p v2 +# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type (DW_TAG_structure_type) name = 't1' +# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_base_type (DW_TAG_base_type) name = 'int' +# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't1' resolving forward declaration... +# CHECK: (t1) {} + +#--- lldb.cmd dwblaikie wrote: Hmm - what about template parameters, even without simplified template names? `t1` t2 should be a declaration, without looking up the definition DIE in this case? (though perhaps clang/LLDB doesn't do that correctly, in which case maybe `t1` might work) https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -24,13 +24,16 @@ class UniqueDWARFASTType { UniqueDWARFASTType() : m_type_sp(), m_die(), m_declaration() {} UniqueDWARFASTType(lldb::TypeSP &type_sp, const DWARFDIE &die, - const Declaration &decl, int32_t byte_size) + const Declaration &decl, int32_t byte_size, + bool is_forward_declaration) : m_type_sp(type_sp), m_die(die), m_declaration(decl), -m_byte_size(byte_size) {} +m_byte_size(byte_size), +m_is_forward_declaration(is_forward_declaration) {} dwblaikie wrote: Ah, cool - thanks, sorry I missed that. If you wanted to remove the dead ctor either before or after this commit, that might be handy/nice. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] 58473d8 - [lldb] Use LLDB_VERSION_STRING instead of CLANG_VERSION_STRING
Any chance of test coverage? On Mon, Dec 13, 2021 at 4:58 PM Jonas Devlieghere via lldb-commits < lldb-commits@lists.llvm.org> wrote: > > Author: Jonas Devlieghere > Date: 2021-12-13T16:58:39-08:00 > New Revision: 58473d84e0c7796de5dcfd3153e5d5cc8ad034b3 > > URL: > https://github.com/llvm/llvm-project/commit/58473d84e0c7796de5dcfd3153e5d5cc8ad034b3 > DIFF: > https://github.com/llvm/llvm-project/commit/58473d84e0c7796de5dcfd3153e5d5cc8ad034b3.diff > > LOG: [lldb] Use LLDB_VERSION_STRING instead of CLANG_VERSION_STRING > > Added: > > > Modified: > lldb/source/Version/Version.cpp > > Removed: > > > > > > diff --git a/lldb/source/Version/Version.cpp > b/lldb/source/Version/Version.cpp > index ae2d83bd3e5f8..b391fe1eacd80 100644 > --- a/lldb/source/Version/Version.cpp > +++ b/lldb/source/Version/Version.cpp > @@ -15,7 +15,7 @@ static const char *GetLLDBVersion() { > #ifdef LLDB_FULL_VERSION_STRING >return LLDB_FULL_VERSION_STRING; > #else > - return "lldb version " CLANG_VERSION_STRING; > + return "lldb version " LLDB_VERSION_STRING; > #endif > } > > > > > ___ > lldb-commits mailing list > lldb-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 8b280df - Rough guess at fixing lldb tests to handle Clang defaulting to DWARFv5
Author: David Blaikie Date: 2022-01-23T21:24:25-08:00 New Revision: 8b280df504b97a13d06a929fbc85348903456fdd URL: https://github.com/llvm/llvm-project/commit/8b280df504b97a13d06a929fbc85348903456fdd DIFF: https://github.com/llvm/llvm-project/commit/8b280df504b97a13d06a929fbc85348903456fdd.diff LOG: Rough guess at fixing lldb tests to handle Clang defaulting to DWARFv5 Added: Modified: lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-dwo-cross-reference.cpp Removed: diff --git a/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py b/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py index 4b9a814764158..f4ae1fc015569 100644 --- a/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py +++ b/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py @@ -15,4 +15,4 @@ lldbinline.MakeInlineTest(__file__, globals(), decorators=decorators+[skipIf(debug_info="dsym")], name="BasicEntryValues_GNU", -build_dict=dict(CXXFLAGS_EXTRAS="-O2 -ggdb")) +build_dict=dict(CXXFLAGS_EXTRAS="-O2 -ggdb -gdwarf-4")) diff --git a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py index cbdf40e2416f7..19aad2ab1ec32 100644 --- a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py +++ b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py @@ -7,5 +7,5 @@ lldbinline.MakeInlineTest(__file__, globals(), name="UnambiguousTailCalls_V5", build_dict=dict(CFLAGS_EXTRAS="-O2 -glldb"), decorators=decor) lldbinline.MakeInlineTest(__file__, globals(), name="UnambiguousTailCalls_GNU", -build_dict=dict(CFLAGS_EXTRAS="-O2 -ggdb"), +build_dict=dict(CFLAGS_EXTRAS="-O2 -ggdb -gdwarf-4"), decorators=decor+[decorators.skipIf(debug_info="dsym")]) diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-dwo-cross-reference.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-dwo-cross-reference.cpp index 29adff62cd1ee..0e29cb3e7f16e 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-dwo-cross-reference.cpp +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-dwo-cross-reference.cpp @@ -7,8 +7,8 @@ // RUN: -fdebug-types-section -gsplit-dwarf -c -o %t1.o -DONE // RUN: %clang %s -target x86_64-pc-linux -fno-standalone-debug -g \ // RUN: -fdebug-types-section -gsplit-dwarf -c -o %t2.o -DTWO -// RUN: llvm-dwarfdump %t1.dwo -debug-types | FileCheck --check-prefix=ONEUNIT %s -// RUN: llvm-dwarfdump %t2.dwo -debug-types | FileCheck --check-prefix=ONEUNIT %s +// RUN: llvm-dwarfdump %t1.dwo -debug-types -debug-info | FileCheck --check-prefix=ONEUNIT %s +// RUN: llvm-dwarfdump %t2.dwo -debug-types -debug-info | FileCheck --check-prefix=ONEUNIT %s // RUN: ld.lld %t1.o %t2.o -o %t // RUN: %lldb %t -o "target var a b **b.a" -b | FileCheck %s ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 65cac0e - Use StringRef to avoid unnecessary copies into std::strings
Author: David Blaikie Date: 2022-07-07T19:50:12Z New Revision: 65cac0ed9266e3551663358de677161ce25a25bf URL: https://github.com/llvm/llvm-project/commit/65cac0ed9266e3551663358de677161ce25a25bf DIFF: https://github.com/llvm/llvm-project/commit/65cac0ed9266e3551663358de677161ce25a25bf.diff LOG: Use StringRef to avoid unnecessary copies into std::strings Added: Modified: lldb/include/lldb/Symbol/TypeList.h lldb/include/lldb/Symbol/TypeMap.h lldb/source/Core/Module.cpp lldb/source/Symbol/TypeList.cpp lldb/source/Symbol/TypeMap.cpp Removed: diff --git a/lldb/include/lldb/Symbol/TypeList.h b/lldb/include/lldb/Symbol/TypeList.h index 03390858025b..403469c989f5 100644 --- a/lldb/include/lldb/Symbol/TypeList.h +++ b/lldb/include/lldb/Symbol/TypeList.h @@ -49,10 +49,11 @@ class TypeList { void ForEach(std::function const &callback); - void RemoveMismatchedTypes(const char *qualified_typename, bool exact_match); + void RemoveMismatchedTypes(llvm::StringRef qualified_typename, + bool exact_match); - void RemoveMismatchedTypes(const std::string &type_scope, - const std::string &type_basename, + void RemoveMismatchedTypes(llvm::StringRef type_scope, + llvm::StringRef type_basename, lldb::TypeClass type_class, bool exact_match); void RemoveMismatchedTypes(lldb::TypeClass type_class); diff --git a/lldb/include/lldb/Symbol/TypeMap.h b/lldb/include/lldb/Symbol/TypeMap.h index ede54c1a09d4..064e2cc948b6 100644 --- a/lldb/include/lldb/Symbol/TypeMap.h +++ b/lldb/include/lldb/Symbol/TypeMap.h @@ -53,10 +53,11 @@ class TypeMap { bool Remove(const lldb::TypeSP &type_sp); - void RemoveMismatchedTypes(const char *qualified_typename, bool exact_match); + void RemoveMismatchedTypes(llvm::StringRef qualified_typename, + bool exact_match); - void RemoveMismatchedTypes(const std::string &type_scope, - const std::string &type_basename, + void RemoveMismatchedTypes(llvm::StringRef type_scope, + llvm::StringRef type_basename, lldb::TypeClass type_class, bool exact_match); void RemoveMismatchedTypes(lldb::TypeClass type_class); diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp index 41c21e1dc326..c05b40c4362e 100644 --- a/lldb/source/Core/Module.cpp +++ b/lldb/source/Core/Module.cpp @@ -1000,8 +1000,7 @@ void Module::FindTypes( FindTypes_Impl(type_basename_const_str, CompilerDeclContext(), max_matches, searched_symbol_files, typesmap); if (typesmap.GetSize()) - typesmap.RemoveMismatchedTypes(std::string(type_scope), - std::string(type_basename), type_class, + typesmap.RemoveMismatchedTypes(type_scope, type_basename, type_class, exact_match); } else { // The type is not in a namespace/class scope, just search for it by @@ -1011,15 +1010,13 @@ void Module::FindTypes( // class prefix (like "struct", "class", "union", "typedef" etc). FindTypes_Impl(ConstString(type_basename), CompilerDeclContext(), UINT_MAX, searched_symbol_files, typesmap); - typesmap.RemoveMismatchedTypes(std::string(type_scope), - std::string(type_basename), type_class, + typesmap.RemoveMismatchedTypes(type_scope, type_basename, type_class, exact_match); } else { FindTypes_Impl(name, CompilerDeclContext(), UINT_MAX, searched_symbol_files, typesmap); if (exact_match) { -std::string name_str(name.AsCString("")); -typesmap.RemoveMismatchedTypes(std::string(type_scope), name_str, +typesmap.RemoveMismatchedTypes(type_scope, name.AsCString(""), type_class, exact_match); } } diff --git a/lldb/source/Symbol/TypeList.cpp b/lldb/source/Symbol/TypeList.cpp index ace715d933ea..494e59e3a0fc 100644 --- a/lldb/source/Symbol/TypeList.cpp +++ b/lldb/source/Symbol/TypeList.cpp @@ -97,7 +97,7 @@ void TypeList::Dump(Stream *s, bool show_context) { } } -void TypeList::RemoveMismatchedTypes(const char *qualified_typename, +void TypeList::RemoveMismatchedTypes(llvm::StringRef qualified_typename, bool exact_match) { llvm::StringRef type_scope; llvm::StringRef type_basename; @@ -107,13 +107,12 @@ void TypeList::RemoveMismatchedTypes(const char *qualified_typename, type_basename = qualified_typename; type_scope = ""; } - return RemoveMismatchedTypes(std::string(type_scope), - std::string(type_basename), type_class, + return RemoveMismatchedTy
[Lldb-commits] [lldb] 856ebe9 - Retrieve as StringRef since that's how it'll be used
Author: David Blaikie Date: 2022-07-07T20:13:36Z New Revision: 856ebe9e8247698095a66f98647ee5d7cb12f337 URL: https://github.com/llvm/llvm-project/commit/856ebe9e8247698095a66f98647ee5d7cb12f337 DIFF: https://github.com/llvm/llvm-project/commit/856ebe9e8247698095a66f98647ee5d7cb12f337.diff LOG: Retrieve as StringRef since that's how it'll be used Added: Modified: lldb/source/Core/Module.cpp Removed: diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp index c05b40c4362e..43d1cdb5bd38 100644 --- a/lldb/source/Core/Module.cpp +++ b/lldb/source/Core/Module.cpp @@ -1016,7 +1016,7 @@ void Module::FindTypes( FindTypes_Impl(name, CompilerDeclContext(), UINT_MAX, searched_symbol_files, typesmap); if (exact_match) { -typesmap.RemoveMismatchedTypes(type_scope, name.AsCString(""), +typesmap.RemoveMismatchedTypes(type_scope, name.GetStringRef(), type_class, exact_match); } } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 6edbde1 - Simplify some AsCString usage that was also explicitly handling default
Author: David Blaikie Date: 2022-07-07T20:27:05Z New Revision: 6edbde100132f5dc025bed64059d9fb770abd19e URL: https://github.com/llvm/llvm-project/commit/6edbde100132f5dc025bed64059d9fb770abd19e DIFF: https://github.com/llvm/llvm-project/commit/6edbde100132f5dc025bed64059d9fb770abd19e.diff LOG: Simplify some AsCString usage that was also explicitly handling default Added: Modified: lldb/source/Core/Module.cpp Removed: diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp index 43d1cdb5bd38..893e20837124 100644 --- a/lldb/source/Core/Module.cpp +++ b/lldb/source/Core/Module.cpp @@ -144,9 +144,7 @@ Module::Module(const ModuleSpec &module_spec) module_spec.GetArchitecture().GetArchitectureName(), module_spec.GetFileSpec().GetPath().c_str(), module_spec.GetObjectName().IsEmpty() ? "" : "(", - module_spec.GetObjectName().IsEmpty() - ? "" - : module_spec.GetObjectName().AsCString(""), + module_spec.GetObjectName().AsCString(""), module_spec.GetObjectName().IsEmpty() ? "" : ")"); auto data_sp = module_spec.GetData(); @@ -254,8 +252,7 @@ Module::Module(const FileSpec &file_spec, const ArchSpec &arch, LLDB_LOGF(log, "%p Module::Module((%s) '%s%s%s%s')", static_cast(this), m_arch.GetArchitectureName(), m_file.GetPath().c_str(), m_object_name.IsEmpty() ? "" : "(", - m_object_name.IsEmpty() ? "" : m_object_name.AsCString(""), - m_object_name.IsEmpty() ? "" : ")"); + m_object_name.AsCString(""), m_object_name.IsEmpty() ? "" : ")"); } Module::Module() : m_file_has_changed(false), m_first_file_changed_log(false) { @@ -283,8 +280,7 @@ Module::~Module() { LLDB_LOGF(log, "%p Module::~Module((%s) '%s%s%s%s')", static_cast(this), m_arch.GetArchitectureName(), m_file.GetPath().c_str(), m_object_name.IsEmpty() ? "" : "(", - m_object_name.IsEmpty() ? "" : m_object_name.AsCString(""), - m_object_name.IsEmpty() ? "" : ")"); + m_object_name.AsCString(""), m_object_name.IsEmpty() ? "" : ")"); // Release any auto pointers before we start tearing down our member // variables since the object file and symbol files might need to make // function calls back into this module object. The ordering is important ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] b92c334 - Remove dead code: TypeMap::RemoveMismatchedTypes(TypeClass type_class)
Author: David Blaikie Date: 2022-07-07T20:46:14Z New Revision: b92c33495aeda7d7fa7f5d3f518c59cc5785fd9f URL: https://github.com/llvm/llvm-project/commit/b92c33495aeda7d7fa7f5d3f518c59cc5785fd9f DIFF: https://github.com/llvm/llvm-project/commit/b92c33495aeda7d7fa7f5d3f518c59cc5785fd9f.diff LOG: Remove dead code: TypeMap::RemoveMismatchedTypes(TypeClass type_class) Added: Modified: lldb/include/lldb/Symbol/TypeMap.h lldb/source/Symbol/TypeMap.cpp Removed: diff --git a/lldb/include/lldb/Symbol/TypeMap.h b/lldb/include/lldb/Symbol/TypeMap.h index 064e2cc948b6..c200ccb9844f 100644 --- a/lldb/include/lldb/Symbol/TypeMap.h +++ b/lldb/include/lldb/Symbol/TypeMap.h @@ -53,15 +53,10 @@ class TypeMap { bool Remove(const lldb::TypeSP &type_sp); - void RemoveMismatchedTypes(llvm::StringRef qualified_typename, - bool exact_match); - void RemoveMismatchedTypes(llvm::StringRef type_scope, llvm::StringRef type_basename, lldb::TypeClass type_class, bool exact_match); - void RemoveMismatchedTypes(lldb::TypeClass type_class); - private: typedef collection::iterator iterator; typedef collection::const_iterator const_iterator; diff --git a/lldb/source/Symbol/TypeMap.cpp b/lldb/source/Symbol/TypeMap.cpp index 7217a29fc002..0d5f6d53e5a0 100644 --- a/lldb/source/Symbol/TypeMap.cpp +++ b/lldb/source/Symbol/TypeMap.cpp @@ -127,20 +127,6 @@ void TypeMap::Dump(Stream *s, bool show_context, lldb::DescriptionLevel level) { } } -void TypeMap::RemoveMismatchedTypes(llvm::StringRef qualified_typename, -bool exact_match) { - llvm::StringRef type_scope; - llvm::StringRef type_basename; - TypeClass type_class = eTypeClassAny; - if (!Type::GetTypeScopeAndBasename(qualified_typename, type_scope, - type_basename, type_class)) { -type_basename = qualified_typename; -type_scope = ""; - } - return RemoveMismatchedTypes(type_scope, type_basename, type_class, - exact_match); -} - void TypeMap::RemoveMismatchedTypes(llvm::StringRef type_scope, llvm::StringRef type_basename, TypeClass type_class, bool exact_match) { @@ -213,25 +199,3 @@ void TypeMap::RemoveMismatchedTypes(llvm::StringRef type_scope, } m_types.swap(matching_types); } - -void TypeMap::RemoveMismatchedTypes(TypeClass type_class) { - if (type_class == eTypeClassAny) -return; - - // Our "collection" type currently is a std::map which doesn't have any good - // way to iterate and remove items from the map so we currently just make a - // new list and add all of the matching types to it, and then swap it into - // m_types at the end - collection matching_types; - - iterator pos, end = m_types.end(); - - for (pos = m_types.begin(); pos != end; ++pos) { -Type *the_type = pos->second.get(); -TypeClass match_type_class = -the_type->GetForwardCompilerType().GetTypeClass(); -if (match_type_class & type_class) - matching_types.insert(*pos); - } - m_types.swap(matching_types); -} ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 72d9390 - Add a little extra test coverage for simple template names
Author: David Blaikie Date: 2022-07-08T00:12:29Z New Revision: 72d9390778966d4f87ec4b1de63c107b2fd46b9a URL: https://github.com/llvm/llvm-project/commit/72d9390778966d4f87ec4b1de63c107b2fd46b9a DIFF: https://github.com/llvm/llvm-project/commit/72d9390778966d4f87ec4b1de63c107b2fd46b9a.diff LOG: Add a little extra test coverage for simple template names This would fail with an overly naive approach to simple template name (clang's -gsimple-template-names) since the names wouldn't be unique per specialization, creating ambiguity/chance that a query for one specialization would find another. Added: Modified: lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py Removed: diff --git a/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py b/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py index b596611b15f2f..81a8876743474 100644 --- a/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py +++ b/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py @@ -14,6 +14,11 @@ def assertComplete(self, typename): self.assertTrue(found_type.IsValid()) self.assertTrue(found_type.IsTypeComplete()) +def assertIsNotPresent(self, typename): +""" Asserts that the type with the given name is not found. """ +found_type = self.target().FindFirstType(typename) +self.assertFalse(found_type.IsValid()) + def assertCompleteWithVar(self, typename): """ Asserts that the type with the given name is complete. """ found_type = self.target().FindFirstType(typename) @@ -41,6 +46,7 @@ def test_forward_declarations(self): self.assertCompleteWithVar("DefinedClass") self.assertCompleteWithVar("DefinedClassTypedef") self.assertCompleteWithVar("DefinedTemplateClass") +self.assertIsNotPresent("DefinedTemplateClass") # Record types without a defining declaration are not complete. self.assertPointeeIncomplete("FwdClass *", "fwd_class") ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 9fc6565 - fold assert-only variable into assert to address non-assert -Wunused-variable
Author: David Blaikie Date: 2022-08-17T04:10:59Z New Revision: 9fc65658f5e5a0af5821f4945d78a9f6b8fd368f URL: https://github.com/llvm/llvm-project/commit/9fc65658f5e5a0af5821f4945d78a9f6b8fd368f DIFF: https://github.com/llvm/llvm-project/commit/9fc65658f5e5a0af5821f4945d78a9f6b8fd368f.diff LOG: fold assert-only variable into assert to address non-assert -Wunused-variable Added: Modified: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp Removed: diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index f815f3ad7d41..e298d7fee421 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -7188,8 +7188,7 @@ GetNthTemplateArgument(const clang::ClassTemplateSpecializationDecl *decl, // (including the ones preceding the parameter pack). const auto &pack = args[last_idx]; const size_t pack_idx = idx - last_idx; - const size_t pack_size = pack.pack_size(); - assert(pack_idx < pack_size && "parameter pack index out-of-bounds"); + assert(pack_idx < pack.pack_size() && "parameter pack index out-of-bounds"); return &pack.pack_elements()[pack_idx]; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lldb] [c++20] P1907R1: Support for generalized non-type template arguments of scalar type. (PR #78041)
@@ -5401,6 +5409,8 @@ std::string CGDebugInfo::GetName(const Decl *D, bool Qualified) const { // feasible some day. return TA.getAsIntegral().getBitWidth() <= 64 && IsReconstitutableType(TA.getIntegralType()); + case TemplateArgument::StructuralValue: +return false; dwblaikie wrote: Sorry for the delays in the debug info review for this - it is on my list :/ https://github.com/llvm/llvm-project/pull/78041 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix a crash when using .dwp files and make type lookup reliable with the index cache (PR #79544)
dwblaikie wrote: I'm not following all of this, but it appears to be based on the premise that it's OK that sometimes split units inside a DWP file are parsed before their skeleton unit? Why is that OK/when/why/where is that happening? I'd think any case where that happens would be a performance (& possibly correctness bug) & it'd be better to maintain the invariant that the only way you ever parse a split unit is starting from the skeleton - rather than adding maps/etc to be able to backtrack to parsing the skeleton when you already have the split unit. https://github.com/llvm/llvm-project/pull/79544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix expressions that involve nested structs/classes/unions. (PR #77029)
dwblaikie wrote: > > Thanks for the fix! I did test this patch with both our regression issue, > > and also allowing us to remove the double-lookup working around #53904 and > > can confirm it addressed both issues. Thanks again! > > Glad it worked! It will also be a huge improvement in the expression parser > and avoid these weird expression parser bugs where it doesn't work sometimes > and does other times, so it was totally worth fixing. I am also glad to know > that the type lookups are being so much more efficient. Having a repro case > is always the best way to help us fix bugs, so thanks for posting the details > so we can repro easily. Turns out it actually got worse, see https://github.com/llvm/llvm-project/issues/79668 - now whether or not a given nested type is accessible via name lookup depends on which copy of the outer type is loaded first & there's no workaround that I know of - the lookup then fails/succeeds permanently. Be good to get this addressed - as now it's not possible to lookup these nested members at all reliably (even the "repeat the lookup" workaround doesn't work - maybe there are other workarounds? But none that I can think of) https://github.com/llvm/llvm-project/pull/77029 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFIndex] Use IDX_parent to implement GetFullyQualifiedType query (PR #79932)
@@ -0,0 +1,210 @@ +//===-- DWARFDIETest.cpp --=---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "Plugins/SymbolFile/DWARF/DWARFDIE.h" +#include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h" +#include "Plugins/SymbolFile/DWARF/DWARFDeclContext.h" +#include "Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h" +#include "TestingSupport/Symbol/YAMLModuleTester.h" +#include "llvm/ADT/STLExtras.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace lldb; +using namespace lldb_private; +using namespace lldb_private::plugin::dwarf; +using StringRef = llvm::StringRef; + +void check_num_matches(DebugNamesDWARFIndex &index, int expected_num_matches, + llvm::ArrayRef ctx_entries) { + DWARFDeclContext ctx(ctx_entries); + int num_matches = 0; + auto increment_matches = [&](DWARFDIE die) { +num_matches++; +return true; + }; + + index.GetFullyQualifiedType(ctx, increment_matches); + ASSERT_EQ(num_matches, expected_num_matches); +} + +TEST(DWARFDebugNamesIndexTest, FullyQualifiedQueryWithIDXParent) { + const char *yamldata = R"( +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data:ELFDATA2LSB + Type:ET_EXEC + Machine: EM_386 +DWARF: + debug_str: +- '1' +- '2' +- '3' + debug_abbrev: +- Table: +# We intentionally don't nest types in debug_info: if the nesting is not +# inferred from debug_names, we want the test to fail. +- Code:0x1 + Tag: DW_TAG_compile_unit + Children:DW_CHILDREN_yes +- Code:0x2 + Tag: DW_TAG_class_type + Children:DW_CHILDREN_no + Attributes: +- Attribute: DW_AT_name + Form:DW_FORM_strp + debug_info: +- Version: 4 + AddrSize:8 + Entries: +- AbbrCode:0x1 +- AbbrCode:0x2 + Values: +- Value: 0x0 # Name "1" +- AbbrCode:0x2 + Values: +- Value: 0x2 # Name "2" +- AbbrCode:0x2 + Values: +- Value: 0x4 # Name "3" +- AbbrCode:0x0 + debug_names: +Abbreviations: +- Code: 0x11 + Tag: DW_TAG_class_type + Indices: +- Idx: DW_IDX_parent + Form: DW_FORM_flag_present +- Idx: DW_IDX_die_offset + Form: DW_FORM_ref4 +- Code: 0x22 + Tag: DW_TAG_class_type + Indices: +- Idx: DW_IDX_parent + Form: DW_FORM_ref4 +- Idx: DW_IDX_die_offset + Form: DW_FORM_ref4 +Entries: +- Name: 0x0 # strp to Name1 + Code: 0x11 + Values: +- 0xc # Die offset to entry named "1" +- Name: 0x2 # strp to Name2 + Code: 0x22 + Values: +- 0x0 # Parent = First entry ("1") +- 0x11 # Die offset to entry named "1:2" +- Name: 0x4 # strp to Name3 + Code: 0x22 + Values: +- 0x6 # Parent = Second entry ("1::2") +- 0x16 # Die offset to entry named "1::2::3" +- Name: 0x4 # strp to Name3 + Code: 0x11 + Values: +- 0x16 # Die offset to entry named "3" +)"; + + YAMLModuleTester t(yamldata); + auto *symbol_file = + llvm::cast(t.GetModule()->GetSymbolFile()); + auto *index = static_cast(symbol_file->getIndex()); + ASSERT_NE(index, nullptr); + + auto make_entry = [](const char *c) { +return DWARFDeclContext::Entry(dwarf::DW_TAG_class_type, c); + }; + check_num_matches(*index, 1, {make_entry("1")}); + check_num_matches(*index, 1, {make_entry("2"), make_entry("1")}); + check_num_matches(*index, 1, +{make_entry("3"), make_entry("2"), make_entry("1")}); + check_num_matches(*index, 0, {make_entry("2")}); + check_num_matches(*index, 1, {make_entry("3")}); +} + +TEST(DWARFDebugNamesIndexTest, FullyQualifiedQueryWithoutIDXParent) { + const char *yamldata = R"( +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data:ELFDATA2LSB + Type:ET_EXEC + Machine: EM_386 +DWARF: + debug_str: +- '1' +- '2' + debug_abbrev: +- Table: +- Code:0x1 + Tag: DW_TAG_compile_unit + Children:DW_CHILDREN_yes +- Code:0x2 + Tag: DW_TAG_class_type + Children:DW_CHILDREN_yes + Attributes: +- Attribute: DW_AT_name + Form:DW_FORM_strp +- Code:0x3 + Tag: DW_TAG_class_type + Children:DW_CHILDREN_
[Lldb-commits] [lldb] [lldb][DWARFIndex] Use IDX_parent to implement GetFullyQualifiedType query (PR #79932)
@@ -218,6 +219,104 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass( m_fallback.GetCompleteObjCClass(class_name, must_be_implementation, callback); } +namespace { +using Entry = llvm::DWARFDebugNames::Entry; + +/// If `entry` and all of its parents have an `IDX_parent`, use that information +/// to build and return a list of at most `max_parents` parent Entries. +/// `entry` itself is not included in the list. +/// If any parent does not have an `IDX_parent`, or the Entry data is corrupted, +/// nullopt is returned. +static std::optional> +getParentChain(Entry entry, uint32_t max_parents) { + llvm::SmallVector parent_entries; + + do { +if (!entry.hasParentInformation()) + return std::nullopt; + +llvm::Expected> parent = entry.getParentDIEEntry(); +if (!parent) { // Bad data. + consumeError(parent.takeError()); + return std::nullopt; +} + +// Last parent in the chain +if (!parent->has_value()) + break; + +parent_entries.push_back(**parent); +entry = **parent; + } while (parent_entries.size() < max_parents); + + return parent_entries; +} +} // namespace + +void DebugNamesDWARFIndex::GetFullyQualifiedType( +const DWARFDeclContext &context, +llvm::function_ref callback) { + if (context.GetSize() == 0) +return; + + // Fallback: use the base class implementation. + auto fallback_impl = [&](const DebugNames::Entry &entry) { +return ProcessEntry(entry, [&](DWARFDIE die) { + return GetFullyQualifiedTypeImpl(context, die, callback); +}); + }; dwblaikie wrote: Not sure this is worth putting in a lambda? Might be simpler to write it inline in the one place it's used? https://github.com/llvm/llvm-project/pull/79932 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits