Hello, Below is a description of a very annoying bug we are witnessing on ARM. I have ideas of possible ways to address it, but don't yet have a patch.
The problem touches the synchronization of register elimination, dataflow analysis and arm_expand_prologue. The overall logic is pretty intricate, so I would appreciate at least insights from maintainers on what would seem the most plausible direction. If someone wants to take it, even better. I can certainly open a PR :) The Ada testcase below, compiled with -Og -mapcs by an arm-eabi compiler or simply with -Og by an arm-wrs-vxworks compiler (apcs by default), produces code which uses a stack slot beyond the allocated stack area for the "for_each" nested function. The slot at hand is used as the stack home for the static chain, which gets clobbered on the first call to Read and then bad things happen. The actual manifestation we observed with the real code was "fun" but I'll save that for the time being - that message is long enough :) We started observing this with a gcc-8 compiler and I could reproduce with a pristine mainline early this week. Here's a example compilation and the output we get, with notes describing what register points where as the code proceeds: $ arm-wrs-vxworks-gcc -S --RTS=kernel p.adb -Og -gnatn -o - |& grep -A 30 "for_each.*:" p__compute_blocks__for_each_item$2970: .LFB5: .cfi_startproc @ Nested: function declared inside another function. @ args = 0, pretend = 0, frame = 36 @ frame_needed = 1, uses_anonymous_args = 0 str ip, [sp, #-4]! -- push ip add ip, sp, #4 -- ip = entry_sp push {r4, r5, r6, r7, r8, r9, r10, fp, ip, lr, pc} -- push 11 regs, 44 bytes sub fp, ip, #8 -- fp = ip - 8, that is, entry_sp - 8, -- first registers save slot. ldr ip, [fp, #4] sub sp, sp, #32 -- allocate 32 more bytes. sp is entry_sp - 80 here -- (push ip = 4 bytes, multi push = 44 bytes, then -- allocate 32) mov r8, r0 str r1, [fp, #-68] str ip, [fp, #-76] -- store ip at fp - 76, that is, entry_sp - 84, -- 4 bytes beyond the end of the allocated frame. The problematic store is initially emitted as p.adb.235r.expand:(insn 3 2 4 2 (set (reg/f:SI 111 [ CHAIN$45 ]) p.adb.235r.expand- (reg:SI 12 ip [ CHAIN$45 ])) "p.adb":24:7 -1 p.adb.235r.expand- (nil)) by expand_function_start, then becomes p.adb.279r.reload:(insn 3 320 4 2 (set (mem/c:SI (plus:SI (reg/f:SI 11 fp) p.adb.279r.reload- (const_int -76 [0xffffffffffffffb4])) [19 %sfp+-36 S4 A32]) p.adb.279r.reload- (reg:SI 12 ip [ CHAIN$47 ])) "p.adb":29:7 179 {*arm_movsi_insn} p.adb.279r.reload- (nil)) The core issue is a disagreement between register elimination (which turns %sfp-36 into %fp-76) and prologue expansion regarding static_chain_stack_bytes(), used both in arm_compute_frame_layout() and arm_expand_prologue(). arm_compute_static_chain_stack_bytes() returns 0 during the whole of register allocation, up to the end of register elimination and reload_completed flipped to true. The key factor is arm_r3_live_at_start_p() returning false all along. Then we very quickly get to: if (optimize) df_analyze (); in do_reload(), and arm_r3_live_at_start_p () starts returning true. arm_expand_prologue proceeds on this basis and decides to use a slot just ahead of "the stack frame" to temporarily store IP. Even though this store isn't really part of the "register save" operations per se, the code sort of pretends it is with: gcc_assert(arm_compute_static_chain_stack_bytes() == 4); saved_regs += 4; which later on shortens the amount of allocated stack by: if (offsets->outgoing_args != offsets->saved_args + saved_regs) { ... amount = GEN_INT (offsets->saved_args + saved_regs - offsets->outgoing_args); All in all, expand_prologue operates from a view of the stack layout which doesn't match what register elimination has seen, and the sfp->fp elimination is off by 4. The first idea that came to mind was that relying on r3_live_at_start_p during register elimination seems fragile, and maybe the prologue expansion should use a different spot for the ip temp storage. The prologue logic is complex though, so I'm not 100% sure this is a reasonable track. Another idea was to detect the perception mismatch and either skip the "use slot above frame" option in this case, or arrange to adjust the final stack pointer update accordingly. The first of these last two options seems cleaner as it goes towards a more precise synchronisation between the layout assumed by register elimination and the layout actually setup by the prologue expander. But I haven't yet experimented with it and I'm not certain how viable that would be in principle. The last option I have thought of was to perform another round of register elimination just for offsets adjustments after df_analyze. Not sure if that makes much sense (are we sure that this would never invalidate dataflow analysis ?) Thoughts ? Taker for a PR ? Thanks much in advance, With Kind Regards, Olivier ------------- package P is procedure Compute_Blocks (Blocks : out Integer); end; package body P is function Is_White_Space (Char : Integer) return Boolean; pragma Inline (Is_White_Space); function Is_White_Space (Char : Integer) return Boolean is begin return Char = 16#20# or else Char = 16#09# or else Char = 16#0A# or else Char = 16#0D#; end Is_White_Space; procedure Read (Str : String; Index : in out Positive; Char : out Integer) with Import; function Foo return String with Import; procedure Compute_Blocks (Blocks : out Integer) is procedure On_Item (Str : String) is begin Blocks := 66; end; procedure For_Each_Item (Ch : String) is Index : Integer := Ch'First; Last, Start : Integer; C : Integer; begin while Index <= Ch'Last loop -- Skip leading spaces while Index <= Ch'Last loop Start := Index; Read (Ch, Index, C); exit when not Is_White_Space (C); Start := Ch'Last + 1; end loop; exit when Start > Ch'Last; -- Move to first whitespace after word while Index <= Ch'Last loop Last := Index; Read (Ch, Index, C); exit when Index > Ch'Last or else Is_White_Space (C); end loop; if Index > Ch'Last and then not Is_White_Space (C) then On_Item (Ch (Start .. Index - 1)); exit; else On_item (Ch (Start .. Last - 1)); -- Last points to a whitespace end if; end loop; end For_Each_Item; begin Blocks := 0; For_Each_Item (Foo); end Compute_Blocks; end;