[PATCH] D53690: [llvm-objdump] Don't Crash When Using `-a` on Non-Archives (PR39402)

[Xing via Phabricator via llvm-commits]

Higuoxing created this revision.
Higuoxing added a reviewer: jhenderson.
Herald added a subscriber: llvm-commits.

The crash was caused when dereferencing nullptr in `DumpObject` and `printArchiveChild`.

In `DumpObject`, we have

  2311    StringRef ArchiveName = a != nullptr ? a->getFileName() : "";
  ...
  2323      printArchiveChild(a->getFileName(), *c);



  2368    StringRef ArchiveName = A ? A->getFileName() : "";
  ...
  2378      printArchiveChild(A->getFileName(), *C);

Look, this will cause segment fault when we use `-a` option on Non-archive files.
Besides, in `printArchiveChild`

  2236  static void printArchiveChild(StringRef Filename, const Archive::Child &C) {
  2237    Expected<sys::fs::perms> ModeOrErr = C.getAccessMode();
  ...

This can also cause segment fault when calling `C.getAccessMode()` (`&C can be nullptr`).

I update the `printArchiveChild` argument by taking a pointer of `Archive::Child` and check whether it's a nullptr.

This is a draft, I will add some tests later.

Any suggestions are welcoming, thanks

Bugzilla – Bug 39402 <https://bugs.llvm.org/show_bug.cgi?id=39402>


Repository:
  rL LLVM

https://reviews.llvm.org/D53690

Files:
  tools/llvm-objdump/llvm-objdump.cpp


Index: tools/llvm-objdump/llvm-objdump.cpp
===================================================================
--- tools/llvm-objdump/llvm-objdump.cpp
+++ tools/llvm-objdump/llvm-objdump.cpp
@@ -2228,8 +2228,9 @@
          << "\n";
 }
 
-static void printArchiveChild(StringRef Filename, const Archive::Child &C) {
-  Expected<sys::fs::perms> ModeOrErr = C.getAccessMode();
+static void printArchiveChild(StringRef Filename, const Archive::Child *C) {
+  if (C == nullptr) return;
+  Expected<sys::fs::perms> ModeOrErr = C->getAccessMode();
   if (!ModeOrErr) {
     errs() << "ill-formed archive entry.\n";
     consumeError(ModeOrErr.takeError());
@@ -2248,24 +2249,24 @@
 
   outs() << " ";
 
-  Expected<unsigned> UIDOrErr = C.getUID();
+  Expected<unsigned> UIDOrErr = C->getUID();
   if (!UIDOrErr)
     report_error(Filename, UIDOrErr.takeError());
   unsigned UID = UIDOrErr.get();
   outs() << format("%d/", UID);
 
-  Expected<unsigned> GIDOrErr = C.getGID();
+  Expected<unsigned> GIDOrErr = C->getGID();
   if (!GIDOrErr)
     report_error(Filename, GIDOrErr.takeError());
   unsigned GID = GIDOrErr.get();
   outs() << format("%-d ", GID);
 
-  Expected<uint64_t> Size = C.getRawSize();
+  Expected<uint64_t> Size = C->getRawSize();
   if (!Size)
     report_error(Filename, Size.takeError());
   outs() << format("%6" PRId64, Size.get()) << " ";
 
-  StringRef RawLastModified = C.getRawLastModified();
+  StringRef RawLastModified = C->getRawLastModified();
   unsigned Seconds;
   if (RawLastModified.getAsInteger(10, Seconds))
     outs() << "(date: \"" << RawLastModified
@@ -2279,10 +2280,10 @@
   }
 
   StringRef Name = "";
-  Expected<StringRef> NameOrErr = C.getName();
+  Expected<StringRef> NameOrErr = C->getName();
   if (!NameOrErr) {
     consumeError(NameOrErr.takeError());
-    Expected<StringRef> RawNameOrErr = C.getRawName();
+    Expected<StringRef> RawNameOrErr = C->getRawName();
     if (!RawNameOrErr)
       report_error(Filename, NameOrErr.takeError());
     Name = RawNameOrErr.get();
@@ -2306,7 +2307,7 @@
   }
 
   if (ArchiveHeaders && !MachOOpt)
-    printArchiveChild(a->getFileName(), *c);
+    printArchiveChild(ArchiveName, c);
   if (Disassemble)
     DisassembleObject(o, Relocations);
   if (Relocations && !Disassemble)
@@ -2360,7 +2361,7 @@
            << "\n\n";
 
   if (ArchiveHeaders && !MachOOpt)
-    printArchiveChild(A->getFileName(), *C);
+    printArchiveChild(ArchiveName, C);
   if (SymbolTable)
     printCOFFSymbolTable(I);
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53690.171045.patch
Type: text/x-patch
Size: 2497 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181025/8942bc20/attachment.bin>