Nico Huber submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Angel Pons: Looks good to me, approved
Enable dynamic memory allocation checks for cmocka unit tests

This commit enables the feature and makes changes to existing
files and tests. I am writing more new tests with this.

Commit includes tests/flashrom.c because after enabling memory
checks the test started to fail (it used to leak memory indeed).

If you are wondering how to verify it works (because at the moment
all tests [still] pass so it’s not obvious that anything has
changed), then for example:

1) Remove free’s in flashbuses_to_text_test_success test, and it
will fail with message similar to this (line numbers from your local
source)
[ ERROR ] --- Blocks allocated...
../flashrom.c:1239: note: block 0x55f42304b640 allocated here
../flashrom.c:1239: note: block 0x55f42304b5c0 allocated here
../flashrom.c:1239: note: block 0x55f42304b3d0 allocated here
../flashrom.c:1239: note: block 0x55f42304b700 allocated here
../flashrom.c:1239: note: block 0x55f42304b780 allocated here
../flashrom.c:1239: note: block 0x55f42304bb00 allocated here
../flashrom.c:1239: note: block 0x55f42304b810 allocated here
ERROR: flashbuses_to_text_test_success leaked 7 block(s)

2) Add char *temp = malloc just before return from strcat_realloc
[ ERROR ] --- Blocks allocated...
../helpers.c:88: note: block 0x55a51307b6c0 allocated here
../helpers.c:88: note: block 0x55a51307b9e0 allocated here
ERROR: strcat_realloc_test_success leaked 2 block(s)

BUG=b:181803212
TEST=builds and ninja test
nm builddir/tests/flashrom_unit_tests.p/.._flashrom.c.o
nm builddir/tests/flashrom_unit_tests.p/flashrom.c.o
nm builddir/flashrom.p/flashrom.c.o

Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/51243
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
---
M meson.build
M tests/flashrom.c
M tests/helpers.c
A unittest_env.h
4 files changed, 81 insertions(+), 12 deletions(-)

diff --git a/meson.build b/meson.build
index d5895d8..5b339e0 100644
--- a/meson.build
+++ b/meson.build
@@ -472,6 +472,10 @@
'cli_output.c',
'flashrom.c',
],
+ compile_args : [
+ '-includestdlib.h',
+ '-includeunittest_env.h',
+ ],
dependencies : [
deps,
],
diff --git a/tests/flashrom.c b/tests/flashrom.c
index 50464dd..c3508c5 100644
--- a/tests/flashrom.c
+++ b/tests/flashrom.c
@@ -17,35 +17,54 @@

#include "programmer.h"

+#define assert_equal_and_free(text, expected) \
+ do { \
+ assert_string_equal(text, expected); \
+ free(text); \
+ } while (0)
+
+#define assert_not_equal_and_free(text, expected) \
+ do { \
+ assert_string_not_equal(text, expected); \
+ free(text); \
+ } while (0)
+
+
void flashbuses_to_text_test_success(void **state)
{
(void) state; /* unused */

enum chipbustype bustype;
+ char *text;

bustype = BUS_NONSPI;
- assert_string_equal(flashbuses_to_text(bustype), "Non-SPI");
+ text = flashbuses_to_text(bustype);
+ assert_equal_and_free(text, "Non-SPI");

bustype |= BUS_PARALLEL;
- assert_string_not_equal(flashbuses_to_text(bustype), "Non-SPI, Parallel");
+ text = flashbuses_to_text(bustype);
+ assert_not_equal_and_free(text, "Non-SPI, Parallel");

bustype = BUS_PARALLEL;
bustype |= BUS_LPC;
- assert_string_equal(flashbuses_to_text(bustype), "Parallel, LPC");
+ text = flashbuses_to_text(bustype);
+ assert_equal_and_free(text, "Parallel, LPC");

bustype |= BUS_FWH;
//BUS_NONSPI = BUS_PARALLEL | BUS_LPC | BUS_FWH,
- assert_string_equal(flashbuses_to_text(bustype), "Non-SPI");
+ text = flashbuses_to_text(bustype);
+ assert_equal_and_free(text, "Non-SPI");

bustype |= BUS_SPI;
- assert_string_equal(flashbuses_to_text(bustype), "Parallel, LPC, FWH, SPI");
+ text = flashbuses_to_text(bustype);
+ assert_equal_and_free(text, "Parallel, LPC, FWH, SPI");

bustype |= BUS_PROG;
- assert_string_equal(
- flashbuses_to_text(bustype),
- "Parallel, LPC, FWH, SPI, Programmer-specific"
- );
+ text = flashbuses_to_text(bustype);
+ assert_equal_and_free(text,
+ "Parallel, LPC, FWH, SPI, Programmer-specific");

bustype = BUS_NONE;
- assert_string_equal(flashbuses_to_text(bustype), "None");
+ text = flashbuses_to_text(bustype);
+ assert_equal_and_free(text, "None");
}
diff --git a/tests/helpers.c b/tests/helpers.c
index a920c15..4376eee 100644
--- a/tests/helpers.c
+++ b/tests/helpers.c
@@ -18,8 +18,6 @@
#include "flash.h"

#include <stdint.h>
-#include <stdlib.h>
-

void address_to_bits_test_success(void **state)
{
diff --git a/unittest_env.h b/unittest_env.h
new file mode 100644
index 0000000..9bd7509
--- /dev/null
+++ b/unittest_env.h
@@ -0,0 +1,48 @@
+/*
+ * This file is part of the flashrom project.
+ *
+ * Copyright 2021 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+/*
+ * This header is included in all files when they are built for unit test
+ * environment, for all the files to be covered by dynamic memory allocation
+ * checks (checks for memory leaks, buffer overflows and underflows).
+ *
+ * See flashrom_test_dep in meson.build for more details.
+ *
+ * https://api.cmocka.org/group__cmocka__alloc.html
+ */
+
+extern void* _test_malloc(const size_t size, const char* file, const int line);
+extern void* _test_realloc(void *ptr, const size_t size, const char* file, const int line);
+extern void* _test_calloc(const size_t number_of_elements, const size_t size,
+ const char* file, const int line);
+extern void _test_free(void* const ptr, const char* file, const int line);
+
+#ifdef malloc
+#undef malloc
+#endif
+#ifdef calloc
+#undef calloc
+#endif
+#ifdef realloc
+#undef realloc
+#endif
+#ifdef free
+#undef free
+#endif
+
+#define malloc(size) _test_malloc(size, __FILE__, __LINE__)
+#define realloc(ptr, size) _test_realloc(ptr, size, __FILE__, __LINE__)
+#define calloc(num, size) _test_calloc(num, size, __FILE__, __LINE__)
+#define free(ptr) _test_free(ptr, __FILE__, __LINE__)

To view, visit change 51243. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f
Gerrit-Change-Number: 51243
Gerrit-PatchSet: 8
Gerrit-Owner: Anastasia Klimchuk <aklm@chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Daniel Kurtz <djkurtz@google.com>
Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Daniel Kurtz <djkurtz@chromium.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-CC: Thomas Heijligen <src@posteo.de>
Gerrit-CC: Victor Ding <victording@chromium.org>
Gerrit-MessageType: merged