From 3d23d32e54bd6897d1460475b0beed7166b5d324 Mon Sep 17 00:00:00 2001 From: metsw24-max Date: Thu, 21 May 2026 15:18:25 +0530 Subject: [PATCH 1/4] Add per-source opt-in support for `-Wunsafe-buffer-usage` --- CMakeLists.txt | 20 ++++++++++++++++ apps/avifgainmaputil/avifgainmaputil.cc | 28 ++++++++++++++++++---- tests/CMakeLists.txt | 32 +++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e82b969410..a693be4cad 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -563,6 +563,25 @@ if(AVIF_LIB_USE_CXX OR AVIF_BUILD_APPS OR (AVIF_BUILD_TESTS AND (AVIF_FUZZTEST O enable_language(CXX) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) + if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") + include(CheckCXXCompilerFlag) + check_cxx_compiler_flag(-Wunsafe-buffer-usage AVIF_HAVE_WUNSAFE_BUFFER_USAGE) + endif() +endif() + +# Opt a list of C++ source files into the -Wunsafe-buffer-usage diagnostic +# (the first step of adopting the Safe Buffers Programming Model). Apply only +# to .cc files that have been audited to be free of raw pointer arithmetic and +# raw-pointer indexing; new code added to those files must keep them clean. +# No-op on toolchains where the flag is not available (Clang < 16, GCC, MSVC). +function(avif_enable_safe_buffers_warning) + if(AVIF_HAVE_WUNSAFE_BUFFER_USAGE) + set_source_files_properties(${ARGN} PROPERTIES COMPILE_OPTIONS "-Wunsafe-buffer-usage") + endif() +endfunction() + +if(AVIF_ENABLE_COMPLIANCE_WARDEN) + avif_enable_safe_buffers_warning(src/compliance.cc) endif() set_target_properties(avif_obj PROPERTIES C_VISIBILITY_PRESET hidden) @@ -723,6 +742,7 @@ if(AVIF_BUILD_APPS) apps/avifgainmaputil/program_command.cc apps/avifgainmaputil/swapbase_command.cc ) + avif_enable_safe_buffers_warning(${AVIFGAINMAPUTIL_SRCS}) add_executable(avifgainmaputil "${AVIFGAINMAPUTIL_SRCS}") if(WIN32) diff --git a/apps/avifgainmaputil/avifgainmaputil.cc b/apps/avifgainmaputil/avifgainmaputil.cc index 25ba16e7ea..25949f5354 100644 --- a/apps/avifgainmaputil/avifgainmaputil.cc +++ b/apps/avifgainmaputil/avifgainmaputil.cc @@ -91,10 +91,30 @@ int main(int argc, char** argv) { return 1; } - const std::string command_name(argv[1]); + // argv is a raw pointer with no associated bound, so -Wunsafe-buffer-usage + // cannot prove these accesses safe. They are bounded by the argc checks + // above and by the host's contract with main(): argv[0..argc-1] are valid. + const char* command_cstr; + const char* sub_command_cstr = nullptr; + char* const* command_argv; +#ifdef __clang__ +#if __has_warning("-Wunsafe-buffer-usage") +#pragma clang unsafe_buffer_usage begin +#endif +#endif + command_cstr = argv[1]; + if (argc >= 3) sub_command_cstr = argv[2]; + command_argv = argv + 1; +#ifdef __clang__ +#if __has_warning("-Wunsafe-buffer-usage") +#pragma clang unsafe_buffer_usage end +#endif +#endif + + const std::string command_name(command_cstr); if (command_name == "help") { - if (argc >= 3) { - const std::string sub_command_name(argv[2]); + if (sub_command_cstr != nullptr) { + const std::string sub_command_name(sub_command_cstr); for (const auto& command : commands) { if (command->name() == sub_command_name) { command->PrintUsage(); @@ -113,7 +133,7 @@ int main(int argc, char** argv) { for (const auto& command : commands) { if (command->name() == command_name) { try { - avifResult result = command->ParseArgs(argc - 1, argv + 1); + avifResult result = command->ParseArgs(argc - 1, command_argv); if (result == AVIF_RESULT_OK) { result = command->Run(); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index bb1199b7c8..510063ff4e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -74,6 +74,38 @@ if(AVIF_GTEST) target_link_libraries(avifincrtest_helpers PRIVATE GTest::GTest avif_enable_warnings) endif() +# Tests that have been audited for the Safe Buffers Programming Model. Adding +# a new test to this list requires that the file compiles cleanly under +# -Wunsafe-buffer-usage. See avif_enable_safe_buffers_warning() in the root +# CMakeLists.txt. +if(AVIF_GTEST) + avif_enable_safe_buffers_warning( + gtest/avifalphapremtest.cc + gtest/avifbasictest.cc + gtest/avifcicptest.cc + gtest/avifclaptest.cc + gtest/avifcllitest.cc + gtest/avifcodectest.cc + gtest/avifcolrtest.cc + gtest/avifgridapitest.cc + gtest/avifimagetest.cc + gtest/avifjpeggainmaptest.cc + gtest/avifopaquetest.cc + gtest/avifpropinternaltest.cc + gtest/avifrgbtest.cc + gtest/avifsampletransformtest.cc + gtest/aviftilingtest.cc + gtest/avifutilstest.cc + gtest/avify4mtest.cc + ) + if(AVIF_CODEC_AVM_ENABLED) + avif_enable_safe_buffers_warning(gtest/avifavmtest.cc) + if(AVIF_ENABLE_EXPERIMENTAL_MINI) + avif_enable_safe_buffers_warning(gtest/avifavmminitest.cc) + endif() + endif() +endif() + if(AVIF_GTEST) add_avif_gtest_with_data(avif16bittest) add_avif_gtest(avifallocationtest) From 63947bccd770ae901cfad62ce5229e2d6b135690 Mon Sep 17 00:00:00 2001 From: metsw24-max Date: Thu, 21 May 2026 15:48:48 +0530 Subject: [PATCH 2/4] updated --- tests/CMakeLists.txt | 1 - tests/gtest/avifrgbtest.cc | 27 ++++++++++++++------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 510063ff4e..546fa66fb8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -89,7 +89,6 @@ if(AVIF_GTEST) gtest/avifcolrtest.cc gtest/avifgridapitest.cc gtest/avifimagetest.cc - gtest/avifjpeggainmaptest.cc gtest/avifopaquetest.cc gtest/avifpropinternaltest.cc gtest/avifrgbtest.cc diff --git a/tests/gtest/avifrgbtest.cc b/tests/gtest/avifrgbtest.cc index faaf47aba3..8af4440bf7 100644 --- a/tests/gtest/avifrgbtest.cc +++ b/tests/gtest/avifrgbtest.cc @@ -1,6 +1,7 @@ // Copyright 2023 Google LLC // SPDX-License-Identifier: BSD-2-Clause +#include #include #include "avif/internal.h" @@ -40,18 +41,18 @@ TEST_P(SetGetRGBATest, SetGetTest) { epsilon = 0.0005f; // Half precision floats are not that precise. } - float pixel_read[4]; + std::array pixel_read; for (uint32_t j = 0; j < rgb.height; ++j) { for (uint32_t i = 0; i < rgb.width; ++i) { // Generate some arbitrary pixel values. - const float pixel_to_write[4] = { + const std::array pixel_to_write = { 0.0f + static_cast(i) / rgb.width, 0.5f + static_cast(j) / (rgb.height * 2), 1.0f - static_cast(i + j) / ((rgb.width + rgb.height) * 2), 1.0f - static_cast(i) / rgb.width}; - avifSetRGBAPixel(&rgb, i, j, &color_space, pixel_to_write); - avifGetRGBAPixel(&rgb, i, j, &color_space, pixel_read); + avifSetRGBAPixel(&rgb, i, j, &color_space, pixel_to_write.data()); + avifGetRGBAPixel(&rgb, i, j, &color_space, pixel_read.data()); EXPECT_NEAR(pixel_read[0], pixel_to_write[0], epsilon); EXPECT_NEAR(pixel_read[1], pixel_to_write[1], epsilon); EXPECT_NEAR(pixel_read[2], pixel_to_write[2], epsilon); @@ -64,17 +65,17 @@ TEST_P(SetGetRGBATest, SetGetTest) { } // Check that 0 maps to 0 and 1.0f maps to 1.0f. - const float pixel_zero[4] = {0.0f, 0.0f, 0.0f, 1.0f}; - avifSetRGBAPixel(&rgb, 0, 0, &color_space, pixel_zero); - avifGetRGBAPixel(&rgb, 0, 0, &color_space, pixel_read); + const std::array pixel_zero = {0.0f, 0.0f, 0.0f, 1.0f}; + avifSetRGBAPixel(&rgb, 0, 0, &color_space, pixel_zero.data()); + avifGetRGBAPixel(&rgb, 0, 0, &color_space, pixel_read.data()); EXPECT_EQ(pixel_read[0], pixel_zero[0]); EXPECT_EQ(pixel_read[1], pixel_zero[1]); EXPECT_EQ(pixel_read[2], pixel_zero[2]); EXPECT_EQ(pixel_read[3], pixel_zero[3]); - const float pixel_one[4] = {1.0f, 1.0f, 1.0f, 1.0f}; - avifSetRGBAPixel(&rgb, 0, 0, &color_space, pixel_one); - avifGetRGBAPixel(&rgb, 0, 0, &color_space, pixel_read); + const std::array pixel_one = {1.0f, 1.0f, 1.0f, 1.0f}; + avifSetRGBAPixel(&rgb, 0, 0, &color_space, pixel_one.data()); + avifGetRGBAPixel(&rgb, 0, 0, &color_space, pixel_read.data()); EXPECT_EQ(pixel_read[0], pixel_one[0]); EXPECT_EQ(pixel_read[1], pixel_one[1]); EXPECT_EQ(pixel_read[2], pixel_one[2]); @@ -103,9 +104,9 @@ TEST_P(SetGetRGBATest, GradientTest) { for (uint32_t j = 0; j < input_rgb.height; ++j) { for (uint32_t i = 0; i < input_rgb.width; ++i) { - float pixel[4]; - avifGetRGBAPixel(&input_rgb, i, j, &color_space, pixel); - avifSetRGBAPixel(&output_rgb, i, j, &color_space, pixel); + std::array pixel; + avifGetRGBAPixel(&input_rgb, i, j, &color_space, pixel.data()); + avifSetRGBAPixel(&output_rgb, i, j, &color_space, pixel.data()); } } EXPECT_TRUE(testutil::AreImagesEqual(input_rgb, output_rgb)); From eee3399666bd33607abce6e2b6c2996de764c7d1 Mon Sep 17 00:00:00 2001 From: metsw24-max Date: Thu, 21 May 2026 19:30:44 +0530 Subject: [PATCH 3/4] updated --- CMakeLists.txt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a693be4cad..d7f13d53ca 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -573,10 +573,16 @@ endif() # (the first step of adopting the Safe Buffers Programming Model). Apply only # to .cc files that have been audited to be free of raw pointer arithmetic and # raw-pointer indexing; new code added to those files must keep them clean. +# The libc-call subgroup (Clang 17+) is disabled because it fires on functions +# such as strdup() that are used in third-party headers (e.g. libargparse) we +# do not own; our own code avoids those by convention. # No-op on toolchains where the flag is not available (Clang < 16, GCC, MSVC). function(avif_enable_safe_buffers_warning) if(AVIF_HAVE_WUNSAFE_BUFFER_USAGE) - set_source_files_properties(${ARGN} PROPERTIES COMPILE_OPTIONS "-Wunsafe-buffer-usage") + set_source_files_properties( + ${ARGN} PROPERTIES COMPILE_OPTIONS + "-Wunsafe-buffer-usage;-Wno-unsafe-buffer-usage-in-libc-call" + ) endif() endfunction() From 36d9282b1c16724a9b7e6259ce562e272ab861a9 Mon Sep 17 00:00:00 2001 From: metsw24-max Date: Thu, 21 May 2026 23:06:42 +0530 Subject: [PATCH 4/4] updated --- apps/avifgainmaputil/avifgainmaputil.cc | 32 ++++++++++--------------- tests/CMakeLists.txt | 1 - tests/gtest/avifrgbtest.cc | 27 ++++++++++----------- 3 files changed, 26 insertions(+), 34 deletions(-) diff --git a/apps/avifgainmaputil/avifgainmaputil.cc b/apps/avifgainmaputil/avifgainmaputil.cc index 25949f5354..dce2f360e6 100644 --- a/apps/avifgainmaputil/avifgainmaputil.cc +++ b/apps/avifgainmaputil/avifgainmaputil.cc @@ -91,30 +91,19 @@ int main(int argc, char** argv) { return 1; } - // argv is a raw pointer with no associated bound, so -Wunsafe-buffer-usage - // cannot prove these accesses safe. They are bounded by the argc checks - // above and by the host's contract with main(): argv[0..argc-1] are valid. - const char* command_cstr; - const char* sub_command_cstr = nullptr; - char* const* command_argv; + // -Wunsafe-buffer-usage doesn't know the raw pointer argv is associated with + // the bound argc, so it cannot prove these accesses safe. They are bounded + // by the argc checks and by the host's contract with main(): + // argv[0..argc-1] are valid. #ifdef __clang__ #if __has_warning("-Wunsafe-buffer-usage") #pragma clang unsafe_buffer_usage begin #endif #endif - command_cstr = argv[1]; - if (argc >= 3) sub_command_cstr = argv[2]; - command_argv = argv + 1; -#ifdef __clang__ -#if __has_warning("-Wunsafe-buffer-usage") -#pragma clang unsafe_buffer_usage end -#endif -#endif - - const std::string command_name(command_cstr); + const std::string command_name(argv[1]); if (command_name == "help") { - if (sub_command_cstr != nullptr) { - const std::string sub_command_name(sub_command_cstr); + if (argc >= 3) { + const std::string sub_command_name(argv[2]); for (const auto& command : commands) { if (command->name() == sub_command_name) { command->PrintUsage(); @@ -133,7 +122,7 @@ int main(int argc, char** argv) { for (const auto& command : commands) { if (command->name() == command_name) { try { - avifResult result = command->ParseArgs(argc - 1, command_argv); + avifResult result = command->ParseArgs(argc - 1, argv + 1); if (result == AVIF_RESULT_OK) { result = command->Run(); } @@ -149,6 +138,11 @@ int main(int argc, char** argv) { } } } +#ifdef __clang__ +#if __has_warning("-Wunsafe-buffer-usage") +#pragma clang unsafe_buffer_usage end +#endif +#endif std::cerr << "Unknown command " << command_name << "\n"; avif::PrintUsage(commands); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 546fa66fb8..9ddffbdf06 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -91,7 +91,6 @@ if(AVIF_GTEST) gtest/avifimagetest.cc gtest/avifopaquetest.cc gtest/avifpropinternaltest.cc - gtest/avifrgbtest.cc gtest/avifsampletransformtest.cc gtest/aviftilingtest.cc gtest/avifutilstest.cc diff --git a/tests/gtest/avifrgbtest.cc b/tests/gtest/avifrgbtest.cc index 8af4440bf7..faaf47aba3 100644 --- a/tests/gtest/avifrgbtest.cc +++ b/tests/gtest/avifrgbtest.cc @@ -1,7 +1,6 @@ // Copyright 2023 Google LLC // SPDX-License-Identifier: BSD-2-Clause -#include #include #include "avif/internal.h" @@ -41,18 +40,18 @@ TEST_P(SetGetRGBATest, SetGetTest) { epsilon = 0.0005f; // Half precision floats are not that precise. } - std::array pixel_read; + float pixel_read[4]; for (uint32_t j = 0; j < rgb.height; ++j) { for (uint32_t i = 0; i < rgb.width; ++i) { // Generate some arbitrary pixel values. - const std::array pixel_to_write = { + const float pixel_to_write[4] = { 0.0f + static_cast(i) / rgb.width, 0.5f + static_cast(j) / (rgb.height * 2), 1.0f - static_cast(i + j) / ((rgb.width + rgb.height) * 2), 1.0f - static_cast(i) / rgb.width}; - avifSetRGBAPixel(&rgb, i, j, &color_space, pixel_to_write.data()); - avifGetRGBAPixel(&rgb, i, j, &color_space, pixel_read.data()); + avifSetRGBAPixel(&rgb, i, j, &color_space, pixel_to_write); + avifGetRGBAPixel(&rgb, i, j, &color_space, pixel_read); EXPECT_NEAR(pixel_read[0], pixel_to_write[0], epsilon); EXPECT_NEAR(pixel_read[1], pixel_to_write[1], epsilon); EXPECT_NEAR(pixel_read[2], pixel_to_write[2], epsilon); @@ -65,17 +64,17 @@ TEST_P(SetGetRGBATest, SetGetTest) { } // Check that 0 maps to 0 and 1.0f maps to 1.0f. - const std::array pixel_zero = {0.0f, 0.0f, 0.0f, 1.0f}; - avifSetRGBAPixel(&rgb, 0, 0, &color_space, pixel_zero.data()); - avifGetRGBAPixel(&rgb, 0, 0, &color_space, pixel_read.data()); + const float pixel_zero[4] = {0.0f, 0.0f, 0.0f, 1.0f}; + avifSetRGBAPixel(&rgb, 0, 0, &color_space, pixel_zero); + avifGetRGBAPixel(&rgb, 0, 0, &color_space, pixel_read); EXPECT_EQ(pixel_read[0], pixel_zero[0]); EXPECT_EQ(pixel_read[1], pixel_zero[1]); EXPECT_EQ(pixel_read[2], pixel_zero[2]); EXPECT_EQ(pixel_read[3], pixel_zero[3]); - const std::array pixel_one = {1.0f, 1.0f, 1.0f, 1.0f}; - avifSetRGBAPixel(&rgb, 0, 0, &color_space, pixel_one.data()); - avifGetRGBAPixel(&rgb, 0, 0, &color_space, pixel_read.data()); + const float pixel_one[4] = {1.0f, 1.0f, 1.0f, 1.0f}; + avifSetRGBAPixel(&rgb, 0, 0, &color_space, pixel_one); + avifGetRGBAPixel(&rgb, 0, 0, &color_space, pixel_read); EXPECT_EQ(pixel_read[0], pixel_one[0]); EXPECT_EQ(pixel_read[1], pixel_one[1]); EXPECT_EQ(pixel_read[2], pixel_one[2]); @@ -104,9 +103,9 @@ TEST_P(SetGetRGBATest, GradientTest) { for (uint32_t j = 0; j < input_rgb.height; ++j) { for (uint32_t i = 0; i < input_rgb.width; ++i) { - std::array pixel; - avifGetRGBAPixel(&input_rgb, i, j, &color_space, pixel.data()); - avifSetRGBAPixel(&output_rgb, i, j, &color_space, pixel.data()); + float pixel[4]; + avifGetRGBAPixel(&input_rgb, i, j, &color_space, pixel); + avifSetRGBAPixel(&output_rgb, i, j, &color_space, pixel); } } EXPECT_TRUE(testutil::AreImagesEqual(input_rgb, output_rgb));