[llvm-objdump] Fix --prefix and --prefix-strip

[gbreynoo]

Summary

In the command guide --prefix and --prefix-strip is used in the form --prefix=<prefix> however currently it is used in the form --prefix <prefix>. This change fixes these options to match the command guide.

[gbreynoo]

Looking at the other options in llvm-objdump, --option=<value> seems to be the consistent format. For this reason I changed the command line option rather than the user guide, if people disagree I'm happy to change the command guide instead.

[jhenderson]

What does GNU objdump allow? We should allow whatever GNU does.

[gbreynoo]

Fixed non windows test.

[gbreynoo]

Looking at the GNU docs its` --prefix=prefix`.

[MaskRay]

Many utilities supporting double-dash long options (not only GNU's) generally use the GNU style getopt_long which supports both --foo value and --foo=value.

We can use lld/llvm-objcopy style tablegen Eq to support both Joined and Separate.

[gbreynoo]

Updated after Maskray's comment.

[MaskRay]

I suggest that you use a multiclass Eq to avoid verbosity for individual options.

[gbreynoo]

Updated after Maskray's comment.

Thanks Maskray, that is cleaner.

[jhenderson]

LGTM! Please wait for @MaskRay too.

[MaskRay]

Thanks! Sorry for the slow response. I was out of town for one week.

llvm/test/tools/llvm-objdump/X86/source-interleave-prefix-windows.test

 ;; Test --prefix option platform specific behavior.
 ; REQUIRES: system-windows
 
 ;; Test removal of trailing separators (both '/' and '\' on Windows systems).
 ;; The prefix 'myprefix/\' is changed to 'myprefix'.
 
 ; RUN: sed -e "s,SRC_COMPDIR,/Inputs,g" %p/Inputs/source-interleave.ll > %t.ll
 ; RUN: llc -o %t.o -filetype=obj -mtriple=x86_64-pc-linux %t.ll
-; RUN: llvm-objdump --prefix 'myprefix/\' --source %t.o 2>&1 | FileCheck %s -DFILE=%t.o -DPREFIX='myprefix'
+; RUN: llvm-objdump --prefix='myprefix/\' --source %t.o 2>&1 | FileCheck %s -DFILE=%t.o -DPREFIX='myprefix'
 ; CHECK: warning: '[[FILE]]': failed to find source [[PREFIX]]/Inputs\source-interleave-x86_64.c
 

llvm/test/tools/llvm-objdump/X86/source-interleave-prefix.test

 ;; Test --prefix option.
 
 ;; Separators change from platform to platform. In POSIX the full path for the
 ;; directory './Inputs' appended with the file 'source-interleave-x86_64.c' is
 ;; './Inputs/source-interleave-x86_64.c'. For Windows it is
 ;; './Inputs\source-interleave-x86_64.c'. Platform specific tests are needed
 ;; since '\' may or may not be a separator.
 
 ;; Test prefix option ignored for relative paths.
 ;; For the test below it is possible to accept both '/' and '\' as a separator.
 
 ; RUN: sed -e "s,SRC_COMPDIR,./Inputs,g" %p/Inputs/source-interleave.ll > %t-relative-path.ll
 ; RUN: llc -o %t-relative-path.o -filetype=obj -mtriple=x86_64-pc-linux %t-relative-path.ll
-; RUN: llvm-objdump --prefix myprefix --source %t-relative-path.o 2>&1 | \
+; RUN: llvm-objdump --prefix=myprefix --source %t-relative-path.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-BROKEN-PREFIX -DFILE=%t-relative-path.o -DPREFIX=. -DCOMPDIR=/Inputs
 ; CHECK-BROKEN-PREFIX: warning: '[[FILE]]': failed to find source [[PREFIX]][[COMPDIR]]{{[/\\]}}source-interleave-x86_64.c
 
 ;; Test invalid source interleave fixed by adding the correct prefix.
 
 ; RUN: sed -e "s,SRC_COMPDIR,/Inputs,g" %p/Inputs/source-interleave.ll > %t-missing-prefix.ll
 ; RUN: llc -o %t-missing-prefix.o -filetype=obj -mtriple=x86_64-pc-linux %t-missing-prefix.ll
-; RUN: llvm-objdump --prefix %p --source %t-missing-prefix.o 2>&1 | \
+; RUN: llvm-objdump --prefix=%p --source %t-missing-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-MISSING-PREFIX-FIX
 ; CHECK-MISSING-PREFIX-FIX: ; int foo() {
 
 ;; Test valid source interleave broken by adding an incorrect prefix.
 
 ; RUN: sed -e "s,SRC_COMPDIR,%/p/Inputs,g" %p/Inputs/source-interleave.ll > %t-correct-prefix.ll
 ; RUN: llc -o %t-correct-prefix.o -filetype=obj -mtriple=x86_64-pc-linux %t-correct-prefix.ll
-; RUN: llvm-objdump --prefix myprefix --source %t-correct-prefix.o 2>&1 | \
+; RUN: llvm-objdump --prefix=myprefix --source %t-correct-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-BROKEN-PREFIX -DFILE=%t-correct-prefix.o -DPREFIX=myprefix%/p -DCOMPDIR=/Inputs
 
 ;; Using only a prefix separator is the same as not using the `--prefix` option.
 
-; RUN: llvm-objdump --prefix / --source %t-missing-prefix.o 2>&1 | \
+; RUN: llvm-objdump --prefix=/ --source %t-missing-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-BROKEN-PREFIX -DFILE=%t-missing-prefix.o -DPREFIX='' -DCOMPDIR=/Inputs
 
 ;; All trailing separators on the prefix are discarded.
 ;; The prefix 'myprefix//' is converted to 'myprefix'.
 
-; RUN: llvm-objdump --prefix myprefix// --source %t-missing-prefix.o 2>&1 | \
+; RUN: llvm-objdump --prefix=myprefix// --source %t-missing-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-BROKEN-PREFIX -DFILE=%t-missing-prefix.o -DPREFIX=myprefix -DCOMPDIR=/Inputs
 
 ;; Test invalid source interleave fixed by adding the correct prefix and
 ;; stripping out an extra directory from the path.
 
 ; RUN: sed -e "s,SRC_COMPDIR,/extra/Inputs,g" %p/Inputs/source-interleave.ll > %t-extra-path-prefix.ll
 ; RUN: llc -o %t-extra-path-prefix.o -filetype=obj -mtriple=x86_64-pc-linux %t-extra-path-prefix.ll
-; RUN: llvm-objdump --prefix %p --prefix-strip 1 --source %t-extra-path-prefix.o 2>&1 | \
+; RUN: llvm-objdump --prefix=%p --prefix-strip=1 --source %t-extra-path-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-MISSING-PREFIX-FIX
 
 ;; Test do not skip extra separators. The --prefix-strip should take into
 ;; account each separator individually. Hence, to fix '/extra/Inputs'
 ;; --prefix-strip needs to be 1. To fix '//extra/Inputs' --prefix-strip
 ;; needs to be 2.
 
 ; RUN: sed -e "s,SRC_COMPDIR,//extra/Inputs,g" %p/Inputs/source-interleave.ll > %t-extra-sep-path-prefix.ll
 ; RUN: llc -o %t-extra-sep-path-prefix.o -filetype=obj -mtriple=x86_64-pc-linux %t-extra-sep-path-prefix.ll
-; RUN: llvm-objdump --prefix %p --prefix-strip 2 --source %t-extra-sep-path-prefix.o 2>&1 | \
+; RUN: llvm-objdump --prefix=%p --prefix-strip=2 --source %t-extra-sep-path-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-MISSING-PREFIX-FIX
 
 ;; Test --prefix-strip value of 0. No effect.
-;; SRC_COMPDIR is set to '/Inputs' before and after --prefix-strip 0.
+;; SRC_COMPDIR is set to '/Inputs' before and after --prefix-strip=0.
 
-; RUN: llvm-objdump --prefix %p --prefix-strip 0 --source %t-missing-prefix.o 2>&1 | \
+; RUN: llvm-objdump --prefix=%p --prefix-strip=0 --source %t-missing-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-MISSING-PREFIX-FIX
 
 ;; Test --prefix-strip value equal to the number of directory components.
-;; SRC_COMPDIR is set to '/Inputs' before --prefix-strip 1.
-;; SRC_COMPDIR becomes '' after --prefix-strip 1.
+;; SRC_COMPDIR is set to '/Inputs' before --prefix-strip=1.
+;; SRC_COMPDIR becomes '' after --prefix-strip=1.
 
-; RUN: llvm-objdump --prefix %p/Inputs --prefix-strip 1 --source %t-missing-prefix.o 2>&1 | \
+; RUN: llvm-objdump --prefix=%p/Inputs --prefix-strip=1 --source %t-missing-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-MISSING-PREFIX-FIX
 
 ;; Test --prefix-strip value greater than the number of components.
-;; SRC_COMPDIR is set to '/Inputs' before --prefix-strip 2.
-;; SRC_COMPDIR becomes '' after --prefix-strip 2.
+;; SRC_COMPDIR is set to '/Inputs' before --prefix-strip=2.
+;; SRC_COMPDIR becomes '' after --prefix-strip=2.
 
-; RUN: llvm-objdump --prefix %p/Inputs --prefix-strip 2 --source %t-missing-prefix.o 2>&1 | \
+; RUN: llvm-objdump --prefix=%p/Inputs --prefix-strip=2 --source %t-missing-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-MISSING-PREFIX-FIX
 
 ;; Test negative value --prefix-strip. Reports an error.
 
-; RUN: not llvm-objdump --prefix %p --prefix-strip '-1' --source %t-missing-prefix.o 2>&1 | \
+; RUN: not llvm-objdump --prefix=%p --prefix-strip='-1' --source %t-missing-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-INVALID-PREFIX-STRIP -DOPTION='-1'
-; CHECK-INVALID-PREFIX-STRIP: {{.*}}llvm-objdump{{.*}}: error: --prefix-strip: expected a non-negative integer, but got '[[OPTION]]'
+; CHECK-INVALID-PREFIX-STRIP: {{.*}}llvm-objdump{{.*}}: error: --prefix-strip=: expected a non-negative integer, but got '[[OPTION]]'
 
 ;; Test text value --prefix-strip. Reports an error.
 
-; RUN: not llvm-objdump --prefix %p --prefix-strip foo --source %t-missing-prefix.o 2>&1 | \
+; RUN: not llvm-objdump --prefix=%p --prefix-strip=foo --source %t-missing-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-INVALID-PREFIX-STRIP -DOPTION='foo'

llvm/tools/llvm-objdump/ObjdumpOpts.td

def unwind_info : Flag<["--"], "unwind-info">,
   HelpText<"Display unwind information">;
 def : Flag<["-"], "u">, Alias<unwind_info>,
   HelpText<"Alias for --unwind-info">;
 
 def wide : Flag<["--"], "wide">,
   HelpText<"Ignored for compatibility with GNU objdump">;
 def : Flag<["-"], "w">, Alias<wide>;
 
-def prefix : Separate<["--"], "prefix">,
+def prefix_EQ : Joined<["--"], "prefix=">,
   HelpText<"Add prefix to absolute paths">;
 
-def prefix_strip : Separate<["--"], "prefix-strip">,
+def prefix_strip_EQ : Joined<["--"], "prefix-strip=">,
   HelpText<"Strip out initial directories from absolute "
            "paths. No effect without --prefix">;
 
 def debug_vars_EQ : Joined<["--"], "debug-vars=">,
   Values<"unicode,ascii">;
 def : Flag<["--"], "debug-vars">,
   HelpText<"Print the locations (in registers or memory) of "
            "source-level variables alongside disassembly">,

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

static void parseObjdumpOptions(const llvm::opt::InputArgList &InputArgs) {
   parseIntArg(InputArgs, OBJDUMP_stop_address_EQ, StopAddress);
   HasStopAddressFlag = InputArgs.hasArg(OBJDUMP_stop_address_EQ);
   SymbolTable = InputArgs.hasArg(OBJDUMP_syms);
   SymbolizeOperands = InputArgs.hasArg(OBJDUMP_symbolize_operands);
   DynamicSymbolTable = InputArgs.hasArg(OBJDUMP_dynamic_syms);
   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);
+  Prefix = InputArgs.getLastArgValue(OBJDUMP_prefix_EQ).str();
+  parseIntArg(InputArgs, OBJDUMP_prefix_strip_EQ, PrefixStrip);
   if (const opt::Arg *A = InputArgs.getLastArg(OBJDUMP_debug_vars_EQ)) {
     DbgVariables = StringSwitch<DebugVarsFormat>(A->getValue())
                        .Case("ascii", DVASCII)
                        .Case("unicode", DVUnicode);
   }
   parseIntArg(InputArgs, OBJDUMP_debug_vars_indent_EQ, DbgIndent);
 
   parseMachOOptions(InputArgs);