Bug 59219 - ____builtin___memcpy_chk and -fno-builtin-memcpy
Summary: ____builtin___memcpy_chk and -fno-builtin-memcpy
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.8.2
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-20 23:18 UTC by Martin Sebor
Modified: 2013-11-25 09:29 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2013-11-21 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2013-11-20 23:18:55 UTC
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
Comment 1 Richard Biener 2013-11-21 12:10:25 UTC
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)
Comment 2 Martin Sebor 2013-11-21 16:21:01 UTC
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.
Comment 3 Richard Biener 2013-11-22 10:29:30 UTC
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).
Comment 4 Martin Sebor 2013-11-22 17:24:47 UTC
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.)
Comment 5 Richard Biener 2013-11-25 09:29:50 UTC
(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.)