From 7da6e955b2ec50068d1e4e7f9e062cb679af4ba2 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Fri, 11 Mar 2016 17:01:11 -0500
Subject: [PATCH 8/8] FIXME: WIP at -Wunsafe-macro-expansion

---
 gcc/c-family/c-macro.c                             | 219 ++++++++++++++++++++-
 gcc/c-family/c.opt                                 |   4 +
 gcc/doc/invoke.texi                                |  28 +++
 .../c-c++-common/Wunsafe-macro-expansion-1.c       |  39 ++++
 .../c-c++-common/Wunsafe-macro-expansion-4.c       |  52 +++++
 .../c-c++-common/Wunsafe-macro-expansion-5.c       |  37 ++++
 gcc/testsuite/g++.dg/Wunsafe-macro-expansion-2.C   |  36 ++++
 gcc/testsuite/g++.dg/Wunsafe-macro-expansion-3.C   |  39 ++++
 gcc/testsuite/gcc.dg/Wunsafe-macro-expansion-2.c   |  46 +++++
 gcc/testsuite/gcc.dg/Wunsafe-macro-expansion-3.c   |  39 ++++
 libcpp/include/line-map.h                          |  22 +++
 11 files changed, 553 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wunsafe-macro-expansion-1.c
 create mode 100644 gcc/testsuite/c-c++-common/Wunsafe-macro-expansion-4.c
 create mode 100644 gcc/testsuite/c-c++-common/Wunsafe-macro-expansion-5.c
 create mode 100644 gcc/testsuite/g++.dg/Wunsafe-macro-expansion-2.C
 create mode 100644 gcc/testsuite/g++.dg/Wunsafe-macro-expansion-3.C
 create mode 100644 gcc/testsuite/gcc.dg/Wunsafe-macro-expansion-2.c
 create mode 100644 gcc/testsuite/gcc.dg/Wunsafe-macro-expansion-3.c

diff --git a/gcc/c-family/c-macro.c b/gcc/c-family/c-macro.c
index 5330ed4..5ceaed3 100644
--- a/gcc/c-family/c-macro.c
+++ b/gcc/c-family/c-macro.c
@@ -24,6 +24,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-common.h"
 #include "c-macro.h"
 #include "calls.h"
+#include "cpp-id-data.h"
 
 
 
@@ -329,24 +330,226 @@ check_for_side_effects_in_assert (tree t)
     }
 }
 
