From 9eb9fa57c2a9eec4c08491715d3341df811b7f9c Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 25 Sep 2007 17:50:26 +0000 Subject: [PATCH] Add cache parameter to get_view. Discard uncached views on unlock. Fix bug this exposed in archive armap symbol name handling. --- gold/archive.cc | 27 +++++++++------- gold/archive.h | 17 +++++----- gold/dynobj.cc | 12 +++++--- gold/fileread.cc | 94 +++++++++++++++++++++++++++----------------------------- gold/fileread.h | 34 ++++++++++++-------- gold/merge.cc | 4 +-- gold/object.cc | 29 +++++++++-------- gold/object.h | 19 +++++++----- gold/reloc.cc | 13 +++++--- gold/symtab.cc | 3 +- 10 files changed, 140 insertions(+), 112 deletions(-) diff --git a/gold/archive.cc b/gold/archive.cc index 7398076..bbd24c8 100644 --- a/gold/archive.cc +++ b/gold/archive.cc @@ -101,7 +101,7 @@ Archive::setup() if (xname == "/") { const unsigned char* p = this->get_view(off + sizeof(Archive_header), - extended_size); + extended_size, false); const char* px = reinterpret_cast(p); this->extended_names_.assign(px, extended_size); } @@ -116,7 +116,7 @@ void Archive::read_armap(off_t start, off_t size) { // Read in the entire armap. - const unsigned char* p = this->get_view(start, size); + const unsigned char* p = this->get_view(start, size, false); // Numbers in the armap are always big-endian. const elfcpp::Elf_Word* pword = reinterpret_cast(p); @@ -125,14 +125,17 @@ Archive::read_armap(off_t start, off_t size) // Note that the addition is in units of sizeof(elfcpp::Elf_Word). const char* pnames = reinterpret_cast(pword + nsyms); + off_t names_size = reinterpret_cast(p) + size - pnames; + this->armap_names_.assign(pnames, names_size); this->armap_.resize(nsyms); + off_t name_offset = 0; for (unsigned int i = 0; i < nsyms; ++i) { - this->armap_[i].name = pnames; - this->armap_[i].offset = elfcpp::Swap<32, true>::readval(pword); - pnames += strlen(pnames) + 1; + this->armap_[i].name_offset = name_offset; + this->armap_[i].file_offset = elfcpp::Swap<32, true>::readval(pword); + name_offset += strlen(pnames + name_offset) + 1; ++pword; } @@ -155,7 +158,7 @@ Archive::read_armap(off_t start, off_t size) off_t Archive::read_header(off_t off, std::string* pname) { - const unsigned char* p = this->get_view(off, sizeof(Archive_header)); + const unsigned char* p = this->get_view(off, sizeof(Archive_header), false); const Archive_header* hdr = reinterpret_cast(p); return this->interpret_header(hdr, off, pname); } @@ -283,20 +286,22 @@ Archive::add_symbols(Symbol_table* symtab, Layout* layout, { if (this->armap_checked_[i]) continue; - if (this->armap_[i].offset == last_seen_offset) + if (this->armap_[i].file_offset == last_seen_offset) { this->armap_checked_[i] = true; continue; } - if (this->seen_offsets_.find(this->armap_[i].offset) + if (this->seen_offsets_.find(this->armap_[i].file_offset) != this->seen_offsets_.end()) { this->armap_checked_[i] = true; - last_seen_offset = this->armap_[i].offset; + last_seen_offset = this->armap_[i].file_offset; continue; } - Symbol* sym = symtab->lookup(this->armap_[i].name); + const char* sym_name = (this->armap_names_.data() + + this->armap_[i].name_offset); + Symbol* sym = symtab->lookup(sym_name); if (sym == NULL) continue; else if (!sym->is_undefined()) @@ -308,7 +313,7 @@ Archive::add_symbols(Symbol_table* symtab, Layout* layout, continue; // We want to include this object in the link. - last_seen_offset = this->armap_[i].offset; + last_seen_offset = this->armap_[i].file_offset; this->seen_offsets_.insert(last_seen_offset); this->armap_checked_[i] = true; this->include_member(symtab, layout, input_objects, diff --git a/gold/archive.h b/gold/archive.h index 7ebdf47..61d4d4b 100644 --- a/gold/archive.h +++ b/gold/archive.h @@ -44,7 +44,8 @@ class Archive { public: Archive(const std::string& name, Input_file* input_file) - : name_(name), input_file_(input_file), armap_(), extended_names_() + : name_(name), input_file_(input_file), armap_(), armap_names_(), + extended_names_(), armap_checked_(), seen_offsets_() { } // The length of the magic string at the start of an archive. @@ -98,8 +99,8 @@ class Archive // Get a view into the underlying file. const unsigned char* - get_view(off_t start, off_t size) - { return this->input_file_->file().get_view(start, size); } + get_view(off_t start, off_t size, bool cache) + { return this->input_file_->file().get_view(start, size, cache); } // Read the archive symbol map. void @@ -126,10 +127,10 @@ class Archive // An entry in the archive map of symbols to object files. struct Armap_entry { - // The symbol name. - const char* name; - // The offset to the file. - off_t offset; + // The offset to the symbol name in armap_names_. + off_t name_offset; + // The file offset to the object in the archive. + off_t file_offset; }; // A simple hash code for off_t values. @@ -146,6 +147,8 @@ class Archive Input_file* input_file_; // The archive map. std::vector armap_; + // The names in the archive map. + std::string armap_names_; // The extended name table. std::string extended_names_; // Track which symbols in the archive map are for elements which are diff --git a/gold/dynobj.cc b/gold/dynobj.cc index b6255f8..2ccb8f5 100644 --- a/gold/dynobj.cc +++ b/gold/dynobj.cc @@ -174,7 +174,8 @@ Sized_dynobj::read_dynsym_section( gold_exit(false); } - *view = this->get_lasting_view(shdr.get_sh_offset(), shdr.get_sh_size()); + *view = this->get_lasting_view(shdr.get_sh_offset(), shdr.get_sh_size(), + false); *view_size = shdr.get_sh_size(); *view_info = shdr.get_sh_info(); } @@ -198,7 +199,7 @@ Sized_dynobj::set_soname(const unsigned char* pshdrs, const off_t dynamic_size = dynamicshdr.get_sh_size(); const unsigned char* pdynamic = this->get_view(dynamicshdr.get_sh_offset(), - dynamic_size); + dynamic_size, false); const unsigned int link = dynamicshdr.get_sh_link(); if (link != strtab_shndx) @@ -223,7 +224,7 @@ Sized_dynobj::set_soname(const unsigned char* pshdrs, } strtab_size = strtabshdr.get_sh_size(); - strtabu = this->get_view(strtabshdr.get_sh_offset(), strtab_size); + strtabu = this->get_view(strtabshdr.get_sh_offset(), strtab_size, false); } for (const unsigned char* p = pdynamic; @@ -295,7 +296,7 @@ Sized_dynobj::do_read_symbols(Read_symbols_data* sd) gold_assert(dynsymshdr.get_sh_type() == elfcpp::SHT_DYNSYM); sd->symbols = this->get_lasting_view(dynsymshdr.get_sh_offset(), - dynsymshdr.get_sh_size()); + dynsymshdr.get_sh_size(), false); sd->symbols_size = dynsymshdr.get_sh_size(); // Get the symbol names. @@ -319,7 +320,8 @@ Sized_dynobj::do_read_symbols(Read_symbols_data* sd) } sd->symbol_names = this->get_lasting_view(strtabshdr.get_sh_offset(), - strtabshdr.get_sh_size()); + strtabshdr.get_sh_size(), + true); sd->symbol_names_size = strtabshdr.get_sh_size(); // Get the version information. diff --git a/gold/fileread.cc b/gold/fileread.cc index 9b800e8..3cb2685 100644 --- a/gold/fileread.cc +++ b/gold/fileread.cc @@ -136,6 +136,8 @@ File_read::unlock() { gold_assert(this->lock_count_ > 0); --this->lock_count_; + if (this->lock_count_ == 0) + this->clear_views(false); } bool @@ -160,54 +162,44 @@ File_read::find_view(off_t start, off_t size) } // Read SIZE bytes from the file starting at offset START. Read into -// the buffer at P. Return the number of bytes read, which should -// always be at least SIZE except at the end of the file. +// the buffer at P. -off_t +void File_read::do_read(off_t start, off_t size, void* p) { gold_assert(this->lock_count_ > 0); + off_t bytes; if (this->contents_ != NULL) { - off_t bytes = this->size_ - start; - if (bytes < 0) - bytes = 0; - else if (bytes > size) - bytes = size; - memcpy(p, this->contents_ + start, bytes); - return bytes; + bytes = this->size_ - start; + if (bytes >= size) + { + memcpy(p, this->contents_ + start, size); + return; + } } - - off_t bytes = ::pread(this->descriptor_, p, size, start); - if (bytes < 0) + else { - fprintf(stderr, _("%s: %s: pread failed: %s\n"), - program_name, this->filename().c_str(), strerror(errno)); - gold_exit(false); - } - - return bytes; -} - -// Read exactly SIZE bytes from the file starting at offset START. -// Read into the buffer at P. + bytes = ::pread(this->descriptor_, p, size, start); + if (bytes == size) + return; -void -File_read::do_read_exact(off_t start, off_t size, void* p) -{ - off_t bytes = this->do_read(start, size, p); - if (bytes != size) - { - fprintf(stderr, - _("%s: %s: file too short: read only %lld of %lld " - "bytes at %lld\n"), - program_name, this->filename().c_str(), - static_cast(bytes), - static_cast(size), - static_cast(start)); - gold_exit(false); + if (bytes < 0) + { + fprintf(stderr, _("%s: %s: pread failed: %s\n"), + program_name, this->filename().c_str(), strerror(errno)); + gold_exit(false); + } } + + fprintf(stderr, + _("%s: %s: file too short: read only %lld of %lld bytes at %lld\n"), + program_name, this->filename().c_str(), + static_cast(bytes), + static_cast(size), + static_cast(start)); + gold_exit(false); } // Read data from the file. @@ -224,13 +216,13 @@ File_read::read(off_t start, off_t size, void* p) return; } - this->do_read_exact(start, size, p); + this->do_read(start, size, p); } // Find an existing view or make a new one. File_read::View* -File_read::find_or_make_view(off_t start, off_t size) +File_read::find_or_make_view(off_t start, off_t size, bool cache) { gold_assert(this->lock_count_ > 0); @@ -245,7 +237,11 @@ File_read::find_or_make_view(off_t start, off_t size) // There was an existing view at this offset. File_read::View* v = ins.first->second; if (v->size() - (start - v->start()) >= size) - return v; + { + if (cache) + v->set_cache(); + return v; + } // This view is not large enough. this->saved_views_.push_back(v); @@ -264,9 +260,9 @@ File_read::find_or_make_view(off_t start, off_t size) unsigned char* p = new unsigned char[psize]; - this->do_read_exact(poff, psize, p); + this->do_read(poff, psize, p); - File_read::View* v = new File_read::View(poff, psize, p); + File_read::View* v = new File_read::View(poff, psize, p, cache); ins.first->second = v; return v; } @@ -276,18 +272,18 @@ File_read::find_or_make_view(off_t start, off_t size) // mmap. const unsigned char* -File_read::get_view(off_t start, off_t size) +File_read::get_view(off_t start, off_t size, bool cache) { gold_assert(this->lock_count_ > 0); - File_read::View* pv = this->find_or_make_view(start, size); + File_read::View* pv = this->find_or_make_view(start, size, cache); return pv->data() + (start - pv->start()); } File_view* -File_read::get_lasting_view(off_t start, off_t size) +File_read::get_lasting_view(off_t start, off_t size, bool cache) { gold_assert(this->lock_count_ > 0); - File_read::View* pv = this->find_or_make_view(start, size); + File_read::View* pv = this->find_or_make_view(start, size, cache); pv->lock(); return new File_view(*this, pv, pv->data() + (start - pv->start())); } @@ -301,7 +297,8 @@ File_read::clear_views(bool destroying) p != this->views_.end(); ++p) { - if (!p->second->is_locked()) + if (!p->second->is_locked() + && (destroying || !p->second->should_cache())) delete p->second; else { @@ -314,7 +311,8 @@ File_read::clear_views(bool destroying) Saved_views::iterator p = this->saved_views_.begin(); while (p != this->saved_views_.end()) { - if (!(*p)->is_locked()) + if (!(*p)->is_locked() + && (destroying || !(*p)->should_cache())) { delete *p; p = this->saved_views_.erase(p); diff --git a/gold/fileread.h b/gold/fileread.h index ef5efc0..0e5caa9 100644 --- a/gold/fileread.h +++ b/gold/fileread.h @@ -88,9 +88,12 @@ class File_read // Return a view into the file starting at file offset START for // SIZE bytes. The pointer will remain valid until the File_read is // unlocked. It is an error if we can not read enough data from the - // file. + // file. The CACHE parameter is a hint as to whether it will be + // useful to cache this data for later accesses--i.e., later calls + // to get_view, read, or get_lasting_view which retrieve the same + // data. const unsigned char* - get_view(off_t start, off_t size); + get_view(off_t start, off_t size, bool cache); // Read data from the file into the buffer P starting at file offset // START for SIZE bytes. @@ -101,9 +104,10 @@ class File_read // for SIZE bytes. This is allocated with new, and the caller is // responsible for deleting it when done. The data associated with // this view will remain valid until the view is deleted. It is an - // error if we can not read enough data from the file. + // error if we can not read enough data from the file. The CACHE + // parameter is as in get_view. File_view* - get_lasting_view(off_t start, off_t size); + get_lasting_view(off_t start, off_t size, bool cache); private: // This class may not be copied. @@ -114,8 +118,9 @@ class File_read class View { public: - View(off_t start, off_t size, const unsigned char* data) - : start_(start), size_(size), data_(data), lock_count_(0) + View(off_t start, off_t size, const unsigned char* data, bool cache) + : start_(start), size_(size), data_(data), lock_count_(0), + cache_(cache) { } ~View(); @@ -141,6 +146,14 @@ class File_read bool is_locked(); + void + set_cache() + { this->cache_ = true; } + + bool + should_cache() const + { return this->cache_; } + private: View(const View&); View& operator=(const View&); @@ -149,6 +162,7 @@ class File_read off_t size_; const unsigned char* data_; int lock_count_; + bool cache_; }; friend class File_view; @@ -158,16 +172,12 @@ class File_read find_view(off_t start, off_t size); // Read data from the file into a buffer. - off_t - do_read(off_t start, off_t size, void* p); - - // Read an exact number of bytes into a buffer. void - do_read_exact(off_t start, off_t size, void* p); + do_read(off_t start, off_t size, void* p); // Find or make a view into the file. View* - find_or_make_view(off_t start, off_t size); + find_or_make_view(off_t start, off_t size, bool cache); // Clear the file views. void diff --git a/gold/merge.cc b/gold/merge.cc index f6f45c9..0db62ef 100644 --- a/gold/merge.cc +++ b/gold/merge.cc @@ -177,7 +177,7 @@ bool Output_merge_data::do_add_input_section(Relobj* object, unsigned int shndx) { off_t len; - const unsigned char* p = object->section_contents(shndx, &len); + const unsigned char* p = object->section_contents(shndx, &len, false); uint64_t entsize = this->entsize(); @@ -237,7 +237,7 @@ Output_merge_string::do_add_input_section(Relobj* object, unsigned int shndx) { off_t len; - const unsigned char* pdata = object->section_contents(shndx, &len); + const unsigned char* pdata = object->section_contents(shndx, &len, false); const Char_type* p = reinterpret_cast(pdata); diff --git a/gold/object.cc b/gold/object.cc index 336eaf6..d64e120 100644 --- a/gold/object.cc +++ b/gold/object.cc @@ -73,11 +73,11 @@ Object::error(const char* format, ...) // Return a view of the contents of a section. const unsigned char* -Object::section_contents(unsigned int shndx, off_t* plen) +Object::section_contents(unsigned int shndx, off_t* plen, bool cache) { Location loc(this->do_section_contents(shndx)); *plen = loc.data_size; - return this->get_view(loc.file_offset, loc.data_size); + return this->get_view(loc.file_offset, loc.data_size, cache); } // Read the section data into SD. This is code common to Sized_relobj @@ -93,7 +93,7 @@ Object::read_section_data(elfcpp::Elf_file* elf_file, // Read the section headers. const off_t shoff = elf_file->shoff(); const unsigned int shnum = this->shnum(); - sd->section_headers = this->get_lasting_view(shoff, shnum * shdr_size); + sd->section_headers = this->get_lasting_view(shoff, shnum * shdr_size, true); // Read the section names. const unsigned char* pshdrs = sd->section_headers->data(); @@ -111,7 +111,7 @@ Object::read_section_data(elfcpp::Elf_file* elf_file, sd->section_names_size = shdrnames.get_sh_size(); sd->section_names = this->get_lasting_view(shdrnames.get_sh_offset(), - sd->section_names_size); + sd->section_names_size, false); } // If NAME is the name of a special .gnu.warning section, arrange for @@ -239,7 +239,7 @@ Sized_relobj::do_read_symbols(Read_symbols_data* sd) off_t extsize = symtabshdr.get_sh_size() - locsize; // Read the symbol table. - File_view* fvsymtab = this->get_lasting_view(extoff, extsize); + File_view* fvsymtab = this->get_lasting_view(extoff, extsize, false); // Read the section header for the symbol names. unsigned int strtab_shndx = symtabshdr.get_sh_link(); @@ -261,7 +261,7 @@ Sized_relobj::do_read_symbols(Read_symbols_data* sd) // Read the symbol names. File_view* fvstrtab = this->get_lasting_view(strtabshdr.get_sh_offset(), - strtabshdr.get_sh_size()); + strtabshdr.get_sh_size(), true); sd->symbols = fvsymtab; sd->symbols_size = extsize; @@ -285,7 +285,7 @@ Sized_relobj::include_section_group( { // Read the section contents. const unsigned char* pcon = this->get_view(shdr.get_sh_offset(), - shdr.get_sh_size()); + shdr.get_sh_size(), false); const elfcpp::Elf_Word* pword = reinterpret_cast(pcon); @@ -314,13 +314,14 @@ Sized_relobj::include_section_group( gold_exit(false); } off_t symoff = symshdr.get_sh_offset() + shdr.get_sh_info() * This::sym_size; - const unsigned char* psym = this->get_view(symoff, This::sym_size); + const unsigned char* psym = this->get_view(symoff, This::sym_size, true); elfcpp::Sym sym(psym); // Read the symbol table names. off_t symnamelen; const unsigned char* psymnamesu; - psymnamesu = this->section_contents(symshdr.get_sh_link(), &symnamelen); + psymnamesu = this->section_contents(symshdr.get_sh_link(), &symnamelen, + true); const char* psymnames = reinterpret_cast(psymnamesu); // Get the section group signature. @@ -557,7 +558,7 @@ Sized_relobj::do_finalize_local_symbols(unsigned int index, gold_assert(loccount == symtabshdr.get_sh_info()); off_t locsize = loccount * sym_size; const unsigned char* psyms = this->get_view(symtabshdr.get_sh_offset(), - locsize); + locsize, true); this->local_values_.resize(loccount); @@ -565,7 +566,8 @@ Sized_relobj::do_finalize_local_symbols(unsigned int index, const unsigned int strtab_shndx = symtabshdr.get_sh_link(); off_t strtab_size; const unsigned char* pnamesu = this->section_contents(strtab_shndx, - &strtab_size); + &strtab_size, + true); const char* pnames = reinterpret_cast(pnamesu); // Loop over the local symbols. @@ -700,13 +702,14 @@ Sized_relobj::write_local_symbols(Output_file* of, const int sym_size = This::sym_size; off_t locsize = loccount * sym_size; const unsigned char* psyms = this->get_view(symtabshdr.get_sh_offset(), - locsize); + locsize, false); // Read the symbol names. const unsigned int strtab_shndx = symtabshdr.get_sh_link(); off_t strtab_size; const unsigned char* pnamesu = this->section_contents(strtab_shndx, - &strtab_size); + &strtab_size, + true); const char* pnames = reinterpret_cast(pnamesu); // Get a view into the output file. diff --git a/gold/object.h b/gold/object.h index 55935c1..cc1d5b2 100644 --- a/gold/object.h +++ b/gold/object.h @@ -168,9 +168,9 @@ class Object { return this->shnum_; } // Return a view of the contents of a section. Set *PLEN to the - // size. + // size. CACHE is a hint as in File_read::get_view. const unsigned char* - section_contents(unsigned int shndx, off_t* plen); + section_contents(unsigned int shndx, off_t* plen, bool cache); // Return the name of a section given a section index. This is only // used for error messages. @@ -224,7 +224,7 @@ class Object // Return a View. View view(off_t file_offset, off_t data_size) - { return View(this->get_view(file_offset, data_size)); } + { return View(this->get_view(file_offset, data_size, true)); } // Report an error. void @@ -243,7 +243,7 @@ class Object // Get a View given a Location. View view(Location loc) - { return View(this->get_view(loc.file_offset, loc.data_size)); } + { return View(this->get_view(loc.file_offset, loc.data_size, true)); } protected: // Read the symbols--implemented by child class. @@ -284,15 +284,18 @@ class Object // Get a view into the underlying file. const unsigned char* - get_view(off_t start, off_t size) - { return this->input_file_->file().get_view(start + this->offset_, size); } + get_view(off_t start, off_t size, bool cache) + { + return this->input_file_->file().get_view(start + this->offset_, size, + cache); + } // Get a lasting view into the underlying file. File_view* - get_lasting_view(off_t start, off_t size) + get_lasting_view(off_t start, off_t size, bool cache) { return this->input_file_->file().get_lasting_view(start + this->offset_, - size); + size, cache); } // Read data from the underlying file. diff --git a/gold/reloc.cc b/gold/reloc.cc index d4674bb..ad3bb05 100644 --- a/gold/reloc.cc +++ b/gold/reloc.cc @@ -172,7 +172,8 @@ Sized_relobj::do_read_relocs(Read_relocs_data* rd) rd->relocs.reserve(shnum / 2); const unsigned char *pshdrs = this->get_view(this->elf_file_.shoff(), - shnum * This::shdr_size); + shnum * This::shdr_size, + true); // Skip the first, dummy, section. const unsigned char *ps = pshdrs + This::shdr_size; for (unsigned int i = 1; i < shnum; ++i, ps += This::shdr_size) @@ -242,7 +243,8 @@ Sized_relobj::do_read_relocs(Read_relocs_data* rd) Section_relocs& sr(rd->relocs.back()); sr.reloc_shndx = i; sr.data_shndx = shndx; - sr.contents = this->get_lasting_view(shdr.get_sh_offset(), sh_size); + sr.contents = this->get_lasting_view(shdr.get_sh_offset(), sh_size, + true); sr.sh_type = sh_type; sr.reloc_count = reloc_count; } @@ -261,7 +263,7 @@ Sized_relobj::do_read_relocs(Read_relocs_data* rd) gold_assert(loccount == symtabshdr.get_sh_info()); off_t locsize = loccount * sym_size; rd->local_symbols = this->get_lasting_view(symtabshdr.get_sh_offset(), - locsize); + locsize, true); } } @@ -316,7 +318,8 @@ Sized_relobj::do_relocate(const General_options& options, // Read the section headers. const unsigned char* pshdrs = this->get_view(this->elf_file_.shoff(), - shnum * This::shdr_size); + shnum * This::shdr_size, + true); Views views; views.resize(shnum); @@ -455,7 +458,7 @@ Sized_relobj::relocate_sections( off_t sh_size = shdr.get_sh_size(); const unsigned char* prelocs = this->get_view(shdr.get_sh_offset(), - sh_size); + sh_size, false); unsigned int reloc_size; if (sh_type == elfcpp::SHT_REL) diff --git a/gold/symtab.cc b/gold/symtab.cc index 8cd55cd..c68b9ca 100644 --- a/gold/symtab.cc +++ b/gold/symtab.cc @@ -1550,7 +1550,8 @@ Warnings::note_warnings(Symbol_table* symtab) Task_locker_obj tl(*p->second.object); const unsigned char* c; off_t len; - c = p->second.object->section_contents(p->second.shndx, &len); + c = p->second.object->section_contents(p->second.shndx, &len, + false); p->second.set_text(reinterpret_cast(c), len); } } -- 2.7.4