Bug 65958 - -fstack-check breaks alloca on architectures using generic stack checking
Summary: -fstack-check breaks alloca on architectures using generic stack checking
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.9.2
: P3 normal
Target Milestone: 6.0
Assignee: Eric Botcazou
URL: https://gcc.gnu.org/ml/gcc-patches/20...
Keywords:
: 14236 (view as bug list)
Depends on:
Blocks: 65964
  Show dependency treegraph
 
Reported: 2015-05-01 06:48 UTC by Felix Janda
Modified: 2017-01-11 23:49 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2015-05-02 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Felix Janda 2015-05-01 06:48:22 UTC
On arm, gcc-4.9.2 compiles the following snippet to a program which
returns 1 instead of 0 when the option -fstack-check is given.

int main(void)
{
	char *p;
	if(1) {
		char i[48];
		p = __builtin_alloca(8);
		p[0] = 1;
	}
	if(1) {
		char i[48], j[64];
		j[48] = 0;
	}
	return !p[0];
}


I could reproduce the behavior already with gcc-4.7.1.

The issue causes miscompilation of bash's lib/glob/glob.c. See:
https://bugs.gentoo.org/show_bug.cgi?id=518598
Comment 1 Anthony G. Basile 2015-05-01 11:20:26 UTC
We should add that we've only seen this on arm arch.
Comment 2 Felix Janda 2015-05-01 12:40:17 UTC
Actually I can hit this issue also with sh4 and microblaze. The test
program needs to be modified slightly:

int main(void)
{
	char *p;
	if(1) {
		char i[48];
		p = __builtin_alloca(8);
		p[0] = 1;
	}
	if(1) {
		char i[48] , j[64];
		j[52] = 0;
	}
	return !p[0];
}
Comment 3 Eric Botcazou 2015-05-02 09:40:33 UTC
Known issue with architectures doing stack-checking the old way like ARM, but the underlying issue is more general and related to VLAs:

extern void abort (void);

int foo (int n)
{
  char *p, *q;

  if (1)
    {
      char i[n];
      p = __builtin_alloca (8);
      p[0] = 1;
    }

  q = __builtin_alloca (64);
  __builtin_memset (q, 0, 64);

  return !p[0];
}

int main (void)
{
  if (foo (48) != 0)
    abort ();

  return 0;
}

fails on x86-64 because of it (with or without -fstack-check).
Comment 4 Eric Botcazou 2015-05-02 09:43:05 UTC
Two things to do here: 1) switch ARM to modern stack-checking and 2) fix the underlying issue with alloca and VLAs.
Comment 5 Eric Botcazou 2015-09-17 11:07:29 UTC
Author: ebotcazou
Date: Thu Sep 17 11:06:57 2015
New Revision: 227860

URL: https://gcc.gnu.org/viewcvs?rev=227860&root=gcc&view=rev
Log:
	PR middle-end/65958
	* config/arm/linux-elf.h (STACK_CHECK_STATIC_BUILTIN): Define.
	* config/arm/arm-protos.h (output_probe_stack_range): Declare.
	* config/arm/arm.c: Include common/common-target.h.
	(use_return_insn): Return 0 if the static chain register was saved
	above a non-APCS frame.
	(arm_compute_static_chain_stack_bytes): Adjust for stack checking.
	(struct scratch_reg): New.
	(get_scratch_register_on_entry): New function.
	(release_scratch_register_on_entry): Likewise.
	(arm_emit_probe_stack_range): Likewise.
	(output_probe_stack_range): Likewise.
	(arm_expand_prologue): Factor out code dealing with the IP register
	for nested function and adjust it for stack checking.
	Invoke arm_emit_probe_stack_range if static builtin stack checking
	is enabled.
	(thumb1_expand_prologue): Sorry out if static builtin stack checking
	is enabled.
	(arm_expand_epilogue): Add the saved static chain register, if any, to
	the amount of pre-pushed registers to pop.
	(arm_frame_pointer_required): Return true if static stack checking is
	enabled and we want to catch the exception with the EABI unwinder.
	* config/arm/unspecs.md (UNSPEC_PROBE_STACK): New constant.
	(UNSPEC_PROBE_STACK_RANGE): Likewise.
	* config/arm/arm.md (probe_stack): New insn.
	(probe_stack_range): Likewise.

