RFR: 8261702: ClhsdbFindPC can fail due to PointerFinder incorrectly thinking an address is in a .so

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8261702: ClhsdbFindPC can fail due to PointerFinder incorrectly thinking an address is in a .so

Chris Plummer-2
See bug description for details. The fix is simple and just a matter of moving some code to avoid incorrectly thinking an address is in a .so when it is in some other area of VM memory that occurs just after the last .so.

-------------

Commit messages:
 - Check if addr is in DSO last to avoid finding addr in DSO when it is not due to DSO.size() returning too large of a value.

Changes: https://git.openjdk.java.net/jdk/pull/2562/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2562&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261702
  Stats: 24 lines in 1 file changed: 13 ins; 11 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2562.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2562/head:pull/2562

PR: https://git.openjdk.java.net/jdk/pull/2562
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261702: ClhsdbFindPC can fail due to PointerFinder incorrectly thinking an address is in a .so

Yasumasa Suenaga-7
On Sat, 13 Feb 2021 19:05:17 GMT, Chris Plummer <[hidden email]> wrote:

> See bug description for details. The fix is simple and just a matter of moving some code to avoid incorrectly thinking an address is in a .so when it is in some other area of VM memory that occurs just after the last .so.

Your change looks good, but according to JBS description, should we change Linux DSO.java ? It can be separate this PR of course.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2562
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261702: ClhsdbFindPC can fail due to PointerFinder incorrectly thinking an address is in a .so

Chris Plummer-2
On Sun, 14 Feb 2021 02:47:13 GMT, Yasumasa Suenaga <[hidden email]> wrote:

> Your change looks good, but according to JBS description, should we change Linux DSO.java ? It can be separate this PR of course.

Thanks. I filed [JDK-8261710](https://bugs.openjdk.java.net/browse/JDK-8261710), but don't have any plans to work on it.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2562
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261702: ClhsdbFindPC can fail due to PointerFinder incorrectly thinking an address is in a .so

Yasumasa Suenaga-7
In reply to this post by Chris Plummer-2
On Sat, 13 Feb 2021 19:05:17 GMT, Chris Plummer <[hidden email]> wrote:

> See bug description for details. The fix is simple and just a matter of moving some code to avoid incorrectly thinking an address is in a .so when it is in some other area of VM memory that occurs just after the last .so.

Marked as reviewed by ysuenaga (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/2562
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261702: ClhsdbFindPC can fail due to PointerFinder incorrectly thinking an address is in a .so

Yasumasa Suenaga-7
On Sun, 14 Feb 2021 07:14:52 GMT, Yasumasa Suenaga <[hidden email]> wrote:

>> See bug description for details. The fix is simple and just a matter of moving some code to avoid incorrectly thinking an address is in a .so when it is in some other area of VM memory that occurs just after the last .so.
>
> Marked as reviewed by ysuenaga (Reviewer).

> Thanks. I filed [JDK-8261710](https://bugs.openjdk.java.net/browse/JDK-8261710), but don't have any plans to work on it.

Ok, I will send PR for  [JDK-8261710](https://bugs.openjdk.java.net/browse/JDK-8261710) ( #2563 ) if all pre-submit checks are passed.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2562
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261702: ClhsdbFindPC can fail due to PointerFinder incorrectly thinking an address is in a .so

Kevin Walls-2
In reply to this post by Chris Plummer-2
On Sat, 13 Feb 2021 19:05:17 GMT, Chris Plummer <[hidden email]> wrote:

> See bug description for details. The fix is simple and just a matter of moving some code to avoid incorrectly thinking an address is in a .so when it is in some other area of VM memory that occurs just after the last .so.

Looks good.

-------------

Marked as reviewed by kevinw (Committer).

PR: https://git.openjdk.java.net/jdk/pull/2562
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261702: ClhsdbFindPC can fail due to PointerFinder incorrectly thinking an address is in a .so

Daniel D.Daugherty
On Tue, 16 Feb 2021 09:10:17 GMT, Kevin Walls <[hidden email]> wrote:

>> See bug description for details. The fix is simple and just a matter of moving some code to avoid incorrectly thinking an address is in a .so when it is in some other area of VM memory that occurs just after the last .so.
>
> Looks good.

While I was characterizing:

    JDK-8261844 serviceability/sa/ClhsdbFindPC.java#id1 failed with "'In code in NMethod for LingeredAppWithTrivialMain.main' missing from stdout/stderr"

I determined that I could easily reproduce serviceability/sa/ClhsdbFindPC.java failures
on my Ubuntu 16.04 test machine. I've applied this version of this PR:

$ git log HEAD^!
commit fea1ed851ce6a93030169e7de0daac2c2ac12a31 (HEAD -> pull/2562)
Author: Chris Plummer <[hidden email]>
Date:   Sat Feb 13 19:02:24 2021 +0000

    Check if addr is in DSO last to avoid finding addr in DSO when it is not due to DSO.size() returning too large of a value.

and retested. I no longer see serviceability/sa/ClhsdbFindPC.java failures.
I did a total of 6 runs (X3 configs) and there were no failures.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2562
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261702: ClhsdbFindPC can fail due to PointerFinder incorrectly thinking an address is in a .so

Chris Plummer-2
On Tue, 16 Feb 2021 21:29:23 GMT, Daniel D. Daugherty <[hidden email]> wrote:

>> Looks good.
>
> While I was characterizing:
>
>     JDK-8261844 serviceability/sa/ClhsdbFindPC.java#id1 failed with "'In code in NMethod for LingeredAppWithTrivialMain.main' missing from stdout/stderr"
>
> I determined that I could easily reproduce serviceability/sa/ClhsdbFindPC.java failures
> on my Ubuntu 16.04 test machine. I've applied this version of this PR:
>
> $ git log HEAD^!
> commit fea1ed851ce6a93030169e7de0daac2c2ac12a31 (HEAD -> pull/2562)
> Author: Chris Plummer <[hidden email]>
> Date:   Sat Feb 13 19:02:24 2021 +0000
>
>     Check if addr is in DSO last to avoid finding addr in DSO when it is not due to DSO.size() returning too large of a value.
>
> and retested. I no longer see serviceability/sa/ClhsdbFindPC.java failures.
> I did a total of 6 runs (X3 configs) and there were no failures.

@dcubed-ojdk Can you also try pull/2563. It's for https://bugs.openjdk.java.net/browse/JDK-8261710, which is an attempt at a more proper fix for this bug (this PR is just avoiding the real issue, which is an incorrect DSO size calculation.)

-------------

PR: https://git.openjdk.java.net/jdk/pull/2562
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261702: ClhsdbFindPC can fail due to PointerFinder incorrectly thinking an address is in a .so

Daniel D.Daugherty
On Tue, 16 Feb 2021 21:43:53 GMT, Chris Plummer <[hidden email]> wrote:

>> While I was characterizing:
>>
>>     JDK-8261844 serviceability/sa/ClhsdbFindPC.java#id1 failed with "'In code in NMethod for LingeredAppWithTrivialMain.main' missing from stdout/stderr"
>>
>> I determined that I could easily reproduce serviceability/sa/ClhsdbFindPC.java failures
>> on my Ubuntu 16.04 test machine. I've applied this version of this PR:
>>
>> $ git log HEAD^!
>> commit fea1ed851ce6a93030169e7de0daac2c2ac12a31 (HEAD -> pull/2562)
>> Author: Chris Plummer <[hidden email]>
>> Date:   Sat Feb 13 19:02:24 2021 +0000
>>
>>     Check if addr is in DSO last to avoid finding addr in DSO when it is not due to DSO.size() returning too large of a value.
>>
>> and retested. I no longer see serviceability/sa/ClhsdbFindPC.java failures.
>> I did a total of 6 runs (X3 configs) and there were no failures.
>
> @dcubed-ojdk Can you also try pull/2563. It's for https://bugs.openjdk.java.net/browse/JDK-8261710, which is an attempt at a more proper fix for this bug (this PR is just avoiding the real issue, which is an incorrect DSO size calculation.)

@plummercj - I tested pull/2563 and added a note over on that PR.
I see failures in 'release' and 'slowdebug' bits with pull/2563.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2562
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261702: ClhsdbFindPC can fail due to PointerFinder incorrectly thinking an address is in a .so

Serguei Spitsyn-2
In reply to this post by Chris Plummer-2
On Sat, 13 Feb 2021 19:05:17 GMT, Chris Plummer <[hidden email]> wrote:

> See bug description for details. The fix is simple and just a matter of moving some code to avoid incorrectly thinking an address is in a .so when it is in some other area of VM memory that occurs just after the last .so.

Looks good.

-------------

Marked as reviewed by sspitsyn (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2562
Reply | Threaded
Open this post in threaded view
|

Integrated: 8261702: ClhsdbFindPC can fail due to PointerFinder incorrectly thinking an address is in a .so

Chris Plummer-2
In reply to this post by Chris Plummer-2
On Sat, 13 Feb 2021 19:05:17 GMT, Chris Plummer <[hidden email]> wrote:

> See bug description for details. The fix is simple and just a matter of moving some code to avoid incorrectly thinking an address is in a .so when it is in some other area of VM memory that occurs just after the last .so.

This pull request has now been integrated.

Changeset: 539c80bf
Author:    Chris Plummer <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/539c80bf
Stats:     24 lines in 1 file changed: 13 ins; 11 del; 0 mod

8261702: ClhsdbFindPC can fail due to PointerFinder incorrectly thinking an address is in a .so

Reviewed-by: ysuenaga, kevinw, sspitsyn

-------------

PR: https://git.openjdk.java.net/jdk/pull/2562