Skip to content

Commit b86a46a

Browse files
authored
build: Fix HARDENING build options (#4996)
Due to typos in the option name in compiler.cmake (HARDENING vs OIIO_HARDENING, oops), I think we were never really setting the intended compiler flags. Fix that all up, and repair the other problems that it revealed -- some compiler version and option combinations weren't happy with each other, etc. One notable change is in encode_iptc_iim_one_tag: I think I made it safer by taking an early out for 0-sized data, and also it needed a warning suppression when certain gcc hardening levels were used. --------- Signed-off-by: Larry Gritz <[email protected]>
1 parent 0a30218 commit b86a46a

File tree

3 files changed

+57
-30
lines changed

3 files changed

+57
-30
lines changed

.github/workflows/ci.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ jobs:
494494
PTEX_VERSION=main
495495
PUGIXML_VERSION=master
496496
WEBP_VERSION=main
497-
OIIO_CMAKE_FLAGS="-DOIIO_HARDENING=2"
497+
OIIO_HARDENING=2
498498
EXTRA_DEP_PACKAGES="python3.12-dev python3-numpy"
499499
USE_OPENVDB=0
500500
FREETYPE_VERSION=master
@@ -518,6 +518,7 @@ jobs:
518518
PTEX_VERSION=v2.4.2
519519
PUGIXML_VERSION=v1.14
520520
WEBP_VERSION=v1.4.0
521+
OIIO_HARDENING=3
521522
- desc: clang18 C++17 avx2 exr3.1 ocio2.3
522523
nametag: linux-clang18
523524
runner: ubuntu-24.04

src/cmake/compiler.cmake

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -485,52 +485,74 @@ endif ()
485485
# recommended default for optimized, shipping code.
486486
# 2 : enable features that trade off performance for security, recommended
487487
# for debugging or deploying in security-sensitive environments.
488-
# 3 : enable features that have a significant performance impact, only
489-
# recommended for debugging.
488+
# 3 : enable features that have a significant performance impact, to maximize
489+
# finding bugs without regard to performance. Only recommended for
490+
# debugging.
490491
#
491492
# Some documentation:
492493
# https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html
493494
# https://www.gnu.org/software/libc/manual/html_node/Source-Fortification.html
494495
# https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
495496
# https://libcxx.llvm.org/Hardening.html
496-
#
497+
# https://www.productive-cpp.com/hardening-cpp-programs-stack-protector/
498+
# https://medium.com/@simontoth/daily-bit-e-of-c-hardened-mode-of-standard-library-implementations-18be2422c372
499+
# https://cheatsheetseries.owasp.org/cheatsheets/C-Based_Toolchain_Hardening_Cheat_Sheet.html
500+
497501
if (${CMAKE_BUILD_TYPE} STREQUAL "Debug")
498-
set (${PROJ_NAME}_HARDENING_DEFAULT 3)
502+
set (${PROJ_NAME}_HARDENING_DEFAULT 2)
499503
else ()
500504
set (${PROJ_NAME}_HARDENING_DEFAULT 1)
501505
endif ()
502506
set_cache (${PROJ_NAME}_HARDENING ${${PROJ_NAME}_HARDENING_DEFAULT}
503507
"Turn on security hardening features 0, 1, 2, 3")
504508
# Implementation:
505-
if (HARDENING GREATER_EQUAL 1)
509+
add_compile_definitions (${PROJ_NAME}_HARDENING_DEFAULT=${${PROJ_NAME}_HARDENING})
510+
if (${PROJ_NAME}_HARDENING GREATER_EQUAL 1)
511+
# Enable PIE and pie to build as position-independent executables and
512+
# libraries, needed for address space randomization used by some kernels.
513+
set (CMAKE_POSITION_INDEPENDENT_CODE ON)
506514
# Features that should not detectably affect performance
507515
if (COMPILER_IS_GCC_OR_ANY_CLANG)
508-
# Enable PIE and pie to build as position-independent executables,
509-
# needed for address space randomiztion used by some kernels.
510-
add_compile_options (-fPIE -pie)
511-
add_link_options (-fPIE -pie)
512516
# Protect against stack overwrites. Is allegedly not a performance
513517
# tradeoff.
514518
add_compile_options (-fstack-protector-strong)
515519
add_link_options (-fstack-protector-strong)
516520
endif ()
517521
# Defining _FORTIFY_SOURCE provides buffer overflow checks in modern gcc &
518522
# clang with some compiler-assisted deduction of buffer lengths) for the
519-
# many C functions such as memcpy, strcpy, sprintf, etc. See:
520-
add_compile_definitions (_FORTIFY_SOURCE=${${PROJ_NAME}_HARDENING})
523+
# many C functions such as memcpy, strcpy, sprintf, etc. But it requires
524+
# optimization, so we don't do it for debug builds.
525+
if ((CMAKE_COMPILER_IS_CLANG OR (GCC_VERSION VERSION_GREATER_EQUAL 14))
526+
AND NOT CMAKE_BUILD_TYPE STREQUAL "Debug")
527+
add_compile_definitions (_FORTIFY_SOURCE=${${PROJ_NAME}_HARDENING})
528+
endif ()
529+
endif ()
530+
if (${PROJ_NAME}_HARDENING EQUAL 1)
521531
# Setting _LIBCPP_HARDENING_MODE enables various hardening features in
522532
# clang/llvm's libc++ 18.0 and later.
523-
add_compiler_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_FAST)
524-
endif ()
525-
if (HARDENING GREATER_EQUAL 2)
533+
if (OIIO_CLANG_VERSION VERSION_GREATER_EQUAL 18.0 OR OIIO_APPLE_CLANG_VERSION VERSION_GREATER_EQUAL 18.0)
534+
add_compile_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_FAST)
535+
endif ()
536+
elseif (${PROJ_NAME}_HARDENING EQUAL 2)
526537
# Features that might impact performance measurably
527-
add_compile_definitions (_GLIBCXX_ASSERTIONS)
528-
add_compiler_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE)
529-
endif ()
530-
if (HARDENING GREATER_EQUAL 3)
538+
if (GCC_VERSION VERSION_GREATER_EQUAL 14)
539+
# I've had trouble turning this on in older gcc
540+
add_compile_definitions (_GLIBCXX_ASSERTIONS)
541+
endif ()
542+
if (OIIO_CLANG_VERSION VERSION_GREATER_EQUAL 18.0 OR OIIO_APPLE_CLANG_VERSION VERSION_GREATER_EQUAL 18.0)
543+
add_compile_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE)
544+
endif ()
545+
elseif (${PROJ_NAME}_HARDENING EQUAL 3)
531546
# Debugging features that might impact performance significantly
532-
add_compile_definitions (_GLIBCXX_DEBUG)
533-
add_compiler_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG)
547+
if (GCC_VERSION VERSION_GREATER_EQUAL 14)
548+
# I've had trouble turning this on in older gcc
549+
add_compile_definitions (_GLIBCXX_ASSERTIONS)
550+
# N.B. _GLIBCXX_DEBUG changes ABI, so don't do this:
551+
# add_compile_definitions (_GLIBCXX_DEBUG)
552+
endif ()
553+
if (OIIO_CLANG_VERSION VERSION_GREATER_EQUAL 18.0 OR OIIO_APPLE_CLANG_VERSION VERSION_GREATER_EQUAL 18.0)
554+
add_compile_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG)
555+
endif ()
534556
endif ()
535557

536558

src/libOpenImageIO/iptc.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -181,17 +181,21 @@ decode_iptc_iim(const void* iptc, int length, ImageSpec& spec)
181181
static void
182182
encode_iptc_iim_one_tag(int tag, string_view data, std::vector<char>& iptc)
183183
{
184-
OIIO_DASSERT(data != nullptr);
184+
if (data.size() == 0)
185+
return;
186+
data = data.substr(0, 0xffff); // Truncate to prevent 16 bit overflow
187+
size_t tagsize = data.size();
185188
iptc.push_back((char)0x1c);
186189
iptc.push_back((char)0x02);
187190
iptc.push_back((char)tag);
188-
if (data.size()) {
189-
int tagsize = std::min(int(data.size()),
190-
0xffff - 1); // Prevent 16 bit overflow
191-
iptc.push_back((char)(tagsize >> 8));
192-
iptc.push_back((char)(tagsize & 0xff));
193-
iptc.insert(iptc.end(), data.data(), data.data() + tagsize);
194-
}
191+
iptc.push_back((char)(tagsize >> 8));
192+
iptc.push_back((char)(tagsize & 0xff));
193+
OIIO_PRAGMA_WARNING_PUSH
194+
OIIO_GCC_ONLY_PRAGMA(GCC diagnostic ignored "-Wstringop-overflow")
195+
// Suppress what I'm sure is a false positive warning when
196+
// _GLIBCXX_ASSERTIONS is enabled.
197+
iptc.insert(iptc.end(), data.begin(), data.end());
198+
OIIO_PRAGMA_WARNING_POP
195199
}
196200

197201

@@ -208,7 +212,7 @@ encode_iptc_iim(const ImageSpec& spec, std::vector<char>& iptc)
208212
std::string allvals = p->get_string(0);
209213
std::vector<std::string> tokens;
210214
Strutil::split(allvals, tokens, ";");
211-
for (auto& token : tokens) {
215+
for (auto token : tokens) {
212216
token = Strutil::strip(token);
213217
if (token.size()) {
214218
if (iimtag[i].maxlen && iimtag[i].maxlen < token.size())

0 commit comments

Comments
 (0)