diff mbox series

[PING] Don't maintain a warning spec for 'UNKNOWN_LOCATION'/'BUILTINS_LOCATION' [PR101574] (was: [PATCH 1/13] v2 [PATCH 1/13] Add support for per-location warning groups (PR 74765))

Message ID 87tuisub5r.fsf@euler.schwinge.homeip.net
State New
Headers show
Series [PING] Don't maintain a warning spec for 'UNKNOWN_LOCATION'/'BUILTINS_LOCATION' [PR101574] (was: [PATCH 1/13] v2 [PATCH 1/13] Add support for per-location warning groups (PR 74765)) | expand

Commit Message

Thomas Schwinge Sept. 10, 2021, 7:45 a.m. UTC 
Hi!

Ping.

On 2021-09-03T21:16:46+0200, Thomas Schwinge <thomas@codesourcery.com> wrote:
> Martin, thanks for your review.  Now need someone to formally approve the
> third patch.

Again attached for easy reference.


Grüße
 Thomas


> On 2021-09-01T18:14:46-0600, Martin Sebor <msebor@gmail.com> wrote:
>> On 9/1/21 1:35 PM, Thomas Schwinge wrote:
>>> On 2021-06-23T13:47:08-0600, Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>>> On 6/22/21 5:28 PM, David Malcolm wrote:
>>>>> On Tue, 2021-06-22 at 19:18 -0400, David Malcolm wrote:
>>>>>> On Fri, 2021-06-04 at 15:41 -0600, Martin Sebor wrote:
>>>>>>> The attached patch introduces the suppress_warning(),
>>>>>>> warning_suppressed(), and copy_no_warning() APIs [etc.]
>
>>> I now had a bit of a deep dive into some aspects of this, in context of
>>> <https://gcc.gnu.org/PR101574> "gcc/sparseset.h:215:20: error: suggest
>>> parentheses around assignment used as truth value [-Werror=parentheses]"
>>> that I recently filed.  This seems difficult to reproduce, but I'm still
>>> able to reliably reproduce it in one specific build
>>> configuration/directory/machine/whatever.  Initially, we all quickly
>>> assumed that it'd be some GC issue -- but "alas", it's not, at least not
>>> directly.  (But I'll certainly assume that some GC aspects are involved
>>> which make this issue come and go across different GCC sources revisions,
>>> and difficult to reproduce.)
>
>>> First, two pieces of cleanup:
>
> ACKed by Martin, again attached for convenience.
>
>
>>>> --- /dev/null
>>>> +++ b/gcc/diagnostic-spec.h
>>>
>>>> +typedef location_t key_type_t;
>>>> +typedef int_hash <key_type_t, 0, UINT_MAX> xint_hash_t;
>
>> By the way, it seems we should probably also use a manifest constant
>> for Empty (probably UNKNOWN_LOCATION since we're reserving it).
>
> Yes, that will be part of another patch here -- waiting for approval of
> "Generalize 'gcc/input.h:struct location_hash'" posted elsewhere.
>
>
>>> attached "Don't maintain a warning spec for
>>> 'UNKNOWN_LOCATION'/'BUILTINS_LOCATION' [PR101574]".  OK to push?
>>
>> [...].  So I agree that it ought to be fixed.
>
>>> I'm reasonably confident that my changes are doing the right things in
>>> general, but please carefully review, especially here:
>>>
>>>    - 'gcc/warning-control.cc:suppress_warning' functions: is it correct to
>>>      conditionalize on '!RESERVED_LOCATION_P' the 'suppress_warning_at'
>>>      calls and 'supp' update?  Or, should instead 'suppress_warning_at'
>>>      handle the case of '!RESERVED_LOCATION_P'?  (How?)
>>
>> It seems like six of one vs half a dozen of the other.  I'd say go
>> with whatever makes more sense to you here :)
>
> OK, was just trying to make sure that I don't fail to see any non-obvious
> intentions here.
>
>>>    - 'gcc/diagnostic-spec.c:copy_warning' and
>>>      'gcc/warning-control.cc:copy_warning': is the rationale correct for
>>>      the 'gcc_checking_assert (!from_spec)': "If we cannot set no-warning
>>>      dispositions for 'to', ascertain that we don't have any for 'from'.
>>>      Otherwise, we'd lose these."?  If the rationale is correct, then
>>>      observing that in 'gcc/warning-control.cc:copy_warning' this
>>>      currently "triggers during GCC build" is something to be looked into,
>>>      later, I suppose, and otherwise, how should I change this code?
>>
>> copy_warning(location_t, location_t) is called [only] from
>> gimple_set_location().  The middle end does clear the location of
>> some statements for which it was previously valid (e.g., return
>> statements).
>
> What I observed was that the 'assert' never triggered for the
> 'location_t' variant "called [only] from gimple_set_location" -- but does
> trigger for some other variant.  Anyway:
>
>> So I wouldn't expect this assumption to be safe.  If
>> that happens, we have no choice but to lose the per-warning detail
>> and fall back on the no-warning bit.
>
> ACK.  I'm thus clarifying that as follows:
>
>     --- gcc/diagnostic-spec.c
>     +++ gcc/diagnostic-spec.c
>     @@ -185,7 +185,5 @@ copy_warning (location_t to, location_t from)
>        if (RESERVED_LOCATION_P (to))
>     -    {
>     -      /* If we cannot set no-warning dispositions for 'to', ascertain that we
>     -    don't have any for 'from'.  Otherwise, we'd lose these.  */
>     -      gcc_checking_assert (!from_spec);
>     -    }
>     +    /* We cannot set no-warning dispositions for 'to', so we have no chance but
>     +       lose those potentially set for 'from'.  */
>     +    ;
>        else
>     --- gcc/warning-control.cc
>     +++ gcc/warning-control.cc
>     @@ -197,9 +197,5 @@ void copy_warning (ToType to, FromType from)
>        if (RESERVED_LOCATION_P (to_loc))
>     -    {
>     -#if 0 //TODO triggers during GCC build
>     -      /* If we cannot set no-warning dispositions for 'to', ascertain that we
>     -    don't have any for 'from'.  Otherwise, we'd lose these.  */
>     -      gcc_checking_assert (!from_spec);
>     -#endif
>     -    }
>     +    /* We cannot set no-warning dispositions for 'to', so we have no chance but
>     +       lose those potentially set for 'from'.  */
>     +    ;
>        else
>
>> (Either David or a middle end maintainer will need to approve the last
>> patch once it's final.)
>
> As far as I'm concerned that would be the attached third patch:
> "Don't maintain a warning spec for 'UNKNOWN_LOCATION'/'BUILTINS_LOCATION'
> [PR101574]".  OK to push?
>
>
> Grüße
>  Thomas
>
>
>>> PS.  Relevant code quoted for reference, in case that's useful:
>>>
>>>> --- /dev/null
>>>> +++ b/gcc/diagnostic-spec.h
>>>
>>>> [...]
>>>
>>>> +typedef location_t key_type_t;
>>>> +typedef int_hash <key_type_t, 0, UINT_MAX> xint_hash_t;
>>>> +typedef hash_map<xint_hash_t, nowarn_spec_t> xint_hash_map_t;
>>>> +
>>>> +/* A mapping from the location of an expression to the warning spec
>>>> +   set for it.  */
>>>> +extern GTY(()) xint_hash_map_t *nowarn_map;
>>>
>>>> [...]
>>>
>>>> --- /dev/null
>>>> +++ b/gcc/diagnostic-spec.c
>>>
>>>> [...]
>>>
>>>> +/* Map from location to its no-warning disposition.  */
>>>> +
>>>> +GTY(()) xint_hash_map_t *nowarn_map;
>>>> +
>>>> +/* Return the no-warning disposition for location LOC and option OPT
>>>> +   or for all/any otions by default.  */
>>>> +
>>>> +bool
>>>> +warning_suppressed_at (location_t loc, opt_code opt /* = all_warnings */)
>>>> +{
>>>> +  if (!nowarn_map)
>>>> +    return false;
>>>> +
>>>> +  if (const nowarn_spec_t* const pspec = nowarn_map->get (loc))
>>>> +    {
>>>> +      const nowarn_spec_t optspec (opt);
>>>> +      return *pspec & optspec;
>>>> +    }
>>>> +
>>>> +  return false;
>>>> +}
>>>> +
>>>> + /* Change the supression of warnings at location LOC.
>>>> +    OPT controls which warnings are affected.
>>>> +    The wildcard OPT of -1 controls all warnings.
>>>> +    If SUPP is true (the default), enable the suppression of the warnings.
>>>> +    If SUPP is false, disable the suppression of the warnings.  */
>>>> +
>>>> +bool
>>>> +suppress_warning_at (location_t loc, opt_code opt /* = all_warnings */,
>>>> +                  bool supp /* = true */)
>>>> +{
>>>> +  const nowarn_spec_t optspec (supp ? opt : opt_code ());
>>>> +
>>>> +  if (nowarn_spec_t *pspec = nowarn_map ? nowarn_map->get (loc) : NULL)
>>>> +    {
>>>> +      if (supp)
>>>> +     {
>>>> +       *pspec |= optspec;
>>>> +       return true;
>>>> +     }
>>>> +
>>>> +      *pspec &= optspec;
>>>> +      if (*pspec)
>>>> +     return true;
>>>> +
>>>> +      nowarn_map->remove (loc);
>>>> +      return false;
>>>> +    }
>>>> +
>>>> +  if (!supp || opt == no_warning)
>>>> +    return false;
>>>> +
>>>> +  if (!nowarn_map)
>>>> +    nowarn_map = xint_hash_map_t::create_ggc (32);
>>>> +
>>>> +  nowarn_map->put (loc, optspec);
>>>> +  return true;
>>>> +}
>>>> +
>>>> +/* Copy the no-warning disposition from one location to another.  */
>>>> +
>>>> +void
>>>> +copy_warning (location_t to, location_t from)
>>>> +{
>>>> +  if (!nowarn_map)
>>>> +    return;
>>>> +
>>>> +  if (nowarn_spec_t *pspec = nowarn_map->get (from))
>>>> +    nowarn_map->put (to, *pspec);
>>>> +  else
>>>> +    nowarn_map->remove (to);
>>>> +}
>>>
>>>> --- /dev/null
>>>> +++ b/gcc/warning-control.cc
>>>
>>>> [...]
>>>
>>>> +/* Return the no-warning bit for EXPR.  */
>>>> +
>>>> +static inline bool
>>>> +get_no_warning_bit (const_tree expr)
>>>> +{
>>>> +  return expr->base.nowarning_flag;
>>>> +}
>>>> +
>>>> +/* Return the no-warning bit for statement STMT.  */
>>>> +
>>>> +static inline bool
>>>> +get_no_warning_bit (const gimple *stmt)
>>>> +{
>>>> +  return stmt->no_warning;
>>>> +}
>>>> +
>>>> +/* Set the no-warning bit for EXPR to VALUE.  */
>>>> +
>>>> +static inline void
>>>> +set_no_warning_bit (tree expr, bool value)
>>>> +{
>>>> +  expr->base.nowarning_flag = value;
>>>> +}
>>>> +
>>>> +/* Set the no-warning bit for statement STMT to VALUE.  */
>>>> +
>>>> +static inline void
>>>> +set_no_warning_bit (gimple *stmt, bool value)
>>>> +{
>>>> +  stmt->no_warning = value;
>>>> +}
>>>> +
>>>> +/* Return EXPR location or zero.  */
>>>> +
>>>> +static inline key_type_t
>>>> +convert_to_key (const_tree expr)
>>>> +{
>>>> +  if (DECL_P (expr))
>>>> +    return DECL_SOURCE_LOCATION (expr);
>>>> +  if (EXPR_P (expr))
>>>> +    return EXPR_LOCATION (expr);
>>>> +  return 0;
>>>> +}
>>>> +
>>>> +/* Return STMT location (may be zero).  */
>>>> +
>>>> +static inline key_type_t
>>>> +convert_to_key (const gimple *stmt)
>>>> +{
>>>> +  return gimple_location (stmt);
>>>> +}
>>>> +
>>>> +/* Return the no-warning bitmap for decl/expression EXPR.  */
>>>> +
>>>> +static nowarn_spec_t *
>>>> +get_nowarn_spec (const_tree expr)
>>>> +{
>>>> +  const key_type_t key = convert_to_key (expr);
>>>> +
>>>> +  if (!get_no_warning_bit (expr) || !key)
>>>> +    return NULL;
>>>> +
>>>> +  return nowarn_map ? nowarn_map->get (key) : NULL;
>>>> +}
>>>> +
>>>> +/* Return the no-warning bitmap for stateemt STMT.  */
>>>> +
>>>> +static nowarn_spec_t *
>>>> +get_nowarn_spec (const gimple *stmt)
>>>> +{
>>>> +  const key_type_t key = convert_to_key (stmt);
>>>> +
>>>> +  if (!get_no_warning_bit (stmt))
>>>> +    return NULL;
>>>> +
>>>> +  return nowarn_map ? nowarn_map->get (key) : NULL;
>>>> +}
>>>> +
>>>> +/* Return true if warning OPT is suppressed for decl/expression EXPR.
>>>> +   By default tests the disposition for any warning.  */
>>>> +
>>>> +bool
>>>> +warning_suppressed_p (const_tree expr, opt_code opt /* = all_warnings */)
>>>> +{
>>>> +  const nowarn_spec_t *spec = get_nowarn_spec (expr);
>>>> +
>>>> +  if (!spec)
>>>> +    return get_no_warning_bit (expr);
>>>> +
>>>> +  const nowarn_spec_t optspec (opt);
>>>> +  bool dis = *spec & optspec;
>>>> +  gcc_assert (get_no_warning_bit (expr) || !dis);
>>>> +  return dis;
>>>> +}
>>>> +
>>>> +/* Return true if warning OPT is suppressed for statement STMT.
>>>> +   By default tests the disposition for any warning.  */
>>>> +
>>>> +bool
>>>> +warning_suppressed_p (const gimple *stmt, opt_code opt /* = all_warnings */)
>>>> +{
>>>> +  const nowarn_spec_t *spec = get_nowarn_spec (stmt);
>>>> +
>>>> +  if (!spec)
>>>> +    /* Fall back on the single no-warning bit.  */
>>>> +    return get_no_warning_bit (stmt);
>>>> +
>>>> +  const nowarn_spec_t optspec (opt);
>>>> +  bool dis = *spec & optspec;
>>>> +  gcc_assert (get_no_warning_bit (stmt) || !dis);
>>>> +  return dis;
>>>> +}
>>>> +
>>>> +/* Enable, or by default disable, a warning for the expression.
>>>> +   The wildcard OPT of -1 controls all warnings.  */
>>>> +
>>>> +void
>>>> +suppress_warning (tree expr, opt_code opt /* = all_warnings */,
>>>> +               bool supp /* = true */)
>>>> +{
>>>> +  if (opt == no_warning)
>>>> +    return;
>>>> +
>>>> +  const key_type_t key = convert_to_key (expr);
>>>> +
>>>> +  supp = suppress_warning_at (key, opt, supp) || supp;
>>>> +  set_no_warning_bit (expr, supp);
>>>> +}
>>>> +
>>>> +/* Enable, or by default disable, a warning for the statement STMT.
>>>> +   The wildcard OPT of -1 controls all warnings.  */
>>>> +
>>>> +void
>>>> +suppress_warning (gimple *stmt, opt_code opt /* = all_warnings */,
>>>> +               bool supp /* = true */)
>>>> +{
>>>> +  if (opt == no_warning)
>>>> +    return;
>>>> +
>>>> +  const key_type_t key = convert_to_key (stmt);
>>>> +
>>>> +  supp = suppress_warning_at (key, opt, supp) || supp;
>>>> +  set_no_warning_bit (stmt, supp);
>>>> +}
>>>> +
>>>> +/* Copy the warning disposition mapping between an expression and/or
>>>> +   a statement.  */
>>>> +
>>>> +template <class ToType, class FromType>
>>>> +void copy_warning (ToType to, FromType from)
>>>> +{
>>>> +  const key_type_t to_key = convert_to_key (to);
>>>> +
>>>> +  if (nowarn_spec_t *from_map = get_nowarn_spec (from))
>>>> +    {
>>>> +      /* If there's an entry in the map the no-warning bit must be set.  */
>>>> +      gcc_assert (get_no_warning_bit (from));
>>>> +
>>>> +      if (!nowarn_map)
>>>> +     nowarn_map = xint_hash_map_t::create_ggc (32);
>>>> +
>>>> +      nowarn_map->put (to_key, *from_map);
>>>> +      set_no_warning_bit (to, true);
>>>> +    }
>>>> +  else
>>>> +    {
>>>> +      if (nowarn_map)
>>>> +     nowarn_map->remove (to_key);
>>>> +
>>>> +      /* The no-warning bit might be set even if there's no entry
>>>> +      in the map.  */
>>>> +      set_no_warning_bit (to, get_no_warning_bit (from));
>>>> +    }
>>>> +}
>>>> +
>>>> +/* Copy the warning disposition mapping from one expression to another.  */
>>>> +
>>>> +void
>>>> +copy_warning (tree to, const_tree from)
>>>> +{
>>>> +  copy_warning<tree, const_tree>(to, from);
>>>> +}
>>>> +
>>>> +/* Copy the warning disposition mapping from a statement to an expression.  */
>>>> +
>>>> +void
>>>> +copy_warning (tree to, const gimple *from)
>>>> +{
>>>> +  copy_warning<tree, const gimple *>(to, from);
>>>> +}
>>>> +
>>>> +/* Copy the warning disposition mapping from an expression to a statement.  */
>>>> +
>>>> +void
>>>> +copy_warning (gimple *to, const_tree from)
>>>> +{
>>>> +  copy_warning<gimple *, const_tree>(to, from);
>>>> +}
>>>> +
>>>> +/* Copy the warning disposition mapping from one statement to another.  */
>>>> +
>>>> +void
>>>> +copy_warning (gimple *to, const gimple *from)
>>>> +{
>>>> +  copy_warning<gimple *, const gimple *>(to, from);
>>>> +}


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

Comments

Jeff Law Sept. 13, 2021, 2 p.m. UTC | #1 
On 9/10/2021 1:45 AM, Thomas Schwinge wrote:
>
> 0001-Simplify-gcc-diagnostic-spec.h-nowarn_map-setup.patch
>
>  From 095c16ead5d432726f2b6de5ce12fd367600076d Mon Sep 17 00:00:00 2001
> From: Thomas Schwinge <thomas@codesourcery.com>
> Date: Wed, 1 Sep 2021 16:48:55 +0200
> Subject: [PATCH 1/3] Simplify 'gcc/diagnostic-spec.h:nowarn_map' setup
>
> If we've just read something from the map, we can be sure that it exists.
>
> 	gcc/
> 	* warning-control.cc (copy_warning): Remove 'nowarn_map' setup.
> OK

>
> 0002-Clarify-key_type_t-to-location_t-as-used-for-gcc-dia.patch
>
>  From 23d9b93401349fca03efaef0fef0960933f4c316 Mon Sep 17 00:00:00 2001
> From: Thomas Schwinge <thomas@codesourcery.com>
> Date: Tue, 31 Aug 2021 22:01:23 +0200
> Subject: [PATCH 2/3] Clarify 'key_type_t' to 'location_t' as used for
>   'gcc/diagnostic-spec.h:nowarn_map'
>
> To make it obvious what exactly the key type is.  No change in behavior.
>
> 	gcc/
> 	* diagnostic-spec.h (typedef xint_hash_t): Use 'location_t' instead of...
> 	(typedef key_type_t): ... this.  Remove.
> 	(nowarn_map): Document.
> 	* diagnostic-spec.c (nowarn_map): Likewise.
> 	* warning-control.cc (convert_to_key): Evolve functions into...
> 	(get_location): ... these.  Adjust all users.
OK

>
> 0003-Don-t-maintain-a-warning-spec-for-UNKNOWN_LOCATION-B.patch
>
>  From 51c9a8ac2caa0432730c78d00989fd01f3ac6fe5 Mon Sep 17 00:00:00 2001
> From: Thomas Schwinge <thomas@codesourcery.com>
> Date: Mon, 30 Aug 2021 22:36:47 +0200
> Subject: [PATCH 3/3] Don't maintain a warning spec for
>   'UNKNOWN_LOCATION'/'BUILTINS_LOCATION' [PR101574]
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> This resolves PR101574 "gcc/sparseset.h:215:20: error: suggest parentheses
> around assignment used as truth value [-Werror=parentheses]", as (bogusly)
> reported at commit a61f6afbee370785cf091fe46e2e022748528307:
>
>      In file included from [...]/source-gcc/gcc/lra-lives.c:43:
>      [...]/source-gcc/gcc/lra-lives.c: In function ‘void make_hard_regno_dead(int)’:
>      [...]/source-gcc/gcc/sparseset.h:215:20: error: suggest parentheses around assignment used as truth value [-Werror=parentheses]
>        215 |        && (((ITER) = sparseset_iter_elm (SPARSESET)) || 1);             \
>            |            ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      [...]/source-gcc/gcc/lra-lives.c:304:3: note: in expansion of macro ‘EXECUTE_IF_SET_IN_SPARSESET’
>        304 |   EXECUTE_IF_SET_IN_SPARSESET (pseudos_live, i)
>            |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> 	gcc/
> 	PR bootstrap/101574
> 	* diagnostic-spec.c (warning_suppressed_at, copy_warning): Handle
> 	'RESERVED_LOCATION_P' locations.
> 	* warning-control.cc (get_nowarn_spec, suppress_warning)
> 	(copy_warning): Likewise.
OK.

Jeff
diff mbox series

Patch

From 51c9a8ac2caa0432730c78d00989fd01f3ac6fe5 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Mon, 30 Aug 2021 22:36:47 +0200
Subject: [PATCH 3/3] Don't maintain a warning spec for
 'UNKNOWN_LOCATION'/'BUILTINS_LOCATION' [PR101574]
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This resolves PR101574 "gcc/sparseset.h:215:20: error: suggest parentheses
around assignment used as truth value [-Werror=parentheses]", as (bogusly)
reported at commit a61f6afbee370785cf091fe46e2e022748528307:

    In file included from [...]/source-gcc/gcc/lra-lives.c:43:
    [...]/source-gcc/gcc/lra-lives.c: In function ‘void make_hard_regno_dead(int)’:
    [...]/source-gcc/gcc/sparseset.h:215:20: error: suggest parentheses around assignment used as truth value [-Werror=parentheses]
      215 |        && (((ITER) = sparseset_iter_elm (SPARSESET)) || 1);             \
          |            ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    [...]/source-gcc/gcc/lra-lives.c:304:3: note: in expansion of macro ‘EXECUTE_IF_SET_IN_SPARSESET’
      304 |   EXECUTE_IF_SET_IN_SPARSESET (pseudos_live, i)
          |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~

	gcc/
	PR bootstrap/101574
	* diagnostic-spec.c (warning_suppressed_at, copy_warning): Handle
	'RESERVED_LOCATION_P' locations.
	* warning-control.cc (get_nowarn_spec, suppress_warning)
	(copy_warning): Likewise.
---
 gcc/diagnostic-spec.c  | 22 ++++++++++++++++---
 gcc/warning-control.cc | 48 +++++++++++++++++++++++++++---------------
 2 files changed, 50 insertions(+), 20 deletions(-)

diff --git a/gcc/diagnostic-spec.c b/gcc/diagnostic-spec.c
index eac5a3317c8..85ffb725c02 100644
--- a/gcc/diagnostic-spec.c
+++ b/gcc/diagnostic-spec.c
@@ -115,6 +115,8 @@  GTY(()) xint_hash_map_t *nowarn_map;
 bool
 warning_suppressed_at (location_t loc, opt_code opt /* = all_warnings */)
 {
+  gcc_checking_assert (!RESERVED_LOCATION_P (loc));
+
   if (!nowarn_map)
     return false;
 
@@ -137,6 +139,8 @@  bool
 suppress_warning_at (location_t loc, opt_code opt /* = all_warnings */,
 		     bool supp /* = true */)
 {
+  gcc_checking_assert (!RESERVED_LOCATION_P (loc));
+
   const nowarn_spec_t optspec (supp ? opt : opt_code ());
 
   if (nowarn_spec_t *pspec = nowarn_map ? nowarn_map->get (loc) : NULL)
@@ -173,8 +177,20 @@  copy_warning (location_t to, location_t from)
   if (!nowarn_map)
     return;
 
-  if (nowarn_spec_t *pspec = nowarn_map->get (from))
-    nowarn_map->put (to, *pspec);
+  nowarn_spec_t *from_spec;
+  if (RESERVED_LOCATION_P (from))
+    from_spec = NULL;
+  else
+    from_spec = nowarn_map->get (from);
+  if (RESERVED_LOCATION_P (to))
+    /* We cannot set no-warning dispositions for 'to', so we have no chance but
+       lose those potentially set for 'from'.  */
+    ;
   else
-    nowarn_map->remove (to);
+    {
+      if (from_spec)
+	nowarn_map->put (to, *from_spec);
+      else
+	nowarn_map->remove (to);
+    }
 }
diff --git a/gcc/warning-control.cc b/gcc/warning-control.cc
index 8d6c0828445..36a47ab6bae 100644
--- a/gcc/warning-control.cc
+++ b/gcc/warning-control.cc
@@ -89,7 +89,7 @@  get_nowarn_spec (const_tree expr)
 {
   const location_t loc = get_location (expr);
 
-  if (loc == UNKNOWN_LOCATION)
+  if (RESERVED_LOCATION_P (loc))
     return NULL;
 
   if (!get_no_warning_bit (expr))
@@ -105,6 +105,9 @@  get_nowarn_spec (const gimple *stmt)
 {
   const location_t loc = get_location (stmt);
 
+  if (RESERVED_LOCATION_P (loc))
+    return NULL;
+
   if (!get_no_warning_bit (stmt))
     return NULL;
 
@@ -158,7 +161,8 @@  suppress_warning (tree expr, opt_code opt /* = all_warnings */,
 
   const location_t loc = get_location (expr);
 
-  supp = suppress_warning_at (loc, opt, supp) || supp;
+  if (!RESERVED_LOCATION_P (loc))
+    supp = suppress_warning_at (loc, opt, supp) || supp;
   set_no_warning_bit (expr, supp);
 }
 
@@ -174,7 +178,8 @@  suppress_warning (gimple *stmt, opt_code opt /* = all_warnings */,
 
   const location_t loc = get_location (stmt);
 
-  supp = suppress_warning_at (loc, opt, supp) || supp;
+  if (!RESERVED_LOCATION_P (loc))
+    supp = suppress_warning_at (loc, opt, supp) || supp;
   set_no_warning_bit (stmt, supp);
 }
 
@@ -186,24 +191,33 @@  void copy_warning (ToType to, FromType from)
 {
   const location_t to_loc = get_location (to);
 
-  if (nowarn_spec_t *from_map = get_nowarn_spec (from))
-    {
-      /* If there's an entry in the map the no-warning bit must be set.  */
-      gcc_assert (get_no_warning_bit (from));
+  bool supp = get_no_warning_bit (from);
 
-      gcc_checking_assert (nowarn_map);
-      nowarn_map->put (to_loc, *from_map);
-      set_no_warning_bit (to, true);
-    }
+  nowarn_spec_t *from_spec = get_nowarn_spec (from);
+  if (RESERVED_LOCATION_P (to_loc))
+    /* We cannot set no-warning dispositions for 'to', so we have no chance but
+       lose those potentially set for 'from'.  */
+    ;
   else
     {
-      if (nowarn_map)
-	nowarn_map->remove (to_loc);
-
-      /* The no-warning bit might be set even if there's no entry
-	 in the map.  */
-      set_no_warning_bit (to, get_no_warning_bit (from));
+      if (from_spec)
+	{
+	  /* If there's an entry in the map the no-warning bit must be set.  */
+	  gcc_assert (supp);
+
+	  gcc_checking_assert (nowarn_map);
+	  nowarn_map->put (to_loc, *from_spec);
+	}
+      else
+	{
+	  if (nowarn_map)
+	    nowarn_map->remove (to_loc);
+	}
     }
+
+  /* The no-warning bit might be set even if the map has not been consulted, or
+     otherwise if there's no entry in the map.  */
+  set_no_warning_bit (to, supp);
 }
 
 /* Copy the warning disposition mapping from one expression to another.  */
-- 
2.25.1