On 4/29/19 6:18 PM, Arnaldo Carvalho de Melo wrote:
>>> Auto-detecting system features:
>>> ... dwarf: [ OFF ]
>>> ... dwarf_getlocations: [ OFF ]
>>> ... glibc: [ on ]
>> Not related to current issue, this run uses a uClibc toolchain and yet it is
>> detecting glibc - doesn't seem right to me.
> Ok, I'll improve that, I think it just tries to detect a libc, yeah,
> see:
>
> [acme@quaco linux]$ cat tools/build/feature/test-glibc.c
> // SPDX-License-Identifier: GPL-2.0
> #include <stdlib.h>
>
> #if !defined(__UCLIBC__)
> #include <gnu/libc-version.h>
> #else
> #define XSTR(s) STR(s)
> #define STR(s) #s
> #endif
>
> int main(void)
> {
> #if !defined(__UCLIBC__)
> const char *version = gnu_get_libc_version();
> #else
> const char *version = XSTR(__GLIBC__) "." XSTR(__GLIBC_MINOR__);
> #endif
>
> return (long)version;
> }
> [acme@quaco linux]$
>
> [perfbuilder@59ca4b424ded /]$ grep __GLIBC__ /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/*.h
> /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/features.h: The macros `__GNU_LIBRARY__', `__GLIBC__', and `__GLIBC_MINOR__' are
> /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/features.h:#define __GLIBC__ 2
> /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/features.h: ((__GLIBC__ << 16) + __GLIBC_MINOR__ >= ((maj) << 16) + (min))
> [perfbuilder@59ca4b424ded /]$
>
> Isn't that part of uClibc?
Right you are. Per the big fat comment right above that code, this gross hack in
uclibc is unavoidable as applications tend to rely on that define.
So a better fix would be to check for various !GLIBC libs explicitly.
#ifdef __UCLIBC__
#elseif defined __MUSL__
...
Not pretty from app usage pov, but that seems to be the only sane way of doing it.
-Vineet
On Tue, Apr 30, 2019 at 03:53:18PM +0000, Vineet Gupta wrote:
> On 4/29/19 6:18 PM, Arnaldo Carvalho de Melo wrote:
> >>> Auto-detecting system features:
> >>> ... dwarf: [ OFF ]
> >>> ... dwarf_getlocations: [ OFF ]
> >>> ... glibc: [ on ]
> >> Not related to current issue, this run uses a uClibc toolchain and yet it is
> >> detecting glibc - doesn't seem right to me.
> > Ok, I'll improve that, I think it just tries to detect a libc, yeah,
> > see:
> >
> > [acme@quaco linux]$ cat tools/build/feature/test-glibc.c
> > // SPDX-License-Identifier: GPL-2.0
> > #include <stdlib.h>
> >
> > #if !defined(__UCLIBC__)
> > #include <gnu/libc-version.h>
> > #else
> > #define XSTR(s) STR(s)
> > #define STR(s) #s
> > #endif
> >
> > int main(void)
> > {
> > #if !defined(__UCLIBC__)
> > const char *version = gnu_get_libc_version();
> > #else
> > const char *version = XSTR(__GLIBC__) "." XSTR(__GLIBC_MINOR__);
> > #endif
> >
> > return (long)version;
> > }
> > [acme@quaco linux]$
> >
> > [perfbuilder@59ca4b424ded /]$ grep __GLIBC__ /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/*.h
> > /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/features.h: The macros `__GNU_LIBRARY__', `__GLIBC__', and `__GLIBC_MINOR__' are
> > /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/features.h:#define __GLIBC__ 2
> > /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/features.h: ((__GLIBC__ << 16) + __GLIBC_MINOR__ >= ((maj) << 16) + (min))
> > [perfbuilder@59ca4b424ded /]$
> >
> > Isn't that part of uClibc?
>
> Right you are. Per the big fat comment right above that code, this gross hack in
> uclibc is unavoidable as applications tend to rely on that define.
> So a better fix would be to check for various !GLIBC libs explicitly.
>
> #ifdef __UCLIBC__
>
> #elseif defined __MUSL__
>
> ....
>
> Not pretty from app usage pov, but that seems to be the only sane way of doing it.
What are you trying to achieve? I was just CC'd and I'm missing the
context.
Rich
On 4/30/19 10:04 AM, Rich Felker wrote:
> On Tue, Apr 30, 2019 at 03:53:18PM +0000, Vineet Gupta wrote:
>> On 4/29/19 6:18 PM, Arnaldo Carvalho de Melo wrote:
>>>>> Auto-detecting system features:
>>>>> ... dwarf: [ OFF ]
>>>>> ... dwarf_getlocations: [ OFF ]
>>>>> ... glibc: [ on ]
>>>> Not related to current issue, this run uses a uClibc toolchain and yet it is
>>>> detecting glibc - doesn't seem right to me.
>>> Ok, I'll improve that, I think it just tries to detect a libc, yeah,
>>> see:
>>>
>>> [acme@quaco linux]$ cat tools/build/feature/test-glibc.c
>>> // SPDX-License-Identifier: GPL-2.0
>>> #include <stdlib.h>
>>>
>>> #if !defined(__UCLIBC__)
>>> #include <gnu/libc-version.h>
>>> #else
>>> #define XSTR(s) STR(s)
>>> #define STR(s) #s
>>> #endif
>>>
>>> int main(void)
>>> {
>>> #if !defined(__UCLIBC__)
>>> const char *version = gnu_get_libc_version();
>>> #else
>>> const char *version = XSTR(__GLIBC__) "." XSTR(__GLIBC_MINOR__);
>>> #endif
>>>
>>> return (long)version;
>>> }
>>> [acme@quaco linux]$
>>>
>>> [perfbuilder@59ca4b424ded /]$ grep __GLIBC__ /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/*.h
>>> /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/features.h: The macros `__GNU_LIBRARY__', `__GLIBC__', and `__GLIBC_MINOR__' are
>>> /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/features.h:#define __GLIBC__ 2
>>> /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/features.h: ((__GLIBC__ << 16) + __GLIBC_MINOR__ >= ((maj) << 16) + (min))
>>> [perfbuilder@59ca4b424ded /]$
>>>
>>> Isn't that part of uClibc?
>>
>> Right you are. Per the big fat comment right above that code, this gross hack in
>> uclibc is unavoidable as applications tend to rely on that define.
>> So a better fix would be to check for various !GLIBC libs explicitly.
>>
>> #ifdef __UCLIBC__
>>
>> #elseif defined __MUSL__
>>
>> ....
>>
>> Not pretty from app usage pov, but that seems to be the only sane way of doing it.
>
> What are you trying to achieve? I was just CC'd and I'm missing the
> context.
Sorry I added you as a subject matter expert but didn't provide enough context.
The original issue [1] was perf failing to build on ARC due to perf tools needing
a copy of unistd.h but this thread [2] was a small side issue of auto-detecting
libc variaint in perf tools where despite uClibc tools, glibc is declared to be
detected, due to uClibc's historical hack of defining __GLIBC__. So __GLIBC__ is
not sufficient (and probably not the right interface to begin wtih) to ensure glibc.
[1] http://lists.infradead.org/pipermail/linux-snps-arc/2019-April/005676.html
[2] http://lists.infradead.org/pipermail/linux-snps-arc/2019-April/005684.html
On Tue, Apr 30, 2019 at 10:13:40AM -0700, Vineet Gupta wrote:
> On 4/30/19 10:04 AM, Rich Felker wrote:
> > On Tue, Apr 30, 2019 at 03:53:18PM +0000, Vineet Gupta wrote:
> >> On 4/29/19 6:18 PM, Arnaldo Carvalho de Melo wrote:
> >>>>> Auto-detecting system features:
> >>>>> ... dwarf: [ OFF ]
> >>>>> ... dwarf_getlocations: [ OFF ]
> >>>>> ... glibc: [ on ]
> >>>> Not related to current issue, this run uses a uClibc toolchain and yet it is
> >>>> detecting glibc - doesn't seem right to me.
> >>> Ok, I'll improve that, I think it just tries to detect a libc, yeah,
> >>> see:
> >>>
> >>> [acme@quaco linux]$ cat tools/build/feature/test-glibc.c
> >>> // SPDX-License-Identifier: GPL-2.0
> >>> #include <stdlib.h>
> >>>
> >>> #if !defined(__UCLIBC__)
> >>> #include <gnu/libc-version.h>
> >>> #else
> >>> #define XSTR(s) STR(s)
> >>> #define STR(s) #s
> >>> #endif
> >>>
> >>> int main(void)
> >>> {
> >>> #if !defined(__UCLIBC__)
> >>> const char *version = gnu_get_libc_version();
> >>> #else
> >>> const char *version = XSTR(__GLIBC__) "." XSTR(__GLIBC_MINOR__);
> >>> #endif
> >>>
> >>> return (long)version;
> >>> }
> >>> [acme@quaco linux]$
> >>>
> >>> [perfbuilder@59ca4b424ded /]$ grep __GLIBC__ /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/*.h
> >>> /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/features.h: The macros `__GNU_LIBRARY__', `__GLIBC__', and `__GLIBC_MINOR__' are
> >>> /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/features.h:#define __GLIBC__ 2
> >>> /arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/features.h: ((__GLIBC__ << 16) + __GLIBC_MINOR__ >= ((maj) << 16) + (min))
> >>> [perfbuilder@59ca4b424ded /]$
> >>>
> >>> Isn't that part of uClibc?
> >>
> >> Right you are. Per the big fat comment right above that code, this gross hack in
> >> uclibc is unavoidable as applications tend to rely on that define.
> >> So a better fix would be to check for various !GLIBC libs explicitly.
> >>
> >> #ifdef __UCLIBC__
> >>
> >> #elseif defined __MUSL__
> >>
> >> ....
> >>
> >> Not pretty from app usage pov, but that seems to be the only sane way of doing it.
> >
> > What are you trying to achieve? I was just CC'd and I'm missing the
> > context.
>
> Sorry I added you as a subject matter expert but didn't provide enough context.
>
> The original issue [1] was perf failing to build on ARC due to perf tools needing
> a copy of unistd.h but this thread [2] was a small side issue of auto-detecting
> libc variaint in perf tools where despite uClibc tools, glibc is declared to be
> detected, due to uClibc's historical hack of defining __GLIBC__. So __GLIBC__ is
> not sufficient (and probably not the right interface to begin wtih) to ensure glibc.
>
> [1] http://lists.infradead.org/pipermail/linux-snps-arc/2019-April/005676.html
> [2] http://lists.infradead.org/pipermail/linux-snps-arc/2019-April/005684.html
I think you misunderstood -- I'm asking what you're trying to achieve
by detecting whether the libc is glibc, rather than whether it has
some particular interface you want to conditionally use. This is a
major smell and is usually something wrong that shouldn't be done.
Rich
On 4/30/19 8:12 PM, Rich Felker wrote:
>>> What are you trying to achieve? I was just CC'd and I'm missing the
>>> context.
>>
>> Sorry I added you as a subject matter expert but didn't provide enough context.
>>
>> The original issue [1] was perf failing to build on ARC due to perf tools needing
>> a copy of unistd.h but this thread [2] was a small side issue of auto-detecting
>> libc variaint in perf tools where despite uClibc tools, glibc is declared to be
>> detected, due to uClibc's historical hack of defining __GLIBC__. So __GLIBC__ is
>> not sufficient (and probably not the right interface to begin wtih) to ensure glibc.
>>
>> [1] http://lists.infradead.org/pipermail/linux-snps-arc/2019-April/005676.html
>> [2] http://lists.infradead.org/pipermail/linux-snps-arc/2019-April/005684.html
>
> I think you misunderstood --
:-)
> I'm asking what you're trying to achieve
> by detecting whether the libc is glibc, rather than whether it has
> some particular interface you want to conditionally use. This is a
> major smell and is usually something wrong that shouldn't be done.
Good question indeed. Back in 2015 I initially ran into some quirks due to subtle
libc differences. At the time perf has a fwd ref for strlcpy which exactly
matched glibc but not uClibc. see commit a83d869f300bf91 "(perf tools: Elide
strlcpy warning with uclibc)" or 0215d59b154 "(tools lib: Reinstate strlcpy()
header guard with __UCLIBC__)"
But this still used the libc defined symbol __UCLIBC__ or __GLIBC__
Your question however pertains to perf glibc feature check where perf generates an
alternate symbol HAVE_GLIBC_SUPPORT.
This is dubious as first of all it detects glibc even for uClibc builds.
Even of we were to improve it, there seems to be no users of this symbol.
$git grep HAVE_GLIBC_SUPPORT
perf/Makefile.config: CFLAGS += -DHAVE_GLIBC_SUPPORT
perf/builtin-version.c: STATUS(HAVE_GLIBC_SUPPORT, glibc)
So I'd propose to remove it !
Em Thu, May 02, 2019 at 09:55:26AM -0700, Vineet Gupta escreveu:
> On 4/30/19 8:12 PM, Rich Felker wrote:
> >>> What are you trying to achieve? I was just CC'd and I'm missing the
> >>> context.
> >>
> >> Sorry I added you as a subject matter expert but didn't provide enough context.
> >>
> >> The original issue [1] was perf failing to build on ARC due to perf tools needing
> >> a copy of unistd.h but this thread [2] was a small side issue of auto-detecting
> >> libc variaint in perf tools where despite uClibc tools, glibc is declared to be
> >> detected, due to uClibc's historical hack of defining __GLIBC__. So __GLIBC__ is
> >> not sufficient (and probably not the right interface to begin wtih) to ensure glibc.
> >>
> >> [1] http://lists.infradead.org/pipermail/linux-snps-arc/2019-April/005676.html
> >> [2] http://lists.infradead.org/pipermail/linux-snps-arc/2019-April/005684.html
> >
> > I think you misunderstood --
>
> :-)
>
> > I'm asking what you're trying to achieve
> > by detecting whether the libc is glibc, rather than whether it has
> > some particular interface you want to conditionally use. This is a
> > major smell and is usually something wrong that shouldn't be done.
>
> Good question indeed. Back in 2015 I initially ran into some quirks due to subtle
> libc differences. At the time perf has a fwd ref for strlcpy which exactly
> matched glibc but not uClibc. see commit a83d869f300bf91 "(perf tools: Elide
> strlcpy warning with uclibc)" or 0215d59b154 "(tools lib: Reinstate strlcpy()
> header guard with __UCLIBC__)"
>
> But this still used the libc defined symbol __UCLIBC__ or __GLIBC__
>
> Your question however pertains to perf glibc feature check where perf generates an
> alternate symbol HAVE_GLIBC_SUPPORT.
>
> This is dubious as first of all it detects glibc even for uClibc builds.
> Even of we were to improve it, there seems to be no users of this symbol.
>
> $git grep HAVE_GLIBC_SUPPORT
> perf/Makefile.config: CFLAGS += -DHAVE_GLIBC_SUPPORT
> perf/builtin-version.c: STATUS(HAVE_GLIBC_SUPPORT, glibc)
>
> So I'd propose to remove it !
This is some remnant of the past, I'll check further but will end up
just ditching it altogether as you suggest :-)
[acme@quaco perf]$ find tools/ -type f | xargs grep HAVE_GLIBC_SUPPORT
tools/perf/builtin-version.c: STATUS(HAVE_GLIBC_SUPPORT, glibc);
tools/perf/Makefile.config: CFLAGS += -DHAVE_GLIBC_SUPPORT
[acme@quaco perf]$
Its just this case that ends up using that feature detection program,
[acme@quaco perf]$ vim tools/perf/Makefile.config
[acme@quaco perf]$ find tools/ -type f | xargs grep feature-glibc
tools/perf/Makefile.config: ifeq ($(feature-glibc), 1)
tools/perf/Makefile.config:ifeq ($(feature-glibc), 1)
[acme@quaco perf]$
BTW the function on it doesn't mean anything, what matters is if the
program builds or not :-)
- Arnaldo