Re: [Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

[Jim Ingham via lldb-commits]

Note, lldb has a bunch of special handling for line 0 code.  For instance, when 
we are pushing a breakpoint past the prologue we will keep pushing it forward 
over line number 0 lines.  Those are compiler generated and in general people 
don't want to stop there.  Similarly, if you are stepping through line 3 and 
the next line entry after 3 is line 0 we keep stepping till we get to a 
non-zero line.  

When the compiler is actually using line 0 to mean "compiler generated code not 
really associated with a particular line, then I am pretty sure the debugger 
has to be aware of this or debugging is going to be a bit awkward...

I don't know if that's directly relevant to this bug, I haven't had time to 
follow the whole discussion.  But I'm not convinced all the problems with line 
0 emission causing debugging oddities can be solved in the line table 
generation.

Jim


> On Dec 3, 2020, at 10:33 AM, David Blaikie via Phabricator via lldb-commits 
>  wrote:
> 
> dblaikie added a comment.
> 
> In D91734#2431247 , @probinson wrote:
> 
>>> Sometihng like this seems plausible to me:
>> 
>> Yes, I was playing with essentially that exact patch last night.  It has no 
>> effect on the final assembly on its own, but combined with my patch it does.
> 
> It might have effects on assembly in other test cases, though. Could be worth 
> running it through a self-host or something to see what other changes it 
> causes and whether they're desirable.
> 
>>> (a more general fix (that would cover cases where the instruction really 
>>> has no location) might be to propagate locations from the first instruction 
>>> in a basic block with a location back up to the start of the basic block - 
>>> I forget if we've considered/tried that before)
>> 
>> We have, but that without flushing the map on every instruction, so it 
>> caught materialization instructions that didn't actually belong to that IR 
>> instruction.  The tactic would likely be more reasonable in conjunction with 
>> my patch.
> 
> (oh, when I was saying that I didn't really think - the materialization in 
> this case wasn't necessarily on a BB boundary - so I guess my suggestion 
> amounts to possibly never using line 0 (unless it's the only location in a 
> whole BB), instead back or forward propagating surrounding locations over any 
> line 0 - and that doesn't sound right when I put it that way)
> 
> But yeah, maybe some amount of it could be done around the flushing thing.
> 
> (FWIW, about this patch in general, I do worry a bit about this being a 
> debug-info motivated optimization decision (even if that decision is applied 
> uniformly/not just when debug info is enabled) - you mention this might have 
> positive performance features due to smaller register live ranges, but also 
> possibly negative ones rematerializing the same constant (" however, only 
> about 5% of those values
> were reused in an experimental self-build of clang.") - do you have 
> performance measurements/benchmarks related to this change? I guess it didn't 
> show up in the perf bot profiles upstream at least)
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D91734/new/
> 
> https://reviews.llvm.org/D91734
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

In D91734#2432188 , @probinson wrote:

> See D92606  for a front-end patch to improve 
> locations in the IR.
> That, plus reapplying this patch, should help out GDB.  I haven't had a 
> chance to run the suite myself with both patches applied and I'm basically 
> off tomorrow, so if @dblaikie doesn't have a chance, I'll do that on Monday.

Had a go - certainly creates a better debugging experience, but still fails the 
gdb test in question.

The code in the original test looks something like:

  int multi_line_while_conditional (int a, int b, int c)
  {
while (a /* set breakpoint 4 here */
&& b 
&& c) 
  { 
a--, b--, c--;
  }
return 0;
  }

And with these changes together, breaking on the function breaks on line 5 
(presumably because this is creating the constant used by the second '&&' which 
is on line 5) instead of line 3. Not the worst thing, but I imagine given 
Sony's interest in less "jumpy" line tables, this might not be super desirable.

Yeah, the gdb experience is less than ideal:

  13multi_line_while_conditional(1, 1, 1);
  (gdb) s
  multi_line_while_conditional (a=1, b=1, c=1) at test.cpp:5
  5 && c) 
  (gdb) n
  3 while (a /* set breakpoint 4 here */
  (gdb) 
  4 && b 
  (gdb) 
  5 && c) 
  (gdb) 
  4 && b 
  (gdb) 
  5 && c) 
  (gdb) 
  3 while (a /* set breakpoint 4 here */
  (gdb) 
  7 a--, b--, c--;

Compared with (without any of these patches):

  13multi_line_while_conditional(1, 1, 1);
  (gdb) s
  multi_line_while_conditional (a=1, b=1, c=1) at test.cpp:3
  3 while (a /* set breakpoint 4 here */
  (gdb) n
  4 && b 
  (gdb) 
  5 && c) 
  (gdb) 
  3 while (a /* set breakpoint 4 here */
  (gdb) 
  7 a--, b--, c--;

And because I was curious about /exactly/ which tokens the debugger was 
stepping to, I split out the tokens onto their own lines:
With patch:

  18multi_line_while_conditional(1, 1, 1);
  (gdb) s
  multi_line_while_conditional (a=1, b=1, c=1) at test.cpp:8
  8 && 
  (gdb) n
  5   a 
  (gdb) 
  6 && 
  (gdb) 
  8 && 
  (gdb) 
  7 b 
  (gdb) 
  8 && 
  (gdb) 
  9 c
  (gdb) 
  3 while 
  (gdb) 
  12a--, b--, c--;

Without patch/with trunk:

  18multi_line_while_conditional(1, 1, 1);
  (gdb) s
  multi_line_while_conditional (a=1, b=1, c=1) at test.cpp:5
  5   a 
  (gdb) n
  6 && 
  (gdb) 
  7 b 
  (gdb) 
  8 && 
  (gdb) 
  9 c
  (gdb) 
  3 while 
  (gdb) 
  12a--, b--, c--;

Maybe it's OK? But at least it's 'interesting' enough that might deserve some 
extra scrutiny.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91734

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

[Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

Also, FWIW, the other gdb test suite failures we saw were:

  FAIL: gdb.base/foll-exec.exp: step through execlp call
  FAIL: gdb.base/foll-exec.exp: step after execlp call
  FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after execlp)
  FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execlp)
  FAIL: gdb.base/foll-exec.exp: print follow-exec/local_k (after execlp)
  FAIL: gdb.base/hbreak2.exp: hardware breakpoint at start of multi line while 
conditional 
  FAIL: gdb.base/hbreak2.exp: hardware breakpoint info

Those last two are different versions of this original issue seen in 
break.exp/c with multi-line conditionals.

The foll-exec.exp ones I'm less sure of - Looks like maybe it's a similar line 
table attribution sort of thing, but it leads the test case not to step across 
the intended process boundary/exec call and things go weird from there.

Yeah, it boils down to something like this:

  void f1(const char*, const char*) { }
  int main() {
char prog[1];
  
f1(prog,
   prog);
  }

Without the patch, you step to the 'f1' line, and then step into or over the 
function call (step or next).
But with these patches applied - you step to the 'f1(' line, then the 'prog);' 
line, then back to the 'f1(' line, then into/over the function call.

Again, could be acceptable - if those arguments had specific non-trivial code 
in them (like other function calls) you'd certainly get that step 
forward/backward behavior - but it's notewhorthy that this is change would make 
more cases like that & it'd be good to understand why/if that's a good thing 
overall.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91734

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

Re: [Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

[Jim Ingham via lldb-commits]

> On Dec 3, 2020, at 5:43 PM, Robinson, Paul  wrote:
> 
> 
> 
>> -Original Message-
>> From: Jim Ingham mailto:jing...@apple.com>>
>> Sent: Thursday, December 3, 2020 5:51 PM
>> To: David Blaikie
>> > >; David 
>> Blaikie
>> via Phabricator mailto:revi...@reviews.llvm.org>>
>> Cc: Robinson, Paul mailto:paul.robin...@sony.com>>; 
>> r...@google.com ;
>> echri...@gmail.com ; sontuan.vu...@gmail.com 
>> ; mcros...@codeaurora.org 
>> ;
>> nikola.te...@syrmia.com ; lldb-commits 
>> mailto:lldb-commits@lists.llvm.org>>;
>> liburd1...@outlook.com ; 
>> stefan.graen...@gmail.com ; 
>> ma...@braunis.de ;
>> nemanja.i@gmail.com ; 
>> simon.dar...@gmail.com ; Tozer, Stephen
>> mailto:stephen.to...@sony.com>>; 
>> alokkumar.sha...@amd.com ; 
>> s...@chromium.org ;
>> sourabhsingh.to...@amd.com ; Jackson, 
>> Chris mailto:chris.jack...@sony.com>>;
>> arpha...@gmail.com ; j...@us.ibm.com 
>> ; ...@gmail.com ;
>> horse10241...@gmail.com ; 
>> pengfei.w...@intel.com ; david.gr...@arm.com 
>> ;
>> higuox...@gmail.com ; ikud...@accesssoftek.com 
>> ; p8u8i7l5t1q9r8w3@ibm-
>> systems-z.slack.com ; Enciso, Carlos 
>> mailto:carlos.enc...@sony.com>>; llvm-
>> comm...@lists.llvm.org ; Cazalet-Hyams, 
>> Orlando mailto:orlando.hy...@sony.com>>;
>> aso...@cisco.com ; sani...@subpath.org 
>> ; si...@atanasyan.com 
>> ;
>> jrt...@jrtc27.com ; djordje.todoro...@syrmia.com 
>> ; ch.besson...@gmail.com 
>> ;
>> quentin.colom...@gmail.com ; 
>> akhu...@google.com ; ahee...@gmail.com 
>> ;
>> avl.laps...@gmail.com ; 
>> david.stenb...@ericsson.com ;
>> david.spick...@linaro.org ; 
>> dougp...@gmail.com ; bhuvanendra.kum...@amd.com 
>> ;
>> serhiy.re...@gmail.com ; 
>> jini.susan.geo...@gmail.com 
>> Subject: Re: [Lldb-commits] [PATCH] D91734: [FastISel] Flush local value
>> map on every instruction
>> 
>> Note, lldb has a bunch of special handling for line 0 code.  For instance,
>> when we are pushing a breakpoint past the prologue we will keep pushing it
>> forward over line number 0 lines.  Those are compiler generated and in
>> general people don't want to stop there.  Similarly, if you are stepping
>> through line 3 and the next line entry after 3 is line 0 we keep stepping
>> till we get to a non-zero line.
>> 
>> When the compiler is actually using line 0 to mean "compiler generated
>> code not really associated with a particular line, then I am pretty sure
>> the debugger has to be aware of this or debugging is going to be a bit
>> awkward...
>> 
>> I don't know if that's directly relevant to this bug, I haven't had time
>> to follow the whole discussion.  But I'm not convinced all the problems
>> with line 0 emission causing debugging oddities can be solved in the line
>> table generation.
>> 
>> Jim
> 
> Hi Jim,
> 
> This issue is not really "line 0 causing debugger oddities" so much as
> "line 0 attached to instructions that should have a real line number."
> 
> It's true that gdb's response to line 0 on the first instruction past
> the prologue_end seems a tad idiosyncratic, but if nothing else, it
> exposed an issue in the compiler that is worth solving.  There are
> clearly instructions attributed to line 0 that shouldn't be, and the
> goal here is to make that better.

That's great!  Getting all the instructions you can assigned correctly to real 
lines is worthy labor!  

Once you are all done with that work, given you really are using line 0 for 
real instances of compiler generated code that aren't attributable to any one 
line, stepping in gdb is going to look odd sometimes if gdb doesn't know to 
handle them.  It might be worth filing some bugs with gdb to cope with this 
situation (though its probably better to use examples where the line 0 is 
correctly attributed...)

Jim


> 
> Tha

Re: [Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

[Robinson, Paul via lldb-commits]

> -Original Message-
> From: Jim Ingham 
> Sent: Thursday, December 3, 2020 5:51 PM
> To: David Blaikie
> ; David Blaikie
> via Phabricator 
> Cc: Robinson, Paul ; r...@google.com;
> echri...@gmail.com; sontuan.vu...@gmail.com; mcros...@codeaurora.org;
> nikola.te...@syrmia.com; lldb-commits ;
> liburd1...@outlook.com; stefan.graen...@gmail.com; ma...@braunis.de;
> nemanja.i@gmail.com; simon.dar...@gmail.com; Tozer, Stephen
> ; alokkumar.sha...@amd.com; s...@chromium.org;
> sourabhsingh.to...@amd.com; Jackson, Chris ;
> arpha...@gmail.com; j...@us.ibm.com; ...@gmail.com;
> horse10241...@gmail.com; pengfei.w...@intel.com; david.gr...@arm.com;
> higuox...@gmail.com; ikud...@accesssoftek.com; p8u8i7l5t1q9r8w3@ibm-
> systems-z.slack.com; Enciso, Carlos ; llvm-
> comm...@lists.llvm.org; Cazalet-Hyams, Orlando ;
> aso...@cisco.com; sani...@subpath.org; si...@atanasyan.com;
> jrt...@jrtc27.com; djordje.todoro...@syrmia.com; ch.besson...@gmail.com;
> quentin.colom...@gmail.com; akhu...@google.com; ahee...@gmail.com;
> avl.laps...@gmail.com; david.stenb...@ericsson.com;
> david.spick...@linaro.org; dougp...@gmail.com; bhuvanendra.kum...@amd.com;
> serhiy.re...@gmail.com; jini.susan.geo...@gmail.com
> Subject: Re: [Lldb-commits] [PATCH] D91734: [FastISel] Flush local value
> map on every instruction
> 
> Note, lldb has a bunch of special handling for line 0 code.  For instance,
> when we are pushing a breakpoint past the prologue we will keep pushing it
> forward over line number 0 lines.  Those are compiler generated and in
> general people don't want to stop there.  Similarly, if you are stepping
> through line 3 and the next line entry after 3 is line 0 we keep stepping
> till we get to a non-zero line.
> 
> When the compiler is actually using line 0 to mean "compiler generated
> code not really associated with a particular line, then I am pretty sure
> the debugger has to be aware of this or debugging is going to be a bit
> awkward...
> 
> I don't know if that's directly relevant to this bug, I haven't had time
> to follow the whole discussion.  But I'm not convinced all the problems
> with line 0 emission causing debugging oddities can be solved in the line
> table generation.
> 
> Jim

Hi Jim,

This issue is not really "line 0 causing debugger oddities" so much as
"line 0 attached to instructions that should have a real line number."

It's true that gdb's response to line 0 on the first instruction past
the prologue_end seems a tad idiosyncratic, but if nothing else, it
exposed an issue in the compiler that is worth solving.  There are
clearly instructions attributed to line 0 that shouldn't be, and the
goal here is to make that better.

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

[Lldb-commits] [lldb] 973f390 - [lldb][NFC] Rename TypeSystemClangForExpressions to ScratchTypeSystemClang

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-12-04T09:41:42+01:00
New Revision: 973f3907a471eee85c35f7d283fe2de91ce953e2

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

LOG: [lldb][NFC] Rename TypeSystemClangForExpressions to ScratchTypeSystemClang

We keep referring to the single object created by this class as
'scratch AST/Context/TypeSystem' so at this point we might as well rename the
class. It's also not involved at all in expression evaluation, so the
'ForExpressions' prefix is a bit misleading.

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index ca109ef9c2fc..0445e4b0a056 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -613,7 +613,7 @@ lldb::TypeSystemSP 
TypeSystemClang::CreateInstance(lldb::LanguageType language,
 "ASTContext for '" + module->GetFileSpec().GetPath() + "'";
 return std::make_shared(ast_name, triple);
   } else if (target && target->IsValid())
-return std::make_shared(*target, triple);
+return std::make_shared(*target, triple);
   return lldb::TypeSystemSP();
 }
 
@@ -9567,8 +9567,8 @@ TypeSystemClang::DeclContextGetTypeSystemClang(const 
CompilerDeclContext &dc) {
   return nullptr;
 }
 
-TypeSystemClangForExpressions::TypeSystemClangForExpressions(
-Target &target, llvm::Triple triple)
+ScratchTypeSystemClang::ScratchTypeSystemClang(Target &target,
+   llvm::Triple triple)
 : TypeSystemClang("scratch ASTContext", triple),
   m_target_wp(target.shared_from_this()),
   m_persistent_variables(new ClangPersistentVariables) {
@@ -9580,16 +9580,15 @@ 
TypeSystemClangForExpressions::TypeSystemClangForExpressions(
   SetExternalSource(proxy_ast_source);
 }
 
-void TypeSystemClangForExpressions::Finalize() {
+void ScratchTypeSystemClang::Finalize() {
   TypeSystemClang::Finalize();
   m_scratch_ast_source_up.reset();
 }
 
-UserExpression *TypeSystemClangForExpressions::GetUserExpression(
+UserExpression *ScratchTypeSystemClang::GetUserExpression(
 llvm::StringRef expr, llvm::StringRef prefix, lldb::LanguageType language,
 Expression::ResultType desired_type,
-const EvaluateExpressionOptions &options,
-ValueObject *ctx_obj) {
+const EvaluateExpressionOptions &options, ValueObject *ctx_obj) {
   TargetSP target_sp = m_target_wp.lock();
   if (!target_sp)
 return nullptr;
@@ -9598,7 +9597,7 @@ UserExpression 
*TypeSystemClangForExpressions::GetUserExpression(
  desired_type, options, ctx_obj);
 }
 
-FunctionCaller *TypeSystemClangForExpressions::GetFunctionCaller(
+FunctionCaller *ScratchTypeSystemClang::GetFunctionCaller(
 const CompilerType &return_type, const Address &function_address,
 const ValueList &arg_value_list, const char *name) {
   TargetSP target_sp = m_target_wp.lock();
@@ -9614,8 +9613,8 @@ FunctionCaller 
*TypeSystemClangForExpressions::GetFunctionCaller(
 }
 
 std::unique_ptr
-TypeSystemClangForExpressions::CreateUtilityFunction(std::string text,
- std::string name) {
+ScratchTypeSystemClang::CreateUtilityFunction(std::string text,
+  std::string name) {
   TargetSP target_sp = m_target_wp.lock();
   if (!target_sp)
 return {};
@@ -9625,6 +9624,6 @@ 
TypeSystemClangForExpressions::CreateUtilityFunction(std::string text,
 }
 
 PersistentExpressionState *
-TypeSystemClangForExpressions::GetPersistentExpressionState() {
+ScratchTypeSystemClang::GetPersistentExpressionState() {
   return m_persistent_variables.get();
 }

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 6879d2566183..15c52b32ba12 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1122,11 +1122,11 @@ class TypeSystemClang : public TypeSystem {
 
 /// The TypeSystemClang instance used for the scratch ASTContext in a
 /// lldb::Target.
-class TypeSystemClangForExpressions : public TypeSystemClang {
+class ScratchTypeSystemClang : public TypeSystemClang {
 public:
-  TypeSystemClangForExpressions(Target &target, llvm::Triple triple);
+  ScratchTypeSystemClang(Target &target, llvm::Triple triple);
 
-  ~TypeSystemClangForExpressions() override = default;
+  ~ScratchTypeSystemClang() override = default;
 
   void Finalize() override;
 
@@ -1148,9 +1148,11 @@ class TypeSystemCl

[Lldb-commits] [PATCH] D82857: [LLDB] Add per-thread register infos shared pointer in gdb-remote

[Diana Picus via Phabricator via lldb-commits]

rovka added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h:29
+  DynamicRegisterInfo(DynamicRegisterInfo &) = default;
   void operator=(DynamicRegisterInfo &) = delete;
 

I think operator= should be default too if the copy constructor is default.


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

https://reviews.llvm.org/D82857

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

[Lldb-commits] [PATCH] D92643: [lldb] Lookup static const members in FindGlobalVariables

[Andy Yankovsky via Phabricator via lldb-commits]

werat created this revision.
werat requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, sstefan1.
Herald added a project: LLDB.

Static const members initialized inside a class definition might not have a 
corresponding DW_TAG_variable, so they're not indexed by ManualDWARFIndex.

Add an additional lookup in FindGlobalVariables. Try looking up the enclosing 
type (e.g. `foo::bar` for `foo::bar::A`) and then searching for a static const 
member (`A`) within this type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92643

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/test/API/python_api/target/globals/Makefile
  lldb/test/API/python_api/target/globals/TestTargetGlobals.py
  lldb/test/API/python_api/target/globals/main.cpp

Index: lldb/test/API/python_api/target/globals/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/target/globals/main.cpp
@@ -0,0 +1,12 @@
+class Vars {
+public:
+  inline static double inline_static = 1.5;
+  static constexpr int static_constexpr = 2;
+  static const int static_const_out_out_class;
+};
+
+const int Vars::static_const_out_out_class = 3;
+
+char global_var_of_char_type = 'X';
+
+int main() {}
Index: lldb/test/API/python_api/target/globals/TestTargetGlobals.py
===
--- /dev/null
+++ lldb/test/API/python_api/target/globals/TestTargetGlobals.py
@@ -0,0 +1,42 @@
+"""
+Test SBTarget::FindGlobalVariables API.
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class TargetAPITestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(['pyapi'])
+def test_find_global_variables(self):
+"""Exercise SBTarget.FindGlobalVariables() API."""
+self.build()
+
+# Don't need to launch a process, since we're only interested in
+# looking up global variables.
+target = self.dbg.CreateTarget(self.getBuildArtifact())
+
+def test_global_var(query, name, type_name, value):
+value_list = target.FindGlobalVariables(query, 1)
+self.assertEqual(value_list.GetSize(), 1)
+var = value_list.GetValueAtIndex(0)
+self.DebugSBValue(var)
+self.assertTrue(var)
+self.assertEqual(var.GetName(), name)
+self.assertEqual(var.GetTypeName(), type_name)
+self.assertEqual(var.GetValue(), value)
+
+test_global_var(
+"Vars::inline_static",
+"Vars::inline_static", "double", "1.5")
+test_global_var(
+"Vars::static_constexpr",
+"Vars::static_constexpr", "const int", "2")
+test_global_var(
+"Vars::static_const_out_out_class",
+"Vars::static_const_out_out_class", "const int", "3")
+test_global_var(
+"global_var_of_char_type",
+"::global_var_of_char_type", "char", "'X'")
Index: lldb/test/API/python_api/target/globals/Makefile
===
--- /dev/null
+++ lldb/test/API/python_api/target/globals/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -372,6 +372,10 @@
 
   lldb_private::Type *ResolveTypeUID(const DIERef &die_ref);
 
+  lldb::VariableSP
+  ParseStaticConstMemberDIE(const lldb_private::SymbolContext &sc,
+const DWARFDIE &die);
+
   lldb::VariableSP ParseVariableDIE(const lldb_private::SymbolContext &sc,
 const DWARFDIE &die,
 const lldb::addr_t func_low_pc);
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2151,6 +2151,47 @@
 return variables.GetSize() - original_size < max_matches;
   });
 
+  // If we don't have enough matches and the variable context is not empty, try
+  // to resolve the context as a type and look for static const members.
+  if (variables.GetSize() - original_size < max_matches && !context.empty()) {
+llvm::StringRef type_scope;
+llvm::StringRef type_name;
+TypeClass type_class;
+if (!Type::GetTypeScopeAndBasename(context, type_scope, type_name,
+   type_class))
+  type_name = context;
+
+m_index->GetTypes(ConstString(type_name), [&](DWARFDIE parent) {
+  llvm::StringRef

[Lldb-commits] [PATCH] D92643: [lldb] Lookup static const members in FindGlobalVariables

[Andy Yankovsky via Phabricator via lldb-commits]

werat edited reviewers, added: labath, jarin, teemperor, jankratochvil; 
removed: jdoerfert.
werat added a subscriber: labath.
werat added a comment.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: JDevlieghere.

Hi, please, take a look at this patch!

This is an approach suggested by @labath in 
https://reviews.llvm.org/D92223#2422184

This approach feels a little bit "adhoc" (i.e. manually going through the DIE 
entries for const members), but it works :) Please, let me know if there's a 
better way to do this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92643

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

[Lldb-commits] [lldb] 594308c - [lldb][NFC] Rename TypeSystemClang::GetScratch to ScratchTypeSystemClang::GetForTarget

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-12-04T11:29:08+01:00
New Revision: 594308c7ad07b4cea20f41d915aa81794e909654

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

LOG: [lldb][NFC] Rename TypeSystemClang::GetScratch to 
ScratchTypeSystemClang::GetForTarget

Also add some documentation while I'm at it.

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Plugins/Language/ObjC/NSArray.cpp
lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
lldb/source/Plugins/Language/ObjC/NSError.cpp
lldb/source/Plugins/Language/ObjC/NSException.cpp
lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
lldb/source/Plugins/Language/ObjC/NSString.cpp

lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp

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

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

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp
lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp
lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp
lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp
lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 569d84d39c807..e35e6063a0f1c 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -1113,7 +1113,7 @@ DynamicLoaderDarwin::GetThreadLocalData(const 
lldb::ModuleSP module_sp,
 StackFrameSP frame_sp = thread_sp->GetStackFrameAtIndex(0);
 if (frame_sp) {
   TypeSystemClang *clang_ast_context =
-  TypeSystemClang::GetScratch(target);
+  ScratchTypeSystemClang::GetForTarget(target);
 
   if (!clang_ast_context)
 return LLDB_INVALID_ADDRESS;

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
index 7bc14061ffe05..9fc5e5daeafb5 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -224,7 +224,7 @@ bool DynamicLoaderMacOS::NotifyBreakpointHit(void *baton,
 // get the values from the ABI:
 
 TypeSystemClang *clang_ast_context =
-TypeSystemClang::GetScratch(process->GetTarget());
+ScratchTypeSystemClang::GetForTarget(process->GetTarget());
 if (!clang_ast_context)
   return false;
 

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
index d425b3587237c..76b4c48110941 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
@@ -343,7 +343,7 @@ bool DynamicLoaderMacOSXDYLD::NotifyBreakpointHit(
 // get the values from the ABI:
 
 TypeSystemClang *clang_ast_context =
-TypeSystemClang::GetScratch(process->GetTarget());
+ScratchTypeSystemClang::GetForTarget(process->GetTarget());
 if (!clang_ast_context)
   return false;
 

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
index 39ba5f4e9e4fd..66a87ba924dba 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
@@ -443,7 +443,7 @@ void ASTResultSynthesizer::CommitPersistentDecls() {
 return;
 
   auto *persistent_vars = llvm::cast(state);
-  TypeSystemClang *scratch_ctx = TypeSystemClang::GetScratch(m_target);
+ 

[Lldb-commits] [PATCH] D82857: [LLDB] Add per-thread register infos shared pointer in gdb-remote

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h:29
+  DynamicRegisterInfo(DynamicRegisterInfo &) = default;
   void operator=(DynamicRegisterInfo &) = delete;
 

rovka wrote:
> I think operator= should be default too if the copy constructor is default.
Yeah, we can make both of them protected to avoid accidental use.


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

https://reviews.llvm.org/D82857

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

[Lldb-commits] [lldb] e97b991 - [lldb] Remove LLDB session dir and just store test traces in the respective test build directory

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-12-04T11:43:10+01:00
New Revision: e97b991eef63663d1f635813fe375354edb7b51a

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

LOG: [lldb] Remove LLDB session dir and just store test traces in the 
respective test build directory

Test runs log some of their output to files inside the LLDB session dir. This
session dir is shared between all tests, so all the tests have to make sure they
choose a unique file name inside that directory. We currently choose by default
`-` as the log file name. However, that means
that if not every test class in the test suite has a unique class name, then we
end up with a race condition as two tests will try to write to the same log
file.

I already tried in D83767 changing the format to use the test file basename
instead (which we already require to be unique for some other functionality),
but it seems the code for getting the basename didn't work on Windows.

This patch instead just changes that dotest stores the log files in the build
directory for the current test. We know that directory is unique for this test,
so no need to generate some unique file name now. Also removes all the
environment vars and parameters related to the now unused session dir.

The new log paths now look like this for a failure in 'TestCppOperators`:
```
./lldb-test-build.noindex/lang/cpp/operators/TestCppOperators.test_dwarf/Failure.log
./lldb-test-build.noindex/lang/cpp/operators/TestCppOperators.test_dsym/Failure.log
./lldb-test-build.noindex/lang/cpp/operators/TestCppOperators.test_gmodules/Failure.log
```

Reviewed By: labath

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

Added: 


Modified: 
lldb/examples/test/.lldb-loggings
lldb/examples/test/usage-lldb-loggings
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/dotest_args.py
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/test/API/CMakeLists.txt
lldb/test/API/lit.cfg.py
lldb/test/API/lit.site.cfg.py.in
lldb/utils/lldb-dotest/CMakeLists.txt
lldb/utils/lldb-dotest/lldb-dotest.in

Removed: 




diff  --git a/lldb/examples/test/.lldb-loggings 
b/lldb/examples/test/.lldb-loggings
index 9c92bd958479..873bb88c844d 100644
--- a/lldb/examples/test/.lldb-loggings
+++ b/lldb/examples/test/.lldb-loggings
@@ -3,8 +3,7 @@ def pre_flight(self):
 import lldb
 import lldbtest
 
-dname = os.path.join(os.environ["LLDB_TEST"],
- os.environ["LLDB_SESSION_DIRNAME"])
+dname = os.path.join(os.environ["LLDB_TEST"])
 if not os.path.isdir(dname):
 os.mkdir(dname)
 dest = os.path.join(dname, "lldb_log-%s-%s-%s.txt" % 
(self.getArchitecture(), self.getCompiler(), self.id()))

diff  --git a/lldb/examples/test/usage-lldb-loggings 
b/lldb/examples/test/usage-lldb-loggings
index b7d7e2e58fc1..4ce9689f3d37 100644
--- a/lldb/examples/test/usage-lldb-loggings
+++ b/lldb/examples/test/usage-lldb-loggings
@@ -88,8 +88,7 @@ lldb.pre_flight: def pre_flight(self):
 import lldb
 import lldbtest
 
-dname = os.path.join(os.environ["LLDB_TEST"],
- os.environ["LLDB_SESSION_DIRNAME"])
+dname = os.path.join(os.environ["LLDB_TEST"])
 if not os.path.isdir(dname):
 os.mkdir(dname)
 dest = os.path.join(dname, "lldb_log-%s-%s-%s.txt" % 
(self.getArchitecture(), self.getCompiler(), self.id()))

diff  --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 7939a27badf0..db4262f65345 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -76,20 +76,6 @@
 skip_tests = None
 xfail_tests = None
 
-# By default, recorded session info for errored/failed test are dumped into its
-# own file under a session directory named after the timestamp of the test 
suite
-# run.  Use '-s session-dir-name' to specify a specific dir name.
-sdir_name = None
-
-# Valid options:
-# f - test file name (without extension)
-# n - test class name
-# m - test method name
-# a - architecture
-# c - compiler path
-# The default is to write all fields.
-session_file_format = 'fnmac'
-
 # Set this flag if there is any session info dumped during the test run.
 sdir_has_content = False
 # svn_info stores the output from 'svn info lldb.base.dir'.

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 7c0a9ec56fce..64a197872f9e 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -388,14 +388,6 @@ def parseOptionsAndInitTestdirs():
 usage(parser)

[Lldb-commits] [PATCH] D92498: [lldb] Remove LLDB session dir and just store test traces in the respective test build directory

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe97b991eef63: [lldb] Remove LLDB session dir and just store 
test traces in the respective… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92498

Files:
  lldb/examples/test/.lldb-loggings
  lldb/examples/test/usage-lldb-loggings
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/test/API/CMakeLists.txt
  lldb/test/API/lit.cfg.py
  lldb/test/API/lit.site.cfg.py.in
  lldb/utils/lldb-dotest/CMakeLists.txt
  lldb/utils/lldb-dotest/lldb-dotest.in

Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -15,7 +15,6 @@
 lldb_build_intel_pt = "@LLDB_BUILD_INTEL_PT@"
 lldb_framework_dir = "@LLDB_FRAMEWORK_DIR_CONFIGURED@"
 lldb_libs_dir = "@LLDB_LIBS_DIR_CONFIGURED@"
-lldb_trace_dir = '@LLDB_TEST_TRACE_DIRECTORY_CONFIGURED@'
 
 if __name__ == '__main__':
 wrapper_args = sys.argv[1:]
@@ -24,7 +23,6 @@
 cmd = [sys.executable, dotest_path]
 cmd.extend(['--arch', arch])
 cmd.extend(dotest_args)
-cmd.extend(['-s', lldb_trace_dir])
 cmd.extend(['--build-dir', lldb_build_dir])
 cmd.extend(['--executable', executable])
 cmd.extend(['--compiler', compiler])
Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -23,7 +23,6 @@
 string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} LLDB_SOURCE_DIR_CONFIGURED "${LLDB_SOURCE_DIR}")
 string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} LLDB_FRAMEWORK_DIR_CONFIGURED "${LLDB_FRAMEWORK_DIR}")
 string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} LLDB_TEST_BUILD_DIRECTORY_CONFIGURED "${LLDB_TEST_BUILD_DIRECTORY}")
-string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} LLDB_TEST_TRACE_DIRECTORY_CONFIGURED "${LLDB_TEST_TRACE_DIRECTORY}")
 string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} LLDB_TEST_EXECUTABLE_CONFIGURED "${LLDB_TEST_EXECUTABLE}")
 string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} LLDB_TEST_COMPILER_CONFIGURED "${LLDB_TEST_COMPILER}")
 string(REPLACE ${LLVM_RUNTIME_OUTPUT_INTDIR} ${config_runtime_output_dir} LLDB_TEST_DSYMUTIL_CONFIGURED "${LLDB_TEST_DSYMUTIL}")
@@ -38,7 +37,6 @@
   string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} LLDB_SOURCE_DIR_CONFIGURED "${LLDB_SOURCE_DIR}")
   string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} LLDB_FRAMEWORK_DIR_CONFIGURED "${LLDB_FRAMEWORK_DIR}")
   string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} LLDB_TEST_BUILD_DIRECTORY_CONFIGURED "${LLDB_TEST_BUILD_DIRECTORY}")
-  string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} LLDB_TEST_TRACE_DIRECTORY_CONFIGURED "${LLDB_TEST_TRACE_DIRECTORY}")
   string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} LLDB_TEST_EXECUTABLE_CONFIGURED "${LLDB_TEST_EXECUTABLE}")
   string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} LLDB_TEST_COMPILER_CONFIGURED "${LLDB_TEST_COMPILER}")
   string(REPLACE ${CMAKE_CFG_INTDIR} ${config_type} LLDB_TEST_DSYMUTIL_CONFIGURED "${LLDB_TEST_DSYMUTIL}")
@@ -52,7 +50,6 @@
   string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_SOURCE_DIR_CONFIGURED "${LLDB_SOURCE_DIR}")
   string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_FRAMEWORK_DIR_CONFIGURED "${LLDB_FRAMEWORK_DIR}")
   string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_TEST_BUILD_DIRECTORY_CONFIGURED "${LLDB_TEST_BUILD_DIRECTORY}")
-  string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_TEST_TRACE_DIRECTORY_CONFIGURED "${LLDB_TEST_TRACE_DIRECTORY}")
   string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_TEST_EXECUTABLE_CONFIGURED "${LLDB_TEST_EXECUTABLE}")
   string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_TEST_COMPILER_CONFIGURED "${LLDB_TEST_COMPILER}")
   string(REPLACE ${CMAKE_CFG_INTDIR} "." LLDB_TEST_DSYMUTIL_CONFIGURED "${LLDB_TEST_DSYMUTIL}")
@@ -74,7 +71,6 @@
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_SOURCE_DIR_CONFIGURED "${LLDB_SOURCE_DIR}")
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_FRAMEWORK_DIR_CONFIGURED "${LLDB_FRAMEWORK_DIR}")
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_BUILD_DIRECTORY_CONFIGURED "${LLDB_TEST_BUILD_DIRECTORY}")
-string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_TRACE_DIRECTORY_CONFIGURED "${LLDB_TEST_TRACE_DIRECTORY}")
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_EXECUTA

[Lldb-commits] [PATCH] D92643: [lldb] Lookup static const members in FindGlobalVariables

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D92643#2433365 , @werat wrote:

> This approach feels a little bit "adhoc" (i.e. manually going through the DIE 
> entries for const members), but it works :) Please, let me know if there's a 
> better way to do this!

Are the static members included in the (SB)Type object that gets created when 
parsing the enclosing type? If yes, we might be able to retrieve them that way. 
Whether that would be cleaner -- I'm not sure...

(I would expect they are included, as otherwise they would be unavailable to 
the expression evaluator.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92643

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

[Lldb-commits] [PATCH] D92643: [lldb] Lookup static const members in FindGlobalVariables

[Andy Yankovsky via Phabricator via lldb-commits]

werat added a comment.

In D92643#2433428 , @labath wrote:

> Are the static members included in the (SB)Type object that gets created when 
> parsing the enclosing type? If yes, we might be able to retrieve them that 
> way. Whether that would be cleaner -- I'm not sure...
>
> (I would expect they are included, as otherwise they would be unavailable to 
> the expression evaluator.)

I think they're not as of now, but there's a patch from @teemperor to add them 
there -- D81471 

Also, I was looking at `Type/TypeSystem` and it seems there's no API right now 
to get static members from a `Type`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92643

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

[Lldb-commits] [PATCH] D82857: [LLDB] Add per-thread register infos shared pointer in gdb-remote

[Diana Picus via Phabricator via lldb-commits]

rovka added a comment.

Sorry, I had one more comment :) I think that should be all from my side.




Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:403
+  m_register_info_sp = std::make_shared();
+  m_register_info_sp->Clear();
 

Is it still necessary to call Clear? I think the default constructor should 
give us a "cleared" object.


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

https://reviews.llvm.org/D82857

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

[Lldb-commits] [PATCH] D92643: [lldb] Lookup static const members in FindGlobalVariables

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D92643#2433441 , @werat wrote:

> In D92643#2433428 , @labath wrote:
>
>> Are the static members included in the (SB)Type object that gets created 
>> when parsing the enclosing type? If yes, we might be able to retrieve them 
>> that way. Whether that would be cleaner -- I'm not sure...
>>
>> (I would expect they are included, as otherwise they would be unavailable to 
>> the expression evaluator.)
>
> I think they're not as of now, but there's a patch from @teemperor to add 
> them there -- D81471 
>
> Also, I was looking at `Type/TypeSystem` and it seems there's no API right 
> now to get static members from a `Type`.

I couldn't tell what you meant by this...  I would expect that a Type would 
tell you that static members exist, and their type, etc.  But I wouldn't expect 
a Type to be able to get a value for a static member.  After all, the type 
comes from some Module, which could be shared amongst a bunch of processes in 
lldb.  The actual member value could be different in each of these processes, 
but since the Type has no process, it has no way to get the correct value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92643

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

[Lldb-commits] [PATCH] D92667: [lldb] [Platform/POSIX] Use gdb-remote plugin when attaching

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

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
Herald added a subscriber: arichardson.
mgorny requested review of this revision.

Force gdb-remote plugin when attaching using the derivatives
of PlatformPOSIX class.  This is consistent with the behavior
for launching processes (via DebugProcess() method) and guarantees
consistent plugin choice on FreeBSD.


https://reviews.llvm.org/D92667

Files:
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/test/API/commands/process/attach-resume/TestAttachResume.py
  lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py


Index: lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
===
--- lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
+++ lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
@@ -19,7 +19,6 @@
 @expectedFailureAll(oslist=["linux"],
 triple=no_match('aarch64-.*-android'))
 # determining the architecture of the process fails
-@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48374")
 @skipIfReproducer # File synchronization is not supported during replay.
 def test(self):
 self.build()
Index: lldb/test/API/commands/process/attach-resume/TestAttachResume.py
===
--- lldb/test/API/commands/process/attach-resume/TestAttachResume.py
+++ lldb/test/API/commands/process/attach-resume/TestAttachResume.py
@@ -18,7 +18,6 @@
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipIfRemote
-@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr19310')
 @expectedFailureNetBSD
 @skipIfWindows # llvm.org/pr24778, llvm.org/pr21753
 @skipIfReproducer # FIXME: Unexpected packet during (active) replay
Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -388,8 +388,7 @@
 
   process_sp =
   target->CreateProcess(attach_info.GetListenerForProcess(debugger),
-attach_info.GetProcessPluginName(), nullptr,
-false);
+"gdb-remote", nullptr, true);
 
   if (process_sp) {
 ListenerSP listener_sp = attach_info.GetHijackListener();


Index: lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
===
--- lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
+++ lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
@@ -19,7 +19,6 @@
 @expectedFailureAll(oslist=["linux"],
 triple=no_match('aarch64-.*-android'))
 # determining the architecture of the process fails
-@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48374")
 @skipIfReproducer # File synchronization is not supported during replay.
 def test(self):
 self.build()
Index: lldb/test/API/commands/process/attach-resume/TestAttachResume.py
===
--- lldb/test/API/commands/process/attach-resume/TestAttachResume.py
+++ lldb/test/API/commands/process/attach-resume/TestAttachResume.py
@@ -18,7 +18,6 @@
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipIfRemote
-@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr19310')
 @expectedFailureNetBSD
 @skipIfWindows # llvm.org/pr24778, llvm.org/pr21753
 @skipIfReproducer # FIXME: Unexpected packet during (active) replay
Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -388,8 +388,7 @@
 
   process_sp =
   target->CreateProcess(attach_info.GetListenerForProcess(debugger),
-attach_info.GetProcessPluginName(), nullptr,
-false);
+"gdb-remote", nullptr, true);
 
   if (process_sp) {
 ListenerSP listener_sp = attach_info.GetHijackListener();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92643: [lldb] Lookup static const members in FindGlobalVariables

[Andy Yankovsky via Phabricator via lldb-commits]

werat added a comment.

In D92643#2434050 , @jingham wrote:

> I couldn't tell what you meant by this...  I would expect that a Type would 
> tell you that static members exist, and their type, etc.  But I wouldn't 
> expect a Type to be able to get a value for a static member.  After all, the 
> type comes from some Module, which could be shared amongst a bunch of 
> processes in lldb.  The actual member value could be different in each of 
> these processes, but since the Type has no process, it has no way to get the 
> correct value.

Yeah, of course you can't get a value for a static member just from the type 
(although, if it's an `inline static` then it could be possible?). But 
something like `VariableList EnumerateStaticMembers(Target, Type)` would be 
nice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92643

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

[Lldb-commits] [PATCH] D92667: [lldb] [Platform/POSIX] Use gdb-remote plugin when attaching

[Ed Maste via Phabricator via lldb-commits]

emaste added a comment.

No objection here. I'm curious why the two modified tests work on Linux or 
NetBSD today though?


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

https://reviews.llvm.org/D92667

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

[Lldb-commits] [PATCH] D92187: [lldb] [POSIX-DYLD] Add libraries from initial eTakeSnapshot action

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

mgorny updated this revision to Diff 309587.
mgorny retitled this revision from "[lldb] [FreeBSD] Fix establishing DT_NEEDED 
libraries from dyld" to "[lldb] [POSIX-DYLD] Add libraries from initial 
eTakeSnapshot action".
mgorny edited the summary of this revision.
mgorny added a comment.

Updated as discussed on IRC, to instead add libs returned by the first 'take 
snapshot' action.


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

https://reviews.llvm.org/D92187

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
  lldb/test/API/api/multithreaded/TestMultithreaded.py
  
lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
  
lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
  lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test

Index: lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test
===
--- lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test
+++ lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test
@@ -3,7 +3,6 @@
 
 # REQUIRES: target-x86_64
 # UNSUPPORTED: system-windows
-# XFAIL: system-freebsd
 
 # RUN: %clang_host %p/Inputs/call-asm.c -x assembler-with-cpp %p/Inputs/thread-step-out-ret-addr-check.s -o %t
 # RUN: not %lldb %t -s %s -b 2>&1 | FileCheck %s
Index: lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
===
--- lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
+++ lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
@@ -120,7 +120,7 @@
 
 @llgs_test
 @skipUnlessPlatform(["linux", "android", "freebsd", "netbsd"])
-@expectedFailureAll(oslist=["freebsd", "netbsd"])
+@expectedFailureNetBSD
 def test_libraries_svr4_load_addr(self):
 self.setup_test()
 self.libraries_svr4_has_correct_load_addr()
Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -918,7 +918,6 @@
 self.qMemoryRegionInfo_is_supported()
 
 @llgs_test
-@expectedFailureAll(oslist=["freebsd"])
 def test_qMemoryRegionInfo_is_supported_llgs(self):
 self.init_llgs_test()
 self.build()
@@ -983,7 +982,6 @@
 self.qMemoryRegionInfo_reports_code_address_as_executable()
 
 @skipIfWindows # No pty support to test any inferior output
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test_qMemoryRegionInfo_reports_code_address_as_executable_llgs(self):
 self.init_llgs_test()
@@ -1050,7 +1048,6 @@
 self.qMemoryRegionInfo_reports_stack_address_as_readable_writeable()
 
 @skipIfWindows # No pty support to test any inferior output
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test_qMemoryRegionInfo_reports_stack_address_as_readable_writeable_llgs(
 self):
@@ -1117,7 +1114,6 @@
 self.qMemoryRegionInfo_reports_heap_address_as_readable_writeable()
 
 @skipIfWindows # No pty support to test any inferior output
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test_qMemoryRegionInfo_reports_heap_address_as_readable_writeable_llgs(
 self):
Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -23,7 +23,6 @@
 'main.cpp',
 '// Run here before printing memory regions')
 
-@expectedFailureAll(oslist=["freebsd"])
 def test(self):
 self.build()
 
Index: lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
===
--- lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
+++ lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
@@ -15,7 +15,6 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
-@expectedFailureAll(oslist=["freebsd"], bugnumber='llvm.org/pr48373')
 @expectedFailureNetBSD
 def test(self):
 self.build()
Index: lldb/test/API/api/multithreaded/TestMultithreaded.py
===
--- lldb/test/API/api/multithreaded/TestMultithreaded.py
+++ lldb/test/API/api/multi

[Lldb-commits] [PATCH] D92667: [lldb] [Platform/POSIX] Use gdb-remote plugin when attaching

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D92667#2434223 , @emaste wrote:

> No objection here. I'm curious why the two modified tests work on Linux or 
> NetBSD today though?

This is a debt of having 2 process plugins. NetBSD and Linux ship with a single 
one only.


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

https://reviews.llvm.org/D92667

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

[Lldb-commits] [PATCH] D92692: Ignore DBGArchitecture from DebugSymbols DBGShellCommands outputs

[Jason Molenda via Phabricator via lldb-commits]

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

The plist returned by a DBGShellCommands program (dsymForUUID etc) may include 
an architecture in the DBGArchitecture kv.  lldb uses this to fill in the 
ArchSpec of the ModuleSpec it returns along with the executable/dsym file path. 
 The bug I'm fixing here is that lldb and the provider of DBGArchitecture may 
*disagree* about what the architecture is called, and when lldb tries to load 
the actual binary, it will find that the ArchSpec of the binary does not match 
the ModuleSpec it is looking for, and ignore the binary.

In the specific case I'm fixing, it was an arm cortex-m corefile where lldb 
calls it armv7em, but dwarfdump (I suspect) was calling it thumbv7em to for MC 
to treat the instructions as pure thumb (lldb does the same when it creates the 
MCDisassembler).  The process of picking the matching ModuleSpec is looking for 
an exact match, and this fails.

The architecture in the plist is of little importance, we're working with 
binaries with UUIDs and that is what we need to be depending on.  I thought 
about different ways of trying to address this, and ultimately, ignoring 
DBGArchitecture which adds no benefits to lldb, is the best.

rdar://problem/71612561


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92692

Files:
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
  lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py


Index: lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
===
--- lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
+++ lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
@@ -85,7 +85,7 @@
 'fi',
 'echo "$uuid"',
 '',
-'echo "DBGArchitecturex86_64"',
+'echo "DBGArchitecturei386"',
 'echo "DBGDSYMPath$dsym"',
 'echo 
"DBGSymbolRichExecutable$bin"',
 'echo ""',
Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -342,13 +342,6 @@
   }
 }
 
-cf_str = (CFStringRef)CFDictionaryGetValue((CFDictionaryRef)uuid_dict,
-   CFSTR("DBGArchitecture"));
-if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) {
-  if (CFCString::FileSystemRepresentation(cf_str, str))
-module_spec.GetArchitecture().SetTriple(str.c_str());
-}
-
 std::string DBGBuildSourcePath;
 std::string DBGSourcePath;
 


Index: lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
===
--- lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
+++ lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
@@ -85,7 +85,7 @@
 'fi',
 'echo "$uuid"',
 '',
-'echo "DBGArchitecturex86_64"',
+'echo "DBGArchitecturei386"',
 'echo "DBGDSYMPath$dsym"',
 'echo "DBGSymbolRichExecutable$bin"',
 'echo ""',
Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -342,13 +342,6 @@
   }
 }
 
-cf_str = (CFStringRef)CFDictionaryGetValue((CFDictionaryRef)uuid_dict,
-   CFSTR("DBGArchitecture"));
-if (cf_str && CFGetTypeID(cf_str) == CFStringGetTypeID()) {
-  if (CFCString::FileSystemRepresentation(cf_str, str))
-module_spec.GetArchitecture().SetTriple(str.c_str());
-}
-
 std::string DBGBuildSourcePath;
 std::string DBGSourcePath;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92692: Ignore DBGArchitecture from DebugSymbols DBGShellCommands outputs

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

Ah, I forgot to add.  I added a test by having an existing corefile test (which 
loads an x86_64 binary) have a plist that claims it is i386.  It's close to the 
same thing, and results in the same kind of erroring if lldb trusts the arch 
from the plist.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92692

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

[Lldb-commits] [PATCH] D92667: [lldb] [Platform/POSIX] Use gdb-remote plugin when attaching

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

mgorny added a comment.

In D92667#2434223 , @emaste wrote:

> No objection here. I'm curious why the two modified tests work on Linux or 
> NetBSD today though?

What Kamil said. We're still having two plugins, while these other platforms 
have only a remote plugin nowadays. Nevertheless, the code here is inconsistent 
and it's worth fixing, even if it will make no difference long-term.


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

https://reviews.llvm.org/D92667

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

[Lldb-commits] [PATCH] D92264: [lldb] [POSIX-DYLD] Update the cached exe path after attach

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

mgorny updated this revision to Diff 309684.
mgorny added a comment.

With D92667 , I've finally found a way to 
reproduce this without running lldb-server explicitly — we need to spawn the 
program via relative path though. I've added a test that does that, and 
compares the address from `image list` with the one gotten after explicitly 
creating target via file path. This clearly reproduces the problem if you don't 
modify the sources (i.e. mismatch between `0x40` and `0x20`).


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

https://reviews.llvm.org/D92264

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/test/API/commands/process/attach/TestProcessAttach.py


Index: lldb/test/API/commands/process/attach/TestProcessAttach.py
===
--- lldb/test/API/commands/process/attach/TestProcessAttach.py
+++ lldb/test/API/commands/process/attach/TestProcessAttach.py
@@ -77,6 +77,41 @@
 process = target.GetProcess()
 self.assertTrue(process, PROCESS_IS_VALID)
 
+def test_attach_to_process_by_id_correct_executable_offset(self):
+"""
+Test that after attaching to a process the executable offset
+is determined correctly on FreeBSD.  This is a regression test
+for dyld plugin getting the correct executable path,
+and therefore being able to identify it in the module list.
+"""
+
+self.build()
+exe = self.getBuildArtifact(exe_name)
+
+# In order to reproduce, we must spawn using a relative path
+popen = self.spawnSubprocess(os.path.relpath(exe))
+
+self.runCmd("process attach -p " + str(popen.pid))
+
+# Grab the offset and filename from the image list
+m = self.match("image list -h -f",
+   [r"^\[\s*0\]\s*(0x[0-9a-fA-F]+)\s*([^\n]*)"])
+self.assertTrue(os.path.samefile(m.group(2), exe))
+offset_attach = m.group(1)
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# Now grab the file itself and compare the offset
+self.runCmd("target create " + exe)
+m = self.match("image list -h -f",
+   [r"^\[\s*0\]\s*(0x[0-9a-fA-F]+)\s*([^\n]*)"])
+self.assertTrue(os.path.samefile(m.group(2), exe))
+offset_target = m.group(1)
+
+self.assertEqual(offset_attach, offset_target)
+
 def tearDown(self):
 # Destroy process before TestBase.tearDown()
 self.dbg.GetSelectedTarget().GetProcess().Destroy()
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -101,6 +101,7 @@
 
   ModuleSP executable_sp = GetTargetExecutable();
   ResolveExecutableModule(executable_sp);
+  m_rendezvous.UpdateExecutablePath();
 
   // find the main process load offset
   addr_t load_offset = ComputeLoadOffset();
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
@@ -60,6 +60,9 @@
 
   DYLDRendezvous(lldb_private::Process *process);
 
+  /// Update the cached executable path.
+  void UpdateExecutablePath();
+
   /// Update the internal snapshot of runtime linker rendezvous and recompute
   /// the currently loaded modules.
   ///
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -94,12 +94,13 @@
 : m_process(process), m_rendezvous_addr(LLDB_INVALID_ADDRESS), m_current(),
   m_previous(), m_loaded_modules(), m_soentries(), m_added_soentries(),
   m_removed_soentries() {
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
-
   m_thread_info.valid = false;
+  UpdateExecutablePath();
+}
 
-  // Cache a copy of the executable path
+void DYLDRendezvous::UpdateExecutablePath() {
   if (m_process) {
+Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
 Module *exe_mod = m_process->GetTarget().GetExecutableModulePointer();
 if (exe_mod) {
   m_exe_file_spec = exe_mod->GetPlatformFileSpec();


Index: lldb/test/API/commands/process/attach/TestProcessAttach.py
=

[Lldb-commits] [PATCH] D92667: [lldb] [Platform/POSIX] Use gdb-remote plugin when attaching

[Ed Maste via Phabricator via lldb-commits]

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

Ah, of course. SGTM.


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

https://reviews.llvm.org/D92667

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

[Lldb-commits] [PATCH] D92712: [debugserver] Honor the cpu sub type if specified

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: jasonmolenda.
Herald added a subscriber: pengfei.
JDevlieghere requested review of this revision.

Use the newly added spawnattr API, `posix_spawnattr_setarchpref_np`, to select 
a slice preferences per cpu and subcpu types, instead of just cpu with 
`posix_spawnattr_setarchpref_np`. In the patch it only does this for `x86_64` 
and `x86_64h`, but I would like to extend the parsing to more subtypes.

Tested by compiling a fat binary with different print statements depending on 
the architecture. I considered adding a test for it, but it doesn't seem worth 
the hassle to figure out the host architecture and the availability.


https://reviews.llvm.org/D92712

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNBArch.cpp
  lldb/tools/debugserver/source/DNBArch.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm

Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -3160,9 +3160,9 @@
 
   case eLaunchFlavorPosixSpawn:
 m_pid = MachProcess::PosixSpawnChildForPTraceDebugging(
-path, DNBArchProtocol::GetArchitecture(), argv, envp, working_directory,
-stdin_path, stdout_path, stderr_path, no_stdio, this, disable_aslr,
-launch_err);
+path, DNBArchProtocol::GetCPUType(), DNBArchProtocol::GetCPUSubType(),
+argv, envp, working_directory, stdin_path, stdout_path, stderr_path,
+no_stdio, this, disable_aslr, launch_err);
 break;
 
   default:
@@ -3222,10 +3222,10 @@
 }
 
 pid_t MachProcess::PosixSpawnChildForPTraceDebugging(
-const char *path, cpu_type_t cpu_type, char const *argv[],
-char const *envp[], const char *working_directory, const char *stdin_path,
-const char *stdout_path, const char *stderr_path, bool no_stdio,
-MachProcess *process, int disable_aslr, DNBError &err) {
+const char *path, cpu_type_t cpu_type, cpu_subtype_t cpu_subtype,
+char const *argv[], char const *envp[], const char *working_directory,
+const char *stdin_path, const char *stdout_path, const char *stderr_path,
+bool no_stdio, MachProcess *process, int disable_aslr, DNBError &err) {
   posix_spawnattr_t attr;
   short flags;
   DNBLogThreadedIf(LOG_PROCESS,
@@ -3268,24 +3268,36 @@
 
 // On SnowLeopard we should set "DYLD_NO_PIE" in the inferior environment
 
-#if !defined(__arm__)
-
-  // We don't need to do this for ARM, and we really shouldn't now that we
-  // have multiple CPU subtypes and no posix_spawnattr call that allows us
-  // to set which CPU subtype to launch...
   if (cpu_type != 0) {
 size_t ocount = 0;
-err.SetError(::posix_spawnattr_setbinpref_np(&attr, 1, &cpu_type, &ocount),
- DNBError::POSIX);
-if (err.Fail() || DNBLogCheckLogBit(LOG_PROCESS))
-  err.LogThreaded("::posix_spawnattr_setbinpref_np ( &attr, 1, cpu_type = "
-  "0x%8.8x, count => %llu )",
-  cpu_type, (uint64_t)ocount);
 
-if (err.Fail() != 0 || ocount != 1)
-  return INVALID_NUB_PROCESS;
+if (cpu_subtype != 0) {
+  if (@available(macOS 10.16, ios 10.14, watchos 7.0, tvos 14.0,
+ bridgeos 5.0, *)) {
+err.SetError(posix_spawnattr_setarchpref_np(&attr, 1, &cpu_type,
+&cpu_subtype, &ocount));
+if (err.Fail() || DNBLogCheckLogBit(LOG_PROCESS))
+  err.LogThreaded(
+  "::posix_spawnattr_setarchpref_np ( &attr, 1, cpu_type = "
+  "0x%8.8x, cpu_subtype = 0x%8.8x, count => %llu )",
+  cpu_type, cpu_subtype, (uint64_t)ocount);
+if (err.Fail() != 0 || ocount != 1)
+  return INVALID_NUB_PROCESS;
+  }
+} else {
+  err.SetError(
+  ::posix_spawnattr_setbinpref_np(&attr, 1, &cpu_type, &ocount),
+  DNBError::POSIX);
+  if (err.Fail() || DNBLogCheckLogBit(LOG_PROCESS))
+err.LogThreaded(
+"::posix_spawnattr_setbinpref_np ( &attr, 1, cpu_type = "
+"0x%8.8x, count => %llu )",
+cpu_type, (uint64_t)ocount);
+
+  if (err.Fail() != 0 || ocount != 1)
+return INVALID_NUB_PROCESS;
+}
   }
-#endif
 
   PseudoTerminal pty;
 
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.h
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -92,10 +92,10 @@
char const *envp[],
MachProcess *process, DNBError &err);
   static pid_t PosixSpawnChildForPTraceDebugging(
-  const char *path, cpu_type_t cpu_type, char const *argv[],
-  ch

[Lldb-commits] [PATCH] D92712: [debugserver] Honor the cpu sub type if specified

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 309710.
JDevlieghere added a comment.

- Fix bug that would not fall back to `posix_spawnattr_setarchpref_np` when the 
availability check failed.
- Add subtypes for `arm64e` and `armv8`


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

https://reviews.llvm.org/D92712

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNBArch.cpp
  lldb/tools/debugserver/source/DNBArch.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm

Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -3160,9 +3160,9 @@
 
   case eLaunchFlavorPosixSpawn:
 m_pid = MachProcess::PosixSpawnChildForPTraceDebugging(
-path, DNBArchProtocol::GetArchitecture(), argv, envp, working_directory,
-stdin_path, stdout_path, stderr_path, no_stdio, this, disable_aslr,
-launch_err);
+path, DNBArchProtocol::GetCPUType(), DNBArchProtocol::GetCPUSubType(),
+argv, envp, working_directory, stdin_path, stdout_path, stderr_path,
+no_stdio, this, disable_aslr, launch_err);
 break;
 
   default:
@@ -3222,10 +3222,10 @@
 }
 
 pid_t MachProcess::PosixSpawnChildForPTraceDebugging(
-const char *path, cpu_type_t cpu_type, char const *argv[],
-char const *envp[], const char *working_directory, const char *stdin_path,
-const char *stdout_path, const char *stderr_path, bool no_stdio,
-MachProcess *process, int disable_aslr, DNBError &err) {
+const char *path, cpu_type_t cpu_type, cpu_subtype_t cpu_subtype,
+char const *argv[], char const *envp[], const char *working_directory,
+const char *stdin_path, const char *stdout_path, const char *stderr_path,
+bool no_stdio, MachProcess *process, int disable_aslr, DNBError &err) {
   posix_spawnattr_t attr;
   short flags;
   DNBLogThreadedIf(LOG_PROCESS,
@@ -3268,24 +3268,40 @@
 
 // On SnowLeopard we should set "DYLD_NO_PIE" in the inferior environment
 
-#if !defined(__arm__)
-
-  // We don't need to do this for ARM, and we really shouldn't now that we
-  // have multiple CPU subtypes and no posix_spawnattr call that allows us
-  // to set which CPU subtype to launch...
   if (cpu_type != 0) {
 size_t ocount = 0;
-err.SetError(::posix_spawnattr_setbinpref_np(&attr, 1, &cpu_type, &ocount),
- DNBError::POSIX);
-if (err.Fail() || DNBLogCheckLogBit(LOG_PROCESS))
-  err.LogThreaded("::posix_spawnattr_setbinpref_np ( &attr, 1, cpu_type = "
-  "0x%8.8x, count => %llu )",
-  cpu_type, (uint64_t)ocount);
+bool slice_preference_set = false;
+
+if (cpu_subtype != 0) {
+  if (@available(macOS 10.16, ios 10.14, watchos 7.0, tvos 14.0,
+ bridgeos 5.0, *)) {
+err.SetError(posix_spawnattr_setarchpref_np(&attr, 1, &cpu_type,
+&cpu_subtype, &ocount));
+slice_preference_set = err.Success();
+if (err.Fail() || DNBLogCheckLogBit(LOG_PROCESS))
+  err.LogThreaded(
+  "::posix_spawnattr_setarchpref_np ( &attr, 1, cpu_type = "
+  "0x%8.8x, cpu_subtype = 0x%8.8x, count => %llu )",
+  cpu_type, cpu_subtype, (uint64_t)ocount);
+if (err.Fail() != 0 || ocount != 1)
+  return INVALID_NUB_PROCESS;
+  }
+}
 
-if (err.Fail() != 0 || ocount != 1)
-  return INVALID_NUB_PROCESS;
+if (!slice_preference_set) {
+  err.SetError(
+  ::posix_spawnattr_setbinpref_np(&attr, 1, &cpu_type, &ocount),
+  DNBError::POSIX);
+  if (err.Fail() || DNBLogCheckLogBit(LOG_PROCESS))
+err.LogThreaded(
+"::posix_spawnattr_setbinpref_np ( &attr, 1, cpu_type = "
+"0x%8.8x, count => %llu )",
+cpu_type, (uint64_t)ocount);
+
+  if (err.Fail() != 0 || ocount != 1)
+return INVALID_NUB_PROCESS;
+}
   }
-#endif
 
   PseudoTerminal pty;
 
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.h
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -92,10 +92,10 @@
char const *envp[],
MachProcess *process, DNBError &err);
   static pid_t PosixSpawnChildForPTraceDebugging(
-  const char *path, cpu_type_t cpu_type, char const *argv[],
-  char const *envp[], const char *working_directory, const char *stdin_path,
-  const char *stdout_path, const char *stderr_path, bool no_stdio,
-  MachProcess *process, int disable_aslr, DNBError &err);
+  const char *path, cpu_type_t cpu_type, cpu_subtype_t cpu_subtype,
+

[Lldb-commits] [PATCH] D92712: [debugserver] Honor the cpu sub type if specified

[Jason Molenda via Phabricator via lldb-commits]

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

Ah very cool, I didn't realize we had a new posix_spawn attr setter that could 
set the cpu subtype.


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

https://reviews.llvm.org/D92712

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

[Lldb-commits] [lldb] 315fab4 - [lldb] Remove unused argument to expectedFailure

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-12-04T20:37:41-08:00
New Revision: 315fab428d6084b09f82468740df5697ab3bb93b

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

LOG: [lldb] Remove unused argument to expectedFailure

Added: 


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

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index 07b1d15810ab..2013c5241007 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -86,8 +86,7 @@ def _match_decorator_property(expected, actual):
 else:
 return expected == actual
 
-
-def expectedFailure(func, bugnumber=None):
+def expectedFailure(func):
 return unittest2.expectedFailure(func)
 
 def expectedFailureIfFn(expected_fn, bugnumber=None):



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

[Lldb-commits] [lldb] 0db3757 - [debugserver] Honor the cpu sub type if specified

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-12-04T20:37:41-08:00
New Revision: 0db37576c1dddbec1021b50320ae84a9d838742c

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

LOG: [debugserver] Honor the cpu sub type if specified

Use the newly added spawnattr API, posix_spawnattr_setarchpref_np, to
select a slice preferences per cpu and subcpu types, instead of just cpu
with posix_spawnattr_setarchpref_np.

rdar://16094957

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

Added: 


Modified: 
lldb/tools/debugserver/source/DNB.cpp
lldb/tools/debugserver/source/DNBArch.cpp
lldb/tools/debugserver/source/DNBArch.h
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm

Removed: 




diff  --git a/lldb/tools/debugserver/source/DNB.cpp 
b/lldb/tools/debugserver/source/DNB.cpp
index 2d6516e8c654..a69fc33fc9b0 100644
--- a/lldb/tools/debugserver/source/DNB.cpp
+++ b/lldb/tools/debugserver/source/DNB.cpp
@@ -1762,19 +1762,52 @@ nub_bool_t DNBSetArchitecture(const char *arch) {
   if (arch && arch[0]) {
 if (strcasecmp(arch, "i386") == 0)
   return DNBArchProtocol::SetArchitecture(CPU_TYPE_I386);
-else if ((strcasecmp(arch, "x86_64") == 0) ||
- (strcasecmp(arch, "x86_64h") == 0))
-  return DNBArchProtocol::SetArchitecture(CPU_TYPE_X86_64);
-else if (strstr(arch, "arm64_32") == arch || 
+else if (strcasecmp(arch, "x86_64") == 0)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_X86_64,
+  CPU_SUBTYPE_X86_64_ALL);
+else if (strcasecmp(arch, "x86_64h") == 0)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_X86_64,
+  CPU_SUBTYPE_X86_64_H);
+else if (strstr(arch, "arm64_32") == arch ||
  strstr(arch, "aarch64_32") == arch)
   return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM64_32);
 else if (strstr(arch, "arm64e") == arch)
-  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM64);
-else if (strstr(arch, "arm64") == arch || strstr(arch, "armv8") == arch ||
- strstr(arch, "aarch64") == arch)
-  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM64);
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM64,
+  CPU_SUBTYPE_ARM64E);
+else if (strstr(arch, "arm64") == arch || strstr(arch, "aarch64") == arch)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM64,
+  CPU_SUBTYPE_ARM64_ALL);
+else if (strstr(arch, "armv8") == arch)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM64,
+  CPU_SUBTYPE_ARM64_V8);
+else if (strstr(arch, "armv7em") == arch)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM,
+  CPU_SUBTYPE_ARM_V7EM);
+else if (strstr(arch, "armv7m") == arch)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM,
+  CPU_SUBTYPE_ARM_V7M);
+else if (strstr(arch, "armv7k") == arch)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM,
+  CPU_SUBTYPE_ARM_V7K);
+else if (strstr(arch, "armv7s") == arch)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM,
+  CPU_SUBTYPE_ARM_V7S);
+else if (strstr(arch, "armv7") == arch)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM, 
CPU_SUBTYPE_ARM_V7);
+else if (strstr(arch, "armv6m") == arch)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM,
+  CPU_SUBTYPE_ARM_V6M);
+else if (strstr(arch, "armv6") == arch)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM, 
CPU_SUBTYPE_ARM_V6);
+else if (strstr(arch, "armv5") == arch)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM,
+  CPU_SUBTYPE_ARM_V5TEJ);
+else if (strstr(arch, "armv4t") == arch)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM,
+  CPU_SUBTYPE_ARM_V4T);
 else if (strstr(arch, "arm") == arch)
-  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM);
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM,
+  CPU_SUBTYPE_ARM_ALL);
   }
   return false;
 }

diff  --git a/lldb/tools/debugserver/source/DNBArch.cpp 
b/lldb/tools/debugserver/source/DNBArch.cpp
index 931d623647fa..f3848bc19bf9 100644
--- a/lldb/tools/debugserver/source/DNBArch.cpp
+++ b/lldb/tools/debugserver/source/DNBArch.cpp
@@ -21,6 +21,7 @@
 typedef std::

[Lldb-commits] [PATCH] D92712: [debugserver] Honor the cpu sub type if specified

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0db37576c1dd: [debugserver] Honor the cpu sub type if 
specified (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D92712?vs=309710&id=309713#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92712

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNBArch.cpp
  lldb/tools/debugserver/source/DNBArch.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm

Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -3160,9 +3160,9 @@
 
   case eLaunchFlavorPosixSpawn:
 m_pid = MachProcess::PosixSpawnChildForPTraceDebugging(
-path, DNBArchProtocol::GetArchitecture(), argv, envp, working_directory,
-stdin_path, stdout_path, stderr_path, no_stdio, this, disable_aslr,
-launch_err);
+path, DNBArchProtocol::GetCPUType(), DNBArchProtocol::GetCPUSubType(),
+argv, envp, working_directory, stdin_path, stdout_path, stderr_path,
+no_stdio, this, disable_aslr, launch_err);
 break;
 
   default:
@@ -3222,10 +3222,10 @@
 }
 
 pid_t MachProcess::PosixSpawnChildForPTraceDebugging(
-const char *path, cpu_type_t cpu_type, char const *argv[],
-char const *envp[], const char *working_directory, const char *stdin_path,
-const char *stdout_path, const char *stderr_path, bool no_stdio,
-MachProcess *process, int disable_aslr, DNBError &err) {
+const char *path, cpu_type_t cpu_type, cpu_subtype_t cpu_subtype,
+char const *argv[], char const *envp[], const char *working_directory,
+const char *stdin_path, const char *stdout_path, const char *stderr_path,
+bool no_stdio, MachProcess *process, int disable_aslr, DNBError &err) {
   posix_spawnattr_t attr;
   short flags;
   DNBLogThreadedIf(LOG_PROCESS,
@@ -3268,24 +3268,40 @@
 
 // On SnowLeopard we should set "DYLD_NO_PIE" in the inferior environment
 
-#if !defined(__arm__)
-
-  // We don't need to do this for ARM, and we really shouldn't now that we
-  // have multiple CPU subtypes and no posix_spawnattr call that allows us
-  // to set which CPU subtype to launch...
   if (cpu_type != 0) {
 size_t ocount = 0;
-err.SetError(::posix_spawnattr_setbinpref_np(&attr, 1, &cpu_type, &ocount),
- DNBError::POSIX);
-if (err.Fail() || DNBLogCheckLogBit(LOG_PROCESS))
-  err.LogThreaded("::posix_spawnattr_setbinpref_np ( &attr, 1, cpu_type = "
-  "0x%8.8x, count => %llu )",
-  cpu_type, (uint64_t)ocount);
+bool slice_preference_set = false;
+
+if (cpu_subtype != 0) {
+  if (@available(macOS 10.16, ios 10.14, watchos 7.0, tvos 14.0,
+ bridgeos 5.0, *)) {
+err.SetError(posix_spawnattr_setarchpref_np(&attr, 1, &cpu_type,
+&cpu_subtype, &ocount));
+slice_preference_set = err.Success();
+if (err.Fail() || DNBLogCheckLogBit(LOG_PROCESS))
+  err.LogThreaded(
+  "::posix_spawnattr_setarchpref_np ( &attr, 1, cpu_type = "
+  "0x%8.8x, cpu_subtype = 0x%8.8x, count => %llu )",
+  cpu_type, cpu_subtype, (uint64_t)ocount);
+if (err.Fail() != 0 || ocount != 1)
+  return INVALID_NUB_PROCESS;
+  }
+}
 
-if (err.Fail() != 0 || ocount != 1)
-  return INVALID_NUB_PROCESS;
+if (!slice_preference_set) {
+  err.SetError(
+  ::posix_spawnattr_setbinpref_np(&attr, 1, &cpu_type, &ocount),
+  DNBError::POSIX);
+  if (err.Fail() || DNBLogCheckLogBit(LOG_PROCESS))
+err.LogThreaded(
+"::posix_spawnattr_setbinpref_np ( &attr, 1, cpu_type = "
+"0x%8.8x, count => %llu )",
+cpu_type, (uint64_t)ocount);
+
+  if (err.Fail() != 0 || ocount != 1)
+return INVALID_NUB_PROCESS;
+}
   }
-#endif
 
   PseudoTerminal pty;
 
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.h
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -92,10 +92,10 @@
char const *envp[],
MachProcess *process, DNBError &err);
   static pid_t PosixSpawnChildForPTraceDebugging(
-  const char *path, cpu_type_t cpu_type, char const *argv[],
-  char const *envp[], const char *working_directory, const char *stdin_path,
-  const char *stdout_path, const char *stderr_path, bool no_stdio,
-  MachProcess *process, int