Page MenuHomeFreeBSD

Fix sys.kern.coredump_phnum_test.coredump_phnum on i386

Authored by lwhsu on Dec 10 2018, 6:04 AM.
Referenced Files
Unknown Object (File)
Mon, Jun 17, 1:50 PM
Unknown Object (File)
Mon, Jun 17, 1:49 PM
Unknown Object (File)
Mon, Jun 17, 1:36 PM
Unknown Object (File)
May 18 2024, 12:06 PM
Unknown Object (File)
May 12 2024, 6:52 PM
Unknown Object (File)
May 8 2024, 9:54 PM
Unknown Object (File)
Apr 20 2024, 1:56 PM
Unknown Object (File)
Apr 19 2024, 8:47 PM



The test code wanted to find "0000000000000001 .* 66[0-9]{3} " in readelf -S
output, but it is too long on 32-bit architectures. Change to grab the wanted
section header information and do the actually intended comparsion.

Further information:

Failure information:

Output on amd64:

root@:/usr/tests/sys/kern # readelf -S coredump_phnum_helper.core | grep -A1 "^  \[ 0\] "
  [ 0] <no-name>         NULL             0000000000000000  00000000
       0000000000000001  0000000000000000           0   66546     0

Output on i386:

root@:/usr/tests/sys/kern # readelf -S coredump_phnum_helper.core | grep -A1 "^  \[ 0\] "
  [ 0] <no-name>         NULL            00000000 000000 000001 00      0 66545  0
Key to Flags:

We can also use:

atf_check -x "test `readelf -SW coredump_phnum_helper.core awk '{print $10}'` -gt 65535"


atf_check -o "match: 000001 .* 66[0-9]{3} " \
	-x 'readelf -SW coredump_phnum_helper.core | grep "^  \[ 0\] "'

but I feel the proposed one is easier to read. I'm also fine to chenge to other
approach to follow the style.

Test Plan

cd /usr/tests/sys/kern && kyua debug coredump_phnum_test:coredump_phnum

Diff Detail

rS FreeBSD src repository - subversion
Lint Passed
No Test Coverage
Build Status
Buildable 21468
Build 20786: arc lint + arc unit

Event Timeline

I'm not familiar with this tool or the test anymore, and not interested in studying it again now. I'm sure your fix is fine, but I am not a good reviewer.


This is a bit fragile since you're assuming that the core file contains exactly one section. We should probably verify this first.

Also, as a minor nit, I would grep for ^[[:space:]]*sh_info:.

ngie requested changes to this revision.Dec 12 2018, 4:58 PM

I remember this test being fragile because it relies on output from a third-party application under a large amount of development and makes assumptions based on a hardcoded value which cannot be determined programmatically. This is something that I tried getting more context in the past, but the ball was dropped with this item. I very much understand how this is broken though, and I will try to think of a better way to check for this (the right solution might involve writing a C program which emulates a stripped down version of readelf, etc).

I don't think we should move forward with this change. In particular, your proposed approach will be hard to diagnose after the fact as well if the number of columns output or other formatting changes: the test will fail with difficult to understand diagnostic messages (in particular if the field is non-numeric); you can't test readelf to make sure it's executing cleanly; etc.

This revision now requires changes to proceed.Dec 12 2018, 4:58 PM

readelf is under our control and is largely unchanging, we should be able to rely on it

readelf is under our control and is largely unchanging, we should be able to rely on it

elfdump we do as well, but I'm a bit nervous about switching over to elfdump from readelf.

I've proposed a different solution in D20005.