On Wed, 11 May 2022 08:40:21 GMT, Yasumasa Suenaga <ysuen...@openjdk.org> wrote:

>> I saw some compiler warnings when I tried to build OpenJDK with GCC 12.0.1 
>> on Fedora 36.
>> As you can see, the warnings spreads several areas. Let me know if I should 
>> separate them by area.
>> 
>> * -Wstringop-overflow
>>     * src/hotspot/share/oops/array.hpp
>>     * 
>> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
>> 
>> In member function 'void Array<T>::at_put(int, const T&) [with T = unsigned 
>> char]',
>>     inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at 
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64,
>>     inlined from 'void ConstantPool::method_at_put(int, int, int)' at 
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:380:15,
>>     inlined from 'ConstantPool* 
>> BytecodeConstantPool::create_constant_pool(JavaThread*) const' at 
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/bytecodeAssembler.cpp:85:26:
>
> Yasumasa Suenaga has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Avoid pragma error in before GCC 12

Changes requested by kbarrett (Reviewer).

make/modules/java.desktop/lib/Awt2dLibraries.gmk line 462:

> 460:    HARFBUZZ_DISABLED_WARNINGS_gcc := type-limits 
> missing-field-initializers strict-aliasing
> 461:    HARFBUZZ_DISABLED_WARNINGS_CXX_gcc := reorder delete-non-virtual-dtor 
> strict-overflow \
> 462:         maybe-uninitialized class-memaccess unused-result extra 
> use-after-free

Globally disabling use-after-free warnings for this package seems really
questionable. If these are problems in the code, just suppressing the warning
and leaving the problems to bite us seems like a bad idea.  And the problems
ought to be reported upstream to the HarfBuzz folks.

src/hotspot/share/utilities/compilerWarnings_gcc.hpp line 51:

> 49: 
> 50: #define PRAGMA_STRINGOP_OVERFLOW_IGNORED \
> 51:   PRAGMA_DISABLE_GCC_WARNING("-Wstringop-overflow")

Are the reported problems with format/stringop-overflow real? Or are they
false positives? If they are real then I'm not going to approve ignoring them,
unless there is some urgent reason and there are followup bug reports for
fixing them.

src/java.base/share/native/libjli/java.c line 1629:

> 1627:         const char *arg = jargv[i];
> 1628:         if (arg[0] == '-' && arg[1] == 'J') {
> 1629:             *nargv++ = (arg[2] == '\0') ? NULL : JLI_StringDup(arg + 2);

Wow!

src/java.base/unix/native/libjli/java_md_common.c line 135:

> 133:     if ((JLI_StrLen(indir) + JLI_StrLen(cmd) + 2) > sizeof(name)) return 
> 0;
> 134:     JLI_Snprintf(name, sizeof(name), "%s%c%s", indir, FILE_SEPARATOR, 
> cmd);
> 135: #pragma GCC diagnostic pop

Wouldn't it be better to just call JLI_Snprintf without the precheck and check 
the result to determine whether it was successful or was truncated?  I think 
that kind of check is supposed to make gcc's truncation checker happy.

src/jdk.jpackage/linux/native/applauncher/LinuxPackage.c line 193:

> 191: #if defined(__GNUC__) && __GNUC__ >= 12
> 192: #pragma GCC diagnostic pop
> 193: #endif

Rather than all this warning suppression cruft and the comment explaining why
it's okay, just compute the `(strBufNextChar - strBufBegin)` offset a few
lines earlier, before the realloc.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8646

Reply via email to