maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Vicentiu!

Looks mostly ok. Does it mean, it will replace my changes in 10.0+
related to calculating the offset?

    struct link_map *lm = (struct link_map*) dlopen(0, RTLD_NOW);

etc

As far as LIBDL and dependencies are concerned...

1. Don't add LIBDL to the link list. The server has it and clients
should not link with my_addr_resove (and it's good to get a linker
error, if by some mistake they do)

2. Check for dladdr in configure.cmake:

  CHECK_FUNCTION_EXISTS (dladdr HAVE_DLADDR)

3. handle dladdr in my_global.h (search for dlopen or dlerror)

But, I think, you won't need any ifdefs in my_addr_resolve.c,
because my_global.h will take care of that.

Ok to push after that, thanks!

On Jan 16, vicentiu@xxxxxxxxxxx wrote:
> revision-id: 6ffd535e415028f414b9d1c9e9efbfb81bf25798 (mariadb-5.5.58-42-g6ffd535e415)
> parent(s): 6267be460ab5147e3bc0fffd03e690b3650f2fe2
> author: Vicențiu Ciorbaru
> committer: Vicențiu Ciorbaru
> timestamp: 2018-01-16 22:57:52 +0200
> message:
> 
> MDEV-14229: Stack trace is not resolved for shared objects
> 
> Resolving a stacktrace including functions in dynamic libraries requires
> us to look inside the libraries for the symbols. Addr2line needs to be
> started with the correct binary for each address on the stack. To do this,
> figure out which library it is using dladdr, then if the addr2line
> binary was started with a different binary, fork it again with the
> correct one.
> 
> We only have one addr2line process running at any point during the
> stacktrace resolving step. The maximum number of forks for addr2line should
> generally be around 6.
> 
> One for server stacktrace code, one for plugin code, one when going back
> into server code, one for pthread library, one for libc, one for the
> _start function in the server. More can come up if plugin calls server
> function which goes back to a plugin, etc.
> 
> ---
>  mysys/CMakeLists.txt    |  2 +-
>  mysys/my_addr_resolve.c | 97 +++++++++++++++++++++++++++++++++----------------
>  2 files changed, 67 insertions(+), 32 deletions(-)
> 
> diff --git a/mysys/CMakeLists.txt b/mysys/CMakeLists.txt
> index cb86850c2de..15f3fbad9bf 100644
> --- a/mysys/CMakeLists.txt
> +++ b/mysys/CMakeLists.txt
> @@ -67,7 +67,7 @@ ENDIF()
>  
>  ADD_CONVENIENCE_LIBRARY(mysys ${MYSYS_SOURCES})
>  TARGET_LINK_LIBRARIES(mysys dbug strings ${ZLIB_LIBRARY} 
> - ${LIBNSL} ${LIBM} ${LIBRT} ${LIBSOCKET} ${LIBEXECINFO})
> +  ${LIBNSL} ${LIBM} ${LIBRT} ${LIBSOCKET} ${LIBEXECINFO} ${LIBDL})
>  DTRACE_INSTRUMENT(mysys)
>  
>  IF(HAVE_BFD_H)
> diff --git a/mysys/my_addr_resolve.c b/mysys/my_addr_resolve.c
> index 90e6f43f390..f831ad5121f 100644
> --- a/mysys/my_addr_resolve.c
> +++ b/mysys/my_addr_resolve.c
> @@ -132,15 +132,79 @@ const char *my_addr_resolve_init()
>  
>  #include <m_string.h>
>  #include <ctype.h>
> +
> +#include <sys/wait.h>
> +
>  static int in[2], out[2];
> -static int initialized= 0;
> +static pid_t pid;
> +static char addr2line_binary[1024];
>  static char output[1024];
> +
> +int start_addr2line_fork(const char *binary_path)
> +{
> +
> +  if (pid > 0)
> +  {
> +    /* Don't leak FDs */
> +    close(in[1]);
> +    close(out[0]);
> +    /* Don't create zombie processes. */
> +    waitpid(pid, NULL, 0);
> +  }
> +
> +  if (pipe(in) < 0)
> +    return 1;
> +  if (pipe(out) < 0)
> +    return 1;
> +
> +  pid = fork();
> +  if (pid == -1)
> +    return 1;
> +
> +  if (!pid) /* child */
> +  {
> +    dup2(in[0], 0);
> +    dup2(out[1], 1);
> +    close(in[0]);
> +    close(in[1]);
> +    close(out[0]);
> +    close(out[1]);
> +    execlp("addr2line", "addr2line", "-C", "-f", "-e", binary_path, NULL);
> +    exit(1);
> +  }
> +
> +  close(in[0]);
> +  close(out[1]);
> +
> +  return 0;
> +}
> +
>  int my_addr_resolve(void *ptr, my_addr_loc *loc)
>  {
>    char input[32], *s;
>    size_t len;
>  
> -  len= my_snprintf(input, sizeof(input), "%p\n", ptr);
> +  Dl_info info;
> +  void *offset;
> +
> +  if (!dladdr(ptr, &info))
> +    return 1;
> +
> +  if (strcmp(addr2line_binary, info.dli_fname))
> +  {
> +    /* We use dli_fname in case the path is longer than the length of our static
> +       string. We don't want to allocate anything dynamicaly here as we are in
> +       a "crashed" state. */
> +    if (start_addr2line_fork(info.dli_fname))
> +    {
> +      addr2line_binary[0] = '\0';
> +      return 1;
> +    }
> +    /* Save result for future comparisons. */
> +    strnmov(addr2line_binary, info.dli_fname, sizeof(addr2line_binary));
> +  }
> +  offset = info.dli_fbase;
> +  len= my_snprintf(input, sizeof(input), "%p\n", ptr - offset);
>    if (write(in[1], input, len) <= 0)
>      return 1;
>    if (read(out[0], output, sizeof(output)) <= 0)
> @@ -168,35 +232,6 @@ int my_addr_resolve(void *ptr, my_addr_loc *loc)
>  
>  const char *my_addr_resolve_init()
>  {
> -  if (!initialized)
> -  {
> -    pid_t pid;
> -
> -    if (pipe(in) < 0)
> -      return "pipe(in)";
> -    if (pipe(out) < 0)
> -      return "pipe(out)";
> -
> -    pid = fork();
> -    if (pid == -1)
> -      return "fork";
> -
> -    if (!pid) /* child */
> -    {
> -      dup2(in[0], 0);
> -      dup2(out[1], 1);
> -      close(in[0]);
> -      close(in[1]);
> -      close(out[0]);
> -      close(out[1]);
> -      execlp("addr2line", "addr2line", "-C", "-f", "-e", my_progname, NULL);
> -      exit(1);
> -    }
> -    
> -    close(in[0]);
> -    close(out[1]);
> -    initialized= 1;
> -  }
>    return 0;
>  }
>  #endif

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx