From: Christophe Lyon <christophe.lyon@linaro.org>
To: Tom de Vries <Tom_deVries@mentor.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
Rainer Orth <ro@cebitec.uni-bielefeld.de>
Subject: Re: [PATCH, testsuite] Add effective target stack_size
Date: Mon, 19 Jun 2017 09:21:00 -0000 [thread overview]
Message-ID: <CAKdteOb9xoC9QvOFDUK_eGK65J38x=-3MK73mxwr=ufN_AuAQw@mail.gmail.com> (raw)
In-Reply-To: <6521e32f-83ce-bc48-f371-8324228c467e@mentor.com>
[-- Attachment #1: Type: text/plain, Size: 3497 bytes --]
On 12 June 2017 at 16:28, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 06/12/2017 02:28 PM, Christophe Lyon wrote:
>>
>> Hi Tom,
>>
>> On 9 June 2017 at 17:25, Mike Stump <mikestump@comcast.net> wrote:
>>>
>>> On Jun 9, 2017, at 7:24 AM, Tom de Vries <Tom_deVries@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
[-- Attachment #2: badalloc.chlog.txt --]
[-- Type: text/plain, Size: 147 bytes --]
2017-06-19 Christophe Lyon <christophe.lyon@linaro.org>
gcc/testsuite/
* g++.old-deja/g++.eh/badalloc1.C: Remove code path for
-DSTACK_SIZE.
[-- Attachment #3: badalloc.patch.txt --]
[-- Type: text/plain, Size: 1587 bytes --]
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 <nathan@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
{
next prev parent reply other threads:[~2017-06-19 9:21 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-09 14:24 Tom de Vries
2017-06-09 15:25 ` Mike Stump
2017-06-12 12:28 ` Christophe Lyon
2017-06-12 14:29 ` Tom de Vries
2017-06-19 9:21 ` Christophe Lyon [this message]
2017-06-19 15:42 ` Mike Stump
2017-06-19 17:11 ` Add dg-add-options feature stack_size Tom de Vries
2017-06-19 17:36 ` Mike Stump
2017-06-21 13:19 ` [PATCH, testsuite] Add effective target stack_size Jakub Jelinek
2017-06-21 14:27 ` [testsuite, committed] Support dg-add-options in profopt.exp Tom de Vries
2022-11-08 20:29 ` nvptx: stack size limits are relevant for execution only (was: [PATCH, testsuite] Add effective target stack_size) Thomas Schwinge
2022-11-25 11:09 ` [PING] " Thomas Schwinge
2022-12-20 7:55 ` [PING^2] " Thomas Schwinge
2023-01-11 11:45 ` [PING^3] " Thomas Schwinge
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKdteOb9xoC9QvOFDUK_eGK65J38x=-3MK73mxwr=ufN_AuAQw@mail.gmail.com' \
--to=christophe.lyon@linaro.org \
--cc=Tom_deVries@mentor.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ro@cebitec.uni-bielefeld.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).