[Lldb-commits] [lldb] 546f8f4 - [lldb/testsuite] Modernize 2 test Makefiles

2020-01-17 Thread Fred Riss via lldb-commits

Author: Fred Riss
Date: 2020-01-17T20:56:28-08:00
New Revision: 546f8f426463c7c22a3a8731803a501ff044ba20

URL: 
https://github.com/llvm/llvm-project/commit/546f8f426463c7c22a3a8731803a501ff044ba20
DIFF: 
https://github.com/llvm/llvm-project/commit/546f8f426463c7c22a3a8731803a501ff044ba20.diff

LOG: [lldb/testsuite] Modernize 2 test Makefiles

Those old Makefiles used completely ad-hoc rules for building files,
which means they didn't obey the test harness' variants.

They were somewhat tricky to update as they use very peculiar build
flags for some files. For this reason I was careful to compare the
build commands before and after the change, which is how I found the
discrepancy fixed by the previous commit.

While some of the make syntax used here might not be easy to grasp for
newcomers (per-target variable overrides), it seems better than to
have to repliacte the Makefile.rules logic for the test variants and
platform support.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/Makefile
lldb/packages/Python/lldbsuite/test/lang/objc/ivar-IMP/Makefile

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/Makefile 
b/lldb/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/Makefile
index 769920c28336..c39743d999da 100644
--- a/lldb/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/Makefile
+++ b/lldb/packages/Python/lldbsuite/test/lang/cpp/incomplete-types/Makefile
@@ -1,33 +1,22 @@
-CXX_SOURCES = main.cpp length.cpp a.cpp
-
-CFLAGS_LIMIT = -c $(CXXFLAGS)
-CFLAGS_NO_LIMIT = -c $(CXXFLAGS)
-
-ifneq (,$(findstring clang,$(CC)))
-  CFLAGS_LIMIT += -flimit-debug-info
-  CFLAGS_NO_LIMIT += -fno-limit-debug-info
-endif
+CXX_SOURCES := length.cpp a.o main.o
+EXE := nolimit
 
 all: limit nolimit
 
-limit: main.o length_limit.o a.o
-   $(CXX) main.o length_limit.o a.o -o limit $(LDFLAGS)
-
-nolimit: main.o length_nolimit.o a.o
-   $(CXX) main.o length_nolimit.o a.o -o nolimit $(LDFLAGS)
-
-main.o: main.cpp
-   $(CXX) $(CFLAGS_LIMIT) $(SRCDIR)/main.cpp -o main.o
-
-length_limit.o: length.cpp
-   $(CXX) $(CFLAGS_LIMIT) $(SRCDIR)/length.cpp -o length_limit.o
+include Makefile.rules
 
-length_nolimit.o: length.cpp
-   $(CXX) $(CFLAGS_NO_LIMIT) $(SRCDIR)/length.cpp -o length_nolimit.o
+# Force a.cpp to be built with no debug inforamtion
+a.o: CFLAGS = $(CFLAGS_NO_DEBUG)
 
-a.o: a.cpp
-   $(CXX) $(CFLAGS_NO_DEBUG) -c $(SRCDIR)/a.cpp -o a.o
+# The default testsuite setup forces -fno-limit-debug-info. Let's not rely on
+# CFLAGS_EXTRAS being passed after the default arguments. This rule makes
+# sure the variable used by Makefile.rules for this argument is cleared.
+main.o: NO_LIMIT_DEBUG_INFO_FLAGS = ""
+main.o: CFLAGS_EXTRAS = -flimit-debug-info
 
-clean: OBJECTS += limit nolimit length_limit.o length_nolimit.o 
length_limit.dwo length_nolimit.dwo
+limit: a.o main.o
+   mkdir -p build_limit
+   $(MAKE) -C $(BUILDDIR)/build_limit -f $(MAKEFILE_RULES) \
+   EXE=../limit CXX_SOURCES="length.cpp ../a.o ../main.o" \
+   CFLAGS_EXTRAS=-flimit-debug-info NO_LIMIT_DEBUG_INFO_FLAGS=""
 
-include Makefile.rules

diff  --git a/lldb/packages/Python/lldbsuite/test/lang/objc/ivar-IMP/Makefile 
b/lldb/packages/Python/lldbsuite/test/lang/objc/ivar-IMP/Makefile
index ba7e23acabab..5d920f442136 100644
--- a/lldb/packages/Python/lldbsuite/test/lang/objc/ivar-IMP/Makefile
+++ b/lldb/packages/Python/lldbsuite/test/lang/objc/ivar-IMP/Makefile
@@ -1,13 +1,8 @@
-CFLAGS := -g -O0
-CFLAGS_NO_DEBUG = 
+OBJC_SOURCES := myclass.m repro.m
+LD_EXTRAS := -framework Foundation
 
-all: aout
-
-aout: 
-   $(CC) $(CFLAGS_NO_DEBUG) $(SRCDIR)/myclass.m -c -o myclass.o
-   $(CC) $(CFLAGS) myclass.o $(SRCDIR)/repro.m -framework Foundation
+include Makefile.rules
 
-clean::
-   rm -f myclass.o
+# Force myclass.m to be compiled without debug info
+myclass.o: CFLAGS = $(CFLAGS_NO_DEBUG)
 
-include Makefile.rules



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


[Lldb-commits] [lldb] 509b788 - [lldb/Makefile.rules] Force the default target to be 'all'

2020-01-17 Thread Fred Riss via lldb-commits

Author: Fred Riss
Date: 2020-01-17T20:34:16-08:00
New Revision: 509b78883d4f8fdb13ccc754bba9782d51b477d8

URL: 
https://github.com/llvm/llvm-project/commit/509b78883d4f8fdb13ccc754bba9782d51b477d8
DIFF: 
https://github.com/llvm/llvm-project/commit/509b78883d4f8fdb13ccc754bba9782d51b477d8.diff

LOG: [lldb/Makefile.rules] Force the default target to be 'all'

The test harness invokes the test Makefiles with an explicit 'all'
target, but it's handy to be able to recursively call Makefile.rules
without speficying a goal.

Some time ago, we rewrote some tests in terms of recursive invocations
of Makefile.rules. It turns out this had an unintended side
effect. While using $(MAKE) for a recursive invocation passes all the
variables set on the command line down, it doesn't pass the make
goals. This means that those recursive invocations would invoke the
default rule. It turns out the default rule of Makefile.rules is not
'all', but $(EXE). This means that ti would work becuase the
executable is always needed, but it also means that the created
binaries would not follow some of the other top-level build
directives, like MAKE_DSYM.

Forcing 'all' to be the default target seems easier than making sure
all the invocations are correct going forward. This patch does this
using the .DEFAULT_GOAL directive rather than hoisting the 'all' rule
to be the first one of the file. It seems like this explicit approach
will be less prone to be broken in the future. Hopefully all the make
implementations we use support it.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/make/Makefile.rules

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules 
b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index f25d062ca43f..ecb75413a0c0 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -41,6 +41,13 @@ MAKEFILE_RULES := $(lastword $(MAKEFILE_LIST))
 THIS_FILE_DIR := $(shell dirname $(MAKEFILE_RULES))
 LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../
 
+# The test harness invokes the test Makefiles with an explicit 'all'
+# target, but its handy to be able to recursively call this Makefile
+# without speficying a goal. You almost certainly want to build 'all',
+# and not only the first target defined in this file (which might vary
+# according to varaible values).
+.DEFAULT_GOAL := all
+
 #--
 # If OS is not defined, use 'uname -s' to determine the OS name.
 #



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


[Lldb-commits] [PATCH] D72971: [debugserver] Share code between Enable/DisableHardwareWatchpoint (NFC)

2020-01-17 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

Yep, looks good.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D72971



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


[Lldb-commits] [PATCH] D72971: [debugserver] Share code between Enable/DisableHardwareWatchpoint (NFC)

2020-01-17 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added a reviewer: jasonmolenda.
Herald added subscribers: lldb-commits, jfb.
Herald added a project: LLDB.

This extract the common functionality of enabling and disabling hardware 
watchpoints into a single function.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D72971

Files:
  lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp
  lldb/tools/debugserver/source/MacOSX/MachThreadList.h

Index: lldb/tools/debugserver/source/MacOSX/MachThreadList.h
===
--- lldb/tools/debugserver/source/MacOSX/MachThreadList.h
+++ lldb/tools/debugserver/source/MacOSX/MachThreadList.h
@@ -83,6 +83,14 @@
   typedef collection::iterator iterator;
   typedef collection::const_iterator const_iterator;
 
+  enum class HardwareBreakpointAction {
+EnableWatchpoint,
+DisableWatchpoint,
+  };
+
+  uint32_t DoHardwareBreakpointAction(const DNBBreakpoint *wp,
+  HardwareBreakpointAction action) const;
+
   uint32_t UpdateThreadList(MachProcess *process, bool update,
 collection *num_threads = NULL);
   //  const_iterator  FindThreadByID (thread_t tid) const;
Index: lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp
===
--- lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp
+++ lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp
@@ -502,65 +502,56 @@
   return false;
 }
 
+uint32_t MachThreadList::DoHardwareBreakpointAction(
+const DNBBreakpoint *wp, HardwareBreakpointAction action) const {
+  if (wp == NULL)
+return INVALID_NUB_HW_INDEX;
+
+  uint32_t hw_index = INVALID_NUB_HW_INDEX;
+  PTHREAD_MUTEX_LOCKER(locker, m_threads_mutex);
+  const size_t num_threads = m_threads.size();
+  // On Mac OS X we have to prime the control registers for new threads.  We do
+  // this using the control register data for the first thread, for lack of a
+  // better way of choosing.
+  bool also_set_on_task = true;
+  for (uint32_t idx = 0; idx < num_threads; ++idx) {
+switch (action) {
+case HardwareBreakpointAction::EnableWatchpoint:
+  hw_index = m_threads[idx]->EnableHardwareWatchpoint(wp, also_set_on_task);
+  break;
+case HardwareBreakpointAction::DisableWatchpoint:
+  hw_index =
+  m_threads[idx]->DisableHardwareWatchpoint(wp, also_set_on_task);
+  break;
+}
+if (hw_index == INVALID_NUB_HW_INDEX) {
+  // We know that idx failed for some reason.  Let's rollback the
+  // transaction for [0, idx).
+  for (uint32_t i = 0; i < idx; ++i)
+m_threads[i]->RollbackTransForHWP();
+  return INVALID_NUB_HW_INDEX;
+}
+also_set_on_task = false;
+  }
+  // Notify each thread to commit the pending transaction.
+  for (uint32_t idx = 0; idx < num_threads; ++idx)
+m_threads[idx]->FinishTransForHWP();
+  return hw_index;
+}
+
 // DNBWatchpointSet() -> MachProcess::CreateWatchpoint() ->
 // MachProcess::EnableWatchpoint()
 // -> MachThreadList::EnableHardwareWatchpoint().
 uint32_t
 MachThreadList::EnableHardwareWatchpoint(const DNBBreakpoint *wp) const {
-  uint32_t hw_index = INVALID_NUB_HW_INDEX;
-  if (wp != NULL) {
-PTHREAD_MUTEX_LOCKER(locker, m_threads_mutex);
-const size_t num_threads = m_threads.size();
-// On Mac OS X we have to prime the control registers for new threads.  We
-// do this
-// using the control register data for the first thread, for lack of a
-// better way of choosing.
-bool also_set_on_task = true;
-for (uint32_t idx = 0; idx < num_threads; ++idx) {
-  if ((hw_index = m_threads[idx]->EnableHardwareWatchpoint(
-   wp, also_set_on_task)) == INVALID_NUB_HW_INDEX) {
-// We know that idx failed for some reason.  Let's rollback the
-// transaction for [0, idx).
-for (uint32_t i = 0; i < idx; ++i)
-  m_threads[i]->RollbackTransForHWP();
-return INVALID_NUB_HW_INDEX;
-  }
-  also_set_on_task = false;
-}
-// Notify each thread to commit the pending transaction.
-for (uint32_t idx = 0; idx < num_threads; ++idx)
-  m_threads[idx]->FinishTransForHWP();
-  }
-  return hw_index;
+  return DoHardwareBreakpointAction(wp,
+HardwareBreakpointAction::EnableWatchpoint);
 }
 
 bool MachThreadList::DisableHardwareWatchpoint(const DNBBreakpoint *wp) const {
-  if (wp != NULL) {
-PTHREAD_MUTEX_LOCKER(locker, m_threads_mutex);
-const size_t num_threads = m_threads.size();
-
-// On Mac OS X we have to prime the control registers for new threads.  We
-// do this
-// using the control register data for the first thread, for lack of a
-// better way of choosing.
-bool also_set_on_task = true;
-for (uint32_t idx = 0; idx < num_threads; ++idx) {
-  if (!m_threads[idx]->DisableHardwareWatchpoint(wp, also_set_on_task)) {
-// 

[Lldb-commits] [PATCH] D72946: [lldb] Remove ClangASTImporter reference from Target

2020-01-17 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment.

In D72946#1827301 , @teemperor wrote:

> I wish we could do this without a global map. Also the ClangASTImporter 
> shouldn't have a dependency on Target (I'm actually surprised this compiles 
> without an additional include).
>
> I'm not sure where the perfect place for the Target's ClangASTImporter is, 
> but putting it in the `ClangPersistentVariables` would solve your problem of 
> getting it out of Target and doesn't need a global map. Also in general the 
> ClangASTImporter of the Target is used for copying stuff to the scratch 
> context, so having it in the `ClangPersistentVariables` makes some sense.


Oh, yes, I'm not a fan of the global map either. I agree that ClangASTImporter 
shouldn't depend on Target but I think that's better than the other way around. 
I also agree that putting it into `ClangPersistentVariables` makes some sense. 
I'll give it a shot and update this when I get the chance. Thanks for pointing 
that out!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72946



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


[Lldb-commits] [PATCH] D72963: When darwin-debug exec's inferior suspended, make debugserver know that suspend count is higher than normal

2020-01-17 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda created this revision.
jasonmolenda added a reviewer: jingham.
jasonmolenda added a project: LLDB.
Herald added a subscriber: mgorny.

darwin-debug is used to launch a binary while setting the current working 
directory/env vars/architecture, exec'ing it stopped so lldb can attach to it.  
This is used on macOS to launch a binary in a new Terminal window via 
AppleScript.  If lldb attaches to the inferior, stopped on the first 
instruction, all is good.  But if lldb attaches before the inferior is running, 
it will attach to darwin-debug, see the Exec mach exception when the inferior 
is run, but instead of the normal suspend count of 1 at this point, because 
darwin-debug asked it to be launch suspended, the suspend count will be 2.  
debugserver needs to double-resume the inferior to make it run.

This patch adds a specially named segment to darwin-debug so that it can be 
detected unambiguously by debugserver.

In debugserver, if we attach to a process with the specially named segment, set 
a flag that will be checked the next time we received an Exec mach exception.  
When that Exec comes in, set a flag that will be checked the next time we go to 
resume the inferior to indicate that we need to resume it twice to allow it to 
run.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72963

Files:
  lldb/tools/darwin-debug/CMakeLists.txt
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/MacOSX/MachTask.h
  lldb/tools/debugserver/source/MacOSX/MachTask.mm

Index: lldb/tools/debugserver/source/MacOSX/MachTask.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachTask.mm
+++ lldb/tools/debugserver/source/MacOSX/MachTask.mm
@@ -73,7 +73,8 @@
 //--
 MachTask::MachTask(MachProcess *process)
 : m_process(process), m_task(TASK_NULL), m_vm_memory(),
-  m_exception_thread(0), m_exception_port(MACH_PORT_NULL) {
+  m_exception_thread(0), m_exception_port(MACH_PORT_NULL),
+  m_exec_will_be_suspended(false), m_do_double_resume(false) {
   memset(_exc_port_info, 0, sizeof(m_exc_port_info));
 }
 
@@ -107,6 +108,14 @@
   err = BasicInfo(task, _info);
 
   if (err.Success()) {
+if (m_do_double_resume && task_info.suspend_count == 2) {
+  err = ::task_resume(task);
+  if (DNBLogCheckLogBit(LOG_TASK) || err.Fail())
+err.LogThreaded("::task_resume double-resume after exec-start-stopped "
+"( target_task = 0x%4.4x )", task);
+}
+m_do_double_resume = false;
+  
 // task_resume isn't counted like task_suspend calls are, are, so if the
 // task is not suspended, don't try and resume it since it is already
 // running
@@ -139,6 +148,8 @@
   m_task = TASK_NULL;
   m_exception_thread = 0;
   m_exception_port = MACH_PORT_NULL;
+  m_exec_will_be_suspended = false;
+  m_do_double_resume = false;
 }
 
 //--
@@ -665,6 +676,9 @@
 err.LogThreaded("::mach_port_deallocate ( task = 0x%4.4x, name = 0x%4.4x )",
 task_self, exception_port);
 
+  m_exec_will_be_suspended = false;
+  m_do_double_resume = false;
+
   return err.Status();
 }
 
@@ -974,4 +988,14 @@
 void MachTask::TaskPortChanged(task_t task)
 {
   m_task = task;
+
+  // If we've just exec'd to a new process, and it
+  // is started suspended, we'll need to do two
+  // task_resume's to get the inferior process to
+  // continue.
+  if (m_exec_will_be_suspended)
+m_do_double_resume = true;
+  else
+m_do_double_resume = false;
+  m_exec_will_be_suspended = false;
 }
Index: lldb/tools/debugserver/source/MacOSX/MachTask.h
===
--- lldb/tools/debugserver/source/MacOSX/MachTask.h
+++ lldb/tools/debugserver/source/MacOSX/MachTask.h
@@ -85,6 +85,7 @@
   const MachProcess *Process() const { return m_process; }
 
   nub_size_t PageSize();
+  void TaskWillExecProcessesSuspended() { m_exec_will_be_suspended = true; }
 
 protected:
   MachProcess *m_process; // The mach process that owns this MachTask
@@ -97,6 +98,12 @@
 // need it
   mach_port_t m_exception_port; // Exception port on which we will receive child
 // exceptions
+  bool m_exec_will_be_suspended; // If this task exec's another process, that
+// process will be launched suspended and we will
+// need to execute one extra Resume to get it
+// to progress from dyld_start.
+  bool m_do_double_resume;  // next time we task_resume(), do it twice to
+// fix a too-high suspend count.
 
   typedef std::map allocation_collection;
   allocation_collection m_allocations;
Index: 

[Lldb-commits] [PATCH] D72813: Fixes to lldb's eLaunchFlagLaunchInTTY feature on macOS

2020-01-17 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda abandoned this revision.
jasonmolenda added a comment.

Taking a different approach to fixing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72813



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


Re: [Lldb-commits] [PATCH] D72953: Fix the handling of unnamed bit-fields when parsing DWARF

2020-01-17 Thread Jim Ingham via lldb-commits


> On Jan 17, 2020, at 2:24 PM, Raphael Isemann via Phabricator 
>  wrote:
> 
> teemperor added a comment.
> 
> (Just some quick comments, will review this properly during normal working 
> hours)
> 
> Without this fix debugging Clang with LLDB is essentially impossible, so I'm 
> in favour of landing this with as few pre-commit refactorings as possible to 
> make backporting easier and getting this in ASAP. We probably also want to 
> ping Hans to get this into the 10 release branch (which was created 2 days 
> ago, so that's just a simple cherry-pick).

If you want to make back porting easier, you might not want to use expect_expr 
in the tests, since that would require back porting that as well as this patch?

Jim


> 
> Also you might want to check out the recently added `expect_expr` command 
> instead of calling `expect` (see the inline comment for an example). The API 
> currently doesn't support the whole fuzzy substr matching (which is on 
> purpose) so you might not be able to use it everywhere (at least the current 
> version).
> 
> 
> 
> 
> Comment at: 
> lldb/packages/Python/lldbsuite/test/lang/c/bitfields/TestBitfields.py:207
> +
> +self.expect("expr (lba.a)", VARIABLES_DISPLAYED_CORRECTLY,
> +substrs=['unsigned int', '2'])
> 
> We can do this since last week: `self.expect_expr("(lba.a)", 
> result_type="unsigned int", result_value="2")` which is a much more safer and 
> convenient way of doing this.
> 
> 
> 
> Comment at: lldb/packages/Python/lldbsuite/test/lang/c/bitfields/main.c:128
> +  uint64_t HasNonTrivialToPrimitiveDestructCUnion : 1;
> +  uint64_t HasNonTrivialToPrimitiveCopyCUnion : 1;
> +};
> 
> I would prefer generic names as otherwise Clang folks randomly see the test 
> when they grep for usages of these variables by name in the LLVM repo.
> 
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D72953/new/
> 
> https://reviews.llvm.org/D72953
> 
> 
> 

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


[Lldb-commits] [PATCH] D72953: Fix the handling of unnamed bit-fields when parsing DWARF

2020-01-17 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

(Just some quick comments, will review this properly during normal working 
hours)

Without this fix debugging Clang with LLDB is essentially impossible, so I'm in 
favour of landing this with as few pre-commit refactorings as possible to make 
backporting easier and getting this in ASAP. We probably also want to ping Hans 
to get this into the 10 release branch (which was created 2 days ago, so that's 
just a simple cherry-pick).

Also you might want to check out the recently added `expect_expr` command 
instead of calling `expect` (see the inline comment for an example). The API 
currently doesn't support the whole fuzzy substr matching (which is on purpose) 
so you might not be able to use it everywhere (at least the current version).




Comment at: 
lldb/packages/Python/lldbsuite/test/lang/c/bitfields/TestBitfields.py:207
+
+self.expect("expr (lba.a)", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['unsigned int', '2'])

We can do this since last week: `self.expect_expr("(lba.a)", 
result_type="unsigned int", result_value="2")` which is a much more safer and 
convenient way of doing this.



Comment at: lldb/packages/Python/lldbsuite/test/lang/c/bitfields/main.c:128
+  uint64_t HasNonTrivialToPrimitiveDestructCUnion : 1;
+  uint64_t HasNonTrivialToPrimitiveCopyCUnion : 1;
+};

I would prefer generic names as otherwise Clang folks randomly see the test 
when they grep for usages of these variables by name in the LLVM repo.


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

https://reviews.llvm.org/D72953



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


[Lldb-commits] [lldb] a93aa53 - [lldb/Docs] Fix formatting for the variable formatting page

2020-01-17 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-01-17T14:17:26-08:00
New Revision: a93aa5347641159aa0d2d48dda9e1a51b2273462

URL: 
https://github.com/llvm/llvm-project/commit/a93aa5347641159aa0d2d48dda9e1a51b2273462
DIFF: 
https://github.com/llvm/llvm-project/commit/a93aa5347641159aa0d2d48dda9e1a51b2273462.diff

LOG: [lldb/Docs] Fix formatting for the variable formatting page

Added: 


Modified: 
lldb/docs/use/variable.rst

Removed: 




diff  --git a/lldb/docs/use/variable.rst b/lldb/docs/use/variable.rst
index f3bde2de4144..13a56637ecea 100644
--- a/lldb/docs/use/variable.rst
+++ b/lldb/docs/use/variable.rst
@@ -366,12 +366,13 @@ simply say ${var.z} because that symbol refers to the 
pointer z. In order to
 dereference it and get the pointed value, you should say ``${*var.z}``. The
 ``${*var`` tells LLDB to get the object that the expression paths leads to, and
 then dereference it. In this example is it equivalent to ``*(bObject.z)`` in
-C/C++ syntax. Because . and -> operators can both be used, there is no need to
-have dereferences in the middle of an expression path (e.g. you do not need to
-type ``${*(var.x).x}``) to read A::x as contained in ``*(B::x)``. To achieve
-that effect you can simply write ``${var.x->x}``, or even ``${var.x.x}``. The
-``*`` operator only binds to the result of the whole expression path, rather
-than piecewise, and there is no way to use parentheses to change that behavior.
+C/C++ syntax. Because ``.`` and ``->`` operators can both be used, there is no
+need to have dereferences in the middle of an expression path (e.g. you do not
+need to type ``${*(var.x).x}``) to read A::x as contained in ``*(B::x)``. To
+achieve that effect you can simply write ``${var.x->x}``, or even
+``${var.x.x}``. The ``*`` operator only binds to the result of the whole
+expression path, rather than piecewise, and there is no way to use parentheses
+to change that behavior.
 
 Of course, a summary string can contain more than one ${var specifier, and can
 use ``${var`` and ``${*var`` specifiers together.
@@ -573,7 +574,7 @@ the pointer value. However, because pointers have no notion 
of their size, the
 empty brackets [] operator does not work, and you must explicitly provide
 higher and lower bounds.
 
-In general, LLDB needs the square brackets operator [] in order to handle
+In general, LLDB needs the square brackets ``operator []`` in order to handle
 arrays and pointers correctly, and for pointers it also needs a range. However,
 a few special cases are defined to make your life easier:
 
@@ -882,9 +883,9 @@ As a shortcut for this, you can inherit from 
lldb.SBSyntheticValueProvider, and
 just define get_value as other methods are defaulted in the superclass as
 returning default no-children responses.
 
-If a synthetic child provider supplies a special child named $$dereference$$
-then it will be used when evaluating opertaor* and operator-> in the frame
-variable command and related SB API functions.
+If a synthetic child provider supplies a special child named
+``$$dereference$$`` then it will be used when evaluating ``operator *`` and
+``operator ->`` in the frame variable command and related SB API functions.
 
 For examples of how synthetic children are created, you are encouraged to look
 at examples/synthetic in the LLDB trunk. Please, be aware that the code in
@@ -960,10 +961,10 @@ expression:
Error [IRForTarget]: Call to a function '_ZNSt33vector >ixEm' that is not present in the target
error: Couldn't convert the expression to DWARF
 
-The reason for this is that classes might have an overloaded operator [], or
-other special provisions and the expression command chooses to ignore synthetic
-children in the interest of equivalency with code you asked to have compiled
-from source.
+The reason for this is that classes might have an overloaded ``operator []``,
+or other special provisions and the expression command chooses to ignore
+synthetic children in the interest of equivalency with code you asked to have
+compiled from source.
 
 Filters
 ---



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


[Lldb-commits] [PATCH] D72920: [lldb/DWARF] Simplify DWARFDebugInfoEntry::LookupAddress

2020-01-17 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

LGTM. Much cleaner using the newer DWARFDIE code. Not sure if we can unit test 
this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72920



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


[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

2020-01-17 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 238886.
JDevlieghere added a comment.

Add PExpect test


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

https://reviews.llvm.org/D72748

Files:
  lldb/include/lldb/Core/Debugger.h
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/main.c
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/source.lldb
  lldb/source/Core/Debugger.cpp

Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -895,23 +895,62 @@
 }
 
 void Debugger::RunIOHandlers() {
+  IOHandlerSP reader_sp = m_io_handler_stack.Top();
   while (true) {
-IOHandlerSP reader_sp(m_io_handler_stack.Top());
 if (!reader_sp)
   break;
 
 reader_sp->Run();
+{
+  std::lock_guard guard(
+  m_io_handler_synchronous_mutex);
+
+  // Remove all input readers that are done from the top of the stack
+  while (true) {
+IOHandlerSP top_reader_sp = m_io_handler_stack.Top();
+if (top_reader_sp && top_reader_sp->GetIsDone())
+  PopIOHandler(top_reader_sp);
+else
+  break;
+  }
+  reader_sp = m_io_handler_stack.Top();
+}
+  }
+  ClearIOHandlers();
+}
+
+void Debugger::RunIOHandlerSync(const IOHandlerSP _sp) {
+  std::lock_guard guard(m_io_handler_synchronous_mutex);
+
+  PushIOHandler(reader_sp);
+  IOHandlerSP top_reader_sp = reader_sp;
 
-// Remove all input readers that are done from the top of the stack
+  while (top_reader_sp) {
+if (!top_reader_sp)
+  break;
+
+top_reader_sp->Run();
+
+// Don't unwind past the starting point.
+if (top_reader_sp.get() == reader_sp.get()) {
+  if (PopIOHandler(reader_sp))
+break;
+}
+
+// If we pushed new IO handlers, pop them if they're done or restart the
+// loop to run them if they're not.
 while (true) {
-  IOHandlerSP top_reader_sp = m_io_handler_stack.Top();
-  if (top_reader_sp && top_reader_sp->GetIsDone())
+  top_reader_sp = m_io_handler_stack.Top();
+  if (top_reader_sp && top_reader_sp->GetIsDone()) {
 PopIOHandler(top_reader_sp);
-  else
+// Don't unwind past the starting point.
+if (top_reader_sp.get() == reader_sp.get())
+  return;
+  } else {
 break;
+  }
 }
   }
-  ClearIOHandlers();
 }
 
 bool Debugger::IsTopIOHandler(const lldb::IOHandlerSP _sp) {
@@ -950,28 +989,6 @@
   PushIOHandler(reader_sp, cancel_top_handler);
 }
 
-void Debugger::RunIOHandlerSync(const IOHandlerSP _sp) {
-  PushIOHandler(reader_sp);
-
-  IOHandlerSP top_reader_sp = reader_sp;
-  while (top_reader_sp) {
-top_reader_sp->Run();
-
-if (top_reader_sp.get() == reader_sp.get()) {
-  if (PopIOHandler(reader_sp))
-break;
-}
-
-while (true) {
-  top_reader_sp = m_io_handler_stack.Top();
-  if (top_reader_sp && top_reader_sp->GetIsDone())
-PopIOHandler(top_reader_sp);
-  else
-break;
-}
-  }
-}
-
 void Debugger::AdoptTopIOHandlerFilesIfInvalid(FileSP , StreamFileSP ,
StreamFileSP ) {
   // Before an IOHandler runs, it must have in/out/err streams. This function
Index: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/source.lldb
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/source.lldb
@@ -0,0 +1 @@
+script foo = 95126
Index: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/main.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/main.c
@@ -0,0 +1,4 @@
+int main (int argc, char const *argv[])
+{
+  return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py
@@ -0,0 +1,43 @@
+"""
+Test completion in our IOHandlers.
+"""
+
+import os
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+
+class BreakpointCallbackCommandSource(PExpectTest):

[Lldb-commits] [PATCH] D72946: [lldb] Remove ClangASTImporter reference from Target

2020-01-17 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Is an AST importer specific to a target? Can we just put it into the Clang AST 
type system subclass and create it lazily?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72946



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


[Lldb-commits] [PATCH] D72953: Fix the handling of unnamed bit-fields when parsing DWARF

2020-01-17 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Very cool. I think we can improve the code quite a bit while we're here.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2427
   if (attributes.ExtractFormValueAtIndex(i, form_value)) {
+// AT_data_member_location indicates the byte offset of the
+// word from the base address of the structure.

`DW_AT_data_member_location`.
`AT_foo` makes git grep less useful



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2674
 
-  if (anon_field_info.IsValid()) {
+  if (unnamed_field_info.IsValid()) {
+if (data_bit_offset != UINT64_MAX)

Can you please add comments explaining what each of these cases handle?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2691
+  } else
+field_bit_offset = this_field_info.bit_offset;
 }

Looks like this could benefit from a refactoring with early exits. Can be a 
follow-up commit though.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2696
+last_bitfield_info = this_field_info;
+  } else {
+last_bitfield_info.Clear();

for example, it's not obvious what happened to end up in this else branch



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h:174
+  struct FieldInfo {
 uint64_t bit_size;
 uint64_t bit_offset;

Not you fault, but: This data structure is a disaster waiting to happen. 
Instead of having magic sentinel values, should we use an Optional 
that is always valid if it exists? Or should the members be Optionals?


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

https://reviews.llvm.org/D72953



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


[Lldb-commits] [PATCH] D72953: Fix the handling of unnamed bit-fields when parsing DWARF

2020-01-17 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/lang/c/bitfields/TestBitfields.py:155
+'(int:32)  = ',
+'(unsigned int:20) a =',
+])

Why don't we inspect the values of fields, too?


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

https://reviews.llvm.org/D72953



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


[Lldb-commits] [PATCH] D72953: Fix the handling of unnamed bit-fields when parsing DWARF

2020-01-17 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik created this revision.
shafik added reviewers: aprantl, teemperor, jingham.

We ran into an assert when debugging clang and performing an expression on a 
class derived from `DeclContext`. The assert was indicating we were getting the 
offsets wrong for `RecordDeclBitfields`. We were getting both the size and 
offset of unnamed bit-field members wrong. We could fix this case with a quick 
change but as I extended the test suite to include more combinations we kept 
finding more cases that were being handled incorrectly. A fix that handled all 
the new cases as well as the cases already covered required a refactor of the 
existing technique.

I removed a duplicate of `BitfieldInfo` and renamed `BitfieldInfo` -> 
`FieldInfo` since it is being used for both bit-fields and non-bit-fields. I 
extended `TestBitfields.py` to cover five more cases we uncovered while fixing 
this bug.


https://reviews.llvm.org/D72953

Files:
  lldb/packages/Python/lldbsuite/test/lang/c/bitfields/TestBitfields.py
  lldb/packages/Python/lldbsuite/test/lang/c/bitfields/main.c
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -170,11 +170,11 @@
   lldb::ModuleSP GetModuleForType(const DWARFDIE );
 
 private:
-  struct BitfieldInfo {
+  struct FieldInfo {
 uint64_t bit_size;
 uint64_t bit_offset;
 
-BitfieldInfo()
+FieldInfo()
 : bit_size(LLDB_INVALID_ADDRESS), bit_offset(LLDB_INVALID_ADDRESS) {}
 
 void Clear() {
@@ -194,7 +194,7 @@
 // + size.
 return (bit_size + bit_offset) <= next_bit_offset;
   } else {
-// If the this BitfieldInfo is not valid, then any offset isOK
+// If the this FieldInfo is not valid, then any offset isOK
 return true;
   }
 }
@@ -208,7 +208,7 @@
 lldb::AccessType _accessibility,
 DelayedPropertyList _properties,
 lldb_private::ClangASTImporter::LayoutInfo _info,
-BitfieldInfo _field_info);
+FieldInfo _bitfield_info, FieldInfo _field_info);
 
   bool CompleteRecordType(const DWARFDIE , lldb_private::Type *type,
   lldb_private::CompilerType _type);
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -85,35 +85,6 @@
   return false;
 }
 
-struct BitfieldInfo {
-  uint64_t bit_size;
-  uint64_t bit_offset;
-
-  BitfieldInfo()
-  : bit_size(LLDB_INVALID_ADDRESS), bit_offset(LLDB_INVALID_ADDRESS) {}
-
-  void Clear() {
-bit_size = LLDB_INVALID_ADDRESS;
-bit_offset = LLDB_INVALID_ADDRESS;
-  }
-
-  bool IsValid() const {
-return (bit_size != LLDB_INVALID_ADDRESS) &&
-   (bit_offset != LLDB_INVALID_ADDRESS);
-  }
-
-  bool NextBitfieldOffsetIsValid(const uint64_t next_bit_offset) const {
-if (IsValid()) {
-  // This bitfield info is valid, so any subsequent bitfields must not
-  // overlap and must be at a higher bit offset than any previous bitfield
-  // + size.
-  return (bit_size + bit_offset) <= next_bit_offset;
-} else {
-  // If the this BitfieldInfo is not valid, then any offset isOK
-  return true;
-}
-  }
-};
 
 ClangASTImporter ::GetClangASTImporter() {
   if (!m_clang_ast_importer_up) {
@@ -2419,7 +2390,7 @@
 lldb::AccessType _accessibility,
 DelayedPropertyList _properties,
 lldb_private::ClangASTImporter::LayoutInfo _info,
-BitfieldInfo _field_info) {
+FieldInfo _bitfield_info, FieldInfo _field_info) {
   ModuleSP module_sp = parent_die.GetDWARF()->GetObjectFile()->GetModule();
   const dw_tag_t tag = die.Tag();
   // Get the parent byte size so we can verify any members will fit
@@ -2453,6 +2424,14 @@
   const dw_attr_t attr = attributes.AttributeAtIndex(i);
   DWARFFormValue form_value;
   if (attributes.ExtractFormValueAtIndex(i, form_value)) {
+// AT_data_member_location indicates the byte offset of the
+// word from the base address of the structure.
+//
+// AT_bit_offset indicates how many bits into the word
+// (according to the host endianness) the low-order bit of the
+// field starts.  AT_bit_offset can be negative.
+//
+// AT_bit_size indicates the size of the field in bits.
 switch (attr) {
 case DW_AT_name:
   name = form_value.AsCString();
@@ -2603,35 +2582,24 @@
   Type *member_type = die.ResolveTypeUID(encoding_form.Reference());
 
   

[Lldb-commits] [lldb] 510758d - debugserver: Pass -arch flags to mig invocation as needed

2020-01-17 Thread Vedant Kumar via lldb-commits

Author: Vedant Kumar
Date: 2020-01-17T13:11:54-08:00
New Revision: 510758dae2a8fa4b0b26dea89d4d1efd576b8ad6

URL: 
https://github.com/llvm/llvm-project/commit/510758dae2a8fa4b0b26dea89d4d1efd576b8ad6
DIFF: 
https://github.com/llvm/llvm-project/commit/510758dae2a8fa4b0b26dea89d4d1efd576b8ad6.diff

LOG: debugserver: Pass -arch flags to mig invocation as needed

Specify -isysroot and any necessary -arch flags in the `mig` invocation
when CMAKE_OSX_ARCHITECTURES is set (needed for the bridgeOS build).

Added: 


Modified: 
lldb/tools/debugserver/source/CMakeLists.txt

Removed: 




diff  --git a/lldb/tools/debugserver/source/CMakeLists.txt 
b/lldb/tools/debugserver/source/CMakeLists.txt
index 607ac11ac96f..5b604126e47d 100644
--- a/lldb/tools/debugserver/source/CMakeLists.txt
+++ b/lldb/tools/debugserver/source/CMakeLists.txt
@@ -132,8 +132,15 @@ set(generated_mach_interfaces
   ${CMAKE_CURRENT_BINARY_DIR}/mach_excServer.c
   ${CMAKE_CURRENT_BINARY_DIR}/mach_excUser.c
   )
+
+set(MIG_ARCH_FLAGS "")
+foreach(ARCH ${CMAKE_OSX_ARCHITECTURES})
+  set(MIG_ARCH_FLAGS "${MIG_ARCH_FLAGS} -arch ${ARCH}")
+endforeach()
+separate_arguments(MIG_ARCH_FLAGS_SEPARTED NATIVE_COMMAND "${MIG_ARCH_FLAGS}")
+
 add_custom_command(OUTPUT ${generated_mach_interfaces}
-  COMMAND mig ${CMAKE_CURRENT_SOURCE_DIR}/MacOSX/dbgnub-mig.defs
+  VERBATIM COMMAND mig ${MIG_ARCH_FLAGS_SEPARTED} -isysroot 
${CMAKE_OSX_SYSROOT} ${CMAKE_CURRENT_SOURCE_DIR}/MacOSX/dbgnub-mig.defs
   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/MacOSX/dbgnub-mig.defs
   )
 



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


[Lldb-commits] [PATCH] D72946: [lldb] Remove ClangASTImporter reference from Target

2020-01-17 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

I wish we could do this without a global map. Also the ClangASTImporter 
shouldn't have a dependency on Target (I'm actually surprised this compiles 
without an additional include).

I'm not sure where the perfect place for the Target's ClangASTImporter is, but 
putting it in the `ClangPersistentVariables` would solve your problem of 
getting it out of Target and doesn't need a global map. Also in general the 
ClangASTImporter of the Target is used for copying stuff to the scratch 
context, so having it in the `ClangPersistentVariables` makes some sense.




Comment at: lldb/include/lldb/Symbol/ClangASTImporter.h:328
 
+
 } // namespace lldb_private

unrelated change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72946



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


[Lldb-commits] [lldb] c17aee6 - Revert "Rename DW_AT_LLVM_isysroot to DW_AT_LLVM_sysroot"

2020-01-17 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2020-01-17T12:52:36-08:00
New Revision: c17aee67f1007426fb12f4081183bb8ec5dc3d15

URL: 
https://github.com/llvm/llvm-project/commit/c17aee67f1007426fb12f4081183bb8ec5dc3d15
DIFF: 
https://github.com/llvm/llvm-project/commit/c17aee67f1007426fb12f4081183bb8ec5dc3d15.diff

LOG: Revert "Rename DW_AT_LLVM_isysroot to DW_AT_LLVM_sysroot"

This reverts commit 12e479475a896f664fb721f98c2d6805185ac352.

I accidentally landed this patch with the wrong commit message ...

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGen/debug-nvptx.c
clang/test/Modules/debug-info-moduleimport.m
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
llvm/bindings/go/llvm/dibuilder.go
llvm/include/llvm-c/DebugInfo.h
llvm/include/llvm/IR/DIBuilder.h
llvm/include/llvm/IR/DebugInfoMetadata.h
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/MetadataLoader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/IR/DIBuilder.cpp
llvm/lib/IR/DebugInfo.cpp
llvm/lib/IR/DebugInfoMetadata.cpp
llvm/lib/IR/LLVMContextImpl.h
llvm/test/Assembler/dicompileunit.ll
llvm/test/Assembler/dimodule.ll
llvm/test/Bindings/llvm-c/debug_info.ll
llvm/test/CodeGen/X86/load-combine-dbg.ll
llvm/test/DebugInfo/X86/DIModule.ll
llvm/test/DebugInfo/X86/DIModuleContext.ll
llvm/test/DebugInfo/X86/clang-module.ll
llvm/tools/llvm-c-test/debuginfo.c
llvm/unittests/IR/MetadataTest.cpp

Removed: 
clang/test/CodeGen/debug-info-sysroot.c
llvm/test/DebugInfo/X86/split-dwarf-sysroot.ll



diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index ffc3870c153b..cbd524eda9d0 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -610,10 +610,6 @@ void CGDebugInfo::CreateCompileUnit() {
   remapDIPath(MainFileName), remapDIPath(getCurrentDirname()), CSInfo,
   getSource(SM, SM.getMainFileID()));
 
-  StringRef Sysroot;
-  if (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB)
-Sysroot = CGM.getHeaderSearchOpts().Sysroot;
-
   // Create new compile unit.
   TheCU = DBuilder.createCompileUnit(
   LangTag, CUFile, CGOpts.EmitVersionIdentMetadata ? Producer : "",
@@ -624,7 +620,7 @@ void CGDebugInfo::CreateCompileUnit() {
   ? llvm::DICompileUnit::DebugNameTableKind::None
   : static_cast(
 CGOpts.DebugNameTable),
-  CGOpts.DebugRangesBaseAddress, Sysroot);
+  CGOpts.DebugRangesBaseAddress);
 }
 
 llvm::DIType *CGDebugInfo::CreateType(const BuiltinType *BT) {
@@ -2475,7 +2471,7 @@ 
CGDebugInfo::getOrCreateModuleRef(ExternalASTSource::ASTSourceDescriptor Mod,
  CreateSkeletonCU);
   llvm::DIModule *DIMod =
   DBuilder.createModule(Parent, Mod.getModuleName(), ConfigMacros,
-Mod.getPath());
+Mod.getPath(), CGM.getHeaderSearchOpts().Sysroot);
   ModuleCache[M].reset(DIMod);
   return DIMod;
 }

diff  --git a/clang/test/CodeGen/debug-info-sysroot.c 
b/clang/test/CodeGen/debug-info-sysroot.c
deleted file mode 100644
index bb3c0c820cee..
--- a/clang/test/CodeGen/debug-info-sysroot.c
+++ /dev/null
@@ -1,15 +0,0 @@
-// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
-// RUN:   %s -isysroot /CLANG_SYSROOT -emit-llvm -o - \
-// RUN:   -debugger-tuning=lldb | FileCheck %s --check-prefix=LLDB
-// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
-// RUN:   %s -isysroot /CLANG_SYSROOT -emit-llvm -o - \
-// RUN:   -debugger-tuning=gdb | FileCheck %s --check-prefix=GDB
-
-void foo() {}
-
-// The sysroot is an LLDB-tuning-specific attribute.
-
-// LLDB: distinct !DICompileUnit({{.*}}sysroot: "/CLANG_SYSROOT"
-// GDB: distinct !DICompileUnit(
-// GDB-NOT: sysroot: "/CLANG_SYSROOT"
-

diff  --git a/clang/test/CodeGen/debug-nvptx.c 
b/clang/test/CodeGen/debug-nvptx.c
index 8780c5db6801..ceff30082960 100644
--- a/clang/test/CodeGen/debug-nvptx.c
+++ b/clang/test/CodeGen/debug-nvptx.c
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -triple nvptx-unknown-unknown -S -o - 
-debug-info-kind=limited %s -emit-llvm | FileCheck %s
 
-// CHECK: DICompileUnit({{.*}}, nameTableKind: None
+// CHECK: DICompileUnit({{.*}}, nameTableKind: None)
 
 void f1(void) {
 }

diff  --git a/clang/test/Modules/debug-info-moduleimport.m 
b/clang/test/Modules/debug-info-moduleimport.m
index f07c6fce784d..df8a66bebfb6 100644
--- a/clang/test/Modules/debug-info-moduleimport.m
+++ b/clang/test/Modules/debug-info-moduleimport.m
@@ -8,14 +8,14 @@
 // RUN: %clang_cc1 -debug-info-kind=limited -fmodules -DGREETING="Hello World" 
-UNDEBUG -fimplicit-module-maps -fmodules-cache-path=%t %s -I %S/Inputs 
-isysroot 

[Lldb-commits] [PATCH] D72823: [Reproducers] Add a tool to transparently capture and replay lldb sessions

2020-01-17 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/tools/lldb-repro/lldb-repro.h.cmake:12
+
+#cmakedefine LLDB_TEST_EXECUTABLE "${LLDB_TEST_EXECUTABLE}"
+

labath wrote:
> JDevlieghere wrote:
> > labath wrote:
> > > labath wrote:
> > > > Are you sure this will work fine with multi-config generators? You 
> > > > might be better off just relying on the fact that the lldb executable 
> > > > will sit right next to this binary...
> > > Actually how, is this thing going to be invoked exactly? Couldn't the 
> > > path to lldb be passed simply as argv[1]?
> > It just needs patching up like lldb-dotest and lit. Assuming you mean 
> > `argv[0]`, it think we could make that work if I replace "%lldb" with a 
> > path to lldb-repro.
> No, I really meant argv[1]. :)
> 
> The idea was that `%lldb` would expand to `/src/path/to/lldb-repro.py 
> /build/path/to/lldb.exe --whatever`. That way, you wouldn't need to rely on 
> the "same directory" trick and could get rid of all the cmake code. In fact, 
> we could even throw in a `--capture/--replay` argument to the command line, 
> and ditch the environment variables too...
I like the idea but FindTool is a class that's resolved by lit, and the 
arguments are strings. So I kept the current approach that expects to find 
`lldb` next to `lldb-repro`.


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

https://reviews.llvm.org/D72823



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


[Lldb-commits] [PATCH] D72946: [lldb] Remove ClangASTImporter reference from Target

2020-01-17 Thread Alex Langford via Phabricator via lldb-commits
xiaobai created this revision.
xiaobai added reviewers: JDevlieghere, teemperor, labath, clayborg.
Herald added a reviewer: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: LLDB.

Target is one of the classes responsible for vending ClangASTImporter.
Target doesn't need to know anything about ClangASTImporter, so if we
instead create a map from Targets to ClangASTImporters, we can preserve
existing behavior while improving layering and removing dependencies.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72946

Files:
  lldb/include/lldb/Symbol/ClangASTImporter.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
  lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/source/Symbol/ClangASTImporter.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -37,7 +37,6 @@
 #include "lldb/Interpreter/OptionGroupWatchpoint.h"
 #include "lldb/Interpreter/OptionValues.h"
 #include "lldb/Interpreter/Property.h"
-#include "lldb/Symbol/ClangASTImporter.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/Symbol.h"
@@ -91,7 +90,7 @@
   m_mutex(), m_arch(target_arch), m_images(this), m_section_load_history(),
   m_breakpoint_list(false), m_internal_breakpoint_list(true),
   m_watchpoint_list(), m_process_sp(), m_search_filter_sp(),
-  m_image_search_paths(ImageSearchPathsChanged, this), m_ast_importer_sp(),
+  m_image_search_paths(ImageSearchPathsChanged, this),
   m_source_manager_up(), m_stop_hooks(), m_stop_hook_next_id(0),
   m_valid(true), m_suppress_stop_hooks(false),
   m_is_dummy_target(is_dummy_target),
@@ -1378,7 +1377,6 @@
   m_section_load_history.Clear();
   m_images.Clear();
   m_scratch_type_system_map.Clear();
-  m_ast_importer_sp.reset();
 }
 
 void Target::DidExec() {
@@ -2258,16 +2256,6 @@
   return utility_fn;
 }
 
-ClangASTImporterSP Target::GetClangASTImporter() {
-  if (m_valid) {
-if (!m_ast_importer_sp) {
-  m_ast_importer_sp = std::make_shared();
-}
-return m_ast_importer_sp;
-  }
-  return ClangASTImporterSP();
-}
-
 void Target::SettingsInitialize() { Process::SettingsInitialize(); }
 
 void Target::SettingsTerminate() { Process::SettingsTerminate(); }
Index: lldb/source/Symbol/ClangASTImporter.cpp
===
--- lldb/source/Symbol/ClangASTImporter.cpp
+++ lldb/source/Symbol/ClangASTImporter.cpp
@@ -25,6 +25,41 @@
 using namespace lldb_private;
 using namespace clang;
 
+typedef std::map ClangASTImporterMap;
+
+static ClangASTImporterMap () {
+  static ClangASTImporterMap *g_map = nullptr;
+  static llvm::once_flag g_initialize;
+
+  llvm::call_once(g_initialize, [] { g_map = new ClangASTImporterMap(); });
+  return *g_map;
+}
+
+static std::mutex () {
+  static std::mutex *g_mutex = nullptr;
+  static llvm::once_flag g_initialize;
+
+  llvm::call_once(g_initialize, [] { g_mutex = new std::mutex(); });
+
+  return *g_mutex;
+}
+
+lldb::ClangASTImporterSP ClangASTImporter::Get(Target *target) {
+  if (!target || !target->IsValid()) {
+return lldb::ClangASTImporterSP();
+  }
+
+  std::lock_guard guard(GetImporterMapMutex());
+  ClangASTImporterMap  = GetImporterMap();
+  ClangASTImporterMap::iterator pos = map.find(target);
+  if (pos == map.end()) {
+auto ast_importer_sp = std::make_shared();
+map[target] = ast_importer_sp;
+return ast_importer_sp;
+  }
+  return pos->second;
+}
+
 CompilerType ClangASTImporter::CopyType(ClangASTContext _ast,
 const CompilerType _type) {
   clang::ASTContext _clang_ast = dst_ast.getASTContext();
Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -9242,7 +9242,7 @@
 : ClangASTContext(triple), m_target_wp(target.shared_from_this()),
   m_persistent_variables(new ClangPersistentVariables) {
   m_scratch_ast_source_up.reset(new ClangASTSource(
-  target.shared_from_this(), target.GetClangASTImporter()));
+  target.shared_from_this(), ClangASTImporter::Get()));
   m_scratch_ast_source_up->InstallASTContext(*this);
   llvm::IntrusiveRefCntPtr proxy_ast_source(
   m_scratch_ast_source_up->CreateProxy());
Index: lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
+++ 

[Lldb-commits] [PATCH] D72823: [Reproducers] Add a tool to transparently capture and replay lldb sessions

2020-01-17 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 238837.
JDevlieghere marked 11 inline comments as done.
JDevlieghere added a comment.

- Remove environment variables


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

https://reviews.llvm.org/D72823

Files:
  lldb/test/Shell/Reproducer/lit.local.cfg
  lldb/test/Shell/helper/toolchain.py
  lldb/test/Shell/lit.cfg.py
  lldb/utils/CMakeLists.txt
  lldb/utils/lldb-repro/CMakeLists.txt
  lldb/utils/lldb-repro/lldb-repro.py

Index: lldb/utils/lldb-repro/lldb-repro.py
===
--- /dev/null
+++ lldb/utils/lldb-repro/lldb-repro.py
@@ -0,0 +1,60 @@
+#!/usr/bin/env python
+"""lldb-repro
+
+lldb-repro is a utility to transparently capture and replay debugger sessions
+through the command line driver. Its used to test the reproducers by running
+the test suite twice.
+
+During the first run, with LLDB_REPRO_UTIL_CAPTURE set, it captures a
+reproducer for every lldb invocation and saves it to a well-know location
+derived from the arguments and current working directory.
+
+During the second run, with LLDB_REPRO_UTIL_CAPTURE set, the test suite is run
+again but this time every invocation of lldb replays the previously recorded
+session.
+"""
+
+import sys
+import os
+import tempfile
+import subprocess
+
+
+def help():
+print("usage: {}: /path/to/lldb capture|replay [args]".fmt(sys.argv[0]))
+
+
+def main():
+if len(sys.argv) < 3:
+help()
+return 1
+
+# Compute a hash based on the input arguments and the current working
+# directory.
+args = ' '.join(sys.argv[3:])
+cwd = os.getcwd()
+input_hash = str(hash((cwd, args)))
+
+# Use the hash to "uniquely" identify a reproducer path.
+reproducer_path = os.path.join(tempfile.gettempdir(), input_hash)
+
+# Create a new lldb invocation with capture or replay enabled.
+lldb = os.path.join(os.path.dirname(sys.argv[0]), 'lldb')
+new_args = [sys.argv[1]]
+if sys.argv[2] == "replay":
+new_args.extend(['--replay', reproducer_path])
+elif sys.argv[2] == "capture":
+new_args.extend([
+'--capture', '--capture-path', reproducer_path,
+'--reproducer-auto-generate'
+])
+new_args.extend(sys.argv[1:])
+else:
+help()
+return 1
+
+return subprocess.call(new_args)
+
+
+if __name__ == '__main__':
+exit(main())
Index: lldb/utils/lldb-repro/CMakeLists.txt
===
--- /dev/null
+++ lldb/utils/lldb-repro/CMakeLists.txt
@@ -0,0 +1,4 @@
+add_custom_target(lldb-repro)
+add_dependencies(lldb-repro lldb-test-deps)
+set_target_properties(lldb-repro PROPERTIES FOLDER "lldb utils")
+configure_file(lldb-repro.py ${LLVM_RUNTIME_OUTPUT_INTDIR}/lldb-repro COPYONLY)
Index: lldb/utils/CMakeLists.txt
===
--- lldb/utils/CMakeLists.txt
+++ lldb/utils/CMakeLists.txt
@@ -1,2 +1,3 @@
 add_subdirectory(lit-cpuid)
 add_subdirectory(lldb-dotest)
+add_subdirectory(lldb-repro)
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -38,16 +38,24 @@
 # test_exec_root: The root path where tests should be run.
 config.test_exec_root = os.path.join(config.lldb_obj_root, 'test')
 
-# Propagate LLDB_CAPTURE_REPRODUCER
+# Propagate reproducer environment vars.
 if 'LLDB_CAPTURE_REPRODUCER' in os.environ:
   config.environment['LLDB_CAPTURE_REPRODUCER'] = os.environ[
   'LLDB_CAPTURE_REPRODUCER']
 
+# Support running the test suite under the lldb-repro wrapper. This makes it
+# possible to capture a test suite run and then rerun all the test from the
+# just captured reproducer.
+lldb_repro_mode = lit_config.params.get('lldb-run-with-repro', None)
+if lldb_repro_mode:
+  config.skip_reproducer_test = True
+  lit_config.note("Running Shell test with lldb-repo in {} mode.".format(lldb_repro_mode))
+  toolchain.use_lldb_repro_substitutions(config, lldb_repro_mode)
+
 llvm_config.use_default_substitutions()
 toolchain.use_lldb_substitutions(config)
 toolchain.use_support_substitutions(config)
 
-
 if re.match(r'^arm(hf.*-linux)|(.*-linux-gnuabihf)', config.target_triple):
 config.available_features.add("armhf-linux")
 
Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -9,6 +9,11 @@
 from lit.llvm.subst import FindTool
 from lit.llvm.subst import ToolSubst
 
+
+def _get_lldb_init_path(config):
+return os.path.join(config.test_exec_root, 'Shell', 'lit-lldb-init')
+
+
 def use_lldb_substitutions(config):
 # Set up substitutions for primary tools.  These tools must come from config.lldb_tools_dir
 # which is basically the build output directory.  We do not want to find these 

Re: [Lldb-commits] LLDB Intel hardware breakpoints patch

2020-01-17 Thread Jim Ingham via lldb-commits
Somebody will have to champion this patch for it to get into lldb.  It has no 
test case, so it’s not acceptable as is.  It should also be cleaned up so that 
it shares more code with the watchpoint implementation in the same file (for 
instance duplicating all the comments and logic to set a hardware breakpoint 
vrs. a hardware watchpoint when they only differ by the size & read/write bits 
seems unfortunate.)

I don’t think the shared resource issue is actually a problem.  Both the 
watchpoint support and your additions call IsWatchpointVacant to find an 
available slot before using it so they won’t overwrite an already in use 
watchpoint.  That is certainly one of the things that should be included in a 
test case however.

I’m presuming you don’t intend to champion this patch yourself? 

Jim


> On Jan 17, 2020, at 8:20 AM, Reverser via lldb-commits 
>  wrote:
> 
> Hi,
>  
> Jonas Devlieghere asked me to send my patch to this mailing list because of 
> potential licensing issues.
>  
> It adds hardware breakpoints support for i386 and x86_64 architectures, 
> something that has been missing for a long time. I have written a blogpost 
> about it at 
> https://reverse.put.as/2019/11/19/how-to-make-lldb-a-real-debugger/ 
> .
>  
> Patch available in attachment and also here 
> https://github.com/gdbinit/llvmpatches 
> .
>  
> You most probably want to tweak it out and adapt to your project style. If 
> I'm not mistaken there is no count between hardware breakpoints and 
> watchpoints since they share the same resources. I didn't bother taking care 
> of that but it's something that should exist in production ready code.
>  
> I declared the patch code as public domain. If it makes your life easier I 
> can change it to whatever license you need, just tell me.
>  
> Best,
> Pedro
>  
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org 
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits 
> 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D72880: Fix a buffer-size bug when the first DW_OP_piece is undefined

2020-01-17 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl marked an inline comment as done.
aprantl added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:2077
+  // with the expression result, so the debugger can print missing
+  // members as "" or something.
   ::memset(curr_piece.GetBuffer().GetBytes(), 0, piece_byte_size);

labath wrote:
> vsk wrote:
> > FTR I'm not really sure this case is allowed by the dwarf standard. Under 
> > DW_OP_piece the standard references a "preceding simple location 
> > description" -- istm like an error if the stack is empty. My $0.0002 :) 
> I think this is fine -- an "empty location description" (section 2.6.1.1.4 of 
> DWARF4) is a kind of a "simple location description" (sect. 2.6.1.1).
> 
> The part that worries me more is this: "Each simple location description that 
> is a DWARF expression is evaluated independently of any others (as though on 
> its own separate stack, if any).". I think that means we should reset/clear 
> the stack after each DW_OP_piece, but we don't currently do that (we just pop 
> the topmost value). However, I am not sure if this has any impact on valid 
> DWARF expressions (though it can cause us to produce bogus values for invalid 
> ones).
That's cute, so with our current implementation one could write a communicating 
DWARF expression like:

DW_OP_lit1 DW_OP_dup DW_OP_piece 1 DW_OP_piece 1

Is the second byte supposed to be 1 or undef?

;-)

I agree, we should probably clear the stack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72880



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


[Lldb-commits] [PATCH] D72917: [lldb/DWARF] Change how we construct a llvm::DWARFContext

2020-01-17 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:130
 
-if (m_main_section_list) {
-  for (auto  : *m_main_section_list)
-AddSection(*section);
-}
-
-if (m_dwo_section_list) {
-  for (auto  : *m_dwo_section_list)
-AddSection(*section);
-}
+AddSection("debug_line_str", getOrLoadLineStrData());
 

This will also work on Darwin where the section names are slightly different 
(shorter)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72917



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


[Lldb-commits] [lldb] ec9a3cc - Update testcase for LLVM IR change (sysroot)

2020-01-17 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2020-01-17T11:04:55-08:00
New Revision: ec9a3cccd4019e3b371175c7ea7a227e0e737c5b

URL: 
https://github.com/llvm/llvm-project/commit/ec9a3cccd4019e3b371175c7ea7a227e0e737c5b
DIFF: 
https://github.com/llvm/llvm-project/commit/ec9a3cccd4019e3b371175c7ea7a227e0e737c5b.diff

LOG: Update testcase for LLVM IR change (sysroot)

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/compilercontext.ll

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/compilercontext.ll 
b/lldb/test/Shell/SymbolFile/DWARF/compilercontext.ll
index 8589d0fe27c2..149f7efbbfbe 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/compilercontext.ll
+++ b/lldb/test/Shell/SymbolFile/DWARF/compilercontext.ll
@@ -37,10 +37,10 @@ target triple = "x86_64-apple-macosx10.14.0"
 !llvm.ident = !{!22}
 
 ; This simulates the debug info for a Clang module.
-!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: 
"clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, 
nameTableKind: GNU, retainedTypes: !{!11})
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: 
"clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, 
nameTableKind: GNU, retainedTypes: !{!11}, sysroot: "/")
 !3 = !DIFile(filename: "t.c", directory: "/")
