| Message ID | 20200112163339.218205-3-xypron.glpk@gmx.de |
|---|---|
| State | New |
| Headers | show |
| Series | cli: allow verbatim character entry with CTRL-v | expand |
On Sun, Jan 12, 2020 at 05:33:39PM +0100, Heinrich Schuchardt wrote: > Provide a unit test checking that CTRL-V can be used to add control > characters to the line buffer. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> I don't know why but this test, run when built with clang fails. A log is here: https://gitlab.denx.de/u-boot/u-boot/-/jobs/50540 but I saw it on Travis and Azure as well.
On 1/25/20 12:43 AM, Tom Rini wrote: > On Sun, Jan 12, 2020 at 05:33:39PM +0100, Heinrich Schuchardt wrote: > >> Provide a unit test checking that CTRL-V can be used to add control >> characters to the line buffer. >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > > I don't know why but this test, run when built with clang fails. A log > is here: https://gitlab.denx.de/u-boot/u-boot/-/jobs/50540 but I saw it > on Travis and Azure as well. > Hello Tom, hello Simon when I clang U-Boot branch bisect-failure with in the Gitlab Docker image and run ./u-boot -d arch/sandbox/dts/test.dtb ut dm usb_keyb the test runs fine. When I run valgrind ./u-boot -d arch/sandbox/dts/test.dtb usb start ut dm usb_keyb I see more than 100 use-after-free errors. dm_test_destroy() calling uclass_destroy() seems to be the culprit. @Simon: Why would we destroy any uclass in `ut dm` that was in use before executing the command? @Marek Aren't usb_kbd_remove() and usb_kbd_deregister() essentially duplicate coding? ==149271== Invalid write of size 4 ==149271== at 0x42CC04: usb_kbd_put_queue (usb_kbd.c:0) ==149271== by 0x42CC04: usb_kbd_translate (usb_kbd.c:255) ==149271== by 0x42C871: usb_kbd_irq_worker (usb_kbd.c:321) ==149271== by 0x42C7DF: usb_kbd_poll_for_event (usb_kbd.c:358) ==149271== by 0x42C7DF: usb_kbd_testc (usb_kbd.c:410) ==149271== by 0x42EF60: console_tstc (console.c:206) ==149271== by 0x50F302: dm_test_usb_keyb (usb.c:0) ==149271== by 0x4F17B4: dm_do_test (test-main.c:103) ==149271== by 0x4F1329: dm_test_main (test-main.c:180) ==149271== by 0x4F1329: do_ut_dm (test-main.c:210) ==149271== by 0x437A3B: cmd_call (command.c:576) ==149271== by 0x437A3B: cmd_process (command.c:631) ==149271== by 0x423DF2: run_pipe_real (cli_hush.c:1678) ==149271== by 0x423DF2: run_list_real (cli_hush.c:1876) ==149271== by 0x422FE2: run_list (cli_hush.c:2025) ==149271== by 0x422FE2: parse_stream_outer (cli_hush.c:3217) ==149271== by 0x423207: parse_file_outer (cli_hush.c:3300) ==149271== by 0x436EAA: cli_loop (cli.c:221) ==149271== Address 0xa681ca4 is 36 bytes inside a block of size 104 free'd ==149271== at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==149271== by 0x42C6C9: usb_kbd_remove (usb_kbd.c:676) ==149271== by 0x442947: device_remove (device-remove.c:184) ==149271== by 0x442910: device_chld_remove (device-remove.c:52) ==149271== by 0x442910: device_remove (device-remove.c:175) ==149271== by 0x442910: device_chld_remove (device-remove.c:52) ==149271== by 0x442910: device_remove (device-remove.c:175) ==149271== by 0x442910: device_chld_remove (device-remove.c:52) ==149271== by 0x442910: device_remove (device-remove.c:175) ==149271== by 0x4415DC: uclass_destroy (uclass.c:121) ==149271== by 0x4F17DA: dm_test_destroy (test-main.c:72) ==149271== by 0x4F17DA: dm_do_test (test-main.c:107) ==149271== by 0x4F12CB: dm_test_main (test-main.c:169) ==149271== by 0x4F12CB: do_ut_dm (test-main.c:210) ==149271== by 0x437A3B: cmd_call (command.c:576) ==149271== by 0x437A3B: cmd_process (command.c:631) ==149271== by 0x423DF2: run_pipe_real (cli_hush.c:1678) ==149271== by 0x423DF2: run_list_real (cli_hush.c:1876) ==149271== by 0x422FE2: run_list (cli_hush.c:2025) ==149271== by 0x422FE2: parse_stream_outer (cli_hush.c:3217) Best regards Heinrich
Hi Heinrich, On Sat, 25 Jan 2020 at 03:45, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > On 1/25/20 12:43 AM, Tom Rini wrote: > > On Sun, Jan 12, 2020 at 05:33:39PM +0100, Heinrich Schuchardt wrote: > > > >> Provide a unit test checking that CTRL-V can be used to add control > >> characters to the line buffer. > >> > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > > > > I don't know why but this test, run when built with clang fails. A log > > is here: https://gitlab.denx.de/u-boot/u-boot/-/jobs/50540 but I saw it > > on Travis and Azure as well. > > > > Hello Tom, hello Simon > > when I clang U-Boot branch bisect-failure with in the Gitlab Docker > image and run > > ./u-boot -d arch/sandbox/dts/test.dtb > ut dm usb_keyb > > the test runs fine. > > When I run > > valgrind ./u-boot -d arch/sandbox/dts/test.dtb > usb start > ut dm usb_keyb > > I see more than 100 use-after-free errors. > > dm_test_destroy() calling uclass_destroy() seems to be the culprit. > > @Simon: > Why would we destroy any uclass in `ut dm` that was in use before > executing the command? This is because the tests might require them to be in their initial state. In general running a unit test along with other code is not safe. I wonder if the situation has improved with the move to SDL2, but I'm not sure. [..] Regards, Simon
diff --git a/test/dm/Makefile b/test/dm/Makefile index dd1ceff86c..0261424168 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -51,7 +51,7 @@ obj-$(CONFIG_DM_SPI_FLASH) += sf.o obj-$(CONFIG_SMEM) += smem.o obj-$(CONFIG_DM_SPI) += spi.o obj-y += syscon.o -obj-$(CONFIG_DM_USB) += usb.o +obj-$(CONFIG_DM_USB) += usb.o usb_console.o obj-$(CONFIG_DM_PMIC) += pmic.o obj-$(CONFIG_DM_REGULATOR) += regulator.o obj-$(CONFIG_TIMER) += timer.o diff --git a/test/dm/usb_console.c b/test/dm/usb_console.c new file mode 100644 index 0000000000..c62a05291d --- /dev/null +++ b/test/dm/usb_console.c @@ -0,0 +1,150 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2019 Heinrich Schuchardt <xypron.glpk at gmx.de> + * + * Tests for the command line interface using the USB keyboard as input. + */ + +#include <common.h> +#include <cli.h> +#include <asm/state.h> +#include <asm/test.h> +#include <dm/test.h> +#include <test/ut.h> +#include <usb.h> + +#define SCAN_CODE_END_OF_BUFFER 0xff + +struct console_test_data { + const char modifiers; + const unsigned char scancode; +}; + +/** + * put_buffer() - copy key strokes to USB keyboard buffer + * + * @key_strokes: key strokes + * Return: 0 = ok + */ +static int put_buffer(const struct console_test_data *key_strokes, + struct unit_test_state *uts) +{ + struct udevice *dev; + const struct console_test_data *pos; + + ut_assertok(uclass_get_device_by_name(UCLASS_USB_EMUL, "keyb at 3", + &dev)); + for (pos = key_strokes; pos->scancode != SCAN_CODE_END_OF_BUFFER; + ++pos) { + char scancodes[USB_KBD_BOOT_REPORT_SIZE] = {0}; + + scancodes[0] = pos->modifiers; + scancodes[2] = pos->scancode; + + ut_assertok(sandbox_usb_keyb_add_string(dev, scancodes)); + } + ut_asserteq(1, tstc()); + + return 0; +} + +/** + * usb_console_setup() - setup for usb keyboard test + * + * Return: 0 = ok + */ +static int usb_console_setup(struct unit_test_state *uts) +{ + state_set_skip_delays(true); + ut_assertok(usb_init()); + + /* Initially there should be no characters */ + ut_asserteq(0, tstc()); + + return 0; +} + +/** + * usb_console_teardown() - tear down after usb keyboard test + * + * Return: 0 = ok + */ +static int usb_console_teardown(struct unit_test_state *uts) +{ + ut_assertok(usb_stop()); + return 0; +} + +/** + * dm_test_usb_console_verbatim() - test verbatim entry in console + * + * This test copies the key strokes + * + * setenv error '<CTRL-V><ESC>[33;1mError<CTRL-V><ESC>[0m'<ENTER> + * + * into the buffer of the USB keyboard emulation driver. It then checks if the + * string is read into a command line interface buffer as + * + * "setenv error '\x1b[33;1mError\x1b[0m'" + * + * This confirms that <CTRL-V> can be used for verbatim input of control + * characters. + * + * @uts: unit test state + * Return: 0 on success + */ +static int dm_test_usb_console_verbatim(struct unit_test_state *uts) +{ + char buffer[CONFIG_SYS_CBSIZE + 1]; /* console I/O buffer */ + const struct console_test_data con_test_data[] = { + {0x00, 0x16}, /* s */ + {0x00, 0x08}, /* e */ + {0x00, 0x17}, /* t */ + {0x00, 0x08}, /* e */ + {0x00, 0x11}, /* n */ + {0x00, 0x19}, /* v */ + {0x00, 0x2c}, /* */ + {0x00, 0x08}, /* e */ + {0x00, 0x15}, /* r */ + {0x00, 0x00}, /* */ + {0x00, 0x15}, /* r */ + {0x00, 0x12}, /* o */ + {0x00, 0x15}, /* r */ + {0x00, 0x2c}, /* */ + {0x00, 0x34}, /* ' */ + {0x01, 0x19}, /* <CONTROL><V> */ + {0x00, 0x29}, /* <ESC> */ + {0x00, 0x2F}, /* [ */ + {0x00, 0x20}, /* 3 */ + {0x00, 0x00}, /* */ + {0x00, 0x20}, /* 3 */ + {0x00, 0x33}, /* ; */ + {0x00, 0x1E}, /* 1 */ + {0x00, 0x10}, /* m */ + {0x20, 0x08}, /* E */ + {0x00, 0x15}, /* r */ + {0x00, 0x00}, /* */ + {0x00, 0x15}, /* r */ + {0x00, 0x12}, /* o */ + {0x00, 0x15}, /* r */ + {0x01, 0x19}, /* <CONTROL><V> */ + {0x00, 0x29}, /* <ESC> */ + {0x00, 0x2F}, /* [ */ + {0x00, 0x27}, /* 0 */ + {0x00, 0x10}, /* m */ + {0x00, 0x34}, /* ' */ + {0x00, 0x28}, /* <ENTER> */ + /* End of list */ + {0x00, SCAN_CODE_END_OF_BUFFER} + }; + + ut_assertok(usb_console_setup(uts)); + ut_assertok(put_buffer(con_test_data, uts)); + ut_asserteq(31, cli_readline_into_buffer("", buffer, 0)); + ut_asserteq_str("setenv error '\x1b[33;1mError\x1b[0m'", buffer); + ut_asserteq(0, tstc()); + ut_assertok(usb_console_teardown(uts)); + + return 0; +} +DM_TEST(dm_test_usb_console_verbatim, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
Provide a unit test checking that CTRL-V can be used to add control characters to the line buffer. Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> --- test/dm/Makefile | 2 +- test/dm/usb_console.c | 150 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 test/dm/usb_console.c -- 2.24.1