RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Erik Joelsson]

Hello,

It's very probable that we have made several such mistakes with our 
flags, since for the most part, we have a 1-1 mapping of compiler and 
OS. We certainly appreciate correcting this whenever possible. I don't 
have the expertise to verify your claim here, but I will take your word 
for it.

The change looks ok, but there is already a big block of clang specific 
stuff close by. Perhaps move this assignment there?

/Erik


On 2018-06-20 16:03, Martin Buchholz wrote:
> Hi David and build-dev folk,
>
> After way too much build/hotspot hacking, I have a better fix:
>
> clang inlined os::current_stack_pointer into its caller __in the same
> translation unit___ (that could be fixed in a separate change) so of course
> in this case it didn't have to follow the ABI.  Fix is obvious in hindsight:
>
> -address os::current_stack_pointer() {
> +NOINLINE address os::current_stack_pointer() {
>
> While working on this I noticed that the clang stack alignment flags on
> Linux are missing.  They should be moved from a macosx-specific check to a
> clang-specific check.  macosx is not synonymous with clang!
>
> --- a/make/autoconf/flags-cflags.m4
> +++ b/make/autoconf/flags-cflags.m4
> @@ -470,6 +470,14 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_HELPER],
>       # COMMON to gcc and clang
>       TOOLCHAIN_CFLAGS_JVM="-pipe -fno-rtti -fno-exceptions \
>           -fvisibility=hidden -fno-strict-aliasing -fno-omit-frame-pointer"
> +
> +    if test "x$TOOLCHAIN_TYPE" = xclang; then
> +      # In principle the stack alignment below is cpu- and ABI-dependent
> and
> +      # should agree with values of StackAlignmentInBytes in various
> +      # src/hotspot/cpu/*/globalDefinitions_*.hpp files, but this value
> +      # currently works for all platforms.
> +      TOOLCHAIN_CFLAGS_JVM="$TOOLCHAIN_CFLAGS_JVM
> -mno-omit-leaf-frame-pointer -mstack-alignment=16"
> +    fi
>     fi
>
>     if test "x$TOOLCHAIN_TYPE" = xgcc; then
> @@ -601,10 +609,6 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_HELPER],
>       fi
>     fi
>
> -  if test "x$OPENJDK_TARGET_OS" = xmacosx; then
> -    OS_CFLAGS_JVM="$OS_CFLAGS_JVM -mno-omit-leaf-frame-pointer
> -mstack-alignment=16"
> -  fi
> -
>     # Optional POSIX functionality needed by the JVM
>     #
>     # Check if clock_gettime is available and in which library. This
> indicates
>
>
> 8186780: clang-4.0 fastdebug assertion failure in
> os_linux_x86:os::verify_stack_alignment()
> http://cr.openjdk.java.net/~martin/webrevs/jdk/clang-stack-alignment/
> https://bugs.openjdk.java.net/browse/JDK-8186780
>
> On Wed, Jun 20, 2018 at 12:30 AM, David Holmes <david.holmes at oracle.com>
> wrote:
>
>> Hi Martin,
>>
>>
>> On 20/06/2018 3:03 AM, Martin Buchholz wrote:
>>
>>> (There's surely a better fix that involves refactoring os/cpu/compiler
>>> support)
>>>
>>> 8186780: clang-4.0 fastdebug assertion failure in
>>> os_linux_x86:os::verify_stack_alignment()
>>> http://cr.openjdk.java.net/~martin/webrevs/jdk/clang-verify_
>>> stack_alignment/
>>> https://bugs.openjdk.java.net/browse/JDK-8186780
>>>
>> I remain concerned about what it may mean for the stack pointer to not be
>> aligned. I would have thought stack pointer alignment was part of the ABI
>> for a CPU architecture, not something the compiler could choose at will?
>> What about all the other code that uses StackAlignmentInBytes ??
>>
>> That aside your fix excludes the assert when building with clang for linux
>> x86 as intended. And I see that for BSD x86 (where we also use clang) that
>> verify_stack_alignment is empty.
>>
>> Thanks,
>> David
>>