[PATCH] D55235: llvm-objcopy: Improve/simplify llvm::Error handling during notes iteration
David Blaikie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 3 14:14:02 PST 2018
dblaikie created this revision.
dblaikie added reviewers: lhames, jakehehrlich.
Herald added subscribers: llvm-commits, rupprecht, JDevlieghere, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a reviewer: alexshap.
Using an Error as an out parameter from an indirect operation like iteration as described in the documentation ( http://llvm.org/docs/ProgrammersManual.html#building-fallible-iterators-and-iterator-ranges ) seems to be a little fussy - so here's /one/ possible solution, though I'm not sure it's the right one.
Alternatively such APIs may be better off being switched to a standard algorithm style, where they take a lambda to do the iteration work that is then called back into (eg: "Error e = obj.for_each_note([](const Note& N) { ... });"). This would be safer than having an unwritten assumption that the user of such an iteration cannot return early from the inside of the function - and must always exit through the gift shop... I mean error checking. (even though it's guaranteed that if you're mid-way through processing an iteration, it's not in an error state).
Alternatively we'd need some other (the super untrustworthy/thing we've generally tried to avoid) error handling primitive that actually clears the error state entirely so it's safe to ignore.
Repository:
rL LLVM
https://reviews.llvm.org/D55235
Files:
include/llvm/Object/ELFTypes.h
tools/llvm-objcopy/ELF/ELFObjcopy.cpp
Index: tools/llvm-objcopy/ELF/ELFObjcopy.cpp
===================================================================
--- tools/llvm-objcopy/ELF/ELFObjcopy.cpp
+++ tools/llvm-objcopy/ELF/ELFObjcopy.cpp
@@ -123,16 +123,16 @@
if (Phdr.p_type != PT_NOTE)
continue;
Error Err = Error::success();
- if (Err)
- llvm_unreachable("Error::success() was an error.");
+ ArrayRef<uint8_t> Result;
for (const auto &Note : In.notes(Phdr, Err)) {
- if (Err)
- return std::move(Err);
- if (Note.getType() == NT_GNU_BUILD_ID && Note.getName() == ELF_NOTE_GNU)
- return Note.getDesc();
+ if (Note.getType() == NT_GNU_BUILD_ID && Note.getName() == ELF_NOTE_GNU) {
+ Result = Note.getDesc();
+ break;
+ }
}
if (Err)
return std::move(Err);
+ return Result;
}
return createStringError(llvm::errc::invalid_argument,
"Could not find build ID.");
Index: include/llvm/Object/ELFTypes.h
===================================================================
--- include/llvm/Object/ELFTypes.h
+++ include/llvm/Object/ELFTypes.h
@@ -631,6 +631,7 @@
// Stop iteration and indicate an overflow.
void stopWithOverflowError() {
Nhdr = nullptr;
+ ErrorAsOutParameter EAOP(Err);
*Err = make_error<StringError>("ELF note overflows container",
object_error::parse_failed);
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55235.176476.patch
Type: text/x-patch
Size: 1424 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181203/9d203ce0/attachment.bin>
More information about the llvm-commits
mailing list