On Tue, 21 Feb 2023 08:58:50 GMT, Johannes Bechberger
wrote:
>> Extends the existing AsyncGetCallTrace test case and fixes the issue.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Update full name
> [db483a3](htt
On Tue, 21 Feb 2023 08:58:50 GMT, Johannes Bechberger wrote:
>> Extends the existing AsyncGetCallTrace test case and fixes the issue.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Update full name
Testing looks go
> Extends the existing AsyncGetCallTrace test case and fixes the issue.
Johannes Bechberger has updated the pull request incrementally with one
additional commit since the last revision:
Update full name
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12535/files
- new: ht
On Mon, 20 Feb 2023 09:18:46 GMT, Johannes Bechberger wrote:
>> Extends the existing AsyncGetCallTrace test case and fixes the issue.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Improve condition again
Looks goo
On Mon, 20 Feb 2023 09:18:46 GMT, Johannes Bechberger wrote:
>> Extends the existing AsyncGetCallTrace test case and fixes the issue.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Improve condition again
The lates
On Mon, 20 Feb 2023 09:18:46 GMT, Johannes Bechberger wrote:
>> Extends the existing AsyncGetCallTrace test case and fixes the issue.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Improve condition again
The new c
On Mon, 20 Feb 2023 09:11:38 GMT, Johannes Bechberger wrote:
>> Probably it's better to adapt the limit though (sorry).
>> Problem with this version is that it allows `unextended_sp` outside the
>> address range reserved for the stack. Let's see what other reviewers think.
>> Suggestion:
>>
>>
> Extends the existing AsyncGetCallTrace test case and fixes the issue.
Johannes Bechberger has updated the pull request incrementally with one
additional commit since the last revision:
Improve condition again
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12535/files
-
On Mon, 20 Feb 2023 09:08:59 GMT, Richard Reingruber wrote:
>> ... and no regression in the other serviceability tests.
>
> Probably it's better to adapt the limit though (sorry).
> Problem with this version is that it allows `unextended_sp` outside the
> address range reserved for the stack. Le
On Mon, 20 Feb 2023 09:04:39 GMT, Johannes Bechberger wrote:
>> The sanity test worked :)
>
> ... and no regression in the other serviceability tests.
Probably it's better to adapt the limit though (sorry).
Problem with this version is that it allows `unextended_sp` outside the address
range re
On Mon, 20 Feb 2023 09:00:28 GMT, Johannes Bechberger wrote:
>> src/hotspot/cpu/x86/frame_x86.cpp line 75:
>>
>>> 73: // interpreted -> interpreted calls that go through a method handle
>>> linker,
>>> 74: // since those pop the last argument (the appendix) from the stack.
>>> 75: if (!th
> Extends the existing AsyncGetCallTrace test case and fixes the issue.
Johannes Bechberger has updated the pull request incrementally with one
additional commit since the last revision:
Improve condition
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12535/files
- new: h
On Mon, 20 Feb 2023 08:46:43 GMT, Richard Reingruber wrote:
>> Johannes Bechberger has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Revert previous change
>
> src/hotspot/cpu/x86/frame_x86.cpp line 75:
>
>> 73: // interpreted -> interp
On Fri, 17 Feb 2023 14:47:56 GMT, Johannes Bechberger wrote:
>> Extends the existing AsyncGetCallTrace test case and fixes the issue.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Revert previous change
src/hotspo
On Fri, 17 Feb 2023 14:47:56 GMT, Johannes Bechberger wrote:
>> Extends the existing AsyncGetCallTrace test case and fixes the issue.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Revert previous change
I'm somewh
On Fri, 17 Feb 2023 14:47:56 GMT, Johannes Bechberger wrote:
>> Extends the existing AsyncGetCallTrace test case and fixes the issue.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Revert previous change
Is this PR
On Thu, 16 Feb 2023 14:25:15 GMT, Johannes Bechberger wrote:
>> Extends the existing AsyncGetCallTrace test case and fixes the issue.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Implement addptr suggestion by @Jo
> Extends the existing AsyncGetCallTrace test case and fixes the issue.
Johannes Bechberger has updated the pull request incrementally with one
additional commit since the last revision:
Revert previous change
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12535/files
- n
On Fri, 17 Feb 2023 14:13:04 GMT, Johannes Bechberger wrote:
> Could you be so kind to synthesize a comment for me? I'm slightly overwhelmed
> by the discussion.
Something like:
// Note: sp can be greater than unextended_sp in the case of
// interpreted -> interpreted calls that go through a
On Fri, 17 Feb 2023 14:00:45 GMT, Jorn Vernee wrote:
> But, in that case, please leave a comment that explains why we can have sp >
> unextended_sp.
Could you be so kind to synthesize a comment for me? I'm slightly overwhelmed
by the discussion.
-
PR: https://git.openjdk.org/jdk/
On Thu, 16 Feb 2023 14:25:15 GMT, Johannes Bechberger wrote:
>> Extends the existing AsyncGetCallTrace test case and fixes the issue by
>> modifying `MethodHandles` code.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
On Thu, 16 Feb 2023 14:25:15 GMT, Johannes Bechberger wrote:
>> Extends the existing AsyncGetCallTrace test case and fixes the issue by
>> modifying `MethodHandles` code.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
On Thu, 16 Feb 2023 14:25:15 GMT, Johannes Bechberger wrote:
>> Extends the existing AsyncGetCallTrace test case and fixes the issue by
>> modifying `MethodHandles` code.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
On Thu, 16 Feb 2023 14:25:15 GMT, Johannes Bechberger wrote:
>> Extends the existing AsyncGetCallTrace test case and fixes the issue by
>> modifying `MethodHandles` code.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
On Thu, 16 Feb 2023 11:00:14 GMT, Richard Reingruber wrote:
>>> (Maybe too) long version:
>>
>> Not at all :) I don't know much about the interpreter, so this is pretty
>> helpful.
>>
>> Ok. I think the main point is that a MH linker method doesn't ~have~ use a
>> c2i adapter, it instead has
> Fixes the issue by applying a fix that is already implemented in PPC.
Johannes Bechberger has updated the pull request incrementally with one
additional commit since the last revision:
Implement addptr suggestion by @JohnVernee and @reinrich
-
Changes:
- all: https://git.open
On Thu, 16 Feb 2023 00:36:03 GMT, Jorn Vernee wrote:
> So... my understanding is that a c2i adapter is used when a callee is
> interpreted, so its `from_compiled_entry` points to the c2i adapter. A
> compiled caller calls the `from_compiled_entry`, so it ends up going through
> the `c2i` adapter
On Wed, 15 Feb 2023 23:20:00 GMT, Richard Reingruber wrote:
> (Maybe too) long version:
Not at all :) I don't know much about the interpreter, so this is pretty
helpful.
Ok. I think the main point is that a MH linker method doesn't have a c2i
adapter, it instead has separate versions for inte
On Wed, 15 Feb 2023 21:39:52 GMT, Jorn Vernee wrote:
> The entry generated by generate_method_handle_interpreter_entry is
> essentially the from_interpreted_entry, AFAIU.
But it is also stored in `Method::_i2i_entry` which is [used by c2i
adapters](https://github.com/openjdk/jdk/blob/3ba156082
On Wed, 15 Feb 2023 21:22:48 GMT, Richard Reingruber wrote:
> If an ordinary interpreter entry is used for a call this means the callee
> will be interpreted
Maybe we're not talking about the same thing?
`Method` has a `from_interpreted_entry` and a `from_compiled_entry`, but which
one is use
On Wed, 15 Feb 2023 21:12:19 GMT, Jorn Vernee wrote:
>>> I think your conclusion is correct wrt `unextended_sp < sp` in that case.
>>> For certain MH linker calls, an additional `MemberName` 'appendix' is
>>> passed as the last argument. The MH adapter/trampoline stub pops the
>>> appendix arg
On Wed, 15 Feb 2023 21:05:07 GMT, Richard Reingruber wrote:
> I don't think you can do that. Modifying the unextended_sp would break
> accesses to compiled frames (gc, locals).
This is for interpreter entries, so the caller is definitely interpreted. If
the callee is compiled, it shouldn't car
On Wed, 15 Feb 2023 20:38:04 GMT, Johannes Bechberger wrote:
>>> I think `unextended_sp < sp` is possible on x86 when interpreter entries
>>> generated by `MethodHandles::generate_method_handle_interpreter_entry()`
>>> are executed. They modify `sp` ([here for
>>> example](https://github.com/o
On Wed, 15 Feb 2023 20:24:16 GMT, Jorn Vernee wrote:
>> Yes.
>
>> I think `unextended_sp < sp` is possible on x86 when interpreter entries
>> generated by `MethodHandles::generate_method_handle_interpreter_entry()` are
>> executed. They modify `sp` ([here for
>> example](https://github.com/ope
On Wed, 15 Feb 2023 09:01:29 GMT, Johannes Bechberger wrote:
>> Are you referring to the test in
>> test/hotspot/jtreg/serviceability/AsyncGetCallTrace/ ?
>
> Yes.
> I think `unextended_sp < sp` is possible on x86 when interpreter entries
> generated by `MethodHandles::generate_method_handle_i
On Wed, 15 Feb 2023 09:00:23 GMT, David Holmes wrote:
>>> What is the test case?
>>
>> The sanity test I amended with more checks.
>>
>>> Is it sending signals and then using AGCT?
>>
>> No. Just calling ASGCT directly in a native method which is called directly
>> in the main method of a JTR
On Wed, 15 Feb 2023 07:50:56 GMT, Johannes Bechberger wrote:
>> What is the test case? Is it sending signals and then using AGCT? If so then
>> maybe a signal hits when a call is made but before the code has a chance to
>> update unextended_sp?
>
>> What is the test case?
>
> The sanity test I
On Wed, 15 Feb 2023 07:46:02 GMT, David Holmes wrote:
> What is the test case?
The sanity test I amended with more checks.
> Is it sending signals and then using AGCT?
No. Just calling ASGCT directly in a native method which is called directly in
the main method of a JTREG test.
On Wed, 15 Feb 2023 07:10:29 GMT, Johannes Bechberger wrote:
>> From `frame_x86.hpp`:
>>
>> // The interpreter and adapters will extend the frame of the caller.
>> // Since oopMaps are based on the sp of the caller before extension
>> // we need to know that value. However in order to comp
On Wed, 15 Feb 2023 05:17:31 GMT, David Holmes wrote:
>> I think `unextended_sp < sp` is possible on x86 when interpreter entries
>> generated by `MethodHandles::generate_method_handle_interpreter_entry()` are
>> executed. They modify `sp` ([here for
>> example](https://github.com/openjdk/jdk/
On Tue, 14 Feb 2023 09:22:00 GMT, Richard Reingruber wrote:
>> The original assumption does not always hold anymore and should therefore
>> relaxed.
>
> I think `unextended_sp < sp` is possible on x86 when interpreter entries
> generated by `MethodHandles::generate_method_handle_interpreter_ent
On Tue, 14 Feb 2023 07:38:21 GMT, Johannes Bechberger wrote:
>> The test fails without this relaxation and the relaxation has been used on
>> PPC to solve a similar issue a few years back.
>
> The original assumption does not always hold anymore and should therefore
> relaxed.
I think `unexten
On Tue, 14 Feb 2023 06:41:21 GMT, David Holmes wrote:
>> Fixes the issue by applying a fix that is already implemented in PPC.
>
> src/hotspot/cpu/x86/frame_x86.cpp line 72:
>
>> 70:
>> 71: // unextended sp must be within the stack
>> 72: if (!thread->is_in_full_stack_checked(unextended_sp)
On Tue, 14 Feb 2023 07:04:16 GMT, Johannes Bechberger wrote:
>> src/hotspot/cpu/x86/frame_x86.cpp line 72:
>>
>>> 70:
>>> 71: // unextended sp must be within the stack
>>> 72: if (!thread->is_in_full_stack_checked(unextended_sp)) {
>>
>> I'm not at all sure this relaxation is valid. What a
On Mon, 13 Feb 2023 14:39:00 GMT, Johannes Bechberger wrote:
> Fixes the issue by applying a fix that is already implemented in PPC.
src/hotspot/cpu/x86/frame_x86.cpp line 72:
> 70:
> 71: // unextended sp must be within the stack
> 72: if (!thread->is_in_full_stack_checked(unextended_sp))
Fixes the issue by applying a fix that is already implemented in PPC.
-
Commit messages:
- Fix typo in ARM change
- Fix ASGCT sanity test
- Check more frames in ASGCT sanity test
Changes: https://git.openjdk.org/jdk/pull/12535/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&p
46 matches
Mail list logo
