This is an archive of the discontinued LLVM Phabricator instance.

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.

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.