[Lldb-commits] [lldb] 7fce3b2 - [lldb][docs] Remove -webkit-hyphens in table cells so that table widths are correct on Safari

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-01-20T09:05:28+01:00
New Revision: 7fce3b240b6b313b1becf19ddf3f2a904c34ced2

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

LOG: [lldb][docs] Remove -webkit-hyphens in table cells so that table widths 
are correct on Safari

The tables in the new LLDB documentation currently are less wide than their
contents. The reason for that seems to be the `-webkit-hyphens: auto` property
that sphinx is setting for all `p` tags. The `p` tags in the generated Python
documentation seem to trigger some Safari layout issue, so Safari is calculating
the cell width to be smaller than it should be (which ends up looking like this
{F15104344} ).

This patch just sets that property back to the browser default `manual`. Not
sure if that's the proper workaround, but I clicked around on the website with
the changed CSS and nothing looked funny (which is I believe how webdev unit
testing works).

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/docs/_static/lldb.css

Removed: 




diff  --git a/lldb/docs/_static/lldb.css b/lldb/docs/_static/lldb.css
index 53b725940831..6df20b7dd6e4 100644
--- a/lldb/docs/_static/lldb.css
+++ b/lldb/docs/_static/lldb.css
@@ -44,6 +44,13 @@ table.mapping td.content {
   padding-bottom: 15px;
 }
 
