Re: [Lldb-commits] [PATCH] D49309: No longer pass a StringRef to the Python API

2018-07-16 Thread David Blaikie via lldb-commits
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

2018-08-28 Thread David Blaikie via lldb-commits
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

2018-01-29 Thread David Blaikie via lldb-commits
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

2017-07-21 Thread David Blaikie via lldb-commits
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

2017-07-22 Thread David Blaikie via lldb-commits
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

2017-07-23 Thread David Blaikie via lldb-commits
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.

2019-01-13 Thread David Blaikie via lldb-commits
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

2017-01-05 Thread David Blaikie via lldb-commits
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

2017-01-05 Thread David Blaikie via lldb-commits
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

2017-01-05 Thread David Blaikie via lldb-commits
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

2017-01-05 Thread David Blaikie via lldb-commits
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

2017-01-05 Thread David Blaikie via lldb-commits
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"

2017-01-06 Thread David Blaikie via lldb-commits
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"

2017-01-06 Thread David Blaikie via lldb-commits
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

2015-11-30 Thread David Blaikie via lldb-commits
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

2015-11-30 Thread David Blaikie via lldb-commits
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

2016-09-09 Thread David Blaikie via lldb-commits
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

2015-09-23 Thread David Blaikie via lldb-commits
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)

2024-06-26 Thread David Blaikie via lldb-commits


@@ -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)

2024-06-26 Thread David Blaikie via lldb-commits

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)

2024-06-26 Thread David Blaikie via lldb-commits

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)

2024-06-26 Thread David Blaikie via lldb-commits

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)

2024-06-26 Thread David Blaikie via lldb-commits

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)

2024-06-26 Thread David Blaikie via lldb-commits

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)

2024-06-26 Thread David Blaikie via lldb-commits

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)

2024-06-28 Thread David Blaikie via lldb-commits


@@ -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)

2024-07-03 Thread David Blaikie via lldb-commits

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)

2024-05-13 Thread David Blaikie via lldb-commits

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)

2024-05-13 Thread David Blaikie via lldb-commits

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)

2024-05-23 Thread David Blaikie via lldb-commits

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)

2024-05-23 Thread David Blaikie via lldb-commits

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)

2024-05-23 Thread David Blaikie via lldb-commits

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)

2024-05-23 Thread David Blaikie via lldb-commits

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)

2024-05-30 Thread David Blaikie via lldb-commits

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)

2024-05-30 Thread David Blaikie via lldb-commits

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)

2024-06-04 Thread David Blaikie via lldb-commits

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)

2024-06-06 Thread David Blaikie via lldb-commits


@@ -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)

2024-06-13 Thread David Blaikie via lldb-commits

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)

2024-06-17 Thread David Blaikie via lldb-commits

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)

2024-06-17 Thread David Blaikie via lldb-commits

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)

2024-06-18 Thread David Blaikie via lldb-commits

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)

2024-06-20 Thread David Blaikie via lldb-commits


@@ -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)

2024-06-22 Thread David Blaikie via lldb-commits

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)

2024-06-24 Thread David Blaikie via lldb-commits

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)

2024-06-24 Thread David Blaikie via lldb-commits

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)

2024-08-11 Thread David Blaikie via lldb-commits

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)

2023-12-06 Thread David Blaikie via lldb-commits

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

2023-12-11 Thread David Blaikie via lldb-commits

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"

2023-12-14 Thread David Blaikie via lldb-commits

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)

2023-12-21 Thread David Blaikie via lldb-commits

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)

2024-01-02 Thread David Blaikie via lldb-commits

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)

2024-01-05 Thread David Blaikie via lldb-commits

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)

2024-01-05 Thread David Blaikie via lldb-commits


@@ -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)

2024-01-05 Thread David Blaikie via lldb-commits


@@ -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

2023-07-31 Thread David Blaikie via lldb-commits

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)

2023-09-20 Thread David Blaikie via lldb-commits


@@ -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)

2023-09-20 Thread David Blaikie via lldb-commits

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)

2023-09-20 Thread David Blaikie via lldb-commits

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)

2023-09-25 Thread David Blaikie via lldb-commits


@@ -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)

2023-09-26 Thread David Blaikie via lldb-commits


@@ -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)

2024-02-23 Thread David Blaikie via lldb-commits

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)

2024-03-04 Thread David Blaikie via lldb-commits

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)

2024-03-04 Thread David Blaikie via lldb-commits

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)

2024-03-05 Thread David Blaikie via lldb-commits

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)

2024-03-05 Thread David Blaikie via lldb-commits

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)

2024-03-19 Thread David Blaikie via lldb-commits

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)

2024-03-25 Thread David Blaikie via lldb-commits

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

2024-04-01 Thread David Blaikie via lldb-commits

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)

2024-04-12 Thread David Blaikie via lldb-commits


@@ -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)

2024-04-12 Thread David Blaikie via lldb-commits


@@ -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)

2024-04-12 Thread David Blaikie via lldb-commits


@@ -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)

2024-04-12 Thread David Blaikie via lldb-commits


@@ -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)

2024-04-12 Thread David Blaikie via lldb-commits


@@ -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)

2024-04-12 Thread David Blaikie via lldb-commits


@@ -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)

2024-04-12 Thread David Blaikie via lldb-commits


@@ -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)

2024-04-14 Thread David Blaikie via lldb-commits


@@ -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)

2024-04-22 Thread David Blaikie via lldb-commits

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)

2024-04-24 Thread David Blaikie via lldb-commits

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)

2024-05-01 Thread David Blaikie via lldb-commits

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)

2024-05-01 Thread David Blaikie via lldb-commits

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

2024-05-05 Thread David Blaikie via lldb-commits

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)

2024-05-07 Thread David Blaikie via lldb-commits


@@ -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)

2024-05-07 Thread David Blaikie via lldb-commits


@@ -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)

2024-05-07 Thread David Blaikie via lldb-commits


@@ -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)

2024-05-07 Thread David Blaikie via lldb-commits


@@ -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)

2024-05-07 Thread David Blaikie via lldb-commits


@@ -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)

2024-05-07 Thread David Blaikie via lldb-commits


@@ -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

2021-12-13 Thread David Blaikie via lldb-commits
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

2022-01-23 Thread David Blaikie via lldb-commits

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

2022-07-07 Thread David Blaikie via lldb-commits

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

2022-07-07 Thread David Blaikie via lldb-commits

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

2022-07-07 Thread David Blaikie via lldb-commits

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)

2022-07-07 Thread David Blaikie via lldb-commits

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

2022-07-07 Thread David Blaikie via lldb-commits

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

2022-08-16 Thread David Blaikie via lldb-commits

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)

2024-01-24 Thread David Blaikie via lldb-commits


@@ -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)

2024-01-26 Thread David Blaikie via lldb-commits

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)

2024-01-30 Thread David Blaikie via lldb-commits

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)

2024-01-30 Thread David Blaikie via lldb-commits


@@ -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)

2024-01-30 Thread David Blaikie via lldb-commits


@@ -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


  1   2   >