New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build with gcc6 broken [CORE5099] #5384
Comments
Commented by: @AlexPeshkoff Michal, do we have an exact reference to the standard explaining reasons of such incompatible change of operator new? |
Commented by: Damyan Ivanov (dam) I tried using -std=gnu++98 with firebird 2.5.5 (technically plus some Debian patches). There are no compilation errors, but running create_db either fails with invalid request BLR at offset 24 or aborts with a segmentation fault. Here's the stack trace: (gdb) thread apply all bt Thread 4 (LWP 28868): Thread 3 (LWP 28869): Thread 2 (LWP 28867): Thread 1 (LWP 28866): Omitting -Ox flags makes the full build pass, including creation of various databases, super and classic. Using -O1 segfaults again, but with a more verbose backtrace: Program terminated with signal SIGSEGV, Segmentation fault. Thread 4 (Thread 0x2b7ff07f1700 (LWP 8051)): Thread 3 (Thread 0x2b7ff03ef700 (LWP 8049)): Thread 2 (Thread 0x2b7ff05f0700 (LWP 8050)): Thread 1 (Thread 0x2b7fed17fb80 (LWP 8048)): |
Commented by: @AlexPeshkoff When optimization level affects result of program execution that's sooner of all compiler issue, is not it? |
Commented by: @dyemanov Looks like UCHAR -> SSHORT conversion triggers an incorrect result somewhere. And yes, this smells like a compiler bug. |
Commented by: Damyan Ivanov (dam) (gdb) explore stream The value seems to come from the nextStream method, which uses csb_n_stream member. The later however seems uninitialized to me: (gdb) explore csb pool_alloc<(BlockType)15> = <Enter 0 to explore this base class of type 'pool_alloc<(BlockType)15>'> Enter the field number of choice: 12 so it is indeed a ushort->sshort conversion (induced by the types used to pass the value), but the source of the problem seems to be the very large value in csb.csb_n_stream. |
Commented by: @AlexPeshkoff 52521 == 0xcd29, looks like garbage at the first look CompilerScratch is using operator new from class pool_alloc, which zeroes memory. Can't an issue be due to using wrong operator new and therefore having not initialized memory including csb_n_stream? |
Commented by: @asfernandes A good technique I use to debug this type of problem is: |
Commented by: Damyan Ivanov (dam) Setting a watchpoint for the address tells me the memory is indeed not initialized. Here's the list of places where the memory location of csb.csb_n_stream is changed: Hardware watchpoint 2: *(USHORT *) 0x7ffff36b7b00 Old value = <unreadable> Old value = 31920 Old value = 30232 Old value = 30856 Old value = 30232 Old value = 30856 Old value = 30968 Old value = 27944 There's no zeroing-out, as it seems. How do I check if the right memory allocator is used? |
Commented by: @dyemanov csb is an instance of class CompilerScratch which inherits from pool_alloc: class CompilerScratch : public pool_alloc<type_csb> Inside pool_alloc:
And MemoryPool::calloc() zero-initializes the memory. If gcc6 decides to use some different operator new() rather than pool_alloc:new(), then Firebird has no chances to work. |
Commented by: @mkubecek As found by Richard Biener (our toochain developer), Firebird 2.5 builds fine with "-std=gnu++98 -fno-lifetime-dse". The gcc documentation for -fno-lifetime-dse says -fno-lifetime-dse IMHO this may explain the behaviour observed in previous comments, the case of operator new clearing object storage is explicitely mentioned as an example. |
Commented by: @asfernandes Here is my findings building trunk with gcc 6: First, I just committed a change for GPRE generate code compatible with C++-14 I build in C++-14 mode with these options: -fno-sized-deallocation: because C++-14 pass the size to the delete operator. Instead of use this options, it would be better to check __cpluplus >= 201402L and do the appropriate change in the sources. -fno-delete-null-pointer-checks: becase the ->as ->is methods that we call in null pointers. May have others similar things. These options are not needed: -no-lifetime-dse But I think -flifetime-dse=1 should be safe. |
Commented by: @hvlad (8) should be fixed now (it produced errors with MSVC14, not warnings) |
Commented by: @pmakowski For information, here the build log for Firebird 3.0.0.32483 under Fedora with gcc 6 : |
Commented by: @AlexPeshkoff Now FB successfully builds with gcc versions 6, 7 & 8. |
Modified by: @AlexPeshkoffstatus: Open [ 1 ] => Resolved [ 5 ] resolution: Fixed [ 1 ] Fix Version: 2.5.9 [ 10862 ] Fix Version: 3.0.5 [ 10885 ] Fix Version: 4.0 Beta 2 [ 10888 ] |
Modified by: @pavel-zotovstatus: Resolved [ 5 ] => Resolved [ 5 ] QA Status: No test => Cannot be tested |
Modified by: @pavel-zotovstatus: Resolved [ 5 ] => Closed [ 6 ] |
Submitted by: @mkubecek
Votes: 3
Note: it will still take some time for gcc6 to reach distributions so this is not really urgent but it would be nice to be prepared.
Main change is default C++ standard: while gcc4 and gcc5 default to -std=gnu++98 (C++98 with some GNU specific extensions), gcc6 defaults to -std=gnu++14 (C++14 with some GNU extensions). This brings some changes, some subtle, some less so. Now when we have a staging project in BuildService, I tried to address some of them (first in 2.5 but 3.0 seems to be show the same problems).
1. invalid implicit conversion of 'false' to 'bool*' in par_primitive_value() (src/gpre/sqe.cpp)
A trivial mistake from replacing unsigned short with bool in 2003, fixed in 2.5 SVN recently, in 3.0 since 2011.
2. Error "declaration of operator new had different exception specifier" (src/common/classes/alloc.cpp)
Can be either worked around by explicitely switching to C++98 mode (-std=c++98) or (as suggested by our toolchain developers) fixed by dropping the redeclaration in alloc.h and exception specifier in alloc.cpp. The result builds with gcc 4.8, gcc5 and gcc6 but I'm not sure if this is OK with other compilers (like MSVC).
For details, see https://bugzilla.suse.com/show_bug.cgi?id=964466#c4
3. Once these two are addressed, "../gen/firebird/bin/create_db empty.fdb" fails with "invalid request BLR at offset 24"
Additional message says "Too many Contexts of Relation/Procedure/Views. Maximum allowed is 255" which is really strange as, according to progam exit code, it fais on isc_create_database() call. No idea so far. Another strange part is that with "--enable-debug", the whole build succeeds (once the following issues are also addressed).
4. Possibly undefined expression in shutdown() (src/jrd/shut.cpp)
In 'for (Attachment* attachment = attachment = dbb->dbb_attachments; ... )', the extra '= attachment' should be dropped, I believe.
5. Warming: narrowing conversion ... inside { }
Some of these come from ODS_8_0 and friends being USHORT but used to initialize UCHAR fields in various structures. IMHO there would be no harm to declare them as UCHAR as they fit into 8 bits and if they didn't (ODS 16?), the implicit conversion would be lossy.
Similar problem in get_prot_mask() (src/dudley/exe.epp) where blr array of SCHAR is initialized with UCHAR constant (blr_end); quick fix is to declare blr[] as UCHAR and add a cast when it's passed to isc_compile_request2().
6. warning: deleting 'void*' is undefined in Database::~Database() (src/jrd/Database.cpp)
Tried to cast to 'SORTP*', seems to work.
7. warning: 'operator new' must not return NULL unless it is declared 'throw()' (or -fcheck-new is in effect)
Two occurences in class pool_alloc and three in class pool_alloc_rpt (both src/include/fb_blk.h). Adding "throw()" specifier seems to work.
8. warning: invalid suffix on literal; C++11 requires a space between literal and string macro
In constructions like "%"SLONGFORMAT",", the macro should be separated from string literals by a space. Non-fatal but the warnings are annoying (and make spotting the more important issues harder). Trivial to fix, however.
Right now, most critical seems issue 3, this will need more investigation.
To minimize changes to 2.5 code, we might try building with explicit -std=c++98 but that one fails for me with strange error about the way FB_VA_COPY macro is used in AbstractString::vprintf() (src/common/classes/fb_string.cpp).
Commits: 1abb10f 9ae426a
The text was updated successfully, but these errors were encountered: