Use castxml on headers instead of special sizes.cc

Figuring out which classes are part of the public API by using library symbols is fragile (dependent on specific compiler optimizations) and unreliable (misses some inline things). Instead, use castxml, a tool that parses C++ to an abstract syntax tree and generates XML, to get a reliable accounting of public classes and their sizes.

Use castxml on headers instead of special sizes.cc
Figuring out which classes are part of the public API by using library symbols is fragile (dependent on specific compiler optimizations) and unreliable (misses some inline things). Instead, use castxml, a tool that parses C++ to an abstract syntax tree and generates XML, to get a reliable accounting of public classes and their sizes.
Jay Berkenbilt
1 parent 9367eb8a
Showing 11 changed files with 102 additions and 228 deletions
CMakeLists.txt
README-maintainer.md
abi-perf-test
cSpell.json
check_abi
job.sums
libqpdf/CMakeLists.txt
manual/installation.rst
manual/release-notes.rst
qpdf/CMakeLists.txt
qpdf/sizes.cc
@@ -39,9 +39,6 @@ CMAKE_DEPENDENT_OPTION(
   WERROR "Treat compiler warnings as errors" OFF
   "NOT MAINTAINER_MODE; NOT CI_MODE" ON)
 CMAKE_DEPENDENT_OPTION(
-  CHECK_SIZES "Compare sizes.cc with classes in public API" OFF
-  "NOT MAINTAINER_MODE" ON)
-CMAKE_DEPENDENT_OPTION(
   GENERATE_AUTO_JOB "Automatically regenerate job files" OFF
   "NOT MAINTAINER_MODE" ON)
 CMAKE_DEPENDENT_OPTION(
@@ -862,6 +862,8 @@ RelWithDebInfo when using external-libs.
 ## ABI checks
+Note: the check_abi program requires [castxml](https://github.com/CastXML/CastXML) to be installed.
+
 Until the conversion of the build to cmake, we relied on running the
 test suite with old executables and a new library. When QPDFJob was
 introduced, this method got much less reliable since a lot of public
@@ -891,9 +893,7 @@ still things that could potentially break ABI, such as
 Not breaking ABI/API still requires care.
 The check_abi script is responsible for performing many of these
-steps. See comments in check_abi for additional notes. Running
-"check_abi check-sizes" is run by ctest on Linux when CHECK_SIZES is
-on.
+steps. See comments in check_abi for additional notes.
 ## CODE FORMATTING
@@ -59,7 +59,7 @@ cmake --build build -j$(nproc)
 echo "** saving old library and size information **"
-./build/qpdf/sizes >| $work/old/sizes
+$source/check_abi get-sizes --include include >| $work/old/sizes
 cp build/libqpdf/libqpdf.so.*.* $work/old
 if [ "$SKIP_PERF" != "1" ]; then
@@ -74,12 +74,11 @@ git checkout abi-new
 cmake -S . -B build \
   -DMAINTAINER_MODE=1 -DBUILD_STATIC_LIBS=0 -DBUILD_DOC=0 \
   -DCMAKE_BUILD_TYPE=RelWithDebInfo
-cmake --build build -j$(nproc) --target sizes
+cmake --build build -j$(nproc) --target libqpdf
 echo "** saving new library and size information **"
-$source/check_abi check-sizes --lib build/libqpdf/libqpdf.so
-./build/qpdf/sizes >| $work/new/sizes
+$source/check_abi get-sizes --include include >| $work/new/sizes
 cp build/libqpdf/libqpdf.so.*.* $work/new
 echo "** running ABI comparison **"
@@ -61,6 +61,7 @@
     "bufsize",
     "buildrules",
     "calledgetallpages",
+    "castxml",
     "ccase",
     "ccitt",
     "cdef",
@@ -4,9 +4,10 @@ import sys
 import argparse
 import subprocess
 import re
+import xml.etree.ElementTree as ET
+from operator import itemgetter
 whoami = os.path.basename(sys.argv[0])
-whereami = os.path.dirname(os.path.realpath(__file__))
 def warn(*args, **kwargs):
@@ -18,8 +19,8 @@ class Main:
         options = self.parse_args(args, prog)
         if options.action == 'dump':
             self.dump(options)
-        elif options.action == 'check-sizes':
-            self.check_sizes(options)
+        elif options.action == 'get-sizes':
+            self.get_sizes(options)
         elif options.action == 'compare':
             self.compare(options)
         else:
@@ -43,10 +44,12 @@ class Main:
             help='dump qpdf symbols in a library')
         p_dump.add_argument(lib_arg[0], **lib_arg[1])
-        p_check_sizes = subparsers.add_parser(
-            'check-sizes',
-            help='check consistency between library and sizes.cc')
-        p_check_sizes.add_argument(lib_arg[0], **lib_arg[1])
+        p_get_sizes = subparsers.add_parser(
+            'get-sizes',
+            help='dump sizes of all public classes')
+        p_get_sizes.add_argument('--include',
+                                 help='include path',
+                                 required=True)
         p_compare = subparsers.add_parser(
             'compare',
@@ -103,60 +106,73 @@ class Main:
         for i in sorted(self.get_symbols(options.lib)):
             print(i)
-    def check_sizes(self, options):
-        # Make sure that every class with methods in the public API
-        # appears in sizes.cc either explicitly ignored or in a
-        # print_size call. This enables us to reliably test whether
-        # any object changed size by following the ABI checking
-        # procedures outlined in README-maintainer.
-
-        # To keep things up to date, whenever we add or remove
-        # objects, we have to update sizes.cc. The check-sizes option
-        # can be run at any time on an up-to-date build.
-
-        lib = self.get_symbols(options.lib)
-        classes = set()
-        for i in sorted(lib):
-            # Find a symbol that looks like a class method.
-            m = re.match(
-                r'(((?:^\S*?::)?(?:[^:\s]+))::([^:\s]+))(?:\[[^\]]+\])?\(',
-                i)
-            if m:
-                full = m.group(1)
-                clas = m.group(2)
-                method = m.group(3)
-                if full.startswith('std::') or method.startswith('~'):
-                    # Sometimes std:: template instantiations make it
-                    # into the library. Ignore those. Also ignore
-                    # classes whose only exported method is a
-                    # destructor.
-                    continue
-                # Otherwise, if the class exports a method, we
-                # potentially care about changes to its size, so add
-                # it.
-                classes.add(clas)
-        in_sizes = set()
-        # Read the sizes.cc to make sure everything's there.
-        with open(os.path.join(whereami, 'qpdf/sizes.cc'), 'r') as f:
-            for line in f.readlines():
-                m = re.search(r'^\s*(?:ignore_class|print_size)\((.*?)\)',
-                              line)
-                if m:
-                    in_sizes.add(m.group(1))
-        sizes_only = in_sizes - classes
-        classes_only = classes - in_sizes
-        if sizes_only or classes_only:
-            if sizes_only:
-                print("classes in sizes.cc but not in the library:")
-                for i in sorted(sizes_only):
-                    print(' ', i)
-            if classes_only:
-                print("classes in the library but not in sizes.cc:")
-                for i in sorted(classes_only):
-                    print(' ', i)
-            exit(f'{whoami}: mismatch between library and sizes.cc')
-        else:
-            print(f'{whoami}: sizes.cc is consistent with the library')
+    @staticmethod
+    def get_header_sizes(include, filename):
+        print(f'{filename}...', file=sys.stderr)
+        p = subprocess.run([
+            'castxml', '--castxml-output=1', f'-I{include}',
+            f'{include}/{filename}', '-o', '-',
+        ], stdout=subprocess.PIPE, check=True)
+        tree = ET.fromstring(p.stdout)
+        this_file = tree.find(f'.//File[@name="{include}/{filename}"]')
+        if this_file is None:
+            # This file doesn't define anything, e.g., DLL.h
+            return []
+        this_file_id = this_file.attrib["id"]
+        wanted = [
+            'Namespace',
+            'Struct', 'Union', 'Class',  # records
+        ]
+
+        by_id = {}
+        for elem in tree:
+            # Reference
+            # https://github.com/CastXML/CastXML/blob/master/doc/manual/castxml.xsd
+            if elem.tag not in wanted:
+                continue
+            is_record = elem.tag != 'Namespace'
+            record = {
+                'is_record': is_record,
+                'in_file': (elem.get('file') == this_file_id and
+                            elem.get('incomplete') is None),
+                'name': elem.get('name'),
+                'access':  elem.get('access'),
+                'context': elem.get('context'),
+                'size': elem.get('size'),
+            }
+            by_id[elem.attrib['id']] = record
+        classes = []
+        for id_, record in by_id.items():
+            cur = record
+            if not cur['in_file']:
+                continue
+            name = ''
+            private = False
+            while cur is not None:
+                if cur.get('access') == 'private':
+                    private = True
+                parent = cur.get('context')
+                name = f'{cur["name"]}{name}'
+                if parent is None or parent == '_1':
+                    break
+                name = f'::{name}'
+                cur = by_id.get(cur.get('context'))
+            if not private:
+                classes.append([name, record['size']])
+        return classes
+
+    def get_sizes(self, options):
+        classes = []
+        for f in os.listdir(f'{options.include}/qpdf'):
+            if f.startswith('auto_') or f == 'QPDFObject.hh':
+                continue
+            if f.endswith('.h') or f.endswith('.hh'):
+                classes.extend(self.get_header_sizes(
+                    options.include, f"qpdf/{f}"))
+
+        classes.sort(key=itemgetter(0))
+        for c, s in classes:
+            print(c, s)
     def read_sizes(self, filename):
         sizes = {}
@@ -217,6 +233,12 @@ class Main:
 if __name__ == '__main__':
     try:
+        subprocess.run(['castxml', '--version'],
+                       stdout=subprocess.DEVNULL,
+                       stderr=subprocess.DEVNULL)
+    except Exception:
+        exit('f{whoami}: castxml must be installed')
+    try:
         Main().main()
     except KeyboardInterrupt:
         exit(130)
 # Generated by generate_auto_job
-CMakeLists.txt f0819695e4867e4f4389d38b0c124e79aa3ec9ace50f16ad8c751ff7f1ec6690
+CMakeLists.txt 88e8974a8b14e10c941a4bb04ff078c3d3063b98af3ea056e02b1dcdff783d22
 generate_auto_job f64733b79dcee5a0e3e8ccc6976448e8ddf0e8b6529987a66a7d3ab2ebc10a86
 include/qpdf/auto_job_c_att.hh 4c2b171ea00531db54720bf49a43f8b34481586ae7fb6cbf225099ee42bc5bb4
 include/qpdf/auto_job_c_copy_att.hh 50609012bff14fd82f0649185940d617d05d530cdc522185c7f3920a561ccb42
@@ -599,13 +599,3 @@ if(INSTALL_CMAKE_PACKAGE)
     COMPONENT ${COMPONENT_DEV}
     DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/qpdf)
 endif()
-
-if(CHECK_SIZES AND BUILD_SHARED_LIBS AND (CMAKE_SYSTEM_NAME STREQUAL "Linux"))
-  # We can only do this check on a system with ELF shared libraries.
-  # Since this is a maintainer-only option, testing for Linux is a
-  # close enough approximation.
-  add_test(
-    NAME check-sizes
-    COMMAND ${qpdf_SOURCE_DIR}/check_abi check-sizes
-    --lib $<TARGET_FILE:libqpdf>)
-endif()
@@ -80,6 +80,13 @@ made because developers have to set the environment variable
 themselves now rather than setting it through the build. Either way,
 they are off by default.
+Maintainer Dependencies
+~~~~~~~~~~~~~~~~~~~~~~~
+
+- To run ABI checks as a maintainer, you need `castxml
+  <https://github.com/CastXML/CastXML>`__, which is used by
+  ``check_abi`` to generate sizes of all public classes.
+
 Additional Requirements on Windows
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -301,14 +308,6 @@ ZOPFLI
 Options for Working on qpdf
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
-CHECK_SIZES
-  The source file :file:`qpdf/sizes.cc` is used to display the sizes
-  of all objects in the public API. Consistency of its output between
-  releases is used as part of the check against accidental breakage of
-  the binary interface (ABI). Turning this on causes a test to be run
-  that ensures an exact match between classes in ``sizes.cc`` and
-  classes in the library's public API. This option requires Python 3.
-
 ENABLE_COVERAGE
   Compile with ``--coverage``. See README-maintainer.md for
   information about generating coverage reports.
@@ -361,8 +360,6 @@ MAINTAINER_MODE
   - ``BUILD_DOC``
-  - ``CHECK_SIZES``
-
   - ``ENABLE_QTC``
   - ``GENERATE_AUTO_JOB``
@@ -132,6 +132,10 @@ more detail.
     - There has also been significant refactoring of how qpdf internally iterates over
       arrays and dictionaries.
+    - The internal mechanism used to check object sizes for binary
+      compatibility between releases has been changed. As such, the
+      ``CHECK_SIZES`` maintainer-only build option has been removed.
+
 11.10.1: February 15, 2025
   - Build fixes
@@ -2,7 +2,6 @@ set(MAIN_CXX_PROGRAMS
   qpdf
   fix-qdf
   pdf_from_scratch
-  sizes
   test_char_sign
   test_driver
   test_large_file
@@ -28,7 +27,6 @@ foreach(PROG ${MAIN_C_PROGRAMS})
   target_link_libraries(${PROG} libqpdf)
   set_property(TARGET ${PROG} PROPERTY LINKER_LANGUAGE CXX)
 endforeach()
-target_include_directories(sizes PRIVATE ${JPEG_INCLUDE})
 set(needs_private_headers
   test_large_file
-// See "ABI checks" in README-maintainer and comments in check_abi.
-
-#include <iostream>
-
-#include <qpdf/Buffer.hh>
-#include <qpdf/BufferInputSource.hh>
-#include <qpdf/ClosedFileInputSource.hh>
-#include <qpdf/FileInputSource.hh>
-#include <qpdf/InputSource.hh>
-#include <qpdf/JSON.hh>
-#include <qpdf/PDFVersion.hh>
-#include <qpdf/Pipeline.hh>
-#include <qpdf/Pl_Buffer.hh>
-#include <qpdf/Pl_Concatenate.hh>
-#include <qpdf/Pl_Count.hh>
-#include <qpdf/Pl_DCT.hh>
-#include <qpdf/Pl_Discard.hh>
-#include <qpdf/Pl_Flate.hh>
-#include <qpdf/Pl_Function.hh>
-#include <qpdf/Pl_OStream.hh>
-#include <qpdf/Pl_QPDFTokenizer.hh>
-#include <qpdf/Pl_RunLength.hh>
-#include <qpdf/Pl_StdioFile.hh>
-#include <qpdf/Pl_String.hh>
-#include <qpdf/QPDF.hh>
-#include <qpdf/QPDFAcroFormDocumentHelper.hh>
-#include <qpdf/QPDFAnnotationObjectHelper.hh>
-#include <qpdf/QPDFCryptoProvider.hh>
-#include <qpdf/QPDFEFStreamObjectHelper.hh>
-#include <qpdf/QPDFEmbeddedFileDocumentHelper.hh>
-#include <qpdf/QPDFExc.hh>
-#include <qpdf/QPDFFileSpecObjectHelper.hh>
-#include <qpdf/QPDFFormFieldObjectHelper.hh>
-#include <qpdf/QPDFJob.hh>
-#include <qpdf/QPDFLogger.hh>
-#include <qpdf/QPDFMatrix.hh>
-#include <qpdf/QPDFNameTreeObjectHelper.hh>
-#include <qpdf/QPDFNumberTreeObjectHelper.hh>
-#include <qpdf/QPDFObjGen.hh>
-#include <qpdf/QPDFObjectHandle.hh>
-#include <qpdf/QPDFOutlineDocumentHelper.hh>
-#include <qpdf/QPDFOutlineObjectHelper.hh>
-#include <qpdf/QPDFPageDocumentHelper.hh>
-#include <qpdf/QPDFPageLabelDocumentHelper.hh>
-#include <qpdf/QPDFPageObjectHelper.hh>
-#include <qpdf/QPDFStreamFilter.hh>
-#include <qpdf/QPDFSystemError.hh>
-#include <qpdf/QPDFTokenizer.hh>
-#include <qpdf/QPDFUsage.hh>
-#include <qpdf/QPDFWriter.hh>
-#include <qpdf/QPDFXRefEntry.hh>
-
-#define ignore_class(cls)
-#define print_size(cls) std::cout << #cls << " " << sizeof(cls) << std::endl
-
-// These are not classes
-// -------
-ignore_class(QUtil);
-ignore_class(QTC);
-
-int
-main()
-{
-    // Print the size of every class in the public API. This file is
-    // read by the check_abi script at the top of the repository as
-    // part of the binary compatibility checks performed before each
-    // release.
-    print_size(Buffer);
-    print_size(BufferInputSource);
-    print_size(ClosedFileInputSource);
-    print_size(FileInputSource);
-    print_size(InputSource);
-    print_size(JSON);
-    print_size(PDFVersion);
-    print_size(Pipeline);
-    print_size(Pl_Buffer);
-    print_size(Pl_Concatenate);
-    print_size(Pl_Count);
-    print_size(Pl_DCT);
-    print_size(Pl_Discard);
-    print_size(Pl_Flate);
-    print_size(Pl_Function);
-    print_size(Pl_OStream);
-    print_size(Pl_QPDFTokenizer);
-    print_size(Pl_RunLength);
-    print_size(Pl_StdioFile);
-    print_size(Pl_String);
-    print_size(QPDF);
-    print_size(QPDFAcroFormDocumentHelper);
-    print_size(QPDFAnnotationObjectHelper);
-    print_size(QPDFCryptoProvider);
-    print_size(QPDFEFStreamObjectHelper);
-    print_size(QPDFEmbeddedFileDocumentHelper);
-    print_size(QPDFExc);
-    print_size(QPDFFileSpecObjectHelper);
-    print_size(QPDFFormFieldObjectHelper);
-    print_size(QPDFJob);
-    print_size(QPDFJob::AttConfig);
-    print_size(QPDFJob::Config);
-    print_size(QPDFJob::CopyAttConfig);
-    print_size(QPDFJob::EncConfig);
-    print_size(QPDFJob::PagesConfig);
-    print_size(QPDFJob::UOConfig);
-    print_size(QPDFJob::PageLabelsConfig);
-    print_size(QPDFLogger);
-    print_size(QPDFMatrix);
-    print_size(QPDFNameTreeObjectHelper);
-    print_size(QPDFNameTreeObjectHelper::iterator);
-    print_size(QPDFNumberTreeObjectHelper);
-    print_size(QPDFNumberTreeObjectHelper::iterator);
-    print_size(QPDFObjectHandle);
-    print_size(QPDFObjectHandle::ParserCallbacks);
-    print_size(QPDFObjectHandle::QPDFArrayItems);
-    print_size(QPDFObjectHandle::QPDFArrayItems::iterator);
-    print_size(QPDFObjectHandle::QPDFDictItems);
-    print_size(QPDFObjectHandle::QPDFDictItems::iterator);
-    print_size(QPDFObjectHandle::StreamDataProvider);
-    print_size(QPDFObjectHandle::TokenFilter);
-    print_size(QPDFObjectHelper);
-    print_size(QPDFOutlineDocumentHelper);
-    print_size(QPDFOutlineObjectHelper);
-    print_size(QPDFPageDocumentHelper);
-    print_size(QPDFPageLabelDocumentHelper);
-    print_size(QPDFPageObjectHelper);
-    print_size(QPDFStreamFilter);
-    print_size(QPDFSystemError);
-    print_size(QPDFTokenizer);
-    print_size(QPDFTokenizer::Token);
-    print_size(QPDFUsage);
-    print_size(QPDFWriter);
-    print_size(QPDFWriter::FunctionProgressReporter);
-    print_size(QPDFXRefEntry);
-    return 0;
-}