[Lldb-commits] [lldb] 8b656b8 - [lldb] Re-eanble and rewrite TestCPPStaticMembers

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-05-25T11:52:28+02:00
New Revision: 8b656b88462f51396c8c4772e0012549f76f204f

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

LOG: [lldb] Re-eanble and rewrite TestCPPStaticMembers

It's not clear why the whole test got disabled, but the linked bug report
has since been fixed and the only part of it that still fails is the test
for the too permissive lookup. This re-enables the test, rewrites it to use
the modern test functions we have and splits the failing part into its
own test that we can skip without disabling the rest.

Added: 


Modified: 
lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
lldb/test/API/lang/cpp/static_members/main.cpp

Removed: 




diff  --git a/lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py 
b/lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
index 301f3bc10269a..8acf498bb7f74 100644
--- a/lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
+++ b/lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
@@ -11,51 +11,30 @@
 from lldbsuite.test import lldbutil
 
 
-class CPPStaticMembersTestCase(TestBase):
+class TestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
-@expectedFailure  # llvm.org/pr15401
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr21765")
-def test_with_run_command(self):
-"""Test that member variables have the correct layout, scope and 
qualifiers when stopped inside and outside C++ methods"""
+def test_access_from_main(self):
 self.build()
-self.runCmd("file " + self.getBuildArtifact("a.out"), 
CURRENT_EXECUTABLE_SET)
+lldbutil.run_to_source_breakpoint(self, "// stop in main", 
lldb.SBFileSpec("main.cpp"))
 
-self.set_breakpoint(line_number('main.cpp', '// breakpoint 1'))
-self.set_breakpoint(line_number('main.cpp', '// breakpoint 2'))
+self.expect_expr("my_a.m_a", result_type="short", result_value="1")
+self.expect_expr("my_a.s_b", result_type="long", result_value="2")
+self.expect_expr("my_a.s_c", result_type="int", result_value="3")
 
