RFR(S): JDK-8203481 Incorrect constraint for unextended_sp in frame:safe_for_sender

[Dmitry Samersoff]

Dan,

On 06/14/2018 11:49 PM, Daniel D. Daugherty wrote:
> On 6/10/18 12:54 PM, Dmitry Samersoff wrote:
>> Hello Everybody,
>>
>> Please, review updated webrev:
>>
>> http://cr.openjdk.java.net/~dsamersoff/JDK-8203481/webrev.02

> This is still very confusing. In one part of this thread, we're talking
> like this is ARM only fix because X64 does things differently. In another
> part of this thread, we're talking like this bug affects all platforms,
> but I haven't seen failures on X64 due to this so...  Something is still
> not making sense here.

jfr tracing testcase uses specific pattern for test, - deep recursion.

On AArch64 this case triggers assert.

On x86 unextended_sp  is equal to sp, in this case so no asserts is
triggered. So you don't have any crashes on x86.

I didn't look closely to x86 code and can't say why is this difference,
so I'm OK to keep this patch for AArh64 only until we experience a
problem on other platform.

> The patch itself is inconsistent between platforms. Here's the key
> logic line in all four platforms:
> 
> +  bool unextended_sp_safe = (unextended_sp < thread->stack_base());
> +                             (unextended_sp <= thread->stack_base()));
> +  bool unextended_sp_safe = (_UNEXTENDED_SP <= thread->stack_base());
> +  bool unextended_sp_safe = (unextended_sp < thread->stack_base());
> 
> Two platforms use '<' and two use '<='.

it's better to say - this code is not consistent across platforms,
I didn't touch this part of assert, just removed check of
unextended_sp against sp.

-Dmitry

> 
> 
> Dan
> 
> 
>>
>> CR link:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8203481
>>
>> -Dmitry
>>
>> On 05/21/2018 04:44 PM, Dmitry Samersoff wrote:
>>> Hello Everybody,
>>>
>>> Please review small fix
>>>
>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8203481/webrev.01/
>>>
>>> CR:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8203481
>>>
>>> Testing:
>>>
>>> jfr tests that depends to safe_for_sender functionality
>>>
>>> ./jdk/jdk/jfr/api/consumer/TestRecordedFullStackTrace.java
>>> ./jdk/jdk/jfr/event/profiling/TestFullStackTrace.java
>>>
>>> fails on AARCH64.
>>>
>>> These tests passed after the fix.
>>>
>>>
>>
> 


-- 
Dmitry Samersoff
http://devnull.samersoff.net
* There will come soft rains ...