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

Martin Buchholz martinrb at google.com
Fri Jun 22 16:41:46 UTC 2018


(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 <thomas.stuefe at gmail.com>
wrote:

> On Fri, Jun 22, 2018 at 1:57 PM, David Holmes <david.holmes at oracle.com>
> wrote:
> > On 21/06/2018 10:37 PM, Thomas Stüfe wrote:
> >>
> >> On Thu, Jun 21, 2018 at 1:27 PM, David Holmes <david.holmes at oracle.com>
> >> wrote:
> >>>
> >>> On 21/06/2018 10:05 AM, Martin Buchholz wrote:
> >>>>
> >>>>
> >>>> On Wed, Jun 20, 2018 at 4:03 PM, Martin Buchholz <martinrb at google.com
> >>>> <mailto:martinrb at 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?
> >
>
> 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_<os>_<cpu>.cpp, and calls from inside
> os_<os>_<cpu>.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
>


More information about the hotspot-runtime-dev mailing list