[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 Jan Hubicka changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #17 from Jan Hubicka --- Thanks! New testcases would be welcome (as new PRs:)
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 --- Comment #16 from Steinar H. Gunderson --- Yes, I'm fine with closing this bug; the spurious cases are gone, and the traces seem to be genuinely helpful now.
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668
--- Comment #15 from Jan Hubicka ---
> I tested with a GCC snapshot (at r267505). I can now build all mysqld with LTO
> and get exactly one LTO warning, and it's a true positive (two Bison parsers
> that we haven't managed to untangle yet).
>
> [1/1] Linking CXX executable runtime_output_directory/mysqld
> ../sql/sql_lex.h:2067:7: warning: type 'union YYSTYPE' violates the C++ One
> Definition Rule [-Wodr]
> 2067 | union YYSTYPE {
> | ^
> ../storage/innobase/include/fts0pars.h:50: note: a different type is defined
> in
> another translation unit
>50 | typedef union YYSTYPE
> |
> ../sql/sql_lex.h:2071:18: note: the first difference of corresponding
> definitions is field 'hint_type'
> 2071 | opt_hints_enum hint_type;
> | ^
> fts0pars.y:62: note: a field with different name is defined in another
> translation unit
Looks good to me. I think it is reported corectly and should be easy for
developer to understand what is going on. So we can close the PR. Any
additional testcases would be great!
Honza
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668
--- Comment #14 from Steinar H. Gunderson ---
I tested with a GCC snapshot (at r267505). I can now build all mysqld with LTO
and get exactly one LTO warning, and it's a true positive (two Bison parsers
that we haven't managed to untangle yet).
[1/1] Linking CXX executable runtime_output_directory/mysqld
../sql/sql_lex.h:2067:7: warning: type 'union YYSTYPE' violates the C++ One
Definition Rule [-Wodr]
2067 | union YYSTYPE {
| ^
../storage/innobase/include/fts0pars.h:50: note: a different type is defined in
another translation unit
50 | typedef union YYSTYPE
|
../sql/sql_lex.h:2071:18: note: the first difference of corresponding
definitions is field 'hint_type'
2071 | opt_hints_enum hint_type;
| ^
fts0pars.y:62: note: a field with different name is defined in another
translation unit
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 --- Comment #13 from Jan Hubicka --- Warnings from comment #8 are fixed now. I would love to know if there are any issues with what GCC 9 outputs. We still can't track locations to the original .o files though.
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 --- Comment #12 from sgunderson at bigfoot dot com --- The spurious warning seems to be gone in GCC 8.
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 Jan Hubicka changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2017-11-19 Assignee|unassigned at gcc dot gnu.org |hubicka at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #11 from Jan Hubicka --- I will try to recover that patch.
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 --- Comment #10 from Jan Hubicka --- Problem is that we do not have location info that would say us the origin. I have sent patch to add location info into TRANSLATION_UNIT_DECL some time ago (at least a year) but I do not think it was approved.
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 --- Comment #9 from sgunderson at bigfoot dot com --- (In reply to Manuel López-Ibáñez from comment #8) > Actually, what would be more useful is to detect that the difference in type > comes from S and point out where S has been declared as different types. Yes, that would be even better. But save for that :-) > Note that this is not the same bug I pointed out for > > ../include/violite.h:288:8: warning: type ‘struct st_vio’ violates the C++ > One Definition Rule [-Wodr] > ../include/violite.h:288:0: note: a different type is defined in another > translation unit > > The :0: indicates something wrong with the location info. If the location is > unknown, it would be better to use UNKNOWN_LOCATION. Yes, I know. It's a bit odd, but it doesn't bother me as much in this case.
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 --- Comment #8 from Manuel López-Ibáñez --- (In reply to sgunderson from comment #7) > What I'd like is some sort of indication about where test.h came in from > (test1.cc and test2.cc). Actually, what would be more useful is to detect that the difference in type comes from S and point out where S has been declared as different types. Note that this is not the same bug I pointed out for ../include/violite.h:288:8: warning: type ‘struct st_vio’ violates the C++ One Definition Rule [-Wodr] ../include/violite.h:288:0: note: a different type is defined in another translation unit The :0: indicates something wrong with the location info. If the location is unknown, it would be better to use UNKNOWN_LOCATION.
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668
--- Comment #7 from sgunderson at bigfoot dot com ---
(In reply to Manuel López-Ibáñez from comment #6)
>> fts0pars.y:62:0: note: a field with different name is defined in another
>> translation unit
> Did you cut the above? It looks like a note without a previous warning.
> Also, GCC will have trouble to point out the correct location when compiling
> a generated file that contains linemarkers, unless the linemarkers exactly
> point out to the original file AND the original file is available to read.
Sorry, yes, it was cut (I didn't intend to include it, as it is related to
another and very real warning).
Let me make a more minimal example to illustrate my issue (adapted from the
case in 81716). I thought I'd pasted it already, but evidently it never made
Bugzilla.
atum17:~> cat test1.cc
#include "test.h"
void foo(S *t)
{
q[0] = nullptr;
}
atum17:~> cat test2.cc
#include
#include "test.h"
class S {
int m;
};
void bar(S *t)
{
printf("%p\n", q[0]);
}
atum17:~> cat test.h
class S;
extern S *q[10];
atum17:~> /usr/lib/gcc-snapshot/bin/g++ -Wall -O2 -flto -o test.so test1.cc
test2.cc
test.h:2:11: warning: type of 'q' does not match original declaration
[-Wlto-type-mismatch]
extern S *q[10];
^
test.h:2:11: note: 'q' was previously declared here
extern S *q[10];
^
test.h:2:11: note: code may be misoptimized unless -fno-strict-aliasing is used
/usr/lib/x86_64-linux-gnu/crt1.o: In function `_start':
(.text+0x20): undefined reference to `main'
collect2: error: ld returned 1 exit status
What I'd like is some sort of indication about where test.h came in from
(test1.cc and test2.cc).
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 Manuel López-Ibáñez changed: What|Removed |Added CC||manu at gcc dot gnu.org --- Comment #6 from Manuel López-Ibáñez --- > fts0pars.y:62:0: note: a field with different name is defined in another > translation unit Did you cut the above? It looks like a note without a previous warning. Also, GCC will have trouble to point out the correct location when compiling a generated file that contains linemarkers, unless the linemarkers exactly point out to the original file AND the original file is available to read. > ../include/violite.h:288:8: warning: type ‘struct st_vio’ violates the C++ > One Definition Rule [-Wodr] > struct st_vio > ^ > ../include/violite.h:288:0: note: a different type is defined in another > translation unit > struct st_vio > This seems a problem with location info that fails to point to the .c file affected. One should never get column 0.
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 --- Comment #5 from sgunderson at bigfoot dot com --- (In reply to Markus Trippelsdorf from comment #3) > I don't see any bug, all relevant information is in the warnings. My point is that all relevant information _isn't_ in the warnings. In particular: The context of the .h file (which .o/.cc file it was compiled as part of in the two cases) is nowhere to be found. If I had that, it would be a lot easier to preprocess the two files and try to find what the difference is. Seemingly at least one of these was a GCC bug (#81716); with some luck, the others I cannot figure out are, too.
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 Martin Sebor changed: What|Removed |Added CC||msebor at gcc dot gnu.org --- Comment #4 from Martin Sebor --- (In reply to sgunderson from comment #2) > file; how would I know which one to look > through to find the two differing definitions? One thing to try might be to add static_assert() statements to the header(s) that define these allegedly incompatible symbols (such as struct st_vio) and using C++ or GCC type traits verify that they and all their members are in fact of the expected type, independent of any macros (like MYSQL_VIO). If you can then compile all the translation units successfully without LTO and still get errors with LTO that would suggest a bug in GCC and might also give more information to debug it. Otherwise, errors without LTO should point to the translation units that define the symbols in incompatible ways.
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 Markus Trippelsdorf changed: What|Removed |Added CC||trippels at gcc dot gnu.org --- Comment #3 from Markus Trippelsdorf --- (In reply to sgunderson from comment #2) > Running with -fno-diagnostics-show-caret does not help any: > > ../include/violite.h:288:8: warning: type ‘struct st_vio’ violates the C++ > One Definition Rule [-Wodr] > ../include/violite.h:288:0: note: a different type is defined in another > translation unit > ../include/violite.h:339:46: note: the first difference of corresponding > definitions is field ‘viodelete’ > ../include/violite.h:339:0: note: a field of same name but different type is > defined in another translation unit > > It's hard for me to look at the preprocessed source code, because I don't > know what to preprocess. Like I said, there's probably a thousand > translation units including this .h file; how would I know which one to look > through to find the two differing definitions? You cannot expect anyone to debug MySQL for you. I don't see any bug, all relevant information is in the warnings. It is up to you to figure out the rest.
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 --- Comment #2 from sgunderson at bigfoot dot com --- Running with -fno-diagnostics-show-caret does not help any: ../include/violite.h:288:8: warning: type ‘struct st_vio’ violates the C++ One Definition Rule [-Wodr] ../include/violite.h:288:0: note: a different type is defined in another translation unit ../include/violite.h:339:46: note: the first difference of corresponding definitions is field ‘viodelete’ ../include/violite.h:339:0: note: a field of same name but different type is defined in another translation unit It's hard for me to look at the preprocessed source code, because I don't know what to preprocess. Like I said, there's probably a thousand translation units including this .h file; how would I know which one to look through to find the two differing definitions?
[Bug c++/81668] LTO ODR warnings are not helpful
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81668 Richard Biener changed: What|Removed |Added Keywords||diagnostic, lto CC||hubicka at gcc dot gnu.org --- Comment #1 from Richard Biener --- Yeah, those cases are obfuscated but usually they have macros expanding to different things. You can debug this by looking at preprocessed source of the two places (look at "included from" messages). Not sure what we do with -fno-diagnostics-show-caret, maybe we dump the actual types that mismatch. With caret locations those would be often redundant.
