[PATCH] D139570: Add -Wreturn-local-addr, GCC alias for -Wreturn-stack-address

[Adrian Dole via Phabricator via cfe-commits]

adriandole updated this revision to Diff 486980.
adriandole added a comment.

Add test and release note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139570

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/test/SemaCXX/return-stack-addr-2.cpp


Index: clang/test/SemaCXX/return-stack-addr-2.cpp
===
--- clang/test/SemaCXX/return-stack-addr-2.cpp
+++ clang/test/SemaCXX/return-stack-addr-2.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -std=c++11 
-Wno-return-stack-address -Wreturn-local-addr %s
 
 namespace PR26599 {
 template 
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -413,6 +413,8 @@
 def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
 def DanglingGsl : DiagGroup<"dangling-gsl">;
 def ReturnStackAddress : DiagGroup<"return-stack-address">;
+// Name of this warning in GCC
+def : DiagGroup<"return-local-addr", [ReturnStackAddress]>;
 def Dangling : DiagGroup<"dangling", [DanglingField,
   DanglingInitializerList,
   DanglingGsl,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -439,6 +439,7 @@
   ``#pragma clang __debug sloc_usage`` can also be used to request this report.
 - Clang no longer permits the keyword 'bool' in a concept declaration as a
   concepts-ts compatibility extension.
+- Add ``-Wreturn-local-addr``, a GCC alias for ``-Wreturn-stack-address``.
 
 Non-comprehensive list of changes in this release
 -


Index: clang/test/SemaCXX/return-stack-addr-2.cpp
===
--- clang/test/SemaCXX/return-stack-addr-2.cpp
+++ clang/test/SemaCXX/return-stack-addr-2.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -std=c++11 -Wno-return-stack-address -Wreturn-local-addr %s
 
 namespace PR26599 {
 template 
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -413,6 +413,8 @@
 def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
 def DanglingGsl : DiagGroup<"dangling-gsl">;
 def ReturnStackAddress : DiagGroup<"return-stack-address">;
+// Name of this warning in GCC
+def : DiagGroup<"return-local-addr", [ReturnStackAddress]>;
 def Dangling : DiagGroup<"dangling", [DanglingField,
   DanglingInitializerList,
   DanglingGsl,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -439,6 +439,7 @@
   ``#pragma clang __debug sloc_usage`` can also be used to request this report.
 - Clang no longer permits the keyword 'bool' in a concept declaration as a
   concepts-ts compatibility extension.
+- Add ``-Wreturn-local-addr``, a GCC alias for ``-Wreturn-stack-address``.
 
 Non-comprehensive list of changes in this release
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141310: [clang] add -Wcompare-function-pointers

[Adrian Dole via Phabricator via cfe-commits]

adriandole created this revision.
Herald added a project: All.
adriandole requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Using `icf=all` with lld can cause comparisons between function pointers that 
previously compared as unequal to compare as equal. This warning is disabled by 
default, since it's only relevant if ICF is explicitly enabled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141310

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/compare-function-pointer.cpp


Index: clang/test/SemaCXX/compare-function-pointer.cpp
===
--- clang/test/SemaCXX/compare-function-pointer.cpp
+++ clang/test/SemaCXX/compare-function-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify 
-Wcompare-function-pointers %s
 
 using fp0_t = void (*)();
 using fp1_t = int (*)();
@@ -6,16 +6,16 @@
 extern fp0_t a, b;
 extern fp1_t c;
 
-bool eq0 = a == b;
-bool ne0 = a != b;
+bool eq0 = a == b;  // expected-warning {{comparison of function pointers}}
+bool ne0 = a != b;  // expected-warning {{comparison of function pointers}}
 bool lt0 = a < b;   // expected-warning {{ordered comparison of function 
pointers ('fp0_t' (aka 'void (*)()') and 'fp0_t')}}
 bool le0 = a <= b;  // expected-warning {{ordered comparison of function 
pointers}}
 bool gt0 = a > b;   // expected-warning {{ordered comparison of function 
pointers}}
 bool ge0 = a >= b;  // expected-warning {{ordered comparison of function 
pointers}}
 auto tw0 = a <=> b; // expected-error {{ordered comparison of function 
pointers}}
 
-bool eq1 = a == c;  // expected-error {{comparison of distinct pointer types}}
-bool ne1 = a != c;  // expected-error {{comparison of distinct pointer types}}
+bool eq1 = a == c;  // expected-error {{comparison of distinct pointer types}} 
expected-warning {{comparison of function pointers}}
+bool ne1 = a != c;  // expected-error {{comparison of distinct pointer types}} 
expected-warning {{comparison of function pointers}}
 bool lt1 = a < c;   // expected-warning {{ordered comparison of function 
pointers ('fp0_t' (aka 'void (*)()') and 'fp1_t' (aka 'int (*)()'))}}
 // expected-error@-1 {{comparison of distinct pointer 
types}}
 bool le1 = a <= c;  // expected-warning {{ordered comparison of function 
pointers}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -12711,6 +12711,13 @@
LHS.get()->getSourceRange());
   }
 
+  if (!IsOrdered && LHSType->isFunctionPointerType() &&
+  RHSType->isFunctionPointerType() && !LHSIsNull && !RHSIsNull) {
+Diag(Loc, diag::warn_typecheck_comparison_of_function_pointers)
+  << LHSType << 0
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+  }
+
   if (IsOrdered && LHSType->isFunctionPointerType() &&
   RHSType->isFunctionPointerType()) {
 // Valid unless a relational comparison of function pointers
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7002,6 +7002,9 @@
   "%0 is %select{|in}2complete and "
   "%1 is %select{|in}3complete">,
   InGroup;
+def warn_typecheck_comparison_of_function_pointers : Warning<
+  "comparison of function pointers (%0 and %1)">,
+  InGroup, DefaultIgnore;
 def warn_typecheck_ordered_comparison_of_function_pointers : Warning<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -561,6 +561,7 @@
 def UnderalignedExceptionObject : DiagGroup<"underaligned-exception-object">;
 def DeprecatedObjCIsaUsage : DiagGroup<"deprecated-objc-isa-usage">;
 def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;
+def CompareFunctionPointers : DiagGroup<"compare-function-pointers">;
 def OrderedCompareFunctionPointers : 
DiagGroup<"ordered-compare-function-pointers">;
 def PackedNonPod : DiagGroup<"packed-non-pod">;
 def Packed : DiagGroup<"packed", [PackedNonPod]>;


Index: clang/test/SemaCXX/compare-function-pointer.cpp
===
--- clang/test/SemaCXX/compare-function-pointer.cpp
+++ clang/test/SemaCXX/compare-function-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -

[PATCH] D141310: [clang] add -Wcompare-function-pointers

[Adrian Dole via Phabricator via cfe-commits]

adriandole updated this revision to Diff 487511.
adriandole added a comment.

Release note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141310

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/compare-function-pointer.cpp


Index: clang/test/SemaCXX/compare-function-pointer.cpp
===
--- clang/test/SemaCXX/compare-function-pointer.cpp
+++ clang/test/SemaCXX/compare-function-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify 
-Wcompare-function-pointers %s
 
 using fp0_t = void (*)();
 using fp1_t = int (*)();
@@ -6,16 +6,16 @@
 extern fp0_t a, b;
 extern fp1_t c;
 
-bool eq0 = a == b;
-bool ne0 = a != b;
+bool eq0 = a == b;  // expected-warning {{comparison of function pointers}}
+bool ne0 = a != b;  // expected-warning {{comparison of function pointers}}
 bool lt0 = a < b;   // expected-warning {{ordered comparison of function 
pointers ('fp0_t' (aka 'void (*)()') and 'fp0_t')}}
 bool le0 = a <= b;  // expected-warning {{ordered comparison of function 