-!8 = !DIModule(scope: !9, name: "SubModule", includePath: "", sysroot: "/")
-!9 = !DIModule(scope: null, name: "CModule", includePath: "", sysroot: "/")
+!8 = !DIModule(scope: !9, name: "SubModule", includePath: "")
+!9 = !DIModule(scope: null, name: "CModule", includePath: "")
 !11 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: 
"FromSubmodule", scope: !8, file: !3, line: 1, size: 96, elements: !13)
 !13 = !{!14, !16, !17}
 !14 = !DIDerivedType(tag: DW_TAG_member, name: "x", scope: !11, file: !3, 
line: 2, baseType: !15, size: 32)



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


[Lldb-commits] [lldb] c1bc094 - [TestQuoting] Use the fully qualified path for remote platforms.

2020-01-17 Thread Davide Italiano via lldb-commits

Author: Davide Italiano
Date: 2020-01-17T10:57:35-08:00
New Revision: c1bc094f361beede4e88ace8e9761391707ee30b

URL: 
https://github.com/llvm/llvm-project/commit/c1bc094f361beede4e88ace8e9761391707ee30b
DIFF: 
https://github.com/llvm/llvm-project/commit/c1bc094f361beede4e88ace8e9761391707ee30b.diff

LOG: [TestQuoting] Use the fully qualified path for remote platforms.

Patch by Jason Molenda, fixes a test failure on arm64 devices.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/commands/settings/quoting/TestQuoting.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/settings/quoting/TestQuoting.py 
b/lldb/packages/Python/lldbsuite/test/commands/settings/quoting/TestQuoting.py
index 5853313db6ff..d7581951a147 100644
--- 
a/lldb/packages/Python/lldbsuite/test/commands/settings/quoting/TestQuoting.py
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/settings/quoting/TestQuoting.py
@@ -74,7 +74,7 @@ def do_test_args(self, args_in, args_out):
 
 local_outfile = self.getBuildArtifact("output.txt")
 if lldb.remote_platform:
-remote_outfile = "output.txt" # Relative to platform's PWD
+remote_outfile = lldb.remote_platform.GetWorkingDirectory() + 
"/output.txt"
 else:
 remote_outfile = local_outfile
 



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


[Lldb-commits] [PATCH] D72694: [lldb] Mark the implicit copy constructor as deleted when a move constructor is provided.

2020-01-17 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik accepted this revision.
shafik added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: 
lldb/packages/Python/lldbsuite/test/commands/expression/deleting-implicit-copy-constructor/main.cpp:14
+  // should have propagated to this record and Clang won't crash.
+  IndirectlyDeletedCopyCstr() { 
//%self.dbg.GetCommandInterpreter().HandleCompletion("e ", len("e "), 0, -1, 
lldb.SBStringList())
+  }

Is this the only way to trigger the assert on the command line?



Comment at: lldb/source/Symbol/ClangASTContext.cpp:7786
 if (auto *cxx_record_decl = llvm::dyn_cast(tag_decl)) {
+  // If we have a move constructor declared but no copy constructor we
+  // need to explicitly mark it as deleted. Usually Sema would do this for

There are other cases where special member functions should be deleted as well. 
I don't know if they will show up as asserts as well but it is unfortunate that 
we have to mirror the logic here. 


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

https://reviews.llvm.org/D72694



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


[Lldb-commits] [lldb] 12e4794 - Rename DW_AT_LLVM_isysroot to DW_AT_LLVM_sysroot

2020-01-17 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2020-01-17T09:36:48-08:00
New Revision: 12e479475a896f664fb721f98c2d6805185ac352

URL: 
https://github.com/llvm/llvm-project/commit/12e479475a896f664fb721f98c2d6805185ac352
DIFF: 
https://github.com/llvm/llvm-project/commit/12e479475a896f664fb721f98c2d6805185ac352.diff

LOG: Rename DW_AT_LLVM_isysroot to DW_AT_LLVM_sysroot

This is a purely cosmetic change that is NFC in terms of the binary
output. I bugs me that I called the attribute DW_AT_LLVM_isysroot
since the "i" is an artifact of GCC command line option syntax
(-isysroot is in the category of -i options) and doesn't carry any
useful information otherwise.

This attribute only appears in Clang module debug info.

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

Added: 
clang/test/CodeGen/debug-info-sysroot.c
llvm/test/DebugInfo/X86/split-dwarf-sysroot.ll

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGen/debug-nvptx.c
clang/test/Modules/debug-info-moduleimport.m
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
llvm/bindings/go/llvm/dibuilder.go
llvm/include/llvm-c/DebugInfo.h
llvm/include/llvm/IR/DIBuilder.h
llvm/include/llvm/IR/DebugInfoMetadata.h
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/MetadataLoader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/IR/DIBuilder.cpp
llvm/lib/IR/DebugInfo.cpp
llvm/lib/IR/DebugInfoMetadata.cpp
llvm/lib/IR/LLVMContextImpl.h
llvm/test/Assembler/dicompileunit.ll
llvm/test/Assembler/dimodule.ll
llvm/test/Bindings/llvm-c/debug_info.ll
llvm/test/CodeGen/X86/load-combine-dbg.ll
llvm/test/DebugInfo/X86/DIModule.ll
llvm/test/DebugInfo/X86/DIModuleContext.ll
llvm/test/DebugInfo/X86/clang-module.ll
llvm/tools/llvm-c-test/debuginfo.c
llvm/unittests/IR/MetadataTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index cbd524eda9d0..ffc3870c153b 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -610,6 +610,10 @@ void CGDebugInfo::CreateCompileUnit() {
   remapDIPath(MainFileName), remapDIPath(getCurrentDirname()), CSInfo,
   getSource(SM, SM.getMainFileID()));
 
+  StringRef Sysroot;
+  if (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB)
+Sysroot = CGM.getHeaderSearchOpts().Sysroot;
+
   // Create new compile unit.
   TheCU = DBuilder.createCompileUnit(
   LangTag, CUFile, CGOpts.EmitVersionIdentMetadata ? Producer : "",
@@ -620,7 +624,7 @@ void CGDebugInfo::CreateCompileUnit() {
   ? llvm::DICompileUnit::DebugNameTableKind::None
   : static_cast(
 CGOpts.DebugNameTable),
-  CGOpts.DebugRangesBaseAddress);
+  CGOpts.DebugRangesBaseAddress, Sysroot);
 }
 
 llvm::DIType *CGDebugInfo::CreateType(const BuiltinType *BT) {
@@ -2471,7 +2475,7 @@ 
CGDebugInfo::getOrCreateModuleRef(ExternalASTSource::ASTSourceDescriptor Mod,
  CreateSkeletonCU);
   llvm::DIModule *DIMod =
   DBuilder.createModule(Parent, Mod.getModuleName(), ConfigMacros,
-Mod.getPath(), CGM.getHeaderSearchOpts().Sysroot);
+Mod.getPath());
   ModuleCache[M].reset(DIMod);
   return DIMod;
 }

diff  --git a/clang/test/CodeGen/debug-info-sysroot.c 
b/clang/test/CodeGen/debug-info-sysroot.c
new file mode 100644
index ..bb3c0c820cee
--- /dev/null
+++ b/clang/test/CodeGen/debug-info-sysroot.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   %s -isysroot /CLANG_SYSROOT -emit-llvm -o - \
+// RUN:   -debugger-tuning=lldb | FileCheck %s --check-prefix=LLDB
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   %s -isysroot /CLANG_SYSROOT -emit-llvm -o - \
+// RUN:   -debugger-tuning=gdb | FileCheck %s --check-prefix=GDB
+
+void foo() {}
+
+// The sysroot is an LLDB-tuning-specific attribute.
+
+// LLDB: distinct !DICompileUnit({{.*}}sysroot: "/CLANG_SYSROOT"
+// GDB: distinct !DICompileUnit(
+// GDB-NOT: sysroot: "/CLANG_SYSROOT"
+

diff  --git a/clang/test/CodeGen/debug-nvptx.c 
b/clang/test/CodeGen/debug-nvptx.c
index ceff30082960..8780c5db6801 100644
--- a/clang/test/CodeGen/debug-nvptx.c
+++ b/clang/test/CodeGen/debug-nvptx.c
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -triple nvptx-unknown-unknown -S -o - 
-debug-info-kind=limited %s -emit-llvm | FileCheck %s
 
-// CHECK: DICompileUnit({{.*}}, nameTableKind: None)
+// CHECK: DICompileUnit({{.*}}, nameTableKind: None
 
 void f1(void) {
 }

diff  --git a/clang/test/Modules/debug-info-moduleimport.m 
b/clang/test/Modules/debug-info-moduleimport.m
index df8a66bebfb6..f07c6fce784d 100644
--- 

[Lldb-commits] LLDB Intel hardware breakpoints patch

2020-01-17 Thread Reverser via lldb-commits
Hi,

 

Jonas Devlieghere asked me to send my patch to this mailing list because of 
potential licensing issues.

 

It adds hardware breakpoints support for i386 and x86_64 architectures, 
something that has been missing for a long time. I have written a blogpost 
about it at https://reverse.put.as/2019/11/19/how-to-make-lldb-a-real-debugger/.

 

Patch available in attachment and also here 
https://github.com/gdbinit/llvmpatches.

 

You most probably want to tweak it out and adapt to your project style. If I'm 
not mistaken there is no count between hardware breakpoints and watchpoints 
since they share the same resources. I didn't bother taking care of that but 
it's something that should exist in production ready code.

 

I declared the patch code as public domain. If it makes your life easier I can 
change it to whatever license you need, just tell me.

 

Best,

Pedro

 



hwbreakpoints.patch
Description: Binary data
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to TypeSystemClang

2020-01-17 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.

Looks good. Making TypeSystem's into real plugins in the future would be great 
too.


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

https://reviews.llvm.org/D72684



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


[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to TypeSystemClang

2020-01-17 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

It looks like everyone is on board with this...


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

https://reviews.llvm.org/D72684



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


[Lldb-commits] [lldb] d035c83 - [lldb] Try to fix writing outside temp dir from 4bafceced6a7641be7b090229c6ccef22cf55bff

2020-01-17 Thread Sam McCall via lldb-commits

Author: Sam McCall
Date: 2020-01-17T17:30:12+01:00
New Revision: d035c832c3f9d29eb1d29b6d22cd8d018a6462c6

URL: 
https://github.com/llvm/llvm-project/commit/d035c832c3f9d29eb1d29b6d22cd8d018a6462c6
DIFF: 
https://github.com/llvm/llvm-project/commit/d035c832c3f9d29eb1d29b6d22cd8d018a6462c6.diff

LOG: [lldb] Try to fix writing outside temp dir from 
4bafceced6a7641be7b090229c6ccef22cf55bff

Added: 


Modified: 
lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml

Removed: 




diff  --git a/lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml 
b/lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
index d72080f40224..852fbb64cc78 100644
--- a/lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
+++ b/lldb/test/Shell/ObjectFile/wasm/unified-debug-sections.yaml
@@ -1,3 +1,6 @@
+# RUN: rm -rf %t
+# RUN: mkdir %t
+# RUN: cd %t
 # RUN: yaml2obj -docnum=1 %s > test.wasm
 # RUN: yaml2obj -docnum=2 %s > test_sym.wasm
 # RUN: lldb-test object-file test.wasm | FileCheck %s



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


[Lldb-commits] [PATCH] D72909: Make SymbolFileDWARF::ParseLineTable use std::sort instead of insertion sort

2020-01-17 Thread Unnar Freyr Erlendsson via Phabricator via lldb-commits
unnar updated this revision to Diff 238771.
unnar added a comment.

Switched back to std::vector and uploaded the full diff.

I don't have commit access so it would be appreciated if you could land this 
for me, thanks!


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

https://reviews.llvm.org/D72909

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

Index: lldb/source/Symbol/LineTable.cpp
===
--- lldb/source/Symbol/LineTable.cpp
+++ lldb/source/Symbol/LineTable.cpp
@@ -21,6 +21,17 @@
 LineTable::LineTable(CompileUnit *comp_unit)
 : m_comp_unit(comp_unit), m_entries() {}
 
+LineTable::LineTable(CompileUnit *comp_unit, std::vector )
+: m_comp_unit(comp_unit), m_entries() {
+  LineTable::Entry::LessThanBinaryPredicate less_than_bp(this);
+  std::sort(sequences.begin(), sequences.end(), less_than_bp);
+  for (auto *sequence : sequences) {
+LineSequenceImpl *seq = reinterpret_cast(sequence);
+m_entries.insert(m_entries.end(), seq->m_entries.begin(),
+  seq->m_entries.end());
+  }
+}
+
 // Destructor
 LineTable::~LineTable() {}
 
@@ -154,6 +165,13 @@
 #undef LT_COMPARE
 }
 
+bool LineTable::Entry::LessThanBinaryPredicate::
+operator()(const LineSequence *sequence_a, const LineSequence *sequence_b) const {
+  auto *seq_a = static_cast(sequence_a);
+  auto *seq_b = static_cast(sequence_b);
+  return (*this)(seq_a->m_entries.front(), seq_b->m_entries.front());
+}
+
 uint32_t LineTable::GetSize() const { return m_entries.size(); }
 
 bool LineTable::GetLineEntryAtIndex(uint32_t idx, LineEntry _entry) {
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1015,20 +1015,22 @@
   // FIXME: Rather than parsing the whole line table and then copying it over
   // into LLDB, we should explore using a callback to populate the line table
   // while we parse to reduce memory usage.
-  std::unique_ptr line_table_up =
-  std::make_unique(_unit);
-  LineSequence *sequence = line_table_up->CreateLineSequenceContainer();
+  LineSequence *sequence = LineTable::CreateLineSequenceContainer();
+  std::vector sequences;
   for (auto  : line_table->Rows) {
-line_table_up->AppendLineEntryToSequence(
+LineTable::AppendLineEntryToSequence(
 sequence, row.Address.Address, row.Line, row.Column, row.File,
 row.IsStmt, row.BasicBlock, row.PrologueEnd, row.EpilogueBegin,
 row.EndSequence);
 if (row.EndSequence) {
-  line_table_up->InsertSequence(sequence);
-  sequence = line_table_up->CreateLineSequenceContainer();
+  sequences.push_back(sequence);
+  sequence = LineTable::CreateLineSequenceContainer();
 }
   }
 
+  std::unique_ptr line_table_up =
+  std::make_unique(_unit, sequences);
+
   if (SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile()) {
 // We have an object file that has a line table with addresses that are not
 // linked. We need to link the line table and convert the addresses that
Index: lldb/include/lldb/Symbol/LineTable.h
===
--- lldb/include/lldb/Symbol/LineTable.h
+++ lldb/include/lldb/Symbol/LineTable.h
@@ -42,6 +42,12 @@
   /// The compile unit to which this line table belongs.
   LineTable(CompileUnit *comp_unit);
 
+  /// Construct with entries found in \a sequences.
+  ///
+  /// \param[in] sequences
+  /// Unsorted list of line sequences.
+  LineTable(CompileUnit *comp_unit, std::vector );
+
   /// Destructor.
   ~LineTable();
 
@@ -64,11 +70,11 @@
bool is_epilogue_begin, bool is_terminal_entry);
 
   // Used to instantiate the LineSequence helper class
-  LineSequence *CreateLineSequenceContainer();
+  static LineSequence *CreateLineSequenceContainer();
 
   // Append an entry to a caller-provided collection that will later be
   // inserted in this line table.
-  void AppendLineEntryToSequence(LineSequence *sequence, lldb::addr_t file_addr,
+  static void AppendLineEntryToSequence(LineSequence *sequence, lldb::addr_t file_addr,
  uint32_t line, uint16_t column,
  uint16_t file_idx, bool is_start_of_statement,
  bool is_start_of_basic_block,
@@ -259,6 +265,7 @@
 public:
   LessThanBinaryPredicate(LineTable *line_table);
   bool operator()(const LineTable::Entry &, const LineTable::Entry &) const;
+  bool operator()(const LineSequence*, const LineSequence*) const;
 
 protected:
   LineTable *m_line_table;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D72920: [lldb/DWARF] Simplify DWARFDebugInfoEntry::LookupAddress

2020-01-17 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: JDevlieghere, aprantl.
Herald added a project: LLDB.

This method was doing a lot more than it's only caller needed
(DWARFDIE::LookupDeepestBlock) needed, so I inline it into the caller,
and remove any code which is not actually used. This includes code for
searching for the deepest function, and the code for working around
incomplete DW_AT_low_pc/high_pc attributes on a compile unit DIE (modern
compiler get this right, and this method is called on function DIEs
anyway).

This also improves our llvm consistency, as llvm::DWARFDebugInfoEntry is
just a very simple struct with no nontrivial logic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72920

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h

Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -50,10 +50,6 @@
   bool Extract(const lldb_private::DWARFDataExtractor ,
const DWARFUnit *cu, lldb::offset_t *offset_ptr);
 
-  bool LookupAddress(const dw_addr_t address, DWARFUnit *cu,
- DWARFDebugInfoEntry **function_die,
- DWARFDebugInfoEntry **block_die);
-
   size_t GetAttributes(const DWARFUnit *cu,
DWARFAttributes ,
uint32_t curr_depth = 0)
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -993,209 +993,6 @@
   return storage.c_str();
 }
 
-bool DWARFDebugInfoEntry::LookupAddress(const dw_addr_t address, DWARFUnit *cu,
-DWARFDebugInfoEntry **function_die,
-DWARFDebugInfoEntry **block_die) {
-  bool found_address = false;
-  if (m_tag) {
-bool check_children = false;
-bool match_addr_range = false;
-//  printf("0x%8.8x: %30s: address = 0x%8.8x - ", m_offset,
-//  DW_TAG_value_to_name(tag), address);
-switch (m_tag) {
-case DW_TAG_array_type:
-  break;
-case DW_TAG_class_type:
-  check_children = true;
-  break;
-case DW_TAG_entry_point:
-case DW_TAG_enumeration_type:
-case DW_TAG_formal_parameter:
-case DW_TAG_imported_declaration:
-case DW_TAG_label:
-  break;
-case DW_TAG_lexical_block:
-  check_children = true;
-  match_addr_range = true;
-  break;
-case DW_TAG_member:
-case DW_TAG_pointer_type:
-case DW_TAG_reference_type:
-  break;
-case DW_TAG_compile_unit:
-  match_addr_range = true;
-  break;
-case DW_TAG_string_type:
-  break;
-case DW_TAG_structure_type:
-  check_children = true;
-  break;
-case DW_TAG_subroutine_type:
-case DW_TAG_typedef:
-case DW_TAG_union_type:
-case DW_TAG_unspecified_parameters:
-case DW_TAG_variant:
-  break;
-case DW_TAG_common_block:
-  check_children = true;
-  break;
-case DW_TAG_common_inclusion:
-case DW_TAG_inheritance:
-  break;
-case DW_TAG_inlined_subroutine:
-  check_children = true;
-  match_addr_range = true;
-  break;
-case DW_TAG_module:
-  match_addr_range = true;
-  break;
-case DW_TAG_ptr_to_member_type:
-case DW_TAG_set_type:
-case DW_TAG_subrange_type:
-case DW_TAG_with_stmt:
-case DW_TAG_access_declaration:
-case DW_TAG_base_type:
-  break;
-case DW_TAG_catch_block:
-  match_addr_range = true;
-  break;
-case DW_TAG_const_type:
-case DW_TAG_constant:
-case DW_TAG_enumerator:
-case DW_TAG_file_type:
-case DW_TAG_friend:
-case DW_TAG_namelist:
-case DW_TAG_namelist_item:
-case DW_TAG_packed_type:
-  break;
-case DW_TAG_subprogram:
-  match_addr_range = true;
-  break;
-case DW_TAG_template_type_parameter:
-case DW_TAG_template_value_parameter:
-case DW_TAG_GNU_template_parameter_pack:
-case DW_TAG_thrown_type:
-  break;
-case DW_TAG_try_block:
-  match_addr_range = true;
-  break;
-case DW_TAG_variant_part:
-case DW_TAG_variable:
-case DW_TAG_volatile_type:
-case DW_TAG_dwarf_procedure:
-case DW_TAG_restrict_type:
-case DW_TAG_interface_type:
-  break;
-case DW_TAG_namespace:
-  check_children = true;
-  break;
-case DW_TAG_imported_module:
-case DW_TAG_unspecified_type:
-  break;
-case DW_TAG_partial_unit:
-  match_addr_range = true;
-  break;
-case DW_TAG_imported_unit:
-case DW_TAG_shared_type:
-

[Lldb-commits] [PATCH] D72909: Make SymbolFileDWARF::ParseLineTable use std::sort instead of insertion sort

2020-01-17 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Yes, that's looks pretty much like it, but it seems you uploaded the diff 
incorrectly -- it looks like its based on the previous version of your patch 
and not master (you should always upload the full set of changes not just the 
recent additions).

Also, when I asked for an ArrayRef, I forgot that you are sorting the thing -- 
a vector does seem reasonable in that case (sorry).

Do you have commit access? If not, I can land this for you (as soon as I get 
the correct diff).


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

https://reviews.llvm.org/D72909



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


[Lldb-commits] [PATCH] D72909: Make SymbolFileDWARF::ParseLineTable use std::sort instead of insertion sort

2020-01-17 Thread Unnar Freyr Erlendsson via Phabricator via lldb-commits
unnar updated this revision to Diff 238759.
unnar marked an inline comment as done.
unnar added a comment.
Herald added a reviewer: jdoerfert.

@labath

Can you take another look and see if this matches what you had in mind?


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

https://reviews.llvm.org/D72909

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

Index: lldb/source/Symbol/LineTable.cpp
===
--- lldb/source/Symbol/LineTable.cpp
+++ lldb/source/Symbol/LineTable.cpp
@@ -21,6 +21,17 @@
 LineTable::LineTable(CompileUnit *comp_unit)
 : m_comp_unit(comp_unit), m_entries() {}
 
+LineTable::LineTable(CompileUnit *comp_unit, llvm::MutableArrayRef sequences)
+: m_comp_unit(comp_unit), m_entries() {
+  LineTable::Entry::LessThanBinaryPredicate less_than_bp(this);
+  std::sort(sequences.begin(), sequences.end(), less_than_bp);
+  for (auto *sequence : sequences) {
+LineSequenceImpl *seq = reinterpret_cast(sequence);
+m_entries.insert(m_entries.end(), seq->m_entries.begin(),
+  seq->m_entries.end());
+  }
+}
+
 // Destructor
 LineTable::~LineTable() {}
 
@@ -130,17 +141,6 @@
   m_entries.insert(pos, seq->m_entries.begin(), seq->m_entries.end());
 }
 
-void LineTable::ReplaceLineTableWithSequences(std::vector ) {
-  m_entries.clear();
-  LineTable::Entry::LessThanBinaryPredicate less_than_bp(this);
-  std::sort(sequences.begin(), sequences.end(), less_than_bp);
-  for (auto *sequence : sequences) {
-LineSequenceImpl *seq = reinterpret_cast(sequence);
-m_entries.insert(m_entries.end(), seq->m_entries.begin(),
-  seq->m_entries.end());
-  }
-}
-
 LineTable::Entry::LessThanBinaryPredicate::LessThanBinaryPredicate(
 LineTable *line_table)
 : m_line_table(line_table) {}
@@ -166,9 +166,9 @@
 }
 
 bool LineTable::Entry::LessThanBinaryPredicate::
-operator()(LineSequence *sequence_a, LineSequence *sequence_b) const {
-  LineSequenceImpl *seq_a = reinterpret_cast(sequence_a);
-  LineSequenceImpl *seq_b = reinterpret_cast(sequence_b);
+operator()(const LineSequence *sequence_a, const LineSequence *sequence_b) const {
+  auto *seq_a = static_cast(sequence_a);
+  auto *seq_b = static_cast(sequence_b);
   return (*this)(seq_a->m_entries.front(), seq_b->m_entries.front());
 }
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1015,22 +1015,23 @@
   // FIXME: Rather than parsing the whole line table and then copying it over
   // into LLDB, we should explore using a callback to populate the line table
   // while we parse to reduce memory usage.
-  std::unique_ptr line_table_up =
-  std::make_unique(_unit);
-  LineSequence *sequence = line_table_up->CreateLineSequenceContainer();
-  std::vector sequences;
+  LineSequence *sequence = LineTable::CreateLineSequenceContainer();
+  std::vector sequences;
   for (auto  : line_table->Rows) {
-line_table_up->AppendLineEntryToSequence(
+LineTable::AppendLineEntryToSequence(
 sequence, row.Address.Address, row.Line, row.Column, row.File,
 row.IsStmt, row.BasicBlock, row.PrologueEnd, row.EpilogueBegin,
 row.EndSequence);
 if (row.EndSequence) {
   sequences.push_back(sequence);
-  sequence = line_table_up->CreateLineSequenceContainer();
+  sequence = LineTable::CreateLineSequenceContainer();
 }
   }
 
-  line_table_up->ReplaceLineTableWithSequences(sequences);
+  std::unique_ptr line_table_up =
+  std::make_unique(_unit,
+  llvm::MutableArrayRef(
+  sequences));
 
   if (SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile()) {
 // We have an object file that has a line table with addresses that are not
Index: lldb/include/lldb/Symbol/LineTable.h
===
--- lldb/include/lldb/Symbol/LineTable.h
+++ lldb/include/lldb/Symbol/LineTable.h
@@ -42,6 +42,13 @@
   /// The compile unit to which this line table belongs.
   LineTable(CompileUnit *comp_unit);
 
+  /// Construct with entries found in \a sequences.
+  ///
+  /// \param[in] sequences
+  /// Unsorted list of line sequences.
+  LineTable(CompileUnit *comp_unit,
+llvm::MutableArrayRef sequences);
+
   /// Destructor.
   ~LineTable();
 
@@ -64,11 +71,11 @@
bool is_epilogue_begin, bool is_terminal_entry);
 
   // Used to instantiate the LineSequence helper class
-  LineSequence *CreateLineSequenceContainer();
+  static LineSequence *CreateLineSequenceContainer();
 
   // Append an entry to a caller-provided collection that will later be
   // inserted in this line table.
-  void 

[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

2020-01-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: 
lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp:90-126
+  ModuleList loaded_module_list;
+  const ModuleList _list = m_process->GetTarget().GetImages();
+  const size_t num_modules = module_list.GetSize();
+  for (uint32_t idx = 0; idx < num_modules; ++idx) {
+ModuleSP module_sp(module_list.GetModuleAtIndexUnlocked(idx));
+if (!module_sp)
+  continue;

Right, so, given that (IIUC) you use the `qXfer:libraries` packet, I believe 
this code should not be needed. In this case `ProcessGDBRemote::LoadModules` 
should do all the work (by calling into 
`DynamicLoaderWasmDYLD::LoadModuleAtAddress`, which will then call into 
`ObjectFileWasm::SetLoadAddress`).

The fact that you need fix up section load addresses after these functions are 
done makes me believe that those functions are not doing their job properly. 
That wouldn't be too bad if there is a reason for that, but right now I don't 
see any indication that this is the case.

Can you explain what is the purpose of this code (specifically, what would 
happen without it, if we only had m_process->LoadModules() here), so we can 
figure out what to do about this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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


[Lldb-commits] [PATCH] D72917: [lldb/DWARF] Change how we construct a llvm::DWARFContext

2020-01-17 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: JDevlieghere, aprantl.
Herald added a project: LLDB.

The goal of this patch is two-fold. First, it fixes a use-after-free in
the construction of the llvm DWARFContext. This happened because the
construction code was throwing away the lldb DataExtractors it got while
reading the sections (unlike their llvm counterparts, these are also
responsible for memory ownership). In most cases this did not matter,
because the sections are just slices of the mmapped file data. But this
isn't the case for compressed elf sections, in which case the section is
decompressed into a heap buffer. A similar thing also happen with object
files which are loaded from process memory.

The second goal is to make it explicit which sections go into the llvm
DWARFContext -- any access to the sections through both DWARF parsers
carries a risk of parsing things twice, so it's better if this is a
conscious decision. Also, this avoids loading completely irrelevant
sections (e.g. .text). At present, the only section that needs to be
present in the llvm DWARFContext is the debug_line_str. Using it through
both APIs is not a problem, as there is no parsing involved.

The first goal is achieved by loading the sections through the existing
lldb DWARFContext APIs, which already do the caching. The second by
explicitly enumerating the sections we wish to load.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72917

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug-names-compressed.cpp


Index: lldb/test/Shell/SymbolFile/DWARF/debug-names-compressed.cpp
===
--- lldb/test/Shell/SymbolFile/DWARF/debug-names-compressed.cpp
+++ lldb/test/Shell/SymbolFile/DWARF/debug-names-compressed.cpp
@@ -3,12 +3,15 @@
 
 // REQUIRES: lld, zlib
 
-// RUN: %clang -g -c -o %t.o --target=x86_64-pc-linux -mllvm 
-accel-tables=Dwarf %s
+// RUN: %clang -c -o %t.o --target=x86_64-pc-linux -gdwarf-5 -gpubnames %s
 // RUN: ld.lld %t.o -o %t --compress-debug-sections=zlib
+// RUN: llvm-readobj --sections %t | FileCheck %s --check-prefix NAMES
 // RUN: lldb-test symbols --find=variable --name=foo %t | FileCheck %s
 
+// NAMES: Name: .debug_names
+
 // CHECK: Found 1 variables:
 int foo;
-// ONE-DAG: name = "foo", type = {{.*}} (int), {{.*}} decl = 
debug-names-compressed.cpp:[[@LINE-1]]
+// CHECK-DAG: name = "foo", type = {{.*}} (int), {{.*}} decl = 
debug-names-compressed.cpp:[[@LINE-1]]
 
 extern "C" void _start() {}
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
@@ -117,32 +117,17 @@
   if (!m_llvm_context) {
 llvm::StringMap> section_map;
 uint8_t addr_size = 0;
-
-auto AddSection = [&](Section ) {
-  DataExtractor section_data;
-  section.GetSectionData(section_data);
-
+auto AddSection = [&](llvm::StringRef name, DWARFDataExtractor data) {
   // Set the address size the first time we see it.
   if (addr_size == 0)
-addr_size = section_data.GetByteSize();
+addr_size = data.GetAddressByteSize();
 
-  llvm::StringRef data = llvm::toStringRef(section_data.GetData());
-  llvm::StringRef name = section.GetName().GetStringRef();
-  if (name.startswith("."))
-name = name.drop_front();
   section_map.try_emplace(
-  name, llvm::MemoryBuffer::getMemBuffer(data, name, false));
+  name, llvm::MemoryBuffer::getMemBuffer(toStringRef(data.GetData()),
+ name, false));
 };
 
-if (m_main_section_list) {
-  for (auto  : *m_main_section_list)
-AddSection(*section);
-}
-
-if (m_dwo_section_list) {
-  for (auto  : *m_dwo_section_list)
-AddSection(*section);
-}
+AddSection("debug_line_str", getOrLoadLineStrData());
 
 m_llvm_context = llvm::DWARFContext::create(section_map, addr_size);
   }


Index: lldb/test/Shell/SymbolFile/DWARF/debug-names-compressed.cpp
===
--- lldb/test/Shell/SymbolFile/DWARF/debug-names-compressed.cpp
+++ lldb/test/Shell/SymbolFile/DWARF/debug-names-compressed.cpp
@@ -3,12 +3,15 @@
 
 // REQUIRES: lld, zlib
 
-// RUN: %clang -g -c -o %t.o --target=x86_64-pc-linux -mllvm -accel-tables=Dwarf %s
+// RUN: %clang -c -o %t.o --target=x86_64-pc-linux -gdwarf-5 -gpubnames %s
 // RUN: ld.lld %t.o -o %t --compress-debug-sections=zlib
+// RUN: llvm-readobj --sections %t | FileCheck %s --check-prefix NAMES
 // RUN: lldb-test symbols --find=variable --name=foo %t | FileCheck %s
 
+// NAMES: Name: .debug_names
+
 // CHECK: Found 1 variables:
 int foo;
-// ONE-DAG: name = "foo", type = {{.*}} (int), {{.*}} decl = debug-names-compressed.cpp:[[@LINE-1]]

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

2020-01-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Not super ideal, but not too bad either. Not clicking accept yet because of the 
missing test case...


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

https://reviews.llvm.org/D72748



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


[Lldb-commits] [lldb] 791f132 - [lldb] Remove out of order OperatingSystemPython::Terminate call in SystemInitializerFull

2020-01-17 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-01-17T13:02:15+01:00
New Revision: 791f132132b2078cc0171e58332159cd5dafa55e

URL: 
https://github.com/llvm/llvm-project/commit/791f132132b2078cc0171e58332159cd5dafa55e
DIFF: 
https://github.com/llvm/llvm-project/commit/791f132132b2078cc0171e58332159cd5dafa55e.diff

LOG: [lldb] Remove out of order OperatingSystemPython::Terminate call in 
SystemInitializerFull

We already call it later in the method (which is in the right order as we 
Initialize it
at the of the constructor).

Added: 


Modified: 
lldb/source/API/SystemInitializerFull.cpp

Removed: 




diff  --git a/lldb/source/API/SystemInitializerFull.cpp 
b/lldb/source/API/SystemInitializerFull.cpp
index df5270f7234f..7e4398a042e2 100644
--- a/lldb/source/API/SystemInitializerFull.cpp
+++ b/lldb/source/API/SystemInitializerFull.cpp
@@ -388,9 +388,6 @@ void SystemInitializerFull::Terminate() {
   DynamicLoaderStatic::Terminate();
   DynamicLoaderWindowsDYLD::Terminate();
 
-#if LLDB_ENABLE_PYTHON
-  OperatingSystemPython::Terminate();
-#endif
 
   platform_freebsd::PlatformFreeBSD::Terminate();
   platform_linux::PlatformLinux::Terminate();



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


[Lldb-commits] [lldb] f2d41ad - [lldb] Add missing terminate calls to Python/Lua subsystems

2020-01-17 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-01-17T12:49:57+01:00
New Revision: f2d41ad0e7e0b6b44641eafa70ef76df6a618810

URL: 
https://github.com/llvm/llvm-project/commit/f2d41ad0e7e0b6b44641eafa70ef76df6a618810
DIFF: 
https://github.com/llvm/llvm-project/commit/f2d41ad0e7e0b6b44641eafa70ef76df6a618810.diff

LOG: [lldb] Add missing terminate calls to Python/Lua subsystems

Added: 


Modified: 
lldb/source/API/SystemInitializerFull.cpp

Removed: 




diff  --git a/lldb/source/API/SystemInitializerFull.cpp 
b/lldb/source/API/SystemInitializerFull.cpp
index 2bc53af91d00..df5270f7234f 100644
--- a/lldb/source/API/SystemInitializerFull.cpp
+++ b/lldb/source/API/SystemInitializerFull.cpp
@@ -414,6 +414,18 @@ void SystemInitializerFull::Terminate() {
   ObjectContainerBSDArchive::Terminate();
   ObjectContainerUniversalMachO::Terminate();
 
+#if LLDB_ENABLE_PYTHON
+  OperatingSystemPython::Terminate();
+#endif
+
+#if LLDB_ENABLE_PYTHON
+  ScriptInterpreterPython::Terminate();
+#endif
+
+#if LLDB_ENABLE_LUA
+  ScriptInterpreterLua::Terminate();
+#endif
+
   // Now shutdown the common parts, in reverse order.
   SystemInitializerCommon::Terminate();
 }



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


[Lldb-commits] [PATCH] D72909: Make SymbolFileDWARF::ParseLineTable use std::sort instead of insertion sort

2020-01-17 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added inline comments.



Comment at: lldb/source/Symbol/LineTable.cpp:172
+  LineSequenceImpl *seq_b = reinterpret_cast(sequence_b);
+  return (*this)(seq_a->m_entries.front(), seq_b->m_entries.front());
+}

Nit pick, this would also work without the `reinterpret_cast` and const 
arguments. E.g.:
```
lang=c++
bool LineTable::Entry::LessThanBinaryPredicate::
operator()(const LineSequence *sequence_a, const LineSequence *sequence_b) 
const {
  auto *seq_a = static_cast(sequence_a);
  auto *seq_b = static_cast(sequence_b);
  return (*this)(seq_a->m_entries.front(), seq_b->m_entries.front());
}
```


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D72909



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


[Lldb-commits] [PATCH] D72909: Make SymbolFileDWARF::ParseLineTable use std::sort instead of insertion sort

2020-01-17 Thread Unnar Freyr Erlendsson via Phabricator via lldb-commits
unnar added a comment.

In D72909#1826110 , @labath wrote:

> Thanks for the patch. I've been wondering how to improve this, and this 
> solution is pretty neat.
>
> I have just one request. Instead of the `ReplaceLineTableWithSequences` 
> thingy, could you just create a LineTable constructor, which takes an 
> `ArrayRef` or similar. It looks like all it takes is to make 
> `CreateLineSequenceContainer` static, so you can invoke it without a 
> `LineTable` instance. (There are other possible cleanups here too, but I 
> don't think its your job do fix all of those...)


Thanks for the quick review!

It also requires AppendLineEntryToSequence to be static but I assume that 
should also be just fine.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D72909



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


[Lldb-commits] [PATCH] D72909: Make SymbolFileDWARF::ParseLineTable use std::sort instead of insertion sort

2020-01-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Thanks for the patch. I've been wondering how to improve this, and this 
solution is pretty neat.

I have just one request. Instead of the `ReplaceLineTableWithSequences` thingy, 
could you just create a LineTable constructor, which takes an 
`ArrayRef` or similar. It looks like all it takes is to make 
`CreateLineSequenceContainer` static, so you can invoke it without a 
`LineTable` instance. (There are other possible cleanups here too, but I don't 
think its your job do fix all of those...)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D72909



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


[Lldb-commits] [PATCH] D72909: Make SymbolFileDWARF::ParseLineTable use std::sort instead of insertion sort

2020-01-17 Thread Unnar Freyr Erlendsson via Phabricator via lldb-commits
unnar created this revision.
unnar added a project: LLDB.
Herald added subscribers: lldb-commits, JDevlieghere, mgrang.

Motivation: When setting breakpoints in certain projects line sequences are 
frequently being inserted out of order.

Rather than inserting sequences one at a time into a sorted line table, store 
all the line sequences as we're building them up and sort and flatten 
afterwards.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D72909

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


Index: lldb/source/Symbol/LineTable.cpp
===
--- lldb/source/Symbol/LineTable.cpp
+++ lldb/source/Symbol/LineTable.cpp
@@ -130,6 +130,17 @@
   m_entries.insert(pos, seq->m_entries.begin(), seq->m_entries.end());
 }
 
+void LineTable::ReplaceLineTableWithSequences(std::vector 
) {
+  m_entries.clear();
+  LineTable::Entry::LessThanBinaryPredicate less_than_bp(this);
+  std::sort(sequences.begin(), sequences.end(), less_than_bp);
+  for (auto *sequence : sequences) {
+LineSequenceImpl *seq = reinterpret_cast(sequence);
+m_entries.insert(m_entries.end(), seq->m_entries.begin(),
+  seq->m_entries.end());
+  }
+}
+
 LineTable::Entry::LessThanBinaryPredicate::LessThanBinaryPredicate(
 LineTable *line_table)
 : m_line_table(line_table) {}
@@ -154,6 +165,13 @@
 #undef LT_COMPARE
 }
 
+bool LineTable::Entry::LessThanBinaryPredicate::
+operator()(LineSequence *sequence_a, LineSequence *sequence_b) const {
+  LineSequenceImpl *seq_a = reinterpret_cast(sequence_a);
+  LineSequenceImpl *seq_b = reinterpret_cast(sequence_b);
+  return (*this)(seq_a->m_entries.front(), seq_b->m_entries.front());
+}
+
 uint32_t LineTable::GetSize() const { return m_entries.size(); }
 
 bool LineTable::GetLineEntryAtIndex(uint32_t idx, LineEntry _entry) {
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1018,17 +1018,20 @@
   std::unique_ptr line_table_up =
   std::make_unique(_unit);
   LineSequence *sequence = line_table_up->CreateLineSequenceContainer();
+  std::vector sequences;
   for (auto  : line_table->Rows) {
 line_table_up->AppendLineEntryToSequence(
 sequence, row.Address.Address, row.Line, row.Column, row.File,
 row.IsStmt, row.BasicBlock, row.PrologueEnd, row.EpilogueBegin,
 row.EndSequence);
 if (row.EndSequence) {
-  line_table_up->InsertSequence(sequence);
+  sequences.push_back(sequence);
   sequence = line_table_up->CreateLineSequenceContainer();
 }
   }
 
+  line_table_up->ReplaceLineTableWithSequences(sequences);
+
   if (SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile()) {
 // We have an object file that has a line table with addresses that are not
 // linked. We need to link the line table and convert the addresses that
Index: lldb/include/lldb/Symbol/LineTable.h
===
--- lldb/include/lldb/Symbol/LineTable.h
+++ lldb/include/lldb/Symbol/LineTable.h
@@ -78,6 +78,12 @@
   // Insert a sequence of entries into this line table.
   void InsertSequence(LineSequence *sequence);
 
+  /// Replace the current line table with entries found in \a sequences.
+  ///
+  /// \param[in] sequences
+  /// Unsorted list of line sequences.
+  void ReplaceLineTableWithSequences(std::vector );
+
   /// Dump all line entries in this line table to the stream \a s.
   ///
   /// \param[in] s
@@ -259,6 +265,7 @@
 public:
   LessThanBinaryPredicate(LineTable *line_table);
   bool operator()(const LineTable::Entry &, const LineTable::Entry &) 
const;
+  bool operator()(LineSequence*, LineSequence*) const;
 
 protected:
   LineTable *m_line_table;


Index: lldb/source/Symbol/LineTable.cpp
===
--- lldb/source/Symbol/LineTable.cpp
+++ lldb/source/Symbol/LineTable.cpp
@@ -130,6 +130,17 @@
   m_entries.insert(pos, seq->m_entries.begin(), seq->m_entries.end());
 }
 
+void LineTable::ReplaceLineTableWithSequences(std::vector ) {
+  m_entries.clear();
+  LineTable::Entry::LessThanBinaryPredicate less_than_bp(this);
+  std::sort(sequences.begin(), sequences.end(), less_than_bp);
+  for (auto *sequence : sequences) {
+LineSequenceImpl *seq = reinterpret_cast(sequence);
+m_entries.insert(m_entries.end(), seq->m_entries.begin(),
+  seq->m_entries.end());
+  }
+}
+
 LineTable::Entry::LessThanBinaryPredicate::LessThanBinaryPredicate(
 LineTable *line_table)
 : m_line_table(line_table) {}
@@ -154,6 +165,13 @@
 #undef LT_COMPARE
 }
 
+bool LineTable::Entry::LessThanBinaryPredicate::
+operator()(LineSequence *sequence_a, LineSequence 

[Lldb-commits] [lldb] c3ab790 - [lldb][NFC] Resynchronize Init/Terminate calls in SystemInitializerFull/Test.cpp files.

2020-01-17 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-01-17T11:34:59+01:00
New Revision: c3ab790c8f5d2946c3e4e4bf78cedf6be11a6f5a

URL: 
https://github.com/llvm/llvm-project/commit/c3ab790c8f5d2946c3e4e4bf78cedf6be11a6f5a
DIFF: 
https://github.com/llvm/llvm-project/commit/c3ab790c8f5d2946c3e4e4bf78cedf6be11a6f5a.diff

LOG: [lldb][NFC] Resynchronize Init/Terminate calls in 
SystemInitializerFull/Test.cpp files.

These files should do the more or less the same initialize/terminate calls in 
the
same order. This just reverts all the differences that have piled up over time
in the SystemInitializerTest that people keep forgetting about.

Added: 


Modified: 
lldb/tools/lldb-test/SystemInitializerTest.cpp

Removed: 




diff  --git a/lldb/tools/lldb-test/SystemInitializerTest.cpp 
b/lldb/tools/lldb-test/SystemInitializerTest.cpp
index db0a9d464146..409ce2ed0bc1 100644
--- a/lldb/tools/lldb-test/SystemInitializerTest.cpp
+++ b/lldb/tools/lldb-test/SystemInitializerTest.cpp
@@ -18,6 +18,7 @@
 #include "Plugins/ABI/MacOSX-arm/ABIMacOSX_arm.h"
 #include "Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.h"
 #include "Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.h"
+#include "Plugins/ABI/SysV-arc/ABISysV_arc.h"
 #include "Plugins/ABI/SysV-arm/ABISysV_arm.h"
 #include "Plugins/ABI/SysV-arm64/ABISysV_arm64.h"
 #include "Plugins/ABI/SysV-hexagon/ABISysV_hexagon.h"
@@ -30,6 +31,7 @@
 #include "Plugins/ABI/SysV-x86_64/ABISysV_x86_64.h"
 #include "Plugins/ABI/Windows-x86_64/ABIWindows_x86_64.h"
 #include "Plugins/Architecture/Arm/ArchitectureArm.h"
+#include "Plugins/Architecture/Mips/ArchitectureMips.h"
 #include "Plugins/Architecture/PPC64/ArchitecturePPC64.h"
 #include "Plugins/Disassembler/llvm/DisassemblerLLVMC.h"
 #include "Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h"
@@ -37,7 +39,10 @@
 #include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
+#include "Plugins/Instruction/ARM/EmulateInstructionARM.h"
 #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h"
+#include "Plugins/Instruction/MIPS/EmulateInstructionMIPS.h"
+#include "Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h"
 #include "Plugins/Instruction/PPC64/EmulateInstructionPPC64.h"
 #include "Plugins/InstrumentationRuntime/ASan/ASanRuntime.h"
 #include 
"Plugins/InstrumentationRuntime/MainThreadChecker/MainThreadCheckerRuntime.h"
@@ -52,6 +57,8 @@
 #include "Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h"
 #include 
"Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h"
 #include "Plugins/MemoryHistory/asan/MemoryHistoryASan.h"
+#include "Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h"
+#include 
"Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h"
 #include "Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h"
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
 #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
@@ -68,6 +75,7 @@
 #include "Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h"
 #include "Plugins/Process/elf-core/ProcessElfCore.h"
 #include "Plugins/Process/gdb-remote/ProcessGDBRemote.h"
+#include "Plugins/Process/mach-core/ProcessMachCore.h"
 #include "Plugins/Process/minidump/ProcessMinidump.h"
 #include "Plugins/ScriptInterpreter/None/ScriptInterpreterNone.h"
 #include "Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h"
@@ -86,11 +94,11 @@
 #include "Plugins/Platform/MacOSX/PlatformAppleTVSimulator.h"
 #include "Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.h"
 #include "Plugins/Platform/MacOSX/PlatformDarwinKernel.h"
+#include "Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.h"
 #include "Plugins/Platform/MacOSX/PlatformRemoteAppleTV.h"
 #include "Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.h"
 #include "Plugins/Platform/MacOSX/PlatformiOSSimulator.h"
 #include "Plugins/Process/MacOSX-Kernel/ProcessKDP.h"
-#include "Plugins/Process/mach-core/ProcessMachCore.h"
 #include "Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.h"
 #endif
 #include "Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.h"
@@ -156,8 +164,10 @@ llvm::Error SystemInitializerTest::Initialize() {
   ObjectFilePECOFF::Initialize();
   wasm::ObjectFileWasm::Initialize();
 
-  ScriptInterpreterNone::Initialize();
+  ObjectContainerBSDArchive::Initialize();
+  ObjectContainerUniversalMachO::Initialize();
 
+  ScriptInterpreterNone::Initialize();
 
   platform_freebsd::PlatformFreeBSD::Initialize();
   platform_linux::PlatformLinux::Initialize();
@@ -184,12 +194,14 @@ llvm::Error SystemInitializerTest::Initialize() {
 #include "llvm/Config/Targets.def"
 
   ArchitectureArm::Initialize();
+  ArchitectureMips::Initialize();
   ArchitecturePPC64::Initialize();
 
   DisassemblerLLVMC::Initialize();
 
   JITLoaderGDB::Initialize();
   

[Lldb-commits] [PATCH] D72823: [Reproducers] Add a tool to transparently capture and replay lldb sessions

2020-01-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/test/Shell/helper/toolchain.py:13
+
+def _lldb_init(config):
+return os.path.join(config.test_exec_root, 'Shell', 'lit-lldb-init')

Maybe `_get_lldb_init_path`? This way it looks like this function is doing some 
fancy initialization..



Comment at: lldb/test/Shell/lit.cfg.py:42-46
 if 'LLDB_CAPTURE_REPRODUCER' in os.environ:
   config.environment['LLDB_CAPTURE_REPRODUCER'] = os.environ[
   'LLDB_CAPTURE_REPRODUCER']
+if 'LLDB_REPRO_CAPTURE' in os.environ:
+  config.environment['LLDB_REPRO_CAPTURE'] = os.environ['LLDB_REPRO_CAPTURE']

aprantl wrote:
> JDevlieghere wrote:
> > labath wrote:
> > > The difference between these two is super-unclear. Any chance to unify 
> > > them?
> > Not really, one is for the tool, the other is to enable reproducer capture 
> > by default. They're totally orthogonal, but I agree it's confusing. 
> How about adding this as a comment here?
Yes, that's better now, though, per the other comment, I think the env vars can 
be removed completely.



Comment at: lldb/tools/lldb-repro/lldb-repro.cpp:63
+args.push_back(repro_dir.c_str());
+args.push_back("--reproducer-auto-generate");
+

JDevlieghere wrote:
> labath wrote:
> > I am wondering if this shouldn't even be the default behavior. If I already 
> > passed `--capture` to lldb then it does not seem unreasonable to always 
> > write it out when lldb exits (we already do it after a crash, right?)
> My ultimate goal is to have reproducers enabled by default. The capture flag 
> is just away to make that behavior opt-in for now. You might use it to get a 
> reproducer on a crash, but you don't necessary want to keep all reproducers 
> around otherwise. 
Ok, that makes sense. That said, I think we will always want a way to disable 
this -- this just looks too much like one of those "this shit is tracking 
everything I do" features a lot of people will want to stay away from.



Comment at: lldb/tools/lldb-repro/lldb-repro.h.cmake:12
+
+#cmakedefine LLDB_TEST_EXECUTABLE "${LLDB_TEST_EXECUTABLE}"
+

JDevlieghere wrote:
> labath wrote:
> > labath wrote:
> > > Are you sure this will work fine with multi-config generators? You might 
> > > be better off just relying on the fact that the lldb executable will sit 
> > > right next to this binary...
> > Actually how, is this thing going to be invoked exactly? Couldn't the path 
> > to lldb be passed simply as argv[1]?
> It just needs patching up like lldb-dotest and lit. Assuming you mean 
> `argv[0]`, it think we could make that work if I replace "%lldb" with a path 
> to lldb-repro.
No, I really meant argv[1]. :)

The idea was that `%lldb` would expand to `/src/path/to/lldb-repro.py 
/build/path/to/lldb.exe --whatever`. That way, you wouldn't need to rely on the 
"same directory" trick and could get rid of all the cmake code. In fact, we 
could even throw in a `--capture/--replay` argument to the command line, and 
ditch the environment variables too...


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

https://reviews.llvm.org/D72823



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


[Lldb-commits] [lldb] 6b84083 - [lldb][NFC] Delete unused lldb/source/Plugins/LanguageRuntime/Go/CMakeLists.txt

2020-01-17 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-01-17T09:57:44+01:00
New Revision: 6b840834cd508aa673a30074ebd4649100bc8d9a

URL: 
https://github.com/llvm/llvm-project/commit/6b840834cd508aa673a30074ebd4649100bc8d9a
DIFF: 
https://github.com/llvm/llvm-project/commit/6b840834cd508aa673a30074ebd4649100bc8d9a.diff

LOG: [lldb][NFC] Delete unused 
lldb/source/Plugins/LanguageRuntime/Go/CMakeLists.txt

Added: 


Modified: 


Removed: 
lldb/source/Plugins/LanguageRuntime/Go/CMakeLists.txt



diff  --git a/lldb/source/Plugins/LanguageRuntime/Go/CMakeLists.txt 
b/lldb/source/Plugins/LanguageRuntime/Go/CMakeLists.txt
deleted file mode 100644
index 62418def58f6..
--- a/lldb/source/Plugins/LanguageRuntime/Go/CMakeLists.txt
+++ /dev/null
@@ -1,11 +0,0 @@
-set(LLVM_NO_RTTI 1)
-
-add_lldb_library(lldbPluginLanguageRuntimeGo PLUGIN
-  LINK_LIBS
-lldbBreakpoint
-lldbCore
-lldbSymbol
-lldbTarget
-  LINK_COMPONENTS
-Support
-  )



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


[Lldb-commits] [PATCH] D72880: Fix a buffer-size bug when the first DW_OP_piece is undefined

2020-01-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:2077
+  // with the expression result, so the debugger can print missing
+  // members as "" or something.
   ::memset(curr_piece.GetBuffer().GetBytes(), 0, piece_byte_size);

vsk wrote:
> FTR I'm not really sure this case is allowed by the dwarf standard. Under 
> DW_OP_piece the standard references a "preceding simple location description" 
> -- istm like an error if the stack is empty. My $0.0002 :) 
I think this is fine -- an "empty location description" (section 2.6.1.1.4 of 
DWARF4) is a kind of a "simple location description" (sect. 2.6.1.1).

The part that worries me more is this: "Each simple location description that 
is a DWARF expression is evaluated independently of any others (as though on 
its own separate stack, if any).". I think that means we should reset/clear the 
stack after each DW_OP_piece, but we don't currently do that (we just pop the 
topmost value). However, I am not sure if this has any impact on valid DWARF 
expressions (though it can cause us to produce bogus values for invalid ones).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72880



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


[Lldb-commits] [PATCH] D72813: Fixes to lldb's eLaunchFlagLaunchInTTY feature on macOS

2020-01-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D72813#1825027 , @jasonmolenda 
wrote:

>   If we attach while darwin-debug is executing, then we get the exec mach 
> exception, 


Not really my thing, but couldn't you just ensure that you *always* attach 
while darwin-debug is executing? E.g., the binary could do something like:

  send_pid();
  while(!am_i_being_debugged()) usleep(1000);
  posix_spawn(...);

I had sort of assumed this wasn't possible because of SIP, but your comment 
makes it sounds that this does work at least sometimes...

BTW, is quite unfortunate that the CLOEXEC trick doesn't work. We use that for 
launching on linux, and it's pretty neat. The best part about it is that if the 
execve() fails, you still get to keep the fd and can send an error message 
about what went wrong. This bit of code uses the libc api directly, so I don't 
know if the problem here is with the lldb Read function or the darwin system 
apis...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72813



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