[Lldb-commits] [clang] [clang-tools-extra] [mlir] [lldb] [llvm] [flang] [compiler-rt] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
https://github.com/MaskRay edited https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [clang-tools-extra] [flang] [mlir] [clang] [llvm] [compiler-rt] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
MaskRay wrote: The patch does not apply cleanly at HEAD. The fix-up commit `resolve conflict` contains the rebased part and a lot of unrelated changes. I think in this case, it's cleaner to squash all the commits and force push to `binary-correlate`. (Force push is fine with me: https://discourse.llvm.org/t/force-push-and-rebase/73766/10) https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [clang-tools-extra] [flang] [mlir] [clang] [llvm] [compiler-rt] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
@@ -0,0 +1,46 @@ +// REQUIRES: linux || windows MaskRay wrote: Add `// REQUIRES: lld-available` https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [compiler-rt] [flang] [llvm] [clang] [clang-tools-extra] [mlir] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
@@ -0,0 +1,46 @@ +// REQUIRES: linux || windows +// Default +// RUN: %clang -o %t.normal -fprofile-instr-generate -fcoverage-mapping -fuse-ld=lld %S/Inputs/instrprof-debug-info-correlate-main.cpp %S/Inputs/instrprof-debug-info-correlate-foo.cpp +// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t.normal +// RUN: llvm-profdata merge -o %t.normal.profdata %t.profraw +// RUN: llvm-cov report --instr-profile=%t.normal.profdata %t.normal > %t.normal.report +// RUN: llvm-cov show --instr-profile=%t.normal.profdata %t.normal > %t.normal.show + +// With -profile-correlate=binary flag +// RUN: %clang -o %t-1.exe -fprofile-instr-generate -fcoverage-mapping -mllvm -profile-correlate=binary -fuse-ld=lld %S/Inputs/instrprof-debug-info-correlate-main.cpp %S/Inputs/instrprof-debug-info-correlate-foo.cpp MaskRay wrote: You can omit `.exe`. Otherwise, I suspect the file will be named `%t-1.exe.exe` on Windows. https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [flang] [clang] [mlir] [lldb] [clang-tools-extra] [llvm] [compiler-rt] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
@@ -46,14 +73,38 @@ const char *InstrProfCorrelator::NumCountersAttributeName =
"Num Counters";
llvm::Expected>
InstrProfCorrelator::Context::get(std::unique_ptr Buffer,
- const object::ObjectFile ) {
+ const object::ObjectFile ,
+ ProfCorrelatorKind FileKind) {
+ auto C = std::make_unique();
auto CountersSection = getInstrProfSection(Obj, IPSK_cnts);
if (auto Err = CountersSection.takeError())
return std::move(Err);
- auto C = std::make_unique();
+ if (FileKind == InstrProfCorrelator::BINARY) {
+auto DataSection = getInstrProfSection(Obj, IPSK_data);
+if (auto Err = DataSection.takeError())
+ return std::move(Err);
+auto DataOrErr = DataSection->getContents();
+if (!DataOrErr)
+ return DataOrErr.takeError();
+auto NameSection = getInstrProfSection(Obj, IPSK_name);
+if (auto Err = NameSection.takeError())
+ return std::move(Err);
+auto NameOrErr = NameSection->getContents();
+if (!NameOrErr)
+ return NameOrErr.takeError();
+C->DataStart = DataOrErr->data();
+C->DataEnd = DataOrErr->data() + DataOrErr->size();
+C->NameStart = NameOrErr->data();
+C->NameSize = NameOrErr->size();
+ }
C->Buffer = std::move(Buffer);
C->CountersSectionStart = CountersSection->getAddress();
C->CountersSectionEnd = C->CountersSectionStart + CountersSection->getSize();
+ // In COFF object file, there's a null byte at the beginning of the counter
+ // section which doesn't exist in raw profile.
+ if (Obj.getTripleObjectFormat() == Triple::COFF)
+C->CountersSectionStart++;
MaskRay wrote:
pre-increment https://llvm.org/docs/CodingStandards.html#prefer-preincrement
https://github.com/llvm/llvm-project/pull/69493
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang-tools-extra] [clang] [lldb] [compiler-rt] [flang] [mlir] [llvm] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
https://github.com/MaskRay edited https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [compiler-rt] [llvm] [flang] [clang-tools-extra] [mlir] [lldb] [clang] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
https://github.com/MaskRay commented: The comment `The data and names sections are omitted in lightweight mode.` in compiler-rt should be updated since binary correlation is different from the lightweight mode https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [compiler-rt] [lldb] [llvm] [flang] [clang] [mlir] [clang-tools-extra] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
@@ -1331,6 +1336,18 @@ static int merge_main(int argc, const char *argv[]) {
"(default: 1)"));
cl::ParseCommandLineOptions(argc, argv, "LLVM profile data merger\n");
+ if (!DebugInfoFilename.empty() && !BinaryFilename.empty()) {
+exitWithError("Expected only one of -debug-info, -binary-file");
MaskRay wrote:
drop braces
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
https://github.com/llvm/llvm-project/pull/69493
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [compiler-rt] [clang-tools-extra] [lldb] [clang] [flang] [mlir] [llvm] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
@@ -0,0 +1,11 @@ +; RUN: opt < %s -passes=instrprof -profile-correlate=binary -S | FileCheck %s MaskRay wrote: This isn't clear how `__profd_foo` is different from `-profile-correlate=none`. This test can be added to `coverage.ll` as a new RUN line https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang-tools-extra] [compiler-rt] [flang] [llvm] [lldb] [mlir] [clang] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
@@ -1331,6 +1336,18 @@ static int merge_main(int argc, const char *argv[]) {
"(default: 1)"));
cl::ParseCommandLineOptions(argc, argv, "LLVM profile data merger\n");
+ if (!DebugInfoFilename.empty() && !BinaryFilename.empty()) {
+exitWithError("Expected only one of -debug-info, -binary-file");
+ }
+ std::string CorrelateFilename = "";
MaskRay wrote:
remove `= ""`
https://github.com/llvm/llvm-project/pull/69493
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [lldb] Fix size in bytes of type DIEs when size in bits is not a multiple of 8 (PR #69741)
@@ -2210,9 +2216,16 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE ,
!layout_info.vbase_offsets.empty()) {
if (type)
layout_info.bit_size = type->GetByteSize(nullptr).value_or(0) * 8;
-if (layout_info.bit_size == 0)
- layout_info.bit_size =
- die.GetAttributeValueAsUnsigned(DW_AT_byte_size, 0) * 8;
+if (layout_info.bit_size == 0) {
+ auto byte_size =
+ die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
+
+ if (byte_size != UINT64_MAX)
+layout_info.bit_size = byte_size;
Michael137 wrote:
How come we don't need to do the `* 8` here to go from bytes to bits?
https://github.com/llvm/llvm-project/pull/69741
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [lldb] Fix size in bytes of type DIEs when size in bits is not a multiple of 8 (PR #69741)
@@ -298,6 +298,12 @@ ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const
DWARFDIE ) {
byte_size = form_value.Unsigned();
break;
+case DW_AT_bit_size:
+ // Convert the bit size to byte size, and round it up to the minimum
about
+ // of bytes that will fit the bits.
+ byte_size = (form_value.Unsigned() + 7) / 8;
Michael137 wrote:
Oh kind-of-related discourse post from last year:
https://discourse.llvm.org/t/rfc-proper-low-level-abstractions-to-llvm-bits-bytes-addresses-and-masks/63081
Looks like it was never resolved
https://github.com/llvm/llvm-project/pull/69741
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [lldb] Fix size in bytes of type DIEs when size in bits is not a multiple of 8 (PR #69741)
@@ -298,6 +298,12 @@ ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const
DWARFDIE ) {
byte_size = form_value.Unsigned();
break;
+case DW_AT_bit_size:
+ // Convert the bit size to byte size, and round it up to the minimum
about
+ // of bytes that will fit the bits.
+ byte_size = (form_value.Unsigned() + 7) / 8;
Michael137 wrote:
Completely out of scope of the PR but wonder if we should introduce something
in `llvm/Support/MathExtras.h` that does this for us.
```
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp:969:
int n = (count + 7) / 8;
clang/test/SemaCXX/constexpr-duffs-device.cpp:6:unsigned long n =
(count + 7) / 8;
clang/lib/CodeGen/Targets/X86.cpp:2988: uint64_t SizeInBytes =
(CGF.getContext().getTypeSize(Ty) + 7) / 8;
flang/runtime/edit-input.cpp:84: auto
significantBytes{static_cast(digits * LOG2_BASE + 7) / 8};
lldb/source/Core/ValueObjectChild.cpp:172:(bitfield_end -
*type_bit_size + 7) / 8;
lldb/source/Core/DumpDataExtractor.cpp:674:
(llvm::APFloat::getSizeInBits(semantics) + 7) / 8;
lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp:1044:
lldb::offset_t value_alignment = (*opt_alignment + 7ull) / 8ull;
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:816: return
(*bit_size + 7) / 8;
lldb/source/Symbol/CompilerType.cpp:561:return (*bit_size + 7) / 8;
lldb/source/Expression/Materializer.cpp:554:size_t byte_align =
(*opt_bit_align + 7) / 8;
lldb/source/Expression/Materializer.cpp:952: size_t byte_align =
(*opt_bit_align + 7) / 8;
lldb/source/Utility/Scalar.cpp:118:StoreIntToMemory(val, storage.data(),
(val.getBitWidth() + 7) / 8);
llvm/include/llvm/CodeGen/MachineValueType.h:354: return
{(BaseSize.getKnownMinValue() + 7) / 8, BaseSize.isScalable()};
llvm/include/llvm/CodeGen/TargetLowering.h:3466: uint32_t
CondCodeActions[ISD::SETCC_INVALID][(MVT::VALUETYPE_SIZE + 7) / 8];
llvm/include/llvm/CodeGen/ValueTypes.h:375: return
{(BaseSize.getKnownMinValue() + 7) / 8, BaseSize.isScalable()};
llvm/include/llvm/CodeGen/LowLevelType.h:187:return
{(BaseSize.getKnownMinValue() + 7) / 8, BaseSize.isScalable()};
llvm/utils/TableGen/AsmWriterEmitter.cpp:407: unsigned BytesNeeded =
((OpcodeInfoBits - BitsLeft) + 7) / 8;
llvm/lib/Analysis/ConstantFolding.cpp:598: unsigned BytesLoaded =
(IntType->getBitWidth() + 7) / 8;
llvm/lib/Target/M68k/M68kISelLowering.cpp:736: uint32_t OpSize =
(VA.getLocVT().getSizeInBits() + 7) / 8;
llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp:1834:size_t NumBytes =
(Val.getBitWidth() + 7) / 8;
llvm/lib/Target/Lanai/MCTargetDesc/LanaiAsmBackend.cpp:101: unsigned NumBytes
=
[Lldb-commits] [lldb] [llvm] Fix size in bytes of type DIEs when size in bits is not a multiple of 8 (PR #69741)
@@ -2866,8 +2879,12 @@ void DWARFASTParserClang::ParseSingleMember( // Get the parent byte size so we can verify any members will fit const uint64_t parent_byte_size = parent_die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX); - const uint64_t parent_bit_size = - parent_byte_size == UINT64_MAX ? UINT64_MAX : parent_byte_size * 8; + uint64_t parent_bit_size; + if (parent_byte_size != UINT64_MAX) +parent_bit_size = parent_byte_size * 8; + else +parent_bit_size = +parent_die.GetAttributeValueAsUnsigned(DW_AT_bit_size, UINT64_MAX); Michael137 wrote: Thoughts on extracting this block into a helper? Then you can use it above as well. https://github.com/llvm/llvm-project/pull/69741 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [lldb] Fix size in bytes of type DIEs when size in bits is not a multiple of 8 (PR #69741)
@@ -2866,8 +2879,12 @@ void DWARFASTParserClang::ParseSingleMember( // Get the parent byte size so we can verify any members will fit const uint64_t parent_byte_size = parent_die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX); - const uint64_t parent_bit_size = - parent_byte_size == UINT64_MAX ? UINT64_MAX : parent_byte_size * 8; + uint64_t parent_bit_size; + if (parent_byte_size != UINT64_MAX) +parent_bit_size = parent_byte_size * 8; + else +parent_bit_size = +parent_die.GetAttributeValueAsUnsigned(DW_AT_bit_size, UINT64_MAX); Michael137 wrote: Do we already have tests in LLDB that cover this? https://github.com/llvm/llvm-project/pull/69741 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Fix size in bytes of type DIEs when size in bits is not a multiple of 8 (PR #69741)
@@ -298,6 +298,12 @@ ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const
DWARFDIE ) {
byte_size = form_value.Unsigned();
break;
+case DW_AT_bit_size:
+ // Convert the bit size to byte size, and round it up to the minimum
about
Michael137 wrote:
```suggestion
// Convert the bit size to byte size, and round it up to the minimum
amount
```
https://github.com/llvm/llvm-project/pull/69741
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang-tools-extra] [llvm] [lldb] [flang] [mlir] [clang] [compiler-rt] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
@@ -1341,20 +1344,26 @@ void
InstrProfiling::createDataVariable(InstrProfCntrInstBase *Inc,
}
auto *Data =
new GlobalVariable(*M, DataTy, false, Linkage, nullptr, DataVarName);
- // Reference the counter variable with a label difference (link-time
- // constant).
- auto *RelativeCounterPtr =
- ConstantExpr::getSub(ConstantExpr::getPtrToInt(CounterPtr, IntPtrTy),
- ConstantExpr::getPtrToInt(Data, IntPtrTy));
-
- // Bitmaps are relative to the same data variable as profile counters.
+ Constant *RelativeCounterPtr;
GlobalVariable *BitmapPtr = PD.RegionBitmaps;
Constant *RelativeBitmapPtr = ConstantInt::get(IntPtrTy, 0);
-
- if (BitmapPtr != nullptr) {
-RelativeBitmapPtr =
-ConstantExpr::getSub(ConstantExpr::getPtrToInt(BitmapPtr, IntPtrTy),
+ // By default counter ptr and bitmap ptr are address relative to data
section.
MaskRay wrote:
This comment duplicates a bit of "// Reference the counter variable with a
label difference (link-time".
We can just say: With binary profile correlation, profd is not loaded into
memory. profd must reference profc with an absolute relocation.
https://github.com/llvm/llvm-project/pull/69493
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang-tools-extra] [compiler-rt] [flang] [llvm] [lldb] [mlir] [clang] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
@@ -1829,6 +1833,22 @@ void CoverageMappingModuleGen::emit() {
llvm::GlobalValue::InternalLinkage, NamesArrVal,
llvm::getCoverageUnusedNamesVarName());
}
+ const StringRef VarName(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR));
+ llvm::Type *IntTy64 = llvm::Type::getInt64Ty(Ctx);
+ uint64_t ProfileVersion = INSTR_PROF_RAW_VERSION;
+ if (llvm::ProfileCorrelate == llvm::InstrProfCorrelator::BINARY)
+ProfileVersion |= VARIANT_MASK_BIN_CORRELATE;
+ auto *VersionVariable = new llvm::GlobalVariable(
+ CGM.getModule(), llvm::Type::getInt64Ty(Ctx), true,
+ llvm::GlobalValue::WeakAnyLinkage,
+ llvm::Constant::getIntegerValue(IntTy64, llvm::APInt(64,
ProfileVersion)),
+ VarName);
+ VersionVariable->setVisibility(llvm::GlobalValue::HiddenVisibility);
+ llvm::Triple TT(CGM.getModule().getTargetTriple());
+ if (TT.supportsCOMDAT()) {
MaskRay wrote:
`CGM.getTriple().supportsCOMDAT()`
https://github.com/llvm/llvm-project/pull/69493
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [flang] [compiler-rt] [lldb] [clang] [llvm] [mlir] [clang-tools-extra] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
@@ -195,8 +195,14 @@ OPTIONS .. option:: --debug-info= Specify the executable or ``.dSYM`` that contains debug info for the raw profile. - When ``-debug-info-correlate`` was used for instrumentation, use this option - to correlate the raw profile. + When ``-profile-correlate=debug-info`` was used for instrumentation, use this MaskRay wrote: `--profile-correlate=debug-info` (double-dash) https://github.com/llvm/llvm-project/pull/69493 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [compiler-rt] [mlir] [llvm] [clang] [clang-tools-extra] [flang] [lldb] [Profile] Add binary profile correlation to offload profile metadata at runtime. (PR #69493)
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm
-o - %s | FileCheck %s
+// RUN: %clang_cc1 -mllvm -profile-correlate=binary -fprofile-instrument=clang
-fcoverage-mapping -emit-llvm -o - %s | FileCheck %s
--check-prefix=BIN-CORRELATE
MaskRay wrote:
Specify an explicit `-target-triple` to test a COMDAT triple (e.g. Linux) and a
non-COMDAT triple (e.g. macOS).
Change `// CHECK: @__llvm_profile_raw_version = {{.*}} i64 9` to test more
properties including the visibility and `comdat`
https://github.com/llvm/llvm-project/pull/69493
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add an option to provide a format for stack frames (PR #71843)
walter-erquinigo wrote: @clayborg , I did pretty much what you asked but with a few changes: - I added the `customFrameFormat` option to the json config, and it'll be used if provided and non-empty. I just find it simpler to control everything with one single option. - I added the SBFormat class that wraps around FormatEntity::Entry. I had to convert FormatEntity into a namespace to be able to use forward declarations, but the change was extremely trivial because FormatEntity was just a bag of static functions. - lldb-dap will dump an error on the debug console if the provided format is wrong, and the original flow is used in this case. https://github.com/llvm/llvm-project/pull/71843 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add an option to provide a format for stack frames (PR #71843)
https://github.com/walter-erquinigo edited https://github.com/llvm/llvm-project/pull/71843 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add an option to provide a format for stack frames (PR #71843)
https://github.com/walter-erquinigo edited https://github.com/llvm/llvm-project/pull/71843 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add an option to show function args in stack frames (PR #71843)
https://github.com/walter-erquinigo updated
https://github.com/llvm/llvm-project/pull/71843
>From 2f3841dc6d68f755b11f6677ed2d034a88a297c8 Mon Sep 17 00:00:00 2001
From: walter erquinigo
Date: Thu, 9 Nov 2023 13:15:55 -0500
Subject: [PATCH] [lldb-dap] Add an option to show function args in stack
frames
When this option is enabled, display names of stack frames are generated using
the `${function.name-with-args}` formatter instead of simply calling
`SBFrame::GetDisplayFunctionName`. This makes lldb-dap show an output similar
to the one in the CLI.
This option is disabled by default because of its performance cost. It's a good
option for non-gigantic programs.
---
lldb/include/lldb/API/SBDefines.h | 1 +
lldb/include/lldb/API/SBError.h | 1 +
lldb/include/lldb/API/SBFormat.h | 72
lldb/include/lldb/API/SBFrame.h | 23 +-
lldb/include/lldb/Core/FormatEntity.h | 404 +-
lldb/include/lldb/Target/StackFrame.h | 24 +-
lldb/include/lldb/lldb-forward.h | 4 +
.../test/tools/lldb-dap/dap_server.py | 4 +
.../test/tools/lldb-dap/lldbdap_testcase.py | 4 +
lldb/source/API/CMakeLists.txt| 1 +
lldb/source/API/SBFormat.cpp | 50 +++
lldb/source/API/SBFrame.cpp | 40 +-
lldb/source/Core/FormatEntity.cpp | 20 +-
lldb/source/Target/StackFrame.cpp | 35 +-
.../lldb-dap/stackTrace/TestDAP_stackTrace.py | 23 +-
lldb/tools/lldb-dap/DAP.cpp | 11 +
lldb/tools/lldb-dap/DAP.h | 4 +
lldb/tools/lldb-dap/JSONUtils.cpp | 17 +-
lldb/tools/lldb-dap/lldb-dap.cpp | 2 +
lldb/tools/lldb-dap/package.json | 10 +
20 files changed, 507 insertions(+), 243 deletions(-)
create mode 100644 lldb/include/lldb/API/SBFormat.h
create mode 100644 lldb/source/API/SBFormat.cpp
diff --git a/lldb/include/lldb/API/SBDefines.h
b/lldb/include/lldb/API/SBDefines.h
index c6f01cc03f263c8..2630a82df0e7135 100644
--- a/lldb/include/lldb/API/SBDefines.h
+++ b/lldb/include/lldb/API/SBDefines.h
@@ -71,6 +71,7 @@ class LLDB_API SBExpressionOptions;
class LLDB_API SBFile;
class LLDB_API SBFileSpec;
class LLDB_API SBFileSpecList;
+class LLDB_API SBFormat;
class LLDB_API SBFrame;
class LLDB_API SBFunction;
class LLDB_API SBHostOS;
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index b241052ed9cc2a2..1a720a479d9a689 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -80,6 +80,7 @@ class LLDB_API SBError {
friend class SBData;
friend class SBDebugger;
friend class SBFile;
+ friend class SBFormat;
friend class SBHostOS;
friend class SBPlatform;
friend class SBProcess;
diff --git a/lldb/include/lldb/API/SBFormat.h b/lldb/include/lldb/API/SBFormat.h
new file mode 100644
index 000..b913091fbada524
--- /dev/null
+++ b/lldb/include/lldb/API/SBFormat.h
@@ -0,0 +1,72 @@
+
+//===-- SBFormat.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_API_SBFORMAT_H
+#define LLDB_API_SBFORMAT_H
+
+#include "lldb/API/SBDefines.h"
+
+namespace lldb_private {
+namespace python {
+class SWIGBridge;
+} // namespace python
+namespace lua {
+class SWIGBridge;
+} // namespace lua
+} // namespace lldb_private
+
+namespace lldb {
+
+/// Class that represents a format string that can be used to generate
+/// descriptions of objects like frames and threads. See
+/// https://lldb.llvm.org/use/formatting.html for more information.
+class LLDB_API SBFormat {
+public:
+ SBFormat();
+
+ SBFormat(const lldb::SBFormat );
+
+ lldb::SBFormat =(const lldb::SBFormat );
+
+ ~SBFormat();
+
+ /// bool operator version of \a IsValid().
+ explicit operator bool() const;
+
+ /// \return
+ /// \b true if and only if this object is valid and can be used for
+ /// formatting.
+ bool IsValid() const;
+
+ /// Parse the given format string.
+ ///
+ /// \param[in] format
+ /// The format string to parse.
+ ///
+ /// \param[out] error
+ /// An object where error messages will be written to if parsing fails.
+ ///
+ /// \return
+ /// An \a SBFormat object with the parsing result, which might be an
invalid
+ /// object if parsing fails.
+ static lldb::SBFormat Parse(const char *format, lldb::SBError );
+
+protected:
+ friend class SBFrame;
+
+ /// \return
+ /// The underlying shared pointer storage for this object.
+ lldb::FormatEntrySP GetFormatEntrySP() const;
+
+ /// The storage for this object.
+ lldb::FormatEntrySP m_opaque_sp;
+};
+
+} // namespace lldb
[Lldb-commits] [lldb] [lldb-dap] Add an option to show function args in stack frames (PR #71843)
https://github.com/walter-erquinigo updated
https://github.com/llvm/llvm-project/pull/71843
>From 7b9b5897e2e801d7bc97639ed302b885481659bd Mon Sep 17 00:00:00 2001
From: walter erquinigo
Date: Thu, 9 Nov 2023 13:15:55 -0500
Subject: [PATCH] [lldb-dap] Add an option to show function args in stack
frames
When this option is enabled, display names of stack frames are generated using
the `${function.name-with-args}` formatter instead of simply calling
`SBFrame::GetDisplayFunctionName`. This makes lldb-dap show an output similar
to the one in the CLI.
This option is disabled by default because of its performance cost. It's a good
option for non-gigantic programs.
---
lldb/include/lldb/API/SBDefines.h | 1 +
lldb/include/lldb/API/SBError.h | 1 +
lldb/include/lldb/API/SBFormat.h | 72
lldb/include/lldb/API/SBFrame.h | 23 +-
lldb/include/lldb/Core/FormatEntity.h | 404 +-
lldb/include/lldb/Target/StackFrame.h | 24 +-
lldb/include/lldb/lldb-forward.h | 4 +
.../test/tools/lldb-dap/dap_server.py | 4 +
.../test/tools/lldb-dap/lldbdap_testcase.py | 4 +
lldb/source/API/CMakeLists.txt| 1 +
lldb/source/API/SBFormat.cpp | 50 +++
lldb/source/API/SBFrame.cpp | 40 +-
lldb/source/Core/FormatEntity.cpp | 20 +-
lldb/source/Target/StackFrame.cpp | 35 +-
.../lldb-dap/stackTrace/TestDAP_stackTrace.py | 23 +-
lldb/tools/lldb-dap/DAP.cpp | 11 +
lldb/tools/lldb-dap/DAP.h | 4 +
lldb/tools/lldb-dap/JSONUtils.cpp | 17 +-
lldb/tools/lldb-dap/lldb-dap.cpp | 2 +
lldb/tools/lldb-dap/package.json | 10 +
20 files changed, 507 insertions(+), 243 deletions(-)
create mode 100644 lldb/include/lldb/API/SBFormat.h
create mode 100644 lldb/source/API/SBFormat.cpp
diff --git a/lldb/include/lldb/API/SBDefines.h
b/lldb/include/lldb/API/SBDefines.h
index c6f01cc03f263c8..2630a82df0e7135 100644
--- a/lldb/include/lldb/API/SBDefines.h
+++ b/lldb/include/lldb/API/SBDefines.h
@@ -71,6 +71,7 @@ class LLDB_API SBExpressionOptions;
class LLDB_API SBFile;
class LLDB_API SBFileSpec;
class LLDB_API SBFileSpecList;
+class LLDB_API SBFormat;
class LLDB_API SBFrame;
class LLDB_API SBFunction;
class LLDB_API SBHostOS;
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index b241052ed9cc2a2..1a720a479d9a689 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -80,6 +80,7 @@ class LLDB_API SBError {
friend class SBData;
friend class SBDebugger;
friend class SBFile;
+ friend class SBFormat;
friend class SBHostOS;
friend class SBPlatform;
friend class SBProcess;
diff --git a/lldb/include/lldb/API/SBFormat.h b/lldb/include/lldb/API/SBFormat.h
new file mode 100644
index 000..b913091fbada524
--- /dev/null
+++ b/lldb/include/lldb/API/SBFormat.h
@@ -0,0 +1,72 @@
+
+//===-- SBFormat.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_API_SBFORMAT_H
+#define LLDB_API_SBFORMAT_H
+
+#include "lldb/API/SBDefines.h"
+
+namespace lldb_private {
+namespace python {
+class SWIGBridge;
+} // namespace python
+namespace lua {
+class SWIGBridge;
+} // namespace lua
+} // namespace lldb_private
+
+namespace lldb {
+
+/// Class that represents a format string that can be used to generate
+/// descriptions of objects like frames and threads. See
+/// https://lldb.llvm.org/use/formatting.html for more information.
+class LLDB_API SBFormat {
+public:
+ SBFormat();
+
+ SBFormat(const lldb::SBFormat );
+
+ lldb::SBFormat =(const lldb::SBFormat );
+
+ ~SBFormat();
+
+ /// bool operator version of \a IsValid().
+ explicit operator bool() const;
+
+ /// \return
+ /// \b true if and only if this object is valid and can be used for
+ /// formatting.
+ bool IsValid() const;
+
+ /// Parse the given format string.
+ ///
+ /// \param[in] format
+ /// The format string to parse.
+ ///
+ /// \param[out] error
+ /// An object where error messages will be written to if parsing fails.
+ ///
+ /// \return
+ /// An \a SBFormat object with the parsing result, which might be an
invalid
+ /// object if parsing fails.
+ static lldb::SBFormat Parse(const char *format, lldb::SBError );
+
+protected:
+ friend class SBFrame;
+
+ /// \return
+ /// The underlying shared pointer storage for this object.
+ lldb::FormatEntrySP GetFormatEntrySP() const;
+
+ /// The storage for this object.
+ lldb::FormatEntrySP m_opaque_sp;
+};
+
+} // namespace lldb
[Lldb-commits] [lldb] [lldb-dap] Add an option to show function args in stack frames (PR #71843)
https://github.com/walter-erquinigo updated
https://github.com/llvm/llvm-project/pull/71843
>From 980f1e0ceb43da5f4d504f7ed04c3abb591e406d Mon Sep 17 00:00:00 2001
From: walter erquinigo
Date: Thu, 9 Nov 2023 13:15:55 -0500
Subject: [PATCH] [lldb-dap] Add an option to show function args in stack
frames
When this option is enabled, display names of stack frames are generated using
the `${function.name-with-args}` formatter instead of simply calling
`SBFrame::GetDisplayFunctionName`. This makes lldb-dap show an output similar
to the one in the CLI.
This option is disabled by default because of its performance cost. It's a good
option for non-gigantic programs.
---
lldb/include/lldb/API/SBDefines.h | 1 +
lldb/include/lldb/API/SBError.h | 1 +
lldb/include/lldb/API/SBFormat.h | 72
lldb/include/lldb/API/SBFrame.h | 23 +-
lldb/include/lldb/Core/FormatEntity.h | 404 +-
lldb/include/lldb/Target/StackFrame.h | 24 +-
lldb/include/lldb/lldb-forward.h | 4 +
.../test/tools/lldb-dap/dap_server.py | 4 +
.../test/tools/lldb-dap/lldbdap_testcase.py | 4 +
lldb/source/API/CMakeLists.txt| 1 +
lldb/source/API/SBFormat.cpp | 50 +++
lldb/source/API/SBFrame.cpp | 39 +-
lldb/source/Core/FormatEntity.cpp | 20 +-
lldb/source/Target/StackFrame.cpp | 35 +-
.../lldb-dap/stackTrace/TestDAP_stackTrace.py | 23 +-
lldb/tools/lldb-dap/DAP.cpp | 11 +
lldb/tools/lldb-dap/DAP.h | 4 +
lldb/tools/lldb-dap/JSONUtils.cpp | 17 +-
lldb/tools/lldb-dap/lldb-dap.cpp | 2 +
lldb/tools/lldb-dap/package.json | 10 +
20 files changed, 506 insertions(+), 243 deletions(-)
create mode 100644 lldb/include/lldb/API/SBFormat.h
create mode 100644 lldb/source/API/SBFormat.cpp
diff --git a/lldb/include/lldb/API/SBDefines.h
b/lldb/include/lldb/API/SBDefines.h
index c6f01cc03f263c8..2630a82df0e7135 100644
--- a/lldb/include/lldb/API/SBDefines.h
+++ b/lldb/include/lldb/API/SBDefines.h
@@ -71,6 +71,7 @@ class LLDB_API SBExpressionOptions;
class LLDB_API SBFile;
class LLDB_API SBFileSpec;
class LLDB_API SBFileSpecList;
+class LLDB_API SBFormat;
class LLDB_API SBFrame;
class LLDB_API SBFunction;
class LLDB_API SBHostOS;
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index b241052ed9cc2a2..1a720a479d9a689 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -80,6 +80,7 @@ class LLDB_API SBError {
friend class SBData;
friend class SBDebugger;
friend class SBFile;
+ friend class SBFormat;
friend class SBHostOS;
friend class SBPlatform;
friend class SBProcess;
diff --git a/lldb/include/lldb/API/SBFormat.h b/lldb/include/lldb/API/SBFormat.h
new file mode 100644
index 000..b913091fbada524
--- /dev/null
+++ b/lldb/include/lldb/API/SBFormat.h
@@ -0,0 +1,72 @@
+
+//===-- SBFormat.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_API_SBFORMAT_H
+#define LLDB_API_SBFORMAT_H
+
+#include "lldb/API/SBDefines.h"
+
+namespace lldb_private {
+namespace python {
+class SWIGBridge;
+} // namespace python
+namespace lua {
+class SWIGBridge;
+} // namespace lua
+} // namespace lldb_private
+
+namespace lldb {
+
+/// Class that represents a format string that can be used to generate
+/// descriptions of objects like frames and threads. See
+/// https://lldb.llvm.org/use/formatting.html for more information.
+class LLDB_API SBFormat {
+public:
+ SBFormat();
+
+ SBFormat(const lldb::SBFormat );
+
+ lldb::SBFormat =(const lldb::SBFormat );
+
+ ~SBFormat();
+
+ /// bool operator version of \a IsValid().
+ explicit operator bool() const;
+
+ /// \return
+ /// \b true if and only if this object is valid and can be used for
+ /// formatting.
+ bool IsValid() const;
+
+ /// Parse the given format string.
+ ///
+ /// \param[in] format
+ /// The format string to parse.
+ ///
+ /// \param[out] error
+ /// An object where error messages will be written to if parsing fails.
+ ///
+ /// \return
+ /// An \a SBFormat object with the parsing result, which might be an
invalid
+ /// object if parsing fails.
+ static lldb::SBFormat Parse(const char *format, lldb::SBError );
+
+protected:
+ friend class SBFrame;
+
+ /// \return
+ /// The underlying shared pointer storage for this object.
+ lldb::FormatEntrySP GetFormatEntrySP() const;
+
+ /// The storage for this object.
+ lldb::FormatEntrySP m_opaque_sp;
+};
+
+} // namespace lldb
[Lldb-commits] [lldb] [lldb-dap] Add an option to show function args in stack frames (PR #71843)
@@ -187,3 +188,19 @@ def test_stackTrace(self): self.assertEquals( 0, len(stackFrames), "verify zero frames with startFrame out of bounds" ) + +@skipIfWindows walter-erquinigo wrote: I think all vscode tests skip on windows... https://github.com/llvm/llvm-project/pull/71843 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)
@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create( return std::make_unique( walter-erquinigo wrote: These asserts might not always pass? I don't know if it's guaranteed that if one of those sections is present, then the rest of them are as well. https://github.com/llvm/llvm-project/pull/71828 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)
https://github.com/clayborg commented: See inline comment https://github.com/llvm/llvm-project/pull/71828 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)
@@ -57,7 +57,9 @@ std::unique_ptr AppleDWARFIndex::Create( return std::make_unique( clayborg wrote: DataExtractor objects don't always have shared buffers. I am guessing in this case it does have them, but you might throw in a `lldbassert(...)` just in case to make sure this doesn't regress in the future. Maybe something like: ``` lldbassert(apple_names.GetSharedDataBuffer()); lldbassert(apple_namespaces.GetSharedDataBuffer()); lldbassert(apple_types.GetSharedDataBuffer()); lldbassert(apple_objc.GetSharedDataBuffer()); ``` https://github.com/llvm/llvm-project/pull/71828 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/71828 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [lldb] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
@@ -62,15 +66,25 @@ bool canUseDebuginfod() {
}
SmallVector getDefaultDebuginfodUrls() {
- const char *DebuginfodUrlsEnv = std::getenv("DEBUGINFOD_URLS");
- if (DebuginfodUrlsEnv == nullptr)
-return SmallVector();
-
- SmallVector DebuginfodUrls;
- StringRef(DebuginfodUrlsEnv).split(DebuginfodUrls, " ");
+ if (!DebuginfodUrlsSet) {
+// Only read from the environment variable if the user hasn't already
+// set the value
+const char *DebuginfodUrlsEnv = std::getenv("DEBUGINFOD_URLS");
+if (DebuginfodUrlsEnv != nullptr) {
+ StringRef(DebuginfodUrlsEnv).split(DebuginfodUrls, " ", -1, false);
+}
+DebuginfodUrlsSet = true;
+ }
return DebuginfodUrls;
}
+// Override the default debuginfod URL list.
+void setDefaultDebuginfodUrls(SmallVector URLs) {
+ DebuginfodUrls.clear();
clayborg wrote:
```
void setDefaultDebuginfodUrls(SmallVector URLs, bool append) {
if (!append)
DebuginfodUrls.clear();
```
https://github.com/llvm/llvm-project/pull/70996
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [lldb] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
@@ -46,6 +46,10 @@ bool canUseDebuginfod(); /// environment variable. SmallVector getDefaultDebuginfodUrls(); +/// Sets the list of debuginfod server URLs to query. This overrides the +/// environment variable DEBUGINFOD_URLS. clayborg wrote: Should this override the env vars? Or maybe add to them? Maybe we add a bool parameter to allow the user to choose like `bool append`? https://github.com/llvm/llvm-project/pull/70996 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [lldb] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
@@ -4180,7 +4180,6 @@ TargetProperties::TargetProperties(Target *target)
ePropertyInheritTCC, [this] { InheritTCCValueChangedCallback(); });
m_collection_sp->SetValueChangedCallback(
ePropertyDisableSTDIO, [this] { DisableSTDIOValueChangedCallback(); });
-
clayborg wrote:
revert this
https://github.com/llvm/llvm-project/pull/70996
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
@@ -0,0 +1,154 @@
+//===-- SymbolLocatorDebuginfod.cpp
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SymbolLocatorDebuginfod.h"
+
+#include
+#include
+
+#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/Host.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/StreamString.h"
+#include "lldb/Utility/Timer.h"
+#include "lldb/Utility/UUID.h"
+
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/Debuginfod/HTTPClient.h"
+#include "llvm/Debuginfod/Debuginfod.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/ThreadPool.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+LLDB_PLUGIN_DEFINE(SymbolLocatorDebuginfod)
+
+namespace {
+
+#define LLDB_PROPERTIES_symbollocatordebuginfod
+#include "SymbolLocatorDebuginfodProperties.inc"
+
+enum {
+#define LLDB_PROPERTIES_symbollocatordebuginfod
+#include "SymbolLocatorDebuginfodPropertiesEnum.inc"
+};
+
+class PluginProperties : public Properties {
+public:
+ static llvm::StringRef GetSettingName() {
+return SymbolLocatorDebuginfod::GetPluginNameStatic();
+ }
+
+ PluginProperties() {
+m_collection_sp =
std::make_shared(GetSettingName());
+m_collection_sp->Initialize(g_symbollocatordebuginfod_properties);
+m_collection_sp->SetValueChangedCallback(
+ePropertyURLs, [this] { URLsChangedCallback(); }
+);
+ }
+
+ Args GetDebugInfoDURLs() const {
+Args urls;
+m_collection_sp->GetPropertyAtIndexAsArgs(ePropertyURLs, urls);
+return urls;
+ }
+
+private:
+ void URLsChangedCallback() {
+Args urls = GetDebugInfoDURLs();
+llvm::SmallVector dbginfod_urls;
+llvm::transform(urls, dbginfod_urls.end(),
+[](const auto ) { return obj.ref(); });
+llvm::setDefaultDebuginfodUrls(dbginfod_urls);
+ }
+};
+
+} // namespace
+
+
+SymbolLocatorDebuginfod::SymbolLocatorDebuginfod() : SymbolLocator() {}
+
+void SymbolLocatorDebuginfod::Initialize() {
+ PluginManager::RegisterPlugin(
+ GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance,
+ LocateExecutableObjectFile, LocateExecutableSymbolFile,
+ DownloadObjectAndSymbolFile);
+ // There's a "safety" concern on this:
+ // Does plugin initialization occur while things are still single threaded?
+ llvm::HTTPClient::initialize();
+}
+
+void SymbolLocatorDebuginfod::Terminate() {
+ PluginManager::UnregisterPlugin(CreateInstance);
+ // There's a "safety" concern on this:
+ // Does plugin termination occur while things are still single threaded?
clayborg wrote:
Most probably won't be signle threaded. If this is a problem, you might leave
this call out.
https://github.com/llvm/llvm-project/pull/70996
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [lldb] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
@@ -0,0 +1,154 @@ +//===-- SymbolLocatorDebuginfod.cpp --===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "SymbolLocatorDebuginfod.h" + +#include +#include + +#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h" +#include "lldb/Core/Debugger.h" +#include "lldb/Core/Module.h" +#include "lldb/Core/ModuleList.h" +#include "lldb/Core/ModuleSpec.h" +#include "lldb/Core/PluginManager.h" +#include "lldb/Core/Progress.h" +#include "lldb/Core/Section.h" +#include "lldb/Host/FileSystem.h" +#include "lldb/Host/Host.h" +#include "lldb/Symbol/ObjectFile.h" +#include "lldb/Target/Target.h" +#include "lldb/Utility/ArchSpec.h" +#include "lldb/Utility/DataBuffer.h" +#include "lldb/Utility/DataExtractor.h" +#include "lldb/Utility/LLDBLog.h" +#include "lldb/Utility/Log.h" +#include "lldb/Utility/StreamString.h" +#include "lldb/Utility/Timer.h" +#include "lldb/Utility/UUID.h" + +#include "llvm/ADT/SmallSet.h" +#include "llvm/Debuginfod/HTTPClient.h" +#include "llvm/Debuginfod/Debuginfod.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/ThreadPool.h" clayborg wrote: Ditto with these include files https://github.com/llvm/llvm-project/pull/70996 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [lldb] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
@@ -4,7 +4,7 @@ let Definition = "modulelist" in {
def EnableExternalLookup: Property<"enable-external-lookup", "Boolean">,
Global,
DefaultTrue,
-Desc<"Control the use of external tools and repositories to locate symbol
files. Directories listed in target.debug-file-search-paths and directory of
the executable are always checked first for separate debug info files. Then
depending on this setting: On macOS, Spotlight would be also used to locate a
matching .dSYM bundle based on the UUID of the executable. On NetBSD, directory
/usr/libdata/debug would be also searched. On platforms other than NetBSD
directory /usr/lib/debug would be also searched.">;
+Desc<"Control the use of external tools and repositories to locate symbol
files. Directories listed in target.debug-file-search-paths and directory of
the executable are always checked first for separate debug info files. Then
depending on this setting: On macOS, Spotlight would be also used to locate a
matching .dSYM bundle based on the UUID of the executable. On NetBSD, directory
/usr/libdata/debug would be also searched. On platforms other than NetBSD
directory /usr/lib/debug would be also searched. If all other methods fail, and
the DEBUGINFOD_URLS environment variable is specified, the Debuginfod protocol
is used to acquire symbols from a compatible Debuginfod service.">;
clayborg wrote:
We we want to mention the new setting users can set here as well for debuginfod?
https://github.com/llvm/llvm-project/pull/70996
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
@@ -0,0 +1,8 @@
+include "../../../../include/lldb/Core/PropertiesBase.td"
+
+let Definition = "symbollocatordebuginfod" in {
+ def URLs: Property<"urls", "String">,
+Global,
+DefaultStringValue<"">,
+Desc<"A space-separated, ordered list of Debuginfod server URLs to query
for symbols">;
+}
clayborg wrote:
What does this look like on the command line? Other plug-in settings look like
this:
```
plugin.dynamic-loader.darwin-kernel.load-kexts (boolean) = true
plugin.dynamic-loader.darwin-kernel.scan-type (enum) = fast-scan
plugin.jit-loader.gdb.enable (enum) = default
plugin.object-file.pe-coff.abi (enum) = default
plugin.object-file.pe-coff.module-abi (dictionary of enums) =
plugin.process.kdp-remote.packet-timeout (unsigned) = 5
plugin.process.gdb-remote.packet-timeout (unsigned) = 5
plugin.process.gdb-remote.target-definition-file (file) =
plugin.process.gdb-remote.use-g-packet-for-reading (boolean) = false
plugin.process.gdb-remote.use-libraries-svr4 (boolean) = true
plugin.symbol-file.dwarf.ignore-file-indexes (boolean) = false
plugin.structured-data.darwin-log.auto-enable-options (string) =
plugin.structured-data.darwin-log.enable-on-startup (boolean) = false
```
So hopefully this show up in `settings set
plugin.symbolvendor.symbollocatordebuginfod.urls []`
https://github.com/llvm/llvm-project/pull/70996
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [lldb] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
@@ -0,0 +1,154 @@
+//===-- SymbolLocatorDebuginfod.cpp
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SymbolLocatorDebuginfod.h"
+
+#include
+#include
+
+#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/Host.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/StreamString.h"
+#include "lldb/Utility/Timer.h"
+#include "lldb/Utility/UUID.h"
+
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/Debuginfod/HTTPClient.h"
+#include "llvm/Debuginfod/Debuginfod.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/ThreadPool.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+LLDB_PLUGIN_DEFINE(SymbolLocatorDebuginfod)
+
+namespace {
+
+#define LLDB_PROPERTIES_symbollocatordebuginfod
+#include "SymbolLocatorDebuginfodProperties.inc"
+
+enum {
+#define LLDB_PROPERTIES_symbollocatordebuginfod
+#include "SymbolLocatorDebuginfodPropertiesEnum.inc"
+};
+
+class PluginProperties : public Properties {
+public:
+ static llvm::StringRef GetSettingName() {
+return SymbolLocatorDebuginfod::GetPluginNameStatic();
+ }
+
+ PluginProperties() {
+m_collection_sp =
std::make_shared(GetSettingName());
+m_collection_sp->Initialize(g_symbollocatordebuginfod_properties);
+m_collection_sp->SetValueChangedCallback(
+ePropertyURLs, [this] { URLsChangedCallback(); }
+);
+ }
+
+ Args GetDebugInfoDURLs() const {
+Args urls;
+m_collection_sp->GetPropertyAtIndexAsArgs(ePropertyURLs, urls);
+return urls;
+ }
+
+private:
+ void URLsChangedCallback() {
+Args urls = GetDebugInfoDURLs();
+llvm::SmallVector dbginfod_urls;
+llvm::transform(urls, dbginfod_urls.end(),
+[](const auto ) { return obj.ref(); });
+llvm::setDefaultDebuginfodUrls(dbginfod_urls);
+ }
+};
+
+} // namespace
+
+
+SymbolLocatorDebuginfod::SymbolLocatorDebuginfod() : SymbolLocator() {}
+
+void SymbolLocatorDebuginfod::Initialize() {
+ PluginManager::RegisterPlugin(
+ GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance,
+ LocateExecutableObjectFile, LocateExecutableSymbolFile,
+ DownloadObjectAndSymbolFile);
+ // There's a "safety" concern on this:
+ // Does plugin initialization occur while things are still single threaded?
clayborg wrote:
This Initialize() function will get called when
`lldb::SBDebugger::Initialize()` is called for the first time.
`lldb::SBDebugger::Initialize()` and `lldb::SBDebugger::Terminate()` are ref
counted, and only the first call will call `lldb::SBDebugger::Initialize()`.
Since LLDB is a shared library, we shouldn't do any work unless clients ask for
it. So it really depends on what program is loading LLDB. Command line lldb
will init on the main thread right away, so will `lldb-vscode`/`lldb-dap` (VS
Code IDE), so will `lldb-rpc-server` (Xcode), not sure about Android Studio or
other IDEs
https://github.com/llvm/llvm-project/pull/70996
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
@@ -0,0 +1,154 @@ +//===-- SymbolLocatorDebuginfod.cpp --===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "SymbolLocatorDebuginfod.h" + +#include +#include + +#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h" +#include "lldb/Core/Debugger.h" +#include "lldb/Core/Module.h" +#include "lldb/Core/ModuleList.h" +#include "lldb/Core/ModuleSpec.h" +#include "lldb/Core/PluginManager.h" +#include "lldb/Core/Progress.h" +#include "lldb/Core/Section.h" +#include "lldb/Host/FileSystem.h" +#include "lldb/Host/Host.h" +#include "lldb/Symbol/ObjectFile.h" +#include "lldb/Target/Target.h" +#include "lldb/Utility/ArchSpec.h" +#include "lldb/Utility/DataBuffer.h" +#include "lldb/Utility/DataExtractor.h" +#include "lldb/Utility/LLDBLog.h" +#include "lldb/Utility/Log.h" +#include "lldb/Utility/StreamString.h" +#include "lldb/Utility/Timer.h" +#include "lldb/Utility/UUID.h" clayborg wrote: Can you try compiling this file with each of these renoved? I am guessing this include list is a bit eager and was from a copy paste? https://github.com/llvm/llvm-project/pull/70996 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
@@ -4892,6 +4894,21 @@ void TargetProperties::SetDebugUtilityExpression(bool
debug) {
SetPropertyAtIndex(idx, debug);
}
+Args TargetProperties::GetDebugInfoDURLs() const {
+ Args urls;
+ m_collection_sp->GetPropertyAtIndexAsArgs(ePropertyDebugInfoDURLs, urls);
+ return urls;
+}
+
+void TargetProperties::DebugInfoDURLsChangedCallback() {
+ Args urls = GetDebugInfoDURLs();
+ llvm::SmallVector dbginfod_urls;
+ std::transform(urls.begin(), urls.end(), dbginfod_urls.end(),
+ [](const auto ) { return obj.ref(); });
+ llvm::setDefaultDebuginfodUrls(dbginfod_urls);
kevinfrei wrote:
It's now a SymbolLocator plugin, so the setting is for the plugin, and follows
that model of settings. I *think* this works the way you described.
https://github.com/llvm/llvm-project/pull/70996
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
@@ -396,8 +398,22 @@ Symbols::LocateExecutableSymbolFile(const ModuleSpec
_spec,
}
}
}
-
- return LocateExecutableSymbolFileDsym(module_spec);
+ FileSpec dsym_bundle = LocateExecutableSymbolFileDsym(module_spec);
+ if (dsym_bundle)
+return dsym_bundle;
+
+ // If we didn't find anything by looking locally, let's try Debuginfod.
+ if (module_uuid.IsValid() && llvm::canUseDebuginfod()) {
+llvm::object::BuildID build_id(module_uuid.GetBytes());
+llvm::Expected result =
+llvm::getCachedOrDownloadDebuginfo(build_id);
+if (result)
+ return FileSpec(*result);
+// An error is just fine, here...
+consumeError(result.takeError());
kevinfrei wrote:
I'm going to add better diagnostics to the llvm library instead, thus I didn't
make a change to this in the updated diff...
https://github.com/llvm/llvm-project/pull/70996
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
kevinfrei wrote: I updated the diff with the plugin-ified version of the work. It's *much* cleaner with no debugger-wide changes to speak of (Thanks for the set up for that, @JDevlieghere!). I did *not* add Debuginfod logging in the LLDB part of the code, as I intend to add diagnostic debugging to the llvm-debuginfod library instead, such that all users will benefit from it. https://github.com/llvm/llvm-project/pull/70996 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Fix size in bytes of type DIEs when size in bits is not a multiple of 8 (PR #69741)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 5195d458587b02f3edf1347fae53417a74538648
b34b8f1786a6fb274710c5e4318bad83b04b0480 --
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
``
View the diff from clang-format here.
``diff
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index aab1dc218973..9633e3b19ec0 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -697,7 +697,6 @@ getMostAppropriateRepresentationAndSize(uint64_t
SizeInBits) {
return {dwarf::DW_AT_bit_size, SizeInBits};
}
-
void DwarfUnit::constructTypeDIE(DIE , const DIBasicType *BTy) {
// Get core information.
StringRef Name = BTy->getName();
``
https://github.com/llvm/llvm-project/pull/69741
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] DEBUGINFOD based DWP acquisition for LLDB (PR #70996)
https://github.com/kevinfrei updated
https://github.com/llvm/llvm-project/pull/70996
>From b04c85dbed0b369e747aa2a3823789203156736b Mon Sep 17 00:00:00 2001
From: Kevin Frei
Date: Wed, 18 Oct 2023 14:37:34 -0700
Subject: [PATCH 1/4] DEBUGINFOD based DWP acquisition for LLDB
Summary:
I've plumbed the LLVM DebugInfoD client into LLDB, and added automatic
downloading of
DWP files to the SymbolFileDWARF.cpp plugin. If you have `DEBUGINFOD_URLS` set
to a
space delimited set of web servers, LLDB will try to use them as a last resort
when
searching for DWP files. If you do *not* have that environment variable set,
nothing
should be changed. There's also a setting, per Greg Clayton's request, that will
override the env variable, or can be used instead of the env var. This setting
is the
reason for the additional API added to the llvm's Debuginfod library.
Test Plan:
Suggestions are welcome here. I should probably have some positive and negative
tests,
but I wanted to get the diff up for people who have a clue what they're doing
to rip it
to pieces before spending too much time validating my implementation.
---
lldb/include/lldb/Target/Target.h | 3 +++
lldb/source/Core/CoreProperties.td| 2 +-
lldb/source/Core/Debugger.cpp | 5
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 1 +
lldb/source/Symbol/CMakeLists.txt | 1 +
lldb/source/Target/Target.cpp | 19 +-
lldb/source/Target/TargetProperties.td| 4 +++
llvm/include/llvm/Debuginfod/Debuginfod.h | 4 +++
llvm/lib/Debuginfod/Debuginfod.cpp| 26 ++-
9 files changed, 57 insertions(+), 8 deletions(-)
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index c37682e2a03859f..7f10f0409fb1315 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -258,6 +258,8 @@ class TargetProperties : public Properties {
bool GetDebugUtilityExpression() const;
+ Args GetDebugInfoDURLs() const;
+
private:
// Callbacks for m_launch_info.
void Arg0ValueChangedCallback();
@@ -270,6 +272,7 @@ class TargetProperties : public Properties {
void DisableASLRValueChangedCallback();
void InheritTCCValueChangedCallback();
void DisableSTDIOValueChangedCallback();
+ void DebugInfoDURLsChangedCallback();
// Settings checker for target.jit-save-objects-dir:
void CheckJITObjectsDir();
diff --git a/lldb/source/Core/CoreProperties.td
b/lldb/source/Core/CoreProperties.td
index 92884258347e9be..865030b0133bbb2 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -4,7 +4,7 @@ let Definition = "modulelist" in {
def EnableExternalLookup: Property<"enable-external-lookup", "Boolean">,
Global,
DefaultTrue,
-Desc<"Control the use of external tools and repositories to locate symbol
files. Directories listed in target.debug-file-search-paths and directory of
the executable are always checked first for separate debug info files. Then
depending on this setting: On macOS, Spotlight would be also used to locate a
matching .dSYM bundle based on the UUID of the executable. On NetBSD, directory
/usr/libdata/debug would be also searched. On platforms other than NetBSD
directory /usr/lib/debug would be also searched.">;
+Desc<"Control the use of external tools and repositories to locate symbol
files. Directories listed in target.debug-file-search-paths and directory of
the executable are always checked first for separate debug info files. Then
depending on this setting: On macOS, Spotlight would be also used to locate a
matching .dSYM bundle based on the UUID of the executable. On NetBSD, directory
/usr/libdata/debug would be also searched. On platforms other than NetBSD
directory /usr/lib/debug would be also searched. If all other methods fail, and
the DEBUGINFOD_URLS environment variable is specified, the Debuginfod protocol
is used to acquire symbols from a compatible Debuginfod service.">;
def EnableBackgroundLookup: Property<"enable-background-lookup", "Boolean">,
Global,
DefaultFalse,
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 21f71e449ca5ed0..9a3e82f3e6a2adf 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -61,6 +61,8 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/iterator.h"
+#include "llvm/Debuginfod/Debuginfod.h"
+#include "llvm/Debuginfod/HTTPClient.h"
#include "llvm/Support/DynamicLibrary.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Process.h"
@@ -594,6 +596,9 @@ lldb::DWIMPrintVerbosity Debugger::GetDWIMPrintVerbosity()
const {
void Debugger::Initialize(LoadPluginCallbackType load_plugin_callback) {
assert(g_debugger_list_ptr == nullptr &&
"Debugger::Initialize called more than once!");
+ // We might be using the Debuginfod service,
[Lldb-commits] [llvm] [lldb] Fix size in bytes of type DIEs when size in bits is not a multiple of 8 (PR #69741)
augusto2112 wrote: @dwblaikie I talked with Adrian, his idea is to emit the size in bits whenever it does not cleanly fit in a byte. His point (and I agree) is that there may be tools that can use this bit size, and if we always round up when emitting the DWARF we will lose this information, with no way to recover it. I updated the patch to implement this behavior. https://github.com/llvm/llvm-project/pull/69741 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [lldb] Fix size in bytes of type DIEs when size in bits is not a multiple of 8 (PR #69741)
https://github.com/augusto2112 edited https://github.com/llvm/llvm-project/pull/69741 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [lldb] Fix size in bytes of type DIEs when size in bits is not a multiple of 8 (PR #69741)
https://github.com/augusto2112 updated
https://github.com/llvm/llvm-project/pull/69741
>From b34b8f1786a6fb274710c5e4318bad83b04b0480 Mon Sep 17 00:00:00 2001
From: Augusto Noronha
Date: Fri, 20 Oct 2023 10:18:03 -0700
Subject: [PATCH] Fix size in bytes of type DIEs when size in bits is not a
multiple of 8
The existing code will always round down the size in bits when
calculating the size in bytes (for example, a types with 1-7 bits will
be emitted as 0 bytes long). Fix this by emitting the size in bits if
the size is not aligned to a byte.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 27 ++--
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp | 31 ++---
.../AArch64/non_standard_bit_sizes.ll | 69 +++
3 files changed, 114 insertions(+), 13 deletions(-)
create mode 100644 llvm/test/DebugInfo/AArch64/non_standard_bit_sizes.ll
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 3174c18c97d888c..ddbec7292eb68d6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -298,6 +298,12 @@ ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const
DWARFDIE ) {
byte_size = form_value.Unsigned();
break;
+case DW_AT_bit_size:
+ // Convert the bit size to byte size, and round it up to the minimum
about
+ // of bytes that will fit the bits.
+ byte_size = (form_value.Unsigned() + 7) / 8;
+ break;
+
case DW_AT_byte_stride:
byte_stride = form_value.Unsigned();
break;
@@ -2210,9 +2216,16 @@ bool DWARFASTParserClang::CompleteRecordType(const
DWARFDIE ,
!layout_info.vbase_offsets.empty()) {
if (type)
layout_info.bit_size = type->GetByteSize(nullptr).value_or(0) * 8;
-if (layout_info.bit_size == 0)
- layout_info.bit_size =
- die.GetAttributeValueAsUnsigned(DW_AT_byte_size, 0) * 8;
+if (layout_info.bit_size == 0) {
+ auto byte_size =
+ die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
+
+ if (byte_size != UINT64_MAX)
+layout_info.bit_size = byte_size;
+ else
+layout_info.bit_size =
+die.GetAttributeValueAsUnsigned(DW_AT_bit_size, 0);
+}
clang::CXXRecordDecl *record_decl =
m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
@@ -2866,8 +2879,12 @@ void DWARFASTParserClang::ParseSingleMember(
// Get the parent byte size so we can verify any members will fit
const uint64_t parent_byte_size =
parent_die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
- const uint64_t parent_bit_size =
- parent_byte_size == UINT64_MAX ? UINT64_MAX : parent_byte_size * 8;
+ uint64_t parent_bit_size;
+ if (parent_byte_size != UINT64_MAX)
+parent_bit_size = parent_byte_size * 8;
+ else
+parent_bit_size =
+parent_die.GetAttributeValueAsUnsigned(DW_AT_bit_size, UINT64_MAX);
// FIXME: Remove the workarounds below and make this const.
MemberAttributes attrs(die, parent_die, module_sp);
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index a5960a5d4a09a17..aab1dc218973365 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -687,6 +687,17 @@ std::string DwarfUnit::getParentContextString(const
DIScope *Context) const {
return CS;
}
+/// Returns the most appropriate dwarf size attribute (bits or bytes) and size
+/// to be used with it, given the input size in bits.
+static std::pair
+getMostAppropriateRepresentationAndSize(uint64_t SizeInBits) {
+ if (SizeInBits % 8 == 0) {
+return {dwarf::DW_AT_byte_size, SizeInBits / 8};
+ }
+ return {dwarf::DW_AT_bit_size, SizeInBits};
+}
+
+
void DwarfUnit::constructTypeDIE(DIE , const DIBasicType *BTy) {
// Get core information.
StringRef Name = BTy->getName();
@@ -702,8 +713,9 @@ void DwarfUnit::constructTypeDIE(DIE , const
DIBasicType *BTy) {
addUInt(Buffer, dwarf::DW_AT_encoding, dwarf::DW_FORM_data1,
BTy->getEncoding());
- uint64_t Size = BTy->getSizeInBits() >> 3;
- addUInt(Buffer, dwarf::DW_AT_byte_size, std::nullopt, Size);
+ auto [SizeAttribute, Size] =
+ getMostAppropriateRepresentationAndSize(BTy->getSizeInBits());
+ addUInt(Buffer, SizeAttribute, std::nullopt, Size);
if (BTy->isBigEndian())
addUInt(Buffer, dwarf::DW_AT_endianity, std::nullopt, dwarf::DW_END_big);
@@ -731,8 +743,9 @@ void DwarfUnit::constructTypeDIE(DIE , const
DIStringType *STy) {
DwarfExpr.addExpression(Expr);
addBlock(Buffer, dwarf::DW_AT_string_length, DwarfExpr.finalize());
} else {
-uint64_t Size = STy->getSizeInBits() >> 3;
-addUInt(Buffer, dwarf::DW_AT_byte_size, std::nullopt, Size);
+auto [SizeAttributte, Size] =
+getMostAppropriateRepresentationAndSize(STy->getSizeInBits());
[Lldb-commits] [clang] [lldb] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/71780 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64][Linux] Add register field information for SME's SVCR register (PR #71809)
https://github.com/jasonmolenda approved this pull request. Looks good. https://github.com/llvm/llvm-project/pull/71809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64][Linux] Add field information for the mte_ctrl register (PR #71808)
https://github.com/jasonmolenda approved this pull request. Read through 71809 before this one, both look fine. ;) https://github.com/llvm/llvm-project/pull/71808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)
walter-erquinigo wrote: The changes seem very trivial to me, so please let me know if there's something else I should do or be aware of. https://github.com/llvm/llvm-project/pull/71828 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)
felipepiovezan wrote: > The correct fix seems to also store the underlying storage along with the > accelerator tables in AppleDWARFIndex. This was my initial reaction as well. Is there some blocker to doing this? https://github.com/llvm/llvm-project/pull/71828 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [clang] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 7c7882fcffbfd204f95a3613ce076abf7da294e8
4784e02edb23b23a6e2cabfa3f7eb20c3ee441c4 --
clang/test/CodeGenCXX/debug-info-static-inline-member.cpp
clang/lib/CodeGen/CGDebugInfo.cpp clang/lib/CodeGen/CGDebugInfo.h
clang/test/CodeGenCXX/debug-info-class.cpp
clang/test/CodeGenCXX/debug-info-static-member.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 20d7f1766948..b336c5b8e3ac 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -5617,9 +5617,10 @@ void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) {
// Omit linkage name for variable definitions that represent constants.
// There hasn't been a need from consumers yet to have it attached.
GV.reset(DBuilder.createGlobalVariableExpression(
- TheCU, DeclName, /* LinkageName */ {}, Unit, LineNo, getOrCreateType(T,
Unit),
- true, true, InitExpr, getOrCreateStaticDataMemberDeclarationOrNull(VD),
- TemplateParameters, Align, Annotations));
+ TheCU, DeclName, /* LinkageName */ {}, Unit, LineNo,
+ getOrCreateType(T, Unit), true, true, InitExpr,
+ getOrCreateStaticDataMemberDeclarationOrNull(VD), TemplateParameters,
+ Align, Annotations));
}
void CGDebugInfo::EmitExternalVariable(llvm::GlobalVariable *Var,
``
https://github.com/llvm/llvm-project/pull/71780
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsString (PR #71613)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/71613 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 1486264 - [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsString (#71613)
Author: Alex Langford
Date: 2023-11-09T13:35:35-08:00
New Revision: 1486264d5fad0ef7d76a44f6358390cf41218c7e
URL:
https://github.com/llvm/llvm-project/commit/1486264d5fad0ef7d76a44f6358390cf41218c7e
DIFF:
https://github.com/llvm/llvm-project/commit/1486264d5fad0ef7d76a44f6358390cf41218c7e.diff
LOG: [lldb] Change interface of StructuredData::Array::GetItemAtIndexAsString
(#71613)
This patch changes the interface of
StructuredData::Array::GetItemAtIndexAsString to return a
`std::optional` instead of taking an out parameter.
More generally, this commit serves as proposal that we change all of the
sibling APIs (`GetItemAtIndexAs`) to do the same thing. The reason this
isn't one giant patch is because it is rather unwieldy changing just one
of these, so if this is approved, I will do all of the other ones as
individual follow-ups.
Added:
Modified:
lldb/include/lldb/Utility/StructuredData.h
lldb/source/Breakpoint/Breakpoint.cpp
lldb/source/Breakpoint/BreakpointOptions.cpp
lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
lldb/source/Breakpoint/BreakpointResolverName.cpp
lldb/source/Commands/CommandObjectBreakpoint.cpp
lldb/source/Core/SearchFilter.cpp
lldb/source/Target/DynamicRegisterInfo.cpp
lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
Removed:
diff --git a/lldb/include/lldb/Utility/StructuredData.h
b/lldb/include/lldb/Utility/StructuredData.h
index 279238f9af76e01..6e39bcff2c0af0b 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -247,23 +248,12 @@ class StructuredData {
return success;
}
-bool GetItemAtIndexAsString(size_t idx, llvm::StringRef ) const {
- ObjectSP value_sp = GetItemAtIndex(idx);
- if (value_sp.get()) {
-if (auto string_value = value_sp->GetAsString()) {
- result = string_value->GetValue();
- return true;
-}
+std::optional GetItemAtIndexAsString(size_t idx) const {
+ if (auto item_sp = GetItemAtIndex(idx)) {
+if (auto *string_value = item_sp->GetAsString())
+ return string_value->GetValue();
}
- return false;
-}
-
-bool GetItemAtIndexAsString(size_t idx, llvm::StringRef ,
-llvm::StringRef default_val) const {
- bool success = GetItemAtIndexAsString(idx, result);
- if (!success)
-result = default_val;
- return success;
+ return {};
}
bool GetItemAtIndexAsDictionary(size_t idx, Dictionary *) const {
diff --git a/lldb/source/Breakpoint/Breakpoint.cpp
b/lldb/source/Breakpoint/Breakpoint.cpp
index a94498a23a0fb27..6e6b51b562496c6 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -205,10 +205,9 @@ lldb::BreakpointSP Breakpoint::CreateFromStructuredData(
if (success && names_array) {
size_t num_names = names_array->GetSize();
for (size_t i = 0; i < num_names; i++) {
- llvm::StringRef name;
- Status error;
- success = names_array->GetItemAtIndexAsString(i, name);
- target.AddNameToBreakpoint(result_sp, name.str().c_str(), error);
+ if (std::optional maybe_name =
+ names_array->GetItemAtIndexAsString(i))
+target.AddNameToBreakpoint(result_sp, *maybe_name, error);
}
}
@@ -238,11 +237,10 @@ bool Breakpoint::SerializedBreakpointMatchesNames(
size_t num_names = names_array->GetSize();
for (size_t i = 0; i < num_names; i++) {
-llvm::StringRef name;
-if (names_array->GetItemAtIndexAsString(i, name)) {
- if (llvm::is_contained(names, name))
-return true;
-}
+std::optional maybe_name =
+names_array->GetItemAtIndexAsString(i);
+if (maybe_name && llvm::is_contained(names, *maybe_name))
+ return true;
}
return false;
}
diff --git a/lldb/source/Breakpoint/BreakpointOptions.cpp
b/lldb/source/Breakpoint/BreakpointOptions.cpp
index 268c52341dfed6e..6c6037dd9edd3a4 100644
--- a/lldb/source/Breakpoint/BreakpointOptions.cpp
+++ b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -88,10 +88,9 @@ BreakpointOptions::CommandData::CreateFromStructuredData(
if (success) {
size_t num_elems = user_source->GetSize();
for (size_t i = 0; i < num_elems; i++) {
- llvm::StringRef elem_string;
- success = user_source->GetItemAtIndexAsString(i, elem_string);
- if (success)
-data_up->user_source.AppendString(elem_string);
+ if (std::optional maybe_elem_string =
+ user_source->GetItemAtIndexAsString(i))
+data_up->user_source.AppendString(*maybe_elem_string);
}
}
diff --git a/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
b/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
index
[Lldb-commits] [lldb] [lldb] Change Breakpoint::AddName return value (PR #71236)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/71236 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] fe98cce - [lldb] Change Breakpoint::AddName return value (#71236)
Author: Alex Langford
Date: 2023-11-09T13:34:59-08:00
New Revision: fe98cce6a91c7240a541b213d43d1132c684fd9c
URL:
https://github.com/llvm/llvm-project/commit/fe98cce6a91c7240a541b213d43d1132c684fd9c
DIFF:
https://github.com/llvm/llvm-project/commit/fe98cce6a91c7240a541b213d43d1132c684fd9c.diff
LOG: [lldb] Change Breakpoint::AddName return value (#71236)
The return value is completely unused. Let's just return nothing.
Added:
Modified:
lldb/include/lldb/Breakpoint/Breakpoint.h
lldb/source/Breakpoint/Breakpoint.cpp
Removed:
diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h
b/lldb/include/lldb/Breakpoint/Breakpoint.h
index f2380157f111dd6..3a8b29aee544c63 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -523,9 +523,8 @@ class Breakpoint : public
std::enable_shared_from_this,
lldb::SearchFilterSP GetSearchFilter() { return m_filter_sp; }
-private: // The target needs to manage adding & removing names. It will do the
- // checking for name validity as well.
- bool AddName(llvm::StringRef new_name);
+private:
+ void AddName(llvm::StringRef new_name);
void RemoveName(const char *name_to_remove) {
if (name_to_remove)
diff --git a/lldb/source/Breakpoint/Breakpoint.cpp
b/lldb/source/Breakpoint/Breakpoint.cpp
index ff4f195ea30909e..a94498a23a0fb27 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -841,9 +841,8 @@ bool Breakpoint::HasResolvedLocations() const {
size_t Breakpoint::GetNumLocations() const { return m_locations.GetSize(); }
-bool Breakpoint::AddName(llvm::StringRef new_name) {
+void Breakpoint::AddName(llvm::StringRef new_name) {
m_name_list.insert(new_name.str());
- return true;
}
void Breakpoint::GetDescription(Stream *s, lldb::DescriptionLevel level,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [clang] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/71780 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reland "[lldb][DWARFASTParserClang] Fetch constant value from variable defintion if available" (PR #71800)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/71800
>From d3e0391685605491383114f82d906c811f899b5d Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Mon, 6 Nov 2023 11:34:09 +
Subject: [PATCH 1/2] Reland "[lldb][DWARFASTParserClang] Fetch constant value
from variable defintion if available"
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 63 ++-
.../SymbolFile/DWARF/DWARFASTParserClang.h| 11
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 7 +--
.../SymbolFile/DWARF/SymbolFileDWARF.h| 10 +--
.../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 7 +++
.../SymbolFile/DWARF/SymbolFileDWARFDwo.h | 5 ++
.../TestConstStaticIntegralMember.py | 53
.../cpp/const_static_integral_member/main.cpp | 20 ++
8 files changed, 166 insertions(+), 10 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 3174c18c97d888c..4e41858674467a3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -31,6 +31,7 @@
#include "lldb/Symbol/SymbolFile.h"
#include "lldb/Symbol/TypeList.h"
#include "lldb/Symbol/TypeMap.h"
+#include "lldb/Symbol/VariableList.h"
#include "lldb/Target/Language.h"
#include "lldb/Utility/LLDBAssert.h"
#include "lldb/Utility/Log.h"
@@ -133,6 +134,54 @@ static lldb::ModuleSP GetContainingClangModule(const
DWARFDIE ) {
return lldb::ModuleSP();
}
+std::optional
+DWARFASTParserClang::FindConstantOnVariableDefinition(DWARFDIE die) {
+ assert(die.Tag() == llvm::dwarf::DW_TAG_member);
+
+ auto *dwarf = die.GetDWARF();
+ if (!dwarf)
+return {};
+
+ ConstString name{die.GetName()};
+ if (!name)
+return {};
+
+ auto *CU = die.GetCU();
+ if (!CU)
+return {};
+
+ DWARFASTParser *dwarf_ast = dwarf->GetDWARFParser(*CU);
+ auto parent_decl_ctx = dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
+
+ // Make sure we populate the GetDieToVariable cache.
+ VariableList variables;
+ dwarf->FindGlobalVariables(name, parent_decl_ctx, UINT_MAX, variables);
+
+ // The cache contains the variable definition whose DW_AT_specification
+ // points to our declaration DIE. Look up that definition using our
+ // declaration.
+ auto const _to_var = dwarf->GetDIEToVariable();
+ auto it = die_to_var.find(die.GetDIE());
+ if (it == die_to_var.end())
+return {};
+
+ auto var_sp = it->getSecond();
+ assert(var_sp != nullptr);
+
+ if (!var_sp->GetLocationIsConstantValueData())
+return {};
+
+ auto def = dwarf->GetDIE(var_sp->GetID());
+ auto def_attrs = def.GetAttributes();
+ DWARFFormValue form_value;
+ if (!def_attrs.ExtractFormValueAtIndex(
+ def_attrs.FindAttributeIndex(llvm::dwarf::DW_AT_const_value),
+ form_value))
+return {};
+
+ return form_value;
+}
+
TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext ,
const DWARFDIE ,
Log *log) {
@@ -2906,9 +2955,21 @@ void DWARFASTParserClang::ParseSingleMember(
bool unused;
// TODO: Support float/double static members as well.
- if (!attrs.const_value_form || !ct.IsIntegerOrEnumerationType(unused))
+ if (!ct.IsIntegerOrEnumerationType(unused))
return;
+ // Newer versions of Clang don't emit the DW_AT_const_value
+ // on the declaration of an inline static data member. Instead
+ // it's attached to the definition DIE. If that's the case,
+ // try and fetch it.
+ if (!attrs.const_value_form) {
+auto maybe_form_value = FindConstantOnVariableDefinition(die);
+if (!maybe_form_value)
+ return;
+
+attrs.const_value_form = *maybe_form_value;
+ }
+
llvm::Expected const_value_or_err =
ExtractIntFromFormValue(ct, *attrs.const_value_form);
if (!const_value_or_err) {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index c381c58fba74263..21fd6f9d7980efc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -373,6 +373,17 @@ class DWARFASTParserClang : public
lldb_private::plugin::dwarf::DWARFASTParser {
lldb_private::CompilerType _clang_type,
const lldb::AccessType default_accesibility,
lldb_private::ClangASTImporter::LayoutInfo
_info);
+
+ /// Tries to find the definition DW_TAG_variable DIE of the the specified
+ /// DW_TAG_member 'die'. If such definition exists, returns the
+ /// DW_AT_const_value of that definition if available. Returns std::nullopt
+ /// otherwise.
+ ///
+ /// In newer versions of clang, DW_AT_const_value
[Lldb-commits] [lldb] [clang] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/71780
>From e5bc858c35b479d29174c9945c6c67f4d2dc085b Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 9 Nov 2023 01:13:21 +
Subject: [PATCH 1/4] Reland "[clang][DebugInfo] Emit global variable
definitions for static data members with constant initializers"
---
clang/lib/CodeGen/CGDebugInfo.cpp | 58 +++
clang/lib/CodeGen/CGDebugInfo.h | 6 ++
clang/test/CodeGenCXX/debug-info-class.cpp| 13 +++--
.../CodeGenCXX/debug-info-static-member.cpp | 52 ++---
.../TestConstStaticIntegralMember.py | 7 ++-
.../lldb-dap/variables/TestDAP_variables.py | 9 +--
6 files changed, 103 insertions(+), 42 deletions(-)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 84a166d3ac3659c..245f7516640d098 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1677,22 +1677,13 @@ CGDebugInfo::CreateRecordStaticField(const VarDecl
*Var, llvm::DIType *RecordTy,
unsigned LineNumber = getLineNumber(Var->getLocation());
StringRef VName = Var->getName();
- llvm::Constant *C = nullptr;
- if (Var->getInit()) {
-const APValue *Value = Var->evaluateValue();
-if (Value) {
- if (Value->isInt())
-C = llvm::ConstantInt::get(CGM.getLLVMContext(), Value->getInt());
- if (Value->isFloat())
-C = llvm::ConstantFP::get(CGM.getLLVMContext(), Value->getFloat());
-}
- }
llvm::DINode::DIFlags Flags = getAccessFlag(Var->getAccess(), RD);
auto Align = getDeclAlignIfRequired(Var, CGM.getContext());
llvm::DIDerivedType *GV = DBuilder.createStaticMemberType(
- RecordTy, VName, VUnit, LineNumber, VTy, Flags, C, Align);
+ RecordTy, VName, VUnit, LineNumber, VTy, Flags, /* Val */ nullptr,
Align);
StaticDataMemberCache[Var->getCanonicalDecl()].reset(GV);
+ StaticDataMemberDefinitionsToEmit.push_back(Var->getCanonicalDecl());
return GV;
}
@@ -5596,6 +5587,39 @@ void CGDebugInfo::EmitGlobalVariable(const ValueDecl
*VD, const APValue ) {
TemplateParameters, Align));
}
+void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) {
+ assert(VD->hasInit());
+ assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
+ if (VD->hasAttr())
+return;
+
+ auto = DeclCache[VD];
+ if (GV)
+return;
+
+ auto const *InitVal = VD->evaluateValue();
+ if (!InitVal)
+return;
+
+ llvm::DIFile *Unit = nullptr;
+ llvm::DIScope *DContext = nullptr;
+ unsigned LineNo;
+ StringRef DeclName, LinkageName;
+ QualType T;
+ llvm::MDTuple *TemplateParameters = nullptr;
+ collectVarDeclProps(VD, Unit, LineNo, T, DeclName, LinkageName,
+ TemplateParameters, DContext);
+
+ auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
+ llvm::DINodeArray Annotations = CollectBTFDeclTagAnnotations(VD);
+ llvm::DIExpression *InitExpr = createConstantValueExpression(VD, *InitVal);
+
+ GV.reset(DBuilder.createGlobalVariableExpression(
+ TheCU, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T, Unit),
+ true, true, InitExpr, getOrCreateStaticDataMemberDeclarationOrNull(VD),
+ TemplateParameters, Align, Annotations));
+}
+
void CGDebugInfo::EmitExternalVariable(llvm::GlobalVariable *Var,
const VarDecl *D) {
assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
@@ -5800,6 +5824,18 @@ void CGDebugInfo::setDwoId(uint64_t Signature) {
}
void CGDebugInfo::finalize() {
+ for (auto const *VD : StaticDataMemberDefinitionsToEmit) {
+assert(VD->isStaticDataMember());
+
+if (DeclCache.contains(VD))
+ continue;
+
+if (!VD->hasInit())
+ continue;
+
+EmitGlobalVariable(VD);
+ }
+
// Creating types might create further types - invalidating the current
// element and the size(), so don't cache/reference them.
for (size_t i = 0; i != ObjCInterfaceCache.size(); ++i) {
diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 7b60e94555d0608..3e4c133b7f2b9f1 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -161,6 +161,9 @@ class CGDebugInfo {
llvm::DenseMap>
StaticDataMemberCache;
+ /// Keeps track of static data members for which we should emit a definition.
+ std::vector StaticDataMemberDefinitionsToEmit;
+
using ParamDecl2StmtTy = llvm::DenseMap;
using Param2DILocTy =
llvm::DenseMap;
@@ -526,6 +529,9 @@ class CGDebugInfo {
/// Emit a constant global variable's debug info.
void EmitGlobalVariable(const ValueDecl *VD, const APValue );
+ /// Emit debug-info for a variable with a constant initializer.
+ void EmitGlobalVariable(const VarDecl *VD);
+
/// Emit information about an external variable.
void EmitExternalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl);
diff --git a/clang/test/CodeGenCXX/debug-info-class.cpp
[Lldb-commits] [lldb] [clang] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/71780
>From e5bc858c35b479d29174c9945c6c67f4d2dc085b Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 9 Nov 2023 01:13:21 +
Subject: [PATCH 1/3] Reland "[clang][DebugInfo] Emit global variable
definitions for static data members with constant initializers"
---
clang/lib/CodeGen/CGDebugInfo.cpp | 58 +++
clang/lib/CodeGen/CGDebugInfo.h | 6 ++
clang/test/CodeGenCXX/debug-info-class.cpp| 13 +++--
.../CodeGenCXX/debug-info-static-member.cpp | 52 ++---
.../TestConstStaticIntegralMember.py | 7 ++-
.../lldb-dap/variables/TestDAP_variables.py | 9 +--
6 files changed, 103 insertions(+), 42 deletions(-)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 84a166d3ac3659c..245f7516640d098 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1677,22 +1677,13 @@ CGDebugInfo::CreateRecordStaticField(const VarDecl
*Var, llvm::DIType *RecordTy,
unsigned LineNumber = getLineNumber(Var->getLocation());
StringRef VName = Var->getName();
- llvm::Constant *C = nullptr;
- if (Var->getInit()) {
-const APValue *Value = Var->evaluateValue();
-if (Value) {
- if (Value->isInt())
-C = llvm::ConstantInt::get(CGM.getLLVMContext(), Value->getInt());
- if (Value->isFloat())
-C = llvm::ConstantFP::get(CGM.getLLVMContext(), Value->getFloat());
-}
- }
llvm::DINode::DIFlags Flags = getAccessFlag(Var->getAccess(), RD);
auto Align = getDeclAlignIfRequired(Var, CGM.getContext());
llvm::DIDerivedType *GV = DBuilder.createStaticMemberType(
- RecordTy, VName, VUnit, LineNumber, VTy, Flags, C, Align);
+ RecordTy, VName, VUnit, LineNumber, VTy, Flags, /* Val */ nullptr,
Align);
StaticDataMemberCache[Var->getCanonicalDecl()].reset(GV);
+ StaticDataMemberDefinitionsToEmit.push_back(Var->getCanonicalDecl());
return GV;
}
@@ -5596,6 +5587,39 @@ void CGDebugInfo::EmitGlobalVariable(const ValueDecl
*VD, const APValue ) {
TemplateParameters, Align));
}
+void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) {
+ assert(VD->hasInit());
+ assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
+ if (VD->hasAttr())
+return;
+
+ auto = DeclCache[VD];
+ if (GV)
+return;
+
+ auto const *InitVal = VD->evaluateValue();
+ if (!InitVal)
+return;
+
+ llvm::DIFile *Unit = nullptr;
+ llvm::DIScope *DContext = nullptr;
+ unsigned LineNo;
+ StringRef DeclName, LinkageName;
+ QualType T;
+ llvm::MDTuple *TemplateParameters = nullptr;
+ collectVarDeclProps(VD, Unit, LineNo, T, DeclName, LinkageName,
+ TemplateParameters, DContext);
+
+ auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
+ llvm::DINodeArray Annotations = CollectBTFDeclTagAnnotations(VD);
+ llvm::DIExpression *InitExpr = createConstantValueExpression(VD, *InitVal);
+
+ GV.reset(DBuilder.createGlobalVariableExpression(
+ TheCU, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T, Unit),
+ true, true, InitExpr, getOrCreateStaticDataMemberDeclarationOrNull(VD),
+ TemplateParameters, Align, Annotations));
+}
+
void CGDebugInfo::EmitExternalVariable(llvm::GlobalVariable *Var,
const VarDecl *D) {
assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
@@ -5800,6 +5824,18 @@ void CGDebugInfo::setDwoId(uint64_t Signature) {
}
void CGDebugInfo::finalize() {
+ for (auto const *VD : StaticDataMemberDefinitionsToEmit) {
+assert(VD->isStaticDataMember());
+
+if (DeclCache.contains(VD))
+ continue;
+
+if (!VD->hasInit())
+ continue;
+
+EmitGlobalVariable(VD);
+ }
+
// Creating types might create further types - invalidating the current
// element and the size(), so don't cache/reference them.
for (size_t i = 0; i != ObjCInterfaceCache.size(); ++i) {
diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 7b60e94555d0608..3e4c133b7f2b9f1 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -161,6 +161,9 @@ class CGDebugInfo {
llvm::DenseMap>
StaticDataMemberCache;
+ /// Keeps track of static data members for which we should emit a definition.
+ std::vector StaticDataMemberDefinitionsToEmit;
+
using ParamDecl2StmtTy = llvm::DenseMap;
using Param2DILocTy =
llvm::DenseMap;
@@ -526,6 +529,9 @@ class CGDebugInfo {
/// Emit a constant global variable's debug info.
void EmitGlobalVariable(const ValueDecl *VD, const APValue );
+ /// Emit debug-info for a variable with a constant initializer.
+ void EmitGlobalVariable(const VarDecl *VD);
+
/// Emit information about an external variable.
void EmitExternalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl);
diff --git a/clang/test/CodeGenCXX/debug-info-class.cpp
[Lldb-commits] [lldb] [lldb-dap] Add an option to show function args in stack frames (PR #71843)
@@ -324,8 +325,23 @@ class StackFrame : public ExecutionContextScope, ///C string with the assembly instructions for this function. const char *Disassemble(); + /// Print a description of this frame using the provided frame format. + /// If the format is invalid, then the default formatter will be used (see \a + /// StackFrame::Dump()), in which case \b false is returned. Otherwise, \b + /// true is returned. + /// + /// \param[in] strm bulbazord wrote: This is an out parameter no? https://github.com/llvm/llvm-project/pull/71843 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add an option to show function args in stack frames (PR #71843)
@@ -1232,6 +1234,32 @@ const char *SBFrame::GetFunctionName() const {
return name;
}
+void SBFrame::GetDisplayFunctionNameWithArgs(SBStream ) {
+ Stream = output.ref();
+
+ std::unique_lock lock;
+ ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
+
+ StackFrame *frame = nullptr;
+ Target *target = exe_ctx.GetTargetPtr();
+ Process *process = exe_ctx.GetProcessPtr();
+
+ if (target && process) {
+Process::StopLocker stop_locker;
+if (stop_locker.TryLock(>GetRunLock())) {
+ frame = exe_ctx.GetFramePtr();
+ if (frame) {
+FormatEntity::Entry format;
+Status s = FormatEntity::Parse("${function.name-with-args}", format);
+assert(
+s.Success() &&
+"The ${function.name-with-args} format must be parsed correctly");
bulbazord wrote:
I don't think this should be an assertion. If something internally went wrong
with LLDB, we shouldn't crash. This might be a good place to return an SBError
https://github.com/llvm/llvm-project/pull/71843
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add an option to show function args in stack frames (PR #71843)
@@ -87,8 +87,15 @@ class LLDB_API SBFrame {
// display to a user
const char *GetDisplayFunctionName();
+ /// Similar to \a GetDisplayFunctionName() but with function arguments and
+ /// their values inserted into the function display name whenever possible.
+ ///
+ /// \param[in] output
bulbazord wrote:
this should be `\param[out]`
https://github.com/llvm/llvm-project/pull/71843
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add an option to show function args in stack frames (PR #71843)
@@ -187,3 +188,19 @@ def test_stackTrace(self): self.assertEquals( 0, len(stackFrames), "verify zero frames with startFrame out of bounds" ) + +@skipIfWindows bulbazord wrote: Why skip on Windows? https://github.com/llvm/llvm-project/pull/71843 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add an option to show function args in stack frames (PR #71843)
@@ -87,8 +87,15 @@ class LLDB_API SBFrame {
// display to a user
const char *GetDisplayFunctionName();
+ /// Similar to \a GetDisplayFunctionName() but with function arguments and
+ /// their values inserted into the function display name whenever possible.
+ ///
+ /// \param[in] output
+ /// The stream where the display name is written to.
+ void GetDisplayFunctionNameWithArgs(SBStream );
bulbazord wrote:
Should this return an `SBError`? How do I know what went wrong if this fails?
https://github.com/llvm/llvm-project/pull/71843
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Read Checksum from DWARF line tables (PR #71458)
dyung wrote: > That would've been helpful information to include in your original message > and saved me the time of having to go through the logs. Sorry about that, I'll be sure to do so next time. https://github.com/llvm/llvm-project/pull/71458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Read Checksum from DWARF line tables (PR #71458)
JDevlieghere wrote: That would've been helpful information to include in your original message and saved me the time of having to go through the logs. https://github.com/llvm/llvm-project/pull/71458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Read Checksum from DWARF line tables (PR #71458)
dyung wrote: For completeness, here is where I forced a build of the commit immediately preceding yours: https://lab.llvm.org/buildbot/#/builders/217/builds/30986 Note the git checkout step: ``` ... using PTY: False 2984156fd34a6969c7461b228d90b72711e3797c program finished with exit code 0 ... ``` That run completed successfully. Then I forced a run with only your changes in https://lab.llvm.org/buildbot/#/builders/217/builds/30987 git checkout output: ``` ... using PTY: False 5da98dec7ab8c3adc3f70147ea666428431adf34 program finished with exit code 0 ... ``` That run had 59 failures. https://github.com/llvm/llvm-project/pull/71458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
jasonmolenda wrote: > @jasonmolenda I was wondering if we should modify > `GetCoreFileSaveRangesDirtyOnly(...)` to try and add all dirty pages and see > if any regions have the dirty page info, but if no memory region infos have > the dirty pages information, then fall back to adding all memory regions with > `write` access. What do you think? Another good idea. Do we get page permissions like that on Linux? I'm curious what system it would work on. I tried a full memory coredump on darwin and there ARE some regions that are marked `r` or `rx` and yet have a dirty page. Some of them are tagged as malloc-metadata so maybe the page protections were flipped to avoid buggy overwriting by the process or something, I didn't look too closely into them, it was only a dozen pages of memory total. So at least on darwin, if debugserver didn't provide the dirty-pages list for each memory region, it'd probably be indistinguishable by developers who are mostly wanting to get stack+heap with these things. https://github.com/llvm/llvm-project/pull/71772 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Read Checksum from DWARF line tables (PR #71458)
dyung wrote: > It was also passing before with the change, so the issue isn't fully > deterministic: https://lab.llvm.org/buildbot/#/builders/217/builds/30986 That was me forcing the build of the commit immediately preceding yours to verify it was passing: `reason A build was forced by '': Build a particular revision ` https://github.com/llvm/llvm-project/pull/71458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
bulbazord wrote: > I addressed most of the feedback. Alex let me know if you still really want > llvm::Error and llvm::Expected to be used as I can add that if you think it > is required. I also ran clang format. I think your answers make sense to me. I don't think you need to add them here since it all eventually turns into Status objects anyway... I generally prefer it over Status though. Thanks for checking. https://github.com/llvm/llvm-project/pull/71772 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Read Checksum from DWARF line tables (PR #71458)
JDevlieghere wrote: It was also passing before with the change, so the issue isn't fully deterministic: https://lab.llvm.org/buildbot/#/builders/217/builds/30986 https://github.com/llvm/llvm-project/pull/71458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Read Checksum from DWARF line tables (PR #71458)
dyung wrote: > Reverted in #71864. Thanks, they seem to be passing now: https://lab.llvm.org/buildbot/#/builders/243/builds/15146 https://lab.llvm.org/buildbot/#/builders/217/builds/30989 https://github.com/llvm/llvm-project/pull/71458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Read Checksum from DWARF line tables (PR #71458)
dyung wrote: > > Hi @JDevlieghere, this change seems to be causing 59 test failures in the > > cross-project-test suite. Can you take a look? > > https://lab.llvm.org/buildbot/#/builders/217/builds/30979 > > https://lab.llvm.org/buildbot/#/builders/243/builds/15142 > > I'll revert the change, but I doubt this is the cause as this patch should > just be storing a member. However there's nothing in the logs to exonerate me > so let's see if the revert changes anything. I did try building the commit immediately before yours, and then yours and that is how I determined it to be caused by your commit. https://github.com/llvm/llvm-project/pull/71458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Revert "[lldb] Read Checksum from DWARF line tables" (PR #71864)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
Changes
Reverts llvm/llvm-project#71458 as it might have caused
cross-project-test failures.
---
Full diff: https://github.com/llvm/llvm-project/pull/71864.diff
1 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+1-8)
``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 79d44bce3d663b6..aabd83a194932ff 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -229,15 +229,8 @@ ParseSupportFilesFromPrologue(const lldb::ModuleSP ,
remapped_file = std::move(*file_path);
}
-Checksum checksum;
-if (prologue.ContentTypes.HasMD5) {
- const llvm::DWARFDebugLine::FileNameEntry _name_entry =
- prologue.getFileNameEntry(idx);
- checksum = file_name_entry.Checksum;
-}
-
// Unconditionally add an entry, so the indices match up.
-support_files.EmplaceBack(remapped_file, style, checksum);
+support_files.EmplaceBack(remapped_file, style);
}
return support_files;
``
https://github.com/llvm/llvm-project/pull/71864
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Read Checksum from DWARF line tables (PR #71458)
JDevlieghere wrote: Reverted in #71864. https://github.com/llvm/llvm-project/pull/71458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Revert "[lldb] Read Checksum from DWARF line tables" (PR #71864)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/71864 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 73519ba - Revert "[lldb] Read Checksum from DWARF line tables" (#71864)
Author: Jonas Devlieghere
Date: 2023-11-09T12:43:53-08:00
New Revision: 73519ba27a29c7e95ec93ed8ce44ff7874851599
URL:
https://github.com/llvm/llvm-project/commit/73519ba27a29c7e95ec93ed8ce44ff7874851599
DIFF:
https://github.com/llvm/llvm-project/commit/73519ba27a29c7e95ec93ed8ce44ff7874851599.diff
LOG: Revert "[lldb] Read Checksum from DWARF line tables" (#71864)
Reverts llvm/llvm-project#71458 as it might have caused
cross-project-test failures.
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 79d44bce3d663b6..aabd83a194932ff 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -229,15 +229,8 @@ ParseSupportFilesFromPrologue(const lldb::ModuleSP ,
remapped_file = std::move(*file_path);
}
-Checksum checksum;
-if (prologue.ContentTypes.HasMD5) {
- const llvm::DWARFDebugLine::FileNameEntry _name_entry =
- prologue.getFileNameEntry(idx);
- checksum = file_name_entry.Checksum;
-}
-
// Unconditionally add an entry, so the indices match up.
-support_files.EmplaceBack(remapped_file, style, checksum);
+support_files.EmplaceBack(remapped_file, style);
}
return support_files;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Revert "[lldb] Read Checksum from DWARF line tables" (PR #71864)
https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/71864
Reverts llvm/llvm-project#71458 as it might have caused cross-project-test
failures.
>From 5b740aafd3631fbe4bbe56cbe9e206da926a3c06 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Thu, 9 Nov 2023 12:42:28 -0800
Subject: [PATCH] Revert "[lldb] Read Checksum from DWARF line tables (#71458)"
This reverts commit 5da98dec7ab8c3adc3f70147ea666428431adf34.
---
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 79d44bce3d663b6..aabd83a194932ff 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -229,15 +229,8 @@ ParseSupportFilesFromPrologue(const lldb::ModuleSP ,
remapped_file = std::move(*file_path);
}
-Checksum checksum;
-if (prologue.ContentTypes.HasMD5) {
- const llvm::DWARFDebugLine::FileNameEntry _name_entry =
- prologue.getFileNameEntry(idx);
- checksum = file_name_entry.Checksum;
-}
-
// Unconditionally add an entry, so the indices match up.
-support_files.EmplaceBack(remapped_file, style, checksum);
+support_files.EmplaceBack(remapped_file, style);
}
return support_files;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Read Checksum from DWARF line tables (PR #71458)
JDevlieghere wrote: > Hi @JDevlieghere, this change seems to be causing 59 test failures in the > cross-project-test suite. Can you take a look? > > https://lab.llvm.org/buildbot/#/builders/217/builds/30979 > https://lab.llvm.org/buildbot/#/builders/243/builds/15142 I'll revert the change, but I doubt this is the cause as this patch should just be storing a member. However there's nothing in the logs to exonerate me so let's see if the revert changes anything. https://github.com/llvm/llvm-project/pull/71458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Ensure the data of apple accelerator tables is kept around (PR #71828)
https://github.com/bulbazord commented: The idea of this makes sense to me, but I think you should let @felipepiovezan take a look first. https://github.com/llvm/llvm-project/pull/71828 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Read Checksum from DWARF line tables (PR #71458)
dyung wrote: Hi @JDevlieghere, this change seems to be causing 59 test failures in the cross-project-test suite. Can you take a look? https://lab.llvm.org/buildbot/#/builders/217/builds/30979 https://lab.llvm.org/buildbot/#/builders/243/builds/15142 https://github.com/llvm/llvm-project/pull/71458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64][Linux] Add field information for the mte_ctrl register (PR #71808)
https://github.com/bulbazord approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/71808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Colorize output when searching for symbols in lldb (PR #69422)
=?utf-8?q?José?= L. Junior Message-ID: In-Reply-To: taalhaataahir0102 wrote: Hi David! `ninja check-lldb` is failing for us. We've re-compiled the latest llvm project (without adding our implementation) but that one is also giving us the same error:  ``` Testing Time: 88.62s Total Discovered Tests: 2888 Skipped :1 (0.03%) Unsupported : 491 (17.00%) Passed : 1674 (57.96%) Expectedly Failed: 19 (0.66%) Unresolved : 656 (22.71%) Failed : 47 (1.63%) 2 warning(s) in tests FAILED: tools/lldb/test/CMakeFiles/check-lldb /home/talha/llvm-latest/llvm-project-main/build/tools/lldb/test/CMakeFiles/check-lldb cd /home/talha/llvm-latest/llvm-project-main/build/tools/lldb/test && /usr/bin/python3.10 /home/talha/llvm-latest/llvm-project-main/build/./bin/llvm-lit -sv /home/talha/llvm-latest/llvm-project-main/build/tools/lldb/test ninja: build stopped: subcommand failed. ``` We've used the following command for building lldb: `cmake -G Ninja -DLLVM_ENABLE_PROJECTS="lldb;clang" -DLLVM_USE_LINKER=lld -DLLVM_BUILD_TESTS=true -DCMAKE_BUILD_TYPE=Release /home/talha/llvm-latest/llvm-project-main/llvm` Can you please confirm if there's something wrong in our build. Also I've the complete log of ninja `check-lldb` but it's pretty long. Do you need it to figure out the issue? Plus there's one more thing we are not sure of as we are new to github. There have been some updates in a file we've changed in our draft (`source/Commands/CommandObjectTarget.cpp`). So do we need a fresh build and add our implementation there before the final review? As the draft also says that > This branch has conflicts that must be resolved > Conflicting files > lldb/source/Commands/CommandObjectTarget.cpp https://github.com/llvm/llvm-project/pull/69422 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add an option to show function args in stack frames (PR #71843)
@@ -1232,6 +1234,32 @@ const char *SBFrame::GetFunctionName() const {
return name;
}
+void SBFrame::GetDisplayFunctionNameWithArgs(SBStream ) {
+ Stream = output.ref();
+
+ std::unique_lock lock;
+ ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
+
+ StackFrame *frame = nullptr;
+ Target *target = exe_ctx.GetTargetPtr();
+ Process *process = exe_ctx.GetProcessPtr();
+
+ if (target && process) {
+Process::StopLocker stop_locker;
+if (stop_locker.TryLock(>GetRunLock())) {
+ frame = exe_ctx.GetFramePtr();
+ if (frame) {
+FormatEntity::Entry format;
clayborg wrote:
Make this static and add a llvm::once call around the intiailization so we
don't need to parse the format each time we call this function. If we make this
static, rename to `g_format` to indicate it is a global.
https://github.com/llvm/llvm-project/pull/71843
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add an option to show function args in stack frames (PR #71843)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/71843 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add an option to show function args in stack frames (PR #71843)
https://github.com/clayborg requested changes to this pull request.
So this hardcodes the frame display to only be the function name, or function
name with args. Do we want to allow users to specify a frame format in the
lldb-vscode settings? We could have two settings: one to enable the feature
with something like `showFramesWithCustomFormat` and on that contains the
format string like `customFrameFormat` which would default to
`"${function.name-with-args}"`. Then users have complete control over how and
what gets displayed. Some people might want to show the pc value and then the
function name like:
```
"customFrameFormat": "${frame.pc} ${function.name-with-args}"
```
If we want this then we will want to modify the SBFrame changes to be something
like:
```
SBError SBFrame::ApplyFormat(SBStream , const char *format_string);
```
We might even consider makding a `lldb::SBFormat` class that allows us to
compile a format string into an object and then use it. If we do this then the
API above will be:
```
SBError SBFrame::ApplyFormat(SBStream , lldb::SBFormat format);
```
https://github.com/llvm/llvm-project/pull/71843
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
clayborg wrote: @bulbazord let me know if you require any changes after reading my inline comments. https://github.com/llvm/llvm-project/pull/71772 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
clayborg wrote: @jasonmolenda I was wondering if we should modify `GetCoreFileSaveRangesDirtyOnly(...)` to try and add all dirty pages and see if any regions have the dirty page info, but if no memory region infos have the dirty pages information, then fall back to adding all memory regions with `write` access. What do you think? This would allow systems to not support dirty pages, but get a more minimal core file saved out. The other option is to add a new `lldb::SaveCoreStyle` enum like `lldb::SaveCoreStyle::eSaveCoreWriteOnly` to save only sections that have write permissions. Right now we have `eSaveCoreFull` which saves all memory regions that have any valid permissions, `eSaveCoreDirtyOnly` which will currently only save dirty pages if it is supported by the process plugin (ProcessGDBRemove for most people, where `debugserver` supports the page stuff, but `lldb-server` doesn't), and `eSaveCoreStackOnly` which only emits the stacks (where it will use the dirty page info if available and fall back to saving the entire stack range if dirty page support is not available). https://github.com/llvm/llvm-project/pull/71772 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
clayborg wrote: I addressed most of the feedback. Alex let me know if you still really want llvm::Error and llvm::Expected to be used as I can add that if you think it is required. I also ran clang format. https://github.com/llvm/llvm-project/pull/71772 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
https://github.com/clayborg updated
https://github.com/llvm/llvm-project/pull/71772
>From feea395f4ef165dfc057dfdc0649c6948895eeb3 Mon Sep 17 00:00:00 2001
From: Greg Clayton
Date: Wed, 8 Nov 2023 21:14:49 -0800
Subject: [PATCH] Centralize the code that figures out which memory ranges to
save into core files.
Prior to this patch, each core file plugin (ObjectFileMachO.cpp and
ObjectFileMinindump.cpp) would calculate the address ranges to save in
different ways. This patch adds a new function to Process.h/.cpp:
```
Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
CoreFileMemoryRanges );
```
The patch updates the ObjectFileMachO::SaveCore(...) and
ObjectFileMinindump::SaveCore(...) to use same code. This will allow core files
to be consistent with the lldb::SaveCoreStyle across different core file
creators and will allow us to add new core file saving features that do more
complex things in future patches.
---
lldb/include/lldb/Target/MemoryRegionInfo.h | 8 +-
lldb/include/lldb/Target/Process.h| 46 -
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 137 +++--
.../Minidump/MinidumpFileBuilder.cpp | 37 +---
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 3 +-
.../Minidump/ObjectFileMinidump.cpp | 9 +-
lldb/source/Target/Process.cpp| 189 +-
lldb/source/Target/TraceDumper.cpp| 7 +-
8 files changed, 267 insertions(+), 169 deletions(-)
diff --git a/lldb/include/lldb/Target/MemoryRegionInfo.h
b/lldb/include/lldb/Target/MemoryRegionInfo.h
index 47d4c9d6398728c..66a4b3ed1b42d5f 100644
--- a/lldb/include/lldb/Target/MemoryRegionInfo.h
+++ b/lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -81,11 +81,11 @@ class MemoryRegionInfo {
// lldb::Permissions
uint32_t GetLLDBPermissions() const {
uint32_t permissions = 0;
-if (m_read)
+if (m_read == eYes)
permissions |= lldb::ePermissionsReadable;
-if (m_write)
+if (m_write == eYes)
permissions |= lldb::ePermissionsWritable;
-if (m_execute)
+if (m_execute == eYes)
permissions |= lldb::ePermissionsExecutable;
return permissions;
}
@@ -151,7 +151,7 @@ class MemoryRegionInfo {
int m_pagesize = 0;
std::optional> m_dirty_pages;
};
-
+
inline bool operator<(const MemoryRegionInfo ,
const MemoryRegionInfo ) {
return lhs.GetRange() < rhs.GetRange();
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index a6d3e6c2d16926e..08e3c60f7c324e6 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -54,6 +54,7 @@
#include "lldb/Utility/UserIDResolver.h"
#include "lldb/lldb-private.h"
+#include "llvm/ADT/AddressRanges.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/Threading.h"
#include "llvm/Support/VersionTuple.h"
@@ -354,12 +355,10 @@ class Process : public
std::enable_shared_from_this,
};
// This is all the event bits the public process broadcaster broadcasts.
// The process shadow listener signs up for all these bits...
- static constexpr int g_all_event_bits = eBroadcastBitStateChanged
-| eBroadcastBitInterrupt
-| eBroadcastBitSTDOUT
-| eBroadcastBitSTDERR
-| eBroadcastBitProfileData
-| eBroadcastBitStructuredData;
+ static constexpr int g_all_event_bits =
+ eBroadcastBitStateChanged | eBroadcastBitInterrupt | eBroadcastBitSTDOUT
|
+ eBroadcastBitSTDERR | eBroadcastBitProfileData |
+ eBroadcastBitStructuredData;
enum {
eBroadcastInternalStateControlStop = (1 << 0),
@@ -390,7 +389,7 @@ class Process : public
std::enable_shared_from_this,
ConstString () const override {
return GetStaticBroadcasterClass();
}
-
+
/// A notification structure that can be used by clients to listen
/// for changes in a process's lifetime.
///
@@ -579,7 +578,7 @@ class Process : public
std::enable_shared_from_this,
/// of CommandObject like CommandObjectRaw, CommandObjectParsed,
/// or CommandObjectMultiword.
virtual CommandObject *GetPluginCommandObject() { return nullptr; }
-
+
/// The underlying plugin might store the low-level communication history for
/// this session. Dump it into the provided stream.
virtual void DumpPluginHistory(Stream ) { return; }
@@ -614,7 +613,7 @@ class Process : public
std::enable_shared_from_this,
return error;
}
- /// The "ShadowListener" for a process is just an ordinary Listener that
+ /// The "ShadowListener" for a process is just an ordinary Listener that
/// listens for all the Process event bits. It's convenient because you can
/// specify it in the LaunchInfo or AttachInfo, so it will get events from
/// the very start of the process.
[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
@@ -6252,3 +6243,188 @@ Status Process::WriteMemoryTags(lldb::addr_t addr,
size_t len,
return DoWriteMemoryTags(addr, len, tag_manager->GetAllocationTagType(),
*packed_tags);
}
+
+// Create a CoreFileMemoryRange from a MemoryRegionInfo
+static Process::CoreFileMemoryRange
+CreateCoreFileMemoryRange(const MemoryRegionInfo ) {
+ const addr_t addr = region.GetRange().GetRangeBase();
+ llvm::AddressRange range(addr, addr + region.GetRange().GetByteSize());
+ return {range, region.GetLLDBPermissions()};
+}
+
+// Add dirty pages to the core file ranges and return true if dirty pages
+// were added. Return false if the dirty page information is not valid or in
+// the region.
+static bool AddDirtyPages(const MemoryRegionInfo ,
+ Process::CoreFileMemoryRanges ) {
+ const auto _page_list = region.GetDirtyPageList();
+ if (!dirty_page_list)
+return false;
+ const uint32_t lldb_permissions = region.GetLLDBPermissions();
+ const addr_t page_size = region.GetPageSize();
+ if (page_size == 0)
+return false;
+ llvm::AddressRange range(0, 0);
+ for (addr_t page_addr : *dirty_page_list) {
+if (range.empty()) {
+ // No range yet, initialize the range with the current dirty page.
+ range = llvm::AddressRange(page_addr, page_addr + page_size);
+} else {
+ if (range.end() == page_addr) {
+// Combine consective ranges.
+range = llvm::AddressRange(range.start(), page_addr + page_size);
+ } else {
+// Add previous contiguous range and init the new range with the
+// current dirty page.
+ranges.push_back({range, lldb_permissions});
+range = llvm::AddressRange(page_addr, page_addr + page_size);
+ }
+}
+ }
+ // The last range
+ if (!range.empty())
+ranges.push_back({range, lldb_permissions});
+ return true;
+}
+
+// Given a region, add the region to \a ranges.
+//
+// Only add the region if it isn't empty and if it has some permissions.
+// If \a try_dirty_pages is true, then try to add only the dirty pages for a
+// given region. If the region has dirty page information, only dirty pages
+// will be added to \a ranges, else the entire range will be added to \a
+// ranges.
+static void AddRegion(const MemoryRegionInfo , bool try_dirty_pages,
+ Process::CoreFileMemoryRanges ) {
+ // Don't add empty ranges or ranges with no permissions.
+ if (region.GetRange().GetByteSize() == 0 || region.GetLLDBPermissions() == 0)
+return;
+ if (try_dirty_pages && AddDirtyPages(region, ranges))
+return;
+ ranges.push_back(CreateCoreFileMemoryRange(region));
+}
+
+// Save all memory regions that are not empty or have at least some permissions
+// for a full core file style.
+static Status
+GetCoreFileSaveRangesFull(Process ,
+ const MemoryRegionInfos ,
+ Process::CoreFileMemoryRanges ) {
+
+ // Don't add only dirty pages, add full regions.
+ const bool try_dirty_pages = false;
+ for (const auto : regions)
+AddRegion(region, try_dirty_pages, ranges);
+ return Status();
+}
+
+// Save only the dirty pages to the core file. Make sure the process has at
+// least some dirty pages, as some OS versions don't support reporting what
+// pages are dirty within an memory region. If no memory regions have dirty
+// page information, return an error.
+static Status
+GetCoreFileSaveRangesDirtyOnly(Process ,
+ const MemoryRegionInfos ,
+ Process::CoreFileMemoryRanges ) {
+ // Iterate over the regions and find all dirty pages.
+ bool have_dirty_page_info = false;
+ for (const auto : regions) {
+if (AddDirtyPages(region, ranges))
+ have_dirty_page_info = true;
+ }
+
+ if (!have_dirty_page_info)
+return Status("no process memory regions have dirty page information");
+
+ return Status();
+}
+
+// Save all thread stacks to the core file. Some OS versions support reporting
+// when a memory region is stack related. We check on this information, but we
+// also use the stack pointers of each thread and add those in case the OS
+// doesn't support reporting stack memory. This function does unique the stack
+// regions and won't add the same range twice. This function also attempts to
clayborg wrote:
sounds good
https://github.com/llvm/llvm-project/pull/71772
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
@@ -6252,3 +6243,188 @@ Status Process::WriteMemoryTags(lldb::addr_t addr,
size_t len,
return DoWriteMemoryTags(addr, len, tag_manager->GetAllocationTagType(),
*packed_tags);
}
+
+// Create a CoreFileMemoryRange from a MemoryRegionInfo
+static Process::CoreFileMemoryRange
+CreateCoreFileMemoryRange(const MemoryRegionInfo ) {
+ const addr_t addr = region.GetRange().GetRangeBase();
+ llvm::AddressRange range(addr, addr + region.GetRange().GetByteSize());
+ return {range, region.GetLLDBPermissions()};
+}
+
+// Add dirty pages to the core file ranges and return true if dirty pages
+// were added. Return false if the dirty page information is not valid or in
+// the region.
+static bool AddDirtyPages(const MemoryRegionInfo ,
+ Process::CoreFileMemoryRanges ) {
+ const auto _page_list = region.GetDirtyPageList();
+ if (!dirty_page_list)
+return false;
+ const uint32_t lldb_permissions = region.GetLLDBPermissions();
+ const addr_t page_size = region.GetPageSize();
+ if (page_size == 0)
+return false;
+ llvm::AddressRange range(0, 0);
+ for (addr_t page_addr : *dirty_page_list) {
+if (range.empty()) {
+ // No range yet, initialize the range with the current dirty page.
+ range = llvm::AddressRange(page_addr, page_addr + page_size);
+} else {
+ if (range.end() == page_addr) {
+// Combine consective ranges.
+range = llvm::AddressRange(range.start(), page_addr + page_size);
+ } else {
+// Add previous contiguous range and init the new range with the
+// current dirty page.
+ranges.push_back({range, lldb_permissions});
+range = llvm::AddressRange(page_addr, page_addr + page_size);
+ }
+}
+ }
+ // The last range
+ if (!range.empty())
+ranges.push_back({range, lldb_permissions});
+ return true;
+}
+
+// Given a region, add the region to \a ranges.
+//
+// Only add the region if it isn't empty and if it has some permissions.
+// If \a try_dirty_pages is true, then try to add only the dirty pages for a
+// given region. If the region has dirty page information, only dirty pages
+// will be added to \a ranges, else the entire range will be added to \a
+// ranges.
+static void AddRegion(const MemoryRegionInfo , bool try_dirty_pages,
+ Process::CoreFileMemoryRanges ) {
+ // Don't add empty ranges or ranges with no permissions.
+ if (region.GetRange().GetByteSize() == 0 || region.GetLLDBPermissions() == 0)
+return;
+ if (try_dirty_pages && AddDirtyPages(region, ranges))
+return;
+ ranges.push_back(CreateCoreFileMemoryRange(region));
+}
+
+// Save all memory regions that are not empty or have at least some permissions
+// for a full core file style.
+static Status
+GetCoreFileSaveRangesFull(Process ,
+ const MemoryRegionInfos ,
+ Process::CoreFileMemoryRanges ) {
+
+ // Don't add only dirty pages, add full regions.
+ const bool try_dirty_pages = false;
+ for (const auto : regions)
+AddRegion(region, try_dirty_pages, ranges);
+ return Status();
clayborg wrote:
Each of the `GetCoreFileSaveRanges*` funtions all have the same return value to
keep it consistent. When we ask explicitly for dirty pages only with the
`eSaveCoreDirtyOnly` core style, that function returns an error if dirty pages
are not supported by any regions. Not much can go wrong in this function. I can
remove the Status return value if needed.
https://github.com/llvm/llvm-project/pull/71772
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
@@ -6252,3 +6243,188 @@ Status Process::WriteMemoryTags(lldb::addr_t addr,
size_t len,
return DoWriteMemoryTags(addr, len, tag_manager->GetAllocationTagType(),
*packed_tags);
}
+
+// Create a CoreFileMemoryRange from a MemoryRegionInfo
+static Process::CoreFileMemoryRange
+CreateCoreFileMemoryRange(const MemoryRegionInfo ) {
+ const addr_t addr = region.GetRange().GetRangeBase();
+ llvm::AddressRange range(addr, addr + region.GetRange().GetByteSize());
+ return {range, region.GetLLDBPermissions()};
+}
+
+// Add dirty pages to the core file ranges and return true if dirty pages
+// were added. Return false if the dirty page information is not valid or in
+// the region.
+static bool AddDirtyPages(const MemoryRegionInfo ,
+ Process::CoreFileMemoryRanges ) {
+ const auto _page_list = region.GetDirtyPageList();
+ if (!dirty_page_list)
+return false;
+ const uint32_t lldb_permissions = region.GetLLDBPermissions();
+ const addr_t page_size = region.GetPageSize();
+ if (page_size == 0)
+return false;
+ llvm::AddressRange range(0, 0);
+ for (addr_t page_addr : *dirty_page_list) {
+if (range.empty()) {
+ // No range yet, initialize the range with the current dirty page.
+ range = llvm::AddressRange(page_addr, page_addr + page_size);
+} else {
+ if (range.end() == page_addr) {
+// Combine consective ranges.
+range = llvm::AddressRange(range.start(), page_addr + page_size);
+ } else {
+// Add previous contiguous range and init the new range with the
+// current dirty page.
+ranges.push_back({range, lldb_permissions});
+range = llvm::AddressRange(page_addr, page_addr + page_size);
+ }
+}
+ }
+ // The last range
+ if (!range.empty())
+ranges.push_back({range, lldb_permissions});
+ return true;
+}
+
+// Given a region, add the region to \a ranges.
+//
+// Only add the region if it isn't empty and if it has some permissions.
+// If \a try_dirty_pages is true, then try to add only the dirty pages for a
+// given region. If the region has dirty page information, only dirty pages
+// will be added to \a ranges, else the entire range will be added to \a
+// ranges.
+static void AddRegion(const MemoryRegionInfo , bool try_dirty_pages,
+ Process::CoreFileMemoryRanges ) {
clayborg wrote:
If we have empty ranges or a memory region without permissions, we don't need
to add it to the core file, and this function will determine if it needs to add
the region to the list of regions to save to the core file. The user won't care
or need to know about any of the error conditions.
https://github.com/llvm/llvm-project/pull/71772
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
@@ -6252,3 +6243,188 @@ Status Process::WriteMemoryTags(lldb::addr_t addr,
size_t len,
return DoWriteMemoryTags(addr, len, tag_manager->GetAllocationTagType(),
*packed_tags);
}
+
+// Create a CoreFileMemoryRange from a MemoryRegionInfo
+static Process::CoreFileMemoryRange
+CreateCoreFileMemoryRange(const MemoryRegionInfo ) {
+ const addr_t addr = region.GetRange().GetRangeBase();
+ llvm::AddressRange range(addr, addr + region.GetRange().GetByteSize());
+ return {range, region.GetLLDBPermissions()};
+}
+
+// Add dirty pages to the core file ranges and return true if dirty pages
+// were added. Return false if the dirty page information is not valid or in
+// the region.
+static bool AddDirtyPages(const MemoryRegionInfo ,
+ Process::CoreFileMemoryRanges ) {
+ const auto _page_list = region.GetDirtyPageList();
+ if (!dirty_page_list)
+return false;
+ const uint32_t lldb_permissions = region.GetLLDBPermissions();
+ const addr_t page_size = region.GetPageSize();
+ if (page_size == 0)
+return false;
+ llvm::AddressRange range(0, 0);
+ for (addr_t page_addr : *dirty_page_list) {
+if (range.empty()) {
+ // No range yet, initialize the range with the current dirty page.
+ range = llvm::AddressRange(page_addr, page_addr + page_size);
+} else {
+ if (range.end() == page_addr) {
+// Combine consective ranges.
+range = llvm::AddressRange(range.start(), page_addr + page_size);
+ } else {
+// Add previous contiguous range and init the new range with the
+// current dirty page.
+ranges.push_back({range, lldb_permissions});
+range = llvm::AddressRange(page_addr, page_addr + page_size);
+ }
+}
+ }
+ // The last range
+ if (!range.empty())
+ranges.push_back({range, lldb_permissions});
+ return true;
+}
+
+// Given a region, add the region to \a ranges.
+//
+// Only add the region if it isn't empty and if it has some permissions.
+// If \a try_dirty_pages is true, then try to add only the dirty pages for a
+// given region. If the region has dirty page information, only dirty pages
+// will be added to \a ranges, else the entire range will be added to \a
+// ranges.
+static void AddRegion(const MemoryRegionInfo , bool try_dirty_pages,
clayborg wrote:
`try_dirty_pages` is saying "if you can add only the dirty pages from the
current region because the process plugin supports it, then please add those,
if not please add the full region. If you note in the code below we only try to
add dirty pages if we are asked to when `try_dirty_pages == true`
https://github.com/llvm/llvm-project/pull/71772
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
@@ -6252,3 +6243,188 @@ Status Process::WriteMemoryTags(lldb::addr_t addr,
size_t len,
return DoWriteMemoryTags(addr, len, tag_manager->GetAllocationTagType(),
*packed_tags);
}
+
+// Create a CoreFileMemoryRange from a MemoryRegionInfo
+static Process::CoreFileMemoryRange
+CreateCoreFileMemoryRange(const MemoryRegionInfo ) {
+ const addr_t addr = region.GetRange().GetRangeBase();
+ llvm::AddressRange range(addr, addr + region.GetRange().GetByteSize());
clayborg wrote:
This is an llvm::AddressRange here. We also have lldb_private::AddressRange
which contains a lldb_private::Address (section offset address) + byte size.
The llvm::AddressRange just has a start address and end address. I could add
accessors to MemoryRegionInfo for these, but as Jason already eluded to we have
llvm::AddressRange and lldb_private::AddressRange and if we include both header
files in MemoryRegionInfo.h, we might end up with more qualifications needed on
bare `AddressRange` types in source files.
https://github.com/llvm/llvm-project/pull/71772
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
@@ -6252,3 +6243,188 @@ Status Process::WriteMemoryTags(lldb::addr_t addr,
size_t len,
return DoWriteMemoryTags(addr, len, tag_manager->GetAllocationTagType(),
*packed_tags);
}
+
+// Create a CoreFileMemoryRange from a MemoryRegionInfo
+static Process::CoreFileMemoryRange
+CreateCoreFileMemoryRange(const MemoryRegionInfo ) {
+ const addr_t addr = region.GetRange().GetRangeBase();
+ llvm::AddressRange range(addr, addr + region.GetRange().GetByteSize());
+ return {range, region.GetLLDBPermissions()};
+}
+
+// Add dirty pages to the core file ranges and return true if dirty pages
+// were added. Return false if the dirty page information is not valid or in
+// the region.
+static bool AddDirtyPages(const MemoryRegionInfo ,
clayborg wrote:
We aren't going to propagate the error, we just need to know if it succeeded or
not, so I didn't have it return an error. Otherwise every single range from a
linux lldb-server will return an error "dirty pages not supported". For Darwin
we added special extra features to track dirty pages, but no one else has
these. Again, I can add an error here, but I will just end up consuming the
error because lack of dirty page information isn't going to stop a core file
from being created. Also many regions on mac will not have any dirty pages if
they don't have write permissions, so most regions would end up returning an
error, which again, we won't do anything with, we will just ignore.
https://github.com/llvm/llvm-project/pull/71772
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the code that figures out which memory ranges to save into core files (PR #71772)
@@ -704,7 +705,37 @@ class Process : public
std::enable_shared_from_this,
/// is not supported by the plugin, error otherwise.
virtual llvm::Expected SaveCore(llvm::StringRef outfile);
+ struct CoreFileMemoryRange {
+llvm::AddressRange range; /// The address range to save into the core file.
+uint32_t lldb_permissions; /// A bit set of lldb::Permissions bits.
+
+bool operator==(const CoreFileMemoryRange ) const {
+ return range == rhs.range && lldb_permissions == rhs.lldb_permissions;
+}
+
+bool operator!=(const CoreFileMemoryRange ) const {
+ return !(*this == rhs);
+}
+
+bool operator<(const CoreFileMemoryRange ) const {
+ if (range < rhs.range)
+return true;
+ if (range == rhs.range)
+return lldb_permissions < rhs.lldb_permissions;
+ return false;
+}
+ };
+
+ using CoreFileMemoryRanges = std::vector;
+
+ /// Helper function for Process::SaveCore(...) that calculates the address
+ /// ranges that should be save. This allows all core file plug-ins to save
+ /// consistent memory ranges given a \a core_style.
+ Status CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
+ CoreFileMemoryRanges );
+
clayborg wrote:
I did that initially but then it was always just converted to Status in the end
so it seemed like using Expected when we didn't need to. I am happy to change
to Expected<> if you really think it should be that way
https://github.com/llvm/llvm-project/pull/71772
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
