[PATCH] D82051: [llvm-install-name-tool] Add -rpath option

[Sameer Arora via Phabricator via llvm-commits]

sameerarora101 marked 5 inline comments as done.
sameerarora101 added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:124-125
+  LC->MachOLoadCommand.load_command_data.cmdsize = NewCmdsize;
+  LC->Payload.assign(NewCmdsize - sizeof(T), 0);
+  std::copy(NewName.begin(), NewName.end(), LC->Payload.begin());
+}
----------------
jhenderson wrote:
> sameerarora101 wrote:
> > jhenderson wrote:
> > > These two lines can be folded together I believe using `std::back_insertor` in the `std::copy` line, right?
> > So I tried folding the 2 lines into `std::copy(NewName.begin(), NewName.end(), std::back_insertor(LC->Payload));` but it fails the rpath test giving the following error: `truncated or malformed object (load command 1 extends past end of file)`.
> > 
> > Surprisingly, even `LC->Payload.assign(NewName.begin(), NewName.end());` didn't work (giving the same error as above).
> > 
> > However, the initial approach seems to work lol, not sure what's the difference in behavior from these new approaches?
> That error message doesn't really give me enough info to understand what's going on. Did you perhaps write one too many bytes somewhere/not write the null byte? Regarding the `copy`, you'd also need to `clear` beforehand, to avoid it just appending the data to the end, so actually `assign` is probably want you want anyway, but I believe you may need to push a null byte to the end too, i.e.
> ```
> LC->Payload.assign(NewName.begin(), NewName.end());
> LC->Payload.push_back('\0');
> ```
Oh I see now why it wasn't working - I didn't align the Payload to a multiple of 8. `NewCmd` has been aligned to a multiple of 8 as
```
uint32_t NewCmdsize = alignTo(sizeof(T) + NewName.size() + 1, 8);
```
So using the following approach
```
LC->Payload.assign(NewName.begin(), NewName.end());
LC->Payload.push_back('\0');
```
still won't be sufficient as we need to align it to multiple of 8. (Something like pushing `'\0'` for `NewCmdSize - sizeof(T) - NewName.size()` times)

I couldn't find a cleaner way of doing it beside the current implementation itself, what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82051