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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

If a test requires a specific value of a setting, would it make more sense to 
just (re)set its value at the start of a test?


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] [lldb] 5aa5c94 - Reland "[DebugInfo] Enable the debug entry values feature by default"

[Djordje Todorovic via lldb-commits]

Author: Djordje Todorovic
Date: 2020-03-10T09:15:06+01:00
New Revision: 5aa5c943f7da155b95564058cd5d50a93eabfc89

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

LOG: Reland "[DebugInfo] Enable the debug entry values feature by default"

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

Added: 
llvm/test/DebugInfo/X86/no-entry-values-with-O0.ll

Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/CC1Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/debug-info-extern-call.c
clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
lldb/packages/Python/lldbsuite/test/decorators.py

lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
llvm/include/llvm/CodeGen/CommandFlags.inc
llvm/include/llvm/Target/TargetMachine.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
llvm/lib/CodeGen/LiveDebugValues.cpp
llvm/lib/CodeGen/TargetOptionsImpl.cpp
llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
llvm/lib/Target/ARM/ARMTargetMachine.cpp
llvm/lib/Target/X86/X86TargetMachine.cpp
llvm/test/CodeGen/MIR/Hexagon/bundled-call-site-info.mir
llvm/test/CodeGen/MIR/X86/call-site-info-error4.mir
llvm/test/CodeGen/X86/call-site-info-output.ll
llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir
llvm/test/DebugInfo/MIR/ARM/call-site-info-vmovd.mir
llvm/test/DebugInfo/MIR/ARM/call-site-info-vmovs.mir
llvm/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir
llvm/test/DebugInfo/MIR/Hexagon/dbgcall-site-instr-before-bundled-call.mir
llvm/test/DebugInfo/MIR/Hexagon/live-debug-values-bundled-entry-values.mir
llvm/test/DebugInfo/MIR/SystemZ/call-site-lzer.mir
llvm/test/DebugInfo/MIR/X86/DW_OP_entry_value.mir
llvm/test/DebugInfo/MIR/X86/call-site-gnu-vs-dwarf5-attrs.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-copy-super-sub.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-lea-interpretation.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-partial-describe.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-reference.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-reg-shuffle.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-two-fwd-reg-defs.mir
llvm/test/DebugInfo/MIR/X86/dbginfo-entryvals.mir
llvm/test/DebugInfo/MIR/X86/debug-call-site-param.mir
llvm/test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir
llvm/test/DebugInfo/MIR/X86/entry-values-diamond-bbs.mir
llvm/test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir
llvm/test/DebugInfo/MIR/X86/unreachable-block-call-site.mir
llvm/test/DebugInfo/X86/dbg-value-range.ll
llvm/test/DebugInfo/X86/dbg-value-regmask-clobber.ll
llvm/test/DebugInfo/X86/dbgcall-site-64-bit-imms.ll
llvm/test/DebugInfo/X86/dbgcall-site-zero-valued-imms.ll
llvm/test/DebugInfo/X86/loclists-dwp.ll
llvm/test/tools/llvm-locstats/locstats.ll

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index 3c8b0eeb47a5..e047054447f3 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -63,7 +63,6 @@ CODEGENOPT(ExperimentalNewPassManager, 1, 0) ///< Enables the 
new, experimental
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
 CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
-CODEGENOPT(EnableDebugEntryValues, 1, 0) ///< Emit call site parameter dbg info
 CODEGENOPT(EmitCallSiteInfo, 1, 0) ///< Emit call site info only in the case of
///< '-g' + 'O>0' level.
 CODEGENOPT(IndirectTlsSegRefs, 1, 0) ///< Set when -mno-tls-direct-seg-refs

diff  --git a/clang/include/clang/Driver/CC1Options.td 
b/clang/include/clang/Driver/CC1Options.td
index b7a2826d8fcb..cc30893703df 100644
--- a/clang/include/clang/Driver/CC1Options.td
+++ b/clang/include/clang/Driver/CC1Options.td
@@ -388,8 +388,6 @@ def flto_visibility_public_std:
 def flto_unit: Flag<["-"], "flto-unit">,
 HelpText<"Emit IR to support LTO unit features (CFI, whole program vtable 
opt)">;
 def fno_lto_unit: Flag<["-"], "fno-lto-unit">;
-def femit_debug_entry_values : Flag<["-"], "femit-debug-entry-values">,
-HelpText<"Enables debug info about call site parameter's entry values">;
 def fdebug_pass_manager : Flag<["-"], "fdebug-pass-manager">,
 HelpText<"Prints debug information for the new pass m

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

A more principled way to make this work would be to intercept (record) the 
Host::FindProcesses api. That way other functionality pertaining to running 
processes (e.g. the "platform process list" command) would also work. But if 
this is all you care about right now, then maybe this is fine...

The part that worries me more is the test. There are a lot of subtleties 
involved in making attach tests (and attach-by-name tests in particular) work 
reliably everywhere. I think this should be a dotest test, as there we already 
have some machinery to do these kinds of things (`lldb_enable_attach`, 
`wait_for_file_on_target`), and python is generally much better at complex 
control flow (I am very much against subshells and background processes in lit 
tests). Reproducers make this somewhat complicated because you cannot use the 
liblldb instance already loaded into the python process. But maybe you could 
run lldb in a subprocess similar to how the pexpect tests do it?




Comment at: lldb/test/Shell/Reproducer/Inputs/sleep.c:5
+int main(int argc, char const *argv[]) {
+  sleep(10);
+  return 0;

For attach to work reliably on linux, you need to ensure the process declares 
itself willing to be attached to. This is what the `lldb_enable_attach` macro 
in dotest inferiors does. Then you also need to ensure that the process has 
executed that statement before you attempt to attach. This is usually done via 
some pid file synchronization.



Comment at: lldb/test/Shell/Reproducer/TestAttach.test:7
+# RUN: %clang_host %S/Inputs/sleep.c -g -o %t/attach.out
+# RUN: python -c 'import os; os.system("%t/attach.out &")'
+

How is this different from a plain `%t/attach.out &` ?



Comment at: lldb/test/Shell/Reproducer/TestAttach.test:9
+
+# 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

Though normally determinism is good, in this case I think it is actually better 
to generate an unpredictable name for the process to avoid having the test be 
impacted by parallel test suite runs or leftover zombies. Other attach-by-name 
tests usually embed some a pid or a timestamp into the process name.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D75877



___
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.

In D75711#1912902 , @jingham wrote:

> In D75711#1912230 , @labath wrote:
>
> > So this is technically not "our" fault. The plugin is doing something 
> > unsupported and gets weird behavior as a result. Now, I am not saying we 
> > should throw this plugin (maybe the only actually used os plugin) 
> > overboard. However, I was hoping that the process of supporting it would 
> > involve some improvements to the os plugin interface (so that it can do 
> > what it wants in a supported way), rather than declaring support for 
> > whatever it is the plugin is doing, even though we know it to be 
> > problematic (like, not being able to tell when we can throw thread plans 
> > away).
>
>
> I think when presenting an OS's threads through the funnel of a monitor stub 
> that only knows about cores and not about the OS's concept of threads, you 
> will very likely run into the problem where reconstructing all the threads at 
> every stop is expensive and not particularly desirable.  Two of the three 
> instances of OS plugins that I know about do it this way.  The other is for 
> wrapping a cooperative multi-threading library, where fetching all the 
> threads was pretty easy and there weren't too many.  But for instance if 
> getting all the threads really is slow, for the OS plugin to be useful we 
> need to allow it to do partial reporting to be useful.
>
> Anyway, I don't know that we need anything more added to the OS interface.  
> There's a "make a thread for a TID" interface, which we can use to reap the 
> persistent part of the thread, for instance when we make a public stop.  If 
> that's fast enough (I haven't gotten to that point with the xnu plugin) we 
> can us that.  We can add an API to query whether the plugin returns all 
> threads, and gate our behavior on that rather than on "Has OS Plugin".  But 
> that's about all I can think of.


Yes, that's sort of what I was expecting. Another idea I had was to have a way 
to avoid reading the register context initially, and only have it be fetched on 
demand.

> I also have to support all the OS plugins in all the xnu dSYM's that are 
> currently in the wild, too.  So whatever we do has to work without requiring 
> added behaviors.

Yep, anything we do shouldn't break existing plugins, but I think its fair to 
require some modification to an existing plugin in order to enable a new 
feature (such as stepping across thread reschedules).

> 
> 
>> That said, I don't really use or care about the os plugins, so if using it 
>> results in leaking thread plans, then whatever. The thing I want to 
>> understand is what kind of requirements does this place on the rest of lldb. 
>> Which leads me to my next comment..
>> 
>>> but I think it fits the kernel folks workflow, since the actual number of 
>>> threads in the kernel is overwhelming and not terribly useful.  But you are 
>>> right, if we wanted to delay getting threads on private stops, but make 
>>> UpdateThreadListIfNeeded force a fetch, then we would have to add some API 
>>> to allow us to fetch only the "on core" threads along the code path that 
>>> handles internal stops.
>>> 
>>> In any case, in order to support having internal stops not fetch all 
>>> threads - which we are currently doing with OS Plugins - we need to be able 
>>> to persist the stateful data lldb holds for that thread across stops where 
>>> we don't realize the Thread object.  That is the sole ambition of this set 
>>> of patches.
>> 
>> We've agreed that we would like to avoid gathering the thread data for 
>> internals stops that don't need it. Avoiding creating (updating) 
>> lldb_private::Thread objects, and having the not-(yet)-realized threads live 
>> as a tid_t is one way to achieve this goal, but I am not sure if it is the 
>> right one. I am not really against the idea -- since threads can come and go 
>> between stops pretty much arbitrarily (even without os plugins), it may make 
>> sense to have everything hold onto them as simple handles, and force a fetch 
>> through some Process api when one needs to access them may simplify some 
>> things (for one, the lifetime of Threads could become much stricter).
>> 
>> I can certainly believe that changing thread plans to use a tid_t instead of 
>> a Thread& is simpler than making Thread object management lazy, but I am not 
>> sure that is a fair comparison. Thread plans are only a single component 
>> which juggles Thread objects. There are many other pieces of code, which 
>> assume that the lifetime of a Thread object matches that of the entity it 
>> describes. SBThread is one such thing. If SBThread keeps referring to the 
>> actual thread as a pointer, then it will lose track of it when the actual 
>> thread goes away and reappears. Same goes for ValueObjects and SBValues -- 
>> if lldb_private::Thread is meant to be transient, then those should also be 
>> up

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

As we've seen, I don't know much about thread plans so I don't have much to say 
on the higher-level aspects of this patch.

But I see a bunch of opportunities to make this more consistent with llvm code 
style.




Comment at: lldb/include/lldb/Target/ThreadPlanStack.h:9
+
+#ifndef liblldb_ThreadPlanStack_h_
+#define liblldb_ThreadPlanStack_h_

the include guards were recently normalized to LLDB_TARGET_THREADPLANSTACK_H



Comment at: lldb/include/lldb/Target/ThreadPlanStack.h:35
+public:
+  ThreadPlanStack(Thread &thread){};
+  ~ThreadPlanStack() {}

superfluous semicolon



Comment at: lldb/include/lldb/Target/ThreadPlanStack.h:113
+
+class ThreadPlanStackMap {
+public:

It's not clear to me what is the value of this class. Most of it is just about 
wrapping the underlying container in a slightly different way. If the 
ThreadDestroyed method can be called in the ThreadPlanStack destructor (which 
appears to be possible and would be a nice clean up regardless) then RemoveTID 
would boil down to `container.erase(tid)`. At that point the only non-trivial 
piece of functionality is the `Update` function (which this patch doesn't even 
implement), which could just as well be implemented outside of the class.



Comment at: lldb/source/Target/Process.cpp:1261
+return llvm::make_error(
+llvm::formatv("Invalid option with value '{0}'.", tid),
+llvm::inconvertibleErrorCode());

This error message doesn't really make sense for a function like this. What 
option is it talking about?

As the only reason this can fail is because there are no plans for the given 
tid, maybe you don't actually need the Expected, and can signal that with a 
simple nullptr?



Comment at: lldb/source/Target/Thread.cpp:505
   saved_state.current_inlined_depth = GetCurrentInlinedDepth();
-  saved_state.m_completed_plan_stack = m_completed_plan_stack;
+  saved_state.m_completed_plan_checkpoint =
+  GetPlans().CheckpointCompletedPlans();

What's the reason for the introduction of the "checkpoint" indirection? Does it 
have something to do with needing to update the checkpointed plans when the 
thread disappears?



Comment at: lldb/source/Target/Thread.cpp:1054-1058
+  llvm::Expected plans =
+  GetProcess()->FindThreadPlans(GetID());
+  if (!plans)
+llvm_unreachable("Thread left with an empty plan stack");
+  return **plans;

maybe `return cantFail(GetProcess()->FindThreadPlans(GetID()));`



Comment at: lldb/source/Target/Thread.cpp:1061
+
 void Thread::PushPlan(ThreadPlanSP &thread_plan_sp) {
+  if (!thread_plan_sp)

You don't modify the callers shared pointer => `const ThreadPlanSP &` or a 
`ThreadPlanSP` with a PushPlan(std::move(thread_plan_sp))`.



Comment at: lldb/source/Target/Thread.cpp:1062-1063
 void Thread::PushPlan(ThreadPlanSP &thread_plan_sp) {
-  if (thread_plan_sp) {
-// If the thread plan doesn't already have a tracer, give it its parent's
-// tracer:
-if (!thread_plan_sp->GetThreadPlanTracer()) {
-  assert(!m_plan_stack.empty());
-  thread_plan_sp->SetThreadPlanTracer(
-  m_plan_stack.back()->GetThreadPlanTracer());
-}
-m_plan_stack.push_back(thread_plan_sp);
+  if (!thread_plan_sp)
+llvm_unreachable("Don't push an empty thread plan.");
 

assert



Comment at: lldb/source/Target/ThreadPlanStack.cpp:72-73
+  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);

The usual way to write that is `assert(result == m_completed_plan_store.end() 
&& "Asked for a checkpoint that didn't exist")`. llvm_unreachable is used where 
an existing control flow path needs to be declared invalid. We don't introduce 
new control flow just for the sake of using it.



Comment at: lldb/source/Target/ThreadPlanStack.cpp:84-90
+  for (auto plan : m_plans)
+plan->ThreadDestroyed();
+
+  for (auto plan : m_discarded_plans)
+plan->ThreadDestroyed();
+
+  for (auto plan : m_completed_plans)

Please put the actual type (ThreadPlanSP ?) instead of `auto`.



Comment at: lldb/source/Target/ThreadPlanStack.cpp:126-127
+  // 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");
+

assert instead of branch-to-unreachable



Comment at: lldb/source/Target/ThreadPlanStack.cpp:373
+  }
+}

This is the usual place where one would add a llvm_unreachable. Some compilers 
do not consider the above switch to be fully covered (a

[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.

(This looks fine to me.)

In D75761#1913585 , @shafik wrote:

> 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.


This is a issue that has come up multiple times now. The presence of template 
parameters makes it very hard to use accelerator tables. I think it would be 
great if we could get rid of them.




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

shafik wrote:
> 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.
How is this lack of support "implemented"? Would it make sense to test that we 
do something reasonable (e.g. ignore it) if we do happen to run into it?

Given that the new version of this patch doesn't treat the spaceship operator 
specially, I don't think it needs to/should be a part of this patch, but it is 
something to think about...


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] [lldb] d00dff8 - [lldb] Make UnwindLLDB a non-plugin

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-03-10T13:56:15+01:00
New Revision: d00dff88b402ea9074b87aa5d3faddfd50c4bc0f

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

LOG: [lldb] Make UnwindLLDB a non-plugin

Summary:
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.

Reviewers: jasonmolenda, JDevlieghere, xiaobai

Subscribers: mgorny, lldb-commits

Tags: #lldb

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

Added: 
lldb/include/lldb/Target/RegisterContextUnwind.h
lldb/include/lldb/Target/UnwindLLDB.h
lldb/source/Target/RegisterContextUnwind.cpp
lldb/source/Target/UnwindLLDB.cpp

Modified: 
lldb/source/Plugins/Process/Utility/CMakeLists.txt
lldb/source/Target/CMakeLists.txt
lldb/source/Target/Thread.cpp

Removed: 
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



diff  --git a/lldb/source/Plugins/Process/Utility/RegisterContextLLDB.h 
b/lldb/include/lldb/Target/RegisterContextUnwind.h
similarity index 91%
rename from lldb/source/Plugins/Process/Utility/RegisterContextLLDB.h
rename to lldb/include/lldb/Target/RegisterContextUnwind.h
index 5bbc24c81c6a..6c91a649684e 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextLLDB.h
+++ b/lldb/include/lldb/Target/RegisterContextUnwind.h
@@ -1,5 +1,4 @@
-//===-- RegisterContextLLDB.h *- 
C++
-//-*-===//
+//===-- RegisterContextUnwind.h -*- C++ 
-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -7,32 +6,33 @@
 //
 
//===--===//
 
-#ifndef LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERCONTEXTLLDB_H
-#define LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERCONTEXTLLDB_H
+#ifndef LLDB_TARGET_REGISTERCONTEXTUNWIND_H
+#define LLDB_TARGET_REGISTERCONTEXTUNWIND_H
 
 #include 
 
-#include "UnwindLLDB.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Symbol/UnwindPlan.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/RegisterNumber.h"
+#include "lldb/Target/UnwindLLDB.h"
 #include "lldb/lldb-private.h"
 
 namespace lldb_private {
 
 class UnwindLLDB;
 
-class RegisterContextLLDB : public lldb_private::RegisterContext {
+class RegisterContextUnwind : public lldb_private::RegisterContext {
 public:
-  typedef std::shared_ptr SharedPtr;
+  typedef std::shared_ptr SharedPtr;
 
-  RegisterContextLLDB(lldb_private::Thread &thread, const SharedPtr 
&next_frame,
-  lldb_private::SymbolContext &sym_ctx,
-  uint32_t frame_number,
-  lldb_private::UnwindLLDB &unwind_lldb);
+  RegisterContextUnwind(lldb_private::Thread &thread,
+const SharedPtr &next_frame,
+lldb_private::SymbolContext &sym_ctx,
+uint32_t frame_number,
+lldb_private::UnwindLLDB &unwind_lldb);
 
-  ~RegisterContextLLDB() override = default;
+  ~RegisterContextUnwind() override = default;
 
   void InvalidateAllRegisters() override;
 
@@ -247,13 +247,11 @@ class RegisterContextLLDB : public 
lldb_private::RegisterContext {
   m_registers; // where to find reg values for this frame
 
   lldb_private::UnwindLLDB &m_parent_unwind; // The UnwindLLDB that is creating
- // this RegisterContextLLDB
+ // this RegisterContextUnwind
 
-  // For RegisterContextLLDB only
-
-  DISALLOW_COPY_AND_ASSIGN(RegisterContextLLDB);
+  DISALLOW_COPY_AND_ASSIGN(RegisterContextUnwind);
 };
 
 } // namespace lldb_private
 
-#endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERCONTEXTLLDB_H
+#endif // LLDB_TARGET_REGISTERCONTEXTUNWIND_H

diff  --git a/lldb/source/Plugins/

[Lldb-commits] [lldb] 1ca1e08 - [lldb] Break up CommandObjectDisassemble::DoExecute

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-03-10T14:03:16+01:00
New Revision: 1ca1e08e7544aea88d5978284a1c18086458d6c0

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

LOG: [lldb] Break up CommandObjectDisassemble::DoExecute

The function consisted of a complicated set of conditions to compute the
address ranges which are to be disassembled (depending on the mode
selected by command line switches). This patch creates a separate
function for each mode, so that DoExecute is only left with the task of
figuring out how to dump the relevant ranges.

This is NFC-ish, except for one change in the error message, which is
actually an improvement.

Added: 


Modified: 
lldb/source/Commands/CommandObjectDisassemble.cpp
lldb/source/Commands/CommandObjectDisassemble.h
lldb/test/Shell/Commands/command-disassemble.s

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectDisassemble.cpp 
b/lldb/source/Commands/CommandObjectDisassemble.cpp
index bd1c7a43afad..511cd6995404 100644
--- a/lldb/source/Commands/CommandObjectDisassemble.cpp
+++ b/lldb/source/Commands/CommandObjectDisassemble.cpp
@@ -214,6 +214,165 @@ CommandObjectDisassemble::CommandObjectDisassemble(
 
 CommandObjectDisassemble::~CommandObjectDisassemble() = default;
 
+llvm::Expected>
+CommandObjectDisassemble::GetContainingAddressRanges() {
+  std::vector ranges;
+  const auto &get_range = [&](Address addr) {
+ModuleSP module_sp(addr.GetModule());
+SymbolContext sc;
+bool resolve_tail_call_address = true;
+addr.GetModule()->ResolveSymbolContextForAddress(
+addr, eSymbolContextEverything, sc, resolve_tail_call_address);
+if (sc.function || sc.symbol) {
+  AddressRange range;
+  sc.GetAddressRange(eSymbolContextFunction | eSymbolContextSymbol, 0,
+ false, range);
+  ranges.push_back(range);
+}
+  };
+
+  Target &target = GetSelectedTarget();
+  if (!target.GetSectionLoadList().IsEmpty()) {
+Address symbol_containing_address;
+if (target.GetSectionLoadList().ResolveLoadAddress(
+m_options.symbol_containing_addr, symbol_containing_address)) {
+  get_range(symbol_containing_address);
+}
+  } else {
+for (lldb::ModuleSP module_sp : target.GetImages().Modules()) {
+  Address file_address;
+  if (module_sp->ResolveFileAddress(m_options.symbol_containing_addr,
+file_address)) {
+get_range(file_address);
+  }
+}
+  }
+
+  if (ranges.empty()) {
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"Could not find function bounds for address 0x%" PRIx64,
+m_options.symbol_containing_addr);
+  }
+  return ranges;
+}
+
+llvm::Expected>
+CommandObjectDisassemble::GetCurrentFunctionRanges() {
+  StackFrame *frame = m_exe_ctx.GetFramePtr();
+  if (!frame) {
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "Cannot disassemble around the current "
+   "function without a selected frame.\n");
+  }
+  SymbolContext sc(
+  frame->GetSymbolContext(eSymbolContextFunction | eSymbolContextSymbol));
+  AddressRange range;
+  if (sc.function)
+range = sc.function->GetAddressRange();
+  else if (sc.symbol && sc.symbol->ValueIsAddress()) {
+range = {sc.symbol->GetAddress(), sc.symbol->GetByteSize()};
+  } else
+range = {frame->GetFrameCodeAddress(), DEFAULT_DISASM_BYTE_SIZE};
+
+  return std::vector{range};
+}
+
+llvm::Expected>
+CommandObjectDisassemble::GetCurrentLineRanges() {
+  StackFrame *frame = m_exe_ctx.GetFramePtr();
+  if (!frame) {
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "Cannot disassemble around the current "
+   "line without a selected frame.\n");
+  }
+
+  LineEntry pc_line_entry(
+  frame->GetSymbolContext(eSymbolContextLineEntry).line_entry);
+  if (pc_line_entry.IsValid())
+return std::vector{pc_line_entry.range};
+
+  // No line entry, so just disassemble around the current pc
+  m_options.show_mixed = false;
+  return GetPCRanges();
+}
+
+llvm::Expected>
+CommandObjectDisassemble::GetNameRanges() {
+  ConstString name(m_options.func_name.c_str());
+  const bool include_symbols = true;
+  const bool include_inlines = true;
+
+  // Find functions matching the given name.
+  SymbolContextList sc_list;
+  GetSelectedTarget().GetImages().FindFunctions(
+  name, eFunctionNameTypeAuto, include_symbols, include_inlines, sc_list);
+
+  std::vector ranges;
+  AddressRange range;
+  const uint32_t scope =
+  eSymbolContextBlock | eSymbolContextFunction | eSymbolContextSymbol;
+  const bool use_inline_block_range = true;
+  for (Symb

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

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd00dff88b402: [lldb] Make UnwindLLDB a non-plugin (authored 
by labath).

Repository:
  rG LLVM Github Monorepo

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

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 @@
 //===--===//
 
 #include "lldb/Target/Thread.h"
-#include "Plugins/Process/Utility/UnwindLLDB.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/FormatEntity.h"
@@ -42,7 +41,7 @@
 #include "lldb/Target/ThreadPlanStepThrough.h"
 #include "lldb/Target/ThreadPlanStepUntil.h"
 #include "lldb/Target/ThreadSpec.h"
-#include "lldb/Target/Unwind.h"
+#include "lldb/Target/UnwindLLDB.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/State.h"
Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Targe

[Lldb-commits] [lldb] 6b37c47 - [lldb] Improve test failure messages in vscode tests

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-03-10T14:32:45+01:00
New Revision: 6b37c476a2d4e63f6c02ca8996e0e92ae3db3282

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

LOG: [lldb] Improve test failure messages in vscode tests

A couple of tests sporadically fail on these assertions, but the error
messages do not give a clue as to what has actually happened.

Improve them so that we can better understand what is going wrong.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
index 1eb23ce56212..54f09e2cdbee 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -230,11 +230,11 @@ def continue_to_exception_breakpoint(self, filter_label):
 def continue_to_exit(self, exitCode=0):
 self.vscode.request_continue()
 stopped_events = self.vscode.wait_for_stopped()
-self.assertTrue(len(stopped_events) == 1,
-"expecting single 'exited' event")
-self.assertTrue(stopped_events[0]['event'] == 'exited',
+self.assertEquals(len(stopped_events), 1,
+"stopped_events = {}".format(stopped_events))
+self.assertEquals(stopped_events[0]['event'], 'exited',
 'make sure program ran to completion')
-self.assertTrue(stopped_events[0]['body']['exitCode'] == exitCode,
+self.assertEquals(stopped_events[0]['body']['exitCode'], exitCode,
 'exitCode == %i' % (exitCode))
 
 def attach(self, program=None, pid=None, waitFor=None, trace=None,



___
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 planned changes to this revision.
JDevlieghere marked 2 inline comments as done.
JDevlieghere added a comment.

In D75877#1913959 , @labath wrote:

> A more principled way to make this work would be to intercept (record) the 
> Host::FindProcesses api. That way other functionality pertaining to running 
> processes (e.g. the "platform process list" command) would also work. But if 
> this is all you care about right now, then maybe this is fine...


We could totally add a provider for that. I didn't because it seemed like 
overkill but if you're on board I also prefer that over a random PID.

> The part that worries me more is the test. There are a lot of subtleties 
> involved in making attach tests (and attach-by-name tests in particular) work 
> reliably everywhere. I think this should be a dotest test, as there we 
> already have some machinery to do these kinds of things 
> (`lldb_enable_attach`, `wait_for_file_on_target`), and python is generally 
> much better at complex control flow (I am very much against subshells and 
> background processes in lit tests). Reproducers make this somewhat 
> complicated because you cannot use the liblldb instance already loaded into 
> the python process. But maybe you could run lldb in a subprocess similar to 
> how the pexpect tests do it?

The problem is the `SBDebugger::Initialize()` that's called from the SWIG 
bindings, as soon as you import lldb it's already too late for the reproducers. 
I'm working on being able to capture/replay the test suite and currently I have 
the following hack:

  if 'DOTEST_CAPTURE_PATH' in os.environ:
 SBReproducer.Capture(os.environ['DOTEST_CAPTURE_PATH'])
  SBDebugger.Initialize()

If you're fine with having that in `python.swig` unconditionally we could make 
a dotest-test work.




Comment at: lldb/test/Shell/Reproducer/TestAttach.test:7
+# RUN: %clang_host %S/Inputs/sleep.c -g -o %t/attach.out
+# RUN: python -c 'import os; os.system("%t/attach.out &")'
+

labath wrote:
> How is this different from a plain `%t/attach.out &` ?
Should that work? That's the first thing I tried and lit complained about 
invalid syntax.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D75877



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

[Lldb-commits] [PATCH] D75925: [lldb] reject `.debug_arange` sections with nonzero segment size

[Luke Drummond via Phabricator via lldb-commits]

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

  If a producer emits a nonzero segment size, `lldb` will silently read
  incorrect values and crash, or do something worse later, as the tuple
  size is expected to be 2, rather than 3.
  
  Neither LLVM, nor GCC produce segmented aranges, but this dangerous case
  should still be checked and handled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75925

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


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
@@ -63,7 +63,8 @@
   // 1 - the version looks good
   // 2 - the address byte size looks plausible
   // 3 - the length seems to make sense
-  // size looks plausible
+  // 4 - size looks plausible
+  // 5 - the arange tuples do not contain a segment field
   if (m_header.version < 2 || m_header.version > 5)
 return llvm::make_error(
 "Invalid arange header version");
@@ -81,6 +82,10 @@
 return llvm::make_error(
 "Invalid arange header length");
 
+  if (m_header.seg_size)
+return llvm::make_error(
+"segmented arange entries are not supported");
+
   // The first tuple following the header in each set begins at an offset
   // that is a multiple of the size of a single tuple (that is, twice the
   // size of an address). The header is padded, if necessary, to the


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
@@ -63,7 +63,8 @@
   // 1 - the version looks good
   // 2 - the address byte size looks plausible
   // 3 - the length seems to make sense
-  // size looks plausible
+  // 4 - size looks plausible
+  // 5 - the arange tuples do not contain a segment field
   if (m_header.version < 2 || m_header.version > 5)
 return llvm::make_error(
 "Invalid arange header version");
@@ -81,6 +82,10 @@
 return llvm::make_error(
 "Invalid arange header length");
 
+  if (m_header.seg_size)
+return llvm::make_error(
+"segmented arange entries are not supported");
+
   // The first tuple following the header in each set begins at an offset
   // that is a multiple of the size of a single tuple (that is, twice the
   // size of an address). The header is padded, if necessary, to the
___
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

[Adrian Prantl via Phabricator via lldb-commits]

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



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

JDevlieghere wrote:
> This won't trip if the tests doesn't run. If you make the assert trip and 
> XFAIL the test it should catch it. 
I'm not sure how that would work. Can you post an example? I want to test the 
skipIf decorator — how would I XFAIL 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] D75864: Add a decorator option to skip tests based on a default setting

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In D75864#1913876 , @labath wrote:

> If a test requires a specific value of a setting, would it make more sense to 
> just (re)set its value at the start of a test?


This is not meant for requiring a setting, but to run the testsuite once in 
normal mode, and then again with --setting 
"experimental-feature-that-doesn't-work-for-all-tests-yet=true" and have a way 
to mark tests as not yet supported with that feature.


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] D75864: Add a decorator option to skip tests based on a default setting

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

LGTM but let's give Pavel another chance to take a look.




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

aprantl wrote:
> JDevlieghere wrote:
> > This won't trip if the tests doesn't run. If you make the assert trip and 
> > XFAIL the test it should catch it. 
> I'm not sure how that would work. Can you post an example? I want to test the 
> skipIf decorator — how would I XFAIL it?
Yeah I think it has the same problem, skipping takes priority over XFAIL, so it 
wouldn't matter. 


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] D75877: [lldb/Reproducers] Fix replay for process attach workflows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D75877#1914755 , @JDevlieghere 
wrote:

> In D75877#1913959 , @labath wrote:
>
> > A more principled way to make this work would be to intercept (record) the 
> > Host::FindProcesses api. That way other functionality pertaining to running 
> > processes (e.g. the "platform process list" command) would also work. But 
> > if this is all you care about right now, then maybe this is fine...
>
>
> We could totally add a provider for that. I didn't because it seemed like 
> overkill but if you're on board I also prefer that over a random PID.


In general, I am in favor of doing the capture at the lowest level possible. 
For this particular feature/bug, it is overkill, but OTOH, this will also make 
it possible to support things other things without adding hacks into random 
pieces of code.

> 
> 
>> The part that worries me more is the test. There are a lot of subtleties 
>> involved in making attach tests (and attach-by-name tests in particular) 
>> work reliably everywhere. I think this should be a dotest test, as there we 
>> already have some machinery to do these kinds of things 
>> (`lldb_enable_attach`, `wait_for_file_on_target`), and python is generally 
>> much better at complex control flow (I am very much against subshells and 
>> background processes in lit tests). Reproducers make this somewhat 
>> complicated because you cannot use the liblldb instance already loaded into 
>> the python process. But maybe you could run lldb in a subprocess similar to 
>> how the pexpect tests do it?
> 
> The problem is the `SBDebugger::Initialize()` that's called from the SWIG 
> bindings, as soon as you import lldb it's already too late for the 
> reproducers. I'm working on being able to capture/replay the test suite and 
> currently I have the following hack:
> 
>   if 'DOTEST_CAPTURE_PATH' in os.environ:
>  SBReproducer.Capture(os.environ['DOTEST_CAPTURE_PATH'])
>   SBDebugger.Initialize()
> 
> 
> If you're fine with having that in `python.swig` unconditionally we could 
> make a dotest-test work.

I'm not sure how that would help because for the test you need to run lldb both 
in capture and replay mode, and I don't think you can currently do that within 
a single process. It would be cool if that was possible, but even then we'd 
have the impendance mismatch because we'd need to run `SBDebugger.Initialize` 
inside a specific test method, whereas normally it gets run much earlier.

That's why I was talking about subprocesses in the previous patch. The test 
would only be responsible for building the inferior and driving the whole 
thing, while capture/replay would happen inside separate processes:

  self.spawnSubproces(randomized_inferior_name, [token_path])
  lldbutil.wait_for_file_on_target(token_path)
  self.spawnSubprocess(lldbtest_config.lldbExec, ['--capture', reproducer, 
'-n', randomized_inferior_name, ...])
  ...
  self.spawnSubprocess(lldbtest_config.lldbExec, ['--replay', reproducer])




Comment at: lldb/test/Shell/Reproducer/TestAttach.test:7
+# RUN: %clang_host %S/Inputs/sleep.c -g -o %t/attach.out
+# RUN: python -c 'import os; os.system("%t/attach.out &")'
+

JDevlieghere wrote:
> labath wrote:
> > How is this different from a plain `%t/attach.out &` ?
> Should that work? That's the first thing I tried and lit complained about 
> invalid syntax.
I've seen tests do that (`RUN: setsid %run %t/LFSIGUSR -merge=1 
-merge_control_file=%t/MCF %t/C1 %t/C2 2>%t/log & export PID=$!` in  
`./compiler-rt/test/fuzzer/merge-sigusr.test`), but as I said, I don't think 
that is a good idea, so I don't really want to encourange it...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D75877



___
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

[Pavel Labath via Phabricator via lldb-commits]

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

In D75864#1914795 , @aprantl wrote:

> In D75864#1913876 , @labath wrote:
>
> > If a test requires a specific value of a setting, would it make more sense 
> > to just (re)set its value at the start of a test?
>
>
> This is not meant for requiring a setting, but to run the testsuite once in 
> normal mode, and then again with --setting 
> "experimental-feature-that-doesn't-work-for-all-tests-yet=true" and have a 
> way to mark tests as not yet supported with that feature.


Right, that makes sense, though it makes me cry a bit every time we add a new 
option to the test decorators...




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

JDevlieghere wrote:
> aprantl wrote:
> > JDevlieghere wrote:
> > > This won't trip if the tests doesn't run. If you make the assert trip and 
> > > XFAIL the test it should catch it. 
> > I'm not sure how that would work. Can you post an example? I want to test 
> > the skipIf decorator — how would I XFAIL it?
> Yeah I think it has the same problem, skipping takes priority over XFAIL, so 
> it wouldn't matter. 
At one point, Todd added some tests for the test harness. There are still some 
remnants of it in `packages/Python/lldbsuite/test/test_runner/` but they're 
hopelessly out of date by now (and even when they were new they only seemed to 
work for Todd).

If we wanted to write self-tests something like that would seem appropriate. 
I'm not sure these tests are very useful as they stand now because they could 
pass or fail depending on what kind of --setting values I pass to dotest.


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] D75877: [lldb/Reproducers] Fix replay for process attach workflows

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D75877#1914900 , @labath wrote:

> In D75877#1914755 , @JDevlieghere 
> wrote:
>
> > In D75877#1913959 , @labath wrote:
> >
> > > A more principled way to make this work would be to intercept (record) 
> > > the Host::FindProcesses api. That way other functionality pertaining to 
> > > running processes (e.g. the "platform process list" command) would also 
> > > work. But if this is all you care about right now, then maybe this is 
> > > fine...
> >
> >
> > We could totally add a provider for that. I didn't because it seemed like 
> > overkill but if you're on board I also prefer that over a random PID.
>
>
> In general, I am in favor of doing the capture at the lowest level possible. 
> For this particular feature/bug, it is overkill, but OTOH, this will also 
> make it possible to support things other things without adding hacks into 
> random pieces of code.
>
> > 
> > 
> >> The part that worries me more is the test. There are a lot of subtleties 
> >> involved in making attach tests (and attach-by-name tests in particular) 
> >> work reliably everywhere. I think this should be a dotest test, as there 
> >> we already have some machinery to do these kinds of things 
> >> (`lldb_enable_attach`, `wait_for_file_on_target`), and python is generally 
> >> much better at complex control flow (I am very much against subshells and 
> >> background processes in lit tests). Reproducers make this somewhat 
> >> complicated because you cannot use the liblldb instance already loaded 
> >> into the python process. But maybe you could run lldb in a subprocess 
> >> similar to how the pexpect tests do it?
> > 
> > The problem is the `SBDebugger::Initialize()` that's called from the SWIG 
> > bindings, as soon as you import lldb it's already too late for the 
> > reproducers. I'm working on being able to capture/replay the test suite and 
> > currently I have the following hack:
> > 
> >   if 'DOTEST_CAPTURE_PATH' in os.environ:
> >  SBReproducer.Capture(os.environ['DOTEST_CAPTURE_PATH'])
> >   SBDebugger.Initialize()
> > 
> > 
> > If you're fine with having that in `python.swig` unconditionally we could 
> > make a dotest-test work.
>
> I'm not sure how that would help because for the test you need to run lldb 
> both in capture and replay mode, and I don't think you can currently do that 
> within a single process. It would be cool if that was possible, but even then 
> we'd have the impendance mismatch because we'd need to run 
> `SBDebugger.Initialize` inside a specific test method, whereas normally it 
> gets run much earlier.
>
> That's why I was talking about subprocesses in the previous patch. The test 
> would only be responsible for building the inferior and driving the whole 
> thing, while capture/replay would happen inside separate processes:


Ah, I misunderstood subprocess as another Python process, yeah launching the 
driver should work. Thanks for the clarification.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D75877



___
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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 249430.
shafik added a comment.

Move to using `expect_expr` in the test.


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,39 @@
 
 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)", result_type="int", result_value="42")
+else:
+  self.expect_expr("foo(42)", result_type="int", result_value="42")
+
+  self.expect_expr("h(10)", result_type="int", result_value="10")
+
+  self.expect_expr("f(A::C{})", result_type="int", result_value="4")
+
+  self.expect_expr("g(A::C{})", result_type="int", result_value="4")
+
+  self.expect_expr("var(1)", result_type="int", result_value="10")
+
+  self.expect_expr("var(1,2)", result_type="int", result_value="10")
+
+  self.expect_expr("b1 > b2", result_type="bool", result_value="true")
+
+  self.expect_expr("b1 >> b2", result_type="bool", result_value="true")
+
+  self.expect_expr("b1 << b2", result_type="bool", result_value="true")
+
+  self.expect_expr("b1 == b2", result_type="bool", result_value="true")
+
+  self.expect_expr("c1 > c2", result_type="bool", result_value="true")
+
+  self.expect_expr("c1 >> c2", result_type="bool", result_value="true")
+
+  self.expect_expr("c1 << c2", result_type="bool", result_value="true")
+
+  self.expect_expr("c1 == c2", result_type="bool", result_value="true")
 
 @skipIfWindows
 def test_template_function_with_cast(self):
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.c

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I haven't digested the patch yet, but I am wondering if you've seen the recent 
discussion (`DWP mixed (DWARFv4/pre-standard + DWARFv5) content`) on 
dwarf-discuss (link1 
,
 link2 
),
 which is very relevant for this patch.

If you have any opinions on that, it's not too late to join in. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75929



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.
Herald added a subscriber: danielkiss.

This broke compiling for mingw, repro.c:

  a(short);
  b() { a(1); }

`clang -target x86_64-w64-mingw32 -c repro.c -g -O2`, which gives `Assertion 
`!MI.isMoveImmediate() && "Unexpected MoveImm instruction"' failed.`


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

https://reviews.llvm.org/D73534



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

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

[Igor Kudrin via Phabricator via lldb-commits]

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

DWARFv5 defines index sections in package files in a slightly different way 
than the pre-standard GNU proposal, see Section 7.3.5 in the DWARF standard and 
https://gcc.gnu.org/wiki/DebugFissionDWP for GNU proposal. The main concern 
here is values for section identifiers, which are partially overlapped with 
changed meanings. The patch adds support for v5 index sections and resolves 
that difficulty by defining a set of identifiers for internal use which can 
represent and distinct values of both standards.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75929

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
  llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
  llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
  llvm/test/DebugInfo/X86/dwp-v2-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v2-loc.s
  llvm/test/DebugInfo/X86/dwp-v2-tu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-loclists.s
  llvm/test/DebugInfo/X86/dwp-v5-tu-index.s
  llvm/test/tools/llvm-dwp/X86/unsupported_cu_index_version.s
  llvm/tools/llvm-dwp/llvm-dwp.cpp

Index: llvm/tools/llvm-dwp/llvm-dwp.cpp
===
--- llvm/tools/llvm-dwp/llvm-dwp.cpp
+++ llvm/tools/llvm-dwp/llvm-dwp.cpp
@@ -214,6 +214,19 @@
   StringRef DWPName;
 };
 
+// Convert an internal section identifier into the index to use with
+// UnitIndexEntry::Contributions.
+static unsigned SectionKindToIndex(DWARFSectionKind Kind) {
+  // Assuming pre-standard DWP format.
+  return SerializeSectionKind(Kind, 2) - 1;
+}
+
+// Convert a UnitIndexEntry::Contributions index to the corresponding on-disk
+// value of the section identifier. 
+static unsigned IndexToOnDiskSectionId(unsigned Index) {
+  return Index + 1;
+}
+
 static StringRef getSubsection(StringRef Section,
const DWARFUnitIndex::Entry &Entry,
DWARFSectionKind Kind) {
@@ -239,15 +252,15 @@
 // Zero out the debug_info contribution
 Entry.Contributions[0] = {};
 for (auto Kind : TUIndex.getColumnKinds()) {
-  auto &C = Entry.Contributions[Kind - DW_SECT_INFO];
+  auto &C = Entry.Contributions[SectionKindToIndex(Kind)];
   C.Offset += I->Offset;
   C.Length = I->Length;
   ++I;
 }
-auto &C = Entry.Contributions[DW_SECT_TYPES - DW_SECT_INFO];
+const unsigned TypesIndex = SectionKindToIndex(DW_SECT_GNU_TYPES);
+auto &C = Entry.Contributions[TypesIndex];
 Out.emitBytes(Types.substr(
-C.Offset - TUEntry.Contributions[DW_SECT_TYPES - DW_SECT_INFO].Offset,
-C.Length));
+C.Offset - TUEntry.Contributions[TypesIndex].Offset, C.Length));
 C.Offset = TypesOffset;
 TypesOffset += C.Length;
   }
@@ -266,7 +279,7 @@
   UnitIndexEntry Entry = CUEntry;
   // Zero out the debug_info contribution
   Entry.Contributions[0] = {};
-  auto &C = Entry.Contributions[DW_SECT_TYPES - DW_SECT_INFO];
+  auto &C = Entry.Contributions[SectionKindToIndex(DW_SECT_GNU_TYPES)];
   C.Offset = TypesOffset;
   auto PrevOffset = Offset;
   // Length of the unit, including the 4 byte length field.
@@ -343,7 +356,7 @@
   // Write the column headers (which sections will appear in the table)
   for (size_t i = 0; i != ContributionOffsets.size(); ++i)
 if (ContributionOffsets[i])
-  Out.emitIntValue(i + DW_SECT_INFO, 4);
+  Out.emitIntValue(IndexToOnDiskSectionId(i), 4);
 
   // Write the offsets.
   writeIndexTable(Out, ContributionOffsets, IndexEntries,
@@ -436,8 +449,8 @@
 return Error::success();
 
   if (DWARFSectionKind Kind = SectionPair->second.second) {
-auto Index = Kind - DW_SECT_INFO;
-if (Kind != DW_SECT_TYPES) {
+auto Index = SectionKindToIndex(Kind);
+if (Kind != DW_SECT_GNU_TYPES) {
   CurEntry.Contributions[Index].Offset = ContributionOffsets[Index];
   ContributionOffsets[Index] +=
   (CurEntry.Contributions[Index].Length = Contents.size());
@@ -521,10 +534,10 @@
   MCSection *const TUIndexSection = MCOFI.getDwarfTUIndexSection();
   const StringMap> KnownSections = {
   {"debug_info.dwo", {MCOFI.getDwarfInfoDWOSection(), DW_SECT_INFO}},
-  {"debug_types.dwo", {MCOFI.getDwarfTypesDWOSection(), DW_SECT_TYPES}},
+  {"debug_types.dwo", {MCOFI.getDwarfTypesDWOSection(), DW_SECT_GNU_TYPES}},
   {"debug_str_offsets.dwo", {StrOffsetSection, DW_SECT_STR_OFFSETS}},
   {"debug_str.dwo", {StrSection, static_cast(0)}},
-  {"debug_lo

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Hsiangkai Wang via Phabricator via lldb-commits]

HsiangKai updated this revision to Diff 249024.
HsiangKai added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Move llvm-readobj changes and tests to D75833 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023

Files:
  lld/ELF/InputFiles.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/include/llvm/Support/ARMAttributeParser.h
  llvm/include/llvm/Support/ARMBuildAttributes.h
  llvm/include/llvm/Support/ELFAttributeParser.h
  llvm/include/llvm/Support/ELFAttributes.h
  llvm/include/llvm/Support/RISCVAttributeParser.h
  llvm/include/llvm/Support/RISCVAttributes.h
  llvm/lib/Object/ELF.cpp
  llvm/lib/Object/ELFObjectFile.cpp
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/lib/Support/ARMAttributeParser.cpp
  llvm/lib/Support/ARMBuildAttrs.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/ELFAttributeParser.cpp
  llvm/lib/Support/ELFAttributes.cpp
  llvm/lib/Support/RISCVAttributeParser.cpp
  llvm/lib/Support/RISCVAttributes.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/attribute-with-insts.s
  llvm/test/MC/RISCV/attribute.s
  llvm/test/MC/RISCV/invalid-attribute.s
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/unittests/Support/ARMAttributeParser.cpp
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Support/ELFAttributeParser.cpp

Index: llvm/unittests/Support/ELFAttributeParser.cpp
===
--- /dev/null
+++ llvm/unittests/Support/ELFAttributeParser.cpp
@@ -0,0 +1,40 @@
+#include "llvm/Support/ELFAttributeParser.h"
+#include "llvm/Support/ARMAttributeParser.h"
+#include "llvm/Support/ELFAttributes.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace llvm;
+
+static void testParseError(ArrayRef bytes, const char *msg) {
+  ARMAttributeParser parser;
+  Error e = parser.parse(bytes, support::little);
+  EXPECT_STREQ(toString(std::move(e)).c_str(), msg);
+}
+
+TEST(ARMAttributeParser, UnrecognizedFormatVersion) {
+  static const uint8_t bytes[] = {1};
+  testParseError(bytes, "unrecognized format-version: 0x1");
+}
+
+TEST(ARMAttributeParser, InvalidSubsectionLength) {
+  static const uint8_t bytes[] = {'A', 3, 0, 0, 0};
+  testParseError(bytes, "invalid subsection length 3 at offset 0x1");
+}
+
+TEST(ARMAttributeParser, UnrecognizedVendorName) {
+  static const uint8_t bytes[] = {'A', 7, 0, 0, 0, 'x', 'y', 0};
+  testParseError(bytes, "unrecognized vendor-name: xy");
+}
+
+TEST(ARMAttributeParser, UnrecognizedTag) {
+  static const uint8_t bytes[] = {'A', 15,  0, 0, 0, 'a', 'e', 'a',
+  'b', 'i', 0, 4, 5, 0,   0,   0};
+  testParseError(bytes, "unrecognized tag 0x4 at offset 0xb");
+}
+
+TEST(ARMAttributeParser, InvalidAttributeSize) {
+  static const uint8_t bytes[] = {'A', 15,  0, 0, 0, 'a', 'e', 'a',
+  'b', 'i', 0, 1, 4, 0,   0,   0};
+  testParseError(bytes, "invalid attribute size 4 at offset 0xb");
+}
Index: llvm/unittests/Support/CMakeLists.txt
===
--- llvm/unittests/Support/CMakeLists.txt
+++ llvm/unittests/Support/CMakeLists.txt
@@ -28,6 +28,7 @@
   DJBTest.cpp
   EndianStreamTest.cpp
   EndianTest.cpp
+  ELFAttributeParser.cpp
   ErrnoTest.cpp
   ErrorOrTest.cpp
   ErrorTest.cpp
Index: llvm/unittests/Support/ARMAttributeParser.cpp
===
--- llvm/unittests/Support/ARMAttributeParser.cpp
+++ llvm/unittests/Support/ARMAttributeParser.cpp
@@ -1,5 +1,6 @@
 #include "llvm/Support/ARMAttributeParser.h"
 #include "llvm/Support/ARMBuildAttributes.h"
+#include "llvm/Support/ELFAttributes.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -36,8 +37,8 @@
   ARMAttributeParser Parser;
   cantFail(Parser.parse(Bytes, support::little));
 
-  return (Parser.hasAttribute(ExpectedTag) &&
-Parser.getAttributeValue(ExpectedTag) == ExpectedValue);
+  Optional Attr = Parser.getAttributeValue(ExpectedTag);
+  return Attr.hasValue() && Attr.getValue() == ExpectedValue;
 }
 
 void testParseError(ArrayRef bytes, const char *msg) {
@@ -47,34 +48,8 @@
 }
 
 bool testTagString(unsigned Tag, const char *name) {
-  return ARMBuildAttrs::AttrTypeAsString(Tag).str() == name;
-}
-
-TEST(ARMAttributeParser, UnrecognizedFormatVersion) {
-  static

[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#1914001 , @labath wrote:

> In D75711#1912902 , @jingham wrote:
>
> > In D75711#1912230 , @labath wrote:
> >
> > > So this is technically not "our" fault. The plugin is doing something 
> > > unsupported and gets weird behavior as a result. Now, I am not saying we 
> > > should throw this plugin (maybe the only actually used os plugin) 
> > > overboard. However, I was hoping that the process of supporting it would 
> > > involve some improvements to the os plugin interface (so that it can do 
> > > what it wants in a supported way), rather than declaring support for 
> > > whatever it is the plugin is doing, even though we know it to be 
> > > problematic (like, not being able to tell when we can throw thread plans 
> > > away).
> >
> >
> > I think when presenting an OS's threads through the funnel of a monitor 
> > stub that only knows about cores and not about the OS's concept of threads, 
> > you will very likely run into the problem where reconstructing all the 
> > threads at every stop is expensive and not particularly desirable.  Two of 
> > the three instances of OS plugins that I know about do it this way.  The 
> > other is for wrapping a cooperative multi-threading library, where fetching 
> > all the threads was pretty easy and there weren't too many.  But for 
> > instance if getting all the threads really is slow, for the OS plugin to be 
> > useful we need to allow it to do partial reporting to be useful.
> >
> > Anyway, I don't know that we need anything more added to the OS interface.  
> > There's a "make a thread for a TID" interface, which we can use to reap the 
> > persistent part of the thread, for instance when we make a public stop.  If 
> > that's fast enough (I haven't gotten to that point with the xnu plugin) we 
> > can us that.  We can add an API to query whether the plugin returns all 
> > threads, and gate our behavior on that rather than on "Has OS Plugin".  But 
> > that's about all I can think of.
>
>
> Yes, that's sort of what I was expecting. Another idea I had was to have a 
> way to avoid reading the register context initially, and only have it be 
> fetched on demand.
>
> > I also have to support all the OS plugins in all the xnu dSYM's that are 
> > currently in the wild, too.  So whatever we do has to work without 
> > requiring added behaviors.
>
> Yep, anything we do shouldn't break existing plugins, but I think its fair to 
> require some modification to an existing plugin in order to enable a new 
> feature (such as stepping across thread reschedules).
>
> > 
> > 
> >> That said, I don't really use or care about the os plugins, so if using it 
> >> results in leaking thread plans, then whatever. The thing I want to 
> >> understand is what kind of requirements does this place on the rest of 
> >> lldb. Which leads me to my next comment..
> >> 
> >>> but I think it fits the kernel folks workflow, since the actual number of 
> >>> threads in the kernel is overwhelming and not terribly useful.  But you 
> >>> are right, if we wanted to delay getting threads on private stops, but 
> >>> make UpdateThreadListIfNeeded force a fetch, then we would have to add 
> >>> some API to allow us to fetch only the "on core" threads along the code 
> >>> path that handles internal stops.
> >>> 
> >>> In any case, in order to support having internal stops not fetch all 
> >>> threads - which we are currently doing with OS Plugins - we need to be 
> >>> able to persist the stateful data lldb holds for that thread across stops 
> >>> where we don't realize the Thread object.  That is the sole ambition of 
> >>> this set of patches.
> >> 
> >> We've agreed that we would like to avoid gathering the thread data for 
> >> internals stops that don't need it. Avoiding creating (updating) 
> >> lldb_private::Thread objects, and having the not-(yet)-realized threads 
> >> live as a tid_t is one way to achieve this goal, but I am not sure if it 
> >> is the right one. I am not really against the idea -- since threads can 
> >> come and go between stops pretty much arbitrarily (even without os 
> >> plugins), it may make sense to have everything hold onto them as simple 
> >> handles, and force a fetch through some Process api when one needs to 
> >> access them may simplify some things (for one, the lifetime of Threads 
> >> could become much stricter).
> >> 
> >> I can certainly believe that changing thread plans to use a tid_t instead 
> >> of a Thread& is simpler than making Thread object management lazy, but I 
> >> am not sure that is a fair comparison. Thread plans are only a single 
> >> component which juggles Thread objects. There are many other pieces of 
> >> code, which assume that the lifetime of a Thread object matches that of 
> >> the entity it describes. SBThread is one such thing. If SBThread keeps 
> >> referring to

[Lldb-commits] [PATCH] D75626: Add support for owning module information to TypeSystemClang.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I'm not a big fan of storing data in the `ClangExternalASTSourceCallbacks`. The 
main reason is that this external source only exists when the ASTContext in the 
TypeSystemClang is created by the TypeSystemClang. When the TypeSystem adopts 
an ASTContext there can be any (or none) ExternalASTSource so 
`GetOrCreateClangModule` would crash/assert. `ClangExternalASTSourceCallbacks` 
always knows its respective `TypeSystemClang`, so storing the data in 
`TypeSystemClang` and then getting it from there when 
`getSourceDescriptor`/`getModule` is called will always work (and saves you 
from RTTI-casting/checking the ExternalASTSource).




Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:1211
+*m_source_manager_up, *m_diagnostics_engine_up, *m_language_options_up,
+m_target_info_up.get(), *m_header_search_up);
+  }

All the `m_*` variables that correspond to a part of ASTContext are only 
initialized when the TypeSystemClang has created its own ASTContext, so this 
code would crash if that's not the case. Calling `getASTContext().getX()` is 
the way that always works.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:310
   CompilerType CreateRecordType(clang::DeclContext *decl_ctx,
+unsigned owning_module,
 lldb::AccessType access_type,

Could we change this (and all similar parameters) from an `unsigned` to at one 
of those:
* A typedef such as `typedef unsigned ModuleID`. It seems to be optional, so 
`llvm::Optional` would be even better so I could pass `llvm::None` 
instead of `0`.
* (preferred) A new `ModuleID` type that doesn't implicitly construct from 
`unsigned`. Many of these functions already take some kind of integer and I 
would prefer if we don't add more.

That would also make the `GetOrCreateClangModule` self-documenting.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:500
+  /// Set the owning module for \p decl.
+  static void SetOwningModule(clang::Decl *decl, unsigned owning_module);
+

Could this be declared next to `GetOrCreateClangModule`? This is currently 
declared in the middle of the CompilerDeclContext backend (which no longer 
makes so much sense with the new version of the patch).



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:1021
+  clang::SourceManager *GetSourceMgr() const {
+return m_source_manager_up.get();
+  }

The correct way to get the SourceManager and LangOpts is to do 
`getASTContext().getSourceManager()`. `m_language_options_up`, 
`m_source_manager_up`etc.  are only set if the TypeSystemClang created its own 
ASTContext but are null if the TypeSystemClang adopted an ASTContext (e.g., in 
an expression).

This can also return a reference instead of a pointer. I removed the option to 
have an empty TypeSystemClang.


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

https://reviews.llvm.org/D75626



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

[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added inline comments.



Comment at: lldb/include/lldb/Symbol/Type.h:198
   uint32_t GetEncodingMask();
-
-  bool IsCompleteObjCClass() { return m_is_complete_objc_class; }
-
-  void SetIsCompleteObjCClass(bool is_complete_objc_class) {
-m_is_complete_objc_class = is_complete_objc_class;
-  }
+  uint32_t &GetPayload() { return m_payload; }
 

Remove reference?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h:91
   DeclContextToDIEMap m_decl_ctx_to_die;
+  DIEToModuleMap m_die_to_module;
   std::unique_ptr m_clang_ast_importer_up;

Maybe I'm missing something but this is `DIEToModuleMap` but it seems the value 
in this map is actually a module **ID** and not a module **map**?


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

https://reviews.llvm.org/D75488



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

[Lldb-commits] [PATCH] D75925: [lldb] reject `.debug_arange` sections with nonzero segment size

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Change looks good, just needs a test. Should be easy to take a simple binary 
that has a .debug_aranges, and run obj2yaml on it, and tweak the segment size 
as needed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75925



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

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

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

In D75929#1915009 , @labath wrote:

> I haven't digested the patch yet, but I am wondering if you've seen the 
> recent discussion (`DWP mixed (DWARFv4/pre-standard + DWARFv5) content`) on 
> dwarf-discuss (link1 
> ,
>  link2 
> ),
>  which is very relevant for this patch.
>
> If you have any opinions on that, it's not too late to join in. :)


+1 to that.

If not joining the conversation - the summary is basically: To support v4 and 
v5 units in a single DWP, extend the v5 index format with new-old columns: 
DW_SECT_LOC and 9 and DW_SECT_MACINFO at 10.

So probably keep the authoritative enum as this extended version of v5 - 
including DW_SECT_TYPES and 2, LOC and 9, and MACINFO at 10. And emit this 
extended index format any time the inputs contain at least one v5 unit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75929



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

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:1265
+  CXXRecordDecl *decl = CXXRecordDecl::CreateDeserialized(ast, 0);
+  decl->setTagKind((TagDecl::TagKind)kind);
+  decl->setDeclContext(decl_ctx);

`static_cast(kind)`



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:1448
+  CXXRecordDecl *template_cxx_decl = CXXRecordDecl::CreateDeserialized(ast, 0);
+  template_cxx_decl->setTagKind((TagDecl::TagKind)kind);
+  // What decl context do we use here? TU? The actual decl context?

`static_cast(kind)`



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:1966
+  func_decl->setType(ClangUtil::GetQualType(function_clang_type));
+  func_decl->setStorageClass((clang::StorageClass)storage);
+  func_decl->setInlineSpecified(is_inline);

`static_cast(storage)`



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:2028
+  decl->setType(ClangUtil::GetQualType(param_type));
+  decl->setStorageClass((clang::StorageClass)storage);
   SetOwningModule(decl, owning_module);

`static_cast(storage)`


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] [PATCH] D75715: Switch TypeSystemClang over to CreateDeserialized() (NFC)

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

A bunch of small comments but a few more serious ones as well.




Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:1394
+  func_tmpl_decl->setDeclName(func_decl->getDeclName());
+  func_tmpl_decl->init(func_decl, template_param_list);
   SetOwningModule(func_tmpl_decl, owning_module);

It does not look like `func_decl->getLocation()` get used at all.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:1526
+  ast, 0);
+  class_template_specialization_decl->setTagKind((TagDecl::TagKind)kind);
+  class_template_specialization_decl->setDeclContext(decl_ctx);

`static_cast(kind)`



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:1836
+  var_decl->setDeclName(&getASTContext().Idents.getOwn(name));
+var_decl->setType(type);
 SetOwningModule(var_decl, owning_module);

I guess `clang::SC_None` is the default?



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7268
+cxx_ctor_decl = clang::CXXConstructorDecl::CreateDeserialized(
+getASTContext(), 0, is_explicit ? 1 << 1 : 0);
+cxx_ctor_decl->setDeclContext(cxx_record_decl);

Where does `1 << 1` come from? What does it mean? 



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7999
+  enumerator_decl->setDeclContext(enutype->getDecl());
+  if (name)
+enumerator_decl->setDeclName(&getASTContext().Idents.get(name));

Everywhere else we do:

```
if (name && name[0])
```

Why not for this case?


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] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

[Jim Ingham via Phabricator via lldb-commits]

jingham marked 9 inline comments as done.
jingham added a comment.

Addressed Pavel's review comments.




Comment at: lldb/include/lldb/Target/ThreadPlanStack.h:113
+
+class ThreadPlanStackMap {
+public:

labath wrote:
> It's not clear to me what is the value of this class. Most of it is just 
> about wrapping the underlying container in a slightly different way. If the 
> ThreadDestroyed method can be called in the ThreadPlanStack destructor (which 
> appears to be possible and would be a nice clean up regardless) then 
> RemoveTID would boil down to `container.erase(tid)`. At that point the only 
> non-trivial piece of functionality is the `Update` function (which this patch 
> doesn't even implement), which could just as well be implemented outside of 
> the class.
Mostly because if not here then all this logic goes in Process and that is 
already pretty huge.  This isn't a class that we will turn around and wire the 
methods through process, so I think it helps keep a little more organization.

The update task is trivial at this point because there aren't actually any plan 
stacks that outlive their threads, but it will grow as we decide when we want 
to do about this.  

I also plan (though I haven't done it yet) to move thread plan 
listing/deleting/etc to go through here.  Right now the "thread plan list" & 
Co. commands go through the thread list to individual threads, but that means 
you can't get at thread plan stacks for unreported threads, which doesn't seem 
right.  That belongs to the map of threads.



Comment at: lldb/source/Target/Process.cpp:1261
+return llvm::make_error(
+llvm::formatv("Invalid option with value '{0}'.", tid),
+llvm::inconvertibleErrorCode());

labath wrote:
> This error message doesn't really make sense for a function like this. What 
> option is it talking about?
> 
> As the only reason this can fail is because there are no plans for the given 
> tid, maybe you don't actually need the Expected, and can signal that with a 
> simple nullptr?
I originally had this as a pointer that returns NULL when not found, but I 
thought we were going towards Expected's for everything that might not produce 
a value.  I'm happier with it as just a pointer, so I put it back to that.



Comment at: lldb/source/Target/Thread.cpp:505
   saved_state.current_inlined_depth = GetCurrentInlinedDepth();
-  saved_state.m_completed_plan_stack = m_completed_plan_stack;
+  saved_state.m_completed_plan_checkpoint =
+  GetPlans().CheckpointCompletedPlans();

labath wrote:
> What's the reason for the introduction of the "checkpoint" indirection? Does 
> it have something to do with needing to update the checkpointed plans when 
> the thread disappears?
Partly.  

This is actually a feature that was present but more or less buried in the old 
way of doing things.  The problem it addresses is:

1) You stop at the end of a step over
2) You run an expression, which also stops because it hit the breakpoint at 
_start
3) The user does "thread list".

What do you show as the stop reason for the thread?  The answer is "step over" 
not "expression completed".  This is not as simple as it seems, since the 
expression evaluation could push lots of plans on the stack.  For instance, the 
expression evaluation could hit a breakpoint, the user could step around a bit, 
run some more function, etc.  But when the expression evaluation eventually 
unwinds, we need to restore the original "step over" as the completed plan that 
gets reported as the "completed plan for this stop."

This part of the job (you also needed to update a few other things) used to be 
managed in the Thread separate from the rest of the SavedState.  That wasn't 
very clean, and since Threads might go away is also not safe.  I moved it into 
the saved state first because you have to if the Thread might go away, but also 
because it makes more sense where it is.



Comment at: lldb/source/Target/ThreadPlanStack.cpp:373
+  }
+}

labath wrote:
> This is the usual place where one would add a llvm_unreachable. Some 
> compilers do not consider the above switch to be fully covered (and if we are 
> to be fully pedantic, it isn't) and will complain about falling off of the 
> end of a non-void function.
I added a default, but what do you mean by "if we are to be fully pedantic, it 
isn't"?  It's an enum with 3 cases, all of which are listed.  How is that not 
fully covered?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75880



___
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 249515.
jingham marked 3 inline comments as done.
jingham added a comment.

Addressed Pavel's review comments.


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,381 @@
+//===-- 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);
+  assert(result != m_completed_plan_store.end()
+ && "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 (ThreadPlanSP plan : m_plans)
+plan->ThreadDestroyed();
+
+  for (ThreadPlanSP plan : m_discarded_plans)
+plan->ThreadDestroyed();
+
+  for (ThreadPlanSP 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 (ThreadPlanSP plan : m_plans) {
+if (plan->GetThreadPlanTracer()) {
+  plan->GetThreadPlanTracer()->EnableTracing(value);
+  plan->GetThreadPlanTracer()->EnableSingleStep(single_stepping);
+}
+  }
+}
+
+void ThreadPlanStack::SetTracer(lldb::ThreadPlanTracerSP &tracer_sp) {
+  for (ThreadPlanSP 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:
+  assert (m_plans.si

[Lldb-commits] [PATCH] D75406: Avoid including FileManager.h from SourceManager.h

[Reid Kleckner via Phabricator via lldb-commits]

rnk added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75406



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

[Lldb-commits] [PATCH] D75784: Avoid including Module.h from ExternalASTSource.h

[Reid Kleckner via Phabricator via lldb-commits]

rnk added a comment.

In D75784#1910688 , @aprantl wrote:

> This will no doubt also need some patches to the Swift compiler, but given 
> the NFC-ness this hopefully should be fine.


True. I could leave behind a typedef for the ASTSourceDescriptor if it matters.

Any concerns, or does someone mind approving?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75784



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

[Lldb-commits] [PATCH] D75626: Add support for owning module information to TypeSystemClang.

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:73
+  unsigned GetOwningModuleID() { return Flags(m_payload).Clear(ObjCClassBit); }
+  void SetOwningModuleID(unsigned id) {
+assert(id < ObjCClassBit);

Why not use `uint32_t` like we did above? If we are going to assume `32 bits` 
we should just use the fixed width type.


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

https://reviews.llvm.org/D75626



___
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 249525.
jingham added a comment.

Fix some clang-format nits.  I didn't change:

condition1 && condition2

  && condition3

to

condition1 && condition2 &&

  condition3

as it suggests.  We use that all over the place in lldb, and I much prefer it.  
It makes it clear that you are looking at a continuation of a logical 
expression, which scanning to the end of the line to find the && is much harder 
to see.


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,381 @@
+//===-- 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);
+  assert(result != m_completed_plan_store.end()
+ && "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 (ThreadPlanSP plan : m_plans)
+plan->ThreadDestroyed();
+
+  for (ThreadPlanSP plan : m_discarded_plans)
+plan->ThreadDestroyed();
+
+  for (ThreadPlanSP 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 (ThreadPlanSP plan : m_plans) {
+if (plan->GetThreadPlanTracer()) {
+  plan->GetThreadPlanTracer()->EnableTracing(value);
+  plan->GetThreadPlanTracer()->EnableSingleStep(single_stepping);
+}
+  }
+}
+
+void ThreadPlanStack::SetTracer(lldb::ThreadPlanTracerSP &tracer_sp) {
+  for

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

[Héctor Luis Díaz Aceves via Phabricator via lldb-commits]

diazhector98 updated this revision to Diff 249527.
diazhector98 added a comment.

Adding support for windows systems


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/completions/main.cpp
  lldb/test/API/tools/lldb-vscode/environmentVariables/Makefile
  
lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
  lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -84,6 +84,11 @@
 "description": "Additional environment variables.",
 "default": []
 			},
+			"inheritEnvironment": {
+"type": "boolean",
+"description": "Inherit the debugger environment when launching a process. Only works for binaries launched directly by LLDB.",
+"default": false
+			},
 			"stopOnEntry": {
 "type": "boolean",
 "description": "Automatically stop after launch.",
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -49,6 +49,12 @@
 #include "LLDBUtils.h"
 #include "VSCode.h"
 
+#  if defined(_WIN32)
+extern __declspec(dllimport) char** environ;
+#  else
+extern char** environ;
+#  endif
+
 #if defined(_WIN32)
 #ifndef PATH_MAX
 #define PATH_MAX MAX_PATH
@@ -1327,6 +1333,8 @@
   auto launchCommands = GetStrings(arguments, "launchCommands");
   g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
   const auto debuggerRoot = GetString(arguments, "debuggerRoot");
+  bool launchWithDebuggerEnvironment = GetBoolean(arguments, "inheritEnvironment", false);
+
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
   // in the debug map of the main executable have relative paths which require
@@ -1369,6 +1377,14 @@
 
   // Pass any environment variables along that the user specified.
   auto envs = GetStrings(arguments, "env");
+  if (launchWithDebuggerEnvironment) {
+std::vector vscode_env_variables;
+int cc;
+for (cc = 0; environ[cc]; ++cc) {
+  vscode_env_variables.push_back(environ[cc]);
+}
+envs.insert(std::end(envs), std::begin(vscode_env_variables), std::end(vscode_env_variables));
+  }
   if (!envs.empty())
 g_vsc.launch_info.SetEnvironmentEntries(MakeArgv(envs).data(), true);
 
Index: lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
@@ -0,0 +1,14 @@
+#include 
+#include 
+#include 
+#include 
+
+extern char **environ;
+
+int main(int argc, char const *argv[], char const *envp[]) {
+char** env_var_pointer = environ;
+for (char* env_variable = *env_var_pointer; env_variable; env_variable=*++env_var_pointer) {
+printf("%s\n", env_variable);
+}
+return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
@@ -0,0 +1,51 @@
+"""
+Test lldb-vscode environment variables
+"""
+
+
+import lldbvscode_testcase
+import unittest2
+import vscode
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def checkIfPathIsSame(self,lines, path_env_variable):
+for line in lines:
+if line.startswith('PATH='):
+self.assertTrue(path_env_variable == line, "PATH environment variable not the same")
+return
+
+@skipIfWindows
+#@skipIfDarwin # Skip this test for now until we can figure out why things aren't working on build bots
+def test_environment_variable(self):
+"""
+Tests the environment variables
+"""
+program = self.getBuildArtifact("a.out")
+path_env_variable = 'PATH='+os.environ['PATH']
+self.build_and_launch(program, inheritEnvironment=True)
+self.continue_to_exit()
+output = self.get_stdout().encode('utf-8')
+lines = output.splitlines()
+self.checkIfPathIsSame(lines, path_env_variable)
+
+@skipIfWindows
+#@skipIfDarwin
+def tes

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

[walter erquinigo via Phabricator via lldb-commits]

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



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1380
   auto envs = GetStrings(arguments, "env");
+  if (launchWithDebuggerEnvironment) {
+std::vector vscode_env_variables;

don't forget to apply Greg's suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636



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

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

[Héctor Luis Díaz Aceves via Phabricator via lldb-commits]

diazhector98 updated this revision to Diff 249536.
diazhector98 added a comment.

Adding remote process check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/completions/main.cpp
  lldb/test/API/tools/lldb-vscode/environmentVariables/Makefile
  
lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
  lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -84,6 +84,11 @@
 "description": "Additional environment variables.",
 "default": []
 			},
+			"inheritEnvironment": {
+"type": "boolean",
+"description": "Inherit the debugger environment when launching a process. Only works for binaries launched directly by LLDB.",
+"default": false
+			},
 			"stopOnEntry": {
 "type": "boolean",
 "description": "Automatically stop after launch.",
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -49,6 +49,12 @@
 #include "LLDBUtils.h"
 #include "VSCode.h"
 
+#  if defined(_WIN32)
+extern __declspec(dllimport) char** environ;
+#  else
+extern char** environ;
+#  endif
+
 #if defined(_WIN32)
 #ifndef PATH_MAX
 #define PATH_MAX MAX_PATH
@@ -1327,6 +1333,8 @@
   auto launchCommands = GetStrings(arguments, "launchCommands");
   g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
   const auto debuggerRoot = GetString(arguments, "debuggerRoot");
+  bool launchWithDebuggerEnvironment = GetBoolean(arguments, "inheritEnvironment", false);
+
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
   // in the debug map of the main executable have relative paths which require
@@ -1369,6 +1377,22 @@
 
   // Pass any environment variables along that the user specified.
   auto envs = GetStrings(arguments, "env");
+  if (launchWithDebuggerEnvironment) {
+const char *platform_name = g_vsc.target.GetPlatform().GetName();
+if (platform_name && strcmp(platform_name, "host") != 0) {
+  response["success"] = llvm::json::Value(false);
+  EmplaceSafeString(response, "message", 
+"Can't use inheritEnvironment on a remote process");
+  g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+  return;
+}
+std::vector vscode_env_variables;
+for (int i = 0; environ[i]; ++i) {
+  vscode_env_variables.push_back(environ[i]);
+}
+envs.insert(std::end(envs), std::begin(vscode_env_variables),
+ std::end(vscode_env_variables));
+  }
   if (!envs.empty())
 g_vsc.launch_info.SetEnvironmentEntries(MakeArgv(envs).data(), true);
 
Index: lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
@@ -0,0 +1,14 @@
+#include 
+#include 
+#include 
+#include 
+
+extern char **environ;
+
+int main(int argc, char const *argv[], char const *envp[]) {
+char** env_var_pointer = environ;
+for (char* env_variable = *env_var_pointer; env_variable; env_variable=*++env_var_pointer) {
+printf("%s\n", env_variable);
+}
+return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
@@ -0,0 +1,51 @@
+"""
+Test lldb-vscode environment variables
+"""
+
+
+import lldbvscode_testcase
+import unittest2
+import vscode
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def checkIfPathIsSame(self,lines, path_env_variable):
+for line in lines:
+if line.startswith('PATH='):
+self.assertTrue(path_env_variable == line, "PATH environment variable not the same")
+return
+
+@skipIfWindows
+#@skipIfDarwin # Skip this test for now until we can figure out why things aren't working on build bots
+def test_environment_variable(self):
+"""
+Tests the environment variables
+"""
+program = self.getBuildArtifac

[Lldb-commits] [PATCH] D75626: Add support for owning module information to TypeSystemClang.

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:996
   m_clang_ast_context->GetUniqueNamespaceDeclaration(
-  g_lldb_local_vars_namespace_cstr, nullptr);
+  g_lldb_local_vars_namespace_cstr, nullptr, 0);
   if (!namespace_decl)

We are passing around `0` everywhere and it is not obvious what it means at all.

Can we name this somehow or make it a type?


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

https://reviews.llvm.org/D75626



___
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 updated this revision to Diff 249555.
JDevlieghere added a comment.

- Add ProcessInfo provider.
- Rewrite test as an dotest-test.


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

https://reviews.llvm.org/D75877

Files:
  lldb/include/lldb/Utility/FileSpec.h
  lldb/include/lldb/Utility/ProcessInfo.h
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Utility/FileSpec.cpp
  lldb/source/Utility/ProcessInfo.cpp
  lldb/test/API/functionalities/reproducers/attach/Makefile
  lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
  lldb/test/API/functionalities/reproducers/attach/main.cpp

Index: lldb/test/API/functionalities/reproducers/attach/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/reproducers/attach/main.cpp
@@ -0,0 +1,14 @@
+#include 
+#include 
+
+using std::chrono::microseconds;
+
+int main(int argc, char const *argv[]) {
+  lldb_enable_attach();
+
+  while (true) {
+std::this_thread::sleep_for(microseconds(1));
+  }
+
+  return 0;
+}
Index: lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
===
--- /dev/null
+++ lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
@@ -0,0 +1,75 @@
+"""
+Test reproducer attach.
+"""
+
+import lldb
+import tempfile
+from lldbsuite.test import lldbtest_config
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class CreateAfterAttachTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfFreeBSD
+@skipIfNetBSD
+@skipIfWindows
+@skipIfRemote
+@skipIfiOSSimulator
+def test_create_after_attach_with_fork(self):
+"""Test thread creation after process attach."""
+self.build(dictionary={'EXE': 'somewhat_unique_name'})
+self.addTearDownHook(self.cleanupSubprocesses)
+
+reproducer_patch = tempfile.NamedTemporaryFile().name
+
+inferior = self.spawnSubprocess(
+self.getBuildArtifact("somewhat_unique_name"))
+pid = inferior.pid
+
+# Use Popen because pexpect is overkill and spawnSubprocess is
+# asynchronous.
+capture = subprocess.Popen([
+lldbtest_config.lldbExec, '-b', '--capture', '--capture-path',
+reproducer_patch, '-o', 'proc att -n somewhat_unique_name', '-o',
+'reproducer generate'
+],
+   stdin=subprocess.PIPE,
+   stdout=subprocess.PIPE,
+   stderr=subprocess.PIPE)
+outs, errs = capture.communicate()
+self.assertTrue('Process {} stopped'.format(pid) in outs)
+self.assertTrue('Reproducer written' in outs)
+
+# Check that replay works.
+replay = subprocess.Popen(
+[lldbtest_config.lldbExec, '-replay', reproducer_patch],
+stdin=subprocess.PIPE,
+stdout=subprocess.PIPE,
+stderr=subprocess.PIPE)
+outs, errs = replay.communicate()
+self.assertTrue('Process {} stopped'.format(pid) in outs)
+
+# Check that reproducer dump works for process info.
+replay = subprocess.Popen([
+lldbtest_config.lldbExec, '-b', '-o',
+'reproducer dump -f {} -p process'.format(reproducer_patch)
+],
+  stdin=subprocess.PIPE,
+  stdout=subprocess.PIPE,
+  stderr=subprocess.PIPE)
+outs, errs = replay.communicate()
+print(outs)
+self.assertTrue('name = {}'.format('somewhat_unique_name'))
+self.assertTrue('pid = {}'.format(pid))
+
+# Remove the reproducer but don't complain in case the directory was
+# never created.
+try:
+shutil.rmtree(reproducer_patch)
+except OSError:
+pass
Index: lldb/test/API/functionalities/reproducers/attach/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/reproducers/attach/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules
Index: lldb/source/Utility/ProcessInfo.cpp
===
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -18,6 +18,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace lldb_private::repro;
 
 ProcessInfo::ProcessInfo()
 : m_executable(), m_arguments(), m_environment(), m_uid(UINT32_MAX),
@@ -331,3 +332,113 @@
   m_name_match_type = NameMatch::Ignore;
   m_match_all_users = false;
 }
+
+llvm::Expected>
+ProcessInfoRecorder::Create(const FileSpec &filename) {
+  std:

[Lldb-commits] [lldb] 4016c6b - [lldb/Reproducer] Prevent crash when GDB multi-loader can't be created.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-10T23:16:55-07:00
New Revision: 4016c6b07f2ade01c65750d1297f72b43f9eb244

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

LOG: [lldb/Reproducer] Prevent crash when GDB multi-loader can't be created.

Check that the multi loader isn't null and print an error otherwise.
This patch also extends the test to cover these error paths.

Added: 


Modified: 
lldb/source/Commands/CommandObjectReproducer.cpp
lldb/test/Shell/Reproducer/TestDump.test

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectReproducer.cpp 
b/lldb/source/Commands/CommandObjectReproducer.cpp
index a3b49b1979b3..d73fa78231a5 100644
--- a/lldb/source/Commands/CommandObjectReproducer.cpp
+++ b/lldb/source/Commands/CommandObjectReproducer.cpp
@@ -422,8 +422,8 @@ class CommandObjectReproducerDump : public 
CommandObjectParsed {
   repro::MultiLoader::Create(loader);
   if (!multi_loader) {
 SetError(result,
- make_error(llvm::inconvertibleErrorCode(),
- "Unable to create command loader."));
+ make_error("Unable to create command loader.",
+ llvm::inconvertibleErrorCode()));
 return false;
   }
 
@@ -448,6 +448,14 @@ class CommandObjectReproducerDump : public 
CommandObjectParsed {
   std::unique_ptr>
   multi_loader =
   repro::MultiLoader::Create(loader);
+
+  if (!multi_loader) {
+SetError(result,
+ make_error("Unable to create GDB loader.",
+ llvm::inconvertibleErrorCode()));
+return false;
+  }
+
   llvm::Optional gdb_file;
   while ((gdb_file = multi_loader->GetNextFile())) {
 auto error_or_file = MemoryBuffer::getFile(*gdb_file);

diff  --git a/lldb/test/Shell/Reproducer/TestDump.test 
b/lldb/test/Shell/Reproducer/TestDump.test
index 3d4d21d98e50..c193b806b547 100644
--- a/lldb/test/Shell/Reproducer/TestDump.test
+++ b/lldb/test/Shell/Reproducer/TestDump.test
@@ -24,3 +24,11 @@
 # GDB: read packet: $OK#9a
 
 # RUN: %lldb --replay %t.repro | FileCheck %s --check-prefix FILES
+
+# RUN: rm %t.repro/gdb-remote.yaml
+# RUN: %lldb -b -o 'reproducer dump -p gdb -f %t.repro' 2>&1 | FileCheck %s 
--check-prefix GDB-ERROR
+# GDB-ERROR: error: Unable to create GDB loader.
+
+# RUN: rm %t.repro/command-interpreter.yaml
+# RUN: %lldb -b -o 'reproducer dump -p commands -f %t.repro' 2>&1 | FileCheck 
%s --check-prefix COMMANDS-ERROR
+# COMMANDS-ERROR: error: Unable to create command loader.



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