lolrepeatlol
commented
6 days ago Update:
Retest exclusively with problematic commit
I've attempted the fix purely on the commit c59bf4de01b67184c19a9f6f04caa1a8d5b55afb as additional testing, which is the specific commit from #256 that changes the .S file to the .c file. As expected, the error gets fixed, and klocalizer attempts to look for a configuration from the proper file (debugging prints attached):
alexei@turing:~/LinuxKernels/TesterKernels/linux$ klocalizer --repair .config -a x86_64 --include-mutex c59bf4de01b67184c19a9f6f04caa1a8d5b55afb.diff
klocalizer, kmax 4.8-rc4
INFO: Diff file was given as input ("c59bf4de01b67184c19a9f6f04caa1a8d5b55afb.diff"): assuming it was applied to the Linux source.
WARNING: No cached formulas for e6ccaf255f26 available for download :(
srcfile: arch/s390/crypto/crc32-vx.c, linux_ksrc: ./
srcfile_fullpath: ./arch/s390/crypto/crc32-vx.c
WARNING: There is no Kbuild Makefile in arch/
INFO: Trying the following architectures: x86_64
INFO: Trying "x86_64"
INFO: Searching for a constraints model that cover all or part of the constraints
ERROR: No satisfying configuration found.
(venv) alexei@turing:~/LinuxKernels/TesterKernels/linux$This is in contrast to the current stable version without the fix:
klocalizer --repair .config -a x86_64 --include-mutex c59bf4de01b67184c19a9f6f04caa1a8d5b55afb.diff
klocalizer, kmax v4.7.3
INFO: Diff file was given as input ("c59bf4de01b67184c19a9f6f04caa1a8d5b55afb.diff"): assuming it was applied to the Linux source.
Traceback (most recent call last):
File "/home/alexei/.local/bin/klocalizer", line 1729, in <module>
klocalizerCLI()
File "/home/alexei/.local/bin/klocalizer", line 530, in klocalizerCLI
include_mutex_list = parse_units_args(include_mutex_arg)
File "/home/alexei/.local/bin/klocalizer", line 514, in parse_units_args
parsed_list.extend(list(get_patch_target_lines(a).items()))
File "/home/alexei/.local/bin/klocalizer", line 377, in get_patch_target_lines
r = patch.get_target_c_lines(patch_txt)
File "/home/alexei/.local/share/pipx/venvs/kmax/lib/python3.8/site-packages/kmax/patch.py", line 244, in get_target_c_lines
r = get_target_lines(patch_txt)
File "/home/alexei/.local/share/pipx/venvs/kmax/lib/python3.8/site-packages/kmax/patch.py", line 200, in get_target_lines
patch_summary = summarize_patch(patch_txt)
File "/home/alexei/.local/share/pipx/venvs/kmax/lib/python3.8/site-packages/kmax/patch.py", line 107, in summarize_patch
assert SourceFileType.get_file_type(old_path) == SourceFileType.get_file_type(new_path)
AssertionErrorTest with arbitrary commit
To ensure that no issues cropped up as part of me having changed the code, I tried the current version of klocalizer 4.8rc4 as well as my fix on master on a completely unrelated commit, 16005147cca41a0f67b5def2a4656286f8c0db4a. I found few differences (both files were within 3 lines of one another) between the two generated 0-x86_64.config files. That being said, there were differences, even when the configurations were sorted -- for instance, the configuration file that my fix applied did not have the CONFIG_MFD_ARIZONA option set to y.
klocalizer runs successfully on both versions, though:
(venv) alexei@turing:~/LinuxKernels/TesterKernels/linux$ klocalizer --repair .config -a x86_64 --include-mutex 16005147cca41a0f67b5def2a4656286f8c0db4a.diff
klocalizer, kmax 4.8-rc4
INFO: Diff file was given as input ("16005147cca41a0f67b5def2a4656286f8c0db4a.diff"): assuming it was applied to the Linux source.
WARNING: No cached formulas for fa064fde94ac available for download :(
srcfile: fs/bcachefs/fs.c, linux_ksrc: ./
srcfile_fullpath: ./fs/bcachefs/fs.c
srcfile: fs/bcachefs/fsck.c, linux_ksrc: ./
srcfile_fullpath: ./fs/bcachefs/fsck.c
INFO: Trying the following architectures: x86_64
INFO: Trying "x86_64"
INFO: Computing the line presence conditions under x86_64 for the following units: fs/bcachefs/fs.o
INFO: Computing line presence conditions for "fs/bcachefs/fs.c"
[STEP 1/3] reading kextract file
[STEP 1/3] finished reading kextract file
[STEP 2/3] translated 17658/17658 configuration option dependencies
[STEP 3/3] converted 17636/17636 constraints to smtlib2 format
[STEP 3/3] pickling the map
[STEP 3/3] done
INFO: Searching for a constraints model that cover all or part of the constraints
INFO: A coverage improving constraints set found: sampling a model.
INFO: Approximating via unsat core approach.
INFO: Found satisfying config by removing 18 assumptions.
INFO: Sampling and writing 1 configuration files from models.
INFO: [1/1] Sampling and writing the configuration file to ./0-x86_64.config
INFO: Build with "KCONFIG_CONFIG=./0-x86_64.config make.cross ARCH=x86_64 olddefconfig clean fs/bcachefs/fs.o fs/bcachefs/fsck.o"
INFO: Coverage summary: all mutex constraints were satisfied.
INFO: Writing the coverage report to coverage_report.json
INFO: Note coverage report is attempted coverage. Actual coverage may be reduced due to limitations of klocalizer's constraint collection.
(venv) alexei@turing:~/LinuxKernels/TesterKernels/linux$While I'm somewhat confident in this fix, I'd recommend some more testing, especially as I had only tested with klocalizer and not any other tool. In addition, given that there was slight variation between the two configuration files, I'm hesitant to say this fix is foolproof. I'm not entirely sure if some level of variation is expected.
Quick update: I've tested again between 4.8rc4 and my fix applied to master, and while the generated configuration files are still a little bit different, the coverage_report.json and coverage_results.json files have both proven to be the exact same. I'm confident this fix can be merged in its current state and apologize for the textwall. :)
At times, the Linux kernel can encounter modifications to file types that will change their
SourceFileTypefromSOURCEtoOTHER, as seen in #256:This change to the code should account for those modifications, provided that the
FileChangeTypeis already set asMOVED_MODIFIED.Important notes:
kmaxwill run into the same error detailed in #274.That being said, it's worth noting that applying the commits brought up in the issue will cause the program run into a different error after applying this change. I currently believe the error is unrelated, but I will continue debugging & testing before turning this into a full PR (some debug prints added as well):I confirmed that the above error is an entirely separate bug in #277.