⚡️ Optimize LibBytes.split null path and dynamicStructInCalldata bounds check fixes

src/utils/LibBytes.sol

@@ -545,11 +545,11 @@ library LibBytes {
     {
         /// @solidity memory-safe-assembly
         assembly {
+            result := mload(0x40)
+            let o := add(result, 0x20)
             let searchLen := mload(needle)
             if iszero(gt(searchLen, mload(subject))) {
-                result := mload(0x40)
                 let i := add(subject, 0x20)
-                let o := add(result, 0x20)
                 let subjectSearchEnd := add(sub(add(i, mload(subject)), searchLen), 1)
                 let h := 0 // The hash of `needle`.
                 if iszero(lt(searchLen, 0x20)) {
@@ -578,11 +578,11 @@ library LibBytes {
                     i := add(i, 1)
                     if iszero(lt(i, subjectSearchEnd)) { break }
                 }
-                mstore(result, shr(5, sub(o, add(result, 0x20)))) // Store the length of `result`.
-                // Allocate memory for result.
-                // We allocate one more word, so this array can be recycled for {split}.
-                mstore(0x40, add(o, 0x20))
             }
+            mstore(result, shr(5, sub(o, add(result, 0x20)))) // Store the length of `result`.
+            // Allocate memory for result.
+            // We allocate one more word, so this array can be recycled for {split}.
+            mstore(0x40, add(o, 0x20))
         }
     }
 
@@ -811,10 +811,15 @@ library LibBytes {
         /// @solidity memory-safe-assembly
         assembly {
             let l := sub(a.length, 0x20)
-            let s := calldataload(add(a.offset, offset)) // Relative offset of `result` from `a.offset`.
-            result.offset := add(a.offset, s)
-            result.length := sub(a.length, s)
-            if or(shr(64, or(s, or(l, a.offset))), gt(offset, l)) { revert(l, 0x00) }
+            // Relative offset of `result` from `a.offset`. Reuses `result.length` to save a stack slot.
+            result.length := calldataload(add(a.offset, offset))
+            result.offset := add(a.offset, result.length)
+            // Wraps if the relative offset exceeds `a.length`, caught below.
+            result.length := sub(a.length, result.length)
+            // `sub(result.offset, a.offset)` recovers the relative offset, even if the add wrapped.
+            // forgefmt: disable-next-item
+            if or(shr(64, or(result.length, or(sub(result.offset, a.offset), or(l, a.offset)))),
+                gt(offset, l)) { revert(l, 0x00) }
         }
     }
 

src/utils/g/LibBytes.sol

@@ -549,11 +549,11 @@ library LibBytes {
     {
         /// @solidity memory-safe-assembly
         assembly {
+            result := mload(0x40)
+            let o := add(result, 0x20)
             let searchLen := mload(needle)
             if iszero(gt(searchLen, mload(subject))) {
-                result := mload(0x40)
                 let i := add(subject, 0x20)
-                let o := add(result, 0x20)
                 let subjectSearchEnd := add(sub(add(i, mload(subject)), searchLen), 1)
                 let h := 0 // The hash of `needle`.
                 if iszero(lt(searchLen, 0x20)) {
@@ -582,11 +582,11 @@ library LibBytes {
                     i := add(i, 1)
                     if iszero(lt(i, subjectSearchEnd)) { break }
                 }
-                mstore(result, shr(5, sub(o, add(result, 0x20)))) // Store the length of `result`.
-                // Allocate memory for result.
-                // We allocate one more word, so this array can be recycled for {split}.
-                mstore(0x40, add(o, 0x20))
             }
+            mstore(result, shr(5, sub(o, add(result, 0x20)))) // Store the length of `result`.
+            // Allocate memory for result.
+            // We allocate one more word, so this array can be recycled for {split}.
+            mstore(0x40, add(o, 0x20))
         }
     }
 
@@ -815,10 +815,15 @@ library LibBytes {
         /// @solidity memory-safe-assembly
         assembly {
             let l := sub(a.length, 0x20)
-            let s := calldataload(add(a.offset, offset)) // Relative offset of `result` from `a.offset`.
-            result.offset := add(a.offset, s)
-            result.length := sub(a.length, s)
-            if or(shr(64, or(s, or(l, a.offset))), gt(offset, l)) { revert(l, 0x00) }
+            // Relative offset of `result` from `a.offset`. Reuses `result.length` to save a stack slot.
+            result.length := calldataload(add(a.offset, offset))
+            result.offset := add(a.offset, result.length)
+            // Wraps if the relative offset exceeds `a.length`, caught below.
+            result.length := sub(a.length, result.length)
+            // `sub(result.offset, a.offset)` recovers the relative offset, even if the add wrapped.
+            // forgefmt: disable-next-item
+            if or(shr(64, or(result.length, or(sub(result.offset, a.offset), or(l, a.offset)))),
+                gt(offset, l)) { revert(l, 0x00) }
         }
     }
 

test/LibBytes.t.sol

@@ -3,6 +3,7 @@ pragma solidity ^0.8.4;
 
 import "./utils/SoladyTest.sol";
 import {LibBytes} from "../src/utils/LibBytes.sol";
+import {EfficientHashLib} from "../src/utils/EfficientHashLib.sol";
 
 contract BytesStore {
     LibBytes.BytesStorage internal value;
@@ -293,6 +294,73 @@ contract LibBytesTest is SoladyTest {
         assertEq(LibBytes.bytesInCalldata(p, 0x40), expected.data);
     }
 
+    function testDynamicStructInCalldataBounds(uint256 s, uint256 n) public {
+        n = (n % 5) << 5;
+        if (_randomChance(2)) s = s % (n + 0x60);
+        bytes memory a = abi.encodePacked(s, new bytes(n));
+        (bool ok, bytes memory r) =
+            address(this).call(abi.encodeWithSignature("_dynamicStructInCalldataBounds(bytes)", a));
+        assertEq(ok, s <= a.length);
+        if (ok) {
+            (uint256 start, uint256 end) = abi.decode(r, (uint256, uint256));
+            assertEq(start, s);
+            assertEq(end, a.length);
+        }
+    }
+
+    function testDynamicStructInCalldataMalformedOffset() public {
+        // 64 bytes, with the struct offset word set to 65 (> `a.length`).
+        bytes memory a = abi.encode(uint256(65), uint256(0));
+        (bool ok, bytes memory reason) =
+            address(this).call(abi.encodeWithSignature("_dynamicStructInCalldataBounds(bytes)", a));
+        assertFalse(ok);
+        assertEq(reason, "");
+    }
+
+    function testDynamicStructInCalldataValid() public {
+        (uint256 start, uint256 end) =
+            this._dynamicStructInCalldataBounds(abi.encode(uint256(0x20), uint256(123)));
+        assertEq(start, 0x20);
+        assertEq(end, 0x40);
+    }
+
+    function _dynamicStructInCalldataBounds(bytes calldata a)
+        public
+        pure
+        returns (uint256 start, uint256 end)
+    {
+        bytes calldata p = LibBytes.dynamicStructInCalldata(a, 0);
+        /// @solidity memory-safe-assembly
+        assembly {
+            start := sub(p.offset, a.offset)
+            end := add(start, p.length)
+        }
+    }
+
+    function testSplitNullPathCleanScratch() public {
+        bytes[] memory p = LibBytes.split("ab", "xyz");
+        assertEq(p.length, 1);
+        assertEq(p[0], "ab");
+    }
+
+    function testSplitNullPathAfterHash() public {
+        // Dirties the scratch space before the split.
+        EfficientHashLib.hash(uint256(1));
+        bytes[] memory p = LibBytes.split("ab", "xyz");
+        assertEq(p.length, 1);
+        assertEq(p[0], "ab");
+    }
+
+    function testSplitNullPathTwoConsecutive() public {
+        // The first split leaves the scratch space dirty for the second.
+        bytes[] memory a = LibBytes.split("ab", "xyz");
+        bytes[] memory b = LibBytes.split("cd", "wxyz");
+        assertEq(a.length, 1);
+        assertEq(a[0], "ab");
+        assertEq(b.length, 1);
+        assertEq(b[0], "cd");
+    }
+
     function testBytesInCalldata() public {
         this._testBytesInCalldata(abi.encode("hello"), 0x00, "hello");
     }

test/LibString.t.sol

@@ -1037,6 +1037,29 @@ contract LibStringTest is SoladyTest {
         assertTrue(_stringArraysAreSame(LibString.split("", ""), elements));
     }
 
+    function testStringSplitDelimiterLongerThanSubject(uint256 dirty) public brutalizeMemory {
+        string[] memory elements = new string[](1);
+        elements[0] = "ab";
+        for (uint256 t; t < 3; ++t) {
+            uint256 w = [uint256(0), 1, dirty][t];
+            /// @solidity memory-safe-assembly
+            assembly {
+                mstore(0x00, w)
+                mstore(0x20, not(w))
+            }
+            _misalignFreeMemoryPointer();
+            string[] memory split = LibString.split("ab", "xyz");
+            /// @solidity memory-safe-assembly
+            assembly {
+                mstore(0x00, not(0))
+                mstore(0x20, not(0))
+            }
+            assertTrue(_stringArraysAreSame(split, elements));
+            _checkMemory(split[0]);
+        }
+        assertEq(LibString.indicesOf("ab", "xyz").length, 0);
+    }
+
     function testStringConcat(string memory a, string memory b) public brutalizeMemory {
         string memory concatenated = LibString.concat(a, b);
         _checkMemory(concatenated);