[Driver][BoundsSafety] Add -fexperimental-bounds-safety flag #70480

rapidsna · 2023-10-27T17:00:21Z

-fexperimental-bounds-safety is an experimental flag for -fbounds-safety, which is a bounds-safety extension for C. -fbounds-safety will require substantial changes across the Clang codebase. So we introduce this experimental flag is to gate our incremental patches until we push the essential functionality of the extension.

-fexperimental-bounds-safety currently doesn't do anything but reporting an error when the flag is used with an unsupported source language (currently only supports C).

-fbounds-safety-experimental is an experimental flag for -fbounds-safety, which is a bounds-safety extension for C. -fbounds-safety will require substantial changes across the Clang codebase. So we introduce this experimental flag is to gate our incremental patches until we push the essential functionality of the extension. -fbounds-safety-experimental currently doesn't do anything but reporting an error when the flag is used with an unsupported source language (currently only supports C).

rapidsna · 2023-10-27T17:02:46Z

I'll push BoundsSafety.rst documentation in a separate PR.

Update: PR open here: #70749

delcypher

In general looks good but there are some minor changes I'd like made.

clang/include/clang/Basic/DiagnosticFrontendKinds.td

clang/test/BoundsSafety/Driver/driver.c

clang/lib/Frontend/CompilerInvocation.cpp

clang/test/BoundsSafety/Frontend/ignored_for_asm.s

delcypher · 2023-10-30T20:34:52Z

clang/test/BoundsSafety/Frontend/ignored_for_ir.ll

@@ -0,0 +1,5 @@
+; RUN: %clang -fbounds-safety-experimental -x ir -S %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: %clang_cc1 -fbounds-safety-experimental -x ir -S %s -o /dev/null 2>&1 | FileCheck %s
+


Nit: It might be worth having a small comment here explaining why you expect these diagnostics to not fire.

We usually test %clang for driver and %clang_cc1 for the others. Also, if we test %clang_cc1, we omit %clang.

@MaskRay fixed!

Also, LLVM IR and Unknown must be unreachable when parsing the language arguments.

delcypher · 2023-10-31T00:34:52Z

clang/include/clang/Basic/DiagnosticFrontendKinds.td

+def err_bounds_safety_lang_not_supported : Error<
+  "bounds safety is only supported for C">;
+def warn_bounds_safety_asm_ignored : Warning<
+  "'-fbounds-safety' is ignored for assembly">,


Nit. In this PR the flag is -fbounds-safety-experimental not -fbounds-safety.

For assembly input, we generally ignore handling the option and leave a warning https://maskray.me/blog/2023-08-25-clang-wunused-command-line-argument#assembly-files

% fclang -fsanitize=address -fpatchable-function-entry=2 -c a.s clang: warning: argument unused during compilation: '-fsanitize=address' [-Wunused-command-line-argument] clang: warning: argument unused during compilation: '-fpatchable-function-entry=2' [-Wunused-command-line-argument]

@MaskRay Thank you! I can use warning: argument unused during compilation for the driver. Do you know what is the convention for unused flags in cc1?

https://maskray.me/blog/2023-08-25-clang-wunused-command-line-argument has some notes. If you don't call functions like getLastArg that claim the option, there will be a warning: argument unused during compilation: '-fsanitize=address' [-Wunused-command-line-argument] for assembly input.

@MaskRay So we aren't explicit using getLastArg but it looks like we are calling it indirectly from addOptInFlag(...) which I think means we are unconditionally claiming the option. In order to make your suggestion work we'd have to conditionally call addOptInFlag in the driver based on the selected input language but I don't think that's going to work because no LangOptions object exists for us query at that point in the driver AFAICT.

Also future patches we will upstream will add additional Args.getLastArg(options::OPT_fbounds_safety_experimental, ...) calls in the driver which means the option will get claimed there too which will break any reliance on the option not being claimed that might be implemented in this PR.

This warning is really a frontend warning and not a driver warning so I think we should use the frontend warning that @rapidsna introduced in this PR.

@MaskRay Thanks. I still need the error for -fbounds-safety not being supported in C++/Obj-C/etc in the frontend because it needs to be an error for cc1.

I will change it so that the unused warning for assembly to be handled in the driver like the rest of the unused options for assembly.

Conventionally the language compatibility checking and other checking is performed in Driver, not in Frontend.. If you move the language check to Driver, the diagnostic will be natural since clang integrated assembler uses ClangAs instead of Clang.

@MaskRay It seems both Clang and ClangAs are invoked for the clang command with an assembly input (i.e., ClangAs is invoked after Clang) so it seems to me that most options are already claimed in Clang and so warning: argument unused during compilation doesn't seem to fire for clang in most cases including -fsanitize=address.

Also, we need to have the input language (InputKind) to report the right diagnostics so the frontend still seems like the right place to handle this.

@MaskRay Sorry, it seems we can still do it in Driver with clang::driver::types. I'm going to make a change to report the "unused" warning from the driver, and report the "unsupported language" error in the frontend. Does it sound okay to you?

@MaskRay It seems both Clang and ClangAs are invoked for the clang command with an assembly input (i.e., ClangAs is invoked after Clang) so it seems to me that most options are already claimed in Clang and so warning: argument unused during compilation doesn't seem to fire for clang in most cases including -fsanitize=address.

I just confirmed that this is happening when the input file format is .s and no -x is provided from the command line. If we do -x assembler then only ClangAs is launched and the unused option warning fires as expected.

@MaskRay Is it expected?

@MaskRay I removed the frontend warning, so that it naturally follows how the driver handles other unused flags. Now only warning: argument unused during compilation fires for -x assembler, and not for -x assembler-with-cpp to conform to most of other options.

delcypher

@rapidsna LGTM other than the nits.

tbaederr · 2023-10-31T03:56:17Z

The other experimental flags I see start with experimental, this one ends with it. Why isn't this called -fexerimental-bounds-safety?

rapidsna · 2023-10-31T14:43:40Z

The other experimental flags I see start with experimental, this one ends with it. Why isn't this called -fexerimental-bounds-safety?

Oh, I can see most of them start with -fexperimental, not everything though. I can fix this. Is there a formal convention?

nickdesaulniers · 2023-10-31T15:53:12Z

clang/include/clang/Basic/DiagnosticFrontendKinds.td

+def warn_bounds_safety_asm_ignored : Warning<
+  "'-fbounds-safety' is ignored for assembly">,


Do we even need a diagnostic specific to assembler? I get the point that some projects Makefiles pass compiler flags to the assembler invocation (which in the case of LLVM based builds, the assembler is clang). But to me "bounds safety is only supported for C" is a superset of of "is ignored for assembly." Seems like we could get away with only one diagnostic (the first "is not C" one).

Or was this particularly helpful for teams that got confused as to why the compiler (invoked as the assembler) was yelling at them?

@nickdesaulniers They are separate because the languages (C++/Obj-C/etc) we want to make an error until we have a proper support vs a warning for assembly (or maybe others?) that are meant to be ignored. For the latter I think we can use warning: argument unused during compilation as @MaskRay suggested.

@nickdesaulniers I managed to move -x ir test to the .c test. Not sure why but adding -### option seemed to make it work.

nickdesaulniers · 2023-10-31T15:57:55Z

clang/test/BoundsSafety/Driver/driver.c

@@ -0,0 +1,11 @@
+// RUN: %clang -c %s -### 2>&1 | FileCheck -check-prefix T0 %s


I would have expected a test like this to be added to clang/test/Driver/clang_f_opts.c

why are the new tests in clang/test/BoundsSafety/ rather than the existing directory structures? I'm guessing to avoid merge conflicts when rebasing against upstream? Either way, let's conform to the existing layout?

This comment applies to ALL newly added test files.

clang/test/Driver/clang_f_opts.c mixes a lot of options. Newer -fxxx tests can go to a dedicated clang/test/Driver/fxxx.c

@nickdesaulniers I was hoping that we could keep -fbounds-safety tests under a separate folder clang/test/BoundsSafety in order to avoid them from getting mixed up with other tests. And make it easier to run bounds safety related tests only. But I can see this is not what other extensions do so I can move them to the existing layout.

@nickdesaulniers As @rapidsna said there are multiple reasons:

Provides a very convenient way to run only -fbounds-safety tests.

More likely to avoid merge conflicts with our internal code.

Out of those two I think (1.) is the bigger problem because it is something I rely on for local development and I think merge conflicts are still unlikely even if we put tests under their proper location under clang/test because we mostly created new tests and tried not to touch existing tests. If lit had a way of labelling tests and only running tests with a particular label then the location of the tests would be irrelevant and I would be happy for the tests to live in their normal place under clang/test/ (e.g. clang/test/Driver). Unfortunately AFAIK that feature doesn't exist in lit.

To workaround this would this kind of layout be acceptable?

test/Driver/BoundsSafety/... test/Frontend/BoundsSafety/... test/Sema/BoundsSafety/... ...

That way the tests are conforming to Clang's test layout but its still possible to run only the -fbounds-safety tests by doing something like:

$ bin/llvm-lit -vs tools/clang/test/{Driver,Frontend,Sema}/BoundsSafety

Test subdirectories are added conservatively. The main use cases are target-specific tests and flang. Driver/XRay is somehow an unneeded use that you can ignore.

% echo {CodeGen,Sema,Driver}/*(/) CodeGen/aarch64_neon_sve_bridge_intrinsics CodeGen/aarch64-sme-intrinsics CodeGen/aarch64-sve2-intrinsics CodeGen/aarch64-sve2p1-intrinsics CodeGen/aarch64-sve-intrinsics CodeGen/arc CodeGen/arm-mve-intrinsics CodeGen/assignment-tracking CodeGen/avr CodeGen/CSKY CodeGen/Inputs CodeGen/LoongArch CodeGen/PowerPC CodeGen/RISCV CodeGen/SystemZ CodeGen/VE CodeGen/WebAssembly CodeGen/X86 Sema/aarch64-sme-intrinsics Sema/aarch64-sve2-intrinsics Sema/aarch64-sve2p1-intrinsics Sema/aarch64-sve-intrinsics Sema/Inputs Driver/flang Driver/Inputs Driver/XRay

FWIW you can do

path/bin/llvm-lit clang/test/Driver/bounds-safety-*

LIT_OPTS=--filter=bounds-safety path/bin/llvm-lit clang/test

and even create an alias for LIT_OPTS=--filter=bounds-safety

And make it easier to run bounds safety related tests only.

Provides a very convenient way to run only -fbounds-safety tests.

Then give each test a similar function name prefix, and then you can do llvm-lit -vv llvm/test/Driver/fbounds-safety-*, as @MaskRay said. Don't start creating new directory structures that don't exist for every other feature implemented by the compiler.

@nickdesaulniers @MaskRay Thank you! I removed the new directory and moved the tests to conform to the existing layout.

nickdesaulniers · 2023-10-31T16:00:19Z

clang/test/BoundsSafety/Frontend/only_c_is_supported.c

+
+// RUN: not %clang_cc1 -fbounds-safety-experimental -x cuda %s 2>&1 | FileCheck -check-prefix ERR %s
+
+// RUN: not %clang_cc1 -fbounds-safety-experimental -x renderscript %s 2>&1 | FileCheck -check-prefix ERR %s


why not test -x ir and -x assembler in the same test file?

I think at least -x ir had to be a separate file because how it recognize comments (// vs ;). I'll double check.

ah, makes sense, consider adding that as a comment

nickdesaulniers · 2023-10-31T16:02:44Z

clang/lib/Frontend/CompilerInvocation.cpp

+  case Language::Asm:
+    Diags.Report(diag::warn_bounds_safety_asm_ignored);
+    break;


What happens if I'm compiling -x assembler-with-cpp (i.e. assembler that needs to be preprocessed, .S files)?

Does this error?

What if I'm using the preprocessor to detect if -fbounds-safety is supported?

#if __has_attribute(bidi_whatever) mov x0, #42 #else #error "oh no" #endif

(or perhaps __has_feature(bounds_safety))

Right, this is something we could potentially allow in future if we see actual use cases.

Until we support this, __has_feature(bounds_safety) or such will return false and we should have the compiler report the warning that -fbounds-safety is ignored for assemblers.

MaskRay · 2023-10-31T16:11:16Z

clang/include/clang/Basic/DiagnosticFrontendKinds.td

+def err_bounds_safety_lang_not_supported : Error<
+  "bounds safety is only supported for C">;
+def warn_bounds_safety_asm_ignored : Warning<
+  "'-fbounds-safety' is ignored for assembly">,


For assembly input, we generally ignore handling the option and leave a warning https://maskray.me/blog/2023-08-25-clang-wunused-command-line-argument#assembly-files

% fclang -fsanitize=address -fpatchable-function-entry=2 -c a.s clang: warning: argument unused during compilation: '-fsanitize=address' [-Wunused-command-line-argument] clang: warning: argument unused during compilation: '-fpatchable-function-entry=2' [-Wunused-command-line-argument]

MaskRay · 2023-10-31T16:16:03Z

clang/test/BoundsSafety/Frontend/only_c_is_supported.c

@@ -0,0 +1,25 @@
+// RUN: not %clang -fbounds-safety-experimental -x c++ %s 2>&1 | FileCheck -check-prefix ERR %s
+


We use a terse style and use a blank line to group different sets of tests. Here the blank line use is unnecessary and harms readability IMO.

@MaskRay Thank you! I removed the blank lines.

…r by default

rapidsna · 2023-11-02T18:58:17Z

The other experimental flags I see start with experimental, this one ends with it. Why isn't this called -fexerimental-bounds-safety?

@tbaederr Thank you! I just renamed the flag to -fexperimental-bounds-safety.

github-actions · 2023-11-02T19:02:31Z

✅ With the latest revision this PR passed the C/C++ code formatter.

rapidsna · 2023-11-02T19:07:27Z

clang/test/Driver/bounds-safety-unused.c

+
+// expected-no-diagnostics
+// RUN: %clang -fexperimental-bounds-safety -Xclang -verify -c -x c %s -o /dev/null
+// Unlike '-x assembler', '-x assembler-with-cpp' silently ignores unused options by default.


@nickdesaulniers The unused warning doesn't appear for -x assembler-with-cpp to follow the common behaviors. Instead, I added a note here that we need to add a targeted warning in future when assembler tries to use preprocessor directives to check if bounds safety ie enabled. WDYT?

delcypher · 2023-11-03T00:05:14Z

clang/test/Driver/bounds-safety-unused.c

@@ -0,0 +1,12 @@
+// This reports a warning to follow the default behavior of ClangAs.
+// RUN: %clang -fexperimental-bounds-safety -x assembler -c %s -o /dev/null 2>&1 | FileCheck -check-prefix WARN %s


@rapidsna Maybe there should be a version of this part of the test that doesn't pass -x <lang> and instead passes to a .s file to make sure the right thing happens when in that case?

I think testing with -x assembler and -x assembler-with-cpp are more specific and should cover what I want.

.s must be -x assembler (except for Darwin which does -x assembler-with-cpp for .s), but that specific behavior should be tested separately by related updates, and not in this PR.

nickdesaulniers

If you make all of the newly added tests share a similar filename prefix, it will be easier to test via llvm-lit -vv clang/test/Driver/bounds-safety*. fbounds-safety.c seems to break that convention.

delcypher · 2023-11-03T17:40:56Z

clang/test/Driver/bounds-safety-unused.c

+// expected-no-diagnostics
+// RUN: %clang -fexperimental-bounds-safety -Xclang -verify -c -x c %s -o /dev/null
+// Unlike '-x assembler', '-x assembler-with-cpp' silently ignores unused options by default.
+// XXX: Should report a targeted warning in future when assembler tries to use preprocessor directives to conditionalize behavior when bounds safety is enabled.


@rapidsna We should file an issue and refer to it in the code.

I've created #71206 for you and created a new clang:bounds-safety tag so we can track all the -fbounds-safety issues in open source.

@delcypher Perfect, thanks!

MaskRay · 2023-11-07T04:39:18Z

clang/lib/Frontend/CompilerInvocation.cpp

@@ -3618,6 +3618,27 @@ void CompilerInvocationBase::GenerateLangArgs(const LangOptions &Opts,
    GenerateArg(Consumer, OPT_frandomize_layout_seed_EQ, Opts.RandstructSeed);
 }

+static void CheckBoundsSafetyLang(InputKind IK, DiagnosticsEngine &Diags) {


I think it's better to handle this language check in Driver, similar to OPT_fminimize_whitespace but for all Inputs.

It's not necessary to mention

This is especially important because some build systems, like xcbuild and somewhat clumsy Makefiles, will pass
C_FLAGS to Clang while building assembly files.

since this is general and applies to other options. No need to emphasize it specifically for bounds checking.

I think it's better to handle this language check in Driver, similar to OPT_fminimize_whitespace but for all Inputs.

@MaskRay If we do it in Driver, wouldn't the option be silently ignored for unsupported languages when we invoke clang -cc1 directly?

IMHO, that might be problematic because people would reasonably expect bounds-safety would work for C++/Obj-C/Obj-C++.

There are various different options here.

The frontend silently accepts-fexperimental-bounds-safety with -x c++ but doesn't actually enable fexperimental-bounds-safety.

The frontend accepts-fexperimental-bounds-safety with -x c++ enables -fexperimental-bounds-safety and probably crashes because it's not supported.

The frontend rejects-fexperimental-bounds-safety with -x c++ with an error.

I don't like (1.) because when writing frontend only tests it is inviting someone to accidentally write a test incorrectly because the frontend would never complain about the invalid configuration.

I don't like (2.) because crashing really isn't good when it could so easily be prevented.

I think we should do (3.) because it has none of the previously mentioned downsides.

To be clear. I think it's fine to also do the language check in the driver but I think the frontend should also do language check so that it is not possible to run it in an invalid configuration.

I think it's better to handle this language check in Driver, similar to OPT_fminimize_whitespace but for all Inputs.

@MaskRay Thanks for the review! I made it a driver error here:
f125532

We also keep the -cc1 check in order to prevent it from running with an unsupported mode (we still need some checks in cc1 anyway in case we want to silently ignore the option, instead of triggering the broken mode).

MaskRay · 2023-11-07T04:41:07Z

clang/include/clang/Basic/DiagnosticGroups.td

@@ -1412,6 +1412,9 @@ def FunctionMultiVersioning

 def NoDeref : DiagGroup<"noderef">;

+// Bounds safety specific warnings
+def IgnoredBoundsSafety : DiagGroup<"ignored-bounds-safety">;


This is -Wignored-bounds-safety is not used.

Removed it!

MaskRay · 2023-11-07T04:43:29Z

[Driver][BoundsSafety] Add -fbounds-safety-experimental flag #70480

The patch implements -fexperimental-bounds-safety

rapidsna · 2023-11-07T20:54:31Z

[Driver][BoundsSafety] Add -fbounds-safety-experimental flag #70480

The patch implements -fexperimental-bounds-safety

I just updated the title. I'll make sure I update the text when I squash the commits.

…ported language

MaskRay · 2023-11-10T01:13:29Z

It depends whether an option leads to a warning or is silently ignored for a language that is not implemented.
Many options are silently ignored for C. For example, -faligned-new and -fexceptions are silently ignored.

Sometimes, it is useful report a warning/error to prevent misuses.
Say, the bounds checking feature may be extended to C++ in the future and before that, we want to discourage C++ projects adopting the option.
Therefore, this patch proposes an error. I think this is fine.

However, implementing the checking in clang/lib/Driver is much more common.
clang/lib/Frontend/CompilerInvocation.cpp has some checking, but the majority is in clang/lib/Driver.

I am not convinced by your argument that you want to catch -cc1 misuses.
A user can use -Xclang xxx or -mllvm yyy to specify a lot of stuff and most are infeasible to verify in clang/lib/Frontend/CompilerInvocation.cpp.
If a user is determined to bypass the compiler' error checking, there is not much the compiler can do.
Sometimes, it is actually useful to expose some experimental features that we don't feel comfortable surfacing as driver options as cc1 options. The cc1 options give the users to experiment with the feature. It's the users's responsibility to adapt when the feature changes.

rapidsna · 2023-11-10T01:22:06Z

However, implementing the checking in clang/lib/Driver is much more common. clang/lib/Frontend/CompilerInvocation.cpp has some checking, but the majority is in clang/lib/Driver.

@MaskRay To be clear, the check is now in Driver as you suggested. It's just that the frontend also has some extra checks too. So, you want me to remove the extra checks in the frontend?

rapidsna · 2023-11-10T01:56:45Z

Sometimes, it is actually useful to expose some experimental features that we don't feel comfortable surfacing as driver options as cc1 options. The cc1 options give the users to experiment with the feature. It's the users's responsibility to adapt when the feature changes.

Ok. I'll fix this.

MaskRay · 2023-11-14T02:39:44Z

@MaskRay To be clear, the check is now in Driver as you suggested. It's just that the frontend also has some extra checks too. So, you want me to remove the extra checks in the frontend?

Yes, otherwise it's duplicated check.

In addition, I think our convention is to add the option when the feature is ready, rather than add a no-op option, than build functional patches on top of it.

MaskRay

-fbounds-safety-experimental is an experimental flag for -fbounds-safety,

-fexperimental-bounds-safety

rapidsna · 2023-11-14T23:23:20Z

-fbounds-safety-experimental is an experimental flag for -fbounds-safety,

-fexperimental-bounds-safety

Changed the description in the PR! I'll adjust the commit message too when I squash all the changes once I get your approval.

rapidsna · 2023-11-14T23:23:52Z

@MaskRay To be clear, the check is now in Driver as you suggested. It's just that the frontend also has some extra checks too. So, you want me to remove the extra checks in the frontend?

Yes, otherwise it's duplicated check.

I just removed the check in the frontend!

rapidsna · 2023-11-15T00:08:30Z

In addition, I think our convention is to add the option when the feature is ready, rather than add a no-op option, than build functional patches on top of it.

@MaskRay This feature will involve a lot of incremental patches. And we will still need an experimental flag to test the incremental functionalities that we will be adding. I can make it a CC1 only flag for now. Would it work for you better?

MaskRay · 2023-11-15T01:53:07Z

In addition, I think our convention is to add the option when the feature is ready, rather than add a no-op option, than build functional patches on top of it.

@MaskRay This feature will involve a lot of incremental patches. And we will still need an experimental flag to test the incremental functionalities that we will be adding. I can make it a CC1 only flag for now. Would it work for you better?

Yes. This sound good to me.

You will need to remove the lib/Driver change and the language compatibility check, then.

I think the patch should still be held off until the first patch that does some basic functionality is ready.

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 27, 2023

hnrklssn requested review from hnrklssn and delcypher October 27, 2023 17:30

delcypher requested changes Oct 27, 2023

View reviewed changes

delcypher reviewed Oct 30, 2023

View reviewed changes

clang/test/BoundsSafety/Frontend/ignored_for_asm.s Outdated Show resolved Hide resolved

delcypher reviewed Oct 30, 2023

View reviewed changes

Report warning when -fbounds-safety is ignored for assembly

8049f6f

Also, LLVM IR and Unknown must be unreachable when parsing the language arguments.

rapidsna force-pushed the bounds-safety-experimental-flag branch from 5f241a6 to 8049f6f Compare October 30, 2023 21:00

delcypher requested review from kees, nickdesaulniers, AaronBallman, bwendling and jansvoboda11 October 31, 2023 00:32

delcypher reviewed Oct 31, 2023

View reviewed changes

delcypher approved these changes Oct 31, 2023

View reviewed changes

nickdesaulniers reviewed Oct 31, 2023

View reviewed changes

MaskRay requested changes Oct 31, 2023

View reviewed changes

MaskRay reviewed Oct 31, 2023

View reviewed changes

rapidsna added 3 commits November 2, 2023 11:13

Remove bounds-safety ignored warning, instead let it handled by drive…

90410be

…r by default

Flag name update: -fexperimental-bounds-safety

3155dc9

Move tests to follow the existing test layout

5e35c30

rapidsna commented Nov 2, 2023

View reviewed changes

clang-format

7bb2a93

delcypher reviewed Nov 3, 2023

View reviewed changes

nickdesaulniers approved these changes Nov 3, 2023

View reviewed changes

delcypher mentioned this pull request Nov 3, 2023

Clang should warn when building with -fexperimental-bounds-safety and -x assembler-with-cpp #71206

Open

delcypher reviewed Nov 3, 2023

View reviewed changes

delcypher added the clang:bounds-safety Issue/PR relating to the experimental -fbounds-safety feature in Clang label Nov 3, 2023

rapidsna requested a review from MaskRay November 6, 2023 21:30

MaskRay requested changes Nov 7, 2023

View reviewed changes

MaskRay reviewed Nov 7, 2023

View reviewed changes

rapidsna added 2 commits November 7, 2023 12:41

Remove unused diag group

c739327

Remove superfluous comment about ignored options

b0774f1

rapidsna changed the title ~~[Driver][BoundsSafety] Add -fbounds-safety-experimental flag~~ [Driver][BoundsSafety] Add -fexperimental-bounds-safety flag Nov 7, 2023

Make it a driver error to use -fexperimental-bounds-safety with unsup…

f125532

…ported language

rapidsna requested a review from MaskRay November 9, 2023 23:04

clang format

05cd678

MaskRay requested changes Nov 14, 2023

View reviewed changes

Remove redundant option check in the frontend

e2ecf9f

rapidsna requested a review from MaskRay November 14, 2023 23:24

delcypher mentioned this pull request May 18, 2024

[BoundsSafety] Add -fexperimental-bounds-safety CC1 and language option and use it to tweak counted_by's semantics #92623

Open

		@@ -0,0 +1,5 @@
		; RUN: %clang -fbounds-safety-experimental -x ir -S %s -o /dev/null 2>&1 \| FileCheck %s
		; RUN: %clang_cc1 -fbounds-safety-experimental -x ir -S %s -o /dev/null 2>&1 \| FileCheck %s

		def warn_bounds_safety_asm_ignored : Warning<
		"'-fbounds-safety' is ignored for assembly">,

		@@ -0,0 +1,11 @@
		// RUN: %clang -c %s -### 2>&1 \| FileCheck -check-prefix T0 %s


		// RUN: not %clang_cc1 -fbounds-safety-experimental -x cuda %s 2>&1 \| FileCheck -check-prefix ERR %s

		// RUN: not %clang_cc1 -fbounds-safety-experimental -x renderscript %s 2>&1 \| FileCheck -check-prefix ERR %s

		@@ -0,0 +1,25 @@
		// RUN: not %clang -fbounds-safety-experimental -x c++ %s 2>&1 \| FileCheck -check-prefix ERR %s

		@@ -0,0 +1,12 @@
		// This reports a warning to follow the default behavior of ClangAs.
		// RUN: %clang -fexperimental-bounds-safety -x assembler -c %s -o /dev/null 2>&1 \| FileCheck -check-prefix WARN %s

[Driver][BoundsSafety] Add -fexperimental-bounds-safety flag #70480

Are you sure you want to change the base?

[Driver][BoundsSafety] Add -fexperimental-bounds-safety flag #70480

Conversation

rapidsna commented Oct 27, 2023 • edited

rapidsna commented Oct 27, 2023 • edited

delcypher left a comment

Choose a reason for hiding this comment

delcypher Oct 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rapidsna Nov 1, 2023 • edited

Choose a reason for hiding this comment

rapidsna Nov 2, 2023 • edited

Choose a reason for hiding this comment

delcypher left a comment

Choose a reason for hiding this comment

tbaederr commented Oct 31, 2023

rapidsna commented Oct 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaskRay Oct 31, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rapidsna commented Nov 2, 2023

github-actions bot commented Nov 2, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rapidsna Nov 3, 2023 • edited

Choose a reason for hiding this comment

nickdesaulniers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaskRay commented Nov 7, 2023

rapidsna commented Nov 7, 2023

MaskRay commented Nov 10, 2023 • edited

rapidsna commented Nov 10, 2023

rapidsna commented Nov 10, 2023

MaskRay commented Nov 14, 2023

MaskRay left a comment

Choose a reason for hiding this comment

rapidsna commented Nov 14, 2023

rapidsna commented Nov 14, 2023

rapidsna commented Nov 15, 2023 • edited

MaskRay commented Nov 15, 2023

rapidsna commented Oct 27, 2023 •

edited

rapidsna commented Oct 27, 2023 •

edited

delcypher Oct 30, 2023 •

edited

rapidsna Nov 1, 2023 •

edited

rapidsna Nov 2, 2023 •

edited

MaskRay Oct 31, 2023 •

edited

github-actions bot commented Nov 2, 2023 •

edited

rapidsna Nov 3, 2023 •

edited

MaskRay commented Nov 10, 2023 •

edited

rapidsna commented Nov 15, 2023 •

edited