[Lldb-commits] [PATCH] D46551: Inject only relevant local variables in the expression evaluation context

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

One could probably concoct an example (using macros, token pasting and such) 
where this would get it wrong, but that's probably not worth supporting.




Comment at: source/Expression/ExpressionSourceCode.cpp:193
+var_name == ConstString(".block_descriptor") ||
+!ExprBodyContainsVar(var_name.AsCString(), expr))
   continue;

`s/AsCString/GetStringRef` (saves a `strlen` operation).


https://reviews.llvm.org/D46551



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [RFC] Type lookup for template types is broken...

[Pavel Labath via lldb-commits]

I am still building a picture for myself of how the accelerator tables and
our name lookup works, but from what I managed to learn so far, adding an
accelerator for "C" seems like a useful thing to do. However, this does go
beyond what the DWARF 5 spec says we should do (we are only required to add
the DW_AT_name string). We are still free to add any extra entries we like,
but if we're going to be relying on this, we should try to get some of this
into the next version of the spec.


On Mon, 7 May 2018 at 22:19, Frédéric Riss via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> (...At least when using accelerator tables)

> If you apply the following patch, TestClassTemplateParameterPack.py will
start failing:
> diff --git
a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
> index 90e63b40f..304872a15 100644
> ---
a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
> +++
b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
> @@ -37,7 +37,7 @@ template <> struct D : D {



>   int main (int argc, char const *argv[])
>   {
> -C myC;
> +C myC; //% self.runCmd("settings set
target.experimental.inject-local-vars false")
>   C myLesserC;
>   myC.member = 64;
>   (void)C().isSixteenThirtyTwo();

> The test does things like invoke methods on temporary template objects:
> //% self.expect("expression -- C().isSixteenThirtyTwo()",
DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])

> The above expression currently works because there’s a local of type
C. With injected locals, the type is made readily available to
Clang. No type lookup is required for this to work in this setup.

> If you stop injecting locals, the test fails. We don’t provide the
information to Clang to understand what C is. The reason is that when Clang
parses “C”, it is going to ask about “C”, not the fully templated
name. Our accelerator tables contain references to the full names, but not
to C alone and we never find it. If I change Clang and dsymutil to add an
accelerator for “C” each time an instance of C is seen then it nearly
works. I just need this additional lldb patch:
> diff --git a/source/Symbol/TypeMap.cpp b/source/Symbol/TypeMap.cpp
> index 2838039ad..d2f2026bf 100644
> --- a/source/Symbol/TypeMap.cpp
> +++ b/source/Symbol/TypeMap.cpp
> @@ -227,8 +227,11 @@ void TypeMap::RemoveMismatchedTypes(const
std::string &type_scope,
> } else {
>   // The type we are currently looking at doesn't exists in a
namespace
>   // or class, so it only matches if there is no type scope...
> -keep_match =
> -type_scope.empty() && type_basename.compare(match_type_name)
== 0;
> +if (type_scope.empty()) {
> +  keep_match = type_basename.compare(match_type_name) == 0 ||
> +(strlen(match_type_name) > type_basename.size() &&
> + match_type_name[type_basename.size()] == '<');
> +}
> }
>   }

> I didn’t post this as a Phabricator review as it requires changes in llvm
before doing anything in LLDB and I wanted to make sure we agree this is
the right thing to do. I’m also not sure if this works out of the box on
platforms without accelerator tables.

It won't work "out of the box", but it should be fairly simple to change
our indexing code to add the extra entries, so that a lookup for "C" works
the same way in both cases. BTW, how were you planning to compute the
untemplated string ("C"). Will you just strip everything after the first
'<' character, or were you thinking of something more fancy?

pl
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46529: Add support to object files for accessing the .debug_types section

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Thanks. Please also update the `lit/Modules/elf-section-types.yaml` test to 
include the new section type.


https://reviews.llvm.org/D46529



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.h:35-58
+  //--
+  /// Slide the data in the buffer so that access to the data will
+  /// start at offset \a offset.
+  ///
+  /// This is currently used to provide access to the .debug_types
+  /// section and pretend it starts at at offset of the size of the
+  /// .debug_info section. This allows a minimally invasive change to

This looks like a thing we could focus on next. Sliding the contents of the 
data buffer seems like a useful fiction, but the way it is implemented now, it 
relies on undefined behavior (out-of-range pointers, possibly even wrapping the 
address space) and it violates the invariants that we have some far been 
assuming about data extractors (namely, that for a DataExtractor, all offsets 
from 0 to GetByteSize()-1 are dereferencable). For example, for a "slid" data 
extractor:
- ValidOffset(0) will return true, even though that is clearly not the case
- Append(data2) will try to access data at *m_start and probably crash
- so will Checksum()

Since there seems to be a consensus (at least, nobody has proposed an 
alternative solution) that this is the way we should treat type units, I think 
it makes sense to spend some time to make sure we built it on solid 
foundations. To me, this sliding seems like a fundamental change in the nature 
of the data extractors, and so I think it should be done in the base 
DataExtractor class. So I'd propose to move this into a separate patch, where 
we can make sure that all other DataExtractor APIs do something sensible even 
if the extractor has been "slid".


https://reviews.llvm.org/D32167



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46576: [DWARF] Align non-accelerated function fullname searching with the apple-tables path

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, JDevlieghere.

Before this patch the two paths were doing very different things

- the apple path searched the .apple_names section, which contained mangled 
names, as well as basenames of all functions. It returned any name it found.
- the non-accelerated path looked in the "full name" index we built ourselves, 
which contained mangled as well as demangled names of all functions (but no 
basenames). Then however, if it did not find a match it did an extra search in 
the basename index, with some special handling for anonymous namespaces.

This aligns the two paths by changing the non-accelerated path to return
the same results as in the apple-tables one. In pratice, this means we
will search in both the "basename" and "method" indexes (in the manual
indexes we keep methods separately). This means the function will return
some slightly inappropriate results (e.g. bar::baz::foo when one asks
for a "full name" foo), but this can be handled by additional filtering,
independently indexing method. To make this work, I've also started
inserting mangled names into the manual "basename" index.

I've also stopped matching demangled names when a regex search is
requested as that is not what apple tables do.

After this change the manual "fullname" is unused and can be removed.


https://reviews.llvm.org/D46576

Files:
  lit/SymbolFile/DWARF/find-basic-function.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2622,44 +2622,25 @@
 if (!m_indexed)
   Index();
 
+DIEArray die_offsets;
 if (name_type_mask & eFunctionNameTypeFull) {
-  FindFunctions(name, m_function_fullname_index, include_inlines, sc_list);
-
-  // FIXME Temporary workaround for global/anonymous namespace
-  // functions debugging FreeBSD and Linux binaries. If we didn't find any
-  // functions in the global namespace try looking in the basename index
-  // but ignore any returned functions that have a namespace but keep
-  // functions which have an anonymous namespace
-  // TODO: The arch in the object file isn't correct for MSVC
-  // binaries on windows, we should find a way to make it correct and
-  // handle those symbols as well.
-  if (sc_list.GetSize() == original_size) {
-ArchSpec arch;
-if (!parent_decl_ctx && GetObjectFile()->GetArchitecture(arch) &&
-arch.GetTriple().isOSBinFormatELF()) {
-  SymbolContextList temp_sc_list;
-  FindFunctions(name, m_function_basename_index, include_inlines,
-temp_sc_list);
-  SymbolContext sc;
-  for (uint32_t i = 0; i < temp_sc_list.GetSize(); i++) {
-if (temp_sc_list.GetContextAtIndex(i, sc)) {
-  ConstString mangled_name =
-  sc.GetFunctionName(Mangled::ePreferMangled);
-  ConstString demangled_name =
-  sc.GetFunctionName(Mangled::ePreferDemangled);
-  // Mangled names on Linux and FreeBSD are of the form:
-  // _ZN18function_namespace13function_nameEv.
-  if (strncmp(mangled_name.GetCString(), "_ZN", 3) ||
-  !strncmp(demangled_name.GetCString(), "(anonymous namespace)",
-   21)) {
-sc_list.Append(sc);
-  }
-}
+  uint32_t num_matches = m_function_basename_index.Find(name, die_offsets);
+  num_matches += m_function_method_index.Find(name, die_offsets);
+  for (uint32_t i = 0; i < num_matches; i++) {
+const DIERef &die_ref = die_offsets[i];
+DWARFDIE die = info->GetDIE(die_ref);
+if (die) {
+  if (!DIEInDeclContext(parent_decl_ctx, die))
+continue; // The containing decl contexts don't match
+
+  if (resolved_dies.find(die.GetDIE()) == resolved_dies.end()) {
+if (ResolveFunction(die, include_inlines, sc_list))
+  resolved_dies.insert(die.GetDIE());
   }
 }
   }
+  die_offsets.clear();
 }
-DIEArray die_offsets;
 if (name_type_mask & eFunctionNameTypeBase) {
   uint32_t num_base = m_function_basename_index.Find(name, die_offsets);
   for (uint32_t i = 0; i < num_base; i++) {
@@ -2751,8 +2732,6 @@
   Index();
 
 FindFunctions(regex, m_function_basename_index, include_inlines, sc_list);
-
-FindFunctions(regex, m_function_fullname_index, include_inlines, sc_list);
   }
 
   // Return the number of variable that were appended to the list
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWA

[Lldb-commits] [lldb] r331764 - [test] Re-enable TestUnicodeSymbols

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue May  8 06:28:34 2018
New Revision: 331764

URL: http://llvm.org/viewvc/llvm-project?rev=331764&view=rev
Log:
[test] Re-enable TestUnicodeSymbols

Re-enable TestUnicodeSymbols now that we use the in-tree dsymutil. This
was disabled because the hashing of unicode symbols was out of sync
between llvm (dsymutil) and lldb.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py?rev=331764&r1=331763&r2=331764&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py 
Tue May  8 06:28:34 2018
@@ -9,7 +9,6 @@ class TestUnicodeSymbols(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
-@skipIf(debug_info=["dsym"])
 def test_union_members(self):
 self.build()
 spec = lldb.SBModuleSpec()


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46580: Modernize and clean-up the Predicate class

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: jingham, clayborg.
Herald added a subscriber: mgorny.

The comments on this class were out of date with the implementation, and
the implementation itself was inconsistent with our usage of the Timeout
class (I started converting everything to use this class back in 
https://reviews.llvm.org/D27136,
but I missed this one). I avoid duplicating the waiting logic by
introducing a templated WaitFor function, and make other functions
delegate to that. This function can be also used as a replacement for
the unused WaitForBitToBeSet functions I removed, if it turns out to be
necessary.

As this changes the meaning of a "zero" timeout, I tracked down all the
callers of these functions and updated them accordingly. Propagating the
changes to all the callers of RunShellCommand was a bit too much for
this patch, so I stopped there and will continue that in a follow-up
patch.

I also add some basic unittests for the functions I modified.


https://reviews.llvm.org/D46580

Files:
  include/lldb/Core/Event.h
  include/lldb/Host/Predicate.h
  include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
  include/lldb/Target/Process.h
  source/Commands/CommandObjectProcess.cpp
  source/Commands/CommandObjectThread.cpp
  source/Host/common/Host.cpp
  source/Host/posix/ConnectionFileDescriptorPosix.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  source/Target/Process.cpp
  unittests/Host/CMakeLists.txt
  unittests/Host/PredicateTest.cpp

Index: unittests/Host/PredicateTest.cpp
===
--- /dev/null
+++ unittests/Host/PredicateTest.cpp
@@ -0,0 +1,34 @@
+//===-- PredicateTest.cpp ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "lldb/Host/Predicate.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace lldb_private;
+
+TEST(Predicate, WaitForValueEqualTo) {
+  Predicate P(0);
+  EXPECT_TRUE(P.WaitForValueEqualTo(0));
+  EXPECT_FALSE(P.WaitForValueEqualTo(1, std::chrono::milliseconds(10)));
+
+  std::thread Setter([&P] {
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
+P.SetValue(1, eBroadcastAlways);
+  });
+  EXPECT_TRUE(P.WaitForValueEqualTo(1));
+  Setter.join();
+}
+
+TEST(Predicate, WaitForValueNotEqualTo) {
+  Predicate P(0);
+  EXPECT_EQ(0, P.WaitForValueNotEqualTo(1));
+  EXPECT_EQ(llvm::None,
+P.WaitForValueNotEqualTo(0, std::chrono::milliseconds(10)));
+}
Index: unittests/Host/CMakeLists.txt
===
--- unittests/Host/CMakeLists.txt
+++ unittests/Host/CMakeLists.txt
@@ -3,6 +3,7 @@
   HostInfoTest.cpp
   HostTest.cpp
   MainLoopTest.cpp
+  PredicateTest.cpp
   SocketAddressTest.cpp
   SocketTest.cpp
   SymbolsTest.cpp
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -947,21 +947,25 @@
   return state;
 }
 
-void Process::SyncIOHandler(uint32_t iohandler_id, uint64_t timeout_msec) {
+void Process::SyncIOHandler(uint32_t iohandler_id,
+const Timeout &timeout) {
   // don't sync (potentially context switch) in case where there is no process
   // IO
   if (!m_process_input_reader)
 return;
 
-  uint32_t new_iohandler_id = 0;
-  m_iohandler_sync.WaitForValueNotEqualTo(
-  iohandler_id, new_iohandler_id, std::chrono::milliseconds(timeout_msec));
+  auto Result = m_iohandler_sync.WaitForValueNotEqualTo(iohandler_id, timeout);
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
-  if (log)
-log->Printf("Process::%s waited for m_iohandler_sync to change from %u, "
-"new value is %u",
-__FUNCTION__, iohandler_id, new_iohandler_id);
+  if (Result) {
+LLDB_LOG(
+log,
+"waited from m_iohandler_sync to change from {0}. New value is {1}.",
+iohandler_id, *Result);
+  } else {
+LLDB_LOG(log, "timed out waiting for m_iohandler_sync to change from {0}.",
+ iohandler_id);
+  }
 }
 
 StateType Process::WaitForProcessToStop(const Timeout &timeout,
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -1133,7 +1133,7 @@
 ConnectionFileDescriptor *connection =
 (ConnectionFileDescriptor *)GetConnection();
 // Wait for 10 seconds to resolve the bound port
-uint16_t port_ = connection->GetListeningPort(10);
+uint16_t port_ = connecti

Re: [Lldb-commits] [RFC] Type lookup for template types is broken...

[Frédéric Riss via lldb-commits]

> On May 8, 2018, at 2:23 AM, Pavel Labath  wrote:
> 
> I am still building a picture for myself of how the accelerator tables and
> our name lookup works, but from what I managed to learn so far, adding an
> accelerator for "C" seems like a useful thing to do. However, this does go
> beyond what the DWARF 5 spec says we should do (we are only required to add
> the DW_AT_name string). We are still free to add any extra entries we like,
> but if we're going to be relying on this, we should try to get some of this
> into the next version of the spec.
> 
> 
> On Mon, 7 May 2018 at 22:19, Frédéric Riss via lldb-commits <
> lldb-commits@lists.llvm.org > wrote:
> 
>> (...At least when using accelerator tables)
> 
>> If you apply the following patch, TestClassTemplateParameterPack.py will
> start failing:
>> diff --git
> a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
> b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
>> index 90e63b40f..304872a15 100644
>> ---
> a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
>> +++
> b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp
>> @@ -37,7 +37,7 @@ template <> struct D : D {
> 
> 
> 
>>  int main (int argc, char const *argv[])
>>  {
>> -C myC;
>> +C myC; //% self.runCmd("settings set
> target.experimental.inject-local-vars false")
>>  C myLesserC;
>>  myC.member = 64;
>>  (void)C().isSixteenThirtyTwo();
> 
>> The test does things like invoke methods on temporary template objects:
>> //% self.expect("expression -- C().isSixteenThirtyTwo()",
> DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
> 
>> The above expression currently works because there’s a local of type
> C. With injected locals, the type is made readily available to
> Clang. No type lookup is required for this to work in this setup.
> 
>> If you stop injecting locals, the test fails. We don’t provide the
> information to Clang to understand what C is. The reason is that when Clang
> parses “C”, it is going to ask about “C”, not the fully templated
> name. Our accelerator tables contain references to the full names, but not
> to C alone and we never find it. If I change Clang and dsymutil to add an
> accelerator for “C” each time an instance of C is seen then it nearly
> works. I just need this additional lldb patch:
>> diff --git a/source/Symbol/TypeMap.cpp b/source/Symbol/TypeMap.cpp
>> index 2838039ad..d2f2026bf 100644
>> --- a/source/Symbol/TypeMap.cpp
>> +++ b/source/Symbol/TypeMap.cpp
>> @@ -227,8 +227,11 @@ void TypeMap::RemoveMismatchedTypes(const
> std::string &type_scope,
>>} else {
>>  // The type we are currently looking at doesn't exists in a
> namespace
>>  // or class, so it only matches if there is no type scope...
>> -keep_match =
>> -type_scope.empty() && type_basename.compare(match_type_name)
> == 0;
>> +if (type_scope.empty()) {
>> +  keep_match = type_basename.compare(match_type_name) == 0 ||
>> +(strlen(match_type_name) > type_basename.size() &&
>> + match_type_name[type_basename.size()] == '<');
>> +}
>>}
>>  }
> 
>> I didn’t post this as a Phabricator review as it requires changes in llvm
> before doing anything in LLDB and I wanted to make sure we agree this is
> the right thing to do. I’m also not sure if this works out of the box on
> platforms without accelerator tables.
> 
> It won't work "out of the box", but it should be fairly simple to change
> our indexing code to add the extra entries, so that a lookup for "C" works
> the same way in both cases. BTW, how were you planning to compute the
> untemplated string ("C"). Will you just strip everything after the first
> '<' character, or were you thinking of something more fancy?

AFAIK, there are no fully qualified names in the debug info we generate so 
taking what’s before the first ‘<‘ should always return the class name. Does 
this logic seem flawed?

Fred


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D46551: Inject only relevant local variables in the expression evaluation context

[Frédéric Riss via lldb-commits]

> On May 8, 2018, at 1:32 AM, Pavel Labath via Phabricator 
>  wrote:
> 
> labath added a comment.
> 
> One could probably concoct an example (using macros, token pasting and such) 
> where this would get it wrong, but that's probably not worth supporting.

Yeah, I thought about this and I’m not sure it’s worth supporting. We could 
support it by doing the filtering after a round of pre-processing, but it seems 
really overkill. 

> 
> 
> 
> Comment at: source/Expression/ExpressionSourceCode.cpp:193
> +var_name == ConstString(".block_descriptor") ||
> +!ExprBodyContainsVar(var_name.AsCString(), expr))
>   continue;
> 
> `s/AsCString/GetStringRef` (saves a `strlen` operation).

Thanks,
Fred

> 
> https://reviews.llvm.org/D46551
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [RFC] Type lookup for template types is broken...

[Pavel Labath via lldb-commits]

Well.. it encodes some assumptions about how a class name looks like, which
are probably valid for C++, but they don't have to hold for any language
frontend LLVM supports. That said, I am not saying this is worth the
trouble of adding a special "these are the additional names you are to
insert into the index" channel that clang should use to communicate this (I
wouldn't be surprised if we make even stronger assumptions elsewhere). I
was just curious about what your thoughts here were.
On Tue, 8 May 2018 at 15:29, Frédéric Riss  wrote:



> On May 8, 2018, at 2:23 AM, Pavel Labath  wrote:

> I am still building a picture for myself of how the accelerator tables and
> our name lookup works, but from what I managed to learn so far, adding an
> accelerator for "C" seems like a useful thing to do. However, this does go
> beyond what the DWARF 5 spec says we should do (we are only required to
add
> the DW_AT_name string). We are still free to add any extra entries we
like,
> but if we're going to be relying on this, we should try to get some of
this
> into the next version of the spec.


> On Mon, 7 May 2018 at 22:19, Frédéric Riss via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:

> (...At least when using accelerator tables)


> If you apply the following patch, TestClassTemplateParameterPack.py will

> start failing:

> diff --git


a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp

b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp

> index 90e63b40f..304872a15 100644
> ---


a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp

> +++


b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-pack/main.cpp

> @@ -37,7 +37,7 @@ template <> struct D : D {




>   int main (int argc, char const *argv[])
>   {
> -C myC;
> +C myC; //% self.runCmd("settings set

> target.experimental.inject-local-vars false")

>   C myLesserC;
>   myC.member = 64;
>   (void)C().isSixteenThirtyTwo();


> The test does things like invoke methods on temporary template objects:
> //% self.expect("expression -- C().isSixteenThirtyTwo()",

> DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])

> The above expression currently works because there’s a local of type

> C. With injected locals, the type is made readily available to
> Clang. No type lookup is required for this to work in this setup.

> If you stop injecting locals, the test fails. We don’t provide the

> information to Clang to understand what C is. The reason is that when
Clang
> parses “C”, it is going to ask about “C”, not the fully
templated
> name. Our accelerator tables contain references to the full names, but not
> to C alone and we never find it. If I change Clang and dsymutil to add an
> accelerator for “C” each time an instance of C is seen then it nearly
> works. I just need this additional lldb patch:

> diff --git a/source/Symbol/TypeMap.cpp b/source/Symbol/TypeMap.cpp
> index 2838039ad..d2f2026bf 100644
> --- a/source/Symbol/TypeMap.cpp
> +++ b/source/Symbol/TypeMap.cpp
> @@ -227,8 +227,11 @@ void TypeMap::RemoveMismatchedTypes(const

> std::string &type_scope,

> } else {
>   // The type we are currently looking at doesn't exists in a

> namespace

>   // or class, so it only matches if there is no type scope...
> -keep_match =
> -type_scope.empty() && type_basename.compare(match_type_name)

> == 0;

> +if (type_scope.empty()) {
> +  keep_match = type_basename.compare(match_type_name) == 0 ||
> +(strlen(match_type_name) > type_basename.size() &&
> + match_type_name[type_basename.size()] == '<');
> +}
> }
>   }


> I didn’t post this as a Phabricator review as it requires changes in llvm

> before doing anything in LLDB and I wanted to make sure we agree this is
> the right thing to do. I’m also not sure if this works out of the box on
> platforms without accelerator tables.

> It won't work "out of the box", but it should be fairly simple to change
> our indexing code to add the extra entries, so that a lookup for "C" works
> the same way in both cases. BTW, how were you planning to compute the
> untemplated string ("C"). Will you just strip everything after the first
> '<' character, or were you thinking of something more fancy?


> AFAIK, there are no fully qualified names in the debug info we generate
so taking what’s before the first ‘<‘ should always return the class name.
Does this logic seem flawed?

> Fred
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [RFC] Type lookup for template types is broken...

[via lldb-commits]

> -Original Message-
> From: lldb-commits [mailto:lldb-commits-boun...@lists.llvm.org] On Behalf
> Of Pavel Labath via lldb-commits
> Sent: Tuesday, May 08, 2018 10:48 AM
> To: fr...@apple.com
> Cc: lldb-commits@lists.llvm.org
> Subject: Re: [Lldb-commits] [RFC] Type lookup for template types is
> broken...
> 
> Well.. it encodes some assumptions about how a class name looks like,
> which
> are probably valid for C++, but they don't have to hold for any language
> frontend LLVM supports. That said, I am not saying this is worth the
> trouble of adding a special "these are the additional names you are to
> insert into the index" channel that clang should use to communicate this
> (I
> wouldn't be surprised if we make even stronger assumptions elsewhere). I
> was just curious about what your thoughts here were.

If you add an accelerator entry for "C" what does it point to?  All the
instantiations of "C"?  The DWARF does not describe the template, only
the concrete instances.
--paulr


> On Tue, 8 May 2018 at 15:29, Frédéric Riss  wrote:
> 
> 
> 
> > On May 8, 2018, at 2:23 AM, Pavel Labath  wrote:
> 
> > I am still building a picture for myself of how the accelerator tables
> and
> > our name lookup works, but from what I managed to learn so far, adding
> an
> > accelerator for "C" seems like a useful thing to do. However, this does
> go
> > beyond what the DWARF 5 spec says we should do (we are only required to
> add
> > the DW_AT_name string). We are still free to add any extra entries we
> like,
> > but if we're going to be relying on this, we should try to get some of
> this
> > into the next version of the spec.
> 
> 
> > On Mon, 7 May 2018 at 22:19, Frédéric Riss via lldb-commits <
> > lldb-commits@lists.llvm.org> wrote:
> 
> > (...At least when using accelerator tables)
> 
> 
> > If you apply the following patch, TestClassTemplateParameterPack.py will
> 
> > start failing:
> 
> > diff --git
> 
> 
> a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
> pack/main.cpp
> 
> b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
> pack/main.cpp
> 
> > index 90e63b40f..304872a15 100644
> > ---
> 
> 
> a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
> pack/main.cpp
> 
> > +++
> 
> 
> b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
> pack/main.cpp
> 
> > @@ -37,7 +37,7 @@ template <> struct D : D {
> 
> 
> 
> 
> >   int main (int argc, char const *argv[])
> >   {
> > -C myC;
> > +C myC; //% self.runCmd("settings set
> 
> > target.experimental.inject-local-vars false")
> 
> >   C myLesserC;
> >   myC.member = 64;
> >   (void)C().isSixteenThirtyTwo();
> 
> 
> > The test does things like invoke methods on temporary template objects:
> > //% self.expect("expression -- C().isSixteenThirtyTwo()",
> 
> > DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
> 
> > The above expression currently works because there’s a local of type
> 
> > C. With injected locals, the type is made readily available to
> > Clang. No type lookup is required for this to work in this setup.
> 
> > If you stop injecting locals, the test fails. We don’t provide the
> 
> > information to Clang to understand what C is. The reason is that when
> Clang
> > parses “C”, it is going to ask about “C”, not the fully
> templated
> > name. Our accelerator tables contain references to the full names, but
> not
> > to C alone and we never find it. If I change Clang and dsymutil to add
> an
> > accelerator for “C” each time an instance of C is seen then it nearly
> > works. I just need this additional lldb patch:
> 
> > diff --git a/source/Symbol/TypeMap.cpp b/source/Symbol/TypeMap.cpp
> > index 2838039ad..d2f2026bf 100644
> > --- a/source/Symbol/TypeMap.cpp
> > +++ b/source/Symbol/TypeMap.cpp
> > @@ -227,8 +227,11 @@ void TypeMap::RemoveMismatchedTypes(const
> 
> > std::string &type_scope,
> 
> > } else {
> >   // The type we are currently looking at doesn't exists in a
> 
> > namespace
> 
> >   // or class, so it only matches if there is no type scope...
> > -keep_match =
> > -type_scope.empty() &&
> type_basename.compare(match_type_name)
> 
> > == 0;
> 
> > +if (type_scope.empty()) {
> > +  keep_match = type_basename.compare(match_type_name) == 0 ||
> > +(strlen(match_type_name) > type_basename.size() &&
> > + match_type_name[type_basename.size()] == '<');
> > +}
> > }
> >   }
> 
> 
> > I didn’t post this as a Phabricator review as it requires changes in
> llvm
> 
> > before doing anything in LLDB and I wanted to make sure we agree this is
> > the right thing to do. I’m also not sure if this works out of the box on
> > platforms without accelerator tables.
> 
> > It won't work "out of the box", but it should be fairly simple to change
> > our indexing code to add the extra entries, so that a lookup for "C"
> works
> > the same way in both cases. BTW, 

[Lldb-commits] [PATCH] D36977: Add 'break' into GDBRemoteClientBase::SendContinuePacketAndWaitForResponse to fix a warning

[Pavel Labath via Phabricator via lldb-commits]

labath closed this revision.
labath added a comment.
Herald added a subscriber: llvm-commits.

Looks like somebody already committed a patch like this.


Repository:
  rL LLVM

https://reviews.llvm.org/D36977



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [RFC] Type lookup for template types is broken...

[Frédéric Riss via lldb-commits]

> On May 8, 2018, at 8:30 AM, paul.robin...@sony.com wrote:
> 
> 
> 
>> -Original Message-
>> From: lldb-commits [mailto:lldb-commits-boun...@lists.llvm.org 
>> ] On Behalf
>> Of Pavel Labath via lldb-commits
>> Sent: Tuesday, May 08, 2018 10:48 AM
>> To: fr...@apple.com 
>> Cc: lldb-commits@lists.llvm.org 
>> Subject: Re: [Lldb-commits] [RFC] Type lookup for template types is
>> broken...
>> 
>> Well.. it encodes some assumptions about how a class name looks like,
>> which
>> are probably valid for C++, but they don't have to hold for any language
>> frontend LLVM supports. That said, I am not saying this is worth the
>> trouble of adding a special "these are the additional names you are to
>> insert into the index" channel that clang should use to communicate this
>> (I
>> wouldn't be surprised if we make even stronger assumptions elsewhere). I
>> was just curious about what your thoughts here were.
> 
> If you add an accelerator entry for "C" what does it point to?  All the
> instantiations of "C"?  The DWARF does not describe the template, only
> the concrete instances.

Yes, there would be a “C” entry for every instantiation of C.

Fred

> --paulr
> 
> 
>> On Tue, 8 May 2018 at 15:29, Frédéric Riss  wrote:
>> 
>> 
>> 
>>> On May 8, 2018, at 2:23 AM, Pavel Labath  wrote:
>> 
>>> I am still building a picture for myself of how the accelerator tables
>> and
>>> our name lookup works, but from what I managed to learn so far, adding
>> an
>>> accelerator for "C" seems like a useful thing to do. However, this does
>> go
>>> beyond what the DWARF 5 spec says we should do (we are only required to
>> add
>>> the DW_AT_name string). We are still free to add any extra entries we
>> like,
>>> but if we're going to be relying on this, we should try to get some of
>> this
>>> into the next version of the spec.
>> 
>> 
>>> On Mon, 7 May 2018 at 22:19, Frédéric Riss via lldb-commits <
>>> lldb-commits@lists.llvm.org> wrote:
>> 
>>> (...At least when using accelerator tables)
>> 
>> 
>>> If you apply the following patch, TestClassTemplateParameterPack.py will
>> 
>>> start failing:
>> 
>>> diff --git
>> 
>> 
>> a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
>> pack/main.cpp
>> 
>> b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
>> pack/main.cpp
>> 
>>> index 90e63b40f..304872a15 100644
>>> ---
>> 
>> 
>> a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
>> pack/main.cpp
>> 
>>> +++
>> 
>> 
>> b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
>> pack/main.cpp
>> 
>>> @@ -37,7 +37,7 @@ template <> struct D : D {
>> 
>> 
>> 
>> 
>>>  int main (int argc, char const *argv[])
>>>  {
>>> -C myC;
>>> +C myC; //% self.runCmd("settings set
>> 
>>> target.experimental.inject-local-vars false")
>> 
>>>  C myLesserC;
>>>  myC.member = 64;
>>>  (void)C().isSixteenThirtyTwo();
>> 
>> 
>>> The test does things like invoke methods on temporary template objects:
>>> //% self.expect("expression -- C().isSixteenThirtyTwo()",
>> 
>>> DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
>> 
>>> The above expression currently works because there’s a local of type
>> 
>>> C. With injected locals, the type is made readily available to
>>> Clang. No type lookup is required for this to work in this setup.
>> 
>>> If you stop injecting locals, the test fails. We don’t provide the
>> 
>>> information to Clang to understand what C is. The reason is that when
>> Clang
>>> parses “C”, it is going to ask about “C”, not the fully
>> templated
>>> name. Our accelerator tables contain references to the full names, but
>> not
>>> to C alone and we never find it. If I change Clang and dsymutil to add
>> an
>>> accelerator for “C” each time an instance of C is seen then it nearly
>>> works. I just need this additional lldb patch:
>> 
>>> diff --git a/source/Symbol/TypeMap.cpp b/source/Symbol/TypeMap.cpp
>>> index 2838039ad..d2f2026bf 100644
>>> --- a/source/Symbol/TypeMap.cpp
>>> +++ b/source/Symbol/TypeMap.cpp
>>> @@ -227,8 +227,11 @@ void TypeMap::RemoveMismatchedTypes(const
>> 
>>> std::string &type_scope,
>> 
>>>} else {
>>>  // The type we are currently looking at doesn't exists in a
>> 
>>> namespace
>> 
>>>  // or class, so it only matches if there is no type scope...
>>> -keep_match =
>>> -type_scope.empty() &&
>> type_basename.compare(match_type_name)
>> 
>>> == 0;
>> 
>>> +if (type_scope.empty()) {
>>> +  keep_match = type_basename.compare(match_type_name) == 0 ||
>>> +(strlen(match_type_name) > type_basename.size() &&
>>> + match_type_name[type_basename.size()] == '<');
>>> +}
>>>}
>>>  }
>> 
>> 
>>> I didn’t post this as a Phabricator review as it requires changes in
>> llvm
>> 
>>> before doing anything in LLDB and I wanted to make su

Re: [Lldb-commits] [RFC] Type lookup for template types is broken...

[Greg Clayton via lldb-commits]

The only way for us to find all classes whose type is "C" is to add the entry 
for all template classes named "C", so I would vote to add them as it is 
accurate. Do we currently add one for "C<12, 16>"?

Greg

> On May 8, 2018, at 8:58 AM, Frédéric Riss via lldb-commits 
>  wrote:
> 
> 
> 
>> On May 8, 2018, at 8:30 AM, paul.robin...@sony.com 
>>  wrote:
>> 
>> 
>> 
>>> -Original Message-
>>> From: lldb-commits [mailto:lldb-commits-boun...@lists.llvm.org 
>>> ] On Behalf
>>> Of Pavel Labath via lldb-commits
>>> Sent: Tuesday, May 08, 2018 10:48 AM
>>> To: fr...@apple.com 
>>> Cc: lldb-commits@lists.llvm.org 
>>> Subject: Re: [Lldb-commits] [RFC] Type lookup for template types is
>>> broken...
>>> 
>>> Well.. it encodes some assumptions about how a class name looks like,
>>> which
>>> are probably valid for C++, but they don't have to hold for any language
>>> frontend LLVM supports. That said, I am not saying this is worth the
>>> trouble of adding a special "these are the additional names you are to
>>> insert into the index" channel that clang should use to communicate this
>>> (I
>>> wouldn't be surprised if we make even stronger assumptions elsewhere). I
>>> was just curious about what your thoughts here were.
>> 
>> If you add an accelerator entry for "C" what does it point to?  All the
>> instantiations of "C"?  The DWARF does not describe the template, only
>> the concrete instances.
> 
> Yes, there would be a “C” entry for every instantiation of C.
> 
> Fred
> 
>> --paulr
>> 
>> 
>>> On Tue, 8 May 2018 at 15:29, Frédéric Riss >> > wrote:
>>> 
>>> 
>>> 
 On May 8, 2018, at 2:23 AM, Pavel Labath >>> > wrote:
>>> 
 I am still building a picture for myself of how the accelerator tables
>>> and
 our name lookup works, but from what I managed to learn so far, adding
>>> an
 accelerator for "C" seems like a useful thing to do. However, this does
>>> go
 beyond what the DWARF 5 spec says we should do (we are only required to
>>> add
 the DW_AT_name string). We are still free to add any extra entries we
>>> like,
 but if we're going to be relying on this, we should try to get some of
>>> this
 into the next version of the spec.
>>> 
>>> 
 On Mon, 7 May 2018 at 22:19, Frédéric Riss via lldb-commits <
 lldb-commits@lists.llvm.org > wrote:
>>> 
 (...At least when using accelerator tables)
>>> 
>>> 
 If you apply the following patch, TestClassTemplateParameterPack.py will
>>> 
 start failing:
>>> 
 diff --git
>>> 
>>> 
>>> a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
>>> pack/main.cpp
>>> 
>>> b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
>>> pack/main.cpp
>>> 
 index 90e63b40f..304872a15 100644
 ---
>>> 
>>> 
>>> a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
>>> pack/main.cpp
>>> 
 +++
>>> 
>>> 
>>> b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
>>> pack/main.cpp
>>> 
 @@ -37,7 +37,7 @@ template <> struct D : D {
>>> 
>>> 
>>> 
>>> 
  int main (int argc, char const *argv[])
  {
 -C myC;
 +C myC; //% self.runCmd("settings set
>>> 
 target.experimental.inject-local-vars false")
>>> 
  C myLesserC;
  myC.member = 64;
  (void)C().isSixteenThirtyTwo();
>>> 
>>> 
 The test does things like invoke methods on temporary template objects:
 //% self.expect("expression -- C().isSixteenThirtyTwo()",
>>> 
 DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
>>> 
 The above expression currently works because there’s a local of type
>>> 
 C. With injected locals, the type is made readily available to
 Clang. No type lookup is required for this to work in this setup.
>>> 
 If you stop injecting locals, the test fails. We don’t provide the
>>> 
 information to Clang to understand what C is. The reason is that when
>>> Clang
 parses “C”, it is going to ask about “C”, not the fully
>>> templated
 name. Our accelerator tables contain references to the full names, but
>>> not
 to C alone and we never find it. If I change Clang and dsymutil to add
>>> an
 accelerator for “C” each time an instance of C is seen then it nearly
 works. I just need this additional lldb patch:
>>> 
 diff --git a/source/Symbol/TypeMap.cpp b/source/Symbol/TypeMap.cpp
 index 2838039ad..d2f2026bf 100644
 --- a/source/Symbol/TypeMap.cpp
 +++ b/source/Symbol/TypeMap.cpp
 @@ -227,8 +227,11 @@ void TypeMap::RemoveMismatchedTypes(const
>>> 
 std::string &type_scope,
>>> 
} else {
  // The type we are currently looking at doesn't exists in a
>>> 
 namespace
>>> 
  // or class, so it only matches if there is no type s

Re: [Lldb-commits] [RFC] Type lookup for template types is broken...

[Frédéric Riss via lldb-commits]

> On May 8, 2018, at 9:04 AM, Greg Clayton  wrote:
> 
> The only way for us to find all classes whose type is "C" is to add the entry 
> for all template classes named "C", so I would vote to add them as it is 
> accurate. Do we currently add one for "C<12, 16>”?

Yes we do, not sure it is actually useful.

Fred

> 
> Greg
> 
>> On May 8, 2018, at 8:58 AM, Frédéric Riss via lldb-commits 
>> mailto:lldb-commits@lists.llvm.org>> wrote:
>> 
>> 
>> 
>>> On May 8, 2018, at 8:30 AM, paul.robin...@sony.com 
>>>  wrote:
>>> 
>>> 
>>> 
 -Original Message-
 From: lldb-commits [mailto:lldb-commits-boun...@lists.llvm.org 
 ] On Behalf
 Of Pavel Labath via lldb-commits
 Sent: Tuesday, May 08, 2018 10:48 AM
 To: fr...@apple.com 
 Cc: lldb-commits@lists.llvm.org 
 Subject: Re: [Lldb-commits] [RFC] Type lookup for template types is
 broken...
 
 Well.. it encodes some assumptions about how a class name looks like,
 which
 are probably valid for C++, but they don't have to hold for any language
 frontend LLVM supports. That said, I am not saying this is worth the
 trouble of adding a special "these are the additional names you are to
 insert into the index" channel that clang should use to communicate this
 (I
 wouldn't be surprised if we make even stronger assumptions elsewhere). I
 was just curious about what your thoughts here were.
>>> 
>>> If you add an accelerator entry for "C" what does it point to?  All the
>>> instantiations of "C"?  The DWARF does not describe the template, only
>>> the concrete instances.
>> 
>> Yes, there would be a “C” entry for every instantiation of C.
>> 
>> Fred
>> 
>>> --paulr
>>> 
>>> 
 On Tue, 8 May 2018 at 15:29, Frédéric Riss >>> > wrote:
 
 
 
> On May 8, 2018, at 2:23 AM, Pavel Labath  > wrote:
 
> I am still building a picture for myself of how the accelerator tables
 and
> our name lookup works, but from what I managed to learn so far, adding
 an
> accelerator for "C" seems like a useful thing to do. However, this does
 go
> beyond what the DWARF 5 spec says we should do (we are only required to
 add
> the DW_AT_name string). We are still free to add any extra entries we
 like,
> but if we're going to be relying on this, we should try to get some of
 this
> into the next version of the spec.
 
 
> On Mon, 7 May 2018 at 22:19, Frédéric Riss via lldb-commits <
> lldb-commits@lists.llvm.org > wrote:
 
> (...At least when using accelerator tables)
 
 
> If you apply the following patch, TestClassTemplateParameterPack.py will
 
> start failing:
 
> diff --git
 
 
 a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
 pack/main.cpp
 
 b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
 pack/main.cpp
 
> index 90e63b40f..304872a15 100644
> ---
 
 
 a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
 pack/main.cpp
 
> +++
 
 
 b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
 pack/main.cpp
 
> @@ -37,7 +37,7 @@ template <> struct D : D {
 
 
 
 
>  int main (int argc, char const *argv[])
>  {
> -C myC;
> +C myC; //% self.runCmd("settings set
 
> target.experimental.inject-local-vars false")
 
>  C myLesserC;
>  myC.member = 64;
>  (void)C().isSixteenThirtyTwo();
 
 
> The test does things like invoke methods on temporary template objects:
> //% self.expect("expression -- C().isSixteenThirtyTwo()",
 
> DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])
 
> The above expression currently works because there’s a local of type
 
> C. With injected locals, the type is made readily available to
> Clang. No type lookup is required for this to work in this setup.
 
> If you stop injecting locals, the test fails. We don’t provide the
 
> information to Clang to understand what C is. The reason is that when
 Clang
> parses “C”, it is going to ask about “C”, not the fully
 templated
> name. Our accelerator tables contain references to the full names, but
 not
> to C alone and we never find it. If I change Clang and dsymutil to add
 an
> accelerator for “C” each time an instance of C is seen then it nearly
> works. I just need this additional lldb patch:
 
> diff --git a/source/Symbol/TypeMap.cpp b/source/Symbol/TypeMap.cpp
> index 2838039ad..d2f2026bf 100644
> --- a/source/Symbol/TypeMap.cpp
> +++ b/source/Symbol/TypeMap.cpp

Re: [Lldb-commits] [RFC] Type lookup for template types is broken...

[via lldb-commits]

So….  if the name in the type entry did not include the  then the 
indexing would automatically do what you want; you would need to reconstruct 
the  from the template parameter children of the DIE.  Would that help?
On the LLVM side we have debated the merits of  in the name back and 
forth.  I would have to investigate the current state of the world; I know that 
for PS4 we prefer to have the children and not put  in the name, while 
others prefer it the other way around.  If LLDB came out preferring not to have 
a decorated name, it should not be hard to accommodate that in Clang.
Not sure where gcc/gdb stand on this point.
--paulr

From: fr...@apple.com [mailto:fr...@apple.com]
Sent: Tuesday, May 08, 2018 12:07 PM
To: Greg Clayton
Cc: Robinson, Paul; lldb-commits@lists.llvm.org
Subject: Re: [Lldb-commits] [RFC] Type lookup for template types is broken...




On May 8, 2018, at 9:04 AM, Greg Clayton 
mailto:clayb...@gmail.com>> wrote:

The only way for us to find all classes whose type is "C" is to add the entry 
for all template classes named "C", so I would vote to add them as it is 
accurate. Do we currently add one for "C<12, 16>”?

Yes we do, not sure it is actually useful.

Fred



Greg


On May 8, 2018, at 8:58 AM, Frédéric Riss via lldb-commits 
mailto:lldb-commits@lists.llvm.org>> wrote:




On May 8, 2018, at 8:30 AM, 
paul.robin...@sony.com wrote:




-Original Message-
From: lldb-commits [mailto:lldb-commits-boun...@lists.llvm.org] On Behalf
Of Pavel Labath via lldb-commits
Sent: Tuesday, May 08, 2018 10:48 AM
To: fr...@apple.com
Cc: lldb-commits@lists.llvm.org
Subject: Re: [Lldb-commits] [RFC] Type lookup for template types is
broken...

Well.. it encodes some assumptions about how a class name looks like,
which
are probably valid for C++, but they don't have to hold for any language
frontend LLVM supports. That said, I am not saying this is worth the
trouble of adding a special "these are the additional names you are to
insert into the index" channel that clang should use to communicate this
(I
wouldn't be surprised if we make even stronger assumptions elsewhere). I
was just curious about what your thoughts here were.

If you add an accelerator entry for "C" what does it point to?  All the
instantiations of "C"?  The DWARF does not describe the template, only
the concrete instances.

Yes, there would be a “C” entry for every instantiation of C.

Fred


--paulr



On Tue, 8 May 2018 at 15:29, Frédéric Riss 
mailto:fr...@apple.com>> wrote:




On May 8, 2018, at 2:23 AM, Pavel Labath 
mailto:lab...@google.com>> wrote:


I am still building a picture for myself of how the accelerator tables
and

our name lookup works, but from what I managed to learn so far, adding
an

accelerator for "C" seems like a useful thing to do. However, this does
go

beyond what the DWARF 5 spec says we should do (we are only required to
add

the DW_AT_name string). We are still free to add any extra entries we
like,

but if we're going to be relying on this, we should try to get some of
this

into the next version of the spec.



On Mon, 7 May 2018 at 22:19, Frédéric Riss via lldb-commits <
lldb-commits@lists.llvm.org> wrote:


(...At least when using accelerator tables)



If you apply the following patch, TestClassTemplateParameterPack.py will


start failing:


diff --git


a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
pack/main.cpp

b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
pack/main.cpp


index 90e63b40f..304872a15 100644
---


a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
pack/main.cpp


+++


b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
pack/main.cpp


@@ -37,7 +37,7 @@ template <> struct D : D {





 int main (int argc, char const *argv[])
 {
-C myC;
+C myC; //% self.runCmd("settings set


target.experimental.inject-local-vars false")


 C myLesserC;
 myC.member = 64;
 (void)C().isSixteenThirtyTwo();



The test does things like invoke methods on temporary template objects:
//% self.expect("expression -- C().isSixteenThirtyTwo()",


DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["false"])


The above expression currently works because there’s a local of type


C. With injected locals, the type is made readily available to
Clang. No type lookup is required for this to work in this setup.


If you stop injecting locals, the test fails. We don’t provide the


information to Clang to understand what C is. The reason is that when
Clang

parses “C”, it is going to ask about “C”, not the fully
templated

name. Our accelerator tables contain references to the full names, but
not

to C alone and we never find it. If I change Clang and dsymutil to add
an

accelerator for “C” each time an instance of C is seen then it nearly
works. I just need this additional lldb patch:


diff

[Lldb-commits] [PATCH] D46576: [DWARF] Align non-accelerated function fullname searching with the apple-tables path

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

See inlined comments




Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:901-902
 Mangled mangled(ConstString(mangled_cstr), true);
+func_basenames.Insert(mangled.GetMangledName(),
+  DIERef(cu_offset, die.GetOffset()));
 func_fullnames.Insert(mangled.GetMangledName(),

Why are we adding the mangled name to the basenames?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:905-908
 ConstString demangled = mangled.GetDemangledName(cu_language);
 if (demangled)
   func_fullnames.Insert(demangled,
 DIERef(cu_offset, die.GetOffset()));

Should we remove these 4 lines now? I thought we weren't going to add demangled 
names to the index?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:928-929
 Mangled mangled(ConstString(mangled_cstr), true);
+func_basenames.Insert(mangled.GetMangledName(),
+  DIERef(cu_offset, die.GetOffset()));
 func_fullnames.Insert(mangled.GetMangledName(),

Why are we adding the mangled name to the basenames?


https://reviews.llvm.org/D46576



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [RFC] Type lookup for template types is broken...

[Pavel Labath via lldb-commits]

On Tue, 8 May 2018 at 17:12, via lldb-commits 
wrote:

> So….  if the name in the type entry did not include the  then the
indexing would automatically do what you want; you would need to
reconstruct the  from the template parameter children of the DIE.
Would that help?

> On the LLVM side we have debated the merits of  in the name back
and forth.  I would have to investigate the current state of the world; I
know that for PS4 we prefer to have the children and not put  in
the name, while others prefer it the other way around.  If LLDB came out
preferring not to have a decorated name, it should not be hard to
accommodate that in Clang.


I like this idea.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46580: Modernize and clean-up the Predicate class

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Looks great!


https://reviews.llvm.org/D46580



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [RFC] Type lookup for template types is broken...

[Greg Clayton via lldb-commits]

I think for display purposes, the type name should include all of the . 
If I have two variables, both using class C, but both have different template 
parameters, I want to see that in the class name and have that properly show up 
in the variable view. LLDB doesn't actually care about the name that is in the 
DWARF because we will show the type name by grabbing it from the 
lldb_private::CompilerType which will end up being correct. Other debuggers 
might just display "C" if that is what is in the DWARF. So I would caution 
against just putting the type basename without params in the DWARF itself.

Greg


> On May 8, 2018, at 9:12 AM,   
> wrote:
> 
> So….  if the name in the type entry did not include the  then the 
> indexing would automatically do what you want; you would need to reconstruct 
> the  from the template parameter children of the DIE.  Would that 
> help?
> On the LLVM side we have debated the merits of  in the name back and 
> forth.  I would have to investigate the current state of the world; I know 
> that for PS4 we prefer to have the children and not put  in the name, 
> while others prefer it the other way around.  If LLDB came out preferring not 
> to have a decorated name, it should not be hard to accommodate that in Clang.
> Not sure where gcc/gdb stand on this point.
> --paulr
>   <>
> From: fr...@apple.com [mailto:fr...@apple.com] 
> Sent: Tuesday, May 08, 2018 12:07 PM
> To: Greg Clayton
> Cc: Robinson, Paul; lldb-commits@lists.llvm.org
> Subject: Re: [Lldb-commits] [RFC] Type lookup for template types is broken...
>  
>  
> 
> 
> On May 8, 2018, at 9:04 AM, Greg Clayton  > wrote:
>  
> The only way for us to find all classes whose type is "C" is to add the entry 
> for all template classes named "C", so I would vote to add them as it is 
> accurate. Do we currently add one for "C<12, 16>”?
>  
> Yes we do, not sure it is actually useful.
>  
> Fred
> 
> 
>  
> Greg
> 
> 
> On May 8, 2018, at 8:58 AM, Frédéric Riss via lldb-commits 
> mailto:lldb-commits@lists.llvm.org>> wrote:
>  
> 
> 
> 
> On May 8, 2018, at 8:30 AM, paul.robin...@sony.com 
>  wrote:
>  
> 
> 
> 
> -Original Message-
> From: lldb-commits [mailto:lldb-commits-boun...@lists.llvm.org 
> ] On Behalf
> Of Pavel Labath via lldb-commits
> Sent: Tuesday, May 08, 2018 10:48 AM
> To: fr...@apple.com 
> Cc: lldb-commits@lists.llvm.org 
> Subject: Re: [Lldb-commits] [RFC] Type lookup for template types is
> broken...
> 
> Well.. it encodes some assumptions about how a class name looks like,
> which
> are probably valid for C++, but they don't have to hold for any language
> frontend LLVM supports. That said, I am not saying this is worth the
> trouble of adding a special "these are the additional names you are to
> insert into the index" channel that clang should use to communicate this
> (I
> wouldn't be surprised if we make even stronger assumptions elsewhere). I
> was just curious about what your thoughts here were.
> 
> If you add an accelerator entry for "C" what does it point to?  All the
> instantiations of "C"?  The DWARF does not describe the template, only
> the concrete instances.
>  
> Yes, there would be a “C” entry for every instantiation of C.
>  
> Fred
> 
> 
> --paulr
> 
> 
> 
> On Tue, 8 May 2018 at 15:29, Frédéric Riss  > wrote:
> 
> 
> 
> 
> On May 8, 2018, at 2:23 AM, Pavel Labath  > wrote:
> 
> 
> I am still building a picture for myself of how the accelerator tables
> and
> 
> our name lookup works, but from what I managed to learn so far, adding
> an
> 
> accelerator for "C" seems like a useful thing to do. However, this does
> go
> 
> beyond what the DWARF 5 spec says we should do (we are only required to
> add
> 
> the DW_AT_name string). We are still free to add any extra entries we
> like,
> 
> but if we're going to be relying on this, we should try to get some of
> this
> 
> into the next version of the spec.
> 
> 
> 
> On Mon, 7 May 2018 at 22:19, Frédéric Riss via lldb-commits <
> lldb-commits@lists.llvm.org > wrote:
> 
> 
> (...At least when using accelerator tables)
> 
> 
> 
> If you apply the following patch, TestClassTemplateParameterPack.py will
> 
> 
> start failing:
> 
> 
> diff --git
> 
> 
> a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
> pack/main.cpp
> 
> b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
> pack/main.cpp
> 
> 
> index 90e63b40f..304872a15 100644
> ---
> 
> 
> a/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
> pack/main.cpp
> 
> 
> +++
> 
> 
> b/packages/Python/lldbsuite/test/lang/cpp/class-template-parameter-
> pack/main.cpp
> 
> 
> @@ -37,7 +37,7 @@ template <> struct D : D {
> 
> 
> 
> 
> 
>  int main (int argc, char const *argv[])
>  {
> -C myC;
>

[Lldb-commits] [PATCH] D46576: [DWARF] Align non-accelerated function fullname searching with the apple-tables path

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:905-908
 ConstString demangled = mangled.GetDemangledName(cu_language);
 if (demangled)
   func_fullnames.Insert(demangled,
 DIERef(cu_offset, die.GetOffset()));

clayborg wrote:
> Should we remove these 4 lines now? I thought we weren't going to add 
> demangled names to the index?
I was saving that for a separate patch.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:928-929
 Mangled mangled(ConstString(mangled_cstr), true);
+func_basenames.Insert(mangled.GetMangledName(),
+  DIERef(cu_offset, die.GetOffset()));
 func_fullnames.Insert(mangled.GetMangledName(),

clayborg wrote:
> Why are we adding the mangled name to the basenames?
It was the simplest way to ensure we get the same results for apple and !apple 
cases. With apple tables we will return a result if someone asks for a function 
with a "base name" `_Z3foov`. I am not sure if this was intended or just an 
accident. Alternatively, I could keep these names in a separate index and then 
search in both when I get a query. Or, if this is really not intended to work, 
I can adding extra filtering to the "apple" path to return only "real" 
basenames. (My goal here is to eliminate platform differences to avoid things 
behaving differently across platforms.)


https://reviews.llvm.org/D46576



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46576: [DWARF] Align non-accelerated function fullname searching with the apple-tables path

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:928-929
 Mangled mangled(ConstString(mangled_cstr), true);
+func_basenames.Insert(mangled.GetMangledName(),
+  DIERef(cu_offset, die.GetOffset()));
 func_fullnames.Insert(mangled.GetMangledName(),

labath wrote:
> clayborg wrote:
> > Why are we adding the mangled name to the basenames?
> It was the simplest way to ensure we get the same results for apple and 
> !apple cases. With apple tables we will return a result if someone asks for a 
> function with a "base name" `_Z3foov`. I am not sure if this was intended or 
> just an accident. Alternatively, I could keep these names in a separate index 
> and then search in both when I get a query. Or, if this is really not 
> intended to work, I can adding extra filtering to the "apple" path to return 
> only "real" basenames. (My goal here is to eliminate platform differences to 
> avoid things behaving differently across platforms.)
I would search "func_fullnames" and "func_basenames" separately and avoid 
adding the extra mangled name to func_basenames.

The _Z3foov seems like an accident. It definitely shouldn't be a basename 
unless the DWARF put that into the DW_AT_name field for some reason.


https://reviews.llvm.org/D46576



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46529: Add support to object files for accessing the .debug_types section

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 145715.
clayborg added a comment.

Added lit test.


https://reviews.llvm.org/D46529

Files:
  include/lldb/lldb-enumerations.h
  lit/Modules/elf-section-types.yaml
  source/Core/Section.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Symbol/ObjectFile.cpp

Index: source/Symbol/ObjectFile.cpp
===
--- source/Symbol/ObjectFile.cpp
+++ source/Symbol/ObjectFile.cpp
@@ -359,6 +359,7 @@
   case eSectionTypeDWARFDebugRanges:
   case eSectionTypeDWARFDebugStr:
   case eSectionTypeDWARFDebugStrOffsets:
+  case eSectionTypeDWARFDebugTypes:
   case eSectionTypeDWARFAppleNames:
   case eSectionTypeDWARFAppleTypes:
   case eSectionTypeDWARFAppleNamespaces:
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -246,6 +246,7 @@
   const lldb_private::DWARFDataExtractor &get_debug_ranges_data();
   const lldb_private::DWARFDataExtractor &get_debug_str_data();
   const lldb_private::DWARFDataExtractor &get_debug_str_offsets_data();
+  const lldb_private::DWARFDataExtractor &get_debug_types_data();
   const lldb_private::DWARFDataExtractor &get_apple_names_data();
   const lldb_private::DWARFDataExtractor &get_apple_types_data();
   const lldb_private::DWARFDataExtractor &get_apple_namespaces_data();
@@ -492,6 +493,7 @@
   DWARFDataSegment m_data_debug_ranges;
   DWARFDataSegment m_data_debug_str;
   DWARFDataSegment m_data_debug_str_offsets;
+  DWARFDataSegment m_data_debug_types;
   DWARFDataSegment m_data_apple_names;
   DWARFDataSegment m_data_apple_types;
   DWARFDataSegment m_data_apple_namespaces;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -665,6 +665,10 @@
   m_data_debug_str_offsets);
 }
 
+const DWARFDataExtractor &SymbolFileDWARF::get_debug_types_data() {
+  return GetCachedSectionData(eSectionTypeDWARFDebugTypes, m_data_debug_types);
+}
+
 const DWARFDataExtractor &SymbolFileDWARF::get_apple_names_data() {
   return GetCachedSectionData(eSectionTypeDWARFAppleNames, m_data_apple_names);
 }
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -696,6 +696,7 @@
 static ConstString g_sect_name_dwarf_debug_pubtypes(".debug_pubtypes");
 static ConstString g_sect_name_dwarf_debug_ranges(".debug_ranges");
 static ConstString g_sect_name_dwarf_debug_str(".debug_str");
+static ConstString g_sect_name_dwarf_debug_types(".debug_types");
 static ConstString g_sect_name_eh_frame(".eh_frame");
 static ConstString g_sect_name_go_symtab(".gosymtab");
 SectionType section_type = eSectionTypeOther;
@@ -744,6 +745,8 @@
   section_type = eSectionTypeDWARFDebugRanges;
 else if (const_sect_name == g_sect_name_dwarf_debug_str)
   section_type = eSectionTypeDWARFDebugStr;
+else if (const_sect_name == g_sect_name_dwarf_debug_types)
+  section_type = eSectionTypeDWARFDebugTypes;
 else if (const_sect_name == g_sect_name_eh_frame)
   section_type = eSectionTypeEHFrame;
 else if (const_sect_name == g_sect_name_go_symtab)
Index: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1205,6 +1205,7 @@
   case eSectionTypeDWARFDebugRanges:
   case eSectionTypeDWARFDebugStr:
   case eSectionTypeDWARFDebugStrOffsets:
+  case eSectionTypeDWARFDebugTypes:
   case eSectionTypeDWARFAppleNames:
   case eSectionTypeDWARFAppleTypes:
   case eSectionTypeDWARFAppleNamespaces:
@@ -1458,6 +1459,7 @@
   static ConstString g_sect_name_dwarf_debug_pubtypes("__debug_pubtypes");
   static ConstString g_sect_name_dwarf_debug_ranges("__debug_ranges");
   static ConstString g_sect_name_dwarf_debug_str("__debug_str");
+  static ConstString g_sect_name_dwarf_debug_types("__debug_types");
   static ConstString g_sect_name_dwarf_apple_names("__apple_names");
   static ConstString g_sect_name_dwarf_apple_types("__apple_types");
   static Cons

[Lldb-commits] [PATCH] D46576: [DWARF] Align non-accelerated function fullname searching with the apple-tables path

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 145717.
labath added a comment.

Stopped inserting demangled names into the fullname index and made the
FindFunctions index search in all three indices (fullname, basename, method).
The last two shouldn't really contain full names, but they seem to be required
for the correct operation (and they match what the apple path does).


https://reviews.llvm.org/D46576

Files:
  lit/SymbolFile/DWARF/find-basic-function.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2622,44 +2622,26 @@
 if (!m_indexed)
   Index();
 
+DIEArray die_offsets;
 if (name_type_mask & eFunctionNameTypeFull) {
-  FindFunctions(name, m_function_fullname_index, include_inlines, sc_list);
-
-  // FIXME Temporary workaround for global/anonymous namespace
-  // functions debugging FreeBSD and Linux binaries. If we didn't find any
-  // functions in the global namespace try looking in the basename index
-  // but ignore any returned functions that have a namespace but keep
-  // functions which have an anonymous namespace
-  // TODO: The arch in the object file isn't correct for MSVC
-  // binaries on windows, we should find a way to make it correct and
-  // handle those symbols as well.
-  if (sc_list.GetSize() == original_size) {
-ArchSpec arch;
-if (!parent_decl_ctx && GetObjectFile()->GetArchitecture(arch) &&
-arch.GetTriple().isOSBinFormatELF()) {
-  SymbolContextList temp_sc_list;
-  FindFunctions(name, m_function_basename_index, include_inlines,
-temp_sc_list);
-  SymbolContext sc;
-  for (uint32_t i = 0; i < temp_sc_list.GetSize(); i++) {
-if (temp_sc_list.GetContextAtIndex(i, sc)) {
-  ConstString mangled_name =
-  sc.GetFunctionName(Mangled::ePreferMangled);
-  ConstString demangled_name =
-  sc.GetFunctionName(Mangled::ePreferDemangled);
-  // Mangled names on Linux and FreeBSD are of the form:
-  // _ZN18function_namespace13function_nameEv.
-  if (strncmp(mangled_name.GetCString(), "_ZN", 3) ||
-  !strncmp(demangled_name.GetCString(), "(anonymous namespace)",
-   21)) {
-sc_list.Append(sc);
-  }
-}
+  uint32_t num_matches = m_function_basename_index.Find(name, die_offsets);
+  num_matches += m_function_method_index.Find(name, die_offsets);
+  num_matches += m_function_fullname_index.Find(name, die_offsets);
+  for (uint32_t i = 0; i < num_matches; i++) {
+const DIERef &die_ref = die_offsets[i];
+DWARFDIE die = info->GetDIE(die_ref);
+if (die) {
+  if (!DIEInDeclContext(parent_decl_ctx, die))
+continue; // The containing decl contexts don't match
+
+  if (resolved_dies.find(die.GetDIE()) == resolved_dies.end()) {
+if (ResolveFunction(die, include_inlines, sc_list))
+  resolved_dies.insert(die.GetDIE());
   }
 }
   }
+  die_offsets.clear();
 }
-DIEArray die_offsets;
 if (name_type_mask & eFunctionNameTypeBase) {
   uint32_t num_base = m_function_basename_index.Find(name, die_offsets);
   for (uint32_t i = 0; i < num_base; i++) {
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -897,13 +897,8 @@
   if (name && name != mangled_cstr &&
   ((mangled_cstr[0] == '_') ||
(::strcmp(name, mangled_cstr) != 0))) {
-Mangled mangled(ConstString(mangled_cstr), true);
-func_fullnames.Insert(mangled.GetMangledName(),
+func_fullnames.Insert(ConstString(mangled_cstr),
   DIERef(cu_offset, die.GetOffset()));
-ConstString demangled = mangled.GetDemangledName(cu_language);
-if (demangled)
-  func_fullnames.Insert(demangled,
-DIERef(cu_offset, die.GetOffset()));
   }
 }
   }
@@ -922,13 +917,8 @@
   if (name && name != mangled_cstr &&
   ((mangled_cstr[0] == '_') ||
(::strcmp(name, mangled_cstr) != 0))) {
-Mangled mangled(ConstString(mangled_cstr), true);
-func_fullnames.Insert(mangled.GetMangledName(),
+func_fullnames.Insert(ConstString(mangled_cstr),
   DIERef(cu_offset, die.GetOffset()));
-Con

[Lldb-commits] [PATCH] D46529: Add support to object files for accessing the .debug_types section

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

BTW: I added the .debug_info verification to the lit test as there was a 
section made, but it wasn't checked.


https://reviews.llvm.org/D46529



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46529: Add support to object files for accessing the .debug_types section

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

Thanks for taking the time to split this!


https://reviews.llvm.org/D46529



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46576: [DWARF] Align non-accelerated function fullname searching with the apple-tables path

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

Much better.


https://reviews.llvm.org/D46576



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46576: [DWARF] Align non-accelerated function fullname searching with the apple-tables path

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Before adding the DWARF 5 index support, we should virtualize this searching 
and indexing into a base class. Then we make 1 class for manual DWARF indexing, 
one for Apple indexes, and one for DWARF5. Then we can unit test each one 
easily. Thoughts?


https://reviews.llvm.org/D46576



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46576: [DWARF] Align non-accelerated function fullname searching with the apple-tables path

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

the class would be something like:

class DWARFIndex {

  virtual bool Initialize(SymbolFileDWARF &dwarf); // Do manual indexing, or 
load indexes. Return true if successful (found apple tables, DWARF 5 tables, or 
had debug info to manually index)
  virtual ... Lookup(ConstString name, type, etc);

};


https://reviews.llvm.org/D46576



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46529: Add support to object files for accessing the .debug_types section

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Pavel, let me know if you are good with this.


https://reviews.llvm.org/D46529



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r331777 - Add support to object files for accessing the .debug_types section

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Tue May  8 10:19:24 2018
New Revision: 331777

URL: http://llvm.org/viewvc/llvm-project?rev=331777&view=rev
Log:
Add support to object files for accessing the .debug_types section

In an effort to make the .debug_types patch smaller, breaking out the part that 
reads the .debug_types from object files into a separate patch

Differential Revision: https://reviews.llvm.org/D46529


Modified:
lldb/trunk/include/lldb/lldb-enumerations.h
lldb/trunk/lit/Modules/elf-section-types.yaml
lldb/trunk/source/Core/Section.cpp
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Symbol/ObjectFile.cpp

Modified: lldb/trunk/include/lldb/lldb-enumerations.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/lldb-enumerations.h?rev=331777&r1=331776&r2=331777&view=diff
==
--- lldb/trunk/include/lldb/lldb-enumerations.h (original)
+++ lldb/trunk/include/lldb/lldb-enumerations.h Tue May  8 10:19:24 2018
@@ -656,6 +656,7 @@ enum SectionType {
   eSectionTypeAbsoluteAddress, // Dummy section for symbols with absolute
// address
   eSectionTypeDWARFGNUDebugAltLink,
+  eSectionTypeDWARFDebugTypes, // DWARF .debug_types section
   eSectionTypeOther
 };
 

Modified: lldb/trunk/lit/Modules/elf-section-types.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/elf-section-types.yaml?rev=331777&r1=331776&r2=331777&view=diff
==
--- lldb/trunk/lit/Modules/elf-section-types.yaml (original)
+++ lldb/trunk/lit/Modules/elf-section-types.yaml Tue May  8 10:19:24 2018
@@ -4,6 +4,16 @@
 # CHECK: Name: .text
 # CHECK-NEXT: Type: code
 
+# CHECK: Name: .debug_info
+# CHECK-NEXT: Type: dwarf-info
+# CHECK-NEXT: VM size: 0
+# CHECK-NEXT: File size: 8
+
+# CHECK: Name: .debug_types
+# CHECK-NEXT: Type: dwarf-types
+# CHECK-NEXT: VM size: 0
+# CHECK-NEXT: File size: 8
+
 # CHECK: Name: .gnu_debugaltlink
 # CHECK-NEXT: Type: dwarf-gnu-debugaltlink
 # CHECK-NEXT: VM size: 0
@@ -13,13 +23,13 @@
 # CHECK-NEXT: Type: code
 
 --- !ELF
-FileHeader:  
+FileHeader:
   Class:   ELFCLASS64
   Data:ELFDATA2LSB
   Type:ET_DYN
   Machine: EM_X86_64
   Entry:   0x07A0
-Sections:
+Sections:
   - Name:.text
 Type:SHT_PROGBITS
 Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
@@ -28,6 +38,10 @@ Sections:
 Type:SHT_PROGBITS
 AddressAlign:0x0001
 Content: DEADBEEFBAADF00D
+  - Name:.debug_types
+Type:SHT_PROGBITS
+AddressAlign:0x0001
+Content: DEADBEEFBAADF00D
   - Name:.gnu_debugaltlink
 Type:SHT_PROGBITS
 AddressAlign:0x0001

Modified: lldb/trunk/source/Core/Section.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Section.cpp?rev=331777&r1=331776&r2=331777&view=diff
==
--- lldb/trunk/source/Core/Section.cpp (original)
+++ lldb/trunk/source/Core/Section.cpp Tue May  8 10:19:24 2018
@@ -89,6 +89,8 @@ const char *Section::GetTypeAsCString()
 return "dwarf-str";
   case eSectionTypeDWARFDebugStrOffsets:
 return "dwarf-str-offsets";
+  case eSectionTypeDWARFDebugTypes:
+return "dwarf-types";
   case eSectionTypeELFSymbolTable:
 return "elf-symbol-table";
   case eSectionTypeELFDynamicSymbols:

Modified: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp?rev=331777&r1=331776&r2=331777&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Tue May  8 
10:19:24 2018
@@ -1806,6 +1806,7 @@ void ObjectFileELF::CreateSections(Secti
   static ConstString g_sect_name_dwarf_debug_str_dwo(".debug_str.dwo");
   static ConstString g_sect_name_dwarf_debug_str_offsets_dwo(
   ".debug_str_offsets.dwo");
+  static ConstString g_sect_name_dwarf_debug_types(".debug_types");
   static ConstString g_sect_name_eh_frame(".eh_frame");
   static ConstString g_sect_name_arm_exidx(".ARM.exidx");
   static ConstString g_sect_name_arm_extab(".ARM.extab");
@@ -1873,6 +1874,8 @@ void ObjectFileELF::CreateSections(Secti
 sect_type = eSectionTypeDWARFDebugRanges;
   else if (name == g_

[Lldb-commits] [PATCH] D46529: Add support to object files for accessing the .debug_types section

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL331777: Add support to object files for accessing the 
.debug_types section (authored by gclayton, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D46529?vs=145715&id=145723#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D46529

Files:
  lldb/trunk/include/lldb/lldb-enumerations.h
  lldb/trunk/lit/Modules/elf-section-types.yaml
  lldb/trunk/source/Core/Section.cpp
  lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/trunk/source/Symbol/ObjectFile.cpp

Index: lldb/trunk/source/Core/Section.cpp
===
--- lldb/trunk/source/Core/Section.cpp
+++ lldb/trunk/source/Core/Section.cpp
@@ -89,6 +89,8 @@
 return "dwarf-str";
   case eSectionTypeDWARFDebugStrOffsets:
 return "dwarf-str-offsets";
+  case eSectionTypeDWARFDebugTypes:
+return "dwarf-types";
   case eSectionTypeELFSymbolTable:
 return "elf-symbol-table";
   case eSectionTypeELFDynamicSymbols:
Index: lldb/trunk/source/Symbol/ObjectFile.cpp
===
--- lldb/trunk/source/Symbol/ObjectFile.cpp
+++ lldb/trunk/source/Symbol/ObjectFile.cpp
@@ -359,6 +359,7 @@
   case eSectionTypeDWARFDebugRanges:
   case eSectionTypeDWARFDebugStr:
   case eSectionTypeDWARFDebugStrOffsets:
+  case eSectionTypeDWARFDebugTypes:
   case eSectionTypeDWARFAppleNames:
   case eSectionTypeDWARFAppleTypes:
   case eSectionTypeDWARFAppleNamespaces:
Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1806,6 +1806,7 @@
   static ConstString g_sect_name_dwarf_debug_str_dwo(".debug_str.dwo");
   static ConstString g_sect_name_dwarf_debug_str_offsets_dwo(
   ".debug_str_offsets.dwo");
+  static ConstString g_sect_name_dwarf_debug_types(".debug_types");
   static ConstString g_sect_name_eh_frame(".eh_frame");
   static ConstString g_sect_name_arm_exidx(".ARM.exidx");
   static ConstString g_sect_name_arm_extab(".ARM.extab");
@@ -1873,6 +1874,8 @@
 sect_type = eSectionTypeDWARFDebugRanges;
   else if (name == g_sect_name_dwarf_debug_str)
 sect_type = eSectionTypeDWARFDebugStr;
+  else if (name == g_sect_name_dwarf_debug_types)
+sect_type = eSectionTypeDWARFDebugTypes;
   else if (name == g_sect_name_dwarf_debug_str_offsets)
 sect_type = eSectionTypeDWARFDebugStrOffsets;
   else if (name == g_sect_name_dwarf_debug_abbrev_dwo)
Index: lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1205,6 +1205,7 @@
   case eSectionTypeDWARFDebugRanges:
   case eSectionTypeDWARFDebugStr:
   case eSectionTypeDWARFDebugStrOffsets:
+  case eSectionTypeDWARFDebugTypes:
   case eSectionTypeDWARFAppleNames:
   case eSectionTypeDWARFAppleTypes:
   case eSectionTypeDWARFAppleNamespaces:
@@ -1458,6 +1459,7 @@
   static ConstString g_sect_name_dwarf_debug_pubtypes("__debug_pubtypes");
   static ConstString g_sect_name_dwarf_debug_ranges("__debug_ranges");
   static ConstString g_sect_name_dwarf_debug_str("__debug_str");
+  static ConstString g_sect_name_dwarf_debug_types("__debug_types");
   static ConstString g_sect_name_dwarf_apple_names("__apple_names");
   static ConstString g_sect_name_dwarf_apple_types("__apple_types");
   static ConstString g_sect_name_dwarf_apple_namespaces("__apple_namespac");
@@ -1490,6 +1492,8 @@
 return eSectionTypeDWARFDebugRanges;
   if (section_name == g_sect_name_dwarf_debug_str)
 return eSectionTypeDWARFDebugStr;
+  if (section_name == g_sect_name_dwarf_debug_types)
+return eSectionTypeDWARFDebugTypes;
   if (section_name == g_sect_name_dwarf_apple_names)
 return eSectionTypeDWARFAppleNames;
   if (section_name == g_sect_name_dwarf_apple_types)
Index: lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -696,6 +696,7 @@
 static ConstString g_sect_name_dwarf_debug_pubtypes(

Re: [Lldb-commits] [RFC] Type lookup for template types is broken...

[via lldb-commits]

Clang can control the emission of  or not in the name, using the 
"debugger tuning" feature.  It sounds like neither LLDB nor PS4 will mind 
losing the  in the name, and it makes the indexing simpler.  If the 
tuning is set for GDB then we can still emit the  in the names.

It would make me uncomfortable to have the index and the actual DWARF be 
inconsistent, but given the way templates work in general (a variety of 
spellings all refer to the same type) it is probably better to use the 
un-decorated name in the index even if the DWARF itself used decorated names.  
This should probably be proposed as a revision for DWARF 6.
--paulr

From: Greg Clayton [mailto:clayb...@gmail.com]
Sent: Tuesday, May 08, 2018 12:25 PM
To: Robinson, Paul
Cc: fr...@apple.com; lldb-commits@lists.llvm.org
Subject: Re: [Lldb-commits] [RFC] Type lookup for template types is broken...

I think for display purposes, the type name should include all of the . 
If I have two variables, both using class C, but both have different template 
parameters, I want to see that in the class name and have that properly show up 
in the variable view. LLDB doesn't actually care about the name that is in the 
DWARF because we will show the type name by grabbing it from the 
lldb_private::CompilerType which will end up being correct. Other debuggers 
might just display "C" if that is what is in the DWARF. So I would caution 
against just putting the type basename without params in the DWARF itself.

Greg



On May 8, 2018, at 9:12 AM, 
mailto:paul.robin...@sony.com>> 
mailto:paul.robin...@sony.com>> wrote:

So….  if the name in the type entry did not include the  then the 
indexing would automatically do what you want; you would need to reconstruct 
the  from the template parameter children of the DIE.  Would that help?
On the LLVM side we have debated the merits of  in the name back and 
forth.  I would have to investigate the current state of the world; I know that 
for PS4 we prefer to have the children and not put  in the name, while 
others prefer it the other way around.  If LLDB came out preferring not to have 
a decorated name, it should not be hard to accommodate that in Clang.
Not sure where gcc/gdb stand on this point.
--paulr

From: fr...@apple.com [mailto:fr...@apple.com]
Sent: Tuesday, May 08, 2018 12:07 PM
To: Greg Clayton
Cc: Robinson, Paul; 
lldb-commits@lists.llvm.org
Subject: Re: [Lldb-commits] [RFC] Type lookup for template types is broken...





On May 8, 2018, at 9:04 AM, Greg Clayton 
mailto:clayb...@gmail.com>> wrote:

The only way for us to find all classes whose type is "C" is to add the entry 
for all template classes named "C", so I would vote to add them as it is 
accurate. Do we currently add one for "C<12, 16>”?

Yes we do, not sure it is actually useful.

Fred




Greg



On May 8, 2018, at 8:58 AM, Frédéric Riss via lldb-commits 
mailto:lldb-commits@lists.llvm.org>> wrote:





On May 8, 2018, at 8:30 AM, 
paul.robin...@sony.com wrote:





-Original Message-
From: lldb-commits [mailto:lldb-commits-boun...@lists.llvm.org] On Behalf
Of Pavel Labath via lldb-commits
Sent: Tuesday, May 08, 2018 10:48 AM
To: fr...@apple.com
Cc: lldb-commits@lists.llvm.org
Subject: Re: [Lldb-commits] [RFC] Type lookup for template types is
broken...

Well.. it encodes some assumptions about how a class name looks like,
which
are probably valid for C++, but they don't have to hold for any language
frontend LLVM supports. That said, I am not saying this is worth the
trouble of adding a special "these are the additional names you are to
insert into the index" channel that clang should use to communicate this
(I
wouldn't be surprised if we make even stronger assumptions elsewhere). I
was just curious about what your thoughts here were.

If you add an accelerator entry for "C" what does it point to?  All the
instantiations of "C"?  The DWARF does not describe the template, only
the concrete instances.

Yes, there would be a “C” entry for every instantiation of C.

Fred



--paulr




On Tue, 8 May 2018 at 15:29, Frédéric Riss 
mailto:fr...@apple.com>> wrote:





On May 8, 2018, at 2:23 AM, Pavel Labath 
mailto:lab...@google.com>> wrote:



I am still building a picture for myself of how the accelerator tables
and


our name lookup works, but from what I managed to learn so far, adding
an


accelerator for "C" seems like a useful thing to do. However, this does
go


beyond what the DWARF 5 spec says we should do (we are only required to
add


the DW_AT_name string). We are still free to add any extra entries we
like,


but if we're going to be relying on this, we should try to get some of
this


into the next version of the spec.




On Mon, 7 May 2018 at 22:19, Frédéric Riss via lldb-commits <
lldb-commits@lists.llvm.org> wrote:



(...At least when

[Lldb-commits] [PATCH] D46588: [WIP][LLDB-MI] Add possibility to set breakpoints without selecting a target.

[Alexander Polyakov via Phabricator via lldb-commits]

polyakov.alex created this revision.
polyakov.alex added reviewers: aprantl, clayborg.
polyakov.alex added a project: LLDB.
Herald added subscribers: llvm-commits, ki.stfu.

It's not a final version of the patch, testcase will be added.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588

Files:
  tools/lldb-mi/MICmdCmdBreak.cpp
  tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp


Index: tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  return GetDebugger().GetDummyTarget();
 }
 
 //++
Index: tools/lldb-mi/MICmdCmdBreak.cpp
===
--- tools/lldb-mi/MICmdCmdBreak.cpp
+++ tools/lldb-mi/MICmdCmdBreak.cpp
@@ -148,6 +148,11 @@
   CMICMDBASE_GETOPTION(pArgRestrictBrkPtToThreadId, OptionShort,
m_constStrArgNamedRestrictBrkPtToThreadId);
 
+  // Ask LLDB for the target to check if we have valid or dummy one.
+  CMICmnLLDBDebugSessionInfo &rSessionInfo(
+  CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
+
   m_bBrkPtEnabled = !pArgDisableBrkPt->GetFound();
   m_bBrkPtIsTemp = pArgTempBrkPt->GetFound();
   m_bHaveArgOptionThreadGrp = pArgThreadGroup->GetFound();
@@ -157,7 +162,12 @@
 nThreadGrp);
 m_strArgOptionThreadGrp = CMIUtilString::Format("i%d", nThreadGrp);
   }
-  m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;
+  else
+m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
   if (pArgLocation->GetFound())
 m_brkName = pArgLocation->GetValue();
   else if (m_bBrkPtIsPending) {
@@ -225,9 +235,6 @@
 
   // Ask LLDB to create a breakpoint
   bool bOk = MIstatus::success;
-  CMICmnLLDBDebugSessionInfo &rSessionInfo(
-  CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
   switch (eBrkPtType) {
   case eBreakPoint_ByAddress:
 m_brkPt = sbTarget.BreakpointCreateByAddress(nAddress);


Index: tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
===
--- tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
+++ tools/lldb-mi/MICmnLLDBDebugSessionInfo.cpp
@@ -871,7 +871,10 @@
 // Throws:  None.
 //--
 lldb::SBTarget CMICmnLLDBDebugSessionInfo::GetTarget() const {
-  return GetDebugger().GetSelectedTarget();
+  auto target = GetDebugger().GetSelectedTarget();
+  if (target.IsValid())
+return target;
+  return GetDebugger().GetDummyTarget();
 }
 
 //++
Index: tools/lldb-mi/MICmdCmdBreak.cpp
===
--- tools/lldb-mi/MICmdCmdBreak.cpp
+++ tools/lldb-mi/MICmdCmdBreak.cpp
@@ -148,6 +148,11 @@
   CMICMDBASE_GETOPTION(pArgRestrictBrkPtToThreadId, OptionShort,
m_constStrArgNamedRestrictBrkPtToThreadId);
 
+  // Ask LLDB for the target to check if we have valid or dummy one.
+  CMICmnLLDBDebugSessionInfo &rSessionInfo(
+  CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
+
   m_bBrkPtEnabled = !pArgDisableBrkPt->GetFound();
   m_bBrkPtIsTemp = pArgTempBrkPt->GetFound();
   m_bHaveArgOptionThreadGrp = pArgThreadGroup->GetFound();
@@ -157,7 +162,12 @@
 nThreadGrp);
 m_strArgOptionThreadGrp = CMIUtilString::Format("i%d", nThreadGrp);
   }
-  m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;
+  else
+m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
+
   if (pArgLocation->GetFound())
 m_brkName = pArgLocation->GetValue();
   else if (m_bBrkPtIsPending) {
@@ -225,9 +235,6 @@
 
   // Ask LLDB to create a breakpoint
   bool bOk = MIstatus::success;
-  CMICmnLLDBDebugSessionInfo &rSessionInfo(
-  CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBTarget sbTarget = rSessionInfo.GetTarget();
   switch (eBrkPtType) {
   case eBreakPoint_ByAddress:
 m_brkPt = sbTarget.BreakpointCreateByAddress(nAddress);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46588: [WIP][LLDB-MI] Add possibility to set breakpoints without selecting a target.

[Alexander Polyakov via Phabricator via lldb-commits]

polyakov.alex added inline comments.



Comment at: tools/lldb-mi/MICmdCmdBreak.cpp:166
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;

I want to discuss is it a good way to use 'pending' breakpoint's here?


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46588: [WIP][LLDB-MI] Add possibility to set breakpoints without selecting a target.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Add tests and this will be good to go.




Comment at: tools/lldb-mi/MICmdCmdBreak.cpp:166
+
+  if (sbTarget == rSessionInfo.GetDebugger().GetDummyTarget())
+m_bBrkPtIsPending = true;

polyakov.alex wrote:
> I want to discuss is it a good way to use 'pending' breakpoint's here?
This looks fine.


Repository:
  rL LLVM

https://reviews.llvm.org/D46588



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [RFC] Type lookup for template types is broken...

[Adrian Prantl via lldb-commits]

I made a small experiment where I manually edited the DW_AT_name in the 
assembler output:


Baseline:

$ lldb with_params
(lldb) target create "with_params"
(lldb) b 5
(lldb) r
Process 28369 stopped
* thread #1, name = 'with_params', stop reason = breakpoint 1.1
frame #0: 0x004004fc with_params`main(argc=1, 
argv=0x7fffe578) at template.cpp:5
   2
   3int main(int argc, char **argv) {
   4  MyC c = {23};
-> 5  return c.t;
   6}
Target 0: (with_params) stopped.
(lldb) p c
(MyC) $0 = (t = 23)
^^


Without the parameters:


$ lldb no_params
(lldb) target create "no_params"
(lldb) b 5
* thread #1, name = 'no_params', stop reason = breakpoint 1.1
frame #0: 0x004004fc no_params`main(argc=1, 
argv=0x7fffe588) at template.cpp:5
   2
   3int main(int argc, char **argv) {
   4  MyC c = {23};
-> 5  return c.t;
   6}
Target 0: (no_params) stopped.
(lldb) p c
(MyC) $0 = (t = 23)
^


Note how lldb uses the typename to print the result type of the expression.

-- adrian
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46606: General cleanup to minimize the .debug_types patch

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: labath, aprantl.
Herald added a subscriber: JDevlieghere.

This cleanup is designed to make the https://reviews.llvm.org/D32167 patch 
smaller and easier to read.

Cleanup in this patch:

Allow DWARFUnit subclasses to hand out the data that should be used when 
decoding data for a DIE. The information might be in .debug_info or could be in 
.debug_types. There is a new virtual function on DWARFUnit that each subclass 
must override:

  virtual const lldb_private::DWARFDataExtractor &DWARFUnit::GetData() const;

This allows DWARFCompileUnit and eventually DWARFTypeUnit to hand out different 
data to be used when decoding the DIE information.

Add a new pure virtual function to get the size of the DWARF unit header:

  virtual uint32_t DWARFUnit::GetHeaderByteSize() const = 0;

This allows DWARFCompileUnit and eventually DWARFTypeUnit to hand out different 
offsets where the first DIE starts when decoding DIE information from the unit.

Added a new function to DWARFDataExtractor to get the size of an offset:

  size_t DWARFDataExtractor::GetDWARFSizeOfOffset() const;

Removed dead dumping and parsing code in the DWARFDebugInfo class.
Inlined a bunch of calls in DWARFUnit for accessors that were just returning 
integer member variables.
Renamed DWARFUnit::Size() to DWARFUnit::GetHeaderByteSize() as it clearly 
states what it is doing and makes more sense.

-


https://reviews.llvm.org/D46606

Files:
  include/lldb/lldb-forward.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserGo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
  source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3776,7 +3776,7 @@
   location_is_const_value_data = true;
   // The constant value will be either a block, a data value or a
   // string.
-  const DWARFDataExtractor &debug_info_data = get_debug_info_data();
+  auto debug_info_data = die.GetData();
   if (DWARFFormValue::IsBlockForm(form_value.Form())) {
 // Retrieve the value as a block expression.
 uint32_t block_offset =
@@ -3833,13 +3833,12 @@
 location_is_const_value_data = false;
 has_explicit_location = true;
 if (DWARFFormValue::IsBlockForm(form_value.Form())) {
-  const DWARFDataExtractor &debug_info_data = get_debug_info_data();
+  auto data = die.GetData();
 
   uint32_t block_offset =
-  form_value.BlockData() - debug_info_data.GetDataStart();
+  form_value.BlockData() - data.GetDataStart();
   uint32_t block_length = form_value.Unsigned();
-  location.CopyOpcodeData(module, get_debug_info_data(),
-  block_offset, block_length);
+  location.CopyOpcodeData(module, data, block_offset, block_length);
 } else {
   const DWARFDataExtractor &debug_loc_data = get_debug_loc_data();
   const dw_offset_t debug_loc_offset = form_value.Unsigned();
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -44,28 +44,57 @@
uint32_t depth = UINT32_MAX) const;
   bool Verify(lldb_private::Stream *s) const;
   virtual void Dump(lldb_private::Stream *s) const = 0;
+  //--
+  /// Get the data that contains the DIE information for this unit.
+  ///
+  /// This will return the correct bytes that contain the data for
+  /// this DWARFUnit. It could be .debug_info or .debug_types
+  /// depending on where the data for this unit originates.
+  ///
+  /// @return
+  ///   The correct data for the DIE information in this unit.
+  //--
+  virtual const lldb_private::DWARFDataExtractor &GetData() const = 0;
+  //-