Skip to content

Commit 617670b

Browse files
author
Kevin Frei
committed
Response to PR feedback #2 
1 parent 5269710 commit 617670b

File tree

9 files changed

+49
-59
lines changed

9 files changed

+49
-59
lines changed

‎llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ class OutputCategoryAggregator {
3838
public:
3939
OutputCategoryAggregator(bool includeDetail = false)
4040
: IncludeDetail(includeDetail) {}
41-
void EnableDetail() { IncludeDetail = true; }
42-
void DisableDetail() { IncludeDetail = false; }
41+
void ShowDetail(bool showDetail) { IncludeDetail = showDetail; }
4342
size_t GetNumCategories() const { return Aggregation.size(); }
4443
void Report(StringRef s, std::function<void()> detailCallback);
4544
void EnumerateResults(std::function<void(StringRef, unsigned)> handleCounts);

‎llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

Lines changed: 18 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -167,61 +167,48 @@ bool DWARFVerifier::verifyUnitHeader(const DWARFDataExtractor DebugInfoData,
167167
if (!ValidLength || !ValidVersion || !ValidAddrSize || !ValidAbbrevOffset ||
168168
!ValidType) {
169169
Success = false;
170-
bool headerShown = false;
170+
bool HeaderShown = false;
171+
auto ShowHeaderOnce = [&]() {
172+
if (!HeaderShown) {
173+
error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n",
174+
UnitIndex, OffsetStart);
175+
HeaderShown = true;
176+
}
177+
};
171178
if (!ValidLength)
172179
ErrorCategory.Report(
173180
"Unit Header Length: Unit too large for .debug_info provided", [&]() {
174-
if (!headerShown) {
175-
error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n",
176-
UnitIndex, OffsetStart);
177-
headerShown = true;
178-
}
181+
ShowHeaderOnce();
179182
note() << "The length for this unit is too "
180183
"large for the .debug_info provided.\n";
181184
});
182185
if (!ValidVersion)
183186
ErrorCategory.Report(
184187
"Unit Header Length: 16 bit unit header version is not valid", [&]() {
185-
if (!headerShown) {
186-
error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n",
187-
UnitIndex, OffsetStart);
188-
headerShown = true;
189-
}
188+
ShowHeaderOnce();
190189
note() << "The 16 bit unit header version is not valid.\n";
191190
});
192191
if (!ValidType)
193192
ErrorCategory.Report(
194193
"Unit Header Length: Unit type encoding is not valid", [&]() {
195-
if (!headerShown) {
196-
error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n",
197-
UnitIndex, OffsetStart);
198-
headerShown = true;
199-
}
194+
ShowHeaderOnce();
200195
note() << "The unit type encoding is not valid.\n";
201196
});
202197
if (!ValidAbbrevOffset)
203198
ErrorCategory.Report(
204199
"Unit Header Length: Offset into the .debug_abbrev section is not "
205200
"valid",
206201
[&]() {
207-
if (!headerShown) {
208-
error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n",
209-
UnitIndex, OffsetStart);
210-
headerShown = true;
211-
}
202+
ShowHeaderOnce();
212203
note() << "The offset into the .debug_abbrev section is "
213204
"not valid.\n";
214205
});
215206
if (!ValidAddrSize)
216-
ErrorCategory.Report(
217-
"Unit Header Length: Address size is unsupported", [&]() {
218-
if (!headerShown) {
219-
error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n",
220-
UnitIndex, OffsetStart);
221-
headerShown = true;
222-
}
223-
note() << "The address size is unsupported.\n";
224-
});
207+
ErrorCategory.Report("Unit Header Length: Address size is unsupported",
208+
[&]() {
209+
ShowHeaderOnce();
210+
note() << "The address size is unsupported.\n";
211+
});
225212
}
226213
*Offset = OffsetStart + Length + (isUnitDWARF64 ? 12 : 4);
227214
return Success;
@@ -1095,9 +1082,7 @@ DWARFVerifier::DWARFVerifier(raw_ostream &S, DWARFContext &D,
10951082
DIDumpOptions DumpOpts)
10961083
: OS(S), DCtx(D), DumpOpts(std::move(DumpOpts)), IsObjectFile(false),
10971084
IsMachOObject(false) {
1098-
if (DumpOpts.Verbose || !DumpOpts.ShowAggregateErrors) {
1099-
ErrorCategory.EnableDetail();
1100-
}
1085+
ErrorCategory.ShowDetail(DumpOpts.Verbose || !DumpOpts.ShowAggregateErrors);
11011086
if (const auto *F = DCtx.getDWARFObj().getFile()) {
11021087
IsObjectFile = F->isRelocatableObject();
11031088
IsMachOObject = F->isMachO();

‎llvm/test/DebugInfo/X86/skeleton-unit-verify.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o %t.o
2-
# RUN: not llvm-dwarfdump --no-aggregate-errors --verify %t.o | FileCheck %s
2+
# RUN: not llvm-dwarfdump --error-display=details --verify %t.o | FileCheck %s
33

44
# CHECK: Verifying .debug_abbrev...
55
# CHECK-NEXT: Verifying .debug_info Unit Header Chain...

‎llvm/test/DebugInfo/dwarfdump-accel.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
RUN: llvm-dwarfdump -v %p/Inputs/dwarfdump-objc.x86_64.o | FileCheck %s
2-
RUN: not llvm-dwarfdump -no-aggregate-errors -verify %p/Inputs/dwarfdump-objc.x86_64.o | FileCheck %s --check-prefix=VERIFY
2+
RUN: not llvm-dwarfdump -error-display=details -verify %p/Inputs/dwarfdump-objc.x86_64.o | FileCheck %s --check-prefix=VERIFY
33

44
Gather some DIE indexes to verify the accelerator table contents.
55
CHECK: .debug_info contents

‎llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error:
1+
# RUN: yaml2obj %s | not llvm-dwarfdump --error-display=details --verify - | FileCheck %s --implicit-check-not=error:
22

33
# CHECK: error: DIE has DW_AT_decl_file with an invalid file index 2 (valid values are [1-1]){{[[:space:]]}}
44
# CHECK-NEXT: 0x0000001e: DW_TAG_subprogram

‎llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error:
1+
# RUN: yaml2obj %s | not llvm-dwarfdump --error-display=details --verify - | FileCheck %s --implicit-check-not=error:
22

33
# CHECK: error: DIE has DW_AT_decl_file with an invalid file index 2 (the file table in the prologue is empty){{[[:space:]]}}
44
# CHECK-NEXT: 0x0000001e: DW_TAG_subprogram

‎llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
#
4040
# 0x00000066: NULL
4141

42-
# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error:
42+
# RUN: yaml2obj %s | not llvm-dwarfdump --error-display=details --verify - | FileCheck %s --implicit-check-not=error:
4343

4444
# CHECK: error: DIE has overlapping ranges in DW_AT_ranges attribute: [0x0000000000000000, 0x0000000000000020) and [0x0000000000000000, 0x0000000000000030)
4545

‎llvm/test/tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
# 0x00000056: NULL
3131

3232

33-
# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error:
33+
# RUN: yaml2obj %s | not llvm-dwarfdump --error-display=details --verify - | FileCheck %s --implicit-check-not=error:
3434

3535
# CHECK: Verifying -: file format Mach-O 64-bit x86-64
3636
# CHECK: Verifying .debug_abbrev...

‎llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,14 @@ template <> class parser<BoolOption> final : public basic_parser<BoolOption> {
124124
namespace {
125125
using namespace cl;
126126

127+
enum ErrorDetailLevel {
128+
OnlyDetailsNoSummary,
129+
NoDetailsOnlySummary,
130+
NoDetailsOrSummary,
131+
BothDetailsAndSummary,
132+
Unspecified
133+
};
134+
127135
OptionCategory DwarfDumpCategory("Specific Options");
128136
static list<std::string>
129137
InputFilenames(Positional, desc("<input object files or .dSYM bundles>"),
@@ -276,13 +284,16 @@ static cl::opt<bool>
276284
cat(DwarfDumpCategory));
277285
static opt<bool> Verify("verify", desc("Verify the DWARF debug info."),
278286
cat(DwarfDumpCategory));
279-
static opt<bool> OnlyAggregateErrors(
280-
"only-aggregate-errors",
281-
desc("Only display the aggregate errors when verifying."),
282-
cat(DwarfDumpCategory));
283-
static opt<bool> NoAggregateErrors(
284-
"no-aggregate-errors",
285-
desc("Do not display the aggregate errors when verifying."),
287+
static opt<ErrorDetailLevel> ErrorDetails(
288+
"error-display", init(Unspecified),
289+
values(clEnumValN(NoDetailsOrSummary, "quiet",
290+
"Only display whether errors occurred."),
291+
clEnumValN(NoDetailsOnlySummary, "summary",
292+
"Display only a summary of the errors found."),
293+
clEnumValN(OnlyDetailsNoSummary, "details",
294+
"Display each error in detail but no summary."),
295+
clEnumValN(BothDetailsAndSummary, "full",
296+
"Display each error as well as a summary. [default]")),
286297
cat(DwarfDumpCategory));
287298
static opt<bool> Quiet("quiet", desc("Use with -verify to not emit to STDOUT."),
288299
cat(DwarfDumpCategory));
@@ -334,8 +345,10 @@ static DIDumpOptions getDumpOpts(DWARFContext &C) {
334345
DumpOpts.RecoverableErrorHandler = C.getRecoverableErrorHandler();
335346
// In -verify mode, print DIEs without children in error messages.
336347
if (Verify) {
337-
DumpOpts.Verbose = !OnlyAggregateErrors;
338-
DumpOpts.ShowAggregateErrors = !NoAggregateErrors;
348+
DumpOpts.Verbose = ErrorDetails != NoDetailsOnlySummary &&
349+
ErrorDetails != NoDetailsOrSummary;
350+
DumpOpts.ShowAggregateErrors = ErrorDetails != OnlyDetailsNoSummary &&
351+
ErrorDetails != NoDetailsOnlySummary;
339352
return DumpOpts.noImplicitRecursion();
340353
}
341354
return DumpOpts;
@@ -821,15 +834,8 @@ int main(int argc, char **argv) {
821834
"-verbose is currently not supported";
822835
return 1;
823836
}
824-
if (Verify && NoAggregateErrors && OnlyAggregateErrors) {
825-
WithColor::error() << "incompatible arguments: specifying both "
826-
" -no-aggregate-errors, and -only-aggregate-errors "
827-
"is not supported";
828-
return 1;
829-
}
830-
if (!Verify && (NoAggregateErrors || OnlyAggregateErrors))
831-
WithColor::warning() << "-no-aggregate-errors and -only-aggregate-errors "
832-
"have no effect without -verify";
837+
if (!Verify && ErrorDetails != Unspecified)
838+
WithColor::warning() << "-error-detail has no affect without -verify";
833839

834840
std::error_code EC;
835841
ToolOutputFile OutputFile(OutputFilename, EC, sys::fs::OF_TextWithCRLF);

0 commit comments

Comments
 (0)