[PATCH 0/1] eglibc option groups fixup

[PATCH 0/1] eglibc option groups fixup

[Peter Seebach]

This is a first pass at addressing 6796, 6797, and 6809. For the
latter two, I've just adopted the suggested fixes, for the former I've
dropped an #if that I don't *think* we need. This might produce bogus
results with the locale option group disabled, but it'll work in the
normal case and reduce the number of -Wundef warnings and build problems.

The following changes since commit dd93b3a950d063f4e3020362c636006f51fb5782:

  image_types.bbclass: whitespace and reorder (2014-11-06 16:41:38 +0000)

are available in the git repository at:
  git://git.yoctoproject.org/poky-contrib seebs/glibcoptions
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=seebs/glibcoptions

Peter Seebach (1):
  eglibc-use-option-groups.patch: Several fixups

 .../glibc/glibc/eglibc-use-option-groups.patch     |   40 +------------------
 1 files changed, 3 insertions(+), 37 deletions(-)

[PATCH 1/1] eglibc-use-option-groups.patch: Several fixups

[Peter Seebach]

This incorporates suggested changes for issues 6797 and 6809 from
bugzilla, and a proposed change for 6796. For 6796, the issue is
that you can't just include <gnu/option-groups.h> in libc-symbols,
so we may not always have the option group defines available. The
workaround is to just leave the HAVE_MBSTATE_T definition visible;
the obvious relevant usage is already also conditionalized on
the option group locale define by this patch. This might not fix
all the cases if the locale group isn't enabled, but it eliminates a
pretty large raft of -Wundef warnings.

The reason HAVE_MBSTATE_T wasn't conditionalized in 2.19 is that
it didn't exist in 2.19.

Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
---
 .../glibc/glibc/eglibc-use-option-groups.patch     |   40 +------------------
 1 files changed, 3 insertions(+), 37 deletions(-)

diff --git a/meta/recipes-core/glibc/glibc/eglibc-use-option-groups.patch b/meta/recipes-core/glibc/glibc/eglibc-use-option-groups.patch
index 40c7832..e4a0502 100644
--- a/meta/recipes-core/glibc/glibc/eglibc-use-option-groups.patch
+++ b/meta/recipes-core/glibc/glibc/eglibc-use-option-groups.patch
@@ -2434,24 +2434,8 @@ Index: git/localedata/Makefile
  
  ifeq ($(run-built-tests),yes)
  # We have to generate locales
-@@ -143,9 +161,13 @@
- $(addprefix $(objpfx),$(CTYPE_FILES)): %: \
-   gen-locale.sh $(common-objpfx)locale/localedef Makefile \
-   $(addprefix charmaps/,$(CHARMAPS)) $(addprefix locales/,$(LOCALE_SRCS))
--	@$(SHELL) gen-locale.sh $(common-objpfx) \
--		  '$(built-program-cmd-before-env)' '$(run-program-env)' \
--		  '$(built-program-cmd-after-env)' $@; \
-+	@$(SHELL) gen-locale.sh $(common-objpfx)	\
-+		 '$(if $(cross-localedef), 		\
-+		       $(cross-localedef),		\
-+		       $(built-program-cmd-before-env)  \
-+		       $(run-program-env)		\
-+		       $(built-program-cmd-after-env))' \
-+		       $@; \
- 	$(evaluate-test)
- 
  $(addsuffix .out,$(addprefix $(objpfx),$(tests))): %: \
-@@ -213,6 +235,11 @@
+@@ -213,6 +231,11 @@
  
  include SUPPORTED
  
@@ -5065,8 +5049,8 @@ Index: git/posix/Makefile
  		 $(objpfx)bug-glob2-mem.out $(objpfx)tst-vfork3-mem.out \
  		 $(objpfx)tst-fnmatch-mem.out $(objpfx)bug-regex36-mem.out
 +ifeq (y,$(OPTION_POSIX_REGEXP_GLIBC))
-+tests-special += $(objpfx)bug-regex14-mem $(objpfx)tst-rxspencer-no-utf8-mem \
-+  		 $(objpfx)tst-pcre-mem $(objpfx)tst-boost-mem
++tests-special += $(objpfx)bug-regex14-mem.out $(objpfx)tst-rxspencer-no-utf8-mem.out \
++  		 $(objpfx)tst-pcre-mem $(objpfx)tst-boost-mem.out
 +endif
 +
  xtests-special += $(objpfx)bug-ga2-mem.out
@@ -16458,24 +16442,6 @@ Index: git/sysdeps/nptl/small-macros-fns.c
 +libc_hidden_def (__libc_lock_unlock_recursive_fn);
 +
 +#endif /*defined _LIBC && (!defined NOT_IN_libc || defined IS_IN_libpthread)*/
-Index: git/include/libc-symbols.h
-===================================================================
---- git.orig/include/libc-symbols.h	2014-08-29 20:00:47.144070587 -0700
-+++ git/include/libc-symbols.h	2014-08-29 20:01:15.248070587 -0700
-@@ -60,8 +60,11 @@
- /* Define these macros for the benefit of portable GNU code that wants to check
-    them.  Of course, STDC_HEADERS is never false when building libc!  */
- #define STDC_HEADERS	1
--#define HAVE_MBSTATE_T	1
--#define HAVE_MBSRTOWCS	1
-+
-+#if __OPTION_EGLIBC_LOCALE_CODE
-+# define HAVE_MBSTATE_T	1
-+# define HAVE_MBSRTOWCS	1
-+#endif
- #define HAVE_LIBINTL_H	1
- #define HAVE_WCTYPE_H	1
- #define HAVE_ISWCTYPE	1
 Index: git/crypt/crypt_common.c
 ===================================================================
 --- /dev/null	1970-01-01 00:00:00.000000000 +0000
-- 
1.7.1

Re: [PATCH 1/1] eglibc-use-option-groups.patch: Several fixups

[Richard Purdie]

On Fri, 2014-11-07 at 13:59 -0600, Peter Seebach wrote:
> This incorporates suggested changes for issues 6797 and 6809 from
> bugzilla, and a proposed change for 6796. For 6796, the issue is
> that you can't just include <gnu/option-groups.h> in libc-symbols,
> so we may not always have the option group defines available. The
> workaround is to just leave the HAVE_MBSTATE_T definition visible;
> the obvious relevant usage is already also conditionalized on
> the option group locale define by this patch. This might not fix
> all the cases if the locale group isn't enabled, but it eliminates a
> pretty large raft of -Wundef warnings.
> 
> The reason HAVE_MBSTATE_T wasn't conditionalized in 2.19 is that
> it didn't exist in 2.19.
> 
> Signed-off-by: Peter Seebach <peter.seebach@windriver.com>

Whilst I haven't 100% confirmed it,
https://autobuilder.yoctoproject.org/main/builders/poky-tiny/builds/102/steps/BuildImages/logs/stdio
looks like it may well be as a result of this change (which was included
in the master-next build in question)...

Cheers,

Richard

Re: [PATCH 1/1] eglibc-use-option-groups.patch: Several fixups

[Peter Seebach]

On Sat, 8 Nov 2014 14:42:45 +0000
Richard Purdie <richard.purdie@linuxfoundation.org> wrote:

> Whilst I haven't 100% confirmed it,
> https://autobuilder.yoctoproject.org/main/builders/poky-tiny/builds/102/steps/BuildImages/logs/stdio
> looks like it may well be as a result of this change (which was included
> in the master-next build in question)...

Hmm. I am *pretty* sure I was successfully building things with this patch.
But it does look suspiciously similar.

-s
-- 
Listen, get this.  Nobody with a good compiler needs to be justified.

Re: [PATCH 1/1] eglibc-use-option-groups.patch: Several fixups

[Richard Purdie]

On Sat, 2014-11-08 at 14:42 +0000, Richard Purdie wrote:
> On Fri, 2014-11-07 at 13:59 -0600, Peter Seebach wrote:
> > This incorporates suggested changes for issues 6797 and 6809 from
> > bugzilla, and a proposed change for 6796. For 6796, the issue is
> > that you can't just include <gnu/option-groups.h> in libc-symbols,
> > so we may not always have the option group defines available. The
> > workaround is to just leave the HAVE_MBSTATE_T definition visible;
> > the obvious relevant usage is already also conditionalized on
> > the option group locale define by this patch. This might not fix
> > all the cases if the locale group isn't enabled, but it eliminates a
> > pretty large raft of -Wundef warnings.
> > 
> > The reason HAVE_MBSTATE_T wasn't conditionalized in 2.19 is that
> > it didn't exist in 2.19.
> > 
> > Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
> 
> Whilst I haven't 100% confirmed it,
> https://autobuilder.yoctoproject.org/main/builders/poky-tiny/builds/102/steps/BuildImages/logs/stdio
> looks like it may well be as a result of this change (which was included
> in the master-next build in question)...

It is 100% confirmed now, with the patch reverted there was a green
build.

Cheers,

Richard

Re: [PATCH 1/1] eglibc-use-option-groups.patch: Several fixups

[Peter Seebach]

On Tue, 11 Nov 2014 10:55:24 +0000
Richard Purdie <richard.purdie@linuxfoundation.org> wrote:

> > Whilst I haven't 100% confirmed it,
> > https://autobuilder.yoctoproject.org/main/builders/poky-tiny/builds/102/steps/BuildImages/logs/stdio
> > looks like it may well be as a result of this change (which was included
> > in the master-next build in question)...  
> 
> It is 100% confirmed now, with the patch reverted there was a green
> build.

Drat. I'll study this more and try to figure out what I missed.

-s
-- 
Listen, get this.  Nobody with a good compiler needs to be justified.

Re: [PATCH 1/1] eglibc-use-option-groups.patch: Several fixups

[Juro Bystricky]

Peter Seebach <peter.seebach@...> writes:

> 
> On Tue, 11 Nov 2014 10:55:24 +0000
> Richard Purdie <richard.purdie@...> wrote:
> 
> > > Whilst I haven't 100% confirmed it,
> > >
https://autobuilder.yoctoproject.org/main/builders/poky-tiny/builds/102/steps/...
> > > looks like it may well be as a result of this change (which was included
> > > in the master-next build in question)...  
> > 
> > It is 100% confirmed now, with the patch reverted there was a green
> > build.
> 
> Drat. I'll study this more and try to figure out what I missed.
> 
> -s


Peter, I noticed you have another patch that fixes this issue:

@@ -16469,7 +16452,7 @@ Index: git/include/libc-symbols.h
-#define HAVE_MBSTATE_T 1
-#define HAVE_MBSRTOWCS 1
+
-+#if __OPTION_EGLIBC_LOCALE_CODE
++#if defined(__OPTION_EGLIBC_LOCALE_CODE) && __OPTION_EGLIBC_LOCALE_CODE
+# define HAVE_MBSTATE_T 1
+# define HAVE_MBSRTOWCS 1
+#endif

This will fix the the issue, but (please correct me if I'm wrong) 
the way I see it, since__OPTION_EGLIBC_LOCALE_CODE is defined in 
gnu/option-groups.h, which is NOT included libc-symbols.h and hence
 
	#if defined(__OPTION_EGLIBC_LOCALE_CODE) 
 
will always evaluate as false and HAVE_MBSTATE_T will be never defined.

I am not sure why the HAVE_MBSTATE_T needs to be conditioned at all, my
understanding is
it is defined if the file <wchar.h> contains type definition "mbstate_t".
This typedef either exists or not and saying it not exists when in fact 
it does might be confusing (as it already is for me) and I don't see any 
relation with __OPTION_EGLIBC_LOCALE_CODE.


The only place in glibc I could find that uses HAVE_MBSTATE_T is in fnmatch.c:

#if defined _LIBC
# include <gnu/option-groups.h>
#endif

#  if (HAVE_MBSTATE_T && HAVE_MBSRTOWCS) || (_LIBC &&
__OPTION_EGLIBC_LOCALE_CODE)
#   define HANDLE_MULTIBYTE    1
#  endif

To me it seems it is actually HANDLE_MULTIBYTE that we want conditioned 
with a new patch, resulting in something like:

#if defined _LIBC
# include <gnu/option-groups.h>
#endif

#  if (HAVE_MBSTATE_T && HAVE_MBSRTOWCS && _LIBC && __OPTION_EGLIBC_LOCALE_CODE)
#   define HANDLE_MULTIBYTE    1
#  endif

Incidentally, I tried a core-image-minimal build with unconditioned
HAVE_MBSTATE_T and encountered no problems, just like you did originally.
This would imply poky-tiny problem is a probably a different problem.

One last unrelated thing: I believe in the patch you missed one ".out"
extension, 
I think it should be:

...
++ $(objpfx)tst-pcre-mem.out $(objpfx)tst-boost-mem.out
...

instead of 

...
++ $(objpfx)tst-pcre-mem $(objpfx)tst-boost-mem.out
...

Re: [PATCH 1/1] eglibc-use-option-groups.patch: Several fixups

[Juro Bystricky]

Peter Seebach <peter.seebach@...> writes:

> 
> On Tue, 11 Nov 2014 10:55:24 +0000
> Richard Purdie <richard.purdie@...> wrote:
> 
> > > Whilst I haven't 100% confirmed it,
> > >
https://autobuilder.yoctoproject.org/main/builders/poky-tiny/builds/102/steps
/BuildImages/logs/stdio
> > > looks like it may well be as a result of this change (which was included
> > > in the master-next build in question)...  
> > 
> > It is 100% confirmed now, with the patch reverted there was a green
> > build.
> 
> Drat. I'll study this more and try to figure out what I missed.
> 
> -s

Peter, I noticed you have another patch that fixes this issue:

@@ -16469,7 +16452,7 @@ Index: git/include/libc-symbols.h
-#define HAVE_MBSTATE_T 1
-#define HAVE_MBSRTOWCS 1
+
-+#if __OPTION_EGLIBC_LOCALE_CODE
++#if defined(__OPTION_EGLIBC_LOCALE_CODE) && __OPTION_EGLIBC_LOCALE_CODE
+# define HAVE_MBSTATE_T 1
+# define HAVE_MBSRTOWCS 1
+#endif

This will fix the the issue, but (please correct me if I'm wrong) 
the way I see it, since__OPTION_EGLIBC_LOCALE_CODE is defined in 
gnu/option-groups.h, which is NOT included libc-symbols.h and hence
 
	#if defined(__OPTION_EGLIBC_LOCALE_CODE) 
 
will always evaluate as false and HAVE_MBSTATE_T will be never defined.

I am not sure why the HAVE_MBSTATE_T needs to be conditioned at all, my
understanding is
it is defined if the file <wchar.h> contains type definition "mbstate_t".
This typedef either exists or not and saying it not exists when in fact 
it does might be confusing (as it already is for me) and I don't see any 
relation with __OPTION_EGLIBC_LOCALE_CODE.


The only place in glibc I could find that uses HAVE_MBSTATE_T is in 
fnmatch.c:

#if defined _LIBC
# include <gnu/option-groups.h>
#endif

#  if (HAVE_MBSTATE_T && HAVE_MBSRTOWCS) || (_LIBC &&
__OPTION_EGLIBC_LOCALE_CODE)
#   define HANDLE_MULTIBYTE    1
#  endif

To me it seems it is actually HANDLE_MULTIBYTE that we want conditioned 
with a new patch, resulting in something like:

#if defined _LIBC
# include <gnu/option-groups.h>
#endif

#if (HAVE_MBSTATE_T && HAVE_MBSRTOWCS)
#  if _LIBC 
#    if __OPTION_EGLIBC_LOCALE_CODE
#      define HANDLE_MULTIBYTE    1
#    endif
#  else 
#    define HANDLE_MULTIBYTE    1
#  endif
#endif

Incidentally, I tried a core-image-minimal build with unconditioned
HAVE_MBSTATE_T and encountered no problems, just like you did originally.
This would imply poky-tiny problem is a probably a different problem.
(The suggested patch in fnmatch.c could possibly fix it as well)

One last unrelated thing: I believe in the patch you missed one ".out"
extension, 
I think it should be:

...
++ $(objpfx)tst-pcre-mem.out $(objpfx)tst-boost-mem.out
...

instead of 

...
++ $(objpfx)tst-pcre-mem $(objpfx)tst-boost-mem.out
...

Re: [PATCH 1/1] eglibc-use-option-groups.patch: Several fixups

[Peter Seebach]

On Wed, 19 Nov 2014 17:59:49 +0000
Juro Bystricky <jurobystricky@hotmail.com> wrote:

> I am not sure why the HAVE_MBSTATE_T needs to be conditioned at all, my
> understanding is
> it is defined if the file <wchar.h> contains type definition "mbstate_t".
> This typedef either exists or not and saying it not exists when in fact 
> it does might be confusing (as it already is for me) and I don't see any 
> relation with __OPTION_EGLIBC_LOCALE_CODE.

If it's not conditional, the build fails when the feature's not included,
because one of the test programs attempts to use it. It may be that something
else ought to be conditional to make that work. So it might be that your
suggested change to fnmatch would fix poky-tiny as well.

And you're right about the .out.

-s
-- 
Listen, get this.  Nobody with a good compiler needs to be justified.

Re: [PATCH 1/1] eglibc-use-option-groups.patch: Several fixups

[Juro Bystricky]

[-- Attachment #1: Type: text/plain, Size: 2284 bytes --]

Hello Peter, 
my concern is that if the file libc-symbols.h is patched like this:

#if defined(__OPTION_EGLIBC_LOCALE_CODE) && __OPTION_EGLIBC_LOCALE_CODE
# define HAVE_MBSTATE_T    1
# define HAVE_MBSRTOWCS    1
#endif

then it will be ALWAYS evaluated as:  

#if 0
# define HAVE_MBSTATE_T    1
# define HAVE_MBSRTOWCS    1
#endif

because

#if defined __OPTION_EGLIBC_LOCALE_CODE

will always evaluate as 0.

To test this hypothesis, I actually build core-image-minimal with
an  explicit #error:

#if defined(__OPTION_EGLIBC_LOCALE_CODE) && __OPTION_EGLIBC_LOCALE_CODE
# error "I was wrong in my assumptions!!!"
# define HAVE_MBSTATE_T    1
# define HAVE_MBSRTOWCS    1
#endif

The image built fine, (Though I did not build/ran the glibc testsuite yet, nor did I try 
to build tiny-poky). 

I believe the purpose of this patching is simply to unconditionally prevent 
suppport for mutibyte character handling if __OPTION_EGLIBC_LOCALE_CODE=0 
If this is indeed the case, then the files posix/fnmatch.c and (possibly posix/fnmatch_loop.c
may need revised patching. ).

Juro


> Date: Wed, 19 Nov 2014 14:09:33 -0600
> From: peter.seebach@windriver.com
> To: jurobystricky@hotmail.com
> CC: openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH 1/1] eglibc-use-option-groups.patch: Several fixups
> 
> On Wed, 19 Nov 2014 17:59:49 +0000
> Juro Bystricky <jurobystricky@hotmail.com> wrote:
> 
> > I am not sure why the HAVE_MBSTATE_T needs to be conditioned at all, my
> > understanding is
> > it is defined if the file <wchar.h> contains type definition "mbstate_t".
> > This typedef either exists or not and saying it not exists when in fact 
> > it does might be confusing (as it already is for me) and I don't see any 
> > relation with __OPTION_EGLIBC_LOCALE_CODE.
> 
> If it's not conditional, the build fails when the feature's not included,
> because one of the test programs attempts to use it. It may be that something
> else ought to be conditional to make that work. So it might be that your
> suggested change to fnmatch would fix poky-tiny as well.
> 
> And you're right about the .out.
> 
> -s
> -- 
> Listen, get this.  Nobody with a good compiler needs to be justified.
 		 	   		  

[-- Attachment #2: Type: text/html, Size: 2870 bytes --]