[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG10264a1b21ae: Introduce noundef attribute at call sites for 
stricter poison analysis (authored by guiand).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -enable-noundef-analysis -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = [[GLOBAL:(dso_local )?global]] i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = [[GLOBAL]] i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK: [[DEFINE:define( dso_local)?]] noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK: [[DEFINE]] i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm -enable-noundef-analysis %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang -cc1 -triple aarch64-gnu-linux -x c++ -S -emit-llvm -enable-noundef-analysis %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: [[DEFINE:define( dso_local)?]] i32 @{{.*}}ret_trivial
+// CHECK-AARCH: [[DEFINE:define( dso_local)?]] i64 @{{.*}}ret_trivial
+// CHECK-INTEL: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i64 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge ret_huge() { return {}; }
+void pass_huge(Huge h) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef
+} // namespace check_structs
+
+// Passing unions by value
+// No unions may be marked noundef
+
+namespace check_unions {
+union Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: [[DEFINE]] i32 @{{.*}}ret_trivial
+// CHECK-AARCH: [[DEFINE]] i64 @{{.*}}ret_trivial
+// CHECK-INTEL: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i64 %
+
+union NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef %
+} // namespace check_unions
+
+// Passing `this` pointers
+// `this` pointer must always be defined
+
+namespace check_this {
+struct Object {
+  int data[];
+
+  Object() {
+this->data[0] = 0;
+  }
+  int getData() {
+return this->data[0];
+  }
+  Object *getThis() {
+return this;
+  }
+};
+
+void use_object() {
+  Object obj;
+  obj.getData();
+  obj.getThis();
+}
+// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 328060.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -enable-noundef-analysis -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = [[GLOBAL:(dso_local )?global]] i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = [[GLOBAL]] i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK: [[DEFINE:define( dso_local)?]] noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK: [[DEFINE]] i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm -enable-noundef-analysis %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang -cc1 -triple aarch64-gnu-linux -x c++ -S -emit-llvm -enable-noundef-analysis %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: [[DEFINE:define( dso_local)?]] i32 @{{.*}}ret_trivial
+// CHECK-AARCH: [[DEFINE:define( dso_local)?]] i64 @{{.*}}ret_trivial
+// CHECK-INTEL: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i64 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge ret_huge() { return {}; }
+void pass_huge(Huge h) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef
+} // namespace check_structs
+
+// Passing unions by value
+// No unions may be marked noundef
+
+namespace check_unions {
+union Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: [[DEFINE]] i32 @{{.*}}ret_trivial
+// CHECK-AARCH: [[DEFINE]] i64 @{{.*}}ret_trivial
+// CHECK-INTEL: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i64 %
+
+union NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef %
+} // namespace check_unions
+
+// Passing `this` pointers
+// `this` pointer must always be defined
+
+namespace check_this {
+struct Object {
+  int data[];
+
+  Object() {
+this->data[0] = 0;
+  }
+  int getData() {
+return this->data[0];
+  }
+  Object *getThis() {
+return this;
+  }
+};
+
+void use_object() {
+  Object obj;
+  obj.getData();
+  obj.getThis();
+}
+// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef %"struct.check_this::Object"* @{{.*}}Object{{.*}}getThis{{.*}

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 328055.
guiand added a comment.

Reupload patch to trigger build


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -enable-noundef-analysis -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = [[GLOBAL:(dso_local )?global]] i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = [[GLOBAL]] i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK: [[DEFINE:define( dso_local)?]] noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK: [[DEFINE]] i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm -enable-noundef-analysis %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang -cc1 -triple aarch64-gnu-linux -x c++ -S -emit-llvm -enable-noundef-analysis %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: [[DEFINE:define( dso_local)?]] i32 @{{.*}}ret_trivial
+// CHECK-AARCH: [[DEFINE:define( dso_local)?]] i64 @{{.*}}ret_trivial
+// CHECK-INTEL: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i64 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge ret_huge() { return {}; }
+void pass_huge(Huge h) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef
+} // namespace check_structs
+
+// Passing unions by value
+// No unions may be marked noundef
+
+namespace check_unions {
+union Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: [[DEFINE]] i32 @{{.*}}ret_trivial
+// CHECK-AARCH: [[DEFINE]] i64 @{{.*}}ret_trivial
+// CHECK-INTEL: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i64 %
+
+union NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef %
+} // namespace check_unions
+
+// Passing `this` pointers
+// `this` pointer must always be defined
+
+namespace check_this {
+struct Object {
+  int data[];
+
+  Object() {
+this->data[0] = 0;
+  }
+  int getData() {
+return this->data[0];
+  }
+  Object *getThis() {
+return this;
+  }
+};
+
+void use_object() {
+  Object obj;
+  obj.getData();
+  obj.getThis();
+}
+// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef %"s

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Nuno Lopes via Phabricator via cfe-commits]

nlopes added a comment.

@rsmith already gave his blessing, so please go ahead.

I hope you guys have a plan to enable this by default at some point as features 
behind a flag get rotten and no one uses them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand added a comment.

For sure. I'll upload a rebased patch shortly and give it another day or so for 
people to look, and then push if there aren't any issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

Any more comments/opinions?
Gui, would you like to push it to main if no one objects?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis accepted this revision.
eugenis 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/D81678/new/

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 322899.
guiand added a comment.

Updated to use -enable-noundef-analysis


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -enable-noundef-analysis -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = [[GLOBAL:(dso_local )?global]] i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = [[GLOBAL]] i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK: [[DEFINE:define( dso_local)?]] noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK: [[DEFINE]] i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm -enable-noundef-analysis %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang -cc1 -triple aarch64-gnu-linux -x c++ -S -emit-llvm -enable-noundef-analysis %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: [[DEFINE:define( dso_local)?]] i32 @{{.*}}ret_trivial
+// CHECK-AARCH: [[DEFINE:define( dso_local)?]] i64 @{{.*}}ret_trivial
+// CHECK-INTEL: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i64 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge ret_huge() { return {}; }
+void pass_huge(Huge h) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef
+} // namespace check_structs
+
+// Passing unions by value
+// No unions may be marked noundef
+
+namespace check_unions {
+union Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: [[DEFINE]] i32 @{{.*}}ret_trivial
+// CHECK-AARCH: [[DEFINE]] i64 @{{.*}}ret_trivial
+// CHECK-INTEL: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: [[DEFINE]] void @{{.*}}pass_trivial{{.*}}(i64 %
+
+union NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: [[DEFINE]] void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret({{[^)]+}}) align 4 %
+// CHECK: [[DEFINE]] void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef %
+} // namespace check_unions
+
+// Passing `this` pointers
+// `this` pointer must always be defined
+
+namespace check_this {
+struct Object {
+  int data[];
+
+  Object() {
+this->data[0] = 0;
+  }
+  int getData() {
+return this->data[0];
+  }
+  Object *getThis() {
+return this;
+  }
+};
+
+void use_object() {
+  Object obj;
+  obj.getData();
+  obj.getThis();
+}
+// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr nou

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

I'm fine with any direction that helps people land test updates/implementations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand added a comment.

Changing the mechanism to explicit opt-in seems like a good idea, and bypasses 
the test issues.

If everyone agrees about proceeding with this, I can fix up this patch within 
the next couple days and we can hopefully wrap this up!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.
Herald added a reviewer: jansvoboda11.

@guiand Do you plan to continue working on this patch? If not, I would try to 
finalize it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

In D81678#2503931 , @nikic wrote:

> As the discussion is spread out across multiple threads, do I understand 
> correctly that the current consensus is to introduce the 
> `-disable-noundef-analysis` flag, and explicitly add it to all the relevant 
> tests (rather than adding it to the substitutions)?

Yes, I think so.

> In any case, I'd recommend changing this patch to default 
> `-disable-noundef-analysis` to true (so you need to compile with 
> `-disable-noundef-analysis=0` to get undef attributes). The flag can then be 
> flipped together with the test changes. That should help get the main 
> technical change landed directly, and avoid the need of landing patches at 
> the same time.

This is a great idea! Aside from splitting the complexity of landing the large 
change, it also makes our downstream cleanup easier.

In that case we should probably give the flag a positive name: 
-enable-noundef-analysis=(0|1).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

As the discussion is spread out across multiple threads, do I understand 
correctly that the current consensus is to introduce the 
`-disable-noundef-analysis` flag, and explicitly add it to all the relevant 
tests (rather than adding it to the substitutions)?

In any case, I'd recommend changing this patch to default 
`-disable-noundef-analysis` to true (so you need to compile with 
`-disable-noundef-analysis=0` to get undef attributes). The flag can then be 
flipped together with the test changes. That should help get the main technical 
change landed directly, and avoid the need of landing patches at the same time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

In D81678#2438264 , @eugenis wrote:

> This change looks fine to me, but I'm slightly concerned about 
> https://reviews.llvm.org/D85788 - see my last comment on that revision.

Thank you for the link! I left a comment there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

This change looks fine to me, but I'm slightly concerned about 
https://reviews.llvm.org/D85788 - see my last comment on that revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

Hello all,
What is the status of this patch?
Do we need someone who look into the lit change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis updated this revision to Diff 298692.
eugenis added a comment.

fix type size check for vscale types


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp
  clang/test/lit.cfg.py
  llvm/utils/update_cc_test_checks.py

Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -28,9 +28,9 @@
 from UpdateTestChecks import common
 
 SUBST = {
-'%clang': [],
-'%clang_cc1': ['-cc1'],
-'%clangxx': ['--driver-mode=g++'],
+'%clang': ['-Xclang -disable-noundef-analysis'],
+'%clang_cc1': ['-cc1', '-disable-noundef-analysis'],
+'%clangxx': ['--driver-mode=g++', '-disable-noundef-analysis'],
 }
 
 def get_line2spell_and_mangled(args, clang_args):
@@ -161,8 +161,8 @@
   try:
 builtin_include_dir = subprocess.check_output(
   [args.clang, '-print-file-name=include']).decode().strip()
-SUBST['%clang_cc1'] = ['-cc1', '-internal-isystem', builtin_include_dir,
-   '-nostdsysteminc']
+SUBST['%clang_cc1'] += ['-internal-isystem', builtin_include_dir,
+'-nostdsysteminc']
   except subprocess.CalledProcessError:
 common.warn('Could not determine clang builtins directory, some tests '
 'might not update correctly.')
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -41,7 +41,8 @@
 
 llvm_config.use_default_substitutions()
 
-llvm_config.use_clang()
+llvm_config.use_clang(cc_additional_flags=['-Xclang -disable-noundef-analysis'],
+  cc1_additional_flags=['-disable-noundef-analysis'])
 
 config.substitutions.append(
 ('%src_include_dir', config.clang_src_dir + '/include'))
Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_bin -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,163 @@
+// RUN: %clang_bin -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang_bin -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang_bin -cc1 -triple aarch64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: define i32 @{{.*}}ret_trivial
+// CHECK-AARCH: define i64 @{{.*}}ret_trivial
+// CHECK-INTEL: define void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: define void @{{.*}}pass_trivial{{.*}}(i64 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define {{(dso_local)?}}void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

I think it makes sense - IIUC, for most of the clang tests, noundef won't be 
the attribute of interest.
For brevity of tests, I think the change is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 284931.
guiand added a comment.

Made the compiler flag non-public


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp
  clang/test/lit.cfg.py
  llvm/utils/update_cc_test_checks.py

Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -28,9 +28,9 @@
 from UpdateTestChecks import common
 
 SUBST = {
-'%clang': [],
-'%clang_cc1': ['-cc1'],
-'%clangxx': ['--driver-mode=g++'],
+'%clang': ['-Xclang -disable-noundef-analysis'],
+'%clang_cc1': ['-cc1', '-disable-noundef-analysis'],
+'%clangxx': ['--driver-mode=g++', '-disable-noundef-analysis'],
 }
 
 def get_line2spell_and_mangled(args, clang_args):
@@ -161,8 +161,8 @@
   try:
 builtin_include_dir = subprocess.check_output(
   [args.clang, '-print-file-name=include']).decode().strip()
-SUBST['%clang_cc1'] = ['-cc1', '-internal-isystem', builtin_include_dir,
-   '-nostdsysteminc']
+SUBST['%clang_cc1'] += ['-internal-isystem', builtin_include_dir,
+'-nostdsysteminc']
   except subprocess.CalledProcessError:
 common.warn('Could not determine clang builtins directory, some tests '
 'might not update correctly.')
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -41,7 +41,8 @@
 
 llvm_config.use_default_substitutions()
 
-llvm_config.use_clang()
+llvm_config.use_clang(cc_additional_flags=['-Xclang -disable-noundef-analysis'],
+  cc1_additional_flags=['-disable-noundef-analysis'])
 
 config.substitutions.append(
 ('%src_include_dir', config.clang_src_dir + '/include'))
Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_bin -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,163 @@
+// RUN: %clang_bin -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang_bin -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang_bin -cc1 -triple aarch64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: define i32 @{{.*}}ret_trivial
+// CHECK-AARCH: define i64 @{{.*}}ret_trivial
+// CHECK-INTEL: define void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: define void @{{.*}}pass_trivial{{.*}}(i64 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define {{(dso_local)?}}void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge 

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand requested review of this revision.
guiand added a comment.

I think I'd like someone to take a look at the `llvm-lit` changes to see if 
this makes sense as an approach


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 284907.
guiand added a comment.
Herald added a subscriber: delcypher.

To try to alleviate the tests issue, @eugenis and I discussed that it might be 
best to take it slow. So now this patch will mask off emitting the attribute on 
clang tests by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp
  clang/test/lit.cfg.py
  llvm/utils/lit/lit/llvm/config.py
  llvm/utils/update_cc_test_checks.py

Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -28,9 +28,9 @@
 from UpdateTestChecks import common
 
 SUBST = {
-'%clang': [],
-'%clang_cc1': ['-cc1'],
-'%clangxx': ['--driver-mode=g++'],
+'%clang': ['-fdisable-noundef-analysis'],
+'%clang_cc1': ['-cc1', '-fdisable-noundef-analysis'],
+'%clangxx': ['--driver-mode=g++', '-fdisable-noundef-analysis'],
 }
 
 def get_line2spell_and_mangled(args, clang_args):
@@ -161,8 +161,8 @@
   try:
 builtin_include_dir = subprocess.check_output(
   [args.clang, '-print-file-name=include']).decode().strip()
-SUBST['%clang_cc1'] = ['-cc1', '-internal-isystem', builtin_include_dir,
-   '-nostdsysteminc']
+SUBST['%clang_cc1'] += ['-internal-isystem', builtin_include_dir,
+'-nostdsysteminc']
   except subprocess.CalledProcessError:
 common.warn('Could not determine clang builtins directory, some tests '
 'might not update correctly.')
Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -412,7 +412,7 @@
 ToolSubst('%clang_analyze_cc1', command='%clang_cc1', extra_args=['-analyze', '%analyze', '-setup-static-analyzer']+additional_flags),
 ToolSubst('%clang_cc1', command=self.config.clang, extra_args=['-cc1', '-internal-isystem', builtin_include_dir, '-nostdsysteminc']+additional_flags),
 ToolSubst('%clang_cpp', command=self.config.clang, extra_args=['--driver-mode=cpp']+additional_flags),
-ToolSubst('%clang_cl', command=self.config.clang, extra_args=['--driver-mode=cl']+additional_flags),
+ToolSubst('%clang_cl', command=self.config.clang, extra_args=['--driver-mode=cl']),
 ToolSubst('%clangxx', command=self.config.clang, extra_args=['--driver-mode=g++']+additional_flags),
 ToolSubst('%clang_bin', command=self.config.clang),
 ]
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -41,7 +41,7 @@
 
 llvm_config.use_default_substitutions()
 
-llvm_config.use_clang()
+llvm_config.use_clang(additional_flags=['-fdisable-noundef-analysis'])
 
 config.substitutions.append(
 ('%src_include_dir', config.clang_src_dir + '/include'))
Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_bin -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,163 @@
+// RUN: %clang_bin -cc1 -triple x86_64-g

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand marked 5 inline comments as done.
guiand added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2148-2150
+} else if (const VarDecl *VDecl = dyn_cast(TargetDecl)) {
+  // Function pointer
+  HasStrictReturn &= !VDecl->isExternC();

rsmith wrote:
> `TargetDecl` (the callee of a function call) should never be a variable. You 
> shouldn't need this check.
I tried replacing the body of this if statement with an assertion to make sure, 
and the assertion fires.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 281676.
guiand added a comment.

Addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,163 @@
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang_cc1 -triple aarch64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: define i32 @{{.*}}ret_trivial
+// CHECK-AARCH: define i64 @{{.*}}ret_trivial
+// CHECK-INTEL: define void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: define void @{{.*}}pass_trivial{{.*}}(i64 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define {{(dso_local)?}}void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge ret_huge() { return {}; }
+void pass_huge(Huge h) {}
+// CHECK: define void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef
+} // namespace check_structs
+
+// Passing unions by value
+// No unions may be marked noundef
+
+namespace check_unions {
+union Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: define i32 @{{.*}}ret_trivial
+// CHECK-AARCH: define i64 @{{.*}}ret_trivial
+// CHECK-INTEL: define void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: define void @{{.*}}pass_trivial{{.*}}(i64 %
+
+union NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef %
+} // namespace check_unions
+
+// Passing `this` pointers
+// `this` pointer must always be defined
+
+namespace check_this {
+struct Object {
+  int data[];
+
+  Object() {
+this->data[0] = 0;
+  }
+  int getData() {
+return this->data[0];
+  }
+  Object *getThis() {
+return this;
+  }
+};
+
+void use_object() {
+  Object obj;
+  obj.getData();
+  obj.getThis();
+}
+// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef %"struct.check_this::Object"* @{{.*}}Object{{.*}}getThis{{.*}}(%"struc

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 281679.
guiand added a comment.

Updated comment on disable-noundef-args option


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,163 @@
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL
+// RUN: %clang_cc1 -triple aarch64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: define i32 @{{.*}}ret_trivial
+// CHECK-AARCH: define i64 @{{.*}}ret_trivial
+// CHECK-INTEL: define void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: define void @{{.*}}pass_trivial{{.*}}(i64 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define {{(dso_local)?}}void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge ret_huge() { return {}; }
+void pass_huge(Huge h) {}
+// CHECK: define void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef
+} // namespace check_structs
+
+// Passing unions by value
+// No unions may be marked noundef
+
+namespace check_unions {
+union Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK-INTEL: define i32 @{{.*}}ret_trivial
+// CHECK-AARCH: define i64 @{{.*}}ret_trivial
+// CHECK-INTEL: define void @{{.*}}pass_trivial{{.*}}(i32 %
+// CHECK-AARCH: define void @{{.*}}pass_trivial{{.*}}(i64 %
+
+union NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef %
+} // namespace check_unions
+
+// Passing `this` pointers
+// `this` pointer must always be defined
+
+namespace check_this {
+struct Object {
+  int data[];
+
+  Object() {
+this->data[0] = 0;
+  }
+  int getData() {
+return this->data[0];
+  }
+  Object *getThis() {
+return this;
+  }
+};
+
+void use_object() {
+  Object obj;
+  obj.getData();
+  obj.getThis();
+}
+// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef %"struct.check_this::Object"* @{{.*}}Objec

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

Looks good with suggestions applied, and with the portability problems in the 
test fixed. (Maybe just add a `-triple`? Though it would be good to also test 
`inalloca` and ARM constructor `this` returns and so on.)




Comment at: clang/include/clang/Driver/Options.td:3957
+def disable_noundef_args : Flag<["-"], "disable-noundef-args">,
+  HelpText<"Disable emitting noundef attribute in LLVM IR">;
 def load : Separate<["-"], "load">, MetaVarName<"">,

(since this doesn't affect return values)



Comment at: clang/lib/CodeGen/CGCall.cpp:2144
+  bool HasStrictReturn =
+  getLangOpts().CPlusPlus || getLangOpts().OpenCLCPlusPlus;
+  if (TargetDecl) {

`OpenCLCPlusPlus` should only ever be `true` when `CPlusPlus` is also `true`, 
so the second half of this `||` should be unnecessary.



Comment at: clang/lib/CodeGen/CGCall.cpp:2148-2150
+} else if (const VarDecl *VDecl = dyn_cast(TargetDecl)) {
+  // Function pointer
+  HasStrictReturn &= !VDecl->isExternC();

`TargetDecl` (the callee of a function call) should never be a variable. You 
shouldn't need this check.



Comment at: clang/test/CodeGen/attr-noundef.cpp:22
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* 
noalias sret align 4 %
+// CHECK: define void 
@{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+

I think this test will fail on 32-bit Windows because `e` will be passed 
`inalloca`.



Comment at: clang/test/CodeGen/attr-noundef.cpp:78
+}
+// CHECK: define linkonce_odr void 
@{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 
@{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %

I believe this test will fail on ARM, due to constructors implicitly returning 
`this`. Consider either specifying a target for this test or weakening the 
`void` to `{{.*}}` here.



Comment at: clang/test/CodeGen/attr-noundef.cpp:79
+// CHECK: define linkonce_odr void 
@{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 
@{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef %"struct.check_this::Object"* 
@{{.*}}Object{{.*}}getThis{{.*}}(%"struct.check_this::Object"* noundef %

I believe this test will fail on Windows, because `getData` and `Object` will 
appear in the opposite order in the mangling. Consider either specifying a 
target or matching these names either way around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 281345.
guiand added a comment.

Fix typo in MayDropFunctionReturn


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,152 @@
+// RUN: %clang_cc1 -x c++ -S -emit-llvm %s -o - | FileCheck %s
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK: define i32 @{{.*}}ret_trivial
+// CHECK: define void @{{.*}}pass_trivial{{.*}}(i32 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge ret_huge() { return {}; }
+void pass_huge(Huge h) {}
+// CHECK: define void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef byval
+} // namespace check_structs
+
+// Passing unions by value
+// No unions may be marked noundef
+
+namespace check_unions {
+union Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK: define i32 @{{.*}}ret_trivial
+// CHECK: define void @{{.*}}pass_trivial{{.*}}(i32 %
+
+union NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef %
+} // namespace check_unions
+
+// Passing `this` pointers
+// `this` pointer must always be defined
+
+namespace check_this {
+struct Object {
+  int data[];
+
+  Object() {
+this->data[0] = 0;
+  }
+  int getData() {
+return this->data[0];
+  }
+  Object *getThis() {
+return this;
+  }
+};
+
+void use_object() {
+  Object obj;
+  obj.getData();
+  obj.getThis();
+}
+// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef %"struct.check_this::Object"* @{{.*}}Object{{.*}}getThis{{.*}}(%"struct.check_this::Object"* noundef %
+} // namespace check_this
+
+// Passing vector types
+
+namespace check_vecs {
+typedef int __attribute__((vector_size(12))) i32x3;
+i32x3 ret_vec() {
+  return {};
+}
+void pass_vec(i32x3 v) {
+}
+
+// CHECK: define noundef <3 x i32> @{{.*}}ret_vec{{.*}}()
+// CHECK: define void @{{.*}}pass_vec{{.*}}(<3 x i32> noundef %
+} // namespace check_vecs
+
+// Passing exotic types
+// Function/Array pointers, Function member / Data member pointers, nullptr_t, ExtInt types
+
+namespace check_exotic {
+struct Object {
+  int mfu

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 281323.
guiand edited the summary of this revision.
guiand added a comment.

Fixes regression; allows emitting noundef for non-FunctionDecls as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,152 @@
+// RUN: %clang_cc1 -x c++ -S -emit-llvm %s -o - | FileCheck %s
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK: define i32 @{{.*}}ret_trivial
+// CHECK: define void @{{.*}}pass_trivial{{.*}}(i32 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge ret_huge() { return {}; }
+void pass_huge(Huge h) {}
+// CHECK: define void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef byval
+} // namespace check_structs
+
+// Passing unions by value
+// No unions may be marked noundef
+
+namespace check_unions {
+union Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK: define i32 @{{.*}}ret_trivial
+// CHECK: define void @{{.*}}pass_trivial{{.*}}(i32 %
+
+union NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef %
+} // namespace check_unions
+
+// Passing `this` pointers
+// `this` pointer must always be defined
+
+namespace check_this {
+struct Object {
+  int data[];
+
+  Object() {
+this->data[0] = 0;
+  }
+  int getData() {
+return this->data[0];
+  }
+  Object *getThis() {
+return this;
+  }
+};
+
+void use_object() {
+  Object obj;
+  obj.getData();
+  obj.getThis();
+}
+// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef %"struct.check_this::Object"* @{{.*}}Object{{.*}}getThis{{.*}}(%"struct.check_this::Object"* noundef %
+} // namespace check_this
+
+// Passing vector types
+
+namespace check_vecs {
+typedef int __attribute__((vector_size(12))) i32x3;
+i32x3 ret_vec() {
+  return {};
+}
+void pass_vec(i32x3 v) {
+}
+
+// CHECK: define noundef <3 x i32> @{{.*}}ret_vec{{.*}}()
+// CHECK: define void @{{.*}}pass_vec{{.*}}(<3 x i32> noundef %
+} // namespace check_vecs
+
+// Passing exotic types
+// Function/Array pointers, Function member / Data member pointe

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 281311.
guiand added a comment.

Incorporate C++'s more conservative checks for omitted return values


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,152 @@
+// RUN: %clang_cc1 -x c++ -S -emit-llvm %s -o - | FileCheck %s
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK: define i32 @{{.*}}ret_trivial
+// CHECK: define void @{{.*}}pass_trivial{{.*}}(i32 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge ret_huge() { return {}; }
+void pass_huge(Huge h) {}
+// CHECK: define void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef byval
+} // namespace check_structs
+
+// Passing unions by value
+// No unions may be marked noundef
+
+namespace check_unions {
+union Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK: define i32 @{{.*}}ret_trivial
+// CHECK: define void @{{.*}}pass_trivial{{.*}}(i32 %
+
+union NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef %
+} // namespace check_unions
+
+// Passing `this` pointers
+// `this` pointer must always be defined
+
+namespace check_this {
+struct Object {
+  int data[];
+
+  Object() {
+this->data[0] = 0;
+  }
+  int getData() {
+return this->data[0];
+  }
+  Object *getThis() {
+return this;
+  }
+};
+
+void use_object() {
+  Object obj;
+  obj.getData();
+  obj.getThis();
+}
+// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef %"struct.check_this::Object"* @{{.*}}Object{{.*}}getThis{{.*}}(%"struct.check_this::Object"* noundef %
+} // namespace check_this
+
+// Passing vector types
+
+namespace check_vecs {
+typedef int __attribute__((vector_size(12))) i32x3;
+i32x3 ret_vec() {
+  return {};
+}
+void pass_vec(i32x3 v) {
+}
+
+// CHECK: define noundef <3 x i32> @{{.*}}ret_vec{{.*}}()
+// CHECK: define void @{{.*}}pass_vec{{.*}}(<3 x i32> noundef %
+} // namespace check_vecs
+
+// Passing exotic types
+// Function/Array pointers, Function member / Data member pointers, nullptr_t, ExtInt types
+
+namespace check_e

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 280622.
guiand added a comment.

Added an across-the-board test with several different interesting cases. 
@rsmith, I'm not sure how to test things like VTables/VTTs, since afaik the way 
they would be exposed to function attributes would be if a struct is being 
passed by value. But in that case, we currently never emit noundef.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,152 @@
+// RUN: %clang_cc1 -x c++ -S -emit-llvm %s -o - | FileCheck %s
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK: define i32 @{{.*}}ret_trivial
+// CHECK: define void @{{.*}}pass_trivial{{.*}}(i32 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge ret_huge() { return {}; }
+void pass_huge(Huge h) {}
+// CHECK: define void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef byval
+} // namespace check_structs
+
+// Passing unions by value
+// No unions may be marked noundef
+
+namespace check_unions {
+union Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK: define i32 @{{.*}}ret_trivial
+// CHECK: define void @{{.*}}pass_trivial{{.*}}(i32 %
+
+union NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef %
+} // namespace check_unions
+
+// Passing `this` pointers
+// `this` pointer must always be defined
+
+namespace check_this {
+struct Object {
+  int data[];
+
+  Object() {
+this->data[0] = 0;
+  }
+  int getData() {
+return this->data[0];
+  }
+  Object *getThis() {
+return this;
+  }
+};
+
+void use_object() {
+  Object obj;
+  obj.getData();
+  obj.getThis();
+}
+// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef %"struct.check_this::Object"* @{{.*}}Object{{.*}}getThis{{.*}}(%"struct.check_this::Object"* noundef %
+} // namespace check_this
+
+// Passing vector types
+
+namespace check_vecs {
+typedef int __attribute__((vector_size(12))) i32x3;
+i32x3 ret_vec() {
+  return {};
+}
+void pass_vec(i32x3 v) {
+}
+
+// CHECK: define noundef <3 x i32> @{{.*}}ret_vec{{.*}}()
+// CHECK: define void @{{.*}}pass_vec{{.*}}(<3 x i32> noundef %
+} // namespace check_vec

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 279949.
guiand added a comment.

Adds additional constraints on `noundef`: Not a `nullptr_t`, not a member 
pointer, and not coerced to a type of larger size. Disabled emitting in return 
position for non-C++ languages (or inside extern "C").

@rsmith, I just didn't address your comment about being conservative with 
black-holeing C++ functions returning undef. At least for msan purposes this 
shouldn't be a *too* much of a problem, but of course this attribute is for 
general use by LLVM passes. I'm just wondering what kind of tradeoff we want to 
make here, I suppose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 { int val; };
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) { return a; }
+
+int main() {
+// CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+indirect_callee_int(0);
+// CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+indirect_callee_union((union u1){0});
+
+indirect_callee_int_ptr = indirect_callee_int;
+indirect_callee_union_ptr = indirect_callee_union;
+
+// CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+indirect_callee_int_ptr(0);
+// CHECK: call i32 %{{.*}}(i32 %
+indirect_callee_union_ptr((union u1){});
+
+return 0;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -924,6 +924,7 @@
 
   Opts.DisableFree = Args.hasArg(OPT_disable_free);
   Opts.DiscardValueNames = Args.hasArg(OPT_discard_value_names);
+  Opts.DisableNoundefArgs = Args.hasArg(OPT_disable_noundef_args);
   Opts.DisableTailCalls = Args.hasArg(OPT_mdisable_tail_calls);
   Opts.NoEscapingBlockTailCalls =
   Args.hasArg(OPT_fno_escaping_block_tail_calls);
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1876,6 +1876,54 @@
   llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
 }
 
+static bool DetermineNoUndef(QualType QTy, CodeGenTypes &Types,
+ const llvm::DataLayout &DL, const ABIArgInfo &AI,
+ bool CheckCoerce = true) {
+  llvm::Type *Ty = Types.ConvertTypeForMem(QTy);
+  if (AI.getKind() == ABIArgInfo::Indirect)
+return true;
+  if (AI.getKind() == ABIArgInfo::Extend)
+return true;
+  if (!DL.typeSizeEqualsStoreSize(Ty))
+// TODO: This will result in a modest amount of values not marked noundef
+// when they could be. We care about values that *invisibly* contain undef
+// bits from the perspective of LLVM IR. 
+return false;
+  if (CheckCoerce && AI.canHaveCoerceToType()) {
+llvm::Type *CoerceTy = AI.getCoerceToType();
+if (DL.getTypeSizeInBits(CoerceTy) > DL.getTypeSizeInBits(Ty))
+  // If we're coercing to a type with a greater size than the canonical one,
+  // we're introducing new undef bits.
+  // Coercing to a type of smaller or equal size is ok, as we know that
+  // there's no internal padding (typeSizeEqualsStoreSize).
+  return false;
+  }
+  if (QTy->isExtIntType())
+return true;
+  if (QTy->isReferenceType())
+return true;
+  if (QTy->isNullPtrType())
+return false;
+  if (QTy->isMemberPointerType())
+// TODO: Some member pointers are `noundef`, but it depends on the ABI. For
+// now, never mark them.
+return false;
+  if (QTy->isScalarType()) {
+if (const ComplexType *Complex = dyn_cast(QTy))
+  return DetermineNoUndef(Complex->getElementType(), Types, DL, AI, false);
+return true;
+  }
+  if (const VectorType *Vector = dyn_cast(QTy))
+return DetermineNoUndef(Vector->getElementType(), Types, DL, AI, false);
+  if (const MatrixType *Matrix = dyn_cast(QTy))
+return DetermineNoUndef(Matrix->getElementType(), Types, DL, AI, false);
+  if (const ArrayType *Array = dyn_cast(

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand added a comment.

Just saw your comment about tests as well. The idea was to have all tests 
ported over as part of a separate commit (I linked it in the main patch 
description) and then only to push either commit once both are ready to land.

To make it easier to be sure this specific feature is working correctly, I can 
port over some of those tests to this patch for review purposes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2110
+  DetermineNoUndef(RetTy, getTypes(), DL, RetAI)) {
+RetAttrs.addAttribute(llvm::Attribute::NoUndef);
+  }

rsmith wrote:
> This only applies in C++. In C, it's valid for a function to omit its 
> `return` statement if the result value is unused, and in that case the 
> returned value will legitimately be `undef`.
> 
> Even in C++, we're conservative about enforcing the constraint that functions 
> return a proper value and control flow doesn't just fall off the bottom of 
> the function. See the full set of checks here: 
> https://github.com/llvm/llvm-project/blob/master/clang/lib/CodeGen/CodeGenFunction.cpp#L1322
It would seem sensible to suppress this for `extern "C"` functions too, on the 
basis that the definition of such functions is probably in C code, and thus 
they might return `undef`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith requested changes to this revision.
rsmith added a comment.
This revision now requires changes to proceed.

I'd like to see some testcases for the C++ side of this. The following things 
all seem like they might be interesting: passing and returning classes and 
unions, especially ones that aren't trivially-copyable, `nullptr_t`, pointers 
to members, `this` parameters, VTTs, the "complete object" flag for MS ABI 
constructors, the `this` return from constructors in the ARM ABI, parameters 
and return values of virtual adjustment thunks.




Comment at: clang/lib/CodeGen/CGCall.cpp:1892
+return true;
+  if (QTy->isScalarType()) {
+if (const ComplexType *Complex = dyn_cast(QTy))

This includes `nullptr_t` (which is never ` noundef`) and member pointers 
(which are sometimes `noundef`, and you'll need to ask the `CXXABI` 
implementation if you don't want to conservatively return `false`).



Comment at: clang/lib/CodeGen/CGCall.cpp:2110
+  DetermineNoUndef(RetTy, getTypes(), DL, RetAI)) {
+RetAttrs.addAttribute(llvm::Attribute::NoUndef);
+  }

This only applies in C++. In C, it's valid for a function to omit its `return` 
statement if the result value is unused, and in that case the returned value 
will legitimately be `undef`.

Even in C++, we're conservative about enforcing the constraint that functions 
return a proper value and control flow doesn't just fall off the bottom of the 
function. See the full set of checks here: 
https://github.com/llvm/llvm-project/blob/master/clang/lib/CodeGen/CodeGenFunction.cpp#L1322


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand closed this revision.
guiand added a comment.

Merged in https://reviews.llvm.org/rG780528d9da707b15849d6c9711cc3ab19f6c7f00


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert resigned from this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

My concerns have been addressed. I am not the right one to look at the clang 
changes but what we merged into LLVM was really useful, thanks again for 
working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

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

Sorry, closed this mistakenly!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand added a comment.
Herald added a subscriber: dang.

Is anything still pending here (besides the tests, of course)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 276487.
guiand added a comment.

Per @nikic's suggestion, I isolated the LLVM side of the changes to a separate 
revision D83412 , which should be good to go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/indirect-noundef.c

Index: clang/test/CodeGen/indirect-noundef.c
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 { int val; };
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @indirect_callee_int(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @indirect_callee_union(i32 %
+union u1 indirect_callee_union(union u1 a) { return a; }
+
+int main() {
+// CHECK: call noundef i32 @indirect_callee_int(i32 noundef 0)
+indirect_callee_int(0);
+// CHECK: call i32 @indirect_callee_union(i32 %
+indirect_callee_union((union u1){0});
+
+indirect_callee_int_ptr = indirect_callee_int;
+indirect_callee_union_ptr = indirect_callee_union;
+
+// CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+indirect_callee_int_ptr(0);
+// CHECK: call i32 %{{.*}}(i32 %
+indirect_callee_union_ptr((union u1){});
+
+return 0;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -925,6 +925,7 @@
 
   Opts.DisableFree = Args.hasArg(OPT_disable_free);
   Opts.DiscardValueNames = Args.hasArg(OPT_discard_value_names);
+  Opts.DisableNoundefArgs = Args.hasArg(OPT_disable_noundef_args);
   Opts.DisableTailCalls = Args.hasArg(OPT_mdisable_tail_calls);
   Opts.NoEscapingBlockTailCalls =
   Args.hasArg(OPT_fno_escaping_block_tail_calls);
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1876,6 +1876,33 @@
   llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
 }
 
+static bool DetermineNoUndef(QualType QTy, CodeGenTypes &Types,
+ const llvm::DataLayout &DL, const ABIArgInfo &AI) {
+  llvm::Type *Ty = Types.ConvertTypeForMem(QTy);
+  if (AI.getKind() == ABIArgInfo::Indirect)
+return true;
+  if (AI.getKind() == ABIArgInfo::Extend)
+return true;
+  if (!DL.typeSizeEqualsStoreSize(Ty))
+return false;
+  if (QTy->isExtIntType())
+return true;
+  if (QTy->isReferenceType())
+return true;
+  if (QTy->isScalarType()) {
+if (const ComplexType *Complex = dyn_cast(QTy))
+  return DetermineNoUndef(Complex->getElementType(), Types, DL, AI);
+return true;
+  }
+  if (const VectorType *Vector = dyn_cast(QTy))
+return DetermineNoUndef(Vector->getElementType(), Types, DL, AI);
+  if (const MatrixType *Matrix = dyn_cast(QTy))
+return DetermineNoUndef(Matrix->getElementType(), Types, DL, AI);
+  if (const ArrayType *Array = dyn_cast(QTy))
+return DetermineNoUndef(Array->getElementType(), Types, DL, AI);
+  return false;
+}
+
 /// Construct the IR attribute list of a function or call.
 ///
 /// When adding an attribute, please consider where it should be handled:
@@ -2075,6 +2102,14 @@
 
   QualType RetTy = FI.getReturnType();
   const ABIArgInfo &RetAI = FI.getReturnInfo();
+  const llvm::DataLayout &DL = getDataLayout();
+
+  // Determine if the return type could be partially undef
+  if (!RetTy->isVoidType() && RetAI.getKind() != ABIArgInfo::Indirect &&
+  DetermineNoUndef(RetTy, getTypes(), DL, RetAI)) {
+RetAttrs.addAttribute(llvm::Attribute::NoUndef);
+  }
+
   switch (RetAI.getKind()) {
   case ABIArgInfo::Extend:
 if (RetAI.isSignExt())
@@ -2160,6 +2195,12 @@
   }
 }
 
+// Decide whether the argument we're handling could be partially undef
+bool ArgNoUndef = DetermineNoUndef(ParamType, getTypes(), DL, AI);
+if (!CodeGenOpts.DisableNoundefArgs && ArgNoUndef) {
+  Attrs.addAttribute(llvm::Attribute::NoUndef);
+}
+
 // 'restrict' -> 'noalias' is done in EmitFunctionProlog when we
 // have the corresponding parameter variable.  It doesn't make
 // sense to do it here because parameters are so messed up.
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 274895.
guiand added a comment.

Addressed comments, added test for indirect calls


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/indirect-noundef.c
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.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
@@ -877,6 +877,7 @@
   case Attribute::NoMerge:
   case Attribute::NoReturn:
   case Attribute::NoSync:
+  case Attribute::NoUndef:
   case Attribute::None:
   case Attribute::NonNull:
   case Attribute::Preallocated:
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -443,6 +443,8 @@
 return "cold";
   if (hasAttribute(Attribute::ImmArg))
 return "immarg";
+  if (hasAttribute(Attribute::NoUndef))
+return "noundef";
 
   if (hasAttribute(Attribute::ByVal)) {
 std::string Result;
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -731,6 +731,8 @@
 return bitc::ATTR_KIND_SANITIZE_MEMTAG;
   case Attribute::Preallocated:
 return bitc::ATTR_KIND_PREALLOCATED;
+  case Attribute::NoUndef:
+return bitc::ATTR_KIND_NOUNDEF;
   case Attribute::EndAttrKinds:
 llvm_unreachable("Can not encode end-attribute kinds marker.");
   case Attribute::None:
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1530,6 +1530,8 @@
 return Attribute::SanitizeMemTag;
   case bitc::ATTR_KIND_PREALLOCATED:
 return Attribute::Preallocated;
+  case bitc::ATTR_KIND_NOUNDEF:
+return Attribute::NoUndef;
   }
 }
 
Index: llvm/lib/AsmParser/LLToken.h
===
--- llvm/lib/AsmParser/LLToken.h
+++ llvm/lib/AsmParser/LLToken.h
@@ -196,6 +196,7 @@
   kw_naked,
   kw_nest,
   kw_noalias,
+  kw_noundef,
   kw_nobuiltin,
   kw_nocapture,
   kw_noduplicate,
Index: llvm/lib/AsmParser/LLParser.cpp
===
--- llvm/lib/AsmParser/LLParser.cpp
+++ llvm/lib/AsmParser/LLParser.cpp
@@ -1374,6 +1374,7 @@
 case lltok::kw_inalloca:
 case lltok::kw_nest:
 case lltok::kw_noalias:
+case lltok::kw_noundef:
 case lltok::kw_nocapture:
 case lltok::kw_nonnull:
 case lltok::kw_returned:
@@ -1677,6 +1678,9 @@
 case lltok::kw_inalloca:B.addAttribute(Attribute::InAlloca); break;
 case lltok::kw_inreg:   B.addAttribute(Attribute::InReg); break;
 case lltok::kw_nest:B.addAttribute(Attribute::Nest); break;
+case lltok::kw_noundef:
+  B.addAttribute(Attribute::NoUndef);
+  break;
 case lltok::kw_noalias: B.addAttribute(Attribute::NoAlias); break;
 case lltok::kw_nocapture:   B.addAttribute(Attribute::NoCapture); break;
 case lltok::kw_nofree:  B.addAttribute(Attribute::NoFree); break;
@@ -1774,6 +1778,9 @@
 }
 case lltok::kw_inreg:   B.addAttribute(Attribute::InReg); break;
 case lltok::kw_noalias: B.addAttribute(Attribute::NoAlias); break;
+case lltok::kw_noundef:
+  B.addAttribute(Attribute::NoUndef);
+  break;
 case lltok::kw_nonnull: B.addAttribute(Attribute::NonNull); break;
 case lltok::kw_signext: B.addAttribute(Attribute::SExt); break;
 case lltok::kw_zeroext: B.addAttribute(Attribute::ZExt); break;
Index: llvm/lib/AsmParser/LLLexer.cpp
===
--- llvm/lib/AsmParser/LLLexer.cpp
+++ llvm/lib/AsmParser/LLLexer.cpp
@@ -696,6 +696,7 @@
   KEYWORD(writeonly);
   KEYWORD(zeroext);
   KEYWORD(immarg);
+  KEYWORD(noundef);
 
   KEYWORD(type);
   KEYWORD(opaque);
Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -39,6 +39,9 @@
 /// Pass structure by value.
 def ByVal : 

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: llvm/lib/Transforms/Utils/CodeExtractor.cpp:932
   case Attribute::NoCfCheck:
+  case Attribute::NoUndef:
 break;

nikic wrote:
> Not familiar with this code, but based on the placement of other similar 
> attributes like nonnull, this should probably be in the first list.
Technically, it should not matter because this is not a function attribute. 
That said, the first list makes logically more sense. I would even prefer three 
lists, OK to propagate, not OK, and assertion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand added a comment.

I've run across an issue when bootstrapping clang, that manifests itself in the 
following code when compiled with optimizations:

  #include 
  
  float my_exp2(float a) {
  return pow(2.0, a);
  }

With this code, the call to `pow` is lifted to a `exp2`, and then the resulting 
call causes `llvm::verifyFunction` to fail. The error is that attributes are 
present past the end of the argument list.
I'm thinking that whatever code does the lifting here fails to remove the 
attribute list corresponding to the `2.0` parameter here.

Could anyone point me to where I might need to make a change for that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

In D81678#2120709 , @guiand wrote:

> Could anyone point me to where I might need to make a change for that?


LibCallSimplifier::optimizePow


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2082
+  const Type *RetTyPtr = RetTy.getTypePtr();
+  if (!RetTy->isVoidType() && !RetTyPtr->isRecordType() &&
+  RetAI.getKind() != ABIArgInfo::Indirect) {

guiand wrote:
> guiand wrote:
> > guiand wrote:
> > > rsmith wrote:
> > > > Other types that we should think about from a padding perspective:
> > > > 
> > > >  * `nullptr_t` (which has the same size and alignment as `void*` but is 
> > > > all padding)
> > > >  * 80-bit `long double` and `_Complex long double` (I don't know 
> > > > whether we guarantee to initialize the 6 padding bytes)
> > > >  * member pointers might contain padding under some ABI rules -- under 
> > > > the MS ABI, you get a struct containing N pointers followed by M ints, 
> > > > which could have 4 bytes of tail padding on 64-bit targets
> > > >  * vector types with tail padding (eg, a vector of 3 `char`s is 
> > > > sometimes passed as an `i32` with one byte `undef`)
> > > >  * matrix types (presumably the same concerns as for vector types apply 
> > > > here)
> > > >  * maybe Obj-C object types?
> > > >  * `_ExtInt` types (eg, returning an `_ExtInt(65)` initialized to 0 
> > > > produces an `{i64, i64}` containing 7 bytes of `undef`)
> > > > 
> > > > It would be safer to list exactly those types for which we know this 
> > > > assumption is correct rather than assuming that it's correct by default 
> > > > -- I think more than half of the `Type` subclasses for concrete 
> > > > canonical types can have undef bits in their IR representation.
> > > This is a good point, and I think there was some nuance that I had 
> > > previously included in determining this for records (since removed) that 
> > > I didn't think to include in CGCall.cpp.
> > > 
> > > I think one particular check, 
> > > `llvm::DataLayout::typeSizeEqualsStoreSize`, handles a lot of these cases:
> > > - `long double`
> > > - oddly-sized vectors
> > > - `_ExtInt` types
> > > 
> > > I don't know if the `matrix` type has internal padding (like structs) but 
> > > if not it would cover that as well.  
> > > I believe that the struct with padding wouldn't be an issue as we're 
> > > excluding record types here.  
> > > And I'd have to take a closer look at `nullptr_t` to see how it behaves 
> > > here.
> > > 
> > > ~~~
> > > 
> > > But if I'd like to build an exhaustive list of types I think will work 
> > > correctly with a check like this:
> > > - scalars
> > > - floating point numbers
> > > - pointers
> > > 
> > > I think I'd need also need an inner `typeSizeEqualsStoreSize` check on 
> > > the base type of vectors/complex numbers.
> > > nullptr_t (which has the same size and alignment as void* but is all 
> > > padding)
> > 
> > From what I can gather looking at the IR of `test/CodeGenCXX/nullptr.cpp`, 
> > it looks like `nullptr_t` arguments and return values are represented as 
> > normal `i8*` types but given the special value `null`. From this it seems 
> > like we can mark them `noundef` without worry?
> Actually I've been thinking more about these cases, and they should only 
> really be a problem if clang coerces them away from their native types to 
> something larger.
> 
> For example, if clang emits `i33` that's not a problem, as potential padding 
> belonging to `i33` can be always be determined by looking at the data layout.
> But if it emits `{ i32, i32 }` or `i64` or something similar, it's 
> introducing new hidden/erased padding and so we can't mark it `noundef`.
> Same thing goes for `long double`: if it's emitted as `x86_fp80` that's no 
> problem.
> 
> There's been some confusion wrt this in the LangRef patch as well. I think 
> what we really want is an attribute that indicates the presence of undef bits 
> *not inherent to the IR type*. So (for the sake of argument, we're not 
> handling structs right now) `struct { char; short }` emitted as `{ i8, i16 } 
> noundef` is fine, but emitting it as `i64` is a problem.
> struct { char; short } emitted as { i8, i16 } noundef is fine, but emitting 
> it as i64 is a problem.

FWIW, I agree with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Nikita Popov via Phabricator via cfe-commits]

nikic added inline comments.



Comment at: llvm/lib/Transforms/Utils/CodeExtractor.cpp:932
   case Attribute::NoCfCheck:
+  case Attribute::NoUndef:
 break;

Not familiar with this code, but based on the placement of other similar 
attributes like nonnull, this should probably be in the first list.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 273558.
guiand added a comment.

Made DetermineNoUndef more robust


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.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
@@ -929,6 +929,7 @@
   case Attribute::StrictFP:
   case Attribute::UWTable:
   case Attribute::NoCfCheck:
+  case Attribute::NoUndef:
 break;
   }
 
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -443,6 +443,8 @@
 return "cold";
   if (hasAttribute(Attribute::ImmArg))
 return "immarg";
+  if (hasAttribute(Attribute::NoUndef))
+return "noundef";
 
   if (hasAttribute(Attribute::ByVal)) {
 std::string Result;
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -731,6 +731,8 @@
 return bitc::ATTR_KIND_SANITIZE_MEMTAG;
   case Attribute::Preallocated:
 return bitc::ATTR_KIND_PREALLOCATED;
+  case Attribute::NoUndef:
+return bitc::ATTR_KIND_NOUNDEF;
   case Attribute::EndAttrKinds:
 llvm_unreachable("Can not encode end-attribute kinds marker.");
   case Attribute::None:
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1532,6 +1532,8 @@
 return Attribute::SanitizeMemTag;
   case bitc::ATTR_KIND_PREALLOCATED:
 return Attribute::Preallocated;
+  case bitc::ATTR_KIND_NOUNDEF:
+return Attribute::NoUndef;
   }
 }
 
Index: llvm/lib/AsmParser/LLToken.h
===
--- llvm/lib/AsmParser/LLToken.h
+++ llvm/lib/AsmParser/LLToken.h
@@ -196,6 +196,7 @@
   kw_naked,
   kw_nest,
   kw_noalias,
+  kw_noundef,
   kw_nobuiltin,
   kw_nocapture,
   kw_noduplicate,
Index: llvm/lib/AsmParser/LLParser.cpp
===
--- llvm/lib/AsmParser/LLParser.cpp
+++ llvm/lib/AsmParser/LLParser.cpp
@@ -1374,6 +1374,7 @@
 case lltok::kw_inalloca:
 case lltok::kw_nest:
 case lltok::kw_noalias:
+case lltok::kw_noundef:
 case lltok::kw_nocapture:
 case lltok::kw_nonnull:
 case lltok::kw_returned:
@@ -1677,6 +1678,9 @@
 case lltok::kw_inalloca:B.addAttribute(Attribute::InAlloca); break;
 case lltok::kw_inreg:   B.addAttribute(Attribute::InReg); break;
 case lltok::kw_nest:B.addAttribute(Attribute::Nest); break;
+case lltok::kw_noundef:
+  B.addAttribute(Attribute::NoUndef);
+  break;
 case lltok::kw_noalias: B.addAttribute(Attribute::NoAlias); break;
 case lltok::kw_nocapture:   B.addAttribute(Attribute::NoCapture); break;
 case lltok::kw_nofree:  B.addAttribute(Attribute::NoFree); break;
@@ -1774,6 +1778,9 @@
 }
 case lltok::kw_inreg:   B.addAttribute(Attribute::InReg); break;
 case lltok::kw_noalias: B.addAttribute(Attribute::NoAlias); break;
+case lltok::kw_noundef:
+  B.addAttribute(Attribute::NoUndef);
+  break;
 case lltok::kw_nonnull: B.addAttribute(Attribute::NonNull); break;
 case lltok::kw_signext: B.addAttribute(Attribute::SExt); break;
 case lltok::kw_zeroext: B.addAttribute(Attribute::ZExt); break;
Index: llvm/lib/AsmParser/LLLexer.cpp
===
--- llvm/lib/AsmParser/LLLexer.cpp
+++ llvm/lib/AsmParser/LLLexer.cpp
@@ -696,6 +696,7 @@
   KEYWORD(writeonly);
   KEYWORD(zeroext);
   KEYWORD(immarg);
+  KEYWORD(noundef);
 
   KEYWORD(type);
   KEYWORD(opaque);
Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -39,6 +39,9 @@
 /// Pass structure by value.
 def ByVal : TypeAttr<"byval">;
 
+/// Parameter or return value may not contain uninitialized or poison bits
+def NoUndef : EnumAttr<"no

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand marked an inline comment as done.
guiand added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2082
+  const Type *RetTyPtr = RetTy.getTypePtr();
+  if (!RetTy->isVoidType() && !RetTyPtr->isRecordType() &&
+  RetAI.getKind() != ABIArgInfo::Indirect) {

guiand wrote:
> guiand wrote:
> > rsmith wrote:
> > > Other types that we should think about from a padding perspective:
> > > 
> > >  * `nullptr_t` (which has the same size and alignment as `void*` but is 
> > > all padding)
> > >  * 80-bit `long double` and `_Complex long double` (I don't know whether 
> > > we guarantee to initialize the 6 padding bytes)
> > >  * member pointers might contain padding under some ABI rules -- under 
> > > the MS ABI, you get a struct containing N pointers followed by M ints, 
> > > which could have 4 bytes of tail padding on 64-bit targets
> > >  * vector types with tail padding (eg, a vector of 3 `char`s is sometimes 
> > > passed as an `i32` with one byte `undef`)
> > >  * matrix types (presumably the same concerns as for vector types apply 
> > > here)
> > >  * maybe Obj-C object types?
> > >  * `_ExtInt` types (eg, returning an `_ExtInt(65)` initialized to 0 
> > > produces an `{i64, i64}` containing 7 bytes of `undef`)
> > > 
> > > It would be safer to list exactly those types for which we know this 
> > > assumption is correct rather than assuming that it's correct by default 
> > > -- I think more than half of the `Type` subclasses for concrete canonical 
> > > types can have undef bits in their IR representation.
> > This is a good point, and I think there was some nuance that I had 
> > previously included in determining this for records (since removed) that I 
> > didn't think to include in CGCall.cpp.
> > 
> > I think one particular check, `llvm::DataLayout::typeSizeEqualsStoreSize`, 
> > handles a lot of these cases:
> > - `long double`
> > - oddly-sized vectors
> > - `_ExtInt` types
> > 
> > I don't know if the `matrix` type has internal padding (like structs) but 
> > if not it would cover that as well.  
> > I believe that the struct with padding wouldn't be an issue as we're 
> > excluding record types here.  
> > And I'd have to take a closer look at `nullptr_t` to see how it behaves 
> > here.
> > 
> > ~~~
> > 
> > But if I'd like to build an exhaustive list of types I think will work 
> > correctly with a check like this:
> > - scalars
> > - floating point numbers
> > - pointers
> > 
> > I think I'd need also need an inner `typeSizeEqualsStoreSize` check on the 
> > base type of vectors/complex numbers.
> > nullptr_t (which has the same size and alignment as void* but is all 
> > padding)
> 
> From what I can gather looking at the IR of `test/CodeGenCXX/nullptr.cpp`, it 
> looks like `nullptr_t` arguments and return values are represented as normal 
> `i8*` types but given the special value `null`. From this it seems like we 
> can mark them `noundef` without worry?
Actually I've been thinking more about these cases, and they should only really 
be a problem if clang coerces them away from their native types to something 
larger.

For example, if clang emits `i33` that's not a problem, as potential padding 
belonging to `i33` can be always be determined by looking at the data layout.
But if it emits `{ i32, i32 }` or `i64` or something similar, it's introducing 
new hidden/erased padding and so we can't mark it `noundef`.
Same thing goes for `long double`: if it's emitted as `x86_fp80` that's no 
problem.

There's been some confusion wrt this in the LangRef patch as well. I think what 
we really want is an attribute that indicates the presence of undef bits *not 
inherent to the IR type*. So (for the sake of argument, we're not handling 
structs right now) `struct { char; short }` emitted as `{ i8, i16 } noundef` is 
fine, but emitting it as `i64` is a problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 273456.
guiand retitled this revision from "Introduce frozen attribute at call sites 
for stricter poison analysis" to "Introduce noundef attribute at call sites for 
stricter poison analysis".
guiand edited the summary of this revision.
guiand added a comment.

Renamed to noundef, added additional checks before determining attribute


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.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
@@ -929,6 +929,7 @@
   case Attribute::StrictFP:
   case Attribute::UWTable:
   case Attribute::NoCfCheck:
+  case Attribute::NoUndef:
 break;
   }
 
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -443,6 +443,8 @@
 return "cold";
   if (hasAttribute(Attribute::ImmArg))
 return "immarg";
+  if (hasAttribute(Attribute::NoUndef))
+return "noundef";
 
   if (hasAttribute(Attribute::ByVal)) {
 std::string Result;
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -731,6 +731,8 @@
 return bitc::ATTR_KIND_SANITIZE_MEMTAG;
   case Attribute::Preallocated:
 return bitc::ATTR_KIND_PREALLOCATED;
+  case Attribute::NoUndef:
+return bitc::ATTR_KIND_NOUNDEF;
   case Attribute::EndAttrKinds:
 llvm_unreachable("Can not encode end-attribute kinds marker.");
   case Attribute::None:
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1532,6 +1532,8 @@
 return Attribute::SanitizeMemTag;
   case bitc::ATTR_KIND_PREALLOCATED:
 return Attribute::Preallocated;
+  case bitc::ATTR_KIND_NOUNDEF:
+return Attribute::NoUndef;
   }
 }
 
Index: llvm/lib/AsmParser/LLToken.h
===
--- llvm/lib/AsmParser/LLToken.h
+++ llvm/lib/AsmParser/LLToken.h
@@ -196,6 +196,7 @@
   kw_naked,
   kw_nest,
   kw_noalias,
+  kw_noundef,
   kw_nobuiltin,
   kw_nocapture,
   kw_noduplicate,
Index: llvm/lib/AsmParser/LLParser.cpp
===
--- llvm/lib/AsmParser/LLParser.cpp
+++ llvm/lib/AsmParser/LLParser.cpp
@@ -1374,6 +1374,7 @@
 case lltok::kw_inalloca:
 case lltok::kw_nest:
 case lltok::kw_noalias:
+case lltok::kw_noundef:
 case lltok::kw_nocapture:
 case lltok::kw_nonnull:
 case lltok::kw_returned:
@@ -1677,6 +1678,9 @@
 case lltok::kw_inalloca:B.addAttribute(Attribute::InAlloca); break;
 case lltok::kw_inreg:   B.addAttribute(Attribute::InReg); break;
 case lltok::kw_nest:B.addAttribute(Attribute::Nest); break;
+case lltok::kw_noundef:
+  B.addAttribute(Attribute::NoUndef);
+  break;
 case lltok::kw_noalias: B.addAttribute(Attribute::NoAlias); break;
 case lltok::kw_nocapture:   B.addAttribute(Attribute::NoCapture); break;
 case lltok::kw_nofree:  B.addAttribute(Attribute::NoFree); break;
@@ -1774,6 +1778,9 @@
 }
 case lltok::kw_inreg:   B.addAttribute(Attribute::InReg); break;
 case lltok::kw_noalias: B.addAttribute(Attribute::NoAlias); break;
+case lltok::kw_noundef:
+  B.addAttribute(Attribute::NoUndef);
+  break;
 case lltok::kw_nonnull: B.addAttribute(Attribute::NonNull); break;
 case lltok::kw_signext: B.addAttribute(Attribute::SExt); break;
 case lltok::kw_zeroext: B.addAttribute(Attribute::ZExt); break;
Index: llvm/lib/AsmParser/LLLexer.cpp
===
--- llvm/lib/AsmParser/LLLexer.cpp
+++ llvm/lib/AsmParser/LLLexer.cpp
@@ -696,6 +696,7 @@
   KEYWORD(writeonly);
   KEYWORD(zeroext);
   KEYWORD(immarg);
+  KEYWORD(noundef);
 
   KEYWORD(type);
   KEYWORD(opaque);
Index: llvm/include/llvm/IR/Attributes.td
==

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand marked an inline comment as done.
guiand added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2082
+  const Type *RetTyPtr = RetTy.getTypePtr();
+  if (!RetTy->isVoidType() && !RetTyPtr->isRecordType() &&
+  RetAI.getKind() != ABIArgInfo::Indirect) {

guiand wrote:
> rsmith wrote:
> > Other types that we should think about from a padding perspective:
> > 
> >  * `nullptr_t` (which has the same size and alignment as `void*` but is all 
> > padding)
> >  * 80-bit `long double` and `_Complex long double` (I don't know whether we 
> > guarantee to initialize the 6 padding bytes)
> >  * member pointers might contain padding under some ABI rules -- under the 
> > MS ABI, you get a struct containing N pointers followed by M ints, which 
> > could have 4 bytes of tail padding on 64-bit targets
> >  * vector types with tail padding (eg, a vector of 3 `char`s is sometimes 
> > passed as an `i32` with one byte `undef`)
> >  * matrix types (presumably the same concerns as for vector types apply 
> > here)
> >  * maybe Obj-C object types?
> >  * `_ExtInt` types (eg, returning an `_ExtInt(65)` initialized to 0 
> > produces an `{i64, i64}` containing 7 bytes of `undef`)
> > 
> > It would be safer to list exactly those types for which we know this 
> > assumption is correct rather than assuming that it's correct by default -- 
> > I think more than half of the `Type` subclasses for concrete canonical 
> > types can have undef bits in their IR representation.
> This is a good point, and I think there was some nuance that I had previously 
> included in determining this for records (since removed) that I didn't think 
> to include in CGCall.cpp.
> 
> I think one particular check, `llvm::DataLayout::typeSizeEqualsStoreSize`, 
> handles a lot of these cases:
> - `long double`
> - oddly-sized vectors
> - `_ExtInt` types
> 
> I don't know if the `matrix` type has internal padding (like structs) but if 
> not it would cover that as well.  
> I believe that the struct with padding wouldn't be an issue as we're 
> excluding record types here.  
> And I'd have to take a closer look at `nullptr_t` to see how it behaves here.
> 
> ~~~
> 
> But if I'd like to build an exhaustive list of types I think will work 
> correctly with a check like this:
> - scalars
> - floating point numbers
> - pointers
> 
> I think I'd need also need an inner `typeSizeEqualsStoreSize` check on the 
> base type of vectors/complex numbers.
> nullptr_t (which has the same size and alignment as void* but is all padding)

From what I can gather looking at the IR of `test/CodeGenCXX/nullptr.cpp`, it 
looks like `nullptr_t` arguments and return values are represented as normal 
`i8*` types but given the special value `null`. From this it seems like we can 
mark them `noundef` without worry?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678



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