This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] remove nested functions from regcomp.c
- From: Konstantin Serebryany <konstantin dot s dot serebryany at gmail dot com>
- To: "Carlos O'Donell" <carlos at redhat dot com>
- Cc: Will Newton <will dot newton at linaro dot org>, Roland McGrath <roland at hack dot frob dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Fri, 6 Mar 2015 10:57:09 -0800
- Subject: Re: [PATCH] remove nested functions from regcomp.c
- Authentication-results: sourceware.org; auth=none
- References: <CAGQ9bdyadYuhF4zzTve8Qn4YtD0shRGq3+Tz0yXSg1cQQDB+qw at mail dot gmail dot com> <CANu=Dmg33DOcA8bWhhWoYPeo9dAG8EYUayv_e+qLHub8LP+UcA at mail dot gmail dot com> <542AC1BE dot 1080009 at redhat dot com> <CAGQ9bdz=EUw8oMJ=nVtkU_gJ7hsN=5ujmR8f5inWAY7N38a71A at mail dot gmail dot com>
>> This should be looked at first before considering any further changes,
>> however, Konstantin need not do this work himself, and we can help.
Any update with this? Is there anything I can do here?
This patch is the last one required to build libc.so with clang.
(A few more patches will be needed to build other targets, such as ld.so)
<shameless plug>
Building glibc with clang already provides benefits not currently
available with gcc: https://sourceware.org/glibc/wiki/FuzzingLibc (see
the last bullet)
</shameless plug>
On Tue, Sep 30, 2014 at 9:55 AM, Konstantin Serebryany
<konstantin.s.serebryany@gmail.com> wrote:
> On Tue, Sep 30, 2014 at 7:44 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 09/30/2014 04:13 AM, Will Newton wrote:
>>> On 29 September 2014 21:54, Konstantin Serebryany
>>> <konstantin.s.serebryany@gmail.com> wrote:
>>>> Hi,
>>>>
>>>> Please review the patch that removes nested functions from posix/regcomp.c.
>>>> The patch does not noticeably affect the generated code (same
>>>> instructions, a few differences in used registers, offsets, etc)
>>>> because all the affected functions have __attribute ((always_inline)).
>>>>
>>>> No regressions in 'make check' on x86_64-linux-gnu (Ubuntu 14.04)
>>>>
>>>> This code has some #ifdef branches for "not _LIBC",
>>>> please let me know if any separate testing is required for that case.
>>>
>>> This file is shared with gnulib so ideally the changes should be OKed
>>> there too. Unfortunately this particular file seems to have diverged a
>>> bit from gnulib so that might not be that straightforward.
>>
>> The first step is to synchronize this file with gnulib.
>>
>> This should be looked at first before considering any further changes,
>> however, Konstantin need not do this work himself, and we can help.
>
> Thanks!
> I'll wait for this being resolved and work on cleaning something else
> in the meantime.
> Let me know if there is anything I can do here.
>
>>
>> Lastly, we want *certainty* in our statements about compiler warnings
>> being wrong. A thorough reasoning needs to be brought forward to say
>> why the compiler warning is wrong. A hand-waving argument doesn't cut it.
>
> I've filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63418
>
> Note that there is a large number of similar bugs in gcc bugzilla:
> https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=SUSPENDED&bug_status=WAITING&bug_status=REOPENED&field0-0-0=product&field0-0-1=component&field0-0-2=alias&field0-0-3=short_desc&field0-0-4=status_whiteboard&field0-0-5=content&list_id=98897&order=bug_id%20DESC&query_based_on=&query_format=advanced&type0-0-0=substring&type0-0-1=substring&type0-0-2=substring&type0-0-3=substring&type0-0-4=substring&type0-0-5=matches&value0-0-0=Wmaybe-uninitialized&value0-0-1=Wmaybe-uninitialized&value0-0-2=Wmaybe-uninitialized&value0-0-3=Wmaybe-uninitialized&value0-0-4=Wmaybe-uninitialized&value0-0-5=%22Wmaybe-uninitialized%22
> In my experience, this gcc warning never worked properly.
>
> Here is a tiny example that demonstrates that the warning is shut down
> in the presence of nested functions, this is why we did not see it
> before on regcomp.c
> (There is no point in complaining to GCC about this, IMHO, since
> nested functions are known to be poorly supported).
>
> % cat local_func_warning.c
> static static_func(unsigned cond, int *a) { *a = 1; }
> void foo(unsigned cond, int *b) {
> int *a;
> if (cond) a = b;
>
> #ifdef USE_NESTED
> void inline
> __attribute ((always_inline))
> nested_func() {
> *a = 1;
> }
> nested_func();
> #else
> static_func1(cond, a);
> #endif
> }
> % ~/gcc-inst/bin/gcc -O2 -c -Wmaybe-uninitialized local_func_warning.c
> local_func_warning.c: In function âfooâ:
> local_func_warning.c:14:3: warning: âaâ may be used uninitialized in
> this function [-Wmaybe-uninitialized]
> static_func1(cond, a);
> ^
> % ~/gcc-inst/bin/gcc -O2 -c -Wmaybe-uninitialized
> local_func_warning.c -DUSE_NESTED
> %
>
> --kcc
>
>
>
>
>>
>> Cheers,
>> Carlos.
>>