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?


>
> +      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
  • [PATCH] D35961: [ll... Alexander Shaposhnikov via Phabricator via cfe-commits
    • [PATCH] D35961... Saleem Abdulrasool via Phabricator via cfe-commits
      • Re: [PATCH... Kevin Enderby via cfe-commits
        • Re: [P... David Blaikie via cfe-commits
          • Re... Alexander Shaposhnikov via cfe-commits
            • ... Kevin Enderby via cfe-commits
          • Re... Kevin Enderby via cfe-commits
            • ... David Blaikie via cfe-commits

Reply via email to