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
Igor Kozhukhov

   



  

   

   
    
    
   


  


   



   







    

     
     

      

       
        

         
          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. 

   


  


    


   



  

 


  

 

















 

  

   


    Igor Kozhukhov

   

   



    

   

  

 



 

  


 


  


   

   

    
Igor Kozhukhov

    

   


  


  


  

   

   



  

   

   
    
    
   


  


   



   







    

     
     

      

       
        

         
          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. 

   


  


    


   



  

 


  

 

















 

 

  

   


    Toomas Soome

   

   



    

   

  

 



 

  


 


  


   

   

    
Toomas Soome

    

   


  


  


  

   

   



  

   

   
    
    
   


  


   



   







    

     
     

      

       
        

         
          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 /



  

   

   
    
    
   


  


    


    

     
     

      

       
        

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

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    



     





 

  
  
   
   
    
    
    
   
   Show all issues
  
 




     


     
/ FALLTHROUGH / ?



  

   

   
    
    
   


  


    


   



  

 


  

 














     

    

   

  

  

   
   Loading...