[PATCH] D65346: [llvm-objcopy] Improve --add-section argument string parsing

[Sergey Dmitriev via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
sdmitriev marked 4 inline comments as done.
Closed by commit rL367236: [llvm-objcopy] Improve --add-section argument string parsing (authored by sdmitriev, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D65346?vs=212057&id=212180#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65346/new/

https://reviews.llvm.org/D65346

Files:
  llvm/trunk/test/tools/llvm-objcopy/COFF/add-section.test
  llvm/trunk/test/tools/llvm-objcopy/ELF/add-section.test
  llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.cpp
  llvm/trunk/tools/llvm-objcopy/CopyConfig.cpp


Index: llvm/trunk/test/tools/llvm-objcopy/ELF/add-section.test
===================================================================
--- llvm/trunk/test/tools/llvm-objcopy/ELF/add-section.test
+++ llvm/trunk/test/tools/llvm-objcopy/ELF/add-section.test
@@ -35,3 +35,19 @@
 # CHECK: SectionData (
 # CHECK-NEXT:   0000: DEADBEEF
 # CHECK-NEXT: )
+
+## Test that llvm-objcopy produces an error if the file with section contents
+## to be added does not exist.
+# RUN: not llvm-objcopy --add-section=.section.name=%t.missing %t %t.out 2>&1 | FileCheck -DFILE1=%t -DFILE2=%t.missing %s --check-prefixes=ERR1
+
+# ERR1: error: '[[FILE1]]': '[[FILE2]]': {{[Nn]}}o such file or directory
+
+## Negative test for invalid --add-sections argument - missing '='.
+# RUN: not llvm-objcopy --add-section=.section.name %t %t.out 2>&1 | FileCheck %s --check-prefixes=ERR2
+
+# ERR2: error: bad format for --add-section: missing '='
+
+## Negative test for invalid --add-sections argument - missing file name.
+# RUN: not llvm-objcopy --add-section=.section.name= %t %t.out 2>&1 | FileCheck %s --check-prefixes=ERR3
+
+# ERR3: error: bad format for --add-section: missing file name
Index: llvm/trunk/test/tools/llvm-objcopy/COFF/add-section.test
===================================================================
--- llvm/trunk/test/tools/llvm-objcopy/COFF/add-section.test
+++ llvm/trunk/test/tools/llvm-objcopy/COFF/add-section.test
@@ -34,14 +34,19 @@
 
 ## Test that llvm-objcopy produces an error if the file with section contents
 ## to be added does not exist.
-# RUN: not llvm-objcopy --add-section=.another.section=%t2 %t %t3 2>&1 | FileCheck -DFILE=%t -DFILE1=%t2 %s --check-prefixes=CHECK-ERR1
+# RUN: not llvm-objcopy --add-section=.another.section=%t2 %t %t3 2>&1 | FileCheck -DFILE1=%t -DFILE2=%t2 %s --check-prefixes=ERR1
 
-# CHECK-ERR1: llvm-objcopy{{(.exe)?}}: error: '[[FILE]]': '[[FILE1]]': {{[Nn]}}o such file or directory
+# ERR1: error: '[[FILE1]]': '[[FILE2]]': {{[Nn]}}o such file or directory
 
 ## Another negative test for invalid --add-sections command line argument.
-# RUN: not llvm-objcopy --add-section=.another.section %t %t3 2>&1 | FileCheck -DFILE=%t %s --check-prefixes=CHECK-ERR2
+# RUN: not llvm-objcopy --add-section=.another.section %t %t3 2>&1 | FileCheck %s --check-prefixes=ERR2
 
-# CHECK-ERR2: llvm-objcopy{{(.exe)?}}: error: '[[FILE]]': bad format for --add-section
+# ERR2: error: bad format for --add-section: missing '='
+
+## Negative test for invalid --add-sections argument - missing file name.
+# RUN: not llvm-objcopy --add-section=.section.name= %t %t3 2>&1 | FileCheck %s --check-prefixes=ERR3
+
+# ERR3: error: bad format for --add-section: missing file name
 
 --- !COFF
 header:
Index: llvm/trunk/tools/llvm-objcopy/CopyConfig.cpp
===================================================================
--- llvm/trunk/tools/llvm-objcopy/CopyConfig.cpp
+++ llvm/trunk/tools/llvm-objcopy/CopyConfig.cpp
@@ -617,8 +617,17 @@
     Config.KeepSection.emplace_back(Arg->getValue(), UseRegex);
   for (auto Arg : InputArgs.filtered(OBJCOPY_only_section))
     Config.OnlySection.emplace_back(Arg->getValue(), UseRegex);
-  for (auto Arg : InputArgs.filtered(OBJCOPY_add_section))
-    Config.AddSection.push_back(Arg->getValue());
+  for (auto Arg : InputArgs.filtered(OBJCOPY_add_section)) {
+    StringRef ArgValue(Arg->getValue());
+    if (!ArgValue.contains('='))
+      return createStringError(errc::invalid_argument,
+                               "bad format for --add-section: missing '='");
+    if (ArgValue.split("=").second.empty())
+      return createStringError(
+          errc::invalid_argument,
+          "bad format for --add-section: missing file name");
+    Config.AddSection.push_back(ArgValue);
+  }
   for (auto Arg : InputArgs.filtered(OBJCOPY_dump_section))
     Config.DumpSection.push_back(Arg->getValue());
   Config.StripAll = InputArgs.hasArg(OBJCOPY_strip_all);
Index: llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.cpp
===================================================================
--- llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.cpp
+++ llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.cpp
@@ -186,9 +186,6 @@
     StringRef SecName, FileName;
     std::tie(SecName, FileName) = Flag.split("=");
 
-    if (FileName.empty())
-      return createStringError(llvm::errc::invalid_argument,
-                               "bad format for --add-section");
     auto BufOrErr = MemoryBuffer::getFile(FileName);
     if (!BufOrErr)
       return createFileError(FileName, errorCodeToError(BufOrErr.getError()));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65346.212180.patch
Type: text/x-patch
Size: 4583 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190729/37e7d0e8/attachment.bin>