+
+
+/* Implementation of -Wunsafe-macro-expansion.  */
+
+/* We will warn about macro expansions containting side-effects,
+   where the location of the side effect is from an argument
+   that is expanded more than once.  */
+
+static idempotent_warning unsafe_macro_expansions;
+
+/* Given MACRO_MAP and the index EXPANSION_TOKEN_IDX of a token
+   within the expanded tokens, return the index of the token
+   within the definition that the expanded token came from.
+
+   We do this by looking at changes in the y_loc within the macro map:
+   runs of equal value correspond to multiple tokens coming from the
+   same argument.  */
+
+static int
+calc_index_within_definition (const line_map_macro *macro_map,
+			      unsigned expansion_token_idx)
+{
+  gcc_assert (macro_map);
+  gcc_assert (expansion_token_idx < macro_map->n_tokens);
+
+  location_t last_y_loc = macro_map->get_y_loc (0);
+  unsigned definition_token_idx = 0;
+  for (unsigned i = 1; i <= expansion_token_idx; i++)
+    {
+      const location_t y_loc = macro_map->get_y_loc (i);
+      gcc_assert (y_loc != UNKNOWN_LOCATION);
+      if (y_loc != last_y_loc)
+	definition_token_idx++;
+      last_y_loc = y_loc;
+    }
+  return definition_token_idx;
+}
+
+/* Helper function for check_for_unsafe_macro_expansion.
+
+   Assuming the presence of a side-effect, issue a warning at LOC
+   if LOC is part of the argument to a macro expansion that
+   has been expanded more than once.  Recurse into nested macro
+   expansions if necessary.   */
+
+static void
+check_for_unsafe_macro_expansion_at (location_t loc)
+{
+  const line_map *map = linemap_lookup (line_table, loc);
+  gcc_assert (map);
+  if (!linemap_macro_expansion_map_p (map))
+    return;
+
+  const line_map_macro *macro_map = linemap_check_macro (map);
+  const cpp_macro *macro = macro_map->macro->value.macro;
+
+  if (macro->traditional)
+    return;
+
+  /* Locate the token within the macro expansion.  */
+  if (IS_ADHOC_LOC (loc))
+    loc = get_location_from_adhoc_loc (line_table, loc);
+
+  gcc_assert (loc >= macro_map->start_location);
+  unsigned expansion_token_idx = loc - MAP_START_LOCATION (macro_map);
+  gcc_assert (expansion_token_idx < MACRO_MAP_NUM_MACRO_TOKENS (macro_map));
+
+  /* We require a token in the expansion which came from an argument.
+     Perform a relatively cheap test to reject tokens
+     that don't come from arguments.
+     Arguments have x_loc != y_loc, so reject other tokens.  */
+  const location_t x_loc = macro_map->get_x_loc (expansion_token_idx);
+  const location_t y_loc = macro_map->get_y_loc (expansion_token_idx);
+  if (x_loc == y_loc)
+    return;
+
+  /* Now locate the corresponding token within the
+     macro *definition*.  */
+  unsigned definition_token_idx
+    = calc_index_within_definition (macro_map, expansion_token_idx);
+  gcc_assert (definition_token_idx < macro->count);
+
+  const cpp_token *token = &macro->exp.tokens[definition_token_idx];
+  gcc_assert (token->type == CPP_MACRO_ARG);
+
+  unsigned arg_no = token->val.macro_arg.arg_no;
+  gcc_assert (arg_no > 0);
+  unsigned arg_idx = arg_no - 1;
+
+  const cpp_hashnode *param = macro->params[arg_idx];
+  gcc_assert (param);
+
+  /* We care about expansions where the given token is one that is expanded
+     more than once.
+     Populate given_arg_indices with the indices of those tokens in the macro
+     expansion that are copies of the given token.
+     TODO: optimize this?  only do it once per macro?  */
+  auto_vec <unsigned> given_arg_indices;
+  for (unsigned i = 0; i < macro->count; i++)
+    {
+      gcc_assert (!macro->traditional);
+      cpp_token *other_tok = &macro->exp.tokens[i];
+      if (other_tok->type == CPP_MACRO_ARG)
+	if (other_tok->val.macro_arg.arg_no == arg_no)
+	  given_arg_indices.safe_push (i);
+    }
+
+  /* Given that we saw the token, we must have at least one mention in the
+     vec.  */
+  gcc_assert (given_arg_indices.length () > 0);
+
+  /* We care if there's *more* than one.  */
+  if (given_arg_indices.length () < 2)
+    {
+      /* However, if we only saw one, it's possible that we still have a
+	 multiple expansion if this is within a nested macro, so recurse,
+	 using the x-location.  */
+      check_for_unsafe_macro_expansion_at (x_loc);
+      return;
+    }
+
+  /* OK: we have an expression with side-effects that came from an
+     argument to a macro, where the argument is expanded more than once
+     by that macro.  */
+
+  /* Only complain about the first expansion seen. */
+  if (definition_token_idx != given_arg_indices[0])
+    return;
+
+  /* We want to only warn once per macro expansion/argument.
+     We can do this by only warning once per virtual location.  */
+  if (unsafe_macro_expansions.warned_about_p (loc))
+    return;
+
+  if (warning_at (loc, OPT_Wunsafe_macro_expansion,
+		  "repeated side-effects within expansion of unsafe macro %qs",
+		  linemap_map_get_macro_name (macro_map)))
+    {
+      unsafe_macro_expansions.set_warned_about (loc);
+
+      /* Show the macro definition, highlighting the locations where the
+	 given parameter is expanded.
+
+	 Build a rich_location, starting with the line containing the
+	 macro definition as the primary location.  Ideally we would use
+	 the location of the given param, but that doesn't seem to be
+	 available.  */
+      rich_location richloc (line_table, macro->line);
+
+      /* Add locations showing each place in the macro definition
+	 where the given parameter is used.  */
+      for (unsigned i = 0; i < given_arg_indices.length (); i++)
+	{
+	  cpp_token *tok = &macro->exp.tokens[given_arg_indices[i]];
+	  richloc.add_range (tok->src_loc, true);
+	}
+
+      /* Emit the note.  */
+      inform_at_rich_loc
+	(&richloc,
+	 "parameter %i (%qs) is expanded %i times by macro %qs",
+	 arg_no,
+	 param->ident.str,
+	 given_arg_indices.length (),
+	 linemap_map_get_macro_name (macro_map));
+    }
+}
+
+/* Implementation of -Wunsafe-macro-expansion, considering tree node T
+   itself (rather than its descendants).
+   Issue a warning if:
+   - T is within the first expansion of a macro argument
+     - where the argument is expanded more than once by the macro
+       (and is thus unsafe), and
+   - where T could be considered a "side effect".  */
+
+static void
+check_for_unsafe_macro_expansion (tree t)
+{
+  if (!CAN_HAVE_LOCATION_P (t))
+    return;
+
+  // FIXME: need a weaker condition, I think
+  if (!assertion_side_effect_at_p (t, NULL))
+    return;
+
+  /* Only warn within macro expansions.  */
+  location_t loc = EXPR_LOCATION (t);
+
+  check_for_unsafe_macro_expansion_at (loc);
+}
+
+
+
+/* Code shared between -Wside-effect-in-assert and
+   -Wunsafe-macro-expansion.  */
+
 /* Tree-walking callback for use by warn_for_macro_side_effects.  */
 
 static tree
-check_for_side_effects_in_assert_r (tree *tp, int */*walk_subtrees*/,
-				    void */*data*/)
+check_for_macro_side_effects_r (tree *tp, int */*walk_subtrees*/,
+				void */*data*/)
 {
-  check_for_side_effects_in_assert (*tp);
+  if (warn_side_effect_in_assert)
+    check_for_side_effects_in_assert (*tp);
+  if (warn_unsafe_macro_expansion)
+    check_for_unsafe_macro_expansion (*tp);
   return NULL_TREE;
 }
 
-/* Top-level implementation of -Wside-effect-in-assert.  */
+/* Top-level implementation of -Wside-effect-in-assert and
+   -Wunsafe-macro-expansion.  */
 
 void
 warn_for_macro_side_effects (tree t)
 {
-  /* Early reject for when warning is disabled.  */
-  if (!warn_side_effect_in_assert)
+  /* Early reject for when both warning are disabled.  */
+  if (!warn_side_effect_in_assert
+      && !warn_unsafe_macro_expansion)
     return;
-  // TODO: do we have to walk every statement within t?
-  walk_tree(&t, check_for_side_effects_in_assert_r, NULL, NULL);
+  walk_tree (&t, check_for_macro_side_effects_r, NULL, NULL);
 }
+
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 6c7c987..a6d10be 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -928,6 +928,10 @@ Wunknown-pragmas
 C ObjC C++ ObjC++ Warning Var(warn_unknown_pragmas) LangEnabledBy(C ObjC C++ ObjC++,Wall, 1, 0)
 Warn about unrecognized pragmas.
 
+Wunsafe-macro-expansion
+C C++ Common Var(warn_unsafe_macro_expansion) Warning
+Warn when an unsafe macro is expanded with arguments that have side-effects.
+
 Wunsuffixed-float-constants
 C ObjC Var(warn_unsuffixed_float_constants) Warning
 Warn about unsuffixed float constants.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index f6f840a..76c1afe 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -301,6 +301,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wsystem-headers  -Wtautological-compare  -Wtrampolines  -Wtrigraphs @gol
 -Wtype-limits  -Wundef @gol
 -Wuninitialized  -Wunknown-pragmas  -Wunsafe-loop-optimizations @gol
+-Wunsafe-macro-expansion @gol
 -Wunsuffixed-float-constants  -Wunused  -Wunused-function @gol
 -Wunused-label  -Wunused-local-typedefs -Wunused-parameter @gol
 -Wno-unused-result -Wunused-value @gol -Wunused-variable @gol
@@ -5762,6 +5763,33 @@ minimum maximum, so we do not diagnose overlength strings in C++@.
 This option is implied by @option{-Wpedantic}, and can be disabled with
 @option{-Wno-overlength-strings}.
 
+@item -Wunsafe-macro-expansion @r{(C and C++ only)}
+@opindex Wunsafe-macro-expansion
+@opindex Wno-unsafe-macro-expansion
+Issue a warning when an expression with a side effect is used within
+certain macro arguments.  Specifically, any argument that occurs
+more than once in the macro expansion is ``unsafe'', since the
+side-effect will occur more than once in the expansion.  For example,
+in:
+
+@smallexample
+  #define MIN(A, B)  ((A) < (B) ? (A) : (B))
+
+  foo = MIN (*p++, *q++);
+@end smallexample
+
+the macro expands to:
+
+@smallexample
+  foo = ((*p++) < (*q++) ? (*p++) : (*q++));
+@end smallexample
+
+which is probably not what the author of the code intended.  Such
+macros can often be fixed by converting them to an inline function,
+or by introducing a temporary copy of each argument, perhaps by using
+the GNU extension @code{typeof} in C, or the @code{auto} keyword in
+C++11 onwards.
+
 @item -Wunsuffixed-float-constants @r{(C and Objective-C only)}
 @opindex Wunsuffixed-float-constants
 
