c: error: allocation of insufficient size with GCC 14

[kou]

What feature or improvement would you like to see?

[ 59%] Building CXX object driver/postgresql/CMakeFiles/adbc_driver_postgresql_objlib.dir/database.cc.o
cd /data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/cpp-build/driver/postgresql && /bin/c++ -DADBC_EXPORTING -I/data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-adbc
-16/c/vendor/fmt/include -isystem /data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-adbc-16/c -isystem /data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-
adbc-16/c/include -isystem /tmp/local/include -isystem /usr/include/mit-krb5 -isystem /data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-adbc-16/c/vendor -isystem /
data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-adbc-16/c/driver -g -Og -std=gnu++17 -fPIC -Wall -Wextra -Wpedantic -Werror -Wno-unused-parameter -Wno-maybe-unini
tialized -MD -MT driver/postgresql/CMakeFiles/adbc_driver_postgresql_objlib.dir/result_helper.cc.o -MF CMakeFiles/adbc_driver_postgresql_objlib.dir/result_helper.cc.o.d -o C
MakeFiles/adbc_driver_postgresql_objlib.dir/result_helper.cc.o -c /data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-adbc-16/c/driver/postgresql/result_helper.cc
cd /data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/cpp-build/driver/postgresql && /bin/c++ -DADBC_EXPORTING -I/data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-adbc
-16/c/vendor/fmt/include -isystem /data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-adbc-16/c -isystem /data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-
adbc-16/c/include -isystem /tmp/local/include -isystem /usr/include/mit-krb5 -isystem /data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-adbc-16/c/vendor -isystem /
data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-adbc-16/c/driver -g -Og -std=gnu++17 -fPIC -Wall -Wextra -Wpedantic -Werror -Wno-unused-parameter -Wno-maybe-unini
tialized -MD -MT driver/postgresql/CMakeFiles/adbc_driver_postgresql_objlib.dir/database.cc.o -MF CMakeFiles/adbc_driver_postgresql_objlib.dir/database.cc.o.d -o CMakeFiles/
adbc_driver_postgresql_objlib.dir/database.cc.o -c /data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-adbc-16/c/driver/postgresql/database.cc
/data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-adbc-16/c/validation/adbc_validation_statement.cc: In member function 'void adbc_validation::StatementTest::TestE
rrorCompatibility()':
/data/arrow/adbc-verify-rc/arrow-adbc-16.vGfE0/apache-arrow-adbc-16/c/validation/adbc_validation_statement.cc:2821:17: error: allocation of insufficient size '32' for type 'AdbcError' with size '48' [-Werror=alloc-size]
 2821 |   auto* error = reinterpret_cast<struct AdbcError*>(malloc(ADBC_ERROR_1_0_0_SIZE));
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[kou]

$ c++ --version
c++ (Debian 14.2.0-8) 14.2.0
Copyright (C) 2024 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[kou]

This works but ignoring -Walloc-size may be better...:

diff --git a/c/validation/adbc_validation_statement.cc b/c/validation/adbc_validation_statement.cc
index cd388623b..e47f183d4 100644
--- a/c/validation/adbc_validation_statement.cc
+++ b/c/validation/adbc_validation_statement.cc
@@ -2817,8 +2817,9 @@ struct ADBC_EXPORT AdbcError100 {
 // Test that an ADBC 1.0.0-sized error still works
 void StatementTest::TestErrorCompatibility() {
   static_assert(sizeof(AdbcError100) == ADBC_ERROR_1_0_0_SIZE, "Wrong size");
+  auto error_buffer = malloc(ADBC_ERROR_1_0_0_SIZE);
   // XXX: sketchy cast
-  auto* error = reinterpret_cast<struct AdbcError*>(malloc(ADBC_ERROR_1_0_0_SIZE));
+  auto* error = reinterpret_cast<struct AdbcError*>(error_buffer);
   std::memset(error, 0, ADBC_ERROR_1_0_0_SIZE);
 
   ASSERT_THAT(AdbcStatementNew(&connection, &statement, error), IsOkStatus(error));
@@ -2831,7 +2832,7 @@ void StatementTest::TestErrorCompatibility() {
               ::testing::Not(IsOkStatus(error)));
   auto* old_error = reinterpret_cast<AdbcError100*>(error);
   old_error->release(old_error);
-  free(error);
+  free(error_buffer);
 }
 
 void StatementTest::TestResultInvalidation() {

[lidavidm]

Hmm, that's intentional to test the binary compatibility, but it is indeed sketchy and GCC is right to warn about it...

[lidavidm]

We should also set up a job with new GCC (and I should backport the nanoarrow patch so that the current job using new Clang can pass)

[paleolimbot]

FWIW that line has caused me to have to turn of UBSAN/ASAN for my local build for quite some time (since Apple clang aborts that test at runtime if compiled with the default CMakeUserPresets.json).

[lidavidm]

Maybe I can change it to allocate the full struct and just check that the new fields were not touched?

[kou]

Ah, it's a good idea.