[llvm] 326bc1d - [Object] Fix handling of large archive members

[James Henderson via llvm-commits]

Author: James Henderson
Date: 2020-03-11T10:29:45Z
New Revision: 326bc1da45b22110a11e0578022829a263ed4311

URL: https://github.com/llvm/llvm-project/commit/326bc1da45b22110a11e0578022829a263ed4311
DIFF: https://github.com/llvm/llvm-project/commit/326bc1da45b22110a11e0578022829a263ed4311.diff

LOG: [Object] Fix handling of large archive members

The archive library truncated the size of archive members whose size was
greater than max uint32_t. This patch fixes the issue and adds some unit
tests to verify.

Reviewed by: ruiu, MaskRay, grimar, rupprecht

Differential Revision: https://reviews.llvm.org/D75742

Added: 
    llvm/unittests/Object/ArchiveTest.cpp

Modified: 
    llvm/lib/Object/Archive.cpp
    llvm/unittests/Object/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Object/Archive.cpp b/llvm/lib/Object/Archive.cpp
index 17d5eec9156c..c18dd11a72cc 100644
--- a/llvm/lib/Object/Archive.cpp
+++ b/llvm/lib/Object/Archive.cpp
@@ -392,12 +392,8 @@ Archive::Child::Child(const Archive *Parent, const char *Start, Error *Err)
 }
 
 Expected<uint64_t> Archive::Child::getSize() const {
-  if (Parent->IsThin) {
-    Expected<uint32_t> Size = Header.getSize();
-    if (!Size)
-      return Size.takeError();
-    return Size.get();
-  }
+  if (Parent->IsThin)
+    return Header.getSize();
   return Data.size() - StartOfFile;
 }
 
@@ -437,7 +433,7 @@ Expected<StringRef> Archive::Child::getBuffer() const {
     return isThinOrErr.takeError();
   bool isThin = isThinOrErr.get();
   if (!isThin) {
-    Expected<uint32_t> Size = getSize();
+    Expected<uint64_t> Size = getSize();
     if (!Size)
       return Size.takeError();
     return StringRef(Data.data() + StartOfFile, Size.get());

diff  --git a/llvm/unittests/Object/ArchiveTest.cpp b/llvm/unittests/Object/ArchiveTest.cpp
new file mode 100644
index 000000000000..df5aeeae02b9
--- /dev/null
+++ b/llvm/unittests/Object/ArchiveTest.cpp
@@ -0,0 +1,81 @@
+//===- ArchiveTest.cpp ----------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Object/Archive.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace object;
+using namespace testing;
+
+static const char ArchiveWithMember[] = "!<arch>\n"        // Global header
+                                        "test/           " // Member name
+                                        "0           "     // Timestamp
+                                        "0     "           // Owner ID
+                                        "0     "           // Group ID
+                                        "0       "         // File mode
+                                        "9999999999"       // Size
+                                        "`\n";
+
+static const char ThinArchiveWithMember[] = "!<thin>\n"        // Global header
+                                            "test/           " // Member name
+                                            "0           "     // Timestamp
+                                            "0     "           // Owner ID
+                                            "0     "           // Group ID
+                                            "0       "         // File mode
+                                            "9999999999"       // Size
+                                            "`\n";
+
+struct ArchiveTestsFixture : Test {
+  Expected<Archive::child_iterator> createChild(StringRef Src) {
+    MemoryBufferRef Source(Src, "archive");
+    Expected<std::unique_ptr<Archive>> AOrErr = Archive::create(Source);
+    if (!AOrErr)
+      return AOrErr.takeError();
+    A = std::move(*AOrErr);
+
+    Error ChildErr = Error::success();
+    auto Child = A->child_begin(ChildErr);
+    if (ChildErr)
+      return std::move(ChildErr);
+    return Child;
+  }
+
+  std::unique_ptr<Archive> A;
+};
+
+TEST_F(ArchiveTestsFixture, ArchiveChildGetSizeRegularArchive) {
+  auto Child = createChild(ArchiveWithMember);
+  ASSERT_THAT_EXPECTED(Child, Succeeded());
+
+  Expected<uint64_t> Size = (*Child)->getSize();
+  ASSERT_THAT_EXPECTED(Size, Succeeded());
+  EXPECT_EQ(9999999999, *Size);
+}
+
+TEST_F(ArchiveTestsFixture, ArchiveChildGetSizeThinArchive) {
+  auto Child = createChild(ThinArchiveWithMember);
+  ASSERT_THAT_EXPECTED(Child, Succeeded());
+
+  Expected<uint64_t> Size = (*Child)->getSize();
+  ASSERT_THAT_EXPECTED(Size, Succeeded());
+  EXPECT_EQ(9999999999, *Size);
+}
+
+TEST_F(ArchiveTestsFixture, ArchiveChildGetBuffer) {
+  auto Child = createChild(ArchiveWithMember);
+  ASSERT_THAT_EXPECTED(Child, Succeeded());
+
+  Expected<StringRef> Buffer = (*Child)->getBuffer();
+  // Cannot use ASSERT_THAT_EXPECTED, as that will attempt to print the
+  // StringRef (which has an invalid size).
+  ASSERT_TRUE((bool)Buffer);
+  EXPECT_EQ(9999999999, Buffer->size());
+  EXPECT_EQ(ArchiveWithMember + sizeof(ArchiveWithMember) - 1, Buffer->data());
+}

diff  --git a/llvm/unittests/Object/CMakeLists.txt b/llvm/unittests/Object/CMakeLists.txt
index 809515a2e78a..c5d1f5476ccd 100644
--- a/llvm/unittests/Object/CMakeLists.txt
+++ b/llvm/unittests/Object/CMakeLists.txt
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_llvm_unittest(ObjectTests
+  ArchiveTest.cpp
   MinidumpTest.cpp
   ObjectFileTest.cpp
   SymbolSizeTest.cpp