[Lldb-commits] [lldb] 3c721b9 - Revert "[lldb] Fix evaluation of expressions with static initializers (#89063)"
Author: Pavel Labath Date: 2024-04-18T07:30:18Z New Revision: 3c721b90d363bf73b78467f6e86c879235bac1b2 URL: https://github.com/llvm/llvm-project/commit/3c721b90d363bf73b78467f6e86c879235bac1b2 DIFF: https://github.com/llvm/llvm-project/commit/3c721b90d363bf73b78467f6e86c879235bac1b2.diff LOG: Revert "[lldb] Fix evaluation of expressions with static initializers (#89063)" It breaks expression evaluation on arm, and the x86 breakage has been fixed in 6cea7c491f4c4c68aa0494a9b18f36ff40c22c81. This reverts commit 915c84b1480bb3c6d2e44ca83822d2c2304b763a. Added: Modified: lldb/source/Expression/IRExecutionUnit.cpp Removed: diff --git a/lldb/source/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp index 7ad0e5ff22b2f6..cb9bee8733e15d 100644 --- a/lldb/source/Expression/IRExecutionUnit.cpp +++ b/lldb/source/Expression/IRExecutionUnit.cpp @@ -13,7 +13,6 @@ #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Module.h" -#include "llvm/Support/CodeGen.h" #include "llvm/Support/SourceMgr.h" #include "llvm/Support/raw_ostream.h" @@ -280,13 +279,10 @@ void IRExecutionUnit::GetRunnableInfo(Status &error, lldb::addr_t &func_addr, llvm::EngineBuilder builder(std::move(m_module_up)); llvm::Triple triple(m_module->getTargetTriple()); - // PIC needed for ELF to avoid generating 32-bit relocations (which overflow - // if the object is loaded into high memory). - bool want_pic = triple.isOSBinFormatMachO() || triple.isOSBinFormatELF(); - builder.setEngineKind(llvm::EngineKind::JIT) .setErrorStr(&error_string) - .setRelocationModel(want_pic ? llvm::Reloc::PIC_ : llvm::Reloc::Static) + .setRelocationModel(triple.isOSBinFormatMachO() ? llvm::Reloc::PIC_ + : llvm::Reloc::Static) .setMCJITMemoryManager(std::make_unique(*this)) .setOptLevel(llvm::CodeGenOptLevel::Less); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 03e841c - [lldb] Correct documentation of LLDB_TEST_USER_ARGS (#89042)
Author: David Spickett Date: 2024-04-18T09:08:53+01:00 New Revision: 03e841c870e7beccf1368e566045e233b3bd8db5 URL: https://github.com/llvm/llvm-project/commit/03e841c870e7beccf1368e566045e233b3bd8db5 DIFF: https://github.com/llvm/llvm-project/commit/03e841c870e7beccf1368e566045e233b3bd8db5.diff LOG: [lldb] Correct documentation of LLDB_TEST_USER_ARGS (#89042) https://discourse.llvm.org/t/running-lldb-in-a-container/76801/4 found that the obvious way to use this variable doesn't work, despite what our docs and examples say. Perhaps in the past it did but now you need to use ";" as a separator to make sure the final command line works properly. For example "-A foo" becomes "-A foo" when python goes to run the runner script. The script sees this as one command line element, not two. What you actually want is "-A;foo" which we convert to "-A" "foo" which the script sees as one option and one value for that option. The "Script:" printout from dotest is misleading here because it does `" ".join(cmd)` so it looks like it's ok but in fact it's not. I'm not changing that format though because printing the command as a Python list is not useful outside of this specific situation. Added: Modified: lldb/docs/resources/test.rst lldb/test/API/CMakeLists.txt Removed: diff --git a/lldb/docs/resources/test.rst b/lldb/docs/resources/test.rst index 2b0e9010fe280a..094fde8b1b5a0a 100644 --- a/lldb/docs/resources/test.rst +++ b/lldb/docs/resources/test.rst @@ -441,23 +441,40 @@ Running the Full Test Suite The easiest way to run the LLDB test suite is to use the ``check-lldb`` build target. +:: + + $ ninja check-lldb + +Changing Test Suite Options +``` + By default, the ``check-lldb`` target builds the test programs with the same compiler that was used to build LLDB. To build the tests with a diff erent compiler, you can set the ``LLDB_TEST_COMPILER`` CMake variable. +You can also add to the test runner options by setting the +``LLDB_TEST_USER_ARGS`` CMake variable. This variable uses ``;`` to separate +items which must be separate parts of the runner's command line. + It is possible to customize the architecture of the test binaries and compiler -used by appending ``-A`` and ``-C`` options respectively to the CMake variable -``LLDB_TEST_USER_ARGS``. For example, to test LLDB against 32-bit binaries -built with a custom version of clang, do: +used by appending ``-A`` and ``-C`` options respectively. For example, to test +LLDB against 32-bit binaries built with a custom version of clang, do: :: - $ cmake -DLLDB_TEST_USER_ARGS="-A i386 -C /path/to/custom/clang" -G Ninja + $ cmake -DLLDB_TEST_USER_ARGS="-A;i386;-C;/path/to/custom/clang" -G Ninja $ ninja check-lldb Note that multiple ``-A`` and ``-C`` flags can be specified to ``LLDB_TEST_USER_ARGS``. +If you want to change the LLDB settings that tests run with then you can set +the ``--setting`` option of the test runner via this same variable. For example +``--setting;target.disable-aslr=true``. + +For a full list of test runner options, see +``/bin/lldb-dotest --help``. + Running a Single Test Suite ``` diff --git a/lldb/test/API/CMakeLists.txt b/lldb/test/API/CMakeLists.txt index 4e02112c29e4cd..9196f54ce1ae32 100644 --- a/lldb/test/API/CMakeLists.txt +++ b/lldb/test/API/CMakeLists.txt @@ -35,7 +35,8 @@ set(LLDB_TEST_ARCH # Users can override LLDB_TEST_USER_ARGS to specify arbitrary arguments to pass to the script set(LLDB_TEST_USER_ARGS "" - CACHE STRING "Specify additional arguments to pass to test runner. For example: '-C gcc -C clang -A i386 -A x86_64'") + CACHE STRING "Specify additional arguments to pass to test runner. Seperate \ +items with \";\". For example: '-C;gcc;-C;clang;-A;i386;-A;x86_64'") set(LLDB_TEST_COMMON_ARGS_VAR -u CXXFLAGS ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Correct documentation of LLDB_TEST_USER_ARGS (PR #89042)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/89042 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/89183 The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once). >From d8e6fdff0df7b1a2e9242d747831966c5db03e44 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 18 Apr 2024 07:34:45 + Subject: [PATCH] [lldb] Make SBType::FindDirectNestedType work with expression ASTs The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once). --- lldb/include/lldb/Symbol/TypeSystem.h | 7 .../TypeSystem/Clang/TypeSystemClang.cpp | 30 .../TypeSystem/Clang/TypeSystemClang.h| 4 +++ lldb/source/Symbol/Type.cpp | 15 +--- lldb/test/API/python_api/type/TestTypeList.py | 35 ++- 5 files changed, 61 insertions(+), 30 deletions(-) diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h index f647fcbf1636ea..3a927d313b823d 100644 --- a/lldb/include/lldb/Symbol/TypeSystem.h +++ b/lldb/include/lldb/Symbol/TypeSystem.h @@ -28,6 +28,7 @@ #include "lldb/Symbol/CompilerDeclContext.h" #include "lldb/Symbol/Type.h" #include "lldb/lldb-private.h" +#include "lldb/lldb-types.h" class PDBASTParser; @@ -363,6 +364,12 @@ class TypeSystem : public PluginInterface, lldb::opaque_compiler_type_t type, llvm::StringRef name, bool omit_empty_base_classes, std::vector &child_indexes) = 0; + virtual CompilerType + GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { +return CompilerType(); + } + virtual bool IsTemplateType(lldb::opaque_compiler_type_t type); virtual size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index be0ddb06f82c18..2621f682011b41 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -6997,6 +6997,36 @@ TypeSystemClang::GetIndexOfChildWithName(lldb::opaque_compiler_type_t type, return UINT32_MAX; } +CompilerType +TypeSystemClang::GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { + if (!type || name.empty()) +return CompilerType(); + + clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type)); + const clang::Type::TypeClass type_class = qual_type->getTypeClass(); + + switch (type_class) { + case clang::Type::Record: { +if (!GetCompleteType(type)) + return CompilerType(); +const clang::RecordType *record_type = +llvm::cast(qual_type.getTypePtr()); +const clang::RecordDecl *record_decl = record_type->getDecl(); + +clang::DeclarationName decl_name(&getASTContext().Idents.get(name)); +for (NamedDecl *decl : record_decl->lookup(decl_name)) { + if (auto *tag_decl = dyn_cast(decl)) +return GetType(getASTContext().getTagDeclType(tag_decl)); +} +break; + } + default: +break; + } + return CompilerType(); +} + bool TypeSystemClang::IsTemplateType(lldb::opaque_compiler_type_t type) { if (!type) return false; diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 05c303baa41640..cfe5327e14f823 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -897,6 +897,10 @@ class TypeSystemClang : public TypeSystem { bool omit_empty_base_classes, std::vector &child_indexes) override; + CompilerType + GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) override; + bool IsTemplateType(lldb::opaque_compiler_type_t type) override; size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp index 44a24d7178f562..5c49c4726157fb 100644 --- a/lldb/source/Symbol/Type.cpp +++ b/lldb/source/Symbol/Type.cpp @@ -1177,20 +1177,7 @@ CompilerType TypeImpl::FindDirectNe
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Pavel Labath (labath) Changes The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once). --- Full diff: https://github.com/llvm/llvm-project/pull/89183.diff 5 Files Affected: - (modified) lldb/include/lldb/Symbol/TypeSystem.h (+7) - (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+30) - (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h (+4) - (modified) lldb/source/Symbol/Type.cpp (+1-14) - (modified) lldb/test/API/python_api/type/TestTypeList.py (+19-16) ``diff diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h index f647fcbf1636ea..3a927d313b823d 100644 --- a/lldb/include/lldb/Symbol/TypeSystem.h +++ b/lldb/include/lldb/Symbol/TypeSystem.h @@ -28,6 +28,7 @@ #include "lldb/Symbol/CompilerDeclContext.h" #include "lldb/Symbol/Type.h" #include "lldb/lldb-private.h" +#include "lldb/lldb-types.h" class PDBASTParser; @@ -363,6 +364,12 @@ class TypeSystem : public PluginInterface, lldb::opaque_compiler_type_t type, llvm::StringRef name, bool omit_empty_base_classes, std::vector &child_indexes) = 0; + virtual CompilerType + GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { +return CompilerType(); + } + virtual bool IsTemplateType(lldb::opaque_compiler_type_t type); virtual size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index be0ddb06f82c18..2621f682011b41 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -6997,6 +6997,36 @@ TypeSystemClang::GetIndexOfChildWithName(lldb::opaque_compiler_type_t type, return UINT32_MAX; } +CompilerType +TypeSystemClang::GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { + if (!type || name.empty()) +return CompilerType(); + + clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type)); + const clang::Type::TypeClass type_class = qual_type->getTypeClass(); + + switch (type_class) { + case clang::Type::Record: { +if (!GetCompleteType(type)) + return CompilerType(); +const clang::RecordType *record_type = +llvm::cast(qual_type.getTypePtr()); +const clang::RecordDecl *record_decl = record_type->getDecl(); + +clang::DeclarationName decl_name(&getASTContext().Idents.get(name)); +for (NamedDecl *decl : record_decl->lookup(decl_name)) { + if (auto *tag_decl = dyn_cast(decl)) +return GetType(getASTContext().getTagDeclType(tag_decl)); +} +break; + } + default: +break; + } + return CompilerType(); +} + bool TypeSystemClang::IsTemplateType(lldb::opaque_compiler_type_t type) { if (!type) return false; diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 05c303baa41640..cfe5327e14f823 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -897,6 +897,10 @@ class TypeSystemClang : public TypeSystem { bool omit_empty_base_classes, std::vector &child_indexes) override; + CompilerType + GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) override; + bool IsTemplateType(lldb::opaque_compiler_type_t type) override; size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp index 44a24d7178f562..5c49c4726157fb 100644 --- a/lldb/source/Symbol/Type.cpp +++ b/lldb/source/Symbol/Type.cpp @@ -1177,20 +1177,7 @@ CompilerType TypeImpl::FindDirectNestedType(llvm::StringRef name) { if (name.empty()) return CompilerType(); auto type_system = GetTypeSystem(/*prefer_dynamic*/ false); - auto *symbol_file = type_system->GetSymbolFile(); - if (!symbol_file) -return CompilerType(); - auto decl_context = type_system->GetCompilerDeclContextForType(m_static_type); - if (!decl_context.IsValid()) -return CompilerType(); - TypeQuery query(decl_context, ConstString(name), - TypeQueryOptions::e_find_one); - TypeResults results; - symbol_file->FindTypes(query, results); - TypeSP type_
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
github-actions[bot] wrote: :warning: Python code formatter, darker found issues in your code. :warning: You can test this locally with the following command: ``bash darker --check --diff -r 3c721b90d363bf73b78467f6e86c879235bac1b2...d8e6fdff0df7b1a2e9242d747831966c5db03e44 lldb/test/API/python_api/type/TestTypeList.py `` View the diff from darker here. ``diff --- TestTypeList.py 2024-04-18 07:52:02.00 + +++ TestTypeList.py 2024-04-18 08:13:06.889902 + @@ -167,11 +167,13 @@ self.assertFalse(invalid_type) # Check that FindDirectNestedType works with types from module and # expression ASTs. self._find_nested_type_in_Task_pointer(frame0.FindVariable("pointer").GetType()) - self._find_nested_type_in_Task_pointer(frame0.EvaluateExpression("pointer").GetType()) +self._find_nested_type_in_Task_pointer( +frame0.EvaluateExpression("pointer").GetType() +) # We'll now get the child member 'id' from 'task_head'. id = task_head.GetChildMemberWithName("id") self.DebugSBValue(id) id_type = id.GetType() `` https://github.com/llvm/llvm-project/pull/89183 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 3c721b90d363bf73b78467f6e86c879235bac1b2 d8e6fdff0df7b1a2e9242d747831966c5db03e44 -- lldb/include/lldb/Symbol/TypeSystem.h lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h lldb/source/Symbol/Type.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index cfe5327e14..68b82e9688 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -897,9 +897,8 @@ public: bool omit_empty_base_classes, std::vector &child_indexes) override; - CompilerType - GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, - llvm::StringRef name) override; + CompilerType GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) override; bool IsTemplateType(lldb::opaque_compiler_type_t type) override; diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp index 5c49c47261..d9894ac355 100644 --- a/lldb/source/Symbol/Type.cpp +++ b/lldb/source/Symbol/Type.cpp @@ -1177,7 +1177,8 @@ CompilerType TypeImpl::FindDirectNestedType(llvm::StringRef name) { if (name.empty()) return CompilerType(); auto type_system = GetTypeSystem(/*prefer_dynamic*/ false); - return type_system->GetDirectNestedTypeWithName(m_static_type.GetOpaqueQualType(), name); + return type_system->GetDirectNestedTypeWithName( + m_static_type.GetOpaqueQualType(), name); } bool TypeMemberFunctionImpl::IsValid() { `` https://github.com/llvm/llvm-project/pull/89183 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
labath wrote: In terms of #68705, I think this will fix the issue where the data formatter works on `frame var foo` but not `expr -- foo`. At least that's the problem I ran into when trying to use this in my own formatter.. One thing I'd like to get feedback on is whether this should be looking into base classes. The discussion on the original PR focuses on lexically nested types, but going through base classes (following usual language rules) is another form of nesting. Both the existing and the new implementations don't do that, but I think it would be more natural to do it. Of course, then the function can definitely return more than one result... https://github.com/llvm/llvm-project/pull/89183 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/89183 >From c24f35cb4f568c42da9a11c7d91cfbaf7bf2f59e Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 18 Apr 2024 07:34:45 + Subject: [PATCH] [lldb] Make SBType::FindDirectNestedType work with expression ASTs The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once). --- lldb/include/lldb/Symbol/TypeSystem.h | 7 .../TypeSystem/Clang/TypeSystemClang.cpp | 30 .../TypeSystem/Clang/TypeSystemClang.h| 3 ++ lldb/source/Symbol/Type.cpp | 16 ++--- lldb/test/API/python_api/type/TestTypeList.py | 35 ++- 5 files changed, 61 insertions(+), 30 deletions(-) diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h index f647fcbf1636ea..3a927d313b823d 100644 --- a/lldb/include/lldb/Symbol/TypeSystem.h +++ b/lldb/include/lldb/Symbol/TypeSystem.h @@ -28,6 +28,7 @@ #include "lldb/Symbol/CompilerDeclContext.h" #include "lldb/Symbol/Type.h" #include "lldb/lldb-private.h" +#include "lldb/lldb-types.h" class PDBASTParser; @@ -363,6 +364,12 @@ class TypeSystem : public PluginInterface, lldb::opaque_compiler_type_t type, llvm::StringRef name, bool omit_empty_base_classes, std::vector &child_indexes) = 0; + virtual CompilerType + GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { +return CompilerType(); + } + virtual bool IsTemplateType(lldb::opaque_compiler_type_t type); virtual size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index be0ddb06f82c18..2621f682011b41 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -6997,6 +6997,36 @@ TypeSystemClang::GetIndexOfChildWithName(lldb::opaque_compiler_type_t type, return UINT32_MAX; } +CompilerType +TypeSystemClang::GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { + if (!type || name.empty()) +return CompilerType(); + + clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type)); + const clang::Type::TypeClass type_class = qual_type->getTypeClass(); + + switch (type_class) { + case clang::Type::Record: { +if (!GetCompleteType(type)) + return CompilerType(); +const clang::RecordType *record_type = +llvm::cast(qual_type.getTypePtr()); +const clang::RecordDecl *record_decl = record_type->getDecl(); + +clang::DeclarationName decl_name(&getASTContext().Idents.get(name)); +for (NamedDecl *decl : record_decl->lookup(decl_name)) { + if (auto *tag_decl = dyn_cast(decl)) +return GetType(getASTContext().getTagDeclType(tag_decl)); +} +break; + } + default: +break; + } + return CompilerType(); +} + bool TypeSystemClang::IsTemplateType(lldb::opaque_compiler_type_t type) { if (!type) return false; diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 05c303baa41640..68b82e9688f12b 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -897,6 +897,9 @@ class TypeSystemClang : public TypeSystem { bool omit_empty_base_classes, std::vector &child_indexes) override; + CompilerType GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) override; + bool IsTemplateType(lldb::opaque_compiler_type_t type) override; size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp index 44a24d7178f562..d9894ac355c6f0 100644 --- a/lldb/source/Symbol/Type.cpp +++ b/lldb/source/Symbol/Type.cpp @@ -1177,20 +1177,8 @@ CompilerType TypeImpl::FindDirectNestedType(llvm::StringRef name) { if (name.empty()) return CompilerType(); auto type_system = GetTypeSystem(/*prefer_dynamic*/ false); - auto *symbol_file = type_system->GetSymbolFile(); - if (!symbol_file) -return CompilerType(); - auto decl_context = type_system->GetCompilerDeclContextForType(m_static_type); - if (!decl_context.IsValid()) -return CompilerType(); - TypeQuery query(decl_conte
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/89183 >From 80ba4f24cdfe8b5f2aa44a016ea69ad08f56d558 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 18 Apr 2024 07:34:45 + Subject: [PATCH] [lldb] Make SBType::FindDirectNestedType work with expression ASTs The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once). --- lldb/include/lldb/Symbol/TypeSystem.h | 7 .../TypeSystem/Clang/TypeSystemClang.cpp | 30 +++ .../TypeSystem/Clang/TypeSystemClang.h| 3 ++ lldb/source/Symbol/Type.cpp | 16 +--- lldb/test/API/python_api/type/TestTypeList.py | 37 +++ 5 files changed, 63 insertions(+), 30 deletions(-) diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h index f647fcbf1636ea..3a927d313b823d 100644 --- a/lldb/include/lldb/Symbol/TypeSystem.h +++ b/lldb/include/lldb/Symbol/TypeSystem.h @@ -28,6 +28,7 @@ #include "lldb/Symbol/CompilerDeclContext.h" #include "lldb/Symbol/Type.h" #include "lldb/lldb-private.h" +#include "lldb/lldb-types.h" class PDBASTParser; @@ -363,6 +364,12 @@ class TypeSystem : public PluginInterface, lldb::opaque_compiler_type_t type, llvm::StringRef name, bool omit_empty_base_classes, std::vector &child_indexes) = 0; + virtual CompilerType + GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { +return CompilerType(); + } + virtual bool IsTemplateType(lldb::opaque_compiler_type_t type); virtual size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index be0ddb06f82c18..2621f682011b41 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -6997,6 +6997,36 @@ TypeSystemClang::GetIndexOfChildWithName(lldb::opaque_compiler_type_t type, return UINT32_MAX; } +CompilerType +TypeSystemClang::GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { + if (!type || name.empty()) +return CompilerType(); + + clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type)); + const clang::Type::TypeClass type_class = qual_type->getTypeClass(); + + switch (type_class) { + case clang::Type::Record: { +if (!GetCompleteType(type)) + return CompilerType(); +const clang::RecordType *record_type = +llvm::cast(qual_type.getTypePtr()); +const clang::RecordDecl *record_decl = record_type->getDecl(); + +clang::DeclarationName decl_name(&getASTContext().Idents.get(name)); +for (NamedDecl *decl : record_decl->lookup(decl_name)) { + if (auto *tag_decl = dyn_cast(decl)) +return GetType(getASTContext().getTagDeclType(tag_decl)); +} +break; + } + default: +break; + } + return CompilerType(); +} + bool TypeSystemClang::IsTemplateType(lldb::opaque_compiler_type_t type) { if (!type) return false; diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 05c303baa41640..68b82e9688f12b 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -897,6 +897,9 @@ class TypeSystemClang : public TypeSystem { bool omit_empty_base_classes, std::vector &child_indexes) override; + CompilerType GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) override; + bool IsTemplateType(lldb::opaque_compiler_type_t type) override; size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp index 44a24d7178f562..d9894ac355c6f0 100644 --- a/lldb/source/Symbol/Type.cpp +++ b/lldb/source/Symbol/Type.cpp @@ -1177,20 +1177,8 @@ CompilerType TypeImpl::FindDirectNestedType(llvm::StringRef name) { if (name.empty()) return CompilerType(); auto type_system = GetTypeSystem(/*prefer_dynamic*/ false); - auto *symbol_file = type_system->GetSymbolFile(); - if (!symbol_file) -return CompilerType(); - auto decl_context = type_system->GetCompilerDeclContextForType(m_static_type); - if (!decl_context.IsValid()) -return CompilerType(); - TypeQuery query(decl_context
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
Endilll wrote: > One thing I'd like to get feedback on is whether this should be looking into > base classes. The discussion on the original PR focuses on lexically nested > types, but going through base classes (following usual language rules) is > another form of nesting. Both the existing and the new implementations don't > do that, but I think it would be more natural to do it. Of course, then the > function can definitely return more than one result... What you're describing here is in line with member lookup described in the Standard. While it might be a useful facility, it's at odds with the intent of `FindDirectNestedTypes()` to be a small building block for efficient AST traversal. For the use case I have for this function, any amount of lookup is going to regress the performance of the formatter, and subsequently responsiveness of the debugger. So if you're going to pursue this, I suggest exposing this as a new function. https://github.com/llvm/llvm-project/pull/89183 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)
@@ -282,17 +282,10 @@ int main_platform(int argc, char *argv[]) { } } - do { -GDBRemoteCommunicationServerPlatform platform( -acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme()); - -if (port_offset > 0) DavidSpickett wrote: `port_offset` still needs to be set. https://github.com/llvm/llvm-project/pull/88845 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)
@@ -301,13 +294,35 @@ int main_platform(int argc, char *argv[]) { exit(socket_error); } printf("Connection established.\n"); + if (g_server) { // Collect child zombie processes. #if !defined(_WIN32) - while (waitpid(-1, nullptr, WNOHANG) > 0) -; + ::pid_t waitResult; + while ((waitResult = waitpid(-1, nullptr, WNOHANG)) > 0) { +// waitResult is the child pid +gdbserver_portmap.FreePortForProcess(waitResult); + } #endif - if (fork()) { + // TODO: Clean up portmap for Windows when children die + + // After collecting zombie ports, get the next available + GDBRemoteCommunicationServerPlatform::PortMap portmap_for_child; + llvm::Expected available_port = + gdbserver_portmap.GetNextAvailablePort(); + if (available_port) +portmap_for_child.AllowPort(*available_port); + else { +fprintf(stderr, +"no available gdbserver port for connection - dropping...\n"); DavidSpickett wrote: The else path needs to consume the error from llvm::Expected otherwise when the Expected destructs we'll get an assertion failure. https://github.com/llvm/llvm-project/pull/88845 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)
@@ -301,13 +294,35 @@ int main_platform(int argc, char *argv[]) { exit(socket_error); } printf("Connection established.\n"); + if (g_server) { // Collect child zombie processes. #if !defined(_WIN32) - while (waitpid(-1, nullptr, WNOHANG) > 0) -; + ::pid_t waitResult; + while ((waitResult = waitpid(-1, nullptr, WNOHANG)) > 0) { +// waitResult is the child pid +gdbserver_portmap.FreePortForProcess(waitResult); + } #endif - if (fork()) { + // TODO: Clean up portmap for Windows when children die DavidSpickett wrote: Please open an issue describing the problem in a bit more detail so we have some context if/when we get around to fixing it. Include an example of when this missing code would be a problem, doesn't have to be super detailed. Then add a link to that issue in the comment here. "lldb-server on Windows in platform mode does not reuse ports correctly", something like that. https://github.com/llvm/llvm-project/pull/88845 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)
@@ -301,13 +294,35 @@ int main_platform(int argc, char *argv[]) { exit(socket_error); } printf("Connection established.\n"); + if (g_server) { // Collect child zombie processes. #if !defined(_WIN32) - while (waitpid(-1, nullptr, WNOHANG) > 0) -; + ::pid_t waitResult; + while ((waitResult = waitpid(-1, nullptr, WNOHANG)) > 0) { +// waitResult is the child pid +gdbserver_portmap.FreePortForProcess(waitResult); + } #endif - if (fork()) { + // TODO: Clean up portmap for Windows when children die + + // After collecting zombie ports, get the next available + GDBRemoteCommunicationServerPlatform::PortMap portmap_for_child; + llvm::Expected available_port = + gdbserver_portmap.GetNextAvailablePort(); + if (available_port) +portmap_for_child.AllowPort(*available_port); + else { +fprintf(stderr, +"no available gdbserver port for connection - dropping...\n"); DavidSpickett wrote: llvm::consumeError can do this iirc. https://github.com/llvm/llvm-project/pull/88845 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
DavidSpickett wrote: @bulbazord please give the final approval if it looks good to you. We can sort out moving from MD5 on an issue. https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
Michael137 wrote: > > One thing I'd like to get feedback on is whether this should be looking > > into base classes. The discussion on the original PR focuses on lexically > > nested types, but going through base classes (following usual language > > rules) is another form of nesting. Both the existing and the new > > implementations don't do that, but I think it would be more natural to do > > it. Of course, then the function can definitely return more than one > > result... > > What you're describing here is in line with member lookup described in the > Standard. While it might be a useful facility, it's at odds with the intent > of `FindDirectNestedTypes()` to be a small building block for efficient AST > traversal. For the use case I have for this function, any amount of lookup is > going to regress the performance of the formatter, and subsequently > responsiveness of the debugger. So if you're going to pursue this, I suggest > exposing this as a new function. I don't expect the simple `DeclContext` lookup proposed here to be expensive. What *is* expensive from the data-formatters point of view is either full-blown expression evaluation or a global search in DWARF for some type. Re. looking at base classes, don't have a strong opinion on it. There's some precedent for looking at base classes already in `TypeSystemClang` (e.g., `GetIndexOfChildWithName`). But as you say @labath, that would require an API change https://github.com/llvm/llvm-project/pull/89183 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][lldb-dap] Cleanup breakpoint filters. (PR #87550)
@@ -36,9 +36,7 @@ DAP::DAP() {{"cpp_catch", "C++ Catch", lldb::eLanguageTypeC_plus_plus}, {"cpp_throw", "C++ Throw", lldb::eLanguageTypeC_plus_plus}, {"objc_catch", "Objective-C Catch", lldb::eLanguageTypeObjC}, - {"objc_throw", "Objective-C Throw", lldb::eLanguageTypeObjC}, - {"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift}, - {"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}), + {"objc_throw", "Objective-C Throw", lldb::eLanguageTypeObjC}}), walter-erquinigo wrote: Btw, it's possible to register typesystems/languages at initialization via runtime plugins. I think it would be ideal if the breakpoint list could get updated/refreshed after the target has been loaded (maybe at the first stop point) https://github.com/llvm/llvm-project/pull/87550 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
labath wrote: > > > One thing I'd like to get feedback on is whether this should be looking > > > into base classes. The discussion on the original PR focuses on lexically > > > nested types, but going through base classes (following usual language > > > rules) is another form of nesting. Both the existing and the new > > > implementations don't do that, but I think it would be more natural to do > > > it. Of course, then the function can definitely return more than one > > > result... > > > > > > What you're describing here is in line with member lookup described in the > > Standard. While it might be a useful facility, it's at odds with the intent > > of `FindDirectNestedTypes()` to be a small building block for efficient AST > > traversal. For the use case I have for this function, any amount of lookup > > is going to regress the performance of the formatter, and subsequently > > responsiveness of the debugger. So if you're going to pursue this, I > > suggest exposing this as a new function. > > I don't expect the simple `DeclContext` lookup proposed here to be expensive. > What _is_ expensive from the data-formatters point of view is either > full-blown expression evaluation or a global search in DWARF for some type. Agreed. This should be the same lookup that clang does every time is sees a `::`. Going through (potentially many) calls to FindTypes would be a different story. The reason I was asking is because I was not sure if this behavior was just a side-effect of how the original implementation worked, or if it was the intention all along. Judging by the response, it's the latter. > > Re. looking at base classes, don't have a strong opinion on it. There's some > precedent for looking at base classes already in `TypeSystemClang` (e.g., > `GetIndexOfChildWithName`). But as you say @labath, that would require an API > change That's makes sense. Even if it was not the intention, the API makes this the only reasonable implementation. I have no use for the base-class traversal (may change in the future). If there's is a need for it, we can add a different API. https://github.com/llvm/llvm-project/pull/89183 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
https://github.com/Michael137 approved this pull request. https://github.com/llvm/llvm-project/pull/89183 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Don't fail when SBProcess::GetMemoryRegionInfo returns error. (PR #87649)
ZequanWu wrote: Ping. https://github.com/llvm/llvm-project/pull/87649 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a1e7c83 - [lldb] Disable break at _dl_debug_state test on arm
Author: Zequan Wu Date: 2024-04-18T10:40:35-04:00 New Revision: a1e7c83af11ee111994ec19029494e6e9ea97dbd URL: https://github.com/llvm/llvm-project/commit/a1e7c83af11ee111994ec19029494e6e9ea97dbd DIFF: https://github.com/llvm/llvm-project/commit/a1e7c83af11ee111994ec19029494e6e9ea97dbd.diff LOG: [lldb] Disable break at _dl_debug_state test on arm Added: Modified: lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py Removed: diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py index 850235fdcefa70..b6bdd2d6041935 100644 --- a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py @@ -672,7 +672,7 @@ def test_breakpoint_statistics_hitcount(self): for breakpoint_stats in breakpoints_stats: self.assertIn("hitCount", breakpoint_stats) -@skipIf(oslist=no_match(["linux"])) +@skipIf(oslist=no_match(["linux"]), archs=["arm", "aarch64"]) def test_break_at__dl_debug_state(self): """ Test lldb is able to stop at _dl_debug_state if it is set before the ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] d3993ac - [lldb][test] Correctly fix break at _dl_debug_state test on arm.
Author: Zequan Wu Date: 2024-04-18T11:13:17-04:00 New Revision: d3993ac1890731d2b24543646961c95680788207 URL: https://github.com/llvm/llvm-project/commit/d3993ac1890731d2b24543646961c95680788207 DIFF: https://github.com/llvm/llvm-project/commit/d3993ac1890731d2b24543646961c95680788207.diff LOG: [lldb][test] Correctly fix break at _dl_debug_state test on arm. If lldb finds the dynamic linker in the search path or if the binary is linked staticlly, it will fail at `lldbutil.run_break_set_by_symbol` because the breakpoint is resolved. Otherwise, it's not resolved at this point. But we don't care if it's resolved or not. This test cares about if the breakpoint is hit or not after launching. This changes the num_expected_locations to -2, which means don't assert on if this breakpoint resolved or not. Added: Modified: lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py Removed: diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py index b6bdd2d6041935..c219a4ee5bd9c6 100644 --- a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py @@ -672,7 +672,7 @@ def test_breakpoint_statistics_hitcount(self): for breakpoint_stats in breakpoints_stats: self.assertIn("hitCount", breakpoint_stats) -@skipIf(oslist=no_match(["linux"]), archs=["arm", "aarch64"]) +@skipIf(oslist=no_match(["linux"])) def test_break_at__dl_debug_state(self): """ Test lldb is able to stop at _dl_debug_state if it is set before the @@ -682,7 +682,7 @@ def test_break_at__dl_debug_state(self): exe = self.getBuildArtifact("a.out") self.runCmd("target create %s" % exe) bpid = lldbutil.run_break_set_by_symbol( -self, "_dl_debug_state", num_expected_locations=0 +self, "_dl_debug_state", num_expected_locations=-2 ) self.runCmd("run") self.assertIsNotNone( ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 6870ac2 - Fix saving minidump from lldb-dap (#89113)
Author: jeffreytan81 Date: 2024-04-18T09:24:33-07:00 New Revision: 6870ac201f9f50a13313213fea7a14b198d7280a URL: https://github.com/llvm/llvm-project/commit/6870ac201f9f50a13313213fea7a14b198d7280a DIFF: https://github.com/llvm/llvm-project/commit/6870ac201f9f50a13313213fea7a14b198d7280a.diff LOG: Fix saving minidump from lldb-dap (#89113) There are users reporting saving minidump from lldb-dap does not work. Turns out our stack trace request always evaluate a function call which caused JIT object file like "__lldb_caller_function" to be created which can fail minidump builder to get its module size. This patch fixes "getModuleFileSize" for ObjectFileJIT so that module list can be saved. I decided to create a lldb-dap test so that this end-to-end functionality can be validated from our tests (instead of only command line lldb). The patch also improves several other small things in the workflow: 1. It logs any minidump stream failure so that it is easier to find out what stream saving fails. In future, we should show process and error to end users. 2. It handles error from "getModuleFileSize" llvm::Expected otherwise it will complain the error is not handled. - Co-authored-by: jeffreytan81 Added: lldb/test/API/tools/lldb-dap/save-core/Makefile lldb/test/API/tools/lldb-dap/save-core/TestDAP_save_core.py lldb/test/API/tools/lldb-dap/save-core/main.cpp Modified: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp Removed: diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 50d1b563f469cf..cefd4cb22b6bae 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -20,6 +20,7 @@ #include "lldb/Target/StopInfo.h" #include "lldb/Target/ThreadList.h" #include "lldb/Utility/DataExtractor.h" +#include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/RegisterValue.h" #include "llvm/ADT/StringRef.h" @@ -154,8 +155,16 @@ Status WriteString(const std::string &to_write, llvm::Expected getModuleFileSize(Target &target, const ModuleSP &mod) { - SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection(); + // JIT module has the same vm and file size. uint64_t SizeOfImage = 0; + if (mod->GetObjectFile()->CalculateType() == ObjectFile::Type::eTypeJIT) { +for (const auto §ion : *mod->GetObjectFile()->GetSectionList()) { + SizeOfImage += section->GetByteSize(); +} +return SizeOfImage; + } + + SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection(); if (!sect_sp) { return llvm::createStringError(std::errc::operation_not_supported, @@ -226,8 +235,13 @@ Status MinidumpFileBuilder::AddModuleList(Target &target) { std::string module_name = mod->GetSpecificationDescription(); auto maybe_mod_size = getModuleFileSize(target, mod); if (!maybe_mod_size) { - error.SetErrorStringWithFormat("Unable to get the size of module %s.", - module_name.c_str()); + llvm::Error mod_size_err = maybe_mod_size.takeError(); + llvm::handleAllErrors(std::move(mod_size_err), +[&](const llvm::ErrorInfoBase &E) { + error.SetErrorStringWithFormat( + "Unable to get the size of module %s: %s.", + module_name.c_str(), E.message().c_str()); +}); return error; } diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp index fe609c7f3d2001..1af5d99f0b1604 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -14,6 +14,7 @@ #include "lldb/Core/PluginManager.h" #include "lldb/Core/Section.h" #include "lldb/Target/Process.h" +#include "lldb/Utility/LLDBLog.h" #include "llvm/Support/FileSystem.h" @@ -68,26 +69,35 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, Target &target = process_sp->GetTarget(); + Log *log = GetLog(LLDBLog::Object); error = builder.AddSystemInfo(target.GetArchitecture().GetTriple()); - if (error.Fail()) + if (error.Fail()) { +LLDB_LOG(log, "AddSystemInfo failed: %s", error.AsCString()); return false; + } error = builder.AddModuleList(target); - if (error.Fail()) + if (error.Fail()) { +LLDB_LOG(log, "AddModuleList failed: %s", error.AsCString()); return false; + } builder.AddMiscInfo(process_sp); error = builder.AddThreadList(process_sp); - if (error.Fail())
[Lldb-commits] [lldb] Fix saving minidump from lldb-dap (PR #89113)
https://github.com/jeffreytan81 closed https://github.com/llvm/llvm-project/pull/89113 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [lldb][test] Remove LLDB_TEST_USE_VENDOR_PACKAGES (PR #89260)
https://github.com/rupprecht created https://github.com/llvm/llvm-project/pull/89260 The `LLDB_TEST_USE_VENDOR_PACKAGES` has defaulted to `Off` for a while. Either installing `pexpect` or skipping those tests with `-DLLDB_TEST_USER_ARGS=--skip-category=pexpect` seems to be enough that we can fully remove this option. This patch removes the `LLDB_TEST_USE_VENDOR_PACKAGES` cmake configuration as well as the associated code to add `third_party/Python/module` to the python path. I'll do the actual deletion of `third_party/Python/module` in a followup PR in the (unlikely, I hope) event this commit needs to be reverted. >From cf75e576ea9a069f7429f68739362780a0171ea9 Mon Sep 17 00:00:00 2001 From: Jordan Rupprecht Date: Thu, 18 Apr 2024 16:02:37 + Subject: [PATCH] [lldb][test] Remove LLDB_TEST_USE_VENDOR_PACKAGES The `LLDB_TEST_USE_VENDOR_PACKAGES` has defaulted to `Off` for a while. Either installing `pexpect` or skipping those tests with `-DLLDB_TEST_USER_ARGS=--skip-category=pexpect` seems to be enough that we can fully remove this option. This patch removes the `LLDB_TEST_USE_VENDOR_PACKAGES` cmake configuration as well as the associated code to add `third_party/Python/module` to the python path. I'll do the actual deletion of `third_party/Python/module` in a followup PR in the (unlikely, I hope) event this commit needs to be reverted. --- clang/cmake/caches/Fuchsia.cmake | 1 - lldb/cmake/modules/LLDBConfig.cmake| 2 -- .../Python/lldbsuite/test/lldb_pylint_helper.py| 8 lldb/test/API/lit.cfg.py | 3 --- lldb/test/API/lit.site.cfg.py.in | 1 - lldb/test/CMakeLists.txt | 3 +-- lldb/use_lldb_suite_root.py| 14 -- lldb/utils/lldb-dotest/CMakeLists.txt | 1 - lldb/utils/lldb-dotest/lldb-dotest.in | 4 llvm/utils/gn/secondary/lldb/test/BUILD.gn | 1 - 10 files changed, 1 insertion(+), 37 deletions(-) diff --git a/clang/cmake/caches/Fuchsia.cmake b/clang/cmake/caches/Fuchsia.cmake index 393d97a4cf1a33..30a3b9116a461f 100644 --- a/clang/cmake/caches/Fuchsia.cmake +++ b/clang/cmake/caches/Fuchsia.cmake @@ -65,7 +65,6 @@ set(_FUCHSIA_BOOTSTRAP_PASSTHROUGH LLDB_EMBED_PYTHON_HOME LLDB_PYTHON_HOME LLDB_PYTHON_RELATIVE_PATH - LLDB_TEST_USE_VENDOR_PACKAGES LLDB_TEST_USER_ARGS Python3_EXECUTABLE Python3_LIBRARIES diff --git a/lldb/cmake/modules/LLDBConfig.cmake b/lldb/cmake/modules/LLDBConfig.cmake index 5d62213c3f5838..a758261073ac57 100644 --- a/lldb/cmake/modules/LLDBConfig.cmake +++ b/lldb/cmake/modules/LLDBConfig.cmake @@ -67,8 +67,6 @@ option(LLDB_SKIP_STRIP "Whether to skip stripping of binaries when installing ll option(LLDB_SKIP_DSYM "Whether to skip generating a dSYM when installing lldb." OFF) option(LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS "Fail to configure if certain requirements are not met for testing." OFF) -option(LLDB_TEST_USE_VENDOR_PACKAGES - "Use packages from lldb/third_party/Python/module instead of system deps." OFF) set(LLDB_GLOBAL_INIT_DIRECTORY "" CACHE STRING "Path to the global lldbinit directory. Relative paths are resolved relative to the diff --git a/lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py b/lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py index 3b746c3f9242df..2558be1364d9be 100644 --- a/lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py +++ b/lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py @@ -157,14 +157,6 @@ def child_dirs(parent_dir): 0, os.path.join(packages_python_child_dir, "test_runner", "lib") ) -# Handle third_party module/package directory. -third_party_module_dir = os.path.join( -check_dir, "third_party", "Python", "module" -) -for child_dir in child_dirs(third_party_module_dir): -# Yes, we embed the module in the module parent dir -sys.path.insert(0, child_dir) - # We're done. break diff --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py index 9ea389c639a013..9d6775917e1370 100644 --- a/lldb/test/API/lit.cfg.py +++ b/lldb/test/API/lit.cfg.py @@ -310,6 +310,3 @@ def delete_module_cache(path): # Propagate XDG_CACHE_HOME if "XDG_CACHE_HOME" in os.environ: config.environment["XDG_CACHE_HOME"] = os.environ["XDG_CACHE_HOME"] - -if is_configured("use_vendor_packages"): -config.environment["LLDB_TEST_USE_VENDOR_PACKAGES"] = "1" diff --git a/lldb/test/API/lit.site.cfg.py.in b/lldb/test/API/lit.site.cfg.py.in index c2602acd2ef85a..053331dc4881f7 100644 --- a/lldb/test/API/lit.site.cfg.py.in +++ b/lldb/test/API/lit.site.cfg.py.in @@ -38,7 +38,6 @@ config.libcxx_include_target_dir = "@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@" # The API tests use their own module caches. config.lldb_module_cache = os.path.join("@LLDB_TEST_
[Lldb-commits] [clang] [lldb] [llvm] [lldb][test] Remove LLDB_TEST_USE_VENDOR_PACKAGES (PR #89260)
llvmbot wrote: @llvm/pr-subscribers-clang @llvm/pr-subscribers-lldb Author: Jordan Rupprecht (rupprecht) Changes The `LLDB_TEST_USE_VENDOR_PACKAGES` has defaulted to `Off` for a while. Either installing `pexpect` or skipping those tests with `-DLLDB_TEST_USER_ARGS=--skip-category=pexpect` seems to be enough that we can fully remove this option. This patch removes the `LLDB_TEST_USE_VENDOR_PACKAGES` cmake configuration as well as the associated code to add `third_party/Python/module` to the python path. I'll do the actual deletion of `third_party/Python/module` in a followup PR in the (unlikely, I hope) event this commit needs to be reverted. --- Full diff: https://github.com/llvm/llvm-project/pull/89260.diff 10 Files Affected: - (modified) clang/cmake/caches/Fuchsia.cmake (-1) - (modified) lldb/cmake/modules/LLDBConfig.cmake (-2) - (modified) lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py (-8) - (modified) lldb/test/API/lit.cfg.py (-3) - (modified) lldb/test/API/lit.site.cfg.py.in (-1) - (modified) lldb/test/CMakeLists.txt (+1-2) - (modified) lldb/use_lldb_suite_root.py (-14) - (modified) lldb/utils/lldb-dotest/CMakeLists.txt (-1) - (modified) lldb/utils/lldb-dotest/lldb-dotest.in (-4) - (modified) llvm/utils/gn/secondary/lldb/test/BUILD.gn (-1) ``diff diff --git a/clang/cmake/caches/Fuchsia.cmake b/clang/cmake/caches/Fuchsia.cmake index 393d97a4cf1a33..30a3b9116a461f 100644 --- a/clang/cmake/caches/Fuchsia.cmake +++ b/clang/cmake/caches/Fuchsia.cmake @@ -65,7 +65,6 @@ set(_FUCHSIA_BOOTSTRAP_PASSTHROUGH LLDB_EMBED_PYTHON_HOME LLDB_PYTHON_HOME LLDB_PYTHON_RELATIVE_PATH - LLDB_TEST_USE_VENDOR_PACKAGES LLDB_TEST_USER_ARGS Python3_EXECUTABLE Python3_LIBRARIES diff --git a/lldb/cmake/modules/LLDBConfig.cmake b/lldb/cmake/modules/LLDBConfig.cmake index 5d62213c3f5838..a758261073ac57 100644 --- a/lldb/cmake/modules/LLDBConfig.cmake +++ b/lldb/cmake/modules/LLDBConfig.cmake @@ -67,8 +67,6 @@ option(LLDB_SKIP_STRIP "Whether to skip stripping of binaries when installing ll option(LLDB_SKIP_DSYM "Whether to skip generating a dSYM when installing lldb." OFF) option(LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS "Fail to configure if certain requirements are not met for testing." OFF) -option(LLDB_TEST_USE_VENDOR_PACKAGES - "Use packages from lldb/third_party/Python/module instead of system deps." OFF) set(LLDB_GLOBAL_INIT_DIRECTORY "" CACHE STRING "Path to the global lldbinit directory. Relative paths are resolved relative to the diff --git a/lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py b/lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py index 3b746c3f9242df..2558be1364d9be 100644 --- a/lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py +++ b/lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py @@ -157,14 +157,6 @@ def child_dirs(parent_dir): 0, os.path.join(packages_python_child_dir, "test_runner", "lib") ) -# Handle third_party module/package directory. -third_party_module_dir = os.path.join( -check_dir, "third_party", "Python", "module" -) -for child_dir in child_dirs(third_party_module_dir): -# Yes, we embed the module in the module parent dir -sys.path.insert(0, child_dir) - # We're done. break diff --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py index 9ea389c639a013..9d6775917e1370 100644 --- a/lldb/test/API/lit.cfg.py +++ b/lldb/test/API/lit.cfg.py @@ -310,6 +310,3 @@ def delete_module_cache(path): # Propagate XDG_CACHE_HOME if "XDG_CACHE_HOME" in os.environ: config.environment["XDG_CACHE_HOME"] = os.environ["XDG_CACHE_HOME"] - -if is_configured("use_vendor_packages"): -config.environment["LLDB_TEST_USE_VENDOR_PACKAGES"] = "1" diff --git a/lldb/test/API/lit.site.cfg.py.in b/lldb/test/API/lit.site.cfg.py.in index c2602acd2ef85a..053331dc4881f7 100644 --- a/lldb/test/API/lit.site.cfg.py.in +++ b/lldb/test/API/lit.site.cfg.py.in @@ -38,7 +38,6 @@ config.libcxx_include_target_dir = "@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@" # The API tests use their own module caches. config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-api") config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-api") -config.use_vendor_packages = @LLDB_TEST_USE_VENDOR_PACKAGES@ # Plugins lldb_build_intel_pt = '@LLDB_BUILD_INTEL_PT@' diff --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt index b6ec5bc4d819e3..6a9ca59f96b0f8 100644 --- a/lldb/test/CMakeLists.txt +++ b/lldb/test/CMakeLists.txt @@ -245,8 +245,7 @@ llvm_canonicalize_cmake_booleans( LLDB_HAS_LIBCXX LLDB_TOOL_LLDB_SERVER_BUILD LLDB_USE_SYSTEM_DEBUGSERVER - LLDB_IS_64_BITS - LLDB_TEST_USE_VENDOR_PACKAGES) + LLDB_IS_64_BITS) # Configure the individual test suites. add_subdirectory(API) diff --git a/lldb/use_lldb_suite_
[Lldb-commits] [lldb] 4681079 - adds additional information to the ProcessInfo object for elf processes (#88995)
Author: Fred Grim Date: 2024-04-18T09:46:58-07:00 New Revision: 468107904098e0b5d32db710db80907befde614e URL: https://github.com/llvm/llvm-project/commit/468107904098e0b5d32db710db80907befde614e DIFF: https://github.com/llvm/llvm-project/commit/468107904098e0b5d32db710db80907befde614e.diff LOG: adds additional information to the ProcessInfo object for elf processes (#88995) This adds some additional bits into a ProcessInfo structure that will be of use in filling structs in an elf core file. This is a demand for implementing process save-core Added: Modified: lldb/include/lldb/Utility/ProcessInfo.h lldb/source/Host/linux/Host.cpp lldb/unittests/Host/linux/HostTest.cpp Removed: diff --git a/lldb/include/lldb/Utility/ProcessInfo.h b/lldb/include/lldb/Utility/ProcessInfo.h index 7fb5b37be0f48f..e9fe71e1b851d1 100644 --- a/lldb/include/lldb/Utility/ProcessInfo.h +++ b/lldb/include/lldb/Utility/ProcessInfo.h @@ -139,6 +139,11 @@ class ProcessInfo { // to that process. class ProcessInstanceInfo : public ProcessInfo { public: + struct timespec { +time_t tv_sec = 0; +long int tv_usec = 0; + }; + ProcessInstanceInfo() = default; ProcessInstanceInfo(const char *name, const ArchSpec &arch, lldb::pid_t pid) @@ -172,6 +177,66 @@ class ProcessInstanceInfo : public ProcessInfo { return m_parent_pid != LLDB_INVALID_PROCESS_ID; } + lldb::pid_t GetProcessGroupID() const { return m_process_group_id; } + + void SetProcessGroupID(lldb::pid_t pgrp) { m_process_group_id = pgrp; } + + bool ProcessGroupIDIsValid() const { +return m_process_group_id != LLDB_INVALID_PROCESS_ID; + } + + lldb::pid_t GetProcessSessionID() const { return m_process_session_id; } + + void SetProcessSessionID(lldb::pid_t session) { +m_process_session_id = session; + } + + bool ProcessSessionIDIsValid() const { +return m_process_session_id != LLDB_INVALID_PROCESS_ID; + } + + struct timespec GetUserTime() const { return m_user_time; } + + void SetUserTime(struct timespec utime) { m_user_time = utime; } + + bool UserTimeIsValid() const { +return m_user_time.tv_sec > 0 || m_user_time.tv_usec > 0; + } + + struct timespec GetSystemTime() const { return m_system_time; } + + void SetSystemTime(struct timespec stime) { m_system_time = stime; } + + bool SystemTimeIsValid() const { +return m_system_time.tv_sec > 0 || m_system_time.tv_usec > 0; + } + + struct timespec GetCumulativeUserTime() const { +return m_cumulative_user_time; + } + + void SetCumulativeUserTime(struct timespec cutime) { +m_cumulative_user_time = cutime; + } + + bool CumulativeUserTimeIsValid() const { +return m_cumulative_user_time.tv_sec > 0 || + m_cumulative_user_time.tv_usec > 0; + } + + struct timespec GetCumulativeSystemTime() const { +return m_cumulative_system_time; + } + + void SetCumulativeSystemTime(struct timespec cstime) { +m_cumulative_system_time = cstime; + } + + bool CumulativeSystemTimeIsValid() const { +return m_cumulative_system_time.tv_sec > 0 || + m_cumulative_system_time.tv_sec > 0; + } + void Dump(Stream &s, UserIDResolver &resolver) const; static void DumpTableHeader(Stream &s, bool show_args, bool verbose); @@ -183,6 +248,12 @@ class ProcessInstanceInfo : public ProcessInfo { uint32_t m_euid = UINT32_MAX; uint32_t m_egid = UINT32_MAX; lldb::pid_t m_parent_pid = LLDB_INVALID_PROCESS_ID; + lldb::pid_t m_process_group_id = LLDB_INVALID_PROCESS_ID; + lldb::pid_t m_process_session_id = LLDB_INVALID_PROCESS_ID; + struct timespec m_user_time {}; + struct timespec m_system_time {}; + struct timespec m_cumulative_user_time {}; + struct timespec m_cumulative_system_time {}; }; typedef std::vector ProcessInstanceInfoList; diff --git a/lldb/source/Host/linux/Host.cpp b/lldb/source/Host/linux/Host.cpp index 6c57384aa38a13..c6490f2fc9e2f5 100644 --- a/lldb/source/Host/linux/Host.cpp +++ b/lldb/source/Host/linux/Host.cpp @@ -49,6 +49,29 @@ enum class ProcessState { TracedOrStopped, Zombie, }; + +constexpr int task_comm_len = 16; + +struct StatFields { + ::pid_t pid = LLDB_INVALID_PROCESS_ID; + char comm[task_comm_len]; + char state; + ::pid_t ppid = LLDB_INVALID_PROCESS_ID; + ::pid_t pgrp = LLDB_INVALID_PROCESS_ID; + ::pid_t session = LLDB_INVALID_PROCESS_ID; + int tty_nr; + int tpgid; + unsigned flags; + long unsigned minflt; + long unsigned cminflt; + long unsigned majflt; + long unsigned cmajflt; + long unsigned utime; + long unsigned stime; + long cutime; + long cstime; + // other things. We don't need them below +}; } namespace lldb_private { @@ -60,11 +83,92 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo &ProcessInfo, ::pid_t &Tgid) { Log *log = GetLog(LLDBLog::Host); - auto BufferOrError = getProcFile(Pid, "status"); + auto Buffer
[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)
https://github.com/feg208 closed https://github.com/llvm/llvm-project/pull/88995 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] adds additional information to the ProcessInfo object for elf processes (PR #88995)
JDevlieghere wrote: Can we get (a subset) of this information on Darwin? I understand it's only necessary for ELF core files, and from the discussion that there's no standard POSIX way to get this, but it still feels a bit unfortunate to implement all these methods but only implement this for one specific platform. https://github.com/llvm/llvm-project/pull/88995 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
https://github.com/bulbazord approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] Revert "NFC: Make clang resource headers an interface library (#88317)" (PR #89266)
https://github.com/etcwilde created https://github.com/llvm/llvm-project/pull/89266 This reverts commit 8d468c132eed7ffe34d601b224220efd51655eb3. @ayermolo reports seeing the following error when the clang-resource-headers are a library instead of a custom target: ``` CMake Error at CMakeLists.txt:86 (get_property): get_property could not find TARGET clang-resource-headers. Perhaps it has not yet been created. ``` I don't see this line in the LLVM repository, and I'm not sure how it worked as a custom target since this change doesn't affect whether or not the CMake target is generated, nor does it impact the order that targets are created in. >From 9b16756a69ddbce1e586535e931f7c2b923ff2ba Mon Sep 17 00:00:00 2001 From: Evan Wilde Date: Thu, 18 Apr 2024 09:58:52 -0700 Subject: [PATCH 1/2] Revert "NFC: Make clang resource headers an interface library (#88317)" This reverts commit 8d468c132eed7ffe34d601b224220efd51655eb3. --- clang/lib/Headers/CMakeLists.txt | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/clang/lib/Headers/CMakeLists.txt b/clang/lib/Headers/CMakeLists.txt index e6ae4e19e81db9..97104ccd8db59c 100644 --- a/clang/lib/Headers/CMakeLists.txt +++ b/clang/lib/Headers/CMakeLists.txt @@ -437,14 +437,14 @@ foreach( f ${generated_files} ) endforeach( f ) function(add_header_target target_name file_list) - add_library(${target_name} INTERFACE ${file_list}) + add_custom_target(${target_name} DEPENDS ${file_list}) set_target_properties(${target_name} PROPERTIES FOLDER "Misc" RUNTIME_OUTPUT_DIRECTORY "${output_dir}") endfunction() # The catch-all clang-resource-headers target -add_library(clang-resource-headers INTERFACE ${out_files}) +add_custom_target("clang-resource-headers" ALL DEPENDS ${out_files}) set_target_properties("clang-resource-headers" PROPERTIES FOLDER "Misc" RUNTIME_OUTPUT_DIRECTORY "${output_dir}") @@ -501,10 +501,6 @@ add_header_target("windows-resource-headers" ${windows_only_files}) add_header_target("utility-resource-headers" ${utility_files}) get_clang_resource_dir(header_install_dir SUBDIR include) -target_include_directories(clang-resource-headers INTERFACE - $ - $) -set_property(GLOBAL APPEND PROPERTY CLANG_EXPORTS clang-resource-headers) # # Install rules for the catch-all clang-resource-headers target >From 216f7532bc9160634a338f1ff09c06b911f1ff53 Mon Sep 17 00:00:00 2001 From: Evan Wilde Date: Thu, 18 Apr 2024 09:59:16 -0700 Subject: [PATCH 2/2] Revert "[lldb] Fix the standalone Xcode build after #88317" This reverts commit a855eea7fe86ef09a87f6251b3b711b821ae32bf. --- lldb/cmake/modules/LLDBFramework.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/cmake/modules/LLDBFramework.cmake b/lldb/cmake/modules/LLDBFramework.cmake index f915839f6b45a5..81fc596ef4244e 100644 --- a/lldb/cmake/modules/LLDBFramework.cmake +++ b/lldb/cmake/modules/LLDBFramework.cmake @@ -119,7 +119,7 @@ add_custom_command(TARGET liblldb POST_BUILD if(NOT APPLE_EMBEDDED) if (TARGET clang-resource-headers) add_dependencies(liblldb clang-resource-headers) -set(clang_resource_headers_dir $) +set(clang_resource_headers_dir $) else() set(clang_resource_headers_dir ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}/include) if(NOT EXISTS ${clang_resource_headers_dir}) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] Revert "NFC: Make clang resource headers an interface library (#88317)" (PR #89266)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Evan Wilde (etcwilde) Changes This reverts commit 8d468c132eed7ffe34d601b224220efd51655eb3. @ayermolo reports seeing the following error when the clang-resource-headers are a library instead of a custom target: ``` CMake Error at CMakeLists.txt:86 (get_property): get_property could not find TARGET clang-resource-headers. Perhaps it has not yet been created. ``` I don't see this line in the LLVM repository, and I'm not sure how it worked as a custom target since this change doesn't affect whether or not the CMake target is generated, nor does it impact the order that targets are created in. --- Full diff: https://github.com/llvm/llvm-project/pull/89266.diff 2 Files Affected: - (modified) clang/lib/Headers/CMakeLists.txt (+2-6) - (modified) lldb/cmake/modules/LLDBFramework.cmake (+1-1) ``diff diff --git a/clang/lib/Headers/CMakeLists.txt b/clang/lib/Headers/CMakeLists.txt index e6ae4e19e81db9..97104ccd8db59c 100644 --- a/clang/lib/Headers/CMakeLists.txt +++ b/clang/lib/Headers/CMakeLists.txt @@ -437,14 +437,14 @@ foreach( f ${generated_files} ) endforeach( f ) function(add_header_target target_name file_list) - add_library(${target_name} INTERFACE ${file_list}) + add_custom_target(${target_name} DEPENDS ${file_list}) set_target_properties(${target_name} PROPERTIES FOLDER "Misc" RUNTIME_OUTPUT_DIRECTORY "${output_dir}") endfunction() # The catch-all clang-resource-headers target -add_library(clang-resource-headers INTERFACE ${out_files}) +add_custom_target("clang-resource-headers" ALL DEPENDS ${out_files}) set_target_properties("clang-resource-headers" PROPERTIES FOLDER "Misc" RUNTIME_OUTPUT_DIRECTORY "${output_dir}") @@ -501,10 +501,6 @@ add_header_target("windows-resource-headers" ${windows_only_files}) add_header_target("utility-resource-headers" ${utility_files}) get_clang_resource_dir(header_install_dir SUBDIR include) -target_include_directories(clang-resource-headers INTERFACE - $ - $) -set_property(GLOBAL APPEND PROPERTY CLANG_EXPORTS clang-resource-headers) # # Install rules for the catch-all clang-resource-headers target diff --git a/lldb/cmake/modules/LLDBFramework.cmake b/lldb/cmake/modules/LLDBFramework.cmake index f915839f6b45a5..81fc596ef4244e 100644 --- a/lldb/cmake/modules/LLDBFramework.cmake +++ b/lldb/cmake/modules/LLDBFramework.cmake @@ -119,7 +119,7 @@ add_custom_command(TARGET liblldb POST_BUILD if(NOT APPLE_EMBEDDED) if (TARGET clang-resource-headers) add_dependencies(liblldb clang-resource-headers) -set(clang_resource_headers_dir $) +set(clang_resource_headers_dir $) else() set(clang_resource_headers_dir ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}/include) if(NOT EXISTS ${clang_resource_headers_dir}) `` https://github.com/llvm/llvm-project/pull/89266 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] Revert "NFC: Make clang resource headers an interface library (#88317)" (PR #89266)
llvmbot wrote: @llvm/pr-subscribers-lldb @llvm/pr-subscribers-backend-x86 Author: Evan Wilde (etcwilde) Changes This reverts commit 8d468c132eed7ffe34d601b224220efd51655eb3. @ayermolo reports seeing the following error when the clang-resource-headers are a library instead of a custom target: ``` CMake Error at CMakeLists.txt:86 (get_property): get_property could not find TARGET clang-resource-headers. Perhaps it has not yet been created. ``` I don't see this line in the LLVM repository, and I'm not sure how it worked as a custom target since this change doesn't affect whether or not the CMake target is generated, nor does it impact the order that targets are created in. --- Full diff: https://github.com/llvm/llvm-project/pull/89266.diff 2 Files Affected: - (modified) clang/lib/Headers/CMakeLists.txt (+2-6) - (modified) lldb/cmake/modules/LLDBFramework.cmake (+1-1) ``diff diff --git a/clang/lib/Headers/CMakeLists.txt b/clang/lib/Headers/CMakeLists.txt index e6ae4e19e81db9..97104ccd8db59c 100644 --- a/clang/lib/Headers/CMakeLists.txt +++ b/clang/lib/Headers/CMakeLists.txt @@ -437,14 +437,14 @@ foreach( f ${generated_files} ) endforeach( f ) function(add_header_target target_name file_list) - add_library(${target_name} INTERFACE ${file_list}) + add_custom_target(${target_name} DEPENDS ${file_list}) set_target_properties(${target_name} PROPERTIES FOLDER "Misc" RUNTIME_OUTPUT_DIRECTORY "${output_dir}") endfunction() # The catch-all clang-resource-headers target -add_library(clang-resource-headers INTERFACE ${out_files}) +add_custom_target("clang-resource-headers" ALL DEPENDS ${out_files}) set_target_properties("clang-resource-headers" PROPERTIES FOLDER "Misc" RUNTIME_OUTPUT_DIRECTORY "${output_dir}") @@ -501,10 +501,6 @@ add_header_target("windows-resource-headers" ${windows_only_files}) add_header_target("utility-resource-headers" ${utility_files}) get_clang_resource_dir(header_install_dir SUBDIR include) -target_include_directories(clang-resource-headers INTERFACE - $ - $) -set_property(GLOBAL APPEND PROPERTY CLANG_EXPORTS clang-resource-headers) # # Install rules for the catch-all clang-resource-headers target diff --git a/lldb/cmake/modules/LLDBFramework.cmake b/lldb/cmake/modules/LLDBFramework.cmake index f915839f6b45a5..81fc596ef4244e 100644 --- a/lldb/cmake/modules/LLDBFramework.cmake +++ b/lldb/cmake/modules/LLDBFramework.cmake @@ -119,7 +119,7 @@ add_custom_command(TARGET liblldb POST_BUILD if(NOT APPLE_EMBEDDED) if (TARGET clang-resource-headers) add_dependencies(liblldb clang-resource-headers) -set(clang_resource_headers_dir $) +set(clang_resource_headers_dir $) else() set(clang_resource_headers_dir ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}/include) if(NOT EXISTS ${clang_resource_headers_dir}) `` https://github.com/llvm/llvm-project/pull/89266 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] this test is flakey on arm in linux because I got too clever (PR #89267)
https://github.com/feg208 created https://github.com/llvm/llvm-project/pull/89267 the assembly jazz doesn't work on arm. oops >From 13e982588d75b240ce515c8ff1b2d9c89a595908 Mon Sep 17 00:00:00 2001 From: Fred Grim Date: Thu, 18 Apr 2024 10:13:03 -0700 Subject: [PATCH] this test is flakey on arm in linux because I got too clever --- lldb/unittests/Host/linux/HostTest.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/unittests/Host/linux/HostTest.cpp b/lldb/unittests/Host/linux/HostTest.cpp index 046736dce18d7a..27d1898969fb44 100644 --- a/lldb/unittests/Host/linux/HostTest.cpp +++ b/lldb/unittests/Host/linux/HostTest.cpp @@ -71,8 +71,9 @@ TEST_F(HostTest, GetProcessInfo) { // Test timings ASSERT_TRUE(Host::GetProcessInfo(getpid(), Info)); ProcessInstanceInfo::timespec user_time = Info.GetUserTime(); + static volatile unsigned u = 0; for (unsigned i = 0; i < 10'000'000; i++) { -__asm__ __volatile__("" : "+g"(i) : :); +u = i; } ASSERT_TRUE(Host::GetProcessInfo(getpid(), Info)); ProcessInstanceInfo::timespec next_user_time = Info.GetUserTime(); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] this test is flakey on arm in linux because I got too clever (PR #89267)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Fred Grim (feg208) Changes the assembly jazz doesn't work on arm. oops --- Full diff: https://github.com/llvm/llvm-project/pull/89267.diff 1 Files Affected: - (modified) lldb/unittests/Host/linux/HostTest.cpp (+2-1) ``diff diff --git a/lldb/unittests/Host/linux/HostTest.cpp b/lldb/unittests/Host/linux/HostTest.cpp index 046736dce18d7a..27d1898969fb44 100644 --- a/lldb/unittests/Host/linux/HostTest.cpp +++ b/lldb/unittests/Host/linux/HostTest.cpp @@ -71,8 +71,9 @@ TEST_F(HostTest, GetProcessInfo) { // Test timings ASSERT_TRUE(Host::GetProcessInfo(getpid(), Info)); ProcessInstanceInfo::timespec user_time = Info.GetUserTime(); + static volatile unsigned u = 0; for (unsigned i = 0; i < 10'000'000; i++) { -__asm__ __volatile__("" : "+g"(i) : :); +u = i; } ASSERT_TRUE(Host::GetProcessInfo(getpid(), Info)); ProcessInstanceInfo::timespec next_user_time = Info.GetUserTime(); `` https://github.com/llvm/llvm-project/pull/89267 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] Revert "NFC: Make clang resource headers an interface library (#88317)" (PR #89266)
etcwilde wrote: @ayermolo, one idea might be to see if `clang-resource-headers` is in the install distribution list? I'm not seeing a matching `get_property` in a CMakeLists.txt that matches the above error exactly, so I'm not sure how to reproduce this or how you're picking up the clang-resource-headers target. https://github.com/llvm/llvm-project/pull/89266 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] this test is flakey on arm in linux because I got too clever (PR #89267)
https://github.com/feg208 updated https://github.com/llvm/llvm-project/pull/89267 >From 24fa0fc2f54ef4141302ed7178ee0dc1b6d6af4e Mon Sep 17 00:00:00 2001 From: Fred Grim Date: Thu, 18 Apr 2024 10:13:03 -0700 Subject: [PATCH] this test is flakey on arm in linux because I got too clever --- lldb/unittests/Host/linux/HostTest.cpp | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lldb/unittests/Host/linux/HostTest.cpp b/lldb/unittests/Host/linux/HostTest.cpp index 046736dce18d7a..733909902474d7 100644 --- a/lldb/unittests/Host/linux/HostTest.cpp +++ b/lldb/unittests/Host/linux/HostTest.cpp @@ -69,13 +69,17 @@ TEST_F(HostTest, GetProcessInfo) { EXPECT_EQ(HostInfo::GetArchitecture(HostInfo::eArchKindDefault), Info.GetArchitecture()); // Test timings + /* + * This is flaky in the buildbots on all archs ASSERT_TRUE(Host::GetProcessInfo(getpid(), Info)); ProcessInstanceInfo::timespec user_time = Info.GetUserTime(); + static volatile unsigned u = 0; for (unsigned i = 0; i < 10'000'000; i++) { -__asm__ __volatile__("" : "+g"(i) : :); +u = i; } ASSERT_TRUE(Host::GetProcessInfo(getpid(), Info)); ProcessInstanceInfo::timespec next_user_time = Info.GetUserTime(); ASSERT_TRUE(user_time.tv_sec < next_user_time.tv_sec || user_time.tv_usec < next_user_time.tv_usec); + */ } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] this test is flakey on arm in linux because I got too clever (PR #89267)
feg208 wrote: I had to just comment this test out because it randomly fails on at least x86_64 and arm https://github.com/llvm/llvm-project/pull/89267 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] Revert "NFC: Make clang resource headers an interface library (#88317)" (PR #89266)
https://github.com/compnerd approved this pull request. Approving it in case it needs to be merged, but I think that we should try to determine how it is breaking. This change feels like it should be correct and is a pretty good cleanup, so I would prefer that fix forward rather than revert. https://github.com/llvm/llvm-project/pull/89266 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] this test is flakey on arm in linux because I got too clever (PR #89267)
https://github.com/feg208 closed https://github.com/llvm/llvm-project/pull/89267 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 9842726 - this test is flakey on arm in linux because I got too clever (#89267)
Author: Fred Grim Date: 2024-04-18T10:29:39-07:00 New Revision: 98427264cab558d285c7222f1f25b904124c6b93 URL: https://github.com/llvm/llvm-project/commit/98427264cab558d285c7222f1f25b904124c6b93 DIFF: https://github.com/llvm/llvm-project/commit/98427264cab558d285c7222f1f25b904124c6b93.diff LOG: this test is flakey on arm in linux because I got too clever (#89267) the assembly jazz doesn't work on arm. oops Added: Modified: lldb/unittests/Host/linux/HostTest.cpp Removed: diff --git a/lldb/unittests/Host/linux/HostTest.cpp b/lldb/unittests/Host/linux/HostTest.cpp index 046736dce18d7a..733909902474d7 100644 --- a/lldb/unittests/Host/linux/HostTest.cpp +++ b/lldb/unittests/Host/linux/HostTest.cpp @@ -69,13 +69,17 @@ TEST_F(HostTest, GetProcessInfo) { EXPECT_EQ(HostInfo::GetArchitecture(HostInfo::eArchKindDefault), Info.GetArchitecture()); // Test timings + /* + * This is flaky in the buildbots on all archs ASSERT_TRUE(Host::GetProcessInfo(getpid(), Info)); ProcessInstanceInfo::timespec user_time = Info.GetUserTime(); + static volatile unsigned u = 0; for (unsigned i = 0; i < 10'000'000; i++) { -__asm__ __volatile__("" : "+g"(i) : :); +u = i; } ASSERT_TRUE(Host::GetProcessInfo(getpid(), Info)); ProcessInstanceInfo::timespec next_user_time = Info.GetUserTime(); ASSERT_TRUE(user_time.tv_sec < next_user_time.tv_sec || user_time.tv_usec < next_user_time.tv_usec); + */ } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
emaste wrote: Ok, submitted https://github.com/llvm/llvm-project/issues/89271 for the MD5 migration. I agree that issue does not block this change. https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [lldb][test] Remove LLDB_TEST_USE_VENDOR_PACKAGES (PR #89260)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/89260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 0e5c28d - [lldb][test] Remove LLDB_TEST_USE_VENDOR_PACKAGES (#89260)
Author: Jordan Rupprecht Date: 2024-04-18T13:17:39-05:00 New Revision: 0e5c28d1930e7210f0f293312382c54d298dda06 URL: https://github.com/llvm/llvm-project/commit/0e5c28d1930e7210f0f293312382c54d298dda06 DIFF: https://github.com/llvm/llvm-project/commit/0e5c28d1930e7210f0f293312382c54d298dda06.diff LOG: [lldb][test] Remove LLDB_TEST_USE_VENDOR_PACKAGES (#89260) The `LLDB_TEST_USE_VENDOR_PACKAGES` has defaulted to `Off` for a while. Either installing `pexpect` or skipping those tests with `-DLLDB_TEST_USER_ARGS=--skip-category=pexpect` seems to be enough that we can fully remove this option. This patch removes the `LLDB_TEST_USE_VENDOR_PACKAGES` cmake configuration as well as the associated code to add `third_party/Python/module` to the python path. I'll do the actual deletion of `third_party/Python/module` in a followup PR in the (unlikely, I hope) event this commit needs to be reverted. Added: Modified: clang/cmake/caches/Fuchsia.cmake lldb/cmake/modules/LLDBConfig.cmake lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py lldb/test/API/lit.cfg.py lldb/test/API/lit.site.cfg.py.in lldb/test/CMakeLists.txt lldb/use_lldb_suite_root.py lldb/utils/lldb-dotest/CMakeLists.txt lldb/utils/lldb-dotest/lldb-dotest.in llvm/utils/gn/secondary/lldb/test/BUILD.gn Removed: diff --git a/clang/cmake/caches/Fuchsia.cmake b/clang/cmake/caches/Fuchsia.cmake index 393d97a4cf1a33..30a3b9116a461f 100644 --- a/clang/cmake/caches/Fuchsia.cmake +++ b/clang/cmake/caches/Fuchsia.cmake @@ -65,7 +65,6 @@ set(_FUCHSIA_BOOTSTRAP_PASSTHROUGH LLDB_EMBED_PYTHON_HOME LLDB_PYTHON_HOME LLDB_PYTHON_RELATIVE_PATH - LLDB_TEST_USE_VENDOR_PACKAGES LLDB_TEST_USER_ARGS Python3_EXECUTABLE Python3_LIBRARIES diff --git a/lldb/cmake/modules/LLDBConfig.cmake b/lldb/cmake/modules/LLDBConfig.cmake index 5d62213c3f5838..a758261073ac57 100644 --- a/lldb/cmake/modules/LLDBConfig.cmake +++ b/lldb/cmake/modules/LLDBConfig.cmake @@ -67,8 +67,6 @@ option(LLDB_SKIP_STRIP "Whether to skip stripping of binaries when installing ll option(LLDB_SKIP_DSYM "Whether to skip generating a dSYM when installing lldb." OFF) option(LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS "Fail to configure if certain requirements are not met for testing." OFF) -option(LLDB_TEST_USE_VENDOR_PACKAGES - "Use packages from lldb/third_party/Python/module instead of system deps." OFF) set(LLDB_GLOBAL_INIT_DIRECTORY "" CACHE STRING "Path to the global lldbinit directory. Relative paths are resolved relative to the diff --git a/lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py b/lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py index 3b746c3f9242df..2558be1364d9be 100644 --- a/lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py +++ b/lldb/packages/Python/lldbsuite/test/lldb_pylint_helper.py @@ -157,14 +157,6 @@ def child_dirs(parent_dir): 0, os.path.join(packages_python_child_dir, "test_runner", "lib") ) -# Handle third_party module/package directory. -third_party_module_dir = os.path.join( -check_dir, "third_party", "Python", "module" -) -for child_dir in child_dirs(third_party_module_dir): -# Yes, we embed the module in the module parent dir -sys.path.insert(0, child_dir) - # We're done. break diff --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py index 9ea389c639a013..9d6775917e1370 100644 --- a/lldb/test/API/lit.cfg.py +++ b/lldb/test/API/lit.cfg.py @@ -310,6 +310,3 @@ def delete_module_cache(path): # Propagate XDG_CACHE_HOME if "XDG_CACHE_HOME" in os.environ: config.environment["XDG_CACHE_HOME"] = os.environ["XDG_CACHE_HOME"] - -if is_configured("use_vendor_packages"): -config.environment["LLDB_TEST_USE_VENDOR_PACKAGES"] = "1" diff --git a/lldb/test/API/lit.site.cfg.py.in b/lldb/test/API/lit.site.cfg.py.in index c2602acd2ef85a..053331dc4881f7 100644 --- a/lldb/test/API/lit.site.cfg.py.in +++ b/lldb/test/API/lit.site.cfg.py.in @@ -38,7 +38,6 @@ config.libcxx_include_target_dir = "@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@" # The API tests use their own module caches. config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-api") config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-api") -config.use_vendor_packages = @LLDB_TEST_USE_VENDOR_PACKAGES@ # Plugins lldb_build_intel_pt = '@LLDB_BUILD_INTEL_PT@' diff --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt index b6ec5bc4d819e3..6a9ca59f96b0f8 100644 --- a/lldb/test/CMakeLists.txt +++ b/lldb/test/CMakeLists.txt @@ -245,8 +245,7 @@ llvm_canonicalize_cmake_booleans( LLDB_HAS_LIBCXX LLDB_TOOL_LLDB_SERVER_BUILD LLDB_USE_SYSTEM_DEBUGSERVER - LLDB_IS_64_BITS - LLDB_TEST_USE_VENDOR_PACKAGES) +
[Lldb-commits] [clang] [lldb] [llvm] [lldb][test] Remove LLDB_TEST_USE_VENDOR_PACKAGES (PR #89260)
https://github.com/rupprecht closed https://github.com/llvm/llvm-project/pull/89260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][nfc] Remove unused member Disassembler::m_base_addr (PR #89289)
https://github.com/bulbazord created https://github.com/llvm/llvm-project/pull/89289 This member variable is completely unused. I also don't think it makes a ton of sense since (1) The "base address" can be obtained from the first Instruction in its InstructionList, and (2) InstructionLists may not be a series of contiguous instructions (even though they are most of the time). >From 821713962ebe64cf72dea81a443c6e30ef52abaf Mon Sep 17 00:00:00 2001 From: Alex Langford Date: Thu, 18 Apr 2024 11:35:21 -0700 Subject: [PATCH] [lldb][nfc] Remove unused member Disassembler::m_base_addr This member variable is completely unused. I also don't think it makes a ton of sense since (1) The "base address" can be obtained from the first Instruction in its InstructionList, and (2) InstructionLists may not be a series of contiguous instructions (even though they are most of the time). --- lldb/include/lldb/Core/Disassembler.h | 1 - lldb/source/Core/Disassembler.cpp | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h index e037a49f152c74..21969aed03c209 100644 --- a/lldb/include/lldb/Core/Disassembler.h +++ b/lldb/include/lldb/Core/Disassembler.h @@ -547,7 +547,6 @@ class Disassembler : public std::enable_shared_from_this, // Classes that inherit from Disassembler can see and modify these ArchSpec m_arch; InstructionList m_instruction_list; - lldb::addr_t m_base_addr; std::string m_flavor; private: diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index e31746fa0b8b9d..9286f62058bc8d 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -1117,8 +1117,7 @@ size_t Disassembler::ParseInstructions(Target &target, Address start, // Disassembler copy constructor Disassembler::Disassembler(const ArchSpec &arch, const char *flavor) -: m_arch(arch), m_instruction_list(), m_base_addr(LLDB_INVALID_ADDRESS), - m_flavor() { +: m_arch(arch), m_instruction_list(), m_flavor() { if (flavor == nullptr) m_flavor.assign("default"); else ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][nfc] Remove unused member Disassembler::m_base_addr (PR #89289)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Alex Langford (bulbazord) Changes This member variable is completely unused. I also don't think it makes a ton of sense since (1) The "base address" can be obtained from the first Instruction in its InstructionList, and (2) InstructionLists may not be a series of contiguous instructions (even though they are most of the time). --- Full diff: https://github.com/llvm/llvm-project/pull/89289.diff 2 Files Affected: - (modified) lldb/include/lldb/Core/Disassembler.h (-1) - (modified) lldb/source/Core/Disassembler.cpp (+1-2) ``diff diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h index e037a49f152c74..21969aed03c209 100644 --- a/lldb/include/lldb/Core/Disassembler.h +++ b/lldb/include/lldb/Core/Disassembler.h @@ -547,7 +547,6 @@ class Disassembler : public std::enable_shared_from_this, // Classes that inherit from Disassembler can see and modify these ArchSpec m_arch; InstructionList m_instruction_list; - lldb::addr_t m_base_addr; std::string m_flavor; private: diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index e31746fa0b8b9d..9286f62058bc8d 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -1117,8 +1117,7 @@ size_t Disassembler::ParseInstructions(Target &target, Address start, // Disassembler copy constructor Disassembler::Disassembler(const ArchSpec &arch, const char *flavor) -: m_arch(arch), m_instruction_list(), m_base_addr(LLDB_INVALID_ADDRESS), - m_flavor() { +: m_arch(arch), m_instruction_list(), m_flavor() { if (flavor == nullptr) m_flavor.assign("default"); else `` https://github.com/llvm/llvm-project/pull/89289 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] Revert "NFC: Make clang resource headers an interface library (#88317)" (PR #89266)
ayermolo wrote: Thanks. It's failing internally in our automatic multi stage build. Trying to dig into it as part of my oncall. :) https://github.com/llvm/llvm-project/pull/89266 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
Awfa wrote: Someone will have to merge for me because it says "Only those with write access to this repository can merge pull requests." Am I supposed to request write access somewhere before making these PRs? I haven't contributed before. https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] this test is flakey on arm in linux because I got too clever (PR #89267)
JDevlieghere wrote: Can you please put up a follow-up that removes the code instead of commenting it out? https://github.com/llvm/llvm-project/pull/89267 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
bulbazord wrote: > Someone will have to merge for me because it says "Only those with write > access to this repository can merge pull requests." > > Am I supposed to request write access somewhere before making these PRs? I > haven't contributed before. First time contributors aren't expected to have commit access, so no worries there. I approved it so I can land this change for you. Let's keep an eye on the build bots in case this inadvertently introduces any regressions. :) https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 22c26fa - [lldb] Skip remote PutFile when MD5 hashes equal (#88812)
Author: Anthony Ha Date: 2024-04-18T12:24:24-07:00 New Revision: 22c26fa13d46e174e3d862e5f40cff06d804af0c URL: https://github.com/llvm/llvm-project/commit/22c26fa13d46e174e3d862e5f40cff06d804af0c DIFF: https://github.com/llvm/llvm-project/commit/22c26fa13d46e174e3d862e5f40cff06d804af0c.diff LOG: [lldb] Skip remote PutFile when MD5 hashes equal (#88812) This PR adds a check within `PutFile` to exit early when both local and destination files have matching MD5 hashes. If they differ, or there is trouble getting the hashes, the regular code path to put the file is run. As I needed this to talk to an `lldb-server` which runs the gdb-remote protocol, I enabled `CalculateMD5` within `Platform/gdb-server` and also found and fixed a parsing bug within it as well. Before this PR, the client is incorrectly parsing the response packet containing the checksum; after this PR, hopefully this is fixed. There is a test for the parsing behavior included in this PR. - Co-authored-by: Anthony Ha Added: Modified: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h lldb/source/Target/Platform.cpp lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp Removed: diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp index 88f1ad15b6b485..0dce5add2e3759 100644 --- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp +++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp @@ -684,6 +684,14 @@ Status PlatformRemoteGDBServer::RunShellCommand( signo_ptr, command_output, timeout); } +bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec &file_spec, + uint64_t &low, uint64_t &high) { + if (!IsConnected()) +return false; + + return m_gdb_client_up->CalculateMD5(file_spec, low, high); +} + void PlatformRemoteGDBServer::CalculateTrapHandlerSymbolNames() { m_trap_handlers.push_back(ConstString("_sigtramp")); } diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h index 638f7db5ef800c..d83fc386f59427 100644 --- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h +++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h @@ -146,6 +146,9 @@ class PlatformRemoteGDBServer : public Platform, private UserIDResolver { void CalculateTrapHandlerSymbolNames() override; + bool CalculateMD5(const FileSpec &file_spec, uint64_t &low, +uint64_t &high) override; + const lldb::UnixSignalsSP &GetRemoteUnixSignals() override; size_t ConnectToWaitingProcesses(lldb_private::Debugger &debugger, diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index 1f6116967a4f64..7498a070c26094 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -3419,7 +3419,7 @@ bool GDBRemoteCommunicationClient::GetFileExists( } bool GDBRemoteCommunicationClient::CalculateMD5( -const lldb_private::FileSpec &file_spec, uint64_t &high, uint64_t &low) { +const lldb_private::FileSpec &file_spec, uint64_t &low, uint64_t &high) { std::string path(file_spec.GetPath(false)); lldb_private::StreamString stream; stream.PutCString("vFile:MD5:"); @@ -3433,8 +3433,44 @@ bool GDBRemoteCommunicationClient::CalculateMD5( return false; if (response.Peek() && *response.Peek() == 'x') return false; -low = response.GetHexMaxU64(false, UINT64_MAX); -high = response.GetHexMaxU64(false, UINT64_MAX); + +// GDBRemoteCommunicationServerCommon::Handle_vFile_MD5 concatenates low and +// high hex strings. We can't use response.GetHexMaxU64 because that can't +// handle the concatenated hex string. What would happen is parsing the low +// would consume the whole response packet which would give incorrect +// results. Instead, we get the byte string for each low and high hex +// separately, and parse them. +// +// An alternate way to handle this is to change the server to put a +// delimiter between the low/high parts, and change the client to parse the +// delimiter. However, we choose not to do this so existing lldb-servers +// don't have to be patched + +// The checksum is 128 bits encoded as hex +// This means low/high are halves o
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
github-actions[bot] wrote: @Awfa Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail [here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr). If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of [LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy). You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -1197,6 +1197,32 @@ Status Platform::PutFile(const FileSpec &source, const FileSpec &destination, if (!source_file) return Status(source_file.takeError()); Status error; + + bool requires_upload = true; + uint64_t dest_md5_low, dest_md5_high; + bool success = CalculateMD5(destination, dest_md5_low, dest_md5_high); + if (!success) { +LLDB_LOGF(log, "[PutFile] couldn't get md5 sum of destination"); + } else { +auto local_md5 = llvm::sys::fs::md5_contents(source.GetPath()); JDevlieghere wrote: It's not obvious from the right hand side what the return value is. Case in point, it returns an `ErrorOr` and if I hadn't looked it up I wouldn't have noticed we're not actually logging the error. https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Skip remote PutFile when MD5 hashes equal (PR #88812)
@@ -3419,7 +3419,7 @@ bool GDBRemoteCommunicationClient::GetFileExists( } bool GDBRemoteCommunicationClient::CalculateMD5( -const lldb_private::FileSpec &file_spec, uint64_t &high, uint64_t &low) { +const lldb_private::FileSpec &file_spec, uint64_t &low, uint64_t &high) { JDevlieghere wrote: If we're changing the interface anyway, could we have this return an `std::optional` to match what `md5_contents` returns? https://github.com/llvm/llvm-project/pull/88812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][nfc] Remove unused member Disassembler::m_base_addr (PR #89289)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/89289 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Check for null oso SymbolFile in SymbolFileDwarfDebugMap::ResolveSymbolContext (PR #89324)
https://github.com/jimingham created https://github.com/llvm/llvm-project/pull/89324 The couple other places that use the oso module's SymbolFile, they check that it's non-null, so this is just an oversight in ResolveSymbolContext. I didn't add a test for this (but did add a log message for the error case) because I can't see how this would actually happen. The .o file had to have valid enough DWARF that the linker's parser could handle it or it would not be in the debug map. If you delete the .o file, we just leave that entry out of the debug map. If you strip it or otherwise mess with it, we'll notice the changed mod time and refuse to read it... This was based on a report from the field, and we don't have access to the project. But if the logging tells me how this happened I can come back and add a test with that example. >From b6021cf31e4c607dc4d541b4dd3b028e93050767 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Thu, 18 Apr 2024 15:02:37 -0700 Subject: [PATCH] Check for null oso SymbolFile in SymbolFileDwarfDebugMap::ResolveSymbolContext. The couple other places that use the oso module's SymbolFile, they check that it's non-null, so this is just an oversight in ResolveSymbolContext. I didn't add a test for this (but did add a log message for the error case) because I can't see how this would actually happen. The .o file had to have valid enough DWARF that the linker's parser could handle it or it would not be in the debug map. If you delete the .o file, we just leave that entry out of the debug map. If you strip it or otherwise mess with it, we'll notice the changed mod time and refuse to read it... This was based on a report from the field, and we don't have access to the project. But if the logging tells me how this happened I can come back and add a test with that example. --- .../SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index 1de585832e3216..d6560e399e4fc4 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -848,9 +848,18 @@ SymbolFileDWARFDebugMap::ResolveSymbolContext(const Address &exe_so_addr, debug_map_entry->data.GetOSOFileAddress(); Address oso_so_addr; if (oso_module->ResolveFileAddress(oso_file_addr, oso_so_addr)) { - resolved_flags |= - oso_module->GetSymbolFile()->ResolveSymbolContext( - oso_so_addr, resolve_scope, sc); + SymbolFile *sym_file = oso_module->GetSymbolFile(); + if (sym_file) { +resolved_flags |= sym_file->ResolveSymbolContext(oso_so_addr, +resolve_scope, sc); + } else { +ObjectFile *obj_file = GetObjectFile(); +LLDB_LOG(GetLog(DWARFLog::DebugMap), + "Failed to get symfile for OSO: {0} in module: {1}", + oso_module->GetFileSpec(), + obj_file ? obj_file->GetFileSpec() : + FileSpec("unknown")); + } } } } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Check for null oso SymbolFile in SymbolFileDwarfDebugMap::ResolveSymbolContext (PR #89324)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: None (jimingham) Changes The couple other places that use the oso module's SymbolFile, they check that it's non-null, so this is just an oversight in ResolveSymbolContext. I didn't add a test for this (but did add a log message for the error case) because I can't see how this would actually happen. The .o file had to have valid enough DWARF that the linker's parser could handle it or it would not be in the debug map. If you delete the .o file, we just leave that entry out of the debug map. If you strip it or otherwise mess with it, we'll notice the changed mod time and refuse to read it... This was based on a report from the field, and we don't have access to the project. But if the logging tells me how this happened I can come back and add a test with that example. --- Full diff: https://github.com/llvm/llvm-project/pull/89324.diff 1 Files Affected: - (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp (+12-3) ``diff diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index 1de585832e3216..d6560e399e4fc4 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -848,9 +848,18 @@ SymbolFileDWARFDebugMap::ResolveSymbolContext(const Address &exe_so_addr, debug_map_entry->data.GetOSOFileAddress(); Address oso_so_addr; if (oso_module->ResolveFileAddress(oso_file_addr, oso_so_addr)) { - resolved_flags |= - oso_module->GetSymbolFile()->ResolveSymbolContext( - oso_so_addr, resolve_scope, sc); + SymbolFile *sym_file = oso_module->GetSymbolFile(); + if (sym_file) { +resolved_flags |= sym_file->ResolveSymbolContext(oso_so_addr, +resolve_scope, sc); + } else { +ObjectFile *obj_file = GetObjectFile(); +LLDB_LOG(GetLog(DWARFLog::DebugMap), + "Failed to get symfile for OSO: {0} in module: {1}", + oso_module->GetFileSpec(), + obj_file ? obj_file->GetFileSpec() : + FileSpec("unknown")); + } } } } `` https://github.com/llvm/llvm-project/pull/89324 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Check for null oso SymbolFile in SymbolFileDwarfDebugMap::ResolveSymbolContext (PR #89324)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 6f1e23b47d428d792866993ed26f4173d479d43d b6021cf31e4c607dc4d541b4dd3b028e93050767 -- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index d6560e399e..448e640e9b 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -850,15 +850,15 @@ SymbolFileDWARFDebugMap::ResolveSymbolContext(const Address &exe_so_addr, if (oso_module->ResolveFileAddress(oso_file_addr, oso_so_addr)) { SymbolFile *sym_file = oso_module->GetSymbolFile(); if (sym_file) { -resolved_flags |= sym_file->ResolveSymbolContext(oso_so_addr, -resolve_scope, sc); +resolved_flags |= sym_file->ResolveSymbolContext( +oso_so_addr, resolve_scope, sc); } else { ObjectFile *obj_file = GetObjectFile(); LLDB_LOG(GetLog(DWARFLog::DebugMap), - "Failed to get symfile for OSO: {0} in module: {1}", - oso_module->GetFileSpec(), - obj_file ? obj_file->GetFileSpec() : - FileSpec("unknown")); + "Failed to get symfile for OSO: {0} in module: {1}", + oso_module->GetFileSpec(), + obj_file ? obj_file->GetFileSpec() + : FileSpec("unknown")); } } } `` https://github.com/llvm/llvm-project/pull/89324 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Check for null oso SymbolFile in SymbolFileDwarfDebugMap::ResolveSymbolContext (PR #89324)
https://github.com/bulbazord approved this pull request. https://github.com/llvm/llvm-project/pull/89324 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Check for null oso SymbolFile in SymbolFileDwarfDebugMap::ResolveSymbolContext (PR #89324)
@@ -848,9 +848,18 @@ SymbolFileDWARFDebugMap::ResolveSymbolContext(const Address &exe_so_addr, debug_map_entry->data.GetOSOFileAddress(); Address oso_so_addr; if (oso_module->ResolveFileAddress(oso_file_addr, oso_so_addr)) { - resolved_flags |= - oso_module->GetSymbolFile()->ResolveSymbolContext( - oso_so_addr, resolve_scope, sc); + SymbolFile *sym_file = oso_module->GetSymbolFile(); + if (sym_file) { bulbazord wrote: suggestion: `if (SymbolFile *sym_file = oso_module->GetSymbolFile()) {` https://github.com/llvm/llvm-project/pull/89324 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Check for null oso SymbolFile in SymbolFileDwarfDebugMap::ResolveSymbolContext (PR #89324)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/89324 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Check for null oso SymbolFile in SymbolFileDwarfDebugMap::ResolveSymbolContext (PR #89324)
https://github.com/jimingham updated https://github.com/llvm/llvm-project/pull/89324 >From b6021cf31e4c607dc4d541b4dd3b028e93050767 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Thu, 18 Apr 2024 15:02:37 -0700 Subject: [PATCH 1/2] Check for null oso SymbolFile in SymbolFileDwarfDebugMap::ResolveSymbolContext. The couple other places that use the oso module's SymbolFile, they check that it's non-null, so this is just an oversight in ResolveSymbolContext. I didn't add a test for this (but did add a log message for the error case) because I can't see how this would actually happen. The .o file had to have valid enough DWARF that the linker's parser could handle it or it would not be in the debug map. If you delete the .o file, we just leave that entry out of the debug map. If you strip it or otherwise mess with it, we'll notice the changed mod time and refuse to read it... This was based on a report from the field, and we don't have access to the project. But if the logging tells me how this happened I can come back and add a test with that example. --- .../SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index 1de585832e3216..d6560e399e4fc4 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -848,9 +848,18 @@ SymbolFileDWARFDebugMap::ResolveSymbolContext(const Address &exe_so_addr, debug_map_entry->data.GetOSOFileAddress(); Address oso_so_addr; if (oso_module->ResolveFileAddress(oso_file_addr, oso_so_addr)) { - resolved_flags |= - oso_module->GetSymbolFile()->ResolveSymbolContext( - oso_so_addr, resolve_scope, sc); + SymbolFile *sym_file = oso_module->GetSymbolFile(); + if (sym_file) { +resolved_flags |= sym_file->ResolveSymbolContext(oso_so_addr, +resolve_scope, sc); + } else { +ObjectFile *obj_file = GetObjectFile(); +LLDB_LOG(GetLog(DWARFLog::DebugMap), + "Failed to get symfile for OSO: {0} in module: {1}", + oso_module->GetFileSpec(), + obj_file ? obj_file->GetFileSpec() : + FileSpec("unknown")); + } } } } >From 50e5143db27ce5f8b0d9c77f485883f0c1eb Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Thu, 18 Apr 2024 16:07:52 -0700 Subject: [PATCH 2/2] Move sym_file definition inside "if ()". --- .../Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index d6560e399e4fc4..4089697f0bbc11 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -848,8 +848,7 @@ SymbolFileDWARFDebugMap::ResolveSymbolContext(const Address &exe_so_addr, debug_map_entry->data.GetOSOFileAddress(); Address oso_so_addr; if (oso_module->ResolveFileAddress(oso_file_addr, oso_so_addr)) { - SymbolFile *sym_file = oso_module->GetSymbolFile(); - if (sym_file) { + if (SymbolFile *sym_file = oso_module->GetSymbolFile()) { resolved_flags |= sym_file->ResolveSymbolContext(oso_so_addr, resolve_scope, sc); } else { ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Check for null oso SymbolFile in SymbolFileDwarfDebugMap::ResolveSymbolContext (PR #89324)
https://github.com/JDevlieghere approved this pull request. LGTM with Alex' suggestion and the code formatted. https://github.com/llvm/llvm-project/pull/89324 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Check for null oso SymbolFile in SymbolFileDwarfDebugMap::ResolveSymbolContext (PR #89324)
https://github.com/jimingham updated https://github.com/llvm/llvm-project/pull/89324 >From b6021cf31e4c607dc4d541b4dd3b028e93050767 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Thu, 18 Apr 2024 15:02:37 -0700 Subject: [PATCH 1/3] Check for null oso SymbolFile in SymbolFileDwarfDebugMap::ResolveSymbolContext. The couple other places that use the oso module's SymbolFile, they check that it's non-null, so this is just an oversight in ResolveSymbolContext. I didn't add a test for this (but did add a log message for the error case) because I can't see how this would actually happen. The .o file had to have valid enough DWARF that the linker's parser could handle it or it would not be in the debug map. If you delete the .o file, we just leave that entry out of the debug map. If you strip it or otherwise mess with it, we'll notice the changed mod time and refuse to read it... This was based on a report from the field, and we don't have access to the project. But if the logging tells me how this happened I can come back and add a test with that example. --- .../SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index 1de585832e3216..d6560e399e4fc4 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -848,9 +848,18 @@ SymbolFileDWARFDebugMap::ResolveSymbolContext(const Address &exe_so_addr, debug_map_entry->data.GetOSOFileAddress(); Address oso_so_addr; if (oso_module->ResolveFileAddress(oso_file_addr, oso_so_addr)) { - resolved_flags |= - oso_module->GetSymbolFile()->ResolveSymbolContext( - oso_so_addr, resolve_scope, sc); + SymbolFile *sym_file = oso_module->GetSymbolFile(); + if (sym_file) { +resolved_flags |= sym_file->ResolveSymbolContext(oso_so_addr, +resolve_scope, sc); + } else { +ObjectFile *obj_file = GetObjectFile(); +LLDB_LOG(GetLog(DWARFLog::DebugMap), + "Failed to get symfile for OSO: {0} in module: {1}", + oso_module->GetFileSpec(), + obj_file ? obj_file->GetFileSpec() : + FileSpec("unknown")); + } } } } >From 50e5143db27ce5f8b0d9c77f485883f0c1eb Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Thu, 18 Apr 2024 16:07:52 -0700 Subject: [PATCH 2/3] Move sym_file definition inside "if ()". --- .../Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index d6560e399e4fc4..4089697f0bbc11 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -848,8 +848,7 @@ SymbolFileDWARFDebugMap::ResolveSymbolContext(const Address &exe_so_addr, debug_map_entry->data.GetOSOFileAddress(); Address oso_so_addr; if (oso_module->ResolveFileAddress(oso_file_addr, oso_so_addr)) { - SymbolFile *sym_file = oso_module->GetSymbolFile(); - if (sym_file) { + if (SymbolFile *sym_file = oso_module->GetSymbolFile()) { resolved_flags |= sym_file->ResolveSymbolContext(oso_so_addr, resolve_scope, sc); } else { >From a13fc0de7e5412d29add4bcb57ab55614e92108a Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Thu, 18 Apr 2024 16:14:10 -0700 Subject: [PATCH 3/3] Bow --- .../SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index 4089697f0bbc11..f066f13d51c5d1 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -849,15 +849,15 @@ SymbolFileDWARFDebugMap::ResolveSymbolContext(const Address &exe_so_addr, Address oso_so_addr; if (oso_module->ResolveFileAddress(oso_file_addr, oso_so_addr)) { if (SymbolFile *sym_file = oso_module->GetSymbolFile()) { -resolved_flags |= sym_file->ResolveSymbolContext(oso_so_addr, -resolve_scope, sc); +resolved_flags |= sym_file->ResolveSymbolContext( +oso_so_addr, resolve_scope, sc);