[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 --- Comment #23 from Jakub Jelinek --- Author: jakub Date: Sat Mar 9 12:08:23 2019 New Revision: 269525 URL: https://gcc.gnu.org/viewcvs?rev=269525=gcc=rev Log: PR c/88568 * attribs.c (handle_dll_attribute): Don't clear TREE_STATIC for dllimport on VAR_DECLs with RECORD_TYPE or UNION_TYPE DECL_CONTEXT. * g++.dg/other/pr88568.C: New test. Added: trunk/gcc/testsuite/g++.dg/other/pr88568.C Modified: trunk/gcc/ChangeLog trunk/gcc/attribs.c trunk/gcc/testsuite/ChangeLog
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 --- Comment #22 from jon_y <10walls at gmail dot com> --- Bootstrapped sucessfully on x86_64-pc-cygwin, test compiles and has the data member dllimported. Patch looks good.
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 --- Comment #21 from Stephen Kitt --- (In reply to Jakub Jelinek from comment #20) > Cross compilers don't bootstrap (build themselves multiple times), only > build. Sorry, I meant I did a library-less cross-build, followed by a MinGW-w64 build using the new compiler, followed by a full cross-build. Obviously that’s nowhere near as useful as a native build on Windows. > Sure, I have built a cross compiler to write and verify this change too, but > I have no access to Windows to bootstrap/regtest it natively. As I said on > gcc-patches, it is highly unlikely it will regress something, because if it > doesn't crash in the newly added condition, then it partially reverts to the > January state. Yes, it does seem highly unlikely. (Now I get to chase down another GCC 7-to-8 regression in SEH. Well, this evening...)
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 --- Comment #20 from Jakub Jelinek --- (In reply to Stephen Kitt from comment #19) > (In reply to Jakub Jelinek from comment #18) > > (In reply to Stephen Kitt from comment #17) > > > jon_y, you can have your weekend back ;-). > > > > Well, it should be tested also with a GCC bootstrap/regtest, that is a usual > > procedure for testing GCC changes. > > Ah, right, of course. I’ve run a cross-bootstrap, but not a bootstrap on > Windows, nor have I run the test suite on Windows. Cross compilers don't bootstrap (build themselves multiple times), only build. Sure, I have built a cross compiler to write and verify this change too, but I have no access to Windows to bootstrap/regtest it natively. As I said on gcc-patches, it is highly unlikely it will regress something, because if it doesn't crash in the newly added condition, then it partially reverts to the January state.
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 --- Comment #19 from Stephen Kitt --- (In reply to Jakub Jelinek from comment #18) > (In reply to Stephen Kitt from comment #17) > > jon_y, you can have your weekend back ;-). > > Well, it should be tested also with a GCC bootstrap/regtest, that is a usual > procedure for testing GCC changes. Ah, right, of course. I’ve run a cross-bootstrap, but not a bootstrap on Windows, nor have I run the test suite on Windows.
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 --- Comment #18 from Jakub Jelinek --- (In reply to Stephen Kitt from comment #17) > Thanks for the fix Jakub, I’ve verified it on Qt and Andre Heinecke has > verified it on Gpg4Win (see https://bugs.debian.org/923214 for details). Thanks. > jon_y, you can have your weekend back ;-). Well, it should be tested also with a GCC bootstrap/regtest, that is a usual procedure for testing GCC changes.
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 --- Comment #17 from Stephen Kitt --- Thanks for the fix Jakub, I’ve verified it on Qt and Andre Heinecke has verified it on Gpg4Win (see https://bugs.debian.org/923214 for details). jon_y, you can have your weekend back ;-).
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 --- Comment #16 from jon_y <10walls at gmail dot com> --- I'll try testing over the weekends.
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 --- Comment #9 from Jakub Jelinek --- Author: jakub Date: Thu Jan 10 10:44:46 2019 New Revision: 267799 URL: https://gcc.gnu.org/viewcvs?rev=267799=gcc=rev Log: PR c/88568 * attribs.c (handle_dll_attribute): Clear TREE_STATIC after setting DECL_EXTERNAL. * gcc.dg/pr88568.c: New test. Added: trunk/gcc/testsuite/gcc.dg/pr88568.c Modified: trunk/gcc/ChangeLog trunk/gcc/attribs.c trunk/gcc/testsuite/ChangeLog
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 --- Comment #8 from jon_y <10walls at gmail dot com> --- I've used a linux hosted mingw toolchain to build a mingw toolchain from the same sources, it seems to be working fine. I've only enabled C and C++.
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 --- Comment #7 from Jakub Jelinek --- The testcase I've checked in the cross-compiler too, I was more interested in knowing whether the compiler with the patch still bootstraps and doesn't regress against unpatched gcc in the testsuite, which is something I can't test easily without some emulator or access to the OS.
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 jon_y <10walls at gmail dot com> changed: What|Removed |Added CC||10walls at gmail dot com --- Comment #6 from jon_y <10walls at gmail dot com> --- Test case seems to be working for me. What else should I test?
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 --- Comment #5 from Jakub Jelinek --- Created attachment 45365 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45365=edit gcc9-pr88568.patch Full untested patch. Can somebody please bootstrap/regtest that on mingw or cygwin?
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 --- Comment #4 from joseph at codesourcery dot com --- On Fri, 4 Jan 2019, jakub at gcc dot gnu.org wrote: > Joseph, is there any meaning for DECL_EXTERNAL & TREE_STATIC, or is that > invalid flag combination? If the latter, we should go with the latter patch, > but I'm afraid I have no access to Windows and no way to test. I think a gnu_inline function definition is both DECL_EXTERNAL and TREE_STATIC.
[Bug c/88568] [7/8/9 Regression] 'dllimport' no longer implies 'extern' in C
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88568 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org, ||jsm28 at gcc dot gnu.org, ||jyong at gcc dot gnu.org Summary|[8/9 Regression]|[7/8/9 Regression] |'dllimport' no longer |'dllimport' no longer |implies 'extern' in C |implies 'extern' in C --- Comment #3 from Jakub Jelinek --- Guess it regressed with the PR24293 change then. I think the problem is that handle_dll_attribute has for dllimport on VAR_DECLs: /* `extern' needn't be specified with dllimport. Specify `extern' now and hope for the best. Sigh. */ DECL_EXTERNAL (node) = 1; /* Also, implicitly give dllimport'd variables declared within a function global scope, unless declared static. */ if (current_function_decl != NULL_TREE && !TREE_STATIC (node)) TREE_PUBLIC (node) = 1; The problem is that the decl is previously TREE_STATIC and TREE_PUBLIC and setting it also DECL_EXTERNAL results in an unusual combination of flags. So, either we can tweak the code reporting the issues and hope TREE_STATIC && DECL_EXTERNAL means something reasonable elsewhere: --- gcc/c/c-decl.c.jj 2019-01-01 12:37:48.628458597 +0100 +++ gcc/c/c-decl.c 2019-01-04 15:13:03.281213304 +0100 @@ -5100,7 +5100,8 @@ finish_decl (tree decl, location_t init_ if ((RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl)) || TREE_CODE (TREE_TYPE (decl)) == ENUMERAL_TYPE) && DECL_SIZE (decl) == NULL_TREE - && TREE_STATIC (decl)) + && TREE_STATIC (decl) + && !DECL_EXTERNAL (decl)) incomplete_record_decls.safe_push (decl); if (is_global_var (decl) && DECL_SIZE (decl) != NULL_TREE) or we should change handle_dll_attribute to: --- gcc/attribs.c 2019-01-01 12:37:18.066960036 +0100 +++ gcc/attribs.c 2019-01-04 15:41:55.949812827 +0100 @@ -1691,6 +1691,7 @@ handle_dll_attribute (tree * pnode, tree a function global scope, unless declared static. */ if (current_function_decl != NULL_TREE && !TREE_STATIC (node)) TREE_PUBLIC (node) = 1; + TREE_STATIC (node) = 0; } if (*no_add_attrs == false) Joseph, is there any meaning for DECL_EXTERNAL & TREE_STATIC, or is that invalid flag combination? If the latter, we should go with the latter patch, but I'm afraid I have no access to Windows and no way to test.
