Project

General

Profile

Bug #14005

ABI types not available in libc++/libc++abi for maybeCastObj

Added by Ben Morgan almost 4 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Paul Russo
Target version:
Start date:
10/03/2016
Due date:
% Done:

100%

Estimated time:
1.00 h
Spent time:
Scope:
Internal
Experiment:
-
SSI Package:
Duration:

Description

Compiling canvas on macOS (El Cap, Xcode 7, 8) results in a compilation error for maybeCastObj:

[ 33%] Building CXX object canvas/Persistency/Common/CMakeFiles/canvas_Persistency_Common.dir/detail/maybeCastObj.cc.o
/.../canvas/Persistency/Common/detail/maybeCastObj.cc:51:1: error: variable has incomplete type
      'void'
visit_class_for_upcast(abi::__class_type_info const* ci,
^
/.../canvas/Persistency/Common/detail/maybeCastObj.cc:51:47: error: expected ')'
visit_class_for_upcast(abi::__class_type_info const* ci,
                                              ^
/.../canvas/Persistency/Common/detail/maybeCastObj.cc:51:23: note: to match this '('
visit_class_for_upcast(abi::__class_type_info const* ci,
                      ^
/.../canvas/Persistency/Common/detail/maybeCastObj.cc:51:29: error: no member named
      '__class_type_info' in namespace '__cxxabiv1'
visit_class_for_upcast(abi::__class_type_info const* ci,
                       ~~~~~^
/.../canvas/Persistency/Common/detail/maybeCastObj.cc:53:56: error: expected ';' after top level
      declarator
                       long offset, upcast_result& res)
                                                       ^
                                                       ;
/.../canvas/Persistency/Common/detail/maybeCastObj.cc:120:36: error: no type named
      '__class_type_info' in namespace '__cxxabiv1'
  auto ci_from = dynamic_cast<abi::__class_type_info const*>(&tid_from);
                              ~~~~~^
/.../canvas/Persistency/Common/detail/maybeCastObj.cc:120:53: error: expected '>'
  auto ci_from = dynamic_cast<abi::__class_type_info const*>(&tid_from);
                                                    ^
                                                    >
/.../canvas/Persistency/Common/detail/maybeCastObj.cc:120:30: note: to match this '<'
  auto ci_from = dynamic_cast<abi::__class_type_info const*>(&tid_from);
                             ^
/.../canvas/Persistency/Common/detail/maybeCastObj.cc:121:34: error: no type named
      '__class_type_info' in namespace '__cxxabiv1'
  auto ci_to = dynamic_cast<abi::__class_type_info const*>(&tid_to);
                            ~~~~~^
/.../canvas/Persistency/Common/detail/maybeCastObj.cc:121:51: error: expected '>'
  auto ci_to = dynamic_cast<abi::__class_type_info const*>(&tid_to);
                                                  ^
                                                  >
/.../canvas/Persistency/Common/detail/maybeCastObj.cc:121:28: note: to match this '<'
  auto ci_to = dynamic_cast<abi::__class_type_info const*>(&tid_to);
                           ^
8 errors generated.

I've tracked that down, I think, to libc++/libc++abi not making the __class_type_info and similar types publicly visible to clients. See these bugs for further detail:

https://bugs.documentfoundation.org/show_bug.cgi?id=97712
https://llvm.org/bugs/show_bug.cgi?id=26562

So far I've not gone further into the Itanium ABI to see if this is an oversight (the LLVM bug suggests it is but is not conclusive).

This use of direct ABI calls was introduced in 90b42 but the commit message is not enlightening to me on why the changes was introduced. I've confirmed that protecting the new code in #ifdefs on __GLIBCXX__ and including the old implementation inside #elif defined(_LIBCPPABI_VERSION) works and passes all tests on macOS and Linux.

However, I'm not sure whether that will play nicely with further code, so I'd like to understand better why 90b42 was implemented using the low level ABI stuff before going further.


Related issues

Related to canvas - Bug #18075: ABI types not available (again) after canvas/canvas_root_io refactoringClosed10/30/2017

History

#1 Updated by Kyle Knoepfel almost 4 years ago

  • Status changed from New to Accepted

The changes were introduced in order to avoid the extra memory overhead from ROOT's auto-parsing behavior. We agree with the proposed solution and will implement as appropriate. However, implementing this solution means that ROOT auto-parsing will occur for native clang builds.

#2 Updated by Kyle Knoepfel over 3 years ago

  • Status changed from Accepted to Assigned
  • Assignee set to Paul Russo
  • Estimated time set to 1.00 h

#3 Updated by Kyle Knoepfel about 3 years ago

  • Target version set to 1209

#4 Updated by Paul Russo about 3 years ago

  • Status changed from Assigned to Resolved
  • % Done changed from 0 to 100

We have restored the old implementation which uses root to perform upcasts in the case of macOS.

The newer implementation using the C++ ABI functions was done to avoid root autoparsing. The cost of the autoparsing is not so high that we absolutely must avoid it, so for the macOS it is best to do things the old way until the clang/llvm hiding of the abi functions is sorted out.

#5 Updated by Kyle Knoepfel about 3 years ago

  • Target version changed from 1209 to 2.07.01

#6 Updated by Kyle Knoepfel about 3 years ago

  • Status changed from Resolved to Closed

#7 Updated by Ben Morgan over 2 years ago

With the split of the ROOT part of canvas into a separate library, this issue has reared its head again. I've been able to resolve it using the techniques outlined in the LLVM/LibreOffice bug reports - canvas then compiles and all tests pass on macOS Sierra with Xcode 9. I can submit this patch, but first want to be sure on the testing.

However, looking at source:canvas/tests, I can't find explicit test cases for the functions in source:canvas/Persistency/Common/detail/maybeCastObj.h. It is used elsewhere, so I wanted to check if the tests exercise it sufficiently, or if further ones would be useful. In the latter case, I can implement these as part of the patch, but would need some guidance on the expected success/failure behaviour to check.

#8 Updated by Kyle Knoepfel over 2 years ago

We have opened issue #18075 to track work on the new behavior you observe.

#9 Updated by Kyle Knoepfel over 2 years ago

  • Related to Bug #18075: ABI types not available (again) after canvas/canvas_root_io refactoring added


Also available in: Atom PDF