Merge pull request #67 from fabiosky/wip/fabiosky/rgb_bgra

Avoid reading invalid data for rgb to bgra conversion

.github/workflows/ci.yml

@@ -123,10 +123,33 @@ jobs:
       - name: Test
         run: wasm-pack test --node
 
+  sanitizer:
+    name: Sanitizer
+    runs-on: ubuntu-latest
+    needs: lint
+
+    steps:
+      - uses: actions/checkout@v2
+
+      - uses: actions-rs/toolchain@v1
+        with:
+          toolchain: nightly
+          target: x86_64-unknown-linux-gnu
+
+      - name: Sanitizer
+        run: cargo +nightly test --verbose --tests --target=x86_64-unknown-linux-gnu
+        env:
+          RUSTFLAGS: -Zsanitizer=address
+
+      - name: Sanitizer (instruction sets)
+        run: cargo +nightly test --verbose --tests --target=x86_64-unknown-linux-gnu --features "test_instruction_sets"
+        env:
+          RUSTFLAGS: -Zsanitizer=address
+
   coverage:
     name: Coverage
     runs-on: ubuntu-latest
-    needs: build
+    needs: sanitizer
 
     steps:
       - uses: actions/checkout@v2

src/convert_image/x86.rs

@@ -666,37 +666,41 @@ pub fn rgb_to_bgra(
     let src_stride_diff = src_stride - (SRC_DEPTH * width);
     let dst_stride_diff = dst_stride - (DST_DEPTH * width);
 
-    // For single swap iteration, since the swap is done on 32 bits while the input is only
-    // 24 bits (RGB), to avoid reading an extra byte of memory that could be outside the
-    // boundaries of the buffer it is necessary to check if the there is at least one byte
-    // of stride in the input buffer, in that case we can read till the end of the buffer.
-    let single_swap_iterations = if src_stride_diff < 1 {
-        width - 1
-    } else {
-        width
-    };
-
-    // For multiple swap iteration, since each swap reads two extra bytes it is necessary
-    // to check if there is enough space to read them without going out of boundaries.
-    // Since each step retrieves items_per_iteration colors, it is checked if the width of
-    // the image is a multiple of items_per_iteration,
-    // in that case it is checked if there are 2 extra bytes of stride, if there is not
-    // enough space then the number of multiple swap iterarions is reduced by items_per_iteration
-    // since it is not safe to read till the end of the buffer, otherwise it is not.
-    let multi_swap_iterations =
-        if ITEMS_PER_ITERATION * (width / ITEMS_PER_ITERATION) == width && src_stride_diff < 2 {
-            ITEMS_PER_ITERATION * ((width - 1) / ITEMS_PER_ITERATION)
-        } else {
-            ITEMS_PER_ITERATION * (width / ITEMS_PER_ITERATION)
-        };
-
     unsafe {
         let src_group = src_buffer.as_ptr();
         let dst_group = dst_buffer.as_mut_ptr();
         let mut src_offset = 0;
         let mut dst_offset = 0;
 
-        for _ in 0..height {
+        for y in 0..height {
+            let is_last_line = y == height - 1;
+
+            // For single swap iteration, since the swap is done on 32 bits while the input is only
+            // 24 bits (RGB), to avoid reading an extra byte of memory that could be outside the
+            // boundaries of the buffer it is necessary to check if the there is at least one byte
+            // of stride in the input buffer, in that case we can read till the end of the buffer.
+            let single_swap_iterations = if is_last_line || src_stride_diff < 1 {
+                width - 1
+            } else {
+                width
+            };
+
+            // For multiple swap iteration, since each swap reads two extra bytes it is necessary
+            // to check if there is enough space to read them without going out of boundaries.
+            // Since each step retrieves items_per_iteration colors, it is checked if the width of
+            // the image is a multiple of items_per_iteration,
+            // in that case it is checked if there are 2 extra bytes of stride, if there is not
+            // enough space then the number of multiple swap iterarions is reduced by items_per_iteration
+            // since it is not safe to read till the end of the buffer, otherwise it is not.
+            let multi_swap_iterations = if is_last_line
+                || (ITEMS_PER_ITERATION * (width / ITEMS_PER_ITERATION) == width
+                    && src_stride_diff < 2)
+            {
+                ITEMS_PER_ITERATION * ((width - 1) / ITEMS_PER_ITERATION)
+            } else {
+                ITEMS_PER_ITERATION * (width / ITEMS_PER_ITERATION)
+            };
+
             let mut x = 0;
 
             // Retrieves items_per_iteration colors per cycle if possible

tests/tests.rs

@@ -1764,7 +1764,7 @@ fn over_4gb() {
         let dst_layout = Layout::from_size_align_unchecked(EXPECTED_DST_BUFFER_SIZE, 1);
         let dst_ptr = alloc_zeroed(dst_layout);
         if !dst_ptr.is_null() {
-            let src_image = from_raw_parts_mut(src_ptr, EXPECTED_SRC_BUFFER_SIZE);
+            let src_image: &[u8] = from_raw_parts_mut(src_ptr, EXPECTED_SRC_BUFFER_SIZE);
             let dst_image = from_raw_parts_mut(dst_ptr, EXPECTED_DST_BUFFER_SIZE);
 
             // Touch output
@@ -1774,7 +1774,7 @@ fn over_4gb() {
 
             dst_image[dst_image.len() - 1] = 0;
 
-            let src_buffers = &[&src_image[..]];
+            let src_buffers = &[src_image];
             let dst_buffers = &mut [&mut *dst_image];
 
             assert!(convert_image(