Make sure not to instrument calls to advice functions.

Along with relevant test cases.

diff --git a/src/aop-main.c b/src/aop-main.c
index 2eaca6062d2bafe6bf0f53a9de84e02f05b455eb..95b21cede07132e5bbaf88cec473ad3d4a198a2d 100644 (file)
--- a/src/aop-main.c
+++ b/src/aop-main.c
@@ -55,6 +55,7 @@
 #include "aop-header.h"
 #include "aop-pointcut.h"
 #include "aop-type.h"
+#include "aop-weave.h"
 
 //#define PAUSE_ON_START
 
@@ -160,6 +161,8 @@ aop_join_on_copy (struct aop_pointcut *pc, int copy, join_callback callback,
 static unsigned int
 execute_pass_init ()
 {
+  clear_advice_table ();
+
   return 0;
 }
 

diff --git a/src/aop-pc-fun-call.c b/src/aop-pc-fun-call.c
index 3502538901e8cc2db8b53b8e37149509a2e3cc34..04aca1f11a961fe606f623a0722cee391e6f7b55 100644 (file)
--- a/src/aop-pc-fun-call.c
+++ b/src/aop-pc-fun-call.c
@@ -39,6 +39,7 @@
 #include "aop-dynval.h"
 #include "aop-pointcut.h"
 #include "aop-type.h"
+#include "aop-weave.h"
 
 /**
  * \defgroup call_pc Function Call Pointcut Functions
@@ -180,6 +181,11 @@ call_matches (struct aop_pointcut *pc, gimple call_stmt)
   aop_assert (pc->kind == ATP_CALL);
   aop_assert (gimple_code (call_stmt) == GIMPLE_CALL);
 
+  /* We never want to match a call that is actually one we
+     inserted! */
+  if (is_stmt_advice (call_stmt))
+    return false;
+
   /* Check function name only if the user filtered by name. */
   if (pc->pc_call.function_name != NULL)
     {

diff --git a/src/aop-weave.c b/src/aop-weave.c
index b3f26a73ed6aac974a7de6df515204d4cc45e39b..afca4c26425ec7a93b16e0fc3a7e818674b262d9 100644 (file)
--- a/src/aop-weave.c
+++ b/src/aop-weave.c
@@ -45,6 +45,58 @@
 #include "aop-pointcut.h"
 #include "aop-type.h"
 
+#define POISON ((void *)-1)
+
+/* This table holds every advice statement that InterAspect inserts so
+   that we can avoid matching any of them during a join operation. */
+static htab_t advice_stmts_table = POISON;
+
+/* Clear out the advice_stmts table.  It is important to call this
+   once for each compiled function (before any other InterAspect
+   passes run) because the statements in the advice table are no
+   longer meaningful once a function is done compiling. */
+void
+clear_advice_table ()
+{
+  advice_stmts_table = NULL;
+}
+
+/* Store a gimple statement in the advice table so that we remember it
+   is an advice statement! 
+   TODO: Also store the statement's birthday so we can send a card. */
+static void
+remember_advice (gimple stmt)
+{
+  gimple *slot;
+
+  /* Create the table if it is empty. */
+  aop_assert (advice_stmts_table != POISON);
+  if (advice_stmts_table == NULL)
+    advice_stmts_table =
+      htab_create_ggc (10, htab_hash_pointer, htab_eq_pointer, NULL);
+
+  slot = (gimple *)htab_find_slot (advice_stmts_table, stmt, INSERT);
+  *slot = stmt;
+}
+
+/* True if a GIMPLE statement was inserted by InterAspect as an advice
+   function. */
+bool
+is_stmt_advice (gimple stmt)
+{
+  gimple *slot;
+
+  /* If the table is empty, then stmt can't possibly be in it! */
+  aop_assert (advice_stmts_table != POISON);
+  if (advice_stmts_table == NULL)
+    return false;
+
+  slot = (gimple *)htab_find_slot (advice_stmts_table, stmt, NO_INSERT);
+  aop_assert (slot == NULL || *slot == stmt);
+
+  return (slot != NULL);
+}
+
 /* Throw a fatal error if a dynval is not allowed in a before-advice
    call. */
 static void
@@ -246,6 +298,8 @@ build_gcc_call (const char *func_name, tree return_type,
 
   VEC_free (tree, heap, arg_list);
 
+  remember_advice (func_call);
+
   return func_call;
 }
 

diff --git a/src/aop-weave.h b/src/aop-weave.h
new file mode 100644 (file)
index 0000000..4f78fa2
--- /dev/null
+++ b/src/aop-weave.h
@@ -0,0 +1,24 @@
+#ifndef __AOP_WEAVE_H__
+#define __AOP_WEAVE_H__
+
+/* This program 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 of the
+   License, or (at your option) any later version.
+
+   This program 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 this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+/* This is a private header.  Do not include it in source files for
+   client plug-ins. */
+
+extern void clear_advice_table ();
+extern bool is_stmt_advice (gimple stmt);
+
+#endif

diff --git a/test/Makefile.am b/test/Makefile.am
index ccc4cffcee1cd870025f027e06106889fbf442fa..11005db262c36d71da904e479991c1b5b0d37c30 100644 (file)
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -2,5 +2,6 @@ if HAVE_PYTHON
 TESTS_ENVIRONMENT = $(PYTHON) $(srcdir)/run-testcase.py --with-gcc=$(CC) \
        --with-ia-lib-dir=$(top_builddir)/src/.libs \
        --with-ia-src-dir=$(top_srcdir) --with-tests-dir=$(srcdir)
-TESTS = int-types.xml float-types.xml pointer-types.xml struct-types.xml
+TESTS = int-types.xml float-types.xml pointer-types.xml struct-types.xml \
+       reinst.xml
 endif

diff --git a/test/plugin-reinst1.c b/test/plugin-reinst1.c
new file mode 100644 (file)
index 0000000..a9e150f
--- /dev/null
+++ b/test/plugin-reinst1.c
@@ -0,0 +1,63 @@
+#include <aop.h>
+#include <stdio.h>
+#include <string.h>
+
+AOP_I_AM_GPL_COMPATIBLE();
+
+static void plugin_join_on_foo(struct aop_joinpoint *jp, void *data)
+{
+  struct aop_dynval *n;
+
+  n = aop_capture_param(jp, 1);
+  aop_insert_advice(jp, "_advice_foo", AOP_INSERT_BEFORE, AOP_DYNVAL(n), AOP_TERM_ARG);
+}
+
+static void plugin_join_on_advice(struct aop_joinpoint *jp, void *data)
+{
+  struct aop_dynval *n;
+
+  n = aop_capture_param(jp, 0);
+  aop_insert_advice(jp, "_advice_hook_bad", AOP_INSERT_BEFORE, AOP_DYNVAL(n), AOP_TERM_ARG);
+}
+
+static unsigned int plugin_reinst1()
+{
+  struct aop_pointcut *pc;
+
+  /* This will match the call to foo, which takes an int as its second
+     paramter. */
+  pc = aop_match_function_call();
+  aop_filter_call_pc_by_name(pc, "foo");
+  aop_filter_call_pc_by_param(pc, 1, aop_t_all_signed());
+  aop_join_on(pc, plugin_join_on_foo, NULL);
+
+  /* This would match the advice function that we inserted in the
+     previous join.  Of course, that should not happen because we
+     added checks to prevent any pointcut from matching an advice
+     call! */
+  pc = aop_match_function_call();
+  aop_filter_call_pc_by_param(pc, 0, aop_t_all_signed());
+  aop_join_on(pc, plugin_join_on_advice, NULL);
+
+  return 0;
+}
+
+static unsigned int plugin_reinst2()
+{
+  struct aop_pointcut *pc;
+
+  /* Try _again_ to match an advice function call!  Being in another
+     pass should not thwart InterAspect's memory of its own advice
+     functions. */
+  pc = aop_match_function_call();
+  aop_filter_call_pc_by_param(pc, 0, aop_t_all_signed());
+  aop_join_on(pc, plugin_join_on_advice, NULL);
+
+  return 0;
+}
+
+AOP_MAIN_PROTO aop_main()
+{
+  aop_register_pass("reinst1", plugin_reinst1);
+  aop_register_pass("reinst2", plugin_reinst2);
+}

diff --git a/test/plugin-reinst2.c b/test/plugin-reinst2.c
new file mode 100644 (file)
index 0000000..ca43799
--- /dev/null
+++ b/test/plugin-reinst2.c
@@ -0,0 +1,35 @@
+#include <aop.h>
+#include <stdio.h>
+#include <string.h>
+
+/* In the test case, this plug-in is called immediately after
+   plugin-reinst1. */
+
+AOP_I_AM_GPL_COMPATIBLE();
+
+static void plugin_join_on_advice(struct aop_joinpoint *jp, void *data)
+{
+  struct aop_dynval *n;
+
+  n = aop_capture_param(jp, 0);
+  aop_insert_advice(jp, "_advice_hook_good", AOP_INSERT_BEFORE, AOP_DYNVAL(n), AOP_TERM_ARG);
+}
+
+static unsigned int plugin_reinst3()
+{
+  struct aop_pointcut *pc;
+
+  /* This _will_ match one of the advice functions inserted by
+     plugin-reinst1.  InterAspect's memory of the advice functions it
+     inserts does not extend to other InterAspect plug-ins.  */
+  pc = aop_match_function_call();
+  aop_filter_call_pc_by_param(pc, 0, aop_t_all_signed());
+  aop_join_on(pc, plugin_join_on_advice, NULL);
+
+  return 0;
+}
+
+AOP_MAIN_PROTO aop_main()
+{
+  aop_register_pass("reinst3", plugin_reinst3);
+}

diff --git a/test/reinst-hooks.c b/test/reinst-hooks.c
new file mode 100644 (file)
index 0000000..9ba8b37
--- /dev/null
+++ b/test/reinst-hooks.c
@@ -0,0 +1,17 @@
+#include <stdio.h>
+
+void _advice_foo(int n)
+{
+  printf("In foo advice: %d\n", n);
+}
+
+void _advice_hook_bad(int n)
+{
+  printf("In bad hook advice: %d.  This advice should not have been called.\n", n);
+}
+
+void _advice_hook_good(int n)
+{
+  printf("In good hook advice: %d.\n", n);
+}
+

diff --git a/test/reinst-target.c b/test/reinst-target.c
new file mode 100644 (file)
index 0000000..3812c0d
--- /dev/null
+++ b/test/reinst-target.c
@@ -0,0 +1,11 @@
+#include <stdio.h>
+
+void foo(const char *str, int n)
+{
+  printf("%s %d\n", str, n);
+}
+
+void run_test()
+{
+  foo("THX", 1138);
+}

diff --git a/test/reinst.xml b/test/reinst.xml
new file mode 100644 (file)
index 0000000..5429487
--- /dev/null
+++ b/test/reinst.xml
@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!DOCTYPE testcase SYSTEM "testcase.dtd">
+
+<!-- In general, it should not be possible to instrument any statement
+  that is actually an InterAspect-inserted advice call.  There is one
+  exception: a second plug-in can instrument advice that a first
+  plug-in added. -->
+
+<testcase name="Instrumenting Advice">
+  <plugin id="plugin-reinst1" source="plugin-reinst1.c" />
+  <plugin id="plugin-reinst2" source="plugin-reinst2.c" />
+  <run name="Attempt to instrument advice" target="reinst-target.c" hooks="reinst-hooks.c">
+    <using plugin="plugin-reinst1" />
+    <output>
+      In foo advice: 1138
+      THX 1138
+    </output>
+  </run>
+  <run name="Instrument advice with second plug-in" target="reinst-target.c" hooks="reinst-hooks.c">
+    <using plugin="plugin-reinst1" />
+    <using plugin="plugin-reinst2" />
+    <output>
+      In good hook advice: 1138.
+      In foo advice: 1138
+      THX 1138
+    </output>
+  </run>
+</testcase>