From: Segher Boessenkool <segher@kernel.crashing.org>
To: Mark Wielaard <mark@klomp.org>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.
Date: Thu, 29 Nov 2018 16:41:00 -0000 [thread overview]
Message-ID: <20181129164113.GC23873@gate.crashing.org> (raw)
In-Reply-To: <f712e22512798f6c01743ac1b7d48418f7cca920.camel@klomp.org>
Hi Mark,
On Thu, Nov 29, 2018 at 12:57:03PM +0100, Mark Wielaard wrote:
> On Wed, 2018-11-28 at 15:00 -0600, Segher Boessenkool wrote:
> > On Tue, Nov 27, 2018 at 08:54:07PM +0100, Mark Wielaard wrote:
> > > On Tue, 2018-11-27 at 12:37 -0600, Segher Boessenkool wrote:
> > > > > diff --git a/gcc/config/rs6000/sysv4.h
> > > > > b/gcc/config/rs6000/sysv4.h
> > > > > index 0c67634..9330acf 100644
> > > > > --- a/gcc/config/rs6000/sysv4.h
> > > > > +++ b/gcc/config/rs6000/sysv4.h
> > > > > @@ -972,6 +972,11 @@ ncrtn.o%s"
> > > > > /* Generate entries in .fixup for relocatable addresses. */
> > > > > #define RELOCATABLE_NEEDS_FIXUP 1
> > > > >
> > > > > +#if defined (POWERPC_LINUX) || defined (POWERPC_FREEBSD)
> > > > > + #define TARGET_HAS_DEFAULT_NOEXEC_STACK (TARGET_32BIT \
> > > > > + || DEFAULT_ABI ==
> > > > > ABI_ELFv2)
> > > > > +#endif
> > > >
> > > > I don't think this belongs in sysv4.h .
> > >
> > > I might have gotten lost in the tree of defines/macros.
> > >
> > > There are two sysv4.h files gcc/config/rs6000/sysv4.h and
> > > gcc/config/powerpcspe/sysv4.h
> >
> > Forget about powerpcspe please, I am talking about rs6000 only.
>
> I don't know the differences between the config backends, but it looks
> like at least some of the configs were just copy/pasted which might
> explain why they both define things the same (in sysv4.h). And they
Yes, things are copied. My point is, my review is for rs6000/ only :-)
> > You want linux.h and freebsd.h, maybe the "64" versions of those separately.
> > Or put this in rs6000.h. sysv4.h is a random header for this, it doesn't
> > belong there.
>
> The reason I added it to sysv4.h is because it matches where the
> TARGET_ASM_FILE_END hook is setup. It might make sense to have
Yeah, but it does not make sense to put it there, since you are handling
other ABIs as well.
> So if I understand correctly you would like to have:
>
> rs6000/linux.h and rs6000/freebsd.h:
> #define TARGET_NEEDS_EXEC_STACK_FLAG_FOR_TRAMPOLINES 1
>
> rs6000/linux64.h and rs6000/freebsd64.h:
> #define TARGET_NEEDS_EXEC_STACK_FLAG_FOR_TRAMPOLINES (DEFAULT_ABI == ABI_ELFv2)
Something like that yes. Or in rs6000.h, maybe. Some location that makes
sense :-)
Segher
prev parent reply other threads:[~2018-11-29 16:41 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-26 9:13 Mark Wielaard
2018-11-26 13:19 ` Paul Koning
2018-11-26 14:36 ` Mark Wielaard
2018-11-26 18:29 ` Joseph Myers
2018-11-26 19:23 ` Mark Wielaard
2018-11-27 18:37 ` Segher Boessenkool
2018-11-27 19:54 ` Mark Wielaard
2018-11-28 21:01 ` Segher Boessenkool
2018-11-29 11:57 ` Mark Wielaard
2018-11-29 16:41 ` Segher Boessenkool [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181129164113.GC23873@gate.crashing.org \
--to=segher@kernel.crashing.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=mark@klomp.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).