Re: [PATCH, testsuite] Add effective target stack_size

[Christophe Lyon <christophe dot lyon at linaro dot org>]

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: badalloc.chlog.txt
Description: Text document

Attachment: badalloc.patch.txt
Description: Text document