Details

    • Bug
    • Status: Closed (View Workflow)
    • Major
    • Resolution: Fixed
    • 10.1.22
    • 10.1.24
    • Compiling, Server, Tests
    • None
    • Fedora 25

    Description

      Hello,
      I created this bug as a tracker of various issues, that emerged from use of GCC 7 instead of GCC 6.3

      1) I wanted to use debug build, but it FTBFS because of compile time error.

      error: 'fts_ast_node_type_get' was not declared in this scope

      and simmilar, the build log can be found here

      2) When built with GCC7, about 120 new tests start to fail.

      To check, if it is caused by GCC, I made 2 containers in which I tried to get as close to Fedora Koji build system machines, as possible. Those 2 containers then differ only in version of GCC and its dependecies.

      I attached list of packages, build logs and lists of failing tests from both containers.

      • files with '_gcc' are after GCC7 upgrade
      • failing tests list from 'fail_docker_25' are long term failing, I don't need to solve them now.

      The source RPM, that I rebuilt, can be found here

      Attachments

        1. fail_docker_25
          0.5 kB
        2. fail_docker_25_gcc
          7 kB
        3. log_docker_25
          5.69 MB
        4. log_docker_25_gcc
          7.54 MB
        5. pkgs_docker_25
          8 kB
        6. pkgs_docker_25_gcc
          8 kB

        Issue Links

          Activity

            mschorm Michal Schorm added a comment -

            I filed bug against GCC7 on Fedora here.

            mschorm Michal Schorm added a comment - I filed bug against GCC7 on Fedora here .

            I've compiled MariaDB with gcc 7.0.1-0.16

            $ gcc --version
            gcc (GCC) 7.0.1 20170425 (Red Hat 7.0.1-0.16)
            

            And I am not getting any failures on my end on a debug build:
            MariaDB Version 10.1.22-MariaDB-debug

            On the other hand, switching to a release build, everything changes.

            Taking a simple test case main.range, I get a crash in xtradb only (consistent with your experience). I've attached a GDB to it. Code crashes in:

            storage/xtradb/btr/btr0cur.cc:1562
             
            	if (UNIV_UNLIKELY(thr && thr_get_trx(thr)->fake_changes)) {
            		/* skip CHANGE, LOG */
            		*big_rec = big_rec_vec;
            		return(err); /* == DB_SUCCESS */
            	}
            

            With a dereference of a nullptr thr. This should not happen, as the first part of the condition guards against it.

            So in this case it is definitely a compiler bug. Assembly debugging confirms:

            # No relevant asm instructions before this.
            mov    0xe0(%rsp),%rax   # $rsp + 0xe0 is the address of thr
            mov    0x48(%rax),%rax   # Here we attempt to load thr->graph and crash
            

            cvicentiu VicenÈ›iu Ciorbaru added a comment - I've compiled MariaDB with gcc 7.0.1-0.16 $ gcc --version gcc (GCC) 7.0.1 20170425 (Red Hat 7.0.1-0.16) And I am not getting any failures on my end on a debug build: MariaDB Version 10.1.22-MariaDB-debug On the other hand, switching to a release build, everything changes. Taking a simple test case main.range, I get a crash in xtradb only (consistent with your experience). I've attached a GDB to it. Code crashes in: storage/xtradb/btr/btr0cur.cc:1562   if (UNIV_UNLIKELY(thr && thr_get_trx(thr)->fake_changes)) { /* skip CHANGE, LOG */ *big_rec = big_rec_vec; return (err); /* == DB_SUCCESS */ } With a dereference of a nullptr thr. This should not happen, as the first part of the condition guards against it. So in this case it is definitely a compiler bug. Assembly debugging confirms: # No relevant asm instructions before this. mov 0xe0(%rsp),%rax # $rsp + 0xe0 is the address of thr mov 0x48(%rax),%rax # Here we attempt to load thr->graph and crash

            Turns out that it's not a compiler bug. _attribute_(nonnull) can affect what optimizations compiler does.

            Dirty fix is to remove all MY_ATTRIBUTES from xtradb.

            The file storage/xtradb/include/univ.i

            @@ -261,7 +261,7 @@
             that are only referenced from within InnoDB, not from MySQL. We disable the
             GCC visibility directive on all Sun operating systems because there is no
             easy way to get it to work. See http://bugs.mysql.com/bug.php?id=52263. */
            -#define MY_ATTRIBUTE __attribute__
            +#define MY_ATTRIBUTE(X) /* empty */
             #if defined(__GNUC__) && (__GNUC__ >= 4) && !defined(sun) || defined(__INTEL_COMPILER)
             # define UNIV_INTERN MY_ATTRIBUTE((visibility ("hidden")))
             #else
            

            cvicentiu Vicențiu Ciorbaru added a comment - Turns out that it's not a compiler bug. _ attribute _(nonnull) can affect what optimizations compiler does. Dirty fix is to remove all MY_ATTRIBUTES from xtradb. The file storage/xtradb/include/univ.i @@ -261,7 +261,7 @@ that are only referenced from within InnoDB, not from MySQL. We disable the GCC visibility directive on all Sun operating systems because there is no easy way to get it to work . See http://bugs.mysql.com/bug.php?id=52263. */ -#define MY_ATTRIBUTE __attribute__ +#define MY_ATTRIBUTE(X) /* empty */ # if defined(__GNUC__) && (__GNUC__ >= 4) && !defined(sun) || defined(__INTEL_COMPILER) # define UNIV_INTERN MY_ATTRIBUTE((visibility ( "hidden" ))) # else

            CC marko This may be something for you to consider fixing later.

            cvicentiu Vicențiu Ciorbaru added a comment - CC marko This may be something for you to consider fixing later.
            mschorm Michal Schorm added a comment -

            I enabled '--big-test' option for the testsuite
            I made some builds with the patch for the file storage/xtradb/include/univ.i

            So far, it looks like the issue has been solved.
            Tests fails by hundreds no more and I haven't encountered any other similar issue.
            I'm going to propose builds with the patch.

            I want to point out, that even though the 'storage/xtradb/include/univ.i' file has been patched, the same line of code appears on one more place:

            # grep -R "MY_ATTRIBUTE __attribute__" mariadb-10.1.23/
            mariadb-10.1.23/storage/xtradb/include/univ.i:#define MY_ATTRIBUTE __attribute__
            mariadb-10.1.23/storage/innobase/include/univ.i:#define MY_ATTRIBUTE __attribute__
            

            However the compilation seems to work fine even if only xtradb file is patched.

            mschorm Michal Schorm added a comment - I enabled '--big-test' option for the testsuite I made some builds with the patch for the file storage/xtradb/include/univ.i So far, it looks like the issue has been solved. Tests fails by hundreds no more and I haven't encountered any other similar issue. I'm going to propose builds with the patch. I want to point out, that even though the 'storage/xtradb/include/univ.i' file has been patched, the same line of code appears on one more place: # grep -R "MY_ATTRIBUTE __attribute__" mariadb-10.1.23/ mariadb-10.1.23 /storage/xtradb/include/univ .i: #define MY_ATTRIBUTE __attribute__ mariadb-10.1.23 /storage/innobase/include/univ .i: #define MY_ATTRIBUTE __attribute__ However the compilation seems to work fine even if only xtradb file is patched.
            mschorm Michal Schorm added a comment -

            I made group of builds and unfortunatelly, it seems, many tests keep failing against the 10.1.21 version.

            Here is example build log, with '--big-test' disabled

            Again, there is huge difference in amount of tests failing in GCC7 builds (up to 100 failing tetsts) and GCC6 builds (up to 12 failing tests)

            mschorm Michal Schorm added a comment - I made group of builds and unfortunatelly, it seems, many tests keep failing against the 10.1.21 version. Here is example build log, with '--big-test' disabled Again, there is huge difference in amount of tests failing in GCC7 builds (up to 100 failing tetsts) and GCC6 builds (up to 12 failing tests)
            danblack Daniel Black added a comment -

            I think I tried 7.1 yesterday against 10.2. And didn't have significant troubles - still a few too many warning however generally isolated to mgrooma and connect.

            danblack Daniel Black added a comment - I think I tried 7.1 yesterday against 10.2. And didn't have significant troubles - still a few too many warning however generally isolated to mgrooma and connect.
            danblack Daniel Black added a comment -

            I tried again with gcc-7.1.0 and gcc-7.1.1(from branch 7 head) and had no troubles on 10.2 however quickly hit the test 40 failures (my limit) on 10.1 branch. This is on Ubuntu-16.04 x86_64 and ppc64le

            danblack Daniel Black added a comment - I tried again with gcc-7.1.0 and gcc-7.1.1(from branch 7 head) and had no troubles on 10.2 however quickly hit the test 40 failures (my limit) on 10.1 branch. This is on Ubuntu-16.04 x86_64 and ppc64le
            mschorm Michal Schorm added a comment -

            I will make new analysis, when I catch some time.
            I'll post the results here later.

            mschorm Michal Schorm added a comment - I will make new analysis, when I catch some time. I'll post the results here later.

            I fixed some GCC 7 warnings, but did not yet attack the MY_ATTRIBUTE problem that cvicentiu mentioned.
            Most notably, -Wimplicit-fallthrough does identify genuine bugs. I plan to silence bogus -Wimplicit-fallthrough warnings in MariaDB 10.0 and later.

            marko Marko Mäkelä added a comment - I fixed some GCC 7 warnings, but did not yet attack the MY_ATTRIBUTE problem that cvicentiu mentioned. Most notably, -Wimplicit-fallthrough does identify genuine bugs. I plan to silence bogus -Wimplicit-fallthrough warnings in MariaDB 10.0 and later.
            danblack Daniel Black added a comment -

            Thanks for attacking -Wimplicit-fallthough. I picked up on https://github.com/codership/galera/issues/464 and https://github.com/codership/galera/issues/465 doing the same fixes in galera which looks much more serious.

            danblack Daniel Black added a comment - Thanks for attacking -Wimplicit-fallthough. I picked up on https://github.com/codership/galera/issues/464 and https://github.com/codership/galera/issues/465 doing the same fixes in galera which looks much more serious.
            danblack Daniel Black added a comment -

            The remaining attribute fixes are the ones provided by Google in patches in MDEV-9998 over a year ago

            The header changes in:

            https://github.com/MariaDB/server/pull/341/files#diff-db850e092e18ea7b5b4fb9872ee2c3ce

            danblack Daniel Black added a comment - The remaining attribute fixes are the ones provided by Google in patches in MDEV-9998 over a year ago The header changes in: https://github.com/MariaDB/server/pull/341/files#diff-db850e092e18ea7b5b4fb9872ee2c3ce

            I pushed the silencing of bogus -Wimplicit-fallthrough warnings to 10.0, and filed MDEV-12824 for the remaining ones (some of which definitely look like bugs).
            I pushed the following to bb-10.0-marko and intend to push to 10.0:
            739b47e220a MDEV-12358 Work around what looks like a bug in GCC 7.1.0
            e22d86a3eb8 fil_create_new_single_table_tablespace(): Correct a bogus nonnull attribute
            956d2540c45 Remove redundant UT_LIST_INIT() calls
            The first and last are working around what I think are GCC 7 -O3 code generation bugs (Debian 7.1.0-5).
            The second one is fixing a bug that was reported by a GCC 7 warning.

            marko Marko Mäkelä added a comment - I pushed the silencing of bogus -Wimplicit-fallthrough warnings to 10.0, and filed MDEV-12824 for the remaining ones (some of which definitely look like bugs). I pushed the following to bb-10.0-marko and intend to push to 10.0: 739b47e220a MDEV-12358 Work around what looks like a bug in GCC 7.1.0 e22d86a3eb8 fil_create_new_single_table_tablespace(): Correct a bogus nonnull attribute 956d2540c45 Remove redundant UT_LIST_INIT() calls The first and last are working around what I think are GCC 7 -O3 code generation bugs (Debian 7.1.0-5). The second one is fixing a bug that was reported by a GCC 7 warning.

            ok to push above ones ^

            jplindst Jan Lindström (Inactive) added a comment - ok to push above ones ^
            marko Marko Mäkelä added a comment - - edited

            To get something good out of GCC 7.1.0, I compiled the debug version with -O3 -fsanitize=undefined, ran the InnoDB tests and fixed all InnoDB errors:
            cbb6db8c0d7 Fix some _attribute_((nonnull)) misuse
            e42a002c2fe ibuf_get_volume_buffered_hash(): Use a proper type cast
            5bc1ed8ba74 Simplify trx_undo_report_row_operation()
            8b34aabf86b Follow-up to MDEV-12534: Align srv_sys

            marko Marko Mäkelä added a comment - - edited To get something good out of GCC 7.1.0, I compiled the debug version with -O3 -fsanitize=undefined, ran the InnoDB tests and fixed all InnoDB errors: cbb6db8c0d7 Fix some _ attribute _((nonnull)) misuse e42a002c2fe ibuf_get_volume_buffered_hash(): Use a proper type cast 5bc1ed8ba74 Simplify trx_undo_report_row_operation() 8b34aabf86b Follow-up to MDEV-12534 : Align srv_sys

            ol to push above ones ^ (one could argue about 5bc simplify claim but I'm ok with it).

            jplindst Jan Lindström (Inactive) added a comment - ol to push above ones ^ (one could argue about 5bc simplify claim but I'm ok with it).

            I pushed the above-mentioned commits to 10.0 (replacing the word "Simplify" with "Refactor" in one commit comment).
            I would expect these to be merged to 10.1 or 10.2 within a week or two.

            marko Marko Mäkelä added a comment - I pushed the above-mentioned commits to 10.0 (replacing the word "Simplify" with "Refactor" in one commit comment). I would expect these to be merged to 10.1 or 10.2 within a week or two.

            I pushed a tentative merge to bb-10.1-marko and compiled and tested it as follows. No UBSAN warnings were reported for InnoDB or XtraDB:

            mkdir build
            cd build
            CC=gcc-7 CXX=g++-7 cmake -DWITH_EMBEDDED_SERVER=OFF -DWITH_UNIT_TESTS=OFF -DCMAKE_BUILD_TYPE=Debug -DCMAKE_{C,CXX}_FLAGS_DEBUG='-g -O3 -fsanitize=undefined' -DPLUGIN_TOKUDB=NO -DPLUGIN_MROONGA=NO -DPLUGIN_OQGRAPH=NO -DPLUGIN_ROCKSDB=NO -DWITH_SSL=bundled ..
            make -kj4
            cd mysql-test
            ./mtr --mem --force --parallel=auto --suite=innodb,innodb_fts,innodb_zip,encryption
            grep 'storage/[ix].*runtime' var/*/log/mysqld*err*
            

            Note: at least one 10.1 target will fail to build with GCC 7.1.0 in a Debian unstable/experimental chroot. In 10.0, storage/connect would fail to build with a similar message:

            In file included from /mariadb/10.1-gcc7/zlib/zconf.h:12:0,
                             from /mariadb/10.1-gcc7/zlib/zlib.h:34,
                             from /mariadb/10.1-gcc7/extra/mariabackup/crc/crc_glue.c:25:
            /mariadb/10.1-gcc7/build/include/config.h:690:2: error: #error <my_config.h> MUST be included first!
            

            marko Marko Mäkelä added a comment - I pushed a tentative merge to bb-10.1-marko and compiled and tested it as follows. No UBSAN warnings were reported for InnoDB or XtraDB: mkdir build cd build CC=gcc-7 CXX=g++-7 cmake -DWITH_EMBEDDED_SERVER=OFF -DWITH_UNIT_TESTS=OFF -DCMAKE_BUILD_TYPE=Debug -DCMAKE_{C,CXX}_FLAGS_DEBUG='-g -O3 -fsanitize=undefined' -DPLUGIN_TOKUDB=NO -DPLUGIN_MROONGA=NO -DPLUGIN_OQGRAPH=NO -DPLUGIN_ROCKSDB=NO -DWITH_SSL=bundled .. make -kj4 cd mysql-test ./mtr --mem --force --parallel=auto --suite=innodb,innodb_fts,innodb_zip,encryption grep 'storage/[ix].*runtime' var/*/log/mysqld*err* Note: at least one 10.1 target will fail to build with GCC 7.1.0 in a Debian unstable/experimental chroot. In 10.0, storage/connect would fail to build with a similar message: In file included from /mariadb/10.1-gcc7/zlib/zconf.h:12:0, from /mariadb/10.1-gcc7/zlib/zlib.h:34, from /mariadb/10.1-gcc7/extra/mariabackup/crc/crc_glue.c:25: /mariadb/10.1-gcc7/build/include/config.h:690:2: error: #error <my_config.h> MUST be included first!
            mschorm Michal Schorm added a comment -

            Can you point me to some specific GIT point, at which I can download the tarball and try your fixes on Fedora builders?
            Beacause you all looks like it more or less works for you, so I would give you some feedback.
            ... But I'm kinda lost at your GitHub.

            mschorm Michal Schorm added a comment - Can you point me to some specific GIT point, at which I can download the tarball and try your fixes on Fedora builders? Beacause you all looks like it more or less works for you, so I would give you some feedback. ... But I'm kinda lost at your GitHub.

            mschorm, I intend to push the merge to 10.1 soon. Meanwhile, you can experiment with commit ae04f2968ea6 which I linked in my previous comment.

            I also performed a tentative merge to 10.2, but pushing that one could take a week. For 10.2, there were a few follow-up fixes necessary which I hope jplindst can review:
            850c39f1cce Remove dict_index_t::is_ngram
            bbe7eb370a2 Remove bogus _attribute_((nonnull))
            7896861d385 Fix a compilation warning
            4fb9a820e51 Fix a compilation warning introduced by MDEV-11782
            0debfe196ef Remove unnecessary conversions from integer to double
            78df4a758d7 Fix some -Wimplicit-fallthrough warnings in InnoDB

            marko Marko Mäkelä added a comment - mschorm , I intend to push the merge to 10.1 soon. Meanwhile, you can experiment with commit ae04f2968ea6 which I linked in my previous comment. I also performed a tentative merge to 10.2, but pushing that one could take a week. For 10.2, there were a few follow-up fixes necessary which I hope jplindst can review: 850c39f1cce Remove dict_index_t::is_ngram bbe7eb370a2 Remove bogus _ attribute _((nonnull)) 7896861d385 Fix a compilation warning 4fb9a820e51 Fix a compilation warning introduced by MDEV-11782 0debfe196ef Remove unnecessary conversions from integer to double 78df4a758d7 Fix some -Wimplicit-fallthrough warnings in InnoDB

            ok to push bbe73, 78968, 4fb9a, 0debf, 78df4. 850c3 requires some explanation to commit message before I can review.

            jplindst Jan Lindström (Inactive) added a comment - ok to push bbe73, 78968, 4fb9a, 0debf, 78df4. 850c3 requires some explanation to commit message before I can review.

            mschorm, I pushed a merge to 10.1. It does not fix the my_config.h issue yet; that you should be able to work around by adding

            #include "my_config.h"
            

            to the start of the affected files.

            marko Marko Mäkelä added a comment - mschorm , I pushed a merge to 10.1. It does not fix the my_config.h issue yet; that you should be able to work around by adding #include "my_config.h" to the start of the affected files.

            I pushed a merge to 10.2 as well.

            marko Marko Mäkelä added a comment - I pushed a merge to 10.2 as well.
            mschorm Michal Schorm added a comment -

            I tried to build MariaDB on Fedora from the commit in this comment Marko.
            And it looks like the issue with hundreds test failing has been solved.

            Great work!

            mschorm Michal Schorm added a comment - I tried to build MariaDB on Fedora from the commit in this comment Marko. And it looks like the issue with hundreds test failing has been solved. Great work!
            mschorm Michal Schorm added a comment -

            MariaDB 10.1.24 is now in stable repos in Fedora.
            The GCC issue looks resolved - good job!

            However, I'm still not able to make a debug build. But that is different issue, so I should probabbly create a new JIRA ticket - shouldn't I?

            mschorm Michal Schorm added a comment - MariaDB 10.1.24 is now in stable repos in Fedora. The GCC issue looks resolved - good job! However, I'm still not able to make a debug build. But that is different issue, so I should probabbly create a new JIRA ticket - shouldn't I?
            danblack Daniel Black added a comment -

            Yeh, I'd do a new ticket. You seem comfortable that all gcc-7 issues raised here been resolved.

            Regarding debug builds look at MDEV-4646 as this introduces a CMake based RPM build of debug symbol packages.

            danblack Daniel Black added a comment - Yeh, I'd do a new ticket. You seem comfortable that all gcc-7 issues raised here been resolved. Regarding debug builds look at MDEV-4646 as this introduces a CMake based RPM build of debug symbol packages.

            People

              marko Marko Mäkelä
              mschorm Michal Schorm
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.