From: Victor Lira <victorm.lira@amd.com>

Add a script that extracts the names of symbols in linker scripts.

Signed-off-by: Victor Lira <victorm.lira@amd.com>
---
Note:
Not included are the "." location name or symbol names enclosed in quotes
since the files dont't use any.
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: roberto.bagnara@bugseng.com
Cc: consulting@bugseng.com
Cc: simone.ballarin@bugseng.com
---
 automation/eclair_analysis/linker_symbols.sh | 41 +++++++++++++++++++
 automation/eclair_analysis/stuff.txt         | 42 ++++++++++++++++++++
 2 files changed, 83 insertions(+)
 create mode 100755 automation/eclair_analysis/linker_symbols.sh
 create mode 100644 automation/eclair_analysis/stuff.txt

diff --git a/automation/eclair_analysis/linker_symbols.sh b/automation/eclair_analysis/linker_symbols.sh
new file mode 100755
index 0000000000..c8c44e235f
--- /dev/null
+++ b/automation/eclair_analysis/linker_symbols.sh
@@ -0,0 +1,41 @@
+#!/bin/bash
+# Stop immediately if any executed command has exit status different from 0.
+set -e
+
+# Extract linker symbol names (except those starting with ".") from assignments.
+
+script_name=`basename "$0"`
+script_dir="$(
+  cd "$(dirname "$0")"
+  echo "${PWD}"
+)"
+src_dir="${script_dir}/../.."
+
+usage() {
+  echo "Usage: ${script_name} <ARM64|X86_64>"
+}
+
+if [ $# -ne 1 ]; then
+  usage
+  exit 1
+fi
+
+if [ "$1" == "X86_64" ]; then
+    filepaths=(
+        "${src_dir}/xen/arch/x86/xen.lds.S"
+    )
+elif [ "$1" == "ARM64" ]; then
+    filepaths=(
+        "${src_dir}/xen/arch/arm/xen.lds.S"
+    )
+else
+    usage
+    exit 1
+fi
+
+(
+    for file in "${filepaths[@]}"
+    do
+        sed -n "s/^\s*\([a-zA-Z_][a-zA-Z_0-9.\-]*\)\s*=.*;\s*$/\1/p" $filepaths
+    done
+)
diff --git a/automation/eclair_analysis/stuff.txt b/automation/eclair_analysis/stuff.txt
new file mode 100644
index 0000000000..efc33e6a59
--- /dev/null
+++ b/automation/eclair_analysis/stuff.txt
@@ -0,0 +1,42 @@
+_start
+_idmap_start
+_idmap_end
+__proc_info_start
+__proc_info_end
+__note_gnu_build_id_start
+__note_gnu_build_id_end
+__ro_after_init_start
+__ro_after_init_end
+__start___ex_table
+__stop___ex_table
+__start___pre_ex_table
+__stop___pre_ex_table
+__start_schedulers_array
+__end_schedulers_array
+_splatform
+_eplatform
+_sdevice
+_edevice
+_asdevice
+_aedevice
+_steemediator
+_eteemediator
+__init_begin
+_sinittext
+_einittext
+__setup_start
+__setup_end
+__initcall_start
+__presmp_initcall_end
+__initcall_end
+__alt_instructions
+__alt_instructions_end
+__ctors_start
+__ctors_end
+__init_end_efi
+__init_end
+__bss_start
+__per_cpu_start
+__per_cpu_data_end
+__bss_end
+_end
--
2.37.6

On 24.07.2024 02:18, victorm.lira@amd.com wrote:
> --- /dev/null
> +++ b/automation/eclair_analysis/linker_symbols.sh

Nit: In names of new files we prefer - over _.

> @@ -0,0 +1,41 @@
> +#!/bin/bash

Can we rely on bash to be there and at that location? As you using any
bash-isms in the script which cannot be avoided?

> +# Stop immediately if any executed command has exit status different from 0.
> +set -e
> +
> +# Extract linker symbol names (except those starting with ".") from assignments.
> +
> +script_name=`basename "$0"`

Personally I consider $(...) more readable. What I'm strictly going to
request is that within a script you at least be consistent, seeing ...

> +script_dir="$(
> +  cd "$(dirname "$0")"
> +  echo "${PWD}"
> +)"

... e.g. this.

