transform: update yaml specification format and simplify them

Author: kumarak

docs/GettingStarted/patch_specifications.md

@@ -58,7 +58,6 @@ execution_order:                         # Order of patch/contract execution
 
 meta_patches:                            # Meta-patch configurations
   - name: ...
-    id: "..."
     description: "..."
     optimization:                        # Optimization settings
       - "inline-patches"
@@ -82,7 +81,6 @@ meta_patches:                            # Meta-patch configurations
 
 meta_contracts:                          # Meta-contract configurations
   - name: ...
-    id: "..."
     description: "..."
     contract_actions:                    # Individual contract actions
       - name: "..."
@@ -146,7 +144,6 @@ meta_contracts:                          # Meta-contract configurations
 | Field | Description | Example |
 |-------|-------------|---------|
 | `name` | Unique identifier for the patch group | `"usb_security_patches"` |
-| `id` | Internal identifier for the patch group | `"usb_security_patches"` |
 | `description` | Description of the patch group purpose | `"USB security monitoring patches"` |
 | `optimization` | List of optimization flags | `["inline-patches", "inline-contracts"]` |
 | `patch_actions` | List of individual patch actions | See [patch action fields](#patch-action-fields) below |
@@ -156,7 +153,6 @@ meta_contracts:                          # Meta-contract configurations
 | Field | Description | Example |
 |-------|-------------|---------|
 | `name` | Unique identifier for the contract group | `"usb_control_flow_contracts"` |
-| `id` | Internal identifier for the contract group | `"usb_control_flow_contracts"` |
 | `description` | Description of the contract group purpose | `"USB control flow integrity contracts"` |
 | `contract_actions` | List of individual contract actions | See [contract action fields](#contract-action-fields) below |
 

include/patchestry/YAML/BaseSpec.hpp

@@ -69,15 +69,6 @@ namespace patchestry::passes {
         std::string type;
         std::string description;
     };
-
-    struct Implementation
-    {
-        std::string language;
-        std::string code_file;
-        std::string function_name;
-        std::vector< Parameter > parameters;
-        std::vector< std::string > dependencies;
-    };
 } // namespace patchestry::passes
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(patchestry::passes::VariableMatch)
@@ -132,17 +123,4 @@ namespace llvm::yaml {
             io.mapOptional("description", param.description);
         }
     };
-
-    // Parse Implementation
-    template<>
-    struct MappingTraits< Implementation >
-    {
-        static void mapping(IO &io, Implementation &impl) {
-            io.mapOptional("language", impl.language);
-            io.mapRequired("code_file", impl.code_file);
-            io.mapOptional("function_name", impl.function_name);
-            io.mapRequired("parameters", impl.parameters);
-            io.mapOptional("dependencies", impl.dependencies);
-        }
-    };
 } // namespace llvm::yaml

include/patchestry/YAML/ConfigurationFile.hpp

@@ -154,11 +154,9 @@ namespace patchestry::yaml {
                 }
 
                 // Validate patch file exists if specified
