Skip to content
Snippets Groups Projects
  1. Jul 04, 2019
  2. Jul 03, 2019
    • Alexander Turenko's avatar
      test: update test-run · 6a0b0cff
      Alexander Turenko authored
      Fixed app/strict.test.lua fail after a test that sets 'a' global
      variable. It was due to the way how 'strict' module tracks declared
      global variables and was fixed in pretest_clean.lua test-run's module.
      
      (cherry picked from commit 1df9c3d2)
      6a0b0cff
  3. Jul 01, 2019
  4. Jun 25, 2019
    • Sergei Voronezhskii's avatar
      test: enable parallel run for python test suites · 759f0a80
      Sergei Voronezhskii authored
      Fixed issues:
      
      - box-py/iproto.test.py
        1) Fixed receive_response() to wait for whole response.
        2) Clean up _cluster space.
      
      - replication-py/cluster.test.py
        1) Clean up _cluster space.
      
      - replication-py/multi.test.py
        1) Removed vclock checking because it fails if previous test make some
           DML and vclock was incremented. Looks like it was used for debug
           and is not part of this test case.
        2) Fixed typo in 'Synchronize' block.
      
      The following test sequences did fail due to unexpected IDs in _cluster
      space:
      
      - [box-py/iproto.test.py, null]
      - [box-py/bootstrap.test.py, null]
      
      - [replication-py/cluster.test.py, null]
      - [replication-py/multi.test.py, null]
      
      Part of #3232
      
      (cherry picked from commit 4ea7d729)
      759f0a80
    • Sergei Voronezhskii's avatar
      test: skip test backtrace if no libunwind support · 67fda078
      Sergei Voronezhskii authored
      Closes #3824
      
      (cherry picked from commit 2aa25ba5)
      67fda078
    • Vladimir Davydov's avatar
      vinyl: free region on vylog commit instead of resetting it · db40dec3
      Vladimir Davydov authored
      region_reset() only frees memory from the last slab. As a result, if
      a vylog transaction happens to use more than one slab, memory used by
      vy_log.pool won't be freed, neither will it be reused, i.e. we'll get
      a memory leak. Fix it by using region_free() instead of region_reset().
      It's okay from performance point of view, because vylog transactions
      are rare events.
      
      Note, the master branch isn't affected to this issue, because the vylog
      memory management was completely reworked there. 2.1 isn't affected
      either, because there region_reset() was modified to free all slabs.
      However, rather than backporting any of those commits, I think it's
      more appropriate to simply use region_free().
      db40dec3
    • Vladimir Davydov's avatar
      vinyl: clean up region after allocating surrogate statement · 0e7c933d
      Vladimir Davydov authored
      vy_stmt_new_surrogate_from_key() and vy_stmt_new_surrogate_delete_raw()
      allocate temporary objects on the region, but don't clean up after
      themselves. Those functions may be called by a vinyl reader threads:
      
        vy_page_read_cb
          vy_page_find_key
            vy_page_stmt
              vy_stmt_decode
      
      In this case the region will grow infinitely, because reader threads
      never call fiber_gc().
      
      The leak was introduced to 1.10 by commit b9072317 ("vinyl: lookup
      key in reader thread"), which moved vy_page_find_key() invocation to
      reader threads for the sake of performance. The fix is trivial - call
      region_truncate() from those functions.
      
      Note, neither the master branch nor 2.1 is affected to this issue,
      because region_truncate() was added there in the scope of the multikey
      index feature.
      0e7c933d
  5. Jun 19, 2019
    • Alexander Turenko's avatar
      test: update test-run · 0acce52a
      Alexander Turenko authored
      Add 'decimal' to a built-in modules list to preserve
      package.loaded.decimal when pretest_clean is set in suite.ini.
      
      Needed for #692.
      
      (cherry picked from commit 0b7cc526)
      0acce52a
  6. Jun 18, 2019
    • Alexander Turenko's avatar
      lua: escape trigraphs in bundled lua sources · ed050296
      Alexander Turenko authored
      Built-in modules are bundled into tarantool in the following way. A lua
      file from src/lua or src/box/lua is stored as a string literal in a C
      file, then built and linked into tarantool. During startup tarantool
      calls luaL_loadbuffer() on this string.
      
      When a Lua source is converted to a C literal, proper escaping is
      performed. However there is one case, which was not covered: trigraphs.
      The patch adds escaping of question mark symbols to avoid matching ??X
      sequences as trigraphs by C preprocessor.
      
      The most simple way to check that it works is to apply the following
      patch:
      
       | diff --git a/src/lua/string.lua b/src/lua/string.lua
       | index 6e12c59ae..2da2dbf4d 100644
       | --- a/src/lua/string.lua
       | +++ b/src/lua/string.lua
       | @@ -425,3 +425,6 @@ string.fromhex    = string_fromhex
       |  string.strip      = string_strip
       |  string.lstrip      = string_lstrip
       |  string.rstrip      = string_rstrip
       | +string.foo = function()
       | +    return '??('
       | +end
      
      And call the function like so:
      
       | ./src/tarantool -e 'print(string.foo()) os.exit()'
      
      If it printfs `??(`, then everything is okay. If it prints `[`, then
      `??(` was preprocessed as the trigraph.
      
      We hit this problem when tried to bundle luarocks-3: it contains
      "^(.-)(%??)$" regexp, where `??)` was interpreted as `]`. Debug build or
      a build with -DENABLE_WERROR reports an error in the case, but usual
      RelWithDebInfo build passes (with -Wtrigraphs warnings) and can show
      this unexpected behaviour.
      
      Fixes #4291.
      
      (cherry picked from commit 177a1713)
      ed050296
    • Vladimir Davydov's avatar
      vinyl: fix assertion failure in vy_tx_handle_deferred_delete · c5232d68
      Vladimir Davydov authored
      vy_tx_handle_deferred_delete() expects (righteously) that in case a
      deferred DELETE overwrites a statement in a secondary index write set
      and the overwritten statement wasn't skipped on commit (i.e. doesn't
      have txv->is_overwritten flag set), both the old and the new statement
      must be REPLACEs (see the comment to the corresponding assertions for
      more details).
      
      The problem is we don't set is_overwritten flag in case a statement
      doesn't have any effect (txv->is_nop is set), even if it was, in fact,
      overwritten in the primary index write set (see vy_tx_prepare). As
      a result, we get an assertion failure when we delete such statement
      in the same transaction, e.g.
      
        s = box.schema.space.create('test', {engine = 'vinyl'})
        s:create_index('pk', {parts = {1, 'unsigned'}})
        s:create_index('sk', {parts = {2, 'unsigned'}})
        s:replace{1, 1, 1}
        box.begin()
        s:update(1, {{'+', 3, 1}}) -- adds no-op REPLACE to sk write set
        s:delete(1)                -- produces deferred DELETE for sk
        box.commit()
      
      results in
      
        vy_tx_handle_deferred_delete: Assertion `vy_stmt_type(stmt) == IPROTO_REPLACE' failed.
      
      Fix this by making sure we set is_overwritten for all overwritten
      statements in a secondary index write set.
      
      Closes #4294
      
      (cherry picked from commit 41219774)
      c5232d68
    • Alexander V. Tikhonov's avatar
      Fix static build · ee27c506
      Alexander V. Tikhonov authored
      Added to cmake environment CMAKE_DL_LIBS (The name of the library
      that has dlopen and dlclose in it, usually -ldl) to openssl build
      to add DL library, to fix the following fails:
      
      Linking CXX executable crypto.test
      /usr/local/lib64/libcrypto.a(dso_dlfcn.o): In function `dlfcn_globallookup':
      dso_dlfcn.c:(.text+0x11): undefined reference to `dlopen'
      dso_dlfcn.c:(.text+0x24): undefined reference to `dlsym'
      dso_dlfcn.c:(.text+0x2f): undefined reference to `dlclose'
      /usr/local/lib64/libcrypto.a(dso_dlfcn.o): In function `dlfcn_bind_func':
      dso_dlfcn.c:(.text+0x334): undefined reference to `dlsym'
      dso_dlfcn.c:(.text+0x3f2): undefined reference to `dlerror'
      /usr/local/lib64/libcrypto.a(dso_dlfcn.o): In function `dlfcn_load':
      dso_dlfcn.c:(.text+0x459): undefined reference to `dlopen'
      dso_dlfcn.c:(.text+0x4c9): undefined reference to `dlclose'
      dso_dlfcn.c:(.text+0x502): undefined reference to `dlerror'
      /usr/local/lib64/libcrypto.a(dso_dlfcn.o): In function `dlfcn_pathbyaddr':
      dso_dlfcn.c:(.text+0x5a1): undefined reference to `dladdr'
      dso_dlfcn.c:(.text+0x601): undefined reference to `dlerror'
      /usr/local/lib64/libcrypto.a(dso_dlfcn.o): In function `dlfcn_unload':
      dso_dlfcn.c:(.text+0x662): undefined reference to `dlclose'
      collect2: error: ld returned 1 exit status
      make[2]: *** [test/unit/crypto.test] Error 1
      make[1]: *** [test/unit/CMakeFiles/crypto.test.dir/all] Error 2
      
      Closes #4245
      
      (cherry picked from commit 31ab6ea9)
      ee27c506
  7. Jun 14, 2019
    • Alexander Turenko's avatar
      test: update test-run · 154d4f52
      Alexander Turenko authored
      Run collectgarbage() between tests to ensure that there are no dangling
      iterators and so on. Such objects can affect statistic counters that may
      be important for a test.
      
      Fixes #4287.
      
      (cherry picked from commit 0cbcf265)
      154d4f52
  8. Jun 11, 2019
    • Ilya Konyukhov's avatar
      Adds a function to determine current source path · 6a7913d5
      Ilya Konyukhov authored
      Currently to determine current source path, user has to do it
      by getting current source info from the stack and parse it manually
      
      This commit adds couple little helpers for this task called
      `debug.sourcefile()` and `debug.sourcedir()`,
      which returns a path to the file being executed.
      
      The path is relative to the current working directory.
      If such path cannot be determined, '.' is returned (i.e. interactive
      mode). There is an exception if you are running script with an absolute
      path. In that case, absolute path will be returned too.
      
      ```bash
      tarantool /absolute/path/to/the/script.lua
      ```
      
      There are also handy shortcuts for that functions: `debug.__file__` and
      `debug.__dir__`
      
      @TarantoolBot document
      Title: Document new debug helper methods
      
      There are two more helpers in debug module `sourcefile()` and
      `sourcedir()`. This two helpers returns relative paths to source file
      and directory respectively.
      
      There are also `debug.__file__` and `debug.__dir__` which are just handy
      ways to call `sourcefile()` and `sourcedir()`.
      
      It should be mentioned that it is only possible to determine real path
      to the file, if the function was defined in a some lua file.
      `loadstring` may be exception here, since lua will store whole string
      in debug info.
      
      There is also a possible pitfall because of tail call optimization, so
      if the function `fn` is defined inside `myapp/init.lua` like this:
      
      ```lua
      function fn()
          return debug.sourcefile()
      end
      ```
      
      It would not be possible to determine file path correctly because that
      function would not appear in a stacktrace. Even worse, it may return
      wrong result, i.e. actual path where "parent" function was defined.
      
      To force the interpreter to avoit this kind of optimizations you may use
      parenthesis like this:
      
      ```lua
      function fn()
          return (debug.sourcefile())
      end
      ```
      
      Also both `sourcefile` and `sourcedir` functions have optional level
      argument, which allows set the level of the call stack function should
      examine for source path. By default, 2 is used.
      
      (cherry picked from commit 70b8583c)
      6a7913d5
  9. Jun 06, 2019
    • Vladimir Davydov's avatar
      vinyl: don't purge deleted runs from vylog on compaction · beb4c0ce
      Vladimir Davydov authored
      After compacting runs, we first mark them as dropped (VY_LOG_DROP_RUN),
      then try to delete their files unless they are needed for recovery from
      the checkpoint, and finally mark them as not needed in the vylog
      (VY_LOG_FORGET_RUN). There's a potential race sitting here: the problem
      is the garbage collector might kick in after files are dropped, but
      before they are marked as not needed. If this happens, there will be
      runs that have two VY_LOG_FORGET_RUN records, which will break recovery:
      
        Run XX is forgotten, but not registered
      
      The following patches make the race more likely to happen so let's
      eliminate it by making the garbage collector the only one who can mark
      runs as not needed (i.e. write VY_LOG_FORGET_RUN record). There will
      be no warnings, because the garbage collector silently ignores ENOENT
      errors, see vy_gc().
      
      Another good thing about this patch is that now we never yield inside
      a vylog transaction, which makes it easier to remove the vylog latch
      blocking implementation of transactional DDL.
      
      (cherry picked from commit 30c9c7c0)
      beb4c0ce
  10. Jun 04, 2019
    • Serge Petrenko's avatar
      test: fix box/on_replace failure on consequent runs. · a978d0d6
      Serge Petrenko authored
      Example:
      ```
      [001] box/on_replace.test.lua                                         [ pass ]
      [001] box/on_replace.test.lua                                         [ fail ]
      [001]
      [001] Test failed! Result content mismatch:
      [001] --- box/on_replace.result	Tue Jun  4 10:44:39 2019
      [001] +++ box/on_replace.reject	Tue Jun  4 10:44:50 2019
      [001] @@ -4,7 +4,7 @@
      [001]  -- test c and lua triggers: must return only lua triggers
      [001]  #box.space._space:on_replace()
      [001]  ---
      [001] -- 0
      [001] +- 2
      [001]  ...
      ```
      This happened because we forgot to remove triggers set on _space.
      Clear them below the test case.
      
      (cherry picked from commit d3552264)
      a978d0d6
  11. Jun 03, 2019
    • Serge Petrenko's avatar
      lua: fix operation type passing to on_replace triggers · c9cd8301
      Serge Petrenko authored
      This commit fixes a regression introduced by commit
      5ab0763b
      (pass operation type to lua triggers)
      
      When passing operation type to the trigger, we relied on a corresponding
      xrow_header, which would later be written to the log. When before_replace
      triggers are fired, there is no row connected to the txn statement yet,
      so we faked one to pass operations. We, however, didn't account for
      temporary spaces, which never have a row associated with a corresponding
      txn stmt. This lead to a segfault on an attemt to use on_replace triggers
      with temporary spaces.
      
      Add a fake row for temporary spaces to pass operation type in on_replace
      triggers and add some tests.
      
      Closes #4266
      
      (cherry picked from commit 63ebc25a)
      c9cd8301
  12. May 31, 2019
    • Vladimir Davydov's avatar
      vinyl: don't throttle DDL · eacf912c
      Vladimir Davydov authored
      Since DDL is triggered by the admin, it can be deliberately initiated
      when the workload is known to be low. Throttling it along with DML
      requests would only cause exasperation in this case. So we don't apply
      disk-based rate limit to DDL. This should be fine, because the
      disk-based limit is set rather strictly to let the workload some space
      to grow, see vy_regulator_update_rate_limit(), and in contrast to the
      memory-based limit, exceeding the disk-based limit doesn't result in
      abrupt stalls - it may only lead to a gradual accumulation of disk space
      usage and read latency.
      
      Closes #4238
      
      (cherry picked from commit 6a44ac62)
      eacf912c
  13. May 30, 2019
    • Vladimir Davydov's avatar
      vinyl: lookup key in reader thread · b9072317
      Vladimir Davydov authored
      If a key isn't found in the tuple cache, we fetch it from a run file. In
      this case disk read and page decompression is done by a reader thread,
      however key lookup in the fetched page is still performed by the tx
      thread. Since pages are immutable, this could as well be done by the
      reader thread, which would allow us to save some precious CPU cycles for
      tx.
      
      Close #4257
      
      (cherry picked from commit 04b19ac1)
      b9072317
    • Vladimir Davydov's avatar
      vinyl: do not allow to cancel a fiber reading a page · 1205c92e
      Vladimir Davydov authored
      To handle fiber cancellation during page read we need to pin all objects
      referenced by vy_page_read_task. Currently, there's the only such
      object, vy_run. It has reference counting so pinning it is trivial.
      However, to move page lookup to a reader thread, we need to also
      reference key def, tuple format, and key. Format and key have reference
      counting, but key def doesn't - we typically copy it. Copying it in this
      case is too heavy.
      
      Actually, cancelling a fiber manually or on timeout while it's reading
      disk doesn't make much sense with PCIE attached flash drives. It used to
      be reasonable with rotating disks, since a rotating disk controller
      could retry reading a block indefinitely on read failure. It is still
      relevant to Network Attached Storage. On the other hand, NAS has never
      been tested, and what isn't tested, can and should be removed. For
      complex SQL queries we'll be forced to rethink timeout handling anyway.
      
      That being said, let's simply drop this functionality.
      
      (cherry picked from commit bab04b25)
      1205c92e
    • Vladimir Davydov's avatar
      vinyl: encapsulate reader thread selection logic in a helper function · 8d7bded3
      Vladimir Davydov authored
      Page reading code is intermixed with the reader thread selection in the
      same function, which makes it difficult to extend the former. So let's
      introduce a helper function encapsulating a call on behalf of a reader
      thread.
      
      (cherry picked from commit d8a95a2a)
      8d7bded3
    • Vladimir Davydov's avatar
      vinyl: pass page info by reference to reader thread · 27fecbfd
      Vladimir Davydov authored
      Since a page read task references the source run file, we don't need to
      pass page info by value.
      
      (cherry picked from commit 67d36ccc)
      27fecbfd
    • Vladimir Davydov's avatar
      vinyl: factor out function to lookup key in page · 776e3c05
      Vladimir Davydov authored
      This function is a part of the run iterator API so we can't use it in
      a reader thread. Let's make it an independent helper. As a good side
      effect, we can now reuse it in the slice stream implementation.
      
      (cherry picked from commit ac8ce023)
      776e3c05
  14. May 29, 2019
  15. May 28, 2019
  16. May 27, 2019
    • Vladimir Davydov's avatar
      vinyl: fix deferred DELETE statement lost on commit · aa4b1021
      Vladimir Davydov authored
      Even if a statement isn't marked as VY_STMT_DEFERRED_DELETE, e.g. it's
      a REPLACE produced by an UPDATE request, it may overwrite a statement in
      the transaction write set that is marked so, for instance:
      
        s = box.schema.space.create('test', {engine = 'vinyl'})
        pk = s:create_index('pk')
        sk = s:create_index('sk', {parts = {2, 'unsigned'}})
      
        s:insert{1, 1}
      
        box.begin()
        s:replace{1, 2}
        s:update(1, {{'=', 2, 3}})
        box.commit()
      
      If we don't mark REPLACE{3,1} produced by the update operatoin with
      VY_STMT_DEFERRED_DELETE flag, we will never generate a DELETE statement
      for INSERT{1,1}. That is, we must inherit the flag from the overwritten
      statement when we insert a new one into a write set.
      
      Closes #4248
      
      (cherry picked from commit b54433d9)
      aa4b1021
    • Vladimir Davydov's avatar
      vinyl: don't produce deferred DELETE on commit if key isn't updated · 61cf6b25
      Vladimir Davydov authored
      Consider the following example:
      
        s = box.schema.space.create('test', {engine = 'vinyl'})
        s:create_index('primary')
        s:create_index('secondary', {parts = {2, 'unsigned'}})
      
        s:insert{1, 1, 1}
        s:replace{1, 1, 2}
      
      When REPLACE{1,1} is committed to the secondary index, the overwritten
      tuple, i.e. INSERT{1,1}, is found in the primary index memory, and so
      deferred DELETE{1,1} is generated right away and committed along with
      REPLACE{1,1}. However, there's no need to commit anything to the
      secondary index in this case, because its key isn't updated. Apart from
      eating memory and loading disk, this also breaks index stats, as vy_tx
      implementation doesn't expect two statements committed for the same key
      in a single transaction.
      
      Fix this by checking if there's a statement in the log for the deleted
      key and if there's skipping them both as we do in the regular case, see
      the comment in vy_tx_set.
      
      Closes #3693
      
      (cherry picked from commit e2f5e1bc)
      61cf6b25
    • Vladimir Davydov's avatar
      vinyl: fix secondary index divergence on update · 0e37af31
      Vladimir Davydov authored
      If an UPDATE request doesn't touch key parts of a secondary index, we
      don't need to re-index it in the in-memory secondary index, as this
      would only increase IO load. Historically, we use column mask set by the
      UPDATE operation to skip secondary indexes that are not affected by the
      operation on commit. However, there's a problem here: the column mask
      isn't precise - it may have a bit set even if the corresponding column
      value isn't changed by the update operation, e.g. consider {'+', 2, 0}.
      Not taking this into account may result in appearance of phantom tuples
      on disk as the write iterator assumes that statements that have no
      effect aren't written to secondary indexes (this is needed to apply
      INSERT+DELETE "annihilation" optimization). We fixed that by clearing
      column mask bits in vy_tx_set in case we detect that the key isn't
      changed, for more details see #3607 and commit e72867cb ("vinyl: fix
      appearance of phantom tuple in secondary index after update"). It was
      rather an ugly hack, but it worked.
      
      However, it turned out that apart from looking hackish this code has
      a nasty bug that may lead to tuples missing from secondary indexes.
      Consider the following example:
      
        s = box.schema.space.create('test', {engine = 'vinyl'})
        s:create_index('pk')
        s:create_index('sk', {parts = {2, 'unsigned'}})
        s:insert{1, 1, 1}
      
        box.begin()
        s:update(1, {{'=', 2, 2}})
        s:update(1, {{'=', 3, 2}})
        box.commit()
      
      The first update operation writes DELETE{1,1} and REPLACE{2,1} to the
      secondary index write set. The second update replaces REPLACE{2,1} with
      DELETE{2,1} and then with REPLACE{2,1}. When replacing DELETE{2,1} with
      REPLACE{2,1} in the write set, we assume that the update doesn't modify
      secondary index key parts and clear the column mask so as not to commit
      a pointless request, see vy_tx_set. As a result, we skip the first
      update too and get key {2,1} missing from the secondary index.
      
      Actually, it was a dumb idea to use column mask to skip statements in
      the first place, as there's a much easier way to filter out statements
      that have no effect for secondary indexes. The thing is every DELETE
      statement inserted into a secondary index write set acts as a "single
      DELETE", i.e. there's exactly one older statement it is supposed to
      purge. This is, because in contrast to the primary index we don't write
      DELETE statements blindly - we always look up the tuple overwritten in
      the primary index first. This means that REPLACE+DELETE for the same key
      is basically a no-op and can be safely skip. Moreover, DELETE+REPLACE
      can be treated as no-op, too, because secondary indexes don't store full
      tuples hence all REPLACE statements for the same key are equivalent.
      By marking both statements as no-op in vy_tx_set, we guarantee that
      no-op statements don't make it to secondary index memory or disk levels.
      
      Closes #4242
      
      (cherry picked from commit 69aee6fc)
      0e37af31
  17. May 23, 2019
  18. May 21, 2019
    • Vladislav Shpilevoy's avatar
      crypto: fix assertion on cipher reinitialization · e408fe50
      Vladislav Shpilevoy authored
      Crypto provides API to create stream objects. These streams
      consume plain and return ecnrypted data. Steps:
      
          1 c = cipher.new([key, iv])
          2 c:init(key, iv)
          3 c:update(input)
          4 c:result()
      
      Step 2 is optional, if key and iv are specified in new(), but if
      it called without key or iv, then result() method crashes.
      
      The commit allows to fill key and iv gradually, in several init()
      calls, and remembers previous results.
      
      Closes #4223
      
      (cherry picked from commit 26333580)
      e408fe50
    • Vladimir Davydov's avatar
      vinyl: fix assertion while recovering dumped statement · d5afe7bd
      Vladimir Davydov authored
      Certain kinds of DML requests don't update secondary indexes, e.g.
      UPDATE that doesn't touch secondary index parts or DELETE for which
      generation of secondary index statements is deferred. For such a request
      vy_is_committed(env, space) may return false on recovery even if it has
      actually been dumped: since such a statement is not dumped for secondary
      indexes, secondary index's vy_lsm::dump_lsn may be less than such
      statement's signature, which makes vy_is_committed() assume that the
      statement hasn't been dumped. Further in the code we have checks that
      ensure that if we execute a request on recovery, it must not be dumped
      for the primary index (as the primary index is always dumped after
      secondary indexes for the sake of recovery), which fires in this case.
      
      To fix that, let's refactor the code basing on the following two facts:
       - Primary index is always updated by a DML request.
       - Primary index may only be dumped after secondary indexes.
      
      Closes #4222
      
      (cherry picked from commit 9566f14c)
      d5afe7bd
    • Vladimir Davydov's avatar
      schema: fix error while altering index with sequence · 285ca40f
      Vladimir Davydov authored
      A check was missing in index.alter. This resulted in an attempt to drop
      the sequence attached to the altered index even if the sequence was not
      modified.
      
      Closes #4214
      
      (cherry picked from commit 7d778de6)
      285ca40f
  19. May 20, 2019
    • Alexander V. Tikhonov's avatar
      travis-ci: fix LTO and clang · 5baf1c11
      Alexander V. Tikhonov authored
      Made fixes:
      
      - Added CMAKE_EXTRA_PARAMS environment to docker's container
        runs to enable -DENABLE_LTO=ON/OFF cmake option.
      
      - Added CC/CXX environment to docker's container runs to set
        clang for cmake. Also the additional environment variables
        {CC,CXX}_FOR_BUILD were postponed, because we didn't
        run cross-compilation at the moment, for more info check:
      
          https://docs.travis-ci.com/user/languages/cpp/#choosing-compilers-to-test-against
      
      - Changed LTO docker's image to 'debian-buster' due to LTO needed
        higher versions of packages, check for more information commit:
      
          f9e28ce4 ('Add LTO support')
      
      - Fixed sources to avoid of failures on builds by GCC with LTO:
      
      1)  src/box/memtx_rtree.c: In function ‘mp_decode_rect’:
          src/box/memtx_rtree.c:86:24: error: ‘c’ may be used uninitialized
            in this function [-Werror=maybe-uninitialized]
              rect->coords[i * 2] = c;
                                  ^
          src/box/memtx_rtree.c:74:10: note: ‘c’ was declared here
            coord_t c;
                    ^
      
      2)  src/box/sql/func.c: In function ‘quoteFunc’:
          src/box/sql/func.c:1103:3: error: ‘b’ may be used uninitialized
            in this function [-Werror=maybe-uninitialized]
             sql_result_text(context, sql_value_boolean(argv[0]) ?
             ^
          src/box/sql/vdbeapi.c:217:7: note: ‘b’ was declared here
            bool b;
                 ^
      
      3)  src/box/tuple_update.c: In function ‘update_read_ops’:
          src/box/tuple_update.c:1022:4: error: ‘field_no’ may be used
            uninitialized in this function [-Werror=maybe-uninitialized]
              diag_set(ClientError, ER_NO_SUCH_FIELD_NO, field_no);
              ^
          src/box/tuple_update.c:1014:11: note: ‘field_no’ was declared here
             int32_t field_no;
                     ^
      
      4)  src/httpc.c: In function ‘httpc_set_verbose’:
          src/httpc.c:267:2: error: call to ‘_curl_easy_setopt_err_long’
            declared with attribute warning: curl_easy_setopt expects a long
            argument for this option [-Werror]
            curl_easy_setopt(req->curl_request.easy, CURLOPT_VERBOSE, curl_verbose);
            ^
      
      5)  src/lua/httpc.c: In function ‘luaT_httpc_request’:
          src/lua/httpc.c:128:64: error: ‘MEM[(int *)&parser + 20B]’ may be used
            uninitialized in this function [-Werror=maybe-uninitialized]
            lua_pushinteger(L, (parser.http_minor > 0) ? parser.http_minor: 0);
                                                                          ^
          src/lua/httpc.c:67:21: note: ‘MEM[(int *)&parser + 20B]’ was declared here
            struct http_parser parser;
                               ^
          src/lua/httpc.c:124:64: error: ‘MEM[(int *)&parser + 16B]’ may be used
            uninitialized in this function [-Werror=maybe-uninitialized]
            lua_pushinteger(L, (parser.http_major > 0) ? parser.http_major: 0);
                                                                          ^
          src/lua/httpc.c:67:21: note: ‘MEM[(int *)&parser + 16B]’ was declared here
            struct http_parser parser;
                               ^
      
      Close #4215
      
      (cherry picked from commit e55396c8)
      5baf1c11
    • Alexander Turenko's avatar
      travis-ci: set right flags in release testing jobs · e0054086
      Alexander Turenko authored
      It is important to have testing jobs that build the project with both
      -Werror and -O2 to keep the code clean. -O2 is needed, because some
      compiler warnings are available only after extra analyzing passes that
      are disabled with lesser optimization levels.
      
      The first attempt to add -Werror for release testing jobs was made in
      da505ee7 ('Add -Werror for CI (1.10
      part)'), but it mistakely doesn't enable -O2 for RelWithDebInfoWError
      build. It is possible to fix it in this way:
      
       | --- a/cmake/compiler.cmake
       | +++ b/cmake/compiler.cmake
       | @@ -113,10 +113,14 @@ set (CMAKE_C_FLAGS_DEBUG
       |      "${CMAKE_C_FLAGS_DEBUG} ${CC_DEBUG_OPT} -O0")
       |  set (CMAKE_C_FLAGS_RELWITHDEBINFO
       |      "${CMAKE_C_FLAGS_RELWITHDEBINFO} ${CC_DEBUG_OPT} -O2")
       | +set (CMAKE_C_FLAGS_RELWITHDEBINFOWERROR
       | +    "${CMAKE_C_FLAGS_RELWITHDEBINFOWERROR} ${CC_DEBUG_OPT} -O2")
       |  set (CMAKE_CXX_FLAGS_DEBUG
       |      "${CMAKE_CXX_FLAGS_DEBUG} ${CC_DEBUG_OPT} -O0")
       |  set (CMAKE_CXX_FLAGS_RELWITHDEBINFO
       |      "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} ${CC_DEBUG_OPT} -O2")
       | +set (CMAKE_CXX_FLAGS_RELWITHDEBINFOWERROR
       | +    "${CMAKE_CXX_FLAGS_RELWITHDEBINFOWERROR} ${CC_DEBUG_OPT} -O2")
       |
       |  unset(CC_DEBUG_OPT)
      
      However I think that a build type (and so `tarantool --version`) should
      not show whether -Werror was passed or not. So I have added
      ENABLE_WERROR CMake option for that. It can be set like so:
      
       | cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON
      
      Enabled the option in testing Travis-CI jobs with the RelWithDebInfo
      build type. Deploy jobs don't include it as before.
      
      Fixed all -Wmaybe-uninitialized and -Wunused-result warnings. A few
      notes about the fixes:
      
      * net.box does not validate received data in general, so I don't add a
        check for autoincrement IDs too. Set the ID to INT64_MIN, because this
        value is less probably will appear here in a normal case and so is the
        best one to signal a user that something probably going wrongly.
      * xrow_decode_*() functions could read uninitialized data from
        row->body[0].iov_base in xrow_on_decode_err() when printing a hex code
        for a row. It could be possible when the received msgpack was empty
        (row->bodycnt == 0), but there were expected keys (key_map != 0).
      * getcwd() is marked with __attribute__((__warn_unused_result__)) in
        glibc, but the buffer filled by this call is not used anywhere and so
        just removed.
      * Vinyl -Wmaybe-uninitialized warnings are false positive ones.
      
      Added comments and quotes into .travis.yml to ease reading. Removed
      "test" word from the CentOS 6 job name, because we don't run tests on
      this distro (disabled in the RPM spec).
      
      Fixes #4178.
      
      (cherry picked from commit c308f35d)
      e0054086
Loading