[clang] 2df06e4 - [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory (#90886)

2024-05-14 Thread via cfe-commits

Author: Michael Klemm
Date: 2024-05-14T09:01:51+02:00
New Revision: 2df06e42d733a1f7a1cdf715894921a5bbbc2956

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

LOG: [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's 
resource directory (#90886)

This should be a NFC change for all drivers, but Flang.

Added: 
flang/test/Driver/print-resource-dir.F90

Modified: 
clang/include/clang/Driver/Driver.h
clang/include/clang/Driver/Options.td
clang/lib/Driver/Driver.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Driver.h 
b/clang/include/clang/Driver/Driver.h
index cc1538372d5f8..084c3ffe69ae8 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -747,6 +747,9 @@ class Driver {
   /// option.
   void setDriverMode(StringRef DriverModeValue);
 
+  /// Set the resource directory, depending on which driver is being used.
+  void setResourceDirectory();
+
   /// Parse the \p Args list for LTO options and record the type of LTO
   /// compilation based on which -f(no-)?lto(=.*)? option occurs last.
   void setLTOMode(const llvm::opt::ArgList &Args);

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index c9d8a1f50fecf..c54eb543d6580 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5491,7 +5491,10 @@ def print_prog_name_EQ : Joined<["-", "--"], 
"print-prog-name=">,
   Visibility<[ClangOption, CLOption]>;
 def print_resource_dir : Flag<["-", "--"], "print-resource-dir">,
   HelpText<"Print the resource directory pathname">,
-  Visibility<[ClangOption, CLOption]>;
+  HelpTextForVariants<[FlangOption],
+  "Print the resource directory pathname that contains lib 
and "
+  "include directories with the runtime libraries and 
MODULE files.">,
+  Visibility<[ClangOption, CLOption, FlangOption]>;
 def print_search_dirs : Flag<["-", "--"], "print-search-dirs">,
   HelpText<"Print the paths used for finding libraries and programs">,
   Visibility<[ClangOption, CLOption]>;

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 7b36d8e5084cf..2868b4f2b02e9 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -229,9 +229,6 @@ Driver::Driver(StringRef ClangExecutable, StringRef 
TargetTriple,
 UserConfigDir = static_cast(P);
   }
 #endif
-
-  // Compute the path to the resource directory.
-  ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
 }
 
 void Driver::setDriverMode(StringRef Value) {
@@ -250,6 +247,24 @@ void Driver::setDriverMode(StringRef Value) {
 Diag(diag::err_drv_unsupported_option_argument) << OptName << Value;
 }
 
+void Driver::setResourceDirectory() {
+  // Compute the path to the resource directory, depending on the driver mode.
+  switch (Mode) {
+  case GCCMode:
+  case GXXMode:
+  case CPPMode:
+  case CLMode:
+  case DXCMode:
+ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
+break;
+  case FlangMode:
+SmallString<64> customResourcePathRelativeToDriver{".."};
+ResourceDir =
+GetResourcesPath(ClangExecutable, customResourcePathRelativeToDriver);
+break;
+  }
+}
+
 InputArgList Driver::ParseArgStrings(ArrayRef ArgStrings,
  bool UseDriverMode, bool &ContainsError) {
   llvm::PrettyStackTraceString CrashInfo("Command line argument parsing");
@@ -1202,6 +1217,7 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) {
   if (!DriverMode.empty())
 setDriverMode(DriverMode);
 
+  setResourceDirectory();
   // FIXME: What are we going to do with -V and -b?
 
   // Arguments specified in command line.

diff  --git a/flang/test/Driver/print-resource-dir.F90 
b/flang/test/Driver/print-resource-dir.F90
new file mode 100644
index 0..8fd35f1800df2
--- /dev/null
+++ b/flang/test/Driver/print-resource-dir.F90
@@ -0,0 +1,4 @@
+! DEFINE: %{resource_dir} = %S/Inputs/resource_dir
+! RUN: %flang -print-resource-dir -resource-dir=%{resource_dir}.. \
+! RUN:  | FileCheck -check-prefix=PRINT-RESOURCE-DIR -DFILE=%{resource_dir} %s
+! PRINT-RESOURCE-DIR: [[FILE]]



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


[clang] [flang] [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory (PR #90886)

2024-05-14 Thread Michael Klemm via cfe-commits

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


[clang] [flang] [Flang][Driver] Add -print-resource-dir command line flag to emit Flang's resource directory (PR #90886)

2024-05-14 Thread Michael Klemm via cfe-commits

mjklemm wrote:

@tblah @banach-space Thanks for the reviews! 

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


[clang] [llvm] [AMDGPU] Add amdgpu-as MMRA for fences (PR #78572)

2024-05-14 Thread Sameer Sahasrabuddhe via cfe-commits

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


[clang] [llvm] [AMDGPU] Add amdgpu-as MMRA for fences (PR #78572)

2024-05-14 Thread Sameer Sahasrabuddhe via cfe-commits


@@ -678,6 +680,50 @@ class SIMemoryLegalizer final : public MachineFunctionPass 
{
   bool runOnMachineFunction(MachineFunction &MF) override;
 };
 
+static const StringMap ASNames = {{
+{"global", SIAtomicAddrSpace::GLOBAL},
+{"local", SIAtomicAddrSpace::LDS},
+{"image", SIAtomicAddrSpace::SCRATCH},
+}};
+
+void diagnoseUnknownMMRAASName(const MachineInstr &MI, StringRef AS) {
+  const MachineFunction *MF = MI.getMF();
+  const Function &Fn = MF->getFunction();
+  std::string Str;
+  raw_string_ostream OS(Str);
+  OS << "unknown address space '" << AS << "'; expected one of ";
+  ListSeparator LS;
+  for (const auto &[Name, Val] : ASNames)
+OS << LS << '\'' << Name << '\'';
+  DiagnosticInfoUnsupported BadTag(Fn, Str, MI.getDebugLoc(), DS_Warning);
+  Fn.getContext().diagnose(BadTag);
+}
+
+/// Reads \p MI's MMRAs to parse the "amdgpu-as" MMRA.
+/// If this tag isn't present, or if it has no meaningful values, returns \p
+/// Default. Otherwise returns all the address spaces concerned by the MMRA.
+static SIAtomicAddrSpace getFenceAddrSpaceMMRA(const MachineInstr &MI,
+   SIAtomicAddrSpace Default) {
+  static constexpr StringLiteral FenceASPrefix = "amdgpu-as";
+
+  auto MMRA = MMRAMetadata(MI.getMMRAMetadata());
+  if (!MMRA)
+return Default;
+
+  SIAtomicAddrSpace Result = SIAtomicAddrSpace::NONE;
+  for (const auto &[Prefix, Suffix] : MMRA) {
+if (Prefix != FenceASPrefix)
+  continue;
+
+if (auto It = ASNames.find(Suffix); It != ASNames.end())

ssahasra wrote:

Wow, I have never considered using a semicolon like this before. But it makes 
so much sense! :)

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


[clang] [llvm] [AMDGPU] Add amdgpu-as MMRA for fences (PR #78572)

2024-05-14 Thread Sameer Sahasrabuddhe via cfe-commits

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

Looks good to me. But I have no opinion about that discussion with whether 
"image" should be available for explicit use!

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


[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

2024-05-14 Thread Fangrui Song via cfe-commits


@@ -2900,7 +2900,7 @@ defm clangir : BoolFOption<"clangir",
   PosFlag,
   NegFlag LLVM 
pipeline to compile">,
   BothFlags<[], [ClangOption, CC1Option], "">>;
-def emit_cir : Flag<["-"], "emit-cir">, Visibility<[CC1Option]>,
+def emit_cir : Flag<["-"], "emit-cir">, Visibility<[ClangOption, CC1Option]>,

MaskRay wrote:

> Notably, -emit-llvm-bc does the wrong thing in all cases. clang -Xclang 
> -emit-llvm-bc hello.c does the wrong thing and emits an executable. clang 
> -Xclang -emit-llvm -S hello.c also does the wrong thing and emits a file 
> named hello.s that is actually llvm bitcode.

The description of #91140 might be useful.

You need `-S` or `-c` to disable linking. Then `-c -Xclang -emit-cir` can be 
used to override the assumed action passed by clangDriver to cc1.

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


[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

2024-05-14 Thread Fangrui Song via cfe-commits

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


[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

2024-05-14 Thread Fangrui Song via cfe-commits

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


[clang] [Clang] Add attribute for consteval builtins; Declare constexpr builtins as constexpr in C++ (PR #91894)

2024-05-14 Thread via cfe-commits


@@ -14,13 +14,18 @@ void __builtin_va_copy(double d);
 // expected-error@+2 {{cannot redeclare builtin function '__builtin_va_end'}}
 // expected-note@+1 {{'__builtin_va_end' is a builtin with type}}
 void __builtin_va_end(__builtin_va_list);
-// RUN: %clang_cc1 %s -fsyntax-only -verify 
-// RUN: %clang_cc1 %s -fsyntax-only -verify -x c
 
 void __va_start(__builtin_va_list*, ...);
 
+  void *__builtin_assume_aligned(const void *, size_t, ...);
 #ifdef __cplusplus
-void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
-#else
-void *__builtin_assume_aligned(const void *, size_t, ...);
+constexpr void *__builtin_assume_aligned(const void *, size_t, ...);
+  void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
+constexpr void *__builtin_assume_aligned(const void *, size_t, ...) noexcept;
+  void *__builtin_assume_aligned(const void *, size_t, ...) throw();
+constexpr void *__builtin_assume_aligned(const void *, size_t, ...) throw();
+

cor3ntin wrote:

@erichkeane 

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


[clang] [Clang][CWG1815] Support lifetime extension of temporary created by aggregate initialization using a default member initializer (PR #87933)

2024-05-14 Thread Vlad Serebrennikov via cfe-commits

Endilll wrote:

@ZequanWu thank you for your help, but next time you should disable renaming 
passes as specified in 
https://github.com/llvm/llvm-project/pull/89807#issuecomment-2102760018. 
C-Reduce output can be typically reduced further manually, but it's a pain 
without names.

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


[clang] [clang][analyzer] Add checker 'security.SetgidSetuidOrder' (PR #91445)

2024-05-14 Thread Balázs Kéri via cfe-commits

https://github.com/balazske updated 
https://github.com/llvm/llvm-project/pull/91445

From d839faf7a30851a172d812137b30635c741870f0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Wed, 8 May 2024 10:10:24 +0200
Subject: [PATCH 1/6] [clang][analyzer] Add checker
 'Security.SetgidSetuidOrder'.

---
 clang/docs/analyzer/checkers.rst  |  28 +++
 .../clang/StaticAnalyzer/Checkers/Checkers.td |   5 +
 .../StaticAnalyzer/Checkers/CMakeLists.txt|   1 +
 .../Checkers/SetgidSetuidOrderChecker.cpp | 196 ++
 .../system-header-simulator-setgid-setuid.h   |  15 ++
 clang/test/Analysis/setgid-setuid-order.c | 170 +++
 6 files changed, 415 insertions(+)
 create mode 100644 
clang/lib/StaticAnalyzer/Checkers/SetgidSetuidOrderChecker.cpp
 create mode 100644 
clang/test/Analysis/Inputs/system-header-simulator-setgid-setuid.h
 create mode 100644 clang/test/Analysis/setgid-setuid-order.c

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 0d87df36ced0e..d0c0c7a535c62 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1179,6 +1179,34 @@ security.insecureAPI.DeprecatedOrUnsafeBufferHandling (C)
strncpy(buf, "a", 1); // warn
  }
 
+security.SetgidSetuidOrder (C)
+""
+When dropping user-level and group-level privileges in a program by using
+``setuid`` and ``setgid`` calls, it is important to reset the group-level
+privileges (with ``setgid``) first. Function ``setgid`` will likely fail if
+the superuser privileges are already dropped.
+
+The checker checks for sequences of ``setuid(getuid())`` and
+``setgid(getgid())`` calls (in this order). If such a sequence is found and
+there is no other privilege-changing function call (``seteuid``, ``setreuid``,
+``setresuid`` and the GID versions of these) in between, a warning is
+generated. The checker finds only exactly ``setuid(getuid())`` calls (and the
+GID versions), not for example if the result of ``getuid()`` is stored in a
+variable.
+
+This check corresponds to SEI CERT Rule `POS36-C 
`_.
+
+.. code-block:: c
+
+ void test1() {
+   if (setuid(getuid()) == -1) {
+ /* handle error condition */
+   }
+   if (setgid(getgid()) == -1) { // warn
+ /* handle error condition */
+   }
+ }
+
 .. _unix-checkers:
 
 unix
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 520286b57c9fd..cc954828901af 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1011,6 +1011,11 @@ def FloatLoopCounter : Checker<"FloatLoopCounter">,
   Dependencies<[SecuritySyntaxChecker]>,
   Documentation;
 
+def SetgidSetuidOrderChecker : Checker<"SetgidSetuidOrder">,
+  HelpText<"Warn on possible reversed order of 'setgid(getgid()))' and 
'setuid(getuid())' (CERT: "
+   "POS36-C)">,
+  Documentation;
+
 } // end "security"
 
 let ParentPackage = ENV in {
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt 
b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 4443ffd092938..45d3788f105dc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -103,6 +103,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   ReturnUndefChecker.cpp
   ReturnValueChecker.cpp
   RunLoopAutoreleaseLeakChecker.cpp
+  SetgidSetuidOrderChecker.cpp
   SimpleStreamChecker.cpp
   SmartPtrChecker.cpp
   SmartPtrModeling.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/SetgidSetuidOrderChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/SetgidSetuidOrderChecker.cpp
new file mode 100644
index 0..11cc748cb40b1
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/SetgidSetuidOrderChecker.cpp
@@ -0,0 +1,196 @@
+//===-- ChrootChecker.cpp - chroot usage checks 
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+//  This file defines chroot checker, which checks improper use of chroot.
+//
+//===--===//
+
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include 

[clang] fcd020d - [Clang][Sema] Fix malformed AST for anonymous class access in template. (#90842)

2024-05-14 Thread via cfe-commits

Author: martinboehme
Date: 2024-05-14T09:45:54+02:00
New Revision: fcd020d561f28a2b33b6cc12a5a0164a6d5e4172

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

LOG: [Clang][Sema] Fix malformed AST for anonymous class access in template. 
(#90842)

# Observed erroneous behavior

Prior to this change, a `MemberExpr` that accesses an anonymous class
might have a prvalue as its base (even though C++ mandates that the base
of a `MemberExpr` must be a glvalue), if the code containing the
`MemberExpr` was in a template.

Here's an example on [godbolt](https://godbolt.org/z/Gz1Mer9oz) (that is
essentially identical to the new test this patch adds).

This example sets up a struct containing an anonymous struct:

```cxx
struct S {
  struct {
int i;
  };
};
```

It then accesses the member `i` using the expression `S().i`.

When we do this in a non-template function, we get the following AST:

```
`-ExprWithCleanups  'int'
  `-ImplicitCastExpr  'int' 
`-MemberExpr  'int' xvalue .i 0xbdcb3c0
  `-MemberExpr  'S::(anonymous struct at line:2:3)' xvalue 
.S::(anonymous struct at line:2:3) 0xbdcb488
`-MaterializeTemporaryExpr  'S' xvalue
  `-CXXTemporaryObjectExpr  'S' 'void () noexcept' 
zeroing
```

As expected, the AST contains a `MaterializeTemporarExpr` to materialize
the prvalue `S()` before accessing its members.

When we perform this access in a function template (that doesn't
actually even use its template parameter), the AST for the template
itself looks the same as above. However, the AST for an instantiation of
the template looks different:

```
`-ExprWithCleanups  'int'
  `-ImplicitCastExpr  'int' 
`-MemberExpr  'int' xvalue .i 0xbdcb3c0
  `-MaterializeTemporaryExpr  'S::(anonymous struct at 
line:2:3)' xvalue
`-MemberExpr  'S::(anonymous struct at line:2:3)' 
.S::(anonymous struct at line:2:3) 0xbdcb488
  `-CXXTemporaryObjectExpr  'S' 'void () noexcept' 
zeroing
```

Note how the inner `MemberExpr` (the one accessing the anonymous struct)
acts on a prvalue.

Interestingly, this does not appear to cause any problems for CodeGen,
probably because CodeGen is set up to deal with `MemberExpr`s on rvalues
in C. However, it does cause issues in the dataflow framework, which
only supports C++ today and expects the base of a `MemberExpr` to be a
glvalue.

Beyond the issues with the dataflow framework, I think this issue should
be fixed because it goes contrary to what the C++ standard mandates, and
the AST produced for the non-template case indicates that we want to
follow the C++ rules here.

# Reasons for erroneous behavior

Here's why we're getting this malformed AST.

First of all, `TreeTransform` [strips any
`MaterializeTemporaryExpr`s](https://github.com/llvm/llvm-project/blob/cd132dcbeb0fc79fd657bd5e0a8e9244c3fb5da6/clang/lib/Sema/TreeTransform.h#L14853)
from the AST.

It is therefore up to
[`TreeTransform::RebuildMemberExpr()`](https://github.com/llvm/llvm-project/blob/cd132dcbeb0fc79fd657bd5e0a8e9244c3fb5da6/clang/lib/Sema/TreeTransform.h#L2853)
to recreate a `MaterializeTemporaryExpr` if needed. In the [general
case](https://github.com/llvm/llvm-project/blob/cd132dcbeb0fc79fd657bd5e0a8e9244c3fb5da6/clang/lib/Sema/TreeTransform.h#L2915),
it does this: It calls `Sema::BuildMemberReferenceExpr()`, which ensures
that the base is a glvalue by [materializing a
temporary](https://github.com/llvm/llvm-project/blob/cd132dcbeb0fc79fd657bd5e0a8e9244c3fb5da6/clang/lib/Sema/SemaExprMember.cpp#L1016)
if needed. However, when `TreeTransform::RebuildMemberExpr()` encounters
an anonymous class, it [calls
`Sema::BuildFieldReferenceExpr()`](https://github.com/llvm/llvm-project/blob/cd132dcbeb0fc79fd657bd5e0a8e9244c3fb5da6/clang/lib/Sema/TreeTransform.h#L2880),
which, unlike `Sema::BuildMemberReferenceExpr()`, does not make sure
that the base is a glvalue.

# Proposed fix

I considered several possible ways to fix this issue:

- Add logic to `Sema::BuildFieldReferenceExpr()` that materializes a
temporary if needed. This appears to work, but it feels like the fix is
in the wrong place:
- AFAIU, other callers of `Sema::BuildFieldReferenceExpr()` don't need
this logic.
- The issue is caused by `TreeTransform` removing the
`MaterializeTemporaryExpr`, so it seems the fix should also be in
`TreeTransform`

- Materialize the temporary directly in
`TreeTransform::RebuildMemberExpr()` if needed (within the case that
deals with anonymous classes).

This would work, too, but it would duplicate logic that already exists
in `Sema::BuildMemberReferenceExpr()` (which we leverage for the general
case).

- Use `Sema::BuildMemberReferenceExpr()` instead of
`Sema::BuildFieldReferenceExpr()` for the anonymous class case, so that
it also uses the existing logic for materializing the temporary.

This is the option I've decided to go with here. There's a slight
w

[clang] [Clang][Sema] Fix malformed AST for anonymous class access in template. (PR #90842)

2024-05-14 Thread via cfe-commits

martinboehme wrote:

CI failure (DataFlowSanitizer-x86_64 :: release_shadow_space.c) looks unrelated.

Will merge now. @shafik If you have any remaining comments, happy to address 
those in a followup PR.

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


[clang] [Clang][Sema] Fix malformed AST for anonymous class access in template. (PR #90842)

2024-05-14 Thread via cfe-commits

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


[clang] [libclc] [llvm] [openmp] [Clang] `__attribute__((assume))` refactor (PR #84934)

2024-05-14 Thread Romaric Jodin via cfe-commits

rjodinchr wrote:

This is the first time I'm trying to add an attribute, and I think I am missing 
something.
I am adding this part in `Attr.td`:
```
def ClspvLibclcBuiltin: DeclOrStmtAttr {
  let Spellings = [Clang<"clspv_libclc_builtin">];
  let Documentation = [Undocumented];
  let SimpleHandler = 1;
}
```
It allows the source to be parsed, but then I don't see the attribute in the 
LLVM IR generated for libclc.

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


[clang] [flang] [flang] Add nsw flag to do-variable increment with a new option (PR #91579)

2024-05-14 Thread Yusuke MINATO via cfe-commits

https://github.com/yus3710-fj updated 
https://github.com/llvm/llvm-project/pull/91579

>From f51cfbe1e50c7a1aa902c684f12a20d0fac39c21 Mon Sep 17 00:00:00 2001
From: Yusuke MINATO 
Date: Wed, 24 Apr 2024 14:42:21 +0900
Subject: [PATCH 1/3] [flang] Add nsw flag to do-variable increment with a new
 option

This patch adds nsw flag to the increment of do-variables when
a new option is enabled.
NOTE 11.10 in the Fortran 2018 standard says they never overflow.

See also the discussion in 74709 and
discourse post.
---
 clang/include/clang/Driver/Options.td |   4 +
 clang/lib/Driver/ToolChains/Flang.cpp |   1 +
 flang/include/flang/Lower/LoweringOptions.def |   4 +
 .../flang/Optimizer/Transforms/Passes.h   |   4 +-
 .../flang/Optimizer/Transforms/Passes.td  |   5 +-
 flang/include/flang/Tools/CLOptions.inc   |  13 +-
 flang/include/flang/Tools/CrossToolHelpers.h  |   1 +
 flang/lib/Frontend/CompilerInvocation.cpp |   6 +
 flang/lib/Frontend/FrontendActions.cpp|   3 +
 flang/lib/Lower/Bridge.cpp|  12 +-
 flang/lib/Lower/IO.cpp|   9 +-
 .../Transforms/ControlFlowConverter.cpp   |  44 +++-
 flang/test/Driver/frontend-forwarding.f90 |   2 +
 flang/test/Fir/loop01.fir | 211 ++
 flang/test/Lower/array-substring.f90  |  40 
 flang/test/Lower/do_loop.f90  |  42 
 flang/test/Lower/do_loop_unstructured.f90 | 189 +++-
 flang/test/Lower/infinite_loop.f90|  34 +++
 flang/test/Lower/io-implied-do-fixes.f90  |  51 -
 flang/tools/bbc/bbc.cpp   |   7 +
 20 files changed, 659 insertions(+), 23 deletions(-)

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 1429528975853..0d032076f7163 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6539,6 +6539,10 @@ def flang_deprecated_no_hlfir : Flag<["-"], 
"flang-deprecated-no-hlfir">,
   Flags<[HelpHidden]>, Visibility<[FlangOption, FC1Option]>,
   HelpText<"Do not use HLFIR lowering (deprecated)">;
 
+def flang_experimental_integer_overflow : Flag<["-"], 
"flang-experimental-integer-overflow">,
+  Flags<[HelpHidden]>, Visibility<[FlangOption, FC1Option]>,
+  HelpText<"Add nsw flag to internal operations such as do-variable increment 
(experimental)">;
+
 
//===--===//
 // FLangOption + CoreOption + NoXarchOption
 
//===--===//
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp 
b/clang/lib/Driver/ToolChains/Flang.cpp
index 436a9c418a5f9..0ae489a823078 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -139,6 +139,7 @@ void Flang::addCodegenOptions(const ArgList &Args,
 
   Args.addAllArgs(CmdArgs, {options::OPT_flang_experimental_hlfir,
 options::OPT_flang_deprecated_no_hlfir,
+options::OPT_flang_experimental_integer_overflow,
 options::OPT_fno_ppc_native_vec_elem_order,
 options::OPT_fppc_native_vec_elem_order});
 }
diff --git a/flang/include/flang/Lower/LoweringOptions.def 
b/flang/include/flang/Lower/LoweringOptions.def
index be080a4d29d73..839d2b46249b0 100644
--- a/flang/include/flang/Lower/LoweringOptions.def
+++ b/flang/include/flang/Lower/LoweringOptions.def
@@ -34,5 +34,9 @@ ENUM_LOWERINGOPT(NoPPCNativeVecElemOrder, unsigned, 1, 0)
 /// On by default.
 ENUM_LOWERINGOPT(Underscoring, unsigned, 1, 1)
 
+/// If true, add nsw flags to arithmetic operations for integer.
+/// Off by default.
+ENUM_LOWERINGOPT(NoSignedWrap, unsigned, 1, 0)
+
 #undef LOWERINGOPT
 #undef ENUM_LOWERINGOPT
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.h 
b/flang/include/flang/Optimizer/Transforms/Passes.h
index 470ed8a125ac4..496201a04e29c 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.h
+++ b/flang/include/flang/Optimizer/Transforms/Passes.h
@@ -54,6 +54,7 @@ namespace fir {
 std::unique_ptr createAffineDemotionPass();
 std::unique_ptr
 createArrayValueCopyPass(fir::ArrayValueCopyOptions options = {});
+std::unique_ptr createCFGConversionPassWithNSW();
 std::unique_ptr createExternalNameConversionPass();
 std::unique_ptr
 createExternalNameConversionPass(bool appendUnderscore);
@@ -89,7 +90,8 @@ createFunctionAttrPass(FunctionAttrTypes &functionAttr, bool 
noInfsFPMath,
bool noSignedZerosFPMath, bool unsafeFPMath);
 
 void populateCfgConversionRewrites(mlir::RewritePatternSet &patterns,
-   bool forceLoopToExecuteOnce = false);
+   bool forceLoopToExecuteOnce = false,
+   bool setNSW = false);
 
 // declarative passes
 #define GEN_PASS_REGISTRATION
diff --git a/

[clang] [flang] [flang] Add nsw flag to do-variable increment with a new option (PR #91579)

2024-05-14 Thread Yusuke MINATO via cfe-commits


@@ -34,5 +34,9 @@ ENUM_LOWERINGOPT(NoPPCNativeVecElemOrder, unsigned, 1, 0)
 /// On by default.
 ENUM_LOWERINGOPT(Underscoring, unsigned, 1, 1)
 
+/// If true, add nsw flags to arithmetic operations for integer.
+/// Off by default.
+ENUM_LOWERINGOPT(NoSignedWrap, unsigned, 1, 0)

yus3710-fj wrote:

Thank you for the review!
I changed the name of the lowering option `NoSignedWrap` to `NSWOnLoopVarInc`.

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


[clang] [libclc] [llvm] [openmp] [Clang] `__attribute__((assume))` refactor (PR #84934)

2024-05-14 Thread Sven van Haastregt via cfe-commits

svenvh wrote:

> It allows the source to be parsed, but then I don't see the attribute in the 
> LLVM IR generated for libclc.

You will need to also convert the attribute into an LLVM IR construct (e.g. 
metadata) in Clang CodeGen.  See `CodeGenFunction::EmitKernelMetadata` for 
inspiration for example.

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


[clang] [clang] Enable C++17 relaxed template template argument matching by default (PR #89807)

2024-05-14 Thread Sam McCall via cfe-commits

sam-mccall wrote:

The immediate deprecation causes a few issues:

- mechanical: we build with `-Wall -Werror -Wno-deprecated-declarations 
-Wno-deprecated-other-stuff` in part to catch driver misuse and fix it early. 
However this warning is not actionable, so now we need `-Wno-deprecated` which 
has undesirable effects and is also just changing unreasonably many things at 
once.
- mixed messages: there was a lot of confusion that this commit made the flag 
both required and deprecated. This isn't usual practice.
- scary with unclear expectations: deprecation warning means we need to migrate 
now, but we can't (not our code, authors probably won't take this seriously 
until clang-19). So our toolchain could break any day now?
- compatibility: there's no non-deprecated argv that results in the same 
behavior before and after this commit. What is a build system supposed to do if 
it can't version-lock clang? (Google actually *does* version-lock clang, this 
is only tangentially relevant to us because of clang-tidy, clangd etc. More 
relevant to others)
 
I think a good alternative would be: fix behavior + change default now in 
clang-19 cycle, deprecate `-fno-relaxed` flag in clang-20 cycle, remove 
`-fno-relaxed` and deprecate `-frelaxed` in clang-21 cycle. I believe this is 
more in line with what we've done in the past. Removing `-fno-relaxed` support 
is the real win here, and as this breaks deployed code doing that before 
clang-21 is probably unrealistic anyway.

---

(I forgot to mention earlier - a consequence of the crashes is that clang-tidy 
pipelines, clangd etc started crashing, which was disruptive to a lot of 
people. For clang per se it's not terribly severe to crash after emitting 
diagnostics, but other tools need to be able to consume real-world code and 
keep running, whether it's valid or not. In that sense this was more severe 
than a usual crash-on-invalid)

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


[clang] [lldb] [llvm] [openmp] [polly] fix(python): fix comparison to True/False (PR #91858)

2024-05-14 Thread David Spickett via cfe-commits

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


[clang] [lldb] [llvm] [openmp] [polly] fix(python): fix comparison to True/False (PR #91858)

2024-05-14 Thread David Spickett via cfe-commits

DavidSpickett wrote:

If this is split out from the other larger PR, should there be `clang/` changes 
in here?

I've copied your commit message into the PR description, because with the way 
llvm is setup, we use the PR's description as the commit message for a squashed 
version of the changes.

(maybe Github uses the commit message if there's only one and the PR 
description is blank, but I wouldn't bet on it)

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


[clang] [ClangFormat] Add DiagHandler for getStyle function (PR #91317)

2024-05-14 Thread via cfe-commits

pointhex wrote:

@mydeveloperday There is no issue, It is my request. I implemented it without 
creating an issue or suggestion. Should I?

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


[clang] [clang][ExprConst] allow single element access of vector object to be constant expression (PR #72607)

2024-05-14 Thread Vikram Hegde via cfe-commits

vikramRH wrote:

@yuanfang-chen , any plans to continue with this PR ?

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


[clang] [compiler-rt] [XRay] Add support for instrumentation of DSOs on x86_64 (PR #90959)

2024-05-14 Thread Sebastian Kreutzer via cfe-commits

sebastiankreutzer wrote:

Ping

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


[clang] [Clang] Throw error when calling atomic with pointer to zero size object (PR #91057)

2024-05-14 Thread Hendrik Hübner via cfe-commits

https://github.com/HendrikHuebner updated 
https://github.com/llvm/llvm-project/pull/91057

From 770e7c543b0651b776b2bc23353e85a7f62a695a Mon Sep 17 00:00:00 2001
From: hhuebner 
Date: Sat, 4 May 2024 13:49:38 +0200
Subject: [PATCH] [Clang] Throw error when calling atomic with pointer to zero
 size object

---
 .../clang/Basic/DiagnosticFrontendKinds.td|  3 ++
 clang/lib/Sema/SemaChecking.cpp   | 12 ++-
 clang/test/Sema/atomic-ops.c  | 32 +++
 3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td 
b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index e456ec2cac461..74d96a8cbe0dc 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -330,6 +330,9 @@ def warn_atomic_op_misaligned : Warning<
   "; the expected alignment (%0 bytes) exceeds the actual alignment (%1 
bytes)">,
   InGroup;
 
+def err_atomic_op_size_zero : Error<
+  "First argument cannot be a pointer to an object of size zero">;
+
 def warn_atomic_op_oversized : Warning<
   "large atomic operation may incur "
   "significant performance penalty"
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ecd1821651140..7db31c2b8c0b8 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -40,6 +40,7 @@
 #include "clang/Basic/AddressSpaces.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
@@ -8497,7 +8498,9 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, 
SourceRange ExprRange,
 << 0 << AdjustedNumArgs << static_cast(Args.size())
 << /*is non object*/ 0 << ExprRange;
 return ExprError();
-  } else if (Args.size() > AdjustedNumArgs) {
+  }
+ 
+  if (Args.size() > AdjustedNumArgs) {
 Diag(Args[AdjustedNumArgs]->getBeginLoc(),
  diag::err_typecheck_call_too_many_args)
 << 0 << AdjustedNumArgs << static_cast(Args.size())
@@ -8544,6 +8547,13 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, 
SourceRange ExprRange,
 }
   }
 
+  // pointer to object of size zero is not allowed
+  if (Context.getTypeInfoInChars(AtomTy).Width.isZero()) {
+Diag(ExprRange.getBegin(), diag::err_atomic_op_size_zero)
+<< Ptr->getSourceRange();
+return ExprError();
+  }
+
   // For an arithmetic operation, the implied arithmetic must be well-formed.
   if (Form == Arithmetic) {
 // GCC does not enforce these rules for GNU atomics, but we do to help 
catch
diff --git a/clang/test/Sema/atomic-ops.c b/clang/test/Sema/atomic-ops.c
index 1d36667d6cf40..97da2582312bb 100644
--- a/clang/test/Sema/atomic-ops.c
+++ b/clang/test/Sema/atomic-ops.c
@@ -639,6 +639,38 @@ void memory_checks(_Atomic(int) *Ap, int *p, int val) {
   (void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, -1); 
// expected-warning {{memory order argument to atomic operation is invalid}}
 }
 
+struct Z {
+  char z[];
+};
+
+void zeroSizeArgError(struct Z *a, struct Z *b, struct Z *c) {
+  __atomic_exchange(b, b, c, memory_order_relaxed); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_exchange(b, b, c, memory_order_acq_rel); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_exchange(b, b, c, memory_order_acquire); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_exchange(b, b, c, memory_order_consume); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_exchange(b, b, c, memory_order_release); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_exchange(b, b, c, memory_order_seq_cst); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_load(a, b, memory_order_relaxed); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_load(a, b, memory_order_acq_rel); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_load(a, b, memory_order_acquire); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_load(a, b, memory_order_consume); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_load(a, b, memory_order_release); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_load(a, b, memory_order_seq_cst); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_store(a, b, memory_order_relaxed); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomi

[clang] [Driver][PS5] Set visibility option defaults (PR #92091)

2024-05-14 Thread via cfe-commits

https://github.com/bd1976bris created 
https://github.com/llvm/llvm-project/pull/92091

Adjust the PS5 driver defaults for the `-fvisibility-from-dllstorageclass`
sub-options so that only globals with dllimport/dllexport annotations
are adjusted. This allows globals without dllimport/export to retain
the visibility and pre-emptability assigned during IR-Gen. Set
`-fvisibility=hidden` on PS5 by default to compensate for no longer
overriding the visibility of definitions without dllexport. Note there
is no behaviour change for PS4 (the behaviour of overriding the
visibility for all globals is retained on PS4).

>From dcf195d902613fc111de80564bfd0d2828bb85bb Mon Sep 17 00:00:00 2001
From: Ben Dunbobbin 
Date: Mon, 13 May 2024 15:17:27 +0100
Subject: [PATCH] Use symbol visibility for export control in the compiler
 frontend for PS5

Adjust the PS5 driver defaults for the `-fvisibility-from-dllstorageclass`
sub-options so that only globals with dllimport/dllexport annotations
are adjusted. This allows globals without dllimport/export to retain
the visibility and pre-emptability assigned during IR-Gen. Set
`-fvisibility=hidden` on PS5 by default to compensate for no longer
overriding the visibility of definitions without dllexport. Note there
is no behaviour change for PS4 (the behaviour of overriding the
visibility for all globals is retained on PS4).
---
 clang/lib/Driver/ToolChains/PS4CPU.cpp| 18 -
 .../ps4-ps5-visibility-dllstorageclass.c  | 39 ---
 clang/test/Driver/ps4-visibility.cl   | 32 +++
 clang/test/Driver/ps5-visibility.cl   | 33 
 4 files changed, 107 insertions(+), 15 deletions(-)
 create mode 100644 clang/test/Driver/ps4-visibility.cl
 create mode 100644 clang/test/Driver/ps5-visibility.cl

diff --git a/clang/lib/Driver/ToolChains/PS4CPU.cpp 
b/clang/lib/Driver/ToolChains/PS4CPU.cpp
index 7bf9aa79384c5..3fd62d9793093 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ b/clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -358,6 +358,12 @@ void toolchains::PS4PS5Base::addClangTargetOptions(
 
   CC1Args.push_back("-fno-use-init-array");
 
+  // Default to `hidden` visibility for PS5.
+  if (getTriple().isPS5() &&
+  !DriverArgs.hasArg(options::OPT_fvisibility_EQ,
+ options::OPT_fvisibility_ms_compat))
+CC1Args.push_back("-fvisibility=hidden");
+
   // Default to -fvisibility-global-new-delete=source for PS5.
   if (getTriple().isPS5() &&
   !DriverArgs.hasArg(options::OPT_fvisibility_global_new_delete_EQ,
@@ -376,11 +382,15 @@ void toolchains::PS4PS5Base::addClangTargetOptions(
 else
   CC1Args.push_back("-fvisibility-dllexport=protected");
 
+// For PS4 we override the visibilty of globals definitions without
+// dllimport or  dllexport annotations.
 if (DriverArgs.hasArg(options::OPT_fvisibility_nodllstorageclass_EQ))
   DriverArgs.AddLastArg(CC1Args,
 options::OPT_fvisibility_nodllstorageclass_EQ);
-else
+else if (getTriple().isPS4())
   CC1Args.push_back("-fvisibility-nodllstorageclass=hidden");
+else
+  CC1Args.push_back("-fvisibility-nodllstorageclass=keep");
 
 if (DriverArgs.hasArg(options::OPT_fvisibility_externs_dllimport_EQ))
   DriverArgs.AddLastArg(CC1Args,
@@ -388,12 +398,16 @@ void toolchains::PS4PS5Base::addClangTargetOptions(
 else
   CC1Args.push_back("-fvisibility-externs-dllimport=default");
 
+// For PS4 we override the visibilty of external globals without
+// dllimport or  dllexport annotations.
 if (DriverArgs.hasArg(
 options::OPT_fvisibility_externs_nodllstorageclass_EQ))
   DriverArgs.AddLastArg(
   CC1Args, options::OPT_fvisibility_externs_nodllstorageclass_EQ);
-else
+else if (getTriple().isPS4())
   CC1Args.push_back("-fvisibility-externs-nodllstorageclass=default");
+else
+  CC1Args.push_back("-fvisibility-externs-nodllstorageclass=keep");
   }
 }
 
diff --git a/clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c 
b/clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c
index 430827805a8fd..71f8661679eb7 100644
--- a/clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c
+++ b/clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c
@@ -1,16 +1,19 @@
 // Check behaviour of -fvisibility-from-dllstorageclass options for PS4/PS5.
 
 // DEFINE: %{triple} =
+// DEFINE: %{prefix} =
 // DEFINE: %{run} = \
 // DEFINE: %clang -### -target %{triple} %s -Werror -o - 2>&1 | \
-// DEFINE:   FileCheck %s --check-prefix=DEFAULTS \
+// DEFINE:   FileCheck %s --check-prefixes=DEFAULTS,%{prefix} \
 // DEFINE: --implicit-check-not=-fvisibility-from-dllstorageclass \
 // DEFINE: --implicit-check-not=-fvisibility-dllexport \
 // DEFINE: --implicit-check-not=-fvisibility-nodllstorageclass \
 // DEFINE: --implicit-check-not=-fvisibility-externs-dllimport \
 // DEFINE: --implicit-check-not=-fvisibility-externs-nodllstor

[clang] [Driver][PS5] Set visibility option defaults (PR #92091)

2024-05-14 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: bd1976bris (bd1976bris)


Changes

Adjust the PS5 driver defaults for the `-fvisibility-from-dllstorageclass`
sub-options so that only globals with dllimport/dllexport annotations
are adjusted. This allows globals without dllimport/export to retain
the visibility and pre-emptability assigned during IR-Gen. Set
`-fvisibility=hidden` on PS5 by default to compensate for no longer
overriding the visibility of definitions without dllexport. Note there
is no behaviour change for PS4 (the behaviour of overriding the
visibility for all globals is retained on PS4).

---
Full diff: https://github.com/llvm/llvm-project/pull/92091.diff


4 Files Affected:

- (modified) clang/lib/Driver/ToolChains/PS4CPU.cpp (+16-2) 
- (modified) clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c (+26-13) 
- (added) clang/test/Driver/ps4-visibility.cl (+32) 
- (added) clang/test/Driver/ps5-visibility.cl (+33) 


``diff
diff --git a/clang/lib/Driver/ToolChains/PS4CPU.cpp 
b/clang/lib/Driver/ToolChains/PS4CPU.cpp
index 7bf9aa79384c5..3fd62d9793093 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ b/clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -358,6 +358,12 @@ void toolchains::PS4PS5Base::addClangTargetOptions(
 
   CC1Args.push_back("-fno-use-init-array");
 
+  // Default to `hidden` visibility for PS5.
+  if (getTriple().isPS5() &&
+  !DriverArgs.hasArg(options::OPT_fvisibility_EQ,
+ options::OPT_fvisibility_ms_compat))
+CC1Args.push_back("-fvisibility=hidden");
+
   // Default to -fvisibility-global-new-delete=source for PS5.
   if (getTriple().isPS5() &&
   !DriverArgs.hasArg(options::OPT_fvisibility_global_new_delete_EQ,
@@ -376,11 +382,15 @@ void toolchains::PS4PS5Base::addClangTargetOptions(
 else
   CC1Args.push_back("-fvisibility-dllexport=protected");
 
+// For PS4 we override the visibilty of globals definitions without
+// dllimport or  dllexport annotations.
 if (DriverArgs.hasArg(options::OPT_fvisibility_nodllstorageclass_EQ))
   DriverArgs.AddLastArg(CC1Args,
 options::OPT_fvisibility_nodllstorageclass_EQ);
-else
+else if (getTriple().isPS4())
   CC1Args.push_back("-fvisibility-nodllstorageclass=hidden");
+else
+  CC1Args.push_back("-fvisibility-nodllstorageclass=keep");
 
 if (DriverArgs.hasArg(options::OPT_fvisibility_externs_dllimport_EQ))
   DriverArgs.AddLastArg(CC1Args,
@@ -388,12 +398,16 @@ void toolchains::PS4PS5Base::addClangTargetOptions(
 else
   CC1Args.push_back("-fvisibility-externs-dllimport=default");
 
+// For PS4 we override the visibilty of external globals without
+// dllimport or  dllexport annotations.
 if (DriverArgs.hasArg(
 options::OPT_fvisibility_externs_nodllstorageclass_EQ))
   DriverArgs.AddLastArg(
   CC1Args, options::OPT_fvisibility_externs_nodllstorageclass_EQ);
-else
+else if (getTriple().isPS4())
   CC1Args.push_back("-fvisibility-externs-nodllstorageclass=default");
+else
+  CC1Args.push_back("-fvisibility-externs-nodllstorageclass=keep");
   }
 }
 
diff --git a/clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c 
b/clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c
index 430827805a8fd..71f8661679eb7 100644
--- a/clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c
+++ b/clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c
@@ -1,16 +1,19 @@
 // Check behaviour of -fvisibility-from-dllstorageclass options for PS4/PS5.
 
 // DEFINE: %{triple} =
+// DEFINE: %{prefix} =
 // DEFINE: %{run} = \
 // DEFINE: %clang -### -target %{triple} %s -Werror -o - 2>&1 | \
-// DEFINE:   FileCheck %s --check-prefix=DEFAULTS \
+// DEFINE:   FileCheck %s --check-prefixes=DEFAULTS,%{prefix} \
 // DEFINE: --implicit-check-not=-fvisibility-from-dllstorageclass \
 // DEFINE: --implicit-check-not=-fvisibility-dllexport \
 // DEFINE: --implicit-check-not=-fvisibility-nodllstorageclass \
 // DEFINE: --implicit-check-not=-fvisibility-externs-dllimport \
 // DEFINE: --implicit-check-not=-fvisibility-externs-nodllstorageclass
+// REDEFINE: %{prefix} = DEFAULTS-PS4
 // REDEFINE: %{triple} = x86_64-scei-ps4
 // RUN: %{run}
+// REDEFINE: %{prefix} = DEFAULTS-PS5
 // REDEFINE: %{triple} = x86_64-sie-ps5
 // RUN: %{run}
 //
@@ -20,25 +23,29 @@
 // REDEFINE: -fvisibility-from-dllstorageclass \
 // REDEFINE: -Werror \
 // REDEFINE: %s -o - 2>&1 | \
-// REDEFINE:   FileCheck %s --check-prefix=DEFAULTS \
+// REDEFINE:   FileCheck %s --check-prefixes=DEFAULTS,%{prefix} \
 // REDEFINE: --implicit-check-not=-fvisibility-from-dllstorageclass \
 // REDEFINE: --implicit-check-not=-fvisibility-dllexport \
 // REDEFINE: --implicit-check-not=-fvisibility-nodllstorageclass \
 // REDEFINE: --implicit-check-not=-fvisibility-externs-dllimport \
 // REDEFINE: --implicit-check-not=-fvisibility-externs-nodllstorageclass
+// REDEFINE: 

[clang] [Driver][PS5] Set visibility option defaults (PR #92091)

2024-05-14 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang-driver

Author: bd1976bris (bd1976bris)


Changes

Adjust the PS5 driver defaults for the `-fvisibility-from-dllstorageclass`
sub-options so that only globals with dllimport/dllexport annotations
are adjusted. This allows globals without dllimport/export to retain
the visibility and pre-emptability assigned during IR-Gen. Set
`-fvisibility=hidden` on PS5 by default to compensate for no longer
overriding the visibility of definitions without dllexport. Note there
is no behaviour change for PS4 (the behaviour of overriding the
visibility for all globals is retained on PS4).

---
Full diff: https://github.com/llvm/llvm-project/pull/92091.diff


4 Files Affected:

- (modified) clang/lib/Driver/ToolChains/PS4CPU.cpp (+16-2) 
- (modified) clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c (+26-13) 
- (added) clang/test/Driver/ps4-visibility.cl (+32) 
- (added) clang/test/Driver/ps5-visibility.cl (+33) 


``diff
diff --git a/clang/lib/Driver/ToolChains/PS4CPU.cpp 
b/clang/lib/Driver/ToolChains/PS4CPU.cpp
index 7bf9aa79384c5..3fd62d9793093 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ b/clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -358,6 +358,12 @@ void toolchains::PS4PS5Base::addClangTargetOptions(
 
   CC1Args.push_back("-fno-use-init-array");
 
+  // Default to `hidden` visibility for PS5.
+  if (getTriple().isPS5() &&
+  !DriverArgs.hasArg(options::OPT_fvisibility_EQ,
+ options::OPT_fvisibility_ms_compat))
+CC1Args.push_back("-fvisibility=hidden");
+
   // Default to -fvisibility-global-new-delete=source for PS5.
   if (getTriple().isPS5() &&
   !DriverArgs.hasArg(options::OPT_fvisibility_global_new_delete_EQ,
@@ -376,11 +382,15 @@ void toolchains::PS4PS5Base::addClangTargetOptions(
 else
   CC1Args.push_back("-fvisibility-dllexport=protected");
 
+// For PS4 we override the visibilty of globals definitions without
+// dllimport or  dllexport annotations.
 if (DriverArgs.hasArg(options::OPT_fvisibility_nodllstorageclass_EQ))
   DriverArgs.AddLastArg(CC1Args,
 options::OPT_fvisibility_nodllstorageclass_EQ);
-else
+else if (getTriple().isPS4())
   CC1Args.push_back("-fvisibility-nodllstorageclass=hidden");
+else
+  CC1Args.push_back("-fvisibility-nodllstorageclass=keep");
 
 if (DriverArgs.hasArg(options::OPT_fvisibility_externs_dllimport_EQ))
   DriverArgs.AddLastArg(CC1Args,
@@ -388,12 +398,16 @@ void toolchains::PS4PS5Base::addClangTargetOptions(
 else
   CC1Args.push_back("-fvisibility-externs-dllimport=default");
 
+// For PS4 we override the visibilty of external globals without
+// dllimport or  dllexport annotations.
 if (DriverArgs.hasArg(
 options::OPT_fvisibility_externs_nodllstorageclass_EQ))
   DriverArgs.AddLastArg(
   CC1Args, options::OPT_fvisibility_externs_nodllstorageclass_EQ);
-else
+else if (getTriple().isPS4())
   CC1Args.push_back("-fvisibility-externs-nodllstorageclass=default");
+else
+  CC1Args.push_back("-fvisibility-externs-nodllstorageclass=keep");
   }
 }
 
diff --git a/clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c 
b/clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c
index 430827805a8fd..71f8661679eb7 100644
--- a/clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c
+++ b/clang/test/Driver/ps4-ps5-visibility-dllstorageclass.c
@@ -1,16 +1,19 @@
 // Check behaviour of -fvisibility-from-dllstorageclass options for PS4/PS5.
 
 // DEFINE: %{triple} =
+// DEFINE: %{prefix} =
 // DEFINE: %{run} = \
 // DEFINE: %clang -### -target %{triple} %s -Werror -o - 2>&1 | \
-// DEFINE:   FileCheck %s --check-prefix=DEFAULTS \
+// DEFINE:   FileCheck %s --check-prefixes=DEFAULTS,%{prefix} \
 // DEFINE: --implicit-check-not=-fvisibility-from-dllstorageclass \
 // DEFINE: --implicit-check-not=-fvisibility-dllexport \
 // DEFINE: --implicit-check-not=-fvisibility-nodllstorageclass \
 // DEFINE: --implicit-check-not=-fvisibility-externs-dllimport \
 // DEFINE: --implicit-check-not=-fvisibility-externs-nodllstorageclass
+// REDEFINE: %{prefix} = DEFAULTS-PS4
 // REDEFINE: %{triple} = x86_64-scei-ps4
 // RUN: %{run}
+// REDEFINE: %{prefix} = DEFAULTS-PS5
 // REDEFINE: %{triple} = x86_64-sie-ps5
 // RUN: %{run}
 //
@@ -20,25 +23,29 @@
 // REDEFINE: -fvisibility-from-dllstorageclass \
 // REDEFINE: -Werror \
 // REDEFINE: %s -o - 2>&1 | \
-// REDEFINE:   FileCheck %s --check-prefix=DEFAULTS \
+// REDEFINE:   FileCheck %s --check-prefixes=DEFAULTS,%{prefix} \
 // REDEFINE: --implicit-check-not=-fvisibility-from-dllstorageclass \
 // REDEFINE: --implicit-check-not=-fvisibility-dllexport \
 // REDEFINE: --implicit-check-not=-fvisibility-nodllstorageclass \
 // REDEFINE: --implicit-check-not=-fvisibility-externs-dllimport \
 // REDEFINE: --implicit-check-not=-fvisibility-externs-nodllstorageclass
+// RED

[clang] [Driver][PS5] Set visibility option defaults (PR #92091)

2024-05-14 Thread via cfe-commits


@@ -376,24 +382,32 @@ void toolchains::PS4PS5Base::addClangTargetOptions(
 else
   CC1Args.push_back("-fvisibility-dllexport=protected");
 
+// For PS4 we override the visibilty of globals definitions without

bd1976bris wrote:

Note for reviewer: This comment is new and replaces a comment in our downstream 
code that wasn't suitable for upstream as it referenced internal tracking 
numbers.

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


[clang] [Driver][PS5] Set visibility option defaults (PR #92091)

2024-05-14 Thread via cfe-commits


@@ -376,24 +382,32 @@ void toolchains::PS4PS5Base::addClangTargetOptions(
 else
   CC1Args.push_back("-fvisibility-dllexport=protected");
 
+// For PS4 we override the visibilty of globals definitions without
+// dllimport or  dllexport annotations.
 if (DriverArgs.hasArg(options::OPT_fvisibility_nodllstorageclass_EQ))
   DriverArgs.AddLastArg(CC1Args,
 options::OPT_fvisibility_nodllstorageclass_EQ);
-else
+else if (getTriple().isPS4())
   CC1Args.push_back("-fvisibility-nodllstorageclass=hidden");
+else
+  CC1Args.push_back("-fvisibility-nodllstorageclass=keep");
 
 if (DriverArgs.hasArg(options::OPT_fvisibility_externs_dllimport_EQ))
   DriverArgs.AddLastArg(CC1Args,
 options::OPT_fvisibility_externs_dllimport_EQ);
 else
   CC1Args.push_back("-fvisibility-externs-dllimport=default");
 
+// For PS4 we override the visibilty of external globals without

bd1976bris wrote:

Note for reviewer: This comment is new and replaces a comment in our downstream 
code that wasn't suitable for upstream as it referenced internal tracking 
numbers.

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


[clang] [clang][SPIR-V] Always add convergence intrinsics (PR #88918)

2024-05-14 Thread Nathan Gauër via cfe-commits

https://github.com/Keenuts updated 
https://github.com/llvm/llvm-project/pull/88918

From 440cdfa4132a969702348c32f2810924012c5ea6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nathan=20Gau=C3=ABr?= 
Date: Mon, 15 Apr 2024 17:05:40 +0200
Subject: [PATCH 1/6] [clang][SPIR-V] Always add convervence intrinsics
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PR #80680 added bits in the codegen to lazily add convergence intrinsics
when required. This logic relied on the LoopStack. The issue is
when parsing the condition, the loopstack doesn't yet reflect the
correct values, as expected since we are not yet in the loop.

However, convergence tokens should sometimes already be available.
The solution which seemed the simplest is to greedily generate the
tokens when we generate SPIR-V.

Fixes #88144

Signed-off-by: Nathan Gauër 
---
 clang/lib/CodeGen/CGBuiltin.cpp   |  88 +
 clang/lib/CodeGen/CGCall.cpp  |   3 +
 clang/lib/CodeGen/CGStmt.cpp  |  94 ++
 clang/lib/CodeGen/CodeGenFunction.cpp |   9 ++
 clang/lib/CodeGen/CodeGenFunction.h   |   9 +-
 .../builtins/RWBuffer-constructor.hlsl|   1 -
 .../CodeGenHLSL/convergence/do.while.hlsl |  90 +
 clang/test/CodeGenHLSL/convergence/for.hlsl   | 121 ++
 clang/test/CodeGenHLSL/convergence/while.hlsl | 119 +
 9 files changed, 445 insertions(+), 89 deletions(-)
 create mode 100644 clang/test/CodeGenHLSL/convergence/do.while.hlsl
 create mode 100644 clang/test/CodeGenHLSL/convergence/for.hlsl
 create mode 100644 clang/test/CodeGenHLSL/convergence/while.hlsl

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index f9ee93049b12d..e251091c6ce3e 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -1141,91 +1141,8 @@ struct BitTest {
   static BitTest decodeBitTestBuiltin(unsigned BuiltinID);
 };
 
-// Returns the first convergence entry/loop/anchor instruction found in |BB|.
-// std::nullptr otherwise.
-llvm::IntrinsicInst *getConvergenceToken(llvm::BasicBlock *BB) {
-  for (auto &I : *BB) {
-auto *II = dyn_cast(&I);
-if (II && isConvergenceControlIntrinsic(II->getIntrinsicID()))
-  return II;
-  }
-  return nullptr;
-}
-
 } // namespace
 
-llvm::CallBase *
-CodeGenFunction::addConvergenceControlToken(llvm::CallBase *Input,
-llvm::Value *ParentToken) {
-  llvm::Value *bundleArgs[] = {ParentToken};
-  llvm::OperandBundleDef OB("convergencectrl", bundleArgs);
-  auto Output = llvm::CallBase::addOperandBundle(
-  Input, llvm::LLVMContext::OB_convergencectrl, OB, Input);
-  Input->replaceAllUsesWith(Output);
-  Input->eraseFromParent();
-  return Output;
-}
-
-llvm::IntrinsicInst *
-CodeGenFunction::emitConvergenceLoopToken(llvm::BasicBlock *BB,
-  llvm::Value *ParentToken) {
-  CGBuilderTy::InsertPoint IP = Builder.saveIP();
-  Builder.SetInsertPoint(&BB->front());
-  auto CB = Builder.CreateIntrinsic(
-  llvm::Intrinsic::experimental_convergence_loop, {}, {});
-  Builder.restoreIP(IP);
-
-  auto I = addConvergenceControlToken(CB, ParentToken);
-  return cast(I);
-}
-
-llvm::IntrinsicInst *
-CodeGenFunction::getOrEmitConvergenceEntryToken(llvm::Function *F) {
-  auto *BB = &F->getEntryBlock();
-  auto *token = getConvergenceToken(BB);
-  if (token)
-return token;
-
-  // Adding a convergence token requires the function to be marked as
-  // convergent.
-  F->setConvergent();
-
-  CGBuilderTy::InsertPoint IP = Builder.saveIP();
-  Builder.SetInsertPoint(&BB->front());
-  auto I = Builder.CreateIntrinsic(
-  llvm::Intrinsic::experimental_convergence_entry, {}, {});
-  assert(isa(I));
-  Builder.restoreIP(IP);
-
-  return cast(I);
-}
-
-llvm::IntrinsicInst *
-CodeGenFunction::getOrEmitConvergenceLoopToken(const LoopInfo *LI) {
-  assert(LI != nullptr);
-
-  auto *token = getConvergenceToken(LI->getHeader());
-  if (token)
-return token;
-
-  llvm::IntrinsicInst *PII =
-  LI->getParent()
-  ? emitConvergenceLoopToken(
-LI->getHeader(), 
getOrEmitConvergenceLoopToken(LI->getParent()))
-  : getOrEmitConvergenceEntryToken(LI->getHeader()->getParent());
-
-  return emitConvergenceLoopToken(LI->getHeader(), PII);
-}
-
-llvm::CallBase *
-CodeGenFunction::addControlledConvergenceToken(llvm::CallBase *Input) {
-  llvm::Value *ParentToken =
-  LoopStack.hasInfo()
-  ? getOrEmitConvergenceLoopToken(&LoopStack.getInfo())
-  : getOrEmitConvergenceEntryToken(Input->getFunction());
-  return addConvergenceControlToken(Input, ParentToken);
-}
-
 BitTest BitTest::decodeBitTestBuiltin(unsigned BuiltinID) {
   switch (BuiltinID) {
 // Main portable variants.
@@ -18402,12 +18319,9 @@ Value *CodeGenFunction::EmitHLSLBuiltinExpr(unsigned 
BuiltinID,
 ArrayRef{Op0}, nullptr, "dx.rsqrt");

[clang] [clang][SPIR-V] Always add convergence intrinsics (PR #88918)

2024-05-14 Thread Nathan Gauër via cfe-commits

Keenuts wrote:

rebased on main, local tests are passing, waiting on CI to merge.

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


[clang] Avoid unevaluated implicit private (PR #92055)

2024-05-14 Thread Simon Pilgrim via cfe-commits

https://github.com/RKSimon updated 
https://github.com/llvm/llvm-project/pull/92055

>From 6946c9f1285d5a27eafcdbf13f79c0641736198d Mon Sep 17 00:00:00 2001
From: Sunil Kuravinakop 
Date: Thu, 9 May 2024 12:09:15 -0500
Subject: [PATCH 1/3] Avoiding DeclRefExpr with "non_odr_use_unevaluated" to
 declare "Implicit Private variable" DeclRefExpr.

  Changes to be committed:
modified:   clang/lib/Sema/SemaOpenMP.cpp
---
 clang/lib/Sema/SemaOpenMP.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index cf5447f223d45..bb6518099b4df 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -3757,7 +3757,8 @@ class DSAAttrChecker final : public 
StmtVisitor {
   void VisitDeclRefExpr(DeclRefExpr *E) {
 if (TryCaptureCXXThisMembers || E->isTypeDependent() ||
 E->isValueDependent() || E->containsUnexpandedParameterPack() ||
-E->isInstantiationDependent())
+E->isInstantiationDependent() ||
+E->isNonOdrUse() == clang::NOUR_Unevaluated)
   return;
 if (auto *VD = dyn_cast(E->getDecl())) {
   // Check the datasharing rules for the expressions in the clauses.

>From 862907f4a6d7cebfb1b816e9ec890c39d0da112e Mon Sep 17 00:00:00 2001
From: Sunil Kuravinakop 
Date: Mon, 13 May 2024 01:28:59 -0500
Subject: [PATCH 2/3] Adding checks for proper declaration of DeclRefExpr under
 the task directive (when variable can be non_odr_use_unevaluated).

  Changes to be committed:
modified:   clang/test/OpenMP/task_ast_print.cpp
---
 clang/test/OpenMP/task_ast_print.cpp | 34 
 1 file changed, 34 insertions(+)

diff --git a/clang/test/OpenMP/task_ast_print.cpp 
b/clang/test/OpenMP/task_ast_print.cpp
index 12923e6ab4244..9d545c5f6716c 100644
--- a/clang/test/OpenMP/task_ast_print.cpp
+++ b/clang/test/OpenMP/task_ast_print.cpp
@@ -5,6 +5,7 @@
 // RUN: %clang_cc1 -verify -Wno-vla -fopenmp-simd -ast-print %s | FileCheck %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -verify -Wno-vla 
%s -ast-print | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fopenmp -ast-dump  %s | 
FileCheck %s --check-prefix=DUMP
 // expected-no-diagnostics
 
 #ifndef HEADER
@@ -208,4 +209,37 @@ int main(int argc, char **argv) {
 extern template int S::TS;
 extern template long S::TS;
 
+int
+implicit_firstprivate() {
+
+#pragma omp parallel num_threads(1)
+  {
+int i = 0;
+// DUMP : OMPTaskDirective
+// DUMP-NEXT : OMPFirstprivateClause
+// DUMP-NEXT : DeclRefExpr {{.+}} 'i' {{.+}} 
refers_to_enclosing_variable_or_capture
+#pragma omp task
+{
+   int j = sizeof(i);
+   j = i;
+}
+  }
+}
+
+int
+no_implicit_firstprivate() {
+
+#pragma omp parallel num_threads(1)
+  {
+int i = 0;
+// DUMP : OMPTaskDirective
+// DUMP-NEXT : CapturedStmt
+// DUMP : DeclRefExpr {{.+}} 'i' {{.+}} non_odr_use_unevaluated 
refers_to_enclosing_variable_or_capture
+#pragma omp task
+{
+   int j = sizeof(i);
+}
+  }
+}
+
 #endif

>From 96c997ad09d87e48af7929b527259c5037242e10 Mon Sep 17 00:00:00 2001
From: Sunil Kuravinakop 
Date: Tue, 14 May 2024 04:47:00 -0500
Subject: [PATCH 3/3] Minor changes to the test case.   Changes to be
 committed: modified:   clang/test/OpenMP/task_ast_print.cpp

---
 clang/test/OpenMP/task_ast_print.cpp | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/clang/test/OpenMP/task_ast_print.cpp 
b/clang/test/OpenMP/task_ast_print.cpp
index 9d545c5f6716c..cb2cc63f63214 100644
--- a/clang/test/OpenMP/task_ast_print.cpp
+++ b/clang/test/OpenMP/task_ast_print.cpp
@@ -209,15 +209,16 @@ int main(int argc, char **argv) {
 extern template int S::TS;
 extern template long S::TS;
 
-int
+// DUMP-LABEL:  FunctionDecl {{.*}} implicit_firstprivate
+void
 implicit_firstprivate() {
 
 #pragma omp parallel num_threads(1)
   {
 int i = 0;
-// DUMP : OMPTaskDirective
-// DUMP-NEXT : OMPFirstprivateClause
-// DUMP-NEXT : DeclRefExpr {{.+}} 'i' {{.+}} 
refers_to_enclosing_variable_or_capture
+// DUMP: OMPTaskDirective 
+// DUMP-NEXT: OMPFirstprivateClause
+// DUMP-NEXT: DeclRefExpr {{.+}} 'i' {{.+}} 
refers_to_enclosing_variable_or_capture
 #pragma omp task
 {
int j = sizeof(i);
@@ -226,15 +227,16 @@ implicit_firstprivate() {
   }
 }
 
-int
+// DUMP-LABEL:  FunctionDecl {{.*}} no_implicit_firstprivate
+void
 no_implicit_firstprivate() {
 
 #pragma omp parallel num_threads(1)
   {
 int i = 0;
-// DUMP : OMPTaskDirective
-// DUMP-NEXT : CapturedStmt
-// DUMP : DeclRefExpr {{.+}} 'i' {{.+}} non_odr_use_unevaluated 
refers_to_enclosing_variable_or_capture
+// DUMP: OMPTaskDirective
+// DUMP-NEXT: CapturedStmt
+// DUMP: DeclRefExpr {{.+}} 'i' {{.+}} non_odr_use_unevaluated 
refers_to_enclosing_variable_or_capture
 #p

[clang] Avoid unevaluated implicit private (PR #92055)

2024-05-14 Thread via cfe-commits

https://github.com/SunilKuravinakop updated 
https://github.com/llvm/llvm-project/pull/92055

>From 6946c9f1285d5a27eafcdbf13f79c0641736198d Mon Sep 17 00:00:00 2001
From: Sunil Kuravinakop 
Date: Thu, 9 May 2024 12:09:15 -0500
Subject: [PATCH 1/3] Avoiding DeclRefExpr with "non_odr_use_unevaluated" to
 declare "Implicit Private variable" DeclRefExpr.

  Changes to be committed:
modified:   clang/lib/Sema/SemaOpenMP.cpp
---
 clang/lib/Sema/SemaOpenMP.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index cf5447f223d45..bb6518099b4df 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -3757,7 +3757,8 @@ class DSAAttrChecker final : public 
StmtVisitor {
   void VisitDeclRefExpr(DeclRefExpr *E) {
 if (TryCaptureCXXThisMembers || E->isTypeDependent() ||
 E->isValueDependent() || E->containsUnexpandedParameterPack() ||
-E->isInstantiationDependent())
+E->isInstantiationDependent() ||
+E->isNonOdrUse() == clang::NOUR_Unevaluated)
   return;
 if (auto *VD = dyn_cast(E->getDecl())) {
   // Check the datasharing rules for the expressions in the clauses.

>From 862907f4a6d7cebfb1b816e9ec890c39d0da112e Mon Sep 17 00:00:00 2001
From: Sunil Kuravinakop 
Date: Mon, 13 May 2024 01:28:59 -0500
Subject: [PATCH 2/3] Adding checks for proper declaration of DeclRefExpr under
 the task directive (when variable can be non_odr_use_unevaluated).

  Changes to be committed:
modified:   clang/test/OpenMP/task_ast_print.cpp
---
 clang/test/OpenMP/task_ast_print.cpp | 34 
 1 file changed, 34 insertions(+)

diff --git a/clang/test/OpenMP/task_ast_print.cpp 
b/clang/test/OpenMP/task_ast_print.cpp
index 12923e6ab4244..9d545c5f6716c 100644
--- a/clang/test/OpenMP/task_ast_print.cpp
+++ b/clang/test/OpenMP/task_ast_print.cpp
@@ -5,6 +5,7 @@
 // RUN: %clang_cc1 -verify -Wno-vla -fopenmp-simd -ast-print %s | FileCheck %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -verify -Wno-vla 
%s -ast-print | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fopenmp -ast-dump  %s | 
FileCheck %s --check-prefix=DUMP
 // expected-no-diagnostics
 
 #ifndef HEADER
@@ -208,4 +209,37 @@ int main(int argc, char **argv) {
 extern template int S::TS;
 extern template long S::TS;
 
+int
+implicit_firstprivate() {
+
+#pragma omp parallel num_threads(1)
+  {
+int i = 0;
+// DUMP : OMPTaskDirective
+// DUMP-NEXT : OMPFirstprivateClause
+// DUMP-NEXT : DeclRefExpr {{.+}} 'i' {{.+}} 
refers_to_enclosing_variable_or_capture
+#pragma omp task
+{
+   int j = sizeof(i);
+   j = i;
+}
+  }
+}
+
+int
+no_implicit_firstprivate() {
+
+#pragma omp parallel num_threads(1)
+  {
+int i = 0;
+// DUMP : OMPTaskDirective
+// DUMP-NEXT : CapturedStmt
+// DUMP : DeclRefExpr {{.+}} 'i' {{.+}} non_odr_use_unevaluated 
refers_to_enclosing_variable_or_capture
+#pragma omp task
+{
+   int j = sizeof(i);
+}
+  }
+}
+
 #endif

>From 96c997ad09d87e48af7929b527259c5037242e10 Mon Sep 17 00:00:00 2001
From: Sunil Kuravinakop 
Date: Tue, 14 May 2024 04:47:00 -0500
Subject: [PATCH 3/3] Minor changes to the test case.   Changes to be
 committed: modified:   clang/test/OpenMP/task_ast_print.cpp

---
 clang/test/OpenMP/task_ast_print.cpp | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/clang/test/OpenMP/task_ast_print.cpp 
b/clang/test/OpenMP/task_ast_print.cpp
index 9d545c5f6716c..cb2cc63f63214 100644
--- a/clang/test/OpenMP/task_ast_print.cpp
+++ b/clang/test/OpenMP/task_ast_print.cpp
@@ -209,15 +209,16 @@ int main(int argc, char **argv) {
 extern template int S::TS;
 extern template long S::TS;
 
-int
+// DUMP-LABEL:  FunctionDecl {{.*}} implicit_firstprivate
+void
 implicit_firstprivate() {
 
 #pragma omp parallel num_threads(1)
   {
 int i = 0;
-// DUMP : OMPTaskDirective
-// DUMP-NEXT : OMPFirstprivateClause
-// DUMP-NEXT : DeclRefExpr {{.+}} 'i' {{.+}} 
refers_to_enclosing_variable_or_capture
+// DUMP: OMPTaskDirective 
+// DUMP-NEXT: OMPFirstprivateClause
+// DUMP-NEXT: DeclRefExpr {{.+}} 'i' {{.+}} 
refers_to_enclosing_variable_or_capture
 #pragma omp task
 {
int j = sizeof(i);
@@ -226,15 +227,16 @@ implicit_firstprivate() {
   }
 }
 
-int
+// DUMP-LABEL:  FunctionDecl {{.*}} no_implicit_firstprivate
+void
 no_implicit_firstprivate() {
 
 #pragma omp parallel num_threads(1)
   {
 int i = 0;
-// DUMP : OMPTaskDirective
-// DUMP-NEXT : CapturedStmt
-// DUMP : DeclRefExpr {{.+}} 'i' {{.+}} non_odr_use_unevaluated 
refers_to_enclosing_variable_or_capture
+// DUMP: OMPTaskDirective
+// DUMP-NEXT: CapturedStmt
+// DUMP: DeclRefExpr {{.+}} 'i' {{.+}} non_odr_use_unevaluated 
refers_to_enclosing_variable_or_captur

[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

2024-05-14 Thread Balazs Benics via cfe-commits

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


[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

2024-05-14 Thread Balazs Benics via cfe-commits


@@ -1964,39 +1964,55 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
 case Stmt::CXXDefaultArgExprClass:
 case Stmt::CXXDefaultInitExprClass: {
   Bldr.takeNodes(Pred);
-  ExplodedNodeSet PreVisit;
-  getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
+  ExplodedNodeSet CheckedSet;
+  getCheckerManager().runCheckersForPreStmt(CheckedSet, Pred, S, *this);
 
   ExplodedNodeSet Tmp;
-  StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
+  StmtNodeBuilder Bldr2(CheckedSet, Tmp, *currBldrCtx);
 
-  const Expr *ArgE;
-  if (const auto *DefE = dyn_cast(S))
+  bool HasRewrittenInit = false;
+  const Expr *ArgE = nullptr;
+  if (const auto *DefE = dyn_cast(S)) {
 ArgE = DefE->getExpr();
-  else if (const auto *DefE = dyn_cast(S))
+HasRewrittenInit = DefE->hasRewrittenInit();
+  } else if (const auto *DefE = dyn_cast(S)) {
 ArgE = DefE->getExpr();
-  else
+HasRewrittenInit = DefE->hasRewrittenInit();
+  } else
 llvm_unreachable("unknown constant wrapper kind");
 
-  bool IsTemporary = false;
-  if (const auto *MTE = dyn_cast(ArgE)) {
-ArgE = MTE->getSubExpr();
-IsTemporary = true;
-  }
+  if (HasRewrittenInit) {
+for (auto *I : CheckedSet) {
+  ProgramStateRef state = (*I).getState();
+  const LocationContext *LCtx = (*I).getLocationContext();
+  SVal Val = state->getSVal(ArgE, LCtx);
+  state = state->BindExpr(S, LCtx, Val);
+  Bldr2.generateNode(S, I, state);
+}
+  } else {
+// If it's not rewritten, the contents of these expressions are not
+// actually part of the current function, so we fall back to constant
+// evaluation.
+bool IsTemporary = false;
+if (const auto *MTE = dyn_cast(ArgE)) {
+  ArgE = MTE->getSubExpr();
+  IsTemporary = true;
+}
+
+std::optional ConstantVal = svalBuilder.getConstantVal(ArgE);
+if (!ConstantVal)
+  ConstantVal = UnknownVal();
 
-  std::optional ConstantVal = svalBuilder.getConstantVal(ArgE);
-  if (!ConstantVal)
-ConstantVal = UnknownVal();
-
-  const LocationContext *LCtx = Pred->getLocationContext();
-  for (const auto I : PreVisit) {
-ProgramStateRef State = I->getState();
-State = State->BindExpr(S, LCtx, *ConstantVal);
-if (IsTemporary)
-  State = createTemporaryRegionIfNeeded(State, LCtx,
-cast(S),
-cast(S));
-Bldr2.generateNode(S, I, State);
+const LocationContext *LCtx = Pred->getLocationContext();
+for (auto *I : CheckedSet) {
+  ProgramStateRef State = I->getState();
+  State = State->BindExpr(S, LCtx, *ConstantVal);

steakhal wrote:

```suggestion
  State = State->BindExpr(S, LCtx, ConstantVal.value_or(UnknownVal()));
```

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


[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

2024-05-14 Thread Balazs Benics via cfe-commits


@@ -1964,39 +1964,55 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
 case Stmt::CXXDefaultArgExprClass:
 case Stmt::CXXDefaultInitExprClass: {
   Bldr.takeNodes(Pred);
-  ExplodedNodeSet PreVisit;
-  getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
+  ExplodedNodeSet CheckedSet;
+  getCheckerManager().runCheckersForPreStmt(CheckedSet, Pred, S, *this);
 
   ExplodedNodeSet Tmp;
-  StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
+  StmtNodeBuilder Bldr2(CheckedSet, Tmp, *currBldrCtx);
 
-  const Expr *ArgE;
-  if (const auto *DefE = dyn_cast(S))
+  bool HasRewrittenInit = false;
+  const Expr *ArgE = nullptr;
+  if (const auto *DefE = dyn_cast(S)) {
 ArgE = DefE->getExpr();
-  else if (const auto *DefE = dyn_cast(S))
+HasRewrittenInit = DefE->hasRewrittenInit();
+  } else if (const auto *DefE = dyn_cast(S)) {
 ArgE = DefE->getExpr();
-  else
+HasRewrittenInit = DefE->hasRewrittenInit();
+  } else
 llvm_unreachable("unknown constant wrapper kind");
 
-  bool IsTemporary = false;
-  if (const auto *MTE = dyn_cast(ArgE)) {
-ArgE = MTE->getSubExpr();
-IsTemporary = true;
-  }
+  if (HasRewrittenInit) {
+for (auto *I : CheckedSet) {
+  ProgramStateRef state = (*I).getState();
+  const LocationContext *LCtx = (*I).getLocationContext();
+  SVal Val = state->getSVal(ArgE, LCtx);
+  state = state->BindExpr(S, LCtx, Val);
+  Bldr2.generateNode(S, I, state);
+}
+  } else {
+// If it's not rewritten, the contents of these expressions are not
+// actually part of the current function, so we fall back to constant
+// evaluation.
+bool IsTemporary = false;
+if (const auto *MTE = dyn_cast(ArgE)) {
+  ArgE = MTE->getSubExpr();
+  IsTemporary = true;
+}
+
+std::optional ConstantVal = svalBuilder.getConstantVal(ArgE);
+if (!ConstantVal)
+  ConstantVal = UnknownVal();

steakhal wrote:

```suggestion
```
I'd just drop this and use `value_or()` at the only use-site instead.

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


[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

2024-05-14 Thread Balazs Benics via cfe-commits

https://github.com/steakhal commented:

In general, this PR looks good to me.
I only have some nits inline.

If it didn't break anything, it should be good to go.

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


[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

2024-05-14 Thread Balazs Benics via cfe-commits


@@ -1114,17 +1114,16 @@ void fCXX11MemberInitTest1() {
   CXX11MemberInitTest1();
 }
 
+#ifdef PEDANTIC

steakhal wrote:

I don't think you need to guard this section of code if you were using `// 
pedantic-note {{...}}` and `// pedantic-warning {{...}}` in the guarded checks.

I also think that the `// TODO: we'd expect the warning: {{2 uninitializeds 
field}}` comment refers to this new appearing report, so you should just drop 
that comment from the `fCXX11MemberInitTest2` function.

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


[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

2024-05-14 Thread Balazs Benics via cfe-commits


@@ -1964,39 +1964,55 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
 case Stmt::CXXDefaultArgExprClass:
 case Stmt::CXXDefaultInitExprClass: {
   Bldr.takeNodes(Pred);
-  ExplodedNodeSet PreVisit;
-  getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
+  ExplodedNodeSet CheckedSet;

steakhal wrote:

I think I'd prefer the old name here. That is how one would find these in the 
other places.

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


[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

2024-05-14 Thread Balazs Benics via cfe-commits


@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -std=c++14 -verify  %s \
+// RUN: %clang_analyze_cc1 -std=c++14 -verify=expected,pedantic  %s \

steakhal wrote:

You introduced the `pedantic` verify prefix, but never actually used it.

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


[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #91879)

2024-05-14 Thread Balazs Benics via cfe-commits


@@ -1964,39 +1964,55 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode 
*Pred,
 case Stmt::CXXDefaultArgExprClass:
 case Stmt::CXXDefaultInitExprClass: {
   Bldr.takeNodes(Pred);
-  ExplodedNodeSet PreVisit;
-  getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
+  ExplodedNodeSet CheckedSet;
+  getCheckerManager().runCheckersForPreStmt(CheckedSet, Pred, S, *this);
 
   ExplodedNodeSet Tmp;
-  StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
+  StmtNodeBuilder Bldr2(CheckedSet, Tmp, *currBldrCtx);
 
-  const Expr *ArgE;
-  if (const auto *DefE = dyn_cast(S))
+  bool HasRewrittenInit = false;
+  const Expr *ArgE = nullptr;
+  if (const auto *DefE = dyn_cast(S)) {
 ArgE = DefE->getExpr();
-  else if (const auto *DefE = dyn_cast(S))
+HasRewrittenInit = DefE->hasRewrittenInit();
+  } else if (const auto *DefE = dyn_cast(S)) {
 ArgE = DefE->getExpr();
-  else
+HasRewrittenInit = DefE->hasRewrittenInit();
+  } else
 llvm_unreachable("unknown constant wrapper kind");
 
-  bool IsTemporary = false;
-  if (const auto *MTE = dyn_cast(ArgE)) {
-ArgE = MTE->getSubExpr();
-IsTemporary = true;
-  }
+  if (HasRewrittenInit) {
+for (auto *I : CheckedSet) {
+  ProgramStateRef state = (*I).getState();
+  const LocationContext *LCtx = (*I).getLocationContext();
+  SVal Val = state->getSVal(ArgE, LCtx);
+  state = state->BindExpr(S, LCtx, Val);
+  Bldr2.generateNode(S, I, state);

steakhal wrote:

```suggestion
for (ExplodedNode *N : CheckedSet) {
  ProgramStateRef State = N->getState();
  const LocationContext *LCtx = N->getLocationContext();
  State = State->BindExpr(S, LCtx, State->getSVal(ArgE, LCtx));
  Bldr2.generateNode(S, N, State);
```

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


[clang] 8fe21fd - [clang][analyzer] Ignore try-statements in UnreachableCode checker (#91675)

2024-05-14 Thread via cfe-commits

Author: Andrew Sukach
Date: 2024-05-14T11:57:10+02:00
New Revision: 8fe21fda7469f2fdf83980a2720a15baad74ae4f

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

LOG: [clang][analyzer] Ignore try-statements in UnreachableCode checker (#91675)

Fixes #90162

Added: 
clang/test/Analysis/unreachable-code-exceptions.cpp

Modified: 
clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
index d24a124f5ffee..7ce9a5b5bb6dc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
@@ -159,6 +159,8 @@ void UnreachableCodeChecker::checkEndAnalysis(ExplodedGraph 
&G,
   SL = DL.asLocation();
   if (SR.isInvalid() || !SL.isValid())
 continue;
+  if (isa(S))
+continue;
 }
 else
   continue;
@@ -254,4 +256,4 @@ void ento::registerUnreachableCodeChecker(CheckerManager 
&mgr) {
 
 bool ento::shouldRegisterUnreachableCodeChecker(const CheckerManager &mgr) {
   return true;
-}
+}
\ No newline at end of file

diff  --git a/clang/test/Analysis/unreachable-code-exceptions.cpp 
b/clang/test/Analysis/unreachable-code-exceptions.cpp
new file mode 100644
index 0..f47674ea8097d
--- /dev/null
+++ b/clang/test/Analysis/unreachable-code-exceptions.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -verify %s -fcxx-exceptions -fexceptions 
-analyzer-checker=core,alpha.deadcode.UnreachableCode
+
+// expected-no-diagnostics
+
+void foo();
+
+void fp_90162() {
+  try { // no-warning: The TryStmt shouldn't be unreachable.
+foo();
+  } catch (int) {
+foo(); // We assume that catch handlers are reachable.
+  }
+}



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


[clang] [clang][analyzer] Ignore try-statements in UnreachableCode checker (PR #91675)

2024-05-14 Thread Balazs Benics via cfe-commits

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


[clang] [analyzer] Refactor recognition of the errno getter functions (PR #91531)

2024-05-14 Thread Balazs Benics via cfe-commits
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy 
Message-ID:
In-Reply-To: 


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

LGTM.
I'd only add some safety barriers for some places, denoted my suggested edits.

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


[clang] [analyzer] Refactor recognition of the errno getter functions (PR #91531)

2024-05-14 Thread Balazs Benics via cfe-commits
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 


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


[clang] [analyzer] Refactor recognition of the errno getter functions (PR #91531)

2024-05-14 Thread Balazs Benics via cfe-commits
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 



@@ -136,53 +100,49 @@ void ErrnoModeling::checkBeginFunction(CheckerContext &C) 
const {
   ASTContext &ACtx = C.getASTContext();
   ProgramStateRef State = C.getState();
 
-  if (const auto *ErrnoVar = dyn_cast_or_null(ErrnoDecl)) {
-// There is an external 'errno' variable.
-// Use its memory region.
-// The memory region for an 'errno'-like variable is allocated in system
-// space by MemRegionManager.
-const MemRegion *ErrnoR =
-State->getRegion(ErrnoVar, C.getLocationContext());
+  const MemRegion *ErrnoR;
+
+  if (ErrnoDecl) {
+// There is an external 'errno' variable, so we can simply use the memory
+// region that's associated with it.
+ErrnoR = State->getRegion(ErrnoDecl, C.getLocationContext());
 assert(ErrnoR && "Memory region should exist for the 'errno' variable.");
-State = State->set(ErrnoR);
-State =
-errno_modeling::setErrnoValue(State, C, 0, errno_modeling::Irrelevant);
-C.addTransition(State);
-  } else if (ErrnoDecl) {
-assert(isa(ErrnoDecl) && "Invalid errno location function.");
-// There is a function that returns the location of 'errno'.
-// We must create a memory region for it in system space.
-// Currently a symbolic region is used with an artifical symbol.
-// FIXME: It is better to have a custom (new) kind of MemRegion for such
-// cases.
+  } else {
+// There is no 'errno' variable, so create a new symbolic memory region
+// that can be used to model the return value of the "get the location of
+// errno" internal functions.
+// NOTE: this `SVal` is created even if errno is not defined or used.
 SValBuilder &SVB = C.getSValBuilder();
 MemRegionManager &RMgr = C.getStateManager().getRegionManager();
 
 const MemSpaceRegion *GlobalSystemSpace =
 RMgr.getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind);
 
 // Create an artifical symbol for the region.
-// It is not possible to associate a statement or expression in this case.
+// Note that it is not possible to associate a statement or expression in
+// this case and the `symbolTag` (opaque pointer tag) is just the address
+// of the data member `ErrnoDecl` of the singleton `ErrnoModeling` checker
+// object.
 const SymbolConjured *Sym = SVB.conjureSymbol(
 nullptr, C.getLocationContext(),
 ACtx.getLValueReferenceType(ACtx.IntTy), C.blockCount(), &ErrnoDecl);
 
 // The symbolic region is untyped, create a typed sub-region in it.
 // The ElementRegion is used to make the errno region a typed region.
-const MemRegion *ErrnoR = RMgr.getElementRegion(
+ErrnoR = RMgr.getElementRegion(
 ACtx.IntTy, SVB.makeZeroArrayIndex(),
 RMgr.getSymbolicRegion(Sym, GlobalSystemSpace), C.getASTContext());
-State = State->set(ErrnoR);
-State =
-errno_modeling::setErrnoValue(State, C, 0, errno_modeling::Irrelevant);
-C.addTransition(State);
   }
+  State = State->set(ErrnoR);

steakhal wrote:

```suggestion
  assert(ErrnoR);
  State = State->set(ErrnoR);
```

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


[clang] [analyzer] Refactor recognition of the errno getter functions (PR #91531)

2024-05-14 Thread Balazs Benics via cfe-commits
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 



@@ -136,53 +100,49 @@ void ErrnoModeling::checkBeginFunction(CheckerContext &C) 
const {
   ASTContext &ACtx = C.getASTContext();
   ProgramStateRef State = C.getState();
 
-  if (const auto *ErrnoVar = dyn_cast_or_null(ErrnoDecl)) {
-// There is an external 'errno' variable.
-// Use its memory region.
-// The memory region for an 'errno'-like variable is allocated in system
-// space by MemRegionManager.
-const MemRegion *ErrnoR =
-State->getRegion(ErrnoVar, C.getLocationContext());
+  const MemRegion *ErrnoR;

steakhal wrote:

```suggestion
  const MemRegion *ErrnoR = nullptr;
```

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


[clang] [flang] [flang] Add nsw flag to do-variable increment with a new option (PR #91579)

2024-05-14 Thread Tom Eccles via cfe-commits

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

Looks great to me. Thanks for all of your work on this!

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


[clang] 58b9564 - [clang][Interp][NFC] Add some assertions

2024-05-14 Thread Timm Bäder via cfe-commits

Author: Timm Bäder
Date: 2024-05-14T12:26:04+02:00
New Revision: 58b9564d5d12063bb9c662039802ede8df615374

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

LOG: [clang][Interp][NFC] Add some assertions

Make sure we pass a non-null Descriptor when creating a new Block.

Added: 


Modified: 
clang/lib/AST/Interp/InterpBlock.h

Removed: 




diff  --git a/clang/lib/AST/Interp/InterpBlock.h 
b/clang/lib/AST/Interp/InterpBlock.h
index 6d5856fbd4ea1..506034e880d0b 100644
--- a/clang/lib/AST/Interp/InterpBlock.h
+++ b/clang/lib/AST/Interp/InterpBlock.h
@@ -51,11 +51,15 @@ class Block final {
   /// Creates a new block.
   Block(const std::optional &DeclID, const Descriptor *Desc,
 bool IsStatic = false, bool IsExtern = false)
-  : DeclID(DeclID), IsStatic(IsStatic), IsExtern(IsExtern), Desc(Desc) {}
+  : DeclID(DeclID), IsStatic(IsStatic), IsExtern(IsExtern), Desc(Desc) {
+assert(Desc);
+  }
 
   Block(const Descriptor *Desc, bool IsStatic = false, bool IsExtern = false)
   : DeclID((unsigned)-1), IsStatic(IsStatic), IsExtern(IsExtern),
-Desc(Desc) {}
+Desc(Desc) {
+  assert(Desc);
+}
 
   /// Returns the block's descriptor.
   const Descriptor *getDescriptor() const { return Desc; }



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


[clang] 0aa5fa9 - [clang][Interp][NFC] Improve Pointer::print()

2024-05-14 Thread Timm Bäder via cfe-commits

Author: Timm Bäder
Date: 2024-05-14T12:26:05+02:00
New Revision: 0aa5fa9630d0f4ea707c5b8d5cfa2f4bc8d06a14

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

LOG: [clang][Interp][NFC] Improve Pointer::print()

Added: 


Modified: 
clang/lib/AST/Interp/Pointer.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/Pointer.cpp 
b/clang/lib/AST/Interp/Pointer.cpp
index 12bef73f7e21c..d2e34f2c7f09e 100644
--- a/clang/lib/AST/Interp/Pointer.cpp
+++ b/clang/lib/AST/Interp/Pointer.cpp
@@ -181,12 +181,12 @@ void Pointer::print(llvm::raw_ostream &OS) const {
   if (isBlockPointer()) {
 OS << "Block) {";
 
-if (PointeeStorage.BS.Base == RootPtrMark)
-  OS << "rootptr, ";
+if (isRoot())
+  OS << "rootptr(" << PointeeStorage.BS.Base << "), ";
 else
   OS << PointeeStorage.BS.Base << ", ";
 
-if (Offset == PastEndMark)
+if (isElementPastEnd())
   OS << "pastend, ";
 else
   OS << Offset << ", ";



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


[clang] 5865482 - [clang][Interp][NFC] Don't pass on metadata size for composite arrays

2024-05-14 Thread Timm Bäder via cfe-commits

Author: Timm Bäder
Date: 2024-05-14T12:26:05+02:00
New Revision: 5865482049872d3ae52ea5559abb9e8f4a1e55e5

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

LOG: [clang][Interp][NFC] Don't pass on metadata size for composite arrays

We don't need the metadata size for every element, just for the topmost
descriptor.

Added: 


Modified: 
clang/lib/AST/Interp/Program.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/Program.cpp 
b/clang/lib/AST/Interp/Program.cpp
index 6606149f1f697..0b95db8492695 100644
--- a/clang/lib/AST/Interp/Program.cpp
+++ b/clang/lib/AST/Interp/Program.cpp
@@ -372,7 +372,7 @@ Descriptor *Program::createDescriptor(const DeclTy &D, 
const Type *Ty,
 // Arrays of composites. In this case, the array is a list of pointers,
 // followed by the actual elements.
 const Descriptor *ElemDesc = createDescriptor(
-D, ElemTy.getTypePtr(), MDSize, IsConst, IsTemporary);
+D, ElemTy.getTypePtr(), std::nullopt, IsConst, IsTemporary);
 if (!ElemDesc)
   return nullptr;
 unsigned ElemSize =



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


[clang] [clang][analyzer] Add checker 'security.SetgidSetuidOrder' (PR #91445)

2024-05-14 Thread Balazs Benics via cfe-commits
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


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


[clang] [clang][analyzer] Add checker 'security.SetgidSetuidOrder' (PR #91445)

2024-05-14 Thread Balazs Benics via cfe-commits
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -30,23 +30,20 @@ enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid 
};
 
 class SetgidSetuidOrderChecker
 : public Checker {
-  const BugType BT_WrongRevocationOrder{
-  this, "Possible wrong order of privilege revocation"};
+  const BugType BT{this, "Possible wrong order of privilege revocation"};
 
   const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
   const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
 
   const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
   const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
 
-  CallDescriptionSet OtherSetPrivilegeDesc{
+  CallDescriptionSet const OtherSetPrivilegeDesc{

steakhal wrote:

I think in LLVM we usually use west-const.
```suggestion
  const CallDescriptionSet OtherSetPrivilegeDesc{
```

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


[clang] [clang][analyzer] Add checker 'security.SetgidSetuidOrder' (PR #91445)

2024-05-14 Thread Balazs Benics via cfe-commits
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -1011,6 +1011,11 @@ def FloatLoopCounter : Checker<"FloatLoopCounter">,
   Dependencies<[SecuritySyntaxChecker]>,
   Documentation;
 
+def SetgidSetuidOrderChecker : Checker<"SetgidSetuidOrder">,
+  HelpText<"Warn on possible reversed order of 'setgid(getgid()))' and 
'setuid(getuid())' (CERT: "
+   "POS36-C)">,

steakhal wrote:

```suggestion
  HelpText<"Warn on possible reversed order of 'setgid(getgid()))' and "
   "'setuid(getuid())' (CERT: POS36-C)">,
```
The previous line-brake seemed so arbitrary. This way we at least obey the 80 
column rule.

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


[clang] [clang][analyzer] Add checker 'security.SetgidSetuidOrder' (PR #91445)

2024-05-14 Thread Balazs Benics via cfe-commits
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


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

I think this looks good now.
I think to really reach the full potential of this checker, we must have a 
visitor/note tag for highlighting the places of the last `setgid` or `setuid` 
call in the report.

I also think that the docs could be improved.

Anyways. I think it's already pretty good, and I don't plan to do another round 
of reviews.

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


[clang] [clang][analyzer] Add checker 'security.SetgidSetuidOrder' (PR #91445)

2024-05-14 Thread Balazs Benics via cfe-commits
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -1179,6 +1179,34 @@ security.insecureAPI.DeprecatedOrUnsafeBufferHandling (C)
strncpy(buf, "a", 1); // warn
  }
 
+security.SetgidSetuidOrder (C)
+""
+When dropping user-level and group-level privileges in a program by using
+``setuid`` and ``setgid`` calls, it is important to reset the group-level
+privileges (with ``setgid``) first. Function ``setgid`` will likely fail if
+the superuser privileges are already dropped.
+
+The checker checks for sequences of ``setuid(getuid())`` and
+``setgid(getgid())`` calls (in this order). If such a sequence is found and
+there is no other privilege-changing function call (``seteuid``, ``setreuid``,
+``setresuid`` and the GID versions of these) in between, a warning is
+generated. The checker finds only exactly ``setuid(getuid())`` calls (and the
+GID versions), not for example if the result of ``getuid()`` is stored in a
+variable.
+
+This check corresponds to SEI CERT Rule `POS36-C 
`_.
+
+.. code-block:: c
+
+ void test1() {
+   if (setuid(getuid()) == -1) {
+ /* handle error condition */
+   }
+   if (setgid(getgid()) == -1) { // warn
+ /* handle error condition */
+   }
+ }
+

steakhal wrote:

I think the docs should explicitly mention how to fix/suppress this issue.
Like: One should call `setgid(getgid())` first and then `setuid(getuid())`.
Maybe also mention to pay attention to not mix up the APIs so that user ID and 
group IDs are not swapped, etc. Also add a remark to not forget to check the 
return value of these APIs. 

And this example looks suspiciously similar to the one present in the CERT docs.
Does it raise license and copy-right issues? If that's a concern, please rework 
the example.

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


[clang] [clang][analyzer] Add checker 'security.SetgidSetuidOrder' (PR #91445)

2024-05-14 Thread Balazs Benics via cfe-commits
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,196 @@
+//===-- SetgidSetuidOrderChecker.cpp - check privilege revocation calls 
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+//  This file defines a checker to detect possible reversed order of privilege
+//  revocations when 'setgid' and 'setuid' is used.
+//
+//===--===//
+
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+enum SetPrivilegeFunctionKind { Irrelevant, Setuid, Setgid };
+
+class SetgidSetuidOrderChecker
+: public Checker {
+  const BugType BT_WrongRevocationOrder{
+  this, "Possible wrong order of privilege revocation"};
+
+  const CallDescription SetuidDesc{CDM::CLibrary, {"setuid"}, 1};
+  const CallDescription SetgidDesc{CDM::CLibrary, {"setgid"}, 1};
+
+  const CallDescription GetuidDesc{CDM::CLibrary, {"getuid"}, 0};
+  const CallDescription GetgidDesc{CDM::CLibrary, {"getgid"}, 0};
+
+  CallDescriptionSet OtherSetPrivilegeDesc{
+  {CDM::CLibrary, {"seteuid"}, 1},   {CDM::CLibrary, {"setegid"}, 1},
+  {CDM::CLibrary, {"setreuid"}, 2},  {CDM::CLibrary, {"setregid"}, 2},
+  {CDM::CLibrary, {"setresuid"}, 3}, {CDM::CLibrary, {"setresgid"}, 3}};
+
+public:
+  SetgidSetuidOrderChecker() {}
+
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
+
+private:
+  ProgramStateRef processSetuid(ProgramStateRef State, const CallEvent &Call,
+CheckerContext &C) const;
+  ProgramStateRef processSetgid(ProgramStateRef State, const CallEvent &Call,
+CheckerContext &C) const;
+  ProgramStateRef processOther(ProgramStateRef State, const CallEvent &Call,
+   CheckerContext &C) const;
+  /// Check if a function like \c getuid or \c getgid is called directly from
+  /// the first argument of function called from \a Call.
+  bool isFunctionCalledInArg(const CallDescription &Desc,
+ const CallEvent &Call) const;
+  void emitReport(ProgramStateRef State, CheckerContext &C) const;
+};
+
+} // end anonymous namespace
+
+/// Store if there was a call to 'setuid(getuid())' or 'setgid(getgid())' not
+/// followed by other different privilege-change functions.
+/// If the value \c Setuid is stored and a 'setgid(getgid())' call is found we
+/// have found the bug to be reported. Value \c Setgid is used too to prevent
+/// warnings at a setgid-setuid-setgid sequence.
+REGISTER_TRAIT_WITH_PROGRAMSTATE(LastSetPrivilegeCall, 
SetPrivilegeFunctionKind)
+/// Store the symbol value of the last 'setuid(getuid())' call. This is used to
+/// detect if the result is compared to -1 and avoid warnings on that branch
+/// (which is the failure branch of the call).
+REGISTER_TRAIT_WITH_PROGRAMSTATE(LastSetuidCallSVal, SymbolRef)
+
+void SetgidSetuidOrderChecker::checkPostCall(const CallEvent &Call,
+ CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  if (SetuidDesc.matches(Call)) {
+State = processSetuid(State, Call, C);
+  } else if (SetgidDesc.matches(Call)) {
+State = processSetgid(State, Call, C);
+  } else if (OtherSetPrivilegeDesc.contains(Call)) {
+State = processOther(State, Call, C);
+  }
+  if (State)
+C.addTransition(State);
+}
+
+void SetgidSetuidOrderChecker::checkDeadSymbols(SymbolReaper &SymReaper,
+CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+
+  SymbolRef LastSetuidSym = State->get();
+  if (!LastSetuidSym)
+return;
+
+  if (!SymReaper.isDead(LastSetuidSym))
+return;
+
+  State = State->set(SymbolRef{});
+  C.addTransition(State, C.getPredecessor());
+}
+
+ProgramStateRef SetgidSetuidOrderChecker::evalAssume(ProgramStateRef

[clang] [clang][analyzer] Add checker 'security.SetgidSetuidOrder' (PR #91445)

2024-05-14 Thread Balazs Benics via cfe-commits
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


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


[clang] c1bd688 - [clang][Interp] Fix some dummy-related FIXME comments

2024-05-14 Thread Timm Bäder via cfe-commits

Author: Timm Bäder
Date: 2024-05-14T12:55:45+02:00
New Revision: c1bd68867497cf6e2f2afdba1a3a2993a47b5856

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

LOG: [clang][Interp] Fix some dummy-related FIXME comments

Added: 


Modified: 
clang/lib/AST/Interp/Interp.h
clang/lib/AST/Interp/Pointer.h
clang/lib/AST/Interp/Program.cpp
clang/test/AST/Interp/arrays.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index a0bf874300120..d9f23a4b8c965 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1569,9 +1569,7 @@ bool OffsetHelper(InterpState &S, CodePtr OpPC, const T 
&Offset,
 APSInt NewIndex =
 (Op == ArithOp::Add) ? (APIndex + APOffset) : (APIndex - APOffset);
 S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_array_index)
-<< NewIndex
-<< /*array*/ static_cast(!Ptr.inArray())
-<< static_cast(MaxIndex);
+<< NewIndex << /*array*/ static_cast(!Ptr.inArray()) << MaxIndex;
 Invalid = true;
   };
 
@@ -1598,7 +1596,7 @@ bool OffsetHelper(InterpState &S, CodePtr OpPC, const T 
&Offset,
 }
   }
 
-  if (Invalid && !Ptr.isDummy() && S.getLangOpts().CPlusPlus)
+  if (Invalid && S.getLangOpts().CPlusPlus)
 return false;
 
   // Offset is valid - compute it on unsigned.
@@ -2110,6 +2108,9 @@ inline bool ArrayDecay(InterpState &S, CodePtr OpPC) {
 return true;
   }
 
+  if (!CheckRange(S, OpPC, Ptr, CSK_ArrayToPointer))
+return false;
+
   if (!Ptr.isUnknownSizeArray() || Ptr.isDummy()) {
 S.Stk.push(Ptr.atIndex(0));
 return true;

diff  --git a/clang/lib/AST/Interp/Pointer.h b/clang/lib/AST/Interp/Pointer.h
index 79fab05670e96..9900f37e60d4e 100644
--- a/clang/lib/AST/Interp/Pointer.h
+++ b/clang/lib/AST/Interp/Pointer.h
@@ -384,11 +384,6 @@ class Pointer {
   bool isUnknownSizeArray() const {
 if (!isBlockPointer())
   return false;
-// If this points inside a dummy block, return true.
-// FIXME: This might change in the future. If it does, we need
-// to set the proper Ctor/Dtor functions for dummy Descriptors.
-if (!isRoot() && isDummy())
-  return true;
 return getFieldDesc()->isUnknownSizeArray();
   }
   /// Checks if the pointer points to an array.
@@ -560,8 +555,6 @@ class Pointer {
 
 if (!asBlockPointer().Pointee)
   return false;
-if (isDummy())
-  return false;
 
 return isElementPastEnd() || getSize() == getOffset();
   }

diff  --git a/clang/lib/AST/Interp/Program.cpp 
b/clang/lib/AST/Interp/Program.cpp
index 0b95db8492695..31a64e13d2b15 100644
--- a/clang/lib/AST/Interp/Program.cpp
+++ b/clang/lib/AST/Interp/Program.cpp
@@ -144,8 +144,12 @@ std::optional Program::getOrCreateDummy(const 
ValueDecl *VD) {
   if (auto It = DummyVariables.find(VD); It != DummyVariables.end())
 return It->second;
 
+  QualType QT = VD->getType();
+  if (const auto *RT = QT->getAs())
+QT = RT->getPointeeType();
+
   Descriptor *Desc;
-  if (std::optional T = Ctx.classify(VD->getType()))
+  if (std::optional T = Ctx.classify(QT))
 Desc = createDescriptor(VD, *T, std::nullopt, true, false);
   else
 Desc = createDescriptor(VD, VD->getType().getTypePtr(), std::nullopt, true,

diff  --git a/clang/test/AST/Interp/arrays.cpp 
b/clang/test/AST/Interp/arrays.cpp
index f6d265d4b3d10..929f25b95fa1f 100644
--- a/clang/test/AST/Interp/arrays.cpp
+++ b/clang/test/AST/Interp/arrays.cpp
@@ -580,3 +580,18 @@ constexpr ptr
diff _t d3 = &melchizedek[0] - &melchizedek[1]; // ok
 /// GH#88018
 const int SZA[] = {};
 void testZeroSizedArrayAccess() { unsigned c = SZA[4]; }
+
+#if __cplusplus >= 202002L
+constexpr int test_multiarray2() { // both-error {{never produces a constant 
expression}}
+  int multi2[2][1]; // both-note {{declared here}}
+  return multi2[2][0]; // both-note {{cannot access array element of pointer 
past the end of object}} \
+   // both-warning {{array index 2 is past the end of the 
array (that has type 'int[2][1]')}}
+}
+
+/// Same but with a dummy pointer.
+int multi22[2][2]; // both-note {{declared here}}
+int test_multiarray22() {
+  return multi22[2][0]; // both-warning {{array index 2 is past the end of the 
array (that has type 'int[2][2]')}}
+}
+
+#endif



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


[clang] clang-format: Allow open brace with trailing comment (no line break) (PR #89956)

2024-05-14 Thread via cfe-commits

GertyP wrote:

Ping

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


[clang] [Clang] Throw error when calling atomic with pointer to zero size object (PR #91057)

2024-05-14 Thread Hendrik Hübner via cfe-commits

https://github.com/HendrikHuebner updated 
https://github.com/llvm/llvm-project/pull/91057

From d587410660127d372fa35963dd26b97fd133d7c5 Mon Sep 17 00:00:00 2001
From: hhuebner 
Date: Sat, 4 May 2024 13:49:38 +0200
Subject: [PATCH] [Clang] Throw error when calling atomic with pointer to zero
 size object

---
 .../clang/Basic/DiagnosticFrontendKinds.td|  3 ++
 clang/lib/Sema/SemaChecking.cpp   | 11 ++-
 clang/test/Sema/atomic-ops.c  | 32 +++
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td 
b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index e456ec2cac461..74d96a8cbe0dc 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -330,6 +330,9 @@ def warn_atomic_op_misaligned : Warning<
   "; the expected alignment (%0 bytes) exceeds the actual alignment (%1 
bytes)">,
   InGroup;
 
+def err_atomic_op_size_zero : Error<
+  "First argument cannot be a pointer to an object of size zero">;
+
 def warn_atomic_op_oversized : Warning<
   "large atomic operation may incur "
   "significant performance penalty"
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ecd1821651140..51dcfe4cc67d9 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -40,6 +40,7 @@
 #include "clang/Basic/AddressSpaces.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
@@ -8497,7 +8498,8 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, 
SourceRange ExprRange,
 << 0 << AdjustedNumArgs << static_cast(Args.size())
 << /*is non object*/ 0 << ExprRange;
 return ExprError();
-  } else if (Args.size() > AdjustedNumArgs) {
+  }
+  if (Args.size() > AdjustedNumArgs) {
 Diag(Args[AdjustedNumArgs]->getBeginLoc(),
  diag::err_typecheck_call_too_many_args)
 << 0 << AdjustedNumArgs << static_cast(Args.size())
@@ -8544,6 +8546,13 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, 
SourceRange ExprRange,
 }
   }
 
+  // pointer to object of size zero is not allowed
+  if (Context.getTypeInfoInChars(AtomTy).Width.isZero()) {
+Diag(ExprRange.getBegin(), diag::err_atomic_op_size_zero)
+<< Ptr->getSourceRange();
+return ExprError();
+  }
+
   // For an arithmetic operation, the implied arithmetic must be well-formed.
   if (Form == Arithmetic) {
 // GCC does not enforce these rules for GNU atomics, but we do to help 
catch
diff --git a/clang/test/Sema/atomic-ops.c b/clang/test/Sema/atomic-ops.c
index 1d36667d6cf40..97da2582312bb 100644
--- a/clang/test/Sema/atomic-ops.c
+++ b/clang/test/Sema/atomic-ops.c
@@ -639,6 +639,38 @@ void memory_checks(_Atomic(int) *Ap, int *p, int val) {
   (void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst, -1); 
// expected-warning {{memory order argument to atomic operation is invalid}}
 }
 
+struct Z {
+  char z[];
+};
+
+void zeroSizeArgError(struct Z *a, struct Z *b, struct Z *c) {
+  __atomic_exchange(b, b, c, memory_order_relaxed); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_exchange(b, b, c, memory_order_acq_rel); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_exchange(b, b, c, memory_order_acquire); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_exchange(b, b, c, memory_order_consume); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_exchange(b, b, c, memory_order_release); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_exchange(b, b, c, memory_order_seq_cst); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_load(a, b, memory_order_relaxed); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_load(a, b, memory_order_acq_rel); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_load(a, b, memory_order_acquire); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_load(a, b, memory_order_consume); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_load(a, b, memory_order_release); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_load(a, b, memory_order_seq_cst); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_store(a, b, memory_order_relaxed); // expected-error {{First 
argument cannot be a pointer to an object of size zero}}
+  __atomic_s

[clang] [analyzer] Refactor recognition of the errno getter functions (PR #91531)

2024-05-14 Thread Donát Nagy via cfe-commits

https://github.com/NagyDonat updated 
https://github.com/llvm/llvm-project/pull/91531

From 07dc4dd5c60c8a04637cce686b379e195deb5b67 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= 
Date: Wed, 8 May 2024 20:01:57 +0200
Subject: [PATCH 1/6] [analyzer] Refactor recognition of the errno getter
 functions

There are many environments where `errno` is a macro that expands to
something like `(*__errno())` (different standard library
implementations use different names instead of "__errno").

In these environments the ErrnoModeling checker creates a symbolic
region which will be used to represent the return value of this "get the
location of errno" function.

Previously this symbol was only created when the checker was able to
find the declaration of the "get the location of errno" function; but
this commit eliminates the complex logic that was responsible for this
and always creates the symbolic region when `errno` is not available as
a "regular" global variable.

This significantly simplifies a code and only introduces a minimal
performance reduction (one extra symbol) in the unlikely case when
`errno` is not declared (neither as a variable nor as a function), but
the `ErrnoModeling` checker is enabled.

In addition to this simplification, this commit specifies that the
`CallDescription`s for the "get the location of errno" functions are
matched in `CDM::CLibrary` mode. (This was my original goal, but I was
sidetracked by resolving a FIXME above the `CallDescriptionSet` in
`ErrnoModeling.cpp`.)

This change is very close to being NFC, but it fixes weird corner
cases like the handling of a C++ method that happens to be named
"__errno()" (previously it could've been recognized as an errno
location getter function).
---
 .../StaticAnalyzer/Checkers/ErrnoChecker.cpp  |   2 +-
 .../StaticAnalyzer/Checkers/ErrnoModeling.cpp | 127 ++
 .../StaticAnalyzer/Checkers/ErrnoModeling.h   |   9 +-
 clang/test/Analysis/memory-model.cpp  |  18 +--
 4 files changed, 53 insertions(+), 103 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
index 18e718e085536..72fd6781a7561 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
@@ -205,7 +205,7 @@ void ErrnoChecker::checkPreCall(const CallEvent &Call,
   // Probably 'strerror'?
   if (CallF->isExternC() && CallF->isGlobal() &&
   C.getSourceManager().isInSystemHeader(CallF->getLocation()) &&
-  !isErrno(CallF)) {
+  !isErrnoLocationCall(Call)) {
 if (getErrnoState(C.getState()) == MustBeChecked) {
   std::optional ErrnoLoc = getErrnoLoc(C.getState());
   assert(ErrnoLoc && "ErrnoLoc should exist if an errno state is set.");
diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
index 1b34ea0e056e5..0612cd4c87248 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -39,10 +39,15 @@ namespace {
 // Name of the "errno" variable.
 // FIXME: Is there a system where it is not called "errno" but is a variable?
 const char *ErrnoVarName = "errno";
+
 // Names of functions that return a location of the "errno" value.
 // FIXME: Are there other similar function names?
-const char *ErrnoLocationFuncNames[] = {"__errno_location", "___errno",
-"__errno", "_errno", "__error"};
+CallDescriptionSet ErrnoLocationCalls{
+{CDM::SimpleFunc, {"__errno_location"}, 0, 0},
+{CDM::SimpleFunc, {"___errno"}, 0, 0},
+{CDM::SimpleFunc, {"__errno"}, 0, 0},
+{CDM::SimpleFunc, {"_errno"}, 0, 0},
+{CDM::SimpleFunc, {"__error"}, 0, 0}};
 
 class ErrnoModeling
 : public Checker, check::BeginFunction,
@@ -54,16 +59,10 @@ class ErrnoModeling
   void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
 
-  // The declaration of an "errno" variable or "errno location" function.
-  mutable const Decl *ErrnoDecl = nullptr;
-
 private:
-  // FIXME: Names from `ErrnoLocationFuncNames` are used to build this set.
-  CallDescriptionSet ErrnoLocationCalls{{{"__errno_location"}, 0, 0},
-{{"___errno"}, 0, 0},
-{{"__errno"}, 0, 0},
-{{"_errno"}, 0, 0},
-{{"__error"}, 0, 0}};
+  // The declaration of an "errno" variable on systems where errno is
+  // represented by a variable (and not a function that queries its location).
+  mutable const Decl *ErrnoDecl = nullptr;
 };
 
 } // namespace
@@ -74,9 +73,13 @@ REGISTER_TRAIT_WITH_PROGRAMSTATE(ErrnoRegion, const 
MemRegion *)
 
 REGISTER_TRAIT_WITH_PROGRAMSTATE(ErrnoState, errno_modeling::ErrnoCheckState)
 
-/// Search for a variable called "errno

[clang] [analyzer] Clean up list of taint propagation functions (PR #91635)

2024-05-14 Thread Balazs Benics via cfe-commits

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


[clang] [analyzer] Clean up list of taint propagation functions (PR #91635)

2024-05-14 Thread Balazs Benics via cfe-commits

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

LGTM.
I haven't checked absolutely everything, rather sampled.
It looked correct, and free of copy-paste mistakes.

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


[clang] [analyzer] Clean up list of taint propagation functions (PR #91635)

2024-05-14 Thread Balazs Benics via cfe-commits


@@ -400,17 +400,14 @@ class GenericTaintChecker : public 
Checker {
   void taintUnsafeSocketProtocol(const CallEvent &Call,
  CheckerContext &C) const;
 
-  /// Default taint rules are initalized with the help of a CheckerContext to
-  /// access the names of built-in functions like memcpy.
+  /// The taint rules are initalized with the help of a CheckerContext to
+  /// access user-provided configuration.
   void initTaintRules(CheckerContext &C) const;
 
-  /// CallDescription currently cannot restrict matches to the global namespace
-  /// only, which is why multiple CallDescriptionMaps are used, as we want to
-  /// disambiguate global C functions from functions inside user-defined
-  /// namespaces.
-  // TODO: Remove separation to simplify matching logic once CallDescriptions
-  // are more expressive.
-
+  // TODO: The two separate `CallDescriptionMap`s were introduced when
+  // `CallDescription` was unable to restric matches to the global namespace

steakhal wrote:

```suggestion
  // `CallDescription` was unable to restrict matches to the global namespace
```

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


[clang] [analyzer] Clean up list of taint propagation functions (PR #91635)

2024-05-14 Thread Balazs Benics via cfe-commits


@@ -572,196 +570,236 @@ void GenericTaintChecker::initTaintRules(CheckerContext 
&C) const {
   std::vector>;
   using TR = GenericTaintRule;
 
-  const Builtin::Context &BI = C.getASTContext().BuiltinInfo;
-
   RulesConstructionTy GlobalCRules{
   // Sources
-  {{{"fdopen"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"fopen"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"freopen"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getch"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getchar"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getchar_unlocked"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"gets"}}, TR::Source({{0}, ReturnValueIndex})},
-  {{{"gets_s"}}, TR::Source({{0}, ReturnValueIndex})},
-  {{{"scanf"}}, TR::Source({{}, 1})},
-  {{{"scanf_s"}}, TR::Source({{}, {1}})},
-  {{{"wgetch"}}, TR::Source({{}, ReturnValueIndex})},
+  {{CDM::CLibrary, {"fdopen"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"fopen"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"freopen"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getch"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getchar"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getchar_unlocked"}}, 
TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"gets"}}, TR::Source({{0, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"gets_s"}}, TR::Source({{0, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"scanf"}}, TR::Source({{}, 1})},
+  {{CDM::CLibrary, {"scanf_s"}}, TR::Source({{}, 1})},
+  {{CDM::CLibrary, {"wgetch"}}, TR::Source({{ReturnValueIndex}})},
   // Sometimes the line between taint sources and propagators is blurry.
   // _IO_getc is choosen to be a source, but could also be a propagator.
   // This way it is simpler, as modeling it as a propagator would require
   // to model the possible sources of _IO_FILE * values, which the _IO_getc
   // function takes as parameters.
-  {{{"_IO_getc"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getcwd"}}, TR::Source({{0, ReturnValueIndex}})},
-  {{{"getwd"}}, TR::Source({{0, ReturnValueIndex}})},
-  {{{"readlink"}}, TR::Source({{1, ReturnValueIndex}})},
-  {{{"readlinkat"}}, TR::Source({{2, ReturnValueIndex}})},
-  {{{"get_current_dir_name"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"gethostname"}}, TR::Source({{0}})},
-  {{{"getnameinfo"}}, TR::Source({{2, 4}})},
-  {{{"getseuserbyname"}}, TR::Source({{1, 2}})},
-  {{{"getgroups"}}, TR::Source({{1, ReturnValueIndex}})},
-  {{{"getlogin"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getlogin_r"}}, TR::Source({{0}})},
+  {{CDM::CLibrary, {"_IO_getc"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getcwd"}}, TR::Source({{0, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getwd"}}, TR::Source({{0, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"readlink"}}, TR::Source({{1, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"readlinkat"}}, TR::Source({{2, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"get_current_dir_name"}},
+   TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"gethostname"}}, TR::Source({{0}})},
+  {{CDM::CLibrary, {"getnameinfo"}}, TR::Source({{2, 4}})},
+  {{CDM::CLibrary, {"getseuserbyname"}}, TR::Source({{1, 2}})},
+  {{CDM::CLibrary, {"getgroups"}}, TR::Source({{1, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getlogin"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getlogin_r"}}, TR::Source({{0}})},
 
   // Props
-  {{{"accept"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"atoi"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"atol"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"atoll"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"fgetc"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"fgetln"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"fgets"}}, TR::Prop({{2}}, {{0, ReturnValueIndex}})},
-  {{{"fgetws"}}, TR::Prop({{2}}, {{0, ReturnValueIndex}})},
-  {{{"fscanf"}}, TR::Prop({{0}}, {{}, 2})},
-  {{{"fscanf_s"}}, TR::Prop({{0}}, {{}, {2}})},
-  {{{"sscanf"}}, TR::Prop({{0}}, {{}, 2})},
-
-  {{{"getc"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"getc_unlocked"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"getdelim"}}, TR::Prop({{3}}, {{0}})},
-  {{{"getline"}}, TR::Prop({{2}}, {{0}})},
-  {{{"getw"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"pread"}}, TR::Prop({{0, 1, 2, 3}}, {{1, ReturnValueIndex}})},
-  {{{"read"}}, TR::Prop({{0, 2}}, {{1, ReturnValueIndex}})},
-  {{{"strchr"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"strrchr"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"tolower"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"toupper"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"fread"}}, TR::Prop({{3}}, {{

[clang] [analyzer] Refactor recognition of the errno getter functions (PR #91531)

2024-05-14 Thread Balazs Benics via cfe-commits
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy ,
=?utf-8?q?Don=C3=A1t?= Nagy 
Message-ID:
In-Reply-To: 


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


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


[clang] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)

2024-05-14 Thread via cfe-commits


@@ -40,10 +38,8 @@ exit:
 
 ; Verify that in the resume part resume call is marked with musttail.
 ; CHECK-LABEL: @f.resume(
-; CHECK: %[[addr2:.+]] = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-; NOPGO-NEXT: musttail call fastcc void %[[addr2]](ptr null)
-; PGO: call void @llvm.instrprof

zmodem wrote:

I looked into this, and I don't think my patch changes the situation for value 
profiling. PGO instrumentation is performed early. Before my patch it would see 
a call to `@llvm.coro.resume`, and now it will see 
`@llvm.coro.await.suspend.handle`. In neither case does it see the lowered 
indirect call to the resumed function, and so no 
`@llvm.instrprof.value.profile` call gets inserted. Supporting that would be a 
nice future improvement.

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


[clang] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)

2024-05-14 Thread via cfe-commits

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


[clang] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)

2024-05-14 Thread via cfe-commits

zmodem wrote:

I think my patch is a significant improvement, both in terms of simplicity and 
reliability of the codegen for symmetric transfer, and would like to move 
forward. @ChuanqiXu9 @mtrofin do you have any further comments?

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


[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)

2024-05-14 Thread Mital Ashok via cfe-commits

https://github.com/MitalAshok created 
https://github.com/llvm/llvm-project/pull/92103

Also changes the behaviour of `__builtin_is_layout_compatible`

None of the historic nor the current definition of layout-compatible classes 
mention anything about base classes (other than implicitly through being 
standard-layout) and are defined in terms of members, not direct members.


>From 147f0a7bb7644bd16584f89e02edbdf4abd81855 Mon Sep 17 00:00:00 2001
From: Mital Ashok 
Date: Tue, 14 May 2024 08:28:19 +0100
Subject: [PATCH] [Clang] Fix definition of layout-compatible to ignore empty
 classes

Also changes the behaviour of __builtin_is_layout_compatible

None of the historic nor the current definition of layout-compatible classes 
mention anything about base classes (other than implicitly through being 
standard-layout) and are defined in terms of members, not direct members.
---
 clang/include/clang/AST/DeclCXX.h  |  7 
 clang/lib/AST/DeclCXX.cpp  | 36 +
 clang/lib/Sema/SemaChecking.cpp| 63 +-
 clang/test/SemaCXX/type-traits.cpp | 11 ++
 llvm/include/llvm/ADT/STLExtras.h  |  6 +++
 5 files changed, 79 insertions(+), 44 deletions(-)

diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index fb52ac804849d..f29592e1e7520 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1229,6 +1229,13 @@ class CXXRecordDecl : public RecordDecl {
   /// C++11 [class]p7, specifically using the C++11 rules without any DRs.
   bool isCXX11StandardLayout() const { return data().IsCXX11StandardLayout; }
 
+  /// If this is a standard-layout class or union per C++11 rules,
+  /// any and all data members will be declared in the same type.
+  ///
+  /// This retrieves the type if this class has any data members,
+  /// or the current class if there is no class with fields.
+  const CXXRecordDecl *getStandardLayoutBaseWithFields() const;
+
   /// Determine whether this class, or any of its class subobjects,
   /// contains a mutable field.
   bool hasMutableFields() const { return data().HasMutableFields; }
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 75c441293d62e..a101b61ad7392 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -561,6 +561,42 @@ void CXXRecordDecl::addedClassSubobject(CXXRecordDecl 
*Subobj) {
 data().StructuralIfLiteral = false;
 }
 
+const CXXRecordDecl *CXXRecordDecl::getStandardLayoutBaseWithFields() const {
+#ifndef NDEBUG
+  {
+assert(
+isCXX11StandardLayout() &&
+"getStandardLayoutBaseWithFields called on a non-standard-layout 
type");
+unsigned NumberOfBasesWithFields = 0;
+if (!field_empty())
+  ++NumberOfBasesWithFields;
+std::set UniqueBases;
+forallBases([&](const CXXRecordDecl *Base) -> bool {
+  if (!Base->field_empty())
+++NumberOfBasesWithFields;
+  assert(
+  UniqueBases.insert(Base->getCanonicalDecl()).second &&
+  "Standard layout struct has multiple base classes of the same type");
+  return true;
+});
+assert(NumberOfBasesWithFields <= 1 &&
+   "Standard layout struct has fields declared in more than one 
class");
+  }
+#endif
+  if (!field_empty())
+return this;
+  const CXXRecordDecl *Result = this;
+  forallBases([&](const CXXRecordDecl *Base) -> bool {
+if (!Base->field_empty()) {
+  // This is the base where the fields are declared; return early
+  Result = Base;
+  return false;
+}
+return true;
+  });
+  return Result;
+}
+
 bool CXXRecordDecl::hasConstexprDestructor() const {
   auto *Dtor = getDestructor();
   return Dtor ? Dtor->isConstexpr() : defaultedDestructorIsConstexpr();
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ecd1821651140..acd142c1932ad 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -19084,7 +19084,8 @@ void Sema::DiagnoseSelfMove(const Expr *LHSExpr, const 
Expr *RHSExpr,
 static bool isLayoutCompatible(ASTContext &C, QualType T1, QualType T2);
 
 /// Check if two enumeration types are layout-compatible.
-static bool isLayoutCompatible(ASTContext &C, EnumDecl *ED1, EnumDecl *ED2) {
+static bool isLayoutCompatible(ASTContext &C, const EnumDecl *ED1,
+   const EnumDecl *ED2) {
   // C++11 [dcl.enum] p8:
   // Two enumeration types are layout-compatible if they have the same
   // underlying type.
@@ -19095,8 +19096,8 @@ static bool isLayoutCompatible(ASTContext &C, EnumDecl 
*ED1, EnumDecl *ED2) {
 /// Check if two fields are layout-compatible.
 /// Can be used on union members, which are exempt from alignment requirement
 /// of common initial sequence.
-static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
-   FieldDecl *Field2,
+static bool isLayoutCompatible(ASTContext &C, const FieldDecl *Field1,
+   

[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)

2024-05-14 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Mital Ashok (MitalAshok)


Changes

Also changes the behaviour of `__builtin_is_layout_compatible`

None of the historic nor the current definition of layout-compatible classes 
mention anything about base classes (other than implicitly through being 
standard-layout) and are defined in terms of members, not direct members.


---
Full diff: https://github.com/llvm/llvm-project/pull/92103.diff


5 Files Affected:

- (modified) clang/include/clang/AST/DeclCXX.h (+7) 
- (modified) clang/lib/AST/DeclCXX.cpp (+36) 
- (modified) clang/lib/Sema/SemaChecking.cpp (+19-44) 
- (modified) clang/test/SemaCXX/type-traits.cpp (+11) 
- (modified) llvm/include/llvm/ADT/STLExtras.h (+6) 


``diff
diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index fb52ac804849d..f29592e1e7520 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1229,6 +1229,13 @@ class CXXRecordDecl : public RecordDecl {
   /// C++11 [class]p7, specifically using the C++11 rules without any DRs.
   bool isCXX11StandardLayout() const { return data().IsCXX11StandardLayout; }
 
+  /// If this is a standard-layout class or union per C++11 rules,
+  /// any and all data members will be declared in the same type.
+  ///
+  /// This retrieves the type if this class has any data members,
+  /// or the current class if there is no class with fields.
+  const CXXRecordDecl *getStandardLayoutBaseWithFields() const;
+
   /// Determine whether this class, or any of its class subobjects,
   /// contains a mutable field.
   bool hasMutableFields() const { return data().HasMutableFields; }
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 75c441293d62e..a101b61ad7392 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -561,6 +561,42 @@ void CXXRecordDecl::addedClassSubobject(CXXRecordDecl 
*Subobj) {
 data().StructuralIfLiteral = false;
 }
 
+const CXXRecordDecl *CXXRecordDecl::getStandardLayoutBaseWithFields() const {
+#ifndef NDEBUG
+  {
+assert(
+isCXX11StandardLayout() &&
+"getStandardLayoutBaseWithFields called on a non-standard-layout 
type");
+unsigned NumberOfBasesWithFields = 0;
+if (!field_empty())
+  ++NumberOfBasesWithFields;
+std::set UniqueBases;
+forallBases([&](const CXXRecordDecl *Base) -> bool {
+  if (!Base->field_empty())
+++NumberOfBasesWithFields;
+  assert(
+  UniqueBases.insert(Base->getCanonicalDecl()).second &&
+  "Standard layout struct has multiple base classes of the same type");
+  return true;
+});
+assert(NumberOfBasesWithFields <= 1 &&
+   "Standard layout struct has fields declared in more than one 
class");
+  }
+#endif
+  if (!field_empty())
+return this;
+  const CXXRecordDecl *Result = this;
+  forallBases([&](const CXXRecordDecl *Base) -> bool {
+if (!Base->field_empty()) {
+  // This is the base where the fields are declared; return early
+  Result = Base;
+  return false;
+}
+return true;
+  });
+  return Result;
+}
+
 bool CXXRecordDecl::hasConstexprDestructor() const {
   auto *Dtor = getDestructor();
   return Dtor ? Dtor->isConstexpr() : defaultedDestructorIsConstexpr();
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ecd1821651140..acd142c1932ad 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -19084,7 +19084,8 @@ void Sema::DiagnoseSelfMove(const Expr *LHSExpr, const 
Expr *RHSExpr,
 static bool isLayoutCompatible(ASTContext &C, QualType T1, QualType T2);
 
 /// Check if two enumeration types are layout-compatible.
-static bool isLayoutCompatible(ASTContext &C, EnumDecl *ED1, EnumDecl *ED2) {
+static bool isLayoutCompatible(ASTContext &C, const EnumDecl *ED1,
+   const EnumDecl *ED2) {
   // C++11 [dcl.enum] p8:
   // Two enumeration types are layout-compatible if they have the same
   // underlying type.
@@ -19095,8 +19096,8 @@ static bool isLayoutCompatible(ASTContext &C, EnumDecl 
*ED1, EnumDecl *ED2) {
 /// Check if two fields are layout-compatible.
 /// Can be used on union members, which are exempt from alignment requirement
 /// of common initial sequence.
-static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
-   FieldDecl *Field2,
+static bool isLayoutCompatible(ASTContext &C, const FieldDecl *Field1,
+   const FieldDecl *Field2,
bool AreUnionMembers = false) {
   [[maybe_unused]] const Type *Field1Parent =
   Field1->getParent()->getTypeForDecl();
@@ -19139,52 +19140,26 @@ static bool isLayoutCompatible(ASTContext &C, 
FieldDecl *Field1,
 
 /// Check if two standard-layout structs are layout-compatible.
 /// (C++11 [class.mem] p17)
-static bool isLayoutCompatibleStruct(ASTContext &C, RecordDecl *RD1,
-   

[clang] [clang-tools-extra] [Clang] Retain the angle loci for invented template parameters of constraints (PR #92104)

2024-05-14 Thread Younan Zhang via cfe-commits

https://github.com/zyn0217 created 
https://github.com/llvm/llvm-project/pull/92104

Clangd uses it to determine whether the argument is within the selection range.

Fixes https://github.com/clangd/clangd/issues/2033

>From d9c0121067162ffb68bc70b9a0a3f10e0b05674c Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 14 May 2024 19:46:20 +0800
Subject: [PATCH] [Clang] Retain the angle loci for invented template
 parameters of constraints

Clangd uses it to determine whether the argument is within the selection range.

Fixes https://github.com/clangd/clangd/issues/2033
---
 .../clangd/unittests/SelectionTests.cpp |  5 +
 clang/lib/Sema/SemaType.cpp |  3 ++-
 clang/test/AST/ast-dump-concepts.cpp| 13 +
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index db516a1f62a35..706286e22cf74 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -589,6 +589,11 @@ TEST(SelectionTest, CommonAncestor) {
 auto x = [[ns::^C]];
   )cpp",
"ConceptReference"},
+  {R"cpp(
+template 
+concept D = true;
+template  void g(D<[[^T]]> auto abc) {}
+  )cpp", "TemplateTypeParmTypeLoc"},
   };
 
   for (const Case &C : Cases) {
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index fddc3545ecb61..7eb7cda63a61f 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -3516,7 +3516,8 @@ InventTemplateParameter(TypeProcessingState &state, 
QualType T,
   // The 'auto' appears in the decl-specifiers; we've not finished forming
   // TypeSourceInfo for it yet.
   TemplateIdAnnotation *TemplateId = D.getDeclSpec().getRepAsTemplateId();
-  TemplateArgumentListInfo TemplateArgsInfo;
+  TemplateArgumentListInfo TemplateArgsInfo(TemplateId->LAngleLoc,
+TemplateId->RAngleLoc);
   bool Invalid = false;
   if (TemplateId->LAngleLoc.isValid()) {
 ASTTemplateArgsPtr TemplateArgsPtr(TemplateId->getTemplateArgs(),
diff --git a/clang/test/AST/ast-dump-concepts.cpp 
b/clang/test/AST/ast-dump-concepts.cpp
index 5bb174e3548ed..a5e0673c241ef 100644
--- a/clang/test/AST/ast-dump-concepts.cpp
+++ b/clang/test/AST/ast-dump-concepts.cpp
@@ -107,3 +107,16 @@ auto FooFunc(C auto V) -> C decltype(auto) {
 }
 
 }
+
+namespace constraint_auto_params {
+
+template 
+concept C = true;
+
+template
+void g(C auto Foo) {}
+
+// CHECK: TemplateTypeParmDecl {{.*}} depth 0 index 1 Foo:auto
+// CHECK-NEXT: `-ConceptSpecializationExpr {{.*}} 
+
+}

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


[clang] [llvm] [Clang] Fix definition of layout-compatible to ignore empty classes (PR #92103)

2024-05-14 Thread Mital Ashok via cfe-commits

https://github.com/MitalAshok updated 
https://github.com/llvm/llvm-project/pull/92103

>From 74e133215e7ba9049fb021eb9bbb130347496503 Mon Sep 17 00:00:00 2001
From: Mital Ashok 
Date: Tue, 14 May 2024 08:28:19 +0100
Subject: [PATCH] [Clang] Fix definition of layout-compatible to ignore empty
 classes

Also changes the behaviour of __builtin_is_layout_compatible

None of the historic nor the current definition of layout-compatible classes 
mention anything about base classes (other than implicitly through being 
standard-layout) and are defined in terms of members, not direct members.
---
 clang/include/clang/AST/DeclCXX.h  |  7 
 clang/lib/AST/DeclCXX.cpp  | 36 +
 clang/lib/Sema/SemaChecking.cpp| 63 +-
 clang/test/SemaCXX/type-traits.cpp | 11 ++
 llvm/include/llvm/ADT/STLExtras.h  |  6 +++
 5 files changed, 79 insertions(+), 44 deletions(-)

diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index fb52ac804849d..f29592e1e7520 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1229,6 +1229,13 @@ class CXXRecordDecl : public RecordDecl {
   /// C++11 [class]p7, specifically using the C++11 rules without any DRs.
   bool isCXX11StandardLayout() const { return data().IsCXX11StandardLayout; }
 
+  /// If this is a standard-layout class or union per C++11 rules,
+  /// any and all data members will be declared in the same type.
+  ///
+  /// This retrieves the type if this class has any data members,
+  /// or the current class if there is no class with fields.
+  const CXXRecordDecl *getStandardLayoutBaseWithFields() const;
+
   /// Determine whether this class, or any of its class subobjects,
   /// contains a mutable field.
   bool hasMutableFields() const { return data().HasMutableFields; }
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 75c441293d62e..a101b61ad7392 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -561,6 +561,42 @@ void CXXRecordDecl::addedClassSubobject(CXXRecordDecl 
*Subobj) {
 data().StructuralIfLiteral = false;
 }
 
+const CXXRecordDecl *CXXRecordDecl::getStandardLayoutBaseWithFields() const {
+#ifndef NDEBUG
+  {
+assert(
+isCXX11StandardLayout() &&
+"getStandardLayoutBaseWithFields called on a non-standard-layout 
type");
+unsigned NumberOfBasesWithFields = 0;
+if (!field_empty())
+  ++NumberOfBasesWithFields;
+std::set UniqueBases;
+forallBases([&](const CXXRecordDecl *Base) -> bool {
+  if (!Base->field_empty())
+++NumberOfBasesWithFields;
+  assert(
+  UniqueBases.insert(Base->getCanonicalDecl()).second &&
+  "Standard layout struct has multiple base classes of the same type");
+  return true;
+});
+assert(NumberOfBasesWithFields <= 1 &&
+   "Standard layout struct has fields declared in more than one 
class");
+  }
+#endif
+  if (!field_empty())
+return this;
+  const CXXRecordDecl *Result = this;
+  forallBases([&](const CXXRecordDecl *Base) -> bool {
+if (!Base->field_empty()) {
+  // This is the base where the fields are declared; return early
+  Result = Base;
+  return false;
+}
+return true;
+  });
+  return Result;
+}
+
 bool CXXRecordDecl::hasConstexprDestructor() const {
   auto *Dtor = getDestructor();
   return Dtor ? Dtor->isConstexpr() : defaultedDestructorIsConstexpr();
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ecd1821651140..acd142c1932ad 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -19084,7 +19084,8 @@ void Sema::DiagnoseSelfMove(const Expr *LHSExpr, const 
Expr *RHSExpr,
 static bool isLayoutCompatible(ASTContext &C, QualType T1, QualType T2);
 
 /// Check if two enumeration types are layout-compatible.
-static bool isLayoutCompatible(ASTContext &C, EnumDecl *ED1, EnumDecl *ED2) {
+static bool isLayoutCompatible(ASTContext &C, const EnumDecl *ED1,
+   const EnumDecl *ED2) {
   // C++11 [dcl.enum] p8:
   // Two enumeration types are layout-compatible if they have the same
   // underlying type.
@@ -19095,8 +19096,8 @@ static bool isLayoutCompatible(ASTContext &C, EnumDecl 
*ED1, EnumDecl *ED2) {
 /// Check if two fields are layout-compatible.
 /// Can be used on union members, which are exempt from alignment requirement
 /// of common initial sequence.
-static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
-   FieldDecl *Field2,
+static bool isLayoutCompatible(ASTContext &C, const FieldDecl *Field1,
+   const FieldDecl *Field2,
bool AreUnionMembers = false) {
   [[maybe_unused]] const Type *Field1Parent =
   Field1->getParent()->getTypeForDecl();
@@ -19139,52 +19140,26 @@ static bool isLayoutCompatible(ASTContext &C, 
FieldDecl *Field1,
 
 /// Che

[clang] [clang-tools-extra] [Clang] Retain the angle loci for invented template parameters of constraints (PR #92104)

2024-05-14 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clangd

Author: Younan Zhang (zyn0217)


Changes

Clangd uses it to determine whether the argument is within the selection range.

Fixes https://github.com/clangd/clangd/issues/2033

---
Full diff: https://github.com/llvm/llvm-project/pull/92104.diff


3 Files Affected:

- (modified) clang-tools-extra/clangd/unittests/SelectionTests.cpp (+5) 
- (modified) clang/lib/Sema/SemaType.cpp (+2-1) 
- (modified) clang/test/AST/ast-dump-concepts.cpp (+13) 


``diff
diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index db516a1f62a35..706286e22cf74 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -589,6 +589,11 @@ TEST(SelectionTest, CommonAncestor) {
 auto x = [[ns::^C]];
   )cpp",
"ConceptReference"},
+  {R"cpp(
+template 
+concept D = true;
+template  void g(D<[[^T]]> auto abc) {}
+  )cpp", "TemplateTypeParmTypeLoc"},
   };
 
   for (const Case &C : Cases) {
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index fddc3545ecb61..7eb7cda63a61f 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -3516,7 +3516,8 @@ InventTemplateParameter(TypeProcessingState &state, 
QualType T,
   // The 'auto' appears in the decl-specifiers; we've not finished forming
   // TypeSourceInfo for it yet.
   TemplateIdAnnotation *TemplateId = D.getDeclSpec().getRepAsTemplateId();
-  TemplateArgumentListInfo TemplateArgsInfo;
+  TemplateArgumentListInfo TemplateArgsInfo(TemplateId->LAngleLoc,
+TemplateId->RAngleLoc);
   bool Invalid = false;
   if (TemplateId->LAngleLoc.isValid()) {
 ASTTemplateArgsPtr TemplateArgsPtr(TemplateId->getTemplateArgs(),
diff --git a/clang/test/AST/ast-dump-concepts.cpp 
b/clang/test/AST/ast-dump-concepts.cpp
index 5bb174e3548ed..a5e0673c241ef 100644
--- a/clang/test/AST/ast-dump-concepts.cpp
+++ b/clang/test/AST/ast-dump-concepts.cpp
@@ -107,3 +107,16 @@ auto FooFunc(C auto V) -> C decltype(auto) {
 }
 
 }
+
+namespace constraint_auto_params {
+
+template 
+concept C = true;
+
+template
+void g(C auto Foo) {}
+
+// CHECK: TemplateTypeParmDecl {{.*}} depth 0 index 1 Foo:auto
+// CHECK-NEXT: `-ConceptSpecializationExpr {{.*}} 
+
+}

``




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


[clang] [clang-tools-extra] [Clang] Retain the angle loci for invented template parameters of constraints (PR #92104)

2024-05-14 Thread Younan Zhang via cfe-commits

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/92104

>From d9c0121067162ffb68bc70b9a0a3f10e0b05674c Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 14 May 2024 19:46:20 +0800
Subject: [PATCH 1/2] [Clang] Retain the angle loci for invented template
 parameters of constraints

Clangd uses it to determine whether the argument is within the selection range.

Fixes https://github.com/clangd/clangd/issues/2033
---
 .../clangd/unittests/SelectionTests.cpp |  5 +
 clang/lib/Sema/SemaType.cpp |  3 ++-
 clang/test/AST/ast-dump-concepts.cpp| 13 +
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index db516a1f62a35..706286e22cf74 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -589,6 +589,11 @@ TEST(SelectionTest, CommonAncestor) {
 auto x = [[ns::^C]];
   )cpp",
"ConceptReference"},
+  {R"cpp(
+template 
+concept D = true;
+template  void g(D<[[^T]]> auto abc) {}
+  )cpp", "TemplateTypeParmTypeLoc"},
   };
 
   for (const Case &C : Cases) {
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index fddc3545ecb61..7eb7cda63a61f 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -3516,7 +3516,8 @@ InventTemplateParameter(TypeProcessingState &state, 
QualType T,
   // The 'auto' appears in the decl-specifiers; we've not finished forming
   // TypeSourceInfo for it yet.
   TemplateIdAnnotation *TemplateId = D.getDeclSpec().getRepAsTemplateId();
-  TemplateArgumentListInfo TemplateArgsInfo;
+  TemplateArgumentListInfo TemplateArgsInfo(TemplateId->LAngleLoc,
+TemplateId->RAngleLoc);
   bool Invalid = false;
   if (TemplateId->LAngleLoc.isValid()) {
 ASTTemplateArgsPtr TemplateArgsPtr(TemplateId->getTemplateArgs(),
diff --git a/clang/test/AST/ast-dump-concepts.cpp 
b/clang/test/AST/ast-dump-concepts.cpp
index 5bb174e3548ed..a5e0673c241ef 100644
--- a/clang/test/AST/ast-dump-concepts.cpp
+++ b/clang/test/AST/ast-dump-concepts.cpp
@@ -107,3 +107,16 @@ auto FooFunc(C auto V) -> C decltype(auto) {
 }
 
 }
+
+namespace constraint_auto_params {
+
+template 
+concept C = true;
+
+template
+void g(C auto Foo) {}
+
+// CHECK: TemplateTypeParmDecl {{.*}} depth 0 index 1 Foo:auto
+// CHECK-NEXT: `-ConceptSpecializationExpr {{.*}} 
+
+}

>From 15a320c21ad25676cb75a1b622405c440d93c313 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 14 May 2024 19:56:19 +0800
Subject: [PATCH 2/2] Run clang-format

---
 clang-tools-extra/clangd/unittests/SelectionTests.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 706286e22cf74..aaaf758e72236 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -593,7 +593,8 @@ TEST(SelectionTest, CommonAncestor) {
 template 
 concept D = true;
 template  void g(D<[[^T]]> auto abc) {}
-  )cpp", "TemplateTypeParmTypeLoc"},
+  )cpp",
+   "TemplateTypeParmTypeLoc"},
   };
 
   for (const Case &C : Cases) {

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


[clang] [clang-tools-extra] [Clang] Retain the angle loci for invented template parameters of constraints (PR #92104)

2024-05-14 Thread via cfe-commits

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 31b45a9d0d91cc3a78446ee379abc6f2a365 
d9c0121067162ffb68bc70b9a0a3f10e0b05674c -- 
clang-tools-extra/clangd/unittests/SelectionTests.cpp 
clang/lib/Sema/SemaType.cpp clang/test/AST/ast-dump-concepts.cpp
``





View the diff from clang-format here.


``diff
diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 706286e22c..aaaf758e72 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -593,7 +593,8 @@ TEST(SelectionTest, CommonAncestor) {
 template 
 concept D = true;
 template  void g(D<[[^T]]> auto abc) {}
-  )cpp", "TemplateTypeParmTypeLoc"},
+  )cpp",
+   "TemplateTypeParmTypeLoc"},
   };
 
   for (const Case &C : Cases) {

``




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


[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

2024-05-14 Thread Aaron Ballman via cfe-commits


@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance &CI) {
   StringRef Action("unknown");
   (void)Action;
 
+  auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;

AaronBallman wrote:

> Nit: technically the coding standard does not say that, I believe you're 
> mentioning a sufficient condition, not a necessary one, see 
> https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
> 
> > Use auto if and only if it makes the code more readable or easier to 
> > maintain.

Yup. FWIW, the rule of thumb we use in Clang is that "readable" means "type is 
spelled out in the initialization somewhere or is otherwise painful to spell 
but is contextually quite obvious (e.g., use of iterators)" + "any time the 
code reviewer asks to switch away from `auto`"

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


[clang] [analyzer] Clean up list of taint propagation functions (PR #91635)

2024-05-14 Thread Donát Nagy via cfe-commits


@@ -572,196 +570,236 @@ void GenericTaintChecker::initTaintRules(CheckerContext 
&C) const {
   std::vector>;
   using TR = GenericTaintRule;
 
-  const Builtin::Context &BI = C.getASTContext().BuiltinInfo;
-
   RulesConstructionTy GlobalCRules{
   // Sources
-  {{{"fdopen"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"fopen"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"freopen"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getch"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getchar"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getchar_unlocked"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"gets"}}, TR::Source({{0}, ReturnValueIndex})},
-  {{{"gets_s"}}, TR::Source({{0}, ReturnValueIndex})},
-  {{{"scanf"}}, TR::Source({{}, 1})},
-  {{{"scanf_s"}}, TR::Source({{}, {1}})},
-  {{{"wgetch"}}, TR::Source({{}, ReturnValueIndex})},
+  {{CDM::CLibrary, {"fdopen"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"fopen"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"freopen"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getch"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getchar"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getchar_unlocked"}}, 
TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"gets"}}, TR::Source({{0, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"gets_s"}}, TR::Source({{0, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"scanf"}}, TR::Source({{}, 1})},
+  {{CDM::CLibrary, {"scanf_s"}}, TR::Source({{}, 1})},
+  {{CDM::CLibrary, {"wgetch"}}, TR::Source({{ReturnValueIndex}})},
   // Sometimes the line between taint sources and propagators is blurry.
   // _IO_getc is choosen to be a source, but could also be a propagator.
   // This way it is simpler, as modeling it as a propagator would require
   // to model the possible sources of _IO_FILE * values, which the _IO_getc
   // function takes as parameters.
-  {{{"_IO_getc"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getcwd"}}, TR::Source({{0, ReturnValueIndex}})},
-  {{{"getwd"}}, TR::Source({{0, ReturnValueIndex}})},
-  {{{"readlink"}}, TR::Source({{1, ReturnValueIndex}})},
-  {{{"readlinkat"}}, TR::Source({{2, ReturnValueIndex}})},
-  {{{"get_current_dir_name"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"gethostname"}}, TR::Source({{0}})},
-  {{{"getnameinfo"}}, TR::Source({{2, 4}})},
-  {{{"getseuserbyname"}}, TR::Source({{1, 2}})},
-  {{{"getgroups"}}, TR::Source({{1, ReturnValueIndex}})},
-  {{{"getlogin"}}, TR::Source({{ReturnValueIndex}})},
-  {{{"getlogin_r"}}, TR::Source({{0}})},
+  {{CDM::CLibrary, {"_IO_getc"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getcwd"}}, TR::Source({{0, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getwd"}}, TR::Source({{0, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"readlink"}}, TR::Source({{1, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"readlinkat"}}, TR::Source({{2, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"get_current_dir_name"}},
+   TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"gethostname"}}, TR::Source({{0}})},
+  {{CDM::CLibrary, {"getnameinfo"}}, TR::Source({{2, 4}})},
+  {{CDM::CLibrary, {"getseuserbyname"}}, TR::Source({{1, 2}})},
+  {{CDM::CLibrary, {"getgroups"}}, TR::Source({{1, ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getlogin"}}, TR::Source({{ReturnValueIndex}})},
+  {{CDM::CLibrary, {"getlogin_r"}}, TR::Source({{0}})},
 
   // Props
-  {{{"accept"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"atoi"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"atol"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"atoll"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"fgetc"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"fgetln"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"fgets"}}, TR::Prop({{2}}, {{0, ReturnValueIndex}})},
-  {{{"fgetws"}}, TR::Prop({{2}}, {{0, ReturnValueIndex}})},
-  {{{"fscanf"}}, TR::Prop({{0}}, {{}, 2})},
-  {{{"fscanf_s"}}, TR::Prop({{0}}, {{}, {2}})},
-  {{{"sscanf"}}, TR::Prop({{0}}, {{}, 2})},
-
-  {{{"getc"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"getc_unlocked"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"getdelim"}}, TR::Prop({{3}}, {{0}})},
-  {{{"getline"}}, TR::Prop({{2}}, {{0}})},
-  {{{"getw"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"pread"}}, TR::Prop({{0, 1, 2, 3}}, {{1, ReturnValueIndex}})},
-  {{{"read"}}, TR::Prop({{0, 2}}, {{1, ReturnValueIndex}})},
-  {{{"strchr"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"strrchr"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"tolower"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"toupper"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
-  {{{"fread"}}, TR::Prop({{3}}, {{

[clang] [analyzer] Clean up list of taint propagation functions (PR #91635)

2024-05-14 Thread Donát Nagy via cfe-commits

https://github.com/NagyDonat updated 
https://github.com/llvm/llvm-project/pull/91635

From 57ad704c30866a7d85f43b016583675e70de8531 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= 
Date: Thu, 9 May 2024 18:32:57 +0200
Subject: [PATCH 1/2] [analyzer] Clean up list of taint propagation functions

This commit refactors GenericTaintChecker and performs various
improvements in the list of taint propagation functions:

(1) The matching mode (usually `CDM::CLibrary` or
`CDM::CLibraryMaybeHardened`) was specified to avoid matching e.g.
C++ methods or functions from a user-defined namespace that happen
to share the name of a well-known library function.
(2) With these matching modes, a `CallDescription` can automatically
match builtin variants of the functions, so entries that explicitly
specified a builtin function were removed. This eliminated
inconsistencies where the "normal" and the builtin variant of the
same function was handled differently (e.g. `__builtin_strlcat` was
covered, while plain `strlcat` wasn't; while `__builtin_memcpy` and
`memcpy` were both on the list with different propagation rules).
(3) The modeling of the functions `strlcat` and `strncat` was updated to
propagate taint from the first argument (index 0), because a tainted
string should remain tainted even if we append something else to it.
Note that this was already applied to `strcat` and `wcsncat` by
commit 6ceb1c0ef9f544be0eed65e46cc7d99941a001bf.
(4) Some functions were updated to propagate taint from a size/length
argument to the result: e.g. `memcmp(p, q, get_tainted_int())` will
now return a tainted value (because the attacker can manipulate it).
This principle was already present in some propagation rules (e.g.
`__builtin_memcpy` was handled this way), and even after this commit
there are still some functions where it isn't applied. (I only aimed
for consistency within the same function family.)
(5) Functions that have hardened `__FOO_chk()` variants are matched in
`CDM:CLibraryMaybeHardened` to ensure consitent handling of the
"normal" and the hardened variant. I added special handling for the
hardened variants of "sprintf" and "snprintf" because there the
extra parameters are inserted into the middle of the parameter list.
(6) Modeling of `sscanf_s` was added, to complete the group of `fscanf`,
fscanf_s` and `sscanf`.
(7) The `Source()` specifications for `gets`, `gets_s` and `wgetch` were
ill-formed: they were specifying variadic arguments starting at
argument index `ReturnValueIndex`. (That is, in addition to the
return value they were propagating taint to all arguments.)
(8) Functions that were related to each other were grouped together. (I
know that this makes the diff harder to read, but I felt that the
full list is unreadable without some reorganization.)
(9) I spotted and removed some redundant curly braces. Perhaps would be
good to switch to a cleaner layout with less nested braces...
(10) I updated some obsolete comments and added two TODOs for issues
that should be fixed in followup commits.
---
 .../Checkers/GenericTaintChecker.cpp  | 370 ++
 1 file changed, 204 insertions(+), 166 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index d17f5ddf07055..a4ade7aaf590c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -400,17 +400,14 @@ class GenericTaintChecker : public 
Checker {
   void taintUnsafeSocketProtocol(const CallEvent &Call,
  CheckerContext &C) const;
 
-  /// Default taint rules are initalized with the help of a CheckerContext to
-  /// access the names of built-in functions like memcpy.
+  /// The taint rules are initalized with the help of a CheckerContext to
+  /// access user-provided configuration.
   void initTaintRules(CheckerContext &C) const;
 
-  /// CallDescription currently cannot restrict matches to the global namespace
-  /// only, which is why multiple CallDescriptionMaps are used, as we want to
-  /// disambiguate global C functions from functions inside user-defined
-  /// namespaces.
-  // TODO: Remove separation to simplify matching logic once CallDescriptions
-  // are more expressive.
-
+  // TODO: The two separate `CallDescriptionMap`s were introduced when
+  // `CallDescription` was unable to restric matches to the global namespace
+  // only. This limitation no longer exists, so the following two maps should
+  // be unified.
   mutable std::optional StaticTaintRules;
   mutable std::optional DynamicTaintRules;
 };
@@ -506,7 +503,8 @@ void GenericTaintRuleParser::consumeRulesFromConfig(const 
Config &C,
 GenericTaintRule &&Rule,
 

[clang] [analyzer] Clean up list of taint propagation functions (PR #91635)

2024-05-14 Thread Donát Nagy via cfe-commits

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


[clang] [clang-tools-extra] [Clang] Retain the angle loci for invented template parameters of constraints (PR #92104)

2024-05-14 Thread Haojian Wu via cfe-commits

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

Thanks!

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


[clang] [ASTMatchers] forCallable should not erase binding on success (PR #89657)

2024-05-14 Thread Marco Borgeaud via cfe-commits

https://github.com/marco-antognini-sonarsource updated 
https://github.com/llvm/llvm-project/pull/89657

>From ebc417fe98f1cb0e030ec77c17c0150c3fcca7f9 Mon Sep 17 00:00:00 2001
From: Marco Borgeaud
 <89914223+marco-antognini-sonarsou...@users.noreply.github.com>
Date: Fri, 19 Apr 2024 17:33:22 +0200
Subject: [PATCH] forCallable should not erase binding on success

Do not erase Builder when the first check fails because it could succeed
on the second stack frame.

The problem was that `InnerMatcher.matches` erases the bindings when it
returns false. The appropriate solution is to pass a copy of the
bindings, similar to what `matchesFirstInRange` does.
---
 clang/include/clang/ASTMatchers/ASTMatchers.h |  16 ++-
 .../ASTMatchers/ASTMatchersTraversalTest.cpp  | 130 ++
 2 files changed, 142 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index dc1f49525a004..54671fe404337 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -8341,20 +8341,28 @@ AST_MATCHER_P(Stmt, forCallable, 
internal::Matcher, InnerMatcher) {
 const auto &CurNode = Stack.back();
 Stack.pop_back();
 if (const auto *FuncDeclNode = CurNode.get()) {
-  if (InnerMatcher.matches(*FuncDeclNode, Finder, Builder)) {
+  BoundNodesTreeBuilder B = *Builder;
+  if (InnerMatcher.matches(*FuncDeclNode, Finder, &B)) {
+*Builder = std::move(B);
 return true;
   }
 } else if (const auto *LambdaExprNode = CurNode.get()) {
+  BoundNodesTreeBuilder B = *Builder;
   if (InnerMatcher.matches(*LambdaExprNode->getCallOperator(), Finder,
-   Builder)) {
+   &B)) {
+*Builder = std::move(B);
 return true;
   }
 } else if (const auto *ObjCMethodDeclNode = CurNode.get()) 
{
-  if (InnerMatcher.matches(*ObjCMethodDeclNode, Finder, Builder)) {
+  BoundNodesTreeBuilder B = *Builder;
+  if (InnerMatcher.matches(*ObjCMethodDeclNode, Finder, &B)) {
+*Builder = std::move(B);
 return true;
   }
 } else if (const auto *BlockDeclNode = CurNode.get()) {
-  if (InnerMatcher.matches(*BlockDeclNode, Finder, Builder)) {
+  BoundNodesTreeBuilder B = *Builder;
+  if (InnerMatcher.matches(*BlockDeclNode, Finder, &B)) {
+*Builder = std::move(B);
 return true;
   }
 } else {
diff --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp 
b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
index 6911d7600a718..2ecbdf659358f 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -5916,6 +5916,37 @@ TEST(StatementMatcher, ForCallable) {
   EXPECT_TRUE(notMatches(CppString2,
  returnStmt(forCallable(functionDecl(hasName("F"));
 
+  StringRef CodeWithDeepCallExpr = R"cpp(
+void Other();
+void Function() {
+  {
+(
+  Other()
+);
+  }
+}
+)cpp";
+  auto ForCallableFirst =
+  callExpr(forCallable(functionDecl(hasName("Function"))),
+   callee(functionDecl(hasName("Other")).bind("callee")))
+  .bind("call");
+  auto ForCallableSecond =
+  callExpr(callee(functionDecl(hasName("Other")).bind("callee")),
+   forCallable(functionDecl(hasName("Function"
+  .bind("call");
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  CodeWithDeepCallExpr, ForCallableFirst,
+  std::make_unique>("call")));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  CodeWithDeepCallExpr, ForCallableFirst,
+  std::make_unique>("callee")));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  CodeWithDeepCallExpr, ForCallableSecond,
+  std::make_unique>("call")));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  CodeWithDeepCallExpr, ForCallableSecond,
+  std::make_unique>("callee")));
+
   // These tests are specific to forCallable().
   StringRef ObjCString1 = "@interface I"
   "-(void) foo;"
@@ -5957,6 +5988,105 @@ TEST(StatementMatcher, ForCallable) {
   binaryOperator(forCallable(blockDecl();
 }
 
+namespace {
+class ForCallablePreservesBindingWithMultipleParentsTestCallback
+: public BoundNodesCallback {
+public:
+  bool run(const BoundNodes *BoundNodes) override {
+FunctionDecl const *FunDecl =
+BoundNodes->getNodeAs("funDecl");
+// Validate test assumptions. This would be expressed as ASSERT_* in
+// a TEST().
+if (!FunDecl) {
+  EXPECT_TRUE(false && "Incorrect test setup");
+  return false;
+}
+auto const *FunDef = FunDecl->getDefinition();
+if (!FunDef || !FunDef->getBody() ||
+FunDef->getNameAsString() != "Function") {
+  EXPECT_TRUE(false && "Incorrect test setup");
+  return false;
+}
+
+ExpectCorrectResult(
+"Baseline",
+callExpr(callee(cxxM

[clang] [analyzer] Variant checker bindings (PR #87886)

2024-05-14 Thread Donát Nagy via cfe-commits

https://github.com/NagyDonat commented:

Some additional remarks.

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


[clang] [analyzer] Variant checker bindings (PR #87886)

2024-05-14 Thread Donát Nagy via cfe-commits

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


[clang] [analyzer] Variant checker bindings (PR #87886)

2024-05-14 Thread Donát Nagy via cfe-commits


@@ -206,23 +221,42 @@ class StdVariantChecker : public Checker {
 if (!ThisMemRegion)
   return;
 
+// Get the first type alternative of the std::variant instance.
+assert((ThisSVal.getType(C.getASTContext())->isPointerType() ||
+ThisSVal.getType(C.getASTContext())->isReferenceType()) &&
+   "The This SVal must be a pointer!");
+
 std::optional DefaultType = getNthTemplateTypeArgFromVariant(
 ThisSVal.getType(C.getASTContext())->getPointeeType().getTypePtr(), 0);
 if (!DefaultType)
   return;
 
+// We conjure a symbol that represents the value-initialized value held by
+// the default constructed std::variant. This could be made more precise
+// if we would actually simulate the value-initialization of the value.
+//
+// We are storing pointer/reference typed SVals because when an
+// std::variant is constructed with a value constructor a reference is
+// received. The SVal representing this parameter will also have reference
+// type. We use this SVal to store information about the value held is an
+// std::variant instance. Here we are conforming to this and also use
+// reference type. Also if we would not use reference typed SVals
+// the analyzer would crash when handling the cast expression with the
+// reason that the SVal is a NonLoc SVal.
+SVal DefaultConstructedHeldValue = C.getSValBuilder().conjureSymbolVal(
+ConstructorCall->getOriginExpr(), C.getLocationContext(),
+C.getASTContext().getLValueReferenceType(*DefaultType), 
C.blockCount());

NagyDonat wrote:

It would be good to model running the default constructor of this `DefaultType` 
at this point.

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


[clang] [analyzer] Variant checker bindings (PR #87886)

2024-05-14 Thread Donát Nagy via cfe-commits


@@ -37,6 +43,19 @@ static SVal conjureOffsetSymbolOnLocation(
   return Symbol;
 }
 
+// Update the SVal bound to the Cast expression with the SVal
+// bound to the casted expression
+static ProgramStateRef updateStateAfterSimpleCast(StmtNodeBuilder& Bldr,

NagyDonat wrote:

Rename `Simple` -> `Trivial` and emphasize in the comment that this is only 
applied to trivial casts. (Without knowing the context where it's called, it's 
a red flag to see that the cast is unconditionally ignored within this 
function.)

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


[clang] [analyzer] Variant checker bindings (PR #87886)

2024-05-14 Thread Donát Nagy via cfe-commits


@@ -355,4 +356,38 @@ void nonInlineFunctionCallPtr() {
   char c = std::get (v); // no-warning
   (void)a;
   (void)c;
-}
\ No newline at end of file
+}
+
+////
+// std::swap for std::variant
+////
+
+void swapForVariants() {
+  std::variant a = 5;
+  std::variant b = 'C';
+  std::swap(a, b);
+  int a1 = std::get(b);
+  char c = std::get(a); // expected-warning {{std::variant 'a' held a 
'char', not an 'int'}}
+  (void)a1;
+  (void)c;
+}
+
+////
+// std::swap for std::variant
+////
+
+void stdEmplace() {
+  std::variant v = 'c';
+  v.emplace (5);
+  int a = std::get (v); // no-warning
+  char c = std::get (v); // no-warning
+  (void)a;
+  (void)c;
+}
+
+void followHeldValue() {

NagyDonat wrote:

It's good that this case highlights that the value is _not yet_ followed by 
this checker; but consider adding a TODO or similar commit to highlight that 
this is a limitation of the checker and eventually it could / will be modeled. 
(Or, if you believe that the value shouldn't be tracked, then explain that.)

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


[clang] [analyzer] Variant checker bindings (PR #87886)

2024-05-14 Thread Donát Nagy via cfe-commits


@@ -681,6 +681,37 @@ ExprEngine::processRegionChanges(ProgramStateRef state,
  LCtx, Call);
 }
 
+ProgramStateRef
+ExprEngine::handleCastingBeforeEvalCall(ExplodedNode *Pred, const Expr *Ex, 
+SVal ValueToBind, ProgramStateRef 
State, StmtNodeBuilder* Bldr) const {
+  // TODO construct new Bldr if Bldr is null
+  bool DeleteAfter = false;
+  if (!Bldr) {
+ExplodedNodeSet dstEvaluated;
+Bldr = new StmtNodeBuilder(Pred, dstEvaluated, *currBldrCtx);
+DeleteAfter = true;
+  }
+
+  if (auto *AsImplCast = dyn_cast_or_null(Ex);
+  AsImplCast && AsImplCast->getSubExpr() && ValueToBind.isUndef()) {
+const LocationContext *LCtx = Pred->getLocationContext();
+if (!isa(AsImplCast->getSubExpr())) {
+  return State;
+}
+SVal V = State->getSVal(AsImplCast->getSubExpr(), LCtx);
+if (V.isUndef()) {
+  return State;
+}
+State = State->BindExpr(Ex, LCtx, V);
+Bldr->generateNode(Ex, Pred, State);
+  }
+
+  if (DeleteAfter) {
+delete Bldr;

NagyDonat wrote:

There are _two_ early return branches where this `delete` is not reached. (I 
know that this is proof-of-concept and will be cleaned up before merging, but 
still...)

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


[clang] [analyzer] Variant checker bindings (PR #87886)

2024-05-14 Thread Donát Nagy via cfe-commits


@@ -602,6 +619,37 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, 
ExplodedNode *Pred,
   ExplodedNode *UpdatedN = N;
   SVal InitVal = state->getSVal(InitEx, LC);
 
+  // The call expression to which we have bound something is hidden behind
+  // an implicit cast expression.
+
+  // This is a workaround for those checkers that are evaluating calls
+  // with return value, and are "behind" a cast expression. A good example
+  // for this is std::variant checker.
+  // Let's see the following code as an example:
+  //
+  // int a = std::get(v);
+  //
+  // The AST for the std::get call shall look something like this:
+  //
+  // ImplicitCastExpr 
+  // `-CallExpr
+  //
+  // First the handling of `ImplicitCastExpr `
+  // happens in the ExprEngine::VisitCast function. After that
+  // std::variant checker evaluates the std::get call and binds
+  // an SVal to the call expression. The problem here is that
+  // the handling of the casting is the responsible to bind the
+  // sub expressions (in our case std::get call expressions) value
+  // to the cast expression.
+  if (auto *AsImplCast = dyn_cast_or_null(InitEx);
+  AsImplCast && InitVal.isUndef()) {
+// InitVal = state->getSVal(AsImplCast->getSubExpr(), LC);

NagyDonat wrote:

Why is this line commented out?

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


[clang] [analyzer] Variant checker bindings (PR #87886)

2024-05-14 Thread Donát Nagy via cfe-commits


@@ -51,27 +47,29 @@ removeInformationStoredForDeadInstances(const CallEvent 
&Call,
 }
 
 template 
-void handleConstructorAndAssignment(const CallEvent &Call, CheckerContext &C,
+bool handleConstructorAndAssignment(const CallEvent &Call, CheckerContext &C,
 SVal ThisSVal) {
   ProgramStateRef State = Call.getState();
 
   if (!State)
-return;
+return false;
 
   auto ArgSVal = Call.getArgSVal(0);
   const auto *ThisRegion = ThisSVal.getAsRegion();
   const auto *ArgMemRegion = ArgSVal.getAsRegion();
+  if (!ArgMemRegion)
+return false;

NagyDonat wrote:

Are you sure that `ThisRegion` cannot be null?

I suspect that `ThisSVal` might be `UnknownVal` in some circumstances (e.g. 
invalidation, complexity limit reached etc.) and that would lead to a 
`getAsRegion()` returning `nullptr` while modeling e.g. an assignment operator 
call.

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


[clang] [analyzer] Variant checker bindings (PR #87886)

2024-05-14 Thread Donát Nagy via cfe-commits


@@ -37,6 +43,19 @@ static SVal conjureOffsetSymbolOnLocation(
   return Symbol;
 }
 
+// Update the SVal bound to the Cast expression with the SVal
+// bound to the casted expression
+static ProgramStateRef updateStateAfterSimpleCast(StmtNodeBuilder& Bldr,
+   ExplodedNode *Pred,
+   const CastExpr *CastE,
+   const Expr *CastedE) {
+  ProgramStateRef state = Pred->getState();
+  const LocationContext *LCtx = Pred->getLocationContext();
+  SVal V = state->getSVal(CastedE, LCtx);
+  return state->BindExpr(CastE, LCtx, V);
+  //Bldr.generateNode(CastE, Pred, state);

NagyDonat wrote:

```suggestion
```
Another leftover comment that should be removed.

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


[clang-tools-extra] [clang-tidy] Rename out-of-line function definitions (PR #91954)

2024-05-14 Thread Edwin Vane via cfe-commits

https://github.com/revane updated 
https://github.com/llvm/llvm-project/pull/91954

>From ffcc0bfc29b577fd727bc00a912e4c0eb515628a Mon Sep 17 00:00:00 2001
From: Edwin Vane 
Date: Mon, 15 Apr 2024 09:38:48 -0400
Subject: [PATCH] [clang-tidy] Rename out-of-line function definitions

Member function templates defined out-of-line were resulting in
conflicting naming failures with overlapping usage sets. With this
change, out-of-line definitions are treated as a usage of the failure
which is the inline declaration.
---
 .../utils/RenamerClangTidyCheck.cpp   |  3 ++
 clang-tools-extra/docs/ReleaseNotes.rst   |  3 +-
 .../readability/identifier-naming-ool.cpp | 30 +++
 3 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-ool.cpp

diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp 
b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index e811f5519de2c..88e4886cd0df9 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -123,6 +123,9 @@ static const NamedDecl *getFailureForNamedDecl(const 
NamedDecl *ND) {
   if (const auto *Method = dyn_cast(ND)) {
 if (const CXXMethodDecl *Overridden = getOverrideMethod(Method))
   Canonical = cast(Overridden->getCanonicalDecl());
+else if (const FunctionTemplateDecl *Primary = 
Method->getPrimaryTemplate())
+  if (const FunctionDecl *TemplatedDecl = Primary->getTemplatedDecl())
+Canonical = cast(TemplatedDecl->getCanonicalDecl());
 
 if (Canonical != ND)
   return Canonical;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index fc976ce3a33d5..e86786cfb301a 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -343,7 +343,8 @@ Changes in existing checks
   ` check in 
`GetConfigPerFile`
   mode by resolving symbolic links to header files. Fixed handling of Hungarian
   Prefix when configured to `LowerCase`. Added support for renaming designated
-  initializers. Added support for renaming macro arguments.
+  initializers. Added support for renaming macro arguments. Fixed renaming
+  conflicts arising from out-of-line member function template definitions.
 
 - Improved :doc:`readability-implicit-bool-conversion
   ` check to provide
diff --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-ool.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-ool.cpp
new file mode 100644
index 0..f807875e27698
--- /dev/null
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-ool.cpp
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -std=c++20 \
+// RUN:   --config='{CheckOptions: { \
+// RUN: readability-identifier-naming.MethodCase: CamelCase, \
+// RUN:  }}'
+
+namespace SomeNamespace {
+namespace Inner {
+
+class SomeClass {
+public:
+template 
+int someMethod();
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for method 
'someMethod' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int SomeMethod();
+};
+template 
+int SomeClass::someMethod() {
+// CHECK-FIXES: {{^}}int SomeClass::SomeMethod() {
+return 5;
+}
+
+} // namespace Inner
+
+void someFunc() {
+Inner::SomeClass S;
+S.someMethod();
+// CHECK-FIXES: {{^}}S.SomeMethod();
+}
+
+} // namespace SomeNamespace

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


[clang-tools-extra] [clang-tidy] Rename out-of-line function definitions (PR #91954)

2024-05-14 Thread Edwin Vane via cfe-commits


@@ -123,6 +123,9 @@ static const NamedDecl *getFailureForNamedDecl(const 
NamedDecl *ND) {
   if (const auto *Method = dyn_cast(ND)) {
 if (const CXXMethodDecl *Overridden = getOverrideMethod(Method))
   Canonical = cast(Overridden->getCanonicalDecl());
+else if (const FunctionTemplateDecl *Primary = 
Method->getPrimaryTemplate())
+  Canonical =
+  cast(Primary->getTemplatedDecl()->getCanonicalDecl());

revane wrote:

Done.

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


[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

2024-05-14 Thread Aaron Ballman via cfe-commits

AaronBallman wrote:

> Thanks for everyone's input so far. Let me try to summarize discussions in 
> this PR so we can set on an approach and give advice to our CIR community 
> (and encode on our webpage) on how to move forward in upcoming patches. 
> Useful resources:
> 
> * [LLVM Coding Standard](https://llvm.org/docs/CodingStandards.html)
> 
> * [MLIR Style 
> Guide](https://mlir.llvm.org/getting_started/DeveloperGuide/#style-guide)
> 
> 
> CC'ing more people who might care: @seven-mile, @Lancern.
> ## Use of `auto`
> 
> As @joker-eph mentioned, MLIR isn't meant to differ from LLVM/Clang, though 
> they encode the differences as part of their guidelines. The `auto` 
> [guidance](https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable)
>  is still in favor of us keeping it for all `isa`, `dyn_cast` and `cast`, 
> which is used in CIR, so it probably doesn't affect most of what we currently 
> care about. Here's my suggestion for the entire `lib/CIR`:
> 
> 1. Use it for function templates such as `isa`, `dyn_cast`, `cast`, 
> `create` and `rewriteOp.*` variants.

Agreed -- anywhere the type is spelled out somewhere in the initializer is fine 
(so also `static_cast`, `getAs`, `new`, etc).

> 2. Use it for things considered obvious/common in MLIR space, examples:
> 
> 
> * `auto loc = op->getLoc()`.

What is obvious in the MLIR space is not necessarily what's obvious in Clang; I 
have no idea whether that returns a `SourceLocation`, a `PresumedLoc`, a 
`FullSourceLoc`, etc, so I don't think that is a use of `auto` I would want to 
have to reason about as a reviewer. That said, if the only use of `loc` is: 
`Diag(loc, diag::err_something);` then the type really doesn't matter and use 
of `auto` is probably fine. It's a judgment call.

> * Getting operands and results from operations (they obey Value 
> Semantics), e.g.: `DepthwiseConv2DNhwcHwcmOp op; ...; auto stride = 
> op.getStrides();`

Again, I have no idea what the type of that is and I have no way to verify that 
it actually *uses* value semantics because the pointer and qualifiers can be 
inferred and references can be dropped. That makes it harder to review the 
code; I would spell out the type in this case as well.

> * Other examples we are happy to provide upon reviewer feedback if it 
> makes sense.

The big things for reviewers are: don't use `auto` if the type isn't incredibly 
obvious (spelled in the initialization or extremely obvious based on 
immediately surrounding code) and don't make us infer important parts the 
declarator (if it's a `const Foo *` and `auto` makes sense for some reason, 
spell it `const auto *` rather than `auto`). And if a reviewer asks for 
something to be converted from `auto` to an explicit type name, assume that 
means the code isn't as readable as it could be and switch to a concrete type 
(we should not be arguing "I think this is clear therefore you should also 
think it's clear" during review, that just makes the review process painful for 
everyone involved).

> Using the logic above, all `auto`s in this current PR have to be changed 
> (since none apply).
> ## Var naming: CamelCase vs camelBack
> 
> From this discussion, seems like @AaronBallman and @erichkeane are fine with 
> us using camelBack and all the other differences from [MLIR Style 
> Guide](https://mlir.llvm.org/getting_started/DeveloperGuide/#style-guide) in 
> CIRGen and the rest of CIR. Is that right? This is based on the comment:
> 
> > The mixed naming conventions in the header should be fixed (preference is 
> > to follow LLVM style if we're changing code around, but if the local 
> > preference is for MLIR, that's fine so long as it's consistent).
>
> However, @AaronBallman also wrote:
> 
> > Also, the fact that the naming keeps being inconsistent is a pretty strong 
> > reason to change to use the LLVM naming convention, at least for external 
> > interfaces.
> 
> Should we ignore this in light of your first comment? If not, can you clarify 
> what do you mean by external interfaces? Just want to make sure we get it 
> right looking fwd.

My thinking is: 1) (most important) the convention should be consistent with 
other nearby code, whatever that convention happens to be. 2) if there's not 
already a convention to follow and if it's code that lives in 
`clang/include/clang`, it should generally follow the LLVM style (it's an 
"external" interface) because those are the APIs that folks outside of CIR will 
be calling into. If it's code that lives in `clang/lib/`, following the MLIR 
style is less of a concern.

So in the case of this review, there's mixed styles being used in the PR and so 
something has to change. If we're changing something anyway, changing to the 
LLVM style is generally preferred over changing to the MLIR style.

One caveat to this is: avoid busywork but use your best judgement on how we 
*eventually* get to a consistent use of the LLVM style. If chan

[clang] [clang][analyzer] Fix alpha.unix.BlockInCriticalSection for CTU (PR #90030)

2024-05-14 Thread Endre Fülöp via cfe-commits

https://github.com/gamesh411 updated 
https://github.com/llvm/llvm-project/pull/90030

From de695f8e556e1efd6b2a4e69f916467af94e0c0d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= 
Date: Tue, 9 Apr 2024 10:44:43 +0200
Subject: [PATCH 1/2] [clang][analyzer] Fix alpha.unix.BlockInCriticalSection
 for CTU

In CTU there is not always an AnalysisDeclContext for a given call. This
led to crashes. The AnalysisDeclContext access is now checked.
---
 .../Checkers/BlockInCriticalSectionChecker.cpp| 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git 
a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index e4373915410fb..9874a68ebe47a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -14,6 +14,7 @@
 //
 
//===--===//
 
+#include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -103,9 +104,10 @@ class RAIIMutexDescriptor {
   // this function is called instead of early returning it. To avoid this, 
a
   // bool variable (IdentifierInfoInitialized) is used and the function 
will
   // be run only once.
-  Guard = &Call.getCalleeAnalysisDeclContext()->getASTContext().Idents.get(
-  GuardName);
-  IdentifierInfoInitialized = true;
+  if (AnalysisDeclContext *CalleCtx = Call.getCalleeAnalysisDeclContext()) 
{
+Guard = &CalleCtx->getASTContext().Idents.get(GuardName);
+IdentifierInfoInitialized = true;
+  }
 }
   }
 

From d811a172dd9d07a895b84aa873eb6636a4fa008d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= 
Date: Tue, 14 May 2024 14:43:30 +0200
Subject: [PATCH 2/2] Add crashing test

---
 clang/test/Analysis/block-in-critical-section.c | 6 ++
 1 file changed, 6 insertions(+)
 create mode 100644 clang/test/Analysis/block-in-critical-section.c

diff --git a/clang/test/Analysis/block-in-critical-section.c 
b/clang/test/Analysis/block-in-critical-section.c
new file mode 100644
index 0..1e174af541b18
--- /dev/null
+++ b/clang/test/Analysis/block-in-critical-section.c
@@ -0,0 +1,6 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.unix.BlockInCriticalSection -verify %s
+// expected-no-diagnostics
+
+// This should not crash
+int (*a)(void);
+void b(void) { a(); }

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


[clang] [clang][analyzer] Fix alpha.unix.BlockInCriticalSection for CTU (PR #90030)

2024-05-14 Thread Endre Fülöp via cfe-commits

https://github.com/gamesh411 updated 
https://github.com/llvm/llvm-project/pull/90030

From ca7be03d939d3375e9075fc287393d3b5e5a2c84 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= 
Date: Tue, 9 Apr 2024 10:44:43 +0200
Subject: [PATCH 1/2] [clang][analyzer] Fix alpha.unix.BlockInCriticalSection
 for CTU

In CTU there is not always an AnalysisDeclContext for a given call. This
led to crashes. The AnalysisDeclContext access is now checked.
---
 .../Checkers/BlockInCriticalSectionChecker.cpp| 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git 
a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index e138debd1361c..d381a30f7e24c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -14,6 +14,7 @@
 //
 
//===--===//
 
+#include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -103,9 +104,10 @@ class RAIIMutexDescriptor {
   // this function is called instead of early returning it. To avoid this, 
a
   // bool variable (IdentifierInfoInitialized) is used and the function 
will
   // be run only once.
-  Guard = &Call.getCalleeAnalysisDeclContext()->getASTContext().Idents.get(
-  GuardName);
-  IdentifierInfoInitialized = true;
+  if (AnalysisDeclContext *CalleCtx = Call.getCalleeAnalysisDeclContext()) 
{
+Guard = &CalleCtx->getASTContext().Idents.get(GuardName);
+IdentifierInfoInitialized = true;
+  }
 }
   }
 

From bc759c4ddde48c7df7faa3beb233c1931388b558 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= 
Date: Tue, 14 May 2024 14:43:30 +0200
Subject: [PATCH 2/2] Add crashing test

---
 clang/test/Analysis/block-in-critical-section.c | 6 ++
 1 file changed, 6 insertions(+)
 create mode 100644 clang/test/Analysis/block-in-critical-section.c

diff --git a/clang/test/Analysis/block-in-critical-section.c 
b/clang/test/Analysis/block-in-critical-section.c
new file mode 100644
index 0..1e174af541b18
--- /dev/null
+++ b/clang/test/Analysis/block-in-critical-section.c
@@ -0,0 +1,6 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.unix.BlockInCriticalSection -verify %s
+// expected-no-diagnostics
+
+// This should not crash
+int (*a)(void);
+void b(void) { a(); }

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


[clang-tools-extra] cc54129 - Add option to exclude headers from clang-tidy analysis (#91400)

2024-05-14 Thread via cfe-commits

Author: Justin Cady
Date: 2024-05-14T08:48:04-04:00
New Revision: cc54129b983799e1aaea77aa0ff3040dc30cbc8c

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

LOG: Add option to exclude headers from clang-tidy analysis (#91400)

This is a renewed attempt to land @toddlipcon's D34654. The comments on
that patch indicate a broad desire for some ability to ignore headers.

After considering various options, including migrating to std::regex, I
believe this is the best path forward. It's intuitive to have separate
regexes for including headers versus excluding them, and this approach
has the added benefit of being completely opt-in. No existing configs
will break, regardless of existing HeaderFilterRegex values.

This functionality is useful for improving performance when analyzing a
targeted subset of code, as well as in cases where some collection of
headers cannot be modified (third party source, for example).

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
clang-tools-extra/clang-tidy/ClangTidyOptions.h
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/index.rst

clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/.clang-tidy

clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/1/.clang-tidy

clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/3/.clang-tidy
clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index de2a3b51422a5..200bb87a5ac3c 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -311,7 +311,18 @@ ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
 : Context(Ctx), ExternalDiagEngine(ExternalDiagEngine),
   RemoveIncompatibleErrors(RemoveIncompatibleErrors),
   GetFixesFromNotes(GetFixesFromNotes),
-  EnableNolintBlocks(EnableNolintBlocks) {}
+  EnableNolintBlocks(EnableNolintBlocks) {
+
+  if (Context.getOptions().HeaderFilterRegex &&
+  !Context.getOptions().HeaderFilterRegex->empty())
+HeaderFilter =
+std::make_unique(*Context.getOptions().HeaderFilterRegex);
+
+  if (Context.getOptions().ExcludeHeaderFilterRegex &&
+  !Context.getOptions().ExcludeHeaderFilterRegex->empty())
+ExcludeHeaderFilter = std::make_unique(
+*Context.getOptions().ExcludeHeaderFilterRegex);
+}
 
 void ClangTidyDiagnosticConsumer::finalizeLastError() {
   if (!Errors.empty()) {
@@ -562,22 +573,17 @@ void 
ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location,
   }
 
   StringRef FileName(File->getName());
-  LastErrorRelatesToUserCode = LastErrorRelatesToUserCode ||
-   Sources.isInMainFile(Location) ||
-   getHeaderFilter()->match(FileName);
+  LastErrorRelatesToUserCode =
+  LastErrorRelatesToUserCode || Sources.isInMainFile(Location) ||
+  (HeaderFilter &&
+   (HeaderFilter->match(FileName) &&
+!(ExcludeHeaderFilter && ExcludeHeaderFilter->match(FileName;
 
   unsigned LineNumber = Sources.getExpansionLineNumber(Location);
   LastErrorPassesLineFilter =
   LastErrorPassesLineFilter || passesLineFilter(FileName, LineNumber);
 }
 
-llvm::Regex *ClangTidyDiagnosticConsumer::getHeaderFilter() {
-  if (!HeaderFilter)
-HeaderFilter =
-std::make_unique(*Context.getOptions().HeaderFilterRegex);
-  return HeaderFilter.get();
-}
-
 void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() {
   // Each error is modelled as the set of intervals in which it applies
   // replacements. To detect overlapping replacements, we use a sweep line

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index 9280eb1e1f218..97e16a12febd0 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -313,6 +313,7 @@ class ClangTidyDiagnosticConsumer : public 
DiagnosticConsumer {
   bool EnableNolintBlocks;
   std::vector Errors;
   std::unique_ptr HeaderFilter;
+  std::unique_ptr ExcludeHeaderFilter;
   bool LastErrorRelatesToUserCode = false;
   bool LastErrorPassesLineFilter = false;
   bool LastErrorWas

  1   2   3   4   >