From ccf393c41ce427a4c8bfab6f20b6100a466620aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Thu, 13 Oct 2022 13:02:37 +0200 Subject: [PATCH 01/37] Use an unsigned type for the JSON printer flags Replace the enum flag in `flatcc_json_printer_set_flags()` with an unsigned integer type. In C enums are signed and bitwise operations on signed integer types may lead to undefined or implementation defined behavior. --- include/flatcc/flatcc_json_printer.h | 17 ++++++++--------- test/json_test/test_json.c | 2 +- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/include/flatcc/flatcc_json_printer.h b/include/flatcc/flatcc_json_printer.h index 0ce49c146..cab49a1a4 100644 --- a/include/flatcc/flatcc_json_printer.h +++ b/include/flatcc/flatcc_json_printer.h @@ -243,14 +243,13 @@ static inline void flatcc_json_printer_set_nonstrict(flatcc_json_printer_t *ctx) flatcc_json_printer_set_noenum(ctx, 0); } -enum flatcc_json_printer_flags { - flatcc_json_printer_f_unquote = 1, - flatcc_json_printer_f_noenum = 2, - flatcc_json_printer_f_skip_default = 4, - flatcc_json_printer_f_force_default = 8, - flatcc_json_printer_f_pretty = 16, - flatcc_json_printer_f_nonstrict = 32, -}; +typedef uint32_t flatcc_json_printer_flags_t; +static const flatcc_json_printer_flags_t flatcc_json_printer_f_unquote = 1; +static const flatcc_json_printer_flags_t flatcc_json_printer_f_noenum = 2; +static const flatcc_json_printer_flags_t flatcc_json_printer_f_skip_default = 4; +static const flatcc_json_printer_flags_t flatcc_json_printer_f_force_default = 8; +static const flatcc_json_printer_flags_t flatcc_json_printer_f_pretty = 16; +static const flatcc_json_printer_flags_t flatcc_json_printer_f_nonstrict = 32; /* * May be called instead of setting operational modes individually. @@ -268,7 +267,7 @@ enum flatcc_json_printer_flags { * `pretty` flag sets indentation to 2. * `nonstrict` implies: `noenum`, `unquote`, `pretty`. */ -static inline void flatcc_json_printer_set_flags(flatcc_json_printer_t *ctx, int flags) +static inline void flatcc_json_printer_set_flags(flatcc_json_printer_t *ctx, flatcc_json_printer_flags_t flags) { ctx->unquote = !!(flags & flatcc_json_printer_f_unquote); ctx->noenum = !!(flags & flatcc_json_printer_f_noenum); diff --git a/test/json_test/test_json.c b/test/json_test/test_json.c index 6d6698b80..4edc7483d 100644 --- a/test/json_test/test_json.c +++ b/test/json_test/test_json.c @@ -47,7 +47,7 @@ static const struct test_scope Movie = { int test_json(const struct test_scope *scope, char *json, char *expect, int expect_err, - int parse_flags, int print_flags, int line) + int parse_flags, flatcc_json_printer_flags_t print_flags, int line) { int ret = -1; int err; From 6e864877b509d308ef40b81a7d4e643c0a748c27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Tue, 18 Oct 2022 15:51:37 +0200 Subject: [PATCH 02/37] Use an unsigned type for the JSON parser flags --- include/flatcc/flatcc_json_parser.h | 23 +++++++++++------------ src/compiler/codegen_c_json_parser.c | 8 ++++---- src/runtime/json_parser.c | 4 ++-- test/json_test/test_json.c | 2 +- test/json_test/test_json_parser.c | 2 +- 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/include/flatcc/flatcc_json_parser.h b/include/flatcc/flatcc_json_parser.h index 1907fc7fc..d86190d79 100644 --- a/include/flatcc/flatcc_json_parser.h +++ b/include/flatcc/flatcc_json_parser.h @@ -22,13 +22,12 @@ extern "C" { #define PDIAGNOSTIC_IGNORE_UNUSED #include "flatcc/portable/pdiagnostic_push.h" -enum flatcc_json_parser_flags { - flatcc_json_parser_f_skip_unknown = 1, - flatcc_json_parser_f_force_add = 2, - flatcc_json_parser_f_with_size = 4, - flatcc_json_parser_f_skip_array_overflow = 8, - flatcc_json_parser_f_reject_array_underflow = 16 -}; +typedef uint32_t flatcc_json_parser_flags_t; +static const flatcc_json_parser_flags_t flatcc_json_parser_f_skip_unknown = 1; +static const flatcc_json_parser_flags_t flatcc_json_parser_f_force_add = 2; +static const flatcc_json_parser_flags_t flatcc_json_parser_f_with_size = 4; +static const flatcc_json_parser_flags_t flatcc_json_parser_f_skip_array_overflow = 8; +static const flatcc_json_parser_flags_t flatcc_json_parser_f_reject_array_underflow = 16; #define FLATCC_JSON_PARSE_ERROR_MAP(XX) \ XX(ok, "ok") \ @@ -92,7 +91,7 @@ typedef struct flatcc_json_parser_ctx flatcc_json_parser_t; struct flatcc_json_parser_ctx { flatcc_builder_t *ctx; const char *line_start; - int flags; + flatcc_json_parser_flags_t flags; #if FLATCC_JSON_PARSE_ALLOW_UNQUOTED int unquoted; #endif @@ -111,7 +110,7 @@ static inline int flatcc_json_parser_get_error(flatcc_json_parser_t *ctx) return ctx->error; } -static inline void flatcc_json_parser_init(flatcc_json_parser_t *ctx, flatcc_builder_t *B, const char *buf, const char *end, int flags) +static inline void flatcc_json_parser_init(flatcc_json_parser_t *ctx, flatcc_builder_t *B, const char *buf, const char *end, flatcc_json_parser_flags_t flags) { memset(ctx, 0, sizeof(*ctx)); ctx->ctx = B; @@ -872,10 +871,10 @@ const char *flatcc_json_parser_union_type_vector(flatcc_json_parser_t *ctx, * `buf`, `bufsiz` may be larger than the parsed json if trailing * space or zeroes are expected, but they must represent a valid memory buffer. * `fid` must be null, or a valid file identifier. - * `flags` default to 0. See also `flatcc_json_parser_flags`. + * `flags` default to 0. See also `flatcc_json_parser_f_` constants. */ int flatcc_json_parser_table_as_root(flatcc_builder_t *B, flatcc_json_parser_t *ctx, - const char *buf, size_t bufsiz, int flags, const char *fid, + const char *buf, size_t bufsiz, flatcc_json_parser_flags_t flags, const char *fid, flatcc_json_parser_table_f *parser); /* @@ -883,7 +882,7 @@ int flatcc_json_parser_table_as_root(flatcc_builder_t *B, flatcc_json_parser_t * * root. */ int flatcc_json_parser_struct_as_root(flatcc_builder_t *B, flatcc_json_parser_t *ctx, - const char *buf, size_t bufsiz, int flags, const char *fid, + const char *buf, size_t bufsiz, flatcc_json_parser_flags_t flags, const char *fid, flatcc_json_parser_struct_f *parser); #include "flatcc/portable/pdiagnostic_pop.h" diff --git a/src/compiler/codegen_c_json_parser.c b/src/compiler/codegen_c_json_parser.c index 29ebc85dd..0e2aa250c 100644 --- a/src/compiler/codegen_c_json_parser.c +++ b/src/compiler/codegen_c_json_parser.c @@ -1424,7 +1424,7 @@ static int gen_struct_parser(fb_output_t *out, fb_compound_type_t *ct) println(out, "return flatcc_json_parser_set_error(ctx, buf, end, flatcc_json_parser_error_runtime);"); unindent(); println(out, "}"); println(out, ""); - println(out, "static inline int %s_parse_json_as_root(flatcc_builder_t *B, flatcc_json_parser_t *ctx, const char *buf, size_t bufsiz, int flags, const char *fid)", snt.text); + println(out, "static inline int %s_parse_json_as_root(flatcc_builder_t *B, flatcc_json_parser_t *ctx, const char *buf, size_t bufsiz, flatcc_json_parser_flags_t flags, const char *fid)", snt.text); println(out, "{"); indent(); println(out, "return flatcc_json_parser_struct_as_root(B, ctx, buf, bufsiz, flags, fid, %s_parse_json_struct);", snt.text); @@ -1527,7 +1527,7 @@ static int gen_table_parser(fb_output_t *out, fb_compound_type_t *ct) println(out, "return flatcc_json_parser_set_error(ctx, buf, end, flatcc_json_parser_error_runtime);"); unindent(); println(out, "}"); println(out, ""); - println(out, "static inline int %s_parse_json_as_root(flatcc_builder_t *B, flatcc_json_parser_t *ctx, const char *buf, size_t bufsiz, int flags, const char *fid)", snt.text); + println(out, "static inline int %s_parse_json_as_root(flatcc_builder_t *B, flatcc_json_parser_t *ctx, const char *buf, size_t bufsiz, flatcc_json_parser_flags_t flags, const char *fid)", snt.text); println(out, "{"); indent(); println(out, "return flatcc_json_parser_table_as_root(B, ctx, buf, bufsiz, flags, fid, %s_parse_json_table);", snt.text); @@ -1661,7 +1661,7 @@ static int gen_root_table_parser(fb_output_t *out, fb_compound_type_t *ct) println(out, "static int %s_parse_json(flatcc_builder_t *B, flatcc_json_parser_t *ctx,", out->S->basename); indent(); indent(); - println(out, "const char *buf, size_t bufsiz, int flags)"); + println(out, "const char *buf, size_t bufsiz, flatcc_json_parser_flags_t flags)"); unindent(); unindent(); println(out, "{"); indent(); println(out, "flatcc_json_parser_t parser;"); @@ -1767,7 +1767,7 @@ static int gen_json_parser_prototypes(fb_output_t *out) println(out, "static int %s_parse_json(flatcc_builder_t *B, flatcc_json_parser_t *ctx,", out->S->basename); indent(); indent(); - println(out, "const char *buf, size_t bufsiz, int flags);"); + println(out, "const char *buf, size_t bufsiz, flatcc_json_parser_flags_t flags);"); unindent(); unindent(); println(out, ""); fallthrough; diff --git a/src/runtime/json_parser.c b/src/runtime/json_parser.c index 0e3aeea98..3f8b70598 100644 --- a/src/runtime/json_parser.c +++ b/src/runtime/json_parser.c @@ -1258,7 +1258,7 @@ const char *flatcc_json_parser_union_type_vector(flatcc_json_parser_t *ctx, } int flatcc_json_parser_table_as_root(flatcc_builder_t *B, flatcc_json_parser_t *ctx, - const char *buf, size_t bufsiz, int flags, const char *fid, + const char *buf, size_t bufsiz, flatcc_json_parser_flags_t flags, const char *fid, flatcc_json_parser_table_f *parser) { flatcc_json_parser_t _ctx; @@ -1278,7 +1278,7 @@ int flatcc_json_parser_table_as_root(flatcc_builder_t *B, flatcc_json_parser_t * } int flatcc_json_parser_struct_as_root(flatcc_builder_t *B, flatcc_json_parser_t *ctx, - const char *buf, size_t bufsiz, int flags, const char *fid, + const char *buf, size_t bufsiz, flatcc_json_parser_flags_t flags, const char *fid, flatcc_json_parser_table_f *parser) { flatcc_json_parser_t _ctx; diff --git a/test/json_test/test_json.c b/test/json_test/test_json.c index 4edc7483d..1a317ea17 100644 --- a/test/json_test/test_json.c +++ b/test/json_test/test_json.c @@ -47,7 +47,7 @@ static const struct test_scope Movie = { int test_json(const struct test_scope *scope, char *json, char *expect, int expect_err, - int parse_flags, flatcc_json_printer_flags_t print_flags, int line) + flatcc_json_parser_flags_t parse_flags, flatcc_json_printer_flags_t print_flags, int line) { int ret = -1; int err; diff --git a/test/json_test/test_json_parser.c b/test/json_test/test_json_parser.c index de2b738b2..a985cfab5 100644 --- a/test/json_test/test_json_parser.c +++ b/test/json_test/test_json_parser.c @@ -80,7 +80,7 @@ int test_parse(void) flatcc_builder_t builder; flatcc_builder_t *B = &builder; int ret = -1; - int flags = 0; + flatcc_json_parser_flags_t flags = 0; flatcc_builder_init(B); From 49d50e2107e9e05075c9aeac9e47f505c13c73c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Tue, 18 Oct 2022 17:18:57 +0200 Subject: [PATCH 03/37] Use an unsigned type for the buffer flags --- include/flatcc/flatcc_builder.h | 17 ++++++++++------- src/runtime/builder.c | 12 ++++++------ src/runtime/json_parser.c | 4 ++-- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/include/flatcc/flatcc_builder.h b/include/flatcc/flatcc_builder.h index 3871b1d07..2e84d2979 100644 --- a/include/flatcc/flatcc_builder.h +++ b/include/flatcc/flatcc_builder.h @@ -710,10 +710,13 @@ static inline void flatcc_builder_refmap_reset(flatcc_builder_t *B) } -enum flatcc_builder_buffer_flags { - flatcc_builder_is_nested = 1, - flatcc_builder_with_size = 2, -}; +typedef uint16_t flatcc_builder_buffer_flags_t; +static const flatcc_builder_buffer_flags_t flatcc_builder_is_nested = 1; +static const flatcc_builder_buffer_flags_t flatcc_builder_with_size = 2; + +/* The flag size in the API needs to match the internal size. */ +static_assert(sizeof(flatcc_builder_buffer_flags_t) == + sizeof(((flatcc_builder_t *)0)->buffer_flags), "flag size mismatch"); /** * An alternative to start buffer, start struct/table ... end buffer. @@ -776,7 +779,7 @@ enum flatcc_builder_buffer_flags { flatcc_builder_ref_t flatcc_builder_create_buffer(flatcc_builder_t *B, const char identifier[FLATBUFFERS_IDENTIFIER_SIZE], uint16_t block_align, - flatcc_builder_ref_t ref, uint16_t align, int flags); + flatcc_builder_ref_t ref, uint16_t align, flatcc_builder_buffer_flags_t flags); /** * Creates a struct within the current buffer without using any @@ -867,7 +870,7 @@ flatcc_builder_ref_t flatcc_builder_end_struct(flatcc_builder_t *B); */ int flatcc_builder_start_buffer(flatcc_builder_t *B, const char identifier[FLATBUFFERS_IDENTIFIER_SIZE], - uint16_t block_align, int flags); + uint16_t block_align, flatcc_builder_buffer_flags_t flags); /** * The root object should be a struct or a table to conform to the @@ -923,7 +926,7 @@ flatcc_builder_ref_t flatcc_builder_end_buffer(flatcc_builder_t *B, flatcc_build */ flatcc_builder_ref_t flatcc_builder_embed_buffer(flatcc_builder_t *B, uint16_t block_align, - const void *data, size_t size, uint16_t align, int flags); + const void *data, size_t size, uint16_t align, flatcc_builder_buffer_flags_t flags); /** * Applies to the innermost open buffer. The identifier may be null or diff --git a/src/runtime/builder.c b/src/runtime/builder.c index 9f54d884f..b62c2b667 100644 --- a/src/runtime/builder.c +++ b/src/runtime/builder.c @@ -723,11 +723,11 @@ static int align_to_block(flatcc_builder_t *B, uint16_t *align, uint16_t block_a flatcc_builder_ref_t flatcc_builder_embed_buffer(flatcc_builder_t *B, uint16_t block_align, - const void *data, size_t size, uint16_t align, int flags) + const void *data, size_t size, uint16_t align, flatcc_builder_buffer_flags_t flags) { uoffset_t size_field, pad; iov_state_t iov; - int with_size = flags & flatcc_builder_with_size; + int with_size = (flags & flatcc_builder_with_size) != 0; if (align_to_block(B, &align, block_align, !is_top_buffer(B))) { return 0; @@ -744,7 +744,7 @@ flatcc_builder_ref_t flatcc_builder_embed_buffer(flatcc_builder_t *B, flatcc_builder_ref_t flatcc_builder_create_buffer(flatcc_builder_t *B, const char identifier[identifier_size], uint16_t block_align, - flatcc_builder_ref_t object_ref, uint16_t align, int flags) + flatcc_builder_ref_t object_ref, uint16_t align, flatcc_builder_buffer_flags_t flags) { flatcc_builder_ref_t buffer_ref; uoffset_t header_pad, id_size = 0; @@ -808,7 +808,7 @@ flatcc_builder_ref_t flatcc_builder_create_struct(flatcc_builder_t *B, const voi } int flatcc_builder_start_buffer(flatcc_builder_t *B, - const char identifier[identifier_size], uint16_t block_align, int flags) + const char identifier[identifier_size], uint16_t block_align, flatcc_builder_buffer_flags_t flags) { /* * This saves the parent `min_align` in the align field since we @@ -845,9 +845,9 @@ int flatcc_builder_start_buffer(flatcc_builder_t *B, flatcc_builder_ref_t flatcc_builder_end_buffer(flatcc_builder_t *B, flatcc_builder_ref_t root) { flatcc_builder_ref_t buffer_ref; - int flags; + flatcc_builder_buffer_flags_t flags; - flags = B->buffer_flags & flatcc_builder_with_size; + flags = (flatcc_builder_buffer_flags_t)B->buffer_flags & flatcc_builder_with_size; flags |= is_top_buffer(B) ? 0 : flatcc_builder_is_nested; check(frame(type) == flatcc_builder_buffer, "expected buffer frame"); set_min_align(B, B->block_align); diff --git a/src/runtime/json_parser.c b/src/runtime/json_parser.c index 3f8b70598..5a847e1f8 100644 --- a/src/runtime/json_parser.c +++ b/src/runtime/json_parser.c @@ -1263,7 +1263,7 @@ int flatcc_json_parser_table_as_root(flatcc_builder_t *B, flatcc_json_parser_t * { flatcc_json_parser_t _ctx; flatcc_builder_ref_t root; - int builder_flags = flags & flatcc_json_parser_f_with_size ? flatcc_builder_with_size : 0; + flatcc_builder_buffer_flags_t builder_flags = flags & flatcc_json_parser_f_with_size ? flatcc_builder_with_size : 0; ctx = ctx ? ctx : &_ctx; flatcc_json_parser_init(ctx, B, buf, buf + bufsiz, flags); @@ -1283,7 +1283,7 @@ int flatcc_json_parser_struct_as_root(flatcc_builder_t *B, flatcc_json_parser_t { flatcc_json_parser_t _ctx; flatcc_builder_ref_t root; - int builder_flags = flags & flatcc_json_parser_f_with_size ? flatcc_builder_with_size : 0; + flatcc_builder_buffer_flags_t builder_flags = flags & flatcc_json_parser_f_with_size ? flatcc_builder_with_size : 0; ctx = ctx ? ctx : &_ctx; flatcc_json_parser_init(ctx, B, buf, buf + bufsiz, flags); From dc90c4358108b5829ee718f195034c896018108c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Tue, 18 Oct 2022 19:35:23 +0200 Subject: [PATCH 04/37] Update CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ede11234..0e8d84f4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ## [0.6.2-pre] - CMake: avoid assuming location of build dir during configuration. +- Use untyped integer constants in place of enums for public interface flags to + allow for safe bit masking operations (PR 248). ## [0.6.1] From 5ce5db0c7936cce8d4559d20a94cfbfab0c075fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Sat, 29 Oct 2022 13:14:26 +0200 Subject: [PATCH 05/37] Add experimental support for clangd config --- .gitignore | 3 +++ CMakeLists.txt | 6 ++++++ README.md | 7 +++++++ 3 files changed, 16 insertions(+) diff --git a/.gitignore b/.gitignore index eba83e37c..2d7483235 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,6 @@ bin/* lib/* release/* scripts/build.cfg +compile_flags.txt +compile_commands.json +.cache diff --git a/CMakeLists.txt b/CMakeLists.txt index 096d13ff1..f1f1a56c5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,6 +4,12 @@ #cmake_minimum_required (VERSION 2.8.11) cmake_minimum_required (VERSION 2.8) +# Experimental for generating compile_commands.json so editors with +# clangd language server support can use it. Symlink +# build/Debug/compile_commands.json to project root where it is +# gitignored. +#set(CMAKE_EXPORT_COMPILE_COMMANDS 1) + # Disable build of tests and samples. Due to custom build step # dependency on flatcc tool, some custom build configurations may # experience issues, and this option can then help. diff --git a/README.md b/README.md index 0b976b5cc..2a60708ca 100644 --- a/README.md +++ b/README.md @@ -1260,6 +1260,13 @@ better padding and cache efficiency. See also [flatc --annotate]. +Note: There is experimental support for text editor that supports +clangd language server or similar. You can edit `CMakeList.txt` +to generate `build/Debug/compile_comands.json`, at least when +using clang as a compiler, and copy or symlink it from root. Or +come with a better suggestion. There are `.gitignore` entries for +`compile_flags.txt` and `compile_commands.json` in project root. + ## File and Type Identifiers From 7134d8daa0b1338723a056401596fffe66355c3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Sat, 29 Oct 2022 13:20:27 +0200 Subject: [PATCH 06/37] Update CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e8d84f4d..7354a8ee5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ - CMake: avoid assuming location of build dir during configuration. - Use untyped integer constants in place of enums for public interface flags to allow for safe bit masking operations (PR 248). +- Added experimental support for generating `compile_commands.json` via + `CMakeList.txt` for use with clangd. ## [0.6.1] From 41e9e7b0b25d698821072f1603d62ca34b57eaf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Mon, 31 Oct 2022 15:51:15 +0100 Subject: [PATCH 07/37] Clean up indentation of if statement --- src/compiler/semantics.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/compiler/semantics.c b/src/compiler/semantics.c index 09b98d30e..e72dbe989 100644 --- a/src/compiler/semantics.c +++ b/src/compiler/semantics.c @@ -131,10 +131,11 @@ static inline void set_type_hash(fb_compound_type_t *ct) uint32_t hash; hash = fb_hash_fnv1a_32_init(); - if (ct->scope) - for (name = ct->scope->name; name; name = name->link) { - hash = fb_hash_fnv1a_32_append(hash, name->ident->text, (size_t)name->ident->len); - hash = fb_hash_fnv1a_32_append(hash, ".", 1); + if (ct->scope) { + for (name = ct->scope->name; name; name = name->link) { + hash = fb_hash_fnv1a_32_append(hash, name->ident->text, (size_t)name->ident->len); + hash = fb_hash_fnv1a_32_append(hash, ".", 1); + } } sym = &ct->symbol; hash = fb_hash_fnv1a_32_append(hash, sym->ident->text, (size_t)sym->ident->len); From d7f50b25c645c3c77640112625adbf2e09ab5e10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Tue, 1 Nov 2022 11:32:51 +0100 Subject: [PATCH 08/37] Add style guide section to README --- README.md | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 2a60708ca..b00ee04b2 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,7 @@ executable also handle optional json parsing or printing in less than 2 us for a * [Using the Compiler and Builder library](#using-the-compiler-and-builder-library) * [FlatBuffers Binary Format](#flatbuffers-binary-format) * [Security Considerations](#security-considerations) +* [Style Guide](#style-guide) * [Benchmarks](#benchmarks) @@ -2561,6 +2562,31 @@ Mostly for implementers: [FlatBuffers Binary Format] See [Security Considerations]. +## Style Guide + +FlatCC coding style is largely similar to the [WebKit Style], with the following notable exceptions: + +* Syntax requiring C99 or later is avoided, except `` types are made available. +* If conditions always use curly brackets. +* NULL and nullptr are generally just represented as `0`. +* Comments are old-school C-style (pre C99). Text is generally cased with punctuation: `/* A comment. */` +* `true` and `false` keywords are not used (pre C99). +* In code generation there is essentially no formatting to avoid excessive bloat. +* Struct names and other types is lower case since this is C, not C++. +* `snake_case` is used over `camelCase`. +* Header guards are used over `#pragma once` because it is non-standard and not always reliable in filesystems with ambigious paths. +* Comma is not placed first in multi-line calls (but maybe that would be a good idea for diff stability). +* `config.h` inclusion might be handled differently in that `flatbuffers.h` includes the config file. +* `unsigned` is not used without `int` for historical reasons. Generally a type like `uint32_t` is preferred. +* Use `TODO:` instead of `FIXME:` in comments for historical reasons. + +All the main source code in compiler and runtime aim to be C11 compatible and +uses many C11 constructs. This is made possible through the included portable +library such that older compilers can also function. Therefore any platform specific adaptations will be provided by updating +the portable library rather than introducing compile time flags in the main +source code. + + ## Benchmarks See [Benchmarks] @@ -2583,4 +2609,4 @@ See [Benchmarks] [readfile.h]: include/flatcc/support/readfile.h [Security Considerations]: https://github.com/dvidelabs/flatcc/blob/master/doc/security.md [flatc --annotate]: https://github.com/google/flatbuffers/tree/master/tests/annotated_binary - +[WebKit Style]: https://webkit.org/code-style-guidelines/ From ba71008384dd60a346f30ab1205ee9fbb54bb7aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Tue, 8 Nov 2022 18:29:01 +0100 Subject: [PATCH 09/37] Remove use of fallthrough macro in the compiler --- src/compiler/codegen_c_json_parser.c | 4 +--- src/compiler/codegen_c_json_printer.c | 4 +--- src/compiler/parser.c | 3 +-- src/compiler/semantics.c | 7 ++----- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/compiler/codegen_c_json_parser.c b/src/compiler/codegen_c_json_parser.c index 0e2aa250c..307ce76f4 100644 --- a/src/compiler/codegen_c_json_parser.c +++ b/src/compiler/codegen_c_json_parser.c @@ -8,8 +8,6 @@ #include #endif -#include "flatcc/portable/pattributes.h" /* fallthrough */ - #define PRINTLN_SPMAX 64 static char println_spaces[PRINTLN_SPMAX]; @@ -1770,7 +1768,7 @@ static int gen_json_parser_prototypes(fb_output_t *out) println(out, "const char *buf, size_t bufsiz, flatcc_json_parser_flags_t flags);"); unindent(); unindent(); println(out, ""); - fallthrough; + break; default: break; } diff --git a/src/compiler/codegen_c_json_printer.c b/src/compiler/codegen_c_json_printer.c index 2bdaba63d..efc4c3d14 100644 --- a/src/compiler/codegen_c_json_printer.c +++ b/src/compiler/codegen_c_json_printer.c @@ -6,8 +6,6 @@ #include #endif -#include "flatcc/portable/pattributes.h" /* fallthrough */ - static int gen_json_printer_pretext(fb_output_t *out) { fprintf(out->fp, @@ -583,7 +581,7 @@ static int gen_json_printer_prototypes(fb_output_t *out) fprintf(out->fp, "static int %s_print_json(flatcc_json_printer_t *ctx, const char *buf, size_t bufsiz);\n\n", out->S->basename); - fallthrough; + break; default: break; } diff --git a/src/compiler/parser.c b/src/compiler/parser.c index 250d6600d..4f31e0b95 100644 --- a/src/compiler/parser.c +++ b/src/compiler/parser.c @@ -16,7 +16,6 @@ #include "codegen.h" #include "fileio.h" #include "pstrutil.h" -#include "flatcc/portable/pattributes.h" /* fallthrough */ #include "flatcc/portable/pparseint.h" void fb_default_error_out(void *err_ctx, const char *buf, size_t len) @@ -920,7 +919,7 @@ static void parse_enum_decl(fb_parser_t *P, fb_compound_type_t *ct) case tok_kw_float32: case tok_kw_float64: error_tok(P, ct->type.t, "integral type expected"); - fallthrough; + break; default: break; } diff --git a/src/compiler/semantics.c b/src/compiler/semantics.c index e72dbe989..d0a766a61 100644 --- a/src/compiler/semantics.c +++ b/src/compiler/semantics.c @@ -11,8 +11,6 @@ #include #endif -#include "flatcc/portable/pattributes.h" /* fallthrough */ - /* Same order as enum! */ static const char *fb_known_attribute_names[] = { "", @@ -525,7 +523,6 @@ static int analyze_struct(fb_parser_t *P, fb_compound_type_t *ct) member = (fb_member_t *)sym; switch (member->type.type) { case vt_fixed_array_type: - /* fall through */ case vt_scalar_type: t = member->type.t; member->type.st = map_scalar_token_type(t); @@ -538,7 +535,6 @@ static int analyze_struct(fb_parser_t *P, fb_compound_type_t *ct) member->size = size * member->type.len; break; case vt_fixed_array_compound_type_ref: - /* fall through */ case vt_compound_type_ref: /* Enums might not be valid, but then it would be detected earlier. */ if (member->type.ct->symbol.kind == fb_is_enum) { @@ -702,8 +698,9 @@ static int process_struct(fb_parser_t *P, fb_compound_type_t *ct) switch (member->type.type) { case vt_fixed_array_type_ref: key_ok = 0; - fallthrough; + goto lbl_type_ref; case vt_type_ref: +lbl_type_ref: type_sym = lookup_type_reference(P, ct->scope, member->type.ref); if (!type_sym) { error_ref_sym(P, member->type.ref, "unknown type reference used with struct field", sym); From 91a0c559494205f2cc7778b3f3236a63b2cd2ae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Tue, 8 Nov 2022 18:31:13 +0100 Subject: [PATCH 10/37] Replace use of fallthrough with goto in runtime Keeping pattributes in the portable library in case usercode requires it. The fallthrough define is no longer used and therefor not defined by default, i.e PORTABLE_EXPOSE_ATTRIBUTES is not enabled. --- include/flatcc/flatcc_json_parser.h | 48 +++++++++++++++++---------- include/flatcc/portable/pattributes.h | 2 +- src/runtime/json_parser.c | 3 +- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/include/flatcc/flatcc_json_parser.h b/include/flatcc/flatcc_json_parser.h index d86190d79..f8281295a 100644 --- a/include/flatcc/flatcc_json_parser.h +++ b/include/flatcc/flatcc_json_parser.h @@ -242,24 +242,38 @@ static inline uint64_t flatcc_json_parser_symbol_part_ext(const char *buf, const } /* This can bloat inlining for a rarely executed case. */ #if 1 - /* Fall through comments needed to silence gcc 7 warnings. */ switch (n) { - case 8: w |= ((uint64_t)buf[7]) << (0 * 8); - fallthrough; - case 7: w |= ((uint64_t)buf[6]) << (1 * 8); - fallthrough; - case 6: w |= ((uint64_t)buf[5]) << (2 * 8); - fallthrough; - case 5: w |= ((uint64_t)buf[4]) << (3 * 8); - fallthrough; - case 4: w |= ((uint64_t)buf[3]) << (4 * 8); - fallthrough; - case 3: w |= ((uint64_t)buf[2]) << (5 * 8); - fallthrough; - case 2: w |= ((uint64_t)buf[1]) << (6 * 8); - fallthrough; - case 1: w |= ((uint64_t)buf[0]) << (7 * 8); - fallthrough; + case 8: + w |= ((uint64_t)buf[7]) << (0 * 8); + goto lbl_n_7; + case 7: +lbl_n_7: + w |= ((uint64_t)buf[6]) << (1 * 8); + goto lbl_n_6; + case 6: +lbl_n_6: + w |= ((uint64_t)buf[5]) << (2 * 8); + goto lbl_n_5; + case 5: +lbl_n_5: + w |= ((uint64_t)buf[4]) << (3 * 8); + goto lbl_n_4; + case 4: +lbl_n_4: + w |= ((uint64_t)buf[3]) << (4 * 8); + goto lbl_n_3; + case 3: +lbl_n_3: + w |= ((uint64_t)buf[2]) << (5 * 8); + goto lbl_n_2; + case 2: +lbl_n_2: + w |= ((uint64_t)buf[1]) << (6 * 8); + goto lbl_n_1; + case 1: +lbl_n_1: + w |= ((uint64_t)buf[0]) << (7 * 8); + break; case 0: break; } diff --git a/include/flatcc/portable/pattributes.h b/include/flatcc/portable/pattributes.h index 30b3b23d3..9240fa319 100644 --- a/include/flatcc/portable/pattributes.h +++ b/include/flatcc/portable/pattributes.h @@ -40,7 +40,7 @@ extern "C" { #endif #ifndef PORTABLE_EXPOSE_ATTRIBUTES -#define PORTABLE_EXPOSE_ATTRIBUTES 1 +#define PORTABLE_EXPOSE_ATTRIBUTES 0 #endif #ifdef __has_c_attribute diff --git a/src/runtime/json_parser.c b/src/runtime/json_parser.c index 5a847e1f8..3fa929cca 100644 --- a/src/runtime/json_parser.c +++ b/src/runtime/json_parser.c @@ -141,11 +141,10 @@ const char *flatcc_json_parser_space_ext(flatcc_json_parser_t *ctx, const char * ++buf; } while (buf != end && *buf <= 0x20) { - /* Fall through comments needed to silence gcc 7 warnings. */ switch (*buf) { case 0x0d: buf += (end - buf > 1 && buf[1] == 0x0a); /* Consume following LF or treating CR as LF. */ - fallthrough; + ++ctx->line; ctx->line_start = ++buf; continue; case 0x0a: ++ctx->line; ctx->line_start = ++buf; continue; case 0x09: ++buf; continue; case 0x20: goto again; /* Don't consume here, sync with power of 2 spaces. */ From e5be5bec4442382c42e9a07c664d20b79946b545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Tue, 8 Nov 2022 21:32:52 +0100 Subject: [PATCH 11/37] Update CHANGELOG --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7354a8ee5..fff3df835 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,10 @@ - CMake: avoid assuming location of build dir during configuration. - Use untyped integer constants in place of enums for public interface flags to - allow for safe bit masking operations (PR 248). + allow for safe bit masking operations (PR #248). - Added experimental support for generating `compile_commands.json` via `CMakeList.txt` for use with clangd. +- Remove `fallthrough` macro for improved portability (#247, #252). ## [0.6.1] From 1f1219c54a3c7c08ef288eaf97cbc735d21d9ab8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Fri, 11 Nov 2022 09:43:33 +0100 Subject: [PATCH 12/37] Update coding style --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b00ee04b2..43ff1a583 100644 --- a/README.md +++ b/README.md @@ -2567,7 +2567,7 @@ See [Security Considerations]. FlatCC coding style is largely similar to the [WebKit Style], with the following notable exceptions: * Syntax requiring C99 or later is avoided, except `` types are made available. -* If conditions always use curly brackets. +* If conditions always use curly brackets, or single line statements without linebreak: `if (err) return -1;`. * NULL and nullptr are generally just represented as `0`. * Comments are old-school C-style (pre C99). Text is generally cased with punctuation: `/* A comment. */` * `true` and `false` keywords are not used (pre C99). From 5885e50f88248bc7ed398880c887ab23db89f05a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Thu, 17 Nov 2022 15:57:43 +0100 Subject: [PATCH 13/37] Fix builds when using Clang 15 The diagnostics in Clang 15 is stricter and finds: [-Werror,-Wstrict-prototypes] function declaration without a prototype is deprecated Example: cgen_test.c:44:9: int main() [-Werror,-Wunused-but-set-variable] variable 'present_id' set but not used --- src/compiler/codegen_c_reader.c | 4 ---- test/cgen_test/cgen_test.c | 2 +- test/json_test/test_basic_parse.c | 2 +- test/json_test/test_json.c | 2 +- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/compiler/codegen_c_reader.c b/src/compiler/codegen_c_reader.c index 67f8055bc..6de0f2166 100644 --- a/src/compiler/codegen_c_reader.c +++ b/src/compiler/codegen_c_reader.c @@ -1593,7 +1593,6 @@ static void gen_table(fb_output_t *out, fb_compound_type_t *ct) const char *nsc = out->nsc; fb_scoped_name_t snt; fb_scoped_name_t snref; - uint64_t present_id; fb_literal_t literal; int is_optional; @@ -1629,7 +1628,6 @@ static void gen_table(fb_output_t *out, fb_compound_type_t *ct) for (sym = ct->members; sym; sym = sym->link) { current_key_processed = 0; member = (fb_member_t *)sym; - present_id = member->id; is_primary_key = ct->primary_key == member; is_optional = !!(member->flags & fb_fm_optional); print_doc(out, "", member->doc); @@ -1807,7 +1805,6 @@ static void gen_table(fb_output_t *out, fb_compound_type_t *ct) } break; case fb_is_union: - present_id--; fprintf(out->fp, "__%sdefine_union_field(%s, %"PRIu64", %s, %.*s, %s, %u)\n", nsc, nsc, (uint64_t)member->id, snt.text, n, s, snref.text, r); @@ -1833,7 +1830,6 @@ static void gen_table(fb_output_t *out, fb_compound_type_t *ct) break; } if (member->type.ct->symbol.kind == fb_is_union) { - present_id--; fprintf(out->fp, "__%sdefine_union_vector_field(%s, %"PRIu64", %s, %.*s, %s, %u)\n", nsc, nsc, (uint64_t)member->id, snt.text, n, s, snref.text, r); diff --git a/test/cgen_test/cgen_test.c b/test/cgen_test/cgen_test.c index 94f4bba9b..6d58ed1a9 100644 --- a/test/cgen_test/cgen_test.c +++ b/test/cgen_test/cgen_test.c @@ -41,7 +41,7 @@ #include #include "flatcc/flatcc.h" -int main() +int main(void) { const char *name = "../xyzzy.fbs"; diff --git a/test/json_test/test_basic_parse.c b/test/json_test/test_basic_parse.c index de27ee775..7b8f4bac9 100644 --- a/test/json_test/test_basic_parse.c +++ b/test/json_test/test_basic_parse.c @@ -277,7 +277,7 @@ const char *test(flatcc_builder_t *B, const char *buf, const char *end, int *ret return buf; } -int main() +int main(void) { int ret = -1; flatcc_builder_t builder; diff --git a/test/json_test/test_json.c b/test/json_test/test_json.c index 1a317ea17..aeee13a04 100644 --- a/test/json_test/test_json.c +++ b/test/json_test/test_json.c @@ -582,7 +582,7 @@ int fixed_array_tests(void) * covered in the printer and parser tests using the golden data * set. */ -int main() +int main(void) { BEGIN_TEST(Monster); From f89fe1ed243f3d464dc25bad872eac28f1e62b43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Fri, 18 Nov 2022 11:45:37 +0100 Subject: [PATCH 14/37] Add float compare functions to work around GCC double precision conversion --- include/flatcc/portable/pparsefp.h | 62 +++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/include/flatcc/portable/pparsefp.h b/include/flatcc/portable/pparsefp.h index 1d2b9da26..f10d6a816 100644 --- a/include/flatcc/portable/pparsefp.h +++ b/include/flatcc/portable/pparsefp.h @@ -5,6 +5,8 @@ extern "C" { #endif +#include /* memcpy */ + /* * Parses a float or double number and returns the length parsed if * successful. The length argument is of limited value due to dependency @@ -34,7 +36,7 @@ extern "C" { * Other compilers such as xlc may require linking with -lm which may not * be convienent so a default isinf is provided. If isinf is available * and there is a noticable performance issue, define - * `PORTABLE_USE_ISINF`. + * `PORTABLE_USE_ISINF`. This flag also affects isnan. */ #if defined(__GNUC__) || defined(__clang__) || defined(_MSC_VER) || defined(PORTABLE_USE_ISINF) #include @@ -48,8 +50,9 @@ extern "C" { * loss warning with -Wconversion flag when cast is absent. */ #if defined(__clang__) -#if __clang_major__ >= 5 && __clang_major__ <= 8 +#if __clang_major__ >= 3 && __clang_major__ <= 8 #define parse_double_isinf(x) isinf((float)x) +#define parse_double_isnan(x) isnan((float)x) #endif #endif #if !defined(parse_double_isinf) @@ -64,19 +67,29 @@ extern "C" { #endif /* Avoid linking with libmath but depends on float/double being IEEE754 */ -static inline int parse_double_isinf(double x) +static inline int parse_double_isinf(const double x) { - union { uint64_t u64; double f64; } v; - v.f64 = x; - return (v.u64 & 0x7fffffff00000000ULL) == 0x7ff0000000000000ULL; + uint64_t u64x; + + memcpy(&u64x, &x, sizeof(u64x)); + return (u64x & 0x7fffffff00000000ULL) == 0x7ff0000000000000ULL; } static inline int parse_float_isinf(float x) { - union { uint32_t u32; float f32; } v; - v.f32 = x; - return (v.u32 & 0x7fffffff) == 0x7f800000; + uint32_t u32x; + + memcpy(&u32x, &x, sizeof(u32x)); + return (u32x & 0x7fffffff) == 0x7f800000; } + +#endif + +#if !defined(parse_double_isnan) +#define parse_double_isnan isnan +#endif +#if !defined(parse_float_isnan) +#define parse_float_isnan isnan #endif /* Returns 0 when in range, 1 on overflow, and -1 on underflow. */ @@ -131,6 +144,37 @@ static inline const char *parse_float(const char *buf, size_t len, float *result return end; } +/* Inspired by https://bitbashing.io/comparing-floats.html */ +static inline int parse_double_compare(const double x, const double y) +{ + /* This also handles NaN */ + if (x == y) return 0; + return x < y ? -1 : 1; +} + +static inline int parse_float_compare(const float x, const float y) +{ + int32_t i32x, i32y; + + if (x == y) return 0; + if (parse_float_isnan(x)) return 1; + if (parse_float_isnan(y)) return 1; + memcpy(&i32x, &x, sizeof(i32x)); + memcpy(&i32y, &y, sizeof(i32y)); + return i32x < i32y ? -1 : 1; +} + +int parse_double_is_equal(const double x, const double y) +{ + return x == y; +} + +/* Works around GCC double precisoni conversion of floats. */ +static inline int parse_float_is_equal(const float x, const float y) +{ + return parse_float_compare(x, y) == 0; +} + #include "pdiagnostic_pop.h" #ifdef __cplusplus From 195246aa7c6a889887f3f254e883333aeee4f8f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Fri, 18 Nov 2022 11:48:20 +0100 Subject: [PATCH 15/37] Update CHANGELOG --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fff3df835..556d32a51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ - Added experimental support for generating `compile_commands.json` via `CMakeList.txt` for use with clangd. - Remove `fallthrough` macro for improved portability (#247, #252). +- Added `parse_float/double_compare`, `parse_float/double_is_equal` to + portable library, and added `parse_float/double_isnan` to mirror isinf. + This should help with GCC 32-bit double precision conversion issue. ## [0.6.1] From 4810b9e9f85d3897776bd13c87d8c1ab059979d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Fri, 18 Nov 2022 12:35:35 +0100 Subject: [PATCH 16/37] Improve on pparse floating point comparison operations --- include/flatcc/portable/pparsefp.h | 64 +++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/include/flatcc/portable/pparsefp.h b/include/flatcc/portable/pparsefp.h index f10d6a816..6ece207de 100644 --- a/include/flatcc/portable/pparsefp.h +++ b/include/flatcc/portable/pparsefp.h @@ -145,34 +145,76 @@ static inline const char *parse_float(const char *buf, size_t len, float *result } /* Inspired by https://bitbashing.io/comparing-floats.html */ -static inline int parse_double_compare(const double x, const double y) + +/* Return signed ULP distance or INT64_MAX if any value is nan. */ +static inline int64_t parse_double_compare(const double x, const double y) { - /* This also handles NaN */ + int64_t i64x, i64y; + if (x == y) return 0; - return x < y ? -1 : 1; + if (parse_double_isnan(x)) return INT64_MAX; + if (parse_double_isnan(y)) return INT64_MAX; + memcpy(&i64x, &x, sizeof(i64x)); + memcpy(&i64y, &y, sizeof(i64y)); + if ((i64x < 0) != (i64y < 0)) return INT64_MAX; + return i64x - i64y; } -static inline int parse_float_compare(const float x, const float y) +/* Same as double, but INT32_MAX if nan. */ +static inline int32_t parse_float_compare(const float x, const float y) { int32_t i32x, i32y; if (x == y) return 0; - if (parse_float_isnan(x)) return 1; - if (parse_float_isnan(y)) return 1; + if (parse_float_isnan(x)) return INT32_MAX; + if (parse_float_isnan(y)) return INT32_MAX; memcpy(&i32x, &x, sizeof(i32x)); memcpy(&i32y, &y, sizeof(i32y)); - return i32x < i32y ? -1 : 1; + if ((i32x < 0) != (i32y < 0)) return INT32_MAX; + return i32x - i32y; +} + +/* + * Returns the absolute distance in flaoting point ULP (representational bit difference). + * Uses signed return value so that INT64_MAX and INT32_MAX indicates NaN similar to + * the compare function. + */ +static inline int64_t parse_double_dist(const double x, const double y) +{ + uint64_t m64; + int64_t i64; + + i64 = parse_double_compare(x, y); + /* Absolute integer value of compare. */ + m64 = -(uint64_t)(i64 < 0); + return (int64_t)(((uint64_t)i64 + m64) ^ m64); +} + +/* Same as double, but INT32_MAX if NaN. */ +static inline int32_t parse_float_dist(const float x, const float y) +{ + uint32_t m32; + int32_t i32; + + i32 = parse_float_compare(x, y); + /* Absolute integer value of compare. */ + m32 = -(uint32_t)(i32 < 0); + return (int32_t)(((uint32_t)i32 + m32) ^ m32); } -int parse_double_is_equal(const double x, const double y) +/* + * Returns 1 if no value is NaN, and the difference is at most one ULP (1 bit), and the + * sign is the same, and 0 otherwise. + */ +static inline int parse_double_is_equal(const double x, const double y) { - return x == y; + return parse_double_dist(x, y) >> 1 == 0; } -/* Works around GCC double precisoni conversion of floats. */ +/* Same as double, but at lower precision. */ static inline int parse_float_is_equal(const float x, const float y) { - return parse_float_compare(x, y) == 0; + return parse_float_dist(x, y) >> 1 == 0; } #include "pdiagnostic_pop.h" From 8695d6fa587e5d5d7b500a0308873dbcdfa94fb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Fri, 18 Nov 2022 12:46:01 +0100 Subject: [PATCH 17/37] Add lower precsion floating point tests to handle GCC 32-bit floats --- test/emit_test/emit_test.c | 7 ++++++- test/monster_test/monster_test.c | 7 +++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/test/emit_test/emit_test.c b/test/emit_test/emit_test.c index fa18034ac..ddb973dde 100644 --- a/test/emit_test/emit_test.c +++ b/test/emit_test/emit_test.c @@ -2,8 +2,13 @@ #include #include "emit_test_builder.h" #include "flatcc/support/hexdump.h" +#include "flatcc/portable/pparsefp.h" #define test_assert(x) do { if (!(x)) { assert(0); return -1; }} while(0) +/* Direct floating point comparisons are not always directly comparable, + * especially not for GCC 32-bit compilers. */ +#define test_assert_floateq(x, y) test_assert(parse_float_is_equal((x), (y))) +#define test_assert_doubleeq(x, y) test_assert(parse_double_is_equal((x), (y))) int dbg_emitter(void *emit_context, const flatcc_iovec_t *iov, int iov_count, @@ -112,7 +117,7 @@ int emit_test(void) test_assert(time == 42); test_assert(main_device(mt) == 1); test_assert(flatbuffers_float_vec_len(main_samples(mt)) == 4); - test_assert(flatbuffers_float_vec_at(main_samples(mt), 2) == 1.2f); + test_assert_floateq(flatbuffers_float_vec_at(main_samples(mt), 2), 1.2f); /* We use get_direct_buffer, so we can't clear the builder until last. */ flatcc_builder_clear(B); diff --git a/test/monster_test/monster_test.c b/test/monster_test/monster_test.c index 400415c5d..02fc6444d 100644 --- a/test/monster_test/monster_test.c +++ b/test/monster_test/monster_test.c @@ -5,6 +5,7 @@ #include "flatcc/support/hexdump.h" #include "flatcc/support/elapsed.h" +#include "flatcc/portable/pparsefp.h" #include "../../config/config.h" /* @@ -371,8 +372,10 @@ int verify_monster(void *buffer) if ((size_t)vec & 15) { printf("Force align of Vec3 struct not correct\n"); } - /* -3.2f is actually -3.20000005 and not -3.2 due to representation loss. */ - if (ns(Vec3_z(vec)) != -3.2f) { + /* -3.2f is actually -3.20000005 and not -3.2 due to representation loss. + * For 32-bit GCC compilers, -3.2f might be another value, so use lower + * precision portable comparison. */ + if (!parse_float_is_equal(ns(Vec3_z(vec)), -3.2f)) { printf("Position failing on z coordinate\n"); return -1; } From a6ba3947f7d1c4f8e0053691ab6f50c29ccb4de5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Mon, 24 Oct 2022 13:01:33 +0200 Subject: [PATCH 18/37] Add Github CI and weekly test runs Also introduces a new build config for forced 32bit builds: > scripts/initbuild.sh make-32bit --- .github/workflows/ci.yml | 60 ++++++++++ .github/workflows/weekly.yml | 188 ++++++++++++++++++++++++++++++ README.md | 14 ++- scripts/build.cfg.make | 1 + scripts/build.cfg.make-32bit | 3 + scripts/build.cfg.make-concurrent | 1 + scripts/build.cfg.ninja | 1 + scripts/initbuild.sh | 4 +- 8 files changed, 267 insertions(+), 5 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 .github/workflows/weekly.yml create mode 100644 scripts/build.cfg.make-32bit diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 000000000..42e227d67 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,60 @@ +name: CI + +on: [push, pull_request] + +env: + CTEST_OUTPUT_ON_FAILURE: 1 + +jobs: + ubuntu-ninja-clang: + name: Ubuntu (ninja, clang) + runs-on: ubuntu-22.04 + steps: + - name: Prepare + run: | + sudo apt update + sudo apt install ninja-build + - uses: actions/checkout@v3 + - name: Build and run tests + env: + CC: clang + CXX: clang++ + run: | + scripts/test.sh + + ubuntu-make-gcc: + name: Ubuntu (make, gcc) + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v3 + - name: Build and run tests + env: + CC: gcc + CXX: g++ + run: | + scripts/initbuild.sh make + scripts/test.sh + + macos: + name: macOS + runs-on: macos-12 + steps: + - name: Prepare + run: | + brew install cmake ninja + - uses: actions/checkout@v3 + - name: Build and run tests + run: | + scripts/test.sh + + windows: + name: Windows + runs-on: windows-2022 + steps: + - uses: microsoft/setup-msbuild@v1.1 + - uses: actions/checkout@v3 + - name: Build and run tests + run: | + cmake . + msbuild FlatCC.sln /m /property:Configuration=Release + ctest -VV diff --git a/.github/workflows/weekly.yml b/.github/workflows/weekly.yml new file mode 100644 index 000000000..d88903f66 --- /dev/null +++ b/.github/workflows/weekly.yml @@ -0,0 +1,188 @@ +name: Weekly + +on: + workflow_dispatch: + schedule: + - cron: '0 10 * * 1' # Mon 10.00 UTC + +env: + CTEST_OUTPUT_ON_FAILURE: 1 + +jobs: + clang: + name: Clang ${{ matrix.clang-version }} + runs-on: ubuntu-20.04 + strategy: + fail-fast: false + matrix: + clang-version: [5, 7, 9, 11, 13, 15] + steps: + - name: Setup Clang + uses: aminya/setup-cpp@v1 + with: + llvm: ${{ matrix.clang-version }} + - uses: actions/checkout@v3 + - name: Build and run tests + run: | + scripts/initbuild.sh make-concurrent + scripts/test.sh + + clang-32bit: + name: Clang 32bit + runs-on: ubuntu-20.04 + steps: + - name: Prepare + run: | + sudo apt update + sudo apt install gcc-multilib g++-multilib + - uses: actions/checkout@v3 + - name: Build and run tests + env: + CC: clang + CXX: clang++ + run: | + scripts/initbuild.sh make-32bit + scripts/test.sh + + gcc-old: + name: GCC 4.4 + runs-on: ubuntu-20.04 + steps: + - name: Setup GCC + run: | + wget http://launchpadlibrarian.net/336269522/libmpfr4_3.1.6-1_amd64.deb + wget http://old-releases.ubuntu.com/ubuntu/pool/universe/g/gcc-4.4/gcc-4.4-base_4.4.7-8ubuntu1_amd64.deb + wget http://old-releases.ubuntu.com/ubuntu/pool/universe/g/gcc-4.4/cpp-4.4_4.4.7-8ubuntu1_amd64.deb + wget http://old-releases.ubuntu.com/ubuntu/pool/universe/g/gcc-4.4/gcc-4.4_4.4.7-8ubuntu1_amd64.deb + wget http://old-releases.ubuntu.com/ubuntu/pool/universe/g/gcc-4.4/libstdc++6-4.4-dev_4.4.7-8ubuntu1_amd64.deb + wget http://old-releases.ubuntu.com/ubuntu/pool/universe/g/gcc-4.4/g++-4.4_4.4.7-8ubuntu1_amd64.deb + sudo dpkg -i ./libmpfr4_3.1.6-1_amd64.deb + sudo dpkg -i ./gcc-4.4-base_4.4.7-8ubuntu1_amd64.deb + sudo dpkg -i ./cpp-4.4_4.4.7-8ubuntu1_amd64.deb + sudo dpkg -i ./gcc-4.4_4.4.7-8ubuntu1_amd64.deb + sudo dpkg -i ./libstdc++6-4.4-dev_4.4.7-8ubuntu1_amd64.deb ./g++-4.4_4.4.7-8ubuntu1_amd64.deb + - uses: actions/checkout@v3 + - name: Build and run tests + env: + CC: gcc-4.4 + CXX: g++-4.4 + run: | + scripts/initbuild.sh make-concurrent + scripts/test.sh + + gcc: + name: GCC ${{ matrix.gcc-version }} + runs-on: ubuntu-20.04 + strategy: + fail-fast: false + matrix: + gcc-version: [7, 9, 11] + steps: + - name: Setup GCC + uses: aminya/setup-cpp@v1 + with: + gcc: ${{ matrix.gcc-version }} + - uses: actions/checkout@v3 + - name: Build and run tests + run: | + scripts/initbuild.sh make-concurrent + scripts/test.sh + + gcc-32bit: + name: GCC 32bit + runs-on: ubuntu-20.04 + steps: + - name: Prepare + run: | + sudo apt update + sudo apt install gcc-multilib g++-multilib + - uses: actions/checkout@v3 + - name: Build and run tests + run: | + scripts/initbuild.sh make-32bit + scripts/test.sh + + intel: + name: Intel ${{ matrix.compiler }} + runs-on: ubuntu-20.04 + strategy: + fail-fast: false + matrix: + compiler: [icc, icx] + steps: + - name: Prepare + run: | + wget -qO - https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS-2023.PUB | sudo apt-key add - + echo "deb https://apt.repos.intel.com/oneapi all main" | sudo tee /etc/apt/sources.list.d/oneAPI.list + sudo apt update + sudo apt install intel-oneapi-compiler-dpcpp-cpp-and-cpp-classic-2021.4.0 + - name: Setup Intel oneAPI + run: | + source /opt/intel/oneapi/setvars.sh + printenv >> $GITHUB_ENV + - uses: actions/checkout@v3 + - name: Build and run tests + env: + CC: ${{ matrix.compiler }} + CXX: ${{ matrix.compiler }} + run: | + scripts/initbuild.sh make-concurrent + scripts/test.sh + + macos-clang: + name: macOS Clang + runs-on: macos-11 + steps: + - uses: actions/checkout@v3 + - name: Build and run tests + run: | + scripts/initbuild.sh make-concurrent + scripts/test.sh + + macos-gcc: + name: macOS GCC ${{ matrix.gcc-version }} + runs-on: macos-11 + strategy: + fail-fast: false + matrix: + gcc-version: [9, 12] + steps: + - uses: actions/checkout@v3 + - name: Build and run tests + env: + CC: gcc-${{ matrix.gcc-version }} + CXX: g++-${{ matrix.gcc-version }} + run: | + scripts/initbuild.sh make-concurrent + scripts/test.sh + + windows: + name: Windows Visual Studio ${{ matrix.version }} + runs-on: windows-${{ matrix.version }} + strategy: + fail-fast: false + matrix: + version: [2019, 2022] + steps: + - uses: microsoft/setup-msbuild@v1.1 + - uses: actions/checkout@v3 + - name: Build and run tests + run: | + cmake . + msbuild FlatCC.sln /m /property:Configuration=Release + ctest -VV + + cmake-minimum-required: + name: CMake 2.8.12 (min. required) + runs-on: ubuntu-20.04 + steps: + - name: Setup cmake + uses: jwlawson/actions-setup-cmake@v1 + with: + cmake-version: 2.8.12 + - uses: actions/checkout@v3 + - name: Build and run tests + run: | + cmake --version + scripts/initbuild.sh make-concurrent + scripts/test.sh diff --git a/README.md b/README.md index 43ff1a583..95fe80866 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,6 @@ -OS-X & Ubuntu: [![Build Status](https://travis-ci.org/dvidelabs/flatcc.svg?branch=master)](https://travis-ci.org/dvidelabs/flatcc) +Ubuntu, macOS and Windows: [![Build Status](https://github.com/dvidelabs/flatcc/actions/workflows/ci.yml/badge.svg)](https://github.com/dvidelabs/flatcc/actions/workflows/ci.yml) Windows: [![Windows Build Status](https://ci.appveyor.com/api/projects/status/github/dvidelabs/flatcc?branch=master&svg=true)](https://ci.appveyor.com/project/dvidelabs/flatcc) +Weekly: [![Build Status](https://github.com/dvidelabs/flatcc/actions/workflows/weekly.yml/badge.svg)](https://github.com/dvidelabs/flatcc/actions/workflows/weekly.yml) _The JSON parser may change the interface for parsing union vectors in a @@ -2226,8 +2227,15 @@ Optionally switch to a different build tool by choosing one of: scripts/initbuild.sh make-concurrent scripts/initbuild.sh ninja -where `ninja` is the default and `make-concurrent` is `make` with the `-j` -flag. A custom build configuration `X` can be added by adding a +where `ninja` is the default and `make-concurrent` is `make` with the `-j` flag. + +To enforce a 32-bit build on a 64-bit machine the following configuration +can be used: + + scripts/initbuild.sh make-32bit + +which uses `make` and provides the `-m32` flag to the compiler. +A custom build configuration `X` can be added by adding a `scripts/build.cfg.X` file. `scripts/initbuild.sh` cleans the build if a specific build diff --git a/scripts/build.cfg.make b/scripts/build.cfg.make index 684ecfc81..ae9c33258 100644 --- a/scripts/build.cfg.make +++ b/scripts/build.cfg.make @@ -1,2 +1,3 @@ FLATCC_BUILD_GEN="Unix Makefiles" FLATCC_BUILD_CMD=make +FLATCC_BUILD_FLAGS="" diff --git a/scripts/build.cfg.make-32bit b/scripts/build.cfg.make-32bit new file mode 100644 index 000000000..2299d679e --- /dev/null +++ b/scripts/build.cfg.make-32bit @@ -0,0 +1,3 @@ +FLATCC_BUILD_GEN="Unix Makefiles" +FLATCC_BUILD_CMD=make +FLATCC_BUILD_FLAGS="-DCMAKE_C_FLAGS=-m32 -DCMAKE_CXX_FLAGS=-m32" diff --git a/scripts/build.cfg.make-concurrent b/scripts/build.cfg.make-concurrent index 35d82f0ea..76846426b 100644 --- a/scripts/build.cfg.make-concurrent +++ b/scripts/build.cfg.make-concurrent @@ -1,2 +1,3 @@ FLATCC_BUILD_GEN="Unix Makefiles" FLATCC_BUILD_CMD="make -j" +FLATCC_BUILD_FLAGS="" diff --git a/scripts/build.cfg.ninja b/scripts/build.cfg.ninja index d69bb6d24..07ead7000 100644 --- a/scripts/build.cfg.ninja +++ b/scripts/build.cfg.ninja @@ -1,2 +1,3 @@ FLATCC_BUILD_GEN=Ninja FLATCC_BUILD_CMD=ninja +FLATCC_BUILD_FLAGS="" diff --git a/scripts/initbuild.sh b/scripts/initbuild.sh index 00561fa73..2b18cd257 100755 --- a/scripts/initbuild.sh +++ b/scripts/initbuild.sh @@ -36,5 +36,5 @@ mkdir -p ${ROOT}/build/Release rm -rf ${ROOT}/build/Debug/* rm -rf ${ROOT}/build/Release/* -cd ${ROOT}/build/Debug && cmake -G "$FLATCC_BUILD_GEN" ../.. -DCMAKE_BUILD_TYPE=Debug -cd ${ROOT}/build/Release && cmake -G "$FLATCC_BUILD_GEN" ../.. -DCMAKE_BUILD_TYPE=Release +cd ${ROOT}/build/Debug && cmake -G "$FLATCC_BUILD_GEN" $FLATCC_BUILD_FLAGS ../.. -DCMAKE_BUILD_TYPE=Debug +cd ${ROOT}/build/Release && cmake -G "$FLATCC_BUILD_GEN" $FLATCC_BUILD_FLAGS ../.. -DCMAKE_BUILD_TYPE=Release From 015cfece7e5a9c31033cef6ab5605ab19e1f9f74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Tue, 15 Nov 2022 09:13:14 +0100 Subject: [PATCH 19/37] Fix GCC on Mac builds aligned_alloc() seems not fully supported when using GCC on Mac, so use posix_memalign() instead. aligned_alloc() returns null in tests and https://en.cppreference.com/w/c/memory/aligned_alloc states: ".. not supported by the implementation causes the function to fail and return a null pointer" --- include/flatcc/portable/paligned_alloc.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/flatcc/portable/paligned_alloc.h b/include/flatcc/portable/paligned_alloc.h index 3dcf4efcd..70b00b9ea 100644 --- a/include/flatcc/portable/paligned_alloc.h +++ b/include/flatcc/portable/paligned_alloc.h @@ -61,6 +61,8 @@ extern "C" { #define PORTABLE_C11_ALIGNED_ALLOC 0 #elif defined (__clang__) #define PORTABLE_C11_ALIGNED_ALLOC 0 +#elif defined (__APPLE__) +#define PORTABLE_C11_ALIGNED_ALLOC 0 #elif defined(__IBMC__) #define PORTABLE_C11_ALIGNED_ALLOC 0 #elif (defined(__STDC__) && __STDC__ && defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) From 30712d9174b5bea61121a0fa5a6be911effe68d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Tue, 15 Nov 2022 09:42:12 +0100 Subject: [PATCH 20/37] Include clang 3-4 in precision warning workaround --- include/flatcc/portable/pparsefp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/flatcc/portable/pparsefp.h b/include/flatcc/portable/pparsefp.h index 6ece207de..d3e6f59ed 100644 --- a/include/flatcc/portable/pparsefp.h +++ b/include/flatcc/portable/pparsefp.h @@ -46,7 +46,7 @@ extern "C" { #define isinf(x) (!_finite(x)) #endif /* - * clang-5 through clang-8 but not clang-9 issues incorrect precision + * clang-3 through clang-8 but not clang-9 issues incorrect precision * loss warning with -Wconversion flag when cast is absent. */ #if defined(__clang__) From 16ac66aba77704546321b0d6f6698abd5eb72971 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Tue, 15 Nov 2022 10:14:15 +0100 Subject: [PATCH 21/37] Suppress missing-field-initializers warning pre-clang6 semantics.c:1551:32: error: missing field 'len' initializer fb_value_t index = { { { 0 } }, 0, 0 }; See https://reviews.llvm.org/D28148 and related https://reviews.llvm.org/rL314838 --- CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index f1f1a56c5..5a304e2fc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -188,6 +188,10 @@ if (CMAKE_C_COMPILER_ID MATCHES "Clang") if (FLATCC_IGNORE_CONST_COND) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-tautological-constant-out-of-range-compare") endif() + # Suppress warning relaxed in clang-6, see https://reviews.llvm.org/D28148 + if (CMAKE_C_COMPILER_VERSION VERSION_LESS 6) + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-missing-field-initializers") + endif() # To get assembly output # set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -save-temps") From 5f07eda43caabd81a2bfa2857af0e3f26dc6d4ee Mon Sep 17 00:00:00 2001 From: Anush Elangovan Date: Mon, 25 Oct 2021 15:46:57 -0700 Subject: [PATCH 22/37] Newer CMake is going to drop <2.8.12 So update to 2.8.12.2 (in Ubuntu 14.04) CentOS 7 has a way to install CMake3. This avoids the warning in newer CMakes: flatcc/CMakeLists.txt:5 (cmake_minimum_required): Compatibility with CMake < 2.8.12 will be removed from a future version of CMake. --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5a304e2fc..3bbfe252d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,8 +1,8 @@ # Ubuntu 14.04 (Trusty) -#cmake_minimum_required (VERSION 2.8.12.2) +cmake_minimum_required (VERSION 2.8.12.2) # Centos 7 #cmake_minimum_required (VERSION 2.8.11) -cmake_minimum_required (VERSION 2.8) +#cmake_minimum_required (VERSION 2.8) # Experimental for generating compile_commands.json so editors with # clangd language server support can use it. Symlink From 53886d26677bf4673be86160434b4263c9ca9342 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Fri, 18 Nov 2022 14:15:48 +0100 Subject: [PATCH 23/37] Add more lowered precision tests of floats (for GCC 32-bit) --- test/monster_test/monster_test.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/test/monster_test/monster_test.c b/test/monster_test/monster_test.c index 02fc6444d..f3150d8e3 100644 --- a/test/monster_test/monster_test.c +++ b/test/monster_test/monster_test.c @@ -163,8 +163,8 @@ int test_type_aliases(flatcc_builder_t *B) if (ns(TypeAliases_u16(ta)) != UINT16_MAX) goto failed; if (ns(TypeAliases_u32(ta)) != UINT32_MAX) goto failed; if (ns(TypeAliases_u64(ta)) != UINT64_MAX) goto failed; - if (ns(TypeAliases_f32(ta)) != 2.3f) goto failed; - if (ns(TypeAliases_f64(ta)) != 2.3) goto failed; + if (!parse_float_is_equal(ns(TypeAliases_f32(ta)), 2.3f)) goto failed; + if (!parse_double_is_equal(ns(TypeAliases_f64(ta)), 2.3)) goto failed; if (sizeof(ns(TypeAliases_i8(ta))) != 1) goto failed; if (sizeof(ns(TypeAliases_i16(ta))) != 2) goto failed; if (sizeof(ns(TypeAliases_i32(ta))) != 4) goto failed; @@ -380,7 +380,9 @@ int verify_monster(void *buffer) return -1; } if (nsc(is_native_pe())) { - if (vec->x != 1.0f || vec->y != 2.0f || vec->z != -3.2f) { + if (!parse_float_is_equal(vec->x, 1.0f) || + !parse_float_is_equal(vec->y, 2.0f) || + !parse_float_is_equal(vec->z, -3.2f)) { printf("Position is incorrect\n"); return -1; } @@ -399,7 +401,9 @@ int verify_monster(void *buffer) */ ns(Vec3_clear(&v)); /* Not strictly needed here. */ ns(Vec3_copy_from_pe(&v, vec)); - if (v.x != 1.0f || v.y != 2.0f || v.z != -3.2f) { + if (!parse_float_is_equal(v.x, 1.0f) || + !parse_float_is_equal(v.y, 2.0f) || + !parse_float_is_equal(v.z, -3.2f)) { printf("Position is incorrect after copy\n"); return -1; } @@ -1604,7 +1608,7 @@ int test_clone_slice(flatcc_builder_t *B) printf("sliced bool has wrong content\n"); goto done; } - if (ns(Monster_pos(mon2)->x != -42.3f)) { + if (!parse_float_is_equal(ns(Monster_pos(mon2))->x, -42.3f)) { printf("cloned pos struct failed\n"); goto done; }; @@ -2564,8 +2568,8 @@ int test_struct_buffer(flatcc_builder_t *B) /* Convert buffer to native in place - a nop on native platform. */ v = (ns(Vec3_t) *)vec3; ns(Vec3_from_pe(v)); - if (v->x != 1.0f || v->y != 2.0f || v->z != 3.0f - || v->test1 != 4.2 || v->test2 != ns(Color_Blue) + if (!parse_float_is_equal(v->x, 1.0f) || !parse_float_is_equal(v->y, 2.0f) || !parse_float_is_equal(v->z, 3.0f) + || !parse_double_is_equal(v->test1, 4.2) || v->test2 != ns(Color_Blue) || v->test3.a != 2730 || v->test3.b != -17 ) { printf("struct buffer not valid\n"); @@ -2629,8 +2633,8 @@ int test_typed_struct_buffer(flatcc_builder_t *B) /* Convert buffer to native in place - a nop on native platform. */ v = (ns(Vec3_t) *)vec3; ns(Vec3_from_pe(v)); - if (v->x != 1.0f || v->y != 2.0f || v->z != 3.0f - || v->test1 != 4.2 || v->test2 != ns(Color_Blue) + if (!parse_float_is_equal(v->x, 1.0f) || !parse_float_is_equal(v->y, 2.0f) || !parse_float_is_equal(v->z, 3.0f) + || !parse_double_is_equal(v->test1, 4.2) || v->test2 != ns(Color_Blue) || v->test3.a != 2730 || v->test3.b != -17 ) { printf("struct buffer not valid\n"); From d073ffd7280c9d15fef5b8155807bdcdaff96b68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Mon, 21 Nov 2022 12:07:21 +0100 Subject: [PATCH 24/37] Update CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 556d32a51..24c80955a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ - Added `parse_float/double_compare`, `parse_float/double_is_equal` to portable library, and added `parse_float/double_isnan` to mirror isinf. This should help with GCC 32-bit double precision conversion issue. +- Add Github Actions builds to replace stale Travis CI build. This also + includes source code fixes for some build variants. Although + Windows build is included it only covers recent 64-bit Windows. More + work is need for older Windows variants. (#250). ## [0.6.1] From f329ce8d82c88ab2b943be1a2051da2e2aab98b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Tue, 29 Nov 2022 18:34:31 +0100 Subject: [PATCH 25/37] Increase maximum allowed schema file siz --- CHANGELOG.md | 1 + config/config.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 24c80955a..bea295697 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ includes source code fixes for some build variants. Although Windows build is included it only covers recent 64-bit Windows. More work is need for older Windows variants. (#250). +- Increase maximum allowed schema file size from 64 KiB to 1 MB (#256). ## [0.6.1] diff --git a/config/config.h b/config/config.h index 041ec228c..b07fa6cc8 100644 --- a/config/config.h +++ b/config/config.h @@ -54,7 +54,7 @@ * is covers the accumulated size of all included files. 0 is unlimited. */ #ifndef FLATCC_MAX_SCHEMA_SIZE -#define FLATCC_MAX_SCHEMA_SIZE 64 * 1024 +#define FLATCC_MAX_SCHEMA_SIZE 1000000 #endif /* From 27a63fbf40676bb91dc7c6b8cbabfaefc204cbc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Mon, 5 Dec 2022 15:37:39 +0100 Subject: [PATCH 26/37] Fix seg fault in json parser --- CHANGELOG.md | 3 ++- src/runtime/json_parser.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bea295697..7ff4a4a95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,8 @@ Windows build is included it only covers recent 64-bit Windows. More work is need for older Windows variants. (#250). - Increase maximum allowed schema file size from 64 KiB to 1 MB (#256). - +- Fix seg fault in json parser while adding null characters to a too + short input string for a fixed length char array struct field (#257). ## [0.6.1] diff --git a/src/runtime/json_parser.c b/src/runtime/json_parser.c index 3fa929cca..4472af2d8 100644 --- a/src/runtime/json_parser.c +++ b/src/runtime/json_parser.c @@ -879,7 +879,7 @@ const char *flatcc_json_parser_char_array(flatcc_json_parser_t *ctx, if (ctx->flags & flatcc_json_parser_f_reject_array_underflow) { return flatcc_json_parser_set_error(ctx, buf, end, flatcc_json_parser_error_array_underflow); } - memset(s, 0, n - k); + memset(s, 0, n); } return flatcc_json_parser_string_end(ctx, buf, end); } From b20f5d1059d50394415453e2a6f719532ee6278d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikkel=20Fahn=C3=B8e=20J=C3=B8rgensen?= Date: Thu, 8 Dec 2022 14:56:06 +0100 Subject: [PATCH 27/37] Fix unary minus of unsigned warning --- include/flatcc/portable/pparsefp.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/flatcc/portable/pparsefp.h b/include/flatcc/portable/pparsefp.h index d3e6f59ed..7fa1c247d 100644 --- a/include/flatcc/portable/pparsefp.h +++ b/include/flatcc/portable/pparsefp.h @@ -175,7 +175,7 @@ static inline int32_t parse_float_compare(const float x, const float y) } /* - * Returns the absolute distance in flaoting point ULP (representational bit difference). + * Returns the absolute distance in floating point ULP (representational bit difference). * Uses signed return value so that INT64_MAX and INT32_MAX indicates NaN similar to * the compare function. */ @@ -186,7 +186,7 @@ static inline int64_t parse_double_dist(const double x, const double y) i64 = parse_double_compare(x, y); /* Absolute integer value of compare. */ - m64 = -(uint64_t)(i64 < 0); + m64 = (uint64_t)-(i64 < 0); return (int64_t)(((uint64_t)i64 + m64) ^ m64); } @@ -198,7 +198,7 @@ static inline int32_t parse_float_dist(const float x, const float y) i32 = parse_float_compare(x, y); /* Absolute integer value of compare. */ - m32 = -(uint32_t)(i32 < 0); + m32 = (uint32_t)-(i32 < 0); return (int32_t)(((uint32_t)i32 + m32) ^ m32); } From f892ef1fcab1805811de1e5c5f13182645fbc72b Mon Sep 17 00:00:00 2001 From: taloz5 <122386516+taloz5@users.noreply.github.com> Date: Sun, 15 Jan 2023 13:51:42 +0200 Subject: [PATCH 28/37] Avoid false detection of Clang while using Clang-cl --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3bbfe252d..492ec8bca 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -171,7 +171,7 @@ message(STATUS "lib install dir ${dist_dir}/${lib_dir}") # and constants should be turned off - those are plentiful. They are # silenced for Clang, GCC and MSVC in generated headers.headers. -if (CMAKE_C_COMPILER_ID MATCHES "Clang") +if (CMAKE_C_COMPILER_ID MATCHES "Clang" AND NOT "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC") # Clang or AppleClang message(STATUS "Setting Clang compiler options") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wstrict-prototypes") From 17cc4304d80145a5e70911687f646e76834cb99e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Fri, 28 Apr 2023 09:05:03 +0200 Subject: [PATCH 29/37] Install required gcc version in weekly CI for macOS gcc-9 was removed from Githubs macOS runner via: https://github.com/actions/runner-images/issues/7136 Adds a prepare step which installs required gcc version. --- .github/workflows/weekly.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/weekly.yml b/.github/workflows/weekly.yml index d88903f66..a46f27502 100644 --- a/.github/workflows/weekly.yml +++ b/.github/workflows/weekly.yml @@ -148,6 +148,9 @@ jobs: gcc-version: [9, 12] steps: - uses: actions/checkout@v3 + - name: Prepare + run: | + brew install gcc@${{ matrix.gcc-version }} - name: Build and run tests env: CC: gcc-${{ matrix.gcc-version }} From 05295db85a7c15fd0983b918fc8efddb9b1b5519 Mon Sep 17 00:00:00 2001 From: mikkelfj Date: Tue, 15 Aug 2023 17:11:19 +0200 Subject: [PATCH 30/37] Fix regression on root scope when setting empty namespace --- src/compiler/parser.c | 5 +++-- src/compiler/parser.h | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/compiler/parser.c b/src/compiler/parser.c index 4f31e0b95..e6a9bc99b 100644 --- a/src/compiler/parser.c +++ b/src/compiler/parser.c @@ -1069,7 +1069,7 @@ static void parse_namespace(fb_parser_t *P) if (optional(P, ';') && t) { /* Revert to global namespace. */ - P->current_scope = 0; + P->current_scope = P->root_scope; return; } if (P->token->id != LEX_TOK_ID) { @@ -1436,7 +1436,8 @@ int fb_init_parser(fb_parser_t *P, fb_options_t *opts, const char *name, P->schema.prefix.s = (char *)opts->ns; P->schema.prefix.len = (int)strlen(opts->ns); } - P->current_scope = fb_add_scope(P, 0); + P->root_scope = fb_add_scope(P, 0); + P->current_scope = P->root_scope; assert(P->current_scope == fb_scope_table_find(&P->schema.root_schema->scope_index, 0, 0)); return 0; } diff --git a/src/compiler/parser.h b/src/compiler/parser.h index ef2ecc15a..f09337fe2 100644 --- a/src/compiler/parser.h +++ b/src/compiler/parser.h @@ -71,6 +71,7 @@ struct fb_parser { int has_schema; fb_options_t opts; fb_schema_t schema; + fb_scope_t *root_scope; fb_scope_t *current_scope; char *path; char *referer_path; From eb5228f76d395bffe31a33398ff73e60dfba5914 Mon Sep 17 00:00:00 2001 From: mikkelfj Date: Tue, 15 Aug 2023 17:17:59 +0200 Subject: [PATCH 31/37] Update CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ff4a4a95..3cd18d4bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ - Increase maximum allowed schema file size from 64 KiB to 1 MB (#256). - Fix seg fault in json parser while adding null characters to a too short input string for a fixed length char array struct field (#257). +- Fix regression where empty namespace in schema does not reset root scope + correctly in parser (#265). ## [0.6.1] From fcaec3543913960cb4038ca96c9f800c5ead8c7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Wed, 4 Oct 2023 15:44:29 +0200 Subject: [PATCH 32/37] Update the apt key for Intel compilers in weekly CI Download the recommended keyfile for Intels APT repository and replace the deprecated `apt-key` command with `gpg`. `apt-key` has security flaws and will be removed after Ubuntu 22.04. --- .github/workflows/weekly.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/weekly.yml b/.github/workflows/weekly.yml index a46f27502..ba78294e4 100644 --- a/.github/workflows/weekly.yml +++ b/.github/workflows/weekly.yml @@ -112,8 +112,10 @@ jobs: steps: - name: Prepare run: | - wget -qO - https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS-2023.PUB | sudo apt-key add - - echo "deb https://apt.repos.intel.com/oneapi all main" | sudo tee /etc/apt/sources.list.d/oneAPI.list + wget -O- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB | \ + gpg --dearmor | sudo tee /usr/share/keyrings/oneapi-archive-keyring.gpg > /dev/null + echo "deb [signed-by=/usr/share/keyrings/oneapi-archive-keyring.gpg] https://apt.repos.intel.com/oneapi all main" | \ + sudo tee /etc/apt/sources.list.d/oneAPI.list sudo apt update sudo apt install intel-oneapi-compiler-dpcpp-cpp-and-cpp-classic-2021.4.0 - name: Setup Intel oneAPI From 7455d13c128f80841974110925b18aa9bd9cc6ef Mon Sep 17 00:00:00 2001 From: mikkelfj Date: Thu, 12 Oct 2023 15:34:53 +0200 Subject: [PATCH 33/37] Add missing unsigned cast to lexer character tests --- external/lex/luthor.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/external/lex/luthor.c b/external/lex/luthor.c index fc81985f3..5f037d726 100644 --- a/external/lex/luthor.c +++ b/external/lex/luthor.c @@ -176,16 +176,16 @@ static const char lex_alnum[256] = { #endif #ifndef lex_isdigit -#define lex_isdigit(c) ((c) >= '0' && (c) <= '9') +#define lex_isdigit(c) ((unsigned)(c) >= '0' && (unsigned)(c) <= '9') #endif #ifndef lex_ishexdigit -#define lex_ishexdigit(c) (((c) >= '0' && (c) <= '9') || ((c | 0x20) >= 'a' && (c | 0x20) <= 'f')) +#define lex_ishexdigit(c) (((c) >= '0' && ((unsigned)c) <= '9') || ((unsigned)(c | 0x20) >= 'a' && (unsigned)(c | 0x20) <= 'f')) #endif #ifndef lex_isctrl #include -#define lex_isctrl(c) ((c) < 0x20 || (c) == 0x7f) +#define lex_isctrl(c) (((unsigned)c) < 0x20 || (c) == 0x7f) #endif #ifndef lex_isblank From 29aa9b2d2e1286a1ff852c1067db1fdbbd38760e Mon Sep 17 00:00:00 2001 From: mikkelfj Date: Thu, 12 Oct 2023 15:35:32 +0200 Subject: [PATCH 34/37] Fix utf-8 mistaken for ctrl chars in comments --- src/compiler/parser.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/compiler/parser.c b/src/compiler/parser.c index e6a9bc99b..76a720258 100644 --- a/src/compiler/parser.c +++ b/src/compiler/parser.c @@ -155,8 +155,6 @@ void error_ref_sym(fb_parser_t *P, fb_ref_t *ref, const char *msg, fb_symbol_t * /* Accept numbers like -0x42 as integer literals. */ #define LEX_HEX_NUMERIC -#define lex_isblank(c) ((c) == ' ' || (c) == '\t') - #include "parser.h" #ifdef LEX_DEBUG @@ -1335,10 +1333,10 @@ static void inject_token(fb_token_t *t, const char *lex, long id) push_token((fb_parser_t*)context, LEX_TOK_COMMENT_UNTERMINATED, pos, pos) #define lex_emit_comment_ctrl(pos) \ - if (lex_isblank(*pos)) { \ + if (lex_isblank(*pos) || !lex_isctrl(*pos)) { \ push_comment((fb_parser_t*)context, pos, pos + 1); \ } else { \ - push_token((fb_parser_t*)context, LEX_TOK_COMMENT_CTRL, \ + push_token((fb_parser_t*)context, LEX_TOK_CTRL, \ pos, pos + 1); \ } From aefd0c8d56f6b24bbd4edb27f3f57cce4cd7cd0a Mon Sep 17 00:00:00 2001 From: mikkelfj Date: Thu, 12 Oct 2023 15:41:01 +0200 Subject: [PATCH 35/37] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cd18d4bf..d2d701c3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ short input string for a fixed length char array struct field (#257). - Fix regression where empty namespace in schema does not reset root scope correctly in parser (#265). +- Fix lexer checks that breaks with UTF-8, notably UTF-8 schema comments (#267). ## [0.6.1] From d53371009403012565746127fdfe3ee6a226c316 Mon Sep 17 00:00:00 2001 From: mikkelfj Date: Mon, 23 Oct 2023 17:52:30 +0200 Subject: [PATCH 36/37] Avoid uninitialized scope prefix leading to UB on memcpy, though len is 0 --- CHANGELOG.md | 1 + src/compiler/parser.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2d701c3f..ec7bece8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ - Fix regression where empty namespace in schema does not reset root scope correctly in parser (#265). - Fix lexer checks that breaks with UTF-8, notably UTF-8 schema comments (#267). +- Fix UB in memcpy(p, 0, 0) by initializing scope prefix (mostly to silence sanitizers). ## [0.6.1] diff --git a/src/compiler/parser.c b/src/compiler/parser.c index 76a720258..88c946132 100644 --- a/src/compiler/parser.c +++ b/src/compiler/parser.c @@ -1430,6 +1430,8 @@ int fb_init_parser(fb_parser_t *P, fb_options_t *opts, const char *name, P->schema.name.name.s.s = s; P->schema.name.name.s.len = (int)n; checkmem((P->schema.errorname = fb_create_basename(name, name_len, ""))); + P->schema.prefix.s = ""; + P->schema.prefix.len = 0; if (opts->ns) { P->schema.prefix.s = (char *)opts->ns; P->schema.prefix.len = (int)strlen(opts->ns); From 0e925e103529f1a76ba84f8f68e8a2cc4510393e Mon Sep 17 00:00:00 2001 From: mikkelfj Date: Tue, 24 Oct 2023 16:19:34 +0200 Subject: [PATCH 37/37] Add clang debug sanitizer flag and fix related warnings --- CHANGELOG.md | 2 +- CMakeLists.txt | 4 ++++ src/compiler/parser.c | 1 - src/runtime/builder.c | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec7bece8f..c9acb87e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ - Fix regression where empty namespace in schema does not reset root scope correctly in parser (#265). - Fix lexer checks that breaks with UTF-8, notably UTF-8 schema comments (#267). -- Fix UB in memcpy(p, 0, 0) by initializing scope prefix (mostly to silence sanitizers). +- Add sanitizer flag for clang debug and related warnings (input from several PRs incl. #237) ## [0.6.1] diff --git a/CMakeLists.txt b/CMakeLists.txt index 492ec8bca..ace0368e1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -188,6 +188,10 @@ if (CMAKE_C_COMPILER_ID MATCHES "Clang" AND NOT "${CMAKE_CXX_SIMULATE_ID}" STREQ if (FLATCC_IGNORE_CONST_COND) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-tautological-constant-out-of-range-compare") endif() + if (CMAKE_BUILD_TYPE MATCHES Debug) + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=undefined") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined") + endif() # Suppress warning relaxed in clang-6, see https://reviews.llvm.org/D28148 if (CMAKE_C_COMPILER_VERSION VERSION_LESS 6) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-missing-field-initializers") diff --git a/src/compiler/parser.c b/src/compiler/parser.c index 88c946132..006bb86f0 100644 --- a/src/compiler/parser.c +++ b/src/compiler/parser.c @@ -1270,7 +1270,6 @@ static void push_token(fb_parser_t *P, long id, const char *first, const char *l size_t offset; fb_token_t *t; - P->te = P->ts + P->tcapacity; if (P->token == P->te) { offset = (size_t)(P->token - P->ts); P->tcapacity = P->tcapacity ? 2 * P->tcapacity : 1024; diff --git a/src/runtime/builder.c b/src/runtime/builder.c index b62c2b667..1e5f8164a 100644 --- a/src/runtime/builder.c +++ b/src/runtime/builder.c @@ -177,7 +177,7 @@ int flatcc_builder_default_alloc(void *alloc_context, iovec_t *b, size_t request return 0; } -#define T_ptr(base, pos) ((void *)((uint8_t *)(base) + (uoffset_t)(pos))) +#define T_ptr(base, pos) ((void *)((size_t)(base) + (size_t)(pos))) #define ds_ptr(pos) (T_ptr(B->buffers[flatcc_builder_alloc_ds].iov_base, (pos))) #define vs_ptr(pos) (T_ptr(B->buffers[flatcc_builder_alloc_vs].iov_base, (pos))) #define pl_ptr(pos) (T_ptr(B->buffers[flatcc_builder_alloc_pl].iov_base, (pos)))