Merge pull request #1789 from hathach/fix-fifo-memory-overflow

Fix fifo memory overflow

2 files changed, 119 insertions, 47 deletions

diff --git a/test/unit-test/project.yml b/test/unit-test/project.yml
index 7708123d5..562dbca09 100644
--- a/test/unit-test/project.yml
+++ b/test/unit-test/project.yml
@@ -78,10 +78,24 @@
     :html_high_threshold: 90
     :xml_report: FALSE
 
-#:tools:
-# Ceedling defaults to using gcc for compiling, linking, etc.
-# As [:tools] is blank, gcc will be used (so long as it's in your system path)
-# See documentation to configure a given toolchain for use
+:tools:
+  :test_compiler:
+     :executable: clang
+     :name: 'clang compiler'
+     :arguments:
+        - -I"$": COLLECTION_PATHS_TEST_TOOLCHAIN_INCLUDE               #expands to -I search paths
+        - -I"$": COLLECTION_PATHS_TEST_SUPPORT_SOURCE_INCLUDE_VENDOR   #expands to -I search paths
+        - -D$: COLLECTION_DEFINES_TEST_AND_VENDOR  #expands to all -D defined symbols
+        - -fsanitize=address
+        - -c ${1}                       #source code input file (Ruby method call param list sub)
+        - -o ${2}                       #object file output (Ruby method call param list sub)
+  :test_linker:
+     :executable: clang
+     :name: 'clang linker'
+     :arguments:
+        - -fsanitize=address
+        - ${1}               #list of object files to link (Ruby method call param list sub)
+        - -o ${2}            #executable file output (Ruby method call param list sub)
 
 # LIBRARIES
 # These libraries are automatically injected into the build process. Those specified as
diff --git a/test/unit-test/test/test_fifo.c b/test/unit-test/test/test_fifo.c
index 1d87081bb..28be6d8fc 100644
--- a/test/unit-test/test/test_fifo.c
+++ b/test/unit-test/test/test_fifo.c
@@ -30,15 +30,23 @@
 #include "osal/osal.h"
 #include "tusb_fifo.h"
 
-#define FIFO_SIZE 10
-TU_FIFO_DEF(tu_ff, FIFO_SIZE, uint8_t, false);
+#define FIFO_SIZE   64
+uint8_t tu_ff_buf[FIFO_SIZE * sizeof(uint8_t)];
+tu_fifo_t tu_ff = TU_FIFO_INIT(tu_ff_buf, FIFO_SIZE, uint8_t, false);
+
 tu_fifo_t* ff = &tu_ff;
 tu_fifo_buffer_info_t info;
 
+uint8_t test_data[4096];
+uint8_t rd_buf[FIFO_SIZE];
+
 void setUp(void)
 {
   tu_fifo_clear(ff);
   memset(&info, 0, sizeof(tu_fifo_buffer_info_t));
+
+  for(int i=0; i<sizeof(test_data); i++) test_data[i] = i;
+  memset(rd_buf, 0, sizeof(rd_buf));
 }
 
 void tearDown(void)
@@ -62,86 +70,136 @@ void test_normal(void)
 
 void test_item_size(void)
 {
-  TU_FIFO_DEF(ff4, FIFO_SIZE, uint32_t, false);
-  tu_fifo_clear(&ff4);
+  uint8_t ff4_buf[FIFO_SIZE * sizeof(uint32_t)];
+  tu_fifo_t ff4 = TU_FIFO_INIT(ff4_buf, FIFO_SIZE, uint32_t, false);
 
-  uint32_t data[20];
-  for(uint32_t i=0; i<sizeof(data)/4; i++) data[i] = i;
+  uint32_t data4[2*FIFO_SIZE];
+  for(uint32_t i=0; i<sizeof(data4)/4; i++) data4[i] = i;
 
-  tu_fifo_write_n(&ff4, data, 10);
+  // fill up fifo
+  tu_fifo_write_n(&ff4, data4, FIFO_SIZE);
 
-  uint32_t rd[10];
+  uint32_t rd_buf4[FIFO_SIZE];
   uint16_t rd_count;
 
   // read 0 -> 4
-  rd_count = tu_fifo_read_n(&ff4, rd, 5);
+  rd_count = tu_fifo_read_n(&ff4, rd_buf4, 5);
   TEST_ASSERT_EQUAL( 5, rd_count );
-  TEST_ASSERT_EQUAL_UINT32_ARRAY( data, rd, rd_count ); // 0 -> 4
+  TEST_ASSERT_EQUAL_UINT32_ARRAY( data4, rd_buf4, rd_count ); // 0 -> 4
 
-  tu_fifo_write_n(&ff4, data+10, 5);
+  tu_fifo_write_n(&ff4, data4+FIFO_SIZE, 5);
 
-  // read 5 -> 14
-  rd_count = tu_fifo_read_n(&ff4, rd, 10);
-  TEST_ASSERT_EQUAL( 10, rd_count );
-  TEST_ASSERT_EQUAL_UINT32_ARRAY( data+5, rd, rd_count ); // 5 -> 14
+  // read all 5 -> 68
+  rd_count = tu_fifo_read_n(&ff4, rd_buf4, FIFO_SIZE);
+  TEST_ASSERT_EQUAL( FIFO_SIZE, rd_count );
+  TEST_ASSERT_EQUAL_UINT32_ARRAY( data4+5, rd_buf4, rd_count ); // 5 -> 68
 }
 
 void test_read_n(void)
 {
-  // prepare data
-  uint8_t data[20];
-  for(int i=0; i<sizeof(data); i++) data[i] = i;
-
-  for(uint8_t i=0; i < FIFO_SIZE; i++) tu_fifo_write(ff, data+i);
-
-  uint8_t rd[10];
   uint16_t rd_count;
 
+  // fill up fifo
+  for(uint8_t i=0; i < FIFO_SIZE; i++) tu_fifo_write(ff, test_data+i);
+
   // case 1: Read index + count < depth
   // read 0 -> 4
-  rd_count = tu_fifo_read_n(ff, rd, 5);
+  rd_count = tu_fifo_read_n(ff, rd_buf, 5);
   TEST_ASSERT_EQUAL( 5, rd_count );
-  TEST_ASSERT_EQUAL_MEMORY( data, rd, rd_count ); // 0 -> 4
+  TEST_ASSERT_EQUAL_MEMORY( test_data, rd_buf, rd_count ); // 0 -> 4
 
   // case 2: Read index + count > depth
   // write 10, 11, 12
-  tu_fifo_write(ff, data+10);
-  tu_fifo_write(ff, data+11);
-  tu_fifo_write(ff, data+12);
+  tu_fifo_write(ff, test_data+FIFO_SIZE);
+  tu_fifo_write(ff, test_data+FIFO_SIZE+1);
+  tu_fifo_write(ff, test_data+FIFO_SIZE+2);
 
-  rd_count = tu_fifo_read_n(ff, rd, 7);
+  rd_count = tu_fifo_read_n(ff, rd_buf, 7);
   TEST_ASSERT_EQUAL( 7, rd_count );
 
-  TEST_ASSERT_EQUAL_MEMORY( data+5, rd, rd_count ); // 5 -> 11
+  TEST_ASSERT_EQUAL_MEMORY( test_data+5, rd_buf, rd_count ); // 5 -> 11
 
   // Should only read until empty
-  TEST_ASSERT_EQUAL( 1, tu_fifo_read_n(ff, rd, 100) );
+  TEST_ASSERT_EQUAL( FIFO_SIZE-5+3-7, tu_fifo_read_n(ff, rd_buf, 100) );
 }
 
 void test_write_n(void)
 {
-  // prepare data
-  uint8_t data[20];
-  for(int i=0; i<sizeof(data); i++) data[i] = i;
-
   // case 1: wr + count < depth
-  tu_fifo_write_n(ff, data, 8); // wr = 8, count = 8
+  tu_fifo_write_n(ff, test_data, 32); // wr = 32, count = 32
 
-  uint8_t rd[10];
   uint16_t rd_count;
 
-  rd_count = tu_fifo_read_n(ff, rd, 5); // wr = 8, count = 3
-  TEST_ASSERT_EQUAL( 5, rd_count );
-  TEST_ASSERT_EQUAL_MEMORY( data, rd, rd_count ); // 0 -> 4
+  rd_count = tu_fifo_read_n(ff, rd_buf, 16); // wr = 32, count = 16
+  TEST_ASSERT_EQUAL( 16, rd_count );
+  TEST_ASSERT_EQUAL_MEMORY( test_data, rd_buf, rd_count );
 
   // case 2: wr + count > depth
-  tu_fifo_write_n(ff, data+8, 6); // wr = 3, count = 9
+  tu_fifo_write_n(ff, test_data+32, 40); // wr = 72 -> 8, count = 56
+
+  tu_fifo_read_n(ff, rd_buf, 32); // count = 24
+  TEST_ASSERT_EQUAL_MEMORY( test_data+16, rd_buf, rd_count);
+
+  TEST_ASSERT_EQUAL(24, tu_fifo_count(ff));
+}
+
+void test_write_double_overflowed(void)
+{
+  tu_fifo_set_overwritable(ff, true);
+
+  uint8_t rd_buf[FIFO_SIZE] = { 0 };
+  uint8_t* buf = test_data;
 
-  for(rd_count=0; rd_count<7; rd_count++) tu_fifo_read(ff, rd+rd_count); // wr = 3, count = 2
+  // full
+  buf += tu_fifo_write_n(ff, buf, FIFO_SIZE);
+  TEST_ASSERT_EQUAL(FIFO_SIZE, tu_fifo_count(ff));
+
+  // write more, should still full
+  buf += tu_fifo_write_n(ff, buf, FIFO_SIZE-8);
+  TEST_ASSERT_EQUAL(FIFO_SIZE, tu_fifo_count(ff));
+
+  // double overflowed: in total, write more than > 2*FIFO_SIZE
+  buf += tu_fifo_write_n(ff, buf, 16);
+  TEST_ASSERT_EQUAL(FIFO_SIZE, tu_fifo_count(ff));
+
+  // reading back should give back data from last FIFO_SIZE write
+  tu_fifo_read_n(ff, rd_buf, FIFO_SIZE);
+
+  TEST_ASSERT_EQUAL_MEMORY(buf-16, rd_buf+FIFO_SIZE-16, 16);
+
+  // TODO whole buffer should match, but we deliberately not implement it
+  // TEST_ASSERT_EQUAL_MEMORY(buf-FIFO_SIZE, rd_buf, FIFO_SIZE);
+}
+
+static uint16_t help_write(uint16_t total, uint16_t n)
+{
+  tu_fifo_write_n(ff, test_data, n);
+  total = tu_min16(FIFO_SIZE, total + n);
+
+  TEST_ASSERT_EQUAL(total, tu_fifo_count(ff));
+  TEST_ASSERT_EQUAL(FIFO_SIZE - total, tu_fifo_remaining(ff));
+
+  return total;
+}
+
+void test_write_overwritable2(void)
+{
+  tu_fifo_set_overwritable(ff, true);
 
-  TEST_ASSERT_EQUAL_MEMORY( data+5, rd, rd_count); // 5 -> 11
+  // based on actual crash tests detected by fuzzing
+  uint16_t total = 0;
 
-  TEST_ASSERT_EQUAL(2, tu_fifo_count(ff));
+  total = help_write(total, 12);
+  total = help_write(total, 55);
+  total = help_write(total, 73);
+  total = help_write(total, 55);
+  total = help_write(total, 75);
+  total = help_write(total, 84);
+  total = help_write(total, 1);
+  total = help_write(total, 10);
+  total = help_write(total, 12);
+  total = help_write(total, 25);
+  total = help_write(total, 192);
 }
 
 void test_peek(void)