gold: fix race in FileRead::~View.

Ian Lance Taylor iant@google.com
Tue Dec 14 15:26:00 GMT 2010


Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:

> BTW, is there a coding style recommendation on leading white space
> (a TAB for eight spaces, or expanded)?

Not really.  I prefer a tab, but many Google programmers use editor
modes to expand all tabs to spaces, and I didn't try to fight it.


> gold/ChangeLog:
> 2010-12-12  Ralf Wildenhues  <Ralf.Wildenhues@gmx.de>
>
> 	* fileread.cc (file_counts_lock, file_counts_initialize_lock)
> 	(total_mapped_bytes, current_mapped_bytes, maximum_mapped_bytes):
> 	Move definition before File_read::View member definitions.
> 	(~View): Initialize and hold lock before updating
> 	current_mapped_bytes.

I would usually write File_read::View::~View in the ChangeLog entry.


>      case DATA_MMAPPED:
> -      if (::munmap(const_cast<unsigned char*>(this->data_), this->size_) != 0)
> -        gold_warning(_("munmap failed: %s"), strerror(errno));
> -      File_read::current_mapped_bytes -= this->size_;
> -      break;
> +      {
> +	if (::munmap(const_cast<unsigned char*>(this->data_), this->size_) != 0)
> +	  gold_warning(_("munmap failed: %s"), strerror(errno));
> +	file_counts_initialize_lock.initialize();
> +	Hold_optional_lock hl(file_counts_lock);
> +	File_read::current_mapped_bytes -= this->size_;
> +	break;

Only do the lock and stats update if necessary.  Write

  if (!parameters->options_valid() || parameters->options().stats())
    {
      file_counts_initialize_lock.initialize();
      Hold_optional_lock hl(file_counts_lock);
      File_read::current_mapped_bytes -= this->size_;
    }

as in File_read::release.

This is OK with that change.

Thanks for noticing this and thanks for the patch.

Ian



More information about the Binutils mailing list