[Lldb-commits] [PATCH] D76697: [lldb] Replace debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType with lldbassert

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

LGTM


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

https://reviews.llvm.org/D76697

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

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

[Davide Italiano via Phabricator via lldb-commits]

davide closed this revision.
davide added a comment.

Somebody will pick this up, probably in a different form.


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

https://reviews.llvm.org/D82813

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

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: lldb/source/Symbol/Type.cpp:378
 m_byte_size_has_value = true;
+return m_byte_size;
   }

shafik wrote:
> labath wrote:
> > davide wrote:
> > > shafik wrote:
> > > > Wouldn't it be better to turn `m_byte_size` into an `Optional`? As this 
> > > > fix shows this having this additional state we carry around is error 
> > > > prone.
> > > This is a much larger project, probably worth considering.
> > > With that in mind, it wouldn't have prevented this bug from happening, as 
> > > this falls through and returns an `Optional` anyway. 
> > > Yay fuzz testing (that found this), I would say.
> > IIRC, this was done this way to reduce sizeof(Type). Note that the external 
> > interface is Optional and the only place that knows (should 
> > know?) about this encoding is this function.
> It would have prevented this bug as you just return `m_byte_size` no matter 
> what.
If you think it's an improvement, please consider submitting a review, I would 
be eager to look at it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86436

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

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

[Davide Italiano via Phabricator via lldb-commits]

davide added a subscriber: teemperor.
davide added a comment.

@labath something we noticed when finding this (and related bugs) is that 
`frame var` carries a decent diagnostic

  (int *) l_125 = 

and the expression parser returns just returns something not particularly 
useful:

  (lldb) p l_125 
  error: :43:31: no member named 'l_125' in namespace 
'$__lldb_local_vars'
  using $__lldb_local_vars::l_125;
^
  error: :1:1: use of undeclared identifier 'l_125'
  l_125

From my testing infrastructure/fuzzing perspective the two are 
indistinguishable, as the script I've written chokes on both, but it would be 
better from an ergonomics point of view if `p` would return something 
meaningful, if possible (even if there's a bug in lldb). Do you think it's 
worth filing a PR? (also, cc: @teemperor for ideas as he spent a fair amount of 
time working on the expression parser)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86436

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

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: lldb/source/Symbol/Type.cpp:378
 m_byte_size_has_value = true;
+return m_byte_size;
   }

shafik wrote:
> Wouldn't it be better to turn `m_byte_size` into an `Optional`? As this fix 
> shows this having this additional state we carry around is error prone.
This is a much larger project, probably worth considering.
With that in mind, it wouldn't have prevented this bug from happening, as this 
falls through and returns an `Optional` anyway. 
Yay fuzz testing (that found this), I would say.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86436

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

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

[Davide Italiano via Phabricator via lldb-commits]

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

I came to the same conclusion when analyzing 
https://bugs.llvm.org/show_bug.cgi?id=47257 (but you beat me to the punch).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86436

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

[Lldb-commits] [PATCH] D85396: Fix a small memory leak in VectorType.cpp and BlockPointer.cpp

[Davide Italiano via Phabricator via lldb-commits]

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

This is correct to the best of my understanding. Thank you Jason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85396

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

[Lldb-commits] [PATCH] D85388: [lldb] Fix bug in skipIfRosetta decorator

[Davide Italiano via Phabricator via lldb-commits]

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

yes, this makes sense. We could refine the check in future.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D85388

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:2
+EXE := a.out
+CFLAGS := -O1
+

davide wrote:
> This is fundamentally a no-go. Depending on the optimization pipeline passes 
> this in a register is an assumption that might go away at some point in the 
> future and this test won't test what it has to & will still pass silently.
> 
> Something like this might work:
> https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html
*depending on the optimization pipeline, the fact that is passed in a register 
is an assumption that


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Davide Italiano via Phabricator via lldb-commits]

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



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:2
+EXE := a.out
+CFLAGS := -O1
+

This is fundamentally a no-go. Depending on the optimization pipeline passes 
this in a register is an assumption that might go away at some point in the 
future and this test won't test what it has to & will still pass silently.

Something like this might work:
https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85049: Unify the code that updates the ArchSpec after finding a fat binary with how it is done for a lean binary

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: lldb/source/Target/TargetList.cpp:110-111
+// architecture so that the platform matching can be more accurate.
+if (!platform_arch.TripleOSWasSpecified() ||
+!platform_arch.TripleVendorWasSpecified()) {
+  prefer_platform_arch = true;

Is this check strict enough? I thought it should be only TripleOSWasSpecified 
-- what we can infer from the vendor?


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

https://reviews.llvm.org/D85049

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

[Lldb-commits] [PATCH] D85365: [lldb] Modify the `skipIfRemote` decorator so we can skip all PExpect tests.

[Davide Italiano via Phabricator via lldb-commits]

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

I assume we're still allowing to put the decorator on a test-by-test basis, and 
that seems the case from what I see.
If so, LG.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D85365

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

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

[Davide Italiano via Phabricator via lldb-commits]

davide closed this revision.
davide added a comment.

https://github.com/llvm/llvm-project/commit/57605758b5de3726eec1d6e587de1003af1ab5b7


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

https://reviews.llvm.org/D84263

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

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

[Davide Italiano via Phabricator via lldb-commits]

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

Reviewed by Jason privately.


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

https://reviews.llvm.org/D84263

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

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

[Davide Italiano via Phabricator via lldb-commits]

davide updated this revision to Diff 282073.
davide added a comment.

Added the check that Jason requested.


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

https://reviews.llvm.org/D84263

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/debugserver.cpp


Index: lldb/tools/debugserver/source/debugserver.cpp
===
--- lldb/tools/debugserver/source/debugserver.cpp
+++ lldb/tools/debugserver/source/debugserver.cpp
@@ -878,6 +878,8 @@
// -F localhost:1234 -- /bin/ls"
 {NULL, 0, NULL, 0}};
 
+int communication_fd = -1;
+
 // main
 int main(int argc, char *argv[]) {
   // If debugserver is launched with DYLD_INSERT_LIBRARIES, unset it so we
@@ -944,7 +946,6 @@
   int ch;
   int long_option_index = 0;
   int debug = 0;
-  int communication_fd = -1;
   std::string compile_options;
   std::string waitfor_pid_name; // Wait for a process that starts with this 
name
   std::string attach_pid_name;
Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -442,6 +442,39 @@
   if (err_str && err_len > 0)
 err_str[0] = '\0';
 
+  if (getenv("LLDB_DEBUGSERVER_PATH") == NULL) {
+int mib[] = {CTL_KERN, KERN_PROC, KERN_PROC_PID,
+ static_cast(attach_pid)};
+struct kinfo_proc processInfo;
+size_t bufsize = sizeof(processInfo);
+if (sysctl(mib, (unsigned)(sizeof(mib) / sizeof(int)), ,
+   , NULL, 0) == 0 &&
+bufsize > 0) {
+
+  if ((processInfo.kp_proc.p_flag & P_TRANSLATED) == P_TRANSLATED) {
+const char *translated_debugserver =
+"/Library/Apple/usr/libexec/oah/debugserver";
+char fdstr[16];
+char pidstr[16];
+extern int communication_fd;
+
+if (communication_fd == -1) {
+  fprintf(stderr, "Trying to attach to a translated process with the "
+  "native debugserver, exiting...\n");
+  exit(1);
+}
+
+snprintf(fdstr, sizeof(fdstr), "--fd=%d", communication_fd);
+snprintf(pidstr, sizeof(pidstr), "--attach=%d", attach_pid);
+execl(translated_debugserver, "--native-regs", "--setsid", fdstr,
+  "--handoff-attach-from-native", pidstr, (char *)0);
+DNBLogThreadedIf(LOG_PROCESS, "Failed to launch debugserver for "
+ "translated process: ", errno, strerror(errno));
+__builtin_trap();
+  }
+}
+  }
+
   pid_t pid = INVALID_NUB_PROCESS;
   MachProcessSP processSP(new MachProcess);
   if (processSP.get()) {


Index: lldb/tools/debugserver/source/debugserver.cpp
===
--- lldb/tools/debugserver/source/debugserver.cpp
+++ lldb/tools/debugserver/source/debugserver.cpp
@@ -878,6 +878,8 @@
// -F localhost:1234 -- /bin/ls"
 {NULL, 0, NULL, 0}};
 
+int communication_fd = -1;
+
 // main
 int main(int argc, char *argv[]) {
   // If debugserver is launched with DYLD_INSERT_LIBRARIES, unset it so we
@@ -944,7 +946,6 @@
   int ch;
   int long_option_index = 0;
   int debug = 0;
-  int communication_fd = -1;
   std::string compile_options;
   std::string waitfor_pid_name; // Wait for a process that starts with this name
   std::string attach_pid_name;
Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -442,6 +442,39 @@
   if (err_str && err_len > 0)
 err_str[0] = '\0';
 
+  if (getenv("LLDB_DEBUGSERVER_PATH") == NULL) {
+int mib[] = {CTL_KERN, KERN_PROC, KERN_PROC_PID,
+ static_cast(attach_pid)};
+struct kinfo_proc processInfo;
+size_t bufsize = sizeof(processInfo);
+if (sysctl(mib, (unsigned)(sizeof(mib) / sizeof(int)), ,
+   , NULL, 0) == 0 &&
+bufsize > 0) {
+
+  if ((processInfo.kp_proc.p_flag & P_TRANSLATED) == P_TRANSLATED) {
+const char *translated_debugserver =
+"/Library/Apple/usr/libexec/oah/debugserver";
+char fdstr[16];
+char pidstr[16];
+extern int communication_fd;
+
+if (communication_fd == -1) {
+  fprintf(stderr, "Trying to attach to a translated process with the "
+  "native debugserver, exiting...\n");
+  exit(1);
+}
+
+snprintf(fdstr, sizeof(fdstr), "--fd=%d", communication_fd);
+snprintf(pidstr, sizeof(pidstr), "--attach=%d", attach_pid);
+execl(translated_debugserver, "--native-regs", "--setsid", fdstr,
+  "--handoff-attach-from-native", pidstr, (char *)0);
+DNBLogThreadedIf(LOG_PROCESS, "Failed to launch debugserver for "
+ "translated process: ", errno, strerror(errno));
+  

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

[Davide Italiano via Phabricator via lldb-commits]

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

why? Do you have a testcase?


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

https://reviews.llvm.org/D84272



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

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

yeah, I think it's reasonable.


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

https://reviews.llvm.org/D84263



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

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added a reviewer: jasonmolenda.

When we detect a process that the native debugserver cannot handle, handoff the 
connection fd to the translated debugserver.


https://reviews.llvm.org/D84263

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/debugserver.cpp


Index: lldb/tools/debugserver/source/debugserver.cpp
===
--- lldb/tools/debugserver/source/debugserver.cpp
+++ lldb/tools/debugserver/source/debugserver.cpp
@@ -878,6 +878,8 @@
// -F localhost:1234 -- /bin/ls"
 {NULL, 0, NULL, 0}};
 
+int communication_fd = -1;
+
 // main
 int main(int argc, char *argv[]) {
   // If debugserver is launched with DYLD_INSERT_LIBRARIES, unset it so we
@@ -944,7 +946,6 @@
   int ch;
   int long_option_index = 0;
   int debug = 0;
-  int communication_fd = -1;
   std::string compile_options;
   std::string waitfor_pid_name; // Wait for a process that starts with this 
name
   std::string attach_pid_name;
Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -442,6 +442,33 @@
   if (err_str && err_len > 0)
 err_str[0] = '\0';
 
+  if (getenv("LLDB_DEBUGSERVER_PATH") == NULL) {
+int mib[] = {CTL_KERN, KERN_PROC, KERN_PROC_PID,
+ static_cast(attach_pid)};
+struct kinfo_proc processInfo;
+size_t bufsize = sizeof(processInfo);
+if (sysctl(mib, (unsigned)(sizeof(mib) / sizeof(int)), ,
+   , NULL, 0) == 0 &&
+bufsize > 0) {
+
+  if ((processInfo.kp_proc.p_flag & P_TRANSLATED) == P_TRANSLATED) {
+const char *translated_debugserver =
+"/Library/Apple/usr/libexec/oah/debugserver";
+char fdstr[16];
+char pidstr[16];
+extern int communication_fd;
+
+snprintf(fdstr, sizeof(fdstr), "--fd=%d", communication_fd);
+snprintf(pidstr, sizeof(pidstr), "--attach=%d", attach_pid);
+execl(translated_debugserver, "--native-regs", "--setsid", fdstr,
+  "--handoff-attach-from-native", pidstr, (char *)0);
+DNBLogThreadedIf(LOG_PROCESS, "Failed to launch debugserver for "
+ "translated process: ", errno, strerror(errno));
+__builtin_trap();
+  }
+}
+  }
+
   pid_t pid = INVALID_NUB_PROCESS;
   MachProcessSP processSP(new MachProcess);
   if (processSP.get()) {


Index: lldb/tools/debugserver/source/debugserver.cpp
===
--- lldb/tools/debugserver/source/debugserver.cpp
+++ lldb/tools/debugserver/source/debugserver.cpp
@@ -878,6 +878,8 @@
// -F localhost:1234 -- /bin/ls"
 {NULL, 0, NULL, 0}};
 
+int communication_fd = -1;
+
 // main
 int main(int argc, char *argv[]) {
   // If debugserver is launched with DYLD_INSERT_LIBRARIES, unset it so we
@@ -944,7 +946,6 @@
   int ch;
   int long_option_index = 0;
   int debug = 0;
-  int communication_fd = -1;
   std::string compile_options;
   std::string waitfor_pid_name; // Wait for a process that starts with this name
   std::string attach_pid_name;
Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -442,6 +442,33 @@
   if (err_str && err_len > 0)
 err_str[0] = '\0';
 
+  if (getenv("LLDB_DEBUGSERVER_PATH") == NULL) {
+int mib[] = {CTL_KERN, KERN_PROC, KERN_PROC_PID,
+ static_cast(attach_pid)};
+struct kinfo_proc processInfo;
+size_t bufsize = sizeof(processInfo);
+if (sysctl(mib, (unsigned)(sizeof(mib) / sizeof(int)), ,
+   , NULL, 0) == 0 &&
+bufsize > 0) {
+
+  if ((processInfo.kp_proc.p_flag & P_TRANSLATED) == P_TRANSLATED) {
+const char *translated_debugserver =
+"/Library/Apple/usr/libexec/oah/debugserver";
+char fdstr[16];
+char pidstr[16];
+extern int communication_fd;
+
+snprintf(fdstr, sizeof(fdstr), "--fd=%d", communication_fd);
+snprintf(pidstr, sizeof(pidstr), "--attach=%d", attach_pid);
+execl(translated_debugserver, "--native-regs", "--setsid", fdstr,
+  "--handoff-attach-from-native", pidstr, (char *)0);
+DNBLogThreadedIf(LOG_PROCESS, "Failed to launch debugserver for "
+ "translated process: ", errno, strerror(errno));
+__builtin_trap();
+  }
+}
+  }
+
   pid_t pid = INVALID_NUB_PROCESS;
   MachProcessSP processSP(new MachProcess);
   if (processSP.get()) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D83796: [ObjC] Wrap namespace-global structure in an anonymous namespace to avoid ODR violations

[Davide Italiano via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3f2d880a9329: [ObjC] Wrap namespace-global structs in an 
anonymous namespace to avoid ODR… (authored by davide).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83796

Files:
  lldb/source/Plugins/Language/ObjC/NSArray.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp

Index: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
+++ lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
@@ -278,64 +278,67 @@
 }
   
 namespace Foundation1428 {
-  struct DataDescriptor_32 {
-uint32_t _used : 26;
-uint32_t _kvo : 1;
-uint32_t _size;
-uint32_t _buffer;
-uint64_t GetSize() { return _size; }
-  };
-  
-  struct DataDescriptor_64 {
-uint64_t _used : 58;
-uint32_t _kvo : 1;
-uint64_t _size;
-uint64_t _buffer;
-uint64_t GetSize() { return _size; }
-  };
-  
-  
-  
+  namespace {
+struct DataDescriptor_32 {
+  uint32_t _used : 26;
+  uint32_t _kvo : 1;
+  uint32_t _size;
+  uint32_t _buffer;
+  uint64_t GetSize() { return _size; }
+};
+
+struct DataDescriptor_64 {
+  uint64_t _used : 58;
+  uint32_t _kvo : 1;
+  uint64_t _size;
+  uint64_t _buffer;
+  uint64_t GetSize() { return _size; }
+};
+  }
+
   using NSDictionaryMSyntheticFrontEnd =
 GenericNSDictionaryMSyntheticFrontEnd;
 }
   
 namespace Foundation1437 {
-  static const uint64_t NSDictionaryCapacities[] = {
-  0, 3, 7, 13, 23, 41, 71, 127, 191, 251, 383, 631, 1087, 1723,
-  2803, 4523, 7351, 11959, 19447, 31231, 50683, 81919, 132607,
-  214519, 346607, 561109, 907759, 1468927, 2376191, 3845119,
-  6221311, 10066421, 16287743, 26354171, 42641881, 68996069,
-  111638519, 180634607, 292272623, 472907251
-  };
-  
-  static const size_t NSDictionaryNumSizeBuckets = sizeof(NSDictionaryCapacities) / sizeof(uint64_t);
-  
-  struct DataDescriptor_32 {
-uint32_t _buffer;
-uint32_t _muts;
-uint32_t _used : 25;
-uint32_t _kvo : 1;
-uint32_t _szidx : 6;
+  namespace {
+static const uint64_t NSDictionaryCapacities[] = {
+0, 3, 7, 13, 23, 41, 71, 127, 191, 251, 383, 631, 1087, 1723,
+2803, 4523, 7351, 11959, 19447, 31231, 50683, 81919, 132607,
+214519, 346607, 561109, 907759, 1468927, 2376191, 3845119,
+6221311, 10066421, 16287743, 26354171, 42641881, 68996069,
+111638519, 180634607, 292272623, 472907251
+};
+
+static const size_t NSDictionaryNumSizeBuckets =
+sizeof(NSDictionaryCapacities) / sizeof(uint64_t);
+
+struct DataDescriptor_32 {
+  uint32_t _buffer;
+  uint32_t _muts;
+  uint32_t _used : 25;
+  uint32_t _kvo : 1;
+  uint32_t _szidx : 6;
 
-uint64_t GetSize() {
-  return (_szidx) >= NSDictionaryNumSizeBuckets ?
-  0 : NSDictionaryCapacities[_szidx];
-}
-  };
-  
-  struct DataDescriptor_64 {
-uint64_t _buffer;
-uint32_t _muts;
-uint32_t _used : 25;
-uint32_t _kvo : 1;
-uint32_t _szidx : 6;
+  uint64_t GetSize() {
+return (_szidx) >= NSDictionaryNumSizeBuckets ?
+0 : NSDictionaryCapacities[_szidx];
+  }
+};
+
+struct DataDescriptor_64 {
+  uint64_t _buffer;
+  uint32_t _muts;
+  uint32_t _used : 25;
+  uint32_t _kvo : 1;
+  uint32_t _szidx : 6;
 
-uint64_t GetSize() {
-  return (_szidx) >= NSDictionaryNumSizeBuckets ?
-  0 : NSDictionaryCapacities[_szidx];
-}
-  };
+  uint64_t GetSize() {
+return (_szidx) >= NSDictionaryNumSizeBuckets ?
+0 : NSDictionaryCapacities[_szidx];
+  }
+};
+  }
   
   using NSDictionaryMSyntheticFrontEnd =
 GenericNSDictionaryMSyntheticFrontEnd;
Index: lldb/source/Plugins/Language/ObjC/NSArray.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSArray.cpp
+++ lldb/source/Plugins/Language/ObjC/NSArray.cpp
@@ -98,42 +98,46 @@
 };
   
 namespace Foundation1010 {
-  struct DataDescriptor_32 {
-uint32_t _used;
-uint32_t _offset;
-uint32_t _size : 28;
-uint64_t _priv1 : 4;
-uint32_t _priv2;
-uint32_t _data;
-  };
-  
-  struct DataDescriptor_64 {
-uint64_t _used;
-uint64_t _offset;
-uint64_t _size : 60;
-uint64_t _priv1 : 4;
-uint32_t _priv2;
-uint64_t _data;
-  };
+  namespace {
+struct DataDescriptor_32 {
+  uint32_t _used;
+  uint32_t _offset;
+  uint32_t _size : 28;
+  uint64_t _priv1 : 4;
+  uint32_t _priv2;
+  uint32_t _data;
+};
+
+struct DataDescriptor_64 {
+  uint64_t _used;
+  uint64_t _offset;
+  uint64_t _size : 60;
+  uint64_t _priv1 : 4;
+  uint32_t 

[Lldb-commits] [PATCH] D83796: [ObjC] Wrap namespace-global structure in an anonymous namespace to avoid ODR violations

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added a reviewer: teemperor.

rdar://problem/65537147


https://reviews.llvm.org/D83796

Files:
  lldb/source/Plugins/Language/ObjC/NSArray.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp

Index: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
+++ lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
@@ -278,64 +278,67 @@
 }
   
 namespace Foundation1428 {
-  struct DataDescriptor_32 {
-uint32_t _used : 26;
-uint32_t _kvo : 1;
-uint32_t _size;
-uint32_t _buffer;
-uint64_t GetSize() { return _size; }
-  };
-  
-  struct DataDescriptor_64 {
-uint64_t _used : 58;
-uint32_t _kvo : 1;
-uint64_t _size;
-uint64_t _buffer;
-uint64_t GetSize() { return _size; }
-  };
-  
-  
-  
+  namespace {
+struct DataDescriptor_32 {
+  uint32_t _used : 26;
+  uint32_t _kvo : 1;
+  uint32_t _size;
+  uint32_t _buffer;
+  uint64_t GetSize() { return _size; }
+};
+
+struct DataDescriptor_64 {
+  uint64_t _used : 58;
+  uint32_t _kvo : 1;
+  uint64_t _size;
+  uint64_t _buffer;
+  uint64_t GetSize() { return _size; }
+};
+  }
+
   using NSDictionaryMSyntheticFrontEnd =
 GenericNSDictionaryMSyntheticFrontEnd;
 }
   
 namespace Foundation1437 {
-  static const uint64_t NSDictionaryCapacities[] = {
-  0, 3, 7, 13, 23, 41, 71, 127, 191, 251, 383, 631, 1087, 1723,
-  2803, 4523, 7351, 11959, 19447, 31231, 50683, 81919, 132607,
-  214519, 346607, 561109, 907759, 1468927, 2376191, 3845119,
-  6221311, 10066421, 16287743, 26354171, 42641881, 68996069,
-  111638519, 180634607, 292272623, 472907251
-  };
-  
-  static const size_t NSDictionaryNumSizeBuckets = sizeof(NSDictionaryCapacities) / sizeof(uint64_t);
-  
-  struct DataDescriptor_32 {
-uint32_t _buffer;
-uint32_t _muts;
-uint32_t _used : 25;
-uint32_t _kvo : 1;
-uint32_t _szidx : 6;
+  namespace {
+static const uint64_t NSDictionaryCapacities[] = {
+0, 3, 7, 13, 23, 41, 71, 127, 191, 251, 383, 631, 1087, 1723,
+2803, 4523, 7351, 11959, 19447, 31231, 50683, 81919, 132607,
+214519, 346607, 561109, 907759, 1468927, 2376191, 3845119,
+6221311, 10066421, 16287743, 26354171, 42641881, 68996069,
+111638519, 180634607, 292272623, 472907251
+};
+
+static const size_t NSDictionaryNumSizeBuckets =
+sizeof(NSDictionaryCapacities) / sizeof(uint64_t);
+
+struct DataDescriptor_32 {
+  uint32_t _buffer;
+  uint32_t _muts;
+  uint32_t _used : 25;
+  uint32_t _kvo : 1;
+  uint32_t _szidx : 6;
 
-uint64_t GetSize() {
-  return (_szidx) >= NSDictionaryNumSizeBuckets ?
-  0 : NSDictionaryCapacities[_szidx];
-}
-  };
-  
-  struct DataDescriptor_64 {
-uint64_t _buffer;
-uint32_t _muts;
-uint32_t _used : 25;
-uint32_t _kvo : 1;
-uint32_t _szidx : 6;
+  uint64_t GetSize() {
+return (_szidx) >= NSDictionaryNumSizeBuckets ?
+0 : NSDictionaryCapacities[_szidx];
+  }
+};
+
+struct DataDescriptor_64 {
+  uint64_t _buffer;
+  uint32_t _muts;
+  uint32_t _used : 25;
+  uint32_t _kvo : 1;
+  uint32_t _szidx : 6;
 
-uint64_t GetSize() {
-  return (_szidx) >= NSDictionaryNumSizeBuckets ?
-  0 : NSDictionaryCapacities[_szidx];
-}
-  };
+  uint64_t GetSize() {
+return (_szidx) >= NSDictionaryNumSizeBuckets ?
+0 : NSDictionaryCapacities[_szidx];
+  }
+};
+  }
   
   using NSDictionaryMSyntheticFrontEnd =
 GenericNSDictionaryMSyntheticFrontEnd;
Index: lldb/source/Plugins/Language/ObjC/NSArray.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSArray.cpp
+++ lldb/source/Plugins/Language/ObjC/NSArray.cpp
@@ -98,42 +98,46 @@
 };
   
 namespace Foundation1010 {
-  struct DataDescriptor_32 {
-uint32_t _used;
-uint32_t _offset;
-uint32_t _size : 28;
-uint64_t _priv1 : 4;
-uint32_t _priv2;
-uint32_t _data;
-  };
-  
-  struct DataDescriptor_64 {
-uint64_t _used;
-uint64_t _offset;
-uint64_t _size : 60;
-uint64_t _priv1 : 4;
-uint32_t _priv2;
-uint64_t _data;
-  };
+  namespace {
+struct DataDescriptor_32 {
+  uint32_t _used;
+  uint32_t _offset;
+  uint32_t _size : 28;
+  uint64_t _priv1 : 4;
+  uint32_t _priv2;
+  uint32_t _data;
+};
+
+struct DataDescriptor_64 {
+  uint64_t _used;
+  uint64_t _offset;
+  uint64_t _size : 60;
+  uint64_t _priv1 : 4;
+  uint32_t _priv2;
+  uint64_t _data;
+};
+  }
   
   using NSArrayMSyntheticFrontEnd =
   GenericNSArrayMSyntheticFrontEnd;
 }
   
 namespace Foundation1428 {
-  struct DataDescriptor_32 {
-uint32_t _used;
-uint32_t _offset;
-uint32_t 

[Lldb-commits] [PATCH] D83600: Add a decorator to skip tests when running under Rosetta

[Davide Italiano via Phabricator via lldb-commits]

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

Good, one minor suggestion.




Comment at: lldb/packages/Python/lldbsuite/test/decorators.py:560
+return False
+return platform.uname()[5] == "arm" and self.getArchitecture() == 
"x86_64"
+return skipTestIfFn(is_running_rosetta, bugnumber)(func)

There might be a more direct way of checking this, maybe `machine()`. We used 
`platform().uname()` for internal limitations & machine not always returning 
the right string, but it should be fixed on Beta 1 (and beyond) for sure


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

https://reviews.llvm.org/D83600



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

[Lldb-commits] [PATCH] D83582: Fix nesting of #ifdef

[Davide Italiano via Phabricator via lldb-commits]

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

LGTM


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

https://reviews.llvm.org/D83582



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

[Lldb-commits] [PATCH] D83306: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true)

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

This broke macOS:

http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22425/

I reverted it in:

  commit 27d52cd86a2cf82214b71519dffd450c54cf87ae (HEAD -> master, 
origin/master, origin/HEAD)
  Author: Davide Italiano 
  Date:   Wed Jul 8 13:00:29 2020 -0700
  Revert "[lldb/API] Overwrite variables with 
SBLaunchInfo::SetEnvironment(append=true)"
  This reverts commit 695b33a56919af8873eecb47cb83fa17a271e99f beacuse
  it broke the macOS bot.

Let me know if you need any help/info to reproduce [just shoot me an e-mail], 
but ToT + `ninja check-lldb` on a recent'ish 10.15 should do it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83306



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

[Lldb-commits] [PATCH] D82804: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed.

[Davide Italiano via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
davide marked an inline comment as done.
Closed by commit rG5832473dcf4e: Do not set LLDB_DEBUGSERVER_PATH if 
--out-of-tree-debugserver is passed. (authored by davide).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82804

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -363,7 +363,7 @@
 args.executable)
 sys.exit(-1)
 
-if args.server:
+if args.server and not args.out_of_tree_debugserver:
 os.environ['LLDB_DEBUGSERVER_PATH'] = args.server
 
 if args.excluded:


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -363,7 +363,7 @@
 args.executable)
 sys.exit(-1)
 
-if args.server:
+if args.server and not args.out_of_tree_debugserver:
 os.environ['LLDB_DEBUGSERVER_PATH'] = args.server
 
 if args.excluded:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82804: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed.

[Davide Italiano via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
davide marked an inline comment as done.
Closed by commit rG5832473dcf4e: Do not set LLDB_DEBUGSERVER_PATH if 
--out-of-tree-debugserver is passed. (authored by davide).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82804

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -363,7 +363,7 @@
 args.executable)
 sys.exit(-1)
 
-if args.server:
+if args.server and not args.out_of_tree_debugserver:
 os.environ['LLDB_DEBUGSERVER_PATH'] = args.server
 
 if args.excluded:


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -363,7 +363,7 @@
 args.executable)
 sys.exit(-1)
 
-if args.server:
+if args.server and not args.out_of_tree_debugserver:
 os.environ['LLDB_DEBUGSERVER_PATH'] = args.server
 
 if args.excluded:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In D83327#2136842 , @jingham wrote:

> In D83327#2136814 , @davide wrote:
>
> > Aside from cosmetics, I'm not entirely sure this is the correct fix. Why 
> > are we calling this code _at all_ if the type is incomplete?
>
>
> Doing so allows one to write a synthetic child provider that provides the 
> fields for an incomplete type.  This is useful if you don't have debug info 
> for a given type but know its layouts by some other means.


Interesting, thanks. Do we have an example of when this triggers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83327



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

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Aside from cosmetics, I'm not entirely sure this is the correct fix. Why are we 
calling this code _at all_ if the type is incomplete?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83327



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

[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

[Davide Italiano via Phabricator via lldb-commits]

davide abandoned this revision.
davide added a comment.

Cleaning up my queue.


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

https://reviews.llvm.org/D43048



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

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

[Davide Italiano via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3424
+if (Host::IsProcessTranslated(process_info)) {
+  FileSpec 
rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
+  debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);

labath wrote:
> I don't think this is a particularly good abstraction, as the debugserver 
> path is still left here, and the path is definitely os- and arch-specific. It 
> would be better if the API returned the path to the debugserver-to-be-used 
> instead.
> 
> Bonus points if you can also hide the `DEBUGSERVER_BASENAME` logic from 
> GDBRemoteCommunication.cpp into the same API.
hmm, I wonder if this code should live in `GDBRemoteCommunication.cpp`


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

https://reviews.llvm.org/D82813



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

[Lldb-commits] [PATCH] D82616: Improve the detection of iOS/tvOS/watchOS simulator binaries in debugserver and lldb

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

Thanks.


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

https://reviews.llvm.org/D82616



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

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

[Davide Italiano via Phabricator via lldb-commits]

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



Comment at: lldb/include/lldb/Host/Host.h:231
 
+  /// Check whether a process is translated (Rosetta).
+  /// \arg process_info The info structure for the process queried.

aprantl wrote:
> Is this supposed to be a generic function call that, e.g, should fire for 
> debugging a process in QEMU under Linux, or is it meant to specifically check 
> for Rosetta on macOS? The comment makes it sound like it's the latter and the 
> API name hints at the former. It would be nice to clarify this in the comment.
It is meant specifically for Rosetta, but this leaves in `Host`, so I decided 
for a generic name.
Do you have a better suggestion for the name or the comment ? Happy to go with 
it 


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

https://reviews.llvm.org/D82813



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

[Lldb-commits] [PATCH] D82804: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed.

[Davide Italiano via Phabricator via lldb-commits]

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



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:366
 
-if args.server:
+if args.server and not args.out_of_tree_debugserver:
 os.environ['LLDB_DEBUGSERVER_PATH'] = args.server

aprantl wrote:
> Does this have any surprising interplay with the existing 
> LLDB_USE_SYSTEM_DEBUGSERVER cmake flag?
I don't think so?


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

https://reviews.llvm.org/D82804



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

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

[Davide Italiano via Phabricator via lldb-commits]

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



Comment at: lldb/include/lldb/Host/Host.h:231
 
+  /// Run a shell command.
+  /// \arg process_info The info structure for the process queried.

Stale comment, I'll update


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

https://reviews.llvm.org/D82813



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

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added reviewers: aprantl, jasonmolenda.

Nothing crazy here, just an organizational cleanup.


https://reviews.llvm.org/D82813

Files:
  lldb/include/lldb/Host/Host.h
  lldb/source/Host/common/Host.cpp
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp


Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -17,9 +17,6 @@
 #include 
 #endif
 #include 
-#if defined(__APPLE__)
-#include 
-#endif
 #include 
 #include 
 
@@ -3420,22 +3417,13 @@
 std::bind(MonitorDebugserverProcess, this_wp, _1, _2, _3, _4), false);
 debugserver_launch_info.SetUserID(process_info.GetUserID());
 
-#if defined(__APPLE__)
 // On macOS 11, we need to support x86_64 applications translated to
 // arm64. We check whether a binary is translated and spawn the correct
 // debugserver accordingly.
-int mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_PID,
-  static_cast(process_info.GetProcessID()) };
-struct kinfo_proc processInfo;
-size_t bufsize = sizeof(processInfo);
-if (sysctl(mib, (unsigned)(sizeof(mib)/sizeof(int)), ,
-   , NULL, 0) == 0 && bufsize > 0) {
-  if (processInfo.kp_proc.p_flag & P_TRANSLATED) {
-FileSpec 
rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
-debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
-  }
+if (Host::IsProcessTranslated(process_info)) {
+  FileSpec 
rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
+  debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
 }
-#endif
 
 int communication_fd = -1;
 #ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION
Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -1462,3 +1462,14 @@
 ::asl_vlog(NULL, g_aslmsg, asl_level, format, args);
   }
 }
+
+bool Host::IsProcessTranslated(const ProcessInfo _info) {
+  int mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_PID,
+static_cast(process_info.GetProcessID()) };
+  struct kinfo_proc processInfo;
+  size_t bufsize = sizeof(processInfo);
+  if (sysctl(mib, (unsigned)(sizeof(mib)/sizeof(int)), ,
+ , NULL, 0) == 0 && bufsize <= 0)
+return false;
+  return processInfo.kp_proc.p_flag & P_TRANSLATED;
+}
Index: lldb/source/Host/common/Host.cpp
===
--- lldb/source/Host/common/Host.cpp
+++ lldb/source/Host/common/Host.cpp
@@ -414,6 +414,10 @@
 }
 
 bool Host::ResolveExecutableInBundle(FileSpec ) { return false; }
+
+bool Host::IsProcessTranslated(const ProcessInfo _info) {
+  return false;
+}
 #endif
 
 #ifndef _WIN32
Index: lldb/include/lldb/Host/Host.h
===
--- lldb/include/lldb/Host/Host.h
+++ lldb/include/lldb/Host/Host.h
@@ -228,6 +228,10 @@
   static bool OpenFileInExternalEditor(const FileSpec _spec,
uint32_t line_no);
 
+  /// Run a shell command.
+  /// \arg process_info The info structure for the process queried.
+  static bool IsProcessTranslated(const ProcessInfo _info);
+
   static Environment GetEnvironment();
 
   static std::unique_ptr


Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -17,9 +17,6 @@
 #include 
 #endif
 #include 
-#if defined(__APPLE__)
-#include 
-#endif
 #include 
 #include 
 
@@ -3420,22 +3417,13 @@
 std::bind(MonitorDebugserverProcess, this_wp, _1, _2, _3, _4), false);
 debugserver_launch_info.SetUserID(process_info.GetUserID());
 
-#if defined(__APPLE__)
 // On macOS 11, we need to support x86_64 applications translated to
 // arm64. We check whether a binary is translated and spawn the correct
 // debugserver accordingly.
-int mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_PID,
-  static_cast(process_info.GetProcessID()) };
-struct kinfo_proc processInfo;
-size_t bufsize = sizeof(processInfo);
-if (sysctl(mib, (unsigned)(sizeof(mib)/sizeof(int)), ,
-   , NULL, 0) == 0 && bufsize > 0) {
-  if (processInfo.kp_proc.p_flag & P_TRANSLATED) {
-FileSpec rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
-debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
-  }
+if (Host::IsProcessTranslated(process_info)) {
+  FileSpec 

[Lldb-commits] [PATCH] D82491: [Apple Silicon] Initial support for Rosetta

[Davide Italiano via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3444
+size_t bufsize = sizeof(processInfo);
+if (sysctl(mib, (unsigned)(sizeof(mib)/sizeof(int)), ,
+   , NULL, 0) == 0 && bufsize > 0) {

echristo wrote:
> JDevlieghere wrote:
> > The `sysctl` call seems like something that would fit into host, we already 
> > have a bunch of those in `Host.mm`. Should we create a function there that 
> > returns whether a given pid runs under Rosetta?
> Agreed. That seems much cleaner.
https://reviews.llvm.org/D82813


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

https://reviews.llvm.org/D82491



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

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

[Davide Italiano via Phabricator via lldb-commits]

davide updated this revision to Diff 274253.

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

https://reviews.llvm.org/D82813

Files:
  lldb/include/lldb/Host/Host.h
  lldb/source/Host/common/Host.cpp
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp


Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -17,9 +17,6 @@
 #include 
 #endif
 #include 
-#if defined(__APPLE__)
-#include 
-#endif
 #include 
 #include 
 
@@ -3420,22 +3417,13 @@
 std::bind(MonitorDebugserverProcess, this_wp, _1, _2, _3, _4), false);
 debugserver_launch_info.SetUserID(process_info.GetUserID());
 
-#if defined(__APPLE__)
 // On macOS 11, we need to support x86_64 applications translated to
 // arm64. We check whether a binary is translated and spawn the correct
 // debugserver accordingly.
-int mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_PID,
-  static_cast(process_info.GetProcessID()) };
-struct kinfo_proc processInfo;
-size_t bufsize = sizeof(processInfo);
-if (sysctl(mib, (unsigned)(sizeof(mib)/sizeof(int)), ,
-   , NULL, 0) == 0 && bufsize > 0) {
-  if (processInfo.kp_proc.p_flag & P_TRANSLATED) {
-FileSpec 
rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
-debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
-  }
+if (Host::IsProcessTranslated(process_info)) {
+  FileSpec 
rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
+  debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
 }
-#endif
 
 int communication_fd = -1;
 #ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION
Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -1462,3 +1462,14 @@
 ::asl_vlog(NULL, g_aslmsg, asl_level, format, args);
   }
 }
+
+bool Host::IsProcessTranslated(const ProcessInfo _info) {
+  int mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_PID,
+static_cast(process_info.GetProcessID()) };
+  struct kinfo_proc processInfo;
+  size_t bufsize = sizeof(processInfo);
+  if (sysctl(mib, (unsigned)(sizeof(mib)/sizeof(int)), ,
+ , NULL, 0) == 0 && bufsize <= 0)
+return false;
+  return processInfo.kp_proc.p_flag & P_TRANSLATED;
+}
Index: lldb/source/Host/common/Host.cpp
===
--- lldb/source/Host/common/Host.cpp
+++ lldb/source/Host/common/Host.cpp
@@ -414,6 +414,10 @@
 }
 
 bool Host::ResolveExecutableInBundle(FileSpec ) { return false; }
+
+bool Host::IsProcessTranslated(const ProcessInfo _info) {
+  return false;
+}
 #endif
 
 #ifndef _WIN32
Index: lldb/include/lldb/Host/Host.h
===
--- lldb/include/lldb/Host/Host.h
+++ lldb/include/lldb/Host/Host.h
@@ -228,6 +228,10 @@
   static bool OpenFileInExternalEditor(const FileSpec _spec,
uint32_t line_no);
 
+  /// Check whether a process is translated (Rosetta).
+  /// \arg process_info The info structure for the process queried.
+  static bool IsProcessTranslated(const ProcessInfo _info);
+
   static Environment GetEnvironment();
 
   static std::unique_ptr


Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -17,9 +17,6 @@
 #include 
 #endif
 #include 
-#if defined(__APPLE__)
-#include 
-#endif
 #include 
 #include 
 
@@ -3420,22 +3417,13 @@
 std::bind(MonitorDebugserverProcess, this_wp, _1, _2, _3, _4), false);
 debugserver_launch_info.SetUserID(process_info.GetUserID());
 
-#if defined(__APPLE__)
 // On macOS 11, we need to support x86_64 applications translated to
 // arm64. We check whether a binary is translated and spawn the correct
 // debugserver accordingly.
-int mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_PID,
-  static_cast(process_info.GetProcessID()) };
-struct kinfo_proc processInfo;
-size_t bufsize = sizeof(processInfo);
-if (sysctl(mib, (unsigned)(sizeof(mib)/sizeof(int)), ,
-   , NULL, 0) == 0 && bufsize > 0) {
-  if (processInfo.kp_proc.p_flag & P_TRANSLATED) {
-FileSpec rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
-debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
-  }
+if (Host::IsProcessTranslated(process_info)) {
+  FileSpec 

[Lldb-commits] [PATCH] D82804: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed.

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added reviewers: vsk, aprantl, jasonmolenda.

Now that there are two implementations of debugserver, one for native and the 
other for Rosetta [on Apple Silicon], this is needed.


https://reviews.llvm.org/D82804

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -363,7 +363,7 @@
 args.executable)
 sys.exit(-1)
 
-if args.server:
+if args.server and not args.out_of_tree_debugserver:
 os.environ['LLDB_DEBUGSERVER_PATH'] = args.server
 
 if args.excluded:


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -363,7 +363,7 @@
 args.executable)
 sys.exit(-1)
 
-if args.server:
+if args.server and not args.out_of_tree_debugserver:
 os.environ['LLDB_DEBUGSERVER_PATH'] = args.server
 
 if args.excluded:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82493: [Apple Silicon] Debugging of process under Rosetta is supported

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

  commit b4fdddf971b191aa9a6643ab637b87bc1d686254 (HEAD -> master, 
origin/master, origin/HEAD)
  Author: Davide Italiano 
  Date:   Wed Jun 24 12:25:01 2020 -0700
  
  [Apple Silicon] Debugging of process under Rosetta is supported.
  
  Remove this early exit. It's vestigial from the ppc -> Intel transition,
  but it doesn't apply anymore.


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

https://reviews.llvm.org/D82493



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

[Lldb-commits] [PATCH] D82491: [Apple Silicon] Initial support for Rosetta

[Davide Italiano via Phabricator via lldb-commits]

davide closed this revision.
davide added a comment.

  commit fd19ddb8f2a2b082f492fc59f7f360adf3495701 (HEAD -> master, 
origin/master, origin/HEAD)
  Author: Davide Italiano 
  Date:   Wed Jun 24 12:18:29 2020 -0700
  
  [Apple Silicon] Initial support for Rosetta
  
  Translated processes talk with a different debugserver, shipped with
  macOS 11. This patch detects whether a process is translated and
  attaches to the correct debugserver implementation.
  It's the first patch of a series. Tested on the lldb test suite.
  
  Differential Revision:  https://reviews.llvm.org/D82491


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

https://reviews.llvm.org/D82491



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

[Lldb-commits] [PATCH] D82491: [Apple Silicon] Initial support for Rosetta

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In D82491#2112269 , @jasonmolenda 
wrote:

> LGTM.  With p_flag, we only need to evaluate (processInfo.kp_proc.p_flag & 
> P_TRANSLATED) as a boolean, but that's a style pref as much as anything.


Fair, let me change that.


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

https://reviews.llvm.org/D82491



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

[Lldb-commits] [PATCH] D82493: [Apple Silicon] Debugging of process under Rosetta is supported

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added reviewers: jasonmolenda, friss, aprantl.

Remove this early exit. It's vestigial from the ppc -> Intel transition, but it 
doesn't apply anymore:


https://reviews.llvm.org/D82493

Files:
  lldb/source/Host/macosx/objcxx/Host.mm


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -631,8 +631,7 @@
 kinfo.kp_proc.p_pid == 0 ||  // Skip kernel (kernel pid is zero)
 kinfo.kp_proc.p_stat == SZOMB || // Zombies are bad, they like 
brains...
 kinfo.kp_proc.p_flag & P_TRACED ||   // Being debugged?
-kinfo.kp_proc.p_flag & P_WEXIT ||// Working on exiting?
-kinfo.kp_proc.p_flag & P_TRANSLATED) // Skip translated ppc (Rosetta)
+kinfo.kp_proc.p_flag & P_WEXIT)
   continue;
 
 ProcessInstanceInfo process_info;


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -631,8 +631,7 @@
 kinfo.kp_proc.p_pid == 0 ||  // Skip kernel (kernel pid is zero)
 kinfo.kp_proc.p_stat == SZOMB || // Zombies are bad, they like brains...
 kinfo.kp_proc.p_flag & P_TRACED ||   // Being debugged?
-kinfo.kp_proc.p_flag & P_WEXIT ||// Working on exiting?
-kinfo.kp_proc.p_flag & P_TRANSLATED) // Skip translated ppc (Rosetta)
+kinfo.kp_proc.p_flag & P_WEXIT)
   continue;
 
 ProcessInstanceInfo process_info;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82491: [Apple Silicon] Initial support for Rosetta

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added reviewers: jasonmolenda, aprantl, friss.
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.
davide closed this revision.

LGTM.  With p_flag, we only need to evaluate (processInfo.kp_proc.p_flag & 
P_TRANSLATED) as a boolean, but that's a style pref as much as anything.


davide added a comment.

In D82491#2112269 , @jasonmolenda 
wrote:

> LGTM.  With p_flag, we only need to evaluate (processInfo.kp_proc.p_flag & 
> P_TRANSLATED) as a boolean, but that's a style pref as much as anything.


Fair, let me change that.


davide added a comment.

  commit fd19ddb8f2a2b082f492fc59f7f360adf3495701 (HEAD -> master, 
origin/master, origin/HEAD)
  Author: Davide Italiano 
  Date:   Wed Jun 24 12:18:29 2020 -0700
  
  [Apple Silicon] Initial support for Rosetta
  
  Translated processes talk with a different debugserver, shipped with
  macOS 11. This patch detects whether a process is translated and
  attaches to the correct debugserver implementation.
  It's the first patch of a series. Tested on the lldb test suite.
  
  Differential Revision:  https://reviews.llvm.org/D82491


Translated processes talk with a different debugserver, shipped with macOS 11. 
This patch detects whether a process is translated and attaches to the correct 
debugserver implementation. It's the first patch of a series. Tested on the 
lldb test suite.


https://reviews.llvm.org/D82491

Files:
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp


Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -17,6 +17,7 @@
 #include 
 #endif
 #include 
+#include 
 #include 
 #include 
 
@@ -3432,6 +3433,23 @@
 std::bind(MonitorDebugserverProcess, this_wp, _1, _2, _3, _4), false);
 debugserver_launch_info.SetUserID(process_info.GetUserID());
 
+#if defined(__APPLE__)
+// On macOS 11, we need to support x86_64 applications translated to
+// arm64. We check whether a binary is translated and spawn the correct
+// debugserver accordingly.
+int mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_PID,
+  static_cast(process_info.GetProcessID()) };
+struct kinfo_proc processInfo;
+size_t bufsize = sizeof(processInfo);
+if (sysctl(mib, (unsigned)(sizeof(mib)/sizeof(int)), ,
+   , NULL, 0) == 0 && bufsize > 0) {
+  if ((processInfo.kp_proc.p_flag & P_TRANSLATED) == P_TRANSLATED) {
+FileSpec 
rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
+debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
+  }
+}
+#endif
+
 int communication_fd = -1;
 #ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION
 // Use a socketpair on non-Windows systems for security and performance


Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -17,6 +17,7 @@
 #include 
 #endif
 #include 
+#include 
 #include 
 #include 
 
@@ -3432,6 +3433,23 @@
 std::bind(MonitorDebugserverProcess, this_wp, _1, _2, _3, _4), false);
 debugserver_launch_info.SetUserID(process_info.GetUserID());
 
+#if defined(__APPLE__)
+// On macOS 11, we need to support x86_64 applications translated to
+// arm64. We check whether a binary is translated and spawn the correct
+// debugserver accordingly.
+int mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_PID,
+  static_cast(process_info.GetProcessID()) };
+struct kinfo_proc processInfo;
+size_t bufsize = sizeof(processInfo);
+if (sysctl(mib, (unsigned)(sizeof(mib)/sizeof(int)), ,
+   , NULL, 0) == 0 && bufsize > 0) {
+  if ((processInfo.kp_proc.p_flag & P_TRANSLATED) == P_TRANSLATED) {
+FileSpec rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
+debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
+  }
+}
+#endif
+
 int communication_fd = -1;
 #ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION
 // Use a socketpair on non-Windows systems for security and performance
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82394: [debugserver] Initial support for Apple Silicon.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

  commit 2276bb48be0175f96b1494ca67b7921f0d4e87d8 (HEAD -> master, 
origin/master, origin/HEAD)
  Author: Davide Italiano 
  Date:   Tue Jun 23 10:18:54 2020 -0700
  
  [debugserver] Initial support for Apple Silicon.
  
  Set the correct os type in the arch triple when running macOS.
  Debugserver currently always assumes macOS == x86_64. This patch
  generalizes the support to make sure it works on a different
  architecture.
  
  Differential Revision:  https://reviews.llvm.org/D82394


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

https://reviews.llvm.org/D82394



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

[Lldb-commits] [PATCH] D82394: [debugserver] Initial support for Apple Silicon.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

  


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

https://reviews.llvm.org/D82394



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

[Lldb-commits] [PATCH] D82394: [debugserver] Initial support for Apple Silicon.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Danke shoen


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

https://reviews.llvm.org/D82394



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

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Greg & Pavel might have opinions on this patch. I'm not qualified to review it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81119



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

[Lldb-commits] [PATCH] D80955: Fix UB in EmulateInstructionARM64.cpp

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

This is great Adrian. I feel like there are several other instances of 
additions that should use `llvm::checkAdd` here instead of `+`. Is this the 
only UB triggered in the testsuite?


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

https://reviews.llvm.org/D80955



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

[Lldb-commits] [PATCH] D80955: Fix UB in EmulateInstructionARM64.cpp

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp:25
 
+#include 
+

shafik wrote:
> Why? also should be `cstdlib` in C++.
It's just something shuffled around, probably a side effect of clang-format.


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

https://reviews.llvm.org/D80955



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

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

[Davide Italiano via Phabricator via lldb-commits]

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



Comment at: lldb/include/lldb/DataFormatters/Mock.h:1-8
+//===-- Mock.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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//

Neat way to test.



Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:801-804
+  // this snippet of code assumes that time_t == seconds since Jan-1-1970 this
+  // is generally true and POSIXly happy, but might break if a library vendor
+  // decides to get creative
+  time_t epoch = GetOSXEpoch();

I think you can drop this comment [you just moved it, but it feels irrelevant 
at this point because there's only one vendor].


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80150



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

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Looking, as I touched `NSDate` last year.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80150



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

[Lldb-commits] [PATCH] D79823: [lldb][Core] Remove dead codepath in Mangled

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

I'm curious what happens if you end up here with something like `_T`. Did you 
check?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79823



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

[Lldb-commits] [PATCH] D79823: [lldb][Core] Remove dead codepath in Mangled

[Davide Italiano via Phabricator via lldb-commits]

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

Presumably.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79823



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

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Saleem is the right person to review this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78972



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

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

I'm really worried about this change -- and we should think about it very 
carefully because it might cause the LLVM verifier to freak out -- or even 
worse, causing random crashes when executing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78972



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

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Weak functions can actually be overridden -- I'm really wondering whether this 
causes issues here. Do you have an example [even if it's in swift] of a symbol 
that's not exported correctly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78972



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

[Lldb-commits] [PATCH] D78588: [lldb/Core] Check that ArchSpec is valid.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

What's exactly the testcase doing?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D78588



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

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

This looks fine to me but please give Pavel another chance to look before 
committing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396



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

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

This is almost ready. After we're done with this round of cosmetics, I'll take 
another look a the algorithm and sign off.




Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:18-19
+  return true;
+else if (m_ptr_size == 8 && m_ht_64)
+  return true;
+  }

`else` after return



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:34-38
+  if (m_ptr_size == 4) {
+return UpdateFor(m_ht_32);
+  } else if (m_ptr_size == 8) {
+return UpdateFor(m_ht_64);
+  }

else after return. no need for `{}`



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:753-776
+
+while (tries < num_children) {
+  key_at_idx = m_keys_ptr + (test_idx * m_ptr_size);
+  val_at_idx = m_values_ptr + (test_idx * m_ptr_size);
+
+  key_at_idx = process_sp->ReadPointerFromMemory(key_at_idx, error);
+  if (error.Fail())

Can you add a comment explaining what this loop does?



Comment at: lldb/source/Plugins/Language/ObjC/NSSet.cpp:653
+default:
+  assert(false && "pointer size is not 4 nor 8 - get out of here ASAP");
+}

lldbassert. Also no need for the second part of the comment.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py:23
 def nscontainers_data_formatter_commands(self):
+
 self.expect(

stray line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396



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

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Thanks for taking care of this. It's a lot of work. First round of comments.




Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:12
+
+CFBasicHash::~CFBasicHash() { m_address = LLDB_INVALID_ADDRESS; }
+

Why do you need this? Can't you just use the default destructor?



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:18-19
+  return m_ht_32;
+  return m_ht_64;
+  return false;
+}

I'm under the impression the second return is never hit.



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:23-24
+bool CFBasicHash::Update(addr_t addr, ExecutionContextRef exe_ctx_rf) {
+  if (addr == LLDB_INVALID_ADDRESS)
+return false;
+

Can this ever happen? What if `addr` is `0` instead?



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:28-29
+  m_exe_ctx_ref = exe_ctx_rf;
+  m_ptr_size =
+  m_exe_ctx_ref.GetTargetSP()->GetArchitecture().GetAddressByteSize();
+  m_byte_order = m_exe_ctx_ref.GetTargetSP()->GetArchitecture().GetByteOrder();

Thanks for doing this, as it will work on remote devices. We really need to 
check the test passes on arm64 before committing.



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:35-71
+
+size = sizeof(__CFBasicHash::RuntimeBase);
+size += sizeof(__CFBasicHash::Bits);
+
+DataBufferHeap buffer(size, 0);
+m_exe_ctx_ref.GetProcessSP()->ReadMemory(addr, buffer.GetBytes(), size,
+ error);

These two pieces of code, `m_ptr_size == 4` and `m_ptr_size == 8` are 
surprisingly similar. I'm really worried we might have a bug in one of them and 
miss the other. Is there anything we can do to share the code? [e.g. 
templatize].



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:139-140
+
+  // Unsupported architecure
+  return false;
+}

This could be an `lldb_assert` or `unreachable`. We really shouldn't be here if 
ptrsize != 8 or ptrsize != 4, unless there's an egregious bug.



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:101
 namespace formatters {
+#pragma mark NSDictionaryI
 class NSDictionaryISyntheticFrontEnd : public SyntheticChildrenFrontEnd {

Why?



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:145
 
+#pragma mark NSCFDictionary
+class NSCFDictionarySyntheticFrontEnd : public SyntheticChildrenFrontEnd {

Again, why?



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:178-188
+private:
+  // Prime numbers. Values above 100 have been adjusted up so that the
+  // malloced block size will be just below a multiple of 512; values
+  // above 1200 have been adjusted up to just below a multiple of 4096.
+  constexpr static const uintptr_t NSDictionaryCapacities[] = {
+  0,3,6, 11,19,32,   52,
+  85,   118,  155,   237,   390,   672,  1065,

Maybe a reference to the foundation header where these are defined, if public.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396



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

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

[Davide Italiano via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1029
+  return false;
+
+case DW_TAG_compile_unit:

aprantl wrote:
> To save everyone the trouble:
> ```
> 0x002a:   DW_TAG_subprogram
> DW_AT_low_pc  (0x)
> DW_AT_high_pc (0x0006)
> DW_AT_frame_base  (DW_OP_reg6 RBP)
> DW_AT_name("f")
> DW_AT_decl_file   ("/tmp/t.c")
> DW_AT_decl_line   (1)
> DW_AT_external(true)
> 
> 0x003f: DW_TAG_variable
>   DW_AT_name  ("g_i")
>   DW_AT_type  (0x0055 "int")
>   DW_AT_decl_file ("/tmp/t.c")
>   DW_AT_decl_line (2)
>   DW_AT_location  (DW_OP_addr 0x280)
> 
> 0x0054: NULL
> ```
hmm, so, what's the DWARF'y way to find out whether a variable is static? 
This code is just moved from another place which did this.


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

https://reviews.llvm.org/D77698



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

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

[Davide Italiano via Phabricator via lldb-commits]

davide closed this revision.
davide added a comment.

commit d51b38f1b3a34c2a8e1869af6434ebd743ce7a5e 
 (HEAD -> 
master, origin/master, origin/HEAD)
Author: Davide Italiano 
Date:   Wed Apr 8 11:06:00 2020 -0700

  [DWARF] Not all the constant variables are "static".
  
  Fixes rdar://problem/61402307
  
  Differential Revision: https://reviews.llvm.org/D77698


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

https://reviews.llvm.org/D77698



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

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

[Davide Italiano via Phabricator via lldb-commits]

davide updated this revision to Diff 256063.
davide added a comment.
Herald added a reviewer: jdoerfert.

Added test, updated comment.


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

https://reviews.llvm.org/D77698

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/static_scope.s

Index: lldb/test/Shell/SymbolFile/DWARF/static_scope.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/static_scope.s
@@ -0,0 +1,312 @@
+# Test that the DWARF parser assigns the right scope to the
+# variable `b`, which is `local` and not `static`.
+
+# REQUIRES: x86
+# UNSUPPORTED: lldb-repro
+
+# RUN: llvm-mc -triple=x86_64-apple-macosx10.15.0 -filetype=obj %s > %t.o
+# RUN: lldb-test symbols %t.o | FileCheck %s
+
+# CHECK: Variable{{.*}}, name = "b", type = {{.*}} (int), scope = local
+
+	.section	__TEXT,__text,regular,pure_instructions
+	.macosx_version_min 10, 15
+	.file	1 "/Users/davide/work/build/bin" "a.c"
+	.globl	_main   ## -- Begin function main
+	.p2align	4, 0x90
+_main:  ## @main
+Lfunc_begin0:
+	.loc	1 2 0   ## a.c:2:0
+	.cfi_startproc
+## %bb.0:
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+Ltmp0:
+	##DEBUG_VALUE: b <- 3
+	.loc	1 5 9 prologue_end  ## a.c:5:9
+	movl	_a(%rip), %eax
+Ltmp1:
+	.loc	1 7 5   ## a.c:7:5
+	xorl	%eax, %eax
+	popq	%rbp
+	retq
+Ltmp2:
+Lfunc_end0:
+	.cfi_endproc
+## -- End function
+	.globl	_a  ## @a
+.zerofill __DATA,__common,_a,4,2
+	.section	__DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+	.byte	1   ## Abbreviation Code
+	.byte	17  ## DW_TAG_compile_unit
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	37  ## DW_AT_producer
+	.byte	14  ## DW_FORM_strp
+	.byte	19  ## DW_AT_language
+	.byte	5   ## DW_FORM_data2
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\202|" ## DW_AT_LLVM_sysroot
+	.byte	14  ## DW_FORM_strp
+	.byte	16  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\341\177"  ## DW_AT_APPLE_optimized
+	.byte	25  ## DW_FORM_flag_present
+	.byte	17  ## DW_AT_low_pc
+	.byte	1   ## DW_FORM_addr
+	.byte	18  ## DW_AT_high_pc
+	.byte	6   ## DW_FORM_data4
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	2   ## Abbreviation Code
+	.byte	52  ## DW_TAG_variable
+	.byte	0   ## DW_CHILDREN_no
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	63  ## DW_AT_external
+	.byte	25  ## DW_FORM_flag_present
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11  ## DW_FORM_data1
+	.byte	2   ## DW_AT_location
+	.byte	24  ## DW_FORM_exprloc
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	3   ## Abbreviation Code
+	.byte	53  ## DW_TAG_volatile_type
+	.byte	0   ## DW_CHILDREN_no
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	4   ## Abbreviation Code
+	.byte	36  ## DW_TAG_base_type
+	.byte	0   ## DW_CHILDREN_no
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	62  ## DW_AT_encoding
+	.byte	11  ## DW_FORM_data1
+	.byte	11  ## DW_AT_byte_size
+	.byte	11  ## DW_FORM_data1
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	5   ## Abbreviation Code
+	.byte	46  ## DW_TAG_subprogram
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	17

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added reviewers: jingham, labath.
Herald added subscribers: arphaman, aprantl.
davide added a comment.
davide added a reviewer: friss.

See also https://bugs.llvm.org/show_bug.cgi?id=45471


SymbolFileDWARF::ParseVariableDIE consider all constant variables as "static".
This is incorrect, and causes `frame var` to fail when printing them.

e.g.

  volatile int a;   
 main() {
  {
int b = 3;
a;
  }
}

const_value variables are more common at `-O1/-O2/..`, so this wasn't noticed 
by many.

Fixes rdar://problem/61402307

I also need to craft a testcase. Ideas on how to do this are appreciated


https://reviews.llvm.org/D77698

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3442,7 +3442,7 @@
 }
   }
 } else {
-  if (location_is_const_value_data)
+  if (location_is_const_value_data && die.GetDIE()->IsGlobalOrStaticVariable())
 scope = eValueTypeVariableStatic;
   else {
 scope = eValueTypeVariableLocal;
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -204,35 +204,8 @@
 case DW_AT_location:
 case DW_AT_const_value:
   has_location_or_const_value = true;
-  if (tag == DW_TAG_variable) {
-const DWARFDebugInfoEntry *parent_die = die.GetParent();
-while (parent_die != nullptr) {
-  switch (parent_die->Tag()) {
-  case DW_TAG_subprogram:
-  case DW_TAG_lexical_block:
-  case DW_TAG_inlined_subroutine:
-// Even if this is a function level static, we don't add it. We
-// could theoretically add these if we wanted to by
-// introspecting into the DW_AT_location and seeing if the
-// location describes a hard coded address, but we don't want
-// the performance penalty of that right now.
-is_global_or_static_variable = false;
-parent_die = nullptr; // Terminate the while loop.
-break;
-
-  case DW_TAG_compile_unit:
-  case DW_TAG_partial_unit:
-is_global_or_static_variable = true;
-parent_die = nullptr; // Terminate the while loop.
-break;
-
-  default:
-parent_die =
-parent_die->GetParent(); // Keep going in the while loop.
-break;
-  }
-}
-  }
+  is_global_or_static_variable = die.IsGlobalOrStaticVariable();
+
   break;
 
 case DW_AT_specification:
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -167,6 +167,8 @@
   void SetSiblingIndex(uint32_t idx) { m_sibling_idx = idx; }
   void SetParentIndex(uint32_t idx) { m_parent_idx = idx; }
 
+  bool IsGlobalOrStaticVariable() const;
+
 protected:
   static DWARFDeclContext
   GetDWARFDeclContextStatic(const DWARFDebugInfoEntry *die, DWARFUnit *cu);
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -1016,6 +1016,34 @@
   return nullptr;
 }
 
+bool DWARFDebugInfoEntry::IsGlobalOrStaticVariable() const {
+  if (Tag() != DW_TAG_variable)
+return false;
+  const DWARFDebugInfoEntry *parent_die = GetParent();
+  while (parent_die != nullptr) {
+switch (parent_die->Tag()) {
+case DW_TAG_subprogram:
+case DW_TAG_lexical_block:
+case DW_TAG_inlined_subroutine:
+  // Even if this is a function level static, we don't add it. We
+  // could theoretically add these if we wanted to by
+  // introspecting into the DW_AT_location and seeing if the
+  // location describes a hard coded address, but we don't want
+  // the performance penalty of that right now.
+  return false;
+
+case DW_TAG_compile_unit:
+case DW_TAG_partial_unit:
+  return true;
+
+default:
+ 

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

See also https://bugs.llvm.org/show_bug.cgi?id=45471


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

https://reviews.llvm.org/D77698



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

[Lldb-commits] [PATCH] D77173: [PATCH] [debugserver/ARM64] Make sure watchpoints hit are attributed correctly.

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added a reviewer: jasonmolenda.
davide added a project: LLDB.
Herald added subscribers: danielkiss, kristof.beyls.

>From e330facaea0c3780734a6a061134551662fb9d74 Mon Sep 17 00:00:00 2001
From: Davide Italiano 
Date: Tue, 31 Mar 2020 13:55:36 -0700
Subject: [PATCH] [debugserver/ARM64] Make sure watchpoints hit are attributed
 correctly.

This didn't happen for arm64 if you have watches for variables
that are contigous in memory.


-

.../watchpoints/watchpoint_count/Makefile |  3 ++
 .../watchpoint_count/TestWatchpointCount.py   | 43 +++
 .../watchpoints/watchpoint_count/main.c   | 13 ++
 .../source/MacOSX/arm64/DNBArchImplARM64.cpp  | 43 ++-
 4 files changed, 82 insertions(+), 20 deletions(-)
 create mode 100644 lldb/test/API/commands/watchpoints/watchpoint_count/Makefile
 create mode 100644 
lldb/test/API/commands/watchpoints/watchpoint_count/TestWatchpointCount.py
 create mode 100644 lldb/test/API/commands/watchpoints/watchpoint_count/main.c


https://reviews.llvm.org/D77173

Files:
  lldb/test/API/commands/watchpoints/watchpoint_count/Makefile
  lldb/test/API/commands/watchpoints/watchpoint_count/TestWatchpointCount.py
  lldb/test/API/commands/watchpoints/watchpoint_count/main.c
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp

Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -1067,31 +1067,34 @@
"DNBArchMachARM64::GetHardwareWatchpointHit() addr = 0x%llx",
(uint64_t)addr);
 
-  // This is the watchpoint value to match against, i.e., word address.
-  nub_addr_t wp_val = addr & ~((nub_addr_t)3);
   if (kret == KERN_SUCCESS) {
 DBG _state = m_state.dbg;
 uint32_t i, num = NumSupportedHardwareWatchpoints();
 for (i = 0; i < num; ++i) {
   nub_addr_t wp_addr = GetWatchAddress(debug_state, i);
-  DNBLogThreadedIf(LOG_WATCHPOINTS, "DNBArchMachARM64::"
-"GetHardwareWatchpointHit() slot: %u "
-"(addr = 0x%llx).",
-   i, (uint64_t)wp_addr);
-  if (wp_val == wp_addr) {
-uint32_t byte_mask = bits(debug_state.__wcr[i], 12, 5);
-
-// Sanity check the byte_mask, first.
-if (LowestBitSet(byte_mask) < 0)
-  continue;
-
-// Check that the watchpoint is enabled.
-if (!IsWatchpointEnabled(debug_state, i))
-  continue;
-
-// Compute the starting address (from the point of view of the
-// debugger).
-addr = wp_addr + LowestBitSet(byte_mask);
+  uint32_t byte_mask = bits(debug_state.__wcr[i], 12, 5);
+
+  DNBLogThreadedIf(LOG_WATCHPOINTS, "DNBArchImplX86_64::"
+   "GetHardwareWatchpointHit() slot: %u "
+   "(addr = 0x%llx; byte_mask = 0x%x)",
+   i, static_cast(wp_addr),
+   byte_mask);
+
+  if (!IsWatchpointEnabled(debug_state, i))
+continue;
+
+  if (bits(wp_addr, 48, 3) != bits(addr, 48, 3))
+continue;
+
+  // Sanity check the byte_mask
+  uint32_t lsb = LowestBitSet(byte_mask);
+  if (lsb < 0)
+continue;
+
+  uint64_t byte_to_match = bits(addr, 2, 0);
+
+  if (byte_mask & (1 << byte_to_match)) {
+addr = wp_addr + lsb;
 return i;
   }
 }
Index: lldb/test/API/commands/watchpoints/watchpoint_count/main.c
===
--- /dev/null
+++ lldb/test/API/commands/watchpoints/watchpoint_count/main.c
@@ -0,0 +1,13 @@
+#include 
+#include 
+
+int main() {
+  uint8_t x1 = 0;
+  uint16_t x2 = 0;
+
+  printf("patatino\n");
+
+  x1 += 1;
+  x2 += 2;
+  return 0;
+}
Index: lldb/test/API/commands/watchpoints/watchpoint_count/TestWatchpointCount.py
===
--- /dev/null
+++ lldb/test/API/commands/watchpoints/watchpoint_count/TestWatchpointCount.py
@@ -0,0 +1,43 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestWatchpointCount(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+TestBase.setUp(self)
+
+def test_watchpoint_count(self):
+self.build()
+(_, process, thread, _) = lldbutil.run_to_source_breakpoint(self, "patatino", lldb.SBFileSpec("main.c"))
+frame = thread.GetFrameAtIndex(0)
+first_var = frame.FindVariable("x1")
+second_var = frame.FindVariable("x2")
+
+error = lldb.SBError()
+first_watch = first_var.Watch(True, False, True, error)
+if 

[Lldb-commits] [PATCH] D76730: [RNBRemote/arm64] Remove a couple of unused functions

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added reviewers: jasonmolenda, LLDB.
Herald added a subscriber: kristof.beyls.

Jason, before submitting this I wanted to run it past you.


https://reviews.llvm.org/D76730

Files:
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -6068,148 +6068,6 @@
   return SendPacket("OK");
 }
 
-static bool MachHeaderIsMainExecutable(nub_process_t pid, uint32_t addr_size,
-   nub_addr_t mach_header_addr,
-   mach_header ) {
-  DNBLogThreadedIf(LOG_RNB_PROC, "GetMachHeaderForMainExecutable(pid = %u, "
- "addr_size = %u, mach_header_addr = "
- "0x%16.16llx)",
-   pid, addr_size, mach_header_addr);
-  const nub_size_t bytes_read =
-  DNBProcessMemoryRead(pid, mach_header_addr, sizeof(mh), );
-  if (bytes_read == sizeof(mh)) {
-DNBLogThreadedIf(
-LOG_RNB_PROC, "GetMachHeaderForMainExecutable(pid = %u, addr_size = "
-  "%u, mach_header_addr = 0x%16.16llx): mh = {\n  magic = "
-  "0x%8.8x\n  cpu = 0x%8.8x\n  sub = 0x%8.8x\n  filetype = "
-  "%u\n  ncmds = %u\n  sizeofcmds = 0x%8.8x\n  flags = "
-  "0x%8.8x }",
-pid, addr_size, mach_header_addr, mh.magic, mh.cputype, mh.cpusubtype,
-mh.filetype, mh.ncmds, mh.sizeofcmds, mh.flags);
-if ((addr_size == 4 && mh.magic == MH_MAGIC) ||
-(addr_size == 8 && mh.magic == MH_MAGIC_64)) {
-  if (mh.filetype == MH_EXECUTE) {
-DNBLogThreadedIf(LOG_RNB_PROC, "GetMachHeaderForMainExecutable(pid = "
-   "%u, addr_size = %u, mach_header_addr = "
-   "0x%16.16llx) -> this is the "
-   "executable!!!",
- pid, addr_size, mach_header_addr);
-return true;
-  }
-}
-  }
-  return false;
-}
-
-static nub_addr_t GetMachHeaderForMainExecutable(const nub_process_t pid,
- const uint32_t addr_size,
- mach_header ) {
-  struct AllImageInfos {
-uint32_t version;
-uint32_t dylib_info_count;
-uint64_t dylib_info_addr;
-  };
-
-  uint64_t mach_header_addr = 0;
-
-  const nub_addr_t shlib_addr = DNBProcessGetSharedLibraryInfoAddress(pid);
-  uint8_t bytes[256];
-  nub_size_t bytes_read = 0;
-  DNBDataRef data(bytes, sizeof(bytes), false);
-  DNBDataRef::offset_t offset = 0;
-  data.SetPointerSize(addr_size);
-
-  // When we are sitting at __dyld_start, the kernel has placed the
-  // address of the mach header of the main executable on the stack. If we
-  // read the SP and dereference a pointer, we might find the mach header
-  // for the executable. We also just make sure there is only 1 thread
-  // since if we are at __dyld_start we shouldn't have multiple threads.
-  if (DNBProcessGetNumThreads(pid) == 1) {
-nub_thread_t tid = DNBProcessGetThreadAtIndex(pid, 0);
-if (tid != INVALID_NUB_THREAD) {
-  DNBRegisterValue sp_value;
-  if (DNBThreadGetRegisterValueByID(pid, tid, REGISTER_SET_GENERIC,
-GENERIC_REGNUM_SP, _value)) {
-uint64_t sp =
-addr_size == 8 ? sp_value.value.uint64 : sp_value.value.uint32;
-bytes_read = DNBProcessMemoryRead(pid, sp, addr_size, bytes);
-if (bytes_read == addr_size) {
-  offset = 0;
-  mach_header_addr = data.GetPointer();
-  if (MachHeaderIsMainExecutable(pid, addr_size, mach_header_addr, mh))
-return mach_header_addr;
-}
-  }
-}
-  }
-
-  // Check the dyld_all_image_info structure for a list of mach header
-  // since it is a very easy thing to check
-  if (shlib_addr != INVALID_NUB_ADDRESS) {
-bytes_read =
-DNBProcessMemoryRead(pid, shlib_addr, sizeof(AllImageInfos), bytes);
-if (bytes_read > 0) {
-  AllImageInfos aii;
-  offset = 0;
-  aii.version = data.Get32();
-  aii.dylib_info_count = data.Get32();
-  if (aii.dylib_info_count > 0) {
-aii.dylib_info_addr = data.GetPointer();
-if (aii.dylib_info_addr != 0) {
-  const size_t image_info_byte_size = 3 * addr_size;
-  for (uint32_t i = 0; i < aii.dylib_info_count; ++i) {
-bytes_read = DNBProcessMemoryRead(pid, aii.dylib_info_addr +
-   i * image_info_byte_size,
-  image_info_byte_size, bytes);
-if (bytes_read != image_info_byte_size)
-  break;
-offset = 0;
-mach_header_addr = 

[Lldb-commits] [PATCH] D76687: [lldb][NFC] Always update m_cache_{hits/misses} in FormatCache

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Testcase?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76687



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

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In D76697#1939731 , @JDevlieghere 
wrote:

> According to the comment there is at least one valid reason for this code 
> path to trigger. An `lldb_assert` will encourage users to file bugs and I 
> think we have enough real issues that we can do without the additional noise.


I do believe users should file bugs if this triggers, yes. In the recent past, 
I looked at ObjC more than most, so these bugs can land on me.


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

https://reviews.llvm.org/D76697



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

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

I don't recall the details.

  Soft assertions. LLDB provides lldb_assert() as a soft alternative to cover 
the middle ground of situations that indicate a recoverable bug in LLDB. In a 
Debug configuration lldb_assert() behaves like assert(). In a Release 
configuration it will print a warning and encourage the user to file a bug 
report, similar to LLVM’s crash handler, and then return execution. Use these 
sparingly and only if error handling is not otherwise feasible. Specifically, 
new code should not be using lldb_assert() and existing uses should be replaced 
by other means of error handling.

This is a recoverable bug, in my books.


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

https://reviews.llvm.org/D76697



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

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Also, this was asserting already -- so it's not introducing any new `assert()`.


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

https://reviews.llvm.org/D76697



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

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Please don't do this. I've seen that assertion triggering. If anything, you 
might want to make a `lldbassert`.


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

https://reviews.llvm.org/D76697



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

[Lldb-commits] [PATCH] D75496: [lldb][NFC] Remove some commented out code in TypeSystemClang

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

@teemperor quite honestly you touched this code enough lately that you can 
commit such trivial changes without asking for review.


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

https://reviews.llvm.org/D75496



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

[Lldb-commits] [PATCH] D75496: [lldb][NFC] Remove some commented out code in TypeSystemClang

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

LGTM.


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

https://reviews.llvm.org/D75496



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

[Lldb-commits] [PATCH] D73921: Assert that a subprogram should have a name when parsing DWARF

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In D73921#1868286 , @JDevlieghere 
wrote:

> In D73921#1868228 , @jingham wrote:
>
> > In D73921#1855961 , @davide wrote:
> >
> > > DWARFASTParserClang looks to me the wrong layer to fix this. Why can't 
> > > this be caught in the generic DWARF Parser?
> > >  I also believe that it's better if dwarfdump -verify crashes on this, 
> > > rather than lldb.
> >
> >
> > Many
> >
> > In D73921#1868191 , @JDevlieghere 
> > wrote:
> >
> > > Given that this error is non-actionable, I don't see any value in 
> > > diagnosing this LLDB. It is important to have this in dwarfdump, which 
> > > does not detect this right now.
> > >
> > > It might be interesting to have LLDB run in a sort of "pedantic" mode 
> > > which verifies all the DWARF it consumes with the dwarf verifier in LLVM. 
> > > We have something similar in dsymutil which runs the verifier over the 
> > > generated dSYM.
> >
> >
> > Note that many OS X developers never debug a dSYM build of their project.  
> > They debug with .o files, then make a dSYM when they do their release 
> > builds.  And they probably don't look at the output of dsymutil amidst all 
> > the noise of a build.  So if we only do this in dsymutil, we are greatly 
> > narrowing the range of folks who might see & report this error to us.
>
>
> I think you misunderstood my suggestion. I'm not saying that we should limit 
> this to dsymutil. I'm saying that dsymutil has a mode where it verifies the 
> dSYM it just created. It's entirely optional and you have to pass `--verify` 
> to enable it. I suggest we have something similar in LLDB, where we have a 
> pedantic mode that, when enabled, checks all the DWARF it reads with the 
> DWARF verifier.
>
> As discussed offline with Shafik, I prefer this over the current approach for 
> a few reasons:
>
> - It would make this behavior opt-in. Verifying the DWARF can be expensive 
> and not every user has control over the debug info it reads. It should be 
> possible to silence these warnings if they don't change LLDB's behavior.
> - It would provide much better coverage than some ad-hoc checks. Currently, 
> not getting these kind of errors form LLDB doesn't tell you much. We may or 
> may not have a check for a particular kind of invalid DWARF, so to be sure 
> you'd still have to run it through `dwarfdump -verify`.
> - It would mean we only have to maintain a single DWARF verifier, which is 
> already tested extensively.
> - It fits with our long-term goal of moving to LLVM's DWARF parser.


I second this motion.
Realistically what this patch is currently doing is diagnosing a very narrow 
problem emitting a somewhat obscure diagnostic for users. People who use 
debuggers aren't necessarily asked to understand DWARF.
If we really want to move towards a verification mode, the plan suggested above 
is much more reasonable than having piecemeal diagnostic sprinkled over the 
parser.


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

https://reviews.llvm.org/D73921



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

[Lldb-commits] [PATCH] D73938: [Host.mm] Check for the right macro instead of inlining it

[Davide Italiano via Phabricator via lldb-commits]

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

LGTM =)


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

https://reviews.llvm.org/D73938



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

[Lldb-commits] [PATCH] D73952: [lldb] Ignore type sugar in TypeSystemClang::GetPointerType

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Good catch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73952



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

[Lldb-commits] [PATCH] D71316: [FormatManager] Upstream and test swift bits for GetCandidateLanguages().

[Davide Italiano via Phabricator via lldb-commits]

davide closed this revision.
davide added a comment.

Not relevant anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71316



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

[Lldb-commits] [PATCH] D73938: [Host.mm] Check for the right macro instead of inlining it.

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added reviewers: jingham, friss, vsk.
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73938

Files:
  lldb/source/Host/macosx/objcxx/Host.mm


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -153,7 +153,7 @@
   return NULL;
 }
 
-#if !defined(__arm__) && !defined(__arm64__) && !defined(__aarch64__)
+#if !NO_XPC_SERVICES
 
 const char *applscript_in_new_tty = "tell application \"Terminal\"\n"
 "   activate\n"
@@ -307,11 +307,11 @@
   return error;
 }
 
-#endif // #if !defined(__arm__) && !defined(__arm64__) && !defined(__aarch64__)
+#endif // #if !NO_XPC_SERVICES
 
 bool Host::OpenFileInExternalEditor(const FileSpec _spec,
 uint32_t line_no) {
-#if defined(__arm__) || defined(__arm64__) || defined(__aarch64__)
+#if NO_XPC_SERVICES
   return false;
 #else
   // We attach this to an 'odoc' event to specify a particular selection
@@ -404,7 +404,7 @@
   }
 
   return true;
-#endif // #if !defined(__arm__) && !defined(__arm64__) && !defined(__aarch64__)
+#endif // #if !NO_XPC_SERVICES
 }
 
 Environment Host::GetEnvironment() { return Environment(*_NSGetEnviron()); }
@@ -1263,7 +1263,7 @@
   }
 
   if (launch_info.GetFlags().Test(eLaunchFlagLaunchInTTY)) {
-#if !defined(__arm__) && !defined(__arm64__) && !defined(__aarch64__)
+#if !NO_XPC_SERVICES
 return LaunchInNewTerminalWithAppleScript(exe_spec.GetPath().c_str(),
   launch_info);
 #else


Index: lldb/source/Host/macosx/objcxx/Host.mm
===
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -153,7 +153,7 @@
   return NULL;
 }
 
-#if !defined(__arm__) && !defined(__arm64__) && !defined(__aarch64__)
+#if !NO_XPC_SERVICES
 
 const char *applscript_in_new_tty = "tell application \"Terminal\"\n"
 "   activate\n"
@@ -307,11 +307,11 @@
   return error;
 }
 
-#endif // #if !defined(__arm__) && !defined(__arm64__) && !defined(__aarch64__)
+#endif // #if !NO_XPC_SERVICES
 
 bool Host::OpenFileInExternalEditor(const FileSpec _spec,
 uint32_t line_no) {
-#if defined(__arm__) || defined(__arm64__) || defined(__aarch64__)
+#if NO_XPC_SERVICES
   return false;
 #else
   // We attach this to an 'odoc' event to specify a particular selection
@@ -404,7 +404,7 @@
   }
 
   return true;
-#endif // #if !defined(__arm__) && !defined(__arm64__) && !defined(__aarch64__)
+#endif // #if !NO_XPC_SERVICES
 }
 
 Environment Host::GetEnvironment() { return Environment(*_NSGetEnviron()); }
@@ -1263,7 +1263,7 @@
   }
 
   if (launch_info.GetFlags().Test(eLaunchFlagLaunchInTTY)) {
-#if !defined(__arm__) && !defined(__arm64__) && !defined(__aarch64__)
+#if !NO_XPC_SERVICES
 return LaunchInNewTerminalWithAppleScript(exe_spec.GetPath().c_str(),
   launch_info);
 #else
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D73921: Assert that a subprogram should have a name when parsing DWARF

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

DWARFASTParserClang looks to me the wrong layer to fix this. Why can't this be 
caught in the generic DWARF Parser?
I also believe that it's better if dwarfdump -verify crashes on this, rather 
than lldb.


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

https://reviews.llvm.org/D73921



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

[Lldb-commits] [PATCH] D73921: Assert that a subprogram should have a name when parsing DWARF

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

`DWARFASTParserClang` looks to me the right layer to fix this. Why can't this 
be caught in the generic DWARF Parser?
I also believe that it's better if `dwarfdump -verify` crashes on this, rather 
than lldb.


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

https://reviews.llvm.org/D73921



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

[Lldb-commits] [PATCH] D73913: [lldb/DataExtractor] Fix UB shift in GetMaxS64Bitfield

[Davide Italiano via Phabricator via lldb-commits]

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

LGTM, thanks for taking care of this.


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

https://reviews.llvm.org/D73913



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

[Lldb-commits] [PATCH] D73913: [lldb/DataExtractor] Fix UB shift in GetMaxS64Bitfield

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In D73913#1855486 , @shafik wrote:

> I wish I had caught that one when I did D70992 
> , might be worth checking out more of the 
> code for similar issues.


I guess this is why we have tooling =)


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

https://reviews.llvm.org/D73913



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

[Lldb-commits] [PATCH] D73389: [lldb/Breakpoint] Include whether or not a breakpoint is a HW BP

[Davide Italiano via Phabricator via lldb-commits]

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

LGTM.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D73389



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

[Lldb-commits] [PATCH] D72879: Add testing for DW_OP_piece and fix a bug with small Scalar values.

[Davide Italiano via Phabricator via lldb-commits]

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

Looks great! Thanks Adrian. My understanding is that `DW_OP_piece` is still 
incomplete, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72879



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

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to TypeSystemClang

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

This is very good, go for it. Should we do the same for Swift? cc: @aprantl 
For the future, please CC: me directly on these kind of changes if you want my 
review, as I might miss them otherwise.


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

https://reviews.llvm.org/D72684



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

[Lldb-commits] [PATCH] D72495: [lldb] Make CompleteTagDeclsScope completion order deterministic

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

LGTM


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D72495



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

[Lldb-commits] [PATCH] D72413: Add missing nullptr checks.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In D72413#1813630 , @teemperor wrote:

> The C++ expression parser will probably behave incredibly incorrectly without 
> a persistent state but before this patch it just crashed, so I think this is 
> good to go.


Should we consider a refactor to `ClangExpressionDeclMap` that makes this more 
obvious? [and maybe allows us to not sprinkling these checks everywhere] -- as 
a followup?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72413



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

[Lldb-commits] [PATCH] D72413: Add missing nullptr checks.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

@teemperor what do you think?


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

https://reviews.llvm.org/D72413



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

[Lldb-commits] [PATCH] D72413: Add missing nullptr checks.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In D72413#1812464 , @aprantl wrote:

> Should we merge this like that, or is there a better way of doing this?


We should merge it like this, IMHO.


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

https://reviews.llvm.org/D72413



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

[Lldb-commits] [PATCH] D72413: Add missing nullptr checks.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Raphael and Jim should look at the expression evaluator bits.


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

https://reviews.llvm.org/D72413



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

[Lldb-commits] [PATCH] D68679: [CMake] Add a cache for iOS.

[Davide Italiano via Phabricator via lldb-commits]

davide abandoned this revision.
davide added a comment.

We don't need this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68679



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

[Lldb-commits] [PATCH] D71379: lldbutil: Forward ASan launch info to test inferiors

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

This looks good to me.


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

https://reviews.llvm.org/D71379



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

[Lldb-commits] [PATCH] D71311: LanguageRuntime: Simplify NSException::GetSummary() output

[Davide Italiano via Phabricator via lldb-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71311



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

[Lldb-commits] [PATCH] D71311: LanguageRuntime: Simplify NSException::GetSummary() output

[Davide Italiano via Phabricator via lldb-commits]

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

Did you test on swift?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71311



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

[Lldb-commits] [PATCH] D71316: [FormatManager] Upstream and test swift bits for GetCandidateLanguages().

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

This is not a bad idea after all. Let me see if I can cook something reasonable 
without getting burned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71316



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

[Lldb-commits] [PATCH] D71316: [FormatManager] Upstream and test swift bits for GetCandidateLanguages().

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Ideally -- if we're able to make this a callback in the language plugins, we 
don't need this patch at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71316



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

[Lldb-commits] [PATCH] D71316: [FormatManager] Upstream and test swift bits for GetCandidateLanguages().

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added reviewers: teemperor, aprantl, labath, friss, jingham.
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71316

Files:
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/unittests/DataFormatter/FormatManagerTests.cpp


Index: lldb/unittests/DataFormatter/FormatManagerTests.cpp
===
--- lldb/unittests/DataFormatter/FormatManagerTests.cpp
+++ lldb/unittests/DataFormatter/FormatManagerTests.cpp
@@ -43,7 +43,11 @@
   EXPECT_EQ(FormatManager::GetCandidateLanguages(eLanguageTypeC_plus_plus_14),
 candidates);
 
-  candidates = {eLanguageTypeObjC};
+  candidates = {eLanguageTypeObjC, eLanguageTypeSwift};
   EXPECT_EQ(FormatManager::GetCandidateLanguages(eLanguageTypeObjC),
 candidates);
+
+  candidates = {eLanguageTypeSwift, eLanguageTypeObjC};
+  EXPECT_EQ(FormatManager::GetCandidateLanguages(eLanguageTypeSwift),
+candidates);
 }
Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -582,6 +582,10 @@
 std::vector
 FormatManager::GetCandidateLanguages(lldb::LanguageType lang_type) {
   switch (lang_type) {
+  case lldb::eLanguageTypeSwift:
+return {lldb::eLanguageTypeSwift, lldb::eLanguageTypeObjC};
+  case lldb::eLanguageTypeObjC:
+return {lldb::eLanguageTypeObjC, lldb::eLanguageTypeSwift};
   case lldb::eLanguageTypeC:
   case lldb::eLanguageTypeC89:
   case lldb::eLanguageTypeC99:


Index: lldb/unittests/DataFormatter/FormatManagerTests.cpp
===
--- lldb/unittests/DataFormatter/FormatManagerTests.cpp
+++ lldb/unittests/DataFormatter/FormatManagerTests.cpp
@@ -43,7 +43,11 @@
   EXPECT_EQ(FormatManager::GetCandidateLanguages(eLanguageTypeC_plus_plus_14),
 candidates);
 
-  candidates = {eLanguageTypeObjC};
+  candidates = {eLanguageTypeObjC, eLanguageTypeSwift};
   EXPECT_EQ(FormatManager::GetCandidateLanguages(eLanguageTypeObjC),
 candidates);
+
+  candidates = {eLanguageTypeSwift, eLanguageTypeObjC};
+  EXPECT_EQ(FormatManager::GetCandidateLanguages(eLanguageTypeSwift),
+candidates);
 }
Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -582,6 +582,10 @@
 std::vector
 FormatManager::GetCandidateLanguages(lldb::LanguageType lang_type) {
   switch (lang_type) {
+  case lldb::eLanguageTypeSwift:
+return {lldb::eLanguageTypeSwift, lldb::eLanguageTypeObjC};
+  case lldb::eLanguageTypeObjC:
+return {lldb::eLanguageTypeObjC, lldb::eLanguageTypeSwift};
   case lldb::eLanguageTypeC:
   case lldb::eLanguageTypeC89:
   case lldb::eLanguageTypeC99:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D71311: LanguageRuntime: Simplify NSException::GetSummary() output

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: lldb/source/Plugins/Language/ObjC/NSException.cpp:107-109
+  if (NSStringSummaryProvider(*reason_sp, reason_str_summary, options) &&
+  !reason_str_summary.Empty()) {
+stream.Printf("%s", reason_str_summary.GetData());

jingham wrote:
> I wonder if it would be better to print "No reason" as the summary when 
> there's not a reason string?  Having the summary generally be there, but then 
> not be there at all when the exception wasn't given a reason string, is a 
> little disconcerting.
Arguably.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71311



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

[Lldb-commits] [PATCH] D71310: RFC: Remove "Validators"

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In D71310#1778534 , @jingham wrote:

> Adrian and I talked about this some more.  Apparently the idea was that you 
> have some type Foo and you want to look for some error state in instances of 
> that type (Foo::a + Foo::b < 10).  So you add a Type Validator for Foo that 
> does this check, and every time lldb prints a variable of type Foo, it will 
> run the Validator on it, and if it fails validation, then the printer will 
> print an ! at the beginning of the printing, and also there's an SB API to 
> get whether the Value passed the validator.
>
> So then you could just debug along, and either look for the ! in the 
> printing, or add a stop hook that checks the Validator result on all locals, 
> and if you ever saw the error state, you would know to investigate further.  
> That's actually a pretty neat idea.
>
> The current state of the code is that there is actually no way to add a Type 
> Validator.  To be really useful, there would need to be a way to create a 
> scripted validator, so the Python bindings and some command/SB API to 
> register the validator.
>
> The implementation is a little cut-and-paste too.  It shares all the same 
> options with the Synthetic child provider & Summaries (skips pointers, 
> cascade, etc.) but I don't think you would ever want a type validator that 
> would only validate references to a type, but not the type itself.  And then 
> the implementation is very cut & paste.  So I'm fine with deleting this for 
> now, but maybe adding it as an interesting project idea to the Projects page 
> - with a reference to the hash of this commit as a starting point?
>
> It's a neat idea but it's also been 5 years now and it was never made 
> useful...


+1. This code can be resurrected as-needed.


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

https://reviews.llvm.org/D71310



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

[Lldb-commits] [PATCH] D71296: Replace redundant code in LanguageCategory with templates (NFC)

[Davide Italiano via Phabricator via lldb-commits]

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

LGTM, in the same vein as D71231 .


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

https://reviews.llvm.org/D71296



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