diff mbox series

string.h: fix __fortified_attr_access macro call [BZ #29162]

Message ID 20220520150609.346566-1-slyfox@gentoo.org
State Committed
Commit 5a5f94af0542f9a35aaa7992c18eb4e2403a29b9
Delegated to: Siddhesh Poyarekar
Headers
Series string.h: fix __fortified_attr_access macro call [BZ #29162] |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Sergei Trofimovich May 20, 2022, 3:06 p.m. UTC 
  From: Sergei Trofimovich <slyich@gmail.com>

commit e938c0274 "Don't add access size hints to fortifiable functions"
converted a few '__attr_access ((...))' into '__fortified_attr_access (...)'
calls.

But one of conversions had double parentheses of '__fortified_attr_access (...)'.

Noticed as a gnat6 build failure:

    /<<NIX>>-glibc-2.34-210-dev/include/bits/string_fortified.h:110:50: error: macro "__fortified_attr_access" requires 3 arguments, but only 1 given

The change fixes parentheses.

Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
---
 string/bits/string_fortified.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Siddhesh Poyarekar May 23, 2022, 3:50 a.m. UTC | #1 
On 20/05/2022 20:36, Sergei Trofimovich via Libc-alpha wrote:
> From: Sergei Trofimovich <slyich@gmail.com>
> 
> commit e938c0274 "Don't add access size hints to fortifiable functions"
> converted a few '__attr_access ((...))' into '__fortified_attr_access (...)'
> calls.
> 
> But one of conversions had double parentheses of '__fortified_attr_access (...)'.
> 
> Noticed as a gnat6 build failure:
> 
>      /<<NIX>>-glibc-2.34-210-dev/include/bits/string_fortified.h:110:50: error: macro "__fortified_attr_access" requires 3 arguments, but only 1 given
> 
> The change fixes parentheses.
> 
> Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
> ---

Oops, LGTM, I'll push it shortly, thanks!

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

>   string/bits/string_fortified.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/string/bits/string_fortified.h b/string/bits/string_fortified.h
> index f4a5dfc2e5..149ebbb08b 100644
> --- a/string/bits/string_fortified.h
> +++ b/string/bits/string_fortified.h
> @@ -107,7 +107,7 @@ __NTH (stpncpy (char *__dest, const char *__src, size_t __n))
>   # else
>   extern char *__stpncpy_chk (char *__dest, const char *__src, size_t __n,
>   			    size_t __destlen) __THROW
> -  __fortified_attr_access ((__write_only__, 1, 3))
> +  __fortified_attr_access (__write_only__, 1, 3)
>     __attr_access ((__read_only__, 2));
>   extern char *__REDIRECT_NTH (__stpncpy_alias, (char *__dest, const char *__src,
>   					       size_t __n), stpncpy);
Sergei Trofimovich May 23, 2022, 6:54 a.m. UTC | #2 
On Mon, 23 May 2022 09:20:15 +0530
Siddhesh Poyarekar <siddhesh@sourceware.org> wrote:

> On 20/05/2022 20:36, Sergei Trofimovich via Libc-alpha wrote:
> > From: Sergei Trofimovich <slyich@gmail.com>
> > 
> > commit e938c0274 "Don't add access size hints to fortifiable functions"
> > converted a few '__attr_access ((...))' into '__fortified_attr_access (...)'
> > calls.
> > 
> > But one of conversions had double parentheses of '__fortified_attr_access (...)'.
> > 
> > Noticed as a gnat6 build failure:
> > 
> >      /<<NIX>>-glibc-2.34-210-dev/include/bits/string_fortified.h:110:50: error: macro "__fortified_attr_access" requires 3 arguments, but only 1 given
> > 
> > The change fixes parentheses.
> > 
> > Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
> > ---  
> 
> Oops, LGTM, I'll push it shortly, thanks!

Sounds good. Would it be possible to backport it to 2.34 branch as well?
I initially observed it there.

Thank you!

> Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
> 
> >   string/bits/string_fortified.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/string/bits/string_fortified.h b/string/bits/string_fortified.h
> > index f4a5dfc2e5..149ebbb08b 100644
> > --- a/string/bits/string_fortified.h
> > +++ b/string/bits/string_fortified.h
> > @@ -107,7 +107,7 @@ __NTH (stpncpy (char *__dest, const char *__src, size_t __n))
> >   # else
> >   extern char *__stpncpy_chk (char *__dest, const char *__src, size_t __n,
> >   			    size_t __destlen) __THROW
> > -  __fortified_attr_access ((__write_only__, 1, 3))
> > +  __fortified_attr_access (__write_only__, 1, 3)
> >     __attr_access ((__read_only__, 2));
> >   extern char *__REDIRECT_NTH (__stpncpy_alias, (char *__dest, const char *__src,
> >   					       size_t __n), stpncpy);  
>
Siddhesh Poyarekar May 23, 2022, 6:55 a.m. UTC | #3 
On 23/05/2022 12:24, Sergei Trofimovich wrote:
> On Mon, 23 May 2022 09:20:15 +0530
> Siddhesh Poyarekar <siddhesh@sourceware.org> wrote:
> 
>> On 20/05/2022 20:36, Sergei Trofimovich via Libc-alpha wrote:
>>> From: Sergei Trofimovich <slyich@gmail.com>
>>>
>>> commit e938c0274 "Don't add access size hints to fortifiable functions"
>>> converted a few '__attr_access ((...))' into '__fortified_attr_access (...)'
>>> calls.
>>>
>>> But one of conversions had double parentheses of '__fortified_attr_access (...)'.
>>>
>>> Noticed as a gnat6 build failure:
>>>
>>>       /<<NIX>>-glibc-2.34-210-dev/include/bits/string_fortified.h:110:50: error: macro "__fortified_attr_access" requires 3 arguments, but only 1 given
>>>
>>> The change fixes parentheses.
>>>
>>> Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
>>> ---
>>
>> Oops, LGTM, I'll push it shortly, thanks!
> 
> Sounds good. Would it be possible to backport it to 2.34 branch as well?
> I initially observed it there.

Yes, I'll be doing that.

Thanks,
Siddhesh
Siddhesh Poyarekar May 23, 2022, 7:19 a.m. UTC | #4 
On 20/05/2022 20:36, Sergei Trofimovich wrote:
> From: Sergei Trofimovich <slyich@gmail.com>
> 
> commit e938c0274 "Don't add access size hints to fortifiable functions"
> converted a few '__attr_access ((...))' into '__fortified_attr_access (...)'
> calls.
> 
> But one of conversions had double parentheses of '__fortified_attr_access (...)'.
> 
> Noticed as a gnat6 build failure:
> 
>      /<<NIX>>-glibc-2.34-210-dev/include/bits/string_fortified.h:110:50: error: macro "__fortified_attr_access" requires 3 arguments, but only 1 given
> 
> The change fixes parentheses.
> 
> Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
> ---
>   string/bits/string_fortified.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/string/bits/string_fortified.h b/string/bits/string_fortified.h
> index f4a5dfc2e5..149ebbb08b 100644
> --- a/string/bits/string_fortified.h
> +++ b/string/bits/string_fortified.h
> @@ -107,7 +107,7 @@ __NTH (stpncpy (char *__dest, const char *__src, size_t __n))
>   # else
>   extern char *__stpncpy_chk (char *__dest, const char *__src, size_t __n,
>   			    size_t __destlen) __THROW
> -  __fortified_attr_access ((__write_only__, 1, 3))
> +  __fortified_attr_access (__write_only__, 1, 3)
>     __attr_access ((__read_only__, 2));
>   extern char *__REDIRECT_NTH (__stpncpy_alias, (char *__dest, const char *__src,
>   					       size_t __n), stpncpy);

Hi Sergei, one question before I pull this in; how did this even happen? 
  The build flags don't appear to have -D_FORTIFY_SOURCE, which is 
prerequisite to including this file.  Also, that bit in the header is 
reachable only with an older compiler, e.g. older than gcc 4.7 or an 
ancient clang.

Thanks,
Siddhesh
Siddhesh Poyarekar May 23, 2022, 7:23 a.m. UTC | #5 
On 23/05/2022 12:49, Siddhesh Poyarekar via Libc-alpha wrote:
> Hi Sergei, one question before I pull this in; how did this even happen? 
>   The build flags don't appear to have -D_FORTIFY_SOURCE, which is 
> prerequisite to including this file.  Also, that bit in the header is 
> reachable only with an older compiler, e.g. older than gcc 4.7 or an 
> ancient clang.

Or a non-gcc, non-clang compiler of course.  The build log in the bug 
report appears to indicate gcc 11.3.0, which should not trip on it.

Siddhesh
Sergei Trofimovich May 23, 2022, 8:24 a.m. UTC | #6 
On Mon, 23 May 2022 12:49:54 +0530
Siddhesh Poyarekar <siddhesh@sourceware.org> wrote:

> On 20/05/2022 20:36, Sergei Trofimovich wrote:
> > From: Sergei Trofimovich <slyich@gmail.com>
> > 
> > commit e938c0274 "Don't add access size hints to fortifiable functions"
> > converted a few '__attr_access ((...))' into '__fortified_attr_access (...)'
> > calls.
> > 
> > But one of conversions had double parentheses of '__fortified_attr_access (...)'.
> > 
> > Noticed as a gnat6 build failure:
> > 
> >      /<<NIX>>-glibc-2.34-210-dev/include/bits/string_fortified.h:110:50: error: macro "__fortified_attr_access" requires 3 arguments, but only 1 given
> > 
> > The change fixes parentheses.
> > 
> > Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
> > ---
> >   string/bits/string_fortified.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/string/bits/string_fortified.h b/string/bits/string_fortified.h
> > index f4a5dfc2e5..149ebbb08b 100644
> > --- a/string/bits/string_fortified.h
> > +++ b/string/bits/string_fortified.h
> > @@ -107,7 +107,7 @@ __NTH (stpncpy (char *__dest, const char *__src, size_t __n))
> >   # else
> >   extern char *__stpncpy_chk (char *__dest, const char *__src, size_t __n,
> >   			    size_t __destlen) __THROW
> > -  __fortified_attr_access ((__write_only__, 1, 3))
> > +  __fortified_attr_access (__write_only__, 1, 3)
> >     __attr_access ((__read_only__, 2));
> >   extern char *__REDIRECT_NTH (__stpncpy_alias, (char *__dest, const char *__src,
> >   					       size_t __n), stpncpy);  
> 
> Hi Sergei, one question before I pull this in; how did this even happen? 
>   The build flags don't appear to have -D_FORTIFY_SOURCE, which is 
> prerequisite to including this file.  Also, that bit in the header is 
> reachable only with an older compiler, e.g. older than gcc 4.7 or an 
> ancient clang.

That's a great question!

NixOS / nixpkgs silently injects -D_FORTIFY_SOURCE=2 via gcc driver for vast majority
of packages even if they don't pass -D_FORTIFY_SOURCE=2 explicitly.

Also nixpkgs builds gnat6 with ... gcc-4.1.0 for historical reasons. We'll
get rid of such ancient toolchains eventually.
Siddhesh Poyarekar May 23, 2022, 8:31 a.m. UTC | #7 
On 23/05/2022 13:54, Sergei Trofimovich wrote:
> That's a great question!
> 
> NixOS / nixpkgs silently injects -D_FORTIFY_SOURCE=2 via gcc driver for vast majority
> of packages even if they don't pass -D_FORTIFY_SOURCE=2 explicitly.
> 
> Also nixpkgs builds gnat6 with ... gcc-4.1.0 for historical reasons. We'll
> get rid of such ancient toolchains eventually.

Thanks, I've pushed it with an added note.  I'll backport to 2.35 and 
2.34 too.

Siddhesh
diff mbox series

Patch

diff --git a/string/bits/string_fortified.h b/string/bits/string_fortified.h
index f4a5dfc2e5..149ebbb08b 100644
--- a/string/bits/string_fortified.h
+++ b/string/bits/string_fortified.h
@@ -107,7 +107,7 @@  __NTH (stpncpy (char *__dest, const char *__src, size_t __n))
 # else
 extern char *__stpncpy_chk (char *__dest, const char *__src, size_t __n,
 			    size_t __destlen) __THROW
-  __fortified_attr_access ((__write_only__, 1, 3))
+  __fortified_attr_access (__write_only__, 1, 3)
   __attr_access ((__read_only__, 2));
 extern char *__REDIRECT_NTH (__stpncpy_alias, (char *__dest, const char *__src,
 					       size_t __n), stpncpy);