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