Make the default symbol visibility hidden for unixen builds

cmake/CMakeLists.txt

@@ -58,6 +58,15 @@ if(APPLE)
   endif()
 endif()
 
+if (OPENEXR_ENABLE_LARGE_STACK)
+  set(OPENEXR_HAVE_LARGE_STACK ON)
+endif()
+if (OPENEXR_USE_DEFAULT_VISIBILITY)
+  set(OPENEXR_ENABLE_API_VISIBILITY OFF)
+else()
+  set(OPENEXR_ENABLE_API_VISIBILITY ON)
+endif()
+
 configure_file(OpenEXRConfig.h.in ${CMAKE_CURRENT_BINARY_DIR}/OpenEXRConfig.h)
 configure_file(OpenEXRConfigInternal.h.in ${CMAKE_CURRENT_BINARY_DIR}/OpenEXRConfigInternal.h)
 
@@ -110,10 +119,6 @@ if(OPENEXR_ENABLE_THREADING AND Threads_FOUND)
   endif()
 endif()
 
-if (OPENEXR_ENABLE_LARGE_STACK)
-  set(OPENEXR_HAVE_LARGE_STACK ON)
-endif()
-
 configure_file(IexConfig.h.in ${CMAKE_CURRENT_BINARY_DIR}/IexConfig.h)
 configure_file(IexConfigInternal.h.in ${CMAKE_CURRENT_BINARY_DIR}/IexConfigInternal.h)
 configure_file(IlmThreadConfig.h.in ${CMAKE_CURRENT_BINARY_DIR}/IlmThreadConfig.h)

cmake/LibraryDefine.cmake

@@ -10,7 +10,7 @@ function(OPENEXR_DEFINE_LIBRARY libname)
   cmake_parse_arguments(OPENEXR_CURLIB "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
 
   if (MSVC)
-    set(_imath_extra_flags "/EHsc")
+    set(_openexr_extra_flags "/EHsc")
   endif()
   set(objlib ${libname})
   add_library(${objlib}
@@ -40,8 +40,17 @@ function(OPENEXR_DEFINE_LIBRARY libname)
     CXX_EXTENSIONS OFF
     POSITION_INDEPENDENT_CODE ON
   )
-  if (_imath_extra_flags)
-    target_compile_options(${objlib} PUBLIC ${_imath_extra_flags})
+  if (NOT OPENEXR_USE_DEFAULT_VISIBILITY)
+    set_target_properties(${objlib} PROPERTIES
+      C_VISIBILITY_PRESET hidden
+      CXX_VISIBILITY_PRESET hidden
+      VISIBILITY_INLINES_HIDDEN ON
+      )
+  else()
+      target_compile_definitions(${objlib} PUBLIC OPENEXR_USE_DEFAULT_VISIBILITY)
+  endif()
+  if (_openexr_extra_flags)
+    target_compile_options(${objlib} PUBLIC ${_openexr_extra_flags})
   endif()
   set_property(TARGET ${objlib} PROPERTY PUBLIC_HEADER ${OPENEXR_CURLIB_HEADERS})
 

cmake/OpenEXRConfig.h.in

@@ -52,4 +52,87 @@
                              (uint32_t(OPENEXR_VERSION_MINOR) << 16) | \
                              (uint32_t(OPENEXR_VERSION_PATCH) <<  8))
 
+// Whether the user configured the library to have symbol visibility
+// tagged
+#cmakedefine OPENEXR_ENABLE_API_VISIBILITY
+
+/// \defgroup ExportMacros Macros to manage symbol visibility
+///
+/// See docs/SymbolVisibility.md for more discussion about the
+/// motivation for these macros
+///
+/// If we are compiling a DLL for Windows, there needs to be custom
+/// rules for each library such that the macro swaps between doing a
+/// dllexport and a dllimport, so the defines here are less
+/// useful. Further, MSVC does not have this concept at all currently,
+/// so is elided.
+///
+/// The top level macros which start with OPENEXR can act as simple
+/// ways to combine the logic however for non-DLL or non-windows
+/// platforms, but until the current patterns change, one should check
+/// the specific library export.h (i.e. @sa IexExport.h,
+/// @sa IlmThreadExport.h, @sa ImfExport.h, @sa ImfUtilExport.h )
+///
+/// These per-library exports define a subset which are used by that
+/// library.
+///
+/// Iex is simple and does not need to do more than expose class types
+/// and functions, and does not have any private members to hide, so
+/// only provides a couple of the possible macros.
+///
+/// Similarly, IlmThread is also reasonably simple.
+///
+/// OpenEXR and OpenEXRUtil have much more logic and have to deal with
+/// templates and template instantiation, and so define more of the
+/// macros.
+///
+/// @{
+
+#if defined(OPENEXR_ENABLE_API_VISIBILITY) && ! ( defined(OPENEXR_DLL) || defined(_MSC_VER) )
+#  define OPENEXR_PUBLIC_SYMBOL_ATTRIBUTE __attribute__ ((__visibility__ ("default")))
+#  define OPENEXR_PRIVATE_SYMBOL_ATTRIBUTE __attribute__ ((__visibility__ ("hidden")))
+   // clang differs from gcc and has type visibility which is needed
+   // for enums and templates, and isn't well documented, but causes
+   // the vtable and typeinfo to be made visible, but not necessarily
+   // all the members
+#  if __has_attribute(__type_visibility__)
+#    define OPENEXR_PUBLIC_TYPE_VISIBILITY_ATTRIBUTE __attribute__ ((__type_visibility__ ("default")))
+#  endif
+
+   // these are always the same, at least in current compilers
+#  define OPENEXR_EXPORT OPENEXR_PUBLIC_SYMBOL_ATTRIBUTE
+#  define OPENEXR_HIDDEN OPENEXR_PRIVATE_SYMBOL_ATTRIBUTE
+   // currently define this as the same between compilers to export
+   // things like default copy ctors etc, and do not use the type
+   // visibility which only exports the typeinfo / vtable
+#  define OPENEXR_EXPORT_TYPE OPENEXR_EXPORT
+#  define OPENEXR_EXPORT_EXTERN_TEMPLATE OPENEXR_EXPORT
+
+#  ifdef OPENEXR_PUBLIC_TYPE_VISIBILITY_ATTRIBUTE
+#    define OPENEXR_EXPORT_ENUM OPENEXR_PUBLIC_TYPE_VISIBILITY_ATTRIBUTE
+#    define OPENEXR_EXPORT_TEMPLATE_TYPE OPENEXR_PUBLIC_TYPE_VISIBILITY_ATTRIBUTE
+     // clang (well, type_visibility) seems empirically need the
+     // default/public symbol tag when specifying explicit template
+     // instantiations, where gcc (no type_visibility) complains if
+     // you set that
+#    define OPENEXR_EXPORT_TEMPLATE_INSTANCE OPENEXR_EXPORT
+#  else
+#    define OPENEXR_EXPORT_ENUM
+#    define OPENEXR_EXPORT_TEMPLATE_TYPE OPENEXR_EXPORT
+#    define OPENEXR_EXPORT_TEMPLATE_INSTANCE
+#  endif
+
+#else // msvc or api visibility disabled, just clear all this out (DLLs will define a set anyway)
+
+#  define OPENEXR_EXPORT
+#  define OPENEXR_HIDDEN
+#  define OPENEXR_EXPORT_TYPE
+#  define OPENEXR_EXPORT_EXTERN_TEMPLATE
+#  define OPENEXR_EXPORT_ENUM
+#  define OPENEXR_EXPORT_TEMPLATE_TYPE
+#  define OPENEXR_EXPORT_TYPE
+#  define OPENEXR_EXPORT_TEMPLATE_INSTANCE
+
+#endif
+
 #endif // INCLUDED_OPENEXR_CONFIG_H

cmake/OpenEXRSetup.cmake

@@ -46,6 +46,8 @@ endif()
 # are still used, just processed immediately
 option(OPENEXR_ENABLE_THREADING "Enables threaded processing of requests" ON)
 
+option(OPENEXR_USE_DEFAULT_VISIBILITY "Makes the compile use default visibility (by default compiles tidy, hidden-by-default)"     OFF)
+
 # This is primarily for the auto array that enables a stack
 # object (if you enable this) that contains member to avoid double allocations
 option(OPENEXR_ENABLE_LARGE_STACK "Enables code to take advantage of large stack support"     OFF)

docs/SymbolVisibility.md

@@ -0,0 +1,103 @@
+# Symbol Visibility in OpenEXR
+
+## Overview
+
+Managing symbol visibility in a C++ library can reduce library sizes,
+and with the extra information, the optimizer may produce faster
+code. To take advantage of this, OpenEXR 3.0 is switching to
+explicitly manage symbols on all platforms, with a hidden-by-default
+behavior on unix-based platforms. Managing symbols has always been
+required for Windows DLLs, where one must explicitly tag functions for
+import and export as appropriate.
+
+For C, this is trivial: just tag public functions or global variable
+as default visibility and leave everything else defaulting to
+hidden. However, in C++, this is not exactly the same story. Functions
+and globals are of course the same. And class member functions are
+largely the same, and other than the attribute specification
+mechanics, follow the same rules between gcc, clang, and
+msvc. However, types have richer information than they do in C. So,
+unless extra work is done, concepts for RTTI like the typeinfo and the
+vtable for virtual classes will be hidden, and not visible. These are
+referred to as "vague" linkage objects in some discussions. 
+
+It is with the "vague" linkage objects where different properties
+arise. For example, if you have a template, it is happily instantiated
+in multiple compile units. If the typeinfo is hidden for one library,
+then this may cause things like dynamic_cast to fail because then the
+same typeinfo is not used, and even though one might think that
+ImfAttribute<Imath::Box2i> are the same in two places, because they
+are instantiated in separate places, they may be considered different
+types. To compound the issue, there are different rules for this in
+different implementations. For example, a default gcc under linux
+allows one to link against otherwise private "vague" linkage objects
+such that the typeinfo ends up as the same entity. clang, for MacOS
+anyway, follows a stricter approach and keeps those types separate,
+perhaps due to the two level namespace they maintain for symbols.
+
+Unfortunately, this is not clearly discussed as an overview of the
+differences between platforms, hence this document to add
+clarity. Each compiler / platform describes their own behavior, but
+not how that behaves relative to
+others. [libc++](https://libcxx.llvm.org/docs/DesignDocs/VisibilityMacros.html)
+from the llvm project is the closest to providing comparitive
+information, where by looking at how they define their macros and the
+comments surrounding, one can infer the behavior among at least
+windows DLL mode, then gcc vs. clang for unixen. Other compilers, for
+example, Intel's icc, tend to adopt the behavior of the predominant
+compiler for that platform (i.e. msvc under windows, gcc under linux),
+and so can generally adopt that behavior and are ignored here. If this
+is not true, the ifdef rules in the various library *Export.h headers
+within OpenEXR may need to be adjusted, and this table updated.
+
+As a summary, below is a table of the attribute or declspec that needs
+to be used for a particular C++ entity to be properly exported. This
+does not address weak symbols, ABI versioning, and only focusing on
+visibility. Under Windows DLL rules, if one exports the entire class,
+it also exports the types for the member types as well, which is not
+desired, so these are marked as N/A even though the compiler does
+allow that to happen.
+
+
+| C++ vs Compiler | MSVC                | mingw               | gcc                   | clang                      |
+|-----------------|---------------------|---------------------|-----------------------|----------------------------|
+| function        | dllexport/dllimport | dllexport/dllimport | visibility("default") | visibility("default")      |
+| hide a function | N/A                 | N/A                 | visibility("hidden")  | visibility("hidden")       |
+| class(typeinfo) | N/A                 | N/A                 | visibility("default") | visibility("default")      |
+| template class  | N/A                 | N/A                 | visibility("default") | type_visibility("default") |
+| template data   | N/A                 | N/A                 | visibility("default") | visibility("default")      |
+| class template<br>instantiation | dllexport/dllimport | N/A | N/A                   | visibility("default")      |
+| enum            | N/A                 | N/A                 | auto unhides (N/A)    | type_visibility("default") |
+| extern template | N/A                 | dllexport/dllimport | visibility("default") | visibility("default")      |
+
+With this matrix in mind, we can see the maximal set of macros we need to
+provide throughout the code. *NB*: This does not mean that we need to
+declare all of these, just that they might be needed. `XXX` should be
+substituted for the particular library name being compiled.
+
+| macro name                     | purpose                                                          |
+|--------------------------------|------------------------------------------------------------------|
+| `XXX_EXPORT`                   | one of export or import for windows, visibility for others       |
+| `XXX_EXPORT_TYPE`              | for declaring a class / struct as public (for typeinfo / vtable) |
+| `XXX_HIDDEN`                   | used to explicitly hide, especially members of types             |
+| `XXX_EXPORT_TEMPLATE_TYPE`     | stating the template type should be visible                      |
+| `XXX_EXPORT_EXTERN_TEMPLATE`   | exporting template types (i.e. extern side of extern template)   |
+| `XXX_EXPORT_TEMPLATE_INSTANCE` | exporting specific template instantiations (in cpp code)         |
+| `XXX_EXPORT_TEMPLATE_DATA`     | exporting templated data blocks                                  |
+| `XXX_EXPORT_ENUM`              | exporting enum types                                             |
+
+For a new library, the preference might be to call `XXX_EXPORT`
+something like `XXX_FUNC`, and rename things such as `XXX_EXPORT_TYPE`
+to `XXX_TYPE` for simplicity. However, historically, OpenEXR has used
+the `_EXPORT` tag, and so that is preserved for consistency.
+
+## References
+
+LLVM libc++ visibility macros:
+https://libcxx.llvm.org/docs/DesignDocs/VisibilityMacros.html
+
+GCC visibility wiki:
+https://gcc.gnu.org/wiki/Visibility
+
+Apple library design docs:
+https://developer.apple.com/library/archive/documentation/DeveloperTools/Conceptual/DynamicLibraries/100-Articles/DynamicLibraryDesignGuidelines.html

src/bin/exrheader/main.cpp

@@ -433,7 +433,7 @@ printInfo (const char fileName[])
                 cout << ": \"" << ta->value() << "\"";
             }
             else if (const StringVectorAttribute * ta =
-                            dynamic_cast<const StringVectorAttribute *>(a))
+                            dynamic_cast <const StringVectorAttribute *> (a))
             {
                 cout << ":";
 

src/bin/exrmaketiled/main.cpp

@@ -13,6 +13,8 @@
 
 #include "makeTiled.h"
 
+#include <ImfHeader.h>
+
 #include <iostream>
 #include <exception>
 #include <string>

src/examples/generalInterfaceExamples.cpp

@@ -21,6 +21,7 @@
 
 #include "drawImage.h"
 
+#include <cfloat>
 #include <iostream>
 #include <limits>
 

src/lib/Iex/IexBaseExc.h

@@ -27,7 +27,7 @@ IEX_INTERNAL_NAMESPACE_HEADER_ENTER
 // Our most basic exception class
 //-------------------------------
 
-class BaseExc: public std::exception
+class IEX_EXPORT_TYPE BaseExc: public std::exception
 {
   public:
 
@@ -93,8 +93,8 @@ class BaseExc: public std::exception
 
   private:
 
-    std::string                     _message;
-    std::string                     _stackTrace;
+    std::string _message;
+    std::string _stackTrace;
 };
 
 
@@ -104,7 +104,7 @@ class BaseExc: public std::exception
 //-----------------------------------------------------
 
 #define DEFINE_EXC_EXP(exp, name, base)                             \
-    class name: public base                                         \
+    class IEX_EXPORT_TYPE name: public base                         \
     {                                                               \
       public:                                                       \
         exp name();                                                 \
@@ -129,7 +129,7 @@ exp name::name (const name &other) : base (other) {}             \
 exp name::name (name &&other) noexcept : base (other) {}         \
 exp name& name::operator = (name &other) { base::operator=(other); return *this; } \
 exp name& name::operator = (name &&other) noexcept { base::operator=(other); return *this; } \
-exp name::~name () throw () {}
+exp name::~name () noexcept {}
 
 // For backward compatibility.
 #define DEFINE_EXC(name, base) DEFINE_EXC_EXP(, name, base)

src/lib/Iex/IexExport.h

@@ -3,24 +3,37 @@
 // Copyright (c) Contributors to the OpenEXR Project.
 //
 
-#ifndef IEXEXPORT_H
-#define IEXEXPORT_H
+#ifndef INCLUDED_IEXEXPORT_H
+#define INCLUDED_IEXEXPORT_H
+
+#include "OpenEXRConfig.h"
 
 #if defined(OPENEXR_DLL)
-#    if defined(IEX_EXPORTS)
-#        define IEX_EXPORT __declspec(dllexport)
-#    else
-#        define IEX_EXPORT __declspec(dllimport)
-#    endif
-#else
-#    define IEX_EXPORT
-#endif
-#ifndef IEX_EXPORT_TYPE
-#    define IEX_EXPORT_TYPE
-#endif
-#ifndef IEX_EXPORT_ENUM
-#    define IEX_EXPORT_ENUM
-#endif
-
-#endif // #ifndef IEXEXPORT_H
+
+// when building as a DLL for windows, typical dllexport / import case
+// where we need to switch depending on whether we are compiling
+// internally or not
+
+#  if defined(IEX_EXPORTS)
+#    define IEX_EXPORT __declspec(dllexport)
+#  else
+#    define IEX_EXPORT __declspec(dllimport)
+#  endif
+
+// DLLs don't support these types of visibility controls, just leave them as empty
+#  define IEX_EXPORT_TYPE
+#  define IEX_EXPORT_ENUM
+
+#else // OPENEXR_DLL
+
+// just pass these through from the top level config
+#  define IEX_EXPORT OPENEXR_EXPORT
+#  define IEX_EXPORT_TYPE OPENEXR_EXPORT_TYPE
+#  define IEX_EXPORT_ENUM OPENEXR_EXPORT_ENUM
+
+#endif // OPENEXR_DLL
+
+/// @}
+
+#endif // #ifndef INCLUDED_IEXEXPORT_H