Details
-
Task
-
Status: Stalled (View Workflow)
-
Major
-
Resolution: Unresolved
-
Sprint 7 (07.04.2025)
Description
There are a number of compiler defines that fake initialization LINT_INIT and UNINIT_VAR for example.
With modern static analysers and the current usage, these are very shallow path to determine the initialisation did happen.
Its better to catch these uninitialised values.
Faking over the initialisation of variables in debug mode is likely to have a release build without the correct initialization.
The UNINIT_VAR(x) x= x construct was causing UBSAN issues like:
sql/sql_view.cc:1299:8: runtime error: load of value 16, which is not a valid value for type 'bool'
|
Attachments
Issue Links
- blocks
-
MDBF-741 Remove the gcc UBSAN builder to use the clang based UBSAN
-
- In Testing
-
- relates to
-
-
- Open
-
-
-
- Open
-
Activity
The problem is that people want to compile with options like -Wall -Werror, and compilers want to warn with false positives about uninitialized variables.
And without a mechanism to deal with this, people will then add random initializations to variables just to prevent those compiler warnings (->errors), regardless of whether that initial value is actually correct.
And then this in turn hides real bugs from tools like Valgrind and MSAN (just as you mentioned in the bug description), when a variable that should not be referenced is accessed incorrectly.
Remember, just because a variable is initialized at the definition of it, doesn't mean it has a valid value!
Maybe the UNINIT_VAR() macro could be extended so it takes also an initial value, but then marks the variable as undefined memory to Valgrind and MSAN?
> and compilers want to warn with false positives about uninitialized variables
I think the assumption of an abundance of warnings isn't the case, perhaps due to over initialisation:
$ cmake -DCMAKE_C{,XX}_FLAGS='-Wall' .
|
$ b
|
[256/1577] Building CXX object tpool/CMakeFiles/tpool.dir/tpool_generic.cc.o
|
/home/dan/repos/mariadb-server-rebase/tpool/tpool_generic.cc:221:22: warning: private field 'm_group_enqueued' is not used [-Wunused-private-field]
|
221 | unsigned long long m_group_enqueued;
|
| ^
|
1 warning generated.
|
[794/1577] [BISON][gen_oracle_cc_hh] Building parser with bison 3.8.2
|
/home/dan/repos/build-mariadb-server-rebase-debug/sql/yy_oracle.cc:35313: warning: suspicious sequence in the output: b4_bin [-Wother]
|
[1533/1577] Generating user.t
|
troff: fatal error: unable to open macro file for -m argument 'm'
|
[1535/1577] Generating user.ps
|
troff: fatal error: unable to open macro file for -m argument 'm'
|
[1577/1577] Linking CXX executable sql/mariadbd
|
> Maybe the UNINIT_VAR() macro could be extended so it takes also an initial value
Why?
There's no need for such an abstracted fakery. If you want an uninitialised variable just declare that way. Plenty are already, and they aren't generating warnings or MSAN errors.
The point of valgrind and MSAN is to run instrumented but unmodified code, once you run modified code, you're testing a different product.
danblack, m_group_enqueued warning is genuine, this variable it is not used anywhere, and it is remainder after something I've tried. But there was never a warning that it is not used, or not initialized, or anything
Going to apply some gcc and clang version based upper limits to the code so the really old buildbot works have the same number of warnings however anyone using a modern compiler effectively has a UNINITVAR no-op to catch the real warnings around uninitialised variables and the other odd thing or so like what wlad saw.
Monty, can I get a review of https://github.com/MariaDB/server/pull/3961 please.