[llvm-objcopy] Fix --add-section when section contain empty bytes

[gmorer]

Summary

Implicit cast between char* and StringRef when writing sections.

Reproduce:

$> llvm-objcopy --dump-section=name=name.data out.wasm
$> llvm-objcopy --remove-section=name out.wasm out_no_name.wasm
$> llvm-objcopy --add-section=name=name.data out_no_name.wasm out_new_name.wasm

# With wasm-objdump -h we can see that the name section is not totally copied in the new wasm file (if it actually contain empty bytes)

[dschuff]

The code looks good to me. Could you add a test case to llvm/test/tools/llvm-objcopy/wasm/add-section.test?
I guess it could be like the 2nd test case (that creates a file using echo and adds it, then checks the YAML payload).

[gmorer]

Tests

[dschuff]

llvm/test/tools/llvm-objcopy/wasm/add-section.test

23

nit: Can you mention in the comment that zero bytes are an important component of this test?

[gmorer]

Update the test explanation

[dschuff]

Thanks! do you need me to commit this?

[gmorer]

Yes please!

[dyung]

@gmorer I had to revert your change because it was causing at least 2 Windows build bots to fail:

• https://lab.llvm.org/buildbot/#/builders/216/builds/13762
• https://lab.llvm.org/buildbot/#/builders/123/builds/14447

[gmorer]

Trying to fix windows test suite, because echo does not behave the same as unix

[jhenderson]

Nit: the reiew title uses --section-add but the option is --add-section. Please update.

llvm/test/tools/llvm-objcopy/wasm/add-section.test

23

Nit: most comments in tests in llvm-objcopy, including some in this file, use ## to mark them rather than just # to distinguish them from lit test directives.

[gmorer]

Update test comment

[gmorer]

Change commit name

[jhenderson]

llvm/test/tools/llvm-objcopy/wasm/add-section.test

30

[gmorer]

Update test comment

[gmorer]

Do i need to prefix the commit message with "Reapply: ", and tell which commit reverted it? Not 100% sure it pass the windows tests, I used echo the same way it's used in other tests. But i do not have a windows build machine with the same env to test on.

[dschuff]

Thanks for fixing this! I tested it on windows and it seems to work (with the caveat that it's not my primary platform either). I'll reland it. I'll add "reland" to the commit message but it also still has the link to this review issue, so that should be good enough for context.

llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp

static Error handleArgs(const CommonConfig &Config, Object &Obj) {
 
   removeSections(Config, Obj);
 
   for (const NewSectionInfo &NewSection : Config.AddSection) {
     Section Sec;
     Sec.SectionType = llvm::wasm::WASM_SEC_CUSTOM;
     Sec.Name = NewSection.SectionName;
 
+    llvm::StringRef InputData =
+        llvm::StringRef(NewSection.SectionData->getBufferStart(),
+                        NewSection.SectionData->getBufferSize());
     std::unique_ptr<MemoryBuffer> BufferCopy = MemoryBuffer::getMemBufferCopy(
-        NewSection.SectionData->getBufferStart(),
-        NewSection.SectionData->getBufferIdentifier());
+        InputData, NewSection.SectionData->getBufferIdentifier());
     Sec.Contents = makeArrayRef<uint8_t>(
         reinterpret_cast<const uint8_t *>(BufferCopy->getBufferStart()),
         BufferCopy->getBufferSize());
 
     Obj.addSectionWithOwnedContents(Sec, std::move(BufferCopy));
   }
 
   return Error::success();

llvm/test/tools/llvm-objcopy/wasm/add-section.test

 # RUN: echo "123" > %t4
 # RUN: llvm-objcopy --remove-section=foo --add-section=foo=%t4 %t %t5
 # RUN: obj2yaml %t5 | FileCheck %s --check-prefix=REPLACE
 
 # REPLACE: - Type:  CUSTOM
 # REPLACE:   Name:    foo
 # REPLACE:   Payload: 3132330A
 
+## Check that raw data bytes can be imported and exported unchanged especially the ones containing empty bytes.
+# RUN: echo -e -n "\x02\x01\x00\x01\x02" > %t6
+# RUN: llvm-objcopy --add-section=bar=%t6 %t %t7
+# RUN: llvm-objcopy --dump-section=bar=%t8 %t7
+# RUN: diff %t8 %t6
+# RUN: obj2yaml %t7 | FileCheck %s --check-prefix=RAW-DATA
+
+## Check that raw data is well formated in the file as well.
+# RAW-DATA:      Name: bar
+# RAW-DATA-NEXT: Payload: '0201000102'
+
 --- !WASM
 FileHeader:
   Version: 0x00000001
 Sections:
   - Type: TYPE
     Signatures:
       - Index: 0
         ParamTypes: