Page MenuHomePhabricator

[lldb] Relax libc++ ABI version checking

Authored by thomasanderson on Jan 30 2019, 12:36 PM.



libc++ has programmable ABI versioning controllable with the _LIBCPP_ABI_VERSION
macro. Currently there are at least 3 settings used in real systems (1 as the
default, ndk1 for Anroid, Cr for Chromium).

Only the 1 and ndk1 cases were handled. This change relaxes the check to allow
any ABI version.

Diff Detail


Event Timeline

aprantl might be a good reviewer for this.

The motivation here is that we set _LIBCPP_ABI_VERSION for chromium, which then confuses lldb.

davide accepted this revision.Jan 30 2019, 1:05 PM
davide added a subscriber: davide.

As long as this doesn't break macOS, it's fine. (just run the tests and watch the bots).

This revision is now accepted and ready to land.Jan 30 2019, 1:05 PM

As long as this doesn't break macOS, it's fine. (just run the tests and watch the bots).

Latest revision fixes 2 test failures on Linux. The first was because ".+" was matching too aggressively, so I changed it to "[[:alnum:]]+". The second was because libstdc++ printers were getting loaded after libc++, but they need to go before to take precedence.
Can confirm that there's no new test failures with this change on Mac and Linux. Does the latest revision still lg?

labath added a subscriber: labath.Jan 31 2019, 10:53 AM

I was going to say that this has the potential of breaking libstdc++ formatters, but it looks like you found that out already. Since the libc++ inline namespace can really be anything, it makes sense to have the regex match that. Since libstdc++ (AFAIK) doesn't have that functionality, we can get away with it by moving it's formatters to the front. If we end up having two c++ standard libraries with configurable inline namespaces, we may have to do something smarter, like having a single regex, but then dispatching the implementation based on the presence of some member variable or something...

labath, was that an lg? :-)

labath accepted this revision.Feb 1 2019, 4:28 AM

Yes, I suppose it is :)

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2019, 11:10 AM