Skip to content

Commit

Permalink
DLPX-87531 Fix SDB test regression due to drgn API change (#339)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sdimitro authored Aug 22, 2023
1 parent e7e1f49 commit 7e9225c
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
union thread_union {
struct task_struct task;
unsigned long stack[2048];
}
@#$ EXIT CODE $#@
0
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
union thread_union {
struct task_struct task;
unsigned long stack[2048];
}
@#$ EXIT CODE $#@
0
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 8 additions & 0 deletions tests/integration/infra.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/test_core_generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 7e9225c

Please sign in to comment.