[Lldb-commits] [lldb] e450f98 - [lldb] Fix Scalar::GetData for non-multiple-of-8-bits values (#90846)
Author: Pavel Labath Date: 2024-05-03T09:07:20+02:00 New Revision: e450f987286b983328e0b9e65630b656dec291de URL: https://github.com/llvm/llvm-project/commit/e450f987286b983328e0b9e65630b656dec291de DIFF: https://github.com/llvm/llvm-project/commit/e450f987286b983328e0b9e65630b656dec291de.diff LOG: [lldb] Fix Scalar::GetData for non-multiple-of-8-bits values (#90846) It was aligning the byte size down. Now it aligns up. This manifested itself as SBTypeStaticField::GetConstantValue returning a zero-sized value for `bool` fields (because clang represents bool as a 1-bit value). I've changed the code for float Scalars as well, although I'm not aware of floating point values that are not multiples of 8 bits. Added: Modified: lldb/source/Utility/Scalar.cpp lldb/test/API/python_api/type/TestTypeList.py lldb/test/API/python_api/type/main.cpp lldb/unittests/Utility/ScalarTest.cpp Removed: diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp index e94fd459623665..c70c5e10799187 100644 --- a/lldb/source/Utility/Scalar.cpp +++ b/lldb/source/Utility/Scalar.cpp @@ -134,9 +134,9 @@ size_t Scalar::GetByteSize() const { case e_void: break; case e_int: -return (m_integer.getBitWidth() / 8); +return (m_integer.getBitWidth() + 7) / 8; case e_float: -return m_float.bitcastToAPInt().getBitWidth() / 8; +return (m_float.bitcastToAPInt().getBitWidth() + 7) / 8; } return 0; } diff --git a/lldb/test/API/python_api/type/TestTypeList.py b/lldb/test/API/python_api/type/TestTypeList.py index 81c44f7a39d61a..17e27b624511cf 100644 --- a/lldb/test/API/python_api/type/TestTypeList.py +++ b/lldb/test/API/python_api/type/TestTypeList.py @@ -52,6 +52,19 @@ def _find_static_field_in_Task_pointer(self, task_pointer): self.DebugSBValue(value) self.assertEqual(value.GetValueAsSigned(), 47) +static_constexpr_bool_field = task_type.GetStaticFieldWithName( +"static_constexpr_bool_field" +) +self.assertTrue(static_constexpr_bool_field) +self.assertEqual( +static_constexpr_bool_field.GetName(), "static_constexpr_bool_field" +) +self.assertEqual(static_constexpr_bool_field.GetType().GetName(), "const bool") + +value = static_constexpr_bool_field.GetConstantValue(self.target()) +self.DebugSBValue(value) +self.assertEqual(value.GetValueAsUnsigned(), 1) + static_mutable_field = task_type.GetStaticFieldWithName("static_mutable_field") self.assertTrue(static_mutable_field) self.assertEqual(static_mutable_field.GetName(), "static_mutable_field") diff --git a/lldb/test/API/python_api/type/main.cpp b/lldb/test/API/python_api/type/main.cpp index c86644d918279a..7384a3d8da16fb 100644 --- a/lldb/test/API/python_api/type/main.cpp +++ b/lldb/test/API/python_api/type/main.cpp @@ -28,6 +28,7 @@ class Task { union U { } u; static constexpr long static_constexpr_field = 47; +static constexpr bool static_constexpr_bool_field = true; static int static_mutable_field; Task(int i, Task *n): id(i), diff --git a/lldb/unittests/Utility/ScalarTest.cpp b/lldb/unittests/Utility/ScalarTest.cpp index 8d957d16593ee7..500cb8bb2286e0 100644 --- a/lldb/unittests/Utility/ScalarTest.cpp +++ b/lldb/unittests/Utility/ScalarTest.cpp @@ -13,8 +13,11 @@ #include "lldb/Utility/Scalar.h" #include "lldb/Utility/Status.h" #include "lldb/Utility/StreamString.h" +#include "lldb/lldb-enumerations.h" +#include "llvm/ADT/APSInt.h" #include "llvm/Testing/Support/Error.h" +#include #include using namespace lldb_private; @@ -163,6 +166,33 @@ TEST(ScalarTest, GetBytes) { ASSERT_EQ(0, memcmp(f, Storage, sizeof(f))); } +TEST(ScalarTest, GetData) { + auto get_data = [](llvm::APSInt v) { +DataExtractor data; +Scalar(v).GetData(data); +return data.GetData().vec(); + }; + + auto vec = [](std::initializer_list l) { +std::vector v(l.begin(), l.end()); +if (endian::InlHostByteOrder() == lldb::eByteOrderLittle) + std::reverse(v.begin(), v.end()); +return v; + }; + + EXPECT_THAT( + get_data(llvm::APSInt::getMaxValue(/*numBits=*/1, /*Unsigned=*/true)), + vec({0x01})); + + EXPECT_THAT( + get_data(llvm::APSInt::getMaxValue(/*numBits=*/8, /*Unsigned=*/true)), + vec({0xff})); + + EXPECT_THAT( + get_data(llvm::APSInt::getMaxValue(/*numBits=*/9, /*Unsigned=*/true)), + vec({0x01, 0xff})); +} + TEST(ScalarTest, SetValueFromData) { uint8_t a[] = {1, 2, 3, 4}; Scalar s; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix Scalar::GetData for non-multiple-of-8-bits values (PR #90846)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/90846 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed unresolved test lldb-api python_api/debugger/TestDebuggerAPI.py on x86_64 host (PR #90580)
labath wrote: FWIW, I agree with @bulbazord. If the user specifies an explicit platform in the CreateTarget function, that platform should really take precedence over anything else. (If it were up to me, I would not even attempt matching other platforms in this case (and just refuse to create the target if the provided platform does not work), but that might be going too far in terms of changing existing behavior). https://github.com/llvm/llvm-project/pull/90580 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [AArch64][TargetParser] autogen ArchExtKind enum - renaming (PR #90320)
davemgreen wrote: @tmatheson-arm reached out and we have a bit of a conversation internally. I do think that there is too much going on in this one pr to be sensible to review, but from what I've looked at my main points I think are: - Some AEK names get renamed in ways I would not expect them to, like AEK_FP16 being changed to AEK_FULLFP16. The command-line names or FEAT_ names should probably be what we are aiming for if we are changing them one-way or the other. - Some of the backend features have been renamed in ways that could cause breakages for external users, like +complxnum becoming +fcma. The new name is better, but ideally the old name would continue to work (maybe by adding an alias/extra target feature dependant on the new name? That might not work with negative features). - Some of changes do look mechanical and a good change (P1->p1, V2->v2 etc), and if they were separate they would be easy to get in and out of the way of the contentious stuff that remains. - The ones that have changed should have release notes. https://github.com/llvm/llvm-project/pull/90320 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [libc] [libclc] [libcxxabi] [lld] [lldb] [llvm] [mlir] Add clarifying parenthesis around non-trivial conditions in ternary expressions. (PR #90391)
https://github.com/luolent updated https://github.com/llvm/llvm-project/pull/90391 >From 54c6c6b7d71f5ff293412f5f91e9f880480284f8 Mon Sep 17 00:00:00 2001 From: luolent Date: Sat, 27 Apr 2024 22:17:58 +0300 Subject: [PATCH] llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:3804: lacking () for clarity --- clang/lib/Basic/Targets/AMDGPU.cpp| 2 +- compiler-rt/lib/xray/xray_utils.h | 2 +- libc/src/__support/FPUtil/aarch64/FEnvImpl.h | 20 .../FPUtil/aarch64/fenv_darwin_impl.h | 48 +-- libc/src/__support/FPUtil/arm/FEnvImpl.h | 40 libc/src/__support/FPUtil/riscv/FEnvImpl.h| 20 libc/src/__support/FPUtil/x86_64/FEnvImpl.h | 24 +- libclc/generic/lib/math/log_base.h| 2 +- libcxxabi/src/cxa_personality.cpp | 4 +- lld/ELF/LinkerScript.cpp | 2 +- .../source/MacOSX/MachException.cpp | 2 +- .../AMDGPU/AsmParser/AMDGPUAsmParser.cpp | 4 +- .../Disassembler/AMDGPUDisassembler.cpp | 4 +- llvm/lib/Target/AVR/AVRAsmPrinter.cpp | 4 +- llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp | 4 +- .../lib/Target/X86/AsmParser/X86AsmParser.cpp | 2 +- .../X86/MCTargetDesc/X86MCCodeEmitter.cpp | 2 +- .../Transforms/TosaDecomposeTransposeConv.cpp | 4 +- 18 files changed, 94 insertions(+), 96 deletions(-) diff --git a/clang/lib/Basic/Targets/AMDGPU.cpp b/clang/lib/Basic/Targets/AMDGPU.cpp index 5742885df0461b..cc7be64656e5b2 100644 --- a/clang/lib/Basic/Targets/AMDGPU.cpp +++ b/clang/lib/Basic/Targets/AMDGPU.cpp @@ -232,7 +232,7 @@ AMDGPUTargetInfo::AMDGPUTargetInfo(const llvm::Triple &Triple, HasLegalHalfType = true; HasFloat16 = true; - WavefrontSize = GPUFeatures & llvm::AMDGPU::FEATURE_WAVE32 ? 32 : 64; + WavefrontSize = (GPUFeatures & llvm::AMDGPU::FEATURE_WAVE32) ? 32 : 64; AllowAMDGPUUnsafeFPAtomics = Opts.AllowAMDGPUUnsafeFPAtomics; // Set pointer width and alignment for the generic address space. diff --git a/compiler-rt/lib/xray/xray_utils.h b/compiler-rt/lib/xray/xray_utils.h index 333826168c0db2..5dc73d7fa8cdea 100644 --- a/compiler-rt/lib/xray/xray_utils.h +++ b/compiler-rt/lib/xray/xray_utils.h @@ -61,7 +61,7 @@ constexpr size_t gcd(size_t a, size_t b) { constexpr size_t lcm(size_t a, size_t b) { return a * b / gcd(a, b); } constexpr size_t nearest_boundary(size_t number, size_t multiple) { - return multiple * ((number / multiple) + (number % multiple ? 1 : 0)); + return multiple * ((number / multiple) + ((number % multiple) ? 1 : 0)); } constexpr size_t next_pow2_helper(size_t num, size_t acc) { diff --git a/libc/src/__support/FPUtil/aarch64/FEnvImpl.h b/libc/src/__support/FPUtil/aarch64/FEnvImpl.h index d1d92169475d15..cd8a5970edd65a 100644 --- a/libc/src/__support/FPUtil/aarch64/FEnvImpl.h +++ b/libc/src/__support/FPUtil/aarch64/FEnvImpl.h @@ -53,19 +53,19 @@ struct FEnv { static constexpr uint32_t ExceptionControlFlagsBitPosition = 8; LIBC_INLINE static uint32_t getStatusValueForExcept(int excepts) { -return (excepts & FE_INVALID ? INVALID : 0) | - (excepts & FE_DIVBYZERO ? DIVBYZERO : 0) | - (excepts & FE_OVERFLOW ? OVERFLOW : 0) | - (excepts & FE_UNDERFLOW ? UNDERFLOW : 0) | - (excepts & FE_INEXACT ? INEXACT : 0); +return ((excepts & FE_INVALID) ? INVALID : 0) | + ((excepts & FE_DIVBYZERO) ? DIVBYZERO : 0) | + ((excepts & FE_OVERFLOW) ? OVERFLOW : 0) | + ((excepts & FE_UNDERFLOW) ? UNDERFLOW : 0) | + ((excepts & FE_INEXACT) ? INEXACT : 0); } LIBC_INLINE static int exceptionStatusToMacro(uint32_t status) { -return (status & INVALID ? FE_INVALID : 0) | - (status & DIVBYZERO ? FE_DIVBYZERO : 0) | - (status & OVERFLOW ? FE_OVERFLOW : 0) | - (status & UNDERFLOW ? FE_UNDERFLOW : 0) | - (status & INEXACT ? FE_INEXACT : 0); +return ((status & INVALID) ? FE_INVALID : 0) | + ((status & DIVBYZERO) ? FE_DIVBYZERO : 0) | + ((status & OVERFLOW) ? FE_OVERFLOW : 0) | + ((status & UNDERFLOW) ? FE_UNDERFLOW : 0) | + ((status & INEXACT) ? FE_INEXACT : 0); } static uint32_t getControlWord() { diff --git a/libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h b/libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h index 5b59ba38d67bb6..feb48e3719bf16 100644 --- a/libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h +++ b/libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h @@ -63,39 +63,39 @@ struct FEnv { // located in a different place from FE_FLUSHTOZERO status bit relative to // the other exceptions. LIBC_INLINE static uint32_t exception_value_from_status(int status) { -return (status & FE_INVALID ? EX_INVALID : 0) | - (status & FE_DIVBYZERO ? EX_DIVBYZERO : 0) | - (status & FE_OVERFLOW ? EX_OVERFLOW : 0) | - (status & FE_UNDERFLOW ? EX_UNDERFLOW : 0) | -
[Lldb-commits] [clang] [lldb] [llvm] [AArch64][TargetParser] autogen ArchExtKind enum - renaming (PR #90320)
jthackray wrote: > The command-line names or FEAT_ names should probably be what we are aiming > for if we are changing them one-way or the other. Yes, standardising on FEAT_* names would be good to match the TRM, so we avoid the AEK_PREDRES/FEAT_SPECRES, AEK_PERFMON/FEAT_PMUv3, etc. mismatches. https://github.com/llvm/llvm-project/pull/90320 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [AArch64][TargetParser] autogen ArchExtKind enum - renaming (PR #90320)
davemgreen wrote: Rust (https://github.com/rust-lang/rust/blob/79734f1db8dbe322192dea32c0f6b80ab14c4c1d/compiler/rustc_codegen_llvm/src/llvm_util.rs#L229) and zig (https://github.com/ziglang/zig/blob/44db92d1ca90c9cfdfb29fe46f04ff8f11c80901/lib/std/Target/aarch64.zig#L43) are two examples of what I meant by external dependencies. They can adapt, especially if there are release notes, but there may be many more projects out there using the old names and it would be good if they kept working, if we can. https://github.com/llvm/llvm-project/pull/90320 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1632,6 +1669,96 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) { return qualified_name; } labath wrote: I am bothered by this name. I would expect that something called `FindDefinitionDIE` returns a DWARFDIE (or something along those lines). Instead it returns bool, and appears to do a lot more than simply finding a DIE (it constructs the full clang ast for it, iiuc). How about, mirroring the SymbolFileDWARF apis this wraps, we call this something like `FindDefinitionTypeForDIE` (and then actually have the function return the parsed Type object)? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -249,11 +270,10 @@ static void ForcefullyCompleteType(CompilerType type) { /// This function serves a similar purpose as RequireCompleteType above, but it /// avoids completing the type if it is not immediately necessary. It only /// ensures we _can_ complete the type later. -static void PrepareContextToReceiveMembers(TypeSystemClang &ast, - ClangASTImporter &ast_importer, - clang::DeclContext *decl_ctx, - DWARFDIE die, - const char *type_name_cstr) { +void DWARFASTParserClang::PrepareContextToReceiveMembers( +TypeSystemClang &ast, clang::DeclContext *decl_ctx, labath wrote: This arg is not necessary now that this is a member https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1632,6 +1669,96 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) { return qualified_name; } +bool DWARFASTParserClang::FindDefinitionDIE(const DWARFDIE &die, +bool &is_forward_declaration) { labath wrote: I'm having trouble finding where is this by-ref argument actually used in the caller. Can we get rid of it (perhaps by moving the `IsForwardDeclaration` call to the caller)? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/labath commented: Modulo comments, this makes sense to me (as much as that can ever be said about this code), but it could definitely use a second (third?) pair of eyes. Michael, what do you make of this? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1921,38 +1970,33 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, GetClangASTImporter().SetRecordLayout(record_decl, layout); } } -} else if (clang_type_was_created) { - // Start the definition if the class is not objective C since the - // underlying decls respond to isCompleteDefinition(). Objective - // C decls don't respond to isCompleteDefinition() so we can't - // start the declaration definition right away. For C++ - // class/union/structs we want to start the definition in case the - // class is needed as the declaration context for a contained class - // or type without the need to complete that type.. - - if (attrs.class_language != eLanguageTypeObjC && - attrs.class_language != eLanguageTypeObjC_plus_plus) -TypeSystemClang::StartTagDeclarationDefinition(clang_type); - - // Leave this as a forward declaration until we need to know the - // details of the type. lldb_private::Type will automatically call - // the SymbolFile virtual function - // "SymbolFileDWARF::CompleteType(Type *)" When the definition - // needs to be defined. - assert(!dwarf->GetForwardDeclCompilerTypeToDIE().count( - ClangUtil::RemoveFastQualifiers(clang_type) - .GetOpaqueQualType()) && - "Type already in the forward declaration map!"); - // Can't assume m_ast.GetSymbolFile() is actually a - // SymbolFileDWARF, it can be a SymbolFileDWARFDebugMap for Apple - // binaries. - dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace( - ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(), - *die.GetDIERef()); - m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true); } +// Start the definition if the class is not objective C since the +// underlying decls respond to isCompleteDefinition(). Objective +// C decls don't respond to isCompleteDefinition() so we can't +// start the declaration definition right away. For C++ +// class/union/structs we want to start the definition in case the +// class is needed as the declaration context for a contained class +// or type without the need to complete that type.. + +if (attrs.class_language != eLanguageTypeObjC && +attrs.class_language != eLanguageTypeObjC_plus_plus) + TypeSystemClang::StartTagDeclarationDefinition(clang_type); labath wrote: Is it a problem that `clang_type` can already have its definition started (and completed) on line 1944 (if the DIE has no children)? Should this maybe be in some sort of an else branch? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1921,38 +1970,33 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, GetClangASTImporter().SetRecordLayout(record_decl, layout); } } -} else if (clang_type_was_created) { - // Start the definition if the class is not objective C since the - // underlying decls respond to isCompleteDefinition(). Objective - // C decls don't respond to isCompleteDefinition() so we can't - // start the declaration definition right away. For C++ - // class/union/structs we want to start the definition in case the - // class is needed as the declaration context for a contained class - // or type without the need to complete that type.. - - if (attrs.class_language != eLanguageTypeObjC && - attrs.class_language != eLanguageTypeObjC_plus_plus) -TypeSystemClang::StartTagDeclarationDefinition(clang_type); - - // Leave this as a forward declaration until we need to know the - // details of the type. lldb_private::Type will automatically call - // the SymbolFile virtual function - // "SymbolFileDWARF::CompleteType(Type *)" When the definition - // needs to be defined. - assert(!dwarf->GetForwardDeclCompilerTypeToDIE().count( - ClangUtil::RemoveFastQualifiers(clang_type) - .GetOpaqueQualType()) && - "Type already in the forward declaration map!"); - // Can't assume m_ast.GetSymbolFile() is actually a - // SymbolFileDWARF, it can be a SymbolFileDWARFDebugMap for Apple - // binaries. - dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace( - ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(), - *die.GetDIERef()); - m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true); } +// Start the definition if the class is not objective C since the +// underlying decls respond to isCompleteDefinition(). Objective +// C decls don't respond to isCompleteDefinition() so we can't +// start the declaration definition right away. For C++ +// class/union/structs we want to start the definition in case the +// class is needed as the declaration context for a contained class +// or type without the need to complete that type.. + +if (attrs.class_language != eLanguageTypeObjC && +attrs.class_language != eLanguageTypeObjC_plus_plus) + TypeSystemClang::StartTagDeclarationDefinition(clang_type); } + // Leave this as a forward declaration until we need to know the labath wrote: I'm having trouble reconciling this comment with the one above (line 1974). How can we leave this as a forward declaration if we have already (conditionally, if we are processing a definition DIE of a non-objc type) started its definition (line 1984). What exactly is this trying to say? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -108,6 +108,9 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { lldb_private::ConstString GetDIEClassTemplateParams( const lldb_private::plugin::dwarf::DWARFDIE &die) override; labath wrote: delete `lldb_private::plugin::dwarf::` (and elsewhere in this file) https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -60,6 +60,12 @@ class DWARFASTParser { virtual ConstString GetDIEClassTemplateParams(const DWARFDIE &die) = 0; + // Return true if we found the definition DIE for it. is_forward_declaration + // is set to true if the parameter die is a declaration. + virtual bool + FindDefinitionDIE(const lldb_private::plugin::dwarf::DWARFDIE &die, labath wrote: Delete lldb_private::plugin::dwarf:: https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)
https://github.com/DavidSpickett approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/90917 ___ 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)
https://github.com/DavidSpickett edited 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)
DavidSpickett wrote: > And make the note at the end of lldb/docs/use/qemu-testing.rst outdated. Removing this is the last thing to do here. 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] Add register field enum class (PR #90063)
@@ -13,11 +13,42 @@ #include #include +#include "llvm/ADT/StringSet.h" + namespace lldb_private { class StreamString; class Log; +class FieldEnum { +public: + struct Enumerator { +uint64_t m_value; +// Short name for the value. Shown in tables and when printing the field's +// value. For example "RZ". +std::string m_name; + +Enumerator(uint64_t value, std::string name) +: m_value(value), m_name(name) {} + +void ToXML(StreamString &strm) const; + }; + + typedef std::vector Enumerators; + + FieldEnum(std::string id, const Enumerators &enumerators); + + const Enumerators &GetEnumerators() const { return m_enumerators; } + + const std::string &GetID() const { return m_id; } + + void ToXML(StreamString &strm, unsigned size) const; + +private: + std::string m_id; + std::vector m_enumerators; DavidSpickett wrote: Yes, I think I've done this in a few other places too. https://github.com/llvm/llvm-project/pull/90063 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add register field enum class (PR #90063)
@@ -13,11 +13,42 @@ #include #include +#include "llvm/ADT/StringSet.h" + namespace lldb_private { class StreamString; class Log; +class FieldEnum { +public: + struct Enumerator { +uint64_t m_value; +// Short name for the value. Shown in tables and when printing the field's +// value. For example "RZ". +std::string m_name; + +Enumerator(uint64_t value, std::string name) +: m_value(value), m_name(name) {} + +void ToXML(StreamString &strm) const; + }; + + typedef std::vector Enumerators; + + FieldEnum(std::string id, const Enumerators &enumerators); + + const Enumerators &GetEnumerators() const { return m_enumerators; } + + const std::string &GetID() const { return m_id; } + + void ToXML(StreamString &strm, unsigned size) const; DavidSpickett wrote: Just because the caller only uses StreamString, this can be Stream though no problem. https://github.com/llvm/llvm-project/pull/90063 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add register field enum class (PR #90063)
DavidSpickett wrote: Thanks for the reviews. The next PR has some test output that depends on the format added in https://github.com/llvm/llvm-project/pull/90059, which itself has some complexity I didn't expect. So I'm going to work on that PR first before updating this. https://github.com/llvm/llvm-project/pull/90063 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add ability to show enum as name and value at the same time (PR #90059)
DavidSpickett wrote: I found a few things: * Yes you can get these flags like enums with a register field as expected. * Only C++ code can manipulate a `DumpValueObjectOptions` object, there's no Python binding at all. * The only Python tests for it are things that call commands that then use the object e.g. watchpoints or evaluating enum expressions. They do not have the low level control like I am adding in this PR. I'm not sure of the utility of flag like enums as register fields yet but whatever I come up with, the lack of tests bothers me. So I'm trying to find a way to test `DumpValueObjectOptions` in relative isolation. https://github.com/llvm/llvm-project/pull/90059 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [libc] [libclc] [libcxxabi] [lld] [lldb] [llvm] [mlir] Add clarifying parenthesis around non-trivial conditions in ternary expressions. (PR #90391)
https://github.com/RKSimon approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/90391 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SBType::GetByteAlign (PR #90960)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/90960 lldb already mostly(*) tracks this information. This just makes it available to the SB users. (*) It does not do that for typedefs right now see llvm.org/pr90958 >From 738c13a680b945a0aea1dc762e7122b5ce5608ad Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Fri, 3 May 2024 11:18:15 + Subject: [PATCH] [lldb] Add SBType::GetByteAlign lldb already mostly(*) tracks this information. This just makes it available to the SB users. (*) It does not do that for typedefs right now see llvm.org/pr90958 --- lldb/include/lldb/API/SBType.h| 2 ++ lldb/source/API/SBType.cpp| 12 +++ lldb/test/API/python_api/type/TestTypeList.py | 21 +++ lldb/test/API/python_api/type/main.cpp| 3 +++ 4 files changed, 38 insertions(+) diff --git a/lldb/include/lldb/API/SBType.h b/lldb/include/lldb/API/SBType.h index 5b9ff2170b2b24..63ba91082d5769 100644 --- a/lldb/include/lldb/API/SBType.h +++ b/lldb/include/lldb/API/SBType.h @@ -150,6 +150,8 @@ class SBType { uint64_t GetByteSize(); + uint64_t GetByteAlign(); + bool IsPointerType(); bool IsReferenceType(); diff --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp index 6cecb5c9ea810b..f9a2a0548ef83a 100644 --- a/lldb/source/API/SBType.cpp +++ b/lldb/source/API/SBType.cpp @@ -25,6 +25,7 @@ #include "lldb/Utility/Stream.h" #include "llvm/ADT/APSInt.h" +#include "llvm/Support/MathExtras.h" #include #include @@ -132,6 +133,17 @@ uint64_t SBType::GetByteSize() { return 0; } +uint64_t SBType::GetByteAlign() { + LLDB_INSTRUMENT_VA(this); + + if (!IsValid()) return 0; + + std::optional bit_align = + m_opaque_sp->GetCompilerType(/*prefer_dynamic=*/false) + .GetTypeBitAlign(nullptr); + return llvm::divideCeil(bit_align.value_or(0), 8); +} + bool SBType::IsPointerType() { LLDB_INSTRUMENT_VA(this); diff --git a/lldb/test/API/python_api/type/TestTypeList.py b/lldb/test/API/python_api/type/TestTypeList.py index 17e27b624511cf..0498396903dcbd 100644 --- a/lldb/test/API/python_api/type/TestTypeList.py +++ b/lldb/test/API/python_api/type/TestTypeList.py @@ -272,3 +272,24 @@ def test(self): self.assertTrue(int_enum_uchar) self.DebugSBType(int_enum_uchar) self.assertEqual(int_enum_uchar.GetName(), "unsigned char") + +def test_GetByteAlign(self): +"""Exercise SBType::GetByteAlign""" +self.build() +spec = lldb.SBModuleSpec() +spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact())) +module = lldb.SBModule(spec) +self.assertTrue(module) + +# Invalid types should not crash. +self.assertEqual(lldb.SBType().GetByteAlign(), 0) + +# Try a type with natural alignment. +void_ptr = module.GetBasicType(lldb.eBasicTypeVoid).GetPointerType() +self.assertTrue(void_ptr) +# Not exactly guaranteed by the spec, but should be true everywhere we +# care about. +self.assertEqual(void_ptr.GetByteSize(), void_ptr.GetByteAlign()) + +# And an over-aligned type. + self.assertEqual(module.FindFirstType("OverAlignedStruct").GetByteAlign(), 128) diff --git a/lldb/test/API/python_api/type/main.cpp b/lldb/test/API/python_api/type/main.cpp index 7384a3d8da16fb..986ed3009a15f6 100644 --- a/lldb/test/API/python_api/type/main.cpp +++ b/lldb/test/API/python_api/type/main.cpp @@ -50,6 +50,9 @@ enum EnumType {}; enum class ScopedEnumType {}; enum class EnumUChar : unsigned char {}; +struct alignas(128) OverAlignedStruct {}; +OverAlignedStruct over_aligned_struct; + int main (int argc, char const *argv[]) { Task *task_head = new Task(-1, NULL); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SBType::GetByteAlign (PR #90960)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Pavel Labath (labath) Changes lldb already mostly(*) tracks this information. This just makes it available to the SB users. (*) It does not do that for typedefs right now see llvm.org/pr90958 --- Full diff: https://github.com/llvm/llvm-project/pull/90960.diff 4 Files Affected: - (modified) lldb/include/lldb/API/SBType.h (+2) - (modified) lldb/source/API/SBType.cpp (+12) - (modified) lldb/test/API/python_api/type/TestTypeList.py (+21) - (modified) lldb/test/API/python_api/type/main.cpp (+3) ``diff diff --git a/lldb/include/lldb/API/SBType.h b/lldb/include/lldb/API/SBType.h index 5b9ff2170b2b24..63ba91082d5769 100644 --- a/lldb/include/lldb/API/SBType.h +++ b/lldb/include/lldb/API/SBType.h @@ -150,6 +150,8 @@ class SBType { uint64_t GetByteSize(); + uint64_t GetByteAlign(); + bool IsPointerType(); bool IsReferenceType(); diff --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp index 6cecb5c9ea810b..f9a2a0548ef83a 100644 --- a/lldb/source/API/SBType.cpp +++ b/lldb/source/API/SBType.cpp @@ -25,6 +25,7 @@ #include "lldb/Utility/Stream.h" #include "llvm/ADT/APSInt.h" +#include "llvm/Support/MathExtras.h" #include #include @@ -132,6 +133,17 @@ uint64_t SBType::GetByteSize() { return 0; } +uint64_t SBType::GetByteAlign() { + LLDB_INSTRUMENT_VA(this); + + if (!IsValid()) return 0; + + std::optional bit_align = + m_opaque_sp->GetCompilerType(/*prefer_dynamic=*/false) + .GetTypeBitAlign(nullptr); + return llvm::divideCeil(bit_align.value_or(0), 8); +} + bool SBType::IsPointerType() { LLDB_INSTRUMENT_VA(this); diff --git a/lldb/test/API/python_api/type/TestTypeList.py b/lldb/test/API/python_api/type/TestTypeList.py index 17e27b624511cf..0498396903dcbd 100644 --- a/lldb/test/API/python_api/type/TestTypeList.py +++ b/lldb/test/API/python_api/type/TestTypeList.py @@ -272,3 +272,24 @@ def test(self): self.assertTrue(int_enum_uchar) self.DebugSBType(int_enum_uchar) self.assertEqual(int_enum_uchar.GetName(), "unsigned char") + +def test_GetByteAlign(self): +"""Exercise SBType::GetByteAlign""" +self.build() +spec = lldb.SBModuleSpec() +spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact())) +module = lldb.SBModule(spec) +self.assertTrue(module) + +# Invalid types should not crash. +self.assertEqual(lldb.SBType().GetByteAlign(), 0) + +# Try a type with natural alignment. +void_ptr = module.GetBasicType(lldb.eBasicTypeVoid).GetPointerType() +self.assertTrue(void_ptr) +# Not exactly guaranteed by the spec, but should be true everywhere we +# care about. +self.assertEqual(void_ptr.GetByteSize(), void_ptr.GetByteAlign()) + +# And an over-aligned type. + self.assertEqual(module.FindFirstType("OverAlignedStruct").GetByteAlign(), 128) diff --git a/lldb/test/API/python_api/type/main.cpp b/lldb/test/API/python_api/type/main.cpp index 7384a3d8da16fb..986ed3009a15f6 100644 --- a/lldb/test/API/python_api/type/main.cpp +++ b/lldb/test/API/python_api/type/main.cpp @@ -50,6 +50,9 @@ enum EnumType {}; enum class ScopedEnumType {}; enum class EnumUChar : unsigned char {}; +struct alignas(128) OverAlignedStruct {}; +OverAlignedStruct over_aligned_struct; + int main (int argc, char const *argv[]) { Task *task_head = new Task(-1, NULL); `` https://github.com/llvm/llvm-project/pull/90960 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SBType::GetByteAlign (PR #90960)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/90960 >From f6b47d2763b8217c329c6c5762ed75420ab3579b Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Fri, 3 May 2024 11:18:15 + Subject: [PATCH] [lldb] Add SBType::GetByteAlign lldb already mostly(*) tracks this information. This just makes it available to the SB users. (*) It does not do that for typedefs right now see llvm.org/pr90958 --- lldb/include/lldb/API/SBType.h| 2 ++ lldb/source/API/SBType.cpp| 13 lldb/test/API/python_api/type/TestTypeList.py | 21 +++ lldb/test/API/python_api/type/main.cpp| 3 +++ 4 files changed, 39 insertions(+) diff --git a/lldb/include/lldb/API/SBType.h b/lldb/include/lldb/API/SBType.h index 5b9ff2170b2b24..63ba91082d5769 100644 --- a/lldb/include/lldb/API/SBType.h +++ b/lldb/include/lldb/API/SBType.h @@ -150,6 +150,8 @@ class SBType { uint64_t GetByteSize(); + uint64_t GetByteAlign(); + bool IsPointerType(); bool IsReferenceType(); diff --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp index 6cecb5c9ea810b..8a063e5ad61d9d 100644 --- a/lldb/source/API/SBType.cpp +++ b/lldb/source/API/SBType.cpp @@ -25,6 +25,7 @@ #include "lldb/Utility/Stream.h" #include "llvm/ADT/APSInt.h" +#include "llvm/Support/MathExtras.h" #include #include @@ -132,6 +133,18 @@ uint64_t SBType::GetByteSize() { return 0; } +uint64_t SBType::GetByteAlign() { + LLDB_INSTRUMENT_VA(this); + + if (!IsValid()) +return 0; + + std::optional bit_align = + m_opaque_sp->GetCompilerType(/*prefer_dynamic=*/false) + .GetTypeBitAlign(nullptr); + return llvm::divideCeil(bit_align.value_or(0), 8); +} + bool SBType::IsPointerType() { LLDB_INSTRUMENT_VA(this); diff --git a/lldb/test/API/python_api/type/TestTypeList.py b/lldb/test/API/python_api/type/TestTypeList.py index 17e27b624511cf..0498396903dcbd 100644 --- a/lldb/test/API/python_api/type/TestTypeList.py +++ b/lldb/test/API/python_api/type/TestTypeList.py @@ -272,3 +272,24 @@ def test(self): self.assertTrue(int_enum_uchar) self.DebugSBType(int_enum_uchar) self.assertEqual(int_enum_uchar.GetName(), "unsigned char") + +def test_GetByteAlign(self): +"""Exercise SBType::GetByteAlign""" +self.build() +spec = lldb.SBModuleSpec() +spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact())) +module = lldb.SBModule(spec) +self.assertTrue(module) + +# Invalid types should not crash. +self.assertEqual(lldb.SBType().GetByteAlign(), 0) + +# Try a type with natural alignment. +void_ptr = module.GetBasicType(lldb.eBasicTypeVoid).GetPointerType() +self.assertTrue(void_ptr) +# Not exactly guaranteed by the spec, but should be true everywhere we +# care about. +self.assertEqual(void_ptr.GetByteSize(), void_ptr.GetByteAlign()) + +# And an over-aligned type. + self.assertEqual(module.FindFirstType("OverAlignedStruct").GetByteAlign(), 128) diff --git a/lldb/test/API/python_api/type/main.cpp b/lldb/test/API/python_api/type/main.cpp index 7384a3d8da16fb..986ed3009a15f6 100644 --- a/lldb/test/API/python_api/type/main.cpp +++ b/lldb/test/API/python_api/type/main.cpp @@ -50,6 +50,9 @@ enum EnumType {}; enum class ScopedEnumType {}; enum class EnumUChar : unsigned char {}; +struct alignas(128) OverAlignedStruct {}; +OverAlignedStruct over_aligned_struct; + int main (int argc, char const *argv[]) { Task *task_head = new Task(-1, NULL); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1664,13 +1793,40 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, } if (dwarf->GetUniqueDWARFASTTypeMap().Find( -unique_typename, die, unique_decl, attrs.byte_size.value_or(-1), -*unique_ast_entry_up)) { +unique_typename, die, unique_decl, byte_size, +attrs.is_forward_declaration, *unique_ast_entry_up)) { type_sp = unique_ast_entry_up->m_type_sp; if (type_sp) { dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get(); LinkDeclContextToDIE( GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die), die); +if (!attrs.is_forward_declaration) { + // If the parameter DIE is definition and the entry in the map is Michael137 wrote: I find `parameter DIE` a bit confusing. Made me think of templates or functions. Could we just reference the parameter, e.g., `If the 'die' being parsed is a definition ...` or something like that https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/Michael137 commented: The idea makes sense and I like that we could factor things out of `ParseStructureLikeDIE`, so generally LGTM (module Pavel's comments). Is any of it testable? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -16,60 +16,65 @@ using namespace lldb_private::plugin::dwarf; bool UniqueDWARFASTTypeList::Find(const DWARFDIE &die, const lldb_private::Declaration &decl, const int32_t byte_size, + bool is_forward_declaration, UniqueDWARFASTType &entry) const { for (const UniqueDWARFASTType &udt : m_collection) { // Make sure the tags match if (udt.m_die.Tag() == die.Tag()) { - // Validate byte sizes of both types only if both are valid. - if (udt.m_byte_size < 0 || byte_size < 0 || - udt.m_byte_size == byte_size) { -// Make sure the file and line match -if (udt.m_declaration == decl) { - // The type has the same name, and was defined on the same file and - // line. Now verify all of the parent DIEs match. - DWARFDIE parent_arg_die = die.GetParent(); - DWARFDIE parent_pos_die = udt.m_die.GetParent(); - bool match = true; - bool done = false; - while (!done && match && parent_arg_die && parent_pos_die) { -const dw_tag_t parent_arg_tag = parent_arg_die.Tag(); -const dw_tag_t parent_pos_tag = parent_pos_die.Tag(); -if (parent_arg_tag == parent_pos_tag) { - switch (parent_arg_tag) { - case DW_TAG_class_type: - case DW_TAG_structure_type: - case DW_TAG_union_type: - case DW_TAG_namespace: { -const char *parent_arg_die_name = parent_arg_die.GetName(); -if (parent_arg_die_name == -nullptr) // Anonymous (i.e. no-name) struct -{ + // If they are not both definition DIEs or both declaration DIEs, then + // don't check for byte size and declaration location, because declaration + // DIEs usually don't have those info. + bool matching_size_declaration = + udt.m_is_forward_declaration != is_forward_declaration + ? true + : (udt.m_byte_size < 0 || byte_size < 0 || + udt.m_byte_size == byte_size) && +udt.m_declaration == decl; + if (matching_size_declaration) { Michael137 wrote: For readability, can we do: ``` if (!match_size_declaration) continue; ``` ? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
labath wrote: > Is any of it testable? Good question. Though this is mostly meant to be "NFC" (with very large quotes), I can imagine us doing something like forcing the parsing of a specific type (`type lookup ` ?), and then checking that the module ast (`image dump ast`) does *not* contain specific types -- as that's basically what we're trying to achieve. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Fix test_exit_status_message_sigterm test. (PR #90223)
https://github.com/mbucko updated https://github.com/llvm/llvm-project/pull/90223 >From c699eff1500a431225d4b292a51a467bd2c74e5d Mon Sep 17 00:00:00 2001 From: Miro Bucko Date: Fri, 26 Apr 2024 08:17:26 -0700 Subject: [PATCH] [lldb-dap] Fix test_exit_status_message_sigterm test. Summary: 'test_exit_status_message_sigterm' is failing due to 'psutil' dependency introduced in PR #89405. This fix removes 'deque' dependency and checks if 'psutil' can be imported before running the test. If 'psutil' cannot be imported, it emits a warning and skips the test. Test Plan: ./bin/llvm-lit -sv /path-to-llvm-project/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py --filter=tools/lldb-dap/console/TestDAP_console.py Reviewers: @jeffreytan81,@clayborg,@kusmour, @JDevlieghere,@walter-erquinigo Subscribers: Tasks: lldb-dap Tags: --- .../tools/lldb-dap/console/TestDAP_console.py | 20 +-- 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py index 8f456aaf890c7f..8769f39633e62f 100644 --- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py +++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py @@ -4,17 +4,15 @@ import dap_server import lldbdap_testcase -import psutil -from collections import deque from lldbsuite.test import lldbutil from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * -def get_subprocess(process_name): -queue = deque([psutil.Process(os.getpid())]) +def get_subprocess(root_process, process_name): +queue = [root_process] while queue: -process = queue.popleft() +process = queue.pop() if process.name() == process_name: return process queue.extend(process.children()) @@ -131,7 +129,17 @@ def test_exit_status_message_sigterm(self): process_name = ( "debugserver" if platform.system() in ["Darwin"] else "lldb-server" ) -process = get_subprocess(process_name) + +try: +import psutil +except ImportError: +print( +"psutil not installed, please install using 'pip install psutil'. " +"Skipping test_exit_status_message_sigterm test.", +file=sys.stderr, +) +return +process = get_subprocess(psutil.Process(os.getpid()), process_name) process.terminate() process.wait() ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 327bfc9 - Revert "[lldb] Fix TestSharedLibStrippedSymbols for #90622"
Author: David Spickett Date: 2024-05-03T13:04:05Z New Revision: 327bfc971e4dce3f6798843c92406cda95c07ba1 URL: https://github.com/llvm/llvm-project/commit/327bfc971e4dce3f6798843c92406cda95c07ba1 DIFF: https://github.com/llvm/llvm-project/commit/327bfc971e4dce3f6798843c92406cda95c07ba1.diff LOG: Revert "[lldb] Fix TestSharedLibStrippedSymbols for #90622" And "LLDB Debuginfod tests and a fix or two (#90622)". f8fedfb6802173372ec923f99f31d4af810fbcb0 / 2d4acb086541577ac6ab3a140b9ceb9659ce7094 As it has caused a test failure on 32 bit Arm: https://lab.llvm.org/buildbot/#/builders/17/builds/52580 Expr/TestStringLiteralExpr.test. The follow up did fix lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py but not the other failure. Added: Modified: lldb/packages/Python/lldbsuite/test/make/Makefile.rules lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolLocator/CMakeLists.txt lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp lldb/test/API/lit.site.cfg.py.in lldb/test/CMakeLists.txt Removed: lldb/test/API/debuginfod/Normal/Makefile lldb/test/API/debuginfod/Normal/TestDebuginfod.py lldb/test/API/debuginfod/Normal/main.c lldb/test/API/debuginfod/SplitDWARF/Makefile lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py lldb/test/API/debuginfod/SplitDWARF/main.c diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules index b2f2516d8423f8..bd8eea3d6f5a04 100644 --- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules +++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules @@ -51,7 +51,7 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../ # # GNUWin32 uname gives "windows32" or "server version windows32" while # some versions of MSYS uname return "MSYS_NT*", but most environments -# standardize on "Windows_NT", so we'll make it consistent here. +# standardize on "Windows_NT", so we'll make it consistent here. # When running tests from Visual Studio, the environment variable isn't # inherited all the way down to the process spawned for make. #-- @@ -210,12 +210,6 @@ else ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" DSYM = $(EXE).debug endif - - ifeq "$(MAKE_DWP)" "YES" - MAKE_DWO := YES - DWP_NAME = $(EXE).dwp - DYLIB_DWP_NAME = $(DYLIB_NAME).dwp - endif endif LIMIT_DEBUG_INFO_FLAGS = @@ -364,7 +358,6 @@ ifneq "$(OS)" "Darwin" OBJCOPY ?= $(call replace_cc_with,objcopy) ARCHIVER ?= $(call replace_cc_with,ar) - DWP ?= $(call replace_cc_with,dwp) override AR = $(ARCHIVER) endif @@ -535,10 +528,6 @@ ifneq "$(CXX)" "" endif endif -ifeq "$(GEN_GNU_BUILD_ID)" "YES" - LDFLAGS += -Wl,--build-id -endif - #-- # DYLIB_ONLY variable can be used to skip the building of a.out. # See the sections below regarding dSYM file as well as the building of @@ -577,17 +566,10 @@ else endif else ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" -ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES" - cp "$(EXE)" "$(EXE).unstripped" -endif $(OBJCOPY) --only-keep-debug "$(EXE)" "$(DSYM)" $(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DSYM)" "$(EXE)" "$(EXE)" endif -ifeq "$(MAKE_DWP)" "YES" - $(DWP) -o "$(DWP_NAME)" $(DWOS) endif -endif - #-- # Make the dylib @@ -629,15 +611,9 @@ endif else $(LD) $(DYLIB_OBJECTS) $(LDFLAGS) -shared -o "$(DYLIB_FILENAME)" ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" -ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES" - cp "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).unstripped" -endif $(OBJCOPY) --only-keep-debug "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).debug" $(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DYLIB_FILENAME).debug" "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME)" endif -ifeq "$(MAKE_DWP)" "YES" - $(DWP) -o $(DYLIB_DWP_FILE) $(DYLIB_DWOS) -endif endif #-- diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index dafdf241f9db0c..49f13d2c89e380 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -4378,38 +4378,26 @@ const std::shared_ptr &SymbolFileDWARF::GetDwpSymbolFile() { FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths(); ModuleSpec module_spec; module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec(); -FileSpec dwp_filespec; for (const auto &symfile : symfiles.files()) { module_spec
[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)
@@ -87,8 +105,15 @@ SymbolVendorELF::CreateInstance(const lldb::ModuleSP &module_sp, FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths(); FileSpec dsym_fspec = PluginManager::LocateExecutableSymbolFile(module_spec, search_paths); - if (!dsym_fspec) -return nullptr; + if (!dsym_fspec || IsDwpSymbolFile(module_sp, dsym_fspec)) { +// If we have a stripped binary or if we got a DWP file, we should prefer +// symbols in the executable acquired through a plugin. +ModuleSpec unstripped_spec = +PluginManager::LocateExecutableObjectFile(module_spec); +if (!unstripped_spec) + return nullptr; +dsym_fspec = unstripped_spec.GetFileSpec(); + } DavidSpickett wrote: Something about this if block has broken `Expr/TestStringLiteralExpr.test` on Arm 32 bit, a platform that has been sensitive to changes in this area in the past. The test is built without debug info and something happens when the test file comes through here. For ld.so and the libc it's all the same as before. ``` (lldb) expr "hello there" lldb ProcessGDBRemote::DoAllocateMemory no direct stub support for memory allocation, and InferiorCallMmap also failed - is stub missing register context save/restore capability? ``` Last time this happened it was because we lost the symbols that let us call `mmap`, but those are in ld.so usually so I'm not sure what's the problem this time. This means we fall back the interpreter, which we didn't need to do before. ``` error: Can't evaluate the expression without a running target due to: Interpreter doesn't handle one of the expression's operands ``` I've reverted this (https://github.com/llvm/llvm-project/commit/327bfc971e4dce3f6798843c92406cda95c07ba1) and the follow up while I investigate locally. https://github.com/llvm/llvm-project/pull/90622 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/90622 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
Michael137 wrote: > > Is any of it testable? > > Good question. Though this is mostly meant to be "NFC" (with very large > quotes), I can imagine us doing something like forcing the parsing of a > specific type (`type lookup ` ?), and then checking that the > module ast (`image dump ast`) does _not_ contain specific types -- as that's > basically what we're trying to achieve. Yea that could work. But if it's going to be very painful or fragile to test then don't let that hold back the PR https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SBType::GetByteAlign (PR #90960)
https://github.com/Michael137 approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/90960 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
llvm-beanz wrote: > I don't know if the pre-commit testing guarantees that. Configuration > settings will permit the files to be checked out in either Unix (`\n`) or > Windows (`\r\n`) line-endings. Today on Windows you basically have to check out LLVM as unix line endings. There are a bunch of tests that fail if you don't. Hopefully this is a step toward fixing that (which is why I'm in favor of this). > If the builders are all configured to run in Unix line endings we lose that > coverage. I suspect the Windows bots are basically all configured that way today. Our instructions tell people they need to disable `autocrlf` (see: https://llvm.org/docs/GettingStarted.html#getting-the-source-code-and-building-llvm), and not disabling it causes problems for users still (see this message from today: https://discord.com/channels/636084430946959380/745925509971705877/1235913415902629958). >While I understand that this change only changes how the LLVM sources are >stored, the issue is that the LLVM sources include the _tests_ directory. >These need to be tested in various manners to ensure that we test how we >handle the different inputs (in clang). One option might be to exclude the >tests directory from the line ending conversion if we cannot verify that we >have tests in both formats. I see two issues with this: 1) because the repo doesn't work with autocrlf today, we actually can't rely on testing on different platforms to provide this coverage. 2) We shouldn't be relying on git settings to get this coverage anyways. We should have tests that require specific line endings, and we should use gitattributes to ensure that we run those tests regardless of the user's git settings. > While, yes, it is possible to get the wrong behaviour currently, if the user > configures git explicitly for a line-ending, I would expect them to be aware > of that. The use of `.gitattributes` means that their defaults are not > necessarily honoured and thus this can catch them by surprise. I think today not only is it possible to get the wrong behavior, if you're developing on Windows Git's default behavior causes the wrong behavior for LLVM. Which often means that contributors (especially new contributors) have no idea that they're doing something wrong. I don't know that this PR is perfect, but IMO it is a huge step in the right direction to allow Windows contributors to build, test and contribute to LLVM using the default configurations of the OS and tools. https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu updated https://github.com/llvm/llvm-project/pull/90663 >From 4e83099b593e66f12dc21be5fbac5279e03e Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Tue, 30 Apr 2024 16:23:11 -0400 Subject: [PATCH 1/4] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 270 +++--- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 105 ++- .../SymbolFile/DWARF/SymbolFileDWARF.h| 14 + .../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 5 + .../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 2 + .../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 95 +++--- .../SymbolFile/DWARF/UniqueDWARFASTType.h | 21 +- 7 files changed, 296 insertions(+), 216 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index bea11e0e3840af..7ad661c9a9d49b 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -252,7 +252,8 @@ static void ForcefullyCompleteType(CompilerType type) { static void PrepareContextToReceiveMembers(TypeSystemClang &ast, ClangASTImporter &ast_importer, clang::DeclContext *decl_ctx, - DWARFDIE die, + const DWARFDIE &decl_ctx_die, + const DWARFDIE &die, const char *type_name_cstr) { auto *tag_decl_ctx = clang::dyn_cast(decl_ctx); if (!tag_decl_ctx) @@ -279,6 +280,13 @@ static void PrepareContextToReceiveMembers(TypeSystemClang &ast, type_name_cstr ? type_name_cstr : "", die.GetOffset()); } + // By searching for the definition DIE of the decl_ctx type, we will either: + // 1. Found the the definition DIE and start its definition with + // TypeSystemClang::StartTagDeclarationDefinition. + // 2. Unable to find it, then need to forcefully complete it. + die.GetDWARF()->FindDefinitionDIE(decl_ctx_die); + if (tag_decl_ctx->isCompleteDefinition() || tag_decl_ctx->isBeingDefined()) +return; // We don't have a type definition and/or the import failed. We must // forcefully complete the type to avoid crashes. ForcefullyCompleteType(type); @@ -620,10 +628,11 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc, if (tag == DW_TAG_typedef) { // DeclContext will be populated when the clang type is materialized in // Type::ResolveCompilerType. -PrepareContextToReceiveMembers( -m_ast, GetClangASTImporter(), -GetClangDeclContextContainingDIE(die, nullptr), die, -attrs.name.GetCString()); +DWARFDIE decl_ctx_die; +clang::DeclContext *decl_ctx = +GetClangDeclContextContainingDIE(die, &decl_ctx_die); +PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(), decl_ctx, + decl_ctx_die, die, attrs.name.GetCString()); if (attrs.type.IsValid()) { // Try to parse a typedef from the (DWARF embedded in the) Clang @@ -1100,32 +1109,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, // struct and see if this is actually a C++ method Type *class_type = dwarf->ResolveType(decl_ctx_die); if (class_type) { - if (class_type->GetID() != decl_ctx_die.GetID() || - IsClangModuleFwdDecl(decl_ctx_die)) { - -// We uniqued the parent class of this function to another -// class so we now need to associate all dies under -// "decl_ctx_die" to DIEs in the DIE for "class_type"... -DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID()); - -if (class_type_die) { - std::vector failures; - - CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die, - class_type, failures); - - // FIXME do something with these failures that's - // smarter than just dropping them on the ground. - // Unfortunately classes don't like having stuff added - // to them after their definitions are complete... - - Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()]; - if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) { -return type_ptr->shared_from_this(); - } -} - } - if (attrs.specification.IsValid()) { // We have a specification which we are going to base our // function prototype off of, so we need this type to be @@ -1260,6 +1243,39 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, } } } + // By here, we should have already completed t
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -249,11 +270,10 @@ static void ForcefullyCompleteType(CompilerType type) { /// This function serves a similar purpose as RequireCompleteType above, but it /// avoids completing the type if it is not immediately necessary. It only /// ensures we _can_ complete the type later. -static void PrepareContextToReceiveMembers(TypeSystemClang &ast, - ClangASTImporter &ast_importer, - clang::DeclContext *decl_ctx, - DWARFDIE die, - const char *type_name_cstr) { +void DWARFASTParserClang::PrepareContextToReceiveMembers( +TypeSystemClang &ast, clang::DeclContext *decl_ctx, ZequanWu wrote: Done. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -16,60 +16,65 @@ using namespace lldb_private::plugin::dwarf; bool UniqueDWARFASTTypeList::Find(const DWARFDIE &die, const lldb_private::Declaration &decl, const int32_t byte_size, + bool is_forward_declaration, UniqueDWARFASTType &entry) const { for (const UniqueDWARFASTType &udt : m_collection) { // Make sure the tags match if (udt.m_die.Tag() == die.Tag()) { - // Validate byte sizes of both types only if both are valid. - if (udt.m_byte_size < 0 || byte_size < 0 || - udt.m_byte_size == byte_size) { -// Make sure the file and line match -if (udt.m_declaration == decl) { - // The type has the same name, and was defined on the same file and - // line. Now verify all of the parent DIEs match. - DWARFDIE parent_arg_die = die.GetParent(); - DWARFDIE parent_pos_die = udt.m_die.GetParent(); - bool match = true; - bool done = false; - while (!done && match && parent_arg_die && parent_pos_die) { -const dw_tag_t parent_arg_tag = parent_arg_die.Tag(); -const dw_tag_t parent_pos_tag = parent_pos_die.Tag(); -if (parent_arg_tag == parent_pos_tag) { - switch (parent_arg_tag) { - case DW_TAG_class_type: - case DW_TAG_structure_type: - case DW_TAG_union_type: - case DW_TAG_namespace: { -const char *parent_arg_die_name = parent_arg_die.GetName(); -if (parent_arg_die_name == -nullptr) // Anonymous (i.e. no-name) struct -{ + // If they are not both definition DIEs or both declaration DIEs, then + // don't check for byte size and declaration location, because declaration + // DIEs usually don't have those info. + bool matching_size_declaration = + udt.m_is_forward_declaration != is_forward_declaration + ? true + : (udt.m_byte_size < 0 || byte_size < 0 || + udt.m_byte_size == byte_size) && +udt.m_declaration == decl; + if (matching_size_declaration) { ZequanWu wrote: Done. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1632,6 +1669,96 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) { return qualified_name; } +bool DWARFASTParserClang::FindDefinitionDIE(const DWARFDIE &die, +bool &is_forward_declaration) { ZequanWu wrote: I was about to use it in SymbolFileDWARF::CompleteType: https://github.com/llvm/llvm-project/pull/90663/files#diff-edef3a65d5d569bbb75a4158d35b827aa5d42ee03ccd3b0c1d4f354afa12210cR1645. But with some refactor change, it's no longer useful anymore. Removed. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1632,6 +1669,96 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) { return qualified_name; } ZequanWu wrote: Yes, it does more than finding the DIE. Updated to return `Type*` and renamed to `FindDefinitionTypeForDIE`. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu commented: > Though this is mostly meant to be "NFC" (with very large quotes) Yeah, this is mostly "NFC". A noticeable difference is we now set the type created from declaration with `TypeSystemClang::SetHasExternalStorage` without knowing if there's a definition or not. Based on my knowledge, it simply tells clang that the type can be completed by external AST source and ultimately calls `SymbolFileDWARF::CompleteType` to complete it. If there isn't one, we just fail complete it. Maybe we should also force completion of the type in this case? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -108,6 +108,9 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { lldb_private::ConstString GetDIEClassTemplateParams( const lldb_private::plugin::dwarf::DWARFDIE &die) override; ZequanWu wrote: Done. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1921,38 +1970,33 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, GetClangASTImporter().SetRecordLayout(record_decl, layout); } } -} else if (clang_type_was_created) { - // Start the definition if the class is not objective C since the - // underlying decls respond to isCompleteDefinition(). Objective - // C decls don't respond to isCompleteDefinition() so we can't - // start the declaration definition right away. For C++ - // class/union/structs we want to start the definition in case the - // class is needed as the declaration context for a contained class - // or type without the need to complete that type.. - - if (attrs.class_language != eLanguageTypeObjC && - attrs.class_language != eLanguageTypeObjC_plus_plus) -TypeSystemClang::StartTagDeclarationDefinition(clang_type); - - // Leave this as a forward declaration until we need to know the - // details of the type. lldb_private::Type will automatically call - // the SymbolFile virtual function - // "SymbolFileDWARF::CompleteType(Type *)" When the definition - // needs to be defined. - assert(!dwarf->GetForwardDeclCompilerTypeToDIE().count( - ClangUtil::RemoveFastQualifiers(clang_type) - .GetOpaqueQualType()) && - "Type already in the forward declaration map!"); - // Can't assume m_ast.GetSymbolFile() is actually a - // SymbolFileDWARF, it can be a SymbolFileDWARFDebugMap for Apple - // binaries. - dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace( - ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(), - *die.GetDIERef()); - m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true); } +// Start the definition if the class is not objective C since the +// underlying decls respond to isCompleteDefinition(). Objective +// C decls don't respond to isCompleteDefinition() so we can't +// start the declaration definition right away. For C++ +// class/union/structs we want to start the definition in case the +// class is needed as the declaration context for a contained class +// or type without the need to complete that type.. + +if (attrs.class_language != eLanguageTypeObjC && +attrs.class_language != eLanguageTypeObjC_plus_plus) + TypeSystemClang::StartTagDeclarationDefinition(clang_type); ZequanWu wrote: Oh, I didn't notice it. Moved this into an else branch. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1921,38 +1970,33 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, GetClangASTImporter().SetRecordLayout(record_decl, layout); } } -} else if (clang_type_was_created) { - // Start the definition if the class is not objective C since the - // underlying decls respond to isCompleteDefinition(). Objective - // C decls don't respond to isCompleteDefinition() so we can't - // start the declaration definition right away. For C++ - // class/union/structs we want to start the definition in case the - // class is needed as the declaration context for a contained class - // or type without the need to complete that type.. - - if (attrs.class_language != eLanguageTypeObjC && - attrs.class_language != eLanguageTypeObjC_plus_plus) -TypeSystemClang::StartTagDeclarationDefinition(clang_type); - - // Leave this as a forward declaration until we need to know the - // details of the type. lldb_private::Type will automatically call - // the SymbolFile virtual function - // "SymbolFileDWARF::CompleteType(Type *)" When the definition - // needs to be defined. - assert(!dwarf->GetForwardDeclCompilerTypeToDIE().count( - ClangUtil::RemoveFastQualifiers(clang_type) - .GetOpaqueQualType()) && - "Type already in the forward declaration map!"); - // Can't assume m_ast.GetSymbolFile() is actually a - // SymbolFileDWARF, it can be a SymbolFileDWARFDebugMap for Apple - // binaries. - dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace( - ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(), - *die.GetDIERef()); - m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true); } +// Start the definition if the class is not objective C since the +// underlying decls respond to isCompleteDefinition(). Objective +// C decls don't respond to isCompleteDefinition() so we can't +// start the declaration definition right away. For C++ +// class/union/structs we want to start the definition in case the +// class is needed as the declaration context for a contained class +// or type without the need to complete that type.. + +if (attrs.class_language != eLanguageTypeObjC && +attrs.class_language != eLanguageTypeObjC_plus_plus) + TypeSystemClang::StartTagDeclarationDefinition(clang_type); } + // Leave this as a forward declaration until we need to know the ZequanWu wrote: I didn't modify the original comments. It doesn't make sense to me as well. I append "If this is a declaration DIE" in front of this comment. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -60,6 +60,12 @@ class DWARFASTParser { virtual ConstString GetDIEClassTemplateParams(const DWARFDIE &die) = 0; + // Return true if we found the definition DIE for it. is_forward_declaration + // is set to true if the parameter die is a declaration. + virtual bool + FindDefinitionDIE(const lldb_private::plugin::dwarf::DWARFDIE &die, ZequanWu wrote: Done. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1664,13 +1793,40 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, } if (dwarf->GetUniqueDWARFASTTypeMap().Find( -unique_typename, die, unique_decl, attrs.byte_size.value_or(-1), -*unique_ast_entry_up)) { +unique_typename, die, unique_decl, byte_size, +attrs.is_forward_declaration, *unique_ast_entry_up)) { type_sp = unique_ast_entry_up->m_type_sp; if (type_sp) { dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get(); LinkDeclContextToDIE( GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die), die); +if (!attrs.is_forward_declaration) { + // If the parameter DIE is definition and the entry in the map is ZequanWu wrote: Done. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] bab1098 - [lldb-dap] Fix test_exit_status_message_sigterm test. (#90223)
Author: Miro Bucko Date: 2024-05-03T08:10:02-07:00 New Revision: bab10981f3c7a15231b8e2008f99a38a5582e3b0 URL: https://github.com/llvm/llvm-project/commit/bab10981f3c7a15231b8e2008f99a38a5582e3b0 DIFF: https://github.com/llvm/llvm-project/commit/bab10981f3c7a15231b8e2008f99a38a5582e3b0.diff LOG: [lldb-dap] Fix test_exit_status_message_sigterm test. (#90223) Summary: 'test_exit_status_message_sigterm' is failing due to 'psutil' dependency introduced in PR #89405. This fix removes 'deque' dependency and checks if 'psutil' can be imported before running the test. If 'psutil' cannot be imported, it emits a warning and skips the test. Test Plan: ./bin/llvm-lit -sv /path-to-llvm-project/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py --filter=tools/lldb-dap/console/TestDAP_console.py Reviewers: @jeffreytan81,@clayborg,@kusmour, @JDevlieghere,@walter-erquinigo Subscribers: Tasks: lldb-dap Tags: Added: Modified: lldb/test/API/tools/lldb-dap/console/TestDAP_console.py Removed: diff --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py index 8f456aaf890c7f..8769f39633e62f 100644 --- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py +++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py @@ -4,17 +4,15 @@ import dap_server import lldbdap_testcase -import psutil -from collections import deque from lldbsuite.test import lldbutil from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * -def get_subprocess(process_name): -queue = deque([psutil.Process(os.getpid())]) +def get_subprocess(root_process, process_name): +queue = [root_process] while queue: -process = queue.popleft() +process = queue.pop() if process.name() == process_name: return process queue.extend(process.children()) @@ -131,7 +129,17 @@ def test_exit_status_message_sigterm(self): process_name = ( "debugserver" if platform.system() in ["Darwin"] else "lldb-server" ) -process = get_subprocess(process_name) + +try: +import psutil +except ImportError: +print( +"psutil not installed, please install using 'pip install psutil'. " +"Skipping test_exit_status_message_sigterm test.", +file=sys.stderr, +) +return +process = get_subprocess(psutil.Process(os.getpid()), process_name) process.terminate() process.wait() ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Fix test_exit_status_message_sigterm test. (PR #90223)
https://github.com/jeffreytan81 closed https://github.com/llvm/llvm-project/pull/90223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)
https://github.com/royitaqi edited https://github.com/llvm/llvm-project/pull/90703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
ldrumm wrote: > There are a bunch of tests that fail if you don't. Yes. Even better than that: this patch just uncovered a [legitimate bug in the C++ AST parser tests for clang > Our instructions tell people they need to disable autocrlf I didn't see that. Shall I remove that instruction now, or after we've merged? > I don't know that this PR is perfect, but IMO it is a huge step in the right > direction to allow Windows contributors to build, test and contribute to LLVM > using the default configurations of the OS and tools. Absolutely. This patch is not a panacea but it enables us to correctly formalize the case where we actually *do* care about line-endings. Before this patch, it was basically incredibly brittle, but now we actually have control for those tests that matter. For things that don't really matter a whole class of irritating paper-cuts go away forever. I understand folks' hesitance around downstream rebases, but since this will go in as two commits, resolving a merge conflict becomes an easy step (I've [given instructions in the commit that will cause conflicts](https://github.com/llvm/llvm-project/pull/86318/commits/0b70d26534ac788741dc412777fa3396f8c1407c)). I've bumped downstream llvm projects through many major releases in my life (llvm v3 through 13), and change-of-method-name has been a much greater source of merge conflicts than whitespace. LLVM's codebase is healthy and clean largely because we give people the ability to making sweeping improvements. In any case, this will be the last time that a downstream will need to resolve major CRLF changes during a bump, so I consider that a long-term win too. https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)
@@ -87,8 +105,15 @@ SymbolVendorELF::CreateInstance(const lldb::ModuleSP &module_sp, FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths(); FileSpec dsym_fspec = PluginManager::LocateExecutableSymbolFile(module_spec, search_paths); - if (!dsym_fspec) -return nullptr; + if (!dsym_fspec || IsDwpSymbolFile(module_sp, dsym_fspec)) { +// If we have a stripped binary or if we got a DWP file, we should prefer +// symbols in the executable acquired through a plugin. +ModuleSpec unstripped_spec = +PluginManager::LocateExecutableObjectFile(module_spec); +if (!unstripped_spec) + return nullptr; +dsym_fspec = unstripped_spec.GetFileSpec(); + } DavidSpickett wrote: We try to allocate memory to JIT the expression and calling `mmap` fails with a signal. In https://github.com/llvm/llvm-project/issues/68987 this was because we had lost the section info that told us whether to call it as Thumb or Arm, if we get that wrong it causes a SIGILL, so it could be the same thing again. I will look into it more next week and assuming I find a fix, reland the changes for you. https://github.com/llvm/llvm-project/pull/90622 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Unify CalculateMD5 return types (PR #90921)
https://github.com/JDevlieghere approved this pull request. Thanks! LGTM. https://github.com/llvm/llvm-project/pull/90921 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/ZequanWu edited https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)
@@ -1347,6 +1347,12 @@ enum DebuggerBroadcastBit { eBroadcastBitProgressCategory = (1 << 3), }; +enum Severity { + eSeverityError, + eSeverityWarning, + eSeverityInfo, adrian-prantl wrote: Can you comment that "Clang also calls these Remarks, it's conceptually the same level" https://github.com/llvm/llvm-project/pull/90917 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)
@@ -1347,6 +1347,12 @@ enum DebuggerBroadcastBit { eBroadcastBitProgressCategory = (1 << 3), }; +enum Severity { + eSeverityError, + eSeverityWarning, + eSeverityInfo, adrian-prantl wrote: Or even `eSeverityRemark = eSeverityInfo` https://github.com/llvm/llvm-project/pull/90917 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)
@@ -1347,6 +1347,12 @@ enum DebuggerBroadcastBit { eBroadcastBitProgressCategory = (1 << 3), }; +enum Severity { adrian-prantl wrote: Would `enum class` prevent people from accidentally casting this to an unrelated enum with similar purpose but potentially different values? https://github.com/llvm/llvm-project/pull/90917 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)
https://github.com/adrian-prantl approved this pull request. https://github.com/llvm/llvm-project/pull/90917 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)
@@ -1347,6 +1347,12 @@ enum DebuggerBroadcastBit { eBroadcastBitProgressCategory = (1 << 3), }; +enum Severity { JDevlieghere wrote: It would, but `lldb-enumerations.h` exclusively uses `enum` and I think consistency outweighs the small benefit of using `enum class`. FWIW this came up previously in https://github.com/llvm/llvm-project/pull/80167#issuecomment-2043721298 too. https://github.com/llvm/llvm-project/pull/90917 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)
@@ -1347,6 +1347,12 @@ enum DebuggerBroadcastBit { eBroadcastBitProgressCategory = (1 << 3), }; +enum Severity { + eSeverityError, + eSeverityWarning, + eSeverityInfo, JDevlieghere wrote: I considered the latter, but we use `eSeverityRemark` so infrequently and not even always for `DiagnosticsEngine::Level::Remark` that I decided against it. The comment however seems like a good compromise. https://github.com/llvm/llvm-project/pull/90917 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/90917 >From 23b16ba8418f03dd11190798ccddf218cbfaf3f1 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 2 May 2024 16:44:18 -0700 Subject: [PATCH 1/2] [lldb] Create a single Severity enum in lldb-enumerations We have 3 different enums all expressing severity (info, warning, error). Remove all uses with a new Severity enum in lldb-enumerations.h. --- lldb/include/lldb/Core/Debugger.h | 3 +- lldb/include/lldb/Core/DebuggerEvents.h | 14 ++--- .../lldb/Expression/DiagnosticManager.h | 18 ++ lldb/include/lldb/Host/Host.h | 9 +-- lldb/include/lldb/lldb-enumerations.h | 6 ++ lldb/source/Core/Debugger.cpp | 37 ++-- lldb/source/Core/DebuggerEvents.cpp | 10 ++-- lldb/source/Expression/DiagnosticManager.cpp | 16 ++--- lldb/source/Expression/FunctionCaller.cpp | 25 lldb/source/Expression/LLVMUserExpression.cpp | 39 ++--- lldb/source/Host/common/Host.cpp | 22 +++ lldb/source/Host/macosx/objcxx/Host.mm| 10 ++-- .../ExpressionParser/Clang/ClangDiagnostic.h | 2 +- .../Clang/ClangExpressionDeclMap.cpp | 3 +- .../Clang/ClangExpressionParser.cpp | 14 ++--- .../Clang/ClangFunctionCaller.cpp | 4 +- .../Clang/ClangUserExpression.cpp | 34 +-- .../Clang/ClangUtilityFunction.cpp| 15 +++-- lldb/source/Target/Process.cpp| 27 + .../Expression/DiagnosticManagerTest.cpp | 58 +++ 20 files changed, 163 insertions(+), 203 deletions(-) diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index 49ff0737acef82..c0f7c732ad2d46 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -630,8 +630,7 @@ class Debugger : public std::enable_shared_from_this, std::optional debugger_id, uint32_t progress_category_bit = eBroadcastBitProgress); - static void ReportDiagnosticImpl(DiagnosticEventData::Type type, - std::string message, + static void ReportDiagnosticImpl(lldb::Severity severity, std::string message, std::optional debugger_id, std::once_flag *once); diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h index 74bb05e6e6bf88..49a4ecf8e537e3 100644 --- a/lldb/include/lldb/Core/DebuggerEvents.h +++ b/lldb/include/lldb/Core/DebuggerEvents.h @@ -76,19 +76,15 @@ class ProgressEventData : public EventData { class DiagnosticEventData : public EventData { public: - enum class Type { -Info, -Warning, -Error, - }; - DiagnosticEventData(Type type, std::string message, bool debugger_specific) - : m_message(std::move(message)), m_type(type), + DiagnosticEventData(lldb::Severity severity, std::string message, + bool debugger_specific) + : m_message(std::move(message)), m_severity(severity), m_debugger_specific(debugger_specific) {} ~DiagnosticEventData() override = default; const std::string &GetMessage() const { return m_message; } bool IsDebuggerSpecific() const { return m_debugger_specific; } - Type GetType() const { return m_type; } + lldb::Severity GetSeverity() const { return m_severity; } llvm::StringRef GetPrefix() const; @@ -105,7 +101,7 @@ class DiagnosticEventData : public EventData { protected: std::string m_message; - Type m_type; + lldb::Severity m_severity; const bool m_debugger_specific; DiagnosticEventData(const DiagnosticEventData &) = delete; diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h index 06bf1d115f1541..d49b7c99b114fb 100644 --- a/lldb/include/lldb/Expression/DiagnosticManager.h +++ b/lldb/include/lldb/Expression/DiagnosticManager.h @@ -28,12 +28,6 @@ enum DiagnosticOrigin { eDiagnosticOriginLLVM }; -enum DiagnosticSeverity { - eDiagnosticSeverityError, - eDiagnosticSeverityWarning, - eDiagnosticSeverityRemark -}; - const uint32_t LLDB_INVALID_COMPILER_ID = UINT32_MAX; class Diagnostic { @@ -55,7 +49,7 @@ class Diagnostic { } } - Diagnostic(llvm::StringRef message, DiagnosticSeverity severity, + Diagnostic(llvm::StringRef message, lldb::Severity severity, DiagnosticOrigin origin, uint32_t compiler_id) : m_message(message), m_severity(severity), m_origin(origin), m_compiler_id(compiler_id) {} @@ -68,7 +62,7 @@ class Diagnostic { virtual bool HasFixIts() const { return false; } - DiagnosticSeverity GetSeverity() const { return m_severity; } + lldb::Severity GetSeverity() const { return m_severity; } uint32_t GetCompilerID() const { return m_compiler_id; } @@ -83,7 +77,7 @@ class Diagnostic {
[Lldb-commits] [lldb] [lldb] Create a single Severity enum in lldb-enumerations (PR #90917)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/90917 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 528f5ba - [lldb] Create a single Severity enum in lldb-enumerations (#90917)
Author: Jonas Devlieghere Date: 2024-05-03T09:25:38-07:00 New Revision: 528f5ba7af2729bb7e23f0846b75e4f25af2bf8d URL: https://github.com/llvm/llvm-project/commit/528f5ba7af2729bb7e23f0846b75e4f25af2bf8d DIFF: https://github.com/llvm/llvm-project/commit/528f5ba7af2729bb7e23f0846b75e4f25af2bf8d.diff LOG: [lldb] Create a single Severity enum in lldb-enumerations (#90917) We have 3 different enums all expressing severity (info, warning, error). Remove all uses with a new Severity enum in lldb-enumerations.h. Added: Modified: lldb/include/lldb/Core/Debugger.h lldb/include/lldb/Core/DebuggerEvents.h lldb/include/lldb/Expression/DiagnosticManager.h lldb/include/lldb/Host/Host.h lldb/include/lldb/lldb-enumerations.h lldb/source/Core/Debugger.cpp lldb/source/Core/DebuggerEvents.cpp lldb/source/Expression/DiagnosticManager.cpp lldb/source/Expression/FunctionCaller.cpp lldb/source/Expression/LLVMUserExpression.cpp lldb/source/Host/common/Host.cpp lldb/source/Host/macosx/objcxx/Host.mm lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangFunctionCaller.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp lldb/source/Target/Process.cpp lldb/unittests/Expression/DiagnosticManagerTest.cpp Removed: diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index 49ff0737acef82..c0f7c732ad2d46 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -630,8 +630,7 @@ class Debugger : public std::enable_shared_from_this, std::optional debugger_id, uint32_t progress_category_bit = eBroadcastBitProgress); - static void ReportDiagnosticImpl(DiagnosticEventData::Type type, - std::string message, + static void ReportDiagnosticImpl(lldb::Severity severity, std::string message, std::optional debugger_id, std::once_flag *once); diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h index 74bb05e6e6bf88..49a4ecf8e537e3 100644 --- a/lldb/include/lldb/Core/DebuggerEvents.h +++ b/lldb/include/lldb/Core/DebuggerEvents.h @@ -76,19 +76,15 @@ class ProgressEventData : public EventData { class DiagnosticEventData : public EventData { public: - enum class Type { -Info, -Warning, -Error, - }; - DiagnosticEventData(Type type, std::string message, bool debugger_specific) - : m_message(std::move(message)), m_type(type), + DiagnosticEventData(lldb::Severity severity, std::string message, + bool debugger_specific) + : m_message(std::move(message)), m_severity(severity), m_debugger_specific(debugger_specific) {} ~DiagnosticEventData() override = default; const std::string &GetMessage() const { return m_message; } bool IsDebuggerSpecific() const { return m_debugger_specific; } - Type GetType() const { return m_type; } + lldb::Severity GetSeverity() const { return m_severity; } llvm::StringRef GetPrefix() const; @@ -105,7 +101,7 @@ class DiagnosticEventData : public EventData { protected: std::string m_message; - Type m_type; + lldb::Severity m_severity; const bool m_debugger_specific; DiagnosticEventData(const DiagnosticEventData &) = delete; diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h index 06bf1d115f1541..d49b7c99b114fb 100644 --- a/lldb/include/lldb/Expression/DiagnosticManager.h +++ b/lldb/include/lldb/Expression/DiagnosticManager.h @@ -28,12 +28,6 @@ enum DiagnosticOrigin { eDiagnosticOriginLLVM }; -enum DiagnosticSeverity { - eDiagnosticSeverityError, - eDiagnosticSeverityWarning, - eDiagnosticSeverityRemark -}; - const uint32_t LLDB_INVALID_COMPILER_ID = UINT32_MAX; class Diagnostic { @@ -55,7 +49,7 @@ class Diagnostic { } } - Diagnostic(llvm::StringRef message, DiagnosticSeverity severity, + Diagnostic(llvm::StringRef message, lldb::Severity severity, DiagnosticOrigin origin, uint32_t compiler_id) : m_message(message), m_severity(severity), m_origin(origin), m_compiler_id(compiler_id) {} @@ -68,7 +62,7 @@ class Diagnostic { virtual bool HasFixIts() const { return false; } - DiagnosticSeverity GetSeverity() const { return m_severity; } + lldb::Severity GetSeverity() const { return m_severity; } uint32_t GetCompilerID() const { return m_compiler_id; } @@ -83,7 +77,7 @@ class Diagnostic { protected: s
[Lldb-commits] [lldb] [lldb] Always emit diagnostic events to the system log (PR #90913)
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/90913 >From 7ed4008b32d6ec7809a9cea0eb4462d1b90f4e52 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 2 May 2024 15:47:46 -0700 Subject: [PATCH] [lldb] Always emit diagnostic events to the system log Always emit diagnostic events to the system log so that they end up in the sysdiagnose on Darwin. --- lldb/source/Core/Debugger.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 065f70c3880aaa..976420a4344394 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -1512,6 +1512,9 @@ void Debugger::ReportDiagnosticImpl(Severity severity, std::string message, std::optional debugger_id, std::once_flag *once) { auto ReportDiagnosticLambda = [&]() { +// Always log diagnostics to the system log. +Host::SystemLog(severity, message); + // The diagnostic subsystem is optional but we still want to broadcast // events when it's disabled. if (Diagnostics::Enabled()) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] f9d91fb - [lldb] Always emit diagnostic events to the system log (#90913)
Author: Jonas Devlieghere Date: 2024-05-03T09:45:16-07:00 New Revision: f9d91fbe86519f3083a9ae37b1e2038f6b8992a6 URL: https://github.com/llvm/llvm-project/commit/f9d91fbe86519f3083a9ae37b1e2038f6b8992a6 DIFF: https://github.com/llvm/llvm-project/commit/f9d91fbe86519f3083a9ae37b1e2038f6b8992a6.diff LOG: [lldb] Always emit diagnostic events to the system log (#90913) Always emit diagnostic events to the system log so that they end up in the sysdiagnose on Darwin. Added: Modified: lldb/source/Core/Debugger.cpp Removed: diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 065f70c3880aaa..976420a4344394 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -1512,6 +1512,9 @@ void Debugger::ReportDiagnosticImpl(Severity severity, std::string message, std::optional debugger_id, std::once_flag *once) { auto ReportDiagnosticLambda = [&]() { +// Always log diagnostics to the system log. +Host::SystemLog(severity, message); + // The diagnostic subsystem is optional but we still want to broadcast // events when it's disabled. if (Diagnostics::Enabled()) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Always emit diagnostic events to the system log (PR #90913)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/90913 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
@@ -0,0 +1 @@ +dos-style-eol.txt MaskRay wrote: missing text=auto eol=crlf ? https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFCI] Unify DW_TAG -> string conversions (PR #90657)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/90657 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
@@ -0,0 +1 @@ +dos-style-eol.txt ldrumm wrote: Thanks `text eol=crlf`. Updated in 64350b342a09ba69803a541a89b5681a12925ff0 https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/90984 Add a T-style log handler that multiplexes messages to two log handlers. The goal is to use this in combination with the SystemLogHandler to log messages both to the user requested file as well as the system log. The latter is part of a sysdiagnose on Darwin which is commonly attached to bug reports. >From d1adf630a9981f275f24e4d0c2c613a90ff38290 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 3 May 2024 10:11:40 -0700 Subject: [PATCH] [lldb] Add TeeLogHandler to log to 2 handlers Add a T-style log handler that multiplexes messages to two log handlers. The goal is to use this in combination with the SystemLogHandler to log messages both to the user requested file as well as the system log. The latter is part of a sysdiagnose on Darwin which is commonly attached to bug reports. --- lldb/include/lldb/Utility/Log.h| 16 lldb/source/Utility/Log.cpp| 13 + lldb/unittests/Utility/LogTest.cpp | 12 3 files changed, 41 insertions(+) diff --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h index 01876ad732d4b5..bea117c440a46d 100644 --- a/lldb/include/lldb/Utility/Log.h +++ b/lldb/include/lldb/Utility/Log.h @@ -112,6 +112,22 @@ class RotatingLogHandler : public LogHandler { static char ID; }; +class TeeLogHandler : public LogHandler { +public: + TeeLogHandler(std::shared_ptr first_log_handler, +std::shared_ptr second_log_handler); + + void Emit(llvm::StringRef message) override; + + bool isA(const void *ClassID) const override { return ClassID == &ID; } + static bool classof(const LogHandler *obj) { return obj->isA(&ID); } + +private: + std::shared_ptr m_first_log_handler; + std::shared_ptr m_second_log_handler; + static char ID; +}; + class Log final { public: /// The underlying type of all log channel enums. Declare them as: diff --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp index 3a45a0285d3e25..1e237e7ff5e219 100644 --- a/lldb/source/Utility/Log.cpp +++ b/lldb/source/Utility/Log.cpp @@ -39,6 +39,7 @@ char LogHandler::ID; char StreamLogHandler::ID; char CallbackLogHandler::ID; char RotatingLogHandler::ID; +char TeeLogHandler::ID; llvm::ManagedStatic Log::g_channel_map; @@ -438,3 +439,15 @@ void RotatingLogHandler::Dump(llvm::raw_ostream &stream) const { } stream.flush(); } + +TeeLogHandler::TeeLogHandler(std::shared_ptr first_log_handler, + std::shared_ptr second_log_handler) +: m_first_log_handler(first_log_handler), + m_second_log_handler(second_log_handler) {} + +void TeeLogHandler::Emit(llvm::StringRef message) { + if (m_first_log_handler) +m_first_log_handler->Emit(message); + if (m_second_log_handler) +m_second_log_handler->Emit(message); +} diff --git a/lldb/unittests/Utility/LogTest.cpp b/lldb/unittests/Utility/LogTest.cpp index 1dac19486a8f7f..b9b0af4133da92 100644 --- a/lldb/unittests/Utility/LogTest.cpp +++ b/lldb/unittests/Utility/LogTest.cpp @@ -200,6 +200,18 @@ TEST(LogHandlerTest, RotatingLogHandler) { EXPECT_EQ(GetDumpAsString(handler), "bazquxquux"); } +TEST(LogHandlerTest, TeeLogHandler) { + auto handler1 = std::make_shared(2); + auto handler2 = std::make_shared(2); + TeeLogHandler handler(handler1, handler2); + + handler.Emit("foo"); + handler.Emit("bar"); + + EXPECT_EQ(GetDumpAsString(*handler1), "foobar"); + EXPECT_EQ(GetDumpAsString(*handler2), "foobar"); +} + TEST_F(LogChannelTest, Enable) { EXPECT_EQ(nullptr, GetLog(TestChannel::FOO)); std::string message; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
https://github.com/MaskRay approved this pull request. https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [AArch64] move extension information into tablgen (PR #90987)
https://github.com/tmatheson-arm created https://github.com/llvm/llvm-project/pull/90987 Generate target features and FMVExtensions from tablegen. Use MArchName/ArchKindEnumSpelling to avoid renamings. Cases where there is simply a case difference are handled by consistently uppercasing the AEK_ name in the emitted code. Remove some Extensions which were not needed. These had AEK entries but were never actually used for anything. They are not present in Extensions[] data. >From 4b8b776348438847c2eb238dac973e93fe93294e Mon Sep 17 00:00:00 2001 From: Tomas Matheson Date: Mon, 29 Apr 2024 19:57:17 +0100 Subject: [PATCH 1/2] [AArch64] move extension information into tablgen Generate target features and FMVExtensions from tablegen. Use MArchName/ArchKindEnumSpelling to avoid renamings. Cases where there is simply a case difference are handled by consistently uppercasing the AEK_ name in the emitted code. Remove some Extensions which were not needed. These had AEK entries but were never actually used for anything. They are not present in Extensions[] data. --- .../command-disassemble-aarch64-extensions.s | 2 +- .../llvm/TargetParser/AArch64TargetParser.h | 139 +-- llvm/lib/Target/AArch64/AArch64Features.td| 216 ++ .../TargetParser/TargetParserTest.cpp | 8 +- llvm/utils/TableGen/ARMTargetDefEmitter.cpp | 60 - 5 files changed, 236 insertions(+), 189 deletions(-) diff --git a/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s index e154f544e7cc6e..685d0a84ec2896 100644 --- a/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s +++ b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s @@ -59,7 +59,7 @@ fn: bdep z0.b, z1.b, z31.b// AEK_SVE2BITPERM rax1 z0.d, z0.d, z0.d // AEK_SVE2SHA3 sm4e z0.s, z0.s, z0.s // AEK_SVE2SM4 - addqv v0.8h, p0, z0.h // AEK_SVE2p1 / AEK_SME2p1 + addqv v0.8h, p0, z0.h // AEK_SVE2P1 / AEK_SME2P1 rcwswp x0, x1, [x2] // AEK_THE tcommit // AEK_TME lbl: diff --git a/llvm/include/llvm/TargetParser/AArch64TargetParser.h b/llvm/include/llvm/TargetParser/AArch64TargetParser.h index 04fbaf07adfbcb..1124420daf8d80 100644 --- a/llvm/include/llvm/TargetParser/AArch64TargetParser.h +++ b/llvm/include/llvm/TargetParser/AArch64TargetParser.h @@ -104,29 +104,9 @@ static_assert(FEAT_MAX < 62, "Number of features in CPUFeatures are limited to 62 entries"); // Each ArchExtKind correponds directly to a possible -target-feature. -enum ArchExtKind : unsigned { - AEK_NONE = 1, -#define ARM_EXTENSION(NAME, ENUM) ENUM, +#define EMIT_ARCHEXTKIND_ENUM #include "llvm/TargetParser/AArch64TargetParserDef.inc" - AEK_NUM_EXTENSIONS, - - // FIXME temporary fixes for inconsistent naming. - AEK_F32MM = AEK_MATMULFP32, - AEK_F64MM = AEK_MATMULFP64, - AEK_FCMA = AEK_COMPLXNUM, - AEK_FP = AEK_FPARMV8, - AEK_FP16 = AEK_FULLFP16, - AEK_I8MM = AEK_MATMULINT8, - AEK_JSCVT = AEK_JS, - AEK_PROFILE = AEK_SPE, - AEK_RASv2 = AEK_RASV2, - AEK_RAND = AEK_RANDGEN, - AEK_SIMD = AEK_NEON, - AEK_SME2p1 = AEK_SME2P1, - AEK_SVE2p1 = AEK_SVE2P1, - AEK_SME_LUTv2 = AEK_SME_LUTV2, -}; using ExtensionBitset = Bitset; // Represents an extension that can be enabled with -march=+. @@ -148,111 +128,8 @@ struct ExtensionInfo { 1000; // Maximum priority for FMV feature }; -// NOTE: If adding a new extension here, consider adding it to ExtensionMap -// in AArch64AsmParser too, if supported as an extension name by binutils. -// clang-format off -inline constexpr ExtensionInfo Extensions[] = { -{"aes", AArch64::AEK_AES, "+aes", "-aes", FEAT_AES, "+fp-armv8,+neon", 150}, -{"b16b16", AArch64::AEK_B16B16, "+b16b16", "-b16b16", FEAT_INIT, "", 0}, -{"bf16", AArch64::AEK_BF16, "+bf16", "-bf16", FEAT_BF16, "+bf16", 280}, -{"brbe", AArch64::AEK_BRBE, "+brbe", "-brbe", FEAT_INIT, "", 0}, -{"bti", AArch64::AEK_NONE, {}, {}, FEAT_BTI, "+bti", 510}, -{"crc", AArch64::AEK_CRC, "+crc", "-crc", FEAT_CRC, "+crc", 110}, -{"crypto", AArch64::AEK_CRYPTO, "+crypto", "-crypto", FEAT_INIT, "+aes,+sha2", 0}, -{"cssc", AArch64::AEK_CSSC, "+cssc", "-cssc", FEAT_INIT, "", 0}, -{"d128", AArch64::AEK_D128, "+d128", "-d128", FEAT_INIT, "", 0}, -{"dgh", AArch64::AEK_NONE, {}, {}, FEAT_DGH, "", 260}, -{"dit", AArch64::AEK_NONE, {}, {}, FEAT_DIT, "+dit", 180}, -{"dotprod", AArch64::AEK_DOTPROD, "+dotprod", "-dotprod", FEAT_DOTPROD, "+dotprod,+fp-armv8,+neon", 104}, -{"dpb", AArch64::AEK_NONE, {}, {}, FEAT_DPB, "+ccpp", 190}, -{"dpb2", AArch64::AEK_NONE, {}, {}, FEAT_DPB2, "+ccpp,+ccdp", 200}, -{"ebf16", AArch64::AEK_NONE, {}, {}, FEAT_EBF16, "+bf16", 290}, -{"f32mm", AArch64::AEK_F32MM, "+f32mm", "-f32mm", FEAT_SVE_F32MM, "+sve,+f32mm,+fullfp16,+fp-armv8
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
@@ -1,3 +1,10 @@ +# Checkout as native, commit as LF except in specific circumstances +* text=auto +*.bat text eol=crlf +*.rc text eol=crlf +*.sln text eol=crlf MaskRay wrote: Do we need `.sln`? There is only one file in `clang/tools/clang-format-vs`. There are a few other text files in this directory, so perhaps a gitattributes in that directory is better? https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)
@@ -87,8 +105,15 @@ SymbolVendorELF::CreateInstance(const lldb::ModuleSP &module_sp, FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths(); FileSpec dsym_fspec = PluginManager::LocateExecutableSymbolFile(module_spec, search_paths); - if (!dsym_fspec) -return nullptr; + if (!dsym_fspec || IsDwpSymbolFile(module_sp, dsym_fspec)) { +// If we have a stripped binary or if we got a DWP file, we should prefer +// symbols in the executable acquired through a plugin. +ModuleSpec unstripped_spec = +PluginManager::LocateExecutableObjectFile(module_spec); +if (!unstripped_spec) + return nullptr; +dsym_fspec = unstripped_spec.GetFileSpec(); + } kevinfrei wrote: @DavidSpickett if there's anything I can do to help, please ping me. Feel free to use my github handle at hotmail to ping me, as I'm going to be on a "doing nothing in nicer weather" vacation next week. https://github.com/llvm/llvm-project/pull/90622 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
ZequanWu wrote: > > > Is any of it testable? > > > > > > Good question. Though this is mostly meant to be "NFC" (with very large > > quotes), I can imagine us doing something like forcing the parsing of a > > specific type (`type lookup ` ?), and then checking that the > > module ast (`image dump ast`) does _not_ contain specific types -- as > > that's basically what we're trying to achieve. > > Yea that could work. But if it's going to be very painful or fragile to test > then don't let that hold back the PR In terms of testing, since this only delays definition DIE searching not type completion, we need to construct a test so that lldb finds the declaration DIE first without trigger a type completion on it and somehow test the incomplete type. The first part is tricky. I'm not sure how to achieve it. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [AArch64] move extension information into tablgen (PR #90987)
https://github.com/tmatheson-arm updated https://github.com/llvm/llvm-project/pull/90987 >From 4b8b776348438847c2eb238dac973e93fe93294e Mon Sep 17 00:00:00 2001 From: Tomas Matheson Date: Mon, 29 Apr 2024 19:57:17 +0100 Subject: [PATCH 1/3] [AArch64] move extension information into tablgen Generate target features and FMVExtensions from tablegen. Use MArchName/ArchKindEnumSpelling to avoid renamings. Cases where there is simply a case difference are handled by consistently uppercasing the AEK_ name in the emitted code. Remove some Extensions which were not needed. These had AEK entries but were never actually used for anything. They are not present in Extensions[] data. --- .../command-disassemble-aarch64-extensions.s | 2 +- .../llvm/TargetParser/AArch64TargetParser.h | 139 +-- llvm/lib/Target/AArch64/AArch64Features.td| 216 ++ .../TargetParser/TargetParserTest.cpp | 8 +- llvm/utils/TableGen/ARMTargetDefEmitter.cpp | 60 - 5 files changed, 236 insertions(+), 189 deletions(-) diff --git a/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s index e154f544e7cc6e..685d0a84ec2896 100644 --- a/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s +++ b/lldb/test/Shell/Commands/command-disassemble-aarch64-extensions.s @@ -59,7 +59,7 @@ fn: bdep z0.b, z1.b, z31.b// AEK_SVE2BITPERM rax1 z0.d, z0.d, z0.d // AEK_SVE2SHA3 sm4e z0.s, z0.s, z0.s // AEK_SVE2SM4 - addqv v0.8h, p0, z0.h // AEK_SVE2p1 / AEK_SME2p1 + addqv v0.8h, p0, z0.h // AEK_SVE2P1 / AEK_SME2P1 rcwswp x0, x1, [x2] // AEK_THE tcommit // AEK_TME lbl: diff --git a/llvm/include/llvm/TargetParser/AArch64TargetParser.h b/llvm/include/llvm/TargetParser/AArch64TargetParser.h index 04fbaf07adfbcb..1124420daf8d80 100644 --- a/llvm/include/llvm/TargetParser/AArch64TargetParser.h +++ b/llvm/include/llvm/TargetParser/AArch64TargetParser.h @@ -104,29 +104,9 @@ static_assert(FEAT_MAX < 62, "Number of features in CPUFeatures are limited to 62 entries"); // Each ArchExtKind correponds directly to a possible -target-feature. -enum ArchExtKind : unsigned { - AEK_NONE = 1, -#define ARM_EXTENSION(NAME, ENUM) ENUM, +#define EMIT_ARCHEXTKIND_ENUM #include "llvm/TargetParser/AArch64TargetParserDef.inc" - AEK_NUM_EXTENSIONS, - - // FIXME temporary fixes for inconsistent naming. - AEK_F32MM = AEK_MATMULFP32, - AEK_F64MM = AEK_MATMULFP64, - AEK_FCMA = AEK_COMPLXNUM, - AEK_FP = AEK_FPARMV8, - AEK_FP16 = AEK_FULLFP16, - AEK_I8MM = AEK_MATMULINT8, - AEK_JSCVT = AEK_JS, - AEK_PROFILE = AEK_SPE, - AEK_RASv2 = AEK_RASV2, - AEK_RAND = AEK_RANDGEN, - AEK_SIMD = AEK_NEON, - AEK_SME2p1 = AEK_SME2P1, - AEK_SVE2p1 = AEK_SVE2P1, - AEK_SME_LUTv2 = AEK_SME_LUTV2, -}; using ExtensionBitset = Bitset; // Represents an extension that can be enabled with -march=+. @@ -148,111 +128,8 @@ struct ExtensionInfo { 1000; // Maximum priority for FMV feature }; -// NOTE: If adding a new extension here, consider adding it to ExtensionMap -// in AArch64AsmParser too, if supported as an extension name by binutils. -// clang-format off -inline constexpr ExtensionInfo Extensions[] = { -{"aes", AArch64::AEK_AES, "+aes", "-aes", FEAT_AES, "+fp-armv8,+neon", 150}, -{"b16b16", AArch64::AEK_B16B16, "+b16b16", "-b16b16", FEAT_INIT, "", 0}, -{"bf16", AArch64::AEK_BF16, "+bf16", "-bf16", FEAT_BF16, "+bf16", 280}, -{"brbe", AArch64::AEK_BRBE, "+brbe", "-brbe", FEAT_INIT, "", 0}, -{"bti", AArch64::AEK_NONE, {}, {}, FEAT_BTI, "+bti", 510}, -{"crc", AArch64::AEK_CRC, "+crc", "-crc", FEAT_CRC, "+crc", 110}, -{"crypto", AArch64::AEK_CRYPTO, "+crypto", "-crypto", FEAT_INIT, "+aes,+sha2", 0}, -{"cssc", AArch64::AEK_CSSC, "+cssc", "-cssc", FEAT_INIT, "", 0}, -{"d128", AArch64::AEK_D128, "+d128", "-d128", FEAT_INIT, "", 0}, -{"dgh", AArch64::AEK_NONE, {}, {}, FEAT_DGH, "", 260}, -{"dit", AArch64::AEK_NONE, {}, {}, FEAT_DIT, "+dit", 180}, -{"dotprod", AArch64::AEK_DOTPROD, "+dotprod", "-dotprod", FEAT_DOTPROD, "+dotprod,+fp-armv8,+neon", 104}, -{"dpb", AArch64::AEK_NONE, {}, {}, FEAT_DPB, "+ccpp", 190}, -{"dpb2", AArch64::AEK_NONE, {}, {}, FEAT_DPB2, "+ccpp,+ccdp", 200}, -{"ebf16", AArch64::AEK_NONE, {}, {}, FEAT_EBF16, "+bf16", 290}, -{"f32mm", AArch64::AEK_F32MM, "+f32mm", "-f32mm", FEAT_SVE_F32MM, "+sve,+f32mm,+fullfp16,+fp-armv8,+neon", 350}, -{"f64mm", AArch64::AEK_F64MM, "+f64mm", "-f64mm", FEAT_SVE_F64MM, "+sve,+f64mm,+fullfp16,+fp-armv8,+neon", 360}, -{"fcma", AArch64::AEK_FCMA, "+complxnum", "-complxnum", FEAT_FCMA, "+fp-armv8,+neon,+complxnum", 220}, -{"flagm", AArch64::AEK_FLAGM, "+flagm", "-flagm", FEAT_FLAGM, "+flagm", 20}, -{"flagm2", AArch64::AEK_NONE, {}, {}, FEAT_FLAGM2, "+flagm,+
[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)
@@ -112,6 +112,22 @@ class RotatingLogHandler : public LogHandler { static char ID; }; +class TeeLogHandler : public LogHandler { adrian-prantl wrote: Doxygen comment? https://github.com/llvm/llvm-project/pull/90984 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/90703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)
https://github.com/bulbazord commented: Looking better. Thanks for sticking with us through the review! :) https://github.com/llvm/llvm-project/pull/90703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)
@@ -289,3 +290,19 @@ void StructuredData::Null::GetDescription(lldb_private::Stream &s) const { void StructuredData::Generic::GetDescription(lldb_private::Stream &s) const { s.Printf("%p", m_object); } + +StructuredData::ArraySP StructuredData::Array::SplitString(llvm::StringRef s, + char separator, + int maxSplit, + bool keepEmpty) { + // Split the string into a small vector. + llvm::SmallVector small_vec; + s.split(small_vec, separator, maxSplit, keepEmpty); + + // Copy the substrings from the small vector into the output array. + auto array_sp = std::make_shared(); + for (auto substring : small_vec) { +array_sp->AddStringItem(std::move(substring)); bulbazord wrote: nit: no need for the move, StringRefs are pretty cheap to copy https://github.com/llvm/llvm-project/pull/90703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)
@@ -85,3 +86,84 @@ def test_command_output(self): self.assertEqual(res.GetOutput(), "") self.assertIsNotNone(res.GetError()) self.assertEqual(res.GetError(), "") + +def test_structured_transcript(self): +"""Test structured transcript generation and retrieval.""" +# Get command interpreter and create a target +self.build() +exe = self.getBuildArtifact("a.out") + +target = self.dbg.CreateTarget(exe) +self.assertTrue(target, VALID_TARGET) + +ci = self.dbg.GetCommandInterpreter() +self.assertTrue(ci, VALID_COMMAND_INTERPRETER) + +# Send a few commands through the command interpreter +res = lldb.SBCommandReturnObject() +ci.HandleCommand("version", res) +ci.HandleCommand("an-unknown-command", res) +ci.HandleCommand("breakpoint set -f main.c -l %d" % self.line, res) +ci.HandleCommand("r", res) +ci.HandleCommand("p a", res) +total_number_of_commands = 5 + +# Retrieve the transcript and convert it into a Python object +transcript = ci.GetTranscript() +self.assertTrue(transcript.IsValid()) + +stream = lldb.SBStream() +self.assertTrue(stream) + +error = transcript.GetAsJSON(stream) +self.assertSuccess(error) + +transcript = json.loads(stream.GetData()) + +# The transcript will contain a bunch of commands that are run +# automatically. We only want to validate for the ones that are +# listed above, hence trimming to the last parts. +transcript = transcript[-total_number_of_commands:] bulbazord wrote: Instead of trimming, why not verify that the transcript contains exactly the number we care about? Or is there a reason there would be more? Perhaps from some setup code? https://github.com/llvm/llvm-project/pull/90703 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)
https://github.com/adrian-prantl edited https://github.com/llvm/llvm-project/pull/90984 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)
@@ -438,3 +439,15 @@ void RotatingLogHandler::Dump(llvm::raw_ostream &stream) const { } stream.flush(); } + +TeeLogHandler::TeeLogHandler(std::shared_ptr first_log_handler, + std::shared_ptr second_log_handler) +: m_first_log_handler(first_log_handler), + m_second_log_handler(second_log_handler) {} + +void TeeLogHandler::Emit(llvm::StringRef message) { + if (m_first_log_handler) adrian-prantl wrote: Is the idea that one will ne optional in the typical use-case? https://github.com/llvm/llvm-project/pull/90984 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)
https://github.com/adrian-prantl approved this pull request. https://github.com/llvm/llvm-project/pull/90984 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)
@@ -438,3 +439,15 @@ void RotatingLogHandler::Dump(llvm::raw_ostream &stream) const { } stream.flush(); } + +TeeLogHandler::TeeLogHandler(std::shared_ptr first_log_handler, + std::shared_ptr second_log_handler) +: m_first_log_handler(first_log_handler), + m_second_log_handler(second_log_handler) {} + +void TeeLogHandler::Emit(llvm::StringRef message) { + if (m_first_log_handler) JDevlieghere wrote: No, it's just being resilient agains the shared_ptrs being null. The alternative would be an assert in the ctor. Let me know if you prefer that? It would be slightly more efficient. https://github.com/llvm/llvm-project/pull/90984 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1631,27 +1631,34 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) { return true; } - DWARFDIE dwarf_die = GetDIE(die_it->getSecond()); - if (dwarf_die) { -// Once we start resolving this type, remove it from the forward -// declaration map in case anyone child members or other types require this -// type to get resolved. The type will get resolved when all of the calls -// to SymbolFileDWARF::ResolveClangOpaqueTypeDefinition are done. -GetForwardDeclCompilerTypeToDIE().erase(die_it); - -Type *type = GetDIEToType().lookup(dwarf_die.GetDIE()); + // Once we start resolving this type, remove it from the forward + // declaration map in case anyone child members or other types require this clayborg wrote: s/anyone/anyone's/ to fix bad previous comment https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -154,6 +154,27 @@ static bool TagIsRecordType(dw_tag_t tag) { } } +static bool +IsForwardDeclaration(const lldb_private::plugin::dwarf::DWARFDIE &die, + const ParsedDWARFTypeAttributes &attrs, + LanguageType cu_language) { + if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name && clayborg wrote: Do we want to check `attrs.is_forward_declaration` before doing any of this and quick return? Then only do this more expensive `if` check if `attrs.is_forward_declaration` is false? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/clayborg commented: Looks pretty good to me as long as the test suite is happy. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] e2b3e4e - [lldb][NFCI] Unify DW_TAG -> string conversions (#90657)
Author: Alex Langford Date: 2024-05-03T11:05:11-07:00 New Revision: e2b3e4ea9f2d0cb34d197439cfbc5090cdacb124 URL: https://github.com/llvm/llvm-project/commit/e2b3e4ea9f2d0cb34d197439cfbc5090cdacb124 DIFF: https://github.com/llvm/llvm-project/commit/e2b3e4ea9f2d0cb34d197439cfbc5090cdacb124.diff LOG: [lldb][NFCI] Unify DW_TAG -> string conversions (#90657) The high level goal is to have 1 way of converting a DW_TAG value into a human-readable string. There are 3 ways this change accomplishes that: 1.) Changing DW_TAG_value_to_name to not create custom error strings. The way it was doing this is error-prone: Specifically, it was using a function-local static char buffer and handing out a pointer to it. Initialization of this is thread-safe, but mutating it is definitely not. Multiple threads that want to call this function could step on each others toes. The implementation in this patch sidesteps the issue by just returning a StringRef with no mention of the tag value in it. 2.) Changing all uses of DW_TAG_value_to_name to log the value of the tag since the function doesn't create a string with the value in it anymore. 3.) Removing `DWARFBaseDIE::GetTagAsCString()`. Callers should call DW_TAG_value_to_name on the tag directly. Added: Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Removed: diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index bea11e0e3840af..f8101aba5c6277 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -452,9 +452,9 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc, log, "DWARFASTParserClang::ParseTypeFromDWARF " "(die = {0:x16}, decl_ctx = {1:p} (die " -"{2:x16})) {3} name = '{4}')", +"{2:x16})) {3} ({4}) name = '{5}')", die.GetOffset(), static_cast(context), context_die.GetOffset(), -die.GetTagAsCString(), die.GetName()); +DW_TAG_value_to_name(die.Tag()), die.Tag(), die.GetName()); } Type *type_ptr = dwarf->GetDIEToType().lookup(die.GetDIE()); @@ -765,9 +765,10 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc, if (log) dwarf->GetObjectFile()->GetModule()->LogMessage( log, -"SymbolFileDWARF::ParseType (die = {0:x16}) {1} '{2}' " +"SymbolFileDWARF::ParseType (die = {0:x16}) {1} ({2}) '{3}' " "is Objective-C 'id' built-in type.", -die.GetOffset(), die.GetTagAsCString(), die.GetName()); +die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(), +die.GetName()); clang_type = m_ast.GetBasicType(eBasicTypeObjCID); encoding_data_type = Type::eEncodingIsUID; attrs.type.Clear(); @@ -776,9 +777,10 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc, if (log) dwarf->GetObjectFile()->GetModule()->LogMessage( log, -"SymbolFileDWARF::ParseType (die = {0:x16}) {1} '{2}' " +"SymbolFileDWARF::ParseType (die = {0:x16}) {1} ({2}) '{3}' " "is Objective-C 'Class' built-in type.", -die.GetOffset(), die.GetTagAsCString(), die.GetName()); +die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(), +die.GetName()); clang_type = m_ast.GetBasicType(eBasicTypeObjCClass); encoding_data_type = Type::eEncodingIsUID; attrs.type.Clear(); @@ -787,9 +789,10 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc, if (log) dwarf->GetObjectFile()->GetModule()->LogMessage( log, -"SymbolFileDWARF::ParseType (die = {0:x16}) {1} '{2}' " +"SymbolFileDWARF::ParseType (die = {0:x16}) {1} ({2}) '{3}' " "is Objective-C 'selector' built-in type.", -die.GetOffset(), die.GetTagAsCString(), die.GetName()); +die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(), +die.GetName()); clang_type = m_ast.GetBasicType(eBasicTypeObjCSel); encoding_data_type = Type::eEncodingIsUID; attrs.type.Clear(); @@ -808,10 +811,10 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc, if (log) dwarf->GetObjectFile()->GetModule()->LogMessage(
[Lldb-commits] [lldb] [lldb][NFCI] Unify DW_TAG -> string conversions (PR #90657)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/90657 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/90984 >From d1adf630a9981f275f24e4d0c2c613a90ff38290 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 3 May 2024 10:11:40 -0700 Subject: [PATCH 1/2] [lldb] Add TeeLogHandler to log to 2 handlers Add a T-style log handler that multiplexes messages to two log handlers. The goal is to use this in combination with the SystemLogHandler to log messages both to the user requested file as well as the system log. The latter is part of a sysdiagnose on Darwin which is commonly attached to bug reports. --- lldb/include/lldb/Utility/Log.h| 16 lldb/source/Utility/Log.cpp| 13 + lldb/unittests/Utility/LogTest.cpp | 12 3 files changed, 41 insertions(+) diff --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h index 01876ad732d4b5..bea117c440a46d 100644 --- a/lldb/include/lldb/Utility/Log.h +++ b/lldb/include/lldb/Utility/Log.h @@ -112,6 +112,22 @@ class RotatingLogHandler : public LogHandler { static char ID; }; +class TeeLogHandler : public LogHandler { +public: + TeeLogHandler(std::shared_ptr first_log_handler, +std::shared_ptr second_log_handler); + + void Emit(llvm::StringRef message) override; + + bool isA(const void *ClassID) const override { return ClassID == &ID; } + static bool classof(const LogHandler *obj) { return obj->isA(&ID); } + +private: + std::shared_ptr m_first_log_handler; + std::shared_ptr m_second_log_handler; + static char ID; +}; + class Log final { public: /// The underlying type of all log channel enums. Declare them as: diff --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp index 3a45a0285d3e25..1e237e7ff5e219 100644 --- a/lldb/source/Utility/Log.cpp +++ b/lldb/source/Utility/Log.cpp @@ -39,6 +39,7 @@ char LogHandler::ID; char StreamLogHandler::ID; char CallbackLogHandler::ID; char RotatingLogHandler::ID; +char TeeLogHandler::ID; llvm::ManagedStatic Log::g_channel_map; @@ -438,3 +439,15 @@ void RotatingLogHandler::Dump(llvm::raw_ostream &stream) const { } stream.flush(); } + +TeeLogHandler::TeeLogHandler(std::shared_ptr first_log_handler, + std::shared_ptr second_log_handler) +: m_first_log_handler(first_log_handler), + m_second_log_handler(second_log_handler) {} + +void TeeLogHandler::Emit(llvm::StringRef message) { + if (m_first_log_handler) +m_first_log_handler->Emit(message); + if (m_second_log_handler) +m_second_log_handler->Emit(message); +} diff --git a/lldb/unittests/Utility/LogTest.cpp b/lldb/unittests/Utility/LogTest.cpp index 1dac19486a8f7f..b9b0af4133da92 100644 --- a/lldb/unittests/Utility/LogTest.cpp +++ b/lldb/unittests/Utility/LogTest.cpp @@ -200,6 +200,18 @@ TEST(LogHandlerTest, RotatingLogHandler) { EXPECT_EQ(GetDumpAsString(handler), "bazquxquux"); } +TEST(LogHandlerTest, TeeLogHandler) { + auto handler1 = std::make_shared(2); + auto handler2 = std::make_shared(2); + TeeLogHandler handler(handler1, handler2); + + handler.Emit("foo"); + handler.Emit("bar"); + + EXPECT_EQ(GetDumpAsString(*handler1), "foobar"); + EXPECT_EQ(GetDumpAsString(*handler2), "foobar"); +} + TEST_F(LogChannelTest, Enable) { EXPECT_EQ(nullptr, GetLog(TestChannel::FOO)); std::string message; >From 523ee979643494060fde757a64e26190e9241801 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 3 May 2024 11:05:03 -0700 Subject: [PATCH 2/2] Address Adrian's feedback --- lldb/include/lldb/Utility/Log.h | 1 + lldb/source/Utility/Log.cpp | 11 ++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h index bea117c440a46d..27707c17f9b824 100644 --- a/lldb/include/lldb/Utility/Log.h +++ b/lldb/include/lldb/Utility/Log.h @@ -112,6 +112,7 @@ class RotatingLogHandler : public LogHandler { static char ID; }; +/// A T-style log handler that multiplexes messages to two log handlers. class TeeLogHandler : public LogHandler { public: TeeLogHandler(std::shared_ptr first_log_handler, diff --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp index 1e237e7ff5e219..6713a5bd758204 100644 --- a/lldb/source/Utility/Log.cpp +++ b/lldb/source/Utility/Log.cpp @@ -443,11 +443,12 @@ void RotatingLogHandler::Dump(llvm::raw_ostream &stream) const { TeeLogHandler::TeeLogHandler(std::shared_ptr first_log_handler, std::shared_ptr second_log_handler) : m_first_log_handler(first_log_handler), - m_second_log_handler(second_log_handler) {} + m_second_log_handler(second_log_handler) { + assert(m_first_log_handler && "first log handler must be valid"); + assert(m_second_log_handler && "second log handler must be valid"); +} void TeeLogHandler::Emit(llvm::StringRef message) { - if (m_first_log_handler) -m_first_log_handler->Emit(message); - if (m_second_log_handler
[Lldb-commits] [lldb] a8fbe50 - [lldb] Add TeeLogHandler to log to 2 handlers (#90984)
Author: Jonas Devlieghere Date: 2024-05-03T11:08:50-07:00 New Revision: a8fbe500fe2ecdbd3c09ed06788092937819411f URL: https://github.com/llvm/llvm-project/commit/a8fbe500fe2ecdbd3c09ed06788092937819411f DIFF: https://github.com/llvm/llvm-project/commit/a8fbe500fe2ecdbd3c09ed06788092937819411f.diff LOG: [lldb] Add TeeLogHandler to log to 2 handlers (#90984) Add a T-style log handler that multiplexes messages to two log handlers. The goal is to use this in combination with the SystemLogHandler to log messages both to the user requested file as well as the system log. The latter is part of a sysdiagnose on Darwin which is commonly attached to bug reports. Added: Modified: lldb/include/lldb/Utility/Log.h lldb/source/Utility/Log.cpp lldb/unittests/Utility/LogTest.cpp Removed: diff --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h index 01876ad732d4b5..27707c17f9b824 100644 --- a/lldb/include/lldb/Utility/Log.h +++ b/lldb/include/lldb/Utility/Log.h @@ -112,6 +112,23 @@ class RotatingLogHandler : public LogHandler { static char ID; }; +/// A T-style log handler that multiplexes messages to two log handlers. +class TeeLogHandler : public LogHandler { +public: + TeeLogHandler(std::shared_ptr first_log_handler, +std::shared_ptr second_log_handler); + + void Emit(llvm::StringRef message) override; + + bool isA(const void *ClassID) const override { return ClassID == &ID; } + static bool classof(const LogHandler *obj) { return obj->isA(&ID); } + +private: + std::shared_ptr m_first_log_handler; + std::shared_ptr m_second_log_handler; + static char ID; +}; + class Log final { public: /// The underlying type of all log channel enums. Declare them as: diff --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp index 3a45a0285d3e25..6713a5bd758204 100644 --- a/lldb/source/Utility/Log.cpp +++ b/lldb/source/Utility/Log.cpp @@ -39,6 +39,7 @@ char LogHandler::ID; char StreamLogHandler::ID; char CallbackLogHandler::ID; char RotatingLogHandler::ID; +char TeeLogHandler::ID; llvm::ManagedStatic Log::g_channel_map; @@ -438,3 +439,16 @@ void RotatingLogHandler::Dump(llvm::raw_ostream &stream) const { } stream.flush(); } + +TeeLogHandler::TeeLogHandler(std::shared_ptr first_log_handler, + std::shared_ptr second_log_handler) +: m_first_log_handler(first_log_handler), + m_second_log_handler(second_log_handler) { + assert(m_first_log_handler && "first log handler must be valid"); + assert(m_second_log_handler && "second log handler must be valid"); +} + +void TeeLogHandler::Emit(llvm::StringRef message) { + m_first_log_handler->Emit(message); + m_second_log_handler->Emit(message); +} diff --git a/lldb/unittests/Utility/LogTest.cpp b/lldb/unittests/Utility/LogTest.cpp index 1dac19486a8f7f..b9b0af4133da92 100644 --- a/lldb/unittests/Utility/LogTest.cpp +++ b/lldb/unittests/Utility/LogTest.cpp @@ -200,6 +200,18 @@ TEST(LogHandlerTest, RotatingLogHandler) { EXPECT_EQ(GetDumpAsString(handler), "bazquxquux"); } +TEST(LogHandlerTest, TeeLogHandler) { + auto handler1 = std::make_shared(2); + auto handler2 = std::make_shared(2); + TeeLogHandler handler(handler1, handler2); + + handler.Emit("foo"); + handler.Emit("bar"); + + EXPECT_EQ(GetDumpAsString(*handler1), "foobar"); + EXPECT_EQ(GetDumpAsString(*handler2), "foobar"); +} + TEST_F(LogChannelTest, Enable) { EXPECT_EQ(nullptr, GetLog(TestChannel::FOO)); std::string message; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add TeeLogHandler to log to 2 handlers (PR #90984)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/90984 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits