Support for static contracts embedding pre/post condition during instrumentation pass by kumarak · Pull Request #107 · lifting-bits/patchestry

kumarak · 2025-10-03T18:41:24Z

No description provided.

Copilot

Pull Request Overview

This PR adds comprehensive support for static contracts during the instrumentation pass, extending the existing runtime contract system. It introduces a new Contracts dialect with MLIR attributes to embed pre/post conditions directly into operations, enabling compile-time contract verification alongside runtime validation.

Support for both static and runtime contract types with type-specific processing
New Contracts MLIR dialect with attributes for static contract specifications
Refactored patch and contract operation implementations into separate modules for better maintainability

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`tools/*/CMakeLists.txt`	Added MLIRContracts library linking to all tools
`tools/patchir-transform/main.cpp`	Added Contracts dialect registration and context initialization
`test/patchir-transform/contracts/usb_security_contracts.yaml`	Extended test contracts with static contract examples
`test/patchir-transform/bl_usb__send_message_before_patch.yaml`	Updated test to use static contract ID
`lib/patchestry/Passes/PatchOperationImpl.*`	New implementation module for patch operations
`lib/patchestry/Passes/ContractOperationImpl.*`	New implementation module for contract operations
`lib/patchestry/Passes/InstrumentationPass.cpp`	Refactored to use new implementation modules and support static contracts
`lib/patchestry/Dialect/Contracts/*`	New Contracts dialect with attributes and tablegen definitions
`include/patchestry/YAML/ContractSpec.hpp`	Extended contract specification to support static contracts
`include/patchestry/YAML/BaseSpec.hpp`	Added ContractType enum for runtime/static distinction
`include/patchestry/Passes/*.hpp`	Updated headers for refactored implementation structure

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

lib/patchestry/Passes/InstrumentationPass.cpp

lib/patchestry/Passes/ContractOperationImpl.cpp

include/patchestry/YAML/ContractSpec.hpp

Copilot

Pull Request Overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 5 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-12T11:58:55Z

lib/patchestry/Passes/ContractOperationImpl.cpp

+
+            default:
+                LOG(ERROR) << "Unsupported contract mode for static contracts: "
+                           << infoModeToString(mode) << "\n";


The function infoModeToString is called but it's not in scope. It should be contract::infoModeToString(mode) to use the function from the contract namespace.

Suggested change

<< infoModeToString(mode) << "\n";

<< contracts::infoModeToString(mode) << "\n";

Copilot · 2025-10-12T11:58:56Z

tools/patchir-transform/main.cpp

    const cl::opt< std::string > output_filename( // NOLINT(cert-err58-cpp)
-        "o", llvm::cl::desc("Output filename for the patched CIR"), llvm::cl::value_desc("filename"),
-        llvm::cl::init("-"), cl::cat(category)
+        "o", llvm::cl::desc("Output filename for the patched CIR"),
+        llvm::cl::value_desc("filename"), llvm::cl::init("-"), cl::cat(category)
    );


[nitpick] The line break changes appear to be only formatting changes that don't affect functionality. Consider using a consistent formatting style throughout the file.

Copilot · 2025-10-12T11:58:56Z

lib/patchestry/Passes/InstrumentationPass.cpp

            auto contracts_file_path =
-                ConfigurationFile::getInstance().resolve_path(spec.implementation.code_file);
+                ConfigurationFile::getInstance().resolve_path(spec.implementation->code_file);


Potential null pointer dereference. The code accesses spec.implementation->code_file without verifying that spec.implementation is not null, even though there's a check for !spec.implementation just above. The logic should be restructured to avoid this inconsistency.

Copilot · 2025-10-12T11:58:56Z

include/patchestry/YAML/ContractSpec.hpp

            std::string severity;
-            Implementation implementation;
-            std::optional< std::string > contract_module;
+            ContractType type; // "STATIC" or "RUNTIME"


The comment should be updated to reflect that type is an enum value, not a string. It should read // ContractType::STATIC or ContractType::RUNTIME

Suggested change

ContractType type; // "STATIC" or "RUNTIME"

ContractType type; // ContractType::STATIC or ContractType::RUNTIME

Copilot · 2025-10-12T11:58:56Z

lib/patchestry/Passes/ContractOperationImpl.cpp

+                    targetAttr = ::contracts::TargetAttr::get(
+                        ctx, pred.target, *pred.arg_index, mlir::FlatSymbolRefAttr()
+                    );
+                    targetAttr.dump();


This debug statement targetAttr.dump() should be removed as it appears to be leftover debugging code that will produce unwanted output in production.

Suggested change

targetAttr.dump();

kumarak requested a review from Copilot October 3, 2025 18:41

kumarak marked this pull request as draft October 3, 2025 18:41

Copilot AI reviewed Oct 3, 2025

View reviewed changes

kumarak requested a review from Copilot October 12, 2025 11:57

Copilot AI reviewed Oct 12, 2025

View reviewed changes

kumarak force-pushed the static_contract branch from 73d0c79 to 9ba4bba Compare October 15, 2025 23:51

kumarak added 14 commits November 4, 2025 17:42

support for static contract dialect as attribute

b07dfd3

refactor patch and contract function implementations

e9b0dd4

fix merge error

c88dbcd

fixup! add contract types

1d81a66

Add static metadata contract to llvm ir

1e4bb06

update changes to build on linux for try patching

28a6698

fixup! update apply before and after patches

68a8cc9

Insert function declaration before inserting patches and contracts

b606283

fixup! remove insert function declaration changes

4b73c32

fixup! remove insert func decl

39708cd

fixup! enable inlining

e722a60

temp changes

bcc205a

temp changes

0c1c5a4

Update with demo script

eb29b8f

kumarak force-pushed the static_contract branch from d5de6ab to eb29b8f Compare November 4, 2025 22:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for static contracts embedding pre/post condition during instrumentation pass#107

Support for static contracts embedding pre/post condition during instrumentation pass#107
kumarak wants to merge 14 commits intomainfrom
static_contract

kumarak commented Oct 3, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 12, 2025

Uh oh!

Copilot AI Oct 12, 2025

Uh oh!

Copilot AI Oct 12, 2025

Uh oh!

Copilot AI Oct 12, 2025

Uh oh!

Copilot AI Oct 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	<< infoModeToString(mode) << "\n";
	<< contracts::infoModeToString(mode) << "\n";

	ContractType type; // "STATIC" or "RUNTIME"
	ContractType type; // ContractType::STATIC or ContractType::RUNTIME

Conversation

kumarak commented Oct 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants