Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-83735

Dumpcpp name collision leads to undefined symbols when method names clash with QAxBase's ones

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P4: Low
    • Resolution: Done
    • Affects Version/s: 5.14.1
    • Fix Version/s: 6.1.1, 6.2.0 Alpha
    • Component/s: ActiveX Support
    • Labels:
      None
    • Platform/s:
      Windows
    • Commits:
      ec019ce68cf70d42d9857711923f28e79a07656e (qt/qtactiveqt/dev)

      Description

      I used dumpcpp to import a typelib of ours that contains a method named clear().

      DumpCPP dutifully wrote a definition of this interface, as

      // code placeholder
      class ECDIFFENGINELIB_EXPORT IFileSet : public QAxObject
      {
          /*
          Method clear
          */
          inline void clear();
      }
      #ifndef QAX_DUMPCPP_ECDIFFENGINELIB_NOINLINES
      
      inline void IFileSet::clear()
      {
          void *_a[] = {0};
          qt_metacall(QMetaObject::InvokeMetaMethod, 9, _a);
      }
      
      #endif
      

       

      So, problem one is that this is inadvertently an override of

      virtual QAxBase::clear()
      

       

      But the second problem is that, as an inline definition of a virtual method, it would seldom actually be inlined, and would usually be called through the vtbl, and instantiated only by the pointer within. The vtbl will in turn usually only be instantiated by one translation unit, whichever one the compiler contains the definition of the key function; i.e. the first non-pure virtual function that is not inline at the point of class definition. At least, I think MSVC uses that same itanium ABI rule that clang/g++ does for this (but I can't find a reference). In any case it seems to be picking the ecdiffenginelib.cpp that dumpcpp created, which makes sense by that rule, as it contains IFileSet::IFileSet().

      But there is a usually-benign (but not this time) ODR-ish issue here. I'm not sure it's strictly an ODR violation, but it's that sort of issue. That is, ecdiffenginelib.cpp has set QAX_DUMPCPP_ECDIFFENGINELIB_NOINLINES. So the only place the compiler would have instantiated the inline function, is a place where its definition was deleted by the preprocessor. Everywhere else is going to see that they don't have a definition of the key method, and while they do have an inline definition for the method body, unless they actually want to inline it they're going to just make an linker reference to the one out-of-line definition they assume some other translation unit will have written. So the end result is that nobody emits it, and you get an undefined symbol at link time, due to the missing definition in in the .cpp that was supposed to have instantiated it. Other places do have a definition, but I'm not sure how to make one of them emit it, because any attempt to call it, take a member function pointer, etc, is going to end up going referencing the vtbl instead of actually inlining it.

      Even if the linker issue could was fixed, though, we'd still have the inadvertent and unwanted override of QAxBase::clear. So I'm not sure the definition issue really needs a solution, it's just the long explanation of a very weird symptom; I think that dumpcpp needs a way to rename imported names that clash with its inherited method names (or even better, a built-in list of reserved member names that it just automatically appends prefixes or suffixes somehow to prevent clashes. In that sense, this is kind of adjacent to QTBUG-48895, in that I think he way forward is a way to rename the imported method, though for somewhat different reasons...

       

        Attachments

          Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

            Activity

              People

              Assignee:
              kleint Friedemann Kleint
              Reporter:
              puetzk Kevin Puetz
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Gerrit Reviews

                  There are no open Gerrit changes