Bug 95378 - __atomic_load will write to objects of cv-qualified types
Summary: __atomic_load will write to objects of cv-qualified types
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: 11.0
Assignee: Jonathan Wakely
URL:
Keywords: accepts-invalid, patch
Depends on:
Blocks:
 
Reported: 2020-05-27 22:08 UTC by Jonathan Wakely
Modified: 2020-06-17 19:06 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-05-27 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Wakely 2020-05-27 22:08:59 UTC
GCC happily compiles this:

void f()
{
  int i = 1;
  const int c = 0;
  __atomic_load(&i, &c, __ATOMIC_SEQ_CST);
  volatile int v = 0;
  __atomic_load(&i, &v, __ATOMIC_SEQ_CST);
  const volatile int cv = 0;
  __atomic_load(&i, &cv, __ATOMIC_SEQ_CST);
}

Clang sensibly rejects it:

af.cc:5:21: error: cannot initialize a parameter of type 'int *' with an rvalue of type 'const int *'
  __atomic_load(&i, &c, __ATOMIC_SEQ_CST);
                    ^~
af.cc:7:21: error: cannot initialize a parameter of type 'int *' with an rvalue of type 'volatile int *'
  __atomic_load(&i, &v, __ATOMIC_SEQ_CST);
                    ^~
af.cc:9:21: error: cannot initialize a parameter of type 'int *' with an rvalue of type 'const volatile int *'
  __atomic_load(&i, &cv, __ATOMIC_SEQ_CST);
                    ^~~
Comment 1 Jonathan Wakely 2020-05-27 22:33:52 UTC
I'm testing a patch.
Comment 2 Jonathan Wakely 2020-05-27 22:57:00 UTC
Every __atomic_xxx built-in has the same problem. They'll all accept cv-qualified types as output parameters.

This seems to fix it, but I'll finish testing it and submit it tomorrow:

--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -6903,6 +6903,8 @@ get_atomic_generic_size (location_t loc, tree function,
 {
   unsigned int n_param;
   unsigned int n_model;
+  unsigned int n_first_output = 0;
+  unsigned int n_last_output = 0;
   unsigned int x;
   int size_0;
   tree type_0;
@@ -6915,11 +6917,14 @@ get_atomic_generic_size (location_t loc, tree function,
       n_model = 1;
       break;
     case BUILT_IN_ATOMIC_LOAD:
+      n_first_output = n_last_output = 1;
+      /* fallthrough  */
     case BUILT_IN_ATOMIC_STORE:
       n_param = 3;
       n_model = 1;
       break;
     case BUILT_IN_ATOMIC_COMPARE_EXCHANGE:
+      n_last_output = 1;
       n_param = 6;
       n_model = 2;
       break;
@@ -7010,6 +7015,23 @@ get_atomic_generic_size (location_t loc, tree function,
                    function);
          return 0;
        }
+
+      if (x >= n_first_output && x <= n_last_output)
+       {
+         tree type = TREE_TYPE (TREE_TYPE ((*params)[x]));
+         if (TYPE_QUALS (type) & (TYPE_QUAL_CONST|TYPE_QUAL_VOLATILE))
+           {
+             if (c_dialect_cxx ())
+               {
+                 error_at (loc, "argument %d of %qE must not be a pointer to "
+                           "a cv-qualified type", x + 1, function);
+                 return 0;
+               }
+             else
+               pedwarn (loc, OPT_Wincompatible_pointer_types, "argument %d "
+                        "of %qE discards qualifiers", x + 1, function);
+           }
+       }
     }
 
   /* Check memory model parameters for validity.  */
Comment 3 Jonathan Wakely 2020-06-16 20:12:59 UTC
GCC correctly refuses to write through const pointers for __atomic_test_and_set, __atomic_clear, and all of __atomic_fetch_op and __atomic_op_fetch.
Comment 4 CVS Commits 2020-06-16 21:35:07 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:e40b11a91cb345db1324c3cb8f75b01e28056693

commit r11-1401-ge40b11a91cb345db1324c3cb8f75b01e28056693
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jun 16 22:34:55 2020 +0100

    libstdc++: Strip cv-qualifiers in std::atomic<FP> (PR 95282)
    
    This improves the previous fix for PR 95282, and extends it to also
    apply to the exchange function (which has a similar problem and would
    become ill-formed with my proposed fix for PR 95378).
    
            PR libstdc++/95282
            * include/bits/atomic_base.h (__atomic_impl::load): Use the _Val
            alias instead of deducing _Tp as an unqualified type.
            (__atomic_impl::exchange): Use the _Val alias to remove volatile
            from the reinterpret_cast result type.
Comment 5 Jonathan Wakely 2020-06-16 22:16:00 UTC
Patch posted for review:
https://gcc.gnu.org/pipermail/gcc-patches/2020-June/548283.html
Comment 6 Jonathan Wakely 2020-06-17 19:06:02 UTC
Fixed on master.