[RFC PATCH v3] automation: add linker symbol name script

victorm.lira@amd.com posted 1 patch 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/29c7f6cd166d5d3c0e9f64dc937e29dc7ecf3f2d.1721933988.git.victorm.lira@amd.com
automation/eclair_analysis/linker-symbols.sh | 34 ++++++++++++++++++++
1 file changed, 34 insertions(+)
create mode 100755 automation/eclair_analysis/linker-symbols.sh
[RFC PATCH v3] automation: add linker symbol name script
Posted by victorm.lira@amd.com 1 month, 2 weeks ago
From: Victor Lira <victorm.lira@amd.com>

Requested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Victor Lira <victorm.lira@amd.com>
---
Notes:
This is a utilty script for help with the MISRA process.
This script matches all linker symbol names in linker script files for
arm or x86.
Not included are symbol names starting with "." or symbol names enclosed
in quotes since the files dont't use any. The regular expression also does
not match for "&=" and similar compound assignments.
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: roberto.bagnara@bugseng.com
Cc: consulting@bugseng.com
Cc: simone.ballarin@bugseng.com
---
Changes v2:
- address style comments
- updated script to use .lds instead of .lds.S
- remove sample output from patch

Changes v3:
- use #!/bin/sh
- update error handling and message similar to ../build.sh
---
 automation/eclair_analysis/linker-symbols.sh | 34 ++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100755 automation/eclair_analysis/linker-symbols.sh

diff --git a/automation/eclair_analysis/linker-symbols.sh b/automation/eclair_analysis/linker-symbols.sh
new file mode 100755
index 0000000000..61790fb281
--- /dev/null
+++ b/automation/eclair_analysis/linker-symbols.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+# 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}/../.."
+
+fatal() {
+  echo "${script_name}: $*" >&2
+  exit 1
+}
+
+usage() {
+  fatal "Usage: ${script_name} <arm|x86>"
+}
+
+if [ $# -ne 1 ]; then
+  usage
+fi
+
+filepath="${src_dir}/xen/arch/${1}/xen.lds"
+
+if [ ! -f "$filepath" ]; then
+  fatal "Could not find ${1} linker script. Must be run after arm/x86 build."
+fi
+
+sed -n "s/^\s*\([a-zA-Z_][a-zA-Z_0-9.\-]*\)\s*=.*;.*$/\1/p" "$filepath"
--
2.25.1
Re: [RFC PATCH v3] automation: add linker symbol name script
Posted by Jan Beulich 1 month, 1 week ago
On 25.07.2024 21:01, victorm.lira@amd.com wrote:
> From: Victor Lira <victorm.lira@amd.com>
> 
> Requested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Victor Lira <victorm.lira@amd.com>

Looks okay to me now, just that I don't see ...

> --- /dev/null
> +++ b/automation/eclair_analysis/linker-symbols.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +# 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}/../.."
> +
> +fatal() {
> +  echo "${script_name}: $*" >&2
> +  exit 1
> +}
> +
> +usage() {
> +  fatal "Usage: ${script_name} <arm|x86>"
> +}
> +
> +if [ $# -ne 1 ]; then
> +  usage
> +fi
> +
> +filepath="${src_dir}/xen/arch/${1}/xen.lds"
> +
> +if [ ! -f "$filepath" ]; then
> +  fatal "Could not find ${1} linker script. Must be run after arm/x86 build."

... why you have "arm/x86" there when the message already includes ${1}.
That'll simply go stale (and unnoticed) when PPC and/or RISC-V make further
progress. Actually in usage() I'm similarly uncertain you want to mention
the two architectures explicitly. Just say <arch> there? Happy to make
adjustments while committing, so long as you agree.

Jan
Re: [RFC PATCH v3] automation: add linker symbol name script
Posted by Stefano Stabellini 1 week, 2 days ago
On Fri, 26 Jul 2024, Jan Beulich wrote:
> On 25.07.2024 21:01, victorm.lira@amd.com wrote:
> > From: Victor Lira <victorm.lira@amd.com>
> > 
> > Requested-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Victor Lira <victorm.lira@amd.com>
> 
> Looks okay to me now, just that I don't see ...
> 
> > --- /dev/null
> > +++ b/automation/eclair_analysis/linker-symbols.sh
> > @@ -0,0 +1,34 @@
> > +#!/bin/sh
> > +
> > +# 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}/../.."
> > +
> > +fatal() {
> > +  echo "${script_name}: $*" >&2
> > +  exit 1
> > +}
> > +
> > +usage() {
> > +  fatal "Usage: ${script_name} <arm|x86>"
> > +}
> > +
> > +if [ $# -ne 1 ]; then
> > +  usage
> > +fi
> > +
> > +filepath="${src_dir}/xen/arch/${1}/xen.lds"
> > +
> > +if [ ! -f "$filepath" ]; then
> > +  fatal "Could not find ${1} linker script. Must be run after arm/x86 build."
> 
> ... why you have "arm/x86" there when the message already includes ${1}.
> That'll simply go stale (and unnoticed) when PPC and/or RISC-V make further
> progress. Actually in usage() I'm similarly uncertain you want to mention
> the two architectures explicitly. Just say <arch> there? Happy to make
> adjustments while committing, so long as you agree.

I made the suggested changes on commit adding my R-b
Re: [RFC PATCH v3] automation: add linker symbol name script
Posted by Lira, Victor M 1 month, 1 week ago
On 7/26/2024 12:44 AM, Jan Beulich wrote:
>> +  fatal "Could not find ${1} linker script. Must be run after arm/x86 build."
> ... why you have "arm/x86" there when the message already includes ${1}.
> That'll simply go stale (and unnoticed) when PPC and/or RISC-V make further
> progress. Actually in usage() I'm similarly uncertain you want to mention
> the two architectures explicitly. Just say <arch> there? Happy to make
> adjustments while committing, so long as you agree.
>
> Jan

OK, I see your point and I agree with both changes.

Victor
Re: [RFC PATCH v3] automation: add linker symbol name script
Posted by Nicola Vetrini 1 month, 2 weeks ago
On 2024-07-25 21:01, victorm.lira@amd.com wrote:
> From: Victor Lira <victorm.lira@amd.com>
> 
> Requested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Victor Lira <victorm.lira@amd.com>
> ---
> Notes:
> This is a utilty script for help with the MISRA process.
> This script matches all linker symbol names in linker script files for
> arm or x86.
> Not included are symbol names starting with "." or symbol names 
> enclosed
> in quotes since the files dont't use any. The regular expression also 
> does
> not match for "&=" and similar compound assignments.
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: roberto.bagnara@bugseng.com
> Cc: consulting@bugseng.com
> Cc: simone.ballarin@bugseng.com
> ---
> Changes v2:
> - address style comments
> - updated script to use .lds instead of .lds.S
> - remove sample output from patch
> 
> Changes v3:
> - use #!/bin/sh
> - update error handling and message similar to ../build.sh
> ---
>  automation/eclair_analysis/linker-symbols.sh | 34 ++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100755 automation/eclair_analysis/linker-symbols.sh
> 
> diff --git a/automation/eclair_analysis/linker-symbols.sh 
> b/automation/eclair_analysis/linker-symbols.sh
> new file mode 100755
> index 0000000000..61790fb281
> --- /dev/null
> +++ b/automation/eclair_analysis/linker-symbols.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +# 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}/../.."
> +
> +fatal() {
> +  echo "${script_name}: $*" >&2
> +  exit 1
> +}
> +
> +usage() {
> +  fatal "Usage: ${script_name} <arm|x86>"
> +}
> +
> +if [ $# -ne 1 ]; then
> +  usage
> +fi
> +
> +filepath="${src_dir}/xen/arch/${1}/xen.lds"
> +
> +if [ ! -f "$filepath" ]; then
> +  fatal "Could not find ${1} linker script. Must be run after arm/x86 
> build."
> +fi
> +

A doubt I came across now: since this script must be run after the build 
(and hence the analysis), but the configuration must be generated before 
the analysis, the only way this could work in my opinion is this:

- a build without analysis is performed, just enough to build xen.lds 
(maybe there is a specific Makefile target to do this)
- generate the configuration, then clean everything and then run the 
analysis

> +sed -n "s/^\s*\([a-zA-Z_][a-zA-Z_0-9.\-]*\)\s*=.*;.*$/\1/p" 
> "$filepath"
> --
> 2.25.1

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [RFC PATCH v3] automation: add linker symbol name script
Posted by Stefano Stabellini 1 month, 2 weeks ago
On Thu, 25 Jul 2024, Nicola Vetrini wrote:
> On 2024-07-25 21:01, victorm.lira@amd.com wrote:
> > From: Victor Lira <victorm.lira@amd.com>
> > 
> > Requested-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Victor Lira <victorm.lira@amd.com>
> > ---
> > Notes:
> > This is a utilty script for help with the MISRA process.
> > This script matches all linker symbol names in linker script files for
> > arm or x86.
> > Not included are symbol names starting with "." or symbol names enclosed
> > in quotes since the files dont't use any. The regular expression also does
> > not match for "&=" and similar compound assignments.
> > ---
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: roberto.bagnara@bugseng.com
> > Cc: consulting@bugseng.com
> > Cc: simone.ballarin@bugseng.com
> > ---
> > Changes v2:
> > - address style comments
> > - updated script to use .lds instead of .lds.S
> > - remove sample output from patch
> > 
> > Changes v3:
> > - use #!/bin/sh
> > - update error handling and message similar to ../build.sh
> > ---
> >  automation/eclair_analysis/linker-symbols.sh | 34 ++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> >  create mode 100755 automation/eclair_analysis/linker-symbols.sh
> > 
> > diff --git a/automation/eclair_analysis/linker-symbols.sh
> > b/automation/eclair_analysis/linker-symbols.sh
> > new file mode 100755
> > index 0000000000..61790fb281
> > --- /dev/null
> > +++ b/automation/eclair_analysis/linker-symbols.sh
> > @@ -0,0 +1,34 @@
> > +#!/bin/sh
> > +
> > +# 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}/../.."
> > +
> > +fatal() {
> > +  echo "${script_name}: $*" >&2
> > +  exit 1
> > +}
> > +
> > +usage() {
> > +  fatal "Usage: ${script_name} <arm|x86>"
> > +}
> > +
> > +if [ $# -ne 1 ]; then
> > +  usage
> > +fi
> > +
> > +filepath="${src_dir}/xen/arch/${1}/xen.lds"
> > +
> > +if [ ! -f "$filepath" ]; then
> > +  fatal "Could not find ${1} linker script. Must be run after arm/x86
> > build."
> > +fi
> > +
> 
> A doubt I came across now: since this script must be run after the build (and
> hence the analysis), but the configuration must be generated before the
> analysis, the only way this could work in my opinion is this:
> 
> - a build without analysis is performed, just enough to build xen.lds (maybe
> there is a specific Makefile target to do this)
> - generate the configuration, then clean everything and then run the analysis

Yes, that is one option and it should work. The other option is to run
this script against the xen.lds.S files instead (if it works.)


> > +sed -n "s/^\s*\([a-zA-Z_][a-zA-Z_0-9.\-]*\)\s*=.*;.*$/\1/p" "$filepath"
Re: [RFC PATCH v3] automation: add linker symbol name script
Posted by Nicola Vetrini 1 month, 1 week ago
On 2024-07-26 00:43, Stefano Stabellini wrote:
> On Thu, 25 Jul 2024, Nicola Vetrini wrote:
>> On 2024-07-25 21:01, victorm.lira@amd.com wrote:
>> > From: Victor Lira <victorm.lira@amd.com>
>> >
>> > Requested-by: Jan Beulich <jbeulich@suse.com>
>> > Signed-off-by: Victor Lira <victorm.lira@amd.com>
>> > ---
>> > Notes:
>> > This is a utilty script for help with the MISRA process.
>> > This script matches all linker symbol names in linker script files for
>> > arm or x86.
>> > Not included are symbol names starting with "." or symbol names enclosed
>> > in quotes since the files dont't use any. The regular expression also does
>> > not match for "&=" and similar compound assignments.
>> > ---
>> > Cc: Jan Beulich <jbeulich@suse.com>
>> > Cc: Stefano Stabellini <sstabellini@kernel.org>
>> > Cc: roberto.bagnara@bugseng.com
>> > Cc: consulting@bugseng.com
>> > Cc: simone.ballarin@bugseng.com
>> > ---
>> > Changes v2:
>> > - address style comments
>> > - updated script to use .lds instead of .lds.S
>> > - remove sample output from patch
>> >
>> > Changes v3:
>> > - use #!/bin/sh
>> > - update error handling and message similar to ../build.sh
>> > ---
>> >  automation/eclair_analysis/linker-symbols.sh | 34 ++++++++++++++++++++
>> >  1 file changed, 34 insertions(+)
>> >  create mode 100755 automation/eclair_analysis/linker-symbols.sh
>> >
>> > diff --git a/automation/eclair_analysis/linker-symbols.sh
>> > b/automation/eclair_analysis/linker-symbols.sh
>> > new file mode 100755
>> > index 0000000000..61790fb281
>> > --- /dev/null
>> > +++ b/automation/eclair_analysis/linker-symbols.sh
>> > @@ -0,0 +1,34 @@
>> > +#!/bin/sh
>> > +
>> > +# 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}/../.."
>> > +
>> > +fatal() {
>> > +  echo "${script_name}: $*" >&2
>> > +  exit 1
>> > +}
>> > +
>> > +usage() {
>> > +  fatal "Usage: ${script_name} <arm|x86>"
>> > +}
>> > +
>> > +if [ $# -ne 1 ]; then
>> > +  usage
>> > +fi
>> > +
>> > +filepath="${src_dir}/xen/arch/${1}/xen.lds"
>> > +
>> > +if [ ! -f "$filepath" ]; then
>> > +  fatal "Could not find ${1} linker script. Must be run after arm/x86
>> > build."
>> > +fi
>> > +
>> 
>> A doubt I came across now: since this script must be run after the 
>> build (and
>> hence the analysis), but the configuration must be generated before 
>> the
>> analysis, the only way this could work in my opinion is this:
>> 
>> - a build without analysis is performed, just enough to build xen.lds 
>> (maybe
>> there is a specific Makefile target to do this)
>> - generate the configuration, then clean everything and then run the 
>> analysis
> 
> Yes, that is one option and it should work. The other option is to run
> this script against the xen.lds.S files instead (if it works.)
> 

However, as Jan pointed out in v1, this second option may yield 
incomplete results.
The way we can notice if there's a discrepancy is by running the 
analysis with the automatically generated deviation and tell whether the 
resulting reports for Rule 18.2 there are any about linker-defined 
symbols.

> 
>> > +sed -n "s/^\s*\([a-zA-Z_][a-zA-Z_0-9.\-]*\)\s*=.*;.*$/\1/p" "$filepath"

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [RFC PATCH v3] automation: add linker symbol name script
Posted by Lira, Victor M 1 month, 2 weeks ago
Thanks all for suggestions in v2.

Do we need to worry about duplicates or alphabetically sorting/filtering 
the output here? As it stands there are no duplicates but I think it can 
happen with linker syntax.

Victor
Re: [RFC PATCH v3] automation: add linker symbol name script
Posted by Nicola Vetrini 1 month, 2 weeks ago
On 2024-07-25 21:06, Lira, Victor M wrote:
> Thanks all for suggestions in v2.
> 
> Do we need to worry about duplicates or alphabetically 
> sorting/filtering the output here? As it stands there are no duplicates 
> but I think it can happen with linker syntax.
> 
> Victor

Hi,

for the specific task it will carry out that's not a particular concern, 
as each symbol will be used to construct a single regex, but for 
readability that might be appreciated by the maintainers.

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