Message ID | 20200515092018.31764-1-maennich@google.com |
---|---|
State | Committed |
Headers |
Return-Path: <libabigail-bounces@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1AC253851C22; Fri, 15 May 2020 09:20:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1AC253851C22 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1589534448; bh=oUVs14YO4F+/jHIH6H3ZeXdHedV7GegA+20kDhGznos=; h=Date:Subject:To:List-Id:List-Unsubscribe:List-Archive:List-Help: List-Subscribe:From:Reply-To:Cc:From; b=uWDzzbPnDdt1NPXk26HExOuxwafCINtcEH8Q3BLgxLLPksU/imKQoacZj9zXeFxmA wtnIhuIyj2lhQd/b5d2w3G97qdhFMQvOD4+jxTPzMF+xaoH/jjhFdMVrVKAoOrZvwV wfvsY4E90PJu5T3X3ttnPO3QmhGXCNla9SkD8MH8= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by sourceware.org (Postfix) with ESMTPS id 061D13851C22 for <libabigail@sourceware.org>; Fri, 15 May 2020 09:20:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 061D13851C22 Received: by mail-yb1-xb49.google.com with SMTP id 186so1987495ybq.1 for <libabigail@sourceware.org>; Fri, 15 May 2020 02:20:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:message-id:mime-version:subject:from:to:cc; bh=oUVs14YO4F+/jHIH6H3ZeXdHedV7GegA+20kDhGznos=; b=lIJ4INoljD6omRx472XoOkdCuB0TNFgMBBf8uBKe3guK9GtlyXDh6h+ZtK0B7fScOS NIoiINJQEgyMBbo1jRrUXqY0N6jYnXCCP/l4nh5gz1SjU3eLy+NLui2UYyx2aXVKj/kf rpiyRY/X+Mr/9MnVcUGRrv3bqrd9K18bMrdsINEldZk1kKwM0kCUkXwithy6jFi1X5n+ M/nk06fnFQ2nkkHaECGxtbpRkCAu/du3EFb4HvxNisIwtiEMFrZ1FhHdUO7+ZE4Fy/ms 7kTxmJAED4OVqotJ59J1IiCmz8t/SGQ9xNuFMyxwoTdzI5jOvGt8bFo4kWkqdW1JK2So vdUw== X-Gm-Message-State: AOAM530EgETMx+qVS61aphv0McwNPrwNoiIbL5FfAX0phIqR9HR8Mc07 t5ppmG1W51v7eWV2cXK30Eygeu7JZDIZPNXE9js1ce2cT8Jebuy8XgofXNh9m7UGcvewxWHUPWO X7Rk+9YqDm69lA7ko6ZBRFJnBf7RpqUKEVp71owlJ1Jvgz0R1wRhsm9JN6jSxEaZA7EEqInM= X-Google-Smtp-Source: ABdhPJw5Yxs6fr8n4Eb+eL+ssxuXpIIPxsXzn8wN04fSZIg8Rm9+xSnf+WLLKTjYJpEbyCoTIIJz4SV2iiV6yQ== X-Received: by 2002:a25:cc04:: with SMTP id l4mr3660460ybf.307.1589534435349; Fri, 15 May 2020 02:20:35 -0700 (PDT) Date: Fri, 15 May 2020 11:20:18 +0200 Message-Id: <20200515092018.31764-1-maennich@google.com> Mime-Version: 1.0 X-Mailer: git-send-email 2.26.2.761.g0e0b3e54be-goog Subject: [PATCH] configure: add more diagnostic options when ABIGAIL_DEVEL is set To: libabigail@sourceware.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-24.5 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project <libabigail.sourceware.org> List-Unsubscribe: <http://sourceware.org/mailman/options/libabigail>, <mailto:libabigail-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libabigail/> List-Help: <mailto:libabigail-request@sourceware.org?subject=help> List-Subscribe: <http://sourceware.org/mailman/listinfo/libabigail>, <mailto:libabigail-request@sourceware.org?subject=subscribe> From: Matthias Maennich via Libabigail <libabigail@sourceware.org> Reply-To: Matthias Maennich <maennich@google.com> Cc: maennich@google.com, gprocida@google.com, kernel-team@android.com Errors-To: libabigail-bounces@sourceware.org Sender: "Libabigail" <libabigail-bounces@sourceware.org> |
Series |
configure: add more diagnostic options when ABIGAIL_DEVEL is set
|
|
Commit Message
Matthias Männich
May 15, 2020, 9:20 a.m. UTC
When exporting ABIGAIL_DEVEL=1, add more flags to ABIGAIL_DEVEL that are
suitable for development to find issues during edit/compile/test time.
To accommodate the need for checked return values of system(), adjust
the tests to explicitely ignore them.
Note, unless bug #25989 is addressed, runtestannotate is failing.
See https://sourceware.org/bugzilla/show_bug.cgi?id=25989 for details.
* configure.ac: add -D_FORTIFY_SOURCE=2 and -D_GLIBCXX_DEBUG
compilation defines if ABIGAIL_DEVEL is set
Signed-off-by: Matthias Maennich <maennich@google.com>
---
configure.ac | 4 ++--
tests/test-diff-filter.cc | 2 +-
tests/test-diff-pkg.cc | 2 +-
tests/test-read-write.cc | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
Comments
Matthias Maennich <maennich@google.com> a écrit: > When exporting ABIGAIL_DEVEL=1, add more flags to ABIGAIL_DEVEL that are > suitable for development to find issues during edit/compile/test time. > > To accommodate the need for checked return values of system(), adjust > the tests to explicitely ignore them. > > Note, unless bug #25989 is addressed, runtestannotate is failing. > See https://sourceware.org/bugzilla/show_bug.cgi?id=25989 for details. > > * configure.ac: add -D_FORTIFY_SOURCE=2 and -D_GLIBCXX_DEBUG > compilation defines if ABIGAIL_DEVEL is set > > Signed-off-by: Matthias Maennich <maennich@google.com> Acked-By: Dodji Seketeli <dodji@redhat.com> When testing this with gcc 4.8.5, it appeared that -D_FORTIFY_SOURCE required optimization to work, so I set the optimization level to -O2. I added some source code changes required by the compiler when building with the resulting new value of CXXFLAGS. I have also updated the ChangeLog part of the commit message. I am adding below the patch that I have applied to master. Thanks. Cheers,
Hi Dodji! Thanks for working on this! On Mon, May 18, 2020 at 10:44:15AM +0200, Dodji Seketeli wrote: >Matthias Maennich <maennich@google.com> a écrit: > >> When exporting ABIGAIL_DEVEL=1, add more flags to ABIGAIL_DEVEL that are >> suitable for development to find issues during edit/compile/test time. >> >> To accommodate the need for checked return values of system(), adjust >> the tests to explicitely ignore them. >> >> Note, unless bug #25989 is addressed, runtestannotate is failing. >> See https://sourceware.org/bugzilla/show_bug.cgi?id=25989 for details. >> >> * configure.ac: add -D_FORTIFY_SOURCE=2 and -D_GLIBCXX_DEBUG >> compilation defines if ABIGAIL_DEVEL is set >> >> Signed-off-by: Matthias Maennich <maennich@google.com> >Acked-By: Dodji Seketeli <dodji@redhat.com> > >When testing this with gcc 4.8.5, it appeared that -D_FORTIFY_SOURCE >required optimization to work, so I set the optimization level to -O2. > >I added some source code changes required by the compiler when building >with the resulting new value of CXXFLAGS. > >I have also updated the ChangeLog part of the commit message. > >I am adding below the patch that I have applied to master. > >Thanks. > >Cheers, > >From d02de5a7846d7f44621c1e100e60f36f5a0c051e Mon Sep 17 00:00:00 2001 >From: Matthias Maennich <maennich@google.com> >Date: Fri, 15 May 2020 11:20:18 +0200 >Subject: [PATCH] configure: add more diagnostic options when ABIGAIL_DEVEL is > set > >When exporting ABIGAIL_DEVEL=1, add more flags to ABIGAIL_DEVEL that are >suitable for development to find issues during edit/compile/test time. > >The subsequent changes to source and test code are needed to make the >code compile with ABIGAIL_DEVEL=yes. > >Note, unless bug #25989 is addressed, runtestannotate is failing. >See https://sourceware.org/bugzilla/show_bug.cgi?id=25989 for details. > > * configure.ac: add -D_FORTIFY_SOURCE=2 and -D_GLIBCXX_DEBUG > compilation defines if ABIGAIL_DEVEL is set. Note that with GCC 4.8.5, > -D_FORTIFY_SOURCE=2 requires options to be set. So I am setting > the optimization level to -Og. > * src/abg-dwarf-reader.cc (read_context::{compute_canonical_die, > get_or_compute_canonical_die, associate_die_to_decl, > set_canonical_die_offset, schedule_type_for_late_canonicalization, > compare_dies}, get_scope_for_die, add_or_update_union_type) > (read_debug_info_into_corpus, build_ir_node_from_die): Initialize > the 'source' variable. > * tests/test-diff-filter.cc (main): Check the return value of the > system function. > * tests/test-diff-pkg.cc (main): Likewise. > * tests/test-read-write.cc (main): Likewise. > >Signed-off-by: Matthias Maennich <maennich@google.com> >Signed-off-by: Dodji Seketeli <dodji@redhat.com> >--- > configure.ac | 4 ++-- > src/abg-dwarf-reader.cc | 22 ++++++++++++---------- > tests/test-diff-filter.cc | 3 ++- > tests/test-diff-pkg.cc | 3 ++- > tests/test-read-write.cc | 3 ++- > 5 files changed, 20 insertions(+), 15 deletions(-) > >diff --git a/configure.ac b/configure.ac >index 1d6fe56..36296d6 100644 >--- a/configure.ac >+++ b/configure.ac >@@ -617,8 +617,8 @@ DEPS_LIBS="$XML_LIBS $LIBZIP_LIBS $ELF_LIBS $DW_LIBS" > AC_SUBST(DEPS_LIBS) > > if test x$ABIGAIL_DEVEL != x; then >- CFLAGS="-g -Wall -Wextra -Werror" >- CXXFLAGS="-g -Wall -Wextra -Werror" >+ CFLAGS="-Og -Wall -Wextra -Werror -D_FORTIFY_SOURCE=2" >+ CXXFLAGS="-Og -Wall -Wextra -Werror -D_FORTIFY_SOURCE=2 -D_GLIBCXX_DEBUG" You mentioned you needed to add -O2, but here it is now -Og. Note that this is likely unsuitable for ABIGAIL_DEVEL as it slows down `make check` quite significantly. Hence I put it into ABIGAIL_DEBUG instead. > fi > > if test x$ABIGAIL_DEBUG != x; then >diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc >index 5024deb..4bcc652 100644 >--- a/src/abg-dwarf-reader.cc >+++ b/src/abg-dwarf-reader.cc >@@ -3060,7 +3060,8 @@ public: > Dwarf_Die &canonical_die, > bool die_as_type) const > { >- die_source source; >+ die_source source = NO_DEBUG_INFO_DIE_SOURCE; Interesting, I did not see those on gcc 9. Maybe the compiler is now able to look through the initialization that is happening right after. >+ > ABG_ASSERT(get_die_source(die, source)); > > Dwarf_Off die_offset = dwarf_dieoffset(const_cast<Dwarf_Die*>(die)); >@@ -3298,7 +3299,7 @@ public: > size_t where, > bool die_as_type) const > { >- die_source source; >+ die_source source = NO_DEBUG_INFO_DIE_SOURCE; > ABG_ASSERT(get_die_source(die, source)); > > offset_offset_map_type &canonical_dies = >@@ -3521,7 +3522,7 @@ public: > size_t where_offset, > bool do_associate_by_repr = false) > { >- die_source source; >+ die_source source = NO_DEBUG_INFO_DIE_SOURCE; > ABG_ASSERT(get_die_source(die, source)); > > die_artefact_map_type& m = >@@ -4151,7 +4152,7 @@ public: > Dwarf_Off canonical_die_offset, > bool die_as_type) const > { >- die_source source; >+ die_source source = NO_DEBUG_INFO_DIE_SOURCE; > ABG_ASSERT(get_die_source(die, source)); > > Dwarf_Off die_offset = dwarf_dieoffset(const_cast<Dwarf_Die*>(die)); >@@ -4732,7 +4733,7 @@ public: > schedule_type_for_late_canonicalization(const Dwarf_Die *die) > { > Dwarf_Off o; >- die_source source; >+ die_source source = NO_DEBUG_INFO_DIE_SOURCE; > > Dwarf_Die equiv_die; > ABG_ASSERT(get_canonical_die(die, equiv_die, >@@ -11860,7 +11861,8 @@ compare_dies(const read_context& ctxt, > Dwarf_Off l_offset = dwarf_dieoffset(const_cast<Dwarf_Die*>(l)), > r_offset = dwarf_dieoffset(const_cast<Dwarf_Die*>(r)); > Dwarf_Off l_canonical_die_offset = 0, r_canonical_die_offset = 0; >- die_source l_die_source, r_die_source; >+ die_source l_die_source = NO_DEBUG_INFO_DIE_SOURCE, >+ r_die_source = NO_DEBUG_INFO_DIE_SOURCE; > ABG_ASSERT(ctxt.get_die_source(l, l_die_source)); > ABG_ASSERT(ctxt.get_die_source(r, r_die_source)); > >@@ -12700,7 +12702,7 @@ get_scope_for_die(read_context& ctxt, > bool called_for_public_decl, > size_t where_offset) > { >- die_source source_of_die; >+ die_source source_of_die = NO_DEBUG_INFO_DIE_SOURCE; > ABG_ASSERT(ctxt.get_die_source(die, source_of_die)); > > if (is_c_language(ctxt.cur_transl_unit()->get_language())) >@@ -14141,7 +14143,7 @@ add_or_update_union_type(read_context& ctxt, > if (tag != DW_TAG_union_type) > return result; > >- die_source source; >+ die_source source = NO_DEBUG_INFO_DIE_SOURCE; > ABG_ASSERT(ctxt.get_die_source(die, source)); > { > die_class_or_union_map_type::const_iterator i = >@@ -16232,7 +16234,7 @@ read_debug_info_into_corpus(read_context& ctxt) > static void > maybe_canonicalize_type(const Dwarf_Die *die, read_context& ctxt) > { >- die_source source; >+ die_source source = NO_DEBUG_INFO_DIE_SOURCE; > ABG_ASSERT(ctxt.get_die_source(die, source)); > > size_t die_offset = dwarf_dieoffset(const_cast<Dwarf_Die*>(die)); >@@ -16445,7 +16447,7 @@ build_ir_node_from_die(read_context& ctxt, > return result; > } > >- die_source source_of_die; >+ die_source source_of_die = NO_DEBUG_INFO_DIE_SOURCE; > ABG_ASSERT(ctxt.get_die_source(die, source_of_die)); > > if ((result = ctxt.lookup_decl_from_die_offset(dwarf_dieoffset(die), >diff --git a/tests/test-diff-filter.cc b/tests/test-diff-filter.cc >index 000da6d..215485b 100644 >--- a/tests/test-diff-filter.cc >+++ b/tests/test-diff-filter.cc >@@ -708,7 +708,8 @@ main() > { > is_ok = false; > if (!t->diff_cmd.empty()) >- system(t->diff_cmd.c_str()); >+ if (system(t->diff_cmd.c_str()) == -1) >+ cerr << "execution of '" << t->diff_cmd << "' failed\n"; According to the man page I have, "Exit status is 0 if inputs are the same, 1 if different, 2 if trouble." Hence, should we rather test for != 0 instead? Same for two more locations of this. Cheers, Matthias > if (!t->error_message.empty()) > cerr << t->error_message << '\n'; > } >diff --git a/tests/test-diff-pkg.cc b/tests/test-diff-pkg.cc >index 51f117a..8e324da 100644 >--- a/tests/test-diff-pkg.cc >+++ b/tests/test-diff-pkg.cc >@@ -840,7 +840,8 @@ main() > { > is_ok = false; > if (!t->diff_cmd.empty()) >- system(t->diff_cmd.c_str()); >+ if (system(t->diff_cmd.c_str()) == -1) >+ cerr << "execution of '" << t->diff_cmd << "' failed\n"; > if (!t->error_message.empty()) > cerr << t->error_message << '\n'; > } >diff --git a/tests/test-read-write.cc b/tests/test-read-write.cc >index f32fc25..7d24bee 100644 >--- a/tests/test-read-write.cc >+++ b/tests/test-read-write.cc >@@ -404,7 +404,8 @@ main() > cerr << t->error_message << '\n'; > > if (!t->diff_cmd.empty()) >- system(t->diff_cmd.c_str()); >+ if (system(t->diff_cmd.c_str()) == -1) >+ cerr << "execution of '" << t->diff_cmd << "' failed\n"; > } > } > >-- >1.8.3.1 > > >-- > Dodji
diff --git a/configure.ac b/configure.ac index 1d6fe56bb757..fa4a0a947969 100644 --- a/configure.ac +++ b/configure.ac @@ -617,8 +617,8 @@ DEPS_LIBS="$XML_LIBS $LIBZIP_LIBS $ELF_LIBS $DW_LIBS" AC_SUBST(DEPS_LIBS) if test x$ABIGAIL_DEVEL != x; then - CFLAGS="-g -Wall -Wextra -Werror" - CXXFLAGS="-g -Wall -Wextra -Werror" + CFLAGS="-g -Wall -Wextra -Werror -D_FORTIFY_SOURCE=2" + CXXFLAGS="-g -Wall -Wextra -Werror -D_FORTIFY_SOURCE=2 -D_GLIBCXX_DEBUG" fi if test x$ABIGAIL_DEBUG != x; then diff --git a/tests/test-diff-filter.cc b/tests/test-diff-filter.cc index 000da6da188b..60f9c769148a 100644 --- a/tests/test-diff-filter.cc +++ b/tests/test-diff-filter.cc @@ -708,7 +708,7 @@ main() { is_ok = false; if (!t->diff_cmd.empty()) - system(t->diff_cmd.c_str()); + (void)system(t->diff_cmd.c_str()); if (!t->error_message.empty()) cerr << t->error_message << '\n'; } diff --git a/tests/test-diff-pkg.cc b/tests/test-diff-pkg.cc index 51f117a47833..68e095226d92 100644 --- a/tests/test-diff-pkg.cc +++ b/tests/test-diff-pkg.cc @@ -840,7 +840,7 @@ main() { is_ok = false; if (!t->diff_cmd.empty()) - system(t->diff_cmd.c_str()); + (void)system(t->diff_cmd.c_str()); if (!t->error_message.empty()) cerr << t->error_message << '\n'; } diff --git a/tests/test-read-write.cc b/tests/test-read-write.cc index f32fc25d6191..52da379461cf 100644 --- a/tests/test-read-write.cc +++ b/tests/test-read-write.cc @@ -404,7 +404,7 @@ main() cerr << t->error_message << '\n'; if (!t->diff_cmd.empty()) - system(t->diff_cmd.c_str()); + (void)system(t->diff_cmd.c_str()); } }