November 2022 - flashrom-gerrit

[M] Change in flashrom[master]: tests: Detect gcov file io and unmock io functions
by Evan Benn (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Anastasia Klimchuk. Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69266 ) Change subject: tests: Detect gcov file io and unmock io functions ...................................................................... tests: Detect gcov file io and unmock io functions Code coverage writes data to disk atexit of the program. We need to unmock the file io at this point so that the data is really written. BUG=b:187647884 BRANCH=None TEST=meson test TEST=ninja coverage Change-Id: If06053ecd78e886c8f7fc55813f4b5635be78c6b Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M Documentation/building.md M tests/io_mock.h M tests/tests.c M tests/wraps.h 4 files changed, 78 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/66/69266/1 diff --git a/Documentation/building.md b/Documentation/building.md index aabb45b..0e66f29 100644 --- a/Documentation/building.md +++ b/Documentation/building.md @@ -48,6 +48,13 @@ ninja -C builddir test ``` +### Run unit tests with code coverage +``` +meson setup buildcov -Db_coverage=true +ninja -C buildcov test +ninja -C buildcov coverage +``` + ## System specific information ### Ubuntu / Debian (Linux) * __linux-headers__ are version specific diff --git a/tests/io_mock.h b/tests/io_mock.h index 8e6ecb9..ef79a61 100644 --- a/tests/io_mock.h +++ b/tests/io_mock.h @@ -107,8 +107,10 @@ FILE* (*fopen)(void *state, const char *pathname, const char *mode); char* (*fgets)(void *state, char *buf, int len, FILE *fp); size_t (*fread)(void *state, void *buf, size_t size, size_t len, FILE *fp); + size_t (*fwrite)(void *state, const void *buf, size_t size, size_t len, FILE *fp); int (*fprintf)(void *state, FILE *fp, const char *fmt, va_list args); int (*fclose)(void *state, FILE *fp); + FILE *(*fdopen)(void*state, int fd, const char *mode); /* * An alternative to custom open mock. A test can either register its diff --git a/tests/tests.c b/tests/tests.c index 530d5a7..1b5b6d6 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -18,10 +18,40 @@ #include "tests.h" #include "wraps.h" +#include "fcntl.h" +#include <ctype.h> +#include <stdarg.h> #include <stdio.h> #include <string.h> #include <stdint.h> + +static int unwrap_open(void *state, const char *pathname, int flags, mode_t mode) { + LOG_ME; + (void)state; + return __real_open(pathname, flags, mode); +} + +static FILE *unwrap_fdopen(void *state, int fd, const char *mode) { + LOG_ME; + (void)state; + return __real_fdopen(fd, mode); +} + +static size_t unwrap_fwrite(void *state, const void *ptr, size_t size, size_t nmemb, FILE *fp) { + LOG_ME; + (void)state; + return __real_fwrite(ptr, size, nmemb, fp); +} + +// Mock ios that defer to the real io functions. +// These exist so that code coverage can print to real files on disk. +struct io_mock real_io_mock = { + .open = unwrap_open, + .fwrite = unwrap_fwrite, + .fdopen = unwrap_fdopen, +}; + void *not_null(void) { return (void *)MOCK_FD; @@ -75,8 +105,22 @@ return (uint8_t)mock(); } +static int check_suffix(const char* string, const char* suffix) { + int len_l = strlen(string); + int len_r = strlen(suffix); + if (len_l > len_r) { + return strcmp(string + len_l - len_r, suffix); + } + return 1; +} + static int mock_open(const char *pathname, int flags, mode_t mode) { + // We detect code coverage writing the log files and defer to real io functions. + if (!check_suffix(pathname, ".gcda")) { + io_mock_register(&real_io_mock); + } + if (get_io() && get_io()->open) return get_io()->open(get_io()->state, pathname, flags, mode); @@ -163,6 +207,8 @@ FILE *__wrap_fdopen(int fd, const char *mode) { LOG_ME; + if (get_io() && get_io()->fdopen) + return get_io()->fdopen(get_io()->state, fd, mode); return not_null(); } @@ -241,6 +287,8 @@ size_t __wrap_fwrite(const void *ptr, size_t size, size_t nmemb, FILE *fp) { LOG_ME; + if (get_io() && get_io()->fwrite) + return get_io()->fwrite(get_io()->state, ptr, size, nmemb, fp); return nmemb; } diff --git a/tests/wraps.h b/tests/wraps.h index 59cd6ff..44f78f7 100644 --- a/tests/wraps.h +++ b/tests/wraps.h @@ -29,6 +29,7 @@ void __wrap_sio_write(uint16_t port, uint8_t reg, uint8_t data); uint8_t __wrap_sio_read(uint16_t port, uint8_t reg); int __wrap_open(const char *pathname, int flags, mode_t mode); +int __real_open(const char *pathname, int flags, mode_t mode); int __wrap_open64(const char *pathname, int flags, mode_t mode); int __wrap___open64_2(const char *pathname, int flags, mode_t mode); int __wrap_ioctl(int fd, unsigned long int request, ...); @@ -37,6 +38,7 @@ FILE *__wrap_fopen(const char *pathname, const char *mode); FILE *__wrap_fopen64(const char *pathname, const char *mode); FILE *__wrap_fdopen(int fd, const char *mode); +FILE *__real_fdopen(int fd, const char *mode); int __wrap_stat(const char *path, void *buf); int __wrap_stat64(const char *path, void *buf); int __wrap___xstat(const char *path, void *buf); @@ -49,6 +51,7 @@ char *__wrap___fgets_chk(char *buf, int len, FILE *fp); size_t __wrap_fread(void *ptr, size_t size, size_t nmemb, FILE *fp); size_t __wrap_fwrite(const void *ptr, size_t size, size_t nmemb, FILE *fp); +size_t __real_fwrite(const void *ptr, size_t size, size_t nmemb, FILE *fp); int __wrap_fflush(FILE *fp); int __wrap_fileno(FILE *fp); int __wrap_fsync(int fd); -- To view, visit https://review.coreboot.org/c/flashrom/+/69266 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If06053ecd78e886c8f7fc55813f4b5635be78c6b Gerrit-Change-Number: 69266 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[L] Change in flashrom[master]: meson: Move meson tests config to top level build file
by Evan Benn (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69265 ) Change subject: meson: Move meson tests config to top level build file ...................................................................... meson: Move meson tests config to top level build file meson has a bug with code coverage, moving the test to the top level build file works around this bug. https://github.com/mesonbuild/meson/issues/6747 BUG=b:187647884 BRANCH=None TEST=meson test Change-Id: Ibba93037831fc0ed526f701cbe360b5a493895d1 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M meson.build D tests/meson.build 2 files changed, 131 insertions(+), 126 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/69265/1 diff --git a/meson.build b/meson.build index 3a6cc41..72ba6d7 100644 --- a/meson.build +++ b/meson.build @@ -621,7 +621,7 @@ ) flashrom_test_dep = declare_dependency( - include_directories : include_dir, + include_directories : include_directories('include', 'tests', 'tests/include'), sources : [ srcs, 'cli_common.c', @@ -637,9 +637,118 @@ ], ) - if cmocka_dep.found() - subdir('tests') + test_srcs = files( + 'tests/io_mock.c', + 'tests/tests.c', + 'tests/libusb_wraps.c', + 'tests/helpers.c', + 'tests/flashrom.c', + 'tests/spi25.c', + 'tests/lifecycle.c', + 'tests/dummyflasher.c', + 'tests/nicrealtek.c', + 'tests/raiden_debug_spi.c', + 'tests/dediprog.c', + 'tests/linux_mtd.c', + 'tests/linux_spi.c', + 'tests/parade_lspcon.c', + 'tests/mediatek_i2c_spi.c', + 'tests/realtek_mst_i2c_spi.c', + 'tests/layout.c', + 'tests/chip.c', + 'tests/chip_wp.c', + ) + + if not programmer.get('dummy').get('active') + test_srcs += programmer.get('dummy').get('srcs') endif + + mocks = [ + '-Wl,--wrap=strdup', + '-Wl,--wrap=physunmap', + '-Wl,--wrap=physmap', + '-Wl,--wrap=pcidev_init', + '-Wl,--wrap=pcidev_readbar', + '-Wl,--wrap=spi_send_command', + '-Wl,--wrap=sio_write', + '-Wl,--wrap=sio_read', + '-Wl,--wrap=open', + '-Wl,--wrap=open64', + '-Wl,--wrap=__open64_2', + '-Wl,--wrap=ioctl', + '-Wl,--wrap=read', + '-Wl,--wrap=write', + '-Wl,--wrap=fopen', + '-Wl,--wrap=fopen64', + '-Wl,--wrap=fdopen', + '-Wl,--wrap=fwrite', + '-Wl,--wrap=fflush', + '-Wl,--wrap=stat', + '-Wl,--wrap=stat64', + '-Wl,--wrap=__xstat', + '-Wl,--wrap=__xstat64', + '-Wl,--wrap=fstat', + '-Wl,--wrap=fstat64', + '-Wl,--wrap=__fxstat', + '-Wl,--wrap=__fxstat64', + '-Wl,--wrap=fileno', + '-Wl,--wrap=fsync', + '-Wl,--wrap=fread', + '-Wl,--wrap=fgets', + '-Wl,--wrap=__fgets_chk', + '-Wl,--wrap=fprintf', + '-Wl,--wrap=__vfprintf_chk', + '-Wl,--wrap=fclose', + '-Wl,--wrap=feof', + '-Wl,--wrap=ferror', + '-Wl,--wrap=clearerr', + '-Wl,--wrap=setvbuf', + '-Wl,--wrap=rget_io_perms', + '-Wl,--wrap=OUTB', + '-Wl,--wrap=INB', + '-Wl,--wrap=OUTW', + '-Wl,--wrap=INW', + '-Wl,--wrap=OUTL', + '-Wl,--wrap=INL', + '-Wl,--wrap=usb_dev_get_by_vid_pid_number', + '-Wl,--wrap=libusb_init', + '-Wl,--wrap=libusb_open', + '-Wl,--wrap=libusb_set_auto_detach_kernel_driver', + '-Wl,--wrap=libusb_detach_kernel_driver', + '-Wl,--wrap=libusb_attach_kernel_driver', + '-Wl,--wrap=libusb_get_device_list', + '-Wl,--wrap=libusb_free_device_list', + '-Wl,--wrap=libusb_get_bus_number', + '-Wl,--wrap=libusb_get_device_address', + '-Wl,--wrap=libusb_get_device_descriptor', + '-Wl,--wrap=libusb_get_config_descriptor', + '-Wl,--wrap=libusb_free_config_descriptor', + '-Wl,--wrap=libusb_get_configuration', + '-Wl,--wrap=libusb_set_configuration', + '-Wl,--wrap=libusb_claim_interface', + '-Wl,--wrap=libusb_control_transfer', + '-Wl,--wrap=libusb_release_interface', + '-Wl,--wrap=libusb_ref_device', + '-Wl,--wrap=libusb_unref_device', + '-Wl,--wrap=libusb_close', + '-Wl,--wrap=libusb_exit', + '-Wl,--gc-sections', + ] + + # The test is declared here to work around a meson code coverage bug: + # https://github.com/mesonbuild/meson/issues/6747 + flashrom_tests = executable('flashrom_unit_tests', + test_srcs, + c_args : [ + cargs, + '-ffunction-sections', + '-fdata-sections', + ], + export_dynamic : true, + link_args : mocks, + dependencies : [cmocka_dep, flashrom_test_dep], + ) + test('cmocka test flashrom', flashrom_tests, timeout: 60) endif programmer_names_active = [] diff --git a/tests/meson.build b/tests/meson.build deleted file mode 100644 index c62cc1b..0000000 --- a/tests/meson.build +++ /dev/null @@ -1,123 +0,0 @@ -# This file is part of the flashrom project. -# -# Copyright 2020 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. - -srcs = files( - 'io_mock.c', - 'tests.c', - 'libusb_wraps.c', - 'helpers.c', - 'flashrom.c', - 'spi25.c', - 'lifecycle.c', - 'dummyflasher.c', - 'nicrealtek.c', - 'raiden_debug_spi.c', - 'dediprog.c', - 'linux_mtd.c', - 'linux_spi.c', - 'parade_lspcon.c', - 'mediatek_i2c_spi.c', - 'realtek_mst_i2c_spi.c', - 'layout.c', - 'chip.c', - 'chip_wp.c', -) - -if not programmer.get('dummy').get('active') - srcs += programmer.get('dummy').get('srcs') -endif - -mocks = [ - '-Wl,--wrap=strdup', - '-Wl,--wrap=physunmap', - '-Wl,--wrap=physmap', - '-Wl,--wrap=pcidev_init', - '-Wl,--wrap=pcidev_readbar', - '-Wl,--wrap=spi_send_command', - '-Wl,--wrap=sio_write', - '-Wl,--wrap=sio_read', - '-Wl,--wrap=open', - '-Wl,--wrap=open64', - '-Wl,--wrap=__open64_2', - '-Wl,--wrap=ioctl', - '-Wl,--wrap=read', - '-Wl,--wrap=write', - '-Wl,--wrap=fopen', - '-Wl,--wrap=fopen64', - '-Wl,--wrap=fdopen', - '-Wl,--wrap=fwrite', - '-Wl,--wrap=fflush', - '-Wl,--wrap=stat', - '-Wl,--wrap=stat64', - '-Wl,--wrap=__xstat', - '-Wl,--wrap=__xstat64', - '-Wl,--wrap=fstat', - '-Wl,--wrap=fstat64', - '-Wl,--wrap=__fxstat', - '-Wl,--wrap=__fxstat64', - '-Wl,--wrap=fileno', - '-Wl,--wrap=fsync', - '-Wl,--wrap=fread', - '-Wl,--wrap=fgets', - '-Wl,--wrap=__fgets_chk', - '-Wl,--wrap=fprintf', - '-Wl,--wrap=__vfprintf_chk', - '-Wl,--wrap=fclose', - '-Wl,--wrap=feof', - '-Wl,--wrap=ferror', - '-Wl,--wrap=clearerr', - '-Wl,--wrap=setvbuf', - '-Wl,--wrap=rget_io_perms', - '-Wl,--wrap=OUTB', - '-Wl,--wrap=INB', - '-Wl,--wrap=OUTW', - '-Wl,--wrap=INW', - '-Wl,--wrap=OUTL', - '-Wl,--wrap=INL', - '-Wl,--wrap=usb_dev_get_by_vid_pid_number', - '-Wl,--wrap=libusb_init', - '-Wl,--wrap=libusb_open', - '-Wl,--wrap=libusb_set_auto_detach_kernel_driver', - '-Wl,--wrap=libusb_detach_kernel_driver', - '-Wl,--wrap=libusb_attach_kernel_driver', - '-Wl,--wrap=libusb_get_device_list', - '-Wl,--wrap=libusb_free_device_list', - '-Wl,--wrap=libusb_get_bus_number', - '-Wl,--wrap=libusb_get_device_address', - '-Wl,--wrap=libusb_get_device_descriptor', - '-Wl,--wrap=libusb_get_config_descriptor', - '-Wl,--wrap=libusb_free_config_descriptor', - '-Wl,--wrap=libusb_get_configuration', - '-Wl,--wrap=libusb_set_configuration', - '-Wl,--wrap=libusb_claim_interface', - '-Wl,--wrap=libusb_control_transfer', - '-Wl,--wrap=libusb_release_interface', - '-Wl,--wrap=libusb_ref_device', - '-Wl,--wrap=libusb_unref_device', - '-Wl,--wrap=libusb_close', - '-Wl,--wrap=libusb_exit', - '-Wl,--gc-sections', -] - -flashrom_tests = executable('flashrom_unit_tests', - srcs, - c_args : [ - cargs, - '-ffunction-sections', - '-fdata-sections', - ], - export_dynamic : true, - link_args : mocks, - dependencies : [cmocka_dep, flashrom_test_dep], -) -test('cmocka test flashrom', flashrom_tests) -- To view, visit https://review.coreboot.org/c/flashrom/+/69265 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibba93037831fc0ed526f701cbe360b5a493895d1 Gerrit-Change-Number: 69265 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: tests: Change LOG_ME to print to stderr
by Evan Benn (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Anastasia Klimchuk. Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69264 ) Change subject: tests: Change LOG_ME to print to stderr ...................................................................... tests: Change LOG_ME to print to stderr Printing to stderr instead of stdout helps when the test is segfaulting. stdout messages may be discarded. BUG=b:187647884 BRANCH=None TEST=meson test Change-Id: I1964832f62b2e13fafa0ea0537a3da499f0f4a5d Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M tests/include/test.h M tests/wraps.h 2 files changed, 20 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/69264/1 diff --git a/tests/include/test.h b/tests/include/test.h index 88ac16d..e14d906 100644 --- a/tests/include/test.h +++ b/tests/include/test.h @@ -26,6 +26,7 @@ #include <stddef.h> #include <setjmp.h> #include <cmocka.h> +#include <wraps.h> #define NON_ZERO (0xf000baaa) @@ -37,6 +38,6 @@ */ void *not_null(void); -#define LOG_ME printf("%s is called\n", __func__) +#define LOG_ME __real_fprintf(stderr, "%s is called\n", __func__) #endif /* _TESTS_TEST_H */ diff --git a/tests/wraps.h b/tests/wraps.h index e2134fd..59cd6ff 100644 --- a/tests/wraps.h +++ b/tests/wraps.h @@ -54,6 +54,7 @@ int __wrap_fsync(int fd); int __wrap_setvbuf(FILE *fp, char *buf, int type, size_t size); int __wrap_fprintf(FILE *fp, const char *fmt, ...); +int __real_fprintf(FILE *fp, const char *fmt, ...); int __wrap___vfprintf_chk(FILE *fp, const char *fmt, va_list args); int __wrap_fclose(FILE *fp); int __wrap_feof(FILE *fp); -- To view, visit https://review.coreboot.org/c/flashrom/+/69264 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1964832f62b2e13fafa0ea0537a3da499f0f4a5d Gerrit-Change-Number: 69264 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: tests: Mock the mode_t variant of open
by Evan Benn (Code Review) 07 Nov '22

07 Nov '22

Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69263 ) Change subject: tests: Mock the mode_t variant of open ...................................................................... tests: Mock the mode_t variant of open open has a second form with a mode_t argument. When mocking without this argument a caller trying to O_CREAT would have their mode_t argument discarded and a random stack variable would be used instead. BUG=b:187647884 BRANCH=None TEST=meson test Change-Id: I8c134e6d36a248d0f51985e389085a9e585fb83d Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M tests/io_mock.h M tests/tests.c M tests/wraps.h 3 files changed, 32 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/69263/1 diff --git a/tests/io_mock.h b/tests/io_mock.h index 9e92a9d..8e6ecb9 100644 --- a/tests/io_mock.h +++ b/tests/io_mock.h @@ -33,6 +33,8 @@ /* Required for `FILE *` */ #include <stdio.h> +/* Required for `mode_t` */ +#include <fcntl.h> #include <stdint.h> @@ -47,11 +49,6 @@ unsigned int device_id; }; -/* POSIX open() flags, avoiding dependency on fcntl.h */ -#define O_RDONLY 0 -#define O_WRONLY 1 -#define O_RDWR 2 - /* Linux I2C interface constants, avoiding linux/i2c-dev.h */ #define I2C_SLAVE 0x0703 @@ -101,7 +98,7 @@ void (*libusb_free_config_descriptor)(void *state, struct libusb_config_descriptor *); /* POSIX File I/O */ - int (*open)(void *state, const char *pathname, int flags); + int (*open)(void *state, const char *pathname, int flags, mode_t mode); int (*ioctl)(void *state, int fd, unsigned long request, va_list args); int (*read)(void *state, int fd, void *buf, size_t sz); int (*write)(void *state, int fd, const void *buf, size_t sz); diff --git a/tests/tests.c b/tests/tests.c index 53bad65..530d5a7 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -75,10 +75,10 @@ return (uint8_t)mock(); } -static int mock_open(const char *pathname, int flags) +static int mock_open(const char *pathname, int flags, mode_t mode) { if (get_io() && get_io()->open) - return get_io()->open(get_io()->state, pathname, flags); + return get_io()->open(get_io()->state, pathname, flags, mode); if (get_io() && get_io()->fallback_open_state) { struct io_mock_fallback_open_state *io_state; @@ -96,22 +96,22 @@ return MOCK_FD; } -int __wrap_open(const char *pathname, int flags) +int __wrap_open(const char *pathname, int flags, mode_t mode) { LOG_ME; - return mock_open(pathname, flags); + return mock_open(pathname, flags, mode); } -int __wrap_open64(const char *pathname, int flags) +int __wrap_open64(const char *pathname, int flags, mode_t mode) { LOG_ME; - return mock_open(pathname, flags); + return mock_open(pathname, flags, mode); } -int __wrap___open64_2(const char *pathname, int flags) +int __wrap___open64_2(const char *pathname, int flags, mode_t mode) { LOG_ME; - return mock_open(pathname, flags); + return mock_open(pathname, flags, mode); } int __wrap_ioctl(int fd, unsigned long int request, ...) diff --git a/tests/wraps.h b/tests/wraps.h index f97eaba..e2134fd 100644 --- a/tests/wraps.h +++ b/tests/wraps.h @@ -28,9 +28,9 @@ uintptr_t __wrap_pcidev_readbar(void *dev, int bar); void __wrap_sio_write(uint16_t port, uint8_t reg, uint8_t data); uint8_t __wrap_sio_read(uint16_t port, uint8_t reg); -int __wrap_open(const char *pathname, int flags); -int __wrap_open64(const char *pathname, int flags); -int __wrap___open64_2(const char *pathname, int flags); +int __wrap_open(const char *pathname, int flags, mode_t mode); +int __wrap_open64(const char *pathname, int flags, mode_t mode); +int __wrap___open64_2(const char *pathname, int flags, mode_t mode); int __wrap_ioctl(int fd, unsigned long int request, ...); int __wrap_write(int fd, const void *buf, size_t sz); int __wrap_read(int fd, void *buf, size_t sz); -- To view, visit https://review.coreboot.org/c/flashrom/+/69263 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8c134e6d36a248d0f51985e389085a9e585fb83d Gerrit-Change-Number: 69263 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: tests: Add prefix to io_mock functions not to clash with macros
by Anastasia Klimchuk (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68433 to look at the new patch set (#7). Change subject: tests: Add prefix to io_mock functions not to clash with macros ...................................................................... tests: Add prefix to io_mock functions not to clash with macros Flashrom I/O mock functions need to be renamed so that they do not have name clash with standard I/O, because the latter are allowed to be macros. Adding a prefix to flashrom mock functions avoids them being accidentally expanded. Standard I/O functions are expanded and flashrom mocks stay as they are. BUG=b:237606255 TEST=ninja test 1) gcc 12.2.0 on Debian 2) clang 15.0 on Chromium OS Ticket: https://ticket.coreboot.org/issues/411 Change-Id: I7998a8fb1b9e65621e12adbfab5460a245d5606b Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/chip.c M tests/io_mock.c M tests/io_mock.h M tests/linux_mtd.c M tests/linux_spi.c M tests/parade_lspcon.c M tests/realtek_mst_i2c_spi.c M tests/tests.c 8 files changed, 65 insertions(+), 43 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/68433/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/68433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7998a8fb1b9e65621e12adbfab5460a245d5606b Gerrit-Change-Number: 68433 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants
by Anastasia Klimchuk (Code Review) 07 Nov '22

07 Nov '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68432 ) ( 5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants ...................................................................... tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants The option _FORTIFY_SOURCE, when enabled, can result in some functions being expanded into _chk variants. For example, `fprintf` can get expanded into `__fprintf_chk`. This makes sense for building a real binary, but is not needed for unit tests. In unit test environment all those functions are wrapped. In the example above, both `fprintf` and `__fprintf_chk` needed to be mocked. Disabling _FORTIFY_SOURCE avoids expanding functions into _chk variants, without any loss of testing coverage because that would be wrapped/mocked anyway. This patch also removes two existing _chk wraps because they are not needed anymore. BUG=b:237606255 TEST=ninja test on 1) gcc 12.2.0 on Debian 2) clang 15.0 on Chromium OS Ticket: https://ticket.coreboot.org/issues/411 Change-Id: I70cb1cd90d1f377ff4606acad3c1b514120ae4f7 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68432 Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M tests/meson.build M tests/tests.c 2 files changed, 35 insertions(+), 18 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Peter Marheine: Looks good to me, but someone else must approve diff --git a/tests/meson.build b/tests/meson.build index b0375a1..66adb92 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -69,9 +69,7 @@ '-Wl,--wrap=fsync', '-Wl,--wrap=fread', '-Wl,--wrap=fgets', - '-Wl,--wrap=__fgets_chk', '-Wl,--wrap=fprintf', - '-Wl,--wrap=__vfprintf_chk', '-Wl,--wrap=fclose', '-Wl,--wrap=feof', '-Wl,--wrap=ferror', @@ -115,6 +113,7 @@ cargs, '-ffunction-sections', '-fdata-sections', + '-U_FORTIFY_SOURCE', ], export_dynamic : true, link_args : mocks, diff --git a/tests/tests.c b/tests/tests.c index 53bad65..77cb1ef 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -222,14 +222,6 @@ return NULL; } -char *__wrap___fgets_chk(char *buf, int len, FILE *fp) -{ - LOG_ME; - if (get_io() && get_io()->fgets) - return get_io()->fgets(get_io()->state, buf, len, fp); - return NULL; -} - size_t __wrap_fread(void *ptr, size_t size, size_t nmemb, FILE *fp) { LOG_ME; @@ -282,14 +274,6 @@ return 0; } -int __wrap___vfprintf_chk(FILE *fp, const char *fmt, va_list args) -{ - LOG_ME; - if (get_io() && get_io()->fprintf) - return get_io()->fprintf(get_io()->state, fp, fmt, args); - return 0; -} - int __wrap_fclose(FILE *fp) { LOG_ME; -- To view, visit https://review.coreboot.org/c/flashrom/+/68432 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I70cb1cd90d1f377ff4606acad3c1b514120ae4f7 Gerrit-Change-Number: 68432 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: flashrom.c: Plumb 'all_skipped' global state into func param
by Edward O'Callaghan (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Aarya. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67473 ) Change subject: flashrom.c: Plumb 'all_skipped' global state into func param ...................................................................... Patch Set 6: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/67473/comment/9c57f221_124f07e3 PS1, Line 12: Change-Id: Ic7990b0afebd49d2a27e89b03638b64e36c329df : Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> keep these: ``` Change-Id: I2346c869c47b48604360b0facf9313aae086c8dd Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> ``` and use `git commit --amend --author=Edward O'Callaghan <quasisec(a)google.com>"`. You can use `Co-Author: ` if it is more than a trivial rebase too. -- To view, visit https://review.coreboot.org/c/flashrom/+/67473 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic7990b0afebd49d2a27e89b03638b64e36c329df Gerrit-Change-Number: 67473 Gerrit-PatchSet: 6 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Mon, 07 Nov 2022 00:46:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: programmer: Drop dead fallback_map() boilerplate
by Edward O'Callaghan (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68963 ) Change subject: programmer: Drop dead fallback_map() boilerplate ...................................................................... Patch Set 3: (1 comment) Patchset: PS1: > Needs rebase done. -- To view, visit https://review.coreboot.org/c/flashrom/+/68963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibb7760f807fae040416cef2797a7dbf6572f7df9 Gerrit-Change-Number: 68963 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Mon, 07 Nov 2022 00:41:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom.c: Make 'chip_to_probe' a param to probe_flash()
by Anastasia Klimchuk (Code Review) 06 Nov '22

06 Nov '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67091 ) Change subject: flashrom.c: Make 'chip_to_probe' a param to probe_flash() ...................................................................... flashrom.c: Make 'chip_to_probe' a param to probe_flash() Apart from the very bespoke case of 'probe_w29ee011()' the override 'chip_to_probe' name is a nature parameter to 'probe_flash()'. However we can deal with w29ee011 by providing a probe specific validation function to check if the chip can indeed be overriden. TEST=`./flashrom -p internal --flash-name`. Change-Id: Ifcdace07ea2135d83dea92cfa5c6bec8d7ddf05d Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67091 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M cli_classic.c M flashrom.c M include/chipdrivers.h M include/flash.h M libflashrom.c M w29ee011.c 6 files changed, 49 insertions(+), 18 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/cli_classic.c b/cli_classic.c index b66094c..574afff 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -646,6 +646,7 @@ char *pparam = NULL; struct layout_include_args *include_args = NULL; char *wp_region = NULL; + const char *chip_to_probe = NULL; /* * Safety-guard against a user who has (mistakenly) closed @@ -977,7 +978,7 @@ for (j = 0; j < registered_master_count; j++) { startchip = 0; while (chipcount < (int)ARRAY_SIZE(flashes)) { - startchip = probe_flash(&registered_masters[j], startchip, &flashes[chipcount], 0); + startchip = probe_flash(&registered_masters[j], startchip, &flashes[chipcount], 0, chip_to_probe); if (startchip == -1) break; chipcount++; @@ -1020,7 +1021,7 @@ "chip, using the first one.\n"); for (j = 0; j < registered_master_count; j++) { mst = &registered_masters[j]; - startchip = probe_flash(mst, 0, &flashes[0], 1); + startchip = probe_flash(mst, 0, &flashes[0], 1, chip_to_probe); if (startchip != -1) break; } diff --git a/flashrom.c b/flashrom.c index 694e2c5..db4dd7d 100644 --- a/flashrom.c +++ b/flashrom.c @@ -35,7 +35,6 @@ #include "chipdrivers.h" const char flashrom_version[] = FLASHROM_VERSION; -const char *chip_to_probe = NULL; static const struct programmer_entry *programmer = NULL; @@ -824,7 +823,7 @@ return NULL; } -int probe_flash(struct registered_master *mst, int startchip, struct flashctx *flash, int force) +int probe_flash(struct registered_master *mst, int startchip, struct flashctx *flash, int force, const char *const chip_to_probe) { const struct flashchip *chip; enum chipbustype buses_common; @@ -864,6 +863,10 @@ if (force) break; + if (probe_func == &probe_w29ee011) + if (!w29ee011_can_override(flash->chip->name, chip_to_probe)) + goto notfound; + if (probe_func(flash) != 1) goto notfound; diff --git a/include/chipdrivers.h b/include/chipdrivers.h index 61d43c9..470d1fe 100644 --- a/include/chipdrivers.h +++ b/include/chipdrivers.h @@ -195,6 +195,7 @@ /* w29ee011.c */ int probe_w29ee011(struct flashctx *flash); +bool w29ee011_can_override(const char *const chip_name, const char *const override_chip); /* stm50.c */ int erase_sector_stm50(struct flashctx *flash, unsigned int block, unsigned int blocksize); diff --git a/include/flash.h b/include/flash.h index 2ba235d..da200ea 100644 --- a/include/flash.h +++ b/include/flash.h @@ -475,13 +475,12 @@ /* flashrom.c */ extern const char flashrom_version[]; -extern const char *chip_to_probe; char *flashbuses_to_text(enum chipbustype bustype); int map_flash(struct flashctx *flash); void unmap_flash(struct flashctx *flash); int read_memmapped(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int erase_flash(struct flashctx *flash); -int probe_flash(struct registered_master *mst, int startchip, struct flashctx *fill_flash, int force); +int probe_flash(struct registered_master *mst, int startchip, struct flashctx *flash, int force, const char *const chip_to_probe); int verify_range(struct flashctx *flash, const uint8_t *cmpbuf, unsigned int start, unsigned int len); void emergency_help_message(void); void print_version(void); diff --git a/libflashrom.c b/libflashrom.c index cbc6243..c83c1bd 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -221,8 +221,6 @@ int i, ret = 2; struct flashrom_flashctx second_flashctx = { 0, }; - chip_to_probe = chip_name; /* chip_to_probe is global in flashrom.c */ - *flashctx = malloc(sizeof(**flashctx)); if (!*flashctx) return 1; @@ -230,10 +228,10 @@ for (i = 0; i < registered_master_count; ++i) { int flash_idx = -1; - if (!ret || (flash_idx = probe_flash(&registered_masters[i], 0, *flashctx, 0)) != -1) { + if (!ret || (flash_idx = probe_flash(&registered_masters[i], 0, *flashctx, 0, chip_name)) != -1) { ret = 0; /* We found one chip, now check that there is no second match. */ - if (probe_flash(&registered_masters[i], flash_idx + 1, &second_flashctx, 0) != -1) { + if (probe_flash(&registered_masters[i], flash_idx + 1, &second_flashctx, 0, chip_name) != -1) { flashrom_layout_release(second_flashctx.default_layout); free(second_flashctx.chip); ret = 3; diff --git a/w29ee011.c b/w29ee011.c index 62d7a0f..234b865 100644 --- a/w29ee011.c +++ b/w29ee011.c @@ -15,9 +15,24 @@ */ #include <string.h> +#include <stdbool.h> + #include "flash.h" #include "chipdrivers.h" +bool w29ee011_can_override(const char *const chip_name, const char *const override_chip) +{ + if (!override_chip || strcmp(override_chip, chip_name)) { + msg_cdbg("Old Winbond W29* probe method disabled because " + "the probing sequence puts the AMIC A49LF040A in " + "a funky state. Use 'flashrom -c %s' if you " + "have a board with such a chip.\n", chip_name); + return false; + } + + return true; +} + /* According to the Winbond W29EE011, W29EE012, W29C010M, W29C011A * datasheets this is the only valid probe function for those chips. */ @@ -26,14 +41,6 @@ chipaddr bios = flash->virtual_memory; uint8_t id1, id2; - if (!chip_to_probe || strcmp(chip_to_probe, flash->chip->name)) { - msg_cdbg("Old Winbond W29* probe method disabled because " - "the probing sequence puts the AMIC A49LF040A in " - "a funky state. Use 'flashrom -c %s' if you " - "have a board with such a chip.\n", flash->chip->name); - return 0; - } - /* Issue JEDEC Product ID Entry command */ chip_writeb(flash, 0xAA, bios + 0x5555); programmer_delay(flash, 10); -- To view, visit https://review.coreboot.org/c/flashrom/+/67091 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ifcdace07ea2135d83dea92cfa5c6bec8d7ddf05d Gerrit-Change-Number: 67091 Gerrit-PatchSet: 15 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: tree/: Convert flashchip erase_block func ptr to enumerate
by Edward O'Callaghan (Code Review) 06 Nov '22

06 Nov '22

Attention is currently required from: Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69133 ) Change subject: tree/: Convert flashchip erase_block func ptr to enumerate ...................................................................... Patch Set 5: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/69133/comment/c3c4e5ea_8ceb8630 PS4, Line 7: [ALT] > We can go with these if it's OK with you. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/69133 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I02ae7e4c67c5bf34ec2fd7ffe4af8a2aba6fd5e5 Gerrit-Change-Number: 69133 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 06 Nov 2022 22:24:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022