[llvm-objdump] support "-M att" and "-M intel" flags

[thakis]

Summary

Increases compatibility with GNU objdump.

[thakis]

tweak comment

[thakis]

clang-format

[MaskRay]

D101695

[thakis]

In D101761#2733553, @MaskRay wrote:

D101695

Oh, cool. Your patch doesn't get interleaving both kinds of options right though afaict.

[MaskRay]

In D101761#2733856, @thakis wrote:

In D101761#2733553, @MaskRay wrote:

D101695

Oh, cool. Your patch doesn't get interleaving both kinds of options right though afaict.

I don't think that is necessary. Users shouldn't use --x86-asm-syntax. Actually we probably should remove it afterwards.

[thakis]

In D101761#2733927, @MaskRay wrote:

In D101761#2733856, @thakis wrote:

In D101761#2733553, @MaskRay wrote:

D101695

Oh, cool. Your patch doesn't get interleaving both kinds of options right though afaict.

I don't think that is necessary. Users shouldn't use --x86-asm-syntax. Actually we probably should remove it afterwards.

Works for me!

But since that's the only way to get this atm, it's mentioned in a bunch of places: https://stackoverflow.com/questions/10362630/how-can-objdump-emit-intel-syntax#comment97536406_10362655

So maybe support both for a release or so?

[MaskRay]

In D101761#2734213, @thakis wrote:

In D101761#2733927, @MaskRay wrote:

In D101761#2733856, @thakis wrote:

In D101761#2733553, @MaskRay wrote:

D101695

Oh, cool. Your patch doesn't get interleaving both kinds of options right though afaict.

I don't think that is necessary. Users shouldn't use --x86-asm-syntax. Actually we probably should remove it afterwards.

Works for me!

But since that's the only way to get this atm, it's mentioned in a bunch of places: https://stackoverflow.com/questions/10362630/how-can-objdump-emit-intel-syntax#comment97536406_10362655

So maybe support both for a release or so?

Deleting --x86-asm-syntax before 14.0.0 branches LGTM.

[thakis]

Conceptually, it seems a bit nicer to me to not add the options to DisassemblerOptions in the first place instead of adding and then removing them again. But I mostly want the feature and don't care too much :) So abandoning this in favor of your CL.

Consider stealing the code bits in this CL for your CL?

llvm/test/tools/llvm-objdump/X86/elf-disassemble-symbololize-operands.yaml

 # RUN: yaml2obj %s -o %t
 # RUN: llvm-objdump %t -d --symbolize-operands --x86-asm-syntax=intel --no-show-raw-insn --no-leading-addr | \
 # RUN:   FileCheck %s --match-full-lines --check-prefix=INTEL
+# RUN: llvm-objdump %t -dM Intel --symbolize-operands --no-show-raw-insn --no-leading-addr | \
+# RUN:   FileCheck %s --match-full-lines --check-prefix=INTEL
 # RUN: llvm-objdump %t -d --symbolize-operands --x86-asm-syntax=att --no-show-raw-insn --no-leading-addr | \
 # RUN:   FileCheck %s --match-full-lines --check-prefix=ATT
+# RUN: llvm-objdump %t -dMatt --symbolize-operands --no-show-raw-insn --no-leading-addr | \
+# RUN:   FileCheck %s --match-full-lines --check-prefix=ATT
 
 ## Expect to find the branch labels and global variable name.
 # ATT:      <_start>:
 # ATT-NEXT:   pushq %rax
 # ATT-NEXT: <L1>:
 # ATT-NEXT:   cmpl  , %eax <symbol>
 # ATT-NEXT:   jge    <L0>
 # ATT-NEXT:   jmp    <L1>

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

   ArchiveHeaders = InputArgs.hasArg(OBJDUMP_archive_headers);
   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);
