[Lldb-commits] green dragon testsuite crash on TestCoroutineHandle.py from coroutines formatter changes, temporarily revert?

[Adrian Vogelsgesang via lldb-commits]

Hm... wait a second...

Which clang-version is
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/ using? Is it
using an older version of clang? Does that version already contain the
patch https://reviews.llvm.org/D132580?

If not, that might explain things...

On Fri, Nov 25, 2022 at 11:18 PM Adrian Vogelsgesang <
avogelsgesang at salesforce.com> wrote:

> Hi Jason,
>
> Sorry for the late reply - your mail got caught in my spam filter, and I
> just realized about the build breakage now after you reverted the commits.
> Just to confirm:
>
> I looked at the original change again, and I think I know what's going
> wrong here: Devirtualization fails, and `promise_type` stays `void`.
> Obviously, `CreateValueObjectFromAddress` cannot create an object of type
> `void` and fails. The previous version was robust against that scenario,
> but https://reviews.llvm.org/D132815 accidentally regressed this.
>
> If a program has all the expected debug info, devirtualization should
> never fail and `promise_type` never stays `void`. It seems something is
> wrong/unexpected with the debug info on Darwin. Devirtualization relies on
> being able to identify the function pointer of the `destroy` function and
> inspects that function. So the root cause seems to be the same as for the
> revert
> https://github.com/llvm/llvm-project/commit/2b2f2f66141d52dc0d3082ddd12805d36872a189:
> For some reason, the debugger cannot find debug info for the function
> pointed to by the `destroy` function.
>
> However, I still don't quite understand what's wrong with the debug info
> on Mac.
>
> Either way, the pretty printer should of course be robust against such
> unexpected debug info. I think a simple additional check should be enough
> to make this robust so we no longer crash:
> if (!promise_type.isVoid()) { // <---- additional check
>   lldb::ValueObjectSP promise = CreateValueObjectFromAddress(
>       "promise", frame_ptr_addr + 2 * ptr_size, exe_ctx, promise_type);
>   Status error;
>   lldb::ValueObjectSP promisePtr = promise->AddressOf(error);
>   if (error.Success())
>     m_promise_ptr_sp = promisePtr->Clone(ConstString("promise"));
> }
>
> Meta question: I am a bit confused about
> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/. I thought
> LLVM's buildbots were set up to send an email in case somebody breaks the
> build. Also, I made sure that all builds in
> https://lab.llvm.org/buildbot/#/builders stayed green after my commit. So
> what are the expectations around which CIs need to stay green after a
> commit? Do I need to setup anything such that green.lab.llvm.org also
> sends me an email if I break it?
>
> Cheers,
> Adrian
>
> On Thu, Nov 24, 2022 at 10:50 PM Jason Molenda <jason at molenda.com> wrote:
>
>> Ah, little misstatement below.  My first thought was that promise_type
>> was null, but when I stepped through here with a debugger, it was not.  I
>> didn't know how to dig in to a CompilerType object but there was some
>> reason why this CreateValueObjectFromAddress failed to return an actual
>> ValueObject, and I assumed it's something to do with that CompilerType.
>> But I didn't dig in far enough to understand what the issue in the type was.
>>
>> > On Nov 24, 2022, at 1:20 PM, Jason Molenda <jason at molenda.com> wrote:
>> >
>> > Hi Adrian, the green dragon Incremental CI bot has been failing the
>> past couple of days after the changes for the coroutines formatter, most
>> directly   https://reviews.llvm.org/D132815 landed -
>> TestCoroutineHandle.py results in a segfault on Darwin systems (both on the
>> CI and on my mac desktop) consistently.
>> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/
>> >
>> > It's crashing in formatters::StdlibCoroutineHandleSyntheticFrontEnd
>> where you're doing
>> >
>> > ```
>> >  // Add the `promise` member. We intentionally add `promise` as a
>> pointer type
>> >  // instead of a value type, and don't automatically dereference this
>> pointer.
>> >  // We do so to avoid potential very deep recursion in case there is a
>> cycle in
>> >  // formed between `std::coroutine_handle`s and their promises.
>> >  lldb::ValueObjectSP promise = CreateValueObjectFromAddress(
>> >      "promise", frame_ptr_addr + 2 * ptr_size, exe_ctx, promise_type);
>> >  Status error;
>> >  lldb::ValueObjectSP promisePtr = promise->AddressOf(error);
>> > ```
>> >
>> > and the promise_type dose not have a CompilerType so promisePtr is
>> nullptr and we crash on the method call to AddressOf. I looked briefly, but
>> this isn't a part of lldb I know well and I'm not sure the nature of the
>> problem.
>> >
>> > It's a long weekend in the US, so if you have a suggestion for
>> addressing this that'd be great, or I can roll back the changes necessary
>> to get the bot clean later today and we can look at this next week.
>> >
>> > Thanks!
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20221125/79cf4fbc/attachment-0001.html>