[clang] [BoundsSafety] Initial documentation for -fbounds-safety (PR #70749)

[Dan Liew via cfe-commits]

delcypher wrote:

@rapidsna The CI is complaining about trailing whitespace

https://buildkite.com/llvm-project/clang-ci/builds/5817#018b830d-fb91-40df-8956-9e4dd325d27f

https://github.com/llvm/llvm-project/pull/70749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fexperimental-bounds-safety flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -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) {

delcypher wrote:

There are various different options here.

1. The frontend silently accepts`-fexperimental-bounds-safety` with `-x c++` 
but doesn't actually enable `fexperimental-bounds-safety`.
2. The frontend accepts`-fexperimental-bounds-safety` with `-x c++` enables 
`-fexperimental-bounds-safety` and probably crashes because it's not supported.
3. 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.



https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fexperimental-bounds-safety flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -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) {

delcypher wrote:

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.

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -0,0 +1,5 @@
+
+// RUN: %clang -fbounds-safety-experimental -fsyntax-only %s 2>&1 | FileCheck 
%s
+// RUN: %clang_cc1 -fbounds-safety-experimental -fsyntax-only %s 2>&1 | 
FileCheck %s
+
+// CHECK: warning: '-fbounds-safety' is ignored for assembly

delcypher wrote:

@rapidsna  Is some code missing from this PR? I see you now have a test but I 
can't find the code change that causes this warning to be emitted in the PR.

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -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
+

delcypher wrote:

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

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

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

delcypher wrote:

@nickdesaulniers As @rapidsna said there are multiple reasons:

1. Provides a very convenient way to run **only `-fbounds-safety` tests**.
2. 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
```



https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -330,6 +330,14 @@ def warn_alias_with_section : Warning<
   "as the %select{aliasee|resolver}2">,
   InGroup;
 
+let CategoryName = "Bounds Safety Issue" in {
+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">,

delcypher wrote:

@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.

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -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

delcypher wrote:

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

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -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
+
+
+// WARN: warning: argument unused during compilation: 
'-fexperimental-bounds-safety'
+
+// 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.

delcypher wrote:

@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.

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [BoundsSafety] Initial documentation for -fbounds-safety (PR #70749)

[Dan Liew via cfe-commits]

https://github.com/delcypher approved this pull request.

I have a few minor nits but other than that LGTM.

Please make sure this builds without errors/warnings.

I think you can configure CMake with `-DLLVM_BUILD_DOCS=ON -D 
LLVM_ENABLE_SPHINX=ON -DSPHINX_WARNINGS_AS_ERROR=ON` to enable building the 
documentation. The target to build the docs is `docs-clang-html`

https://github.com/llvm/llvm-project/pull/70749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [BoundsSafety] Initial documentation for -fbounds-safety (PR #70749)

[Dan Liew via cfe-commits]

@@ -0,0 +1,480 @@
+==
+-fbounds-safety: Enforcing bounds safety for C
+==
+
+.. contents::
+   :local:
+
+Overview
+
+
+-fbounds-safety is a C extension to enforce bounds safety to prevent 
out-of-bounds (OOB) memory accesses, which remain a major source of security 
vulnerabilities in C. -fbounds-safety aims to eliminate this class of bugs by 
turning OOB accesses into deterministic traps.
+
+The -fbounds-safety extension offers bounds annotations that programmers can 
use to attach bounds to pointers. For example, programmers can add the 
__counted_by(N) annotation to parameter ptr, indicating that the pointer has N 
valid elements:
+
+.. code-block:: c
+
+   void foo(int *__counted_by(N) ptr, size_t N);
+
+Using this bounds information, the compiler inserts bounds checks on every 
pointer dereference, ensuring that the program does not access memory outside 
the specified bounds. The compiler requires programmers to provide enough 
bounds information so that the accesses can be checked at either run time or 
compile time — and it rejects code if it cannot.
+
+The most important contribution of “-fbounds-safety” is how it reduces the 
programmer’s annotation burden by reconciling bounds annotations at ABI 
boundaries with the use of implicit wide pointers (a.k.a. “fat” pointers) that 
carry bounds information on local variables without the need for annotations. 
We designed this model so that it preserves ABI compatibility with C while 
minimizing adoption effort.

delcypher wrote:

Did you mean to use open and close quotes (`“` and `”`) rather than symmetrical 
quote `"`? I would prefer to use the symmetrical quote because it makes edits 
to the document easier.

https://github.com/llvm/llvm-project/pull/70749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [BoundsSafety] Initial documentation for -fbounds-safety (PR #70749)

[Dan Liew via cfe-commits]

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/70749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [BoundsSafety] Initial documentation for -fbounds-safety (PR #70749)

[Dan Liew via cfe-commits]

@@ -0,0 +1,480 @@
+==
+-fbounds-safety: Enforcing bounds safety for C
+==
+
+.. contents::
+   :local:
+
+Overview
+
+
+-fbounds-safety is a C extension to enforce bounds safety to prevent 
out-of-bounds (OOB) memory accesses, which remain a major source of security 
vulnerabilities in C. -fbounds-safety aims to eliminate this class of bugs by 
turning OOB accesses into deterministic traps.

delcypher wrote:

Should `-fbounds-safety` be quoted to be monospaced?

https://github.com/llvm/llvm-project/pull/70749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [BoundsSafety] Initial documentation for -fbounds-safety (PR #70749)

[Dan Liew via cfe-commits]

@@ -0,0 +1,480 @@
+==
+-fbounds-safety: Enforcing bounds safety for C
+==
+
+.. contents::
+   :local:
+
+Overview
+
+
+-fbounds-safety is a C extension to enforce bounds safety to prevent 
out-of-bounds (OOB) memory accesses, which remain a major source of security 
vulnerabilities in C. -fbounds-safety aims to eliminate this class of bugs by 
turning OOB accesses into deterministic traps.
+
+The -fbounds-safety extension offers bounds annotations that programmers can 
use to attach bounds to pointers. For example, programmers can add the 
__counted_by(N) annotation to parameter ptr, indicating that the pointer has N 
valid elements:
+
+.. code-block:: c
+
+   void foo(int *__counted_by(N) ptr, size_t N);
+
+Using this bounds information, the compiler inserts bounds checks on every 
pointer dereference, ensuring that the program does not access memory outside 
the specified bounds. The compiler requires programmers to provide enough 
bounds information so that the accesses can be checked at either run time or 
compile time — and it rejects code if it cannot.
+
+The most important contribution of “-fbounds-safety” is how it reduces the 
programmer’s annotation burden by reconciling bounds annotations at ABI 
boundaries with the use of implicit wide pointers (a.k.a. “fat” pointers) that 
carry bounds information on local variables without the need for annotations. 
We designed this model so that it preserves ABI compatibility with C while 
minimizing adoption effort.
+
+The -fbounds-safety extension has been adopted on millions of lines of 
production C code and proven to work in a consumer operating system setting. 
The extension was designed to enable incremental adoption — a key requirement 
in real-world settings where modifying an entire project and its dependencies 
all at once is often not possible. It also addresses multiple of other 
practical challenges that have made existing approaches to safer C dialects 
difficult to adopt, offering these properties that make it widely adoptable in 
practice:
+
+* It is designed to preserve the Application Binary Interface (ABI)
+* It interoperates well with plain C code
+* It can be adopted partially and incrementally while still providing safety 
benefits
+* It is syntactically and semantically compatible with C
+* Consequently, source code that adopts the extension can continue to be 
compiled by toolchains that do not support the extension.
+* It has a relatively low adoption cost
+* It can be implemented on top of Clang
+
+
+Programming Model
+
+
+Overview
+-

delcypher wrote:

Nit: Too may `-` characters?

https://github.com/llvm/llvm-project/pull/70749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [BoundsSafety] Initial documentation for -fbounds-safety (PR #70749)

[Dan Liew via cfe-commits]

@@ -0,0 +1,480 @@
+==
+-fbounds-safety: Enforcing bounds safety for C
+==
+
+.. contents::
+   :local:
+
+Overview
+
+
+-fbounds-safety is a C extension to enforce bounds safety to prevent 
out-of-bounds (OOB) memory accesses, which remain a major source of security 
vulnerabilities in C. -fbounds-safety aims to eliminate this class of bugs by 
turning OOB accesses into deterministic traps.
+
+The -fbounds-safety extension offers bounds annotations that programmers can 
use to attach bounds to pointers. For example, programmers can add the 
__counted_by(N) annotation to parameter ptr, indicating that the pointer has N 
valid elements:
+
+.. code-block:: c
+
+   void foo(int *__counted_by(N) ptr, size_t N);
+
+Using this bounds information, the compiler inserts bounds checks on every 
pointer dereference, ensuring that the program does not access memory outside 
the specified bounds. The compiler requires programmers to provide enough 
bounds information so that the accesses can be checked at either run time or 
compile time — and it rejects code if it cannot.
+
+The most important contribution of “-fbounds-safety” is how it reduces the 
programmer’s annotation burden by reconciling bounds annotations at ABI 
boundaries with the use of implicit wide pointers (a.k.a. “fat” pointers) that 
carry bounds information on local variables without the need for annotations. 
We designed this model so that it preserves ABI compatibility with C while 
minimizing adoption effort.
+
+The -fbounds-safety extension has been adopted on millions of lines of 
production C code and proven to work in a consumer operating system setting. 
The extension was designed to enable incremental adoption — a key requirement 
in real-world settings where modifying an entire project and its dependencies 
all at once is often not possible. It also addresses multiple of other 
practical challenges that have made existing approaches to safer C dialects 
difficult to adopt, offering these properties that make it widely adoptable in 
practice:
+
+* It is designed to preserve the Application Binary Interface (ABI)
+* It interoperates well with plain C code
+* It can be adopted partially and incrementally while still providing safety 
benefits
+* It is syntactically and semantically compatible with C
+* Consequently, source code that adopts the extension can continue to be 
compiled by toolchains that do not support the extension.
+* It has a relatively low adoption cost
+* It can be implemented on top of Clang
+
+
+Programming Model
+
+
+Overview
+-
+
+-fbounds-safety ensures that pointers are not used to access memory beyond 
their bounds by performing bounds checking. If a bounds check fails, the 
program will deterministically trap before out-of-bounds memory is accessed.
+
+In our model, every pointer has an explicit or implicit bounds attribute that 
determines its bounds and ensures guaranteed bounds checking. Consider the 
example below where the __counted_by(count) annotation indicates that parameter 
ppoints to a buffer of int s containing count elements. An off-by-one error is 
present in the loop condition, leading to p[i]being out-of-bounds access during 
the loop’s final iteration. The compiler inserts a bounds check before p is 
dereferenced to ensure that the access remains within the specified bounds.
+
+.. code-block:: c
+
+   void fill_array_with_indices(int *__counted_by(count) p, unsigned count) {  
+   // off-by-one error (i < count)  
+  for (unsigned i = 0; i <= count; ++i) {  
+ // bounds check inserted:  
+ //   if (i >= count) trap();  
+ p[i] = i;  
+  }  
+   }
+
+A bounds annotation defines an invariant for the pointer type, and the model 
ensures that this invariant remains true. In the example below, pointer p 
annotated with __counted_by(count) must always point to a memory buffer 
containing at least count elements of the pointee type. Increasing the value of 
count , like in the example below, would violate this invariant and permit 
out-of-bounds access to the pointer. To avoid this, the compiler emits either a 
compile-time error or a run-time trap. Section “Maintaining correctness of 
bounds annotations”) provides more details about the programming model.
+
+.. code-block:: c
+
+   void foo(int *__counted_by(count) p, size_t count) {  
+  count++; // violates the invariant of __counted_by   
+   }
+
+The requirement to annotate all pointers with explicit bounds information 
could present a significant adoption burden. To tackle this issue, the model 
incorporates the concept of a “wide pointer” (a.k.a. fat pointer) – a larger 
pointer that carries bounds information alongside the pointer value. Utilizing 
wide pointers can potentially reduce the adoption burden, as it contains bounds 
information internally and

[clang] [BoundsSafety] Initial documentation for -fbounds-safety (PR #70749)

[Dan Liew via cfe-commits]

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/70749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [BoundsSafety] Initial documentation for -fbounds-safety (PR #70749)

[Dan Liew via cfe-commits]

@@ -0,0 +1,480 @@
+==
+-fbounds-safety: Enforcing bounds safety for C
+==
+
+.. contents::
+   :local:
+
+Overview
+
+
+-fbounds-safety is a C extension to enforce bounds safety to prevent 
out-of-bounds (OOB) memory accesses, which remain a major source of security 
vulnerabilities in C. -fbounds-safety aims to eliminate this class of bugs by 
turning OOB accesses into deterministic traps.
+
+The -fbounds-safety extension offers bounds annotations that programmers can 
use to attach bounds to pointers. For example, programmers can add the 
__counted_by(N) annotation to parameter ptr, indicating that the pointer has N 
valid elements:

delcypher wrote:

Should `__counted_by(...)` and other attributes be quoted to be monospaced?

https://github.com/llvm/llvm-project/pull/70749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [BoundsSafety] Initial documentation for -fbounds-safety (PR #70749)

[Dan Liew via cfe-commits]

@@ -0,0 +1,480 @@
+==
+-fbounds-safety: Enforcing bounds safety for C
+==
+
+.. contents::
+   :local:
+
+Overview
+
+
+-fbounds-safety is a C extension to enforce bounds safety to prevent 
out-of-bounds (OOB) memory accesses, which remain a major source of security 
vulnerabilities in C. -fbounds-safety aims to eliminate this class of bugs by 
turning OOB accesses into deterministic traps.
+
+The -fbounds-safety extension offers bounds annotations that programmers can 
use to attach bounds to pointers. For example, programmers can add the 
__counted_by(N) annotation to parameter ptr, indicating that the pointer has N 
valid elements:
+
+.. code-block:: c
+
+   void foo(int *__counted_by(N) ptr, size_t N);
+
+Using this bounds information, the compiler inserts bounds checks on every 
pointer dereference, ensuring that the program does not access memory outside 
the specified bounds. The compiler requires programmers to provide enough 
bounds information so that the accesses can be checked at either run time or 
compile time — and it rejects code if it cannot.
+
+The most important contribution of “-fbounds-safety” is how it reduces the 
programmer’s annotation burden by reconciling bounds annotations at ABI 
boundaries with the use of implicit wide pointers (a.k.a. “fat” pointers) that 
carry bounds information on local variables without the need for annotations. 
We designed this model so that it preserves ABI compatibility with C while 
minimizing adoption effort.
+
+The -fbounds-safety extension has been adopted on millions of lines of 
production C code and proven to work in a consumer operating system setting. 
The extension was designed to enable incremental adoption — a key requirement 
in real-world settings where modifying an entire project and its dependencies 
all at once is often not possible. It also addresses multiple of other 
practical challenges that have made existing approaches to safer C dialects 
difficult to adopt, offering these properties that make it widely adoptable in 
practice:
+
+* It is designed to preserve the Application Binary Interface (ABI)
+* It interoperates well with plain C code
+* It can be adopted partially and incrementally while still providing safety 
benefits
+* It is syntactically and semantically compatible with C
+* Consequently, source code that adopts the extension can continue to be 
compiled by toolchains that do not support the extension.
+* It has a relatively low adoption cost
+* It can be implemented on top of Clang
+
+
+Programming Model
+
+
+Overview
+-
+
+-fbounds-safety ensures that pointers are not used to access memory beyond 
their bounds by performing bounds checking. If a bounds check fails, the 
program will deterministically trap before out-of-bounds memory is accessed.
+
+In our model, every pointer has an explicit or implicit bounds attribute that 
determines its bounds and ensures guaranteed bounds checking. Consider the 
example below where the __counted_by(count) annotation indicates that parameter 
ppoints to a buffer of int s containing count elements. An off-by-one error is 
present in the loop condition, leading to p[i]being out-of-bounds access during 
the loop’s final iteration. The compiler inserts a bounds check before p is 
dereferenced to ensure that the access remains within the specified bounds.
+
+.. code-block:: c
+
+   void fill_array_with_indices(int *__counted_by(count) p, unsigned count) {  
+   // off-by-one error (i < count)  
+  for (unsigned i = 0; i <= count; ++i) {  
+ // bounds check inserted:  
+ //   if (i >= count) trap();  
+ p[i] = i;  
+  }  
+   }
+
+A bounds annotation defines an invariant for the pointer type, and the model 
ensures that this invariant remains true. In the example below, pointer p 
annotated with __counted_by(count) must always point to a memory buffer 
containing at least count elements of the pointee type. Increasing the value of 
count , like in the example below, would violate this invariant and permit 
out-of-bounds access to the pointer. To avoid this, the compiler emits either a 
compile-time error or a run-time trap. Section “Maintaining correctness of 
bounds annotations”) provides more details about the programming model.
+
+.. code-block:: c
+
+   void foo(int *__counted_by(count) p, size_t count) {  
+  count++; // violates the invariant of __counted_by   
+   }
+
+The requirement to annotate all pointers with explicit bounds information 
could present a significant adoption burden. To tackle this issue, the model 
incorporates the concept of a “wide pointer” (a.k.a. fat pointer) – a larger 
pointer that carries bounds information alongside the pointer value. Utilizing 
wide pointers can potentially reduce the adoption burden, as it contains bounds 
information internally and

[clang] 3d612a9 - [NFC] Avoid unnecessary duplication of code generating diagnostic.

[Dan Liew via cfe-commits]

Author: Dan Liew
Date: 2022-04-20T10:50:21-07:00
New Revision: 3d612a930dce229c887cd9a731084df419f43791

URL: 
https://github.com/llvm/llvm-project/commit/3d612a930dce229c887cd9a731084df419f43791
DIFF: 
https://github.com/llvm/llvm-project/commit/3d612a930dce229c887cd9a731084df419f43791.diff

LOG: [NFC] Avoid unnecessary duplication of code generating diagnostic.

The previous code unneccessarily duplicated the creation of a diagnostic
where the only difference was the `AssignmentAction` being passed.

rdar://88664722

Differential Revision: https://reviews.llvm.org/D124054

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 7fbe083571e67..58aab637ff308 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16932,10 +16932,12 @@ bool Sema::DiagnoseAssignmentResult(AssignConvertType 
ConvTy,
   }
 
   PartialDiagnostic FDiag = PDiag(DiagKind);
+  AssignmentAction ActionForDiag = Action;
   if (Action == AA_Passing_CFAudited)
-FDiag << FirstType << SecondType << AA_Passing << 
SrcExpr->getSourceRange();
-  else
-FDiag << FirstType << SecondType << Action << SrcExpr->getSourceRange();
+ActionForDiag = AA_Passing;
+
+  FDiag << FirstType << SecondType << ActionForDiag
+<< SrcExpr->getSourceRange();
 
   if (DiagKind == diag::ext_typecheck_convert_incompatible_pointer_sign ||
   DiagKind == diag::err_typecheck_convert_incompatible_pointer_sign) {



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -std=c++20 -emit-llvm 
-debug-info-kind=limited %s -o - | FileCheck %s

delcypher wrote:

Really? We don't test the generated IR in an optimized build? That seems like a 
bad idea given that code built for production use typically is optimized.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -std=c++20 -emit-llvm 
-debug-info-kind=limited %s -o - | FileCheck %s

delcypher wrote:

Really? We don't test the generated IR in an optimized build? That seems like a 
bad idea given that code built for production use typically is optimized.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -3452,6 +3452,18 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
   case Builtin::BI__builtin_trap:
 EmitTrapCall(Intrinsic::trap);
 return RValue::get(nullptr);
+  case Builtin::BI__builtin_verbose_trap: {
+llvm::DILocation *TrapLocation = Builder.getCurrentDebugLocation();
+if (getDebugInfo()) {
+  std::string Str;
+  E->getArg(0)->tryEvaluateString(Str, getContext());
+  TrapLocation =
+  getDebugInfo()->CreateTrapFailureMessageFor(TrapLocation, Str);
+}
+ApplyDebugLocation ApplyTrapDI(*this, TrapLocation);
+EmitTrapCall(Intrinsic::trap);

delcypher wrote:

Please leave a comment in the code indicating that currently no attempt is made 
to prevent traps being merged.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -3424,6 +3447,24 @@ llvm::DIMacroFile 
*CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent,
   return DBuilder.createTempMacroFile(Parent, Line, FName);
 }
 
+llvm::DILocation *
+CGDebugInfo::CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation,
+ StringRef FailureMsg) {
+  // Create a debug location from `TrapLocation` that adds an artificial inline
+  // frame.
+  const char *Prefix = CLANG_VERBOSE_TRAP_PREFIX;

delcypher wrote:

The `Prefix` should not be hardcoded. It should be a parameter to 
`CreateTrapFailureMessageFor` so it can be re-used by other things like 
`-fbounds-safety`

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

https://github.com/delcypher requested changes to this pull request.

It looks like there are still some unresolved discussions in this PR.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -verify %s
+
+#if !__has_builtin(__builtin_verbose_trap)
+#error
+#endif
+
+constexpr char const* constMsg1 = "hello";
+char const* const constMsg2 = "hello";
+char const constMsg3[] = "hello";
+
+template 
+void f(const char * arg) {
+  __builtin_verbose_trap("Argument_must_not_be_null");

delcypher wrote:

As long as the compiler isn't imposing an artificial limit then that seems fine 
to me.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -3379,6 +3379,57 @@ Query for this feature with 
``__has_builtin(__builtin_debugtrap)``.
 
 Query for this feature with ``__has_builtin(__builtin_trap)``.
 
+``__builtin_verbose_trap``
+--
+
+``__builtin_verbose_trap`` causes the program to stop its execution abnormally
+and shows a human-readable description of the reason for the termination when a
+debugger is attached or in a symbolicated crash log.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_verbose_trap(const char *reason)
+
+**Description**
+
+``__builtin_verbose_trap`` is lowered to the ` ``llvm.trap`` 
`_ builtin.
+Additionally, clang emits debugging information that represents an artificial
+inline frame whose name encodes the string passed to the builtin, prefixed by a
+"magic" prefix.

delcypher wrote:

@ahatanak Any follow up?

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -3379,6 +3379,60 @@ Query for this feature with 
``__has_builtin(__builtin_debugtrap)``.
 
 Query for this feature with ``__has_builtin(__builtin_trap)``.
 
+``__builtin_verbose_trap``
+--
+
+``__builtin_verbose_trap`` causes the program to stop its execution abnormally
+and shows a human-readable description of the reason for the termination when a
+debugger is attached or in a symbolicated crash log.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_verbose_trap(const char *reason)
+
+**Description**
+
+``__builtin_verbose_trap`` is lowered to the ` ``llvm.trap`` 
`_ builtin.
+Additionally, clang emits debugging information that represents an artificial
+inline frame whose name encodes the string passed to the builtin, prefixed by a
+"magic" prefix.
+
+For example, consider the following code:
+
+.. code-block:: c++
+
+void foo(int* p) {
+  if (p == nullptr)
+__builtin_verbose_trap("Argument must not be null!");
+}
+
+The debugging information would look as if it were produced for the following 
code:
+
+.. code-block:: c++
+
+__attribute__((always_inline))
+inline void "__llvm_verbose_trap: Argument must not be null!"() {
+  __builtin_trap();
+}
+
+void foo(int* p) {
+  if (p == nullptr)
+"__llvm_verbose_trap: Argument must not be null!"();
+}
+
+However, the LLVM IR would not actually contain a call to the artificial

delcypher wrote:

```suggestion
However, the generated code would not actually contain a call to the artificial
```

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -3379,6 +3379,54 @@ Query for this feature with 
``__has_builtin(__builtin_debugtrap)``.
 
 Query for this feature with ``__has_builtin(__builtin_trap)``.
 
+``__builtin_verbose_trap``
+--
+
+``__builtin_verbose_trap`` causes the program to stop its execution abnormally
+and shows a human-readable description of the reason for the termination when a
+debugger is attached or in a symbolicated crash log.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_verbose_trap(const char *reason)
+
+**Description**
+
+``__builtin_verbose_trap`` is lowered to the ` ``llvm.trap`` 
`_ builtin.
+Additionally, clang emits debug metadata that represents an artificial inline
+frame whose name encodes the string passed to the builtin, prefixed by a 
"magic"
+prefix.
+
+For example, consider the following code:
+
+.. code-block:: c++
+
+void foo(int* p) {
+  if (p == nullptr)
+__builtin_verbose_trap("Argument_must_not_be_null");
+}
+
+The debug metadata would look as if it were produced for the following code:
+
+.. code-block:: c++
+
+__attribute__((always_inline))
+inline void "__llvm_verbose_trap: Argument_must_not_be_null"() {
+  __builtin_trap();
+}
+
+void foo(int* p) {
+  if (p == nullptr)
+"__llvm_verbose_trap: Argument_must_not_be_null"();
+}
+
+However, the LLVM IR would not actually contain a call to the artificial
+function — it only exists in the debug metadata.

delcypher wrote:

@ahatanak Any follow up?

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -346,6 +348,15 @@ class CGDebugInfo {
   const FieldDecl *BitFieldDecl, const llvm::DIDerivedType *BitFieldDI,
   llvm::ArrayRef PreviousFieldsDI, const RecordDecl *RD);
 
+  // A cache that maps artificial inlined function names used for
+  // __builtin_verbose_trap to subprograms.
+  llvm::StringMap InlinedTrapFuncMap;
+
+  // A function that returns the subprogram corresponding to the artificial
+  // inlined function for traps.

delcypher wrote:

@ahatanak Any follow up?

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -346,6 +348,15 @@ class CGDebugInfo {
   const FieldDecl *BitFieldDecl, const llvm::DIDerivedType *BitFieldDI,
   llvm::ArrayRef PreviousFieldsDI, const RecordDecl *RD);
 
+  // A cache that maps artificial inlined function names used for
+  // __builtin_verbose_trap to subprograms.

delcypher wrote:

@ahatanak Any follow up?

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -602,6 +613,19 @@ class CGDebugInfo {
 return CoroutineParameterMappings;
   }
 
+  // Create a debug location from `TrapLocation` that adds an artificial inline
+  // frame where the frame name is
+  //
+  // * `: ` if `` is not empty.
+  // * `` if `` is empty. Note `` must
+  //   contain a space.
+  //
+  // Currently `` is always "__llvm_verbose_trap".
+  //
+  // This is used to store failure reasons for traps.

delcypher wrote:

@ahatanak Any follow up?

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -std=c++20 -emit-llvm 
-debug-info-kind=limited %s -o - | FileCheck %s

delcypher wrote:

@ahatanak Any follow up?

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

https://github.com/delcypher deleted 
https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -3424,6 +3445,26 @@ llvm::DIMacroFile 
*CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent,
   return DBuilder.createTempMacroFile(Parent, Line, FName);
 }
 
+llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor(

delcypher wrote:

@dwblaikie We currently use this function for `-fbounds-safety` internally. We 
are currently in the process of upstreaming this which is why the function 
should **not** hardcode the `Prefix`.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -std=c++20 -emit-llvm 
-debug-info-kind=limited %s -o - | FileCheck %s

delcypher wrote:

@pogo59 @ahatanak Thanks for the explanation.

A slight issue with that is some cases Clang will generate different 
(unoptimized IR) depending on the optimization level (see 
`CodeGenFunction::EmitTrapCheck`) so in those cases you have to test Clang's 
optimizing behavior. You're not using that here so that's fine

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Turn 'counted_by' into a type attribute and parse it into 'CountAttributedType' (PR #78000)

[Dan Liew via cfe-commits]

delcypher wrote:

@nathanchance Thanks for reporting this. I'm going to have a quick go at 
reproducing this to see if fixing this is straight forward. If it is not I will 
revert this PR and then we can re-land this change with the problem you 
reported fixed.

https://github.com/llvm/llvm-project/pull/78000
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Unwrap CountAttributed for debug info (PR #86017)

[Dan Liew via cfe-commits]

https://github.com/delcypher approved this pull request.

LGTM. Thanks for the quick fix.

https://github.com/llvm/llvm-project/pull/86017
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -3379,6 +3379,60 @@ Query for this feature with 
``__has_builtin(__builtin_debugtrap)``.
 
 Query for this feature with ``__has_builtin(__builtin_trap)``.
 
+``__builtin_verbose_trap``

delcypher wrote:

@ahatanak Is this in the wrong place in the document? `__builtin_verbose_trap` 
is lexicographically after `__builtin_nondeterministic_value`

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -1628,6 +1628,25 @@ llvm::DIType *CGDebugInfo::createFieldType(
offsetInBits, flags, debugType, 
Annotations);
 }
 
+llvm::DISubprogram *
+CGDebugInfo::createInlinedTrapSubprogram(StringRef FuncName,
+ llvm::DIFile *FileScope) {
+  llvm::DISubprogram *&SP = InlinedTrapFuncMap[FuncName];

delcypher wrote:

I think it's worth adding a comment here explaining **why** we are caching the 
sub-programs.

The goal is to avoid creating duplicate sub program nodes because 
`llvm::DISubprogram::SPFlagDefinition` prevents unique-ing.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -775,6 +775,11 @@ class Expr : public ValueStmt {
  const Expr *PtrExpression, ASTContext &Ctx,
  EvalResult &Status) const;
 
+  /// If the current Expr can be evaluated to a pointer to a null-terminated
+  /// constant string, return the constant string (without the terminating 
null)
+  /// in Result. Return true if it succeeds.
+  bool tryEvaluateString(std::string &Result, ASTContext &Ctx) const;

delcypher wrote:

Nit:  Returning `std::optional` might be a cleaner API to provide.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -std=c++20 -emit-llvm 
-debug-info-kind=limited %s -o - | FileCheck %s
+
+// CHECK-LABEL: define void @_Z2f0v()
+// CHECK: call void @llvm.trap(), !dbg ![[LOC17:.*]]
+
+// CHECK-LABEL: define void @_Z2f1v()
+// CHECK: call void @llvm.trap(), !dbg ![[LOC23:.*]]
+// CHECK: call void @llvm.trap(), !dbg ![[LOC25:.*]]
+
+// CHECK-LABEL: define void @_Z2f3v()
+// CHECK: call void @_Z2f2IXadsoKcL_ZL8constMsgvv()
+
+// CHECK-LABEL: define internal void @_Z2f2IXadsoKcL_ZL8constMsgvv()
+// CHECK: call void @llvm.trap(), !dbg ![[LOC36:.*]]
+
+// CHECK: ![[FILESCOPE:.*]] = !DIFile(filename: 
"{{.*}}debug-info-verbose-trap.cpp"
+// CHECK: ![[SUBPROG14:.*]] = distinct !DISubprogram(name: "f0", linkageName: 
"_Z2f0v",
+// CHECK: ![[LOC17]] = !DILocation(line: 0, scope: ![[SUBPROG18:.*]], 
inlinedAt: ![[LOC20:.*]])
+// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__llvm_verbose_trap: 
Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: 
!{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}})

delcypher wrote:

Can we check that `!DISubprogram(name: "__llvm_verbose_trap: 
Argument_must_not_be_null" ...)` is not duplicated? I.e. what the cache is for.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -std=c++20 -emit-llvm 
-debug-info-kind=limited %s -o - | FileCheck %s
+
+// CHECK-LABEL: define void @_Z2f0v()
+// CHECK: call void @llvm.trap(), !dbg ![[LOC17:.*]]
+
+// CHECK-LABEL: define void @_Z2f1v()
+// CHECK: call void @llvm.trap(), !dbg ![[LOC23:.*]]
+// CHECK: call void @llvm.trap(), !dbg ![[LOC25:.*]]
+
+// CHECK-LABEL: define void @_Z2f3v()
+// CHECK: call void @_Z2f2IXadsoKcL_ZL8constMsgvv()
+
+// CHECK-LABEL: define internal void @_Z2f2IXadsoKcL_ZL8constMsgvv()
+// CHECK: call void @llvm.trap(), !dbg ![[LOC36:.*]]
+
+// CHECK: ![[FILESCOPE:.*]] = !DIFile(filename: 
"{{.*}}debug-info-verbose-trap.cpp"
+// CHECK: ![[SUBPROG14:.*]] = distinct !DISubprogram(name: "f0", linkageName: 
"_Z2f0v",
+// CHECK: ![[LOC17]] = !DILocation(line: 0, scope: ![[SUBPROG18:.*]], 
inlinedAt: ![[LOC20:.*]])
+// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__llvm_verbose_trap: 
Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: 
!{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}})
+// CHECK: ![[LOC20]] = !DILocation(line: 34, column: 3, scope: ![[SUBPROG14]])

