Re: [PATCH] Adjust 'malloc' attribute documentation to match implementation
On Tuesday 21 February 2012 10:19:15 Richard Guenther wrote:
On Mon, Feb 20, 2012 at 8:55 PM, Tijl Coosemans t...@coosemans.org wrote:
On Monday 9 January 2012 10:05:08 Richard Guenther wrote:
Since GCC 4.4 applying the malloc attribute to realloc-like
functions does not work under the documented constraints because
the contents of the memory pointed to are not properly transfered
from the realloc argument (or treated as pointing to anything,
like 4.3 behaved).
The following adjusts documentation to reflect implementation
reality (we do have an implementation detail that treats the
memory blob returned for non-builtins as pointing to any global
variable, but that is neither documented nor do I plan to do
so - I presume it is to allow allocation + initialization
routines to be marked with malloc, but even that area looks
susceptible to misinterpretation to me).
Any comments?
The new text says the memory must be undefined, but gives calloc as an
example for which the memory is defined to be zero. Also, GCC has
built-ins for strdup and strndup with the malloc attribute and GLIBC
further adds it to wcsdup (wchar_t version of strdup) and tempnam. In
all of these cases the memory is defined.
Isn't the reason the attribute doesn't apply to realloc simply because
the returned pointer may alias the one given as argument, rather than
having defined memory content?
The question is really what the alias-analysis code can derive from a
function that is declared with the malloc attribute. The most useful
property for alias analysis would be that te non-aliasing holds
transitively, thus reading (with any level of indirection) from the returned
pointer does not produce memory that is aliased by any other pointer.
That's what happens for 'malloc' (also for 'calloc' - you can't do any
further indirections through the NULL pointers the memory holds). It
does not happen for realloc. Currently the alias-analysis code does
assume exactly this properly (only very slightly weakened, possibly
because we broke some code I guess).
Internally, all builtins with interesting allocation properties are handled
explicitely, so we probably should not rely on the malloc attribute present
on those (and maybe simply drop it there).
The question is really what is useful for users, and what's the most natural
behavior? For example
int **my_initialized_malloc (int *p)
{
int **q = malloc (sizeof (int *));
*q = p;
return q;
}
would not qualify for the 'malloc' attribute (but we've taken measures to not
miscompile this kind of code, it seems to be a very common misconception
to place annotate these with 'malloc').
I'm not sure how to exactly constrain the documentation for 'malloc' better.
Maybe
The @code{malloc} attribute is used to tell the compiler that a function
may be treated as if any non-@code{NULL} pointer it returns cannot
alias any other pointer valid when the function returns and that the memory
does not contain any pointer value.
? Because that is what is relevant. That you can in no way extract
a pointer value from the memory pointed to by the return value. Because
alias analysis will assume any such extracted pointer value points
nowhere (so, extracting a NULL pointer is ok).
The reasoning why the string functions have the malloc attribute was
probably that strings do not contain pointer values. Of course they
can, you can store a character encoding of a pointer, copy the
string and decode it from the copy again. We'd miscompile then
int i = 1;
int *p = i;
char ptr[16];
... inline encode p into ptr ...
char *x = strdup (ptr);
int *q = ... inline decode x to q
*q = 2;
return i;
to return 1 because we do not see that q may point to i. Of course
we properly handle the transfer of pointers for str[n]dup, so the
'malloc' attribute on it is a lie...
Thanks, that was very informative.
Is it correct to say that the attribute applies to deep copies, but not to
shallow ones?
How about the following text:
@item malloc
@cindex @code{malloc} attribute
The @code{malloc} attribute is used to tell the compiler that a pointer
returned by a function is either @code{NULL} or points to a newly
allocated object and that any pointer within that object is either
uninitialised, @code{NULL} or pointing to a newly allocated object for
which the same conditions hold recursively. The compiler assumes that
existing variables and memory cannot be accessed through the returned
pointer which will often improve optimization.
Standard functions with this property include @code{malloc} and
@code{calloc}. @code{realloc}-like functions do not have this
property as the returned pointer may alias the one given as argument
or the memory pointed to may contain initialised pointers.
@code{strdup}-like functions have this property as long as the string
does not encode a memory address. More generally the attribute applies
to deep memory copies
Re: Add -lssp_nonshared to LINK_SSP_SPEC
On Tuesday 07 February 2012 09:54:43 Jakub Jelinek wrote: On Mon, Jan 23, 2012 at 12:03:27PM +0100, Richard Guenther wrote: On Mon, Jan 23, 2012 at 12:23 AM, Gerald Pfeifer ger...@pfeifer.com wrote: On Sat, 21 Jan 2012, Tijl Coosemans wrote: I've been using this patch now. It's similar to the above url, but conditional on TARGET_LIBC_PROVIDES_SSP to support older FreeBSD versions. Gerald volunteered to commit. Gerald, just trunk for now or older branches too? If Richi agries, I'd apply this on trunk and the GCC 4.6 branch, since that is the stable release our users employ. Sure, go ahead. Richard. Gerald PS: We also need to update the copyright date at the top, and I'll take care of that when committing. 2011-01-20 Tijl Coosemans t...@coosemans.org * gcc/config/freebsd-spec.h [TARGET_LIBC_PROVIDES_SSP] (LINK_SSP_SPEC): Define. This change unfortunately broke all non-freebsd powerpc* targets. For some weird reason freebsd-spec.h is included for all powerpc* targets (to support -mcall-freebsd), which means that e.g. on powerpc64-linux the above results in -fstack-protector being broken. So, if this mess is really needed (does anybody actually use -mcall-freebsd on non-freebsd targets?), IMHO freebsd-spec.h must avoid defining non-FBSD_ prefixed macros, as the following (completely untested) patch. Don't have access to FreeBSD to test it there though, can test on powerpc64-linux. Everything still works on FreeBSD. --- gcc/config/freebsd.h.jj 2010-11-26 18:39:09.0 +0100 +++ gcc/config/freebsd.h 2012-02-07 09:48:50.872294367 +0100 @@ -45,6 +45,21 @@ along with GCC; see the file COPYING3. #undef LIB_SPEC #define LIB_SPEC FBSD_LIB_SPEC +#ifdef FBSD_LINK_EH_SPEC +#undef LINK_EH_SPEC +#define LINK_EH_SPEC FBSD_LINK_EH_SPEC +#endif + +#ifdef FBSD_LINK_SSP_SPEC +#undef LINK_SSP_SPEC +#define LINK_SSP_SPEC FBSD_LINK_SSP_SPEC +#endif + FYI, there are extra spaces on this line.
Re: Add -lssp_nonshared to LINK_SSP_SPEC
On Tuesday 07 February 2012 12:53:24 Jakub Jelinek wrote: On Tue, Feb 07, 2012 at 12:17:59PM +0100, Tijl Coosemans wrote: Everything still works on FreeBSD. After discussion about this on IRC Richard expressed his preference for the following variant instead: Works too.
Re: Add -lssp_nonshared to LINK_SSP_SPEC
On Wednesday 11 January 2012 10:06:42 Richard Guenther wrote:
On Tue, Jan 10, 2012 at 8:50 PM, Tijl Coosemans t...@freebsd.org wrote:
On Tuesday 10 January 2012 15:40:15 Richard Guenther wrote:
On Tue, Jan 10, 2012 at 3:38 PM, Richard Guenther
richard.guent...@gmail.com wrote:
On Tue, Jan 10, 2012 at 3:14 PM, Tijl Coosemans t...@coosemans.org wrote:
On targets where libc implements stack protector functions (GNU libc,
FreeBSD libc), and where gcc (as an optimisation) generates calls to
a locally defined __stack_chk_fail_local instead of directly calling
the global function __stack_chk_fail (e.g. -fpic code on i386), one
must explicitly specify -lssp_nonshared or -lc -lc_nonshared on the
command line to statically link in __stack_chk_fail_local.
It would be more convenient if the compiler kept the details of this
target specific optimisation hidden by passing -lssp_nonshared to the
linker internally.
Here's a simple test case that shows the problem on i386-freebsd, but
works just fine on e.g. x86_64 targets:
% cat test.c
int
main( void ) {
return( 0 );
}
% gcc46 -o test test.c -fstack-protector-all -fPIE
/var/tmp//ccjYQxKu.o: In function `main':
test.c:(.text+0x37): undefined reference to `__stack_chk_fail_local'
/usr/local/bin/ld: test: hidden symbol `__stack_chk_fail_local' isn't
defined
/usr/local/bin/ld: final link failed: Bad value
collect2: ld returned 1 exit status
I don't have commit access, so please commit when approved.
Works fine for me on i?86-linux without -lssp_nonshared (which I do not
have, so linking would fail).
So my patch would actually break Linux?
Yes.
Probably because libc.so is a linker script:
/* GNU ld script
Use the shared library, but some functions are only in
the static library, so try that secondarily. */
OUTPUT_FORMAT(elf64-x86-64)
GROUP ( /lib64/libc.so.6 /usr/lib64/libc_nonshared.a AS_NEEDED (
/lib64/ld-linux-x86-64.so.2 ) )
and /usr/lib64/libc_nonshared.a provides the symbol. Why not fix it that
way on *BSD?
I'll discuss it with some FreeBSD developers. Currently FreeBSD doesn't
use linker scripts anywhere. Would a FreeBSD specific version of the
patch be acceptable? For instance, the version of GCC shipped with
FreeBSD has been patched like this:
http://svnweb.freebsd.org/base/head/contrib/gcc/config/freebsd-spec.h?r1=195697r2=195696
That looks better to me.
I've been using this patch now. It's similar to the above url, but
conditional on TARGET_LIBC_PROVIDES_SSP to support older FreeBSD
versions.
Gerald volunteered to commit. Gerald, just trunk for now or older
branches too?
2011-01-20 Tijl Coosemans t...@coosemans.org
* gcc/config/freebsd-spec.h [TARGET_LIBC_PROVIDES_SSP] (LINK_SSP_SPEC):
Define.
--- gcc/config/freebsd-spec.h
+++ gcc/config/freebsd-spec.h
@@ -138,6 +138,10 @@ is built with the --enable-threads confi
#define LINK_EH_SPEC %{!static:--eh-frame-hdr}
#endif
+#ifdef TARGET_LIBC_PROVIDES_SSP
+#define LINK_SSP_SPEC
%{fstack-protector|fstack-protector-all:-lssp_nonshared}
+#endif
+
/* Use --as-needed -lgcc_s for eh support. */
#ifdef HAVE_LD_AS_NEEDED
#define USE_LD_AS_NEEDED 1
Fix checks for !TARGET_MACHO
TARGET_MACHO is always defined (either 0 or 1) so using #ifndef to test for !TARGET_MACHO is incorrect. Introduced here: http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00623.html The i386 case has been tested on i386-freebsd. The compiler now emits calls to __stack_chk_fail_local again when using -fstack-protector -fpic. The rs6000 cases are untested. I don't have commit access, so please commit when approved. 2011-01-10 Tijl Coosemans t...@coosemans.org * config/i386/i386.c: Fix checks for !TARGET_MACHO. * config/rs6000/rs6000.c: Likewise. --- gcc/config/i386/i386.c +++ gcc/config/i386/i386.c @@ -38624,7 +38624,7 @@ ix86_autovectorize_vector_sizes (void) #undef TARGET_MANGLE_TYPE #define TARGET_MANGLE_TYPE ix86_mangle_type -#ifndef TARGET_MACHO +#if !TARGET_MACHO #undef TARGET_STACK_PROTECT_FAIL #define TARGET_STACK_PROTECT_FAIL ix86_stack_protect_fail #endif --- gcc/config/rs6000/rs6000.c +++ gcc/config/rs6000/rs6000.c @@ -939,7 +939,7 @@ static bool legitimate_lo_sum_address_p static struct machine_function * rs6000_init_machine_status (void); static bool rs6000_assemble_integer (rtx, unsigned int, int); static bool no_global_regs_above (int, bool); -#if defined (HAVE_GAS_HIDDEN) !defined (TARGET_MACHO) +#if defined (HAVE_GAS_HIDDEN) !TARGET_MACHO static void rs6000_assemble_visibility (tree, int); #endif static int rs6000_ra_ever_killed (void); @@ -1387,7 +1387,7 @@ static const struct attribute_spec rs600 #undef TARGET_ASM_INTEGER #define TARGET_ASM_INTEGER rs6000_assemble_integer -#if defined (HAVE_GAS_HIDDEN) !defined (TARGET_MACHO) +#if defined (HAVE_GAS_HIDDEN) !TARGET_MACHO #undef TARGET_ASM_ASSEMBLE_VISIBILITY #define TARGET_ASM_ASSEMBLE_VISIBILITY rs6000_assemble_visibility #endif @@ -1576,7 +1576,7 @@ static const struct attribute_spec rs600 #define TARGET_VECTORIZE_BUILTIN_VECTORIZED_FUNCTION \ rs6000_builtin_vectorized_function -#ifndef TARGET_MACHO +#if !TARGET_MACHO #undef TARGET_STACK_PROTECT_FAIL #define TARGET_STACK_PROTECT_FAIL rs6000_stack_protect_fail #endif @@ -15515,7 +15515,7 @@ rs6000_assemble_integer (rtx x, unsigned return default_assemble_integer (x, size, aligned_p); } -#if defined (HAVE_GAS_HIDDEN) !defined (TARGET_MACHO) +#if defined (HAVE_GAS_HIDDEN) !TARGET_MACHO /* Emit an assembler directive to set symbol visibility for DECL to VISIBILITY_TYPE. */
Add -lssp_nonshared to LINK_SSP_SPEC
On targets where libc implements stack protector functions (GNU libc,
FreeBSD libc), and where gcc (as an optimisation) generates calls to
a locally defined __stack_chk_fail_local instead of directly calling
the global function __stack_chk_fail (e.g. -fpic code on i386), one
must explicitly specify -lssp_nonshared or -lc -lc_nonshared on the
command line to statically link in __stack_chk_fail_local.
It would be more convenient if the compiler kept the details of this
target specific optimisation hidden by passing -lssp_nonshared to the
linker internally.
Here's a simple test case that shows the problem on i386-freebsd, but
works just fine on e.g. x86_64 targets:
% cat test.c
int
main( void ) {
return( 0 );
}
% gcc46 -o test test.c -fstack-protector-all -fPIE
/var/tmp//ccjYQxKu.o: In function `main':
test.c:(.text+0x37): undefined reference to `__stack_chk_fail_local'
/usr/local/bin/ld: test: hidden symbol `__stack_chk_fail_local' isn't defined
/usr/local/bin/ld: final link failed: Bad value
collect2: ld returned 1 exit status
I don't have commit access, so please commit when approved.
2011-01-10 Tijl Coosemans t...@coosemans.org
* gcc.c [TARGET_LIBC_PROVIDES_SSP] (LINK_SSP_SPEC): Add -lssp_nonshared.
--- gcc/gcc.c.orig
+++ gcc/gcc.c
@@ -602,7 +602,7 @@ proper position among the other output f
#ifndef LINK_SSP_SPEC
#ifdef TARGET_LIBC_PROVIDES_SSP
-#define LINK_SSP_SPEC %{fstack-protector:}
+#define LINK_SSP_SPEC
%{fstack-protector|fstack-protector-all:-lssp_nonshared}
#else
#define LINK_SSP_SPEC %{fstack-protector|fstack-protector-all:-lssp_nonshared
-lssp}
#endif
Re: Add -lssp_nonshared to LINK_SSP_SPEC
On Tuesday 10 January 2012 15:40:15 Richard Guenther wrote:
On Tue, Jan 10, 2012 at 3:38 PM, Richard Guenther
richard.guent...@gmail.com wrote:
On Tue, Jan 10, 2012 at 3:14 PM, Tijl Coosemans t...@coosemans.org wrote:
On targets where libc implements stack protector functions (GNU libc,
FreeBSD libc), and where gcc (as an optimisation) generates calls to
a locally defined __stack_chk_fail_local instead of directly calling
the global function __stack_chk_fail (e.g. -fpic code on i386), one
must explicitly specify -lssp_nonshared or -lc -lc_nonshared on the
command line to statically link in __stack_chk_fail_local.
It would be more convenient if the compiler kept the details of this
target specific optimisation hidden by passing -lssp_nonshared to the
linker internally.
Here's a simple test case that shows the problem on i386-freebsd, but
works just fine on e.g. x86_64 targets:
% cat test.c
int
main( void ) {
return( 0 );
}
% gcc46 -o test test.c -fstack-protector-all -fPIE
/var/tmp//ccjYQxKu.o: In function `main':
test.c:(.text+0x37): undefined reference to `__stack_chk_fail_local'
/usr/local/bin/ld: test: hidden symbol `__stack_chk_fail_local' isn't
defined
/usr/local/bin/ld: final link failed: Bad value
collect2: ld returned 1 exit status
I don't have commit access, so please commit when approved.
Works fine for me on i?86-linux without -lssp_nonshared (which I do not
have, so linking would fail).
So my patch would actually break Linux?
Probably because libc.so is a linker script:
/* GNU ld script
Use the shared library, but some functions are only in
the static library, so try that secondarily. */
OUTPUT_FORMAT(elf64-x86-64)
GROUP ( /lib64/libc.so.6 /usr/lib64/libc_nonshared.a AS_NEEDED (
/lib64/ld-linux-x86-64.so.2 ) )
and /usr/lib64/libc_nonshared.a provides the symbol. Why not fix it that
way on *BSD?
I'll discuss it with some FreeBSD developers. Currently FreeBSD doesn't
use linker scripts anywhere. Would a FreeBSD specific version of the
patch be acceptable? For instance, the version of GCC shipped with
FreeBSD has been patched like this:
http://svnweb.freebsd.org/base/head/contrib/gcc/config/freebsd-spec.h?r1=195697r2=195696
2011-01-10 Tijl Coosemans t...@coosemans.org
* gcc.c [TARGET_LIBC_PROVIDES_SSP] (LINK_SSP_SPEC): Add
-lssp_nonshared.
--- gcc/gcc.c.orig
+++ gcc/gcc.c
@@ -602,7 +602,7 @@ proper position among the other output f
#ifndef LINK_SSP_SPEC
#ifdef TARGET_LIBC_PROVIDES_SSP
-#define LINK_SSP_SPEC %{fstack-protector:}
+#define LINK_SSP_SPEC
%{fstack-protector|fstack-protector-all:-lssp_nonshared}
#else
#define LINK_SSP_SPEC
%{fstack-protector|fstack-protector-all:-lssp_nonshared -lssp}
#endif
-fstack-protector, __stack_chk_fail_local and TARGET_LIBC_PROVIDES_SSP
Hi,
I ran into an issue with -fstack-protector on FreeBSD/i386. GCC
generates calls to __stack_chk_fail_local that the linker complains are
undefined. The following test case shows it:
% cat test.c
int
main( void ) {
return( 0 );
}
% gcc46 -o test test.c -fstack-protector-all -fPIE
/var/tmp//ccjYQxKu.o: In function `main':
test.c:(.text+0x37): undefined reference to `__stack_chk_fail_local'
/usr/local/bin/ld: test: hidden symbol `__stack_chk_fail_local' isn't defined
/usr/local/bin/ld: final link failed: Bad value
collect2: ld returned 1 exit status
I managed to fix the problem with the following one line change:
--- gcc/gcc.c.orig
+++ gcc/gcc.c
@@ -601,7 +601,7 @@
#ifndef LINK_SSP_SPEC
#ifdef TARGET_LIBC_PROVIDES_SSP
-#define LINK_SSP_SPEC %{fstack-protector:}
+#define LINK_SSP_SPEC
%{fstack-protector|fstack-protector-all:-lssp_nonshared}
#else
#define LINK_SSP_SPEC %{fstack-protector|fstack-protector-all:-lssp_nonshared
-lssp}
#endif
The configure script detects __stack_chk_fail in FreeBSD libc and
defines TARGET_LIBC_PROVIDES_SSP, but __stack_chk_fail_local should be
statically linked in so (a dynamic) libc should not provide it.
My question is now whether the problem is with FreeBSD's SSP
implementation (where exactly does GCC expect __stack_chk_fail_local to
be defined?) or with GCC (should GCC just always link in ssp_nonshared
as in the patch above?).