-self.runCmd("process launch", RUN_SUCCEEDED)
-self.expect("expression my_a.access()",
-startstr="(long) $0 = 10")
-
-self.expect("expression my_a.m_a",
-startstr="(short) $1 = 1")
-
-# Note: SymbolFileDWARF::ParseChildMembers doesn't call
-# AddFieldToRecordType, consistent with clang's AST layout.
-self.expect("expression my_a.s_d",
-startstr="(int) $2 = 4")
-
-self.expect("expression my_a.s_b",
-startstr="(long) $3 = 2")
-
-self.expect("expression A::s_b",
-startstr="(long) $4 = 2")
-
-# should not be available in global scope
-self.expect("expression s_d",
+def test_access_from_member_function(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// stop in member function", 
lldb.SBFileSpec("main.cpp"))
+self.expect_expr("m_a", result_type="short", result_value="1")
+self.expect_expr("s_b", result_type="long", result_value="2")
+self.expect_expr("s_c", result_type="int", result_value="3")
+
+# Currently lookups find variables that are in any scope.
+@expectedFailureAll()
+def test_access_without_scope(self):
+self.build()
+self.createTestTarget()
+self.expect("expression s_c", error=True,
 startstr="error: use of undeclared identifier 's_d'")
-
-self.runCmd("process continue")
-self.expect("expression m_c",
-startstr="(char) $5 = \'\\x03\'")
-
-self.expect("expression s_b",
-startstr="(long) $6 = 2")
-
-self.runCmd("process continue")
-
-def set_breakpoint(self, line):
-lldbutil.run_break_set_by_file_and_line(
-self, "main.cpp", line, num_expected_locations=1, loc_exact=False)

diff  --git a/lldb/test/API/lang/cpp/static_members/main.cpp 
b/lldb/test/API/lang/cpp/static_members/main.cpp
index 1503ec6e0ebf6..87ea6aa877472 100644
--- a/lldb/test/API/lang/cpp/static_members/main.cpp
+++ b/lldb/test/API/lang/cpp/static_members/main.cpp
@@ -1,25 +1,20 @@
-struct A
-{
-short m_a;
-static long s_b;
-char m_c;
-static int s_d;
+struct A {
+  short m_a;
+  static long s_b;
+  static int s_c;
 
-long access() {
-return m_a + s_b + m_c + s_d; // breakpoint 2
-}
+  long access() {
+return m_a + s_b + s_c; // stop in member function
+  }
 };
 
 long A::s_b = 2;
-int A::s_d = 4;
+int A::s_c = 3;
 
-int main()
-{
-A my_a;
-my_a.m_a = 1;
-my_a.m_c = 3;
+int main() {
+  A my_a;

[Lldb-commits] [lldb] 3bf96b0 - [lldb] Disable minimal import mode for RecordDecls that back FieldDecls

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-05-25T12:08:50+02:00
New Revision: 3bf96b0329be554c67282b0d7d8da6a864b9e38f

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

LOG: [lldb] Disable minimal import mode for RecordDecls that back FieldDecls

Clang adds a Decl in two phases to a DeclContext. First it adds it invisible and
then it makes it visible (which will add it to the lookup data structures). It's
important that we can't do lookups into the DeclContext we are currently adding
the Decl to during this process as once the Decl has been added, any lookup will
automatically build a new lookup map and add the added Decl to it. The second
step would then add the Decl a second time to the lookup which will lead to
weird errors later one. I made adding a Decl twice to a lookup an assertion
error in D84827.

In the first step Clang also does some computations on the added Decl if it's
for example a FieldDecl that is added to a RecordDecl.

One of these computations is checking if the FieldDecl is of a record type
and the record type has a deleted constexpr destructor which will delete
the constexpr destructor of the record that got the FieldDecl.

This can lead to a bug with the way we implement MinimalImport in LLDB
and the following code:

```
struct Outer {
  typedef int HookToOuter;
  struct NestedClass {
HookToOuter RefToOuter;
  } NestedClassMember; // We are adding this.
};
```

1. We just imported `Outer` minimally so far.
2. We are now asked to add `NestedClassMember` as a FieldDecl.
3. We import `NestedClass` minimally.
4. We add `NestedClassMember` and clang does a lookup for a constexpr dtor in
   `NestedClass`. `NestedClassMember` hasn't been added to the lookup.
5. The lookup into `NestedClass` will now load the members of `NestedClass`.
6. We try to import the type of `RefToOuter` which will try to import the 
`HookToOuter` typedef.
7. We import the typedef and while importing we check for conflicts in `Outer` 
via a lookup.
8. The lookup into `Outer` will cause the invisible `NestedClassMember` to be 
added to the lookup.
9. We continue normally until we get back to the `addDecl` call in step 2.
10. We now add `NestedClassMember` to the lookup even though we already did 
that in step 8.

The fix here is disabling the minimal import for RecordTypes from FieldDecls. We
actually already did this, but so far we only force the definition of the type
to be imported *after* we imported the FieldDecl. This just moves that code
*before* we import the FieldDecl so prevent the issue above.

Reviewed By: shafik, aprantl

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

Added: 
lldb/test/API/lang/cpp/reference-to-outer-type/Makefile

lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
lldb/test/API/lang/cpp/reference-to-outer-type/main.cpp

Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
index ad72f01f4060c..d3f471301d1b3 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -888,6 +888,37 @@ ClangASTImporter::ASTImporterDelegate::ImportImpl(Decl 
*From) {
 LLDB_LOG(log, "[ClangASTImporter] Complete definition not found");
   }
 
+  // Disable the minimal import for fields that have record types. There is
+  // no point in minimally importing the record behind their type as Clang
+  // will anyway request their definition when the FieldDecl is added to the
+  // RecordDecl (as Clang will query the FieldDecl's type for things such
+  // as a deleted constexpr destructor).
+  // By importing the type ahead of time we avoid some corner cases where
+  // the FieldDecl's record is importing in the middle of Clang's
+  // `DeclContext::addDecl` logic.
+  if (clang::FieldDecl *fd = dyn_cast(From)) {
+// This is only necessary because we do the 'minimal import'. Remove this
+// once LLDB stopped using that mode.
+assert(isMinimalImport() && "Only necessary for minimal import");
+QualType field_type = fd->getType();
+if (field_type->isRecordType()) {
+  // First get the underlying record and minimally import it.
+  clang::TagDecl *record_decl = field_type->getAsTagDecl();
+  llvm::Expected imported = Import(record_decl);
+  if (!imported)
+return imported.takeError();
+  // Check how/if the import got redirected to a 
diff erent AST. Now
+  // import the definition of what was actually imported. If there is no
+  // origin then that means the rec

[Lldb-commits] [PATCH] D102993: [lldb] Disable minimal import mode for RecordDecls that back FieldDecls

[Raphael Isemann 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 rG3bf96b0329be: [lldb] Disable minimal import mode for 
RecordDecls that back FieldDecls (authored by teemperor).
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D102993?vs=347274&id=347618#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102993

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/test/API/lang/cpp/reference-to-outer-type/Makefile
  lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
  lldb/test/API/lang/cpp/reference-to-outer-type/main.cpp

Index: lldb/test/API/lang/cpp/reference-to-outer-type/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/reference-to-outer-type/main.cpp
@@ -0,0 +1,23 @@
+struct Outer {
+  typedef int HookToOuter;
+  // When importing this type, we have to import all of it before adding it
+  // via the FieldDecl to 'Outer'. If we don't do this, then Clang will
+  // while adding 'NestedClassMember' ask for the full type of 'NestedClass'
+  // which then will start pulling in the 'RefToOuter' member. That member
+  // will import the typedef above and add it to 'Outer'. And adding a
+  // Decl to a DeclContext that is currently already in the process of adding
+  // another Decl will cause an inconsistent lookup.
+  struct NestedClass {
+HookToOuter RefToOuter;
+int SomeMember;
+  } NestedClassMember;
+};
+
+// We query the members of base classes of a type by doing a lookup via Clang.
+// As this tests is trying to find a borked lookup, we need a base class here
+// to make our 'GetChildMemberWithName' use the Clang lookup.
+struct In : Outer {};
+
+In test_var;
+
+int main() { return test_var.NestedClassMember.SomeMember; }
Index: lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/reference-to-outer-type/TestCppReferenceToOuterClass.py
@@ -0,0 +1,16 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+test_var = self.expect_expr("test_var", result_type="In")
+nested_member = test_var.GetChildMemberWithName('NestedClassMember')
+self.assertEqual("Outer::NestedClass",
+ nested_member.GetType().GetName())
Index: lldb/test/API/lang/cpp/reference-to-outer-type/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/reference-to-outer-type/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -479,10 +479,7 @@
decl->getDeclKindName(), ast_dump);
   }
 
-  Decl *copied_decl = CopyDecl(decl);
-
-  if (!copied_decl)
-continue;
+  CopyDecl(decl);
 
   // FIXME: We should add the copied decl to the 'decls' list. This would
   // add the copied Decl into the DeclContext and make sure that we
@@ -492,12 +489,6 @@
   // lookup issues later on.
   // We can't just add them for now as the ASTImporter already added the
   // decl into the DeclContext and this would add it twice.
-
-  if (FieldDecl *copied_field = dyn_cast(copied_decl)) {
-QualType copied_field_type = copied_field->getType();
-
-m_ast_importer_sp->RequireCompleteType(copied_field_type);
-  }
 } else {
   SkippedDecls = true;
 }
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -888,6 +888,37 @@
 LLDB_LOG(log, "[ClangASTImporter] Complete definition not found");
   }
 
+  // Disable the minimal import for fields that have record types. There is
+  // no point in minimally importing the record behind their type as Clang
+  // will anyway request their definition when the FieldDecl is added to the
+  // RecordDecl (as Clang will query the FieldDecl's type for things such
+  // as a deleted constexpr destructor).
+  // By importing the t

[Lldb-commits] [lldb] a3a9528 - [lldb] X-FAIL TestCPPStaticMembers on Windows

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-05-25T13:10:19+02:00
New Revision: a3a95286a73fddc4027de930fac29728cd4259fc

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

LOG: [lldb] X-FAIL TestCPPStaticMembers on Windows

This was originally failed because of llvm.org/pr21765 which describes that
LLDB can't call a debugee's functions, but I removed the (unnecessary)
function call in the rewrite. It seems that the actual bug here is that we
can't lookup static members at all, so let's X-FAIL the test for the right
reason.

Added: 


Modified: 
lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py

Removed: 




diff  --git a/lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py 
b/lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
index 8acf498bb7f7..07ec5c52ea4c 100644
--- a/lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
+++ b/lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
@@ -15,7 +15,8 @@ class TestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr21765")
+# We fail to lookup static members on Windows.
+@expectedFailureAll(oslist=["windows"])
 def test_access_from_main(self):
 self.build()
 lldbutil.run_to_source_breakpoint(self, "// stop in main", 
lldb.SBFileSpec("main.cpp"))
@@ -24,6 +25,8 @@ def test_access_from_main(self):
 self.expect_expr("my_a.s_b", result_type="long", result_value="2")
 self.expect_expr("my_a.s_c", result_type="int", result_value="3")
 
+# We fail to lookup static members on Windows.
+@expectedFailureAll(oslist=["windows"])
 def test_access_from_member_function(self):
 self.build()
 lldbutil.run_to_source_breakpoint(self, "// stop in member function", 
lldb.SBFileSpec("main.cpp"))



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

[Lldb-commits] [lldb] 1dee479 - [lldb][NFC] Remove misleading ModulePass base class for IRForTarget

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-05-25T13:27:07+02:00
New Revision: 1dee479ff632ef841ca7b28485779d898dd15e84

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

LOG: [lldb][NFC] Remove misleading ModulePass base class for IRForTarget

IRForTarget is never used by a pass manager or any other interface that requires
this class to inherit from `Pass`.

Also IRForTarget doesn't implement the current interface correctly because it
uses the `runOnModule` return value to indicate success/failure instead of
changed/not-changed, so if this ever ends up being used as a pass it would most
likely not work as intended.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index 0173c8f263a5..bb69471841fe 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -41,8 +41,6 @@
 
 using namespace llvm;
 
-static char ID;
-
 typedef SmallVector InstrList;
 
 IRForTarget::FunctionValueCache::FunctionValueCache(Maker const &maker)
@@ -72,7 +70,7 @@ IRForTarget::IRForTarget(lldb_private::ClangExpressionDeclMap 
*decl_map,
  lldb_private::IRExecutionUnit &execution_unit,
  lldb_private::Stream &error_stream,
  const char *func_name)
-: ModulePass(ID), m_resolve_vars(resolve_vars), m_func_name(func_name),
+: m_resolve_vars(resolve_vars), m_func_name(func_name),
   m_decl_map(decl_map), m_error_stream(error_stream),
   m_execution_unit(execution_unit),
   m_entry_instruction_finder(FindEntryInstruction) {}
@@ -101,8 +99,6 @@ static std::string PrintType(const llvm::Type *type, bool 
truncate = false) {
   return s;
 }
 
-IRForTarget::~IRForTarget() {}
-
 bool IRForTarget::FixFunctionLinkage(llvm::Function &llvm_function) {
   llvm_function.setLinkage(GlobalValue::ExternalLinkage);
 
@@ -2018,10 +2014,3 @@ bool IRForTarget::runOnModule(Module &llvm_module) {
 
   return true;
 }
-
-void IRForTarget::assignPassManager(PMStack &pass_mgr_stack,
-PassManagerType pass_mgr_type) {}
-
-PassManagerType IRForTarget::getPotentialPassManagerType() const {
-  return PMT_ModulePassManager;
-}

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h 
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
index 6ff50ec5f645..5f212fa8f918 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
@@ -58,7 +58,7 @@ class IRMemoryMap;
 /// transformations to the IR which make it relocatable.  These
 /// transformations are discussed in more detail next to their relevant
 /// functions.
-class IRForTarget : public llvm::ModulePass {
+class IRForTarget {
 public:
   enum class LookupResult { Success, Fail, Ignore };
 
@@ -87,9 +87,6 @@ class IRForTarget : public llvm::ModulePass {
   lldb_private::Stream &error_stream,
   const char *func_name = "$__lldb_expr");
 
-  /// Destructor
-  ~IRForTarget() override;
-
   /// Run this IR transformer on a single module
   ///
   /// Implementation of the llvm::ModulePass::runOnModule() function.
@@ -101,20 +98,7 @@ class IRForTarget : public llvm::ModulePass {
   ///
   /// \return
   /// True on success; false otherwise
-  bool runOnModule(llvm::Module &llvm_module) override;
-
-  /// Interface stub
-  ///
-  /// Implementation of the llvm::ModulePass::assignPassManager() function.
-  void assignPassManager(llvm::PMStack &pass_mgr_stack,
- llvm::PassManagerType pass_mgr_type =
- llvm::PMT_ModulePassManager) override;
-
-  /// Returns PMT_ModulePassManager
-  ///
-  /// Implementation of the llvm::ModulePass::getPotentialPassManagerType()
-  /// function.
-  llvm::PassManagerType getPotentialPassManagerType() const override;
+  bool runOnModule(llvm::Module &llvm_module);
 
 private:
   /// Ensures that the current function's linkage is set to external.



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

[Lldb-commits] [PATCH] D102677: [lldb][NFC] Remove misleading ModulePass base class for IRForTarget

[Raphael Isemann 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 rG1dee479ff632: [lldb][NFC] Remove misleading ModulePass base 
class for IRForTarget (authored by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102677

Files:
  lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
  lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h


Index: lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
===
--- lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
+++ lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
@@ -58,7 +58,7 @@
 /// transformations to the IR which make it relocatable.  These
 /// transformations are discussed in more detail next to their relevant
 /// functions.
-class IRForTarget : public llvm::ModulePass {
+class IRForTarget {
 public:
   enum class LookupResult { Success, Fail, Ignore };
 
@@ -87,9 +87,6 @@
   lldb_private::Stream &error_stream,
   const char *func_name = "$__lldb_expr");
 
-  /// Destructor
-  ~IRForTarget() override;
-
   /// Run this IR transformer on a single module
   ///
   /// Implementation of the llvm::ModulePass::runOnModule() function.
@@ -101,20 +98,7 @@
   ///
   /// \return
   /// True on success; false otherwise
-  bool runOnModule(llvm::Module &llvm_module) override;
-
-  /// Interface stub
-  ///
-  /// Implementation of the llvm::ModulePass::assignPassManager() function.
-  void assignPassManager(llvm::PMStack &pass_mgr_stack,
- llvm::PassManagerType pass_mgr_type =
- llvm::PMT_ModulePassManager) override;
-
-  /// Returns PMT_ModulePassManager
-  ///
-  /// Implementation of the llvm::ModulePass::getPotentialPassManagerType()
-  /// function.
-  llvm::PassManagerType getPotentialPassManagerType() const override;
+  bool runOnModule(llvm::Module &llvm_module);
 
 private:
   /// Ensures that the current function's linkage is set to external.
Index: lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -41,8 +41,6 @@
 
 using namespace llvm;
 
-static char ID;
-
 typedef SmallVector InstrList;
 
 IRForTarget::FunctionValueCache::FunctionValueCache(Maker const &maker)
@@ -72,7 +70,7 @@
  lldb_private::IRExecutionUnit &execution_unit,
  lldb_private::Stream &error_stream,
  const char *func_name)
-: ModulePass(ID), m_resolve_vars(resolve_vars), m_func_name(func_name),
+: m_resolve_vars(resolve_vars), m_func_name(func_name),
   m_decl_map(decl_map), m_error_stream(error_stream),
   m_execution_unit(execution_unit),
   m_entry_instruction_finder(FindEntryInstruction) {}
@@ -101,8 +99,6 @@
   return s;
 }
 
-IRForTarget::~IRForTarget() {}
-
 bool IRForTarget::FixFunctionLinkage(llvm::Function &llvm_function) {
   llvm_function.setLinkage(GlobalValue::ExternalLinkage);
 
@@ -2018,10 +2014,3 @@
 
   return true;
 }
-
-void IRForTarget::assignPassManager(PMStack &pass_mgr_stack,
-PassManagerType pass_mgr_type) {}
-
-PassManagerType IRForTarget::getPotentialPassManagerType() const {
-  return PMT_ModulePassManager;
-}


Index: lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
===
--- lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
+++ lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
@@ -58,7 +58,7 @@
 /// transformations to the IR which make it relocatable.  These
 /// transformations are discussed in more detail next to their relevant
 /// functions.
-class IRForTarget : public llvm::ModulePass {
+class IRForTarget {
 public:
   enum class LookupResult { Success, Fail, Ignore };
 
@@ -87,9 +87,6 @@
   lldb_private::Stream &error_stream,
   const char *func_name = "$__lldb_expr");
 
-  /// Destructor
-  ~IRForTarget() override;
-
   /// Run this IR transformer on a single module
   ///
   /// Implementation of the llvm::ModulePass::runOnModule() function.
@@ -101,20 +98,7 @@
   ///
   /// \return
   /// True on success; false otherwise
-  bool runOnModule(llvm::Module &llvm_module) override;
-
-  /// Interface stub
-  ///
-  /// Implementation of the llvm::ModulePass::assignPassManager() function.
-  void assignPassManager(llvm::PMStack &pass_mgr_stack,
- llvm::PassManagerType pass_mgr_type =
- llvm::PMT_ModulePassManager) override;
-
-  /// Returns PMT_ModulePassManager
-  ///
-  /// Implementa

[Lldb-commits] [PATCH] D102845: [lldb] Fix that LLDB doesn't print NaN's sign on Darwin

[Raphael Isemann 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 rGae58cf5f45a9: [lldb] Fix that LLDB doesn't print 
NaN's sign on Darwin (authored by teemperor).
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D102845?vs=346744&id=347635#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102845

Files:
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/unittests/Core/DumpDataExtractorTest.cpp

Index: lldb/unittests/Core/DumpDataExtractorTest.cpp
===
--- lldb/unittests/Core/DumpDataExtractorTest.cpp
+++ lldb/unittests/Core/DumpDataExtractorTest.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Utility/StreamString.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -197,13 +198,34 @@
lldb::Format::eFormatVectorOfFloat16, "{6.10352e-05 65504}");
   TestDump(std::vector{0xabcd, 0x1234},
lldb::Format::eFormatVectorOfFloat16, "{-0.0609436 0.000757217}");
+
+  // quiet/signaling NaNs.
+  TestDump(std::vector{0x, 0xffc0, 0x7fff, 0x7fc0},
+   lldb::Format::eFormatVectorOfFloat16, "{-nan -nan nan nan}");
+  // +/-Inf.
+  TestDump(std::vector{0xfc00, 0x7c00},
+   lldb::Format::eFormatVectorOfFloat16, "{-inf inf}");
+
   TestDump(std::vector{std::numeric_limits::min(),
   std::numeric_limits::max()},
lldb::Format::eFormatVectorOfFloat32, "{1.17549e-38 3.40282e+38}");
+  TestDump(std::vector{std::numeric_limits::quiet_NaN(),
+  std::numeric_limits::signaling_NaN(),
+  -std::numeric_limits::quiet_NaN(),
+  -std::numeric_limits::signaling_NaN()},
+   lldb::Format::eFormatVectorOfFloat32, "{nan nan -nan -nan}");
   TestDump(std::vector{std::numeric_limits::min(),
std::numeric_limits::max()},
lldb::Format::eFormatVectorOfFloat64,
"{2.2250738585072e-308 1.79769313486232e+308}");
+  TestDump(
+  std::vector{
+  std::numeric_limits::quiet_NaN(),
+  std::numeric_limits::signaling_NaN(),
+  -std::numeric_limits::quiet_NaN(),
+  -std::numeric_limits::signaling_NaN(),
+  },
+  lldb::Format::eFormatVectorOfFloat64, "{nan nan -nan -nan}");
 
   // Not sure we can rely on having uint128_t everywhere so emulate with
   // uint64_t.
Index: lldb/source/Core/DumpDataExtractor.cpp
===
--- lldb/source/Core/DumpDataExtractor.cpp
+++ lldb/source/Core/DumpDataExtractor.cpp
@@ -37,7 +37,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #include 
 #include 
@@ -230,6 +230,29 @@
   s.Printf("\\x%2.2x", c);
 }
 
+/// Dump a floating point type.
+template 
+void DumpFloatingPoint(std::ostringstream &ss, FloatT f) {
+  static_assert(std::is_floating_point::value,
+"Only floating point types can be dumped.");
+  // NaN and Inf are potentially implementation defined and on Darwin it
+  // seems NaNs are printed without their sign. Manually implement dumping them
+  // here to avoid having to deal with platform differences.
+  if (std::isnan(f)) {
+if (std::signbit(f))
+  ss << '-';
+ss << "nan";
+return;
+  }
+  if (std::isinf(f)) {
+if (std::signbit(f))
+  ss << '-';
+ss << "inf";
+return;
+  }
+  ss << f;
+}
+
 lldb::offset_t lldb_private::DumpDataExtractor(
 const DataExtractor &DE, Stream *s, offset_t start_offset,
 lldb::Format item_format, size_t item_byte_size, size_t item_count,
@@ -570,14 +593,14 @@
 f = DE.GetFloat(&offset);
   }
   ss.precision(std::numeric_limits::digits10);
-  ss << f;
+  DumpFloatingPoint(ss, f);
 } else if (item_byte_size == sizeof(double)) {
   ss.precision(std::numeric_limits::digits10);
-  ss << DE.GetDouble(&offset);
+  DumpFloatingPoint(ss, DE.GetDouble(&offset));
 } else if (item_byte_size == sizeof(long double) ||
item_byte_size == 10) {
   ss.precision(std::numeric_limits::digits10);
-  ss << DE.GetLongDouble(&offset);
+  DumpFloatingPoint(ss, DE.GetLongDouble(&offset));
 } else {
   s->Printf("error: unsupported byte size (%" PRIu64
 ") for float format",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ae58cf5 - [lldb] Fix that LLDB doesn't print NaN's sign on Darwin

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-05-25T13:33:28+02:00
New Revision: ae58cf5f45a9c159afbf86e93c0c257a22c4ee02

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

LOG: [lldb] Fix that LLDB doesn't print NaN's sign on Darwin

It seems std::ostringstream ignores NaN signs on Darwin while it prints them on
Linux. This causes that LLDB behaves differently on those platforms which is
both confusing for users and it also means we have to deal with that in our
tests.

This patch manually implements the NaN/Inf printing (which are apparently
implementation defined) to make LLDB print the same thing on all platforms. The
only output difference in practice seems to be that we now print negative NaNs
as `-nan`, but this potentially also changes the output on other systems I
haven't tested this on.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/source/Core/DumpDataExtractor.cpp
lldb/unittests/Core/DumpDataExtractorTest.cpp

Removed: 




diff  --git a/lldb/source/Core/DumpDataExtractor.cpp 
b/lldb/source/Core/DumpDataExtractor.cpp
index 34c9353c9feaa..63e2cf2f2cc1e 100644
--- a/lldb/source/Core/DumpDataExtractor.cpp
+++ b/lldb/source/Core/DumpDataExtractor.cpp
@@ -37,7 +37,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #include 
 #include 
@@ -230,6 +230,29 @@ static void DumpCharacter(Stream &s, const char c) {
   s.Printf("\\x%2.2x", c);
 }
 
+/// Dump a floating point type.
+template 
+void DumpFloatingPoint(std::ostringstream &ss, FloatT f) {
+  static_assert(std::is_floating_point::value,
+"Only floating point types can be dumped.");
+  // NaN and Inf are potentially implementation defined and on Darwin it
+  // seems NaNs are printed without their sign. Manually implement dumping them
+  // here to avoid having to deal with platform 
diff erences.
+  if (std::isnan(f)) {
+if (std::signbit(f))
+  ss << '-';
+ss << "nan";
+return;
+  }
+  if (std::isinf(f)) {
+if (std::signbit(f))
+  ss << '-';
+ss << "inf";
+return;
+  }
+  ss << f;
+}
+
 lldb::offset_t lldb_private::DumpDataExtractor(
 const DataExtractor &DE, Stream *s, offset_t start_offset,
 lldb::Format item_format, size_t item_byte_size, size_t item_count,
@@ -570,14 +593,14 @@ lldb::offset_t lldb_private::DumpDataExtractor(
 f = DE.GetFloat(&offset);
   }
   ss.precision(std::numeric_limits::digits10);
-  ss << f;
+  DumpFloatingPoint(ss, f);
 } else if (item_byte_size == sizeof(double)) {
   ss.precision(std::numeric_limits::digits10);
-  ss << DE.GetDouble(&offset);
+  DumpFloatingPoint(ss, DE.GetDouble(&offset));
 } else if (item_byte_size == sizeof(long double) ||
item_byte_size == 10) {
   ss.precision(std::numeric_limits::digits10);
-  ss << DE.GetLongDouble(&offset);
+  DumpFloatingPoint(ss, DE.GetLongDouble(&offset));
 } else {
   s->Printf("error: unsupported byte size (%" PRIu64
 ") for float format",

diff  --git a/lldb/unittests/Core/DumpDataExtractorTest.cpp 
b/lldb/unittests/Core/DumpDataExtractorTest.cpp
index cfd24ff41491b..f76014aa938c0 100644
--- a/lldb/unittests/Core/DumpDataExtractorTest.cpp
+++ b/lldb/unittests/Core/DumpDataExtractorTest.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Utility/StreamString.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -197,13 +198,34 @@ TEST(DumpDataExtractorTest, Formats) {
lldb::Format::eFormatVectorOfFloat16, "{6.10352e-05 65504}");
   TestDump(std::vector{0xabcd, 0x1234},
lldb::Format::eFormatVectorOfFloat16, "{-0.0609436 0.000757217}");
+
+  // quiet/signaling NaNs.
+  TestDump(std::vector{0x, 0xffc0, 0x7fff, 0x7fc0},
+   lldb::Format::eFormatVectorOfFloat16, "{-nan -nan nan nan}");
+  // +/-Inf.
+  TestDump(std::vector{0xfc00, 0x7c00},
+   lldb::Format::eFormatVectorOfFloat16, "{-inf inf}");
+
   TestDump(std::vector{std::numeric_limits::min(),
   std::numeric_limits::max()},
lldb::Format::eFormatVectorOfFloat32, "{1.17549e-38 3.40282e+38}");
+  TestDump(std::vector{std::numeric_limits::quiet_NaN(),
+  std::numeric_limits::signaling_NaN(),
+  -std::numeric_limits::quiet_NaN(),
+  -std::numeric_limits::signaling_NaN()},
+   lldb::Format::eFormatVectorOfFloat32, "{nan nan -nan -nan}");
   TestDump(std::vector{std::numeric_limits::min(),
std::numeric_limits::max()},
lldb::Format::eFormatVectorOfFloat64,
   

Re: [Lldb-commits] [lldb] a3a9528 - [lldb] X-FAIL TestCPPStaticMembers on Windows

[David Blaikie via lldb-commits]

Might be worth a bug report and CC'ing some Windows-LLDB type folks on that
for notice.

On Tue, May 25, 2021 at 4:11 AM Raphael Isemann via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

>
> Author: Raphael Isemann
> Date: 2021-05-25T13:10:19+02:00
> New Revision: a3a95286a73fddc4027de930fac29728cd4259fc
>
> URL:
> https://github.com/llvm/llvm-project/commit/a3a95286a73fddc4027de930fac29728cd4259fc
> DIFF:
> https://github.com/llvm/llvm-project/commit/a3a95286a73fddc4027de930fac29728cd4259fc.diff
>
> LOG: [lldb] X-FAIL TestCPPStaticMembers on Windows
>
> This was originally failed because of llvm.org/pr21765 which describes
> that
> LLDB can't call a debugee's functions, but I removed the (unnecessary)
> function call in the rewrite. It seems that the actual bug here is that we
> can't lookup static members at all, so let's X-FAIL the test for the right
> reason.
>
> Added:
>
>
> Modified:
> lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
>
> Removed:
>
>
>
>
> 
> diff  --git
> a/lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
> b/lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
> index 8acf498bb7f7..07ec5c52ea4c 100644
> --- a/lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
> +++ b/lldb/test/API/lang/cpp/static_members/TestCPPStaticMembers.py
> @@ -15,7 +15,8 @@ class TestCase(TestBase):
>
>  mydir = TestBase.compute_mydir(__file__)
>
> -@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr21765")
> +# We fail to lookup static members on Windows.
> +@expectedFailureAll(oslist=["windows"])
>  def test_access_from_main(self):
>  self.build()
>  lldbutil.run_to_source_breakpoint(self, "// stop in main",
> lldb.SBFileSpec("main.cpp"))
> @@ -24,6 +25,8 @@ def test_access_from_main(self):
>  self.expect_expr("my_a.s_b", result_type="long", result_value="2")
>  self.expect_expr("my_a.s_c", result_type="int", result_value="3")
>
> +# We fail to lookup static members on Windows.
> +@expectedFailureAll(oslist=["windows"])
>  def test_access_from_member_function(self):
>  self.build()
>  lldbutil.run_to_source_breakpoint(self, "// stop in member
> function", lldb.SBFileSpec("main.cpp"))
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D102428: [StopInfoMachException] Summarize arm64e BLRAx/LDRAx auth failures

[Vedant Kumar via Phabricator via lldb-commits]

vsk added a comment.

Friendly ping.




Comment at: lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp:126
+Address brk_address;
+if (!target.ResolveLoadAddress(fixed_bad_address, brk_address))
+  return false;

DavidSpickett wrote:
> vsk wrote:
> > DavidSpickett wrote:
> > > vsk wrote:
> > > > DavidSpickett wrote:
> > > > > What does it mean here that the address failed to resolve?
> > > > It's possible that lldb doesn't know about the image the fixed address 
> > > > points to (it could be a garbage value). In this case we conservatively 
> > > > don't hint that there's a ptrauth issue.
> > > So in that case we would report stopped due to a breakpoint, that's a 
> > > special pac breakpoint but no pointer authentication issue? Isn't that 
> > > confusing for the user?
> > > 
> > > Maybe not because it's hinting at accidental corruption vs. deliberate 
> > > misdirection, you probably have the experiences to inform that.
> > > 
> > > This is an improvement as is so no need to change it I'm just curious.
> > > 
> > > Can you add a test for this situation? Assuming you can find an address 
> > > you know would never be valid.
> > The image containing the fixed address from x16 is usually loaded. If it's 
> > not, that's indeed a very confusing situation (& would more likely than not 
> > implicate an AppleClang bug). I don't believe the situation is made *more* 
> > confusing because lldb declines to print a ptrauth hint. I've added a test 
> > for this (it just sets x16 = 0xbad).
> Thanks, reading the test I see what you mean.
> 
> You convert to `EXC_BAD_ACCESS` even if the x16 address isn't loaded, so I'm 
> not seeing `EXC_BREAKPOINT` and wondering why I hit this breakpoint that I 
> didn't add. (didn't add manually at least)
I see what you mean. In this scenario, EXC_BAD_ACCESS seems a bit more to the 
point (to me, at least -- since with that particular trap code, there's no way 
this is a debugger breakpoint).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102428

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

[Lldb-commits] [PATCH] D103107: [lldb] Remove cache in get_demangled_name_without_arguments

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: LLDB.
teemperor added a project: LLDB.
Herald added a subscriber: JDevlieghere.
teemperor requested review of this revision.

This function has a single-value caching based on function local static 
variables.

This causes two problems:

- As there is no synchronization, so this function randomly returns the 
demangled name of other functions that are demangled at the same time.
- The 1-element cache is not very effective (the cache rate is around 0% when 
running the LLDB test suite).

I would propose just removing it.

To prevent anyone else the git archeology: the static result variables were 
originally
added as this returned a ConstString reference, but that has since been changed
so that this returns by value.


https://reviews.llvm.org/D103107

Files:
  lldb/source/Core/Mangled.cpp


Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -35,26 +35,8 @@
   return Mangled::GetManglingScheme(s) != Mangled::eManglingSchemeNone;
 }
 
-static ConstString 
-get_demangled_name_without_arguments(ConstString mangled,
- ConstString demangled) {
-  // This pair is 
-  static std::pair
-  g_most_recent_mangled_to_name_sans_args;
-
-  // Need to have the mangled & demangled names we're currently examining as
-  // statics so we can return a const ref to them at the end of the func if we
-  // don't have anything better.
-  static ConstString g_last_mangled;
-  static ConstString g_last_demangled;
-
-  if (mangled && g_most_recent_mangled_to_name_sans_args.first == mangled) {
-return g_most_recent_mangled_to_name_sans_args.second;
-  }
-
-  g_last_demangled = demangled;
-  g_last_mangled = mangled;
-
+static ConstString GetDemangledNameWithoutArguments(ConstString mangled,
+ConstString demangled) {
   const char *mangled_name_cstr = mangled.GetCString();
 
   if (demangled && mangled_name_cstr && mangled_name_cstr[0]) {
@@ -73,17 +55,13 @@
 if (!cxx_method.GetContext().empty())
   shortname = cxx_method.GetContext().str() + "::";
 shortname += cxx_method.GetBasename().str();
-ConstString result(shortname.c_str());
-g_most_recent_mangled_to_name_sans_args.first = mangled;
-g_most_recent_mangled_to_name_sans_args.second = result;
-return g_most_recent_mangled_to_name_sans_args.second;
+return ConstString(shortname);
   }
 }
   }
-
   if (demangled)
-return g_last_demangled;
-  return g_last_mangled;
+return demangled;
+  return mangled;
 }
 
 #pragma mark Mangled
@@ -347,7 +325,7 @@
   ConstString demangled = GetDemangledName();
 
   if (preference == ePreferDemangledWithoutArguments) {
-return get_demangled_name_without_arguments(m_mangled, demangled);
+return GetDemangledNameWithoutArguments(m_mangled, demangled);
   }
   if (preference == ePreferDemangled) {
 // Call the accessor to make sure we get a demangled name in case it hasn't


Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -35,26 +35,8 @@
   return Mangled::GetManglingScheme(s) != Mangled::eManglingSchemeNone;
 }
 
-static ConstString 
-get_demangled_name_without_arguments(ConstString mangled,
- ConstString demangled) {
-  // This pair is 
-  static std::pair
-  g_most_recent_mangled_to_name_sans_args;
-
-  // Need to have the mangled & demangled names we're currently examining as
-  // statics so we can return a const ref to them at the end of the func if we
-  // don't have anything better.
-  static ConstString g_last_mangled;
-  static ConstString g_last_demangled;
-
-  if (mangled && g_most_recent_mangled_to_name_sans_args.first == mangled) {
-return g_most_recent_mangled_to_name_sans_args.second;
-  }
-
-  g_last_demangled = demangled;
-  g_last_mangled = mangled;
-
+static ConstString GetDemangledNameWithoutArguments(ConstString mangled,
+ConstString demangled) {
   const char *mangled_name_cstr = mangled.GetCString();
 
   if (demangled && mangled_name_cstr && mangled_name_cstr[0]) {
@@ -73,17 +55,13 @@
 if (!cxx_method.GetContext().empty())
   shortname = cxx_method.GetContext().str() + "::";
 shortname += cxx_method.GetBasename().str();
-ConstString result(shortname.c_str());
-g_most_recent_mangled_to_name_sans_args.first = mangled;
-g_most_recent_mangled_to_name_sans_args.second = result;
-return g_most_recent_mangled_to_name_sans_args.second;
+return ConstString(shortname);
   }
 }
   }
-
   if (demangled)
-return g_last_demangled;
-  return g_last_mangled;
+return demangled;
+  return

[Lldb-commits] [PATCH] D103107: [lldb] Remove cache in get_demangled_name_without_arguments

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added inline comments.



Comment at: lldb/source/Core/Mangled.cpp:38
 
-static ConstString 
-get_demangled_name_without_arguments(ConstString mangled,
- ConstString demangled) {
-  // This pair is 
-  static std::pair
-  g_most_recent_mangled_to_name_sans_args;
-
-  // Need to have the mangled & demangled names we're currently examining as
-  // statics so we can return a const ref to them at the end of the func if we
-  // don't have anything better.
-  static ConstString g_last_mangled;
-  static ConstString g_last_demangled;
-
-  if (mangled && g_most_recent_mangled_to_name_sans_args.first == mangled) {
-return g_most_recent_mangled_to_name_sans_args.second;
-  }
-
-  g_last_demangled = demangled;
-  g_last_mangled = mangled;
-
+static ConstString GetDemangledNameWithoutArguments(ConstString mangled,
+ConstString demangled) {

I changed the name to reflect the code style. I'll do the same for the function 
above as a NFC follow up.


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

https://reviews.llvm.org/D103107

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

[Lldb-commits] [PATCH] D103107: [lldb] Remove cache in get_demangled_name_without_arguments

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision as: JDevlieghere.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D103107

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

[Lldb-commits] [PATCH] D103107: [lldb] Remove cache in get_demangled_name_without_arguments

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik accepted this revision.
shafik added a comment.

was there a bug that inspired this?


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

https://reviews.llvm.org/D103107

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

[Lldb-commits] [PATCH] D103107: [lldb] Remove cache in get_demangled_name_without_arguments

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D103107#2780769 , @shafik wrote:

> was there a bug that inspired this?

Nope, the code just looked bogus when I scrolled over it while looking for 
something else.


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

https://reviews.llvm.org/D103107

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

[Lldb-commits] [PATCH] D103124: [lldb] add LLDB_SKIP_DSYM option

[Richard Howell via Phabricator via lldb-commits]

rmaz created this revision.
Herald added a subscriber: mgorny.
rmaz requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Add an option to skip generating a dSYM when installing the LLDB framework on 
Darwin.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103124

Files:
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -65,6 +65,7 @@
 option(LLDB_NO_INSTALL_DEFAULT_RPATH "Disable default RPATH settings in 
binaries" OFF)
 option(LLDB_USE_SYSTEM_DEBUGSERVER "Use the system's debugserver for testing 
(Darwin only)." OFF)
 option(LLDB_SKIP_STRIP "Whether to skip stripping of binaries when installing 
lldb." OFF)
+option(LLDB_SKIP_DSYM "Whether to skip generating a dSYM when installing 
lldb." OFF)
 
 if (LLDB_USE_SYSTEM_DEBUGSERVER)
   # The custom target for the system debugserver has no install target, so we
Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -276,19 +276,21 @@
   endif()
 
   # Generate dSYM
-  set(dsym_name ${output_name}.dSYM)
-  if(is_framework)
-set(dsym_name ${output_name}.framework.dSYM)
-  endif()
-  if(LLDB_DEBUGINFO_INSTALL_PREFIX)
-# This makes the path absolute, so we must respect DESTDIR.
-set(dsym_name 
"\$ENV\{DESTDIR\}${LLDB_DEBUGINFO_INSTALL_PREFIX}/${dsym_name}")
-  endif()
+  if(NOT LLDB_SKIP_DSYM)
+set(dsym_name ${output_name}.dSYM)
+if(is_framework)
+  set(dsym_name ${output_name}.framework.dSYM)
+endif()
+if(LLDB_DEBUGINFO_INSTALL_PREFIX)
+  # This makes the path absolute, so we must respect DESTDIR.
+  set(dsym_name 
"\$ENV\{DESTDIR\}${LLDB_DEBUGINFO_INSTALL_PREFIX}/${dsym_name}")
+endif()
 
-  set(buildtree_name ${buildtree_dir}/${bundle_subdir}${output_name})
-  install(CODE "message(STATUS \"Externalize debuginfo: ${dsym_name}\")" 
COMPONENT ${name})
-  install(CODE "execute_process(COMMAND xcrun dsymutil -o=${dsym_name} 
${buildtree_name})"
-  COMPONENT ${name})
+set(buildtree_name ${buildtree_dir}/${bundle_subdir}${output_name})
+install(CODE "message(STATUS \"Externalize debuginfo: ${dsym_name}\")" 
COMPONENT ${name})
+install(CODE "execute_process(COMMAND xcrun dsymutil -o=${dsym_name} 
${buildtree_name})"
+COMPONENT ${name})
+  endif()
 
   if(NOT LLDB_SKIP_STRIP)
 # Strip distribution binary with -ST (removing debug symbol table entries 
and


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -65,6 +65,7 @@
 option(LLDB_NO_INSTALL_DEFAULT_RPATH "Disable default RPATH settings in binaries" OFF)
 option(LLDB_USE_SYSTEM_DEBUGSERVER "Use the system's debugserver for testing (Darwin only)." OFF)
 option(LLDB_SKIP_STRIP "Whether to skip stripping of binaries when installing lldb." OFF)
+option(LLDB_SKIP_DSYM "Whether to skip generating a dSYM when installing lldb." OFF)
 
 if (LLDB_USE_SYSTEM_DEBUGSERVER)
   # The custom target for the system debugserver has no install target, so we
Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -276,19 +276,21 @@
   endif()
 
   # Generate dSYM
-  set(dsym_name ${output_name}.dSYM)
-  if(is_framework)
-set(dsym_name ${output_name}.framework.dSYM)
-  endif()
-  if(LLDB_DEBUGINFO_INSTALL_PREFIX)
-# This makes the path absolute, so we must respect DESTDIR.
-set(dsym_name "\$ENV\{DESTDIR\}${LLDB_DEBUGINFO_INSTALL_PREFIX}/${dsym_name}")
-  endif()
+  if(NOT LLDB_SKIP_DSYM)
+set(dsym_name ${output_name}.dSYM)
+if(is_framework)
+  set(dsym_name ${output_name}.framework.dSYM)
+endif()
+if(LLDB_DEBUGINFO_INSTALL_PREFIX)
+  # This makes the path absolute, so we must respect DESTDIR.
+  set(dsym_name "\$ENV\{DESTDIR\}${LLDB_DEBUGINFO_INSTALL_PREFIX}/${dsym_name}")
+endif()
 
-  set(buildtree_name ${buildtree_dir}/${bundle_subdir}${output_name})
-  install(CODE "message(STATUS \"Externalize debuginfo: ${dsym_name}\")" COMPONENT ${name})
-  install(CODE "execute_process(COMMAND xcrun dsymutil -o=${dsym_name} ${buildtree_name})"
-  COMPONENT ${name})
+set(buildtree_name ${buildtree_dir}/${bundle_subdir}${output_name})
+install(CODE "message(STATUS \"Externalize debuginfo: ${dsym_name}\")" COMPONENT ${name})
+install(CODE "execute_process(COMMAND xcrun dsymutil -o=${dsym_name} ${buildtree_name})"
+COMPONENT ${name})
+  endif()
 
   if(NOT LLDB_SKIP_STRIP)
 # Strip distribution binary with -ST (r

[Lldb-commits] [PATCH] D103127: lldb: Don't check for CMAKE_SYSTEM_NAME==Android.

[Peter Collingbourne via Phabricator via lldb-commits]

pcc created this revision.
pcc added reviewers: JDevlieghere, labath.
Herald added subscribers: danielkiss, krytarowski, mgorny.
pcc requested review of this revision.
Herald added a project: LLDB.

CMAKE_SYSTEM_NAME seems to be unreliable for detecting whether the
target is Android. At least on my machine it always turns out to
have the value Linux when targeting Android. Even explicitly passing
-DCMAKE_SYSTEM_NAME=Android to CMake doesn't seem to help. It seems
that CMake is overriding the value that I pass in with a value that
it computed somehow.

To avoid relying on whichever fragile mechanism is used to detect
Android targets, let's just compile the Android source files
unconditionally and use the preprocessor to exclude their contents
on non-Android platforms.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103127

Files:
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/android/HostInfoAndroid.cpp
  lldb/source/Host/android/LibcGlue.cpp


Index: lldb/source/Host/android/LibcGlue.cpp
===
--- lldb/source/Host/android/LibcGlue.cpp
+++ lldb/source/Host/android/LibcGlue.cpp
@@ -8,6 +8,8 @@
 
 // This files adds functions missing from libc on earlier versions of Android
 
+#if defined(__ANDROID__)
+
 #include 
 
 #include 
@@ -26,3 +28,5 @@
 int posix_openpt(int flags) { return open("/dev/ptmx", flags); }
 
 #endif
+
+#endif // __ANDROID__
Index: lldb/source/Host/android/HostInfoAndroid.cpp
===
--- lldb/source/Host/android/HostInfoAndroid.cpp
+++ lldb/source/Host/android/HostInfoAndroid.cpp
@@ -6,6 +6,8 @@
 //
 
//===--===//
 
+#if defined(__ANDROID__)
+
 #include "lldb/Host/android/HostInfoAndroid.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/linux/HostInfoLinux.h"
@@ -92,3 +94,5 @@
 
   return FileSystem::Instance().Exists(file_spec);
 }
+
+#endif // __ANDROID__
Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -106,12 +106,10 @@
   linux/LibcGlue.cpp
   linux/Support.cpp
   )
-if (CMAKE_SYSTEM_NAME MATCHES "Android")
-  add_host_subdirectory(android
-android/HostInfoAndroid.cpp
-android/LibcGlue.cpp
-)
-endif()
+add_host_subdirectory(android
+  android/HostInfoAndroid.cpp
+  android/LibcGlue.cpp
+  )
   elseif (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
 add_host_subdirectory(freebsd
   freebsd/Host.cpp


Index: lldb/source/Host/android/LibcGlue.cpp
===
--- lldb/source/Host/android/LibcGlue.cpp
+++ lldb/source/Host/android/LibcGlue.cpp
@@ -8,6 +8,8 @@
 
 // This files adds functions missing from libc on earlier versions of Android
 
+#if defined(__ANDROID__)
+
 #include 
 
 #include 
@@ -26,3 +28,5 @@
 int posix_openpt(int flags) { return open("/dev/ptmx", flags); }
 
 #endif
+
+#endif // __ANDROID__
Index: lldb/source/Host/android/HostInfoAndroid.cpp
===
--- lldb/source/Host/android/HostInfoAndroid.cpp
+++ lldb/source/Host/android/HostInfoAndroid.cpp
@@ -6,6 +6,8 @@
 //
 //===--===//
 
+#if defined(__ANDROID__)
+
 #include "lldb/Host/android/HostInfoAndroid.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/linux/HostInfoLinux.h"
@@ -92,3 +94,5 @@
 
   return FileSystem::Instance().Exists(file_spec);
 }
+
+#endif // __ANDROID__
Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -106,12 +106,10 @@
   linux/LibcGlue.cpp
   linux/Support.cpp
   )
-if (CMAKE_SYSTEM_NAME MATCHES "Android")
-  add_host_subdirectory(android
-android/HostInfoAndroid.cpp
-android/LibcGlue.cpp
-)
-endif()
+add_host_subdirectory(android
+  android/HostInfoAndroid.cpp
+  android/LibcGlue.cpp
+  )
   elseif (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
 add_host_subdirectory(freebsd
   freebsd/Host.cpp
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading to lldb

[Peter Collingbourne via Phabricator via lldb-commits]

pcc added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:633
+
+  if (SendPacketAndWaitForResponse(packet.GetString(), response) !=
+  PacketResult::Success ||




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95602

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

[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add memory tag reading to lldb-server

[Peter Collingbourne via Phabricator via lldb-commits]

pcc added inline comments.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:15
 
 #include "lldb/Host/common/NativeProcessProtocol.h"
 #include "lldb/Utility/DataBufferHeap.h"




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95601

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

[Lldb-commits] [lldb] bbcb343 - [lldb] Avoid format string in LLDB_SCOPED_TIMER

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-05-25T17:14:08-07:00
New Revision: bbcb3433d4e8f5fa385b14e74d4314bd5409022d

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

LOG: [lldb] Avoid format string in LLDB_SCOPED_TIMER

Pass LLVM_PRETTY_FUNCTION directly for the no-argument macro.

Added: 


Modified: 
lldb/include/lldb/Utility/Timer.h

Removed: 




diff  --git a/lldb/include/lldb/Utility/Timer.h 
b/lldb/include/lldb/Utility/Timer.h
index 32422b34b1ad..edc064b23b57 100644
--- a/lldb/include/lldb/Utility/Timer.h
+++ b/lldb/include/lldb/Utility/Timer.h
@@ -76,7 +76,7 @@ class Timer {
 
 #define LLDB_SCOPED_TIMER()
\
   static ::lldb_private::Timer::Category _cat(LLVM_PRETTY_FUNCTION);   
\
-  ::lldb_private::Timer _scoped_timer(_cat, "%s", LLVM_PRETTY_FUNCTION)
+  ::lldb_private::Timer _scoped_timer(_cat, LLVM_PRETTY_FUNCTION)
 #define LLDB_SCOPED_TIMERF(...)
\
   static ::lldb_private::Timer::Category _cat(LLVM_PRETTY_FUNCTION);   
\
   ::lldb_private::Timer _scoped_timer(_cat, __VA_ARGS__)



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

[Lldb-commits] [lldb] 564eb20 - Revert "[lldb] Avoid format string in LLDB_SCOPED_TIMER"

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-05-25T17:22:51-07:00
New Revision: 564eb20e0deecd173a7b990dcfd0e57fb045c522

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

LOG: Revert "[lldb] Avoid format string in LLDB_SCOPED_TIMER"

Right after pushing, I remembered that this was added to silence a GCC
warning (https://reviews.llvm.org/D99120). This reverts my patch and
adds a comment.

Added: 


Modified: 
lldb/include/lldb/Utility/Timer.h

Removed: 




diff  --git a/lldb/include/lldb/Utility/Timer.h 
b/lldb/include/lldb/Utility/Timer.h
index edc064b23b57..0249c697b8ed 100644
--- a/lldb/include/lldb/Utility/Timer.h
+++ b/lldb/include/lldb/Utility/Timer.h
@@ -74,9 +74,11 @@ class Timer {
 
 } // namespace lldb_private
 
+// Use a format string because LLVM_PRETTY_FUNCTION might not be a string
+// literal.
 #define LLDB_SCOPED_TIMER()
\
   static ::lldb_private::Timer::Category _cat(LLVM_PRETTY_FUNCTION);   
\
-  ::lldb_private::Timer _scoped_timer(_cat, LLVM_PRETTY_FUNCTION)
+  ::lldb_private::Timer _scoped_timer(_cat, "%s", LLVM_PRETTY_FUNCTION)
 #define LLDB_SCOPED_TIMERF(...)
\
   static ::lldb_private::Timer::Category _cat(LLVM_PRETTY_FUNCTION);   
\
   ::lldb_private::Timer _scoped_timer(_cat, __VA_ARGS__)



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

[Lldb-commits] [PATCH] D103124: [lldb] add LLDB_SKIP_DSYM option

[Shoaib Meenai via Phabricator via lldb-commits]

smeenai accepted this revision.
smeenai added a subscriber: sgraenitz.
smeenai added a comment.
This revision is now accepted and ready to land.

Swift's LLDB fork has a TODO for this :) CC @sgraenitz, who added that TODO.

LGTM. Do you need this to be committed for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103124

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

[Lldb-commits] [PATCH] D99944: [LLDB] AArch64 Linux and elf-core PAC stack unwinder support

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 347847.
omjavaid added a comment.

This add skipped linux-aarch64-pac.out file.


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

https://reviews.llvm.org/D99944

Files:
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-pac.out
  lldb/test/API/functionalities/unwind/aarch64_unwind_pac/Makefile
  
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
  lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c

Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
@@ -0,0 +1,24 @@
+// This program makes a multi tier nested function call to test AArch64
+// Pointer Authentication feature.
+
+// To enable PAC return address signing compile with following clang arguments:
+// -march=armv8.5-a -mbranch-protection=pac-ret+leaf
+
+#include 
+
+static void __attribute__((noinline)) func_c(void) {
+  exit(0); // Frame func_c
+}
+
+static void __attribute__((noinline)) func_b(void) {
+  func_c(); // Frame func_b
+}
+
+static void __attribute__((noinline)) func_a(void) {
+  func_b(); // Frame func_a
+}
+
+int main(int argc, char *argv[]) {
+  func_a(); // Frame main
+  return 0;
+}
Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
===
--- /dev/null
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
@@ -0,0 +1,44 @@
+"""
+Test that we can backtrace correctly when AArch64 PAC is enabled
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64UnwindPAC(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(['linux']))
+def test(self):
+"""Test that we can backtrace correctly when AArch64 PAC is enabled"""
+self.build()
+
+self.line = line_number('main.c', '// Frame func_c')
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1)
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."])
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+thread = process.GetThreadAtIndex(0)
+
+backtrace = ["func_c", "func_b", "func_a", "main", "__libc_start_main", "_start"]
+self.assertEqual(thread.GetNumFrames(), len(backtrace))
+for frame_idx, frame in enumerate(thread.frames):
+frame = thread.GetFrameAtIndex(frame_idx)
+self.assertTrue(frame)
+self.assertEqual(frame.GetFunctionName(), backtrace[frame_idx])
+			# Check line number for functions in main.c
+if (frame_idx < 4):
+self.assertEqual(frame.GetLineEntry().GetLine(),
+ line_number("main.c", "Frame " + backtrace[frame_idx]))
Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/Makefile
@@ -0,0 +1,5 @@
+C_SOURCES := main.c
+
+CFLAGS := -g -Os -march=armv8.5-a -mbranch-protection=pac-ret+leaf
+
+include Makefile.rules
Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -20,6 +20,7 @@
 mydir = TestBase.compute_mydir(__file__)
 
 _aarch64_pid = 37688
+_aarch64_pac_pid = 387
 _i386_pid = 32306
 _x86_64_pid = 32259
 _s390x_pid = 1045
@@ -257,6 +258,18 @@
 
 self.dbg.DeleteTarget(target)
 
+@skipIfLLVMTargetMissing("AArch64")
+def test_aarch64_pac(self):
+"""Test that lldb can unwind stack for AArch64 elf core file with PAC enabled."""
+
+target = self.dbg.CreateTarget("linux-aarch64-pac.out")
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore("linux-aarch64-pac.core")
+
+self.check_all(process, self._aarch64_pac_pid, self._aarch64_regions, "a.out")
+
+self.dbg.DeleteTarget(target)
+
 @skipIfLLVMTargetMissing("AArch64")
 @expectedFailureAll(archs=["aarch64"], oslist=["freebsd"],
 

[Lldb-commits] [PATCH] D99944: [LLDB] AArch64 Linux and elf-core PAC stack unwinder support

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 347848.
omjavaid added a comment.
Herald added a project: LLDB.

Uploading linux-aarch64-pac.out using arc

differential was not uploading binary file properly using arc to do the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99944

Files:
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-pac.out
  lldb/test/API/functionalities/unwind/aarch64_unwind_pac/Makefile
  
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
  lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c

Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
@@ -0,0 +1,24 @@
+// This program makes a multi tier nested function call to test AArch64
+// Pointer Authentication feature.
+
+// To enable PAC return address signing compile with following clang arguments:
+// -march=armv8.5-a -mbranch-protection=pac-ret+leaf
+
+#include 
+
+static void __attribute__((noinline)) func_c(void) {
+  exit(0); // Frame func_c
+}
+
+static void __attribute__((noinline)) func_b(void) {
+  func_c(); // Frame func_b
+}
+
+static void __attribute__((noinline)) func_a(void) {
+  func_b(); // Frame func_a
+}
+
+int main(int argc, char *argv[]) {
+  func_a(); // Frame main
+  return 0;
+}
Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
===
--- /dev/null
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
@@ -0,0 +1,44 @@
+"""
+Test that we can backtrace correctly when AArch64 PAC is enabled
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64UnwindPAC(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(['linux']))
+def test(self):
+"""Test that we can backtrace correctly when AArch64 PAC is enabled"""
+self.build()
+
+self.line = line_number('main.c', '// Frame func_c')
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1)
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."])
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+thread = process.GetThreadAtIndex(0)
+
+backtrace = ["func_c", "func_b", "func_a", "main", "__libc_start_main", "_start"]
+self.assertEqual(thread.GetNumFrames(), len(backtrace))
+for frame_idx, frame in enumerate(thread.frames):
+frame = thread.GetFrameAtIndex(frame_idx)
+self.assertTrue(frame)
+self.assertEqual(frame.GetFunctionName(), backtrace[frame_idx])
+			# Check line number for functions in main.c
+if (frame_idx < 4):
+self.assertEqual(frame.GetLineEntry().GetLine(),
+ line_number("main.c", "Frame " + backtrace[frame_idx]))
Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/Makefile
@@ -0,0 +1,5 @@
+C_SOURCES := main.c
+
+CFLAGS := -g -Os -march=armv8.5-a -mbranch-protection=pac-ret+leaf
+
+include Makefile.rules
Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -20,6 +20,7 @@
 mydir = TestBase.compute_mydir(__file__)
 
 _aarch64_pid = 37688
+_aarch64_pac_pid = 387
 _i386_pid = 32306
 _x86_64_pid = 32259
 _s390x_pid = 1045
@@ -257,6 +258,18 @@
 
 self.dbg.DeleteTarget(target)
 
+@skipIfLLVMTargetMissing("AArch64")
+def test_aarch64_pac(self):
+"""Test that lldb can unwind stack for AArch64 elf core file with PAC enabled."""
+
+target = self.dbg.CreateTarget("linux-aarch64-pac.out")
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore("linux-aarch64-pac.core")
+
+self.check_all(process, self._aarch64_pac_pid, self._aarch64_regions, "a.out")
+
+s