Hi Richard,
> On 5 Nov 2023, at 12:11, Richard Sandiford <richard.sandif...@arm.com> wrote:
>
> Iain Sandoe <i...@sandoe.co.uk> writes:
>>> On 26 Oct 2023, at 21:00, Iain Sandoe <i...@sandoe.co.uk> wrote:
>>
>>>> On 26 Oct 2023, at 20:49, Richard Sandiford <richard.sandif...@arm.com>
>> wrote:
>>>>
>>>> Iain Sandoe <iains....@gmail.com> writes:
>>>>> This was written before Thomas' modification to the ELF-handling to allow
>>>>> a config-based change for target details. I did consider updating this
>>>>> to try and use that scheme, but I think that it would sit a little
>>>>> awkwardly, since there are some differences in the start-up scanning for
>>>>> Mach-O. I would say that in all probability we could improve things but
>>>>> I'd like to put this forward as a well-tested initial implementation.
>>>>
>>>> Sorry, I would prefer to extend the existing function instead.
>>>> E.g. there's already some divergence between the Mach-O version
>>>> and the default version, in that the Mach-O version doesn't print
>>>> verbose messages. I also don't think that the current default code
>>>> is so watertight that it'll never need to be updated in future.
>>>
>>> Fair enough, will explore what can be done (as I recall last I looked the
>>> primary difference was in the initial start-up scan).
>>
>> I’ve done this as attached.
>>
>> For the record, when doing it, it gave rise to the same misgivings that led
>> to the separate implementation before.
>>
>> * as we add formats and uncover asm oddities, they all need to be handled
>> in one set of code, IMO it could be come quite convoluted.
>>
>> * now making a change to the MACH-O code, means I have to check I did not
>> inadvertently break ELF (and likewise, in theory, an ELF change should
>> check
>> MACH-O, but many folks do/can not do that).
>>
>> Maybe there’s some half-way-house where code can usefully be shared without
>> those down-sides.
>>
>> Anyway, to make progress, is the revised version OK for trunk? (tested on
>> aarch64-linux and aarch64-darwin).
>
> Sorry for the slow reply. I was hoping we'd be able to share a bit more
> code than that, and avoid an isMACHO toggle. Does something like the
> attached adaption of your patch work? Only spot-checked on
> aarch64-linux-gnu so far.
>
> (The patch tries to avoid capturing the user label prefix, hopefully
> avoiding the needsULP thing.)
Yes, this works for me too for Arm64 Darwin (and probably is fine for other
Darwin archs in case we implement body tests there). If we decide to emit
some comment-based markers to delineat functions without unwind data,
we can just amend the start and end.
thanks,
Iain
(doing some wider testing, but for now the only mach-o cases are in the
arm64 code, so the fact that those passed so far is pretty good indication).
-----
As an aside what’s the intention for cases like this?
.data
foo:
.xxxx ….
.size foo, .-foo
>
> Thanks,
> Richard
>
>
> diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp
> index 5df80325dff..2434550f0c3 100644
> --- a/gcc/testsuite/lib/scanasm.exp
> +++ b/gcc/testsuite/lib/scanasm.exp
> @@ -785,23 +785,34 @@ proc configure_check-function-bodies { config } {
>
> # Regexp for the start of a function definition (name in \1).
> if { [istarget nvptx*-*-*] } {
> - set up_config(start) {^// BEGIN(?: GLOBAL|) FUNCTION DEF:
> ([a-zA-Z_]\S+)$}
> + set up_config(start) {
> + {^// BEGIN(?: GLOBAL|) FUNCTION DEF: ([a-zA-Z_]\S+)$}
> + }
> + } elseif { [istarget *-*-darwin*] } {
> + set up_config(start) {
> + {^_([a-zA-Z_]\S+):$}
> + {^LFB[0-9]+:}
> + }
> } else {
> - set up_config(start) {^([a-zA-Z_]\S+):$}
> + set up_config(start) {{^([a-zA-Z_]\S+):$}}
> }
>
> # Regexp for the end of a function definition.
> if { [istarget nvptx*-*-*] } {
> set up_config(end) {^\}$}
> + } elseif { [istarget *-*-darwin*] } {
> + set up_config(end) {^LFE[0-9]+}
> } else {
> set up_config(end) {^\s*\.size}
> }
> -
> +
> # Regexp for lines that aren't interesting.
> if { [istarget nvptx*-*-*] } {
> # Skip lines beginning with '//' comments ('-fverbose-asm', for
> # example).
> set up_config(fluff) {^\s*(?://)}
> + } elseif { [istarget *-*-darwin*] } {
> + set up_config(fluff) {^\s*(?:\.|//|@)|^L[0-9ACESV]}
> } else {
> # Skip lines beginning with labels ('.L[...]:') or other directives
> # ('.align', '.cfi_startproc', '.quad [...]', '.text', etc.), '//' or
> @@ -833,9 +844,19 @@ proc parse_function_bodies { config filename result } {
> set fd [open $filename r]
> set in_function 0
> while { [gets $fd line] >= 0 } {
> - if { [regexp $up_config(start) $line dummy function_name] } {
> - set in_function 1
> - set function_body ""
> + if { $in_function == 0 } {
> + if { [regexp [lindex $up_config(start) 0] \
> + $line dummy function_name] } {
> + set in_function 1
> + set function_body ""
> + }
> + } elseif { $in_function < [llength $up_config(start)] } {
> + if { [regexp [lindex $up_config(start) $in_function] $line] } {
> + incr in_function
> + } else {
> + verbose "parse_function_bodies: skipped $function_name"
> + set in_function 0
> + }
> } elseif { $in_function } {
> if { [regexp $up_config(end) $line] } {
> verbose "parse_function_bodies: $function_name:\n$function_body"
> --
> 2.25.1
