[PATCH] D92545: [llvm-readobj/elf] - Refine the implementation of "printFunctionStackSize".
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 7 01:25:27 PST 2020
jhenderson added inline comments.
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5810-5811
const Elf_Shdr &StackSizeSec, DataExtractor Data, uint64_t *Offset) {
// This function ignores potentially erroneous input, unless it is directly
// related to stack size reporting.
+ uint32_t FuncSymIndex = 0;
----------------
You've added several new warnings with this refactor. Does this comment still apply?
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5841-5846
+ std::string Name = this->dumper().getStaticSymbolName(Index);
+ // Note: it is impossible to trigger this error currently, it is
+ // untested.
+ reportUniqueWarning("unable to get section of symbol '" + Name +
+ "': " + toString(SecOrErr.takeError()));
+ break;
----------------
Should this not just be `cantFail`?
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5874
if (*Offset == PrevOffset) {
- reportWarning(createStringError(object_error::parse_failed,
- "could not extract a valid stack size in " +
- describe(Obj, StackSizeSec)),
- FileName);
+ reportUniqueWarning("could not extract a valid stack size in " +
+ describe(Obj, StackSizeSec));
----------------
Are you planning on further improving these messages in another change? The offset would be useful here, for example.
(Suggested fix could be deferred too, if it causes too many test changes)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92545/new/
https://reviews.llvm.org/D92545
More information about the llvm-commits
mailing list