Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-4224

[PATCH] Item_func_neg relies on undefined behavior and can misbehave

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 10.0.1
    • Fix Version/s: 10.0.2, 5.5.31
    • Component/s: None
    • Labels:
      None

      Description

      Class Item_func_neg has a lot of conversions between signed and unsigned datatypes. I believe at least to some degree these conversions are undefined behavior in C++, and thus Item_func_neg can misbehave when compiled with optimizations. In particular I've experienced misbehavior in the main.func_math test on line 483. This line expects that negation of -9223372036854775808 will throw DATA_OUT_OF_RANGE error, but sometimes it doesn't. The following patch fixes the problem. Please consider including it (or some modification of it) into MariaDB.

      --- a/sql/item_func.cc
      +++ b/sql/item_func.cc
      @@ -1908,6 +1908,8 @@ longlong Item_func_neg::int_op()
         if (args[0]->unsigned_flag &&
             (ulonglong) value > (ulonglong) LONGLONG_MAX + 1)
           return raise_integer_overflow();
      +  if (!args[0]->unsigned_flag && value == LONGLONG_MIN)
      +    return raise_integer_overflow();
         return check_integer_overflow(-value, !args[0]->unsigned_flag && value < 0);
       }
       
      

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            elenst Elena Stepanova added a comment -

            Assigned to Wlad arbitrarily, just because I remember (possibly wrongly) him having an opinion on how to deal with longlong. Please reassign if needed.

            Show
            elenst Elena Stepanova added a comment - Assigned to Wlad arbitrarily, just because I remember (possibly wrongly) him having an opinion on how to deal with longlong. Please reassign if needed.
            Hide
            wlad Vladislav Vaintroub added a comment -

            "This line expects that negation of -9223372036854775808 will throw DATA_OUT_OF_RANGE error, but sometimes it doesn't" .Pavel , can you please describe the environment necessary to reproduce this problem "sometimes does not"?

            Show
            wlad Vladislav Vaintroub added a comment - "This line expects that negation of -9223372036854775808 will throw DATA_OUT_OF_RANGE error, but sometimes it doesn't" .Pavel , can you please describe the environment necessary to reproduce this problem "sometimes does not"?
            Hide
            pivanof Pavel Ivanov added a comment -

            Here are my reproduction steps:

            > clang --version
            Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final) (based on LLVM 3.0)
            Target: x86_64-pc-linux-gnu
            Thread model: posix
            > CC=clang CXX=clang++ cmake . DCMAKE_BUILD_TYPE=Release
            ...
            > make
            ...
            > cd mysql-test/
            > ./mtr main.func_math
            ...
            mysqltest: At line 483: query 'SELECT -a FROM t1' succeeded - should have failed with errno 1690...
            ...

            Show
            pivanof Pavel Ivanov added a comment - Here are my reproduction steps: > clang --version Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final) (based on LLVM 3.0) Target: x86_64-pc-linux-gnu Thread model: posix > CC=clang CXX=clang++ cmake . DCMAKE_BUILD_TYPE=Release ... > make ... > cd mysql-test/ > ./mtr main.func_math ... mysqltest: At line 483: query 'SELECT -a FROM t1' succeeded - should have failed with errno 1690... ...

              People

              • Assignee:
                wlad Vladislav Vaintroub
                Reporter:
                pivanof Pavel Ivanov
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 1 day
                  1d