[llvm-objcopy] Fix RPathLC.cmdsize

[sameerarora101]

Summary

Add 1 to the cmdsize field under buildRPathLoadCommand in
MachOObjcopy.cpp to account for the NUL terminator. For
reference: the issue is highlighted in comments under
https://reviews.llvm.org/D81527.

[smeenai]

The change looks good, but it'd be helpful for the commit message to just contain the details about the issue directly instead of pointing to another review. (The + 1 is to account for the NUL terminator, as far as I can tell.)

[sameerarora101]

Got it thanks, i'll update it

[smeenai]

LGTM.

[smeenai]

Hmm, actually, I think it should be possible to add a test for this? You'd need an rpath where the length of the string (not including the terminating NUL) was exactly a multiple of 8 for this to make a difference.

Resigning as reviewer to clear my accept.

[sameerarora101]

Adding a test case for the change. String "abcd" has size 4 and sizeof(MachO::rpath_command) = 12. Since the sizes add up to 16, cmdsize should be assigned a size of 24 bytes if we take the NULL terminator into account.

[alexander-shaposhnikov]

LGTM

[jhenderson]

LGTM, with nit.

llvm/test/tools/llvm-objcopy/MachO/install-name-tool-add-rpath.test

25

if -> that

(more clearly indicates that it is intended that it does account for the terminator)

[sameerarora101]

if -> that

llvm/test/tools/llvm-objcopy/MachO/install-name-tool-add-rpath.test

 # RUN: | FileCheck --check-prefix=DUPLICATE-RPATH %s
 
 # DUPLICATE-RPATH: duplicate load command
 
 # RUN: not llvm-install-name-tool -add_rpath @executable_path/. 2>&1 \
 # RUN: | FileCheck --check-prefix=NO-INPUT %s
 
 # NO-INPUT: no input file specified
+
+## Check that cmdsize accounts for NULL terminator.
+# RUN: yaml2obj %p/Inputs/x86_64.yaml -o %t.x86_64
+# RUN: llvm-install-name-tool -add_rpath abcd %t.x86_64
+# RUN: llvm-objdump -p %t.x86_64 | FileCheck %s --check-prefix=RPATH-SIZE
+
+# RPATH-SIZE: cmd LC_RPATH
+# RPATH-SIZE-NEXT: cmdsize 24
+# RPATH-SIZE-NEXT: path abcd

llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp

static void updateAndRemoveSymbols(const CopyConfig &Config, Object &Obj) {
   Obj.SymTable.removeSymbols(RemovePred);
 }
 
 static LoadCommand buildRPathLoadCommand(StringRef Path) {
   LoadCommand LC;
   MachO::rpath_command RPathLC;
   RPathLC.cmd = MachO::LC_RPATH;
   RPathLC.path = sizeof(MachO::rpath_command);
-  RPathLC.cmdsize = alignTo(sizeof(MachO::rpath_command) + Path.size(), 8);
+  RPathLC.cmdsize = alignTo(sizeof(MachO::rpath_command) + Path.size() + 1, 8);
   LC.MachOLoadCommand.rpath_command_data = RPathLC;
   LC.Payload.assign(RPathLC.cmdsize - sizeof(MachO::rpath_command), 0);
   std::copy(Path.begin(), Path.end(), LC.Payload.begin());
   return LC;
 }
 
 static Error dumpSectionToFile(StringRef SecName, StringRef Filename,
                                Object &Obj) {