MDEV-37502 Make Server Debug + Clang compile ready #4145

grooverdan · 2025-06-25T04:29:02Z

The Jira issue number for this PR is: MDEV-37502

Description

Clang under a debug build triggered a few too many -Werror conditions on stack.

The code on mroonga is kept be ready for its update #3864 where these directives aren’t' necessary.

https://github.com/MariaDB/server/pull/3961/files#diff-8adababce2dec657ca89490f1922da7d4d0ec4db824261594114d9542bc575cc

Didn't seem to hit this in 10.11 so this patch is omitted.

Release Notes

nothing - compile/debug only.

How can this PR be tested?

Compile with clang and -DCMAKE_BUILD_TYPE=Debug

Basing the PR against the correct MariaDB version

This is a new feature or a refactoring, and the PR is based against the main branch.
This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

ParadoxV5

Are these blocking releases?
Because I would rather not publish temporary kludges if we can focus on proper solutions.

While I wonder why there exists -Wframe-larger-than only to shackle so many places, I’m equally curious about how those places manage to exhaust all 16KiB of the allowance.

storage/maria/ma_loghandler.c

ParadoxV5 · 2025-07-22T16:44:07Z

storage/mroonga/vendor/groonga/CMakeLists.txt

Regarding Groonga, I heard that Mroonga is looking to address issues as part of #4185, including your comment on the stack size, #3864 (comment).

Let’s tune in on their progress first and ask if they’ll support 10.11.

The PR hasn’t had any activity for just over a month. Do we know what’s up?

3 weeks on mroonga/mroonga#900 so I pinged. Thanks.

ParadoxV5 · 2025-07-22T16:49:21Z

Marko closed MDEV-34388 two months ago. You may prefer to track these follow-ups on a split issue.

storage/mroonga/vendor/groonga/plugins/tokenizers/mecab.c

grooverdan · 2025-08-05T23:51:42Z

3c0e114#diff-dba75117b883ebb66223a784bdc33554eb05a18864e35a40f097eb57161da91c

ParadoxV5 · 2025-08-30T02:01:39Z

client/mysql.cc

+    warning: passing an object that undergoes default argument promotion
+    to 'va_start' has undefined behavior [-Wvarargs]


This is new since my last review, right?

I don’t get how this warning could arise.
Can you paste me the full message?

UBSAN makes sure the functions match in args.

SUMMARY: UndefinedBehaviorSanitizer: function-type-mismatch /source/libmariadb/libmariadb/mariadb_lib.c:2723:17 --- /source/mysql-test/main/mysqlbinlog.result 2025-07-31 08:07:44.470657901 +0000 +++ /build/mysql-test/var/5/log/mysqlbinlog.reject 2025-09-01 01:40:41.565758662 +0000 @@ -904,6 +904,50 @@ DROP TABLE t1; DROP DATABASE test1; FLUSH LOGS; +/source/libmariadb/libmariadb/mariadb_lib.c:2628:3: runtime error: call to function status_info_cb(void*, enum_mariadb_status_info, enum_session_state_type, st_ma_const_string*) through pointer to incorrect function type 'void (*)(void *, enum enum_mariadb_status_info, ...)' +/source/client/mysql.cc:3204: note: status_info_cb(void*, enum_mariadb_status_info, enum_session_state_type, st_ma_const_string*) defined here + #0 0x5585a84e8211 in ma_read_ok_packet /source/libmariadb/libmariadb/mariadb_lib.c:2628:3 + #1 0x5585a857f920 in run_plugin_auth /source/libmariadb/plugins/auth/my_auth.c:744:12 + #2 0x5585a84de81b in mthd_my_real_connect /source/libmariadb/libmariadb/mariadb_lib.c:1982:7 + #3 0x5585a84da759 in mysql_real_connect /source/libmariadb/libmariadb/mariadb_lib.c:1555:10 + #4 0x5585a84b4c93 in do_connect(st_mysql*, char const*, char const*, char const*, char const*, unsigned long) /source/client/mysql.cc:1521:10 + #5 0x5585a849eb1a in sql_real_connect(char*, char*, char*, char*, unsigned int) /source/client/mysql.cc:5017:8 + #6 0x5585a849eb1a in sql_connect(char*, char*, char*, char*, unsigned int) /source/client/mysql.cc:5074:16 + #7 0x5585a849da79 in main /source/client/mysql.cc:1304:7 + #8 0x7f9db31fc249 (/lib/x86_64-linux-gnu/libc.so.6+0x27249) (BuildId: 79005c16293efa45b441fed45f4f29b138557e9e) + #9 0x7f9db31fc304 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x27304) (BuildId: 79005c16293efa45b441fed45f4f29b138557e9e) + #10 0x5585a83b2f80 in _start (/build/client/mariadb+0x1b2f80) (BuildId: b015c96ff77d7c9ddf4f3547b595f7bdd9eb9a30)

Oh, why don’t we change this static function to variadic to match the struct member?

static void status_info_cb(void *data, enum enum_mariadb_status_info type, ...) { enum enum_session_state_type state_type; [[maybe_unused]] MARIADB_CONST_STRING *val; va_list args; va_start(args, fmt); state_type= va_arg(args, int); // enum is promoted to 'int' when passed through '...' if (…) { val= va_arg(args, MARIADB_CONST_STRING *); … } va_end(args); }

Alternatively, it looks like Connector/C only calls the function passed to MARIADB_OPT_STATUS_CALLBACK with specific argument types, which means st_mysql_options_extension::status_callback doesn’t need to be variadic.
Though switching to non-variadic would impact user projects that were passing variadic callbacks already. Bug fixes should not have API changes, after all.

It used to 3e43606 - a variadic is a compile warning on undefined behaviour.

The same C/C callback is called in a few ways:
mariadb-corporation/mariadb-connector-c@a8832af

But yes, too late to change API with undefined behaviour.

Googling a bit…
Looks like this -Wvarargs problem comes from preceding the varargs with an enum parameter.
See this example and its explanation: mamh-mixed/glusterfs@11ea746

It means the API itself is problematic, and there aren’t really “user projects with correct signatures” because the “correct” signature is stuck with UB.
The best a caller could do is to write int instead of enum enum_mariadb_status_info with fingers crossed.

We should inform the Connector/C team.

@ParadoxV5 I'm happy to revert 3e43606 and apply a disable of the warning varargs around this block too as an alternate to this.

In the just deployed amd64-ubasan-clang-20 its causing the test failures to hit the 30 limit rather quickly.

While we wait for Con/C to solve this, yap, let’s do that instead, as it is one layer of kludge thinner.

ack, have done this now.

ParadoxV5 · 2025-08-30T02:10:20Z

storage/mroonga/vendor/groonga/CMakeLists.txt

The PR hasn’t had any activity for just over a month. Do we know what’s up?

storage/rocksdb/CMakeLists.txt

grooverdan · 2025-09-03T06:27:03Z

storage/rocksdb/build_rocksdb.cmake

  # Workaround Win8.1 SDK bug, that breaks /permissive-
  string(REPLACE "/permissive-" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
 else()
+  set_source_files_properties(${ROCKSDB_SOURCE_DIR}/options/db_options.cc


@dr-m @vlad-lesin on the assumption that the two warnings in the commit message are what was seen, I pushed the warning down to just these two files. I didn't use CHECK_CXX_COMPILER_FLAG because like many of the other things in the build_rocksdb.cmake file that assumed clang or gcc (because of the other flags used).

Could you add a couple of commas to your GitHub comment here?

Update: The comment has been updated inline.

storage/mroonga/vendor/groonga/lib/ii.c

ParadoxV5

I do not know what happened to the codebase that led to these issues.
But I acknowledge your patch, including parts that are currently “waiting for the dependency to update”.

storage/mroonga/vendor/groonga/lib/ii.c

storage/mroonga/vendor/groonga/plugins/tokenizers/mecab.c

ParadoxV5 · 2025-09-10T21:44:23Z

storage/rocksdb/build_rocksdb.cmake

  # Workaround Win8.1 SDK bug, that breaks /permissive-
  string(REPLACE "/permissive-" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
 else()
+  set_source_files_properties(${ROCKSDB_SOURCE_DIR}/options/db_options.cc


Could you add a couple of commas to your GitHub comment here?

Update: The comment has been updated inline.

This is frequently violated within the mroonga implementatation and therefore should not error. Reviewer: Jimmy Hu <jimmy.hu@mariadb.com>

Stack limits exceeded for clang-20 +Debug + MSAN. Added disables for the following functions. storage/mroonga/vendor/groonga/lib/ii.c:303:1: error: stack frame size (16696) exceeds limit (16384) in 'buffer_segment_reserve' [-Werror,-Wframe-larger-than] 303 | buffer_segment_reserve(grn_ctx *ctx, grn_ii *ii, | ^ storage/mroonga/vendor/groonga/lib/ii.c:4803:1: error: stack frame size (20936) exceeds limit (16384) in 'grn_ii_delete_one' [-Werror,-Wframe-larger-than] 4803 | grn_ii_delete_one(grn_ctx *ctx, grn_ii *ii, grn_id tid, grn_ii_updspec *u, grn_hash *h) | ^ storage/mroonga/vendor/groonga/lib/ii.c:6313:1: error: stack frame size (25736) exceeds limit (16384) in 'grn_ii_column_update' [-Werror,-Wframe-larger-than] 6313 | grn_ii_column_update(grn_ctx *ctx, grn_ii *ii, grn_id rid, unsigned int section, | ^ For non-Debug the following stack frame sizes wher exceeded: storage/mroonga/vendor/groonga/lib/proc/proc_select.c:3575:1: warning: stack frame size (94072) exceeds limit (49152) in 'command_select' [-Wframe-larger-than] 3575 | command_select(grn_ctx *ctx, int nargs, grn_obj **args, grn_user_data *user_data) | ^ storage/mroonga/vendor/groonga/lib/proc/proc_schema.c:1134:1: warning: stack frame size (98360) exceeds limit (49152) in 'command_schema_output_tables' [-Wframe-larger-than] 1134 | command_schema_output_tables(grn_ctx *ctx, grn_schema_data *data) Reviewer: Jimmy Hu <jimmy.hu@mariadb.com>

…lags sign-compare Under Debug build this becomes a Werror. Resolved this by changing the grn_mecab_chunk_size_threshold to a ptrdiff_t along with chunked_tokenize_utf8's string_bytes argument so there is no need to case. Reviewer: Jimmy Hu <jimmy.hu@mariadb.com>

Follow up to MDEV-34388 82d7419 Relax limit on specific files only. clang-20 + CMAKE_BUILD_TYPE=Debug: options/cf_options.cc:0:0: stack frame size (17624) exceeds limit (16384) in function '__cxx_global_var_init.33' options/db_options.cc:0:0: stack frame size (34328) exceeds limit (32768) in function '__cxx_global_var_init.45' Reviewer: Jimmy Hu <jimmy.hu@mariadb.com>

…formation (fix)" This reverts commit 3e43606. This caused Clang-18+ UBSAN errors: SUMMARY: UndefinedBehaviorSanitizer: function-type-mismatch libmariadb/libmariadb/mariadb_lib.c:2723:17 +/libmariadb/libmariadb/mariadb_lib.c:2628:3: runtime error: call to function status_info_cb(void*, enum_mariadb_status_info, enum_session_state_type, st_ma_const_string*) through pointer to incorrect function type 'void (*)(void *, enum enum_mariadb_status_info, ...)' +/client/mysql.cc:3204: note: status_info_cb(void*, enum_mariadb_status_info, enum_session_state_type, st_ma_const_string*) defined here Reviewer: Jimmy Hu <jimmy.hu@mariadb.com>

Clang complains that the callback is using a variadic based on an enum. client/mysql.cc:3207:16: error: passing an object that undergoes default argument promotion to 'va_start' has undefined behavior [-Werror,-Wvarargs] 3207 | va_start(ap, type); This is part of the C/C API this has been referred as bug: * CONC-789 MARIADB_OPT_STATUS_CALLBACK Variadic around enums is undefined behaviour In the mean time, we are just disabling the warning. Reviewer: Jimmy Hu <jimmy.hu@mariadb.com>

svoj added the MariaDB Foundation Pull requests created by MariaDB Foundation label Jun 27, 2025

grooverdan assigned ParadoxV5 Jul 22, 2025

grooverdan requested a review from ParadoxV5 July 22, 2025 05:31

ParadoxV5 reviewed Jul 22, 2025

View reviewed changes

ParadoxV5 removed their assignment Jul 22, 2025

ParadoxV5 reviewed Jul 22, 2025

View reviewed changes

storage/mroonga/vendor/groonga/plugins/tokenizers/mecab.c Outdated Show resolved Hide resolved

grooverdan mentioned this pull request Aug 19, 2025

MDBF-741 Sequence version of ubasan MariaDB/buildbot#815

Merged

3 tasks

grooverdan force-pushed the MDEV-34388_debug_clang branch 2 times, most recently from d46fa58 to a3f7d90 Compare August 27, 2025 00:25

grooverdan changed the title ~~MDEV-34388 debug clang~~ MDEV-37502 Make Server Debug + Clang compile ready Aug 27, 2025

grooverdan requested a review from ParadoxV5 August 27, 2025 03:58

ParadoxV5 reviewed Aug 30, 2025

View reviewed changes

grooverdan force-pushed the MDEV-34388_debug_clang branch from a3f7d90 to 89a38dd Compare September 3, 2025 03:01

grooverdan commented Sep 3, 2025

View reviewed changes

grooverdan commented Sep 4, 2025

View reviewed changes

storage/mroonga/vendor/groonga/lib/ii.c Show resolved Hide resolved

grooverdan force-pushed the MDEV-34388_debug_clang branch 2 times, most recently from 21ea528 to 8238122 Compare September 8, 2025 06:06

ParadoxV5 approved these changes Sep 10, 2025

View reviewed changes

grooverdan added 6 commits September 11, 2025 09:35

MDEV-37502: clang+debug mroonga remove -Wno-unused-but-set-variable

3a72de7

This is frequently violated within the mroonga implementatation and therefore should not error. Reviewer: Jimmy Hu <jimmy.hu@mariadb.com>

grooverdan force-pushed the MDEV-34388_debug_clang branch from 8238122 to c7f2de5 Compare September 10, 2025 23:39

grooverdan enabled auto-merge (rebase) September 10, 2025 23:48

grooverdan merged commit bf60478 into MariaDB:10.11 Sep 11, 2025
15 of 17 checks passed

grooverdan deleted the MDEV-34388_debug_clang branch September 11, 2025 00:05

		warning: passing an object that undergoes default argument promotion
		to 'va_start' has undefined behavior [-Wvarargs]

Uh oh!

MDEV-37502 Make Server Debug + Clang compile ready #4145

MDEV-37502 Make Server Debug + Clang compile ready #4145

Uh oh!

Conversation

grooverdan commented Jun 25, 2025 • edited by ParadoxV5 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Release Notes

How can this PR be tested?

Basing the PR against the correct MariaDB version

PR quality check

Uh oh!

ParadoxV5 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ParadoxV5 commented Jul 22, 2025

Uh oh!

Uh oh!

grooverdan commented Aug 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

grooverdan Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ParadoxV5 Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ParadoxV5 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ParadoxV5 Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

grooverdan commented Jun 25, 2025 •

edited by ParadoxV5

Loading

grooverdan Sep 3, 2025 •

edited

Loading

ParadoxV5 Sep 10, 2025 •

edited

Loading

ParadoxV5 Sep 10, 2025 •

edited

Loading