On Thu, Jul 28, 2022 at 5:43 PM Ilya Maximets <i.maxim...@ovn.org> wrote:
> LeakSanitizer for some reason reports these json objects as leaked,
> even though we do have references to them at the moment ovs_fatal()
> called from check_ovsdb_error().
>
> Previously it complained only with -O2, but with newer versions of
> clang/llvm it started complaining even with -O1. For example, negative
> ovsdb parsing tests are failing on ubuntu 22.04 with clang 14 if built
> with ASan and detect_leaks=1.
>
> Fix that by destroying the json object before aborting the process.
> And we may also build with default -O2 in CI with that change.
>
> Alternative implementation might be to just pass the json to destroy
> to every check_ovsdb_error() call, but indirect registering of the
> pointer seems a bit less invasive.
>
> Signed-off-by: Ilya Maximets <i.maxim...@ovn.org>
> ---
> .ci/linux-build.sh | 6 ++---
> tests/test-ovsdb.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 63 insertions(+), 4 deletions(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index c396ec1e8..22e138df3 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -245,15 +245,13 @@ fi
> if [ "$ASAN" ]; then
> # This will override default option configured in tests/atlocal.in.
> export ASAN_OPTIONS='detect_leaks=1'
> - # -O2 generates few false-positive memory leak reports in test-ovsdb
> - # application, so lowering optimizations to -O1 here.
> - CFLAGS_ASAN="-O1 -fno-omit-frame-pointer -fno-common
> -fsanitize=address"
> + CFLAGS_ASAN="-fno-omit-frame-pointer -fno-common -fsanitize=address"
> CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${CFLAGS_ASAN}"
> fi
>
> if [ "$UBSAN" ]; then
> # Use the default options configured in tests/atlocal.in, in
> UBSAN_OPTIONS.
> - CFLAGS_UBSAN="-O1 -fno-omit-frame-pointer -fno-common
> -fsanitize=undefined"
> + CFLAGS_UBSAN="-fno-omit-frame-pointer -fno-common
> -fsanitize=undefined"
> CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${CFLAGS_UBSAN}"
> fi
>
> diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
> index 343833151..965b0f913 100644
> --- a/tests/test-ovsdb.c
> +++ b/tests/test-ovsdb.c
> @@ -300,11 +300,24 @@ print_and_free_ovsdb_error(struct ovsdb_error *error)
> free(string);
> }
>
> +static struct json **json_to_destroy;
> +
> +static void
> +destroy_on_ovsdb_error(struct json **json)
> +{
> + json_to_destroy = json;
> +}
> +
> static void
> check_ovsdb_error(struct ovsdb_error *error)
> {
> if (error) {
> char *s = ovsdb_error_to_string_free(error);
> +
> + if (json_to_destroy) {
> + json_destroy(*json_to_destroy);
> + json_to_destroy = NULL;
> + }
> ovs_fatal(0, "%s", s);
> }
> }
> @@ -487,6 +500,8 @@ do_diff_data(struct ovs_cmdl_context *ctx)
> struct json *json;
> struct ovsdb_datum new, old, diff, reincarnation;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_type_from_json(&type, json));
> json_destroy(json);
> @@ -562,6 +577,8 @@ do_parse_atomic_type(struct ovs_cmdl_context *ctx)
> enum ovsdb_atomic_type type;
> struct json *json;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_atomic_type_from_json(&type, json));
> json_destroy(json);
> @@ -574,6 +591,8 @@ do_parse_base_type(struct ovs_cmdl_context *ctx)
> struct ovsdb_base_type base;
> struct json *json;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_base_type_from_json(&base, json));
> json_destroy(json);
> @@ -587,6 +606,8 @@ do_parse_type(struct ovs_cmdl_context *ctx)
> struct ovsdb_type type;
> struct json *json;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_type_from_json(&type, json));
> json_destroy(json);
> @@ -601,6 +622,8 @@ do_parse_atoms(struct ovs_cmdl_context *ctx)
> struct json *json;
> int i;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_base_type_from_json(&base, json));
> json_destroy(json);
> @@ -630,6 +653,8 @@ do_parse_atom_strings(struct ovs_cmdl_context *ctx)
> struct json *json;
> int i;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_base_type_from_json(&base, json));
> json_destroy(json);
> @@ -675,6 +700,8 @@ do_parse_data__(int argc, char *argv[],
> struct json *json;
> int i;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = unbox_json(parse_json(argv[1]));
> check_ovsdb_error(ovsdb_type_from_json(&type, json));
> json_destroy(json);
> @@ -706,6 +733,8 @@ do_parse_data_strings(struct ovs_cmdl_context *ctx)
> struct json *json;
> int i;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_type_from_json(&type, json));
> json_destroy(json);
> @@ -746,6 +775,8 @@ do_sort_atoms(struct ovs_cmdl_context *ctx)
> size_t n_atoms;
> int i;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_base_type_from_json(&base, json));
> json_destroy(json);
> @@ -785,6 +816,8 @@ do_parse_column(struct ovs_cmdl_context *ctx)
> struct ovsdb_column *column;
> struct json *json;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = parse_json(ctx->argv[2]);
> check_ovsdb_error(ovsdb_column_from_json(json, ctx->argv[1],
> &column));
> json_destroy(json);
> @@ -801,6 +834,8 @@ do_parse_table(struct ovs_cmdl_context *ctx)
>
> default_is_root = ctx->argc > 3 && !strcmp(ctx->argv[3], "true");
>
> + destroy_on_ovsdb_error(&json);
> +
> json = parse_json(ctx->argv[2]);
> check_ovsdb_error(ovsdb_table_schema_from_json(json, ctx->argv[1],
> &ts));
> json_destroy(json);
> @@ -817,6 +852,8 @@ do_parse_rows(struct ovs_cmdl_context *ctx)
> struct json *json;
> int i;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_table_schema_from_json(json, "mytable", &ts));
> json_destroy(json);
> @@ -876,6 +913,8 @@ do_compare_rows(struct ovs_cmdl_context *ctx)
> int n_rows;
> int i, j;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_table_schema_from_json(json, "mytable", &ts));
> json_destroy(json);
> @@ -935,6 +974,8 @@ do_parse_conditions(struct ovs_cmdl_context *ctx)
> int exit_code = 0;
> int i;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_table_schema_from_json(json, "mytable", &ts));
> json_destroy(json);
> @@ -977,6 +1018,8 @@ do_evaluate_condition__(struct ovs_cmdl_context *ctx,
> int mode)
> struct json *json;
> size_t i, j;
>
> + destroy_on_ovsdb_error(&json);
> +
> /* Parse table schema, create table. */
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_table_schema_from_json(json, "mytable", &ts));
> @@ -1064,6 +1107,8 @@ do_compare_conditions(struct ovs_cmdl_context *ctx)
> struct json *json;
> size_t i;
>
> + destroy_on_ovsdb_error(&json);
> +
> /* Parse table schema, create table. */
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_table_schema_from_json(json, "mytable", &ts));
> @@ -1105,6 +1150,8 @@ do_parse_mutations(struct ovs_cmdl_context *ctx)
> int exit_code = 0;
> int i;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_table_schema_from_json(json, "mytable", &ts));
> json_destroy(json);
> @@ -1144,6 +1191,8 @@ do_execute_mutations(struct ovs_cmdl_context *ctx)
> struct json *json;
> size_t i, j;
>
> + destroy_on_ovsdb_error(&json);
> +
> /* Parse table schema, create table. */
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_table_schema_from_json(json, "mytable", &ts));
> @@ -1268,6 +1317,8 @@ do_query(struct ovs_cmdl_context *ctx)
> int exit_code = 0;
> size_t i;
>
> + destroy_on_ovsdb_error(&json);
> +
> /* Parse table schema, create table. */
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_table_schema_from_json(json, "mytable", &ts));
> @@ -1362,6 +1413,8 @@ do_query_distinct(struct ovs_cmdl_context *ctx)
> int exit_code = 0;
> size_t i;
>
> + destroy_on_ovsdb_error(&json);
> +
> /* Parse table schema, create table. */
> json = unbox_json(parse_json(ctx->argv[1]));
> check_ovsdb_error(ovsdb_table_schema_from_json(json, "mytable", &ts));
> @@ -1489,6 +1542,8 @@ do_parse_schema(struct ovs_cmdl_context *ctx)
> struct ovsdb_schema *schema;
> struct json *json;
>
> + destroy_on_ovsdb_error(&json);
> +
> json = parse_json(ctx->argv[1]);
> check_ovsdb_error(ovsdb_schema_from_json(json, &schema));
> json_destroy(json);
> @@ -1504,6 +1559,8 @@ do_execute__(struct ovs_cmdl_context *ctx, bool ro)
> struct ovsdb *db;
> int i;
>
> + destroy_on_ovsdb_error(&json);
> +
> /* Create database. */
> json = parse_json(ctx->argv[1]);
> check_ovsdb_error(ovsdb_schema_from_json(json, &schema));
> @@ -1570,6 +1627,8 @@ do_trigger(struct ovs_cmdl_context *ctx)
> int number;
> int i;
>
> + destroy_on_ovsdb_error(&json);
> +
> /* Create database. */
> json = parse_json(ctx->argv[1]);
> check_ovsdb_error(ovsdb_schema_from_json(json, &schema));
> @@ -1796,6 +1855,8 @@ do_transact(struct ovs_cmdl_context *ctx)
> struct json *json;
> int i;
>
> + destroy_on_ovsdb_error(&json);
> +
> /* Create table. */
> json = parse_json("{\"name\": \"testdb\", "
> " \"tables\": "
> --
> 2.34.3
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Hi Ilya,
I actually saw that couple of times even when running the OVN tests.
Acked-by: Ales Musil <amu...@redhat.com>
Thanks,
Ales
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA <https://www.redhat.com>
amu...@redhat.com IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
