| Message ID | 06e4ad1126b8e9231bf6dcf88205857081968274.1721779872.git.victorm.lira@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] automation: add linker symbol name script | expand |
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
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
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 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,
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
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