What to do: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Martin Buchholz]

On Wed, Jul 11, 2018 at 8:18 AM, Christian Thalinger <cthalinger at twitter.com
> wrote:

>
>
> On Jul 11, 2018, at 10:56 AM, Martin Buchholz <martinrb at google.com> wrote:
>
>
>
> On Wed, Jul 11, 2018 at 7:34 AM, Christian Thalinger <
> cthalinger at twitter.com> wrote:
>
>>
>> > - why does stack alignment even matter? Isn't it the alignment of c++
>> > objects on the stack that matter?
>>
>> C++ compilers can (and actually do) emit instructions that need
>> alignment, like movaps.  I’ve seen many crashes in the past with movaps and
>> an unaligned stack coming from JIT or stub code.
>
>
> Sure, actual instructions need alignment.  But os::verify_stack_alignment
> isn't doing a good job of finding misaligned instructions, while causing
> trouble for clang builds.  Christian, what do you suggest we do to fix the
> failing assertion in os::verify_stack_alignment ?
>
> I don't know what movaps does, but perhaps os::verify_stack_alignment
> could simply use that instruction via inline asm?
>
>
> movaps is used in the prologue to spill to the stack.  If the stack isn’t
> 16-byte aligned it crashes.  os::verify_stack_alignment just makes it
> easier to find these bugs.
>
> I’m not exactly sure what to do.  I think the best solution is to avoid
> that os::verify_stack_alignment is being inlined.
>
>
I just commented on the bug:
clang inlined os::curent_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. One possible fix is obvious
in hindsight:

-address os::current_stack_pointer() {
+NOINLINE address os::current_stack_pointer() {

BUT logically a call like current_stack_pointer should return the stack
pointer of the __current__ frame, so should probably be a macro that does
inline assembly instead of doing a function call.

   -
   <https://bugs.openjdk.java.net/secure/AddComment!default.jspa?id=4938621>