Bernd Edlinger - [PATCH] Fix unsafe function attributes for special functions (PR 71876) (original) (raw)

This is the mail archive of the gcc-patches@gcc.gnu.orgmailing list for the GCC project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

From: Bernd Edlinger
To: "gcc-patches at gcc dot gnu dot org"
Date: Tue, 19 Jul 2016 16:20:55 +0000
Subject: [PATCH] Fix unsafe function attributes for special functions (PR 71876)
Authentication-results: sourceware.org; auth=none
Authentication-results: spf=softfail (sender IP is 10.152.2.60) smtp.mailfrom=hotmail.de; gcc.gnu.org; dkim=none (message not signed) header.d=none;gcc.gnu.org; dmarc=none action=none header.from=hotmail.de;
References: AM4PR0701MB216226C00439A01515A3AA5FE4370@AM4PR0701MB2162.eurprd07.prod.outlook.com

Hi!

As discussed at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71876, we have a very old hack in gcc, that recognizes certain functions by name, and inserts in some cases unsafe attributes, that don't work for a freestanding environment.

It is unsafe to return ECF_MAY_BE_ALLOCA, ECF_LEAF and ECF_NORETURN from special_function_p, just by the name of the function, especially for less well known functions, like "getcontext" or "savectx", which could easily used for something completely different.

Moreover, from the backend library we cannot check flag_hosted, or if the function has "C" or "C++" binding.

So all these functions would get the leaf attribute, which makes it very easy to construct some kind of wrong code examples.

This patch removes the unsafe flags, and adds them to built-in functions instead.

The patch removes also support for some completely unknown function names from the middle-end, because these functions were apparently in use at some point in time, but are certainly dead by now.

It is however not possible to remove the special handling by name altogether, because the glibc does not add the return_twice function attribute on _setjmp, __sigsetjmp and getcontext until today; a glibc BZ is filed at: https://sourceware.org/bugzilla/show_bug.cgi?id=20382

Without the return_twice attribute we would loose the -Wclobbered warning, and some targets (spark, ia64, maybe others too) would even generate wrong code.

Boot-strapped and reg-tested on x86_64-pc-linux-gnu. OK for trunk?

Thanks Bernd.

Attachment:changelog-pr71876.txt
Description: changelog-pr71876.txt

Index: gcc/builtin-attrs.def

--- gcc/builtin-attrs.def (revision 238382) +++ gcc/builtin-attrs.def (working copy) @@ -131,6 +131,8 @@ DEF_ATTR_TREE_LIST (ATTR_NORETURN_NOTHROW_LIST, AT ATTR_NULL, ATTR_NOTHROW_LIST) DEF_ATTR_TREE_LIST (ATTR_NORETURN_NOTHROW_LEAF_LIST, ATTR_NORETURN,
ATTR_NULL, ATTR_NOTHROW_LEAF_LIST) +DEF_ATTR_TREE_LIST (ATTR_RT_NOTHROW_LEAF_LIST, ATTR_RETURNS_TWICE,
+ ATTR_NULL, ATTR_NOTHROW_LEAF_LIST) DEF_ATTR_TREE_LIST (ATTR_COLD_NOTHROW_LEAF_LIST, ATTR_COLD,
ATTR_NULL, ATTR_NOTHROW_LEAF_LIST) DEF_ATTR_TREE_LIST (ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST, ATTR_COLD,
Index: gcc/builtins.def

--- gcc/builtins.def (revision 238382) +++ gcc/builtins.def (working copy) @@ -796,6 +796,7 @@ DEF_EXT_LIB_BUILTIN (BUILT_IN_FINITED32, "finit DEF_EXT_LIB_BUILTIN (BUILT_IN_FINITED64, "finited64", BT_FN_INT_DFLOAT64, ATTR_CONST_NOTHROW_LEAF_LIST) DEF_EXT_LIB_BUILTIN (BUILT_IN_FINITED128, "finited128", BT_FN_INT_DFLOAT128, ATTR_CONST_NOTHROW_LEAF_LIST) DEF_GCC_BUILTIN (BUILT_IN_FPCLASSIFY, "fpclassify", BT_FN_INT_INT_INT_INT_INT_INT_VAR, ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF) +DEF_EXT_LIB_BUILTIN (BUILT_IN_GETCONTEXT, "getcontext", BT_FN_INT_PTR, ATTR_RT_NOTHROW_LEAF_LIST) DEF_GCC_BUILTIN (BUILT_IN_ISFINITE, "isfinite", BT_FN_INT_VAR, ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF) DEF_GCC_BUILTIN (BUILT_IN_ISINF_SIGN, "isinf_sign", BT_FN_INT_VAR, ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF) DEF_C99_C90RES_BUILTIN (BUILT_IN_ISINF, "isinf", BT_FN_INT_VAR, ATTR_CONST_NOTHROW_TYPEGENERIC) @@ -837,7 +838,7 @@ DEF_LIB_BUILTIN (BUILT_IN_REALLOC, "realloc DEF_GCC_BUILTIN (BUILT_IN_RETURN, "return", BT_FN_VOID_PTR, ATTR_NORETURN_NOTHROW_LEAF_LIST) DEF_GCC_BUILTIN (BUILT_IN_RETURN_ADDRESS, "return_address", BT_FN_PTR_UINT, ATTR_LEAF_LIST) DEF_GCC_BUILTIN (BUILT_IN_SAVEREGS, "saveregs", BT_FN_PTR_VAR, ATTR_NULL) -DEF_GCC_BUILTIN (BUILT_IN_SETJMP, "setjmp", BT_FN_INT_PTR, ATTR_NOTHROW_LEAF_LIST) +DEF_GCC_BUILTIN (BUILT_IN_SETJMP, "setjmp", BT_FN_INT_PTR, ATTR_RT_NOTHROW_LEAF_LIST) DEF_EXT_LIB_BUILTIN (BUILT_IN_STRFMON, "strfmon", BT_FN_SSIZE_STRING_SIZE_CONST_STRING_VAR, ATTR_FORMAT_STRFMON_NOTHROW_3_4) DEF_LIB_BUILTIN (BUILT_IN_STRFTIME, "strftime", BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_PTR, ATTR_FORMAT_STRFTIME_NOTHROW_3_0) DEF_GCC_BUILTIN (BUILT_IN_TRAP, "trap", BT_FN_VOID, ATTR_NORETURN_NOTHROW_LEAF_LIST) @@ -849,6 +850,7 @@ DEF_GCC_BUILTIN (BUILT_IN_VA_END, "va_end", DEF_GCC_BUILTIN (BUILT_IN_VA_START, "va_start", BT_FN_VOID_VALIST_REF_VAR, ATTR_NOTHROW_LEAF_LIST) DEF_GCC_BUILTIN (BUILT_IN_VA_ARG_PACK, "va_arg_pack", BT_FN_INT, ATTR_PURE_NOTHROW_LEAF_LIST) DEF_GCC_BUILTIN (BUILT_IN_VA_ARG_PACK_LEN, "va_arg_pack_len", BT_FN_INT, ATTR_PURE_NOTHROW_LEAF_LIST) +DEF_EXT_LIB_BUILTIN (BUILT_IN_VFORK, "vfork", BT_FN_PID, ATTR_RT_NOTHROW_LEAF_LIST) DEF_EXT_LIB_BUILTIN (BUILT_IN__EXIT, "_exit", BT_FN_VOID_INT, ATTR_NORETURN_NOTHROW_LEAF_LIST) DEF_C99_BUILTIN (BUILT_IN__EXIT2, "_Exit", BT_FN_VOID_INT, ATTR_NORETURN_NOTHROW_LEAF_LIST) Index: gcc/calls.c

--- gcc/calls.c (revision 238382) +++ gcc/calls.c (working copy) @@ -474,8 +474,6 @@ emit_call_1 (rtx funexp, tree fntree ATTRIBUTE_UNU For example, if the function might return more than one time (setjmp), then set RETURNS_TWICE to a nonzero value. - Similarly set NORETURN if the function is in the longjmp family.

Set MAY_BE_ALLOCA for any memory allocation function that might allocate
space from the stack such as alloca.  */

@@ -491,7 +489,7 @@ special_function_p (const_tree fndecl, int flags) name_decl = DECL_NAME (cgraph_node::get (fndecl)->orig_decl); if (fndecl && name_decl - && IDENTIFIER_LENGTH (name_decl) <= 17 + && IDENTIFIER_LENGTH (name_decl) <= 12 /* Exclude functions not at the file scope, or not `extern', since they are not the magic functions we would otherwise think they are. @@ -506,55 +504,21 @@ special_function_p (const_tree fndecl, int flags) const char name = IDENTIFIER_POINTER (name_decl); const char tname = name; - /* We assume that alloca will always be called by name. It - makes no sense to pass it as a pointer-to-function to - anything that does not understand its behavior. */ - if (IDENTIFIER_LENGTH (name_decl) == 6 - && name[0] == 'a' - && ! strcmp (name, "alloca")) - flags |= ECF_MAY_BE_ALLOCA;

 /* Disregard prefix _, __, __x or __builtin_.  */

 /* Disregard prefix _ or __.  */
 if (name[0] == '_')

{

```
 if (name[1] == '_'
```
```
     && name[2] == 'b'
```

     && !strncmp (name + 3, "uiltin_", 7))

```
   tname += 10;
```

 else if (name[1] == '_' && name[2] == 'x')

```
   tname += 3;
```
```
 else if (name[1] == '_')
```

 if (name[1] == '_')
    tname += 2;
  else
    tname += 1;
}

```
 if (tname[0] == 's')
```
{
```
 if ((tname[1] == 'e'
```
```
      && (! strcmp (tname, "setjmp")
```

     || ! strcmp (tname, "setjmp_syscall")))

```
     || (tname[1] == 'i'
```
```
    && ! strcmp (tname, "sigsetjmp"))
```
```
     || (tname[1] == 'a'
```
```
    && ! strcmp (tname, "savectx")))
```

   flags |= ECF_RETURNS_TWICE | ECF_LEAF;

```
 if (tname[1] == 'i'
```
```
     && ! strcmp (tname, "siglongjmp"))
```
```
   flags |= ECF_NORETURN;
```
}

 else if ((tname[0] == 'q' && tname[1] == 's'

```
  && ! strcmp (tname, "qsetjmp"))
```

      || (tname[0] == 'v' && tname[1] == 'f'

```
     && ! strcmp (tname, "vfork"))
```

      || (tname[0] == 'g' && tname[1] == 'e'

```
     && !strcmp (tname, "getcontext")))
```
flags |= ECF_RETURNS_TWICE | ECF_LEAF;

 else if (tname[0] == 'l' && tname[1] == 'o'

```
      && ! strcmp (tname, "longjmp"))
```
flags |= ECF_NORETURN;

 /* ECF_RETURNS_TWICE is safe even for -ffrestanding.  */

```
 if (! strcmp (tname, "setjmp")
```
```
 || ! strcmp (tname, "sigsetjmp")
```
```
 || ! strcmp (tname, "vfork")
```
```
 || ! strcmp (tname, "getcontext"))
```
flags |= ECF_RETURNS_TWICE; } if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)

Follow-Ups:
- Re: [PATCH] Fix unsafe function attributes for special functions (PR 71876)
  * From: Jakub Jelinek
- Re: [PATCH] Fix unsafe function attributes for special functions (PR 71876)
  * From: Joseph Myers

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]