10413 SPARC needs some gcc7 fixes

Review Request #1483 - Created Feb. 17, 2019 and updated

Information
Peter Tribble
illumos-gate
10413
Reviewers
general

See ticket for details.

The one fix I'm unsure of is the sizeof() in prmachdep.c

I've done a gcc4-only build with these changes, and that's clean. Attempting a build with these issues fixed and a gcc7 shadow shows that these errors are gone (but there's a new set to investigate).

Issues

  • 4
  • 0
  • 0
  • 4
Description From Last Updated
i'm not sure it is correct, in some cases you should to check others definitions, but you are ignore it ... Igor Kozhukhov Igor Kozhukhov
it is incorrect. variable is 'long' but you try to use ULONG_MAX - it has double size of variable max Igor Kozhukhov Igor Kozhukhov
Does this comment silence gcc? I have memory that the default comment in this case should be / FALLTHROUGH / Toomas Soome Toomas Soome
/ FALLTHROUGH / ? Toomas Soome Toomas Soome
Igor Kozhukhov

   
usr/src/uts/sun4/os/trap.c (Diff revision 1)
 
 

i'm not sure it is correct, in some cases you should to check others definitions, but you are ignore it with 'break' will be more correct to add "FALLTHROUGH" or try to find correct word from tsoome updates

  1. A better approach here (and one that tsoome has been doing for several of his gcc7 fixes) is to mark the die() function as _NORETURN.
    An example from https://github.com/illumos/illumos-gate/blob/master/usr/src/cmd/cmd-inet/usr.sbin/ipadm/ipadm.c#L342

    static void die(const char *, ...) __NORETURN;
    

    and leave the NOTREACHED comments in place.

  2. Even there, looking at that file, about half the die_opterr() have a break.
    
    I was actually using the fix for 10194 as a template
    
    https://github.com/illumos/illumos-gate/commit/7c9ce9e029489432cc8d45437d8ecd0b9750d68d#diff-0db36ed894a3ee54b0f6705632e14654
  3. I would have used noreturn there too, but. Even if we see that this implementation of die() does indeed not return - both legs of if statement end up with panic(), it is unclear how the gcc wil handle the optimization at the end of it and especially combined with noreturn. It could be that optimization was just for studio cc and we could in fact just nuke it and still go with noreturn.

Igor Kozhukhov

   
usr/src/uts/sun4/vm/vm_dep.h (Diff revision 1)
 
 

it is incorrect.
variable is 'long' but you try to use ULONG_MAX - it has double size of variable max

  1. ULONG_MAX is value for unsigned long, that is, the type with the same storage size as long. The reason people often use (-1) in such context is that -1 is all bits set, ULONG_MAX with this data type does give us the same effect.

    For example, if we start with (-1) == 0xffffffffffffffff and we left shift it 10 bits, we will have value: -1024 (0xfffffffffffffc00).

Toomas Soome

   
usr/src/uts/sun4/os/trap.c (Diff revision 1)
 
 

Does this comment silence gcc? I have memory that the default comment in this case should be / FALLTHROUGH /

usr/src/uts/sun4/vm/vm_dep.h (Diff revision 1)
 
 

/ FALLTHROUGH / ?

Loading...