[Lldb-commits] [PATCH] D98822: [lldb] [Process/Linux] Watch for fork/vfork notifications

2021-03-25 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added inline comments.



Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:145
 
+  // Remove all software breakpoints and return a vector of breakpoint data
+  // that can be used to readd them.

Long term, these functions are probably unnecessary. It just occurred to me to 
check how GDB handles it, and it handles clearing and restoring breakpoints on 
client side (i.e. by sending `z` and `Z` packets).


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

https://reviews.llvm.org/D98822

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

[Lldb-commits] [PATCH] D98822: [lldb] [Process/Linux] Watch for fork/vfork notifications

2021-03-25 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added inline comments.



Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:145
 
+  // Remove all software breakpoints and return a vector of breakpoint data
+  // that can be used to readd them.

mgorny wrote:
> Long term, these functions are probably unnecessary. It just occurred to me 
> to check how GDB handles it, and it handles clearing and restoring 
> breakpoints on client side (i.e. by sending `z` and `Z` packets).
Hmm, actually that depends on how much compatibility with old client versions 
we want to preserve. If we want forks/vforks to stop being broken with 
breakpoints when using an old client, we need to keep them as a fallback logic 
for when fork/vfork-events aren't supported by client.


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

https://reviews.llvm.org/D98822

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

[Lldb-commits] [lldb] d90b123 - [lldb] Fix TestVSCode.test_progress_events on Linux due to vdso

2021-03-25 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2021-03-25T10:48:58+01:00
New Revision: d90b1230ea62eb9738ea369df38607551ea412e0

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

LOG: [lldb] Fix TestVSCode.test_progress_events on Linux due to vdso

This currently fails when we get the module for `[vdso]` which doesn't have
any parsing event associated with it as it's just created from memory.

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py 
b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
index 07e455d94a6ba..1bd16f4819486 100644
--- a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -499,8 +499,13 @@ def test_progress_events(self):
 self.assertTrue(progressStart_ids == progressEnd_ids,
 ('Make sure we got a "progressEnd" for each '
  '"progressStart" event that we have.'))
+
+ignored_libraries = {"[vdso]"}
+
 # Verify we got a symbol table parsing progress event for each shared
 # library in our target.
 for target_shlib_basename in target_shlibs.keys():