delcypher wrote:

These check lines that have line numbers in them are fragile with respect to 
people adding/removing lines. Could we move the check next to the relevant line 
and use `[[@LINE+1]` to avoid this?

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -std=c++20 -emit-llvm 
-debug-info-kind=limited %s -o - | FileCheck %s

delcypher wrote:

Could we add a separate test that tests the trap merging behavior in optimized 
code?

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -3452,6 +3452,18 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
   case Builtin::BI__builtin_trap:
 EmitTrapCall(Intrinsic::trap);
 return RValue::get(nullptr);
+  case Builtin::BI__builtin_verbose_trap: {
+llvm::DILocation *TrapLocation = Builder.getCurrentDebugLocation();
+if (getDebugInfo()) {
+  std::string Str;
+  E->getArg(0)->tryEvaluateString(Str, getContext());
+  TrapLocation =
+  getDebugInfo()->CreateTrapFailureMessageFor(TrapLocation, Str);
+}
+ApplyDebugLocation ApplyTrapDI(*this, TrapLocation);
+EmitTrapCall(Intrinsic::trap);

delcypher wrote:

We aren't doing anything here to prevent traps being merged that have either or 
different or same traps reasons.

We could support the following modes:

1. Merge all traps (this will drop the trap reason from the debug info). Good 
for code size, terrible for debugging
2. Prevent merging traps with different trap reasons. Good for debugging, not 
good for code size.
3. Prevent merging traps, even ones with the same trap reason. Even better for 
debugging because we can see in the debugger exactly where the trap came from. 
Even worse for code size.

I think it would be good to have a front end flag to control this.

I would be fine with you tackling this in a separate patch.


https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -3379,6 +3379,57 @@ Query for this feature with 
``__has_builtin(__builtin_debugtrap)``.
 
 Query for this feature with ``__has_builtin(__builtin_trap)``.
 
+``__builtin_verbose_trap``
+--
+
+``__builtin_verbose_trap`` causes the program to stop its execution abnormally
+and shows a human-readable description of the reason for the termination when a
+debugger is attached or in a symbolicated crash log.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_verbose_trap(const char *reason)
+
+**Description**
+
+``__builtin_verbose_trap`` is lowered to the ` ``llvm.trap`` 
`_ builtin.
+Additionally, clang emits debugging information that represents an artificial
+inline frame whose name encodes the string passed to the builtin, prefixed by a
+"magic" prefix.

delcypher wrote:

This documentation doesn't say anything about whether or not Clang will merge 
traps (either with the same reason or different reasons). Should it?

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -verify %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions -verify %s
+
+#if !__has_builtin(__builtin_verbose_trap)
+#error
+#endif
+
+constexpr char const* constMsg1 = "hello";
+char const* const constMsg2 = "hello";
+char const constMsg3[] = "hello";
+
+template 
+void f(const char * arg) {
+  __builtin_verbose_trap("Arbitrary string literals can be used!");
+  __builtin_verbose_trap("Argument_must_not_be_null");
+  __builtin_verbose_trap("hello" "world");
+  __builtin_verbose_trap(constMsg1);
+  __builtin_verbose_trap(constMsg2);
+  __builtin_verbose_trap("");
+  __builtin_verbose_trap(); // expected-error {{too few arguments}}
+  __builtin_verbose_trap(0); // expected-error {{argument to 
__builtin_verbose_trap must be a pointer to a constant string}}
+  __builtin_verbose_trap(1); // expected-error {{cannot initialize a parameter 
of type 'const char *' with}}

delcypher wrote:

Did this error message get truncated? Is this deliberate?

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -verify %s
+
+#if !__has_builtin(__builtin_verbose_trap)
+#error
+#endif
+
+constexpr char const* constMsg1 = "hello";
+char const* const constMsg2 = "hello";
+char const constMsg3[] = "hello";
+
+template 
+void f(const char * arg) {
+  __builtin_verbose_trap("Argument_must_not_be_null");

delcypher wrote:

Imposing this length limit seems pretty arbitrary.  What's the actually 
restriction on the debug info/LLDB side? If there isn't one I don't think we 
should be adding an artificial limit without a good reason for it.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

https://github.com/delcypher requested changes to this pull request.

Looks pretty good. I have a few minor comments. Some things I suggest we may 
want to do in a follow up patch.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -3424,6 +3443,26 @@ llvm::DIMacroFile 
*CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent,
   return DBuilder.createTempMacroFile(Parent, Line, FName);
 }
 
+llvm::DILocation *
+CGDebugInfo::CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation,
+ StringRef FailureMsg) {
+  // Create a debug location from `TrapLocation` that adds an artificial inline
+  // frame.
+  const char *Prefix = "__llvm_verbose_trap";

delcypher wrote:

Why isn't the `Prefix` a parameter? This prevent re-use for things like 
`-fbounds-safety`

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -std=c++20 -emit-llvm 
-debug-info-kind=limited %s -o - | FileCheck %s
+
+// CHECK-LABEL: define void @_Z2f0v()
+// CHECK: call void @llvm.trap(), !dbg ![[LOC17:.*]]
+
+// CHECK-LABEL: define void @_Z2f1v()
+// CHECK: call void @llvm.trap(), !dbg ![[LOC23:.*]]
+// CHECK: call void @llvm.trap(), !dbg ![[LOC25:.*]]
+
+// CHECK-LABEL: define void @_Z2f3v()
+// CHECK: call void @_Z2f2IXadsoKcL_ZL8constMsgvv()
+
+// CHECK-LABEL: define internal void @_Z2f2IXadsoKcL_ZL8constMsgvv()
+// CHECK: call void @llvm.trap(), !dbg ![[LOC36:.*]]
+
+// CHECK: ![[FILESCOPE:.*]] = !DIFile(filename: 
"{{.*}}debug-info-verbose-trap.cpp"
+// CHECK: ![[SUBPROG14:.*]] = distinct !DISubprogram(name: "f0", linkageName: 
"_Z2f0v",
+// CHECK: ![[LOC17]] = !DILocation(line: 0, scope: ![[SUBPROG18:.*]], 
inlinedAt: ![[LOC20:.*]])
+// CHECK: ![[SUBPROG18]] = distinct !DISubprogram(name: "__llvm_verbose_trap: 
Argument_must_not_be_null", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: 
!{{.*}}, flags: DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}})
+// CHECK: ![[LOC20]] = !DILocation(line: 34, column: 3, scope: ![[SUBPROG14]])
+// CHECK: ![[SUBPROG22:.*]] = distinct !DISubprogram(name: "f1", linkageName: 
"_Z2f1v",
+// CHECK: ![[LOC23]] = !DILocation(line: 0, scope: ![[SUBPROG18]], inlinedAt: 
![[LOC24:.*]])
+// CHECK: ![[LOC24]] = !DILocation(line: 38, column: 3, scope: ![[SUBPROG22]])
+// CHECK: ![[LOC25]] = !DILocation(line: 0, scope: ![[SUBPROG26:.*]], 
inlinedAt: ![[LOC27:.*]])
+// CHECK: ![[SUBPROG26]] = distinct !DISubprogram(name: "__llvm_verbose_trap: 
hello", scope: ![[FILESCOPE]], file: ![[FILESCOPE]], type: !{{.*}}, flags: 
DIFlagArtificial, spFlags: DISPFlagDefinition, unit: !{{.*}})
+// CHECK: ![[LOC27]] = !DILocation(line: 39, column: 3, scope: ![[SUBPROG22]])
+// CHECK: ![[SUBPROG32:.*]] = distinct !DISubprogram(name: "f2<&constMsg[0]>", 
linkageName: "_Z2f2IXadsoKcL_ZL8constMsgvv",
+// CHECK: ![[LOC36]] = !DILocation(line: 0, scope: ![[SUBPROG26]], inlinedAt: 
![[LOC37:.*]])
+// CHECK: ![[LOC37]] = !DILocation(line: 44, column: 3, scope: ![[SUBPROG32]])
+

delcypher wrote:

Should this test check that the `llvm.trap()` function has the `cold` attribute 
(hint to the optimizer that its unlikely for the function to be called) on it?

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -3416,6 +3437,27 @@ llvm::DIMacroFile 
*CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent,
   return DBuilder.createTempMacroFile(Parent, Line, FName);
 }
 
+llvm::DILocation *CGDebugInfo::CreateTrapFailureMessageFor(
+llvm::DebugLoc TrapLocation, StringRef Prefix, StringRef FailureMsg) {
+  // Create debug info that describes a fake function whose name is the failure
+  // message.
+  std::string FuncName(Prefix);
+  if (!FailureMsg.empty()) {
+// A space in the function name identifies this as not being a real 
function
+// because it's not a valid symbol name.
+FuncName += ": ";
+FuncName += FailureMsg;
+  }
+
+  assert(FuncName.size() > 0);
+  assert(FuncName.find(' ') != std::string::npos &&

delcypher wrote:

@ahatanak 
> I think it means Prefix is required to have a space only when FailureMsg is 
> empty. If FailureMsg is empty, FuncName is the same as Prefix, so the checks 
> seem correct to me. @delcypher is that correct?

The actually intent here was to ensure the artificial function has a space in 
it. That's it, nothing more complicated than that. The check is done on the 
final function name and not earlier so the assert doesn't need to care about 
the prefix.

The origin of requiring a space was based on an off-hand comment (I think) 
@adrian-prantl made a while back to me that having a space in the function name 
(in C at least) would not be a valid function identifier (and therefore 
wouldn't conflict with any existing function).

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -3424,6 +3443,26 @@ llvm::DIMacroFile 
*CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent,
   return DBuilder.createTempMacroFile(Parent, Line, FName);
 }
 
+llvm::DILocation *
+CGDebugInfo::CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation,
+ StringRef FailureMsg) {
+  // Create a debug location from `TrapLocation` that adds an artificial inline
+  // frame.
+  const char *Prefix = "__llvm_verbose_trap";
+  SmallString<64> FuncName(Prefix);
+  if (!FailureMsg.empty()) {
+// A space in the function name identifies this as not being a real 
function
+// because it's not a valid symbol name.

delcypher wrote:

@adrian-prantl Sorry. This comment is based on an off-hand comment you made a 
while back in the context of `-fbounds-safety` (which only supports C) where I 
think you said having a space in the function name would distinguish the 
artificial function from any real function. While that's true for C I guess 
it's not true for other languages.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -3424,6 +3443,26 @@ llvm::DIMacroFile 
*CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent,
   return DBuilder.createTempMacroFile(Parent, Line, FName);
 }
 
+llvm::DILocation *
+CGDebugInfo::CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation,
+ StringRef FailureMsg) {
+  // Create a debug location from `TrapLocation` that adds an artificial inline
+  // frame.
+  const char *Prefix = "__llvm_verbose_trap";

delcypher wrote:

Could "__llvm_verbose_trap" live in a shared header file? That way LLDB doesn't 
need to hardcode the string in a frame recognizer implementation. It can just 
use the string in the header file and that way things stay in sync 
automatically.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add support for builtin_verbose_trap (PR #79230)

[Dan Liew via cfe-commits]

@@ -346,6 +348,15 @@ class CGDebugInfo {
   const FieldDecl *BitFieldDecl, const llvm::DIDerivedType *BitFieldDI,
   llvm::ArrayRef PreviousFieldsDI, const RecordDecl *RD);
 
+  // A cache that maps artificial inlined function names used for
+  // __builtin_verbose_trap to subprograms.
+  llvm::StringMap InlinedTrapFuncMap;

delcypher wrote:

Just to check. What's the life time of CGDebugInfo? Is it a single instance per 
complication unit? If it's longer than that (i.e. the same instance gets used 
for multiple compilation units) then we might cache the sub programs for too 
long.

https://github.com/llvm/llvm-project/pull/79230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

https://github.com/delcypher requested changes to this pull request.

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

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -3618,6 +3618,23 @@ void CompilerInvocationBase::GenerateLangArgs(const 
LangOptions &Opts,
 GenerateArg(Consumer, OPT_frandomize_layout_seed_EQ, Opts.RandstructSeed);
 }
 
+static bool SupportsBoundsSafety(Language Lang) {
+  // Currently, bounds safety is only supported for C. However, it's also
+  // possible to pass assembly files and LLVM IR through Clang, and
+  // those should be trivially supported. This is especially important because
+  // some build systems, like xcbuild and somewhat clumsy Makefiles, will pass
+  // C_FLAGS to Clang while building assembly files.
+  switch (Lang) {
+  case Language::Unknown:
+  case Language::Asm:

delcypher wrote:

Can we have the compiler emit a warning that `-fbounds-safety-experimental` is 
ignored when ASM/LLVM IR/Unknown is compiled? I think that's better than 
silently accepting the flag but the flag does nothing.

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -3835,6 +3852,12 @@ bool CompilerInvocation::ParseLangArgs(LangOptions 
&Opts, ArgList &Args,
   Opts.Trigraphs =
   Args.hasFlag(OPT_ftrigraphs, OPT_fno_trigraphs, Opts.Trigraphs);
 
+  Opts.BoundsSafety = Args.hasFlag(OPT_fbounds_safety, OPT_fno_bounds_safety,

delcypher wrote:

I thought using `LangOpts<"BoundsSafety">` in the flag definition was supposed 
to automatically propagate the flag value in the frontend into the language 
options for you. Is that not happening?

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -330,6 +330,9 @@ def warn_alias_with_section : Warning<
   "as the %select{aliasee|resolver}2">,
   InGroup;
 
+def error_bounds_safety_lang_not_supported : Error<

delcypher wrote:

Nit: A lot of the error diagnostics use `err` as a preifx rather than `error`.

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -0,0 +1,9 @@
+// RUN: %clang -c %s -### 2>&1 | not grep fbounds-safety-experimental

delcypher wrote:

Could we use `FileCheck` here instead of `grep`?

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -330,6 +330,9 @@ def warn_alias_with_section : Warning<
   "as the %select{aliasee|resolver}2">,
   InGroup;
 
+def error_bounds_safety_lang_not_supported : Error<

delcypher wrote:

Should we introduce a "bounds safety" category and have the diagnostics fall 
under that. E.g. like the `let CategoryName = "Instrumentation Issue" in` code 
below?

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -3618,6 +3618,23 @@ void CompilerInvocationBase::GenerateLangArgs(const 
LangOptions &Opts,
 GenerateArg(Consumer, OPT_frandomize_layout_seed_EQ, Opts.RandstructSeed);
 }
 
+static bool SupportsBoundsSafety(Language Lang) {
+  // Currently, bounds safety is only supported for C. However, it's also
+  // possible to pass assembly files and LLVM IR through Clang, and
+  // those should be trivially supported. This is especially important because
+  // some build systems, like xcbuild and somewhat clumsy Makefiles, will pass
+  // C_FLAGS to Clang while building assembly files.
+  switch (Lang) {
+  case Language::Unknown:
+  case Language::Asm:
+  case Language::LLVM_IR:

delcypher wrote:

Can we add test cases for LLVM IR and ASM if this is something we want the 
frontend to allow?

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Driver][BoundsSafety] Add -fbounds-safety-experimental flag (PR #70480)

[Dan Liew via cfe-commits]

@@ -3835,6 +3852,12 @@ bool CompilerInvocation::ParseLangArgs(LangOptions 
&Opts, ArgList &Args,
   Opts.Trigraphs =
   Args.hasFlag(OPT_ftrigraphs, OPT_fno_trigraphs, Opts.Trigraphs);
 
+  Opts.BoundsSafety = Args.hasFlag(OPT_fbounds_safety, OPT_fno_bounds_safety,

delcypher wrote:

I think this is supposed to happen in 
`CompilerInvocationBase::GenerateLangArgs(...)`

https://github.com/llvm/llvm-project/pull/70480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fdce098 - [Clang][ASan] Teach Clang to not emit ASan module destructors when compiling with `-mkernel` or `-fapple-kext`.

[Dan Liew via cfe-commits]

Author: Dan Liew
Date: 2021-02-25T12:02:21-08:00
New Revision: fdce098b49cb038996441741a7b2ab3652502aec

URL: 
https://github.com/llvm/llvm-project/commit/fdce098b49cb038996441741a7b2ab3652502aec
DIFF: 
https://github.com/llvm/llvm-project/commit/fdce098b49cb038996441741a7b2ab3652502aec.diff

LOG: [Clang][ASan] Teach Clang to not emit ASan module destructors when 
compiling with `-mkernel` or `-fapple-kext`.

rdar://71609176

Differential Revision: https://reviews.llvm.org/D96573

Added: 
clang/test/Driver/darwin-asan-mkernel-kext.c

Modified: 
clang/lib/Driver/SanitizerArgs.cpp

Removed: 




diff  --git a/clang/lib/Driver/SanitizerArgs.cpp 
b/clang/lib/Driver/SanitizerArgs.cpp
index a3d5a2f567c5..0f9f5d87696c 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -825,6 +825,12 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   AsanInvalidPointerSub = true;
 }
 
+if (TC.getTriple().isOSDarwin() &&
+(Args.hasArg(options::OPT_mkernel) ||
+ Args.hasArg(options::OPT_fapple_kext))) {
+  AsanDtorKind = llvm::AsanDtorKind::None;
+}
+
 if (const auto *Arg =
 Args.getLastArg(options::OPT_sanitize_address_destructor_kind_EQ)) 
{
   auto parsedAsanDtorKind = AsanDtorKindFromString(Arg->getValue());

diff  --git a/clang/test/Driver/darwin-asan-mkernel-kext.c 
b/clang/test/Driver/darwin-asan-mkernel-kext.c
new file mode 100644
index ..7827d2ec3fd1
--- /dev/null
+++ b/clang/test/Driver/darwin-asan-mkernel-kext.c
@@ -0,0 +1,15 @@
+// RUN: %clang -target x86_64-apple-darwin10 -fsanitize=address -mkernel -### \
+// RUN:   %s 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-apple-darwin10 -fsanitize=address -fapple-kext \
+// RUN:   -### %s 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-apple-darwin10 -fsanitize=address -fapple-kext \
+// RUN:   -mkernel -### %s 2>&1 | FileCheck %s
+
+// CHECK: "-fsanitize-address-destructor-kind=none"
+
+// Check it's possible to override the driver's decision.
+// RUN: %clang -target x86_64-apple-darwin10 -fsanitize=address -fapple-kext \
+// RUN:   -mkernel -### -fsanitize-address-destructor-kind=global %s 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-OVERRIDE %s
+
+// CHECK-OVERRIDE: "-fsanitize-address-destructor-kind=global"



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 5d64dd8 - [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

[Dan Liew via cfe-commits]

Author: Dan Liew
Date: 2021-02-25T12:02:21-08:00
New Revision: 5d64dd8e3c22e12e4f7b3d08ffe88fc41e727117

URL: 
https://github.com/llvm/llvm-project/commit/5d64dd8e3c22e12e4f7b3d08ffe88fc41e727117
DIFF: 
https://github.com/llvm/llvm-project/commit/5d64dd8e3c22e12e4f7b3d08ffe88fc41e727117.diff

LOG: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & 
frontend option.

The new `-fsanitize-address-destructor-kind=` option allows control over how 
module
destructors are emitted by ASan.

The new option is consumed by both the driver and the frontend and is 
propagated into
codegen options by the frontend.

Both the legacy and new pass manager code have been updated to consume the new 
option
from the codegen options.

It would be nice if the new utility functions (`AsanDtorKindToString` and
`AsanDtorKindFromString`) could live in LLVM instead of Clang so they could be
consumed by other language frontends. Unfortunately that doesn't work because
the clang driver doesn't link against the LLVM instrumentation library.

rdar://71609176

Differential Revision: https://reviews.llvm.org/D96572

Added: 
clang/test/CodeGen/asan-destructor-kind.cpp
clang/test/Driver/fsanitize-address-destructor-kind.c

Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Basic/Sanitizers.h
clang/include/clang/Driver/Options.td
clang/include/clang/Driver/SanitizerArgs.h
clang/lib/Basic/Sanitizers.cpp
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/SanitizerArgs.cpp
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index f7ac97484d91..0038dccd53f9 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -870,6 +870,17 @@ Enable use-after-scope detection in AddressSanitizer
 
 Enable ODR indicator globals to avoid false ODR violation reports in partially 
sanitized programs at the cost of an increase in binary size
 
+.. option:: -fsanitize-address-destructor-kind=
+
+Set the kind of module destructors emitted by AddressSanitizer instrumentation.
+These destructors are emitted to unregister instrumented global variables when
+code is unloaded (e.g. via `dlclose()`).
+
+Valid options are:
+
+* ``global`` - Emit module destructors that are called via a platform specific 
array (see `llvm.global_dtors`).
+* ``none`` - Do not emit module destructors.
+
 .. option:: -fsanitize-blacklist=
 
 Path to blacklist file for sanitizers

diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index 9d53b5b923bb..832f7fad3551 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -214,6 +214,9 @@ CODEGENOPT(SanitizeAddressGlobalsDeadStripping, 1, 0) ///< 
Enable linker dead st
 CODEGENOPT(SanitizeAddressUseOdrIndicator, 1, 0) ///< Enable ODR indicator 
globals
 CODEGENOPT(SanitizeMemoryTrackOrigins, 2, 0) ///< Enable tracking origins in
  ///< MemorySanitizer
+ENUM_CODEGENOPT(SanitizeAddressDtorKind, llvm::AsanDtorKind, 2,
+llvm::AsanDtorKind::Global)  ///< Set how ASan global
+ ///< destructors are emitted.
 CODEGENOPT(SanitizeMemoryUseAfterDtor, 1, 0) ///< Enable use-after-delete 
detection
  ///< in MemorySanitizer
 CODEGENOPT(SanitizeCfiCrossDso, 1, 0) ///< Enable cross-dso support in CFI.

diff  --git a/clang/include/clang/Basic/CodeGenOptions.h 
b/clang/include/clang/Basic/CodeGenOptions.h
index 878d55402e32..fe83887ad19d 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -20,6 +20,7 @@
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Target/TargetOptions.h"
+#include "llvm/Transforms/Instrumentation/AddressSanitizerOptions.h"
 #include 
 #include 
 #include 

diff  --git a/clang/include/clang/Basic/Sanitizers.h 
b/clang/include/clang/Basic/Sanitizers.h
index 675fa15fdec8..1acce0a6e09e 100644
--- a/clang/include/clang/Basic/Sanitizers.h
+++ b/clang/include/clang/Basic/Sanitizers.h
@@ -17,6 +17,7 @@
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MathExtras.h"
+#include "llvm/Transforms/Instrumentation/AddressSanitizerOptions.h"
 #include 
 #include 
 
@@ -193,6 +194,10 @@ inline SanitizerMask getPPTransparentSanitizers() {
  SanitizerKind::Undefined | SanitizerKind::FloatDivideByZero;
 }
 
+StringRef AsanDtorKindToString(llvm::AsanDtorKind kind);
+
+llvm::AsanDtorKind AsanDtorKindFromString(StringRef kind);
+
 } // namespace clang
 
 #endif // LLVM_CLAN

[clang] 7b1d2a2 - [NFC] Switch to auto marshalling infrastructure for `-fsanitize-address-destructor-kind=` flag.

[Dan Liew via cfe-commits]

Author: Dan Liew
Date: 2021-02-25T13:24:50-08:00
New Revision: 7b1d2a2891d812ffc1bb08712143c79e457acbd4

URL: 
https://github.com/llvm/llvm-project/commit/7b1d2a2891d812ffc1bb08712143c79e457acbd4
DIFF: 
https://github.com/llvm/llvm-project/commit/7b1d2a2891d812ffc1bb08712143c79e457acbd4.diff

LOG: [NFC] Switch to auto marshalling infrastructure for 
`-fsanitize-address-destructor-kind=` flag.

This change simplifies `clang/lib/Frontend/CompilerInvocation.cpp`
because we no longer need to manually parse the flag and set codegen
options in the frontend. However, we still need to manually parse the
flag in the driver because:

* The marshalling infrastructure doesn't operate there.
* We need to do some platform specific checks in the driver
  that will likely never be supported by any kind of marshalling
  infrastructure.

rdar://71609176

Differential Revision: https://reviews.llvm.org/D97327

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/asan-destructor-kind.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index fbf27d14258e..2724bab32b84 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1500,11 +1500,16 @@ defm sanitize_address_use_odr_indicator : 
BoolOption<"f", "sanitize-address-use-
 " reports in partially sanitized programs at the cost of an 
increase in binary size">,
   NegFlag>,
   Group;
-def sanitize_address_destructor_kind_EQ : Joined<["-"], 
"fsanitize-address-destructor-kind=">,
-  MetaVarName<"">,
-  Flags<[CC1Option]>,
-  HelpText<"Set destructor type used in ASan instrumentation">,
-  Group;
+def sanitize_address_destructor_kind_EQ
+: Joined<["-"], "fsanitize-address-destructor-kind=">,
+  MetaVarName<"">,
+  Flags<[CC1Option]>,
+  HelpText<"Set destructor type used in ASan instrumentation">,
+  Group,
+  Values<"none,global">,
+  NormalizedValuesScope<"llvm::AsanDtorKind">,
+  NormalizedValues<["None", "Global"]>,
+  MarshallingInfoEnum, "Global">;
 // Note: This flag was introduced when it was necessary to distinguish between
 //   ABI for correct codegen.  This is no longer needed, but the flag is
 //   not removed since targeting either ABI will behave the same.

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 12dbd459d07e..eb447541124a 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1937,19 +1937,6 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions 
&Opts, ArgList &Args,
 
   Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true);
 
-  if (LangOptsRef.Sanitize.has(SanitizerKind::Address)) {
-if (Arg *A =
-Args.getLastArg(options::OPT_sanitize_address_destructor_kind_EQ)) 
{
-  auto destructorKind = AsanDtorKindFromString(A->getValue());
-  if (destructorKind == llvm::AsanDtorKind::Invalid) {
-Diags.Report(clang::diag::err_drv_unsupported_option_argument)
-<< A->getOption().getName() << A->getValue();
-  } else {
-Opts.setSanitizeAddressDtorKind(destructorKind);
-  }
-}
-  }
-
   if (Args.hasArg(options::OPT_ffinite_loops))
 Opts.FiniteLoops = CodeGenOptions::FiniteLoopsKind::Always;
   else if (Args.hasArg(options::OPT_fno_finite_loops))

diff  --git a/clang/test/CodeGen/asan-destructor-kind.cpp 
b/clang/test/CodeGen/asan-destructor-kind.cpp
index 2bdb782db2b5..567482b72643 100644
--- a/clang/test/CodeGen/asan-destructor-kind.cpp
+++ b/clang/test/CodeGen/asan-destructor-kind.cpp
@@ -3,7 +3,7 @@
 // RUN:   -fsanitize-address-destructor-kind=bad_arg -emit-llvm -o - \
 // RUN:   -triple x86_64-apple-macosx10.15 %s 2>&1 | \
 // RUN:   FileCheck %s --check-prefixes=CHECK-BAD-ARG
-// CHECK-BAD-ARG: unsupported argument 'bad_arg' to option 
'fsanitize-address-destructor-kind='
+// CHECK-BAD-ARG: invalid value 'bad_arg' in 
'-fsanitize-address-destructor-kind=bad_arg'
 
 // Default is global dtor
 // RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple 
x86_64-apple-macosx10.15 \



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a159f91 - [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

[Dan Liew via cfe-commits]

Author: Dan Liew
Date: 2021-03-10T09:42:24-08:00
New Revision: a159f91c8d068cae660a200868b7fc28fcbcd3ff

URL: 
https://github.com/llvm/llvm-project/commit/a159f91c8d068cae660a200868b7fc28fcbcd3ff
DIFF: 
https://github.com/llvm/llvm-project/commit/a159f91c8d068cae660a200868b7fc28fcbcd3ff.diff

LOG: [compiler-rt] Fix stale incremental builds when using 
`LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

When building with `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON` (e.g. Swift does
this) we do an "external" build of compiler-rt where we build
compiler-rt with the just built clang.

Unfortunately building in this mode had a bug where compiler-rt would
not get rebuilt if compiler-rt sources changed. This is problematic
for incremental builds because it meant that the compiler-rt binaries
were stale.

The fix is to use the `BUILD_ALWAYS` ExternalProject_Add option which
means the build command for compiler-rt is always run.

In principle if all of the following are true:

* compiler-rt has already been built.
* there are no compiler-rt source changes.
* the compiler hasn't changed.
* ninja is being used as the generator for the compiler-rt build.

then the overhead for always running the build command for incremental
builds is negligible.

However, in practice clang gets rebuilt everytime the HEAD commit
changes (due to commit hash being embedded in the output of `--version`)
which means all of compiler-rt will be rebuilt everytime this happens.
While this is annoying it's better to do the slow but correct thing
rather than the fast but incorrect thing.

rdar://75150660

Differential Revision: https://reviews.llvm.org/D98291

Added: 


Modified: 
clang/runtime/CMakeLists.txt

Removed: 




diff  --git a/clang/runtime/CMakeLists.txt b/clang/runtime/CMakeLists.txt
index 61bbbf8faedd..1716c53b031e 100644
--- a/clang/runtime/CMakeLists.txt
+++ b/clang/runtime/CMakeLists.txt
@@ -95,6 +95,8 @@ if(LLVM_BUILD_EXTERNAL_COMPILER_RT AND EXISTS 
${COMPILER_RT_SRC_ROOT}/)
 USES_TERMINAL_CONFIGURE 1
 USES_TERMINAL_BUILD 1
 USES_TERMINAL_INSTALL 1
+# Always run the build command so that incremental builds are correct.
+BUILD_ALWAYS 1
 )
 
   get_ext_project_build_command(run_clean_compiler_rt clean)



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 03512b2 - [NFC][Driver] Add dummy compiler-rt sanitizer dylibs for Darwin.

[Dan Liew via cfe-commits]

Author: Dan Liew
Date: 2019-12-20T11:32:21-08:00
New Revision: 03512b267d9abd054d56c6d5fa118e78184cf015

URL: 
https://github.com/llvm/llvm-project/commit/03512b267d9abd054d56c6d5fa118e78184cf015
DIFF: 
https://github.com/llvm/llvm-project/commit/03512b267d9abd054d56c6d5fa118e78184cf015.diff

LOG: [NFC][Driver] Add dummy compiler-rt sanitizer dylibs for Darwin.

This adds dummy files to the test resource directory used by the Clang
driver tests.

rdar://problem/58118584

Added: 

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_ios_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_iossim_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_osx_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_tvos_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_tvossim_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_watchos_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_watchossim_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.lsan_ios_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.lsan_iossim_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.lsan_osx_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.lsan_tvos_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.lsan_tvossim_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.lsan_watchos_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.lsan_watchossim_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.tsan_iossim_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.tsan_osx_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.tsan_tvossim_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_ios_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_iossim_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_minimal_ios_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_minimal_iossim_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_minimal_osx_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_minimal_tvos_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_minimal_tvossim_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_minimal_watchos_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_minimal_watchossim_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_osx_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_tvos_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_tvossim_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_watchos_dynamic.dylib

clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.ubsan_watchossim_dynamic.dylib

Modified: 


Removed: 




diff  --git 
a/clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_ios_dynamic.dylib
 
b/clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_ios_dynamic.dylib
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_iossim_dynamic.dylib
 
b/clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_iossim_dynamic.dylib
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_osx_dynamic.dylib
 
b/clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_osx_dynamic.dylib
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_tvos_dynamic.dylib
 
b/clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_tvos_dynamic.dylib
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_tvossim_dynamic.dylib
 
b/clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_tvossim_dynamic.dylib
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_watchos_dynamic.dylib
 
b/clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_watchos_dynamic.dylib
new file mode 100644
index ..e69de29bb2d1

diff  --gi

Re: [PATCH] D12265: [ZORG] Add support for libc++ to SphinxDocBuilder.py

[Dan Liew via cfe-commits]

delcypher added a comment.

The rest LGTM



Comment at: buildbot/osuosl/master/config/builders.py:1166
@@ +1165,3 @@
+   'name':"libcxx-sphinx-docs",
+   'slavenames':["ericwf-buildslave2],
+   'builddir':"libcxx-sphinx-docs",

Is there a reason you're using the slave `ericwf-buildslave2` rather than 
`gribozavr4` which is used by the other documentation builders?




Comment at: buildbot/osuosl/master/config/builders.py:1341
@@ -1333,3 +1340,3 @@
  'factory' : LNTBuilder.getLNTFactory(triple='x86_64-apple-darwin10',
-  nt_flags=['--multisample=3', 
+  nt_flags=['--multisample=3',
 '--optimize-option',

This spacing change doesn't really belong in this patch


Comment at: zorg/buildbot/builders/SphinxDocsBuilder.py:13
@@ -13,1 +12,3 @@
+lld_html= False, # Build LLD HTML documentation
+libcxx_html = False # Build Libc++ HTML documentation
 ):

Minor nit, the ``#`` should probably be horizontally aligned with the others


http://reviews.llvm.org/D12265



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22463: [RFC] Moving to GitHub Proposal: NOT DECISION!

[Dan Liew via cfe-commits]

delcypher added a subscriber: delcypher.


Comment at: docs/Proposals/GitHub.rst:102
@@ +101,3 @@
+
+How will the new workflow look like
+===

s/How will/What will/


Comment at: docs/Proposals/GitHub.rst:136
@@ +135,3 @@
+
+We will need additional server hooks to avoid non-fast-forwards commits (ex.
+merges, forced pushes, etc) in order to keep the linearity of the history.

@rengolin : I know GitHub enterprise has a "protected branch" feature to 
prevent forced pushed ( 
https://github.com/blog/2051-protected-branches-and-required-status-checks ). 
You might want to speak to them to see if they can offer us that feature. I 
think there's a limited support to do a merge as a squash and rebase too ( 
https://github.com/blog/2141-squash-your-commits )


Comment at: docs/Proposals/GitHub.rst:233
@@ +232,3 @@
+
+10. Collect peoples GitHub account information, adding them to the project.
+11. Switch SVN repository to read-only and allow pushes to the GitHub 
repository.

GitHub organizations support the notion of teams which can each have different 
permissions (for example you'll want to make sure only the right people have 
admin access and give the rest write/read access). You could also make a team 
per project so that write access in one project does not give write access to 
another. I think it would be good to decide on how teams will be organized and 
state this in the document.


https://reviews.llvm.org/D22463



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)

[Dan Liew via cfe-commits]

https://github.com/delcypher approved this pull request.

Thanks for addressing my comment. I have some nits but I'll leave it to your 
discretion whether or not you fix them.

https://github.com/llvm/llvm-project/pull/93231
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)

[Dan Liew via cfe-commits]

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/93231
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)

[Dan Liew via cfe-commits]

@@ -8320,6 +8320,15 @@ static const RecordDecl 
*GetEnclosingNamedOrTopAnonRecord(const FieldDecl *FD) {
   return RD;
 }
 
+static CountAttributedType::DynamicCountPointerKind
+getCountAttrKind(bool CountInBytes, bool OrNull) {

delcypher wrote:

Nit. Should this be a class method on `CountAttributedType`?

https://github.com/llvm/llvm-project/pull/93231
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)

[Dan Liew via cfe-commits]

@@ -425,6 +425,12 @@ Attribute Changes in Clang
size_t count;
  };
 
+- The attributes ``sized_by``, ``counted_by_or_null`` and ``sized_by_or_null```
+  have been added as variants on ``counted_by``, each with slightly different 
semantics.
+  ``sized_by`` takes a byte size parameter instead of an element count, 
allowing pointees
+  with unknown size. The ``counted_by_or_null`` and ``sized_by_or_null`` 
variants are equivalent
+  to their base variants, except the pointer can be null regardless of 
count/size value.

delcypher wrote:

Nit: Maybe it's worth calling out (because it's really not obvious) that the 
only valid count for `__sized_by` when the pointer is `NULL` is `0`, 
`__sized_by_or_null` allows **any** count value when the pointer is `NULL`. We 
could also note that this isn't currently enforced.

https://github.com/llvm/llvm-project/pull/93231
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)

[Dan Liew via cfe-commits]

@@ -0,0 +1,141 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define __counted_by_or_null(f)  __attribute__((counted_by_or_null(f)))
+
+// This has been adapted from clang/test/Sema/attr-counted-by-vla.c, but with 
VLAs replaced with pointers
+
+struct bar;
+
+struct not_found {
+  int count;
+  struct bar *ptr __counted_by_or_null(bork); // expected-error {{use of 
undeclared identifier 'bork'}}
+};
+
+struct no_found_count_not_in_substruct {
+  unsigned long flags;
+  unsigned char count; // expected-note {{'count' declared here}}
+  struct A {
+int dummy;
+int * ptr __counted_by_or_null(count); // expected-error 
{{'counted_by_or_null' field 'count' isn't within the same struct as the 
annotated pointer}}

delcypher wrote:

Nit: This wording is a little odd. I would expect something like.

```
'counted_by_or_null' references field 'count' which isn't within the same 
struct as the annotated pointer
```

https://github.com/llvm/llvm-project/pull/93231
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)

[Dan Liew via cfe-commits]

@@ -0,0 +1,141 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define __counted_by_or_null(f)  __attribute__((counted_by_or_null(f)))
+
+// This has been adapted from clang/test/Sema/attr-counted-by-vla.c, but with 
VLAs replaced with pointers
+
+struct bar;
+
+struct not_found {
+  int count;
+  struct bar *ptr __counted_by_or_null(bork); // expected-error {{use of 
undeclared identifier 'bork'}}
+};
+
+struct no_found_count_not_in_substruct {
+  unsigned long flags;
+  unsigned char count; // expected-note {{'count' declared here}}
+  struct A {
+int dummy;
+int * ptr __counted_by_or_null(count); // expected-error 
{{'counted_by_or_null' field 'count' isn't within the same struct as the 
annotated pointer}}

delcypher wrote:

@hnrklssn Looks like you just modified the existing wording. I definitely think 
it could be improved but we can do that in a separate patch if you prefer.

https://github.com/llvm/llvm-project/pull/93231
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)

[Dan Liew via cfe-commits]

@@ -29,6 +30,22 @@ struct LOCKABLE Mutex {};
 
 struct Foo {
   struct Mutex *mu_;
+  int  a_value GUARDED_BY(mu_);
+
+  struct Bar {
+struct Mutex *other_mu ACQUIRED_AFTER(mu_);

delcypher wrote:

@pdherbemont Did you add the comment? I can't find it in the diff shown on 
GitHub

https://github.com/llvm/llvm-project/pull/95455
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)

[Dan Liew via cfe-commits]

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/95455
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)

[Dan Liew via cfe-commits]

https://github.com/delcypher approved this pull request.

@pdherbemont Thanks for working on this LGTM. Please confirm that the comments 
that @aaronpuchert have been added because I couldn't see them.

Once you've done that we can get this merged :) 

https://github.com/llvm/llvm-project/pull/95455
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)

[Dan Liew via cfe-commits]

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/95455
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Parser][BoundsSafety] Print attribute as macro if it's system defined (PR #107619)

[Dan Liew via cfe-commits]

@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -fsyntax-only -I%S/Inputs -Wthread-safety %s 2>&1 | 
FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -I%S/Inputs -Wthread-safety -std=c++98 %s 
2>&1 | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -I%S/Inputs -Wthread-safety -std=c++11 %s -D 
CPP11 2>&1 | FileCheck %s

delcypher wrote:

@rapidsna Looks like there might be some missing test coverage here

* Late parsing isn't enabled in this test but the PR appears to be touching 
that code path
* There's no C version of this test that enables late parsing but this PR 
appears to be touching that code path.

https://github.com/llvm/llvm-project/pull/107619
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Parser][BoundsSafety] Print attribute as macro if it's system defined (PR #107619)

[Dan Liew via cfe-commits]

@@ -5080,6 +5083,17 @@ void Parser::ParseLexedCAttribute(LateParsedAttribute 
&LA, bool EnterScope,
   ParseGNUAttributeArgs(&LA.AttrName, LA.AttrNameLoc, Attrs, nullptr, nullptr,
 SourceLocation(), ParsedAttr::Form::GNU(), nullptr);
 
+  const auto &SM = PP.getSourceManager();

delcypher wrote:

There should probably be a comment here explaining what this is trying to do.

https://github.com/llvm/llvm-project/pull/107619
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Parser][BoundsSafety] Print attribute as macro if it's system defined (PR #107619)

[Dan Liew via cfe-commits]

@@ -5080,6 +5083,17 @@ void Parser::ParseLexedCAttribute(LateParsedAttribute 
&LA, bool EnterScope,
   ParseGNUAttributeArgs(&LA.AttrName, LA.AttrNameLoc, Attrs, nullptr, nullptr,
 SourceLocation(), ParsedAttr::Form::GNU(), nullptr);
 
+  const auto &SM = PP.getSourceManager();
+  CharSourceRange ExpansionRange = SM.getExpansionRange(LA.AttrNameLoc);
+  StringRef FoundName =
+  Lexer::getSourceText(ExpansionRange, SM, PP.getLangOpts())
+  .split('(')
+  .first;
+  IdentifierInfo *MacroII = PP.getIdentifierInfo(FoundName);

delcypher wrote:

What's the failure path here when the attribute isn't a macro? We seem to be 
calling `setMacroIdentifier` unconditionally.

https://github.com/llvm/llvm-project/pull/107619
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

@@ -673,6 +673,9 @@ option(LLVM_USE_OPROFILE
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
+option(LLVM_ENABLE_EXPORTED_SYMBOLS

delcypher wrote:

We may want to rename this. The name hints that this option applies to 
everything but actually it only applies to executables and not libraries. E.g. 
`LLVM_ENABLE_TOOLS_WITH_EXPORTED_SYMBOLS`

https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

@@ -654,6 +654,11 @@ enabled sub-projects. Nearly all of these variable names 
begin with
   Generate dSYM files and strip executables and libraries (Darwin Only).
   Defaults to OFF.
 
+**LLVM_ENABLE_EXPORTED_SYMBOLS**:BOOL
+  When building executables, preserve symbol exports. Defaults to ON. 
+  You can use this option to disable exported symbols on all executable build

delcypher wrote:

```suggestion
  Setting this option to ``OFF`` removes exported symbols from all executables.
```

https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

https://github.com/delcypher requested changes to this pull request.

Generally looks good. I have some minor nits. I'd like to see 
`LLVM_ENABLE_EXPORTED_SYMBOLS` renamed to avoid ambiguity.

https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

@@ -654,6 +654,11 @@ enabled sub-projects. Nearly all of these variable names 
begin with
   Generate dSYM files and strip executables and libraries (Darwin Only).
   Defaults to OFF.
 
+**LLVM_ENABLE_EXPORTED_SYMBOLS**:BOOL
+  When building executables, preserve symbol exports. Defaults to ON. 
+  You can use this option to disable exported symbols on all executable build

delcypher wrote:

Nit: Documentation tends not to use "You" as the subject.

https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

@@ -673,6 +673,9 @@ option(LLVM_USE_OPROFILE
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
+option(LLVM_ENABLE_EXPORTED_SYMBOLS

danliew-apple wrote:

I'm fine with that. You could also do 
`LLVM_ENABLE_EXPORTED_SYMBOLS_IN_EXECUTABLES` which is ever so slightly shorter 
🤷‍♂️

https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

@@ -673,6 +673,9 @@ option(LLVM_USE_OPROFILE
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
+option(LLVM_ENABLE_EXPORTED_SYMBOLS

delcypher wrote:

> "tools" has a pretty specific meaning for LLVM so I think that will actually 
> cause more confusion (i.e. I would expect that option to apply to 
> llvm-dwarfdump, but not to clang). I'm personally fine with the current 
> concise name.

I'm not exactly sure what that "specific meaning" is but I'm happy to go with 
something else. I think using the name "EXECUTABLE" as @cyndyishida is 
suggesting is consistent with the option being used in `add_llvm_executable()`.

https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

https://github.com/delcypher deleted 
https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

@@ -673,6 +673,9 @@ option(LLVM_USE_OPROFILE
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
+option(LLVM_ENABLE_EXPORTED_SYMBOLS

delcypher wrote:

@JDevlieghere 
> "tools" has a pretty specific meaning for LLVM so I think that will actually 
> cause more confusion (i.e. I would expect that option to apply to 
> llvm-dwarfdump, but not to clang). I'm personally fine with the current 
> concise name.

I'm not exactly sure what that "specific meaning" is but I'm happy to go with 
something else. I think using the name "EXECUTABLE" as @cyndyishida is 
suggesting is consistent with the option being used in the 
`add_llvm_executable()` macro.

https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

@@ -654,6 +654,11 @@ enabled sub-projects. Nearly all of these variable names 
begin with
   Generate dSYM files and strip executables and libraries (Darwin Only).
   Defaults to OFF.
 
+**LLVM_ENABLE_EXPORTED_SYMBOLS**:BOOL
+  When building executables, preserve symbol exports. Defaults to ON. 
+  You can use this option to disable exported symbols on all executable build

delcypher wrote:

Well if there's precedent I guess you can go either way. I'm personally not a 
fan but you should go with whatever you prefer.

https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

@@ -1029,6 +1038,11 @@ macro(add_llvm_executable name)
 add_llvm_symbol_exports( ${name} ${LLVM_EXPORTED_SYMBOL_FILE} )
   endif(LLVM_EXPORTED_SYMBOL_FILE)
 
+  if (NOT LLVM_ENABLE_EXPORTED_SYMBOLS_IN_EXECUTABLES AND 
LLVM_LINKER_SUPPORTS_NO_EXPORTED_SYMBOLS)

delcypher wrote:

Currently we silently ignore `LLVM_ENABLE_EXPORTED_SYMBOLS_IN_EXECUTABLES` if 
the linker doesn't support the flag. It would probably better to produce a 
fatal error to avoid an invalid configuration being used.

```cmake
if (NOT LLVM_ENABLE_EXPORTED_SYMBOLS_IN_EXECUTABLES)
  if (LLVM_LINKER_SUPPORTS_NO_EXPORTED_SYMBOLS)
  set_property(TARGET ${name} APPEND_STRING PROPERTY
  LINK_FLAGS " -Wl,-no_exported_symbols")
  else()
message(FATAL_ERROR "LLVM_ENABLE_EXPORTED_SYMBOLS_IN_EXECUTABLES cannot be 
disabled when linker does not support  \" -Wl,-no_exported_symbols\""
  endif()
endif()
```

https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

@@ -258,15 +258,24 @@ if (NOT DEFINED LLVM_LINKER_DETECTED AND NOT WIN32)
 endif()
   endif()
 
-  # Apple's linker complains about duplicate libraries, which CMake likes to do
-  # to support ELF platforms. To silence that warning, we can use
-  # -no_warn_duplicate_libraries, but only in versions of the linker that
-  # support that flag.
-  if(NOT LLVM_USE_LINKER AND ${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
+  if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
 include(CheckLinkerFlag)
-check_linker_flag(C "-Wl,-no_warn_duplicate_libraries" 
LLVM_LINKER_SUPPORTS_NO_WARN_DUPLICATE_LIBRARIES)
-  else()
-set(LLVM_LINKER_SUPPORTS_NO_WARN_DUPLICATE_LIBRARIES OFF CACHE INTERNAL "")
+# Linkers that support Darwin allow a setting to internalize all symbol 
exports, 
+# aiding in reducing binary size and often is applicable for executables.
+check_linker_flag(C "-Wl,-no_exported_symbols" 
LLVM_LINKER_SUPPORTS_NO_EXPORTED_SYMBOLS)
+
+if (NOT LLVM_USE_LINKER) 
+  # Apple's linker complains about duplicate libraries, which CMake likes 
to do
+  # to support ELF platforms. To silence that warning, we can use
+  # -no_warn_duplicate_libraries, but only in versions of the linker that
+  # support that flag.
+  check_linker_flag(C "-Wl,-no_warn_duplicate_libraries" 
LLVM_LINKER_SUPPORTS_NO_WARN_DUPLICATE_LIBRARIES)
+else()
+  set(LLVM_LINKER_SUPPORTS_NO_WARN_DUPLICATE_LIBRARIES OFF CACHE INTERNAL 
"")

delcypher wrote:

Nit: This won't do anything if 
`LLVM_LINKER_SUPPORTS_NO_WARN_DUPLICATE_LIBRARIES` is already set in the cache 
(e.g. from a previous configure). You can use `FORCE` to always set the value.

https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

@@ -258,15 +258,24 @@ if (NOT DEFINED LLVM_LINKER_DETECTED AND NOT WIN32)
 endif()
   endif()
 
-  # Apple's linker complains about duplicate libraries, which CMake likes to do
-  # to support ELF platforms. To silence that warning, we can use
-  # -no_warn_duplicate_libraries, but only in versions of the linker that
-  # support that flag.
-  if(NOT LLVM_USE_LINKER AND ${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
+  if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
 include(CheckLinkerFlag)
-check_linker_flag(C "-Wl,-no_warn_duplicate_libraries" 
LLVM_LINKER_SUPPORTS_NO_WARN_DUPLICATE_LIBRARIES)
-  else()
-set(LLVM_LINKER_SUPPORTS_NO_WARN_DUPLICATE_LIBRARIES OFF CACHE INTERNAL "")
+# Linkers that support Darwin allow a setting to internalize all symbol 
exports, 
+# aiding in reducing binary size and often is applicable for executables.
+check_linker_flag(C "-Wl,-no_exported_symbols" 
LLVM_LINKER_SUPPORTS_NO_EXPORTED_SYMBOLS)
+
+if (NOT LLVM_USE_LINKER) 
+  # Apple's linker complains about duplicate libraries, which CMake likes 
to do
+  # to support ELF platforms. To silence that warning, we can use
+  # -no_warn_duplicate_libraries, but only in versions of the linker that
+  # support that flag.
+  check_linker_flag(C "-Wl,-no_warn_duplicate_libraries" 
LLVM_LINKER_SUPPORTS_NO_WARN_DUPLICATE_LIBRARIES)
+else()
+  set(LLVM_LINKER_SUPPORTS_NO_WARN_DUPLICATE_LIBRARIES OFF CACHE INTERNAL 
"")
+endif()
+  
+  else() 
+set(LLVM_LINKER_SUPPORTS_NO_EXPORTED_SYMBOLS OFF CACHE INTERNAL "")

delcypher wrote:

Same here. Consider using `FORCE`

https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

[Dan Liew via cfe-commits]

https://github.com/delcypher approved this pull request.

LGTM. Other than the nit about not using `FORCE` to set the CMake cache 
variable.

https://github.com/llvm/llvm-project/pull/87684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits