[PATCH] D105555: [PoC][RISCV][Clang] Compute the default target-abi if it's empty.

2021-07-18 Thread Zakk Chen via Phabricator via cfe-commits
khchen updated this revision to Diff 359670.
khchen added a comment.

rebase on D105168 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D10/new/

https://reviews.llvm.org/D10

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/CodeGen/RISCV/riscv-metadata.c
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/TargetParser.cpp

Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -331,5 +331,18 @@
   return true;
 }
 
+StringRef computeABIByArch(bool HasD, bool HasE, bool Is64Bit) {
+  if (!Is64Bit) {
+if (HasD)
+  return "ilp32d";
+if (HasE)
+  return "ilp32e";
+return "ilp32";
+  }
+  if (HasD)
+return "lp64d";
+  return "lp64";
+}
+
 } // namespace RISCV
 } // namespace llvm
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -174,6 +174,7 @@
 void fillValidTuneCPUArchList(SmallVectorImpl , bool IsRV64);
 bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector );
 StringRef resolveTuneCPUAlias(StringRef TuneCPU, bool IsRV64);
+StringRef computeABIByArch(bool HasD, bool HasE, bool IsRV64);
 
 } // namespace RISCV
 
Index: clang/test/CodeGen/RISCV/riscv-metadata.c
===
--- clang/test/CodeGen/RISCV/riscv-metadata.c
+++ clang/test/CodeGen/RISCV/riscv-metadata.c
@@ -1,14 +1,28 @@
+// RUN: %clang_cc1 -triple riscv32 -emit-llvm -o - %s | FileCheck -check-prefix=EMPTY-ILP32 %s
+// RUN: %clang_cc1 -triple riscv32 -emit-llvm -target-feature +f -target-feature +d -o - %s | FileCheck -check-prefix=EMPTY-ILP32D %s
 // RUN: %clang_cc1 -triple riscv32 -target-abi ilp32 -emit-llvm -o - %s | FileCheck -check-prefix=ILP32 %s
 // RUN: %clang_cc1 -triple riscv32 -target-feature +f -target-abi ilp32f -emit-llvm -o - %s | FileCheck -check-prefix=ILP32F %s
 // RUN: %clang_cc1 -triple riscv32 -target-feature +d -target-abi ilp32d -emit-llvm -o - %s | FileCheck -check-prefix=ILP32D %s
+// RUN: %clang_cc1 -triple riscv64 -emit-llvm -o - %s | FileCheck -check-prefix=EMPTY-LP64 %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d -emit-llvm -o - %s | FileCheck -check-prefix=EMPTY-LP64D %s
 // RUN: %clang_cc1 -triple riscv64 -target-abi lp64 -emit-llvm -o - %s | FileCheck -check-prefix=LP64 %s
 // RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-abi lp64f -emit-llvm -o - %s | FileCheck -check-prefix=LP64F %s
 // RUN: %clang_cc1 -triple riscv64 -target-feature +d -target-abi lp64d -emit-llvm -o - %s | FileCheck -check-prefix=LP64D %s
 
+// Test expected behavior when giving -target-cpu
+// This cc1 test is similar to clang with -march=rv32ifd -mcpu=sifive-e31, default abi is ilp32d
+// RUN: %clang_cc1 -triple riscv32 -emit-llvm -target-feature +f -target-feature +d -target-cpu sifive-e31 -o - %s | FileCheck -check-prefix=EMPTY-ILP32D %s
+// This cc1 test is similar to clang with -march=rv64i -mcpu=sifive-u74, default abi is lp64
+// RUN: %clang_cc1 -triple riscv64 -emit-llvm -o - -target-cpu sifive-u74 %s | FileCheck -check-prefix=EMPTY-LP64 %s
+
+// EMPTY-ILP32: !{{[0-9]+}} = !{i32 1, !"target-abi", !"ilp32"}
+// EMPTY-ILP32D: !{{[0-9]+}} = !{i32 1, !"target-abi", !"ilp32d"}
 // ILP32: !{{[0-9]+}} = !{i32 1, !"target-abi", !"ilp32"}
 // ILP32F: !{{[0-9]+}} = !{i32 1, !"target-abi", !"ilp32f"}
 // ILP32D: !{{[0-9]+}} = !{i32 1, !"target-abi", !"ilp32d"}
 
+// EMPTY-LP64: !{{[0-9]+}} = !{i32 1, !"target-abi", !"lp64"}
+// EMPTY-LP64D: !{{[0-9]+}} = !{i32 1, !"target-abi", !"lp64d"}
 // LP64: !{{[0-9]+}} = !{i32 1, !"target-abi", !"lp64"}
 // LP64F: !{{[0-9]+}} = !{i32 1, !"target-abi", !"lp64f"}
 // LP64D: !{{[0-9]+}} = !{i32 1, !"target-abi", !"lp64d"}
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -197,22 +197,10 @@
 // Ignore parsing error, just go 3rd step.
 consumeError(std::move(E));
   } else {
-bool HasD = ISAInfo.hasExtension("d");
 unsigned XLen = ISAInfo.getXLen();
-if (XLen == 32) {
-  bool HasE = ISAInfo.hasExtension("e");
-  if (HasD)
-return "ilp32d";
-  else if (HasE)
-return "ilp32e";
-  else
-return "ilp32";
-} else if (XLen == 64) {
-  if (HasD)
-return "lp64d";
-  else
-return "lp64";
-}
+bool HasD = ISAInfo.hasExtension("d");
+bool HasE = ISAInfo.hasExtension("e");
+return llvm::RISCV::computeABIByArch(HasD, HasE, /*IsRV64=*/XLen == 64);
   }
 
   // 3. Choose a default based on the triple

[PATCH] D105001: [Clang][RISCV] Support half-precision floating point for RVV intrinsics.

2021-07-18 Thread Hsiangkai Wang via Phabricator via cfe-commits
HsiangKai updated this revision to Diff 359667.
HsiangKai added a comment.

Add else and llvm_unreachable() for unhandled floating types.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105001/new/

https://reviews.llvm.org/D105001

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Basic/riscv_vector.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfadd.c
  clang/utils/TableGen/RISCVVEmitter.cpp

Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -411,7 +411,7 @@
   case ScalarTypeKind::Float:
 switch (ElementBitwidth) {
 case 16:
-  BuiltinStr += "h";
+  BuiltinStr += "x";
   break;
 case 32:
   BuiltinStr += "f";
@@ -507,8 +507,10 @@
 Str += "double";
   else if (ElementBitwidth == 32)
 Str += "float";
-  assert((ElementBitwidth == 32 || ElementBitwidth == 64) &&
- "Unhandled floating type");
+  else if (ElementBitwidth == 16)
+Str += "_Float16";
+  else
+llvm_unreachable("Unhandled floating type.");
 } else
   Str += getTypeString("float");
 break;
@@ -565,7 +567,7 @@
 ElementBitwidth = 64;
 ScalarType = ScalarTypeKind::SignedInteger;
 break;
-  case 'h':
+  case 'x':
 ElementBitwidth = 16;
 ScalarType = ScalarTypeKind::Float;
 break;
@@ -931,7 +933,7 @@
   }
   OS << "#if defined(__riscv_zfh)\n";
   for (int Log2LMUL : Log2LMULs) {
-auto T = computeType('h', Log2LMUL, "v");
+auto T = computeType('x', Log2LMUL, "v");
 if (T.hasValue())
   printType(T.getValue());
   }
Index: clang/test/CodeGen/RISCV/rvv-intrinsics/vfadd.c
===
--- clang/test/CodeGen/RISCV/rvv-intrinsics/vfadd.c
+++ clang/test/CodeGen/RISCV/rvv-intrinsics/vfadd.c
@@ -1,367 +1,487 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // REQUIRES: riscv-registered-target
 // RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d -target-feature +experimental-v \
-// RUN:   -target-feature +experimental-zfh -disable-O0-optnone -emit-llvm %s -o - | opt -S -mem2reg | FileCheck --check-prefix=CHECK-RV64 %s
+// RUN:   -target-feature +experimental-zfh -disable-O0-optnone -emit-llvm %s -o - \
+// RUN:   | opt -S -mem2reg | FileCheck --check-prefix=CHECK-RV64 %s
 
 #include 
 
-//
+// CHECK-RV64-LABEL: @test_vfadd_vv_f16mf4(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.riscv.vfadd.nxv1f16.nxv1f16.i64( [[OP1:%.*]],  [[OP2:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+vfloat16mf4_t test_vfadd_vv_f16mf4 (vfloat16mf4_t op1, vfloat16mf4_t op2, size_t vl) {
+  return vfadd_vv_f16mf4(op1, op2, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vfadd_vf_f16mf4(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.riscv.vfadd.nxv1f16.f16.i64( [[OP1:%.*]], half [[OP2:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+vfloat16mf4_t test_vfadd_vf_f16mf4 (vfloat16mf4_t op1, _Float16 op2, size_t vl) {
+  return vfadd_vf_f16mf4(op1, op2, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vfadd_vv_f16mf2(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.riscv.vfadd.nxv2f16.nxv2f16.i64( [[OP1:%.*]],  [[OP2:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+vfloat16mf2_t test_vfadd_vv_f16mf2 (vfloat16mf2_t op1, vfloat16mf2_t op2, size_t vl) {
+  return vfadd_vv_f16mf2(op1, op2, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vfadd_vf_f16mf2(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.riscv.vfadd.nxv2f16.f16.i64( [[OP1:%.*]], half [[OP2:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+vfloat16mf2_t test_vfadd_vf_f16mf2 (vfloat16mf2_t op1, _Float16 op2, size_t vl) {
+  return vfadd_vf_f16mf2(op1, op2, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vfadd_vv_f16m1(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.riscv.vfadd.nxv4f16.nxv4f16.i64( [[OP1:%.*]],  [[OP2:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+vfloat16m1_t test_vfadd_vv_f16m1 (vfloat16m1_t op1, vfloat16m1_t op2, size_t vl) {
+  return vfadd_vv_f16m1(op1, op2, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vfadd_vf_f16m1(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.riscv.vfadd.nxv4f16.f16.i64( [[OP1:%.*]], half [[OP2:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+vfloat16m1_t test_vfadd_vf_f16m1 (vfloat16m1_t op1, _Float16 op2, size_t vl) {
+  return vfadd_vf_f16m1(op1, op2, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vfadd_vv_f16m2(
+// CHECK-RV64-NEXT:  entry:

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

2021-07-18 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur updated this revision to Diff 359666.
Meinersbur marked an inline comment as done.
Meinersbur added a comment.

- Rename -fnormalize-whitespace to -felide-unnecessary-whitespace
- error when used without -E
- Reabse


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104601/new/

https://reviews.llvm.org/D104601

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/PreprocessorOutputOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/test/CXX/lex/lex.pptoken/p3-2a.cpp
  clang/test/Preprocessor/c99-6_10_3_4_p5.c
  clang/test/Preprocessor/c99-6_10_3_4_p6.c
  clang/test/Preprocessor/comment_save.c
  clang/test/Preprocessor/elide-unnecessary-whitespace-messages.c
  clang/test/Preprocessor/elide-unnecessary-whitespace.c
  clang/test/Preprocessor/first-line-indent.c
  clang/test/Preprocessor/hash_line.c
  clang/test/Preprocessor/line-directive-output-normcol.c
  clang/test/Preprocessor/line-directive-output.c
  clang/test/Preprocessor/macro_space.c
  clang/test/Preprocessor/print_line_include.c
  clang/test/Preprocessor/stringize_space.c

Index: clang/test/Preprocessor/stringize_space.c
===
--- clang/test/Preprocessor/stringize_space.c
+++ clang/test/Preprocessor/stringize_space.c
@@ -1,16 +1,18 @@
 // RUN: %clang_cc1 -E %s | FileCheck --strict-whitespace %s
+// RUN: %clang_cc1 -E -P -felide-unnecessary-whitespace %s | FileCheck --strict-whitespace %s --check-prefix=NORMWS
 
 #define A(b) -#b  ,  - #b  ,  -# b  ,  - # b
 A()
 
 // CHECK: {{^}}-"" , - "" , -"" , - ""{{$}}
-
+// NORMWS: {{^}}-"",-"",-"",-""
 
 #define t(x) #x
 t(a
 c)
 
 // CHECK: {{^}}"a c"{{$}}
+// NORMWS-SAME: "a c"
 
 #define str(x) #x
 #define f(x) str(-x)
@@ -18,6 +20,7 @@
 1)
 
 // CHECK: {{^}}"-1"
+// NORMWS-SAME: "-1"
 
 #define paste(a,b) str(a < > <> <> < > <> < > < >
+// NORMWS: FOO1<><><><><><><><>
 
 TEST(FOO2,)
 // CHECK: FOO2 <> < > <> <> < > <> < > < >
+// NORMWS-SAME: FOO2<><><><><><><><>
 
 TEST(FOO3,)
 // CHECK: FOO3 <> < > <> <> < > <> < > < >
+// NORMWS-SAME: FOO3<><><><><><><><>
 
 TEST(FOO4,)
 // CHECK: FOO4 < > < > < > < > < > < > < > < >
+// NORMWS-SAME: FOO4<><><><><><><><>
 
 TEST(FOO5,)
 // CHECK: FOO5 < > < > < > < > < > < > < > < >
+// NORMWS-SAME: FOO5<><><><><><><><>
 
 TEST(FOO6,)
 // CHECK: FOO6 <[]> < []> <[]> <[]> <[] > <[]> <[] > < []>
+// NORMWS-SAME: FOO6<[]><[]><[]><[]><[]><[]><[]><[]>
 
 TEST(FOO7,)
 // CHECK: FOO7 <[ ]> < [ ]> <[ ]> <[ ]> <[ ] > <[ ]> <[ ] > < [ ]>
+// NORMWS-SAME: FOO7<[]><[]><[]><[]><[]><[]><[]><[]>
 
 TEST(FOO8,)
 // CHECK: FOO8 <[ ]> < [ ]> <[ ]> <[ ]> <[ ] > <[ ]> <[ ] > < [ ]>
+// NORMWS-SAME: FOO8<[]><[]><[]><[]><[]><[]><[]><[]>
Index: clang/test/Preprocessor/line-directive-output.c
===
--- clang/test/Preprocessor/line-directive-output.c
+++ clang/test/Preprocessor/line-directive-output.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -E %s 2>&1 | FileCheck %s -strict-whitespace
+// RUN: %clang_cc1 -E -felide-unnecessary-whitespace %s 2>&1 | FileCheck %s -strict-whitespace
 // PR6101
 int a;
 // CHECK: # 1 "{{.*}}line-directive-output.c"
Index: clang/test/Preprocessor/line-directive-output-normcol.c
===
--- /dev/null
+++ clang/test/Preprocessor/line-directive-output-normcol.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -E -felide-unnecessary-whitespace %s 2>&1 | FileCheck %s -strict-whitespace
+
+// CHECK:  # 6 "{{.*}}line-directive-output-normcol.c"
+// CHECK-NEXT: int x;
+// CHECK-NEXT: int y;
+int x;
+int y;
+// CHECK-NEXT: # 10 "{{.*}}line-directive-output-normcol.c"
+// CHECK-NEXT: int z;
+int z;
Index: clang/test/Preprocessor/hash_line.c
===
--- clang/test/Preprocessor/hash_line.c
+++ clang/test/Preprocessor/hash_line.c
@@ -4,6 +4,10 @@
 // CHECK-NEXT: {{^  #$}}
 // CHECK-NEXT: {{^2$}}
 // CHECK-NEXT: {{^   #$}}
+
+// RUN: %clang_cc1 -E -P -felide-unnecessary-whitespace %s | FileCheck --strict-whitespace %s --check-prefix=NORMWS
+// NORMWS:  {{^}}1#2#{{$}}
+
 #define EMPTY
 #define IDENTITY(X) X
 1
Index: clang/test/Preprocessor/first-line-indent.c
===
--- clang/test/Preprocessor/first-line-indent.c
+++ clang/test/Preprocessor/first-line-indent.c
@@ -1,7 +1,14 @@
foo
 // RUN: %clang_cc1 -E %s | FileCheck -strict-whitespace %s
+// RUN: %clang_cc1 -E -felide-unnecessary-whitespace %s | FileCheck -strict-whitespace %s --check-prefix=NORMCOL
+// RUN: %clang_cc1 -E -felide-unnecessary-whitespace -P %s | FileCheck -strict-whitespace %s --check-prefix=NORMWS
bar
 
 // CHECK: {{^   }}foo
 // CHECK: {{^   }}bar
 
+// NORMCOL: {{^}}foo
+// NORMCOL: {{^}}bar
+
+// NORMWS: 

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

2021-07-18 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur marked an inline comment as done.
Meinersbur added a comment.

In D104601#2877855 , @aaron.ballman 
wrote:

> In D104601#2848366 , @Meinersbur 
> wrote:
>
>> In D104601#2847400 , 
>> @aaron.ballman wrote:
>>
>>> ... would add that it's very common for implementers to ask developers to 
>>> run their code through `-E` mode to submit preprocessed output in order to 
>>> reproduce a customer issue with the compiler, and I worry that uses of this 
>>> flag will have unintended consequences in that scenario.
>>
>> Why would one add `-fnormalize-whitespace` for this use case?
>
> I don't think they'd *add* it, I worry it's already used by their build 
> system for reasons unknown and the developer replicates it when reporting the 
> bug because they're not looking at what flags can be removed.

I made the flag an error if used without `-E`, so the build system cannot add 
it.

Flags such as `-P`, `-fuse-line-directives`, `-frewrite-imports` and 
`-frewrite-includes` are also silently ignored without `-E`. This requirement 
seems exclusive for `-felide-unnecessary-whitespace`. It also suggests that it 
is not a problematic scenario.

>>> The "very long line" example mentioned by @dblaikie is sort of along these 
>>> lines (sorry for the bad pun).
>>
>> I can add forced line breaks (after/before 80 cols? configurable?) if 
>> requested.
>
> I don't think that's necessary just yet. If there's an issue in practice, it 
> seems like we could address it once we have the real world use in front of 
> us. However, It'd help my confidence if you were able to run this 
> functionality over a large corpus of code to see if any issues do pop out, if 
> that's plausible for you.

I ran it (using ccache) over llvm-project and the llvm-test-suite. Is there 
another large corpus do you have in mind?

> I'd like to see it diagnosed when used outside of a preprocessor invocation 
> because it really serves no purpose there.

I made the flag an error if not used without `-E`.

> I also think we should rename it because "normalize whitespace" can be 
> ambiguous depending on what context you're reading the words in.

Changed to your suggested `-felide-unnecessary-whitespace`, although I think 
name will not allow use to insert whitespace e.g. for keeping the line length 
down. If that's not required, I would prefer `-fminimize-whitespace` over 
-felide-unnecessary-whitespace`.




Comment at: clang/docs/ClangCommandLineReference.rst:2484
+-P option to normlize whitespace such that two files with only formatted
+changes are equal.
+

aaron.ballman wrote:
> Should this option be ignored when not performing a preprocessing action (and 
> documented as such)?
I changed it to an error if used without `-E`.



Comment at: clang/include/clang/Driver/Options.td:1789
   PosFlag, 
NegFlag>;
+defm normalize_whitespace : BoolFOption<"normalize-whitespace",
+  PreprocessorOutputOpts<"NormalizeWhitespace">, DefaultFalse,

aaron.ballman wrote:
> I think I'd feel most comfortable if this didn't use "normalize" in the name. 
> When I hear that, I think it's going to be doing something like converting 
> all (perhaps funky Unicode) notions of whitespace into ASCII space 
> characters. But this option doesn't actually do that -- it's removing as much 
> whitespace from the preprocessed output as possible. Would it be better named 
> as `-felide-unnecessary-whitespace` or something along those lines?
I don't think it is ambiguous what whitespace means for the preprocessor. 
Currently, `-E` already converts into `'\n'` and `'\n'` only.

I don't like the suggested name but still applied it to this patch. I'd prefer 
`-fminimize-whitespace`. However, the name would be wrong if we add additional 
whitespace such as line-breaks to avoid the long line problem, of we 
unconditionally always emit a space between tokens to not having to call 
`AvoidConcat` which would still serve the same goal.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104601/new/

https://reviews.llvm.org/D104601

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


[PATCH] D105168: [RISCV] Unify the arch string parsing logic to RISCVISAInfo.

2021-07-18 Thread Zakk Chen via Phabricator via cfe-commits
khchen added a comment.

This all looks good to me except some tidy warning.
wait for others comments.




Comment at: llvm/lib/Support/RISCVISAInfo.cpp:460
+
+addExtension("e");
+  }

nit: add `break;` to avoid the implicit-fallthrough warning.



Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:57
+}
+
 namespace {

nit: please fix this tidy warning


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105168/new/

https://reviews.llvm.org/D105168

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


[PATCH] D94098: [Clang][AArch64] Inline assembly support for the ACLE type 'data512_t'.

2021-07-18 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

In D94098#2886319 , @labrinea wrote:

> Firstly, the information that the load/store comes from an inline asm operand 
> gets lost by the time the SelectionDAG processes those nodes, and so we 
> cannot use a target hook to select a special value type for them (as 
> discussed in D94097  we want to narrow down 
> the MVT specialization for an llvm type to only apply to asm operands and not 
> universally).

We don't want a special value for the load/store operations feeding into a 
inline asm, I think?  For an input, we just want to convert the final 
insertelement to i64x8, using something like along the lines of REG_SEQUENCE.  
This means we won't use an ld64b to load the registers, but I think that's what 
we want; in general, the input registers won't come from some contiguous hunk 
of memory.  For example, say someone wrote something like this:

  struct foo { unsigned long long x[8]; };
  void store(int *in, void *addr)
  {
  struct foo x = { in[0], in[1], in[4], in[16], in[25], in[36], in[49], 
in[64] };
  __asm__ volatile ("st64b %0,[%1]" : : "r" (x), "r" (addr) : "memory" );
  }

Intuitively, I would expect this to compile to a sequence of ldr, followed by 
st64b.  But you're expecting this should compile to a sequence of ldr, followed 
by a sequence of stp, followed by an ld64b, followed by an st64b?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94098/new/

https://reviews.llvm.org/D94098

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


[PATCH] D104854: Introduce intrinsic llvm.isnan

2021-07-18 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

> The options '-ffast-math' and '-fno-honor-nans' imply that FP operation
> operands are never NaNs. This assumption however should not be applied
> to the functions that check FP number properties, including 'isnan'. If
> such function returns expected result instead of actually making
> checks, it becomes useless in many cases.

This doesn't work the way you want it to, at least given the way nnan/ninf are 
currently defined in LangRef.  It's possible to end up in a situation where 
`isnan(x) == isnan(x)` evaluates to false at runtime.  It doesn't matter how 
you compute isnan; the problem is that the input is poisoned.

I think the right solution to this sort of issue is to insert a "freeze" in the 
LLVM IR, or something like that.  Not sure how we'd expect users to write this 
in C.  Suggestions welcome.




Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:6964
+return DAG.getSetCC(DL, ResultVT, Op, DAG.getConstantFP(0.0, DL, 
OperandVT),
+ISD::SETUO);
+

Maybe we want to consider falling back to the integer path if SETCC isn't legal 
for the given operand type?  We could do that as a followup, though.



Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:6984
+  return DAG.getSetCC(DL, ResultVT, Sub, DAG.getConstant(0, DL, IntVT),
+  ISD::SETNE);
+}

Instead of emitting `ExpMaskV - AbsV != 0`, can we just emit `ExpMaskV != AbsV`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104854/new/

https://reviews.llvm.org/D104854

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


[PATCH] D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source.

2021-07-18 Thread Michael Benfield via Phabricator via cfe-commits
mbenfield updated this revision to Diff 359655.
mbenfield added a comment.

Rebased and addressed comments.

- Renamed ComputeCheckVariantSize.

- const Expr *.

- Changed existing test case to write only one excess byte.

- Added new, non-diagnosing test case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104887/new/

https://reviews.llvm.org/D104887

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Analysis/security-syntax-checks.m
  clang/test/Sema/warn-fortify-source.c

Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -58,6 +58,19 @@
   __builtin_stpncpy(s1, s2, 20); // expected-warning {{'stpncpy' size argument is too large; destination buffer has size 10, but size argument is 20}}
 }
 
+void call_strcpy() {
+  const char *const src = "abcd";
+  char dst[4];
+  __builtin_strcpy(dst, src); // expected-warning {{'strcpy' will always overflow; destination buffer has size 4, but the source string has length 5 (including NUL byte)}}
+}
+
+void call_strcpy_nowarn() {
+  const char *const src = "abcd";
+  char dst[5];
+  // We should not get a warning here.
+  __builtin_strcpy(dst, src);
+}
+
 void call_memmove() {
   char s1[10], s2[20];
   __builtin_memmove(s2, s1, 20);
Index: clang/test/Analysis/security-syntax-checks.m
===
--- clang/test/Analysis/security-syntax-checks.m
+++ clang/test/Analysis/security-syntax-checks.m
@@ -1,37 +1,37 @@
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 %s -verify \
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 %s -verify -Wno-fortify-source \
 // RUN:   -analyzer-checker=security.insecureAPI \
 // RUN:   -analyzer-checker=security.FloatLoopCounter
 
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 %s -verify \
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 %s -verify -Wno-fortify-source \
 // RUN:   -DUSE_BUILTINS \
 // RUN:   -analyzer-checker=security.insecureAPI \
 // RUN:   -analyzer-checker=security.FloatLoopCounter
 
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 %s -verify \
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 %s -verify -Wno-fortify-source \
 // RUN:   -DVARIANT \
 // RUN:   -analyzer-checker=security.insecureAPI \
 // RUN:   -analyzer-checker=security.FloatLoopCounter
 
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 %s -verify \
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 %s -verify -Wno-fortify-source \
 // RUN:   -DUSE_BUILTINS -DVARIANT \
 // RUN:   -analyzer-checker=security.insecureAPI \
 // RUN:   -analyzer-checker=security.FloatLoopCounter
 
-// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi %s -verify \
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi %s -verify -Wno-fortify-source \
 // RUN:   -analyzer-checker=security.insecureAPI \
 // RUN:   -analyzer-checker=security.FloatLoopCounter
 
-// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi %s -verify \
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi %s -verify -Wno-fortify-source \
 // RUN:   -DUSE_BUILTINS \
 // RUN:   -analyzer-checker=security.insecureAPI \
 // RUN:   -analyzer-checker=security.FloatLoopCounter
 
-// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi %s -verify \
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi %s -verify -Wno-fortify-source \
 // RUN:   -DVARIANT \
 // RUN:   -analyzer-checker=security.insecureAPI \
 // RUN:   -analyzer-checker=security.FloatLoopCounter
 
-// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi %s -verify \
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-cloudabi %s -verify -Wno-fortify-source \
 // RUN:   -DUSE_BUILTINS -DVARIANT \
 // RUN:   -analyzer-checker=security.insecureAPI \
 // RUN:   -analyzer-checker=security.FloatLoopCounter
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -588,14 +588,8 @@
 
 } // namespace
 
-/// Check a call to BuiltinID for buffer overflows. If BuiltinID is a
-/// __builtin_*_chk function, then use the object size argument specified in the
-/// source. Otherwise, infer the object size using __builtin_object_size.
 void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
CallExpr *TheCall) {
-  // FIXME: There are some more useful checks we could be doing here:
-  //  - Evaluate strlen of strcpy arguments, use as object size.
-
   if (TheCall->isValueDependent() || TheCall->isTypeDependent() ||
   isConstantEvaluated())
 return;
@@ -607,13 +601,66 @@
   const TargetInfo  = 

[PATCH] D106119: [Driver] Detect libstdc++ include paths for native gcc on 32-bit non-Debian Linux

2021-07-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 359654.
MaskRay added a comment.

fix test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106119/new/

https://reviews.llvm.org/D106119

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/Inputs/archlinux_i686_tree/lib/.keep
  
clang/test/Driver/Inputs/archlinux_i686_tree/usr/include/c++/11.1.0/backward/.keep
  
clang/test/Driver/Inputs/archlinux_i686_tree/usr/include/c++/11.1.0/i686-pc-linux-gnu/.keep
  clang/test/Driver/Inputs/archlinux_i686_tree/usr/lib/crt1.o
  clang/test/Driver/Inputs/archlinux_i686_tree/usr/lib/crti.o
  clang/test/Driver/Inputs/archlinux_i686_tree/usr/lib/crtn.o
  
clang/test/Driver/Inputs/archlinux_i686_tree/usr/lib/gcc/i686-pc-linux-gnu/11.1.0/crtbegin.o
  
clang/test/Driver/Inputs/archlinux_i686_tree/usr/lib/gcc/i686-pc-linux-gnu/11.1.0/crtend.o
  clang/test/Driver/linux-cross.cpp

Index: clang/test/Driver/linux-cross.cpp
===
--- clang/test/Driver/linux-cross.cpp
+++ clang/test/Driver/linux-cross.cpp
@@ -1,5 +1,27 @@
 // UNSUPPORTED: system-windows
 
+/// Test native GCC installation on Arch Linux i686.
+// RUN: %clang -### %s --target=i686-linux-gnu --sysroot=%S/Inputs/archlinux_i686_tree \
+// RUN:   -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --stdlib=platform --rtlib=platform 2>&1 | FileCheck %s --check-prefix=ARCH_I686
+// ARCH_I686:  "-resource-dir" "[[RESOURCE:[^"]+]]"
+// ARCH_I686:  "-internal-isystem"
+// ARCH_I686-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-pc-linux-gnu/11.1.0/../../../../include/c++/11.1.0"
+// ARCH_I686-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-pc-linux-gnu/11.1.0/../../../../include/c++/11.1.0/i686-pc-linux-gnu"
+// ARCH_I686-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-pc-linux-gnu/11.1.0/../../../../include/c++/11.1.0/backward"
+// ARCH_I686-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]/include"
+// ARCH_I686-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/local/include"
+/// This resolves to /usr/i686-linux-gnu/include. Because it does not exist,
+/// having it does no harm albeit not ideal.
+// ARCH_I686-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/lib/gcc/i686-pc-linux-gnu/11.1.0/../../../../i686-pc-linux-gnu/include"
+// ARCH_I686:  "-internal-externc-isystem"
+// ARCH_I686-SAME: {{^}} "[[SYSROOT]]/include"
+// ARCH_I686-SAME: {{^}} "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
+// ARCH_I686:  "-L
+// ARCH_I686-SAME: {{^}}[[SYSROOT]]/usr/lib/gcc/i686-pc-linux-gnu/11.1.0"
+// ARCH_I686-SAME: {{^}} "-L[[SYSROOT]]/lib"
+// ARCH_I686-SAME: {{^}} "-L[[SYSROOT]]/usr/lib"
+
 /// Test native x86-64 in the tree.
 // RUN: %clang -### %s --target=x86_64-linux-gnu --sysroot=%S/Inputs/debian_multiarch_tree \
 // RUN:   -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2941,31 +2941,27 @@
   if (!getVFS().exists(IncludeDir))
 return false;
 
+  // Debian native gcc uses g++-multiarch-incdir.diff which uses
+  // include/x86_64-linux-gnu/c++/10$IncludeSuffix instead of
+  // include/c++/10/x86_64-linux-gnu$IncludeSuffix.
+  std::string Dir = IncludeDir.str();
+  StringRef Include =
+  llvm::sys::path::parent_path(llvm::sys::path::parent_path(Dir));
+  std::string Path =
+  (Include + "/" + Triple + Dir.substr(Include.size()) + IncludeSuffix)
+  .str();
+  if (DetectDebian && !getVFS().exists(Path))
+return false;
+
   // GPLUSPLUS_INCLUDE_DIR
   addSystemInclude(DriverArgs, CC1Args, IncludeDir);
   // GPLUSPLUS_TOOL_INCLUDE_DIR. If Triple is not empty, add a target-dependent
   // include directory.
-  if (!Triple.empty()) {
-if (DetectDebian) {
-  // Debian native gcc has an awful patch g++-multiarch-incdir.diff which
-  // uses include/x86_64-linux-gnu/c++/10$IncludeSuffix instead of
-  // include/c++/10/x86_64-linux-gnu$IncludeSuffix.
-  std::string Dir = IncludeDir.str();
-  StringRef Include =
-  llvm::sys::path::parent_path(llvm::sys::path::parent_path(Dir));
-  std::string Path =
-  (Include + "/" + Triple + Dir.substr(Include.size()) + IncludeSuffix)
-  .str();
-  if (getVFS().exists(Path))
-addSystemInclude(DriverArgs, CC1Args, Path);
-  else
-addSystemInclude(DriverArgs, CC1Args,
- IncludeDir + "/" + Triple + IncludeSuffix);
-} else {
-  addSystemInclude(DriverArgs, CC1Args,
-   IncludeDir + "/" + Triple + IncludeSuffix);
-}
-  }
+  if (DetectDebian)
+addSystemInclude(DriverArgs, CC1Args, Path);
+  else if (!Triple.empty())
+

[PATCH] D94098: [Clang][AArch64] Inline assembly support for the ACLE type 'data512_t'.

2021-07-18 Thread Alexandros Lamprineas via Phabricator via cfe-commits
labrinea added a comment.

Ok, I've tried a few things. If we add a couple of new target hooks we can make 
clang pass both input and output asm operands by value as `type { [8 x i64] }` 
avoiding the integer conversion. One issue with that is that the inline asm 
verifier asserts if an inline asm statement returns a struct with one result 
(struct return types are meant to carry multiple results). By making 
adjustments to the existing target hook `adjustInlineAsmType()` we can even 
alter the asm operand type and make it `[8 x i64]` for example if that's 
preferable. Adding new calls to this hook without removing the existing ones 
will look ugly though, but at the same time I found it challenging given the 
complexity of the 400-line function `CodeGenFunction::EmitAsmStmt`, which needs 
tidying up. Unfortunately this is half of the story as by choosing an aggregate 
type for the asm operands we are allowing InstCombine (at -O1 and above) to 
turn the load/store instructions before/after the inline asm statement into 
insert/extract element + smaller loads/stores. I see two problems with that. 
Firstly, the information that the load/store comes from an inline asm operand 
gets lost by the time the SelectionDAG processes those nodes, and so we cannot 
use a target hook to select a special value type for them (as discussed in 
D94097  we want to narrow down the MVT 
specialization for an llvm type to only apply to asm operands and not 
universally). Moreover, having insert/extract element is pointless when the 
backend expects a load/store of `MVT::i64x8` for custom lowering. All that said 
I think that the best choice is to use `i512` for the asm operands since llvm 
cannot optimize that. The only change in clang's user visible behavior is that 
large aggregate output operands will not be diagnosed, like in the example at 
the description, but instead we'll be passing them by reference, which is what 
is already happening with input operands anyway.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94098/new/

https://reviews.llvm.org/D94098

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


[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-18 Thread Jon Chesterfield via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3e649f8ef187: [openmp][nfc] Simplify macros guarding math 
complex headers (authored by JonChesterfield).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105221/new/

https://reviews.llvm.org/D105221

Files:
  clang/lib/Headers/openmp_wrappers/complex
  clang/lib/Headers/openmp_wrappers/complex.h


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,7 +17,6 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,7 +17,6 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
@@ -26,9 +25,6 @@
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +44,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif


Index: clang/lib/Headers/openmp_wrappers/complex.h
===
--- clang/lib/Headers/openmp_wrappers/complex.h
+++ clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,7 +17,6 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
Index: clang/lib/Headers/openmp_wrappers/complex
===
--- clang/lib/Headers/openmp_wrappers/complex
+++ clang/lib/Headers/openmp_wrappers/complex
@@ -17,7 +17,6 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
@@ -26,9 +25,6 @@
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +44,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 3e649f8 - [openmp][nfc] Simplify macros guarding math complex headers

2021-07-18 Thread Jon Chesterfield via cfe-commits

Author: Jon Chesterfield
Date: 2021-07-18T23:30:35+01:00
New Revision: 3e649f8ef1875f943537b5fcecdb132c9442cb7d

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

LOG: [openmp][nfc] Simplify macros guarding math complex headers

The `__CUDA__` macro is already defined for openmp/nvptx and is not used by
`__clang_cuda_complex_builtins.h`, so dropping that macro slightly simplifies
nvptx and avoids defining it on amdgcn (where it is likely to be harmful).

Also dropped a cplusplus test from a C++ header as compilation will have
failed on cmath earlier if it was included from C.

Reviewed By: jdoerfert, fodinabor

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

Added: 


Modified: 
clang/lib/Headers/openmp_wrappers/complex
clang/lib/Headers/openmp_wrappers/complex.h

Removed: 




diff  --git a/clang/lib/Headers/openmp_wrappers/complex 
b/clang/lib/Headers/openmp_wrappers/complex
index 142e526b81b35..dfd6193c97cbd 100644
--- a/clang/lib/Headers/openmp_wrappers/complex
+++ b/clang/lib/Headers/openmp_wrappers/complex
@@ -17,7 +17,6 @@
 // We require std::math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__
@@ -26,9 +25,6 @@
 // Grab the host header too.
 #include_next 
 
-
-#ifdef __cplusplus
-
 // If we are compiling against libc++, the macro _LIBCPP_STD_VER should be set
 // after including  above. Since the complex header we use is a
 // simplified version of the libc++, we don't need it in this case. If we
@@ -48,5 +44,3 @@
 #pragma omp end declare variant
 
 #endif
-
-#endif

diff  --git a/clang/lib/Headers/openmp_wrappers/complex.h 
b/clang/lib/Headers/openmp_wrappers/complex.h
index 00d278548f826..15dc415b8126d 100644
--- a/clang/lib/Headers/openmp_wrappers/complex.h
+++ b/clang/lib/Headers/openmp_wrappers/complex.h
@@ -17,7 +17,6 @@
 // We require math functions in the complex builtins below.
 #include 
 
-#define __CUDA__
 #define __OPENMP_NVPTX__
 #include <__clang_cuda_complex_builtins.h>
 #undef __OPENMP_NVPTX__



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


[PATCH] D106130: [PowerPC] Implemented mtmsr, mfspr, mtspr Builtins

2021-07-18 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106130/new/

https://reviews.llvm.org/D106130

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


[PATCH] D105946: [PowerPC] Store, load, move from and to registers related builtins

2021-07-18 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105946/new/

https://reviews.llvm.org/D105946

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


[PATCH] D103986: [PowerPC] Floating Point Builtins for XL Compat.

2021-07-18 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

LGTM aside from a very minor nit.




Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:4990
   case ISD::INTRINSIC_WO_CHAIN: {
+// We emit the ppc_fsels intrinsic here because of type conflicts with the
+// comparison operand. The FSELS instruction is defined to use an 8-byte

s/ppc_fels intrinsic/PPC::FSELS instruction

since we aren't emitting an intrinsic, we are consuming it and emitting the 
instruction (well a `MachineSDNode`, but it represents an instruction).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103986/new/

https://reviews.llvm.org/D103986

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


[PATCH] D106210: [MS] Preserve base register %esi around movs[bwl]

2021-07-18 Thread namazso via Phabricator via cfe-commits
namazso added a comment.

i'd prefer it as just namazso with email ad...@namazso.eu


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106210/new/

https://reviews.llvm.org/D106210

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


[PATCH] D105907: [CallGraphSection] Add call graph section options and documentation

2021-07-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

High-level comments:

I'd use a bottom-up patch order. llvm patches are before clang patches.
The clang driver patch is the last - having the user-facing option requires the 
functionality to be fully available.
llvm-objdump patch is orthogonal to clang patches, so its order is flexible.

If the section only contains indirect call edges, perhaps it should have 
`indirect` in the option name.




Comment at: clang/docs/CallGraphSection.rst:9
+With ``-fcall-graph-section``, the compiler will create a call graph section 
+in the binary. It will include type identifiers for indirect calls and 
+targets. This information can be used to map indirect calls to their receivers 

object file



Comment at: clang/docs/CallGraphSection.rst:20
+any function in the program. This approach ensures completeness since no
+indirect call edge is missed. However, it is generally poor in precision
+due to having a much bigger than actual call graph with infeasible indirect

missing



Comment at: clang/docs/CallGraphSection.rst:21
+indirect call edge is missed. However, it is generally poor in precision
+due to having a much bigger than actual call graph with infeasible indirect
+call edges.

due to having unneeded edges 



Comment at: clang/docs/CallGraphSection.rst:29
+
+The ``llvm-objdump`` utility provides a ``-call-graph-info`` option to extract
+full call graph information by parsing the content of the call graph section

``--call-graph-info``



Comment at: clang/include/clang/Basic/CodeGenOptions.def:423
+/// Whether to emit a call graph section into the object file.
+CODEGENOPT(CallGraphSection, 1, 0)
+

make sense to move it near EmitCallSiteInfo



Comment at: clang/include/clang/Driver/Options.td:1258
   BothFlags<[CoreOption], " an address-significance table">>;
+defm call_graph_section : BoolFOption<"call-graph-section",
+  CodeGenOpts<"CallGraphSection">, DefaultFalse,

make sense to move it near stacksizessection



Comment at: clang/lib/CodeGen/BackendUtil.cpp:571
   Options.EmitAddrsig = CodeGenOpts.Addrsig;
+  Options.EmitCallGraphSection = CodeGenOpts.CallGraphSection;
   Options.ForceDwarfFrameSection = CodeGenOpts.ForceDwarfFrameSection;

make sense to move it near EmitCallSiteInfo



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6416
 
+  if (Args.hasFlag(options::OPT_fcall_graph_section,
+   options::OPT_fno_call_graph_section, false))

make sense to move it near stacksizessection


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105907/new/

https://reviews.llvm.org/D105907

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


[PATCH] D106243: [Utils] Support class template specializations in update_cc_test_checks

2021-07-18 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson accepted this revision.
arichardson added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106243/new/

https://reviews.llvm.org/D106243

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


[PATCH] D105911: [CallGraphSection] Introduce CGSectionFuncComdatCreator pass

2021-07-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1420
+
+if (CodeGenOpts.CallGraphSection) {
+  MPM.addPass(CGSectionFuncComdatCreatorPass());

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements



Comment at: llvm/lib/Transforms/Utils/CGSectionFuncComdatCreator.cpp:64
+ F.hasAddressTaken(nullptr,
+   /* IgnoreCallbackUses */ true,
+   /* IgnoreAssumeLikeCalls */ true,

clang-format prefers `/*IgnoreCallbackUses=*/true`



Comment at: llvm/lib/Transforms/Utils/CGSectionFuncComdatCreator.cpp:91
+  ModuleCGSectionFuncComdatCreator ModuleCG;
+  if (ModuleCG.instrumentModule(M)) {
+return PreservedAnalyses::none();

delete braces



Comment at: llvm/test/Transforms/Util/create-function-comdats.ll:1
+; Tests that we create function comdats properly and only for those with no
+; comdats.

This should be within a call-graph-section specific directory.

Use `test/Instrumentation/InstrProfiling/comdat.ll` as a reference


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105911/new/

https://reviews.llvm.org/D105911

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


[PATCH] D106210: [MS] Preserve base register %esi around movs[bwl]

2021-07-18 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

In D106210#2885538 , @namazso wrote:

> I don't have commit access, could someone commit this for me? Thanks in 
> advance.

I can commit it. How would you like your name to appear in the author line of 
the git log?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106210/new/

https://reviews.llvm.org/D106210

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


[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

2021-07-18 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 added a comment.

This patch causes wrong arguments in nested parallel regions. 
`openmp/libomptarget/test/offloading/bug49779.cpp` fails and after reverting 
this patch, it passed. You can easily find out the wrong pointer with a tiny 
change of the test case:

  void work(int *C) {
printf("work:  = %p\n", C);
  #pragma omp atomic
++(*C);
  }
  
  void use(int *C) {
printf("use:  = %p\n", C);
  #pragma omp parallel num_threads(2)
work(C);
  }


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102107/new/

https://reviews.llvm.org/D102107

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


[PATCH] D106243: [Utils] Support class template specializations in update_cc_test_checks

2021-07-18 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 updated this revision to Diff 359643.
jrtc27 added a comment.

Drop the --llvm-bin test; only basic-cplusplus.test does that (which happened 
to be the one I used as a reference), and that only needs to be done for one 
file as it has no relation to the input.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106243/new/

https://reviews.llvm.org/D106243

Files:
  
clang/test/utils/update_cc_test_checks/Inputs/explicit-template-instantiation.cpp
  
clang/test/utils/update_cc_test_checks/Inputs/explicit-template-instantiation.cpp.expected
  clang/test/utils/update_cc_test_checks/explicit-template-instantiation.test
  llvm/utils/update_cc_test_checks.py

Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -33,8 +33,8 @@
 '%clangxx': ['--driver-mode=g++'],
 }
 
-def get_line2spell_and_mangled(args, clang_args):
-  ret = {}
+def get_line2func_list(args, clang_args):
+  ret = collections.defaultdict(list)
   # Use clang's JSON AST dump to get the mangled name
   json_dump_args = [args.clang] + clang_args + ['-fsyntax-only', '-o', '-']
   if '-cc1' not in json_dump_args:
@@ -55,26 +55,37 @@
 
   # Parse the clang JSON and add all children of type FunctionDecl.
   # TODO: Should we add checks for global variables being emitted?
-  def parse_clang_ast_json(node):
+  def parse_clang_ast_json(node, loc, search):
 node_kind = node['kind']
 # Recurse for the following nodes that can contain nested function decls:
 if node_kind in ('NamespaceDecl', 'LinkageSpecDecl', 'TranslationUnitDecl',
- 'CXXRecordDecl'):
+ 'CXXRecordDecl', 'ClassTemplateSpecializationDecl'):
+  # Specializations must use the loc from the specialization, not the
+  # template, and search for the class's spelling as the specialization
+  # does not mention the method names in the source.
+  if node_kind == 'ClassTemplateSpecializationDecl':
+inner_loc = node['loc']
+inner_search = node['name']
+  else:
+inner_loc = None
+inner_search = None
   if 'inner' in node:
 for inner in node['inner']:
-  parse_clang_ast_json(inner)
+  parse_clang_ast_json(inner, inner_loc, inner_search)
 # Otherwise we ignore everything except functions:
 if node_kind not in ('FunctionDecl', 'CXXMethodDecl', 'CXXConstructorDecl',
  'CXXDestructorDecl', 'CXXConversionDecl'):
   return
+if loc is None:
+  loc = node['loc']
 if node.get('isImplicit') is True and node.get('storageClass') == 'extern':
-  common.debug('Skipping builtin function:', node['name'], '@', node['loc'])
+  common.debug('Skipping builtin function:', node['name'], '@', loc)
   return
-common.debug('Found function:', node['kind'], node['name'], '@', node['loc'])
-line = node['loc'].get('line')
+common.debug('Found function:', node['kind'], node['name'], '@', loc)
+line = loc.get('line')
 # If there is no line it is probably a builtin function -> skip
 if line is None:
-  common.debug('Skipping function without line number:', node['name'], '@', node['loc'])
+  common.debug('Skipping function without line number:', node['name'], '@', loc)
   return
 
 # If there is no 'inner' object, it is a function declaration and we can
@@ -88,20 +99,23 @@
   has_body = True
   break
 if not has_body:
-  common.debug('Skipping function without body:', node['name'], '@', node['loc'])
+  common.debug('Skipping function without body:', node['name'], '@', loc)
   return
 spell = node['name']
+if search is None:
+  search = spell
 mangled = node.get('mangledName', spell)
-ret[int(line)-1] = (spell, mangled)
+ret[int(line)-1].append((spell, mangled, search))
 
   ast = json.loads(stdout)
   if ast['kind'] != 'TranslationUnitDecl':
 common.error('Clang AST dump JSON format changed?')
 sys.exit(2)
-  parse_clang_ast_json(ast)
+  parse_clang_ast_json(ast, None, None)
 
-  for line, func_name in sorted(ret.items()):
-common.debug('line {}: found function {}'.format(line+1, func_name), file=sys.stderr)
+  for line, funcs in sorted(ret.items()):
+for func in funcs:
+  common.debug('line {}: found function {}'.format(line+1, func), file=sys.stderr)
   if not ret:
 common.warn('Did not find any functions using', ' '.join(json_dump_args))
   return ret
@@ -222,7 +236,7 @@
  comment_prefix='//', argparse_callback=infer_dependent_args):
 # Build a list of filechecked and non-filechecked RUN lines.
 run_list = []
-line2spell_and_mangled_list = collections.defaultdict(list)
+line2func_list = collections.defaultdict(list)
 
 subs = {
   '%s' : ti.path,
@@ -296,8 +310,8 @@
 
   # Invoke 

[PATCH] D106243: [Utils] Support class template specializations in update_cc_test_checks

2021-07-18 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 created this revision.
jrtc27 added reviewers: arichardson, jdoerfert.
jrtc27 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

ClassTemplateSpecializationDecl not within a ClassTemplateDecl
represents an explicit instatiation of a template and so should be
handled as if it were a normal CXXRecordDecl. Unfortunately, having an
equivalent for FunctionTemplateDecl remains a TODO in ASTDumper's
VisitFunctionTemplateDecl, with all the explicit instantiations just
being emitted inside the FunctionTemplateDecl along with all the other
specializations, meaning we can't easily support explicit function
instantiations in update_cc_test_checks.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106243

Files:
  
clang/test/utils/update_cc_test_checks/Inputs/explicit-template-instantiation.cpp
  
clang/test/utils/update_cc_test_checks/Inputs/explicit-template-instantiation.cpp.expected
  clang/test/utils/update_cc_test_checks/explicit-template-instantiation.test
  llvm/utils/update_cc_test_checks.py

Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -33,8 +33,8 @@
 '%clangxx': ['--driver-mode=g++'],
 }
 
-def get_line2spell_and_mangled(args, clang_args):
-  ret = {}
+def get_line2func_list(args, clang_args):
+  ret = collections.defaultdict(list)
   # Use clang's JSON AST dump to get the mangled name
   json_dump_args = [args.clang] + clang_args + ['-fsyntax-only', '-o', '-']
   if '-cc1' not in json_dump_args:
@@ -55,26 +55,37 @@
 
   # Parse the clang JSON and add all children of type FunctionDecl.
   # TODO: Should we add checks for global variables being emitted?
-  def parse_clang_ast_json(node):
+  def parse_clang_ast_json(node, loc, search):
 node_kind = node['kind']
 # Recurse for the following nodes that can contain nested function decls:
 if node_kind in ('NamespaceDecl', 'LinkageSpecDecl', 'TranslationUnitDecl',
- 'CXXRecordDecl'):
+ 'CXXRecordDecl', 'ClassTemplateSpecializationDecl'):
+  # Specializations must use the loc from the specialization, not the
+  # template, and search for the class's spelling as the specialization
+  # does not mention the method names in the source.
+  if node_kind == 'ClassTemplateSpecializationDecl':
+inner_loc = node['loc']
+inner_search = node['name']
+  else:
+inner_loc = None
+inner_search = None
   if 'inner' in node:
 for inner in node['inner']:
-  parse_clang_ast_json(inner)
+  parse_clang_ast_json(inner, inner_loc, inner_search)
 # Otherwise we ignore everything except functions:
 if node_kind not in ('FunctionDecl', 'CXXMethodDecl', 'CXXConstructorDecl',
  'CXXDestructorDecl', 'CXXConversionDecl'):
   return
+if loc is None:
+  loc = node['loc']
 if node.get('isImplicit') is True and node.get('storageClass') == 'extern':
-  common.debug('Skipping builtin function:', node['name'], '@', node['loc'])
+  common.debug('Skipping builtin function:', node['name'], '@', loc)
   return
-common.debug('Found function:', node['kind'], node['name'], '@', node['loc'])
-line = node['loc'].get('line')
+common.debug('Found function:', node['kind'], node['name'], '@', loc)
+line = loc.get('line')
 # If there is no line it is probably a builtin function -> skip
 if line is None:
-  common.debug('Skipping function without line number:', node['name'], '@', node['loc'])
+  common.debug('Skipping function without line number:', node['name'], '@', loc)
   return
 
 # If there is no 'inner' object, it is a function declaration and we can
@@ -88,20 +99,23 @@
   has_body = True
   break
 if not has_body:
-  common.debug('Skipping function without body:', node['name'], '@', node['loc'])
+  common.debug('Skipping function without body:', node['name'], '@', loc)
   return
 spell = node['name']
+if search is None:
+  search = spell
 mangled = node.get('mangledName', spell)
-ret[int(line)-1] = (spell, mangled)
+ret[int(line)-1].append((spell, mangled, search))
 
   ast = json.loads(stdout)
   if ast['kind'] != 'TranslationUnitDecl':
 common.error('Clang AST dump JSON format changed?')
 sys.exit(2)
-  parse_clang_ast_json(ast)
+  parse_clang_ast_json(ast, None, None)
 
-  for line, func_name in sorted(ret.items()):
-common.debug('line {}: found function {}'.format(line+1, func_name), file=sys.stderr)
+  for line, funcs in sorted(ret.items()):
+for func in funcs:
+  common.debug('line {}: found function {}'.format(line+1, func), file=sys.stderr)
   if not ret:
 common.warn('Did not find any functions using', ' '.join(json_dump_args))
   return ret
@@ 

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-18 Thread Joachim Meyer via Phabricator via cfe-commits
fodinabor accepted this revision.
fodinabor added a comment.
This revision is now accepted and ready to land.

LGTM as well :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105221/new/

https://reviews.llvm.org/D105221

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


[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

2021-07-18 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment.

@fodinabor?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105221/new/

https://reviews.llvm.org/D105221

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


[PATCH] D105421: [analyzer] Handle << operator for std::unique_ptr

2021-07-18 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD closed this revision.
RedDocMD added a comment.

For some reason this revision did not get automatically closed after commit, 
manually merging it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105421/new/

https://reviews.llvm.org/D105421

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


[PATCH] D103750: [analyzer] Handle std::make_unique for SmartPtrModeling

2021-07-18 Thread Deep Majumder via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd825309352b4: [analyzer] Handle std::make_unique (authored 
by RedDocMD).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103750/new/

https://reviews.llvm.org/D103750

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/test/Analysis/Inputs/system-header-simulator-cxx.h
  clang/test/Analysis/smart-ptr-text-output.cpp

Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -1,10 +1,17 @@
 // RUN: %clang_analyze_cc1\
-// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\
+// RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
+// RUN:  -analyzer-output=text -std=c++20 %s -verify=expected
+
+// RUN: %clang_analyze_cc1\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\
 // RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
 // RUN:  -analyzer-output=text -std=c++11 %s -verify=expected
 
 #include "Inputs/system-header-simulator-cxx.h"
 
+void clang_analyzer_eval(bool);
+
 class A {
 public:
   A(){};
@@ -310,3 +317,61 @@
 // expected-note@-1{{Dereference of null smart pointer 'P'}}
   }
 }
+
+void makeUniqueReturnsNonNullUniquePtr() {
+  auto P = std::make_unique();
+  if (!P) {   // expected-note {{Taking false branch}}
+P->foo(); // should have no warning here, path is impossible
+  }
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  // Now P is null
+  if (!P) {
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+#if __cplusplus >= 202002L
+
+void makeUniqueForOverwriteReturnsNullUniquePtr() {
+  auto P = std::make_unique_for_overwrite();
+  if (!P) {   // expected-note {{Taking false branch}}
+P->foo(); // should have no warning here, path is impossible
+  }
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  // Now P is null
+  if (!P) {
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+#endif
+
+struct G {
+  int *p;
+  G(int *p): p(p) {}
+  ~G() { *p = 0; }
+};
+
+void foo() {
+  int x = 1;
+  {
+auto P = std::make_unique();
+// FIXME: There should not be a state split here, it should take the true path.
+clang_analyzer_eval(*P->p == 1); // expected-warning {{TRUE}}
+// expected-warning@-1 {{FALSE}}
+// expected-note@-2 {{Assuming the condition is true}}
+// expected-note@-3 {{Assuming the condition is false}}
+// expected-note@-4 {{TRUE}}
+// expected-note@-5 {{FALSE}}
+// expected-note@-6 {{Assuming the condition is false}}
+  }
+  // FIXME: Should be fixed when unique_ptr desctructors are
+  // properly modelled. This includes modelling the call to
+  // the destructor of the inner pointer type.
+  clang_analyzer_eval(x == 0); // expected-warning {{FALSE}}
+  // expected-note@-1 {{FALSE}}
+}
Index: clang/test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -1033,6 +1033,16 @@
 template 
 bool operator<=(nullptr_t x, const unique_ptr );
 
+template 
+unique_ptr make_unique(Args &&...args);
+
+#if __cplusplus >= 202002L
+
+template 
+unique_ptr make_unique_for_overwrite();
+
+#endif
+
 } // namespace std
 #endif
 
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -192,12 +192,19 @@
   const LocationContext *LCtx,
   unsigned VisitCount) {
   QualType T = E->getType();
-  assert(Loc::isLocType(T));
-  assert(SymbolManager::canSymbolicate(T));
-  if (T->isNullPtrType())
-return makeZeroVal(T);
+  return getConjuredHeapSymbolVal(E, LCtx, T, VisitCount);
+}
+
+DefinedOrUnknownSVal
+SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
+  const LocationContext *LCtx,
+  

[clang] d825309 - [analyzer] Handle std::make_unique

2021-07-18 Thread Deep Majumder via cfe-commits

Author: Deep Majumder
Date: 2021-07-18T19:54:28+05:30
New Revision: d825309352b4c5c01da7c935e49994f8f257

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

LOG: [analyzer] Handle std::make_unique

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
clang/test/Analysis/Inputs/system-header-simulator-cxx.h
clang/test/Analysis/smart-ptr-text-output.cpp

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 64c61a3514be..87a49cf4ffe9 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -238,6 +238,14 @@ class SValBuilder {
 const LocationContext *LCtx,
 unsigned Count);
 
+  /// Conjure a symbol representing heap allocated memory region.
+  ///
+  /// Note, now, the expression *doesn't* need to represent a location.
+  /// But the type need to!
+  DefinedOrUnknownSVal getConjuredHeapSymbolVal(const Expr *E,
+const LocationContext *LCtx,
+QualType type, unsigned Count);
+
   DefinedOrUnknownSVal getDerivedRegionValueSymbolVal(
   SymbolRef parentSymbol, const TypedValueRegion *region);
 

diff  --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
index 2793980de1b6..253606b97ec6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -38,6 +38,7 @@ using namespace clang;
 using namespace ento;
 
 namespace {
+
 class SmartPtrModeling
 : public Checker {
@@ -88,6 +89,9 @@ class SmartPtrModeling
   {{"swap", 1}, ::handleSwapMethod},
   {{"get"}, ::handleGet}};
   const CallDescription StdSwapCall{{"std", "swap"}, 2};
+  const CallDescription StdMakeUniqueCall{{"std", "make_unique"}};
+  const CallDescription StdMakeUniqueForOverwriteCall{
+  {"std", "make_unique_for_overwrite"}};
 };
 } // end of anonymous namespace
 
@@ -187,12 +191,27 @@ static QualType getInnerPointerType(CheckerContext C, 
const CXXRecordDecl *RD) {
 return {};
 
   auto TemplateArgs = TSD->getTemplateArgs().asArray();
-  if (TemplateArgs.size() == 0)
+  if (TemplateArgs.empty())
 return {};
   auto InnerValueType = TemplateArgs[0].getAsType();
   return C.getASTContext().getPointerType(InnerValueType.getCanonicalType());
 }
 
+// This is for use with standalone-functions like std::make_unique,
+// std::make_unique_for_overwrite, etc. It reads the template parameter and
+// returns the pointer type corresponding to it,
+static QualType getPointerTypeFromTemplateArg(const CallEvent ,
+  CheckerContext ) {
+  const auto *FD = dyn_cast_or_null(Call.getDecl());
+  if (!FD || !FD->isFunctionTemplateSpecialization())
+return {};
+  const auto  = FD->getTemplateSpecializationArgs()->asArray();
+  if (TemplateArgs.size() == 0)
+return {};
+  auto ValueType = TemplateArgs[0].getAsType();
+  return C.getASTContext().getPointerType(ValueType.getCanonicalType());
+}
+
 // Helper method to get the inner pointer type of specialized smart pointer
 // Returns empty type if not found valid inner pointer type.
 static QualType getInnerPointerType(const CallEvent , CheckerContext ) {
@@ -248,6 +267,7 @@ bool isStdOstreamOperatorCall(const CallEvent ) {
 
 bool SmartPtrModeling::evalCall(const CallEvent ,
 CheckerContext ) const {
+
   ProgramStateRef State = C.getState();
 
   // If any one of the arg is a unique_ptr, then
@@ -271,6 +291,50 @@ bool SmartPtrModeling::evalCall(const CallEvent ,
 return handleSwap(State, Call.getArgSVal(0), Call.getArgSVal(1), C);
   }
 
+  if (Call.isCalled(StdMakeUniqueCall) ||
+  Call.isCalled(StdMakeUniqueForOverwriteCall)) {
+if (!ModelSmartPtrDereference)
+  return false;
+
+const Optional ThisRegionOpt = 
Call.getReturnValueUnderConstruction();
+if (!ThisRegionOpt)
+  return false;
+
+const auto PtrVal = C.getSValBuilder().getConjuredHeapSymbolVal(
+Call.getOriginExpr(), C.getLocationContext(),
+getPointerTypeFromTemplateArg(Call, C), C.blockCount());
+
+const MemRegion *ThisRegion = ThisRegionOpt->getAsRegion();
+State = State->set(ThisRegion, PtrVal);
+State = 

[PATCH] D103750: [analyzer] Handle std::make_unique for SmartPtrModeling

2021-07-18 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD updated this revision to Diff 359627.
RedDocMD added a comment.

Marked test with FIXME notes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103750/new/

https://reviews.llvm.org/D103750

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/test/Analysis/Inputs/system-header-simulator-cxx.h
  clang/test/Analysis/smart-ptr-text-output.cpp

Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -1,10 +1,17 @@
 // RUN: %clang_analyze_cc1\
-// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\
+// RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
+// RUN:  -analyzer-output=text -std=c++20 %s -verify=expected
+
+// RUN: %clang_analyze_cc1\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\
 // RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
 // RUN:  -analyzer-output=text -std=c++11 %s -verify=expected
 
 #include "Inputs/system-header-simulator-cxx.h"
 
+void clang_analyzer_eval(bool);
+
 class A {
 public:
   A(){};
@@ -310,3 +317,61 @@
 // expected-note@-1{{Dereference of null smart pointer 'P'}}
   }
 }
+
+void makeUniqueReturnsNonNullUniquePtr() {
+  auto P = std::make_unique();
+  if (!P) {   // expected-note {{Taking false branch}}
+P->foo(); // should have no warning here, path is impossible
+  }
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  // Now P is null
+  if (!P) {
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+#if __cplusplus >= 202002L
+
+void makeUniqueForOverwriteReturnsNullUniquePtr() {
+  auto P = std::make_unique_for_overwrite();
+  if (!P) {   // expected-note {{Taking false branch}}
+P->foo(); // should have no warning here, path is impossible
+  }
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  // Now P is null
+  if (!P) {
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+#endif
+
+struct G {
+  int *p;
+  G(int *p): p(p) {}
+  ~G() { *p = 0; }
+};
+
+void foo() {
+  int x = 1;
+  {
+auto P = std::make_unique();
+// FIXME: There should not be a state split here, it should take the true path.
+clang_analyzer_eval(*P->p == 1); // expected-warning {{TRUE}}
+// expected-warning@-1 {{FALSE}}
+// expected-note@-2 {{Assuming the condition is true}}
+// expected-note@-3 {{Assuming the condition is false}}
+// expected-note@-4 {{TRUE}}
+// expected-note@-5 {{FALSE}}
+// expected-note@-6 {{Assuming the condition is false}}
+  }
+  // FIXME: Should be fixed when unique_ptr desctructors are
+  // properly modelled. This includes modelling the call to
+  // the destructor of the inner pointer type.
+  clang_analyzer_eval(x == 0); // expected-warning {{FALSE}}
+  // expected-note@-1 {{FALSE}}
+}
Index: clang/test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -1033,6 +1033,16 @@
 template 
 bool operator<=(nullptr_t x, const unique_ptr );
 
+template 
+unique_ptr make_unique(Args &&...args);
+
+#if __cplusplus >= 202002L
+
+template 
+unique_ptr make_unique_for_overwrite();
+
+#endif
+
 } // namespace std
 #endif
 
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -192,12 +192,19 @@
   const LocationContext *LCtx,
   unsigned VisitCount) {
   QualType T = E->getType();
-  assert(Loc::isLocType(T));
-  assert(SymbolManager::canSymbolicate(T));
-  if (T->isNullPtrType())
-return makeZeroVal(T);
+  return getConjuredHeapSymbolVal(E, LCtx, T, VisitCount);
+}
+
+DefinedOrUnknownSVal
+SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
+  const LocationContext *LCtx,
+  QualType type, unsigned VisitCount) {
+  assert(Loc::isLocType(type));
+  

[PATCH] D103750: [analyzer] Handle std::make_unique for SmartPtrModeling

2021-07-18 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD updated this revision to Diff 359625.
RedDocMD added a comment.

Fixed up tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103750/new/

https://reviews.llvm.org/D103750

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/test/Analysis/Inputs/system-header-simulator-cxx.h
  clang/test/Analysis/smart-ptr-text-output.cpp

Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -1,10 +1,17 @@
 // RUN: %clang_analyze_cc1\
-// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\
+// RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
+// RUN:  -analyzer-output=text -std=c++20 %s -verify=expected
+
+// RUN: %clang_analyze_cc1\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\
 // RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
 // RUN:  -analyzer-output=text -std=c++11 %s -verify=expected
 
 #include "Inputs/system-header-simulator-cxx.h"
 
+void clang_analyzer_eval(bool);
+
 class A {
 public:
   A(){};
@@ -310,3 +317,57 @@
 // expected-note@-1{{Dereference of null smart pointer 'P'}}
   }
 }
+
+void makeUniqueReturnsNonNullUniquePtr() {
+  auto P = std::make_unique();
+  if (!P) {   // expected-note {{Taking false branch}}
+P->foo(); // should have no warning here, path is impossible
+  }
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  // Now P is null
+  if (!P) {
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+#if __cplusplus >= 202002L
+
+void makeUniqueForOverwriteReturnsNullUniquePtr() {
+  auto P = std::make_unique_for_overwrite();
+  if (!P) {   // expected-note {{Taking false branch}}
+P->foo(); // should have no warning here, path is impossible
+  }
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  // Now P is null
+  if (!P) {
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+#endif
+
+struct G {
+  int *p;
+  G(int *p): p(p) {}
+  ~G() { *p = 0; }
+};
+
+void foo() {
+  int x = 1;
+  {
+auto P = std::make_unique();
+clang_analyzer_eval(*P->p == 1); // expected-warning {{TRUE}}
+// expected-warning@-1 {{FALSE}}
+// expected-note@-2 {{Assuming the condition is true}}
+// expected-note@-3 {{Assuming the condition is false}}
+// expected-note@-4 {{TRUE}}
+// expected-note@-5 {{FALSE}}
+// expected-note@-6 {{Assuming the condition is false}}
+  }
+  clang_analyzer_eval(x == 0); // expected-warning {{FALSE}}
+  // expected-note@-1 {{FALSE}}
+}
Index: clang/test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -1033,6 +1033,16 @@
 template 
 bool operator<=(nullptr_t x, const unique_ptr );
 
+template 
+unique_ptr make_unique(Args &&...args);
+
+#if __cplusplus >= 202002L
+
+template 
+unique_ptr make_unique_for_overwrite();
+
+#endif
+
 } // namespace std
 #endif
 
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -192,12 +192,19 @@
   const LocationContext *LCtx,
   unsigned VisitCount) {
   QualType T = E->getType();
-  assert(Loc::isLocType(T));
-  assert(SymbolManager::canSymbolicate(T));
-  if (T->isNullPtrType())
-return makeZeroVal(T);
+  return getConjuredHeapSymbolVal(E, LCtx, T, VisitCount);
+}
+
+DefinedOrUnknownSVal
+SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
+  const LocationContext *LCtx,
+  QualType type, unsigned VisitCount) {
+  assert(Loc::isLocType(type));
+  assert(SymbolManager::canSymbolicate(type));
+  if (type->isNullPtrType())
+return makeZeroVal(type);
 
-  SymbolRef sym = SymMgr.conjureSymbol(E, LCtx, T, VisitCount);
+  SymbolRef sym = SymMgr.conjureSymbol(E, LCtx, type, VisitCount);
   return 

[PATCH] D103750: [analyzer] Handle std::make_unique for SmartPtrModeling

2021-07-18 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD updated this revision to Diff 359605.
RedDocMD added a comment.

Post-rebase cleanup


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103750/new/

https://reviews.llvm.org/D103750

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/test/Analysis/Inputs/system-header-simulator-cxx.h
  clang/test/Analysis/smart-ptr-text-output.cpp

Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -1,10 +1,17 @@
 // RUN: %clang_analyze_cc1\
-// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\
+// RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
+// RUN:  -analyzer-output=text -std=c++20 %s -verify=expected
+
+// RUN: %clang_analyze_cc1\
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr,debug.ExprInspection\
 // RUN:  -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
 // RUN:  -analyzer-output=text -std=c++11 %s -verify=expected
 
 #include "Inputs/system-header-simulator-cxx.h"
 
+void clang_analyzer_eval(bool);
+
 class A {
 public:
   A(){};
@@ -310,3 +317,55 @@
 // expected-note@-1{{Dereference of null smart pointer 'P'}}
   }
 }
+
+void makeUniqueReturnsNonNullUniquePtr() {
+  auto P = std::make_unique();
+  if (!P) {   // expected-note {{Taking false branch}}
+P->foo(); // should have no warning here, path is impossible
+  }
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  // Now P is null
+  if (!P) {
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+#if __cplusplus >= 202002L
+
+void makeUniqueForOverwriteReturnsNullUniquePtr() {
+  auto P = std::make_unique_for_overwrite();
+  if (!P) {   // expected-note {{Taking false branch}}
+P->foo(); // should have no warning here, path is impossible
+  }
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  // Now P is null
+  if (!P) {
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+#endif
+
+struct G {
+  int *p;
+  G(int *p): p(p) {}
+  ~G() { *p = 0; }
+};
+
+void foo() {
+  int x = 1;
+  {
+auto P = std::make_unique();
+clang_analyzer_eval(*P->p == 1); // expected-warning {{TRUE}}
+// expected-note@-1 {{Assuming the condition is false}}
+// expected-note@-2 {{FALSE}}
+// expected-note@-3 {{Assuming the condition is true}}
+// expected-note@-4 {{TRUE}}
+  }
+  clang_analyzer_eval(x == 0); // expected-warning {{FALSE}}
+  // expected-note@-1 {{FALSE}}
+}
Index: clang/test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -1033,6 +1033,16 @@
 template 
 bool operator<=(nullptr_t x, const unique_ptr );
 
+template 
+unique_ptr make_unique(Args &&...args);
+
+#if __cplusplus >= 202002L
+
+template 
+unique_ptr make_unique_for_overwrite();
+
+#endif
+
 } // namespace std
 #endif
 
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -192,12 +192,19 @@
   const LocationContext *LCtx,
   unsigned VisitCount) {
   QualType T = E->getType();
-  assert(Loc::isLocType(T));
-  assert(SymbolManager::canSymbolicate(T));
-  if (T->isNullPtrType())
-return makeZeroVal(T);
+  return getConjuredHeapSymbolVal(E, LCtx, T, VisitCount);
+}
+
+DefinedOrUnknownSVal
+SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
+  const LocationContext *LCtx,
+  QualType type, unsigned VisitCount) {
+  assert(Loc::isLocType(type));
+  assert(SymbolManager::canSymbolicate(type));
+  if (type->isNullPtrType())
+return makeZeroVal(type);
 
-  SymbolRef sym = SymMgr.conjureSymbol(E, LCtx, T, VisitCount);
+  SymbolRef sym = SymMgr.conjureSymbol(E, LCtx, type, VisitCount);
   return loc::MemRegionVal(MemMgr.getSymbolicHeapRegion(sym));
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

[PATCH] D104300: [analyzer] Handle std::swap for std::unique_ptr

2021-07-18 Thread Deep Majumder via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0cd98bef1b6f: [analyzer] Handle std::swap for 
std::unique_ptr (authored by RedDocMD).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104300/new/

https://reviews.llvm.org/D104300

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/test/Analysis/smart-ptr-text-output.cpp

Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -69,20 +69,17 @@
 
 void derefOnSwappedNullPtr() {
   std::unique_ptr P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
-  std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
-  P.swap(PNull); // expected-note {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
+  std::unique_ptr PNull;
+  P.swap(PNull);
   PNull->foo(); // No warning.
   (*P).foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
 
-// FIXME: Fix this test when "std::swap" is modeled seperately.
 void derefOnStdSwappedNullPtr() {
   std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
-  std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
-  std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:979 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
-  // expected-note@-1 {{Calling 'swap'}}
-  // expected-note@-2 {{Returning from 'swap'}}
+  std::unique_ptr PNull;
+  std::swap(P, PNull);
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2263,6 +2263,8 @@
   // The metadata part of markInteresting is not reversed here.
   // Just making the same region not interesting is incorrect
   // in specific cases.
+  if (const auto *meta = dyn_cast(sym))
+markNotInteresting(meta->getRegion());
 }
 
 void PathSensitiveBugReport::markInteresting(const MemRegion *R,
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -63,7 +63,7 @@
 private:
   void handleReset(const CallEvent , CheckerContext ) const;
   void handleRelease(const CallEvent , CheckerContext ) const;
-  void handleSwap(const CallEvent , CheckerContext ) const;
+  void handleSwapMethod(const CallEvent , CheckerContext ) const;
   void handleGet(const CallEvent , CheckerContext ) const;
   bool handleAssignOp(const CallEvent , CheckerContext ) const;
   bool handleMoveCtr(const CallEvent , CheckerContext ,
@@ -73,6 +73,8 @@
   void handleBoolConversion(const CallEvent , CheckerContext ) const;
   bool handleComparisionOp(const CallEvent , CheckerContext ) const;
   bool handleOstreamOperator(const CallEvent , CheckerContext ) const;
+  bool handleSwap(ProgramStateRef State, SVal First, SVal Second,
+  CheckerContext ) const;
   std::pair
   retrieveOrConjureInnerPtrVal(ProgramStateRef State,
const MemRegion *ThisRegion, const Expr *E,
@@ -83,8 +85,9 @@
   CallDescriptionMap SmartPtrMethodHandlers{
   {{"reset"}, ::handleReset},
   {{"release"}, ::handleRelease},
-  {{"swap", 1}, ::handleSwap},
+  {{"swap", 1}, ::handleSwapMethod},
   {{"get"}, ::handleGet}};
+  const CallDescription StdSwapCall{{"std", "swap"}, 2};
 };
 } // end of anonymous namespace
 
@@ -259,6 +262,15 @@
   if (isStdOstreamOperatorCall(Call))
 return handleOstreamOperator(Call, C);
 
+  if (Call.isCalled(StdSwapCall)) {
+// Check the first arg, if it is of std::unique_ptr type.
+assert(Call.getNumArgs() == 2 && "std::swap should have two arguments");
+const Expr *FirstArg = Call.getArgExpr(0);
+if (!smartptr::isStdSmartPtr(FirstArg->getType()->getAsCXXRecordDecl()))
+  return false;
+return handleSwap(State, Call.getArgSVal(0), Call.getArgSVal(1), C);
+  }
+
   if (!smartptr::isStdSmartPtrCall(Call))
 return false;
 
@@ -578,43 +590,52 @@
   // pointer.
 }
 
-void SmartPtrModeling::handleSwap(const CallEvent ,
-  CheckerContext ) const {
+void 

[clang] 0cd98be - [analyzer] Handle std::swap for std::unique_ptr

2021-07-18 Thread Deep Majumder via cfe-commits

Author: Deep Majumder
Date: 2021-07-18T14:38:55+05:30
New Revision: 0cd98bef1b6feec067a4c60df4df4d44a842811d

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

LOG: [analyzer] Handle std::swap for std::unique_ptr

This patch handles the `std::swap` function specialization
for `std::unique_ptr`. Implemented to be very similar to
how `swap` method is handled

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/test/Analysis/smart-ptr-text-output.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h 
b/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
index b4352b450c7fa..6a40f8eda5fa8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
@@ -25,6 +25,8 @@ bool isStdSmartPtrCall(const CallEvent );
 bool isStdSmartPtr(const CXXRecordDecl *RD);
 bool isStdSmartPtr(const Expr *E);
 
+bool isStdSmartPtr(const CXXRecordDecl *RD);
+
 /// Returns whether the smart pointer is null or not.
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion);
 

diff  --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
index b56c969ca5a43..2793980de1b60 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -63,7 +63,7 @@ class SmartPtrModeling
 private:
   void handleReset(const CallEvent , CheckerContext ) const;
   void handleRelease(const CallEvent , CheckerContext ) const;
-  void handleSwap(const CallEvent , CheckerContext ) const;
+  void handleSwapMethod(const CallEvent , CheckerContext ) const;
   void handleGet(const CallEvent , CheckerContext ) const;
   bool handleAssignOp(const CallEvent , CheckerContext ) const;
   bool handleMoveCtr(const CallEvent , CheckerContext ,
@@ -73,6 +73,8 @@ class SmartPtrModeling
   void handleBoolConversion(const CallEvent , CheckerContext ) const;
   bool handleComparisionOp(const CallEvent , CheckerContext ) const;
   bool handleOstreamOperator(const CallEvent , CheckerContext ) const;
+  bool handleSwap(ProgramStateRef State, SVal First, SVal Second,
+  CheckerContext ) const;
   std::pair
   retrieveOrConjureInnerPtrVal(ProgramStateRef State,
const MemRegion *ThisRegion, const Expr *E,
@@ -83,8 +85,9 @@ class SmartPtrModeling
   CallDescriptionMap SmartPtrMethodHandlers{
   {{"reset"}, ::handleReset},
   {{"release"}, ::handleRelease},
-  {{"swap", 1}, ::handleSwap},
+  {{"swap", 1}, ::handleSwapMethod},
   {{"get"}, ::handleGet}};
+  const CallDescription StdSwapCall{{"std", "swap"}, 2};
 };
 } // end of anonymous namespace
 
@@ -259,6 +262,15 @@ bool SmartPtrModeling::evalCall(const CallEvent ,
   if (isStdOstreamOperatorCall(Call))
 return handleOstreamOperator(Call, C);
 
+  if (Call.isCalled(StdSwapCall)) {
+// Check the first arg, if it is of std::unique_ptr type.
+assert(Call.getNumArgs() == 2 && "std::swap should have two arguments");
+const Expr *FirstArg = Call.getArgExpr(0);
+if (!smartptr::isStdSmartPtr(FirstArg->getType()->getAsCXXRecordDecl()))
+  return false;
+return handleSwap(State, Call.getArgSVal(0), Call.getArgSVal(1), C);
+  }
+
   if (!smartptr::isStdSmartPtrCall(Call))
 return false;
 
@@ -578,43 +590,52 @@ void SmartPtrModeling::handleRelease(const CallEvent 
,
   // pointer.
 }
 
-void SmartPtrModeling::handleSwap(const CallEvent ,
-  CheckerContext ) const {
+void SmartPtrModeling::handleSwapMethod(const CallEvent ,
+CheckerContext ) const {
   // To model unique_ptr::swap() method.
   const auto *IC = dyn_cast();
   if (!IC)
 return;
 
-  const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
-  if (!ThisRegion)
-return;
+  auto State = C.getState();
+  handleSwap(State, IC->getCXXThisVal(), Call.getArgSVal(0), C);
+}
 
-  const auto *ArgRegion = Call.getArgSVal(0).getAsRegion();
-  if (!ArgRegion)
-return;
+bool SmartPtrModeling::handleSwap(ProgramStateRef State, SVal First,
+  SVal Second, CheckerContext ) const {
+  const MemRegion *FirstThisRegion = First.getAsRegion();
+  if (!FirstThisRegion)
+return false;
+  const MemRegion *SecondThisRegion = Second.getAsRegion();
+  if (!SecondThisRegion)
+return false;
 
-  auto State = C.getState();
-  const auto *ThisRegionInnerPointerVal =
-  State->get(ThisRegion);
-  const auto *ArgRegionInnerPointerVal =
-  

[PATCH] D104300: [analyzer] Handle std::swap for std::unique_ptr

2021-07-18 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD updated this revision to Diff 359600.
RedDocMD added a comment.

Post rebase cleanup


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104300/new/

https://reviews.llvm.org/D104300

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/test/Analysis/smart-ptr-text-output.cpp

Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -69,20 +69,17 @@
 
 void derefOnSwappedNullPtr() {
   std::unique_ptr P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
-  std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
-  P.swap(PNull); // expected-note {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
+  std::unique_ptr PNull;
+  P.swap(PNull);
   PNull->foo(); // No warning.
   (*P).foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
 
-// FIXME: Fix this test when "std::swap" is modeled seperately.
 void derefOnStdSwappedNullPtr() {
   std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
-  std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
-  std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:979 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
-  // expected-note@-1 {{Calling 'swap'}}
-  // expected-note@-2 {{Returning from 'swap'}}
+  std::unique_ptr PNull;
+  std::swap(P, PNull);
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2263,6 +2263,8 @@
   // The metadata part of markInteresting is not reversed here.
   // Just making the same region not interesting is incorrect
   // in specific cases.
+  if (const auto *meta = dyn_cast(sym))
+markNotInteresting(meta->getRegion());
 }
 
 void PathSensitiveBugReport::markInteresting(const MemRegion *R,
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -63,7 +63,7 @@
 private:
   void handleReset(const CallEvent , CheckerContext ) const;
   void handleRelease(const CallEvent , CheckerContext ) const;
-  void handleSwap(const CallEvent , CheckerContext ) const;
+  void handleSwapMethod(const CallEvent , CheckerContext ) const;
   void handleGet(const CallEvent , CheckerContext ) const;
   bool handleAssignOp(const CallEvent , CheckerContext ) const;
   bool handleMoveCtr(const CallEvent , CheckerContext ,
@@ -73,6 +73,8 @@
   void handleBoolConversion(const CallEvent , CheckerContext ) const;
   bool handleComparisionOp(const CallEvent , CheckerContext ) const;
   bool handleOstreamOperator(const CallEvent , CheckerContext ) const;
+  bool handleSwap(ProgramStateRef State, SVal First, SVal Second,
+  CheckerContext ) const;
   std::pair
   retrieveOrConjureInnerPtrVal(ProgramStateRef State,
const MemRegion *ThisRegion, const Expr *E,
@@ -83,8 +85,9 @@
   CallDescriptionMap SmartPtrMethodHandlers{
   {{"reset"}, ::handleReset},
   {{"release"}, ::handleRelease},
-  {{"swap", 1}, ::handleSwap},
+  {{"swap", 1}, ::handleSwapMethod},
   {{"get"}, ::handleGet}};
+  const CallDescription StdSwapCall{{"std", "swap"}, 2};
 };
 } // end of anonymous namespace
 
@@ -259,6 +262,15 @@
   if (isStdOstreamOperatorCall(Call))
 return handleOstreamOperator(Call, C);
 
+  if (Call.isCalled(StdSwapCall)) {
+// Check the first arg, if it is of std::unique_ptr type.
+assert(Call.getNumArgs() == 2 && "std::swap should have two arguments");
+const Expr *FirstArg = Call.getArgExpr(0);
+if (!smartptr::isStdSmartPtr(FirstArg->getType()->getAsCXXRecordDecl()))
+  return false;
+return handleSwap(State, Call.getArgSVal(0), Call.getArgSVal(1), C);
+  }
+
   if (!smartptr::isStdSmartPtrCall(Call))
 return false;
 
@@ -578,43 +590,52 @@
   // pointer.
 }
 
-void SmartPtrModeling::handleSwap(const CallEvent ,
-  CheckerContext ) const {
+void SmartPtrModeling::handleSwapMethod(const CallEvent ,
+CheckerContext ) const {
   // To model unique_ptr::swap() method.
   const auto *IC = 

[clang] dac582a - DebugInfo: Name class templates with default arguments consistently (both direct naming, and as a template argument for a function template)

2021-07-18 Thread David Blaikie via cfe-commits

Author: David Blaikie
Date: 2021-07-17T23:58:15-07:00
New Revision: dac582ad3a78b18bdd2e6615f1ec105ee05adfe1

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

LOG: DebugInfo: Name class templates with default arguments consistently (both 
direct naming, and as a template argument for a function template)

It's noteworthy that GCC has the same bug here, which is a bit
surprising. Both Clang and GCC's bug is only for function template
arguments that are themselves templates with default template arguments
(f1>). Probably because function name
matching isn't generally necessary - whereas type matching is necessary
for DWARF consumers to associate declarations and definitions across
translation units, so the bug's been addressed there already - but
continued to exist for function templates since it's fairly benign
there.

I came across this while working on a change that could reconstitute
these pretty printed names based on the rest of the DWARF, reducing the
size of the DWARF by not having to encode all the template parameters in
the name string. That reconstitution code can't tell the difference
between a defaulted argument or not, so couldn't create the current
buggy-ish output.

Making the names more consistent between direct and indirect references,
and between function and class templates seems all to the good.

(I fixed the function template version of this a few years back in
9fdd09a4ccd01feb8e00be22b17e944e46807746 - clearly I should've looked
more closely and generalized the code better so it only had to be fixed
once - well, doing that here now)

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
clang/test/CodeGenCXX/debug-info-template.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index e4c3af07e664..432e2400a440 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -249,26 +249,7 @@ PrintingPolicy CGDebugInfo::getPrintingPolicy() const {
 }
 
 StringRef CGDebugInfo::getFunctionName(const FunctionDecl *FD) {
-  assert(FD && "Invalid FunctionDecl!");
-  IdentifierInfo *FII = FD->getIdentifier();
-  FunctionTemplateSpecializationInfo *Info =
-  FD->getTemplateSpecializationInfo();
-
-  if (!Info && FII)
-return FII->getName();
-
-  SmallString<128> NS;
-  llvm::raw_svector_ostream OS(NS);
-  FD->printName(OS);
-
-  // Add any template specialization args.
-  if (Info) {
-const TemplateArgumentList *TArgs = Info->TemplateArguments;
-printTemplateArgumentList(OS, TArgs->asArray(), getPrintingPolicy());
-  }
-
-  // Copy this name on the side and use its reference.
-  return internString(OS.str());
+  return internString(GetName(FD));
 }
 
 StringRef CGDebugInfo::getObjCMethodName(const ObjCMethodDecl *OMD) {
@@ -301,16 +282,8 @@ StringRef CGDebugInfo::getSelectorName(Selector S) {
 
 StringRef CGDebugInfo::getClassName(const RecordDecl *RD) {
   if (isa(RD)) {
-SmallString<128> Name;
-llvm::raw_svector_ostream OS(Name);
-PrintingPolicy PP = getPrintingPolicy();
-PP.PrintCanonicalTypes = true;
-PP.SuppressInlineNamespace = false;
-RD->getNameForDiagnostic(OS, PP,
- /*Qualified*/ false);
-
 // Copy this name on the side and use its reference.
-return internString(Name);
+return internString(GetName(RD));
   }
 
   // quick optimization to avoid having to intern strings that are already
@@ -4003,12 +3976,7 @@ void CGDebugInfo::EmitFunctionDecl(GlobalDecl GD, 
SourceLocation Loc,
 return;
 
   llvm::TimeTraceScope TimeScope("DebugFunction", [&]() {
-std::string Name;
-llvm::raw_string_ostream OS(Name);
-if (const NamedDecl *ND = dyn_cast(D))
-  ND->getNameForDiagnostic(OS, getPrintingPolicy(),
-   /*Qualified=*/true);
-return Name;
+return GetName(D, true);
   });
 
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagZero;
@@ -4776,6 +4744,18 @@ llvm::DIGlobalVariableExpression 
*CGDebugInfo::CollectAnonRecordDecls(
   return GVE;
 }
 
+std::string CGDebugInfo::GetName(const Decl *D, bool Qualified) const {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  if (const NamedDecl *ND = dyn_cast(D)) {
+PrintingPolicy PP = getPrintingPolicy();
+PP.PrintCanonicalTypes = true;
+PP.SuppressInlineNamespace = false;
+ND->getNameForDiagnostic(OS, PP, Qualified);
+  }
+  return Name;
+}
+
 void CGDebugInfo::EmitGlobalVariable(llvm::GlobalVariable *Var,
  const VarDecl *D) {
   assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
@@ -4783,11 +4763,7 @@ void 
CGDebugInfo::EmitGlobalVariable(llvm::GlobalVariable *Var,
 return;
 
   llvm::TimeTraceScope