[Lldb-commits] [PATCH] D53435: [x86] Fix issues with a realigned stack in MSVC compiled applications

2018-10-19 Thread Aleksandr Urakov via Phabricator via lldb-commits 
aleksandr.urakov created this revision.
aleksandr.urakov added reviewers: labath, zturner, jasonmolenda, 
stella.stamenova.
aleksandr.urakov added a project: LLDB.
Herald added subscribers: lldb-commits, abidh.

This patch fixes issues with a stack realignment.

MSVC maintains two frame pointers (`ebx` and `ebp`) for a realigned stack - one 
is used for access to function parameters, while another is used for access to 
locals. To support this the patch:

- adds an alternative frame pointer (`ebx`);
- considers stack realignment instructions (e.g. `and esp, -32`);
- along with CFA (Canonical Frame Address) which point to the position next to 
the saved return address (or to the first parameter on the stack) introduces 
AFA (Aligned Frame Address) which points to the position of the stack pointer 
right after realignment. AFA is used for access to registers saved after the 
realignment (see the test);

Here is an example of the code with the realignment:

  struct __declspec(align(256)) OverAligned {
char c;
  };
  
  void foo(int foo_arg) {
OverAligned oa_foo = { 1 };
auto aaa_foo = 1234;
  }
  
  void bar(int bar_arg) {
OverAligned oa_bar = { 2 };
auto aaa_bar = 5678;
foo();
  }
  
  int main() {
bar();
return 0;
  }

and here is the `bar` disassembly:

  pushebx
  mov ebx, esp
  sub esp, 8
  and esp, -100h
  add esp, 4
  pushebp
  mov ebp, [ebx+4]
  mov [esp+4], ebp
  mov ebp, esp
  sub esp, 200h
  mov byte ptr [ebp-200h], 2
  mov dword ptr [ebp-4], 5678
  push; foo_arg
  callj_?foo@@YAXH@Z  ; foo(int)
  add esp, 4
  mov esp, ebp
  pop ebp
  mov esp, ebx
  pop ebx
  retn

Btw, it seems that the code of `x86AssemblyInspectionEngine` has overgrown. I 
have some ideas how to refactor this, if you don't mind I can do it in the 
future?

https://reviews.llvm.org/D53086 also contains some discussion on the topic.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53435

Files:
  include/lldb/Symbol/UnwindPlan.h
  source/Plugins/Process/Utility/RegisterContextLLDB.cpp
  source/Plugins/Process/Utility/RegisterContextLLDB.h
  source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp
  source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
  source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.h
  source/Symbol/UnwindPlan.cpp
  unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp

Index: unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
===
--- unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
+++ unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
@@ -133,7 +133,7 @@
 
 namespace lldb_private {
 static std::ostream &operator<<(std::ostream &OS,
-const UnwindPlan::Row::CFAValue &CFA) {
+const UnwindPlan::Row::FAValue &CFA) {
   StreamString S;
   CFA.Dump(S, nullptr, nullptr);
   return OS << S.GetData();
@@ -2368,7 +2368,7 @@
   ASSERT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(data, sizeof(data),
sample_range, plan));
 
-  UnwindPlan::Row::CFAValue esp_plus_4, esp_plus_8, ebp_plus_8;
+  UnwindPlan::Row::FAValue esp_plus_4, esp_plus_8, ebp_plus_8;
   esp_plus_4.SetIsRegisterPlusOffset(k_esp, 4);
   esp_plus_8.SetIsRegisterPlusOffset(k_esp, 8);
   ebp_plus_8.SetIsRegisterPlusOffset(k_ebp, 8);
@@ -2402,7 +2402,7 @@
   ASSERT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(data, sizeof(data),
sample_range, plan));
 
-  UnwindPlan::Row::CFAValue rsp_plus_8, rsp_plus_16, rbp_plus_16;
+  UnwindPlan::Row::FAValue rsp_plus_8, rsp_plus_16, rbp_plus_16;
   rsp_plus_8.SetIsRegisterPlusOffset(k_rsp, 8);
   rsp_plus_16.SetIsRegisterPlusOffset(k_rsp, 16);
   rbp_plus_16.SetIsRegisterPlusOffset(k_rbp, 16);
@@ -2416,6 +2416,65 @@
 plan.GetRowForFunctionOffset(sizeof(data) - 1)->GetCFAValue());
 }
 
+TEST_F(Testx86AssemblyInspectionEngine, TestStackRealignMSVC_i386) {
+  std::unique_ptr engine = Geti386Inspector();
+
+  uint8_t data[] = {
+  0x53,   // offset 00 -- pushl %ebx
+  0x8b, 0xdc, // offset 01 -- movl %esp, %ebx
+  0x83, 0xec, 0x08,   // offset 03 -- subl $8, %esp
+  0x81, 0xe4, 0x00, 0xff, 0xff, 0xff, // offset 06 -- andl $-256, %esp
+  0x83, 0xc4, 0x04,   // offset 12 -- addl $4, %esp
+  0x55,   // offset 15 -- pushl %ebp
+  0x8b, 0xec, // offset 16 -- movl %esp, %ebp
+  0x81, 0xec, 0x00, 0x02, 0x00, 0x00, // offset 18 -- subl $512, %esp
+  0x89, 0x7d, 0xfc,   // offset 24 -- movl %edi, -4(%ebp)
+  0x8b, 0xe5, // offset 27 -- movl %ebp,

[Lldb-commits] [PATCH] D53436: [LLDB] - Implement the support for the .debug_loclists section.

2018-10-19 Thread George Rimar via Phabricator via lldb-commits 
grimar created this revision.
grimar added reviewers: clayborg, LLDB.
Herald added subscribers: JDevlieghere, arichardson, aprantl, emaste.
Herald added a reviewer: espindola.

This implements the support for .debug_loclists section, which is
DWARF 5 version of .debug_loc.

Currently, clang is able to emit it with the use of 
https://reviews.llvm.org/D53365.


https://reviews.llvm.org/D53436

Files:
  include/lldb/Expression/DWARFExpression.h
  include/lldb/lldb-enumerations.h
  source/Core/Section.cpp
  source/Expression/DWARFExpression.cpp
  source/Expression/IRExecutionUnit.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
  source/Symbol/ObjectFile.cpp

Index: source/Symbol/ObjectFile.cpp
===
--- source/Symbol/ObjectFile.cpp
+++ source/Symbol/ObjectFile.cpp
@@ -352,6 +352,7 @@
   case eSectionTypeDWARFDebugLine:
   case eSectionTypeDWARFDebugLineStr:
   case eSectionTypeDWARFDebugLoc:
+  case eSectionTypeDWARFDebugLocLists:
   case eSectionTypeDWARFDebugMacInfo:
   case eSectionTypeDWARFDebugMacro:
   case eSectionTypeDWARFDebugNames:
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
@@ -30,7 +30,7 @@
   case lldb::eSectionTypeDWARFDebugLine:
 return llvm::DW_SECT_LINE;
   case lldb::eSectionTypeDWARFDebugLoc:
-return llvm::DW_SECT_LOC;
+return llvm::DW_SECT_LOC; 
   case lldb::eSectionTypeDWARFDebugStrOffsets:
 return llvm::DW_SECT_STR_OFFSETS;
   // case lldb::eSectionTypeDWARFDebugMacinfo:
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -244,6 +244,7 @@
   const lldb_private::DWARFDataExtractor &get_debug_line_str_data();
   const lldb_private::DWARFDataExtractor &get_debug_macro_data();
   const lldb_private::DWARFDataExtractor &get_debug_loc_data();
+  const lldb_private::DWARFDataExtractor &get_debug_loclists_data();
   const lldb_private::DWARFDataExtractor &get_debug_ranges_data();
   const lldb_private::DWARFDataExtractor &get_debug_rnglists_data();
   const lldb_private::DWARFDataExtractor &get_debug_str_data();
@@ -267,6 +268,8 @@
 
   const DWARFDebugRanges *DebugRanges() const;
 
+  const lldb_private::DWARFDataExtractor &DebugLocData();
+
   static bool SupportedVersion(uint16_t version);
 
   DWARFDIE
@@ -475,6 +478,7 @@
   DWARFDataSegment m_data_debug_line_str;
   DWARFDataSegment m_data_debug_macro;
   DWARFDataSegment m_data_debug_loc;
+  DWARFDataSegment m_data_debug_loclists;
   DWARFDataSegment m_data_debug_ranges;
   DWARFDataSegment m_data_debug_rnglists;
   DWARFDataSegment m_data_debug_str;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -650,10 +650,22 @@
   return GetCachedSectionData(eSectionTypeDWARFDebugMacro, m_data_debug_macro);
 }
 
+const DWARFDataExtractor &SymbolFileDWARF::DebugLocData() {
+  const DWARFDataExtractor &debugLocData = get_debug_loc_data();
+  if (debugLocData.GetByteSize() > 0)
+return debugLocData;
+  return get_debug_loclists_data();
+}
+
 const DWARFDataExtractor &SymbolFileDWARF::get_debug_loc_data() {
   return GetCachedSectionData(eSectionTypeDWARFDebugLoc, m_data_debug_loc);
 }
 
+const DWARFDataExtractor &SymbolFileDWARF::get_debug_loclists_data() {
+  return GetCachedSectionData(eSectionTypeDWARFDebugLocLists,
+  m_data_debug_loclists);
+}
+
 const DWARFDataExtractor &SymbolFileDWARF::get_debug_ranges_data() {
   return GetCachedSectionData(eSectionTypeDWARFDebugRanges,
   m_data_debug_ranges);
@@ -3307,7 +3319,7 @@
   uint32_t block_length = form_value.Unsigned();
   location.CopyOpcodeData(module, data, block_offset, block_length);
 } else {
-  const DWARFDataExtractor &debug_loc_data = get_debug_loc_data();
+  const DWARFDataExtractor &debug_loc_data = DebugLocData();
   const dw_offset_t debug_loc_offset = form_value.Unsigned();
 
   size_t loc_list_length = DWARFExpression::LocationListSize(
@@ -3821,6 +3833,8 @@
 
 DWARFExpression::LocationListFormat
 SymbolFileDW

[Lldb-commits] [PATCH] D53436: [LLDB] - Implement the support for the .debug_loclists section.

2018-10-19 Thread George Rimar via Phabricator via lldb-commits 
grimar added a comment.

Since .debug_loclists contains locations descriptions for objects
that can change location during lifetime, I do not see a way to
write the test without relying on a compiler that has support for
DWARF5. We should compile and run executable to test that location
was calculated properly I believe. Other our tests for DWARF5 sections
recently implemented used YAML and never run any executables.
So, the test case is not included.

At the same time I was able to test it with the following code and invocation:
(trunk clang + https://reviews.llvm.org/D53365 applied):

  ~/LLVM/build/bin/clang -gdwarf-5 test.cc  -o test_v5-fuse-ld=lld -fno-rtti



  struct A { 
int x = 0; 
virtual void foo(); 
  };
  
  void baz(struct A a) { 
   a.foo();
  }
  
  void A::foo() { int x = 0; ++x; }
  
  int main() {
A objA;
objA.x = 3;
baz(objA);
return 0;
  }

After executing the following commands, the location of `a` is properly 
evaluated:

  (lldb) target create "test_v5"
  Current executable set to 'test_v5' (x86_64).
  (lldb) b baz
  Breakpoint 1: where = test_v5`baz(A) + 4 at test.cc:9:6, address = 
0x002010e4
  (lldb) run
  Process 115974 launched: '/home/umb/tests_2018/110_lldbloclists/test_v5' 
(x86_64)
  Process 115974 stopped
  * thread #1, name = 'test_v5', stop reason = breakpoint 1.1
  frame #0: 0x002010e4 test_v5`baz(a=(x = 3)) at test.cc:9:6
 6};
 7   
 8void baz(struct A a) {
  -> 9   a.foo();
 10   }

Without this patch output will be:

  ...
  frame #0: 0x002010e4 test_v5`baz(a=) at test.cc:9:6
  ...


https://reviews.llvm.org/D53436



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

[Lldb-commits] [PATCH] D53415: [lldbsuite, windows] Disable two tail call frames tests that fail on Windows

2018-10-19 Thread Vedant Kumar via Phabricator via lldb-commits 
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

Thanks, lgtm.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53415



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

[Lldb-commits] [lldb] r344788 - [lldbsuite, windows] Disable two tail call frames tests that fail on Windows

2018-10-19 Thread Stella Stamenova via lldb-commits 
Author: stella.stamenova
Date: Fri Oct 19 09:00:58 2018
New Revision: 344788

URL: http://llvm.org/viewvc/llvm-project?rev=344788&view=rev
Log:
[lldbsuite, windows] Disable two tail call frames tests that fail on Windows

Summary: These tests fail on Windows because of known limitations (a.k.a. bugs) 
with the current implementation of GetFrameAtIndex

Reviewers: asmith, vsk

Reviewed By: vsk

Subscribers: abidh, lldb-commits

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py?rev=344788&r1=344787&r2=344788&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
 Fri Oct 19 09:00:58 2018
@@ -4,6 +4,7 @@ Test SB API support for identifying arti
 
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 class TestTailCallFrameSBAPI(TestBase):
@@ -14,6 +15,7 @@ class TestTailCallFrameSBAPI(TestBase):
 # each debug info format.
 NO_DEBUG_INFO_TESTCASE = True
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
 def test_tail_call_frame_sbapi(self):
 self.build()
 self.do_test()

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py?rev=344788&r1=344787&r2=344788&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
 Fri Oct 19 09:00:58 2018
@@ -4,6 +4,7 @@ Test SB API support for identifying arti
 
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 class TestArtificialFrameThreadStepOut1(TestBase):
@@ -51,6 +52,7 @@ class TestArtificialFrameThreadStepOut1(
 #   frame #4: ... a.out`main at main.cpp:23:3 [opt]
 return thread
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
 def test_stepping_out_past_artificial_frame(self):
 self.build()
 thread = self.prepare_thread()
@@ -68,6 +70,7 @@ class TestArtificialFrameThreadStepOut1(
 self.assertEqual(frame4.GetDisplayFunctionName(), "main")
 self.assertFalse(frame2.IsArtificial())
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
 def test_return_past_artificial_frame(self):
 self.build()
 thread = self.prepare_thread()


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

[Lldb-commits] [PATCH] D53415: [lldbsuite, windows] Disable two tail call frames tests that fail on Windows

2018-10-19 Thread Stella Stamenova via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344788: [lldbsuite, windows] Disable two tail call frames 
tests that fail on Windows (authored by stella.stamenova, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53415?vs=170135&id=170209#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53415

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py


Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
@@ -4,6 +4,7 @@
 
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 class TestTailCallFrameSBAPI(TestBase):
@@ -14,6 +15,7 @@
 # each debug info format.
 NO_DEBUG_INFO_TESTCASE = True
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
 def test_tail_call_frame_sbapi(self):
 self.build()
 self.do_test()
Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
@@ -4,6 +4,7 @@
 
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 class TestArtificialFrameThreadStepOut1(TestBase):
@@ -51,6 +52,7 @@
 #   frame #4: ... a.out`main at main.cpp:23:3 [opt]
 return thread
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
 def test_stepping_out_past_artificial_frame(self):
 self.build()
 thread = self.prepare_thread()
@@ -68,6 +70,7 @@
 self.assertEqual(frame4.GetDisplayFunctionName(), "main")
 self.assertFalse(frame2.IsArtificial())
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
 def test_return_past_artificial_frame(self):
 self.build()
 thread = self.prepare_thread()


Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
@@ -4,6 +4,7 @@
 
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 class TestTailCallFrameSBAPI(TestBase):
@@ -14,6 +15,7 @@
 # each debug info format.
 NO_DEBUG_INFO_TESTCASE = True
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
 def test_tail_call_frame_sbapi(self):
 self.build()
 self.do_test()
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
@@ -4,6 +4,7 @@
 
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 class TestArtificialFrameThreadStepOut1(TestBase):
@@ -51,6 +52,7 @@
 #   frame #4: ... a.out`main at main.cpp:23:3 [opt]
 return thread
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
 def test_stepping_out_past_artificial_frame(self):
 self.build()
 thread = self.prepare_thread()
@@ -68,6 +70,7 @@
 self.assertEqual(frame4.GetDisplayFunctionName(), "main")
 self.assertFalse(frame2.IsArtificial())
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
 def

[Lldb-commits] [PATCH] D53361: [API] Extend the `SBThreadPlan` interface

2018-10-19 Thread Jim Ingham via Phabricator via lldb-commits 
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

And tick the checkbox...


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53361



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

[Lldb-commits] [PATCH] D53361: [API] Extend the `SBThreadPlan` interface

2018-10-19 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

Presumably you are doing this so one scripted thread plan can use another one?  
This is the right way to do that, so this is great.  Not sure why I didn't 
think of it.

BTW, looks like I added this without any tests.  Totally my bad, this was a 
weekend experiment and was inert if not used, so I got the bits in and then 
wrote a bug on myself to write tests - demonstrating again why that's 
inadvisable...

If you are playing around with this and have a moment to formalize what you are 
doing into a test or two, I'd be grateful...


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53361



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

[Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file

2018-10-19 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a reviewer: clayborg.
jingham added a comment.

This seems like the sort of thing Greg should have a look at as well.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53368



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

[Lldb-commits] [PATCH] D53092: [lldb] Add support in Status::AsCString to retrieve win32 system error strings

2018-10-19 Thread Aaron Smith via Phabricator via lldb-commits 
asmith added inline comments.



Comment at: unittests/Utility/StatusTest.cpp:68-74
+  EXPECT_STREQ("Access is denied. ", s.AsCString());
+
+  s.SetError(ERROR_IPSEC_IKE_TIMED_OUT, ErrorType::eErrorTypeWin32);
+  EXPECT_STREQ("Negotiation timed out ", s.AsCString());
+
+  s.SetError(16000, ErrorType::eErrorTypeWin32);
+  EXPECT_STREQ("unknown error", s.AsCString());

aleksandr.urakov wrote:
> I'm not sure, can we rely on the fact that messages are the same in different 
> versions of Windows?
I don't think the errors are different but I don't have old versions of Windows 
setup to test. Everyone should be on Win10 right :-) I'll keep an eye on the 
buildbots after I commit.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53092



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

[Lldb-commits] [lldb] r344798 - [lldb] Add support in Status::AsCString to retrieve win32 system error strings

2018-10-19 Thread Aaron Smith via lldb-commits 
Author: asmith
Date: Fri Oct 19 11:58:24 2018
New Revision: 344798

URL: http://llvm.org/viewvc/llvm-project?rev=344798&view=rev
Log:
[lldb] Add support in Status::AsCString to retrieve win32 system error strings

Reviewers: rnk, zturner, aleksandr.urakov

Subscribers: lldb-commits

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

Modified:
lldb/trunk/source/Utility/Status.cpp
lldb/trunk/unittests/Utility/StatusTest.cpp

Modified: lldb/trunk/source/Utility/Status.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/Status.cpp?rev=344798&r1=344797&r2=344798&view=diff
==
--- lldb/trunk/source/Utility/Status.cpp (original)
+++ lldb/trunk/source/Utility/Status.cpp Fri Oct 19 11:58:24 2018
@@ -27,6 +27,9 @@
 #include 
 #endif
 
+#ifdef _WIN32
+#include 
+#endif
 #include  // for uint32_t
 
 namespace llvm {
@@ -87,7 +90,8 @@ llvm::Error Status::ToError() const {
   if (Success())
 return llvm::Error::success();
   if (m_type == ErrorType::eErrorTypePOSIX)
-return llvm::errorCodeToError(std::error_code(m_code, 
std::generic_category()));
+return llvm::errorCodeToError(
+std::error_code(m_code, std::generic_category()));
   return llvm::make_error(AsCString(),
  llvm::inconvertibleErrorCode());
 }
@@ -106,6 +110,23 @@ const Status &Status::operator=(const St
 
 Status::~Status() = default;
 
+#ifdef _WIN32
+static std::string RetrieveWin32ErrorString(uint32_t error_code) {
+  char *buffer = nullptr;
+  std::string message;
+  // Retrieve win32 system error.
+  if (::FormatMessageA(
+  FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM |
+  FORMAT_MESSAGE_MAX_WIDTH_MASK,
+  NULL, error_code, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+  (LPSTR)&buffer, 0, NULL)) {
+message.assign(buffer);
+::LocalFree(buffer);
+  }
+  return message;
+}
+#endif
+
 //--
 // Get the error value as a NULL C string. The error string will be fetched and
 // cached on demand. The cached error string value will remain until the error
@@ -128,6 +149,12 @@ const char *Status::AsCString(const char
   m_string = llvm::sys::StrError(m_code);
   break;
 
+case eErrorTypeWin32:
+#if defined(_WIN32)
+  m_string = RetrieveWin32ErrorString(m_code);
+#endif
+  break;
+
 default:
   break;
 }

Modified: lldb/trunk/unittests/Utility/StatusTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/StatusTest.cpp?rev=344798&r1=344797&r2=344798&view=diff
==
--- lldb/trunk/unittests/Utility/StatusTest.cpp (original)
+++ lldb/trunk/unittests/Utility/StatusTest.cpp Fri Oct 19 11:58:24 2018
@@ -10,6 +10,10 @@
 #include "lldb/Utility/Status.h"
 #include "gtest/gtest.h"
 
+#ifdef _WIN32
+#include 
+#endif
+
 using namespace lldb_private;
 using namespace lldb;
 
@@ -51,3 +55,22 @@ TEST(StatusTest, ErrorConversion) {
   EXPECT_TRUE(bool(foo));
   EXPECT_EQ("foo", llvm::toString(std::move(foo)));
 }
+
+#ifdef _WIN32
+TEST(StatusTest, ErrorWin32) {
+  auto success = Status(NO_ERROR, ErrorType::eErrorTypeWin32);
+  EXPECT_STREQ(NULL, success.AsCString());
+  EXPECT_FALSE(success.ToError());
+  EXPECT_TRUE(success.Success());
+
+  auto s = Status(ERROR_ACCESS_DENIED, ErrorType::eErrorTypeWin32);
+  EXPECT_TRUE(s.Fail());
+  EXPECT_STREQ("Access is denied. ", s.AsCString());
+
+  s.SetError(ERROR_IPSEC_IKE_TIMED_OUT, ErrorType::eErrorTypeWin32);
+  EXPECT_STREQ("Negotiation timed out ", s.AsCString());
+
+  s.SetError(16000, ErrorType::eErrorTypeWin32);
+  EXPECT_STREQ("unknown error", s.AsCString());
+}
+#endif


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

[Lldb-commits] [PATCH] D53092: [lldb] Add support in Status::AsCString to retrieve win32 system error strings

2018-10-19 Thread Aaron Smith via Phabricator via lldb-commits 
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB344798: [lldb] Add support in Status::AsCString to 
retrieve win32 system error strings (authored by asmith, committed by ).

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53092

Files:
  source/Utility/Status.cpp
  unittests/Utility/StatusTest.cpp


Index: unittests/Utility/StatusTest.cpp
===
--- unittests/Utility/StatusTest.cpp
+++ unittests/Utility/StatusTest.cpp
@@ -10,6 +10,10 @@
 #include "lldb/Utility/Status.h"
 #include "gtest/gtest.h"
 
+#ifdef _WIN32
+#include 
+#endif
+
 using namespace lldb_private;
 using namespace lldb;
 
@@ -51,3 +55,22 @@
   EXPECT_TRUE(bool(foo));
   EXPECT_EQ("foo", llvm::toString(std::move(foo)));
 }
+
+#ifdef _WIN32
+TEST(StatusTest, ErrorWin32) {
+  auto success = Status(NO_ERROR, ErrorType::eErrorTypeWin32);
+  EXPECT_STREQ(NULL, success.AsCString());
+  EXPECT_FALSE(success.ToError());
+  EXPECT_TRUE(success.Success());
+
+  auto s = Status(ERROR_ACCESS_DENIED, ErrorType::eErrorTypeWin32);
+  EXPECT_TRUE(s.Fail());
+  EXPECT_STREQ("Access is denied. ", s.AsCString());
+
+  s.SetError(ERROR_IPSEC_IKE_TIMED_OUT, ErrorType::eErrorTypeWin32);
+  EXPECT_STREQ("Negotiation timed out ", s.AsCString());
+
+  s.SetError(16000, ErrorType::eErrorTypeWin32);
+  EXPECT_STREQ("unknown error", s.AsCString());
+}
+#endif
Index: source/Utility/Status.cpp
===
--- source/Utility/Status.cpp
+++ source/Utility/Status.cpp
@@ -27,6 +27,9 @@
 #include 
 #endif
 
+#ifdef _WIN32
+#include 
+#endif
 #include  // for uint32_t
 
 namespace llvm {
@@ -87,7 +90,8 @@
   if (Success())
 return llvm::Error::success();
   if (m_type == ErrorType::eErrorTypePOSIX)
-return llvm::errorCodeToError(std::error_code(m_code, 
std::generic_category()));
+return llvm::errorCodeToError(
+std::error_code(m_code, std::generic_category()));
   return llvm::make_error(AsCString(),
  llvm::inconvertibleErrorCode());
 }
@@ -106,6 +110,23 @@
 
 Status::~Status() = default;
 
+#ifdef _WIN32
+static std::string RetrieveWin32ErrorString(uint32_t error_code) {
+  char *buffer = nullptr;
+  std::string message;
+  // Retrieve win32 system error.
+  if (::FormatMessageA(
+  FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM |
+  FORMAT_MESSAGE_MAX_WIDTH_MASK,
+  NULL, error_code, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+  (LPSTR)&buffer, 0, NULL)) {
+message.assign(buffer);
+::LocalFree(buffer);
+  }
+  return message;
+}
+#endif
+
 //--
 // Get the error value as a NULL C string. The error string will be fetched and
 // cached on demand. The cached error string value will remain until the error
@@ -128,6 +149,12 @@
   m_string = llvm::sys::StrError(m_code);
   break;
 
+case eErrorTypeWin32:
+#if defined(_WIN32)
+  m_string = RetrieveWin32ErrorString(m_code);
+#endif
+  break;
+
 default:
   break;
 }


Index: unittests/Utility/StatusTest.cpp
===
--- unittests/Utility/StatusTest.cpp
+++ unittests/Utility/StatusTest.cpp
@@ -10,6 +10,10 @@
 #include "lldb/Utility/Status.h"
 #include "gtest/gtest.h"
 
+#ifdef _WIN32
+#include 
+#endif
+
 using namespace lldb_private;
 using namespace lldb;
 
@@ -51,3 +55,22 @@
   EXPECT_TRUE(bool(foo));
   EXPECT_EQ("foo", llvm::toString(std::move(foo)));
 }
+
+#ifdef _WIN32
+TEST(StatusTest, ErrorWin32) {
+  auto success = Status(NO_ERROR, ErrorType::eErrorTypeWin32);
+  EXPECT_STREQ(NULL, success.AsCString());
+  EXPECT_FALSE(success.ToError());
+  EXPECT_TRUE(success.Success());
+
+  auto s = Status(ERROR_ACCESS_DENIED, ErrorType::eErrorTypeWin32);
+  EXPECT_TRUE(s.Fail());
+  EXPECT_STREQ("Access is denied. ", s.AsCString());
+
+  s.SetError(ERROR_IPSEC_IKE_TIMED_OUT, ErrorType::eErrorTypeWin32);
+  EXPECT_STREQ("Negotiation timed out ", s.AsCString());
+
+  s.SetError(16000, ErrorType::eErrorTypeWin32);
+  EXPECT_STREQ("unknown error", s.AsCString());
+}
+#endif
Index: source/Utility/Status.cpp
===
--- source/Utility/Status.cpp
+++ source/Utility/Status.cpp
@@ -27,6 +27,9 @@
 #include 
 #endif
 
+#ifdef _WIN32
+#include 
+#endif
 #include  // for uint32_t
 
 namespace llvm {
@@ -87,7 +90,8 @@
   if (Success())
 return llvm::Error::success();
   if (m_type == ErrorType::eErrorTypePOSIX)
-return llvm::errorCodeToError(std::error_code(m_code, std::generic_category()));
+return llvm::errorCodeToError(
+std::error_code(m_code, std::generic_category()));
   return llvm::make_error(AsCString(),

[Lldb-commits] [PATCH] D53086: [PDB] Fix flaky `variables-locations.test` after PR38857

2018-10-19 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

Yes it sounds like the situation is very much similar here as it is in  dwarf 
land -- the compilers there also don't tend to emit unwind info for prologues 
and epilogues.

The thing to realize about the instruction emulation is that it is always going 
to be a best effort enterprise. We can try to pattern match the sequences 
generated by the compilers, but it's always going to be playing catch up, and 
prone to false positives. That's why it's important to use the debug info 
provided by the compiler, as it is in a much better position to generate the 
unwind info.

Even the emulator itself has two modes of operation: in the first one it 
synthesizes the unwind info out-of-the-blue. In the second one it takes the 
existing unwind info and "augments" it with additional rows. Obviously, the 
second one is much more easier to implement and reliable.

The conversion of the FPO programs into DWARF does not sound like a 
particularly fun enterprise, though I'm not sure if that's the only way to do 
this kind of thing. In any case, it sounds like the process should be easily 
unit-testable. As for the saved registers, won't the compiler's unwind info 
contain that information? I would expect that is necessary for exception 
unwinding..

tl;dr; I am not against improving the instruction emulator. I just think it's 
not the most important priority right now. Getting the compiler-generated info 
working puts the emulator out of the critical path, and leaves it just 
responsible for the less frequent cases when someone is stopped in the middle 
of the prologue, or needs to access a variable that resides in a spilled 
register. But if you want to work on the emulator first, then go ahead..


https://reviews.llvm.org/D53086



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

[Lldb-commits] [PATCH] D53375: [PDB] Improve performance of the PDB DIA plugin

2018-10-19 Thread Aaron Smith via Phabricator via lldb-commits 
asmith accepted this revision.
asmith added a comment.
This revision is now accepted and ready to land.

Thanks for adding the cache. We noticed the slowdown also and were discussing 
the same thing. This LGTM if the other reviewers done have any comments.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53375



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

[Lldb-commits] [lldb] r344809 - [PDB] Test variadic function type in PDB

2018-10-19 Thread Aaron Smith via lldb-commits 
Author: asmith
Date: Fri Oct 19 12:30:59 2018
New Revision: 344809

URL: http://llvm.org/viewvc/llvm-project?rev=344809&view=rev
Log:
[PDB] Test variadic function type in PDB

This adds back the test case reverted in commit: 
d260a269200824c5c1c8c6de531fd5aa63db9c35

Modified:
lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp
lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.exe
lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.pdb
lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Modified: lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp?rev=344809&r1=344808&r2=344809&view=diff
==
--- lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp (original)
+++ lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp Fri Oct 19 
12:30:59 2018
@@ -34,7 +34,7 @@ class NSClass {
   float f;
   double d;
 };
-}
+} // namespace NS
 
 class Class {
 public:
@@ -48,6 +48,10 @@ int test_func(int a, int b) { return a +
 
 typedef Class ClassTypedef;
 typedef NS::NSClass NSClassTypedef;
+typedef int (*FuncPointerTypedef)();
+typedef int (*VariadicFuncPointerTypedef)(char, ...);
+FuncPointerTypedef GlobalFunc;
+VariadicFuncPointerTypedef GlobalVariadicFunc;
 int GlobalArray[10];
 
 static const int sizeof_NSClass = sizeof(NS::NSClass);
@@ -57,6 +61,9 @@ static const int sizeof_Enum = sizeof(En
 static const int sizeof_ShortEnum = sizeof(ShortEnum);
 static const int sizeof_ClassTypedef = sizeof(ClassTypedef);
 static const int sizeof_NSClassTypedef = sizeof(NSClassTypedef);
+static const int sizeof_FuncPointerTypedef = sizeof(FuncPointerTypedef);
+static const int sizeof_VariadicFuncPointerTypedef =
+sizeof(VariadicFuncPointerTypedef);
 static const int sizeof_GlobalArray = sizeof(GlobalArray);
 
 int main(int argc, char **argv) {

Modified: lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.exe
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.exe?rev=344809&r1=344808&r2=344809&view=diff
==
Binary files lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.exe 
(original) and lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.exe 
Fri Oct 19 12:30:59 2018 differ

Modified: lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.pdb
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.pdb?rev=344809&r1=344808&r2=344809&view=diff
==
Binary files lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.pdb 
(original) and lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-types.pdb 
Fri Oct 19 12:30:59 2018 differ

Modified: lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp?rev=344809&r1=344808&r2=344809&view=diff
==
--- lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp (original)
+++ lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp Fri Oct 19 
12:30:59 2018
@@ -200,8 +200,7 @@ TEST_F(SymbolFilePDBTests, TestResolveSy
   EXPECT_TRUE(ContainsCompileUnit(sc_list, header_spec));
 }
 
-TEST_F(SymbolFilePDBTests,
-   TestLookupOfHeaderFileWithInlines) {
+TEST_F(SymbolFilePDBTests, TestLookupOfHeaderFileWithInlines) {
   // Test that when looking up a header file via ResolveSymbolContext (i.e. a
   // file that was not by itself
   // compiled, but only contributes to the combined code of other source 
files),
@@ -531,7 +530,9 @@ TEST_F(SymbolFilePDBTests, TestTypedefs)
   llvm::DenseSet searched_files;
   TypeMap results;
 
-  const char *TypedefsToCheck[] = {"ClassTypedef", "NSClassTypedef"};
+  const char *TypedefsToCheck[] = {"ClassTypedef", "NSClassTypedef",
+   "FuncPointerTypedef",
+   "VariadicFuncPointerTypedef"};
   for (auto Typedef : TypedefsToCheck) {
 TypeMap results;
 EXPECT_EQ(1u, symfile->FindTypes(sc, ConstString(Typedef), nullptr, false,
@@ -561,7 +562,7 @@ TEST_F(SymbolFilePDBTests, TestRegexName
   SymbolFilePDB *symfile =
   static_cast(plugin->GetSymbolFile());
   TypeMap results;
-   
+
   symfile->FindTypesByRegex(RegularExpression(".*"), 0, results);
   EXPECT_GT(results.GetSize(), 1u);
 
@@ -583,8 +584,8 @@ TEST_F(SymbolFilePDBTests, TestMaxMatche
   llvm::DenseSet searched_files;
   TypeMap results;
   const ConstString name("ClassTypedef");
-  uint32_t num_results = symfile->FindTypes(sc, name, nullptr,
-false, 0, searched_files, results);
+  uint32_t num_r

[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Add the module lock where necessary and assert that we own it.

2018-10-19 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 170242.
JDevlieghere retitled this revision from "[DWARFSymbolFile] Adds the module 
lock to all virtual methods from SymbolFile" to "[DWARFSymbolFile] Add the 
module lock where necessary and assert that we own it.".
JDevlieghere edited the summary of this revision.

https://reviews.llvm.org/D52543

Files:
  include/lldb/Symbol/SymbolFile.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Symbol/SymbolFile.cpp

Index: source/Symbol/SymbolFile.cpp
===
--- source/Symbol/SymbolFile.cpp
+++ source/Symbol/SymbolFile.cpp
@@ -19,12 +19,18 @@
 #include "lldb/Utility/StreamString.h"
 #include "lldb/lldb-private.h"
 
+#include 
+
 using namespace lldb_private;
 
 void SymbolFile::PreloadSymbols() {
   // No-op for most implementations.
 }
 
+std::recursive_mutex &SymbolFile::GetModuleMutex() const {
+  return GetObjectFile()->GetModule()->GetMutex();
+}
+
 SymbolFile *SymbolFile::FindPlugin(ObjectFile *obj_file) {
   std::unique_ptr best_symfile_ap;
   if (obj_file != nullptr) {
@@ -150,3 +156,13 @@
 types.Clear();
   return 0;
 }
+
+void SymbolFile::AssertModuleLock() {
+  // We assert that we have to module lock by trying to acquire the lock from a
+  // different thread. Note that we must abort if the result is true to
+  // guarantee correctness.
+  lldbassert(std::async(std::launch::async,
+[this] { return this->GetModuleMutex().try_lock(); })
+ .get() == false &&
+ "Module is not locked");
+}
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -228,6 +228,8 @@
 
   void PreloadSymbols() override;
 
+  std::recursive_mutex &GetModuleMutex() const override;
+
   //--
   // PluginInterface protocol
   //--
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -260,6 +260,9 @@
 }
 
 TypeList *SymbolFileDWARF::GetTypeList() {
+  // This method can be called without going through the symbol vendor so we
+  // need to lock the module.
+  std::lock_guard guard(GetModuleMutex());
   SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
   if (debug_map_symfile)
 return debug_map_symfile->GetTypeList();
@@ -341,6 +344,7 @@
  uint32_t type_mask, TypeList &type_list)
 
 {
+  AssertModuleLock();
   TypeSet type_set;
 
   CompileUnit *comp_unit = NULL;
@@ -860,6 +864,7 @@
 }
 
 CompUnitSP SymbolFileDWARF::ParseCompileUnitAtIndex(uint32_t cu_idx) {
+  AssertModuleLock();
   CompUnitSP cu_sp;
   DWARFDebugInfo *info = DebugInfo();
   if (info) {
@@ -872,6 +877,7 @@
 
 Function *SymbolFileDWARF::ParseCompileUnitFunction(const SymbolContext &sc,
 const DWARFDIE &die) {
+  AssertModuleLock();
   if (die.IsValid()) {
 TypeSystem *type_system =
 GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
@@ -895,6 +901,7 @@
 }
 lldb::LanguageType
 SymbolFileDWARF::ParseCompileUnitLanguage(const SymbolContext &sc) {
+  AssertModuleLock();
   assert(sc.comp_unit);
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu)
@@ -904,6 +911,7 @@
 }
 
 size_t SymbolFileDWARF::ParseCompileUnitFunctions(const SymbolContext &sc) {
+  AssertModuleLock();
   assert(sc.comp_unit);
   size_t functions_added = 0;
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
@@ -926,6 +934,7 @@
 
 bool SymbolFileDWARF::ParseCompileUnitSupportFiles(
 const SymbolContext &sc, FileSpecList &support_files) {
+  AssertModuleLock();
   assert(sc.comp_unit);
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu) {
@@ -951,6 +960,7 @@
 
 bool SymbolFileDWARF::ParseCompileUnitIsOptimized(
 const lldb_private::SymbolContext &sc) {
+  AssertModuleLock();
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu)
 return dwarf_cu->GetIsOptimized();
@@ -960,6 +970,7 @@
 bool SymbolFileDWARF::ParseImportedModules(
 const lldb_private::SymbolContext &sc,
 std::vector &imported_modules) {
+  AssertModuleLock();
   assert(sc.comp_unit);
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu) {
@@ -1037,6 +1048,7 @@
 }
 
 bool SymbolFileDWARF::ParseCompileUnitLineTable(const SymbolContext &sc) {
+  AssertModuleLock();
   assert(sc.comp_unit);
   if (sc.comp_unit->GetLineTable() != NULL)
 return true;
@@ -1122,6 +1134,7 @@
 }
 
 bool Sym

[Lldb-commits] [PATCH] D52953: [lldb-mi] Implement -gdb-set breakpoint pending on/off

2018-10-19 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a reviewer: apolyakov.
jingham added a comment.

This looks okay to me, there's a trivial comment nit...  But Alexander has been 
doing the most work on MI recently, he might want to give this a look-over.


Repository:
  rL LLVM

https://reviews.llvm.org/D52953



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

[Lldb-commits] [PATCH] D52651: Add functionality to export settings

2018-10-19 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 170252.
JDevlieghere added a comment.

Only clear setting when the force flag is set (as suggested offline by 
@jingham).


https://reviews.llvm.org/D52651

Files:
  lit/Settings/TestSettingsSet.test
  source/Commands/CommandObjectSettings.cpp


Index: source/Commands/CommandObjectSettings.cpp
===
--- source/Commands/CommandObjectSettings.cpp
+++ source/Commands/CommandObjectSettings.cpp
@@ -30,7 +30,8 @@
 
 static constexpr OptionDefinition g_settings_set_options[] = {
 // clang-format off
-  { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, 
{}, 0, eArgTypeNone, "Apply the new value to the global default value." }
+  { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, 
{}, 0, eArgTypeNone, "Apply the new value to the global default value." },
+  { LLDB_OPT_SET_2, false, "force",  'f', OptionParser::eNoArgument, nullptr, 
{}, 0, eArgTypeNone, "Force an empty value to be accepted as the default." }
 // clang-format on
 };
 
@@ -108,6 +109,9 @@
   const int short_option = m_getopt_table[option_idx].val;
 
   switch (short_option) {
+  case 'f':
+m_force = true;
+break;
   case 'g':
 m_global = true;
 break;
@@ -122,15 +126,16 @@
 
 void OptionParsingStarting(ExecutionContext *execution_context) override {
   m_global = false;
+  m_force = false;
 }
 
 llvm::ArrayRef GetDefinitions() override {
   return llvm::makeArrayRef(g_settings_set_options);
 }
 
 // Instance variables to hold the values for command options.
-
 bool m_global;
+bool m_force;
   };
 
   int HandleArgumentCompletion(
@@ -184,8 +189,10 @@
 if (!ParseOptions(cmd_args, result))
   return false;
 
+const size_t min_argc = m_options.m_force ? 1 : 2;
 const size_t argc = cmd_args.GetArgumentCount();
-if ((argc < 2) && (!m_options.m_global)) {
+
+if ((argc < min_argc) && (!m_options.m_global)) {
   result.AppendError("'settings set' takes more arguments");
   result.SetStatus(eReturnStatusFailed);
   return false;
@@ -199,6 +206,19 @@
   return false;
 }
 
+// A missing value corresponds to clearing the setting when "force" is
+// specified.
+if (argc == 1 && m_options.m_force) {
+  Status error(m_interpreter.GetDebugger().SetPropertyValue(
+  &m_exe_ctx, eVarSetOperationClear, var_name, llvm::StringRef()));
+  if (error.Fail()) {
+result.AppendError(error.AsCString());
+result.SetStatus(eReturnStatusFailed);
+return false;
+  }
+  return result.Succeeded();
+}
+
 // Split the raw command into var_name and value pair.
 llvm::StringRef raw_str(command);
 std::string var_value_string = raw_str.split(var_name).second.str();
Index: lit/Settings/TestSettingsSet.test
===
--- /dev/null
+++ lit/Settings/TestSettingsSet.test
@@ -0,0 +1,15 @@
+# This tests setting setting values.
+
+# Check that setting an empty value with -f(orce) clears the value.
+# RUN: %lldb -b -s %s 2>&1 | FileCheck %s
+
+settings set tab-size 16
+settings show tab-size
+# CHECK: tab-size (unsigned) = 16
+
+settings set -f tab-size
+settings show tab-size
+# CHECK: tab-size (unsigned) = 4
+
+settings set tab-size
+# CHECK: error: 'settings set' takes more arguments


Index: source/Commands/CommandObjectSettings.cpp
===
--- source/Commands/CommandObjectSettings.cpp
+++ source/Commands/CommandObjectSettings.cpp
@@ -30,7 +30,8 @@
 
 static constexpr OptionDefinition g_settings_set_options[] = {
 // clang-format off
-  { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Apply the new value to the global default value." }
+  { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Apply the new value to the global default value." },
+  { LLDB_OPT_SET_2, false, "force",  'f', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Force an empty value to be accepted as the default." }
 // clang-format on
 };
 
@@ -108,6 +109,9 @@
   const int short_option = m_getopt_table[option_idx].val;
 
   switch (short_option) {
+  case 'f':
+m_force = true;
+break;
   case 'g':
 m_global = true;
 break;
@@ -122,15 +126,16 @@
 
 void OptionParsingStarting(ExecutionContext *execution_context) override {
   m_global = false;
+  m_force = false;
 }
 
 llvm::ArrayRef GetDefinitions() override {
   return llvm::makeArrayRef(g_settings_set_options);
 }
 
 // Instance variables to hold the values for command options.
-
 bool m_global;
+bool m_force;
   };
 
   int HandleArgumentCompletion(
@@ -184,8 +189,10 @@
 if (!ParseOptions(c

[Lldb-commits] [PATCH] D52772: [Settings] Make "settings set" without a value equivalent to "settings clear"

2018-10-19 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 170257.
JDevlieghere added a comment.

Only clear setting when the force flag is set (as suggested offline by 
@jingham).


https://reviews.llvm.org/D52772

Files:
  lit/Settings/TestSettingsSet.test
  source/Commands/CommandObjectSettings.cpp


Index: source/Commands/CommandObjectSettings.cpp
===
--- source/Commands/CommandObjectSettings.cpp
+++ source/Commands/CommandObjectSettings.cpp
@@ -30,7 +30,8 @@
 
 static constexpr OptionDefinition g_settings_set_options[] = {
 // clang-format off
-  { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, 
{}, 0, eArgTypeNone, "Apply the new value to the global default value." }
+  { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, 
{}, 0, eArgTypeNone, "Apply the new value to the global default value." },
+  { LLDB_OPT_SET_2, false, "force",  'f', OptionParser::eNoArgument, nullptr, 
{}, 0, eArgTypeNone, "Force an empty value to be accepted as the default." }
 // clang-format on
 };
 
@@ -108,6 +109,9 @@
   const int short_option = m_getopt_table[option_idx].val;
 
   switch (short_option) {
+  case 'f':
+m_force = true;
+break;
   case 'g':
 m_global = true;
 break;
@@ -122,15 +126,16 @@
 
 void OptionParsingStarting(ExecutionContext *execution_context) override {
   m_global = false;
+  m_force = false;
 }
 
 llvm::ArrayRef GetDefinitions() override {
   return llvm::makeArrayRef(g_settings_set_options);
 }
 
 // Instance variables to hold the values for command options.
-
 bool m_global;
+bool m_force;
   };
 
   int HandleArgumentCompletion(
@@ -184,8 +189,10 @@
 if (!ParseOptions(cmd_args, result))
   return false;
 
+const size_t min_argc = m_options.m_force ? 1 : 2;
 const size_t argc = cmd_args.GetArgumentCount();
-if ((argc < 2) && (!m_options.m_global)) {
+
+if ((argc < min_argc) && (!m_options.m_global)) {
   result.AppendError("'settings set' takes more arguments");
   result.SetStatus(eReturnStatusFailed);
   return false;
@@ -199,6 +206,19 @@
   return false;
 }
 
+// A missing value corresponds to clearing the setting when "force" is
+// specified.
+if (argc == 1 && m_options.m_force) {
+  Status error(m_interpreter.GetDebugger().SetPropertyValue(
+  &m_exe_ctx, eVarSetOperationClear, var_name, llvm::StringRef()));
+  if (error.Fail()) {
+result.AppendError(error.AsCString());
+result.SetStatus(eReturnStatusFailed);
+return false;
+  }
+  return result.Succeeded();
+}
+
 // Split the raw command into var_name and value pair.
 llvm::StringRef raw_str(command);
 std::string var_value_string = raw_str.split(var_name).second.str();
Index: lit/Settings/TestSettingsSet.test
===
--- /dev/null
+++ lit/Settings/TestSettingsSet.test
@@ -0,0 +1,15 @@
+# This tests setting setting values.
+
+# Check that setting an empty value with -f(orce) clears the value.
+# RUN: %lldb -b -s %s 2>&1 | FileCheck %s
+
+settings set tab-size 16
+settings show tab-size
+# CHECK: tab-size (unsigned) = 16
+
+settings set -f tab-size
+settings show tab-size
+# CHECK: tab-size (unsigned) = 4
+
+settings set tab-size
+# CHECK: error: 'settings set' takes more arguments


Index: source/Commands/CommandObjectSettings.cpp
===
--- source/Commands/CommandObjectSettings.cpp
+++ source/Commands/CommandObjectSettings.cpp
@@ -30,7 +30,8 @@
 
 static constexpr OptionDefinition g_settings_set_options[] = {
 // clang-format off
-  { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Apply the new value to the global default value." }
+  { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Apply the new value to the global default value." },
+  { LLDB_OPT_SET_2, false, "force",  'f', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Force an empty value to be accepted as the default." }
 // clang-format on
 };
 
@@ -108,6 +109,9 @@
   const int short_option = m_getopt_table[option_idx].val;
 
   switch (short_option) {
+  case 'f':
+m_force = true;
+break;
   case 'g':
 m_global = true;
 break;
@@ -122,15 +126,16 @@
 
 void OptionParsingStarting(ExecutionContext *execution_context) override {
   m_global = false;
+  m_force = false;
 }
 
 llvm::ArrayRef GetDefinitions() override {
   return llvm::makeArrayRef(g_settings_set_options);
 }
 
 // Instance variables to hold the values for command options.
-
 bool m_global;
+bool m_force;
   };
 
   int HandleArgumentCompletion(
@@ -184,8 +189,10 @@
 if (!ParseOptions(c

[Lldb-commits] [PATCH] D52651: Add functionality to export settings

2018-10-19 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 170258.
JDevlieghere added a comment.

- Back to the right diff.
- Use `-f` as per the other patch.


https://reviews.llvm.org/D52651

Files:
  include/lldb/Interpreter/OptionValue.h
  lit/Settings/TestExport.test
  source/Commands/CommandObjectSettings.cpp
  source/Interpreter/OptionValueArray.cpp
  source/Interpreter/OptionValueDictionary.cpp
  source/Interpreter/OptionValueFileSpecLIst.cpp
  source/Interpreter/OptionValueFormatEntity.cpp
  source/Interpreter/OptionValueLanguage.cpp
  source/Interpreter/Property.cpp

Index: source/Interpreter/Property.cpp
===
--- source/Interpreter/Property.cpp
+++ source/Interpreter/Property.cpp
@@ -233,7 +233,10 @@
 uint32_t dump_mask) const {
   if (m_value_sp) {
 const bool dump_desc = dump_mask & OptionValue::eDumpOptionDescription;
+const bool dump_cmd = dump_mask & OptionValue::eDumpOptionCommand;
 const bool transparent = m_value_sp->ValueIsTransparent();
+if (dump_cmd && !transparent)
+  strm << "settings set -f ";
 if (dump_desc || !transparent) {
   if ((dump_mask & OptionValue::eDumpOptionName) && m_name) {
 DumpQualifiedName(strm);
Index: source/Interpreter/OptionValueLanguage.cpp
===
--- source/Interpreter/OptionValueLanguage.cpp
+++ source/Interpreter/OptionValueLanguage.cpp
@@ -28,7 +28,8 @@
   if (dump_mask & eDumpOptionValue) {
 if (dump_mask & eDumpOptionType)
   strm.PutCString(" = ");
-strm.PutCString(Language::GetNameForLanguageType(m_current_value));
+if (m_current_value != eLanguageTypeUnknown)
+  strm.PutCString(Language::GetNameForLanguageType(m_current_value));
   }
 }
 
Index: source/Interpreter/OptionValueFormatEntity.cpp
===
--- source/Interpreter/OptionValueFormatEntity.cpp
+++ source/Interpreter/OptionValueFormatEntity.cpp
@@ -61,10 +61,10 @@
 strm.Printf("(%s)", GetTypeAsCString());
   if (dump_mask & eDumpOptionValue) {
 if (dump_mask & eDumpOptionType)
-  strm.PutCString(" = \"");
+  strm.PutCString(" = ");
 std::string escaped;
 EscapeBackticks(m_current_format, escaped);
-strm << escaped << '"';
+strm << '"' << escaped << '"';
   }
 }
 
Index: source/Interpreter/OptionValueFileSpecLIst.cpp
===
--- source/Interpreter/OptionValueFileSpecLIst.cpp
+++ source/Interpreter/OptionValueFileSpecLIst.cpp
@@ -25,16 +25,24 @@
   if (dump_mask & eDumpOptionType)
 strm.Printf("(%s)", GetTypeAsCString());
   if (dump_mask & eDumpOptionValue) {
-if (dump_mask & eDumpOptionType)
-  strm.Printf(" =%s", m_current_value.GetSize() > 0 ? "\n" : "");
-strm.IndentMore();
+const bool one_line = dump_mask & eDumpOptionCommand;
 const uint32_t size = m_current_value.GetSize();
+if (dump_mask & eDumpOptionType)
+  strm.Printf(" =%s",
+  (m_current_value.GetSize() > 0 && !one_line) ? "\n" : "");
+if (!one_line)
+  strm.IndentMore();
 for (uint32_t i = 0; i < size; ++i) {
-  strm.Indent();
-  strm.Printf("[%u]: ", i);
+  if (!one_line) {
+strm.Indent();
+strm.Printf("[%u]: ", i);
+  }
   m_current_value.GetFileSpecAtIndex(i).Dump(&strm);
+  if (one_line)
+strm << ' ';
 }
-strm.IndentLess();
+if (!one_line)
+  strm.IndentLess();
   }
 }
 
Index: source/Interpreter/OptionValueDictionary.cpp
===
--- source/Interpreter/OptionValueDictionary.cpp
+++ source/Interpreter/OptionValueDictionary.cpp
@@ -33,16 +33,23 @@
   strm.Printf("(%s)", GetTypeAsCString());
   }
   if (dump_mask & eDumpOptionValue) {
+const bool one_line = dump_mask & eDumpOptionCommand;
 if (dump_mask & eDumpOptionType)
   strm.PutCString(" =");
 
 collection::iterator pos, end = m_values.end();
 
-strm.IndentMore();
+if (!one_line)
+  strm.IndentMore();
 
 for (pos = m_values.begin(); pos != end; ++pos) {
   OptionValue *option_value = pos->second.get();
-  strm.EOL();
+
+  if (one_line)
+strm << ' ';
+  else
+strm.EOL();
+
   strm.Indent(pos->first.GetCString());
 
   const uint32_t extra_dump_options = m_raw_value_dump ? eDumpOptionRaw : 0;
@@ -74,7 +81,8 @@
 break;
   }
 }
-strm.IndentLess();
+if (!one_line)
+  strm.IndentLess();
   }
 }
 
Index: source/Interpreter/OptionValueArray.cpp
===
--- source/Interpreter/OptionValueArray.cpp
+++ source/Interpreter/OptionValueArray.cpp
@@ -31,13 +31,17 @@
   strm.Printf("(%s)", GetTypeAsCString());
   }
   if (dump_mask & eDumpOptionValue) {
-if (dump_mask & eDumpOptionType)
-  strm.Printf(" =%s", (

Re: [Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

2018-10-19 Thread Vedant Kumar via lldb-commits 
Hi Stella,

The logs are really helpful, thanks. This part is unexpected:

python   Finding frames between main and sink(), retn-pc=0x4005b8
python   GetCallEdges: Attempting to parse call site info for main
python   CollectCallEdges: Found call site info in main
python   CollectCallEdges: Found call origin: _Z5func2v (retn-PC: 
0x4005b8)
python   CollectCallEdges: Found call origin: _Z5func1v (retn-PC: 
0x4005c2)
python   FindInterveningFrames: found call with retn-PC = 0x800a38
python   FindInterveningFrames: found call with retn-PC = 0x800a42

LLDB finds a call from main() with return PC = 0x4005b8. It’s able to parse the 
call site info within main’s debug info. It finds a call from main() into func2 
with that exact return PC. But, the address calculation in 
CallEdge::GetReturnPCAddress adds the wrong slide to this return PC, giving 
0x800a38. This doesn’t match the PC value in the register state, so lldb can’t 
create a tail call frame.

I think Address::GetLoadAddress is the right API to use, but it’s clearly not 
achieving the right result here. I’ll experiment with replacing return PC 
addresses with function-local offsets to the instruction after a call. The idea 
would be to simply add this offset to the base address of the function, instead 
of doing a load address calculation.

vedant

> On Oct 18, 2018, at 9:47 AM, Stella Stamenova  wrote:
> 
> Hey Vedant,
>  
> I’ve attached the logs from Linux.
>  
> Most of the tests now pass on Windows with the exception of 
> TestSteppingOutWithArtificialFrames and TestTailCallFrameSBAPI. Both of these 
> attempt to get a specific frame by calling GetFrameAtIndex which only works 
> partially on Windows right now. I think we should mark these as XFAIL on 
> Windows and link them to: https://bugs.llvm.org/show_bug.cgi?id=26265 
> .
>  
> Thanks,
> -Stella
>  
> From: v...@apple.com   > 
> Sent: Tuesday, October 16, 2018 11:17 AM
> To: Stella Stamenova mailto:sti...@microsoft.com>>
> Cc: Frédéric Riss mailto:fr...@apple.com>>; 
> reviews+d50478+public+7e86b794a0909...@reviews.llvm.org 
> ; Adrian 
> Prantl mailto:apra...@apple.com>>; paul.robin...@sony.com 
> ; jdevliegh...@apple.com 
> ; Jim Ingham  >; ztur...@google.com ; 
> abidh@gmail.com ; teempe...@gmail.com 
> ; sgraen...@apple.com 
> ; mgr...@codeaurora.org 
> ; dblai...@gmail.com 
> ; lldb-commits@lists.llvm.org 
> 
> Subject: Re: [PATCH] D50478: Add support for artificial tail call frames
>  
>  
> 
> 
> On Oct 16, 2018, at 10:59 AM, Stella Stamenova  > wrote:
>  
> The windows error is because the names are different, as you expected:
> AssertionError: 'void sink(void)' != 'sink()'
> You can probably update the test to look for a different name on Windows 
> (though if I recall correctly, different versions of the DIA sdk provide 
> different detail on the names, so that might not be robust either) or look 
> for a substring in the full name.
>  
> I used a substring check in r344634.
>  
> 
> 
> I’ll look into the Linux error as well and let you know what I find.
>  
> Thank you very much! I really appreciate your help and patience with this.
>  
> The "step" logging channel should provide detailed information about what 
> goes wrong when parsing the DWARF for call site information and creating 
> artificial frames.
>  
> vedant
> 
> 
> From: v...@apple.com   > 
> Sent: Monday, October 15, 2018 8:34 PM
> To: Frédéric Riss mailto:fr...@apple.com>>
> Cc: reviews+d50478+public+7e86b794a0909...@reviews.llvm.org 
> ; Adrian 
> Prantl mailto:apra...@apple.com>>; paul.robin...@sony.com 
> ; jdevliegh...@apple.com 
> ; Jim Ingham  >; ztur...@google.com ; 
> Stella Stamenova mailto:sti...@microsoft.com>>; 
> abidh@gmail.com ; teempe...@gmail.com 
> ; sgraen...@apple.com 
> ; mgr...@codeaurora.org 
> ; dblai...@gmail.com 
> ; lldb-commits@lists.llvm.org 
> 
> Subject: Re: [PATCH] D50478: Add support for artificial tail call frames
> 
> 
> 
> On Oct 15, 2018, at 4:46 PM, Frédéric Riss  > wrote:
> 
> 
> 
> 
> On Oct 15, 2018, at 4:40 PM, Vedant Kumar

Re: [Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

2018-10-19 Thread Vedant Kumar via lldb-commits 


> On Oct 19, 2018, at 5:37 PM, Vedant Kumar  wrote:
> 
> Hi Stella,
> 
> The logs are really helpful, thanks. This part is unexpected:
> 
> python   Finding frames between main and sink(), retn-pc=0x4005b8
> python   GetCallEdges: Attempting to parse call site info for main
> python   CollectCallEdges: Found call site info in main
> python   CollectCallEdges: Found call origin: _Z5func2v (retn-PC: 
> 0x4005b8)
> python   CollectCallEdges: Found call origin: _Z5func1v (retn-PC: 
> 0x4005c2)
> python   FindInterveningFrames: found call with retn-PC = 0x800a38
> python   FindInterveningFrames: found call with retn-PC = 0x800a42
> 
> LLDB finds a call from main() with return PC = 0x4005b8. It’s able to parse 
> the call site info within main’s debug info. It finds a call from main() into 
> func2 with that exact return PC. But, the address calculation in 
> CallEdge::GetReturnPCAddress adds the wrong slide to this return PC, giving 
> 0x800a38. This doesn’t match the PC value in the register state, so lldb 
> can’t create a tail call frame.
> 
> I think Address::GetLoadAddress is the right API to use, but it’s clearly not 
> achieving the right result here. I’ll experiment with replacing return PC 
> addresses with function-local offsets to the instruction after a call. The 
> idea would be to simply add this offset to the base address of the function, 
> instead of doing a load address calculation.

FWIW this works just fine on Darwin, but I haven't verified that it works on 
Linux. Here's an illustrative patch: https://reviews.llvm.org/D53469 


vedant

> 
> vedant
> 
>> On Oct 18, 2018, at 9:47 AM, Stella Stamenova > > wrote:
>> 
>> Hey Vedant,
>>  
>> I’ve attached the logs from Linux.
>>  
>> Most of the tests now pass on Windows with the exception of 
>> TestSteppingOutWithArtificialFrames and TestTailCallFrameSBAPI. Both of 
>> these attempt to get a specific frame by calling GetFrameAtIndex which only 
>> works partially on Windows right now. I think we should mark these as XFAIL 
>> on Windows and link them to: https://bugs.llvm.org/show_bug.cgi?id=26265 
>> .
>>  
>> Thanks,
>> -Stella
>>  
>> From: v...@apple.com  > > 
>> Sent: Tuesday, October 16, 2018 11:17 AM
>> To: Stella Stamenova mailto:sti...@microsoft.com>>
>> Cc: Frédéric Riss mailto:fr...@apple.com>>; 
>> reviews+d50478+public+7e86b794a0909...@reviews.llvm.org 
>> ; Adrian 
>> Prantl mailto:apra...@apple.com>>; 
>> paul.robin...@sony.com ; 
>> jdevliegh...@apple.com ; Jim Ingham 
>> mailto:jing...@apple.com>>; ztur...@google.com 
>> ; abidh@gmail.com 
>> ; teempe...@gmail.com 
>> ; sgraen...@apple.com 
>> ; mgr...@codeaurora.org 
>> ; dblai...@gmail.com 
>> ; lldb-commits@lists.llvm.org 
>> 
>> Subject: Re: [PATCH] D50478: Add support for artificial tail call frames
>>  
>>  
>> 
>> 
>> On Oct 16, 2018, at 10:59 AM, Stella Stamenova > > wrote:
>>  
>> The windows error is because the names are different, as you expected:
>> AssertionError: 'void sink(void)' != 'sink()'
>> You can probably update the test to look for a different name on Windows 
>> (though if I recall correctly, different versions of the DIA sdk provide 
>> different detail on the names, so that might not be robust either) or look 
>> for a substring in the full name.
>>  
>> I used a substring check in r344634.
>>  
>> 
>> 
>> I’ll look into the Linux error as well and let you know what I find.
>>  
>> Thank you very much! I really appreciate your help and patience with this.
>>  
>> The "step" logging channel should provide detailed information about what 
>> goes wrong when parsing the DWARF for call site information and creating 
>> artificial frames.
>>  
>> vedant
>> 
>> 
>> From: v...@apple.com  > > 
>> Sent: Monday, October 15, 2018 8:34 PM
>> To: Frédéric Riss mailto:fr...@apple.com>>
>> Cc: reviews+d50478+public+7e86b794a0909...@reviews.llvm.org 
>> ; Adrian 
>> Prantl mailto:apra...@apple.com>>; 
>> paul.robin...@sony.com ; 
>> jdevliegh...@apple.com ; Jim Ingham 
>> mailto:jing...@apple.com>>; ztur...@google.com 
>> ; Stella Stamenova > >; abidh@gmail.com 
>> ; teempe...@gmail.com 
>> ; sgraen...@apple.com 
>>

[Lldb-commits] [PATCH] D52953: [lldb-mi] Implement -gdb-set breakpoint pending on/off

2018-10-19 Thread Marc-Andre Laperle via Phabricator via lldb-commits 
malaperle updated this revision to Diff 170301.
malaperle added a comment.

Clang-format it


https://reviews.llvm.org/D52953

Files:
  packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py
  tools/lldb-mi/MICmdCmdBreak.cpp
  tools/lldb-mi/MICmdCmdGdbSet.cpp
  tools/lldb-mi/MICmdCmdGdbSet.h
  tools/lldb-mi/MICmdCmdGdbShow.cpp
  tools/lldb-mi/MICmdCmdGdbShow.h
  tools/lldb-mi/MICmnResources.cpp
  tools/lldb-mi/MICmnResources.h

Index: tools/lldb-mi/MICmnResources.h
===
--- tools/lldb-mi/MICmnResources.h
+++ tools/lldb-mi/MICmnResources.h
@@ -264,11 +264,14 @@
   IDS_CMD_ERR_INFO_PRINTFN_NOT_FOUND,
   IDS_CMD_ERR_INFO_PRINTFN_FAILED,
   IDS_CMD_ERR_GDBSET_OPT_TARGETASYNC,
+  IDS_CMD_ERR_GDBSET_OPT_BREAKPOINT,
   IDS_CMD_ERR_GDBSET_OPT_SOLIBSEARCHPATH,
   IDS_CMD_ERR_GDBSET_OPT_PRINT_BAD_ARGS,
   IDS_CMD_ERR_GDBSET_OPT_PRINT_UNKNOWN_OPTION,
   IDS_CMD_ERR_GDBSHOW_OPT_PRINT_BAD_ARGS,
   IDS_CMD_ERR_GDBSHOW_OPT_PRINT_UNKNOWN_OPTION,
+  IDS_CMD_ERR_GDBSHOW_OPT_BREAKPOINT_BAD_ARGS,
+  IDS_CMD_ERR_GDBSHOW_OPT_BREAKPOINT_UNKNOWN_OPTION,
   IDS_CMD_ERR_EXPR_INVALID,
   IDS_CMD_ERR_ATTACH_FAILED,
   IDS_CMD_ERR_ATTACH_BAD_ARGS
Index: tools/lldb-mi/MICmnResources.cpp
===
--- tools/lldb-mi/MICmnResources.cpp
+++ tools/lldb-mi/MICmnResources.cpp
@@ -439,6 +439,8 @@
 {IDS_CMD_ERR_INFO_PRINTFN_FAILED, "The request '%s' failed."},
 {IDS_CMD_ERR_GDBSET_OPT_TARGETASYNC,
  "'target-async' expects \"on\" or \"off\""},
+{IDS_CMD_ERR_GDBSET_OPT_BREAKPOINT,
+ "'breakpoint' expects \"pending on\" or \"pending off\""},
 {IDS_CMD_ERR_GDBSET_OPT_SOLIBSEARCHPATH,
  "'solib-search-path' requires at least one argument"},
 {IDS_CMD_ERR_GDBSET_OPT_PRINT_BAD_ARGS,
@@ -449,6 +451,10 @@
  "'print' expects option-name and \"on\" or \"off\""},
 {IDS_CMD_ERR_GDBSHOW_OPT_PRINT_UNKNOWN_OPTION,
  "'print' error. The option '%s' not found"},
+{IDS_CMD_ERR_GDBSHOW_OPT_BREAKPOINT_BAD_ARGS,
+"'breakpoint' expects option-name"},
+{IDS_CMD_ERR_GDBSHOW_OPT_BREAKPOINT_UNKNOWN_OPTION,
+"'breakpoint' error. The option '%s' not found"},
 {IDS_CMD_ERR_EXPR_INVALID, "Failed to evaluate expression: %s"},
 {IDS_CMD_ERR_ATTACH_FAILED,
  "Command '%s'. Attach to process failed: %s"},
Index: tools/lldb-mi/MICmdCmdGdbShow.h
===
--- tools/lldb-mi/MICmdCmdGdbShow.h
+++ tools/lldb-mi/MICmdCmdGdbShow.h
@@ -80,6 +80,7 @@
   bool OptionFnLanguage(const CMIUtilString::VecString_t &vrWords);
   bool OptionFnDisassemblyFlavor(const CMIUtilString::VecString_t &vrWords);
   bool OptionFnFallback(const CMIUtilString::VecString_t &vrWords);
+  bool OptionFnBreakpoint(const CMIUtilString::VecString_t &vrWords);
 
   // Attributes:
 private:
Index: tools/lldb-mi/MICmdCmdGdbShow.cpp
===
--- tools/lldb-mi/MICmdCmdGdbShow.cpp
+++ tools/lldb-mi/MICmdCmdGdbShow.cpp
@@ -32,7 +32,8 @@
 {"print", &CMICmdCmdGdbShow::OptionFnPrint},
 {"language", &CMICmdCmdGdbShow::OptionFnLanguage},
 {"disassembly-flavor", &CMICmdCmdGdbShow::OptionFnDisassemblyFlavor},
-{"fallback", &CMICmdCmdGdbShow::OptionFnFallback}};
+{"fallback", &CMICmdCmdGdbShow::OptionFnFallback},
+{"breakpoint", &CMICmdCmdGdbShow::OptionFnBreakpoint}};
 
 //++
 //
@@ -347,6 +348,40 @@
   return MIstatus::success;
 }
 
+//++
+//
+// Details: Carry out work to complete the GDB show option 'breakpoint' to prepare
+//  and send back the requested information.
+// Type:Method.
+// Args:vrWords - (R) List of additional parameters used by this option.
+// Return:  MIstatus::success - Function succeeded.
+//  MIstatus::failure - Function failed.
+// Throws:  None.
+//--
+bool CMICmdCmdGdbShow::OptionFnBreakpoint(const CMIUtilString::VecString_t &vrWords) {
+if (vrWords.size() != 1) {
+m_bGbbOptionFnHasError = true;
+m_strGdbOptionFnError = MIRSRC(IDS_CMD_ERR_GDBSHOW_OPT_BREAKPOINT_BAD_ARGS);
+return MIstatus::failure;
+}
+
+const CMIUtilString strOption(vrWords[0]);
+if (CMIUtilString::Compare(strOption, "pending")) {
+if (!m_rLLDBDebugSessionInfo.SharedDataRetrieve("breakpoint.pending", m_strValue)) {
+if (m_strValue.empty())
+m_strValue = "off";
+}
+} else {
+m_bGbbOptionFnHasError = true;
+m_strGdbOptionFnError = CMIUtilString::Format(
+  MIRSRC(IDS_CMD_ERR_GDBSHOW_OPT_BREAKPOINT_UNKNOWN_OPTION),
+