On Wed, Mar 4, 2020 at 2:26 AM Patricia Alfonso trishalfonso@google.com wrote:
On Sat, Feb 29, 2020 at 10:29 PM Dmitry Vyukov dvyukov@google.com wrote:
On Sat, Feb 29, 2020 at 2:23 AM Patricia Alfonso trishalfonso@google.com wrote:
On Thu, Feb 27, 2020 at 3:44 AM 'Patricia Alfonso' via kasan-dev kasan-dev@googlegroups.com wrote:
--- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -141,7 +141,7 @@ class LinuxSourceTree(object): return True
def run_kernel(self, args=[], timeout=None, build_dir=''):
args.extend(['mem=256M'])
args.extend(['mem=256M', 'kasan_multi_shot'])This is better done somewhere else (different default value if KASAN_TEST is enabled or something). Or overridden in the KASAN tests. Not everybody uses tools/testing/kunit/kunit_kernel.py and this seems to be a mandatory part now. This means people will always hit this, be confused, figure out they need to flip the value, and only then be able to run kunit+kasan.
I agree. Is the best way to do this with "bool multishot = kasan_save_enable_multi_shot();" and "kasan_restore_multi_shot(multishot);" inside test_kasan.c like what was done in the tests before?
This will fix KASAN tests, but not non-KASAN tests running under KUNIT and triggering KASAN reports. You set kasan_multi_shot for all KUNIT tests. I am reading this as that we don't want to abort on the first test that triggered a KASAN report. Or not?
I don't think I understand the question, but let me try to explain my thinking and see if that resonates with you. We know that the KASAN tests will require more than one report, and we want that. For most users, since a KASAN error can cause unexpected kernel behavior for anything after a KASAN error, it is best for just one unexpected KASAN error to be the only error printed to the user, unless they specify kasan-multi-shot. The way I understand it, the way to implement this is to use "bool multishot = kasan_save_enable_multi_shot();" and "kasan_restore_multi_shot(multishot);" around the KASAN tests so that kasan-multi-shot is temporarily enabled for the tests we expect multiple reports. I assume "kasan_restore_multi_shot(multishot);" restores the value to what the user input was so after the KASAN tests are finished, if the user did not specify kasan-multi-shot and an unexpected kasan error is reported, it will print the full report and only that first one. Is this understanding correct? If you have a better way of implementing this or a better expected behavior, I appreciate your thoughts.
Everything you say is correct. What I tried to point at is that this new behavior is different from the original behavior of your change. Initially you added kasan_multi_shot to command line for _all_ kunit tests (not just KASAN). The question is: do we want kasan_multi_shot for non-KASAN tests or not?