-  DisassemblerOptions =
-      commaSeparatedValues(InputArgs, OBJDUMP_disassembler_options_EQ);
+
+  // DisassemblerOptions are passed to
+  // MCInstPrinter::applyTargetSpecificCLOption, but handling "att" and "intel"
+  // in that hook is too late: createX86MCInstPrinter() returns a different
+  // MCInstPrinter subclass depending on the assembler dialect. So we have
+  // to handle `-M att` and `-M intel` specially: They don't go into
+  // DisassemblerOptions like the other `-M` flags.
+  // llvm-objdump hasn't supported `-M att` and `-M intel` for a long time, so
+  // keep supporting the old `--x86-asm-syntax=att/intel` spelling that people
+  // used to use with it before too, even though that's not how GNU objdump
+  // spells it.
+  const char *AsmSyntax = nullptr;
+  for (const auto *A : InputArgs.filtered(OBJDUMP_disassembler_options_EQ,
+                                          OBJDUMP_x86_asm_syntax_att,
+                                          OBJDUMP_x86_asm_syntax_intel)) {
+    switch (A->getOption().getID()) {
+    case OBJDUMP_x86_asm_syntax_att:
+      AsmSyntax = "--x86-asm-syntax=att";
+      continue;
+    case OBJDUMP_x86_asm_syntax_intel:
+      AsmSyntax = "--x86-asm-syntax=intel";
+      continue;
+    }
+
+    assert(A->getOption().getID() == OBJDUMP_disassembler_options_EQ);
+    for (StringRef Value : A->getValues()) {
+      if (Value.equals_lower("att")) {
+        AsmSyntax = "--x86-asm-syntax=att";
+        continue;
+      }
+      if (Value.equals_lower("intel")) {
+        AsmSyntax = "--x86-asm-syntax=intel";
+        continue;
+      }
+      llvm::SmallVector<StringRef, 2> SplitValues;
+      llvm::SplitString(Value, SplitValues, ",");
+      for (StringRef SplitValue : SplitValues)
+        DisassemblerOptions.push_back(SplitValue.str());
+    }
+  }
+
   if (const opt::Arg *A = InputArgs.getLastArg(OBJDUMP_dwarf_EQ)) {
     DwarfDumpType =
         StringSwitch<DIDumpType>(A->getValue()).Case("frames", DIDT_DebugFrame);
   }
   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);

                        .Case("ascii", DVASCII)
                        .Case("unicode", DVUnicode);
   }
   parseIntArg(InputArgs, OBJDUMP_debug_vars_indent_EQ, DbgIndent);
 
   parseMachOOptions(InputArgs);
 
   // Handle options that get forwarded to cl::opt<>s in libraries.
-  // FIXME: Depending on https://reviews.llvm.org/D84191#inline-946075 ,
-  // hopefully remove this again.
   std::vector<const char *> LLVMArgs;
   LLVMArgs.push_back("llvm-objdump (LLVM option parsing)");
-  if (const opt::Arg *A = InputArgs.getLastArg(OBJDUMP_x86_asm_syntax_att,
-                                               OBJDUMP_x86_asm_syntax_intel)) {
-    switch (A->getOption().getID()) {
-    case OBJDUMP_x86_asm_syntax_att:
-      LLVMArgs.push_back("--x86-asm-syntax=att");
-      break;
-    case OBJDUMP_x86_asm_syntax_intel:
-      LLVMArgs.push_back("--x86-asm-syntax=intel");
-      break;
-    }
-  }
+  if (AsmSyntax)
+    LLVMArgs.push_back(AsmSyntax);
   if (InputArgs.hasArg(OBJDUMP_mhvx))
     LLVMArgs.push_back("--mhvx");
   if (InputArgs.hasArg(OBJDUMP_mhvx_v66))
     LLVMArgs.push_back("--mhvx=v66");
   if (InputArgs.hasArg(OBJDUMP_mv60))
     LLVMArgs.push_back("--mv60");
   if (InputArgs.hasArg(OBJDUMP_mv65))
     LLVMArgs.push_back("--mv65");