This is the mail archive of the gcc-patches@gcc.gnu.org mailing 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]

Re: Fix PR43404, PR48470, PR64744 ICE on naked functions


On 06/26/2015 12:33 AM, Alexander Basov wrote:
2015-06-25 21:47 GMT+03:00 Jeff Law <law@redhat.com>:
On 06/03/2015 02:15 PM, Alexander Basov wrote:

Hello Jeff,
please find updated patch attached

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index b190f91..c6db8a9 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1382,7 +1382,15 @@ expand_one_var (tree var, bool toplevel, bool
really_expand)
      else
        {
          if (really_expand)
-        expand_one_stack_var (origvar);
+        {
+          if (!targetm.calls.allocate_stack_slots_for_args ())
+            error ("cannot allocate stack for variable %q+D, naked
function.",
+                   var);
+
+          expand_one_stack_var (origvar);
+        }

So how do you know ORIGVAR is an argument here before issuing the
error?  ie, shouldn't you verify that the underlying object is a
PARM_DECL? If there's some way we already know we're dealing with a
PARM_DECL, then just say so.

In case of naked function stack should not be used not only for function
args, but also for any local variables.
So, i think we don't need to check if underlying object is a PARM_DECL.

Then that would indicate that we're using the wrong test
(allocate_stack_slot_for_args).  That hook is for whether or not arguments
should have stack slots allocated.  Yet you're issuing an error for more
than just PARM_DECLs.

Shouldn't you instead be checking if the current function is a naked
function or not by checking the attributes of the current function?

Jeff

What allocate_stack_slots_for_args  does, it only checks if current
function is naked or not.
May be it will be better to remove allocate_stack_slots_for_args and
replace if with explicit checking of naked attribute?
allocate_stack_slots_for_args is used for more than just naked functions. It indicates whether or not the compiler should try to keep arguments in registers or in stack slots when optimization is disabled.

The implementation of allocate_stack_slots_for_args has special constraints in a naked function (should always return false).

One could easily argue that if we have a naked function, then having the generic parts of the compiler do the right thing rather than having each port that defines allocate_stack_slot_for_args handle naked functions specially would be an interface improvement -- and I would agree. That would probably be a useful follow-up.

Anyway, I still need to look at your follow-up patch...  It's in the queue.

Jeff


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