JDK-8235767 : Compilation failure caused by JDK-8212780: Packaging Tool Implementation
  • Type: Bug
  • Component: tools
  • Affected Version: 14
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2019-12-11
  • Updated: 2019-12-19
  • Resolved: 2019-12-11
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 14 JDK 15
14 b27Fixed 15Fixed
Related Reports
Relates :  
Description
JDK-8212780 causes a compilation failure in our CI. It's using gcc 4.8.7 on RHEL 7.8.

/home/jenkins/workspace/nightly/jdk-jdk/src/jdk.incubator.jpackage/share/native/libapplauncher/IniFile.cpp: In member function 'bool IniFile::SaveToFile(TString, bool)':
/home/jenkins/workspace/nightly/jdk-jdk/src/jdk.incubator.jpackage/share/native/libapplauncher/IniFile.cpp:116:58: error: 'section' may be used uninitialized in this function [-Werror=maybe-uninitialized]
             std::list<TString> lines = section->GetLines();
                                                          ^
cc1plus: all warnings being treated as errors
gmake[3]: *** [/home/jenkins/workspace/nightly/jdk-jdk/build/linux-x86_64-server-fastdebug/support/native/jdk.incubator.jpackage/libapplauncher/IniFile.o] Error 1
gmake[3]: *** Waiting for unfinished jobs....
gmake[2]: *** [jdk.incubator.jpackage-libs] Error 1
gmake[2]: *** Waiting for unfinished jobs....

ERROR: Build failed for target 'images' in configuration 'linux-x86_64-server-fastdebug' (exit code 2) 
Stopping sjavac server

=== Output from failing command(s) repeated here ===
* For target support_native_jdk.incubator.jpackage_libapplauncher_IniFile.o:
/home/jenkins/workspace/nightly/jdk-jdk/src/jdk.incubator.jpackage/share/native/libapplauncher/IniFile.cpp: In member function 'bool IniFile::SaveToFile(TString, bool)':
/home/jenkins/workspace/nightly/jdk-jdk/src/jdk.incubator.jpackage/share/native/libapplauncher/IniFile.cpp:116:58: error: 'section' may be used uninitialized in this function [-Werror=maybe-uninitialized]
             std::list<TString> lines = section->GetLines();
                                                          ^
cc1plus: all warnings being treated as errors

* All command lines available in /home/jenkins/workspace/nightly/jdk-jdk/build/linux-x86_64-server-fastdebug/make-support/failure-logs.
=== End of repeated output ===

No indication of failed target found.
Hint: Try searching the build log for '] Error'.
Hint: See doc/building.html#troubleshooting for assistance.

make[1]: *** [main] Error 1
make: *** [images] Error 2
Comments
URL: https://hg.openjdk.java.net/jdk/jdk/rev/29ca931d8f86 User: herrick Date: 2019-12-11 16:53:11 +0000
11-12-2019

I confirm that the revised patch builds, and also looks reasonable to me.
11-12-2019

I agree, it seems odd that the compiler complains about line 112, but not the other lines. I suspect that in the other cases, we are passing a const known name into GetValue(), and in that function, section is only initialized on certain values, and the compiler can infer that there, but not in line 112, where we pass a non-const unknown (to the compiler) name. Certainly, the patch above fixes the build. It seems good practice to do the same fix in all other instances too, and also have NULL checks in all instances, because we dereference section right after GetValue().
11-12-2019

I can only assume from what you are saying is that the compilers I use automatically initialize pointers to NULL, and yours doesn't. Aren't there then many other instances of this in our native code. my revised webrev would be: http://cr.openjdk.java.net/~herrick/8235767/webrev.02/
11-12-2019

I saw that construct everywhere else (" if (FMap.GetValue(name, section) == true && section != NULL) {" instead of just " if (FMap.GetValue(name, section) == true) {" so thought it should be here too. but everywhere else there is the construction "IniSectionData *section;" without "IniSectionData *section= NULL". won't fixing line 112 just push the same problem to line 131 ? so don't we need to fix all 6 places this exists: - IniSectionData *section;
11-12-2019

This doesn't seem to help. What *does* seem to help is the below. Note this is not about dereferencing NULL, it is about section being uninitialized. diff -r 6afc12975478 src/jdk.incubator.jpackage/share/native/libapplauncher/IniFile.cpp --- a/src/jdk.incubator.jpackage/share/native/libapplauncher/IniFile.cpp Wed Dec 11 13:08:45 2019 +0100 +++ b/src/jdk.incubator.jpackage/share/native/libapplauncher/IniFile.cpp Wed Dec 11 09:51:09 2019 -0500 @@ -109,7 +109,7 @@ for (unsigned int index = 0; index < keys.size(); index++) { TString name = keys[index]; - IniSectionData *section; + IniSectionData *section = NULL; if (FMap.GetValue(name, section) == true) { contents.push_back(_T("[") + name + _T("]"));
11-12-2019

simple fix: http://cr.openjdk.java.net/~herrick/8235767/webrev.01/
11-12-2019