pointers}}
 bool gt0 = a > b;   // expected-warning {{ordered comparison of function 
pointers}}
 bool ge0 = a >= b;  // expected-warning {{ordered comparison of function 
pointers}}
 auto tw0 = a <=> b; // expected-error {{ordered comparison of function 
pointers}}
 
-bool eq1 = a == c;  // expected-error {{comparison of distinct pointer types}}
-bool ne1 = a != c;  // expected-error {{comparison of distinct pointer types}}
+bool eq1 = a == c;  // expected-error {{comparison of distinct pointer types}} 
expected-warning {{comparison of function pointers}}
+bool ne1 = a != c;  // expected-error {{comparison of distinct pointer types}} 
expected-warning {{comparison of function pointers}}
 bool lt1 = a < c;   // expected-warning {{ordered comparison of function 
pointers ('fp0_t' (aka 'void (*)()') and 'fp1_t' (aka 'int (*)()'))}}
 // expected-error@-1 {{comparison of distinct pointer 
types}}
 bool le1 = a <= c;  // expected-warning {{ordered comparison of function 
pointers}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -12711,6 +12711,13 @@
LHS.get()->getSourceRange());
   }
 
+  if (!IsOrdered && LHSType->isFunctionPointerType() &&
+  RHSType->isFunctionPointerType() && !LHSIsNull && !RHSIsNull) {
+Diag(Loc, diag::warn_typecheck_comparison_of_function_pointers)
+  << LHSType << 0
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+  }
+
   if (IsOrdered && LHSType->isFunctionPointerType() &&
   RHSType->isFunctionPointerType()) {
 // Valid unless a relational comparison of function pointers
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7002,6 +7002,9 @@
   "%0 is %select{|in}2complete and "
   "%1 is %select{|in}3complete">,
   InGroup;
+def warn_typecheck_comparison_of_function_pointers : Warning<
+  "comparison of function pointers (%0 and %1)">,
+  InGroup, DefaultIgnore;
 def warn_typecheck_ordered_comparison_of_function_pointers : Warning<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -561,6 +561,7 @@
 def UnderalignedExceptionObject : DiagGroup<"underaligned-exception-object">;
 def DeprecatedObjCIsaUsage : DiagGroup<"deprecated-objc-isa-usage">;
 def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;
+def CompareFunctionPointers : DiagGroup<"compare-function-pointers">;
 def OrderedCompareFunctionPointers : 
DiagGroup<"ordered-compare-function-pointers">;
 def PackedNonPod : DiagGroup<"packed-non-pod">;
 def Packed : DiagGroup<"packed", [PackedNonPod]>;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -447,6 +447,8 @@
   ``#pragma clang __debug sloc_usage`` can also be used to request this report.
 - Clang no longer permits the keyword 'bool' in a concept declaration as a
   concepts-ts compatibility extension.
+- Add ``-Wcompare-function-pointers`` to warn about comparisons that may have
+  their behavior change when using ``icf=all``.
 
 Non-comprehensive list of changes in this release
 -

[PATCH] D141310: [clang] add -Wcompare-function-pointers

[Adrian Dole via Phabricator via cfe-commits]

adriandole added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7006
+def warn_typecheck_comparison_of_function_pointers : Warning<
+  "comparison of function pointers (%0 and %1)">,
+  InGroup, DefaultIgnore;

aaron.ballman wrote:
> cjdb wrote:
> > This diagnostic feels very bare bones, and doesn't explain why the user 
> > should care. It would be good to rephrase the message so that it can 
> > incorporate some kind of reason too.
> Agreed -- diagnostic wording is usually trying to tell the user what they did 
> wrong to guide them how to fix it, but this reads more like the diagnostic 
> explains what the code does. This one will be especially interesting because 
> people would naturally expect that comparison to work per the standard. And 
> if enabling ICF breaks that then ICF is non-conforming because it makes valid 
> code silently invalid. I think this is an ICF bug (it may still be worth 
> warning users about though because you can use newer Clang with an older lld 
> and hit the issue even if lld addresses this issue).
> 
> CC @lhames @sbc100 @ruiu in to try to scare up an lld code owner to see if 
> this is known and if there's something that can be done on the lld side to 
> not break valid code.
There's already an ICF option that doesn't break valid code: `-icf=safe`. Only 
if you explicitly decide that you don't care about the results of function 
pointer comparisons would you use `-icf=all`, which enables more code folding 
to be done than the safe option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141310

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

[PATCH] D141310: [clang] add -Wcompare-function-pointers

[Adrian Dole via Phabricator via cfe-commits]

adriandole added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7006
+def warn_typecheck_comparison_of_function_pointers : Warning<
+  "comparison of function pointers (%0 and %1)">,
+  InGroup, DefaultIgnore;

aaron.ballman wrote:
> adriandole wrote:
> > aaron.ballman wrote:
> > > cjdb wrote:
> > > > This diagnostic feels very bare bones, and doesn't explain why the user 
> > > > should care. It would be good to rephrase the message so that it can 
> > > > incorporate some kind of reason too.
> > > Agreed -- diagnostic wording is usually trying to tell the user what they 
> > > did wrong to guide them how to fix it, but this reads more like the 
> > > diagnostic explains what the code does. This one will be especially 
> > > interesting because people would naturally expect that comparison to work 
> > > per the standard. And if enabling ICF breaks that then ICF is 
> > > non-conforming because it makes valid code silently invalid. I think this 
> > > is an ICF bug (it may still be worth warning users about though because 
> > > you can use newer Clang with an older lld and hit the issue even if lld 
> > > addresses this issue).
> > > 
> > > CC @lhames @sbc100 @ruiu in to try to scare up an lld code owner to see 
> > > if this is known and if there's something that can be done on the lld 
> > > side to not break valid code.
> > There's already an ICF option that doesn't break valid code: `-icf=safe`. 
> > Only if you explicitly decide that you don't care about the results of 
> > function pointer comparisons would you use `-icf=all`, which enables more 
> > code folding to be done than the safe option.
> Ohhh interesting, so it's probably known that `-icf=all` will break code 
> because it's not conforming and thus isn't an lld bug at all. Do (most?) 
> other linkers also have the same functionality as `-icf=all`? I'm trying to 
> understand whether this should be added to clang at all as it seems like it's 
> a warning for a very particular usage pattern (a specific linker with a 
> specific option ), which make it more reasonable for clang-tidy instead of 
> the compiler.
ld.gold and mold have it (same name, `-icf=all`) as does MSVC (`/OPT:ICF`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141310

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

[PATCH] D139570: Add -Wreturn-local-addr, GCC alias for -Wreturn-stack-address

[Adrian Dole via Phabricator via cfe-commits]

adriandole updated this revision to Diff 488699.
adriandole added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139570

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/test/SemaCXX/return-stack-addr-2.cpp


Index: clang/test/SemaCXX/return-stack-addr-2.cpp
===
--- clang/test/SemaCXX/return-stack-addr-2.cpp
+++ clang/test/SemaCXX/return-stack-addr-2.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -std=c++11 
-Wno-return-stack-address -Wreturn-local-addr %s
 
 namespace PR26599 {
 template 
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -413,6 +413,8 @@
 def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
 def DanglingGsl : DiagGroup<"dangling-gsl">;
 def ReturnStackAddress : DiagGroup<"return-stack-address">;
+// Name of this warning in GCC
+def : DiagGroup<"return-local-addr", [ReturnStackAddress]>;
 def Dangling : DiagGroup<"dangling", [DanglingField,
   DanglingInitializerList,
   DanglingGsl,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -449,6 +449,7 @@
   concepts-ts compatibility extension.
 - Clang now diagnoses overflow undefined behavior in a constant expression 
while
   evaluating a compound assignment with remainder as operand.
+- Add ``-Wreturn-local-addr``, a GCC alias for ``-Wreturn-stack-address``.
 
 Non-comprehensive list of changes in this release
 -


Index: clang/test/SemaCXX/return-stack-addr-2.cpp
===
--- clang/test/SemaCXX/return-stack-addr-2.cpp
+++ clang/test/SemaCXX/return-stack-addr-2.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -std=c++11 -Wno-return-stack-address -Wreturn-local-addr %s
 
 namespace PR26599 {
 template 
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -413,6 +413,8 @@
 def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
 def DanglingGsl : DiagGroup<"dangling-gsl">;
 def ReturnStackAddress : DiagGroup<"return-stack-address">;
+// Name of this warning in GCC
+def : DiagGroup<"return-local-addr", [ReturnStackAddress]>;
 def Dangling : DiagGroup<"dangling", [DanglingField,
   DanglingInitializerList,
   DanglingGsl,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -449,6 +449,7 @@
   concepts-ts compatibility extension.
 - Clang now diagnoses overflow undefined behavior in a constant expression while
   evaluating a compound assignment with remainder as operand.
+- Add ``-Wreturn-local-addr``, a GCC alias for ``-Wreturn-stack-address``.
 
 Non-comprehensive list of changes in this release
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141310: [clang] add -Wcompare-function-pointers

[Adrian Dole via Phabricator via cfe-commits]

adriandole added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:565
+def CompareFunctionPointers : DiagGroup<"compare-function-pointers">;
 def OrderedCompareFunctionPointers : 
DiagGroup<"ordered-compare-function-pointers">;
 def PackedNonPod : DiagGroup<"packed-non-pod">;

aaron.ballman wrote:
> Should `-Wcompare-function-pointers` enable 
> `-Wordered-compare-function-pointers`? I think I would normally assume such a 
> relationship as the ordered variant sounds like a subset of the unordered 
> variant.
Makes sense, and it also fixes the issue of raising both this warning and 
`-Wordered-compare-function-pointers` on the same line. Will update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141310

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

[PATCH] D141310: [clang] add -Wcompare-function-pointers

[Adrian Dole via Phabricator via cfe-commits]

adriandole updated this revision to Diff 489109.
adriandole added a comment.

- More helpful diagnostic message.
- Check variable and type names are printed in tests.
- Enable `-Wordered-function-pointer-comparison` when this warning is enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141310

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/compare-function-pointer.cpp


Index: clang/test/SemaCXX/compare-function-pointer.cpp
===
--- clang/test/SemaCXX/compare-function-pointer.cpp
+++ clang/test/SemaCXX/compare-function-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify 
-Wcompare-function-pointers %s
 
 using fp0_t = void (*)();
 using fp1_t = int (*)();
@@ -6,8 +6,8 @@
 extern fp0_t a, b;
 extern fp1_t c;
 
-bool eq0 = a == b;
-bool ne0 = a != b;
+bool eq0 = a == b;  // expected-warning {{distinct function pointers ('fp0_t' 
(aka 'void (*)()') and 'fp0_t')}}
+bool ne0 = a != b;  // expected-warning {{distinct function pointers}}
 bool lt0 = a < b;   // expected-warning {{ordered comparison of function 
pointers ('fp0_t' (aka 'void (*)()') and 'fp0_t')}}
 bool le0 = a <= b;  // expected-warning {{ordered comparison of function 
pointers}}
 bool gt0 = a > b;   // expected-warning {{ordered comparison of function 
pointers}}
@@ -15,7 +15,9 @@
 auto tw0 = a <=> b; // expected-error {{ordered comparison of function 
pointers}}
 
 bool eq1 = a == c;  // expected-error {{comparison of distinct pointer types}}
+// expected-warning@-1 {{distinct function pointers 
('fp0_t' (aka 'void (*)()') and 'fp1_t' (aka 'int (*)()'))}}
 bool ne1 = a != c;  // expected-error {{comparison of distinct pointer types}}
+// expected-warning@-1 {{distinct function pointers}}
 bool lt1 = a < c;   // expected-warning {{ordered comparison of function 
pointers ('fp0_t' (aka 'void (*)()') and 'fp1_t' (aka 'int (*)()'))}}
 // expected-error@-1 {{comparison of distinct pointer 
types}}
 bool le1 = a <= c;  // expected-warning {{ordered comparison of function 
pointers}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -12711,6 +12711,12 @@
LHS.get()->getSourceRange());
   }
 
+  if (!IsOrdered && LHSType->isFunctionPointerType() &&
+  RHSType->isFunctionPointerType() && !LHSIsNull && !RHSIsNull)
+Diag(Loc, diag::warn_typecheck_comparison_of_function_pointers)
+  << LHSType << RHSType
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+
   if (IsOrdered && LHSType->isFunctionPointerType() &&
   RHSType->isFunctionPointerType()) {
 // Valid unless a relational comparison of function pointers
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7009,6 +7009,10 @@
   "%0 is %select{|in}2complete and "
   "%1 is %select{|in}3complete">,
   InGroup;
+def warn_typecheck_comparison_of_function_pointers : Warning<
+  "distinct function pointers (%0 and %1) may compare equal "
+  "when using identical code folding">,
+  InGroup, DefaultIgnore;
 def warn_typecheck_ordered_comparison_of_function_pointers : Warning<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -564,6 +564,7 @@
 def DeprecatedObjCIsaUsage : DiagGroup<"deprecated-objc-isa-usage">;
 def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;
 def OrderedCompareFunctionPointers : 
DiagGroup<"ordered-compare-function-pointers">;
+def CompareFunctionPointers : DiagGroup<"compare-function-pointers", 
[OrderedCompareFunctionPointers]>;
 def PackedNonPod : DiagGroup<"packed-non-pod">;
 def Packed : DiagGroup<"packed", [PackedNonPod]>;
 def Padded : DiagGroup<"padded">;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -455,6 +455,8 @@
 - Add ``-Wreturn-local-addr``, a GCC alias for ``-Wreturn-stack-address``.
 - Clang now suppresses ``-Wlogical-op-parentheses`` on ``(x && a || b)`` and 
``(a || b && x)``
   only when ``x`` is a string literal.
+- Add ``-Wcompare-function-pointers`` to warn about comparisons that

[PATCH] D141310: [clang] add -Wcompare-function-pointers

[Adrian Dole via Phabricator via cfe-commits]

adriandole added a comment.

@dblaikie, we would use this warning in Chrome OS. We use `icf=all` and have 
encountered bugs caused by function pointer comparisons.

It's not that noisy compiling clang (eight hits). Working on testing it for 
Chrome OS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141310

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

[PATCH] D141310: [clang] add -Wcompare-function-pointers

[Adrian Dole via Phabricator via cfe-commits]

adriandole updated this revision to Diff 490671.
adriandole added a comment.

Only trigger this warning when comparing function pointers of the same type, 
since comparing distinct types is already an error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141310

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/compare-function-pointer.cpp


Index: clang/test/SemaCXX/compare-function-pointer.cpp
===
--- clang/test/SemaCXX/compare-function-pointer.cpp
+++ clang/test/SemaCXX/compare-function-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify 
-Wcompare-function-pointers %s
 
 using fp0_t = void (*)();
 using fp1_t = int (*)();
@@ -6,8 +6,8 @@
 extern fp0_t a, b;
 extern fp1_t c;
 
-bool eq0 = a == b;
-bool ne0 = a != b;
+bool eq0 = a == b;  // expected-warning {{distinct function pointers ('fp0_t' 
(aka 'void (*)()') and 'fp0_t')}}
+bool ne0 = a != b;  // expected-warning {{distinct function pointers}}
 bool lt0 = a < b;   // expected-warning {{ordered comparison of function 
pointers ('fp0_t' (aka 'void (*)()') and 'fp0_t')}}
 bool le0 = a <= b;  // expected-warning {{ordered comparison of function 
pointers}}
 bool gt0 = a > b;   // expected-warning {{ordered comparison of function 
pointers}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -12714,6 +12714,13 @@
LHS.get()->getSourceRange());
   }
 
+  if (!IsOrdered && LHSType->isFunctionPointerType() &&
+  RHSType->isFunctionPointerType() && !LHSIsNull && !RHSIsNull &&
+  RHSType == LHSType)
+Diag(Loc, diag::warn_typecheck_comparison_of_function_pointers)
+  << LHSType << RHSType
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+
   if (IsOrdered && LHSType->isFunctionPointerType() &&
   RHSType->isFunctionPointerType()) {
 // Valid unless a relational comparison of function pointers
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7010,6 +7010,10 @@
   "%0 is %select{|in}2complete and "
   "%1 is %select{|in}3complete">,
   InGroup;
+def warn_typecheck_comparison_of_function_pointers : Warning<
+  "distinct function pointers (%0 and %1) may compare equal "
+  "when using identical code folding">,
+  InGroup, DefaultIgnore;
 def warn_typecheck_ordered_comparison_of_function_pointers : Warning<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -565,6 +565,7 @@
 def DeprecatedObjCIsaUsage : DiagGroup<"deprecated-objc-isa-usage">;
 def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;
 def OrderedCompareFunctionPointers : 
DiagGroup<"ordered-compare-function-pointers">;
+def CompareFunctionPointers : DiagGroup<"compare-function-pointers", 
[OrderedCompareFunctionPointers]>;
 def PackedNonPod : DiagGroup<"packed-non-pod">;
 def Packed : DiagGroup<"packed", [PackedNonPod]>;
 def Padded : DiagGroup<"padded">;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -465,6 +465,8 @@
 - Clang now automatically adds ``[[clang::lifetimebound]]`` to the parameters 
of
   ``std::move, std::forward`` et al, this enables Clang to diagnose more cases
   where the returned reference outlives the object.
+- Add ``-Wcompare-function-pointers`` to warn about comparisons that may have 
their behavior
+  change when enabling the identical code folding optimization feature of some 
linkers.
 
 Non-comprehensive list of changes in this release
 -


Index: clang/test/SemaCXX/compare-function-pointer.cpp
===
--- clang/test/SemaCXX/compare-function-pointer.cpp
+++ clang/test/SemaCXX/compare-function-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -Wcompare-function-pointers %s
 
 using fp0_t = void (*)();
 using fp1_t = int (*)();
@@ -6,8 +6,8 @@
 extern fp0_t a, b;
 extern fp1_t c;
 
-bool eq0 = a == b;
-bool ne0 = a != b;
+bool eq0 = a == b;  // expected-warning

[PATCH] D139570: Add -Wreturn-local-addr, GCC alias for -Wreturn-stack-address

[Adrian Dole via Phabricator via cfe-commits]

adriandole created this revision.
Herald added a subscriber: jeroen.dobbelaere.
Herald added a project: All.
adriandole requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139570

Files:
  clang/include/clang/Basic/DiagnosticGroups.td


Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -413,6 +413,8 @@
 def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
 def DanglingGsl : DiagGroup<"dangling-gsl">;
 def ReturnStackAddress : DiagGroup<"return-stack-address">;
+// Name of this warning in GCC
+def : DiagGroup<"return-local-addr", [ReturnStackAddress]>;
 def Dangling : DiagGroup<"dangling", [DanglingField,
   DanglingInitializerList,
   DanglingGsl,


Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -413,6 +413,8 @@
 def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
 def DanglingGsl : DiagGroup<"dangling-gsl">;
 def ReturnStackAddress : DiagGroup<"return-stack-address">;
+// Name of this warning in GCC
+def : DiagGroup<"return-local-addr", [ReturnStackAddress]>;
 def Dangling : DiagGroup<"dangling", [DanglingField,
   DanglingInitializerList,
   DanglingGsl,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits