c++/modules: Handle location exhaustion in write_location [PR105443]

This patch is missing a testcase because I wasn't able to easily
construct one that reliably exhausts all location_t values and then
causes an error.

Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

-- >8 --

The 'location_t' type currently only stores a limited number of distinct
locations.  In some cases, if many modules are imported that sum up to a
large number of locations, we may run out of room to represent new
locations for these imported declarations.  In such a case, any new
declarations from the affected modules simply get given a location of
"the module interface as a whole".

'write_locations' sometimes gets confused when this happens: it finds that
the location is a location we've noted to get streamed out, but it's
inconsistent whether it's an ordinary location from the current module
or an imported location from a different module.  This causes
random-looking locations to be associated with these declarations, and
occasionally (checking-only) ICEs.

This patch fixes the issue by first checking whether an ordinary
location represents a module (rather than a location inside a module);
if so, we instead write the location of the point that we imported this
module.  This will continue recursively in case the importing location
also was not able to be stored.

We only need to handle this in the IS_ORDINARY_LOC case: even for
locations originally within macro expansions, the remapping logic for
location exhaustion will make them look like ordinary locs again.

This is a relatively expensive addition, so this new check only occurs
if we've noted resource exhaustion has occurred while preparing imported
line maps, or in checking builds.

	PR c++/105443

gcc/cp/ChangeLog:

	* module.cc (loc_spans::locs_exhausted_p): New field.
	(loc_spans::loc_spans): Initialise it.
	(loc_spans::locations_exhausted_p): New function.
	(module_state::read_prepare_maps): Move inform into...
	(loc_spans::report_location_exhaustion): ...this new function.
	(module_state::write_location): Check for writing module
	locations stored due to resource exhaustion.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/module.cc | 42 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

On 11/4/24 5:35 PM, Nathaniel Shead wrote:
> This patch is missing a testcase because I wasn't able to easily
> construct one that reliably exhausts all location_t values and then
> causes an error.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

OK.

> -- >8 --
> 
> The 'location_t' type currently only stores a limited number of distinct
> locations.  In some cases, if many modules are imported that sum up to a
> large number of locations, we may run out of room to represent new
> locations for these imported declarations.  In such a case, any new
> declarations from the affected modules simply get given a location of
> "the module interface as a whole".
> 
> 'write_locations' sometimes gets confused when this happens: it finds that
> the location is a location we've noted to get streamed out, but it's
> inconsistent whether it's an ordinary location from the current module
> or an imported location from a different module.  This causes
> random-looking locations to be associated with these declarations, and
> occasionally (checking-only) ICEs.
> 
> This patch fixes the issue by first checking whether an ordinary
> location represents a module (rather than a location inside a module);
> if so, we instead write the location of the point that we imported this
> module.  This will continue recursively in case the importing location
> also was not able to be stored.
> 
> We only need to handle this in the IS_ORDINARY_LOC case: even for
> locations originally within macro expansions, the remapping logic for
> location exhaustion will make them look like ordinary locs again.
> 
> This is a relatively expensive addition, so this new check only occurs
> if we've noted resource exhaustion has occurred while preparing imported
> line maps, or in checking builds.
> 
> 	PR c++/105443
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (loc_spans::locs_exhausted_p): New field.
> 	(loc_spans::loc_spans): Initialise it.
> 	(loc_spans::locations_exhausted_p): New function.
> 	(module_state::read_prepare_maps): Move inform into...
> 	(loc_spans::report_location_exhaustion): ...this new function.
> 	(module_state::write_location): Check for writing module
> 	locations stored due to resource exhaustion.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/module.cc | 42 ++++++++++++++++++++++++++++++++++--------
>   1 file changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index dde7e5f6dbf..e64e6a2103a 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -3233,12 +3233,13 @@ public:
>   
>   private:
>     vec<span> *spans;
> +  bool locs_exhausted_p;
>   
>   public:
>     loc_spans ()
>       /* Do not preallocate spans, as that causes
>          --enable-detailed-mem-stats problems.  */
> -    : spans (nullptr)
> +    : spans (nullptr), locs_exhausted_p (false)
>     {
>     }
>     ~loc_spans ()
> @@ -3293,6 +3294,22 @@ public:
>     /* Propagate imported linemaps to us, if needed.  */
>     bool maybe_propagate (module_state *import, location_t loc);
>   
> +public:
> +  /* Whether we can no longer represent new imported locations.  */
> +  bool locations_exhausted_p () const
> +  {
> +    return locs_exhausted_p;
> +  }
> +  void report_location_exhaustion (location_t loc)
> +  {
> +    if (!locs_exhausted_p)
> +      {
> +	/* Just give the notice once.  */
> +	locs_exhausted_p = true;
> +	inform (loc, "unable to represent further imported source locations");
> +      }
> +  }
> +
>   public:
>     const span *ordinary (location_t);
>     const span *macro (location_t);
> @@ -16453,6 +16470,21 @@ module_state::write_location (bytes_out &sec, location_t loc)
>       }
>     else if (IS_ORDINARY_LOC (loc))
>       {
> +      /* If we ran out of locations for imported decls, this location could
> +	 be a module unit's location.  In that case, remap the location
> +	 to be where we imported the module from.  */
> +      if (spans.locations_exhausted_p () || CHECKING_P)
> +	{
> +	  const line_map_ordinary *map
> +	    = linemap_check_ordinary (linemap_lookup (line_table, loc));
> +	  if (MAP_MODULE_P (map) && loc == MAP_START_LOCATION (map))
> +	    {
> +	      gcc_checking_assert (spans.locations_exhausted_p ());
> +	      write_location (sec, linemap_included_from (map));
> +	      return;
> +	    }
> +	}
> +
>         const ord_loc_info *info = nullptr;
>         unsigned offset = 0;
>         if (unsigned hwm = ord_loc_remap->length ())
> @@ -16783,13 +16815,7 @@ module_state::read_prepare_maps (const module_state_config *cfg)
>     ordinary_locs.first = ordinary_locs.second = 0;
>     macro_locs.first = macro_locs.second = 0;
>   
> -  static bool informed = false;
> -  if (!informed)
> -    {
> -      /* Just give the notice once.  */
> -      informed = true;
> -      inform (loc, "unable to represent further imported source locations");
> -    }
> +  spans.report_location_exhaustion (loc);
>   
>     return false;
>   }