Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix build with GCC 15 #1548

Merged
merged 1 commit into from
Feb 3, 2025
Merged

Fix build with GCC 15 #1548

merged 1 commit into from
Feb 3, 2025

Conversation

ac000
Copy link
Member

@ac000 ac000 commented Jan 27, 2025

    Fix build with GCC 15
    
    The upcoming GCC 15 release introduces a new compiler warning,
    Wunterminated-string-initialization.
    
    This is intended to catch things like
    
      static const u_char  hex[16] = "0123456789ABCDEF";
    
    Where we are creating a character array from a string literal, but the
    specified size is not enough for the terminating NUL byte.
    
    In the above example that is intended as it is used as a lookup table
    and only the individual indices are accessed.
    
    As it happens, Unit uses the above idiom in a few places, triggering
    this warning (which we treat as an error by default).
    
    While I don't like disabling compiler warnings, lets just disable this
    one temporarily, as there is a patch in the works to make the
    "nonstring" variable attribute quell this warning.
    
    We just disable this on GCC as this isn't in Clang and we don't need to
    worry about older compilers as GCC silently ignores unknown -Wno-*
    options.
    
    Link: <https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=44c9403ed1833ae71a59e84f9e37af3182be0df5>
    Link: <https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Common-Variable-Attributes.html>
    Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117178#c21>
    Cc: Alejandro Colomar <alx@kernel.org>
    Reviewed-by: Alejandro Colomar <alx@kernel.org>
    Signed-off-by: Andrew Clayton <a.clayton@nginx.com>

@ac000
Copy link
Member Author

ac000 commented Jan 27, 2025

This supplants #1543

Cc: @alejandro-colomar

@ac000 ac000 marked this pull request as ready for review January 27, 2025 17:57
@ac000 ac000 requested review from hongzhidao and avahahn January 27, 2025 17:57
Copy link
Contributor

@alejandro-colomar alejandro-colomar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Alejandro Colomar <alx@kernel.org>

(But I think hex[16] is precisely one of the places where this is not a false positive, and should be addressed by removing the 16.)

@ac000
Copy link
Member Author

ac000 commented Jan 27, 2025

Reviewed-by: Alejandro Colomar <alx@kernel.org>

(But I think hex[16] is precisely one of the places where this is not a false positive, and should be addressed by removing the 16.)

Hmm, but we don't need the terminating NUL byte... We simply access the individual characters (indices), 0-15... It's one of these lookup tables cases...

I do like specifying the size/length in these cases, if you always follow the rule, [n] for character arrays that don't require the NUL byte and [] for creating character string literals.

@ac000
Copy link
Member Author

ac000 commented Jan 27, 2025

  • Added a link to the commit that added this new warning
  • Added @alejandro-colomar 's Reviewed-By
$ git range-diff 3fc00a72...928cfe68
1:  3fc00a72 ! 1:  928cfe68 Fix build with GCC 15
    @@ Commit message
         worry about older compilers as GCC silently ignores unknown -Wno-*
         options.
     
    +    Link: <https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=44c9403ed1833ae71a59e84f9e37af3182be0df5>
         Link: <https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Common-Variable-Attributes.html>
         Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117178#c21>
         Cc: Alejandro Colomar <alx@kernel.org>
    +    Reviewed-by: Alejandro Colomar <alx@kernel.org>
         Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
     
      ## auto/cc/test ##

@alejandro-colomar
Copy link
Contributor

alejandro-colomar commented Jan 27, 2025

Reviewed-by: Alejandro Colomar <alx@kernel.org>

(But I think hex[16] is precisely one of the places where this is not a false positive, and should be addressed by removing the 16.)

Hmm, but we don't need the terminating NUL byte... We simply access the individual characters (indices), 0-15... It's one of these lookup tables cases...

I do like specifying the size/length in these cases, if you always follow the rule, [n] for character arrays that don't require the NUL byte and [] for creating character string literals.

The thing is that the NUL wouldn't hurt at all, so I would do something slightly different:

[] for when a NUL doesn't hurt. [n] for when the NUL would hurt, and thus we need to actively remove it. This would favor strings, having non-strings only when strictly necessary.

But that's a minor thing. Whatever you prefer should be good. :)

@ac000
Copy link
Member Author

ac000 commented Jan 27, 2025

Reviewed-by: Alejandro Colomar <alx@kernel.org>

(But I think hex[16] is precisely one of the places where this is not a false positive, and should be addressed by removing the 16.)

Hmm, but we don't need the terminating NUL byte... We simply access the individual characters (indices), 0-15... It's one of these lookup tables cases...
I do like specifying the size/length in these cases, if you always follow the rule, [n] for character arrays that don't require the NUL byte and [] for creating character string literals.

The thing is that the NUL wouldn't hurt at all, so I would do something slightly different:

[] for when a NUL doesn't hurt. [n] for when the NUL would hurt, and thus we need to actively remove it. This would favor strings, having non-strings only when strictly necessary.

Heh, I think that's basically what I said (at least it's what I was trying to say...)

@alejandro-colomar
Copy link
Contributor

alejandro-colomar commented Jan 27, 2025 via email

The upcoming GCC 15 release introduces a new compiler warning,
Wunterminated-string-initialization.

This is intended to catch things like

  static const u_char  hex[16] = "0123456789ABCDEF";

Where we are creating a character array from a string literal, but the
specified size is not enough for the terminating NUL byte.

In the above example that is intended as it is used as a lookup table
and only the individual indices are accessed.

As it happens, Unit uses the above idiom in a few places, triggering
this warning (which we treat as an error by default).

While I don't like disabling compiler warnings, lets just disable this
one temporarily, as there is a patch in the works to make the
"nonstring" variable attribute quell this warning.

We just disable this on GCC as this isn't in Clang and we don't need to
worry about older compilers as GCC silently ignores unknown -Wno-*
options.

Link: <https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=44c9403ed1833ae71a59e84f9e37af3182be0df5>
Link: <https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Common-Variable-Attributes.html>
Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117178#c21>
Cc: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
@ac000
Copy link
Member Author

ac000 commented Feb 3, 2025

Rebased with master

$ git range-diff 928cfe68...15037822
-:  -------- > 1:  7b7b29fc ruby: Fix build failures with Ruby 3.4
1:  928cfe68 = 2:  15037822 Fix build with GCC 15

@ac000 ac000 merged commit 1503782 into nginx:master Feb 3, 2025
23 checks passed
@ac000 ac000 deleted the no-wusi branch February 3, 2025 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants