JDK-8213153 : Clean up GCC 8 errors
  • Type: Task
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 12
  • Priority: P4
  • Status: Closed
  • Resolution: Delivered
  • Submitted: 2018-10-30
  • Updated: 2020-05-01
  • Resolved: 2019-03-02
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 13
13Resolved
Related Reports
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Description
GCC 8 reports errors like following:

== Excess strncat limits ==

src/hotspot/share/adlc/dfa.cpp:724:14: error: 'char* strncat(char*, const char*, size_t)' specified bound 2048 equals destination size [-Werror=stringop-overflow=]
       strncat(string_buffer, "+", STRING_BUFFER_LENGTH);

Buffers are declared like:
char  Expr::string_buffer[STRING_BUFFER_LENGTH];

So all such usages should use "STRING_BUFFER_LENGTH - 1".

Also:

src/hotspot/share/classfile/classLoader.cpp:1233:53: error: argument to ���sizeof��� in ���char* strncpy(char*, const char*, size_t)��� call is the same expression as the source; did you mean to use the size of the destination? [-Werror=sizeof-pointer-memaccess]
   strncpy(&file_name[class_name_len], class_suffix, sizeof(class_suffix));

class_suffix already includes NULL and strncpy adds one more, so it looks like there also should be " - 1".

== Class arrays zeroing ===

In multiple places there are errors reported like:

src/hotspot/share/services/memoryManager.cpp:172:40: error: ���void* memset(void*, int, size_t)��� clearing an object of non-trivial type ���class MemoryUsage���; use assignment or value-initialization instead [-Werror=class-memaccess]
   memset(_before_gc_usage_array, 0, len);
                                        ^

Can be fixed by casting to void*.
Comments
Reproduced with 8.3 built from source on Ubuntu. Most common upstream is 8.2 so I created separate JDK-8220070.
04-03-2019

I still see some warnings with GCC 8.3.1: - java.desktop/liblcms/cmstypes.c - java.desktop/liblcms/cmscgats.c - java.base/libjava/canonicalize_md.c I use gcc-8.3.1-2.fc29.x86_64 and gcc-c++-8.3.1-2.fc29.x86_64 on Fedora 29. We can fix warning in canonicalize_md.c, but others seem to be in LCMS. Should we change LCMS code? or should we avoid them with compiler option(s)?
04-03-2019

"Fixed" is reserved for issues that have changesets recorded against them. This issue is better classified as a task than a bug.
02-03-2019

OpenJDK is now buildable with GCC 8.x
02-03-2019

[~dchuyko] I believe all the issues have now been fixed?
02-03-2019

Some more adlc issues are tracked in JDK-8214059
20-11-2018

The JDWP issue is being tracked with JDK-8214061
20-11-2018

Agree, I'll create subtasks for different parts.
01-11-2018

BTW, I prefer individual bugs for specific issues, rather than omnibus bugs like this. Fixes for individual bugs can be created, reviewed, and pushed, making easy incremental progress. With an omibus bug like this, it's harder to tell wether an offered patch covers everything, and harder to review all the different pieces.
01-11-2018

Some extras: == Functional type casts == A couple things like: src/java.base/unix/native/libjli/java_md_solinux.c: In function ���ContinueInNewThread0���: /home/tp/jdk12/src/java.base/unix/native/libjli/java_md_solinux.c:747:37: error: cast between incompatible function types from ���int (*)(void *)��� to ���void * (*)(void *)��� [-Werror=cast-function-type] if (pthread_create(&tid, &attr, (void *(*)(void*))continuation, (void*)args) == 0) { May be silenced by extra intermediate (void*) cast since we know what happens. == Restrict parameters == src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c: In function ���jniFatalError���: src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c:650:24: error: passing argument 1 to restrict-qualified parameter aliases with argument 4 [-Werror=restrict] (void)snprintf(buf, sizeof(buf), "JDWP %s", buf); ^~~ ~~~ src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c: In function ���jniFatalError.constprop���: src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c:650:48: error: ���%s��� directive output may be truncated writing up to 511 bytes into a region of size 507 [-Werror=format-truncation=] (void)snprintf(buf, sizeof(buf), "JDWP %s", buf); ^~ ~~~ This line should instead look like (void)snprintf(buf, sizeof(buf), "JDWP %s", msg); == Output truncation == src/jdk.jdwp.agent/share/native/libjdwp/log_messages.c:75:24: error: ���%.3d��� directive output may be truncated writing between 3 and 11 bytes into a region of size between 0 and 80 [-Werror=format-truncation=] "%s.%.3d %s", timestamp_prefix, ^~~~ Can be fixed by making prefix and postfix reasonably short, like: char timestamp_prefix[MAXLEN_TIMESTAMP / 2 - 4]; char timestamp_postfix[MAXLEN_TIMESTAMP / 2]; == Harfbuzz == At least there are template attributes and member access errors. E.g.: src/java.desktop/share/native/libfontmanager/harfbuzz/hb-debug.hh:130:1: error: explicit specialization ���void _hb_debug_msg_va(const char*, const void*, const char*, bool, unsigned int, int, const char*, __va_list_tag*) [with int max_level = 0]��� may be missing attributes [-Werror=missing-attributes] _hb_debug_msg_va<0> (const char *what HB_UNUSED, ^~~~~~~~~~~~~~~~~~~ src/java.desktop/share/native/libfontmanager/harfbuzz/hb-debug.hh:81:1: note: missing primary template attribute ���format��� Things like this may be fixed by adding macros like HB_PRINTF_FUNC(7, 8) to template specializations. Visibility also may be fixed. But I'm not sure if it makes sense to make fixes in adopted library code. Maybe it makes more sense to suppress errors there or to adopt latest library version.
31-10-2018

Kim, I agree with all above.
31-10-2018

Regarding the classLoader.cpp use of strncpy, the code is correct, but perhaps confusing. Certainly it confused the compiler's checking. This is actually a case where strcpy could be used, since the buffer size is known to be sufficient. The analysis in the description above is wrong; the compiler is complaining that the size appears to be based on the wrong argument, which could be a bug, but happens to not be in this case. Changing both strncpy's to strcpy's would be correct, but might trigger warnings about "unsafe" strcpy somewhere. Computing "file_name_len = class_name_len + sizeof(class_suffix)" and using that as the allocation size, and changing the offending size argument to "file_name_len - class_name_len" (which is == sizeof(class_suffix)) would probably also silence the warning.
31-10-2018

Regarding the memoryManager.cpp use of memset, cleaner would be "::new (_before_gc_usage_array) MemoryUsage()".
30-10-2018

Regarding the dfa.cpp uses of strncat, whoever wrote that code seems to have not carefully read the documentation for strncat. Neither STRING_BUFFER_LENGTH nor the proposed STRING_BUFFER_LENGTH-1 are correct values for the size argument. The size is the number of characters from src to be copied, and is not directly based on the size of the destination (other than if too big then one can get buffer overruns). The proposed change in the description here is incorrect.
30-10-2018