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

fix unaligned access UB in sint4korr() and similar functions

Details

    • Bug
    • Status: Closed (View Workflow)
    • Major
    • Resolution: Fixed
    • 5.5(EOL), 10.0(EOL), 10.1(EOL), 10.2(EOL), 10.3(EOL), 10.4(EOL)
    • 10.2.24
    • Compiling, Server
    • None

    Description

      Exaple UB:

      #define sint2korr(A)	(int16) (*((int16 *) (A)))
      

      (int16*) casts uchar* to a bigger aligned which is UB. There is not knows observable harm but this stuff makes A LOT of noise for UBSAN builds. This reason is alone enough to fix it.

      One way to fix it is by using memcpy(). GCC, Clang and MSVC optimizes it away in release builds.

      There is also a room for micro optimizations while preserving correctness. Here is an example from marko and me:

      #define saint3korr(A) ((int32)((char)(A[2]) << 16 | (int32)(uchar)A[1] << 8 | (int32)(uchar)A[0]))
      

      Probably it's also a good idea to replace macros with functions. This will bring a bit of type safety and will allow to put breakpoints on these functions.

      Attachments

        Activity

          Yes, recent GCC would optimize away memcpy() on x86 and amd64, but not on all platforms that I tried on http://godbolt.org/. For older server versions that may be built on an older compiler, we probably should avoid memcpy(). Or we should put some compiler-version check into the code.

          I agree that it would be a good idea to define the macros with inline functions.

          marko Marko Mäkelä added a comment - Yes, recent GCC would optimize away memcpy() on x86 and amd64, but not on all platforms that I tried on http://godbolt.org/ . For older server versions that may be built on an older compiler, we probably should avoid memcpy() . Or we should put some compiler-version check into the code. I agree that it would be a good idea to define the macros with inline functions.

          Comments in the PR.

          svoj Sergey Vojtovich added a comment - Comments in the PR.
          kevg Eugene Kosov (Inactive) added a comment - Issue was fixed in https://github.com/MariaDB/server/commit/bcc1359223c254e7649383c0993b6d27d5002c5d

          People

            kevg Eugene Kosov (Inactive)
            kevg Eugene Kosov (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Git Integration

                Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.