10413 SPARC needs some gcc7 fixes

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

Information
ptribble
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).
  • 4
  • 0
  • 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 ... igork igork
Does this comment silence gcc? I have memory that the default comment in this case should be / FALLTHROUGH / tsoome tsoome
it is incorrect. variable is 'long' but you try to use ULONG_MAX - it has double size of variable max igork igork
/ FALLTHROUGH / ? tsoome tsoome
igork
igork 
  1. 
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/uts/sun4/os/trap.c
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    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. 



    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. 



    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.
      
 
      

      4. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   




igork




   

   







  

 



 

  


   

   

    
 
igork


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/uts/sun4/vm/vm_dep.h
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    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).
      
 
      

      2. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   




tsoome




   

   







  

 



 

  


   

   

    
 
tsoome


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/uts/sun4/os/trap.c
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



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

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/uts/sun4/vm/vm_dep.h
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    / FALLTHROUGH / ?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  4. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  

 









     

    

   

  

  

   
   Loading...