From 81271ece9c5835ea8c5e347b2cb372717aad7d78 Mon Sep 17 00:00:00 2001 From: Serapheim Dimitropoulos Date: Mon, 21 Aug 2023 16:04:30 -0700 Subject: [PATCH] DLPX-87531 Fix SDB test regression due to drgn API change There were a few changes recently in drgn involving data structures used for holding pointers to type information data. These changes introduced subtle differences in the APIs behavior, one of which affected our test suite. Before when a type was looked up by name and there was no externed/global type with that name but multiple local (source file-specific ones), then one of those local ones was returned consistently. With the recent drgn changes, the behavior is that one of those local types will be randomly picked making one of our regression tests fail sometimes while other times it passed. The specific test is `ptype 'struct v'` which we expect to output: ``` struct v { uint8_t b[32]; } ``` But sometimes we get: ``` struct v { uint8_t b[64]; } ``` The reason is that there are multiple definitions of this struct in the vdev_raidz code of ZFS: ``` ~/repos/zfs$ git grep -B 5 -A 2 "struct v " ...snip... module/zfs/vdev_raidz_math_avx2.c- module/zfs/vdev_raidz_math_avx2.c-extern const uint8_t gf_clmul_mod_lt[4*256][16]; module/zfs/vdev_raidz_math_avx2.c- module/zfs/vdev_raidz_math_avx2.c-#define ELEM_SIZE 32 module/zfs/vdev_raidz_math_avx2.c- module/zfs/vdev_raidz_math_avx2.c:typedef struct v { module/zfs/vdev_raidz_math_avx2.c- uint8_t b[ELEM_SIZE] __attribute__((aligned(ELEM_SIZE))); module/zfs/vdev_raidz_math_avx2.c-} v_t; -- module/zfs/vdev_raidz_math_avx512bw.c- module/zfs/vdev_raidz_math_avx512bw.c-extern const uint8_t gf_clmul_mod_lt[4*256][16]; module/zfs/vdev_raidz_math_avx512bw.c- module/zfs/vdev_raidz_math_avx512bw.c-#define ELEM_SIZE 64 module/zfs/vdev_raidz_math_avx512bw.c- module/zfs/vdev_raidz_math_avx512bw.c:typedef struct v { module/zfs/vdev_raidz_math_avx512bw.c- uint8_t b[ELEM_SIZE] __attribute__((aligned(ELEM_SIZE))); module/zfs/vdev_raidz_math_avx512bw.c-} v_t; ...snip... ``` This patch gets rid of this regression test as it causes occassional failures AND it's not really needed as the functionality that it tests is already covered by other tests. A side-improvement of this patch is also printing in the Github test result output the differences between what we expect the output to be vs what we get from our ref crash dumps. --- .../dump.201912060006/core/ptype thread_union | 6 ++++++ ...pe zfs_case 'struct v' thread_union => ptype zfs_case} | 7 ------- .../dump.202303131823/core/ptype thread_union | 6 ++++++ ...pe zfs_case 'struct v' thread_union => ptype zfs_case} | 7 ------- tests/integration/infra.py | 8 ++++++++ tests/integration/test_core_generic.py | 3 ++- 6 files changed, 22 insertions(+), 15 deletions(-) create mode 100644 tests/integration/data/regression_output/dump.201912060006/core/ptype thread_union rename tests/integration/data/regression_output/dump.201912060006/core/{ptype zfs_case 'struct v' thread_union => ptype zfs_case} (51%) create mode 100644 tests/integration/data/regression_output/dump.202303131823/core/ptype thread_union rename tests/integration/data/regression_output/dump.202303131823/core/{ptype zfs_case 'struct v' thread_union => ptype zfs_case} (51%) diff --git a/tests/integration/data/regression_output/dump.201912060006/core/ptype thread_union b/tests/integration/data/regression_output/dump.201912060006/core/ptype thread_union new file mode 100644 index 00000000..3271d9fd --- /dev/null +++ b/tests/integration/data/regression_output/dump.201912060006/core/ptype thread_union @@ -0,0 +1,6 @@ +union thread_union { + struct task_struct task; + unsigned long stack[2048]; +} +@#$ EXIT CODE $#@ +0 diff --git a/tests/integration/data/regression_output/dump.201912060006/core/ptype zfs_case 'struct v' thread_union b/tests/integration/data/regression_output/dump.201912060006/core/ptype zfs_case similarity index 51% rename from tests/integration/data/regression_output/dump.201912060006/core/ptype zfs_case 'struct v' thread_union rename to tests/integration/data/regression_output/dump.201912060006/core/ptype zfs_case index d9d5efeb..b7d88dfd 100644 --- a/tests/integration/data/regression_output/dump.201912060006/core/ptype zfs_case 'struct v' thread_union +++ b/tests/integration/data/regression_output/dump.201912060006/core/ptype zfs_case @@ -3,12 +3,5 @@ enum zfs_case { ZFS_CASE_INSENSITIVE = 1, ZFS_CASE_MIXED = 2, } -struct v { - uint8_t b[16]; -} -union thread_union { - struct task_struct task; - unsigned long stack[2048]; -} @#$ EXIT CODE $#@ 0 diff --git a/tests/integration/data/regression_output/dump.202303131823/core/ptype thread_union b/tests/integration/data/regression_output/dump.202303131823/core/ptype thread_union new file mode 100644 index 00000000..3271d9fd --- /dev/null +++ b/tests/integration/data/regression_output/dump.202303131823/core/ptype thread_union @@ -0,0 +1,6 @@ +union thread_union { + struct task_struct task; + unsigned long stack[2048]; +} +@#$ EXIT CODE $#@ +0 diff --git a/tests/integration/data/regression_output/dump.202303131823/core/ptype zfs_case 'struct v' thread_union b/tests/integration/data/regression_output/dump.202303131823/core/ptype zfs_case similarity index 51% rename from tests/integration/data/regression_output/dump.202303131823/core/ptype zfs_case 'struct v' thread_union rename to tests/integration/data/regression_output/dump.202303131823/core/ptype zfs_case index 2aa32ee9..b7d88dfd 100644 --- a/tests/integration/data/regression_output/dump.202303131823/core/ptype zfs_case 'struct v' thread_union +++ b/tests/integration/data/regression_output/dump.202303131823/core/ptype zfs_case @@ -3,12 +3,5 @@ enum zfs_case { ZFS_CASE_INSENSITIVE = 1, ZFS_CASE_MIXED = 2, } -struct v { - uint8_t b[32]; -} -union thread_union { - struct task_struct task; - unsigned long stack[2048]; -} @#$ EXIT CODE $#@ 0 diff --git a/tests/integration/infra.py b/tests/integration/infra.py index 39ff9f5e..930416cc 100644 --- a/tests/integration/infra.py +++ b/tests/integration/infra.py @@ -281,6 +281,14 @@ def verify_cmd_output_and_code(self, assert self.repl_invoke(cmd) == ref_code captured = capsys.readouterr() if not stripped: + with capsys.disabled(): + if captured.out != ref_output: + # + # If we are about to fail the assertion print the + # actual mismatch in the logs before failing. + # + print("expected:\n" + ref_output) + print("got:\n" + captured.out) assert captured.out == ref_output else: for i, n in enumerate(captured.out): diff --git a/tests/integration/test_core_generic.py b/tests/integration/test_core_generic.py index 9f439822..8a5ec055 100644 --- a/tests/integration/test_core_generic.py +++ b/tests/integration/test_core_generic.py @@ -125,7 +125,8 @@ # ptype "ptype spa_t", "ptype spa vdev", - "ptype zfs_case 'struct v' thread_union", + "ptype zfs_case", + "ptype thread_union", "ptype 'struct spa'", # sizeof