[PATCH] fix alias cache breakage

[James Bottomley <James dot Bottomley at HansenPartnership dot com>]

commit 7bb73781111a053767a520ed45f1caeb8b9c9dd1
Author: Frank Ch. Eigler <fche@elastic.org>
Date:   Fri Jul 11 16:50:18 2008 -0400

    PR6739: speed up decl-alias cache by avoiding its recomputation;
move to per
-module/cu

Actually completely broke the alias cache.  The reason is that we have
to cross CUs to do resolution of DW_AT_declaration.  If it could be
resolved within the CU, the compiler wouldn't have stubbed the
declaration.

It seems that the intent of this change (although never discussed on the
mailing list) was to only run the cache addition once per CU in the
interests of efficiency.  This update actually fixes the alias cache
again, while preserving that goal.

I also updated the test to make it actually work (the reason why this
breakage was only spotted running a script, not running the tests).

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

---
 tapsets.cxx                              |   41 ++++++++++++++----------------
 testsuite/systemtap.base/declaration.exp |   15 ++++------
 2 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/tapsets.cxx b/tapsets.cxx
index adfe10e..f35c50f 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -1166,7 +1166,8 @@ struct dwflpp
    * cache is indexed by name.  If other declaration lookups were
    * added to it, it would have to be indexed by name and tag
    */
-  mod_cu_function_cache_t global_alias_cache;
+  cu_function_cache_t global_alias_cache;
+  mod_cu_function_cache_t global_alias_cache_lookup;
   static int global_alias_caching_callback(Dwarf_Die *die, void *arg)
   {
     cu_function_cache_t *cache = static_cast<cu_function_cache_t*>(arg);
@@ -1184,6 +1185,19 @@ struct dwflpp
     return DWARF_CB_OK;
   }
 
+  void update_alias_cache(void)
+  {
+    string key = module_name + ":" + cu_name;
+    cu_function_cache_t *v = global_alias_cache_lookup[key];
+
+    if (v)
+      // already seen this module and cu in the cache
+      return;
+
+    v = global_alias_cache_lookup[key] = &global_alias_cache;
+    iterate_over_globals(global_alias_caching_callback, v);
+  }
+
   Dwarf_Die *declaration_resolve(Dwarf_Die *die)
   {
     const char *name = dwarf_diename(die);
@@ -1191,29 +1205,10 @@ struct dwflpp
     if (!name)
       return NULL;
 
-    string key = module_name + ":" + cu_name;
-    cu_function_cache_t *v = global_alias_cache[key];
-    if (v == 0) // need to build the cache, just once per encountered module/cu
-      {
-        v = new cu_function_cache_t;
-        global_alias_cache[key] = v;
-        iterate_over_globals(global_alias_caching_callback, v);
-        if (sess.verbose > 4)
-          clog << "global alias cache " << key << " size " << v->size() << endl;
-      }
-
-    // XXX: it may be desirable to search other modules' declarations
-    // too, in case a module/shared-library processes a
-    // forward-declared pointer type only, where the actual definition
-    // may only be in vmlinux or the application.
-
-    // XXX: it is probably desirable to search other CU's declarations
-    // in the same module.
-    
-    if (v->find(name) == v->end())
+    if (global_alias_cache.find(name) == global_alias_cache.end())
       return NULL;
 
-    return & ((*v)[name]);
+    return &global_alias_cache[name];
   }
 
   mod_cu_function_cache_t cu_function_cache;
@@ -3605,6 +3600,8 @@ query_cu (Dwarf_Die * cudie, void * arg)
     {
       q->dw.focus_on_cu (cudie);
 
+      q->dw.update_alias_cache();
+
       if (false && q->sess.verbose>2)
         clog << "focused on CU '" << q->dw.cu_name
              << "', in module '" << q->dw.module_name << "'\n";
diff --git a/testsuite/systemtap.base/declaration.exp b/testsuite/systemtap.base/declaration.exp
index e47898a..2a9672c 100644
--- a/testsuite/systemtap.base/declaration.exp
+++ b/testsuite/systemtap.base/declaration.exp
@@ -3,17 +3,14 @@
 
 set TEST_NAME "empty-struct resolve-fail"
 
-# this test just makes sure $device is still stubbed with DW_AT_declaration
-set failscript {
-    probe\ module(\"libata\").function(\"ata_qc_issue\")\ \{\ print(\$qc->\$scsicmd->\$device->\$host->\$host_no)\ \};
-}
+# this test just makes sure device is still stubbed with DW_AT_declaration
+set failscript {probe module(\"libata\").function(\"ata_qc_issue\") { print(\$qc->scsicmd->device->host->host_no) }}
 
-stap_compile $TEST_NAME 0 $failscript
+stap_compile $TEST_NAME 0 [join [list {"} $failscript {"}]]
 
 set TEST_NAME "empty-struct resolve-pass"
 
-set passscript {
-    probe\ module(\"scsi_mod\").function(\"scsi_request_fn\")\ \{print(\$q)\}
-}
+# prepend a scsi_module command that adds the declaration of struct scsi_device
+set passscript [join [list {"probe module(\"scsi_mod\").function(\"scsi_request_fn\") {print(\$q)}} $failscript {"}]]
 
-stap_compile $TEST_NAME 1 [concat $passscript $failscript ]
+stap_compile $TEST_NAME 1 $passscript
-- 
1.5.6