Remove debug asserts in tests [Rebase & FF[

Cargo.toml

@@ -82,7 +82,7 @@ opt-level = 3
 [profile.test]
 opt-level = 0
 debug = true
-debug-assertions = true
+debug-assertions = false
 overflow-checks = true
 lto = false
 incremental = false

core/patina_internal_depex/src/lib.rs

@@ -442,21 +442,18 @@ mod tests {
     }
 
     #[test]
-    #[should_panic(expected = "Invalid SOR not at start of depex")]
     fn sor_not_first_opcode_should_eval_false() {
         let mut depex = Depex::from(vec![0x06, 0x09, 0x08]);
         assert!(!depex.eval(&[]));
     }
 
     #[test]
-    #[should_panic(expected = "Exiting early due to an unknown opcode.")]
     fn replacetrue_should_eval_false() {
         let mut depex = Depex::from(vec![0xFF, 0x08]);
         assert!(!depex.eval(&[]));
     }
 
     #[test]
-    #[should_panic(expected = "Exiting early due to an unknown opcode.")]
     fn unknown_opcode_should_return_false() {
         let mut depex = Depex::from(vec![0xE0, 0x08]);
         assert!(!depex.eval(&[]));
@@ -675,39 +672,34 @@ mod tests {
     }
 
     #[test]
-    #[should_panic(expected = "Invalid BEFORE or AFTER not at start of depex")]
     fn opcode_before_should_panic_when_not_at_start_of_depex() {
         let opcodes = [Opcode::And, Opcode::Before(Uuid::from_str("76b6bdfa-2acd-4462-9e3f-cb58c969d937").unwrap())];
         let mut depex = Depex::from(opcodes.as_slice());
         depex.eval(&[]);
     }
 
     #[test]
-    #[should_panic(expected = "Invalid BEFORE or AFTER not at start of depex")]
     fn opcode_after_should_panic_when_not_at_start_of_depex() {
         let opcodes = [Opcode::And, Opcode::After(Uuid::from_str("76b6bdfa-2acd-4462-9e3f-cb58c969d937").unwrap())];
         let mut depex = Depex::from(opcodes.as_slice());
         depex.eval(&[]);
     }
 
     #[test]
-    #[should_panic(expected = "Invalid BEFORE or AFTER with additional opcodes")]
     fn opcode_before_should_panic_when_final_opcode_is_not_end() {
         let opcodes = [Opcode::Before(Uuid::from_str("76b6bdfa-2acd-4462-9e3f-cb58c969d937").unwrap()), Opcode::And];
         let mut depex = Depex::from(opcodes.as_slice());
         depex.eval(&[]);
     }
 
     #[test]
-    #[should_panic(expected = "Invalid BEFORE or AFTER with additional opcodes")]
     fn opcode_after_should_panic_when_final_opcode_is_not_end() {
         let opcodes = [Opcode::After(Uuid::from_str("76b6bdfa-2acd-4462-9e3f-cb58c969d937").unwrap()), Opcode::And];
         let mut depex = Depex::from(opcodes.as_slice());
         depex.eval(&[]);
     }
 
     #[test]
-    #[should_panic(expected = "Invalid BEFORE or AFTER with additional opcodes")]
     fn opcode_before_should_panic_when_additional_opcodes_after() {
         let opcodes =
             [Opcode::Before(Uuid::from_str("76b6bdfa-2acd-4462-9e3f-cb58c969d937").unwrap()), Opcode::And, Opcode::End];
@@ -716,7 +708,6 @@ mod tests {
     }
 
     #[test]
-    #[should_panic(expected = "Invalid BEFORE or AFTER with additional opcodes")]
     fn opcode_after_should_panic_when_additional_opcodes_after() {
         let opcodes =
             [Opcode::After(Uuid::from_str("76b6bdfa-2acd-4462-9e3f-cb58c969d937").unwrap()), Opcode::And, Opcode::End];
@@ -725,7 +716,6 @@ mod tests {
     }
 
     #[test]
-    #[should_panic(expected = "Exiting early because opcode [0x0] expects a guid, only has a length of: 0")]
     fn malformed_opcode_should_panic_with_well_defined_message() {
         let opcodes = [Opcode::Malformed { opcode: 0x00, len: 0 }];
         let mut depex = Depex::from(opcodes.as_slice());

patina_dxe_core/src/allocator.rs

@@ -984,53 +984,53 @@ fn process_hob_allocations(hob_list: &HobList) {
         let stack_address = stack_hob.memory_base_address;
         let stack_length = stack_hob.memory_length;
 
-        if (stack_address == 0) || (stack_length == 0) {
-            log::error!("Stack base address {:#X} for len {:#X}", stack_address, stack_length);
-            debug_assert!(false);
-        } else {
-            match GCD.get_memory_descriptor_for_address(stack_address) {
-                Ok(gcd_desc) => {
-                    // Set Stack region to execute protect. We use the allocated memory protection policy here because
-                    // that matches our standard policy
-                    let attributes =
-                        GCD.memory_protection_policy.apply_allocated_memory_protection_policy(gcd_desc.attributes);
-                    match GCD.set_memory_space_attributes(stack_address as usize, stack_length as usize, attributes) {
-                        Ok(_) | Err(EfiError::NotReady) => (),
-                        Err(e) => {
-                            log::error!(
-                                "Could not set NX for memory address {:#X} for len {:#X} with error {:?}",
-                                stack_address,
-                                stack_length,
-                                e
-                            );
-                            debug_assert!(false);
-                        }
-                    }
-                    // Set Guard page to read protect. We keep the NX and cache attributes from above
-                    match GCD.set_memory_space_attributes(
-                        stack_address as usize,
-                        UEFI_PAGE_SIZE,
-                        MemoryProtectionPolicy::apply_image_stack_guard_policy(attributes),
-                    ) {
-                        Ok(_) | Err(EfiError::NotReady) => (),
-                        Err(e) => {
-                            log::error!(
-                                "Could not set RP for memory address {:#X} for len {:#X} with error {:?}",
-                                stack_address,
-                                UEFI_PAGE_SIZE,
-                                e
-                            );
-                            debug_assert!(false);
-                        }
+        assert!(
+            stack_address != 0 && stack_length != 0,
+            "Invalid Stack Configuration: Stack base address {stack_address:#X} for len {stack_length:#X}"
+        );
+
+        match GCD.get_memory_descriptor_for_address(stack_address) {
+            Ok(gcd_desc) => {
+                // Set Stack region to execute protect. We use the allocated memory protection policy here because
+                // that matches our standard policy
+                let attributes =
+                    GCD.memory_protection_policy.apply_allocated_memory_protection_policy(gcd_desc.attributes);
+                match GCD.set_memory_space_attributes(stack_address as usize, stack_length as usize, attributes) {
+                    Ok(_) | Err(EfiError::NotReady) => (),
+                    Err(e) => {
+                        log::error!(
+                            "Could not set NX for memory address {:#X} for len {:#X} with error {:?}",
+                            stack_address,
+                            stack_length,
+                            e
+                        );
+                        debug_assert!(false);
                     }
                 }
-                Err(_) => {
-                    log::error!("Failed to get memory descriptor for address {:#x?} in GCD", stack_address);
+                // Set Guard page to read protect. We keep the NX and cache attributes from above
+                match GCD.set_memory_space_attributes(
+                    stack_address as usize,
+                    UEFI_PAGE_SIZE,
+                    MemoryProtectionPolicy::apply_image_stack_guard_policy(attributes),
+                ) {
+                    Ok(_) | Err(EfiError::NotReady) => (),
+                    Err(e) => {
+                        log::error!(
+                            "Could not set RP for memory address {:#X} for len {:#X} with error {:?}",
+                            stack_address,
+                            UEFI_PAGE_SIZE,
+                            e
+                        );
+                        debug_assert!(false);
+                    }
                 }
             }
+            Err(_) => {
+                log::error!("Failed to get memory descriptor for address {:#x?} in GCD", stack_address);
+            }
         }
     } else {
-        debug_assert!(false, "No stack hob found\n");
+        panic!("No stack hob found");
     }
 
     // now that we've processed HOBs, lets allocate page 0 because we are going to use it for null pointer detection
@@ -1257,6 +1257,49 @@ mod tests {
         .unwrap();
     }
 
+    #[test]
+    fn process_hob_allocations_should_handle_stack_attribute_set_failure() {
+        // A stack HOB is created but the corresponding memory region is not added
+        // to the GCD. This should cause set_memory_space_attributes to fail with NotFound.
+        test_support::with_global_lock(|| {
+            let physical_hob_list = build_test_hob_list(0x1000000);
+            unsafe {
+                GCD.reset();
+                gcd::init_gcd(physical_hob_list);
+                test_support::init_test_protocol_db();
+                reset_allocators();
+            }
+
+            let mut hob_list = HobList::default();
+            hob_list.discover_hobs(physical_hob_list);
+
+            // Create a stack HOB at an address NOT in the GCD (such as 0xEB000)
+            let stack_base_address = 0xEB000;
+            let stack_pages = 0x20;
+
+            let stack_hob = Hob::MemoryAllocation(&patina::pi::hob::MemoryAllocation {
+                header: patina::pi::hob::header::Hob {
+                    r#type: hob::MEMORY_ALLOCATION,
+                    length: core::mem::size_of::<hob::MemoryAllocation>() as u16,
+                    reserved: 0x00000000,
+                },
+                alloc_descriptor: patina::pi::hob::header::MemoryAllocation {
+                    name: HOB_MEMORY_ALLOC_STACK,
+                    memory_base_address: stack_base_address,
+                    memory_length: stack_pages * UEFI_PAGE_SIZE as u64,
+                    memory_type: efi::BOOT_SERVICES_DATA,
+                    reserved: Default::default(),
+                },
+            });
+            hob_list.push(stack_hob);
+
+            // This should fail to set attributes on the stack because the address
+            // is not in the GCD, but should continue processing without panicking
+            process_hob_allocations(&hob_list);
+        })
+        .unwrap();
+    }
+
     #[test]
     fn init_memory_support_should_process_resource_allocations() {
         test_support::with_global_lock(|| {

patina_dxe_core/src/component_dispatcher.rs

@@ -450,8 +450,7 @@ mod tests {
     }
 
     #[test]
-    #[should_panic]
-    fn test_error_in_component_debug_asserts() {
+    fn test_dispatch_still_succeeds_with_error_in_component() {
         #[derive(patina::component::IntoComponent)]
         struct TestComponent;
 
@@ -463,6 +462,6 @@ mod tests {
 
         let mut dispatcher = ComponentDispatcher::default();
         dispatcher.insert_component(0, TestComponent.into_component());
-        dispatcher.dispatch();
+        assert!(dispatcher.dispatch(), "Dispatch should succeed even if component fails");
     }
 }

patina_dxe_core/src/image.rs

@@ -593,6 +593,7 @@ fn core_load_pe_image(
                         resource_section_size
                     );
                     debug_assert!(false);
+                    return Err(EfiError::LoadError);
                 }
             }
         }
@@ -2410,11 +2411,8 @@ mod tests {
         //
         // We craft a malformed PE image with a section virtual_size that will cause
         // align_up() to overflow when aligning to section_alignment.
-        //
-        // Note: In debug builds, this hits a debug_assert!(false) which panics.
-        // The panic is caught by with_global_lock(), so we check for that.
 
-        let result = test_support::with_global_lock(|| {
+        test_support::with_global_lock(|| {
             // SAFETY: These test initialization functions require unsafe because they
             // manipulate global state (GCD, protocol DB, system table)
             unsafe {
@@ -2450,27 +2448,12 @@ mod tests {
             image[virtual_size_offset..virtual_size_offset + 4].copy_from_slice(&overflow_value.to_le_bytes());
 
             // Try to load the malformed image by calling core_load_pe_image directly
-            // (bypassing the FFI extern "efiapi" fn load_image which cannot unwind)
             let image_info = empty_image_info();
             let result = super::core_load_pe_image(&image, image_info);
 
-            // The load should FAIL due to alignment overflow
-            // This verifies that the error from apply_image_memory_protections
-            // is properly propagated (Fix #176)
-            match result {
-                Err(EfiError::LoadError) => { /* Expected error */ }
-                Err(e) => panic!("Expected LoadError from alignment overflow, got {:?}", e),
-                Ok(_) => panic!("Image with overflowing section alignment should fail to load"),
-            }
-        });
-
-        // In debug builds, debug_assert!(false) panics and with_global_lock catches it
-        #[cfg(debug_assertions)]
-        assert!(result.is_err(), "Expected panic from debug_assert! in debug build");
-
-        // In release builds, debug_assert is compiled away and function returns error normally
-        #[cfg(not(debug_assertions))]
-        assert!(result.is_ok(), "Expected successful test execution in release build");
+            assert!(matches!(result, Err(EfiError::LoadError)), "Expected LoadError from alignment overflow");
+        })
+        .unwrap();
     }
 
     #[test]
@@ -2488,7 +2471,7 @@ mod tests {
         // the debug_assert!(false) panic path. In release mode, this particular error
         // case doesn't result in a load failure.
 
-        let result = test_support::with_global_lock(|| {
+        test_support::with_global_lock(|| {
             // SAFETY: These test initialization functions require unsafe because they
             // manipulate global state (GCD, protocol DB, system table)
             unsafe {
@@ -2524,14 +2507,11 @@ mod tests {
             // The load should FAIL because when apply_image_memory_protections calculates
             // section_base_addr = image_base + 0x1001, the address will be unaligned.
             // set_memory_space_attributes will check (base_address & 0xFFF) == 0 and fail.
-            match result {
-                Err(EfiError::InvalidParameter) => { /* Expected error */ }
-                Err(e) => panic!("Expected InvalidParameter from unaligned address, got {:?}", e),
-                Ok(_) => panic!("Image with unaligned section address should fail to load"),
-            }
-        });
-
-        // Verify that the panic was caught by with_global_lock
-        assert!(result.is_err(), "Expected panic from debug_assert!");
+            assert!(
+                matches!(result, Err(EfiError::InvalidParameter)),
+                "Expected InvalidParameter from unaligned section address"
+            );
+        })
+        .unwrap();
     }
 }

sdk/patina/src/component.rs

@@ -48,7 +48,8 @@
 //!
 //! Note: Certain combinations of parameters may lead to undefined behavior as they can allow multiple mutable
 //! accesses to the same data. Each parameter type checks for conflicts with previously registered accesses, but
-//! **ONLY** on debug builds. In release builds, these checks are disabled for performance and size reasons.
+//! **ONLY** on debug builds (and not during testing). In release builds, these checks are disabled for performance
+//! and size reasons. In test builds, they are disabled to focus testing on functionality rather than debug assertions.
 //!
 //! ### `Param` types
 //!