On 12 June 2017 at 16:28, Tom de Vries <tom_devr...@mentor.com> wrote: > On 06/12/2017 02:28 PM, Christophe Lyon wrote: >> >> Hi Tom, >> >> On 9 June 2017 at 17:25, Mike Stump <mikest...@comcast.net> wrote: >>> >>> On Jun 9, 2017, at 7:24 AM, Tom de Vries <tom_devr...@mentor.com> wrote: >>>> >>>> this patch adds effective target stack_size. >>> >>> >>>> OK for trunk if x86_64 and nvptx testing succeeds? >>> >>> >>> Ok. >>> >>> The only last issue in this area that I know about is that there are a >>> few more test cases that need up to 48 MB to run, the problem is that >>> targets might have substantially less memory. Stack size is one of the ways >>> this problem can be exposed. The failure to load case is or can be handled >>> in other ways, but the dynamic allocation case I think is relatively poorly >>> handled. On my machine, I just punted by running on a virtual simulator >>> that I pushed memory up to 48 MB and ignored the issue. If anyone wants to >>> try their hand at it, I'd be happy to review some patches. For those on >>> demand virtual memory systems, of course, the problem is invisible. I >>> didn't have any good ideas in this area. Marking large memory test cases >>> with size information, and then just trimming based upon size was my only >>> thought. Not exactly portable, as the exact size of any test case is of >>> course target dependent; but, if we get close enough, it can provide enough >>> of a solution I think. >>> >>> If people have better ideas in this area, even if you don't want to >>> implement them, it'd be nice to hear about them. >> >> >> After this commit (r249090), I've noticed that badalloc1.C fails at >> execution on aarch64 and arm bare-metal targets. >> >> It is compiled with -DSTACK_SIZE=16384, maybe that's too small? > > > I think that what's going on is the following: > - your board description file for aarch64 and arm bare-metal sets > gcc,stack_size > - before I committed the patch, STACK_SIZE was not defined when > compiling this testcase, because the activated .exp files do not > define it > - after I committed the patch, STACK_SIZE started to be defined, and > the test started to fail >
I think you are right. > I'm not sure if this test was ever compiled with STACK_SIZE defined. > > Either way, the test-case uses the presence of STACK_SIZE, not the actual > value, so changing the value of gcc,stack_size won't make a difference. > > Ideally you'd find out what the exact reason for the failure is, and update > the test-case accordingly. > > The easiest thing we can do is to remove the STACK_SIZE setting in the > test-case (and to avoid confusion, remove all the dead STACK_SIZE-enabled > code), which returns the status quo of before the patch. > I tried to compile with -DSTACK_SIZE & execute the test on x86, and the first call to malloc() (as defined in the testcase) aborts. This call occurs before entering main() and tries to allocate size=72704, which is way larger than arena_size = 256 + 8 * 128 (=1280). This is with a shared libstdc++. Linking with -static also implies using -Wl,--allow-multiple-definition, and leads to a failure to allocate size=5280. I too wonder whether the test ever worked with STACK_SIZE defined? (Yet, arena_size was updated when PR64535 was fixed) The attached patch removes the support for STACK_SIZE in the testcase as you suggested, and it works fine (cross-tested on aarch64/arm targets) OK for trunk? Thanks, Christophe > Thanks, > - Tom
2017-06-19 Christophe Lyon <christophe.l...@linaro.org> gcc/testsuite/ * g++.old-deja/g++.eh/badalloc1.C: Remove code path for -DSTACK_SIZE.
diff --git a/gcc/testsuite/g++.old-deja/g++.eh/badalloc1.C b/gcc/testsuite/g++.old-deja/g++.eh/badalloc1.C index f63d5c6..b660e84 100644 --- a/gcc/testsuite/g++.old-deja/g++.eh/badalloc1.C +++ b/gcc/testsuite/g++.old-deja/g++.eh/badalloc1.C @@ -3,7 +3,6 @@ // itself call malloc(), and will fail if there is no more // memory available. // { dg-do run { xfail { { xstormy16-*-* *-*-darwin[3-7]* } || vxworks_rtp } } } -// { dg-additional-options "-DSTACK_SIZE=[dg-effective-target-value stack_size]" { target { stack_size } } } // Copyright (C) 2000, 2002, 2003, 2010, 2012, 2014 Free Software Foundation, Inc. // Contributed by Nathan Sidwell 6 June 2000 <nat...@codesourcery.com> @@ -16,12 +15,6 @@ extern "C" void *memcpy(void *, const void *, size_t); // libstdc++ requires a large initialization time allocation for the // emergency EH allocation pool. Add that to the arena size. -// Assume that STACK_SIZE defined implies a system that does not have a -// large data space either, and additionally that we're not linking against -// a shared libstdc++ (which requires quite a bit more initialization space). -#ifdef STACK_SIZE -const int arena_size = 256 + 8 * 128; -#else #if defined(__FreeBSD__) || defined(__sun__) || defined(__hpux__) // FreeBSD, Solaris and HP-UX require even more space at initialization time. // FreeBSD 5 now requires over 131072 bytes. @@ -32,7 +25,6 @@ const int arena_size = 262144 + 72 * 1024; // 32-bit-systems-based estimate. const int arena_size = 32768 * ((sizeof (void *) + 3)/4) + 72 * 1024; #endif -#endif struct object {