[llvm-objdump] Fix the Assertion failure when providing invalid --debug-vars or --dwarf values

[gbreynoo]

Summary

As seen in https://bugs.llvm.org/show_bug.cgi?id=52213 llvm-objdump asserts if either the --debug-vars or the --dwarf options are provided with invalid values. As suggested, this fix adds use of a default value to these options and errors when given bad input.

[jhenderson]

llvm/test/tools/llvm-objdump/X86/debug-vars.yaml

1

Copy/paste error.

There's already testing in ARM/debug-vars-dwarf4.yaml for the valid values for --debug-vars, so I'd just simplify by putting the error check in there.

llvm/test/tools/llvm-objdump/X86/eh_frame.yaml

1

I'd suggest calling this test dwarf_frames.yaml, since that's the switch under test.

[jhenderson]

llvm/test/tools/llvm-objdump/X86/debug-vars.yaml

1

To clarify further: I think the test there is just a generic test, so the fact that it's in ARM doesn't mean we need another test case for X86, since the functionality is generic.

llvm/test/tools/llvm-objdump/X86/eh_frame.yaml

1

Actually, having looked at what testing there already is, I'd consider naming this dwarf_invalid.yaml, and putting it outside the X86 directory: this test contains no X86-specific functionality, so it doesn't need to be there.

17

I believe you can drop the Machine line, since you're not trying to do anything that needs a target (but you might not be able to).

[gbreynoo]

Updated after James' comments and fixed the format issue.

[jhenderson]

llvm/test/tools/llvm-objdump/ELF/ARM/debug-vars-dwarf4.s

48 ↗

(On Diff #381543)

I don't think this is going to do what you think it will.

I think you're trying to feed the llvm-mc output into llvm-objdump (the - means read input from stdin), but you aren't piping any llvm-mc output into llvm-objdump.

llvm/test/tools/llvm-objdump/dwarf_invalid.yaml

1 ↗

(On Diff #381543)

This comment is out-of-date, given the contents and test name.

There is already testing of --dwarf=frames output for llvm-objdump. You only need to check the invalid value case.

[gbreynoo]

Fixed comment and added input to llvm-objdump call as suggested by James.

[gbreynoo]

I have also removed the successful test case from dwarf_invalid.yaml.

llvm/test/tools/llvm-objdump/ELF/ARM/debug-vars-dwarf4.s

48 ↗

(On Diff #381543)

As the commandline parsing gave the error before the validity of the input was checked I didn't think we needed this line. To avoid confusion I have added the input.

[jhenderson]

LGTM, with nit.

llvm/test/tools/llvm-objdump/ELF/ARM/debug-vars-dwarf4.s

48 ↗

(On Diff #381543)

We want to ensure that there are no other errors being emitted too, otherwise, we don't know that this error actually prevents continuation of the process, hence we need a valid input.

llvm/test/tools/llvm-objdump/dwarf_invalid.yaml

1 ↗

(On Diff #384035)

Nit: add missing trailing full stop.

[gbreynoo]

Fixed James' Nit and reduced the multiple identical calls to llvm-mc

[jhenderson]

LGTM again.

[jhenderson]

llvm/test/tools/llvm-objdump/ELF/ARM/debug-vars-dwarf4.s

14 ↗

(On Diff #384478)

Please indent this and all similar lines in this file. The logic is that by indenting it, you show that this is a continuation of the previous RUN line.

llvm/test/tools/llvm-objdump/X86/debug-vars.yaml

+## Test parsing of --dwarf=frames
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %S/disassemble-print-comments.s -o %t 
+
+# RUN: llvm-objdump -d --no-show-raw-insn --debug-vars  %t | FileCheck %s --check-prefix=UNICODE
+# RUN: llvm-objdump -d --no-show-raw-insn --debug-vars=unicode  %t | FileCheck %s --check-prefix=UNICODE
+
+# UNICODE:     0000000000000000 <foo>:
+# UNICODE-NEXT:                                                                    ┠─ bar = RDX
+# UNICODE-NEXT:       0:       nop                                                 ┃
+# UNICODE-NEXT:       1:       cmpl    $305419896, %eax        # imm = 0x12345678  ┻
+
+# RUN: llvm-objdump -d --no-show-raw-insn --debug-vars=ascii  %t | FileCheck %s --check-prefix=ASCII
+
+# ASCII:     0000000000000000 <foo>:
+# ASCII-NEXT:                                                                    |- bar = RDX
+# ASCII-NEXT:       0:       nop                                                 |
+# ASCII-NEXT:       1:       cmpl    $305419896, %eax        # imm = 0x12345678  v
+
+# RUN: not llvm-objdump -d --no-show-raw-insn --debug-vars=bad_value %t 2>&1 | FileCheck %s --check-prefix=ERROR
+
+# ERROR: error: 'bad_value' is not a valid value for '--debug-vars='

llvm/test/tools/llvm-objdump/X86/eh_frame.yaml

+## Test parsing of --dwarf=frames
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objdump --dwarf=frames %t | FileCheck %s
+# CHECK: .debug_frame contents:
+# CHECK: .eh_frame contents:
+
+# RUN: not llvm-objdump --dwarf=bad_value %t 2>&1 | FileCheck %s --check-prefix=ERROR
+
+# ERROR: error: 'bad_value' is not a valid value for '--dwarf='
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64

llvm/tools/llvm-objdump/llvm-objdump.h

 class MachOObjectFile;
 class MachOUniversalBinary;
 class RelocationRef;
 struct VersionEntry;
 } // namespace object
 
 namespace objdump {
 
 enum DebugVarsFormat {
   DVDisabled,
   DVUnicode,
   DVASCII,
+  DVInvalid
 };
 
 extern bool ArchiveHeaders;
 extern int DbgIndent;
 extern DebugVarsFormat DbgVariables;
 extern bool Demangle;
 extern bool Disassemble;
 extern bool DisassembleAll;

llvm/tools/llvm-objdump/llvm-objdump.cpp

if (const opt::Arg *A = InputArgs.getLastArg(ID)) {
     if (!llvm::to_integer(V, Value, 0)) {
       reportCmdLineError(A->getSpelling() +
                          ": expected a non-negative integer, but got '" + V +
                          "'");
     }
   }
 }
 
+static void invalidArgValue(const opt::Arg *A) {
+  reportCmdLineError("'" + StringRef(A->getValue()) +
+                     "' is not a valid value for '" + A->getSpelling() + "'");
+}
+
 static std::vector<std::string>
 commaSeparatedValues(const llvm::opt::InputArgList &InputArgs, int ID) {
   std::vector<std::string> Values;
   for (StringRef Value : InputArgs.getAllArgValues(ID)) {
     llvm::SmallVector<StringRef, 2> SplitValues;
     llvm::SplitString(Value, SplitValues, ",");
     for (StringRef SplitValue : SplitValues)
       Values.push_back(SplitValue.str());
static void parseObjdumpOptions(const llvm::opt::InputArgList &InputArgs) {
   Demangle = InputArgs.hasArg(OBJDUMP_demangle);
   Disassemble = InputArgs.hasArg(OBJDUMP_disassemble);
   DisassembleAll = InputArgs.hasArg(OBJDUMP_disassemble_all);
   SymbolDescription = InputArgs.hasArg(OBJDUMP_symbol_description);
   DisassembleSymbols =
       commaSeparatedValues(InputArgs, OBJDUMP_disassemble_symbols_EQ);
   DisassembleZeroes = InputArgs.hasArg(OBJDUMP_disassemble_zeroes);
   if (const opt::Arg *A = InputArgs.getLastArg(OBJDUMP_dwarf_EQ)) {
-    DwarfDumpType =
-        StringSwitch<DIDumpType>(A->getValue()).Case("frames", DIDT_DebugFrame);
+    DwarfDumpType = StringSwitch<DIDumpType>(A->getValue())
+                        .Case("frames", DIDT_DebugFrame)
+                        .Default(DIDT_Null);
+    if (DwarfDumpType == DIDT_Null)
+      invalidArgValue(A);
   }
   DynamicRelocations = InputArgs.hasArg(OBJDUMP_dynamic_reloc);
   FaultMapSection = InputArgs.hasArg(OBJDUMP_fault_map_section);
   FileHeaders = InputArgs.hasArg(OBJDUMP_file_headers);
   SectionContents = InputArgs.hasArg(OBJDUMP_full_contents);
   PrintLines = InputArgs.hasArg(OBJDUMP_line_numbers);
   InputFilenames = InputArgs.getAllArgValues(OBJDUMP_INPUT);
   MachOOpt = InputArgs.hasArg(OBJDUMP_macho);
static void parseObjdumpOptions(const llvm::opt::InputArgList &InputArgs) {
   TripleName = InputArgs.getLastArgValue(OBJDUMP_triple_EQ).str();
   UnwindInfo = InputArgs.hasArg(OBJDUMP_unwind_info);
   Wide = InputArgs.hasArg(OBJDUMP_wide);
   Prefix = InputArgs.getLastArgValue(OBJDUMP_prefix).str();
   parseIntArg(InputArgs, OBJDUMP_prefix_strip, PrefixStrip);
   if (const opt::Arg *A = InputArgs.getLastArg(OBJDUMP_debug_vars_EQ)) {
     DbgVariables = StringSwitch<DebugVarsFormat>(A->getValue())
                        .Case("ascii", DVASCII)
-                       .Case("unicode", DVUnicode);
+                       .Case("unicode", DVUnicode)
+                       .Default(DVInvalid);
+    if (DbgVariables == DVInvalid)
+      invalidArgValue(A);
   }
   parseIntArg(InputArgs, OBJDUMP_debug_vars_indent_EQ, DbgIndent);
 
   parseMachOOptions(InputArgs);
 
   // Parse -M (--disassembler-options) and deprecated
   // --x86-asm-syntax={att,intel}.
   //