Message ID | 20181030214236.29081-1-sergiodj@redhat.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 71063 invoked by alias); 30 Oct 2018 21:42:56 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 71049 invoked by uid 89); 30 Oct 2018 21:42:56 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=bugs.gentoo.org, commondefsh, gdbo, gdb.o X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 30 Oct 2018 21:42:54 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BB51131256A6 for <gdb-patches@sourceware.org>; Tue, 30 Oct 2018 21:42:53 +0000 (UTC) Received: from psique.yyz.redhat.com (unused-10-15-17-196.yyz.redhat.com [10.15.17.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0FDB160C55; Tue, 30 Oct 2018 21:42:39 +0000 (UTC) From: Sergio Durigan Junior <sergiodj@redhat.com> To: GDB Patches <gdb-patches@sourceware.org> Cc: Sergio Durigan Junior <sergiodj@redhat.com> Subject: [PATCH] Fix PR gdb/23835: Don't redefine _FORTIFY_SOURCE if it's already defined Date: Tue, 30 Oct 2018 17:42:36 -0400 Message-Id: <20181030214236.29081-1-sergiodj@redhat.com> X-IsSubscribed: yes |
Commit Message
Sergio Durigan Junior
Oct. 30, 2018, 9:42 p.m. UTC
Gentoo has a local GCC patch which always defines _FORTIFY_SOURCE=2. This causes a build problem when building GDB there, because "common/common-defs.h" also defines _FORTIFY_SOURCE=2: CXX gdb.o In file included from ../../gdb/defs.h:28:0, from ../../gdb/gdb.c:19: ../../gdb/common/common-defs.h:71:0: error: "_FORTIFY_SOURCE" redefined [-Werror] #define _FORTIFY_SOURCE 2 <built-in>: note: this is the location of the previous definition cc1plus: all warnings being treated as errors make[2]: *** [Makefile:1619: gdb.o] Error 1 Even though it is questionable whether Gentoo's approach is the correct one: https://jira.mongodb.org/browse/SERVER-29982 https://bugs.gentoo.org/621036 it is still possible for GDB to be a bit more robust here and make sure it just defines _FORTIFY_SOURCE if it hasn't been defined already. This patch does that. Tested by rebuilding and making sure the macro was defined. gdb/ChangeLog: 2018-10-30 Sergio Durigan Junior <sergiodj@redhat.com> PR gdb/23835 * common/common-defs.h: Don't redefine _FORTIFY_SOURCE is it's already defined. --- gdb/ChangeLog | 6 ++++++ gdb/common/common-defs.h | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-)
Comments
On 2018-10-30 17:42, Sergio Durigan Junior wrote: > Gentoo has a local GCC patch which always defines _FORTIFY_SOURCE=2. > This causes a build problem when building GDB there, because > "common/common-defs.h" also defines _FORTIFY_SOURCE=2: > > CXX gdb.o > In file included from ../../gdb/defs.h:28:0, > from ../../gdb/gdb.c:19: > ../../gdb/common/common-defs.h:71:0: error: "_FORTIFY_SOURCE" > redefined [-Werror] > #define _FORTIFY_SOURCE 2 > > <built-in>: note: this is the location of the previous definition > cc1plus: all warnings being treated as errors > make[2]: *** [Makefile:1619: gdb.o] Error 1 > > Even though it is questionable whether Gentoo's approach is the > correct one: > > https://jira.mongodb.org/browse/SERVER-29982 > https://bugs.gentoo.org/621036 > > it is still possible for GDB to be a bit more robust here and make > sure it just defines _FORTIFY_SOURCE if it hasn't been defined > already. This patch does that. > > Tested by rebuilding and making sure the macro was defined. I think it makes sense, it also gives the user the possibility to override it, if they don't like our value. Give it a few days to give others a change to respond. If you don't hear anything in ~1 week, please go ahead and push. > gdb/ChangeLog: > 2018-10-30 Sergio Durigan Junior <sergiodj@redhat.com> > > PR gdb/23835 > * common/common-defs.h: Don't redefine _FORTIFY_SOURCE is it's > already defined. > --- > gdb/ChangeLog | 6 ++++++ > gdb/common/common-defs.h | 5 +++-- > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 940300f95a..239d7e16c2 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,9 @@ > +2018-10-30 Sergio Durigan Junior <sergiodj@redhat.com> > + > + PR gdb/23835 > + * common/common-defs.h: Don't redefine _FORTIFY_SOURCE is it's is -> if. Simon
> > Gentoo has a local GCC patch which always defines _FORTIFY_SOURCE=2. > > This causes a build problem when building GDB there, because > > "common/common-defs.h" also defines _FORTIFY_SOURCE=2: > > > > CXX gdb.o > > In file included from ../../gdb/defs.h:28:0, > > from ../../gdb/gdb.c:19: > > ../../gdb/common/common-defs.h:71:0: error: "_FORTIFY_SOURCE" > > redefined [-Werror] > > #define _FORTIFY_SOURCE 2 > > > > <built-in>: note: this is the location of the previous definition > > cc1plus: all warnings being treated as errors > > make[2]: *** [Makefile:1619: gdb.o] Error 1 > > > > Even though it is questionable whether Gentoo's approach is the > > correct one: > > > > https://jira.mongodb.org/browse/SERVER-29982 > > https://bugs.gentoo.org/621036 > > > > it is still possible for GDB to be a bit more robust here and make > > sure it just defines _FORTIFY_SOURCE if it hasn't been defined > > already. This patch does that. > > > > Tested by rebuilding and making sure the macro was defined. > > I think it makes sense, it also gives the user the possibility to override > it, if they don't like our value. Give it a few days to give others a > change to respond. If you don't hear anything in ~1 week, please go ahead > and push. Good point about allowing the user to override it. The patch looks OK to me too.
On Tuesday, October 30 2018, Simon Marchi wrote: > On 2018-10-30 17:42, Sergio Durigan Junior wrote: >> Gentoo has a local GCC patch which always defines _FORTIFY_SOURCE=2. >> This causes a build problem when building GDB there, because >> "common/common-defs.h" also defines _FORTIFY_SOURCE=2: >> >> CXX gdb.o >> In file included from ../../gdb/defs.h:28:0, >> from ../../gdb/gdb.c:19: >> ../../gdb/common/common-defs.h:71:0: error: "_FORTIFY_SOURCE" >> redefined [-Werror] >> #define _FORTIFY_SOURCE 2 >> >> <built-in>: note: this is the location of the previous definition >> cc1plus: all warnings being treated as errors >> make[2]: *** [Makefile:1619: gdb.o] Error 1 >> >> Even though it is questionable whether Gentoo's approach is the >> correct one: >> >> https://jira.mongodb.org/browse/SERVER-29982 >> https://bugs.gentoo.org/621036 >> >> it is still possible for GDB to be a bit more robust here and make >> sure it just defines _FORTIFY_SOURCE if it hasn't been defined >> already. This patch does that. >> >> Tested by rebuilding and making sure the macro was defined. > > I think it makes sense, it also gives the user the possibility to > override it, if they don't like our value. Give it a few days to give > others a change to respond. If you don't hear anything in ~1 week, > please go ahead and push. Thanks for the review. I'll wait a week. >> gdb/ChangeLog: >> 2018-10-30 Sergio Durigan Junior <sergiodj@redhat.com> >> >> PR gdb/23835 >> * common/common-defs.h: Don't redefine _FORTIFY_SOURCE is it's >> already defined. >> --- >> gdb/ChangeLog | 6 ++++++ >> gdb/common/common-defs.h | 5 +++-- >> 2 files changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/gdb/ChangeLog b/gdb/ChangeLog >> index 940300f95a..239d7e16c2 100644 >> --- a/gdb/ChangeLog >> +++ b/gdb/ChangeLog >> @@ -1,3 +1,9 @@ >> +2018-10-30 Sergio Durigan Junior <sergiodj@redhat.com> >> + >> + PR gdb/23835 >> + * common/common-defs.h: Don't redefine _FORTIFY_SOURCE is it's > > is -> if. Fixed.
On Tuesday, October 30 2018, Joel Brobecker wrote: >> > Gentoo has a local GCC patch which always defines _FORTIFY_SOURCE=2. >> > This causes a build problem when building GDB there, because >> > "common/common-defs.h" also defines _FORTIFY_SOURCE=2: >> > >> > CXX gdb.o >> > In file included from ../../gdb/defs.h:28:0, >> > from ../../gdb/gdb.c:19: >> > ../../gdb/common/common-defs.h:71:0: error: "_FORTIFY_SOURCE" >> > redefined [-Werror] >> > #define _FORTIFY_SOURCE 2 >> > >> > <built-in>: note: this is the location of the previous definition >> > cc1plus: all warnings being treated as errors >> > make[2]: *** [Makefile:1619: gdb.o] Error 1 >> > >> > Even though it is questionable whether Gentoo's approach is the >> > correct one: >> > >> > https://jira.mongodb.org/browse/SERVER-29982 >> > https://bugs.gentoo.org/621036 >> > >> > it is still possible for GDB to be a bit more robust here and make >> > sure it just defines _FORTIFY_SOURCE if it hasn't been defined >> > already. This patch does that. >> > >> > Tested by rebuilding and making sure the macro was defined. >> >> I think it makes sense, it also gives the user the possibility to override >> it, if they don't like our value. Give it a few days to give others a >> change to respond. If you don't hear anything in ~1 week, please go ahead >> and push. > > Good point about allowing the user to override it. > > The patch looks OK to me too. Thanks for the review, Joel. Given that two global maintainers replied and approved the patch, I assume it's OK if I go ahead and push it...? Thanks,
> Thanks for the review, Joel. > > Given that two global maintainers replied and approved the patch, I > assume it's OK if I go ahead and push it...? Yes, sorry. That was part of the intent of my message; it's a small patch anyway, so if we find there are problems with it, or objections we can't resolve quickly, it's easy to revert.
On Wednesday, October 31 2018, Joel Brobecker wrote: >> Thanks for the review, Joel. >> >> Given that two global maintainers replied and approved the patch, I >> assume it's OK if I go ahead and push it...? > > Yes, sorry. That was part of the intent of my message; it's > a small patch anyway, so if we find there are problems with it, > or objections we can't resolve quickly, it's easy to revert. Thanks, Joel. I pushed the patch now. 656efb5e2691b2bd29573d9985d20206c47b6927
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 940300f95a..239d7e16c2 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2018-10-30 Sergio Durigan Junior <sergiodj@redhat.com> + + PR gdb/23835 + * common/common-defs.h: Don't redefine _FORTIFY_SOURCE is it's + already defined. + 2018-10-30 Tom Tromey <tom@tromey.com> * main.c (captured_main_1): Check return value of bfd_init. diff --git a/gdb/common/common-defs.h b/gdb/common/common-defs.h index 58445b1611..86f7c1ab9a 100644 --- a/gdb/common/common-defs.h +++ b/gdb/common/common-defs.h @@ -65,9 +65,10 @@ enable it here in order to try to catch these problems earlier; plus this seems like a reasonable safety measure. The check for optimization is required because _FORTIFY_SOURCE only works when - optimization is enabled. */ + optimization is enabled. If _FORTIFY_SOURCE is already defined, + then we don't do anything. */ -#if defined __OPTIMIZE__ && __OPTIMIZE__ > 0 +#if !defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ > 0 #define _FORTIFY_SOURCE 2 #endif