Hello Kazu, Thanks for reviewing the patch. I agree with your modifications, which is much better.
Thanks, Tao Liu On Fri, Apr 2, 2021 at 8:45 AM HAGIO KAZUHITO(萩尾 一仁) <[email protected]> wrote: > -----Original Message----- > > If a same extension(Eg: extensions/trace.so) with two different names > are loaded by > > "extend" command twice, it sometimes segfaults crash. > > > > It's because crash uses RTLD_NOW|RTLD_GLOBAL flags of dlopen to load an > extension. > > RTDL_GLOBAL will make symbols defined by this shared object available for > > symbol resolution of subsequently loaded shared objects. So symbols in > subsequently > > loaded extensions are overwritten by the former loaded one with a same > name. > > Not only can it cause segfaults, but some other unexpected behaviours. > > The phenomenon of the first paragraph is an example and a rare situation if > not on purpose, please let me change the order and generalize a bit more > here: > --- > The crash utility uses RTLD_NOW|RTLD_GLOBAL flags ... with the same name. > > This can cause unexpected behaviors when loading two extension modules that > have a symbol with the same name. For example, we can reproduce a > segmentation > violation by loading the current trace.so extension module with two > different > names. > --- > > I'll edit when applying. Otherwise, the patch looks good to me. > Please wait for another ack. > > Acked-by: Kazuhito Hagio <[email protected]> > > Thanks, > Kazu > > > > > This patch changes functions within extensions/echo.c to be static, and > documents > > the issue in code comments, for extensions developers who takes echo.c > as reference. > > > > Signed-off-by: Tao Liu <[email protected]> > > --- > > extensions/echo.c | 22 ++++++++++++++-------- > > 1 file changed, 14 insertions(+), 8 deletions(-) > > > > diff --git a/extensions/echo.c b/extensions/echo.c > > index ff1e09a..f4bb88f 100644 > > --- a/extensions/echo.c > > +++ b/extensions/echo.c > > @@ -17,19 +17,25 @@ > > > > #include "defs.h" /* From the crash source top-level directory */ > > > > -void echo_init(void); /* constructor function */ > > -void echo_fini(void); /* destructor function (optional) */ > > +static void echo_init(void); /* constructor function */ > > +static void echo_fini(void); /* destructor function (optional) */ > > > > -void cmd_echo(void); /* Declare the commands and their help data. */ > > -char *help_echo[]; > > +static void cmd_echo(void); /* Declare the commands and their help > data. */ > > +static char *help_echo[]; > > > > +/* > > + * Please making the functions and global variables static within your > > + * extension if you don't want to make them visiable to subsequently > > + * loaded extensions. Otherwise, non-static symbols within 2 extensions > and > > + * have a same name can cause confliction. > > + */ > > static struct command_table_entry command_table[] = { > > { "echo", cmd_echo, help_echo, 0}, /* One or more > commands, */ > > { NULL }, /* terminated by > NULL, */ > > }; > > > > > > -void __attribute__((constructor)) > > +static void __attribute__((constructor)) > > echo_init(void) /* Register the command set. */ > > { > > register_extension(command_table); > > @@ -39,7 +45,7 @@ echo_init(void) /* Register the command set. */ > > * This function is called if the shared object is unloaded. > > * If desired, perform any cleanups here. > > */ > > -void __attribute__((destructor)) > > +static void __attribute__((destructor)) > > echo_fini(void) { } > > > > > > @@ -49,7 +55,7 @@ echo_fini(void) { } > > * other crash commands for usage of the myriad of utility routines > available > > * to accomplish what your task. > > */ > > -void > > +static void > > cmd_echo(void) > > { > > int c; > > @@ -94,7 +100,7 @@ cmd_echo(void) > > * > > */ > > > > -char *help_echo[] = { > > +static char *help_echo[] = { > > "echo", /* command name */ > > "echoes back its arguments", /* short description */ > > "arg ...", /* argument synopsis, or " " if > none */ > > -- > > 2.29.2 > >
-- Crash-utility mailing list [email protected] https://listman.redhat.com/mailman/listinfo/crash-utility
