[Bug c/59219] New: ____builtin___memcpy_chk and -fno-builtin-memcpy

[Bug c/59219] New: ____builtin___memcpy_chk and -fno-builtin-memcpy

[msebor at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59219

            Bug ID: 59219
           Summary: ____builtin___memcpy_chk and -fno-builtin-memcpy
           Product: gcc
           Version: 4.8.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: msebor at gmail dot com

The __builtin___xxx_chk intrinsics are useful in detecting buffer overflows via
the Object Size Checking feature. But in a freestanding/embeeded environment
with its own implementation of function xxx (such as memcpy), the
__builtin___xxx_chk intrinsics cannot be used even with the -ffreestanding or
-fno-builtin option because they result in the inline expansion of the related
xxx function irrespective of the option (see the test program below). To get
the benefit of Object Size Checking in these environments, it's necessary to
hand-code __builtin___xxx_chk instead. It would simplify the adoption of Object
Size Checking in these environments if instead of expanding xxx inline when
-fno-builtin is specified, GCC emitted a call to xxx. (As a side note, this
happens to be the behavior of the Intel compiler.)

$ cat v.c && gcc -O2 -c -fno-builtin -std=c11 v.c && objdump -d v.o | sed -n
"/<foo>:/,/^$/p"
typedef __typeof__ (sizeof 0) size_t;

extern inline __attribute__ ((always_inline, artificial)) void*
memcpy (void* restrict d, const void* restrict s, size_t n) {
    return __builtin___memcpy_chk (d, s, n, __builtin_object_size (d, 1));
}

char b [4];

void foo (const void *p) {
    memcpy (b, p, sizeof b);
}
0000000000000010 <foo>:
  10:    8b 07                    mov    (%rdi),%eax
  12:    89 05 00 00 00 00        mov    %eax,0(%rip)        # 18 <foo+0x8>
  18:    c3                       retq

[Bug c/59219] ____builtin___memcpy_chk and -fno-builtin-memcpy

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59219

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2013-11-21
     Ever confirmed|0                           |1

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Please specify more thoroughly what you are asking for.  Are you complaining
that eventually calls to __memcpy_chk are emitted?  Or are you complaining
that we still expand memcpy inline even though you specified -fno-builtin?
(but you used __builtin__memcpy_chk which retains its 'memcpy' specification
because of the __builtin prefix)

[Bug c/59219] ____builtin___memcpy_chk and -fno-builtin-memcpy

[msebor at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59219

--- Comment #2 from Martin Sebor <msebor at gmail dot com> ---
I'm suggesting that when -fno-builtin is used, __builtin___memcpy_chk (and
other __bultin_xxx_chk) invocations that are determined not to exceed the size
of the buffer boundaries expand to a call to memcpy instead of being expanded
inline. Invocations that aren't guaranteed to be safe will continue to expand
to calls to __memcpy_chk as they do now (i.e., I'm not suggesting a change
there because freestanding implementations can and provide their own
implementation of __memcpy_chk).

I understand the convention of expanding __builtin_xxx invocations inline
regardless of -fno-builtin but this convention makes the __builtin_xxx_chk
intrinsics unusable in freestanding environments where the xxx functions have
different semantics than those required of hosted implementations (and those
assumed by GCC).

A simple example where the inline expansion of __builtin___memcpy_chk is
undesirable is a freestanding environment with a null-safe memcpy. This example
can be dealt with by modifying the extern inline definiton of memcpy to avoid
invoking __builtin___memcpy_chk when either of the pointers is null, so that
can be easily solved with no compiler change.

An example that can't be as easily solved without the proposed change is one
involving __builtin___sprintf_chk where the freestanding implementation of
sprintf behaves differently than the standard specifies for hosted
implementations (and GCC's inline expansion assumes). For instance, the
implementation could treat the $ character as the beginning of a formatting
directive.

[Bug c/59219] ____builtin___memcpy_chk and -fno-builtin-memcpy

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59219

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
But if you are using __builtin__xxx_chk you are using a builtin and thus
are required to follow the builtins semantic.  You can use __memcpy_chk
instead (but you won't get the optimization to memcpy for unknown
object sizes then).

[Bug c/59219] ____builtin___memcpy_chk and -fno-builtin-memcpy

[msebor at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59219

--- Comment #4 from Martin Sebor <msebor at gmail dot com> ---
I understand. The current semantics of __builtin__xxx_chk are to:

a) check the constraints of the xxx function at compile time, and
b) diagnose constraint violations detected in (a) and call __xxx_chk, or
c) expand xxx inline if constraints are satisfied

I'm suggesting that it would be useful to change the semantics in (c):

c) if constraints are satisfied, then if -fbuiltin is used, expand xxx inline,
or
d) if -fno-builtin is used, call xxx

I.e., reduce the effects of the __builtin__xxx_chk intrinsics to just checking
the constraints, and (when -fno-builtin is used) make it possible to customize
the implementation within those constraints.

This would let freestanding implementations use the __builtin__xxx_chk
intrinsics and also provide their own semantics for the xxx functions within
the constraints specified by the language.

(PS I belatedly realized that my mention of a freestanding sprintf using '$' to
introduce a freestanding format directive didn't make sense as it would violate
the function's constraint. Please diregard that part.)

[Bug c/59219] ____builtin___memcpy_chk and -fno-builtin-memcpy

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59219

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Martin Sebor from comment #4)
> I understand. The current semantics of __builtin__xxx_chk are to:
> 
> a) check the constraints of the xxx function at compile time, and
> b) diagnose constraint violations detected in (a) and call __xxx_chk, or
> c) expand xxx inline if constraints are satisfied

c) isn't correct - the rule is that __xxx_chk is either expanded to
a call to __builtin_xxx or to a call to __xxx_chk.

__builtin_xxx is then either expanded inline or as a call to xxx
dependent on cost and flags.

> I'm suggesting that it would be useful to change the semantics in (c):
> 
> c) if constraints are satisfied, then if -fbuiltin is used, expand xxx
> inline, or
> d) if -fno-builtin is used, call xxx

I think you are asking that __builtin_xxx_chk should expand to
__xxx_chk or xxx (instead of __builtin_xxx).

> I.e., reduce the effects of the __builtin__xxx_chk intrinsics to just
> checking the constraints, and (when -fno-builtin is used) make it possible
> to customize the implementation within those constraints.
> 
> This would let freestanding implementations use the __builtin__xxx_chk
> intrinsics and also provide their own semantics for the xxx functions within
> the constraints specified by the language.

A target independent flag that controls inline expansion of xxx would be
more useful I think.  You can use -mstringop-strategy=libcall on x86.

> (PS I belatedly realized that my mention of a freestanding sprintf using '$'
> to introduce a freestanding format directive didn't make sense as it would
> violate the function's constraint. Please diregard that part.)