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.hol...@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


Reply via email to