bug#64058: [PATCH] wc: Fix crashes due to incomplete AVX2 enumeration

[Axel Beckert]

Control: tag 1037264 + patch

Hi Pádraig,

On Wed, Jun 14, 2023 at 11:46:58AM +0100, Pádraig Brady wrote:
> On 14/06/2023 05:14, Paul Eggert wrote:
> > Thanks for the bug report. I installed the attached patch into coreutils
> > on Savannah. It builds on your idea with several other changes:
> > 
> > * There's a similar issue with cksum.c and pclmul.
> > 
> > * configure.ac can be simplified, since it seems there's no point
> > compiling these instructions if __builtin_cpu_supports doesn't work.
> > 
> > * This lets us simplify the source code a bit more.
> > 
> > Please let me know if the attached patch works for you.
> 
> __builtin_cpu_supports() looks to have sufficient support in
> Arch + compiler versions for our needs, so that's good.
> 
> Paul you removed the "avx" check from cksum.c. Was that intended?
> 
> > PS. Does the attached cksum.c / pclmul change fix any user-visible
> > misbehavior? If so, what should we put into the NEWS file?
> 
> We have an illegal instruction issue with cksum under Xen DomU
> which may be related, as discussed at: https://bugs.debian.org/1037264
> 
> Axel does the attached patch change anything for you?

Yes! This patch helps! Thanks a lot! :-)

Cc'ing the Debian bug report again (and doing a nearly fullquote for
that) and tagging Debian's bug report as containing a patch. (Given
it's high bug report number it should do any harm in GNU's BTS even if
it uses the same control syntax…)

> diff --git a/src/cksum.c b/src/cksum.c
> index 85afab0ac..881d90413 100644
> --- a/src/cksum.c
> +++ b/src/cksum.c
> @@ -160,29 +160,16 @@ static bool
>  pclmul_supported (void)
>  {
>  # if USE_PCLMUL_CRC32
> -  unsigned int eax = 0;
> -  unsigned int ebx = 0;
> -  unsigned int ecx = 0;
> -  unsigned int edx = 0;
> -
> -  if (! __get_cpuid (1, &eax, &ebx, &ecx, &edx))
> -    {
> -      if (cksum_debug)
> -        error (0, 0, "%s", _("failed to get cpuid"));
> -      return false;
> -    }
> -
> -  if (! (ecx & bit_PCLMUL) || ! (ecx & bit_AVX))
> -    {
> -      if (cksum_debug)
> -        error (0, 0, "%s", _("pclmul support not detected"));
> -      return false;
> -    }
> +  bool pclmul_enabled = 0 < __builtin_cpu_supports ("pclmul")
> +                        && 0 < __builtin_cpu_supports ("avx");
>  
>    if (cksum_debug)
> -    error (0, 0, "%s", _("using pclmul hardware support"));
> +    error (0, 0, "%s",
> +           (pclmul_enabled
> +            ? _("using pclmul hardware support")
> +            : _("pclmul support not detected")));
>  
> -  return true;
> +  return pclmul_enabled;
>  # else
>    if (cksum_debug)
>      error (0, 0, "%s", _("using generic hardware support"));

                Regards, Axel
-- 
 ,''`.  |  Axel Beckert <abe@debian.org>, https://people.debian.org/~abe/
: :' :  |  Debian Developer, ftp.ch.debian.org Admin
`. `'   |  4096R: 2517 B724 C5F6 CA99 5329  6E61 2FF9 CD59 6126 16B5
  `-    |  1024D: F067 EA27 26B9 C3FC 1486  202E C09E 1D89 9593 0EDE