Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check if pointer is in SCC #8327

Merged
merged 2 commits into from
Jan 22, 2020
Merged

Check if pointer is in SCC #8327

merged 2 commits into from
Jan 22, 2020

Conversation

dsouzai
Copy link
Contributor

@dsouzai dsouzai commented Jan 16, 2020

Yet another PR to catch a case where HCR caused a class that used to be in the SCC to not be in the SCC. I missed this path when I opened #7379.

@dsouzai dsouzai requested a review from ymanton January 16, 2020 21:58
@dsouzai
Don't sound like Yoda
360061b 
Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
@dsouzai
Check if pointer is in SCC
d029510 
TR_J9SharedCache::getClassChainOffsetOfIdentifyingLoaderForClazzInSharedCache
makes a call to offsetInSharedCacheFromPointer which will assert if the
pointer is not in the SCC. However, because of HCR, it is possible that
the class pointer we have no longer has a romclass in the SCC; see
eclipse-openj9#7379 for details on this - this
PR adds a check that was missed in 7379.

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
@pshipton pshipton added this to the Release 0.19 (Java 14) milestone Jan 16, 2020
ymanton
ymanton approved these changes Jan 20, 2020
View reviewed changes
Copy link
Member

@ymanton ymanton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@ymanton
Copy link
Member

ymanton commented Jan 20, 2020

Jenkins test sanity all jdk8,jdk13

@dsouzai
Copy link
Contributor Author

dsouzai commented Jan 20, 2020 •
edited
Loading 

Creating placeholder flownodes because failed loading originals.
Setting status of d029510efdeedfc78059d807fdcf9c03e88d2fe4 to FAILURE with url https://ci.eclipse.org/openj9/job/PullRequest-OpenJ9/2464/ and message: ' '
Using context: Pull Request - OpenJ9
java.io.IOException: Tried to load head FlowNodes for execution Owner[PullRequest-OpenJ9/2464:PullRequest-OpenJ9 #2464] but FlowNode was not found in storage for head id:FlowNodeId 1:177
	at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.initializeStorage(CpsFlowExecution.java:679)
	at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.onLoad(CpsFlowExecution.java:716)

not sure if we're still only supposed to launch tests for one jdk version at a time. Will kick off only jdk8 for now.

@dsouzai
Copy link
Contributor Author

dsouzai commented Jan 20, 2020

Jenkins test sanity all jdk8

@dsouzai
Copy link
Contributor Author

dsouzai commented Jan 21, 2020

Jenkins test sanity all jdk11

@ymanton
Copy link
Member

ymanton commented Jan 22, 2020

s390x jdk8 tests failed to run due to infra issues, but since jdk11 tests ran and passed and everything else passed I think that's sufficient.

@ymanton ymanton merged commit edb9df2 into eclipse-openj9:master Jan 22, 2020
@pshipton
Copy link
Member

@dsouzai is there an Issue that is fixed by this?

@pshipton
Copy link
Member

not sure if we're still only supposed to launch tests for one jdk version at a time

all jdk8,jdk13 is still likely to take down jenkins, as it starts too many things all at the same time.

@dsouzai
Copy link
Contributor Author

dsouzai commented Jan 22, 2020 •
edited
Loading

@dsouzai is there an Issue that is fixed by this?

No issue created as this assert was seen internally.

@pshipton
Copy link
Member

@dsouzai so #7890 which is seen internally is a different problem?

@dsouzai
Copy link
Contributor Author

dsouzai commented Jan 23, 2020

Yeah that's right; I have no idea what the root cause is for that issue. I'll likely have to add some asserts and kick off Jenkins builds and "debug" it that way.

@dsouzai dsouzai deleted the fixSCCAssert branch January 4, 2021 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ymanton ymanton ymanton approved these changes

Assignees

@ymanton ymanton

Labels
bug comp:jit:aot comp:jit
Projects
None yet
Milestone
Release 0.19 (Java 14)
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dsouzai @ymanton @pshipton