[PING] [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

2016-11-11 Thread Bernd Edlinger 
Ping...

the latest version of the patch was here:
https://gcc.gnu.org/ml/gcc-patches/2016-11/msg00505.html

Thanks
Bernd.

On 11/02/16 22:15, Bernd Edlinger wrote:
> On 11/02/16 18:51, Jason Merrill wrote:
>> On 11/02/2016 02:11 AM, Bernd Edlinger wrote:
>>> On 11/01/16 19:15, Bernd Edlinger wrote:
 On 11/01/16 18:11, Jason Merrill wrote:
> On Tue, Nov 1, 2016 at 11:45 AM, Bernd Edlinger
>  wrote:
>> On 11/01/16 16:20, Jason Merrill wrote:
>>> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>>> I'm not even sure we need a new warning.  Can we combine this
>>> warning
>>> with the block that currently follows?
>>
>> After 20 years of not having a warning on that,
>> an implicitly enabled warning would at least break lots of bogus
>> test cases.
>
> Would it, though?  Which test cases still break with the current
> patch?

 Less than before, but there are still at least a few of them.

 I can make a list and send it tomorrow.
>>>
>>> FAIL: g++.dg/cpp1y/lambda-generic-udt.C  -std=gnu++14 (test for excess
>>> errors)
>>> FAIL: g++.dg/cpp1y/lambda-generic-xudt.C  -std=gnu++14 (test for excess
>>> errors)
>>> FAIL: g++.dg/init/new15.C  -std=c++11 (test for excess errors)
>>> FAIL: g++.dg/init/new15.C  -std=c++14 (test for excess errors)
>>> FAIL: g++.dg/init/new15.C  -std=c++98 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++11 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++14 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++98 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++11 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++14 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++98 (test for excess errors)
>>> FAIL: g++.dg/tc1/dr20.C  -std=c++11 (test for excess errors)
>>> FAIL: g++.dg/tc1/dr20.C  -std=c++14 (test for excess errors)
>>> FAIL: g++.dg/tc1/dr20.C  -std=c++98 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++11 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++14 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++98 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++11 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++14 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++98 (test for excess errors)
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
>>> -flto-partition=1to1 -fno-use-linker-plugin
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
>>> -flto-partition=none -fuse-linker-plugin
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
>>> -fuse-linker-plugin -fno-fat-lto-objects
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
>>> -flto-partition=1to1 -fno-use-linker-plugin
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
>>> -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
>>> -fuse-linker-plugin
>>> FAIL: g++.dg/lto/pr68811 cp_lto_pr68811_0.o assemble, -O2
>>> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++98 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++98 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++98 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++98 (test for excess
>>> errors)
>>>
>>>
>>> The lto test case does emit the warning when assembling, but
>>> it still produces an executable and even executes it.
>>>
>>> Also g++.dg/cpp1y/lambda-generic-udt.C, g++.dg/tc1/dr20.C
>>> and g++.old-deja/g++.other/vbase5.C are execution tests.
>>>
>>> So I was wrong to assume these were all compile-only tests.
>>>
>>> I think that list should be fixable, if we decide to enable
>>> the warning by default.
>>
>> Yes, either by fixing the prototypes or disabling the warning.
>>
>
> Yes, I am inclined to enable the warning by

[PING] [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

2016-10-24 Thread Bernd Edlinger 
Hi!

I'd like to ping for my patch here:
https://gcc.gnu.org/ml/gcc-patches/2016-10/msg01348.html


Thanks
Bernd.

On 10/17/16 21:18, Bernd Edlinger wrote:
> On 10/17/16 20:05, Joseph Myers wrote:
>> On Sun, 16 Oct 2016, Bernd Edlinger wrote:
>>
>>> Second, the declaration in the glibc header files simply look wrong,
>>> because the type of argv, and envp is "char *const *" while the
>>> builtin function wants "const char**", thus only the array of char*
>>> itself is const, not the actual char stings they point to.
>>
>> char *const * is the POSIX type.  (It can't be const char ** or const
>> char
>> *const * because you can't convert char ** implicitly to those types in
>> ISO C.)  You'd need to check the list archives for rationale for the
>> built-in functions doing something different.
>>
>
> Yes, that was discussed here:
> https://gcc.gnu.org/ml/gcc-patches/2004-03/msg01148.html
>
> No mention why the BT_PTR_CONST_TYPE does not match the posix type.
> But the right types were used on __gcov_execv/e/p stubs, so the author
> did know the right types at least.
>
> So I think that was broken from the beginning, but that was hidden by
> the loose checking in the C FE and not warning in the C++ FE when
> prototypes don't match.
>
>>> Third, in C the  builtins are not diagnosed, because C does only look
>>> at the mode of the parameters see match_builtin_function_types in
>>> c/c-decl.c, which may itself be wrong, because that makes an ABI
>>> decision dependent on the mode of the parameter.
>>
>> The matching needs to be loose because of functions using types such as
>> FILE * where the compiler doesn't know the exact contents of the type
>> when
>> processing built-in function definitions.  (Historically there were also
>> issues with pre-ISO headers, but that may be less relevant now.)
>>
>
> The C++ FE has exactly the same problem with FILE* and struct tm*
> but it solves it differently and "learns" the type but only for FILE*
> and with this patch also for const struct tm*.  It is a lot more
> restrictive than C, but that is because of the ++ ;)
>
> Well in that case the posix functions have to use the prototypes
> from POSIX.1.2008 although their rationale is a bit silly...
>
> This updated patch fixes the prototype of execv/p/e,
> and adds a new test case that checks that no type conflict
> exists in the execve built-in any more.
>
> Now we have no -Wsystem-headers warnings with glibc-headers any more.
> And the gcov builtin also is working with C++.
>
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
>
>
> Thanks
> Bernd.