[Lldb-commits] [PATCH] D77043: Fix process gdb-remote usage of value_regs/invalidate_regs

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 25.
omjavaid added a comment.

Posting full diff.


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

https://reviews.llvm.org/D77043

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Host/common/NativeRegisterContext.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/Utility/NativeRegisterContextRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -553,6 +553,10 @@
   } else if (name.equals("generic")) {
 reg_info.kinds[eRegisterKindGeneric] =
 Args::StringToGenericRegister(value);
+  } else if (name.equals("regnum")) {
+if (value.getAsInteger(0,
+   reg_info.kinds[eRegisterKindProcessPlugin]))
+  reg_info.kinds[eRegisterKindProcessPlugin] = reg_num;
   } else if (name.equals("container-regs")) {
 SplitCommaSeparatedRegisterNumberString(value, value_regs, 16);
   } else if (name.equals("invalidate-regs")) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -242,11 +242,15 @@
   // Index of the primordial register.
   bool success = true;
   for (uint32_t idx = 0; success; ++idx) {
-const uint32_t prim_reg = reg_info->value_regs[idx];
+uint32_t prim_reg = reg_info->value_regs[idx];
 if (prim_reg == LLDB_INVALID_REGNUM)
   break;
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
+uint32_t regnum = ConvertRegisterKindToRegisterNumber(
+eRegisterKindProcessPlugin, prim_reg);
+if (regnum != LLDB_INVALID_REGNUM)
+  prim_reg = regnum;
 const RegisterInfo *prim_reg_info = GetRegisterInfoAtIndex(prim_reg);
 if (prim_reg_info == nullptr)
   success = false;
@@ -375,11 +379,15 @@
   // Invalidate this composite register first.
 
   for (uint32_t idx = 0; success; ++idx) {
-const uint32_t reg = reg_info->value_regs[idx];
+uint32_t reg = reg_info->value_regs[idx];
 if (reg == LLDB_INVALID_REGNUM)
   break;
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
+uint32_t lldb_regnum = ConvertRegisterKindToRegisterNumber(
+eRegisterKindProcessPlugin, reg);
+if (lldb_regnum != LLDB_INVALID_REGNUM)
+  reg = lldb_regnum;
 const RegisterInfo *value_reg_info = GetRegisterInfoAtIndex(reg);
 if (value_reg_info == nullptr)
   success = false;
@@ -397,6 +405,10 @@
   for (uint32_t idx = 0, reg = reg_info->invalidate_regs[0];
reg != LLDB_INVALID_REGNUM;
reg = reg_info->invalidate_regs[++idx]) {
+uint32_t lldb_regnum = ConvertRegisterKindToRegisterNumber(
+eRegisterKindProcessPlugin, reg);
+if (lldb_regnum != LLDB_INVALID_REGNUM)
+  reg = lldb_regnum;
 SetRegisterIsValid(reg, false);
   }
 }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1772,7 +1772,9 @@
   if (reg_index >= reg_context.GetUserRegisterCount())
 return SendErrorResponse(69);
 
-  const RegisterInfo *reg_info = reg_context.GetRegisterInfoAtIndex(reg_index);
+  uint32_t native_reg_index = reg_context.GetNativeRegisterIndex(reg_index);
+  const RegisterInfo *reg_info =
+  reg_context.GetRegisterInfoAtIndex(native_reg_index);
   if (!reg_info)
 return SendErrorResponse(69);
 
@@ -1801,7 +1803,7 @@
 response << "format:" << format << ';';
 
   const char *const register_set_name =
-  reg_context.GetRegisterSetNameForRegisterAtIndex(reg_index);
+  reg_context.GetRegisterSetNameForRegisterAtIndex(native_reg_index);
   if (register_set_name)
 response << "set:" << register_set_name << '

[Lldb-commits] [PATCH] D77186: [source maps] Ensure all valid source maps are added instead of failing with the first invalid one

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D77186#1955640 , @wallace wrote:

> Thanks for the heads up. I think I got a false impression of how comments 
> after accept vs requesting changes work in this repo.


The rules on that are somewhat fuzzy, but there has been a recent effort to 
formalize  that. Generally, in a 
situation like this (where a patch already got one LGTM and so is "green"), I 
would have used "request changes" if I had major objections and was sure I want 
to see the patch again. If I only had cosmetic comments/trivial comments, I 
would have clicked "approve" and trusted you to apply them before committing. 
Not doing either of those is a sort of a "meh" position, where I'm 
uncomfortable lgtm-ing something, but I also don't want to reject it (as that's 
fairly aggressive). In this case, I don't think you have done anything really 
wrong strictly speaking, but it would have been polite to wait a while so that 
I have a chance to respond.




Comment at: lldb/source/Interpreter/OptionValuePathMappings.cpp:73
 changed = true;
+idx++;
   } else {

wallace wrote:
> labath wrote:
> > does this actually change anything?
> Indeed. It's used in line 70 as the index to replace in the replace operation
Yes, but that would have also worked (I think) if you had left the increment in 
the for loop. My question was why did you need to move the increment into the 
loop body.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77186



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

[Lldb-commits] [PATCH] D77043: Fix process gdb-remote usage of value_regs/invalidate_regs

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D77043#1954672 , @labath wrote:

> I am still thinking this over, but for now I have two comments. First, could 
> you re-upload the diff with full context (e.g. `git show -U`). That would 
> make it a lot easier to review this.
>  Second, would it be possible to change the meaning of the `invalidate_regs` 
> and `value_regs` lists so that they do the right thing even in your case 
> (instead of introducing a new number)? We already have too many numbering 
> schemes to begin with, and introducing a new one is definitely something I'd 
> like to avoid (in particular when the number is stored as `eRegisterKindLLDB` 
> on the server side, but then becomes `eRegisterKindProcessPlugin` on the 
> other end).


There has to be a unique register no for each register in a particular 
architecture and that can be single register no (eRegisterKindLLDB for both 
server and host) if we somehow stop the use of index in register info packet. 
Right now register info packet iterates over an index to get next register info 
and uses that as LLDB register no which is in theory wrong. We need to assign 
every register in a particular architecture a unique LLDB specific number and 
exchange that in register info or target xml packets. Right now if we change 
the order of iteration over register set or skip one of the register sets the 
register nos exchanged over target xml will also change. Thats why i felt the 
need for this patch and thought it would be the most backward compatible 
solution available without disturbing any existing functionality specially when 
various other architectures also depend on this.

There


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

https://reviews.llvm.org/D77043



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

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and ptrace support

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 254454.
omjavaid added a comment.

Posting full diff.


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

https://reviews.llvm.org/D77047

Files:
  lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
  lldb/source/Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h
  lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h
  lldb/source/Plugins/Process/Utility/lldb-arm64-register-enums.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Utility/ARM64_DWARF_Registers.h
  lldb/source/Utility/ARM64_ehframe_Registers.h
  lldb/test/API/commands/register/register/aarch64_sve_registers/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_registers/TestSVERegisters.py
  lldb/test/API/commands/register/register/aarch64_sve_registers/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_registers/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/main.c
@@ -0,0 +1,5 @@
+int main() {
+  asm volatile("ptrue p0.s\n\t");
+  asm volatile("fcpy  z0.s, p0/m, #5.\n\t");
+  return 0; // Set a break point here.
+}
\ No newline at end of file
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/TestSVERegisters.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/TestSVERegisters.py
@@ -0,0 +1,128 @@
+"""
+Test the AArch64 SVE registers.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class RegisterCommandsTestCase(TestBase):
+
+def check_sve_register_size(self, set, name, expected):
+reg_value = set.GetChildMemberWithName(name)
+self.assertTrue(reg_value.IsValid(),
+'Verify we have a register named "%s"' % (name))
+self.assertEqual(reg_value.GetByteSize(), expected,
+ 'Verify "%s" == %i' % (name, expected))
+
+mydir = TestBase.compute_mydir(__file__)
+@skipIf
+def test_sve_registers_configuration(self):
+"""Test AArch64 SVE registers size configuration."""
+self.build()
+self.line = line_number('main.c', '// Set a break point here.')
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(self, "main.c", self.line, num_expected_locations=1)
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+substrs = ["stop reason = breakpoint 1."])
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+thread = process.GetThreadAtIndex(0)
+currentFrame = thread.GetFrameAtIndex(0)
+
+has_sve = False
+for registerSet in currentFrame.GetRegisters():
+if 'sve registers' in registerSet.GetName().lower():
+has_sve = True
+
+if not has_sve:
+self.skipTest('SVE registers must be supported.')
+
+registerSets = process.GetThreadAtIndex(0).GetFrameAtIndex(0).GetRegisters()
+
+sve_registers = registerSets.GetValueAtIndex(2)
+
+vg_reg = sve_registers.GetChildMemberWithName("vg")
+
+vg_reg_value = sve_registers.GetChildMemberWithName("vg").GetValueAsUnsigned()
+
+z_reg_size = vg_reg_value * 8
+
+p_reg_size = z_reg_size / 8
+
+for i in range(32):
+self.check_sve_register_size(sve_registers, 'z%i' % (i), z_reg_size)
+
+for i in range(16):
+self.check_sve_register_size(sve_registers, 'p%i' % (i), p_reg_size)
+
+self.check_sve_register_size(sve_registers, 'ffr', p_reg_size)
+
+mydir = TestBase.compute_mydir(__file__)
+@no_debug_info_test
+@skipIf
+def test_sve_registers_read_write(self):
+"""Test AArch64 SVE registers read and write."""
+self.build()
+self.line = line_number('main.c', '// Set a break point here.')
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(self, "main.c", self.line, num_expected_locations=1)
+

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and ptrace support

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D77047#1954696 , @labath wrote:

> > There is no physical hardware currently available to test SVE and we make 
> > use of QEMU for the purpose of testing this feature.
>
> Are these registers presented in core files in any way? Would it be possible 
> to at least test the `RegisterInfoPOSIX_arm64.h` changes that way?


Yes. Core file section is added for SVE registers but i have not tried to get 
that working with LLDB. I will get back to you on this.


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

https://reviews.llvm.org/D77047



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

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D77153#1955829 , @friss wrote:

> I was checking whether there is a way to catch null pointer before a type 
> summary is even chosen. And found out that such logic exists, but only for 
> Objective-C. Languages have a `IsNilReference()` virtual method that can tell 
> whether a `ValueObject` is a nil reference. It's only implemented for 
> ObjectiveC and value objects that test positive for this method are displayed 
> as "nil" is the generic code of `ValueObjectPrinter.cpp`. I can see a few 
> options:
>
> - I can rework the patch to affect only the libc++ string formatter which was 
> the issue I was trying to solve in the first place
> - I can implement `IsNilReference()` for C++ and change `ValueObjectPrinter` 
> to print C++ "nil references" as "nullptr".
> - I do the above and add another customization point in the Language plugins 
> which tells `ValueObjectPrinter` out to display "nil" references, so that 
> there's no Language specific code in `ValueObjectPrinter`.


All of those options sound fine to me. Making that generic is obviously 
preferable, but I don't know how much work that would be.

The thing that I found surprising when looking at this is that `std::string` 
has the exact same summary as a `std::string *` which points to it. I would 
have expected at least some subtle hint (perhaps by prepending the summary with 
a hex pointer value?) to indicate that we're looking at a pointer. This would 
be doable if the generic code had some way to understand pointers (and their 
nullness).

> A few additional questions come to mind though:
> 
> - Was the intent if `IsNilReference()` to be Objective-C specific in the 
> first place?
> - Should we do the same thing for C and NULL?
> - C, C++ and Objective-C(++) being highly intermingled, would something like 
> this even do the right thing in all cases?

Good questions, and I don't really have an answer for that. I don't think that 
`IsNilReference` should be c++-specific. Printing C pointers as NULL would be 
nice, but i don't know if we have any summaries for C types anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153



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

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Target/TargetProperties.td:183
 Desc<"A path to a python OS plug-in module file that contains a 
OperatingSystemPlugIn class.">;
+  def PluginReportsAllThreads: Property<"plugin-reports-all-threads", 
"Boolean">,
+Global,

jingham wrote:
> labath wrote:
> > jingham wrote:
> > > labath wrote:
> > > > jingham wrote:
> > > > > labath wrote:
> > > > > > If this is relevant only for os plugins, then it would be good to 
> > > > > > reflect that in the name as well.
> > > > > I thought about that.  In the future a regular Process plugin might 
> > > > > decide it was too expensive to report all threads as well.  There's 
> > > > > nothing in this patch that wouldn't "just work" with that case as 
> > > > > well.  Leaving the OS out was meant to indicate this was about how 
> > > > > the Process plugin OR any of its helpers (e.g. the OS Plugin) 
> > > > > produces threads.
> > > > Well, I am hoping that if we ever extend this support to the regular 
> > > > process plugins, we will implement it in a way where the plugin itself 
> > > > can tell us whether it is operating/supporting this mode (which I guess 
> > > > would involve a new gdb-remote packet and some specification of what 
> > > > exactly should happen when it gets sent), instead of relying on the 
> > > > user to set this correctly.
> > > > 
> > > > I mean, in an ideal world this is I would want to happen with the 
> > > > python plugins as well, but it seems that we are stuck with some 
> > > > existing plugins which already do that. However, I wouldn't want to 
> > > > plan on the same thing happening again. :)
> > > Right, I put this in mostly to help backwards compatibility.  For 
> > > instance, another OS Plugin I know about handles some older cooperative 
> > > threading scheme.  That one does report all threads on each stop.  I 
> > > didn't want to force them to do anything to keep their plugin working as 
> > > well as it did before.  That's also why I set the default to true here.
> > > 
> > > Even when we have plugins that actually support not reporting all 
> > > threads, you could imagine somebody having a Process plugin that supports 
> > > both modes - particularly early in the development of its support for 
> > > this feature, and in some corner cases the "doesn't report all threads" 
> > > mode has some subtle problem.  Having this setting will allow people to 
> > > get the slower but more assuredly correct behavior till it works 100% 
> > > reliably.  So I still think the setting has some value.
> > > 
> > > But I agree, going forward there should be some kind of handshake between 
> > > the ThreadPlanStackMap and the Process Plugin, either a "I've reported 
> > > all threads now" which could trigger a prune, or a "Is TID X still alive" 
> > > which the generic code could use to balance the cost of keeping outdated 
> > > stacks alive against when we want to ask about all threads.
> > All of this sounds fine, and I wouldn't mind having a setting like that, 
> > even after the support for partial thread lists is considered "stable". 
> > However, that sounds like a different setting to me -- that's like a 
> > directive to the process plugin about how it should behave, whereas this 
> > setting is more like a statement of fact about what the plugin does.
> > 
> > The setting could be later repurposed to do both, but it's not clear to me 
> > whether that is useful. Like, since we already support plugin-specific 
> > settings, the plugin which decides to implement both modes of operation 
> > could expose that as a custom setting. That way, one wouldn't get the 
> > impression that this setting applies to any process plugin...
> Okay.  I don't want to have to design that right now.  
> 
> Since this is just a workaround for the fact that the extant OS plugins don't 
> have a way to tell me this fact, I'll go back to using this just for OS 
> plugins.  In that case, I think I should move it to experimental.  Once we 
> add an API to the various plugins to advertise how they work, it won't be 
> needed.  The benefit of experimental settings is that their absence doesn't 
> cause the "settings set" to raise an error, so people can leave this in their 
> lldbinit's and it won't cause problems once we use it.
> 
> Now I just have to figure out how experimental settings work in the new 
> tablegen way of doing properties...
An experimental setting sounds good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76814



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

[Lldb-commits] [PATCH] D76471: Remap the target SDK directory to the host SDK directory

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Ok, I think we have converged onto something here. I just think it would be 
good to rename some functions/variables to make it clear they are working with 
an xcode sdk, and not some generic entity.

In D76471#1955908 , @aprantl wrote:

> Great! I think the current version of the patch reflects all of that, with 
> only one difference — something that I actually overlooked in the previous 
> version. We now have Platform that provides the interface to provide an SDK, 
> and will forward the question to Host. Each Host can define its own 
> mechanisms for doing so. So far I have only provided an implementation for 
> macOS, which runs `xcrun`. What I can't do is remove the SDK parameter from 
> the interface in platform, because Xcode may contain differently versioned 
> SDKs (e.g., a MacOSX10.14.sdk and a MacOSX10.15.sdk) plus some wrinkles that 
> are only relevant when you are developing the OS itself. That said, even 
> without removing the SDK parameter from GetSDKPath(), we can still implement 
> HostInfoLinux::GetXcodeSDK() for hypothetical Linux -> macOS cross-debugger 
> you mentioned above, so I don't think the extra parameter tarnishes the 
> design.


Yeah, it seems acceptable. The "sdk type" argument still seems somewhat 
redundant. Theoretically we could pass in just the triple component out the 
xcodesdk object, but that would just mean the individual platforms would have 
to reconstruct the XcodeSDK object, which is already present in the caller, so 
it's not ideal either...




Comment at: lldb/include/lldb/Core/Module.h:517
+  /// \param sysroot will be added to the path remapping dictionary.
+  void RegisterSDK(llvm::StringRef sdk, llvm::StringRef sysroot);
+

labath wrote:
> RegisterXCodeSDK?
ping



Comment at: lldb/include/lldb/Core/Module.h:983
+  /// The SDK this module was compiled with.
+  XcodeSDK m_sdk;
+  

labath wrote:
> m_xcode_sdk
ping?


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

https://reviews.llvm.org/D76471



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

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

We have some existing tests 
(`functionalities/load_using_paths/TestLoadUsingPaths.py`) for this 
functionality, but they are are all skipped on windows. Can you check if they 
work now, or create a new test for the subset of functionality which does work?

>   Ideally, the user input would be converted from UTF-8 to UTF-16.

Would it be possible to do the conversion within lldb 
(`llvm::ConvertUTF8toWide`), and pass that as a prebaked string?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77287



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

[Lldb-commits] [lldb] 62be834 - Recommit "[lldb] Fix TestSettings.test_pass_host_env_vars on windows"

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-04-02T11:52:56+02:00
New Revision: 62be83463a3713612bd85cfa45140ef92c130d57

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

LOG: Recommit "[lldb] Fix TestSettings.test_pass_host_env_vars on windows"

This patch was reverted because it introduced a failure in
TestHelloWorld.py. The reason for that was running "ls" shell command
failed as it was evaluated in an environment with an empty path. This
has now been fixed with D77123, which ensures that all shell commands
inherit the host environment, so this patch should be safe to recommit.

The original commit message was:

A defensive check in ProcessLauncherWindows meant that we would never
attempt to launch a process with a completely empty environment -- the
host environment would be used instead. Instead, I make the function add
an extra null wchar_t at the end of an empty environment. The
documentation on this is a bit fuzzy, but it seems to be what is needed
to make windows accept these kinds of environments.

Reviewers: amccarth, friss

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

Added: 


Modified: 
lldb/source/Host/windows/ProcessLauncherWindows.cpp
lldb/test/API/commands/settings/TestSettings.py

Removed: 




diff  --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp 
b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
index 31101944d75c..00470f558e94 100644
--- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -23,10 +23,9 @@ using namespace lldb_private;
 namespace {
 void CreateEnvironmentBuffer(const Environment &env,
  std::vector &buffer) {
-  if (env.size() == 0)
-return;
-
-  // Environment buffer is a null terminated list of null terminated strings
+  // The buffer is a list of null-terminated UTF-16 strings, followed by an
+  // extra L'\0' (two bytes of 0).  An empty environment must have one
+  // empty string, followed by an extra L'\0'.
   for (const auto &KV : env) {
 std::wstring warg;
 if (llvm::ConvertUTF8toWide(Environment::compose(KV), warg)) {
@@ -38,6 +37,9 @@ void CreateEnvironmentBuffer(const Environment &env,
   // One null wchar_t (to end the block) is two null bytes
   buffer.push_back(0);
   buffer.push_back(0);
+  // Insert extra two bytes, just in case the environment was empty.
+  buffer.push_back(0);
+  buffer.push_back(0);
 }
 
 bool GetFlattenedWindowsCommandString(Args args, std::string &command) {
@@ -94,8 +96,7 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo 
&launch_info,
 
   LPVOID env_block = nullptr;
   ::CreateEnvironmentBuffer(launch_info.GetEnvironment(), environment);
-  if (!environment.empty())
-env_block = environment.data();
+  env_block = environment.data();
 
   executable = launch_info.GetExecutableFile().GetPath();
   GetFlattenedWindowsCommandString(launch_info.GetArguments(), commandLine);

diff  --git a/lldb/test/API/commands/settings/TestSettings.py 
b/lldb/test/API/commands/settings/TestSettings.py
index c0cdc085f129..29360856a735 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -286,7 +286,6 @@ def do_test_run_args_and_env_vars(self, use_launchsimple):
 "Environment variable 'MY_ENV_VAR' successfully passed."])
 
 @skipIfRemote  # it doesn't make sense to send host env to remote target
-@skipIf(oslist=["windows"])
 def test_pass_host_env_vars(self):
 """Test that the host env vars are passed to the launched process."""
 self.build()



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

[Lldb-commits] [PATCH] D77045: Add invalidate list to primary regs in arm64 register infos

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid marked an inline comment as done.
omjavaid added a comment.

In D77045#1954690 , @labath wrote:

> This sounds like it could use a test case.


Adding a testcase would be tricky as these register overlap in memory and we 
store them with overlapping offsets with their children we should not need to 
invalidate the children when we write the parent but for some strange 
unexplainable reason QEMU was behaving strangely and not updating the first 
half in certain random cases. I just thought invalidation of children will 
force a read after write for that case.




Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h:593-594
 
+#define STRINGIZE2(x) #x
+#define STRINGIZE(x) STRINGIZE2(x)
+

labath wrote:
> What's up with the indirection?
We want to pass nullptr or register name as is then do stringize on regname 
only. According to C macro expansion rules if we want our macros expanded first 
and then do the # then we ll need double indirection.


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

https://reviews.llvm.org/D77045



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

[Lldb-commits] [PATCH] D77044: Extend max register size to accommodate AArch64 SVE vector regs

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid marked 2 inline comments as done.
omjavaid added a comment.

In D77044#1954685 , @labath wrote:

> Sounds fairly noncontroversial. I don't think we have too many of these 
> objects floating around, but if it turns out we do, we could switch to a 
> SmallVector to optimize for the common case of smaller registers.




  DataExtractor is used for caching register values on stop by gdb-remote 
register context. RegisterValue is mostly used for passing around register 
values temporarily during read/write.




Comment at: lldb/include/lldb/Utility/RegisterValue.h:264
  // register for any supported target.
-uint8_t length;
+uint32_t length;
 lldb::ByteOrder byte_order;

labath wrote:
> how about we stick to uint16_t here ?
ACK.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2051
   // Parse out the value.
-  uint8_t reg_bytes[32]; // big enough to support up to 256 bit ymmN register
+  uint8_t reg_bytes[256]; // big enough to support up to 256 byte AArch64 SVE
+  // registers

labath wrote:
> danielkiss wrote:
> > Could we use the kMaxRegisterByteSize here? 
> An excellent idea.
ACK.


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

https://reviews.llvm.org/D77044



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

[Lldb-commits] [PATCH] D77044: Extend max register size to accommodate AArch64 SVE vector regs

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 254479.
omjavaid added a comment.

Posting full diff.


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

https://reviews.llvm.org/D77044

Files:
  lldb/include/lldb/Utility/RegisterValue.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Utility/RegisterValue.cpp


Index: lldb/source/Utility/RegisterValue.cpp
===
--- lldb/source/Utility/RegisterValue.cpp
+++ lldb/source/Utility/RegisterValue.cpp
@@ -810,7 +810,7 @@
   if (buffer.length != rhs.buffer.length)
 return false;
   else {
-uint8_t length = buffer.length;
+uint16_t length = buffer.length;
 if (length > kMaxRegisterByteSize)
   length = kMaxRegisterByteSize;
 return memcmp(buffer.bytes, rhs.buffer.bytes, length) == 0;
Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2048,7 +2048,7 @@
 packet, "P packet missing '=' char after register number");
 
   // Parse out the value.
-  uint8_t reg_bytes[32]; // big enough to support up to 256 bit ymmN register
+  uint8_t reg_bytes[RegisterValue::kMaxRegisterByteSize];
   size_t reg_size = packet.GetHexBytesAvail(reg_bytes);
 
   // Get the thread to use.
Index: lldb/include/lldb/Utility/RegisterValue.h
===
--- lldb/include/lldb/Utility/RegisterValue.h
+++ lldb/include/lldb/Utility/RegisterValue.h
@@ -26,7 +26,8 @@
 
 class RegisterValue {
 public:
-  enum { kMaxRegisterByteSize = 64u };
+  // big enough to support up to 256 byte AArch64 SVE
+  enum { kMaxRegisterByteSize = 256u };
 
   enum Type {
 eTypeInvalid,
@@ -261,7 +262,7 @@
   struct {
 uint8_t bytes[kMaxRegisterByteSize]; // This must be big enough to hold any
  // register for any supported target.
-uint8_t length;
+uint16_t length;
 lldb::ByteOrder byte_order;
   } buffer;
 };


Index: lldb/source/Utility/RegisterValue.cpp
===
--- lldb/source/Utility/RegisterValue.cpp
+++ lldb/source/Utility/RegisterValue.cpp
@@ -810,7 +810,7 @@
   if (buffer.length != rhs.buffer.length)
 return false;
   else {
-uint8_t length = buffer.length;
+uint16_t length = buffer.length;
 if (length > kMaxRegisterByteSize)
   length = kMaxRegisterByteSize;
 return memcmp(buffer.bytes, rhs.buffer.bytes, length) == 0;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2048,7 +2048,7 @@
 packet, "P packet missing '=' char after register number");
 
   // Parse out the value.
-  uint8_t reg_bytes[32]; // big enough to support up to 256 bit ymmN register
+  uint8_t reg_bytes[RegisterValue::kMaxRegisterByteSize];
   size_t reg_size = packet.GetHexBytesAvail(reg_bytes);
 
   // Get the thread to use.
Index: lldb/include/lldb/Utility/RegisterValue.h
===
--- lldb/include/lldb/Utility/RegisterValue.h
+++ lldb/include/lldb/Utility/RegisterValue.h
@@ -26,7 +26,8 @@
 
 class RegisterValue {
 public:
-  enum { kMaxRegisterByteSize = 64u };
+  // big enough to support up to 256 byte AArch64 SVE
+  enum { kMaxRegisterByteSize = 256u };
 
   enum Type {
 eTypeInvalid,
@@ -261,7 +262,7 @@
   struct {
 uint8_t bytes[kMaxRegisterByteSize]; // This must be big enough to hold any
  // register for any supported target.
-uint8_t length;
+uint16_t length;
 lldb::ByteOrder byte_order;
   } buffer;
 };
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77295: [lldb/Core] Fix a race in the Communication class

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, JDevlieghere.
Herald added a subscriber: mgorny.
Herald added a project: LLDB.

Communication::SynchronizeWithReadThread is called whenever a process
stops to ensure that we process all of its stdout before we report the
stop. If the process exits, we first call this method, and then close
the connection.

However, when the child process exits, the thread reading its stdout
will usually (but not always) read an EOF because the other end of the
pty has been closed. In response to an EOF, the Communication read
thread closes it's end of the connection too.

This can result in a race where the read thread is closing the
connection while the synchronizing thread is attempting to get its
attention via Connection::InterruptRead.

The fix is to hold the synchronization mutex while closing the
connection.

I've found this issue while tracking down a rare flake in some of the
vscode tests. I am not sure this is the cause of those failures (as I
would have expected this issue to manifest itself differently), but it
is an issue nonetheless.

The attached test demonstrates the steps needed to reproduce the race.
It will fail under tsan without this patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77295

Files:
  lldb/source/Core/Communication.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/CommunicationTest.cpp


Index: lldb/unittests/Core/CommunicationTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/CommunicationTest.cpp
@@ -0,0 +1,36 @@
+//===-- CommunicationTest.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/Communication.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
+#include "lldb/Host/Pipe.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(CommunicationTest, SynchronizeWhileClosing) {
+  // Set up a communication object reading from a pipe.
+  Pipe pipe;
+  ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(),
+llvm::Succeeded());
+  auto conn_up = std::make_unique(
+  pipe.ReleaseReadFileDescriptor(), /*owns_fd=*/true);
+
+  Communication comm("test");
+  comm.SetConnection(conn_up.release());
+  comm.SetCloseOnEOF(true);
+  ASSERT_TRUE(comm.StartReadThread());
+
+  // Ensure that we can safely synchronize with the read thread while it is
+  // closing the read end (in response to us closing the write end).
+  pipe.CloseWriteFileDescriptor();
+  comm.SynchronizeWithReadThread();
+
+  ASSERT_TRUE(comm.StopReadThread());
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(LLDBCoreTests
+  CommunicationTest.cpp
   MangledTest.cpp
   RichManglingContextTest.cpp
   StreamCallbackTest.cpp
Index: lldb/source/Core/Communication.cpp
===
--- lldb/source/Core/Communication.cpp
+++ lldb/source/Core/Communication.cpp
@@ -315,14 +315,14 @@
   Status error;
   ConnectionStatus status = eConnectionStatusSuccess;
   bool done = false;
+  bool disconnect = false;
   while (!done && comm->m_read_thread_enabled) {
 size_t bytes_read = comm->ReadFromConnection(
 buf, sizeof(buf), std::chrono::seconds(5), status, &error);
 if (bytes_read > 0)
   comm->AppendBytesToCache(buf, bytes_read, true, status);
 else if ((bytes_read == 0) && status == eConnectionStatusEndOfFile) {
-  if (comm->GetCloseOnEOF())
-comm->Disconnect();
+  disconnect = comm->GetCloseOnEOF();
   comm->AppendBytesToCache(buf, bytes_read, true, status);
 }
 
@@ -336,7 +336,7 @@
 case eConnectionStatusError: // Check GetError() for details
   if (error.GetType() == eErrorTypePOSIX && error.GetError() == EIO) {
 // EIO on a pipe is usually caused by remote shutdown
-comm->Disconnect();
+disconnect = comm->GetCloseOnEOF();
 done = true;
   }
   if (error.Fail())
@@ -365,9 +365,15 @@
   if (log)
 LLDB_LOGF(log, "%p Communication::ReadThread () thread exiting...", p);
 
-  comm->m_read_thread_did_exit = true;
-  // Let clients know that this thread is exiting
   comm->BroadcastEvent(eBroadcastBitNoMorePendingInput);
+  {
+std::lock_guard guard(comm->m_synchronize_mutex);
+comm->m_read_thread_did_exit = true;
+if (disconnect)
+  comm->Disconnect();
+  }
+
+  // Let clients know that this thread is exiting
   comm->BroadcastEvent

[Lldb-commits] [lldb] 451741a - [lldb] Change Communication::SetConnection to take a unique_ptr

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-04-02T14:42:25+02:00
New Revision: 451741a9d778a260ceee608a26b5fdf2d9926982

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

LOG: [lldb] Change Communication::SetConnection to take a unique_ptr

The function takes ownership of the object. This makes that explicit,
and avoids unowned pointers floating around.

Added: 


Modified: 
lldb/include/lldb/Core/Communication.h
lldb/source/API/SBCommunication.cpp
lldb/source/Core/Communication.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Target/Process.cpp
lldb/tools/lldb-server/lldb-platform.cpp
lldb/unittests/Process/gdb-remote/GDBRemoteTestUtils.h
lldb/unittests/tools/lldb-server/tests/TestClient.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Communication.h 
b/lldb/include/lldb/Core/Communication.h
index 048541aa4c7c..4233d5bf5f81 100644
--- a/lldb/include/lldb/Core/Communication.h
+++ b/lldb/include/lldb/Core/Communication.h
@@ -221,7 +221,7 @@ class Communication : public Broadcaster {
   ///
   /// \see
   /// class Connection
-  void SetConnection(Connection *connection);
+  void SetConnection(std::unique_ptr connection);
 
   /// Starts a read thread whose sole purpose it to read bytes from the
   /// current connection. This function will call connection's read function:

diff  --git a/lldb/source/API/SBCommunication.cpp 
b/lldb/source/API/SBCommunication.cpp
index 7c7ba39257d9..d55ecd35b557 100644
--- a/lldb/source/API/SBCommunication.cpp
+++ b/lldb/source/API/SBCommunication.cpp
@@ -63,7 +63,7 @@ ConnectionStatus SBCommunication::Connect(const char *url) {
 
   if (m_opaque) {
 if (!m_opaque->HasConnection())
-  m_opaque->SetConnection(Host::CreateDefaultConnection(url).release());
+  m_opaque->SetConnection(Host::CreateDefaultConnection(url));
 return m_opaque->Connect(url, nullptr);
   }
   return eConnectionStatusNoConnection;
@@ -79,7 +79,8 @@ ConnectionStatus SBCommunication::AdoptFileDesriptor(int fd, 
bool owns_fd) {
   if (m_opaque->IsConnected())
 m_opaque->Disconnect();
 }
-m_opaque->SetConnection(new ConnectionFileDescriptor(fd, owns_fd));
+m_opaque->SetConnection(
+std::make_unique(fd, owns_fd));
 if (m_opaque->IsConnected())
   status = eConnectionStatusSuccess;
 else

diff  --git a/lldb/source/Core/Communication.cpp 
b/lldb/source/Core/Communication.cpp
index 88f6a21ea4d5..f4163847e4bb 100644
--- a/lldb/source/Core/Communication.cpp
+++ b/lldb/source/Core/Communication.cpp
@@ -399,10 +399,10 @@ void Communication::SynchronizeWithReadThread() {
   listener_sp->GetEvent(event_sp, llvm::None);
 }
 
-void Communication::SetConnection(Connection *connection) {
+void Communication::SetConnection(std::unique_ptr connection) {
   Disconnect(nullptr);
   StopReadThread(nullptr);
-  m_connection_sp.reset(connection);
+  m_connection_sp = std::move(connection);
 }
 
 const char *

diff  --git 
a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp 
b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
index b0467555665c..657b8fdc729a 100644
--- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -290,7 +290,7 @@ Status PlatformRemoteGDBServer::ConnectRemote(Args &args) {
GetHostname());
   } else {
 if (args.GetArgumentCount() == 1) {
-  m_gdb_client.SetConnection(new ConnectionFileDescriptor());
+  m_gdb_client.SetConnection(std::make_unique());
   // we're going to reuse the hostname when we connect to the debugserver
   int port;
   std::string path;

diff  --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp 
b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
index 09d1965f25ed..5b728a5f2960 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -250,7 +250,7 @@ Status ProcessKDP::DoConnectRemote(Stream *strm, 
llvm::StringRef remote_url) {
 const uint16_t reply_port = socket.GetLocalPortNumber();
 
 if (reply_port != 0) {
-  m_comm.SetConnection(conn_up.release());
+  m_comm.SetConnection(std::move(conn_up));
 
   if (m_comm.

[Lldb-commits] [PATCH] D76729: Convert for loops to entry-based iteration

[Shivam Mittal via Phabricator via lldb-commits]

shivammittal99 added a comment.

Can you please commit this for me? I do not have commit access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76729



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

[Lldb-commits] [lldb] 55e32e9 - [lldb] update eArgTypeScriptLang description to mention lua

[Ed Maste via lldb-commits]

Author: Ed Maste
Date: 2020-04-02T09:43:01-04:00
New Revision: 55e32e92cda79a9eb4487ea8e4ec1b968fac5145

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

LOG: [lldb] update eArgTypeScriptLang description to mention lua

--script-language python and --script-language lua are both valid now.

Reviewed by:JDevlieghere

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

Added: 


Modified: 
lldb/source/Interpreter/CommandObject.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/CommandObject.cpp 
b/lldb/source/Interpreter/CommandObject.cpp
index 01fa86750b12..f1f17dbd66ef 100644
--- a/lldb/source/Interpreter/CommandObject.cpp
+++ b/lldb/source/Interpreter/CommandObject.cpp
@@ -1091,7 +1091,7 @@ CommandObject::ArgumentTableEntry 
CommandObject::g_arguments_data[] = {
 { eArgTypeRunArgs, "run-args", CommandCompletions::eNoCompletion, { 
nullptr, false }, "Arguments to be passed to the target program when it starts 
executing." },
 { eArgTypeRunMode, "run-mode", CommandCompletions::eNoCompletion, { 
nullptr, false }, "Help text goes here." },
 { eArgTypeScriptedCommandSynchronicity, "script-cmd-synchronicity", 
CommandCompletions::eNoCompletion, { nullptr, false }, "The synchronicity to 
use to run scripted commands with regard to LLDB event system." },
-{ eArgTypeScriptLang, "script-language", 
CommandCompletions::eNoCompletion, { nullptr, false }, "The scripting language 
to be used for script-based commands.  Currently only Python is valid." },
+{ eArgTypeScriptLang, "script-language", 
CommandCompletions::eNoCompletion, { nullptr, false }, "The scripting language 
to be used for script-based commands.  Supported languages are python and lua." 
},
 { eArgTypeSearchWord, "search-word", CommandCompletions::eNoCompletion, { 
nullptr, false }, "Any word of interest for search purposes." },
 { eArgTypeSelector, "selector", CommandCompletions::eNoCompletion, { 
nullptr, false }, "An Objective-C selector name." },
 { eArgTypeSettingIndex, "setting-index", 
CommandCompletions::eNoCompletion, { nullptr, false }, "An index into a 
settings variable that is an array (try 'settings list' to see all the possible 
settings variables and their types)." },



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

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin added a comment.

Pavel, could you land this for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74398



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

[Lldb-commits] [PATCH] D77241: update eArgTypeScriptLang description to mention lua

[Ed Maste via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG55e32e92cda7: [lldb] update eArgTypeScriptLang description 
to mention lua (authored by emaste).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77241

Files:
  lldb/source/Interpreter/CommandObject.cpp


Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -1091,7 +1091,7 @@
 { eArgTypeRunArgs, "run-args", CommandCompletions::eNoCompletion, { 
nullptr, false }, "Arguments to be passed to the target program when it starts 
executing." },
 { eArgTypeRunMode, "run-mode", CommandCompletions::eNoCompletion, { 
nullptr, false }, "Help text goes here." },
 { eArgTypeScriptedCommandSynchronicity, "script-cmd-synchronicity", 
CommandCompletions::eNoCompletion, { nullptr, false }, "The synchronicity to 
use to run scripted commands with regard to LLDB event system." },
-{ eArgTypeScriptLang, "script-language", 
CommandCompletions::eNoCompletion, { nullptr, false }, "The scripting language 
to be used for script-based commands.  Currently only Python is valid." },
+{ eArgTypeScriptLang, "script-language", 
CommandCompletions::eNoCompletion, { nullptr, false }, "The scripting language 
to be used for script-based commands.  Supported languages are python and lua." 
},
 { eArgTypeSearchWord, "search-word", CommandCompletions::eNoCompletion, { 
nullptr, false }, "Any word of interest for search purposes." },
 { eArgTypeSelector, "selector", CommandCompletions::eNoCompletion, { 
nullptr, false }, "An Objective-C selector name." },
 { eArgTypeSettingIndex, "setting-index", 
CommandCompletions::eNoCompletion, { nullptr, false }, "An index into a 
settings variable that is an array (try 'settings list' to see all the possible 
settings variables and their types)." },


Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -1091,7 +1091,7 @@
 { eArgTypeRunArgs, "run-args", CommandCompletions::eNoCompletion, { nullptr, false }, "Arguments to be passed to the target program when it starts executing." },
 { eArgTypeRunMode, "run-mode", CommandCompletions::eNoCompletion, { nullptr, false }, "Help text goes here." },
 { eArgTypeScriptedCommandSynchronicity, "script-cmd-synchronicity", CommandCompletions::eNoCompletion, { nullptr, false }, "The synchronicity to use to run scripted commands with regard to LLDB event system." },
-{ eArgTypeScriptLang, "script-language", CommandCompletions::eNoCompletion, { nullptr, false }, "The scripting language to be used for script-based commands.  Currently only Python is valid." },
+{ eArgTypeScriptLang, "script-language", CommandCompletions::eNoCompletion, { nullptr, false }, "The scripting language to be used for script-based commands.  Supported languages are python and lua." },
 { eArgTypeSearchWord, "search-word", CommandCompletions::eNoCompletion, { nullptr, false }, "Any word of interest for search purposes." },
 { eArgTypeSelector, "selector", CommandCompletions::eNoCompletion, { nullptr, false }, "An Objective-C selector name." },
 { eArgTypeSettingIndex, "setting-index", CommandCompletions::eNoCompletion, { nullptr, false }, "An index into a settings variable that is an array (try 'settings list' to see all the possible settings variables and their types)." },
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 183fba6 - Add OMPIterator case in switch statement to silence warnings

[via lldb-commits]

Author: scentini
Date: 2020-04-02T16:16:11+02:00
New Revision: 183fba635d5ece8466af3085c2e831f9a93ee775

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

LOG: Add OMPIterator case in switch statement to silence warnings

Added: 


Modified: 
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 22f401fa4e95..9f425ae7c30b 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4666,6 +4666,7 @@ lldb::Encoding 
TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
 case clang::BuiltinType::Kind::OCLSampler:
 case clang::BuiltinType::Kind::OMPArraySection:
 case clang::BuiltinType::Kind::OMPArrayShaping:
+case clang::BuiltinType::Kind::OMPIterator:
 case clang::BuiltinType::Kind::Overload:
 case clang::BuiltinType::Kind::PseudoObject:
 case clang::BuiltinType::Kind::UnknownAny:



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

[Lldb-commits] [PATCH] D77302: [DebugInfo] Rename getOffset() to getContribution(). NFC.

[James Henderson via Phabricator via lldb-commits]

jhenderson accepted this revision.
jhenderson 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/D77302/new/

https://reviews.llvm.org/D77302



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

[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

[David Blaikie via Phabricator via lldb-commits]

dblaikie accepted this revision.
dblaikie added a comment.

In D75929#1954443 , @labath wrote:

> Thanks for doing this. This looks fine to me. @dblaikie, @jhenderson, do you 
> have any additional comments?


nah, nothing dire that comes to mind - thanks for checking


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

https://reviews.llvm.org/D75929



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

[Lldb-commits] [PATCH] D77302: [DebugInfo] Rename getOffset() to getContribution(). NFC.

[Igor Kudrin via Phabricator via lldb-commits]

ikudrin created this revision.
ikudrin added reviewers: jhenderson, dblaikie, probinson, aprantl.
ikudrin added projects: LLVM, debug-info.
Herald added subscribers: lldb-commits, arphaman, hiraditya.
Herald added a project: LLDB.

The old name was a bit misleading because the functions actually return 
contributions to the corresponding sections. See D77146 
 for the discussion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77302

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
  llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp

Index: llvm/tools/llvm-dwp/llvm-dwp.cpp
===
--- llvm/tools/llvm-dwp/llvm-dwp.cpp
+++ llvm/tools/llvm-dwp/llvm-dwp.cpp
@@ -219,7 +219,7 @@
 static StringRef getSubsection(StringRef Section,
const DWARFUnitIndex::Entry &Entry,
DWARFSectionKind Kind) {
-  const auto *Off = Entry.getOffset(Kind);
+  const auto *Off = Entry.getContribution(Kind);
   if (!Off)
 return StringRef();
   return Section.substr(Off->Offset, Off->Length);
@@ -231,7 +231,7 @@
 const UnitIndexEntry &TUEntry, uint32_t &TypesOffset) {
   Out.SwitchSection(OutputTypes);
   for (const DWARFUnitIndex::Entry &E : TUIndex.getRows()) {
-auto *I = E.getOffsets();
+auto *I = E.getContributions();
 if (!I)
   continue;
 auto P = TypeIndexEntries.insert(std::make_pair(E.getSignature(), TUEntry));
@@ -599,7 +599,7 @@
   return make_error("failed to parse cu_index");
 
 for (const DWARFUnitIndex::Entry &E : CUIndex.getRows()) {
-  auto *I = E.getOffsets();
+  auto *I = E.getContributions();
   if (!I)
 continue;
   auto P = IndexEntries.insert(std::make_pair(E.getSignature(), CurEntry));
Index: llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
===
--- llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
+++ llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
@@ -154,7 +154,7 @@
 }
 
 const DWARFUnitIndex::Entry::SectionContribution *
-DWARFUnitIndex::Entry::getOffset(DWARFSectionKind Sec) const {
+DWARFUnitIndex::Entry::getContribution(DWARFSectionKind Sec) const {
   uint32_t i = 0;
   for (; i != Index->Header.NumColumns; ++i)
 if (Index->ColumnKinds[i] == Sec)
@@ -163,7 +163,7 @@
 }
 
 const DWARFUnitIndex::Entry::SectionContribution *
-DWARFUnitIndex::Entry::getOffset() const {
+DWARFUnitIndex::Entry::getContribution() const {
   return &Contributions[Index->InfoColumn];
 }
 
Index: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
===
--- llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
+++ llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
@@ -139,7 +139,7 @@
 
 DWARFUnit *
 DWARFUnitVector::getUnitForIndexEntry(const DWARFUnitIndex::Entry &E) {
-  const auto *CUOff = E.getOffset(DW_SECT_INFO);
+  const auto *CUOff = E.getContribution(DW_SECT_INFO);
   if (!CUOff)
 return nullptr;
 
@@ -183,7 +183,7 @@
 // data based on the index entries.
 StringRef Data = LocSection->Data;
 if (auto *IndexEntry = Header.getIndexEntry())
-  if (const auto *C = IndexEntry->getOffset(DW_SECT_LOC))
+  if (const auto *C = IndexEntry->getContribution(DW_SECT_LOC))
 Data = Data.substr(C->Offset, C->Length);
 
 DWARFDataExtractor DWARFData =
@@ -294,11 +294,11 @@
   if (IndexEntry) {
 if (AbbrOffset)
   return false;
-auto *UnitContrib = IndexEntry->getOffset();
+auto *UnitContrib = IndexEntry->getContribution();
 if (!UnitContrib ||
 UnitContrib->Length != (Length + getUnitLengthFieldByteSize()))
   return false;
-auto *AbbrEntry = IndexEntry->getOffset(DW_SECT_ABBREV);
+auto *AbbrEntry = IndexEntry->getContribution(DW_SECT_ABBREV);
 if (!AbbrEntry)
   return false;
 AbbrOffset = AbbrEntry->Offset;
@@ -966,7 +966,7 @@
   uint64_t Offset = 0;
   auto IndexEntry = Header.getIndexEntry();
   const auto *C =
-  IndexEntry ? IndexEntry->getOffset(DW_SECT_STR_OFFSETS) : nullptr;
+  IndexEntry ? IndexEntry->getContribution(DW_SECT_STR_OFFSETS) : nullptr;
   if (C)
 Offset = C->Offset;
   if (getVersion() >= 5) {
Index: llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
===
--- llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
+++ llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
@@ -56,10 +56,10 @@
 friend class DWARFUnitIndex;
 
   public:
-const SectionContribution *getOffset(DWARFSectionKind Sec) const;
-const SectionContribution *getOffset() const;
+const SectionContribution *getContribution(DWARFSectionKind Sec) 

[Lldb-commits] [PATCH] D77108: [lldb/DWARF] Fix evaluator crash when accessing empty stack

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

In D77108#1952039 , @labath wrote:

> In D77108#1951997 , @kwk wrote:
>
> > In D77108#1951879 , @labath wrote:
> >
> > > Most DW_OP cases check their stack, but it's quite possible that others 
> > > were missed too. It might be a nice cleanup to make a function like 
> > > (`getMinimalStackSize(op)`) and move this check up in front of the big 
> > > switch. That could not handle all operators, as for some of them, the 
> > > value is not statically known, but it would handle the vast majority of 
> > > them.
> >
> >
> > @labath I somewhat like that the logic for each `op` is close to the `case` 
> > statement. Creating the function that you suggested would separate this 
> > check out somewhere else and you would have to look at two places. At least 
> > for code review, the current solution is much clearer to me.
>
>
> I don't have a problem with the current patch (modulo the test tweak) -- it 
> fixes a real problem and it follows the style of the surrounding code. 
> However, DWARFExpression::Evaluate is gigantic (2600LOC), and nearly half of 
> that is error handling. Tastes may vary, but I think that's a bigger 
> readability problem than having to look at two places to understand an opcode.


@labath okay, that makes sense. Sorry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77108



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

[Lldb-commits] [PATCH] D76955: [lldb/Test] Decode stdout and stderr in case it contains UTF-8

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere abandoned this revision.
JDevlieghere added a comment.

After looking at `util.py` I'm convinced that this should never happen, unless 
there's a bug in the `to_string` implementation in which case we'd need to fix 
it there.


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

https://reviews.llvm.org/D76955



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

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added a comment.

Thanks for the hint about the string conversion, however, I think that it's 
going to complicate things as the string is going to be a mixture of UTF-8 and 
UTF-16 content.

As to the testing, `functionalities/load_using_paths/TestLoadUsingPaths.py` is 
not applicable to Windows.  In fact, I don't really see a good way to really 
test much of this outside the context of the swift REPL which forces the 
loading of a DLL which is in fact how I discovered this.  If there is an easy 
way to ensure that the dll that is needed is in the user's `PATH`, then I 
suppose creating an empty dll and loading that is theoretically possible, but 
that too can have a lot of flakiness due to dependencies to build and all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77287



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

[Lldb-commits] [lldb] 51b3874 - Convert for loops to entry-based iteration

[Raphael Isemann via lldb-commits]

Author: Shivam Mittal
Date: 2020-04-02T18:56:29+02:00
New Revision: 51b38746295fc18664f766fe6cb7974b862c28eb

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

LOG: Convert for loops to entry-based iteration

Summary: Convert index-based loops marked TODO in CommandObjectSettings and 
CommandObjectTarget to entry-based.

Reviewers: labath

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectSettings.cpp
lldb/source/Commands/CommandObjectTarget.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectSettings.cpp 
b/lldb/source/Commands/CommandObjectSettings.cpp
index cc1080c8cc0c..87e0352636e1 100644
--- a/lldb/source/Commands/CommandObjectSettings.cpp
+++ b/lldb/source/Commands/CommandObjectSettings.cpp
@@ -531,10 +531,8 @@ class CommandObjectSettingsList : public 
CommandObjectParsed {
 if (argc > 0) {
   const bool dump_qualified_name = true;
 
-  // TODO: Convert to StringRef based enumeration.  Requires converting
-  // GetPropertyAtPath first.
-  for (size_t i = 0; i < argc; ++i) {
-const char *property_path = args.GetArgumentAtIndex(i);
+  for (const Args::ArgEntry &arg : args) {
+const char *property_path = arg.c_str();
 
 const Property *property =
 GetDebugger().GetValueProperties()->GetPropertyAtPath(

diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index fb2854d90ba8..decdd9c53de2 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -876,21 +876,18 @@ class CommandObjectTargetVariable : public 
CommandObjectParsed {
 Stream &s = result.GetOutputStream();
 
 if (argc > 0) {
-
-  // TODO: Convert to entry-based iteration.  Requires converting
-  // DumpValueObject.
-  for (size_t idx = 0; idx < argc; ++idx) {
+  for (const Args::ArgEntry &arg : args) {
 VariableList variable_list;
 ValueObjectList valobj_list;
 
-const char *arg = args.GetArgumentAtIndex(idx);
 size_t matches = 0;
 bool use_var_name = false;
 if (m_option_variable.use_regex) {
-  RegularExpression regex(llvm::StringRef::withNullAsEmpty(arg));
+  RegularExpression regex(
+  llvm::StringRef::withNullAsEmpty(arg.c_str()));
   if (!regex.IsValid()) {
 result.GetErrorStream().Printf(
-"error: invalid regular expression: '%s'\n", arg);
+"error: invalid regular expression: '%s'\n", arg.c_str());
 result.SetStatus(eReturnStatusFailed);
 return false;
   }
@@ -900,14 +897,14 @@ class CommandObjectTargetVariable : public 
CommandObjectParsed {
   matches = variable_list.GetSize();
 } else {
   Status error(Variable::GetValuesForVariableExpressionPath(
-  arg, m_exe_ctx.GetBestExecutionContextScope(),
+  arg.c_str(), m_exe_ctx.GetBestExecutionContextScope(),
   GetVariableCallback, target, variable_list, valobj_list));
   matches = variable_list.GetSize();
 }
 
 if (matches == 0) {
   result.GetErrorStream().Printf(
-  "error: can't find global variable '%s'\n", arg);
+  "error: can't find global variable '%s'\n", arg.c_str());
   result.SetStatus(eReturnStatusFailed);
   return false;
 } else {
@@ -923,7 +920,7 @@ class CommandObjectTargetVariable : public 
CommandObjectParsed {
   if (valobj_sp)
 DumpValueObject(s, var_sp, valobj_sp,
 use_var_name ? var_sp->GetName().GetCString()
- : arg);
+ : arg.c_str());
 }
   }
 }
@@ -3058,17 +3055,14 @@ class CommandObjectTargetModulesList : public 
CommandObjectParsed {
   module_list_ptr = &target->GetImages();
 }
   } else {
-// TODO: Convert to entry based iteration.  Requires converting
-// FindModulesByName.
-for (size_t i = 0; i < argc; ++i) {
+for (const Args::ArgEntry &arg : command) {
   // Dump specified images (by basename or fullpath)
-  const char *arg_cstr = command.GetArgumentAtIndex(i);
   const size_t num_matches = FindModulesByName(
-  target, arg_cstr, module_list, use_global_module_list);
+  target, arg.c_str(), module_list, use_global_module_list);
   if (num_matches == 0) {
 if (argc == 1) {
   result.AppendErrorWithFormat("

[Lldb-commits] [PATCH] D76729: Convert for loops to entry-based iteration

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG51b38746295f: Convert for loops to entry-based iteration 
(authored by shivammittal99, committed by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76729

Files:
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectTarget.cpp


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -876,21 +876,18 @@
 Stream &s = result.GetOutputStream();
 
 if (argc > 0) {
-
-  // TODO: Convert to entry-based iteration.  Requires converting
-  // DumpValueObject.
-  for (size_t idx = 0; idx < argc; ++idx) {
+  for (const Args::ArgEntry &arg : args) {
 VariableList variable_list;
 ValueObjectList valobj_list;
 
-const char *arg = args.GetArgumentAtIndex(idx);
 size_t matches = 0;
 bool use_var_name = false;
 if (m_option_variable.use_regex) {
-  RegularExpression regex(llvm::StringRef::withNullAsEmpty(arg));
+  RegularExpression regex(
+  llvm::StringRef::withNullAsEmpty(arg.c_str()));
   if (!regex.IsValid()) {
 result.GetErrorStream().Printf(
-"error: invalid regular expression: '%s'\n", arg);
+"error: invalid regular expression: '%s'\n", arg.c_str());
 result.SetStatus(eReturnStatusFailed);
 return false;
   }
@@ -900,14 +897,14 @@
   matches = variable_list.GetSize();
 } else {
   Status error(Variable::GetValuesForVariableExpressionPath(
-  arg, m_exe_ctx.GetBestExecutionContextScope(),
+  arg.c_str(), m_exe_ctx.GetBestExecutionContextScope(),
   GetVariableCallback, target, variable_list, valobj_list));
   matches = variable_list.GetSize();
 }
 
 if (matches == 0) {
   result.GetErrorStream().Printf(
-  "error: can't find global variable '%s'\n", arg);
+  "error: can't find global variable '%s'\n", arg.c_str());
   result.SetStatus(eReturnStatusFailed);
   return false;
 } else {
@@ -923,7 +920,7 @@
   if (valobj_sp)
 DumpValueObject(s, var_sp, valobj_sp,
 use_var_name ? var_sp->GetName().GetCString()
- : arg);
+ : arg.c_str());
 }
   }
 }
@@ -3058,17 +3055,14 @@
   module_list_ptr = &target->GetImages();
 }
   } else {
-// TODO: Convert to entry based iteration.  Requires converting
-// FindModulesByName.
-for (size_t i = 0; i < argc; ++i) {
+for (const Args::ArgEntry &arg : command) {
   // Dump specified images (by basename or fullpath)
-  const char *arg_cstr = command.GetArgumentAtIndex(i);
   const size_t num_matches = FindModulesByName(
-  target, arg_cstr, module_list, use_global_module_list);
+  target, arg.c_str(), module_list, use_global_module_list);
   if (num_matches == 0) {
 if (argc == 1) {
   result.AppendErrorWithFormat("no modules found that match '%s'",
-   arg_cstr);
+   arg.c_str());
   result.SetStatus(eReturnStatusFailed);
   return false;
 }
Index: lldb/source/Commands/CommandObjectSettings.cpp
===
--- lldb/source/Commands/CommandObjectSettings.cpp
+++ lldb/source/Commands/CommandObjectSettings.cpp
@@ -531,10 +531,8 @@
 if (argc > 0) {
   const bool dump_qualified_name = true;
 
-  // TODO: Convert to StringRef based enumeration.  Requires converting
-  // GetPropertyAtPath first.
-  for (size_t i = 0; i < argc; ++i) {
-const char *property_path = args.GetArgumentAtIndex(i);
+  for (const Args::ArgEntry &arg : args) {
+const char *property_path = arg.c_str();
 
 const Property *property =
 GetDebugger().GetValueProperties()->GetPropertyAtPath(


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -876,21 +876,18 @@
 Stream &s = result.GetOutputStream();
 
 if (argc > 0) {
-
-  // TODO: Convert to entry-based iteration.  Requires converting
-  // DumpValueObject.
-  for (size_t idx = 0; idx < argc; ++idx) {
+  for (const Args::ArgEntry &arg : args) {
 VariableLi

[Lldb-commits] [lldb] b78157c - [intel-pt] Implement a basic test case

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-04-02T11:36:05-07:00
New Revision: b78157c88b327f4a683f2cdfe3d4f331492576a5

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

LOG: [intel-pt] Implement a basic test case

* This is a reattempted commit due to a previous builtbot failure

- Now using a env var to determine whether to run the test, as
someone might have built liblldbIntelFeatures.so without intelPT
support, which would make this test fail.

Summary:
Depends on D76872.

There was no test for the Intel PT support on LLDB, so I'm creating one, which
will help making progress on solid grounds.

The test is skipped if the Intel PT plugin library is not built.

Reviewers: clayborg, labath, kusmour, aadsm

Subscribers: lldb-commits

Tags: #lldb

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

Added: 
lldb/test/API/tools/intel-features/intel-pt/test/Makefile
lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
lldb/test/API/tools/intel-features/intel-pt/test/main.cpp

Modified: 
lldb/tools/intel-features/intel-pt/cli-wrapper-pt.cpp

Removed: 




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

diff  --git 
a/lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py 
b/lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
new file mode 100644
index ..ea7f6a469f8f
--- /dev/null
+++ 
b/lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -0,0 +1,60 @@
+from __future__ import print_function
+
+import os
+import lldb
+import time
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestIntelPTSimpleBinary(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIf(oslist=no_match(['linux']))
+@skipIf(archs=no_match(['i386', 'x86_64']))
+@skipIfRemote
+def test_basic_flow(self):
+"""Test collection, decoding, and dumping instructions"""
+if os.environ.get('TEST_INTEL_PT') != '1':
+self.skipTest("The environment variable TEST_INTEL_PT=1 is needed 
to run this test.")
+
+lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
+lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
+plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
+
+self.build()
+
+self.runCmd("plugin load " + plugin_file)
+
+exe = self.getBuildArtifact("a.out")
+lldbutil.run_to_name_breakpoint(self, "main", exe_name=exe)
+# We start tracing from main
+self.runCmd("processor-trace start all")
+
+# We check the trace after the for loop
+self.runCmd("b " + str(line_number('main.cpp', '// Break 1')))
+self.runCmd("c")
+
+# We wait a little bit to ensure the processor has send the PT packets 
to
+# the memory
+time.sleep(.1)
+
+# We find the start address of the 'fun' function for a later check
+target = self.dbg.GetSelectedTarget()
+fun_start_adddress = target.FindFunctions("fun")[0].GetSymbol() \
+.GetStartAddress().GetLoadAddress(target)
+
+# We print the last instructions
+self.expect("processor-trace show-instr-log -c 100",
+patterns=[
+# We expect to have seen the first instruction of 'fun'
+hex(fun_start_adddress),  
+# We expect to see the exit condition of the for loop
+"at main.cpp:" + str(line_number('main.cpp', '// Break for 
loop')) 
+])
+
+self.runCmd("processor-trace stop")

diff  --git a/lldb/test/API/tools/intel-features/intel-pt/test/main.cpp 
b/lldb/test/API/tools/intel-features/intel-pt/test/main.cpp
new file mode 100644
index ..d2750fff56b5
--- /dev/null
+++ b/lldb/test/API/tools/intel-features/intel-pt/test/main.cpp
@@ -0,0 +1,10 @@
+int fun(int a) { return a * a + 1; }
+
+int main() {
+  int z = 0;
+  for (int i = 0; i < 1; i++) { // Break for loop
+z += fun(z);
+  }
+
+  return 0; // Break 1
+}

diff  --git a/lldb/tools/intel-features/intel-pt/cli-wrapper-pt.cpp 
b/lldb/tools/intel-features/intel-pt/cli-wrapper-pt.cpp
index 8db1c0f82d66..5e409a269fa4 100644
--- a/lldb/tools/intel-features/intel-pt/cli-wrapper-pt.cpp
+++ b/lldb/tools/intel-features/intel-pt/cli-wrapper-pt.cpp
@@ -191,6 +191,7 @@ class ProcessorTraceStart : public 
lldb::SBCommandPluginIn

[Lldb-commits] [PATCH] D77324: [source maps] Fix remove, insert-after and replace

[walter erquinigo via Phabricator via lldb-commits]

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

fix typos

I think the quarantine is reducing my IQ overall...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77324

Files:
  lldb/source/Interpreter/OptionValuePathMappings.cpp
  lldb/test/API/functionalities/source-map/TestTargetSourceMap.py

Index: lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
===
--- lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
+++ lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
@@ -42,67 +42,93 @@
 self.assertEquals(bp.GetNumLocations(), 0,
 "make sure no breakpoints were resolved without map")
 
+valid_path = os.path.dirname(src_dir)
+valid_path2 = os.path.dirname(valid_path)
 invalid_path = src_dir + "invalid_path"
 invalid_path2 = src_dir + "invalid_path2"
 
 # We make sure the error message contains all the invalid paths
 self.expect(
-'settings set target.source-map . "%s" . "%s" . "%s"' % (invalid_path, src_dir, invalid_path2),
+'settings set target.source-map . "%s" . "%s" . "%s" . "%s' \
+% (invalid_path, src_dir, invalid_path2, valid_path),
 substrs=[
-'the replacement path doesn\'t exist: "%s"' % (invalid_path),
+'error: the replacement path doesn\'t exist: "%s"' % (invalid_path),
 'the replacement path doesn\'t exist: "%s"' % (invalid_path2),
 ],
 error=True,
 )
 self.expect(
 'settings show target.source-map',
-substrs=['[0] "." -> "%s"' % (src_dir)],
+substrs=[
+'[0] "." -> "%s"' % (src_dir),
+'[1] "." -> "%s"' % (valid_path),
+],
 )
 assertBreakpointWithSourceMap(src_path)
 
-# Index 0 is the valid mapping, and modifying it to an invalid one should have no effect
+# Attempts to replace an index to an invalid mapping should have no effect.
+# Modifications to valid mappings should work.
 self.expect(
-'settings replace target.source-map 0 . "%s"' % (invalid_path),
-substrs=['error: the replacement path doesn\'t exist: "%s"' % (invalid_path)],
+'settings replace target.source-map 0 . "%s" . "%s"' % (invalid_path, valid_path2),
+substrs=[
+'error: the replacement path doesn\'t exist: "%s"' % (invalid_path),
+],
 error=True,
 )
 self.expect(
 'settings show target.source-map',
-substrs=['[0] "." -> "%s"' % (src_dir)]
+substrs=[
+'[0] "." -> "%s"' % (src_dir),
+'[1] "." -> "%s"' % (valid_path2),
+]
 )
 assertBreakpointWithSourceMap(src_path)
 
-# Let's clear and add the mapping in with insert-after
+# Let's clear and add the mapping back with insert-after
 self.runCmd('settings remove target.source-map 0')
 self.expect(
 'settings show target.source-map',
-endstr="target.source-map (path-map) =\n",
+substrs=['[0] "." -> "%s"' % (valid_path2)],
 )
-
-# We add a valid but useless mapping so that we can use insert-after
-another_valid_path = os.path.dirname(src_dir)
-self.runCmd('settings set target.source-map . "%s"' % (another_valid_path))
 
 self.expect(
-'settings replace target.source-map 0 . "%s"' % (invalid_path),
-substrs=['error: the replacement path doesn\'t exist: "%s"' % (invalid_path)],
+'settings insert-after target.source-map 0 . "%s" . "%s" . "%s"' \
+% (invalid_path, invalid_path2, src_dir),
+substrs=[
+'error: the replacement path doesn\'t exist: "%s"' % (invalid_path),
+'the replacement path doesn\'t exist: "%s"' % (invalid_path2),
+],
 error=True,
 )
 self.expect(
 'settings show target.source-map',
-substrs=['[0] "." -> "%s"' % (another_valid_path)]
+substrs=[
+'[0] "." -> "%s"' % (valid_path2),
+'[1] "." -> "%s"' % (src_dir),
+]
 )
 
-# Let's clear and add the mapping in with append
-self.expect('settings remove target.source-map 0')
+# Let's clear using remove and add the mapping in with append
+self.runCmd('settings remove target.source-map 1')
 self.expect(
 'settings show target.source-map',
-endstr="target.source-map (path-map) =\n",
+substrs=[
+'[0] "." -> "%s"' % (valid_path2),
+]
 )
-
+self.

[Lldb-commits] [PATCH] D77324: [source maps] Fix remove, insert-after and replace

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: labath, clayborg.
Herald added subscribers: lldb-commits, mgrang.
Herald added a project: LLDB.
wallace updated this revision to Diff 254576.
wallace added a comment.

fix typos

I think the quarantine is reducing my IQ overall...


In this diff of mine D77186  I introduce a bug 
in the replace operation, where I was failing fast by mistake.
Besides, a similar problem existed in the insert-after operation, where it was 
failing fast.

Finally, the remove operation was wrong, as it was not using the indices 
provided by the users.

I fixed those issues and added some tests account for cases with multiple 
elements in these requests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77324

Files:
  lldb/source/Interpreter/OptionValuePathMappings.cpp
  lldb/test/API/functionalities/source-map/TestTargetSourceMap.py

Index: lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
===
--- lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
+++ lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
@@ -42,67 +42,93 @@
 self.assertEquals(bp.GetNumLocations(), 0,
 "make sure no breakpoints were resolved without map")
 
+valid_path = os.path.dirname(src_dir)
+valid_path2 = os.path.dirname(valid_path)
 invalid_path = src_dir + "invalid_path"
 invalid_path2 = src_dir + "invalid_path2"
 
 # We make sure the error message contains all the invalid paths
 self.expect(
-'settings set target.source-map . "%s" . "%s" . "%s"' % (invalid_path, src_dir, invalid_path2),
+'settings set target.source-map . "%s" . "%s" . "%s" . "%s' \
+% (invalid_path, src_dir, invalid_path2, valid_path),
 substrs=[
-'the replacement path doesn\'t exist: "%s"' % (invalid_path),
+'error: the replacement path doesn\'t exist: "%s"' % (invalid_path),
 'the replacement path doesn\'t exist: "%s"' % (invalid_path2),
 ],
 error=True,
 )
 self.expect(
 'settings show target.source-map',
-substrs=['[0] "." -> "%s"' % (src_dir)],
+substrs=[
+'[0] "." -> "%s"' % (src_dir),
+'[1] "." -> "%s"' % (valid_path),
+],
 )
 assertBreakpointWithSourceMap(src_path)
 
-# Index 0 is the valid mapping, and modifying it to an invalid one should have no effect
+# Attempts to replace an index to an invalid mapping should have no effect.
+# Modifications to valid mappings should work.
 self.expect(
-'settings replace target.source-map 0 . "%s"' % (invalid_path),
-substrs=['error: the replacement path doesn\'t exist: "%s"' % (invalid_path)],
+'settings replace target.source-map 0 . "%s" . "%s"' % (invalid_path, valid_path2),
+substrs=[
+'error: the replacement path doesn\'t exist: "%s"' % (invalid_path),
+],
 error=True,
 )
 self.expect(
 'settings show target.source-map',
-substrs=['[0] "." -> "%s"' % (src_dir)]
+substrs=[
+'[0] "." -> "%s"' % (src_dir),
+'[1] "." -> "%s"' % (valid_path2),
+]
 )
 assertBreakpointWithSourceMap(src_path)
 
-# Let's clear and add the mapping in with insert-after
+# Let's clear and add the mapping back with insert-after
 self.runCmd('settings remove target.source-map 0')
 self.expect(
 'settings show target.source-map',
-endstr="target.source-map (path-map) =\n",
+substrs=['[0] "." -> "%s"' % (valid_path2)],
 )
-
-# We add a valid but useless mapping so that we can use insert-after
-another_valid_path = os.path.dirname(src_dir)
-self.runCmd('settings set target.source-map . "%s"' % (another_valid_path))
 
 self.expect(
-'settings replace target.source-map 0 . "%s"' % (invalid_path),
-substrs=['error: the replacement path doesn\'t exist: "%s"' % (invalid_path)],
+'settings insert-after target.source-map 0 . "%s" . "%s" . "%s"' \
+% (invalid_path, invalid_path2, src_dir),
+substrs=[
+'error: the replacement path doesn\'t exist: "%s"' % (invalid_path),
+'the replacement path doesn\'t exist: "%s"' % (invalid_path2),
+],
 error=True,
 )
 self.expect(
 'settings show target.source-map',
-substrs=['[0] "." -> "%s"' % (another_valid_path)]
+substrs=[
+'[0] "." -> "%s"' % (valid_path2),
+'[1] "." -> "%s"' % (sr

[Lldb-commits] [PATCH] D77327: [lldb] [almost-nfc] 2/2: Introduce DWARF callbacks

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil created this revision.
jankratochvil added reviewers: labath, clayborg.
jankratochvil added a project: LLDB.
Herald added subscribers: arphaman, aprantl.
Herald added a reviewer: shafik.

As requested by @labath in https://reviews.llvm.org/D73206#1949516 (D73206 
) providing DWARF index callbacks 
refactorization.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77327

Files:
  lldb/include/lldb/Core/UniqueCStringMap.h
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -31,8 +31,8 @@
 
   DWARFCompileUnit *GetDWOCompileUnitForHash(uint64_t hash);
 
-  size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
- DIEArray &method_die_offsets) override;
+  bool GetObjCMethods(lldb_private::ConstString class_name,
+  std::function callback) override;
 
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -95,10 +95,10 @@
   return GetBaseSymbolFile().GetForwardDeclClangTypeToDie();
 }
 
-size_t SymbolFileDWARFDwo::GetObjCMethodDIEOffsets(
-lldb_private::ConstString class_name, DIEArray &method_die_offsets) {
-  return GetBaseSymbolFile().GetObjCMethodDIEOffsets(class_name,
- method_die_offsets);
+bool SymbolFileDWARFDwo::GetObjCMethods(
+lldb_private::ConstString class_name,
+std::function callback) {
+  return GetBaseSymbolFile().GetObjCMethods(class_name, callback);
 }
 
 UniqueDWARFASTTypeMap &SymbolFileDWARFDwo::GetUniqueDWARFASTTypeMap() {
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -237,8 +237,8 @@
   lldb_private::CompileUnit *
   GetCompUnitForDWARFCompUnit(DWARFCompileUnit &dwarf_cu);
 
-  virtual size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
- DIEArray &method_die_offsets);
+  virtual bool GetObjCMethods(lldb_private::ConstString class_name,
+  std::function callback);
 
   bool Supports_DW_AT_APPLE_objc_complete_type(DWARFUnit *cu);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1457,11 +1457,9 @@
   return static_cast(non_dwo_cu->GetUserData());
 }
 
-size_t SymbolFileDWARF::GetObjCMethodDIEOffsets(ConstString class_name,
-DIEArray &method_die_offsets) {
-  method_die_offsets.clear();
-  m_index->GetObjCMethods(class_name, method_die_offsets);
-  return method_die_offsets.size();
+bool SymbolFileDWARF::GetObjCMethods(ConstString class_name,
+ std::function callback) {
+  return m_index->GetObjCMethods(class_name, callback);
 }
 
 bool SymbolFileDWARF::GetFunction(const DWARFDIE &die, SymbolContext &sc) {
@@ -2042,24 +2040,20 @@
   context, basename))
 basename = name.GetStringRef();
 
-  DIEArray die_offsets;
-  m_index->GetGlobalVariables(ConstString(basename), die_offsets);
-  const size_t num_die_matches = die_offsets.size();
-
-  SymbolContext sc;
-  sc.module_sp = m_objfile_sp->GetModule();
-  assert(s

[Lldb-commits] [PATCH] D77326: [nfc] [lldb] Unindent code

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil created this revision.
jankratochvil added reviewers: labath, clayborg.
jankratochvil added a project: LLDB.
Herald added subscribers: arphaman, aprantl.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: shafik.

It removes some needless deep indentation and some redundant statements.
It prepares the code for a more clean next patch - DWARF index callbacks.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77326

Files:
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2045,64 +2045,61 @@
   DIEArray die_offsets;
   m_index->GetGlobalVariables(ConstString(basename), die_offsets);
   const size_t num_die_matches = die_offsets.size();
-  if (num_die_matches) {
-SymbolContext sc;
-sc.module_sp = m_objfile_sp->GetModule();
-assert(sc.module_sp);
 
-// Loop invariant: Variables up to this index have been checked for context
-// matches.
-uint32_t pruned_idx = original_size;
+  SymbolContext sc;
+  sc.module_sp = m_objfile_sp->GetModule();
+  assert(sc.module_sp);
 
-bool done = false;
-for (size_t i = 0; i < num_die_matches && !done; ++i) {
-  const DIERef &die_ref = die_offsets[i];
-  DWARFDIE die = GetDIE(die_ref);
+  // Loop invariant: Variables up to this index have been checked for context
+  // matches.
+  uint32_t pruned_idx = original_size;
 
-  if (die) {
-switch (die.Tag()) {
-default:
-case DW_TAG_subprogram:
-case DW_TAG_inlined_subroutine:
-case DW_TAG_try_block:
-case DW_TAG_catch_block:
-  break;
+  for (size_t i = 0; i < num_die_matches; ++i) {
+const DIERef &die_ref = die_offsets[i];
+DWARFDIE die = GetDIE(die_ref);
+if (!die) {
+  m_index->ReportInvalidDIERef(die_ref, name.GetStringRef());
+  continue;
+}
 
-case DW_TAG_variable: {
-  auto *dwarf_cu = llvm::dyn_cast(die.GetCU());
-  if (!dwarf_cu)
-continue;
-  sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
-
-  if (parent_decl_ctx) {
-if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
-  CompilerDeclContext actual_parent_decl_ctx =
-  dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
-  if (!actual_parent_decl_ctx ||
-  actual_parent_decl_ctx != parent_decl_ctx)
-continue;
-}
-  }
+switch (die.Tag()) {
+default:
+case DW_TAG_subprogram:
+case DW_TAG_inlined_subroutine:
+case DW_TAG_try_block:
+case DW_TAG_catch_block:
+  continue;
 
-  ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false,
- &variables);
-  while (pruned_idx < variables.GetSize()) {
-VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
-if (name_is_mangled ||
-var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
-  ++pruned_idx;
-else
-  variables.RemoveVariableAtIndex(pruned_idx);
-  }
+case DW_TAG_variable:
+  break;
+}
+auto *dwarf_cu = llvm::dyn_cast(die.GetCU());
+if (!dwarf_cu)
+  continue;
+sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
 
-  if (variables.GetSize() - original_size >= max_matches)
-done = true;
-} break;
-}
-  } else {
-m_index->ReportInvalidDIERef(die_ref, name.GetStringRef());
+if (parent_decl_ctx) {
+  if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
+CompilerDeclContext actual_parent_decl_ctx =
+dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
+if (!actual_parent_decl_ctx ||
+actual_parent_decl_ctx != parent_decl_ctx)
+  continue;
   }
 }
+
+ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
+while (pruned_idx < variables.GetSize()) {
+  VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
+  if (name_is_mangled ||
+  var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
+++pruned_idx;
+  else
+variables.RemoveVariableAtIndex(pruned_idx);
+}
+
+if (variables.GetSize() - original_size >= max_matches)
+  break;
   }
 
   // Return the number of variable that 

[Lldb-commits] [PATCH] D77327: [lldb] [almost-nfc] 2/2: Introduce DWARF callbacks

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil marked 3 inline comments as done.
jankratochvil added inline comments.



Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:134
+ m_map.begin(), m_map.end(), unique_cstr, Compare(
+  if (callback(entry.value))
+return true;

Semantics of the callback is apparent - return `true` if the caller no longer 
needs any more data. Not sure where to document that, maybe it is simple enough 
in the code.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:68
   DWARFMappedHash::DIEInfoArray hash_data;
-  if (m_apple_names_up->AppendAllDIEsThatMatchingRegex(regex, hash_data))
-DWARFMappedHash::ExtractDIEArray(hash_data, offsets);
+  m_apple_names_up->AppendAllDIEsThatMatchingRegex(regex, hash_data);
+  return DWARFMappedHash::ExtractDIEArray(hash_data, callback);

`DIEInfoArray` is not callback-optimized in this patch. That could be done as 
an add-on patch.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp:84
+if (is_implementation != return_implementation_only_if_available)
+  continue;
+if (return_implementation_only_if_available) {

The meaning of `return_implementation_only_if_available` has changed here. The 
only caller was using it with `true` so now the only existing caller calls it 
twice - first with `true` and then with `false` as we can no longer do the 
original `die_offsets.clear();`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327



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

[Lldb-commits] [PATCH] D77328: [intel-mpx] Delete an unnecessary license header

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: labath, clayborg.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

@labath mentioned to me that test files shouldn't have a license header.
I saw this one some days ago, so I'm doing some cleaning.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77328

Files:
  lldb/tools/intel-features/intel-mpx/test/main.cpp


Index: lldb/tools/intel-features/intel-mpx/test/main.cpp
===
--- lldb/tools/intel-features/intel-mpx/test/main.cpp
+++ lldb/tools/intel-features/intel-mpx/test/main.cpp
@@ -1,12 +1,3 @@
-//===-- main.cpp *- C++ 
-*-===//
-
- Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
- See https://llvm.org/LICENSE.txt for license information.
- SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-===--===//
-//
-
 const int size = 5;
 
 #include 


Index: lldb/tools/intel-features/intel-mpx/test/main.cpp
===
--- lldb/tools/intel-features/intel-mpx/test/main.cpp
+++ lldb/tools/intel-features/intel-mpx/test/main.cpp
@@ -1,12 +1,3 @@
-//===-- main.cpp *- C++ -*-===//
-
- Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
- See https://llvm.org/LICENSE.txt for license information.
- SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-===--===//
-//
-
 const int size = 5;
 
 #include 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

Overall looks good to me except for one larger logic change. Maybe a your 
comment can clarify this. The code was in a very bad shape before, given the 
countless amounts of shortcuts you could take.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2014
 
-if (!method_die_offsets.empty()) {
-  DWARFDebugInfo &debug_info = dwarf->DebugInfo();

I assume this can be removed because you're iterating over `num_matches == 0` 
when it's empty. But I cannot tell about the `DWARFDebugInfo &debug_info = 
dwarf->DebugInfo();` and how costly this call is and if it makes sense to run 
when the offsets are empty.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp:524
   DIEInfoArray die_info_array;
-  if (FindByName(name, die_info_array))
-DWARFMappedHash::ExtractDIEArray(die_info_array, die_offsets);
+  FindByName(name, die_info_array);
+  DWARFMappedHash::ExtractDIEArray(die_info_array, die_offsets);

Why is the `if` no longer needed?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2061
+if (!die) {
+  m_index->ReportInvalidDIERef(die_ref, name.GetStringRef());
+  continue;

Nice shortcut.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2943
+// Make sure the decl contexts match all the way up
+if (dwarf_decl_ctx != type_dwarf_decl_ctx)
+  continue;

This looks like a logic change and I wonder if it should go in a separate 
patch, maybe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326



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

[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I am generally very okay with this change, though some changes look suspicious, 
as @kwk noted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326



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

[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil marked 6 inline comments as done.
jankratochvil added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2014
 
-if (!method_die_offsets.empty()) {
-  DWARFDebugInfo &debug_info = dwarf->DebugInfo();

kwk wrote:
> I assume this can be removed because you're iterating over `num_matches == 0` 
> when it's empty. But I cannot tell about the `DWARFDebugInfo &debug_info = 
> dwarf->DebugInfo();` and how costly this call is and if it makes sense to run 
> when the offsets are empty.
I did not put an attention to it, thanks for catching it. Personally I think it 
is OK but when it has been brought up I am fixing this possible performance 
regression. `SymbolFileDWARF::DebugInfo()` contains some 
`llvm::call_once(m_info_once_flag` which does have some cost.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp:524
   DIEInfoArray die_info_array;
-  if (FindByName(name, die_info_array))
-DWARFMappedHash::ExtractDIEArray(die_info_array, die_offsets);
+  FindByName(name, die_info_array);
+  DWARFMappedHash::ExtractDIEArray(die_info_array, die_offsets);

kwk wrote:
> Why is the `if` no longer needed?
It will now run `DWARFMappedHash::ExtractDIEArray` on empty array which is 
cheap, it does nothing.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2943
+// Make sure the decl contexts match all the way up
+if (dwarf_decl_ctx != type_dwarf_decl_ctx)
+  continue;

kwk wrote:
> This looks like a logic change and I wonder if it should go in a separate 
> patch, maybe?
This should be NFC, do I miss something?
From:
```
if (a == b) {
  c;
}
```
The patch changes it to:
```
if (a != b)
  continue;
c;
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326



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

[Lldb-commits] [PATCH] D77327: [lldb] [almost-nfc] 2/2: Introduce DWARF callbacks

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 254617.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327

Files:
  lldb/include/lldb/Core/UniqueCStringMap.h
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -31,8 +31,8 @@
 
   DWARFCompileUnit *GetDWOCompileUnitForHash(uint64_t hash);
 
-  size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
- DIEArray &method_die_offsets) override;
+  bool GetObjCMethods(lldb_private::ConstString class_name,
+  std::function callback) override;
 
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -95,10 +95,10 @@
   return GetBaseSymbolFile().GetForwardDeclClangTypeToDie();
 }
 
-size_t SymbolFileDWARFDwo::GetObjCMethodDIEOffsets(
-lldb_private::ConstString class_name, DIEArray &method_die_offsets) {
-  return GetBaseSymbolFile().GetObjCMethodDIEOffsets(class_name,
- method_die_offsets);
+bool SymbolFileDWARFDwo::GetObjCMethods(
+lldb_private::ConstString class_name,
+std::function callback) {
+  return GetBaseSymbolFile().GetObjCMethods(class_name, callback);
 }
 
 UniqueDWARFASTTypeMap &SymbolFileDWARFDwo::GetUniqueDWARFASTTypeMap() {
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -237,8 +237,8 @@
   lldb_private::CompileUnit *
   GetCompUnitForDWARFCompUnit(DWARFCompileUnit &dwarf_cu);
 
-  virtual size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
- DIEArray &method_die_offsets);
+  virtual bool GetObjCMethods(lldb_private::ConstString class_name,
+  std::function callback);
 
   bool Supports_DW_AT_APPLE_objc_complete_type(DWARFUnit *cu);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1457,11 +1457,9 @@
   return static_cast(non_dwo_cu->GetUserData());
 }
 
-size_t SymbolFileDWARF::GetObjCMethodDIEOffsets(ConstString class_name,
-DIEArray &method_die_offsets) {
-  method_die_offsets.clear();
-  m_index->GetObjCMethods(class_name, method_die_offsets);
-  return method_die_offsets.size();
+bool SymbolFileDWARF::GetObjCMethods(ConstString class_name,
+ std::function callback) {
+  return m_index->GetObjCMethods(class_name, callback);
 }
 
 bool SymbolFileDWARF::GetFunction(const DWARFDIE &die, SymbolContext &sc) {
@@ -2042,24 +2040,20 @@
   context, basename))
 basename = name.GetStringRef();
 
-  DIEArray die_offsets;
-  m_index->GetGlobalVariables(ConstString(basename), die_offsets);
-  const size_t num_die_matches = die_offsets.size();
-
-  SymbolContext sc;
-  sc.module_sp = m_objfile_sp->GetModule();
-  assert(sc.module_sp);
-
   // Loop invariant: Variables up to this index have been checked for context
   // matches.
   uint32_t pruned_idx = original_size;
 
-  for (size_t i = 0; i < num_die_matches; ++i) {
-const DIERef &die_ref = die_offsets[i];

[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 254616.
jankratochvil marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326

Files:
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2045,64 +2045,61 @@
   DIEArray die_offsets;
   m_index->GetGlobalVariables(ConstString(basename), die_offsets);
   const size_t num_die_matches = die_offsets.size();
-  if (num_die_matches) {
-SymbolContext sc;
-sc.module_sp = m_objfile_sp->GetModule();
-assert(sc.module_sp);
 
-// Loop invariant: Variables up to this index have been checked for context
-// matches.
-uint32_t pruned_idx = original_size;
+  SymbolContext sc;
+  sc.module_sp = m_objfile_sp->GetModule();
+  assert(sc.module_sp);
 
-bool done = false;
-for (size_t i = 0; i < num_die_matches && !done; ++i) {
-  const DIERef &die_ref = die_offsets[i];
-  DWARFDIE die = GetDIE(die_ref);
+  // Loop invariant: Variables up to this index have been checked for context
+  // matches.
+  uint32_t pruned_idx = original_size;
 
-  if (die) {
-switch (die.Tag()) {
-default:
-case DW_TAG_subprogram:
-case DW_TAG_inlined_subroutine:
-case DW_TAG_try_block:
-case DW_TAG_catch_block:
-  break;
+  for (size_t i = 0; i < num_die_matches; ++i) {
+const DIERef &die_ref = die_offsets[i];
+DWARFDIE die = GetDIE(die_ref);
+if (!die) {
+  m_index->ReportInvalidDIERef(die_ref, name.GetStringRef());
+  continue;
+}
 
-case DW_TAG_variable: {
-  auto *dwarf_cu = llvm::dyn_cast(die.GetCU());
-  if (!dwarf_cu)
-continue;
-  sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
-
-  if (parent_decl_ctx) {
-if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
-  CompilerDeclContext actual_parent_decl_ctx =
-  dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
-  if (!actual_parent_decl_ctx ||
-  actual_parent_decl_ctx != parent_decl_ctx)
-continue;
-}
-  }
+switch (die.Tag()) {
+default:
+case DW_TAG_subprogram:
+case DW_TAG_inlined_subroutine:
+case DW_TAG_try_block:
+case DW_TAG_catch_block:
+  continue;
 
-  ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false,
- &variables);
-  while (pruned_idx < variables.GetSize()) {
-VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
-if (name_is_mangled ||
-var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
-  ++pruned_idx;
-else
-  variables.RemoveVariableAtIndex(pruned_idx);
-  }
+case DW_TAG_variable:
+  break;
+}
+auto *dwarf_cu = llvm::dyn_cast(die.GetCU());
+if (!dwarf_cu)
+  continue;
+sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
 
-  if (variables.GetSize() - original_size >= max_matches)
-done = true;
-} break;
-}
-  } else {
-m_index->ReportInvalidDIERef(die_ref, name.GetStringRef());
+if (parent_decl_ctx) {
+  if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
+CompilerDeclContext actual_parent_decl_ctx =
+dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
+if (!actual_parent_decl_ctx ||
+actual_parent_decl_ctx != parent_decl_ctx)
+  continue;
   }
 }
+
+ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
+while (pruned_idx < variables.GetSize()) {
+  VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
+  if (name_is_mangled ||
+  var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
+++pruned_idx;
+  else
+variables.RemoveVariableAtIndex(pruned_idx);
+}
+
+if (variables.GetSize() - original_size >= max_matches)
+  break;
   }
 
   // Return the number of variable that were appended to the list
@@ -2142,25 +2139,23 @@
   assert(sc.module_sp);
 
   const size_t num_matches = die_offsets.size();
-  if (num_matches) {
-for (size_t i = 0; i < num_matches; ++i) {
-  const DIERef &di

[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

I feel like this should have been split in two. It feels like there are 
straight forward obviously correct changes and a several others that require 
some thinking about but look correct.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp:524
   DIEInfoArray die_info_array;
-  if (FindByName(name, die_info_array))
-DWARFMappedHash::ExtractDIEArray(die_info_array, die_offsets);
+  FindByName(name, die_info_array);
+  DWARFMappedHash::ExtractDIEArray(die_info_array, die_offsets);

jankratochvil wrote:
> kwk wrote:
> > Why is the `if` no longer needed?
> It will now run `DWARFMappedHash::ExtractDIEArray` on empty array which is 
> cheap, it does nothing.
If we stick with this change, please add a comment explaining this. It is not 
obvious and someone later on may come and change it back thinking this is a bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326



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

[Lldb-commits] [PATCH] D77336: Findtypes -gmodules fix

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil created this revision.
jankratochvil added reviewers: labath, kwk.
jankratochvil added a project: LLDB.

A little fixup so that D77327  can be called 
NFC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77336

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2408,7 +2408,7 @@
   // Next search through the reachable Clang modules. This only applies for
   // DWARF objects compiled with -gmodules that haven't been processed by
   // dsymutil.
-  if (num_die_matches < max_matches) {
+  if (types.GetSize() < max_matches) {
 UpdateExternalModuleListIfNeeded();
 
 for (const auto &pair : m_external_type_modules)


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2408,7 +2408,7 @@
   // Next search through the reachable Clang modules. This only applies for
   // DWARF objects compiled with -gmodules that haven't been processed by
   // dsymutil.
-  if (num_die_matches < max_matches) {
+  if (types.GetSize() < max_matches) {
 UpdateExternalModuleListIfNeeded();
 
 for (const auto &pair : m_external_type_modules)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil marked an inline comment as done.
jankratochvil added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2387
   // dsymutil.
-  if (num_die_matches < max_matches) {
+  if (types.GetSize() < max_matches) {
 UpdateExternalModuleListIfNeeded();

This is not NFC but it is required by the refactorization. I have posted this 
bugfix separately as D77336.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327



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

[Lldb-commits] [PATCH] D77337: [lldb/Symbol] Reimplement Symbols::FindSymbolFileInBundle to use the VFS

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: aprantl, jasonmolenda, labath.

This reimplements Symbols::FindSymbolFileInBundle to use the VFS-aware 
recursive directory iterator. This is needed for reproducer replay.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D77337

Files:
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp


Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -253,48 +253,32 @@
 FileSpec Symbols::FindSymbolFileInBundle(const FileSpec &dsym_bundle_fspec,
  const lldb_private::UUID *uuid,
  const ArchSpec *arch) {
-  char path[PATH_MAX];
-  if (dsym_bundle_fspec.GetPath(path, sizeof(path)) == 0)
-return {};
-
-  ::strncat(path, "/Contents/Resources/DWARF", sizeof(path) - strlen(path) - 
1);
-
-  DIR *dirp = opendir(path);
-  if (!dirp)
-return {};
-
-  // Make sure we close the directory before exiting this scope.
-  auto cleanup_dir = llvm::make_scope_exit([&]() { closedir(dirp); });
-
-  FileSpec dsym_fspec;
-  dsym_fspec.GetDirectory().SetCString(path);
-  struct dirent *dp;
-  while ((dp = readdir(dirp)) != NULL) {
-// Only search directories
-if (dp->d_type == DT_DIR || dp->d_type == DT_UNKNOWN) {
-  if (dp->d_namlen == 1 && dp->d_name[0] == '.')
-continue;
-
-  if (dp->d_namlen == 2 && dp->d_name[0] == '.' && dp->d_name[1] == '.')
-continue;
-}
-
-if (dp->d_type == DT_REG || dp->d_type == DT_UNKNOWN) {
-  dsym_fspec.GetFilename().SetCString(dp->d_name);
-  ModuleSpecList module_specs;
-  if (ObjectFile::GetModuleSpecifications(dsym_fspec, 0, 0, module_specs)) 
{
-ModuleSpec spec;
-for (size_t i = 0; i < module_specs.GetSize(); ++i) {
-  bool got_spec = module_specs.GetModuleSpecAtIndex(i, spec);
-  UNUSED_IF_ASSERT_DISABLED(got_spec);
-  assert(got_spec);
-  if ((uuid == NULL ||
-   (spec.GetUUIDPtr() && spec.GetUUID() == *uuid)) &&
-  (arch == NULL ||
-   (spec.GetArchitecturePtr() &&
-spec.GetArchitecture().IsCompatibleMatch(*arch {
-return dsym_fspec;
-  }
+  std::string dsym_bundle_path = dsym_bundle_fspec.GetPath();
+  llvm::SmallString<128> buffer(dsym_bundle_path);
+  llvm::sys::path::append(buffer, "Contents", "Resources", "DWARF");
+
+  std::error_code EC;
+  llvm::IntrusiveRefCntPtr vfs =
+  FileSystem::Instance().GetVirtualFileSystem();
+  llvm::vfs::recursive_directory_iterator Iter(*vfs, buffer.str(), EC);
+  llvm::vfs::recursive_directory_iterator End;
+  for (; Iter != End && !EC; Iter.increment(EC)) {
+llvm::ErrorOr Status = vfs->status(Iter->path());
+if (Status->isDirectory())
+  continue;
+
+FileSpec dsym_fspec(Iter->path());
+ModuleSpecList module_specs;
+if (ObjectFile::GetModuleSpecifications(dsym_fspec, 0, 0, module_specs)) {
+  ModuleSpec spec;
+  for (size_t i = 0; i < module_specs.GetSize(); ++i) {
+assert(module_specs.GetModuleSpecAtIndex(i, spec));
+if ((uuid == nullptr ||
+ (spec.GetUUIDPtr() && spec.GetUUID() == *uuid)) &&
+(arch == nullptr ||
+ (spec.GetArchitecturePtr() &&
+  spec.GetArchitecture().IsCompatibleMatch(*arch {
+  return dsym_fspec;
 }
   }
 }


Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -253,48 +253,32 @@
 FileSpec Symbols::FindSymbolFileInBundle(const FileSpec &dsym_bundle_fspec,
  const lldb_private::UUID *uuid,
  const ArchSpec *arch) {
-  char path[PATH_MAX];
-  if (dsym_bundle_fspec.GetPath(path, sizeof(path)) == 0)
-return {};
-
-  ::strncat(path, "/Contents/Resources/DWARF", sizeof(path) - strlen(path) - 1);
-
-  DIR *dirp = opendir(path);
-  if (!dirp)
-return {};
-
-  // Make sure we close the directory before exiting this scope.
-  auto cleanup_dir = llvm::make_scope_exit([&]() { closedir(dirp); });
-
-  FileSpec dsym_fspec;
-  dsym_fspec.GetDirectory().SetCString(path);
-  struct dirent *dp;
-  while ((dp = readdir(dirp)) != NULL) {
-// Only search directories
-if (dp->d_type == DT_DIR || dp->d_type == DT_UNKNOWN) {
-  if (dp->d_namlen == 1 && dp->d_name[0] == '.')
-continue;
-
-  if (dp->d_namlen == 2 && dp->d_name[0] == '.' && dp->d_name[1] == '.')
-continue;
-}
-
-if (dp->d_type == DT_REG || dp->d_type == DT_UNKNOWN) {
-  dsym_fspec.GetFilename().SetCString(dp->d_name);
-  ModuleSpecList module_specs;
-  if (ObjectFile::GetModuleS

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd updated this revision to Diff 254631.

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

https://reviews.llvm.org/D77287

Files:
  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/source/Plugins/Platform/Windows/PlatformWindows.h

Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.h
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.h
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.h
@@ -49,6 +49,15 @@
 
   lldb_private::Status DisconnectRemote() override;
 
+  uint32_t DoLoadImage(lldb_private::Process *process,
+   const lldb_private::FileSpec &remote_file,
+   const std::vector *paths,
+   lldb_private::Status &error,
+   lldb_private::FileSpec *loaded_path) override;
+
+  lldb_private::Status UnloadImage(lldb_private::Process *process,
+   uint32_t image_token) override;
+
   lldb::ProcessSP DebugProcess(lldb_private::ProcessLaunchInfo &launch_info,
lldb_private::Debugger &debugger,
lldb_private::Target *target,
@@ -73,6 +82,10 @@
 
 private:
   DISALLOW_COPY_AND_ASSIGN(PlatformWindows);
+
+  lldb_private::Status EvaluateLoaderExpression(lldb_private::Process *process,
+const char *expression,
+lldb::ValueObjectSP &value);
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -20,7 +20,10 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/Status.h"
 
@@ -306,6 +309,111 @@
   return error;
 }
 
+Status PlatformWindows::EvaluateLoaderExpression(Process *process,
+ const char *expression,
+ ValueObjectSP &value) {
+  // FIXME(compnerd) `-fdeclspec` is not passed to the clang instance?
+  static const char kLoaderDecls[] =
+  R"(
+  // libloaderapi.h
+
+  // WINBASEAPI BOOL WINAPI FreeModule(HMODULE);
+  extern "C" /* __declspec(dllimport) */ BOOL __stdcall FreeModule(void *hLibModule);
+
+  // WINBASEAPI HMODULE WINAPI LoadLibraryA(LPCSTR);
+  extern "C" /* __declspec(dllimport) */ void * __stdcall LoadLibraryA(const char *);
+)";
+
+  if (DynamicLoader *loader = process->GetDynamicLoader()) {
+Status result = loader->CanLoadImage();
+if (result.Fail())
+  return result;
+  }
+
+  ThreadSP thread = process->GetThreadList().GetExpressionExecutionThread();
+  if (!thread)
+return Status("selected thread is invalid");
+
+  StackFrameSP frame = thread->GetStackFrameAtIndex(0);
+  if (!frame)
+return Status("frame 0 is invalid");
+
+  ExecutionContext context;
+  frame->CalculateExecutionContext(context);
+
+  EvaluateExpressionOptions options;
+  options.SetUnwindOnError(true);
+  options.SetIgnoreBreakpoints(true);
+  options.SetExecutionPolicy(eExecutionPolicyAlways);
+  options.SetLanguage(eLanguageTypeC_plus_plus);
+  // LoadLibrary{A,W}/FreeLibrary cannot raise exceptions which we can handle.
+  // They may potentially throw SEH exceptions which we do not know how to
+  // handle currently.
+  options.SetTrapExceptions(false);
+  options.SetTimeout(process->GetUtilityExpressionTimeout());
+
+  Status error;
+  ExpressionResults result = UserExpression::Evaluate(
+  context, options, expression, kLoaderDecls, value, error);
+  if (result != eExpressionCompleted)
+return error;
+
+  if (value->GetError().Fail())
+return value->GetError();
+
+  return Status();
+}
+
+uint32_t PlatformWindows::DoLoadImage(Process *process,
+  const FileSpec &remote_file,
+  const std::vector *paths,
+  Status &error, FileSpec *loaded_path) {
+  if (loaded_path)
+loaded_path->Clear();
+
+  StreamString expression;
+  expression.Printf("LoadLibraryA(\"%s\")", remote_file.GetPath().c_str());
+
+  ValueObjectSP value;
+  Status result =
+  EvaluateLoaderExpression(process, expression.GetData(), value);
+  if (result.Fail())
+return LLDB_INVALID_IMAGE_TOKEN;
+
+  Scalar scalar;
+  if (value->ResolveValue(scalar)) {
+lldb::addr_t address = scalar.ULongLong();
+if (address == 0)
+  return LLDB_INVALID_IMAGE_TOKEN;
+return process

[Lldb-commits] [PATCH] D77295: [lldb/Core] Fix a race in the Communication class

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Core/Communication.cpp:318
   bool done = false;
+  bool disconnect = false;
   while (!done && comm->m_read_thread_enabled) {

Can we just init "disconnect" right here?:

```
const bool disconnect = comm->GetCloseOnEOF();
```




Comment at: lldb/source/Core/Communication.cpp:322-327
 if (bytes_read > 0)
   comm->AppendBytesToCache(buf, bytes_read, true, status);
 else if ((bytes_read == 0) && status == eConnectionStatusEndOfFile) {
-  if (comm->GetCloseOnEOF())
-comm->Disconnect();
+  disconnect = comm->GetCloseOnEOF();
   comm->AppendBytesToCache(buf, bytes_read, true, status);
 }

Is there a reason we call AppendBytesToCache with zero bytes in the else? If we 
do need to call it (it might listen to the "status"?) then this:

```
if (bytes_read > 0)
  comm->AppendBytesToCache(buf, bytes_read, true, status);
else if ((bytes_read == 0) && status == eConnectionStatusEndOfFile) {
  disconnect = comm->GetCloseOnEOF();
  comm->AppendBytesToCache(buf, bytes_read, true, status);
}
```
can just be:

```
comm->AppendBytesToCache(buf, bytes_read, true, status);
```

bytes_read is a size_t so it can't be less than zero. Then we can move the 
"disconnect = comm->GetCloseOnEOF();" into the eConnectionStatusEndOfFile case 
in the switch below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77295



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

[Lldb-commits] [PATCH] D77347: Have lldb-vscode update the currently selecte thread and frame when it receives a "scopes" request.

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: labath, aadsm, wallace, JDevlieghere.
Herald added a project: LLDB.
wallace accepted this revision.
wallace added a comment.
This revision is now accepted and ready to land.

I agree with the implementation.
I also find no way notify the IDE that the user has switched to a different 
thread or frame in the console, and I actually think it's better this way.




Comment at: lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py:20
+def check_lldb_command(self, lldb_command, contains_string, assert_msg):
+response = self.vscode.request_evaluate('`%s' % (lldb_command))
+output = response['body']['result']

just use lldb_command instead of using a formatted string



Comment at: lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py:56
+self.vscode.get_local_variables(frameIndex=0)
+# Verify frame #1 is selected in the command interpreter by running
+# the "frame seelct" command with no frame index which will print the

frame #0



Comment at: lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py:57
+# Verify frame #1 is selected in the command interpreter by running
+# the "frame seelct" command with no frame index which will print the
+# currently selected frame.

select


The IDE has no packets that are sent to lldb-vscode that say which thread and 
frame are selected. The only way we know is we get a request for variables for 
a stack frame via a "scopes" request. When we receive this packet we make that 
thread and frame the selected thread and frame in lldb. This way when people 
execute lldb commands in the debug console by prefixing the expression with the 
backtick character, we will have the right thread and frame selected. 
Previously this was not updated as new stack frames were selected.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77347

Files:
  lldb/test/API/tools/lldb-vscode/console/Makefile
  lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py
  lldb/test/API/tools/lldb-vscode/console/main.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -959,7 +959,7 @@
   for (size_t i = 0; i < count; i++) {
 std::string match = matches.GetStringAtIndex(i);
 std::string description = descriptions.GetStringAtIndex(i);
-
+
 llvm::json::Object item;
 
 llvm::StringRef match_ref = match;
@@ -1262,7 +1262,7 @@
   // The debug adapter supports the stepInTargetsRequest.
   body.try_emplace("supportsStepInTargetsRequest", false);
   // We need to improve the current implementation of completions in order to
-  // enable it again. For some context, this is how VSCode works: 
+  // enable it again. For some context, this is how VSCode works:
   // - VSCode sends a completion request whenever chars are added, the user
   //   triggers completion manually via CTRL-space or similar mechanisms, but
   //   not when there's a deletion. Besides, VSCode doesn't let us know which
@@ -1595,6 +1595,24 @@
   llvm::json::Object body;
   auto arguments = request.getObject("arguments");
   lldb::SBFrame frame = g_vsc.GetLLDBFrame(*arguments);
+  // As the user selects different stack frames in the GUI, a "scopes" request
+  // will be sent to the DAP. This is the only way we know that the user has
+  // selected a frame in a thread. There are no other notifications that are
+  // sent and VS code doesn't allow multiple frames to show variables
+  // concurrently. If we select the thread and frame as the "scopes" requests
+  // are sent, this allows users to type commands in the debugger console
+  // with a backtick character to run lldb commands and these lldb commands
+  // will now have the right context selected as they are run. If the user
+  // types "`bt" into the debugger console and we had another thread selected
+  // in the LLDB library, we would show the wrong thing to the user. If the
+  // users switches threads with a lldb command like "`thread select 14", the
+  // GUI will not update as there are no "event" notification packets that
+  // allow us to change the currently selected thread or frame in the GUI that
+  // I am aware of.
+  if (frame.IsValid()) {
+frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
+frame.GetThread().SetSelectedFrame(frame.GetFrameID());
+  }
   g_vsc.variables.Clear();
   g_vsc.variables.Append(frame.GetVariables(true,   // arguments
 true,   // locals
Index: lldb/test/API/tools/lldb-vscode/console/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/console/

[Lldb-commits] [PATCH] D77347: Have lldb-vscode update the currently selecte thread and frame when it receives a "scopes" request.

[walter erquinigo via Phabricator via lldb-commits]

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

I agree with the implementation.
I also find no way notify the IDE that the user has switched to a different 
thread or frame in the console, and I actually think it's better this way.




Comment at: lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py:20
+def check_lldb_command(self, lldb_command, contains_string, assert_msg):
+response = self.vscode.request_evaluate('`%s' % (lldb_command))
+output = response['body']['result']

just use lldb_command instead of using a formatted string



Comment at: lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py:56
+self.vscode.get_local_variables(frameIndex=0)
+# Verify frame #1 is selected in the command interpreter by running
+# the "frame seelct" command with no frame index which will print the

frame #0



Comment at: lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py:57
+# Verify frame #1 is selected in the command interpreter by running
+# the "frame seelct" command with no frame index which will print the
+# currently selected frame.

select


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77347



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

[Lldb-commits] [PATCH] D77347: Have lldb-vscode update the currently selecte thread and frame when it receives a "scopes" request.

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py:20
+def check_lldb_command(self, lldb_command, contains_string, assert_msg):
+response = self.vscode.request_evaluate('`%s' % (lldb_command))
+output = response['body']['result']

wallace wrote:
> just use lldb_command instead of using a formatted string
I didn't notice the backtick. Forget about this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77347



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

[Lldb-commits] [PATCH] D77351: [lldb/Support] Treat empty FileSpec as an invalid file.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, teemperor.

LLDB relies on empty FileSpecs being invalid files, for example, they don't 
exists. Currently this assumption does not always hold during reproducer 
replay, because we pass the result of `GetPath` to the VFS. This is an empty 
string, which the VFS converts to an absolute directory by prepending the 
current working directory, before looking it up in the YAML mapping. This means 
that an empty FileSpec will exist when the current working directory does. This 
breaks at least one test (`TestAddDsymCommand.py`) when ran from replay.

This patch special cases empty FileSpecs and returns a sensible result before 
calling GetPath and forwarding the call.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D77351

Files:
  lldb/source/Host/common/FileSystem.cpp
  lldb/unittests/Host/FileSystemTest.cpp

Index: lldb/unittests/Host/FileSystemTest.cpp
===
--- lldb/unittests/Host/FileSystemTest.cpp
+++ lldb/unittests/Host/FileSystemTest.cpp
@@ -303,3 +303,29 @@
   EXPECT_EQ(code.value(), ENOENT);
 }
 
+TEST(FileSystemTest, EmptyTest) {
+  FileSpec spec;
+  FileSystem fs;
+
+  {
+std::error_code ec;
+fs.DirBegin(spec, ec);
+EXPECT_EQ(ec.category(), std::system_category());
+EXPECT_EQ(ec.value(), ENOENT);
+  }
+
+  {
+llvm::ErrorOr status = fs.GetStatus(spec);
+ASSERT_FALSE(status);
+EXPECT_EQ(status.getError().category(), std::system_category());
+EXPECT_EQ(status.getError().value(), ENOENT);
+  }
+
+  EXPECT_EQ(sys::TimePoint<>(), fs.GetModificationTime(spec));
+  EXPECT_EQ(static_cast(0), fs.GetByteSize(spec));
+  EXPECT_EQ(llvm::sys::fs::perms::perms_not_known, fs.GetPermissions(spec));
+  EXPECT_FALSE(fs.Exists(spec));
+  EXPECT_FALSE(fs.Readable(spec));
+  EXPECT_FALSE(fs.IsDirectory(spec));
+  EXPECT_FALSE(fs.IsLocal(spec));
+}
Index: lldb/source/Host/common/FileSystem.cpp
===
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -87,6 +87,11 @@
 
 vfs::directory_iterator FileSystem::DirBegin(const FileSpec &file_spec,
  std::error_code &ec) {
+  if (!file_spec) {
+ec = std::error_code(static_cast(errc::no_such_file_or_directory),
+ std::system_category());
+return {};
+  }
   return DirBegin(file_spec.GetPath(), ec);
 }
 
@@ -97,6 +102,9 @@
 
 llvm::ErrorOr
 FileSystem::GetStatus(const FileSpec &file_spec) const {
+  if (!file_spec)
+return std::error_code(static_cast(errc::no_such_file_or_directory),
+   std::system_category());
   return GetStatus(file_spec.GetPath());
 }
 
@@ -106,6 +114,8 @@
 
 sys::TimePoint<>
 FileSystem::GetModificationTime(const FileSpec &file_spec) const {
+  if (!file_spec)
+return sys::TimePoint<>();
   return GetModificationTime(file_spec.GetPath());
 }
 
@@ -117,6 +127,8 @@
 }
 
 uint64_t FileSystem::GetByteSize(const FileSpec &file_spec) const {
+  if (!file_spec)
+return 0;
   return GetByteSize(file_spec.GetPath());
 }
 
@@ -133,6 +145,8 @@
 
 uint32_t FileSystem::GetPermissions(const FileSpec &file_spec,
 std::error_code &ec) const {
+  if (!file_spec)
+return sys::fs::perms::perms_not_known;
   return GetPermissions(file_spec.GetPath(), ec);
 }
 
@@ -154,7 +168,7 @@
 bool FileSystem::Exists(const Twine &path) const { return m_fs->exists(path); }
 
 bool FileSystem::Exists(const FileSpec &file_spec) const {
-  return Exists(file_spec.GetPath());
+  return file_spec && Exists(file_spec.GetPath());
 }
 
 bool FileSystem::Readable(const Twine &path) const {
@@ -162,7 +176,7 @@
 }
 
 bool FileSystem::Readable(const FileSpec &file_spec) const {
-  return Readable(file_spec.GetPath());
+  return file_spec && Readable(file_spec.GetPath());
 }
 
 bool FileSystem::IsDirectory(const Twine &path) const {
@@ -173,7 +187,7 @@
 }
 
 bool FileSystem::IsDirectory(const FileSpec &file_spec) const {
-  return IsDirectory(file_spec.GetPath());
+  return file_spec && IsDirectory(file_spec.GetPath());
 }
 
 bool FileSystem::IsLocal(const Twine &path) const {
@@ -183,7 +197,7 @@
 }
 
 bool FileSystem::IsLocal(const FileSpec &file_spec) const {
-  return IsLocal(file_spec.GetPath());
+  return file_spec && IsLocal(file_spec.GetPath());
 }
 
 void FileSystem::EnumerateDirectory(Twine path, bool find_directories,
@@ -261,6 +275,9 @@
 }
 
 void FileSystem::Resolve(FileSpec &file_spec) {
+  if (!file_spec)
+return;
+
   // Extract path from the FileSpec.
   SmallString<128> path;
   file_spec.GetPath(path);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In D77326#1958082 , @shafik wrote:

> I feel like this should have been split in two. It feels like there are 
> straight forward obviously correct changes and a several others that require 
> some thinking about but look correct.


That's a really good point. I agree. Can you split all changes of one kind into 
separate patches?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326



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

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

[Jim Ingham via Phabricator via lldb-commits]

jingham updated this revision to Diff 254664.
jingham added a comment.

I changed target.process.plugin-reports-all-threads to 
target.experimental.os-plugin-reports-all-threads.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76814

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanStack.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadList.cpp
  lldb/source/Target/ThreadPlan.cpp
  lldb/source/Target/ThreadPlanStack.cpp
  lldb/source/Target/ThreadPlanStepOut.cpp
  
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/Makefile
  
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
  
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/main.cpp
  
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/operating_system.py
  lldb/test/API/functionalities/thread_plan/Makefile
  lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
  lldb/test/API/functionalities/thread_plan/main.c

Index: lldb/test/API/functionalities/thread_plan/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/thread_plan/main.c
@@ -0,0 +1,16 @@
+#include 
+
+void
+call_me(int value) {
+  printf("called with %d\n", value); // Set another here.
+}
+
+int
+main(int argc, char **argv)
+{
+  call_me(argc); // Set a breakpoint here.
+  printf("This just spaces the two calls\n");
+  call_me(argc); // Run here to step over again.
+  printf("More spacing\n");
+  return 0; // Make sure we get here on last continue
+}
Index: lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
===
--- /dev/null
+++ lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
@@ -0,0 +1,164 @@
+"""
+Test that thread plan listing, and deleting works.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestThreadPlanCommands(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_thread_plan_actions(self):
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.c")
+self.thread_plan_test()
+
+def check_list_output(self, command, active_plans = [], completed_plans = [], discarded_plans = []):
+# Check the "thread plan list" output against a list of active & completed and discarded plans.
+# If all three check arrays are empty, that means the command is expected to fail. 
+
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+
+num_active = len(active_plans)
+num_completed = len(completed_plans)
+num_discarded = len(discarded_plans)
+
+interp.HandleCommand(command, result)
+if self.TraceOn():
+print("Command: %s"%(command))
+print(result.GetOutput())
+
+if num_active == 0 and num_completed == 0 and num_discarded == 0:
+self.assertFalse(result.Succeeded(), "command: '%s' succeeded when it should have failed: '%s'"%
+ (command, result.GetError()))
+return
+
+self.assertTrue(result.Succeeded(), "command: '%s' failed: '%s'"%(command, result.GetError()))
+result_arr = result.GetOutput().splitlines()
+num_results = len(result_arr)
+
+# Match the expected number of elements.
+# Adjust the count for the number of header lines we aren't matching:
+fudge = 0
+
+if num_completed == 0 and num_discarded == 0:
+# The fudge is 3: Thread header, Active Plan header and base plan
+fudge = 3
+elif num_completed == 0 or num_discarded == 0:
+# The fudge is 4: The above plus either the Completed or Discarded Plan header:
+fudge = 4
+else:
+# The fudge is 5 since we have both headers:
+fudge = 5
+
+self.assertEqual(num_results, num_active + num_completed + num_discarded + fudge,
+ "Too many elements in match arrays")
+
+# Now iterate through the results array and pick out the results.
+result_idx = 0
+self.assertIn("thread #", result_arr[result_idx], "Found thread header") ; result_idx += 1
+self.assertIn("Active plan stack", result_arr[result_idx], "Found active header") ; result_idx += 1
+self.assertIn("Elemen

[Lldb-commits] [lldb] 5998ace - Have lldb-vscode update the currently selecte thread and frame when it receives a "scopes" request.

[Greg Clayton via lldb-commits]

Author: Greg Clayton
Date: 2020-04-02T18:35:17-07:00
New Revision: 5998aceda9fdca04db4d9ee390cec660896bf0bf

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

LOG: Have lldb-vscode update the currently selecte thread and frame when it 
receives a "scopes" request.

Summary: The IDE has no packets that are sent to lldb-vscode that say which 
thread and frame are selected. The only way we know is we get a request for 
variables for a stack frame via a "scopes" request. When we receive this packet 
we make that thread and frame the selected thread and frame in lldb. This way 
when people execute lldb commands in the debug console by prefixing the 
expression with the backtick character, we will have the right thread and frame 
selected. Previously this was not updated as new stack frames were selected.

Reviewers: labath, aadsm, wallace, JDevlieghere

Subscribers: lldb-commits

Tags: #lldb

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

Added: 
lldb/test/API/tools/lldb-vscode/console/Makefile
lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py
lldb/test/API/tools/lldb-vscode/console/main.cpp

Modified: 
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




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

diff  --git a/lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py 
b/lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py
new file mode 100644
index ..fd42b66ccac2
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py
@@ -0,0 +1,70 @@
+"""
+Test lldb-vscode setBreakpoints request
+"""
+
+from __future__ import print_function
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+
+
+class TestVSCode_console(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def check_lldb_command(self, lldb_command, contains_string, assert_msg):
+response = self.vscode.request_evaluate('`%s' % (lldb_command))
+output = response['body']['result']
+self.assertTrue(contains_string in output,
+("""Verify %s by checking the command output:\n"""
+ """'''\n%s'''\nfor the string: "%s" """ % (
+ assert_msg, output, contains_string)))
+
+@skipIfWindows
+@skipIfRemote
+def test_scopes_variables_setVariable_evaluate(self):
+'''
+Tests that the "scopes" request causes the currently selected
+thread and frame to be updated. There are no DAP packets that tell
+lldb-vscode which thread and frame are selected other than the
+"scopes" request. lldb-vscode will now select the thread and frame
+for the latest "scopes" request that it receives.
+
+The LLDB command interpreter needs to have the right thread and
+frame selected so that commands executed in the debug console act
+on the right scope. This applies both to the expressions that are
+evaluated and the lldb commands that start with the backtick
+character.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = 'main.cpp'
+breakpoint1_line = line_number(source, '// breakpoint 1')
+lines = [breakpoint1_line]
+# Set breakpoint in the thread function so we can step the threads
+breakpoint_ids = self.set_source_breakpoints(source, lines)
+self.assertTrue(len(breakpoint_ids) == len(lines),
+"expect correct number of breakpoints")
+self.continue_to_breakpoints(breakpoint_ids)
+# Cause a "scopes" to be sent for frame zero which should update the
+# selected thread and frame to frame 0.
+self.vscode.get_local_variables(frameIndex=0)
+# Verify frame #0 is selected in the command interpreter by running
+# the "frame select" command with no frame index which will print the
+# currently selected frame.
+self.check_lldb_command("frame select", "frame #0",
+"frame 0 is selected")
+
+# Cause a "scopes" to be sent for frame one which should update the
+# selected thread and frame to frame 1.
+self.vscode.get_local_variables(frameIndex=1)
+# Verify frame #1 is selected in the command interp

[Lldb-commits] [PATCH] D77347: Have lldb-vscode update the currently selecte thread and frame when it receives a "scopes" request.

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 254670.
clayborg added a comment.

Fix typos.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77347

Files:
  lldb/test/API/tools/lldb-vscode/console/Makefile
  lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py
  lldb/test/API/tools/lldb-vscode/console/main.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -959,7 +959,7 @@
   for (size_t i = 0; i < count; i++) {
 std::string match = matches.GetStringAtIndex(i);
 std::string description = descriptions.GetStringAtIndex(i);
-
+
 llvm::json::Object item;
 
 llvm::StringRef match_ref = match;
@@ -1262,7 +1262,7 @@
   // The debug adapter supports the stepInTargetsRequest.
   body.try_emplace("supportsStepInTargetsRequest", false);
   // We need to improve the current implementation of completions in order to
-  // enable it again. For some context, this is how VSCode works: 
+  // enable it again. For some context, this is how VSCode works:
   // - VSCode sends a completion request whenever chars are added, the user
   //   triggers completion manually via CTRL-space or similar mechanisms, but
   //   not when there's a deletion. Besides, VSCode doesn't let us know which
@@ -1595,6 +1595,24 @@
   llvm::json::Object body;
   auto arguments = request.getObject("arguments");
   lldb::SBFrame frame = g_vsc.GetLLDBFrame(*arguments);
+  // As the user selects different stack frames in the GUI, a "scopes" request
+  // will be sent to the DAP. This is the only way we know that the user has
+  // selected a frame in a thread. There are no other notifications that are
+  // sent and VS code doesn't allow multiple frames to show variables
+  // concurrently. If we select the thread and frame as the "scopes" requests
+  // are sent, this allows users to type commands in the debugger console
+  // with a backtick character to run lldb commands and these lldb commands
+  // will now have the right context selected as they are run. If the user
+  // types "`bt" into the debugger console and we had another thread selected
+  // in the LLDB library, we would show the wrong thing to the user. If the
+  // users switches threads with a lldb command like "`thread select 14", the
+  // GUI will not update as there are no "event" notification packets that
+  // allow us to change the currently selected thread or frame in the GUI that
+  // I am aware of.
+  if (frame.IsValid()) {
+frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
+frame.GetThread().SetSelectedFrame(frame.GetFrameID());
+  }
   g_vsc.variables.Clear();
   g_vsc.variables.Append(frame.GetVariables(true,   // arguments
 true,   // locals
Index: lldb/test/API/tools/lldb-vscode/console/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/console/main.cpp
@@ -0,0 +1,9 @@
+
+int multiply(int x, int y) {
+  return x * y; // breakpoint 1
+}
+
+int main(int argc, char const *argv[]) {
+  int result = multiply(argc, 20);
+  return result < 0;
+}
Index: lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py
@@ -0,0 +1,70 @@
+"""
+Test lldb-vscode setBreakpoints request
+"""
+
+from __future__ import print_function
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+
+
+class TestVSCode_console(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def check_lldb_command(self, lldb_command, contains_string, assert_msg):
+response = self.vscode.request_evaluate('`%s' % (lldb_command))
+output = response['body']['result']
+self.assertTrue(contains_string in output,
+("""Verify %s by checking the command output:\n"""
+ """'''\n%s'''\nfor the string: "%s" """ % (
+ assert_msg, output, contains_string)))
+
+@skipIfWindows
+@skipIfRemote
+def test_scopes_variables_setVariable_evaluate(self):
+'''
+Tests that the "scopes" request causes the currently selected
+thread and frame to be updated. There are no DAP packets that tell
+lldb-vscode which thread and frame are selected other than the
+"scopes" request. lldb-vscode will now select the thread and frame
+for the latest "scopes" request that it receives.
+
+The LLDB command interpreter need

[Lldb-commits] [PATCH] D77347: Have lldb-vscode update the currently selecte thread and frame when it receives a "scopes" request.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

commit 5998aceda9fdca04db4d9ee390cec660896bf0bf 
 (HEAD -> 
master, origin/master, origin/HEAD)
Author: Greg Clayton 
Date:   Thu Apr 2 16:30:33 2020 -0700

  Have lldb-vscode update the currently selecte thread and frame when it 
receives a "scopes" request.
  
  Summary: The IDE has no packets that are sent to lldb-vscode that say which 
thread and frame are selected. The only way we know is we get a request for 
variables for a stack frame via a "scopes" request. When we receive this packet 
we make that thread and frame the selected thread and frame in lldb. This way 
when people execute lldb commands in the debug console by prefixing the 
expression with the backtick character, we will have the right thread and frame 
selected. Previously this was not updated as new stack frames were selected.
  
  Reviewers: labath, aadsm, wallace, JDevlieghere
  
  Subscribers: lldb-commits
  
  Tags: #lldb
  
  Differential Revision: https://reviews.llvm.org/D77347


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77347



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

[Lldb-commits] [PATCH] D76471: Remap the target SDK directory to the host SDK directory

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

> Yeah, it seems acceptable. The "sdk type" argument still seems somewhat 
> redundant. Theoretically we could pass in just the triple component out the 
> xcodesdk object, but that would just mean the individual platforms would have 
> to reconstruct the XcodeSDK object, which is already present in the caller, 
> so it's not ideal either...

It's an entire string, not just the type, because it also contains an 
(optional) version number.




Comment at: lldb/include/lldb/Target/Platform.h:437
 
+  virtual ConstString GetSDKPath(int type) { return {}; }
+

labath wrote:
> GetXCodeSDKPath? (and maybe the argument can be the proper enum type now.
That would look clean, but unfortunately it has to be a full XcodeSDK string, 
because in addition to the type, it optionally also contains a version number 
and some other bits that are applicable only when developing the OS itself.


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

https://reviews.llvm.org/D76471



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

[Lldb-commits] [PATCH] D76471: Remap the target SDK directory to the host SDK directory

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 254696.
aprantl marked 7 inline comments as done.
aprantl added a comment.

Renamed SDK -> XcodeSDK where applicable. Sorry I missed this earlier.


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

https://reviews.llvm.org/D76471

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Host/HostInfoBase.h
  lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Utility/XcodeSDK.h
  lldb/source/Core/Module.cpp
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.h
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
  lldb/source/Plugins/Platform/MacOSX/PlatformiOSSimulator.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/XcodeSDK.cpp
  lldb/unittests/Platform/PlatformDarwinTest.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/XcodeSDKTest.cpp

Index: lldb/unittests/Utility/XcodeSDKTest.cpp
===
--- /dev/null
+++ lldb/unittests/Utility/XcodeSDKTest.cpp
@@ -0,0 +1,84 @@
+//===-- XcodeSDKTest.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/XcodeSDK.h"
+
+#include "llvm/ADT/StringRef.h"
+
+#include 
+
+using namespace lldb_private;
+
+TEST(XcodeSDKTest, ParseTest) {
+  EXPECT_EQ(XcodeSDK::GetAnyMacOS().GetType(), XcodeSDK::MacOSX);
+  EXPECT_EQ(XcodeSDK("MacOSX.sdk").GetType(), XcodeSDK::MacOSX);
+  EXPECT_EQ(XcodeSDK("iPhoneSimulator.sdk").GetType(), XcodeSDK::iPhoneSimulator);
+  EXPECT_EQ(XcodeSDK("iPhoneOS.sdk").GetType(), XcodeSDK::iPhoneOS);
+  EXPECT_EQ(XcodeSDK("AppleTVSimulator.sdk").GetType(), XcodeSDK::AppleTVSimulator);
+  EXPECT_EQ(XcodeSDK("AppleTVOS.sdk").GetType(), XcodeSDK::AppleTVOS);
+  EXPECT_EQ(XcodeSDK("WatchSimulator.sdk").GetType(), XcodeSDK::WatchSimulator);
+  EXPECT_EQ(XcodeSDK("WatchOS.sdk").GetType(), XcodeSDK::watchOS);
+  EXPECT_EQ(XcodeSDK("Linux.sdk").GetType(), XcodeSDK::Linux);
+  EXPECT_EQ(XcodeSDK("MacOSX.sdk").GetVersion(), llvm::VersionTuple());
+  EXPECT_EQ(XcodeSDK("MacOSX10.9.sdk").GetVersion(), llvm::VersionTuple(10, 9));
+  EXPECT_EQ(XcodeSDK("MacOSX10.15.4.sdk").GetVersion(), llvm::VersionTuple(10, 15));
+}
+
+TEST(XcodeSDKTest, MergeTest) {
+  XcodeSDK sdk("MacOSX.sdk");
+  sdk.Merge(XcodeSDK("WatchOS.sdk"));
+  // This doesn't make any particular sense and shouldn't happen in practice, we
+  // just want to guarantee a well-defined behavior when choosing one
+  // SDK to fit all CUs in an lldb::Module.
+  // -> The higher number wins.
+  EXPECT_EQ(sdk.GetType(), XcodeSDK::watchOS);
+  sdk.Merge(XcodeSDK("WatchOS1.1.sdk"));
+  EXPECT_EQ(sdk.GetVersion(), llvm::VersionTuple(1, 1));
+  sdk.Merge(XcodeSDK("WatchOS2.0.sdk"));
+  EXPECT_EQ(sdk.GetVersion(), llvm::VersionTuple(2, 0));
+}
+
+TEST(XcodeSDKTest, SDKSupportsModules) {
+  std::string base = "/Applications/Xcode.app/Contents/Developer/Platforms/";
+  EXPECT_TRUE(XcodeSDK::SDKSupportsModules(
+  XcodeSDK::Type::iPhoneSimulator,
+  FileSpec(
+  base +
+  "iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator12.0.sdk")));
+  EXPECT_FALSE(XcodeSDK::SDKSupportsModules(
+  XcodeSDK::Type::iPhoneSimulator,
+  FileSpec(
+  base +
+  "iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator7.2.sdk")));
+  EXPECT_TRUE(XcodeSDK::SDKSupportsModules(
+  XcodeSDK::Type::MacOSX,
+  FileSpec(base + "MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk")));
+  EXPECT_FALSE(XcodeSDK::SDKSupportsModules(
+  XcodeSDK::Type::MacOSX,
+  FileSpec(base + "MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk")));
+}
+
+TEST(XcodeSDKTest, GetSDKNameForType) {
+  EXPECT_EQ("macosx", XcodeSDK::GetSDKNameForType(XcodeSDK::Type::MacOSX));
+  EXPECT_EQ("iphonesimulator",
+XcodeSDK::GetSDKNameForType(XcodeSDK::Type::iPhoneSimulator));
+  EXPECT_EQ("iphoneos", XcodeSDK::GetSDKNameForType(XcodeSDK::Type::iPhoneOS));
+  EXPECT_EQ("appletvsimulator",
+XcodeSDK::GetSDKNameForType(XcodeSDK::Type::AppleTVSimulator));
+  EXPECT_EQ("appletvos",
+XcodeSDK::GetSDKNameForType(XcodeSDK::Type::AppleTVOS));
+  EX

[Lldb-commits] [PATCH] D77369: Augment the symbol type classification for dyld trie exports with more detail

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added reviewers: labath, clayborg.
jasonmolenda added a project: LLDB.

My patch to augment the symbol table in Mach-O files with the dyld trie exports 
data structure only categorized symbols as code or data, but Greg Clayton had a 
patch to do something similar to swift a few years ago that had a more 
extensive categorization of symbols, as well as extracting some objc class/ivar 
names from the entries. This patch is basically just Greg's, updated a bit and 
with a test case added to it.

I'm most interested for any better ideas for a test where I get two symbols by 
a name search, and want to verify that one symbol has an objc type and the 
other symbol has a second objc type, it doesn't matter the order of the symbols 
returned.  The test case addition works, but it's not pretty - I was trying to 
think of a better way to do this but didn't come up with anything in the moment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77369

Files:
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/test/API/macosx/dyld-trie-symbols/Makefile
  lldb/test/API/macosx/dyld-trie-symbols/TestDyldTrieSymbols.py
  lldb/test/API/macosx/dyld-trie-symbols/main.cpp
  lldb/test/API/macosx/dyld-trie-symbols/main.mm

Index: lldb/test/API/macosx/dyld-trie-symbols/main.mm
===
--- /dev/null
+++ lldb/test/API/macosx/dyld-trie-symbols/main.mm
@@ -0,0 +1,149 @@
+#import 
+
+// SourceBase will be the base class of Source.  We'll pass a Source object into a
+// function as a SourceBase, and then see if the dynamic typing can get us through the KVO
+// goo and all the way back to Source.
+
+@interface SourceBase: NSObject
+{
+uint32_t _value;
+}
+- (SourceBase *) init;
+- (uint32_t) getValue;
+@end
+
+@implementation SourceBase
+- (SourceBase *) init
+{
+[super init];
+_value = 10;
+return self;
+}
+- (uint32_t) getValue
+{
+return _value;
+}
+@end
+
+// Source is a class that will be observed by the Observer class below.
+// When Observer sets itself up to observe this property (in initWithASource)
+// the KVO system will overwrite the "isa" pointer of the object with the "kvo'ed" 
+// one.
+
+@interface Source : SourceBase
+{
+int _property;
+}
+- (Source *) init;
+- (void) setProperty: (int) newValue;
+@end
+
+@implementation Source
+- (Source *) init
+{
+[super init];
+_property = 20;
+return self;
+}
+- (void) setProperty: (int) newValue
+{
+_property = newValue;  // This is the line in setProperty, make sure we step to here.
+}
+@end
+
+@interface SourceDerived : Source
+{
+int _derivedValue;
+}
+- (SourceDerived *) init;
+- (uint32_t) getValue;
+@end
+
+@implementation SourceDerived
+- (SourceDerived *) init
+{
+[super init];
+_derivedValue = 30;
+return self;
+}
+- (uint32_t) getValue
+{
+return _derivedValue;
+}
+@end
+
+// Observer is the object that will watch Source and cause KVO to swizzle it...
+
+@interface Observer : NSObject
+{
+Source *_source;
+}
++ (Observer *) observerWithSource: (Source *) source;
+- (Observer *) initWithASource: (Source *) source;
+- (void) observeValueForKeyPath: (NSString *) path 
+		   ofObject: (id) object
+			 change: (NSDictionary *) change
+			context: (void *) context;
+@end
+
+@implementation Observer
+
++ (Observer *) observerWithSource: (Source *) inSource;
+{
+Observer *retval;
+
+retval = [[Observer alloc] initWithASource: inSource];
+return retval;
+}
+
+- (Observer *) initWithASource: (Source *) source
+{
+[super init];
+_source = source;
+[_source addObserver: self 
+	forKeyPath: @"property" 
+	options: (NSKeyValueObservingOptionNew | NSKeyValueObservingOptionOld)
+	context: NULL];
+return self;
+}
+
+- (void) observeValueForKeyPath: (NSString *) path 
+		   ofObject: (id) object
+			 change: (NSDictionary *) change
+			context: (void *) context
+{
+printf ("Observer function called.\n");
+return;
+}
+@end
+
+
+int patval;  // external symbol, will not be completely stripped
+int pat(int in) {// external symbol, will not be completely stripped
+  if (patval == 0)
+patval = in;
+  return patval;
+}
+
+static int fooval;  // static symbol, stripped
+int foo() { // external symbol, will not be completely stripped
+  if (fooval == 0)
+fooval = 5;
+  return fooval;
+}
+
+int bazval = 10;   // external symbol, will not be completely stripped
+int baz () {   // external symbol, will not be completely stripped
+  return foo() + bazval;
+}
+
+static int barval = 15; // static symbol, stripped
+static int bar () { // static symbol, stripped; __lldb_unnamed_symbol from func starts
+  return baz() + barval;
+}
+
+int calculate ()   // external symbol, will not be completely stripped
+{
+  return bar();
+}
+
Index: lldb/test/API/macosx/dyld-trie-symbols/main.cpp
=

Re: [Lldb-commits] [lldb] b78157c - [intel-pt] Implement a basic test case

[Pavel Labath via lldb-commits]

On 02/04/2020 20:37, Walter Erquinigo via lldb-commits wrote:
> - Now using a env var to determine whether to run the test, as
> someone might have built liblldbIntelFeatures.so without intelPT
> support, which would make this test fail.

That's a very nonstandard way of doing things. I don't think we should
be doing that.

Is there any way of checking this automatically (for example, by looking
at /proc/cpuinfo)? We could then have a test decorator or a test
category to annotate these tests with...

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

[Lldb-commits] [PATCH] D77337: [lldb/Symbol] Reimplement Symbols::FindSymbolFileInBundle to use the VFS

[Pavel Labath via Phabricator via lldb-commits]

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

looks fine, modulo the assert fail




Comment at: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp:275
+  for (size_t i = 0; i < module_specs.GetSize(); ++i) {
+assert(module_specs.GetModuleSpecAtIndex(i, spec));
+if ((uuid == nullptr ||

this assert has side effects. that won't work in no-assert builds


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D77337



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

[Lldb-commits] [PATCH] D77324: [source maps] Fix remove, insert-after and replace

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Interpreter/OptionValuePathMappings.cpp:184-185
+  // Sort and then erase in reverse so indexes are always valid
+  llvm::sort(remove_indexes.rbegin(), remove_indexes.rend());
+  for (auto index : remove_indexes)
+m_path_mappings.Remove(index, m_notify_changes);

sorting on reverse iterators causes some head-scratching. I think it would be 
simpler to just sort normally, and then iterate in reverse (`for (int /*not 
auto*/ index: llvm::reverse(remove_indexes)) ...`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77324



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