Re: [WIP PATCH] add object access attributes (PR 83859)

[Richard Biener <richard.guenther@gmail.com>]

On Sun, Sep 29, 2019 at 9:52 PM Martin Sebor <msebor@gmail.com> wrote:
>
> -Wstringop-overflow detects a subset of past-the-end read and write
> accesses by built-in functions such as memcpy and strcpy.  It relies
> on the functions' effects the knowledge of which is hardwired into
> GCC.  Although it's possible for users to create wrappers for their
> own functions to detect similar problems, it's quite cumbersome and
> so only lightly used outside system libraries like Glibc.  Even Glibc
> only checks for buffer overflow and not for reading past the end.
>
> PR 83859 asks to expose the same checking that GCC does natively for
> built-in calls via a function attribute that associates a pointer
> argument with the size argument, such as:
>
>    __attribute__((buffer_size (1, 2))) void
>    f (char* dst, size_t dstsize);
>
> The attached patch is my initial stab at providing this feature by
> introducing three new attributes:
>
>    * read_only (ptr-argno, size-argno)
>    * read_only (ptr-argno, size-argno)
>    * read_write (ptr-argno, size-argno)
>
> As requested, the attributes associate a pointer parameter to
> a function with a size parameter.  In addition, they also specify
> how the function accesses the object the pointer points to: either
> it only reads from it, or it only writes to it, or it does both.
>
> Besides enabling the same buffer overflow detection as for built-in
> string functions they also let GCC issue -Wuninitialized warnings
> for uninitialized objects passed to read-only functions by reference,
> and -Wunused-but-set warnings for objects passed to write-only
> functions that are otherwise unused (PR 80806).  The -Wununitialized
> part is done. The -Wunused-but-set detection is implemented only in
> the C FE and not yet in C++.
>
> Besides the diagnostic improvements above the attributes also open
> up optimization opportunities such as DCE.  I'm still working on this
> and so it's not yet part of the initial patch.

There's the "fn spec" attribute which you can use for the optimization
part.  Note "fn spec" also likes to know whether the address of the
argument escapes and whether the argument is only dereferenced
directly or also indirectly (when passing a pointer to a struct is
transitively reachable memory through the pointer accessed or not?).

So you should at least make sure to document the full
semantics of your proposed read_only/write_only/read_write atributes.

I guess that "read_only" means that direct accesses do not write
but the attribute does not constrain indirect accesses?

Note "fn spec" doesn't offer read/write constraints when not at the
same time constraining escaping since when a pointer escapes
through a call we cannot make any optimization for a positional
read/write constraint since a function can access global memory
(and all escaped pointed-to data ends up as "global memory").
There's no way to tell (via "fn spec") that the function only accesses
memory reachable via function arguments.

So I'm not sure there's a 1:1 mapping for your desired semantics
to "fn spec" plus your desired semantics may offer no
opportunity for optimization.  Useful would be if read_only
would map to "R" and read_write and write_only would map to "W".

Richard.

> I plan to finish the patch for GCC 10 but I don't expect to have
> the time to start taking advantage of the attributes for optimization
> until GCC 11.
>
> Besides regression testing on x86_64-linux, I also tested the patch
> by compiling Binutils/GDB, Glibc, and the Linux kernel with it.  It
> found no new problems but caused a handful of -Wunused-but-set-variable
> false positives due to an outstanding bug in the C front-end introduced
> by the patch that I still need to fix.
>
> Martin