[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 Jan Hubicka changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #15 from Jan Hubicka --- Fixed some time ago.
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #14 from Jan Hubicka --- Author: hubicka Date: Wed Nov 21 02:38:43 2018 New Revision: 266334 URL: https://gcc.gnu.org/viewcvs?rev=266334=gcc=rev Log: PR lto/84044 * ipa-devirt.c (odr_types_equivalent_p): Use operand_equal_p to compare ENUM values. * g++.dg/lto/odr-4_0.C: New testcase. * g++.dg/lto/odr-4_1.C: New testcase. Added: trunk/gcc/testsuite/g++.dg/lto/odr-4_0.C trunk/gcc/testsuite/g++.dg/lto/odr-4_1.C Modified: trunk/gcc/ChangeLog trunk/gcc/ipa-devirt.c trunk/gcc/testsuite/ChangeLog
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #13 from Jan Hubicka --- ... and I can also confirm that the original testcase no longer triggers.
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #12 from Jan Hubicka --- Actually the issue here is that enum is context that is the type B and it has keyed vtable which makes it unmerged. So we are correct to not merge here provided that we want to keep TYPE_CONTEXT here (which I am not 100% sure we want) I will check in the testcase along the fix.
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #11 from Jan Hubicka --- Warning from #9 is caused by fact that I compare pointers rather than test for equality so when the enums are not merged, they triggers the warning even if the values are in fact equivalent. The following silences the warning but enums should be really merged here, so I need to look into why. Concerning the original bug I believe it was fixed a while ago when we cleaned up the BINFO streaming. Index: ipa-devirt.c === --- ipa-devirt.c(revision 266325) +++ ipa-devirt.c(working copy) @@ -1343,7 +1343,7 @@ odr_types_equivalent_p (tree t1, tree t2 " is defined in another translation unit")); return false; } - if (TREE_VALUE (v1) != TREE_VALUE (v2)) + if (!operand_equal_p (TREE_VALUE (v1), TREE_VALUE (v2), 0)) { warn_odr (t1, t2, NULL, NULL, warn, warned, G_("an enum with different values is defined"
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #10 from Martin Liška --- > Thread model: posix > gcc version 9.0.0 20181110 (experimental) [trunk revision 266003] (Debian > 20181110-2) > > Regards, > Arnaud Giersch The warning appeared again on trunk in r265875 Author: hubicka * ipa-devirt.c (odr_types_equivalent_p): Expect constants than const decls in TREE_VALUE of enum. (dump_type_inheritance_graph): Improve duplicate dumping. (free_enum_values): New. (build_type_inheritance_graph): Use it. * tree.c (free_lang_data_in_type): Free TYPE_VALUES of enums which are not main variants or not ODR types. (verify_type_variant): Expect variants to have no TYPE_VALUES.
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 Arnaud Giersch changed: What|Removed |Added CC||arnaud.giersch at free dot fr --- Comment #9 from Arnaud Giersch --- Hi, A similar warning appeared recently with a gcc9 snapshot with a slightly modified code. The main difference seems to be the enum declaration. By "recently", I mean late october or early november (2018). a.cpp = struct B { enum class E { V0, V1 }; virtual ~B(); E e; }; B b; int main() {} b.cpp = struct B { enum class E { V0, V1 }; virtual ~B(); E e; }; B::~B() = default; = $ g++ -O2 -flto a.cpp b.cpp a.cpp:2:14: warning: type 'E' violates the C++ One Definition Rule [-Wodr] 2 | enum class E { V0, V1 }; | ^ a.cpp:2:14: note: an enum with different values is defined in another translation unit a.cpp:4:11: warning: '__dt_del ' violates the C++ One Definition Rule [-Wodr] 4 | virtual ~B(); | ^ b.cpp:1:8: note: '__dt_del ' was previously declared here 1 | struct B { |^ a.cpp:4:11: warning: '__dt_comp ' violates the C++ One Definition Rule [-Wodr] 4 | virtual ~B(); | ^ b.cpp:1:8: note: '__dt_comp ' was previously declared here 1 | struct B { |^ $ g++ -v Using built-in specs. COLLECT_GCC=g++ COLLECT_LTO_WRAPPER=/usr/lib/gcc-snapshot/libexec/gcc/x86_64-linux-gnu/9/lto-wrapper OFFLOAD_TARGET_NAMES=nvptx-none Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Debian 20181110-2' --with-bugurl=file:///usr/share/doc/gcc-snapshot/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr/lib/gcc-snapshot --with-gcc-major-version-only --program-prefix= --enable-shared --enable-linker-build-id --disable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --with-system-zlib --with-target-system-zlib --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none --enable-checking=yes --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 9.0.0 20181110 (experimental) [trunk revision 266003] (Debian 20181110-2) Regards, Arnaud Giersch
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 Martin Liška changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |hubicka at gcc dot gnu.org
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #8 from Jan Hubicka --- > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 > > --- Comment #7 from rguenther at suse dot de --- > On Fri, 26 Jan 2018, marxin at gcc dot gnu.org wrote: > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 > > > > --- Comment #5 from Martin Liška --- > > (In reply to Richard Biener from comment #4) > > > (In reply to Martin Liška from comment #3) > > > > Fixed on trunk by Richi's r256685. Is it intentional Richi that the > > > > revision > > > > should fix such situations? > > > > > > Not really. It means that the following hunk removes too many TYPE_BINFOs > > > I guess? > > > > > > @@ -5150,15 +5145,9 @@ free_lang_data_in_type (tree type) > > > { > > > free_lang_data_in_binfo (TYPE_BINFO (type)); > > > /* We need to preserve link to bases and virtual table for all > > > -polymorphic types to make devirtualization machinery working. > > > -Debug output cares only about bases, but output also > > > -virtual table pointers so merging of -fdevirtualize and > > > --fno-devirtualize units is easier. */ > > > - if ((!BINFO_VTABLE (TYPE_BINFO (type)) > > > - || !flag_devirtualize) > > > - && ((!BINFO_N_BASE_BINFOS (TYPE_BINFO (type)) > > > - && !BINFO_VTABLE (TYPE_BINFO (type))) > > > - || debug_info_level != DINFO_LEVEL_NONE)) > > > +polymorphic types to make devirtualization machinery > > > working. > > > */ > > > + if (!BINFO_VTABLE (TYPE_BINFO (type)) > > > + || !flag_devirtualize) > > > TYPE_BINFO (type) = NULL; > > > } > > > } > > > > Reverting this hunk makes the warning visible again. > > Interesting. This means that with -g it didn't warn before either? > Which means that maybe unconditionally > > && !BINFO_N_BASE_BINFOS (TYPE_BINFO (type)) > > was intented? Honza? Yep that was my tought exactly, but I want to take deeper look what we really use and what not. Honza > > -- > You are receiving this mail because: > You are on the CC list for the bug.
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #7 from rguenther at suse dot de --- On Fri, 26 Jan 2018, marxin at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 > > --- Comment #5 from Martin Liška --- > (In reply to Richard Biener from comment #4) > > (In reply to Martin Liška from comment #3) > > > Fixed on trunk by Richi's r256685. Is it intentional Richi that the > > > revision > > > should fix such situations? > > > > Not really. It means that the following hunk removes too many TYPE_BINFOs > > I guess? > > > > @@ -5150,15 +5145,9 @@ free_lang_data_in_type (tree type) > > { > > free_lang_data_in_binfo (TYPE_BINFO (type)); > > /* We need to preserve link to bases and virtual table for all > > -polymorphic types to make devirtualization machinery working. > > -Debug output cares only about bases, but output also > > -virtual table pointers so merging of -fdevirtualize and > > --fno-devirtualize units is easier. */ > > - if ((!BINFO_VTABLE (TYPE_BINFO (type)) > > - || !flag_devirtualize) > > - && ((!BINFO_N_BASE_BINFOS (TYPE_BINFO (type)) > > - && !BINFO_VTABLE (TYPE_BINFO (type))) > > - || debug_info_level != DINFO_LEVEL_NONE)) > > +polymorphic types to make devirtualization machinery working. > > */ > > + if (!BINFO_VTABLE (TYPE_BINFO (type)) > > + || !flag_devirtualize) > > TYPE_BINFO (type) = NULL; > > } > > } > > Reverting this hunk makes the warning visible again. Interesting. This means that with -g it didn't warn before either? Which means that maybe unconditionally && !BINFO_N_BASE_BINFOS (TYPE_BINFO (type)) was intented? Honza?
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #6 from Jan Hubicka --- > > @@ -5150,15 +5145,9 @@ free_lang_data_in_type (tree type) > > { > > free_lang_data_in_binfo (TYPE_BINFO (type)); > > /* We need to preserve link to bases and virtual table for all > > -polymorphic types to make devirtualization machinery working. > > -Debug output cares only about bases, but output also > > -virtual table pointers so merging of -fdevirtualize and > > --fno-devirtualize units is easier. */ > > - if ((!BINFO_VTABLE (TYPE_BINFO (type)) > > - || !flag_devirtualize) > > - && ((!BINFO_N_BASE_BINFOS (TYPE_BINFO (type)) > > - && !BINFO_VTABLE (TYPE_BINFO (type))) > > - || debug_info_level != DINFO_LEVEL_NONE)) > > +polymorphic types to make devirtualization machinery working. > > */ > > + if (!BINFO_VTABLE (TYPE_BINFO (type)) > > + || !flag_devirtualize) > > TYPE_BINFO (type) = NULL; > > } > > } > > Reverting this hunk makes the warning visible again. Hmm, I will take a look. BIFOs are bit of a magic. Thanks! Honza
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #5 from Martin Liška --- (In reply to Richard Biener from comment #4) > (In reply to Martin Liška from comment #3) > > Fixed on trunk by Richi's r256685. Is it intentional Richi that the revision > > should fix such situations? > > Not really. It means that the following hunk removes too many TYPE_BINFOs > I guess? > > @@ -5150,15 +5145,9 @@ free_lang_data_in_type (tree type) > { > free_lang_data_in_binfo (TYPE_BINFO (type)); > /* We need to preserve link to bases and virtual table for all > -polymorphic types to make devirtualization machinery working. > -Debug output cares only about bases, but output also > -virtual table pointers so merging of -fdevirtualize and > --fno-devirtualize units is easier. */ > - if ((!BINFO_VTABLE (TYPE_BINFO (type)) > - || !flag_devirtualize) > - && ((!BINFO_N_BASE_BINFOS (TYPE_BINFO (type)) > - && !BINFO_VTABLE (TYPE_BINFO (type))) > - || debug_info_level != DINFO_LEVEL_NONE)) > +polymorphic types to make devirtualization machinery working. > */ > + if (!BINFO_VTABLE (TYPE_BINFO (type)) > + || !flag_devirtualize) > TYPE_BINFO (type) = NULL; > } > } Reverting this hunk makes the warning visible again.
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #4 from Richard Biener --- (In reply to Martin Liška from comment #3) > Fixed on trunk by Richi's r256685. Is it intentional Richi that the revision > should fix such situations? Not really. It means that the following hunk removes too many TYPE_BINFOs I guess? @@ -5150,15 +5145,9 @@ free_lang_data_in_type (tree type) { free_lang_data_in_binfo (TYPE_BINFO (type)); /* We need to preserve link to bases and virtual table for all -polymorphic types to make devirtualization machinery working. -Debug output cares only about bases, but output also -virtual table pointers so merging of -fdevirtualize and --fno-devirtualize units is easier. */ - if ((!BINFO_VTABLE (TYPE_BINFO (type)) - || !flag_devirtualize) - && ((!BINFO_N_BASE_BINFOS (TYPE_BINFO (type)) - && !BINFO_VTABLE (TYPE_BINFO (type))) - || debug_info_level != DINFO_LEVEL_NONE)) +polymorphic types to make devirtualization machinery working. */ + if (!BINFO_VTABLE (TYPE_BINFO (type)) + || !flag_devirtualize) TYPE_BINFO (type) = NULL; } } Or this change drops required information for the ODR warning. @@ -5186,6 +5175,11 @@ free_lang_data_in_type (tree type) while (ctx && TREE_CODE (ctx) == BLOCK); TYPE_CONTEXT (type) = ctx; } + + /* Drop TYPE_DECLs in TYPE_NAME in favor of the identifier in the + TYPE_DECL if the type doesn't have linkage. */ + if (! type_with_linkage_p (type)) +TYPE_NAME (type) = TYPE_IDENTIFIER (type); } Maybe you can "bisect" that. But I guess the warning is "correct" and -fno-semantic-interposition affects details in the vtable that lead to the warning. Honza?
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 Martin Liška changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2018-01-26 Known to work||8.0 Ever confirmed|0 |1 --- Comment #3 from Martin Liška --- Fixed on trunk by Richi's r256685. Is it intentional Richi that the revision should fix such situations?
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #2 from Geoffrey Allott --- Or even simply A a;
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #1 from Geoffrey Allott --- I discovered that in b.cpp a free function A get() { return A(); } also triggers the error. Struct B is not necessary.