Page MenuHomePhabricator

Centralize libc++ test skipping logic
ClosedPublic

Authored by labath on Mar 15 2017, 8:39 AM.

Details

Summary

This aims to replace the different decorators we've had on each libc++
test with a single solution. Each libc++ will be assigned to the
"libc++" category and a single central piece of code will decide whether
we are actually able to run libc++ test in the given configuration by
enabling or disabling the category (while giving the user the
opportunity to override this).

I started this effort because I wanted to get libc++ tests running on
android, and none of the existing decorators worked for this use case:

  • skipIfGcc - incorrect, we can build libc++ executables on android with gcc (in fact, after this, we can now do it on linux as well)
  • lldbutil.skip_if_library_missing - this checks whether libc++.so is loaded in the proces, which fails in case of a statically linked libc++ (this makes copying executables to the remote target easier to manage).

To make this work I needed to split out the pseudo_barrier code from the
force-included file, as libc++'s atomic does not play well with gcc on
linux, and this made every test fail, even though we need the code only
in the threading tests.

So far, I am only annotating one of the tests with this category. If
this does not break anything, I'll proceed to update the rest.

Diff Detail

Repository
rL LLVM

Event Timeline

labath created this revision.Mar 15 2017, 8:39 AM

Does anyone object to me landing this? I am going to be careful and wait for the change to trickle through CI before submitting any followup changes.

EricWF edited edge metadata.Mar 27 2017, 7:46 PM

I don't see anything wrong with this, but I only know libc++ and not LLDB.

libc++'s atomic does not play well with gcc on linux

It should... Can you elaborate on this issue? I suspect this may be a libc++ bug.

I don't see anything wrong with this, but I only know libc++ and not LLDB.

I'm not sure why you ended up here. I think you have too wide phabricator filter somewhere. :)

libc++'s atomic does not play well with gcc on linux

It should... Can you elaborate on this issue? I suspect this may be a libc++ bug.

It checks for #if __has_feature(cxx_atomic) and then aborts if it is not found. However, this is in the system libc++ on ubuntu (lldb uses system libc++ for reasons which are too long to go into right now), which is quite old. I don't see a similar check in the current trunk, so it's possible that has been fixed since then.

I don't see anything wrong with this, but I only know libc++ and not LLDB.

I'm not sure why you ended up here. I think you have too wide phabricator filter somewhere. :)

libc++'s atomic does not play well with gcc on linux

It should... Can you elaborate on this issue? I suspect this may be a libc++ bug.

It checks for #if __has_feature(cxx_atomic) and then aborts if it is not found. However, this is in the system libc++ on ubuntu (lldb uses system libc++ for reasons which are too long to go into right now), which is quite old. I don't see a similar check in the current trunk, so it's possible that has been fixed since then.

GCC <atomic> support has been present since ~2015, so I suspect the issue has since been fixed. And the libc++ version on <ubuntu> is unbelievable old.

This revision was automatically updated to reflect the committed changes.