Bug 68491 - libgcc calls __get_cpuid with 0 level breaks on early 486
Summary: libgcc calls __get_cpuid with 0 level breaks on early 486
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.8.4
: P3 normal
Target Milestone: 5.5
Assignee: Uroš Bizjak
URL:
Keywords:
: 81218 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-11-23 01:36 UTC by Christos Zoulas
Modified: 2017-06-27 17:20 UTC (History)
1 user (show)

See Also:
Host:
Target: x86
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-05-01 00:00:00


Attachments
Amended cpuid patch. (475 bytes, patch)
2017-04-28 18:53 UTC, Christos Zoulas
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christos Zoulas 2015-11-23 01:36:16 UTC
http://nxr.netbsd.org/xref/src/external/gpl3/gcc/dist/libgcc/config/i386/cpuinfo.c#284
calls __get_cpuid with 0 level, then we end up in:
http://nxr.netbsd.org/xref/src/external/gpl3/gcc/dist/gcc/config/i386/cpuid.h#263
and end up calling __cpuid even though __get_cpuid_max returns 0. This breaks on early i486 processors that don't have the cpuid instruction and we end up getting SIGILL. Our fix is to explicitly check for 0 to prevent that:

             unsigned int *__ecx, unsigned int *__edx)
 {
   unsigned int __ext = __level & 0x80000000;
+  unsigned int __maxlevel = __get_cpuid_max (__ext, 0);
 
-  if (__get_cpuid_max (__ext, 0) < __level)
+  if (__maxlevel == 0 || __maxlevel < __level)
     return 0;
 
   __cpuid (__level, *__eax, *__ebx, *__ecx, *__edx);
Comment 1 Andrew Pinski 2016-01-23 21:47:43 UTC
Patches should goto gcc-patches@ and should be based off the trunk of GCC.
Comment 2 Christos Zoulas 2017-04-28 18:53:45 UTC
Created attachment 41284 [details]
Amended cpuid patch.

Here's an amended patch against the trunk. Also sent mail to gcc-patches@
Comment 3 uros 2017-05-01 15:38:46 UTC
Author: uros
Date: Mon May  1 15:38:14 2017
New Revision: 247439

URL: https://gcc.gnu.org/viewcvs?rev=247439&root=gcc&view=rev
Log:
	PR target/68491
	* config/i386/cpuid.h (__get_cpuid): Always return 0 when
	__get_cpuid_max returns 0.
	(__get_cpuid_count): Ditto.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/cpuid.h
Comment 4 uros 2017-05-02 20:36:58 UTC
Author: uros
Date: Tue May  2 20:36:26 2017
New Revision: 247523

URL: https://gcc.gnu.org/viewcvs?rev=247523&root=gcc&view=rev
Log:
	Backport from mainline
	2017-05-01  Uros Bizjak  <ubizjak@gmail.com>

	PR target/68491
	* config/i386/cpuid.h (__get_cpuid): Always return 0 when
	__get_cpuid_max returns 0.
	(__get_cpuid_count): Ditto.


Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/config/i386/cpuid.h
Comment 5 uros 2017-05-03 18:19:59 UTC
Author: uros
Date: Wed May  3 18:19:28 2017
New Revision: 247561

URL: https://gcc.gnu.org/viewcvs?rev=247561&root=gcc&view=rev
Log:
	Backport from mainline
	2017-05-01  Uros Bizjak  <ubizjak@gmail.com>

	PR target/68491
	* config/i386/cpuid.h (__get_cpuid): Always return 0 when
	__get_cpuid_max returns 0.
	(__get_cpuid_count): Ditto.


Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/config/i386/cpuid.h
Comment 6 uros 2017-05-03 20:01:22 UTC
Author: uros
Date: Wed May  3 20:00:50 2017
New Revision: 247566

URL: https://gcc.gnu.org/viewcvs?rev=247566&root=gcc&view=rev
Log:
	Backport from mainline
	2017-05-01  Uros Bizjak  <ubizjak@gmail.com>

	PR target/68491
	* config/i386/cpuid.h (__get_cpuid): Always return 0 when
	__get_cpuid_max returns 0.


Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/config/i386/cpuid.h
Comment 7 Uroš Bizjak 2017-05-03 20:01:57 UTC
Fixed everywhere.
Comment 8 Uroš Bizjak 2017-06-26 20:28:00 UTC
*** Bug 81218 has been marked as a duplicate of this bug. ***
Comment 9 Andy Lutomirski 2017-06-27 16:26:53 UTC
I'm a bit late to the party, but this patch seems dubious to me.  __get_cpuid_max() fails to distinguish between CPUs that have max level 0 (although I doubt that any such CPUs exist) and CPUs that don't have CPUID at all.

Shouldn't __get_cpuid_max return -1 if CPUID isn't there or, alternatively, just generally return the last level + 1 (and 0 if CPUID isn't there)?
Comment 10 Christos Zoulas 2017-06-27 16:56:24 UTC
On Jun 27,  4:26pm, gcc-bugzilla@gcc.gnu.org ("luto at kernel dot org") wrote:
-- Subject: [Bug target/68491] libgcc calls __get_cpuid with 0 level breaks o

| I'm a bit late to the party, but this patch seems dubious to me.=20
| __get_cpuid_max() fails to distinguish between CPUs that have max level 0
| (although I doubt that any such CPUs exist) and CPUs that don't have CPUID =
| at
| all.
| 
| Shouldn't __get_cpuid_max return -1 if CPUID isn't there or, alternatively,=
| 
| just generally return the last level + 1 (and 0 if CPUID isn't there)?
| 

Yes, it should. That would be cleaner, but my goal was minimal change
not refactoring :-)

christos
Comment 11 Uroš Bizjak 2017-06-27 17:20:41 UTC
(In reply to Andy Lutomirski from comment #9)
> I'm a bit late to the party, but this patch seems dubious to me. 
> __get_cpuid_max() fails to distinguish between CPUs that have max level 0
> (although I doubt that any such CPUs exist) and CPUs that don't have CPUID
> at all.

No, cpuid leaf 0 will never return zero [1].

[1] https://en.wikipedia.org/wiki/CPUID#EAX.3D0:_Highest_Function_Parameter