From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 95268 invoked by alias); 7 Jan 2020 23:43:35 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 95260 invoked by uid 89); 7 Jan 2020 23:43:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-28.0 required=5.0 tests=AWL,BAYES_00,ENV_AND_HDR_SPF_MATCH,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_JMF_BL,SPF_PASS,USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy=HX-Spam-Relays-External:209.85.167.193, H*RU:209.85.167.193, activating, closing X-HELO: mail-oi1-f193.google.com Received: from mail-oi1-f193.google.com (HELO mail-oi1-f193.google.com) (209.85.167.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 07 Jan 2020 23:43:34 +0000 Received: by mail-oi1-f193.google.com with SMTP id p125so1071702oif.10 for ; Tue, 07 Jan 2020 15:43:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=p8xpuLiVibhF6OdM3kKbEGci3DIZyyg0CsEc8nzDspQ=; b=Cj/zWeGATLw3zs7yQ8van1c8l2ln/tGIlgYgfQsVXE50XXeMvmdvxCiy6skzreMGqt SlD/ZJjfcTciZJNpA+BNSvd3q0CPr0bWA6lAJawYZn6V6UfMUHnFa9xi8XN6RQb6YErx ycWH2XVPxEPTmDf9gMmRNSakfH8ADC/nHA2+enGqvkno/gg6NvZJ/EZngOsc2RHcwWZD 0tPP8YFK1c2j0LB7KU8QmSlPoEyp6VwpL0EGq8jL7EhKTBNvZ8qcA07yqiYknDe783RK gjwhLlCI2gDyQviM4CSIiPbN9+jbHfSeN7wLwdWXgAD2hW3Qpaoebznaal/0znzZ6AE4 w+vw== MIME-Version: 1.0 References: <20191218190705.161582-1-cbiesinger@google.com> <20200102113102.GA4137@adacore.com> In-Reply-To: <20200102113102.GA4137@adacore.com> From: "Christian Biesinger via gdb-patches" Reply-To: Christian Biesinger Date: Tue, 07 Jan 2020 23:43:00 -0000 Message-ID: Subject: Re: [PATCH v2] Don't define _FORTIFY_SOURCE on mingw To: Joel Brobecker Cc: Christian Biesinger via gdb-patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2020-01/txt/msg00166.txt.bz2 Hi Joel, On Thu, Jan 2, 2020 at 5:31 AM Joel Brobecker wrote: > > Hi everyone, > > On Wed, Dec 18, 2019 at 01:07:05PM -0600, Christian Biesinger via gdb-patches wrote: > > Recent mingw versions require -lssp when using _FORTIFY_SOURCE, which > > gdb does (in common-defs.h) > > https://github.com/msys2/MINGW-packages/issues/5868#issuecomment-544107564 > > > > To avoid all the complications with checking for -lssp and making sure it's > > linked statically, just don't define it. > > > > gdb/ChangeLog: > > > > 2019-12-18 Christian Biesinger > > > > * gdbsupport/common-defs.h: Don't define _FORTIFY_SOURCE on mingw. > > Thanks for the patch! > > I hestitated a lot on this patch. On the one hand, it wasn't clear > to me that libssp was any different from the other library dependencies > that one can build against... Until it was mentioned that static linking > requires libssp_nonshared! Gaaah... Also, while some people consider it > a necessity to link statically, others would can be in a situation where > it is definitely better for them to link dynamically. Sorry, to clarify the issue there -- libssp_nonshared is not needed, what is needed are the right linker flags: -Wl,-Bstatic -lssp -Wl,-Bdynamic (since, per Eli, we want to link this statically) However, that means I can't just use AC_SEARCH_LIBS. So I'd have to either use AC_LINK_IFELSE, which is more complicated, or hardcode those flags if target matches mingw (doesn't sound too great). > At the end of the day, I think this might be the most sensible approach > after all. I imagine that someone wanting to build with FORTIFY can do > so by adding -D_FORTIFY_SOURCE=2 to the compilation flags, and then > the necessary linking options to add the libraries. For static linking, > one can pass the full path to the archive instead of using -lxxx. > In other words, this patch isn't closing the door for activating > _FORTIFY_SOURCE. Yes, that is true. > I was also thinking of adding a NEWS entry. However, IIUC, this option > had no effect with previous versions of MinGW? Thus, in the end, > this patch makes the situation stay the same regardless of MinGW > version, meaning no NEWS updated needed. My understanding is that this did use to work (maybe it automatically linked against -lssp?). But I'm not sure a NEWS entry is needed? This does not seem user-visible. > If my understanding is correct, then I am OK with this patch, and > you can push it to both master and gdb-9-branch. Even if people > object to the approach, this patch doesn't make it any more difficult > to enhance the build to have fortify on by default. Let me know your thoughts on the above, especially the NEWS question. > One small comment below... > > > > > Change-Id: Ide6870ab57198219a2ef78bc675768a789ca2b1d > > --- > > gdb/gdbsupport/common-defs.h | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/gdb/gdbsupport/common-defs.h b/gdb/gdbsupport/common-defs.h > > index 203bd8972d..53ce3c96ea 100644 > > --- a/gdb/gdbsupport/common-defs.h > > +++ b/gdb/gdbsupport/common-defs.h > > @@ -66,9 +66,13 @@ > > plus this seems like a reasonable safety measure. The check for > > optimization is required because _FORTIFY_SOURCE only works when > > optimization is enabled. If _FORTIFY_SOURCE is already defined, > > - then we don't do anything. */ > > + then we don't do anything. Also, on mingw, fortify requires > > mingw -> MinGW ;-) Thanks, will fix locally. Christian