[PATCH] gettext: Fix overloadable error with clang

[PATCH] gettext: Fix overloadable error with clang

[Khem Raj]

Clang detects that getcwd is being re-declared and signatures don't
match, simple solution is to let clang use overloadable attribute

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 .../gettext/gettext-0.20.1/overloadable.patch | 22 +++++++++++++++++++
 meta/recipes-core/gettext/gettext_0.20.1.bb   |  1 +
 2 files changed, 23 insertions(+)
 create mode 100644 meta/recipes-core/gettext/gettext-0.20.1/overloadable.patch

diff --git a/meta/recipes-core/gettext/gettext-0.20.1/overloadable.patch b/meta/recipes-core/gettext/gettext-0.20.1/overloadable.patch
new file mode 100644
index 0000000000..1b41cb464e
--- /dev/null
+++ b/meta/recipes-core/gettext/gettext-0.20.1/overloadable.patch
@@ -0,0 +1,22 @@
+Use overloadable attribute to aid clang
+
+Fixes
+dcigettext.c:147:7: error: redeclaration of 'getcwd' must have the 'overloadable' attribute
+
+Upstream-Status: Pending
+Signed-off-by: Khem Raj <raj.khem@gmail.com>
+--- a/gettext-runtime/intl/dcigettext.c
++++ b/gettext-runtime/intl/dcigettext.c
+@@ -144,7 +144,11 @@ char *getwd ();
+ #  if VMS
+ #   define getcwd(buf, max) (getcwd) (buf, max, 0)
+ #  else
+-char *getcwd ();
++char 
++#ifdef __clang__
++__attribute__((overloadable))
++#endif
++*getcwd ();
+ #  endif
+ # endif
+ # ifndef HAVE_STPCPY
diff --git a/meta/recipes-core/gettext/gettext_0.20.1.bb b/meta/recipes-core/gettext/gettext_0.20.1.bb
index 09628bc4a3..3b576f45e9 100644
--- a/meta/recipes-core/gettext/gettext_0.20.1.bb
+++ b/meta/recipes-core/gettext/gettext_0.20.1.bb
@@ -25,6 +25,7 @@ SRC_URI = "${GNU_MIRROR}/gettext/gettext-${PV}.tar.gz \
            file://serial-tests-config.patch \
            file://0001-msgmerge-Fix-behaviour-of-for-msgfmt-on-PO-files-wit.patch \
            file://0001-tests-autopoint-3-unset-MAKEFLAGS.patch \
+           file://overloadable.patch \
            "
 
 SRC_URI[md5sum] = "bb5b0c0caa028105f3ca1905ddc306e2"
-- 
2.25.0

Re: [PATCH] gettext: Fix overloadable error with clang

[Adrian Bunk]

On Wed, Jan 15, 2020 at 08:46:09PM -0800, Khem Raj wrote:
> Clang detects that getcwd is being re-declared and signatures don't
> match, simple solution is to let clang use overloadable attribute
>...
> +Fixes
> +dcigettext.c:147:7: error: redeclaration of 'getcwd' must have the 'overloadable' attribute
>...
> +-char *getcwd ();
>...

Looks like a bug in clang to me, and should be fixed there.

The code does not tell anything regarding the parameters,
but clang seems to misinterpret it as "no parameters".

cu
Adrian

Re: [PATCH] gettext: Fix overloadable error with clang

[Khem Raj]

On Thu, Jan 16, 2020 at 5:13 AM Adrian Bunk <bunk@stusta.de> wrote:
>
> On Wed, Jan 15, 2020 at 08:46:09PM -0800, Khem Raj wrote:
> > Clang detects that getcwd is being re-declared and signatures don't
> > match, simple solution is to let clang use overloadable attribute
> >...
> > +Fixes
> > +dcigettext.c:147:7: error: redeclaration of 'getcwd' must have the 'overloadable' attribute
> >...
> > +-char *getcwd ();
> >...
>
> Looks like a bug in clang to me, and should be fixed there.
>
> The code does not tell anything regarding the parameters,
> but clang seems to misinterpret it as "no parameters".
>
its conflicting with declaration from glibc system headers

usr/include/unistd.h:extern char *getcwd (char *__buf, size_t __size)
__THROW __wur;

> cu
> Adrian

Re: [PATCH] gettext: Fix overloadable error with clang

[Adrian Bunk]

On Thu, Jan 16, 2020 at 07:17:20AM -0800, Khem Raj wrote:
> On Thu, Jan 16, 2020 at 5:13 AM Adrian Bunk <bunk@stusta.de> wrote:
> >
> > On Wed, Jan 15, 2020 at 08:46:09PM -0800, Khem Raj wrote:
> > > Clang detects that getcwd is being re-declared and signatures don't
> > > match, simple solution is to let clang use overloadable attribute
> > >...
> > > +Fixes
> > > +dcigettext.c:147:7: error: redeclaration of 'getcwd' must have the 'overloadable' attribute
> > >...
> > > +-char *getcwd ();
> > >...
> >
> > Looks like a bug in clang to me, and should be fixed there.
> >
> > The code does not tell anything regarding the parameters,
> > but clang seems to misinterpret it as "no parameters".
> >
> its conflicting with declaration from glibc system headers
>...

Why did the glibc 2.31 upgrade add a not upstreamed patch from 2017 that 
created these conflicts?

The commit message does not mention that this patch was added,
and an OE-only patch that makes a compiler reject valid C code
is not good.

cu
Adrian

Re: [PATCH] gettext: Fix overloadable error with clang

[Khem Raj]

On Tue, Jan 21, 2020 at 8:32 AM Adrian Bunk <bunk@stusta.de> wrote:
>
> On Thu, Jan 16, 2020 at 07:17:20AM -0800, Khem Raj wrote:
> > On Thu, Jan 16, 2020 at 5:13 AM Adrian Bunk <bunk@stusta.de> wrote:
> > >
> > > On Wed, Jan 15, 2020 at 08:46:09PM -0800, Khem Raj wrote:
> > > > Clang detects that getcwd is being re-declared and signatures don't
> > > > match, simple solution is to let clang use overloadable attribute
> > > >...
> > > > +Fixes
> > > > +dcigettext.c:147:7: error: redeclaration of 'getcwd' must have the 'overloadable' attribute
> > > >...
> > > > +-char *getcwd ();
> > > >...
> > >
> > > Looks like a bug in clang to me, and should be fixed there.
> > >
> > > The code does not tell anything regarding the parameters,
> > > but clang seems to misinterpret it as "no parameters".
> > >
> > its conflicting with declaration from glibc system headers
> >...
>
> Why did the glibc 2.31 upgrade add a not upstreamed patch from 2017 that
> created these conflicts?
>

This supports building userspace with clang better and find more
errors when fortify sources option is on.
this patch was already proposed to glibc and I will follow up on it.
It definitely improves fortify when using clang

> The commit message does not mention that this patch was added,
> and an OE-only patch that makes a compiler reject valid C code
> is not good.
>

I think thats my bad as it slipped my mind with numerous rebases I did
over the life of the glibc patchset.
however, I think its probably best to leave it outside core for now.
There is another patchset I will do for
glibc 2.31 where I will drop it and perhaps house it in meta-clang.

> cu
> Adrian

Re: [PATCH] gettext: Fix overloadable error with clang

[Adrian Bunk]

On Wed, Jan 22, 2020 at 12:28:02PM -0800, Khem Raj wrote:
> On Tue, Jan 21, 2020 at 8:32 AM Adrian Bunk <bunk@stusta.de> wrote:
> >
> > On Thu, Jan 16, 2020 at 07:17:20AM -0800, Khem Raj wrote:
> > > On Thu, Jan 16, 2020 at 5:13 AM Adrian Bunk <bunk@stusta.de> wrote:
> > > >
> > > > On Wed, Jan 15, 2020 at 08:46:09PM -0800, Khem Raj wrote:
> > > > > Clang detects that getcwd is being re-declared and signatures don't
> > > > > match, simple solution is to let clang use overloadable attribute
> > > > >...
> > > > > +Fixes
> > > > > +dcigettext.c:147:7: error: redeclaration of 'getcwd' must have the 'overloadable' attribute
> > > > >...
> > > > > +-char *getcwd ();
> > > > >...
> > > >
> > > > Looks like a bug in clang to me, and should be fixed there.
> > > >
> > > > The code does not tell anything regarding the parameters,
> > > > but clang seems to misinterpret it as "no parameters".
> > > >
> > > its conflicting with declaration from glibc system headers
> > >...
> >
> > Why did the glibc 2.31 upgrade add a not upstreamed patch from 2017 that
> > created these conflicts?
> 
> This supports building userspace with clang better and find more
> errors when fortify sources option is on.

What "errors" are you referring to?
From a semantic point of view the code is correct.

This is a relict from K&R C that novice C programmers often misinterpret, 
but I'd say gcc made the right call by not including the warning for the
more general case in -Wall.

The cases with an actual problem are being caught by a different gcc 
warning that has been included in -Wall for decades.

> this patch was already proposed to glibc and I will follow up on it.

This is an area with different semantics in C and C++.
All your "fixes" indicate that the result is C++ semantics in C code.
Which is not something you can do in the public glibc headers.

> It definitely improves fortify when using clang

What is the point of all this when you "fix" the bogus error caused by 
this patch with a function attribute like in this case for gettext?

The gettext patch is simply wrong.

> > The commit message does not mention that this patch was added,
> > and an OE-only patch that makes a compiler reject valid C code
> > is not good.
>
> I think thats my bad as it slipped my mind with numerous rebases I did
> over the life of the glibc patchset.
>...

Was the glibc upgrade sent to the mailing list for review?

cu
Adrian