[PATCH] D91028: [llvm-objcopy][NFC] replace class Buffer/MemBuffer/FileBuffer with streams.

Thu Mar 4 00:34:16 PST 2021

jhenderson added a comment.

In D91028#2602589 <https://reviews.llvm.org/D91028#2602589>, @jhenderson wrote:

> In D91028#2599949 <https://reviews.llvm.org/D91028#2599949>, @avl wrote:
>
>>> Sorry, I've been unwell, and not been able to look more at this yet.
>>
>>
>>
>>> Also, one of our downstream users has recently run into a bug which probably impacts users of raw_fd_ostream, when writing from a Windows VM to a MacOS shared directory - they encounter "bad file descriptor" errors, in a manner simialr to https://bugs.llvm.org/show_bug.cgi?id=42623. I haven't filed a bug for this yet, as we're still trying to narrow down what is likely causing the problem, but I'm about 90% certain it is a general bug in raw_fd_ostream in this situation (copying to a memory mapped file as opposed to calling ::write works). At the moment, this bug affects llvm-objcopy when copying archives, but not objects, as the two wirting paths are different. It would be a shame if we introduced the bug into more areas, and I think this change does that. Perhaps we should try fixing that problem first?
>>
>> I think yes it would be better to fix the problem first. Though we need a reproducer for that problem to not wait forever...
>>
>> It looks like https://bugs.llvm.org/show_bug.cgi?id=42623 is already resolved in llvm by  https://reviews.llvm.org/D81803. (But it has a problem with Windows7, which caused reverting in the rust compiler https://github.com/rust-lang/rust/issues/82677)
>>
>> Please, let me known if I may help with fixing this.
>
> Ah, that might be good news. The user is using an LLVM 10 based toolchain, I believe, so wouldn't have this fix in. It's quite possible that when they upgrade to a later LLVM version, they will no longer see the problem. Unfortunately, the user is external to our company, and we don't currently have the setup to confirm the issue either way ourselves. However, by the sounds of it, if somebody does have a Windows 10 VM running using Parallels on a Mac, they should be able to reproduce by doing any generic archive copying with llvm-objcopy or writing using llvm-ar. If anybody on here has such a setup, it would be great if we can get that verification. LLVM 12 meanwhile hasn't yet been released, so I don't think it's practical (yet) for us to ask our user to try to confirm the fix for us.
>
> If nobody does have the ability to reproduce this, I don't think we need to block this review on that, given the likely chance this has been fixed. I'll take a look at this patch again later today, assuming I get a moment.

Aside: can https://bugs.llvm.org/show_bug.cgi?id=42623 be resolved now?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91028