Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • D dynamorio
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 1,467
    • Issues 1,467
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 44
    • Merge requests 44
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • DynamoRIO
  • dynamorio
  • Issues
  • #5061
Closed
Open
Issue created Aug 26, 2021 by Derek Bruening@derekbrueningContributor

Indirect branch hashtable target_delete path broken by tag invalidation during fragment removal

Our indirect branch lookup (IBL) has a "target_delete" path that we use to redirect control to exit the cache. This was added in the early days of the code base. At that time the tag was left intact. Later, in fragment_prepare_for_removal_from_table(), we invalidated the tag too (by setting to FAKE_TAG), and updated the open-address linear probing to continue past that sentinel. However, the target_delete path obtains its target app PC value from that very tag entry: so the clobber of that value with the sentinel breaks the whole scheme and causes a crash when DR thinks the app wants to go execute code at the sentinel PC value (-1).

This was discovered on SPECjvm: #3733

If we remove the tag invalidation: we have IBL hits on the tag that then exit every time, causing performance issues. We would need to add some subsequent tag invalidation later in the fragment removal process.

My proposal is to remove the target_delete path altogether. Since we have the tag invalidation, lookups will miss on the deleted fragment, and the only issue is a thread that just did the lookup and is about to jump to the target fragment in the cache. But that thread could already be inside the target anyway. I don't recall a scheme where a synch is done and a thread left alone at that point in the IBL while the fragment is truly deleted from the cache: the thread would be translated somewhere fresh. So I believe this would be safe.

We would need to audit all the other uses of target_delete, such as for the null lookup default value: maybe we'd need to keep it there and just not use it for fragment_prepare_for_removal_from_table().

Assignee
Assign to
Time tracking