Resolve PR comments, add new Error tests.

Author: dsalinas_amdeng

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

@@ -210,27 +210,6 @@ static Error dumpSectionToFile(StringRef SecName, StringRef Filename,
                          "section '%s' not found", SecName.str().c_str());
 }
 
-static Error dumpRawDataURIToFile(StringRef Filename, int64_t Offset,
-                                  int64_t Size, ObjectFile &Obj) {
-  SmallString<2048> NameBuf;
-  raw_svector_ostream OutputFileName(NameBuf);
-  OutputFileName << Obj.getFileName().str() << "-offset" << Offset << "-size"
-                 << Size << ".co";
-
-  Expected<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
-      FileOutputBuffer::create(OutputFileName.str(), Size);
-
-  if (!BufferOrErr)
-    return BufferOrErr.takeError();
-
-  MemoryBufferRef Input = Obj.getMemoryBufferRef();
-  std::unique_ptr<FileOutputBuffer> Buf = std::move(*BufferOrErr);
-  std::copy(Input.getBufferStart(), Input.getBufferStart() + Size,
-            Buf->getBufferStart());
-
-  return Buf->commit();
-}
-
 Error Object::compressOrDecompressSections(const CommonConfig &Config) {
   // Build a list of sections we are going to replace.
   // We can't call `addSection` while iterating over sections,

llvm/lib/Object/OffloadBundle.cpp

@@ -211,12 +211,23 @@ Error object::extractCodeObject(const ObjectFile &Source, int64_t Offset,
   if (Error Err = InputBuffOrErr.takeError())
     return Err;
 
+  if (Size > InputBuffOrErr->getBufferSize())
+    return createStringError(inconvertibleErrorCode(),
+                             "size in URI is larger than source");
+  if (Offset > InputBuffOrErr->getBufferSize())
+    return createStringError(inconvertibleErrorCode(),
+                             "offset in URI is beyond the size of the source");
+  if (Offset + Size > InputBuffOrErr->getBufferSize())
+    return createStringError(
+        inconvertibleErrorCode(),
+        "offset + size in URI is beyond the size of the source");
+
   std::unique_ptr<FileOutputBuffer> Buf = std::move(*BufferOrErr);
   std::copy(InputBuffOrErr->getBufferStart() + Offset,
             InputBuffOrErr->getBufferStart() + Offset + Size,
             Buf->getBufferStart());
   if (Error E = Buf->commit())
-    return E;
+    return createFileError(OutputFileName, std::move(E));
 
   return Error::success();
 }
@@ -229,6 +240,7 @@ Error object::extractOffloadBundleByURI(StringRef URIstr) {
       OffloadBundleURI::createOffloadBundleURI(URIstr, FILE_URI));
   if (!UriOrErr)
     return UriOrErr.takeError();
+
   OffloadBundleURI &Uri = **UriOrErr;
   std::string OutputFile = Uri.FileName.str();
   OutputFile +=
@@ -237,12 +249,12 @@ Error object::extractOffloadBundleByURI(StringRef URIstr) {
   // Create an ObjectFile object from uri.file_uri
   auto ObjOrErr = ObjectFile::createObjectFile(Uri.FileName);
   if (!ObjOrErr)
-    return ObjOrErr.takeError();
+    return createFileError(Uri.FileName, ObjOrErr.takeError());
 
   auto Obj = ObjOrErr->getBinary();
   if (Error Err =
           object::extractCodeObject(*Obj, Uri.Offset, Uri.Size, OutputFile))
-    return Err;
+    return std::move(Err);
 
   return Error::success();
 }

llvm/test/tools/llvm-objcopy/ELF/dump-offload-bundle.test

@@ -6,6 +6,15 @@
 # RUN: llvm-objcopy --dump-offload-bundle=file://%t.elf#offset=8192\&size=4048
 # RUN: llvm-objdump -d %t.elf-offset8192-size4048.co  | FileCheck %s
 
+# RUN: not llvm-objcopy --dump-offload-bundle=file://%t.elf#offset=8192\&size=4048000 2>&1 | \
+# RUN:  FileCheck %s --check-prefix=ERR1
+
+# RUN: not llvm-objcopy --dump-offload-bundle=file://%t.elf#offset=819200000\&size=4048 2>&1 | \
+# RUN:  FileCheck %s --check-prefix=ERR2
+
+# RUN: not llvm-objcopy --dump-offload-bundle=file://%t.elf#offset=8192\&size=8048 2>&1 | \
+# RUN:  FileCheck %s --check-prefix=ERR3
+
 # CHECK:        s_load_dword s7, s[4:5], 0x24                              // 000000001900: C00201C2 00000024
 # CHECK-NEXT:        s_load_dwordx4 s[0:3], s[4:5], 0x0                         // 000000001908: C00A0002 00000000
 # CHECK-NEXT:        v_mov_b32_e32 v1, 0                                        // 000000001910: 7E020280
@@ -27,6 +36,10 @@
 # CHECK-NEXT:        global_store_dword v[0:1], v2, off                         // 000000001960: DC708000 007F0200
 # CHECK-NEXT:        s_endpgm                                                   // 000000001968: BF810000
 
+# ERR1: error: size in URI is larger than source
+# ERR2: error: offset in URI is beyond the size of the source
+# ERR3: error: offset + size in URI is beyond the size of the source
+
 --- !ELF
 FileHeader:
   Class:           ELFCLASS64

llvm/tools/llvm-objcopy/ObjcopyOptions.cpp

@@ -288,8 +288,8 @@ static Expected<uint8_t> parseVisibilityType(StringRef VisType) {
 
 Expected<StringRef> llvm::objcopy::parseDumpOffloadBundle(StringRef URI) {
   if (Error Err = object::extractOffloadBundleByURI(URI))
-    return createStringError(errc::invalid_argument,
-                             "failed to extract from URI");
+    return std::move(Err);
+
   return URI;
 }
 
@@ -1445,7 +1445,6 @@ objcopy::parseInstallNameToolOptions(ArrayRef<const char *> ArgsArr) {
     return createStringError(
         errc::invalid_argument,
         "llvm-install-name-tool expects a single input file");
-
   Config.InputFilename = Positional[0];
   Config.OutputFilename = Positional[0];
 
@@ -1499,9 +1498,8 @@ objcopy::parseBitcodeStripOptions(ArrayRef<const char *> ArgsArr,
   for (auto *Arg : InputArgs.filtered(BITCODE_STRIP_INPUT))
     Positional.push_back(Arg->getValue());
   if (Positional.size() > 1)
-    return createStringError(
-        errc::invalid_argument,
-        "llvm-bitcode-strip expects a single input file");
+    return createStringError(errc::invalid_argument,
+                             "llvm-bitcode-strip expects a single input file");
   assert(!Positional.empty());
   Config.InputFilename = Positional[0];
 
@@ -1579,8 +1577,7 @@ objcopy::parseStripOptions(ArrayRef<const char *> RawArgsArr,
   std::copy(DashDash, RawArgsArr.end(), std::back_inserter(Positional));
 
   if (Positional.empty())
-    return createStringError(errc::invalid_argument,
-                             "no input file specified");
+    return createStringError(errc::invalid_argument, "no input file specified");
 
   if (Positional.size() > 1 && InputArgs.hasArg(STRIP_output))
     return createStringError(

llvm/tools/llvm-objcopy/ObjcopyOptions.h

@@ -52,8 +52,8 @@ Expected<DriverConfig>
 parseStripOptions(ArrayRef<const char *> ArgsArr,
                   llvm::function_ref<Error(Error)> ErrorCallback);
 
-// parseDumpOffloadBundle reads a URI as a string and extracts the raw memory into a
-// code object file named from the URI string given.
+// parseDumpOffloadBundle reads a URI as a string and extracts the raw memory
+// into a code object file named from the URI string given.
 Expected<StringRef> parseDumpOffloadBundle(StringRef URI);
 
 } // namespace objcopy