Improve the tail call identification logic to prefer local jumps unless the target is a known function entry #296
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
It turns out that we have to be more conservative with tail call identification, as incorrectly identifying a block as the target of a tail call (instead of a branch) can cause other branch classifiers to fail if that block is the target of another jump. Ultimately, we will need to give up some tail call recognition (since they are in general indistinguishable from jumps), and instead only identify known call targets as tail call candidates. With additional global analysis we could do better. Fixes #294
travitch
changed the title
Is there an issue tracking this idea? |
RyanGlScott
approved these changes
Jun 27, 2022
For posterity's sake, this is now tracked in #298. |
RyanGlScott
added a commit
to GaloisInc/ambient-verifier
that referenced
this pull request
Mar 1, 2023
This is an alternative fix for GaloisInc/macaw#285 (i.e., allowing tail calls into PLT stubs) without needing to rearrange the order in which `macaw`'s classifiers run (the culprit behind GaloisInc/macaw#294). Our new approach is to mark all PLT stub addresses as trusted function entry points so that code discovery will be more likely to analyze them. See the comments on the new `lbpTrustedPltEntryPoints` function for the full details. This bumps the `macaw` submodule to bring in the changes from GaloisInc/macaw#296, which prompted the need for changing the behavior on the `ambient-verifier` side in the first place.
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.
It turns out that we have to be more conservative with tail call identification, as incorrectly identifying a block as the target of a tail call (instead of a branch) can cause other branch classifiers to fail if that block is the target of another jump.
The tradeoff is that we will recognize fewer tail calls, but we will also encounter fewer situations where intra-procedural jumps are not resolved. Ultimately, we will need to give up some tail call recognition (since they are in general indistinguishable from jumps), and instead only identify known call targets as tail call candidates.
With additional global analysis we could do better.
Fixes #294