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

[Martin Buchholz]

Since the question of how to get the stack pointer in hotspot is
unexpectedly difficult, I propose we split into two separate changes and we
can submit the make/ changes that Erik is happy with.

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

[Martin Buchholz]

I see stack pointers (and frame pointers) declared as intptr_t* which makes
no sense to me.  These are all "just" pointers, so declare them as either
void* or intptr_t or address

In the construct below, we could elide the cast if we had declared esp
right in the first place?!

  intptr_t* esp;
  __asm__ __volatile__ ("mov %%"SPELL_REG_SP", %0":"=r"(esp):);
  return (address) esp;

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

[Martin Buchholz]

There sure seems to be a lot of confusion about how many stack frames we're
getting at the hot end of the stack, e.g.

  register intptr_t **fp __asm__ (SPELL_REG_FP);

  // fp is for this frame (_get_previous_fp). We want the fp for the
  // caller of os::current_frame*(), so go up two frames. However, for
  // optimized builds, _get_previous_fp() will be inlined, so only go
  // up 1 frame in that case.
  #ifdef _NMT_NOINLINE_
return **(intptr_t***)fp;
  #else
return *fp;
  #endif

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

[Martin Buchholz]

(I keep trying not to become a hotspot engineer...)

I would define current_stack_pointer as a macro using expression
statements: prototype is:

#if defined(__clang__)
#define sp_3() ({ intptr_t rsp; __asm__ __volatile__ ("mov %% rsp,
%0":"=r"(rsp):); rsp; })
#else
#define sp_3() ({ register intptr_t rsp asm ("rsp"); rsp; })
#endif

Then we can call that everywhere and actually get the right answer.
(Other compilers and cpu architectures left as an exercise for the reader)
(Probably we won't be able to do this for every compiler we'd like to use)

Then we can make all the
os::verify_stack_alignment functions in non-product mode NOINLINE so that
they have a real stack frame with a real stack pointer.

But that's starting to be a big change and a hotspot culture change, so can
hotspot engineers please take over ?!

On Fri, Jun 22, 2018 at 8:27 AM, Thomas Stüfe 
wrote:

> On Fri, Jun 22, 2018 at 1:57 PM, David Holmes 
> wrote:
> > On 21/06/2018 10:37 PM, Thomas Stüfe wrote:
> >>
> >> On Thu, Jun 21, 2018 at 1:27 PM, David Holmes 
> >> wrote:
> >>>
> >>> On 21/06/2018 10:05 AM, Martin Buchholz wrote:
> 
> 
>  On Wed, Jun 20, 2018 at 4:03 PM, 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() {
> 
> 
>  If y'all like the addition of NOINLINE, it should probably be added to
>  all
>  of the 14 variants of os::current_stack_pointer.
>  Gives me a chance to try out the submit repo.
> >>>
> >>>
> >>>
> >>> I can't help but think other platforms actually rely on it being
> inlined
> >>> so
> >>> that it really does return the stack pointer of the method calling
> >>> os::current_stack_pointer!
> >>>
> >>
> >> But we only inline today if caller is in the same translation unit and
> >> builds with optimization, no?
> >
> >
> > Don't know, but Martin's encountering a case where it is being inlined -
> is
> > that likely to be unique for some reason?
> >
>
> Okay I may have confused myself.
>
> My original reasoning was: A lot of calls to
> os::current_stack_pointer() today happen not-inlined. That includes
> calls from outside os__.cpp, and calls from inside
> os__.cpp if slowdebug. Hence, since the VM - in particular
> the slowdebug one - seems to work fine, it should be okay to mark
> os::current_stack_pointer() unconditionally as NOINLINE.
>
> However, then I saw that the only "real" function (real meaning not
> just asserting something) using os::current_stack_pointer() and living
> in the same translation unit is os::current_frame(), e.g. on linux:
>
> frame os::current_frame() {
>   intptr_t* fp = _get_previous_fp();
>   frame myframe((intptr_t*)os::current_stack_pointer(),
> (intptr_t*)fp,
> CAST_FROM_FN_PTR(address, os::current_frame));
>   if (os::is_first_C_frame(&myframe)) {
> // stack is not walkable
> return frame();
>   } else {
> return os::get_sender_for_C_frame(&myframe);
>   }
> }
>
> how does this even work if os::current_stack_pointer() is not inlined?
> In slowdebug? Would the frame object in this case not contain the SP
> from the frame built up for os::current_stack_pointer()?
>
> So, now I wonder if making os::current_stack_pointer() NOINLINE would
> break os::current_frame() - make if produce frames with a slightly-off
> SP. os::current_frame() is used e.g. in NMT. So, I wonder if NMT still
> works if os::current_stack_pointer is made NOINLINE, or in slowdebug.
>
> > I never assume the compiler does the obvious these days :) or that there
> > can't be clever link-time tricks as well.
>
> Oh. I did not think of that at all, you are right.
>
> >
> > Maybe the safest thing to do is to only make a change for the clang case
> and
> > leave everything else alone.
> >
>
> It would be better to know for sure, though.
>
> ..Thomas
>
> > David
> > -
> >
> >>
> >> ..Thomas
> >>
> >>> David
>

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

[Thomas Stüfe]

On Fri, Jun 22, 2018 at 1:57 PM, David Holmes  wrote:
> On 21/06/2018 10:37 PM, Thomas Stüfe wrote:
>>
>> On Thu, Jun 21, 2018 at 1:27 PM, David Holmes 
>> wrote:
>>>
>>> On 21/06/2018 10:05 AM, Martin Buchholz wrote:


 On Wed, Jun 20, 2018 at 4:03 PM, 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() {


 If y'all like the addition of NOINLINE, it should probably be added to
 all
 of the 14 variants of os::current_stack_pointer.
 Gives me a chance to try out the submit repo.
>>>
>>>
>>>
>>> I can't help but think other platforms actually rely on it being inlined
>>> so
>>> that it really does return the stack pointer of the method calling
>>> os::current_stack_pointer!
>>>
>>
>> But we only inline today if caller is in the same translation unit and
>> builds with optimization, no?
>
>
> Don't know, but Martin's encountering a case where it is being inlined - is
> that likely to be unique for some reason?
>

Okay I may have confused myself.

My original reasoning was: A lot of calls to
os::current_stack_pointer() today happen not-inlined. That includes
calls from outside os__.cpp, and calls from inside
os__.cpp if slowdebug. Hence, since the VM - in particular
the slowdebug one - seems to work fine, it should be okay to mark
os::current_stack_pointer() unconditionally as NOINLINE.

However, then I saw that the only "real" function (real meaning not
just asserting something) using os::current_stack_pointer() and living
in the same translation unit is os::current_frame(), e.g. on linux:

frame os::current_frame() {
  intptr_t* fp = _get_previous_fp();
  frame myframe((intptr_t*)os::current_stack_pointer(),
(intptr_t*)fp,
CAST_FROM_FN_PTR(address, os::current_frame));
  if (os::is_first_C_frame(&myframe)) {
// stack is not walkable
return frame();
  } else {
return os::get_sender_for_C_frame(&myframe);
  }
}

how does this even work if os::current_stack_pointer() is not inlined?
In slowdebug? Would the frame object in this case not contain the SP
from the frame built up for os::current_stack_pointer()?

So, now I wonder if making os::current_stack_pointer() NOINLINE would
break os::current_frame() - make if produce frames with a slightly-off
SP. os::current_frame() is used e.g. in NMT. So, I wonder if NMT still
works if os::current_stack_pointer is made NOINLINE, or in slowdebug.

> I never assume the compiler does the obvious these days :) or that there
> can't be clever link-time tricks as well.

Oh. I did not think of that at all, you are right.

>
> Maybe the safest thing to do is to only make a change for the clang case and
> leave everything else alone.
>

It would be better to know for sure, though.

..Thomas

> David
> -
>
>>
>> ..Thomas
>>
>>> David

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

[David Holmes]

On 21/06/2018 10:37 PM, Thomas Stüfe wrote:

On Thu, Jun 21, 2018 at 1:27 PM, David Holmes  wrote:

On 21/06/2018 10:05 AM, Martin Buchholz wrote:


On Wed, Jun 20, 2018 at 4:03 PM, Martin Buchholz mailto:marti...@google.com>> 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() {


If y'all like the addition of NOINLINE, it should probably be added to all
of the 14 variants of os::current_stack_pointer.
Gives me a chance to try out the submit repo.



I can't help but think other platforms actually rely on it being inlined so
that it really does return the stack pointer of the method calling
os::current_stack_pointer!



But we only inline today if caller is in the same translation unit and
builds with optimization, no?


Don't know, but Martin's encountering a case where it is being inlined - 
is that likely to be unique for some reason?


I never assume the compiler does the obvious these days :) or that there 
can't be clever link-time tricks as well.


Maybe the safest thing to do is to only make a change for the clang case 
and leave everything else alone.


David
-



..Thomas


David

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

[Martin Buchholz]

I see:

Don't use
  // os::current_stack_pointer(), as its result can be slightly below
current
  // stack pointer, causing us to not alloca enough to reach "bottom".

If you really really want to get the stack pointer of the current frame,
you can't put it in a function!  Use magic compiler extensions via a macro.

gcc and clang both have __builtin_frame_address(0).

gcc BUT not clang has
register uint64_t rsp asm ("rsp");
BUT that gives a slightly different value from __builtin_frame_address(0)
(different register? don't know much about x86 assembly)


On Thu, Jun 21, 2018 at 5:37 AM, Thomas Stüfe 
wrote:

> On Thu, Jun 21, 2018 at 1:27 PM, David Holmes 
> wrote:
> > On 21/06/2018 10:05 AM, Martin Buchholz wrote:
> >>
> >> On Wed, Jun 20, 2018 at 4:03 PM, 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() {
> >>
> >>
> >> If y'all like the addition of NOINLINE, it should probably be added to
> all
> >> of the 14 variants of os::current_stack_pointer.
> >> Gives me a chance to try out the submit repo.
> >
> >
> > I can't help but think other platforms actually rely on it being inlined
> so
> > that it really does return the stack pointer of the method calling
> > os::current_stack_pointer!
> >
>
> But we only inline today if caller is in the same translation unit and
> builds with optimization, no?
>
> ..Thomas
>
> > David
>

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

[Thomas Stüfe]

On Thu, Jun 21, 2018 at 1:27 PM, David Holmes  wrote:
> On 21/06/2018 10:05 AM, Martin Buchholz wrote:
>>
>> On Wed, Jun 20, 2018 at 4:03 PM, 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() {
>>
>>
>> If y'all like the addition of NOINLINE, it should probably be added to all
>> of the 14 variants of os::current_stack_pointer.
>> Gives me a chance to try out the submit repo.
>
>
> I can't help but think other platforms actually rely on it being inlined so
> that it really does return the stack pointer of the method calling
> os::current_stack_pointer!
>

But we only inline today if caller is in the same translation unit and
builds with optimization, no?

..Thomas

> David

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

[David Holmes]

On 21/06/2018 10:05 AM, Martin Buchholz wrote:
On Wed, Jun 20, 2018 at 4:03 PM, 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() {


If y'all like the addition of NOINLINE, it should probably be added to 
all of the 14 variants of os::current_stack_pointer.

Gives me a chance to try out the submit repo.


I can't help but think other platforms actually rely on it being inlined 
so that it really does return the stack pointer of the method calling 
os::current_stack_pointer!


David

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

[Martin Buchholz]

On Wed, Jun 20, 2018 at 4:03 PM, 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() {
>

If y'all like the addition of NOINLINE, it should probably be added to all
of the 14 variants of  os::current_stack_pointer.
Gives me a chance to try out the submit repo.

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

[Erik Joelsson]

Looks good to me.

/Erik


On 2018-06-20 16:49, Martin Buchholz wrote:

Thanks Erik.

On Wed, Jun 20, 2018 at 4:19 PM, Erik Joelsson 
mailto:erik.joels...@oracle.com>> wrote:


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?


Yes, that does look like a better place:

 --- a/make/autoconf/flags-cflags.m4
+++ b/make/autoconf/flags-cflags.m4
@@ -470,14 +470,6 @@ 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
@@ -499,6 +491,12 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_HELPER],
     # (see http://llvm.org/bugs/show_bug.cgi?id=7554)
     TOOLCHAIN_CFLAGS_JVM="$TOOLCHAIN_CFLAGS_JVM -flimit-debug-info"
+    # 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"

+
     if test "x$OPENJDK_TARGET_OS" = xlinux; then
       TOOLCHAIN_CFLAGS_JDK="-pipe"
 TOOLCHAIN_CFLAGS_JDK_CONLY="-fno-strict-aliasing" # technically NOT 
for CXX

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

[Martin Buchholz]

Thanks Erik.

On Wed, Jun 20, 2018 at 4:19 PM, Erik Joelsson 
wrote:

> 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?


Yes, that does look like a better place:

 --- a/make/autoconf/flags-cflags.m4
+++ b/make/autoconf/flags-cflags.m4
@@ -470,14 +470,6 @@ 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
@@ -499,6 +491,12 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_HELPER],
 # (see http://llvm.org/bugs/show_bug.cgi?id=7554)
 TOOLCHAIN_CFLAGS_JVM="$TOOLCHAIN_CFLAGS_JVM -flimit-debug-info"

+# 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"
+
 if test "x$OPENJDK_TARGET_OS" = xlinux; then
   TOOLCHAIN_CFLAGS_JDK="-pipe"
   TOOLCHAIN_CFLAGS_JDK_CONLY="-fno-strict-aliasing" # technically NOT
for CXX

Re: 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 
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

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

[Martin Buchholz]

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 
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
>