Iain Sandoe <i...@sandoe.co.uk> writes:
> Hi
>
>> On 23 Nov 2023, at 16:11, Christophe Lyon <christophe.l...@linaro.org> wrote:
>>
>> Hi Iain,
>>
>> Thanks for dealing with this :-)
>>
>> On Thu, 23 Nov 2023 at 10:58, Iain Sandoe <iains....@gmail.com> wrote:
>>>
>>> Tested on a cross to armv8l-unknown-linux-gnueabihf where the failing
>>> testcase is restored, and on aarch64-linux-gnu where no change is seen
>>> on the aarch64.exp suite. Also tested on arm64 Darwin for aarch64.exp
>>> and aarch64-darwin.exp.
>>>
>>> OK for trunk, or some alternative would be better?
>>> Iain
>>>
>>> --- 8< ---
>>>
>>> The change applied in r14-5760-g2a46e0e7e20 changed the behaviour of
>>> functions with assembly like:
>>>
>>> bar:
>>> __acle_se_bar:
>>>
>>> Where both bar and __acle_se_bar are globals refering to the same
>>> function body. The old behaviour overrides 'bar' with '__acle_se_bar'
>>> and the scan tests for that label.
>>>
>>> The change here re-allows the override.
>>>
>>> Case like this are not legal Mach-O (where two global symbols cannot
>>> have the same address in the assembler output). However, given the
>>> constraints on the Mach-O scanning, it does not seem that it is
>>> necessary to skip the change (any incorrect case should be easily
>>> evident in the assembler).
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * lib/scanasm.exp: Allow multiple function start symbols,
>>> taking the last as the function name.
>>>
>>> Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
>>> ---
>>> gcc/testsuite/lib/scanasm.exp | 10 +++++++++-
>>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp
>>> index 85ee54ff9a8..7ec3cfce02b 100644
>>> --- a/gcc/testsuite/lib/scanasm.exp
>>> +++ b/gcc/testsuite/lib/scanasm.exp
>>> @@ -877,7 +877,15 @@ proc parse_function_bodies { config filename result } {
>>> set in_function 0
>>> }
>>> } elseif { $in_function } {
>>> - if { [regexp $up_config(end) $line] } {
>>> + # We allow multiple function start labels, taking the last one
>>> seen
>>> + # as the function name.
>>> + if { [regexp [lindex $up_config(start) 0] \
>>> + $line dummy maybe_function_name] } {
>>> + verbose "parse_function_bodies: overriding $function_name
>>> with $maybe_function_name"
>>> + set function_name $maybe_function_name
>>> + set in_function 1
>> this is not necessary, since we are already inside if ($in_function) ?
>
> It resets the state to “first line matched” for mutli-line function start
> cases. Currently, those cases are only for Darwin and as noted in the
> changelog actually such code is not legal Mach-O, so in practice, at present
> we could remove it - but it might be better to be consistent (I am OK either
> way).
>
> ====
>
> Of course, an alternate fix would be to match the first label always and
> change the testcase to scan for ‘bar’ - since that seems to be the only
> instance needing this facility - but I’ll leave that to you folks to
> consider.
A third option would be to collect a list of function names and record
them all. But that won't be any simpler than the posted patch, and there
are no known use cases for multiple names. So yeah, the patch as posted
is OK. The:
>>> + set function_body ""
is redundant though, so IMO it's clearer to drop it.
Thanks,
Richard
>
> Iain
>
>>
>>> + } elseif { [regexp $up_config(end) $line] } {
>>> verbose "parse_function_bodies:
>>> $function_name:\n$function_body"
>>> set up_result($function_name) $function_body
>>> set in_function 0
>>> --
>>> 2.39.2 (Apple Git-143)
>>>
>>
>> Thanks,
>>
>> Christophe
