On Fri, Jul 28, 2017 at 4:01 PM Kevin Enderby <ende...@apple.com> wrote:
> On Jul 28, 2017, at 10:30 AM, David Blaikie <dblai...@gmail.com> wrote: > > > > On Fri, Jul 28, 2017 at 10:24 AM Kevin Enderby <ende...@apple.com> wrote: > >> On Jul 27, 2017, at 9:50 PM, Saleem Abdulrasool via Phabricator < >> revi...@reviews.llvm.org> wrote: >> >> compnerd added a comment. >> >> Does anyone use the overload with the `O` for `exports` with `nullptr` >> instead of `this`? If not, we could just inline `this` throughout. >> >> >> This will break lld as it needs the default nullptr. See r308691. >> >> The reason that O was added was so that this check from r308690 could be >> added. >> > > When O is specified is it always == this, though? That seems to be implied > by the original post. > > If that's the case, then the ability to pass null is really only passing > the on/off state for the diagnostic? Is that necessary, or would it be good > to just always produce the Error, even in lld? > > > Without O one can’t use O->getLibraryCount() which is what is needed for > the test. I guess one could change the interface if lld really wanted this > check. But I’d leave that to the lld folks as to what level of error > checking they want. > No worries - looks like Alex explained the disconnect that there were different functions under discussion. (: > > > >> >> + if (O != nullptr) { >> + if (State.Other > O->getLibraryCount()) { >> + *E = malformedError("bad library ordinal: " + >> Twine((int)State.Other) >> + + " (max " + Twine((int)O->getLibraryCount()) + ") in >> export " >> + "trie data at node: 0x" + utohexstr(offset)); >> + moveToEnd(); >> + return; >> + } >> >> This is needed for the test case: >> >> +RUN: not llvm-objdump -macho -exports-trie >> %p/Inputs/macho-trie-bad-library-ordinal 2>&1 | FileCheck -check-prefix >> BAD_LIBRARY_ORDINAL %s >> +BAD_LIBRARY_ORDINAL: macho-trie-bad-library-ordinal': truncated or >> malformed object (bad library ordinal: 69 (max 3) in export trie data at >> node: 0x33) >> >> >> >> >> ================ >> Comment at: tools/llvm-nm/llvm-nm.cpp:1230 >> Error Err = Error::success(); >> - for (const llvm::object::ExportEntry &Entry : MachO->exports(Err, >> - >> MachO)) { >> + for (const llvm::object::ExportEntry &Entry : MachO->exports(Err)) >> { >> bool found = false; >> ---------------- >> I think that using `auto` here instead of `llvm::object:ExportEntry` is >> better for readability. >> >> >> ================ >> Comment at: tools/llvm-objdump/MachODump.cpp:9406 >> Error Err = Error::success(); >> - for (const llvm::object::ExportEntry &Entry : Obj->exports(Err, Obj)) { >> + for (const llvm::object::ExportEntry &Entry : Obj->exports(Err)) { >> uint64_t Flags = Entry.flags(); >> ---------------- >> Similar. >> >> >> Repository: >> rL LLVM >> >> https://reviews.llvm.org/D35961 >> >> >> >> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits