[llvm] r268111 - [Orc] Make sure we don't drop the internal error in OrcRemoteTargetClient when

[David Blaikie via llvm-commits]

On Fri, Apr 29, 2016 at 4:49 PM, Lang Hames <lhames at gmail.com> wrote:

> Hi Dave,
>
> The noisy error checking guarantees can only check code that's traversed
> at runtime, and this is the first time we'd traversed this particular
> error-dropping path.
>
> Specifically: ORCRemoteTarget has an 'ExistingError' field that all
> clients were checking correctly, but when the constructor failed today (the
> first time this had happened) none of the clients got a copy of
> ORCRemoteTargetClient to check - it was destructed inside the
> ORCRemoteTargetClient::Create method, and its (unchecked) ExistingError
> field blew up.
>

The usual follow-up - is this being/can this be tested (it's not clear
(perhaps I've missed the details in other patches) whether "today" means
"in some out of tree/untested use" or "in a case I committed earlier but
didn't notice the failure until the buildbots ran it"?)?


>
> I don't like the Error-as-member style that ORCRemoteTargetClient is using
> - I feel like it's prone to these kinds of mistakes, but I haven't got a
> better solution yet.
>
> - Lang.
>
> On Fri, Apr 29, 2016 at 3:35 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> was there some reason this wasn't caught before by the noisy checked
>> error handling?
>>
>> On Fri, Apr 29, 2016 at 2:29 PM, Lang Hames via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Author: lhames
>>> Date: Fri Apr 29 16:29:48 2016
>>> New Revision: 268111
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=268111&view=rev
>>> Log:
>>> [Orc] Make sure we don't drop the internal error in
>>> OrcRemoteTargetClient when
>>> the constructor fails, as this would lead to an 'unchecked error' crash.
>>>
>>> Modified:
>>>     llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h
>>>
>>> Modified:
>>> llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h?rev=268111&r1=268110&r2=268111&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h
>>> (original)
>>> +++ llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h
>>> Fri Apr 29 16:29:48 2016
>>> @@ -693,8 +693,9 @@ private:
>>>        std::tie(RemoteTargetTriple, RemotePointerSize, RemotePageSize,
>>>                 RemoteTrampolineSize, RemoteIndirectStubSize) = *RIOrErr;
>>>        Err = Error::success();
>>> -    } else
>>> -      Err = RIOrErr.takeError();
>>> +    } else {
>>> +      Err = joinErrors(RIOrErr.takeError(), std::move(ExistingError));
>>> +    }
>>>    }
>>>
>>>    Error deregisterEHFrames(TargetAddress Addr, uint32_t Size) {
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160429/341ef362/attachment.html>