Re: [PATCH] tree-optimization/71831 - __builtin_object_size poor results with no optimization

[Martin Sebor <msebor at gmail dot com>]

On 08/20/2016 01:02 AM, Florian Weimer wrote:
> * Martin Sebor:
>
> > As requested in the review of the following patch
> >
> >    https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01363.html
> >
> > attached is the small enhancement to compute_builtin_object_size to
> > make the function usable even without optimization without the full
> > overhead of the tree-object-size pass.
>
> Is its overhead that significant?

That's a good question.  I haven't measured it but having limited
access to object sizes beyond what my simple patch adds would make
it possible to improve the quality of other warnings as well (e.g.,
-Warray-bounds and -Wplacement-new) and improve buffer overflow
detection in libc function calls without optimization (if enabled
as suggested below).

> Does this mean that with this patch, glibc should remove its
> _FORTIFY_SOURCE warning for non-optimized builds when compiling under
> GCC >= 7?

No, but I see no reason why GCC couldn't provide the same (limited)
overflow checking for other libc built-ins that the -Wformat-length
patch adds to __builtin_sprintf et al.  The proof of concept patch
below shows that it would be nearly trivial to do (the warning
mentions memcpy and strcpy because that's what GCC has transformed
the strcpy call into by the time it's expanded but presumably that
could be fixed).

Martin

$ cat z.c && /build/gcc-71831/gcc/xgcc -B /build/gcc-71831/gcc -O0 -S 
-Wall -Wextra z.c
char a[2];

extern char* strcpy (char*, const char*);

void f (void)
{
  strcpy (a, "abc");
}
z.c: In function ‘f’:
z.c:7:3: warning: call to __builtin_memcpy writing 4 bytes into 
destination of size 2
   strcpy (a, "abc");
   ^~~~~~~~~~~~~~~~~


--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -2985,6 +2985,19 @@ expand_builtin_memcpy (tree exp, rtx target)
       tree dest = CALL_EXPR_ARG (exp, 0);
       tree src = CALL_EXPR_ARG (exp, 1);
       tree len = CALL_EXPR_ARG (exp, 2);
+
+      unsigned HOST_WIDE_INT dstsize;
+      compute_builtin_object_size (dest, 0, &dstsize);
+
+      unsigned HOST_WIDE_INT srclen
+       = tree_fits_uhwi_p (len) ? tree_to_uhwi (len) : HOST_WIDE_INT_M1U;
+
+      if (dstsize <= srclen)
+       warning_at (tree_nonartificial_location (exp),
+                   0, "%Kcall to %D writing %wu bytes into destination "
+                   "of size %wu",
+                   exp, get_callee_fndecl (exp), srclen, dstsize);
+
       return expand_builtin_memcpy_args (dest, src, len, target, exp);
     }
 }