[Lldb-commits] [lldb] ee4dc98 - [lldb/test] Remove skip arm/aarch64 decorator from instruction counting tests

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2020-03-09T12:54:42+05:00
New Revision: ee4dc980c031d00ba729dfd731808b214e01ee58

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

LOG: [lldb/test] Remove skip arm/aarch64 decorator from instruction counting 
tests

This patch removes skipIf decorator from instruction counting tests.
We now use inline intruction in testing inferior to make sure that
number of instructions stays fixed. This was tested on aarch64 linux.

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemote_vCont.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemote_vCont.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemote_vCont.py
index c06c0ca7cba5..758b0bc72fc6 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemote_vCont.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemote_vCont.py
@@ -106,15 +106,6 @@ def 
test_single_step_only_steps_one_instruction_with_Hc_vCont_s_debugserver(
 
 @skipIfWindows # No pty support to test O* & I* notification packets.
 @llgs_test
-@expectedFailureAndroid(
-bugnumber="llvm.org/pr24739",
-archs=[
-"arm",
-"aarch64"])
-@skipIf(
-oslist=["linux"],
-archs=["arm","aarch64"],
-bugnumber="llvm.org/pr24739")
 @skipIf(triple='^mips')
 @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], 
bugnumber="rdar://27005337")
 def test_single_step_only_steps_one_instruction_with_Hc_vCont_s_llgs(self):
@@ -136,15 +127,6 @@ def 
test_single_step_only_steps_one_instruction_with_vCont_s_thread_debugserver(
 
 @skipIfWindows # No pty support to test O* & I* notification packets.
 @llgs_test
-@expectedFailureAndroid(
-bugnumber="llvm.org/pr24739",
-archs=[
-"arm",
-"aarch64"])
-@skipIf(
-oslist=["linux"],
-archs=["arm","aarch64"],
-bugnumber="llvm.org/pr24739")
 @skipIf(triple='^mips')
 @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], 
bugnumber="rdar://27005337")
 def test_single_step_only_steps_one_instruction_with_vCont_s_thread_llgs(



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

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Host/DebugInfoD.h:26
+
+llvm::Error findSource(UUID buildID, const std::string &path,
+   std::string &result_path);

Expected ?



Comment at: lldb/source/Host/common/DebugInfoD.cpp:43-67
+UUID getBuildIDFromModule(const ModuleSP &module) {
+  UUID buildID;
+
+  if (!module)
+return buildID;
+
+  const FileSpec &moduleFileSpec = module->GetFileSpec();

How is all this different from `module->GetUUID()` ?



Comment at: lldb/source/Host/common/DebugInfoD.cpp:97
+   "debuginfod_find_source query failed: %s",
+   strerror(-rc));
+

llvm::sys::StrError(-rc)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D75555: [GlobalISel][Localizer] Enable intra-block localization of already-local uses.

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D7#1909194 , @labath wrote:

> Yep, that test really shouldn't be doing that. Historically, lldb has been 
> avoiding architecture specific artifacts (like assembly) in its tests, but 
> that didn't really work out here. That test has become a nightmare of 
> architecture-specific assertions.
>
> If we're going to be testing assembly-level properties, we really should be 
> using assembly inputs. I have now rewritten it to use inline assembly to 
> guarantee a known sequence of instructions. I think the arm assembly is 
> correct (it compiles), but I don't have the hardware to try it on.
>
> @omjavaid, if you're able to test that locally, can you remove the skip 
> decorator and give this a spin? Otherwise, I can just remove the decorator 
> and watch the bot...


Hi @labath  thanks for the quick fix moving to inline assembly fixed this on 
AArch64 linux. I have removed skipIf decorators now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D7



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

[Lldb-commits] [PATCH] D75761: Fix to get the AST we generate for function templates to be closer to what clang generates and expects

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It's a pity that the clang's DW_AT_name value is so ambiguous. For example, gcc 
would output the name in the commit message as `operator< `, which is a 
lot more parsable (for computers and people). Have you looked at changing 
clang's output to make it more like gcc's ? I know it would only help new 
compilers, but maybe for such a special case, this does not matter?

Or, even if we do end up adding some compat parsing code, that would reduce the 
need for it to be super exact. I'm pretty sure that the current code does not 
handle all cases correctly. E.g., I believe it will break on something like 
`operator<<<&operator>> >` (mangled name `_ZlsIXadL_Zrs1AS0_EEEvS0_S0_`, 
godbolt link ).

As an alternative, we could try extracting the same information from the 
mangled name (via llvm::ItaniumPartialDemangler::getFunctionBaseName), at least 
when DW_AT_linkage_name is present (not all compilers emit that attribute but I 
am not sure if anyone has tested if lldb actually works without it).




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:839
+  size_t RightAngleCount = Name.count('>');
+  size_t LeftAngleCount = Name.count('<');
+

aprantl wrote:
> We are scanning the entire string multiple times here and that seems 
> unnecessarily expensive since C++ template function names get looong. I think 
> we could do this as a single for-loop with a state machine instead, without 
> being too difficult to read?
Would it be sufficient to always strip one `<` from the first sequence of `<`'s 
that you find? You can't have two template `<` one after the other, so in a 
valid name, the last `<` will be the template angle bracket, while everything 
else must be a part of the operator name.

So, something like `return {Name.data(), Name.find_first_not_of('<', 
Name.find_first_of('<')) -1}` (with some error checks and an explicit check for 
the spaceship operator).



Comment at: lldb/test/API/lang/cpp/template-function/main.cpp:1
 template
 int foo(T t1) {

Do we have a test for the spaceship operator?


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

https://reviews.llvm.org/D75761



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

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 249047.
kwk marked 3 inline comments as done.
kwk added a comment.

Changes suggested by elfutils maintainers:

- Silently ignore error when no DEBUGINFOD_URLS was given as an environment 
variable (ENOSYS).
- Silently ignore error when the build ID could not be found on any server 
(ENOENT).
- End debuginfod client before dealing with return code.

Applied review comments from labath:

- Removed getBuildIDFromModule because we have Module->GetUUID()
- Make findSource return an llvm::Expected instead of an error
- Various formatting issues with clang-format
- use llvm::sys::StrError instead of strerror directly

Other changes:

- Comments on functions in lldb_privat::debuginfod


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp

Index: lldb/source/Host/common/DebugInfoD.cpp
===
--- /dev/null
+++ lldb/source/Host/common/DebugInfoD.cpp
@@ -0,0 +1,96 @@
+//===-- DebugInfoD.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/Host/DebugInfoD.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Errno.h"
+
+#if LLDB_ENABLE_DEBUGINFOD
+#include "elfutils/debuginfod.h"
+#endif
+
+namespace lldb_private {
+
+namespace debuginfod {
+
+using namespace lldb;
+using namespace lldb_private;
+
+#if !LLDB_ENABLE_DEBUGINFOD
+bool isAvailable() { return false; }
+
+llvm::Error findSource(UUID buildID, const std::string &path,
+   std::string &cache_path) {
+  llvm_unreachable("debuginfod::findSource is unavailable");
+}
+
+#else // LLDB_ENABLE_DEBUGINFOD
+
+bool isAvailable() { return true; }
+
+llvm::Expected findSource(UUID buildID, const std::string &path) {
+  if (!buildID.IsValid())
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid build ID: %s",
+   buildID.GetAsString("").c_str());
+
+  debuginfod_client *client = debuginfod_begin();
+
+  if (!client)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to create debuginfod connection handle: %s", strerror(errno));
+
+  // debuginfod_set_progressfn(client, [](debuginfod_client *client, long a,
+  // long b) -> int {
+  //   fprintf(stderr, "KWK === a: %ld b : %ld \n", a, b);
+  //   return 0; // continue
+  // });
+
+  char *cache_path = nullptr;
+  int rc = debuginfod_find_source(client, buildID.GetBytes().data(),
+  buildID.GetBytes().size(), path.c_str(),
+  &cache_path);
+
+  debuginfod_end(client);
+
+  std::string result_path;
+  if (cache_path) {
+result_path = std::string(cache_path);
+free(cache_path);
+  }
+
+  if (rc < 0) {
+if (rc == -ENOSYS) // No DEBUGINFO_URLS were specified
+  return result_path;
+else if (rc == -ENOENT) // No such file or directory, aka build-id not
+// available on servers.
+  return result_path;
+else
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "debuginfod_find_source query failed: %s",
+ llvm::sys::StrError(-rc).c_str());
+  }
+
+  if (close(rc) < 0) {
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to close result of call to debuginfo_find_source: %s",
+llvm::sys::StrError(errno).c_str());
+  }
+
+  return result_path;
+}
+
+#endif // LLDB_ENABLE_DEBUGINFOD
+
+} // end of namespace debuginfod
+} // namespace lldb_private
Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -30,6 +30,7 @@
   common/HostThread.cpp
   common/LockFileBase.cpp
   common/LZMA.cpp
+  common/DebugInfoD.cpp
   common/MainLoop.cpp
   common/MonitoringProcessLauncher.cpp
   common/NativeProcessProtocol.cpp
@@ -161,6 +162,9 @@
 if (LLDB_ENABLE_LZMA)
   list(APPEND EXTRA_LIBS ${LIBLZMA_LIBRARIES})
 endif()
+if (LLDB_ENABLE_DEBUGINFOD)
+  list(APPEND EXTRA

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

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

@labath thank you for your early feedback. It was helpful even though this is 
still a work in progress.




Comment at: lldb/source/Host/common/DebugInfoD.cpp:43-67
+UUID getBuildIDFromModule(const ModuleSP &module) {
+  UUID buildID;
+
+  if (!module)
+return buildID;
+
+  const FileSpec &moduleFileSpec = module->GetFileSpec();

labath wrote:
> How is all this different from `module->GetUUID()` ?
I didn't know about that :) . Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for the detailed response. I'll try to be equally understandable.

In D75711#1910155 , @jingham wrote:

> In D75711#1909055 , @labath wrote:
>
> > I am somewhat worried about the direction this is taking. Maybe vanishing 
> > threads are fine for os-level debugging (because there the "real" threads 
> > are the cpu cores, and the os threads are somewhat imaginary), but I 
> > definitely wouldn't want to do it for regular debugging. If my process has 
> > 1000 threads (not completely uncommon) then I don't think my debugger would 
> > be doing me a service showing me just the three it thinks are interesting 
> > and completely disregarding the others. Having a mode (it could even be 
> > default) which only highlights the interesting ones -- yes, that would 
> > definitely be useful. Making sure it doesn't touch the other threads when 
> > it doesn't need to -- absolutely. But I don't think it should pretend the 
> > other threads don't exist at all, because I may still have a reason to 
> > inspect those threads (or just to know that they're there). In fact, I 
> > wouldn't be surprised if this happened to the said kernel folks too, and 
> > they end up adding a custom command, which would enumerate all "threads".
>
>
> First off, I also think that hiding some threads from users, or forcing 
> people to say "thread list --force" or something like that to see them all 
> would not be a user-friendly design.  I'm not suggesting imposing that on 
> general debugging.  But if an OS plugin, for instance, decides it is too 
> expensive to do this except as an explicit gesture, we need to support that 
> decision.
>
> But lldb stops many times during the course of a step where it doesn't return 
> control to the user - all the step into code with no debug info, step out 
> again, step across dynamic linker stubs or step through ObjC message dispatch 
> dances we do involve many private stops per public spot...  If a thread has 
> not stopped "for a reason" at one of those stops, it takes no part in the 
> "should stop" and "will resume" negotiations and reconstructing it for that 
> stop really does serve no purpose.  There's no way that the user will get a 
> chance to see it.
>
> So if we could avoid reading them in, that might make stepping go faster in 
> the presence of lots of threads.  BTW, with the accelerated stop packets 
> where we get all the threads and their stop reasons on stop, I'm not sure 
> this would really save all that much time.  But talking to less capable 
> stubs, it really can make a difference.  Greg did a bunch of work to make 
> sure stepping stopped as few times as possible because the Facebook 
> gdb-remote stub didn't support these accelerated packets and getting all the 
> thread info on each private stop was making stepping go really slowly.  I 
> didn't understand the motivation at first because for debugserver the changes 
> didn't really make any difference.
>
> Also, I don't think these two desires: not fetching threads when we don't 
> need them (e.g. private stops) and showing them all to the user whenever they 
> ask, are contradictory.  After all we still have the gate of 
> UpdateThreadListIfNeeded.  All the commands that touch the thread list go 
> through this. So even if we don't gather all the threads when we stop, we 
> already have a mechanism in place whereby any command that actually touches 
> the thread list can fetch them on demand.


Yes, I agree with everything above. If lldb does not need information about all 
threads in order to service a internal stop, then it shouldn't read it in, and 
the os plugin (or the remote stub) should not need to provide it. In fact I 
would even go so far as to say that we shouldn't need to read in this 
information for a public stop until a user performs an operation (e.g. `thread 
list`) that requires information about all threads.

> 
> 
>> What was actually the slow part here? Was it the os plugin reading the 
>> thread lists from the kernel memory? Or the fact that lldb is slow when it 
>> is confronted with them?
>> 
>> If it is the latter, then maybe we should fix that. For example, I think I 
>> remember lldb likes to check the PC of every thread before it does any kind 
>> of a resume -- I am not convinced that is really needed.
>> 
>> If fetching the threads is slow, then perhaps we could have an api, where 
>> the os threads are produced incrementally, and we ensure that lldb does not 
>> query for os threads needlessly during the hot paths (e.g. internal stops 
>> for servicing thread plans).
> 
> The thing that is slow is gathering all the threads.  On Darwin, there's a 
> kernel activation per user space thread and then a whole bunch of kernel 
> threads.  On a busy system, that ends up being a whole lot of threads.  
> Producing them through the OS plugin interface involves a bunch of memory 
> read

[Lldb-commits] [PATCH] D75810: [lldb] Add .clang-tidy with customization to readability-identifier-naming.{Function, Member, Parameter, Variable}Case

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Yeah, naming conventions in lldb have always been a controversial topic. Most 
of the code uses the lldb convention, but some of the new code (particularly if 
it interfaces with llvm a lot) uses more llvm-like conventions. I think we used 
to have a description of the naming convention somewhere on the website. That 
is no longer there, but neither do we have a plan for migration.

I don't really know what that means for this patch...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75810



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

[Lldb-commits] [PATCH] D75537: Clear all settings during a test's setUp

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Commands/CommandObjectSettings.cpp:1119-1122
+if (m_options.m_clear_all) {
+  GetDebugger().GetValueProperties()->Clear();
+  return result.Succeeded();
+}

What will happen if I pass both `--all` and an actual setting argument. I think 
that should be an error.



Comment at: lldb/test/API/commands/settings/TestSettings.py:545-548
+# Apply Setup commands again.
+for s in self.setUpCommands():
+self.runCmd(s)
+

This dependence on setUpCommands looks weird. Any way we can get rid of that? 
Maybe if we check the exact four settings that you modified before `clear 
--all` and verifying they have the correct (hardcoded) value? (and maybe not 
use the term-width setting, since the default value of that is unpredictable)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D75537



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

[Lldb-commits] [lldb] af3db4e - [lldb] Reduce duplication in the Disassembler class

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-03-09T13:41:43+01:00
New Revision: af3db4e9aa8fbe7e43f89cdde780c6acc35368be

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

LOG: [lldb] Reduce duplication in the Disassembler class

Summary:
The class has two pairs of functions whose functionalities differ in
only how one specifies how much he wants to disasseble. One limits the
process by the size of the input memory region. The other based on the
total amount of instructions disassembled. They also differ in various
features (like error reporting) that were only added to one of the
versions.

There are various ways in which this could be addressed. This patch
does it by introducing a helper struct called "Limit", which is
effectively a pair specifying the value that you want to limit, and the
actual limit itself.

Reviewers: JDevlieghere

Subscribers: sdardis, jrtc27, atanasyan, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/Core/Disassembler.h
lldb/source/Commands/CommandObjectDisassemble.cpp
lldb/source/Core/Disassembler.cpp
lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
lldb/source/Target/StackFrame.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Disassembler.h 
b/lldb/include/lldb/Core/Disassembler.h
index 145bed31a155..7ffdac74ccfc 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -384,6 +384,11 @@ class Disassembler : public 
std::enable_shared_from_this,
   const char *flavor,
   const char *plugin_name);
 
+  struct Limit {
+enum { Bytes, Instructions } kind;
+lldb::addr_t value;
+  };
+
   static lldb::DisassemblerSP
   DisassembleRange(const ArchSpec &arch, const char *plugin_name,
const char *flavor, Target &target,
@@ -395,19 +400,10 @@ class Disassembler : public 
std::enable_shared_from_this,
size_t length, uint32_t max_num_instructions,
bool data_from_file);
 
-  static bool Disassemble(Debugger &debugger, const ArchSpec &arch,
-  const char *plugin_name, const char *flavor,
-  const ExecutionContext &exe_ctx,
-  const AddressRange &range, uint32_t num_instructions,
-  bool mixed_source_and_assembly,
-  uint32_t num_mixed_context_lines, uint32_t options,
-  Stream &strm);
-
   static bool Disassemble(Debugger &debugger, const ArchSpec &arch,
   const char *plugin_name, const char *flavor,
   const ExecutionContext &exe_ctx, const Address 
&start,
-  uint32_t num_instructions,
-  bool mixed_source_and_assembly,
+  Limit limit, bool mixed_source_and_assembly,
   uint32_t num_mixed_context_lines, uint32_t options,
   Stream &strm);
 
@@ -423,17 +419,13 @@ class Disassembler : public 
std::enable_shared_from_this,
 
   void PrintInstructions(Debugger &debugger, const ArchSpec &arch,
  const ExecutionContext &exe_ctx,
- uint32_t num_instructions,
  bool mixed_source_and_assembly,
  uint32_t num_mixed_context_lines, uint32_t options,
  Stream &strm);
 
-  size_t ParseInstructions(Target &target, AddressRange range,
+  size_t ParseInstructions(Target &target, Address address, Limit limit,
Stream *error_strm_ptr, bool prefer_file_cache);
 
-  size_t ParseInstructions(Target &target, Address address,
-   uint32_t num_instructions, bool prefer_file_cache);
-
   virtual size_t DecodeInstructions(const Address &base_addr,
 const DataExtractor &data,
 lldb::offset_t data_offset,

diff  --git a/lldb/source/Commands/CommandObjectDisassemble.cpp 
b/lldb/source/Commands/CommandObjectDisassemble.cpp
index a11af68835d9..bd1c7a43afad 100644
--- a/lldb/source/Commands/CommandObjectDisassemble.cpp
+++ b/lldb/source/Commands/CommandObjectDisassemble.cpp
@@ -459,25 +459,19 @@ bool CommandObjectDisassemble::DoExecute(Args &command,
 
   bool print_sc_header = ranges.size() > 1;
   for (AddressRange cur_range : ranges) {
-bool success;
-if (m_options.num_instructions != 0) {
-  success = Disassembler::Disassemble(
-  GetDebugger(), m_options.arch, plugin_name, flavor_string, m_exe_ctx,
-  cur_range.GetBaseAddress(

[Lldb-commits] [PATCH] D75730: [lldb] Reduce duplication in the Disassembler class

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf3db4e9aa8f: [lldb] Reduce duplication in the Disassembler 
class (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D75730?vs=248676&id=249072#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75730

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
  lldb/source/Target/StackFrame.cpp

Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -1940,16 +1940,14 @@
   const uint32_t disasm_lines = debugger.GetDisassemblyLineCount();
   if (disasm_lines > 0) {
 const ArchSpec &target_arch = target->GetArchitecture();
-AddressRange pc_range;
-pc_range.GetBaseAddress() = GetFrameCodeAddress();
-pc_range.SetByteSize(disasm_lines *
- target_arch.GetMaximumOpcodeByteSize());
 const char *plugin_name = nullptr;
 const char *flavor = nullptr;
 const bool mixed_source_and_assembly = false;
 Disassembler::Disassemble(
 target->GetDebugger(), target_arch, plugin_name, flavor,
-exe_ctx, pc_range, disasm_lines, mixed_source_and_assembly, 0,
+exe_ctx, GetFrameCodeAddress(),
+{Disassembler::Limit::Instructions, disasm_lines},
+mixed_source_and_assembly, 0,
 Disassembler::eOptionMarkPCAddress, strm);
   }
 }
Index: lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
===
--- lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
+++ lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
@@ -168,10 +168,11 @@
   for (uint32_t i = 1; i <= loop_count; i++) {
 // Adjust the address to read from.
 addr.Slide(-2);
-AddressRange range(addr, i * 2);
 uint32_t insn_size = 0;
 
-disasm_sp->ParseInstructions(target, range, nullptr, prefer_file_cache);
+disasm_sp->ParseInstructions(target, addr,
+ {Disassembler::Limit::Bytes, i * 2}, nullptr,
+ prefer_file_cache);
 
 uint32_t num_insns = disasm_sp->GetInstructionList().GetSize();
 if (num_insns) {
Index: lldb/source/Core/Disassembler.cpp
===
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -137,8 +137,9 @@
   if (!disasm_sp)
 return {};
 
-  const size_t bytes_disassembled =
-  disasm_sp->ParseInstructions(target, range, nullptr, prefer_file_cache);
+  const size_t bytes_disassembled = disasm_sp->ParseInstructions(
+  target, range.GetBaseAddress(), {Limit::Bytes, range.GetByteSize()},
+  nullptr, prefer_file_cache);
   if (bytes_disassembled == 0)
 return {};
 
@@ -170,52 +171,25 @@
 bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch,
const char *plugin_name, const char *flavor,
const ExecutionContext &exe_ctx,
-   const AddressRange &range,
-   uint32_t num_instructions,
+   const Address &address, Limit limit,
bool mixed_source_and_assembly,
uint32_t num_mixed_context_lines,
uint32_t options, Stream &strm) {
-  if (!range.GetByteSize() || !exe_ctx.GetTargetPtr())
+  if (!exe_ctx.GetTargetPtr())
 return false;
 
   lldb::DisassemblerSP disasm_sp(Disassembler::FindPluginForTarget(
   exe_ctx.GetTargetRef(), arch, flavor, plugin_name));
-
   if (!disasm_sp)
 return false;
 
   const bool prefer_file_cache = false;
   size_t bytes_disassembled = disasm_sp->ParseInstructions(
-  exe_ctx.GetTargetRef(), range, &strm, prefer_file_cache);
+  exe_ctx.GetTargetRef(), address, limit, &strm, prefer_file_cache);
   if (bytes_disassembled == 0)
 return false;
 
-  disasm_sp->PrintInstructions(debugger, arch, exe_ctx, num_instructions,
-   mixed_source_and_assembly,
-   num_mixed_context_lines, options, strm);
-  return true;
-}
-
-bool Disassembler::Disassemble(
-Debugger &debugger, const ArchSpec &arch, const char *plugin_name,
-const char *flavor, const ExecutionContext &exe_ctx, const Address &address,
-uint32_t num_instructions, bool mixed_source_and_assembly,
-uint32_t num_mixed_context_lines, uint32_t options, Stream &strm) {
-  if

[Lldb-commits] [lldb] c0b1af6 - [lldb] Return Unwinder& from Thread::GetUnwinder

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-03-09T14:13:22+01:00
New Revision: c0b1af6878444f075a17d87f523bc6be3343db35

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

LOG: [lldb] Return Unwinder& from Thread::GetUnwinder

The function always returns a valid object. Let the return type reflect
that, and remove some null checks.

Added: 


Modified: 
lldb/include/lldb/Target/StackFrameList.h
lldb/include/lldb/Target/Thread.h
lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/Thread.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/StackFrameList.h 
b/lldb/include/lldb/Target/StackFrameList.h
index 44b389764bc6..be5e009e8a1e 100644
--- a/lldb/include/lldb/Target/StackFrameList.h
+++ b/lldb/include/lldb/Target/StackFrameList.h
@@ -94,7 +94,7 @@ class StackFrameList {
 
   void GetFramesUpTo(uint32_t end_idx);
 
-  void GetOnlyConcreteFramesUpTo(uint32_t end_idx, Unwind *unwinder);
+  void GetOnlyConcreteFramesUpTo(uint32_t end_idx, Unwind &unwinder);
 
   void SynthesizeTailCallFrames(StackFrame &next_frame);
 

diff  --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index 02e0d3369729..b0bc1b29eb78 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -1195,7 +1195,7 @@ class Thread : public 
std::enable_shared_from_this,
 
   typedef std::vector plan_stack;
 
-  virtual lldb_private::Unwind *GetUnwinder();
+  virtual Unwind &GetUnwinder();
 
   // Check to see whether the thread is still at the last breakpoint hit that
   // stopped it.

diff  --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp 
b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
index 9016e14f93b6..7469e7633e71 100644
--- a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
+++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
@@ -54,20 +54,14 @@ RegisterContextSP ThreadMemory::GetRegisterContext() {
 
 RegisterContextSP
 ThreadMemory::CreateRegisterContextForFrame(StackFrame *frame) {
-  RegisterContextSP reg_ctx_sp;
   uint32_t concrete_frame_idx = 0;
 
   if (frame)
 concrete_frame_idx = frame->GetConcreteFrameIndex();
 
-  if (concrete_frame_idx == 0) {
-reg_ctx_sp = GetRegisterContext();
-  } else {
-Unwind *unwinder = GetUnwinder();
-if (unwinder != nullptr)
-  reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame);
-  }
-  return reg_ctx_sp;
+  if (concrete_frame_idx == 0)
+return GetRegisterContext();
+  return GetUnwinder().CreateRegisterContextForFrame(frame);
 }
 
 bool ThreadMemory::CalculateStopInfo() {

diff  --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp 
b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
index c3ae95a13b31..714090459cbb 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -229,9 +229,7 @@ ThreadElfCore::CreateRegisterContextForFrame(StackFrame 
*frame) {
 
 reg_ctx_sp = m_thread_reg_ctx_sp;
   } else {
-Unwind *unwinder = GetUnwinder();
-if (unwinder != nullptr)
-  reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame);
+reg_ctx_sp = GetUnwinder().CreateRegisterContextForFrame(frame);
   }
   return reg_ctx_sp;
 }

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
index 7615ae290d0d..6deabf8d5d71 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -311,9 +311,7 @@ ThreadGDBRemote::CreateRegisterContextForFrame(StackFrame 
*frame) {
   read_all_registers_at_once, write_all_registers_at_once);
 }
   } else {
-Unwind *unwinder = GetUnwinder();
-if (unwinder != nullptr)
-  reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame);
+reg_ctx_sp = GetUnwinder().CreateRegisterContextForFrame(frame);
   }
   return reg_ctx_sp;
 }

diff  --git a/lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp 
b/lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
index 4c3d2a5004a5..1950baea4127 100644
--- a/lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
@@ -83,9 +83,7 @@ ThreadMachCore::CreateRegisterContextForFrame(StackFrame 
*frame) {
 }
 reg_ctx_sp = m_thread_reg_ctx_sp;
   } else {
-Unwind *unwinder = GetUnwinder();
-if (unwinder != nullptr)
-  reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame);
+

[Lldb-commits] [lldb] 24b1831 - [lldb] Fix windows&freebsd builds for c0b1af68

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-03-09T14:55:43+01:00
New Revision: 24b1831ebfb50a2ded01506049d6c1c2f34c4a27

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

LOG: [lldb] Fix windows&freebsd builds for c0b1af68

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp
lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.h
lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp 
b/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp
index 7d6ecf0b445a..c01ab750b948 100644
--- a/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp
@@ -26,7 +26,6 @@
 #include "Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.h"
 #include "Plugins/Process/Utility/RegisterInfoPOSIX_arm.h"
 #include "Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h"
-#include "Plugins/Process/Utility/UnwindLLDB.h"
 #include "ProcessFreeBSD.h"
 #include "ProcessMonitor.h"
 #include "RegisterContextPOSIXProcessMonitor_arm.h"
@@ -254,8 +253,7 @@ 
FreeBSDThread::CreateRegisterContextForFrame(lldb_private::StackFrame *frame) {
   if (concrete_frame_idx == 0)
 reg_ctx_sp = GetRegisterContext();
   else {
-assert(GetUnwinder());
-reg_ctx_sp = GetUnwinder()->CreateRegisterContextForFrame(frame);
+reg_ctx_sp = GetUnwinder().CreateRegisterContextForFrame(frame);
   }
 
   return reg_ctx_sp;
@@ -275,13 +273,6 @@ bool FreeBSDThread::CalculateStopInfo() {
   return true;
 }
 
-Unwind *FreeBSDThread::GetUnwinder() {
-  if (!m_unwinder_up)
-m_unwinder_up.reset(new UnwindLLDB(*this));
-
-  return m_unwinder_up.get();
-}
-
 void FreeBSDThread::DidStop() {
   // Don't set the thread state to stopped unless we really stopped.
 }

diff  --git a/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.h 
b/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.h
index 6d3c253a519e..774ffb511bc6 100644
--- a/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.h
+++ b/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.h
@@ -102,8 +102,6 @@ class FreeBSDThread : public lldb_private::Thread {
   void ExitNotify(const ProcessMessage &message);
   void ExecNotify(const ProcessMessage &message);
 
-  lldb_private::Unwind *GetUnwinder() override;
-
   // FreeBSDThread internal API.
 
   // POSIXThread override

diff  --git 
a/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp 
b/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
index 8e700ced97e5..782edfe68279 100644
--- a/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
@@ -15,7 +15,6 @@
 #include "lldb/Utility/Logging.h"
 #include "lldb/Utility/State.h"
 
-#include "Plugins/Process/Utility/UnwindLLDB.h"
 #include "ProcessWindows.h"
 #include "ProcessWindowsLog.h"
 #include "TargetThreadWindows.h"
@@ -113,9 +112,7 @@ 
TargetThreadWindows::CreateRegisterContextForFrame(StackFrame *frame) {
 }
 reg_ctx_sp = m_thread_reg_ctx_sp;
   } else {
-Unwind *unwinder = GetUnwinder();
-if (unwinder != nullptr)
-  reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame);
+reg_ctx_sp = GetUnwinder().CreateRegisterContextForFrame(frame);
   }
 
   return reg_ctx_sp;
@@ -126,14 +123,6 @@ bool TargetThreadWindows::CalculateStopInfo() {
   return true;
 }
 
-Unwind *TargetThreadWindows::GetUnwinder() {
-  // FIXME: Implement an unwinder based on the Windows unwinder exposed through
-  // DIA SDK.
-  if (!m_unwinder_up)
-m_unwinder_up.reset(new UnwindLLDB(*this));
-  return m_unwinder_up.get();
-}
-
 Status TargetThreadWindows::DoResume() {
   StateType resume_state = GetTemporaryResumeState();
   StateType current_state = GetState();

diff  --git a/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.h 
b/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.h
index fc68cb73db91..2845847738f6 100644
--- a/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.h
+++ b/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.h
@@ -34,7 +34,6 @@ class TargetThreadWindows : public lldb_private::Thread {
   lldb::RegisterContextSP
   CreateRegisterContextForFrame(StackFrame *frame) override;
   bool CalculateStopInfo() override;
-  Unwind *GetUnwinder() override;
 
   Status DoResume();
 



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

[Lldb-commits] [PATCH] D75848: [lldb] Make UnwindLLDB a non-plugin

[Pavel Labath via Phabricator via lldb-commits]

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

This is the only real unwinder, and things have been this way for quite
a long time. At this point, the class has accumulated so many features
it is unlikely that anyone will want to reimplement the whole thing.

The class is also fairly closely coupled (through UnwindPlans and
FuncUnwinders) with a lot of other lldb components that it is hard to
imagine a different unwinder implementation being substantially
different without reimplementing all of those.

The existing unwinding functionality is nonetheless fairly complex and
there is space for adding more structure to it, but I believe a more
worthwhile effort would be to take the existing UnwindLLDB class and try
to break it down and introduce extension/customization points, instead
of writing a brand new Unwind implementation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75848

Files:
  lldb/include/lldb/Target/RegisterContextUnwind.h
  lldb/include/lldb/Target/UnwindLLDB.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextLLDB.h
  lldb/source/Plugins/Process/Utility/UnwindLLDB.cpp
  lldb/source/Plugins/Process/Utility/UnwindLLDB.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Target/UnwindLLDB.cpp

Index: lldb/source/Target/UnwindLLDB.cpp
===
--- lldb/source/Target/UnwindLLDB.cpp
+++ lldb/source/Target/UnwindLLDB.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 
+#include "lldb/Target/UnwindLLDB.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/FuncUnwinders.h"
 #include "lldb/Symbol/Function.h"
@@ -13,13 +14,11 @@
 #include "lldb/Target/ABI.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/RegisterContextUnwind.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/Log.h"
 
-#include "RegisterContextLLDB.h"
-#include "UnwindLLDB.h"
-
 using namespace lldb;
 using namespace lldb_private;
 
@@ -77,7 +76,7 @@
 
   // First, set up the 0th (initial) frame
   CursorSP first_cursor_sp(new Cursor());
-  RegisterContextLLDBSP reg_ctx_sp(new RegisterContextLLDB(
+  RegisterContextLLDBSP reg_ctx_sp(new RegisterContextUnwind(
   m_thread, RegisterContextLLDBSP(), first_cursor_sp->sctx, 0, *this));
   if (reg_ctx_sp.get() == nullptr)
 goto unwind_done;
@@ -126,7 +125,7 @@
   uint32_t cur_idx = m_frames.size();
 
   CursorSP cursor_sp(new Cursor());
-  RegisterContextLLDBSP reg_ctx_sp(new RegisterContextLLDB(
+  RegisterContextLLDBSP reg_ctx_sp(new RegisterContextUnwind(
   m_thread, prev_frame->reg_ctx_lldb_sp, cursor_sp->sctx, cur_idx, *this));
 
   uint64_t max_stack_depth = m_thread.GetMaxBacktraceDepth();
@@ -148,7 +147,7 @@
   }
 
   if (reg_ctx_sp.get() == nullptr) {
-// If the RegisterContextLLDB has a fallback UnwindPlan, it will switch to
+// If the RegisterContextUnwind has a fallback UnwindPlan, it will switch to
 // that and return true.  Subsequent calls to TryFallbackUnwindPlan() will
 // return false.
 if (prev_frame->reg_ctx_lldb_sp->TryFallbackUnwindPlan()) {
@@ -187,7 +186,7 @@
 return nullptr;
   }
   if (!reg_ctx_sp->GetCFA(cursor_sp->cfa)) {
-// If the RegisterContextLLDB has a fallback UnwindPlan, it will switch to
+// If the RegisterContextUnwind has a fallback UnwindPlan, it will switch to
 // that and return true.  Subsequent calls to TryFallbackUnwindPlan() will
 // return false.
 if (prev_frame->reg_ctx_lldb_sp->TryFallbackUnwindPlan()) {
@@ -243,7 +242,7 @@
 }
   }
   if (!reg_ctx_sp->ReadPC(cursor_sp->start_pc)) {
-// If the RegisterContextLLDB has a fallback UnwindPlan, it will switch to
+// If the RegisterContextUnwind has a fallback UnwindPlan, it will switch to
 // that and return true.  Subsequent calls to TryFallbackUnwindPlan() will
 // return false.
 if (prev_frame->reg_ctx_lldb_sp->TryFallbackUnwindPlan()) {
@@ -262,7 +261,7 @@
 return nullptr;
   }
   if (abi && !abi->CodeAddressIsValid(cursor_sp->start_pc)) {
-// If the RegisterContextLLDB has a fallback UnwindPlan, it will switch to
+// If the RegisterContextUnwind has a fallback UnwindPlan, it will switch to
 // that and return true.  Subsequent calls to TryFallbackUnwindPlan() will
 // return false.
 if (prev_frame->reg_ctx_lldb_sp->TryFallbackUnwindPlan()) {
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -7,7 +7,6 @@
 //===---

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 249096.
kwk added a comment.

- Fix include ordering based on clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp

Index: lldb/source/Host/common/DebugInfoD.cpp
===
--- /dev/null
+++ lldb/source/Host/common/DebugInfoD.cpp
@@ -0,0 +1,96 @@
+//===-- DebugInfoD.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/Host/DebugInfoD.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Errno.h"
+
+#if LLDB_ENABLE_DEBUGINFOD
+#include "elfutils/debuginfod.h"
+#endif
+
+namespace lldb_private {
+
+namespace debuginfod {
+
+using namespace lldb;
+using namespace lldb_private;
+
+#if !LLDB_ENABLE_DEBUGINFOD
+bool isAvailable() { return false; }
+
+llvm::Error findSource(UUID buildID, const std::string &path,
+   std::string &cache_path) {
+  llvm_unreachable("debuginfod::findSource is unavailable");
+}
+
+#else // LLDB_ENABLE_DEBUGINFOD
+
+bool isAvailable() { return true; }
+
+llvm::Expected findSource(UUID buildID, const std::string &path) {
+  if (!buildID.IsValid())
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid build ID: %s",
+   buildID.GetAsString("").c_str());
+
+  debuginfod_client *client = debuginfod_begin();
+
+  if (!client)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to create debuginfod connection handle: %s", strerror(errno));
+
+  // debuginfod_set_progressfn(client, [](debuginfod_client *client, long a,
+  // long b) -> int {
+  //   fprintf(stderr, "KWK === a: %ld b : %ld \n", a, b);
+  //   return 0; // continue
+  // });
+
+  char *cache_path = nullptr;
+  int rc = debuginfod_find_source(client, buildID.GetBytes().data(),
+  buildID.GetBytes().size(), path.c_str(),
+  &cache_path);
+
+  debuginfod_end(client);
+
+  std::string result_path;
+  if (cache_path) {
+result_path = std::string(cache_path);
+free(cache_path);
+  }
+
+  if (rc < 0) {
+if (rc == -ENOSYS) // No DEBUGINFO_URLS were specified
+  return result_path;
+else if (rc == -ENOENT) // No such file or directory, aka build-id not
+// available on servers.
+  return result_path;
+else
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "debuginfod_find_source query failed: %s",
+ llvm::sys::StrError(-rc).c_str());
+  }
+
+  if (close(rc) < 0) {
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to close result of call to debuginfo_find_source: %s",
+llvm::sys::StrError(errno).c_str());
+  }
+
+  return result_path;
+}
+
+#endif // LLDB_ENABLE_DEBUGINFOD
+
+} // end of namespace debuginfod
+} // namespace lldb_private
Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -30,6 +30,7 @@
   common/HostThread.cpp
   common/LockFileBase.cpp
   common/LZMA.cpp
+  common/DebugInfoD.cpp
   common/MainLoop.cpp
   common/MonitoringProcessLauncher.cpp
   common/NativeProcessProtocol.cpp
@@ -161,6 +162,9 @@
 if (LLDB_ENABLE_LZMA)
   list(APPEND EXTRA_LIBS ${LIBLZMA_LIBRARIES})
 endif()
+if (LLDB_ENABLE_DEBUGINFOD)
+  list(APPEND EXTRA_LIBS ${Debuginfod_LIBRARIES})
+endif()
 if (WIN32)
   list(APPEND LLDB_SYSTEM_LIBS psapi)
 endif ()
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -15,6 +15,7 @@
 #include "lldb/Core/Highlighter.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleList.h"
+#include "lldb/Host/DebugInfoD.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
@@ -402,7 +403,9 @@
 if (target) {
   m_source_map_m

[Lldb-commits] [lldb] 34d7143 - [lldb] Fix windows build, second attempt

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-03-09T16:24:34+01:00
New Revision: 34d7143b035b3db64077968a09185834c1e7e9a7

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

LOG: [lldb] Fix windows build, second attempt

Added: 


Modified: 
lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp 
b/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
index 782edfe68279..6d608b2b6ce3 100644
--- a/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Host/windows/HostThreadWindows.h"
 #include "lldb/Host/windows/windows.h"
 #include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/Unwind.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Logging.h"
 #include "lldb/Utility/State.h"



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

[Lldb-commits] [lldb] 2b6ad82 - [lldb/test] Fix arch arm for 32-bit armv7l/armv8l

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2020-03-09T20:32:18+05:00
New Revision: 2b6ad82f8d0b20c6733217fcc6b3a07333287875

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

LOG: [lldb/test] Fix arch arm for 32-bit armv7l/armv8l

This patch forces architecture "arm" if underlying os reports core
armv7l or armv8l. On linux systems 32 bit sysroot running on 64bit
AArch64 hardware reports armv7l or armv8l which is essently arm
32bit mode. This fixes 5 testcases on 32bit arm.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 866daada3b9e..a9d6e50ce01f 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1241,6 +1241,8 @@ def getArchitecture(self):
 arch = module.getArchitecture()
 if arch == 'amd64':
 arch = 'x86_64'
+if arch in ['armv7l', 'armv8l'] :
+arch = 'arm'
 return arch
 
 def getLldbArchitecture(self):



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

[Lldb-commits] [PATCH] D75810: [lldb] Add .clang-tidy with customization to readability-identifier-naming.{Function, Member, Parameter, Variable}Case

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D75810#1912248 , @labath wrote:

> Yeah, naming conventions in lldb have always been a controversial topic. Most 
> of the code uses the lldb convention, but some of the new code (particularly 
> if it interfaces with llvm a lot) uses more llvm-like conventions. I think we 
> used to have a description of the naming convention somewhere on the website. 
> That is no longer there, but neither do we have a plan for migration.
>
> I don't really know what that means for this patch...


I would exclude `readability-identifier-naming` from the list of checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75810



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

[Lldb-commits] [PATCH] D75848: [lldb] Make UnwindLLDB a non-plugin

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Jason should make the call but looks good to me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75848



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

[Lldb-commits] [lldb] 12ba989 - [lldb/Process] Update ThreadKDP for API change

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-09T10:01:53-07:00
New Revision: 12ba989eeffbca12b501b9fde0a697c833fdfb1a

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

LOG: [lldb/Process] Update ThreadKDP for API change

GetUnwinder now returns a reference instead of a pointer.

Added: 


Modified: 
lldb/source/Plugins/Process/MacOSX-Kernel/ThreadKDP.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ThreadKDP.cpp 
b/lldb/source/Plugins/Process/MacOSX-Kernel/ThreadKDP.cpp
index a838b1038946..a4af5e030ff7 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/ThreadKDP.cpp
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ThreadKDP.cpp
@@ -118,9 +118,7 @@ ThreadKDP::CreateRegisterContextForFrame(StackFrame *frame) 
{
   }
 }
   } else {
-Unwind *unwinder = GetUnwinder();
-if (unwinder != nullptr)
-  reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame);
+reg_ctx_sp = GetUnwinder().CreateRegisterContextForFrame(frame);
   }
   return reg_ctx_sp;
 }



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

[Lldb-commits] [PATCH] D75810: [lldb] Add .clang-tidy with customization to disable readability-identifier-naming

[Fangrui Song via Phabricator via lldb-commits]

MaskRay updated this revision to Diff 249139.
MaskRay retitled this revision from "[lldb] Add .clang-tidy with customization 
to readability-identifier-naming.{Function,Member,Parameter,Variable}Case" to 
"[lldb] Add .clang-tidy with customization to disable 
readability-identifier-naming".
MaskRay added a comment.

Disable readability-identifier-naming


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75810

Files:
  lldb/.clang-tidy


Index: lldb/.clang-tidy
===
--- /dev/null
+++ lldb/.clang-tidy
@@ -0,0 +1,2 @@
+# Checks enabled in the top-level .clang-tidy minus 
readability-identifier-naming
+Checks: 
'-*,clang-diagnostic-*,llvm-*,misc-*,-misc-unused-parameters,-misc-non-private-member-variables-in-classes'


Index: lldb/.clang-tidy
===
--- /dev/null
+++ lldb/.clang-tidy
@@ -0,0 +1,2 @@
+# Checks enabled in the top-level .clang-tidy minus readability-identifier-naming
+Checks: '-*,clang-diagnostic-*,llvm-*,misc-*,-misc-unused-parameters,-misc-non-private-member-variables-in-classes'
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D75711#1912230 , @labath wrote:

> Thanks for the detailed response. I'll try to be equally understandable.
>
> In D75711#1910155 , @jingham wrote:
>
> > In D75711#1909055 , @labath wrote:
> >
> > > I am somewhat worried about the direction this is taking. Maybe vanishing 
> > > threads are fine for os-level debugging (because there the "real" threads 
> > > are the cpu cores, and the os threads are somewhat imaginary), but I 
> > > definitely wouldn't want to do it for regular debugging. If my process 
> > > has 1000 threads (not completely uncommon) then I don't think my debugger 
> > > would be doing me a service showing me just the three it thinks are 
> > > interesting and completely disregarding the others. Having a mode (it 
> > > could even be default) which only highlights the interesting ones -- yes, 
> > > that would definitely be useful. Making sure it doesn't touch the other 
> > > threads when it doesn't need to -- absolutely. But I don't think it 
> > > should pretend the other threads don't exist at all, because I may still 
> > > have a reason to inspect those threads (or just to know that they're 
> > > there). In fact, I wouldn't be surprised if this happened to the said 
> > > kernel folks too, and they end up adding a custom command, which would 
> > > enumerate all "threads".
> >
> >
> > First off, I also think that hiding some threads from users, or forcing 
> > people to say "thread list --force" or something like that to see them all 
> > would not be a user-friendly design.  I'm not suggesting imposing that on 
> > general debugging.  But if an OS plugin, for instance, decides it is too 
> > expensive to do this except as an explicit gesture, we need to support that 
> > decision.
> >
> > But lldb stops many times during the course of a step where it doesn't 
> > return control to the user - all the step into code with no debug info, 
> > step out again, step across dynamic linker stubs or step through ObjC 
> > message dispatch dances we do involve many private stops per public spot... 
> >  If a thread has not stopped "for a reason" at one of those stops, it takes 
> > no part in the "should stop" and "will resume" negotiations and 
> > reconstructing it for that stop really does serve no purpose.  There's no 
> > way that the user will get a chance to see it.
> >
> > So if we could avoid reading them in, that might make stepping go faster in 
> > the presence of lots of threads.  BTW, with the accelerated stop packets 
> > where we get all the threads and their stop reasons on stop, I'm not sure 
> > this would really save all that much time.  But talking to less capable 
> > stubs, it really can make a difference.  Greg did a bunch of work to make 
> > sure stepping stopped as few times as possible because the Facebook 
> > gdb-remote stub didn't support these accelerated packets and getting all 
> > the thread info on each private stop was making stepping go really slowly.  
> > I didn't understand the motivation at first because for debugserver the 
> > changes didn't really make any difference.
> >
> > Also, I don't think these two desires: not fetching threads when we don't 
> > need them (e.g. private stops) and showing them all to the user whenever 
> > they ask, are contradictory.  After all we still have the gate of 
> > UpdateThreadListIfNeeded.  All the commands that touch the thread list go 
> > through this. So even if we don't gather all the threads when we stop, we 
> > already have a mechanism in place whereby any command that actually touches 
> > the thread list can fetch them on demand.
>
>
> Yes, I agree with everything above. If lldb does not need information about 
> all threads in order to service a internal stop, then it shouldn't read it 
> in, and the os plugin (or the remote stub) should not need to provide it. In 
> fact I would even go so far as to say that we shouldn't need to read in this 
> information for a public stop until a user performs an operation (e.g. 
> `thread list`) that requires information about all threads.
>
> > 
> > 
> >> What was actually the slow part here? Was it the os plugin reading the 
> >> thread lists from the kernel memory? Or the fact that lldb is slow when it 
> >> is confronted with them?
> >> 
> >> If it is the latter, then maybe we should fix that. For example, I think I 
> >> remember lldb likes to check the PC of every thread before it does any 
> >> kind of a resume -- I am not convinced that is really needed.
> >> 
> >> If fetching the threads is slow, then perhaps we could have an api, where 
> >> the os threads are produced incrementally, and we ensure that lldb does 
> >> not query for os threads needlessly during the hot paths (e.g. internal 
> >> stops for servicing thread plans).
> > 
> > The thing that is slow is gathering all the threads.  On Darwin, the

[Lldb-commits] [PATCH] D75864: Add a decorator option to skip tests based on a default setting

[Adrian Prantl via Phabricator via lldb-commits]

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

This patch allows skipping a test based on a default setting, which is useful 
when running the testsuite in different "modes" based on a default setting. 
This is a feature I need for the Swift testsuite, but I think it's generally 
useful.


https://reviews.llvm.org/D75864

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/test/API/sanity/TestSettingSkipping.py


Index: lldb/test/API/sanity/TestSettingSkipping.py
===
--- /dev/null
+++ lldb/test/API/sanity/TestSettingSkipping.py
@@ -0,0 +1,29 @@
+"""
+This is a sanity check that verifies that test can be sklipped based on 
settings.
+"""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+class SettingSkipSanityTestCase(TestBase):
+
+  mydir = TestBase.compute_mydir(__file__)
+
+  NO_DEBUG_INFO_TESTCASE = True
+
+  @skipIf(setting=('target.prefer-dynamic-value', 'no-dynamic-values'))
+  def testSkip(self):
+"""This setting is on by default"""
+self.assertTrue(False, "This test should not run!")
+
+  @skipIf(setting=('target.prefer-dynamic-value', 'run-target'))
+  def testNoMatch(self):
+self.assertTrue(True, "This test should run!")
+
+  @skipIf(setting=('target.i-made-this-one-up', 'true'))
+  def testNotExisting(self):
+self.assertTrue(True, "This test should run!")
+
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -158,7 +158,8 @@
   debug_info=None,
   swig_version=None, py_version=None,
   macos_version=None,
-  remote=None, dwarf_version=None):
+  remote=None, dwarf_version=None,
+  setting=None):
 def fn(self):
 skip_for_os = _match_decorator_property(
 lldbplatform.translate(oslist), self.getPlatform())
@@ -196,6 +197,8 @@
 skip_for_dwarf_version = (dwarf_version is None) or (
 _check_expected_version(dwarf_version[0], dwarf_version[1],
 self.getDwarfVersion()))
+skip_for_setting = (setting is None) or (
+setting in configuration.settings)
 
 # For the test to be skipped, all specified (e.g. not None) parameters 
must be True.
 # An unspecified parameter means "any", so those are marked skip by 
default.  And we skip
@@ -210,7 +213,8 @@
   (py_version, skip_for_py_version, "python version"),
   (macos_version, skip_for_macos_version, "macOS version"),
   (remote, skip_for_remote, "platform locality 
(remote/local)"),
-  (dwarf_version, skip_for_dwarf_version, "dwarf version")]
+  (dwarf_version, skip_for_dwarf_version, "dwarf version"),
+  (setting, skip_for_setting, "setting")]
 reasons = []
 final_skip_result = True
 for this_condition in conditions:
@@ -279,7 +283,8 @@
debug_info=None,
swig_version=None, py_version=None,
macos_version=None,
-   remote=None, dwarf_version=None):
+   remote=None, dwarf_version=None,
+   setting=None):
 return _decorateTest(DecorateMode.Skip,
  bugnumber=bugnumber,
  oslist=oslist, hostoslist=hostoslist,
@@ -288,7 +293,8 @@
  debug_info=debug_info,
  swig_version=swig_version, py_version=py_version,
  macos_version=macos_version,
- remote=remote, dwarf_version=dwarf_version)
+ remote=remote, dwarf_version=dwarf_version,
+ setting=setting)
 
 
 def _skip_for_android(reason, api_levels, archs):


Index: lldb/test/API/sanity/TestSettingSkipping.py
===
--- /dev/null
+++ lldb/test/API/sanity/TestSettingSkipping.py
@@ -0,0 +1,29 @@
+"""
+This is a sanity check that verifies that test can be sklipped based on settings.
+"""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+class SettingSkipSanityTestCase(TestBase):
+
+  mydir = TestBase.compute_mydir(__file__)
+
+  NO_DEBUG_INFO_TESTCASE = True
+
+  @skipIf(setting=('target.prefer-dynamic-value', 'no-dynamic-values'))
+  def testSkip(self):
+"""This setting is on by default"""
+self.assertTrue(False, "This test should not run!")
+
+  @skipIf(setting=('target.prefer-dynamic-value', 'run-target'))
+  def testNoMatch(self):
+self.assertTrue(True, "This test should run!")
+
+  @skipIf(setting=('target.i-made-this-one-up', 'true'))
+  

[Lldb-commits] [PATCH] D75848: [lldb] Make UnwindLLDB a non-plugin

[Alex Langford via Phabricator via lldb-commits]

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

Thanks for doing this. LGTM, might want to wait for Jason's approval like Jonas 
said though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75848



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

[Lldb-commits] [PATCH] D75848: [lldb] Make UnwindLLDB a non-plugin

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda accepted this revision.
jasonmolenda added a comment.

Thanks for doing this, lgtm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75848



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

[Lldb-commits] [lldb] 71269a1 - [lldb] Add .clang-tidy with customization to disable readability-identifier-naming

[Fangrui Song via lldb-commits]

Author: Fangrui Song
Date: 2020-03-09T12:50:28-07:00
New Revision: 71269a1f172cdad1cc0d7e2e6c94a7ece11ddb27

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

LOG: [lldb] Add .clang-tidy with customization to disable 
readability-identifier-naming

Reviewed By: JDevlieghere

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

Added: 
lldb/.clang-tidy

Modified: 


Removed: 




diff  --git a/lldb/.clang-tidy b/lldb/.clang-tidy
new file mode 100644
index ..e949902171e7
--- /dev/null
+++ b/lldb/.clang-tidy
@@ -0,0 +1,2 @@
+# Checks enabled in the top-level .clang-tidy minus 
readability-identifier-naming
+Checks: 
'-*,clang-diagnostic-*,llvm-*,misc-*,-misc-unused-parameters,-misc-non-private-member-variables-in-classes'



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

[Lldb-commits] [PATCH] D75810: [lldb] Add .clang-tidy with customization to disable readability-identifier-naming

[Fangrui Song via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG71269a1f172c: [lldb] Add .clang-tidy with customization to 
disable readability-identifier… (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75810

Files:
  lldb/.clang-tidy


Index: lldb/.clang-tidy
===
--- /dev/null
+++ lldb/.clang-tidy
@@ -0,0 +1,2 @@
+# Checks enabled in the top-level .clang-tidy minus 
readability-identifier-naming
+Checks: 
'-*,clang-diagnostic-*,llvm-*,misc-*,-misc-unused-parameters,-misc-non-private-member-variables-in-classes'


Index: lldb/.clang-tidy
===
--- /dev/null
+++ lldb/.clang-tidy
@@ -0,0 +1,2 @@
+# Checks enabled in the top-level .clang-tidy minus readability-identifier-naming
+Checks: '-*,clang-diagnostic-*,llvm-*,misc-*,-misc-unused-parameters,-misc-non-private-member-variables-in-classes'
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

> Everything that holds onto its state using an ExecutionContextRef already 
> holds onto the m_tid as the real entity, and the Thread is only a cache (see 
> ExecutionContextRef::GetThreadSP).

I'm sorry I missed that part. (I saw the ThreadWP, but didn't look further into 
how it is used).

I'm going to respond in more detail tomorrow, but for now I am just wanted to 
let you know that my main concern about this has been addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75711



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

[Lldb-commits] [PATCH] D75877: [lldb/Reproducers] Fix replay for process attach workflows

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, jasonmolenda.
Herald added a subscriber: teemperor.
Herald added a project: LLDB.

Support replaying debug sessions that attach to an existing process instead of 
lldb launching the inferior. Bypass the logic that looks for a process with the 
given name and use an arbitrary PID. The value of the PID doesn't matter as the 
gdb remote replay infrastructure intercepts the attach and pretends that we're 
connected to the original process.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D75877

Files:
  lldb/source/Target/Process.cpp
  lldb/test/Shell/Reproducer/Inputs/sleep.c
  lldb/test/Shell/Reproducer/TestAttach.test


Index: lldb/test/Shell/Reproducer/TestAttach.test
===
--- /dev/null
+++ lldb/test/Shell/Reproducer/TestAttach.test
@@ -0,0 +1,11 @@
+# UNSUPPORTED: system-windows, system-freebsd
+
+# RUN: rm -rf %t.repro
+# RUN: mkdir -p %t
+
+# RUN: %clang_host %S/Inputs/sleep.c -g -o %t/attach.out
+# RUN: python -c 'import os; os.system("%t/attach.out &")'
+
+# RUN: %lldb --capture --capture-path %t.repro -o 'pro att -n attach.out' -o 
'reproducer generate' | FileCheck %s
+# RUN: %lldb --replay %t.repro | FileCheck %s
+# CHECK: stop reason = signal SIGSTOP
Index: lldb/test/Shell/Reproducer/Inputs/sleep.c
===
--- /dev/null
+++ lldb/test/Shell/Reproducer/Inputs/sleep.c
@@ -0,0 +1,7 @@
+#include 
+#include 
+
+int main(int argc, char const *argv[]) {
+  sleep(10);
+  return 0;
+}
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -65,6 +65,7 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/NameMatches.h"
 #include "lldb/Utility/ProcessInfo.h"
+#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/SelectHelper.h"
 #include "lldb/Utility/State.h"
 
@@ -2790,6 +2791,11 @@
   }
   return error;
 }
+  } else if (repro::Reproducer::Instance().IsReplaying()) {
+// The actual PID doesn't matter, we just need to set it to something
+// that's not LLDB_INVALID_PROCESS_ID. The GDB remote replay
+// infrastructure will intercept our attach later on.
+attach_pid = 1;
   } else {
 ProcessInstanceInfoList process_infos;
 PlatformSP platform_sp(GetTarget().GetPlatform());


Index: lldb/test/Shell/Reproducer/TestAttach.test
===
--- /dev/null
+++ lldb/test/Shell/Reproducer/TestAttach.test
@@ -0,0 +1,11 @@
+# UNSUPPORTED: system-windows, system-freebsd
+
+# RUN: rm -rf %t.repro
+# RUN: mkdir -p %t
+
+# RUN: %clang_host %S/Inputs/sleep.c -g -o %t/attach.out
+# RUN: python -c 'import os; os.system("%t/attach.out &")'
+
+# RUN: %lldb --capture --capture-path %t.repro -o 'pro att -n attach.out' -o 'reproducer generate' | FileCheck %s
+# RUN: %lldb --replay %t.repro | FileCheck %s
+# CHECK: stop reason = signal SIGSTOP
Index: lldb/test/Shell/Reproducer/Inputs/sleep.c
===
--- /dev/null
+++ lldb/test/Shell/Reproducer/Inputs/sleep.c
@@ -0,0 +1,7 @@
+#include 
+#include 
+
+int main(int argc, char const *argv[]) {
+  sleep(10);
+  return 0;
+}
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -65,6 +65,7 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/NameMatches.h"
 #include "lldb/Utility/ProcessInfo.h"
+#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/SelectHelper.h"
 #include "lldb/Utility/State.h"
 
@@ -2790,6 +2791,11 @@
   }
   return error;
 }
+  } else if (repro::Reproducer::Instance().IsReplaying()) {
+// The actual PID doesn't matter, we just need to set it to something
+// that's not LLDB_INVALID_PROCESS_ID. The GDB remote replay
+// infrastructure will intercept our attach later on.
+attach_pid = 1;
   } else {
 ProcessInstanceInfoList process_infos;
 PlatformSP platform_sp(GetTarget().GetPlatform());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 3cabd17 - [ObjC] Dynamic type resolution logging should go to the types log.

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-03-09T15:35:51-07:00
New Revision: 3cabd173a1e34febbf0351599b760a5516e64ec4

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

LOG: [ObjC] Dynamic type resolution logging should go to the types log.

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index 6b1cc56fc2b0..dc3753eb44b7 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1207,7 +1207,8 @@ AppleObjCRuntimeV2::GetClassDescriptor(ValueObject 
&valobj) {
 if (isa != LLDB_INVALID_ADDRESS) {
   objc_class_sp = GetClassDescriptorFromISA(isa);
   if (isa && !objc_class_sp) {
-Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS |
+  LIBLLDB_LOG_TYPES));
 LLDB_LOGF(log,
   "0x%" PRIx64
   ": AppleObjCRuntimeV2::GetClassDescriptor() ISA was "



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

[Lldb-commits] [lldb] a3c4e6b - [AppleObjC2RuntimeV2] Remove dead code. NFC.

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-03-09T15:37:12-07:00
New Revision: a3c4e6b44a18ffafec022d6ab76dc4f58c4ef129

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

LOG: [AppleObjC2RuntimeV2] Remove dead code. NFC.

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index dc3753eb44b7..cf0a914c26d4 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1434,8 +1434,6 @@ AppleObjCRuntimeV2::UpdateISAToDescriptorMapDynamic(
 
 Value return_value;
 return_value.SetValueType(Value::eValueTypeScalar);
-// return_value.SetContext (Value::eContextTypeClangType,
-// clang_uint32_t_type);
 return_value.SetCompilerType(clang_uint32_t_type);
 return_value.GetScalar() = 0;
 
@@ -1660,13 +1658,11 @@ 
AppleObjCRuntimeV2::UpdateISAToDescriptorMapSharedCache() {
 // Next make the function caller for our implementation utility function.
 Value value;
 value.SetValueType(Value::eValueTypeScalar);
-// value.SetContext (Value::eContextTypeClangType, 
clang_void_pointer_type);
 value.SetCompilerType(clang_void_pointer_type);
 arguments.PushValue(value);
 arguments.PushValue(value);
 
 value.SetValueType(Value::eValueTypeScalar);
-// value.SetContext (Value::eContextTypeClangType, clang_uint32_t_type);
 value.SetCompilerType(clang_uint32_t_type);
 arguments.PushValue(value);
 arguments.PushValue(value);
@@ -1732,8 +1728,6 @@ AppleObjCRuntimeV2::UpdateISAToDescriptorMapSharedCache() 
{
 
 Value return_value;
 return_value.SetValueType(Value::eValueTypeScalar);
-// return_value.SetContext (Value::eContextTypeClangType,
-// clang_uint32_t_type);
 return_value.SetCompilerType(clang_uint32_t_type);
 return_value.GetScalar() = 0;
 



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

[Lldb-commits] [PATCH] D75864: Add a decorator option to skip tests based on a default setting

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/test/API/sanity/TestSettingSkipping.py:28
+  def testNotExisting(self):
+self.assertTrue(True, "This test should run!")
+

This won't trip if the tests doesn't run. If you make the assert trip and XFAIL 
the test it should catch it. 


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

https://reviews.llvm.org/D75864



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

[Lldb-commits] [PATCH] D75715: Switch TypeSystemClang over to CreateDeserialized() (NFC)

[Raphael Isemann via Phabricator via lldb-commits]

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

The changes that remove the non-existent module parameter slipped in by mistake 
I think and the rest LGTM. I didn't verify if all the `setX` calls are 100% 
identical to what the constructor would do but *usually* Decl constructor don't 
any fancy logic so the new code should be equivalent. Maybe dump some module 
AST before and after this patch and just diff it to see if anything changed.


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

https://reviews.llvm.org/D75715



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

[Lldb-commits] [lldb] 9d389f7 - [AppleObjCRuntimeV2] Fix a typo. Evalulate -> evaluate.

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-03-09T15:40:09-07:00
New Revision: 9d389f78589d71adf822c97c329baa62da70b34c

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

LOG: [AppleObjCRuntimeV2] Fix a typo. Evalulate -> evaluate.

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index cf0a914c26d4..cb1d808d0f1e 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2471,7 +2471,7 @@ bool 
AppleObjCRuntimeV2::NonPointerISACache::EvaluateNonPointerISA(
 ObjCISA isa, ObjCISA &ret_isa) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES));
 
-  LLDB_LOGF(log, "AOCRT::NPI Evalulate(isa = 0x%" PRIx64 ")", (uint64_t)isa);
+  LLDB_LOGF(log, "AOCRT::NPI Evaluate(isa = 0x%" PRIx64 ")", (uint64_t)isa);
 
   if ((isa & ~m_objc_debug_isa_class_mask) == 0)
 return false;
@@ -2552,7 +2552,7 @@ bool 
AppleObjCRuntimeV2::NonPointerISACache::EvaluateNonPointerISA(
   if (index > m_indexed_isa_cache.size())
 return false;
 
-  LLDB_LOGF(log, "AOCRT::NPI Evalulate(ret_isa = 0x%" PRIx64 ")",
+  LLDB_LOGF(log, "AOCRT::NPI Evaluate(ret_isa = 0x%" PRIx64 ")",
 (uint64_t)m_indexed_isa_cache[index]);
 
   ret_isa = m_indexed_isa_cache[index];



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

[Lldb-commits] [PATCH] D75761: Fix to get the AST we generate for function templates to be closer to what clang generates and expects

[Raphael Isemann via Phabricator via lldb-commits]

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

Small nit pick: Use `expect_expr(` over `expect("expr ...` if you do a Python 
test.


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

https://reviews.llvm.org/D75761



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

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: friss, clayborg, labath.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.
jingham added parent revisions: D75711: [NFC] Have ThreadPlans hold onto the 
Process & TID, rather than the Thread, D75720: Make ThreadPlanTracer use 
{Process,TID} rather than Thread to find it's underlying thread

This is the third in the series of patches that move the ThreadPlan stacks out 
of the Threads and lets the process manage them instead.  The first two are:

https://reviews.llvm.org/D75720
https://reviews.llvm.org/D75711

This patch just moves the three thread plan stacks - current, discarded and 
completed - out of ThreadPlan and into the ThreadPlanStack class.  This is a 
nice cleanup on its own as it gets all the thread plan manipulation logic out 
of Thread.  Then the ThreadPlanStacks move out of Thread to Process where they 
are managed in a map ordered by TID.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75880

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanStack.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Process.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadList.cpp
  lldb/source/Target/ThreadPlan.cpp
  lldb/source/Target/ThreadPlanStack.cpp

Index: lldb/source/Target/ThreadPlanStack.cpp
===
--- /dev/null
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -0,0 +1,379 @@
+//===-- ThreadPlanStack.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
+//
+//===--===//
+
+#include "lldb/Target/ThreadPlanStack.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Target/ThreadPlan.h"
+#include "lldb/Utility/Log.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static void PrintPlanElement(Stream *s, const ThreadPlanSP &plan,
+ lldb::DescriptionLevel desc_level,
+ int32_t elem_idx) {
+  s->IndentMore();
+  s->Indent();
+  s->Printf("Element %d: ", elem_idx);
+  plan->GetDescription(s, desc_level);
+  s->EOL();
+  s->IndentLess();
+}
+
+void ThreadPlanStack::DumpThreadPlans(Stream *s, 
+  lldb::DescriptionLevel desc_level,
+  bool include_internal) const {
+
+  uint32_t stack_size;
+
+  s->IndentMore();
+  s->Indent();
+  s->Printf("Active plan stack:\n");
+  int32_t print_idx = 0;
+  for (auto plan : m_plans) {
+PrintPlanElement(s, plan, desc_level, print_idx++);
+  }
+
+  if (AnyCompletedPlans()) {
+print_idx = 0;
+s->Indent();
+s->Printf("Completed Plan Stack:\n");
+for (auto plan : m_completed_plans)
+  PrintPlanElement(s, plan, desc_level, print_idx++);
+  }
+
+  if (AnyDiscardedPlans()) {
+print_idx = 0;
+s->Indent();
+s->Printf("Discarded Plan Stack:\n");
+for (auto plan : m_discarded_plans)
+  PrintPlanElement(s, plan, desc_level, print_idx++);
+  }
+
+  s->IndentLess();
+}
+
+
+size_t ThreadPlanStack::CheckpointCompletedPlans() {
+  m_completed_plan_checkpoint++;
+  m_completed_plan_store
+  .insert(std::make_pair(m_completed_plan_checkpoint, 
+ m_completed_plans));
+  return m_completed_plan_checkpoint;
+}
+
+void ThreadPlanStack::RestoreCompletedPlanCheckpoint(size_t checkpoint) {
+  auto result = m_completed_plan_store.find(checkpoint);
+  if (result == m_completed_plan_store.end())
+llvm_unreachable("Asked for a checkpoint that didn't exist");
+  m_completed_plans.swap((*result).second);
+  m_completed_plan_store.erase(result);
+}
+
+void ThreadPlanStack::DiscardCompletedPlanCheckpoint(size_t checkpoint) {
+  m_completed_plan_store.erase(checkpoint);
+}
+
+void ThreadPlanStack::ThreadDestroyed(Thread *thread) {
+  // Tell the plan stacks that this thread is going away:
+  for (auto plan : m_plans)
+plan->ThreadDestroyed();
+
+  for (auto plan : m_discarded_plans)
+plan->ThreadDestroyed();
+
+  for (auto plan : m_completed_plans)
+plan->ThreadDestroyed();
+  
+  // Now clear the current plan stacks:
+  m_plans.clear();
+  m_discarded_plans.clear();
+  m_completed_plans.clear();
+  
+  // Push a ThreadPlanNull on the plan stack.  That way we can continue
+  // assuming that the plan stack is never empty, but if somebody errantly asks
+  // questions of a destroyed thread without checking first whether it is
+  // destroyed, they won't crash.
+  if (thread != nullptr) {
+lldb::ThreadPlanSP null_plan_sp(new ThreadPlanNull(*thread));
+ 

[Lldb-commits] [PATCH] D75761: Fix to get the AST we generate for function templates to be closer to what clang generates and expects

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 249247.
shafik marked 11 inline comments as done.
shafik added a comment.

Moving to using `ItaniumPartialDemangler` for now.


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

https://reviews.llvm.org/D75761

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
  lldb/test/API/lang/cpp/template-function/main.cpp

Index: lldb/test/API/lang/cpp/template-function/main.cpp
===
--- lldb/test/API/lang/cpp/template-function/main.cpp
+++ lldb/test/API/lang/cpp/template-function/main.cpp
@@ -3,6 +3,67 @@
 return int(t1);
 }
 
+// Some cases to cover ADL
+namespace A {
+struct C {};
+
+template  int f(T) { return 4; }
+
+template  int g(T) { return 4; }
+} // namespace A
+
+int f(int) { return 1; }
+
+template  int h(T x) { return x; }
+
+int h(double d) { return 5; }
+
+template  int var(Us... pargs) { return 10; }
+
+// Having the templated overloaded operators in a namespace effects the
+// mangled name generated in the IR e.g. _ZltRK1BS1_ Vs _ZN1AltERKNS_1BES2_
+// One will be in the symbol table but the other won't. This results in a
+// different code path that will result in CPlusPlusNameParser being used.
+// This allows us to cover that code as well.
+namespace A {
+template  bool operator<(const T &, const T &) { return true; }
+
+template  bool operator>(const T &, const T &) { return true; }
+
+template  bool operator<<(const T &, const T &) { return true; }
+
+template  bool operator>>(const T &, const T &) { return true; }
+
+template  bool operator==(const T &, const T &) { return true; }
+
+struct B {};
+} // namespace A
+
+struct C {};
+
+// Make sure we cover more straight forward cases as well.
+bool operator<(const C &, const C &) { return true; }
+bool operator>(const C &, const C &) { return true; }
+bool operator>>(const C &, const C &) { return true; }
+bool operator<<(const C &, const C &) { return true; }
+bool operator==(const C &, const C &) { return true; }
+
 int main() {
-return foo(42);
+  A::B b1;
+  A::B b2;
+  C c1;
+  C c2;
+
+  bool result_b = b1 < b2 && b1 << b2 && b1 == b2 && b1 > b2 && b1 >> b2;
+  bool result_c = c1 < c2 && c1 << c2 && c1 == c2 && c1 > c2 && c1 >> c2;
+
+  return foo(42) + result_b + result_c +
+ // ADL lookup case,
+ f(A::C{}) +
+ // ADL lookup but no overload
+ g(A::C{}) +
+ // overload with template
+ h(10) + h(1.) +
+ // variadic function
+ var(1) + var(1, 2); // break here
 }
Index: lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
===
--- lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
+++ lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
@@ -13,14 +13,54 @@
 
 def do_test_template_function(self, add_cast):
 self.build()
-(_, _, thread, _) = lldbutil.run_to_name_breakpoint(self, "main")
-frame = thread.GetSelectedFrame()
-expr = "foo(42)"
+(self.target, self.process, _, bkpt) = lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
 if add_cast:
-expr = "(int)" + expr
-expr_result = frame.EvaluateExpression(expr)
-self.assertTrue(expr_result.IsValid())
-self.assertEqual(expr_result.GetValue(), "42")
+  self.expect("expr (int) foo(42)",
+substrs=['(int)', '= 42'])
+else:
+  self.expect("expr foo(42)",
+substrs=['(int)', '= 42'])
+
+  self.expect("expr h(10)",
+substrs=['(int)', '= 10'])
+
+  self.expect("expr f(A::C{})",
+   substrs=['(int)', '= 4'])
+
+  self.expect("expr g(A::C{})",
+   substrs=['(int)', '= 4'])
+
+  self.expect("expr var(1)",
+   substrs=['(int)', '= 10'])
+
+  self.expect("expr var(1,2)",
+   substrs=['(int)', '= 10'])
+
+  self.expect("expr b1 > b2",
+   substrs=['(bool)', '= true'])
+
+  self.expect("expr b1 >> b2",
+   substrs=['(bool)', '= true'])
+
+  self.expect("expr b1 << b2",
+   substrs=['(bool)', '= true'])
+
+  self.expect("expr b1 == b2",
+   substrs=['(bool)', '= true'])
+
+  self.expect("expr c1 > c2",
+   substrs=['(bool)', '= true'])
+
+  self.expect("expr c1 >> c2",
+   substrs=['(bool)', '= true'])
+
+  self.expect("expr c1 << c2",
+   substrs=['(bool)', '= true'])
+
+  self.expect("expr c1 == c2",
+   substrs=['(bool)', '= true'])
 
 @skipIfWindows
 def test_template_function_with_cast(self):
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

[Lldb-commits] [PATCH] D75761: Fix to get the AST we generate for function templates to be closer to what clang generates and expects

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D75761#1912038 , @labath wrote:

> It's a pity that the clang's DW_AT_name value is so ambiguous. For example, 
> gcc would output the name in the commit message as `operator< `, which 
> is a lot more parsable (for computers and people). Have you looked at 
> changing clang's output to make it more like gcc's ? I know it would only 
> help new compilers, but maybe for such a special case, this does not matter?
>
> Or, even if we do end up adding some compat parsing code, that would reduce 
> the need for it to be super exact. I'm pretty sure that the current code does 
> not handle all cases correctly. E.g., I believe it will break on something 
> like `operator<<<&operator>> >` (mangled name `_ZlsIXadL_Zrs1AS0_EEEvS0_S0_`, 
> godbolt link ).
>
> As an alternative, we could try extracting the same information from the 
> mangled name (via llvm::ItaniumPartialDemangler::getFunctionBaseName), at 
> least when DW_AT_linkage_name is present (not all compilers emit that 
> attribute but I am not sure if anyone has tested if lldb actually works 
> without it).


I was planning on looking into removing the template parameters from the names 
altogether for lldb but we would still need to do this on the lldb side to 
support older compilers. I have to try this and see how much it breaks.

I am going to move to use `llvm::ItaniumPartialDemangler::getFunctionBaseName)` 
and I am going to see if I can test subroutines w/o `DW_AT_linkage_name` and 
see if it works at all. If it does not work then maybe we don't need to support 
the case where we don't have it.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:829
+  // have something like operator>>. We check for the operator<=> case.
+  if (!Name.endswith(">") || Name.count("<") == 0 || Name.endswith("<=>"))
+return {};

aprantl wrote:
> Should we scan for the complete "operator<" "operator<=>" instead, to be 
> clearer and more future-proof?
> Should we assert(Name.count("<")), too?
I don't believe looking for the full name buys us anything correctness wise, we 
would have to be getting malformed names AFAIK but perhaps I am missing some 
odd case.

I don't think the assert makes sense b/c we may end up sharing this code w/ 
dsymutil where we don't know before hand if we have template params in the 
name. Also perhaps in the future we may change clang to not generate 
DW_AT_names like this but still support older compilers that do.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:839
+  size_t RightAngleCount = Name.count('>');
+  size_t LeftAngleCount = Name.count('<');
+

labath wrote:
> aprantl wrote:
> > We are scanning the entire string multiple times here and that seems 
> > unnecessarily expensive since C++ template function names get looong. I 
> > think we could do this as a single for-loop with a state machine instead, 
> > without being too difficult to read?
> Would it be sufficient to always strip one `<` from the first sequence of 
> `<`'s that you find? You can't have two template `<` one after the other, so 
> in a valid name, the last `<` will be the template angle bracket, while 
> everything else must be a part of the operator name.
> 
> So, something like `return {Name.data(), Name.find_first_not_of('<', 
> Name.find_first_of('<')) -1}` (with some error checks and an explicit check 
> for the spaceship operator).
You correct, I was moving toward a state machine before you pointed out 
`ItaniumPartialDemangler`



Comment at: lldb/test/API/lang/cpp/template-function/main.cpp:1
 template
 int foo(T t1) {

labath wrote:
> Do we have a test for the spaceship operator?
We currently don't support C++2a but when we do we should add a test.


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

https://reviews.llvm.org/D75761



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

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

[Jim Ingham via Phabricator via lldb-commits]

jingham updated this revision to Diff 249257.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75880

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanStack.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Process.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadList.cpp
  lldb/source/Target/ThreadPlan.cpp
  lldb/source/Target/ThreadPlanStack.cpp

Index: lldb/source/Target/ThreadPlanStack.cpp
===
--- /dev/null
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -0,0 +1,373 @@
+//===-- ThreadPlanStack.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
+//
+//===--===//
+
+#include "lldb/Target/ThreadPlanStack.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Target/ThreadPlan.h"
+#include "lldb/Utility/Log.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static void PrintPlanElement(Stream *s, const ThreadPlanSP &plan,
+ lldb::DescriptionLevel desc_level,
+ int32_t elem_idx) {
+  s->IndentMore();
+  s->Indent();
+  s->Printf("Element %d: ", elem_idx);
+  plan->GetDescription(s, desc_level);
+  s->EOL();
+  s->IndentLess();
+}
+
+void ThreadPlanStack::DumpThreadPlans(Stream *s,
+  lldb::DescriptionLevel desc_level,
+  bool include_internal) const {
+
+  uint32_t stack_size;
+
+  s->IndentMore();
+  s->Indent();
+  s->Printf("Active plan stack:\n");
+  int32_t print_idx = 0;
+  for (auto plan : m_plans) {
+PrintPlanElement(s, plan, desc_level, print_idx++);
+  }
+
+  if (AnyCompletedPlans()) {
+print_idx = 0;
+s->Indent();
+s->Printf("Completed Plan Stack:\n");
+for (auto plan : m_completed_plans)
+  PrintPlanElement(s, plan, desc_level, print_idx++);
+  }
+
+  if (AnyDiscardedPlans()) {
+print_idx = 0;
+s->Indent();
+s->Printf("Discarded Plan Stack:\n");
+for (auto plan : m_discarded_plans)
+  PrintPlanElement(s, plan, desc_level, print_idx++);
+  }
+
+  s->IndentLess();
+}
+
+size_t ThreadPlanStack::CheckpointCompletedPlans() {
+  m_completed_plan_checkpoint++;
+  m_completed_plan_store.insert(
+  std::make_pair(m_completed_plan_checkpoint, m_completed_plans));
+  return m_completed_plan_checkpoint;
+}
+
+void ThreadPlanStack::RestoreCompletedPlanCheckpoint(size_t checkpoint) {
+  auto result = m_completed_plan_store.find(checkpoint);
+  if (result == m_completed_plan_store.end())
+llvm_unreachable("Asked for a checkpoint that didn't exist");
+  m_completed_plans.swap((*result).second);
+  m_completed_plan_store.erase(result);
+}
+
+void ThreadPlanStack::DiscardCompletedPlanCheckpoint(size_t checkpoint) {
+  m_completed_plan_store.erase(checkpoint);
+}
+
+void ThreadPlanStack::ThreadDestroyed(Thread *thread) {
+  // Tell the plan stacks that this thread is going away:
+  for (auto plan : m_plans)
+plan->ThreadDestroyed();
+
+  for (auto plan : m_discarded_plans)
+plan->ThreadDestroyed();
+
+  for (auto plan : m_completed_plans)
+plan->ThreadDestroyed();
+
+  // Now clear the current plan stacks:
+  m_plans.clear();
+  m_discarded_plans.clear();
+  m_completed_plans.clear();
+
+  // Push a ThreadPlanNull on the plan stack.  That way we can continue
+  // assuming that the plan stack is never empty, but if somebody errantly asks
+  // questions of a destroyed thread without checking first whether it is
+  // destroyed, they won't crash.
+  if (thread != nullptr) {
+lldb::ThreadPlanSP null_plan_sp(new ThreadPlanNull(*thread));
+m_plans.push_back(null_plan_sp);
+  }
+}
+
+void ThreadPlanStack::EnableTracer(bool value, bool single_stepping) {
+  for (auto plan : m_plans) {
+if (plan->GetThreadPlanTracer()) {
+  plan->GetThreadPlanTracer()->EnableTracing(value);
+  plan->GetThreadPlanTracer()->EnableSingleStep(single_stepping);
+}
+  }
+}
+
+void ThreadPlanStack::SetTracer(lldb::ThreadPlanTracerSP &tracer_sp) {
+  for (auto plan : m_plans)
+plan->SetThreadPlanTracer(tracer_sp);
+}
+
+void ThreadPlanStack::PushPlan(lldb::ThreadPlanSP &new_plan_sp) {
+  // If the thread plan doesn't already have a tracer, give it its parent's
+  // tracer:
+  // The first plan has to be a base plan:
+  if (m_plans.size() == 0 && !new_plan_sp->IsBasePlan())
+llvm_unreachable("Zeroth plan must be a base plan");
+
+  if (!new_plan_sp->GetThreadPlanTracer()) {
+assert(!m_plans.empty());