[Lldb-commits] [PATCH] D62316: DWARFContext: Make loading of sections thread-safe

[Pavel Labath via Phabricator via lldb-commits]

labath marked an inline comment as done.
labath added a comment.

In D62316#1513905 , @clayborg wrote:

> In D62316#1513894 , @labath wrote:
>
> > Two other options I see are:
> >
> > - initialize the sections immediately after creating the dwarf context. The 
> > main advantage of that would that it alings to code more with llvm (which 
> > also loads the sections up-front), and slighly faster subsequent accesses 
> > to the debug info. I don't think this should negatively impact the start up 
> > time, as the files are mmapped anyway, and so the "loading" will consist of 
> > some basic pointer arithmetic. Also, the SymbolFileDWARF object as a whole 
> > is created lazily, so the fact that it is being created means that somebody 
> > is going to access it immediately after that. And he cannot do anything 
> > with the symbol file without touching at least the debug_info section, 
> > which accounts for about 80% of all debug info.
>
>
> I'd be fine with this.


Ok, so let's go with the current solution to restore status quo, and I'll 
return to this idea later.




Comment at: source/Plugins/SymbolFile/DWARF/DWARFContext.h:25
+  struct SectionData {
+llvm::once_flag flag;
+DWARFDataExtractor data;

clayborg wrote:
> is llvm::once_flag better than std::once_flag? 
Not really, but it's needed because std::once_flag does not work on some more 
exotic platforms. Elsewhere, it's equivalent to std::once_flag.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62316/new/

https://reviews.llvm.org/D62316



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

[Lldb-commits] [lldb] r361602 - DWARFContext: Make loading of sections thread-safe

[Pavel Labath via lldb-commits]

Author: labath
Date: Fri May 24 01:04:03 2019
New Revision: 361602

URL: http://llvm.org/viewvc/llvm-project?rev=361602&view=rev
Log:
DWARFContext: Make loading of sections thread-safe

Summary:
SymbolFileDWARF used to load debug sections in a thread-safe manner.
When we moved to DWARFContext, we dropped the thread-safe part, because
we thought it was not necessary.

It turns out this was only mostly correct.

The "mostly" part is there because this is a problem only if we use the
manual index, as that is the only source of intra-module paralelism.
Also, this only seems to occur for extremely simple files (like the ones
I've been creating for tests lately), where we've managed to start
indexing before loading the debug_str section. Then, two threads start
to load the section simultaneously and produce wrong results.

On more complex files, something seems to be loading the debug_str section
before we start indexing, as I haven't been able to reproduce this
there, but I have not investigated what it is.

I've tried to come up with a test for this, but I haven't been able to
reproduce the problem reliably. Still, while doing so, I created a way
to generate many compile units on demand. Given that most of our tests
work with only one or two compile units, it seems like this could be
useful anyway.

Reviewers: aprantl, JDevlieghere, clayborg

Subscribers: arphaman, lldb-commits

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

Added:
lldb/trunk/lit/SymbolFile/DWARF/parallel-indexing-stress.s
Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h

Added: lldb/trunk/lit/SymbolFile/DWARF/parallel-indexing-stress.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/parallel-indexing-stress.s?rev=361602&view=auto
==
--- lldb/trunk/lit/SymbolFile/DWARF/parallel-indexing-stress.s (added)
+++ lldb/trunk/lit/SymbolFile/DWARF/parallel-indexing-stress.s Fri May 24 
01:04:03 2019
@@ -0,0 +1,82 @@
+# Stress-test the parallel indexing of compile units.
+
+# RUN: llvm-mc -triple x86_64-pc-linux %s -o %t -filetype=obj
+# RUN: %lldb %t -o "target variable A" -b | FileCheck %s
+
+# CHECK-COUNT-256: A = 47
+
+   .section.debug_str,"MS",@progbits,1
+.Linfo_string0:
+   .asciz  "Hand-written DWARF"
+.Lname:
+   .asciz  "A"
+.Linfo_string4:
+   .asciz  "int"   # string offset=95
+
+   .section.debug_abbrev,"",@progbits
+   .byte   1   # Abbreviation Code
+   .byte   17  # DW_TAG_compile_unit
+   .byte   1   # DW_CHILDREN_yes
+   .byte   37  # DW_AT_producer
+   .byte   14  # DW_FORM_strp
+   .byte   0   # EOM(1)
+   .byte   0   # EOM(2)
+   .byte   2   # Abbreviation Code
+   .byte   52  # DW_TAG_variable
+   .byte   0   # DW_CHILDREN_no
+   .byte   3   # DW_AT_name
+   .byte   14  # DW_FORM_strp
+   .byte   73  # DW_AT_type
+   .byte   19  # DW_FORM_ref4
+   .byte   2   # DW_AT_location
+   .byte   24  # DW_FORM_exprloc
+   .byte   0   # EOM(1)
+   .byte   0   # EOM(2)
+   .byte   3   # Abbreviation Code
+   .byte   36  # DW_TAG_base_type
+   .byte   0   # DW_CHILDREN_no
+   .byte   3   # DW_AT_name
+   .byte   14  # DW_FORM_strp
+   .byte   62  # DW_AT_encoding
+   .byte   11  # DW_FORM_data1
+   .byte   11  # DW_AT_byte_size
+   .byte   11  # DW_FORM_data1
+   .byte   0   # EOM(1)
+   .byte   0   # EOM(2)
+   .byte   0   # EOM(3)
+
+.macro generate_unit
+   .data
+A\@:
+   .long   47
+
+   .section.debug_str,"MS",@progbits,1
+
+   .section.debug_info,"",@progbits
+.Lcu_begin\@:
+   .long   .Ldebug_info_end\@-.Ldebug_info_start\@ # Length of Unit
+.Ldebug_info_start\@:
+   .short  4   # DWARF version number
+   .long   .debug_abbrev   # Offset Into Abbrev. Section
+   .byte   8   # Address Size (in bytes)
+   .byte   1   # Abbrev [1] 0xb:0x30 
DW_TAG_compile_unit
+   .long   .Linfo_string0  # DW_AT_producer
+   .byte   2   # Abbrev [2] 0x1e:0x15 DW_TAG_variable
+   .long   .Lname  # DW_AT_name
+   .long   .Lt

[Lldb-commits] [PATCH] D62316: DWARFContext: Make loading of sections thread-safe

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB361602: DWARFContext: Make loading of sections 
thread-safe (authored by labath, committed by ).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D62316?vs=200970&id=201134#toc

Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62316/new/

https://reviews.llvm.org/D62316

Files:
  lit/SymbolFile/DWARF/parallel-indexing-stress.s
  source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
  source/Plugins/SymbolFile/DWARF/DWARFContext.h

Index: source/Plugins/SymbolFile/DWARF/DWARFContext.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFContext.h
+++ source/Plugins/SymbolFile/DWARF/DWARFContext.h
@@ -12,6 +12,7 @@
 #include "DWARFDataExtractor.h"
 #include "lldb/Core/Section.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/Support/Threading.h"
 #include 
 
 namespace lldb_private {
@@ -20,19 +21,29 @@
   SectionList *m_main_section_list;
   SectionList *m_dwo_section_list;
 
-  llvm::Optional m_data_debug_abbrev;
-  llvm::Optional m_data_debug_addr;
-  llvm::Optional m_data_debug_aranges;
-  llvm::Optional m_data_debug_info;
-  llvm::Optional m_data_debug_line;
-  llvm::Optional m_data_debug_line_str;
-  llvm::Optional m_data_debug_macro;
-  llvm::Optional m_data_debug_str;
-  llvm::Optional m_data_debug_str_offsets;
-  llvm::Optional m_data_debug_types;
+  struct SectionData {
+llvm::once_flag flag;
+DWARFDataExtractor data;
+  };
+
+  SectionData m_data_debug_abbrev;
+  SectionData m_data_debug_addr;
+  SectionData m_data_debug_aranges;
+  SectionData m_data_debug_info;
+  SectionData m_data_debug_line;
+  SectionData m_data_debug_line_str;
+  SectionData m_data_debug_macro;
+  SectionData m_data_debug_str;
+  SectionData m_data_debug_str_offsets;
+  SectionData m_data_debug_types;
 
   bool isDwo() { return m_dwo_section_list != nullptr; }
 
+  const DWARFDataExtractor &
+  LoadOrGetSection(lldb::SectionType main_section_type,
+   llvm::Optional dwo_section_type,
+   SectionData &data);
+
 public:
   explicit DWARFContext(SectionList *main_section_list,
 SectionList *dwo_section_list)
Index: source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
@@ -27,72 +27,66 @@
   return data;
 }
 
-static const DWARFDataExtractor &
-LoadOrGetSection(SectionList *section_list, SectionType section_type,
- llvm::Optional &extractor) {
-  if (!extractor)
-extractor = LoadSection(section_list, section_type);
-  return *extractor;
+const DWARFDataExtractor &
+DWARFContext::LoadOrGetSection(SectionType main_section_type,
+   llvm::Optional dwo_section_type,
+   SectionData &data) {
+  llvm::call_once(data.flag, [&] {
+if (dwo_section_type && isDwo())
+  data.data = LoadSection(m_dwo_section_list, *dwo_section_type);
+else
+  data.data = LoadSection(m_main_section_list, main_section_type);
+  });
+  return data.data;
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadAbbrevData() {
-  if (isDwo())
-return LoadOrGetSection(m_dwo_section_list, eSectionTypeDWARFDebugAbbrevDwo,
-m_data_debug_abbrev);
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugAbbrev,
-  m_data_debug_abbrev);
+  return LoadOrGetSection(eSectionTypeDWARFDebugAbbrev,
+  eSectionTypeDWARFDebugAbbrevDwo, m_data_debug_abbrev);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadArangesData() {
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugAranges,
+  return LoadOrGetSection(eSectionTypeDWARFDebugAranges, llvm::None,
   m_data_debug_aranges);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadAddrData() {
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugAddr,
+  return LoadOrGetSection(eSectionTypeDWARFDebugAddr, llvm::None,
   m_data_debug_addr);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadDebugInfoData() {
-  if (isDwo())
-return LoadOrGetSection(m_dwo_section_list, eSectionTypeDWARFDebugInfoDwo,
-m_data_debug_info);
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugInfo,
-  m_data_debug_info);
+  return LoadOrGetSection(eSectionTypeDWARFDebugInfo,
+  eSectionTypeDWARFDebugInfoDwo, m_data_debug_info);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadLineData() {
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugLine,
+  return LoadOrGetSection(eSectionTypeDWARFDebugLine, llvm::None,
  

[Lldb-commits] [lldb] r361603 - DWARF: Implement DW_AT_signature lookup for type unit support

[Pavel Labath via lldb-commits]

Author: labath
Date: Fri May 24 01:11:12 2019
New Revision: 361603

URL: http://llvm.org/viewvc/llvm-project?rev=361603&view=rev
Log:
DWARF: Implement DW_AT_signature lookup for type unit support

Summary:
This patch implements the main feature of type units. When completing a
type, if we encounter a DW_AT_signature attribute, we use it's value to
lookup the complete definition of the type in the relevant type unit.

To enable this lookup, we build up a map of all type units in a symbol
file when parsing the units. Then we consult this map when resolving the
DW_AT_signature attribute.

I include add a couple of tests which exercise the type lookup feature,
including one that ensure we do something reasonable in case we fail to
lookup the type.

A lot of the ideas in this patch have been taken from D32167 and D61505.

Reviewers: clayborg, JDevlieghere, aprantl, alexshap

Subscribers: mgrang, lldb-commits

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

Added:
lldb/trunk/lit/SymbolFile/DWARF/debug-types-missing-signature.test
lldb/trunk/lit/SymbolFile/DWARF/debug-types-signature-loop.s
Modified:
lldb/trunk/lit/SymbolFile/DWARF/Inputs/debug-types-basic.cpp
lldb/trunk/lit/SymbolFile/DWARF/Inputs/debug-types-expressions.cpp
lldb/trunk/lit/SymbolFile/DWARF/debug-types-basic.test
lldb/trunk/lit/SymbolFile/DWARF/debug-types-expressions.test
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h

Modified: lldb/trunk/lit/SymbolFile/DWARF/Inputs/debug-types-basic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/Inputs/debug-types-basic.cpp?rev=361603&r1=361602&r2=361603&view=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/Inputs/debug-types-basic.cpp (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/Inputs/debug-types-basic.cpp Fri May 24 
01:11:12 2019
@@ -1,13 +1,15 @@
+enum E { e1, e2, e3 };
+enum class EC { e1, e2, e3 };
+
 struct A {
   int i;
   long l;
   float f;
   double d;
+  E e;
+  EC ec;
 };
 
-enum E { e1, e2, e3 };
-enum class EC { e1, e2, e3 };
-
-extern constexpr A a{42, 47l, 4.2f, 4.7};
+extern constexpr A a{42, 47l, 4.2f, 4.7, e1, EC::e3};
 extern constexpr E e(e2);
 extern constexpr EC ec(EC::e2);

Modified: lldb/trunk/lit/SymbolFile/DWARF/Inputs/debug-types-expressions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/Inputs/debug-types-expressions.cpp?rev=361603&r1=361602&r2=361603&view=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/Inputs/debug-types-expressions.cpp 
(original)
+++ lldb/trunk/lit/SymbolFile/DWARF/Inputs/debug-types-expressions.cpp Fri May 
24 01:11:12 2019
@@ -11,11 +11,19 @@ struct B: public A {
 namespace ns {
 struct A {
   int i = 147;
+  ::A getA();
   A();
 };
 A::A() = default;
+
+::A A::getA() {
+  ::A a;
+  a.i = i - 1;
+  return a;
 }
 
+} // namespace ns
+
 int foo(A *a) {
   return a->f();
 }

Modified: lldb/trunk/lit/SymbolFile/DWARF/debug-types-basic.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/debug-types-basic.test?rev=361603&r1=361602&r2=361603&view=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/debug-types-basic.test (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/debug-types-basic.test Fri May 24 01:11:12 
2019
@@ -19,6 +19,8 @@ type lookup A
 # CHECK-NEXT:   long l;
 # CHECK-NEXT:   float f;
 # CHECK-NEXT:   double d;
+# CHECK-NEXT:   E e;
+# CHECK-NEXT:   EC ec;
 # CHECK-NEXT: }
 
 type lookup E
@@ -44,3 +46,9 @@ print (E) 1
 print (EC) 1
 # CHECK-LABEL: print (EC) 1
 # CHECK: (EC) $1 = e2
+
+target variable a e ec
+# CHECK-LABEL: target variable a e ec
+# CHECK: (const A) a = (i = 42, l = 47, f = 4.{{[12].*}}, d = 4.{{[67].*}}, e 
= e1, ec = e3)
+# CHECK: (const E) e = e2
+# CHECK: (const EC) ec = e2

Modified: lldb/trunk/lit/SymbolFile/DWARF/debug-types-expressions.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/debug-types-expressions.test?rev=361603&r1=361602&r2=361603&view=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/debug-types-expressions.test (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/debug-types-expressions.test Fri May 24 
01:11:12 2019
@@ -19,6 +19,13 @@ frame variable a
 # CHECK-LABEL: frame variable a
 # CHECK: (B *) a =
 
+frame variable *a
+# CHECK-LA

[Lldb-commits] [PATCH] D62246: DWARF: Implement DW_AT_signature lookup for type unit support

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361603: DWARF: Implement DW_AT_signature lookup for type 
unit support (authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62246/new/

https://reviews.llvm.org/D62246

Files:
  lldb/trunk/lit/SymbolFile/DWARF/Inputs/debug-types-basic.cpp
  lldb/trunk/lit/SymbolFile/DWARF/Inputs/debug-types-expressions.cpp
  lldb/trunk/lit/SymbolFile/DWARF/debug-types-basic.test
  lldb/trunk/lit/SymbolFile/DWARF/debug-types-expressions.test
  lldb/trunk/lit/SymbolFile/DWARF/debug-types-missing-signature.test
  lldb/trunk/lit/SymbolFile/DWARF/debug-types-signature-loop.s
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h

Index: lldb/trunk/lit/SymbolFile/DWARF/debug-types-signature-loop.s
===
--- lldb/trunk/lit/SymbolFile/DWARF/debug-types-signature-loop.s
+++ lldb/trunk/lit/SymbolFile/DWARF/debug-types-signature-loop.s
@@ -0,0 +1,115 @@
+# REQUIRES: lld
+
+# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux -o %t.o %s
+# RUN: ld.lld %t.o -o %t
+# RUN: %lldb %t -o "target variable e" -b | FileCheck %s
+
+# CHECK: e = 
+
+.type   e,@object   # @e
+.section.rodata,"a",@progbits
+.globl  e
+.p2align2
+e:
+.long   0   # 0x0
+.size   e, 4
+
+.Lstr_offsets_base0:
+.section.debug_str,"MS",@progbits,1
+.Linfo_string0:
+.asciz  "Hand-written DWARF"
+.Linfo_string1:
+.asciz  "a.cpp"
+.Linfo_string3:
+.asciz  "e"   
+.Linfo_string4:
+.asciz  "unsigned int"
+.Linfo_string5:
+.asciz  "e1"  
+.Linfo_string6:
+.asciz  "E"   
+
+.section.debug_abbrev,"",@progbits
+.byte   1   # Abbreviation Code
+.byte   65  # DW_TAG_type_unit
+.byte   1   # DW_CHILDREN_yes
+.byte   19  # DW_AT_language
+.byte   5   # DW_FORM_data2
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   5   # Abbreviation Code
+.byte   17  # DW_TAG_compile_unit
+.byte   1   # DW_CHILDREN_yes
+.byte   37  # DW_AT_producer
+.byte   14  # DW_FORM_strp
+.byte   19  # DW_AT_language
+.byte   5   # DW_FORM_data2
+.byte   3   # DW_AT_name
+.byte   14  # DW_FORM_strp
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   6   # Abbreviation Code
+.byte   52  # DW_TAG_variable
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   14  # DW_FORM_strp
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   2   # DW_AT_location
+.byte   24  # DW_FORM_exprloc
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   8   # Abbreviation Code
+.byte   4   # DW_TAG_enumeration_type
+.byte   0   # DW_CHILDREN_no
+.byte   60  # DW_AT_declaration
+.byte   25  # DW_FORM_flag_present
+.byte   105 # DW_AT_signature
+.byte   32  # DW_FORM_ref_sig8
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   0   # EOM(3)
+
+.section.debug_info,"",@progbits
+.Ltu_begin0:
+.long   .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+.short  5   # DWARF version number
+.byte   2   # DWARF Unit Type
+.byte   8   # Address Size (in bytes)
+.long   .debug_abbrev  

[Lldb-commits] [PATCH] D62352: Call to HandleNameConflict in VisitRecordDecl mistakeningly using Name instead of SearchName

[Gabor Marton via Phabricator via lldb-commits]

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

Looks good, thank you!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62352/new/

https://reviews.llvm.org/D62352



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

[Lldb-commits] [lldb] r361618 - [FuncUnwinders] Use "symbol file" unwind plans for unwinding

[Pavel Labath via lldb-commits]

Author: labath
Date: Fri May 24 02:54:39 2019
New Revision: 361618

URL: http://llvm.org/viewvc/llvm-project?rev=361618&view=rev
Log:
[FuncUnwinders] Use "symbol file" unwind plans for unwinding

Summary:
Previous patch (r360409) introduced the "symbol file unwind plan"
concept, but that plan wasn't used for unwinding yet. With this patch,
we start to consider the new plan as a possible strategy for both
synchronous and asynchronous unwinding. I also add a test that asserts
that unwinding via breakpad STACK CFI info works end-to-end.

Reviewers: jasonmolenda, clayborg

Subscribers: lldb-commits, amccarth, markmentovai

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

Added:
lldb/trunk/lit/SymbolFile/Breakpad/Inputs/unwind-via-stack-cfi.syms
lldb/trunk/lit/SymbolFile/Breakpad/Inputs/unwind-via-stack-cfi.yaml
lldb/trunk/lit/SymbolFile/Breakpad/unwind-via-stack-cfi.test
Modified:
lldb/trunk/include/lldb/Symbol/FuncUnwinders.h
lldb/trunk/lit/SymbolFile/Breakpad/stack-cfi-parsing.test
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
lldb/trunk/source/Symbol/FuncUnwinders.cpp

Modified: lldb/trunk/include/lldb/Symbol/FuncUnwinders.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/FuncUnwinders.h?rev=361618&r1=361617&r2=361618&view=diff
==
--- lldb/trunk/include/lldb/Symbol/FuncUnwinders.h (original)
+++ lldb/trunk/include/lldb/Symbol/FuncUnwinders.h Fri May 24 02:54:39 2019
@@ -35,7 +35,7 @@ public:
 
   ~FuncUnwinders();
 
-  lldb::UnwindPlanSP GetUnwindPlanAtCallSite(Target &target);
+  lldb::UnwindPlanSP GetUnwindPlanAtCallSite(Target &target, Thread &thread);
 
   lldb::UnwindPlanSP GetUnwindPlanAtNonCallSite(Target &target,
 lldb_private::Thread &thread);

Added: lldb/trunk/lit/SymbolFile/Breakpad/Inputs/unwind-via-stack-cfi.syms
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/Breakpad/Inputs/unwind-via-stack-cfi.syms?rev=361618&view=auto
==
--- lldb/trunk/lit/SymbolFile/Breakpad/Inputs/unwind-via-stack-cfi.syms (added)
+++ lldb/trunk/lit/SymbolFile/Breakpad/Inputs/unwind-via-stack-cfi.syms Fri May 
24 02:54:39 2019
@@ -0,0 +1,6 @@
+MODULE Linux x86_64 603FCF6CA7FF4BCC86AE8FF44DB2576A0 linux-x86_64_not_crashed
+INFO CODE_ID 6CCF3F60FFA7CC4B86AE8FF44DB2576A68983611
+PUBLIC 420 0 bar
+PUBLIC 450 0 foo
+PUBLIC 480 0 _start
+STACK CFI INIT 420 29 .cfa: $rbp ^ .ra: .cfa 8 + ^ $rsp: .cfa 16 + $rbp: .cfa ^

Added: lldb/trunk/lit/SymbolFile/Breakpad/Inputs/unwind-via-stack-cfi.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/Breakpad/Inputs/unwind-via-stack-cfi.yaml?rev=361618&view=auto
==
--- lldb/trunk/lit/SymbolFile/Breakpad/Inputs/unwind-via-stack-cfi.yaml (added)
+++ lldb/trunk/lit/SymbolFile/Breakpad/Inputs/unwind-via-stack-cfi.yaml Fri May 
24 02:54:39 2019
@@ -0,0 +1,43 @@
+--- !minidump
+Streams: 
+  - Type:ThreadList
+Threads: 
+  - Thread Id:   0x74F3
+Context: 
0B0010006CAE6B7FC05AC81D415AA2BF9E5A6B7F8850C14BFD7F9850C14BFD7F0100B04AC14BFD7F60812D010800B065E05A6B7F80044000E050C14BFD7F300440007F03801F6B7F0400B84CC14BFD7F304D405A6B7FC84DC14BFD7FC0AA405A6B7F4F033D00B84DC14BFD7FE84DC14BFD7F0070E05A6B7F78629E5A6B7FC81D415A6B7F804F9E5A6B7F0100E6030100E093115A6B7F804EC14BFD7F584EC14BFD7F99ADC05A6B7F0100D77D02000800B065E05A6B7FE6B7C05A6B7F01006B7F884DC14BFD7F106F7C5A6B7F984EC14BFD7F488B7C5A6B7FC4A71CB901000800B065E05A6B7F48B6C05A6B7F702AE25A6B7FD84DC14BFD7F30489E5A6B7FE84EC14BFD7FE05E9E5A6B7F0991F04601000800B065E05A6B7F48B6C05A6B7F0100284EC14BFD7F

[Lldb-commits] [PATCH] D61853: [FuncUnwinders] Use "symbol file" unwind plans for unwinding

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB361618: [FuncUnwinders] Use "symbol file" 
unwind plans for unwinding (authored by labath, committed by ).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D61853?vs=200737&id=201169#toc

Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61853/new/

https://reviews.llvm.org/D61853

Files:
  include/lldb/Symbol/FuncUnwinders.h
  lit/SymbolFile/Breakpad/Inputs/unwind-via-stack-cfi.syms
  lit/SymbolFile/Breakpad/Inputs/unwind-via-stack-cfi.yaml
  lit/SymbolFile/Breakpad/stack-cfi-parsing.test
  lit/SymbolFile/Breakpad/unwind-via-stack-cfi.test
  source/Commands/CommandObjectTarget.cpp
  source/Plugins/Process/Utility/RegisterContextLLDB.cpp
  source/Symbol/FuncUnwinders.cpp

Index: include/lldb/Symbol/FuncUnwinders.h
===
--- include/lldb/Symbol/FuncUnwinders.h
+++ include/lldb/Symbol/FuncUnwinders.h
@@ -35,7 +35,7 @@
 
   ~FuncUnwinders();
 
-  lldb::UnwindPlanSP GetUnwindPlanAtCallSite(Target &target);
+  lldb::UnwindPlanSP GetUnwindPlanAtCallSite(Target &target, Thread &thread);
 
   lldb::UnwindPlanSP GetUnwindPlanAtNonCallSite(Target &target,
 lldb_private::Thread &thread);
Index: source/Plugins/Process/Utility/RegisterContextLLDB.cpp
===
--- source/Plugins/Process/Utility/RegisterContextLLDB.cpp
+++ source/Plugins/Process/Utility/RegisterContextLLDB.cpp
@@ -244,8 +244,8 @@
 }
 
 if (func_unwinders_sp.get() != nullptr)
-  call_site_unwind_plan =
-  func_unwinders_sp->GetUnwindPlanAtCallSite(process->GetTarget());
+  call_site_unwind_plan = func_unwinders_sp->GetUnwindPlanAtCallSite(
+  process->GetTarget(), m_thread);
 
 if (call_site_unwind_plan.get() != nullptr) {
   m_fallback_unwind_plan_sp = call_site_unwind_plan;
@@ -873,7 +873,8 @@
 // location what helps in the most common cases when the instruction
 // emulation fails.
 UnwindPlanSP call_site_unwind_plan =
-func_unwinders_sp->GetUnwindPlanAtCallSite(process->GetTarget());
+func_unwinders_sp->GetUnwindPlanAtCallSite(process->GetTarget(),
+   m_thread);
 if (call_site_unwind_plan &&
 call_site_unwind_plan.get() != unwind_plan_sp.get() &&
 call_site_unwind_plan->GetSourceName() !=
@@ -909,8 +910,8 @@
   // Typically this is unwind info from an eh_frame section intended for
   // exception handling; only valid at call sites
   if (process) {
-unwind_plan_sp =
-func_unwinders_sp->GetUnwindPlanAtCallSite(process->GetTarget());
+unwind_plan_sp = func_unwinders_sp->GetUnwindPlanAtCallSite(
+process->GetTarget(), m_thread);
   }
   int valid_offset = -1;
   if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp, valid_offset)) {
@@ -940,7 +941,8 @@
 // code it is often written in a way that it valid at all location what
 // helps in the most common cases when the instruction emulation fails.
 UnwindPlanSP call_site_unwind_plan =
-func_unwinders_sp->GetUnwindPlanAtCallSite(process->GetTarget());
+func_unwinders_sp->GetUnwindPlanAtCallSite(process->GetTarget(),
+   m_thread);
 if (call_site_unwind_plan &&
 call_site_unwind_plan.get() != unwind_plan_sp.get() &&
 call_site_unwind_plan->GetSourceName() !=
Index: source/Commands/CommandObjectTarget.cpp
===
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -3521,7 +3521,7 @@
 non_callsite_unwind_plan->GetSourceName().AsCString());
   }
   UnwindPlanSP callsite_unwind_plan =
-  func_unwinders_sp->GetUnwindPlanAtCallSite(*target);
+  func_unwinders_sp->GetUnwindPlanAtCallSite(*target, *thread);
   if (callsite_unwind_plan) {
 result.GetOutputStream().Printf(
 "Synchronous (restricted to call-sites) UnwindPlan is '%s'\n",
Index: source/Symbol/FuncUnwinders.cpp
===
--- source/Symbol/FuncUnwinders.cpp
+++ source/Symbol/FuncUnwinders.cpp
@@ -54,9 +54,12 @@
 
 FuncUnwinders::~FuncUnwinders() {}
 
-UnwindPlanSP FuncUnwinders::GetUnwindPlanAtCallSite(Target &target) {
+UnwindPlanSP FuncUnwinders::GetUnwindPlanAtCallSite(Target &target,
+Thread &thread) {
   std::lock_guard guard(m_mutex);
 
+  if (UnwindPlanSP plan_sp = GetSymbolFileUnwindPlan(thread))
+return plan_sp;
   if (UnwindPlanSP plan_sp = GetEHFrameUnwindPlan(target))
 return plan_sp;
   if (UnwindPlanSP plan_sp = GetDebugFrameUnwindPlan(target))
@@ -357,6 +360,8 @@
 

[Lldb-commits] [PATCH] D62374: FuncUnwinders: prefer debug_frame over eh_frame

[Pavel Labath via Phabricator via lldb-commits]

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

The two sections usually contain the same information, and we rarely
have both kinds of entries for a single function. However, in theory the
debug_frame plan can be more complete, whereas eh_frame is only required
to be correct at places where exceptions can be thrown.


https://reviews.llvm.org/D62374

Files:
  lit/Unwind/Inputs/prefer-debug-over-eh-frame.s
  lit/Unwind/prefer-debug-over-eh-frame.test
  source/Symbol/FuncUnwinders.cpp


Index: source/Symbol/FuncUnwinders.cpp
===
--- source/Symbol/FuncUnwinders.cpp
+++ source/Symbol/FuncUnwinders.cpp
@@ -60,10 +60,10 @@
 
   if (UnwindPlanSP plan_sp = GetSymbolFileUnwindPlan(thread))
 return plan_sp;
-  if (UnwindPlanSP plan_sp = GetEHFrameUnwindPlan(target))
-return plan_sp;
   if (UnwindPlanSP plan_sp = GetDebugFrameUnwindPlan(target))
 return plan_sp;
+  if (UnwindPlanSP plan_sp = GetEHFrameUnwindPlan(target))
+return plan_sp;
   if (UnwindPlanSP plan_sp = GetCompactUnwindUnwindPlan(target))
 return plan_sp;
   if (UnwindPlanSP plan_sp = GetArmUnwindUnwindPlan(target))
@@ -323,10 +323,10 @@
 
   if (UnwindPlanSP plan_sp = GetSymbolFileUnwindPlan(thread))
 return plan_sp;
-  if (UnwindPlanSP plan_sp = GetEHFrameAugmentedUnwindPlan(target, thread))
-return plan_sp;
   if (UnwindPlanSP plan_sp = GetDebugFrameAugmentedUnwindPlan(target, thread))
 return plan_sp;
+  if (UnwindPlanSP plan_sp = GetEHFrameAugmentedUnwindPlan(target, thread))
+return plan_sp;
 
   return assembly_sp;
 }
Index: lit/Unwind/prefer-debug-over-eh-frame.test
===
--- /dev/null
+++ lit/Unwind/prefer-debug-over-eh-frame.test
@@ -0,0 +1,21 @@
+# Test that we prefer debug_frame over eh_frame unwind plans. They usually
+# contain the same information, and we rarely have both kinds of entries for a
+# single function. However, in theory the debug_frame plan can be more 
complete,
+# whereas eh_frame is only required to be correct at places where exceptions 
can
+# be thrown.
+
+# UNSUPPORTED: system-windows
+# REQUIRES: target-x86_64, native
+
+# RUN: %clang %p/Inputs/call-asm.c %p/Inputs/prefer-debug-over-eh-frame.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+breakpoint set -n bar
+# CHECK: Breakpoint 1: where = {{.*}}`bar
+
+process launch
+# CHECK: stop reason = breakpoint 1.1
+
+target modules show-unwind -n foo
+# CHECK: Asynchronous (not restricted to call-sites) UnwindPlan is 'DWARF CFI 
plus augmentation from assembly parsing'
+# CHECK: Synchronous (restricted to call-sites) UnwindPlan is 'DWARF CFI'
Index: lit/Unwind/Inputs/prefer-debug-over-eh-frame.s
===
--- /dev/null
+++ lit/Unwind/Inputs/prefer-debug-over-eh-frame.s
@@ -0,0 +1,38 @@
+.cfi_sections .eh_frame, .debug_frame
+.text
+.globl  bar
+bar:
+.cfi_startproc
+leal(%edi, %edi), %eax
+ret
+.cfi_endproc
+
+.globl  foo
+foo:
+.cfi_startproc
+pushq   %rbp
+.cfi_def_cfa_offset 16
+.cfi_offset %rbp, -16
+movq%rsp, %rbp
+.cfi_def_cfa_register %rbp
+callbar
+addl$1, %eax
+popq%rbp
+ret
+.cfi_endproc
+
+.globl  asm_main
+asm_main:
+.cfi_startproc
+pushq   %rbp
+.cfi_def_cfa_offset 16
+.cfi_offset 6, -16
+movq%rsp, %rbp
+.cfi_def_cfa_register 6
+movl$47, %edi
+
+call foo
+popq%rbp
+.cfi_def_cfa 7, 8
+ret
+.cfi_endproc


Index: source/Symbol/FuncUnwinders.cpp
===
--- source/Symbol/FuncUnwinders.cpp
+++ source/Symbol/FuncUnwinders.cpp
@@ -60,10 +60,10 @@
 
   if (UnwindPlanSP plan_sp = GetSymbolFileUnwindPlan(thread))
 return plan_sp;
-  if (UnwindPlanSP plan_sp = GetEHFrameUnwindPlan(target))
-return plan_sp;
   if (UnwindPlanSP plan_sp = GetDebugFrameUnwindPlan(target))
 return plan_sp;
+  if (UnwindPlanSP plan_sp = GetEHFrameUnwindPlan(target))
+return plan_sp;
   if (UnwindPlanSP plan_sp = GetCompactUnwindUnwindPlan(target))
 return plan_sp;
   if (UnwindPlanSP plan_sp = GetArmUnwindUnwindPlan(target))
@@ -323,10 +323,10 @@
 
   if (UnwindPlanSP plan_sp = GetSymbolFileUnwindPlan(thread))
 return plan_sp;
-  if (UnwindPlanSP plan_sp = GetEHFrameAugmentedUnwindPlan(target, thread))
-return plan_sp;
   if (UnwindPlanSP plan_sp = GetDebugFrameAugmentedUnwindPlan(target, thread))
 return plan_sp;
+  if (UnwindPlanSP plan_sp = GetEHFrameAugmentedUnwindPlan(target, thread))
+return plan_sp;
 
   return assembly_sp;
 }
Index: lit/Unwind/prefer-debug-over-eh-frame.test
===
--- /dev/null

[Lldb-commits] [PATCH] D62395: WIP: Dont vend lldb_private::CompileUnits for DWARFTypeUnits

[Pavel Labath via Phabricator via lldb-commits]

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

This patch isn't meant to be committed, but it is here to demostrate
what it would take to implement the idea proposed by Greg in
D62073#1507063 .

The background here is that a type unit build will have substantially
more "units" than a regular one (e.g., a type unit build of lldb will
have about 1k compile units, but almost 400k type units). Vending them
all as lldb_private::CompileUnits is a bit wasteful. It can be made less
wasteful by making sure the CompileUnits share line tables and stuff
(just like DWARF units do), but it's not clear to me whether this is
really useful/needed. For instance, lldb CompileUnits expect to have a
"name" (a file path), but type units don't have that as they're supposed
to be independent/shared between real compile units.

The majority of changes here just deal with the remapping of dwarf vs.
lldb unit indexes, as now we can now no longer use a 1:1 mapping.
However, there are a couple of things that are worth calling out
explicitly:

- DWARFASTParserClang goes through the lldb CompileUnits in order to fetch file 
names. Right now I've just deleted that code. In a proper solution, we'd need 
to find a way to get this information strictly through DWARF structures.
- The main visible side effect I can see from this is that we will stop filling 
out the SymbolContextScope field 
https://lldb.llvm.org/cpp_reference/classlldb__private_1_1Type.html#aea1b2a6336880f67455036803bf42ea1
 of the Type objects that we hand about. More precisely, we can still fill out 
the "module" part, but not the "compile unit". I'm not sure what are the 
implications of that, but maybe that is even more correct as the type unit does 
not really belong to any single compilation. This patch breaks just three 
tests, and all of them seem related to me stopping to parse the file names, and 
not this...

So, does anyone see why (a cleaned up version of) this would be a bad
idea?


https://reviews.llvm.org/D62395

Files:
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -17,8 +17,7 @@
 
   ~SymbolFileDWARFDwo() override = default;
 
-  lldb::CompUnitSP ParseCompileUnit(DWARFUnit *dwarf_cu,
-uint32_t cu_idx) override;
+  lldb::CompUnitSP ParseCompileUnit(DWARFCompileUnit *dwarf_cu) override;
 
   DWARFUnit *GetCompileUnit();
 
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -12,9 +12,11 @@
 #include "lldb/Expression/DWARFExpression.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/LLDBAssert.h"
+#include "llvm/Support/Casting.h"
 
-#include "DWARFUnit.h"
+#include "DWARFCompileUnit.h"
 #include "DWARFDebugInfo.h"
+#include "DWARFUnit.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -46,12 +48,12 @@
 }
 
 lldb::CompUnitSP
-SymbolFileDWARFDwo::ParseCompileUnit(DWARFUnit *dwarf_cu,
- uint32_t cu_idx) {
+SymbolFileDWARFDwo::ParseCompileUnit(DWARFCompileUnit *dwarf_cu) {
   assert(GetCompileUnit() == dwarf_cu && "SymbolFileDWARFDwo::ParseCompileUnit "
  "called with incompatible compile "
  "unit");
-  return GetBaseSymbolFile()->ParseCompileUnit(m_base_dwarf_cu, UINT32_MAX);
+  return GetBaseSymbolFile()->ParseCompileUnit(
+  llvm::cast(m_base_dwarf_cu));
 }
 
 DWARFUnit *SymbolFileDWARFDwo::GetCompileUnit() {
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -49,6 +49,7 @@
 class DWARFDebugRangesBase;
 class DWARFDeclContext;
 class DWARFFormValue;
+class DWARFCompileUnit;
 class SymbolFileDWARFDebugMap;
 class SymbolFileDWARFDwo;
 class SymbolFileDWARFDwp;
@@ -252,8 +253,7 @@
 
   static DWARFDIE GetParentSymbolContextDIE(const DWARFDIE &die);
 
-  virtual lldb::CompUnitSP ParseCompileUnit(DWARFUnit *dwarf_cu,
-uint

[Lldb-commits] [PATCH] D62352: Call to HandleNameConflict in VisitRecordDecl mistakeningly using Name instead of SearchName

[Davide Italiano via Phabricator via lldb-commits]

davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.

Do you have a testcase?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62352/new/

https://reviews.llvm.org/D62352



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

[Lldb-commits] [PATCH] D62352: Call to HandleNameConflict in VisitRecordDecl mistakeningly using Name instead of SearchName

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@davide We have a reproducer but so far it has proven difficult to narrow it 
down to a test case. This is serious regression b/c this leads to fields being 
dropped in records during expression evaluation leading to results that looks 
like a bug but are actually expression parsing issues.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62352/new/

https://reviews.llvm.org/D62352



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

[Lldb-commits] [PATCH] D62352: Call to HandleNameConflict in VisitRecordDecl mistakeningly using Name instead of SearchName

[Davide Italiano via Phabricator via lldb-commits]

davide resigned from this revision.
davide added a comment.
This revision is now accepted and ready to land.

I don't think I'm qualified enough to comment further on this patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62352/new/

https://reviews.llvm.org/D62352



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

[Lldb-commits] [PATCH] D62352: Call to HandleNameConflict in VisitRecordDecl mistakeningly using Name instead of SearchName

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

To elaborate further, I'm not familiar with this code so I really shouldn't be 
the person to approve the patch.  On the other hand I'm somewhat nervous as 
refactors in this area of the codebase caused bugs/crashes in lldb, 
fundamentally due to changes in behavior previously left untested.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62352/new/

https://reviews.llvm.org/D62352



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

[Lldb-commits] [PATCH] D62352: Call to HandleNameConflict in VisitRecordDecl mistakeningly using Name instead of SearchName

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361650: [ASTImporter] Call to HandleNameConflict in 
VisitRecordDecl mistakeningly using… (authored by shafik, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62352?vs=201085&id=201278#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62352/new/

https://reviews.llvm.org/D62352

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -2585,7 +2585,7 @@
 } // for
 
 if (!ConflictingDecls.empty() && SearchName) {
-  Name = Importer.HandleNameConflict(Name, DC, IDNS,
+  Name = Importer.HandleNameConflict(SearchName, DC, IDNS,
  ConflictingDecls.data(),
  ConflictingDecls.size());
   if (!Name)


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -2585,7 +2585,7 @@
 } // for
 
 if (!ConflictingDecls.empty() && SearchName) {
-  Name = Importer.HandleNameConflict(Name, DC, IDNS,
+  Name = Importer.HandleNameConflict(SearchName, DC, IDNS,
  ConflictingDecls.data(),
  ConflictingDecls.size());
   if (!Name)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62395: WIP: Dont vend lldb_private::CompileUnits for DWARFTypeUnits

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:318-319
   case DW_AT_decl_file:
-decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-form_value.Unsigned()));
 break;

We should still be able to fill this in. We just need to unique all of the 
DWARF line tables internally and then access the line table using the DWARF 
compile/type unit, and not the lldb_private::CompileUnit



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:470-471
   bool translation_unit_is_objc =
-  (sc.comp_unit->GetLanguage() == eLanguageTypeObjC ||
-   sc.comp_unit->GetLanguage() == eLanguageTypeObjC_plus_plus);
+  (die.GetLanguage() == eLanguageTypeObjC ||
+   die.GetLanguage() == eLanguageTypeObjC_plus_plus);
 

Cache this in local. Kind of expensive to get twice from the DIE since it will 
have to grab the DW_AT_language attribute from the DIE and not just use a 
cached version? Unless this just grabs it from the DWARFUnit underneath? And 
actually a DIE's language could differ from the CU/TU language. We might have a 
C type in C++, so it is theoretically possible to have a die with a 
DW_AT_language attribute? If "die.GetLanguage()" is just using the language of 
the DWARFUnit, then this is fine, otherwise we should explicitly use the 
DWARFUnit language.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:567-568
   case DW_AT_decl_file:
-decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-form_value.Unsigned()));
 break;

Again, we can still fill this in correctly, just don't use the 
lldb_private::CompileUnit, grab the uniqued line table from DWARFContext or 
SymbolFileDWARF. We need to cache line tables anyway because many type units 
re-use existing compile unit line tables and we don't want to re-parse line 
table for each TU.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:667
 if (byte_size && *byte_size == 0 && type_name_cstr && !die.HasChildren() &&
-sc.comp_unit->GetLanguage() == eLanguageTypeObjC) {
+die.GetLanguage() == eLanguageTypeObjC) {
   // Work around an issue with clang at the moment where forward

Again, do we want CU/TU language or DIE language?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:998-999
   case DW_AT_decl_file:
-decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-form_value.Unsigned()));
 break;

fill in using uniqued DWARF line tables



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1169-1170
   case DW_AT_decl_file:
-decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-form_value.Unsigned()));
 break;

fill in using uniqued DWARF line tables



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1677-1678
   case DW_AT_decl_file:
-decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-form_value.Unsigned()));
 break;

fill in using uniqued DWARF line tables



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2471-2472
 case DW_AT_decl_file:
-  decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-  form_value.Unsigned()));
   break;

fill in using uniqued DWARF line tables



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2720-2721
 case DW_AT_decl_file:
-  decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-  form_value.Unsigned()));
   break;

fill in using uniqued DWARF line tables



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3188-3189
 case DW_AT_decl_file:
-  decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-  form_value.Unsigned()));
   break;

fill in using uniqued DWARF line tables



Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:22
+  static bool classof(const DWARFUnit *unit) {
+return unit->GetUnitType() != DW_UT_type;
+  }

What about DW_UT_split_type?



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:715
 
+void SymbolFileDWARF::BuildLldbCuTranslationTable() {
+  if (!m_lldb_cu_to_dwarf_unit.empty())

BuildCUTranslationTable? Not sure we need the "Lldb" in the name?


CH

[Lldb-commits] [PATCH] D62416: Cleanup fixed form sizes.

[Greg Clayton via Phabricator via lldb-commits]

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

The fix form sizes use to have two arrays: one for 4 byte addresses and in for 
8 byte addresses. The table had an issue where DW_FORM_flag_present wasn't 
being represented as a fixed size form because its actual size _is_ zero and 
zero was used to indicate the form isn't fixed in size. Any code that needed to 
quickly access the DWARF had to get a FixedFormSizes instance using the address 
byte size.

This fix cleans things up by adding a DWARFFormValue::GetFixedSize() both as a 
static method and as a member function on DWARFFormValue. It correctly can 
indicate if a form size is zero. This cleanup is a precursor to a follow up 
patch where I hope to speed up DWARF parsing.

I verified performance doesn't regress by loading hundreds of DWARF files and 
setting a breakpoint by file and line and by name in files that do not have 
DWARF indexes. Performance remained consistent between the two approaches.


https://reviews.llvm.org/D62416

Files:
  source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.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
@@ -3166,13 +3166,12 @@
 block_length);
   } else if (DWARFFormValue::IsDataForm(form_value.Form())) {
 // Retrieve the value as a data expression.
-DWARFFormValue::FixedFormSizes fixed_form_sizes =
-DWARFFormValue::GetFixedFormSizesForAddressSize(
-attributes.CompileUnitAtIndex(i)->GetAddressByteSize());
 uint32_t data_offset = attributes.DIEOffsetAtIndex(i);
-uint32_t data_length =
-fixed_form_sizes.GetSize(form_value.Form());
-if (data_length == 0) {
+auto data_length = form_value.GetFixedSize();
+if (data_length)
+  location.CopyOpcodeData(module, debug_info_data, data_offset,
+  *data_length);
+else {
   const uint8_t *data_pointer = form_value.BlockData();
   if (data_pointer) {
 form_value.Unsigned();
@@ -3181,21 +3180,14 @@
 // create the variable
 const_value = form_value;
   }
-} else
-  location.CopyOpcodeData(module, debug_info_data, data_offset,
-  data_length);
+}
   } else {
 // Retrieve the value as a string expression.
 if (form_value.Form() == DW_FORM_strp) {
-  DWARFFormValue::FixedFormSizes fixed_form_sizes =
-  DWARFFormValue::GetFixedFormSizesForAddressSize(
-  attributes.CompileUnitAtIndex(i)
-  ->GetAddressByteSize());
   uint32_t data_offset = attributes.DIEOffsetAtIndex(i);
-  uint32_t data_length =
-  fixed_form_sizes.GetSize(form_value.Form());
+  auto data_length = form_value.GetFixedSize();
   location.CopyOpcodeData(module, debug_info_data, data_offset,
-  data_length);
+  *data_length);
 } else {
   const char *str = form_value.AsCString();
   uint32_t string_offset =
Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -59,7 +59,6 @@
 
   static void
   IndexUnitImpl(DWARFUnit &unit, const lldb::LanguageType cu_language,
-const DWARFFormValue::FixedFormSizes &fixed_form_sizes,
 const dw_offset_t cu_offset, IndexSet &set);
 
   /// Non-null value means we haven't built the index yet.
Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

[Lldb-commits] [PATCH] D62416: Cleanup fixed form sizes.

[Greg Clayton via Phabricator via lldb-commits]

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

Removed unused function added to DWARFFormValue


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62416/new/

https://reviews.llvm.org/D62416

Files:
  source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.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
@@ -3166,13 +3166,12 @@
 block_length);
   } else if (DWARFFormValue::IsDataForm(form_value.Form())) {
 // Retrieve the value as a data expression.
-DWARFFormValue::FixedFormSizes fixed_form_sizes =
-DWARFFormValue::GetFixedFormSizesForAddressSize(
-attributes.CompileUnitAtIndex(i)->GetAddressByteSize());
 uint32_t data_offset = attributes.DIEOffsetAtIndex(i);
-uint32_t data_length =
-fixed_form_sizes.GetSize(form_value.Form());
-if (data_length == 0) {
+auto data_length = form_value.GetFixedSize();
+if (data_length)
+  location.CopyOpcodeData(module, debug_info_data, data_offset,
+  *data_length);
+else {
   const uint8_t *data_pointer = form_value.BlockData();
   if (data_pointer) {
 form_value.Unsigned();
@@ -3181,21 +3180,14 @@
 // create the variable
 const_value = form_value;
   }
-} else
-  location.CopyOpcodeData(module, debug_info_data, data_offset,
-  data_length);
+}
   } else {
 // Retrieve the value as a string expression.
 if (form_value.Form() == DW_FORM_strp) {
-  DWARFFormValue::FixedFormSizes fixed_form_sizes =
-  DWARFFormValue::GetFixedFormSizesForAddressSize(
-  attributes.CompileUnitAtIndex(i)
-  ->GetAddressByteSize());
   uint32_t data_offset = attributes.DIEOffsetAtIndex(i);
-  uint32_t data_length =
-  fixed_form_sizes.GetSize(form_value.Form());
+  auto data_length = form_value.GetFixedSize();
   location.CopyOpcodeData(module, debug_info_data, data_offset,
-  data_length);
+  *data_length);
 } else {
   const char *str = form_value.AsCString();
   uint32_t string_offset =
Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -59,7 +59,6 @@
 
   static void
   IndexUnitImpl(DWARFUnit &unit, const lldb::LanguageType cu_language,
-const DWARFFormValue::FixedFormSizes &fixed_form_sizes,
 const dw_offset_t cu_offset, IndexSet &set);
 
   /// Non-null value means we haven't built the index yet.
Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -100,20 +100,18 @@
   }
 
   const LanguageType cu_language = unit.GetLanguageType();
-  DWARFFormValue::FixedFormSizes fixed_form_sizes = unit.GetFixedFormSizes();
 
-  IndexUnitImpl(unit, cu_language, fixed_form_sizes, unit.GetOffset(), set);
+  IndexUnitImpl(unit, cu_language, unit.GetOffset(), set);
 
   SymbolFileDWARFDwo *dwo_symbol_file = unit.GetDwoSymbolFile();
   if (dwo_symbol_file && dwo_symbol_file->GetCompileUnit()) {
 IndexUnitImpl(*dwo_symbol_file->GetCompileUnit(), cu_language,
-  fixed_form_sizes, unit.GetOffset(), set);
+  unit.GetOffset(), set);
   }
 }
 
 void ManualDWARFIndex::IndexUnitImpl(
 DWARFUnit &unit, const LanguageType cu_language,
-const DWARFFormValue::FixedFormSizes &fixed_form_sizes,
 const dw_offset_t cu_offset, IndexSet &set) {
   for (const DWARFDebugInfoEnt

[Lldb-commits] [lldb] r361666 - [Process] Clean up some logic around LanguageRuntimes

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Fri May 24 12:39:50 2019
New Revision: 361666

URL: http://llvm.org/viewvc/llvm-project?rev=361666&view=rev
Log:
[Process] Clean up some logic around LanguageRuntimes

Modified:
lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/source/Target/Process.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=361666&r1=361665&r2=361666&view=diff
==
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Fri May 24 12:39:50 2019
@@ -1569,19 +1569,22 @@ CPPLanguageRuntime *Process::GetCPPLangu
   std::lock_guard guard(m_language_runtimes_mutex);
   LanguageRuntime *runtime =
   GetLanguageRuntime(eLanguageTypeC_plus_plus, retry_if_null);
-  if (runtime != nullptr &&
-  runtime->GetLanguageType() == eLanguageTypeC_plus_plus)
-return static_cast(runtime);
-  return nullptr;
+  if (!runtime)
+return nullptr;
+
+  assert(runtime->GetLanguageType() == eLanguageTypeC_plus_plus);
+  return static_cast(runtime);
 }
 
 ObjCLanguageRuntime *Process::GetObjCLanguageRuntime(bool retry_if_null) {
   std::lock_guard guard(m_language_runtimes_mutex);
   LanguageRuntime *runtime =
   GetLanguageRuntime(eLanguageTypeObjC, retry_if_null);
-  if (runtime != nullptr && runtime->GetLanguageType() == eLanguageTypeObjC)
-return static_cast(runtime);
-  return nullptr;
+  if (!runtime)
+return nullptr;
+
+  assert(runtime->GetLanguageType() == eLanguageTypeObjC);
+  return static_cast(runtime);
 }
 
 bool Process::IsPossibleDynamicValue(ValueObject &in_value) {


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

[Lldb-commits] [PATCH] D62416: Cleanup fixed form sizes.

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Looks like a good change. Small comment inline but otherwise LGTM.




Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3170
 uint32_t data_offset = attributes.DIEOffsetAtIndex(i);
-uint32_t data_length =
-fixed_form_sizes.GetSize(form_value.Form());
-if (data_length == 0) {
+auto data_length = form_value.GetFixedSize();
+if (data_length)

```
if (auto data_length = form_value.GetFixedSize()) 
...
```



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3188
   uint32_t data_offset = attributes.DIEOffsetAtIndex(i);
-  uint32_t data_length =
-  fixed_form_sizes.GetSize(form_value.Form());
+  auto data_length = form_value.GetFixedSize();
   location.CopyOpcodeData(module, debug_info_data, data_offset,

Same as previous comment. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62416/new/

https://reviews.llvm.org/D62416



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

[Lldb-commits] [PATCH] D62425: [DWARFExpression] Remove ctor that takes just a compile unit.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 201331.
JDevlieghere added a comment.

s/SetOpcodeData/UpdateValue/


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62425/new/

https://reviews.llvm.org/D62425

Files:
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/source/Symbol/Function.cpp

Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -184,7 +184,7 @@
const AddressRange &range)
 : UserID(func_uid), m_comp_unit(comp_unit), m_type_uid(type_uid),
   m_type(type), m_mangled(mangled), m_block(func_uid), m_range(range),
-  m_frame_base(nullptr), m_flags(), m_prologue_byte_size(0) {
+  m_frame_base(), m_flags(), m_prologue_byte_size(0) {
   m_block.SetParentScope(this);
   assert(comp_unit != nullptr);
 }
Index: lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
@@ -69,7 +69,7 @@
   is_constant = true;
 
   if (!module)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   const ArchSpec &architecture = module->GetArchitecture();
   llvm::Triple::ArchType arch_type = architecture.GetMachine();
@@ -77,7 +77,7 @@
   uint32_t address_size = architecture.GetAddressByteSize();
   uint32_t byte_size = architecture.GetDataByteSize();
   if (byte_order == eByteOrderInvalid || address_size == 0)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   RegisterKind register_kind = eRegisterKindDWARF;
   StreamBuffer<32> stream(Stream::eBinary, address_size, byte_order);
@@ -88,13 +88,13 @@
 
 SectionList *section_list = module->GetSectionList();
 if (!section_list)
-  return DWARFExpression(nullptr);
+  return DWARFExpression();
 
 uint32_t section_id = symbol.getAddressSection();
 
 auto section = section_list->FindSectionByID(section_id);
 if (!section)
-  return DWARFExpression(nullptr);
+  return DWARFExpression();
 
 uint32_t offset = symbol.getAddressOffset();
 stream.PutMaxHex64(section->GetFileAddress() + offset, address_size,
@@ -129,7 +129,7 @@
   register_kind = eRegisterKindLLDB;
   reg_num = GetLLDBRegisterNumber(arch_type, reg_id);
   if (reg_num == LLDB_INVALID_REGNUM)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 }
 
 if (reg_num > 31) {
@@ -149,7 +149,7 @@
 register_kind = eRegisterKindLLDB;
 uint32_t reg_num = GetLLDBRegisterNumber(arch_type, symbol.getRegisterId());
 if (reg_num == LLDB_INVALID_REGNUM)
-  return DWARFExpression(nullptr);
+  return DWARFExpression();
 
 if (reg_num > 31) {
   stream.PutHex8(DW_OP_regx);
@@ -168,7 +168,7 @@
 break;
   }
   default:
-return DWARFExpression(nullptr);
+return DWARFExpression();
   }
 
   DataBufferSP buffer =
Index: lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
@@ -111,13 +111,13 @@
   uint32_t address_size = architecture.GetAddressByteSize();
   uint32_t byte_size = architecture.GetDataByteSize();
   if (byte_order == eByteOrderInvalid || address_size == 0)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   RegisterKind register_kind = eRegisterKindDWARF;
   StreamBuffer<32> stream(Stream::eBinary, address_size, byte_order);
 
   if (!writer(stream, register_kind))
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   DataBufferSP buffer =
   std::make_shared(stream.GetData(), stream.GetSize());
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3111,7 +3111,7 @@
   Declaration decl;
   uint32_t i;
   DWARFFormValue type_die_form;
-  DWARFExpression location(die.GetCU());
+  DWARFExpression location;
   bool is_external = false;
   bool is_artificial = false;
   bool location_is_const_value_data = false;
@@ -3162,8 +3162,8 @@
 uint32_t block_offset =
 form_value.BlockData

[Lldb-commits] [PATCH] D62425: [DWARFExpression] Remove ctor that takes just a compile unit.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: aprantl, clayborg, labath.
Herald added a project: LLDB.
JDevlieghere updated this revision to Diff 201331.
JDevlieghere added a comment.

s/SetOpcodeData/UpdateValue/


Like many of our DWARF classes, the DWARFExpression can be initialized in many 
ways. One such way was through a constructor that takes just the compile unit. 
This constructor is used to initialize both empty DWARFExpressions, and 
DWARFExpression that will be populated later. To make the distinction more 
clear, I changed the constructor to a default constructor and updated its call 
sites. Where the DWARFExpression was being populated later, I replaced that 
with a call to the copy assignment constructor.


https://reviews.llvm.org/D62425

Files:
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/source/Symbol/Function.cpp

Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -184,7 +184,7 @@
const AddressRange &range)
 : UserID(func_uid), m_comp_unit(comp_unit), m_type_uid(type_uid),
   m_type(type), m_mangled(mangled), m_block(func_uid), m_range(range),
-  m_frame_base(nullptr), m_flags(), m_prologue_byte_size(0) {
+  m_frame_base(), m_flags(), m_prologue_byte_size(0) {
   m_block.SetParentScope(this);
   assert(comp_unit != nullptr);
 }
Index: lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
@@ -69,7 +69,7 @@
   is_constant = true;
 
   if (!module)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   const ArchSpec &architecture = module->GetArchitecture();
   llvm::Triple::ArchType arch_type = architecture.GetMachine();
@@ -77,7 +77,7 @@
   uint32_t address_size = architecture.GetAddressByteSize();
   uint32_t byte_size = architecture.GetDataByteSize();
   if (byte_order == eByteOrderInvalid || address_size == 0)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   RegisterKind register_kind = eRegisterKindDWARF;
   StreamBuffer<32> stream(Stream::eBinary, address_size, byte_order);
@@ -88,13 +88,13 @@
 
 SectionList *section_list = module->GetSectionList();
 if (!section_list)
-  return DWARFExpression(nullptr);
+  return DWARFExpression();
 
 uint32_t section_id = symbol.getAddressSection();
 
 auto section = section_list->FindSectionByID(section_id);
 if (!section)
-  return DWARFExpression(nullptr);
+  return DWARFExpression();
 
 uint32_t offset = symbol.getAddressOffset();
 stream.PutMaxHex64(section->GetFileAddress() + offset, address_size,
@@ -129,7 +129,7 @@
   register_kind = eRegisterKindLLDB;
   reg_num = GetLLDBRegisterNumber(arch_type, reg_id);
   if (reg_num == LLDB_INVALID_REGNUM)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 }
 
 if (reg_num > 31) {
@@ -149,7 +149,7 @@
 register_kind = eRegisterKindLLDB;
 uint32_t reg_num = GetLLDBRegisterNumber(arch_type, symbol.getRegisterId());
 if (reg_num == LLDB_INVALID_REGNUM)
-  return DWARFExpression(nullptr);
+  return DWARFExpression();
 
 if (reg_num > 31) {
   stream.PutHex8(DW_OP_regx);
@@ -168,7 +168,7 @@
 break;
   }
   default:
-return DWARFExpression(nullptr);
+return DWARFExpression();
   }
 
   DataBufferSP buffer =
Index: lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
@@ -111,13 +111,13 @@
   uint32_t address_size = architecture.GetAddressByteSize();
   uint32_t byte_size = architecture.GetDataByteSize();
   if (byte_order == eByteOrderInvalid || address_size == 0)
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   RegisterKind register_kind = eRegisterKindDWARF;
   StreamBuffer<32> stream(Stream::eBinary, address_size, byte_order);
 
   if (!writer(stream, register_kind))
-return DWARFExpression(nullptr);
+return DWARFExpression();
 
   DataBufferSP buffer =
   std::make_shared(stream.GetData(), stream.GetSize());
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
=

[Lldb-commits] [lldb] r361672 - [DWARFExpression] Remove commented-out code (NFC)

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Fri May 24 14:26:30 2019
New Revision: 361672

URL: http://llvm.org/viewvc/llvm-project?rev=361672&view=rev
Log:
[DWARFExpression] Remove commented-out code (NFC)

Modified:
lldb/trunk/source/Expression/DWARFExpression.cpp

Modified: lldb/trunk/source/Expression/DWARFExpression.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/DWARFExpression.cpp?rev=361672&r1=361671&r2=361672&view=diff
==
--- lldb/trunk/source/Expression/DWARFExpression.cpp (original)
+++ lldb/trunk/source/Expression/DWARFExpression.cpp Fri May 24 14:26:30 2019
@@ -483,23 +483,6 @@ void DWARFExpression::DumpLocation(Strea
 case DW_OP_call_ref: // 0x9a DWARF3 1 4- or 8-byte offset of DIE
   s->Printf("DW_OP_call_ref(0x%8.8" PRIx64 ")", 
m_data.GetAddress(&offset));
   break;
-//  case DW_OP_call_frame_cfa: s << "call_frame_cfa"; break;
-//  // 0x9c DWARF3
-//  case DW_OP_bit_piece: // 0x9d DWARF3 2
-//  s->Printf("DW_OP_bit_piece(0x%x, 0x%x)",
-//  m_data.GetULEB128(&offset), m_data.GetULEB128(&offset));
-//  break;
-//  case DW_OP_lo_user: s->PutCString("DW_OP_lo_user"); break;
-//  // 0xe0
-//  case DW_OP_hi_user: s->PutCString("DW_OP_hi_user"); break;
-//  // 0xff
-//case DW_OP_APPLE_extern:
-//s->Printf("DW_OP_APPLE_extern(%" PRIu64 ")",
-//m_data.GetULEB128(&offset));
-//break;
-//case DW_OP_APPLE_array_ref:
-//s->PutCString("DW_OP_APPLE_array_ref");
-//break;
 case DW_OP_form_tls_address:
   s->PutCString("DW_OP_form_tls_address"); // 0x9b
   break;
@@ -521,62 +504,6 @@ void DWARFExpression::DumpLocation(Strea
 case DW_OP_APPLE_uninit:
   s->PutCString("DW_OP_APPLE_uninit"); // 0xF0
   break;
-  //case DW_OP_APPLE_assign:// 0xF1 - pops value off and
-  //assigns it to second item on stack (2nd item must have
-  //assignable context)
-  //s->PutCString("DW_OP_APPLE_assign");
-  //break;
-  //case DW_OP_APPLE_address_of:// 0xF2 - gets the address of
-  //the top stack item (top item must be a variable, or have
-  //value_type that is an address already)
-  //s->PutCString("DW_OP_APPLE_address_of");
-  //break;
-  //case DW_OP_APPLE_value_of:  // 0xF3 - pops the value off 
the
-  //stack and pushes the value of that object (top item must be a
-  //variable, or expression local)
-  //s->PutCString("DW_OP_APPLE_value_of");
-  //break;
-  //case DW_OP_APPLE_deref_type:// 0xF4 - gets the address of
-  //the top stack item (top item must be a variable, or a clang
-  //type)
-  //s->PutCString("DW_OP_APPLE_deref_type");
-  //break;
-  //case DW_OP_APPLE_expr_local:// 0xF5 - ULEB128 expression
-  //local index
-  //s->Printf("DW_OP_APPLE_expr_local(%" PRIu64 ")",
-  //m_data.GetULEB128(&offset));
-  //break;
-  //case DW_OP_APPLE_constf:// 0xF6 - 1 byte float size,
-  //followed by constant float data
-  //{
-  //uint8_t float_length = m_data.GetU8(&offset);
-  //s->Printf("DW_OP_APPLE_constf(<%u> ", float_length);
-  //m_data.Dump(s, offset, eFormatHex, float_length, 1,
-  //UINT32_MAX, DW_INVALID_ADDRESS, 0, 0);
-  //s->PutChar(')');
-  //// Consume the float data
-  //m_data.GetData(&offset, float_length);
-  //}
-  //break;
-  //case DW_OP_APPLE_scalar_cast:
-  //s->Printf("DW_OP_APPLE_scalar_cast(%s)",
-  //Scalar::GetValueTypeAsCString
-  //((Scalar::Type)m_data.GetU8(&offset)));
-  //break;
-  //case DW_OP_APPLE_clang_cast:
-  //{
-  //clang::Type *clang_type = (clang::Type
-  //*)m_data.GetMaxU64(&offset, sizeof(void*));
-  //s->Printf("DW_OP_APPLE_clang_cast(%p)", clang_type);
-  //}
-  //break;
-  //case DW_OP_APPLE_clear:
-  //s->PutCString("DW_OP_APPLE_clear");
-  //break;
-  //case DW_OP_APPLE_error: // 0xFF - Stops expression
-  //evaluation and returns an error (no args)
-  //s->PutCString("DW_OP_APPLE_error");
-  //break;
 }
   }
 }
@@ -691,52 +618,6 @@ static bool ReadRegisterValueAs

[Lldb-commits] [lldb] r361673 - [Target] Make Processes' GetLanguageRuntime non-virtual

[Alex Langford via lldb-commits]

Author: xiaobai
Date: Fri May 24 14:27:37 2019
New Revision: 361673

URL: http://llvm.org/viewvc/llvm-project?rev=361673&view=rev
Log:
[Target] Make Processes' GetLanguageRuntime non-virtual

Modified:
lldb/trunk/include/lldb/Target/Process.h

Modified: lldb/trunk/include/lldb/Target/Process.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=361673&r1=361672&r2=361673&view=diff
==
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Fri May 24 14:27:37 2019
@@ -2178,13 +2178,12 @@ public:
 
   OperatingSystem *GetOperatingSystem() { return m_os_up.get(); }
 
-  virtual LanguageRuntime *GetLanguageRuntime(lldb::LanguageType language,
-  bool retry_if_null = true);
+  LanguageRuntime *GetLanguageRuntime(lldb::LanguageType language,
+  bool retry_if_null = true);
 
-  virtual CPPLanguageRuntime *GetCPPLanguageRuntime(bool retry_if_null = true);
+  CPPLanguageRuntime *GetCPPLanguageRuntime(bool retry_if_null = true);
 
-  virtual ObjCLanguageRuntime *
-  GetObjCLanguageRuntime(bool retry_if_null = true);
+  ObjCLanguageRuntime *GetObjCLanguageRuntime(bool retry_if_null = true);
 
   bool IsPossibleDynamicValue(ValueObject &in_value);
 


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

[Lldb-commits] [lldb] r361675 - Cleanup fixed form sizes.

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Fri May 24 15:08:50 2019
New Revision: 361675

URL: http://llvm.org/viewvc/llvm-project?rev=361675&view=rev
Log:
Cleanup fixed form sizes.

The fix form sizes use to have two arrays: one for 4 byte addresses and in for 
8 byte addresses. The table had an issue where DW_FORM_flag_present wasn't 
being represented as a fixed size form because its actual size _is_ zero and 
zero was used to indicate the form isn't fixed in size. Any code that needed to 
quickly access the DWARF had to get a FixedFormSizes instance using the address 
byte size.

This fix cleans things up by adding a DWARFFormValue::GetFixedSize() both as a 
static method and as a member function on DWARFFormValue. It correctly can 
indicate if a form size is zero. This cleanup is a precursor to a follow up 
patch where I hope to speed up DWARF parsing.

I verified performance doesn't regress by loading hundreds of DWARF files and 
setting a breakpoint by file and line and by name in files that do not have 
DWARF indexes. Performance remained consistent between the two approaches.


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


Modified:
lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Modified: lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme?rev=361675&r1=361674&r2=361675&view=diff
==
--- lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme (original)
+++ lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme Fri May 
24 15:08:50 2019
@@ -45,7 +45,7 @@
   buildConfiguration = "DebugClang"
   selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
   selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-  launchStyle = "1"
+  launchStyle = "0"
   useCustomWorkingDirectory = "NO"
   ignoresPersistentStateOnLaunch = "NO"
   debugDocumentVersioning = "YES"
@@ -61,6 +61,12 @@
 ReferencedContainer = "container:lldb.xcodeproj">
  
   
+  
+ 
+ 
+  
   
   


Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp?rev=361675&r1=361674&r2=361675&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp Fri May 24 
15:08:50 2019
@@ -136,10 +136,8 @@ bool DWARFBaseDIE::Supports_DW_AT_APPLE_
 
 size_t DWARFBaseDIE::GetAttributes(DWARFAttributes &attributes,
uint32_t depth) const {
-  if (IsValid()) {
-return m_die->GetAttributes(m_cu, m_cu->GetFixedFormSizes(), attributes,
-depth);
-  }
+  if (IsValid())
+return m_die->GetAttributes(m_cu, attributes, depth);
   if (depth == 0)
 attributes.Clear();
   return 0;

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp?rev=361675&r1=361674&r2=361675&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp Fri May 
24 15:08:50 2019
@@ -33,7 +33,6 @@ extern int g_verbose;
 
 bool DWARFDebugInfoEntry::FastExtract(
 const DWARFDataExtractor &debug_info_data, const DWARFUnit *cu,
-const DWARFFormValue::FixedFormSizes &fixed_form_sizes,
 lldb::offset_t *offset_ptr) {
   m_offset = *offset_ptr;
   m_parent_idx = 0;
@@ -69,9 +68,9 @@ bool DWARFDebugInfoEntry::FastExtract(
 for (i = 0; i < numAttributes; ++i) {
   form = abbrevDecl->GetFormByIndexUnchecked(i);
 
-  const uint8_t fixed_skip_size = fixed_form_sizes.GetSize(form);
+  llvm::Optional fixed_skip_size = 
DWARFFormValue::GetFixedSize(form, cu);
   if (fixed_skip_size)
-offset += fixed_skip_size;
+offset += 

[Lldb-commits] [PATCH] D62416: Cleanup fixed form sizes.

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361675: Cleanup fixed form sizes. (authored by gclayton, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62416?vs=201302&id=201345#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62416/new/

https://reviews.llvm.org/D62416

Files:
  lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme
===
--- lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme
+++ lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme
@@ -45,7 +45,7 @@
   buildConfiguration = "DebugClang"
   selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
   selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-  launchStyle = "1"
+  launchStyle = "0"
   useCustomWorkingDirectory = "NO"
   ignoresPersistentStateOnLaunch = "NO"
   debugDocumentVersioning = "YES"
@@ -61,6 +61,12 @@
 ReferencedContainer = "container:lldb.xcodeproj">
  
   
+  
+ 
+ 
+  
   
   

Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
@@ -136,10 +136,8 @@
 
 size_t DWARFBaseDIE::GetAttributes(DWARFAttributes &attributes,
uint32_t depth) const {
-  if (IsValid()) {
-return m_die->GetAttributes(m_cu, m_cu->GetFixedFormSizes(), attributes,
-depth);
-  }
+  if (IsValid())
+return m_die->GetAttributes(m_cu, attributes, depth);
   if (depth == 0)
 attributes.Clear();
   return 0;
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -33,7 +33,6 @@
 
 bool DWARFDebugInfoEntry::FastExtract(
 const DWARFDataExtractor &debug_info_data, const DWARFUnit *cu,
-const DWARFFormValue::FixedFormSizes &fixed_form_sizes,
 lldb::offset_t *offset_ptr) {
   m_offset = *offset_ptr;
   m_parent_idx = 0;
@@ -69,9 +68,9 @@
 for (i = 0; i < numAttributes; ++i) {
   form = abbrevDecl->GetFormByIndexUnchecked(i);
 
-  const uint8_t fixed_skip_size = fixed_form_sizes.GetSize(form);
+  llvm::Optional fixed_skip_size = DWARFFormValue::GetFixedSize(form, cu);
   if (fixed_skip_size)
-offset += fixed_skip_size;
+offset += *fixed_skip_size;
   else {
 bool form_is_indirect = false;
 do {
@@ -723,8 +722,8 @@
 // results. Any duplicate attributes will have the first instance take
 // precedence (this can happen for declaration attributes).
 size_t DWARFDebugInfoEntry::GetAttributes(
-const DWARFUnit *cu, DWARFFormValue::FixedFormSizes fixed_form_sizes,
-DWARFAttributes &attributes, uint32_t curr_depth) const {
+const DWARFUnit *cu, DWARFAttributes &attributes,
+uint32_t curr_depth) const {
   const DWARFAbbreviationDeclaration *abbrevDecl = nullptr;
   lldb::offset_t offset = 0;
   if (cu)
@@ -733,10 +732,6 @@
   if (abbrevDecl) {
 const DWARFDataExtractor &debug_info_data = cu->GetData();
 
-if (fixed_form_sizes.Empty())
-  fixed_form_sizes = DWARFFormValue::GetFixedFormSizesForAddressSize(
-  cu->GetAddressByteSize());
-
 const uint32_t num_attributes = abbrevDecl->NumAttributes();
 for (uint32_t i = 0; i < num_attributes; ++i) {
   DWARFFormValue form_value(cu);
@@ -769,9 +764,9 @@
 spec_die.GetAttributes(attributes, curr_depth + 1);
 }
   } else {
-const uint8_t fixed_skip_size = fixed_form_sizes.GetSize(form);
+llvm::Optional fixed_skip_size = DWARFFormValue::GetFixedSize(form, cu);
 if (fixed_skip_size)
-  offset += fixed_skip_size;
+  offset += *fixed_skip_size;
 else
   

[Lldb-commits] [lldb] r361676 - Revert Xcode scheme changes from 361675

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Fri May 24 15:12:01 2019
New Revision: 361676

URL: http://llvm.org/viewvc/llvm-project?rev=361676&view=rev
Log:
Revert Xcode scheme changes from 361675 

Modified:
lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme

Modified: lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme?rev=361676&r1=361675&r2=361676&view=diff
==
--- lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme (original)
+++ lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/desktop.xcscheme Fri May 
24 15:12:01 2019
@@ -45,7 +45,7 @@
   buildConfiguration = "DebugClang"
   selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
   selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-  launchStyle = "0"
+  launchStyle = "1"
   useCustomWorkingDirectory = "NO"
   ignoresPersistentStateOnLaunch = "NO"
   debugDocumentVersioning = "YES"
@@ -61,12 +61,6 @@
 ReferencedContainer = "container:lldb.xcodeproj">
  
   
-  
- 
- 
-  
   
   



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

[Lldb-commits] [PATCH] D61235: Add more information to the log timer dump

[António Afonso via Phabricator via lldb-commits]

aadsm updated this revision to Diff 201349.
aadsm added a comment.

Rebase and fixed an issue with one of the asserts.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61235/new/

https://reviews.llvm.org/D61235

Files:
  lldb/include/lldb/Utility/Timer.h
  lldb/source/Utility/Timer.cpp
  lldb/unittests/Utility/TimerTest.cpp

Index: lldb/unittests/Utility/TimerTest.cpp
===
--- lldb/unittests/Utility/TimerTest.cpp
+++ lldb/unittests/Utility/TimerTest.cpp
@@ -61,7 +61,9 @@
   StreamString ss;
   Timer::DumpCategoryTimes(&ss);
   double seconds1, seconds2;
-  ASSERT_EQ(2, sscanf(ss.GetData(), "%lf sec for CAT1%*[\n ]%lf sec for CAT2",
+  ASSERT_EQ(2, sscanf(ss.GetData(),
+  "%lf sec (total: %*lfs; child: %*lfs; count: %*d) for "
+  "CAT1%*[\n ]%lf sec for CAT2",
   &seconds1, &seconds2))
   << "String: " << ss.GetData();
   EXPECT_LT(0.01, seconds1);
@@ -69,3 +71,38 @@
   EXPECT_LT(0.001, seconds2);
   EXPECT_GT(0.1, seconds2);
 }
+
+TEST(TimerTest, CategoryTimesStats) {
+  Timer::ResetCategoryTimes();
+  {
+static Timer::Category tcat1("CAT1");
+Timer t1(tcat1, ".");
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
+static Timer::Category tcat2("CAT2");
+{
+  Timer t2(tcat2, ".");
+  std::this_thread::sleep_for(std::chrono::milliseconds(10));
+}
+{
+  Timer t3(tcat2, ".");
+  std::this_thread::sleep_for(std::chrono::milliseconds(10));
+}
+  }
+  // Example output:
+  // 0.105202764 sec (total: 0.132s; child: 0.027s; count: 1) for CAT1
+  // 0.026772798 sec (total: 0.027s; child: 0.000s; count: 2) for CAT2
+  StreamString ss;
+  Timer::DumpCategoryTimes(&ss);
+  double seconds1, total1, child1, seconds2;
+  int count1, count2;
+  ASSERT_EQ(
+  6, sscanf(ss.GetData(),
+"%lf sec (total: %lfs; child: %lfs; count: %d) for CAT1%*[\n ]"
+"%lf sec (total: %*lfs; child: %*lfs; count: %d) for CAT2",
+&seconds1, &total1, &child1, &count1, &seconds2, &count2))
+  << "String: " << ss.GetData();
+  EXPECT_NEAR(total1 - child1, seconds1, 0.002);
+  EXPECT_EQ(1, count1);
+  EXPECT_NEAR(child1, seconds2, 0.002);
+  EXPECT_EQ(2, count2);
+}
Index: lldb/source/Utility/Timer.cpp
===
--- lldb/source/Utility/Timer.cpp
+++ lldb/source/Utility/Timer.cpp
@@ -41,6 +41,8 @@
 
 Timer::Category::Category(const char *cat) : m_name(cat) {
   m_nanos.store(0, std::memory_order_release);
+  m_nanos_total.store(0, std::memory_order_release);
+  m_count.store(0, std::memory_order_release);
   Category *expected = g_categories;
   do {
 m_next = expected;
@@ -93,6 +95,8 @@
 
   // Keep total results for each category so we can dump results.
   m_category.m_nanos += std::chrono::nanoseconds(timer_dur).count();
+  m_category.m_nanos_total += std::chrono::nanoseconds(total_dur).count();
+  m_category.m_count++;
 }
 
 void Timer::SetDisplayDepth(uint32_t depth) { g_display_depth = depth; }
@@ -100,25 +104,38 @@
 /* binary function predicate:
  * - returns whether a person is less than another person
  */
-
-typedef std::pair TimerEntry;
-
-static bool CategoryMapIteratorSortCriterion(const TimerEntry &lhs,
- const TimerEntry &rhs) {
-  return lhs.second > rhs.second;
+namespace {
+struct Stats {
+  const char *name;
+  uint64_t nanos;
+  uint64_t nanos_total;
+  uint64_t count;
+};
+} // namespace
+
+static bool CategoryMapIteratorSortCriterion(const Stats &lhs,
+ const Stats &rhs) {
+  return lhs.nanos > rhs.nanos;
 }
 
 void Timer::ResetCategoryTimes() {
-  for (Category *i = g_categories; i; i = i->m_next)
+  for (Category *i = g_categories; i; i = i->m_next) {
 i->m_nanos.store(0, std::memory_order_release);
+i->m_nanos_total.store(0, std::memory_order_release);
+i->m_count.store(0, std::memory_order_release);
+  }
 }
 
 void Timer::DumpCategoryTimes(Stream *s) {
-  std::vector sorted;
+  std::vector sorted;
   for (Category *i = g_categories; i; i = i->m_next) {
 uint64_t nanos = i->m_nanos.load(std::memory_order_acquire);
-if (nanos)
-  sorted.push_back(std::make_pair(i->m_name, nanos));
+if (nanos) {
+  uint64_t nanos_total = i->m_nanos_total.load(std::memory_order_acquire);
+  uint64_t count = i->m_count.load(std::memory_order_acquire);
+  Stats stats{i->m_name, nanos, nanos_total, count};
+  sorted.push_back(stats);
+}
   }
   if (sorted.empty())
 return; // Later code will break without any elements.
@@ -126,6 +143,9 @@
   // Sort by time
   llvm::sort(sorted.begin(), sorted.end(), CategoryMapIteratorSortCriterion);
 
-  for (const auto &timer : sorted)
-s->Printf("%.9f sec for %s\n", timer.second / 10., timer.first);
+  for (const auto

[Lldb-commits] [PATCH] D62213: [ABI] Implement Windows ABI for x86_64

[Wanyi Ye via Phabricator via lldb-commits]

kusmour updated this revision to Diff 201377.
kusmour added a comment.

update the `CreateDefaultUnwindPlan`.
return false or give empty Unwind Plan may cause lldb to crash and break tests 
on windows.
So for now, copy the SysV-x86_64


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62213/new/

https://reviews.llvm.org/D62213

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ABI/CMakeLists.txt
  lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
  lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.cpp
  lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h
  lldb/source/Plugins/ABI/Windows-x86_64/CMakeLists.txt
  lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp

Index: lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
+++ lldb/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
@@ -133,14 +133,14 @@
  nullptr,
  0},
 {DEFINE_GPR(r10, nullptr),
- {dwarf_r10_x86_64, dwarf_r10_x86_64, LLDB_REGNUM_GENERIC_ARG5,
+ {dwarf_r10_x86_64, dwarf_r10_x86_64, LLDB_INVALID_REGNUM,
   LLDB_INVALID_REGNUM, lldb_r10_x86_64},
  nullptr,
  nullptr,
  nullptr,
  0},
 {DEFINE_GPR(r11, nullptr),
- {dwarf_r11_x86_64, dwarf_r11_x86_64, LLDB_REGNUM_GENERIC_ARG6,
+ {dwarf_r11_x86_64, dwarf_r11_x86_64, LLDB_INVALID_REGNUM,
   LLDB_INVALID_REGNUM, lldb_r11_x86_64},
  nullptr,
  nullptr,
Index: lldb/source/Plugins/ABI/Windows-x86_64/CMakeLists.txt
===
--- /dev/null
+++ lldb/source/Plugins/ABI/Windows-x86_64/CMakeLists.txt
@@ -0,0 +1,10 @@
+add_lldb_library(lldbPluginABIWindows_x86_64 PLUGIN
+  ABIWindows_x86_64.cpp
+
+  LINK_LIBS
+lldbCore
+lldbSymbol
+lldbTarget
+  LINK_COMPONENTS
+Support
+  )
Index: lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h
===
--- /dev/null
+++ lldb/source/Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h
@@ -0,0 +1,99 @@
+//===-- ABIWindows_x86_64.h *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_ABIWindows_x86_64_h_
+#define liblldb_ABIWindows_x86_64_h_
+
+#include "lldb/Target/ABI.h"
+#include "lldb/lldb-private.h"
+
+class ABIWindows_x86_64 : public lldb_private::ABI {
+public:
+  ~ABIWindows_x86_64() override = default;
+
+  size_t GetRedZoneSize() const override;
+
+  bool PrepareTrivialCall(lldb_private::Thread &thread, lldb::addr_t sp,
+  lldb::addr_t functionAddress,
+  lldb::addr_t returnAddress,
+  llvm::ArrayRef args) const override;
+
+  bool GetArgumentValues(lldb_private::Thread &thread,
+ lldb_private::ValueList &values) const override;
+
+  lldb_private::Status
+  SetReturnValueObject(lldb::StackFrameSP &frame_sp,
+   lldb::ValueObjectSP &new_value) override;
+
+  lldb::ValueObjectSP
+  GetReturnValueObjectImpl(lldb_private::Thread &thread,
+   lldb_private::CompilerType &type) const override;
+
+  bool
+  CreateFunctionEntryUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
+
+  bool CreateDefaultUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
+
+  bool RegisterIsVolatile(const lldb_private::RegisterInfo *reg_info) override;
+
+  // In Windows_x86_64 ABI, stack will always be maintained 16-byte aligned
+  bool CallFrameAddressIsValid(lldb::addr_t cfa) override {
+	  if (cfa & (16ull - 1ull))
+  return false; // Not 16 byte aligned
+if (cfa == 0)
+  return false; // Zero is not a valid stack address
+return true;
+  }
+
+  bool CodeAddressIsValid(lldb::addr_t pc) override {
+// We have a 64 bit address space, so anything is valid as opcodes
+// aren't fixed width...
+return true;
+  }
+
+  const lldb_private::RegisterInfo *
+  GetRegisterInfoArray(uint32_t &count) override;
+
+  bool GetPointerReturnRegister(const char *&name) override;
+
+  //--
+  // Static Functions
+  //--
+
+  static void Initialize();
+
+  static void Terminate();
+
+  static lldb::ABISP CreateInstance(lldb::ProcessSP process_sp, const lldb_private::ArchSpec &arch);
+
+  static lldb_private::ConstString GetPluginNameStatic();
+
+  //