From 50006b1cd745c1d12d778d86c6e8272aa0f02207 Mon Sep 17 00:00:00 2001 From: Dan Liew Date: Wed, 5 Nov 2025 17:33:20 -0800 Subject: [PATCH] [BoundsSafety] Set the default bounds check mode for driver tests Previously the `%clang` substitution was using the default bounds check mode rather than the default of the diretory (i.e. new checks for `test/BoundsSafety` and legacy checks for `BoundsSafety-legacy-checks`). This patch 1. Adds the relevant flag bounds check flag to the `%clang` substitution. 2. Adds a new `%clang_no_bounds_check_mode_specified` substitution. This is the same as `%clang` but it does not have the bounds check flag present (e.g. `-fbounds-safety-bringup-missing-checks=all`). This is needed for some test cases. Note some tests in `BoundsSafety/Frontend` needed fixing because they were trying to invoke cc1 using `%clang` instead of `%clang_cc1`. rdar://164123351 --- .../lit.local.cfg.py | 63 +++++++++++++---- .../Driver/bounds-safety-attributes.c | 2 +- clang/test/BoundsSafety/Driver/driver.c | 4 +- clang/test/BoundsSafety/Frontend/cmdl_opts.c | 6 +- .../Frontend/objc_experimental_is_supported.m | 2 +- ...rimental_without_bounds_safety_ignored.cpp | 2 +- .../Frontend/only_c_is_supported.c | 8 +-- clang/test/BoundsSafety/lit.local.cfg.py | 69 +++++++++++++++++++ 8 files changed, 130 insertions(+), 26 deletions(-) create mode 100644 clang/test/BoundsSafety/lit.local.cfg.py diff --git a/clang/test/BoundsSafety-legacy-checks/lit.local.cfg.py b/clang/test/BoundsSafety-legacy-checks/lit.local.cfg.py index 69b13058285e3..24e22ff0ab2ed 100644 --- a/clang/test/BoundsSafety-legacy-checks/lit.local.cfg.py +++ b/clang/test/BoundsSafety-legacy-checks/lit.local.cfg.py @@ -1,27 +1,62 @@ +import lit import os +# clang/test uses external shell by default, but all tests in clang/test/BoundsSafety +# support lit's internal shell, so use that instead +config.test_format = lit.formats.ShTest(False) + cc1_substituted = [ False ] +clang_substituted = [ False ] extra_flags = '-fno-bounds-safety-bringup-missing-checks=all' -def patch_cc1(sub_tuple): +def get_subst(subst_name): + subst_re = r'%\b{}\b'.format(subst_name) + for subst_name, value in config.substitutions: + if subst_re not in subst_name: + continue + return value + raise Exception(f'Could not find %{subst_name} substitution') + +def patch_subst_impl(sub_tuple, subst_to_patch): assert isinstance(sub_tuple, tuple) assert len(sub_tuple) == 2 subst_name = sub_tuple[0] - # Match `%clang_cc1`. The `ToolSubst` class inside `lit` adds a bunch of - # regexes around this substitution so we try to match them here to avoid - # matching a substitution that has `%clang_cc1` as a substring. This is - # fragile - if r'%\bclang_cc1\b' not in subst_name: - return sub_tuple + # Match `%subst_name` (e.g. `%clang_cc1`). The `ToolSubst` class inside + # `lit` adds a bunch of regexes around this substitution so we try to match + # them here to avoid matching a substitution that has `%clang_cc1` as a + # substring. This is fragile + subst_re = r'%\b{}\b'.format(subst_to_patch) + if subst_re not in subst_name: + return (sub_tuple[0], sub_tuple[1], False) patched_sub = sub_tuple[1] + f' {extra_flags}' - cc1_substituted[0] = True - return (sub_tuple[0], patched_sub) + return (sub_tuple[0], patched_sub, True) + +def patch_cc1_subst(sub_tuple): + subst_name, subst, patched = patch_subst_impl(sub_tuple, 'clang_cc1') + if patched: + cc1_substituted[0] = True + return (subst_name, subst) + +def patch_clang_subst(sub_tuple): + subst_name, subst, patched = patch_subst_impl(sub_tuple, 'clang') + if patched: + clang_substituted[0] = True + return (subst_name, subst) + + +# Provide an un-patched substitution +default_clang_subst = get_subst('clang') +config.substitutions.append( + (r'%\bclang_no_bounds_check_mode_specified\b', default_clang_subst) +) -config.substitutions = list(map(patch_cc1, config.substitutions)) +config.substitutions = list(map(patch_cc1_subst, config.substitutions)) +config.substitutions = list(map(patch_clang_subst, config.substitutions)) dir_with_patched_sub = os.path.dirname(__file__) -if not cc1_substituted[0]: - lit_config.fatal(f'Failed to add `{extra_flags}` to %clang_cc1 invocations under {dir_with_patched_sub}') -else: - lit_config.note(f'Implicitly passing `{extra_flags}` to %clang_cc1 invocations under {dir_with_patched_sub}') +for subst, patched in [('%clang_cc1', cc1_substituted[0]), ('%clang', clang_substituted[0])]: + if not patched: + lit_config.fatal(f'Failed to add `{extra_flags}` to {subst} invocations under {dir_with_patched_sub}') + else: + lit_config.note(f'Implicitly passing `{extra_flags}` to {subst} invocations under {dir_with_patched_sub}') diff --git a/clang/test/BoundsSafety/Driver/bounds-safety-attributes.c b/clang/test/BoundsSafety/Driver/bounds-safety-attributes.c index 725a3f8ac368c..576e64a178c4c 100644 --- a/clang/test/BoundsSafety/Driver/bounds-safety-attributes.c +++ b/clang/test/BoundsSafety/Driver/bounds-safety-attributes.c @@ -4,7 +4,7 @@ // RUN: %clang -c %s -### 2>&1 | not grep fexperimental-bounds-safety-attributes -// RUN: %clang -fexperimental-bounds-safety-attributes -c %s -### 2>&1 | not grep fbounds-safety +// RUN: %clang_no_bounds_check_mode_specified -fexperimental-bounds-safety-attributes -c %s -### 2>&1 | not grep fbounds-safety // RUN: %clang -fexperimental-bounds-safety-attributes -c %s -### 2>&1 | FileCheck -check-prefixes ALL,T0 %s // T0: -fexperimental-bounds-safety-attributes diff --git a/clang/test/BoundsSafety/Driver/driver.c b/clang/test/BoundsSafety/Driver/driver.c index 6a9dc44990812..839380e9371ca 100644 --- a/clang/test/BoundsSafety/Driver/driver.c +++ b/clang/test/BoundsSafety/Driver/driver.c @@ -1,13 +1,13 @@ // ALL-NOT: unknown argument // Warning: careful to not grep some directory in the buid -// RUN: %clang -c %s -### 2>&1 | not grep fbounds-safety +// RUN: %clang_no_bounds_check_mode_specified -c %s -### 2>&1 | not grep fbounds-safety // RUN: %clang -fbounds-safety -### %s 2>&1 | FileCheck -check-prefixes ALL,T0 %s // T0: -fbounds-safety // T0: -enable-constraint-elimination -// RUN: %clang -fbounds-safety -fno-bounds-safety -c %s -### 2>&1 | not grep -e fbounds-safety -e enable-constraint-elimination +// RUN: %clang_no_bounds_check_mode_specified -fbounds-safety -fno-bounds-safety -c %s -### 2>&1 | not grep -e fbounds-safety -e enable-constraint-elimination // RUN: %clang -fno-bounds-safety -fbounds-safety -c %s -### 2>&1 | FileCheck -check-prefixes ALL,T4 %s // T4: -fbounds-safety diff --git a/clang/test/BoundsSafety/Frontend/cmdl_opts.c b/clang/test/BoundsSafety/Frontend/cmdl_opts.c index 40df17e7fb711..db8015f75d09b 100644 --- a/clang/test/BoundsSafety/Frontend/cmdl_opts.c +++ b/clang/test/BoundsSafety/Frontend/cmdl_opts.c @@ -2,8 +2,8 @@ // Making sure the Frontend accepts the -fbounds-safety flags - it would otherwise produce error: unknown argument '...' -// RUN: %clang -cc1 -fbounds-safety -fsyntax-only %s +// RUN: %clang_cc1 -fbounds-safety -fsyntax-only %s -// RUN: %clang -cc1 -fbounds-safety -fexperimental-bounds-safety-cxx -fsyntax-only %s +// RUN: %clang_cc1 -fbounds-safety -fexperimental-bounds-safety-cxx -fsyntax-only %s -// RUN: %clang -cc1 -fbounds-safety -fexperimental-bounds-safety-objc -fsyntax-only %s +// RUN: %clang_cc1 -fbounds-safety -fexperimental-bounds-safety-objc -fsyntax-only %s diff --git a/clang/test/BoundsSafety/Frontend/objc_experimental_is_supported.m b/clang/test/BoundsSafety/Frontend/objc_experimental_is_supported.m index be510175e83b1..048f26d299cc8 100644 --- a/clang/test/BoundsSafety/Frontend/objc_experimental_is_supported.m +++ b/clang/test/BoundsSafety/Frontend/objc_experimental_is_supported.m @@ -1,2 +1,2 @@ -// RUN: %clang -cc1 -fbounds-safety -fexperimental-bounds-safety-objc -fsyntax-only %s +// RUN: %clang_cc1 -fbounds-safety -fexperimental-bounds-safety-objc -fsyntax-only %s diff --git a/clang/test/BoundsSafety/Frontend/objc_experimental_without_bounds_safety_ignored.cpp b/clang/test/BoundsSafety/Frontend/objc_experimental_without_bounds_safety_ignored.cpp index cac52bee642b7..06719fc41e268 100644 --- a/clang/test/BoundsSafety/Frontend/objc_experimental_without_bounds_safety_ignored.cpp +++ b/clang/test/BoundsSafety/Frontend/objc_experimental_without_bounds_safety_ignored.cpp @@ -1,5 +1,5 @@ -// RUN: %clang -cc1 -fexperimental-bounds-safety-objc -fsyntax-only %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -fexperimental-bounds-safety-objc -fsyntax-only %s 2>&1 | FileCheck %s // CHECK: warning: -fexperimental-bounds-safety-objc without -fbounds-safety is ignored diff --git a/clang/test/BoundsSafety/Frontend/only_c_is_supported.c b/clang/test/BoundsSafety/Frontend/only_c_is_supported.c index b98ccd2fd9ab7..280fa2ce56532 100644 --- a/clang/test/BoundsSafety/Frontend/only_c_is_supported.c +++ b/clang/test/BoundsSafety/Frontend/only_c_is_supported.c @@ -1,10 +1,10 @@ -// RUN: not %clang -cc1 -fbounds-safety -x c++ %s 2>&1 | FileCheck %s +// RUN: not %clang_cc1 -fbounds-safety -x c++ %s 2>&1 | FileCheck %s -// RUN: not %clang -cc1 -fbounds-safety -x objective-c %s 2>&1 | FileCheck %s +// RUN: not %clang_cc1 -fbounds-safety -x objective-c %s 2>&1 | FileCheck %s -// RUN: not %clang -cc1 -fbounds-safety -x objective-c++ %s 2>&1 | FileCheck %s +// RUN: not %clang_cc1 -fbounds-safety -x objective-c++ %s 2>&1 | FileCheck %s -// RUN: not %clang -cc1 -fbounds-safety -x objective-c++ %s 2>&1 | FileCheck %s +// RUN: not %clang_cc1 -fbounds-safety -x objective-c++ %s 2>&1 | FileCheck %s // CHECK: error: -fbounds-safety is supported only for C language diff --git a/clang/test/BoundsSafety/lit.local.cfg.py b/clang/test/BoundsSafety/lit.local.cfg.py new file mode 100644 index 0000000000000..7e1344d5936a8 --- /dev/null +++ b/clang/test/BoundsSafety/lit.local.cfg.py @@ -0,0 +1,69 @@ +import lit.formats +import os + +# clang/test uses external shell by default, but all tests in clang/test/BoundsSafety +# support lit's internal shell, so use that instead +config.test_format = lit.formats.ShTest(False) + +cc1_substituted = [ False ] +clang_substituted = [ False ] +extra_flags = '-fbounds-safety-bringup-missing-checks=all' + +def get_subst(subst_name): + subst_re = r'%\b{}\b'.format(subst_name) + for subst_name, value in config.substitutions: + if subst_re not in subst_name: + continue + return value + raise Exception(f'Could not find %{subst_name} substitution') + +def patch_subst_impl(sub_tuple, subst_to_patch): + assert isinstance(sub_tuple, tuple) + assert len(sub_tuple) == 2 + subst_name = sub_tuple[0] + + # Match `%subst_name` (e.g. `%clang_cc1`). The `ToolSubst` class inside + # `lit` adds a bunch of regexes around this substitution so we try to match + # them here to avoid matching a substitution that has `%clang_cc1` as a + # substring. This is fragile + subst_re = r'%\b{}\b'.format(subst_to_patch) + if subst_re not in subst_name: + return (sub_tuple[0], sub_tuple[1], False) + patched_sub = sub_tuple[1] + f' {extra_flags}' + return (sub_tuple[0], patched_sub, True) + +def patch_cc1_subst(sub_tuple): + subst_name, subst, patched = patch_subst_impl(sub_tuple, 'clang_cc1') + if patched: + cc1_substituted[0] = True + return (subst_name, subst) + +def patch_clang_subst(sub_tuple): + subst_name, subst, patched = patch_subst_impl(sub_tuple, 'clang') + if patched: + clang_substituted[0] = True + return (subst_name, subst) + + +force_new_bounds_checks_on = False + +# Provide an un-patched substitution +default_clang_subst = get_subst('clang') +config.substitutions.append( + (r'%\bclang_no_bounds_check_mode_specified\b', default_clang_subst) +) + +if False: + force_new_bounds_checks_on = True + +if force_new_bounds_checks_on: + config.substitutions = list(map(patch_cc1_subst, config.substitutions)) + config.substitutions = list(map(patch_clang_subst, config.substitutions)) + + dir_with_patched_sub = os.path.dirname(__file__) + for subst, patched in [('%clang_cc1', cc1_substituted[0]), ('%clang', clang_substituted[0])]: + if not patched: + lit_config.fatal(f'Failed to add `{extra_flags}` to {subst} invocations under {dir_with_patched_sub}') + else: + lit_config.note(f'Implicitly passing `{extra_flags}` to {subst} invocations under {dir_with_patched_sub}') +