Possible bug in sysdeps/x86_64/cacheinfo.c

Possible bug in sysdeps/x86_64/cacheinfo.c

[Paul Pluzhnikov]

Greetings,

In sysdeps/x86_64/cacheinfo.c intel_check_word(), we have this code:

commit 2a1156010784332cbe4bf033ccedb19f52e56a75
Author: Ulrich Drepper <drepper@gmail.com>
Date:   Sun Mar 20 08:14:30 2011 -0400

    Implement x86 cpuid handling of leaf4 for cache information.

...

  asm volatile ("xchgl %%ebx, %1; cpuid; xchgl %%ebx, %1"
		: "=a" (eax), "=r" (ebx), "=c" (ecx), "=d" (edx)
		: "0" (4), "2" (round));

AFAICT, this clobbers the high 32 bits of %rbx, which causes a crash
if the caller had a live %rbx and non-0 high bits in it.

(I am not sure why this isn't causing a problem with current glibc
builds; possibly %rbx is not live?)

-- 
Paul Pluzhnikov

Re: Possible bug in sysdeps/x86_64/cacheinfo.c

[H.J. Lu]

On Mon, Nov 10, 2014 at 9:50 AM, Paul Pluzhnikov <ppluzhnikov@gmail.com> wrote:
> Greetings,
>
> In sysdeps/x86_64/cacheinfo.c intel_check_word(), we have this code:
>
> commit 2a1156010784332cbe4bf033ccedb19f52e56a75
> Author: Ulrich Drepper <drepper@gmail.com>
> Date:   Sun Mar 20 08:14:30 2011 -0400
>
>     Implement x86 cpuid handling of leaf4 for cache information.
>
> ...
>
>   asm volatile ("xchgl %%ebx, %1; cpuid; xchgl %%ebx, %1"
>                 : "=a" (eax), "=r" (ebx), "=c" (ecx), "=d" (edx)
>                 : "0" (4), "2" (round));
>
> AFAICT, this clobbers the high 32 bits of %rbx, which causes a crash
> if the caller had a live %rbx and non-0 high bits in it.
>
> (I am not sure why this isn't causing a problem with current glibc
> builds; possibly %rbx is not live?)
>

It was

https://sourceware.org/bugzilla/show_bug.cgi?id=17259

and fixed by

commit b4acef1ffe2e1ba6c608f31c1954a8100d3eabb0
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Tue Aug 12 17:02:51 2014 -0700

    Replace cpuid asm statement with __cpuid_count

    The compiler doesn't know that the cpuid asm statement in intel_check_word
    will trash RBX.  We are lucky that it doesn't cause any problems since
    RBX is also used by compiler for other purposes so that RBX is saved and
    restored.  This patch replaces it with __cpuid_count.

      [BZ #17259]
      * sysdeps/x86_64/cacheinfo.c (intel_check_word): Replace cpuid
      asm statement with __cpuid_count.



-- 
H.J.

Re: Possible bug in sysdeps/x86_64/cacheinfo.c

[Paul Pluzhnikov]

On Mon, Nov 10, 2014 at 11:23 AM, H.J. Lu <hjl.tools@gmail.com> wrote:

> https://sourceware.org/bugzilla/show_bug.cgi?id=17259
> and fixed by
>
> commit b4acef1ffe2e1ba6c608f31c1954a8100d3eabb0
> Author: H.J. Lu <hjl.tools@gmail.com>
> Date:   Tue Aug 12 17:02:51 2014 -0700
>
>     Replace cpuid asm statement with __cpuid_count

Thanks.

I did 'git pull' before sending my message, and did not see that commit
there. I do see it after 'git pull --all', and I do see it now.


-- 
Paul Pluzhnikov