RFR: 8343698: Linux x86_64 lto build gives a lot of warnings and fails lto-wrapper: fatal error: make returned 2 exit status

[Kim Barrett]

On Wed, 13 Nov 2024 12:17:07 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

> When trying LTO (configure flag --enable-jvm-feature-link-time-opt=yes) on Linux x86_64, gcc 11.3.0, we run into a lot of warnings and finally into this error :
> 
>  .. tons of free and malloc related warnings ...
> 
> 
>     inlined from 'pop_segment' at src/hotspot/share/utilities/stack.inline.hpp:188:9,
>     inlined from 'pop' at src/hotspot/share/utilities/stack.inline.hpp:84:30,
>     inlined from 'pop_overflow' at src/hotspot/share/gc/shared/taskqueue.inline.hpp:231:28,
>     inlined from 'trim_queue_to_threshold' at src/hotspot/share/gc/g1/g1ParScanThreadState.cpp:328:37:
> src/hotspot/share/runtime/os.cpp:666:3: warning: call to 'malloc' declared with attribute warning: use os::malloc [-Wattribute-warning]
> src/hotspot/share/runtime/os.cpp:666:3: warning: call to 'malloc' declared with attribute warning: use os::malloc [-Wattribute-warning]
> In function 'malloc',
>     inlined from 'malloc' at src/hotspot/share/runtime/os.cpp:637:0,
>     inlined from 'AllocateHeap' at src/hotspot/share/memory/allocation.cpp:42:31,
>     inlined from 'new_entry' at src/hotspot/share/nmt/mallocSiteTable.cpp:181:25,
>     inlined from 'lookup_or_add' at src/hotspot/share/nmt/mallocSiteTable.cpp:122:48,
>     inlined from 'allocation_at' at src/hotspot/share/nmt/mallocSiteTable.hpp:151:37,
>     inlined from 'record_malloc' at src/hotspot/share/nmt/mallocTracker.cpp:179:35,
>     inlined from 'record_malloc' at src/hotspot/share/nmt/memTracker.hpp:81:42,
>     inlined from 'realloc' at src/hotspot/share/runtime/os.cpp:746:58,
>     inlined from 'ReallocateHeap' at src/hotspot/share/memory/allocation.cpp:59:32,
>     inlined from 'grow' at src/hotspot/share/utilities/ostream.cpp:377:15,
>     inlined from 'write' at src/hotspot/share/utilities/ostream.cpp:400:11,
>     inlined from 'write' at src/hotspot/share/utilities/ostream.cpp:382:0,
>     inlined from 'put' at src/hotspot/share/utilities/ostream.cpp:212:8,
>     inlined from 'print_ascii_form' at src/hotspot/share/runtime/os.cpp:965:19,
>     inlined from 'print_hex_location' at src/hotspot/share/runtime/os.cpp:1008:21,
>     inlined from 'print_hex_dump' at src/hotspot/share/runtime/os.cpp:1050:23,
>     inlined from 'print_hex_dump' at src/hotspot/share/runtime/os.hpp:869:19,
>     inlined from 'print_block_on_error' at src/hotspot/share/nmt/mallocHeader.cpp:64:23,
>     inlined from 'resolve_checked_impl' at src/hotspot/share/nmt/mallocHeader.inline.hpp:106:41,
>     inlined from 'resolve_checked' at src/hotspot/share/nmt/mallocHeader.inline.hpp:113:66,
>     inlined f...

I think the approach being taken by the proposed changes is not the right way to
address the issue.

make/hotspot/lib/JvmFeatures.gmk line 173:

> 171:   JVM_OPTIMIZATION := HIGHEST_JVM
> 172:   ifeq ($(call isCompiler, gcc), true)
> 173:     JVM_CFLAGS_FEATURES += -DINCLUDE_LTO=1 -flto=auto -fuse-linker-plugin -fno-strict-aliasing \

I'm not sure why this was included in this PR (and similarly below), since
there's no use of it anywhere. However, it would be useful for
conditionalizing the definition of FORBID_C_FUNCTION, as suggested in JBS. But
just adding the defines here is not sufficient. In utilities/macros.hpp, there
should be a conditional #define to the default value (0) if the macro is not
already defined, so that we can consistently use `#if INCLUDE_LTO` and
`#if !INCLUDE_LTO`.

make/hotspot/lib/JvmOverrideFiles.gmk line 43:

> 41:   ifeq ($(call check-jvm-feature, link-time-opt), true)
> 42:     BUILD_LIBJVM_g1ParScanThreadState.cpp_CXXFLAGS := -fno-lto
> 43:   endif

Even with this change I still get ~200 warnings when building with gcc13.2,
mostly -Wattribute-warning. This is consistent with what I reported previously
with ATTRIBUTE_WARNING as a nop. So this change isn't sufficient.

I'm also not sure it's desirable. We don't know the impact of LTO on top of
flattening. It might be beneficial. This change ought to come with some
performance measurements, if made at all. And not under the rubric of a
warnings fix.

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

Changes requested by kbarrett (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22069#pullrequestreview-2435209120
PR Review Comment: https://git.openjdk.org/jdk/pull/22069#discussion_r1841672314
PR Review Comment: https://git.openjdk.org/jdk/pull/22069#discussion_r1841670496