Added:
    trunk/gcc/testsuite/gcc.target/arm/stack-checking.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm-protos.h
    trunk/gcc/config/arm/arm.c
    trunk/gcc/config/arm/arm.md
    trunk/gcc/config/arm/linux-elf.h
    trunk/gcc/config/arm/unspecs.md
    trunk/gcc/testsuite/ChangeLog
Comment 6 Eric Botcazou 2015-12-04 11:52:35 UTC
Author: ebotcazou
Date: Fri Dec  4 11:52:04 2015
New Revision: 231259

URL: https://gcc.gnu.org/viewcvs?rev=231259&root=gcc&view=rev
Log:
	PR middle-end/65958
	* config/aarch64/aarch64-protos.h (aarch64_output_probe_stack-range):
	Declare.
	* config/aarch64/aarch64.md: Declare UNSPECV_BLOCKAGE and
	UNSPEC_PROBE_STACK_RANGE.
	(blockage): New instruction.
	(probe_stack_range_<PTR:mode>): Likewise.
	* config/aarch64/aarch64.c (aarch64_emit_probe_stack_range): New
	function.
	(aarch64_output_probe_stack_range): Likewise.
	(aarch64_expand_prologue): Invoke aarch64_emit_probe_stack_range if
	static builtin stack checking is enabled.
	* config/aarch64/aarch64-linux.h (STACK_CHECK_STATIC_BUILTIN):
	Define to 1.

Added:
    trunk/gcc/testsuite/gcc.target/aarch64/stack-checking.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/aarch64/aarch64-linux.h
    trunk/gcc/config/aarch64/aarch64-protos.h
    trunk/gcc/config/aarch64/aarch64.c
    trunk/gcc/config/aarch64/aarch64.md
    trunk/gcc/testsuite/ChangeLog
Comment 7 Eric Botcazou 2015-12-04 11:57:47 UTC
Author: ebotcazou
Date: Fri Dec  4 11:57:15 2015
New Revision: 231260

URL: https://gcc.gnu.org/viewcvs?rev=231260&root=gcc&view=rev
Log:
	PR middle-end/65958
	* gimplify.c (struct gimplify_ctx): Turn boolean fields into 1-bit
	fields, add keep_stack and reorder them.
	(gimplify_bind_expr): Save gimplify_ctxp->keep_stack on entry then
	set it to false.  Do not insert a stack save/restore pair if it has
	been set to true by the gimplification of the statements.
	Restore it to the saved value on exit if it is still false.
	(gimplify_vla_decl): Do not set gimplify_ctxp->save_stack here.
	(gimplify_call_expr) <BUILT_IN_ALLOCA[_WITH_ALIGN]>: New case.  Set
	either save_stack or keep_stack depending on CALL_ALLOCA_FOR_VAR_P.
	* doc/extend.texi (Variable Length): Document new behavior.
	* doc/generic.texi (Blocks): Document new handling of VLAs.

Added:
    trunk/gcc/testsuite/gcc.dg/vla-24.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/doc/extend.texi
    trunk/gcc/doc/generic.texi
    trunk/gcc/gimplify.c
    trunk/gcc/testsuite/ChangeLog
Comment 8 Eric Botcazou 2015-12-04 12:00:27 UTC
Fixed in GCC 6 for all architectures (and doubly so for ARM and Aarch64).
Comment 9 Eric Botcazou 2015-12-04 18:26:26 UTC
Author: ebotcazou
Date: Fri Dec  4 18:25:54 2015
New Revision: 231295

URL: https://gcc.gnu.org/viewcvs?rev=231295&root=gcc&view=rev
Log:
	PR middle-end/65958
	* config/arm/unspecs.md (unspec): Remove UNSPEC_PROBE_STACK_RANGE.
	(unspecv): Add VUNSPEC_PROBE_STACK_RANGE.
	* config/arm/arm.md (probe_stack_range): Adjust.
	* config/aarch64/aarch64.md (unspec): Remove UNSPEC_PROBE_STACK_RANGE.
	(unspecv): Add UNSPECV_PROBE_STACK_RANGE.
	(probe_stack_range_<PTR:mode>): Adjust.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/aarch64/aarch64.md
    trunk/gcc/config/arm/arm.md
    trunk/gcc/config/arm/unspecs.md
Comment 10 Martin Sebor 2017-01-11 23:49:36 UTC
*** Bug 14236 has been marked as a duplicate of this bug. ***