[v2,2/2] buildtools: check symbol maps in developer mode
Checks
Commit Message
Hook check-symbol-maps.sh in the symbol check when in developer mode to
help developers catch issues before submitting their changes.
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v1:
- now that we have the developer mode, added this check in the build
process,
---
buildtools/check-symbols.sh | 9 ++++++++-
devtools/check-symbol-maps.sh | 12 ++++++++----
2 files changed, 16 insertions(+), 5 deletions(-)
Comments
On 05/05/2021 15:30, David Marchand wrote:
> Hook check-symbol-maps.sh in the symbol check when in developer mode to
> help developers catch issues before submitting their changes.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changes since v1:
> - now that we have the developer mode, added this check in the build
> process,
>
> ---
> buildtools/check-symbols.sh | 9 ++++++++-
> devtools/check-symbol-maps.sh | 12 ++++++++----
> 2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/buildtools/check-symbols.sh b/buildtools/check-symbols.sh
> index 83b3a0182f..e458c0af72 100755
> --- a/buildtools/check-symbols.sh
> +++ b/buildtools/check-symbols.sh
> @@ -5,7 +5,9 @@
> MAPFILE=$1
> OBJFILE=$2
>
> -LIST_SYMBOL=$(dirname $(readlink -f $0))/map-list-symbol.sh
> +ROOTDIR=$(readlink -f $(dirname $(readlink -f $0))/..)
> +LIST_SYMBOL=$ROOTDIR/buildtools/map-list-symbol.sh
> +CHECK_SYMBOL_MAPS=$ROOTDIR/devtools/check-symbol-maps.sh
>
> # added check for "make -C test/" usage
> if [ ! -e $MAPFILE ] || [ ! -f $OBJFILE ]
> @@ -23,6 +25,11 @@ trap 'rm -f "$DUMPFILE"' EXIT
> objdump -t $OBJFILE >$DUMPFILE
>
> ret=0
> +
> +if ! $CHECK_SYMBOL_MAPS $MAPFILE; then
> + ret=1
> +fi
> +
> for SYM in `$LIST_SYMBOL -S EXPERIMENTAL $MAPFILE |cut -d ' ' -f 3`
> do
> if grep -q "\.text.*[[:space:]]$SYM$" $DUMPFILE &&
> diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh
> index f2c7d29663..c52e28bd69 100755
> --- a/devtools/check-symbol-maps.sh
> +++ b/devtools/check-symbol-maps.sh
> @@ -7,11 +7,15 @@ cd $(dirname $0)/..
> # speed up by ignoring Unicode details
> export LC_ALL=C
>
> +if [ $# = 0 ] ; then
> + set -- $(find lib drivers -name '*.map')
> +fi
> +
> ret=0
>
> find_orphan_symbols ()
> {
> - for map in $(find lib drivers -name '*.map') ; do
> + for map in $@ ; do
> for sym in $(sed -rn 's,^([^}]*_.*);.*$,\1,p' $map) ; do
> if echo $sym | grep -q '^per_lcore_' ; then
> symsrc=${sym#per_lcore_}
> @@ -28,7 +32,7 @@ find_orphan_symbols ()
> done
> }
>
> -orphan_symbols=$(find_orphan_symbols)
> +orphan_symbols=$(find_orphan_symbols $@)
> if [ -n "$orphan_symbols" ] ; then
> echo "Found only in symbol map file:"
> echo "$orphan_symbols" | sed 's,^,\t,'
> @@ -37,13 +41,13 @@ fi
>
> find_duplicate_symbols ()
> {
> - for map in $(find lib drivers -name '*.map') ; do
> + for map in $@ ; do
> buildtools/map-list-symbol.sh $map | \
> sort | uniq -c | grep -v " 1 $map" || true
> done
> }
>
> -duplicate_symbols=$(find_duplicate_symbols)
> +duplicate_symbols=$(find_duplicate_symbols $@)
> if [ -n "$duplicate_symbols" ] ; then
> echo "Found duplicates in symbol map file:"
> echo "$duplicate_symbols"
>
+1
Reviewed-by: Ray Kinsella <mdr@ashroe.eu>
> > Hook check-symbol-maps.sh in the symbol check when in developer mode to
> > help developers catch issues before submitting their changes.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> +1
>
> Reviewed-by: Ray Kinsella <mdr@ashroe.eu>
Series applied, thanks
12/05/2021 22:24, Thomas Monjalon:
> > > Hook check-symbol-maps.sh in the symbol check when in developer mode to
> > > help developers catch issues before submitting their changes.
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > +1
> >
> > Reviewed-by: Ray Kinsella <mdr@ashroe.eu>
>
> Series applied, thanks
Oops, sorry no it cannot be merged as-is because of an incompatibility
with Busybox grep: unrecognized option: exclude=version.map
I think it is because of the old function find_orphan_symbols()
using the option --exclude.
On Wed, May 12, 2021 at 10:38 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 12/05/2021 22:24, Thomas Monjalon:
> > > > Hook check-symbol-maps.sh in the symbol check when in developer mode to
> > > > help developers catch issues before submitting their changes.
> > > >
> > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > +1
> > >
> > > Reviewed-by: Ray Kinsella <mdr@ashroe.eu>
> >
> > Series applied, thanks
>
> Oops, sorry no it cannot be merged as-is because of an incompatibility
> with Busybox grep: unrecognized option: exclude=version.map
>
> I think it is because of the old function find_orphan_symbols()
> using the option --exclude.
This could be fixed with:
- if ! grep -q -r --exclude=$(basename $map) \
- -w $symsrc $(dirname $map) ; then
+ if [ -z "$(grep -rlw $symsrc $(dirname $map) |grep -v
$map)" ] ; then
Do you want a v3 with this change?
@@ -5,7 +5,9 @@
MAPFILE=$1
OBJFILE=$2
-LIST_SYMBOL=$(dirname $(readlink -f $0))/map-list-symbol.sh
+ROOTDIR=$(readlink -f $(dirname $(readlink -f $0))/..)
+LIST_SYMBOL=$ROOTDIR/buildtools/map-list-symbol.sh
+CHECK_SYMBOL_MAPS=$ROOTDIR/devtools/check-symbol-maps.sh
# added check for "make -C test/" usage
if [ ! -e $MAPFILE ] || [ ! -f $OBJFILE ]
@@ -23,6 +25,11 @@ trap 'rm -f "$DUMPFILE"' EXIT
objdump -t $OBJFILE >$DUMPFILE
ret=0
+
+if ! $CHECK_SYMBOL_MAPS $MAPFILE; then
+ ret=1
+fi
+
for SYM in `$LIST_SYMBOL -S EXPERIMENTAL $MAPFILE |cut -d ' ' -f 3`
do
if grep -q "\.text.*[[:space:]]$SYM$" $DUMPFILE &&
@@ -7,11 +7,15 @@ cd $(dirname $0)/..
# speed up by ignoring Unicode details
export LC_ALL=C
+if [ $# = 0 ] ; then
+ set -- $(find lib drivers -name '*.map')
+fi
+
ret=0
find_orphan_symbols ()
{
- for map in $(find lib drivers -name '*.map') ; do
+ for map in $@ ; do
for sym in $(sed -rn 's,^([^}]*_.*);.*$,\1,p' $map) ; do
if echo $sym | grep -q '^per_lcore_' ; then
symsrc=${sym#per_lcore_}
@@ -28,7 +32,7 @@ find_orphan_symbols ()
done
}
-orphan_symbols=$(find_orphan_symbols)
+orphan_symbols=$(find_orphan_symbols $@)
if [ -n "$orphan_symbols" ] ; then
echo "Found only in symbol map file:"
echo "$orphan_symbols" | sed 's,^,\t,'
@@ -37,13 +41,13 @@ fi
find_duplicate_symbols ()
{
- for map in $(find lib drivers -name '*.map') ; do
+ for map in $@ ; do
buildtools/map-list-symbol.sh $map | \
sort | uniq -c | grep -v " 1 $map" || true
done
}
-duplicate_symbols=$(find_duplicate_symbols)
+duplicate_symbols=$(find_duplicate_symbols $@)
if [ -n "$duplicate_symbols" ] ; then
echo "Found duplicates in symbol map file:"
echo "$duplicate_symbols"