Skip to content

8355077: Compiler error at splashscreen_gif.c due to unterminated string initialization#24770

Closed
YaSuenag wants to merge 1 commit intoopenjdk:masterfrom
YaSuenag:JDK-8355077
Closed

8355077: Compiler error at splashscreen_gif.c due to unterminated string initialization#24770
YaSuenag wants to merge 1 commit intoopenjdk:masterfrom
YaSuenag:JDK-8355077

Conversation

@YaSuenag
Copy link
Member

@YaSuenag YaSuenag commented Apr 20, 2025

I tried to build OpenJDK with GCC 15.0.1 on Fedora 42 x86_64, however I saw following error.

* For target support_native_java.desktop_libsplashscreen_splashscreen_gif.o:
/home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c:51:41: error: initializer-string for array of ‘char’ truncates NUL terminator but destination lacks ‘nonstring’ attribute (12 chars into 11 available) [-Werror=unterminated-string-initialization]
   51 | static const char szNetscape20ext[11] = "NETSCAPE2.0";
      | ^~~~~~~~~~~~~
cc1: all warnings being treated as errors

This constant seems to be used to detect Netscape 2.0 extension in GIF image. It should be used to compare with extension block without NUL char, but we should tweak initialization to avoid this error for safety code.


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue

Issue

  • JDK-8355077: Compiler error at splashscreen_gif.c due to unterminated string initialization (Bug - P4)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24770/head:pull/24770
$ git checkout pull/24770

Update a local copy of the PR:
$ git checkout pull/24770
$ git pull https://git.openjdk.org/jdk.git pull/24770/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24770

View PR using the GUI difftool:
$ git pr show -t 24770

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24770.diff

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Apr 20, 2025

👋 Welcome back ysuenaga! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented Apr 20, 2025

@YaSuenag This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8355077: Compiler error at splashscreen_gif.c due to unterminated string initialization

Reviewed-by: prr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 126 new commits pushed to the master branch:

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk
Copy link

openjdk bot commented Apr 20, 2025

@YaSuenag The following label will be automatically applied to this pull request:

  • client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the client client-libs-dev@openjdk.org label Apr 20, 2025
@YaSuenag YaSuenag marked this pull request as ready for review April 20, 2025 05:03
@openjdk openjdk bot added the rfr Pull request is ready for review label Apr 20, 2025
@mlbridge
Copy link

mlbridge bot commented Apr 20, 2025

Webrevs

@prrace
Copy link
Contributor

prrace commented Apr 22, 2025

Isn't gcc wrong to complain ?
I'm looking at what is admittedly an old draft of ANSI C
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
and on p130 it has this where t is the same as the usage in splashscreen.

======
EXAMPLE 8 The declaration

char s[] = "abc", t[3] = "abc";

defines ‘‘plain’’ char array objects s and t whose elements are initialized with character string literals.
This declaration is identical to

  char s[] = { 'a', 'b', 'c', '\0' },
          t[] = { 'a', 'b', 'c' };

=====

@YaSuenag
Copy link
Member Author

I think this behavior is not a bug in GCC.

splashscreen_gif.c would be compiled with -Wextra, then -Wunterminated-string-initialization is enabled by default, then the warning would be reported as I shown.
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wunterminated-string-initialization

As an option, we can set nonstring attribute as following. Similar use case in Linux Kernel has introduced in GCC bugzilla. Is this more suitable in this case?

diff --git a/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c b/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c
index 3654c677493..047f08835ad 100644
--- a/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c
+++ b/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -48,7 +48,7 @@
                                 // restore the area overwritten by the graphic with
                                 // what was there prior to rendering the graphic.

-static const char szNetscape20ext[11] = "NETSCAPE2.0";
+static const char szNetscape20ext[11] __attribute__((nonstring)) = "NETSCAPE2.0";

 #define NSEXT_LOOP      0x01    // Loop Count field code

@prrace
Copy link
Contributor

prrace commented Apr 25, 2025

hmm. yes, it looks like it is enabled by -Wextra
https://gcc.gnu.org/pipermail/gcc-patches/2024-June/656014.html

Ok. let's make the change as the easiest thing to do.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Apr 25, 2025
@YaSuenag
Copy link
Member Author

/integrate

@openjdk
Copy link

openjdk bot commented Apr 27, 2025

Going to push as commit 898d479.
Since your change was applied there have been 130 commits pushed to the master branch:

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Apr 27, 2025
@openjdk openjdk bot closed this Apr 27, 2025
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Apr 27, 2025
@openjdk
Copy link

openjdk bot commented Apr 27, 2025

@YaSuenag Pushed as commit 898d479.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@YaSuenag YaSuenag deleted the JDK-8355077 branch April 27, 2025 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

client client-libs-dev@openjdk.org integrated Pull request has been integrated

Development

Successfully merging this pull request may close these issues.

2 participants