public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Hans-Peter Nilsson <hp@bitrange.com>
To: Thomas Klein <th.r.klein@web.de>
Cc: gcc-patches@gcc.gnu.org, joey.ye.cc@gmail.com
Subject: Re: Ping: C-family stack check for threads
Date: Wed, 13 Jul 2011 12:57:00 -0000	[thread overview]
Message-ID: <alpine.BSF.2.00.1107130754240.21776@dair.pair.com> (raw)
In-Reply-To: <4E108558.7000300@web.de>

On Sun, 3 Jul 2011, Thomas Klein wrote:
> Ye Joey wrote:
> >  Thomas,
> >
> >  I think your are working on a very useful feature. I have ARM MCU
> >  applications running of out stack space and resulting strange
> >  behaviors silently. I'd like to try your patch and probably give
> >  further comments

I also think this will be a very useful feature (not just "for
threads"), and I hope you'll persevere through the review
process. ;)  Not your first patch and you have copyright
assignments in place, so that's covered.

The first thing I see is that you need to fix the issues
regarding the GCC coding standards,
<http://gcc.gnu.org/codingconventions.html> as that is a hurdle
for reviewers, and you don't want that.  Be very careful.  I
haven't ran contrib/check_GNU_style.sh myself but maybe it'll be
helpful.

The second issue I see is that documentation for the new
patterns is missing, that should go in gcc/doc/md.texi,
somewhere under @node Standard Names.  I can imagine there'll be
a thing or two to tweak regarding them and that best reviewed
through the documentation.

Generally, as much as possible should be general and not
ARM-specific.  If you need helper functions, add them to libgcc.


>
> Regards
>   Thomas Klein
>
> gcc/ChangeLog
> 2011-07-03  Thomas Klein<th.r.klein@web.de>  <mailto:th.r.klein@web.de>
>
>     * opts.c (common_handle_option): introduce additional stack checking
>     parameters "direct" and "indirect"
>     * flag-types.h (enum stack_check_type): Likewise
>     * explow.c (allocate_dynamic_stack_space):
>     - suppress stack probing if parameter "direct", "indirect" or if a
>     stack-limit is given
>     - do additional read of limit value if parameter "indirect" and a
>     stack-limit symbol is given
>     - emit a call to a stack_failure function [as an alternative to a trap
>     call]

No bullet list in the changelog, please.  Individual sentences.
Follow the existing format; full sentences with capitalization
and all that.

brgds, H-P

  parent reply	other threads:[~2011-07-13 12:14 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-03 13:09 Thomas Klein
2011-07-03 17:56 ` Richard Henderson
2011-07-13 12:57 ` Hans-Peter Nilsson [this message]
  -- strict thread matches above, loose matches on Subject: below --
2011-09-20 17:09 Thomas Klein
2011-09-20 22:07 ` Joseph S. Myers
2011-09-04 15:48 Thomas Klein
2011-09-05  9:45 ` Ye Joey
2011-09-05 18:25   ` Thomas Klein
2011-07-04 20:28 Thomas Klein
2011-07-05 16:11 ` Richard Henderson
2011-08-02 17:22   ` Thomas Klein
2011-06-24 14:10 Thomas Klein
2011-06-30  9:36 ` Ye Joey

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=alpine.BSF.2.00.1107130754240.21776@dair.pair.com \
    --to=hp@bitrange.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=joey.ye.cc@gmail.com \
    --cc=th.r.klein@web.de \
    /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).