[Lldb-commits] [PATCH] D92187: [lldb] [POSIX-DYLD] Add libraries from initial rendezvous brkpt hit

2020-12-17 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.

Let's give this another shot.


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

https://reviews.llvm.org/D92187

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


[Lldb-commits] [PATCH] D93299: [lldb] [unittests] Add tests for NetBSD register offsets/sizes

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



Comment at: lldb/unittests/Process/Utility/CMakeLists.txt:4-5
   RegisterContextFreeBSDTest.cpp
+  RegisterContextNetBSDTest_i386.cpp
+  RegisterContextNetBSDTest_x86_64.cpp
   LinuxProcMapsTest.cpp

What's reason for having these as separate files (vs. the all-in-one freebsd 
approach)?



Comment at: lldb/unittests/Process/Utility/RegisterContextNetBSDTest_i386.cpp:9
+
+#if defined(__NetBSD__)
+

I guess we could now (that we have other, unconditionally compiled files), 
upgrade this to `if(CMAKE_SYSTEM_NAME == NetBSD`)


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

https://reviews.llvm.org/D93299

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


[Lldb-commits] [PATCH] D93299: [lldb] [unittests] Add tests for NetBSD register offsets/sizes

2020-12-17 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: lldb/unittests/Process/Utility/CMakeLists.txt:4-5
   RegisterContextFreeBSDTest.cpp
+  RegisterContextNetBSDTest_i386.cpp
+  RegisterContextNetBSDTest_x86_64.cpp
   LinuxProcMapsTest.cpp

labath wrote:
> What's reason for having these as separate files (vs. the all-in-one freebsd 
> approach)?
`struct reg` collision. NetBSD doesn't have unique names like FreeBSD does.


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

https://reviews.llvm.org/D93299

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


[Lldb-commits] [lldb] dbfdb13 - [lldb] [POSIX-DYLD] Update the cached exe path after attach

2020-12-17 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-12-17T09:31:22+01:00
New Revision: dbfdb139f75470a9abc78e7c9faf743fdd963c2d

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

LOG: [lldb] [POSIX-DYLD] Update the cached exe path after attach

Fix the POSIX-DYLD plugin to update the cached executable path after
attaching.  Previously, the path was cached in DYLDRendezvous
constructor and not updated afterwards.  This meant that if LLDB was
attaching to a process (e.g. via connecting to lldb-server), the code
stored the empty path before DidAttach() resolved it.  The fix updates
the cached path in DidAttach().

This fixes a new instance of https://llvm.org/pr17880

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

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
lldb/test/API/commands/process/attach/TestProcessAttach.py
lldb/test/API/commands/process/attach/main.cpp

Removed: 




diff  --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
index 15b3805003a5..866acbddbdc8 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -94,12 +94,13 @@ DYLDRendezvous::DYLDRendezvous(Process *process)
 : m_process(process), m_rendezvous_addr(LLDB_INVALID_ADDRESS), m_current(),
   m_previous(), m_loaded_modules(), m_soentries(), m_added_soentries(),
   m_removed_soentries() {
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
-
   m_thread_info.valid = false;
+  UpdateExecutablePath();
+}
 
