[PATCH] manual: Mark perror as MT-unsafe and update check-safety.sh

[Adhemerval Zanella Netto]

On 20/02/25 20:41, Carlos O'Donell wrote:
> The manual marked perror as MT-safe, but then listed a remark
> indicating that it was unsafe because of a race between the function
> and access to stderr.  The function is indeed MT-unsafe because
> of the unlocked access to stderr internals and bug 32730 has been
> filed to address this issue.
> 
> The script manual/check-safety.sh should have caught this issue,
> but a missed escaping of "?" along with searching of all inputs
> again via "$@" resulted in a non-functional regexp.
> 
> In order to avoid regressions we also update check-safety.sh.
> 
> The script manual/check-safety.sh is updated in the following ways:
> 
>  * The MT-unsafe remarks in MT-safe context check is fixed.
>    - It now detects the perror safety note mistake.
> 
>  * Comments updated indicating that we allow MT context marks
>    to count for other contexts if they are related.
>    - This is why commit ad9c4c536115ba38be3e63592a632709ec8209b4
>      failed and the failure is now understood as expected.
> 
>  * All checks now have verbose output.
> 
>  * Back reference based duplicate checks are removed.
>    - They are too complex and don't cover all cases.
> 
> No regressions on x86_64.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>


> ---
>  manual/check-safety.sh | 84 ++++++++++++++++++++++++++++++------------
>  manual/errno.texi      |  2 +-
>  2 files changed, 61 insertions(+), 25 deletions(-)
> 
> diff --git a/manual/check-safety.sh b/manual/check-safety.sh
> index f0654a73fe..c758c468a1 100644
> --- a/manual/check-safety.sh
> +++ b/manual/check-safety.sh
> @@ -22,7 +22,7 @@
>  # in proper order (mt then as then ac), that remarks appear within
>  # corresponding sections (mt within mt, etc), that unsafety always has
>  # an explicit reason and when there's a reason for unsafety it's not
> -# safe, and that there aren't duplicates remarks.
> +# safe.
>  
>  
>  success=:
> @@ -37,10 +37,15 @@ test $# != 0 || set *.texi
>  
>  # Check that all safety remarks have entries for all of MT, AS and AC,
>  # in this order, with an optional prelim note before them.
> +ret=true
>  grep -n '^@safety' "$@" |
>  grep -v ':@safety{\(@prelim{}\)\?@mt\(un\)\?safe{.*}'\
>  '@as\(un\)\?safe{.*}@ac\(un\)\?safe{.*}}' &&
> -success=false
> +ret=false
> +if ! $ret; then
> +    echo "FAIL: #1 - Missing context in @safety macro."
> +    success=false
> +fi
>  
>  # Check that @mt-started notes appear within @mtsafe or @mtunsafe,
>  # that @as-started notes appear within @assafe or @asunsafe, and that
> @@ -49,6 +54,7 @@ success=false
>  # unsafe), but let @mt have as, ac or asc before [su], and let @as
>  # have a c (for cancel) before [su].  Also make sure blanks separate
>  # each of the annotations.
> +ret=true
>  grep -n '^@safety' "$@" |
>  grep -v ':@safety{\(@prelim{}\)\?'\
>  '@mt\(un\)\?safe{\(@mt\(asc\?\|ac\)\?[su][^ ]*}\)\?'\
> @@ -57,68 +63,98 @@ grep -v ':@safety{\(@prelim{}\)\?'\
>  '\( @asc\?[su][^ ]*}\)*}'\
>  '@ac\(un\)\?safe{\(@ac[su][^ ]*}\)\?'\
>  '\( @ac[su][^ ]*}\)*}}' &&
> -success=false
> +ret=false
> +if ! $ret; then
> +    echo "FAIL: #2 - Incorrect @safety macro formatting."
> +    success=false
> +fi
>  
>  # Make sure safety lines marked as @mtsafe do not contain any
>  # MT-Unsafe remark; that would be @mtu, but there could be as, ac or
>  # asc between mt and u.
> +ret=true
>  grep -n '^@safety.*@mtsafe' "$@" |
> -grep '@mt\(asc\?\|ac\)?u' "$@" &&
> -success=false
> +grep '@mt\(asc\?\|ac\)\?u' &&
> +ret=false
> +if ! $ret; then
> +    echo "FAIL: #3 - MT context contains remarks when it should not."
> +    success=false
> +fi
>  
>  # Make sure @mtunsafe lines contain at least one @mtu remark (with
>  # optional as, ac or asc between mt and u).
> +ret=true
>  grep -n '^@safety.*@mtunsafe' "$@" |
>  grep -v '@mtunsafe{.*@mt\(asc\?\|ac\)\?u' &&
> -success=false
> +ret=false
> +if ! $ret; then
> +    echo "FAIL: #4 - MT context lacks at least one remark."
> +    success=false
> +fi
>  
>  # Make sure safety lines marked as @assafe do not contain any AS-Unsafe
>  # remark, which could be @asu or @mtasu note (with an optional c
>  # between as and u in both cases).
> +ret=true
>  grep -n '^@safety.*@assafe' "$@" |
>  grep '@\(mt\)\?asc\?u' &&
> -success=false
> +ret=false
> +if ! $ret; then
> +    echo "FAIL: #5 - AS context contains remarks when it should not."
> +    success=false
> +fi
>  
>  # Make sure @asunsafe lines contain at least one @asu remark (which
> -# could be @ascu, or @mtasu or even @mtascu).
> +# could be @ascu, or @mtasu or even @mtascu).  We allow the AS safety
> +# remark from an earlier MT-unsafe to count towards such a remark
> +# e.g. @mtunsafe{@mtasurace{:LogMask}}@asunsafe{}@acsafe{} is allowed.
> +ret=true
>  grep -n '^@safety.*@asunsafe' "$@" |
>  grep -v '@mtasc\?u.*@asunsafe\|@asunsafe{.*@asc\?u' &&
> -success=false
> +ret=false
> +if ! $ret; then
> +    echo "FAIL: #6 - AS unsafe context lacks at least one remark."
> +    success=false
> +fi
>  
>  # Make sure safety lines marked as @acsafe do not contain any
>  # AC-Unsafe remark, which could be @acu, @ascu or even @mtacu or
>  # @mtascu.
> +ret=true
>  grep -n '^@safety.*@acsafe' "$@" |
>  grep '@\(mt\)\?as\?cu' &&
> -success=false
> +ret=false
> +if ! $ret; then
> +    echo "FAIL: #7 - AC context contains remarks when it should not."
> +    success=false
> +fi
>  
>  # Make sure @acunsafe lines contain at least one @acu remark (possibly
>  # implied by @ascu, @mtacu or @mtascu).
> +ret=true
>  grep -n '^@safety.*@acunsafe' "$@" |
>  grep -v '@\(mtas\?\|as\)cu.*@acunsafe\|@acunsafe{.*@acu' &&
> -success=false
> -
> -# Make sure there aren't duplicate remarks in the same safety note.
> -grep -n '^@safety' "$@" |
> -grep '[^:]\(@\(mt\|a[sc]\)[^ {]*{[^ ]*}\).*[^:]\1' &&
> -success=false
> +ret=false
> +if ! $ret; then
> +    echo "FAIL: #8 - AC unsafe context lacks at least one remark."
> +    success=false
> +fi
>  
>  # Check that comments containing safety remarks do not contain {}s,
>  # that all @mt remarks appear before @as remarks, that in turn appear
>  # before @ac remarks, all properly blank-separated, and that an
>  # optional comment about exclusions is between []s at the end of the
>  # line.
> +ret=true
>  grep -n '^@c \+[^@ ]\+\( dup\)\?'\
>  '\( @\(mt\|a[sc]\)[^ ]*\)*\( \[.*]\)\?$' "$@" |
>  grep -v ':@c *[^@{}]*\( @mt[^ {}]*\)*'\
>  '\( @as[^ {}]*\)*\( @ac[^ {}]*\)*\( \[.*]\)\?$' &&
> -success=false
> -
> -# Check that comments containing safety remarks do not contain
> -# duplicate remarks.
> -grep -n '^@c \+[^@ ]\+\( dup\)\?'\
> -'\( @\(mt\|a[sc]\)[^ ]*\)*\( \[.*]\)\?$' "$@" |
> -grep '[^:]\(@\(mt\|a[sc]\)[^ ]*\) \(.*[^:]\)\?\1\($\| \)' &&
> -success=false
> +ret=false
> +if ! $ret; then
> +    echo "FAIL: #9 - Comment safety note ordering, brackets, or"\
> +	 "exclusions are incorrect."
> +    success=false
> +fi
>  
>  $success

Ok, this is a nice improvement to know exactly what has failed.

> diff --git a/manual/errno.texi b/manual/errno.texi
> index ce07806bf5..bb8a6cb4e4 100644
> --- a/manual/errno.texi
> +++ b/manual/errno.texi
> @@ -1256,7 +1256,7 @@ It is a POSIX extension.
>  
>  @deftypefun void perror (const char *@var{message})
>  @standards{ISO, stdio.h}
> -@safety{@prelim{}@mtsafe{@mtasurace{:stderr}}@asunsafe{@asucorrupt{} @ascuintl{} @ascuheap{} @asulock{}}@acunsafe{@acucorrupt{} @aculock{} @acsmem{} @acsfd{}}}
> +@safety{@prelim{}@mtunsafe{@mtasurace{:stderr}}@asunsafe{@asucorrupt{} @ascuintl{} @ascuheap{} @asulock{}}@acunsafe{@acucorrupt{} @aculock{} @acsmem{} @acsfd{}}}
>  @c Besides strerror_r's and some of fprintf's issues, if stderr is not
>  @c oriented yet, create a new stream with a dup of stderr's fd and write
>  @c to that instead of stderr, to avoid orienting it.

Ok.