[PATCH] D54473: [sanitizers] Initial implementation for -fsanitize=init-locals

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
glider added reviewers: kcc, rjmccall, rsmith.
Herald added a subscriber: cfe-commits.

This patch adds a new feature, -fsanitize=init-locals, which generates zero 
initializers for uninitialized local variables.

There's been discussions in the security community about the impact of 
zero-initializing all locals to prevent information leaks. The new feature 
shall help evaluating the pros and cons of such an approach.

Credits for the code go to Daniel Micay (original patch is at 
https://github.com/AndroidHardeningArchive/platform_external_clang/commit/776a0955ef6686d23a82d2e6a3cbd4a6a882c31c)


Repository:
  rC Clang

https://reviews.llvm.org/D54473

Files:
  include/clang/Basic/Sanitizers.def
  lib/CodeGen/CGDecl.cpp
  lib/Driver/ToolChain.cpp
  test/CodeGen/sanitize-init-locals.c

Index: test/CodeGen/sanitize-init-locals.c
===
--- test/CodeGen/sanitize-init-locals.c
+++ test/CodeGen/sanitize-init-locals.c
@@ -0,0 +1,42 @@
+// Test for -fsanitize=init-locals.
+
+// RUN: %clang_cc1 %s -O0 -triple x86_64-unknown-linux-gnu -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -O0 -triple x86_64-unknown-linux-gnu -fsanitize=init-locals -emit-llvm -o - | FileCheck -check-prefixes=CHECK,CHECK-INIT-LOCALS %s
+// RUN: %clang_cc1 %s -O0 -triple x86_64-unknown-linux-gnu -Wuninitialized -emit-llvm -o - 2>&1 | FileCheck -check-prefix=CHECK-WUNINITIALIZED %s
+// RUN: %clang_cc1 %s -O0 -triple x86_64-unknown-linux-gnu -Wuninitialized -fsanitize=init-locals -emit-llvm -o - 2>&1 | FileCheck -check-prefix=CHECK-WUNINITIALIZED %s
+
+
+// CHECK: @testSanitizeInitLocals.local_array_init_part = private unnamed_addr constant [5 x i32] [i32 0, i32 1, i32 2, i32 0, i32 0], align 16
+// CHECK: @testSanitizeInitLocals.local_array_init = private unnamed_addr constant [5 x i32] [i32 0, i32 1, i32 2, i32 3, i32 4], align 16
+
+// CHECK: %local_int_uninit = alloca i32, align 4
+// CHECK: %local_int_init_0 = alloca i32, align 4
+// CHECK: %local_int_init = alloca i32, align 4
+// CHECK: %local_array_uninit = alloca [5 x i32], align 16
+// CHECK: %local_array_init_part = alloca [5 x i32], align 16
+// CHECK: %local_array_init = alloca [5 x i32], align 16
+
+// CHECK-INIT-LOCALS: store i32 0, i32* %local_int_uninit, align 4
+// CHECK: store i32 0, i32* %local_int_init_0, align 4
+// CHECK: store i32 1, i32* %local_int_init, align 4
+//
+// CHECK-INIT-LOCALS: [[CAST0:%.*]] = {{.*}}%local_array_uninit
+// CHECK-INIT-LOCALS: call void @llvm.memset{{.*}}({{.*}}[[CAST0]], i8 0, i64 20
+// CHECK: [[CAST1:%.*]] = {{.*}}%local_array_init_part
+// CHECK: call void @llvm.memcpy{{.*}}({{.*}}[[CAST1]], {{.*}}@testSanitizeInitLocals.local_array_init_part{{.*}}, {{.*}} 20
+// CHECK: [[CAST2:%.*]] = {{.*}}%local_array_init
+// CHECK: call void @llvm.memcpy{{.*}}({{.*}}[[CAST2]], {{.*}}@testSanitizeInitLocals.local_array_init{{.*}}, {{.*}} 20
+
+// CHECK-WUNINITIALIZED: warning: variable 'local_int_uninit' is uninitialized when used here
+
+int testSanitizeInitLocals(void)
+{
+  const int local_int_uninit;
+  const int local_int_init_0 = 0;
+  const int local_int_init = 1;
+  const int local_array_uninit[5];
+  const int local_array_init_part[5] = {0, 1, 2};
+  const int local_array_init[5] = {0, 1, 2, 3, 4};
+  return local_int_uninit;
+}
+
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -819,7 +819,7 @@
 
   SanitizerMask Res = (Undefined & ~Vptr & ~Function) | (CFI & ~CFIICall) |
   CFICastStrict | UnsignedIntegerOverflow |
-  ImplicitConversion | Nullability | LocalBounds;
+  ImplicitConversion | Nullability | LocalBounds | InitLocals;
   if (getTriple().getArch() == llvm::Triple::x86 ||
   getTriple().getArch() == llvm::Triple::x86_64 ||
   getTriple().getArch() == llvm::Triple::arm ||
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1434,7 +1434,8 @@
 return;
   }
 
-  if (isTrivialInitializer(Init))
+  bool trivial = isTrivialInitializer(Init);
+  if (trivial && !getLangOpts().Sanitize.has(SanitizerKind::InitLocals))
 return;
 
   // Check whether this is a byref variable that's potentially
@@ -1445,8 +1446,15 @@
   Address Loc =
 capturedByInit ? emission.Addr : emission.getObjectAddress(*this);
 
+  bool constantAggregate = emission.IsConstantAggregate;
+
   llvm::Constant *constant = nullptr;
-  if (emission.IsConstantAggregate || D.isConstexpr()) {
+  if (trivial) {
+QualType Ty = D.getType();
+constant = CGM.EmitNullConstant(Ty);
+if (Ty->isArrayType() || Ty->isRecordType())
+  constantAggregate = true;
+  } else if (emission.IsConstantAggregate || D.isConstexpr()) {
 assert(!capturedByInit && "constant init contains a capturing 

[PATCH] D54473: [sanitizers] Initial implementation for -fsanitize=init-locals

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

In https://reviews.llvm.org/D54473#1297460, @kcc wrote:

> This new flag inhibits the warnings from -Wuninitialized, right? 
>  While this is fine for experimenting (and I want to have this in ToT to 
> enable wide experimentation)
>  we should clearly state (in the comments) that the final intent is to make 
> the feature work together with -Wuninitialized.


No, as far as I can see, the warnings from -Wuninitialized are still present 
(see the test).

> Another thing that we will need (and not necessary in the first change) is to 
> have fine grained controls over what we zero-initialize. 
>  We may want to make separate decisions for pointer/non-pointer scalars, 
> PODs, arrays of {scalars, pointers, PODs}, etc.

Ok, noted.


Repository:
  rC Clang

https://reviews.llvm.org/D54473



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

[PATCH] D54473: [sanitizers] Initial implementation for -fsanitize=init-locals

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: include/clang/Basic/Sanitizers.def:163
+// Initialize local variables.
+SANITIZER("init-locals", InitLocals)
+

lebedev.ri wrote:
> Unless i'm mistaken, I suspect you may see some surprising behavior here, 
> unfortunately.
> [[ https://bugs.llvm.org/show_bug.cgi?id=39425 | Bug 39425 - SanitizerOrdinal 
> is out of bits ]]
Um, this is unfortunate.
We don't strictly need this feature to live under -fsanitize= flag, although 
having the corresponding attribute could've been handy.


Repository:
  rC Clang

https://reviews.llvm.org/D54473



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

[PATCH] D57898: CodeGen: Fix PR40605 by splitting constant struct initializers

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: clang/test/CodeGenCXX/auto-var-init.cpp:130
+// PATTERN-NOT-O1: @__const.test_bool4_custom.custom
+// ZERO-NOT-O1: @__const.test_bool4_custom.custom
+

jfb wrote:
> `-NOT` is in the wrong place above.
Hm, I wonder if lit could automatically check that all the patterns in the file 
are used in RUN lines.
(Probably no, it's hard to tell if an all-caps word is a pattern or just a 
comment)


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: CodeGen: Fix PR40605 by splitting constant struct initializers

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 188694.
glider marked 3 inline comments as done.
glider retitled this revision from "CodeGen: Fix PR40605" to "CodeGen: Fix 
PR40605 by splitting constant struct initializers".

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

https://reviews.llvm.org/D57898

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/CodeGenCXX/auto-var-init.cpp

Index: clang/test/CodeGenCXX/auto-var-init.cpp
===
--- clang/test/CodeGenCXX/auto-var-init.cpp
+++ clang/test/CodeGenCXX/auto-var-init.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks %s -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s -check-prefix=PATTERN
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s -check-prefix=ZERO
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks %s -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK,CHECK-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK-O0,PATTERN,PATTERN-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -O1 -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK-O1,PATTERN,PATTERN-O1
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK-O0,ZERO,ZERO-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -O1 -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK-O1,ZERO,ZERO-O1
 
 template void used(T &) noexcept;
 
@@ -30,120 +32,192 @@
 // PATTERN-NOT: undef
 // ZERO-NOT: undef
 
-// PATTERN: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
+// PATTERN-O1-NOT: @__const.test_empty_uninit.uninit
 struct empty {};
-// PATTERN: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
-// PATTERN: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
-// ZERO: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// PATTERN-O0: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// ZERO-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// PATTERN-O1-NOT: @__const.test_small_uninit.uninit
+// PATTERN-O1-NOT: @__const.test_small_custom.custom
+// ZERO-O1-NOT: @__const.test_small_custom.custom
 struct small { char c; };
-// PATTERN: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O1-NOT: @__const.test_smallinit_uninit.uninit
+// PATTERN-O1-NOT: @__const.test_smallinit_braces.braces
+// PATTERN-O1-NOT: @__const.test_smallinit_custom.custom
 struct smallinit { char c = 42; };
-// PATTERN: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O1-NOT: @__const.test_smallpartinit_uninit.uninit
+// 

[PATCH] D57898: CodeGen: Fix PR40605

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 188548.
glider marked an inline comment as done.
glider retitled this revision from "CodeGen: Fix PR40605: split constant 
structures generated by -ftrivial-auto-var-init when emitting initializators" 
to "CodeGen: Fix PR40605".
glider edited the summary of this revision.

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

https://reviews.llvm.org/D57898

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/CodeGenCXX/auto-var-init.cpp

Index: clang/test/CodeGenCXX/auto-var-init.cpp
===
--- clang/test/CodeGenCXX/auto-var-init.cpp
+++ clang/test/CodeGenCXX/auto-var-init.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks %s -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s -check-prefix=PATTERN
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s -check-prefix=ZERO
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks %s -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK,CHECK-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK-O0,PATTERN,PATTERN-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -O1 -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK-O1,PATTERN,PATTERN-O1
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK-O0,ZERO,ZERO-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -O1 -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK-O1,ZERO,ZERO-O1
 
 template void used(T &) noexcept;
 
@@ -30,120 +32,192 @@
 // PATTERN-NOT: undef
 // ZERO-NOT: undef
 
-// PATTERN: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
+// PATTERN-O1-NOT: @__const.test_empty_uninit.uninit
 struct empty {};
-// PATTERN: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
-// PATTERN: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
-// ZERO: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// PATTERN-O0: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// ZERO-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// PATTERN-O1-NOT: @__const.test_small_uninit.uninit
+// PATTERN-O1-NOT: @__const.test_small_custom.custom
+// ZERO-O1-NOT: @__const.test_small_custom.custom
 struct small { char c; };
-// PATTERN: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O1-NOT: @__const.test_smallinit_uninit.uninit
+// PATTERN-O1-NOT: @__const.test_smallinit_braces.braces
+// PATTERN-O1-NOT: @__const.test_smallinit_custom.custom
 struct smallinit { char c = 42; };
-// PATTERN: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { 

[PATCH] D57898: CodeGen: Fix PR40605

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Rebased the code, updated the tests, added FIXME.


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

https://reviews.llvm.org/D57898



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

[PATCH] D58885: Variable auto-init: split out small arrays

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

The change itself looks good.
It doesn't seem to regress kernel performance on ARM64. I haven't got to 
testing on x86 yet, but don't anticipate any problems either.




Comment at: lib/CodeGen/CGDecl.cpp:1206
+  bool canDoSingleStore = Ty->isIntOrIntVectorTy() ||
+  Ty->isPtrOrPtrVectorTy() || Ty->isFPOrFPVectorTy();
+  if (canDoSingleStore) {

Is the second expression being moved to line 1206 a result of clang-format? 
Otherwise it'll migrate back at some point.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58885



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

[PATCH] D58885: Variable auto-init: split out small arrays

[Alexander Potapenko via Phabricator via cfe-commits]

glider accepted this revision.
glider added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/CodeGen/CGDecl.cpp:1206
+  bool canDoSingleStore = Ty->isIntOrIntVectorTy() ||
+  Ty->isPtrOrPtrVectorTy() || Ty->isFPOrFPVectorTy();
+  if (canDoSingleStore) {

jfb wrote:
> glider wrote:
> > Is the second expression being moved to line 1206 a result of clang-format? 
> > Otherwise it'll migrate back at some point.
> Yes, the slightly longer name pushes it past 80 columns, and I just 
> auto-format stuff before uploading.
> 
> I can do this change separately, I just noticed that the name I originally 
> used was now misleading because vectors aren't scalars :)
Up to you, this doesn't matter IMO :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D58885



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

[PATCH] D58885: Variable auto-init: split out small arrays

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: test/CodeGenCXX/auto-var-init.cpp:1025
 // PATTERN-LABEL: @test_intptr4_uninit()
-// PATTERN: call void @llvm.memcpy{{.*}} @__const.test_intptr4_uninit.uninit
-// ZERO-LABEL: @test_intptr4_uninit()
-// ZERO: call void @llvm.memset{{.*}}, i8 0,
+// PATTERN:   %1 = getelementptr inbounds [4 x i32*], [4 x i32*]* %uninit, 
i64 0, i64 0
+// PATTERN-NEXT:  store i32* inttoptr (i64 -6148914691236517206 to i32*), 
i32** %1, align 16

This check fails for me locally.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58885



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

[PATCH] D57898: CodeGen: Fix PR40605 by splitting constant struct initializers

[Alexander Potapenko via Phabricator via cfe-commits]

glider closed this revision.
glider added a comment.

Landed r355181, thank you!


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: [RFC] Split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider marked 2 inline comments as not done.
glider added a comment.

Sorry, didn't mean to mark these done.


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: [RFC] Split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider marked 2 inline comments as done.
glider added inline comments.



Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:1143
+const llvm::StructLayout *Layout =
+CGM.getDataLayout().getStructLayout(cast(Ty));
+for (unsigned i = 0; i != constant->getNumOperands(); i++) {

jfb wrote:
> I think you need a heuristic here, where you don't emit multiple stores if 
> `getNumOperands` is greater than some number. In that case you'd keep doing 
> memcpy instead. Note that a struct containing sub-structs (or arrays) should 
> also count the number of sub-structs (so just checking `getNumOperands` isn't 
> sufficient).
> 
> Other parts of this code chose 6 stores...
Do we ever need to not split these stores? Can't the  MemCpyOpt pass take care 
of them later on?



Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:1765
+  emitStoresForConstant(CGM, D, Loc, isVolatile, Builder, constant,
+/*forInit*/ false);
 }

jfb wrote:
> Can you explain why this case needs to be different? IIUC it's because the 
> types can differ which makes the struct's elements not match up the ones from 
> the constant in the loop above? I think the code above should automatically 
> handle that case instead of passing in a flag here.
> 
> You also definitely need to test the case where that happens :)
I just thought we shouldn't break the constants that we didn't create with 
-ftrivial-var-auto-init.
I'll take a closer look though (indeed, this crashes on one of the structs from 
the Linux kernel)


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Re: case when STy != Loc.getElementType() - this is already covered by other 
Clang tests.

I'm still unsure about the heuristic here. I believe that for 
auto-initialization we want to be quite aggressive with these splits (unlike 
for regular constant stores).
Perhaps we should do the split in the case all bytes are pattern bytes? (This 
is probably another use case for `forInit`)


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 185990.
glider retitled this revision from "[RFC] Split constant structures generated 
by -ftrivial-auto-var-init when emitting initializators" to "CodeGen: Fix 
PR40605: split constant structures generated by -ftrivial-auto-var-init when 
emitting initializators".
glider edited the summary of this revision.
glider added a comment.

Updated the patch: instead of passing around forInit, match the constant type 
to pointer type.

I'm not quite sure how to show the resulting difference in code. Do you mean we 
need Clang to support both modes and to compare the resulting assembly?


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

https://reviews.llvm.org/D57898

Files:
  tools/clang/lib/CodeGen/CGDecl.cpp
  tools/clang/test/CodeGenCXX/auto-var-init.cpp

Index: tools/clang/test/CodeGenCXX/auto-var-init.cpp
===
--- tools/clang/test/CodeGenCXX/auto-var-init.cpp
+++ tools/clang/test/CodeGenCXX/auto-var-init.cpp
@@ -30,42 +30,24 @@
 // PATTERN-NOT: undef
 // ZERO-NOT: undef
 
-// PATTERN: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
 struct empty {};
-// PATTERN: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
 // PATTERN: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
 // ZERO: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
 struct small { char c; };
-// PATTERN: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
 struct smallinit { char c = 42; };
-// PATTERN: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
 struct smallpartinit { char c = 42, d; };
-// PATTERN: @__const.test_nullinit_uninit.uninit = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, align 8
-// PATTERN: @__const.test_nullinit_braces.braces = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, align 8
-// PATTERN: @__const.test_nullinit_custom.custom = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, align 8
 struct nullinit { char* null = nullptr; };
-// PATTERN: @__const.test_padded_uninit.uninit = private unnamed_addr constant %struct.padded { i8 -86, i32 -1431655766 }, align 4
 // PATTERN: @__const.test_padded_custom.custom = private unnamed_addr constant %struct.padded { i8 42, i32 13371337 }, align 4
 // ZERO: @__const.test_padded_custom.custom = private unnamed_addr constant %struct.padded { i8 42, i32 13371337 }, align 4
 struct padded { char c; int i; };
-// PATTERN: @__const.test_paddednullinit_uninit.uninit = private unnamed_addr constant %struct.paddednullinit { i8 -86, i32 -1431655766 }, align 4
-// PATTERN: @__const.test_paddednullinit_braces.braces = private unnamed_addr constant %struct.paddednullinit { i8 -86, i32 -1431655766 }, align 4
-// PATTERN: @__const.test_paddednullinit_custom.custom = private unnamed_addr constant %struct.paddednullinit { i8 -86, i32 -1431655766 }, align 4
 struct paddednullinit { char c = 0; int i = 0; };
-// PATTERN: @__const.test_bitfield_uninit.uninit = private unnamed_addr constant %struct.bitfield { i8 -86, [3 x i8] c"\AA\AA\AA" }, align 4
 // PATTERN: @__const.test_bitfield_custom.custom = private unnamed_addr constant %struct.bitfield { i8 20, [3 x i8] zeroinitializer }, align 4
 // ZERO: @__const.test_bitfield_custom.custom = private unnamed_addr constant %struct.bitfield { i8 20, [3 x i8] zeroinitializer }, align 4
 struct bitfield { int i : 4; int j : 2; };
-// PATTERN: @__const.test_bitfieldaligned_uninit.uninit = private unnamed_addr constant %struct.bitfieldaligned { i8 -86, [3 x i8] c"\AA\AA\AA", i8 -86, [3 x i8] c"\AA\AA\AA" }, align 4
 // PATTERN: @__const.test_bitfieldaligned_custom.custom = private unnamed_addr constant %struct.bitfieldaligned { i8 4, [3 x i8] zeroinitializer, i8 1, [3 x i8] zeroinitializer }, align 4
 // ZERO: @__const.test_bitfieldaligned_custom.custom = private unnamed_addr constant %struct.bitfieldaligned { i8 4, [3 x i8] zeroinitializer, i8 1, [3 x i8] zeroinitializer }, align 4
 struct bitfieldaligned { int i : 4; int : 0; int j : 2; };
 struct big { unsigned 

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

> Can you add a link to bug 40605 in the commit message?

It's in the title now, doesn't that count? :)


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 186469.
glider marked an inline comment as done.
glider added a subscriber: pcc.
glider added a comment.

Added a helper function to decide whether we want to break the structure or not.
Right now it only checks for optimization level and structure size.

The constant 64 is picked to match the cacheline size. Other interesting 
numbers I've considered  were:

- 160 - a size of a kernel structure for which the initialization was inlined 
without any noticeable profit (no stores to that uninit structure in the same 
function)
- 256 - the maximum size for an inlined memset/memcpy with -mno-sse
- 512 - the maximum size for an inlined memset/memcpy with SSE enabled/

I don't think we have enough data to back any of the choices though.

I've also fixed the code to skip zero-size memset() calls.


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

https://reviews.llvm.org/D57898

Files:
  tools/clang/lib/CodeGen/CGDecl.cpp
  tools/clang/test/CodeGenCXX/auto-var-init.cpp

Index: tools/clang/test/CodeGenCXX/auto-var-init.cpp
===
--- tools/clang/test/CodeGenCXX/auto-var-init.cpp
+++ tools/clang/test/CodeGenCXX/auto-var-init.cpp
@@ -1,6 +1,8 @@
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks %s -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s -check-prefix=PATTERN
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s -check-prefix=ZERO
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s -check-prefixes=PATTERN,PATTERN-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -O1 -emit-llvm -o - | FileCheck %s -check-prefixes=PATTERN,PATTERN-O1
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s -check-prefixes=ZERO,ZERO-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -O1 -emit-llvm -o - | FileCheck %s -check-prefixes=ZERO,ZERO-O1
 
 template void used(T &) noexcept;
 
@@ -30,104 +32,104 @@
 // PATTERN-NOT: undef
 // ZERO-NOT: undef
 
-// PATTERN: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
 struct empty {};
-// PATTERN: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
-// PATTERN: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
-// ZERO: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// PATTERN-O0: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// ZERO-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
 struct small { char c; };
-// PATTERN: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
 struct smallinit { char c = 42; };
-// PATTERN: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { i8 

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider marked 4 inline comments as done.
glider added inline comments.
Herald added a subscriber: jdoerfert.



Comment at: tools/clang/test/CodeGenCXX/auto-var-init.cpp:496
 // ZERO-LABEL: @test_smallpartinit_uninit()
 // ZERO: call void @llvm.memset{{.*}}, i8 0,
 

jfb wrote:
> I wonder if (maybe in a separate patch) you also want to avoid `memset` when 
> something is pretty small.
If I'm understanding your request correctly, the same code also handles the 
memset() case for zero-initialization.


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

This patch shaved off 2.5% of performance slowdown on 
https://github.com/kamalmarhubi/linux-ipc-benchmarks/blob/master/af_inet_loopback.c
 on a kernel built in pattern-initialization mode.
The .rodata size went down by only 12K (0.5%, since we now split only 
structures smaller than 64 bytes)
The .text size went down by 19K (0.4%)


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:979
+  if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+return false;
+  if (GlobalSize <= SizeLimit)

jfb wrote:
> The general 64-byte heuristic is fine with me. It's just a bit weird to 
> special-case `-O0`, but we do it elsewhere too (and it keeps the current 
> status-quo of letting the backend decide what to do). From that perspective 
> I'm fine with it.
> 
> @rjmccall do you have a strong preference either way?
> 
> One small nit: can you use `ByteSize` instead of just `Size`? Makes it easier 
> to figure out what's going on in code IMO.
I don't have a strong opinion on variable naming, but this:
```
 974 static bool shouldSplitStructStore(CodeGenModule ,
 975uint64_t GlobalByteSize) {
 976   // Don't break structures that occupy more than one cacheline.
 977   uint64_t ByteSizeLimit = 64;
 978   if (CGM.getCodeGenOpts().OptimizationLevel == 0)
 979 return false;
 980   if (GlobalByteSize <= ByteSizeLimit)
 981 return true;
 982   return false;
```
looks a bit more heavyweight than the previous function, in which we also have 
`GlobalSize` and `SizeLimit`.



Comment at: tools/clang/test/CodeGenCXX/auto-var-init.cpp:476
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
 // PATTERN-LABEL: @test_empty_uninit()
+// PATTERN-O0: call void @llvm.memcpy{{.*}} @__const.test_empty_uninit.uninit

jfb wrote:
> The tests aren't matching labels anymore: `PATTERN-LABEL` is a dead label 
> check now. I think forking things at `-O0` and `-O1` is fine, but I think you 
> want a small `-O0` test (separate from this patch) which does one or two 
> things, and then you want this test to only look at `-O1`. That way you don't 
> need so much stuff changing in the test (and `-O0` is still tested).
Why? We're passing both PATTERN and PATTERN-OX to FileCheck when testing.
I've checked that replacing `@test_empty_uninit()` on this line with a 
different function name makes the test fail.


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Well, now we somewhat break padding initialization.

E.g. for the following struct:

  struct s { 
int a;
char b;
long int c;
  };

we generate the following constant initializer with `-O0`:

  .L__const.foo.local:
  .long   2863311530  # 0x
  .byte   170 # 0xaa
  .zero   3   
  .quad   -6148914691236517206# 0x
  .size   .L__const.foo.local, 16

, which overwrites the padding bytes on stack, but with a wrong constant.

OTOH with `-O1` (i.e. with my patch) we generate the following sequence of 
stores:

  movl$-1431655766, 8(%rsp)   # imm = 0x
  movb$-86, 12(%rsp)
  movabsq $-6148914691236517206, %rax # imm = 0x
  movq%rax, 16(%rsp)


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 186605.
glider marked 4 inline comments as done.
glider added a comment.

Addressed the comments.


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

https://reviews.llvm.org/D57898

Files:
  tools/clang/lib/CodeGen/CGDecl.cpp
  tools/clang/test/CodeGenCXX/auto-var-init.cpp

Index: tools/clang/test/CodeGenCXX/auto-var-init.cpp
===
--- tools/clang/test/CodeGenCXX/auto-var-init.cpp
+++ tools/clang/test/CodeGenCXX/auto-var-init.cpp
@@ -1,6 +1,8 @@
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks %s -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s -check-prefix=PATTERN
-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s -check-prefix=ZERO
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -emit-llvm -o - | FileCheck %s -check-prefixes=PATTERN,PATTERN-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=pattern %s -O1 -emit-llvm -o - | FileCheck %s -check-prefixes=PATTERN,PATTERN-O1
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -emit-llvm -o - | FileCheck %s -check-prefixes=ZERO,ZERO-O0
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -fblocks -ftrivial-auto-var-init=zero %s -O1 -emit-llvm -o - | FileCheck %s -check-prefixes=ZERO,ZERO-O1
 
 template void used(T &) noexcept;
 
@@ -30,104 +32,104 @@
 // PATTERN-NOT: undef
 // ZERO-NOT: undef
 
-// PATTERN: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
 struct empty {};
-// PATTERN: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
-// PATTERN: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
-// ZERO: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// PATTERN-O0: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
+// ZERO-O0: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
 struct small { char c; };
-// PATTERN: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
 struct smallinit { char c = 42; };
-// PATTERN: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
+// PATTERN-O0: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
 struct smallpartinit { char c = 42, d; };
-// PATTERN: @__const.test_nullinit_uninit.uninit = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, align 8
-// PATTERN: @__const.test_nullinit_braces.braces = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, align 8
-// PATTERN: @__const.test_nullinit_custom.custom = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, align 8
+// PATTERN-O0: @__const.test_nullinit_uninit.uninit = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, 

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

... which happily skips the padding.

It occurs to me that we need to properly handle the padding in `patternFor` 
before we'll be able to split the structures.


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

FWIW I think I've almost finished handling the padding: 
https://reviews.llvm.org/D58188
I haven't checked whether it works correctly with padding in custom 
initializers (like `struct s local = {1, 2}`), but TEST_UNINIT and TEST_BRACES 
are covered already.


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

https://reviews.llvm.org/D57898



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

[PATCH] D54604: Automatic variable initialization

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Not sure if a similar problem was mentioned already or not, but the following 
program:

  void alloc_sock(int *err);
  int try_send();
  
  int send(int len) {
int err;
if (len) {
  err = -1; 
  alloc_sock();
  err = try_send();
}
return -1; 
  }

produces a redundant store of `0x` to `err`:

   :
  void alloc_sock(int *err);
  int try_send();
  
  int send(int len) {
 0:   50  push   %rax
int err;
 1:   c7 44 24 04 aa aa aamovl   $0x,0x4(%rsp)
 8:   aa 
if (len) {
 9:   85 ff   test   %edi,%edi
 b:   74 1d   je 2a 
  err = -1;
 d:   c7 44 24 04 ff ff ffmovl   $0x,0x4(%rsp)
14:   ff 
15:   48 8d 7c 24 04  lea0x4(%rsp),%rdi
  alloc_sock();
1a:   e8 00 00 00 00  callq  1f 
  1b: R_X86_64_PLT32  alloc_sock-0x4
  err = try_send();
1f:   31 c0   xor%eax,%eax
21:   e8 00 00 00 00  callq  26 
  22: R_X86_64_PLT32  try_send-0x4
26:   89 44 24 04 mov%eax,0x4(%rsp)
}
return -1;
2a:   b8 ff ff ff ff  mov$0x,%eax
2f:   59  pop%rcx
30:   c3  retq  

Not sure whether this isn't a bug in DSE though.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54604



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

[PATCH] D54604: Automatic variable initialization

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

(Overall, since this is an "unsupported" feature, is it ok to file Bugzilla 
bugs for it?)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54604



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

[PATCH] D54604: Automatic variable initialization

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.
Herald added a project: LLVM.

I think doing separate stores of 0xAA into struct members instead of copying 
part of .rodata over it will let us do a much better job in optimizing away 
redundant stores.
I've opened https://bugs.llvm.org/show_bug.cgi?id=40605 to track that.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54604



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

[PATCH] D61664: [NewPM] Setup Passes for KASan and KMSan

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Rubberstamp LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61664



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

[PATCH] D61879: WIP: Prototype of DSE optimizations for -ftrivial-auto-var-init

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: llvm/lib/Transforms/Scalar/DeadStoreEliminationExpGlobal.cpp:20
+
+static const GlobalValue::GUID GUIDS_LOOKUP[] = {
+// grep  -P -o "(?<=LOOKUP_FN: ).*"  | sort -u >

error: zero-size array ‘llvm::GUIDS_LOOKUP’

Can you please provide an example of 
DeadStoreEliminationExpGlobalGUIDListGen.h? The instruction seems insufficient.



Comment at: llvm/lib/Transforms/Scalar/DeadStoreEliminationExpGlobal.cpp:27
+static const char GlobalArgumentMemAccess[] = {
+// grep  -P -o "(?<=FUNCTION_INFO: ).*"  | sort -u >
+// ../llvm-project/llvm/lib/Transforms/Scalar/DeadStoreEliminationExpData.h

Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61879



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

[PATCH] D61879: WIP: Prototype of DSE optimizations for -ftrivial-auto-var-init

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Vitaly, can you please rebase the patch?
As far as I can see, you've submitted parts of it already.
(not that I can't resolve the conflicts locally, but keeping it up-to-date may 
save others some time)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61879



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

[PATCH] D61879: WIP: Prototype of DSE optimizations for -ftrivial-auto-var-init

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: llvm/lib/Transforms/Scalar/DeadStoreEliminationExp.cpp:709
+   size_t argN) {
+  const Function *F = cast(Callee);
+  if (argN >= F->arg_size())

This cast fails in the following case:

```
%21 = call i64 @probe_kernel_read(i8* nonnull %9, i8* %20, i64 8) #10, !dbg 
!7740
```
, where the callee is declared as:
```
@probe_kernel_read = weak hidden alias i64 (i8*, i8*, i64), i64 (i8*, i8*, 
i64)* @__probe_kernel_read
```

When building Android kernel with LTO enabled, the gold plugin crashes on an 
assertion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61879



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

[PATCH] D61879: WIP: Prototype of DSE optimizations for -ftrivial-auto-var-init

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: llvm/lib/Transforms/Scalar/DeadStoreEliminationExp.cpp:556
+  LocationSize LS = MemoryLocation::getForDest(MSI).Size;
+  ConstantRange Range(PointerSizeInBits, false);
+  if (!LS.isPrecise() ||

An assertion fires here on the Android kernel:

```
aarch64-linux-android-ld.gold: 
/usr/local/google/src/llvm-git-monorepo/llvm/lib/IR/ConstantRange.cpp:54: 
llvm::ConstantRange::ConstantRange(llvm::APInt, llvm::APInt): Assertion `(Lower 
!= Upper || (Lower.isMaxValue() || Lower.isMinValue())) && "Lower == Upper, but 
they aren't min or max value!"' failed.
```

For some reason the (APInt, APInt) version of the constructor is being invoked.
Probably PointerSizeInBits should be declared as `int32_t` here and in 
`findDeadStores()`



Comment at: llvm/lib/Transforms/Scalar/DeadStoreEliminationExp.cpp:563
+assert(RR.getBitWidth() == Range.getBitWidth());
+ConstantRange RRR = {Range.getLower(),
+ Range.getLower() + LS.getValue()};

I'm seeing a case in which Range.getLower() is 20179 and LS.getValue() is 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61879



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

[PATCH] D64742: Allow using -ftrivial-auto-var-init=zero in C mode without extra flags

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Initially concerns have been raised that -ftrivial-auto-var-init=zero
potentially defines a new dialect of C++, therefore this option was
guarded with
-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang.

The guard flag name suggests that at some point
-ftrivial-auto-var-init=pattern will perform on par with
-ftrivial-auto-var-init=zero, thus making it possible to remove the
latter from Clang.
However this isn't going to happen in the nearest future, at least not
on X86, where `memset(object, 0, size)` is still lowered to a more
efficient code than `memset(object, 0xAA, size)`.
Therefore security-minded people may still need an easy way to
zero-initialize all the locals to keep the performance penalty low.

For Linux kernel, which already uses a non-standard dialect of C,
introducing yet another hardening feature doesn't radically change the
situation. Other C codebases also tend to use non-standard features more
often, so the mentioned guard flag only complicates the adoption of
auto-initialization for them, not actually protecting their purity.

As a compromise solution, let
-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
remain mandatory for -ftrivial-auto-var-init=zero in C++ mode and make
it optional in the C mode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64742

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/clang_f_opts.c


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -566,7 +566,10 @@
 // RUN: %clang -### -S -ftrivial-auto-var-init=uninitialized %s 2>&1 | 
FileCheck -check-prefix=CHECK-TRIVIAL-UNINIT %s
 // RUN: %clang -### -S -ftrivial-auto-var-init=pattern %s 2>&1 | FileCheck 
-check-prefix=CHECK-TRIVIAL-PATTERN %s
 // RUN: %clang -### -S -ftrivial-auto-var-init=zero 
-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang %s 
2>&1 | FileCheck -check-prefix=CHECK-TRIVIAL-ZERO-GOOD %s
-// RUN: %clang -### -S -ftrivial-auto-var-init=zero %s 2>&1 | FileCheck 
-check-prefix=CHECK-TRIVIAL-ZERO-BAD %s
+// RUN: %clang -### -S -ftrivial-auto-var-init=zero %s 2>&1 | FileCheck 
-check-prefix=CHECK-TRIVIAL-ZERO-GOOD %s
+// RUN: %clangxx -### -S -ftrivial-auto-var-init=zero 
-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang %s 
2>&1 | FileCheck -check-prefix=CHECK-TRIVIAL-ZERO-GOOD %s
+// RUN: %clangxx -### -S -ftrivial-auto-var-init=zero %s 2>&1 | FileCheck 
-check-prefix=CHECK-TRIVIAL-ZERO-BAD %s
+
 // CHECK-TRIVIAL-UNINIT-NOT: hasn't been enabled
 // CHECK-TRIVIAL-PATTERN-NOT: hasn't been enabled
 // CHECK-TRIVIAL-ZERO-GOOD-NOT: hasn't been enabled
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2544,7 +2544,8 @@
 static void RenderTrivialAutoVarInitOptions(const Driver ,
 const ToolChain ,
 const ArgList ,
-ArgStringList ) {
+ArgStringList ,
+const InputInfo ) {
   auto DefaultTrivialAutoVarInit = TC.GetDefaultTrivialAutoVarInit();
   StringRef TrivialAutoVarInit = "";
 
@@ -2578,7 +2579,9 @@
 }
 
   if (!TrivialAutoVarInit.empty()) {
-if (TrivialAutoVarInit == "zero" && 
!Args.hasArg(options::OPT_enable_trivial_var_init_zero))
+if (TrivialAutoVarInit == "zero" &&
+!Args.hasArg(options::OPT_enable_trivial_var_init_zero) &&
+(Input.getType() != types::TY_C))
   D.Diag(diag::err_drv_trivial_auto_var_init_zero_disabled);
 CmdArgs.push_back(
 Args.MakeArgString("-ftrivial-auto-var-init=" + TrivialAutoVarInit));
@@ -4623,7 +4626,7 @@
 CmdArgs.push_back(Args.MakeArgString("-mspeculative-load-hardening"));
 
   RenderSSPOptions(TC, Args, CmdArgs, KernelOrKext);
-  RenderTrivialAutoVarInitOptions(D, TC, Args, CmdArgs);
+  RenderTrivialAutoVarInitOptions(D, TC, Args, CmdArgs, Input);
 
   // Translate -mstackrealign
   if (Args.hasFlag(options::OPT_mstackrealign, options::OPT_mno_stackrealign,


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -566,7 +566,10 @@
 // RUN: %clang -### -S -ftrivial-auto-var-init=uninitialized %s 2>&1 | FileCheck -check-prefix=CHECK-TRIVIAL-UNINIT %s
 // RUN: %clang -### -S -ftrivial-auto-var-init=pattern %s 2>&1 | FileCheck -check-prefix=CHECK-TRIVIAL-PATTERN %s
 // RUN: %clang -### -S -ftrivial-auto-var-init=zero 

[PATCH] D61879: WIP: Prototype of DSE optimizations for -ftrivial-auto-var-init

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: llvm/lib/Transforms/Scalar/DeadStoreEliminationExpGlobal.cpp:27
+static const char GlobalArgumentMemAccess[] = {
+// grep  -P -o "(?<=FUNCTION_INFO: ).*"  | sort -u >
+// ../llvm-project/llvm/lib/Transforms/Scalar/DeadStoreEliminationExpData.h

vitalybuka wrote:
> glider wrote:
> > Ditto.
> These files should be empty. Raw diff 
> https://reviews.llvm.org/file/data/o6sk6gw2gqs4u4pmodrn/PHID-FILE-s6c6nsofxnqekkcvzdzs/D61879.diff
>  already contains them. 
> It's ThinLTO replacement experiments. They don't improve results enough, so I 
> didn't bother to create real ThinLTO stuff. Anyway it is not needed for full 
> LTO.
> 
```
$ ninja -j64 check-clang
...
/usr/local/google/src/llvm-git-monorepo/llvm/lib/Transforms/Scalar/DeadStoreEliminationExpGlobal.cpp:20:32:
 error: zero-size array ‘llvm::GUIDS_LOOKUP’
 static const GlobalValue::GUID GUIDS_LOOKUP[] = {
^~~~
/usr/local/google/src/llvm-git-monorepo/llvm/lib/Transforms/Scalar/DeadStoreEliminationExpGlobal.cpp:26:19:
 error: zero-size array ‘llvm::GlobalArgumentMemAccess’
 static const char GlobalArgumentMemAccess[] = {
   ^~~
```
Am I doing something wrong? Looks like empty files aren't enough.
I've fixed the problem by putting "0" into each file, but it's strange it works 
differently for us.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61879



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

[PATCH] D66948: [CodeGen]: fix error message for "=r" asm constraint

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 218037.
glider added a comment.

Minor comment fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66948

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/x86_64-PR42672.c


Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- clang/test/CodeGen/x86_64-PR42672.c
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -4,6 +4,7 @@
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s 
-o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
 
 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -57,7 +58,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm: can't store value into 
a register
 
 // Check Clang reports an error if attempting to return a big structure via a 
register.
 void big_struct(void) {
@@ -69,7 +70,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm: can't store value into 
a register
 
 // Clang is able to emit LLVM IR for an 16-byte structure.
 void x_constraint_fit() {
@@ -100,3 +101,17 @@
 }
 
 // CHECK-IMPOSSIBLE_X: invalid output size for constraint
+
+// http://crbug.com/999160
+// Clang used to report the following message:
+//   "impossible constraint in asm: can't store struct into a register"
+// for the assembly directive below, although there's no struct.
+void crbug_999160_regtest() {
+#ifdef IMPOSSIBLE_9BYTES
+  char buf[9];
+  asm(""
+  : "=r"(buf));
+#endif
+}
+
+// CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value 
into a register
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2326,7 +2326,7 @@
 const Expr *OutExpr = S.getOutputExpr(i);
 CGM.Error(
 OutExpr->getExprLoc(),
-"impossible constraint in asm: can't store struct into a 
register");
+"impossible constraint in asm: can't store value into a register");
 return;
   }
   Dest = MakeAddrLValue(A, Ty);


Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- clang/test/CodeGen/x86_64-PR42672.c
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -4,6 +4,7 @@
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
 
 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -57,7 +58,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm: can't store value into a register
 
 // Check Clang reports an error if attempting to return a big structure via a register.
 void big_struct(void) {
@@ -69,7 +70,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm: can't store value into a register
 
 // Clang is able to emit LLVM IR for an 16-byte structure.
 void x_constraint_fit() {
@@ -100,3 +101,17 @@
 }
 
 // CHECK-IMPOSSIBLE_X: invalid output size for constraint
+
+// http://crbug.com/999160
+// Clang used to report the following message:
+//   "impossible constraint in asm: can't store struct into a register"
+// for the assembly directive below, although there's no struct.
+void crbug_999160_regtest() {
+#ifdef IMPOSSIBLE_9BYTES
+  char buf[9];
+  asm(""
+  : "=r"(buf));
+#endif
+}
+
+// CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value into a register
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2326,7 +2326,7 @@

[PATCH] D66948: [CodeGen]: fix error message for "=r" asm constraint

[Alexander Potapenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370444: [CodeGen]: fix error message for =r asm 
constraint (authored by glider, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66948?vs=218037=218040#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66948

Files:
  cfe/trunk/lib/CodeGen/CGStmt.cpp
  cfe/trunk/test/CodeGen/x86_64-PR42672.c


Index: cfe/trunk/test/CodeGen/x86_64-PR42672.c
===
--- cfe/trunk/test/CodeGen/x86_64-PR42672.c
+++ cfe/trunk/test/CodeGen/x86_64-PR42672.c
@@ -4,6 +4,7 @@
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s 
-o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
 
 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -57,7 +58,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm: can't store value into 
a register
 
 // Check Clang reports an error if attempting to return a big structure via a 
register.
 void big_struct(void) {
@@ -69,7 +70,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm: can't store value into 
a register
 
 // Clang is able to emit LLVM IR for an 16-byte structure.
 void x_constraint_fit() {
@@ -100,3 +101,17 @@
 }
 
 // CHECK-IMPOSSIBLE_X: invalid output size for constraint
+
+// http://crbug.com/999160
+// Clang used to report the following message:
+//   "impossible constraint in asm: can't store struct into a register"
+// for the assembly directive below, although there's no struct.
+void crbug_999160_regtest() {
+#ifdef IMPOSSIBLE_9BYTES
+  char buf[9];
+  asm(""
+  : "=r"(buf));
+#endif
+}
+
+// CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value 
into a register
Index: cfe/trunk/lib/CodeGen/CGStmt.cpp
===
--- cfe/trunk/lib/CodeGen/CGStmt.cpp
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp
@@ -2326,7 +2326,7 @@
 const Expr *OutExpr = S.getOutputExpr(i);
 CGM.Error(
 OutExpr->getExprLoc(),
-"impossible constraint in asm: can't store struct into a 
register");
+"impossible constraint in asm: can't store value into a register");
 return;
   }
   Dest = MakeAddrLValue(A, Ty);


Index: cfe/trunk/test/CodeGen/x86_64-PR42672.c
===
--- cfe/trunk/test/CodeGen/x86_64-PR42672.c
+++ cfe/trunk/test/CodeGen/x86_64-PR42672.c
@@ -4,6 +4,7 @@
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
 
 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -57,7 +58,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm: can't store value into a register
 
 // Check Clang reports an error if attempting to return a big structure via a register.
 void big_struct(void) {
@@ -69,7 +70,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm: can't store value into a register
 
 // Clang is able to emit LLVM IR for an 16-byte structure.
 void x_constraint_fit() {
@@ -100,3 +101,17 @@
 }
 
 // CHECK-IMPOSSIBLE_X: invalid output size for constraint
+
+// http://crbug.com/999160
+// Clang used to report the following message:
+//   "impossible constraint in asm: can't store struct into a register"
+// for the assembly directive below, although there's no struct.
+void crbug_999160_regtest() {
+#ifdef IMPOSSIBLE_9BYTES
+  char buf[9];
+  asm(""
+  : "=r"(buf));
+#endif
+}
+
+// 

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370335: [CodeGen]: dont treat structures returned in 
registers as memory inputs (authored by glider, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65234?vs=217831=217833#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65234

Files:
  cfe/trunk/lib/CodeGen/CGStmt.cpp
  cfe/trunk/test/CodeGen/asm-attrs.c
  cfe/trunk/test/CodeGen/x86_64-PR42672.c

Index: cfe/trunk/lib/CodeGen/CGStmt.cpp
===
--- cfe/trunk/lib/CodeGen/CGStmt.cpp
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp
@@ -1984,6 +1984,7 @@
   std::vector ResultTruncRegTypes;
   std::vector ArgTypes;
   std::vector Args;
+  llvm::BitVector ResultTypeRequiresCast;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2022,13 +2023,23 @@
 
 // If this is a register output, then make the inline asm return it
 // by-value.  If this is a memory result, return the value by-reference.
-if (!Info.allowsMemory() && hasScalarEvaluationKind(OutExpr->getType())) {
+bool isScalarizableAggregate =
+hasAggregateEvaluationKind(OutExpr->getType());
+if (!Info.allowsMemory() && (hasScalarEvaluationKind(OutExpr->getType()) ||
+ isScalarizableAggregate)) {
   Constraints += "=" + OutputConstraint;
   ResultRegQualTys.push_back(OutExpr->getType());
   ResultRegDests.push_back(Dest);
-  ResultRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
-  ResultTruncRegTypes.push_back(ResultRegTypes.back());
-
+  ResultTruncRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
+  if (Info.allowsRegister() && isScalarizableAggregate) {
+ResultTypeRequiresCast.push_back(true);
+unsigned Size = getContext().getTypeSize(OutExpr->getType());
+llvm::Type *ConvTy = llvm::IntegerType::get(getLLVMContext(), Size);
+ResultRegTypes.push_back(ConvTy);
+  } else {
+ResultTypeRequiresCast.push_back(false);
+ResultRegTypes.push_back(ResultTruncRegTypes.back());
+  }
   // If this output is tied to an input, and if the input is larger, then
   // we need to set the actual result type of the inline asm node to be the
   // same as the input type.
@@ -2271,6 +2282,9 @@
   assert(RegResults.size() == ResultRegTypes.size());
   assert(RegResults.size() == ResultTruncRegTypes.size());
   assert(RegResults.size() == ResultRegDests.size());
+  // ResultRegDests can be also populated by addReturnRegisterOutputs() above,
+  // in which case its size may grow.
+  assert(ResultTypeRequiresCast.size() <= ResultRegDests.size());
   for (unsigned i = 0, e = RegResults.size(); i != e; ++i) {
 llvm::Value *Tmp = RegResults[i];
 
@@ -2300,7 +2314,24 @@
   }
 }
 
-EmitStoreThroughLValue(RValue::get(Tmp), ResultRegDests[i]);
+LValue Dest = ResultRegDests[i];
+// ResultTypeRequiresCast elements correspond to the first
+// ResultTypeRequiresCast.size() elements of RegResults.
+if ((i < ResultTypeRequiresCast.size()) && ResultTypeRequiresCast[i]) {
+  unsigned Size = getContext().getTypeSize(ResultRegQualTys[i]);
+  Address A = Builder.CreateBitCast(Dest.getAddress(),
+ResultRegTypes[i]->getPointerTo());
+  QualType Ty = getContext().getIntTypeForBitwidth(Size, /*Signed*/ false);
+  if (Ty.isNull()) {
+const Expr *OutExpr = S.getOutputExpr(i);
+CGM.Error(
+OutExpr->getExprLoc(),
+"impossible constraint in asm: can't store struct into a register");
+return;
+  }
+  Dest = MakeAddrLValue(A, Ty);
+}
+EmitStoreThroughLValue(RValue::get(Tmp), Dest);
   }
 }
 
Index: cfe/trunk/test/CodeGen/asm-attrs.c
===
--- cfe/trunk/test/CodeGen/asm-attrs.c
+++ cfe/trunk/test/CodeGen/asm-attrs.c
@@ -8,7 +8,7 @@
 // CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
 // CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
 // CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
-// CHECK: call void asm "foo8", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
 // CHECK: attributes [[READNONE]] = { nounwind readnone }
 // CHECK: attributes [[NOATTRS]] = { nounwind }
Index: cfe/trunk/test/CodeGen/x86_64-PR42672.c
===
--- cfe/trunk/test/CodeGen/x86_64-PR42672.c
+++ cfe/trunk/test/CodeGen/x86_64-PR42672.c
@@ -0,0 +1,102 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DSTRUCT -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-STRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -USTRUCT -emit-llvm %s -o - 2>&1 | FileCheck %s 

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Landed r370335, thank you!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65234



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 217831.
glider added a comment.

Rebased the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/asm-attrs.c
  clang/test/CodeGen/x86_64-PR42672.c

Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- /dev/null
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -0,0 +1,102 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DSTRUCT -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-STRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -USTRUCT -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NOSTRUCT
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_ODD -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_ODD
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+
+// Make sure Clang doesn't treat |lockval| as asm input.
+void _raw_spin_lock(void) {
+#ifdef STRUCT
+  struct {
+unsigned short owner, next;
+  } lockval;
+  lockval.owner = 1;
+  lockval.next = 2;
+#else
+  int lockval;
+  lockval = 3;
+#endif
+  asm("nop"
+  : "=r"(lockval));
+}
+// CHECK-LABEL: _raw_spin_lock
+// CHECK-LABEL: entry:
+
+// CHECK-STRUCT:  %lockval = alloca %struct.anon, align 2
+// CHECK-STRUCT:  store i16 1
+// CHECK-STRUCT:  store i16 2
+// CHECK-STRUCT: [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-STRUCT: [[CAST:%[0-9]+]] = bitcast %struct.anon* %lockval to i32*
+// CHECK-STRUCT: store i32 [[RES]], i32* [[CAST]], align 2
+
+// CHECK-NOSTRUCT: %lockval = alloca i32, align 4
+// CHECK-NOSTRUCT: store i32 3
+// CHECK-NOSTRUCT:  [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-NOSTRUCT: store i32 [[RES]], i32* %lockval, align 4
+
+// Check Clang correctly handles a structure with padding.
+void unusual_struct(void) {
+  struct {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+
+// Check Clang reports an error if attempting to return a structure for which
+// no direct conversion to a register is possible.
+void odd_struct(void) {
+#ifdef IMPOSSIBLE_ODD
+  struct __attribute__((__packed__)) {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+#endif
+}
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+
+// Check Clang reports an error if attempting to return a big structure via a register.
+void big_struct(void) {
+#ifdef IMPOSSIBLE_BIG
+  struct {
+long long int v1, v2, v3, v4;
+  } str;
+  asm("nop"
+  : "=r"(str));
+#endif
+}
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+
+// Clang is able to emit LLVM IR for an 16-byte structure.
+void x_constraint_fit() {
+#ifdef POSSIBLE_X
+  struct S {
+unsigned x[4];
+  } z;
+  asm volatile("nop"
+   : "=x"(z));
+#endif
+}
+// CHECK-LABEL: x_constraint_fit
+// CHECK-X: %z = alloca %struct.S, align 4
+// CHECK-X: [[RES:%[0-9]+]] = call i128 asm sideeffect "nop", "=x,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-X: [[CAST:%[0-9]+]] = bitcast %struct.S* %z to i128*
+// CHECK-X: store i128 [[RES]], i128* [[CAST]], align 4
+// CHECK-X: ret
+
+// Clang is unable to emit LLVM IR for a 32-byte structure.
+void x_constraint_nofit() {
+#ifdef IMPOSSIBLE_X
+  struct S {
+unsigned x[8];
+  } z;
+  asm volatile("nop"
+   : "=x"(z));
+#endif
+}
+
+// CHECK-IMPOSSIBLE_X: invalid output size for constraint
Index: clang/test/CodeGen/asm-attrs.c
===
--- clang/test/CodeGen/asm-attrs.c
+++ clang/test/CodeGen/asm-attrs.c
@@ -8,7 +8,7 @@
 // CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
 // CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
 // CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
-// CHECK: call void asm "foo8", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
 // CHECK: attributes [[READNONE]] = { nounwind readnone }
 // CHECK: attributes [[NOATTRS]] = { nounwind }
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1984,6 +1984,7 @@
   std::vector ResultTruncRegTypes;
   std::vector ArgTypes;
   std::vector Args;
+  llvm::BitVector ResultTypeRequiresCast;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2022,13 +2023,23 @@
 
 // If this is a register 

[PATCH] D66948: [CodeGen]: fix error message for "=r" asm constraint

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
glider added reviewers: eli.friedman, thakis.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Nico Weber reported that the following code:

  char buf[9];
  asm("" : "=r" (buf));

yields the "impossible constraint in asm: can't store struct into a register"
error message, although |buf| is not a struct (see
http://crbug.com/999160).

Make the error message more generic and add a test for it.
Also make sure other tests in x86_64-PR42672.c check for the full error
message.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66948

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/x86_64-PR42672.c


Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- clang/test/CodeGen/x86_64-PR42672.c
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -4,6 +4,7 @@
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s 
-o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
 
 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -57,7 +58,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm: can't store value into 
a register
 
 // Check Clang reports an error if attempting to return a big structure via a 
register.
 void big_struct(void) {
@@ -69,7 +70,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm: can't store value into 
a register
 
 // Clang is able to emit LLVM IR for an 16-byte structure.
 void x_constraint_fit() {
@@ -100,3 +101,17 @@
 }
 
 // CHECK-IMPOSSIBLE_X: invalid output size for constraint
+
+
+// http://crbug.com/999160
+// Clang used to report the following message
+//   impossible constraint in asm: can't store struct into a register
+// for the assembly directive below, although there's no struct.
+void crbug_999160_regtest() {
+#ifdef IMPOSSIBLE_9BYTES
+  char buf[9];
+  asm("" : "=r" (buf));
+#endif
+}
+
+// CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value 
into a register
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2326,7 +2326,7 @@
 const Expr *OutExpr = S.getOutputExpr(i);
 CGM.Error(
 OutExpr->getExprLoc(),
-"impossible constraint in asm: can't store struct into a 
register");
+"impossible constraint in asm: can't store value into a register");
 return;
   }
   Dest = MakeAddrLValue(A, Ty);


Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- clang/test/CodeGen/x86_64-PR42672.c
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -4,6 +4,7 @@
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
 
 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -57,7 +58,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm: can't store value into a register
 
 // Check Clang reports an error if attempting to return a big structure via a register.
 void big_struct(void) {
@@ -69,7 +70,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm: can't store value into a register
 
 // Clang is able to emit LLVM IR for an 16-byte structure.
 void x_constraint_fit() {
@@ -100,3 +101,17 @@
 }
 
 // CHECK-IMPOSSIBLE_X: invalid output size for constraint
+
+
+// http://crbug.com/999160
+// Clang used to report the following message
+//   impossible constraint in asm: can't store struct into a register
+// for the assembly directive below, although there's no struct.
+void crbug_999160_regtest() {
+#ifdef 

[PATCH] D66948: [CodeGen]: fix error message for "=r" asm constraint

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 217887.
glider added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66948

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/x86_64-PR42672.c


Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- clang/test/CodeGen/x86_64-PR42672.c
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -4,6 +4,7 @@
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s 
-o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
 
 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -57,7 +58,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm: can't store value into 
a register
 
 // Check Clang reports an error if attempting to return a big structure via a 
register.
 void big_struct(void) {
@@ -69,7 +70,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm: can't store value into 
a register
 
 // Clang is able to emit LLVM IR for an 16-byte structure.
 void x_constraint_fit() {
@@ -100,3 +101,17 @@
 }
 
 // CHECK-IMPOSSIBLE_X: invalid output size for constraint
+
+// http://crbug.com/999160
+// Clang used to report the following message
+//   impossible constraint in asm: can't store struct into a register
+// for the assembly directive below, although there's no struct.
+void crbug_999160_regtest() {
+#ifdef IMPOSSIBLE_9BYTES
+  char buf[9];
+  asm(""
+  : "=r"(buf));
+#endif
+}
+
+// CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value 
into a register
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2326,7 +2326,7 @@
 const Expr *OutExpr = S.getOutputExpr(i);
 CGM.Error(
 OutExpr->getExprLoc(),
-"impossible constraint in asm: can't store struct into a 
register");
+"impossible constraint in asm: can't store value into a register");
 return;
   }
   Dest = MakeAddrLValue(A, Ty);


Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- clang/test/CodeGen/x86_64-PR42672.c
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -4,6 +4,7 @@
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
 
 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -57,7 +58,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm: can't store value into a register
 
 // Check Clang reports an error if attempting to return a big structure via a register.
 void big_struct(void) {
@@ -69,7 +70,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm: can't store value into a register
 
 // Clang is able to emit LLVM IR for an 16-byte structure.
 void x_constraint_fit() {
@@ -100,3 +101,17 @@
 }
 
 // CHECK-IMPOSSIBLE_X: invalid output size for constraint
+
+// http://crbug.com/999160
+// Clang used to report the following message
+//   impossible constraint in asm: can't store struct into a register
+// for the assembly directive below, although there's no struct.
+void crbug_999160_regtest() {
+#ifdef IMPOSSIBLE_9BYTES
+  char buf[9];
+  asm(""
+  : "=r"(buf));
+#endif
+}
+
+// CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value into a register
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2326,7 +2326,7 @@
 const 

[PATCH] D61879: WIP: Prototype of DSE optimizations for -ftrivial-auto-var-init

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

In D61879#1621423 , @vitalybuka wrote:

> Fix https://godbolt.org/z/-PinQP


Please consider adding a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61879



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

+srhines
Heads up: this patch reduces the size of the following Android kernel functions:

  _raw_spin_lock and _raw_spin_lock_bh: 132 bytes->68 bytes
  _raw_spin_lock_irq: 136 bytes->72 bytes
  _raw_spin_lock_irqsave: 144 bytes->80 bytes
  _raw_spin_trylock: 156 bytes->112 bytes
  _raw_spin_trylock_bh: 136 bytes->92 bytes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 212293.
glider added a comment.

Test for =X somehow sneaked in - drop it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/asm-attrs.c
  clang/test/CodeGen/x86_64-PR42672.c

Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- /dev/null
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DSTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-STRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -USTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-NOSTRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_ODD -emit-llvm %s -o - 2> %t || true
+// RUN: grep "impossible constraint in asm" %t
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG -emit-llvm %s -o - 2> %t || true
+// RUN: grep "impossible constraint in asm" %t
+
+// Make sure Clang doesn't treat |lockval| as asm input.
+void _raw_spin_lock(void) {
+#if STRUCT
+  struct {
+unsigned short owner, next;
+  } lockval;
+  lockval.owner = 1;
+  lockval.next = 2;
+#else
+  int lockval;
+  lockval = 3;
+#endif
+  asm("nop"
+  : "=r"(lockval));
+}
+// CHECK-LABEL: _raw_spin_lock
+// CHECK-LABEL: entry:
+
+// CHECK-STRUCT:  %lockval = alloca %struct.anon, align 2
+// CHECK-STRUCT:  store i16 1
+// CHECK-STRUCT:  store i16 2
+// CHECK-STRUCT: [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-STRUCT: [[CAST:%[0-9]+]] = bitcast %struct.anon* %lockval to i32*
+// CHECK-STRUCT: store i32 [[RES]], i32* [[CAST]], align 2
+
+// CHECK-NOSTRUCT: %lockval = alloca i32, align 4
+// CHECK-NOSTRUCT: store i32 3
+// CHECK-NOSTRUCT:  [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-NOSTRUCT: store i32 [[RES]], i32* %lockval, align 4
+
+// Check Clang correctly handles a structure with padding.
+void unusual_struct(void) {
+  struct {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+
+// Check Clang reports an error if attempting to return a structure for which
+// no direct conversion to a register is possible.
+#ifdef IMPOSSIBLE_ODD
+void odd_struct(void) {
+  struct __attribute__((__packed__)) {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+#endif
+
+// Check Clang reports an error if attempting to return a big structure via a register.
+#ifdef IMPOSSIBLE_BIG
+void big_struct(void) {
+  struct {
+long long int v1, v2, v3, v4;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+#endif
Index: clang/test/CodeGen/asm-attrs.c
===
--- clang/test/CodeGen/asm-attrs.c
+++ clang/test/CodeGen/asm-attrs.c
@@ -8,7 +8,7 @@
 // CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
 // CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
 // CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
-// CHECK: call void asm "foo8", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
 // CHECK: attributes [[READNONE]] = { nounwind readnone }
 // CHECK: attributes [[NOATTRS]] = { nounwind }
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1986,6 +1986,7 @@
   std::vector ResultTruncRegTypes;
   std::vector ArgTypes;
   std::vector Args;
+  llvm::BitVector ResultTypeRequiresCast;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2024,13 +2025,23 @@
 
 // If this is a register output, then make the inline asm return it
 // by-value.  If this is a memory result, return the value by-reference.
-if (!Info.allowsMemory() && hasScalarEvaluationKind(OutExpr->getType())) {
+bool isScalarizableAggregate =
+hasAggregateEvaluationKind(OutExpr->getType());
+if (!Info.allowsMemory() && (hasScalarEvaluationKind(OutExpr->getType()) ||
+ isScalarizableAggregate)) {
   Constraints += "=" + OutputConstraint;
   ResultRegQualTys.push_back(OutExpr->getType());
   ResultRegDests.push_back(Dest);
-  ResultRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
-  ResultTruncRegTypes.push_back(ResultRegTypes.back());
-
+  ResultTruncRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
+  if (Info.allowsRegister() && isScalarizableAggregate) {
+ResultTypeRequiresCast.push_back(true);
+unsigned Size = getContext().getTypeSize(OutExpr->getType());
+llvm::Type *ConvTy = llvm::IntegerType::get(getLLVMContext(), Size);
+ResultRegTypes.push_back(ConvTy);
+  } else {
+ResultTypeRequiresCast.push_back(false);
+

[PATCH] D61879: WIP: Prototype of DSE optimizations for -ftrivial-auto-var-init

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Vitaly, what's the current status of these patches? What's your plan on 
submitting them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61879



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

[PATCH] D64742: Allow using -ftrivial-auto-var-init=zero in C mode without extra flags

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

As a data point, Linus Torvalds suggested that we need a similar feature for 
GCC so that the "kernel C standard" mandates zero-initialization for locals: 
https://lkml.org/lkml/2019/7/28/206


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64742



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 213920.
glider added a comment.

Added test cases for =x, replaced grep with not


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/asm-attrs.c
  clang/test/CodeGen/x86_64-PR42672.c

Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- /dev/null
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -0,0 +1,102 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DSTRUCT -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-STRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -USTRUCT -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NOSTRUCT
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_ODD -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_ODD
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+
+// Make sure Clang doesn't treat |lockval| as asm input.
+void _raw_spin_lock(void) {
+#ifdef STRUCT
+  struct {
+unsigned short owner, next;
+  } lockval;
+  lockval.owner = 1;
+  lockval.next = 2;
+#else
+  int lockval;
+  lockval = 3;
+#endif
+  asm("nop"
+  : "=r"(lockval));
+}
+// CHECK-LABEL: _raw_spin_lock
+// CHECK-LABEL: entry:
+
+// CHECK-STRUCT:  %lockval = alloca %struct.anon, align 2
+// CHECK-STRUCT:  store i16 1
+// CHECK-STRUCT:  store i16 2
+// CHECK-STRUCT: [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-STRUCT: [[CAST:%[0-9]+]] = bitcast %struct.anon* %lockval to i32*
+// CHECK-STRUCT: store i32 [[RES]], i32* [[CAST]], align 2
+
+// CHECK-NOSTRUCT: %lockval = alloca i32, align 4
+// CHECK-NOSTRUCT: store i32 3
+// CHECK-NOSTRUCT:  [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-NOSTRUCT: store i32 [[RES]], i32* %lockval, align 4
+
+// Check Clang correctly handles a structure with padding.
+void unusual_struct(void) {
+  struct {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+
+// Check Clang reports an error if attempting to return a structure for which
+// no direct conversion to a register is possible.
+void odd_struct(void) {
+#ifdef IMPOSSIBLE_ODD
+  struct __attribute__((__packed__)) {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+#endif
+}
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+
+// Check Clang reports an error if attempting to return a big structure via a register.
+void big_struct(void) {
+#ifdef IMPOSSIBLE_BIG
+  struct {
+long long int v1, v2, v3, v4;
+  } str;
+  asm("nop"
+  : "=r"(str));
+#endif
+}
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+
+// Clang is able to emit LLVM IR for an 16-byte structure.
+void x_constraint_fit() {
+#ifdef POSSIBLE_X
+  struct S {
+unsigned x[4];
+  } z;
+  asm volatile("nop"
+   : "=x"(z));
+#endif
+}
+// CHECK-LABEL: x_constraint_fit
+// CHECK-X: %z = alloca %struct.S, align 4
+// CHECK-X: [[RES:%[0-9]+]] = call i128 asm sideeffect "nop", "=x,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-X: [[CAST:%[0-9]+]] = bitcast %struct.S* %z to i128*
+// CHECK-X: store i128 [[RES]], i128* [[CAST]], align 4
+// CHECK-X: ret
+
+// Clang is unable to emit LLVM IR for a 32-byte structure.
+void x_constraint_nofit() {
+#ifdef IMPOSSIBLE_X
+  struct S {
+unsigned x[8];
+  } z;
+  asm volatile("nop"
+   : "=x"(z));
+#endif
+}
+
+// CHECK-IMPOSSIBLE_X: invalid output size for constraint
Index: clang/test/CodeGen/asm-attrs.c
===
--- clang/test/CodeGen/asm-attrs.c
+++ clang/test/CodeGen/asm-attrs.c
@@ -8,7 +8,7 @@
 // CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
 // CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
 // CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
-// CHECK: call void asm "foo8", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
 // CHECK: attributes [[READNONE]] = { nounwind readnone }
 // CHECK: attributes [[NOATTRS]] = { nounwind }
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1984,6 +1984,7 @@
   std::vector ResultTruncRegTypes;
   std::vector ArgTypes;
   std::vector Args;
+  llvm::BitVector ResultTypeRequiresCast;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2022,13 +2023,23 @@
 
 

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider marked an inline comment as done.
glider added a comment.

In D65234#1615649 , @efriedma wrote:

> For the "=x" thing I was talking about, try the following testcase:
>
>   void a() { struct S { unsigned x[4]; } z; asm volatile("%0":"=x"(z)); }
>   void a2() { struct S { unsigned x[8]; } z; asm volatile("%0":"=x"(z)); }
>
>
> clang trunk gives "error: couldn't allocate output register for constraint 
> 'x'" in the backend for both functions.  gcc prints "%xmm0" for the first, 
> and rejects the second; not exactly sure why it's rejecting the second, 
> though.  It would be nice if both worked, although I guess it's okay if we 
> print a reasonable error message.  Please add a testcase, at least.


Note that this is invalid assembly, as it emits a register name instead of an 
assembly instruction. I don't think it should work at all.
If we replace "%0" with a "nop", Clang will reject the second test, but will 
accept the first one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider marked 3 inline comments as done.
glider added inline comments.



Comment at: clang/test/CodeGen/PR42672.c:50
+  struct {
+long long int v1, v2, v3, v4;
+  } str;

efriedma wrote:
> glider wrote:
> > The acceptable size actually depends on the target platform. Not sure we 
> > can test for all of them, and we'll probably need to restrict this test to 
> > e.g. x86
> The interesting case for a 32-byte struct would actually be something like 
> `"=x"(str)`... which currently passes the clang frontend, since 32 is a legal 
> size for that constraint (although it eventually fails in the backend).
Changing "=r" to "=X" indeed makes this particular test pass (there's nothing 
to fail in the backend, as we don't actually generate instructions that write 
to memory)
I'm however unsure adding a test for "=X" makes any difference, as our patch is 
irrelevant to this constraint.



Comment at: clang/test/CodeGen/PR42672.c:40
+unsigned short first;
+unsigned char second;
+  } str;

efriedma wrote:
> This isn't a three-byte struct; it's a four-byte struct where one of the 
> bytes is only used for padding. 
You're right. Making this structure packed yields an error, which conforms to 
GCC behavior.
I've added a test for that as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 212164.
glider added a comment.

Addressed Eli's comments, added a test for a packed struct


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/asm-attrs.c
  clang/test/CodeGen/x86_64-PR42672.c

Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- /dev/null
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DSTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-STRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -USTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-NOSTRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_ODD -emit-llvm %s -o - 2> %t || true
+// RUN: grep "impossible constraint in asm" %t
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG -emit-llvm %s -o - 2> %t || true
+// RUN: grep "impossible constraint in asm" %t
+
+// Make sure Clang doesn't treat |lockval| as asm input.
+void _raw_spin_lock(void) {
+#if STRUCT
+  struct {
+unsigned short owner, next;
+  } lockval;
+  lockval.owner = 1;
+  lockval.next = 2;
+#else
+  int lockval;
+  lockval = 3;
+#endif
+  asm("nop"
+  : "=r"(lockval));
+}
+// CHECK-LABEL: _raw_spin_lock
+// CHECK-LABEL: entry:
+
+// CHECK-STRUCT:  %lockval = alloca %struct.anon, align 2
+// CHECK-STRUCT:  store i16 1
+// CHECK-STRUCT:  store i16 2
+// CHECK-STRUCT: [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-STRUCT: [[CAST:%[0-9]+]] = bitcast %struct.anon* %lockval to i32*
+// CHECK-STRUCT: store i32 [[RES]], i32* [[CAST]], align 2
+
+// CHECK-NOSTRUCT: %lockval = alloca i32, align 4
+// CHECK-NOSTRUCT: store i32 3
+// CHECK-NOSTRUCT:  [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-NOSTRUCT: store i32 [[RES]], i32* %lockval, align 4
+
+// Check Clang correctly handles a structure with padding.
+void unusual_struct(void) {
+  struct {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+
+// Check Clang reports an error if attempting to return a structure for which
+// no direct conversion to a register is possible.
+#ifdef IMPOSSIBLE_ODD
+void odd_struct(void) {
+  struct __attribute__((__packed__)) {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+#endif
+
+// Check Clang reports an error if attempting to return a big structure via a register.
+#ifdef IMPOSSIBLE_BIG
+void big_struct(void) {
+  struct {
+long long int v1, v2, v3, v4;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+#endif
+
+// Check Clang reports an error if attempting to return a big structure via a register.
+void big_struct(void) {
+  struct {
+long long int v1, v2, v3, v4;
+  } str;
+  asm("nop"
+  : "=X"(str));
+}
Index: clang/test/CodeGen/asm-attrs.c
===
--- clang/test/CodeGen/asm-attrs.c
+++ clang/test/CodeGen/asm-attrs.c
@@ -8,7 +8,7 @@
 // CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
 // CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
 // CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
-// CHECK: call void asm "foo8", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
 // CHECK: attributes [[READNONE]] = { nounwind readnone }
 // CHECK: attributes [[NOATTRS]] = { nounwind }
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1986,6 +1986,7 @@
   std::vector ResultTruncRegTypes;
   std::vector ArgTypes;
   std::vector Args;
+  llvm::BitVector ResultTypeRequiresCast;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2024,13 +2025,23 @@
 
 // If this is a register output, then make the inline asm return it
 // by-value.  If this is a memory result, return the value by-reference.
-if (!Info.allowsMemory() && hasScalarEvaluationKind(OutExpr->getType())) {
+bool isScalarizableAggregate =
+hasAggregateEvaluationKind(OutExpr->getType());
+if (!Info.allowsMemory() && (hasScalarEvaluationKind(OutExpr->getType()) ||
+ isScalarizableAggregate)) {
   Constraints += "=" + OutputConstraint;
   ResultRegQualTys.push_back(OutExpr->getType());
   ResultRegDests.push_back(Dest);
-  ResultRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
-  ResultTruncRegTypes.push_back(ResultRegTypes.back());
-
+  ResultTruncRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
+  if (Info.allowsRegister() && isScalarizableAggregate) {
+ResultTypeRequiresCast.push_back(true);
+unsigned Size = 

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Eli, any other comments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
glider added reviewers: eli.friedman, jyknight.
glider added a subscriber: vitalybuka.

The "=r" output constraint for a structure variable passed to inline asm
shouldn't be converted to "=*r", as this changes the asm directive
semantics and prevents DSE optimizations.
Instead, preserve the constraints and return such structures as integers
of corresponding size, which are converted back to structures when
storing the result.

Fixes PR42672.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65234

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/PR42672.c
  clang/test/CodeGen/asm-attrs.c

Index: clang/test/CodeGen/asm-attrs.c
===
--- clang/test/CodeGen/asm-attrs.c
+++ clang/test/CodeGen/asm-attrs.c
@@ -8,7 +8,7 @@
 // CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
 // CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
 // CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
-// CHECK: call void asm "foo8", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
 // CHECK: attributes [[READNONE]] = { nounwind readnone }
 // CHECK: attributes [[NOATTRS]] = { nounwind }
Index: clang/test/CodeGen/PR42672.c
===
--- /dev/null
+++ clang/test/CodeGen/PR42672.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -DSTRUCT -O2 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -USTRUCT -O2 -emit-llvm %s -o - | FileCheck %s
+//
+// Make sure Clang doesn't treat |lockval| as asm input, and therefore
+// optimizes away all stores to |lockval| together with the asm directive.
+void _raw_spin_lock(void) {
+#if STRUCT
+  struct {
+unsigned short owner, next;
+  } lockval;
+  lockval.owner = 1;
+  lockval.next = 2;
+#else
+  int lockval;
+  lockval = 3;
+#endif
+  asm("nop"
+  : "=r"(lockval));
+}
+// CHECK-LABEL: _raw_spin_lock
+// CHECK-LABEL: entry:
+// CHECK-NEXT: ret void
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1986,6 +1986,7 @@
   std::vector ResultTruncRegTypes;
   std::vector ArgTypes;
   std::vector Args;
+  std::vector ResultTypeRequiresCast;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2024,13 +2025,23 @@
 
 // If this is a register output, then make the inline asm return it
 // by-value.  If this is a memory result, return the value by-reference.
-if (!Info.allowsMemory() && hasScalarEvaluationKind(OutExpr->getType())) {
+bool isScalarizableAggregate =
+hasAggregateEvaluationKind(OutExpr->getType());
+if (!Info.allowsMemory() && (hasScalarEvaluationKind(OutExpr->getType()) ||
+ isScalarizableAggregate)) {
   Constraints += "=" + OutputConstraint;
   ResultRegQualTys.push_back(OutExpr->getType());
   ResultRegDests.push_back(Dest);
-  ResultRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
-  ResultTruncRegTypes.push_back(ResultRegTypes.back());
-
+  ResultTruncRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
+  if (Info.allowsRegister() && isScalarizableAggregate) {
+ResultTypeRequiresCast.push_back(true);
+unsigned Size = getContext().getTypeSize(OutExpr->getType());
+llvm::Type *ConvTy = llvm::IntegerType::get(getLLVMContext(), Size);
+ResultRegTypes.push_back(ConvTy);
+  } else {
+ResultTypeRequiresCast.push_back(false);
+ResultRegTypes.push_back(ResultTruncRegTypes.back());
+  }
   // If this output is tied to an input, and if the input is larger, then
   // we need to set the actual result type of the inline asm node to be the
   // same as the input type.
@@ -2273,6 +2284,7 @@
   assert(RegResults.size() == ResultRegTypes.size());
   assert(RegResults.size() == ResultTruncRegTypes.size());
   assert(RegResults.size() == ResultRegDests.size());
+  assert(ResultTypeRequiresCast.size() <= ResultRegDests.size());
   for (unsigned i = 0, e = RegResults.size(); i != e; ++i) {
 llvm::Value *Tmp = RegResults[i];
 
@@ -2302,7 +2314,17 @@
   }
 }
 
-EmitStoreThroughLValue(RValue::get(Tmp), ResultRegDests[i]);
+LValue Dest = ResultRegDests[i];
+// ResultTypeRequiresCast elements correspond to the first
+// ResultTypeRequiresCast.size() elements of RegResults.
+if ((i < ResultTypeRequiresCast.size()) && ResultTypeRequiresCast[i]) {
+  unsigned Size = getContext().getTypeSize(ResultRegQualTys[i]);
+  Address A = Builder.CreateBitCast(Dest.getAddress(),
+ResultRegTypes[i]->getPointerTo());
+  Dest = MakeAddrLValue(
+  A, getContext().getIntTypeForBitwidth(Size, /*Signed*/ false));
+}
+

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider marked 2 inline comments as done.
glider added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:1989
   std::vector Args;
+  std::vector ResultTypeRequiresCast;
 

nickdesaulniers wrote:
> Are we able to use something other than `std::vector` here from ADT?
> http://llvm.org/docs/ProgrammersManual.html#bit-storage-containers-bitvector-sparsebitvector
I don't think `std::vector` is a bottleneck here, but since it might be 
deprecated someday let's just not use it.



Comment at: clang/lib/CodeGen/CGStmt.cpp:2287
   assert(RegResults.size() == ResultRegDests.size());
+  assert(ResultTypeRequiresCast.size() <= ResultRegDests.size());
   for (unsigned i = 0, e = RegResults.size(); i != e; ++i) {

efriedma wrote:
> Not "=="?
Turns out ResultRegDests can be also extended by `addReturnRegisterOutputs` 
above (line 2122), so no.
I've added a comment to clarify that.



Comment at: clang/lib/CodeGen/CGStmt.cpp:2325
+  Dest = MakeAddrLValue(
+  A, getContext().getIntTypeForBitwidth(Size, /*Signed*/ false));
+}

efriedma wrote:
> Will this work if the struct is an unusual size, like `sizeof(struct s) == 3` 
> or `sizeof(struct s) == 32`?  (3 is unlikely to show up in real code, but 32 
> could correspond to a vector register.)
For a 3-byte struct this works as follows: a 3-byte structure is allocated on 
the stack with alignment 2, then the assembly returns a 4-byte integer that's 
written to that 3-byte structure.
This conforms to what GCC does, and I think it's legal to let the assembly 
write more than a structure size here, as the corresponding register size is 
bigger (i.e. it's a bug in the C code, not Clang).

For a 32-bit struct we crash now, whereas GCC reports an "impossible constraint 
in ‘asm’"
It's interesting that the crash happens in the frontend, i.e. it's somehow 
knows the maximum possible size for the register operand.
We can check that `getContext().getIntTypeForBitwidth(Size, /*Signed*/ false)` 
is a nonnull type to prevent the crashes.



Comment at: clang/test/CodeGen/PR42672.c:50
+  struct {
+long long int v1, v2, v3, v4;
+  } str;

The acceptable size actually depends on the target platform. Not sure we can 
test for all of them, and we'll probably need to restrict this test to e.g. x86


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 211919.
glider marked 2 inline comments as done.
glider added a comment.

Fixed comments from Eli and Nick, added tests for unusual struct sizes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/PR42672.c
  clang/test/CodeGen/asm-attrs.c

Index: clang/test/CodeGen/asm-attrs.c
===
--- clang/test/CodeGen/asm-attrs.c
+++ clang/test/CodeGen/asm-attrs.c
@@ -8,7 +8,7 @@
 // CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
 // CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
 // CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
-// CHECK: call void asm "foo8", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
 // CHECK: attributes [[READNONE]] = { nounwind readnone }
 // CHECK: attributes [[NOATTRS]] = { nounwind }
Index: clang/test/CodeGen/PR42672.c
===
--- /dev/null
+++ clang/test/CodeGen/PR42672.c
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -DSTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-STRUCT
+// RUN: %clang_cc1 -USTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-NOSTRUCT
+// RUN: %clang_cc1 -DIMPOSSIBLE -emit-llvm %s -o - 2> %t || true
+// RUN: grep "impossible constraint in asm" %t
+
+// Make sure Clang doesn't treat |lockval| as asm input.
+void _raw_spin_lock(void) {
+#if STRUCT
+  struct {
+unsigned short owner, next;
+  } lockval;
+  lockval.owner = 1;
+  lockval.next = 2;
+#else
+  int lockval;
+  lockval = 3;
+#endif
+  asm("nop"
+  : "=r"(lockval));
+}
+// CHECK-LABEL: _raw_spin_lock
+// CHECK-LABEL: entry:
+
+// CHECK-STRUCT:  %lockval = alloca %struct.anon, align 2
+// CHECK-STRUCT:  store i16 1
+// CHECK-STRUCT:  store i16 2
+// CHECK-STRUCT: [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-STRUCT: [[CAST:%[0-9]+]] = bitcast %struct.anon* %lockval to i32*
+// CHECK-STRUCT: store i32 [[RES]], i32* [[CAST]], align 2
+
+// CHECK-NOSTRUCT: %lockval = alloca i32, align 4
+// CHECK-NOSTRUCT: store i32 3
+// CHECK-NOSTRUCT:  [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-NOSTRUCT: store i32 [[RES]], i32* %lockval, align 4
+
+// Check Clang correctly handles sizes for which no direct register conversion is possible.
+void unusual_struct(void) {
+  struct {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+
+// Check Clang reports an error if attempting to return a big structure via a register.
+#ifdef IMPOSSIBLE
+void big_struct(void) {
+  struct {
+long long int v1, v2, v3, v4;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+#endif
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1986,6 +1986,7 @@
   std::vector ResultTruncRegTypes;
   std::vector ArgTypes;
   std::vector Args;
+  llvm::BitVector ResultTypeRequiresCast;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2024,13 +2025,23 @@
 
 // If this is a register output, then make the inline asm return it
 // by-value.  If this is a memory result, return the value by-reference.
-if (!Info.allowsMemory() && hasScalarEvaluationKind(OutExpr->getType())) {
+bool isScalarizableAggregate =
+hasAggregateEvaluationKind(OutExpr->getType());
+if (!Info.allowsMemory() && (hasScalarEvaluationKind(OutExpr->getType()) ||
+ isScalarizableAggregate)) {
   Constraints += "=" + OutputConstraint;
   ResultRegQualTys.push_back(OutExpr->getType());
   ResultRegDests.push_back(Dest);
-  ResultRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
-  ResultTruncRegTypes.push_back(ResultRegTypes.back());
-
+  ResultTruncRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
+  if (Info.allowsRegister() && isScalarizableAggregate) {
+ResultTypeRequiresCast.push_back(true);
+unsigned Size = getContext().getTypeSize(OutExpr->getType());
+llvm::Type *ConvTy = llvm::IntegerType::get(getLLVMContext(), Size);
+ResultRegTypes.push_back(ConvTy);
+  } else {
+ResultTypeRequiresCast.push_back(false);
+ResultRegTypes.push_back(ResultTruncRegTypes.back());
+  }
   // If this output is tied to an input, and if the input is larger, then
   // we need to set the actual result type of the inline asm node to be the
   // same as the input type.
@@ -2273,6 +2284,9 @@
   assert(RegResults.size() == ResultRegTypes.size());
   assert(RegResults.size() == ResultTruncRegTypes.size());
   assert(RegResults.size() == ResultRegDests.size());
+  // ResultRegDests can be also populated by 

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 211920.
glider added a comment.

Make big_struct() test triple-specific


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/PR42672.c
  clang/test/CodeGen/asm-attrs.c

Index: clang/test/CodeGen/asm-attrs.c
===
--- clang/test/CodeGen/asm-attrs.c
+++ clang/test/CodeGen/asm-attrs.c
@@ -8,7 +8,7 @@
 // CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
 // CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
 // CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
-// CHECK: call void asm "foo8", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
 // CHECK: attributes [[READNONE]] = { nounwind readnone }
 // CHECK: attributes [[NOATTRS]] = { nounwind }
Index: clang/test/CodeGen/PR42672.c
===
--- /dev/null
+++ clang/test/CodeGen/PR42672.c
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -DSTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-STRUCT
+// RUN: %clang_cc1 -USTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-NOSTRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE -emit-llvm %s -o - 2> %t || true
+// RUN: grep "impossible constraint in asm" %t
+
+// Make sure Clang doesn't treat |lockval| as asm input.
+void _raw_spin_lock(void) {
+#if STRUCT
+  struct {
+unsigned short owner, next;
+  } lockval;
+  lockval.owner = 1;
+  lockval.next = 2;
+#else
+  int lockval;
+  lockval = 3;
+#endif
+  asm("nop"
+  : "=r"(lockval));
+}
+// CHECK-LABEL: _raw_spin_lock
+// CHECK-LABEL: entry:
+
+// CHECK-STRUCT:  %lockval = alloca %struct.anon, align 2
+// CHECK-STRUCT:  store i16 1
+// CHECK-STRUCT:  store i16 2
+// CHECK-STRUCT: [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-STRUCT: [[CAST:%[0-9]+]] = bitcast %struct.anon* %lockval to i32*
+// CHECK-STRUCT: store i32 [[RES]], i32* [[CAST]], align 2
+
+// CHECK-NOSTRUCT: %lockval = alloca i32, align 4
+// CHECK-NOSTRUCT: store i32 3
+// CHECK-NOSTRUCT:  [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-NOSTRUCT: store i32 [[RES]], i32* %lockval, align 4
+
+// Check Clang correctly handles sizes for which no direct register conversion is possible.
+void unusual_struct(void) {
+  struct {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+
+// Check Clang reports an error if attempting to return a big structure via a register.
+#ifdef IMPOSSIBLE
+void big_struct(void) {
+  struct {
+long long int v1, v2, v3, v4;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+#endif
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1986,6 +1986,7 @@
   std::vector ResultTruncRegTypes;
   std::vector ArgTypes;
   std::vector Args;
+  llvm::BitVector ResultTypeRequiresCast;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2024,13 +2025,23 @@
 
 // If this is a register output, then make the inline asm return it
 // by-value.  If this is a memory result, return the value by-reference.
-if (!Info.allowsMemory() && hasScalarEvaluationKind(OutExpr->getType())) {
+bool isScalarizableAggregate =
+hasAggregateEvaluationKind(OutExpr->getType());
+if (!Info.allowsMemory() && (hasScalarEvaluationKind(OutExpr->getType()) ||
+ isScalarizableAggregate)) {
   Constraints += "=" + OutputConstraint;
   ResultRegQualTys.push_back(OutExpr->getType());
   ResultRegDests.push_back(Dest);
-  ResultRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
-  ResultTruncRegTypes.push_back(ResultRegTypes.back());
-
+  ResultTruncRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
+  if (Info.allowsRegister() && isScalarizableAggregate) {
+ResultTypeRequiresCast.push_back(true);
+unsigned Size = getContext().getTypeSize(OutExpr->getType());
+llvm::Type *ConvTy = llvm::IntegerType::get(getLLVMContext(), Size);
+ResultRegTypes.push_back(ConvTy);
+  } else {
+ResultTypeRequiresCast.push_back(false);
+ResultRegTypes.push_back(ResultTruncRegTypes.back());
+  }
   // If this output is tied to an input, and if the input is larger, then
   // we need to set the actual result type of the inline asm node to be the
   // same as the input type.
@@ -2273,6 +2284,9 @@
   assert(RegResults.size() == ResultRegTypes.size());
   assert(RegResults.size() == ResultTruncRegTypes.size());
   assert(RegResults.size() == ResultRegDests.size());
+  // ResultRegDests can be also populated by addReturnRegisterOutputs() above,
+  // in which case 

[PATCH] D64742: Allow using -ftrivial-auto-var-init=zero in C mode without extra flags

[Alexander Potapenko via Phabricator via cfe-commits]

glider added subscribers: rsmith, dvyukov, dblaikie, arthur.j.odwyer, fwyzard, 
kristina, mehdi_amini, troyj.
glider added a comment.

Adding more people from the original discussion.

Folks, we're now stuck in a situation where there's a potential buy-in from the 
Linux kernel community for stack initialization, but they won't ever use an 
option guarded by 
-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang. On 
the other hand, using pattern initialization is still costly, and will probably 
remain such, as there are little investments into Clang's DSE. (The nature of 
pattern initialization also suggests there'll always be corner cases in which 
zero initialization is still cheaper).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64742



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

[PATCH] D64742: Allow using -ftrivial-auto-var-init=zero in C mode without extra flags

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

In D64742#1717054 , @mehdi_amini wrote:

> In D64742#1606244 , @glider wrote:
>
> > As a data point, Linus Torvalds suggested that we need a similar feature 
> > for GCC so that the "kernel C standard" mandates zero-initialization for 
> > locals: https://lkml.org/lkml/2019/7/28/206
>
>
> I'm wondering why they never pushed all the way for a `-std=linux-c` flag 
> instead, and produced a documentation with the behavior they want with 
> respect to the base C standard they align on.


Guess this was never necessary, as GCC used to be the only compiler that could 
build the Linux kernel, and the standard could be defined by a combination of 
GCC flags.
This is no more the case, but adding a separate "standard" (which may end up 
being vaguely defined and containing ad-hoc hacks) will require a lot of effort 
from the three communities (Linux, GCC and LLVM)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64742



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

[PATCH] D80668: [Clang][Sanitizers] Expect test failure on {arm,thumb}v7

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: clang/test/CodeGen/sanitize-coverage.c:8
+//
+// XFAIL: armv7, thumbv7
 

Is there a Bugzilla entry for this? Please add a link to the code and to the 
patch description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80668



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

[PATCH] D81390: [KernelAddressSanitizer] Make globals constructors compatible with kernel [v2]

[Alexander Potapenko via Phabricator via cfe-commits]

glider accepted this revision.
glider added a comment.
This revision is now accepted and ready to land.

LGTM assuming allyesconfig builds.




Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:759
 // do globals-gc.
-UseCtorComdat(UseGlobalsGC && ClWithComdat) {
-this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
-this->CompileKernel =
-ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan : CompileKernel;
-
+UseCtorComdat(UseGlobalsGC && ClWithComdat && !this->CompileKernel) {
 C = &(M.getContext());

melver wrote:
> glider wrote:
> > `UseGlobalsGC` assumes `!this->CompileKernel` already, doesn't it?
> I experimented a bit and found that even if one of them is enabled things 
> break (say if you remove UseGlobalsGC here). This is more explicit and less 
> error-prone in case somebody removes UseGlobalsGC from UseCtorComdat. And 
> there is no real penalty here for having it on both.
That's strange, but ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81390



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

[PATCH] D80805: [KernelAddressSanitizer] Make globals constructors compatible with kernel

[Alexander Potapenko via Phabricator via cfe-commits]

glider accepted this revision.
glider 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/D80805/new/

https://reviews.llvm.org/D80805



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

[PATCH] D81390: [KernelAddressSanitizer] Make globals constructors compatible with kernel [v2]

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: clang/test/CodeGen/asan-globals.cpp:16
+int aliased_global;
+extern int __attribute__((alias("aliased_global"))) __global_alias;
+int __special_global;

Need a comment explaining this case a bit.



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:759
 // do globals-gc.
-UseCtorComdat(UseGlobalsGC && ClWithComdat) {
-this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
-this->CompileKernel =
-ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan : CompileKernel;
-
+UseCtorComdat(UseGlobalsGC && ClWithComdat && !this->CompileKernel) {
 C = &(M.getContext());

`UseGlobalsGC` assumes `!this->CompileKernel` already, doesn't it?



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1796
+const GlobalAlias ) const {
+  if (CompileKernel) {
+// When compiling the kernel, globals that are aliased by symbols prefixed

Maybe it's better to call this function only if CompileKernel is true?
I don't think we need it except for the kernel.



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1938
 // supported on recent versions of ld64.
-bool ModuleAddressSanitizer::ShouldUseMachOGlobalsSection() const {
+bool ModuleAddressSanitizer::shouldUseMachOGlobalsSection() const {
   if (!TargetTriple.isOSBinFormatMachO())

This change looks unrelated to the rest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81390



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

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko 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 rGb0391dfc737e: [clang][Codegen] Introduce the 
disable_sanitizer_instrumentation attribute (authored by glider).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-disable-sanitizer-instrumentation.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/docs/BitCodeFormat.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Bitcode/attributes.ll
  llvm/test/Bitcode/compatibility.ll

Index: llvm/test/Bitcode/compatibility.ll
===
--- llvm/test/Bitcode/compatibility.ll
+++ llvm/test/Bitcode/compatibility.ll
@@ -1510,7 +1510,7 @@
   ; CHECK: select <2 x i1> , <2 x i8> , <2 x i8> 
 
   call void @f.nobuiltin() builtin
-  ; CHECK: call void @f.nobuiltin() #45
+  ; CHECK: call void @f.nobuiltin() #46
 
   call fastcc noalias i32* @f.noalias() noinline
   ; CHECK: call fastcc noalias i32* @f.noalias() #12
@@ -1907,6 +1907,9 @@
 declare void @f.nosanitize_coverage() nosanitize_coverage
 ; CHECK: declare void @f.nosanitize_coverage() #44
 
+declare void @f.disable_sanitizer_instrumentation() disable_sanitizer_instrumentation
+; CHECK: declare void @f.disable_sanitizer_instrumentation() #45
+
 ; immarg attribute
 declare void @llvm.test.immarg.intrinsic(i32 immarg)
 ; CHECK: declare void @llvm.test.immarg.intrinsic(i32 immarg)
@@ -1965,7 +1968,8 @@
 ; CHECK: attributes #42 = { speculatable }
 ; CHECK: attributes #43 = { strictfp }
 ; CHECK: attributes #44 = { nosanitize_coverage }
-; CHECK: attributes #45 = { builtin }
+; CHECK: attributes #45 = { disable_sanitizer_instrumentation }
+; CHECK: attributes #46 = { builtin }
 
 ;; Metadata
 
Index: llvm/test/Bitcode/attributes.ll
===
--- llvm/test/Bitcode/attributes.ll
+++ llvm/test/Bitcode/attributes.ll
@@ -472,6 +472,12 @@
   ret void
 }
 
+; CHECK: define void @f80() #50
+define void @f80() disable_sanitizer_instrumentation
+{
+ret void;
+}
+
 ; CHECK: attributes #0 = { noreturn }
 ; CHECK: attributes #1 = { nounwind }
 ; CHECK: attributes #2 = { readnone }
@@ -522,4 +528,5 @@
 ; CHECK: attributes #47 = { vscale_range(1,0) }
 ; CHECK: attributes #48 = { nosanitize_coverage }
 ; CHECK: attributes #49 = { noprofile }
+; CHECK: attributes #50 = { disable_sanitizer_instrumentation }
 ; CHECK: attributes #[[NOBUILTIN]] = { nobuiltin }
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -943,6 +943,7 @@
   // Those attributes should be safe to propagate to the extracted function.
   case Attribute::AlwaysInline:
   case Attribute::Cold:
+  case Attribute::DisableSanitizerInstrumentation:
   case Attribute::Hot:
   case Attribute::NoRecurse:
   case Attribute::InlineHint:
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -626,6 +626,8 @@
 return bitc::ATTR_KIND_IN_ALLOCA;
   case Attribute::Cold:
 return bitc::ATTR_KIND_COLD;
+  case Attribute::DisableSanitizerInstrumentation:
+return bitc::ATTR_KIND_DISABLE_SANITIZER_INSTRUMENTATION;
   case Attribute::Hot:
 return bitc::ATTR_KIND_HOT;
   case Attribute::ElementType:
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1388,6 +1388,8 @@
 return Attribute::Cold;
   case bitc::ATTR_KIND_CONVERGENT:
 return Attribute::Convergent;
+  case bitc::ATTR_KIND_DISABLE_SANITIZER_INSTRUMENTATION:
+return Attribute::DisableSanitizerInstrumentation;
   case bitc::ATTR_KIND_ELEMENTTYPE:
 return Attribute::ElementType;
   case bitc::ATTR_KIND_INACCESSIBLEMEM_ONLY:
Index: llvm/lib/AsmParser/LLLexer.cpp
===
--- llvm/lib/AsmParser/LLLexer.cpp
+++ llvm/lib/AsmParser/LLLexer.cpp
@@ -643,6 +643,7 @@
   KEYWORD(convergent);
   KEYWORD(dereferenceable);
   KEYWORD(dereferenceable_or_null);
+  

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 367759.
glider added a comment.

Updated LangRef.rst


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-disable-sanitizer-instrumentation.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/docs/BitCodeFormat.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Bitcode/attributes.ll
  llvm/test/Bitcode/compatibility.ll

Index: llvm/test/Bitcode/compatibility.ll
===
--- llvm/test/Bitcode/compatibility.ll
+++ llvm/test/Bitcode/compatibility.ll
@@ -1510,7 +1510,7 @@
   ; CHECK: select <2 x i1> , <2 x i8> , <2 x i8> 
 
   call void @f.nobuiltin() builtin
-  ; CHECK: call void @f.nobuiltin() #45
+  ; CHECK: call void @f.nobuiltin() #46
 
   call fastcc noalias i32* @f.noalias() noinline
   ; CHECK: call fastcc noalias i32* @f.noalias() #12
@@ -1907,6 +1907,9 @@
 declare void @f.nosanitize_coverage() nosanitize_coverage
 ; CHECK: declare void @f.nosanitize_coverage() #44
 
+declare void @f.disable_sanitizer_instrumentation() disable_sanitizer_instrumentation
+; CHECK: declare void @f.disable_sanitizer_instrumentation() #45
+
 ; immarg attribute
 declare void @llvm.test.immarg.intrinsic(i32 immarg)
 ; CHECK: declare void @llvm.test.immarg.intrinsic(i32 immarg)
@@ -1965,7 +1968,8 @@
 ; CHECK: attributes #42 = { speculatable }
 ; CHECK: attributes #43 = { strictfp }
 ; CHECK: attributes #44 = { nosanitize_coverage }
-; CHECK: attributes #45 = { builtin }
+; CHECK: attributes #45 = { disable_sanitizer_instrumentation }
+; CHECK: attributes #46 = { builtin }
 
 ;; Metadata
 
Index: llvm/test/Bitcode/attributes.ll
===
--- llvm/test/Bitcode/attributes.ll
+++ llvm/test/Bitcode/attributes.ll
@@ -472,6 +472,12 @@
   ret void
 }
 
+; CHECK: define void @f80() #50
+define void @f80() disable_sanitizer_instrumentation
+{
+ret void;
+}
+
 ; CHECK: attributes #0 = { noreturn }
 ; CHECK: attributes #1 = { nounwind }
 ; CHECK: attributes #2 = { readnone }
@@ -522,4 +528,5 @@
 ; CHECK: attributes #47 = { vscale_range(1,0) }
 ; CHECK: attributes #48 = { nosanitize_coverage }
 ; CHECK: attributes #49 = { noprofile }
+; CHECK: attributes #50 = { disable_sanitizer_instrumentation }
 ; CHECK: attributes #[[NOBUILTIN]] = { nobuiltin }
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -943,6 +943,7 @@
   // Those attributes should be safe to propagate to the extracted function.
   case Attribute::AlwaysInline:
   case Attribute::Cold:
+  case Attribute::DisableSanitizerInstrumentation:
   case Attribute::Hot:
   case Attribute::NoRecurse:
   case Attribute::InlineHint:
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -626,6 +626,8 @@
 return bitc::ATTR_KIND_IN_ALLOCA;
   case Attribute::Cold:
 return bitc::ATTR_KIND_COLD;
+  case Attribute::DisableSanitizerInstrumentation:
+return bitc::ATTR_KIND_DISABLE_SANITIZER_INSTRUMENTATION;
   case Attribute::Hot:
 return bitc::ATTR_KIND_HOT;
   case Attribute::ElementType:
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1388,6 +1388,8 @@
 return Attribute::Cold;
   case bitc::ATTR_KIND_CONVERGENT:
 return Attribute::Convergent;
+  case bitc::ATTR_KIND_DISABLE_SANITIZER_INSTRUMENTATION:
+return Attribute::DisableSanitizerInstrumentation;
   case bitc::ATTR_KIND_ELEMENTTYPE:
 return Attribute::ElementType;
   case bitc::ATTR_KIND_INACCESSIBLEMEM_ONLY:
Index: llvm/lib/AsmParser/LLLexer.cpp
===
--- llvm/lib/AsmParser/LLLexer.cpp
+++ llvm/lib/AsmParser/LLLexer.cpp
@@ -643,6 +643,7 @@
   KEYWORD(convergent);
   KEYWORD(dereferenceable);
   KEYWORD(dereferenceable_or_null);
+  KEYWORD(disable_sanitizer_instrumentation);
   KEYWORD(elementtype);
   KEYWORD(inaccessiblememonly);
   KEYWORD(inaccessiblemem_or_argmemonly);
Index: llvm/include/llvm/IR/Attributes.td

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

In D108029#2954604 , @glider wrote:

> In D108029#2954566 , @melver wrote:
>
>> llvm/docs/LangRef.rst also needs a corresponding change.
>
> Right, will do. I thought LangRef was missing the sanitizer bits as well, and 
> was planning to add them together, but apparently I was just looking for the 
> wrong attributes.

I've added the function attribute description to LangRef. Looks like global 
attributes aren't listed explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D108199: [msan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 367767.
glider added a comment.

Updated after landing the disable_sanitizer_instrumentation to LLVM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108199

Files:
  clang/docs/MemorySanitizer.rst
  clang/test/CodeGen/sanitize-memory-disable.c
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp


Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -5330,6 +5330,9 @@
   if (!CompileKernel && F.getName() == kMsanModuleCtorName)
 return false;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return false;
+
   MemorySanitizerVisitor Visitor(F, *this, TLI);
 
   // Clear out readonly/readnone attributes.
Index: clang/test/CodeGen/sanitize-memory-disable.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-memory-disable.c
@@ -0,0 +1,60 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes 
CHECK,WITHOUT %s
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=memory | FileCheck 
-check-prefixes CHECK,MSAN %s
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck 
-check-prefixes CHECK,KMSAN %s
+
+// Instrumented function.
+// MSan uses memset(addr, -1, size) to poison allocas and stores shadow of the 
return value in
+// __msan_retval_tls. KMSAN uses __msan_poison_alloca() to poison allocas and 
calls
+// __msan_get_context_state() at function prologue to access the task context 
struct (including the
+// shadow of the return value).
+//
+// CHECK: @instrumented1
+// KMSAN: __msan_get_context_state
+// WITHOUT-NOT: __msan_poison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN: @llvm.memset{{.*}}({{.*}}, i8 -1
+// KMSAN: __msan_poison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN: __msan_retval_tls
+// CHECK: ret i32
+int instrumented1(int *a) {
+  volatile char buf[8];
+  return *a;
+}
+
+// Function with no_sanitize("memory")/no_sanitize("kernel-memory"): no shadow 
propagation, but
+// unpoisons memory to prevent false positives.
+// MSan uses memset(addr, 0, size) to unpoison locals, KMSAN uses 
__msan_unpoison_alloca(). Both
+// tools still access the retval shadow to write 0 to it.
+//
+// CHECK: @no_false_positives1
+// KMSAN: __msan_get_context_state
+// WITHOUT-NOT: __msan_unpoison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN: @llvm.memset{{.*}}({{.*}}, i8 0
+// KMSAN: __msan_unpoison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN: __msan_retval_tls
+// CHECK: ret i32
+__attribute__((no_sanitize("memory")))
+__attribute__((no_sanitize("kernel-memory"))) int
+no_false_positives1(int *a) {
+  volatile char buf[8];
+  return *a;
+}
+
+// Function with disable_sanitizer_instrumentation: no instrumentation at all.
+//
+// CHECK: @no_instrumentation1
+// KMSAN-NOT: __msan_get_context_state
+// WITHOUT-NOT: __msan_poison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN-NOT: @llvm.memset{{.*}}({{.*}}, i8 0
+// KMSAN-NOT: __msan_unpoison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN-NOT: __msan_retval_tls
+// CHECK: ret i32
+__attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int 
*a) {
+  volatile char buf[8];
+  return *a;
+}
Index: clang/docs/MemorySanitizer.rst
===
--- clang/docs/MemorySanitizer.rst
+++ clang/docs/MemorySanitizer.rst
@@ -85,6 +85,15 @@
 avoid false positives.  This attribute may not be supported by other compilers,
 so we suggest to use it together with ``__has_feature(memory_sanitizer)``.
 
+``__attribute__((disable_sanitizer_instrumentation))``
+
+
+The ``disable_sanitizer_instrumentation`` attribute can be applied to a certain
+function to prevent all kinds of instrumentation. This attribute overrides
+``no_sanitize("memory")`` and may introduce false positives, so it should
+be used with care, e.g. when the user wants to ensure critical code does not
+have unexpected side effects.
+
 Ignorelist
 --
 


Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -5330,6 +5330,9 @@
   if (!CompileKernel && F.getName() == kMsanModuleCtorName)
 return false;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return false;
+
   MemorySanitizerVisitor Visitor(F, *this, TLI);
 
   // Clear out readonly/readnone attributes.
Index: clang/test/CodeGen/sanitize-memory-disable.c
===
--- /dev/null
+++ 

[PATCH] D108199: [msan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 367774.
glider marked 2 inline comments as done.
glider added a comment.

Addressed Marco's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108199

Files:
  clang/docs/MemorySanitizer.rst
  clang/test/CodeGen/sanitize-memory-disable.c
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp


Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -5330,6 +5330,9 @@
   if (!CompileKernel && F.getName() == kMsanModuleCtorName)
 return false;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return false;
+
   MemorySanitizerVisitor Visitor(F, *this, TLI);
 
   // Clear out readonly/readnone attributes.
Index: clang/test/CodeGen/sanitize-memory-disable.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-memory-disable.c
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes 
CHECK,WITHOUT %s
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=memory | FileCheck 
-check-prefixes CHECK,MSAN %s
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck 
-check-prefixes CHECK,KMSAN %s
+
+// Instrumented function.
+// MSan uses memset(addr, -1, size) to poison allocas and stores shadow of the 
return value in
+// __msan_retval_tls. KMSAN uses __msan_poison_alloca() to poison allocas and 
calls
+// __msan_get_context_state() at function prologue to access the task context 
struct (including the
+// shadow of the return value).
+//
+// CHECK-LABEL: i32 @instrumented1
+// KMSAN: __msan_get_context_state
+// WITHOUT-NOT: __msan_poison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN: @llvm.memset{{.*}}({{.*}}, i8 -1
+// KMSAN: __msan_poison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN: __msan_retval_tls
+// CHECK: ret i32
+int instrumented1(int *a) {
+  volatile char buf[8];
+  return *a;
+}
+
+// Function with no_sanitize("memory")/no_sanitize("kernel-memory"): no shadow 
propagation, but
+// unpoisons memory to prevent false positives.
+// MSan uses memset(addr, 0, size) to unpoison locals, KMSAN uses 
__msan_unpoison_alloca(). Both
+// tools still access the retval shadow to write 0 to it.
+//
+// CHECK-LABEL: i32 @no_false_positives1
+// KMSAN: __msan_get_context_state
+// WITHOUT-NOT: __msan_unpoison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN: @llvm.memset{{.*}}({{.*}}, i8 0
+// KMSAN: __msan_unpoison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN: __msan_retval_tls
+// CHECK: ret i32
+__attribute__((no_sanitize("memory"))) 
__attribute__((no_sanitize("kernel-memory"))) int no_false_positives1(int *a) {
+  volatile char buf[8];
+  return *a;
+}
+
+// Function with disable_sanitizer_instrumentation: no instrumentation at all.
+//
+// CHECK-LABEL: i32 @no_instrumentation1
+// KMSAN-NOT: __msan_get_context_state
+// WITHOUT-NOT: __msan_poison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN-NOT: @llvm.memset{{.*}}({{.*}}, i8 0
+// KMSAN-NOT: __msan_unpoison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN-NOT: __msan_retval_tls
+// CHECK: ret i32
+__attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int 
*a) {
+  volatile char buf[8];
+  return *a;
+}
Index: clang/docs/MemorySanitizer.rst
===
--- clang/docs/MemorySanitizer.rst
+++ clang/docs/MemorySanitizer.rst
@@ -85,6 +85,15 @@
 avoid false positives.  This attribute may not be supported by other compilers,
 so we suggest to use it together with ``__has_feature(memory_sanitizer)``.
 
+``__attribute__((disable_sanitizer_instrumentation))``
+
+
+The ``disable_sanitizer_instrumentation`` attribute can be applied to functions
+to prevent all kinds of instrumentation. As a result, it may introduce false
+positives and therefore should be used with care, and only if absolutely
+required; for example for certain code that cannot tolerate any instrumentation
+and resulting side-effects. This attribute overrides ``no_sanitize("memory")``.
+
 Ignorelist
 --
 


Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -5330,6 +5330,9 @@
   if (!CompileKernel && F.getName() == kMsanModuleCtorName)
 return false;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return false;
+
   MemorySanitizerVisitor Visitor(F, *this, TLI);
 
   // Clear out readonly/readnone attributes.
Index: clang/test/CodeGen/sanitize-memory-disable.c

[PATCH] D108199: [msan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko 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 rG8dc7dcdca1e0: [msan] Add support for 
disable_sanitizer_instrumentation attribute (authored by glider).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108199

Files:
  clang/docs/MemorySanitizer.rst
  clang/test/CodeGen/sanitize-memory-disable.c
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp


Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -5330,6 +5330,9 @@
   if (!CompileKernel && F.getName() == kMsanModuleCtorName)
 return false;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return false;
+
   MemorySanitizerVisitor Visitor(F, *this, TLI);
 
   // Clear out readonly/readnone attributes.
Index: clang/test/CodeGen/sanitize-memory-disable.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-memory-disable.c
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes 
CHECK,WITHOUT %s
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=memory | FileCheck 
-check-prefixes CHECK,MSAN %s
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck 
-check-prefixes CHECK,KMSAN %s
+
+// Instrumented function.
+// MSan uses memset(addr, -1, size) to poison allocas and stores shadow of the 
return value in
+// __msan_retval_tls. KMSAN uses __msan_poison_alloca() to poison allocas and 
calls
+// __msan_get_context_state() at function prologue to access the task context 
struct (including the
+// shadow of the return value).
+//
+// CHECK-LABEL: i32 @instrumented1
+// KMSAN: __msan_get_context_state
+// WITHOUT-NOT: __msan_poison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN: @llvm.memset{{.*}}({{.*}}, i8 -1
+// KMSAN: __msan_poison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN: __msan_retval_tls
+// CHECK: ret i32
+int instrumented1(int *a) {
+  volatile char buf[8];
+  return *a;
+}
+
+// Function with no_sanitize("memory")/no_sanitize("kernel-memory"): no shadow 
propagation, but
+// unpoisons memory to prevent false positives.
+// MSan uses memset(addr, 0, size) to unpoison locals, KMSAN uses 
__msan_unpoison_alloca(). Both
+// tools still access the retval shadow to write 0 to it.
+//
+// CHECK-LABEL: i32 @no_false_positives1
+// KMSAN: __msan_get_context_state
+// WITHOUT-NOT: __msan_unpoison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN: @llvm.memset{{.*}}({{.*}}, i8 0
+// KMSAN: __msan_unpoison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN: __msan_retval_tls
+// CHECK: ret i32
+__attribute__((no_sanitize("memory"))) 
__attribute__((no_sanitize("kernel-memory"))) int no_false_positives1(int *a) {
+  volatile char buf[8];
+  return *a;
+}
+
+// Function with disable_sanitizer_instrumentation: no instrumentation at all.
+//
+// CHECK-LABEL: i32 @no_instrumentation1
+// KMSAN-NOT: __msan_get_context_state
+// WITHOUT-NOT: __msan_poison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN-NOT: @llvm.memset{{.*}}({{.*}}, i8 0
+// KMSAN-NOT: __msan_unpoison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN-NOT: __msan_retval_tls
+// CHECK: ret i32
+__attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int 
*a) {
+  volatile char buf[8];
+  return *a;
+}
Index: clang/docs/MemorySanitizer.rst
===
--- clang/docs/MemorySanitizer.rst
+++ clang/docs/MemorySanitizer.rst
@@ -85,6 +85,15 @@
 avoid false positives.  This attribute may not be supported by other compilers,
 so we suggest to use it together with ``__has_feature(memory_sanitizer)``.
 
+``__attribute__((disable_sanitizer_instrumentation))``
+
+
+The ``disable_sanitizer_instrumentation`` attribute can be applied to functions
+to prevent all kinds of instrumentation. As a result, it may introduce false
+positives and therefore should be used with care, and only if absolutely
+required; for example for certain code that cannot tolerate any instrumentation
+and resulting side-effects. This attribute overrides ``no_sanitize("memory")``.
+
 Ignorelist
 --
 


Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -5330,6 +5330,9 @@
   if (!CompileKernel && F.getName() == kMsanModuleCtorName)
 return false;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return false;
+
   MemorySanitizerVisitor Visitor(F, *this, 

[PATCH] D108465: [msan] Hotfix clang/test/CodeGen/sanitize-memory-disable.c

[Alexander Potapenko 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 rG417a49e78e73: [msan] Hotfix 
clang/test/CodeGen/sanitize-memory-disable.c (authored by glider).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108465

Files:
  clang/test/CodeGen/sanitize-memory-disable.c


Index: clang/test/CodeGen/sanitize-memory-disable.c
===
--- clang/test/CodeGen/sanitize-memory-disable.c
+++ clang/test/CodeGen/sanitize-memory-disable.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes 
CHECK,WITHOUT %s
-// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=memory | FileCheck 
-check-prefixes CHECK,MSAN %s
-// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck 
-check-prefixes CHECK,KMSAN %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck 
-check-prefixes CHECK,WITHOUT %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s 
-fsanitize=memory | FileCheck -check-prefixes CHECK,MSAN %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s 
-fsanitize=kernel-memory | FileCheck -check-prefixes CHECK,KMSAN %s
 
 // Instrumented function.
 // MSan uses memset(addr, -1, size) to poison allocas and stores shadow of the 
return value in


Index: clang/test/CodeGen/sanitize-memory-disable.c
===
--- clang/test/CodeGen/sanitize-memory-disable.c
+++ clang/test/CodeGen/sanitize-memory-disable.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s
-// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=memory | FileCheck -check-prefixes CHECK,MSAN %s
-// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck -check-prefixes CHECK,KMSAN %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s -fsanitize=memory | FileCheck -check-prefixes CHECK,MSAN %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck -check-prefixes CHECK,KMSAN %s
 
 // Instrumented function.
 // MSan uses memset(addr, -1, size) to poison allocas and stores shadow of the return value in
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108199: [msan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: clang/docs/MemorySanitizer.rst:91
+
+The ``disable_sanitizer_instrumentation`` attribute can be applied to a certain
+function to prevent all kinds of instrumentation. This attribute overrides

melver wrote:
> Could apply a similar wording to what I suggested on 
> https://reviews.llvm.org/D108202 to deter people from using it.
There's no difference in stack traces (MSan doesn't instrument function 
entries/exits). Apart from that, done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108199

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

[PATCH] D108465: [msan] Hotfix clang/test/CodeGen/sanitize-memory-disable.c

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
glider added reviewers: eugenis, melver.
Herald added a subscriber: pengfei.
glider requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Because KMSAN is not supported on many architectures, explicitly build
the test with -target x86_64-linux-gnu.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108465

Files:
  clang/test/CodeGen/sanitize-memory-disable.c


Index: clang/test/CodeGen/sanitize-memory-disable.c
===
--- clang/test/CodeGen/sanitize-memory-disable.c
+++ clang/test/CodeGen/sanitize-memory-disable.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes 
CHECK,WITHOUT %s
-// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=memory | FileCheck 
-check-prefixes CHECK,MSAN %s
-// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck 
-check-prefixes CHECK,KMSAN %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck 
-check-prefixes CHECK,WITHOUT %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s 
-fsanitize=memory | FileCheck -check-prefixes CHECK,MSAN %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s 
-fsanitize=kernel-memory | FileCheck -check-prefixes CHECK,KMSAN %s
 
 // Instrumented function.
 // MSan uses memset(addr, -1, size) to poison allocas and stores shadow of the 
return value in


Index: clang/test/CodeGen/sanitize-memory-disable.c
===
--- clang/test/CodeGen/sanitize-memory-disable.c
+++ clang/test/CodeGen/sanitize-memory-disable.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s
-// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=memory | FileCheck -check-prefixes CHECK,MSAN %s
-// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck -check-prefixes CHECK,KMSAN %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s -fsanitize=memory | FileCheck -check-prefixes CHECK,MSAN %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck -check-prefixes CHECK,KMSAN %s
 
 // Instrumented function.
 // MSan uses memset(addr, -1, size) to poison allocas and stores shadow of the return value in
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108465: [msan] Hotfix clang/test/CodeGen/sanitize-memory-disable.c

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

TBR


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108465

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

[PATCH] D108465: [msan] Hotfix clang/test/CodeGen/sanitize-memory-disable.c

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

In D108465#2957355 , @melver wrote:

> Who reported the issue? Might be worth mentioning in commit message, 
> otherwise it appears to come out of nowhere (although it's semi-obvious given 
> x86-64 is only supported).

Right. Added the links to the bot failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108465

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

[PATCH] D108465: [msan] Hotfix clang/test/CodeGen/sanitize-memory-disable.c

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 367783.
glider added a comment.

Updated patch description with links to bot failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108465

Files:
  clang/test/CodeGen/sanitize-memory-disable.c


Index: clang/test/CodeGen/sanitize-memory-disable.c
===
--- clang/test/CodeGen/sanitize-memory-disable.c
+++ clang/test/CodeGen/sanitize-memory-disable.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes 
CHECK,WITHOUT %s
-// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=memory | FileCheck 
-check-prefixes CHECK,MSAN %s
-// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck 
-check-prefixes CHECK,KMSAN %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck 
-check-prefixes CHECK,WITHOUT %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s 
-fsanitize=memory | FileCheck -check-prefixes CHECK,MSAN %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s 
-fsanitize=kernel-memory | FileCheck -check-prefixes CHECK,KMSAN %s
 
 // Instrumented function.
 // MSan uses memset(addr, -1, size) to poison allocas and stores shadow of the 
return value in


Index: clang/test/CodeGen/sanitize-memory-disable.c
===
--- clang/test/CodeGen/sanitize-memory-disable.c
+++ clang/test/CodeGen/sanitize-memory-disable.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s
-// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=memory | FileCheck -check-prefixes CHECK,MSAN %s
-// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck -check-prefixes CHECK,KMSAN %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s -fsanitize=memory | FileCheck -check-prefixes CHECK,MSAN %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck -check-prefixes CHECK,KMSAN %s
 
 // Instrumented function.
 // MSan uses memset(addr, -1, size) to poison allocas and stores shadow of the return value in
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108202: [tsan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 368056.
glider marked 5 inline comments as done.
glider added a comment.

Addressed Marco's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108202

Files:
  clang/docs/ThreadSanitizer.rst
  clang/test/CodeGen/sanitize-thread-disable.c
  llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp


Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -562,6 +562,12 @@
   // all.
   if (F.hasFnAttribute(Attribute::Naked))
 return false;
+
+  // __attribute__(disable_sanitizer_instrumentation) prevents all kinds of
+  // instrumentation.
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return false;
+
   initialize(*F.getParent());
   SmallVector AllLoadsAndStores;
   SmallVector LocalLoadsAndStores;
Index: clang/test/CodeGen/sanitize-thread-disable.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-thread-disable.c
@@ -0,0 +1,57 @@
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck 
-check-prefixes CHECK,WITHOUT %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s 
-fsanitize=thread | FileCheck -check-prefixes CHECK,TSAN %s
+
+#include 
+
+// Instrumented function.
+// TSan inserts calls to __tsan_func_entry() and __tsan_func_exit() to 
prologue/epilogue.
+// Non-atomic loads are instrumented with __tsan_readXXX(), atomic loads - with
+// __tsan_atomicXXX_load().
+//
+// CHECK-LABEL: @instrumented1
+// TSAN: call void @__tsan_func_entry
+// WITHOUT-NOT: call void @__tsan_func_entry
+// TSAN: call void @__tsan_read4
+// WITHOUT-NOT: call void @__tsan_read4
+// TSAN: call i32 @__tsan_atomic32_load
+// WITHOUT-NOT: call i32 @__tsan_atomic32_load
+// TSAN: call void @__tsan_func_exit
+// WITHOUT-NOT: call void @__tsan_func_exit
+// CHECK: ret i32
+int instrumented1(int *a, _Atomic int *b) {
+  return *a + atomic_load(b);
+}
+
+// Function with no_sanitize("thread").
+// TSan only inserts instrumentation necessary to prevent false positives: 
calls are inserted for
+// function entry/exit and atomics, but not plain memory accesses.
+//
+// CHECK-LABEL: @no_false_positives1
+// TSAN: call void @__tsan_func_entry
+// WITHOUT-NOT: call void @__tsan_func_entry
+// TSAN-NOT: call void @__tsan_read4
+// WITHOUT-NOT: call void @__tsan_read4
+// TSAN: call i32 @__tsan_atomic32_load
+// WITHOUT-NOT: call i32 @__tsan_atomic32_load
+// TSAN: call void @__tsan_func_exit
+// WITHOUT-NOT: call void @__tsan_func_exit
+// CHECK: ret i32
+__attribute__((no_sanitize("thread"))) int no_false_positives1(int *a, _Atomic 
int *b) {
+  return *a + atomic_load(b);
+}
+
+// Function with disable_sanitizer_instrumentation: no instrumentation at all.
+//
+// CHECK-LABEL: @no_instrumentation1
+// TSAN-NOT: call void @__tsan_func_entry
+// WITHOUT-NOT: call void @__tsan_func_entry
+// TSAN-NOT: call void @__tsan_read4
+// WITHOUT-NOT: call void @__tsan_read4
+// TSAN-NOT: call i32 @__tsan_atomic32_load
+// WITHOUT-NOT: call i32 @__tsan_atomic32_load
+// TSAN-NOT: call void @__tsan_func_exit
+// WITHOUT-NOT: call void @__tsan_func_exit
+// CHECK: ret i32
+__attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int 
*a, _Atomic int *b) {
+  return *a + atomic_load(b);
+}
Index: clang/docs/ThreadSanitizer.rst
===
--- clang/docs/ThreadSanitizer.rst
+++ clang/docs/ThreadSanitizer.rst
@@ -100,6 +100,16 @@
 traces.  This attribute may not be supported by other compilers, so we suggest
 to use it together with ``__has_feature(thread_sanitizer)``.
 
+``__attribute__((disable_sanitizer_instrumentation))``
+
+
+The ``disable_sanitizer_instrumentation`` attribute can be applied to functions
+to prevent all kinds of instrumentation. As a result, it may introduce false
+positives and incorrect stack traces. Therefore, it should be used with care,
+and only if absolutely required; for example for certain code that cannot
+tolerate any instrumentation and resulting side-effects. This attribute
+overrides ``no_sanitize("thread")``.
+
 Ignorelist
 --
 


Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -562,6 +562,12 @@
   // all.
   if (F.hasFnAttribute(Attribute::Naked))
 return false;
+
+  // __attribute__(disable_sanitizer_instrumentation) prevents all kinds of
+  // instrumentation.
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+  

[PATCH] D108202: [tsan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko 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 rG8300d52e8cbf: [tsan] Add support for 
disable_sanitizer_instrumentation attribute (authored by glider).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108202

Files:
  clang/docs/ThreadSanitizer.rst
  clang/test/CodeGen/sanitize-thread-disable.c
  llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp


Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -562,6 +562,12 @@
   // all.
   if (F.hasFnAttribute(Attribute::Naked))
 return false;
+
+  // __attribute__(disable_sanitizer_instrumentation) prevents all kinds of
+  // instrumentation.
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return false;
+
   initialize(*F.getParent());
   SmallVector AllLoadsAndStores;
   SmallVector LocalLoadsAndStores;
Index: clang/test/CodeGen/sanitize-thread-disable.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-thread-disable.c
@@ -0,0 +1,57 @@
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck 
-check-prefixes CHECK,WITHOUT %s
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s 
-fsanitize=thread | FileCheck -check-prefixes CHECK,TSAN %s
+
+#include 
+
+// Instrumented function.
+// TSan inserts calls to __tsan_func_entry() and __tsan_func_exit() to 
prologue/epilogue.
+// Non-atomic loads are instrumented with __tsan_readXXX(), atomic loads - with
+// __tsan_atomicXXX_load().
+//
+// CHECK-LABEL: @instrumented1
+// TSAN: call void @__tsan_func_entry
+// WITHOUT-NOT: call void @__tsan_func_entry
+// TSAN: call void @__tsan_read4
+// WITHOUT-NOT: call void @__tsan_read4
+// TSAN: call i32 @__tsan_atomic32_load
+// WITHOUT-NOT: call i32 @__tsan_atomic32_load
+// TSAN: call void @__tsan_func_exit
+// WITHOUT-NOT: call void @__tsan_func_exit
+// CHECK: ret i32
+int instrumented1(int *a, _Atomic int *b) {
+  return *a + atomic_load(b);
+}
+
+// Function with no_sanitize("thread").
+// TSan only inserts instrumentation necessary to prevent false positives: 
calls are inserted for
+// function entry/exit and atomics, but not plain memory accesses.
+//
+// CHECK-LABEL: @no_false_positives1
+// TSAN: call void @__tsan_func_entry
+// WITHOUT-NOT: call void @__tsan_func_entry
+// TSAN-NOT: call void @__tsan_read4
+// WITHOUT-NOT: call void @__tsan_read4
+// TSAN: call i32 @__tsan_atomic32_load
+// WITHOUT-NOT: call i32 @__tsan_atomic32_load
+// TSAN: call void @__tsan_func_exit
+// WITHOUT-NOT: call void @__tsan_func_exit
+// CHECK: ret i32
+__attribute__((no_sanitize("thread"))) int no_false_positives1(int *a, _Atomic 
int *b) {
+  return *a + atomic_load(b);
+}
+
+// Function with disable_sanitizer_instrumentation: no instrumentation at all.
+//
+// CHECK-LABEL: @no_instrumentation1
+// TSAN-NOT: call void @__tsan_func_entry
+// WITHOUT-NOT: call void @__tsan_func_entry
+// TSAN-NOT: call void @__tsan_read4
+// WITHOUT-NOT: call void @__tsan_read4
+// TSAN-NOT: call i32 @__tsan_atomic32_load
+// WITHOUT-NOT: call i32 @__tsan_atomic32_load
+// TSAN-NOT: call void @__tsan_func_exit
+// WITHOUT-NOT: call void @__tsan_func_exit
+// CHECK: ret i32
+__attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int 
*a, _Atomic int *b) {
+  return *a + atomic_load(b);
+}
Index: clang/docs/ThreadSanitizer.rst
===
--- clang/docs/ThreadSanitizer.rst
+++ clang/docs/ThreadSanitizer.rst
@@ -100,6 +100,16 @@
 traces.  This attribute may not be supported by other compilers, so we suggest
 to use it together with ``__has_feature(thread_sanitizer)``.
 
+``__attribute__((disable_sanitizer_instrumentation))``
+
+
+The ``disable_sanitizer_instrumentation`` attribute can be applied to functions
+to prevent all kinds of instrumentation. As a result, it may introduce false
+positives and incorrect stack traces. Therefore, it should be used with care,
+and only if absolutely required; for example for certain code that cannot
+tolerate any instrumentation and resulting side-effects. This attribute
+overrides ``no_sanitize("thread")``.
+
 Ignorelist
 --
 


Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -562,6 +562,12 @@
   // all.
   if (F.hasFnAttribute(Attribute::Naked))
 return false;
+
+  // __attribute__(disable_sanitizer_instrumentation) 

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 366845.
glider added a comment.

Address Aaron's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-no-sanitize-coverage.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp

Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -943,6 +943,7 @@
   // Those attributes should be safe to propagate to the extracted function.
   case Attribute::AlwaysInline:
   case Attribute::Cold:
+  case Attribute::DisableSanitizerInstrumentation:
   case Attribute::Hot:
   case Attribute::NoRecurse:
   case Attribute::InlineHint:
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -628,6 +628,8 @@
 return bitc::ATTR_KIND_IN_ALLOCA;
   case Attribute::Cold:
 return bitc::ATTR_KIND_COLD;
+  case Attribute::DisableSanitizerInstrumentation:
+return bitc::ATTR_DISABLE_SANITIZER_INSTRUMENTATION;
   case Attribute::Hot:
 return bitc::ATTR_KIND_HOT;
   case Attribute::ElementType:
Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -86,6 +86,9 @@
 def DereferenceableOrNull : IntAttr<"dereferenceable_or_null",
 [ParamAttr, RetAttr]>;
 
+/// Do not instrument function with sanitizers.
+def DisableSanitizerInstrumentation: EnumAttr<"disable_sanitizer_instrumentation", [FnAttr]>;
+
 /// Provide pointer element type to intrinsic.
 def ElementType : TypeAttr<"elementtype", [ParamAttr]>;
 
Index: llvm/include/llvm/Bitcode/LLVMBitCodes.h
===
--- llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -671,6 +671,7 @@
   ATTR_KIND_SWIFT_ASYNC = 75,
   ATTR_KIND_NO_SANITIZE_COVERAGE = 76,
   ATTR_KIND_ELEMENTTYPE = 77,
+  ATTR_DISABLE_SANITIZER_INSTRUMENTATION = 78,
 };
 
 enum ComdatSelectionKindCodes {
Index: llvm/include/llvm/AsmParser/LLToken.h
===
--- llvm/include/llvm/AsmParser/LLToken.h
+++ llvm/include/llvm/AsmParser/LLToken.h
@@ -190,6 +190,7 @@
   kw_convergent,
   kw_dereferenceable,
   kw_dereferenceable_or_null,
+  kw_disable_sanitizer_instrumentation,
   kw_elementtype,
   kw_inaccessiblememonly,
   kw_inaccessiblemem_or_argmemonly,
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -56,6 +56,7 @@
 // CHECK-NEXT: DLLExport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: DLLImport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: Destructor (SubjectMatchRule_function)
+// CHECK-NEXT: DisableSanitizerInstrumentation (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: EnableIf (SubjectMatchRule_function)
 // CHECK-NEXT: EnforceTCB (SubjectMatchRule_function)
Index: clang/test/CodeGen/attr-no-sanitize-coverage.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-no-sanitize-coverage.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -debug-info-kind=limited %s -emit-llvm -o - | FileCheck %s
+
+void t1() __attribute__((disable_sanitizer_instrumentation)) {
+}
+// CHECK: disable_sanitizer_instrumentation
+// CHECK-NEXT: void @t1
+
+// CHECK-NOT: disable_sanitizer_instrumentation
+// CHECK: void @t2
+void t2() {
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2286,6 +2286,10 @@
   /// instrumented with __cyg_profile_func_* calls
   bool ShouldInstrumentFunction();
 
+  /// 

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:2945
+  let Spellings = [Clang<"disable_sanitizer_instrumentation">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [DisableSanitizerInstrumentationDocs];

aaron.ballman wrote:
> Do we want this to also appertain to `GlobalVar` and `ObjCMethod` so it's got 
> the same subjects as `no_sanitize`?
Good catch, thank you. When adding the new attribute to ASan, we probably want 
it to mimic the behavior of `no_sanitize("address")`



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:526-528
+  if (CurFuncDecl->hasAttr())
+return true;
+  return false;

aaron.ballman wrote:
> 
Right :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 366887.
glider added a comment.

Fix the docs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-no-sanitize-coverage.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp

Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -943,6 +943,7 @@
   // Those attributes should be safe to propagate to the extracted function.
   case Attribute::AlwaysInline:
   case Attribute::Cold:
+  case Attribute::DisableSanitizerInstrumentation:
   case Attribute::Hot:
   case Attribute::NoRecurse:
   case Attribute::InlineHint:
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -628,6 +628,8 @@
 return bitc::ATTR_KIND_IN_ALLOCA;
   case Attribute::Cold:
 return bitc::ATTR_KIND_COLD;
+  case Attribute::DisableSanitizerInstrumentation:
+return bitc::ATTR_DISABLE_SANITIZER_INSTRUMENTATION;
   case Attribute::Hot:
 return bitc::ATTR_KIND_HOT;
   case Attribute::ElementType:
Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -86,6 +86,9 @@
 def DereferenceableOrNull : IntAttr<"dereferenceable_or_null",
 [ParamAttr, RetAttr]>;
 
+/// Do not instrument function with sanitizers.
+def DisableSanitizerInstrumentation: EnumAttr<"disable_sanitizer_instrumentation", [FnAttr]>;
+
 /// Provide pointer element type to intrinsic.
 def ElementType : TypeAttr<"elementtype", [ParamAttr]>;
 
Index: llvm/include/llvm/Bitcode/LLVMBitCodes.h
===
--- llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -671,6 +671,7 @@
   ATTR_KIND_SWIFT_ASYNC = 75,
   ATTR_KIND_NO_SANITIZE_COVERAGE = 76,
   ATTR_KIND_ELEMENTTYPE = 77,
+  ATTR_DISABLE_SANITIZER_INSTRUMENTATION = 78,
 };
 
 enum ComdatSelectionKindCodes {
Index: llvm/include/llvm/AsmParser/LLToken.h
===
--- llvm/include/llvm/AsmParser/LLToken.h
+++ llvm/include/llvm/AsmParser/LLToken.h
@@ -190,6 +190,7 @@
   kw_convergent,
   kw_dereferenceable,
   kw_dereferenceable_or_null,
+  kw_disable_sanitizer_instrumentation,
   kw_elementtype,
   kw_inaccessiblememonly,
   kw_inaccessiblemem_or_argmemonly,
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -56,6 +56,7 @@
 // CHECK-NEXT: DLLExport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: DLLImport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: Destructor (SubjectMatchRule_function)
+// CHECK-NEXT: DisableSanitizerInstrumentation (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: EnableIf (SubjectMatchRule_function)
 // CHECK-NEXT: EnforceTCB (SubjectMatchRule_function)
Index: clang/test/CodeGen/attr-no-sanitize-coverage.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-no-sanitize-coverage.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -debug-info-kind=limited %s -emit-llvm -o - | FileCheck %s
+
+void t1() __attribute__((disable_sanitizer_instrumentation)) {
+}
+// CHECK: disable_sanitizer_instrumentation
+// CHECK-NEXT: void @t1
+
+// CHECK-NOT: disable_sanitizer_instrumentation
+// CHECK: void @t2
+void t2() {
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2286,6 +2286,10 @@
   /// instrumented with __cyg_profile_func_* calls
   bool ShouldInstrumentFunction();
 
+  /// 

[PATCH] D108199: [msan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
glider added reviewers: eugenis, melver.
Herald added a subscriber: hiraditya.
glider requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Unlike __attribute__((no_sanitize("memory"))), this one will cause MSan
to skip the entire function during instrumentation.

Depends on https://reviews.llvm.org/D108029


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108199

Files:
  clang/docs/MemorySanitizer.rst
  clang/test/CodeGen/sanitize-memory-disable.c
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -5330,6 +5330,9 @@
   if (!CompileKernel && F.getName() == kMsanModuleCtorName)
 return false;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return false;
+
   MemorySanitizerVisitor Visitor(F, *this, TLI);
 
   // Clear out readonly/readnone attributes.
Index: clang/test/CodeGen/sanitize-memory-disable.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-memory-disable.c
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=memory | FileCheck -check-prefixes CHECK,MSAN %s
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck -check-prefixes CHECK,KMSAN %s
+
+// Instrumented function.
+// MSan uses memset(addr, -1, size) to poison allocas and stores shadow of the return value in
+// __msan_retval_tls. KMSAN uses __msan_poison_alloca() to poison allocas and calls
+// __msan_get_context_state() at function prologue to access the task context struct (including the
+// shadow of the return value).
+//
+// CHECK: @instrumented1
+// KMSAN: __msan_get_context_state
+// WITHOUT-NOT: __msan_poison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN: @llvm.memset{{.*}}({{.*}}, i8 -1
+// KMSAN: __msan_poison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN: __msan_retval_tls
+// CHECK: ret i32
+int instrumented1(int *a) {
+  volatile char buf[8];
+  return *a;
+}
+
+// Function with no_sanitize("memory")/no_sanitize("kernel-memory"): no shadow propagation, but
+// unpoisons memory to prevent false positives.
+// MSan uses memset(addr, 0, size) to unpoison locals, KMSAN uses __msan_unpoison_alloca(). Both
+// tools still access the retval shadow to write 0 to it.
+//
+// CHECK: @no_false_positives1
+// KMSAN: __msan_get_context_state
+// WITHOUT-NOT: __msan_unpoison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN: @llvm.memset{{.*}}({{.*}}, i8 0
+// KMSAN: __msan_unpoison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN: __msan_retval_tls
+// CHECK: ret i32
+__attribute__((no_sanitize("memory")))
+__attribute__((no_sanitize("kernel-memory")))
+int no_false_positives1(int *a) {
+  volatile char buf[8];
+  return *a;
+}
+
+// Function with disable_sanitizer_instrumentation: no instrumentation at all.
+//
+// CHECK: @no_instrumentation1
+// KMSAN-NOT: __msan_get_context_state
+// WITHOUT-NOT: __msan_poison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN-NOT: @llvm.memset{{.*}}({{.*}}, i8 0
+// KMSAN-NOT: __msan_unpoison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN-NOT: __msan_retval_tls
+// CHECK: ret i32
+__attribute__((disable_sanitizer_instrumentation))
+int no_instrumentation1(int *a) {
+  volatile char buf[8];
+  return *a;
+}
+
Index: clang/docs/MemorySanitizer.rst
===
--- clang/docs/MemorySanitizer.rst
+++ clang/docs/MemorySanitizer.rst
@@ -85,6 +85,15 @@
 avoid false positives.  This attribute may not be supported by other compilers,
 so we suggest to use it together with ``__has_feature(memory_sanitizer)``.
 
+``__attribute__((disable_sanitizer_instrumentation))``
+
+
+The ``disable_sanitizer_instrumentation`` attribute can be applied to a certain
+function to prevent all kinds of instrumentation. This attribute overrides
+``no_sanitize("memory")`` and may introduce false positives, so it should
+be used with care, e.g. when the user wants to ensure critical code does not
+have unexpected side effects.
+
 Ignorelist
 --
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: llvm/include/llvm/IR/Attributes.td:90
+/// Do not instrument function with sanitizers.
+def DisableSanitizerInstrumentation: 
EnumAttr<"disable_sanitizer_instrumentation", [FnAttr]>;
+

melver wrote:
> There's this long-tail of changes required for adding new keywords to the IR. 
> Have a look at https://reviews.llvm.org/D102772 -- things that I see 
> currently missing are various tests etc.
It's ridiculous that we have so many handwritten files that list all the 
attributes (all those llvm.vim etc)
But I'll definitely need to update BitcodeReader and the tests. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D108202: [tsan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
glider added a reviewer: melver.
Herald added subscribers: jfb, hiraditya.
glider requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Unlike __attribute__((no_sanitize("thread"))), this one will cause TSan
to skip the entire function during instrumentation.

Depends on https://reviews.llvm.org/D108029


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108202

Files:
  clang/docs/ThreadSanitizer.rst
  clang/test/CodeGen/sanitize-thread-disable.c
  llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp


Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -563,6 +563,12 @@
   // all.
   if (F.hasFnAttribute(Attribute::Naked))
 return false;
+
+  // __attribute__(disable_sanitizer_instrumentation) prevents all kinds of
+  // instrumentation.
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return false;
+
   initialize(*F.getParent());
   SmallVector AllLoadsAndStores;
   SmallVector LocalLoadsAndStores;
Index: clang/test/CodeGen/sanitize-thread-disable.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-thread-disable.c
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes 
CHECK,WITHOUT %s
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=thread | FileCheck 
-check-prefixes CHECK,TSAN %s
+
+#include 
+
+// Instrumented function.
+// TSan inserts calls to __tsan_func_entry() and __tsan_func_exit() to 
prologue/epilogue.
+// Non-atomic loads are instrumented with __tsan_readXXX(), atomic loads - with
+// __tsan_atomicXXX_load().
+//
+// CHECK: @instrumented1
+// TSAN: call void @__tsan_func_entry
+// WITHOUT-NOT: call void @__tsan_func_entry
+// TSAN: call void @__tsan_read4
+// WITHOUT-NOT: call void @__tsan_read4
+// TSAN: call i32 @__tsan_atomic32_load
+// WITHOUT-NOT: call i32 @__tsan_atomic32_load
+// TSAN: call void @__tsan_func_exit
+// WITHOUT-NOT: call void @__tsan_func_exit
+// CHECK: ret i32
+int instrumented1(int *a, _Atomic int *b) {
+  return *a + atomic_load(b);
+}
+
+// Function with no_sanitize("thread").
+// TSan only inserts instrumentation necessary to prevent false positives: 
calls are inserted for
+// function entry/exit and atomics, but not plain memory accesses.
+//
+// CHECK: @no_false_positives1
+// TSAN: call void @__tsan_func_entry
+// WITHOUT-NOT: call void @__tsan_func_entry
+// TSAN-NOT: call void @__tsan_read4
+// WITHOUT-NOT: call void @__tsan_read4
+// TSAN: call i32 @__tsan_atomic32_load
+// WITHOUT-NOT: call i32 @__tsan_atomic32_load
+// TSAN: call void @__tsan_func_exit
+// WITHOUT-NOT: call void @__tsan_func_exit
+// CHECK: ret i32
+__attribute__((no_sanitize("thread"))) int no_false_positives1(int *a, _Atomic 
int *b) {
+  return *a + atomic_load(b);
+}
+
+// Function with disable_sanitizer_instrumentation: no instrumentation at all.
+//
+// CHECK: @no_instrumentation1
+// TSAN-NOT: call void @__tsan_func_entry
+// WITHOUT-NOT: call void @__tsan_func_entry
+// TSAN-NOT: call void @__tsan_read4
+// WITHOUT-NOT: call void @__tsan_read4
+// TSAN-NOT: call i32 @__tsan_atomic32_load
+// WITHOUT-NOT: call i32 @__tsan_atomic32_load
+// TSAN-NOT: call void @__tsan_func_exit
+// WITHOUT-NOT: call void @__tsan_func_exit
+// CHECK: ret i32
+__attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int 
*a, _Atomic int *b) {
+  return *a + atomic_load(b);
+}
Index: clang/docs/ThreadSanitizer.rst
===
--- clang/docs/ThreadSanitizer.rst
+++ clang/docs/ThreadSanitizer.rst
@@ -100,6 +100,15 @@
 traces.  This attribute may not be supported by other compilers, so we suggest
 to use it together with ``__has_feature(thread_sanitizer)``.
 
+``__attribute__((disable_sanitizer_instrumentation))``
+
+
+The ``disable_sanitizer_instrumentation`` attribute can be applied to a certain
+function to prevent all kinds of instrumentation. This attribute overrides
+``no_sanitize("thread")`` and may introduce false positives, so it should
+be used with care, e.g. when the user wants to ensure critical code does not
+have unexpected side effects.
+
 Ignorelist
 --
 


Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -563,6 +563,12 @@
   // all.
   if (F.hasFnAttribute(Attribute::Naked))
 return false;
+
+  // __attribute__(disable_sanitizer_instrumentation) prevents all kinds of
+  // instrumentation.
+  if 

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
glider added reviewers: eugenis, melver, browneee, dvyukov.
Herald added subscribers: ormris, dexonsmith, jdoerfert, steven_wu, hiraditya.
Herald added a reviewer: aaron.ballman.
glider requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

The purpose of __attribute__((no_sanitizer_instrumentation)) is to
prevent all kinds of sanitizer instrumentation applied to a certain
function.

The no_sanitize(...) attribute drops instrumentation checks, but may
still insert code preventing false positive reports. In some cases
though (e.g. when building Linux kernel with -fsanitize=kernel-memory
or -fsanitize=thread) the users may want to avoid any kind of
instrumentation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108029

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-no-sanitize-coverage.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/IR/Attributes.td

Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -175,6 +175,9 @@
 /// No SanitizeCoverage instrumentation.
 def NoSanitizeCoverage : EnumAttr<"nosanitize_coverage", [FnAttr]>;
 
+/// Do not instrument function with sanitizers.
+def NoSanitizerInstrumentation: EnumAttr<"no_sanitizer_instrumentation", [FnAttr]>;
+
 /// Null pointer in address space zero is valid.
 def NullPointerIsValid : EnumAttr<"null_pointer_is_valid", [FnAttr]>;
 
Index: llvm/include/llvm/AsmParser/LLToken.h
===
--- llvm/include/llvm/AsmParser/LLToken.h
+++ llvm/include/llvm/AsmParser/LLToken.h
@@ -219,6 +219,7 @@
   kw_nocf_check,
   kw_nounwind,
   kw_nosanitize_coverage,
+  kw_no_sanitizer_instrumentation,
   kw_null_pointer_is_valid,
   kw_optforfuzzing,
   kw_optnone,
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -102,6 +102,7 @@
 // CHECK-NEXT: NoProfileFunction (SubjectMatchRule_function)
 // CHECK-NEXT: NoSanitize (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSanitizeSpecific (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
+// CHECK-NEXT: NoSanitizerInstrumentation (SubjectMatchRule_function)
 // CHECK-NEXT: NoSpeculativeLoadHardening (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: NoSplitStack (SubjectMatchRule_function)
 // CHECK-NEXT: NoStackProtector (SubjectMatchRule_function)
Index: clang/test/CodeGen/attr-no-sanitize-coverage.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-no-sanitize-coverage.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -debug-info-kind=limited %s -emit-llvm -o - | FileCheck %s
+
+void t1() __attribute__((no_sanitizer_instrumentation)) {
+}
+// CHECK: no_sanitizer_instrumentation
+// CHECK-NEXT: void @t1
+
+// CHECK-NOT: no_sanitizer_instrumentation
+// CHECK: void @t2
+void t2() {
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2285,6 +2285,7 @@
   /// ShouldInstrumentFunction - Return true if the current function should be
   /// instrumented with __cyg_profile_func_* calls
   bool ShouldInstrumentFunction();
+  bool ShouldSkipSanitizerInstrumentation();
 
   /// ShouldXRayInstrument - Return true if the current function should be
   /// instrumented with XRay nop sleds.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -381,6 +381,9 @@
"__cyg_profile_func_exit");
   }
 
+  if (ShouldSkipSanitizerInstrumentation())
+CurFn->addFnAttr(llvm::Attribute::NoSanitizerInstrumentation);
+
   // Emit debug descriptor for function end.
   if (CGDebugInfo *DI = getDebugInfo())
 DI->EmitFunctionEnd(Builder, CurFn);
@@ -517,6 +520,14 @@
   return true;
 }
 
+bool CodeGenFunction::ShouldSkipSanitizerInstrumentation() {
+  if (!CurFuncDecl)
+return false;
+  if (CurFuncDecl->hasAttr())
+return true;
+  return false;
+}
+
 /// ShouldXRayInstrument - Return true if the current function should be
 /// instrumented with XRay nop sleds.
 bool CodeGenFunction::ShouldXRayInstrumentFunction() const {
Index: 

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:2601-2602
+
+This is not the same as ``__attribute__((no_sanitize(...)))``, which depending
+on the tool may still insert instrumentation to prevent false positive reports.
+  }];

aaron.ballman wrote:
> Has there been agreement that this isn't actually a bug? My understanding of 
> `no_sanitize` is that it disables sanitizer support for a function or global. 
> The documentation for that attribute says:
> ```
> Use the ``no_sanitize`` attribute on a function or a global variable
> declaration to specify that a particular instrumentation or set of
> instrumentations should not be applied.
> ```
> That sure reads like "do not instrument this at all" to me, and makes me 
> think we don't need a second attribute that says "no, really, I mean it this 
> time."
No, this isn't a bug, but might need to be better clarified in the docs.
ThreadSanitizer and MemorySanitizer do insert some instrumentation into ignore 
functions to prevent false positives:

> ThreadSanitizer still instruments such functions to avoid false positives and 
> provide meaningful stack traces.

(https://clang.llvm.org/docs/ThreadSanitizer.html#attribute-no-sanitize-thread)

and 

> MemorySanitizer may still instrument such functions to avoid false positives.

(https://clang.llvm.org/docs/MemorySanitizer.html#attribute-no-sanitize-memory)

This is the behavior people rely onto, although at this point I don't think 
`no_sanitize(...)` is the best name for attribute not disabling instrumentation 
completely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 366256.
glider added a comment.

Added a comment to clang/lib/CodeGen/CodeGenFunction.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-no-sanitize-coverage.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/IR/Attributes.td

Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -175,6 +175,9 @@
 /// No SanitizeCoverage instrumentation.
 def NoSanitizeCoverage : EnumAttr<"nosanitize_coverage", [FnAttr]>;
 
+/// Do not instrument function with sanitizers.
+def NoSanitizerInstrumentation: EnumAttr<"no_sanitizer_instrumentation", [FnAttr]>;
+
 /// Null pointer in address space zero is valid.
 def NullPointerIsValid : EnumAttr<"null_pointer_is_valid", [FnAttr]>;
 
Index: llvm/include/llvm/AsmParser/LLToken.h
===
--- llvm/include/llvm/AsmParser/LLToken.h
+++ llvm/include/llvm/AsmParser/LLToken.h
@@ -219,6 +219,7 @@
   kw_nocf_check,
   kw_nounwind,
   kw_nosanitize_coverage,
+  kw_no_sanitizer_instrumentation,
   kw_null_pointer_is_valid,
   kw_optforfuzzing,
   kw_optnone,
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -102,6 +102,7 @@
 // CHECK-NEXT: NoProfileFunction (SubjectMatchRule_function)
 // CHECK-NEXT: NoSanitize (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSanitizeSpecific (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
+// CHECK-NEXT: NoSanitizerInstrumentation (SubjectMatchRule_function)
 // CHECK-NEXT: NoSpeculativeLoadHardening (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: NoSplitStack (SubjectMatchRule_function)
 // CHECK-NEXT: NoStackProtector (SubjectMatchRule_function)
Index: clang/test/CodeGen/attr-no-sanitize-coverage.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-no-sanitize-coverage.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -debug-info-kind=limited %s -emit-llvm -o - | FileCheck %s
+
+void t1() __attribute__((no_sanitizer_instrumentation)) {
+}
+// CHECK: no_sanitizer_instrumentation
+// CHECK-NEXT: void @t1
+
+// CHECK-NOT: no_sanitizer_instrumentation
+// CHECK: void @t2
+void t2() {
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2286,6 +2286,10 @@
   /// instrumented with __cyg_profile_func_* calls
   bool ShouldInstrumentFunction();
 
+  /// ShouldSkipSanitizerInstrumentation - Return true if the current function
+  /// should not be instrumented with sanitizers.
+  bool ShouldSkipSanitizerInstrumentation();
+
   /// ShouldXRayInstrument - Return true if the current function should be
   /// instrumented with XRay nop sleds.
   bool ShouldXRayInstrumentFunction() const;
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -381,6 +381,9 @@
"__cyg_profile_func_exit");
   }
 
+  if (ShouldSkipSanitizerInstrumentation())
+CurFn->addFnAttr(llvm::Attribute::NoSanitizerInstrumentation);
+
   // Emit debug descriptor for function end.
   if (CGDebugInfo *DI = getDebugInfo())
 DI->EmitFunctionEnd(Builder, CurFn);
@@ -517,6 +520,14 @@
   return true;
 }
 
+bool CodeGenFunction::ShouldSkipSanitizerInstrumentation() {
+  if (!CurFuncDecl)
+return false;
+  if (CurFuncDecl->hasAttr())
+return true;
+  return false;
+}
+
 /// ShouldXRayInstrument - Return true if the current function should be
 /// instrumented with XRay nop sleds.
 bool CodeGenFunction::ShouldXRayInstrumentFunction() const {
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -2592,6 +2592,17 @@
   }];
 }
 
+def NoSanitizerInstrumentationDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+Use the ``no_sanitizer_instrumentation`` attribute on a function to specify
+that no sanitizer instrumentation should be applied.
+
+This is not the same as 

[PATCH] D108555: [tsan] Make sanitize-thread-disable.c an X86-only test

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Not really sure what's the best solution here, but I think restricting the test 
to x86 should help.
So far only ARM and PPC bots reported failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108555

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

[PATCH] D108555: [tsan] Make sanitize-thread-disable.c an X86-only test

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: clang/test/CodeGen/X86/sanitize-thread-disable.c:22
 int instrumented1(int *a, _Atomic int *b) {
   return *a + atomic_load(b);
 }

melver wrote:
> melver wrote:
> > I think you do not need to use atomic_load.
> > 
> > You can just deref b, and because it's _Atomic type it *should* just use an 
> > atomic seq_cst load implicitly.
> Alternatively, there are the `__atomic` builtins. You could just use 
> `__atomic_load_n()` instead (it works with either _Atomic or non-_Atomic 
> type).
You are right, turns out I didn't need the header at all. Removed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108555

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

[PATCH] D108555: [tsan] Make sanitize-thread-disable.c an X86-only test

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 368096.
glider added a comment.

Removed the header


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108555

Files:
  clang/test/CodeGen/sanitize-thread-disable.c


Index: clang/test/CodeGen/sanitize-thread-disable.c
===
--- clang/test/CodeGen/sanitize-thread-disable.c
+++ clang/test/CodeGen/sanitize-thread-disable.c
@@ -1,8 +1,6 @@
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck 
-check-prefixes CHECK,WITHOUT %s
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s 
-fsanitize=thread | FileCheck -check-prefixes CHECK,TSAN %s
 
-#include 
-
 // Instrumented function.
 // TSan inserts calls to __tsan_func_entry() and __tsan_func_exit() to 
prologue/epilogue.
 // Non-atomic loads are instrumented with __tsan_readXXX(), atomic loads - with
@@ -19,7 +17,7 @@
 // WITHOUT-NOT: call void @__tsan_func_exit
 // CHECK: ret i32
 int instrumented1(int *a, _Atomic int *b) {
-  return *a + atomic_load(b);
+  return *a + *b;
 }
 
 // Function with no_sanitize("thread").
@@ -37,7 +35,7 @@
 // WITHOUT-NOT: call void @__tsan_func_exit
 // CHECK: ret i32
 __attribute__((no_sanitize("thread"))) int no_false_positives1(int *a, _Atomic 
int *b) {
-  return *a + atomic_load(b);
+  return *a + *b;
 }
 
 // Function with disable_sanitizer_instrumentation: no instrumentation at all.
@@ -53,5 +51,5 @@
 // WITHOUT-NOT: call void @__tsan_func_exit
 // CHECK: ret i32
 __attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int 
*a, _Atomic int *b) {
-  return *a + atomic_load(b);
+  return *a + *b;
 }


Index: clang/test/CodeGen/sanitize-thread-disable.c
===
--- clang/test/CodeGen/sanitize-thread-disable.c
+++ clang/test/CodeGen/sanitize-thread-disable.c
@@ -1,8 +1,6 @@
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s -fsanitize=thread | FileCheck -check-prefixes CHECK,TSAN %s
 
-#include 
-
 // Instrumented function.
 // TSan inserts calls to __tsan_func_entry() and __tsan_func_exit() to prologue/epilogue.
 // Non-atomic loads are instrumented with __tsan_readXXX(), atomic loads - with
@@ -19,7 +17,7 @@
 // WITHOUT-NOT: call void @__tsan_func_exit
 // CHECK: ret i32
 int instrumented1(int *a, _Atomic int *b) {
-  return *a + atomic_load(b);
+  return *a + *b;
 }
 
 // Function with no_sanitize("thread").
@@ -37,7 +35,7 @@
 // WITHOUT-NOT: call void @__tsan_func_exit
 // CHECK: ret i32
 __attribute__((no_sanitize("thread"))) int no_false_positives1(int *a, _Atomic int *b) {
-  return *a + atomic_load(b);
+  return *a + *b;
 }
 
 // Function with disable_sanitizer_instrumentation: no instrumentation at all.
@@ -53,5 +51,5 @@
 // WITHOUT-NOT: call void @__tsan_func_exit
 // CHECK: ret i32
 __attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int *a, _Atomic int *b) {
-  return *a + atomic_load(b);
+  return *a + *b;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108555: [tsan] Make sanitize-thread-disable.c an X86-only test

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

In D108555#2960034 , @melver wrote:

> LGTM, thanks!
>
> Patch title ("...an X86-only test..") also needs adjustment.

It's strange that Phab doesn't automatically update the title when I update the 
commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108555

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

[PATCH] D108555: [tsan] Make sanitize-thread-disable.c an X86-only test

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
glider added a reviewer: melver.
Herald added subscribers: pengfei, jfb, kristof.beyls.
glider requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Looks like non-x86 bots are unhappy with inclusion of 
e.g.:

clang-armv7-vfpv3-2stage - 
https://lab.llvm.org/buildbot/#/builders/182/builds/626
clang-ppc64le-linux - https://lab.llvm.org/buildbot/#/builders/76/builds/3619
llvm-clang-win-x-armv7l - 
https://lab.llvm.org/buildbot/#/builders/60/builds/4514

Move the test to CodeGen/X86 to fix the builds.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108555

Files:
  clang/test/CodeGen/X86/sanitize-thread-disable.c
  clang/test/CodeGen/sanitize-thread-disable.c


Index: clang/test/CodeGen/sanitize-thread-disable.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-thread-disable.c
@@ -1,57 +0,0 @@
-// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck 
-check-prefixes CHECK,WITHOUT %s
-// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s 
-fsanitize=thread | FileCheck -check-prefixes CHECK,TSAN %s
-
-#include 
-
-// Instrumented function.
-// TSan inserts calls to __tsan_func_entry() and __tsan_func_exit() to 
prologue/epilogue.
-// Non-atomic loads are instrumented with __tsan_readXXX(), atomic loads - with
-// __tsan_atomicXXX_load().
-//
-// CHECK-LABEL: @instrumented1
-// TSAN: call void @__tsan_func_entry
-// WITHOUT-NOT: call void @__tsan_func_entry
-// TSAN: call void @__tsan_read4
-// WITHOUT-NOT: call void @__tsan_read4
-// TSAN: call i32 @__tsan_atomic32_load
-// WITHOUT-NOT: call i32 @__tsan_atomic32_load
-// TSAN: call void @__tsan_func_exit
-// WITHOUT-NOT: call void @__tsan_func_exit
-// CHECK: ret i32
-int instrumented1(int *a, _Atomic int *b) {
-  return *a + atomic_load(b);
-}
-
-// Function with no_sanitize("thread").
-// TSan only inserts instrumentation necessary to prevent false positives: 
calls are inserted for
-// function entry/exit and atomics, but not plain memory accesses.
-//
-// CHECK-LABEL: @no_false_positives1
-// TSAN: call void @__tsan_func_entry
-// WITHOUT-NOT: call void @__tsan_func_entry
-// TSAN-NOT: call void @__tsan_read4
-// WITHOUT-NOT: call void @__tsan_read4
-// TSAN: call i32 @__tsan_atomic32_load
-// WITHOUT-NOT: call i32 @__tsan_atomic32_load
-// TSAN: call void @__tsan_func_exit
-// WITHOUT-NOT: call void @__tsan_func_exit
-// CHECK: ret i32
-__attribute__((no_sanitize("thread"))) int no_false_positives1(int *a, _Atomic 
int *b) {
-  return *a + atomic_load(b);
-}
-
-// Function with disable_sanitizer_instrumentation: no instrumentation at all.
-//
-// CHECK-LABEL: @no_instrumentation1
-// TSAN-NOT: call void @__tsan_func_entry
-// WITHOUT-NOT: call void @__tsan_func_entry
-// TSAN-NOT: call void @__tsan_read4
-// WITHOUT-NOT: call void @__tsan_read4
-// TSAN-NOT: call i32 @__tsan_atomic32_load
-// WITHOUT-NOT: call i32 @__tsan_atomic32_load
-// TSAN-NOT: call void @__tsan_func_exit
-// WITHOUT-NOT: call void @__tsan_func_exit
-// CHECK: ret i32
-__attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int 
*a, _Atomic int *b) {
-  return *a + atomic_load(b);
-}


Index: clang/test/CodeGen/sanitize-thread-disable.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-thread-disable.c
@@ -1,57 +0,0 @@
-// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s
-// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s -fsanitize=thread | FileCheck -check-prefixes CHECK,TSAN %s
-
-#include 
-
-// Instrumented function.
-// TSan inserts calls to __tsan_func_entry() and __tsan_func_exit() to prologue/epilogue.
-// Non-atomic loads are instrumented with __tsan_readXXX(), atomic loads - with
-// __tsan_atomicXXX_load().
-//
-// CHECK-LABEL: @instrumented1
-// TSAN: call void @__tsan_func_entry
-// WITHOUT-NOT: call void @__tsan_func_entry
-// TSAN: call void @__tsan_read4
-// WITHOUT-NOT: call void @__tsan_read4
-// TSAN: call i32 @__tsan_atomic32_load
-// WITHOUT-NOT: call i32 @__tsan_atomic32_load
-// TSAN: call void @__tsan_func_exit
-// WITHOUT-NOT: call void @__tsan_func_exit
-// CHECK: ret i32
-int instrumented1(int *a, _Atomic int *b) {
-  return *a + atomic_load(b);
-}
-
-// Function with no_sanitize("thread").
-// TSan only inserts instrumentation necessary to prevent false positives: calls are inserted for
-// function entry/exit and atomics, but not plain memory accesses.
-//
-// CHECK-LABEL: @no_false_positives1
-// TSAN: call void @__tsan_func_entry
-// WITHOUT-NOT: call void @__tsan_func_entry
-// TSAN-NOT: call void @__tsan_read4
-// WITHOUT-NOT: call void @__tsan_read4
-// TSAN: call i32 @__tsan_atomic32_load
-// WITHOUT-NOT: call i32 @__tsan_atomic32_load
-// TSAN: call void @__tsan_func_exit
-// WITHOUT-NOT: call void 

[PATCH] D108555: [tsan] Do not include from sanitize-thread-disable.c

[Alexander Potapenko 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 rGcdb391698bb2: [tsan] Do not include stdatomic.h from 
sanitize-thread-disable.c (authored by glider).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108555

Files:
  clang/test/CodeGen/sanitize-thread-disable.c


Index: clang/test/CodeGen/sanitize-thread-disable.c
===
--- clang/test/CodeGen/sanitize-thread-disable.c
+++ clang/test/CodeGen/sanitize-thread-disable.c
@@ -1,8 +1,6 @@
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck 
-check-prefixes CHECK,WITHOUT %s
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s 
-fsanitize=thread | FileCheck -check-prefixes CHECK,TSAN %s
 
-#include 
-
 // Instrumented function.
 // TSan inserts calls to __tsan_func_entry() and __tsan_func_exit() to 
prologue/epilogue.
 // Non-atomic loads are instrumented with __tsan_readXXX(), atomic loads - with
@@ -19,7 +17,7 @@
 // WITHOUT-NOT: call void @__tsan_func_exit
 // CHECK: ret i32
 int instrumented1(int *a, _Atomic int *b) {
-  return *a + atomic_load(b);
+  return *a + *b;
 }
 
 // Function with no_sanitize("thread").
@@ -37,7 +35,7 @@
 // WITHOUT-NOT: call void @__tsan_func_exit
 // CHECK: ret i32
 __attribute__((no_sanitize("thread"))) int no_false_positives1(int *a, _Atomic 
int *b) {
-  return *a + atomic_load(b);
+  return *a + *b;
 }
 
 // Function with disable_sanitizer_instrumentation: no instrumentation at all.
@@ -53,5 +51,5 @@
 // WITHOUT-NOT: call void @__tsan_func_exit
 // CHECK: ret i32
 __attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int 
*a, _Atomic int *b) {
-  return *a + atomic_load(b);
+  return *a + *b;
 }


Index: clang/test/CodeGen/sanitize-thread-disable.c
===
--- clang/test/CodeGen/sanitize-thread-disable.c
+++ clang/test/CodeGen/sanitize-thread-disable.c
@@ -1,8 +1,6 @@
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s -fsanitize=thread | FileCheck -check-prefixes CHECK,TSAN %s
 
-#include 
-
 // Instrumented function.
 // TSan inserts calls to __tsan_func_entry() and __tsan_func_exit() to prologue/epilogue.
 // Non-atomic loads are instrumented with __tsan_readXXX(), atomic loads - with
@@ -19,7 +17,7 @@
 // WITHOUT-NOT: call void @__tsan_func_exit
 // CHECK: ret i32
 int instrumented1(int *a, _Atomic int *b) {
-  return *a + atomic_load(b);
+  return *a + *b;
 }
 
 // Function with no_sanitize("thread").
@@ -37,7 +35,7 @@
 // WITHOUT-NOT: call void @__tsan_func_exit
 // CHECK: ret i32
 __attribute__((no_sanitize("thread"))) int no_false_positives1(int *a, _Atomic int *b) {
-  return *a + atomic_load(b);
+  return *a + *b;
 }
 
 // Function with disable_sanitizer_instrumentation: no instrumentation at all.
@@ -53,5 +51,5 @@
 // WITHOUT-NOT: call void @__tsan_func_exit
 // CHECK: ret i32
 __attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int *a, _Atomic int *b) {
-  return *a + atomic_load(b);
+  return *a + *b;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider marked 2 inline comments as done.
glider added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:2601-2602
+
+This is not the same as ``__attribute__((no_sanitize(...)))``, which depending
+on the tool may still insert instrumentation to prevent false positive reports.
+  }];

aaron.ballman wrote:
> melver wrote:
> > aaron.ballman wrote:
> > > glider wrote:
> > > > aaron.ballman wrote:
> > > > > Has there been agreement that this isn't actually a bug? My 
> > > > > understanding of `no_sanitize` is that it disables sanitizer support 
> > > > > for a function or global. The documentation for that attribute says:
> > > > > ```
> > > > > Use the ``no_sanitize`` attribute on a function or a global variable
> > > > > declaration to specify that a particular instrumentation or set of
> > > > > instrumentations should not be applied.
> > > > > ```
> > > > > That sure reads like "do not instrument this at all" to me, and makes 
> > > > > me think we don't need a second attribute that says "no, really, I 
> > > > > mean it this time."
> > > > No, this isn't a bug, but might need to be better clarified in the docs.
> > > > ThreadSanitizer and MemorySanitizer do insert some instrumentation into 
> > > > ignore functions to prevent false positives:
> > > > 
> > > > > ThreadSanitizer still instruments such functions to avoid false 
> > > > > positives and provide meaningful stack traces.
> > > > 
> > > > (https://clang.llvm.org/docs/ThreadSanitizer.html#attribute-no-sanitize-thread)
> > > > 
> > > > and 
> > > > 
> > > > > MemorySanitizer may still instrument such functions to avoid false 
> > > > > positives.
> > > > 
> > > > (https://clang.llvm.org/docs/MemorySanitizer.html#attribute-no-sanitize-memory)
> > > > 
> > > > This is the behavior people rely onto, although at this point I don't 
> > > > think `no_sanitize(...)` is the best name for attribute not disabling 
> > > > instrumentation completely.
> > > Thank you for the information!
> > > 
> > > Having two attributes with basically the same name to perform this 
> > > functionality is confusing because users (understandably) will reach for 
> > > the succinctly named one and make assumptions about what it does from the 
> > > name.
> > > 
> > > One possibility would be to change `no_sanitize` to take an additional 
> > > argument, as in: `__attribute__((no_sanitize(fully_disable, "thread")))`. 
> > > Perhaps another solution would be to give the proposed attribute a more 
> > > distinct name, like `disable_sanitizer_instrumentation`, 
> > > `sanitizer_instrumentation_disabled`, or something else.
> > Last I looked at `no_sanitize`, it's quite awkward that it is an attribute 
> > that accepts arguments, because it makes it very hard to query for 
> > existence of attribute additions/changes with `__has_attribute()`. Given 
> > this new attribute is meant to be semantically quite different, the cleaner 
> > and less intrusive way with that in mind is to create a new attribute. 
> > Unless of course there's a nice way to make `__has_attribute()` work.
> > 
> > Here's another suggestion for name, which actually makes the difference 
> > between `no_sanitize` and the new one obvious: 
> > `no_sanitize_any_permit_false_positives`
> > 
> > At least it would semantically tell a user what might happen, which in turn 
> > would hopefully make them avoid this attribute (also because it's hard 
> > enough to type) unless they are absolutely sure.
> > Given this new attribute is meant to be semantically quite different, the 
> > cleaner and less intrusive way with that in mind is to create a new 
> > attribute. Unless of course there's a nice way to make __has_attribute() 
> > work.
> 
> That sounds like good rationale for a separate attribute.
> 
> > Here's another suggestion for name, which actually makes the difference 
> > between no_sanitize and the new one obvious: 
> > no_sanitize_any_permit_false_positives
> >
> > At least it would semantically tell a user what might happen, which in turn 
> > would hopefully make them avoid this attribute (also because it's hard 
> > enough to type) unless they are absolutely sure.
> 
> That would certainly solve my concerns! Do you envision this being used far 
> less often than `no_sanitize`? (That's my intuition, so I'm just 
> double-checking that this isn't expected to be a popular replacement or 
> something where the long name may be really undesirable.)
Yes, right now we only want to apply this attribute to a couple of critical 
places in the Linux kernel.
Other users may pop up in e.g. other kernels (NetBSD is using KMSAN) and maybe 
userspace programs, but we don't expect it to be popular, so having a lengthy 
attribute name is fine.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:524
+bool CodeGenFunction::ShouldSkipSanitizerInstrumentation() {
+  if (!CurFuncDecl)
+return false;

dvyukov wrote:
> When is CurFuncDecl 

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 366591.
glider added a comment.

Updated patch description


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-no-sanitize-coverage.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp

Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -943,6 +943,7 @@
   // Those attributes should be safe to propagate to the extracted function.
   case Attribute::AlwaysInline:
   case Attribute::Cold:
+  case Attribute::DisableSanitizerInstrumentation:
   case Attribute::Hot:
   case Attribute::NoRecurse:
   case Attribute::InlineHint:
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -628,6 +628,8 @@
 return bitc::ATTR_KIND_IN_ALLOCA;
   case Attribute::Cold:
 return bitc::ATTR_KIND_COLD;
+  case Attribute::DisableSanitizerInstrumentation:
+return bitc::ATTR_DISABLE_SANITIZER_INSTRUMENTATION;
   case Attribute::Hot:
 return bitc::ATTR_KIND_HOT;
   case Attribute::ElementType:
Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -86,6 +86,9 @@
 def DereferenceableOrNull : IntAttr<"dereferenceable_or_null",
 [ParamAttr, RetAttr]>;
 
+/// Do not instrument function with sanitizers.
+def DisableSanitizerInstrumentation: EnumAttr<"disable_sanitizer_instrumentation", [FnAttr]>;
+
 /// Provide pointer element type to intrinsic.
 def ElementType : TypeAttr<"elementtype", [ParamAttr]>;
 
Index: llvm/include/llvm/Bitcode/LLVMBitCodes.h
===
--- llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -671,6 +671,7 @@
   ATTR_KIND_SWIFT_ASYNC = 75,
   ATTR_KIND_NO_SANITIZE_COVERAGE = 76,
   ATTR_KIND_ELEMENTTYPE = 77,
+  ATTR_DISABLE_SANITIZER_INSTRUMENTATION = 78,
 };
 
 enum ComdatSelectionKindCodes {
Index: llvm/include/llvm/AsmParser/LLToken.h
===
--- llvm/include/llvm/AsmParser/LLToken.h
+++ llvm/include/llvm/AsmParser/LLToken.h
@@ -190,6 +190,7 @@
   kw_convergent,
   kw_dereferenceable,
   kw_dereferenceable_or_null,
+  kw_disable_sanitizer_instrumentation,
   kw_elementtype,
   kw_inaccessiblememonly,
   kw_inaccessiblemem_or_argmemonly,
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -56,6 +56,7 @@
 // CHECK-NEXT: DLLExport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: DLLImport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: Destructor (SubjectMatchRule_function)
+// CHECK-NEXT: DisableSanitizerInstrumentation (SubjectMatchRule_function)
 // CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: EnableIf (SubjectMatchRule_function)
 // CHECK-NEXT: EnforceTCB (SubjectMatchRule_function)
Index: clang/test/CodeGen/attr-no-sanitize-coverage.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-no-sanitize-coverage.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -debug-info-kind=limited %s -emit-llvm -o - | FileCheck %s
+
+void t1() __attribute__((disable_sanitizer_instrumentation)) {
+}
+// CHECK: disable_sanitizer_instrumentation
+// CHECK-NEXT: void @t1
+
+// CHECK-NOT: disable_sanitizer_instrumentation
+// CHECK: void @t2
+void t2() {
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2286,6 +2286,10 @@
   /// instrumented with __cyg_profile_func_* calls
   bool ShouldInstrumentFunction();
 
+  /// ShouldSkipSanitizerInstrumentation - Return true if the current function
+  

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a subscriber: krytarowski.
glider marked 2 inline comments as done.
glider added a comment.

+krytarowski FYI


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 366570.
glider added a comment.

Renamed no_sanitizer_instrumentation to disable_sanitizer_instrumentation
Added bitcode attribute declaration and missing handling of the new attribute 
in switches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-no-sanitize-coverage.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp

Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -943,6 +943,7 @@
   // Those attributes should be safe to propagate to the extracted function.
   case Attribute::AlwaysInline:
   case Attribute::Cold:
+  case Attribute::DisableSanitizerInstrumentation:
   case Attribute::Hot:
   case Attribute::NoRecurse:
   case Attribute::InlineHint:
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -628,6 +628,8 @@
 return bitc::ATTR_KIND_IN_ALLOCA;
   case Attribute::Cold:
 return bitc::ATTR_KIND_COLD;
+  case Attribute::DisableSanitizerInstrumentation:
+return bitc::ATTR_DISABLE_SANITIZER_INSTRUMENTATION;
   case Attribute::Hot:
 return bitc::ATTR_KIND_HOT;
   case Attribute::ElementType:
Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -86,6 +86,9 @@
 def DereferenceableOrNull : IntAttr<"dereferenceable_or_null",
 [ParamAttr, RetAttr]>;
 
+/// Do not instrument function with sanitizers.
+def DisableSanitizerInstrumentation: EnumAttr<"disable_sanitizer_instrumentation", [FnAttr]>;
+
 /// Provide pointer element type to intrinsic.
 def ElementType : TypeAttr<"elementtype", [ParamAttr]>;
 
Index: llvm/include/llvm/Bitcode/LLVMBitCodes.h
===
--- llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -671,6 +671,7 @@
   ATTR_KIND_SWIFT_ASYNC = 75,
   ATTR_KIND_NO_SANITIZE_COVERAGE = 76,
   ATTR_KIND_ELEMENTTYPE = 77,
+  ATTR_DISABLE_SANITIZER_INSTRUMENTATION = 78,
 };
 
 enum ComdatSelectionKindCodes {
Index: llvm/include/llvm/AsmParser/LLToken.h
===
--- llvm/include/llvm/AsmParser/LLToken.h
+++ llvm/include/llvm/AsmParser/LLToken.h
@@ -190,6 +190,7 @@
   kw_convergent,
   kw_dereferenceable,
   kw_dereferenceable_or_null,
+  kw_disable_sanitizer_instrumentation,
   kw_elementtype,
   kw_inaccessiblememonly,
   kw_inaccessiblemem_or_argmemonly,
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -56,6 +56,7 @@
 // CHECK-NEXT: DLLExport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: DLLImport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: Destructor (SubjectMatchRule_function)
+// CHECK-NEXT: DisableSanitizerInstrumentation (SubjectMatchRule_function)
 // CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: EnableIf (SubjectMatchRule_function)
 // CHECK-NEXT: EnforceTCB (SubjectMatchRule_function)
Index: clang/test/CodeGen/attr-no-sanitize-coverage.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-no-sanitize-coverage.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -debug-info-kind=limited %s -emit-llvm -o - | FileCheck %s
+
+void t1() __attribute__((disable_sanitizer_instrumentation)) {
+}
+// CHECK: disable_sanitizer_instrumentation
+// CHECK-NEXT: void @t1
+
+// CHECK-NOT: disable_sanitizer_instrumentation
+// CHECK: void @t2
+void t2() {
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2286,6 +2286,10 @@
   /// instrumented with 

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: llvm/include/llvm/Bitcode/LLVMBitCodes.h:674
   ATTR_KIND_ELEMENTTYPE = 77,
+  ATTR_DISABLE_SANITIZER_INSTRUMENTATION = 78,
 };

Missing "_KIND_"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 367480.
glider added a comment.

Updated BitcodeReader.cpp and several tests per Marco's suggestion
Renamed ATTR_DISABLE_SANITIZER_INSTRUMENTATION to 
ATTR_KIND_DISABLE_SANITIZER_INSTRUMENTATION


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-disable-sanitizer-instrumentation.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/docs/BitCodeFormat.rst
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Bitcode/attributes.ll
  llvm/test/Bitcode/compatibility.ll

Index: llvm/test/Bitcode/compatibility.ll
===
--- llvm/test/Bitcode/compatibility.ll
+++ llvm/test/Bitcode/compatibility.ll
@@ -1510,7 +1510,7 @@
   ; CHECK: select <2 x i1> , <2 x i8> , <2 x i8> 
 
   call void @f.nobuiltin() builtin
-  ; CHECK: call void @f.nobuiltin() #45
+  ; CHECK: call void @f.nobuiltin() #46
 
   call fastcc noalias i32* @f.noalias() noinline
   ; CHECK: call fastcc noalias i32* @f.noalias() #12
@@ -1907,6 +1907,9 @@
 declare void @f.nosanitize_coverage() nosanitize_coverage
 ; CHECK: declare void @f.nosanitize_coverage() #44
 
+declare void @f.disable_sanitizer_instrumentation() disable_sanitizer_instrumentation
+; CHECK: declare void @f.disable_sanitizer_instrumentation() #45
+
 ; immarg attribute
 declare void @llvm.test.immarg.intrinsic(i32 immarg)
 ; CHECK: declare void @llvm.test.immarg.intrinsic(i32 immarg)
@@ -1965,7 +1968,8 @@
 ; CHECK: attributes #42 = { speculatable }
 ; CHECK: attributes #43 = { strictfp }
 ; CHECK: attributes #44 = { nosanitize_coverage }
-; CHECK: attributes #45 = { builtin }
+; CHECK: attributes #45 = { disable_sanitizer_instrumentation }
+; CHECK: attributes #46 = { builtin }
 
 ;; Metadata
 
Index: llvm/test/Bitcode/attributes.ll
===
--- llvm/test/Bitcode/attributes.ll
+++ llvm/test/Bitcode/attributes.ll
@@ -472,6 +472,12 @@
   ret void
 }
 
+; CHECK: define void @f80() #50
+define void @f80() disable_sanitizer_instrumentation
+{
+ret void;
+}
+
 ; CHECK: attributes #0 = { noreturn }
 ; CHECK: attributes #1 = { nounwind }
 ; CHECK: attributes #2 = { readnone }
@@ -522,4 +528,5 @@
 ; CHECK: attributes #47 = { vscale_range(1,0) }
 ; CHECK: attributes #48 = { nosanitize_coverage }
 ; CHECK: attributes #49 = { noprofile }
+; CHECK: attributes #50 = { disable_sanitizer_instrumentation }
 ; CHECK: attributes #[[NOBUILTIN]] = { nobuiltin }
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -943,6 +943,7 @@
   // Those attributes should be safe to propagate to the extracted function.
   case Attribute::AlwaysInline:
   case Attribute::Cold:
+  case Attribute::DisableSanitizerInstrumentation:
   case Attribute::Hot:
   case Attribute::NoRecurse:
   case Attribute::InlineHint:
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -627,6 +627,8 @@
 return bitc::ATTR_KIND_IN_ALLOCA;
   case Attribute::Cold:
 return bitc::ATTR_KIND_COLD;
+  case Attribute::DisableSanitizerInstrumentation:
+return bitc::ATTR_KIND_DISABLE_SANITIZER_INSTRUMENTATION;
   case Attribute::Hot:
 return bitc::ATTR_KIND_HOT;
   case Attribute::ElementType:
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1388,6 +1388,8 @@
 return Attribute::Cold;
   case bitc::ATTR_KIND_CONVERGENT:
 return Attribute::Convergent;
+  case bitc::ATTR_KIND_DISABLE_SANITIZER_INSTRUMENTATION:
+return Attribute::DisableSanitizerInstrumentation;
   case bitc::ATTR_KIND_ELEMENTTYPE:
 return Attribute::ElementType;
   case bitc::ATTR_KIND_INACCESSIBLEMEM_ONLY:
Index: llvm/lib/AsmParser/LLLexer.cpp
===
--- llvm/lib/AsmParser/LLLexer.cpp
+++ llvm/lib/AsmParser/LLLexer.cpp
@@ -643,6 +643,7 @@
   KEYWORD(convergent);
   KEYWORD(dereferenceable);
   KEYWORD(dereferenceable_or_null);
+  KEYWORD(disable_sanitizer_instrumentation);
   KEYWORD(elementtype);

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

In D108029#2954566 , @melver wrote:

> llvm/docs/LangRef.rst also needs a corresponding change.

Right, will do. I thought LangRef was missing the sanitizer bits as well, and 
was planning to add them together, but apparently I was just looking for the 
wrong attributes.




Comment at: llvm/lib/AsmParser/LLLexer.cpp:646
   KEYWORD(dereferenceable_or_null);
+  KEYWORD(disable_sanitizer_instrumentation);
   KEYWORD(elementtype);

melver wrote:
> Do the tests pass?
> 
> There should also be the code that actually turns the token into the 
> attribute in llvm/lib/AsmParser/LLParser.cpp
check-llvm and check-clang pass for me, check-all seems to choke on some 
unrelated bugs.

LLParser is using Tablegen now, no need to change it: 
https://github.com/llvm/llvm-project/blob/main/llvm/lib/AsmParser/LLParser.cpp#L1247


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D114421: [asan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
glider added reviewers: melver, vitalybuka.
glider requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For ASan this will effectively serve as a synonym for
__attribute__((no_sanitize("address")))


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114421

Files:
  clang/docs/AddressSanitizer.rst
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/test/CodeGen/address-safety-attr-flavors.cpp
  clang/test/CodeGen/asan-globals.cpp

Index: clang/test/CodeGen/asan-globals.cpp
===
--- clang/test/CodeGen/asan-globals.cpp
+++ clang/test/CodeGen/asan-globals.cpp
@@ -10,6 +10,7 @@
 int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
+int __attribute__((disable_sanitizer_instrumentation)) disable_instrumentation_global;
 int blacklisted_global;
 
 int __attribute__((section("__DATA, __common"))) sectioned_global; // KASAN - ignore globals in a section
@@ -50,7 +51,7 @@
 // UWTABLE: attributes #[[#ATTR]] = { nounwind uwtable }
 // UWTABLE: ![[#]] = !{i32 7, !"uwtable", i32 1}
 
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[DISABLE_INSTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
 // CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, i1 false}
@@ -58,22 +59,24 @@
 // CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], !"dyn_init_global", i1 true, i1 false}
 // CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 11, i32 5}
 // CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// CHECK: ![[DISABLE_INSTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
 // CHECK: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
 // CHECK: ![[SECTIONED_GLOBAL]] = !{{{.*}} ![[SECTIONED_GLOBAL_LOC:[0-9]+]], !"sectioned_global", i1 false, i1 false}
-// CHECK: ![[SECTIONED_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 15, i32 50}
+// CHECK: ![[SECTIONED_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 16, i32 50}
 // CHECK: ![[SPECIAL_GLOBAL]] = !{{{.*}} ![[SPECIAL_GLOBAL_LOC:[0-9]+]], !"__special_global", i1 false, i1 false}
-// CHECK: ![[SPECIAL_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 17, i32 5}
+// CHECK: ![[SPECIAL_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 18, i32 5}
 // CHECK: ![[STATIC_VAR]] = !{{{.*}} ![[STATIC_LOC:[0-9]+]], !"static_var", i1 false, i1 false}
-// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 21, i32 14}
+// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 22, i32 14}
 // CHECK: ![[LITERAL]] = !{{{.*}} ![[LITERAL_LOC:[0-9]+]], !"", i1 false, i1 false}
-// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 22, i32 25}
+// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 23, i32 25}
 
-// BLACKLIST-SRC: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// BLACKLIST-SRC: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[DISABLE_INSTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // BLACKLIST-SRC: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
 // BLACKLIST-SRC: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // BLACKLIST-SRC: ![[GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
 // BLACKLIST-SRC: ![[DYN_INIT_GLOBAL]] = !{{{.*}} null, null, i1 true, i1 true}
 // BLACKLIST-SRC: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// BLACKLIST-SRC: ![[DISABLE_INSTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
 // BLACKLIST-SRC: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
 // BLACKLIST-SRC: ![[SECTIONED_GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
 // BLACKLIST-SRC: ![[SPECIAL_GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
Index: clang/test/CodeGen/address-safety-attr-flavors.cpp

[PATCH] D114421: [asan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Vitaly, Evgenii, can one of you please take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114421

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

[PATCH] D114421: [asan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 393168.
glider added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114421

Files:
  clang/docs/AddressSanitizer.rst
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/test/CodeGen/address-safety-attr-flavors.cpp
  clang/test/CodeGen/asan-globals.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  
llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll

Index: llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
@@ -0,0 +1,47 @@
+; This test checks that we are not instrumenting sanitizer code.
+; RUN: opt < %s -passes='asan-pipeline' -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function with sanitize_address is instrumented.
+; Function Attrs: nounwind uwtable
+define void @instr_sa(i32* %a) sanitize_address {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @instr_sa
+; CHECK: call void @__asan_report_load
+
+
+; Function with disable_sanitizer_instrumentation is not instrumented.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi(i32* %a) disable_sanitizer_instrumentation {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi
+; CHECK-NOT: call void @__asan_report_load
+
+
+; disable_sanitizer_instrumentation takes precedence over sanitize_address.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi_sa(i32* %a) disable_sanitizer_instrumentation sanitize_address {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi_sa
+; CHECK-NOT: call void @__asan_report_load
+
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2890,6 +2890,9 @@
   // Leave if the function doesn't need instrumentation.
   if (!F.hasFnAttribute(Attribute::SanitizeAddress)) return FunctionModified;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return FunctionModified;
+
   LLVM_DEBUG(dbgs() << "ASAN instrumenting:\n" << F << "\n");
 
   initializeCallbacks(*F.getParent());
Index: clang/test/CodeGen/asan-globals.cpp
===
--- clang/test/CodeGen/asan-globals.cpp
+++ clang/test/CodeGen/asan-globals.cpp
@@ -10,6 +10,7 @@
 int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
+int __attribute__((disable_sanitizer_instrumentation)) disable_instrumentation_global;
 int ignorelisted_global;
 
 int __attribute__((section("__DATA, __common"))) sectioned_global; // KASAN - ignore globals in a section
@@ -50,31 +51,33 @@
 // UWTABLE: attributes #[[#ATTR]] = { nounwind uwtable }
 // UWTABLE: ![[#]] = !{i32 7, !"uwtable", i32 1}
 
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[IGNORELISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[DISABLE_INSTR_GLOBAL:[0-9]+]], ![[IGNORELISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
 // CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, i1 false}
 // CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 10, i32 5}
 // CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], !"dyn_init_global", i1 true, i1 false}
 // CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 11, i32 5}
-// CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
-// CHECK: ![[IGNORELISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// CHECK: ![[ATTR_GLOBAL]] = !{{{.*@attributed_global.*}}, null, null, i1 false, i1 true}
+// CHECK: ![[DISABLE_INSTR_GLOBAL]] = !{{{.*@disable_instrumentation_global.*}}, null, null, i1 false, i1 true}
+// CHECK: