[Maria-developers] Error Compiling MariaDB with clang on Mac Os X
Hi Sergei, While working on features for 10.2 I've encountered a problem when compiling MariaDB as a Debug build. According to your change on commit 29dd634a4c0b9c3579cf9d318ed64d748d848b1d dbug: correct trace for DBUG_RETURN(func()); This causes a small problem, in two places within the codebase. One is in the myisam storage engine and one in the maria storage engine. Within thr_find_all_keys, there is a hack with goto statements and using DBUG_ENTER after my_thread_init(). /Users/cvicentiu/Workspace/MariaDB/storage/myisam/sort.c:352:5: error: cannot jump from this goto statement to its label goto err; ^ /Users/cvicentiu/Workspace/MariaDB/storage/myisam/sort.c:355:5: note: jump bypasses initialization of variable with __attribute__((cleanup)) DBUG_ENTER("thr_find_all_keys"); ^ /Users/cvicentiu/Workspace/MariaDB/include/my_dbug.h:74:47: note: expanded from macro 'DBUG_ENTER' #define DBUG_ENTER(a) struct _db_stack_frame_ _db_stack_frame_ __attribute__((cleanup(_db_return_))); \ In the current code state, the attribute_cleanup callback does not make sense if my_thread_init() fails and forces the goto err; Technically, it is undefined behaviour, as we're jumping within the block that has the variables defined, without initialising it. The sensible thing to do is to refactor the block that has DBUG_ENTER into a separate function and decoupling the error cleanup from the my_thread_init() call. I've looked at that code and have seen that much of the logic is redundant during the cleanup part. I can refactor it to fix the problem, but I'm looking for input on your side if you're ok with me doing that. I've also found a bug in that same code in the error checking that I can fix. Regards, Vicentiu
Hi, Vicențiu! On Dec 01, Vicențiu Ciorbaru wrote:
Hi Sergei,
While working on features for 10.2 I've encountered a problem when compiling MariaDB as a Debug build.
According to your change on commit 29dd634a4c0b9c3579cf9d318ed64d748d848b1d dbug: correct trace for DBUG_RETURN(func());
This causes a small problem, in two places within the codebase. One is in the myisam storage engine and one in the maria storage engine. Within thr_find_all_keys, there is a hack with goto statements and using DBUG_ENTER after my_thread_init().
/Users/cvicentiu/Workspace/MariaDB/storage/myisam/sort.c:352:5: error: cannot jump from this goto statement to its label goto err; ^ In the current code state, the attribute_cleanup callback does not make sense if my_thread_init() fails and forces the goto err; Technically, it is undefined behaviour, as we're jumping within the block that has the variables defined, without initialising it.
That's, probably, even worse. At the err: label there's a cleanup, like close_cached_file(&sort_param->tempfile); but if we jump into the block, sort_param->tempfile will be not initialized. That could cause a crash. This first goto should be to the very end, to my_thread_end().
The sensible thing to do is to refactor the block that has DBUG_ENTER into a separate function and decoupling the error cleanup from the my_thread_init() call. I've looked at that code and have seen that much of the logic is redundant during the cleanup part. I can refactor it to fix the problem, but I'm looking for input on your side if you're ok with me doing that.
Yes, sure, go ahead. But please let me see the patch, before pushing.
I've also found a bug in that same code in the error checking that I can fix.
Sounds good. Regards, Sergei Chief Architect MariaDB and security@mariadb.org -- Vote for my Percona Live 2016 talks: https://www.percona.com/live/data-performance-conference-2016/sessions/maria... https://www.percona.com/live/data-performance-conference-2016/sessions/maria...
participants (2)
-
Sergei Golubchik -
Vicențiu Ciorbaru