> +src_dir="${script_dir}/../.."
> +
> +usage() {
> +  echo "Usage: ${script_name} <ARM64|X86_64>"

Why require arguments that then ...

> +}
> +
> +if [ $# -ne 1 ]; then
> +  usage
> +  exit 1
> +fi
> +
> +if [ "$1" == "X86_64" ]; then
> +    filepaths=(
> +        "${src_dir}/xen/arch/x86/xen.lds.S"
> +    )
> +elif [ "$1" == "ARM64" ]; then
> +    filepaths=(
> +        "${src_dir}/xen/arch/arm/xen.lds.S"
> +    )

... you need to special case? What's wrong with having the arguments be
"arm" or "x86"?

However, you also cannot use xen.lds.S here, as that may yield incomplete
results. This script needs running _after_ a successful build, on the
generated xen.lds.

> +else
> +    usage
> +    exit 1
> +fi
> +
> +(
> +    for file in "${filepaths[@]}"
> +    do
> +        sed -n "s/^\s*\([a-zA-Z_][a-zA-Z_0-9.\-]*\)\s*=.*;\s*$/\1/p" $filepaths
> +    done
> +)

Why the extra parentheses? And why a loop and filepaths being an array,
when there's guaranteed to be only one file?

Jan

On 7/24/2024 12:44 AM, Jan Beulich wrote:
> Nit: In names of new files we prefer - over _.
> +script_name=`basename "$0"`
I have fixed the above comments in v2.

>> +#!/bin/bash
> Can we rely on bash to be there and at that location? As you using any
> bash-isms in the script which cannot be avoided?

Are the automation scripts required to be portable? Can you please point 
me to a resource where I can learn how to make the script portable?

Victor

On 24.07.2024 19:48, Lira, Victor M wrote:
> On 7/24/2024 12:44 AM, Jan Beulich wrote:
>> Nit: In names of new files we prefer - over _.
>> +script_name=`basename "$0"`
> I have fixed the above comments in v2.
> 
>>> +#!/bin/bash
>> Can we rely on bash to be there and at that location? As you using any
>> bash-isms in the script which cannot be avoided?
> 
> Are the automation scripts required to be portable? Can you please point 
> me to a resource where I can learn how to make the script portable?

In addition to what Jason pointed you at, the more abstract answer is to
look at the shell specification. E.g.

https://pubs.opengroup.org/onlinepubs/007904975/utilities/contents.html
(issue 6, i.e. a little dated by now)

https://pubs.opengroup.org/onlinepubs/9699919799/
(issue 7, also already about 6 years old)

https://pubs.opengroup.org/onlinepubs/9799919799/
(issue 8, very resent)

The older variants may be relevant when backward compatibility matters.

Jan

On 2024-07-24 13:48, Lira, Victor M wrote:
> 
> On 7/24/2024 12:44 AM, Jan Beulich wrote:
>> Nit: In names of new files we prefer - over _.
>> +script_name=`basename "$0"`
> I have fixed the above comments in v2.
> 
>>> +#!/bin/bash
>> Can we rely on bash to be there and at that location? As you using any
>> bash-isms in the script which cannot be avoided?
> 
> Are the automation scripts required to be portable? Can you please point 
> me to a resource where I can learn how to make the script portable?

Hi Victor,

You might want to check out `checkbashisms`:

$ checkbashisms
Usage: checkbashisms [-n] [-f] [-x] [-e] [-l] script ...
    or: checkbashisms --help
    or: checkbashisms --version
This script performs basic checks for the presence of bashisms
in /bin/sh scripts and the lack of bashisms in /bin/bash ones.

Since your script has '#!/bin/bash', you'd run it with -f to force the 
bashism checks (or change to /bin/sh first).

Regards,
Jason

Le mer. 24 juil. 2024 à 21:56, Jason Andryuk <jason.andryuk@amd.com> a écrit :
>
> On 2024-07-24 13:48, Lira, Victor M wrote:
> >
> > On 7/24/2024 12:44 AM, Jan Beulich wrote:
> >> Nit: In names of new files we prefer - over _.
> >> +script_name=`basename "$0"`
> > I have fixed the above comments in v2.
> >
> >>> +#!/bin/bash
> >> Can we rely on bash to be there and at that location? As you using any
> >> bash-isms in the script which cannot be avoided?
> >
> > Are the automation scripts required to be portable? Can you please point
> > me to a resource where I can learn how to make the script portable?
>
> Hi Victor,
>
> You might want to check out `checkbashisms`:
>
> $ checkbashisms
> Usage: checkbashisms [-n] [-f] [-x] [-e] [-l] script ...
>     or: checkbashisms --help
>     or: checkbashisms --version
> This script performs basic checks for the presence of bashisms
> in /bin/sh scripts and the lack of bashisms in /bin/bash ones.
>
> Since your script has '#!/bin/bash', you'd run it with -f to force the
> bashism checks (or change to /bin/sh first).

Just for completeness, you could also use shellcheck.

https://www.shellcheck.net/

I haven't tried checkbashisms so I can't really provide any comparison
between the two.

Cheers,

-- 
Julien Grall

On 2024-07-24 02:18, victorm.lira@amd.com wrote:
> From: Victor Lira <victorm.lira@amd.com>
> 

Hi,

> Add a script that extracts the names of symbols in linker scripts.
> 
> Signed-off-by: Victor Lira <victorm.lira@amd.com>
> ---
> Note:
> Not included are the "." location name or symbol names enclosed in 
> quotes
> since the files dont't use any.
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: roberto.bagnara@bugseng.com
> Cc: consulting@bugseng.com
> Cc: simone.ballarin@bugseng.com
> ---
>  automation/eclair_analysis/linker_symbols.sh | 41 +++++++++++++++++++
>  automation/eclair_analysis/stuff.txt         | 42 ++++++++++++++++++++
>  2 files changed, 83 insertions(+)
>  create mode 100755 automation/eclair_analysis/linker_symbols.sh
>  create mode 100644 automation/eclair_analysis/stuff.txt
> 
> diff --git a/automation/eclair_analysis/linker_symbols.sh 
> b/automation/eclair_analysis/linker_symbols.sh
> new file mode 100755
> index 0000000000..c8c44e235f
> --- /dev/null
> +++ b/automation/eclair_analysis/linker_symbols.sh
> @@ -0,0 +1,41 @@
> +#!/bin/bash
> +# Stop immediately if any executed command has exit status different 
> from 0.
> +set -e
> +
> +# Extract linker symbol names (except those starting with ".") from 
> assignments.
> +
> +script_name=`basename "$0"`
> +script_dir="$(
> +  cd "$(dirname "$0")"
> +  echo "${PWD}"
> +)"
> +src_dir="${script_dir}/../.."
> +
> +usage() {
> +  echo "Usage: ${script_name} <ARM64|X86_64>"
> +}
> +
> +if [ $# -ne 1 ]; then
> +  usage
> +  exit 1
> +fi
> +
> +if [ "$1" == "X86_64" ]; then
> +    filepaths=(
> +        "${src_dir}/xen/arch/x86/xen.lds.S"
> +    )
> +elif [ "$1" == "ARM64" ]; then
> +    filepaths=(
> +        "${src_dir}/xen/arch/arm/xen.lds.S"
> +    )
> +else
> +    usage
> +    exit 1
> +fi
> +
> +(
> +    for file in "${filepaths[@]}"
> +    do
> +        sed -n "s/^\s*\([a-zA-Z_][a-zA-Z_0-9.\-]*\)\s*=.*;\s*$/\1/p" 
> $filepaths
> +    done
> +)
> diff --git a/automation/eclair_analysis/stuff.txt 
> b/automation/eclair_analysis/stuff.txt
> new file mode 100644
> index 0000000000..efc33e6a59
> --- /dev/null
> +++ b/automation/eclair_analysis/stuff.txt

I wouldn't include the actual output in the patch. It' much better if I 
have a script that produces a list of symbols, and then use that to 
generate a configuration file right before the analysis.

> @@ -0,0 +1,42 @@
> +_start
> +_idmap_start
> +_idmap_end
> +__proc_info_start
> +__proc_info_end
> +__note_gnu_build_id_start
> +__note_gnu_build_id_end
> +__ro_after_init_start
> +__ro_after_init_end
> +__start___ex_table
> +__stop___ex_table
> +__start___pre_ex_table
> +__stop___pre_ex_table
> +__start_schedulers_array
> +__end_schedulers_array
> +_splatform
> +_eplatform
> +_sdevice
> +_edevice
> +_asdevice
> +_aedevice
> +_steemediator
> +_eteemediator
> +__init_begin
> +_sinittext
> +_einittext
> +__setup_start
> +__setup_end
> +__initcall_start
> +__presmp_initcall_end
> +__initcall_end
> +__alt_instructions
> +__alt_instructions_end
> +__ctors_start
> +__ctors_end
> +__init_end_efi
> +__init_end
> +__bss_start
> +__per_cpu_start
> +__per_cpu_data_end
> +__bss_end
> +_end
> --
> 2.37.6

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

On 24.07.2024 08:27, Nicola Vetrini wrote:
> On 2024-07-24 02:18, victorm.lira@amd.com wrote:
>> --- /dev/null
>> +++ b/automation/eclair_analysis/stuff.txt
> 
> I wouldn't include the actual output in the patch. It' much better if I 
> have a script that produces a list of symbols, and then use that to 
> generate a configuration file right before the analysis.

+1

Jan