[PATCH] gold: ensure file_counts_lock is initialized before using

Nick Gasson nick.gasson@arm.com
Fri Oct 30 02:38:27 GMT 2020


> Since upgrading to binutils 2.35 I've been experiencing random memory
> corruption related crashes with ld.gold --threads. It's caused by
> multiple threads concurrently pushing elements onto the shared
> std::vector in File_read::record_file_read(). This vector is supposed to
> be protected by file_counts_lock, but that is initialized lazily and
> might be NULL when File_read::open() is called, in which case
> Hold_optional_lock silently skips locking it.
>

Is anyone able review this patch? This bug is quite easy to reproduce
with e.g. the binutils in Debian unstable:

$ gcc -fuse-ld=gold -Wl,--threads test.c 
malloc(): invalid next size (unsorted)
collect2: fatal error: ld terminated with signal 6 [Aborted], core dumped
compilation terminated.
$ gcc -fuse-ld=gold -Wl,--threads test.c 
$ gcc -fuse-ld=gold -Wl,--threads test.c 
double free or corruption (out)
collect2: fatal error: ld terminated with signal 11 [Segmentation fault], core dumped
compilation terminated.

> Fix by calling the initialize() method before attempting to acquire the
> lock, the same as other places that use file_counts_lock.
>
>        * fileread.cc (File_read::open): Ensure file_counts_lock is
>        initialized.
> ---
>  gold/fileread.cc | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/gold/fileread.cc b/gold/fileread.cc
> index f5ca719360d6..0b5228e2afd4 100644
> --- a/gold/fileread.cc
> +++ b/gold/fileread.cc
> @@ -212,6 +212,7 @@ File_read::open(const Task* task, const std::string& name)
>        gold_debug(DEBUG_FILES, "Attempt to open %s succeeded",
>  		 this->name_.c_str());
>        this->token_.add_writer(task);
> +      file_counts_initialize_lock.initialize();
>        Hold_optional_lock hl(file_counts_lock);
>        record_file_read(this->name_);
>      }

--
Thanks,
Nick


More information about the Binutils mailing list