-self.assertTrue(target_shlib_basename in symtab_progress_shlibs,
+if target_shlib_basename in ignored_libraries:
+continue
+self.assertIn(target_shlib_basename, symtab_progress_shlibs,
 'Make sure we got a symbol table progress event 
for "%s"' % (target_shlib_basename))



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

[Lldb-commits] [PATCH] D98822: [lldb] [Process/Linux] Watch for fork/vfork notifications

2021-03-25 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 333254.
mgorny added a comment.

Move saved breakpoint list directly into `NativeProcessProtocol`.


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

https://reviews.llvm.org/D98822

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Host/linux/Host.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Host/linux/Host.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/test/Shell/Subprocess/Inputs/fork.c
  lldb/test/Shell/Subprocess/Inputs/vfork.c
  lldb/test/Shell/Subprocess/fork-follow-parent-softbp.test
  lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
  lldb/test/Shell/Subprocess/fork-follow-parent.test
  lldb/test/Shell/Subprocess/vfork-follow-parent-softbp.test
  lldb/test/Shell/Subprocess/vfork-follow-parent.test

Index: lldb/test/Shell/Subprocess/vfork-follow-parent.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/vfork-follow-parent.test
@@ -0,0 +1,10 @@
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/vfork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/vfork-follow-parent-softbp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/vfork-follow-parent-softbp.test
@@ -0,0 +1,11 @@
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/vfork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+b child_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/fork-follow-parent.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/fork-follow-parent.test
@@ -0,0 +1,11 @@
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/fork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK: function run in child
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
@@ -0,0 +1,13 @@
+# REQUIRES: native && (target-x86 || target-x86_64 || target-aarch64) && dbregs-set
+# RUN: %clang_host -g %p/Inputs/fork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch -s
+watchpoint set variable -w write g_val
+# CHECK: Watchpoint created:
+continue
+# CHECK: function run in child
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = watchpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/fork-follow-parent-softbp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/fork-follow-parent-softbp.test
@@ -0,0 +1,12 @@
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/fork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+b child_func
+process launch
+# CHECK: function run in child
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/Inputs/vfork.c
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/Inputs/vfork.c
@@ -0,0 +1,37 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int g_val = 0;
+
+void parent_func() {
+  g_val = 1;
+  printf("function run in parent\n");
+}
+
+void child_func() {
+  // do something relatively safe
+  volatile int val = 0;
+  ++val;
+}
+
+int main() {
+  pid_t pid = vfork();
+  assert(pid != -1);
+
+  if (pid == 0) {
+child_func();
+_exit(0);
+  }
+
+  parent_func();
+  int status;
+  pid_t waited = waitpid(pid, &status, 0);
+  assert(waited == pid);
+  assert(WIFEXITED(status));
+  printf("child exited: %d\n", WEXITSTATUS(status));
+
+  return 0;
+}
Index: lldb/test/Shell/Subprocess/Inputs/fork.c
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/Inputs/fork.c
@@ -0,0 +1,36 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int g_val = 0;
+
+void parent_func() {
+  g_val = 1;
+  printf("function run in parent\n");
+}
+
+void child_func() {
+  g_val = 2;
+  printf("function run in child\n");
+}
+
+int main() {
+  pid_t pid = fork();
+  assert(pid != -1);
+
+  if (pid == 0) {
+child_func();
+_exit(0);
+  }
+
+  parent_func();
+  int stat

[Lldb-commits] [PATCH] D99331: [TESTS] Fix TestInlineStepping with ccac compiler

2021-03-25 Thread Alexey Vasilyev via Phabricator via lldb-commits 
vasilyev created this revision.
vasilyev added a reviewer: LLVM.
vasilyev added projects: LLVM, LLDB test suite on simulator.
vasilyev requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The tests expect the debugger steps into the function, not the string 
constructor. But that depends on the compiler. Thus, with "ssas" the tests are 
failing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99331

Files:
  lldb/test/API/functionalities/inline-stepping/calling.cpp


Index: lldb/test/API/functionalities/inline-stepping/calling.cpp
===
--- lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -129,8 +129,12 @@
 
 function_to_call (); // Make sure debug info for this function gets 
generated.
 
-max_value(123, 456);// Call max_value 
template
-max_value(std::string("abc"), std::string("0022")); // Call max_value 
specialized
+max_value(123, 456); // Call max_value template
+
+std::string s1 = "aba";
+std::string s2 = "0022";
+
+max_value(s1, s2);  // Call max_value specialized
 
 return 0;// About to return from main.
 }


Index: lldb/test/API/functionalities/inline-stepping/calling.cpp
===
--- lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -129,8 +129,12 @@
 
 function_to_call (); // Make sure debug info for this function gets generated.
 
-max_value(123, 456);// Call max_value template
-max_value(std::string("abc"), std::string("0022")); // Call max_value specialized
+max_value(123, 456); // Call max_value template
+
+std::string s1 = "aba";
+std::string s2 = "0022";
+
+max_value(s1, s2);  // Call max_value specialized
 
 return 0;// About to return from main.
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D99182: [NFC] Reordering parameters in getFile and getFileOrSTDIN

2021-03-25 Thread James Henderson via Phabricator via lldb-commits 
jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, with nit.




Comment at: llvm/lib/Support/MemoryBuffer.cpp:275
+  return getFileAux(
+  Filename, /*MapSize=*/-1, 0, /*IsText=*/false,
+  /*RequiresNullTerminator=*/false, IsVolatile);

Whilst you're modifying, add the named parameter comment for the `0`, so that 
it's not just a magic number.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99182

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

[Lldb-commits] [PATCH] D99182: [NFC] Reordering parameters in getFile and getFileOrSTDIN

2021-03-25 Thread Abhina Sree via Phabricator via lldb-commits 
abhina.sreeskantharajan updated this revision to Diff 333266.
abhina.sreeskantharajan added a comment.

Rebase and fix formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99182

Files:
  clang/lib/Tooling/JSONCompilationDatabase.cpp
  clang/tools/arcmt-test/arcmt-test.cpp
  lld/COFF/Driver.cpp
  lld/COFF/DriverUtils.cpp
  lld/ELF/InputFiles.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
  lldb/unittests/TestingSupport/TestUtilities.cpp
  llvm/include/llvm/Support/MemoryBuffer.h
  llvm/lib/BinaryFormat/Magic.cpp
  llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp
  llvm/lib/FuzzMutate/FuzzerCLI.cpp
  llvm/lib/IRReader/IRReader.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/Object/Binary.cpp
  llvm/lib/ProfileData/GCOV.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/TableGen/Main.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/lli/lli.cpp
  llvm/tools/llvm-ar/llvm-ar.cpp
  llvm/tools/llvm-cov/gcov.cpp
  llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
  llvm/tools/llvm-pdbutil/InputFile.cpp
  llvm/tools/llvm-pdbutil/llvm-pdbutil.cpp
  llvm/tools/llvm-rc/ResourceFileWriter.cpp
  llvm/tools/llvm-readobj/llvm-readobj.cpp
  llvm/tools/obj2yaml/obj2yaml.cpp
  llvm/tools/sanstats/sanstats.cpp
  llvm/utils/FileCheck/FileCheck.cpp

Index: llvm/utils/FileCheck/FileCheck.cpp
===
--- llvm/utils/FileCheck/FileCheck.cpp
+++ llvm/utils/FileCheck/FileCheck.cpp
@@ -821,9 +821,7 @@
 
   // Read the expected strings from the check file.
   ErrorOr> CheckFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(CheckFilename, /*FileSize=*/-1,
-   /*RequiresNullTerminator=*/true,
-   /*IsText=*/true);
+  MemoryBuffer::getFileOrSTDIN(CheckFilename, /*IsText=*/true);
   if (std::error_code EC = CheckFileOrErr.getError()) {
 errs() << "Could not open check file '" << CheckFilename
<< "': " << EC.message() << '\n';
@@ -845,9 +843,7 @@
 
   // Open the file to check and add it to SourceMgr.
   ErrorOr> InputFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename, /*FileSize=*/-1,
-   /*RequiresNullTerminator=*/true,
-   /*IsText=*/true);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, /*IsText=*/true);
   if (InputFilename == "-")
 InputFilename = ""; // Overwrite for improved diagnostic messages
   if (std::error_code EC = InputFileOrErr.getError()) {
Index: llvm/tools/sanstats/sanstats.cpp
===
--- llvm/tools/sanstats/sanstats.cpp
+++ llvm/tools/sanstats/sanstats.cpp
@@ -125,8 +125,8 @@
   cl::ParseCommandLineOptions(argc, argv,
   "Sanitizer Statistics Processing Tool");
 
-  ErrorOr> MBOrErr =
-  MemoryBuffer::getFile(ClInputFile, -1, false);
+  ErrorOr> MBOrErr = MemoryBuffer::getFile(
+  ClInputFile, /*IsText=*/false, /*RequiresNullTerminator=*/false);
   if (!MBOrErr) {
 errs() << argv[0] << ": " << ClInputFile << ": "
<< MBOrErr.getError().message() << '\n';
Index: llvm/tools/obj2yaml/obj2yaml.cpp
===
--- llvm/tools/obj2yaml/obj2yaml.cpp
+++ llvm/tools/obj2yaml/obj2yaml.cpp
@@ -36,7 +36,7 @@
 
 static Error dumpInput(StringRef File) {
   ErrorOr> FileOrErr =
-  MemoryBuffer::getFileOrSTDIN(File, /*FileSize=*/-1,
+  MemoryBuffer::getFileOrSTDIN(File, /*IsText=*/false,
/*RequiresNullTerminator=*/false);
   if (std::error_code EC = FileOrErr.getError())
 return errorCodeToError(EC);
Index: llvm/tools/llvm-readobj/llvm-readobj.cpp
===
--- llvm/tools/llvm-readobj/llvm-readobj.cpp
+++ llvm/tools/llvm-readobj/llvm-readobj.cpp
@@ -653,7 +653,7 @@
 /// Opens \a File and dumps it.
 static void dumpInput(StringRef File, ScopedPrinter &Writer) {
   ErrorOr> FileOrErr =
-  MemoryBuffer::getFileOrSTDIN(File, /*FileSize=*/-1,
+  MemoryBuffer::getFileOrSTDIN(File, /*IsText=*/false,
/*RequiresNullTerminator=*/false);
   if (std::error_code EC = FileOrErr.getError())
 return reportError(errorCodeToError(EC), File);
Index: llvm/tools/llvm-rc/ResourceFileWriter.cpp
===
--- llvm/tools/llvm-rc/ResourceFileWriter.cpp
+++ llvm/tools/llvm-rc/ResourceFileWriter.cpp
@@ -1524,14 +1524,16 @@
   // properly though, so if using that to append paths below, this early
   // exception case could be removed.)
   if (sys::path::has_root_directory(File))
-return errorOrToExpected(MemoryBuffer::getFile(File, -1, false));
+return errorOrToExpected(MemoryBuffer::getFile(
+F

[Lldb-commits] [PATCH] D99331: [TESTS] Fix TestInlineStepping with ccac compiler

2021-03-25 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor edited reviewers, added: jingham; removed: LLVM.
teemperor added a comment.

Thanks for the patch!

A few general notes:

- You probably want to upload the diffs with more context (`git diff -U` or 
something like that) to get rid of the "Context not available" in the source 
changes part. That's sadly how Phabricator works. If you use arcanist to upload 
patches then I believe it does that automatically for you.
- Please don't add unrelated projects such as "LLDB test suite on simulator" 
(those are like tags that are used to group reviews that are all related to a 
specific task/goal). "LLDB" is the right project for general LLDB patches 
("LLVM" for patches that target the general LLVM part in `llvm/`).

Regarding the patch itself: From what I understand what this test (maybe 
unintentionally) tests it that when we do `step-in` we apply the 
`target.process.thread.step-avoid-regexp` setting (which will skip all `std::*` 
functions when doing a step-in from what I know). So It's not clear how to me 
how LLDB ends up stepping into the `std::string` constructor (even if `ccac` is 
using a custom standard library). Can you post some more details about the test 
failure you're setting (e.g., where you end up with the source breakpoint and 
where the step-in actually takes you).

I think this is in any case something for Jim to review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99331

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

[Lldb-commits] [PATCH] D81810: LLDB step-instruction gets stuck on jump to self

2021-03-25 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.
Herald added subscribers: JDevlieghere, pengfei.

(Marking as requested-changes to get this out of my review queue)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81810

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

[Lldb-commits] [lldb] c83cd8f - [NFC] Reordering parameters in getFile and getFileOrSTDIN

2021-03-25 Thread Abhina Sreeskantharajan via lldb-commits 

Author: Abhina Sreeskantharajan
Date: 2021-03-25T09:47:49-04:00
New Revision: c83cd8feef7eb8319131d968bb8c94fdc8dbb6a6

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

LOG: [NFC] Reordering parameters in getFile and getFileOrSTDIN

In future patches I will be setting the IsText parameter frequently so I will 
refactor the args to be in the following order. I have removed the FileSize 
parameter because it is never used.

```
  static ErrorOr>
  getFile(const Twine &Filename, bool IsText = false,
  bool RequiresNullTerminator = true, bool IsVolatile = false);

  static ErrorOr>
  getFileOrSTDIN(const Twine &Filename, bool IsText = false,
 bool RequiresNullTerminator = true);

 static ErrorOr>
 getFileAux(const Twine &Filename, uint64_t MapSize, uint64_t Offset,
bool IsText, bool RequiresNullTerminator, bool IsVolatile);

  static ErrorOr>
  getFile(const Twine &Filename, bool IsVolatile = false);
```

Reviewed By: jhenderson

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

Added: 


Modified: 
clang/lib/Tooling/JSONCompilationDatabase.cpp
clang/tools/arcmt-test/arcmt-test.cpp
lld/COFF/Driver.cpp
lld/COFF/DriverUtils.cpp
lld/ELF/InputFiles.cpp
lldb/source/Host/common/FileSystem.cpp
lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
lldb/unittests/TestingSupport/TestUtilities.cpp
llvm/include/llvm/Support/MemoryBuffer.h
llvm/lib/BinaryFormat/Magic.cpp
llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp
llvm/lib/FuzzMutate/FuzzerCLI.cpp
llvm/lib/IRReader/IRReader.cpp
llvm/lib/LTO/LTOCodeGenerator.cpp
llvm/lib/Object/Binary.cpp
llvm/lib/ProfileData/GCOV.cpp
llvm/lib/Support/MemoryBuffer.cpp
llvm/lib/TableGen/Main.cpp
llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
llvm/tools/lli/lli.cpp
llvm/tools/llvm-ar/llvm-ar.cpp
llvm/tools/llvm-cov/gcov.cpp
llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
llvm/tools/llvm-pdbutil/InputFile.cpp
llvm/tools/llvm-pdbutil/llvm-pdbutil.cpp
llvm/tools/llvm-rc/ResourceFileWriter.cpp
llvm/tools/llvm-readobj/llvm-readobj.cpp
llvm/tools/obj2yaml/obj2yaml.cpp
llvm/tools/sanstats/sanstats.cpp
llvm/utils/FileCheck/FileCheck.cpp

Removed: 




diff  --git a/clang/lib/Tooling/JSONCompilationDatabase.cpp 
b/clang/lib/Tooling/JSONCompilationDatabase.cpp
index 2d8847a7a327..97ba7e411fbb 100644
--- a/clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ b/clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -198,7 +198,7 @@ JSONCompilationDatabase::loadFromFile(StringRef FilePath,
   JSONCommandLineSyntax Syntax) {
   // Don't mmap: if we're a long-lived process, the build system may overwrite.
   llvm::ErrorOr> DatabaseBuffer =
-  llvm::MemoryBuffer::getFile(FilePath, /*FileSize=*/-1,
+  llvm::MemoryBuffer::getFile(FilePath, /*IsText=*/false,
   /*RequiresNullTerminator=*/true,
   /*IsVolatile=*/true);
   if (std::error_code Result = DatabaseBuffer.getError()) {

diff  --git a/clang/tools/arcmt-test/arcmt-test.cpp 
b/clang/tools/arcmt-test/arcmt-test.cpp
index 778587d4f111..26e123c59d59 100644
--- a/clang/tools/arcmt-test/arcmt-test.cpp
+++ b/clang/tools/arcmt-test/arcmt-test.cpp
@@ -207,15 +207,13 @@ static bool performTransformations(StringRef 
resourcesPath,
 static bool filesCompareEqual(StringRef fname1, StringRef fname2) {
   using namespace llvm;
 
-  ErrorOr> file1 = MemoryBuffer::getFile(
-  fname1, /*FileSize=*/-1, /*RequiresNullTerminator=*/true,
-  /*IsVolatile=*/false, /*IsText=*/true);
+  ErrorOr> file1 =
+  MemoryBuffer::getFile(fname1, /*IsText=*/true);
   if (!file1)
 return false;
 
-  ErrorOr> file2 = MemoryBuffer::getFile(
-  fname2, /*FileSize=*/-1, /*RequiresNullTerminator=*/true,
-  /*IsVolatile=*/false, /*IsText=*/true);
+  ErrorOr> file2 =
+  MemoryBuffer::getFile(fname2, /*IsText=*/true);
   if (!file2)
 return false;
 
@@ -244,9 +242,7 @@ static bool verifyTransformedFiles(ArrayRef 
resultFiles) {
   if (RemappingsFile.empty())
 inputBuf = MemoryBuffer::getSTDIN();
   else
-inputBuf = MemoryBuffer::getFile(RemappingsFile, /*FileSize=*/-1,
- /*RequiresNullTerminator=*/true,
- /*IsVolatile=*/false, /*IsText=*/true);
+inputBuf = MemoryBuffer::getFile(RemappingsFile, /*IsText=*/true);
   if (!inputBuf) {
 errs() << "error: could not read remappings input\n";
 return true;

diff  --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index c5c8d5c0cf90..cf96ecb731a2 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -157,9 +157,8 @@ static std::future 
create

[Lldb-commits] [PATCH] D95713: [lldb/Plugins] Add ScriptedProcess Process Plugin

2021-03-25 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:69
+
+  StructuredData::ObjectSP object_sp = GetInterface().CreatePluginObject(
+  m_launch_info.GetClassName().c_str(), target_sp,

stella.stamenova wrote:
> This is where the AV happens on Windows. It looks like GetInterface returns 
> null
Thanks @stella.stamenova for taking a closer look at this. I don't know why 
would the test call this code and crash, but I'll continue investigating it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95713

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

[Lldb-commits] [PATCH] D99331: [TESTS] Fix TestInlineStepping with ccac compiler

2021-03-25 Thread Jim Ingham via Phabricator via lldb-commits 
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

Raphael is right.  The default values of the 
target.process.thread.step-avoid-regexp are set such that if lldb finds itself 
in a function inlined from std:: it will automatically step back out.  This 
test is testing that (among other things).  The fact that the test is failing 
means that something in how your compiler is generating symbols is defeating 
this.  Maybe it's emitting the std:: symbols in such a way that the regexp 
isn't matching them?  Maybe the inline records in the DWARF are confusing lldb 
somehow?

In any case, changing the test is not the right resolution, if you are stopping 
in a std::string constructor that is a real failure.

Our test runner allow you to x-fail a test based on compiler and compiler 
version.  In fact this test is x-failed for icc because of some other 
optimizations that compiler does.  So if you're trying to get a clean run it 
would be fine to file a bug about this failure, then add an 
expectedFailureAll(compiler="ssas", bugnumber="") to the test, and 
come back and fix the behavior when you or somebody else working on this 
compiler gets around to it.  Or of course, figure out why the behavior is 
incorrect with the ssas output, and fix that, if you have the time for that now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99331

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

[Lldb-commits] [PATCH] D99315: [lldb] Support lazily named classes in the Objective-C classes

2021-03-25 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik added inline comments.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:199
+
+ClassInfo *class_infos = (ClassInfo *)class_infos_ptr;
+

Is this pointer and `realized_class_list` always non-NULL?



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:201
+
+unsigned int count = 0;
+Class* realized_class_list = objc_copyRealizedClassList(&count);

You use `unsigned int` here, `uint32_t` next and then `unsigned` in the for 
loop. We should pick one for consistency, probably `uint32_t` since it is fixed 
width.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:1585
   num_class_infos = return_value.GetScalar().ULong();
-  LLDB_LOGF(log, "Discovered %u ObjC classes\n", num_class_infos);
+  LLDB_LOGF(log, "Discovered %u Objective-C classes", num_class_infos);
   if (num_class_infos > 0) {

For `uint32_t` we should use `PRIu32` for the format.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:1867
   num_class_infos = return_value.GetScalar().ULong();
-  LLDB_LOGF(log, "Discovered %u ObjC classes in shared cache\n",
+  LLDB_LOGF(log, "Discovered %u Objective-C classes in the shared cache",
 num_class_infos);

For `uint32_t` we need to use `PRIu32`


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

https://reviews.llvm.org/D99315

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

[Lldb-commits] [PATCH] D98886: Strip pointer authentication codes from MacOSX arc pc.

2021-03-25 Thread Mark Mentovai via Phabricator via lldb-commits 
markmentovai added inline comments.



Comment at: lldb/source/Plugins/Process/minidump/MinidumpParser.cpp:241-242
+
+  const CrashpadInfo *crashpad_info =
+  reinterpret_cast(data.data());
+

Check that crashpad_info->version is 1 before attempting to treat what follows 
as pointer_authentication_address_mask.



Comment at: lldb/source/Plugins/Process/minidump/MinidumpTypes.h:113
+
+struct CrashpadInfo {
+  llvm::support::ulittle32_t version;

Ensure alignment is compatible with what Crashpad uses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98886

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

[Lldb-commits] [PATCH] D99315: [lldb] Support lazily named classes in the Objective-C classes

2021-03-25 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 94.
JDevlieghere marked 4 inline comments as done.
JDevlieghere added a comment.

Address @shafik's code review comments.


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

https://reviews.llvm.org/D99315

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h

Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -293,6 +293,45 @@
 }
   };
 
+  /// We can read the class info from the Objective-C runtime using
+  /// gdb_objc_realized_classes or objc_copyRealizedClassList. The latter is
+  /// preferred because it includes lazily named classes, but it's not always
+  /// available or safe to call.
+  ///
+  /// We potentially need both for the same process,
+  /// because we may need to use gdb_objc_realized_classes until dyld is
+  /// initialized and then switch over to objc_copyRealizedClassList for lazily
+  /// named classes.
+  class GetClassInfo {
+  public:
+enum Helper { gdb_objc_realized_classes, objc_copyRealizedClassList };
+
+UtilityFunction *GetClassInfoUtilityFunction(ExecutionContext &exe_ctx,
+ Helper helper);
+lldb::addr_t &GetClassInfoArgs(Helper helper);
+std::mutex &GetMutex() { return m_mutex; }
+
+/// Compute which helper to use. Prefer objc_copyRealizedClassList if it's
+/// available and it's safe to call (i.e. dyld is fully initialized). Use
+/// gdb_objc_realized_classes otherwise.
+static Helper ComputeHelper(AppleObjCRuntimeV2 &runtime);
+
+  private:
+std::unique_ptr
+GetClassInfoUtilityFunctionImpl(ExecutionContext &exe_ctx, std::string code,
+std::string name);
+
+std::mutex m_mutex;
+
+/// Utility function to read class info using gdb_objc_realized_classes.
+std::unique_ptr m_get_class_info_code;
+lldb::addr_t m_get_class_info_args = LLDB_INVALID_ADDRESS;
+
+/// Utility function to read class info using objc_copyRealizedClassList.
+std::unique_ptr m_get_class_info2_code;
+lldb::addr_t m_get_class_info2_args = LLDB_INVALID_ADDRESS;
+  };
+
   AppleObjCRuntimeV2(Process *process, const lldb::ModuleSP &objc_module_sp);
 
   ObjCISA GetPointerISA(ObjCISA isa);
@@ -301,6 +340,12 @@
 
   bool UpdateISAToDescriptorMapFromMemory(RemoteNXMapTable &hash_table);
 
+  /// Update the generation count of realized classes. This is not an exact
+  /// count but rather a value that is incremented when new classes are realized
+  /// or destroyed. Unlike the count in gdb_objc_realized_classes, it will
+  /// change when lazily named classes get realized.
+  bool RealizedClassGenerationCountChanged();
+
   DescriptorMapUpdateResult
   UpdateISAToDescriptorMapDynamic(RemoteNXMapTable &hash_table);
 
@@ -320,6 +365,8 @@
 
   bool GetCFBooleanValuesIfNeeded();
 
+  bool HasSymbol(ConstString Name);
+
   NonPointerISACache *GetNonPointerIsaCache() {
 if (!m_non_pointer_isa_cache_up)
   m_non_pointer_isa_cache_up.reset(
@@ -331,9 +378,7 @@
 
   lldb::ModuleSP m_objc_module_sp;
 
-  std::unique_ptr m_get_class_info_code;
-  lldb::addr_t m_get_class_info_args;
-  std::mutex m_get_class_info_args_mutex;
+  GetClassInfo m_get_class_info;
 
   std::unique_ptr m_get_shared_cache_class_info_code;
   lldb::addr_t m_get_shared_cache_class_info_args;
@@ -344,12 +389,14 @@
   lldb::addr_t m_isa_hash_table_ptr;
   HashTableSignature m_hash_signature;
   bool m_has_object_getClass;
+  bool m_has_objc_copyRealizedClassList;
   bool m_loaded_objc_opt;
   std::unique_ptr m_non_pointer_isa_cache_up;
   std::unique_ptr m_tagged_pointer_vendor_up;
   EncodingToTypeSP m_encoding_to_type_sp;
   bool m_noclasses_warning_emitted;
   llvm::Optional> m_CFBoolean_values;
+  uint64_t m_realized_class_generation_count;
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -39,6 +39,7 @@
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/ABI.h"
+#include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/Process.h"
@@ -163,6 +164,72 @@
 
 )";
 
+static const char *g_get_dynamic_class_info2_name =
+"__lldb_apple_objc_v2_get_dynamic_class_info2";
+
+static const char *g_get_dynamic_class_info2_

[Lldb-commits] [PATCH] D99315: [lldb] Support lazily named classes in the Objective-C classes

2021-03-25 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:199
+
+ClassInfo *class_infos = (ClassInfo *)class_infos_ptr;
+

shafik wrote:
> Is this pointer and `realized_class_list` always non-NULL?
Yep, LLDB allocates the space and passes the pointer to the utility function. 


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

https://reviews.llvm.org/D99315

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

[Lldb-commits] [PATCH] D98529: [lldb] Strip pointer authentication codes from aarch64 pc.

2021-03-25 Thread Justin Cohen via Phabricator via lldb-commits 
justincohen added a comment.

> OK we may need to retain the manual setting when I upstream this, instead of 
> going with the pure Process-maintained value determined dynamically by gdb 
> packet or corefile metadata.  If this is something you need for your own 
> FixCodeAddress prelim patch, I can upstream the 
> target.process.virtual-addressable-bits setting (I think the name is fine, 
> even once Process can determine this dynamically).  We'll need to decide at 
> some point what the correct behavior is when they conflict, but if only one 
> is set the choice is straightforward.

Were you able to confirm if sysctlbyname "machdep.virtual_address_size" works 
on iOS? I'm currently hard coding this information in minidump creation, as 
it's failing for me.

I uploaded https://reviews.llvm.org/D98886 which reads a mask from a minidump 
and sets target.process.pointer-authentication-address-mask.  Would you 
consider that (as I assume this is going to be converted into a mask 
regardless, and I if there's a possibility for future non-contiguous bits)

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98529

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

[Lldb-commits] [lldb] 414412d - [lldb/Commands] Fix spelling of target.move-to-nearest-code in helptext

2021-03-25 Thread Vedant Kumar via lldb-commits 

Author: Vedant Kumar
Date: 2021-03-25T14:25:10-07:00
New Revision: 414412d3dcbcf1fae0dc217085b63ab08ac41f37

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

LOG: [lldb/Commands] Fix spelling of target.move-to-nearest-code in helptext

Added: 


Modified: 
lldb/source/Commands/Options.td

Removed: 




diff  --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index c19874f73120..1cc190ebc211 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -200,7 +200,7 @@ let Command = "breakpoint set" in {
   def breakpoint_set_move_to_nearest_code : Option<"move-to-nearest-code", 
"m">,
 Groups<[1,9,12]>, Arg<"Boolean">,
 Desc<"Move breakpoints to nearest code. If not set the "
-"target.move-to-nearest-codesetting is used.">;
+"target.move-to-nearest-code setting is used.">;
   def breakpoint_set_file_colon_line : Option<"joint-specifier", "y">, 
Group<12>, Arg<"FileLineColumn">,
 Required, Completion<"SourceFile">,
 Desc<"A specifier in the form filename:line[:column] for setting file & 
line breakpoints.">;



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

[Lldb-commits] [PATCH] D98529: [lldb] Strip pointer authentication codes from aarch64 pc.

2021-03-25 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda added a comment.

In D98529#2651468 , @justincohen wrote:

> 



> Were you able to confirm if sysctlbyname "machdep.virtual_address_size" works 
> on iOS? I'm currently hard coding this information in minidump creation, as 
> it's failing for me.

Hey Justin, sorry I dropped this one.  I'll check later tonight or tomorrow.  
Sometimes our internal devices are set up a little different than customer 
devices and things can work on the internal-configure one, so I need to play 
with it a bit.

>> I uploaded https://reviews.llvm.org/D98886 which reads a mask from a 
>> minidump and sets target.process.pointer-authentication-address-mask.  Would 
>> you consider that (as I assume this is going to be converted into a mask 
>> regardless, and I if there's a possibility for future non-contiguous bits)
>
> What do you think?

Clearing PAC bits is a little more complicated than just clearing the bits, 
though.  Bit 55 tells us whether the high bits are all 0's or all 1's (on 
Darwin, in EL0 processes they're all 0's, in EL1, all 1's).  If we had a 
setting to provide a mask instead of the number of bits that are valid in 
addressing, that might lead someone to try to use it for a different purpose.  
Trying to imagine a scenario like this, maybe someone could know that a certain 
range of the address space isn't used for a certain type of pointer, and that 
they could reuse those bits as a Top Byte Ignore kind of thing, but the 
generated code would need to clear/set those bits before dereferencing, or we'd 
need a CPU with that kind of capability.  Maybe there could be examples of this 
today like the thumb bit on armv7, where the 0th bit on something with 
alignment restrictions can be used to carry metadata, although I can't think of 
anything like that on AArch/x86_64 (the only two targets I can really remember 
well these days).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98529

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

[Lldb-commits] [PATCH] D95713: [lldb/Plugins] Add ScriptedProcess Process Plugin

2021-03-25 Thread David Zarzycki via Phabricator via lldb-commits 
davezarzycki added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:69
+
+  StructuredData::ObjectSP object_sp = GetInterface().CreatePluginObject(
+  m_launch_info.GetClassName().c_str(), target_sp,

mib wrote:
> stella.stamenova wrote:
> > This is where the AV happens on Windows. It looks like GetInterface returns 
> > null
> Thanks @stella.stamenova for taking a closer look at this. I don't know why 
> would the test call this code and crash, but I'll continue investigating it.
I've seen this crash on Fedora (x86-64) too so I don't think it's Windows 
specific.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95713

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

[Lldb-commits] [PATCH] D99314: [lldb] Add IsSafeToCallAPI to DynamicLoader

2021-03-25 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.



Comment at: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp:1118
+bool DynamicLoaderMacOSXDYLD::IsSafeToCallAPI() {
+  if (ReadAllImageInfosStructure())
+return m_dyld_all_image_infos.libSystemInitialized;

Out of curiosity: Is this an error when ReadAllImageInfosStructure() returns 
false? In that case a better API might be one that also bubbles up the error.


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

https://reviews.llvm.org/D99314

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

[Lldb-commits] [PATCH] D99314: [lldb] Add IsSafeToCallAPI to DynamicLoader

2021-03-25 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp:1118
+bool DynamicLoaderMacOSXDYLD::IsSafeToCallAPI() {
+  if (ReadAllImageInfosStructure())
+return m_dyld_all_image_infos.libSystemInitialized;

aprantl wrote:
> Out of curiosity: Is this an error when ReadAllImageInfosStructure() returns 
> false? In that case a better API might be one that also bubbles up the error.
It can fail if `m_dyld_all_image_infos_addr` is not known or if we fail to read 
memory. I guess the latter could be considered an error and something we might 
want to surface, but probably not from this API. If we don't know the answer to 
whether it's safe to do so is still no. 


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

https://reviews.llvm.org/D99314

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

[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

2021-03-25 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 333441.
mgorny retitled this revision from "[lldb] [Process/Linux] Watch for fork/vfork 
notifications" to "[lldb] [Process] Watch for fork/vfork notifications".
mgorny added a comment.

Now includes initial FreeBSD support. The watchpoint test still fails, we 
probably need to clean dbregs on fork too.


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

https://reviews.llvm.org/D98822

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Host/linux/Host.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Host/linux/Host.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/test/Shell/Subprocess/Inputs/fork.c
  lldb/test/Shell/Subprocess/Inputs/vfork.c
  lldb/test/Shell/Subprocess/fork-follow-parent-softbp.test
  lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
  lldb/test/Shell/Subprocess/fork-follow-parent.test
  lldb/test/Shell/Subprocess/vfork-follow-parent-softbp.test
  lldb/test/Shell/Subprocess/vfork-follow-parent.test

Index: lldb/test/Shell/Subprocess/vfork-follow-parent.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/vfork-follow-parent.test
@@ -0,0 +1,10 @@
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/vfork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/vfork-follow-parent-softbp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/vfork-follow-parent-softbp.test
@@ -0,0 +1,11 @@
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/vfork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+b child_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/fork-follow-parent.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/fork-follow-parent.test
@@ -0,0 +1,11 @@
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/fork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK: function run in child
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
@@ -0,0 +1,13 @@
+# REQUIRES: native && (target-x86 || target-x86_64 || target-aarch64) && dbregs-set
+# RUN: %clang_host -g %p/Inputs/fork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch -s
+watchpoint set variable -w write g_val
+# CHECK: Watchpoint created:
+continue
+# CHECK: function run in child
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = watchpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/fork-follow-parent-softbp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/fork-follow-parent-softbp.test
@@ -0,0 +1,12 @@
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/fork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+b child_func
+process launch
+# CHECK: function run in child
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/Inputs/vfork.c
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/Inputs/vfork.c
@@ -0,0 +1,37 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int g_val = 0;
+
+void parent_func() {
+  g_val = 1;
+  printf("function run in parent\n");
+}
+
+void child_func() {
+  // do something relatively safe
+  volatile int val = 0;
+  ++val;
+}
+
+int main() {
+  pid_t pid = vfork();
+  assert(pid != -1);
+
+  if (pid == 0) {
+child_func();
+_exit(0);
+  }
+
+  parent_func();
+  int status;
+  pid_t waited = waitpid(pid, &status, 0);
+  assert(waited == pid);
+  assert(WIFEXITED(status));
+  printf("child exited: %d\n", WEXITSTATUS(status));
+
+  return 0;
+}
Index: lldb/test/Shell/Subprocess/Inputs/fork.c
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/Inputs/fork.c
@@ -0,0 +1,36 @@
+#include 
+#include 
+#include 
+#include 
+#include

[Lldb-commits] [PATCH] D98529: [lldb] Strip pointer authentication codes from aarch64 pc.

2021-03-25 Thread Justin Cohen via Phabricator via lldb-commits 
justincohen added a comment.



> Clearing PAC bits is a little more complicated than just clearing the bits, 
> though.  Bit 55 tells us whether the high bits are all 0's or all 1's (on 
> Darwin, in EL0 processes they're all 0's, in EL1, all 1's).  If we had a 
> setting to provide a mask instead of the number of bits that are valid in 
> addressing, that might lead someone to try to use it for a different purpose. 
>  Trying to imagine a scenario like this, maybe someone could know that a 
> certain range of the address space isn't used for a certain type of pointer, 
> and that they could reuse those bits as a Top Byte Ignore kind of thing, but 
> the generated code would need to clear/set those bits before dereferencing, 
> or we'd need a CPU with that kind of capability.  Maybe there could be 
> examples of this today like the thumb bit on armv7, where the 0th bit on 
> something with alignment restrictions can be used to carry metadata, although 
> I can't think of anything like that on AArch/x86_64 (the only two targets I 
> can really remember well these days).

Copying over a comment from pcc@ on the minidump change here: 
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2773358/5//COMMIT_MSG#15
 :
 > On Linux, the mask that you get from NT_ARM_PAC_MASK specifies which bits 
 > need to be cleared from the pointer. So to use the mask, you AND with the 
 > inverse.
 > This also has the advantage that 0 is not a special case, since you can AND 
 > with its inverse and get the same pointer back.

What if we invert the mask, and do something like instead?
`(ptr & (1ULL << 55)) ? (ptr | mask) : (ptr & ~mask);`

This should be very similar to the pseudocode here: 
https://webcache.googleusercontent.com/search?q=cache:3fCUm601caMJ:https://developer.arm.com/ja/docs/ddi0596/latest/shared-pseudocode-functions/aarch64-functionspac-pseudocode+&cd=2&hl=en&ct=clnk&gl=us


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98529

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

[Lldb-commits] [PATCH] D99315: [lldb] Support lazily named classes in the Objective-C classes

2021-03-25 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added inline comments.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:305
+  /// named classes.
+  class GetClassInfo {
+  public:

GetClassInfo is an odd name for a class. It sounds more like a function. I'm 
also not suggesting ClassInfoFactory :-p



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:309
+
+UtilityFunction *GetClassInfoUtilityFunction(ExecutionContext &exe_ctx,
+ Helper helper);

Can this return a nullptr? If not, then this should return a reference.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:317
+/// gdb_objc_realized_classes otherwise.
+static Helper ComputeHelper(AppleObjCRuntimeV2 &runtime);
+

Why does this need to be public? I thought the idea of the helper class was to 
hide the actual implementation?



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:331
+/// Utility function to read class info using objc_copyRealizedClassList.
+std::unique_ptr m_get_class_info2_code;
+lldb::addr_t m_get_class_info2_args = LLDB_INVALID_ADDRESS;

Any better naming ideas?


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

https://reviews.llvm.org/D99315

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

[Lldb-commits] [PATCH] D99314: [lldb] Add IsSafeToCallAPI to DynamicLoader

2021-03-25 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added inline comments.



Comment at: lldb/include/lldb/Target/DynamicLoader.h:260
+  /// safe to call certain APIs or SPIs.
+  virtual bool IsSafeToCallAPI() { return true; }
+

*Perhaps* IsFullyInitialized() ?


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

https://reviews.llvm.org/D99314

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

[Lldb-commits] [lldb] bbb4191 - [lldb] Add IsFullyInitialized to DynamicLoader

2021-03-25 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2021-03-25T15:44:37-07:00
New Revision: bbb419151cc8994b3447f184fe841e87e159e5a3

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

LOG: [lldb] Add IsFullyInitialized to DynamicLoader

On Darwin based systems, lldb will get notified by dyld before it itself
finished initializing, at which point it's not safe to call certain APIs
or SPIs. Add a method to the DynamicLoader to query that.

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

Added: 


Modified: 
lldb/include/lldb/Target/DynamicLoader.h
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h

Removed: 




diff  --git a/lldb/include/lldb/Target/DynamicLoader.h 
b/lldb/include/lldb/Target/DynamicLoader.h
index dead3eec70133..1e4038bb208e9 100644
--- a/lldb/include/lldb/Target/DynamicLoader.h
+++ b/lldb/include/lldb/Target/DynamicLoader.h
@@ -251,6 +251,14 @@ class DynamicLoader : public PluginInterface {
 return false;
   }
 
+  /// Return whether the dynamic loader is fully initialized and it's safe to
+  /// call its APIs.
+  ///
+  /// On some systems (e.g. Darwin based systems), lldb will get notified by
+  /// the dynamic loader before it itself finished initializing and it's not
+  /// safe to call certain APIs or SPIs.
+  virtual bool IsFullyInitialized() { return true; }
+
 protected:
   // Utility methods for derived classes
 
@@ -294,7 +302,7 @@ class DynamicLoader : public PluginInterface {
   // Read a pointer from memory at the given addr. Return LLDB_INVALID_ADDRESS
   // if the read fails.
   lldb::addr_t ReadPointer(lldb::addr_t addr);
-  
+
   // Calls into the Process protected method LoadOperatingSystemPlugin:
   void LoadOperatingSystemPlugin(bool flush);
 

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
index 5cc6a6475d12e..6872dd72efad8 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
@@ -1114,6 +1114,12 @@ bool DynamicLoaderMacOSXDYLD::GetSharedCacheInformation(
   return false;
 }
 
+bool DynamicLoaderMacOSXDYLD::IsFullyInitialized() {
+  if (ReadAllImageInfosStructure())
+return m_dyld_all_image_infos.libSystemInitialized;
+  return false;
+}
+
 void DynamicLoaderMacOSXDYLD::Initialize() {
   PluginManager::RegisterPlugin(GetPluginNameStatic(),
 GetPluginDescriptionStatic(), CreateInstance);

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
index 21bf5875dc13e..b3cb3e50fa30f 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
@@ -68,6 +68,8 @@ class DynamicLoaderMacOSXDYLD : public 
lldb_private::DynamicLoaderDarwin {
 
   uint32_t GetPluginVersion() override;
 
+  bool IsFullyInitialized() override;
+
 protected:
   void PutToLog(lldb_private::Log *log) const;
 



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

[Lldb-commits] [PATCH] D99314: [lldb] Add IsSafeToCallAPI to DynamicLoader

2021-03-25 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbbb419151cc8: [lldb] Add IsFullyInitialized to DynamicLoader 
(authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D99314?vs=333192&id=333443#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99314

Files:
  lldb/include/lldb/Target/DynamicLoader.h
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h


Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
===
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
@@ -68,6 +68,8 @@
 
   uint32_t GetPluginVersion() override;
 
+  bool IsFullyInitialized() override;
+
 protected:
   void PutToLog(lldb_private::Log *log) const;
 
Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
@@ -1114,6 +1114,12 @@
   return false;
 }
 
+bool DynamicLoaderMacOSXDYLD::IsFullyInitialized() {
+  if (ReadAllImageInfosStructure())
+return m_dyld_all_image_infos.libSystemInitialized;
+  return false;
+}
+
 void DynamicLoaderMacOSXDYLD::Initialize() {
   PluginManager::RegisterPlugin(GetPluginNameStatic(),
 GetPluginDescriptionStatic(), CreateInstance);
Index: lldb/include/lldb/Target/DynamicLoader.h
===
--- lldb/include/lldb/Target/DynamicLoader.h
+++ lldb/include/lldb/Target/DynamicLoader.h
@@ -251,6 +251,14 @@
 return false;
   }
 
+  /// Return whether the dynamic loader is fully initialized and it's safe to
+  /// call its APIs.
+  ///
+  /// On some systems (e.g. Darwin based systems), lldb will get notified by
+  /// the dynamic loader before it itself finished initializing and it's not
+  /// safe to call certain APIs or SPIs.
+  virtual bool IsFullyInitialized() { return true; }
+
 protected:
   // Utility methods for derived classes
 
@@ -294,7 +302,7 @@
   // Read a pointer from memory at the given addr. Return LLDB_INVALID_ADDRESS
   // if the read fails.
   lldb::addr_t ReadPointer(lldb::addr_t addr);
-  
+
   // Calls into the Process protected method LoadOperatingSystemPlugin:
   void LoadOperatingSystemPlugin(bool flush);
 


Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
===
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
@@ -68,6 +68,8 @@
 
   uint32_t GetPluginVersion() override;
 
+  bool IsFullyInitialized() override;
+
 protected:
   void PutToLog(lldb_private::Log *log) const;
 
Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
@@ -1114,6 +1114,12 @@
   return false;
 }
 
+bool DynamicLoaderMacOSXDYLD::IsFullyInitialized() {
+  if (ReadAllImageInfosStructure())
+return m_dyld_all_image_infos.libSystemInitialized;
+  return false;
+}
+
 void DynamicLoaderMacOSXDYLD::Initialize() {
   PluginManager::RegisterPlugin(GetPluginNameStatic(),
 GetPluginDescriptionStatic(), CreateInstance);
Index: lldb/include/lldb/Target/DynamicLoader.h
===
--- lldb/include/lldb/Target/DynamicLoader.h
+++ lldb/include/lldb/Target/DynamicLoader.h
@@ -251,6 +251,14 @@
 return false;
   }
 
+  /// Return whether the dynamic loader is fully initialized and it's safe to
+  /// call its APIs.
+  ///
+  /// On some systems (e.g. Darwin based systems), lldb will get notified by
+  /// the dynamic loader before it itself finished initializing and it's not
+  /// safe to call certain APIs or SPIs.
+  virtual bool IsFullyInitialized() { return true; }
+
 protected:
   // Utility methods for derived classes
 
@@ -294,7 +302,7 @@
   // Read a pointer from memory at the given addr. Return LLDB_INVALID_ADDRESS
   // if the read fails.
   lldb::addr_t ReadPointer(lldb::addr_t addr);
-  
+
   // Calls into the Process protected method LoadOperatingSystemPlugin:
   void LoadOperatingSystemPlugin(bool flush);
 
_

[Lldb-commits] [lldb] c315253 - [LLDB] Skip TestVSCode_launch.test_progress_events arm/linux

2021-03-25 Thread Muhammad Omair Javaid via lldb-commits 

Author: Muhammad Omair Javaid
Date: 2021-03-26T04:38:31+05:00
New Revision: c3152536fda152b0956a0865059be8517697c921

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

LOG: [LLDB] Skip TestVSCode_launch.test_progress_events arm/linux

TestVSCode_launch.test_progress_events is mysteriously failing on arm
linux. I am marking it skipped for the buildbot while looking into
failure.

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py 
b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
index 1bd16f481948..3cfebb689a5b 100644
--- a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -454,6 +454,7 @@ def test_terminate_commands(self):
 
 @skipIfWindows
 @skipIfRemote
+@skipIf(oslist=["linux"], archs=["arm"])
 def test_progress_events(self):
 '''
 Tests the progress events to ensure we are receiving them.



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

[Lldb-commits] [PATCH] D99315: [lldb] Support lazily named classes in the Objective-C classes

2021-03-25 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere marked 4 inline comments as done.
JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:305
+  /// named classes.
+  class GetClassInfo {
+  public:

aprantl wrote:
> GetClassInfo is an odd name for a class. It sounds more like a function. I'm 
> also not suggesting ClassInfoFactory :-p
I went with `DynamicClassInfoExtractor`. I'm planning on moving the 
corresponding 3 instance variables for the shared cache into a 
`SharedCacheClassInfoExtractor` in a follow up patch. 



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:309
+
+UtilityFunction *GetClassInfoUtilityFunction(ExecutionContext &exe_ctx,
+ Helper helper);

aprantl wrote:
> Can this return a nullptr? If not, then this should return a reference.
Yes, if we fail to create the utility function, we'll log the error and return 
a nullptr. 



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:317
+/// gdb_objc_realized_classes otherwise.
+static Helper ComputeHelper(AppleObjCRuntimeV2 &runtime);
+

aprantl wrote:
> Why does this need to be public? I thought the idea of the helper class was 
> to hide the actual implementation?
We need to make sure we stick to one way of updating the class info. At some 
point I want to move most of `UpdateISAToDescriptorMapDynamic` into this class, 
at which this no longer needs to be private. For this patch I tried to keep 
unrelated refactorings to a minimum. 



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:331
+/// Utility function to read class info using objc_copyRealizedClassList.
+std::unique_ptr m_get_class_info2_code;
+lldb::addr_t m_get_class_info2_args = LLDB_INVALID_ADDRESS;

aprantl wrote:
> Any better naming ideas?
Not proud of this, but I found that `gdb_objc_realized_classes` and 
`objc_copyRealizedClassList` are looked so similar that it was choosing between 
the pest and the cholera. 


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

https://reviews.llvm.org/D99315

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

[Lldb-commits] [PATCH] D99315: [lldb] Support lazily named classes in the Objective-C classes

2021-03-25 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 333464.
JDevlieghere marked 4 inline comments as done.
JDevlieghere added a comment.

Address @aprantl's comments.


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

https://reviews.llvm.org/D99315

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h

Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -293,6 +293,50 @@
 }
   };
 
+  /// We can read the class info from the Objective-C runtime using
+  /// gdb_objc_realized_classes or objc_copyRealizedClassList. The latter is
+  /// preferred because it includes lazily named classes, but it's not always
+  /// available or safe to call.
+  ///
+  /// We potentially need both for the same process,
+  /// because we may need to use gdb_objc_realized_classes until dyld is
+  /// initialized and then switch over to objc_copyRealizedClassList for lazily
+  /// named classes.
+  class DynamicClassInfoExtractor {
+  public:
+DynamicClassInfoExtractor(AppleObjCRuntimeV2 &runtime)
+: m_runtime(runtime) {}
+
+enum Helper { gdb_objc_realized_classes, objc_copyRealizedClassList };
+
+/// Compute which helper to use. Prefer objc_copyRealizedClassList if it's
+/// available and it's safe to call (i.e. dyld is fully initialized). Use
+/// gdb_objc_realized_classes otherwise.
+Helper ComputeHelper() const;
+
+UtilityFunction *GetClassInfoUtilityFunction(ExecutionContext &exe_ctx,
+ Helper helper);
+lldb::addr_t &GetClassInfoArgs(Helper helper);
+std::mutex &GetMutex() { return m_mutex; }
+
+  private:
+std::unique_ptr
+GetClassInfoUtilityFunctionImpl(ExecutionContext &exe_ctx, std::string code,
+std::string name);
+
+/// The lifetime of this object is tied to that of the runtime.
+AppleObjCRuntimeV2 &m_runtime;
+std::mutex m_mutex;
+
+/// Utility function to read class info using gdb_objc_realized_classes.
+std::unique_ptr m_get_class_info_code;
+lldb::addr_t m_get_class_info_args = LLDB_INVALID_ADDRESS;
+
+/// Utility function to read class info using objc_copyRealizedClassList.
+std::unique_ptr m_get_class_info2_code;
+lldb::addr_t m_get_class_info2_args = LLDB_INVALID_ADDRESS;
+  };
+
   AppleObjCRuntimeV2(Process *process, const lldb::ModuleSP &objc_module_sp);
 
   ObjCISA GetPointerISA(ObjCISA isa);
@@ -301,6 +345,12 @@
 
   bool UpdateISAToDescriptorMapFromMemory(RemoteNXMapTable &hash_table);
 
+  /// Update the generation count of realized classes. This is not an exact
+  /// count but rather a value that is incremented when new classes are realized
+  /// or destroyed. Unlike the count in gdb_objc_realized_classes, it will
+  /// change when lazily named classes get realized.
+  bool RealizedClassGenerationCountChanged();
+
   DescriptorMapUpdateResult
   UpdateISAToDescriptorMapDynamic(RemoteNXMapTable &hash_table);
 
@@ -320,6 +370,8 @@
 
   bool GetCFBooleanValuesIfNeeded();
 
+  bool HasSymbol(ConstString Name);
+
   NonPointerISACache *GetNonPointerIsaCache() {
 if (!m_non_pointer_isa_cache_up)
   m_non_pointer_isa_cache_up.reset(
@@ -331,9 +383,7 @@
 
   lldb::ModuleSP m_objc_module_sp;
 
-  std::unique_ptr m_get_class_info_code;
-  lldb::addr_t m_get_class_info_args;
-  std::mutex m_get_class_info_args_mutex;
+  DynamicClassInfoExtractor m_class_info_extractor;
 
   std::unique_ptr m_get_shared_cache_class_info_code;
   lldb::addr_t m_get_shared_cache_class_info_args;
@@ -344,12 +394,14 @@
   lldb::addr_t m_isa_hash_table_ptr;
   HashTableSignature m_hash_signature;
   bool m_has_object_getClass;
+  bool m_has_objc_copyRealizedClassList;
   bool m_loaded_objc_opt;
   std::unique_ptr m_non_pointer_isa_cache_up;
   std::unique_ptr m_tagged_pointer_vendor_up;
   EncodingToTypeSP m_encoding_to_type_sp;
   bool m_noclasses_warning_emitted;
   llvm::Optional> m_CFBoolean_values;
+  uint64_t m_realized_class_generation_count;
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -39,6 +39,7 @@
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/ABI.h"
+#include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/Pr