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: Linker plugins should be aware of --defsym during symbol resolution


> include/
> * plugin-api.h (LDPR_LINKER_REDEFINED): New enum value.

As always, this change needs to be synced with the GCC tree, and the
whopr/driver wiki page needs updating.

Can you explain why this new resolution is needed? Why would
LDPR_PREEMPTED_REG not work?

As we did with LDPR_PREVAILING_DEF_IRONLY_EXP and LDPS_NO_SYMS, you'll
need to create a new version of get_symbols (LDPT_GET_SYMBOLS_V4), so
that old plugins that aren't prepared for the new resolution enum
don't break.

Hmmm, in get_symbol_resolution_info, we have this:

  if (nsyms > this->nsyms_)
    return LDPS_NO_SYMS;

That needs to be qualified with a test for version > 2 (as was done a
few lines below).

> gold/
> * expression.cc (Symbol_expression::is_symbol_in_expression):
>       New method.
> (Unary_expression::is_symbol_in_expression): New method.
> (Binary_expression::is_symbol_in_expression): New method.
> (Trinary_expression::is_symbol_in_expression): New method.
> * plugin.cc (is_referenced_from_outside): Check if the symbol is
> used in a defsym expression.
> (get_symbol_resolution_info): Fix symbol resolution if defined or
> used in defsyms.
> * script.cc (Script_options::is_defsym_def): New method.
> (Script_options::is_defsym_use): New method.
> * script.h (Expression::is_symbol_in_expression): New method.
> (Symbol_assignment::is_defsym): New method.
> (Symbol_assignment::value): New method.
> (Script_options::is_defsym_def): New method.
> (Script_options::is_defsym_use): New method.
> * testsuite/Makefile.am (plugin_test_defsym): New test.
> * testsuite/Makefile.in: Regenerate.
> * testsuite/plugin_test.c: Check for new symbol resolution.
> * testsuite/plugin_test_defsym.sh: New script.
> * testsuite/plugin_test_defsym.c: New test source.


+bool
+Script_options::is_defsym_use(const char* name)
+{
+  for (Symbol_assignments::iterator p = this->symbol_assignments_.begin();
+       p != this->symbol_assignments_.end();
+       ++p)
+    {
+      if ((*p)->is_defsym() && (*p)->value()->is_symbol_in_expression(name))
+        return true;
+    }
+  return false;
+}

This looks like a pretty expensive test. Wouldn't it be better to just
call Symbol::set_in_real_elf() for each symbol we see used in an
expression?

+  bool
+  is_symbol_in_expression(const char* name)
+  {
+    return (this->left_->is_symbol_in_expression(name) ||
+            this->right_->is_symbol_in_expression(name));
+  }

Operators should be moved to the beginning of the next line.

+  bool
+  is_symbol_in_expression(const char* name)
+  { return (this->arg1_->is_symbol_in_expression(name) ||
+     this->arg2_->is_symbol_in_expression(name) ||
+     this->arg3_->is_symbol_in_expression(name)); }

Same, and the braces should be on their own lines here.

-cary


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