[llvm] c1baf94 - [ORC] Avoid invalidating iterators in EHFrameRegistrationPlugin.

[David Blaikie via llvm-commits]

On Wed, May 5, 2021 at 8:36 AM Lang Hames <lhames at gmail.com> wrote:
>
> Found by an out-of-tree user. Unfortunately this only shows up when we exercise the resource-transfer APIs, which none of our existing regression test infrastructure exercises, so there's no easy way to write a testcase for it yet.

Hrm, that's unfortunate - how'd it get committed without any test coverage?

> I'm revisiting unit tests for some of the more obscure JIT APIs (see e.g. the recent C API unit test) -- If I'm able to work the kinks out of those then I'll look at writing a unit test for this.

Cool - happy to chat about test strategies any time :)

- Dave

>
> -- Lang.
>
> On Sun, May 2, 2021 at 12:31 PM David Blaikie <dblaikie at gmail.com> wrote:
>>
>> Was this covered by existing tests/found by a novel UB failure on a buildbot or the like?
>>
>> On Sun, Apr 25, 2021 at 6:04 PM Lang Hames via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>>
>>>
>>> Author: Lang Hames
>>> Date: 2021-04-25T16:55:19-07:00
>>> New Revision: c1baf946e6cf611ae871e34db5cfea0f94f4b5a0
>>>
>>> URL: https://github.com/llvm/llvm-project/commit/c1baf946e6cf611ae871e34db5cfea0f94f4b5a0
>>> DIFF: https://github.com/llvm/llvm-project/commit/c1baf946e6cf611ae871e34db5cfea0f94f4b5a0.diff
>>>
>>> LOG: [ORC] Avoid invalidating iterators in EHFrameRegistrationPlugin.
>>>
>>> In EHFrameRegistrationPlugin::notifyTransferringResources if SrcKey had
>>> eh-frames associated but DstKey did not we would create a new entry for DskKey,
>>> invalidating the iterator for SrcKey in the process. This commit fixes that by
>>> removing SrcKey first in this case.
>>>
>>> Added:
>>>
>>>
>>> Modified:
>>>     llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
>>>
>>> Removed:
>>>
>>>
>>>
>>> ################################################################################
>>> diff  --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
>>> index dc3cf87798cf..ccac4b8c545b 100644
>>> --- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
>>> +++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
>>> @@ -654,13 +654,23 @@ Error EHFrameRegistrationPlugin::notifyRemovingResources(ResourceKey K) {
>>>  void EHFrameRegistrationPlugin::notifyTransferringResources(
>>>      ResourceKey DstKey, ResourceKey SrcKey) {
>>>    auto SI = EHFrameRanges.find(SrcKey);
>>> -  if (SI != EHFrameRanges.end()) {
>>> +  if (SI == EHFrameRanges.end())
>>> +    return;
>>> +
>>> +  auto DI = EHFrameRanges.find(DstKey);
>>> +  if (DI != EHFrameRanges.end()) {
>>>      auto &SrcRanges = SI->second;
>>> -    auto &DstRanges = EHFrameRanges[DstKey];
>>> +    auto &DstRanges = DI->second;
>>>      DstRanges.reserve(DstRanges.size() + SrcRanges.size());
>>>      for (auto &SrcRange : SrcRanges)
>>>        DstRanges.push_back(std::move(SrcRange));
>>>      EHFrameRanges.erase(SI);
>>> +  } else {
>>> +    // We need to move SrcKey's ranges over without invalidating the SI
>>> +    // iterator.
>>> +    auto Tmp = std::move(SI->second);
>>> +    EHFrameRanges.erase(SI);
>>> +    EHFrameRanges[DstKey] = std::move(Tmp);
>>>    }
>>>  }
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits