This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
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.
> >>
> >>