Re: [PATCH v4] kbuild: rewrite check-local-export in sh/awk

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 9/6/22 02:28, Masahiro Yamada wrote:
> On Sun, Sep 4, 2022 at 1:01 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
>>
>> On Thu, Sep 1, 2022 at 6:03 AM Owen Rafferty <owen@xxxxxxxxxxxxxxxx> wrote:
>>>
>>> Signed-off-by: Owen Rafferty <owen@xxxxxxxxxxxxxxxx>
>>> ---
>>
>>
>> Please input something in the commit log.
>>
>> I think the benchmark in v2 is worth mentioning
>> because "awk is faster than bash" is one benefit
>> of applying this patch.
>>
>>
> 
> 
> 
> Applied to linux-kbuild. Thanks.
> 
> 
> (V5 was not delivered to ML somehow,
> but I found it in my mailbox.)

Yeah, I haven't seen that one either.

For whatever is in linux-next-20220927, I am seeing something
unpleasant. I'm not positive that it's due to this patch, so I'm
still checking/testing (but I'm about to leave home for awhile so
I wanted to go ahead and let people know about this).

I do N number of randconfig builds in a script (say 10).
What I am seeing is that when an 'nm' error happens, the
script is Terminated and not continued. E.g., if the error
is on randconfig build #4, builds 5-10 are never started.
The controlling script dies.


>>>  scripts/check-local-export | 96 +++++++++++++++++++-------------------
>>>  1 file changed, 47 insertions(+), 49 deletions(-)
>>>
>>> diff --git a/scripts/check-local-export b/scripts/check-local-export
>>> index 6ccc2f467416..0c049ff44aca 100755
>>> --- a/scripts/check-local-export
>>> +++ b/scripts/check-local-export
>>> @@ -1,26 +1,14 @@
>>> -#!/usr/bin/env bash
>>> +#!/bin/sh
>>>  # SPDX-License-Identifier: GPL-2.0-only
>>>  #
>>>  # Copyright (C) 2022 Masahiro Yamada <masahiroy@xxxxxxxxxx>
>>> +# Copyright (C) 2022 Owen Rafferty <owen@xxxxxxxxxxxxxxxx>
>>>  #
>>>  # Exit with error if a local exported symbol is found.
>>>  # EXPORT_SYMBOL should be used for global symbols.
>>>
>>>  set -e
>>>
>>> -# catch errors from ${NM}
>>> -set -o pipefail
>>> -
>>> -# Run the last element of a pipeline in the current shell.
>>> -# Without this, the while-loop would be executed in a subshell, and
>>> -# the changes made to 'symbol_types' and 'export_symbols' would be lost.
>>> -shopt -s lastpipe
>>> -
>>> -declare -A symbol_types
>>> -declare -a export_symbols
>>> -
>>> -exit_code=0
>>> -
>>>  # If there is no symbol in the object, ${NM} (both GNU nm and llvm-nm) shows
>>>  # 'no symbols' diagnostic (but exits with 0). It is harmless and hidden by
>>>  # '2>/dev/null'. However, it suppresses real error messages as well. Add a
>>> @@ -29,43 +17,53 @@ exit_code=0
>>>  # TODO:
>>>  # Use --quiet instead of 2>/dev/null when we upgrade the minimum version of
>>>  # binutils to 2.37, llvm to 13.0.0.
>>> -# Then, the following line will be really simple:
>>> -#   ${NM} --quiet ${1} |
>>> +# Then, the following line will be simpler:
>>> +#   { ${NM} --quiet ${1} || kill 0; } |
>>> +
>>> +{ ${NM} ${1} 2>/dev/null || { echo "${0}: ${NM} failed" >&2; kill 0; } } |
>>> +${AWK} -v "file=${1}" '
>>> +BEGIN {
>>> +       i = 0
>>> +}
>>> +
>>> +# Skip the line if the number of fields is less than 3.
>>> +#
>>> +# case 1)
>>> +#   For undefined symbols, the first field (value) is empty.
>>> +#   The outout looks like this:
>>> +#     "                 U _printk"
>>> +#   It is unneeded to record undefined symbols.
>>> +#
>>> +# case 2)
>>> +#   For Clang LTO, llvm-nm outputs a line with type t but empty name:
>>> +#     "---------------- t"
>>> +!length($3) {
>>> +       next
>>> +}
>>>
>>> -{ ${NM} ${1} 2>/dev/null || { echo "${0}: ${NM} failed" >&2; false; } } |
>>> -while read value type name
>>> -do
>>> -       # Skip the line if the number of fields is less than 3.
>>> -       #
>>> -       # case 1)
>>> -       #   For undefined symbols, the first field (value) is empty.
>>> -       #   The outout looks like this:
>>> -       #     "                 U _printk"
>>> -       #   It is unneeded to record undefined symbols.
>>> -       #
>>> -       # case 2)
>>> -       #   For Clang LTO, llvm-nm outputs a line with type 't' but empty name:
>>> -       #     "---------------- t"
>>> -       if [[ -z ${name} ]]; then
>>> -               continue
>>> -       fi
>>> +# save (name, type) in the associative array
>>> +{ symbol_types[$3]=$2 }
>>>
>>> -       # save (name, type) in the associative array
>>> -       symbol_types[${name}]=${type}
>>> +# append the exported symbol to the array
>>> +($3 ~ /^__ksymtab_/) {
>>> +       export_symbols[i] = $3
>>> +       sub(/^__ksymtab_/, "", export_symbols[i])
>>> +       i++
>>> +}
>>>
>>> -       # append the exported symbol to the array
>>> -       if [[ ${name} == __ksymtab_* ]]; then
>>> -               export_symbols+=(${name#__ksymtab_})
>>> -       fi
>>> -done
>>> +END {
>>> +       exit_code = 0
>>> +       for (j = 0; j < i; ++j) {
>>> +               name = export_symbols[j]
>>> +               # nm(3) says "If lowercase, the symbol is usually local"
>>> +               if (symbol_types[name] ~ /[a-z]/) {
>>> +                       printf "%s: error: local symbol %s was exported\n",
>>> +                               file, name | "cat 1>&2"
>>> +                       exit_code = 1
>>> +               }
>>> +       }
>>>
>>> -for name in "${export_symbols[@]}"
>>> -do
>>> -       # nm(3) says "If lowercase, the symbol is usually local"
>>> -       if [[ ${symbol_types[$name]} =~ [a-z] ]]; then
>>> -               echo "$@: error: local symbol '${name}' was exported" >&2
>>> -               exit_code=1
>>> -       fi
>>> -done
>>> +       exit exit_code
>>> +}'
>>>
>>> -exit ${exit_code}
>>> +exit $?
>>> --
>>> 2.37.3

Thanks.

-- 
~Randy



[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux