From a55ce7febfaa52670ce3d9c236d3033de80ac091 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 14 Nov 2007 01:03:01 +0000 Subject: [PATCH] From Craig Silverstein: Rework debug info code a bit, add option for ODR violations, add test case. --- gold/dwarf_reader.cc | 68 +++++++++++++++++++++++++++++++--------- gold/dwarf_reader.h | 38 +++++++++++++++++++--- gold/object.cc | 2 +- gold/options.cc | 4 +++ gold/options.h | 10 ++++++ gold/parameters.cc | 3 +- gold/parameters.h | 10 ++++++ gold/resolve.cc | 3 +- gold/symtab.cc | 52 ++---------------------------- gold/testsuite/Makefile.am | 8 +++-- gold/testsuite/Makefile.in | 8 +++-- gold/testsuite/debug_msg.cc | 12 +++++++ gold/testsuite/debug_msg.sh | 5 +++ gold/testsuite/odr_violation1.cc | 12 +++++++ gold/testsuite/odr_violation2.cc | 14 +++++++++ 15 files changed, 174 insertions(+), 75 deletions(-) create mode 100644 gold/testsuite/odr_violation1.cc create mode 100644 gold/testsuite/odr_violation2.cc diff --git a/gold/dwarf_reader.cc b/gold/dwarf_reader.cc index 4e7217b..d07e3e7 100644 --- a/gold/dwarf_reader.cc +++ b/gold/dwarf_reader.cc @@ -25,6 +25,7 @@ #include "elfcpp_swap.h" #include "dwarf.h" #include "object.h" +#include "parameters.h" #include "reloc.h" #include "dwarf_reader.h" @@ -117,7 +118,7 @@ ResetLineStateMachine(struct LineStateMachine* lsm, bool default_is_stmt) } template -Dwarf_line_info::Dwarf_line_info(Object* object) +Sized_dwarf_line_info::Sized_dwarf_line_info(Object* object) : data_valid_(false), buffer_(NULL), symtab_buffer_(NULL), directories_(), files_(), current_header_index_(-1) { @@ -177,7 +178,7 @@ Dwarf_line_info::Dwarf_line_info(Object* object) template const unsigned char* -Dwarf_line_info::read_header_prolog( +Sized_dwarf_line_info::read_header_prolog( const unsigned char* lineptr) { uint32_t initial_length = elfcpp::Swap<32, big_endian>::readval(lineptr); @@ -238,7 +239,7 @@ Dwarf_line_info::read_header_prolog( template const unsigned char* -Dwarf_line_info::read_header_tables( +Sized_dwarf_line_info::read_header_tables( const unsigned char* lineptr) { ++this->current_header_index_; @@ -311,7 +312,7 @@ Dwarf_line_info::read_header_tables( template bool -Dwarf_line_info::process_one_opcode( +Sized_dwarf_line_info::process_one_opcode( const unsigned char* start, struct LineStateMachine* lsm, size_t* len) { size_t oplen = 0; @@ -490,7 +491,7 @@ Dwarf_line_info::process_one_opcode( template unsigned const char* -Dwarf_line_info::read_lines(unsigned const char* lineptr) +Sized_dwarf_line_info::read_lines(unsigned const char* lineptr) { struct LineStateMachine lsm; @@ -530,7 +531,7 @@ Dwarf_line_info::read_lines(unsigned const char* lineptr) template unsigned int -Dwarf_line_info::symbol_section( +Sized_dwarf_line_info::symbol_section( unsigned int sym, typename elfcpp::Elf_types::Elf_Addr* value) { @@ -545,7 +546,7 @@ Dwarf_line_info::symbol_section( template void -Dwarf_line_info::read_relocs() +Sized_dwarf_line_info::read_relocs() { if (this->symtab_buffer_ == NULL) return; @@ -565,7 +566,7 @@ Dwarf_line_info::read_relocs() template void -Dwarf_line_info::read_line_mappings() +Sized_dwarf_line_info::read_line_mappings() { gold_assert(this->data_valid_ == true); @@ -595,7 +596,7 @@ Dwarf_line_info::read_line_mappings() template bool -Dwarf_line_info::input_is_relobj() +Sized_dwarf_line_info::input_is_relobj() { // Only .o files have relocs and the symtab buffer that goes with them. return this->symtab_buffer_ != NULL; @@ -606,7 +607,8 @@ Dwarf_line_info::input_is_relobj() template std::string -Dwarf_line_info::addr2line(unsigned int shndx, off_t offset) +Sized_dwarf_line_info::do_addr2line(unsigned int shndx, + off_t offset) { if (this->data_valid_ == false) return ""; @@ -669,24 +671,62 @@ Dwarf_line_info::addr2line(unsigned int shndx, off_t offset) return ret; } +// Dwarf_line_info routines. + +// Note: this routine instantiates the appropriate +// Sized_dwarf_line_info templates for this config, so we don't have +// to have a separte instantiation section for them. + +std::string +Dwarf_line_info::one_addr2line(Object* object, + unsigned int shndx, off_t offset) +{ + if (parameters->get_size() == 32 && !parameters->is_big_endian()) +#ifdef HAVE_TARGET_32_LITTLE + return Sized_dwarf_line_info<32, false>(object).addr2line(shndx, offset); +#else + gold_unreachable(); +#endif + else if (parameters->get_size() == 32 && parameters->is_big_endian()) +#ifdef HAVE_TARGET_32_BIG + return Sized_dwarf_line_info<32, true>(object).addr2line(shndx, offset); +#else + gold_unreachable(); +#endif + else if (parameters->get_size() == 64 && !parameters->is_big_endian()) +#ifdef HAVE_TARGET_64_LITTLE + return Sized_dwarf_line_info<64, false>(object).addr2line(shndx, offset); +#else + gold_unreachable(); +#endif + else if (parameters->get_size() == 64 && parameters->is_big_endian()) +#ifdef HAVE_TARGET_64_BIT + return Sized_dwarf_line_info<64, true>(object).addr2line(shndx, offset); +#else + gold_unreachable(); +#endif + else + gold_unreachable(); +} + #ifdef HAVE_TARGET_32_LITTLE template -class Dwarf_line_info<32, false>; +class Sized_dwarf_line_info<32, false>; #endif #ifdef HAVE_TARGET_32_BIG template -class Dwarf_line_info<32, true>; +class Sized_dwarf_line_info<32, true>; #endif #ifdef HAVE_TARGET_64_LITTLE template -class Dwarf_line_info<64, false>; +class Sized_dwarf_line_info<64, false>; #endif #ifdef HAVE_TARGET_64_BIG template -class Dwarf_line_info<64, true>; +class Sized_dwarf_line_info<64, true>; #endif } // End namespace gold. diff --git a/gold/dwarf_reader.h b/gold/dwarf_reader.h index d35cbf1..cefc027 100644 --- a/gold/dwarf_reader.h +++ b/gold/dwarf_reader.h @@ -41,21 +41,51 @@ struct LineStateMachine; // This class is used to read the line information from the debugging // section of an object file. -template class Dwarf_line_info { public: - // Initializes a .debug_line reader for a given object file. - Dwarf_line_info(Object* object); + Dwarf_line_info() + { } + + virtual + ~Dwarf_line_info() + { } // Given a section number and an offset, returns the associated // file and line-number, as a string: "file:lineno". If unable // to do the mapping, returns the empty string. You must call // read_line_mappings() before calling this function. std::string - addr2line(unsigned int shndx, off_t offset); + addr2line(unsigned int shndx, off_t offset) + { return do_addr2line(shndx, offset); } + + // A helper function for a single addr2line lookup. It uses + // parameters() to figure out the size and endianness. This is less + // efficient than using the templatized size and endianness, so only + // call this from an un-templatized context. + static std::string + one_addr2line(Object* object, unsigned int shndx, off_t offset); + + private: + virtual std::string + do_addr2line(unsigned int shndx, off_t offset) = 0; +}; + +template +class Sized_dwarf_line_info +{ + public: + // Initializes a .debug_line reader for a given object file. + Sized_dwarf_line_info(Object* object); + + std::string + addr2line(unsigned int shndx, off_t offset) + { return do_addr2line(shndx, offset); } private: + std::string + do_addr2line(unsigned int shndx, off_t offset); + // Start processing line info, and populates the offset_map_. void read_line_mappings(); diff --git a/gold/object.cc b/gold/object.cc index 608226f..c0e2acd 100644 --- a/gold/object.cc +++ b/gold/object.cc @@ -1093,7 +1093,7 @@ Relocate_info::location(size_t, off_t offset) const std::string filename; std::string file_and_lineno; // Better than filename-only, if available. - Dwarf_line_info line_info(this->object); + Sized_dwarf_line_info line_info(this->object); // This will be "" if we failed to parse the debug info for any reason. file_and_lineno = line_info.addr2line(this->data_shndx, offset); diff --git a/gold/options.cc b/gold/options.cc index 9a0b9f8..d4d9e16 100644 --- a/gold/options.cc +++ b/gold/options.cc @@ -353,6 +353,9 @@ options::Command_line_options::options[] = &Position_dependent_options::set_static_search), GENERAL_NOARG('\0', "Bsymbolic", N_("Bind defined symbols locally"), NULL, ONE_DASH, &General_options::set_symbolic), + GENERAL_NOARG('\0', "detect-odr-violations", + N_("Try to detect violations of the One Definition Rule"), + NULL, TWO_DASHES, &General_options::set_detect_odr_violations), GENERAL_NOARG('E', "export-dynamic", N_("Export all dynamic symbols"), NULL, TWO_DASHES, &General_options::set_export_dynamic), GENERAL_NOARG('\0', "eh-frame-hdr", N_("Create exception frame header"), @@ -473,6 +476,7 @@ General_options::General_options() is_relocatable_(false), strip_(STRIP_NONE), symbolic_(false), + detect_odr_violations_(false), create_eh_frame_hdr_(false), rpath_(), rpath_link_(), diff --git a/gold/options.h b/gold/options.h index 4b774ac..c54af77 100644 --- a/gold/options.h +++ b/gold/options.h @@ -153,6 +153,11 @@ class General_options symbolic() const { return this->symbolic_; } + // --detect-odr-violations: Whether to search for One Defn Rule violations. + bool + detect_odr_violations() const + { return this->detect_odr_violations_; } + // --eh-frame-hdr: Whether to generate an exception frame header. bool create_eh_frame_hdr() const @@ -300,6 +305,10 @@ class General_options { this->symbolic_ = true; } void + set_detect_odr_violations() + { this->detect_odr_violations_ = true; } + + void set_create_eh_frame_hdr() { this->create_eh_frame_hdr_ = true; } @@ -412,6 +421,7 @@ class General_options bool is_relocatable_; Strip strip_; bool symbolic_; + bool detect_odr_violations_; bool create_eh_frame_hdr_; Dir_list rpath_; Dir_list rpath_link_; diff --git a/gold/parameters.cc b/gold/parameters.cc index 52deac0..f332134 100644 --- a/gold/parameters.cc +++ b/gold/parameters.cc @@ -33,7 +33,7 @@ namespace gold Parameters::Parameters(Errors* errors) : errors_(errors), output_file_name_(NULL), output_file_type_(OUTPUT_INVALID), sysroot_(), - strip_(STRIP_INVALID), symbolic_(false), + strip_(STRIP_INVALID), symbolic_(false), detect_odr_violations_(false), optimization_level_(0), export_dynamic_(false), is_doing_static_link_valid_(false), doing_static_link_(false), is_size_and_endian_valid_(false), size_(0), is_big_endian_(false) @@ -48,6 +48,7 @@ Parameters::set_from_options(const General_options* options) this->output_file_name_ = options->output_file_name(); this->sysroot_ = options->sysroot(); this->symbolic_ = options->symbolic(); + this->detect_odr_violations_ = options->detect_odr_violations(); this->optimization_level_ = options->optimization_level(); this->export_dynamic_ = options->export_dynamic(); diff --git a/gold/parameters.h b/gold/parameters.h index 79545ac..353f01f 100644 --- a/gold/parameters.h +++ b/gold/parameters.h @@ -121,6 +121,14 @@ class Parameters return this->symbolic_; } + // Whether we should try to detect violations of the One Definition Rule. + bool + detect_odr_violations() const + { + gold_assert(this->options_valid_); + return this->detect_odr_violations_; + } + // The general linker optimization level. int optimization_level() const @@ -218,6 +226,8 @@ class Parameters Strip strip_; // Whether we are doing a symbolic link. bool symbolic_; + // Whether we try to detect One Definition Rule violations. + bool detect_odr_violations_; // The optimization level. int optimization_level_; // Whether the -E/--export-dynamic flag is set. diff --git a/gold/resolve.cc b/gold/resolve.cc index 980831b..b3328d5 100644 --- a/gold/resolve.cc +++ b/gold/resolve.cc @@ -243,7 +243,8 @@ Symbol_table::resolve(Sized_symbol* to, // is an ODR violation. But it's helpful to warn about.) // We use orig_sym here because we want the symbol exactly as it // appears in the object file, not munged via our future processing. - if (orig_sym.get_st_bind() == elfcpp::STB_WEAK + if (parameters->detect_odr_violations() + && orig_sym.get_st_bind() == elfcpp::STB_WEAK && to->binding() == elfcpp::STB_WEAK && orig_sym.get_st_shndx() != elfcpp::SHN_UNDEF && to->shndx() != elfcpp::SHN_UNDEF diff --git a/gold/symtab.cc b/gold/symtab.cc index be88534..9bee283 100644 --- a/gold/symtab.cc +++ b/gold/symtab.cc @@ -1808,54 +1808,6 @@ Symbol_table::sized_write_section_symbol(const Output_section* os, void Symbol_table::detect_odr_violations() const { - if (parameters->get_size() == 32) - { - if (!parameters->is_big_endian()) - { -#ifdef HAVE_TARGET_32_LITTLE - this->sized_detect_odr_violations<32, false>(); -#else - gold_unreachable(); -#endif - } - else - { -#ifdef HAVE_TARGET_32_BIG - this->sized_detect_odr_violations<32, true>(); -#else - gold_unreachable(); -#endif - } - } - else if (parameters->get_size() == 64) - { - if (!parameters->is_big_endian()) - { -#ifdef HAVE_TARGET_64_LITTLE - this->sized_detect_odr_violations<64, false>(); -#else - gold_unreachable(); -#endif - } - else - { -#ifdef HAVE_TARGET_64_BIG - this->sized_detect_odr_violations<64, true>(); -#else - gold_unreachable(); -#endif - } - } - else - gold_unreachable(); -} - -// Implement detect_odr_violations. - -template -void -Symbol_table::sized_detect_odr_violations() const -{ for (Odr_map::const_iterator it = candidate_odr_violations_.begin(); it != candidate_odr_violations_.end(); ++it) @@ -1874,9 +1826,9 @@ Symbol_table::sized_detect_odr_violations() const // one Task for object, plus appropriate locking to ensure // that we don't conflict with other uses of the object. locs->object->lock(); - Dwarf_line_info line_info(locs->object); + std::string lineno = Dwarf_line_info::one_addr2line( + locs->object, locs->shndx, locs->offset); locs->object->unlock(); - std::string lineno = line_info.addr2line(locs->shndx, locs->offset); if (!lineno.empty()) line_nums.insert(lineno); } diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am index 4ac2e5c..e193fc5 100644 --- a/gold/testsuite/Makefile.am +++ b/gold/testsuite/Makefile.am @@ -118,8 +118,12 @@ if GCC debug_msg.o: debug_msg.cc $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/debug_msg.cc -debug_msg.err: debug_msg.o - if $(CXXLINK) -Bgcctestdir/ -o debug_msg debug_msg.o 2>$@; \ +odr_violation1.o: odr_violation1.cc + $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation1.cc +odr_violation2.o: odr_violation2.cc + $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation2.cc +debug_msg.err: debug_msg.o odr_violation1.o odr_violation2.o + if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg debug_msg.o odr_violation1.o odr_violation2.o 2>$@; \ then \ echo 2>&1 "Link of debug_msg.o should have failed"; \ exit 1; \ diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in index 44214f1..dcbbf52 100644 --- a/gold/testsuite/Makefile.in +++ b/gold/testsuite/Makefile.in @@ -1206,8 +1206,12 @@ uninstall-am: uninstall-info-am @GCC_TRUE@debug_msg.o: debug_msg.cc @GCC_TRUE@ $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/debug_msg.cc -@GCC_TRUE@debug_msg.err: debug_msg.o -@GCC_TRUE@ if $(CXXLINK) -Bgcctestdir/ -o debug_msg debug_msg.o 2>$@; \ +@GCC_TRUE@odr_violation1.o: odr_violation1.cc +@GCC_TRUE@ $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation1.cc +@GCC_TRUE@odr_violation2.o: odr_violation2.cc +@GCC_TRUE@ $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation2.cc +@GCC_TRUE@debug_msg.err: debug_msg.o odr_violation1.o odr_violation2.o +@GCC_TRUE@ if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg debug_msg.o odr_violation1.o odr_violation2.o 2>$@; \ @GCC_TRUE@ then \ @GCC_TRUE@ echo 2>&1 "Link of debug_msg.o should have failed"; \ @GCC_TRUE@ exit 1; \ diff --git a/gold/testsuite/debug_msg.cc b/gold/testsuite/debug_msg.cc index ab73a8d..45a0be6 100644 --- a/gold/testsuite/debug_msg.cc +++ b/gold/testsuite/debug_msg.cc @@ -55,6 +55,10 @@ class Derived : public Base virtual void virtfn() { undef_fn2(); } }; +// This tests One Definition Rule (ODR) violations. +void SortAscending(int array[], int size); // in odr_violation1.cc +void SortDescending(int array[], int size); // in odr_violation2.cc + int main() { testfn(5); @@ -63,5 +67,13 @@ int main() Base b; Derived d; + int kInput1[] = {1, 6, 9, 7, 3, 4, 2, 10, 5, 8}; + int kSize1 = sizeof(kInput1) / sizeof(int); + SortAscending(kInput1, kSize1); + + int kInput2[] = {1, 6, 9, 7, 3, 4, 2, 10, 5, 8}; + int kSize2 = sizeof(kInput2) / sizeof(int); + SortDescending(kInput2, kSize2); + return 0; } diff --git a/gold/testsuite/debug_msg.sh b/gold/testsuite/debug_msg.sh index 53e9d88..26c8ded 100755 --- a/gold/testsuite/debug_msg.sh +++ b/gold/testsuite/debug_msg.sh @@ -55,4 +55,9 @@ check "debug_msg.o: in function int testfn(double):${srcdir}/debug_msg.c check "debug_msg.o: in function int testfn(double):${srcdir}/debug_msg.cc:44: undefined reference to 'undef_fn2()'" check "debug_msg.o: in function int testfn(double):${srcdir}/debug_msg.cc:45: undefined reference to 'undef_int'" +# Check we detected the ODR (One Definition Rule) violation. +check "warning: symbol Ordering::operator()(int, int) *defined in multiple places (possible ODR violation):" +check "odr_violation1.cc:5" +check "odr_violation2.cc:5" + exit 0 diff --git a/gold/testsuite/odr_violation1.cc b/gold/testsuite/odr_violation1.cc new file mode 100644 index 0000000..7f6f6d9 --- /dev/null +++ b/gold/testsuite/odr_violation1.cc @@ -0,0 +1,12 @@ +#include + +class Ordering { + public: + bool operator()(int a, int b) { + return a < b; + } +}; + +void SortAscending(int array[], int size) { + std::sort(array, array + size, Ordering()); +} diff --git a/gold/testsuite/odr_violation2.cc b/gold/testsuite/odr_violation2.cc new file mode 100644 index 0000000..d569279 --- /dev/null +++ b/gold/testsuite/odr_violation2.cc @@ -0,0 +1,14 @@ +#include + +class Ordering { + public: + bool operator()(int a, int b) { + // We need the "+ 1" here to force this operator() to be a + // different size than the one in odr_violation1.cc. + return a + 1 > b + 1; + } +}; + +void SortDescending(int array[], int size) { + std::sort(array, array + size, Ordering()); +} -- 2.7.4