| Message ID | 201107261952.p6QJqA8s007635@greed.delorie.com |
|---|---|
| State | New |
| Headers | show |
On 07/26/2011 12:52 PM, DJ Delorie wrote: > This patch tests for at least one user-caused reason for this > assertion failing - requiring a local frame in a naked function. For > this case at least, it would be better to trigger an error than to > ICE. OK? > > static int bar; > void __attribute__((naked)) function(void) { > int foo, result; > result = subFunction(&foo, &bar); // ICE here > } > > * expr.c (expand_expr_addr_expr_1): Detect a user request for > a local frame in a naked function, and produce a suitable > error for that specific case. Err... why would naked affect anything at all this early in the compilation path? Without an answer to that, this seems like an odd place to patch. r~
Naked is related to TARGET_ALLOCATE_STACK_SLOTS_FOR_ARGS - ARM and RX set the latter based on the former, and nobody else uses that target hook. So, naked functions don't have stack slots for args. Without stack slots, args can't be assigned to memory locations - even if they're TREE_ADDRESSABLE.
On 07/28/2011 12:38 PM, DJ Delorie wrote: > Naked is related to TARGET_ALLOCATE_STACK_SLOTS_FOR_ARGS - ARM and RX > set the latter based on the former, and nobody else uses that target > hook. So, naked functions don't have stack slots for args. Without > stack slots, args can't be assigned to memory locations - even if > they're TREE_ADDRESSABLE. But your test case has a local variable, not an argument at all. So I don't immediately see the connection. r~
Fails the same way with a parameter as with a local:
int result;
void __attribute__((naked)) ISRFunction(int x) {
result = subFunction(&x);
}
but I'm certainly open to a better explanation of how a user program
can trigger an ICE that way.
Hmmm... the only use of targetm.calls.allocate_stack_slots_for_args()
is in use_register_for_decl() - which is used by expand_decl() for
automatic variables.
Maybe the hook should have been called allocate_stack_slots_for_decls() ?
On 07/28/2011 01:30 PM, DJ Delorie wrote: > Fails the same way with a parameter as with a local: > > int result; > void __attribute__((naked)) ISRFunction(int x) { > result = subFunction(&x); > } > > but I'm certainly open to a better explanation of how a user program > can trigger an ICE that way. > > Hmmm... the only use of targetm.calls.allocate_stack_slots_for_args() > is in use_register_for_decl() - which is used by expand_decl() for > automatic variables. > > Maybe the hook should have been called allocate_stack_slots_for_decls() ? Probably. Thanks for the leg-work. I'll approve the patch as-is. r~
Thanks! Committed.
> Probably. Thanks for the leg-work. I'll approve the patch as-is.
May I apply it to the 4.5 and 4.6 branches? The same patch applies
as-is to both.
Index: expr.c =================================================================== --- expr.c (revision 176766) +++ expr.c (working copy) @@ -6943,13 +6943,22 @@ expand_expr_addr_expr_1 (tree exp, rtx t modifier == EXPAND_INITIALIZER ? EXPAND_INITIALIZER : EXPAND_CONST_ADDRESS); /* If the DECL isn't in memory, then the DECL wasn't properly marked TREE_ADDRESSABLE, which will be either a front-end or a tree optimizer bug. */ - gcc_assert (MEM_P (result)); + + if (TREE_ADDRESSABLE (exp) + && ! MEM_P (result) + && ! targetm.calls.allocate_stack_slots_for_args()) + { + error ("local frame unavailable (naked function?)"); + return result; + } + else + gcc_assert (MEM_P (result)); result = XEXP (result, 0); /* ??? Is this needed anymore? */ if (DECL_P (exp) && !TREE_USED (exp) == 0) { assemble_external (exp);
This patch tests for at least one user-caused reason for this assertion failing - requiring a local frame in a naked function. For this case at least, it would be better to trigger an error than to ICE. OK? static int bar; void __attribute__((naked)) function(void) { int foo, result; result = subFunction(&foo, &bar); // ICE here } * expr.c (expand_expr_addr_expr_1): Detect a user request for a local frame in a naked function, and produce a suitable error for that specific case.