ASTImporter patches and improvements, please help

Dear LLDB Developers,

There is an ongoing activity to improve ASTImporter in Clang to make it support
C++ (and C) completely and correctly. Our team works on cross translation unit
(CTU) static analysis where we use the in-tree Clang static analyzer in
combination with the ASTImporter (via the scan-build.py script or CodeChecker).
In the past 18 months we have committed more than 70 patches to make the
ASTImporter better. Our primary target is Linux and we do run the LLDB tests
on Linux before we commit. Unfortunately, quite recently 3 of these patches
broke the LLDB macOS buildbots (green.lab.llvm.org/green/view/LLDB) and this
caused some turmoil. We are very sorry about this and we are making actions to
avoid such inconveniences in the future: We ordered a Mac Mini, until it
arrives we are using a borrowed Mac Book. We are going to create a CI job which
will execute the macOS LLDB test suite for a specific patch. Besides this, for
every patch we are going to monitor the macOS buildbots once they are
committed.

However, we are experiencing problems both with the buildbots and with the LLDB
tests, therefore we are asking help from the LLDB community in the following:

(1) Apparently, the green lab macOS buildbots are not displayed in the build
bot console (http://lab.llvm.org:8011/console). More importantly they are not
capable of sending emails to the authors of the commit in case of failure.
Would it be possible to setup the these buildbots to send out the emails for
the authors?

(2) We are facing difficulties with the LLDB lit tests both on macOS and on
Linux. E.g. on a fresh macOS mojave install, checking out master LLVM, Clang,
LLDB and then executing ninja check-lldb fails immediately. On Linux we
experienced that some tests fail because they try to read a non-existent
register. Some tests fail non-deterministically because they can’t kill a
process or a timeout is not long enough. Some tests fail because of a linker
error of libcxx. We would like to address all these issues. Would you like to
discuss these issues on lldb-dev or should we create a bugzilla ticket for
each?

(3) ASTImporter unit tests and lit tests in Clang for the LLDB specific usage
are very-very limited. LLDB uses the so called “minimal” import, but none of
the unit tests exercises that (CTU uses the full import). We should have a
unit test suite where the LLDB use cases are covered, e.g. after a minimal
import an importDefinition is called. Also, a test double which implements
the ExternalASTSource could be used. I believe it would be possible to cover
with the unit tests all LLDB/macOS related scenarios and these tests could run
on Linux too. We do something similar in case of windows: we execute all unit
tests with “-fdelayed-template-parsing” and with “-fms-compatibility” too. I
think, the more unit tests we have in Clang the sooner we catch the LLDB
specific importer errors. I am asking the LLDB community’s help here, because
we lack the domain knowledge in LLDB so we don’t know what expectations should
we write in each unit test cases. (About lit tests, there is clang-import-test
but its coverage seems pretty low and I don’t know if that really exercises all
LLDB use cases.)

I think that from a better ASTImporter the LLDB debugger can benefit too: the
expr command might give better experience by supporting more C++ constructs
and by giving better diagnostics.

Thank you,
Gabor

Hi Gábor,

I replied inline.

Dear LLDB Developers,

There is an ongoing activity to improve ASTImporter in Clang to make it support
C++ (and C) completely and correctly. Our team works on cross translation unit
(CTU) static analysis where we use the in-tree Clang static analyzer in
combination with the ASTImporter (via the scan-build.py script or CodeChecker).
In the past 18 months we have committed more than 70 patches to make the
ASTImporter better. Our primary target is Linux and we do run the LLDB tests
on Linux before we commit. Unfortunately, quite recently 3 of these patches
broke the LLDB macOS buildbots (green.lab.llvm.org/green/view/LLDB) and this
caused some turmoil. We are very sorry about this and we are making actions to
avoid such inconveniences in the future: We ordered a Mac Mini, until it
arrives we are using a borrowed Mac Book. We are going to create a CI job which
will execute the macOS LLDB test suite for a specific patch. Besides this, for
every patch we are going to monitor the macOS buildbots once they are
committed.

Thanks a lot! And no worries about breaking the bots.

However, we are experiencing problems both with the buildbots and with the LLDB
tests, therefore we are asking help from the LLDB community in the following:

(1) Apparently, the green lab macOS buildbots are not displayed in the build
bot console (http://lab.llvm.org:8011/console). More importantly they are not
capable of sending emails to the authors of the commit in case of failure.
Would it be possible to setup the these buildbots to send out the emails for
the authors?

(2) We are facing difficulties with the LLDB lit tests both on macOS and on
Linux. E.g. on a fresh macOS mojave install, checking out master LLVM, Clang,
LLDB and then executing ninja check-lldb fails immediately. On Linux we
experienced that some tests fail because they try to read a non-existent
register. Some tests fail non-deterministically because they can't kill a
process or a timeout is not long enough. Some tests fail because of a linker
error of libcxx. We would like to address all these issues. Would you like to
discuss these issues on lldb-dev or should we create a bugzilla ticket for
each?

I would just post about them on lldb-dev and then I think we can sort
out what are issues with the local setup and what are actual LLDB bugs
that should go to the bugtracker. The LLDB setup needs some small
extra attention that I don't think is really documented anywhere, so
that's a good chance for us to do so.

(3) ASTImporter unit tests and lit tests in Clang for the LLDB specific usage
are very-very limited. LLDB uses the so called "minimal" import, but none of
the unit tests exercises that (CTU uses the full import). We should have a
unit test suite where the LLDB use cases are covered, e.g. after a minimal
import an `importDefinition` is called. Also, a test double which implements
the ExternalASTSource could be used. I believe it would be possible to cover
with the unit tests all LLDB/macOS related scenarios and these tests could run
on Linux too. We do something similar in case of windows: we execute all unit
tests with "-fdelayed-template-parsing" and with "-fms-compatibility" too. I
think, the more unit tests we have in Clang the sooner we catch the LLDB
specific importer errors. I am asking the LLDB community's help here, because
we lack the domain knowledge in LLDB so we don't know what expectations should
we write in each unit test cases. (About lit tests, there is clang-import-test
but its coverage seems pretty low and I don't know if that really exercises all
LLDB use cases.)

Yes, having full coverage of the ASTImporter in Clang would be very
nice. I started writing tests a few months back (with the help of
Aleksei who reviewed all of them!), but I currently don't have enough
time for doing the rest.

The clang-import-test tests are I think a good starting point. IIRC,
they actually do test the minimal import feature (as the first stage
of lazy building the AST). Their coverage should be ok-ish by now, but
it's obviously far from what we should have.

I think that from a better ASTImporter the LLDB debugger can benefit too: the
`expr` command might give better experience by supporting more C++ constructs
and by giving better diagnostics.

Yes, I fully agree.

Hi Gabor,

Thank you for taking care of this issue!
I completely agree with what you said. In my opinion, there are also two more things we have to care about.

1. We need to involve LLDB developers into ASTImporter reviews. While we are familiar with the ASTImporter usage in CSA, LLDB developers are much more familiar with the usage of this component in LLDB so their opinion is always appreciated. If there is anybody who can participate in the reviews - please let us know.
2. We have to get more familiar with LLDB ourself. The code related to the ASTImporter looks pretty well isolated so I think there will be no too much problems with it. The build bots breakage already made us take a look on the related components.

01.12.2018 19:00, Gábor Márton via cfe-dev пишет:

Hi Davide,

Thank you for your email.

In particular, what’s the error you get when lldb fails immediately running the tests?
Also, have you checked libcxx and libcxx-abi in your build? We might
consider making that a mandatory dependency for the Cmake build.

Finally I could run the test suite ("ninja check-lldb) on our macOS without errors. Here are my findings:

  1. “sudo /usr/sbin/DevToolsSecurity --enable” is a must.
  2. I indeed did not checked out libcxx. (libcxx-abi was not needed to have a successful run). Would be great to make that a mandatory dependency.
  3. Besides that I recognized that “ninja check-lldb” target does not build all it’s dependencies. E.g. lldb-server tests failed until I executed “ninja debugserver” before “ninja check-lldb”. I am not sure, but I think “ninja obj2yaml” and “ninja llvm-pdbutil” are also prerequisites (and these targets seem not to be built by a simple “ninja” command).

Thanks,
Gabor

Any chance you can submit fixes for this? Or, if you can't just open a
PR and I'll take a look.

Sure, I have created the reports:

https://bugs.llvm.org/show_bug.cgi?id=39909

https://bugs.llvm.org/show_bug.cgi?id=39910

And there was one already which I extended:
https://bugs.llvm.org/show_bug.cgi?id=22988

Gabor

Thanks for reporting this problem. I cc:ed Mike, as he owns the bots,
and he should be able to help us with this.
(I do have access to the Jenkins configuration but I don’t feel
confident enough to make the change myself).

Hi Davide and Mike,

I have received an email from llvm.greendragon@gmail.com about a broken job of lldb-xcode. This is definitely an improvement and I would like to thank you. (This time it seems it is not my commit which breaks the bot, it looks like this is a commit related to breakpad).

Cheers,
Gabor