* [Tarantool-patches] [PATCH 1/3] cmake: ignore warnings on alignof() and offsetof()
2020-09-11 11:24 [Tarantool-patches] [PATCH 0/3] small: Update to use region_alloc_object Cyrill Gorcunov
@ 2020-09-11 11:24 ` Cyrill Gorcunov
2020-09-11 11:24 ` [Tarantool-patches] [PATCH 2/3] Prepare for small library update Cyrill Gorcunov
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Cyrill Gorcunov @ 2020-09-11 11:24 UTC (permalink / raw)
To: tml; +Cc: Vladislav Shpilevoy
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Warning about invalid offsetof() (used on non-POD types) was set
for g++, but wasn't for clang++.
Warning about invalid alignof() (when expression is passed to it
instead of a type) wasn't ignored, but is going to be very
useful in upcoming unaligned memory access patches. That allows
to write something like:
struct some_long_type *object = region_aligned_alloc(
region, size, alignof(*object));
This will work even if type of 'object' will change in future,
and so it is safer. And shorter.
Part of #4609
---
cmake/compiler.cmake | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/cmake/compiler.cmake b/cmake/compiler.cmake
index 56429dc20..b0908f3b3 100644
--- a/cmake/compiler.cmake
+++ b/cmake/compiler.cmake
@@ -276,11 +276,17 @@ macro(enable_tnt_compile_flags)
add_compile_flags("C;CXX" "-Wno-format-truncation")
endif()
- if (CMAKE_COMPILER_IS_GNUCXX)
+ if (CMAKE_COMPILER_IS_CLANG OR CMAKE_COMPILER_IS_GNUCXX)
# G++ bug. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31488
+ # Also offsetof() is widely used in Tarantool source code
+ # for classes and structs to implement intrusive lists and
+ # some other data structures. G++ and clang++ both
+ # complain about classes, having a virtual table. They
+ # complain fair, but this can't be fixed for now.
add_compile_flags("CXX"
"-Wno-invalid-offsetof"
)
+ add_compile_flags("C;CXX" "-Wno-gnu-alignof-expression")
endif()
if (CMAKE_COMPILER_IS_GNUCC)
--
2.26.2
^ permalink raw reply [flat|nested] 6+ messages in thread* [Tarantool-patches] [PATCH 2/3] Prepare for small library update
2020-09-11 11:24 [Tarantool-patches] [PATCH 0/3] small: Update to use region_alloc_object Cyrill Gorcunov
2020-09-11 11:24 ` [Tarantool-patches] [PATCH 1/3] cmake: ignore warnings on alignof() and offsetof() Cyrill Gorcunov
@ 2020-09-11 11:24 ` Cyrill Gorcunov
2020-09-11 11:24 ` [Tarantool-patches] [PATCH 3/3] small: sanitized rlist, bug in lsregion, and new region API Cyrill Gorcunov
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Cyrill Gorcunov @ 2020-09-11 11:24 UTC (permalink / raw)
To: tml; +Cc: Vladislav Shpilevoy
Backport of the commit 277fc6958fda07f212251d30e471b329d1028638,
note that we do NOT update small library here immediately otherwise
we simply won't built after this commit.
Thus real update will be done a bit later.
To bring new rlist methods - rlist_foreach_entry_safe_reverse and
rlist_cut_before. Also, remove unit/rlist test as it is now a part
of the small suite.
Part-of #5060
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
test/unit/CMakeLists.txt | 2 -
test/unit/rlist.c | 137 ---------------------------------------
test/unit/rlist.result | 88 -------------------------
3 files changed, 227 deletions(-)
delete mode 100644 test/unit/rlist.c
delete mode 100644 test/unit/rlist.result
diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt
index cdafd785f..d0138e8fd 100644
--- a/test/unit/CMakeLists.txt
+++ b/test/unit/CMakeLists.txt
@@ -18,8 +18,6 @@ add_executable(heap.test heap.c)
target_link_libraries(heap.test unit)
add_executable(heap_iterator.test heap_iterator.c)
target_link_libraries(heap_iterator.test unit)
-add_executable(rlist.test rlist.c)
-target_link_libraries(rlist.test unit)
add_executable(stailq.test stailq.c)
target_link_libraries(stailq.test unit)
add_executable(uri.test uri.c unit.c)
diff --git a/test/unit/rlist.c b/test/unit/rlist.c
deleted file mode 100644
index c0c29a3f1..000000000
--- a/test/unit/rlist.c
+++ /dev/null
@@ -1,137 +0,0 @@
-#include "small/rlist.h"
-#include <stdio.h>
-#include <stdarg.h>
-#include "unit.h"
-
-
-#define PLAN 87
-
-#define ITEMS 7
-
-struct test {
- char ch;
- int no;
- struct rlist list;
-};
-
-static struct test items[ITEMS];
-
-static RLIST_HEAD(head);
-static RLIST_HEAD(head2);
-
-int
-main(void)
-{
- int i;
- struct test *it;
- struct rlist *rlist;
-
- plan(PLAN);
- ok(rlist_empty(&head), "list is empty");
- for (i = 0; i < ITEMS; i++) {
- items[i].no = i;
- rlist_add_tail(&head, &(items[i].list));
- }
- RLIST_HEAD(empty_list);
- ok(rlist_empty(&empty_list), "rlist_nil is empty");
- ok(rlist_empty(&head2), "head2 is empty");
- rlist_swap(&head2, &empty_list);
- ok(rlist_empty(&empty_list), "rlist_nil is empty after swap");
- ok(rlist_empty(&head2), "head2 is empty after swap");
- rlist_swap(&head, &head2);
- ok(rlist_empty(&head), "head is empty after swap");
- is(rlist_first(&head2), &items[0].list, "first item");
- is(rlist_last(&head2), &items[ITEMS - 1].list, "last item");
- i = 0;
- rlist_foreach(rlist, &head2) {
- is(rlist, &items[i].list, "element (foreach) %d", i);
- i++;
- }
- rlist_foreach_reverse(rlist, &head2) {
- i--;
- is(rlist, &items[i].list, "element (foreach_reverse) %d", i);
- }
- rlist_swap(&head2, &head);
-
-
- is(rlist_first(&head), &items[0].list, "first item");
- isnt(rlist_first(&head), &items[ITEMS - 1].list, "first item");
-
- is(rlist_last(&head), &items[ITEMS - 1].list, "last item");
- isnt(rlist_last(&head), &items[0].list, "last item");
-
- is(rlist_next(&head), &items[0].list, "rlist_next");
- is(rlist_prev(&head), &items[ITEMS - 1].list, "rlist_prev");
-
- i = 0;
- rlist_foreach(rlist, &head) {
- is(rlist, &items[i].list, "element (foreach) %d", i);
- i++;
- }
- rlist_foreach_reverse(rlist, &head) {
- i--;
- is(rlist, &items[i].list, "element (foreach_reverse) %d", i);
- }
-
-
- is(rlist_entry(&items[0].list, struct test, list), &items[0],
- "rlist_entry");
- is(rlist_first_entry(&head, struct test, list), &items[0],
- "rlist_first_entry");
- is(rlist_next_entry(&items[0], list), &items[1], "rlist_next_entry");
- is(rlist_prev_entry(&items[2], list), &items[1], "rlist_prev_entry");
-
-
- i = 0;
- rlist_foreach_entry(it, &head, list) {
- is(it, items + i, "element (foreach_entry) %d", i);
- i++;
- }
- rlist_foreach_entry_reverse(it, &head, list) {
- i--;
- is(it, items + i, "element (foreach_entry_reverse) %d", i);
- }
-
- rlist_del(&items[2].list);
- ok(rlist_empty(&head2), "head2 is empty");
- rlist_move(&head2, &items[3].list);
- ok(!rlist_empty(&head2), "head2 isnt empty");
- is(rlist_first_entry(&head2, struct test, list),
- &items[3], "Item was moved");
- rlist_move_tail(&head2, &items[4].list);
- rlist_foreach_entry(it, &head, list) {
- is(it, items + i, "element (second deleted) %d", i);
- i++;
- if (i == 2)
- i += 3;
- }
- rlist_foreach_entry_reverse(it, &head, list) {
- i--;
- if (i == 4)
- i -= 3;
- is(it, items + i, "element (second deleted) %d", i);
- }
-
-
- rlist_create(&head);
- ok(rlist_empty(&head), "list is empty");
- for (i = 0; i < ITEMS; i++) {
- items[i].no = i;
- rlist_add(&head, &(items[i].list));
- }
- i = 0;
- rlist_foreach_entry_reverse(it, &head, list) {
- is(it, items + i, "element (foreach_entry_reverse) %d", i);
- i++;
- }
- rlist_foreach_entry(it, &head, list) {
- i--;
- is(it, items + i, "element (foreach_entry) %d", i);
- }
- rlist_create(&head);
- rlist_add_entry(&head, &items[0], list);
- ok(rlist_prev_entry_safe(&items[0], &head, list) == NULL,
- "prev is null");
- return check_plan();
-}
-
diff --git a/test/unit/rlist.result b/test/unit/rlist.result
deleted file mode 100644
index fa99a87cf..000000000
--- a/test/unit/rlist.result
+++ /dev/null
@@ -1,88 +0,0 @@
-1..87
-ok 1 - list is empty
-ok 2 - rlist_nil is empty
-ok 3 - head2 is empty
-ok 4 - rlist_nil is empty after swap
-ok 5 - head2 is empty after swap
-ok 6 - head is empty after swap
-ok 7 - first item
-ok 8 - last item
-ok 9 - element (foreach) 0
-ok 10 - element (foreach) 1
-ok 11 - element (foreach) 2
-ok 12 - element (foreach) 3
-ok 13 - element (foreach) 4
-ok 14 - element (foreach) 5
-ok 15 - element (foreach) 6
-ok 16 - element (foreach_reverse) 6
-ok 17 - element (foreach_reverse) 5
-ok 18 - element (foreach_reverse) 4
-ok 19 - element (foreach_reverse) 3
-ok 20 - element (foreach_reverse) 2
-ok 21 - element (foreach_reverse) 1
-ok 22 - element (foreach_reverse) 0
-ok 23 - first item
-ok 24 - first item
-ok 25 - last item
-ok 26 - last item
-ok 27 - rlist_next
-ok 28 - rlist_prev
-ok 29 - element (foreach) 0
-ok 30 - element (foreach) 1
-ok 31 - element (foreach) 2
-ok 32 - element (foreach) 3
-ok 33 - element (foreach) 4
-ok 34 - element (foreach) 5
-ok 35 - element (foreach) 6
-ok 36 - element (foreach_reverse) 6
-ok 37 - element (foreach_reverse) 5
-ok 38 - element (foreach_reverse) 4
-ok 39 - element (foreach_reverse) 3
-ok 40 - element (foreach_reverse) 2
-ok 41 - element (foreach_reverse) 1
-ok 42 - element (foreach_reverse) 0
-ok 43 - rlist_entry
-ok 44 - rlist_first_entry
-ok 45 - rlist_next_entry
-ok 46 - rlist_prev_entry
-ok 47 - element (foreach_entry) 0
-ok 48 - element (foreach_entry) 1
-ok 49 - element (foreach_entry) 2
-ok 50 - element (foreach_entry) 3
-ok 51 - element (foreach_entry) 4
-ok 52 - element (foreach_entry) 5
-ok 53 - element (foreach_entry) 6
-ok 54 - element (foreach_entry_reverse) 6
-ok 55 - element (foreach_entry_reverse) 5
-ok 56 - element (foreach_entry_reverse) 4
-ok 57 - element (foreach_entry_reverse) 3
-ok 58 - element (foreach_entry_reverse) 2
-ok 59 - element (foreach_entry_reverse) 1
-ok 60 - element (foreach_entry_reverse) 0
-ok 61 - head2 is empty
-ok 62 - head2 isnt empty
-ok 63 - Item was moved
-ok 64 - element (second deleted) 0
-ok 65 - element (second deleted) 1
-ok 66 - element (second deleted) 5
-ok 67 - element (second deleted) 6
-ok 68 - element (second deleted) 6
-ok 69 - element (second deleted) 5
-ok 70 - element (second deleted) 1
-ok 71 - element (second deleted) 0
-ok 72 - list is empty
-ok 73 - element (foreach_entry_reverse) 0
-ok 74 - element (foreach_entry_reverse) 1
-ok 75 - element (foreach_entry_reverse) 2
-ok 76 - element (foreach_entry_reverse) 3
-ok 77 - element (foreach_entry_reverse) 4
-ok 78 - element (foreach_entry_reverse) 5
-ok 79 - element (foreach_entry_reverse) 6
-ok 80 - element (foreach_entry) 6
-ok 81 - element (foreach_entry) 5
-ok 82 - element (foreach_entry) 4
-ok 83 - element (foreach_entry) 3
-ok 84 - element (foreach_entry) 2
-ok 85 - element (foreach_entry) 1
-ok 86 - element (foreach_entry) 0
-ok 87 - prev is null
--
2.26.2
^ permalink raw reply [flat|nested] 6+ messages in thread* [Tarantool-patches] [PATCH 3/3] small: sanitized rlist, bug in lsregion, and new region API
2020-09-11 11:24 [Tarantool-patches] [PATCH 0/3] small: Update to use region_alloc_object Cyrill Gorcunov
2020-09-11 11:24 ` [Tarantool-patches] [PATCH 1/3] cmake: ignore warnings on alignof() and offsetof() Cyrill Gorcunov
2020-09-11 11:24 ` [Tarantool-patches] [PATCH 2/3] Prepare for small library update Cyrill Gorcunov
@ 2020-09-11 11:24 ` Cyrill Gorcunov
2020-09-14 16:07 ` [Tarantool-patches] [PATCH 0/3] small: Update to use region_alloc_object Cyrill Gorcunov
2020-09-15 13:50 ` Kirill Yukhin
4 siblings, 0 replies; 6+ messages in thread
From: Cyrill Gorcunov @ 2020-09-11 11:24 UTC (permalink / raw)
To: tml; +Cc: Vladislav Shpilevoy
Backport of commit ffec9e40d8695bffe76109b721b1050db84b384e,
I've to update all region_alloc_array calls to use fuber()->gc
as there was no txn->region yet.
Rlist used a hack to implement offsetof() leading to crash under
undefined behaviour clang sanitizer. It was fixed in this update.
Additionally, region_alloc_object() is changed to return the used
size and a new macro region_alloc_array() is added. This small
API change is supposed to simplify switching lots of region
allocations to aligned versions in scope of #4609.
And finally - there was a bug in lsregion, when allocation was
exactly of slab size. It is fixed in small's master.
Part-of #4609
Part-of #5060
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/memtx_engine.c | 9 +++++----
src/box/txn.c | 23 +++++++++++++----------
src/box/vy_write_iterator.c | 7 +++++--
src/lib/small | 2 +-
4 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index e52397976..cc5bacc28 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -650,10 +650,10 @@ checkpoint_add_space(struct space *sp, void *data)
return 0;
struct checkpoint *ckpt = (struct checkpoint *)data;
struct checkpoint_entry *entry;
- entry = region_alloc_object(&fiber()->gc, struct checkpoint_entry);
+ int size;
+ entry = region_alloc_object(&fiber()->gc, struct checkpoint_entry, &size);
if (entry == NULL) {
- diag_set(OutOfMemory, sizeof(*entry),
- "region", "struct checkpoint_entry");
+ diag_set(OutOfMemory, sizeof(*entry), "region", "checkpoint_entry");
return -1;
}
rlist_add_tail_entry(&ckpt->entries, entry, link);
@@ -715,7 +715,8 @@ memtx_engine_begin_checkpoint(struct engine *engine)
struct memtx_engine *memtx = (struct memtx_engine *)engine;
assert(memtx->checkpoint == NULL);
- memtx->checkpoint = region_alloc_object(&fiber()->gc, struct checkpoint);
+ int size;
+ memtx->checkpoint = region_alloc_object(&fiber()->gc, struct checkpoint, &size);
if (memtx->checkpoint == NULL) {
diag_set(OutOfMemory, sizeof(*memtx->checkpoint),
"region", "struct checkpoint");
diff --git a/src/box/txn.c b/src/box/txn.c
index 4823ec0df..4cf107d45 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -48,10 +48,10 @@ txn_add_redo(struct txn_stmt *stmt, struct request *request)
{
/* Create a redo log row for Lua requests */
struct xrow_header *row;
- row = region_alloc_object(&fiber()->gc, struct xrow_header);
+ int size;
+ row = region_alloc_object(&fiber()->gc, struct xrow_header, &size);
if (row == NULL) {
- diag_set(OutOfMemory, sizeof(*row),
- "region", "struct xrow_header");
+ diag_set(OutOfMemory, size, "region_alloc_object", "row");
return -1;
}
if (request->header != NULL) {
@@ -85,10 +85,10 @@ static struct txn_stmt *
txn_stmt_new(struct txn *txn)
{
struct txn_stmt *stmt;
- stmt = region_alloc_object(&fiber()->gc, struct txn_stmt);
+ int size;
+ stmt = region_alloc_object(&fiber()->gc, struct txn_stmt, &size);
if (stmt == NULL) {
- diag_set(OutOfMemory, sizeof(*stmt),
- "region", "struct txn_stmt");
+ diag_set(OutOfMemory, size, "region_alloc_object", "stmt");
return NULL;
}
@@ -141,9 +141,10 @@ txn_begin(bool is_autocommit)
{
static int64_t txn_id = 0;
assert(! in_txn());
- struct txn *txn = region_alloc_object(&fiber()->gc, struct txn);
+ int size;
+ struct txn *txn = region_alloc_object(&fiber()->gc, struct txn, &size);
if (txn == NULL) {
- diag_set(OutOfMemory, sizeof(*txn), "region", "struct txn");
+ diag_set(OutOfMemory, size, "region_alloc_object", "txn");
return NULL;
}
/* Initialize members explicitly to save time on memset() */
@@ -512,12 +513,14 @@ box_txn_savepoint()
diag_set(ClientError, ER_SAVEPOINT_NO_TRANSACTION);
return NULL;
}
+ int size;
struct txn_savepoint *svp =
(struct txn_savepoint *) region_alloc_object(&fiber()->gc,
- struct txn_savepoint);
+ struct txn_savepoint,
+ &size);
if (svp == NULL) {
diag_set(OutOfMemory, sizeof(*svp),
- "region", "struct txn_savepoint");
+ "region_alloc_object", "txn_savepoint");
return NULL;
}
svp->stmt = stailq_last(&txn->stmts);
diff --git a/src/box/vy_write_iterator.c b/src/box/vy_write_iterator.c
index 41ef7d04b..2150b172b 100644
--- a/src/box/vy_write_iterator.c
+++ b/src/box/vy_write_iterator.c
@@ -100,9 +100,12 @@ static inline struct vy_write_history *
vy_write_history_new(struct tuple *tuple, struct vy_write_history *next)
{
struct vy_write_history *h;
- h = region_alloc_object(&fiber()->gc, struct vy_write_history);
- if (h == NULL)
+ int size;
+ h = region_alloc_object(&fiber()->gc, struct vy_write_history, &size);
+ if (h == NULL) {
+ diag_set(OutOfMemory, size, "region_alloc_object", "h");
return NULL;
+ }
h->tuple = tuple;
assert(next == NULL || (next->tuple != NULL &&
vy_stmt_lsn(next->tuple) > vy_stmt_lsn(tuple)));
diff --git a/src/lib/small b/src/lib/small
index 2fb5cf230..fcac155db 160000
--- a/src/lib/small
+++ b/src/lib/small
@@ -1 +1 @@
-Subproject commit 2fb5cf2303956175fd13b738c34d622499efb11b
+Subproject commit fcac155dba18e97b10bc668d1a2fae01184adc13
--
2.26.2
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Tarantool-patches] [PATCH 0/3] small: Update to use region_alloc_object
2020-09-11 11:24 [Tarantool-patches] [PATCH 0/3] small: Update to use region_alloc_object Cyrill Gorcunov
` (2 preceding siblings ...)
2020-09-11 11:24 ` [Tarantool-patches] [PATCH 3/3] small: sanitized rlist, bug in lsregion, and new region API Cyrill Gorcunov
@ 2020-09-14 16:07 ` Cyrill Gorcunov
2020-09-15 13:50 ` Kirill Yukhin
4 siblings, 0 replies; 6+ messages in thread
From: Cyrill Gorcunov @ 2020-09-14 16:07 UTC (permalink / raw)
Cc: To, Vladislav Shpilevoy
On Fri, Sep 11, 2020 at 02:24:28PM +0300, Cyrill Gorcunov wrote:
> A few commits from the master branch backporting commits
> to 1.10 series as Vlad requested.
>
> Since I've to update "small: sanitized rlist, bug in lsregion,
> and new region API" patch significantly during backport I
> put my "author" tag so any errors are mine while the real
> functionality is introduced by Vlad.
>
> issue https://github.com/tarantool/tarantool/issues/5060
> branch gorcunov/gh-5060-small-110
>
> Note the patches are for 1.10 series!
Kirill, these are backports of Vlads' patches I think it
is safe to merge them and close the issue.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Tarantool-patches] [PATCH 0/3] small: Update to use region_alloc_object
2020-09-11 11:24 [Tarantool-patches] [PATCH 0/3] small: Update to use region_alloc_object Cyrill Gorcunov
` (3 preceding siblings ...)
2020-09-14 16:07 ` [Tarantool-patches] [PATCH 0/3] small: Update to use region_alloc_object Cyrill Gorcunov
@ 2020-09-15 13:50 ` Kirill Yukhin
4 siblings, 0 replies; 6+ messages in thread
From: Kirill Yukhin @ 2020-09-15 13:50 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: tml, Vladislav Shpilevoy
Hello,
On 11 сен 14:24, Cyrill Gorcunov wrote:
> A few commits from the master branch backporting commits
> to 1.10 series as Vlad requested.
>
> Since I've to update "small: sanitized rlist, bug in lsregion,
> and new region API" patch significantly during backport I
> put my "author" tag so any errors are mine while the real
> functionality is introduced by Vlad.
>
> issue https://github.com/tarantool/tarantool/issues/5060
> branch gorcunov/gh-5060-small-110
>
> Note the patches are for 1.10 series!
>
> Cyrill Gorcunov (2):
> Prepare for small library update
> small: sanitized rlist, bug in lsregion, and new region API
>
> Vladislav Shpilevoy (1):
> cmake: ignore warnings on alignof() and offsetof()
LGTM.
I've checked your patch set into 1.10.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 6+ messages in thread