Fix mmap_is_attached_or_noreserve

[Takashi Yano]

Hi Ken,

On Thu, 26 Dec 2024 11:56:00 -0500
Ken Brown wrote:
> diff --git a/winsup/cygwin/mm/mmap.cc b/winsup/cygwin/mm/mmap.cc
> index 13418d782baf..10e3f5bdc2b3 100644
> --- a/winsup/cygwin/mm/mmap.cc
> +++ b/winsup/cygwin/mm/mmap.cc
> @@ -580,18 +580,61 @@ mmap_record::free_fh (fhandler_base *fh)
>      delete fh;
>  }
>  
> +/* Keep the list of anonymous mappings sorted as follows: All attached
> +   records come first, then those that are noreserve but not attached,
> +   then those that are not noreserve.  Those that are noreserve but
> +   not attached are sorted by address.  This order of the mappings is
> +   used in mmap_is_attached_or_noreserve.  Note: The code that follows
> +   makes implicit use of the fact that attached maps are also
> +   noreserve. */
>  mmap_record *
>  mmap_list::add_record (mmap_record &r)
>  {
> -  mmap_record *rec = (mmap_record *) ccalloc (HEAP_MMAP,
> -		      sizeof (mmap_record)
> -		      + MAPSIZE (PAGE_CNT (r.get_len ())) * sizeof (DWORD), 1);
> -  if (!rec)
> +  mmap_record *new_rec = (mmap_record *) ccalloc (HEAP_MMAP,
> +			 sizeof (mmap_record)
> +			 + MAPSIZE (PAGE_CNT (r.get_len ()))
> +			 * sizeof (DWORD), 1);
> +  if (!new_rec)
>      return NULL;
> -  rec->init_page_map (r);
> +  new_rec->init_page_map (r);
> +
> +  bool new_attached = new_rec->attached ();
> +  bool new_noreserve = new_rec->noreserve ();
> +  caddr_t new_addr = new_rec->get_address ();
> +
> +  LIST_WRITE_LOCK ();

This patch atempt to acquire lock despite the lock has been
already acquired. I think this is the cause of deadlock.

> +  if (fd != -1 || LIST_EMPTY (&recs))
> +    {
> +      LIST_INSERT_HEAD (&recs, new_rec, mr_next);
> +      goto out;
> +    }
>  
> -  LIST_INSERT_HEAD (&recs, rec, mr_next);
> -  return rec;
> +  /* Now we're working on the anonymous list, which is non-empty. */
> +  mmap_record *rec, *prev;
> +  LIST_FOREACH (rec, &recs, mr_next)
> +    {
> +      prev = rec;
> +      if (new_attached)
> +	{
> +	  LIST_INSERT_HEAD (&recs, new_rec, mr_next);
> +	  goto out;
> +	}
> +      if (new_noreserve && (rec->attached ()
> +			    || (rec->noreserve ()
> +				&& new_addr > rec->get_address ())))
> +	continue;
> +      if (!new_noreserve && rec->noreserve ())
> +	continue;
> +      LIST_INSERT_BEFORE (rec, new_rec, mr_next);
> +      goto out;
> +    }
> +
> +  /* If we get here, new_rec should be inserted at the end of the
> +     list, and prev points to the last record. */
> +  LIST_INSERT_AFTER (prev, new_rec, mr_next);
> +out:
> +  LIST_WRITE_UNLOCK ();
> +  return new_rec;
>  }
>  
>  void

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>