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.

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

Improve DDR support #3099

Merged
merged 9 commits into from
Oct 5, 2018
Merged

Improve DDR support #3099

merged 9 commits into from
Oct 5, 2018

Conversation

keithc-ca
Copy link
Contributor

@keithc-ca keithc-ca commented Oct 2, 2018

DDR support in OpenJ9 makes use of the ddrgen tool from OMR which operates on debug information generated by compilers.

Not all compilers retain references to typedefs. For example a field involving type UDATA may instead be recorded as using uint32_t or uint64_t depending on the definition of uintptr_t. This means that we cannot rely on generating DDR pointer classes at build time because they would reflect the type definitions in that build which would be incompatible with core files produced by a VM with a different pointer size.

This makes the necessary foundation changes to allow DDR pointer classes to be created dynamically according to the core file being examined. Small changes will be required in the extension repositories to adopt this new capability.

doc issue eclipse-openj9/openj9-docs#97
Fixes #2507

* remove most type overrides for IDATA/UDATA
* remove unnecessary struct tags
* J9SRP(void) -> J9SRP

Signed-off-by: Keith W. Campbell <keithc@ca.ibm.com>
@pshipton
Copy link
Member

pshipton commented Oct 2, 2018

@keithc-ca please create an issue at https://github.com/eclipse/openj9-docs in order to coordinate updating the release notes

#include "ddrhelp.h"

/*
* This struct has fields correspnding to StructureTypeManager.TYPE_XXX categories
Copy link
Member

Choose a reason for hiding this comment

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

I think this comment could include a little more context, i.e. mention DDR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps the fact that this file is in a folder called ddr provides that context?

@@ -220,15 +221,15 @@ private static String getOption(J9ROMClassPointer romClass, long option) throws
}

private static VoidPointer getStructure(J9ROMClassPointer romClass, long option) throws CorruptDataException {
SelfRelativePointer ptr = getSRPPtr(romClass.optionalInfo(), romClass.optionalFlags(), option);
SelfRelativePointer ptr = getSRPPtr(J9ROMClassHelper.optionalInfo(romClass), romClass.optionalFlags(), option);
Copy link
Member

Choose a reason for hiding this comment

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

What happens if there is code outside this project using optionalInfo(), cpShapeDescription()? jdmpview supports adding debug extensions at runtime via the plugins command. There are many extensions that exist outside of this project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there's a way to avoid that problem - it's a consequence of the behavior of ddrgen (being unable to distinguish among U32Pointer, U64Pointer and UDATAPointer).

Copy link
Member

Choose a reason for hiding this comment

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

What will happen?

Copy link
Member

Choose a reason for hiding this comment

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

Keith created #3131 to deal with this.

*/
private void doArrayMethod(FieldDescriptor field) {
String fieldType = field.getType();
String componentType = fieldType.substring(0, fieldType.lastIndexOf('[')).trim();
Copy link
Member

Choose a reason for hiding this comment

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

Assumes arrays are always single dimension.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. If the field type is Whatever[4][5] it will analyze Whatever[4] and notice the component type has type TYPE_ARRAY. This is the same as the behavior found in PointerGenerator.java.

Include fields corresponding to StructureTypeManager.TYPE_XXX
categories that don't appear elsewhere so that all paths in
BytecodeGenerator and PointerGenerator are exercised.

Signed-off-by: Keith W. Campbell <keithc@ca.ibm.com>
* add these methods which throw NullPointerDereference
-- add AbstractPointer.nonNullAddress()
-- add StructurePointer.nonNullFieldEA(long offset)

Signed-off-by: Keith W. Campbell <keithc@ca.ibm.com>
BytecodeGenerator
* 'structure' classes are generated by a new helper class
* helper classes have been added to generate 'flags' and 'pointers'

J9DDRClassLoader
* delegate loading pointer classes to StructureReader
* generate pointer classes only for version 29 or newer
  (if not present in j9ddr.jar)

StructureAliases29.dat
* rename legacy version and add copyright notice
* add new version for use with blobs produced by ddrgen

StructureReader:
* add StructureTypeManager
* add PackageNameType.POINTER_PACKAGE_SLASH_NAME
* simplify implementation of getPackageName(PackageNameType)
* choose the appropriate set of aliases to use

Signed-off-by: Keith W. Campbell <keithc@ca.ibm.com>
cpShapeDescription
optionalInfo

Signed-off-by: Keith W. Campbell <keithc@ca.ibm.com>
With the new DDR tooling, these may be indistinguishable
from IDATA/UDATA.

Signed-off-by: Keith W. Campbell <keithc@ca.ibm.com>
* add alias GC_ArrayletObjectModel=GC_ArrayObjectModel

Signed-off-by: Keith W. Campbell <keithc@ca.ibm.com>
Signed-off-by: Keith W. Campbell <keithc@ca.ibm.com>
@pshipton
Copy link
Member

pshipton commented Oct 3, 2018

jenkins test all win,zlinux jdk8

@pshipton
Copy link
Member

pshipton commented Oct 3, 2018

@keithc-ca note the test failures

@keithc-ca
Copy link
Contributor Author

I see two failures of the form

[FindVMOutputParser] [ERROR] ParserUtil: missing line J9WSRP(struct J9PoolPuddleList) puddleList

where in the new output the struct tag is omitted: J9WSRP(J9PoolPuddleList): I think the test should be adjusted to match either format.

@keithc-ca
Copy link
Contributor Author

The newest commit fixes the test failure.

* now accepts either of:
    `J9WSRP(J9PoolPuddleList) puddleList`
    `J9WSRP(struct J9PoolPuddleList) puddleList`
* no longer accepts obsolete pattern:
    `J9WSRP(struct J9PoolPuddle) puddleList`
* enhance ParserUtil to allow using patterns

Signed-off-by: Keith W. Campbell <keithc@ca.ibm.com>
@pshipton
Copy link
Member

pshipton commented Oct 5, 2018

jenkins test all win,zlinux jdk8

@pshipton
Copy link
Member

pshipton commented Oct 5, 2018

Looks like the win jdk8 sanity failure is a jenkins issue and all the testing passed.

@pshipton pshipton merged commit f69c425 into eclipse-openj9:master Oct 5, 2018
@keithc-ca keithc-ca deleted the ddr branch October 9, 2018 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants