Re: bk commit into 5.1 tree (davi:1.2644) BUG#31891

[Davi Arnaut]

Sergei Golubchik wrote:
> Hi!
> 
> On Dec 27, Davi Arnaut wrote:
>> Sergei Golubchik wrote:
>>
>>>> diff -Nrup a/sql/stacktrace.c b/sql/stacktrace.c
>>>> --- a/sql/stacktrace.c	2007-07-23 14:39:47 -03:00
>>>> +++ b/sql/stacktrace.c	2007-12-20 15:11:14 -02:00
>>>> @@ -93,9 +98,82 @@ inline uint32* find_prev_pc(uint32* pc, 
>>>>  }
>>>>  #endif /* defined(__alpha__) && defined(__GNUC__) */
>>>>  
>>>> +#if HAVE_BACKTRACE && HAVE_ABI_CXA_DEMANGLE &&
> HAVE_WEAK_SYMBOL
>>>> +#define BACKTRACE_DEMANGLE 1
>>>> +#endif
>>>> +
>>>> +#ifdef BACKTRACE_DEMANGLE
>>>> +
>>>> +char *__cxa_demangle(const char *mangled_name, char *output_buffer,
>>>> +                     unsigned int *length, int *status)
> __attribute__((__weak__));
>>>> +
>>>> +static char *my_demangle(const char *mangled_name, int *status)
>>>> +{
>>>> +  if (__cxa_demangle)
>>>> +    return __cxa_demangle(mangled_name, NULL, NULL, status);
>>>> +  else
>>>> +    return NULL;
>>>> +}
>>> Aren't you overdoing it ?
>>>
>>> Why are you doing configure check for __cxa_demangle and configure check
>>> for __weak__ and runtime detection of __cxa_demangle ?
>> The __weak__ symbol check is a compiler check and __cxa_demangle a
>> library one. Thus, for the sake of usefulness and modularity, I chose to
>> separate both.
> 
> I didn't mean one combined check. I mean either one of the three:
> 
> 1. *only* __cxa_demangle check in configure, that is only
> AC_CHECK_FUNC(__cxa_demangle). No weak symbols, no nothing. Only
> 
> #ifdef HAVE_CXA_DEMANGLE
>     return __cxa_demangle(mangled_name, NULL, NULL, status);
> #endif

The configure check for __cxa_demangle is for a *configure* time check
to hopefully ensure that in the future (runtime) we will use the
intended API/ABI (__cxa_demangle might be implemented with a different
"signature" in some other libraries).

> 
> 2. Only __weak__ check, and in the code you do
> 
> char *__cxa_demangle(const char *mangled_name, char *output_buffer,
>                      unsigned int *length, int *status) __attribute__((__weak__))
> {
>   return mangled_name;
> }
> 
> and then use __cxa_demangle without any checks.

This won't work the way you think.

> 3. same as 2. but without configure check for __weak__, using __GNUC__
> instead.

Works, but we loose a non-expensive configure time check.

Regards,

-- 
Davi Arnaut, Software Engineer
MySQL Inc, www.mysql.com

Are you MySQL certified?  www.mysql.com/certification