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

Mon Nov 28 12:21:36 PST 2022

> I don't know what tools are installed on the builder but I'll ask around
the office on Monday, I know there are people who are more familiar with
how these bots are set up.

Sounds good. Thank you!

> FWIW I could repo promise_type failure on my macOS desktop which has the
current Xcode 14 tools installed

You mean you were able to reproduce the crash? Or were you able to
reproduce the test failure described in
https://github.com/llvm/llvm-project/commit/2b2f2f66141d52dc0d3082ddd12805d36872a189
also on your macOS desktop?

My current hypothesis is:

   1.
   https://github.com/llvm/llvm-project/commit/4346318f5c700f4e85f866610fb8328fc429319b
   failed because
   - green dragon is using an outdated pre-release version of clang
      - as commented in the test case "clang version < 16 did not yet write
      debug info for the noop coroutines."
      - the test case hence uses the condition "if not (is_clang and
      self.expectedCompilerVersion(["<", "16"])):"" to only expect check this
      test expectation on recent enough compilers
      - my understanding is that "clang-16 development builds" would pass
      this version check, even if they did not yet include
      https://reviews.llvm.org/D132580 and then fail the test expectation
      - If it turns out to be true that green dragon uses an older
      clang-version, I would propose the following course of action:
         - Somebody should upgrades the version of clang used in green
         dragon
         - I resubmit this change unchanged
      2.
   https://github.com/llvm/llvm-project/commit/cd3091a88f7c55c90d9b5fff372ce1cdfc71948d
   - was indeed buggy. This commit turned a "missing debug information"
      situation into a crash
      - I am pretty sure that Apple clang version 14.0.0 did not contain
      https://reviews.llvm.org/D132580, and is hence missing the debug
      information. This would explain the crash on your mac Desktop.
      - I would first wait after we resolved the issues around the first
      commit, though, because rebasing this commit so it can be applied before
      4346318f5c700f4e85f866610fb8328fc429319b would be cumbersome
      - I will then add the additional `if (!promise_type.isVoid())` check,
      so we no longer crash on missing debug info, and then re-apply the patch

Do you agree with that plan? Do you know somebody who can update clang on
green dragon?

Cheers,
Adrian

On Sat, Nov 26, 2022 at 3:24 AM Jason Molenda <jason at molenda.com> wrote:

> FWIW I could repo promise_type failure on my macOS desktop which has the
> current Xcode 14 tools installed, but I can't tell you exactly what this
> corresponds to in github hash terms (it's "Apple clang version 14.0.0
> (clang-1400.0.29.100)"), I think it was originally branched 2021-10-26,
> although there have been a lot of cherrypicks pulled in too, so it's not an
> exact snapshot of October 26 sources.
>
> J
>
> > On Nov 25, 2022, at 2:22 PM, Adrian Vogelsgesang <
> avogelsgesang at salesforce.com> wrote:
> >
> > 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/20221128/fff0cd0b/attachment-0001.html>