Catch invalid uses of {must,no}_preserve_cheri_tag #648
+74
−78
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 does not make sense to set this attribute for memory transfer intrinsics that copy less than sizeof(cap) and doing so could indicate a logic error in an LLVM pass/the clang logic that sets this attribute initially. This adds a check to the verified as well as an assertion when setting the attribute (to make it easier to create reduced test cases in case we discover further bad calls). Additionally, this also checks that we don't add the attributes for non-CHERI targets.
It was not actually testing what it claimed it was testing and the code to do what it was supposed to test has long been replaced. It just happened to work by chance because must_preserve_cheri_tags was being treated as "force libcall" even for non-CHERI targets. This problem was found by a new IRVerifier check for the preserve tags function attribute.
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 does not make sense to set this attribute for memory transfer
intrinsics that copy less than sizeof(cap) and doing so could indicate
a logic error in an LLVM pass/the clang logic that sets this attribute
initially. This adds a check to the verified as well as an assertion
when setting the attribute (to make it easier to create reduced test
cases in case we discover further bad calls).
Additionally, this also checks that we don't add the attributes for
non-CHERI targets.
This depends on #594 and #650 since we currently create invalid calls.