public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r13-6912] Don't emit -Wxor-used-as-pow on macro expansions [PR107002]
@ 2023-03-28 18:36 David Malcolm
  0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2023-03-28 18:36 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:22c3a6c3c118283dfef1b9928dd21110098679b7

commit r13-6912-g22c3a6c3c118283dfef1b9928dd21110098679b7
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Tue Mar 28 14:34:49 2023 -0400

    Don't emit -Wxor-used-as-pow on macro expansions [PR107002]
    
    PR c/107002 reports an assertion failure from deep inside the
    diagnostic_shows_locus when attempting to print fix-it hints relating
    to -Wxor-used-as-pow.  The case involves macro expansions with
    -ftrack-macro-expansion=0.
    
    It doesn't seem to make much sense to emit this warning for macro
    expansions, so this patch updates the warning not to (which seems
    to also be clang's behavior).  The patch also adds some bulletproofing
    to diagnostic-show-locus.cc to be more robust against such invalid
    fix-it hints.
    
    Doing so fixes the ICE.
    
    gcc/c-family/ChangeLog:
            PR c/107002
            * c-common.h (check_for_xor_used_as_pow): Add "rhs_loc" param.
            * c-warn.cc (check_for_xor_used_as_pow): Add "rhs_loc" param.
            Reject cases where involving macro expansions.
    
    gcc/c/ChangeLog:
            PR c/107002
            * c-typeck.cc (parser_build_binary_op): Update for new param of
            check_for_xor_used_as_pow.
    
    gcc/cp/ChangeLog:
            PR c/107002
            * parser.cc (cp_parser_binary_expression): Update for new param of
            check_for_xor_used_as_pow.
    
    gcc/ChangeLog:
            PR c/107002
            * diagnostic-show-locus.cc (column_range::column_range): Factor
            out assertion conditional into...
            (column_range::valid_p): ...this new function.
            (line_corrections::add_hint): Don't attempt to consolidate hints
            if it would lead to invalid column_range instances.
    
    gcc/testsuite/ChangeLog:
            PR c/107002
            * c-c++-common/Wxor-used-as-pow-1.c: Add macro test.
            * c-c++-common/Wxor-used-as-pow-pr107002-0.c: New test.
            * c-c++-common/Wxor-used-as-pow-pr107002-1.c: New test.
            * c-c++-common/Wxor-used-as-pow-pr107002-2.c: New test.
    
    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

Diff:
---
 gcc/c-family/c-common.h                            |  2 +-
 gcc/c-family/c-warn.cc                             | 23 ++++++++++++++++++----
 gcc/c/c-typeck.cc                                  |  2 +-
 gcc/cp/parser.cc                                   |  1 +
 gcc/diagnostic-show-locus.cc                       | 13 +++++++++---
 gcc/testsuite/c-c++-common/Wxor-used-as-pow-1.c    |  4 ++++
 .../c-c++-common/Wxor-used-as-pow-pr107002-0.c     |  9 +++++++++
 .../c-c++-common/Wxor-used-as-pow-pr107002-1.c     |  5 +++++
 .../c-c++-common/Wxor-used-as-pow-pr107002-2.c     |  5 +++++
 9 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index e128e3e7379..f96350b64af 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1524,7 +1524,7 @@ extern void warn_for_multistatement_macros (location_t, location_t,
 
 extern void check_for_xor_used_as_pow (location_t lhs_loc, tree lhs_val,
 				       location_t operator_loc,
-				       tree rhs_val);
+				       location_t rhs_loc, tree rhs_val);
 
 /* In c-attribs.cc.  */
 extern bool attribute_takes_identifier_p (const_tree);
diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
index ccbede95ea4..9ac43a1af6e 100644
--- a/gcc/c-family/c-warn.cc
+++ b/gcc/c-family/c-warn.cc
@@ -3835,14 +3835,15 @@ do_warn_array_compare (location_t location, tree_code code, tree op0, tree op1)
     }
 }
 
-/* Given LHS_VAL ^ RHS_VAL, where LHS_LOC is the location of the LHS and
-   OPERATOR_LOC is the location of the ^, complain with -Wxor-used-as-pow
-   if it looks like the user meant exponentiation rather than xor.  */
+/* Given LHS_VAL ^ RHS_VAL, where LHS_LOC is the location of the LHS,
+   OPERATOR_LOC is the location of the ^, and RHS_LOC the location of the
+   RHS, complain with -Wxor-used-as-pow if it looks like the user meant
+   exponentiation rather than xor.  */
 
 void
 check_for_xor_used_as_pow (location_t lhs_loc, tree lhs_val,
 			   location_t operator_loc,
-			   tree rhs_val)
+			   location_t rhs_loc, tree rhs_val)
 {
   /* Only complain if both args are non-negative integer constants that fit
      in uhwi.  */
@@ -3859,6 +3860,20 @@ check_for_xor_used_as_pow (location_t lhs_loc, tree lhs_val,
   binary_op_rich_location loc (operator_loc,
 			       lhs_val, rhs_val, false);
 
+  /* Reject cases where we don't have 3 distinct locations.
+     This can happen e.g. due to macro expansion with
+     -ftrack-macro-expansion=0 */
+  if (!(lhs_loc != operator_loc
+	&& lhs_loc != rhs_loc
+	&& operator_loc != rhs_loc))
+    return;
+
+  /* Reject cases in which any of the locations came from a macro.  */
+  if (from_macro_expansion_at (lhs_loc)
+      || from_macro_expansion_at (operator_loc)
+      || from_macro_expansion_at (rhs_loc))
+    return;
+
   /* If we issue fix-it hints with the warning then we will also issue a
      note suggesting how to suppress the warning with a different change.
      These proposed changes are incompatible.  */
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 45bacc06c47..7079d4ee145 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -4083,7 +4083,7 @@ parser_build_binary_op (location_t location, enum tree_code code,
       && arg2.m_decimal)
     check_for_xor_used_as_pow (arg1.get_location (), arg1.value,
 			       location,
-			       arg2.value);
+			       arg2.get_location (), arg2.value);
 
   return result;
 }
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index c54972be85a..e8efc32f2c2 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -10289,6 +10289,7 @@ cp_parser_binary_expression (cp_parser* parser, bool cast_p,
 	  (current.lhs.get_location (),
 	   tree_strip_any_location_wrapper (current.lhs),
 	   current.loc,
+	   rhs.get_location (),
 	   tree_strip_any_location_wrapper (rhs));
 
       overload = NULL;
diff --git a/gcc/diagnostic-show-locus.cc b/gcc/diagnostic-show-locus.cc
index 301102779b9..0514815b51f 100644
--- a/gcc/diagnostic-show-locus.cc
+++ b/gcc/diagnostic-show-locus.cc
@@ -2232,8 +2232,7 @@ class column_range
 public:
   column_range (int start_, int finish_) : start (start_), finish (finish_)
   {
-    /* We must have either a range, or an insertion.  */
-    gcc_assert (start <= finish || finish == start - 1);
+    gcc_assert (valid_p (start, finish));
   }
 
   bool operator== (const column_range &other) const
@@ -2241,6 +2240,12 @@ public:
     return start == other.start && finish == other.finish;
   }
 
+  static bool valid_p (int start, int finish)
+  {
+    /* We must have either a range, or an insertion.  */
+    return (start <= finish || finish == start - 1);
+  }
+
   int start;
   int finish;
 };
@@ -2470,7 +2475,9 @@ line_corrections::add_hint (const fixit_hint *hint)
       gcc_assert (printed_columns.start
 		  >= last_correction->m_printed_columns.start);
 
-      if (printed_columns.start <= last_correction->m_printed_columns.finish)
+      if (printed_columns.start <= last_correction->m_printed_columns.finish
+	  && column_range::valid_p (last_correction->m_affected_bytes.finish + 1,
+				    affected_bytes.start - 1))
 	{
 	  /* We have two hints for which the printed forms of the hints
 	     would touch or overlap, so we need to consolidate them to avoid
diff --git a/gcc/testsuite/c-c++-common/Wxor-used-as-pow-1.c b/gcc/testsuite/c-c++-common/Wxor-used-as-pow-1.c
index 962902c3a05..33c418c2479 100644
--- a/gcc/testsuite/c-c++-common/Wxor-used-as-pow-1.c
+++ b/gcc/testsuite/c-c++-common/Wxor-used-as-pow-1.c
@@ -55,3 +55,7 @@ int h10_3 = 0xa ^ 3;
 /* Don't complain if the RHS isn't literal decimal.  */
 int t2_x16 = 2^0x10;
 int h10_x3 = 10 ^ 0x3;
+
+/* Don't complain about uses in macros.  */
+#define AMT (10^2)
+int amt = AMT;
diff --git a/gcc/testsuite/c-c++-common/Wxor-used-as-pow-pr107002-0.c b/gcc/testsuite/c-c++-common/Wxor-used-as-pow-pr107002-0.c
new file mode 100644
index 00000000000..37a1e301d12
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wxor-used-as-pow-pr107002-0.c
@@ -0,0 +1,9 @@
+/* Regression test for ICE seen in -Wxor-used-as-pow with
+   -ftrack-macro-expansion=0 in source-printing (fix-it-hints,
+   specifically).  */
+
+/* { dg-options "-ftrack-macro-expansion=0 -fdiagnostics-show-caret" } */
+
+#define test(lower, higher, a, b, c, d)					\
+  char test##line[ (a higher b lower c higher d) == 0 ? -1 : 1];
+test (|, ^, 1, 2, 2, 1)
diff --git a/gcc/testsuite/c-c++-common/Wxor-used-as-pow-pr107002-1.c b/gcc/testsuite/c-c++-common/Wxor-used-as-pow-pr107002-1.c
new file mode 100644
index 00000000000..10e2a0d55a3
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wxor-used-as-pow-pr107002-1.c
@@ -0,0 +1,5 @@
+/* { dg-options "-ftrack-macro-expansion=1 -fdiagnostics-show-caret" } */
+
+#define test(lower, higher, a, b, c, d)					\
+  char test##line[ (a higher b lower c higher d) == 0 ? -1 : 1];
+test (|, ^, 1, 2, 2, 1)
diff --git a/gcc/testsuite/c-c++-common/Wxor-used-as-pow-pr107002-2.c b/gcc/testsuite/c-c++-common/Wxor-used-as-pow-pr107002-2.c
new file mode 100644
index 00000000000..8e9a4bf0f55
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wxor-used-as-pow-pr107002-2.c
@@ -0,0 +1,5 @@
+/* { dg-options "-ftrack-macro-expansion=2 -fdiagnostics-show-caret" } */
+
+#define test(lower, higher, a, b, c, d)					\
+  char test##line[ (a higher b lower c higher d) == 0 ? -1 : 1];
+test (|, ^, 1, 2, 2, 1)

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-03-28 18:36 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-28 18:36 [gcc r13-6912] Don't emit -Wxor-used-as-pow on macro expansions [PR107002] David Malcolm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).