Bug 215422
| Summary: | bhnd.ko does not build reproducibly | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Ed Maste <emaste> | ||||
| Component: | kern | Assignee: | freebsd-bugs (Nobody) <bugs> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Only Me | Keywords: | patch | ||||
| Priority: | --- | ||||||
| Version: | CURRENT | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| URL: | https://reviews.freebsd.org/D8859 | ||||||
| Attachments: |
|
||||||
It seems nvram_map_gen.sh -d has non-deterministic output.
I ran "sh sys/dev/bhnd/tools/nvram_map_gen.sh sys/dev/bhnd/nvram/nvram_map -d" fifty times, and produced eight different versions of bhnd_nvram_map_data.h.
For example:
% diff -u3 bhnd_nvram_map_data.h.10 bhnd_nvram_map_data.h.28
--- bhnd_nvram_map_data.h.10 2016-12-19 13:43:52.861866000 -0500
+++ bhnd_nvram_map_data.h.28 2016-12-19 13:45:04.762683000 -0500
@@ -620,7 +620,7 @@
/* ag1 (0x75) */
(SPROM_OPCODE_VAR_IMM|BHND_NVAR_AG1_ID),
(SPROM_OPCODE_DO_BIND|0|(1<<SPROM_OP_BIND_SKIP_IN_SHIFT)|(1<<SPROM_OP_BIND_SKIP_OUT_SHIFT)),
- (SPROM_OPCODE_REV_IMM|2),
+ (SPROM_OPCODE_REV_IMM|3),
/* bind (u8 @ 0x75 -> 0x76) */
/* boardflags (0x76) */
(SPROM_OPCODE_VAR_IMM|BHND_NVAR_BOARDFLAGS_ID),
@@ -629,31 +629,30 @@
114,
(SPROM_OPCODE_DO_BIND|0|(1<<SPROM_OP_BIND_SKIP_IN_SHIFT)|(0<<SPROM_OP_BIND_SKIP_OUT_SHIFT)),
/* bind (u16 @ 0x72 -> 0x74) */
- (SPROM_OPCODE_OFFSET|SPROM_OP_DATA_U8),
- 56,
+ /* 0x74 + 0x6 -> 0x7a */
+ (SPROM_OPCODE_OFFSET_REL_IMM|3),
(SPROM_OPCODE_SHIFT|SPROM_OP_DATA_I8),
-16,
(SPROM_OPCODE_DO_BIND|0|(1<<SPROM_OP_BIND_SKIP_IN_SHIFT)|(1<<SPROM_OP_BIND_SKIP_OUT_SHIFT)),
- (SPROM_OPCODE_REV_IMM|3),
- /* bind (u16 @ 0x38 -> 0x3a) */
- /* boardflags (0x3a) */
+ (SPROM_OPCODE_REV_IMM|2),
+ /* bind (u16 @ 0x7a -> 0x7c) */
+ /* boardflags (0x7c) */
(SPROM_OPCODE_VAR_IMM|BHND_NVAR_BOARDFLAGS_ID),
SPROM_OPCODE_TYPE_IMM|BHND_NVRAM_TYPE_UINT16,
(SPROM_OPCODE_OFFSET|SPROM_OP_DATA_U8),
114,
(SPROM_OPCODE_DO_BIND|0|(1<<SPROM_OP_BIND_SKIP_IN_SHIFT)|(0<<SPROM_OP_BIND_SKIP_OUT_SHIFT)),
/* bind (u16 @ 0x72 -> 0x74) */
- /* 0x74 + 0x6 -> 0x7a */
- (SPROM_OPCODE_OFFSET_REL_IMM|3),
+ (SPROM_OPCODE_OFFSET|SPROM_OP_DATA_U8),
+ 56,
(SPROM_OPCODE_SHIFT|SPROM_OP_DATA_I8),
-16,
(SPROM_OPCODE_DO_BIND|0|(1<<SPROM_OP_BIND_SKIP_IN_SHIFT)|(1<<SPROM_OP_BIND_SKIP_OUT_SHIFT)),
- (SPROM_OPCODE_REV_IMM|2),
- /* bind (u16 @ 0x7a -> 0x7c) */
- /* boardnum (0x7c) */
+ /* bind (u16 @ 0x38 -> 0x3a) */
+ /* boardnum (0x3a) */
(SPROM_OPCODE_VAR_REL_IMM|3),
- (SPROM_OPCODE_OFFSET|SPROM_OP_DATA_U8),
- 76,
+ /* 0x3a + 0x12 -> 0x4c */
+ (SPROM_OPCODE_OFFSET_REL_IMM|9),
(SPROM_OPCODE_DO_BIND|0|(1<<SPROM_OP_BIND_SKIP_IN_SHIFT)|(1<<SPROM_OP_BIND_SKIP_OUT_SHIFT)),
(SPROM_OPCODE_REV_IMM|3),
/* bind (u16 @ 0x4c -> 0x4e) */
lidl@ noted the use of srand(), used to select the qsort pivot. The output is reproducible for me with the following diff:
diff --git a/sys/dev/bhnd/tools/nvram_map_gen.awk b/sys/dev/bhnd/tools/nvram_map_gen.awk
index 834bd1f..74fa58f 100755
--- a/sys/dev/bhnd/tools/nvram_map_gen.awk
+++ b/sys/dev/bhnd/tools/nvram_map_gen.awk
@@ -52,9 +52,6 @@ function main(_i) {
AWK_REQ_HEX_PARSING=1
}
- # Seed rand()
- srand()
-
# Output type
OUT_T = null
OUT_T_HEADER = "HEADER"
> lidl@ noted the use of srand(), used to select the qsort pivot.
Rather, rand() is used to select the qsort pivot. The script initializes awk's random number generator using the current time, via srand().
IMO landonf has the right fix in https://reviews.freebsd.org/D8859, although it seems reasonable to apply https://reviews.freebsd.org/D8857 as well. A commit references this bug: Author: landonf Date: Wed Dec 21 00:50:21 UTC 2016 New revision: 310342 URL: https://svnweb.freebsd.org/changeset/base/310342 Log: bhnd(4): Use a stable sort key to produce deterministic nvram_map_gen.awk output. When ordering SROM layout entries, we now use the unique (var_id, rev_start, rev_end) tuple as the sort key; this fixes the previously non-deterministic output when sorting entries with overlapping var_ids. PR: 215422 Reported by: emaste Reviewed by: emaste Approved by: adrian (mentor) Differential Revision: https://reviews.freebsd.org/D8859 Changes: head/sys/dev/bhnd/tools/nvram_map_gen.awk A commit references this bug: Author: emaste Date: Wed Dec 21 15:45:23 UTC 2016 New revision: 310371 URL: https://svnweb.freebsd.org/changeset/base/310371 Log: bhnd: remove srand() to ensure deterministic output r310342 fixed non-deterministic nvram_map_gen.awk output and thus a non- reproducible bhnd(4) build by using a unique sort key. Go one step further and also remove the srand() call. There's no reason we want non-deterministic behaviour from this script. PR: 215422 Reported by: gjb (non-reproducibility of bhnd) Reported by: lidl (srand as the cause) Reviewed by: landonf Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D8857 Changes: head/sys/dev/bhnd/tools/nvram_map_gen.awk |
Created attachment 178108 [details] diffoscope output comparing two bhnd builds Successive iterations of 'make buildkernel' produced differing bhnd.ko & bhnd.ko.debug, as in the attached diffoscope output. Additional details to be added after further investigation.