+/* Workaround for a Safari bug that would otherwise make table cells less wide
+than the containing text. This just sets it back to the default browser
+property.*/
+td p {
+  -webkit-hyphens: manual !important;
+}
+
 div.sphinxsidebar .caption {
   font-family: Helvetica, Verdana, sans-serif;
   font-size: 10pt;



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

[Lldb-commits] [PATCH] D94991: [lldb][docs] Remove -webkit-hyphens in table cells so that table widths are correct on Safari

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7fce3b240b6b: [lldb][docs] Remove -webkit-hyphens in table 
cells so that table widths are… (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/D94991/new/

https://reviews.llvm.org/D94991

Files:
  lldb/docs/_static/lldb.css


Index: lldb/docs/_static/lldb.css
===
--- lldb/docs/_static/lldb.css
+++ lldb/docs/_static/lldb.css
@@ -44,6 +44,13 @@
   padding-bottom: 15px;
 }
 
+/* Workaround for a Safari bug that would otherwise make table cells less wide
+than the containing text. This just sets it back to the default browser
+property.*/
+td p {
+  -webkit-hyphens: manual !important;
+}
+
 div.sphinxsidebar .caption {
   font-family: Helvetica, Verdana, sans-serif;
   font-size: 10pt;


Index: lldb/docs/_static/lldb.css
===
--- lldb/docs/_static/lldb.css
+++ lldb/docs/_static/lldb.css
@@ -44,6 +44,13 @@
   padding-bottom: 15px;
 }
 
+/* Workaround for a Safari bug that would otherwise make table cells less wide
+than the containing text. This just sets it back to the default browser
+property.*/
+td p {
+  -webkit-hyphens: manual !important;
+}
+
 div.sphinxsidebar .caption {
   font-family: Helvetica, Verdana, sans-serif;
   font-size: 10pt;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 3c69ff4 - [lldb][docs] Filter out 'thisown' attribute and inheritance boilerplate

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-01-20T09:07:36+01:00
New Revision: 3c69ff4b03abaa3b7b80f4f3f2a1c1806e2d4495

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

LOG: [lldb][docs] Filter out 'thisown' attribute and inheritance boilerplate

This patch implements a filter that post-processes some of the generated RST 
sources
of the Python API docs. I mainly want to avoid two things:

1. Filter out all the inheritance boilerplate that just keeps mentioning for
every class that it inherits from the builtin 'object'. There is no inheritance
in the SB API.

2. More importantly, removes the SWIG generated `thisown` attribute from the
public documentation. I don't think we want users to mess with that attribute
and this is probably causing more confusion than it would help anyone. It also
makes the documentation for some smaller classes more verbose than necessary.

This patch just uses the sphinx event for reading source and removes the parts
that we don't want in documentation.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/docs/conf.py

Removed: 




diff  --git a/lldb/docs/conf.py b/lldb/docs/conf.py
index 2c7cd5d94c1f..b9b94672cbde 100644
--- a/lldb/docs/conf.py
+++ b/lldb/docs/conf.py
@@ -298,3 +298,36 @@ def process_md(name):
 
 # How to display URL addresses: 'footnote', 'no', or 'inline'.
 #texinfo_show_urls = 'footnote'
+
+empty_attr_summary = re.compile(r'\.\. rubric:: Attributes Summary\s*\.\. 
autosummary::\s*\.\. rubric::')
+empty_attr_documentation = re.compile(r'\.\. rubric:: Attributes 
Documentation\s*\.\. rubric::')
+
+def cleanup_source(app, docname, source):
+""" Cleans up source files generated by automodapi. """
+# Don't cleanup anything beside automodapi-generated sources.
+if not automodapi_toctreedirnm in docname:
+  return
+processed = source[0]
+
+# Don't show the list of inheritance info as there is no inheritance in the
+# SBI API. This avoids all the repeated text on all doc pages that a
+# class inherits from 'object'.
+
+processed = processed.replace(":show-inheritance:", "")
+# Remove the SWIG generated 'thisown' attribute. It just bloats the 
generated
+# documentation and users shouldn't fiddle with the value anyway.
+processed = re.sub(r'~SB[a-zA-Z]+\.thisown', "", processed)
+processed = processed.replace("  .. autoattribute:: thisown", "")
+
+# After removing 'thisown', many objects don't have any attributes left.
+# Remove all now empty attribute summary/documentation sections with
+# some rather ugly regex.
+processed = empty_attr_summary.sub('.. rubric::', processed)
+processed = empty_attr_documentation.sub('.. rubric::', processed)
+
+# Replace the original source with the processed one (source is a single
+# element list).
+source[0] = processed
+
+def setup(app):
+app.connect('source-read', cleanup_source)



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

[Lldb-commits] [PATCH] D94967: [lldb][docs] Filter out 'thisown' attribute and inheritance boilerplate

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c69ff4b03ab: [lldb][docs] Filter out 'thisown' 
attribute and inheritance boilerplate (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/D94967/new/

https://reviews.llvm.org/D94967

Files:
  lldb/docs/conf.py


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -298,3 +298,36 @@
 
 # How to display URL addresses: 'footnote', 'no', or 'inline'.
 #texinfo_show_urls = 'footnote'
+
+empty_attr_summary = re.compile(r'\.\. rubric:: Attributes Summary\s*\.\. 
autosummary::\s*\.\. rubric::')
+empty_attr_documentation = re.compile(r'\.\. rubric:: Attributes 
Documentation\s*\.\. rubric::')
+
+def cleanup_source(app, docname, source):
+""" Cleans up source files generated by automodapi. """
+# Don't cleanup anything beside automodapi-generated sources.
+if not automodapi_toctreedirnm in docname:
+  return
+processed = source[0]
+
+# Don't show the list of inheritance info as there is no inheritance in the
+# SBI API. This avoids all the repeated text on all doc pages that a
+# class inherits from 'object'.
+
+processed = processed.replace(":show-inheritance:", "")
+# Remove the SWIG generated 'thisown' attribute. It just bloats the 
generated
+# documentation and users shouldn't fiddle with the value anyway.
+processed = re.sub(r'~SB[a-zA-Z]+\.thisown', "", processed)
+processed = processed.replace("  .. autoattribute:: thisown", "")
+
+# After removing 'thisown', many objects don't have any attributes left.
+# Remove all now empty attribute summary/documentation sections with
+# some rather ugly regex.
+processed = empty_attr_summary.sub('.. rubric::', processed)
+processed = empty_attr_documentation.sub('.. rubric::', processed)
+
+# Replace the original source with the processed one (source is a single
+# element list).
+source[0] = processed
+
+def setup(app):
+app.connect('source-read', cleanup_source)


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -298,3 +298,36 @@
 
 # How to display URL addresses: 'footnote', 'no', or 'inline'.
 #texinfo_show_urls = 'footnote'
+
+empty_attr_summary = re.compile(r'\.\. rubric:: Attributes Summary\s*\.\. autosummary::\s*\.\. rubric::')
+empty_attr_documentation = re.compile(r'\.\. rubric:: Attributes Documentation\s*\.\. rubric::')
+
+def cleanup_source(app, docname, source):
+""" Cleans up source files generated by automodapi. """
+# Don't cleanup anything beside automodapi-generated sources.
+if not automodapi_toctreedirnm in docname:
+  return
+processed = source[0]
+
+# Don't show the list of inheritance info as there is no inheritance in the
+# SBI API. This avoids all the repeated text on all doc pages that a
+# class inherits from 'object'.
+
+processed = processed.replace(":show-inheritance:", "")
+# Remove the SWIG generated 'thisown' attribute. It just bloats the generated
+# documentation and users shouldn't fiddle with the value anyway.
+processed = re.sub(r'~SB[a-zA-Z]+\.thisown', "", processed)
+processed = processed.replace("  .. autoattribute:: thisown", "")
+
+# After removing 'thisown', many objects don't have any attributes left.
+# Remove all now empty attribute summary/documentation sections with
+# some rather ugly regex.
+processed = empty_attr_summary.sub('.. rubric::', processed)
+processed = empty_attr_documentation.sub('.. rubric::', processed)
+
+# Replace the original source with the processed one (source is a single
+# element list).
+source[0] = processed
+
+def setup(app):
+app.connect('source-read', cleanup_source)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] b3c260d - [lldb][docs] Expand CSS fix for LLDB doc tables

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-01-20T10:30:00+01:00
New Revision: b3c260d8fa07ed1202afdda9ca4c437a2a847080

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

LOG: [lldb][docs] Expand CSS fix for LLDB doc tables

Apparently the sphinx version on the server doesn't place  tags in the
table cells, so the previous fix from commit 7fce3b240b6b313b1becf19ddf3f2a90
didn't fix the bug for that sphinx version. Just expand the CSS workaround
to all  tags.

Added: 


Modified: 
lldb/docs/_static/lldb.css

Removed: 




diff  --git a/lldb/docs/_static/lldb.css b/lldb/docs/_static/lldb.css
index 6df20b7dd6e4..e1e49f84c903 100644
--- a/lldb/docs/_static/lldb.css
+++ b/lldb/docs/_static/lldb.css
@@ -47,7 +47,7 @@ table.mapping td.content {
 /* Workaround for a Safari bug that would otherwise make table cells less wide
 than the containing text. This just sets it back to the default browser
 property.*/
-td p {
+td {
   -webkit-hyphens: manual !important;
 }
 



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

[Lldb-commits] [PATCH] D94997: [lldb][lldb-vscode] Updated implementation of 'launch' and 'attach' requests to not create auxiliary target in case "launchCommands" and "attachCommands" are provided.

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added a comment.

D92164  was intended for fixing the "settings 
set" issue, however, it revealed some deadlocks and data races, and had to be 
reverted temporarily. Currently, I'm working on those multithreading issues and 
will submit a patch as soon as possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94997

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

[Lldb-commits] [PATCH] D94672: Implement vAttachOrWait

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

@labath, @clayborg since both of you approved of it, I think this patch is 
ready to be merged, right? Could one of you do it? I don't have commit access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94672

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

[Lldb-commits] [PATCH] D93939: [elf-core] Improve reading memory from core file

[Djordje Todorovic via Phabricator via lldb-commits]

djtodoro updated this revision to Diff 317873.
djtodoro edited the summary of this revision.

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

https://reviews.llvm.org/D93939

Files:
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  
lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-for-disassemble.core
  
lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-for-disassemble.out


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -155,6 +155,37 @@
 self.do_test("linux-x86_64", self._x86_64_pid, self._x86_64_regions,
  "a.out")
 
+
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+@skipIfReproducer
+def test_frame_disassemble(self):
+"""Test that we are able to disassemble all the frames"""
+disasmtarget = 
self.dbg.CreateTarget("linux-x86_64-for-disassemble.out")
+disasmprocess = 
disasmtarget.LoadCore("linux-x86_64-for-disassemble.core")
+self.assertTrue(disasmprocess, PROCESS_IS_VALID)
+
+disasmthread = disasmprocess.GetSelectedThread()
+framenum = disasmthread.GetNumFrames()
+for i in range(framenum):
+frame = disasmthread.GetFrameAtIndex(i)
+disassembly = frame.Disassemble()
+self.assertNotEqual(disassembly, "")
+self.assertNotIn("error", disassembly)
+# Make sure we don't have some dummy disassembly.
+# Each function should start with:
+#   pushq %rbp
+#   ...
+# Sometimes it just prints some dummy code as:
+#   addb %al, (%rax)
+#   addb %al, (%rax)
+#   ...
+framesetup = disassembly.splitlines()[1]
+self.assertNotIn("addb", framesetup)
+
+self.dbg.DeleteTarget(disasmtarget)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("X86")
 def test_FPR_SSE(self):
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -353,7 +353,6 @@
   const lldb::addr_t file_end = address_range->data.GetRangeEnd();
   size_t bytes_to_read = size; // Number of bytes to read from the core file
   size_t bytes_copied = 0;   // Number of bytes actually read from the core 
file
-  size_t zero_fill_size = 0; // Padding
   lldb::addr_t bytes_left =
   0; // Number of bytes available in the core file from the given address
 
@@ -369,22 +368,15 @@
 
   // Figure out how many bytes we need to zero-fill if we are reading more
   // bytes than available in the on-disk segment
-  if (bytes_to_read > bytes_left) {
-zero_fill_size = bytes_to_read - bytes_left;
+  if (bytes_to_read > bytes_left)
 bytes_to_read = bytes_left;
-  }
 
   // If there is data available on the core file read it
   if (bytes_to_read)
 bytes_copied =
 core_objfile->CopyData(offset + file_start, bytes_to_read, buf);
 
-  assert(zero_fill_size <= size);
-  // Pad remaining bytes
-  if (zero_fill_size)
-memset(((char *)buf) + bytes_copied, 0, zero_fill_size);
-
-  return bytes_copied + zero_fill_size;
+  return bytes_copied;
 }
 
 void ProcessElfCore::Clear() {


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -155,6 +155,37 @@
 self.do_test("linux-x86_64", self._x86_64_pid, self._x86_64_regions,
  "a.out")
 
+
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+@skipIfReproducer
+def test_frame_disassemble(self):
+"""Test that we are able to disassemble all the frames"""
+disasmtarget = self.dbg.CreateTarget("linux-x86_64-for-disassemble.out")
+disasmprocess = disasmtarget.LoadCore("linux-x86_64-for-disassemble.core")
+self.assertTrue(disasmprocess, PROCESS_IS_VALID)
+
+disasmthread = disasmprocess.GetSelectedThread()
+framenum = disasmthread.GetNumFrames()
+for i in range(framenum):
+frame = disasmthread.GetFrameAtIndex(i)
+disassembly = frame.Disassemble()
+self.assertNotEqual(disassembly, "")
+self.assertNotIn("error", disassembly)
+# Make sure we don't have some dummy disassembly.
+# Each function should start with:
+#   pushq %rbp
+   

[Lldb-commits] [PATCH] D95059: [lldb/Commands] Refactor ProcessLaunchCommandOptions to use TableGen (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added a reviewer: teemperor.
Herald added subscribers: dang, mgorny.
mib requested review of this revision.
Herald added a project: LLDB.

This patch refactors the current implementation of
`ProcessLaunchCommandOptions` to be generated by TableGen.

The patch also renames the class to `CommandOptionsProcessLaunch` to
align better with the rest of the codebase style and moves it to
separate files.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95059

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandOptionsProcessLaunch.cpp
  lldb/source/Commands/CommandOptionsProcessLaunch.h
  lldb/source/Commands/Options.td
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -307,175 +307,6 @@
 nullptr, ePropertyOSPluginReportsAllThreads, does_report);
 }
 
-Status ProcessLaunchCommandOptions::SetOptionValue(
-uint32_t option_idx, llvm::StringRef option_arg,
-ExecutionContext *execution_context) {
-  Status error;
-  const int short_option = m_getopt_table[option_idx].val;
-
-  switch (short_option) {
-  case 's': // Stop at program entry point
-launch_info.GetFlags().Set(eLaunchFlagStopAtEntry);
-break;
-
-  case 'i': // STDIN for read only
-  {
-FileAction action;
-if (action.Open(STDIN_FILENO, FileSpec(option_arg), true, false))
-  launch_info.AppendFileAction(action);
-break;
-  }
-
-  case 'o': // Open STDOUT for write only
-  {
-FileAction action;
-if (action.Open(STDOUT_FILENO, FileSpec(option_arg), false, true))
-  launch_info.AppendFileAction(action);
-break;
-  }
-
-  case 'e': // STDERR for write only
-  {
-FileAction action;
-if (action.Open(STDERR_FILENO, FileSpec(option_arg), false, true))
-  launch_info.AppendFileAction(action);
-break;
-  }
-
-  case 'p': // Process plug-in name
-launch_info.SetProcessPluginName(option_arg);
-break;
-
-  case 'n': // Disable STDIO
-  {
-FileAction action;
-const FileSpec dev_null(FileSystem::DEV_NULL);
-if (action.Open(STDIN_FILENO, dev_null, true, false))
-  launch_info.AppendFileAction(action);
-if (action.Open(STDOUT_FILENO, dev_null, false, true))
-  launch_info.AppendFileAction(action);
-if (action.Open(STDERR_FILENO, dev_null, false, true))
-  launch_info.AppendFileAction(action);
-break;
-  }
-
-  case 'w':
-launch_info.SetWorkingDirectory(FileSpec(option_arg));
-break;
-
-  case 't': // Open process in new terminal window
-launch_info.GetFlags().Set(eLaunchFlagLaunchInTTY);
-break;
-
-  case 'a': {
-TargetSP target_sp =
-execution_context ? execution_context->GetTargetSP() : TargetSP();
-PlatformSP platform_sp =
-target_sp ? target_sp->GetPlatform() : PlatformSP();
-launch_info.GetArchitecture() =
-Platform::GetAugmentedArchSpec(platform_sp.get(), option_arg);
-  } break;
-
-  case 'A': // Disable ASLR.
-  {
-bool success;
-const bool disable_aslr_arg =
-OptionArgParser::ToBoolean(option_arg, true, &success);
-if (success)
-  disable_aslr = disable_aslr_arg ? eLazyBoolYes : eLazyBoolNo;
-else
-  error.SetErrorStringWithFormat(
-  "Invalid boolean value for disable-aslr option: '%s'",
-  option_arg.empty() ? "" : option_arg.str().c_str());
-break;
-  }
-
-  case 'X': // shell expand args.
-  {
-bool success;
-const bool expand_args =
-OptionArgParser::ToBoolean(option_arg, true, &success);
-if (success)
-  launch_info.SetShellExpandArguments(expand_args);
-else
-  error.SetErrorStringWithFormat(
-  "Invalid boolean value for shell-expand-args option: '%s'",
-  option_arg.empty() ? "" : option_arg.str().c_str());
-break;
-  }
-
-  case 'c':
-if (!option_arg.empty())
-  launch_info.SetShell(FileSpec(option_arg));
-else
-  launch_info.SetShell(HostInfo::GetDefaultShell());
-break;
-
-  case 'v':
-launch_info.GetEnvironment().insert(option_arg);
-break;
-
-  default:
-error.SetErrorStringWithFormat("unrecognized short option character '%c'",
-   short_option);
-break;
-  }
-  return error;
-}
-
-static constexpr OptionDefinition g_process_launch_options[] = {
-{LLDB_OPT_SET_ALL, false, "stop-at-entry", 's', OptionParser::eNoArgument,
- nullptr, {}, 0, eArgTypeNone,
- "Stop at the entry point of the program when launching a process."},
-{LLDB_OPT_SET_ALL, false, "disable-aslr", 'A',
- OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeBoolean,
- "Set whether to disable address space layout randomization when 

[Lldb-commits] [PATCH] D95059: [lldb/Commands] Refactor ProcessLaunchCommandOptions to use TableGen (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 317911.
mib added a comment.

Add header guards and split tablegen long description into multiple lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95059

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandOptionsProcessLaunch.cpp
  lldb/source/Commands/CommandOptionsProcessLaunch.h
  lldb/source/Commands/Options.td
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -307,175 +307,6 @@
 nullptr, ePropertyOSPluginReportsAllThreads, does_report);
 }
 
-Status ProcessLaunchCommandOptions::SetOptionValue(
-uint32_t option_idx, llvm::StringRef option_arg,
-ExecutionContext *execution_context) {
-  Status error;
-  const int short_option = m_getopt_table[option_idx].val;
-
-  switch (short_option) {
-  case 's': // Stop at program entry point
-launch_info.GetFlags().Set(eLaunchFlagStopAtEntry);
-break;
-
-  case 'i': // STDIN for read only
-  {
-FileAction action;
-if (action.Open(STDIN_FILENO, FileSpec(option_arg), true, false))
-  launch_info.AppendFileAction(action);
-break;
-  }
-
-  case 'o': // Open STDOUT for write only
-  {
-FileAction action;
-if (action.Open(STDOUT_FILENO, FileSpec(option_arg), false, true))
-  launch_info.AppendFileAction(action);
-break;
-  }
-
-  case 'e': // STDERR for write only
-  {
-FileAction action;
-if (action.Open(STDERR_FILENO, FileSpec(option_arg), false, true))
-  launch_info.AppendFileAction(action);
-break;
-  }
-
-  case 'p': // Process plug-in name
-launch_info.SetProcessPluginName(option_arg);
-break;
-
-  case 'n': // Disable STDIO
-  {
-FileAction action;
-const FileSpec dev_null(FileSystem::DEV_NULL);
-if (action.Open(STDIN_FILENO, dev_null, true, false))
-  launch_info.AppendFileAction(action);
-if (action.Open(STDOUT_FILENO, dev_null, false, true))
-  launch_info.AppendFileAction(action);
-if (action.Open(STDERR_FILENO, dev_null, false, true))
-  launch_info.AppendFileAction(action);
-break;
-  }
-
-  case 'w':
-launch_info.SetWorkingDirectory(FileSpec(option_arg));
-break;
-
-  case 't': // Open process in new terminal window
-launch_info.GetFlags().Set(eLaunchFlagLaunchInTTY);
-break;
-
-  case 'a': {
-TargetSP target_sp =
-execution_context ? execution_context->GetTargetSP() : TargetSP();
-PlatformSP platform_sp =
-target_sp ? target_sp->GetPlatform() : PlatformSP();
-launch_info.GetArchitecture() =
-Platform::GetAugmentedArchSpec(platform_sp.get(), option_arg);
-  } break;
-
-  case 'A': // Disable ASLR.
-  {
-bool success;
-const bool disable_aslr_arg =
-OptionArgParser::ToBoolean(option_arg, true, &success);
-if (success)
-  disable_aslr = disable_aslr_arg ? eLazyBoolYes : eLazyBoolNo;
-else
-  error.SetErrorStringWithFormat(
-  "Invalid boolean value for disable-aslr option: '%s'",
-  option_arg.empty() ? "" : option_arg.str().c_str());
-break;
-  }
-
-  case 'X': // shell expand args.
-  {
-bool success;
-const bool expand_args =
-OptionArgParser::ToBoolean(option_arg, true, &success);
-if (success)
-  launch_info.SetShellExpandArguments(expand_args);
-else
-  error.SetErrorStringWithFormat(
-  "Invalid boolean value for shell-expand-args option: '%s'",
-  option_arg.empty() ? "" : option_arg.str().c_str());
-break;
-  }
-
-  case 'c':
-if (!option_arg.empty())
-  launch_info.SetShell(FileSpec(option_arg));
-else
-  launch_info.SetShell(HostInfo::GetDefaultShell());
-break;
-
-  case 'v':
-launch_info.GetEnvironment().insert(option_arg);
-break;
-
-  default:
-error.SetErrorStringWithFormat("unrecognized short option character '%c'",
-   short_option);
-break;
-  }
-  return error;
-}
-
-static constexpr OptionDefinition g_process_launch_options[] = {
-{LLDB_OPT_SET_ALL, false, "stop-at-entry", 's', OptionParser::eNoArgument,
- nullptr, {}, 0, eArgTypeNone,
- "Stop at the entry point of the program when launching a process."},
-{LLDB_OPT_SET_ALL, false, "disable-aslr", 'A',
- OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeBoolean,
- "Set whether to disable address space layout randomization when launching "
- "a process."},
-{LLDB_OPT_SET_ALL, false, "plugin", 'p', OptionParser::eRequiredArgument,
- nullptr, {}, 0, eArgTypePlugin,
- "Name of the process plugin you want to use."},
-{LLDB_OPT_SET_ALL, false, "working-dir", 'w',
- O

[Lldb-commits] [PATCH] D95059: [lldb/Commands] Refactor ProcessLaunchCommandOptions to use TableGen (NFC)

[Raphael Isemann via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95059

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

[Lldb-commits] [PATCH] D95059: [lldb/Commands] Refactor ProcessLaunchCommandOptions to use TableGen (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 317914.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95059

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandOptionsProcessLaunch.cpp
  lldb/source/Commands/CommandOptionsProcessLaunch.h
  lldb/source/Commands/Options.td
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -307,175 +307,6 @@
 nullptr, ePropertyOSPluginReportsAllThreads, does_report);
 }
 
-Status ProcessLaunchCommandOptions::SetOptionValue(
-uint32_t option_idx, llvm::StringRef option_arg,
-ExecutionContext *execution_context) {
-  Status error;
-  const int short_option = m_getopt_table[option_idx].val;
-
-  switch (short_option) {
-  case 's': // Stop at program entry point
-launch_info.GetFlags().Set(eLaunchFlagStopAtEntry);
-break;
-
-  case 'i': // STDIN for read only
-  {
-FileAction action;
-if (action.Open(STDIN_FILENO, FileSpec(option_arg), true, false))
-  launch_info.AppendFileAction(action);
-break;
-  }
-
-  case 'o': // Open STDOUT for write only
-  {
-FileAction action;
-if (action.Open(STDOUT_FILENO, FileSpec(option_arg), false, true))
-  launch_info.AppendFileAction(action);
-break;
-  }
-
-  case 'e': // STDERR for write only
-  {
-FileAction action;
-if (action.Open(STDERR_FILENO, FileSpec(option_arg), false, true))
-  launch_info.AppendFileAction(action);
-break;
-  }
-
-  case 'p': // Process plug-in name
-launch_info.SetProcessPluginName(option_arg);
-break;
-
-  case 'n': // Disable STDIO
-  {
-FileAction action;
-const FileSpec dev_null(FileSystem::DEV_NULL);
-if (action.Open(STDIN_FILENO, dev_null, true, false))
-  launch_info.AppendFileAction(action);
-if (action.Open(STDOUT_FILENO, dev_null, false, true))
-  launch_info.AppendFileAction(action);
-if (action.Open(STDERR_FILENO, dev_null, false, true))
-  launch_info.AppendFileAction(action);
-break;
-  }
-
-  case 'w':
-launch_info.SetWorkingDirectory(FileSpec(option_arg));
-break;
-
-  case 't': // Open process in new terminal window
-launch_info.GetFlags().Set(eLaunchFlagLaunchInTTY);
-break;
-
-  case 'a': {
-TargetSP target_sp =
-execution_context ? execution_context->GetTargetSP() : TargetSP();
-PlatformSP platform_sp =
-target_sp ? target_sp->GetPlatform() : PlatformSP();
-launch_info.GetArchitecture() =
-Platform::GetAugmentedArchSpec(platform_sp.get(), option_arg);
-  } break;
-
-  case 'A': // Disable ASLR.
-  {
-bool success;
-const bool disable_aslr_arg =
-OptionArgParser::ToBoolean(option_arg, true, &success);
-if (success)
-  disable_aslr = disable_aslr_arg ? eLazyBoolYes : eLazyBoolNo;
-else
-  error.SetErrorStringWithFormat(
-  "Invalid boolean value for disable-aslr option: '%s'",
-  option_arg.empty() ? "" : option_arg.str().c_str());
-break;
-  }
-
-  case 'X': // shell expand args.
-  {
-bool success;
-const bool expand_args =
-OptionArgParser::ToBoolean(option_arg, true, &success);
-if (success)
-  launch_info.SetShellExpandArguments(expand_args);
-else
-  error.SetErrorStringWithFormat(
-  "Invalid boolean value for shell-expand-args option: '%s'",
-  option_arg.empty() ? "" : option_arg.str().c_str());
-break;
-  }
-
-  case 'c':
-if (!option_arg.empty())
-  launch_info.SetShell(FileSpec(option_arg));
-else
-  launch_info.SetShell(HostInfo::GetDefaultShell());
-break;
-
-  case 'v':
-launch_info.GetEnvironment().insert(option_arg);
-break;
-
-  default:
-error.SetErrorStringWithFormat("unrecognized short option character '%c'",
-   short_option);
-break;
-  }
-  return error;
-}
-
-static constexpr OptionDefinition g_process_launch_options[] = {
-{LLDB_OPT_SET_ALL, false, "stop-at-entry", 's', OptionParser::eNoArgument,
- nullptr, {}, 0, eArgTypeNone,
- "Stop at the entry point of the program when launching a process."},
-{LLDB_OPT_SET_ALL, false, "disable-aslr", 'A',
- OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeBoolean,
- "Set whether to disable address space layout randomization when launching "
- "a process."},
-{LLDB_OPT_SET_ALL, false, "plugin", 'p', OptionParser::eRequiredArgument,
- nullptr, {}, 0, eArgTypePlugin,
- "Name of the process plugin you want to use."},
-{LLDB_OPT_SET_ALL, false, "working-dir", 'w',
- OptionParser::eRequiredArgument, nullptr, {}, 0,
- eArgTypeDirectoryName,
- "Set the curren

[Lldb-commits] [PATCH] D94997: [lldb][lldb-vscode] Updated implementation of 'launch' and 'attach' requests to not create auxiliary target in case "launchCommands" and "attachCommands" are provided.

[Serhiy Redko via Phabricator via lldb-commits]

serhiy.redko planned changes to this revision.
serhiy.redko added a comment.

Thanks for review and your input, @clayborg

> So I agree we need to fix the "settings set" issue as it surfaces a quirk in 
> the order and number of targets we create. The main questions is if we care 
> that we don't auto create a target when "launchCommands" are used and what 
> the right solution is for lldb-vscode going forward.

Please correct me if I'm wrong, but my understanding is: from lldb-vscode 
launch.json configuration we generally should create a single target/process 
which user will debug.
The fact that 'debugger' instance has several targets after 'launch', 'attach' 
doesn't look correct to me, while things still work. It introduces ambiguity 
and conundrum in implementation what exactly user wants to debug. 
Do we have use cases for several targets in single lldb-vscode session? If we 
need to debug several processes, my understanding is we need to launch several 
lldb-vscode instances for each process to debug, is it correct?
So ideally from launch.json we should construct single target and use it.

So I think it is important for user to have a clear understanding about the way 
the debug target is created. 
Now it can be created with:

1. 'convenience' fields like "program", "pid"..,
2. 'attachCommands'/'launchCommands'
3. potentially with any other fields that execute lldb commands e.g 
"initCommands"

I assume all we want is to get clear instructions from user about final target, 
but not a mix of all ways listed above to create target and use last to work 
with. Otherwise it will be confusion like why target I specified with my 
"program" field is ignored when I provided 'launchCommands' that also create 
target?

With this change I try to resolve ambiguity for user and implementation on the 
approach to create the debug target and don't break many existing use cases.

So I suggest to document and 'enforce' the following:

1. 'convenience' fields like "program", "pid".. for which we create auto target 
because eventually we need a target.
2. 'attachCommands'/'launchCommands' where user will be responsible to provide 
LLDB commands that will create a target. No auto target is needed.

#1 and #2 will be stated as mutually exclusive in documentation and made in 
implementation.

I'm not sure we can 'enforce' user to not create target in "initCommands" 
(unless we assert(debugger.GetTargetsNum() == 1)) and other similar launch.json 
fields but it should be easy to document.

> Another solution would be to not auto create a target if the "program" 
> argument is missing from the launch config and document this in the 
> "attachCommands" or "launchCommands"? Or is "program" a required launch 
> config argument?

While "program" is listed as required in launch.json it is not required in case 
'launchCommands' create a target, so we can make it optional. 
Since there are many target creating fields like "program" with described 
changes it will be easier to check whether  'attachCommands'/'launchCommands' 
are provided and avoid auto target creation.

I agree that we need to do sanitization of input parameters and warn about 
ambiguity. Since "program" is listed as required, I think some clients, that 
use 'launchCommands' are passing dummy value for "program" (exactly what our 
current tests are doing) and the "program" is ignored eventually. To not break 
such clients, probably, it would be better to log warnings vs stop with an 
error. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94997

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

[Lldb-commits] [lldb] 7169d3a - [lldb/Commands] Refactor ProcessLaunchCommandOptions to use TableGen (NFC)

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2021-01-20T18:53:06+01:00
New Revision: 7169d3a315f4cdc19c4ab6b8f20c6f91b46ba9b8

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

LOG: [lldb/Commands] Refactor ProcessLaunchCommandOptions to use TableGen (NFC)

This patch refactors the current implementation of
`ProcessLaunchCommandOptions` to be generated by TableGen.

The patch also renames the class to `CommandOptionsProcessLaunch` to
align better with the rest of the codebase style and moves it to
separate files.

Differential Review: https://reviews.llvm.org/D95059

Signed-off-by: Med Ismail Bennani 

Added: 
lldb/source/Commands/CommandOptionsProcessLaunch.cpp
lldb/source/Commands/CommandOptionsProcessLaunch.h

Modified: 
lldb/include/lldb/Target/Process.h
lldb/source/Commands/CMakeLists.txt
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/Options.td
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 6f30787f7e5b..5ca5dd28fd8f 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -30,7 +30,6 @@
 #include "lldb/Host/HostThread.h"
 #include "lldb/Host/ProcessLaunchInfo.h"
 #include "lldb/Host/ProcessRunLock.h"
-#include "lldb/Interpreter/Options.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/ExecutionContextScope.h"
 #include "lldb/Target/InstrumentationRuntime.h"
@@ -210,32 +209,6 @@ class ProcessAttachInfo : public ProcessInstanceInfo {
 // call SBProcess::Stop() to cancel attach)
 };
 
-class ProcessLaunchCommandOptions : public Options {
-public:
-  ProcessLaunchCommandOptions() : Options() {
-// Keep default values of all options in one place: OptionParsingStarting
-// ()
-OptionParsingStarting(nullptr);
-  }
-
-  ~ProcessLaunchCommandOptions() override = default;
-
-  Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
-ExecutionContext *execution_context) override;
-
-  void OptionParsingStarting(ExecutionContext *execution_context) override {
-launch_info.Clear();
-disable_aslr = eLazyBoolCalculate;
-  }
-
-  llvm::ArrayRef GetDefinitions() override;
-
-  // Instance variables to hold the values for command options.
-
-  ProcessLaunchInfo launch_info;
-  lldb_private::LazyBool disable_aslr;
-};
-
 // This class tracks the Modification state of the process.  Things that can
 // currently modify the program are running the program (which will up the
 // StopID) and writing memory (which will up the MemoryID.)

diff  --git a/lldb/source/Commands/CMakeLists.txt 
b/lldb/source/Commands/CMakeLists.txt
index 4f10516c2f69..988ff894ea67 100644
--- a/lldb/source/Commands/CMakeLists.txt
+++ b/lldb/source/Commands/CMakeLists.txt
@@ -37,6 +37,7 @@ add_lldb_library(lldbCommands
   CommandObjectVersion.cpp
   CommandObjectWatchpoint.cpp
   CommandObjectWatchpointCommand.cpp
+  CommandOptionsProcessLaunch.cpp
 
   LINK_LIBS
 lldbBase

diff  --git a/lldb/source/Commands/CommandObjectPlatform.cpp 
b/lldb/source/Commands/CommandObjectPlatform.cpp
index 3a5af9f91cf1..f306da3c8543 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "CommandObjectPlatform.h"
+#include "CommandOptionsProcessLaunch.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
@@ -1083,7 +1084,7 @@ class CommandObjectPlatformProcessLaunch : public 
CommandObjectParsed {
 return result.Succeeded();
   }
 
-  ProcessLaunchCommandOptions m_options;
+  CommandOptionsProcessLaunch m_options;
 };
 
 // "platform process list"

diff  --git a/lldb/source/Commands/CommandObjectProcess.cpp 
b/lldb/source/Commands/CommandObjectProcess.cpp
index 1eef2800ce16..35835f638557 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "CommandObjectProcess.h"
+#include "CommandOptionsProcessLaunch.h"
 #include "lldb/Breakpoint/Breakpoint.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Breakpoint/BreakpointSite.h"
@@ -251,7 +252,7 @@ class CommandObjectProcessLaunch : public 
CommandObjectProcessLaunchOrAttach {
 return result.Succeeded();
   }
 
-  ProcessLaunchCommandOptions m_options;
+  CommandOptionsProcessLaunch m_options;
 };
 
 #define LLDB_OPTIONS_process_attach

diff  --git a/lldb/source/Commands/CommandOptionsProcessLaunch.c

[Lldb-commits] [PATCH] D95059: [lldb/Commands] Refactor ProcessLaunchCommandOptions to use TableGen (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib closed this revision.
mib added a comment.

Landed in 7169d3a315f4 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95059

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

[Lldb-commits] [PATCH] D94997: [lldb][lldb-vscode] Updated implementation of 'launch' and 'attach' requests to not create auxiliary target in case "launchCommands" and "attachCommands" are provided.

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

aadsm added inline comments.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1558
+  if (!launchCommands.empty()) {
+// if "launchCommands" are provided, then they are expected to make the
+// launch happen for launch requests and they replace the normal logic that

clayborg wrote:
> We need to check if any arguments are set in the launch config that 
> "launchCommands" will ignore and return an error if any of them are set. Or 
> we need to emit an error or warning to the debug console so the users can 
> know that these settings are now ignored because "launchCommands" will cause 
> them to be.
I vote for a warning here otherwise it might break people's current setups, 
assuming the error is an indication that the launch won't happen, but we should 
totally do it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94997

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

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation and make restart

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

Is this good? :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94890

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

[Lldb-commits] [lldb] 599fdfc - Revert "[lldb] Re-enable TestPlatformProcessConnect on macos"

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-01-20T20:49:03+01:00
New Revision: 599fdfc5db8f44582ee9bd05544769268ec9b4a3

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

LOG: Revert "[lldb] Re-enable TestPlatformProcessConnect on macos"

This reverts commit 079e664661770a78e30c0d27a12d50047f1b1ea8. It needs
more work.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py

lldb/test/API/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
index d16549420a14..07136108b2a4 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
@@ -72,6 +72,9 @@ def get_lldb_server_exe():
 A path to the lldb-server exe if it is found to exist; otherwise,
 returns None.
 """
+if "LLDB_DEBUGSERVER_PATH" in os.environ:
+return os.environ["LLDB_DEBUGSERVER_PATH"]
+
 return _get_debug_monitor_from_lldb(
 lldbtest_config.lldbExec, "lldb-server")
 

diff  --git 
a/lldb/test/API/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py
 
b/lldb/test/API/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py
index 3607c49c9c97..8ddab260b494 100644
--- 
a/lldb/test/API/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py
+++ 
b/lldb/test/API/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py
@@ -11,6 +11,7 @@ class 
TestPlatformProcessConnect(gdbremote_testcase.GdbRemoteTestCaseBase):
 @skipIfRemote
 @expectedFailureAll(hostoslist=["windows"], triple='.*-android')
 @skipIfWindows # lldb-server does not terminate correctly
+@skipIfDarwin # lldb-server not found correctly
 def test_platform_process_connect(self):
 self.build()
 



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

Re: [Lldb-commits] [lldb] 079e664 - [lldb] Re-enable TestPlatformProcessConnect on macos

[Pavel Labath via lldb-commits]

On 20/01/2021 04:35, Eric Christopher wrote:

+Jordan Rupprecht 

Interesting, we were using this internally to point to an lldb-server to 
test. I've disabled some tests at the moment, but I'm not sure it isn't 
entirely useful functionality :)


-eric



Yes, I don't think I have fully considered all the aspects of this 
patch. I'll revert this and start over.


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

Re: [Lldb-commits] [lldb] 079e664 - [lldb] Re-enable TestPlatformProcessConnect on macos

[Eric Christopher via lldb-commits]

On Wed, Jan 20, 2021 at 2:51 PM Pavel Labath  wrote:

> On 20/01/2021 04:35, Eric Christopher wrote:
> > +Jordan Rupprecht 
> >
> > Interesting, we were using this internally to point to an lldb-server to
> > test. I've disabled some tests at the moment, but I'm not sure it isn't
> > entirely useful functionality :)
> >
> > -eric
> >
>
> Yes, I don't think I have fully considered all the aspects of this
> patch. I'll revert this and start over.
>
>
Cheers!  :)

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

[Lldb-commits] [lldb] 8fc9b6c - [lldb/Commands] Align process launch --plugin with process attach (NFC)

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2021-01-20T21:01:23+01:00
New Revision: 8fc9b6c2c560fc5945ce2115de345efb1617d59d

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

LOG: [lldb/Commands] Align process launch --plugin with process attach (NFC)

Following `7169d3a315f4cdc19c4ab6b8f20c6f91b46ba9b8`, this patch updates
the short option for the plugin command option to (`-p` to `-P`) to
align with the `process attach` command options.

The long option remains the same since there are already the same for both
commands.

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/source/Commands/CommandOptionsProcessLaunch.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandOptionsProcessLaunch.cpp 
b/lldb/source/Commands/CommandOptionsProcessLaunch.cpp
index e94a89469ca9..4445457ca852 100644
--- a/lldb/source/Commands/CommandOptionsProcessLaunch.cpp
+++ b/lldb/source/Commands/CommandOptionsProcessLaunch.cpp
@@ -61,7 +61,7 @@ Status CommandOptionsProcessLaunch::SetOptionValue(
 break;
   }
 
-  case 'p': // Process plug-in name
+  case 'P': // Process plug-in name
 launch_info.SetProcessPluginName(option_arg);
 break;
 



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

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation and make restart

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D94890#2510446 , @MaskRay wrote:

> Is this good? :)

I think you forgot to update the patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94890

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

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation and make restart

[Fangrui Song via Phabricator via lldb-commits]

MaskRay updated this revision to Diff 317983.
MaskRay added a comment.

Inline archive rule


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94890

Files:
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  lldb/test/API/functionalities/archives/Makefile

Index: lldb/test/API/functionalities/archives/Makefile
===
--- lldb/test/API/functionalities/archives/Makefile
+++ lldb/test/API/functionalities/archives/Makefile
@@ -1,7 +1,13 @@
-C_SOURCES := main.c
-
+C_SOURCES := main.c a.c b.c
+EXE :=  # Define a.out explicitly
 MAKE_DSYM := NO
-ARCHIVE_NAME := libfoo.a
-ARCHIVE_C_SOURCES := a.c b.c
+
+all: a.out
+
+a.out: main.o libfoo.a
+	$(LD) $(LDFLAGS) $^ -o $@
+
+libfoo.a: a.o b.o
+	ar $(ARFLAGS) $@ $^
 
 include Makefile.rules
Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -26,15 +26,13 @@
 # SPLIT_DEBUG_SYMBOLS := YES
 # CROSS_COMPILE :=
 # USE_PRIVATE_MODULE_CACHE := YES
-#
-# And test/functionalities/archives/Makefile:
-# MAKE_DSYM := NO
-# ARCHIVE_NAME := libfoo.a
-# ARCHIVE_C_SOURCES := a.c b.c
 
 # Uncomment line below for debugging shell commands
 # SHELL = /bin/sh -x
 
+# Suppress built-in suffix rules. We explicitly define rules for %.o.
+.SUFFIXES:
+
 SRCDIR := $(shell dirname $(firstword $(MAKEFILE_LIST)))
 BUILDDIR := $(shell pwd)
 MAKEFILE_RULES := $(lastword $(MAKEFILE_LIST))
@@ -477,42 +475,6 @@
 	endif
 endif
 
-#--
-# Check if we have any C source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_C_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_C_SOURCES:.c=.o))
-endif
-
-#--
-# Check if we have any C++ source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_CXX_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_CXX_SOURCES:.cpp=.o))
-	CXX = $(call cxx_compiler,$(CC))
-	LD = $(call cxx_linker,$(CC))
-endif
-
-#--
-# Check if we have any ObjC source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_OBJC_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_OBJC_SOURCES:.m=.o))
-	LDFLAGS +=-lobjc
-endif
-
-#--
-# Check if we have any ObjC++ source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_OBJCXX_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_OBJCXX_SOURCES:.mm=.o))
-	CXX = $(call cxx_compiler,$(CC))
-	LD = $(call cxx_linker,$(CC))
-	ifeq "$(findstring lobjc,$(LDFLAGS))" ""
-		LDFLAGS +=-lobjc
-	endif
-endif
-
 ifeq ($(findstring clang, $(CXX)), clang)
 	CXXFLAGS += --driver-mode=g++
 endif
@@ -534,7 +496,7 @@
 #--
 ifneq "$(DYLIB_NAME)" ""
 ifeq "$(DYLIB_ONLY)" ""
-$(EXE) : $(OBJECTS) $(ARCHIVE_NAME) $(DYLIB_FILENAME)
+$(EXE) : $(OBJECTS) $(DYLIB_FILENAME)
 	$(LD) $(OBJECTS) $(ARCHIVE_NAME) -L. -l$(DYLIB_NAME) $(LDFLAGS) -o "$(EXE)"
 ifneq "$(CODESIGN)" ""
 	$(CODESIGN) -s - "$(EXE)"
@@ -566,19 +528,6 @@
 endif
 endif
 
-#--
-# Make the archive
-#--
-ifneq "$(ARCHIVE_NAME)" ""
-ifeq "$(OS)" "Darwin"
-$(ARCHIVE_NAME) : $(ARCHIVE_OBJECTS)
-	$(AR) $(ARFLAGS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS)
-	$(RM) $(ARCHIVE_OBJECTS)
-else
-$(ARCHIVE_NAME) : $(foreach ar_obj,$(ARCHIVE_OBJECTS),$(ARCHIVE_NAME)($(ar_obj)))
-endif
-endif
-
 #--
 # Make the dylib
 #--
@@ -628,12 +577,22 @@
 # Make the precompiled header and compile C++ sources against it
 #--
 
-#ifneq "$(PCH_OUTPUT)" ""
+ifneq "$(PCH_OUTPUT)" ""
 $(PCH_OUTPUT) : $(PCH_CXX_SOURCE)
 	$(CXX) $(CXXFLAGS) -x c++-header -o $@ $<
-%.o : %.cpp $(PCH_OUTPUT)
-	$(CXX) $(PCHFLAGS) $(CXXFLAGS) -c -o $@ $<
-#endif
+endif
+
+%.o: %.c %.d
+	$(CC) $(CFLAGS) -MT $@ -MD -MP -MF $*.d -c -o $@ $<
+
+%.o: %.cpp %.d $(PCH_OUTPUT)
+	$(CXX) $(PCHFLAGS) $(CXXFLAGS) -MT $@ -MD -MP -MF $*.d -c -o $@ $<
+
+%.o: %.m %.d
+	$(CC) $(CFLAGS) -MT $@ -MD -MP -MF $*.d -c -o $@ $<
+
+%.o: %.mm %.d
+	$(CXX) $(CXXFLAGS) -MT $@ -MD -MP -MF $*.d -c -o $@ $<
 
 #-

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation and make restart

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

In D94890#2510627 , @JDevlieghere 
wrote:

> In D94890#2510446 , @MaskRay wrote:
>
>> Is this good? :)
>
> I think you forgot to update the patch?

Ah, looks like you want to do inline (`ARCHIVE_OBJECTS`) into the test in this 
patch... Done (it is a bit tricky).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94890

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

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test

[Fangrui Song via Phabricator via lldb-commits]

MaskRay updated this revision to Diff 317985.
MaskRay retitled this revision from "Makefile.rules: Avoid redundant .d 
generation and make restart" to "Makefile.rules: Avoid redundant .d generation 
(make restart) and inline archive rule to the only test".
MaskRay edited the summary of this revision.
MaskRay added a comment.

Use $(AR)

Remove more ARCHIVE_NAME


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94890

Files:
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  lldb/test/API/functionalities/archives/Makefile

Index: lldb/test/API/functionalities/archives/Makefile
===
--- lldb/test/API/functionalities/archives/Makefile
+++ lldb/test/API/functionalities/archives/Makefile
@@ -1,7 +1,13 @@
-C_SOURCES := main.c
-
+C_SOURCES := main.c a.c b.c
+EXE :=  # Define a.out explicitly
 MAKE_DSYM := NO
-ARCHIVE_NAME := libfoo.a
-ARCHIVE_C_SOURCES := a.c b.c
+
+all: a.out
+
+a.out: main.o libfoo.a
+	$(LD) $(LDFLAGS) $^ -o $@
+
+libfoo.a: a.o b.o
+	$(AR) $(ARFLAGS) $@ $^
 
 include Makefile.rules
Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -26,15 +26,13 @@
 # SPLIT_DEBUG_SYMBOLS := YES
 # CROSS_COMPILE :=
 # USE_PRIVATE_MODULE_CACHE := YES
-#
-# And test/functionalities/archives/Makefile:
-# MAKE_DSYM := NO
-# ARCHIVE_NAME := libfoo.a
-# ARCHIVE_C_SOURCES := a.c b.c
 
 # Uncomment line below for debugging shell commands
 # SHELL = /bin/sh -x
 
+# Suppress built-in suffix rules. We explicitly define rules for %.o.
+.SUFFIXES:
+
 SRCDIR := $(shell dirname $(firstword $(MAKEFILE_LIST)))
 BUILDDIR := $(shell pwd)
 MAKEFILE_RULES := $(lastword $(MAKEFILE_LIST))
@@ -477,42 +475,6 @@
 	endif
 endif
 
-#--
-# Check if we have any C source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_C_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_C_SOURCES:.c=.o))
-endif
-
-#--
-# Check if we have any C++ source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_CXX_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_CXX_SOURCES:.cpp=.o))
-	CXX = $(call cxx_compiler,$(CC))
-	LD = $(call cxx_linker,$(CC))
-endif
-
-#--
-# Check if we have any ObjC source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_OBJC_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_OBJC_SOURCES:.m=.o))
-	LDFLAGS +=-lobjc
-endif
-
-#--
-# Check if we have any ObjC++ source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_OBJCXX_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_OBJCXX_SOURCES:.mm=.o))
-	CXX = $(call cxx_compiler,$(CC))
-	LD = $(call cxx_linker,$(CC))
-	ifeq "$(findstring lobjc,$(LDFLAGS))" ""
-		LDFLAGS +=-lobjc
-	endif
-endif
-
 ifeq ($(findstring clang, $(CXX)), clang)
 	CXXFLAGS += --driver-mode=g++
 endif
@@ -534,8 +496,8 @@
 #--
 ifneq "$(DYLIB_NAME)" ""
 ifeq "$(DYLIB_ONLY)" ""
-$(EXE) : $(OBJECTS) $(ARCHIVE_NAME) $(DYLIB_FILENAME)
-	$(LD) $(OBJECTS) $(ARCHIVE_NAME) -L. -l$(DYLIB_NAME) $(LDFLAGS) -o "$(EXE)"
+$(EXE) : $(OBJECTS) $(DYLIB_FILENAME)
+	$(LD) $(OBJECTS) -L. -l$(DYLIB_NAME) $(LDFLAGS) -o "$(EXE)"
 ifneq "$(CODESIGN)" ""
 	$(CODESIGN) -s - "$(EXE)"
 endif
@@ -543,8 +505,8 @@
 EXE = $(DYLIB_FILENAME)
 endif
 else
-$(EXE) : $(OBJECTS) $(ARCHIVE_NAME)
-	$(LD) $(OBJECTS) $(LDFLAGS) $(ARCHIVE_NAME) -o "$(EXE)"
+$(EXE) : $(OBJECTS)
+	$(LD) $(OBJECTS) $(LDFLAGS) -o "$(EXE)"
 ifneq "$(CODESIGN)" ""
 	$(CODESIGN) -s - "$(EXE)"
 endif
@@ -566,19 +528,6 @@
 endif
 endif
 
-#--
-# Make the archive
-#--
-ifneq "$(ARCHIVE_NAME)" ""
-ifeq "$(OS)" "Darwin"
-$(ARCHIVE_NAME) : $(ARCHIVE_OBJECTS)
-	$(AR) $(ARFLAGS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS)
-	$(RM) $(ARCHIVE_OBJECTS)
-else
-$(ARCHIVE_NAME) : $(foreach ar_obj,$(ARCHIVE_OBJECTS),$(ARCHIVE_NAME)($(ar_obj)))
-endif
-endif
-
 #--
 # Make the dylib
 #--
@@ -628,12 +577,22 @@
 # Make the precompiled header and compile C++ sources against it
 #-

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM with the inline comment addressed.




Comment at: lldb/test/API/functionalities/archives/Makefile:11
+libfoo.a: a.o b.o
+   $(AR) $(ARFLAGS) $@ $^
 

I would remove the object files just to be sure. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94890

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

[Lldb-commits] [PATCH] D95059: [lldb/Commands] Refactor ProcessLaunchCommandOptions to use TableGen (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Commands/CommandOptionsProcessLaunch.h:1
+//===-- CommandOptionsProcessLaunch.h 
-===//
+//

Nit: header files should have the `-*- C++ -*-===//` at the end. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95059

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

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test

[Fangrui Song via Phabricator via lldb-commits]

MaskRay updated this revision to Diff 318011.
MaskRay marked an inline comment as done.
MaskRay added a comment.

$(RM) a.o b.o


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94890

Files:
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  lldb/test/API/functionalities/archives/Makefile

Index: lldb/test/API/functionalities/archives/Makefile
===
--- lldb/test/API/functionalities/archives/Makefile
+++ lldb/test/API/functionalities/archives/Makefile
@@ -1,7 +1,14 @@
-C_SOURCES := main.c
-
+C_SOURCES := main.c a.c b.c
+EXE :=  # Define a.out explicitly
 MAKE_DSYM := NO
-ARCHIVE_NAME := libfoo.a
-ARCHIVE_C_SOURCES := a.c b.c
+
+all: a.out
+
+a.out: main.o libfoo.a
+	$(LD) $(LDFLAGS) $^ -o $@
+
+libfoo.a: a.o b.o
+	$(AR) $(ARFLAGS) $@ $^
+	$(RM) $^
 
 include Makefile.rules
Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -26,15 +26,13 @@
 # SPLIT_DEBUG_SYMBOLS := YES
 # CROSS_COMPILE :=
 # USE_PRIVATE_MODULE_CACHE := YES
-#
-# And test/functionalities/archives/Makefile:
-# MAKE_DSYM := NO
-# ARCHIVE_NAME := libfoo.a
-# ARCHIVE_C_SOURCES := a.c b.c
 
 # Uncomment line below for debugging shell commands
 # SHELL = /bin/sh -x
 
+# Suppress built-in suffix rules. We explicitly define rules for %.o.
+.SUFFIXES:
+
 SRCDIR := $(shell dirname $(firstword $(MAKEFILE_LIST)))
 BUILDDIR := $(shell pwd)
 MAKEFILE_RULES := $(lastword $(MAKEFILE_LIST))
@@ -477,42 +475,6 @@
 	endif
 endif
 
-#--
-# Check if we have any C source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_C_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_C_SOURCES:.c=.o))
-endif
-
-#--
-# Check if we have any C++ source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_CXX_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_CXX_SOURCES:.cpp=.o))
-	CXX = $(call cxx_compiler,$(CC))
-	LD = $(call cxx_linker,$(CC))
-endif
-
-#--
-# Check if we have any ObjC source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_OBJC_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_OBJC_SOURCES:.m=.o))
-	LDFLAGS +=-lobjc
-endif
-
-#--
-# Check if we have any ObjC++ source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_OBJCXX_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_OBJCXX_SOURCES:.mm=.o))
-	CXX = $(call cxx_compiler,$(CC))
-	LD = $(call cxx_linker,$(CC))
-	ifeq "$(findstring lobjc,$(LDFLAGS))" ""
-		LDFLAGS +=-lobjc
-	endif
-endif
-
 ifeq ($(findstring clang, $(CXX)), clang)
 	CXXFLAGS += --driver-mode=g++
 endif
@@ -534,8 +496,8 @@
 #--
 ifneq "$(DYLIB_NAME)" ""
 ifeq "$(DYLIB_ONLY)" ""
-$(EXE) : $(OBJECTS) $(ARCHIVE_NAME) $(DYLIB_FILENAME)
-	$(LD) $(OBJECTS) $(ARCHIVE_NAME) -L. -l$(DYLIB_NAME) $(LDFLAGS) -o "$(EXE)"
+$(EXE) : $(OBJECTS) $(DYLIB_FILENAME)
+	$(LD) $(OBJECTS) -L. -l$(DYLIB_NAME) $(LDFLAGS) -o "$(EXE)"
 ifneq "$(CODESIGN)" ""
 	$(CODESIGN) -s - "$(EXE)"
 endif
@@ -543,8 +505,8 @@
 EXE = $(DYLIB_FILENAME)
 endif
 else
-$(EXE) : $(OBJECTS) $(ARCHIVE_NAME)
-	$(LD) $(OBJECTS) $(LDFLAGS) $(ARCHIVE_NAME) -o "$(EXE)"
+$(EXE) : $(OBJECTS)
+	$(LD) $(OBJECTS) $(LDFLAGS) -o "$(EXE)"
 ifneq "$(CODESIGN)" ""
 	$(CODESIGN) -s - "$(EXE)"
 endif
@@ -566,19 +528,6 @@
 endif
 endif
 
-#--
-# Make the archive
-#--
-ifneq "$(ARCHIVE_NAME)" ""
-ifeq "$(OS)" "Darwin"
-$(ARCHIVE_NAME) : $(ARCHIVE_OBJECTS)
-	$(AR) $(ARFLAGS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS)
-	$(RM) $(ARCHIVE_OBJECTS)
-else
-$(ARCHIVE_NAME) : $(foreach ar_obj,$(ARCHIVE_OBJECTS),$(ARCHIVE_NAME)($(ar_obj)))
-endif
-endif
-
 #--
 # Make the dylib
 #--
@@ -628,12 +577,22 @@
 # Make the precompiled header and compile C++ sources against it
 #--
 
-#ifneq "$(PCH_OUTPUT)" ""
+ifneq "$(PCH_OUTPUT)" ""
 $(PCH_OUTPUT) : $(PCH_CXX_SOURCE)
 	$(CXX) $(CXXFLAGS) -x c++-header -o $@ $<
-%.o : %.cpp $(PCH_OUTPUT)
-	$(CXX) $

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94890

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

[Lldb-commits] [lldb] 6afdf13 - Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test

[Fangrui Song via lldb-commits]

Author: Fangrui Song
Date: 2021-01-20T14:22:33-08:00
New Revision: 6afdf13ae4ccf00296065960a0b311c87e6f8dd5

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

LOG: Makefile.rules: Avoid redundant .d generation (make restart) and inline 
archive rule to the only test

Take an example when `CXX_SOURCES` is main.cpp.

main.d is an included file. make will rebuild main.d, re-executes itself [1] to 
read
in the new main.d file, then rebuild main.o, finally link main.o into a.out.
main.cpp is parsed twice in this process.

This patch merges .d generation into .o generation [2], writes explicit rules
for .c/.m and deletes suffix rules for %.m and %.o. Since a target can be
satisfied by either of .c/.cpp/.m/.mm, we use multiple pattern rules. The
rule with the prerequisite (with VPATH considered) satisfied is used [3].

Since suffix rules are disabled, the implicit rule for archive member targets is
no long available [4]. Rewrite, simplify the archive rule and inline it into the
only test `test/API/functionalities/archives/Makefile`.

[1]: https://www.gnu.org/software/make/manual/html_node/Remaking-Makefiles.html
[2]: http://make.mad-scientist.net/papers/advanced-auto-dependency-generation/
[3]: https://www.gnu.org/software/make/manual/html_node/Pattern-Match.html
[4]: https://www.gnu.org/software/make/manual/html_node/Archive-Update.html

ObjC/ObjCXX tests only run on macOS. I don't have testing environment.  Hope
someone can do it for me.

Reviewed By: JDevlieghere

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

Added: 


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

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules 
b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index d715f1ca24e4..374dd6865d88 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -26,15 +26,13 @@
 # SPLIT_DEBUG_SYMBOLS := YES
 # CROSS_COMPILE :=
 # USE_PRIVATE_MODULE_CACHE := YES
-#
-# And test/functionalities/archives/Makefile:
-# MAKE_DSYM := NO
-# ARCHIVE_NAME := libfoo.a
-# ARCHIVE_C_SOURCES := a.c b.c
 
 # Uncomment line below for debugging shell commands
 # SHELL = /bin/sh -x
 
+# Suppress built-in suffix rules. We explicitly define rules for %.o.
+.SUFFIXES:
+
 SRCDIR := $(shell dirname $(firstword $(MAKEFILE_LIST)))
 BUILDDIR := $(shell pwd)
 MAKEFILE_RULES := $(lastword $(MAKEFILE_LIST))
@@ -477,42 +475,6 @@ ifneq "$(strip $(OBJCXX_SOURCES))" ""
endif
 endif
 
-#--
-# Check if we have any C source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_C_SOURCES))" ""
-   ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_C_SOURCES:.c=.o))
-endif
-
-#--
-# Check if we have any C++ source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_CXX_SOURCES))" ""
-   ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_CXX_SOURCES:.cpp=.o))
-   CXX = $(call cxx_compiler,$(CC))
-   LD = $(call cxx_linker,$(CC))
-endif
-
-#--
-# Check if we have any ObjC source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_OBJC_SOURCES))" ""
-   ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_OBJC_SOURCES:.m=.o))
-   LDFLAGS +=-lobjc
-endif
-
-#--
-# Check if we have any ObjC++ source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_OBJCXX_SOURCES))" ""
-   ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_OBJCXX_SOURCES:.mm=.o))
-   CXX = $(call cxx_compiler,$(CC))
-   LD = $(call cxx_linker,$(CC))
-   ifeq "$(findstring lobjc,$(LDFLAGS))" ""
-   LDFLAGS +=-lobjc
-   endif
-endif
-
 ifeq ($(findstring clang, $(CXX)), clang)
CXXFLAGS += --driver-mode=g++
 endif
@@ -534,8 +496,8 @@ endif
 #--
 ifneq "$(DYLIB_NAME)" ""
 ifeq "$(DYLIB_ONLY)" ""
-$(EXE) : $(OBJECTS) $(ARCHIVE_NAME) $(DYLIB_FILENAME)
-   $(LD) $(OBJECTS) $(ARCHIVE_NAME) -L. -l$(DYLIB_NAME) $(LDFLAGS) -o 
"$(EXE)"
+$(EXE) : $(OBJECTS) $(DYLIB_FILENAME)
+   $(LD) $(OBJECTS) -L. -l$(DYLIB_NAME) $(LDFLAGS) -o "$(EXE)"
 ifneq "$(CODESIGN)" ""
$(CODESIGN) -s - "$(EXE)"
 endif
@@ -543,8 +505,8 @@ else
 EXE = $(DYLIB_FILENAME)
 endif
 else
-$(EXE) : $(

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test

[Fangrui Song 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 rG6afdf13ae4cc: Makefile.rules: Avoid redundant .d generation 
(make restart) and inline archive… (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94890

Files:
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  lldb/test/API/functionalities/archives/Makefile

Index: lldb/test/API/functionalities/archives/Makefile
===
--- lldb/test/API/functionalities/archives/Makefile
+++ lldb/test/API/functionalities/archives/Makefile
@@ -1,7 +1,14 @@
-C_SOURCES := main.c
-
+C_SOURCES := main.c a.c b.c
+EXE :=  # Define a.out explicitly
 MAKE_DSYM := NO
-ARCHIVE_NAME := libfoo.a
-ARCHIVE_C_SOURCES := a.c b.c
+
+all: a.out
+
+a.out: main.o libfoo.a
+	$(LD) $(LDFLAGS) $^ -o $@
+
+libfoo.a: a.o b.o
+	$(AR) $(ARFLAGS) $@ $^
+	$(RM) $^
 
 include Makefile.rules
Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -26,15 +26,13 @@
 # SPLIT_DEBUG_SYMBOLS := YES
 # CROSS_COMPILE :=
 # USE_PRIVATE_MODULE_CACHE := YES
-#
-# And test/functionalities/archives/Makefile:
-# MAKE_DSYM := NO
-# ARCHIVE_NAME := libfoo.a
-# ARCHIVE_C_SOURCES := a.c b.c
 
 # Uncomment line below for debugging shell commands
 # SHELL = /bin/sh -x
 
+# Suppress built-in suffix rules. We explicitly define rules for %.o.
+.SUFFIXES:
+
 SRCDIR := $(shell dirname $(firstword $(MAKEFILE_LIST)))
 BUILDDIR := $(shell pwd)
 MAKEFILE_RULES := $(lastword $(MAKEFILE_LIST))
@@ -477,42 +475,6 @@
 	endif
 endif
 
-#--
-# Check if we have any C source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_C_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_C_SOURCES:.c=.o))
-endif
-
-#--
-# Check if we have any C++ source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_CXX_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_CXX_SOURCES:.cpp=.o))
-	CXX = $(call cxx_compiler,$(CC))
-	LD = $(call cxx_linker,$(CC))
-endif
-
-#--
-# Check if we have any ObjC source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_OBJC_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_OBJC_SOURCES:.m=.o))
-	LDFLAGS +=-lobjc
-endif
-
-#--
-# Check if we have any ObjC++ source files for archive
-#--
-ifneq "$(strip $(ARCHIVE_OBJCXX_SOURCES))" ""
-	ARCHIVE_OBJECTS +=$(strip $(ARCHIVE_OBJCXX_SOURCES:.mm=.o))
-	CXX = $(call cxx_compiler,$(CC))
-	LD = $(call cxx_linker,$(CC))
-	ifeq "$(findstring lobjc,$(LDFLAGS))" ""
-		LDFLAGS +=-lobjc
-	endif
-endif
-
 ifeq ($(findstring clang, $(CXX)), clang)
 	CXXFLAGS += --driver-mode=g++
 endif
@@ -534,8 +496,8 @@
 #--
 ifneq "$(DYLIB_NAME)" ""
 ifeq "$(DYLIB_ONLY)" ""
-$(EXE) : $(OBJECTS) $(ARCHIVE_NAME) $(DYLIB_FILENAME)
-	$(LD) $(OBJECTS) $(ARCHIVE_NAME) -L. -l$(DYLIB_NAME) $(LDFLAGS) -o "$(EXE)"
+$(EXE) : $(OBJECTS) $(DYLIB_FILENAME)
+	$(LD) $(OBJECTS) -L. -l$(DYLIB_NAME) $(LDFLAGS) -o "$(EXE)"
 ifneq "$(CODESIGN)" ""
 	$(CODESIGN) -s - "$(EXE)"
 endif
@@ -543,8 +505,8 @@
 EXE = $(DYLIB_FILENAME)
 endif
 else
-$(EXE) : $(OBJECTS) $(ARCHIVE_NAME)
-	$(LD) $(OBJECTS) $(LDFLAGS) $(ARCHIVE_NAME) -o "$(EXE)"
+$(EXE) : $(OBJECTS)
+	$(LD) $(OBJECTS) $(LDFLAGS) -o "$(EXE)"
 ifneq "$(CODESIGN)" ""
 	$(CODESIGN) -s - "$(EXE)"
 endif
@@ -566,19 +528,6 @@
 endif
 endif
 
-#--
-# Make the archive
-#--
-ifneq "$(ARCHIVE_NAME)" ""
-ifeq "$(OS)" "Darwin"
-$(ARCHIVE_NAME) : $(ARCHIVE_OBJECTS)
-	$(AR) $(ARFLAGS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS)
-	$(RM) $(ARCHIVE_OBJECTS)
-else
-$(ARCHIVE_NAME) : $(foreach ar_obj,$(ARCHIVE_OBJECTS),$(ARCHIVE_NAME)($(ar_obj)))
-endif
-endif
-
 #--
 # Make the dylib
 #--
@@ -628,12 +577,22 @@
 # Make the precompiled header and compile C++ sources against it
 #--
 
-#ifneq "$(PCH_OUTPUT)" ""
+if

[Lldb-commits] [PATCH] D94997: [lldb][lldb-vscode] Updated implementation of 'launch' and 'attach' requests to not create auxiliary target in case "launchCommands" and "attachCommands" are provided.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D94997#2510144 , @serhiy.redko 
wrote:

> Thanks for review and your input, @clayborg
>
>> So I agree we need to fix the "settings set" issue as it surfaces a quirk in 
>> the order and number of targets we create. The main questions is if we care 
>> that we don't auto create a target when "launchCommands" are used and what 
>> the right solution is for lldb-vscode going forward.
>
> Please correct me if I'm wrong, but my understanding is: from lldb-vscode 
> launch.json configuration we generally should create a single target/process 
> which user will debug.

It just needs to debug one of the targets. My comments below detail why it was 
created this way, for better or worse.

> The fact that 'debugger' instance has several targets after 'launch', 
> 'attach' doesn't look correct to me, while things still work. It introduces 
> ambiguity and conundrum in implementation what exactly user wants to debug. 
> Do we have use cases for several targets in single lldb-vscode session?

We do not currently. The only reason the target was created when using 
"launchCommands" was to be able to have it just be one command:

  "launchCommands": ["gdb-remote :"]

I created "launchCommands" for this very case. That being said, it doesn't need 
to stay this way, just an explanation of why the target is created currently. 
Then someone added "attachCommands" and copied what "launchCommands" was doing, 
including the creation of the target. You could easily use the pre-created 
target with "attachCommands":

  "attachCommands": ["process attach --waitfor"]

Since the current stuff creates a target for you with the "program", it would 
know to use the program's path as the name of the process to attach to. Again, 
that is just how it is currently coded and we are here to fix this in this 
patch.

> If we need to debug several processes, my understanding is we need to launch 
> several lldb-vscode instances for each process to debug, is it correct?

We don't need them, it was just out I originally coded "launchCommands" because 
my primary use case was for attaching to a remote GDB server for android apps.

> So ideally from launch.json we should construct single target and use it.

A single target will solve any "settings set target." issues, so I am fine 
with any solution that can create one target.

> So I think it is important for user to have a clear understanding about the 
> way the debug target is created. 
> Now it can be created with:
>
> 1. 'convenience' fields like "program", "pid"..,
> 2. 'attachCommands'/'launchCommands'
> 3. potentially with any other fields that execute lldb commands e.g 
> "initCommands"

A target can be created with "initCommands" but should be discouraged. Maybe we 
need to add something to the description for "initCommands" in package.json

> I assume all we want is to get clear instructions from user about final 
> target, but not a mix of all ways listed above to create target and use last 
> to work with. Otherwise it will be confusion like why target I specified with 
> my "program" field is ignored when I provided 'launchCommands' that also 
> create target?

Yes, we should figure out the best solution that makes sense as I agree that 
the way it is coded right now has issues.

> With this change I try to resolve ambiguity for user and implementation on 
> the approach to create the debug target and don't break many existing use 
> cases.
>
> So I suggest to document and 'enforce' the following:
>
> 1. 'convenience' fields like "program", "pid".. for which we create auto 
> target because eventually we need a target.
> 2. 'attachCommands'/'launchCommands' where user will be responsible to 
> provide LLDB commands that will create a target. No auto target is needed.
>
> #1 and #2 will be stated as mutually exclusive in documentation and made in 
> implementation.
>
> I'm not sure we can 'enforce' user to not create target in "initCommands" 
> (unless we assert(debugger.GetTargetsNum() == 1)) and other similar 
> launch.json fields but it should be easy to document.

It might be nice to check and log a warning to the console if the does this, 
but yes, hard to enforce and we don't want to take away someone's ability to do 
what they need to to get something working.

>> Another solution would be to not auto create a target if the "program" 
>> argument is missing from the launch config and document this in the 
>> "attachCommands" or "launchCommands"? Or is "program" a required launch 
>> config argument?
>
> While "program" is listed as required in launch.json it is not required in 
> case 'launchCommands' create a target, so we can make it optional. 
> Since there are many target creating fields like "program" with described 
> changes it will be easier to check whether  'attachCommands'/'launchCommands' 
> are provided and avoid auto target creation.
>
> I agree that we need to do sanitization of input para

[Lldb-commits] [PATCH] D94997: [lldb][lldb-vscode] Updated implementation of 'launch' and 'attach' requests to not create auxiliary target in case "launchCommands" and "attachCommands" are provided.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D94997#2509175 , @tatyana-krasnukha 
wrote:

> D92164  was intended for fixing the 
> "settings set" issue, however, it revealed some deadlocks and data races, and 
> had to be reverted temporarily. Currently, I'm working on those 
> multithreading issues and will submit a patch as soon as possible.

This settings set issue is not related to your diff, it is just because a 
target is created, the auto created target gets the settings set on it, and any 
new target created by "launchCommands" or "attachCommands" would not due to the 
way settings work in general.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94997

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

[Lldb-commits] [PATCH] D95100: [lldb/Commands] Fix short option collision for `process launch`

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: JDevlieghere, jingham, teemperor.
Herald added a subscriber: dang.
mib requested review of this revision.
Herald added a project: LLDB.

This patch changes the short option used in the
CommandOptionsProcessLaunch for the `-v|--environment` command option to
`-V|--environment`.

The reason for that is, that it collides with the `-v|structured-data-value`
command option generated by `OptionGroupPythonClassWithDict` that
I'm using in an upcoming patch for the `process launch` command.

The long option `--environment` remains the same.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95100

Files:
  lldb/docs/use/map.rst
  lldb/source/Commands/CommandOptionsProcessLaunch.cpp
  lldb/source/Commands/Options.td


Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -655,7 +655,7 @@
 Desc<"Set the current working directory to  when running the 
inferior.">;
   def process_launch_arch : Option<"arch", "a">, Arg<"Architecture">,
 Desc<"Set the architecture for the process to launch when ambiguous.">;
-  def process_launch_environment : Option<"environment", "v">,
+  def process_launch_environment : Option<"environment", "V">,
 Arg<"None">, Desc<"Specify an environment variable name/value string "
 "(--environment NAME=VALUE). Can be specified multiple times for 
subsequent "
 "environment entries.">;
Index: lldb/source/Commands/CommandOptionsProcessLaunch.cpp
===
--- lldb/source/Commands/CommandOptionsProcessLaunch.cpp
+++ lldb/source/Commands/CommandOptionsProcessLaunch.cpp
@@ -130,7 +130,7 @@
   launch_info.SetShell(HostInfo::GetDefaultShell());
 break;
 
-  case 'v':
+  case 'V':
 launch_info.GetEnvironment().insert(option_arg);
 break;
 
Index: lldb/docs/use/map.rst
===
--- lldb/docs/use/map.rst
+++ lldb/docs/use/map.rst
@@ -199,7 +199,7 @@



-  (lldb) process launch -v DEBUG=1
+  (lldb) process launch -V DEBUG=1
   

  


Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -655,7 +655,7 @@
 Desc<"Set the current working directory to  when running the inferior.">;
   def process_launch_arch : Option<"arch", "a">, Arg<"Architecture">,
 Desc<"Set the architecture for the process to launch when ambiguous.">;
-  def process_launch_environment : Option<"environment", "v">,
+  def process_launch_environment : Option<"environment", "V">,
 Arg<"None">, Desc<"Specify an environment variable name/value string "
 "(--environment NAME=VALUE). Can be specified multiple times for subsequent "
 "environment entries.">;
Index: lldb/source/Commands/CommandOptionsProcessLaunch.cpp
===
--- lldb/source/Commands/CommandOptionsProcessLaunch.cpp
+++ lldb/source/Commands/CommandOptionsProcessLaunch.cpp
@@ -130,7 +130,7 @@
   launch_info.SetShell(HostInfo::GetDefaultShell());
 break;
 
-  case 'v':
+  case 'V':
 launch_info.GetEnvironment().insert(option_arg);
 break;
 
Index: lldb/docs/use/map.rst
===
--- lldb/docs/use/map.rst
+++ lldb/docs/use/map.rst
@@ -199,7 +199,7 @@



-  (lldb) process launch -v DEBUG=1
+  (lldb) process launch -V DEBUG=1
   

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

[Lldb-commits] [PATCH] D95100: [lldb/Commands] Fix short option collision for `process launch`

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

@jingham should weigh in here, but I think we kind of guarantee (at least 
informally) that the command options to be stable. Is there an alternative way 
to fix this without breaking the existing option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95100

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

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I replied before I actually tried to understand what your'e trying to achieve, 
and it's still not entirely clear to me. I think what you need is something 
similar like `python-typemaps.swig` which helps swig understand `char**` types 
and such.

For context, originally SWIG wasn't great at handling `std::string`s (which has 
since been fixed) and the designers of the SB API were worried about the ABI 
stability of the `std::string` class. None of these are concerns anymore today, 
but the ABI being stable means we're stuck with it for now. If we ever do an SB 
API v2, this is definitely one of the things we'd fix, but currently there are 
no plans for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

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

[Lldb-commits] [PATCH] D95100: [lldb/Commands] Fix short option collision for `process launch`

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

We shouldn't lightly change options that are commonly used.  But we haven't 
made a strong statement about this the way we do for API.  I'm more concerned 
about people having to rewrite/build and distribute their code to accommodate a 
new lldb than having to learn a new character for a not very commonly used 
option.

I think it's more common to run `env FOO=Bar` or set the env-vars.  And 
building up complex "process launch" commands doesn't seem like the best way to 
use lldb in a scripted manner.  So this seems like a pretty benign change to me.

I would really like it if wherever we are introducing a scripted plugin to 
lldb, we pass its arguments in the same way.  Since we're passing in 
dictionaries as the most flexible way to muster configuration info into the 
script interpreter, so --key --value are the most natural options, and so -k 
and -v...  I'd really rather not have this be different across commands.  OTOH 
-v doesn't really scream "environment" and we don't use that as an option 
anywhere else, so it seems more natural to change --environment's short option. 
 Might be good to change it to -E not -V.  -e wasn't free because we use it for 
stderr everywhere, but -E is free and makes more sense.

But it might be worthwhile to bring this up on lldb-dev to see if anybody has 
reasons not to do this that I haven't thought about?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95100

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

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

In D94937#2511222 , @JDevlieghere 
wrote:

> I replied before I actually tried to understand what your'e trying to 
> achieve, and it's still not entirely clear to me. I think what you need is 
> something similar like `python-typemaps.swig` which helps swig understand 
> `char**` types and such.
>
> For context, originally SWIG wasn't great at handling `std::string`s (which 
> has since been fixed) and the designers of the SB API were worried about the 
> ABI stability of the `std::string` class. None of these are concerns anymore 
> today, but the ABI being stable means we're stuck with it for now. If we ever 
> do an SB API v2, this is definitely one of the things we'd fix, but currently 
> there are no plans for that.

That's exactly what I need, thanks. I will adjust accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

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

[Lldb-commits] [PATCH] D95100: [lldb/Commands] Fix short option collision for `process launch`

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 318072.
mib added a comment.

Changed short option to `-E|--environment` as @jingham suggested it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95100

Files:
  lldb/docs/use/map.rst
  lldb/source/Commands/CommandOptionsProcessLaunch.cpp
  lldb/source/Commands/Options.td


Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -655,7 +655,7 @@
 Desc<"Set the current working directory to  when running the 
inferior.">;
   def process_launch_arch : Option<"arch", "a">, Arg<"Architecture">,
 Desc<"Set the architecture for the process to launch when ambiguous.">;
-  def process_launch_environment : Option<"environment", "v">,
+  def process_launch_environment : Option<"environment", "E">,
 Arg<"None">, Desc<"Specify an environment variable name/value string "
 "(--environment NAME=VALUE). Can be specified multiple times for 
subsequent "
 "environment entries.">;
Index: lldb/source/Commands/CommandOptionsProcessLaunch.cpp
===
--- lldb/source/Commands/CommandOptionsProcessLaunch.cpp
+++ lldb/source/Commands/CommandOptionsProcessLaunch.cpp
@@ -130,7 +130,7 @@
   launch_info.SetShell(HostInfo::GetDefaultShell());
 break;
 
-  case 'v':
+  case 'E':
 launch_info.GetEnvironment().insert(option_arg);
 break;
 
Index: lldb/docs/use/map.rst
===
--- lldb/docs/use/map.rst
+++ lldb/docs/use/map.rst
@@ -199,7 +199,7 @@



-  (lldb) process launch -v DEBUG=1
+  (lldb) process launch -E DEBUG=1
   

  


Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -655,7 +655,7 @@
 Desc<"Set the current working directory to  when running the inferior.">;
   def process_launch_arch : Option<"arch", "a">, Arg<"Architecture">,
 Desc<"Set the architecture for the process to launch when ambiguous.">;
-  def process_launch_environment : Option<"environment", "v">,
+  def process_launch_environment : Option<"environment", "E">,
 Arg<"None">, Desc<"Specify an environment variable name/value string "
 "(--environment NAME=VALUE). Can be specified multiple times for subsequent "
 "environment entries.">;
Index: lldb/source/Commands/CommandOptionsProcessLaunch.cpp
===
--- lldb/source/Commands/CommandOptionsProcessLaunch.cpp
+++ lldb/source/Commands/CommandOptionsProcessLaunch.cpp
@@ -130,7 +130,7 @@
   launch_info.SetShell(HostInfo::GetDefaultShell());
 break;
 
-  case 'v':
+  case 'E':
 launch_info.GetEnvironment().insert(option_arg);
 break;
 
Index: lldb/docs/use/map.rst
===
--- lldb/docs/use/map.rst
+++ lldb/docs/use/map.rst
@@ -199,7 +199,7 @@



-  (lldb) process launch -v DEBUG=1
+  (lldb) process launch -E DEBUG=1
   

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

[Lldb-commits] [lldb] bff3891 - Fix a bug with setting breakpoints on C++11 inline initialization statements.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2021-01-20T17:58:34-08:00
New Revision: bff389120fa2368d123612449c938958cfd7f45e

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

LOG: Fix a bug with setting breakpoints on C++11 inline initialization 
statements.

If they occurred before the constructor that used them, we would refuse to
set the breakpoint because we thought they were crossing function boundaries.

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

Added: 
lldb/test/API/lang/cpp/break-on-initializers/Makefile
lldb/test/API/lang/cpp/break-on-initializers/TestBreakOnCPP11Initializers.py
lldb/test/API/lang/cpp/break-on-initializers/main.cpp

Modified: 
lldb/source/Breakpoint/BreakpointResolverFileLine.cpp

Removed: 




diff  --git a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp 
b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
index 22a4b4ae33ae..5ca4ef5834e0 100644
--- a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
@@ -187,6 +187,14 @@ void 
BreakpointResolverFileLine::FilterContexts(SymbolContextList &sc_list,
 // is 0, then we can't do this calculation.  That can happen if
 // GetStartLineSourceInfo gets an error, or if the first line number in
 // the function really is 0 - which happens for some languages.
+
+// But only do this calculation if the line number we found in the SC
+// was 
diff erent from the one requested in the source file.  If we actually
+// found an exact match it must be valid.
+
+if (m_line_number == sc.line_entry.line)
+  continue;
+
 const int decl_line_is_too_late_fudge = 1;
 if (line && m_line_number < line - decl_line_is_too_late_fudge) {
   LLDB_LOG(log, "removing symbol context at {0}:{1}", file, line);

diff  --git a/lldb/test/API/lang/cpp/break-on-initializers/Makefile 
b/lldb/test/API/lang/cpp/break-on-initializers/Makefile
new file mode 100644
index ..7714f26e52bc
--- /dev/null
+++ b/lldb/test/API/lang/cpp/break-on-initializers/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CXXFLAGS_EXTRAS := -std=c++11
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/lang/cpp/break-on-initializers/TestBreakOnCPP11Initializers.py 
b/lldb/test/API/lang/cpp/break-on-initializers/TestBreakOnCPP11Initializers.py
new file mode 100644
index ..8456a7cae96e
--- /dev/null
+++ 
b/lldb/test/API/lang/cpp/break-on-initializers/TestBreakOnCPP11Initializers.py
@@ -0,0 +1,52 @@
+"""
+When using C++11 in place member initialization, show that we
+can set and hit breakpoints on initialization lines.  This is a
+little bit tricky because we try not to move file and line breakpoints 
+across function boundaries but these lines are outside the source range
+of the constructor.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class RenameThisSampleTestTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_breakpoints_on_initializers(self):
+"""Show we can set breakpoints on initializers appearing both before
+   and after the constructor body, and hit them."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.cpp")
+self.first_initializer_line = line_number("main.cpp", "Set the before 
constructor breakpoint here")
+self.second_initializer_line = line_number("main.cpp", "Set the after 
constructor breakpoint here")
+self.sample_test()
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Set up your test case here. If your test doesn't need any set up then
+# remove this method from your TestCase class.
+
+def sample_test(self):
+"""You might use the test implementation in several ways, say so 
here."""
+
+(target, process, thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
+   " Set a breakpoint here to get started", 
self.main_source_file)
+
+# Now set breakpoints on the two initializer lines we found in the 
test startup:
+bkpt1 = target.BreakpointCreateByLocation(self.main_source_file, 
self.first_initializer_line)
+self.assertEqual(bkpt1.GetNumLocations(), 1)
+bkpt2 = target.BreakpointCreateByLocation(self.main_source_file, 
self.second_initializer_line)
+self.assertEqual(bkpt2.GetNumLocations(), 1)
+
+# Now continue, we should stop at the two breakpoints above, first the 
one before, then
+# the one after.
+self.assertEqual(len(lldbutil.continue_to_breakpoint(process, bkpt1)), 
1, "Hit first breakpoint")
+self.assertEqual(len(lldbutil.continue_to_breakpoint(process, bkpt2

[Lldb-commits] [PATCH] D94846: Allow breakpoints to be set on C++11 inline initializers

[Jim Ingham via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbff389120fa2: Fix a bug with setting breakpoints on C++11 
inline initialization statements. (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94846

Files:
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/test/API/lang/cpp/break-on-initializers/Makefile
  lldb/test/API/lang/cpp/break-on-initializers/TestBreakOnCPP11Initializers.py
  lldb/test/API/lang/cpp/break-on-initializers/main.cpp

Index: lldb/test/API/lang/cpp/break-on-initializers/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/break-on-initializers/main.cpp
@@ -0,0 +1,31 @@
+#include 
+#include 
+
+class Trivial {
+public:
+  Trivial(int input) : m_int(input) {}
+private:
+  int m_int;
+
+};
+
+class Foo {
+private:
+  Trivial m_trivial = Trivial(100); // Set the before constructor breakpoint here
+
+public:
+  Foo(int input) {
+printf("I have been made!\n");
+  }
+
+private:
+  Trivial m_other_trivial = Trivial(200); // Set the after constructor breakpoint here
+};
+
+int
+main()
+{
+  Foo myFoo(10); // Set a breakpoint here to get started
+  return 0;
+}
+
Index: lldb/test/API/lang/cpp/break-on-initializers/TestBreakOnCPP11Initializers.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/break-on-initializers/TestBreakOnCPP11Initializers.py
@@ -0,0 +1,52 @@
+"""
+When using C++11 in place member initialization, show that we
+can set and hit breakpoints on initialization lines.  This is a
+little bit tricky because we try not to move file and line breakpoints 
+across function boundaries but these lines are outside the source range
+of the constructor.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class RenameThisSampleTestTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_breakpoints_on_initializers(self):
+"""Show we can set breakpoints on initializers appearing both before
+   and after the constructor body, and hit them."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.cpp")
+self.first_initializer_line = line_number("main.cpp", "Set the before constructor breakpoint here")
+self.second_initializer_line = line_number("main.cpp", "Set the after constructor breakpoint here")
+self.sample_test()
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Set up your test case here. If your test doesn't need any set up then
+# remove this method from your TestCase class.
+
+def sample_test(self):
+"""You might use the test implementation in several ways, say so here."""
+
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+   " Set a breakpoint here to get started", self.main_source_file)
+
+# Now set breakpoints on the two initializer lines we found in the test startup:
+bkpt1 = target.BreakpointCreateByLocation(self.main_source_file, self.first_initializer_line)
+self.assertEqual(bkpt1.GetNumLocations(), 1)
+bkpt2 = target.BreakpointCreateByLocation(self.main_source_file, self.second_initializer_line)
+self.assertEqual(bkpt2.GetNumLocations(), 1)
+
+# Now continue, we should stop at the two breakpoints above, first the one before, then
+# the one after.
+self.assertEqual(len(lldbutil.continue_to_breakpoint(process, bkpt1)), 1, "Hit first breakpoint")
+self.assertEqual(len(lldbutil.continue_to_breakpoint(process, bkpt2)), 1, "Hit second breakpoint")
+
+
Index: lldb/test/API/lang/cpp/break-on-initializers/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/break-on-initializers/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CXXFLAGS_EXTRAS := -std=c++11
+
+include Makefile.rules
Index: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
+++ lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
@@ -187,6 +187,14 @@
 // is 0, then we can't do this calculation.  That can happen if
 // GetStartLineSourceInfo gets an error, or if the first line number in
 // the function really is 0 - which happens for some languages.
+
+// But only do this calculation if the line number we found in the SC
+// was different from the one requested in the source file.  If we actually
+// found an exact match 

[Lldb-commits] [PATCH] D95110: [lldb] Upstream eCore_arm_arm64e enum value

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: jasonmolenda.
Herald added subscribers: pzheng, kristof.beyls.
JDevlieghere requested review of this revision.

Upstream the `eCore_arm_arm64e` in ArchSpec.


https://reviews.llvm.org/D95110

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/Utility/ArchSpec.cpp


Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -99,8 +99,10 @@
  ArchSpec::eCore_arm_arm64, "arm64"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
  ArchSpec::eCore_arm_armv8, "armv8"},
-{eByteOrderLittle, 4, 2, 4, llvm::Triple::arm,
- ArchSpec::eCore_arm_armv8l, "armv8l"},
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arm, ArchSpec::eCore_arm_armv8l,
+ "armv8l"},
+{eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
+ ArchSpec::eCore_arm_arm64e, "arm64e"},
 {eByteOrderLittle, 4, 4, 4, llvm::Triple::aarch64_32,
  ArchSpec::eCore_arm_arm64_32, "arm64_32"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
@@ -288,8 +290,7 @@
 {ArchSpec::eCore_arm_armv7k,  llvm::MachO::CPU_TYPE_ARM,
llvm::MachO::CPU_SUBTYPE_ARM_V7K,   UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_armv7m,  llvm::MachO::CPU_TYPE_ARM,
llvm::MachO::CPU_SUBTYPE_ARM_V7M,   UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_armv7em, llvm::MachO::CPU_TYPE_ARM,
llvm::MachO::CPU_SUBTYPE_ARM_V7EM,  UINT32_MAX, SUBTYPE_MASK},
-// FIXME: This should be arm64e once the triple exists.
-{ArchSpec::eCore_arm_arm64,   llvm::MachO::CPU_TYPE_ARM64,  
llvm::MachO::CPU_SUBTYPE_ARM64E,UINT32_MAX, SUBTYPE_MASK},
+{ArchSpec::eCore_arm_arm64e,  llvm::MachO::CPU_TYPE_ARM64,  
llvm::MachO::CPU_SUBTYPE_ARM64E,UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_arm64,   llvm::MachO::CPU_TYPE_ARM64,  
llvm::MachO::CPU_SUBTYPE_ARM64_V8,  UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_arm64,   llvm::MachO::CPU_TYPE_ARM64,  
llvm::MachO::CPU_SUBTYPE_ARM64_ALL, UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_arm64,   llvm::MachO::CPU_TYPE_ARM64,  13,
 UINT32_MAX, SUBTYPE_MASK},
@@ -1198,16 +1199,31 @@
 return true;
   if (core2 == ArchSpec::eCore_arm_aarch64)
 return true;
+  if (core2 == ArchSpec::eCore_arm_arm64e)
+return true;
   try_inverse = false;
 }
 break;
 
+  case ArchSpec::eCore_arm_arm64e:
+if (!enforce_exact_match) {
+  if (core2 == ArchSpec::eCore_arm_arm64)
+return true;
+  if (core2 == ArchSpec::eCore_arm_aarch64)
+return true;
+  if (core2 == ArchSpec::eCore_arm_armv8)
+return true;
+  try_inverse = false;
+}
+break;
   case ArchSpec::eCore_arm_aarch64:
 if (!enforce_exact_match) {
   if (core2 == ArchSpec::eCore_arm_arm64)
 return true;
   if (core2 == ArchSpec::eCore_arm_armv8)
 return true;
+  if (core2 == ArchSpec::eCore_arm_arm64e)
+return true;
   try_inverse = false;
 }
 break;
@@ -1218,6 +1234,8 @@
 return true;
   if (core2 == ArchSpec::eCore_arm_armv8)
 return true;
+  if (core2 == ArchSpec::eCore_arm_arm64e)
+return true;
   try_inverse = false;
 }
 break;
Index: lldb/include/lldb/Utility/ArchSpec.h
===
--- lldb/include/lldb/Utility/ArchSpec.h
+++ lldb/include/lldb/Utility/ArchSpec.h
@@ -131,6 +131,7 @@
 eCore_arm_arm64,
 eCore_arm_armv8,
 eCore_arm_armv8l,
+eCore_arm_arm64e,
 eCore_arm_arm64_32,
 eCore_arm_aarch64,
 


Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -99,8 +99,10 @@
  ArchSpec::eCore_arm_arm64, "arm64"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
  ArchSpec::eCore_arm_armv8, "armv8"},
-{eByteOrderLittle, 4, 2, 4, llvm::Triple::arm,
- ArchSpec::eCore_arm_armv8l, "armv8l"},
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arm, ArchSpec::eCore_arm_armv8l,
+ "armv8l"},
+{eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
+ ArchSpec::eCore_arm_arm64e, "arm64e"},
 {eByteOrderLittle, 4, 4, 4, llvm::Triple::aarch64_32,
  ArchSpec::eCore_arm_arm64_32, "arm64_32"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
@@ -288,8 +290,7 @@
 {ArchSpec::eCore_arm_armv7k,  llvm::MachO::CPU_TYPE_ARM,llvm::MachO::CPU_SUBTYPE_ARM_V7K,   UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_armv7m,  llvm::MachO::CPU_TYPE_ARM,llvm::MachO::CPU_SUBTYPE_ARM_V7M,   UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_armv7em, llvm::MachO::CPU_TYPE_ARM,  

[Lldb-commits] [lldb] 98feb08 - Use CXX_SOURCES and point to the right source file.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2021-01-20T18:38:07-08:00
New Revision: 98feb08e449f179c3c5ccc6878c31cf16c160b06

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

LOG: Use CXX_SOURCES and point to the right source file.

Copy paste error, but the test still built on macOS.  Weird.
It failed on debian linux with an error about -fno-limit-debug-info
not being a supported flag???  Not sure how this goof would cause
that error, but let's see if it did...

Added: 


Modified: 
lldb/test/API/lang/cpp/break-on-initializers/Makefile

Removed: 




diff  --git a/lldb/test/API/lang/cpp/break-on-initializers/Makefile 
b/lldb/test/API/lang/cpp/break-on-initializers/Makefile
index 7714f26e52bc..e78030cbf752 100644
--- a/lldb/test/API/lang/cpp/break-on-initializers/Makefile
+++ b/lldb/test/API/lang/cpp/break-on-initializers/Makefile
@@ -1,4 +1,4 @@
-C_SOURCES := main.c
+CXX_SOURCES := main.cpp
 CXXFLAGS_EXTRAS := -std=c++11
 
 include Makefile.rules



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

[Lldb-commits] [PATCH] D95110: [lldb] Upstream eCore_arm_arm64e enum value

[Jason Molenda via Phabricator via lldb-commits]

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

Looks good.


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

https://reviews.llvm.org/D95110

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

[Lldb-commits] [lldb] baf6c29 - [lldb] Upstream eCore_arm_arm64e enum value in ArchSpec

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-01-20T19:39:47-08:00
New Revision: baf6c2987e576e319857c586120e98e917d8b47f

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

LOG: [lldb] Upstream eCore_arm_arm64e enum value in ArchSpec

Upstream the eCore_arm_arm64e enum value in ArchSpec. All the other
arm64e triple changes already landed in LLVM.

Differential revision: https://reviews.llvm.org/D95110

Added: 


Modified: 
lldb/include/lldb/Utility/ArchSpec.h
lldb/source/Utility/ArchSpec.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/ArchSpec.h 
b/lldb/include/lldb/Utility/ArchSpec.h
index b35766d3d9cf..fdfe6aceb033 100644
--- a/lldb/include/lldb/Utility/ArchSpec.h
+++ b/lldb/include/lldb/Utility/ArchSpec.h
@@ -131,6 +131,7 @@ class ArchSpec {
 eCore_arm_arm64,
 eCore_arm_armv8,
 eCore_arm_armv8l,
+eCore_arm_arm64e,
 eCore_arm_arm64_32,
 eCore_arm_aarch64,
 

diff  --git a/lldb/source/Utility/ArchSpec.cpp 
b/lldb/source/Utility/ArchSpec.cpp
index 8025f37c4b38..c13e2389cfed 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -99,8 +99,10 @@ static const CoreDefinition g_core_definitions[] = {
  ArchSpec::eCore_arm_arm64, "arm64"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
  ArchSpec::eCore_arm_armv8, "armv8"},
-{eByteOrderLittle, 4, 2, 4, llvm::Triple::arm,
- ArchSpec::eCore_arm_armv8l, "armv8l"},
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arm, ArchSpec::eCore_arm_armv8l,
+ "armv8l"},
+{eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
+ ArchSpec::eCore_arm_arm64e, "arm64e"},
 {eByteOrderLittle, 4, 4, 4, llvm::Triple::aarch64_32,
  ArchSpec::eCore_arm_arm64_32, "arm64_32"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
@@ -288,8 +290,7 @@ static const ArchDefinitionEntry g_macho_arch_entries[] = {
 {ArchSpec::eCore_arm_armv7k,  llvm::MachO::CPU_TYPE_ARM,
llvm::MachO::CPU_SUBTYPE_ARM_V7K,   UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_armv7m,  llvm::MachO::CPU_TYPE_ARM,
llvm::MachO::CPU_SUBTYPE_ARM_V7M,   UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_armv7em, llvm::MachO::CPU_TYPE_ARM,
llvm::MachO::CPU_SUBTYPE_ARM_V7EM,  UINT32_MAX, SUBTYPE_MASK},
-// FIXME: This should be arm64e once the triple exists.
-{ArchSpec::eCore_arm_arm64,   llvm::MachO::CPU_TYPE_ARM64,  
llvm::MachO::CPU_SUBTYPE_ARM64E,UINT32_MAX, SUBTYPE_MASK},
+{ArchSpec::eCore_arm_arm64e,  llvm::MachO::CPU_TYPE_ARM64,  
llvm::MachO::CPU_SUBTYPE_ARM64E,UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_arm64,   llvm::MachO::CPU_TYPE_ARM64,  
llvm::MachO::CPU_SUBTYPE_ARM64_V8,  UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_arm64,   llvm::MachO::CPU_TYPE_ARM64,  
llvm::MachO::CPU_SUBTYPE_ARM64_ALL, UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_arm64,   llvm::MachO::CPU_TYPE_ARM64,  13,
 UINT32_MAX, SUBTYPE_MASK},
@@ -1198,16 +1199,31 @@ static bool cores_match(const ArchSpec::Core core1, 
const ArchSpec::Core core2,
 return true;
   if (core2 == ArchSpec::eCore_arm_aarch64)
 return true;
+  if (core2 == ArchSpec::eCore_arm_arm64e)
+return true;
   try_inverse = false;
 }
 break;
 
+  case ArchSpec::eCore_arm_arm64e:
+if (!enforce_exact_match) {
+  if (core2 == ArchSpec::eCore_arm_arm64)
+return true;
+  if (core2 == ArchSpec::eCore_arm_aarch64)
+return true;
+  if (core2 == ArchSpec::eCore_arm_armv8)
+return true;
+  try_inverse = false;
+}
+break;
   case ArchSpec::eCore_arm_aarch64:
 if (!enforce_exact_match) {
   if (core2 == ArchSpec::eCore_arm_arm64)
 return true;
   if (core2 == ArchSpec::eCore_arm_armv8)
 return true;
+  if (core2 == ArchSpec::eCore_arm_arm64e)
+return true;
   try_inverse = false;
 }
 break;
@@ -1218,6 +1234,8 @@ static bool cores_match(const ArchSpec::Core core1, const 
ArchSpec::Core core2,
 return true;
   if (core2 == ArchSpec::eCore_arm_armv8)
 return true;
+  if (core2 == ArchSpec::eCore_arm_arm64e)
+return true;
   try_inverse = false;
 }
 break;



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

[Lldb-commits] [PATCH] D95110: [lldb] Upstream eCore_arm_arm64e enum value

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbaf6c2987e57: [lldb] Upstream eCore_arm_arm64e enum value in 
ArchSpec (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95110

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/Utility/ArchSpec.cpp


Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -99,8 +99,10 @@
  ArchSpec::eCore_arm_arm64, "arm64"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
  ArchSpec::eCore_arm_armv8, "armv8"},
-{eByteOrderLittle, 4, 2, 4, llvm::Triple::arm,
- ArchSpec::eCore_arm_armv8l, "armv8l"},
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arm, ArchSpec::eCore_arm_armv8l,
+ "armv8l"},
+{eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
+ ArchSpec::eCore_arm_arm64e, "arm64e"},
 {eByteOrderLittle, 4, 4, 4, llvm::Triple::aarch64_32,
  ArchSpec::eCore_arm_arm64_32, "arm64_32"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
@@ -288,8 +290,7 @@
 {ArchSpec::eCore_arm_armv7k,  llvm::MachO::CPU_TYPE_ARM,
llvm::MachO::CPU_SUBTYPE_ARM_V7K,   UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_armv7m,  llvm::MachO::CPU_TYPE_ARM,
llvm::MachO::CPU_SUBTYPE_ARM_V7M,   UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_armv7em, llvm::MachO::CPU_TYPE_ARM,
llvm::MachO::CPU_SUBTYPE_ARM_V7EM,  UINT32_MAX, SUBTYPE_MASK},
-// FIXME: This should be arm64e once the triple exists.
-{ArchSpec::eCore_arm_arm64,   llvm::MachO::CPU_TYPE_ARM64,  
llvm::MachO::CPU_SUBTYPE_ARM64E,UINT32_MAX, SUBTYPE_MASK},
+{ArchSpec::eCore_arm_arm64e,  llvm::MachO::CPU_TYPE_ARM64,  
llvm::MachO::CPU_SUBTYPE_ARM64E,UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_arm64,   llvm::MachO::CPU_TYPE_ARM64,  
llvm::MachO::CPU_SUBTYPE_ARM64_V8,  UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_arm64,   llvm::MachO::CPU_TYPE_ARM64,  
llvm::MachO::CPU_SUBTYPE_ARM64_ALL, UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_arm64,   llvm::MachO::CPU_TYPE_ARM64,  13,
 UINT32_MAX, SUBTYPE_MASK},
@@ -1198,16 +1199,31 @@
 return true;
   if (core2 == ArchSpec::eCore_arm_aarch64)
 return true;
+  if (core2 == ArchSpec::eCore_arm_arm64e)
+return true;
   try_inverse = false;
 }
 break;
 
+  case ArchSpec::eCore_arm_arm64e:
+if (!enforce_exact_match) {
+  if (core2 == ArchSpec::eCore_arm_arm64)
+return true;
+  if (core2 == ArchSpec::eCore_arm_aarch64)
+return true;
+  if (core2 == ArchSpec::eCore_arm_armv8)
+return true;
+  try_inverse = false;
+}
+break;
   case ArchSpec::eCore_arm_aarch64:
 if (!enforce_exact_match) {
   if (core2 == ArchSpec::eCore_arm_arm64)
 return true;
   if (core2 == ArchSpec::eCore_arm_armv8)
 return true;
+  if (core2 == ArchSpec::eCore_arm_arm64e)
+return true;
   try_inverse = false;
 }
 break;
@@ -1218,6 +1234,8 @@
 return true;
   if (core2 == ArchSpec::eCore_arm_armv8)
 return true;
+  if (core2 == ArchSpec::eCore_arm_arm64e)
+return true;
   try_inverse = false;
 }
 break;
Index: lldb/include/lldb/Utility/ArchSpec.h
===
--- lldb/include/lldb/Utility/ArchSpec.h
+++ lldb/include/lldb/Utility/ArchSpec.h
@@ -131,6 +131,7 @@
 eCore_arm_arm64,
 eCore_arm_armv8,
 eCore_arm_armv8l,
+eCore_arm_arm64e,
 eCore_arm_arm64_32,
 eCore_arm_aarch64,
 


Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -99,8 +99,10 @@
  ArchSpec::eCore_arm_arm64, "arm64"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
  ArchSpec::eCore_arm_armv8, "armv8"},
-{eByteOrderLittle, 4, 2, 4, llvm::Triple::arm,
- ArchSpec::eCore_arm_armv8l, "armv8l"},
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arm, ArchSpec::eCore_arm_armv8l,
+ "armv8l"},
+{eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
+ ArchSpec::eCore_arm_arm64e, "arm64e"},
 {eByteOrderLittle, 4, 4, 4, llvm::Triple::aarch64_32,
  ArchSpec::eCore_arm_arm64_32, "arm64_32"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::aarch64,
@@ -288,8 +290,7 @@
 {ArchSpec::eCore_arm_armv7k,  llvm::MachO::CPU_TYPE_ARM,llvm::MachO::CPU_SUBTYPE_ARM_V7K,   UINT32_MAX, SUBTYPE_MASK},
 {ArchSpec::eCore_arm_armv7m,  llvm::MachO::CPU_TYPE_ARM,llvm::MachO::CPU_SUBTYPE_ARM_V7M,   

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

In D94888#2506140 , @labath wrote:

> In D94888#2506000 , @MaskRay wrote:
>
>> In D94888#2505992 , @labath wrote:
>>
>>> It looks like this is removing the ability to build libc++ tests with gcc 
>>> (as it does not have the `-stdlib` option). While having that ability would 
>>> be nice, I don't believe there's anyone currently using that configuration, 
>>> so it shouldn't stand in the way of other things. But we should also update 
>>> the python detection code then (in `canRunLibcxxTests` in 
>>> `packages/Python/lldbsuite/test/dotest.py` -- I guess you just need to 
>>> remove the `if os.path.isdir("/usr/include/c++/v1"):` blurb)
>>>
>>> As for testing against the system libc++ with clang, I guess that should 
>>> still work, as the extra rpath will be just ignored in that case...
>>
>> I do not know whether the following few lines D9426 
>>  were intentional.
>
> I am pretty sure they were. This flag is needed to use (system) libc++ with 
> gcc, and this was happening at a time when we were running the test suite 
> with gcc, as it was still the default compiler for android.

Is anyone running tests in this build mode (gcc + libc++) still? Should we keep 
this support around?




Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:765-770
 cmd = [configuration.compiler, "-xc++", "-stdlib=libc++", "-o", 
f.name, "-"]
 p = subprocess.Popen(cmd, stdin=subprocess.PIPE, 
stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
 _, stderr = p.communicate("#include \nint main() {}")
 if not p.returncode:
 return True, "Compiling with -stdlib=libc++ works"
 return False, "Compiling with -stdlib=libc++ fails with the error: 
%s" % stderr

I don't know if this is the right place to do it, but it would be good to also 
check somewhere that binaries built w/ libc++ actually run. That would help 
with the error you were seeing. Maybe something like this would work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94888

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