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

[FIRRTL] Update LowerClasses to alter what PathInfo stores. #7522

Merged
merged 1 commit into from
Aug 15, 2024

Commits on Aug 15, 2024

  1. [FIRRTL] Update LowerClasses to alter what PathInfo stores.

    PathInfo stores a pointer to an Operation, which was problematic
    because that Operation may be deleted in updateInstanceInModule. The
    test case added in this patch would lead to a use-after-free.
    
    This pointer was only really used for a few things, which can be
    handled differently to avoid needing to consider Operation lifetimes.
    
    One use was the operator bool implementation, to check if a PathInfo is
    empty. In the one place this was used, an equivalent check is to query
    the PathInfoTable, and check if the key was not in the table.
    
    Another use was adding the Operation's location in error messages and
    notes. We can safely store a Location directly for these messages.
    
    The final use was to do an isa check while determining the target
    kind. This is where the use in the use-after-free would manifest. For
    this, we do the isa check early, and store the result in a bool.
    
    In summary, we are able to simplify the data in PathInfo in order to
    avoid hanging on to an Operation pointer and needing to worry about
    its lifetime.
    mikeurbach committed Aug 15, 2024
    Configuration menu
    Copy the full SHA
    d8ddc60 View commit details
    Browse the repository at this point in the history