| Bug #90473 | sql_load.cc: bitwise comparison always evaluates to true | ||
|---|---|---|---|
| Submitted: | 17 Apr 2018 13:08 | Modified: | 18 Apr 2018 8:55 |
| Reporter: | Przemysław Skibiński (OCA) | Email Updates: | |
| Status: | Duplicate | Impact on me: | |
| Category: | MySQL Server: Compiling | Severity: | S3 (Non-critical) |
| Version: | 5.7 | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
[17 Apr 2018 14:05]
MySQL Verification Team
Hi, Sorry, but I do not see in our code what you are writing about. I am staring at our latest release, 5.7.21. I can not find that expression in sql_insert.cc and I can find only one such expression in sql_load.cc. That part is, hence, imprecise. If this is a patch in the yet unavailable release, then we can not verify it, because a code can change when it is finally released.
[17 Apr 2018 14:23]
Przemysław Skibiński
In short, you should change: + if (thd->slave_thread & ((SYSTEM_THREAD_SLAVE_SQL | + (SYSTEM_THREAD_SLAVE_WORKER))!=0)) to + if ((thd->slave_thread & (SYSTEM_THREAD_SLAVE_SQL | + SYSTEM_THREAD_SLAVE_WORKER))!=0) because the following is always true: (SYSTEM_THREAD_SLAVE_SQL |(SYSTEM_THREAD_SLAVE_WORKER))!=0
[17 Apr 2018 14:28]
Przemysław Skibiński
It concerns sql_load.cc in the current 5.7 branch: https://github.com/mysql/mysql-server/blob/5.7/sql/sql_load.cc#L450-L451
[17 Apr 2018 14:28]
MySQL Verification Team
Hi, Yes, we understand your point, but you did not reply to all of our questions .....
[17 Apr 2018 14:34]
Przemysław Skibiński
What was the question? I don't see any questions.
[17 Apr 2018 15:37]
MySQL Verification Team
The question is: Sorry, but I do not see in our code what you are writing about. I am staring at our latest release, 5.7.21. I can not find that expression in sql_insert.cc and I can find only one such expression in sql_load.cc. That part is, hence, imprecise. If this is a patch in the yet unavailable release, then we can not verify it, because a code can change when it is finally released.
[17 Apr 2018 16:05]
Przemysław Skibiński
As you may see in the bug header and in my comment, it concerns only sql_load.cc in the current 5.7 branch. I did not check 8.0. I just wanted to note that the issue comes from the following commit: https://github.com/mysql/mysql-server/commit/84fca39d685db285fea4b88f3b11a496fdea40b1 In this commit the author showed that his intentions were + if ((thd->slave_thread & (SYSTEM_THREAD_SLAVE_SQL | + SYSTEM_THREAD_SLAVE_WORKER))!=0) The above code can be found it the original commit in sql_insert.cc (2 times) but he made a mistake with parentheses in sql_load.cc.
[17 Apr 2018 16:20]
MySQL Verification Team
Hi! You are quite right. Verified as reported.
[18 Apr 2018 8:55]
Tor Didriksen
Fixed in current head of 5.7
- if (thd->slave_thread & ((SYSTEM_THREAD_SLAVE_SQL |
- (SYSTEM_THREAD_SLAVE_WORKER))!=0))
+ if ((thd->slave_thread &
+ (SYSTEM_THREAD_SLAVE_SQL | SYSTEM_THREAD_SLAVE_WORKER)) != 0)
commit b63b91b8727a4fb00961c3ee2ab6557eadeaef8d
Author: Tor Didriksen <tor.didriksen@oracle.com>
Date: Fri Feb 16 13:41:53 2018 +0100
Bug#27558169 BACKPORT TO 5.7 BUG #26826272: REMOVE GCC 8 WARNINGS [noclose]
MySQL 5.7 should compile warning-free (ie., maintainer mode should work)
also with GCC 8.
This is a backport of similar patches from current MySQL trunk.
Patches have been rewritten from C++11 to C++03
This patch fixes all warnings in Debug build
Description: gcc-8 shows a valid issue: [ 26%] Building CXX object sql/CMakeFiles/sql.dir/sql_load.cc.o /sql/sql_load.cc: In function ‘int mysql_load(THD*, sql_exchange*, TABLE_LIST*, List<Item>&, List<Item>&, List<Item>&, enum_duplicates, bool)’: /sql/sql_load.cc:451:59: error: bitwise comparison always evaluates to true [-Werror=tautological-compare] if (thd->slave_thread & ((SYSTEM_THREAD_SLAVE_SQL | ~~~~~~~~~~~~~~~~~~~~~~~~~~ (SYSTEM_THREAD_SLAVE_WORKER))!=0)) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~ How to repeat: Check the original commit at https://github.com/mysql/mysql-server/commit/84fca39d685db285fea4b88f3b11a496fdea40b1 Suggested fix: The original commit has 2 correct conditions in sql_insert.cc (2 times): + if ((thd->slave_thread & (SYSTEM_THREAD_SLAVE_SQL | + SYSTEM_THREAD_SLAVE_WORKER))!=0) There is one non-correct condition in sql_load.cc: + if (thd->slave_thread & ((SYSTEM_THREAD_SLAVE_SQL | + (SYSTEM_THREAD_SLAVE_WORKER))!=0))