[PATCH] D120073: [LLD] Fix for race condition.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D120073#3330269 <https://reviews.llvm.org/D120073#3330269>, @ayermolo wrote:

> In D120073#3330235 <https://reviews.llvm.org/D120073#3330235>, @MaskRay wrote:
>
>> This will harm performance. If only `elf::getErrorPlace` is problematic, we can add a mutex there.
>>
>>> There is a potential race condition when getErrPlace is invoked in multithreaded context
>>
>> How?
>
> Well problem fundamentally in uncompress. It already has a mutex. Which tells to me there was some thought that was put into making it multi threading safe, but check is not complete, as it doesn't handle uncompressedSize/rawData pointer.
> As for how. Eh... There is some internal code I inherited that exposes this issue, and zlib error hid the real error of relocation overflow.

If it's your internal code and not upstream code, I feel uneasy to make the change since it will hurt performance.

If your internal code needs to call `data()` on one input section concurrently, consider adding a separate pass (undo D52917 <https://reviews.llvm.org/D52917>) uncompressing section contents upfront?

For an upstreamable patch, there are many directions:

- just undo D52917 <https://reviews.llvm.org/D52917> (if the original performance claim can be mitigated via other means)
- figure out whether `isec->data()` for uncompressed buffers in getErrorPlace can be avoided


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120073