[PATCH] D35961: [llvm] Update MachOObjectFile::exports interface

Kevin Enderby via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 28 10:24:26 PDT 2017


> On Jul 27, 2017, at 9:50 PM, Saleem Abdulrasool via Phabricator <reviews at 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.

+      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
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170728/d3849d78/attachment.html>


More information about the llvm-commits mailing list