Re: [DynInst_API:] [PATCH 1/2] common: rewrite caching in Linux P_cplus_demangle()

[Bill Williams <bill@xxxxxxxxxxx>]

On 07/30/2015 05:28 PM, Josh Stone wrote:
> This had a bug where a last_typed value was saved even for parameter
> includeTypes==false, where cplus_demangle opts lacked DMGL_PARAMS.
> Similarly, the nativeCompiler parameter wasn't considered at all for
> caching.
>
> This caused strange issues in boost::multi_index, where its internal
> rehashing was sometimes allocating buffers incorrectly for its buckets.
> This was because Symbol->getTypedName() was returning different values
> depending on how P_cplus_demangle was cached, and silently changing keys
> in a map is no good at all.
>
> P_cplus_demangle caching now requires an exact match on all three input
> parameters before returning the saved result.
Approved, please apply. Do you happen to have a simple test case for 
reproducing the bad behavior here?
---
>   common/src/linuxKludges.C | 55 ++++++++++++++++++++++-------------------------
>   1 file changed, 26 insertions(+), 29 deletions(-)
>
> diff --git a/common/src/linuxKludges.C b/common/src/linuxKludges.C
> index 1fde2df1f2d6..f494c836836e 100644
> --- a/common/src/linuxKludges.C
> +++ b/common/src/linuxKludges.C
> @@ -167,30 +167,17 @@ using namespace abi;
>   char * P_cplus_demangle( const char * symbol, bool nativeCompiler,
>   				bool includeTypes )
>   {
> -  static char* last_symbol = NULL, *last_typed = NULL, *last_pretty = NULL;
> +  static char* last_symbol = NULL;
> +  static bool last_native = false;
> +  static bool last_typed = false;
> +  static char* last_demangled = NULL;
>
> -  if(last_symbol && (strcmp(symbol, last_symbol) == 0))
> +  if(last_symbol && last_demangled && (nativeCompiler == last_native)
> +      && (includeTypes == last_typed) && (strcmp(symbol, last_symbol) == 0))
>     {
> -    if(includeTypes && last_typed)
> -    {
> -      return strdup(last_typed);
> -    }
> -    else if(last_pretty)
> -    {
> -      return strdup(last_pretty);
> -    }
> +      return strdup(last_demangled);
>     }
>
> -
> -
> -   int opts = 0;
> -   opts |= includeTypes ? DMGL_PARAMS | DMGL_ANSI : 0;
> -   //   [ pgcc/CC are the "native" compilers on Linux. Go figure. ]
> -   // pgCC's mangling scheme most closely resembles that of the Annotated
> -   // C++ Reference Manual, only with "some exceptions" (to quote the PGI
> -   // documentation). I guess we'll demangle names with "some exceptions".
> -   opts |= nativeCompiler ? DMGL_ARM : 0;
> -
>   #if defined(cap_gnu_demangler)
>      int status;
>      char *demangled = __cxa_demangle(symbol, NULL, NULL, &status);
> @@ -204,14 +191,18 @@ char * P_cplus_demangle( const char * symbol, bool nativeCompiler,
>      }
>      assert(status == 0); //Success
>   #else
> +   int opts = 0;
> +   opts |= includeTypes ? DMGL_PARAMS | DMGL_ANSI : 0;
> +   //   [ pgcc/CC are the "native" compilers on Linux. Go figure. ]
> +   // pgCC's mangling scheme most closely resembles that of the Annotated
> +   // C++ Reference Manual, only with "some exceptions" (to quote the PGI
> +   // documentation). I guess we'll demangle names with "some exceptions".
> +   opts |= nativeCompiler ? DMGL_ARM : 0;
>      char * demangled = cplus_demangle( const_cast< char *>(symbol), opts);
>   #endif
> +
>      if( demangled == NULL ) { return NULL; }
> -   free(last_typed);
> -   free(last_symbol);
> -   free(last_pretty);
> -   last_symbol = strdup(symbol);
> -   last_typed = strdup(demangled);
> +
>      if( ! includeTypes ) {
>           /* de-demangling never increases the length */
>           char * dedemangled = strdup( demangled );
> @@ -220,10 +211,16 @@ char * P_cplus_demangle( const char * symbol, bool nativeCompiler,
>           assert( dedemangled != NULL );
>
>           free( demangled );
> -	last_pretty = strdup(dedemangled);
> -        return dedemangled;
> -        }
> -   last_pretty = NULL;
> +        demangled = dedemangled;
> +   }
> +
> +   free(last_symbol);
> +   free(last_demangled);
> +   last_native = nativeCompiler;
> +   last_typed = includeTypes;
> +   last_symbol = strdup(symbol);
> +   last_demangled = strdup(demangled);
> +
>      return demangled;
>   } /* end P_cplus_demangle() */

--
--bw

Bill Williams
Paradyn Project
bill@xxxxxxxxxxx