From 6bd1241136618ff066be71d00b04f1ebfa2d07e0 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Thu, 25 Feb 2016 14:01:34 -0500
Subject: [PATCH 6/8] FIXME: PR c/6906: WIP

---
 gcc/Makefile.in                                    |   2 +-
 gcc/c-family/c-macro.c                             | 195 +++++++++++++++++++++
 gcc/c-family/c-macro.h                             |  26 +++
 gcc/c-family/c.opt                                 |   4 +
 gcc/c/c-decl.c                                     |   3 +
 gcc/cp/semantics.c                                 |   3 +
 .../c-c++-common/Wside-effect-in-assert.c          |  74 ++++++++
 7 files changed, 306 insertions(+), 1 deletion(-)
 create mode 100644 gcc/c-family/c-macro.c
 create mode 100644 gcc/c-family/c-macro.h
 create mode 100644 gcc/testsuite/c-c++-common/Wside-effect-in-assert.c

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 3d6f0e6..6b99c66 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1169,7 +1169,7 @@ C_COMMON_OBJS = c-family/c-common.o c-family/c-cppbuiltin.o c-family/c-dump.o \
   c-family/c-lex.o c-family/c-omp.o c-family/c-opts.o c-family/c-pch.o \
   c-family/c-ppoutput.o c-family/c-pragma.o c-family/c-pretty-print.o \
   c-family/c-semantics.o c-family/c-ada-spec.o \
-  c-family/c-cilkplus.o \
+  c-family/c-cilkplus.o c-family/c-macro.o \
   c-family/array-notation-common.o c-family/cilk.o c-family/c-ubsan.o
 
 # Language-independent object files.
diff --git a/gcc/c-family/c-macro.c b/gcc/c-family/c-macro.c
new file mode 100644
index 0000000..9d422ef
--- /dev/null
+++ b/gcc/c-family/c-macro.c
@@ -0,0 +1,195 @@
+/* Implementation of warnings relating to macros.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tm.h"
+#include "c-common.h"
+#include "c-macro.h"
+
+// FIXME:
+#include "print-tree.h" // for debug_tree
+
+
+
+/* Implementation of -Wside-effect-in-assert.  */
+
+/* Return whether a macro with name MACRO_NAME should be regarded as
+   an assertion macro.  Currently any macro with "assert" or "ASSERT"
+   within the name is treated as an assertion macro.
+
+   TODO: make this case-insensitive.  */
+
+static bool macro_name_is_assertion_p (const char *macro_name)
+{
+  gcc_assert (macro_name);
+
+  /* FIXME: glibc has strcasestr as a non-standard extension.  */
+
+  if (strstr (macro_name, "assert"))
+    return true;
+  if (strstr (macro_name, "ASSERT"))
+    return true;
+
+  return false;
+}
+
+/* Return whether location LOC is within the expansion of an assertion
+   macro (by looking for at the names of macros, using
+   macro_name_is_assertion_p).  */
+
+static bool
+within_assert_expansion_p (location_t loc)
+{
+  const line_map *map = linemap_lookup (line_table, loc);
+
+  while (linemap_macro_expansion_map_p (map))
+    {
+      const line_map_macro *macro_map = linemap_check_macro (map);
+      const char* macro_name = linemap_map_get_macro_name (macro_map);
+      if (macro_name_is_assertion_p (macro_name))
+	return true;
+      loc = linemap_unwind_toward_expansion (line_table, loc, &map);
+    }
+
+  return false;
+}
+
+/* Given a CALL_EXPR, return the IDENTIFIER_NODE for the name of the
+   function being called, if it's statically-known, otherwise return
+   NULL_TREE.  */
+
+static tree
+get_callee_identifier (tree call_expr)
+{
+  gcc_assert (TREE_CODE (call_expr) == CALL_EXPR);
+  tree fndecl = get_callee_fndecl (call_expr);
+  if (!fndecl)
+    return NULL_TREE;
+  return DECL_NAME (fndecl);
+}
+
+/* Return whether CALL_EXPR is a call to an assertion-failure handler.
+   For now we just look for calls to "__assert_fail".  */
+
+static bool
+call_expr_is_assertion_failure_handler_p (tree call_expr)
+{
+  gcc_assert (TREE_CODE (call_expr) == CALL_EXPR);
+  tree identifier = get_callee_identifier (call_expr);
+  if (!identifier)
+    return false;
+
+  gcc_assert (TREE_CODE (identifier) == IDENTIFIER_NODE);
+  if (0 == strcmp (IDENTIFIER_POINTER (identifier),
+		   "__assert_fail"))
+    return true;
+
+  return false;
+}
+
+/* Return whether T could be considered a "side effect" if it occurs
+   within the expansion of an assertion macro.  */
+
+static bool
+assertion_side_effect_p (tree t)
+{
+  switch (TREE_CODE (t))
+    {
+      case MODIFY_EXPR:
+      case PREDECREMENT_EXPR:
+      case PREINCREMENT_EXPR:
+      case POSTDECREMENT_EXPR:
+      case POSTINCREMENT_EXPR:
+	return true;
+
+      case CALL_EXPR:
+	{
+	  /* Ignore calls to "__assert_fail" and the like.  */
+	  if (call_expr_is_assertion_failure_handler_p (t))
+	    return false;
+	  else
+	    return true;
+	}
+
+      default:
+	return false;
+    }
+}
+
+/* Implementation of -Wside-effect-in-assert, considering tree node T
+   itself (rather than its descendants).
+   Issue a warning if T is within the expansion of an assertion macro,
+   and could be considered a "side effect".  */
+
+static void
+check_for_side_effects_in_assert (tree t)
+{
+  if (!CAN_HAVE_LOCATION_P (t))
+    return;
+
+  if (!assertion_side_effect_p (t))
+    return;
+
+  location_t loc = EXPR_LOCATION (t);
+  if (!within_assert_expansion_p (loc))
+    return;
+
+
+  /* TODO: don't count the assertion failure handler as being a
+     side-effect.
+     For example, if we have a call to __assert_fail, don't
+     issue a warning for that.  */
+
+#if 0
+  /* We probably want to suppress this warning if it occurs in a system header,
+     but note that <assert.h> is itself a system header, so need to be
+     careful here.  */
+  if (!linemap_location_in_system_header_p (struct line_maps *,
+					 source_location);
+#endif
+
+  warning_at (loc, OPT_Wside_effect_in_assert,
+	      "statement with side-effects within assertion macro");
+  /* TODO: would it be even better to tell the user what the
+     macro expanded to? */
+}
+
+/* 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_side_effects_in_assert (*tp);
+  return NULL_TREE;
+}
+
+/* Top-level implementation of -Wside-effect-in-assert.  */
+
+void
+warn_for_macro_side_effects (tree t)
+{
+  /* Early reject for when warning is disabled.  */
+  if (!warn_side_effect_in_assert)
+    return;
+  // TODO: do we have to walk every statement within t?
+  walk_tree(&t, check_for_side_effects_in_assert_r, NULL, NULL);
+}
diff --git a/gcc/c-family/c-macro.h b/gcc/c-family/c-macro.h
new file mode 100644
index 0000000..6208c93
--- /dev/null
+++ b/gcc/c-family/c-macro.h
@@ -0,0 +1,26 @@
+/* Declarations for c-macro.c.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_C_MACRO_H
+#define GCC_C_MACRO_H
+
+extern void
+warn_for_macro_side_effects (tree);
+
+#endif  /* ! GCC_C_MACRO_H  */
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 7c5f6c7..6c7c987 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -585,6 +585,10 @@ Wnamespaces
 C++ ObjC++ Var(warn_namespaces) Warning
 Warn on namespace definition.
 
+Wside-effect-in-assert
+C C++ Common Var(warn_side_effect_in_assert) Warning
+Warn when the condition within an \"assert\" macro has side-effects.
+
 Wsized-deallocation
 C++ ObjC++ Var(warn_sized_deallocation) Warning EnabledBy(Wextra)
 Warn about missing sized deallocation functions.
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index bab4715..7c0ee5b 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -51,6 +51,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-family/c-ada-spec.h"
 #include "cilk.h"
 #include "builtins.h"
+#include "c-family/c-macro.h"
 
 /* In grokdeclarator, distinguish syntactic contexts of declarators.  */
 enum decl_context
@@ -617,6 +618,8 @@ add_stmt (tree t)
   if (code == LABEL_EXPR || code == CASE_LABEL_EXPR)
     STATEMENT_LIST_HAS_LABEL (cur_stmt_list) = 1;
 
+  warn_for_macro_side_effects (t);
+
   /* Add T to the statement-tree.  Non-side-effect statements need to be
      recorded during statement expressions.  */
   if (!building_stmt_list_p ())
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index fd83c46..a3c0fe9 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "omp-low.h"
 #include "convert.h"
 #include "gomp-constants.h"
+#include "c-family/c-macro.h"
 
 /* There routines provide a modular interface to perform many parsing
    operations.  They may therefore be used during actual parsing, or
@@ -380,6 +381,8 @@ add_stmt (tree t)
   if (code == LABEL_EXPR || code == CASE_LABEL_EXPR)
     STATEMENT_LIST_HAS_LABEL (cur_stmt_list) = 1;
 
+  warn_for_macro_side_effects (t);
+
   /* Add T to the statement-tree.  Non-side-effect statements need to be
      recorded during statement expressions.  */
   gcc_checking_assert (!stmt_list_stack->is_empty ());
diff --git a/gcc/testsuite/c-c++-common/Wside-effect-in-assert.c b/gcc/testsuite/c-c++-common/Wside-effect-in-assert.c
new file mode 100644
index 0000000..5ad8d71
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wside-effect-in-assert.c
@@ -0,0 +1,74 @@
+/* Verify "-Wside-effect-in-assert" (PR c/6906).  */
+
+/* { dg-options "-Wside-effect-in-assert" } */
+
+#include <stdlib.h>
+
+/* Reduced form of glibc's <assert.h>.  */
+
+#if defined __cplusplus
+# define __ASSERT_VOID_CAST static_cast<void>
+#else
+# define __ASSERT_VOID_CAST (void)
+#endif
+
+# define assert(expr)							\
+  ((expr)								\
+   ? __ASSERT_VOID_CAST (0)						\
+   : __assert_fail (__STRING(expr), __FILE__, __LINE__, __PRETTY_FUNCTION__))
+
+extern void __assert_fail (const char *__assertion, const char *__file,
+      unsigned int __line, const char *__function)
+     __attribute__ ((__nothrow__ , __leaf__)) __attribute__ ((__noreturn__));
+
+void *ptr;
+extern int f(void);
+
+#define HARMLESS_MACRO(X) \
+  do { X; } while (0)
+
+/* A macro that embeds an assert.  */
+#define WRAPPER_MACRO(X) \
+  do { assert (X); } while (0)
+
+void test_1 (void)
+{
+  ptr = malloc (64);
+}
+
+void test_2 (void)
+{
+  HARMLESS_MACRO (ptr = malloc (64));
+}
+
+void test_3 (void)
+{
+  assert(ptr = malloc(64)); /* { dg-warning "statement with side-effects within assertion macro" } */
+}
+
+/* A side-effect within a macro that embeds "assert";
+   we ought to warn.  */
+
+void test_4 (void)
+{
+  WRAPPER_MACRO (ptr = malloc (64)); /* { dg-warning "statement with side-effects within assertion macro" } */
+}
+
+/* Various assertions that contain side-effects.
+   We ought to warn about these.  */
+
+void test_5 (const char *p)
+{
+  assert(*p++);  /* { dg-warning "statement with side-effects within assertion macro" } */
+}
+
+void test_6 (void)
+{
+  assert(f());  /* { dg-warning "statement with side-effects within assertion macro" } */
+}
+
+int i;
+void test_7 (void)
+{
+  assert(i = 1); /* { dg-warning "statement with side-effects within assertion macro" } */
+}
-- 
1.8.5.3