diff --git a/gcc/testsuite/c-c++-common/Wunsafe-macro-expansion-1.c b/gcc/testsuite/c-c++-common/Wunsafe-macro-expansion-1.c
new file mode 100644
index 0000000..f8b4f85
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wunsafe-macro-expansion-1.c
@@ -0,0 +1,39 @@
+/* { dg-options "-Wunsafe-macro-expansion -fdiagnostics-show-caret" } */
+
+/* The macro invocation:
+     ABS (++n)
+   expands to:
+     m = (((++n) < 0) ? -(++n) : (++n));
+   which is probably not what was desired.  */
+
+#define ABS(ARG) (((ARG) < 0) ? -(ARG) : (ARG))
+  /* { dg-message "parameter 1 ..ARG.. is expanded 3 times by macro .ABS." "" { target *-*-* } 9 } */
+
+int
+test_1 (int n)
+{
+  int m;
+  m = ABS (++n); /* { dg-warning "repeated side-effects within expansion of unsafe macro .ABS." } */
+  return m;
+
+  /* { dg-begin-multiline-output "" }
+   m = ABS (++n);
+            ^
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+ #define ABS(ARG) (((ARG) < 0) ? -(ARG) : (ARG))
+                     ^~~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+ #define ABS(ARG) (((ARG) < 0) ? -(ARG) : (ARG))
+                     ^~~           ^~~     ^~~
+     { dg-end-multiline-output "" } */
+}
+
+/* This invocation doesn't have side-effects; we shouldn't warn.  */
+
+int
+test_2 (int n, int m)
+{
+  return ABS (n * m);
+}
diff --git a/gcc/testsuite/c-c++-common/Wunsafe-macro-expansion-4.c b/gcc/testsuite/c-c++-common/Wunsafe-macro-expansion-4.c
new file mode 100644
index 0000000..98ff951
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wunsafe-macro-expansion-4.c
@@ -0,0 +1,52 @@
+/* { dg-options "-Wunsafe-macro-expansion" } */
+
+/* Tests of macro with arguments that occur a varying number of
+   times: 3 (A), 2, 1, and 0 (D).  */
+
+#define TEST(A, B, C, D) ((A) * (A) * (A) + (B) * (B) + (C))
+
+/* No side effects: we shouldn't warn.  */
+
+int
+test_1 (int a, int b, int c, int d)
+{
+  return TEST(a, b, c, d);
+}
+
+/* Side effects only affect arg with zero instantiations: we shouldn't
+   warn.  */
+
+int
+test_2 (int a, int b, int c, int d)
+{
+  return TEST(a, b, c, d++);
+}
+
+/* Side effects only affect arg with one instantiation: we shouldn't
+   warn.  */
+
+int
+test_3 (int a, int b, int c, int d)
+{
+  return TEST(a, b, c++, d);
+}
+
+/* Side effects affect an arg with two instantiations: we should
+   warn.  */
+
+int
+test_4 (int a, int b, int c, int d)
+{
+  return TEST(a, b++, c, d); /* { dg-warning "unsafe macro .TEST." } */
+  /* { dg-message "parameter 2 ..B.. is expanded 2 times by macro .TEST." "" { target *-*-* } 6 } */
+}
+
+/* Side effects affect an arg with three instantiations: we should
+   warn.  */
+
+int
+test_5 (int a, int b, int c, int d)
+{
+  return TEST(a++, b, c, d); /* { dg-warning "unsafe macro .TEST." } */
+  /* { dg-message "parameter 1 ..A.. is expanded 3 times by macro .TEST." "" { target *-*-* } 6 } */
+}
diff --git a/gcc/testsuite/c-c++-common/Wunsafe-macro-expansion-5.c b/gcc/testsuite/c-c++-common/Wunsafe-macro-expansion-5.c
new file mode 100644
index 0000000..46fea27
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wunsafe-macro-expansion-5.c
@@ -0,0 +1,37 @@
+/* { dg-options "-Wunsafe-macro-expansion" } */
+
+/* Tests of nested macro expansion.  */
+
+#define PASSTHROUGH(ARG) ARG
+
+#define OUTER(FOO, BAR) \
+  (((PASSTHROUGH(FOO)) * (PASSTHROUGH(FOO))) \
+   + ((PASSTHROUGH(BAR)) * (PASSTHROUGH(BAR))))
+
+/* No side-effects; we shouldn't warn here.  */
+
+int
+test_1 (int lorem, int ipsum)
+{
+  return OUTER (lorem, ipsum);
+}
+
+/* Both params have side-effects; we should warn twice.  */
+
+int
+test_2 (int lorem, int ipsum)
+{
+  return OUTER (lorem++, ipsum++); /* { dg-warning "repeated side-effects within expansion of unsafe macro .OUTER." } */
+  /* { dg-message "parameter 1 ..FOO.. is expanded 2 times by macro .OUTER." "" { target *-*-* } 7 } */
+  /* { dg-message "parameter 2 ..BAR.. is expanded 2 times by macro .OUTER." "" { target *-*-* } 7 } */
+}
+
+/* Wrap it all in another macro.  We should again warn twice.  */
+
+int
+test_3 (int lorem, int ipsum)
+{
+#define DO_IT OUTER (lorem++, ipsum++) /* { dg-warning "repeated side-effects within expansion of unsafe macro .OUTER." } */
+
+  return DO_IT;
+}
diff --git a/gcc/testsuite/g++.dg/Wunsafe-macro-expansion-2.C b/gcc/testsuite/g++.dg/Wunsafe-macro-expansion-2.C
new file mode 100644
index 0000000..8d15599
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wunsafe-macro-expansion-2.C
@@ -0,0 +1,36 @@
+/* Reduced reproducer for the issue described in
+   https://dmalcolm.fedorapeople.org/presentations/PyCon-US-2013/PyCon-US-2013-dmalcolm-StaticAnalysis.html#(38)
+*/
+
+/* { dg-options "-Wunsafe-macro-expansion" } */
+
+#define NULL ((void *)0)
+typedef struct _object { int ob_refcnt; } PyObject;
+#define Py_DECREF(op)                                   \
+    do {                                                \
+        if (--((PyObject*)(op))->ob_refcnt == 0)        \
+          _Py_Dealloc((PyObject *)(op));                \
+    } while (0)
+#define Py_XDECREF(op) do { if ((op) == NULL) ; else Py_DECREF(op); } while (0)
+#define _Py_Dealloc(op) /* no-op for simplicity */
+
+extern PyObject *
+PyObject_CallObject (PyObject *callable_object, PyObject *args);
+
+void
+test_1 (PyObject *callable, PyObject *args)
+{
+  Py_XDECREF(PyObject_CallObject(callable, args)); /* { dg-warning "repeated side-effects within expansion of unsafe macro .Py_XDECREF." } */
+  /* { dg-message "parameter 1 ..op.. is expanded 2 times by macro .Py_XDECREF." "" { target *-*-* } 14 } */
+
+  /* { dg-warning "repeated side-effects within expansion of unsafe macro .Py_DECREF." "" { target *-*-* } 23 } */
+  /* { dg-message "parameter 1 ..op.. is expanded 2 times by macro .Py_DECREF." "" { target *-*-* } 9 } */
+}
+
+/* This invocation doesn't have side-effects; we shouldn't warn.  */
+
+void
+test_2 (PyObject *obj)
+{
+  Py_XDECREF (obj);
+}
diff --git a/gcc/testsuite/g++.dg/Wunsafe-macro-expansion-3.C b/gcc/testsuite/g++.dg/Wunsafe-macro-expansion-3.C
new file mode 100644
index 0000000..d414253
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wunsafe-macro-expansion-3.C
@@ -0,0 +1,39 @@
+/* { dg-options "-Wunsafe-macro-expansion -fdiagnostics-show-caret" } */
+
+/* Example taken from our docs for "typeof".  */
+
+#define min(A, B)  ((A) < (B) ? (A) : (B))
+
+/* { dg-message "parameter 2 ..B.. is expanded 2 times by macro .min." "" { target *-*-* } 5 } */
+
+int x, y, z, next;
+extern int foo (int);
+
+/* This invocation has side-effects; we should warn.  */
+
+void
+test_1 (void)
+{
+  next = min (x + y, foo (z)); /* { dg-warning "repeated side-effects within expansion of unsafe macro .min." }
+
+  /* { dg-begin-multiline-output "" }
+   next = min (x + y, foo (z));
+                          ^
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+ #define min(A, B)  ((A) < (B) ? (A) : (B))
+                            ^
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+ #define min(A, B)  ((A) < (B) ? (A) : (B))
+                            ^           ^
+     { dg-end-multiline-output "" } */
+}
+
+/* This invocation doesn't have side-effects; we shouldn't warn.  */
+
+int
+test_2 (int p, int q)
+{
+  return min (p, q);
+}
diff --git a/gcc/testsuite/gcc.dg/Wunsafe-macro-expansion-2.c b/gcc/testsuite/gcc.dg/Wunsafe-macro-expansion-2.c
new file mode 100644
index 0000000..6d95b62
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wunsafe-macro-expansion-2.c
@@ -0,0 +1,46 @@
+/* Reduced reproducer for the issue described in
+   https://dmalcolm.fedorapeople.org/presentations/PyCon-US-2013/PyCon-US-2013-dmalcolm-StaticAnalysis.html#(38)
+*/
+
+/* { dg-options "-Wunsafe-macro-expansion -fdiagnostics-show-caret" } */
+
+#define NULL ((void *)0)
+typedef struct _object { int ob_refcnt; } PyObject;
+#define Py_DECREF(op)                                   \
+    do {                                                \
+        if (--((PyObject*)(op))->ob_refcnt == 0)        \
+          _Py_Dealloc((PyObject *)(op));                \
+    } while (0)
+#define Py_XDECREF(op) do { if ((op) == NULL) ; else Py_DECREF(op); } while (0)
+/* { dg-message "parameter 1 ..op.. is expanded 2 times by macro .Py_XDECREF." "" { target *-*-* } 14 } */
+#define _Py_Dealloc(op) /* no-op for simplicity */
+
+extern PyObject *
+PyObject_CallObject (PyObject *callable_object, PyObject *args);
+
+void
+test_1 (PyObject *callable, PyObject *args)
+{
+  Py_XDECREF(PyObject_CallObject(callable, args)); /* { dg-warning "repeated side-effects within expansion of unsafe macro .Py_XDECREF." } */
+
+  /* { dg-begin-multiline-output "" }
+   Py_XDECREF(PyObject_CallObject(callable, args));
+              ^
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+ #define Py_XDECREF(op) do { if ((op) == NULL) ; else Py_DECREF(op); } while (0)
+                                  ^~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+ #define Py_XDECREF(op) do { if ((op) == NULL) ; else Py_DECREF(op); } while (0)
+                                  ^~                            ^~
+     { dg-end-multiline-output "" } */
+}
+
+/* This invocation doesn't have side-effects; we shouldn't warn.  */
+
+void
+test_2 (PyObject *obj)
+{
+  Py_XDECREF (obj);
+}
diff --git a/gcc/testsuite/gcc.dg/Wunsafe-macro-expansion-3.c b/gcc/testsuite/gcc.dg/Wunsafe-macro-expansion-3.c
new file mode 100644
index 0000000..2fe9bb5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wunsafe-macro-expansion-3.c
@@ -0,0 +1,39 @@
+/* { dg-options "-Wunsafe-macro-expansion -fdiagnostics-show-caret" } */
+
+/* Example taken from our docs for "typeof".  */
+
+#define min(A, B)  ((A) < (B) ? (A) : (B))
+
+/* { dg-message "parameter 2 ..B.. is expanded 2 times by macro .min." "" { target *-*-* } 5 } */
+
+int x, y, z, next;
+extern int foo (int);
+
+/* This invocation has side-effects; we should warn.  */
+
+void
+test_1 (void)
+{
+  next = min (x + y, foo (z)); /* { dg-warning "repeated side-effects within expansion of unsafe macro .min." }
+
+  /* { dg-begin-multiline-output "" }
+   next = min (x + y, foo (z));
+                      ^
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+ #define min(A, B)  ((A) < (B) ? (A) : (B))
+                            ^
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+ #define min(A, B)  ((A) < (B) ? (A) : (B))
+                            ^           ^
+     { dg-end-multiline-output "" } */
+}
+
+/* This invocation doesn't have side-effects; we shouldn't warn.  */
+
+int
+test_2 (int p, int q)
+{
+  return min (p, q);
+}
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 6ead57c..bf26496 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -389,6 +389,10 @@ struct cpp_hashnode;
    The offset from START_LOCATION is used to index into
    MACRO_LOCATIONS; this holds the original location of the token.  */
 struct GTY((tag ("2"))) line_map_macro : public line_map {
+
+  source_location get_x_loc (unsigned token_idx) const;
+  source_location get_y_loc (unsigned token_idx) const;
+
   /* The cpp macro which expansion gave birth to this macro map.  */
   struct cpp_hashnode * GTY ((nested_ptr (union tree_node,
 				   "%h ? CPP_HASHNODE (GCC_IDENT_TO_HT_IDENT (%h)) : NULL",
@@ -486,6 +490,24 @@ struct GTY((tag ("2"))) line_map_macro : public line_map {
 #define linemap_assert_fails(EXPR) (! (EXPR))
 #endif
 
+/* FIXME.  */
+
+inline source_location
+line_map_macro::get_x_loc (unsigned token_idx) const
+{
+  linemap_assert (token_idx < n_tokens);
+  return macro_locations[token_idx * 2];
+}
+
+/* FIXME.  */
+
+inline source_location
+line_map_macro::get_y_loc (unsigned token_idx) const
+{
+  linemap_assert (token_idx < n_tokens);
+  return macro_locations[(token_idx * 2) + 1];
+}
+
 /* Return TRUE if MAP encodes locations coming from a macro
    replacement-list at macro expansion point.  */
 bool
-- 
1.8.5.3

