This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [gold patch] Fix usage of invalidated version string in Symbol_table::define_special_symbol


Hm, you are right. I agree.

I have checked out all method calls in according how pversion is used
after it, just in case. I found the following problem

- possible usage of a NULL reference in
Symbol_table::add_undefined_symbol_from_command_line at 2535:
sym->init_undefined(name, version, elfcpp::STT_NOTYPE,
elfcpp::STB_GLOBAL, elfcpp::STV_DEFAULT, 0);

The similar methods (like do_define_as_constant & etc) do not have this
problem. Would you fix it also?

Viktor.

On Wed, 2011-09-28 at 16:02 -0700, Ian Lance Taylor wrote:
> Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:
> 
> > This patch fixes a problem with canonicalization of the symbols, but we
> > still have a problem with the invalidated version string. You left an
> > assignment of *pversion from v at its old place, but in case of true
> > condition at line 1679 we will get a pointer to an invalidated string in
> > *pversion by leaving the method.
> >
> > symtab.cc ---
> >   ...
> >   if (only_if_ref)
> >     {
> >       // Look up for the externaly specified or default (NULL) versions.
> >       oldsym = this->lookup(*pname, *pversion);
> >       // Look up for the script specified default version.
> >       if (oldsym == NULL && is_default_version)
> > 	oldsym = this->lookup(*pname, v.c_str());
> >       if (oldsym == NULL || !oldsym->is_undefined())
> > 	return NULL;	<<<<<<<< HERE !!!
> 
> But if this function returns NULL, the value stored in *pversion is
> irrelevant.  Isn't it?
> 
> Ian
> 
> > On Tue, 2011-09-27 at 17:55 -0700, Ian Lance Taylor wrote:
> >> Viktor Kutuzov <vkutuzov@accesssoftek.com> writes:
> >> 
> >> > I found that the Symbol_table::define_special_symbol method can return a
> >> > pointer to an invalidated version string from the local std::string
> >> > object (v) in case of condition only_if_ref == true. This patch should
> >> > fix this problem.
> >> >
> >> > -Viktor.
> >> >
> >> > 	* symtab.cc (Symbol_table::define_special_symbol): Fix usage of
> >> > 	the invalidated version string.
> >> 
> >> Thanks for the bug report and patch.  I don't think your approach is
> >> entirely correct.  If the version is the default, we still want to look
> >> up the symbol with a NULL version.  Also, if is_default_version is true
> >> we know that *pversion != NULL.  I think this patch will fix the
> >> problem.  Committed to mainline and 2.22 branch.
> >> 
> >> Ian
> >> 
> >> 
> >> 2011-09-27  Viktor Kutuzov  <vkutuzov@accesssoftek.com>
> >> 	    Ian Lance Taylor  <iant@google.com>
> >> 
> >> 	* symtab.cc (Symbol_table::define_special_symbol): Always
> >> 	canonicalize version string.
> >> 
> >> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]