-  // Cache a copy of the executable path
+void DYLDRendezvous::UpdateExecutablePath() {
   if (m_process) {
+Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
 Module *exe_mod = m_process->GetTarget().GetExecutableModulePointer();
 if (exe_mod) {
   m_exe_file_spec = exe_mod->GetPlatformFileSpec();

diff  --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
index b028120eb0d4..3e88d88f407a 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
@@ -60,6 +60,9 @@ class DYLDRendezvous {
 
   DYLDRendezvous(lldb_private::Process *process);
 
+  /// Update the cached executable path.
+  void UpdateExecutablePath();
+
   /// Update the internal snapshot of runtime linker rendezvous and recompute
   /// the currently loaded modules.
   ///

diff  --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 01f746ada3ba..160faa74af23 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -102,6 +102,7 @@ void DynamicLoaderPOSIXDYLD::DidAttach() {
 
   ModuleSP executable_sp = GetTargetExecutable();
   ResolveExecutableModule(executable_sp);
+  m_rendezvous.UpdateExecutablePath();
 
   // find the main process load offset
   addr_t load_offset = ComputeLoadOffset();

diff  --git a/lldb/test/API/commands/process/attach/TestProcessAttach.py 
b/lldb/test/API/commands/process/attach/TestProcessAttach.py
index 5dfec5c76339..88429333edd3 100644
--- a/lldb/test/API/commands/process/attach/TestProcessAttach.py
+++ b/lldb/test/API/commands/process/attach/TestProcessAttach.py
@@ -20,6 +20,13 @@ class ProcessAttachTestCase(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
 
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Find the line number to break for main.c.
+self.line = line_number('main.cpp',
+'// Waiting to be attached...')
+
 @skipIfiOSSimulator
 def test_attach_to_process_by_id(self):
 """Test attach by process id"""
@@ -77,6 +84,28 @@ def test_attach_to_process_by_name(self):
 process = target.GetProcess()
 self.assertTrue(process, PROCESS_IS_VALID)
 
+def test_attach_to_process_by_id_correct_executable_offset(self):
+"""
+Test that after attaching to a process the executable offset
+is determined correctly on FreeBSD.  This is a regression test
+for dyld plugin getting the correct executable path,
+and therefore being able to identify it in the module list.
+"""
+
+self.build()
+exe = self.getBuildArtifact(exe_name)
+
+# In order to reproduce, w

[Lldb-commits] [lldb] 8666b90 - [lldb] [POSIX-DYLD] Add libraries from initial rendezvous brkpt hit

2020-12-17 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-12-17T09:31:10+01:00
New Revision: 8666b9057f23badfe90548297f3c01937daa4a9c

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

LOG: [lldb] [POSIX-DYLD] Add libraries from initial rendezvous brkpt hit

Explicitly consider the libraries reported on the initial rendezvous
breakpoint hit added.  This is necessary on FreeBSD since the dynamic
loader issues only a single 'consistent' state rendezvous breakpoint hit
for all the libraries present in DT_NEEDED.  It is also helpful on Linux
where it ensures that ld-linux is considered loaded as well
as the shared system libraries reported afterwards.

Reenable memory maps on FreeBSD since this fixed the issue triggered
by them.

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

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
lldb/test/API/api/multithreaded/TestMultithreaded.py

lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index ac60af5336ed..01f746ada3ba 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -76,7 +76,8 @@ DynamicLoaderPOSIXDYLD::DynamicLoaderPOSIXDYLD(Process 
*process)
   m_load_offset(LLDB_INVALID_ADDRESS), m_entry_point(LLDB_INVALID_ADDRESS),
   m_auxv(), m_dyld_bid(LLDB_INVALID_BREAK_ID),
   m_vdso_base(LLDB_INVALID_ADDRESS),
-  m_interpreter_base(LLDB_INVALID_ADDRESS) {}
+  m_interpreter_base(LLDB_INVALID_ADDRESS), m_initial_modules_added(false) 
{
+}
 
 DynamicLoaderPOSIXDYLD::~DynamicLoaderPOSIXDYLD() {
   if (m_dyld_bid != LLDB_INVALID_BREAK_ID) {
@@ -418,14 +419,38 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
 
   ModuleList &loaded_modules = m_process->GetTarget().GetImages();
 
-  if (m_rendezvous.ModulesDidLoad()) {
+  if (m_rendezvous.ModulesDidLoad() || !m_initial_modules_added) {
 ModuleList new_modules;
 
-E = m_rendezvous.loaded_end();
-for (I = m_rendezvous.loaded_begin(); I != E; ++I) {
+// If this is the first time rendezvous breakpoint fires, we need
+// to take care of adding all the initial modules reported by
+// the loader.  This is necessary to list ld-linux.so on Linux,
+// and all DT_NEEDED entries on *BSD.
+if (m_initial_modules_added) {
+  I = m_rendezvous.loaded_begin();
+  E = m_rendezvous.loaded_end();
+} else {
+  I = m_rendezvous.begin();
+  E = m_rendezvous.end();
+  m_initial_modules_added = true;
+}
+for (; I != E; ++I) {
   ModuleSP module_sp =
   LoadModuleAtAddress(I->file_spec, I->link_addr, I->base_addr, true);
   if (module_sp.get()) {
+if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
+&m_process->GetTarget()) == m_interpreter_base &&
+module_sp != m_interpreter_module.lock()) {
+  // If this is a duplicate instance of ld.so, unload it.  We may end 
up
+  // with it if we load it via a 
diff erent path than before (symlink
+  // vs real path).
+  // TODO: remove this once we either fix library matching or avoid
+  // loading the interpreter when setting the rendezvous breakpoint.
+  UnloadSections(module_sp);
+  loaded_modules.Remove(module_sp);
+  continue;
+}
+
 loaded_modules.AppendIfNeeded(module_sp);
 new_modules.Append(module_sp);
   }
@@ -544,6 +569,7 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() {
 true /* notify */)) {
 UpdateLoadedSections(module_sp, LLDB_INVALID_ADDRESS, m_interpreter_base,
  false);
+m_interpreter_module = module_sp;
 return module_sp;
   }
   return nullptr;

diff  --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
index a7fcdfbadeaf..61567801fdd0 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ b/lldb/source/Plugins/DynamicLoader

[Lldb-commits] [PATCH] D92187: [lldb] [POSIX-DYLD] Add libraries from initial rendezvous brkpt hit

2020-12-17 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8666b9057f23: [lldb] [POSIX-DYLD] Add libraries from initial 
rendezvous brkpt hit (authored by mgorny).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92187

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
  lldb/test/API/api/multithreaded/TestMultithreaded.py
  
lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
  
lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
  lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test

Index: lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test
===
--- lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test
+++ lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test
@@ -3,7 +3,6 @@
 
 # REQUIRES: target-x86_64
 # UNSUPPORTED: system-windows
-# XFAIL: system-freebsd
 
 # RUN: %clang_host %p/Inputs/call-asm.c -x assembler-with-cpp %p/Inputs/thread-step-out-ret-addr-check.s -o %t
 # RUN: not %lldb %t -s %s -b 2>&1 | FileCheck %s
Index: lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
===
--- lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
+++ lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
@@ -120,7 +120,7 @@
 
 @llgs_test
 @skipUnlessPlatform(["linux", "android", "freebsd", "netbsd"])
-@expectedFailureAll(oslist=["freebsd", "netbsd"])
+@expectedFailureNetBSD
 def test_libraries_svr4_load_addr(self):
 self.setup_test()
 self.libraries_svr4_has_correct_load_addr()
Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -843,7 +843,6 @@
 self.qMemoryRegionInfo_is_supported()
 
 @llgs_test
-@expectedFailureAll(oslist=["freebsd"])
 def test_qMemoryRegionInfo_is_supported_llgs(self):
 self.init_llgs_test()
 self.build()
@@ -908,7 +907,6 @@
 self.qMemoryRegionInfo_reports_code_address_as_executable()
 
 @skipIfWindows # No pty support to test any inferior output
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test_qMemoryRegionInfo_reports_code_address_as_executable_llgs(self):
 self.init_llgs_test()
@@ -975,7 +973,6 @@
 self.qMemoryRegionInfo_reports_stack_address_as_readable_writeable()
 
 @skipIfWindows # No pty support to test any inferior output
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test_qMemoryRegionInfo_reports_stack_address_as_readable_writeable_llgs(
 self):
@@ -1042,7 +1039,6 @@
 self.qMemoryRegionInfo_reports_heap_address_as_readable_writeable()
 
 @skipIfWindows # No pty support to test any inferior output
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test_qMemoryRegionInfo_reports_heap_address_as_readable_writeable_llgs(
 self):
Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -23,7 +23,6 @@
 'main.cpp',
 '// Run here before printing memory regions')
 
-@expectedFailureAll(oslist=["freebsd"])
 def test(self):
 self.build()
 
Index: lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
===
--- lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
+++ lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
@@ -15,7 +15,6 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
-@expectedFailureAll(oslist=["freebsd"], bugnumber='llvm.org/pr48373')
 @expectedFailureNetBSD
 def test(self):
 self.build()
Index: lldb/test/API/api/multithreaded/TestMultithreaded.py
===
--- lldb/test/API/api/multithreaded/TestMultithreaded.py
+++ lldb/test/API/api/multithreaded/TestMultithreaded.py
@@ -31,7 +31,6 @@
 @skipIfNoSBHeaders
 

[Lldb-commits] [PATCH] D92264: [lldb] [POSIX-DYLD] Update the cached exe path after attach

2020-12-17 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdbfdb139f754: [lldb] [POSIX-DYLD] Update the cached exe path 
after attach (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92264

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/test/API/commands/process/attach/TestProcessAttach.py
  lldb/test/API/commands/process/attach/main.cpp

Index: lldb/test/API/commands/process/attach/main.cpp
===
--- lldb/test/API/commands/process/attach/main.cpp
+++ lldb/test/API/commands/process/attach/main.cpp
@@ -3,6 +3,8 @@
 #include 
 #include 
 
+volatile int g_val = 12345;
+
 int main(int argc, char const *argv[]) {
 int temp;
 lldb_enable_attach();
Index: lldb/test/API/commands/process/attach/TestProcessAttach.py
===
--- lldb/test/API/commands/process/attach/TestProcessAttach.py
+++ lldb/test/API/commands/process/attach/TestProcessAttach.py
@@ -20,6 +20,13 @@
 
 NO_DEBUG_INFO_TESTCASE = True
 
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Find the line number to break for main.c.
+self.line = line_number('main.cpp',
+'// Waiting to be attached...')
+
 @skipIfiOSSimulator
 def test_attach_to_process_by_id(self):
 """Test attach by process id"""
@@ -77,6 +84,28 @@
 process = target.GetProcess()
 self.assertTrue(process, PROCESS_IS_VALID)
 
+def test_attach_to_process_by_id_correct_executable_offset(self):
+"""
+Test that after attaching to a process the executable offset
+is determined correctly on FreeBSD.  This is a regression test
+for dyld plugin getting the correct executable path,
+and therefore being able to identify it in the module list.
+"""
+
+self.build()
+exe = self.getBuildArtifact(exe_name)
+
+# In order to reproduce, we must spawn using a relative path
+popen = self.spawnSubprocess(os.path.relpath(exe))
+
+self.runCmd("process attach -p " + str(popen.pid))
+
+# Make suer we did not attach to early
+lldbutil.run_break_set_by_file_and_line(
+self, "main.cpp", self.line, num_expected_locations=1, loc_exact=False)
+self.runCmd("process continue")
+self.expect("p g_val", substrs=["$0 = 12345"])
+
 def tearDown(self):
 # Destroy process before TestBase.tearDown()
 self.dbg.GetSelectedTarget().GetProcess().Destroy()
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -102,6 +102,7 @@
 
   ModuleSP executable_sp = GetTargetExecutable();
   ResolveExecutableModule(executable_sp);
+  m_rendezvous.UpdateExecutablePath();
 
   // find the main process load offset
   addr_t load_offset = ComputeLoadOffset();
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
@@ -60,6 +60,9 @@
 
   DYLDRendezvous(lldb_private::Process *process);
 
+  /// Update the cached executable path.
+  void UpdateExecutablePath();
+
   /// Update the internal snapshot of runtime linker rendezvous and recompute
   /// the currently loaded modules.
   ///
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -94,12 +94,13 @@
 : m_process(process), m_rendezvous_addr(LLDB_INVALID_ADDRESS), m_current(),
   m_previous(), m_loaded_modules(), m_soentries(), m_added_soentries(),
   m_removed_soentries() {
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
-
   m_thread_info.valid = false;
+  UpdateExecutablePath();
+}
 
-  // Cache a copy of the executable path
+void DYLDRendezvous::UpdateExecutablePath() {
   if (m_process) {
+Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
 Module *exe_mod = m_process->GetTarget().GetExecutableModulePointer();
 if (exe_mod) {
   m_exe_file_spec = exe_mod->GetPlatformFileSpec();
___
lldb-commi

[Lldb-commits] [lldb] 722247c - [lldb] Unify the two CreateTypedef implementations in TypeSystemClang

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

Author: Raphael Isemann
Date: 2020-12-17T10:49:26+01:00
New Revision: 722247c8124a6b840686757ae128b16cea248130

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

LOG: [lldb] Unify the two CreateTypedef implementations in TypeSystemClang

To get LLDB one step closer to fulfil the software redundancy requirements of
modern aircrafts, we apparently decided to have two separately maintained
implementations of `CreateTypedef` in TypeSystemClang. Let's pass on the idea of
an LLDB-powered jetliner and deleted one implementation.

On a more serious note: This function got duplicated a long time ago when the
idea of CompilerType with a backing TypeSystemClang subclass happened
(56939cb31061d24ae3d1fc62da38b57e78bb2556). One implementation was supposed to
be called from CompilerType::CreateTypedef and the other has just always been
around to create typedefs. By accident one of the implementations is only used
by the PDB parser while the CompilerType::CreateTypedef backend is used by the
rest of LLDB.

We also had some patches over the year that only fixed one of the two functions
(D18099 for example only fixed up the CompilerType::CreateTypedef
implementation). D51162 and D86140 both fixed the same missing `addDecl` call
for one of the two implementations.

This patch:
* deletes the `CreateTypedefType` function as its only used by the PDB parser
  and the `CreateTypedef` implementation is anyway needed as it's the backend
  implementation of CompilerType.
* replaces the calls in the PDB parser by just calling the CompilerType wrapper.
* moves the documentation to the remaining function.
* moves the check for empty typedef names that was only in the deleted
  implementation to the other (I don't think this fixes anything as I believe
  all callers are already doing the same check).

I'll fix up the usual stuff (not using StringRef, not doing early exit) in a NFC
follow-up.

This patch is not NFC as the PDB parser now calls the function that has the fix
from D18099.

Reviewed By: labath, JDevlieghere

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/unittests/Symbol/TestTypeSystemClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index 21f8b13bf07f..5b4ab78ac219 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -881,8 +881,8 @@ PdbAstBuilder::GetOrCreateTypedefDecl(PdbGlobalSymId id) {
 
   std::string uname = std::string(DropNameScope(udt.Name));
 
-  CompilerType ct = m_clang.CreateTypedefType(ToCompilerType(qt), 
uname.c_str(),
-  ToCompilerDeclContext(*scope), 
0);
+  CompilerType ct = ToCompilerType(qt).CreateTypedef(
+  uname.c_str(), ToCompilerDeclContext(*scope), 0);
   clang::TypedefNameDecl *tnd = m_clang.GetAsTypedefDecl(ct);
   DeclStatus status;
   status.resolved = true;

diff  --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp 
b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
index 7649e8a90f9a..f9c12e634140 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -550,8 +550,8 @@ lldb::TypeSP PDBASTParser::CreateLLDBTypeFromPDBType(const 
PDBSymbol &type) {
 if (!ast_typedef.IsValid()) {
   CompilerType target_ast_type = target_type->GetFullCompilerType();
 
-  ast_typedef = m_ast.CreateTypedefType(
-  target_ast_type, name.c_str(), m_ast.CreateDeclContext(decl_ctx), 0);
+  ast_typedef = target_ast_type.CreateTypedef(
+  name.c_str(), m_ast.CreateDeclContext(decl_ctx), 0);
   if (!ast_typedef)
 return nullptr;
 

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index f46b145da66c..643ea7e02206 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4402,39 +4402,6 @@ 
TypeSystemClang::GetNonReferenceType(lldb::opaque_compiler_type_t type) {
   return CompilerType();
 }
 
-CompilerType TypeSystemClang::CreateTypedefType(
-const CompilerType &type, const char *typedef_name,
-const CompilerDeclContext &compiler_decl_ctx, uint32_t payload) {
-  if (type && typedef_name && typedef_name[0]) {
-TypeSystemClang *ast =
-llvm::dyn_cast(type.GetTypeSystem());
-if (!ast

[Lldb-commits] [PATCH] D93382: [lldb] Unify the two CreateTypedef implementations in TypeSystemClang

2020-12-17 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG722247c8124a: [lldb] Unify the two CreateTypedef 
implementations in TypeSystemClang (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93382

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/unittests/Symbol/TestTypeSystemClang.cpp

Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -482,9 +482,8 @@
   m_ast->CompleteTagDeclarationDefinition(type);
 
   // typedef foo foo_def;
-  CompilerType typedef_type = m_ast->CreateTypedefType(
-  type, "foo_def",
-  m_ast->CreateDeclContext(m_ast->GetTranslationUnitDecl()), 0);
+  CompilerType typedef_type = type.CreateTypedef(
+  "foo_def", m_ast->CreateDeclContext(m_ast->GetTranslationUnitDecl()), 0);
 
   CompilerType auto_type(
   m_ast.get(),
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -665,14 +665,6 @@
 
   // Creating related types
 
-  /// Using the current type, create a new typedef to that type using
-  /// "typedef_name" as the name and "decl_ctx" as the decl context.
-  /// \param opaque_payload is an opaque TypePayloadClang.
-  static CompilerType
-  CreateTypedefType(const CompilerType &type, const char *typedef_name,
-const CompilerDeclContext &compiler_decl_ctx,
-uint32_t opaque_payload);
-
   CompilerType GetArrayElementType(lldb::opaque_compiler_type_t type,
ExecutionContextScope *exe_scope) override;
 
@@ -720,6 +712,9 @@
 
   CompilerType AddRestrictModifier(lldb::opaque_compiler_type_t type) override;
 
+  /// Using the current type, create a new typedef to that type using
+  /// "typedef_name" as the name and "decl_ctx" as the decl context.
+  /// \param opaque_payload is an opaque TypePayloadClang.
   CompilerType CreateTypedef(lldb::opaque_compiler_type_t type,
  const char *name,
  const CompilerDeclContext &decl_ctx,
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4402,39 +4402,6 @@
   return CompilerType();
 }
 
-CompilerType TypeSystemClang::CreateTypedefType(
-const CompilerType &type, const char *typedef_name,
-const CompilerDeclContext &compiler_decl_ctx, uint32_t payload) {
-  if (type && typedef_name && typedef_name[0]) {
-TypeSystemClang *ast =
-llvm::dyn_cast(type.GetTypeSystem());
-if (!ast)
-  return CompilerType();
-clang::ASTContext &clang_ast = ast->getASTContext();
-clang::QualType qual_type(ClangUtil::GetQualType(type));
-
-clang::DeclContext *decl_ctx =
-TypeSystemClang::DeclContextGetAsDeclContext(compiler_decl_ctx);
-if (!decl_ctx)
-  decl_ctx = ast->getASTContext().getTranslationUnitDecl();
-
-clang::TypedefDecl *decl =
-clang::TypedefDecl::CreateDeserialized(clang_ast, 0);
-decl->setDeclContext(decl_ctx);
-decl->setDeclName(&clang_ast.Idents.get(typedef_name));
-decl->setTypeSourceInfo(clang_ast.getTrivialTypeSourceInfo(qual_type));
-
-SetOwningModule(decl, TypePayloadClang(payload).GetOwningModule());
-decl->setAccess(clang::AS_public); // TODO respect proper access specifier
-
-decl_ctx->addDecl(decl);
-
-// Get a uniqued clang::QualType for the typedef decl type
-return ast->GetType(clang_ast.getTypedefType(decl));
-  }
-  return CompilerType();
-}
-
 CompilerType
 TypeSystemClang::GetPointeeType(lldb::opaque_compiler_type_t type) {
   if (type) {
@@ -4516,7 +4483,7 @@
 CompilerType TypeSystemClang::CreateTypedef(
 lldb::opaque_compiler_type_t type, const char *typedef_name,
 const CompilerDeclContext &compiler_decl_ctx, uint32_t payload) {
-  if (type) {
+  if (type && typedef_name && typedef_name[0]) {
 clang::ASTContext &clang_ast = getASTContext();
 clang::QualType qual_type(GetQualType(type));
 
@@ -4525,10 +4492,11 @@
 if (!decl_ctx)
   decl_ctx = getASTContext().getTranslationUnitDecl();
 
-clang::TypedefDecl *decl = clang::TypedefDecl::Create(
-clang_ast, decl_ctx, clang::SourceLocation(), clang

[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

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

This seems suspicious to me, on a couple of levels. You claim that BOOL is an 
unsigned type, but the apple documentation 
 says "BOOL is 
explicitly signed so @encode(BOOL) is c rather than C even if -funsigned-char 
is used." Which one is true?  Could this have something to do with the fact 
that the documentation assumes obj-c (which does not have a native `bool` type) 
but in lldb, we use obj-c++ (which, I guess, inherits `bool` from c++) 
everywhere?

If BOOL is unsigned, then it's not clear to me why would 
Scalar::ExtractBitfield be sign-extending anything. OTOH, if it is _signed_, 
then sign-extension seems to be the right behavior.

I think we may have some deeper problems with the handling bitfields whose 
underlying types are 1 byte long. Lldb (I think, correctly) sign-extends all 
signed bitfields, except the signed char one:

  (lldb) p a
  (A) $0 = {
bool_ = true
signed_char = '\x01'
signed_short = -1
signed_int = -1
unsigned_char = '\x01'
unsigned_short = 1
unsigned_int = 1
  }
  (lldb) p/x a
  (A) $1 = {
bool_ = 0x01
signed_char = 0x01
signed_short = 0x0001
signed_int = 0x0001
unsigned_char = 0x01
unsigned_short = 0x0001
unsigned_int = 0x0001
  }

This patch does not have any impact on this behavior, but the fact that signed 
char comes out as '\x01' seems suspicious. This is what gdb does for the same 
input:

  (gdb) p a
  $1 = {bool_ = true, signed_char = -1 '\377', signed_short = -1, signed_int = 
-1, unsigned_char = 1 '\001', unsigned_short = 1, unsigned_int = 1}
  (gdb) p/x a
  $2 = {bool_ = 0x1, signed_char = 0xff, signed_short = 0x, signed_int = 
0x, unsigned_char = 0x1, unsigned_short = 0x1, unsigned_int = 0x1}

Finally, even though I'm not sure this is the right solution, I am sure that 
`GetValueAsSigned` should get the same fix as `GetValueAsUnsigned`


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

https://reviews.llvm.org/D93421

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


[Lldb-commits] [PATCH] D93438: ObjectFileELF: Test whether reloc_header is non-null instead of asserting.

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

The fix seems reasonable but it would use a test case. Is it possible to 
generate appropriate input with `yaml2obj`? If so you could run `lldb-test 
symbols` on the resulting object file, and verify that the Symbol table is 
printed out correctly (whatever "correct" means in this case).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93438

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


[Lldb-commits] [PATCH] D93299: [lldb] [unittests] Add tests for NetBSD register offsets/sizes

2020-12-17 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added inline comments.



Comment at: lldb/unittests/Process/Utility/CMakeLists.txt:4-5
   RegisterContextFreeBSDTest.cpp
+  RegisterContextNetBSDTest_i386.cpp
+  RegisterContextNetBSDTest_x86_64.cpp
   LinuxProcMapsTest.cpp

mgorny wrote:
> labath wrote:
> > What's reason for having these as separate files (vs. the all-in-one 
> > freebsd approach)?
> `struct reg` collision. NetBSD doesn't have unique names like FreeBSD does.
I see. Somewhat unfortunate, but such is life... I'd consider splitting the 
FreeBSD version too, for consistency, but I don't feel very strongly about 
that..


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

https://reviews.llvm.org/D93299

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


[Lldb-commits] [PATCH] D93299: [lldb] [unittests] Add tests for NetBSD register offsets/sizes

2020-12-17 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 312418.
mgorny added a comment.

Made sources conditional to platform via CMake. If this is good, I'll update 
FreeBSD afterwards.


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

https://reviews.llvm.org/D93299

Files:
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextNetBSDTest_i386.cpp
  lldb/unittests/Process/Utility/RegisterContextNetBSDTest_x86_64.cpp

Index: lldb/unittests/Process/Utility/RegisterContextNetBSDTest_x86_64.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextNetBSDTest_x86_64.cpp
@@ -0,0 +1,139 @@
+//===-- RegisterContextNetBSDTest_x86_64.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#if defined(__x86_64__)
+
+// clang-format off
+#include 
+#include 
+// clang-format on
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+#include "Plugins/Process/Utility/RegisterContextNetBSD_i386.h"
+#include "Plugins/Process/Utility/RegisterContextNetBSD_x86_64.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static std::pair GetRegParams(RegisterInfoInterface &ctx,
+  uint32_t reg) {
+  const RegisterInfo &info = ctx.GetRegisterInfo()[reg];
+  return {info.byte_offset, info.byte_size};
+}
+
+#define EXPECT_OFF(regname, offset, size)  \
+  EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname),   \
+  ::testing::Pair(offset + base_offset, size))
+
+#define EXPECT_GPR_X86_64(regname, regconst)   \
+  EXPECT_THAT( \
+  GetRegParams(reg_ctx, lldb_##regname##_x86_64),  \
+  ::testing::Pair(offsetof(reg, regs[regconst]),   \
+  sizeof(reg::regs[regconst])))
+#define EXPECT_DBR_X86_64(num) \
+  EXPECT_OFF(dr##num##_x86_64, offsetof(dbreg, dr[num]), sizeof(dbreg::dr[num]))
+
+TEST(RegisterContextNetBSDTest, x86_64) {
+  ArchSpec arch{"x86_64-unknown-netbsd"};
+  RegisterContextNetBSD_x86_64 reg_ctx{arch};
+
+  EXPECT_GPR_X86_64(rdi, _REG_RDI);
+  EXPECT_GPR_X86_64(rsi, _REG_RSI);
+  EXPECT_GPR_X86_64(rdx, _REG_RDX);
+  EXPECT_GPR_X86_64(rcx, _REG_RCX);
+  EXPECT_GPR_X86_64(r8, _REG_R8);
+  EXPECT_GPR_X86_64(r9, _REG_R9);
+  EXPECT_GPR_X86_64(r10, _REG_R10);
+  EXPECT_GPR_X86_64(r11, _REG_R11);
+  EXPECT_GPR_X86_64(r12, _REG_R12);
+  EXPECT_GPR_X86_64(r13, _REG_R13);
+  EXPECT_GPR_X86_64(r14, _REG_R14);
+  EXPECT_GPR_X86_64(r15, _REG_R15);
+  EXPECT_GPR_X86_64(rbp, _REG_RBP);
+  EXPECT_GPR_X86_64(rbx, _REG_RBX);
+  EXPECT_GPR_X86_64(rax, _REG_RAX);
+  EXPECT_GPR_X86_64(gs, _REG_GS);
+  EXPECT_GPR_X86_64(fs, _REG_FS);
+  EXPECT_GPR_X86_64(es, _REG_ES);
+  EXPECT_GPR_X86_64(ds, _REG_DS);
+  EXPECT_GPR_X86_64(rip, _REG_RIP);
+  EXPECT_GPR_X86_64(cs, _REG_CS);
+  EXPECT_GPR_X86_64(rflags, _REG_RFLAGS);
+  EXPECT_GPR_X86_64(rsp, _REG_RSP);
+  EXPECT_GPR_X86_64(ss, _REG_SS);
+
+  // fctrl is the first FPR field, it is used to determine offset of the whole
+  // FPR struct
+  size_t base_offset = reg_ctx.GetRegisterInfo()[lldb_fctrl_x86_64].byte_offset;
+
+  // assert against FXSAVE struct
+  EXPECT_OFF(fctrl_x86_64, 0x00, 2);
+  EXPECT_OFF(fstat_x86_64, 0x02, 2);
+  // TODO: This is a known bug, abridged ftag should is 8 bits in length.
+  EXPECT_OFF(ftag_x86_64, 0x04, 2);
+  EXPECT_OFF(fop_x86_64, 0x06, 2);
+  // NB: Technically fiseg/foseg are 16-bit long and the higher 16 bits
+  // are reserved.  However, LLDB defines them to be 32-bit long for backwards
+  // compatibility, as they were used to reconstruct FIP/FDP before explicit
+  // register entries for them were added.  Also, this is still how GDB does it.
+  EXPECT_OFF(fioff_x86_64, 0x08, 4);
+  EXPECT_OFF(fiseg_x86_64, 0x0C, 4);
+  EXPECT_OFF(fip_x86_64, 0x08, 8);
+  EXPECT_OFF(fooff_x86_64, 0x10, 4);
+  EXPECT_OFF(foseg_x86_64, 0x14, 4);
+  EXPECT_OFF(fdp_x86_64, 0x10, 8);
+  EXPECT_OFF(mxcsr_x86_64, 0x18, 4);
+  EXPECT_OFF(mxcsrmask_x86_64, 0x1C, 4);
+  EXPECT_OFF(st0_x86_64, 0x20, 10);
+  EXPECT_OFF(st1_x86_64, 0x30, 10);
+  EXPECT_OFF(st2_x86_64, 0x40, 10);
+  EXPECT_OFF(st3_x86_64, 0x50, 10);
+  EXPECT_OFF(st4_x86_64, 0x60, 10);
+  EXPECT_OFF(st5_x86_64, 0x70, 10);
+  EXPECT_OFF(st6_x86_64, 0x80, 10);
+  EXPECT_OFF(st7_x86_64, 0x90, 10);
+  EXPECT_OFF(mm0_x86_64, 0x20, 8);
+  EXPECT_OFF(mm1_x86_64, 0x30, 8);
+  EXPECT_OFF(mm2_x86_64, 0x40, 8);
+  EXPECT_OFF(mm3_x86_64, 0x50, 8

[Lldb-commits] [lldb] b833898 - [lldb] Add std::array to the supported template list of the CxxModuleHandler

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

Author: Raphael Isemann
Date: 2020-12-17T11:47:58+01:00
New Revision: b8338983e6f6ec6ebd48a7fc640b5d859e653b27

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

LOG: [lldb] Add std::array to the supported template list of the 
CxxModuleHandler

Identical to the other patches that add STL containers to the supported
templated list.

Added: 
lldb/test/API/commands/expression/import-std-module/array/Makefile

lldb/test/API/commands/expression/import-std-module/array/TestArrayFromStdModule.py
lldb/test/API/commands/expression/import-std-module/array/main.cpp

Modified: 
lldb/source/Plugins/ExpressionParser/Clang/CxxModuleHandler.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/CxxModuleHandler.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/CxxModuleHandler.cpp
index 8a8450245990..f953e860969c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/CxxModuleHandler.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/CxxModuleHandler.cpp
@@ -22,6 +22,7 @@ CxxModuleHandler::CxxModuleHandler(ASTImporter &importer, 
ASTContext *target)
 
   std::initializer_list supported_names = {
   // containers
+  "array",
   "deque",
   "forward_list",
   "list",

diff  --git 
a/lldb/test/API/commands/expression/import-std-module/array/Makefile 
b/lldb/test/API/commands/expression/import-std-module/array/Makefile
new file mode 100644
index ..f938f7428468
--- /dev/null
+++ b/lldb/test/API/commands/expression/import-std-module/array/Makefile
@@ -0,0 +1,3 @@
+USE_LIBCPP := 1
+CXX_SOURCES := main.cpp
+include Makefile.rules

diff  --git 
a/lldb/test/API/commands/expression/import-std-module/array/TestArrayFromStdModule.py
 
b/lldb/test/API/commands/expression/import-std-module/array/TestArrayFromStdModule.py
new file mode 100644
index ..ba9a7853b2f6
--- /dev/null
+++ 
b/lldb/test/API/commands/expression/import-std-module/array/TestArrayFromStdModule.py
@@ -0,0 +1,86 @@
+"""
+Test basic std::array functionality.
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(["libc++"])
+@skipIf(compiler=no_match("clang"))
+def test(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self,
+  "// Set break point at this line.",
+  lldb.SBFileSpec("main.cpp"))
+
+self.runCmd("settings set target.import-std-module true")
+
+
+# Test inspecting an array of integers.
+array_type = "std::array"
+size_type = "std::array::size_type"
+value_type = array_type + "::value_type"
+
+iterator = array_type + "::iterator"
+riterator = array_type + "::reverse_iterator"
+
+self.expect_expr("a",
+ result_type=array_type,
+ result_children=[
+ ValueCheck(name="__elems_", children=[
+ ValueCheck(value="3"),
+ ValueCheck(value="1"),
+ ValueCheck(value="2"),
+ ])
+ ])
+self.expect_expr("a.size()", result_type=size_type, result_value="3")
+self.expect_expr("a.front()", result_type=value_type, result_value="3")
+self.expect_expr("a[1]", result_type=value_type, result_value="1")
+self.expect_expr("a.back()", result_type=value_type, result_value="2")
+
+# Both are just pointers to the underlying elements.
+self.expect_expr("a.begin()", result_type=iterator)
+self.expect_expr("a.rbegin()", result_type=riterator)
+
+self.expect_expr("*a.begin()", result_type=value_type, 
result_value="3")
+self.expect_expr("*a.rbegin()", result_type="int", result_value="2")
+
+self.expect_expr("a.at(0)", result_type=value_type, result_value="3")
+
+
+# Same again with an array that has an element type from debug info.
+array_type = "std::array"
+size_type = "std::array::size_type"
+value_type = array_type + "::value_type"
+
+iterator = array_type + "::iterator"
+riterator = array_type + "::reverse_iterator"
+dbg_info_elem_children = [ValueCheck(value="4")]
+dbg_info_elem = [ValueCheck(children=dbg_info_elem_children)]
+
+self.expect_expr("b",
+ result_type=array_type,
+ result_children=[
+ ValueCheck(name="__elems_", 
children=dbg_info_elem)
+ ])
+self.expec

[Lldb-commits] [PATCH] D93450: [lldb] [Process/FreeBSDRemote] Use RegSetKind consistently [NFC]

2020-12-17 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision.
mgorny added reviewers: emaste, krytarowski, labath.
mgorny requested review of this revision.

Use RegSetKind enum for register sets everything, rather than int.
Always spell it as 'RegSetKind', without unnecessary 'enum'.  Add
missing switch case.  While at it, use uint32_t for regnums
consistently.


https://reviews.llvm.org/D93450

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -74,12 +74,12 @@
   std::array m_xsave_offsets;
   std::array m_regset_offsets;
 
-  llvm::Optional GetSetForNativeRegNum(int reg_num) const;
+  llvm::Optional GetSetForNativeRegNum(uint32_t reg_num) const;
 
-  Status ReadRegisterSet(uint32_t set);
-  Status WriteRegisterSet(uint32_t set);
+  Status ReadRegisterSet(RegSetKind set);
+  Status WriteRegisterSet(RegSetKind set);
 
-  uint8_t *GetOffsetRegSetData(uint32_t set, size_t reg_offset);
+  uint8_t *GetOffsetRegSetData(RegSetKind set, size_t reg_offset);
 
   struct YMMSplitPtr {
 void *xmm;
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -303,8 +303,9 @@
   }
 }
 
-llvm::Optional
-NativeRegisterContextFreeBSD_x86_64::GetSetForNativeRegNum(int reg_num) const {
+llvm::Optional
+NativeRegisterContextFreeBSD_x86_64::GetSetForNativeRegNum(
+uint32_t reg_num) const {
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
 if (reg_num >= k_first_gpr_i386 && reg_num <= k_last_gpr_i386)
@@ -341,7 +342,7 @@
   llvm_unreachable("Register does not belong to any register set");
 }
 
-Status NativeRegisterContextFreeBSD_x86_64::ReadRegisterSet(uint32_t set) {
+Status NativeRegisterContextFreeBSD_x86_64::ReadRegisterSet(RegSetKind set) {
   switch (set) {
   case GPRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
@@ -382,7 +383,7 @@
   llvm_unreachable("NativeRegisterContextFreeBSD_x86_64::ReadRegisterSet");
 }
 
-Status NativeRegisterContextFreeBSD_x86_64::WriteRegisterSet(uint32_t set) {
+Status NativeRegisterContextFreeBSD_x86_64::WriteRegisterSet(RegSetKind set) {
   switch (set) {
   case GPRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_SETREGS, m_thread.GetID(),
@@ -428,7 +429,7 @@
 return error;
   }
 
-  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
+  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
   if (!opt_set) {
 // This is likely an internal register for lldb use only and should not be
 // directly queried.
@@ -437,7 +438,7 @@
 return error;
   }
 
-  enum RegSetKind set = opt_set.getValue();
+  RegSetKind set = opt_set.getValue();
   error = ReadRegisterSet(set);
   if (error.Fail())
 return error;
@@ -494,7 +495,7 @@
 return error;
   }
 
-  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
+  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
   if (!opt_set) {
 // This is likely an internal register for lldb use only and should not be
 // directly queried.
@@ -503,7 +504,7 @@
 return error;
   }
 
-  enum RegSetKind set = opt_set.getValue();
+  RegSetKind set = opt_set.getValue();
   error = ReadRegisterSet(set);
   if (error.Fail())
 return error;
@@ -610,7 +611,7 @@
 }
 
 uint8_t *
-NativeRegisterContextFreeBSD_x86_64::GetOffsetRegSetData(uint32_t set,
+NativeRegisterContextFreeBSD_x86_64::GetOffsetRegSetData(RegSetKind set,
  size_t reg_offset) {
   uint8_t *base;
   switch (set) {
@@ -625,6 +626,8 @@
 break;
   case YMMRegSet:
 llvm_unreachable("GetRegSetData() is unsuitable for this regset.");
+  case MPXRegSet:
+llvm_unreachable("MPX regset should have returned error");
   }
   assert(reg_offset >= m_regset_offsets[set]);
   return base + (reg_offset - m_regset_offsets[set]);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

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

Whether BOOL is `signed char` or `bool` is apparently depending on the 
architecture (???). See `ClangExpressionSourceCode::GetText`, Clang's 
`__OBJC_BOOL_IS_BOOL` macro and then there is also some preprocessor 
shenanigans for non-Clang ObjC (?) in the `objc.h` header from what I can see. 
Whether C++ is active is never actually checked anywhere (the `bool` type is 
coming from `stdbool.h` which is unconditionally includes by `objc.h`).

I think this patch just gets the formatter printing 'YES' because the formatter 
only knows `0` and `1` as valid values, but when we (correctly) sign-extend the 
1-bit `BOOL` (=`signed char`) to 255, it just does it's fall-back logic of 
printing the integer value (also, that fall-back logic is completely ignoring 
the specified int format and just always prints as decimal. That seems like a 
bug...)

Anyway, the real solution is that the current behaviour is actually ... *drum 
roll* correct:

  $ cat bool.m
  #import 
  
  typedef struct {
  BOOL fieldOne : 1;
  BOOL fieldTwo : 1;
  BOOL fieldThree : 1;
  BOOL fieldFour : 1;
  BOOL fieldfive : 1;
  } BoolBitFields;
  
  int main (int argc, const char * argv[]) {
  
BoolBitFields myField = {0};
myField.fieldTwo = YES;
if (myField.fieldTwo == YES)
  printf("YES\n");
else
  printf("NO\n");
  
return 0;
  }
  $ ~/test clang bool.m -o bool -framework Foundation -Wpedantic ; and ./bool
  NO

That bitfield is just not 'YES' but '255' which is neither `YES` nor `NO`. Also 
there is no Clang warning for this, so this is apparently very cool and very 
legal code. I am tempted to say the real fix here is to tell people to just not 
use Objective-C...


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

https://reviews.llvm.org/D93421

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


[Lldb-commits] [PATCH] D93299: [lldb] [unittests] Add tests for NetBSD register offsets/sizes

2020-12-17 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.

Looks good.


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

https://reviews.llvm.org/D93299

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


[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

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

In D93421#2459853 , @labath wrote:

> This seems suspicious to me, on a couple of levels. You claim that BOOL is an 
> unsigned type, but the apple documentation 
>  says "BOOL is 
> explicitly signed so @encode(BOOL) is c rather than C even if -funsigned-char 
> is used." Which one is true?  Could this have something to do with the fact 
> that the documentation assumes obj-c (which does not have a native `bool` 
> type) but in lldb, we use obj-c++ (which, I guess, inherits `bool` from c++) 
> everywhere?
>
> If BOOL is unsigned, then it's not clear to me why would 
> Scalar::ExtractBitfield be sign-extending anything. OTOH, if it is _signed_, 
> then sign-extension seems to be the right behavior.
>
> I think we may have some deeper problems with the handling bitfields whose 
> underlying types are 1 byte long. Lldb (I think, correctly) sign-extends all 
> signed bitfields, except the signed char one:
>
>   (lldb) p a
>   (A) $0 = {
> bool_ = true
> signed_char = '\x01'
> signed_short = -1
> signed_int = -1
> unsigned_char = '\x01'
> unsigned_short = 1
> unsigned_int = 1
>   }
>   (lldb) p/x a
>   (A) $1 = {
> bool_ = 0x01
> signed_char = 0x01
> signed_short = 0x0001
> signed_int = 0x0001
> unsigned_char = 0x01
> unsigned_short = 0x0001
> unsigned_int = 0x0001
>   }
>
> This patch does not have any impact on this behavior, but the fact that 
> signed char comes out as '\x01' seems suspicious. This is what gdb does for 
> the same input:
>
>   (gdb) p a
>   $1 = {bool_ = true, signed_char = -1 '\377', signed_short = -1, signed_int 
> = -1, unsigned_char = 1 '\001', unsigned_short = 1, unsigned_int = 1}
>   (gdb) p/x a
>   $2 = {bool_ = 0x1, signed_char = 0xff, signed_short = 0x, signed_int = 
> 0x, unsigned_char = 0x1, unsigned_short = 0x1, unsigned_int = 0x1}
>
> Finally, even though I'm not sure this is the right solution, I am sure that 
> `GetValueAsSigned` should get the same fix as `GetValueAsUnsigned`

DumpDataExtractor doesn't know what types its printing, it just gets a 
`lldb_private::Format` (where we sometimes encode whether a type is signed). 
That is on my TODO list, but I just didn't get around to refactor this yet.

In short: If you get a `eFormatChar` (which is the default for 'char' stuff) 
then we always pretend its unsigned when printing the character. If you do 
'hex', all ints become unsigned. If you print 'decimal', all types because 
signed, so there is an 'unsigned decimal' format that works around this:

  (lldb) frame var --format default f
  (F) f = (sc1 = '\x01', uc1 = '\x01', sc2 = '\x01', uc2 = '\x01')
  (lldb) frame var --format 'decimal' f
  (F) f = (sc1 = -1, uc1 = -1, sc2 = 1, uc2 = 1)
  (lldb) frame var --format 'unsigned decimal' f
  (F) f = (sc1 = 1, uc1 = 1, sc2 = 1, uc2 = 1)
  (lldb) frame var --format 'hex' f
  (F) f = (sc1 = 0x01, uc1 = 0x01, sc2 = 0x01, uc2 = 0x01)


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

https://reviews.llvm.org/D93421

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


[Lldb-commits] [lldb] 5644035 - [lldb] [unittests] Add tests for NetBSD register offsets/sizes

2020-12-17 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-12-17T13:55:42+01:00
New Revision: 56440359d093ea6f8e9c91064fdd47928cf07092

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

LOG: [lldb] [unittests] Add tests for NetBSD register offsets/sizes

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

Added: 
lldb/unittests/Process/Utility/RegisterContextNetBSDTest_i386.cpp
lldb/unittests/Process/Utility/RegisterContextNetBSDTest_x86_64.cpp

Modified: 
lldb/unittests/Process/Utility/CMakeLists.txt

Removed: 




diff  --git a/lldb/unittests/Process/Utility/CMakeLists.txt 
b/lldb/unittests/Process/Utility/CMakeLists.txt
index 78e29ed262c9..8e4696b7211d 100644
--- a/lldb/unittests/Process/Utility/CMakeLists.txt
+++ b/lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,7 +1,19 @@
+set(NETBSD_SOURCES
+  RegisterContextNetBSDTest_i386.cpp
+  RegisterContextNetBSDTest_x86_64.cpp)
+
+if (CMAKE_SYSTEM_NAME MATCHES "NetBSD")
+  list(APPEND PLATFORM_SOURCES ${NETBSD_SOURCES})
+endif()
+
+set(LLVM_OPTIONAL_SOURCES
+  ${NETBSD_SOURCES})
+
 add_lldb_unittest(ProcessUtilityTests
   RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
   LinuxProcMapsTest.cpp
+  ${PLATFORM_SOURCES}
 
   LINK_LIBS
 lldbPluginProcessUtility)

diff  --git a/lldb/unittests/Process/Utility/RegisterContextNetBSDTest_i386.cpp 
b/lldb/unittests/Process/Utility/RegisterContextNetBSDTest_i386.cpp
new file mode 100644
index ..07e09d34d191
--- /dev/null
+++ b/lldb/unittests/Process/Utility/RegisterContextNetBSDTest_i386.cpp
@@ -0,0 +1,118 @@
+//===-- RegisterContextNetBSDTest_i386.cpp 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#if defined(__i386__) || defined(__x86_64__)
+
+// clang-format off
+#include 
+#include 
+// clang-format on
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+#include "Plugins/Process/Utility/RegisterContextNetBSD_i386.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static std::pair GetRegParams(RegisterInfoInterface &ctx,
+  uint32_t reg) {
+  const RegisterInfo &info = ctx.GetRegisterInfo()[reg];
+  return {info.byte_offset, info.byte_size};
+}
+
+#define EXPECT_OFF(regname, offset, size)  
\
+  EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname),   
\
+  ::testing::Pair(offset + base_offset, size))
+
+#define EXPECT_GPR_I386(regname)   
\
+  EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname##_i386),
\
+  ::testing::Pair(offsetof(reg, r_##regname), \
+  sizeof(reg::r_##regname)))
+#define EXPECT_DBR_I386(num)   
\
+  EXPECT_OFF(dr##num##_i386, offsetof(dbreg, dr[num]),\
+ sizeof(dbreg::dr[num]))
+
+TEST(RegisterContextNetBSDTest, i386) {
+  ArchSpec arch{"i686-unknown-netbsd"};
+  RegisterContextNetBSD_i386 reg_ctx{arch};
+
+  EXPECT_GPR_I386(eax);
+  EXPECT_GPR_I386(ecx);
+  EXPECT_GPR_I386(edx);
+  EXPECT_GPR_I386(ebx);
+  EXPECT_GPR_I386(esp);
+  EXPECT_GPR_I386(ebp);
+  EXPECT_GPR_I386(esi);
+  EXPECT_GPR_I386(edi);
+  EXPECT_GPR_I386(eip);
+  EXPECT_GPR_I386(eflags);
+  EXPECT_GPR_I386(cs);
+  EXPECT_GPR_I386(ss);
+  EXPECT_GPR_I386(ds);
+  EXPECT_GPR_I386(es);
+  EXPECT_GPR_I386(fs);
+  EXPECT_GPR_I386(gs);
+
+  // fctrl is the first FPR field, it is used to determine offset of the whole
+  // FPR struct
+  size_t base_offset = reg_ctx.GetRegisterInfo()[lldb_fctrl_i386].byte_offset;
+
+  // assert against FXSAVE struct
+  EXPECT_OFF(fctrl_i386, 0x00, 2);
+  EXPECT_OFF(fstat_i386, 0x02, 2);
+  // TODO: This is a known bug, abridged ftag should is 8 bits in length.
+  EXPECT_OFF(ftag_i386, 0x04, 2);
+  EXPECT_OFF(fop_i386, 0x06, 2);
+  // NB: Technically fiseg/foseg are 16-bit long and the higher 16 bits
+  // are reserved.  However, we use them to access/recombine 64-bit FIP/FDP.
+  EXPECT_OFF(fioff_i386, 0x08, 4);
+  EXPECT_OFF(fiseg_i386, 0x0C, 4);
+  EXPECT_OFF(fooff_i386, 0x10, 4);
+  EXPECT_OFF(foseg_i386, 0x14, 4);
+  EXPECT_OFF(mxcsr_i386, 0x18, 4);
+  EXPECT_OFF(mxcsrmask_i386, 0x1C, 4);
+  EXPECT_OFF(st0_i386, 0x20, 10);
+  EXPECT_OFF(st1_i386, 0x30, 10);
+  EXPECT_OFF(st2_i386, 0x40, 10);
+  EXPECT_OFF(st3_i386, 0x50, 10);
+  EXPECT_OFF(st4_i386, 0x60, 10);
+  EXPECT_OFF(st5_i386, 0x70, 10);
+  EXPECT_OFF(st6_i386

[Lldb-commits] [lldb] 37f99a5 - [lldb] [unittests] Filter FreeBSD through CMake rather than #ifdef

2020-12-17 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-12-17T13:55:42+01:00
New Revision: 37f99a56065209627020428cecb78f55dfa90580

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

LOG: [lldb] [unittests] Filter FreeBSD through CMake rather than #ifdef

Added: 


Modified: 
lldb/unittests/Process/Utility/CMakeLists.txt
lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp

Removed: 




diff  --git a/lldb/unittests/Process/Utility/CMakeLists.txt 
b/lldb/unittests/Process/Utility/CMakeLists.txt
index 8e4696b7211d..772e781b5cc5 100644
--- a/lldb/unittests/Process/Utility/CMakeLists.txt
+++ b/lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,17 +1,21 @@
+set(FREEBSD_SOURCES
+  RegisterContextFreeBSDTest.cpp)
 set(NETBSD_SOURCES
   RegisterContextNetBSDTest_i386.cpp
   RegisterContextNetBSDTest_x86_64.cpp)
 
-if (CMAKE_SYSTEM_NAME MATCHES "NetBSD")
+if (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
+  list(APPEND PLATFORM_SOURCES ${FREEBSD_SOURCES})
+elseif (CMAKE_SYSTEM_NAME MATCHES "NetBSD")
   list(APPEND PLATFORM_SOURCES ${NETBSD_SOURCES})
 endif()
 
 set(LLVM_OPTIONAL_SOURCES
+  ${FREEBSD_SOURCES}
   ${NETBSD_SOURCES})
 
 add_lldb_unittest(ProcessUtilityTests
   RegisterContextTest.cpp
-  RegisterContextFreeBSDTest.cpp
   LinuxProcMapsTest.cpp
   ${PLATFORM_SOURCES}
 

diff  --git a/lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp 
b/lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
index 7d875c9bd8a1..fe516d537662 100644
--- a/lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ b/lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -6,8 +6,6 @@
 //
 
//===--===//
 
-#if defined(__FreeBSD__)
-
 // clang-format off
 #include 
 #include 
@@ -233,5 +231,3 @@ TEST(RegisterContextFreeBSDTest, i386) {
 }
 
 #endif // defined(__i386__) || defined(__x86_64__)
-
-#endif // defined(__FreeBSD__)



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


[Lldb-commits] [PATCH] D93299: [lldb] [unittests] Add tests for NetBSD register offsets/sizes

2020-12-17 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG56440359d093: [lldb] [unittests] Add tests for NetBSD 
register offsets/sizes (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93299

Files:
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextNetBSDTest_i386.cpp
  lldb/unittests/Process/Utility/RegisterContextNetBSDTest_x86_64.cpp

Index: lldb/unittests/Process/Utility/RegisterContextNetBSDTest_x86_64.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextNetBSDTest_x86_64.cpp
@@ -0,0 +1,139 @@
+//===-- RegisterContextNetBSDTest_x86_64.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#if defined(__x86_64__)
+
+// clang-format off
+#include 
+#include 
+// clang-format on
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+#include "Plugins/Process/Utility/RegisterContextNetBSD_i386.h"
+#include "Plugins/Process/Utility/RegisterContextNetBSD_x86_64.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static std::pair GetRegParams(RegisterInfoInterface &ctx,
+  uint32_t reg) {
+  const RegisterInfo &info = ctx.GetRegisterInfo()[reg];
+  return {info.byte_offset, info.byte_size};
+}
+
+#define EXPECT_OFF(regname, offset, size)  \
+  EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname),   \
+  ::testing::Pair(offset + base_offset, size))
+
+#define EXPECT_GPR_X86_64(regname, regconst)   \
+  EXPECT_THAT( \
+  GetRegParams(reg_ctx, lldb_##regname##_x86_64),  \
+  ::testing::Pair(offsetof(reg, regs[regconst]),   \
+  sizeof(reg::regs[regconst])))
+#define EXPECT_DBR_X86_64(num) \
+  EXPECT_OFF(dr##num##_x86_64, offsetof(dbreg, dr[num]), sizeof(dbreg::dr[num]))
+
+TEST(RegisterContextNetBSDTest, x86_64) {
+  ArchSpec arch{"x86_64-unknown-netbsd"};
+  RegisterContextNetBSD_x86_64 reg_ctx{arch};
+
+  EXPECT_GPR_X86_64(rdi, _REG_RDI);
+  EXPECT_GPR_X86_64(rsi, _REG_RSI);
+  EXPECT_GPR_X86_64(rdx, _REG_RDX);
+  EXPECT_GPR_X86_64(rcx, _REG_RCX);
+  EXPECT_GPR_X86_64(r8, _REG_R8);
+  EXPECT_GPR_X86_64(r9, _REG_R9);
+  EXPECT_GPR_X86_64(r10, _REG_R10);
+  EXPECT_GPR_X86_64(r11, _REG_R11);
+  EXPECT_GPR_X86_64(r12, _REG_R12);
+  EXPECT_GPR_X86_64(r13, _REG_R13);
+  EXPECT_GPR_X86_64(r14, _REG_R14);
+  EXPECT_GPR_X86_64(r15, _REG_R15);
+  EXPECT_GPR_X86_64(rbp, _REG_RBP);
+  EXPECT_GPR_X86_64(rbx, _REG_RBX);
+  EXPECT_GPR_X86_64(rax, _REG_RAX);
+  EXPECT_GPR_X86_64(gs, _REG_GS);
+  EXPECT_GPR_X86_64(fs, _REG_FS);
+  EXPECT_GPR_X86_64(es, _REG_ES);
+  EXPECT_GPR_X86_64(ds, _REG_DS);
+  EXPECT_GPR_X86_64(rip, _REG_RIP);
+  EXPECT_GPR_X86_64(cs, _REG_CS);
+  EXPECT_GPR_X86_64(rflags, _REG_RFLAGS);
+  EXPECT_GPR_X86_64(rsp, _REG_RSP);
+  EXPECT_GPR_X86_64(ss, _REG_SS);
+
+  // fctrl is the first FPR field, it is used to determine offset of the whole
+  // FPR struct
+  size_t base_offset = reg_ctx.GetRegisterInfo()[lldb_fctrl_x86_64].byte_offset;
+
+  // assert against FXSAVE struct
+  EXPECT_OFF(fctrl_x86_64, 0x00, 2);
+  EXPECT_OFF(fstat_x86_64, 0x02, 2);
+  // TODO: This is a known bug, abridged ftag should is 8 bits in length.
+  EXPECT_OFF(ftag_x86_64, 0x04, 2);
+  EXPECT_OFF(fop_x86_64, 0x06, 2);
+  // NB: Technically fiseg/foseg are 16-bit long and the higher 16 bits
+  // are reserved.  However, LLDB defines them to be 32-bit long for backwards
+  // compatibility, as they were used to reconstruct FIP/FDP before explicit
+  // register entries for them were added.  Also, this is still how GDB does it.
+  EXPECT_OFF(fioff_x86_64, 0x08, 4);
+  EXPECT_OFF(fiseg_x86_64, 0x0C, 4);
+  EXPECT_OFF(fip_x86_64, 0x08, 8);
+  EXPECT_OFF(fooff_x86_64, 0x10, 4);
+  EXPECT_OFF(foseg_x86_64, 0x14, 4);
+  EXPECT_OFF(fdp_x86_64, 0x10, 8);
+  EXPECT_OFF(mxcsr_x86_64, 0x18, 4);
+  EXPECT_OFF(mxcsrmask_x86_64, 0x1C, 4);
+  EXPECT_OFF(st0_x86_64, 0x20, 10);
+  EXPECT_OFF(st1_x86_64, 0x30, 10);
+  EXPECT_OFF(st2_x86_64, 0x40, 10);
+  EXPECT_OFF(st3_x86_64, 0x50, 10);
+  EXPECT_OFF(st4_x86_64, 0x60, 10);
+  EXPECT_OFF(st5_x86_64, 0x70, 10);
+  EXPECT_OFF(st6_x86_64, 0x80, 10);
+  EXPECT_OFF(st7_x86_64, 0x90, 10);
+  EXPECT_OFF(mm0_x86_64, 0x20, 8);
+  EXPECT_OF

[Lldb-commits] [lldb] e7a3c4c - [lldb-vscode] Speculative fix for raciness in TestVSCode_attach

2020-12-17 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-12-17T14:19:52+01:00
New Revision: e7a3c4c11e84ba99c3682ae6cf20c398f16cf3f5

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

LOG: [lldb-vscode] Speculative fix for raciness in TestVSCode_attach

The test appears to expect the inferior to be stopped, but the custom
"attach commands" leave it in a running state.

It's unclear how this could have ever worked.

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py 
b/lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py
index 7955b6a97b04..aa7a3ae17cb0 100644
--- a/lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py
+++ b/lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py
@@ -145,7 +145,7 @@ def test_commands(self):
 # and use it for debugging
 attachCommands = [
 'target create -d "%s"' % (program),
-'process launch'
+'process launch --stop-at-entry'
 ]
 initCommands = ['target list', 'platform list']
 preRunCommands = ['image list a.out', 'image dump sections a.out']



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


[Lldb-commits] [lldb] 5e31e22 - Remove Python2 fallback and only advertise Python3 in the doc

2020-12-17 Thread via lldb-commits

Author: serge-sans-paille
Date: 2020-12-17T15:40:16+01:00
New Revision: 5e31e226b5b2b682607a6578ff5adb33daf4fe39

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

LOG: Remove Python2 fallback and only advertise Python3 in the doc

Differential Revision: https://www.youtube.com/watch?v=RsL0cipURA0

Added: 


Modified: 
clang/CMakeLists.txt
clang/tools/scan-build-py/README.md
clang/tools/scan-build/bin/set-xcode-analyzer
lld/CMakeLists.txt
lldb/docs/resources/build.rst
llvm/CMakeLists.txt
llvm/docs/GettingStarted.rst
llvm/docs/GettingStartedVS.rst
llvm/docs/HowToBuildOnARM.rst
llvm/docs/TestingGuide.rst

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index f947b820bdac..f1e5a39cfe05 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -135,20 +135,7 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR )
   set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
 
   if(LLVM_INCLUDE_TESTS)
-find_package(Python3 COMPONENTS Interpreter)
-if(NOT Python3_Interpreter_FOUND)
-  message(WARNING "Python3 not found, using python2 as a fallback")
-  find_package(Python2 COMPONENTS Interpreter REQUIRED)
-  if(Python2_VERSION VERSION_LESS 2.7)
-message(SEND_ERROR "Python 2.7 or newer is required")
-  endif()
-
-  # Treat python2 as python3
-  add_executable(Python3::Interpreter IMPORTED)
-  set_target_properties(Python3::Interpreter PROPERTIES
-IMPORTED_LOCATION ${Python2_EXECUTABLE})
-  set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
-endif()
+find_package(Python3 REQUIRED COMPONENTS Interpreter)
 
 # Check prebuilt llvm/utils.
 if(EXISTS ${LLVM_TOOLS_BINARY_DIR}/FileCheck${CMAKE_EXECUTABLE_SUFFIX}

diff  --git a/clang/tools/scan-build-py/README.md 
b/clang/tools/scan-build-py/README.md
index 0f89b6fa43d8..63ce0273f22e 100644
--- a/clang/tools/scan-build-py/README.md
+++ b/clang/tools/scan-build-py/README.md
@@ -19,7 +19,7 @@ Should be working on UNIX operating systems.
 Prerequisites
 -
 
-1. **python** interpreter (version 2.7, 3.2, 3.3, 3.4, 3.5).
+1. **python** interpreter (version 3.6 or later).
 
 
 How to use

diff  --git a/clang/tools/scan-build/bin/set-xcode-analyzer 
b/clang/tools/scan-build/bin/set-xcode-analyzer
index c2a65c908598..9faaec1e8e6e 100755
--- a/clang/tools/scan-build/bin/set-xcode-analyzer
+++ b/clang/tools/scan-build/bin/set-xcode-analyzer
@@ -5,8 +5,8 @@
 # This one has the scripting bridge enabled.
 
 import sys
-if sys.version_info < (2, 7):
-print "set-xcode-analyzer requires Python 2.7 or later"
+if sys.version_info < (3, 6):
+print "set-xcode-analyzer requires Python 3.6 or later"
 sys.exit(1)
 
 import os

diff  --git a/lld/CMakeLists.txt b/lld/CMakeLists.txt
index 82b4b9b9b198..d4e561b50d8f 100644
--- a/lld/CMakeLists.txt
+++ b/lld/CMakeLists.txt
@@ -57,20 +57,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   include(CheckAtomic)
 
   if(LLVM_INCLUDE_TESTS)
-find_package(Python3 COMPONENTS Interpreter)
-if(NOT Python3_Interpreter_FOUND)
-  message(WARNING "Python3 not found, using python2 as a fallback")
-  find_package(Python2 COMPONENTS Interpreter REQUIRED)
-  if(Python2_VERSION VERSION_LESS 2.7)
-message(SEND_ERROR "Python 2.7 or newer is required")
-  endif()
-
-  # Treat python2 as python3
-  add_executable(Python3::Interpreter IMPORTED)
-  set_target_properties(Python3::Interpreter PROPERTIES
-IMPORTED_LOCATION ${Python2_EXECUTABLE})
-  set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
-endif()
+find_package(Python3 REQUIRED COMPONENTS Interpreter)
 
 # Check prebuilt llvm/utils.
 if(EXISTS ${LLVM_TOOLS_BINARY_DIR}/FileCheck${CMAKE_EXECUTABLE_SUFFIX}

diff  --git a/lldb/docs/resources/build.rst b/lldb/docs/resources/build.rst
index b4e58ca977a9..8aadd126ed0b 100644
--- a/lldb/docs/resources/build.rst
+++ b/lldb/docs/resources/build.rst
@@ -73,7 +73,7 @@ commands below.
   > yum install libedit-devel libxml2-devel ncurses-devel python-devel swig
   > sudo apt-get install build-essential subversion swig python3-dev 
libedit-dev libncurses5-dev
   > pkg install swig python
-  > pkgin install swig python27 cmake ninja-build
+  > pkgin install swig python36 cmake ninja-build
   > brew install swig cmake ninja
 
 Note that there's an `incompatibility

diff  --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 54009573ed43..ee1b646ab651 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -708,20 +708,7 @@ set(ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER FALSE CACHE BOOL
 
 include(HandleLLVMOptions)
 
-find_package(Python3 COMPONENTS Interpreter)
-if(NOT Python3_Interpreter_FOUN

[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

2020-12-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 think the proper fix here is to warn about BOOL bitfields with size 1 if BOOL 
is a typedef for signed char. I can make a Clang warning for that. I guess on 
the LLDB side we should make it clear that BOOL here has an invalid value (and 
then print the int value as we currently do)?


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

https://reviews.llvm.org/D93421

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


[Lldb-commits] [lldb] 122a4eb - Revert "[lldb] Make CommandInterpreter's execution context the same as debugger's one."

2020-12-17 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-12-17T17:47:53+01:00
New Revision: 122a4ebde3f4394a84e9f93b9c7085f088be6dd7

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

LOG: Revert "[lldb] Make CommandInterpreter's execution context the same as 
debugger's one."

This reverts commit a01b26fb51c710a3a8ef88cc83b0701461f5b9ab, because it
breaks the "finish" command in some way -- the command does not
terminate after it steps out, but continues running the target. The
exact blast radius is not clear, but it at least affects the usage of
the "finish" command in TestGuiBasicDebug.py. The error is *not*
gui-related, as the same issue can be reproduced by running the same
steps outside of the gui.

There is some kind of a race going on, as the test fails only 20% of the
time on the buildbot.

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/source/API/SBCommandInterpreter.cpp
lldb/source/Breakpoint/BreakpointOptions.cpp
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/CommandObjectRegexCommand.cpp
lldb/source/Commands/CommandObjectSettings.cpp
lldb/source/Commands/CommandObjectWatchpointCommand.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Target/Target.cpp
lldb/test/API/python_api/debugger/TestDebuggerAPI.py

Removed: 
lldb/test/API/python_api/debugger/Makefile
lldb/test/API/python_api/debugger/main.cpp



diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 40b649411f7f..d35f7e22b9ea 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -24,9 +24,7 @@
 #include "lldb/Utility/StringList.h"
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-private.h"
-
 #include 
-#include 
 
 namespace lldb_private {
 class CommandInterpreter;
@@ -247,7 +245,7 @@ class CommandInterpreter : public Broadcaster,
 
   CommandInterpreter(Debugger &debugger, bool synchronous_execution);
 
-  ~CommandInterpreter() override = default;
+  ~CommandInterpreter() override;
 
   // These two functions fill out the Broadcaster interface:
 
@@ -302,11 +300,10 @@ class CommandInterpreter : public Broadcaster,
   CommandReturnObject &result);
 
   bool HandleCommand(const char *command_line, LazyBool add_to_history,
- const ExecutionContext &override_context,
- CommandReturnObject &result);
-
-  bool HandleCommand(const char *command_line, LazyBool add_to_history,
- CommandReturnObject &result);
+ CommandReturnObject &result,
+ ExecutionContext *override_context = nullptr,
+ bool repeat_on_empty_command = true,
+ bool no_context_switching = false);
 
   bool WasInterrupted() const;
 
@@ -315,7 +312,9 @@ class CommandInterpreter : public Broadcaster,
   /// \param[in] commands
   ///The list of commands to execute.
   /// \param[in,out] context
-  ///The execution context in which to run the commands.
+  ///The execution context in which to run the commands. Can be nullptr in
+  ///which case the default
+  ///context will be used.
   /// \param[in] options
   ///This object holds the options used to control when to stop, whether to
   ///execute commands,
@@ -325,13 +324,8 @@ class CommandInterpreter : public Broadcaster,
   ///safely,
   ///and failed with some explanation if we aborted executing the commands
   ///at some point.
-  void HandleCommands(const StringList &commands,
-  const ExecutionContext &context,
-  const CommandInterpreterRunOptions &options,
-  CommandReturnObject &result);
-
-  void HandleCommands(const StringList &commands,
-  const CommandInterpreterRunOptions &options,
+  void HandleCommands(const StringList &commands, ExecutionContext *context,
+  CommandInterpreterRunOptions &options,
   CommandReturnObject &result);
 
   /// Execute a list of commands from a file.
@@ -339,7 +333,9 @@ class CommandInterpreter : public Broadcaster,
   /// \param[in] file
   ///The file from which to read in commands.
   /// \param[in,out] context
-  ///The execution context in which to run the commands.
+  ///The execution context in which to run the commands. Can be nullptr in
+  ///which case the default
+  ///context will be used.
   /// \param[in] options
   ///This object hold

[Lldb-commits] [PATCH] D92164: Make CommandInterpreter's execution context the same as debugger's one.

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

Sorry for not keeping track of this patch -- I'm not very active these days.

Nevertheless, I did notice that this patch introduced some kind of 
nondeterminism/raciness/flakyness to the "finish" command. Specifically, it 
causes the command (as used in TestGuiBasicDebug.py) to fail occasionally (~20% 
of the time). Since this patch landed (in build 3660 
, there has been a constant 
stream of builds (build 3667 
, build 3674 
, build 3678 
, ...) where this test is 
failing. The test has been stable up to that point.

The breakage can be also by running the test commands without the gui mode:

  $ bin/lldb 
lldb-test-build.noindex/commands/gui/basicdebug/TestGuiBasicDebug.test_gui/a.out
 -o 'br set -f main.c -p "// Break here"' -o r
  (lldb) target create 
"lldb-test-build.noindex/commands/gui/basicdebug/TestGuiBasicDebug.test_gui/a.out"
  Current executable set to 
'/home/pavelo/ll/build/opt/lldb-test-build.noindex/commands/gui/basicdebug/TestGuiBasicDebug.test_gui/a.out'
 (x86_64).
  (lldb) br set -f main.c -p "// Break here"
  Breakpoint 1: where = a.out`main + 22 at main.c:4:3, address = 
0x00401116
  (lldb) r
  Process 32346 stopped
  * thread #1, name = 'a.out', stop reason = breakpoint 1.1
  frame #0: 0x00401116 a.out`main(argc=1, argv=0x7fffdb58) 
at main.c:4:3
 1  extern int func();
 2  
 3  int main(int argc, char **argv) {
  -> 4func(); // Break here
 5func(); // Second
 6return 0;
 7  }
  
  Process 32346 launched: 
'/home/pavelo/ll/build/opt/lldb-test-build.noindex/commands/gui/basicdebug/TestGuiBasicDebug.test_gui/a.out'
 (x86_64)
  (lldb) s
  Process 32346 stopped
  * thread #1, name = 'a.out', stop reason = step in
  frame #0: 0x00401134 a.out`func at func.c:2:3
 1  int func() {
  -> 2return 1; // In function
 3  }
  (lldb) fin
  Process 32346 exited with status = 0 (0x) 

I've reverted this patch to get the build green. I think it should be fairly 
easy to reproduce this problem (the command line approach fails 100% 
consistently for me), but if you're having trouble reproducing it, let me know, 
and I can send some logs or something. (The logs seem to indicate that the 
"step out" thread plan does not dequeue itself after steping out, but I guess 
the root cause is somewhere else.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92164

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


[Lldb-commits] [lldb] 9ead4e7 - [lldb] [Process/FreeBSDRemote] Replace GetRegisterSetCount()

2020-12-17 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-12-17T18:01:14+01:00
New Revision: 9ead4e7b4a68d162122d861f5d5b6a3baf8d23c1

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

LOG: [lldb] [Process/FreeBSDRemote] Replace GetRegisterSetCount()

Replace the wrong code in GetRegisterSetCount() with a constant return.
The original code passed register index in place of register set index,
effectively getting always true.  Correcting the code to check for
register set existence is not possible as LLDB supports only eliminating
last register sets.  Just return the full number for now which should
be NFC.

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

Added: 


Modified: 

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
index 8f1ba2eb4137..740ac522d303 100644
--- 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -288,13 +288,7 @@ 
NativeRegisterContextFreeBSD_x86_64::NativeRegisterContextFreeBSD_x86_64(
 }
 
 uint32_t NativeRegisterContextFreeBSD_x86_64::GetRegisterSetCount() const {
-  uint32_t sets = 0;
-  for (uint32_t set_index = 0; set_index < k_num_register_sets; ++set_index) {
-if (GetSetForNativeRegNum(set_index))
-  ++sets;
-  }
-
-  return sets;
+  return k_num_register_sets;
 }
 
 const RegisterSet *



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


[Lldb-commits] [lldb] 835f8de - [lldb] [Process/FreeBSDRemote] Use RegSetKind consistently [NFC]

2020-12-17 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-12-17T18:01:46+01:00
New Revision: 835f8de8508953f4624534e36d54cd256e8800c9

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

LOG: [lldb] [Process/FreeBSDRemote] Use RegSetKind consistently [NFC]

Use RegSetKind enum for register sets everything, rather than int.
Always spell it as 'RegSetKind', without unnecessary 'enum'.  Add
missing switch case.  While at it, use uint32_t for regnums
consistently.

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

Added: 


Modified: 

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
index 740ac522d303..b3b4a6cb0578 100644
--- 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -303,8 +303,9 @@ 
NativeRegisterContextFreeBSD_x86_64::GetRegisterSet(uint32_t set_index) const {
   }
 }
 
-llvm::Optional
-NativeRegisterContextFreeBSD_x86_64::GetSetForNativeRegNum(int reg_num) const {
+llvm::Optional
+NativeRegisterContextFreeBSD_x86_64::GetSetForNativeRegNum(
+uint32_t reg_num) const {
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
 if (reg_num >= k_first_gpr_i386 && reg_num <= k_last_gpr_i386)
@@ -341,7 +342,7 @@ 
NativeRegisterContextFreeBSD_x86_64::GetSetForNativeRegNum(int reg_num) const {
   llvm_unreachable("Register does not belong to any register set");
 }
 
-Status NativeRegisterContextFreeBSD_x86_64::ReadRegisterSet(uint32_t set) {
+Status NativeRegisterContextFreeBSD_x86_64::ReadRegisterSet(RegSetKind set) {
   switch (set) {
   case GPRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
@@ -382,7 +383,7 @@ Status 
NativeRegisterContextFreeBSD_x86_64::ReadRegisterSet(uint32_t set) {
   llvm_unreachable("NativeRegisterContextFreeBSD_x86_64::ReadRegisterSet");
 }
 
-Status NativeRegisterContextFreeBSD_x86_64::WriteRegisterSet(uint32_t set) {
+Status NativeRegisterContextFreeBSD_x86_64::WriteRegisterSet(RegSetKind set) {
   switch (set) {
   case GPRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_SETREGS, m_thread.GetID(),
@@ -428,7 +429,7 @@ NativeRegisterContextFreeBSD_x86_64::ReadRegister(const 
RegisterInfo *reg_info,
 return error;
   }
 
-  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
+  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
   if (!opt_set) {
 // This is likely an internal register for lldb use only and should not be
 // directly queried.
@@ -437,7 +438,7 @@ NativeRegisterContextFreeBSD_x86_64::ReadRegister(const 
RegisterInfo *reg_info,
 return error;
   }
 
-  enum RegSetKind set = opt_set.getValue();
+  RegSetKind set = opt_set.getValue();
   error = ReadRegisterSet(set);
   if (error.Fail())
 return error;
@@ -494,7 +495,7 @@ Status NativeRegisterContextFreeBSD_x86_64::WriteRegister(
 return error;
   }
 
-  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
+  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
   if (!opt_set) {
 // This is likely an internal register for lldb use only and should not be
 // directly queried.
@@ -503,7 +504,7 @@ Status NativeRegisterContextFreeBSD_x86_64::WriteRegister(
 return error;
   }
 
-  enum RegSetKind set = opt_set.getValue();
+  RegSetKind set = opt_set.getValue();
   error = ReadRegisterSet(set);
   if (error.Fail())
 return error;
@@ -610,7 +611,7 @@ llvm::Error 
NativeRegisterContextFreeBSD_x86_64::CopyHardwareWatchpointsFrom(
 }
 
 uint8_t *
-NativeRegisterContextFreeBSD_x86_64::GetOffsetRegSetData(uint32_t set,
+NativeRegisterContextFreeBSD_x86_64::GetOffsetRegSetData(RegSetKind set,
  size_t reg_offset) {
   uint8_t *base;
   switch (set) {
@@ -625,6 +626,8 @@ 
NativeRegisterContextFreeBSD_x86_64::GetOffsetRegSetData(uint32_t set,
 break;
   case YMMRegSet:
 llvm_unreachable("GetRegSetData() is unsuitable for this regset.");
+  case MPXRegSet:
+llvm_unreachable("MPX regset should have returned error");
   }
   assert(reg_offset >= m_regset_offsets[set]);
   return base + (reg_offset - m_regset_offsets[set]);

diff  --git 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
index b70fa962707d..673cffd6e849 100644
--- 
a/lldb/source/Plugins/Process/FreeBSDRemo

[Lldb-commits] [PATCH] D93396: [lldb] [Process/FreeBSDRemote] Replace GetRegisterSetCount()

2020-12-17 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ead4e7b4a68: [lldb] [Process/FreeBSDRemote] Replace 
GetRegisterSetCount() (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93396

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp


Index: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
===
--- 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -288,13 +288,7 @@
 }
 
 uint32_t NativeRegisterContextFreeBSD_x86_64::GetRegisterSetCount() const {
-  uint32_t sets = 0;
-  for (uint32_t set_index = 0; set_index < k_num_register_sets; ++set_index) {
-if (GetSetForNativeRegNum(set_index))
-  ++sets;
-  }
-
-  return sets;
+  return k_num_register_sets;
 }
 
 const RegisterSet *


Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -288,13 +288,7 @@
 }
 
 uint32_t NativeRegisterContextFreeBSD_x86_64::GetRegisterSetCount() const {
-  uint32_t sets = 0;
-  for (uint32_t set_index = 0; set_index < k_num_register_sets; ++set_index) {
-if (GetSetForNativeRegNum(set_index))
-  ++sets;
-  }
-
-  return sets;
+  return k_num_register_sets;
 }
 
 const RegisterSet *
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D93450: [lldb] [Process/FreeBSDRemote] Use RegSetKind consistently [NFC]

2020-12-17 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG835f8de85089: [lldb] [Process/FreeBSDRemote] Use RegSetKind 
consistently [NFC] (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93450

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -74,12 +74,12 @@
   std::array m_xsave_offsets;
   std::array m_regset_offsets;
 
-  llvm::Optional GetSetForNativeRegNum(int reg_num) const;
+  llvm::Optional GetSetForNativeRegNum(uint32_t reg_num) const;
 
-  Status ReadRegisterSet(uint32_t set);
-  Status WriteRegisterSet(uint32_t set);
+  Status ReadRegisterSet(RegSetKind set);
+  Status WriteRegisterSet(RegSetKind set);
 
-  uint8_t *GetOffsetRegSetData(uint32_t set, size_t reg_offset);
+  uint8_t *GetOffsetRegSetData(RegSetKind set, size_t reg_offset);
 
   struct YMMSplitPtr {
 void *xmm;
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -303,8 +303,9 @@
   }
 }
 
-llvm::Optional
-NativeRegisterContextFreeBSD_x86_64::GetSetForNativeRegNum(int reg_num) const {
+llvm::Optional
+NativeRegisterContextFreeBSD_x86_64::GetSetForNativeRegNum(
+uint32_t reg_num) const {
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
 if (reg_num >= k_first_gpr_i386 && reg_num <= k_last_gpr_i386)
@@ -341,7 +342,7 @@
   llvm_unreachable("Register does not belong to any register set");
 }
 
-Status NativeRegisterContextFreeBSD_x86_64::ReadRegisterSet(uint32_t set) {
+Status NativeRegisterContextFreeBSD_x86_64::ReadRegisterSet(RegSetKind set) {
   switch (set) {
   case GPRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
@@ -382,7 +383,7 @@
   llvm_unreachable("NativeRegisterContextFreeBSD_x86_64::ReadRegisterSet");
 }
 
-Status NativeRegisterContextFreeBSD_x86_64::WriteRegisterSet(uint32_t set) {
+Status NativeRegisterContextFreeBSD_x86_64::WriteRegisterSet(RegSetKind set) {
   switch (set) {
   case GPRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_SETREGS, m_thread.GetID(),
@@ -428,7 +429,7 @@
 return error;
   }
 
-  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
+  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
   if (!opt_set) {
 // This is likely an internal register for lldb use only and should not be
 // directly queried.
@@ -437,7 +438,7 @@
 return error;
   }
 
-  enum RegSetKind set = opt_set.getValue();
+  RegSetKind set = opt_set.getValue();
   error = ReadRegisterSet(set);
   if (error.Fail())
 return error;
@@ -494,7 +495,7 @@
 return error;
   }
 
-  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
+  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
   if (!opt_set) {
 // This is likely an internal register for lldb use only and should not be
 // directly queried.
@@ -503,7 +504,7 @@
 return error;
   }
 
-  enum RegSetKind set = opt_set.getValue();
+  RegSetKind set = opt_set.getValue();
   error = ReadRegisterSet(set);
   if (error.Fail())
 return error;
@@ -610,7 +611,7 @@
 }
 
 uint8_t *
-NativeRegisterContextFreeBSD_x86_64::GetOffsetRegSetData(uint32_t set,
+NativeRegisterContextFreeBSD_x86_64::GetOffsetRegSetData(RegSetKind set,
  size_t reg_offset) {
   uint8_t *base;
   switch (set) {
@@ -625,6 +626,8 @@
 break;
   case YMMRegSet:
 llvm_unreachable("GetRegSetData() is unsuitable for this regset.");
+  case MPXRegSet:
+llvm_unreachable("MPX regset should have returned error");
   }
   assert(reg_offset >= m_regset_offsets[set]);
   return base + (reg_offset - m_regset_offsets[set]);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

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

I just realized that I got the char sign also mixed up. It's an `unsigned char` 
in LLDB, so the 255 is wrong (But I believe our char signedness handling is not 
correct in any C language). But the fact that we don't print YES/NO for that 
bitfield is right (just the int value is wrong).


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

https://reviews.llvm.org/D93421

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


[Lldb-commits] [PATCH] D93479: [lldb] Simplify the is_finalized logic in process and make it thread safe.

2020-12-17 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, vsk.
Herald added a subscriber: jfb.
JDevlieghere requested review of this revision.

This is a speculative fix when looking at the finalization code in Process. It 
tackles the following issues:

- Adds synchronization to prevent races between threads.
- Marks the process as finalized/invalid as soon as `Finalize` is called rather 
than at the end.
- Simplifies the code by using only a single instance variable to track 
finalization.


https://reviews.llvm.org/D93479

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -556,7 +556,7 @@
   m_profile_data_comm_mutex(), m_profile_data(), m_iohandler_sync(0),
   m_memory_cache(*this), m_allocated_memory_cache(*this),
   m_should_detach(false), m_next_event_action_up(), m_public_run_lock(),
-  m_private_run_lock(), m_finalizing(false), m_finalize_called(false),
+  m_private_run_lock(), m_finalized(false),
   m_clear_thread_plans_on_stop(false), m_force_next_event_delivery(false),
   m_last_broadcast_state(eStateInvalid), m_destroy_in_process(false),
   m_can_interpret_function_calls(false), m_warnings_issued(),
@@ -632,7 +632,8 @@
 }
 
 void Process::Finalize() {
-  m_finalizing = true;
+  if (m_finalized.exchange(true))
+return;
 
   // Destroy this process if needed
   switch (GetPrivateState()) {
@@ -644,7 +645,7 @@
   case eStateStepping:
   case eStateCrashed:
   case eStateSuspended:
-Destroy(false);
+DestroyImpl(false);
 break;
 
   case eStateInvalid:
@@ -707,7 +708,6 @@
   m_private_run_lock.TrySetRunning(); // This will do nothing if already locked
   m_private_run_lock.SetStopped();
   m_structured_data_plugin_map.clear();
-  m_finalize_called = true;
 }
 
 void Process::RegisterNotificationCallbacks(const Notifications &callbacks) {
@@ -1516,7 +1516,7 @@
 StateType Process::GetPrivateState() { return m_private_state.GetValue(); }
 
 void Process::SetPrivateState(StateType new_state) {
-  if (m_finalize_called)
+  if (!IsValid())
 return;
 
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STATE |
@@ -1567,10 +1567,7 @@
 }
 
 // Use our target to get a shared pointer to ourselves...
-if (m_finalize_called && !PrivateStateThreadIsValid())
-  BroadcastEvent(event_sp);
-else
-  m_private_state_broadcaster.BroadcastEvent(event_sp);
+m_private_state_broadcaster.BroadcastEvent(event_sp);
   } else {
 LLDB_LOGF(log,
   "Process::SetPrivateState (%s) state didn't change. Ignoring...",
@@ -1597,7 +1594,7 @@
 std::vector Process::GetLanguageRuntimes() {
   std::vector language_runtimes;
 
-  if (m_finalizing)
+  if (m_finalized)
 return language_runtimes;
 
   std::lock_guard guard(m_language_runtimes_mutex);
@@ -1615,7 +1612,7 @@
 }
 
 LanguageRuntime *Process::GetLanguageRuntime(lldb::LanguageType language) {
-  if (m_finalizing)
+  if (m_finalized)
 return nullptr;
 
   LanguageRuntime *runtime = nullptr;
@@ -1643,7 +1640,7 @@
 }
 
 bool Process::IsPossibleDynamicValue(ValueObject &in_value) {
-  if (m_finalizing)
+  if (m_finalized)
 return false;
 
   if (in_value.IsDynamic())
@@ -3345,9 +3342,12 @@
 Status Process::Destroy(bool force_kill) {
   // If we've already called Process::Finalize then there's nothing useful to
   // be done here.  Finalize has actually called Destroy already.
-  if (m_finalize_called)
+  if (m_finalized)
 return {};
+  return DestroyImpl(force_kill);
+}
 
+Status Process::DestroyImpl(bool force_kill) {
   // Tell ourselves we are in the process of destroying the process, so that we
   // don't do any unnecessary work that might hinder the destruction.  Remember
   // to set this back to false when we are done.  That way if the attempt
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -594,7 +594,7 @@
   /// \return
   /// Returns \b true if this Process has not been finalized
   /// and \b false otherwise.
-  bool IsValid() const { return !m_finalize_called; }
+  bool IsValid() const { return !m_finalized; }
 
   /// Return a multi-word command object that can be used to expose plug-in
   /// specific commands.
@@ -2831,10 +2831,11 @@
// m_currently_handling_do_on_removals are true,
// Resume will only request a resume, using this
// flag to check.
-  bool m_finalizing; // This is set at the beginning of Process::Finalize() to
- // stop functions from looking up or creating things
- // during a finalize call
-  bool m_finalize_called; // This is set at the end of

[Lldb-commits] [PATCH] D93481: [lldb/Lua] add support for multiline scripted breakpoints

2020-12-17 Thread Pedro Tammela via Phabricator via lldb-commits
tammela created this revision.
tammela requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

1 - Partial Statements

The interpreter loop runs every line it receives, so partial
Lua statements are not being handled properly. This is a problem for
multiline breakpoint scripts since the interpreter loop, for this
particular case, is just an abstraction to a partially parsed function
body declaration.

This patch addresses this issue and as a side effect improves the
general Lua interpreter loop as well. It's now possible to write partial
statements in the 'script' command.

Example:

  (lldb) script
  >>>   do
  ..>   local a = 123
  ..>   print(a)
  ..>   end
  123

The technique implemented is the same as the one employed by Lua's own REPL 
implementation.
Partial statements always errors out with the '' tag in the error
message.

2 - LoadBuffer in Lua.h

In order to support (1), we need an API for just loading string buffers. This
was not available as so far we only needed to run string buffers.

3 - Multiline scripted breakpoints

Finally, with all the base features implemented this feature is
straightforward. The interpreter loop behaves exactly the same, the
difference is that it will aggregate all Lua statements into the body of
the breakpoint function. An explicit 'quit' statement is needed to exit the
interpreter loop.

Example:

  (lldb) breakpoint command add -s lua
  Enter your Lua command(s). Type 'quit' to end.
  The commands are compiled as the body of the following Lua function
  function (frame, bp_loc, ...) end
  ..> print(456)
  ..> a = 123
  ..> quit


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93481

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
  lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test

Index: lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/partial_statements.test
@@ -0,0 +1,15 @@
+# REQUIRES: lua
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+script
+do
+local a = 123
+print(a)
+end
+# CHECK: 123
+str = 'hello there!'
+function callme()
+print(str)
+end
+callme()
+# CHECK: hello there!
+# CHECK-NOT: error
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_callback.test
@@ -1,5 +1,13 @@
 # REQUIRES: lua
-# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+# RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t
+# RUN: %lldb -s %s --script-language lua %t 2>&1 | FileCheck %s
 b main
 breakpoint command add -s lua
-# CHECK: error: This script interpreter does not support breakpoint callbacks
+local a = 123
+print(a)
+quit
+run
+# CHECK: 123
+breakpoint command add -s lua
+789_nil
+# CHECK: unexpected symbol near '789'
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -65,6 +65,10 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  void CollectDataForBreakpointCommandCallback(
+  std::vector &bp_options_vec,
+  CommandReturnObject &result) override;
+
   Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
   const char *command_body_text) override;
 
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -17,23 +17,33 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/Timer.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatAdapters.h"
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
 
 LLDB_PLUGIN_DEFINE(ScriptInterpreterLua)
 
+enum ActiveIOHandler {
+  eIOHandlerNone,
+  eIOHandlerBreakpoint,
+  eIOHandlerWatchpoint
+};
+
 class IOHandlerLuaInterpreter : public IOHandlerDelegate,
 public IOHandlerEditline {
 public:
   IOHandlerLuaInterpreter(Debugger &debugger,
-  ScriptInterpreterLua &script_interpreter)
+  ScriptInterpreterLua &scr

[Lldb-commits] [PATCH] D93225: [lldb] Add helper class for dealing with key:value; GDB responses

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

I think that streamlining the packet (de)serialization process is badly 
needing. I'm not so sure about the approach though. On one hand, it's 
definitely an improvement. On the other, it does not handle the deserialization 
process, and I was hoping we could have a solution which handles both, for two 
reasons:

- it makes things simpler
- it allows us to avoid the need for tests which explicitly test serialization 
and deserialization of each packet, as the things is implemented (and tested) 
once, centrally.

Have you looked at the how llvm YAML and JSON libraries handle 
(de)serialization? I was hoping that we could implement something similar to 
that...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93225

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


[Lldb-commits] [PATCH] D93438: ObjectFileELF: Test whether reloc_header is non-null instead of asserting.

2020-12-17 Thread Peter Collingbourne via Phabricator via lldb-commits
pcc updated this revision to Diff 312608.
pcc added a comment.

Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93438

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/test/Shell/ObjectFile/ELF/null-jmprel.yaml

Index: lldb/test/Shell/ObjectFile/ELF/null-jmprel.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/null-jmprel.yaml
@@ -0,0 +1,148 @@
+# RUN: yaml2obj %s -o %t
+# RUN: lldb-test symbols %t | FileCheck %s
+
+# CHECK: _DYNAMIC
+# CHECK: _start
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_DYN
+  Machine: EM_X86_64
+  Entry:   0x1000
+ProgramHeaders:
+  - Type:PT_PHDR
+Flags:   [ PF_R ]
+VAddr:   0x40
+Align:   0x8
+  - Type:PT_LOAD
+Flags:   [ PF_R ]
+FirstSec:.dynsym
+LastSec: .dynstr
+Align:   0x1000
+  - Type:PT_LOAD
+Flags:   [ PF_X, PF_R ]
+FirstSec:.text
+LastSec: .text
+VAddr:   0x1000
+Align:   0x1000
+  - Type:PT_LOAD
+Flags:   [ PF_W, PF_R ]
+FirstSec:.data
+LastSec: .bss
+VAddr:   0x2000
+Align:   0x1000
+  - Type:PT_DYNAMIC
+Flags:   [ PF_W, PF_R ]
+FirstSec:.data
+LastSec: .dynamic
+VAddr:   0x2000
+Align:   0x8
+  - Type:PT_GNU_RELRO
+Flags:   [ PF_R ]
+FirstSec:.data
+LastSec: .bss
+VAddr:   0x2000
+  - Type:PT_GNU_STACK
+Flags:   [ PF_W, PF_R ]
+Align:   0x0
+Sections:
+  - Name:.dynsym
+Type:SHT_DYNSYM
+Flags:   [ SHF_ALLOC ]
+Address: 0x1C8
+Link:.dynstr
+AddressAlign:0x8
+EntSize: 0x18
+  - Name:.gnu.hash
+Type:SHT_GNU_HASH
+Flags:   [ SHF_ALLOC ]
+Address: 0x1F8
+Link:.dynsym
+AddressAlign:0x8
+Header:
+  SymNdx:  0x1
+  Shift2:  0x6
+BloomFilter: [ 0x40100 ]
+HashBuckets: [ 0x1 ]
+HashValues:  [ 0xEDDB6233 ]
+  - Name:.hash
+Type:SHT_HASH
+Flags:   [ SHF_ALLOC ]
+Address: 0x218
+Link:.dynsym
+AddressAlign:0x4
+Bucket:  [ 1, 0 ]
+Chain:   [ 0, 0 ]
+  - Name:.dynstr
+Type:SHT_STRTAB
+Flags:   [ SHF_ALLOC ]
+Address: 0x230
+AddressAlign:0x1
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x1000
+AddressAlign:0x1
+Offset:  0x1000
+Content: C3
+  - Name:.data
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x2000
+AddressAlign:0x1
+Offset:  0x2000
+  - Name:.dynamic
+Type:SHT_DYNAMIC
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x2000
+Link:.dynstr
+AddressAlign:0x8
+Entries:
+  - Tag: DT_SYMTAB
+Value:   0x1C8
+  - Tag: DT_SYMENT
+Value:   0x18
+  - Tag: DT_STRTAB
+Value:   0x230
+  - Tag: DT_STRSZ
+Value:   0x8
+  - Tag: DT_GNU_HASH
+Value:   0x1F8
+  - Tag: DT_HASH
+Value:   0x218
+  - Tag: DT_JMPREL
+Value:   0x0
+  - Tag: DT_PLTRELSZ
+Value:   0x0
+  - Tag: DT_NULL
+Value:   0x0
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x3000
+AddressAlign:0x1
+  - Name:.comment
+Type:SHT_PROGBITS
+Flags:   [ SHF_MERGE, SHF_STRINGS ]
+AddressAlign:0x1
+EntSize: 0x1
+Content: 4C696E6B65723A204C4C4420372E302E3000
+Symbols:
+  - Name:_DYNAMIC
+Section: .dynamic
+Value:   0x2000
+Other:   [ STV_HIDDEN ]
+  - Name:_start
+Type:STT_FUNC
+Section: .text
+Binding: STB_GLOBAL
+Value:   0x1000
+DynamicSymbols:
+  - Name:_start
+Type:STT_FUNC
+Section: .text
+Binding: STB_GLOBAL
+Value:   0x1000
+...
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileEL

[Lldb-commits] [PATCH] D93495: CrashReason: Add MTE tag check faults to the list of crash reasons.

2020-12-17 Thread Peter Collingbourne via Phabricator via lldb-commits
pcc created this revision.
pcc added a reviewer: labath.
Herald added a subscriber: emaste.
pcc requested review of this revision.
Herald added a project: LLDB.

As of Linux 5.10, the kernel may report either of the two following
crash reasons:

- SEGV_MTEAERR: async MTE tag check fault
- SEGV_MTESERR: sync MTE tag check fault

Teach LLDB about them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93495

Files:
  lldb/source/Plugins/Process/POSIX/CrashReason.cpp
  lldb/source/Plugins/Process/POSIX/CrashReason.h


Index: lldb/source/Plugins/Process/POSIX/CrashReason.h
===
--- lldb/source/Plugins/Process/POSIX/CrashReason.h
+++ lldb/source/Plugins/Process/POSIX/CrashReason.h
@@ -22,6 +22,8 @@
   eInvalidAddress,
   ePrivilegedAddress,
   eBoundViolation,
+  eAsyncTagCheckFault,
+  eSyncTagCheckFault,
 
   // SIGILL crash reasons.
   eIllegalOpcode,
Index: lldb/source/Plugins/Process/POSIX/CrashReason.cpp
===
--- lldb/source/Plugins/Process/POSIX/CrashReason.cpp
+++ lldb/source/Plugins/Process/POSIX/CrashReason.cpp
@@ -58,6 +58,16 @@
 #endif
   case SEGV_BNDERR:
 return CrashReason::eBoundViolation;
+#ifndef SEGV_MTEAERR
+#define SEGV_MTEAERR 8
+#endif
+  case SEGV_MTEAERR:
+return CrashReason::eAsyncTagCheckFault;
+#ifndef SEGV_MTESERR
+#define SEGV_MTESERR 9
+#endif
+  case SEGV_MTESERR:
+return CrashReason::eSyncTagCheckFault;
   }
 
   return CrashReason::eInvalidCrashReason;
@@ -166,6 +176,12 @@
   case CrashReason::eBoundViolation:
 str = "signal SIGSEGV: bound violation";
 break;
+  case CrashReason::eAsyncTagCheckFault:
+str = "signal SIGSEGV: async tag check fault";
+break;
+  case CrashReason::eSyncTagCheckFault:
+str = "signal SIGSEGV: sync tag check fault";
+break;
   case CrashReason::eIllegalOpcode:
 str = "signal SIGILL: illegal instruction";
 break;
@@ -246,6 +262,12 @@
   case CrashReason::eBoundViolation:
 str = "eBoundViolation";
 break;
+  case CrashReason::eAsyncTagCheckFault:
+str = "eAsyncTagCheckFault";
+break;
+  case CrashReason::eSyncTagCheckFault:
+str = "eSyncTagCheckFault";
+break;
 
   // SIGILL crash reasons.
   case CrashReason::eIllegalOpcode:


Index: lldb/source/Plugins/Process/POSIX/CrashReason.h
===
--- lldb/source/Plugins/Process/POSIX/CrashReason.h
+++ lldb/source/Plugins/Process/POSIX/CrashReason.h
@@ -22,6 +22,8 @@
   eInvalidAddress,
   ePrivilegedAddress,
   eBoundViolation,
+  eAsyncTagCheckFault,
+  eSyncTagCheckFault,
 
   // SIGILL crash reasons.
   eIllegalOpcode,
Index: lldb/source/Plugins/Process/POSIX/CrashReason.cpp
===
--- lldb/source/Plugins/Process/POSIX/CrashReason.cpp
+++ lldb/source/Plugins/Process/POSIX/CrashReason.cpp
@@ -58,6 +58,16 @@
 #endif
   case SEGV_BNDERR:
 return CrashReason::eBoundViolation;
+#ifndef SEGV_MTEAERR
+#define SEGV_MTEAERR 8
+#endif
+  case SEGV_MTEAERR:
+return CrashReason::eAsyncTagCheckFault;
+#ifndef SEGV_MTESERR
+#define SEGV_MTESERR 9
+#endif
+  case SEGV_MTESERR:
+return CrashReason::eSyncTagCheckFault;
   }
 
   return CrashReason::eInvalidCrashReason;
@@ -166,6 +176,12 @@
   case CrashReason::eBoundViolation:
 str = "signal SIGSEGV: bound violation";
 break;
+  case CrashReason::eAsyncTagCheckFault:
+str = "signal SIGSEGV: async tag check fault";
+break;
+  case CrashReason::eSyncTagCheckFault:
+str = "signal SIGSEGV: sync tag check fault";
+break;
   case CrashReason::eIllegalOpcode:
 str = "signal SIGILL: illegal instruction";
 break;
@@ -246,6 +262,12 @@
   case CrashReason::eBoundViolation:
 str = "eBoundViolation";
 break;
+  case CrashReason::eAsyncTagCheckFault:
+str = "eAsyncTagCheckFault";
+break;
+  case CrashReason::eSyncTagCheckFault:
+str = "eSyncTagCheckFault";
+break;
 
   // SIGILL crash reasons.
   case CrashReason::eIllegalOpcode:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

2020-12-17 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment.

After chatting with Raphael offline, we realize that `BOOL` will indeed always 
be signed. I was under the impression for some reason that it was actually 
unsigned. So the fix is simpler is that we just need to make the formatter use 
`GetValueAsSigned(...)`


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

https://reviews.llvm.org/D93421

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


[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

2020-12-17 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik updated this revision to Diff 312636.
shafik added a comment.

Updating diff to reflect comments.


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

https://reviews.llvm.org/D93421

Files:
  lldb/source/Plugins/Language/ObjC/Cocoa.cpp
  
lldb/test/API/functionalities/data-formatter/boolreference/TestFormattersBoolRefPtr.py
  lldb/test/API/functionalities/data-formatter/boolreference/main.mm
  lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
  lldb/test/API/lang/objc/bitfield_ivars/main.m

Index: lldb/test/API/lang/objc/bitfield_ivars/main.m
===
--- lldb/test/API/lang/objc/bitfield_ivars/main.m
+++ lldb/test/API/lang/objc/bitfield_ivars/main.m
@@ -1,5 +1,13 @@
 #import 
 
+typedef struct {
+unsigned char fieldOne : 1;
+unsigned char fieldTwo : 1;
+unsigned char fieldThree : 1;
+unsigned char fieldFour : 1;
+unsigned char fieldfive : 1;
+} UCBitFields;
+
 @interface HasBitfield : NSObject {
 @public
 unsigned field1 : 1;
@@ -59,6 +67,10 @@
 hb2->field2 = 3;
 hb2->field3 = 4;
 
+UCBitFields myField = {0};
+myField.fieldTwo = 1;
+myField.fieldfive = 1;
+
 return 0; // break here
 }
 
Index: lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
===
--- lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
+++ lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
@@ -24,6 +24,8 @@
  'field1 =', '10',
  'field2 =', '3',
  'field3 =', '4'])
+self.expect('p myField',
+   substrs=['(UCBitFields)', 'fieldOne = \'\\0\'', 'fieldTwo = \'\\x01\'', 'fieldThree = \'\\0\'', 'fieldFour = \'\\0\'', 'fieldfive = \'\\x01\''])
 
 # This test is meant to be xfailed, but running the test triggers an ASan
 # issue, so it must be skipped for now.
Index: lldb/test/API/functionalities/data-formatter/boolreference/main.mm
===
--- lldb/test/API/functionalities/data-formatter/boolreference/main.mm
+++ lldb/test/API/functionalities/data-formatter/boolreference/main.mm
@@ -1,12 +1,20 @@
 #import 
 
+typedef struct {
+BOOL fieldOne : 1;
+BOOL fieldTwo : 1;
+BOOL fieldThree : 1;
+BOOL fieldFour : 1;
+BOOL fieldfive : 1;
+} BoolBitFields;
+
 int main (int argc, const char * argv[])
 {
-NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+  NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
 
 	BOOL yes  = YES;
 	BOOL no = NO;
-BOOL unset = 12;
+  BOOL unset = 12;
 	
 	BOOL &yes_ref = yes;
 	BOOL &no_ref = no;
@@ -16,6 +24,10 @@
 	BOOL* no_ptr = &no;
 	BOOL* unset_ptr = &unset;
 
+  BoolBitFields myField = {0};
+  myField.fieldTwo = YES;
+  myField.fieldfive = YES;
+
 [pool drain];// Set break point at this line.
 return 0;
 }
Index: lldb/test/API/functionalities/data-formatter/boolreference/TestFormattersBoolRefPtr.py
===
--- lldb/test/API/functionalities/data-formatter/boolreference/TestFormattersBoolRefPtr.py
+++ lldb/test/API/functionalities/data-formatter/boolreference/TestFormattersBoolRefPtr.py
@@ -75,3 +75,6 @@
 substrs=['NO'])
 if not(isArm):
 self.expect('frame variable unset', substrs=['12'])
+
+self.expect('p myField',
+   substrs=['(BoolBitFields)', 'fieldOne = NO', 'fieldTwo = 255', 'fieldThree = NO', 'fieldFour = NO', 'fieldfive = 255'])
Index: lldb/source/Plugins/Language/ObjC/Cocoa.cpp
===
--- lldb/source/Plugins/Language/ObjC/Cocoa.cpp
+++ lldb/source/Plugins/Language/ObjC/Cocoa.cpp
@@ -1035,7 +1035,7 @@
 if (!real_guy_sp)
   return false;
   }
-  uint8_t value = (real_guy_sp->GetValueAsUnsigned(0) & 0xFF);
+  uint8_t value = (real_guy_sp->GetValueAsSigned(0) & 0xFF);
   switch (value) {
   case 0:
 stream.Printf("NO");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits