[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [avr-gcc-list] Potential stack corruption in naked functions at -O0

From: Georg-Johann Lay
Subject: Re: [avr-gcc-list] Potential stack corruption in naked functions at -O0
Date: Tue, 07 Aug 2012 19:41:04 +0200
User-agent: Thunderbird (Windows/20100228)

Senthil Kumar Selvaraj schrieb:

When tracking down a different (but related) bug, I noticed that,
at -O0, code to copy function parameters from registers to the stack is generated at the start of the function. This is done even for naked functions (__attribute__((naked))), which I guess is wrong, as
those functions don't have a prologue generated to setup the stack frame.

The you request to skip prologue/epilogue generation, the compiler will
skip them, of course.  The code generated for a naked function will be
the same as for a non-naked function except for pro- and epilogue
generation, e.g. for tail calls, and TARGET_CANNOT_MODIFY_JUMPS_P.

Note the stores to Y+2 and Y+1 in the example below.

[scratch]$ cat test.c
void __attribute__((naked)) func(int x)
    __asm volatile ("ret");
[scratch]$ avr-gcc -O0 -S test.c
[scratch]$ cat test.s
        .file   "test.c"
__SREG__ = 0x3f
__SP_H__ = 0x3e
__SP_L__ = 0x3d
__CCP__ = 0x34
__tmp_reg__ = 0
__zero_reg__ = 1
        .global __do_copy_data
        .global __do_clear_bss
.global func
        .type   func, @function
/* prologue: naked */
/* frame size = 2 */
/* stack size = 0 */
.L__stack_usage = 0
        std Y+2,r25
        std Y+1,r24
/* #APP */
 ;  3 "test.c" 1
 ;  0 "" 2
/* epilogue start */
/* #NOAPP */
        .size   func, .-func

It's not save to use naked with -O0.  naked is a "you must know what you
are doing" feature.

Implementing TARGET_ALLOCATE_STACK_SLOTS_FOR_ARGS will improve the
situation, but as soon as you add some more instructions to the function
the frame will be used again.  That's what you request per -O0.

A save use of naked is for functions that only contain inline assembler
with no arguments; for more complex code there is no guarantee that no
frame is needed/used.

If you actually need that feature, you might are better of with
attribute constructor to call a function before main.

And I am wondering why a naked function gets a parameter.
naked is typically used for code in .initN/.finiN or short ISR
completely in inline assembler.

I looked at what other targets have done and created a patch (pasted below). 
I file a bug first?

A bug report is nice so that wjen other users hit the problem, they can
be pointed to the PR and read more about the issue and see if and for
what version the problem is fixed.  And a source comment can refer to

diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c
index e0d2e82..418e4d5 100644
--- a/gcc/config/avr/avr.c
+++ b/gcc/config/avr/avr.c
@@ -2489,6 +2489,13 @@ avr_function_arg_advance (cumulative_args_t cum_v, enum 
machine_mode mode,
+static bool
+  return !cfun->machine->is_naked;

Shouldn't there be a diagnose for the case when a naked touches a

 /* Decide whether we can make a sibling call to a function.  DECL is the
    declaration of the function being targeted by the call and EXP is the
@@ -10778,6 +10785,8 @@ avr_fold_builtin (tree fndecl, int n_args 
 #define TARGET_FUNCTION_ARG avr_function_arg
 #define TARGET_FUNCTION_ARG_ADVANCE avr_function_arg_advance
+#define TARGET_ALLOCATE_STACK_SLOTS_FOR_ARGS avr_allocate_stack_slots_for_args
 #define TARGET_SET_CURRENT_FUNCTION avr_set_current_function
diff --git a/gcc/testsuite/gcc.target/avr/naked-nostackpush.c 
new file mode 100644
index 0000000..c9c8865
--- /dev/null
+++ b/gcc/testsuite/gcc.target/avr/naked-nostackpush.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O0 -g3" } */
+/* This testcase verifies that compilation with O0
+   for naked functions does not generate code for
+   copying arguments into the stack at the beginning
+   of the function.
+void __attribute__((naked)) func(int code) +{
+    __asm volatile ("ret");
+/* { dg-final { scan-assembler-not "\tstd" } } */

std appears a bit restrictive. You could add -dP and scan against
"(set (mem" for example or "frame size = [1-9]".

Why does the test case need -g3?

If there is a PR, the testcase could have the PR number in the
file name, and the first line of the test refers to the PR.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]