[Lldb-commits] [PATCH] D89315: [debugserver] Add option to propagate SIGSEGV to target process

[Alessandro Arzilli via Phabricator via lldb-commits]

aarzilli updated this revision to Diff 298319.
aarzilli added a comment.

Like this?


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

https://reviews.llvm.org/D89315

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/MacOSX/MachTask.h
  lldb/tools/debugserver/source/MacOSX/MachTask.mm
  lldb/tools/debugserver/source/RNBContext.h
  lldb/tools/debugserver/source/RNBRemote.cpp
  lldb/tools/debugserver/source/debugserver.cpp

Index: lldb/tools/debugserver/source/debugserver.cpp
===
--- lldb/tools/debugserver/source/debugserver.cpp
+++ lldb/tools/debugserver/source/debugserver.cpp
@@ -245,8 +245,8 @@
: ctx.GetWorkingDirectory());
   const char *process_event = ctx.GetProcessEvent();
   nub_process_t pid = DNBProcessLaunch(
-  resolved_path, &inferior_argv[0], &inferior_envp[0], cwd, stdin_path,
-  stdout_path, stderr_path, no_stdio, launch_flavor, g_disable_aslr,
+  &ctx, resolved_path, &inferior_argv[0], &inferior_envp[0], cwd,
+  stdin_path, stdout_path, stderr_path, no_stdio, g_disable_aslr,
   process_event, launch_err_str, sizeof(launch_err_str));
 
   g_pid = pid;
@@ -368,7 +368,8 @@
   DNBLogThreadedIf(LOG_RNB_MINIMAL, "%s Attaching to pid %i...", __FUNCTION__,
attach_pid);
   char err_str[1024];
-  pid = DNBProcessAttach(attach_pid, NULL, err_str, sizeof(err_str));
+  pid = DNBProcessAttach(attach_pid, NULL, ctx.GetUnmaskSignals(), err_str,
+ sizeof(err_str));
   g_pid = pid;
 
   if (pid == INVALID_NUB_PROCESS) {
@@ -889,6 +890,10 @@
  'F'}, // When debugserver launches the process, forward debugserver's
// current environment variables to the child process ("./debugserver
// -F localhost:1234 -- /bin/ls"
+{"unmask-signals", no_argument, NULL,
+ 'U'}, // debugserver will ignore EXC_MASK_BAD_ACCESS,
+   // EXC_MASK_BAD_INSTRUCTION and EXC_MASK_ARITHMETIC, which results in
+   // SIGSEGV, SIGILL and SIGFPE being propagated to the target process.
 {NULL, 0, NULL, 0}};
 
 int communication_fd = -1;
@@ -1260,6 +1265,10 @@
   forward_env = true;
   break;
 
+case 'U':
+  ctx.SetUnmaskSignals(true);
+  break;
+
 case '2':
   // File descriptor passed to this process during fork/exec and is already
   // open and ready for communication.
@@ -1514,8 +1523,8 @@
 RNBLogSTDOUT("Waiting to attach to process %s...\n",
  waitfor_pid_name.c_str());
 nub_process_t pid = DNBProcessAttachWait(
-waitfor_pid_name.c_str(), launch_flavor, ignore_existing,
-timeout_ptr, waitfor_interval, err_str, sizeof(err_str));
+&ctx, waitfor_pid_name.c_str(), ignore_existing, timeout_ptr,
+waitfor_interval, err_str, sizeof(err_str));
 g_pid = pid;
 
 if (pid == INVALID_NUB_PROCESS) {
@@ -1550,7 +1559,8 @@
 
 RNBLogSTDOUT("Attaching to process %s...\n", attach_pid_name.c_str());
 nub_process_t pid = DNBProcessAttachByName(
-attach_pid_name.c_str(), timeout_ptr, err_str, sizeof(err_str));
+attach_pid_name.c_str(), timeout_ptr, ctx.GetUnmaskSignals(),
+err_str, sizeof(err_str));
 g_pid = pid;
 if (pid == INVALID_NUB_PROCESS) {
   ctx.LaunchStatus().SetError(-1, DNBError::Generic);
Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -3927,8 +3927,8 @@
   }
   const bool ignore_existing = true;
   attach_pid = DNBProcessAttachWait(
-  attach_name.c_str(), m_ctx.LaunchFlavor(), ignore_existing, NULL,
-  1000, err_str, sizeof(err_str), RNBRemoteShouldCancelCallback);
+  &m_ctx, attach_name.c_str(), ignore_existing, NULL, 1000, err_str,
+  sizeof(err_str), RNBRemoteShouldCancelCallback);
 
 } else if (strstr(p, "vAttachOrWait;") == p) {
   p += strlen("vAttachOrWait;");
@@ -3939,8 +3939,8 @@
   }
   const bool ignore_existing = false;
   attach_pid = DNBProcessAttachWait(
-  attach_name.c_str(), m_ctx.LaunchFlavor(), ignore_existing, NULL,
-  1000, err_str, sizeof(err_str), RNBRemoteShouldCancelCallback);
+  &m_ctx, attach_name.c_str(), ignore_existing, NULL, 1000, err_str,
+  sizeof(err_str), RNBRemoteShouldCancelCallback);
 } else if (strstr(p, "vAttachName;") == p) {
   p += strlen("vAttachName;");
   if (!GetProcessNameFrom_vAttach(p, attach_name)) {
@@ -3948,7 +3948,8 @@
 __FILE__, __LINE__, p, "non-hex char in arg on 'vAttachName' pkt");
   }

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Vitaly Buka via lldb-commits]

Ok, I can switch them back to staging. However today's staging was
frequently offline.


On Wed, 14 Oct 2020 at 21:44, Galina Kistanova  wrote:

> Thanks everyone!
>
> I would like to keep the bots in the staging till Friday. And if
> everything is good, then apply the changes to the production and let you
> move all the green bots back there.
>
> Thanks
>
> Galina
>
>
> On Tue, Oct 13, 2020 at 10:43 PM Vitaly Buka 
> wrote:
>
>> They do on staging.
>> http://lab.llvm.org:8014/#/builders/sanitizer-windows
>> http://lab.llvm.org:8014/#/builders/clang-x64-windows-msvc
>>
>> Galina, when do you plan to push this to the primary server?
>> If it's a few days, I'd rather keep bots on staging to have colors.
>>
>>
>>
>> On Tue, 13 Oct 2020 at 11:11, Reid Kleckner  wrote:
>>
>>> FWIW, I don't see any issues with my two bots that use buildbot
>>> annotated commands:
>>> http://lab.llvm.org:8011/#/builders/sanitizer-windows
>>> http://lab.llvm.org:8011/#/builders/clang-x64-windows-msvc
>>> The individual steps don't highlight as green or red, but that's OK for
>>> now.
>>>
>>> On Mon, Oct 12, 2020 at 7:19 PM Galina Kistanova 
>>> wrote:
>>>
 We have a better version of AnnotatedCommand on the staging. It should
 be a functional equivalent of the old one.
 We need to stress test it well before moving to the production build
 bot.

 For that we need all sanitizer + other bots which use the
 AnnotatedCommand directly or indirectly moved temporarily to the staging.

 Please let me know when that could be arranged.

 Thanks

 Galina

 On Mon, Oct 12, 2020 at 11:39 AM Reid Kleckner  wrote:

> On Wed, Oct 7, 2020 at 4:32 PM Galina Kistanova via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>> They are online now -
>> http://lab.llvm.org:8011/#/waterfall?tags=sanitizer
>>
>> AnnotatedCommand has severe design conflict with the new buildbot.
>> We have changed it to be safe and still do something useful, but it
>> will need more love and care.
>>
>> Please let me know if you have some spare time to work on porting
>> AnnotatedCommand.
>>
>
> That's unfortunate, it would've been good to know that earlier. I and
> another team member have spent a fair amount of time porting things to use
> more AnnotatedCommand steps, because it gives us the flexibility to test
> steps locally and make changes to the steps without restarting the 
> buildbot
> master. IMO that is the Right Way to define steps: a script that you can
> run locally on a machine that satisfies the OS and dep requirements of the
> script.
>
> I am restarting the two bots that I am responsible for, and may need
> some help debugging further issues soon. I'll let you know.
>

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

[Lldb-commits] [PATCH] D89428: Add support for more OS types to AddClangModuleCompilationOptionsForSDKType()

[Raphael Isemann via Phabricator via lldb-commits]

teemperor requested changes to this revision.
teemperor added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1525
+  // If the SDK type is for the host OS, use its version number.
+  auto get_host_os = []() { return HostInfo::GetTargetTriple().getOS(); };
   switch (sdk_type) {

`Triple::OSType host_os = HostInfo::GetTargetTriple().getOS();` ?



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1576
+case XcodeSDK::Type::AppleTVOS:
+  minimum_version_option.PutCString(opt_mtvos_simulator_version_min_EQ);
   break;

`opt_mtvos_version_min_EQ`



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1585
+case XcodeSDK::Type::bridgeOS:
+  minimum_version_option.PutCString(opt_mwatchos_version_min_EQ);
+  break;

If this is on purpose, then I think there should be a comment explaining why 
this is not `-mbridgeos-version-min=` ?


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

https://reviews.llvm.org/D89428

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

[Lldb-commits] [PATCH] D89428: Add support for more OS types to AddClangModuleCompilationOptionsForSDKType()

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Thanks for refactoring this!


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

https://reviews.llvm.org/D89428

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

[Lldb-commits] [lldb] af5504e - Increase timeout to find a dSYM in macos DownloadObjectAndSymbolFile

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2020-10-15T00:57:23-07:00
New Revision: af5504edd6815d085de8aa66efcd7150d6315a51

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

LOG: Increase timeout to find a dSYM in macos DownloadObjectAndSymbolFile

With a large dSYM over a slow home connection, the two minute timeout
would sometimes be exceeded, and we haven't seen instances of a
long timeout causing people any problems, so we're bumping it up.

640 seconds ought to be enough for anyone.



Added: 


Modified: 
lldb/source/Symbol/LocateSymbolFileMacOSX.cpp

Removed: 




diff  --git a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp 
b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
index fe30ceeabcb2..344bac8e0632 100644
--- a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -621,7 +621,7 @@ bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec 
&module_spec,
 &signo,  // Signal int *
 &command_output, // Command output
 std::chrono::seconds(
-   120), // Large timeout to allow for long dsym download times
+   640), // Large timeout to allow for long dsym download times
 false);  // Don't run in a shell (we don't need shell expansion)
 if (error.Success() && exit_status == 0 && !command_output.empty()) {
   CFCData data(CFDataCreateWithBytesNoCopy(



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

[Lldb-commits] [PATCH] D89413: [lldb] [Process/FreeBSDRemote] Initial multithreading support

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Seems pretty straight forward.


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

https://reviews.llvm.org/D89413

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

[Lldb-commits] [PATCH] D89408: [trace] rename ThreadIntelPT to TraceThread

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Making this a separate patch is a good idea, but it does not materially address 
my objections to having ProcessTrace and ThreadTrace live in two different 
places. :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89408

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

[Lldb-commits] [lldb] 82ed186 - [lldb] Explicitly test the template argument SB API

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-10-15T11:17:43+02:00
New Revision: 82ed18601dbc816e1f64407a926602f95bbeda32

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

LOG: [lldb] Explicitly test the template argument SB API

Added: 
lldb/test/API/lang/cpp/template-arguments/Makefile
lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py
lldb/test/API/lang/cpp/template-arguments/main.cpp

Modified: 


Removed: 




diff  --git a/lldb/test/API/lang/cpp/template-arguments/Makefile 
b/lldb/test/API/lang/cpp/template-arguments/Makefile
new file mode 100644
index ..8b20bcb0
--- /dev/null
+++ b/lldb/test/API/lang/cpp/template-arguments/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py 
b/lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py
new file mode 100644
index ..3ba86bc44e25
--- /dev/null
+++ b/lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py
@@ -0,0 +1,30 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+def test(self):
+self.build()
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+value = self.expect_expr("temp1", result_type="C")
+template_type = value.GetType()
+self.assertEqual(template_type.GetNumberOfTemplateArguments(), 2)
+
+# Check a type argument.
+self.assertEqual(template_type.GetTemplateArgumentKind(0), 
lldb.eTemplateArgumentKindType)
+self.assertEqual(template_type.GetTemplateArgumentType(0).GetName(), 
"int")
+
+# Check a integral argument.
+self.assertEqual(template_type.GetTemplateArgumentKind(1), 
lldb.eTemplateArgumentKindIntegral)
+self.assertEqual(template_type.GetTemplateArgumentType(1).GetName(), 
"unsigned int")
+#FIXME: There is no way to get the actual value of the parameter.
+
+# Try to get an invalid template argument.
+self.assertEqual(template_type.GetTemplateArgumentKind(2), 
lldb.eTemplateArgumentKindNull)
+self.assertEqual(template_type.GetTemplateArgumentType(2).GetName(), 
"")

diff  --git a/lldb/test/API/lang/cpp/template-arguments/main.cpp 
b/lldb/test/API/lang/cpp/template-arguments/main.cpp
new file mode 100644
index ..728bd400c258
--- /dev/null
+++ b/lldb/test/API/lang/cpp/template-arguments/main.cpp
@@ -0,0 +1,8 @@
+template
+struct C {
+  T member = value;
+};
+
+C temp1;
+
+int main() {}



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

[Lldb-commits] [PATCH] D82863: [LLDB] Add support to resize SVE registers at run-time

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D82863#2330122 , @labath wrote:

> In D82863#2324560 , @omjavaid wrote:
>
>> In D82863#2321647 , @labath wrote:
>>
>>> In D82863#2321370 , @omjavaid 
>>> wrote:
>>>
 In D82863#2320342 , @jasonmolenda 
 wrote:

> The original g/G packets were designed for little embedded systems where 
> the stub had to be very small and dumb, and could just memcpy the payload 
> in the packet into the register context & back out again.  Any sensible 
> design today would, at least, have some form of an array of regnum:regval 
> to eliminate many of the problems.
>
>> Unless of course, we make sure SVE regs come last, but that imposes some 
>> constraints on future registers sets. I suppose we might be able to say 
>> that all variable-length or otherwise-funny registers must come last.
>
> Yeah, Omair's patch currently assumes that the SVE regs come last already 
> when they copy & truncate the registers context to heap.  I fear that 
> we'll get to armv12 and we'll be adding registers after the SVE and 
> wonder why they're being truncated somewhere in lldb. :)
>>>
>>> Well.. we could design it such that the SVE registers (and any other 
>>> dynamically-sized regs) always come last. Then they wouldn't impact the 
>>> offsets of static registers.
>>>
>>> I don't think we have a requirement that newer register sets must be 
>>> appended. Or at least, we shouldn't have, as now both intel and arm have 
>>> cpu features (and registers) that may be arbitrarily combined in future 
>>> processors.
>>
>> For now I am considering variable sized registers i-e SVE Z and P registers 
>> to always come last which is currently the case and will be in future 
>> patches which add support for Pointer Authentication and MTE registers.
>>
> @omjavaid , what do you think about disabling g/G when we're working with 
> SVE registers (GDBRemoteCommunicationClient::AvoidGPackets)?  There are 
> some gdb-remote stubs that can *only* read/write registers with g/G, but 
> I think it's reasonable to say "you must implement p/P for a target with 
> SVE", that's a generic packet shared by both lldb and gdb.  We could add 
> a more-modern g/G replacement packet, but no one would have that 
> implemented, and if they were going to add anything, I'd have them 
> implement p/P unless it's perf problems and they need a 
> read-all-registers / write-all-registers packet that works with SVE.
>>
>> Just scrolling through GDB log one of the answers about target XML is that 
>> target XML is not exchanged for native connection where gdbserver is not 
>> being used. For gdbserver connection, target xml is exchanged onces per 
>> connection and register sizes are updated based on vg after that internally. 
>> However testing gdb with same executable as the one I wrote for LLDB was 
>> having gdb remote connection crash which could be something to do with SVE 
>> or some other bug.
>
> Interesting.
>
> I started drafting an email to the gdb mailing list to try to clarify this 
> thing. For that, I was re-reading the gdb docs for the qXfer:target.xml 
> packet,  I realized that the gdb interpretation is mostly unambiguous about 
> this thing:
>
> - **regnum**: The register’s number. If omitted,  algorithm...>. This register number is used to read or write the register; 
> e.g. it is used in the remote p and P packets, and **registers appear in the 
> g and G packets in order of increasing register number**.
>
> I'm sure that whoever wrote this did not have variable-length registers in 
> mind. However, it's interpretation for variable-length registers is pretty 
> clear: take each register, and dump its bytes in sequence (no matter many of 
> them are in the register at this particular time). The reason this is 
> unambiguous is because gdb does not have the "offset" attribute of the "reg" 
> element, and so the only way to find its 'g' offset is via this algorithm. 
> And given your explanation about "switching" target.xml descriptions at 
> runtime, I'm would expect this is what is happening within gdb.
>
> The problem is that he "offset" attribute (added by lldb) makes things 
> ambiguous, as it provides another way to find a register in the 'g' blob. So 
> how about we "fix" that ambiguity by having lldb-server omit the "offset" 
> attribute for variable-length registers?
>
> Sending it over is actively harmful, as the value is very likely to be wrong, 
> and I believe that in your current implementation these numbers are pretty 
> much ignored by the client anyway. Lldb should already have code to 
> automatically compute the regiser offsets when the "offset" attribute is 
> missing (though it's possible it may not handle parti

[Lldb-commits] [PATCH] D89334: [lldb] Support Python imports relative the to the current file being sourced

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D89334#2330287 , @JDevlieghere 
wrote:

> In D89334#2329667 , @labath wrote:
>
>> The main question on my mind is should we be adding the directory of the 
>> python file to the path (which is what I believe is happening now), or if we 
>> should add the directory of the command file that is being sourced (and 
>> adjust the way we import the file). The main impact of this is how will the 
>> imported module "see" itself (what will it's name be), and how it will be 
>> able to import other modules.
>>
>> Imagine the user has the following (reasonable, I think) file structure.
>>
>>   $ROOT/utils/consult_oracle.py
>>   $ROOT/automatic_bug_finder/main.py # uses consult_oracle.py
>>   $ROOT/awesome_backtrace_analyzer/main.py # uses consult_oracle.py
>>   $ROOT/install_super_scripts.lldbinit # calls command script import 
>> awesome_backtrace_analyzer/main.py
>>
>> If "command script import awesome_backtrace_analyzer/main.py" ends up adding 
>> `$ROOT/awesome_backtrace_analyzer`  to the path, then this module will have 
>> a hard time importing the modules it depends on (it would either have to use 
>> weird relative imports, or mess with sys.path itself. If we add just `$ROOT` 
>> then it could simply `import utils.consult_oracle`.
>
> I guess then the user should have called `command script import 
> awesome_backtrace_analyzer` to import the package rather than the `main.py` 
> inside it. But I get your point. FWIW just adding the `$ROOT` is how I did 
> the original implementation before adding the tests for the nested 
> directories and `.py` files that Dave suggested. It would solve this issues 
> but then doesn't support those scenarios. I don't know if it would be less 
> confusing that you can't pass a relative path to a `.py` file or that you 
> can't import another module as you described.

I don't think the two options are mutually exclusive. I'm pretty sure this is 
just a limitation of our current importing code, which could be fixed to import 
`awesome_backtrace_analyzer/main.py` as `awesome_backtrace_analyzer.main` like 
it would be from python.

>> I think setting the import path to $ROOT would actually make the sys.path 
>> manipulation serve some useful purpose (and also reduce the number of 
>> sys.path entries we add). If, on the other hand, we are not interested 
>> making cross-module imports work "out of the box" (like, we could say that 
>> it's the responsibility of individual modules to ensure that), we could also 
>> try to import the file without messing with sys.path at all 
>> (https://stackoverflow.com/questions/67631/how-to-import-a-module-given-the-full-path
>>  gives one way to do that).
>
> I would prefer this approach if it didn't require to name the module ourself. 
> Any heuristic will have the risk of being ambitious as well (which is 
> probably why the API makes you specify the module name).

(I assume you meant ambiguous, not ambitious :P)

Well... yes, if we do the simplest thing of naming the "module" according to 
the file basename then it will be ambiguous. But I would say that even _that_ 
is better than what we do now, because it avoids funky interactions between all 
the sys.path entries that we're adding -- e.g. a random file in the same 
directory as one of the files user imported becoming visible to python import 
machinery, and shadowing some other real module. It also gives us the option to 
do something about that ambiguity -- we could add numerical suffixes to the 
imported names (and have the import command print the name it used) or 
whatever...


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

https://reviews.llvm.org/D89334

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

[Lldb-commits] [PATCH] D89413: [lldb] [Process/FreeBSDRemote] Initial multithreading support

[Ed Maste via Phabricator via lldb-commits]

emaste added a comment.

Looks fine to me


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

https://reviews.llvm.org/D89413

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

[Lldb-commits] [PATCH] D87442: [lldb] Show flags for memory regions

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 298371.
DavidSpickett added a comment.

- Move to llvm::Expected for callbacks
- Use flag names based on Linux names but for all platforms
- Show generic names/descriptions in memory region output
- Add minidump cases for error handling


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87442

Files:
  lldb/bindings/interface/SBMemoryRegionInfo.i
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/API/SBMemoryRegionInfo.h
  lldb/include/lldb/Target/MemoryRegionInfo.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/API/SBMemoryRegionInfo.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp
  lldb/source/Plugins/Process/Utility/LinuxProcMaps.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
  lldb/source/Target/MemoryRegionInfo.cpp
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
  lldb/unittests/Process/CMakeLists.txt
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
  lldb/unittests/Process/minidump/MinidumpParserTest.cpp
  lldb/unittests/Target/MemoryRegionInfoTest.cpp

Index: lldb/unittests/Target/MemoryRegionInfoTest.cpp
===
--- lldb/unittests/Target/MemoryRegionInfoTest.cpp
+++ lldb/unittests/Target/MemoryRegionInfoTest.cpp
@@ -17,3 +17,42 @@
   EXPECT_EQ("no", llvm::formatv("{0}", MemoryRegionInfo::eNo).str());
   EXPECT_EQ("don't know", llvm::formatv("{0}", MemoryRegionInfo::eDontKnow).str());
 }
+
+typedef std::pair MemoryRegionInfoFlagTestParams;
+
+class MemoryRegionInfoFlagTestFixture
+: public ::testing::TestWithParam {
+protected:
+  void do_test(MemoryRegionInfoFlagTestParams params) {
+MemoryRegionInfo info;
+const char *in;
+std::string expected;
+std::tie(in, expected) = params;
+info.SetFlagsFromShortFlags(in);
+ASSERT_TRUE(info.HasFlags());
+ASSERT_EQ(expected, info.GetShortFlagNames());
+  }
+};
+
+static MemoryRegionInfoFlagTestParams make_roundtrip(const char *flag) {
+  return std::make_pair(flag, flag);
+}
+
+TEST_P(MemoryRegionInfoFlagTestFixture, TestShortFlags) { do_test(GetParam()); }
+
+INSTANTIATE_TEST_CASE_P(
+MemoryRegionInfoFlagsTests, MemoryRegionInfoFlagTestFixture,
+::testing::Values(
+std::make_pair("", ""), std::make_pair("  ", ""),
+std::make_pair("rd", "rd"), std::make_pair("rd wr ex", "rd wr ex"),
+// Output order is the same as the flags enum
+std::make_pair("ex wr rd", "rd wr ex"),
+// Duplicates ignored
+std::make_pair("ex rd ex", "rd ex"),
+// Unknown flags ignored
+std::make_pair("ex rd zz", "rd ex"), std::make_pair("foo bar", ""),
+// Spaces ignored
+std::make_pair("   ex rdwr   ", "rd wr ex"),
+// Roundtrip all flags we currently support
+make_roundtrip("rd wr ex sh mr mw me ms gd pf dw lo io sr rr dc de "
+   "ac nr ht sf nl ar wf dd sd mm hg nh mg um uw")), );
Index: lldb/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -374,20 +374,20 @@
 )"),
 llvm::Succeeded());
 
-  EXPECT_THAT(
-  parser->BuildMemoryRegions(),
-  testing::Pair(testing::ElementsAre(
-MemoryRegionInfo({0x0, 0x1}, no, no, no, no,
- ConstString(), unknown, 0),
-MemoryRegionInfo({0x1, 0x21000}, yes, yes, no, yes,
- ConstString(), unknown, 0),
-MemoryRegionInfo({0x4, 0x1000}, yes, no, no, yes,
- ConstString(), unknown, 0),
-MemoryRegionInfo({0x7ffe, 0x1000}, yes, no, no, yes,
- ConstString(), unknown, 0),
-MemoryRegionInfo({0x7ffe1000, 0xf000}, no, no, no, yes,
- ConstString(), unknown, 0)),
-true));
+  EXPECT_THAT(parser->BuildMemoryRegions(),
+  testing::Pair(
+  testing::ElementsAre(
+  MemoryRegionInfo({0x0, 0x1}, no, no, no, no,
+   ConstString(), unknown, 0, llvm::None),
+  

[Lldb-commits] [PATCH] D87442: [lldb] Show flags for memory regions

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

If you wondering, the average number of flags for a hello world's memory 
regions is 8 so 8 lines of output. Here's an example I had that does madvise.

  (lldb) memory region addr
  [0x77ed5000-0x77fd5000) rw- /dev/zero (deleted)
  flags:
  readable
  writeable
  shared
  may read
  may write
  may execute
  may share
  soft-dirty
  (lldb) n
  (lldb) n (over madvise(addr, len, MADV_DONTFORK);)
  (lldb) memory region addr
  [0x77ed5000-0x77fd5000) rw- /dev/zero (deleted)
  flags:
  readable
  writeable
  shared
  may read
  may write
  may execute
  may share
  do not copy area on fork
  soft-dirty

I could add this as a test case but it only adds checking that the region cache 
is updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87442

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

[Lldb-commits] [PATCH] D89334: [lldb] Support Python imports relative the to the current file being sourced

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D89334#2331903 , @labath wrote:

> In D89334#2330287 , @JDevlieghere 
> wrote:
>
>> In D89334#2329667 , @labath wrote:
>>
>>> The main question on my mind is should we be adding the directory of the 
>>> python file to the path (which is what I believe is happening now), or if 
>>> we should add the directory of the command file that is being sourced (and 
>>> adjust the way we import the file). The main impact of this is how will the 
>>> imported module "see" itself (what will it's name be), and how it will be 
>>> able to import other modules.
>>>
>>> Imagine the user has the following (reasonable, I think) file structure.
>>>
>>>   $ROOT/utils/consult_oracle.py
>>>   $ROOT/automatic_bug_finder/main.py # uses consult_oracle.py
>>>   $ROOT/awesome_backtrace_analyzer/main.py # uses consult_oracle.py
>>>   $ROOT/install_super_scripts.lldbinit # calls command script import 
>>> awesome_backtrace_analyzer/main.py
>>>
>>> If "command script import awesome_backtrace_analyzer/main.py" ends up 
>>> adding `$ROOT/awesome_backtrace_analyzer`  to the path, then this module 
>>> will have a hard time importing the modules it depends on (it would either 
>>> have to use weird relative imports, or mess with sys.path itself. If we add 
>>> just `$ROOT` then it could simply `import utils.consult_oracle`.
>>
>> I guess then the user should have called `command script import 
>> awesome_backtrace_analyzer` to import the package rather than the `main.py` 
>> inside it. But I get your point. FWIW just adding the `$ROOT` is how I did 
>> the original implementation before adding the tests for the nested 
>> directories and `.py` files that Dave suggested. It would solve this issues 
>> but then doesn't support those scenarios. I don't know if it would be less 
>> confusing that you can't pass a relative path to a `.py` file or that you 
>> can't import another module as you described.
>
> I don't think the two options are mutually exclusive. I'm pretty sure this is 
> just a limitation of our current importing code, which could be fixed to 
> import `awesome_backtrace_analyzer/main.py` as 
> `awesome_backtrace_analyzer.main` like it would be from python.

I don't think we can do that in the general case without breaking users (see 
below). I guess we could do it for imports not relative to the current working 
directory, as this has never worked before. It would then replace the logic 
described by the comment on line  2793.

>>> I think setting the import path to $ROOT would actually make the sys.path 
>>> manipulation serve some useful purpose (and also reduce the number of 
>>> sys.path entries we add). If, on the other hand, we are not interested 
>>> making cross-module imports work "out of the box" (like, we could say that 
>>> it's the responsibility of individual modules to ensure that), we could 
>>> also try to import the file without messing with sys.path at all 
>>> (https://stackoverflow.com/questions/67631/how-to-import-a-module-given-the-full-path
>>>  gives one way to do that).
>>
>> I would prefer this approach if it didn't require to name the module 
>> ourself. Any heuristic will have the risk of being ambitious as well (which 
>> is probably why the API makes you specify the module name).
>
> (I assume you meant ambiguous, not ambitious :P)

😅

> Well... yes, if we do the simplest thing of naming the "module" according to 
> the file basename then it will be ambiguous. But I would say that even _that_ 
> is better than what we do now, because it avoids funky interactions between 
> all the sys.path entries that we're adding -- e.g. a random file in the same 
> directory as one of the files user imported becoming visible to python import 
> machinery, and shadowing some other real module. It also gives us the option 
> to do something about that ambiguity -- we could add numerical suffixes to 
> the imported names (and have the import command print the name it used) or 
> whatever...

From a technical point I agree a 100%, but I just don't see how we can do it 
without breaking tons of users that are using the module name in `command 
script add`:

  def __lldb_init_module(debugger, internal_dict):
  debugger.HandleCommand('command script add -f module.function 
my_function')

I'll hold off on updating the patch until we reached consensus.


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

https://reviews.llvm.org/D89334

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

[Lldb-commits] [lldb] 87d3883 - [lldb] [Process/FreeBSDRemote] Initial multithreading support

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2020-10-15T17:37:37+02:00
New Revision: 87d38831d909bf937039a97aa63220929d498047

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

LOG: [lldb] [Process/FreeBSDRemote] Initial multithreading support

Implement initial support for watching thread creation and termination.
Update ptrace() calls to correctly indicate requested thread.
Watchpoints are not supported yet.

This patch fixes at least multithreaded register tests.

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

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h

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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
index e99d38f57eea..c234c0e023fb 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -93,7 +93,7 @@ NativeProcessFreeBSD::Factory::Launch(ProcessLaunchInfo 
&launch_info,
   pid, launch_info.GetPTY().ReleasePrimaryFileDescriptor(), 
native_delegate,
   Info.GetArchitecture(), mainloop));
 
-  status = process_up->ReinitializeThreads();
+  status = process_up->SetupTrace();
   if (status.Fail())
 return status.ToError();
 
@@ -125,6 +125,10 @@ NativeProcessFreeBSD::Factory::Attach(
   if (!status.Success())
 return status.ToError();
 
+  status = process_up->SetupTrace();
+  if (status.Fail())
+return status.ToError();
+
   return std::move(process_up);
 }
 
@@ -191,14 +195,26 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t 
pid) {
 return;
   }
   assert(info.pl_event == PL_EVENT_SIGNAL);
-  // TODO: do we need to handle !PL_FLAG_SI?
-  assert(info.pl_flags & PL_FLAG_SI);
-  assert(info.pl_siginfo.si_signo == SIGTRAP);
-
-  LLDB_LOG(log, "got SIGTRAP, pid = {0}, lwpid = {1}, si_code = {2}", pid,
-   info.pl_lwpid, info.pl_siginfo.si_code);
 
+  LLDB_LOG(log, "got SIGTRAP, pid = {0}, lwpid = {1}", pid, info.pl_lwpid);
   NativeThreadFreeBSD *thread = nullptr;
+
+  if (info.pl_flags & (PL_FLAG_BORN | PL_FLAG_EXITED)) {
+if (info.pl_flags & PL_FLAG_BORN) {
+  LLDB_LOG(log, "monitoring new thread, tid = {0}", info.pl_lwpid);
+  AddThread(info.pl_lwpid);
+} else /*if (info.pl_flags & PL_FLAG_EXITED)*/ {
+  LLDB_LOG(log, "thread exited, tid = {0}", info.pl_lwpid);
+  RemoveThread(info.pl_lwpid);
+}
+
+Status error =
+PtraceWrapper(PT_CONTINUE, pid, reinterpret_cast(1), 0);
+if (error.Fail())
+  SetState(StateType::eStateInvalid);
+return;
+  }
+
   if (info.pl_lwpid > 0) {
 for (const auto &t : m_threads) {
   if (t->GetID() == static_cast(info.pl_lwpid)) {
@@ -212,19 +228,23 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t 
pid) {
info.pl_lwpid);
   }
 
-  switch (info.pl_siginfo.si_code) {
-  case TRAP_BRKPT:
-if (thread) {
-  thread->SetStoppedByBreakpoint();
-  FixupBreakpointPCAsNeeded(*thread);
+  if (info.pl_flags & PL_FLAG_SI) {
+assert(info.pl_siginfo.si_signo == SIGTRAP);
+
+switch (info.pl_siginfo.si_code) {
+case TRAP_BRKPT:
+  if (thread) {
+thread->SetStoppedByBreakpoint();
+FixupBreakpointPCAsNeeded(*thread);
+  }
+  SetState(StateType::eStateStopped, true);
+  break;
+case TRAP_TRACE:
+  if (thread)
+thread->SetStoppedByTrace();
+  SetState(StateType::eStateStopped, true);
+  break;
 }
-SetState(StateType::eStateStopped, true);
-break;
-  case TRAP_TRACE:
-if (thread)
-  thread->SetStoppedByTrace();
-SetState(StateType::eStateStopped, true);
-break;
   }
 }
 
@@ -743,6 +763,21 @@ NativeProcessFreeBSD::GetAuxvData() const {
   return buf;
 }
 
+Status NativeProcessFreeBSD::SetupTrace() {
+  // Enable event reporting
+  int events;
+  Status status =
+  PtraceWrapper(PT_GET_EVENT_MASK, GetID(), &events, sizeof(events));
+  if (status.Fail())
+return status;
+  events |= PTRACE_LWP;
+  status = PtraceWrapper(PT_SET_EVENT_MASK, GetID(), &events, sizeof(events));
+  if (status.Fail())
+return status;
+
+  return ReinitializeThreads();
+}
+
 Status NativeProcessFreeBSD::ReinitializeThreads() {
   // Clear old threads
   m_threads.clear();

diff  --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcess

[Lldb-commits] [PATCH] D89413: [lldb] [Process/FreeBSDRemote] Initial multithreading support

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG87d38831d909: [lldb] [Process/FreeBSDRemote] Initial 
multithreading support (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89413

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

Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
@@ -40,32 +40,27 @@
   m_stop_description() {}
 
 Status NativeThreadFreeBSD::Resume() {
-  Status ret = NativeProcessFreeBSD::PtraceWrapper(PT_RESUME, m_process.GetID(),
-   nullptr, GetID());
+  Status ret = NativeProcessFreeBSD::PtraceWrapper(PT_RESUME, GetID());
   if (!ret.Success())
 return ret;
-  ret = NativeProcessFreeBSD::PtraceWrapper(PT_CLEARSTEP, m_process.GetID(),
-nullptr, GetID());
+  ret = NativeProcessFreeBSD::PtraceWrapper(PT_CLEARSTEP, GetID());
   if (ret.Success())
 SetRunning();
   return ret;
 }
 
 Status NativeThreadFreeBSD::SingleStep() {
-  Status ret = NativeProcessFreeBSD::PtraceWrapper(PT_RESUME, m_process.GetID(),
-   nullptr, GetID());
+  Status ret = NativeProcessFreeBSD::PtraceWrapper(PT_RESUME, GetID());
   if (!ret.Success())
 return ret;
-  ret = NativeProcessFreeBSD::PtraceWrapper(PT_SETSTEP, m_process.GetID(),
-nullptr, GetID());
+  ret = NativeProcessFreeBSD::PtraceWrapper(PT_SETSTEP, GetID());
   if (ret.Success())
 SetStepping();
   return ret;
 }
 
 Status NativeThreadFreeBSD::Suspend() {
-  Status ret = NativeProcessFreeBSD::PtraceWrapper(
-  PT_SUSPEND, m_process.GetID(), nullptr, GetID());
+  Status ret = NativeProcessFreeBSD::PtraceWrapper(PT_SUSPEND, GetID());
   if (ret.Success())
 SetStopped();
   return ret;
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -429,15 +429,19 @@
 Status NativeRegisterContextFreeBSD_x86_64::ReadRegisterSet(uint32_t set) {
   switch (set) {
   case GPRegSet:
-return DoRegisterSet(PT_GETREGS, &m_gpr);
+return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
+   &m_gpr);
   case FPRegSet:
 #if defined(__x86_64__)
-return DoRegisterSet(PT_GETFPREGS, &m_fpr);
+return NativeProcessFreeBSD::PtraceWrapper(PT_GETFPREGS, m_thread.GetID(),
+   &m_fpr);
 #else
-return DoRegisterSet(PT_GETXMMREGS, &m_fpr);
+return NativeProcessFreeBSD::PtraceWrapper(PT_GETXMMREGS, m_thread.GetID(),
+   &m_fpr);
 #endif
   case DBRegSet:
-return DoRegisterSet(PT_GETDBREGS, &m_dbr);
+return NativeProcessFreeBSD::PtraceWrapper(PT_GETDBREGS, m_thread.GetID(),
+   &m_dbr);
   case XSaveRegSet: {
 struct ptrace_xstate_info info;
 Status ret = NativeProcessFreeBSD::PtraceWrapper(
@@ -466,15 +470,19 @@
 Status NativeRegisterContextFreeBSD_x86_64::WriteRegisterSet(uint32_t set) {
   switch (set) {
   case GPRegSet:
-return DoRegisterSet(PT_SETREGS, &m_gpr);
+return NativeProcessFreeBSD::PtraceWrapper(PT_SETREGS, m_thread.GetID(),
+   &m_gpr);
   case FPRegSet:
 #if defined(__x86_64__)
-return DoRegisterSet(PT_SETFPREGS, &m_fpr);
+return NativeProcessFreeBSD::PtraceWrapper(PT_SETFPREGS, m_thread.GetID(),
+   &m_fpr);
 #else
-return DoRegisterSet(PT_SETXMMREGS, &m_fpr);
+return NativeProcessFreeBSD::PtraceWrapper(PT_SETXMMREGS, m_thread.GetID(),
+   &m_fpr);
 #endif
   case DBRegSet:
-return DoRegisterSet(PT_SETDBREGS, &m_dbr);
+return NativeProcessFreeBSD::PtraceWrapper(PT_SETDBREGS, m_thread.GetID(),
+   &m_dbr);
   case XSaveRegSet:
 // ReadRegisterSet() must always be call

[Lldb-commits] [PATCH] D89477: [lldb] Port lldb gdb-server to libOption

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, MaskRay.
Herald added subscribers: dang, mgorny.
Herald added a project: LLDB.
labath requested review of this revision.

The existing help text was very terse and was missing several important
options. In the new version, I add a short description of each option
and a slightly longer description of the tool as a whole.

The new option list does not include undocumented no-op options:
--debug and --verbose. It also does not include undocumented short
aliases for long options, with two exceptions: -h, because it's
well-known; and -S (--setsid), as it's used in one test. Using these
options will now produce an error. I believe that is acceptable as users
aren't generally invoking lldb-server directly, and the only way to
learn about the short aliases was by looking at the source.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89477

Files:
  lldb/include/lldb/Utility/Args.h
  lldb/source/Utility/Args.cpp
  lldb/test/Shell/lldb-server/TestErrorMessages.test
  lldb/tools/lldb-server/CMakeLists.txt
  lldb/tools/lldb-server/LLGSOptions.td
  lldb/tools/lldb-server/lldb-gdbserver.cpp

Index: lldb/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -17,7 +17,6 @@
 #include 
 #endif
 
-
 #include "Acceptor.h"
 #include "LLDBServerUtilities.h"
 #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h"
@@ -25,8 +24,6 @@
 #include "lldb/Host/Config.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/FileSystem.h"
-#include "lldb/Host/HostGetOpt.h"
-#include "lldb/Host/OptionParser.h"
 #include "lldb/Host/Pipe.h"
 #include "lldb/Host/Socket.h"
 #include "lldb/Host/StringConvert.h"
@@ -34,7 +31,11 @@
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/Status.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Option/OptTable.h"
+#include "llvm/Option/Option.h"
 #include "llvm/Support/Errno.h"
+#include "llvm/Support/WithColor.h"
 
 #if defined(__linux__)
 #include "Plugins/Process/Linux/NativeProcessLinux.h"
@@ -88,31 +89,6 @@
 #endif
 }
 
-// option descriptors for getopt_long_only()
-
-static int g_debug = 0;
-static int g_verbose = 0;
-
-static struct option g_long_options[] = {
-{"debug", no_argument, &g_debug, 1},
-{"verbose", no_argument, &g_verbose, 1},
-{"log-file", required_argument, nullptr, 'l'},
-{"log-channels", required_argument, nullptr, 'c'},
-{"attach", required_argument, nullptr, 'a'},
-{"named-pipe", required_argument, nullptr, 'N'},
-{"pipe", required_argument, nullptr, 'U'},
-{"native-regs", no_argument, nullptr,
- 'r'}, // Specify to use the native registers instead of the gdb defaults
-   // for the architecture.  NOTE: this is a do-nothing arg as it's
-   // behavior is default now.  FIXME remove call from lldb-platform.
-{"reverse-connect", no_argument, nullptr,
- 'R'}, // Specifies that llgs attaches to the client address:port rather
-   // than llgs listening for a connection from address on port.
-{"setsid", no_argument, nullptr,
- 'S'}, // Call setsid() to make llgs run in its own session.
-{"fd", required_argument, nullptr, 'F'},
-{nullptr, 0, nullptr, 0}};
-
 #ifndef _WIN32
 // Watch for signals
 static int g_sighup_received_count = 0;
@@ -129,20 +105,6 @@
 }
 #endif // #ifndef _WIN32
 
-static void display_usage(const char *progname, const char *subcommand) {
-  fprintf(stderr, "Usage:\n  %s %s "
-  "[--log-file log-file-name] "
-  "[--log-channels log-channel-list] "
-  "[--setsid] "
-  "[--fd file-descriptor]"
-  "[--named-pipe named-pipe-path] "
-  "[--native-regs] "
-  "[--attach pid] "
-  "[[HOST]:PORT] "
-  "[-- PROGRAM ARG1 ARG2 ...]\n",
-  progname, subcommand);
-}
-
 void handle_attach_to_pid(GDBRemoteCommunicationServerLLGS &gdb_server,
   lldb::pid_t pid) {
   Status error = gdb_server.AttachToProcess(pid);
@@ -176,12 +138,12 @@
 handle_attach_to_process_name(gdb_server, attach_target);
 }
 
-void handle_launch(GDBRemoteCommunicationServerLLGS &gdb_server, int argc,
-   const char *const argv[]) {
+void handle_launch(GDBRemoteCommunicationServerLLGS &gdb_server,
+   llvm::ArrayRef Arguments) {
   ProcessLaunchInfo info;
   info.GetFlags().Set(eLaunchFlagStopAtEntry | eLaunchFlagDebug |
   eLaunchFlagDisableASLR);
-  info.SetArguments(const_cast(argv), true);
+  info.SetArguments(Args(Arguments), true);
 
   llvm::SmallString<64> cwd;
   if (std::error_code ec = llvm::sys::fs::current_path(cwd)) {
@@ -198,7 +160,7 @@
   Status error = gdb_server.LaunchProcess();
 

[Lldb-commits] [PATCH] D89315: [debugserver] Add option to propagate SIGSEGV to target process

[Jim Ingham via Phabricator via lldb-commits]

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

That's nicer, thanks!


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

https://reviews.llvm.org/D89315

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

[Lldb-commits] [PATCH] D89477: [lldb] Port lldb gdb-server to libOption

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Utility/Args.cpp:178
 
+Args::Args(llvm::ArrayRef args) : Args() {
+  for (llvm::StringRef arg : args)

Maybe StringList should have a ctor that takes an `ArrayRef`, but 
probably not worth the extra copies here. 



Comment at: lldb/tools/lldb-server/lldb-gdbserver.cpp:414
 
+  std::string HelpText =
+  "Use '" + Name + " --help' for a complete list of options.\n";

Should we extract this code into a utility that we can use here and in the lldb 
driver?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89477

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

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:54
+if (sc_list.GetContextAtIndex(i, sc) &&
+(sc.symbol->IsExternal() || sc.symbol->IsWeak())) {
   const uint32_t range_scope =

labath wrote:
> clayborg wrote:
> > labath wrote:
> > > aadsm wrote:
> > > > labath wrote:
> > > > > clayborg wrote:
> > > > > > aadsm wrote:
> > > > > > > clayborg wrote:
> > > > > > > > Why are we checking "IsWeak()" here?
> > > > > > > A weak symbol is also an external symbol, but it's weak in the 
> > > > > > > sense that another external symbol with the same name will take 
> > > > > > > precedence over it (as far as I understood).
> > > > > > I think we only need to check for external here. Any weak symbol 
> > > > > > will also need to be external, but if it isn't we don't want that 
> > > > > > symbol.
> > > > > Your understanding is correct, at least at the object file level -- 
> > > > > I'm not sure whether `IsWeak()` implies `IsExternal()` inside lldb. 
> > > > > However, I would actually argue for removal of IsWeak() for a 
> > > > > different reason -- any weak definition of mmap is not going to be 
> > > > > used by the process since libc already has a strong definition of 
> > > > > that symbol.
> > > > > 
> > > > > If we really end up in a situation where we only have a weak mmap 
> > > > > symbol around, then this is probably a sufficiently strange setup 
> > > > > that we don't want to be randomly calling that function.
> > > > All (with the exception of the one overriden by the leak sanitizer) 
> > > > mmap symbols in the symbol table are Weak bound but none are External 
> > > > bound, this was the main reason that lead me to investigate the 
> > > > difference between the two.
> > > > 
> > > > Not sure how to check how lldb interprets the Weak overall, but I think 
> > > > it kind of does, because that's what I saw when I dumped the symbol 
> > > > table:
> > > > ```
> > > > (lldb) target modules dump symtab libc.so.6
> > > >Debug symbol
> > > >|Synthetic symbol
> > > >||Externally Visible
> > > >|||
> > > > Index   UserID DSX TypeFile Address/Value Load Address  
> > > >  Size   Flags  Name
> > > > --- -- --- --- -- 
> > > > -- -- -- 
> > > > --
> > > > ...
> > > > [ 6945]   6947   X Code0x0010b5e0 
> > > > 0x769db5e0 0x00f9 0x0012 __mmap
> > > > ...
> > > > ```
> > > > 
> > > > Another solution I thought was to add a IsLocal to the Symbol (and use 
> > > > !IsLocal) but then not really sure how to implement this for all 
> > > > Symbols not ELFSymbol.
> > > Interesting. I guess I should have verified my assumptions. With that in 
> > > mind, I think checking for both weak and external (strong) symbols is 
> > > fine.
> > I think this is a bug in the ObjectFileELF where it is setting weak but not 
> > external for weak symbols. Weak symbols would be useless if they weren't 
> > external as the first one to get used should end up winning and all other 
> > weak symbols should resolve to the first symbol to come along that matches 
> > when dyld does a lookup. So I think we should verify this with some linker 
> > folks and remove IsWeak from this if statement, and fix the ObjectFileELF 
> > symbol table parsing code.
> That sounds reasonable. The documentation for SBSymbol::IsExternal says "A 
> read only property that returns a boolean value that indicates if this symbol 
> is externally visible (exported) from the module that contains it." An elf 
> weak symbol definitely fits that definition.
> 
> Note that this would be a different meaning of "external" than what is used 
> for the "external" llvm [[ https://llvm.org/docs/LangRef.html#linkage-types | 
> linkage type ]]. There, it refers to a specific linkage type (externally 
> visible and strong), distinct from weak (and linkonce, etc) linkage types, 
> though most of them are still externally visible.
I checked ObjectFileMachO and it indeed sets external and weak separately. For 
the debugger, we just want to know if the symbol is externally visible from an 
object file standpoint. So we can fix ObjectFileELF.cpp with code:

```
const bool is_weak = symbol.getBinding() == STB_WEAK;
Symbol dc_symbol(
i + start_id, // ID is the original symbol table index.
mangled,
symbol_type,// Type of this symbol
is_global | is_weak,// Is this globally visible?
false,  // Is this symbol debug info?
false,  // Is this symbol a trampoline?
false,  // Is this symbol artificial?
AddressRange(symbol_section_sp, // Section in which this symbol is
  

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

[Ilya Bukonkin via Phabricator via lldb-commits]

fallkrum updated this revision to Diff 298445.
fallkrum added a comment.

API test added.


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

https://reviews.llvm.org/D88483

Files:
  lldb/bindings/interface/SBType.i
  lldb/include/lldb/API/SBModule.h
  lldb/include/lldb/API/SBType.h
  lldb/include/lldb/Symbol/Type.h
  lldb/source/API/SBType.cpp
  lldb/source/Symbol/Type.cpp
  lldb/test/API/functionalities/type_get_module/Makefile
  lldb/test/API/functionalities/type_get_module/TestTypeGetModule.py
  lldb/test/API/functionalities/type_get_module/compile_unit1.c
  lldb/test/API/functionalities/type_get_module/compile_unit2.c
  lldb/test/API/functionalities/type_get_module/main.c

Index: lldb/test/API/functionalities/type_get_module/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/type_get_module/main.c
@@ -0,0 +1,2 @@
+
+int main() { return 0; }
Index: lldb/test/API/functionalities/type_get_module/compile_unit2.c
===
--- /dev/null
+++ lldb/test/API/functionalities/type_get_module/compile_unit2.c
@@ -0,0 +1,7 @@
+
+struct compile_unit2_type {
+  int x;
+  int y;
+};
+
+struct compile_unit2_type compile_unit2_var = {2, 2};
Index: lldb/test/API/functionalities/type_get_module/compile_unit1.c
===
--- /dev/null
+++ lldb/test/API/functionalities/type_get_module/compile_unit1.c
@@ -0,0 +1,7 @@
+
+struct compile_unit1_type {
+  int x;
+  int y;
+};
+
+struct compile_unit1_type compile_unit1_var = {1, 1};
Index: lldb/test/API/functionalities/type_get_module/TestTypeGetModule.py
===
--- /dev/null
+++ lldb/test/API/functionalities/type_get_module/TestTypeGetModule.py
@@ -0,0 +1,46 @@
+"""
+Test that SBType returns SBModule of executable file but not
+of compile unit's object file.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class TestTypeGetModule(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+target  = lldbutil.run_to_breakpoint_make_target(self)
+exeModule = target.GetModuleAtIndex(0)
+
+type1Name = 'compile_unit1_type'
+type2Name = 'compile_unit2_type'
+
+numCompUnits = exeModule.GetNumCompileUnits()
+self.assertEqual(numCompUnits, 3)
+
+compUnit = exeModule.GetCompileUnitAtIndex(1)
+typeName = compUnit.GetTypes().GetTypeAtIndex(0).GetName()
+self.assertEqual(typeName, type1Name)
+
+compUnit = exeModule.GetCompileUnitAtIndex(2)
+typeName = compUnit.GetTypes().GetTypeAtIndex(0).GetName()
+self.assertEqual(typeName, type2Name)
+
+type1 = target.FindFirstType(type1Name)
+self.assertTrue(type1.IsValid() == True)
+
+type2 = target.FindFirstType(type2Name)
+self.assertTrue(type2.IsValid() == True)
+
+type1Module = type1.GetModule()
+type2Module = type2.GetModule()
+
+result = \
+(exeModule == type1Module) and (exeModule == type2Module)
+self.assertTrue(result == True)
Index: lldb/test/API/functionalities/type_get_module/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/type_get_module/Makefile
@@ -0,0 +1,3 @@
+
+C_SOURCES := main.c compile_unit1.c compile_unit2.c
+include Makefile.rules
Index: lldb/source/Symbol/Type.cpp
===
--- lldb/source/Symbol/Type.cpp
+++ lldb/source/Symbol/Type.cpp
@@ -727,6 +727,14 @@
   return ModuleSP();
 }
 
+ModuleSP Type::GetExeModule() {
+  if (m_compiler_type) {
+SymbolFile *symbol_file = m_compiler_type.GetTypeSystem()->GetSymbolFile();
+return symbol_file->GetObjectFile()->GetModule();
+  }
+  return ModuleSP();
+}
+
 TypeAndOrName::TypeAndOrName(TypeSP &in_type_sp) {
   if (in_type_sp) {
 m_compiler_type = in_type_sp->GetForwardCompilerType();
@@ -821,6 +829,7 @@
 void TypeImpl::SetType(const lldb::TypeSP &type_sp) {
   if (type_sp) {
 m_static_type = type_sp->GetForwardCompilerType();
+m_exe_module_wp = type_sp->GetExeModule();
 m_module_wp = type_sp->GetModule();
   } else {
 m_static_type.Clear();
@@ -847,6 +856,15 @@
 }
 
 bool TypeImpl::CheckModule(lldb::ModuleSP &module_sp) const {
+  return CheckModuleCommon(m_module_wp, module_sp);
+}
+
+bool TypeImpl::CheckExeModule(lldb::ModuleSP &module_sp) const {
+  return CheckModuleCommon(m_exe_module_wp, module_sp);
+}
+
+bool TypeImpl::CheckModuleCommon(const lldb::ModuleWP &input_module_wp,
+ lldb::ModuleSP &module_sp) const {
   // Check if we have a module for this type. 

[Lldb-commits] [PATCH] D89428: Add support for more OS types to AddClangModuleCompilationOptionsForSDKType()

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 298453.
aprantl added a comment.

Address review feedback


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

https://reviews.llvm.org/D89428

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1521,25 +1521,20 @@
 
   StreamString minimum_version_option;
   bool use_current_os_version = false;
+  // If the SDK type is for the host OS, use its version number.
+  auto get_host_os = []() { return HostInfo::GetTargetTriple().getOS(); };
   switch (sdk_type) {
+  case XcodeSDK::Type::MacOSX:
+use_current_os_version = get_host_os() == llvm::Triple::MacOSX;
+break;
   case XcodeSDK::Type::iPhoneOS:
-#if defined(__arm__) || defined(__arm64__) || defined(__aarch64__)
-use_current_os_version = true;
-#else
-use_current_os_version = false;
-#endif
+use_current_os_version = get_host_os() == llvm::Triple::IOS;
 break;
-
-  case XcodeSDK::Type::iPhoneSimulator:
-use_current_os_version = false;
+  case XcodeSDK::Type::AppleTVOS:
+use_current_os_version = get_host_os() == llvm::Triple::TvOS;
 break;
-
-  case XcodeSDK::Type::MacOSX:
-#if defined(__i386__) || defined(__x86_64__)
-use_current_os_version = true;
-#else
-use_current_os_version = false;
-#endif
+  case XcodeSDK::Type::watchOS:
+use_current_os_version = get_host_os() == llvm::Triple::WatchOS;
 break;
   default:
 break;
@@ -1559,31 +1554,43 @@
 }
   }
   // Only add the version-min options if we got a version from somewhere
-  if (!version.empty()) {
+  if (!version.empty() && sdk_type != XcodeSDK::Type::Linux) {
+#define OPTION(PREFIX, NAME, VAR, ...) const char *opt_##VAR = NAME;
+#include "clang/Driver/Options.inc"
+#undef OPTION
+minimum_version_option.PutCString("-");
 switch (sdk_type) {
-case XcodeSDK::Type::iPhoneOS:
-  minimum_version_option.PutCString("-mios-version-min=");
-  minimum_version_option.PutCString(version.getAsString());
+case XcodeSDK::Type::MacOSX:
+  minimum_version_option.PutCString(opt_mmacosx_version_min_EQ);
   break;
 case XcodeSDK::Type::iPhoneSimulator:
-  minimum_version_option.PutCString("-mios-simulator-version-min=");
-  minimum_version_option.PutCString(version.getAsString());
+  minimum_version_option.PutCString(opt_mios_simulator_version_min_EQ);
   break;
-case XcodeSDK::Type::MacOSX:
-  minimum_version_option.PutCString("-mmacosx-version-min=");
-  minimum_version_option.PutCString(version.getAsString());
+case XcodeSDK::Type::iPhoneOS:
+  minimum_version_option.PutCString(opt_mios_version_min_EQ);
+  break;
+case XcodeSDK::Type::AppleTVSimulator:
+  minimum_version_option.PutCString(opt_mtvos_simulator_version_min_EQ);
+  break;
+case XcodeSDK::Type::AppleTVOS:
+  minimum_version_option.PutCString(opt_mtvos_version_min_EQ);
   break;
 case XcodeSDK::Type::WatchSimulator:
-  minimum_version_option.PutCString("-mwatchos-simulator-version-min=");
-  minimum_version_option.PutCString(version.getAsString());
+  minimum_version_option.PutCString(opt_mwatchos_simulator_version_min_EQ);
   break;
-case XcodeSDK::Type::AppleTVSimulator:
-  minimum_version_option.PutCString("-mtvos-version-min=");
-  minimum_version_option.PutCString(version.getAsString());
+case XcodeSDK::Type::watchOS:
+  minimum_version_option.PutCString(opt_mwatchos_version_min_EQ);
   break;
-default:
-  llvm_unreachable("unsupported sdk");
+case XcodeSDK::Type::bridgeOS:
+  LLDB_LOGF(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST),
+"Clang modules on bridgeOS are not supported");
+  return;
+case XcodeSDK::Type::Linux:
+  LLDB_LOGF(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST),
+"Clang modules on Linux are not supported");
+  return;
 }
+minimum_version_option.PutCString(version.getAsString());
 options.push_back(std::string(minimum_version_option.GetString()));
   }
 


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1521,25 +1521,20 @@
 
   StreamString minimum_version_option;
   bool use_current_os_version = false;
+  // If the SDK type is for the host OS, use its version number.
+  auto get_host_os = []() { return HostInfo::GetTargetTriple().getOS(); };
   switch (sdk_type) {
+  case XcodeSDK::Type::MacOSX:
+use_current_os_version = get_host_os() == llvm::Triple::MacOSX;
+break;
   case XcodeSDK::Type::iPhoneOS:
-#if defined(__arm__) 

[Lldb-commits] [PATCH] D89358: Add an API to get an SBBreakpoint's owning SBTarget

[Jim Ingham 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 rG6754caa9bf21: Add an SB API to get the SBTarget from an 
SBBreakpoint (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89358

Files:
  lldb/bindings/interface/SBBreakpoint.i
  lldb/include/lldb/API/SBBreakpoint.h
  lldb/source/API/SBBreakpoint.cpp
  lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py


Index: lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
===
--- lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
+++ lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
@@ -66,6 +66,9 @@
 location = breakpoint.GetLocationAtIndex(0)
 self.assertTrue(location.IsValid())
 
+# Make sure the breakpoint's target is right:
+self.assertEqual(target, breakpoint.GetTarget(), "Breakpoint reports 
its target correctly")
+
 self.assertTrue(self.dbg.DeleteTarget(target))
 self.assertFalse(breakpoint.IsValid())
 self.assertFalse(location.IsValid())
Index: lldb/source/API/SBBreakpoint.cpp
===
--- lldb/source/API/SBBreakpoint.cpp
+++ lldb/source/API/SBBreakpoint.cpp
@@ -81,6 +81,16 @@
   return m_opaque_wp.lock() != rhs.m_opaque_wp.lock();
 }
 
+SBTarget SBBreakpoint::GetTarget() const {
+  LLDB_RECORD_METHOD_CONST_NO_ARGS(lldb::SBTarget, SBBreakpoint, GetTarget);
+
+  BreakpointSP bkpt_sp = GetSP();
+  if (bkpt_sp)
+return LLDB_RECORD_RESULT(SBTarget(bkpt_sp->GetTargetSP()));
+
+  return LLDB_RECORD_RESULT(SBTarget());
+}
+
 break_id_t SBBreakpoint::GetID() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(lldb::break_id_t, SBBreakpoint, GetID);
 
@@ -987,6 +997,7 @@
SBBreakpoint, operator==,(const lldb::SBBreakpoint &));
   LLDB_REGISTER_METHOD(bool,
SBBreakpoint, operator!=,(const lldb::SBBreakpoint &));
+  LLDB_REGISTER_METHOD_CONST(lldb::SBTarget, SBBreakpoint, GetTarget, ());
   LLDB_REGISTER_METHOD_CONST(lldb::break_id_t, SBBreakpoint, GetID, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBBreakpoint, IsValid, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBBreakpoint, operator bool, ());
Index: lldb/include/lldb/API/SBBreakpoint.h
===
--- lldb/include/lldb/API/SBBreakpoint.h
+++ lldb/include/lldb/API/SBBreakpoint.h
@@ -42,6 +42,8 @@
 
   void ClearAllBreakpointSites();
 
+  lldb::SBTarget GetTarget() const;
+
   lldb::SBBreakpointLocation FindLocationByAddress(lldb::addr_t vm_addr);
 
   lldb::break_id_t FindLocationIDByAddress(lldb::addr_t vm_addr);
Index: lldb/bindings/interface/SBBreakpoint.i
===
--- lldb/bindings/interface/SBBreakpoint.i
+++ lldb/bindings/interface/SBBreakpoint.i
@@ -100,6 +100,9 @@
 void
 ClearAllBreakpointSites ();
 
+lldb::SBTarget
+GetTarget() const;
+  
 lldb::SBBreakpointLocation
 FindLocationByAddress (lldb::addr_t vm_addr);
 


Index: lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
===
--- lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
+++ lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
@@ -66,6 +66,9 @@
 location = breakpoint.GetLocationAtIndex(0)
 self.assertTrue(location.IsValid())
 
+# Make sure the breakpoint's target is right:
+self.assertEqual(target, breakpoint.GetTarget(), "Breakpoint reports its target correctly")
+
 self.assertTrue(self.dbg.DeleteTarget(target))
 self.assertFalse(breakpoint.IsValid())
 self.assertFalse(location.IsValid())
Index: lldb/source/API/SBBreakpoint.cpp
===
--- lldb/source/API/SBBreakpoint.cpp
+++ lldb/source/API/SBBreakpoint.cpp
@@ -81,6 +81,16 @@
   return m_opaque_wp.lock() != rhs.m_opaque_wp.lock();
 }
 
+SBTarget SBBreakpoint::GetTarget() const {
+  LLDB_RECORD_METHOD_CONST_NO_ARGS(lldb::SBTarget, SBBreakpoint, GetTarget);
+
+  BreakpointSP bkpt_sp = GetSP();
+  if (bkpt_sp)
+return LLDB_RECORD_RESULT(SBTarget(bkpt_sp->GetTargetSP()));
+
+  return LLDB_RECORD_RESULT(SBTarget());
+}
+
 break_id_t SBBreakpoint::GetID() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(lldb::break_id_t, SBBreakpoint, GetID);
 
@@ -987,6 +997,7 @@
SBBreakpoint, operator==,(const lldb::SBBreakpoint &));
   LLDB_REGISTER_METHOD(bool,
SBBreakpoint, operator!=,(const lldb::SBBreakpoint &));
+  LLDB_REGISTER_METHOD_CONST(lldb::SBTarget, SBBreakpoint, GetTarget, ());
   LLDB_REGISTER_METHOD_CONST(lldb::break_id_t, SBBreakpoint, GetID, ());
   LLDB_REGISTER_METHOD_CO

[Lldb-commits] [lldb] 6754caa - Add an SB API to get the SBTarget from an SBBreakpoint

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2020-10-15T14:28:44-07:00
New Revision: 6754caa9bf21a759c4080a797f23e2b7a77a71e1

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

LOG: Add an SB API to get the SBTarget from an SBBreakpoint

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

Added: 


Modified: 
lldb/bindings/interface/SBBreakpoint.i
lldb/include/lldb/API/SBBreakpoint.h
lldb/source/API/SBBreakpoint.cpp
lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py

Removed: 




diff  --git a/lldb/bindings/interface/SBBreakpoint.i 
b/lldb/bindings/interface/SBBreakpoint.i
index e386ace9dee8..696795241b11 100644
--- a/lldb/bindings/interface/SBBreakpoint.i
+++ b/lldb/bindings/interface/SBBreakpoint.i
@@ -100,6 +100,9 @@ public:
 void
 ClearAllBreakpointSites ();
 
+lldb::SBTarget
+GetTarget() const;
+  
 lldb::SBBreakpointLocation
 FindLocationByAddress (lldb::addr_t vm_addr);
 

diff  --git a/lldb/include/lldb/API/SBBreakpoint.h 
b/lldb/include/lldb/API/SBBreakpoint.h
index 39a021145fb7..e13dbc5c3516 100644
--- a/lldb/include/lldb/API/SBBreakpoint.h
+++ b/lldb/include/lldb/API/SBBreakpoint.h
@@ -42,6 +42,8 @@ class LLDB_API SBBreakpoint {
 
   void ClearAllBreakpointSites();
 
+  lldb::SBTarget GetTarget() const;
+
   lldb::SBBreakpointLocation FindLocationByAddress(lldb::addr_t vm_addr);
 
   lldb::break_id_t FindLocationIDByAddress(lldb::addr_t vm_addr);

diff  --git a/lldb/source/API/SBBreakpoint.cpp 
b/lldb/source/API/SBBreakpoint.cpp
index 96b77bd8539e..96ae305ffce5 100644
--- a/lldb/source/API/SBBreakpoint.cpp
+++ b/lldb/source/API/SBBreakpoint.cpp
@@ -81,6 +81,16 @@ bool SBBreakpoint::operator!=(const lldb::SBBreakpoint &rhs) 
{
   return m_opaque_wp.lock() != rhs.m_opaque_wp.lock();
 }
 
+SBTarget SBBreakpoint::GetTarget() const {
+  LLDB_RECORD_METHOD_CONST_NO_ARGS(lldb::SBTarget, SBBreakpoint, GetTarget);
+
+  BreakpointSP bkpt_sp = GetSP();
+  if (bkpt_sp)
+return LLDB_RECORD_RESULT(SBTarget(bkpt_sp->GetTargetSP()));
+
+  return LLDB_RECORD_RESULT(SBTarget());
+}
+
 break_id_t SBBreakpoint::GetID() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(lldb::break_id_t, SBBreakpoint, GetID);
 
@@ -987,6 +997,7 @@ void RegisterMethods(Registry &R) {
SBBreakpoint, operator==,(const lldb::SBBreakpoint &));
   LLDB_REGISTER_METHOD(bool,
SBBreakpoint, operator!=,(const lldb::SBBreakpoint &));
+  LLDB_REGISTER_METHOD_CONST(lldb::SBTarget, SBBreakpoint, GetTarget, ());
   LLDB_REGISTER_METHOD_CONST(lldb::break_id_t, SBBreakpoint, GetID, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBBreakpoint, IsValid, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBBreakpoint, operator bool, ());

diff  --git a/lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py 
b/lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
index 1c0c334fbeeb..80324ee61b70 100644
--- a/lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
+++ b/lldb/test/API/python_api/breakpoint/TestBreakpointAPI.py
@@ -66,6 +66,9 @@ def test_target_delete(self):
 location = breakpoint.GetLocationAtIndex(0)
 self.assertTrue(location.IsValid())
 
+# Make sure the breakpoint's target is right:
+self.assertEqual(target, breakpoint.GetTarget(), "Breakpoint reports 
its target correctly")
+
 self.assertTrue(self.dbg.DeleteTarget(target))
 self.assertFalse(breakpoint.IsValid())
 self.assertFalse(location.IsValid())



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

[Lldb-commits] [PATCH] D89477: [lldb] Port lldb gdb-server to libOption

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added inline comments.



Comment at: lldb/tools/lldb-server/CMakeLists.txt:61
 LINK_COMPONENTS
   Support
 )

Otherwise it fails in a BUILD_SHARED_LIBS=on build because the -Wl,-z,defs 
linker option requires a module to have fully specified dependencies


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89477

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

[Lldb-commits] [PATCH] D89428: Add support for more OS types to AddClangModuleCompilationOptionsForSDKType()

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1525
+  // If the SDK type is for the host OS, use its version number.
+  auto get_host_os = []() { return HostInfo::GetTargetTriple().getOS(); };
   switch (sdk_type) {

teemperor wrote:
> `Triple::OSType host_os = HostInfo::GetTargetTriple().getOS();` ?
I think the idea was to avoid pointlessly calling it for the simulators.



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1576
+case XcodeSDK::Type::AppleTVOS:
+  minimum_version_option.PutCString(opt_mtvos_simulator_version_min_EQ);
   break;

teemperor wrote:
> `opt_mtvos_version_min_EQ`
Thank you!



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1585
+case XcodeSDK::Type::bridgeOS:
+  minimum_version_option.PutCString(opt_mwatchos_version_min_EQ);
+  break;

teemperor wrote:
> If this is on purpose, then I think there should be a comment explaining why 
> this is not `-mbridgeos-version-min=` ?
The Clang support for this doesn't exist on llvm.org. I have removed it.


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

https://reviews.llvm.org/D89428

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

[Lldb-commits] [PATCH] D89408: [trace] rename ThreadIntelPT to TraceThread

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 298493.
wallace added a comment.
Herald added a reviewer: JDevlieghere.

Addressed the issues that @labath pointed out regarding the TraceProcess and 
TraceThread:

- These classes were not together
- TraceProcess was an optional plug-in, which would break the Trace parsing 
logic if plugged-out

So I ended up moving TraceProcess and TraceThread next to Trace in LLDB core, 
guaranteeing that the "trace" process plug-in will always exist and now the 
source file structure looks better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89408

Files:
  lldb/include/lldb/Target/TraceProcess.h
  lldb/include/lldb/Target/TraceSessionFileParser.h
  lldb/include/lldb/Target/TraceThread.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/Process/CMakeLists.txt
  lldb/source/Plugins/Process/Trace/CMakeLists.txt
  lldb/source/Plugins/Process/Trace/ProcessTrace.cpp
  lldb/source/Plugins/Process/Trace/ProcessTrace.h
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/ThreadIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/TraceProcess.cpp
  lldb/source/Target/TraceSessionFileParser.cpp
  lldb/source/Target/TraceThread.cpp

Index: lldb/source/Target/TraceThread.cpp
===
--- lldb/source/Target/TraceThread.cpp
+++ lldb/source/Target/TraceThread.cpp
@@ -1,4 +1,4 @@
-//===-- ThreadIntelPT.cpp -===//
+//===-- TraceThread.cpp ---===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#include "ThreadIntelPT.h"
+#include "lldb/Target/TraceThread.h"
 
 #include 
 
@@ -16,11 +16,10 @@
 
 using namespace lldb;
 using namespace lldb_private;
-using namespace lldb_private::trace_intel_pt;
 
-void ThreadIntelPT::RefreshStateAfterStop() {}
+void TraceThread::RefreshStateAfterStop() {}
 
-RegisterContextSP ThreadIntelPT::GetRegisterContext() {
+RegisterContextSP TraceThread::GetRegisterContext() {
   if (!m_reg_context_sp)
 m_reg_context_sp = CreateRegisterContextForFrame(nullptr);
 
@@ -28,11 +27,13 @@
 }
 
 RegisterContextSP
-ThreadIntelPT::CreateRegisterContextForFrame(StackFrame *frame) {
+TraceThread::CreateRegisterContextForFrame(StackFrame *frame) {
   // Eventually this will calculate the register context based on the current
   // trace position.
   return std::make_shared(
   *this, 0, GetProcess()->GetAddressByteSize(), LLDB_INVALID_ADDRESS);
 }
 
-bool ThreadIntelPT::CalculateStopInfo() { return false; }
+bool TraceThread::CalculateStopInfo() { return false; }
+
+const FileSpec &TraceThread::GetTraceFile() const { return m_trace_file; }
Index: lldb/source/Target/TraceSessionFileParser.cpp
===
--- lldb/source/Target/TraceSessionFileParser.cpp
+++ lldb/source/Target/TraceSessionFileParser.cpp
@@ -10,8 +10,11 @@
 
 #include 
 
+#include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
+#include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/TraceThread.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -87,6 +90,81 @@
   return schema_builder.str();
 }
 
+TraceThreadSP TraceSessionFileParser::ParseThread(ProcessSP &process_sp,
+  const JSONThread &thread) {
+  lldb::tid_t tid = static_cast(thread.tid);
+
+  FileSpec trace_file(thread.trace_file);
+  NormalizePath(trace_file);
+
+  TraceThreadSP thread_sp =
+  std::make_shared(*process_sp, tid, trace_file);
+  process_sp->GetThreadList().AddThread(thread_sp);
+  return thread_sp;
+}
+
+Expected
+TraceSessionFileParser::ParseProcess(const JSONProcess &process) {
+  TargetSP target_sp;
+  Status error = m_debugger.GetTargetList().CreateTarget(
+  m_debugger, /*user_exe_path*/ StringRef(), process.triple,
+  eLoadDependentsNo,
+  /*platform_options*/ nullptr, target_sp);
+
+  if (!target_sp)
+return error.ToError();
+
+  ParsedProcess parsed_process;
+  parsed_process.target_sp = target_sp;
+
+  m_debugger.GetTargetList().SetSelectedTarget(target_sp.get());
+
+  ProcessSP process_sp = target_sp->CreateProcess(
+  /*listener*/ nullptr, "trace",
+  /*crash_file*/ nullptr);
+
+  process_sp->SetID(static_cast(process.p

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 298495.
wallace added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89283

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Target/TraceProcess.h
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/Trace.cpp
  lldb/source/Target/TraceProcess.cpp
  lldb/source/Target/TraceSessionFileParser.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/intelpt-trace-multi-file/a.out
  lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.cpp
  lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.h
  lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.cpp
  lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.h
  lldb/test/API/commands/trace/intelpt-trace-multi-file/ld-2.17.so
  lldb/test/API/commands/trace/intelpt-trace-multi-file/libbar.so
  lldb/test/API/commands/trace/intelpt-trace-multi-file/libfoo.so
  lldb/test/API/commands/trace/intelpt-trace-multi-file/main.cpp
  lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file-no-ld.json
  lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file.json
  lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file.trace
  lldb/test/API/commands/trace/intelpt-trace/trace_bad_image.json
  lldb/test/API/commands/trace/intelpt-trace/trace_wrong_cpu.json
  lldb/tools/intel-features/intel-pt/Decoder.cpp

Index: lldb/tools/intel-features/intel-pt/Decoder.cpp
===
--- lldb/tools/intel-features/intel-pt/Decoder.cpp
+++ lldb/tools/intel-features/intel-pt/Decoder.cpp
@@ -248,6 +248,8 @@
   }
 }
 
+#include 
+
 void Decoder::ReadTraceDataAndImageInfo(lldb::SBProcess &sbprocess,
 lldb::tid_t tid, lldb::SBError &sberror,
 ThreadTraceInfo &threadTraceInfo) {
@@ -255,6 +257,7 @@
   // for the first time in class
   lldb::SBTrace &trace = threadTraceInfo.GetUniqueTraceInstance();
   Buffer &pt_buffer = threadTraceInfo.GetPTBuffer();
+
   lldb::SBError error;
   if (pt_buffer.size() == 0) {
 lldb::SBTraceOptions traceoptions;
@@ -295,6 +298,11 @@
   }
   std::fill(pt_buffer.begin() + bytes_written, pt_buffer.end(), 0);
 
+  std::ofstream of("/tmp/multi-file.trace", std::ios::binary | std::ios::out);
+  for (auto bait : pt_buffer)
+of << bait;
+  of.close();
+
   // Get information of all the modules of the inferior
   lldb::SBTarget sbtarget = sbprocess.GetTarget();
   ReadExecuteSectionInfos &readExecuteSectionInfos =
Index: lldb/test/API/commands/trace/intelpt-trace/trace_wrong_cpu.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace_wrong_cpu.json
@@ -0,0 +1,31 @@
+{
+  "trace": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 2123123,
+  "model": 12123123,
+  "stepping": 1231231
+}
+  },
+  "processes": [
+{
+  "pid": 1234,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 3842849,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+}
+  ]
+}
Index: lldb/test/API/commands/trace/intelpt-trace/trace_bad_image.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace_bad_image.json
@@ -0,0 +1,31 @@
+{
+  "trace": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "processes": [
+{
+  "pid": 1234,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 3842849,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x00F0",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+}
+  ]
+}
Index: lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file.json
@@ -0,0 +1,49 @@
+{
+  "trace": {
+"type": "intel-pt"

Re: [Lldb-commits] [lldb] ea3a547 - [lldb] Remove bogus ProcessMonitor forward-decls

[Ed Maste via lldb-commits]

On Wed, 14 Oct 2020 at 10:44, Pavel Labath via lldb-commits
 wrote:
>
> Author: Pavel Labath
> Date: 2020-10-14T16:43:45+02:00
> New Revision: ea3a547f0be20d86b041778ae8e2779f2031f714
>
> URL: 
> https://github.com/llvm/llvm-project/commit/ea3a547f0be20d86b041778ae8e2779f2031f714
> DIFF: 
> https://github.com/llvm/llvm-project/commit/ea3a547f0be20d86b041778ae8e2779f2031f714.diff
>
> LOG: [lldb] Remove bogus ProcessMonitor forward-decls
>
> This class is not used in those files.
...
> diff  --git 
> a/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.h 
> b/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.h
> index 1843a2a6aff3..b66dc3f44524 100644
> --- a/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.h
> +++ b/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.h
> @@ -14,10 +14,6 @@
>  #include "lldb/Target/RegisterContext.h"
>  #include "lldb/Utility/Log.h"
>
> -using namespace lldb_private;
> -
> -class ProcessMonitor;
> -

The build failed with:

In file included from
/tmp/cirrus-ci-build/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp:30:
/tmp/cirrus-ci-build/lldb/source/Plugins/Process/FreeBSD/RegisterContextPOSIXProcessMonitor_mips64.h:76:16:
error: use of undeclared identifier 'k_num_gpr_registers_mips64'; did
you mean 'lldb_private::k_num_gpr_registers_mips64'?
m_gpr_mips64[k_num_gpr_registers_mips64]; // general purpose registers.
^~
lldb_private::k_num_gpr_registers_mips64
/tmp/cirrus-ci-build/lldb/source/./Plugins/Process/Utility/lldb-mips-freebsd-register-enums.h:62:3:
note: 'lldb_private::k_num_gpr_registers_mips64' declared here
k_num_gpr_registers_mips64 = k_last_gpr_mips64 - k_first_gpr_mips64 + 1
^
1 error generated.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D89477: [lldb] Port lldb gdb-server to libOption

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added inline comments.



Comment at: lldb/tools/lldb-server/lldb-gdbserver.cpp:404
+  unsigned MissingArgCount;
+  opt::InputArgList Args = Opts.ParseArgs(makeArrayRef(argv + 2, argc - 2),
+  MissingArgIndex, MissingArgCount);

Consider using `parseArgs` I added in D83639


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89477

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

[Lldb-commits] [PATCH] D89477: [lldb] Port lldb gdb-server to libOption

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added inline comments.



Comment at: lldb/tools/lldb-server/lldb-gdbserver.cpp:369
+
+  If no target is selected a startup, the lldb-server can be directed to launch
+  or attach a process by the LLDB client.

`a startup` -> `at startup`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89477

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

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

[Med Ismail Bennani via Phabricator via lldb-commits]

mib requested changes to this revision.
mib added a comment.
This revision now requires changes to proceed.

Could please follow PEP8 style guide regarding variable names 
 :

  Function names should be lowercase, with words separated by underscores as 
necessary to improve readability.
  
  Variable names follow the same convention as function names.

I also left some inline suggestions regarding the test.
After changing that, it should be good for me.

Thanks in advance.




Comment at: 
lldb/test/API/functionalities/type_get_module/TestTypeGetModule.py:36
+type1 = target.FindFirstType(type1Name)
+self.assertTrue(type1.IsValid() == True)
+





Comment at: 
lldb/test/API/functionalities/type_get_module/TestTypeGetModule.py:39
+type2 = target.FindFirstType(type2Name)
+self.assertTrue(type2.IsValid() == True)
+





Comment at: 
lldb/test/API/functionalities/type_get_module/TestTypeGetModule.py:41-46
+type1Module = type1.GetModule()
+type2Module = type2.GetModule()
+
+result = \
+(exeModule == type1Module) and (exeModule == type2Module)
+self.assertTrue(result == True)




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

https://reviews.llvm.org/D88483

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

[Lldb-commits] [PATCH] D89428: Add support for more OS types to AddClangModuleCompilationOptionsForSDKType()

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1593
 }
+minimum_version_option.PutCString(version.getAsString());
 options.push_back(std::string(minimum_version_option.GetString()));

I think this applies to the rest of the function and would make things a bit 
more readable, but here it would certainly improve things. 



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1594
+minimum_version_option.PutCString(version.getAsString());
 options.push_back(std::string(minimum_version_option.GetString()));
   }

Would save a copy 🤷‍♂️


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

https://reviews.llvm.org/D89428

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

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Symbol/Type.h:122
+   GetModule and GetExeModule may return the same value.
+   */
+  lldb::ModuleSP GetExeModule();

These should be `///` doxygen-style comments. 


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

https://reviews.llvm.org/D88483

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