Commit 90cfe80bacdd0d398631afce76c4182e08cc78b9
1 parent
8c504c9a
Clean up/fix DLL.h
* Change DLL_EXPORT to libqpdf_EXPORTS (internal to the build). The new name is cmake's default, is more conventional, and is less likely to clash with other symbols. * Add QPDF_DLL_PRIVATE for non-Windows * Make logic around when to define QPDF_DLL et al more explicit * Add detailed comments
Showing
5 changed files
with
104 additions
and
12 deletions
.dir-locals.el
| @@ -20,7 +20,8 @@ | @@ -20,7 +20,8 @@ | ||
| 20 | ) | 20 | ) |
| 21 | ) | 21 | ) |
| 22 | ) | 22 | ) |
| 23 | - (c-noise-macro-names . ("QPDF_DLL" "QPDF_DLL_CLASS" "QPDF_DLL_LOCAL")) | 23 | + (c-noise-macro-names |
| 24 | + . ("QPDF_DLL" "QPDF_DLL_CLASS" "QPDF_DLL_PRIVATE")) | ||
| 24 | ) | 25 | ) |
| 25 | ) | 26 | ) |
| 26 | (c++-mode . ((eval . (progn | 27 | (c++-mode . ((eval . (progn |
TODO
| @@ -37,9 +37,6 @@ cmake | @@ -37,9 +37,6 @@ cmake | ||
| 37 | ===== | 37 | ===== |
| 38 | 38 | ||
| 39 | * DLL.h | 39 | * DLL.h |
| 40 | - * Change DLL_EXPORT to QPDF_EXPORT. Be sure to call attention to | ||
| 41 | - this in the release notes. There should be a "migrating to cmake" | ||
| 42 | - in the manual, and ./configure should draw attention to it. | ||
| 43 | * The effect of QPDF_DLL_CLASS is to export everything in the class, | 40 | * The effect of QPDF_DLL_CLASS is to export everything in the class, |
| 44 | not just the vtable. On MSVC, we don't need this as the vtable | 41 | not just the vtable. On MSVC, we don't need this as the vtable |
| 45 | gets exported automatically when needed. With gcc, we need it to | 42 | gets exported automatically when needed. With gcc, we need it to |
include/qpdf/DLL.h
| @@ -29,17 +29,101 @@ | @@ -29,17 +29,101 @@ | ||
| 29 | #define QPDF_PATCH_VERSION 0 | 29 | #define QPDF_PATCH_VERSION 0 |
| 30 | #define QPDF_VERSION "11.0.0" | 30 | #define QPDF_VERSION "11.0.0" |
| 31 | 31 | ||
| 32 | -#if (defined _WIN32 || defined __CYGWIN__) && defined(DLL_EXPORT) | ||
| 33 | -# define QPDF_DLL __declspec(dllexport) | 32 | +/* |
| 33 | + * This file defines symbols that control the which functions, | ||
| 34 | + * classes, and methods are exposed to the public ABI (application | ||
| 35 | + * binary interface). See below for a detailed explanation. | ||
| 36 | + */ | ||
| 37 | + | ||
| 38 | +#if defined _WIN32 || defined __CYGWIN__ | ||
| 39 | +# ifdef libqpdf_EXPORTS | ||
| 40 | +# define QPDF_DLL __declspec(dllexport) | ||
| 41 | +# else | ||
| 42 | +# define QPDF_DLL | ||
| 43 | +# endif | ||
| 44 | +# define QPDF_DLL_PRIVATE | ||
| 45 | +# define QPDF_DLL_CLASS | ||
| 34 | #elif defined __GNUC__ | 46 | #elif defined __GNUC__ |
| 35 | # define QPDF_DLL __attribute__((visibility("default"))) | 47 | # define QPDF_DLL __attribute__((visibility("default"))) |
| 36 | -#else | ||
| 37 | -# define QPDF_DLL | ||
| 38 | -#endif | ||
| 39 | -#ifdef __GNUC__ | 48 | +# define QPDF_DLL_PRIVATE __attribute__((visibility("hidden"))) |
| 40 | # define QPDF_DLL_CLASS QPDF_DLL | 49 | # define QPDF_DLL_CLASS QPDF_DLL |
| 41 | #else | 50 | #else |
| 51 | +# define QPDF_DLL | ||
| 52 | +# define QPDF_DLL_PRIVATE | ||
| 42 | # define QPDF_DLL_CLASS | 53 | # define QPDF_DLL_CLASS |
| 43 | #endif | 54 | #endif |
| 44 | 55 | ||
| 56 | +/* | ||
| 57 | + | ||
| 58 | +Here's what's happening. See also https://gcc.gnu.org/wiki/Visibility | ||
| 59 | +for a more in-depth discussion. | ||
| 60 | + | ||
| 61 | +* Everything in the public ABI must be exported. Things not in the | ||
| 62 | + public ABI should not be exported. | ||
| 63 | + | ||
| 64 | +* A class's runtime type information is need if the class is going to | ||
| 65 | + be used as an exception, inherited from, or tested with | ||
| 66 | + dynamic_claass. To do these things across a shared object boundary, | ||
| 67 | + runtime type information must be exported. | ||
| 68 | + | ||
| 69 | +* On Windows: | ||
| 70 | + | ||
| 71 | + * For a symbol (function, method, etc.) to be exported into the | ||
| 72 | + public ABI, it must be explicitly marked for export. | ||
| 73 | + | ||
| 74 | + * If you mark a class for export, all symbols in the class, | ||
| 75 | + including private methods, are exported into the DLL, and there is | ||
| 76 | + no way to exclude something from export. | ||
| 77 | + | ||
| 78 | + * A class's run-time type information is made available based on the | ||
| 79 | + presence of a compiler flag (with MSVC), which is always on for | ||
| 80 | + qpdf builds. | ||
| 81 | + | ||
| 82 | + * Marking classes for export should be done only when *building* the | ||
| 83 | + DLL, not when *using* the DLL. | ||
| 84 | + | ||
| 85 | + * It is possible to mark symbols for import for DLL users, but it is | ||
| 86 | + not necessary, and doing it right is complex in our case of being | ||
| 87 | + multi-platform and building both static and shared libraries that | ||
| 88 | + use the same headers, so we don't bother. | ||
| 89 | + | ||
| 90 | +* On Linux (and other similar systems): | ||
| 91 | + | ||
| 92 | + * Common compilers such as gcc and clang export all symbols into the | ||
| 93 | + public ABI by default. The qpdf build overrides this by using | ||
| 94 | + "visibility=hidden", which makes it behave more like Windows in | ||
| 95 | + that things have to be explicitly exported to appear in the public | ||
| 96 | + ABI. | ||
| 97 | + | ||
| 98 | + * As with Windows, marking a class for export causes everything in | ||
| 99 | + the class, including private methods, the be exported. However, | ||
| 100 | + unlike in Windows: | ||
| 101 | + | ||
| 102 | + * It is possible to explicitly mark symbols as private | ||
| 103 | + | ||
| 104 | + * The only way to get the runtime type and vtable information into | ||
| 105 | + the ABI is to mark the class as exported. | ||
| 106 | + | ||
| 107 | + * It is harmless and sometimes necessary to include the visibility | ||
| 108 | + marks when using the library as well as when building it. In | ||
| 109 | + particular, clang on MacOS requires the visibility marks to | ||
| 110 | + match in both cases. | ||
| 111 | + | ||
| 112 | +What does this mean: | ||
| 113 | + | ||
| 114 | +* On Windows, we never have to export a class, and while there is no | ||
| 115 | + way to "unexport" something, we also have no need to do it. | ||
| 116 | + | ||
| 117 | +* On non-Windows, we have to export some classes, and when we do, we | ||
| 118 | + have to "unexport" some of their parts. | ||
| 119 | + | ||
| 120 | +* We only use the libqpdf_EXPORTS as a conditional for defining the | ||
| 121 | + symbols for Windows builds. | ||
| 122 | + | ||
| 123 | +To achieve this, we use QPDF_DLL_CLASS to export classes, QPDF_DLL to | ||
| 124 | +export methods, and QPDF_DLL_PRIVATE to unexport private methods in | ||
| 125 | +exported classes. | ||
| 126 | + | ||
| 127 | +*/ | ||
| 128 | + | ||
| 45 | #endif /* QPDF_DLL_HH */ | 129 | #endif /* QPDF_DLL_HH */ |
libqpdf/CMakeLists.txt
| @@ -398,6 +398,9 @@ endif() | @@ -398,6 +398,9 @@ endif() | ||
| 398 | # use PIC for the object library so we don't have to compile twice. | 398 | # use PIC for the object library so we don't have to compile twice. |
| 399 | set(OBJECT_LIB libqpdf_object) | 399 | set(OBJECT_LIB libqpdf_object) |
| 400 | add_library(${OBJECT_LIB} OBJECT ${libqpdf_SOURCES}) | 400 | add_library(${OBJECT_LIB} OBJECT ${libqpdf_SOURCES}) |
| 401 | +if(OBJECT_LIB_IS_PIC) | ||
| 402 | + target_compile_definitions(${OBJECT_LIB} PRIVATE libqpdf_EXPORTS) | ||
| 403 | +endif() | ||
| 401 | set_target_properties(${OBJECT_LIB} PROPERTIES | 404 | set_target_properties(${OBJECT_LIB} PROPERTIES |
| 402 | POSITION_INDEPENDENT_CODE ${OBJECT_LIB_IS_PIC}) | 405 | POSITION_INDEPENDENT_CODE ${OBJECT_LIB_IS_PIC}) |
| 403 | target_include_directories(${OBJECT_LIB} | 406 | target_include_directories(${OBJECT_LIB} |
| @@ -446,8 +449,6 @@ if(NOT WIN32) | @@ -446,8 +449,6 @@ if(NOT WIN32) | ||
| 446 | endif() | 449 | endif() |
| 447 | 450 | ||
| 448 | if(BUILD_SHARED_LIBS) | 451 | if(BUILD_SHARED_LIBS) |
| 449 | - add_compile_definitions(DLL_EXPORT) | ||
| 450 | - | ||
| 451 | set(SHARED_LIB libqpdf) | 452 | set(SHARED_LIB libqpdf) |
| 452 | if(OBJECT_LIB_IS_PIC) | 453 | if(OBJECT_LIB_IS_PIC) |
| 453 | add_library(${SHARED_LIB} SHARED $<TARGET_OBJECTS:libqpdf_object>) | 454 | add_library(${SHARED_LIB} SHARED $<TARGET_OBJECTS:libqpdf_object>) |
| @@ -485,6 +486,7 @@ if(BUILD_SHARED_LIBS) | @@ -485,6 +486,7 @@ if(BUILD_SHARED_LIBS) | ||
| 485 | SOVERSION ${qpdf_SOVERSION} | 486 | SOVERSION ${qpdf_SOVERSION} |
| 486 | POSITION_INDEPENDENT_CODE ${BUILD_SHARED_LIBS}) | 487 | POSITION_INDEPENDENT_CODE ${BUILD_SHARED_LIBS}) |
| 487 | 488 | ||
| 489 | + target_compile_definitions(${SHARED_LIB} PRIVATE libqpdf_EXPORTS) | ||
| 488 | target_include_directories(${SHARED_LIB} | 490 | target_include_directories(${SHARED_LIB} |
| 489 | SYSTEM PRIVATE ${dep_include_directories}) | 491 | SYSTEM PRIVATE ${dep_include_directories}) |
| 490 | target_include_directories(${SHARED_LIB} | 492 | target_include_directories(${SHARED_LIB} |
manual/installation.rst
| @@ -617,6 +617,14 @@ and cmake options. There are a few exceptions: | @@ -617,6 +617,14 @@ and cmake options. There are a few exceptions: | ||
| 617 | ``QPDF_TEST_COMPARE_IMAGES`` to ``1`` to *enable* image comparison | 617 | ``QPDF_TEST_COMPARE_IMAGES`` to ``1`` to *enable* image comparison |
| 618 | tests. Either way, they are off by default. | 618 | tests. Either way, they are off by default. |
| 619 | 619 | ||
| 620 | +- Non-user-visible change: the preprocessor symbol that triggers the | ||
| 621 | + export of functions into the public ABI (application binary | ||
| 622 | + interface) has been changed from ``DLL_EXPORT`` to | ||
| 623 | + ``libqpdf_EXPORTS``. This detail is encapsulated in the build and is | ||
| 624 | + only relevant to people who are building qpdf on their own or who | ||
| 625 | + may have previously needed to work around a collision between qpdf's | ||
| 626 | + use of ``DLL_EXPORT`` and someone else's use of the same symbol. | ||
| 627 | + | ||
| 620 | - A handful of options that were specific to autoconf or the old build | 628 | - A handful of options that were specific to autoconf or the old build |
| 621 | system have been dropped. | 629 | system have been dropped. |
| 622 | 630 |