Paths
| Differential D74433
[llvm-objdump] Print file format in lowercase to match GNU output. ClosedPublic Authored by rupprecht on Feb 11 2020, 12:00 PM.
Details Summary GNU objdump prints the file format in lowercase, e.g. elf64-x86-64. llvm-objdump prints ELF64-x86-64 right now, even though piping that into llvm-objcopy refuses that as a valid arch to use. As an example of a problem this causes, see: https://github.com/ClangBuiltLinux/linux/issues/779
Diff Detail
Unit TestsFailed Event TimelineComment Actions Verified that coff, elf, and mach-o are printed as lower cases. I believe bpf is similar.
This revision is now accepted and ready to land.Feb 11 2020, 1:23 PM Comment Actions Wait. I wonder whether we can change llvm-readobj to use lower case names as well. The following should be updated: StringRef ELFObjectFile<ELFT>::getFileFormatName() const { bool IsLittleEndian = ELFT::TargetEndianness == support::little; switch (EF.getHeader()->e_ident[ELF::EI_CLASS]) { case ELF::ELFCLASS32: switch (EF.getHeader()->e_machine) { case ELF::EM_386: return "ELF32-i386"; case ELF::EM_IAMCU: return "ELF32-iamcu"; case ELF::EM_X86_64: return "ELF32-x86-64"; case ELF::EM_ARM: These are almost bfdnames, except that they use upper cases. I don't find a compelling argument using upper case ELF, so I vote for changing these ELF to elf. @jhenderson @grimar @sbc100 thoughts? // lib/Object/WasmObjectFile.cpp
StringRef WasmObjectFile::getFileFormatName() const { return "WASM"; }Comment Actions I think the patch is good to go on its own, after the outs() << ":\tfile format " << O->getFileFormatName().lower() << "\n\n"; change. If the consensus is to also change llvm-readobj output, we can teach getFileFormatName() to use lower case names and remove .lower() from llvm-objdump.cpp . It will involve a bunch of other test case updates. They can be done in a separate change. Comment Actions
Yep... I was originally thinking we could update those to be lowercase, but that's kinda a broader change, and I imagine most llvm developers are used to seeing "ELF64-..." and I'm not sure it's worth changing all the tools just so that objdump can be more GNU compatible. I personally wouldn't mind it. If others are in favor of it too, I'm all for it and can make the change. Comment Actions I don't really have a strong opinion either way. By strict style rules, "ELF" is an acronym, so should be all upper-case. In regular text, I'd expect it to be ELF everywhere too. On the other hand, as pointed out, "elf" is the bfd naming style. For WASM, I recall there being some discussion to standardise on a particular format, and it would make sense if we can be consistent across all file formats. Perhaps @sbc100 could bring his insights from that side over. Comment Actions Wasm is not an acronym so we don't usually display it in uppercase. So yes, I think we should change WasmObjectFile::getFileFormatName() to return lowercase. Closed by commit rG60a8a504f16d: [llvm-objdump] Print file format in lowercase to match GNU output. (authored by rupprecht). · Explain WhyFeb 12 2020, 8:23 AM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 243946 clang/test/Modules/pch_container.mlld/test/COFF/savetemps.ll
llvm/test/CodeGen/AArch64/arm64-simplest-elf.ll
llvm/test/CodeGen/ARM/Windows/trivial-gnu-object.ll
llvm/test/CodeGen/BPF/reloc-btf-2.ll
llvm/test/CodeGen/BPF/reloc-btf.ll
llvm/test/CodeGen/BPF/reloc.ll
llvm/test/Object/AMDGPU/objdump.s
llvm/test/Object/X86/objdump-disassembly-inline-relocations.test
llvm/test/Object/X86/objdump-label.test
llvm/test/Object/X86/objdump-trivial-object.test
llvm/test/Object/dynamic-reloc.test
llvm/test/Object/objdump-symbol-table.test
llvm/test/tools/llvm-objdump/X86/disassemble-section-name.s
llvm/test/tools/llvm-objdump/X86/elf-disassemble-symbol-labels-exec.test
llvm/test/tools/llvm-objdump/X86/elf-dynamic-relocs.test
llvm/test/tools/llvm-objdump/X86/output-ordering.test
llvm/test/tools/llvm-objdump/X86/warn-missing-disasm-func.test
llvm/test/tools/llvm-objdump/all-headers.test
llvm/test/tools/llvm-objdump/archive-headers.test
llvm/test/tools/llvm-objdump/file-headers-coff.test
llvm/test/tools/llvm-objdump/file-headers-elf.test
llvm/test/tools/llvm-objdump/non-archive-object.test
llvm/test/tools/llvm-objdump/relocations-in-nonreloc.test
llvm/tools/llvm-objdump/llvm-objdump.cpp
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
clang-format: please reformat the code