Skip to content

Address various issues related to the crash in #402#410

Draft
kelloggm wants to merge 29 commits intomainfrom
returnTypeFail
Draft

Address various issues related to the crash in #402#410
kelloggm wants to merge 29 commits intomainfrom
returnTypeFail

Conversation

@kelloggm
Copy link
Collaborator

@kelloggm kelloggm commented Apr 1, 2025

Fixes #402.

I think the test for #403 was incorrectly added to the branch for #402. I've added an @Ignore flag to it, which we can remove when we fix that properly. I fixed some issues related to it, but the test still doesn't pass (although it no longer crashes); this PR is too big already, though, so I'll leave that for later.

This PR incorporates the changes in #407 and should not be reviewed properly until after #407 is merged.

Summary of changes:

  • make SpeciminRunner keep the list of previous iterations in a linked hash set, to help with debugging (I want them printed in the order they occur). This should have minimal overhead, because the number of elements in the set is small (usually < 10, even for large inputs; it is bounded by the program's longest def-use chain, like CF WPI).
  • make the JavaTypeCorrect logic for updating types based on feedback from the compiler also search synthetic inner classes for methods and fields
  • make inner class logic non-idempotent when a more-precise version of the inner class is discovered (necessary if it e.g. is found to have a method in a later iteration, as is the case in the returnTypeFail test case for JsonInclude.Value)
  • fix the class and package map to avoid having the last package part in a wildcard import be in the map as if it were a type. This was just a bug, and is the proximate cause of the String OOB in string index out of bounds crash #403 (iff there is a variable with the same name as the last part of the wildcard import's package name)
  • make the updateMissingClass method return the class it actually updated, so that further uses of the classes don't use an malformed class that should be an outer/inner pair. This change had to be threaded through a lot of UnsolvedSymbolVisitor, but probably fixes a few latent miscompilation bugs
  • unified a bunch of logic for splitting apart FQNs into package and class names
  • fixed a bug in the logic for processing type names from javac that broke on unconstrained wildcards (it handled extends and super bounds, but not ? by itself); this one is related to string index out of bounds crash #403's test, not to this PR's test, but oh well
  • some other misc updates to support inner classes, using the heuristic that class names are capitalized and package names are not

@kelloggm kelloggm requested a review from M3CHR0M4NC3R April 1, 2025 16:51
@kelloggm kelloggm linked an issue Apr 1, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

failed to solve return type crash

2 participants