-                if (!patch.implementation.code_file.empty()) {
-                    if (!llvm::sys::fs::exists(patch.implementation.code_file)) {
-                        LOG(ERROR)
-                            << "Patch file does not exist: " << patch.implementation.code_file
-                            << "\n";
+                if (!patch.code_file.empty()) {
+                    if (!llvm::sys::fs::exists(patch.code_file)) {
+                        LOG(ERROR) << "Patch file does not exist: " << patch.code_file << "\n";
                         return false;
                     }
                 }

include/patchestry/YAML/ContractSpec.hpp

@@ -73,18 +73,18 @@ namespace patchestry::passes {
         struct ContractSpec
         {
             std::string name;
-            std::string id;
             std::string description;
             std::string category;
             std::string severity;
-            Implementation implementation;
+            std::string code_file;
+            std::string function_name;
+            std::vector< Parameter > parameters;
             std::optional< std::string > contract_module;
         };
 
         struct MetaContractConfig
         {
             std::string name;
-            std::string id;
             std::string description;
             std::vector< ContractAction > contract_actions;
             std::set< std::string > optimization;
@@ -122,12 +122,12 @@ namespace llvm::yaml {
     {
         static void mapping(IO &io, contract::ContractSpec &spec) {
             io.mapRequired("name", spec.name);
-            io.mapRequired("id", spec.id);
             io.mapOptional("description", spec.description);
             io.mapOptional("category", spec.category);
             io.mapOptional("severity", spec.severity);
-            io.mapRequired("implementation", spec.implementation);
-            io.mapOptional("contract_module", spec.contract_module);
+            io.mapRequired("code_file", spec.code_file);
+            io.mapRequired("function_name", spec.function_name);
+            io.mapOptional("parameters", spec.parameters);
         }
     };
 
@@ -208,7 +208,6 @@ namespace llvm::yaml {
     {
         static void mapping(IO &io, contract::MetaContractConfig &meta_contract) {
             io.mapRequired("name", meta_contract.name);
-            io.mapOptional("id", meta_contract.id);
             io.mapOptional("description", meta_contract.description);
             io.mapRequired("contract_actions", meta_contract.contract_actions);
 

include/patchestry/YAML/PatchSpec.hpp

@@ -65,18 +65,18 @@ namespace patchestry::passes {
         struct PatchSpec
         {
             std::string name;
-            std::string id;
             std::string description;
             std::string category;
             std::string severity;
-            Implementation implementation;
+            std::string code_file;
+            std::string function_name;
+            std::vector< Parameter > parameters;
             std::optional< std::string > patch_module;
         };
 
         struct MetaPatchConfig
         {
             std::string name;
-            std::string id;
             std::string description;
             std::set< std::string > optimization;
             std::vector< PatchAction > patch_actions;
@@ -114,11 +114,12 @@ namespace llvm::yaml {
     {
         static void mapping(IO &io, patch::PatchSpec &spec) {
             io.mapRequired("name", spec.name);
-            io.mapRequired("id", spec.id);
             io.mapOptional("description", spec.description);
             io.mapOptional("category", spec.category);
             io.mapOptional("severity", spec.severity);
-            io.mapRequired("implementation", spec.implementation);
+            io.mapRequired("code_file", spec.code_file);
+            io.mapRequired("function_name", spec.function_name);
+            io.mapOptional("parameters", spec.parameters);
         }
     };
 
@@ -202,7 +203,6 @@ namespace llvm::yaml {
     {
         static void mapping(IO &io, patch::MetaPatchConfig &meta_patch) {
             io.mapRequired("name", meta_patch.name);
-            io.mapRequired("id", meta_patch.id);
             io.mapOptional("description", meta_patch.description);
             io.mapRequired("patch_actions", meta_patch.patch_actions);
 

lib/patchestry/Passes/InstrumentationPass.cpp

@@ -307,12 +307,6 @@ don't yet pass)
      */
     template< typename T >
     std::optional< T > lookup(const std::vector< T > &items, const std::string &name) {
-        for (const auto &item : items) {
-            if (item.id == name) {
-                return item;
-            }
-        }
-        LOG(WARNING) << "Falling back to lookup by name: '" << name << "'\n";
         return lookup_by_name(items, name);
     }
 
@@ -358,14 +352,14 @@ don't yet pass)
         config = std::move(config_or_err.value());
         for (auto &spec : config->libraries.patches) {
             auto patches_file_path =
-                ConfigurationFile::getInstance().resolve_path(spec.implementation.code_file);
+                ConfigurationFile::getInstance().resolve_path(spec.code_file);
             if (!llvm::sys::fs::exists(patches_file_path)) {
                 LOG(ERROR) << "Patch file " << patches_file_path << " does not exist\n";
                 continue;
             }
 
             auto patch_file_path =
-                ConfigurationFile::getInstance().resolve_path(spec.implementation.code_file);
+                ConfigurationFile::getInstance().resolve_path(spec.code_file);
             spec.patch_module = emitModuleAsString(patch_file_path, config->target.arch);
             if (!spec.patch_module) {
                 LOG(ERROR) << "Failed to emit patch module for " << spec.name << "\n";
@@ -374,7 +368,7 @@ don't yet pass)
         }
         for (auto &spec : config->libraries.contracts) {
             auto contracts_file_path =
-                ConfigurationFile::getInstance().resolve_path(spec.implementation.code_file);
+                ConfigurationFile::getInstance().resolve_path(spec.code_file);
             if (!llvm::sys::fs::exists(contracts_file_path)) {
                 LOG(ERROR) << "Contract file " << contracts_file_path << " does not exist\n";
                 continue;
@@ -861,9 +855,6 @@ don't yet pass)
             arg_map[symbol_reference] =
                 create_cast_if_needed(builder, call_op, load_op, patch_arg_type);
         }
-
-        // LOG(DEBUG) << "arg_map[symbol] key=" << valueToString(symbol_reference)
-        //            << " value=" << valueToString(arg_map[symbol_reference]) << "\n";
     }
 
     void InstrumentationPass::handle_return_value_argument(
@@ -909,9 +900,6 @@ don't yet pass)
         }
 
         arg_map[arg_value] = create_cast_if_needed(builder, call_op, arg_value, patch_arg_type);
-
-        // LOG(DEBUG) << "arg_map[constant] key=" << valueToString(arg_value)
-        //        << " value=" << valueToString(arg_map[arg_value]) << "\n";
     }
 
     mlir::Value InstrumentationPass::parse_constant_operand(
@@ -1255,11 +1243,11 @@ don't yet pass)
         builder.setInsertionPoint(target_op);
         auto module = target_op->getParentOfType< mlir::ModuleOp >();
 
-        std::string patch_function_name =
-            namifyFunction(patch_spec.implementation.function_name);
+        std::string patch_function_name = namifyFunction(patch_spec.function_name);
         auto input_types = llvm::to_vector(target_op->getOperandTypes());
         if (!patch_module.lookupSymbol< cir::FuncOp >(patch_function_name)) {
-            LOG(ERROR) << "Patch module not found or patch function not defined\n";
+            LOG(ERROR) << "Patch module not found or patch function not defined: "
+                       << patch_function_name << "\n";
             return;
         }
 
@@ -1331,11 +1319,11 @@ don't yet pass)
         auto module = target_op->getParentOfType< mlir::ModuleOp >();
         builder.setInsertionPointAfter(target_op);
 
-        std::string patch_function_name =
-            namifyFunction(patch_spec.implementation.function_name);
+        std::string patch_function_name = namifyFunction(patch_spec.function_name);
         auto input_types = llvm::to_vector(target_op->getResultTypes());
         if (!patch_module.lookupSymbol< cir::FuncOp >(patch_function_name)) {
-            LOG(ERROR) << "Patch module not found or patch function not defined\n";
+            LOG(ERROR) << "Patch module not found or patch function not defined: "
+                       << patch_function_name << "\n";
             return;
         }
 
@@ -1409,11 +1397,12 @@ don't yet pass)
         auto callee_name = call_op.getCallee()->str();
         assert(!callee_name.empty() && "Wrap around patch: callee name is empty");
 
-        auto patch_function_name = namifyFunction(patch_spec.implementation.function_name);
+        auto patch_function_name = namifyFunction(patch_spec.function_name);
         auto result_types        = llvm::to_vector(call_op.getResultTypes());
 
         if (!patch_module.lookupSymbol< cir::FuncOp >(patch_function_name)) {
-            LOG(ERROR) << "Patch module not found or patch function not defined\n";
+            LOG(ERROR) << "Patch module not found or patch function not defined: "
+                       << patch_function_name << "\n";
             return;
         }
 
@@ -1431,8 +1420,7 @@ don't yet pass)
 
         auto wrap_func = module.lookupSymbol< cir::FuncOp >(patch_function_name);
         if (!wrap_func) {
-            LOG(ERROR) << "Wrap around patch: patch function "
-                       << patch_spec.implementation.function_name
+            LOG(ERROR) << "Wrap around patch: patch function " << patch_function_name
                        << " not defined. Patching failed...\n";
             return;
         }
@@ -1953,7 +1941,7 @@ don't yet pass)
         builder.setInsertionPoint(target_op);
         auto module = target_op->getParentOfType< mlir::ModuleOp >();
 
-        std::string contract_function_name = namifyFunction(spec.implementation.function_name);
+        std::string contract_function_name = namifyFunction(spec.function_name);
         auto input_types                   = llvm::to_vector(target_op->getOperandTypes());
         if (!contract_module.lookupSymbol< cir::FuncOp >(contract_function_name)) {
             LOG(ERROR) << "Contract module not found or contract function not defined: "
@@ -2018,10 +2006,11 @@ don't yet pass)
         auto module = target_op->getParentOfType< mlir::ModuleOp >();
         builder.setInsertionPointAfter(target_op);
 
-        std::string contract_function_name = namifyFunction(spec.implementation.function_name);
+        std::string contract_function_name = namifyFunction(spec.function_name);
         auto input_types                   = llvm::to_vector(target_op->getResultTypes());
         if (!contract_module.lookupSymbol< cir::FuncOp >(contract_function_name)) {
-            LOG(ERROR) << "Contract module not found or contract function not defined\n";
+            LOG(ERROR) << "Contract module not found or contract function not defined: "
+                       << contract_function_name << "\n";
             return;
         }
 
@@ -2077,10 +2066,10 @@ don't yet pass)
         }
 
         const auto &contract_spec = contract.spec.value();
-        std::string contract_function_name =
-            namifyFunction(contract_spec.implementation.function_name);
+        std::string contract_function_name = namifyFunction(contract_spec.function_name);
         if (!contract_module.lookupSymbol< cir::FuncOp >(contract_function_name)) {
-            LOG(ERROR) << "Contract module not found or contract function not defined\n";
+            LOG(ERROR) << "Contract module not found or contract function not defined: "
+                       << contract_function_name << "\n";
             return;
         }
 

test/patchir-transform/bl_led_loop_before_cmp_operation.yaml

@@ -16,16 +16,15 @@ libraries:
   - "patches/led_loop_before_cmp_operation.yaml"
 
 execution_order:
-  - "meta_patches::LED-LOOP-BEFORE-CMP-OPERATION"
+  - "meta_patches::led_loop_before_cmp_operation"
 
 
 meta_patches:
   - name: "led_loop_before_cmp_operation"
-    id: "LED-LOOP-BEFORE-CMP-OPERATION"
     description: "Apply before cmp operation"
 
     patch_actions:
-      - id: "LED-LOOP-BEFORE-CMP-OPERATION"
+      - id: "led_loop_before_cmp_operation"
         description: "Apply before cmp operation"
 
         # Match Configuration
@@ -39,7 +38,7 @@ meta_patches:
         # Actions to Apply
         action:
           - mode: "apply_before"
-            patch_id: "LED-LOOP-BEFORE-CMP-OPERATION-001"
+            patch_id: "led_loop_before_cmp_operation"
             description: "Apply before cmp operation"
             arguments:
               - name: "global_var"

test/patchir-transform/bl_usb__send_message_after_patch.yaml

@@ -15,15 +15,13 @@ target:
 libraries:
  # these references are in relation to the location of this file
   - "patches/usb_security_patches.yaml"
-#  contracts: "contracts/usb_security_contracts.yaml"
 
 execution_order:
   - "meta_patches::usb_security_meta_patches"
 #  - "meta_contracts::usb_security_meta_contracts"
 
 meta_patches:
   - name: usb_security_meta_patches
-    id: "usb_security_meta_patches"
     description: "Meta patches for USB security"
 
     # Patch Action Configuration
@@ -45,7 +43,7 @@ meta_patches:
          # Actions to Apply
          action:
           - mode: "apply_after"
-            patch_id: "USB-PATCH-AFTER-001"
+            patch_id: "usb_endpoint_write_validation_after"
             description: "Checks function return value"
             arguments:
               - name: "function_return_value"