This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] gold: Implement --exclude-libs option
- From: Ian Lance Taylor <iant at google dot com>
- To: Doug Kwan (éæå) <dougkwan at google dot com>
- Cc: binutils at sourceware dot org
- Date: Tue, 19 May 2009 10:08:44 -0700
- Subject: Re: [PATCH] gold: Implement --exclude-libs option
- References: <498552560905190118u44e40037x421150a8abe5ed3e@mail.gmail.com>
"Doug Kwan (éæå)" <dougkwan@google.com> writes:
> 2009-05-19 Doug Kwan <dougkwan@google.com>
>
> * archive.cc (Archive::get_elf_object_for_member): Set no_export
> flag of object.
> * archive.h (option.h, parameters.h): New includes.
> (Archive::Archive): Set no_explort_.
> (Archive::no_export): New method.
> (Archive::no_export_): New field.
> * object.h (Object::Object): Initialize no_export_ to false.
> (Object::no_export, Object::set_no_export): New methods.
> (Object::no_export_): New field.
> * options.cc (General_options::parse_exclude_libs): New method.
> (General_options::check_excluded_libs) Same.
> * options.h (exclude_libs): New option.
> (General_options::check_excluded_libs): New method declaration.
> (General_options::excluded_libs_): New field.
> * symtab.cc (Symbol_table::add_from_relobj): Hide symbols with
> default or protected visibility if an object has no-export flag set.
> testsuite/Makefile.am (check_PROGRAMS): Add exclude_lib_test.
> (check_SCRIPTS): Add exclude_libs_test.sh.
> (check_DATA): Add exclude_libs_test.syms.
> (MOSTLYCLEANFILES): Add exlude_libs_test.syms,
> libexclude_libs_test_1.a and libexclude_libs_test_2.a.
> (exclude_libs_test_SOURCES, exclude_libs_test_DEPENDENDICES,
> exclude_libs_test_LDFLAGS and exclude_libs_test_LDADD): Define.
> (exclude_libs_test.syms, libexclude_libs_test_1.a,
> libexclude_libs_test_2.a): New rules.
> * testsuite/Makefile.in: Regenerate.
> * testsuite/exclude_libs_test.c: New file.
> * testsuite/exclude_libs_test_1.c: Ditto.
> * testsuite/exclude_libs_test_2.c: Ditto.
> * testsuite/exclude_libs_test_2.sh: Ditto.
> diff -Nrpu gold-cvs-3/src/gold/archive.cc gold-cvs-2/src/gold/archive.cc
> --- gold-cvs-3/src/gold/archive.cc 2009-03-24 11:42:10.000000000 -0700
> +++ gold-cvs-2/src/gold/archive.cc 2009-05-18 16:42:37.360972000 -0700
> @@ -549,10 +549,12 @@ Archive::get_elf_object_for_member(off_t
> return NULL;
> }
>
> - return make_elf_object((std::string(this->input_file_->filename())
> - + "(" + member_name + ")"),
> - input_file, memoff, ehdr, read_size,
> - punconfigured);
> + Object *obj = make_elf_object((std::string(this->input_file_->filename())
> + + "(" + member_name + ")"),
> + input_file, memoff, ehdr, read_size,
> + punconfigured);
> + obj->set_no_export(no_export());
> + return obj;
Gold is heavily templated, and in templates name lookup can be
unexpected. Therefore, gold code consistently refers to local fields
and methods using "this->". This is particular important for methods,
which do not have the trailing underscore. So, this should me
obj->set_no_export(this->no_export());
> diff -Nrpu gold-cvs-3/src/gold/archive.h gold-cvs-2/src/gold/archive.h
> --- gold-cvs-3/src/gold/archive.h 2009-03-13 22:56:46.000000000 -0700
> +++ gold-cvs-2/src/gold/archive.h 2009-05-19 00:06:45.165003000 -0700
> @@ -27,6 +27,8 @@
> #include <vector>
>
> #include "fileread.h"
> +#include "options.h"
> +#include "parameters.h"
Instead of doing this, just move the constructor into archive.cc.
> @@ -54,7 +56,10 @@ class Archive
> extended_names_(), armap_checked_(), seen_offsets_(), members_(),
> is_thin_archive_(is_thin_archive), included_member_(false),
> nested_archives_(), dirpath_(dirpath), task_(task), num_members_(0)
> - { }
> + {
> + no_export_ =
> + parameters->options().check_excluded_libs(input_file->found_name());
> + }
this->no_export_ = ...
> +void
> +General_options::parse_exclude_libs(const char*, const char* arg,
> + Command_line*)
> +{
> + const char *p = arg, *end;
> +
> + while (*p != '\0')
> + {
> + end = strpbrk(p, ",:");
> + if (end == NULL)
> + end = p + strlen(p);
> + size_t length = end - p;
> + std::string s(p, length);
> + excluded_libs_.insert(s);
> + if (*end == '\0')
> + break;
> + p = end + 1;
> + }
> +}
This function needs a comment. In particular, the comment should say
that the implementation is based on what the GNU linker does.
This is pretty minor, but this code would be simpler if you used strcspn
instead of strpbrk.
while (*p != '\0')
{
size_t length = strcspn(p, ",:");
this->excluded_libs_.insert(std::string(p, length));
p += length;
}
> +bool
> +General_options::check_excluded_libs (const std::string &name) const
This function needs a comment referring to the GNU linker, since
otherwise the behaviour seems kind of weird.
This is OK with those changes.
Thanks.
Ian