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

Hibernate's bytecode enhancement DLS_DEAD_LOCAL_STORE false positives #2865

Merged
merged 4 commits into from
Feb 28, 2024

Conversation

gtoison
Copy link
Contributor

@gtoison gtoison commented Feb 10, 2024

The methods added by Hibernate's bytecode enhancement trigger DLS_DEAD_LOCAL_STORE false positives

Fixes #2864

The methods added by Hibernate's bytecode enhancement trigger
DLS_DEAD_LOCAL_STORE false positives
@Test
void testIssue(SpotBugsRunner spotbugs) {
BugCollection bugCollection = spotbugs.performAnalysis(
Paths.get("../spotbugsTestCases/src/classSamples/classEnhanceByHibernate/HibernateEnhancedEntity.class"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

When running this test, several classes are missing:

The following classes needed for analysis were missing:
  org.hibernate.engine.spi.ManagedEntity
  org.hibernate.engine.spi.ExtendedSelfDirtinessTracker
  org.hibernate.bytecode.enhance.internal.tracker.SimpleFieldTracker
  org.hibernate.bytecode.enhance.internal.tracker.SimpleCollectionTracker
  org.hibernate.engine.spi.EntityEntry
  org.hibernate.bytecode.enhance.internal.tracker.DirtyTracker
  org.hibernate.bytecode.enhance.spi.interceptor.LazyAttributeLoadingInterceptor
  org.hibernate.bytecode.enhance.spi.CollectionTracker
  org.hibernate.collection.spi.PersistentCollection

I'm not sure, how much these classes are really needed, and what exactly causes when there are missing classes. The PR solves the referenced issue, so maybe it's not a big deal in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The classes references are from the bytecode added by Hibernate's enhancer.
I looked into adding Hibernate as a dependency and also adding the enhancer plugin to the build but that seems to be overkill, so I just compiled a class and copied it into the samples folder.

@@ -209,6 +209,10 @@ private void analyzeMethod(ClassContext classContext, Method method) throws Data
return;
}

if (method.getName().startsWith("$$_hibernate_")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth ignoring other $$ methods?

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 thought about it by I think the underlying issue might very well be a bug in Hibernate's bytecode enhancer, to at least it seems to be producing suboptimal bytecode here.
In case other false positives are reported it will be easy to ignore more methods

@gtoison
Copy link
Contributor Author

gtoison commented Feb 28, 2024

Is it OK if I merge this PR?
There are a few outstanding PRs and its getting time consuming to merge the conflicts in the changelog
Let me know if you think I could/should/shouldn't/musn't/can't merge my own PR once approved :)

@JuditKnoll
Copy link
Collaborator

Is it OK if I merge this PR? There are a few outstanding PRs and its getting time consuming to merge the conflicts in the changelog Let me know if you think I could/should/shouldn't/musn't/can't merge my own PR once approved :)

I don't see any reason not to merge it.
Usually @hazendaz or @iloveeclipse merge the PRs, so if they could express their opinion it would be really useful.

@hazendaz
Copy link
Member

hazendaz commented Feb 28, 2024 via email

@gtoison gtoison merged commit 654f037 into spotbugs:master Feb 28, 2024
15 checks passed
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.

DLS_DEAD_LOCAL_STORE false positives for Hibernate enhanced entities
4 participants