pkg/bisect: detect incorrect bisections #1271
Comments
|
If the bisection result is not "certainly incorrect" but rather just "probably incorrect", IMO it needs to undergo human review before being mailed out. Otherwise syzbot will continue spamming too many wrong results, decreasing the percent of correct results that get looked into. So I suggest just not automatically mailing out "probably incorrect" results, but leaving them available on the syzbot dashboard and marked as such. |
|
Makes sense, thanks. |
|
Detecting comment-only changes is not possible looking at the patch alone. It would require getting full file copy before/after the patch and then doing some analysis. |
Kbuild inserts build timestamp into every build. This makes vmlinux change always even if no present. Make the build more deterministic. We plan to use it for detecting no-op changes during bisection. Update #1271
Add optional build signature for images, currently only implemented for linux. This can be used in bisection process to detect changes that does not affect kernel. Update #1271
|
@zsm-oss FYI I am going to tackle this, just so that we don't step on each other. This is going to be useful for fix bisection too. |
Detect bisection to merge commits and to commits that don't affect kernel binary (comments, other arches, whitespaces, etc). Such bisections are not reported in emails (but shown on web). Update #1271
|
This should be mostly done. |
Show [report pending] for fix bisections that we will send. Help to analyze what will happen when we enable mailing of fix bisections. Update #1271
|
One case of non-deterministic/non-hermetic kernel build: Trying to reproduce this locally with: building on commits 910cd32e552ea09caa89cdbe328e468979b030dd and 4f4acc9472e54ce702f1d85fc9e6d57767dec91f. I see small content diffs in .rodata and .init.data. |
|
The .rodata diff is: .init.data: |
|
The .rodata diff points somewhere into kallsyms_addresses? |
|
For .init.data I can't figure what it belongs to: That's 0xffffffff830d7000 + 0xa1670 = 0xffffffff83178670 |
3/4 added tests currently fail. The problem is that we don't collect results from initial bisection range detection. As the result we won't detect "same binary" for release and first commit in a release. Update #1271
They should have been detected by "same binary" logic. But the problem is that we may use different compilers for different commits and they switch exactly at release commits. So we can build the release with a differnet compiler than the rest of commits and then obviously it won't be "same binary". Detect release commits separately. Update #1271
|
Weird case or I don't understand something: Another similar case: |
|
Moved the git bisect parent commit issue to #1527 (too much context and info). |
|
Another non-deterministic build: |
|
init.data part of this bug could be due to initramfs which is part of vmlinux. if CONFIG_BLK_DEV_INITRD is enabled in configuration and CONFIG_INITRAMFS_SOURCE is not -> small default initramfs is generated. This is in usr/Makefile. For some reason content of this generated initramfs in init.data is varying from build to build. This could be easily get rid of by disabling CONFIG_BLK_DEV_INITRD from syzbot configs? |
|
Good find! |
|
I guess it's this guy in usr/gen_init_cpio.c which means we can't fix it in the older kernels. I hoped for some env var or something similar. |
|
I think we need to remove CONFIG_BLK_DEV_INITRD in pkg/vcs/linux.go:linuxAlterConfigs. This will auto-heal all bisections even with already stored configs with CONFIG_BLK_DEV_INITRD=y. Jouni you previously provided a link to some page called along the lines of "Deterministic Kernel Builds". Can make sense to notify that group about the issue with CONFIG_BLK_DEV_INITRD. |
Yes, I will ping them or update the document by myself. |
@hogander-unikie will you send a PR for this? Want to understand if I need to keep track of this or I can wait for a PR. |
Yes, I will prepare pull request for this. |
Branched off #1051
We could detect incorrect bisection results based on some patch characteristics. The following checks were proposed:
These seem to suggest 2 types of incorrect results:
Open question: do we want to handle them differently? E.g. a change bisected to syscall.*.tbl change is a relevant bit of information too.
Open question: what to do with merges? Usually we falsely point to merges because of flaky crashes or disabled configs. However, they can as well be the right commit. They can also belong to type 2. But they also can be the ultimately correct commit too (depending on what the actual merge did).
The text was updated successfully, but these errors were encountered: