Warn for reads from write-only arguments [PR101734]

Warn for reads from write-only arguments [PR101734]

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

The write_only mode to attribute access specifies that the pointer
applies to is used to write to the referenced object but not read
from it.

A function that uses the pointer to read the referenced object might 
rely on the contents of uninitialized memory and so such attempts
should be diagnosed.  The attached enhancement makes that happen.
It was tested on x86_64-linux and by building Glibc where it found
an inappropriate use of the attribute on a function documented to
read from the argument as an extension [BZ 28170].

I plan to implement a similar warning for writes to read-only objects
(either with attribute read_only, or those declared const, or const
restrict) pointers in a followup (as part of my solution for PR 90404).

Martin

BZ 28170: https://sourceware.org/bugzilla/show_bug.cgi?id=28170

[-- Attachment #2: gcc-101374.diff --]
[-- Type: text/x-patch, Size: 5624 bytes --]

Warn for reads from write-only arguments [PR101734].

Resolves:
PR middle-end/101734 - missing warning reading from a write-only object

gcc/ChangeLog:

	PR middle-end/101734
	* tree-ssa-uninit.c (maybe_warn_read_write_only): New function.
	(maybe_warn_operand): Call it.

gcc/testsuite/ChangeLog:

	PR middle-end/101734
	* gcc.dg/uninit-42.c: New test.

diff --git a/gcc/testsuite/gcc.dg/uninit-42.c b/gcc/testsuite/gcc.dg/uninit-42.c
new file mode 100644
index 00000000000..5e0a2b4ea5f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/uninit-42.c
@@ -0,0 +1,85 @@
+/* Verify that reading objects pointed to by arguments declared with
+   attribute access none or write-only is diagnosed.
+   { dg-do compile }
+   { dg-options "-Wall" } */
+
+#define A(mode, ...) __attribute__ ((access (mode, __VA_ARGS__)))
+
+void sink (void *, ...);
+
+
+A (write_only, 1, 2)
+int nowarn_wo_assign_r0 (int *p, int n)
+{
+  *p = n;
+  return *p;
+}
+
+A (write_only, 1, 2)
+int nowarn_wo_sink_r0 (int *p, int n)
+{
+  sink (p, p + 1, p + n);
+  return *p;
+}
+
+A (write_only, 1, 2)
+int warn_wo_r0 (int *p, int n)
+{
+  return *p;        // { dg-warning "'\\*p' is used uninitialized" }
+}
+
+
+A (write_only, 1, 2)
+int nowarn_wo_w1_r1 (int *p, int n)
+{
+  p[1] = n;
+  return p[1];
+}
+
+A (write_only, 1, 2)
+int warn_wo_r1 (int *p, int n)
+{
+  return p[1];      // { dg-warning "'p\\\[1]' is used uninitialized" }
+}
+
+
+A (write_only, 1, 2)
+int nowarn_wo_rwi_rj (int *p, int n, int i, int j)
+{
+  p[i] = n;
+  return p[j];
+}
+
+A (write_only, 1, 2)
+  int warn_wo_ri (int *p, int n, int i)
+{
+  return p[i];      // { dg-warning " is used uninitialized" }
+}
+
+
+
+A (none, 1, 2)
+int* nowarn_none_sink_return (int *p, int n)
+{
+  sink (p, p + 1, p + n);
+  return p;
+}
+
+A (none, 1, 2)
+int warn_none_r0 (int *p, int n)
+{
+  (void)&n;
+  return *p;        // { dg-warning "'\\*p' is used uninitialized" }
+}
+
+A (none, 1, 2)
+int warn_none_r1 (int *p, int n)
+{
+  return p[1];      // { dg-warning "'p\\\[1]' is used uninitialized" }
+}
+
+A (write_only, 1, 2)
+int warn_none_ri (int *p, int n, int i)
+{
+  return p[i];      // { dg-warning " is used uninitialized" }
+}
diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
index 5d7bc800419..62cd680b1db 100644
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -283,6 +283,64 @@ builtin_call_nomodifying_p (gimple *stmt)
   return true;
 }
 
+/* If ARG is a FNDECL parameter declared with attribute access none or
+   write_only issue a warning for its read access via PTR.  */
+
+static void
+maybe_warn_read_write_only (tree fndecl, gimple *stmt, tree arg, tree ptr)
+{
+  if (!fndecl)
+    return;
+
+  if (get_no_uninit_warning (arg))
+    return;
+
+  tree fntype = TREE_TYPE (fndecl);
+  if (!fntype)
+    return;
+
+  /* Initialize a map of attribute access specifications for arguments
+     to the function function call.  */
+  rdwr_map rdwr_idx;
+  init_attr_rdwr_indices (&rdwr_idx, TYPE_ATTRIBUTES (fntype));
+
+  unsigned argno = 0;
+  tree parms = DECL_ARGUMENTS (fndecl);
+  for (tree parm = parms; parm; parm = TREE_CHAIN (parm), ++argno)
+    {
+      if (parm != arg)
+	continue;
+
+      const attr_access* access = rdwr_idx.get (argno);
+      if (!access)
+	break;
+
+      if (access->mode != access_none
+	  && access->mode != access_write_only)
+	continue;
+
+      location_t stmtloc
+	= linemap_resolve_location (line_table, gimple_location (stmt),
+				    LRK_SPELLING_LOCATION, NULL);
+
+      if (!warning_at (stmtloc, OPT_Wuninitialized,
+		       "%qE is used uninitialized", ptr))
+	break;
+
+      suppress_warning (arg, OPT_Wuninitialized);
+
+      const char* const access_str =
+	TREE_STRING_POINTER (access->to_external_string ());
+
+      location_t parmloc = DECL_SOURCE_LOCATION (parm);
+      inform (parmloc, "accessing argument %u of a function declared with "
+	      "attribute %<%s%>",
+	      argno + 1, access_str);
+
+      break;
+    }
+}
+
 /* Callback for walk_aliased_vdefs.  */
 
 static bool
@@ -350,7 +408,9 @@ struct wlimits
 };
 
 /* Determine if REF references an uninitialized operand and diagnose
-   it if so.  */
+   it if so.  STMS is the referencing statement.  LHS is the result
+   of the access and may be null.  RHS is the variable referenced by
+   the access; it may not be null.  */
 
 static tree
 maybe_warn_operand (ao_ref &ref, gimple *stmt, tree lhs, tree rhs,
@@ -497,14 +557,25 @@ maybe_warn_operand (ao_ref &ref, gimple *stmt, tree lhs, tree rhs,
   /* Do not warn if it can be initialized outside this function.
      If we did not reach function entry then we found killing
      clobbers on all paths to entry.  */
-  if (!found_alloc
-      && fentry_reached
-      /* ???  We'd like to use ref_may_alias_global_p but that
-	 excludes global readonly memory and thus we get bogus
-	 warnings from p = cond ? "a" : "b" for example.  */
-      && (!VAR_P (base)
-	  || is_global_var (base)))
-    return NULL_TREE;
+  if (!found_alloc && fentry_reached)
+    {
+      if (TREE_CODE (base) == SSA_NAME)
+	{
+	  tree var = SSA_NAME_VAR (base);
+	  if (var && TREE_CODE (var) == PARM_DECL)
+	    {
+	      maybe_warn_read_write_only (cfun->decl, stmt, var, rhs);
+	      return NULL_TREE;
+	    }
+	}
+
+      if (!VAR_P (base)
+	  || is_global_var (base))
+	/* ???  We'd like to use ref_may_alias_global_p but that
+	   excludes global readonly memory and thus we get bogus
+	   warnings from p = cond ? "a" : "b" for example.  */
+	return NULL_TREE;
+    }
 
   /* Strip the address-of expression from arrays passed to functions. */
   if (TREE_CODE (rhs) == ADDR_EXPR)

Re: Warn for reads from write-only arguments [PR101734]

[Jeff Law]

On 8/2/2021 3:44 PM, Martin Sebor via Gcc-patches wrote:
> The write_only mode to attribute access specifies that the pointer
> applies to is used to write to the referenced object but not read
> from it.
>
> A function that uses the pointer to read the referenced object might 
> rely on the contents of uninitialized memory and so such attempts
> should be diagnosed.  The attached enhancement makes that happen.
> It was tested on x86_64-linux and by building Glibc where it found
> an inappropriate use of the attribute on a function documented to
> read from the argument as an extension [BZ 28170].
>
> I plan to implement a similar warning for writes to read-only objects
> (either with attribute read_only, or those declared const, or const
> restrict) pointers in a followup (as part of my solution for PR 90404).
>
> Martin
>
> BZ 28170: https://sourceware.org/bugzilla/show_bug.cgi?id=28170
>
> gcc-101374.diff
>
> Warn for reads from write-only arguments [PR101734].
>
> Resolves:
> PR middle-end/101734 - missing warning reading from a write-only object
>
> gcc/ChangeLog:
>
> 	PR middle-end/101734
> 	* tree-ssa-uninit.c (maybe_warn_read_write_only): New function.
> 	(maybe_warn_operand): Call it.
>
> gcc/testsuite/ChangeLog:
>
> 	PR middle-end/101734
> 	* gcc.dg/uninit-42.c: New test.
OK
jeff