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.
Details
Diff Detail
Event Timeline
llvm/test/tools/llvm-objdump/X86/debug-vars.yaml | ||
---|---|---|
1 ↗ | (On Diff #381094) | 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 ↗ | (On Diff #381094) | I'd suggest calling this test dwarf_frames.yaml, since that's the switch under test. |
llvm/test/tools/llvm-objdump/X86/debug-vars.yaml | ||
---|---|---|
1 ↗ | (On Diff #381094) | 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 ↗ | (On Diff #381094) | 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 ↗ | (On Diff #381094) | 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). |
llvm/test/tools/llvm-objdump/ELF/ARM/debug-vars-dwarf4.s | ||
---|---|---|
40 | 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 | ||
2 | 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. |
I have also removed the successful test case from dwarf_invalid.yaml.
llvm/test/tools/llvm-objdump/ELF/ARM/debug-vars-dwarf4.s | ||
---|---|---|
40 | 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. |
LGTM, with nit.
llvm/test/tools/llvm-objdump/ELF/ARM/debug-vars-dwarf4.s | ||
---|---|---|
40 | 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 | ||
2 | Nit: add missing trailing full stop. |
llvm/test/tools/llvm-objdump/ELF/ARM/debug-vars-dwarf4.s | ||
---|---|---|
14 | 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. |
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.