Re: [PATCH] Add security_sensitive attribute to clean function stack and regs.

[Bernd Schmidt <bschmidt at redhat dot com>]

On 03/22/2016 03:15 PM, Marcos Díaz wrote:
>     the attached patch adds a new attribute 'security_sensitive' for functions.
> The idea was discussed in PR middle-end/69976.

The first question would be: I see several submissions from folks 
@tallertechnologies.com. Do you and your employer have copyright 
assignments on file with the FSF?

The patch violates gcc coding guidelines in many ways. I'll point out 
some issues, but in general you are expected to familiarize yourself 
with the requirements first. Please review the entire patch yourself for 
such issues.

> +bool ix86_sec_sensitive_attr_p(void)

Break line after return types.
>   {
> -  return TARGET_RED_ZONE && !TARGET_64BIT_MS_ABI;
> +  return TARGET_RED_ZONE && !TARGET_64BIT_MS_ABI
> +	&& !ix86_sec_sensitive_attr_p();

Wrap multi-line expressions in parentheses to get correct formatting 
from the editor.

> +  return (regno == BX_REG)
> +	 || (regno == BP_REG)
> +	 || (regno == SP_REG)
> +	 || (regno == R12_REG)
> +	 || (regno == R13_REG)
> +	 || (regno == R14_REG)
> +	 || (regno == R15_REG);

Same here but also remove unnecessary parentheses (in many places in 
this patch).

> +}
> +
> +/* Returns true iff regno is an integer register.*/
> +static inline bool is_integer_reg(unsigned int regno)

Space before open parens.

> +      rtx ret = ix86_function_value
> +		(TREE_TYPE (DECL_RESULT (cfun->decl)), cfun->decl, true);

Probably better to split this line after one of the commas.

> +	  // Is parallel

Use C style comments consistently.
> +/* Adds to str in pos position the neame of the register regno.*/

Always two spaces after a period, including at the end of a comment.

In general it would be ideal if this functionality was not x86 specific. 
I could imagine that thread_prologue_and_epilogue_insns could do this in 
a machine-independent fashion.


Bernd