[llvm-readobj][ELF] Print ELF e_entry in lowercase mode #156683
Conversation
Currently llvm-readobj prints e_entry address in uppercase mode, however all other places such as symbols, relocs the addresses are printed in lowercase mode. This PR aims to print the addresses in uniform manner. Other use-case: FileCheck comparison is case-sensitive by default, so if you would like to compare e_entry address with the same address in the symbol table it won't match by default.
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-llvm-binary-utilities Author: Ádám Kallai (kaadam) ChangesCurrently llvm-readobj prints e_entry address in uppercase mode, however all other places such as 'symbols', 'relocs', the addresses are printed in lowercase mode. This PR aims to print the addresses in uniform manner. Other use-case: Full diff: https://github.com/llvm/llvm-project/pull/156683.diff 1 Files Affected:
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index a440bad130f4c..79a6ce1fd88b8 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -3640,7 +3640,7 @@ template <class ELFT> void GNUELFDumper<ELFT>::printFileHeaders() {
printFields(OS, "Machine:", Str);
Str = "0x" + utohexstr(e.e_version);
printFields(OS, "Version:", Str);
- Str = "0x" + utohexstr(e.e_entry);
+ Str = "0x" + utohexstr(e.e_entry, /*LowerCase=*/true);
printFields(OS, "Entry point address:", Str);
Str = to_string(e.e_phoff) + " (bytes into file)";
printFields(OS, "Start of program headers:", Str);
|
There was a problem hiding this comment.
Thanks for the PR! In principle, I think this makes sense. The majority of our hex printing is via Twine::utohexstr, which is always lower-case. I think the missing lower case argument in the calls to llvm::utohexstr are likely just oversights and should be updated.
There are several other places in this file that use llvm::utohexstr, some of which don't use the lower-case argument. Please could you update them all (and any tests that fail as a result of course). I note that in some cases (e.g. e_version), it's unlikely to matter due to the range of values that could be printed, but it's better to be consistent.
Adjusted the related tests to the newer format.
|
@jh7370 Thanks for your review. I've updated the |
There was a problem hiding this comment.
llvm::utohexstr defaults to upper-case while Twine::utohexstr defaults to lower-case. I think lower-case is more popular and llvm-readelf always uses lower-case.
Using more lower-case for llvm-readobj makes sense. Unfortunately, changing some fields (relocation offsets and addends) for llvm-readobj might cause a lot of changes.
Currently llvm-readobj prints e.g. e_entry address in upper-case mode, however all other places such as 'symbols', 'relocs', the addresses are printed in lower-case mode. This PR aims to print the addresses in uniform manner.
Other use-case:
FileCheck comparison is case-sensitive by default, so if you would like to compare e_entry address with the same address in the symbol table it won't match by default.
The corresponding tests are adjusted as well.