Commit 58f184a4 authored by Marko Mäkelä's avatar Marko Mäkelä

MDEV-24745 Generic CRC-32C computation wrongly uses SSE4.2 instructions

In commit d25f806d (MDEV-22749)
the CRC-32C implementation of MariaDB was broken on some
IA-32 and AMD64 builds, depending on the compiler version and
build options. This was verified for IA-32 on GCC 10.2.1.

Even though we try to identify the SSE4.2 extensions and the
availaibility of the PCLMULQDQ instruction by executing CPUID,
the fall-back code could be generated with extended instructions,
because the entire file mysys/crc32/crc32c.c was being compiled
with -msse4.2 -mpclmul. This would cause SIGILL on a PINSRD
instruction on affected IA-32 targets (such as some Intel Atom
processors). This might also affect old AMD64 processors
(predating the 2007 Intel Nehalem microarchitecture), if some
compiler chose to emit the offending instructions.

While it is fine to pass a target-specific option to a target-specific
compilation unit (like -mpclmul to a PCLMUL-specific compilation unit),
that is not safe for mixed-architecture compilation units.

For mixed-architecture compilation units, the correct way is to set
target attributes on the target-specific functions.

There does not seem to be a way to pass target attributes to
function template instantiation. Hence, we must replace the
ExtendImpl template with plain functions crc32_sse42() and
crc32_slow().

We will also remove some inconsistency between
my_crc32_implementation() and mysys_namespace::crc32::Choose_Extend().

The function crc32_pclmul_enabled() will be moved to mysys/crc32/crc32c.cc
so that the detection code will be compiled without -msse4.2 -mpclmul.

The AMD64 PCLMUL accelerated crc32c_3way() will be moved to a new
file crc32c_amd64.cc. In this way, only a few functions that depend
on -msse4.2 in mysys/crc32/crc32c.cc can be declared with
__attribute__((target("sse4.2"))), and most of the file can be compiled
for the generic target.

Last, the file mysys/crc32ieee.cc will be omitted on 64-bit POWER,
because it was dead code (no symbols were exported).

Reviewed by: Vladislav Vaintroub
parent 18a82901
......@@ -16,7 +16,7 @@
INCLUDE_DIRECTORIES(${ZLIB_INCLUDE_DIR} ${CMAKE_SOURCE_DIR}/include ${CMAKE_SOURCE_DIR}/mysys)
SET(MYSYS_SOURCES array.c charset-def.c charset.c crc32ieee.cc my_default.c
SET(MYSYS_SOURCES array.c charset-def.c charset.c my_default.c
get_password.c
errors.c hash.c list.c
mf_cache.c mf_dirname.c mf_fn_ext.c
......@@ -60,19 +60,29 @@ ENDIF()
IF(MSVC)
SET(MYSYS_SOURCES ${MYSYS_SOURCES} crc32/crc32_x86.c)
IF(CMAKE_SIZEOF_VOID_P EQUAL 8)
SET (MYSYS_SOURCES ${MYSYS_SOURCES} crc32/crc32c_amd64.cc)
ENDIF()
ADD_DEFINITIONS(-DHAVE_SSE42 -DHAVE_PCLMUL)
IF(CLANG_CL)
SET_SOURCE_FILES_PROPERTIES(crc32/crc32_x86.cc crc32/crc32c.c PROPERTIES COMPILE_FLAGS "-msse4.2 -mpclmul")
SET_SOURCE_FILES_PROPERTIES(crc32/crc32_x86.c PROPERTIES COMPILE_FLAGS "-msse4.2 -mpclmul")
ENDIF()
ELSEIF(CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|amd64|i386|i686")
MY_CHECK_C_COMPILER_FLAG(-msse4.2)
MY_CHECK_C_COMPILER_FLAG(-mpclmul)
MY_CHECK_CXX_COMPILER_FLAG(-msse4.2)
MY_CHECK_CXX_COMPILER_FLAG(-mpclmul)
CHECK_INCLUDE_FILE(cpuid.h HAVE_CPUID_H)
CHECK_INCLUDE_FILE(x86intrin.h HAVE_X86INTRIN_H)
IF(have_C__msse4.2 AND have_C__mpclmul AND HAVE_CPUID_H AND HAVE_X86INTRIN_H)
SET(MYSYS_SOURCES ${MYSYS_SOURCES} crc32/crc32_x86.c)
SET_SOURCE_FILES_PROPERTIES(crc32/crc32_x86.c crc32/crc32c.cc PROPERTIES COMPILE_FLAGS "-msse4.2 -mpclmul")
ADD_DEFINITIONS(-DHAVE_SSE42 -DHAVE_PCLMUL)
IF(have_CXX__msse4.2 AND HAVE_CPUID_H)
ADD_DEFINITIONS(-DHAVE_SSE42)
IF (have_CXX__mpclmul AND HAVE_X86INTRIN_H)
ADD_DEFINITIONS(-DHAVE_PCLMUL)
SET(MYSYS_SOURCES ${MYSYS_SOURCES} crc32/crc32_x86.c)
SET_SOURCE_FILES_PROPERTIES(crc32/crc32_x86.c PROPERTIES COMPILE_FLAGS "-msse4.2 -mpclmul")
IF(CMAKE_SIZEOF_VOID_P EQUAL 8)
SET(MYSYS_SOURCES ${MYSYS_SOURCES} crc32/crc32c_amd64.cc)
SET_SOURCE_FILES_PROPERTIES(crc32/crc32c_amd64.cc PROPERTIES COMPILE_FLAGS "-msse4.2 -mpclmul")
ENDIF()
ENDIF()
ENDIF()
ELSEIF(CMAKE_SYSTEM_PROCESSOR MATCHES "aarch64|AARCH64")
IF(CMAKE_COMPILER_IS_GNUCC)
......@@ -129,11 +139,15 @@ ELSEIF(CMAKE_SYSTEM_PROCESSOR MATCHES "aarch64|AARCH64")
COMPILE_FLAGS "-march=armv8-a+crc+crypto")
ENDIF()
ENDIF()
ELSEIF(CMAKE_SYSTEM_PROCESSOR MATCHES "ppc64|powerpc64" OR CMAKE_SYSTEM_NAME MATCHES AIX)
ENDIF()
IF(CMAKE_SYSTEM_PROCESSOR MATCHES "ppc64|powerpc64" OR CMAKE_SYSTEM_NAME MATCHES AIX)
SET(MYSYS_SOURCES ${MYSYS_SOURCES} crc32/crc32_ppc64.c crc32/crc32c_ppc.c)
SET_SOURCE_FILES_PROPERTIES(crc32/crc32_ppc64.c crc32/crc32c_ppc.c PROPERTIES
COMPILE_FLAGS "${COMPILE_FLAGS} -maltivec -mvsx -mpower8-vector -mcrypto -mpower8-vector")
ADD_DEFINITIONS(-DHAVE_POWER8 -DHAS_ALTIVEC)
ELSE()
SET (MYSYS_SOURCES ${MYSYS_SOURCES} crc32ieee.cc)
ENDIF()
IF(UNIX)
......
/* Copyright (c) 2020 MariaDB
/* Copyright (c) 2020, 2021, MariaDB
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
......@@ -55,38 +55,14 @@
#include <stdint.h>
#include <stddef.h>
#if defined(__GNUC__)
#ifdef __GNUC__
#include <x86intrin.h>
#include <cpuid.h>
#elif defined(_MSC_VER)
#include <intrin.h>
#else
#error "unknown compiler"
#endif
static int has_sse42_and_pclmul(uint32_t recx)
{
/* 1 << 20 is SSE42, 1 << 1 is PCLMULQDQ */
#define bits_SSE42_AND_PCLMUL (1 << 20 | 1 << 1)
return (recx & bits_SSE42_AND_PCLMUL) == bits_SSE42_AND_PCLMUL;
}
#ifdef __GNUC__
int crc32_pclmul_enabled(void)
{
uint32_t reax= 0, rebx= 0, recx= 0, redx= 0;
__cpuid(1, reax, rebx, recx, redx);
return has_sse42_and_pclmul(recx);
}
#elif defined(_MSC_VER)
int crc32_pclmul_enabled(void)
{
int regs[4];
__cpuid(regs, 1);
return has_sse42_and_pclmul(regs[2]);
}
#endif
/**
* @brief Shifts left 128 bit register by specified number of bytes
*
......
This diff is collapsed.
This diff is collapsed.
/* Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved.
/* Copyright (c) 2020, 2021, MariaDB
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
......@@ -39,25 +39,23 @@ typedef unsigned int (*my_crc32_t)(unsigned int, const void *, size_t);
static my_crc32_t init_crc32()
{
my_crc32_t func= my_crc32_zlib;
#ifdef HAVE_PCLMUL
if (crc32_pclmul_enabled())
func = crc32_pclmul;
return crc32_pclmul;
#elif defined(__GNUC__) && defined(HAVE_ARMV8_CRC)
if (crc32_aarch64_available())
func= crc32_aarch64;
return crc32_aarch64;
#endif
return func;
return my_crc32_zlib;
}
static const my_crc32_t my_checksum_func= init_crc32();
#ifndef __powerpc64__
/* For powerpc, my_checksum is defined elsewhere.*/
extern "C" unsigned int my_checksum(unsigned int crc, const void *data, size_t len)
#ifdef __powerpc64__
# error "my_checksum() is defined in mysys/crc32/crc32_ppc64.c"
#endif
extern "C"
unsigned int my_checksum(unsigned int crc, const void *data, size_t len)
{
return my_checksum_func(crc, data, len);
}
#endif
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment