[Lldb-commits] [lldb] 5a95ff2 - [lldb] Fix -Wunused-but-set-variable

2021-08-16 Thread Fangrui Song via lldb-commits 

Author: Fangrui Song
Date: 2021-08-16T16:41:17-07:00
New Revision: 5a95ff2bfcc191f3ffecb69ac1d762a93b149eee

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

LOG: [lldb] Fix -Wunused-but-set-variable

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 79e6b3f609651..f2ed6330e36d1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1584,6 +1584,7 @@ DWARFASTParserClang::ParseStructureLikeDIE(const 
SymbolContext ,
 }
   }
   assert(tag_decl_kind != -1);
+  (void)tag_decl_kind;
   bool clang_type_was_created = false;
   clang_type.SetCompilerType(
   _ast, dwarf->GetForwardDeclDieToClangType().lookup(die.GetDIE()));



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

[Lldb-commits] [PATCH] D101406: Rename human-readable name for DW_LANG_Mips_Assembler

2021-08-16 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

Everyone uses this for any assembly in their code. Should we get rid of this 
warning? I find it is useless and would like to see it go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101406

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

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

2021-08-16 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

In D108061#2947526 , @rdhindsa wrote:

> Reverted code changes from ObjectFileELF as suggested and added directly to 
> DYLDRendezvous.cpp.
>
> Is there a better place to place the test, or is it okay to keep under 
> ObjectFile.

I think a test would be better added to the python test area of LLDB.

The test will look something like:

  """
  Test that LLDB can launch a linux executable through the dynamic loader and 
still hit a breakpoint.
  """
  
  import lldb
  
  from lldbsuite.test.decorators import *
  from lldbsuite.test.lldbtest import *
  from lldbsuite.test import lldbutil
  
  class TestLinux64LaunchingViaDynamicLoader(TestBase):
  mydir = TestBase.compute_mydir(__file__)
  
  def test(self):
  """Test we can launch and hit a breakpoint when we run our program 
through the dynamic loader"""
  self.build()
  exe = "/lib64/ld-linux-x86-64.so.2"
  launch_info = lldb.SBLaunchInfo([exe, self.getBuildArtifact("a.out")])
  launch_info.SetWorkingDirectory(self.get_process_working_directory())
  lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"), exe=exe, in_cwd=False, launch_info=launch_info)

This should do everything that is needed!

So the steps to create the test are:

- make a new directory in "lldb/test/API/functionalities/dyld-launch-linux"
  - create the main.cpp as noted above
  - add a TestDyldLaunchLinux.py and put the above test case python into it
  - copy a Makefile from another test like 
"lldb/test/API/commands/expression/anonymous-struct/Makefile"  into this 
directory

To run the test, build "check-lldb" first and kill it once it starts running 
the test suite. Then you can type this from the llvm build directory:

  $ ./bin/llvm-lit -sv ../lldb/test/API/functionalities/dyld-launch-linux

Let me know if there are any questions. You can then remove all of the test 
files  from lldb/test/Shell/ObjectFile/ELF




Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:356-361
+, nullptr /* containingSourceFiles */,
+DebugStateCandidates, eFunctionNameTypeFull, eLanguageTypeC,
+0,   /* offset */
+eLazyBoolNo, /* skip_prologue */
+true,/* internal */
+false /* request_hardware */);

I know you copied this from below, but LLVM coding conventions say to do 
comments like:

https://llvm.org/docs/CodingStandards.html#id13

And might be nice to fix the ones below too. 



Comment at: lldb/test/Shell/ObjectFile/ELF/ld_test.test:15
+#
+# RUN: %lldb -b -s %t.in -- /lib64/ld-linux-x86-64.so.2 --library-path %t  
%tmain 2>&1 | FileCheck %s 
+#

So I worry the "/lib64/ld-linux-x86-64.so.2" path won't be portable across 
different linux distributions. Anyone know if it is the same on every linux?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

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

[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

2021-08-16 Thread walter erquinigo via Phabricator via lldb-commits 
wallace accepted this revision.
wallace added a comment.

good job


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

https://reviews.llvm.org/D107669

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

[Lldb-commits] [lldb] aa575ed - Ah, fix formatting, I didn't notice lldb-instr's code

2021-08-16 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2021-08-16T13:46:54-07:00
New Revision: aa575ed918632358b65da3e161d55254092b7416

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

LOG: Ah, fix formatting, I didn't notice lldb-instr's code
additions were unusually formatted.

Added: 


Modified: 
lldb/source/API/SBMemoryRegionInfo.cpp

Removed: 




diff  --git a/lldb/source/API/SBMemoryRegionInfo.cpp 
b/lldb/source/API/SBMemoryRegionInfo.cpp
index d924a31cca0d..c0f5456223da 100644
--- a/lldb/source/API/SBMemoryRegionInfo.cpp
+++ b/lldb/source/API/SBMemoryRegionInfo.cpp
@@ -116,14 +116,14 @@ const char *SBMemoryRegionInfo::GetName() {
   return m_opaque_up->GetName().AsCString();
 }
 
-bool SBMemoryRegionInfo::HasDirtyMemoryPageList() 
{LLDB_RECORD_METHOD_NO_ARGS(bool, SBMemoryRegionInfo, HasDirtyMemoryPageList);
-
+bool SBMemoryRegionInfo::HasDirtyMemoryPageList() {
+  LLDB_RECORD_METHOD_NO_ARGS(bool, SBMemoryRegionInfo, HasDirtyMemoryPageList);
 
   return m_opaque_up->GetDirtyPageList().hasValue();
 }
 
-uint32_t SBMemoryRegionInfo::GetNumDirtyPages() 
{LLDB_RECORD_METHOD_NO_ARGS(uint32_t, SBMemoryRegionInfo, GetNumDirtyPages);
-
+uint32_t SBMemoryRegionInfo::GetNumDirtyPages() {
+  LLDB_RECORD_METHOD_NO_ARGS(uint32_t, SBMemoryRegionInfo, GetNumDirtyPages);
 
   uint32_t num_dirty_pages = 0;
   llvm::Optional> dirty_page_list =
@@ -134,8 +134,9 @@ uint32_t SBMemoryRegionInfo::GetNumDirtyPages() 
{LLDB_RECORD_METHOD_NO_ARGS(uint
   return num_dirty_pages;
 }
 
-addr_t SBMemoryRegionInfo::GetDirtyPageAddressAtIndex(uint32_t idx) 
{LLDB_RECORD_METHOD(lldb::addr_t, SBMemoryRegionInfo, 
GetDirtyPageAddressAtIndex, (uint32_t), idx);
-
+addr_t SBMemoryRegionInfo::GetDirtyPageAddressAtIndex(uint32_t idx) {
+  LLDB_RECORD_METHOD(lldb::addr_t, SBMemoryRegionInfo,
+ GetDirtyPageAddressAtIndex, (uint32_t), idx);
 
   addr_t dirty_page_addr = LLDB_INVALID_ADDRESS;
   const llvm::Optional> _page_list =
@@ -146,8 +147,8 @@ addr_t 
SBMemoryRegionInfo::GetDirtyPageAddressAtIndex(uint32_t idx) {LLDB_RECORD
   return dirty_page_addr;
 }
 
-int SBMemoryRegionInfo::GetPageSize() {LLDB_RECORD_METHOD_NO_ARGS(int, 
SBMemoryRegionInfo, GetPageSize);
-
+int SBMemoryRegionInfo::GetPageSize() {
+  LLDB_RECORD_METHOD_NO_ARGS(int, SBMemoryRegionInfo, GetPageSize);
 
   return m_opaque_up->GetPageSize();
 }



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

[Lldb-commits] [lldb] 65bc8ba - Correctly update reproducer hooks for SB API I added

2021-08-16 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2021-08-16T13:41:18-07:00
New Revision: 65bc8ba1a2d434ba72f6f476bfd4a9ae69cb16b2

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

LOG: Correctly update reproducer hooks for SB API I added

In 9ea6dd5cfac0b233fbb148c1e2d0f81f816737c8 /
https://reviews.llvm.org/D88387 where I added skinny corefile
creation, I added new SB API and tried to manually update the hooks
for the reproducers. I missed a spot, and I should have used
lldb-instr to update the instrumentation automatically.

Added: 


Modified: 
lldb/source/API/SBMemoryRegionInfo.cpp

Removed: 




diff  --git a/lldb/source/API/SBMemoryRegionInfo.cpp 
b/lldb/source/API/SBMemoryRegionInfo.cpp
index ab74d559387fb..d924a31cca0d4 100644
--- a/lldb/source/API/SBMemoryRegionInfo.cpp
+++ b/lldb/source/API/SBMemoryRegionInfo.cpp
@@ -116,14 +116,14 @@ const char *SBMemoryRegionInfo::GetName() {
   return m_opaque_up->GetName().AsCString();
 }
 
-bool SBMemoryRegionInfo::HasDirtyMemoryPageList() {
-  LLDB_RECORD_METHOD_NO_ARGS(bool, SBMemoryRegionInfo, HasDirtyMemoryPageList);
+bool SBMemoryRegionInfo::HasDirtyMemoryPageList() 
{LLDB_RECORD_METHOD_NO_ARGS(bool, SBMemoryRegionInfo, HasDirtyMemoryPageList);
+
 
   return m_opaque_up->GetDirtyPageList().hasValue();
 }
 
-uint32_t SBMemoryRegionInfo::GetNumDirtyPages() {
-  LLDB_RECORD_METHOD_NO_ARGS(uint32_t, SBMemoryRegionInfo, GetNumDirtyPages);
+uint32_t SBMemoryRegionInfo::GetNumDirtyPages() 
{LLDB_RECORD_METHOD_NO_ARGS(uint32_t, SBMemoryRegionInfo, GetNumDirtyPages);
+
 
   uint32_t num_dirty_pages = 0;
   llvm::Optional> dirty_page_list =
@@ -134,9 +134,8 @@ uint32_t SBMemoryRegionInfo::GetNumDirtyPages() {
   return num_dirty_pages;
 }
 
-addr_t SBMemoryRegionInfo::GetDirtyPageAddressAtIndex(uint32_t idx) {
-  LLDB_RECORD_METHOD(addr_t, SBMemoryRegionInfo, GetDirtyPageAddressAtIndex,
- (uint32_t), idx);
+addr_t SBMemoryRegionInfo::GetDirtyPageAddressAtIndex(uint32_t idx) 
{LLDB_RECORD_METHOD(lldb::addr_t, SBMemoryRegionInfo, 
GetDirtyPageAddressAtIndex, (uint32_t), idx);
+
 
   addr_t dirty_page_addr = LLDB_INVALID_ADDRESS;
   const llvm::Optional> _page_list =
@@ -147,8 +146,9 @@ addr_t 
SBMemoryRegionInfo::GetDirtyPageAddressAtIndex(uint32_t idx) {
   return dirty_page_addr;
 }
 
-int SBMemoryRegionInfo::GetPageSize() {
-  LLDB_RECORD_METHOD_NO_ARGS(int, SBMemoryRegionInfo, GetPageSize);
+int SBMemoryRegionInfo::GetPageSize() {LLDB_RECORD_METHOD_NO_ARGS(int, 
SBMemoryRegionInfo, GetPageSize);
+
+
   return m_opaque_up->GetPageSize();
 }
 
@@ -196,6 +196,10 @@ void RegisterMethods(Registry ) {
   LLDB_REGISTER_METHOD(const char *, SBMemoryRegionInfo, GetName, ());
   LLDB_REGISTER_METHOD(bool, SBMemoryRegionInfo, GetDescription,
(lldb::SBStream &));
+  LLDB_REGISTER_METHOD(bool, SBMemoryRegionInfo, HasDirtyMemoryPageList, ());
+  LLDB_REGISTER_METHOD(uint32_t, SBMemoryRegionInfo, GetNumDirtyPages, ());
+  LLDB_REGISTER_METHOD(lldb::addr_t, SBMemoryRegionInfo, 
GetDirtyPageAddressAtIndex, (uint32_t));
+  LLDB_REGISTER_METHOD(int, SBMemoryRegionInfo, GetPageSize, ());
 }
 
 }



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

[Lldb-commits] [PATCH] D106466: [llvm+lldb] Fix#2 of DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

2021-08-16 Thread David Blaikie via Phabricator via lldb-commits 
dblaikie added a comment.

I assume there's already test coverage for rnglistx in debug_info.dwo/split 
unit? (because in that case there's no rnglists_base, but rnglistx is usable) - 
could you explain how this code avoids treating the split unit rnglists_base == 
0 case as "there is no rnglists_base and so rnglistx isn't usable"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106466

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

[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

2021-08-16 Thread hanbing wang via Phabricator via lldb-commits 
hanbingwang updated this revision to Diff 366713.
hanbingwang marked an inline comment as not done.
hanbingwang added a comment.

merge into one line: "json_module["loadAddress"] = oss.str();"


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

https://reviews.llvm.org/D107669

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/common/CMakeLists.txt
  lldb/source/Plugins/Trace/common/TraceJSONStructs.cpp
  lldb/source/Plugins/Trace/common/TraceJSONStructs.h
  lldb/source/Plugins/Trace/common/TraceSessionFileParser.cpp
  lldb/source/Plugins/Trace/common/TraceSessionFileParser.h
  lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp
  lldb/source/Plugins/Trace/common/TraceSessionSaver.h
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.h
  lldb/test/API/commands/trace/TestTraceSave.py

Index: lldb/test/API/commands/trace/TestTraceSave.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceSave.py
@@ -0,0 +1,97 @@
+import lldb
+from intelpt_testcase import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceSave(TraceIntelPTTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def testErrorMessages(self):
+# We first check the output when there are no targets
+self.expect("process trace save",
+substrs=["error: invalid target, create a target using the 'target create' command"],
+error=True)
+
+# We now check the output when there's a non-running target
+self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+
+self.expect("process trace save",
+substrs=["error: invalid process"],
+error=True)
+
+# Now we check the output when there's a running target without a trace
+self.expect("b main")
+self.expect("run")
+
+self.expect("process trace save",
+substrs=["error: Process is not being traced"],
+error=True)
+
+def testSaveToInvalidDir(self):
+self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+self.expect("b main")
+self.expect("r")
+self.expect("thread trace start")
+self.expect("n")
+
+# Check the output when saving without providing the directory argument
+self.expect("process trace save -d",
+substrs=["error: last option requires an argument"],
+error=True)
+
+# Check the output when saving to an invalid directory
+self.expect("process trace save -d /",
+substrs=["error: couldn't write to the file"],
+error=True)
+
+def testSaveWhenNotLiveTrace(self):
+self.expect("trace load -v " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
+substrs=["intel-pt"])
+
+# Check the output when not doing live tracing
+self.expect("process trace save -d " +
+os.path.join(self.getBuildDir(), "intelpt-trace", "trace_not_live_dir"),
+substrs=["error: Saving a trace requires a live process."],
+error=True)
+
+
+def testSaveTrace(self):
+self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+self.expect("b main")
+self.expect("r")
+self.expect("thread trace start")
+self.expect("b 7")
+
+ci = self.dbg.GetCommandInterpreter()
+res = lldb.SBCommandReturnObject()
+
+ci.HandleCommand("thread trace dump instructions -c 10 --forwards", res)
+self.assertEqual(res.Succeeded(), True)
+first_ten_instructions = res.GetOutput()
+
+ci.HandleCommand("thread trace dump instructions -c 10", res)
+self.assertEqual(res.Succeeded(), True)
+last_ten_instructions = res.GetOutput()
+
+# Now, save the trace to 
+self.expect("process trace save -d " +
+os.path.join(self.getBuildDir(), "intelpt-trace", "trace_copy_dir"))
+
+# Load the trace just saved
+self.expect("trace load -v " +
+os.path.join(self.getBuildDir(), "intelpt-trace",

[Lldb-commits] [PATCH] D107761: [LLDB][GUI] Refactor form drawing using subsurfaces

2021-08-16 Thread Omar Emara via Phabricator via lldb-commits 
OmarEmaraDev added a comment.

@clayborg  Perhaps you missed this, it is essential the same as D107182 
 but without the unsupported function. It 
would be good to have this committed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107761

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

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

2021-08-16 Thread Rumeet Dhindsa via Phabricator via lldb-commits 
rdhindsa updated this revision to Diff 366692.
rdhindsa added a comment.

Reverted code changes from ObjectFileELF as suggested and added directly to 
DYLDRendezvous.cpp.

Is there a better place to place the test, or is it okay to keep under 
ObjectFile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/test/Shell/ObjectFile/ELF/Inputs/main.cpp
  lldb/test/Shell/ObjectFile/ELF/Inputs/signal_file.cpp
  lldb/test/Shell/ObjectFile/ELF/Inputs/signal_file.h
  lldb/test/Shell/ObjectFile/ELF/ld_test.test

Index: lldb/test/Shell/ObjectFile/ELF/ld_test.test
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/ld_test.test
@@ -0,0 +1,20 @@
+# REQUIRES: x86
+# REQUIRES: system-linux
+#
+# RUN: %clang -target x86_64-pc-linux -o %t1.o -c %S/Inputs/signal_file.cpp
+# RUN: %clang -target x86_64-pc-linux -o %t2.o -c %S/Inputs/main.cpp
+# RUN: %clang -target x86_64-pc-linux -shared %t1.o -o %t3.so
+# RUN: %clang -o %tmain %t2.o %t3.so -L. -Wl,-rpath,%t
+#
+# RUN: echo '-n' > %t.in
+# RUN: echo 'run' >> %t.in
+# RUN: echo 'bt' >> %t.in
+#
+# RUN: %lldb -b -s %t.in -- %tmain 2>&1 | FileCheck %s
+#
+# RUN: %lldb -b -s %t.in -- /lib64/ld-linux-x86-64.so.2 --library-path %t  %tmain 2>&1 | FileCheck %s 
+#
+# bt
+# CHECK: (lldb) bt
+# CHECK: ld_test.test.tmp3.so`get_signal_crash()
+#
Index: lldb/test/Shell/ObjectFile/ELF/Inputs/signal_file.h
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/Inputs/signal_file.h
@@ -0,0 +1 @@
+int get_signal_crash(void);
Index: lldb/test/Shell/ObjectFile/ELF/Inputs/signal_file.cpp
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/Inputs/signal_file.cpp
@@ -0,0 +1,7 @@
+#include "signal_file.h"
+#include 
+
+int get_signal_crash(void) {
+  raise(SIGSEGV);
+  return 0;
+}
Index: lldb/test/Shell/ObjectFile/ELF/Inputs/main.cpp
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/Inputs/main.cpp
@@ -0,0 +1,3 @@
+#include "signal_file.h"
+
+int main() { return get_signal_crash(); }
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
@@ -333,28 +333,48 @@
 LLDB_LOG(log, "Rendezvous structure is not set up yet. "
   "Trying to locate rendezvous breakpoint in the interpreter "
   "by symbol name.");
-ModuleSP interpreter = LoadInterpreterModule();
-if (!interpreter) {
-  LLDB_LOG(log, "Can't find interpreter, rendezvous breakpoint isn't set.");
-  return false;
-}
-
-// Function names from different dynamic loaders that are known to be used
-// as rendezvous between the loader and debuggers.
+// Function names from different dynamic loaders that are known to be
+// used as rendezvous between the loader and debuggers.
 static std::vector DebugStateCandidates{
 "_dl_debug_state", "rtld_db_dlactivity", "__dl_rtld_db_dlactivity",
 "r_debug_state",   "_r_debug_state", "_rtld_debug_state",
 };
 
-FileSpecList containingModules;
-containingModules.Append(interpreter->GetFileSpec());
-dyld_break = target.CreateBreakpoint(
-, nullptr /* containingSourceFiles */,
-DebugStateCandidates, eFunctionNameTypeFull, eLanguageTypeC,
-0,   /* offset */
-eLazyBoolNo, /* skip_prologue */
-true,/* internal */
-false /* request_hardware */);
+ModuleSP interpreter = LoadInterpreterModule();
+if (!interpreter) {
+  if (NameMatches(m_process->GetTarget()
+  .GetExecutableModulePointer()
+  ->GetFileSpec()
+  .GetFilename()
+  .GetCString(),
+  NameMatch::StartsWith, "ld-")) {
+FileSpecList containingModules;
+containingModules.Append(
+m_process->GetTarget().GetExecutableModulePointer()->GetFileSpec());
+
+dyld_break = target.CreateBreakpoint(
+, nullptr /* containingSourceFiles */,
+DebugStateCandidates, eFunctionNameTypeFull, eLanguageTypeC,
+0,   /* offset */
+eLazyBoolNo, /* skip_prologue */
+true,/* internal */
+false /* request_hardware */);
+  } else {
+LLDB_LOG(log,
+ "Can't find interpreter, rendezvous breakpoint isn't set.");
+

[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

2021-08-16 Thread hanbing wang via Phabricator via lldb-commits 
hanbingwang updated this revision to Diff 366690.
hanbingwang added a comment.

*TraceIntelPT.h, TraceIntelPT.cpp:
-add new function GetLiveProcess()
*TraceIntelPTSessionSaver.h, TraceIntelPTSessionSaver.cpp

- the function SaveToDisk() no longer requires "m_live_process" as input param.

*TestTraceSave.py:
-added new test cases.


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

https://reviews.llvm.org/D107669

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/common/CMakeLists.txt
  lldb/source/Plugins/Trace/common/TraceJSONStructs.cpp
  lldb/source/Plugins/Trace/common/TraceJSONStructs.h
  lldb/source/Plugins/Trace/common/TraceSessionFileParser.cpp
  lldb/source/Plugins/Trace/common/TraceSessionFileParser.h
  lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp
  lldb/source/Plugins/Trace/common/TraceSessionSaver.h
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.h
  lldb/test/API/commands/trace/TestTraceSave.py

Index: lldb/test/API/commands/trace/TestTraceSave.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceSave.py
@@ -0,0 +1,97 @@
+import lldb
+from intelpt_testcase import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceSave(TraceIntelPTTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def testErrorMessages(self):
+# We first check the output when there are no targets
+self.expect("process trace save",
+substrs=["error: invalid target, create a target using the 'target create' command"],
+error=True)
+
+# We now check the output when there's a non-running target
+self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+
+self.expect("process trace save",
+substrs=["error: invalid process"],
+error=True)
+
+# Now we check the output when there's a running target without a trace
+self.expect("b main")
+self.expect("run")
+
+self.expect("process trace save",
+substrs=["error: Process is not being traced"],
+error=True)
+
+def testSaveToInvalidDir(self):
+self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+self.expect("b main")
+self.expect("r")
+self.expect("thread trace start")
+self.expect("n")
+
+# Check the output when saving without providing the directory argument
+self.expect("process trace save -d",
+substrs=["error: last option requires an argument"],
+error=True)
+
+# Check the output when saving to an invalid directory
+self.expect("process trace save -d /",
+substrs=["error: couldn't write to the file"],
+error=True)
+
+def testSaveWhenNotLiveTrace(self):
+self.expect("trace load -v " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
+substrs=["intel-pt"])
+
+# Check the output when not doing live tracing
+self.expect("process trace save -d " +
+os.path.join(self.getBuildDir(), "intelpt-trace", "trace_not_live_dir"),
+substrs=["error: Saving a trace requires a live process."],
+error=True)
+
+
+def testSaveTrace(self):
+self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+self.expect("b main")
+self.expect("r")
+self.expect("thread trace start")
+self.expect("b 7")
+
+ci = self.dbg.GetCommandInterpreter()
+res = lldb.SBCommandReturnObject()
+
+ci.HandleCommand("thread trace dump instructions -c 10 --forwards", res)
+self.assertEqual(res.Succeeded(), True)
+first_ten_instructions = res.GetOutput()
+
+ci.HandleCommand("thread trace dump instructions -c 10", res)
+self.assertEqual(res.Succeeded(), True)
+last_ten_instructions = res.GetOutput()
+
+# Now, save the trace to 
+self.expect("process trace save -d " +
+os.path.join(self.getBuildDir(), "intelpt-trace", "trace_copy_dir"))
+

[Lldb-commits] [PATCH] D107470: [llvm+lldb] Remove dead-code in DWARFListTableHeader::extract modifying DWARFDataExtractor

2021-08-16 Thread Jan Kratochvil via Phabricator via lldb-commits 
jankratochvil updated this revision to Diff 366684.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107470

Files:
  lldb/unittests/SymbolFile/DWARF/DWARFUnitTest.cpp
  llvm/include/llvm/DebugInfo/DWARF/DWARFListTable.h
  llvm/lib/DebugInfo/DWARF/DWARFListTable.cpp
  llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
  llvm/unittests/DebugInfo/DWARF/DWARFListTableTest.cpp

Index: llvm/unittests/DebugInfo/DWARF/DWARFListTableTest.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DWARFListTableTest.cpp
+++ llvm/unittests/DebugInfo/DWARF/DWARFListTableTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "llvm/DebugInfo/DWARF/DWARFListTable.h"
+#include "llvm/DebugInfo/DWARF/DWARFDebugRnglists.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 
@@ -99,4 +100,136 @@
   EXPECT_EQ(Header.length(), sizeof(SecData) - 1);
 }
 
+TEST(DWARFListTableHeader, AddressSize64Header) {
+  static const char SecData[] = "\x1a\x00\x00\x00" // Length
+"\x05\x00" // Version
+"\x08" // Address size
+"\x00" // Segment selector size
+"\x00\x00\x00\x00" // Offset entry count
+"\x06" // DW_RLE_start_end
+"\x07\x06\x05\x04" // 64-bit starting address
+"\x03\x02\x01\x00" // 64-bit starting address
+"\x87\x86\x85\x84" // 64-bit ending address
+"\x83\x82\x81\x80" // 64-bit ending address
+"\x00";// DW_RLE_end_of_list
+  DWARFDataExtractor Extractor(StringRef(SecData, sizeof(SecData) - 1),
+   /*isLittleEndian=*/true,
+   /*AddrSize=*/4);
+  DWARFListTableHeader Header(/*SectionName=*/".debug_rnglists",
+  /*ListTypeString=*/"range");
+  uint64_t Offset = 0;
+  llvm::DWARFDebugRnglistTable Table;
+  llvm::Error E = Table.extractHeaderAndOffsets(Extractor, );
+  ASSERT_FALSE(!!E);
+  EXPECT_EQ(Offset, 12U);
+  EXPECT_EQ(Table.length(), sizeof(SecData) - 1);
+  EXPECT_EQ(Extractor.getAddressSize(), 4U);
+  EXPECT_EQ(Table.getAddrSize(), 8U);
+  Expected List = Table.findList(Extractor, Offset);
+  ASSERT_THAT_EXPECTED(List, Succeeded());
+  ASSERT_EQ(List->getEntries().size(), 2U);
+  EXPECT_EQ(List->getEntries()[0].Offset, 12 + 0U);
+  EXPECT_EQ(List->getEntries()[0].EntryKind, dwarf::DW_RLE_start_end);
+  EXPECT_EQ(List->getEntries()[0].Value0, 0x0001020304050607U);
+  EXPECT_EQ(List->getEntries()[0].Value1, 0x8081828384858687U);
+  EXPECT_EQ(List->getEntries()[1].Offset, 12 + 17U);
+  EXPECT_EQ(List->getEntries()[1].EntryKind, dwarf::DW_RLE_end_of_list);
+}
+
+TEST(DWARFListTableHeader, AddressSize64Offset) {
+  static const char SecData[] = "\x1a\x00\x00\x00" // Length
+"\x05\x00" // Version
+"\x08" // Address size
+"\x00" // Segment selector size
+"\x00\x00\x00\x00" // Offset entry count
+"\x06" // DW_RLE_start_end
+"\x07\x06\x05\x04" // 64-bit starting address
+"\x03\x02\x01\x00" // 64-bit starting address
+"\x87\x86\x85\x84" // 64-bit ending address
+"\x83\x82\x81\x80" // 64-bit ending address
+"\x00";// DW_RLE_end_of_list
+  DWARFDataExtractor Extractor(StringRef(SecData, sizeof(SecData) - 1),
+   /*isLittleEndian=*/true,
+   /*AddrSize=*/4);
+  DWARFListTableHeader Header(/*SectionName=*/".debug_rnglists",
+  /*ListTypeString=*/"range");
+  llvm::DWARFDebugRnglistTable Table;
+  // Table.getAddrSize() is uninitialized.
+  EXPECT_EQ(Extractor.getAddressSize(), 4U);
+  Extractor.setAddressSize(8);
+  Expected List = Table.findList(Extractor, 12);
+  ASSERT_THAT_EXPECTED(List, Succeeded());
+  ASSERT_EQ(List->getEntries().size(), 2U);
+  EXPECT_EQ(List->getEntries()[0].Offset, 12 + 0U);
+  EXPECT_EQ(List->getEntries()[0].EntryKind, dwarf::DW_RLE_start_end);
+  EXPECT_EQ(List->getEntries()[0].Value0, 0x0001020304050607U);
+  EXPECT_EQ(List->getEntries()[0].Value1, 0x8081828384858687U);
+  EXPECT_EQ(List->getEntries()[1].Offset, 12 + 17U);
+  EXPECT_EQ(List->getEntries()[1].EntryKind, dwarf::DW_RLE_end_of_list);
+}
+
+TEST(DWARFListTableHeader, AddressSize32Header) {
+  static const char SecData[] =

[Lldb-commits] [PATCH] D107470: [llvm+lldb] Remove dead-code in DWARFListTableHeader::extract modifying DWARFDataExtractor

2021-08-16 Thread Jan Kratochvil via Phabricator via lldb-commits 
jankratochvil marked 3 inline comments as done.
jankratochvil added a comment.

In D107470#2946689 , @ikudrin wrote:

> `DWARFDebugInfo.TestRnglistsAddressSize`, 
> `DWARFListTableHeader.AddressSize64Offset`, and 
> `DWARFListTableHeader.AddressSize32Offset` pass without applying the patch. 
> Why adding them?

This code has shown it is prone to errors so future changes could break some 
other combination. Better code coverage by the testsuite is AFAIK always 
preferred.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107470

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

[Lldb-commits] [PATCH] D106466: [llvm+lldb] Fix#2 of DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

2021-08-16 Thread Jan Kratochvil via Phabricator via lldb-commits 
jankratochvil updated this revision to Diff 366676.
jankratochvil edited the summary of this revision.
jankratochvil added a comment.

@dblaikie do you think it is OK for check-in now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106466

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s


Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
===
--- lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
@@ -29,7 +29,7 @@
 # RUN:   -o exit 2>&1 | FileCheck --check-prefix=RNGLISTBASE %s
 
 # RNGLISTBASE-LABEL: image lookup -v -s lookup_rnglists
-# RNGLISTBASE: error: {{.*}}-rnglistbase {0x0043}: DIE has 
DW_AT_ranges(DW_FORM_rnglistx 0x0) attribute, but range extraction failed 
(invalid range list table index 0; OffsetEntryCount is 0, DW_AT_rnglists_base 
is 12), please file a bug and attach the file at the start of this error message
+# RNGLISTBASE: error: {{.*}}-rnglistbase {0x0043}: DIE has 
DW_AT_ranges(DW_FORM_rnglistx 0x0) attribute, but range extraction failed 
(invalid range list table index 0; OffsetEntryCount is 0, DW_AT_rnglists_base 
is 24), please file a bug and attach the file at the start of this error message
 
 .text
 rnglists:
@@ -97,7 +97,7 @@
 .long   .Lrnglists_end-rnglists # DW_AT_high_pc
 .long   .Laddr_table_base0  # DW_AT_addr_base
 .ifdef RNGLISTBASE
-.long   .Ldebug_ranges0 # DW_AT_rnglists_base
+.long   .Ldebug_ranges1 # DW_AT_rnglists_base
 .endif
 .byte   2   # Abbrev [2] 0x2b:0x37 
DW_TAG_subprogram
 .quad   rnglists# DW_AT_low_pc
@@ -105,7 +105,7 @@
 .asciz  "rnglists"  # DW_AT_name
 .byte   5   # Abbrev [5] 0x52:0xf 
DW_TAG_lexical_block
 .ifndef RNGLISTX
-.long   .Ldebug_ranges0 # DW_AT_ranges DW_FORM_sec_offset
+.long   .Ldebug_ranges1 # DW_AT_ranges DW_FORM_sec_offset
 .else
 .uleb128 0  # DW_AT_ranges DW_FORM_rnglistx
 .endif
@@ -130,9 +130,17 @@
 .byte   8   # Address size
 .byte   0   # Segment selector size
 .long   0   # Offset entry count
-.Ldebug_ranges0:
+.Ldebug_rnglist_table_end0:
+
+.long   .Ldebug_rnglist_table_end1-.Ldebug_rnglist_table_start1 # 
Length
+.Ldebug_rnglist_table_start1:
+.short  5   # Version
+.byte   8   # Address size
+.byte   0   # Segment selector size
+.long   0   # Offset entry count
+.Ldebug_ranges1:
 .byte   4   # DW_RLE_offset_pair
 .uleb128 .Lblock1_begin-rnglists  #   starting offset
 .uleb128 .Lblock1_end-rnglists#   ending offset
 .byte   0   # DW_RLE_end_of_list
-.Ldebug_rnglist_table_end0:
+.Ldebug_rnglist_table_end1:
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -437,15 +437,20 @@
   // We are expected to be called with Offset 0 or pointing just past the table
   // header. Correct Offset in the latter case so that it points to the start
   // of the header.
-  if (offset > 0) {
-uint64_t HeaderSize = llvm::DWARFListTableHeader::getHeaderSize(format);
-if (offset < HeaderSize)
-  return llvm::createStringError(errc::invalid_argument,
- "did not detect a valid"
- " list table with base = 0x%" PRIx64 "\n",
- offset);
-offset -= HeaderSize;
+  if (offset == 0) {
+// This means DW_AT_rnglists_base is missing and therefore DW_FORM_rnglistx
+// cannot be handled. Returning a default-constructed ListTableType allows
+// DW_FORM_sec_offset to be supported.
+return ListTableType();
   }
+
+  uint64_t HeaderSize = llvm::DWARFListTableHeader::getHeaderSize(format);
+  if (offset < HeaderSize)
+return llvm::createStringError(errc::invalid_argument,
+   "did not detect a valid"
+   " list table with base = 0x%" PRIx64 "\n",
+   offset);
+  offset -= HeaderSize;
   ListTableType Table;
   if (llvm::Error E = Table.extractHeaderAndOffsets(data, ))
 return std::move(E);
@@ -996,8 +1001,12 @@
 return llvm::createStringError(errc::invalid_argument,
"missing

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

2021-08-16 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Thanks for getting to this. I had a diff going a few weeks ago, but never got 
things fully working!




Comment at: lldb/include/lldb/Symbol/ObjectFile.h:465-479
+  /// Extracts the rendezvous structure address from r_debug.
+  ///
+  /// Especially in the case of running a program through the interpreter, the
+  /// GetImageInfoAddress is not able to extract the address. However, the
+  /// rendezvous address in that case can be extracted from the load address of
+  /// _r_debug symbol. This method returns the address of such a structure if
+  /// the information can be resolved via entries in the object file.

No need to add this to the ObjectFile class, just search for the symbol using 
the object file you already have in DYLDRendezvous.cpp. See comments down below.

We try to keep ObjectFile as an abstraction that sits on top of any object file 
type (ELF, mach-p, COFF), and this one is very specific to ELF. I would rather 
keep this out of the ObjectFile API, and we have an easy work around where we 
just search for the symbol by name in the module anyway. 



Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:59-60
   } else {
-LLDB_LOGF(log,
-  "%s FAILED - direct object file approach did not yield a "
-  "valid address",
-  __FUNCTION__);
+Address rendezvous_addr =
+obj_file->GetRendezvousStructureAddress(target);
+if (rendezvous_addr.IsValid())

Inline contents of ObjectFileELF::GetRendezvousStructureAddress(...) to here. 
We already have the object file we need, so we can just extract the symbol from 
here. 

I had started to try and fix this as well a few weeks ago, and I used 
Module::FindFirstSymbolWithNameAndType(...) to avoid having to get multiple 
results back:

```
const Symbol* _r_debug = 
target->GetExecutableModule()->FindFirstSymbolWithNameAndType(ConstString("_r_debug"));
if (_r_debug) {
  info_addr = _r_debug->GetAddress().GetLoadAddress(target);
  if (info_addr != LLDB_INVALID_ADDRESS) {
LLDB_LOGF(log,
  "%s resolved by finding symbol '_r_debug' whose value is 
0x%" PRIx64,
__FUNCTION__, info_addr);
return info_addr;
  }
}
```



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:885-899
+Address ObjectFileELF::GetRendezvousStructureAddress(Target *target) {
+  ModuleSP module_sp(GetModule());
+  if (module_sp) {
+SymbolContextList contexts;
+SymbolContext context;
+module_sp->FindSymbolsWithNameAndType(ConstString("_r_debug"),
+  eSymbolTypeAny, contexts);

Revert and move contents to where this was called in DYLDRendezvous.cpp. It is 
very easy to have the DYLDRendezvous just search for the symbol itself.



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h:134-136
+  lldb_private::Address
+  GetRendezvousStructureAddress(lldb_private::Target *target) override;
+

Revert


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

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

[Lldb-commits] [PATCH] D108148: [lldb] [gdb-remote] Use standardized GDB errno values

2021-08-16 Thread Michał Górny via Phabricator via lldb-commits 
mgorny created this revision.
mgorny added reviewers: krytarowski, emaste, JDevlieghere, labath, jasonmolenda.
mgorny requested review of this revision.

GDB uses normalized errno values for vFile errors.  Implement
the translation between them and system errno values in the gdb-remote
plugin.


https://reviews.llvm.org/D108148

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -15,7 +15,7 @@
 return "Fa"
 elif packet.startswith("vFile:close:"):
 return "F0"
-return "F-1,16"
+return "F-1,58"
 
 self.server.responder = Responder()
 
@@ -47,7 +47,9 @@
 
 class Responder(MockGDBServerResponder):
 def vFile(self, packet):
-return "F-1,16"
+# use ENOSYS as this constant differs between GDB Remote
+# Protocol and Linux, so we can test the translation
+return "F-1,58"
 
 self.server.responder = Responder()
 
@@ -58,16 +60,16 @@
 self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
 
 self.match("platform file open /some/file.txt -v 0755",
-   [r"error: Invalid argument"],
+   [r"error: Function not implemented"],
error=True)
 self.match("platform file read 16 -o 11 -c 13",
-   [r"error: Invalid argument"],
+   [r"error: Function not implemented"],
error=True)
 self.match("platform file write 16 -o 11 -d teststring",
-   [r"error: Invalid argument"],
+   [r"error: Function not implemented"],
error=True)
 self.match("platform file close 16",
-   [r"error: Invalid argument"],
+   [r"error: Function not implemented"],
error=True)
 self.assertPacketLogContains([
 "vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -535,6 +535,55 @@
   return SendErrorResponse(18);
 }
 
+static GDBErrno system_errno_to_gdb(int err) {
+  switch (err) {
+  case EPERM:
+return GDB_EPERM;
+  case ENOENT:
+return GDB_ENOENT;
+  case EINTR:
+return GDB_EINTR;
+  case EIO:
+return GDB_EIO;
+  case EBADF:
+return GDB_EBADF;
+  case EACCES:
+return GDB_EACCES;
+  case EFAULT:
+return GDB_EFAULT;
+  case EBUSY:
+return GDB_EBUSY;
+  case EEXIST:
+return GDB_EEXIST;
+  case ENODEV:
+return GDB_ENODEV;
+  case ENOTDIR:
+return GDB_ENOTDIR;
+  case EISDIR:
+return GDB_EISDIR;
+  case EINVAL:
+return GDB_EINVAL;
+  case ENFILE:
+return GDB_ENFILE;
+  case EMFILE:
+return GDB_EMFILE;
+  case EFBIG:
+return GDB_EFBIG;
+  case ENOSPC:
+return GDB_ENOSPC;
+  case ESPIPE:
+return GDB_ESPIPE;
+  case EROFS:
+return GDB_EROFS;
+  case ENOSYS:
+return GDB_ENOSYS;
+  case ENAMETOOLONG:
+return GDB_ENAMETOOLONG;
+  default:
+return GDB_EUNKNOWN;
+  }
+}
+
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_Close(
 StringExtractorGDBRemote ) {
@@ -554,7 +603,7 @@
   response.PutChar('F');
   response.Printf("%x", err);
   if (save_errno)
-response.Printf(",%x", save_errno);
+response.Printf(",%x", system_errno_to_gdb(save_errno));
   return SendPacketNoLock(response.GetString());
 }
 
@@ -585,7 +634,7 @@
   } else {
 response.PutCString("-1");
 if (save_errno)
-  response.Printf(",%x", save_errno);
+  response.Printf(",%x", system_errno_to_gdb(save_errno));
   }
   return SendPacketNoLock(response.GetString());
 }
@@ -617,7 +666,7 @@
 else {
   response.PutCString("-1");
   if (save_errno)
-response.Printf(",%x", save_errno);
+response.Printf(",%x", system_errno_to_gdb(save_errno));
 }
   } else {
 response.Printf("-1,%x", EINVAL);
@@ -769,7 +818,7 @@
   struct stat file_stats;
   if (::fstat(fd,

[Lldb-commits] [PATCH] D108121: [lldb] Add tests for TypeSystemMap::GetTypeSystemForLanguage

2021-08-16 Thread Raphael Isemann via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf521e78dfd2: [lldb] Add tests for 
TypeSystemMap::GetTypeSystemForLanguage (authored by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108121

Files:
  lldb/unittests/Symbol/CMakeLists.txt
  lldb/unittests/Symbol/TestTypeSystem.cpp

Index: lldb/unittests/Symbol/TestTypeSystem.cpp
===
--- /dev/null
+++ lldb/unittests/Symbol/TestTypeSystem.cpp
@@ -0,0 +1,92 @@
+//===-- TestTypeSystem.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
+//
+//===--===//
+
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/TypeSystem.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class TestTypeSystemMap : public testing::Test {
+public:
+  SubsystemRAII subsystems;
+};
+
+TEST_F(TestTypeSystemMap, GetTypeSystemForLanguageWithInvalidModule) {
+  // GetTypeSystemForLanguage called with an invalid Module.
+  TypeSystemMap map;
+  Module module{ModuleSpec()};
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeUnknown, ,
+   /*can_create=*/true),
+  llvm::FailedWithMessage("TypeSystem for language unknown doesn't exist"));
+
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeUnknown, ,
+   /*can_create=*/false),
+  llvm::FailedWithMessage("TypeSystem for language unknown doesn't exist"));
+
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeC, ,
+   /*can_create=*/true),
+  llvm::FailedWithMessage("TypeSystem for language c doesn't exist"));
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeC, ,
+   /*can_create=*/false),
+  llvm::FailedWithMessage("TypeSystem for language c doesn't exist"));
+}
+
+TEST_F(TestTypeSystemMap, GetTypeSystemForLanguageWithNoModule) {
+  // GetTypeSystemForLanguage called with no Module.
+  TypeSystemMap map;
+  Module *module = nullptr;
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeUnknown, module,
+   /*can_create=*/true),
+  llvm::FailedWithMessage("TypeSystem for language unknown doesn't exist"));
+
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeUnknown, module,
+   /*can_create=*/false),
+  llvm::FailedWithMessage("TypeSystem for language unknown doesn't exist"));
+
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeC, module, /*can_create=*/true),
+  llvm::FailedWithMessage("TypeSystem for language c doesn't exist"));
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeC, module,
+   /*can_create=*/false),
+  llvm::FailedWithMessage("TypeSystem for language c doesn't exist"));
+}
+
+TEST_F(TestTypeSystemMap, GetTypeSystemForLanguageWithNoTarget) {
+  // GetTypeSystemForLanguage called with no Target.
+  TypeSystemMap map;
+  Target *target = nullptr;
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeUnknown, target,
+   /*can_create=*/true),
+  llvm::FailedWithMessage("TypeSystem for language unknown doesn't exist"));
+
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeUnknown, target,
+   /*can_create=*/false),
+  llvm::FailedWithMessage("TypeSystem for language unknown doesn't exist"));
+
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeC, target, /*can_create=*/true),
+  llvm::FailedWithMessage("TypeSystem for language c doesn't exist"));
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeC, target,
+   /*can_create=*/false),
+  llvm::FailedWithMessage("TypeSystem for language c doesn't exist"));
+}
Index: lldb/unittests/Symbol/CMakeLists.txt
===
--- lldb/unittests/Symbol/CMakeLists.txt
+++ lldb/unittests/Symbol/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(SymbolTests
   LocateSymbolFileTest.cpp
   PostfixExpressionTest.cpp
+  TestTypeSystem.cpp
   TestTypeSystemClang.cpp
   TestClangASTImporter.cpp

[Lldb-commits] [lldb] cf521e7 - [lldb] Add tests for TypeSystemMap::GetTypeSystemForLanguage

2021-08-16 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2021-08-16T18:00:01+02:00
New Revision: cf521e78dfd2a418970358e5d55dcc9a862664e5

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

LOG: [lldb] Add tests for TypeSystemMap::GetTypeSystemForLanguage

Regression tests for D108088 .

Reviewed By: mib

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

Added: 
lldb/unittests/Symbol/TestTypeSystem.cpp

Modified: 
lldb/unittests/Symbol/CMakeLists.txt

Removed: 




diff  --git a/lldb/unittests/Symbol/CMakeLists.txt 
b/lldb/unittests/Symbol/CMakeLists.txt
index 76c7b645f277a..748faf33b556b 100644
--- a/lldb/unittests/Symbol/CMakeLists.txt
+++ b/lldb/unittests/Symbol/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(SymbolTests
   LocateSymbolFileTest.cpp
   PostfixExpressionTest.cpp
+  TestTypeSystem.cpp
   TestTypeSystemClang.cpp
   TestClangASTImporter.cpp
   TestDWARFCallFrameInfo.cpp

diff  --git a/lldb/unittests/Symbol/TestTypeSystem.cpp 
b/lldb/unittests/Symbol/TestTypeSystem.cpp
new file mode 100644
index 0..59297a7475d27
--- /dev/null
+++ b/lldb/unittests/Symbol/TestTypeSystem.cpp
@@ -0,0 +1,92 @@
+//===-- TestTypeSystem.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
+//
+//===--===//
+
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/TypeSystem.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class TestTypeSystemMap : public testing::Test {
+public:
+  SubsystemRAII subsystems;
+};
+
+TEST_F(TestTypeSystemMap, GetTypeSystemForLanguageWithInvalidModule) {
+  // GetTypeSystemForLanguage called with an invalid Module.
+  TypeSystemMap map;
+  Module module{ModuleSpec()};
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeUnknown, ,
+   /*can_create=*/true),
+  llvm::FailedWithMessage("TypeSystem for language unknown doesn't 
exist"));
+
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeUnknown, ,
+   /*can_create=*/false),
+  llvm::FailedWithMessage("TypeSystem for language unknown doesn't 
exist"));
+
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeC, ,
+   /*can_create=*/true),
+  llvm::FailedWithMessage("TypeSystem for language c doesn't exist"));
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeC, ,
+   /*can_create=*/false),
+  llvm::FailedWithMessage("TypeSystem for language c doesn't exist"));
+}
+
+TEST_F(TestTypeSystemMap, GetTypeSystemForLanguageWithNoModule) {
+  // GetTypeSystemForLanguage called with no Module.
+  TypeSystemMap map;
+  Module *module = nullptr;
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeUnknown, module,
+   /*can_create=*/true),
+  llvm::FailedWithMessage("TypeSystem for language unknown doesn't 
exist"));
+
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeUnknown, module,
+   /*can_create=*/false),
+  llvm::FailedWithMessage("TypeSystem for language unknown doesn't 
exist"));
+
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeC, module, 
/*can_create=*/true),
+  llvm::FailedWithMessage("TypeSystem for language c doesn't exist"));
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeC, module,
+   /*can_create=*/false),
+  llvm::FailedWithMessage("TypeSystem for language c doesn't exist"));
+}
+
+TEST_F(TestTypeSystemMap, GetTypeSystemForLanguageWithNoTarget) {
+  // GetTypeSystemForLanguage called with no Target.
+  TypeSystemMap map;
+  Target *target = nullptr;
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeUnknown, target,
+   /*can_create=*/true),
+  llvm::FailedWithMessage("TypeSystem for language unknown doesn't 
exist"));
+
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeUnknown, target,
+   /*can_create=*/false),
+  llvm::FailedWithMessage("TypeSystem for language unknown doesn't 
exist"));
+
+  EXPECT_THAT_EXPECTED(
+  map.GetTypeSystemForLanguage(eLanguageTypeC, target, 
/*can_create=*/true),
+  llvm::FailedWithMessage("TypeSystem for

[Lldb-commits] [PATCH] D108088: [lldb] Avoid unhandled Error in TypeSystemMap::GetTypeSystemForLanguage

2021-08-16 Thread Dimitry Andric via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5033f0793fe6: [lldb] Avoid unhandled Error in 
TypeSystemMap::GetTypeSystemForLanguage (authored by dim).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108088

Files:
  lldb/source/Symbol/TypeSystem.cpp

Index: lldb/source/Symbol/TypeSystem.cpp
===
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -223,62 +223,32 @@
 llvm::Expected TypeSystemMap::GetTypeSystemForLanguage(
 lldb::LanguageType language,
 llvm::Optional create_callback) {
-  llvm::Error error = llvm::Error::success();
-  assert(!error); // Check the success value when assertions are enabled
   std::lock_guard guard(m_mutex);
-  if (m_clear_in_progress) {
-error = llvm::make_error(
+  if (m_clear_in_progress)
+return llvm::make_error(
 "Unable to get TypeSystem because TypeSystemMap is being cleared",
 llvm::inconvertibleErrorCode());
-  } else {
-collection::iterator pos = m_map.find(language);
-if (pos != m_map.end()) {
-  auto *type_system = pos->second.get();
-  if (type_system) {
-llvm::consumeError(std::move(error));
-return *type_system;
-  }
-  error = llvm::make_error(
-  "TypeSystem for language " +
-  llvm::StringRef(Language::GetNameForLanguageType(language)) +
-  " doesn't exist",
-  llvm::inconvertibleErrorCode());
-  return std::move(error);
-}
 
-for (const auto  : m_map) {
-  if (pair.second && pair.second->SupportsLanguage(language)) {
-// Add a new mapping for "language" to point to an already existing
-// TypeSystem that supports this language
-m_map[language] = pair.second;
-if (pair.second.get()) {
-  llvm::consumeError(std::move(error));
-  return *pair.second.get();
-}
-error = llvm::make_error(
-"TypeSystem for language " +
-llvm::StringRef(Language::GetNameForLanguageType(language)) +
-" doesn't exist",
-llvm::inconvertibleErrorCode());
-return std::move(error);
-  }
-}
+  collection::iterator pos = m_map.find(language);
+  if (pos != m_map.end()) {
+auto *type_system = pos->second.get();
+if (type_system)
+  return *type_system;
+return llvm::make_error(
+"TypeSystem for language " +
+llvm::StringRef(Language::GetNameForLanguageType(language)) +
+" doesn't exist",
+llvm::inconvertibleErrorCode());
+  }
 
-if (!create_callback) {
-  error = llvm::make_error(
-  "Unable to find type system for language " +
-  llvm::StringRef(Language::GetNameForLanguageType(language)),
-  llvm::inconvertibleErrorCode());
-} else {
-  // Cache even if we get a shared pointer that contains a null type system
-  // back
-  TypeSystemSP type_system_sp = (*create_callback)();
-  m_map[language] = type_system_sp;
-  if (type_system_sp.get()) {
-llvm::consumeError(std::move(error));
-return *type_system_sp.get();
-  }
-  error = llvm::make_error(
+  for (const auto  : m_map) {
+if (pair.second && pair.second->SupportsLanguage(language)) {
+  // Add a new mapping for "language" to point to an already existing
+  // TypeSystem that supports this language
+  m_map[language] = pair.second;
+  if (pair.second.get())
+return *pair.second.get();
+  return llvm::make_error(
   "TypeSystem for language " +
   llvm::StringRef(Language::GetNameForLanguageType(language)) +
   " doesn't exist",
@@ -286,7 +256,23 @@
 }
   }
 
-  return std::move(error);
+  if (!create_callback)
+return llvm::make_error(
+"Unable to find type system for language " +
+llvm::StringRef(Language::GetNameForLanguageType(language)),
+llvm::inconvertibleErrorCode());
+
+  // Cache even if we get a shared pointer that contains a null type system
+  // back
+  TypeSystemSP type_system_sp = (*create_callback)();
+  m_map[language] = type_system_sp;
+  if (type_system_sp.get())
+return *type_system_sp.get();
+  return llvm::make_error(
+  "TypeSystem for language " +
+  llvm::StringRef(Language::GetNameForLanguageType(language)) +
+  " doesn't exist",
+  llvm::inconvertibleErrorCode());
 }
 
 llvm::Expected
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5033f07 - [lldb] Avoid unhandled Error in TypeSystemMap::GetTypeSystemForLanguage

2021-08-16 Thread Dimitry Andric via lldb-commits 

Author: Dimitry Andric
Date: 2021-08-16T17:34:42+02:00
New Revision: 5033f0793fe6e8b36990e6ce9f9ec2bf2ff79923

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

LOG: [lldb] Avoid unhandled Error in TypeSystemMap::GetTypeSystemForLanguage

When assertions are turned off, the `llvm::Error` value created at the
start of this function is overwritten using the move-assignment
operator, but the success value is never checked. Whenever a TypeSystem
cannot be found or created, this can lead to lldb core dumping with:

Program aborted due to an unhandled Error:
Error value was Success. (Note: Success values must still be checked prior 
to being destroyed).

Fix this by not creating a `llvm::Error` value in advance, and directly
returning the result of `llvm::make_error` instead, whenever an error is
encountered.

See also:  and
.

Reviewed By: teemperor

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

Added: 


Modified: 
lldb/source/Symbol/TypeSystem.cpp

Removed: 




diff  --git a/lldb/source/Symbol/TypeSystem.cpp 
b/lldb/source/Symbol/TypeSystem.cpp
index 252b06e269d6b..0b3f7e4f3bd4e 100644
--- a/lldb/source/Symbol/TypeSystem.cpp
+++ b/lldb/source/Symbol/TypeSystem.cpp
@@ -223,62 +223,32 @@ void TypeSystemMap::ForEach(std::function const ) {
 llvm::Expected TypeSystemMap::GetTypeSystemForLanguage(
 lldb::LanguageType language,
 llvm::Optional create_callback) {
-  llvm::Error error = llvm::Error::success();
-  assert(!error); // Check the success value when assertions are enabled
   std::lock_guard guard(m_mutex);
-  if (m_clear_in_progress) {
-error = llvm::make_error(
+  if (m_clear_in_progress)
+return llvm::make_error(
 "Unable to get TypeSystem because TypeSystemMap is being cleared",
 llvm::inconvertibleErrorCode());
-  } else {
-collection::iterator pos = m_map.find(language);
-if (pos != m_map.end()) {
-  auto *type_system = pos->second.get();
-  if (type_system) {
-llvm::consumeError(std::move(error));
-return *type_system;
-  }
-  error = llvm::make_error(
-  "TypeSystem for language " +
-  llvm::StringRef(Language::GetNameForLanguageType(language)) +
-  " doesn't exist",
-  llvm::inconvertibleErrorCode());
-  return std::move(error);
-}
 
-for (const auto  : m_map) {
-  if (pair.second && pair.second->SupportsLanguage(language)) {
-// Add a new mapping for "language" to point to an already existing
-// TypeSystem that supports this language
-m_map[language] = pair.second;
-if (pair.second.get()) {
-  llvm::consumeError(std::move(error));
-  return *pair.second.get();
-}
-error = llvm::make_error(
-"TypeSystem for language " +
-llvm::StringRef(Language::GetNameForLanguageType(language)) +
-" doesn't exist",
-llvm::inconvertibleErrorCode());
-return std::move(error);
-  }
-}
+  collection::iterator pos = m_map.find(language);
+  if (pos != m_map.end()) {
+auto *type_system = pos->second.get();
+if (type_system)
+  return *type_system;
+return llvm::make_error(
+"TypeSystem for language " +
+llvm::StringRef(Language::GetNameForLanguageType(language)) +
+" doesn't exist",
+llvm::inconvertibleErrorCode());
+  }
 
-if (!create_callback) {
-  error = llvm::make_error(
-  "Unable to find type system for language " +
-  llvm::StringRef(Language::GetNameForLanguageType(language)),
-  llvm::inconvertibleErrorCode());
-} else {
-  // Cache even if we get a shared pointer that contains a null type system
-  // back
-  TypeSystemSP type_system_sp = (*create_callback)();
-  m_map[language] = type_system_sp;
-  if (type_system_sp.get()) {
-llvm::consumeError(std::move(error));
-return *type_system_sp.get();
-  }
-  error = llvm::make_error(
+  for (const auto  : m_map) {
+if (pair.second && pair.second->SupportsLanguage(language)) {
+  // Add a new mapping for "language" to point to an already existing
+  // TypeSystem that supports this language
+  m_map[language] = pair.second;
+  if (pair.second.get())
+return *pair.second.get();
+  return llvm::make_error(
   "TypeSystem for language " +
   llvm::StringRef(Language::GetNameForLanguageType(language)) +
   " doesn't exist",
@@ -286,7 +256,23 @@ llvm::Expected 
TypeSystemMap::GetTypeSystemForLanguage(
 }
   }
 
-  return std::move(error);
+  if (!create_callback)
+return llvm::make_error(

[Lldb-commits] [PATCH] D108078: [lldb] Support gdbserver signals

2021-08-16 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 366621.
mgorny retitled this revision from "[lldb] Support gdbserver signals [WIP]" to 
"[lldb] Support gdbserver signals".
mgorny edited the summary of this revision.
mgorny added a comment.

Included all signals from `include/gdb/signals.def`.


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

https://reviews.llvm.org/D108078

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/Utility/GDBRemoteSignals.cpp
  lldb/source/Plugins/Process/Utility/GDBRemoteSignals.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -351,3 +351,49 @@
   "QEnvironmentHexEncoded:4e45454453454e43343d6623726f62",
   "QEnvironmentHexEncoded:455155414c533d666f6f3d626172",
 ])
+
+def test_signal_gdb(self):
+class MyResponder(MockGDBServerResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+"
+
+def haltReason(self):
+return "S0a"
+
+def cont(self):
+return self.haltReason()
+
+self.server.responder = MyResponder()
+
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+
+self.assertEqual(process.threads[0].GetStopReason(),
+ lldb.eStopReasonSignal)
+self.assertEqual(process.threads[0].GetStopDescription(100),
+ 'signal SIGBUS')
+
+def test_signal_lldb(self):
+class MyResponder(MockGDBServerResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+;native-signals+"
+
+def qHostInfo(self):
+return "triple:61726d76372d756e6b6e6f776e2d6c696e75782d676e75;"
+
+def haltReason(self):
+return "S0a"
+
+def cont(self):
+return self.haltReason()
+
+self.server.responder = MyResponder()
+
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+
+self.assertEqual(process.threads[0].GetStopReason(),
+ lldb.eStopReasonSignal)
+# NB: this may need adjusting per current platform
+self.assertEqual(process.threads[0].GetStopDescription(100),
+ 'signal SIGUSR1')
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -719,14 +719,6 @@
 __FUNCTION__, GetID(),
 GetTarget().GetArchitecture().GetTriple().getTriple().c_str());
 
-  if (error.Success()) {
-PlatformSP platform_sp = GetTarget().GetPlatform();
-if (platform_sp && platform_sp->IsConnected())
-  SetUnixSignals(platform_sp->GetUnixSignals());
-else
-  SetUnixSignals(UnixSignals::Create(GetTarget().GetArchitecture()));
-  }
-
   return error;
 }
 
@@ -,6 +1103,18 @@
   if (StructuredData::Array *supported_packets =
   m_gdb_comm.GetSupportedStructuredDataPlugins())
 MapSupportedStructuredDataPlugins(*supported_packets);
+
+  // If connected to LLDB ("native-signals+"), use signal defs for
+  // the remote platform.  If connected to GDB, just use the standard set.
+  if (!m_gdb_comm.UsesNativeSignals()) {
+SetUnixSignals(std::make_shared());
+  } else {
+PlatformSP platform_sp = GetTarget().GetPlatform();
+if (platform_sp && platform_sp->IsConnected())
+  SetUnixSignals(platform_sp->GetUnixSignals());
+else
+  SetUnixSignals(UnixSignals::Create(GetTarget().GetArchitecture()));
+  }
 }
 
 void ProcessGDBRemote::MaybeLoadExecutableModule() {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -1341,5 +1341,6 @@
   llvm::formatv("PacketSize={0}", max_packet_size),
   "QStartNoAckMode+",
   "qEcho+",
+  "native-signals+",
   };
 }
Index:

[Lldb-commits] [PATCH] D107470: [llvm+lldb] Remove dead-code in DWARFListTableHeader::extract modifying DWARFDataExtractor

2021-08-16 Thread Igor Kudrin via Phabricator via lldb-commits 
ikudrin added a comment.

`DWARFDebugInfo.TestRnglistsAddressSize`, 
`DWARFListTableHeader.AddressSize64Offset`, and 
`DWARFListTableHeader.AddressSize32Offset` pass without applying the patch. Why 
adding them?




Comment at: llvm/unittests/DebugInfo/DWARF/DWARFListTableTest.cpp:123
+  llvm::Error E = Table.extractHeaderAndOffsets(Extractor, );
+  EXPECT_FALSE(!!E);
+  EXPECT_EQ(Offset, 12U);

The rest of the test makes no sense if `extractHeaderAndOffsets()` returns an 
error.



Comment at: llvm/unittests/DebugInfo/DWARF/DWARFListTableTest.cpp:129-130
+  Expected List = Table.findList(Extractor, Offset);
+  EXPECT_TRUE(!!List);
+  EXPECT_EQ(List->getEntries().size(), 2U);
+  EXPECT_EQ(List->getEntries()[0].Offset, 12 + 0U);

If these checks fail, the execution of the test should be terminated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107470

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

[Lldb-commits] [PATCH] D108088: [lldb] Avoid unhandled Error in TypeSystemMap::GetTypeSystemForLanguage

2021-08-16 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

Thanks for fixing this (and cleaning up the code), LGTM. I wrote some 
regression tests for this in D108121  that 
I'll land as a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108088

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

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

2021-08-16 Thread Pedro Tammela via Phabricator via lldb-commits 
tammela requested changes to this revision.
tammela added a comment.
This revision now requires changes to proceed.

Missing test cases!
Either a script that test it all, individual unit tests or a combination of 
both.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

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

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

2021-08-16 Thread Pedro Tammela via Phabricator via lldb-commits 
tammela added inline comments.



Comment at: lldb/bindings/lua/lua-typemaps.swig:198
+  size_t size = lua_rawlen(L, $input);
+  $1 = (char **)malloc((size + 1) * sizeof(char *));
+  int i = 0, j = 0;

This seems it could leak.
Are you sure it doesn't? If so add a comment here explaining where it's freed.



Comment at: lldb/bindings/lua/lua-typemaps.swig:221
+   luaL_Stream *p = (luaL_Stream *)luaL_checkudata(L, $input, LUA_FILEHANDLE);
+   FileSP file_sp;
+   file_sp = std::make_shared(p->f, false);







Comment at: lldb/bindings/lua/lua-typemaps.swig:222
+   FileSP file_sp;
+   file_sp = std::make_shared(p->f, false);
+   if (!file_sp->IsValid())





Comment at: lldb/bindings/lua/lua-typemaps.swig:229
+%typecheck(SWIG_TYPECHECK_POINTER) lldb::FileSP {
+   $1 = lua_isuserdata(L, $input) ? 1 : 0;
+}





Comment at: lldb/bindings/lua/lua-typemaps.swig:249
+
+%typemap(in) (uint64_t* array, size_t array_len),
+ (uint32_t* array, size_t array_len),

The raw variants here are overkill, you can the use normal ones



Comment at: lldb/bindings/lua/lua-typemaps.swig:256
+  $2 = lua_rawlen(L, $input);
+  $1 = ($1_ltype)malloc(($2) * sizeof($*1_type));
+  int i = 0, j = 0;

Leaking?



Comment at: lldb/bindings/lua/lua-typemaps.swig:266
+  $2 = 0;
+   }
+}

else Lua error?



Comment at: lldb/bindings/lua/lua-typemaps.swig:284
+//===--===//
\ No newline at end of file


Please fix this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

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

[Lldb-commits] [PATCH] D106466: [llvm+lldb] Fix#2 of DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

2021-08-16 Thread Igor Kudrin via Phabricator via lldb-commits 
ikudrin added a comment.

The code looks good, but please improve the comments and wait for approval from 
a more LLDB-knowledgeable person than me,




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:441
+  if (offset == 0) {
+// Caller must not use this default initializater for GetRnglistOffset.
+return ListTableType();

Please extend the comment to emphasize that even if `DW_AT_rnglists_base` is 
missing and `DW_FORM_rnglistx` cannot be handled, returning a 
default-constructed Table allows `DW_FORM_sec_offset` to be supported.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:1005
+
+  // As \a offset can be zero we need to call setAddressSize.
+  data.setAddressSize(m_header.GetAddressByteSize());

This comment is quite misleading as it references `offset` which is the 
argument of the method.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106466

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