Check RomClass is in SCC before storing IProf Data #2510
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@mpirvu Could you please review. |
mpirvu
requested changes
Jul 30, 2018
| * the romClass is within the SCC. | ||
| */ | ||
| if (romClass >= cacheStartAddress) | ||
| { |
There was a problem hiding this comment.
Where do we test that the romClass after the SCC?
|
@mpirvu Changes made. On a related note, shouldn't this https://github.com/eclipse/openj9/pull/2510/files#diff-31c18c7318f7ce5d1b5699cf26523d07R2999 be |
|
In reply to #2510 (comment) |
The following race is possible: 1. Thread 1 calls TR_IPBCDataCallGraph::canBePersisted on Entry A, which succeeds. However, at least one class in the list is NULL 2. Thread 2 calls TR_IPBCDataCallGraph::setData on Entry A, which also succeeds, setting one of the classes that was previous NULL to something non-NULL. 3. Thread 1 calls TR_IPBCDataCallGraph::createPersistentCopy. Normally if a class in the list is NULL, the value stored is also NULL. However, because Thread 2 updated some previously NULL class, Thread 1 will compute the difference of (ramClass->romClass - startOfSCC) and potentially get a value that's bigger than the SCC. This commit checks for this race before persisting IProfiler data. Signed-off-by: Irwin D'Souza <dsouzai@ca.ibm.com>
|
Fixed the copyright and added the |
|
jenkins test all |
|
The few failures are not related to this PR, so merging. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The following race is possible:
which succeeds. However, at least one class in the list is NULL
also succeeds, setting one of the classes that was previous NULL
to something non-NULL.
Normally if a class in the list is NULL, the value stored is also
NULL. However, because Thread 2 updated some previously NULL class,
Thread 1 will compute the difference of
(ramClass->romClass - startOfSCC) and potentially get a value
that's bigger than the SCC.
This commit checks for this race before persisting IProfiler data.