diff options
| -rw-r--r-- | gold/ChangeLog | 38 | ||||
| -rw-r--r-- | gold/dwarf_reader.cc | 130 | ||||
| -rw-r--r-- | gold/dwarf_reader.h | 47 | ||||
| -rw-r--r-- | gold/object.cc | 2 | ||||
| -rw-r--r-- | gold/symtab.cc | 178 | ||||
| -rw-r--r-- | gold/symtab.h | 60 | ||||
| -rw-r--r-- | gold/testsuite/Makefile.am | 8 | ||||
| -rw-r--r-- | gold/testsuite/Makefile.in | 8 | ||||
| -rw-r--r-- | gold/testsuite/debug_msg.cc | 7 | ||||
| -rwxr-xr-x | gold/testsuite/debug_msg.sh | 30 | ||||
| -rw-r--r-- | gold/testsuite/odr_header1.h | 6 | ||||
| -rw-r--r-- | gold/testsuite/odr_header2.h | 4 | ||||
| -rw-r--r-- | gold/testsuite/odr_violation1.cc | 6 | ||||
| -rw-r--r-- | gold/testsuite/odr_violation2.cc | 16 |
14 files changed, 371 insertions, 169 deletions
diff --git a/gold/ChangeLog b/gold/ChangeLog index bbb8185..77c2870 100644 --- a/gold/ChangeLog +++ b/gold/ChangeLog @@ -1,3 +1,41 @@ +2011-03-09 Jeffrey Yasskin <jyasskin@google.com> + + * dwarf_reader.cc (Sized_dwarf_line_info): Include all lines, + but mark earlier ones as non-canonical + (offset_to_iterator): Update search target and example + (do_addr2line): Return extra lines in a vector* + (format_file_lineno): Extract from do_addr2line + (one_addr2line): Add vector* out-param + * dwarf_reader.h (Offset_to_lineno_entry): New field recording + when a lineno entry appeared last for its instruction + (Dwarf_line_info): Add vector* out-param + * object.cc (Relocate_info): Pass NULL for the vector* out-param + * symtab.cc (Odr_violation_compare): Include the lineno in the + comparison again. + (linenos_from_loc): New. Combine the canonical line for an + address with its other lines. + (True_if_intersect): New. Helper functor to make + std::set_intersection a query. + (detect_odr_violations): Compare sets of lines instead of just + one line for each function. This became less deterministic, but + has fewer false positives. + * symtab.h: Declarations. + * testsuite/Makefile.am (odr_violation2.o): Compile with -O2 to + mix an optimized and non-optimized object in the same binary + (odr_violation2.so): Same. + * testsuite/Makefile.in: Regenerate from Makefile.am. + * testsuite/debug_msg.cc (main): Make OdrDerived classes. + * testsuite/debug_msg.sh: Update line numbers and add + assertions. + * testsuite/odr_violation1.cc: Use OdrDerived, in a + non-optimized context. + * testsuite/odr_violation2.cc: Make sure Ordering::operator() + isn't inlined, and use OdrDerived in an optimized context. + * testsuite/odr_header1.h: Defines OdrDerived, where + optimization will change the + first-instruction-in-the-destructor's file and line number. + * testsuite/odr_header2.h: Defines OdrBase. + 2011-03-09 Ian Lance Taylor <iant@google.com> * fileread.cc (File_read::clear_views): Don't delete the whole diff --git a/gold/dwarf_reader.cc b/gold/dwarf_reader.cc index 8110f38..e8fe04e 100644 --- a/gold/dwarf_reader.cc +++ b/gold/dwarf_reader.cc @@ -492,19 +492,18 @@ Sized_dwarf_line_info<size, big_endian>::read_lines(unsigned const char* lineptr { Offset_to_lineno_entry entry = { lsm.address, this->current_header_index_, - lsm.file_num, lsm.line_num }; + lsm.file_num, true, lsm.line_num }; std::vector<Offset_to_lineno_entry>& map(this->line_number_map_[lsm.shndx]); // If we see two consecutive entries with the same - // offset and a real line number, then always use the - // second one. + // offset and a real line number, then mark the first + // one as non-canonical. if (!map.empty() && (map.back().offset == static_cast<off_t>(lsm.address)) && lsm.line_num != -1 && map.back().line_num != -1) - map.back() = entry; - else - map.push_back(entry); + map.back().last_line_for_offset = false; + map.push_back(entry); } lineptr += oplength; } @@ -612,7 +611,7 @@ static std::vector<Offset_to_lineno_entry>::const_iterator offset_to_iterator(const std::vector<Offset_to_lineno_entry>* offsets, off_t offset) { - const Offset_to_lineno_entry lookup_key = { offset, 0, 0, 0 }; + const Offset_to_lineno_entry lookup_key = { offset, 0, 0, true, 0 }; // lower_bound() returns the smallest offset which is >= lookup_key. // If no offset in offsets is >= lookup_key, returns end(). @@ -621,25 +620,27 @@ offset_to_iterator(const std::vector<Offset_to_lineno_entry>* offsets, // This code is easiest to understand with a concrete example. // Here's a possible offsets array: - // {{offset = 3211, header_num = 0, file_num = 1, line_num = 16}, // 0 - // {offset = 3224, header_num = 0, file_num = 1, line_num = 20}, // 1 - // {offset = 3226, header_num = 0, file_num = 1, line_num = 22}, // 2 - // {offset = 3231, header_num = 0, file_num = 1, line_num = 25}, // 3 - // {offset = 3232, header_num = 0, file_num = 1, line_num = -1}, // 4 - // {offset = 3232, header_num = 0, file_num = 1, line_num = 65}, // 5 - // {offset = 3235, header_num = 0, file_num = 1, line_num = 66}, // 6 - // {offset = 3236, header_num = 0, file_num = 1, line_num = -1}, // 7 - // {offset = 5764, header_num = 0, file_num = 1, line_num = 47}, // 8 - // {offset = 5765, header_num = 0, file_num = 1, line_num = 48}, // 9 - // {offset = 5767, header_num = 0, file_num = 1, line_num = 49}, // 10 - // {offset = 5768, header_num = 0, file_num = 1, line_num = 50}, // 11 - // {offset = 5773, header_num = 0, file_num = 1, line_num = -1}, // 12 - // {offset = 5787, header_num = 1, file_num = 1, line_num = 19}, // 13 - // {offset = 5790, header_num = 1, file_num = 1, line_num = 20}, // 14 - // {offset = 5793, header_num = 1, file_num = 1, line_num = 67}, // 15 - // {offset = 5793, header_num = 1, file_num = 1, line_num = -1}, // 16 - // {offset = 5795, header_num = 1, file_num = 1, line_num = 68}, // 17 - // {offset = 5798, header_num = 1, file_num = 1, line_num = -1}, // 18 + // {{offset = 3211, header_num = 0, file_num = 1, last, line_num = 16}, // 0 + // {offset = 3224, header_num = 0, file_num = 1, last, line_num = 20}, // 1 + // {offset = 3226, header_num = 0, file_num = 1, last, line_num = 22}, // 2 + // {offset = 3231, header_num = 0, file_num = 1, last, line_num = 25}, // 3 + // {offset = 3232, header_num = 0, file_num = 1, last, line_num = -1}, // 4 + // {offset = 3232, header_num = 0, file_num = 1, last, line_num = 65}, // 5 + // {offset = 3235, header_num = 0, file_num = 1, last, line_num = 66}, // 6 + // {offset = 3236, header_num = 0, file_num = 1, last, line_num = -1}, // 7 + // {offset = 5764, header_num = 0, file_num = 1, last, line_num = 48}, // 8 + // {offset = 5764, header_num = 0, file_num = 1,!last, line_num = 47}, // 9 + // {offset = 5765, header_num = 0, file_num = 1, last, line_num = 49}, // 10 + // {offset = 5767, header_num = 0, file_num = 1, last, line_num = 50}, // 11 + // {offset = 5768, header_num = 0, file_num = 1, last, line_num = 51}, // 12 + // {offset = 5773, header_num = 0, file_num = 1, last, line_num = -1}, // 13 + // {offset = 5787, header_num = 1, file_num = 1, last, line_num = 19}, // 14 + // {offset = 5790, header_num = 1, file_num = 1, last, line_num = 20}, // 15 + // {offset = 5793, header_num = 1, file_num = 1, last, line_num = 67}, // 16 + // {offset = 5793, header_num = 1, file_num = 1, last, line_num = -1}, // 17 + // {offset = 5793, header_num = 1, file_num = 1,!last, line_num = 66}, // 18 + // {offset = 5795, header_num = 1, file_num = 1, last, line_num = 68}, // 19 + // {offset = 5798, header_num = 1, file_num = 1, last, line_num = -1}, // 20 // The entries with line_num == -1 mark the end of a function: the // associated offset is one past the last instruction in the // function. This can correspond to the beginning of the next @@ -651,7 +652,7 @@ offset_to_iterator(const std::vector<Offset_to_lineno_entry>* offsets, // offsets[0]. Since it's not an exact match and we're // at the beginning of offsets, we return end() (invalid). // Case 2: lookup_key has offset 10000. lower_bound returns - // offset[19] (end()). We return end() (invalid). + // offset[21] (end()). We return end() (invalid). // Case 3: lookup_key has offset == 3211. lower_bound matches // offsets[0] exactly, and that's the entry we return. // Case 4: lookup_key has offset == 3232. lower_bound returns @@ -670,16 +671,17 @@ offset_to_iterator(const std::vector<Offset_to_lineno_entry>* offsets, // end-of-function, we know lookup_key is between // functions, so we return end() (not a valid offset). // Case 7: lookup_key has offset == 5794. lower_bound returns - // offsets[17]. Since it's not an exact match, we back - // up to offsets[15]. Note we back up to the *first* - // entry with offset 5793, not just offsets[17-1]. - // We note offsets[15] is a valid entry, so we return it. - // If offsets[15] had had line_num == -1, we would have - // checked offsets[16]. The reason for this is that - // 15 and 16 can be in an arbitrary order, since we sort - // only by offset. (Note it doesn't help to use line_number - // as a secondary sort key, since sometimes we want the -1 - // to be first and sometimes we want it to be last.) + // offsets[19]. Since it's not an exact match, we back + // up to offsets[16]. Note we back up to the *first* + // entry with offset 5793, not just offsets[19-1]. + // We note offsets[16] is a valid entry, so we return it. + // If offsets[16] had had line_num == -1, we would have + // checked offsets[17]. The reason for this is that + // 16 and 17 can be in an arbitrary order, since we sort + // only by offset and last_line_for_offset. (Note it + // doesn't help to use line_number as a tertiary sort key, + // since sometimes we want the -1 to be first and sometimes + // we want it to be last.) // This deals with cases (1) and (2). if ((it == offsets->begin() && offset < it->offset) @@ -710,19 +712,25 @@ offset_to_iterator(const std::vector<Offset_to_lineno_entry>* offsets, // This handles cases (5), (6), and (7): if any entry in the // equal_range [it, range_end) has a line_num != -1, it's a valid - // match. If not, we're not in a function. + // match. If not, we're not in a function. The line number we saw + // last for an offset will be sorted first, so it'll get returned if + // it's present. for (; it != range_end; ++it) if (it->line_num != -1) return it; return offsets->end(); } -// Return a string for a file name and line number. +// Returns the canonical filename:lineno for the address passed in. +// If other_lines is not NULL, appends the non-canonical lines +// assigned to the same address. template<int size, bool big_endian> std::string -Sized_dwarf_line_info<size, big_endian>::do_addr2line(unsigned int shndx, - off_t offset) +Sized_dwarf_line_info<size, big_endian>::do_addr2line( + unsigned int shndx, + off_t offset, + std::vector<std::string>* other_lines) { if (this->data_valid_ == false) return ""; @@ -743,21 +751,38 @@ Sized_dwarf_line_info<size, big_endian>::do_addr2line(unsigned int shndx, if (it == offsets->end()) return ""; - // Convert the file_num + line_num into a string. + std::string result = this->format_file_lineno(*it); + if (other_lines != NULL) + for (++it; it != offsets->end() && it->offset == offset; ++it) + { + if (it->line_num == -1) + continue; // The end of a previous function. + other_lines->push_back(this->format_file_lineno(*it)); + } + return result; +} + +// Convert the file_num + line_num into a string. + +template<int size, bool big_endian> +std::string +Sized_dwarf_line_info<size, big_endian>::format_file_lineno( + const Offset_to_lineno_entry& loc) const +{ std::string ret; - gold_assert(it->header_num < static_cast<int>(this->files_.size())); - gold_assert(it->file_num - < static_cast<int>(this->files_[it->header_num].size())); + gold_assert(loc.header_num < static_cast<int>(this->files_.size())); + gold_assert(loc.file_num + < static_cast<int>(this->files_[loc.header_num].size())); const std::pair<int, std::string>& filename_pair - = this->files_[it->header_num][it->file_num]; + = this->files_[loc.header_num][loc.file_num]; const std::string& filename = filename_pair.second; - gold_assert(it->header_num < static_cast<int>(this->directories_.size())); + gold_assert(loc.header_num < static_cast<int>(this->directories_.size())); gold_assert(filename_pair.first - < static_cast<int>(this->directories_[it->header_num].size())); + < static_cast<int>(this->directories_[loc.header_num].size())); const std::string& dirname - = this->directories_[it->header_num][filename_pair.first]; + = this->directories_[loc.header_num][filename_pair.first]; if (!dirname.empty()) { @@ -769,7 +794,7 @@ Sized_dwarf_line_info<size, big_endian>::do_addr2line(unsigned int shndx, ret = "(unknown)"; char buffer[64]; // enough to hold a line number - snprintf(buffer, sizeof(buffer), "%d", it->line_num); + snprintf(buffer, sizeof(buffer), "%d", loc.line_num); ret += ":"; ret += buffer; @@ -803,7 +828,8 @@ static std::vector<Addr2line_cache_entry> addr2line_cache; std::string Dwarf_line_info::one_addr2line(Object* object, unsigned int shndx, off_t offset, - size_t cache_size) + size_t cache_size, + std::vector<std::string>* other_lines) { Dwarf_line_info* lineinfo = NULL; std::vector<Addr2line_cache_entry>::iterator it; @@ -854,7 +880,7 @@ Dwarf_line_info::one_addr2line(Object* object, } // Now that we have our object, figure out the answer - std::string retval = lineinfo->addr2line(shndx, offset); + std::string retval = lineinfo->addr2line(shndx, offset, other_lines); // Finally, if our cache has grown too big, delete old objects. We // assume the common (probably only) case is deleting only one object. diff --git a/gold/dwarf_reader.h b/gold/dwarf_reader.h index 8fe7898..3f92dd3 100644 --- a/gold/dwarf_reader.h +++ b/gold/dwarf_reader.h @@ -25,6 +25,7 @@ #include <vector> #include <map> +#include <limits.h> #include "elfcpp.h" #include "elfcpp_swap.h" @@ -44,14 +45,25 @@ struct Offset_to_lineno_entry { off_t offset; int header_num; // which file-list to use (i.e. which .o file are we in) - int file_num; // a pointer into files_ - int line_num; // the line number in the source file - - // When we add entries to the table, we always use the last entry - // with a given offset. Given proper DWARF info, this should ensure - // that the offset is a sufficient sort key. + // A pointer into files_. + unsigned int file_num : sizeof(int) * CHAR_BIT - 1; + // True if this was the last entry for the current offset, meaning + // it's the line that actually applies. + unsigned int last_line_for_offset : 1; + // The line number in the source file. -1 to indicate end-of-function. + int line_num; + + // This sorts by offsets first, and then puts the correct line to + // report for a given offset at the beginning of the run of equal + // offsets (so that asking for 1 line gives the best answer). This + // is not a total ordering. bool operator<(const Offset_to_lineno_entry& that) const - { return this->offset < that.offset; } + { + if (this->offset != that.offset) + return this->offset < that.offset; + // Note the '>' which makes this sort 'true' first. + return this->last_line_for_offset > that.last_line_for_offset; + } }; // This class is used to read the line information from the debugging @@ -70,10 +82,13 @@ class 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. + // read_line_mappings() before calling this function. If + // 'other_lines' is non-NULL, fills that in with other line + // numbers assigned to the same offset. std::string - addr2line(unsigned int shndx, off_t offset) - { return do_addr2line(shndx, offset); } + addr2line(unsigned int shndx, off_t offset, + std::vector<std::string>* other_lines) + { return this->do_addr2line(shndx, offset, other_lines); } // A helper function for a single addr2line lookup. It also keeps a // cache of the last CACHE_SIZE Dwarf_line_info objects it created; @@ -83,7 +98,7 @@ class Dwarf_line_info // NOTE: Not thread-safe, so only call from one thread at a time. static std::string one_addr2line(Object* object, unsigned int shndx, off_t offset, - size_t cache_size); + size_t cache_size, std::vector<std::string>* other_lines); // This reclaims all the memory that one_addr2line may have cached. // Use this when you know you will not be calling one_addr2line again. @@ -92,7 +107,8 @@ class Dwarf_line_info private: virtual std::string - do_addr2line(unsigned int shndx, off_t offset) = 0; + do_addr2line(unsigned int shndx, off_t offset, + std::vector<std::string>* other_lines) = 0; }; template<int size, bool big_endian> @@ -106,7 +122,12 @@ class Sized_dwarf_line_info : public Dwarf_line_info private: std::string - do_addr2line(unsigned int shndx, off_t offset); + do_addr2line(unsigned int shndx, off_t offset, + std::vector<std::string>* other_lines); + + // Formats a file and line number to a string like "dirname/filename:lineno". + std::string + format_file_lineno(const Offset_to_lineno_entry& lineno) const; // Start processing line info, and populates the offset_map_. // If SHNDX is non-negative, only store debug information that diff --git a/gold/object.cc b/gold/object.cc index b85e9e0..e2c0113 100644 --- a/gold/object.cc +++ b/gold/object.cc @@ -2582,7 +2582,7 @@ Relocate_info<size, big_endian>::location(size_t, off_t offset) const Sized_dwarf_line_info<size, big_endian> 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); + file_and_lineno = line_info.addr2line(this->data_shndx, offset, NULL); std::string ret(this->object->name()); ret += ':'; diff --git a/gold/symtab.cc b/gold/symtab.cc index d4ac297..e882dea 100644 --- a/gold/symtab.cc +++ b/gold/symtab.cc @@ -3025,7 +3025,7 @@ Symbol_table::print_stats() const // We check for ODR violations by looking for symbols with the same // name for which the debugging information reports that they were -// defined in different source locations. When comparing the source +// defined in disjoint source locations. When comparing the source // location, we consider instances with the same base filename to be // the same. This is because different object files/shared libraries // can include the same header file using different paths, and @@ -3035,7 +3035,7 @@ Symbol_table::print_stats() const // This struct is used to compare line information, as returned by // Dwarf_line_info::one_addr2line. It implements a < comparison -// operator used with std::set. +// operator used with std::sort. struct Odr_violation_compare { @@ -3043,32 +3043,77 @@ struct Odr_violation_compare operator()(const std::string& s1, const std::string& s2) const { // Inputs should be of the form "dirname/filename:linenum" where - // "dirname/" is optional. We want to compare just the filename. + // "dirname/" is optional. We want to compare just the filename:linenum. - // Find the last '/' and ':' in each string. + // Find the last '/' in each string. std::string::size_type s1begin = s1.rfind('/'); std::string::size_type s2begin = s2.rfind('/'); - std::string::size_type s1end = s1.rfind(':'); - std::string::size_type s2end = s2.rfind(':'); // If there was no '/' in a string, start at the beginning. if (s1begin == std::string::npos) s1begin = 0; if (s2begin == std::string::npos) s2begin = 0; - // If the ':' appeared in the directory name, compare to the end - // of the string. - if (s1end < s1begin) - s1end = s1.size(); - if (s2end < s2begin) - s2end = s2.size(); - // Compare takes lengths, not end indices. - return s1.compare(s1begin, s1end - s1begin, - s2, s2begin, s2end - s2begin) < 0; + return s1.compare(s1begin, std::string::npos, + s2, s2begin, std::string::npos) < 0; } }; +// Returns all of the lines attached to LOC, not just the one the +// instruction actually came from. +std::vector<std::string> +Symbol_table::linenos_from_loc(const Task* task, + const Symbol_location& loc) +{ + // We need to lock the object in order to read it. This + // means that we have to run in a singleton Task. If we + // want to run this in a general Task for better + // performance, we will need one Task for object, plus + // appropriate locking to ensure that we don't conflict with + // other uses of the object. Also note, one_addr2line is not + // currently thread-safe. + Task_lock_obj<Object> tl(task, loc.object); + + std::vector<std::string> result; + // 16 is the size of the object-cache that one_addr2line should use. + std::string canonical_result = Dwarf_line_info::one_addr2line( + loc.object, loc.shndx, loc.offset, 16, &result); + if (!canonical_result.empty()) + result.push_back(canonical_result); + return result; +} + +// OutputIterator that records if it was ever assigned to. This +// allows it to be used with std::set_intersection() to check for +// intersection rather than computing the intersection. +struct Check_intersection +{ + Check_intersection() + : value_(false) + {} + + bool had_intersection() const + { return this->value_; } + + Check_intersection& operator++() + { return *this; } + + Check_intersection& operator*() + { return *this; } + + template<typename T> + Check_intersection& operator=(const T&) + { + this->value_ = true; + return *this; + } + + private: + bool value_; +}; + // Check candidate_odr_violations_ to find symbols with the same name -// but apparently different definitions (different source-file/line-no). +// but apparently different definitions (different source-file/line-no +// for each line assigned to the first instruction). void Symbol_table::detect_odr_violations(const Task* task, @@ -3078,48 +3123,73 @@ Symbol_table::detect_odr_violations(const Task* task, it != candidate_odr_violations_.end(); ++it) { - const char* symbol_name = it->first; - // Maps from symbol location to a sample object file we found - // that location in. We use a sorted map so the location order - // is deterministic, but we only store an arbitrary object file - // to avoid copying lots of names. - std::map<std::string, std::string, Odr_violation_compare> line_nums; - - for (Unordered_set<Symbol_location, Symbol_location_hash>::const_iterator - locs = it->second.begin(); - locs != it->second.end(); - ++locs) + const char* const symbol_name = it->first; + + std::string first_object_name; + std::vector<std::string> first_object_linenos; + + Unordered_set<Symbol_location, Symbol_location_hash>::const_iterator + locs = it->second.begin(); + const Unordered_set<Symbol_location, Symbol_location_hash>::const_iterator + locs_end = it->second.end(); + for (; locs != locs_end && first_object_linenos.empty(); ++locs) { - // We need to lock the object in order to read it. This - // means that we have to run in a singleton Task. If we - // want to run this in a general Task for better - // performance, we will need one Task for object, plus - // appropriate locking to ensure that we don't conflict with - // other uses of the object. Also note, one_addr2line is not - // currently thread-safe. - Task_lock_obj<Object> tl(task, locs->object); - // 16 is the size of the object-cache that one_addr2line should use. - std::string lineno = Dwarf_line_info::one_addr2line( - locs->object, locs->shndx, locs->offset, 16); - if (!lineno.empty()) - { - std::string& sample_object = line_nums[lineno]; - if (sample_object.empty()) - sample_object = locs->object->name(); - } + // Save the line numbers from the first definition to + // compare to the other definitions. Ideally, we'd compare + // every definition to every other, but we don't want to + // take O(N^2) time to do this. This shortcut may cause + // false negatives that appear or disappear depending on the + // link order, but it won't cause false positives. + first_object_name = locs->object->name(); + first_object_linenos = this->linenos_from_loc(task, *locs); } - if (line_nums.size() > 1) + // Sort by Odr_violation_compare to make std::set_intersection work. + std::sort(first_object_linenos.begin(), first_object_linenos.end(), + Odr_violation_compare()); + + for (; locs != locs_end; ++locs) { - gold_warning(_("while linking %s: symbol '%s' defined in multiple " - "places (possible ODR violation):"), - output_file_name, demangle(symbol_name).c_str()); - for (std::map<std::string, std::string>::const_iterator it2 = - line_nums.begin(); - it2 != line_nums.end(); - ++it2) - fprintf(stderr, _(" %s from %s\n"), - it2->first.c_str(), it2->second.c_str()); + std::vector<std::string> linenos = + this->linenos_from_loc(task, *locs); + // linenos will be empty if we couldn't parse the debug info. + if (linenos.empty()) + continue; + // Sort by Odr_violation_compare to make std::set_intersection work. + std::sort(linenos.begin(), linenos.end(), Odr_violation_compare()); + + Check_intersection intersection_result = + std::set_intersection(first_object_linenos.begin(), + first_object_linenos.end(), + linenos.begin(), + linenos.end(), + Check_intersection(), + Odr_violation_compare()); + if (!intersection_result.had_intersection()) + { + gold_warning(_("while linking %s: symbol '%s' defined in " + "multiple places (possible ODR violation):"), + output_file_name, demangle(symbol_name).c_str()); + // This only prints one location from each definition, + // which may not be the location we expect to intersect + // with another definition. We could print the whole + // set of locations, but that seems too verbose. + gold_assert(!first_object_linenos.empty()); + gold_assert(!linenos.empty()); + fprintf(stderr, _(" %s from %s\n"), + first_object_linenos[0].c_str(), + first_object_name.c_str()); + fprintf(stderr, _(" %s from %s\n"), + linenos[0].c_str(), + locs->object->name().c_str()); + // Only print one broken pair, to avoid needing to + // compare against a list of the disjoint definition + // locations we've found so far. (If we kept comparing + // against just the first one, we'd get a lot of + // redundant complaints about the second definition + // location.) + break; + } } } // We only call one_addr2line() in this function, so we can clear its cache. diff --git a/gold/symtab.h b/gold/symtab.h index c0ae358..d350d1d 100644 --- a/gold/symtab.h +++ b/gold/symtab.h @@ -1560,6 +1560,33 @@ class Symbol_table typedef Unordered_map<Symbol_table_key, Symbol*, Symbol_table_hash, Symbol_table_eq> Symbol_table_type; + // A map from symbol name (as a pointer into the namepool) to all + // the locations the symbols is (weakly) defined (and certain other + // conditions are met). This map will be used later to detect + // possible One Definition Rule (ODR) violations. + struct Symbol_location + { + Object* object; // Object where the symbol is defined. + unsigned int shndx; // Section-in-object where the symbol is defined. + off_t offset; // Offset-in-section where the symbol is defined. + bool operator==(const Symbol_location& that) const + { + return (this->object == that.object + && this->shndx == that.shndx + && this->offset == that.offset); + } + }; + + struct Symbol_location_hash + { + size_t operator()(const Symbol_location& loc) const + { return reinterpret_cast<uintptr_t>(loc.object) ^ loc.offset ^ loc.shndx; } + }; + + typedef Unordered_map<const char*, + Unordered_set<Symbol_location, Symbol_location_hash> > + Odr_map; + // Make FROM a forwarder symbol to TO. void make_forwarder(Symbol* from, Symbol* to); @@ -1707,6 +1734,12 @@ class Symbol_table do_allocate_commons_list(Layout*, Commons_section_type, Commons_type*, Mapfile*, Sort_commons_order); + // Returns all of the lines attached to LOC, not just the one the + // instruction actually came from. This helps the ODR checker avoid + // false positives. + static std::vector<std::string> + linenos_from_loc(const Task* task, const Symbol_location& loc); + // Implement detect_odr_violations. template<int size, bool big_endian> void @@ -1760,33 +1793,6 @@ class Symbol_table // they are defined. typedef Unordered_map<const Symbol*, Dynobj*> Copied_symbol_dynobjs; - // A map from symbol name (as a pointer into the namepool) to all - // the locations the symbols is (weakly) defined (and certain other - // conditions are met). This map will be used later to detect - // possible One Definition Rule (ODR) violations. - struct Symbol_location - { - Object* object; // Object where the symbol is defined. - unsigned int shndx; // Section-in-object where the symbol is defined. - off_t offset; // Offset-in-section where the symbol is defined. - bool operator==(const Symbol_location& that) const - { - return (this->object == that.object - && this->shndx == that.shndx - && this->offset == that.offset); - } - }; - - struct Symbol_location_hash - { - size_t operator()(const Symbol_location& loc) const - { return reinterpret_cast<uintptr_t>(loc.object) ^ loc.offset ^ loc.shndx; } - }; - - typedef Unordered_map<const char*, - Unordered_set<Symbol_location, Symbol_location_hash> > - Odr_map; - // We increment this every time we see a new undefined symbol, for // use in archive groups. size_t saw_undefined_; diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am index 53e6660..5ccd397 100644 --- a/gold/testsuite/Makefile.am +++ b/gold/testsuite/Makefile.am @@ -849,8 +849,10 @@ debug_msg.o: debug_msg.cc $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/debug_msg.cc odr_violation1.o: odr_violation1.cc $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation1.cc +# Compile with different optimization flags to check that rearranged +# instructions don't cause a false positive. odr_violation2.o: odr_violation2.cc - $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation2.cc + $(CXXCOMPILE) -O2 -g -c -w -o $@ $(srcdir)/odr_violation2.cc debug_msg.err: debug_msg.o odr_violation1.o odr_violation2.o gcctestdir/ld @echo $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg debug_msg.o odr_violation1.o odr_violation2.o "2>$@" @if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg debug_msg.o odr_violation1.o odr_violation2.o 2>$@; \ @@ -868,7 +870,7 @@ debug_msg.so: debug_msg.cc gcctestdir/ld odr_violation1.so: odr_violation1.cc gcctestdir/ld $(CXXCOMPILE) -Bgcctestdir/ -O0 -g -shared -fPIC -w -o $@ $(srcdir)/odr_violation1.cc odr_violation2.so: odr_violation2.cc gcctestdir/ld - $(CXXCOMPILE) -Bgcctestdir/ -O0 -g -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc + $(CXXCOMPILE) -Bgcctestdir/ -O2 -g -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc debug_msg_so.err: debug_msg.so odr_violation1.so odr_violation2.so gcctestdir/ld @echo $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_so debug_msg.so odr_violation1.so odr_violation2.so "2>$@" @if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_so debug_msg.so odr_violation1.so odr_violation2.so 2>$@; \ @@ -887,7 +889,7 @@ debug_msg_ndebug.so: debug_msg.cc gcctestdir/ld odr_violation1_ndebug.so: odr_violation1.cc gcctestdir/ld $(CXXCOMPILE) -Bgcctestdir/ -O0 -g0 -shared -fPIC -w -o $@ $(srcdir)/odr_violation1.cc odr_violation2_ndebug.so: odr_violation2.cc gcctestdir/ld - $(CXXCOMPILE) -Bgcctestdir/ -O0 -g0 -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc + $(CXXCOMPILE) -Bgcctestdir/ -O2 -g0 -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc debug_msg_ndebug.err: debug_msg_ndebug.so odr_violation1_ndebug.so odr_violation2_ndebug.so gcctestdir/ld @echo $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_ndebug debug_msg_ndebug.so odr_violation1_ndebug.so odr_violation2_ndebug.so "2>$@" @if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_ndebug debug_msg_ndebug.so odr_violation1_ndebug.so odr_violation2_ndebug.so 2>$@; \ diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in index 6d9bb7a..f7489ea 100644 --- a/gold/testsuite/Makefile.in +++ b/gold/testsuite/Makefile.in @@ -4017,8 +4017,10 @@ uninstall-am: @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/debug_msg.cc @GCC_TRUE@@NATIVE_LINKER_TRUE@odr_violation1.o: odr_violation1.cc @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation1.cc +# Compile with different optimization flags to check that rearranged +# instructions don't cause a false positive. @GCC_TRUE@@NATIVE_LINKER_TRUE@odr_violation2.o: odr_violation2.cc -@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation2.cc +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O2 -g -c -w -o $@ $(srcdir)/odr_violation2.cc @GCC_TRUE@@NATIVE_LINKER_TRUE@debug_msg.err: debug_msg.o odr_violation1.o odr_violation2.o gcctestdir/ld @GCC_TRUE@@NATIVE_LINKER_TRUE@ @echo $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg debug_msg.o odr_violation1.o odr_violation2.o "2>$@" @GCC_TRUE@@NATIVE_LINKER_TRUE@ @if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg debug_msg.o odr_violation1.o odr_violation2.o 2>$@; \ @@ -4032,7 +4034,7 @@ uninstall-am: @GCC_TRUE@@NATIVE_LINKER_TRUE@odr_violation1.so: odr_violation1.cc gcctestdir/ld @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -Bgcctestdir/ -O0 -g -shared -fPIC -w -o $@ $(srcdir)/odr_violation1.cc @GCC_TRUE@@NATIVE_LINKER_TRUE@odr_violation2.so: odr_violation2.cc gcctestdir/ld -@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -Bgcctestdir/ -O0 -g -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -Bgcctestdir/ -O2 -g -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc @GCC_TRUE@@NATIVE_LINKER_TRUE@debug_msg_so.err: debug_msg.so odr_violation1.so odr_violation2.so gcctestdir/ld @GCC_TRUE@@NATIVE_LINKER_TRUE@ @echo $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_so debug_msg.so odr_violation1.so odr_violation2.so "2>$@" @GCC_TRUE@@NATIVE_LINKER_TRUE@ @if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_so debug_msg.so odr_violation1.so odr_violation2.so 2>$@; \ @@ -4046,7 +4048,7 @@ uninstall-am: @GCC_TRUE@@NATIVE_LINKER_TRUE@odr_violation1_ndebug.so: odr_violation1.cc gcctestdir/ld @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -Bgcctestdir/ -O0 -g0 -shared -fPIC -w -o $@ $(srcdir)/odr_violation1.cc @GCC_TRUE@@NATIVE_LINKER_TRUE@odr_violation2_ndebug.so: odr_violation2.cc gcctestdir/ld -@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -Bgcctestdir/ -O0 -g0 -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -Bgcctestdir/ -O2 -g0 -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc @GCC_TRUE@@NATIVE_LINKER_TRUE@debug_msg_ndebug.err: debug_msg_ndebug.so odr_violation1_ndebug.so odr_violation2_ndebug.so gcctestdir/ld @GCC_TRUE@@NATIVE_LINKER_TRUE@ @echo $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_ndebug debug_msg_ndebug.so odr_violation1_ndebug.so odr_violation2_ndebug.so "2>$@" @GCC_TRUE@@NATIVE_LINKER_TRUE@ @if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_ndebug debug_msg_ndebug.so odr_violation1_ndebug.so odr_violation2_ndebug.so 2>$@; \ diff --git a/gold/testsuite/debug_msg.cc b/gold/testsuite/debug_msg.cc index 1d77bc9..0912002 100644 --- a/gold/testsuite/debug_msg.cc +++ b/gold/testsuite/debug_msg.cc @@ -58,6 +58,10 @@ class Derived : public Base // 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 +// This tests One Definition Rule (ODR) non-violations. +#include "odr_header2.h" +OdrBase* CreateOdrDerived1(); // in odr_violation1.cc +OdrBase* CreateOdrDerived2(); // in odr_violation2.cc extern "C" int OverriddenCFunction(int i); // in odr_violation*.cc @@ -85,5 +89,8 @@ int main() OverriddenCFunction(3); SometimesInlineFunction(3); + delete CreateOdrDerived1(); + delete CreateOdrDerived2(); + return 0; } diff --git a/gold/testsuite/debug_msg.sh b/gold/testsuite/debug_msg.sh index 74b4b05..b8624e7 100755 --- a/gold/testsuite/debug_msg.sh +++ b/gold/testsuite/debug_msg.sh @@ -73,18 +73,22 @@ check debug_msg.err "debug_msg.o: in function int testfn<double>(double):.*/debu # Check we detected the ODR (One Definition Rule) violation. check debug_msg.err ": symbol 'Ordering::operator()(int, int)' defined in multiple places (possible ODR violation):" -check debug_msg.err "odr_violation1.cc:5" -check debug_msg.err "odr_violation2.cc:5" +check debug_msg.err "odr_violation1.cc:6" +check debug_msg.err "odr_violation2.cc:9" + +# Check we don't have ODR false positives: +check_missing debug_msg.err "OdrDerived::~OdrDerived()" +check_missing debug_msg.err "__adjust_heap" # We block ODR detection for combinations of C weak and strong # symbols, to allow people to use the linker to override things. We # still flag it for C++ symbols since those are more likely to be # unintentional. check_missing debug_msg.err ": symbol 'OverriddenCFunction' defined in multiple places (possible ODR violation):" -check_missing debug_msg.err "odr_violation1.cc:15" -check_missing debug_msg.err "odr_violation2.cc:17" +check_missing debug_msg.err "odr_violation1.cc:16" +check_missing debug_msg.err "odr_violation2.cc:20" check debug_msg.err ": symbol 'SometimesInlineFunction(int)' defined in multiple places (possible ODR violation):" -check debug_msg.err "debug_msg.cc:64" -check debug_msg.err "odr_violation2.cc:21" +check debug_msg.err "debug_msg.cc:68" +check debug_msg.err "odr_violation2.cc:24" # When linking together .so's, we don't catch the line numbers, but we # still find all the undefined variables, and the ODR violation. @@ -92,14 +96,16 @@ check debug_msg_so.err "debug_msg.so: error: undefined reference to 'undef_fn1() check debug_msg_so.err "debug_msg.so: error: undefined reference to 'undef_fn2()'" check debug_msg_so.err "debug_msg.so: error: undefined reference to 'undef_int'" check debug_msg_so.err ": symbol 'Ordering::operator()(int, int)' defined in multiple places (possible ODR violation):" -check debug_msg_so.err "odr_violation1.cc:5" -check debug_msg_so.err "odr_violation2.cc:5" +check debug_msg_so.err "odr_violation1.cc:6" +check debug_msg_so.err "odr_violation2.cc:9" +check_missing debug_msg.err "OdrDerived::~OdrDerived()" +check_missing debug_msg.err "__adjust_heap" check_missing debug_msg.err ": symbol 'OverriddenCFunction' defined in multiple places (possible ODR violation):" -check_missing debug_msg.err "odr_violation1.cc:15" -check_missing debug_msg.err "odr_violation2.cc:17" +check_missing debug_msg.err "odr_violation1.cc:16" +check_missing debug_msg.err "odr_violation2.cc:20" check debug_msg.err ": symbol 'SometimesInlineFunction(int)' defined in multiple places (possible ODR violation):" -check debug_msg.err "debug_msg.cc:64" -check debug_msg.err "odr_violation2.cc:21" +check debug_msg.err "debug_msg.cc:68" +check debug_msg.err "odr_violation2.cc:24" # These messages shouldn't need any debug info to detect: check debug_msg_ndebug.err "debug_msg_ndebug.so: error: undefined reference to 'undef_fn1()'" diff --git a/gold/testsuite/odr_header1.h b/gold/testsuite/odr_header1.h new file mode 100644 index 0000000..3d9c488 --- /dev/null +++ b/gold/testsuite/odr_header1.h @@ -0,0 +1,6 @@ +#include "odr_header2.h" + +struct OdrDerived : OdrBase { + ~OdrDerived() { + } +}; diff --git a/gold/testsuite/odr_header2.h b/gold/testsuite/odr_header2.h new file mode 100644 index 0000000..0fb9449 --- /dev/null +++ b/gold/testsuite/odr_header2.h @@ -0,0 +1,4 @@ +struct OdrBase { + virtual ~OdrBase() { + } +}; diff --git a/gold/testsuite/odr_violation1.cc b/gold/testsuite/odr_violation1.cc index 6c40496..8b37cea 100644 --- a/gold/testsuite/odr_violation1.cc +++ b/gold/testsuite/odr_violation1.cc @@ -1,4 +1,5 @@ #include <algorithm> +#include "odr_header1.h" class Ordering { public: @@ -15,3 +16,8 @@ extern "C" int OverriddenCFunction(int i) __attribute__ ((weak)); extern "C" int OverriddenCFunction(int i) { return i; } + +// Instantiate the Derived vtable, without optimization. +OdrBase* CreateOdrDerived1() { + return new OdrDerived; +} diff --git a/gold/testsuite/odr_violation2.cc b/gold/testsuite/odr_violation2.cc index 09940d4..6c57b6f 100644 --- a/gold/testsuite/odr_violation2.cc +++ b/gold/testsuite/odr_violation2.cc @@ -1,13 +1,16 @@ #include <algorithm> +#include "odr_header1.h" 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. + bool operator()(int a, int b) __attribute__((never_inline)); +}; + +bool Ordering::operator()(int a, int b) { + // Optimization makes this operator() 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()); @@ -21,3 +24,8 @@ extern "C" int OverriddenCFunction(int i) { int SometimesInlineFunction(int i) { return i * i; } + +// Instantiate the Derived vtable, with optimization (see Makefile.am). +OdrBase* CreateOdrDerived2() { + return new OdrDerived; +} |