Re: [PATCH-v4] [SPARC] Add a workaround for the LEON3FT store-store errata
> 2017-07-07 Daniel Cederman > > * config/sparc/sparc.c (sparc_do_work_around_errata): Insert NOP > instructions to prevent sequences that can trigger the store-store > errata for certain LEON3FT processors. > (sparc_option_override): -mfix-ut699, -mfix-ut700, and > -mfix-gr712rc enables the errata workaround. > * config/sparc/sparc.md: Prevent stores in delay slot. > * config/sparc/sparc.opt: Add -mfix-ut700 and -mfix-gr712rc flag. > * doc/invoke.texi: Document -mfix-ut700 and -mfix-gr712rc flag. Applied without the undocumented tweaks to the divdf3_fix and sqrtdf2_fix patterns. Why are 2 nops necessary here? The stored value doesn't matter. And the length attribute should be adjusted if nops are added to the pattern. -- Eric Botcazou
RE: [PATCH][Aarch64] Relational compare zero not merged into subtract
James,
The subtract instruction only reliably sets the N and Z flags. We convey this
information in aarch64_seelct_cc_mode.
Regards,
Michael Collison
-Original Message-
From: James Greenhalgh [mailto:james.greenha...@arm.com]
Sent: Monday, July 10, 2017 10:12 AM
To: Michael Collison
Cc: gcc-patches@gcc.gnu.org; nd
Subject: Re: [PATCH][Aarch64] Relational compare zero not merged into subtract
On Thu, Jun 01, 2017 at 11:54:33PM +, Michael Collison wrote:
> This patch improves code generation for relational compares against
> zero that are not merged into a subtract instruction. This patch
> improves the >= and < cases.
>
> An example of the '<' case:
>
> int lt (int x, int y)
> {
> if ((x - y) < 0)
> return 10;
>
> return 0;
> }
>
> Trunk generates:
>
> lt:
> sub w1, w0, w1
> mov w0, 10
> cmp w1, 0
> cselw0, w0, wzr, lt
> ret
>
> With the patch we can eliminate the redundant subtract and now generate:
>
> lt:
> cmp w0, w1
> mov w0, 10
> cselw0, w0, wzr, mi
> ret
I'm not up to speed on the way we use CC register modes in the AArch64 Backend.
On the one hand looking at patterns like *sub3_compare0, this patch looks
correct. Those too generate subs instructions, and only set the CC_NZ CC
register. But on the other hand cmp sets the full CC register. As cmp is
an alias for subs, should they not set the same CC register mode? Certainly the
instruction sets more than just N and Z.
As I say, I don't understand this area, and I'm not sure if my objection is
reasonable. Hopefully someone who knows CC modes better could help me
understand why this is correct.
Thanks,
James
>
> Bootstrapped and tested on aarch64-linux-gnu. Okay for trunk?
>
> 2017-06-01 Michael Collison
>
> * config/aarch64/aarch64-simd.md(aarch64_sub_compare0):
> New pattern.
> * testsuite/gcc.target/aarch64/cmp-2.c: New testcase.
[PATCHv4][PR 57371] Remove useless floating point casts in comparisons
Hi all, This is an updated version of patch in https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00409.html . It prevents optimization in presense of sNaNs (and qNaNs when comparison operator is > >= < <=) to preserve FP exceptions. Note that I had to use -fsignaling-nans in pr57371-5.c test because by default this option is off and some existing patterns in match.pd happily optimize NaN comparisons, even with sNaNs (!). Bootstrapped and regtested on x64. Ok for trunk? -Y pr57371-4.patch Description: Binary data
Re: A potential bug in lra-constraints.c for special_memory_constraint?
> From the above doc, the major difference between a memory_constraint and a > special_memory_constraint is: whether "reload can or cannot make them match > by reloading the address". Right, i.e. by just changing the form of the address (instead of the address itself). > For memory_constraint, the reload is Okay, however, for > special_memory_constraint, the reload is NOT Okay. > > I am not sure whether the RELOAD includes Spill or not, if it is, then the > current handling of special_memory_constraint is NOT correct: > (lra-constraints.c) > > 2088 case CT_SPECIAL_MEMORY: > 2089 if (MEM_P (op) > 2090 && satisfies_memory_constraint_p (op, cn)) > 2091 win = true; > 2092 else if (spilled_pseudo_p (op)) > 2093 win = true; > 2094 break; > > line 2092-2093 permits the memory spill, which seems need to be avoided for > SPECIAL_MEMORY_Constraint. > > the thing I need to confirm is: > > whether “spill” is considered as RELOAD or NOT? Yes, spilling is part of reloading but is not reloading the address since it's changing the address, so I think it's OK. Why is that problematic for you? -- Eric Botcazou
Re: [PATCH-v4] [SPARC] Add a workaround for the LEON3FT store-store errata
On 2017-07-11 09:21, Eric Botcazou wrote: Applied without the undocumented tweaks to the divdf3_fix and sqrtdf2_fix patterns. Why are 2 nops necessary here? The stored value doesn't matter. And the length attribute should be adjusted if nops are added to the pattern. The first nop was added to prevent sequence A from appearing (store -> fdivd -> std). But as you say, it is not needed as we do not read the value written by the std. The second nop was added to prevent sequence B (std -> store) and this one seems necessary as the value written by the store might be used later. OK to submit a new patch with only the second nop and a correct length attribute? -- Daniel Cederman Cobham Gaisler
[ping #4][patch] Fix PR80929: Realistic PARALLEL cost in seq_cost.
Ping #4 This small addition improves costs of PARALLELs in rtlanal.c:seq_cost(). Up to now, these costs are assumed to be 1 which gives gross inexact costs for, e.g. divmod which is represented as PARALLEL. The patch just forwards cost computation to insn_rtx_cost which uses the cost of the 1st SET (if any) and otherwise assign costs of 1 insn. Bootstrapped & regtested on x86_64. Moreover, it fixed the division by constant on avr where the problem popped up since PR79665. Ok to install? Johann gcc/ PR middle-end/80929 * rtlanal.c (seq_cost) [PARALLEL]: Get cost from insn_rtx_cost instead of assuming cost of 1. Index: rtlanal.c === --- rtlanal.c (revision 248745) +++ rtlanal.c (working copy) @@ -5300,6 +5300,9 @@ seq_cost (const rtx_insn *seq, bool spee set = single_set (seq); if (set) cost += set_rtx_cost (set, speed); + else if (INSN_P (seq) + && PARALLEL == GET_CODE (PATTERN (seq))) + cost += 1 + insn_rtx_cost (PATTERN (seq), speed); else cost++; }
[PATCH] [SPARC] Avoid b2bst errata when using -mfix-ut699
The errata fix for the UT699 fdivd and fsqrtd might cause a sequence that can trigger the b2bst errata. Adding a NOP prevents this. gcc/ChangeLog: 2017-07-11 Daniel Cederman * config/sparc/sparc.md (divdf3_fix): Add NOP to prevent back to back store errata sensitive sequence from being generated. (sqrtdf2_fix): Likewise. --- gcc/config/sparc/sparc.md | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gcc/config/sparc/sparc.md b/gcc/config/sparc/sparc.md index afdc7d1..b154003 100644 --- a/gcc/config/sparc/sparc.md +++ b/gcc/config/sparc/sparc.md @@ -6171,10 +6171,10 @@ visl") (div:DF (match_operand:DF 1 "register_operand" "e") (match_operand:DF 2 "register_operand" "e")))] "TARGET_FPU && sparc_fix_ut699" - "fdivd\t%1, %2, %0\n\tstd\t%0, [%%sp-8]" + "fdivd\t%1, %2, %0\n\tstd\t%0, [%%sp-8]\n\tnop" [(set_attr "type" "fpdivd") (set_attr "fptype" "double") - (set_attr "length" "2")]) + (set_attr "length" "3")]) (define_insn "divsf3" [(set (match_operand:SF 0 "register_operand" "=f") @@ -6423,10 +6423,10 @@ visl") [(set (match_operand:DF 0 "register_operand" "=e") (sqrt:DF (match_operand:DF 1 "register_operand" "e")))] "TARGET_FPU && sparc_fix_ut699" - "fsqrtd\t%1, %0\n\tstd\t%0, [%%sp-8]" + "fsqrtd\t%1, %0\n\tstd\t%0, [%%sp-8]\n\tnop" [(set_attr "type" "fpsqrtd") (set_attr "fptype" "double") - (set_attr "length" "2")]) + (set_attr "length" "3")]) (define_insn "sqrtsf2" [(set (match_operand:SF 0 "register_operand" "=f") -- 2.9.3
Re: [PATCH][Aarch64] Relational compare zero not merged into subtract
On 02/06/17 00:54, Michael Collison wrote:
> This patch improves code generation for relational compares against zero that
> are not merged into a subtract instruction. This patch improves the >= and <
> cases.
>
> An example of the '<' case:
>
> int lt (int x, int y)
> {
> if ((x - y) < 0)
> return 10;
>
> return 0;
> }
>
> Trunk generates:
>
> lt:
> sub w1, w0, w1
> mov w0, 10
> cmp w1, 0
> cselw0, w0, wzr, lt
> ret
>
> With the patch we can eliminate the redundant subtract and now generate:
>
> lt:
> cmp w0, w1
> mov w0, 10
> cselw0, w0, wzr, mi
> ret
>
> Bootstrapped and tested on aarch64-linux-gnu. Okay for trunk?
>
> 2017-06-01 Michael Collison
>
> * config/aarch64/aarch64-simd.md(aarch64_sub_compare0):
> New pattern.
> * testsuite/gcc.target/aarch64/cmp-2.c: New testcase.
>
>
OK.
R.
> pr7261.patch.patch
>
>
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 51368e2..b90c728 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -2011,6 +2011,17 @@
>[(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
> )
>
> +(define_insn "aarch64_sub_compare0"
> + [(set (reg:CC_NZ CC_REGNUM)
> + (compare:CC_NZ
> + (minus:GPI (match_operand:GPI 0 "register_operand" "r")
> +(match_operand:GPI 1 "aarch64_plus_operand" "r"))
> + (const_int 0)))]
> + ""
> + "cmp\\t%0, %1"
> + [(set_attr "type" "alus_sreg")]
> +)
> +
> (define_insn "*compare_neg"
>[(set (reg:CC_Z CC_REGNUM)
> (compare:CC_Z
> diff --git a/gcc/testsuite/gcc.target/aarch64/cmp-2.c
> b/gcc/testsuite/gcc.target/aarch64/cmp-2.c
> new file mode 100644
> index 000..1201664
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/cmp-2.c
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +int lt (int x, int y)
> +{
> + if ((x - y) < 0)
> +return 10;
> +
> + return 0;
> +}
> +
> +int ge (int x, int y)
> +{
> + if ((x - y) >= 0)
> +return 10;
> +
> + return 0;
> +}
> +
> +/* { dg-final { scan-assembler-times "csel\t" 2 } } */
> +/* { dg-final { scan-assembler-not "sub\t" } } */
>
RE: [PATCH][Aarch64] Relational compare zero not merged into subtract
Michael Collison wrote: > > The subtract instruction only reliably sets the N and Z flags. We convey this > information in > aarch64_seelct_cc_mode. The SUBS and CMP set the N and Z flags identically - although they also set C and V, they are different if there is overflow. CC_NZmode is used after merging a compare with zero into an ALU instruction - generally N and Z are valid. This means that LT and GE condition codes must be translated into MI and PL (which happens in aarch64_get_condition_code_1). At a higher level like match.pd you could transform (x - y) < 0 into x < y if there is no signed overflow, but this isn't safe in RTL given source types are not available. Wilco > Trunk generates: > > lt: > sub w1, w0, w1 > mov w0, 10 > cmp w1, 0 > cselw0, w0, wzr, lt > ret > > With the patch we can eliminate the redundant subtract and now generate: > > lt: > cmp w0, w1 > mov w0, 10 > cselw0, w0, wzr, mi > ret
Re: [Libgomp, Fortran] Fix canadian cross build
On 3 July 2017 at 11:21, Yvan Roux wrote: > On 23 June 2017 at 15:44, Yvan Roux wrote: >> Hello, >> >> Fortran parts of libgomp (omp_lib.mod, openacc.mod, etc...) are >> missing in a canadian cross build, at least when target gfortran >> compiler comes from PATH and not from GFORTRAN_FOR_TARGET. >> >> Back in 2010, executability test of GFORTRAN was added to fix libgomp >> build on cygwin, but when the executable doesn't contain the path, >> "test -x" fails and part of the library are not built. >> >> This patch fixes the issue by using M4 macro AC_PATH_PROG (which >> returns the absolute name) instead of AC_CHECK_PROG in the function >> defined in config/acx.m4: NCN_STRICT_CHECK_TARGET_TOOLS. I renamed it >> into NCN_STRICT_PATH_TARGET_TOOLS to keep the semantic used in M4. >> >> Tested by building cross and candian cross toolchain (host: >> i686-w64-mingw32) for arm-linux-gnueabihf with issue and with a >> complete libgomp. >> >> ok for trunk ? > > ping? ping? > >> Thanks >> Yvan >> >> config/ChangeLog >> 2017-06-23 Yvan Roux >> >> * acx.m4 (NCN_STRICT_CHECK_TARGET_TOOLS): Renamed to ... >> (NCN_STRICT_PATH_TARGET_TOOLS): ... this. It reflects the >> replacement >> of AC_CHECK_PROG by AC_PATH_PROG to get the absolute name of the >> program. >> (ACX_CHECK_INSTALLED_TARGET_TOOL): Use renamed function. >> >> ChangeLog >> 2017-06-23 Yvan Roux >> >> * configure.ac: Use NCN_STRICT_PATH_TARGET_TOOLS instead of >> NCN_STRICT_CHECK_TARGET_TOOLS. >> * configure: Regenerate.
Re: [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol
On 3 July 2017 at 12:48, Yvan Roux wrote: > On 27 June 2017 at 13:14, Yvan Roux wrote: >> Hi Wilco >> >> On 27 June 2017 at 12:53, Wilco Dijkstra wrote: >>> Hi Yvan, >>> Here is the backport of Wilco's patch (r237607) along with Kyrill's one (r244643, which removed the remaining occurences of aarch64_nopcrelative_literal_loads). To fix the issue the original patch has to be modified, to keep aarch64_pcrelative_literal_loads test for large models in aarch64_classify_symbol. >>> >>> The patch looks good to me, however I can't approve it. >> >> ok thanks for the review. >> On trunk and gcc-7-branch the :lo12: relocations are not generated because of Wilco's fix for pr78733 (r243456 and 243486), but my understanding is that the bug is still present since compiling gcc.target/aarch64/pr78733.c with -mcmodel=large brings back the :lo12: relocations (I'll submit a patch to add the test back if my understanding is correct). >>> >>> You're right, eventhough -mpc-relative-literal-loads doesn't make much sense >>> in the large memory model, it seems best to keep the option orthogonal to >>> enable the workaround. I've prepared a patch to fix this on trunk/GCC7. >>> It also adds a test which we should add to your changes to GCC6 too. >> >> ok, I think it is what kugan's proposed earlier today in: >> >> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01967.html >> >> I agree that -mpc-relative-literal-loads and large memory model >> doesn't make much sense, now it is what is used in kernel build >> system, but if you handle that in a bigger fix already, that's awesome >> :) > > ping? > https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01708.html ping >> Thanks >> Yvan >> >>> Wilco
[patch,committed] Remove external links that texinfo would shred.
texinfo is shredding external links.
Applied the following patch to prevent uses from 404 not found.
Johann
gcc/
* doc/extend.texi (AVR Function Attributes): Remove weblink to
Binutils doc as TEXI will mess them up.
* doc/invoke.texi (AVR Options): Same here.
Index: doc/extend.texi
===
--- doc/extend.texi (revision 250123)
+++ doc/extend.texi (working copy)
@@ -3820,8 +3820,6 @@ depended upon to work reliably and are n
Do not use @code{__gcc_isr} pseudo instructions in a function with
the @code{interrupt} or @code{signal} attribute aka. interrupt
service routine (ISR).
-For details on @code{__gcc_isr}, see the GNU Binutils
-@w{@uref{https://sourceware.org/binutils/docs/as/AVR_002dDependent.html,AVR
assembler manual}}.
Use this attribute if the preamble of the ISR prologue should always read
@example
push __zero_reg__
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 250123)
+++ doc/invoke.texi (working copy)
@@ -15981,9 +15981,7 @@ subroutines. Code size is smaller.
@item -mgas-isr-prologues
@opindex mgas-isr-prologues
Interrupt service routines (ISRs) may use the @code{__gcc_isr} pseudo
-instruction supported by GNU Binutils, see the
-@w{@uref{https://sourceware.org/binutils/docs/as/AVR_002dDependent.html,AVR
assembler manual}}
-for details.
+instruction supported by GNU Binutils.
If this option is on, the feature can still be disabled for individual
ISRs by means of the @ref{AVR Function Attributes,,@code{no_gccisr}}
function attribute. This feature is activated per default
[PATCH] Initialize counters in autoFDO to zero, not to uninitialized.
Hello. This fixes majority of autoFDO test-cases. Patch can boostrap and survives regression tests. Ready for trunk? Thanks, Martin gcc/ChangeLog: 2017-07-11 Martin Liska * auto-profile.c (afdo_annotate_cfg): Assign zero counts to BBs and edges seen by autoFDO. --- gcc/auto-profile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/auto-profile.c b/gcc/auto-profile.c index 71c06f30449..334f38be109 100644 --- a/gcc/auto-profile.c +++ b/gcc/auto-profile.c @@ -1547,9 +1547,9 @@ afdo_annotate_cfg (const stmt_set &promoted_stmts) edge e; edge_iterator ei; -bb->count = profile_count::uninitialized (); +bb->count = profile_count::zero ().afdo (); FOR_EACH_EDGE (e, ei, bb->succs) - e->count = profile_count::uninitialized (); + e->count = profile_count::zero ().afdo (); if (afdo_set_bb_count (bb, promoted_stmts)) set_bb_annotated (bb, &annotated_bb);
[PATCH] Fix indirect call optimization done by autoFDO.
Hello.
Following is a typo fix which nobody has noticed during testing of
e.g. gcc/testsuite/gcc.dg/tree-prof/indir-call-prof.c.
Patch can bootstrap and survives regression tests.
Ready for trunk?
Thanks,
Martin
gcc/ChangeLog:
2017-07-11 Martin Liska
* auto-profile.c (autofdo_source_profile::update_inlined_ind_target):
Fix wrong condition.
---
gcc/auto-profile.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/gcc/auto-profile.c b/gcc/auto-profile.c
index 334f38be109..d8b0d04bf15 100644
--- a/gcc/auto-profile.c
+++ b/gcc/auto-profile.c
@@ -777,12 +777,12 @@ autofdo_source_profile::update_inlined_ind_target (gcall *stmt,
count of the unpromoted targets (stored in old_info). If it is no less
than half of the callsite count (stored in INFO), the original promoted
target is considered not hot any more. */
- if (total >= info->count / 2)
+ if (info->count < total / 2)
{
if (dump_file)
- fprintf (dump_file, " not hot anymore %ld >= %ld",
- (long)total,
- (long)info->count /2);
+ fprintf (dump_file, " not hot anymore %ld < %ld",
+ (long)info->count,
+ (long)total /2);
return false;
}
[PATCH] Improvements to the libstdc++ FAQ and manual
* doc/xml/faq.xml: Update several old entries. Improve cross-references. * doc/xml/manual/intro.xml: Add anchors to each DR. * doc/html/*: Regenerate. Committed to trunk. commit c59da1a826c9fa0ffa4cd5b89db88581d481489e Author: Jonathan Wakely Date: Tue Jul 11 12:09:44 2017 +0100 Improvements to the libstdc++ FAQ and manual * doc/xml/faq.xml: Update several old entries. Improve cross-references. * doc/xml/manual/intro.xml: Add anchors to each DR. * doc/html/*: Regenerate. diff --git a/libstdc++-v3/doc/xml/faq.xml b/libstdc++-v3/doc/xml/faq.xml index 8041c14..703ade5 100644 --- a/libstdc++-v3/doc/xml/faq.xml +++ b/libstdc++-v3/doc/xml/faq.xml @@ -66,10 +66,10 @@ that are the hallmarks of an open-source project are applied to libstdc++. -All of the standard classes and functions from C++98/C++03 +All of the standard classes and functions from C++98/C++03, C++11 and C++14 (such as string, vector<>, iostreams, algorithms etc.) -are freely available and atempt to be fully compliant. +are freely available and attempt to be fully compliant. Work is ongoing to complete support for the current revision of the ISO C++ Standard. @@ -539,6 +539,9 @@ + + This answer is old and probably no longer be relevant. + By default we try to support the C99 long long type. This requires that certain functions from your C library be present. @@ -692,7 +695,7 @@ - Can't use wchar_t/wstring on FreeBSD + Can't use wchar_t/wstring on FreeBSD @@ -764,7 +767,8 @@ published on http://www.w3.org/1999/xlink"; xlink:href="http://www.open-std.org/jtc1/sc22/wg21/";>the WG21 website. -Many of these issues have resulted in code changes in libstdc++. +Many of these issues have resulted in +code changes in libstdc++. If you think you've discovered a new bug that is not listed, @@ -794,8 +798,8 @@ Before reporting a bug, please examine the -http://www.w3.org/1999/xlink"; xlink:href="http://gcc.gnu.org/bugs/";>bugs database with the -category set to g++. +http://www.w3.org/1999/xlink"; xlink:href="https://gcc.gnu.org/bugs/";>bugs database, with the +component set to c++. @@ -813,8 +817,12 @@ + + This answer is old and probably no longer be relevant. + -One of the most-reported non-bug reports. Executing a sequence like: +Prior to GCC 4.0 this was one of the most-reported non-bug reports. +Executing a sequence like this would fail: @@ -829,19 +837,20 @@ -All operations on the re-opened fs will fail, or at -least act very strangely. Yes, they often will, especially if -fs reached the EOF state on the previous file. The -reason is that the state flags are not cleared -on a successful call to open(). The standard unfortunately did -not specify behavior in this case, and to everybody's great sorrow, -the proposed LWG resolution in - DR #22 is to leave the flags unchanged. You must insert a call -to fs.clear() between the calls to close() and open(), -and then everything will work like we all expect it to work. -Update: for GCC 4.0 we implemented the resolution -of DR #409 and open() -now calls clear() on success! +All operations on the re-opened fs would fail, or at +least act very strangely, especially if fs reached the +EOF state on the previous file. +The original C++98 standard did not specify behavior in this case, and +the resolution of DR #22 was to +leave the state flags unchanged on a successful call to +open(). +You had to insert a call to fs.clear() between the +calls to close() and open(), +and then everything will work as expected. +Update: For GCC 4.0 we implemented the resolution +of DR #409 and +open() +now calls clear() on success. @@ -858,7 +867,9 @@ libstdc++ -Weffc++-clean is not a goal of the project, for a few reasons. Mainly, that option tries to enforce object-oriented programming, while the Standard Library isn't -necessarily trying to be OO. +necessarily trying to be OO. The option also enforces outdated guidelines +from old editions of the books, and the advice isn't all relevant to +modern C++ (especially C++11 and later). We do, however, try to have libstdc++ sources as clean as possible. If @@ -879,9 +890,10 @@ Another problem is the rel_ops namespace and the template comparison operator functions contained therein. If they become visible in the same namespace as other comparison functions -(e.g., using them and theheader), +(e.g., using them and the + header), then you will suddenly be faced with huge number
Re: [PATCH] PR libstdc++/80316 make promise::set_value throw no_state error
On 21/04/17 15:54 +0100, Jonathan Wakely wrote: On 4 April 2017 at 20:44, Jonathan Wakely wrote: We got a bug report from a customer pointing out that calling promise::set_value on a moved-from promise crashes instead of throwing an exception with error code future_errc::no_state. This fixes it, by moving the _S_check calls to *before* we deference the pointer that the calls check! This passes all tests, including the more comprehensive ones I've added as part of this commit, but I think it can wait for stage 1 anyway. We've been shipping this bug for a couple of releases already. PR libstdc++/80316 * include/std/future (_State_baseV2::_Setter::operator()): Remove _S_check calls that are done after the pointer to the shared state is already dereferenced. (_State_baseV2::_Setter<_Res, void>): Define specialization for void as partial specialization so it can be defined within the definition of _State_baseV2. (_State_baseV2::__setter): Call _S_check. (_State_baseV2::__setter(promise*)): Add overload for use by promise::set_value and promise::set_value_at_thread_exit. (promise, promise, promise): Make _State a friend. (_State_baseV2::_Setter): Remove explicit specialization. (promise::set_value, promise::set_value_at_thread_exit): Use new __setter overload. * testsuite/30_threads/promise/members/at_thread_exit2.cc: New test. * testsuite/30_threads/promise/members/set_exception.cc: Test promise and promise specializations. * testsuite/30_threads/promise/members/set_exception2.cc: Likewise. Test for no_state error condition. * testsuite/30_threads/promise/members/set_value2.cc: Likewise. This is now committed to trunk. And now also to gcc-7-branch.
Re: Add support to trace comparison instructions and switch statements
Hi
I wrote a test for "-fsanitize-coverage=trace-cmp" .
Is there anybody tells me if these codes could be merged into gcc ?
Index: gcc/testsuite/gcc.dg/sancov/basic3.c
===
--- gcc/testsuite/gcc.dg/sancov/basic3.c (nonexistent)
+++ gcc/testsuite/gcc.dg/sancov/basic3.c (working copy)
@@ -0,0 +1,42 @@
+/* Basic test on number of inserted callbacks. */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize-coverage=trace-cmp -fdump-tree-optimized" } */
+
+void foo(char *a, short *b, int *c, long long *d, float *e, double *f)
+{
+ if (*a)
+*a += 1;
+ if (*b)
+*b = *a;
+ if (*c)
+*c += 1;
+ if(*d)
+*d = *c;
+ if(*e == *c)
+*e = *c;
+ if(*f == *e)
+*f = *e;
+ switch(*a)
+{
+case 2:
+ *b += 2;
+ break;
+default:
+ break;
+}
+ switch(*d)
+{
+case 3:
+ *d += 3;
+case -4:
+ *d -= 4;
+}
+}
+
+/* { dg-final { scan-tree-dump-times
"__builtin___sanitizer_cov_trace_cmp1 \\(" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times
"__builtin___sanitizer_cov_trace_cmp2 \\(" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times
"__builtin___sanitizer_cov_trace_cmp4 \\(" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times
"__builtin___sanitizer_cov_trace_cmp8 \\(" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times
"__builtin___sanitizer_cov_trace_cmpf \\(" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times
"__builtin___sanitizer_cov_trace_cmpd \\(" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times
"__builtin___sanitizer_cov_trace_switch \\(" 2 "optimized" } } */
With Regards
Wish Wu
On Mon, Jul 10, 2017 at 8:07 PM, 吴潍浠(此彼) wrote:
> Hi
>
> I write some codes to make gcc support comparison-guided fuzzing.
> It is very like
> http://clang.llvm.org/docs/SanitizerCoverage.html#tracing-data-flow .
> With -fsanitize-coverage=trace-cmp the compiler will insert extra
> instrumentation around comparison instructions and switch statements.
> I think it is useful for fuzzing. :D
>
> Patch is below, I may supply test cases later.
>
> With Regards
> Wish Wu
>
> Index: gcc/asan.c
> ===
> --- gcc/asan.c (revision 250082)
> +++ gcc/asan.c (working copy)
> @@ -2705,6 +2705,29 @@ initialize_sanitizer_builtins (void)
>tree BT_FN_SIZE_CONST_PTR_INT
> = build_function_type_list (size_type_node, const_ptr_type_node,
> integer_type_node, NULL_TREE);
> +
> + tree BT_FN_VOID_UINT8_UINT8
> += build_function_type_list (void_type_node, unsigned_char_type_node,
> + unsigned_char_type_node, NULL_TREE);
> + tree BT_FN_VOID_UINT16_UINT16
> += build_function_type_list (void_type_node, uint16_type_node,
> + uint16_type_node, NULL_TREE);
> + tree BT_FN_VOID_UINT32_UINT32
> += build_function_type_list (void_type_node, uint32_type_node,
> + uint32_type_node, NULL_TREE);
> + tree BT_FN_VOID_UINT64_UINT64
> += build_function_type_list (void_type_node, uint64_type_node,
> + uint64_type_node, NULL_TREE);
> + tree BT_FN_VOID_FLOAT_FLOAT
> += build_function_type_list (void_type_node, float_type_node,
> + float_type_node, NULL_TREE);
> + tree BT_FN_VOID_DOUBLE_DOUBLE
> += build_function_type_list (void_type_node, double_type_node,
> + double_type_node, NULL_TREE);
> + tree BT_FN_VOID_UINT64_PTR
> += build_function_type_list (void_type_node, uint64_type_node,
> + ptr_type_node, NULL_TREE);
> +
>tree BT_FN_BOOL_VPTR_PTR_IX_INT_INT[5];
>tree BT_FN_IX_CONST_VPTR_INT[5];
>tree BT_FN_IX_VPTR_IX_INT[5];
> Index: gcc/builtin-types.def
> ===
> --- gcc/builtin-types.def (revision 250082)
> +++ gcc/builtin-types.def (working copy)
> @@ -338,8 +338,20 @@ DEF_FUNCTION_TYPE_2 (BT_FN_VOID_PTRMODE_PTR,
> BT_VOID, BT_PTRMODE, BT_PTR)
> DEF_FUNCTION_TYPE_2 (BT_FN_VOID_PTR_PTRMODE,
> BT_VOID, BT_PTR, BT_PTRMODE)
> +DEF_FUNCTION_TYPE_2 (BT_FN_VOID_UINT8_UINT8,
> +BT_VOID, BT_UINT8, BT_UINT8)
> +DEF_FUNCTION_TYPE_2 (BT_FN_VOID_UINT16_UINT16,
> +BT_VOID, BT_UINT16, BT_UINT16)
> +DEF_FUNCTION_TYPE_2 (BT_FN_VOID_UINT32_UINT32,
> +BT_VOID, BT_UINT32, BT_UINT32)
> DEF_FUNCTION_TYPE_2 (BT_FN_VOID_UINT64_UINT64,
> BT_VOID, BT_UINT64, BT_UINT64)
> +DEF_FUNCTION_TYPE_2 (BT_FN_VOID_FLOAT_FLOAT,
> +BT_VOID, BT_FLOAT, BT_FLOAT)
> +DEF_FUNCTION_TYPE_2 (BT_FN_VOID_DOUBLE_DOUBLE,
> +BT_VOID, BT_DOUBLE, BT_DOUBLE)
> +DEF_FUNCTION_TYPE_2 (BT_FN_VOID_UINT64_PTR,
> +BT_VOID, BT_UINT64, BT_PTR)
>
[PATCH] remove useless check
In killing TYPE_METHODS I discovered this useless check. We already cull these ctors from the methods just after creating the struct. bootstrap with the continue turned into gcc_unreachable worked just fine. applied to trunk. nathan -- Nathan Sidwell 2017-07-11 Nathan Sidwell * dwarf2out.c (gen_member_die): Remove useless check for anon ctors. Index: dwarf2out.c === --- dwarf2out.c (revision 250090) +++ dwarf2out.c (working copy) @@ -24207,10 +24207,6 @@ gen_member_die (tree type, dw_die_ref co /* Don't include clones in the member list. */ if (DECL_ABSTRACT_ORIGIN (member)) continue; - /* Nor constructors for anonymous classes. */ - if (DECL_ARTIFICIAL (member) - && dwarf2_name (member, 0) == NULL) - continue; child = lookup_decl_die (member); if (child)
Re: [1/2] PR 78736: New warning -Wenum-conversion
On 13 June 2017 at 01:47, Joseph Myers wrote:
> This is OK with one fix:
>
>> +C ObjC Var(warn_enum_conversion) Init(0) Warning LangEnabledBy(C Objc,Wall)
>
> I believe the LangEnabledBy arguments are case-sensitive, so you need to
> have ObjC not Objc there for it to work correctly. (*.opt parsing isn't
> very good at detecting typos and giving errors rather than silently
> ignoring things.)
Hi,
Sorry for the late response, I was on a vacation.
The attached patch is rebased and bootstrap+tested on x86_64-unknown-linux-gnu.
I have modified it slightly to not warn for enums with different names
but having same value ranges.
For eg:
enum e1 { e1_1, e1_2 };
enum e2 { e2_1, e2_2 };
enum e1 x = e2_1;
With this version, there would be no warning for the above assignment
since both e1 and e2 have
same value ranges. Is that OK ?
The patch has following fallouts in the testsuite:
a) libgomp:
I initially assume it was a false positive because I thought enum
gomp_schedule_type
and enum omp_sched_t have same value-ranges but it looks like omp_sched_t
has range [1, 4] while gomp_schedule_type has range [0, 4] with one
extra element.
Is the warning then correct for this case ?
b) libgfortran:
i) Implicit conversion from unit_mode to file_mode
ii) Implicit conversion from unit_sign_s to unit_sign.
I suppose the warning is OK for these cases since unit_mode, file_mode
have different value-ranges and similarly for unit_sign_s, unit_sign ?
Also I tested the warning by compiling the kernel for x86_64 with
allmodconifg (attached), and there
have been quite few instances of the warning (attached). I have been
through few cases which I don't think are false positives
but I wonder then whether we should relegate the warning to Wextra instead ?
Thanks,
Prathamesh
>
> --
> Joseph S. Myers
> jos...@codesourcery.com
mm/page-writeback.c:2436:3: warning: implicit conversion from ‘enum
node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion]
mm/page-writeback.c:2458:3: warning: implicit conversion from ‘enum
node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion]
mm/page-writeback.c:2715:4: warning: implicit conversion from ‘enum
node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion]
mm/page-writeback.c:2762:3: warning: implicit conversion from ‘enum
node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion]
mm/page-writeback.c:2817:3: warning: implicit conversion from ‘enum
node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion]
mm/vmscan.c:2058:14: warning: implicit conversion from ‘enum node_stat_item’ to
‘enum memcg_stat_item’ [-Wenum-conversion]
mm/vmscan.c:2745:15: warning: implicit conversion from ‘enum node_stat_item’ to
‘enum memcg_stat_item’ [-Wenum-conversion]
mm/workingset.c:292:2: warning: implicit conversion from ‘enum node_stat_item’
to ‘enum memcg_stat_item’ [-Wenum-conversion]
mm/workingset.c:296:3: warning: implicit conversion from ‘enum node_stat_item’
to ‘enum memcg_stat_item’ [-Wenum-conversion]
mm/workingset.c:478:2: warning: implicit conversion from ‘enum node_stat_item’
to ‘enum memcg_stat_item’ [-Wenum-conversion]
mm/rmap.c:1161:2: warning: implicit conversion from ‘enum node_stat_item’ to
‘enum memcg_stat_item’ [-Wenum-conversion]
mm/rmap.c:1201:2: warning: implicit conversion from ‘enum node_stat_item’ to
‘enum memcg_stat_item’ [-Wenum-conversion]
mm/memcontrol.c:3653:12: warning: implicit conversion from ‘enum
node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion]
mm/memcontrol.c:3656:16: warning: implicit conversion from ‘enum
node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion]
drivers/acpi/dock.c:249:3: warning: implicit conversion from ‘enum ’
to ‘enum dock_callback_type’ [-Wenum-conversion]
mm/zsmalloc.c:756:2: warning: implicit conversion from ‘enum fullness_group’ to
‘enum zs_stat_type’ [-Wenum-conversion]
mm/zsmalloc.c:784:2: warning: implicit conversion from ‘enum fullness_group’ to
‘enum zs_stat_type’ [-Wenum-conversion]
arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum
kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion]
arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum
kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion]
arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum
kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion]
arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum
kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion]
arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum
kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion]
arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum
kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion]
arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum
kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion]
arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum
kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion]
arch/x86/kvm/
[C++ PATCH] ctor name should not change
We no longer need to frob the ctor name here, and indeed the name lookup stuff
I'm working on would really rather it didn't.
Applied to trunk
nathan
--
Nathan Sidwell
2017-07-11 Nathan Sidwell
* decl2.c (reset_type_linkage_2): Dont't change ctor name.
Index: decl2.c
===
--- decl2.c (revision 250090)
+++ decl2.c (working copy)
@@ -2622,13 +2622,6 @@ reset_type_linkage_2 (tree type)
{
tree mem = STRIP_TEMPLATE (m);
reset_decl_linkage (mem);
- if (DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (mem))
- {
- /* Also update its name, for cxx_dwarf_name. */
- DECL_NAME (mem) = TYPE_IDENTIFIER (type);
- if (m != mem)
- DECL_NAME (m) = TYPE_IDENTIFIER (type);
- }
}
binding_table_foreach (CLASSTYPE_NESTED_UTDS (type),
bt_reset_linkage_2, NULL);
[nvptx, committed] Add extra initialization of broadcasted condition variables
Hi,
we've run into a PTX JIT bug with cuda driver version 381.22 for sm_61
at -O1 and higher. This patch adds a workaround, guarded by a macro,
enabling the workaround by default.
Tested on x86_64 with nvidia accelerator.
Committed.
Thanks,
- Tom
Add extra initialization of broadcasted condition variables
2017-07-11 Tom de Vries
* config/nvptx/nvptx.c (WORKAROUND_PTXJIT_BUG): New macro.
(bb_first_real_insn): New function.
(nvptx_single): Add extra initialization of broadcasted condition
variables.
---
gcc/config/nvptx/nvptx.c | 53
1 file changed, 53 insertions(+)
diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index daeec27..c8847a5 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -74,6 +74,8 @@
/* This file should be included last. */
#include "target-def.h"
+#define WORKAROUND_PTXJIT_BUG 1
+
/* The various PTX memory areas an object might reside in. */
enum nvptx_data_area
{
@@ -3844,6 +3846,24 @@ nvptx_wsync (bool after)
return gen_nvptx_barsync (GEN_INT (after));
}
+#if WORKAROUND_PTXJIT_BUG
+/* Return first real insn in BB, or return NULL_RTX if BB does not contain
+ real insns. */
+
+static rtx_insn *
+bb_first_real_insn (basic_block bb)
+{
+ rtx_insn *insn;
+
+ /* Find first insn of from block. */
+ FOR_BB_INSNS (bb, insn)
+if (INSN_P (insn))
+ return insn;
+
+ return 0;
+}
+#endif
+
/* Single neutering according to MASK. FROM is the incoming block and
TO is the outgoing block. These may be the same block. Insert at
start of FROM:
@@ -3958,6 +3978,39 @@ nvptx_single (unsigned mask, basic_block from, basic_block to)
if (GOMP_DIM_MASK (GOMP_DIM_VECTOR) == mask)
{
/* Vector mode only, do a shuffle. */
+#if WORKAROUND_PTXJIT_BUG
+ /* The branch condition %rcond is propagated like this:
+
+ {
+ .reg .u32 %x;
+ mov.u32 %x,%tid.x;
+ setp.ne.u32 %rnotvzero,%x,0;
+ }
+
+ @%rnotvzero bra Lskip;
+ setp.. %rcond,op1,op2;
+ Lskip:
+ selp.u32 %rcondu32,1,0,%rcond;
+ shfl.idx.b32 %rcondu32,%rcondu32,0,31;
+ setp.ne.u32 %rcond,%rcondu32,0;
+
+ There seems to be a bug in the ptx JIT compiler (observed at driver
+ version 381.22, at -O1 and higher for sm_61), that drops the shfl
+ unless %rcond is initialized to something before 'bra Lskip'. The
+ bug is not observed with ptxas from cuda 8.0.61.
+
+ It is true that the code is non-trivial: at Lskip, %rcond is
+ uninitialized in threads 1-31, and after the selp the same holds
+ for %rcondu32. But shfl propagates the defined value in thread 0
+ to threads 1-31, so after the shfl %rcondu32 is defined in threads
+ 0-31, and after the setp.ne %rcond is defined in threads 0-31.
+
+ There is nothing in the PTX spec to suggest that this is wrong, or
+ to explain why the extra initialization is needed. So, we classify
+ it as a JIT bug, and the extra initialization as workaround. */
+ emit_insn_before (gen_movbi (pvar, const0_rtx),
+ bb_first_real_insn (from));
+#endif
emit_insn_before (nvptx_gen_vcast (pvar), tail);
}
else
Re: [PATCH] PR libstdc++/80316 make promise::set_value throw no_state error
On 11/07/17 12:53 +0100, Jonathan Wakely wrote: On 21/04/17 15:54 +0100, Jonathan Wakely wrote: On 4 April 2017 at 20:44, Jonathan Wakely wrote: We got a bug report from a customer pointing out that calling promise::set_value on a moved-from promise crashes instead of throwing an exception with error code future_errc::no_state. This fixes it, by moving the _S_check calls to *before* we deference the pointer that the calls check! This passes all tests, including the more comprehensive ones I've added as part of this commit, but I think it can wait for stage 1 anyway. We've been shipping this bug for a couple of releases already. PR libstdc++/80316 * include/std/future (_State_baseV2::_Setter::operator()): Remove _S_check calls that are done after the pointer to the shared state is already dereferenced. (_State_baseV2::_Setter<_Res, void>): Define specialization for void as partial specialization so it can be defined within the definition of _State_baseV2. (_State_baseV2::__setter): Call _S_check. (_State_baseV2::__setter(promise*)): Add overload for use by promise::set_value and promise::set_value_at_thread_exit. (promise, promise, promise): Make _State a friend. (_State_baseV2::_Setter): Remove explicit specialization. (promise::set_value, promise::set_value_at_thread_exit): Use new __setter overload. * testsuite/30_threads/promise/members/at_thread_exit2.cc: New test. * testsuite/30_threads/promise/members/set_exception.cc: Test promise and promise specializations. * testsuite/30_threads/promise/members/set_exception2.cc: Likewise. Test for no_state error condition. * testsuite/30_threads/promise/members/set_value2.cc: Likewise. This is now committed to trunk. And now also to gcc-7-branch. And gcc-6-branch.
C PATCH to fix bogus warning with -Wmultistatement-macros (PR c/81364)
This patch fixes a bogus -Wmultistatement-macros warning. The code didn't
notice that what came after a guard such as else was actually wrapped in { }
which is a correct use. This bogus warning only triggered when the body of
a conditional was coming from a different expansion than the conditional
itself.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2017-07-11 Marek Polacek
PR c/81364
* c-parser.c (c_parser_else_body): Don't warn about multistatement
macro expansion if the body is in { }.
(c_parser_while_statement): Likewise.
(c_parser_for_statement): Likewise.
* Wmultistatement-macros-12.c: New test.
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index f8fbc92..7524a73 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -5557,7 +5557,8 @@ c_parser_else_body (c_parser *parser, const
token_indent_info &else_tinfo,
}
else
{
- body_loc_after_labels = c_parser_peek_token (parser)->location;
+ if (!c_parser_next_token_is (parser, CPP_OPEN_BRACE))
+ body_loc_after_labels = c_parser_peek_token (parser)->location;
c_parser_statement_after_labels (parser, NULL, chain);
}
@@ -5811,6 +5812,7 @@ c_parser_while_statement (c_parser *parser, bool ivdep,
bool *if_p)
= get_token_indent_info (c_parser_peek_token (parser));
location_t loc_after_labels;
+ bool open_brace = c_parser_next_token_is (parser, CPP_OPEN_BRACE);
body = c_parser_c99_block_statement (parser, if_p, &loc_after_labels);
c_finish_loop (loc, cond, NULL, body, c_break_label, c_cont_label, true);
add_stmt (c_end_compound_stmt (loc, block, flag_isoc99));
@@ -5820,7 +5822,7 @@ c_parser_while_statement (c_parser *parser, bool ivdep,
bool *if_p)
= get_token_indent_info (c_parser_peek_token (parser));
warn_for_misleading_indentation (while_tinfo, body_tinfo, next_tinfo);
- if (next_tinfo.type != CPP_SEMICOLON)
+ if (next_tinfo.type != CPP_SEMICOLON && !open_brace)
warn_for_multistatement_macros (loc_after_labels, next_tinfo.location,
while_tinfo.location, RID_WHILE);
@@ -6109,6 +6111,7 @@ c_parser_for_statement (c_parser *parser, bool ivdep,
bool *if_p)
= get_token_indent_info (c_parser_peek_token (parser));
location_t loc_after_labels;
+ bool open_brace = c_parser_next_token_is (parser, CPP_OPEN_BRACE);
body = c_parser_c99_block_statement (parser, if_p, &loc_after_labels);
if (is_foreach_statement)
@@ -6122,7 +6125,7 @@ c_parser_for_statement (c_parser *parser, bool ivdep,
bool *if_p)
= get_token_indent_info (c_parser_peek_token (parser));
warn_for_misleading_indentation (for_tinfo, body_tinfo, next_tinfo);
- if (next_tinfo.type != CPP_SEMICOLON)
+ if (next_tinfo.type != CPP_SEMICOLON && !open_brace)
warn_for_multistatement_macros (loc_after_labels, next_tinfo.location,
for_tinfo.location, RID_FOR);
diff --git gcc/testsuite/c-c++-common/Wmultistatement-macros-12.c
gcc/testsuite/c-c++-common/Wmultistatement-macros-12.c
index e69de29..ac8915c 100644
--- gcc/testsuite/c-c++-common/Wmultistatement-macros-12.c
+++ gcc/testsuite/c-c++-common/Wmultistatement-macros-12.c
@@ -0,0 +1,43 @@
+/* PR c/81364 */
+/* { dg-do compile } */
+/* { dg-options "-Wmultistatement-macros" } */
+
+#define FOO0 if (1) { } else
+#define TST0 \
+void bar0 (void) \
+{ \
+ FOO0 { } /* { dg-bogus "macro expands to multiple statements" } */ \
+}
+TST0
+
+#define FOO1 for (;;)
+#define TST1 \
+void bar1 (void) \
+{ \
+ FOO1 { } /* { dg-bogus "macro expands to multiple statements" } */ \
+}
+TST1
+
+#define FOO2 while (1)
+#define TST2 \
+void bar2 (void) \
+{ \
+ FOO2 { } /* { dg-bogus "macro expands to multiple statements" } */ \
+}
+TST2
+
+#define FOO3 switch (1)
+#define TST3 \
+void bar3 (void) \
+{ \
+ FOO3 { } /* { dg-bogus "macro expands to multiple statements" } */ \
+}
+TST3
+
+#define FOO4 if (1)
+#define TST4 \
+void bar4 (void) \
+{ \
+ FOO4 { } /* { dg-bogus "macro expands to multiple statements" } */ \
+}
+TST4
Marek
Re: [PATCH] document IntegerRange in internals manual
On 07/10/2017 05:08 PM, Martin Sebor wrote:
On 07/10/2017 02:35 AM, Martin Liška wrote:
On 07/07/2017 09:20 PM, Martin Sebor wrote:
A conflict in my patch for bug 81345 made me notice that r249734
recently added a new option property, IntegerRange. The change
below adds brief documentation of the property to the manual.
Martin, can you please check to make sure I didn't miss anything?
Btw., while experimenting with the property I noticed that there
is no error when option that specifies IntegerRange is set in
the .opt file to a value outside that range. Would it be hard
to add some checks the the awk scripts to validate that the
argument values are in the range? It might help avoid bugs
similar to 81345).
Sure, please take a look at attached patch. Can you please test it?
The detection works fine for the Init problem (thanks!) but it
doesn't catch the out-of-range initializer in LangEnabledBy(C,
Wall, 2, 0) or in Alias(Wfoobar=, 1, 0). I don't know enough
about the option scripts yet to gauge how difficult handling
these might be. Do you have any idea? If you think it's
doable but outside the scope of this tweak let me know and I'll
open a bug for it to help us remember to handle it at some point
too.
Well, it's definitely doable, but doing that in the current awk script is quite
cumbersome.
I would prefer to have the option generation rewritten e.g. in Python where
current awk
script is not nice to reuse an already parsed information. More class-oriented
approach would
be desired here.
Please create a PR, I maybe rewrite it in future if we can benefit from that.
Are you interested in the current patch that handles 'Init' directive to go in?
Martin
By the way of an example, the following invalid specification
is accepted but then causes errors when GCC runs.
Wfoobar
C ObjC C++ ObjC++ Warning Alias(Wfoobar=, 1, 0)
Wfoobar=
C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_foobar) Warning
LangEnabledBy(C ObjC C++ ObjC++, Wall, 2, 0) Init (7) IntegerRange(3, 5)
Here one needs to have 'Init (7)' without space!
Ugh. I only recently realized this but keep forgetting. It
seems like another unnecessary trap that would be nice to fix
at some point.
Thanks
Martin
Martin
diff --git a/gcc/doc/options.texi b/gcc/doc/options.texi
index 3b68aab..af56e9f 100644
--- a/gcc/doc/options.texi
+++ b/gcc/doc/options.texi
@@ -264,6 +264,12 @@ option handler. @code{UInteger} should also be used on
options like
@code{-falign-loops}=@var{n} are supported to make sure the saved
options are given a full integer.
+@item IntegerRange(@var{min}, @var{max})
+The option's integer argument is expected to be in the range specified
+by @var{min} and @var{max}, inclusive. The option parser will check
+and reject option arguments that are outside the range before passing
+it to the relevant option handler.
LGTM, thanks for the documentation entry.
Martin
+
@item ToLower
The option's argument should be converted to lowercase as part of
putting it in canonical form, and before comparing with the strings
Re: [PATCH] Add quotes to error messages related to Sanitizers.
On 07/10/2017 09:35 PM, Martin Sebor wrote: On 07/10/2017 03:36 AM, Martin Liška wrote: Hi. This adds missing quotes to various error messages related to AddressSanitizer. Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. Ready to be installed? Martin gcc/ChangeLog: 2017-07-04 Martin Liska * opts.c (finish_options): Add quotes to error messages. (parse_sanitizer_options): Likewise. It would make a nice enhancement to -Wformat to have it detect the missing quotes around the command line options. It seems that it should be doable without excessive overhead simply by scanning the format string for the " -[fmW][a-z]" pattern (or something like it). An only slightly more involved/expensive solution would also compare the rest of the option name against known options and detect spelling errors in the option names. Yep, would be nice, however I'm not sure it worth doing that. We do not add new error messages so often. Martin This is not an objection to your patch which is a valuable improvement independent of the enhancement above. Martin
Re: [PATCH v2][RFC] Canonize names of attributes.
On 07/03/2017 11:00 PM, Jason Merrill wrote: On Mon, Jul 3, 2017 at 5:52 AM, Martin Liška wrote: On 06/30/2017 09:34 PM, Jason Merrill wrote: On Fri, Jun 30, 2017 at 5:23 AM, Martin Liška wrote: This is v2 of the patch, where just names of attributes are canonicalized. Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. What is the purpose of the new "strict" parameter to cmp_attribs* ? I don't see any discussion of it. It's needed for arguments of attribute names, like: /usr/include/stdio.h:391:62: internal compiler error: in cmp_attribs, at tree.h:5523 __THROWNL __attribute__ ((__format__ (__printf__, 3, 4))); Mm. Although we don't want to automatically canonicalize all identifier arguments to attributes in the parser, we could still do it for specific attributes, e.g. in handle_format_attribute or handle_mode_attribute. Yep, that was done in my previous version of the patch (https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00996.html). Where only attribute that was preserved unchanged was 'cleanup': diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 8f638785e0e..08b4db5e5bd 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -24765,7 +24765,8 @@ cp_parser_gnu_attribute_list (cp_parser* parser) tree tv; if (arguments != NULL_TREE && ((tv = TREE_VALUE (arguments)) != NULL_TREE) - && TREE_CODE (tv) == IDENTIFIER_NODE) + && TREE_CODE (tv) == IDENTIFIER_NODE + && !id_equal (TREE_PURPOSE (attribute), "cleanup")) TREE_VALUE (arguments) = canonize_attr_name (tv); release_tree_vector (vec); } Does it work for you to do it so? Martin Jason
Re: A potential bug in lra-constraints.c for special_memory_constraint?
thanks for the replying.
> On Jul 11, 2017, at 2:44 AM, Eric Botcazou wrote:
>
>> From the above doc, the major difference between a memory_constraint and a
>> special_memory_constraint is: whether "reload can or cannot make them match
>> by reloading the address".
>
> Right, i.e. by just changing the form of the address (instead of the address
> itself).
>
>> For memory_constraint, the reload is Okay, however, for
>> special_memory_constraint, the reload is NOT Okay.
>>
>> I am not sure whether the RELOAD includes Spill or not, if it is, then the
>> current handling of special_memory_constraint is NOT correct:
>> (lra-constraints.c)
>>
>> 2088 case CT_SPECIAL_MEMORY:
>> 2089 if (MEM_P (op)
>> 2090 && satisfies_memory_constraint_p (op, cn))
>> 2091 win = true;
>> 2092 else if (spilled_pseudo_p (op))
>> 2093 win = true;
>> 2094 break;
>>
>> line 2092-2093 permits the memory spill, which seems need to be avoided for
>> SPECIAL_MEMORY_Constraint.
>>
>> the thing I need to confirm is:
>>
>> whether “spill” is considered as RELOAD or NOT?
>
> Yes, spilling is part of reloading but is not reloading the address since
> it's
> changing the address, so I think it's OK. Why is that problematic for you?
the problem I had is:
1. we added a new special_memory_constraint for misaligned memory access, one
important requirement
for this new special_memory_constraint is, the address of the memory access is
misaligned.
2. per the current code in lra-constraints.c:
2286 case CT_SPECIAL_MEMORY:
2287 if (MEM_P (op)
2288 && satisfies_memory_constraint_p (op, cn))
2289 win = true;
2290 else if (spilled_pseudo_p (op))
2291 win = true;
2292 break;
if the op is a pseudo_p that can be spilled, then it's treated as a
PERFECT MATCH.
the issue only can be exposed by the following kind of RTL:
(insn 34 13 14 2 (set (reg:DI 122)
(reg:DI 129)) misalign-3.c:12 125 {*movdi_insn_sp64}
(nil))
i.e.
(1). REG2 move to REG1
and. (2). REG2 is a virtual reg (> the max hard regno, on Sparc, its 103),
therefore, must be spilled to stack.
the current interpretation of special memory treat such REG2 as a perfect
match to special memory, and then spill it.
however, such spilled memory RTL is NOT match the MISALIGN requirement, (i.e,
the address of the memory
access for the spilled RTL is not misaligned)
therefore triggered the failure later.
That’s the reason I tracked down to this potential issue in the handling of
“CT_SPECIAL_MEMORY”.
thanks a lot.
Qing
> --
> Eric Botcazou
Re: [PATCH] Fix ifunc and resolver (PR ipa/81213).
PING^1 Martin On 06/30/2017 10:47 AM, Martin Liška wrote: Hello. Following patch does refactoring of make_resolver_func where ifunc alias and resolver were probably confused. Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. i386.exp tests work on x86_64-linux-gnu. Ready to be installed? Martin gcc/ChangeLog: 2017-06-29 Martin Liska PR ipa/81213 * config/i386/i386.c (make_resolver_func): Do complete refactoring of the function. gcc/testsuite/ChangeLog: 2017-06-29 Martin Liska PR ipa/81213 * gcc.target/i386/pr81213.c: New test. --- gcc/config/i386/i386.c | 37 - gcc/testsuite/gcc.target/i386/pr81213.c | 19 + 2 files changed, 37 insertions(+), 19 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr81213.c
Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization.
On Jul 10 2017, Maxim Ostapenko wrote:
> diff --git a/gcc/asan.c b/gcc/asan.c
> index 95004d7..89c2731 100644
> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -1567,9 +1567,10 @@ asan_emit_allocas_unpoison (rtx top, rtx bot, rtx_insn
> *before)
>else
> start_sequence ();
>rtx ret = init_one_libfunc ("__asan_allocas_unpoison");
> + top = convert_memory_address (ptr_mode, top);
> + bot = convert_memory_address (ptr_mode, bot);
>ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top,
> - TYPE_MODE (pointer_sized_int_node), bot,
> - TYPE_MODE (pointer_sized_int_node));
> + ptr_mode, bot, ptr_mode);
There is another similar occurence:
/opt/gcc/gcc-20170711/gcc/testsuite/gcc.dg/asan/pr80168.c:7:1: internal
compiler error: in emit_library_call_value_1, at calls.c:4555
0x701577 emit_library_call_value_1
../../gcc/calls.c:4554
0x7068d7 emit_library_call_value(rtx_def*, rtx_def*, libcall_type,
machine_mode, int, ...)
../../gcc/calls.c:5159
0x6f2307 expand_asan_emit_allocas_unpoison
../../gcc/builtins.c:4978
0x6f2307 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
../../gcc/builtins.c:6787
0x81fa6f expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
../../gcc/expr.c:10841
0x716517 expand_expr
../../gcc/expr.h:276
0x716517 expand_call_stmt
../../gcc/cfgexpand.c:2664
0x716517 expand_gimple_stmt_1
../../gcc/cfgexpand.c:3583
0x716517 expand_gimple_stmt
../../gcc/cfgexpand.c:3749
0x719077 expand_gimple_basic_block
../../gcc/cfgexpand.c:5753
0x71dfc7 execute
../../gcc/cfgexpand.c:6360
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
[committed] diagnostics: support compact printing of secondary locations
On Mon, 2017-07-03 at 19:57 +0100, Richard Sandiford wrote:
> [Thanks for all your diagnostic work btw.]
>
> David Malcolm writes:
> > clang can also print notes about matching opening symbols
> > e.g. the note here:
> >
> > missing-symbol-2.c:25:22: error: expected ']'
> > const char test [42;
> >^
> > missing-symbol-2.c:25:19: note: to match this '['
> > const char test [42;
> > ^
> > which, although somewhat redundant for this example, seems much
> > more
> > useful if there's non-trivial nesting of constructs, or more than a
> > few
> > lines separating the open/close symbols (e.g. showing a stray
> > "namespace {"
> > that the user forgot to close).
> >
> > I'd like to implement both of these ideas as followups, but in
> > the meantime, is the fix-it hint patch OK for trunk?
> > (successfully bootstrapped & regrtested on x86_64-pc-linux-gnu)
>
> Just wondering: how easy would it be to restrict the note to the
> kinds
> of cases you mention? TBH I think clang goes in for extra notes too
> much, and it's not always that case that an "expected 'foo'" message
> really is caused by a missing 'foo'. It'd be great if there was some
> way of making the notes a bit more discerning. :-)
>
> Or maybe do something like restrict the extra note to cases in which
> the
> opening character is on a different line and use an underlined range
> when the opening character is on the same line?
>
> Thanks,
> Richard
Thanks.
This patch implements a new method:
bool gcc_rich_location::add_location_if_nearby (location_t);
to make it easy for a diagnostic to compactly print secondary locations
for these kinds of cases, falling back to printing them via a note
otherwise.
Usage example (adapted from the one in the header):
gcc_rich_location richloc (primary_loc);
bool added secondary = richloc.add_location_if_nearby (secondary_loc);
error_at_rich_loc (&richloc, "missing %qs", "}");
if (!added secondary)
inform (secondary_loc, "here's the associated %qs", "{");
When primary_loc and secondary_loc are on the same line this will print:
test.c:1:39: error: missing '}'
struct same_line { double x; double y; ;
~^
When they are on different lines, this will print:
test.c:6:1: error: missing '}'
;
^
test.c:3:1: note: here's the associated '{'
{
^
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu;
takes -fself-test from 39233 passes to 39328 passes.
Committed to trunk as r250133 (and r250134 to fix a ChangeLog snafu).
gcc/ChangeLog:
* diagnostic-show-locus.c: Include "gcc-rich-location.h".
(layout::m_primary_loc): New field.
(layout::layout): Initialize new field. Move location filtering
logic from here to...
(layout::maybe_add_location_range): ...this new method. Add
support for filtering to just the lines already specified by other
locations.
(layout::will_show_line_p): New method.
(gcc_rich_location::add_location_if_nearby): New method.
(selftest::test_add_location_if_nearby): New test function.
(selftest::diagnostic_show_locus_c_tests): Call it.
* gcc-rich-location.h (gcc_rich_location::add_location_if_nearby):
New method.
---
gcc/diagnostic-show-locus.c | 273 +---
gcc/gcc-rich-location.h | 21
2 files changed, 228 insertions(+), 66 deletions(-)
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index 8a4fd5f..5227400 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -27,6 +27,7 @@ along with GCC; see the file COPYING3. If not see
#include "backtrace.h"
#include "diagnostic.h"
#include "diagnostic-color.h"
+#include "gcc-rich-location.h"
#include "selftest.h"
#ifdef HAVE_TERMIOS_H
@@ -196,6 +197,9 @@ class layout
rich_location *richloc,
diagnostic_t diagnostic_kind);
+ bool maybe_add_location_range (const location_range *loc_range,
+bool restrict_to_current_line_spans);
+
int get_num_line_spans () const { return m_line_spans.length (); }
const line_span *get_line_span (int idx) const { return &m_line_spans[idx]; }
@@ -206,6 +210,7 @@ class layout
void print_line (int row);
private:
+ bool will_show_line_p (int row) const;
void print_leading_fixits (int row);
void print_source_line (int row, const char *line, int line_width,
line_bounds *lbounds_out);
@@ -241,6 +246,7 @@ class layout
diagnostic_context *m_context;
pretty_printer *m_pp;
diagnostic_t m_diagnostic_kind;
+ location_t m_primary_loc;
expanded_location m_exploc;
colorizer m_colorizer;
bool m_colorize_source_p;
@@ -767,6 +773,7 @@ layout::layout (diagnostic_context * context,
: m_context (context),
m_pp (context->printer),
m_diagnostic_kind (diagnostic_kind),
+ m_primary_loc (rich
Re: [PATCH][testsuite] Add dg-require-stack-check
On 10 July 2017 at 10:01, Christophe Lyon wrote: > Hi, > > > On 6 July 2017 at 06:50, Jeff Law wrote: >> On 07/04/2017 02:50 AM, Christophe Lyon wrote: >>> On 3 July 2017 at 17:30, Jeff Law wrote: On 07/03/2017 09:00 AM, Christophe Lyon wrote: > Hi, > > This is a follow-up to > https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01791.html > > This patch adds dg-require-stack-check and updates the tests that use > dg-options "-fstack-check" to avoid failures on configurations that to > not support it. > > I merely copied what we currently do to check if visibility flags are > supported, and cross-tested on aarch64 and arm targets with the > results I expected. > > This means that my testing does not cover the changes I propose for > i386 and gnat. > > Is it OK nonetheless? > > Thanks, > > Christophe > > > stack-check-et.chlog.txt > > > 2017-07-03 Christophe Lyon > > * lib/target-supports-dg.exp (dg-require-stack-check): New. > * lib/target-supports.exp (check_stack_check_available): New. > * g++.dg/other/i386-9.C: Add dg-require-stack-check. > * gcc.c-torture/compile/stack-check-1.c: Likewise. > * gcc.dg/graphite/run-id-pr47653.c: Likewise. > * gcc.dg/pr47443.c: Likewise. > * gcc.dg/pr48134.c: Likewise. > * gcc.dg/pr70017.c: Likewise. > * gcc.target/aarch64/stack-checking.c: Likewise. > * gcc.target/arm/stack-checking.c: Likewise. > * gcc.target/i386/pr48723.c: Likewise. > * gcc.target/i386/pr55672.c: Likewise. > * gcc.target/i386/pr67265-2.c: Likewise. > * gcc.target/i386/pr67265.c: Likewise. > * gnat.dg/opt49.adb: Likewise. > * gnat.dg/stack_check1.adb: Likewise. > * gnat.dg/stack_check2.adb: Likewise. > * gnat.dg/stack_check3.adb: Likewise. ACK once you address Rainer's comments. I've got further stack-check tests in the queue which I'll update once your change goes in. jeff >>> Here is an updated version, which adds documentation for >>> dg-require-stack-check. >>> >>> I also ran make-check on and x86_64 with ada enabled and checked the logs: >>> the updated i386/* and gnat.dg* tests all pass, and are preceded by >>> the compilation >>> of the "stack_check" sample. >>> >>> OK? >>> >>> Thanks, >>> >>> Christophe >>> >>> >>> stack-check-et.chlog.txt >>> >>> >>> 2017-07-04 Christophe Lyon >>> >>> gcc/ >>> * doc/sourcebuild.texi (Test Directives, Variants of >>> dg-require-support): Add documentation for dg-require-stack-check. >>> >>> gcc/testsuite/ >>> * lib/target-supports-dg.exp (dg-require-stack-check): New. >>> * lib/target-supports.exp (check_stack_check_available): New. >>> * g++.dg/other/i386-9.C: Add dg-require-stack-check. >>> * gcc.c-torture/compile/stack-check-1.c: Likewise. >>> * gcc.dg/graphite/run-id-pr47653.c: Likewise. >>> * gcc.dg/pr47443.c: Likewise. >>> * gcc.dg/pr48134.c: Likewise. >>> * gcc.dg/pr70017.c: Likewise. >>> * gcc.target/aarch64/stack-checking.c: Likewise. >>> * gcc.target/arm/stack-checking.c: Likewise. >>> * gcc.target/i386/pr48723.c: Likewise. >>> * gcc.target/i386/pr55672.c: Likewise. >>> * gcc.target/i386/pr67265-2.c: Likewise. >>> * gcc.target/i386/pr67265.c: Likewise. >>> * gnat.dg/opt49.adb: Likewise. >>> * gnat.dg/stack_check1.adb: Likewise. >>> * gnat.dg/stack_check2.adb: Likewise. >>> * gnat.dg/stack_check3.adb: Likewise. >> OK for the trunk. Thanks for doing this! >> > > I've committed this as r250013. > > Since then, I've noticed that pr48134 randomly fails. > > According to gcc.log, this seems related the order wrt pr47443. > pr48134 uses -fstack-check=specific, while pr47443 uses -fstack-check=generic. > > When pr47443 appears before pr48134 in gcc.log, the latter fails, > otherwise it is unsupported. > > Looking at gcc.log, it seems that dg-require-stack-check is not always called. > Is there some caching in dejagnu I'm not aware of, that would ignore > the value of the > parameter (assuming that dg-require-stack-check "specific" and > dg-require-stack-check "generic" return the same value?) > > Am I missing anything obvious? > It turns out I was... check_no_compiler_messages actually caches the results using the testcase name, so using "stack_check" was insufficient. The attached patch uses "stack_check_$stack_kind" instead, to make it unique per fstack-check option. OK? Thanks, Christophe > Thanks, > > Christophe > > >> Jeff 2017-07-11 Christophe Lyon gcc/testsuite/ * lib/target-supports.exp (check_stack_check_available): Make testcase name depend on stack_kind. diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 7fb51cc..97d834c 1
Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization.
On 11/07/17 16:51, Andreas Schwab wrote:
On Jul 10 2017, Maxim Ostapenko wrote:
diff --git a/gcc/asan.c b/gcc/asan.c
index 95004d7..89c2731 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1567,9 +1567,10 @@ asan_emit_allocas_unpoison (rtx top, rtx bot, rtx_insn
*before)
else
start_sequence ();
rtx ret = init_one_libfunc ("__asan_allocas_unpoison");
+ top = convert_memory_address (ptr_mode, top);
+ bot = convert_memory_address (ptr_mode, bot);
ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top,
-TYPE_MODE (pointer_sized_int_node), bot,
-TYPE_MODE (pointer_sized_int_node));
+ptr_mode, bot, ptr_mode);
There is another similar occurence:
/opt/gcc/gcc-20170711/gcc/testsuite/gcc.dg/asan/pr80168.c:7:1: internal
compiler error: in emit_library_call_value_1, at calls.c:4555
0x701577 emit_library_call_value_1
../../gcc/calls.c:4554
0x7068d7 emit_library_call_value(rtx_def*, rtx_def*, libcall_type,
machine_mode, int, ...)
../../gcc/calls.c:5159
0x6f2307 expand_asan_emit_allocas_unpoison
../../gcc/builtins.c:4978
0x6f2307 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
../../gcc/builtins.c:6787
0x81fa6f expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
../../gcc/expr.c:10841
0x716517 expand_expr
../../gcc/expr.h:276
0x716517 expand_call_stmt
../../gcc/cfgexpand.c:2664
0x716517 expand_gimple_stmt_1
../../gcc/cfgexpand.c:3583
0x716517 expand_gimple_stmt
../../gcc/cfgexpand.c:3749
0x719077 expand_gimple_basic_block
../../gcc/cfgexpand.c:5753
0x71dfc7 execute
../../gcc/cfgexpand.c:6360
Oh, I see. Does attached patch fix the issue?
-Maxim
Andreas.
gcc/ChangeLog:
2017-07-11 Maxim Ostapenko
* asan.c (asan_emit_allocas_unpoison): Use ptr_mode for arguments
during expansion.
* builtins.c (expand_asan_emit_allocas_unpoison): Likewise.
diff --git a/gcc/asan.c b/gcc/asan.c
index 95004d7..89c2731 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1567,9 +1567,10 @@ asan_emit_allocas_unpoison (rtx top, rtx bot, rtx_insn *before)
else
start_sequence ();
rtx ret = init_one_libfunc ("__asan_allocas_unpoison");
+ top = convert_memory_address (ptr_mode, top);
+ bot = convert_memory_address (ptr_mode, bot);
ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top,
- TYPE_MODE (pointer_sized_int_node), bot,
- TYPE_MODE (pointer_sized_int_node));
+ ptr_mode, bot, ptr_mode);
do_pending_stack_adjust ();
rtx_insn *insns = get_insns ();
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 608993a..6437979 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -4976,9 +4976,7 @@ expand_asan_emit_allocas_unpoison (tree exp)
EXPAND_NORMAL);
rtx ret = init_one_libfunc ("__asan_allocas_unpoison");
ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top,
- TYPE_MODE (pointer_sized_int_node),
- virtual_stack_dynamic_rtx,
- TYPE_MODE (pointer_sized_int_node));
+ ptr_mode, virtual_stack_dynamic_rtx, ptr_mode);
return ret;
}
[PATCH] Remove Pascal language in source code.
Hi.
Similar for GNU Pascal language.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Ready to be installed?
Martin
gcc/ChangeLog:
2017-07-11 Martin Liska
* dbxout.c (get_lang_number): Do not handle GNU Pascal.
* dbxout.h (extern void dbxout_stab_value_internal_label_diff):
Remove N_SO_PASCAL.
* dwarf2out.c (lower_bound_default): Do not handle
DW_LANG_Pascal83.
(gen_compile_unit_die): Likewise.
* gcc.c: Remove default extension binding for GNU Pascal.
* stmt.c: Remove Pascal language from a comment.
* xcoffout.c: Likewise.
---
gcc/dbxout.c| 2 --
gcc/dbxout.h| 1 -
gcc/dwarf2out.c | 3 ---
gcc/gcc.c | 1 -
gcc/stmt.c | 2 +-
gcc/xcoffout.c | 2 +-
6 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/gcc/dbxout.c b/gcc/dbxout.c
index bb8ca3254c0..783a70bec4f 100644
--- a/gcc/dbxout.c
+++ b/gcc/dbxout.c
@@ -952,8 +952,6 @@ get_lang_number (void)
return N_SO_FORTRAN;
else if (lang_GNU_Fortran ())
return N_SO_FORTRAN90; /* CHECKME */
- else if (strcmp (language_string, "GNU Pascal") == 0)
-return N_SO_PASCAL;
else if (strcmp (language_string, "GNU Objective-C") == 0)
return N_SO_OBJC;
else if (strcmp (language_string, "GNU Objective-C++") == 0)
diff --git a/gcc/dbxout.h b/gcc/dbxout.h
index ee6a08d2deb..c3582603253 100644
--- a/gcc/dbxout.h
+++ b/gcc/dbxout.h
@@ -53,7 +53,6 @@ extern void dbxout_stab_value_internal_label_diff (const char *, int *,
#define N_SO_ANSI_C 3
#define N_SO_CC 4 /* c++*/
#define N_SO_FORTRAN 5
-#define N_SO_PASCAL 6
#define N_SO_FORTRAN90 7
#define N_SO_OBJC50
#define N_SO_OBJCPLUS51
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 491c778d58a..9357a100f6a 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -19835,7 +19835,6 @@ lower_bound_default (void)
case DW_LANG_Ada83:
case DW_LANG_Cobol74:
case DW_LANG_Cobol85:
-case DW_LANG_Pascal83:
case DW_LANG_Modula2:
case DW_LANG_PLI:
return dwarf_version >= 4 ? 1 : -1;
@@ -23565,8 +23564,6 @@ gen_compile_unit_die (const char *filename)
}
else if (strcmp (language_string, "GNU F77") == 0)
language = DW_LANG_Fortran77;
- else if (strcmp (language_string, "GNU Pascal") == 0)
-language = DW_LANG_Pascal83;
else if (dwarf_version >= 3 || !dwarf_strict)
{
if (strcmp (language_string, "GNU Ada") == 0)
diff --git a/gcc/gcc.c b/gcc/gcc.c
index ea8858da5d1..d8c5260e36b 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -1305,7 +1305,6 @@ static const struct compiler default_compilers[] =
{".f03", "#Fortran", 0, 0, 0}, {".F03", "#Fortran", 0, 0, 0},
{".f08", "#Fortran", 0, 0, 0}, {".F08", "#Fortran", 0, 0, 0},
{".r", "#Ratfor", 0, 0, 0},
- {".p", "#Pascal", 0, 0, 0}, {".pas", "#Pascal", 0, 0, 0},
{".go", "#Go", 0, 1, 0},
/* Next come the entries for C. */
{".c", "@c", 0, 0, 1},
diff --git a/gcc/stmt.c b/gcc/stmt.c
index 10d394eee69..05e24f00707 100644
--- a/gcc/stmt.c
+++ b/gcc/stmt.c
@@ -1105,7 +1105,7 @@ compute_cases_per_edge (gswitch *stmt)
}
}
-/* Terminate a case (Pascal/Ada) or switch (C) statement
+/* Terminate a case Ada or switch (C) statement
in which ORIG_INDEX is the expression to be tested.
If ORIG_TYPE is not NULL, it is the original ORIG_INDEX
type as given in the source before any compiler conversions.
diff --git a/gcc/xcoffout.c b/gcc/xcoffout.c
index c6eab21a55d..17b201aced6 100644
--- a/gcc/xcoffout.c
+++ b/gcc/xcoffout.c
@@ -143,7 +143,7 @@ static const struct xcoff_type_number xcoff_type_numbers[] = {
{ "float", -12 },
{ "double", -13 },
{ "long double", -14 },
- /* Pascal and Fortran types run from -15 to -29. */
+ /* Fortran types run from -15 to -29. */
{ "wchar", -30 }, /* XXX Should be "wchar_t" ? */
{ "long long int", -31 },
{ "long long unsigned int", -32 },
[RFC] Remaining references of Java
Hi.
There's a small follow-up with remaining occurrences:
1) dwarf2out.c:
20213 origin_die = lookup_type_die (origin);
20214else if (TREE_CODE (origin) == BLOCK)
20215 origin_die = BLOCK_DIE (origin);
20216
20217/* XXX: Functions that are never lowered don't always have correct
block
20218 trees (in the case of java, they simply have no block tree, in
some other
20219 languages). For these functions, there is nothing we can really
do to
20220 output correct debug info for inlined functions in all cases.
Rather
20221 than die, we'll just produce deficient debug info now, in that we
will
20222 have variables without a proper abstract origin. In the future,
when all
20223 functions are lowered, we should re-add a gcc_assert (origin_die)
Probably Jakub can help with that?
2) fold-const.c:
1882/* The following code implements the floating point to integer
1883 conversion rules required by the Java Language Specification,
1884 that IEEE NaNs are mapped to zero and values that overflow
1885 the target precision saturate, i.e. values greater than
1886 INT_MAX are mapped to INT_MAX, and values less than INT_MIN
1887 are mapped to INT_MIN. These semantics are allowed by the
1888 C and C++ standards that simply state that the behavior of
1889 FP-to-integer conversion is unspecified upon overflow. */
1890
1891wide_int val;
1892REAL_VALUE_TYPE r;
1893REAL_VALUE_TYPE x = TREE_REAL_CST (arg1);
Can we somehow remove that Richi?
3) gimplify.c:
2771 Java requires that we elaborated nodes in source order. That
2772 means we must gimplify the inner expression followed by each of
2773 the indices, in order. But we can't gimplify the inner
2774 expression until we deal with any variable bounds, sizes, or
2775 positions in order to deal with PLACEHOLDER_EXPRs.
2776
2777 So we do this in three steps. First we deal with the annotations
2778 for any variables in the components, then we gimplify the base,
2779 then we gimplify any indices, from left to right. */
2780for (i = expr_stack.length () - 1; i >= 0; i--)
Richi?
4) tree.c:
13535if (RECORD_OR_UNION_TYPE_P (t) && TYPE_BINFO (t) && TYPE_BINFO (tv)
13536&& TYPE_BINFO (t) != TYPE_BINFO (tv)
13537/* FIXME: Java sometimes keep dump TYPE_BINFOs on variant types.
13538 Since there is no cheap way to tell C++/Java type w/o LTO, do
checking
13539 at LTO time only. */
13540&& (in_lto_p && odr_type_p (t)))
13541 {
13542error ("type variant has different TYPE_BINFO");
13543debug_tree (tv);
13544error ("type variant's TYPE_BINFO");
13545debug_tree (TYPE_BINFO (tv));
13546error ("type's TYPE_BINFO");
13547debug_tree (TYPE_BINFO (t));
13548return false;
Can we Honza remove that?
Thanks,
Martin
[RFC] Remaining references of Pascal
And there are remaining references of Pascal:
1) dbxout.c:
1661 {
1662stabstr_C ('r');
1663if (TREE_TYPE (type))
1664 dbxout_type (TREE_TYPE (type), 0);
1665else if (TREE_CODE (type) != INTEGER_TYPE)
1666 dbxout_type (type, 0); /* E.g. Pascal's ARRAY [BOOLEAN] of INTEGER
*/
1667else
1668 {
Can we remove that Jason? It's dead according to LCOV output.
2) dwarf2out.c:
23295
23296 #if 0
23297 /* Don't generate either pointer_type DIEs or reference_type DIEs here.
23298 Use modified_type_die instead.
23299 We keep this code here just in case these types of DIEs may be
needed to
23300 represent certain things in other languages (e.g. Pascal) someday.
*/
23301
23302 static void
23303 gen_pointer_type_die (tree type, dw_die_ref context_die)
23304 {
23305dw_die_ref ptr_die
--
23312 }
23313
23314 /* Don't generate either pointer_type DIEs or reference_type DIEs here.
23315 Use modified_type_die instead.
23316 We keep this code here just in case these types of DIEs may be
needed to
23317 represent certain things in other languages (e.g. Pascal) someday.
*/
23318
23319 static void
23320 gen_reference_type_die (tree type, dw_die_ref context_die)
23321 {
23322dw_die_ref ref_die, scope_die = scope_die_for (type, context_die);
The piece of code is guarded in #if 0, is it candidate for removal?
3) stor-layout.c:
2648 /* Set the extreme values of TYPE based on its precision in bits,
2649 then lay it out. Used when make_signed_type won't do
2650 because the tree code is not INTEGER_TYPE.
2651 E.g. for Pascal, when the -fsigned-char option is given. */
2652
2653 void
2654 fixup_signed_type (tree type)
2655 {
2656int precision = TYPE_PRECISION (type);
2657
2658set_min_and_max_values_for_integral_type (type, precision, SIGNED);
2659
2660/* Lay out the type: set its alignment, size, etc. */
2661layout_type (type);
This is probably useful not just for Pascal?
Thanks,
Martin
Re: [Patch, fortran] PR34640 - ICE when assigning item of a derived-component to a pointer
On 07/11/2017 07:23 AM, Paul Richard Thomas wrote: > Well, a bit earlier than anticipated, here is the final version that > puts right all the wrinkles that I know about. > > Bootstraps and regtests - OK for trunk? > > Paul Somewhere in the threads on this, there was mentioned ABI breakage/change. Does it really do this? If the significant change is in the descriptor and you just added the span on the end of the structure, I am not convinced this is an issue. (I have not studied the patch at all, I would rather not bump library version) Jerry
[PATCH 0/3] C/C++: show pertinent open token when missing a close token
[This patch kit is effectively just one patch; I've split it up into
3 parts, in the hope of making it easier to review:
the c-family parts, the C parts, and the C++ parts]
This patch adds a hint to the user to various errors generated
in the C frontend by:
c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")
c_parser_skip_until_found (parser, CPP_CLOSE_BRACE, "expected %<}%>")
etc (where there's a non-NULL msgid), and in the C++ frontend by:
cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)
cp_parser_require (parser, CPP_CLOSE_BRACE, RT_CLOSE_BRACE)
The hint shows the user where the pertinent open paren or open brace
is, which ought to be very helpful for complicated nested collections
of parentheses, and somewhat helpful even for simple cases;
consider e.g.:
...lots of lines of code...
extern "C" {
...lots of lines of code...
int test ();
EOF
where the user currently has to hunt through the source file to find
the unclosed '{':
test.cc:262:12: error: expected '}' at end of input
int test ();
^
With this patch we tell them:
test.cc:262:12: error: expected '}' at end of input
int test ();
^
test.cc:98:12: note: to match this '{'
extern "C" {
^
The patch avoids using a note if the tokens are on the same line,
highlighting the unclosed open token with an underline:
test.c:3:32: error: expected ')' before ';' token
return ((b * b) - (4 * a * c);
~ ^
The bulk of the changes in the patch are to the parsers, done using
new classes "matching_braces" and "matching_parens", which stash the
location of the opening token during parsing, so that e.g.:
if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
return;
...do stuff...
c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>");
becomes:
matching_parens parens;
if (!parens.require_open (parser))
return;
...do stuff...
parens.require_close (parser);
The exact implementation of these classes varies somewhat between the
C and C++ frontends, to deal with implementation differences between
them (I tried to keep them as similar as possible).
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu;
adds 23 PASS results to gcc.sum; adds 99 PASS results to g++.sum.
OK for trunk?
gcc/c-family/c-common.c| 17 +-
gcc/c-family/c-common.h| 3 +-
gcc/c/c-parser.c | 647 ++--
gcc/c/c-parser.h | 8 +-
gcc/cp/parser.c| 821 ++---
gcc/testsuite/c-c++-common/missing-close-symbol.c | 33 +
gcc/testsuite/c-c++-common/missing-symbol.c| 50 ++
.../g++.dg/diagnostic/unclosed-extern-c.C | 3 +
.../g++.dg/diagnostic/unclosed-function.C | 3 +
.../g++.dg/diagnostic/unclosed-namespace.C | 2 +
gcc/testsuite/g++.dg/diagnostic/unclosed-struct.C | 3 +
gcc/testsuite/g++.dg/parse/pragma2.C | 4 +-
gcc/testsuite/gcc.dg/unclosed-init.c | 3 +
13 files changed, 1084 insertions(+), 513 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/missing-close-symbol.c
create mode 100644 gcc/testsuite/c-c++-common/missing-symbol.c
create mode 100644 gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/unclosed-function.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/unclosed-namespace.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/unclosed-struct.C
create mode 100644 gcc/testsuite/gcc.dg/unclosed-init.c
--
1.8.5.3
[PATCH 1/3] matching tokens: c-family parts
OK for trunk? (assuming the rest is approved)
gcc/c-family/ChangeLog:
* c-common.c (c_parse_error): Add rich_location * param, using it
rather implicitly using input_location.
* c-common.h (c_parse_error): Add rich_location * param.
gcc/testsuite/ChangeLog:
* c-c++-common/missing-close-symbol.c: New test case.
* c-c++-common/missing-symbol.c: New test case.
---
gcc/c-family/c-common.c | 17
gcc/c-family/c-common.h | 3 +-
gcc/testsuite/c-c++-common/missing-close-symbol.c | 33 +++
gcc/testsuite/c-c++-common/missing-symbol.c | 50 +++
4 files changed, 94 insertions(+), 9 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/missing-close-symbol.c
create mode 100644 gcc/testsuite/c-c++-common/missing-symbol.c
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index b4217f3..b168cb5 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5949,12 +5949,13 @@ catenate_strings (const char *lhs, const char
*rhs_start, int rhs_size)
return result;
}
-/* Issue the error given by GMSGID, indicating that it occurred before
- TOKEN, which had the associated VALUE. */
+/* Issue the error given by GMSGID at RICHLOC, indicating that it occurred
+ before TOKEN, which had the associated VALUE. */
void
c_parse_error (const char *gmsgid, enum cpp_ttype token_type,
- tree value, unsigned char token_flags)
+ tree value, unsigned char token_flags,
+ rich_location *richloc)
{
#define catenate_messages(M1, M2) catenate_strings ((M1), (M2), sizeof (M2))
@@ -5995,7 +5996,7 @@ c_parse_error (const char *gmsgid, enum cpp_ttype
token_type,
else
message = catenate_messages (gmsgid, " before %s'\\x%x'");
- error (message, prefix, val);
+ error_at_rich_loc (richloc, message, prefix, val);
free (message);
message = NULL;
}
@@ -6023,7 +6024,7 @@ c_parse_error (const char *gmsgid, enum cpp_ttype
token_type,
else if (token_type == CPP_NAME)
{
message = catenate_messages (gmsgid, " before %qE");
- error (message, value);
+ error_at_rich_loc (richloc, message, value);
free (message);
message = NULL;
}
@@ -6036,16 +6037,16 @@ c_parse_error (const char *gmsgid, enum cpp_ttype
token_type,
else if (token_type < N_TTYPES)
{
message = catenate_messages (gmsgid, " before %qs token");
- error (message, cpp_type2name (token_type, token_flags));
+ error_at_rich_loc (richloc, message, cpp_type2name (token_type,
token_flags));
free (message);
message = NULL;
}
else
-error (gmsgid);
+error_at_rich_loc (richloc, gmsgid);
if (message)
{
- error (message);
+ error_at_rich_loc (richloc, message);
free (message);
}
#undef catenate_messages
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 7e7efb2..de92701 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1124,7 +1124,8 @@ extern void builtin_define_with_int_value (const char *,
HOST_WIDE_INT);
extern void builtin_define_type_sizeof (const char *, tree);
extern void c_stddef_cpp_builtins (void);
extern void fe_file_change (const line_map_ordinary *);
-extern void c_parse_error (const char *, enum cpp_ttype, tree, unsigned char);
+extern void c_parse_error (const char *, enum cpp_ttype, tree, unsigned char,
+ rich_location *richloc);
/* In c-ppoutput.c */
extern void init_pp_output (FILE *);
diff --git a/gcc/testsuite/c-c++-common/missing-close-symbol.c
b/gcc/testsuite/c-c++-common/missing-close-symbol.c
new file mode 100644
index 000..85b96f28
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/missing-close-symbol.c
@@ -0,0 +1,33 @@
+/* { dg-options "-fdiagnostics-show-caret" } */
+
+/* Verify that the C/C++ frontends show the pertinent opening symbol when
+ a closing symbol is missing. */
+
+/* Verify that, when they are on the same line, that the opening symbol is
+ shown as a secondary range within the main diagnostic. */
+
+void test_static_assert_same_line (void)
+{
+ _Static_assert(sizeof(int) >= sizeof(char), "msg"; /* { dg-error "expected
'\\)' before ';' token" } */
+ /* { dg-begin-multiline-output "" }
+ _Static_assert(sizeof(int) >= sizeof(char), "msg";
+ ~ ^
+ { dg-end-multiline-output "" } */
+}
+
+/* Verify that, when they are on different lines, that the opening symbol is
+ shown via a secondary diagnostic. */
+
+void test_static_assert_different_line (void)
+{
+ _Static_assert(sizeof(int) >= sizeof(char), /* { dg-message "to match this
'\\('" } */
+"msg"; /* { dg-error "expected '\\)' before ';' token" } */
+ /* { dg-begin-multiline-output "" }
+"msg";
+ ^
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-mu
[PATCH 2/3] matching tokens: C parts
OK for trunk? (assuming the rest is approved) gcc/c/ChangeLog: * c-parser.c (c_parser_error): Rename to... (c_parser_error_richloc): ...this, making static, and adding "richloc" parameter, passing it to the c_parse_error call, rather than calling c_parser_set_source_position_from_token. (c_parser_error): Reintroduce, reimplementing in terms of the above, converting return type from void to bool. (class token_pair): New class. (class matching_parens): New class. (class matching_braces): New class. (get_matching_symbol): New function. (c_parser_require): Add param MATCHING_LOCATION, using it to highlight matching "opening" tokens for missing "closing" tokens. (c_parser_skip_until_found): Likewise. (c_parser_static_assert_declaration_no_semi): Convert explicit parsing of CPP_OPEN_PAREN and CPP_CLOSE_PAREN to use of class matching_parens, so that the pertinent open parenthesis is highlighted when there are problems locating the close parenthesis. (c_parser_struct_or_union_specifier): Likewise. (c_parser_typeof_specifier): Likewise. (c_parser_alignas_specifier): Likewise. (c_parser_simple_asm_expr): Likewise. (c_parser_braced_init): Likewise, for matching_braces. (c_parser_paren_condition): Likewise, for matching_parens. (c_parser_switch_statement): Likewise. (c_parser_for_statement): Likewise. (c_parser_asm_statement): Likewise. (c_parser_asm_operands): Likewise. (c_parser_cast_expression): Likewise. (c_parser_sizeof_expression): Likewise. (c_parser_alignof_expression): Likewise. (c_parser_generic_selection): Likewise. (c_parser_postfix_expression): Likewise for cases RID_VA_ARG, RID_OFFSETOF, RID_TYPES_COMPATIBLE_P, RID_AT_SELECTOR, RID_AT_PROTOCOL, RID_AT_ENCODE, reindenting as necessary. In case CPP_OPEN_PAREN, pass loc_open_paren to the c_parser_skip_until_found call. (c_parser_objc_class_definition): Use class matching_parens as above. (c_parser_objc_method_decl): Likewise. (c_parser_objc_try_catch_finally_statement): Likewise. (c_parser_objc_synchronized_statement): Likewise. (c_parser_objc_at_property_declaration): Likewise. (c_parser_oacc_wait_list): Likewise. (c_parser_omp_var_list_parens): Likewise. (c_parser_omp_clause_collapse): Likewise. (c_parser_omp_clause_default): Likewise. (c_parser_omp_clause_if): Likewise. (c_parser_omp_clause_num_threads): Likewise. (c_parser_omp_clause_num_tasks): Likewise. (c_parser_omp_clause_grainsize): Likewise. (c_parser_omp_clause_priority): Likewise. (c_parser_omp_clause_hint): Likewise. (c_parser_omp_clause_defaultmap): Likewise. (c_parser_oacc_single_int_clause): Likewise. (c_parser_omp_clause_ordered): Likewise. (c_parser_omp_clause_reduction): Likewise. (c_parser_omp_clause_schedule): Likewise. (c_parser_omp_clause_num_teams): Likewise. (c_parser_omp_clause_thread_limit): Likewise. (c_parser_omp_clause_aligned): Likewise. (c_parser_omp_clause_linear): Likewise. (c_parser_omp_clause_safelen): Likewise. (c_parser_omp_clause_simdlen): Likewise. (c_parser_omp_clause_depend): Likewise. (c_parser_omp_clause_map): Likewise. (c_parser_omp_clause_device): Likewise. (c_parser_omp_clause_dist_schedule): Likewise. (c_parser_omp_clause_proc_bind): Likewise. (c_parser_omp_clause_uniform): Likewise. (c_parser_omp_for_loop): Likewise. (c_parser_cilk_clause_vectorlength): Likewise. (c_parser_cilk_clause_linear): Likewise. (c_parser_transaction_expression): Likewise. * c-parser.h (c_parser_require): Add param matching_location with default UNKNOWN_LOCATION. (c_parser_error): Convert return type from void to bool. (c_parser_skip_until_found): Add param matching_location with default UNKNOWN_LOCATION. gcc/testsuite/ChangeLog: * gcc.dg/unclosed-init.c: New test case. --- gcc/c/c-parser.c | 647 +++ gcc/c/c-parser.h | 8 +- gcc/testsuite/gcc.dg/unclosed-init.c | 3 + 3 files changed, 441 insertions(+), 217 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/unclosed-init.c diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index f8fbc92..2dca060 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -850,21 +850,26 @@ c_parser_peek_conflict_marker (c_parser *parser, enum cpp_ttype tok1_kind, MESSAGE (specified by the caller) is usually of the form "expected OTHER-TOKEN". + Use RICHLOC as the location of the diagnostic. + Do not issue a diagnostic if still rec
[PATCH 3/3] matching tokens: C++ parts
OK for trunk? (assuming the rest is approved) gcc/cp/ChangeLog: * parser.c (cp_parser_error): Update for new param to c_parse_error. (class token_pair): New class. (class matching_parens): New class. (class matching_braces): New class. (cp_parser_statement_expr): Convert explicit parsing of CPP_OPEN_PAREN and CPP_CLOSE_PAREN to use of class matching_parens, so that the pertinent open parenthesis is highlighted when there are problems locating the close parenthesis. (cp_parser_primary_expression): Likewise. (cp_parser_postfix_expression): Likewise. (cp_parser_parenthesized_expression_list): Likewise. (cp_parser_unary_expression): Likewise. (cp_parser_new_expression): Likewise. (cp_parser_cast_expression): Likewise. (cp_parser_builtin_offsetof): Likewise. (cp_parser_trait_expr): Likewise. (cp_parser_lambda_declarator_opt): Likewise. (cp_parser_lambda_body): Likewise, for matching_braces. (cp_parser_compound_statement): Likewise. (cp_parser_selection_statement): Likewise, for matching_parens. (cp_parser_iteration_statement): Likewise. (cp_parser_already_scoped_statement): Likewise, for matching_braces. (cp_parser_linkage_specification): Likewise. (cp_parser_static_assert): Likewise, for matching_parens. (cp_parser_decltype): Likewise. (cp_parser_operator): Likewise. (cp_parser_enum_specifier): Likewise. (cp_parser_namespace_definition): Likewise. (cp_parser_direct_declarator): Likewise. (cp_parser_braced_list): Likewise. (cp_parser_class_specifier_1): Likewise, for matching_braces. (cp_parser_constant_initializer): Likewise. (cp_parser_noexcept_specification_opt): Likewise, for matching_parens. (cp_parser_exception_specification_opt): Likewise. (cp_parser_handler): Likewise. (cp_parser_asm_specification_opt): Likewise. (cp_parser_asm_operand_list): Likewise. (cp_parser_gnu_attributes_opt): Likewise. (cp_parser_std_attribute_spec): Likewise. (cp_parser_requirement_parameter_list): Likewise. (cp_parser_requirement_body): Likewise, for matching_braces. (cp_parser_compound_requirement): Likewise. (cp_parser_template_introduction): Likewise. (cp_parser_sizeof_pack): Likewise, for matching_parens. (cp_parser_sizeof_operand): Likewise. (get_matching_symbol): New function. (cp_parser_required_error): Add param "matching_location". Remove calls to cp_parser_error, instead setting a non-NULL gmsgid, and handling it if set by calling c_parse_error, potentially with a secondary location if matching_location was set. (cp_parser_require): Add param "matching_location", with a default value of UNKNOWN_LOCATION. (cp_parser_require_keyword): Update for new param of cp_parser_required_error. (cp_parser_objc_encode_expression): Update to class matching_parens as above. (cp_parser_objc_defs_expression): Likewise. (cp_parser_objc_protocol_expression): Likewise. (cp_parser_objc_selector_expression): Likewise. (cp_parser_objc_typename): Likewise. (cp_parser_objc_superclass_or_category): Likewise. (cp_parser_objc_try_catch_finally_statement): Likewise. (cp_parser_objc_synchronized_statement): Likewise. (cp_parser_objc_at_property_declaration): Likewise. (cp_parser_oacc_single_int_clause): Likewise. (cp_parser_oacc_shape_clause): Likewise. (cp_parser_omp_clause_collapse): Likewise. (cp_parser_omp_clause_default): Likewise. (cp_parser_omp_clause_final): Likewise. (cp_parser_omp_clause_if): Likewise. (cp_parser_omp_clause_num_threads): Likewise. (cp_parser_omp_clause_num_tasks): Likewise. (cp_parser_omp_clause_grainsize): Likewise. (cp_parser_omp_clause_priority): Likewise. (cp_parser_omp_clause_hint): Likewise. (cp_parser_omp_clause_defaultmap): Likewise. (cp_parser_omp_clause_ordered): Likewise. (cp_parser_omp_clause_schedule): Likewise. (cp_parser_omp_clause_num_teams): Likewise. (cp_parser_omp_clause_thread_limit): Likewise. (cp_parser_omp_clause_aligned): Likewise. (cp_parser_omp_clause_linear): Likewise. (cp_parser_omp_clause_safelen): Likewise. (cp_parser_omp_clause_simdlen): Likewise. (cp_parser_omp_clause_depend): Likewise. (cp_parser_omp_clause_device): Likewise. (cp_parser_omp_clause_dist_schedule): Likewise. (cp_parser_oacc_clause_async): Likewise. (cp_parser_omp_critical): Likewise. (cp_parser_omp_for_loop): Likewise. (cp_parser_omp_sections_scope): Likewise. (cp_pars
Re: [PATCH] document IntegerRange in internals manual
On 07/11/2017 07:32 AM, Martin Liška wrote:
On 07/10/2017 05:08 PM, Martin Sebor wrote:
On 07/10/2017 02:35 AM, Martin Liška wrote:
On 07/07/2017 09:20 PM, Martin Sebor wrote:
A conflict in my patch for bug 81345 made me notice that r249734
recently added a new option property, IntegerRange. The change
below adds brief documentation of the property to the manual.
Martin, can you please check to make sure I didn't miss anything?
Btw., while experimenting with the property I noticed that there
is no error when option that specifies IntegerRange is set in
the .opt file to a value outside that range. Would it be hard
to add some checks the the awk scripts to validate that the
argument values are in the range? It might help avoid bugs
similar to 81345).
Sure, please take a look at attached patch. Can you please test it?
The detection works fine for the Init problem (thanks!) but it
doesn't catch the out-of-range initializer in LangEnabledBy(C,
Wall, 2, 0) or in Alias(Wfoobar=, 1, 0). I don't know enough
about the option scripts yet to gauge how difficult handling
these might be. Do you have any idea? If you think it's
doable but outside the scope of this tweak let me know and I'll
open a bug for it to help us remember to handle it at some point
too.
Well, it's definitely doable, but doing that in the current awk script
is quite cumbersome.
I would prefer to have the option generation rewritten e.g. in Python
where current awk
script is not nice to reuse an already parsed information. More
class-oriented approach would
be desired here.
Please create a PR, I maybe rewrite it in future if we can benefit from
that.
I created bug 81397 for all these problems.
Are you interested in the current patch that handles 'Init' directive to
go in?
Yes please, thank you!
Martin
Martin
By the way of an example, the following invalid specification
is accepted but then causes errors when GCC runs.
Wfoobar
C ObjC C++ ObjC++ Warning Alias(Wfoobar=, 1, 0)
Wfoobar=
C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_foobar)
Warning LangEnabledBy(C ObjC C++ ObjC++, Wall, 2, 0) Init (7)
IntegerRange(3, 5)
Here one needs to have 'Init (7)' without space!
Ugh. I only recently realized this but keep forgetting. It
seems like another unnecessary trap that would be nice to fix
at some point.
Thanks
Martin
Martin
diff --git a/gcc/doc/options.texi b/gcc/doc/options.texi
index 3b68aab..af56e9f 100644
--- a/gcc/doc/options.texi
+++ b/gcc/doc/options.texi
@@ -264,6 +264,12 @@ option handler. @code{UInteger} should also be
used on options like
@code{-falign-loops}=@var{n} are supported to make sure the saved
options are given a full integer.
+@item IntegerRange(@var{min}, @var{max})
+The option's integer argument is expected to be in the range specified
+by @var{min} and @var{max}, inclusive. The option parser will check
+and reject option arguments that are outside the range before passing
+it to the relevant option handler.
LGTM, thanks for the documentation entry.
Martin
+
@item ToLower
The option's argument should be converted to lowercase as part of
putting it in canonical form, and before comparing with the strings
Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization.
On Jul 11 2017, Maxim Ostapenko wrote:
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index 608993a..6437979 100644
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -4976,9 +4976,7 @@ expand_asan_emit_allocas_unpoison (tree exp)
>EXPAND_NORMAL);
>rtx ret = init_one_libfunc ("__asan_allocas_unpoison");
>ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top,
> - TYPE_MODE (pointer_sized_int_node),
> - virtual_stack_dynamic_rtx,
> - TYPE_MODE (pointer_sized_int_node));
> + ptr_mode, virtual_stack_dynamic_rtx, ptr_mode);
That doesn't work, same backtrace.
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Re: [PATCH v2][RFC] Canonize names of attributes.
On Tue, Jul 11, 2017 at 9:37 AM, Martin Liška wrote: > On 07/03/2017 11:00 PM, Jason Merrill wrote: >> On Mon, Jul 3, 2017 at 5:52 AM, Martin Liška wrote: >>> On 06/30/2017 09:34 PM, Jason Merrill wrote: On Fri, Jun 30, 2017 at 5:23 AM, Martin Liška wrote: > > This is v2 of the patch, where just names of attributes are > canonicalized. > Patch can bootstrap on ppc64le-redhat-linux and survives regression > tests. What is the purpose of the new "strict" parameter to cmp_attribs* ? I don't see any discussion of it. >>> >>> >>> It's needed for arguments of attribute names, like: >>> >>> /usr/include/stdio.h:391:62: internal compiler error: in cmp_attribs, at >>> tree.h:5523 >>>__THROWNL __attribute__ ((__format__ (__printf__, 3, 4))); >>> >> >> Mm. Although we don't want to automatically canonicalize all >> identifier arguments to attributes in the parser, we could still do it >> for specific attributes, e.g. in handle_format_attribute or >> handle_mode_attribute. > > Yep, that was done in my previous version of the patch > (https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00996.html). > Where only attribute that was preserved unchanged was 'cleanup': > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index 8f638785e0e..08b4db5e5bd 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -24765,7 +24765,8 @@ cp_parser_gnu_attribute_list (cp_parser* parser) > tree tv; > if (arguments != NULL_TREE > && ((tv = TREE_VALUE (arguments)) != NULL_TREE) > - && TREE_CODE (tv) == IDENTIFIER_NODE) > + && TREE_CODE (tv) == IDENTIFIER_NODE > + && !id_equal (TREE_PURPOSE (attribute), "cleanup")) > TREE_VALUE (arguments) = canonize_attr_name (tv); > release_tree_vector (vec); > } > > Does it work for you to do it so? This is canonicalizing arguments by default; I want the default to be not canonicalizing arguments. I think we only want to canonicalize arguments for format and mode, and we can do that in their handle_* functions. Jason
Re: [RFC] Remaining references of Pascal
On Tue, Jul 11, 2017 at 10:42 AM, Martin Liška wrote:
> And there are remaining references of Pascal:
>
> 1) dbxout.c:
>
> 1661 {
> 1662stabstr_C ('r');
> 1663if (TREE_TYPE (type))
> 1664 dbxout_type (TREE_TYPE (type), 0);
> 1665else if (TREE_CODE (type) != INTEGER_TYPE)
> 1666 dbxout_type (type, 0); /* E.g. Pascal's ARRAY [BOOLEAN] of
> INTEGER */
> 1667else
> 1668 {
>
> Can we remove that Jason? It's dead according to LCOV output.
I don't know dbxout, but it seems pretty harmless; I'd be inclined to
keep it even if no current front ends use it.
> 2) dwarf2out.c:
>
> 23295
> 23296 #if 0
> 23297 /* Don't generate either pointer_type DIEs or reference_type DIEs
> here.
> 23298 Use modified_type_die instead.
> 23299 We keep this code here just in case these types of DIEs may be
> needed to
> 23300 represent certain things in other languages (e.g. Pascal)
> someday. */
> 23301
> 23302 static void
> 23303 gen_pointer_type_die (tree type, dw_die_ref context_die)
> 23304 {
> 23305dw_die_ref ptr_die
> --
> 23312 }
> 23313
> 23314 /* Don't generate either pointer_type DIEs or reference_type DIEs
> here.
> 23315 Use modified_type_die instead.
> 23316 We keep this code here just in case these types of DIEs may be
> needed to
> 23317 represent certain things in other languages (e.g. Pascal)
> someday. */
> 23318
> 23319 static void
> 23320 gen_reference_type_die (tree type, dw_die_ref context_die)
> 23321 {
> 23322dw_die_ref ref_die, scope_die = scope_die_for (type, context_die);
>
> The piece of code is guarded in #if 0, is it candidate for removal?
Yes, go ahead.
> 3) stor-layout.c:
>
> 2648 /* Set the extreme values of TYPE based on its precision in bits,
> 2649 then lay it out. Used when make_signed_type won't do
> 2650 because the tree code is not INTEGER_TYPE.
> 2651 E.g. for Pascal, when the -fsigned-char option is given. */
> 2652
> 2653 void
> 2654 fixup_signed_type (tree type)
> 2655 {
> 2656int precision = TYPE_PRECISION (type);
> 2657
> 2658set_min_and_max_values_for_integral_type (type, precision,
> SIGNED);
> 2659
> 2660/* Lay out the type: set its alignment, size, etc. */
> 2661layout_type (type);
>
> This is probably useful not just for Pascal?
Agreed.
Jason
Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization.
On 11/07/17 17:56, Andreas Schwab wrote:
On Jul 11 2017, Maxim Ostapenko wrote:
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 608993a..6437979 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -4976,9 +4976,7 @@ expand_asan_emit_allocas_unpoison (tree exp)
EXPAND_NORMAL);
rtx ret = init_one_libfunc ("__asan_allocas_unpoison");
ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top,
-TYPE_MODE (pointer_sized_int_node),
-virtual_stack_dynamic_rtx,
-TYPE_MODE (pointer_sized_int_node));
+ptr_mode, virtual_stack_dynamic_rtx, ptr_mode);
That doesn't work, same backtrace.
Andreas.
Ok, I see, it seems that we need to add convert in
expand_asan_emit_allocas_unpoison too. This patch seems to work for me
on aarch64 -mabi=ilp32, could you check it as well?
-Maxim
gcc/ChangeLog:
2017-07-11 Maxim Ostapenko
* asan.c (asan_emit_allocas_unpoison): Use ptr_mode for arguments
during expansion.
* builtins.c (expand_asan_emit_allocas_unpoison): Likewise.
diff --git a/gcc/asan.c b/gcc/asan.c
index 95004d7..89c2731 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1567,9 +1567,10 @@ asan_emit_allocas_unpoison (rtx top, rtx bot, rtx_insn *before)
else
start_sequence ();
rtx ret = init_one_libfunc ("__asan_allocas_unpoison");
+ top = convert_memory_address (ptr_mode, top);
+ bot = convert_memory_address (ptr_mode, bot);
ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top,
- TYPE_MODE (pointer_sized_int_node), bot,
- TYPE_MODE (pointer_sized_int_node));
+ ptr_mode, bot, ptr_mode);
do_pending_stack_adjust ();
rtx_insn *insns = get_insns ();
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 608993a..2deef72 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -4972,13 +4972,11 @@ static rtx
expand_asan_emit_allocas_unpoison (tree exp)
{
tree arg0 = CALL_EXPR_ARG (exp, 0);
- rtx top = expand_expr (arg0, NULL_RTX, GET_MODE (virtual_stack_dynamic_rtx),
- EXPAND_NORMAL);
+ rtx top = expand_expr (arg0, NULL_RTX, ptr_mode, EXPAND_NORMAL);
+ rtx bot = convert_memory_address (ptr_mode, virtual_stack_dynamic_rtx);
rtx ret = init_one_libfunc ("__asan_allocas_unpoison");
ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top,
- TYPE_MODE (pointer_sized_int_node),
- virtual_stack_dynamic_rtx,
- TYPE_MODE (pointer_sized_int_node));
+ ptr_mode, bot, ptr_mode);
return ret;
}
[Patch][Aarch64] Refactor comments in aarch64_print_operand
Hi all,
This patch refactors comments in config/aarch64/aarch64.c
aarch64_print_operand
to provide a table of aarch64 specific formating options.
I've tested the patch with a bootstrap and testsuite run on aarch64.
OK for trunk?
Changelog:
gcc/
2017-07-04 Jackson Woodruff
* config/aarch64/aarch64.c (aarch64_print_operand):
Move comments to top of function.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
037339d431d80c49699446e548d6b2707883b6a8..91bf4b3e9792e4ba01232f099ed844bdf23392fa
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -5053,12 +5053,39 @@ static const int aarch64_nzcv_codes[] =
0/* NV, Any. */
};
+/* aarch64 specific string formatting commands:
+ 'c': An integer or symbol address without a preceding # sign.
+ 'e': Print the sign/zero-extend size as a character 8->b,
+ 16->h, 32->w.
+ 'p': Prints N such that 2^N == X (X must be power of 2 and
+ const int).
+ 'P': Print the number of non-zero bits in X (a const_int).
+ 'H': Print the higher numbered register of a pair (TImode)
+ of regs.
+ 'm': Print a condition (eq, ne, etc).
+ 'M': Same as 'm', but invert condition.
+ 'b/q/h/s/d': Print a scalar FP/SIMD register name.
+ 'S/T/U/V':Print the first FP/SIMD register name in a list.
+ 'R': Print a scalar FP/SIMD register name + 1.
+ 'X': Print bottom 16 bits of integer constant in hex.
+ 'w/x':Print a general register name or the zero register
+ (32-bit or 64-bit).
+ '0': Print a normal operand, if it's a general register,
+ then we assume DImode.
+ 'k': Print nzcv.
+ 'A': Output address constant representing the first
+ argument of X, specifying a relocation offset
+ if appropriate.
+ 'L': Output constant address specified by X
+ with a relocation offset if appropriate.
+ 'G': Prints address of X, specifying a PC relative
+ relocation mode if appropriate. */
+
static void
aarch64_print_operand (FILE *f, rtx x, int code)
{
switch (code)
{
-/* An integer or symbol address without a preceding # sign. */
case 'c':
switch (GET_CODE (x))
{
@@ -5085,7 +5112,6 @@ aarch64_print_operand (FILE *f, rtx x, int code)
break;
case 'e':
- /* Print the sign/zero-extend size as a character 8->b, 16->h, 32->w. */
{
int n;
@@ -5118,7 +5144,6 @@ aarch64_print_operand (FILE *f, rtx x, int code)
{
int n;
- /* Print N such that 2^N == X. */
if (!CONST_INT_P (x) || (n = exact_log2 (INTVAL (x))) < 0)
{
output_operand_lossage ("invalid operand for '%%%c'", code);
@@ -5130,7 +5155,6 @@ aarch64_print_operand (FILE *f, rtx x, int code)
break;
case 'P':
- /* Print the number of non-zero bits in X (a const_int). */
if (!CONST_INT_P (x))
{
output_operand_lossage ("invalid operand for '%%%c'", code);
@@ -5141,7 +5165,6 @@ aarch64_print_operand (FILE *f, rtx x, int code)
break;
case 'H':
- /* Print the higher numbered register of a pair (TImode) of regs. */
if (!REG_P (x) || !GP_REGNUM_P (REGNO (x) + 1))
{
output_operand_lossage ("invalid operand for '%%%c'", code);
@@ -5155,8 +5178,6 @@ aarch64_print_operand (FILE *f, rtx x, int code)
case 'm':
{
int cond_code;
- /* Print a condition (eq, ne, etc) or its inverse. */
-
/* CONST_TRUE_RTX means al/nv (al is the default, don't print it). */
if (x == const_true_rtx)
{
@@ -5184,7 +5205,6 @@ aarch64_print_operand (FILE *f, rtx x, int code)
case 's':
case 'd':
case 'q':
- /* Print a scalar FP/SIMD register name. */
if (!REG_P (x) || !FP_REGNUM_P (REGNO (x)))
{
output_operand_lossage ("incompatible floating point / vector
register operand for '%%%c'", code);
@@ -5197,7 +5217,6 @@ aarch64_print_operand (FILE *f, rtx x, int code)
case 'T':
case 'U':
case 'V':
- /* Print the first FP/SIMD register name in a list. */
if (!REG_P (x) || !FP_REGNUM_P (REGNO (x)))
{
output_operand_lossage ("incompatible floating point / vector
register operand for '%%%c'", code);
@@ -5207,7 +5226,6 @@ aarch64_print_operand (FILE *f, rtx x, int code)
break;
case 'R':
- /* Print a scalar FP/SIMD register name + 1. */
if (!REG_P (x) || !FP_REGNUM_P (REGNO (x)))
{
output_operand_lossage ("incompatible floating point / vector
register
Re: [PATCH 0/3] C/C++: show pertinent open token when missing a close token
On 07/11/2017 09:24 AM, David Malcolm wrote:
[This patch kit is effectively just one patch; I've split it up into
3 parts, in the hope of making it easier to review:
the c-family parts, the C parts, and the C++ parts]
This patch adds a hint to the user to various errors generated
in the C frontend by:
c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")
c_parser_skip_until_found (parser, CPP_CLOSE_BRACE, "expected %<}%>")
etc (where there's a non-NULL msgid), and in the C++ frontend by:
cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)
cp_parser_require (parser, CPP_CLOSE_BRACE, RT_CLOSE_BRACE)
The hint shows the user where the pertinent open paren or open brace
is, which ought to be very helpful for complicated nested collections
of parentheses, and somewhat helpful even for simple cases;
I've played with the patch a bit. First, let me say that I like
how it associates the curlies. I agree that it will be helpful.
There are other cases where highlighting mismatched or missing
tokens might be useful, such as for pairs of < and > in complex
template declarations.
But I mainly experimented with it to see if I could get it to
manifest some of the same symptoms I described in bug 81269.
I'm not sure it does reproduce the exact same thing or if it's
a feature, so let me use this as an opportunity to ask. Given
something like
namespace {
enum { e
I see this output:
a.C:3:8: error: expected ‘}’ at end of input
enum { e
~ ^
a.C:3:8: error: expected unqualified-id at end of input
a.C:3:8: error: expected ‘}’ at end of input
a.C:1:11: note: to match this ‘{’
namespace {
^
with the first open curly/caret in green, the 'e' in red, and
the last open curly/caret in cyan.
Is the green color intended? And if yes, what is the intent of
distinguishing it from the red 'e'? I note that the caret is
red (and there are no other colors in the output) in this case:
namespace { enum {
but becomes green again when I add an enumerator:
namespace { enum { e
I ask because in the test case in 81269, highlighting the different
tokens in the three colors seems especially confusing and I'd like
to better understand if it's intentional (and what it means).
Incidentally, I tried to make use of this feature in the middle
end (in gimple-ssa-sprintf.c), to achieve the same effect as
-Wrestric does, but it led to even stranger-looking results so
I went back to using plain old warning. See the attachment to
bug 81269:
https://gcc.gnu.org/bugzilla/attachment.cgi?id=41660
Martin
consider e.g.:
...lots of lines of code...
extern "C" {
...lots of lines of code...
int test ();
EOF
where the user currently has to hunt through the source file to find
the unclosed '{':
test.cc:262:12: error: expected '}' at end of input
int test ();
^
With this patch we tell them:
test.cc:262:12: error: expected '}' at end of input
int test ();
^
test.cc:98:12: note: to match this '{'
extern "C" {
^
The patch avoids using a note if the tokens are on the same line,
highlighting the unclosed open token with an underline:
test.c:3:32: error: expected ')' before ';' token
return ((b * b) - (4 * a * c);
~ ^
The bulk of the changes in the patch are to the parsers, done using
new classes "matching_braces" and "matching_parens", which stash the
location of the opening token during parsing, so that e.g.:
if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
return;
...do stuff...
c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>");
becomes:
matching_parens parens;
if (!parens.require_open (parser))
return;
...do stuff...
parens.require_close (parser);
The exact implementation of these classes varies somewhat between the
C and C++ frontends, to deal with implementation differences between
them (I tried to keep them as similar as possible).
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu;
adds 23 PASS results to gcc.sum; adds 99 PASS results to g++.sum.
OK for trunk?
gcc/c-family/c-common.c| 17 +-
gcc/c-family/c-common.h| 3 +-
gcc/c/c-parser.c | 647 ++--
gcc/c/c-parser.h | 8 +-
gcc/cp/parser.c| 821 ++---
gcc/testsuite/c-c++-common/missing-close-symbol.c | 33 +
gcc/testsuite/c-c++-common/missing-symbol.c| 50 ++
.../g++.dg/diagnostic/unclosed-extern-c.C | 3 +
.../g++.dg/diagnostic/unclosed-function.C | 3 +
.../g++.dg/diagnostic/unclosed-namespace.C | 2 +
gcc/testsuite/g++.dg/diagnostic/unclosed-struct.C | 3 +
gcc/testsuite/g++.dg/parse/pragma2.C | 4 +-
gcc/testsuite/gcc.dg/unclosed-init.c | 3 +
13 files changed, 1084 insertions(+), 513 deletions(
Re: [committed] diagnostics: support compact printing of secondary locations
David Malcolm writes:
> On Mon, 2017-07-03 at 19:57 +0100, Richard Sandiford wrote:
>> [Thanks for all your diagnostic work btw.]
>>
>> David Malcolm writes:
>> > clang can also print notes about matching opening symbols
>> > e.g. the note here:
>> >
>> > missing-symbol-2.c:25:22: error: expected ']'
>> > const char test [42;
>> >^
>> > missing-symbol-2.c:25:19: note: to match this '['
>> > const char test [42;
>> > ^
>> > which, although somewhat redundant for this example, seems much
>> > more
>> > useful if there's non-trivial nesting of constructs, or more than a
>> > few
>> > lines separating the open/close symbols (e.g. showing a stray
>> > "namespace {"
>> > that the user forgot to close).
>> >
>> > I'd like to implement both of these ideas as followups, but in
>> > the meantime, is the fix-it hint patch OK for trunk?
>> > (successfully bootstrapped & regrtested on x86_64-pc-linux-gnu)
>>
>> Just wondering: how easy would it be to restrict the note to the
>> kinds
>> of cases you mention? TBH I think clang goes in for extra notes too
>> much, and it's not always that case that an "expected 'foo'" message
>> really is caused by a missing 'foo'. It'd be great if there was some
>> way of making the notes a bit more discerning. :-)
>>
>> Or maybe do something like restrict the extra note to cases in which
>> the
>> opening character is on a different line and use an underlined range
>> when the opening character is on the same line?
>>
>> Thanks,
>> Richard
>
> Thanks.
>
> This patch implements a new method:
>
>bool gcc_rich_location::add_location_if_nearby (location_t);
>
> to make it easy for a diagnostic to compactly print secondary locations
> for these kinds of cases, falling back to printing them via a note
> otherwise.
>
> Usage example (adapted from the one in the header):
>
> gcc_rich_location richloc (primary_loc);
> bool added secondary = richloc.add_location_if_nearby (secondary_loc);
> error_at_rich_loc (&richloc, "missing %qs", "}");
> if (!added secondary)
> inform (secondary_loc, "here's the associated %qs", "{");
>
> When primary_loc and secondary_loc are on the same line this will print:
>
> test.c:1:39: error: missing '}'
>struct same_line { double x; double y; ;
> ~^
>
> When they are on different lines, this will print:
>
> test.c:6:1: error: missing '}'
>;
>^
> test.c:3:1: note: here's the associated '{'
>{
>^
Thanks, this looks great!
Richard
Re: [PATCH] Add quotes to error messages related to Sanitizers.
On Mon, 2017-07-10 at 11:36 +0200, Martin Liška wrote: > Hi. > > This adds missing quotes to various error messages related to > AddressSanitizer. > Patch can bootstrap on ppc64le-redhat-linux and survives regression > tests. > > Ready to be installed? LGTM, with my "diagnostic messages" maintainer hat on. Grepping for "-f" within opts.c shows a few other diagnostics there that could use quotes, but that's not a reason not to go ahead with this patch. Thanks Dave > Martin > > gcc/ChangeLog: > > 2017-07-04 Martin Liska > > * opts.c (finish_options): Add quotes to error messages. > (parse_sanitizer_options): Likewise. > > gcc/testsuite/ChangeLog: > > 2017-07-04 Martin Liska > > * c-c++-common/ubsan/sanitize-all-1.c: Update scanned pattern. > * c-c++-common/ubsan/sanitize-recover-1.c:Likewise. > * c-c++-common/ubsan/sanitize-recover-2.c:Likewise. > * c-c++-common/ubsan/sanitize-recover-5.c:Likewise. > * c-c++-common/ubsan/sanitize-recover-7.c:Likewise. > * c-c++-common/ubsan/sanitize-recover-8.c:Likewise. > * c-c++-common/ubsan/sanitize-recover-9.c:Likewise. > --- > gcc/opts.c| 18 + > - > gcc/testsuite/c-c++-common/ubsan/sanitize-all-1.c | 2 +- > gcc/testsuite/c-c++-common/ubsan/sanitize-recover-1.c | 2 +- > gcc/testsuite/c-c++-common/ubsan/sanitize-recover-2.c | 2 +- > gcc/testsuite/c-c++-common/ubsan/sanitize-recover-5.c | 2 +- > gcc/testsuite/c-c++-common/ubsan/sanitize-recover-7.c | 2 +- > gcc/testsuite/c-c++-common/ubsan/sanitize-recover-8.c | 2 +- > gcc/testsuite/c-c++-common/ubsan/sanitize-recover-9.c | 2 +- > 8 files changed, 16 insertions(+), 16 deletions(-) > >
Re: [Patch, fortran] PR34640 - ICE when assigning item of a derived-component to a pointer
Am 11.07.2017 um 16:48 schrieb Jerry DeLisle:
Somewhere in the threads on this, there was mentioned ABI breakage/change.
That was me.
Does it really do this?
Yes. Look at this part:
Index: libgfortran/libgfortran.h
===
*** libgfortran/libgfortran.h(revision 250082)
--- libgfortran/libgfortran.h(working copy)
*** struct {\
*** 339,344
--- 339,345
type *base_addr;\
size_t offset;\
index_type dtype;\
+ index_type span;\
descriptor_dimension dim[r];\
}
> If the significant change is in the descriptor and you
> just added the span on the end of the structure, I am not convinced
this is an
> issue. (I have not studied the patch at all, I would rather not bump
library
> version)
Unless I am mistaken, we only build the required dimensions for
an array descriptor. Putting it on the end would not work
unless we changed that behavior.
But we are doing something wrong with the array descriptors anyway. See
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68649#c7 for a description.
Other comments in the same PR have some suggestions, but nothing that
works (or so I think).
So, if we do break the ABI, we could try to fix the remaining
issues with the array descriptors - not with this patch, but
before 8.1 is released. Flexible array members come to mind.
Regards
Thomas
Re: [PATCH 0/3] C/C++: show pertinent open token when missing a close token
On Tue, 2017-07-11 at 11:28 -0600, Martin Sebor wrote:
> On 07/11/2017 09:24 AM, David Malcolm wrote:
> > [This patch kit is effectively just one patch; I've split it up
> > into
> > 3 parts, in the hope of making it easier to review:
> > the c-family parts, the C parts, and the C++ parts]
> >
> > This patch adds a hint to the user to various errors generated
> > in the C frontend by:
> >
> > c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")
> > c_parser_skip_until_found (parser, CPP_CLOSE_BRACE, "expected
> > %<}%>")
> >
> > etc (where there's a non-NULL msgid), and in the C++ frontend by:
> >
> > cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)
> > cp_parser_require (parser, CPP_CLOSE_BRACE, RT_CLOSE_BRACE)
> >
> > The hint shows the user where the pertinent open paren or open
> > brace
> > is, which ought to be very helpful for complicated nested
> > collections
> > of parentheses, and somewhat helpful even for simple cases;
>
> I've played with the patch a bit. First, let me say that I like
> how it associates the curlies. I agree that it will be helpful.
> There are other cases where highlighting mismatched or missing
> tokens might be useful, such as for pairs of < and > in complex
> template declarations.
Indeed; braces and parens seemed most useful; my plan is to leave
< > and [ ] for followups.
> But I mainly experimented with it to see if I could get it to
> manifest some of the same symptoms I described in bug 81269.
> I'm not sure it does reproduce the exact same thing or if it's
> a feature, so let me use this as an opportunity to ask. Given
> something like
>
>namespace {
>
>enum { e
>
>
> I see this output:
>
>a.C:3:8: error: expected ‘}’ at end of input
> enum { e
> ~ ^
>a.C:3:8: error: expected unqualified-id at end of input
>a.C:3:8: error: expected ‘}’ at end of input
>a.C:1:11: note: to match this ‘{’
> namespace {
> ^
I think this is an issue with how the diagnostics subsystem chooses
colors; it looks like it's time to rethink that.
Here's what's currently going on:
The color-selection is done in class colorizer in diagnostic-show
-locus.c; the exact colors are in diagnostic-colors.c
> with the first open curly/caret in green,
This is range 1 within its rich_location, and so colorizer uses state
1, and hence uses the color named "range1", which is implemented as
COLOR_FG_GREEN aka "range1=32" in GCC_COLORS.
> the 'e' in red, and
> the last open curly/caret in cyan.
This is range 0 within its rich_location, and so colorizer uses the
state 0:
/* Make range 0 be the same color as the "kind" text
(error vs warning vs note). */
This diagnostic is an error, and hence it uses the color named "error",
which is bold red.
> Is the green color intended? And if yes, what is the intent of
> distinguishing it from the red 'e'? I note that the caret is
> red (and there are no other colors in the output) in this case:
>
>namespace { enum {
>
>
> but becomes green again when I add an enumerator:
>
>namespace { enum { e
>
Is there an extra line here that you trimmed? Presumably in this case
the '{' is being underlined with a "~", and the "e" has the "^" (the
caret)?
In this case, the red is used for that of the caret, and the green for
the secondary location.
I picked this system for coloring the source and annotations in gcc 6
(IIRC), but it seems garish to me now; I think I now favor emulating
what clang does, which is to *not* color the printed source lines, and
to use just one color (green) for underlines and carets.
Can I use PR 81269 for tracking a refresh of how we do colorization in
diagnostics?
> I ask because in the test case in 81269, highlighting the different
> tokens in the three colors seems especially confusing and I'd like
> to better understand if it's intentional (and what it means).
>
> Incidentally, I tried to make use of this feature in the middle
> end (in gimple-ssa-sprintf.c), to achieve the same effect as
> -Wrestric does, but it led to even stranger-looking results so
> I went back to using plain old warning. See the attachment to
> bug 81269:
> https://gcc.gnu.org/bugzilla/attachment.cgi?id=41660
Looks like you're seeing a different bug there: you're seeing:
sprintf (d, "%s%s%s", d, d + 1, d + 2);
~^
with weird-looking colorization of some arguments.
I think what's going on is we have:
primary location:
sprintf (d, "%s%s%s", d, d + 1, d + 2);
^
secondary location 1:
sprintf (d, "%s%s%s", d, d + 1, d + 2);
~
to cover: ^
but this PARAM_DECL usage doesn't have a location, and so
it uses the whole of the call
secondary location 1:
sprintf (d, "%s%s%s", d, d + 1, d + 2);
~~^~~
The bug here I think is that diagnostic_show_locus is printing all of these
annotation on top of each
Re: [PATCH 1/2] x86,s390: add compiler memory barriers when expanding atomic_thread_fence (PR 80640)
On Thu, 8 Jun 2017, Alexander Monakov wrote:
> Ping^3.
Ping^4: https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00782.html
This is a wrong-code issue with C11 atomics: even if no machine barrier is
needed for a given fence type on this architecture, a compiler barrier must
be present in RTL.
Alternatively, it's possible to have a more complete and future-proof solution
by explicitly emitting a compiler barrier from the middle-end, leaving it up
to the backend to emit a machine barrier if needed. The following patch could
achieve that (but at the cost of creating slightly redundant RTL on targets
that always emit some kind of memory barrier).
* optabs.c (expand_mem_thread_fence): Always emit a compiler barrier
if using mem_thread_fence expansion.
diff --git a/gcc/optabs.c b/gcc/optabs.c
index 8fd5d91..92080c3 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -6297,7 +6297,11 @@ void
expand_mem_thread_fence (enum memmodel model)
{
if (targetm.have_mem_thread_fence ())
-emit_insn (targetm.gen_mem_thread_fence (GEN_INT (model)));
+{
+ emit_insn (targetm.gen_mem_thread_fence (GEN_INT (model)));
+ if (!is_mm_relaxed (model))
+ expand_asm_memory_barrier ();
+}
else if (!is_mm_relaxed (model))
{
if (targetm.have_memory_barrier ())
C++ PATCH for Core DR 393, parm w/ pointer to array of unknown bound
Core DR 393 in C++17 removed the restriction on parameters with
pointer to array of unknown bound type; accordingly, I've reduced the
diagnostic for earlier standards to a pedwarn.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit a89d6192b65fc0eef3d95592c212a389928b6cba
Author: Jason Merrill
Date: Tue Jul 11 12:08:10 2017 -0400
Core DR 393 - parameter pointer to array of unknown bound
* decl.c (grokparms): Downgrade error about array of unknown bound
to pedwarn and disable it for C++17.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 43a94d9..b9b8794 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -12591,9 +12591,11 @@ grokparms (tree parmlist, tree *parms)
}
else if (abstract_virtuals_error (decl, type))
any_error = 1; /* Seems like a good idea. */
- else if (POINTER_TYPE_P (type))
+ else if (cxx_dialect < cxx1z
+ && POINTER_TYPE_P (type))
{
- /* [dcl.fct]/6, parameter types cannot contain pointers
+ /* Before C++17 DR 393:
+[dcl.fct]/6, parameter types cannot contain pointers
(references) to arrays of unknown bound. */
tree t = TREE_TYPE (type);
int ptr = TYPE_PTR_P (type);
@@ -12609,7 +12611,8 @@ grokparms (tree parmlist, tree *parms)
t = TREE_TYPE (t);
}
if (TREE_CODE (t) == ARRAY_TYPE)
- error (ptr
+ pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wpedantic,
+ptr
? G_("parameter %qD includes pointer to array of "
"unknown bound %qT")
: G_("parameter %qD includes reference to array of "
diff --git a/gcc/testsuite/g++.dg/cpp1z/dr393.C
b/gcc/testsuite/g++.dg/cpp1z/dr393.C
new file mode 100644
index 000..4a7645a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/dr393.C
@@ -0,0 +1,4 @@
+// DR 393
+// { dg-options -Wpedantic }
+
+void f(int (&)[]); // { dg-warning "unknown bound" "" { target c++14_down } }
Re: A potential bug in lra-constraints.c for special_memory_constraint?
> the problem I had is:
>
> 1. we added a new special_memory_constraint for misaligned memory access,
> one important requirement for this new special_memory_constraint is, the
> address of the memory access is misaligned.
OK, it's the other way around from the usage of special_memory_constraint.
In other words, I'm not sure that you really need to use it in this case,
a standard memory_constraint could be sufficient.
> 2. per the current code in lra-constraints.c:
>
> 2286 case CT_SPECIAL_MEMORY:
> 2287 if (MEM_P (op)
> 2288 && satisfies_memory_constraint_p (op, cn))
> 2289 win = true;
> 2290 else if (spilled_pseudo_p (op))
> 2291 win = true;
> 2292 break;
>
> if the op is a pseudo_p that can be spilled, then it's treated as a
> PERFECT MATCH.
>
> the issue only can be exposed by the following kind of RTL:
>
> (insn 34 13 14 2 (set (reg:DI 122)
>(reg:DI 129)) misalign-3.c:12 125 {*movdi_insn_sp64}
> (nil))
>
> i.e.
> (1). REG2 move to REG1
> and. (2). REG2 is a virtual reg (> the max hard regno, on Sparc, its 103),
> therefore, must be spilled to stack.
>
> the current interpretation of special memory treat such REG2 as a perfect
> match to special memory, and then spill it.
> however, such spilled memory RTL is NOT match the MISALIGN requirement,
> (i.e, the address of the memory access for the spilled RTL is not
> misaligned)
Yes, spilling will automatically meet alignment requirements, that's why it's
allowed for special_memory_constraint.
Why do you absolutely need to have a misaligned address? Can't you just avert
your eyes and pretend that the address is misaligned? This will be suboptimal
but presumably work. To be honest, I'm not even sure that you really need an
additional constraint, but I haven't investigated the subject seriously.
--
Eric Botcazou
Re: A potential bug in lra-constraints.c for special_memory_constraint?
> On Jul 11, 2017, at 2:00 PM, Eric Botcazou wrote:
>
>> the problem I had is:
>>
>> 1. we added a new special_memory_constraint for misaligned memory access,
>> one important requirement for this new special_memory_constraint is, the
>> address of the memory access is misaligned.
>
> OK, it's the other way around from the usage of special_memory_constraint.
> In other words, I'm not sure that you really need to use it in this case,
> a standard memory_constraint could be sufficient.
A standard memory_constraint has the same handling on spilled code as
special_memory_constraint:
2250 case CT_MEMORY:
2251 if (MEM_P (op)
2252 && satisfies_memory_constraint_p (op, cn))
2253 win = true;
2254 else if (spilled_pseudo_p (op))
2255 win = true;
as a result, if the new misaligned memory access was defined as a standard
memory_constraint, will have the same issue.
>
>> 2. per the current code in lra-constraints.c:
>>
>> 2286 case CT_SPECIAL_MEMORY:
>> 2287 if (MEM_P (op)
>> 2288 && satisfies_memory_constraint_p (op, cn))
>> 2289 win = true;
>> 2290 else if (spilled_pseudo_p (op))
>> 2291 win = true;
>> 2292 break;
>>
>> if the op is a pseudo_p that can be spilled, then it's treated as a
>> PERFECT MATCH.
>>
>> the issue only can be exposed by the following kind of RTL:
>>
>> (insn 34 13 14 2 (set (reg:DI 122)
>> (reg:DI 129)) misalign-3.c:12 125 {*movdi_insn_sp64}
>>(nil))
>>
>> i.e.
>>(1). REG2 move to REG1
>> and. (2). REG2 is a virtual reg (> the max hard regno, on Sparc, its 103),
>> therefore, must be spilled to stack.
>>
>> the current interpretation of special memory treat such REG2 as a perfect
>> match to special memory, and then spill it.
>> however, such spilled memory RTL is NOT match the MISALIGN requirement,
>> (i.e, the address of the memory access for the spilled RTL is not
>> misaligned)
>
> Yes, spilling will automatically meet alignment requirements, that's why it's
> allowed for special_memory_constraint.
You mean, even for the mis-alignment requirement, the spilled memory access
will met the mis-alignment?
>
> Why do you absolutely need to have a misaligned address?
we need to generate misaligned load/store insns ONLY for misaligned memory
access, therefore need a new
constraints for misaligned address.
As I checked the GCC source code, the special_memory_constraints only were
defined in i386 and sparc target, not
used quite often.
Qing
> Can't you just avert
> your eyes and pretend that the address is misaligned? This will be
> suboptimal
> but presumably work. To be honest, I'm not even sure that you really need an
> additional constraint, but I haven't investigated the subject seriously.
>
> --
> Eric Botcazou
Re: [PATCH 0/3] C/C++: show pertinent open token when missing a close token
On 07/11/2017 12:32 PM, David Malcolm wrote:
On Tue, 2017-07-11 at 11:28 -0600, Martin Sebor wrote:
On 07/11/2017 09:24 AM, David Malcolm wrote:
[This patch kit is effectively just one patch; I've split it up
into
3 parts, in the hope of making it easier to review:
the c-family parts, the C parts, and the C++ parts]
This patch adds a hint to the user to various errors generated
in the C frontend by:
c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")
c_parser_skip_until_found (parser, CPP_CLOSE_BRACE, "expected
%<}%>")
etc (where there's a non-NULL msgid), and in the C++ frontend by:
cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)
cp_parser_require (parser, CPP_CLOSE_BRACE, RT_CLOSE_BRACE)
The hint shows the user where the pertinent open paren or open
brace
is, which ought to be very helpful for complicated nested
collections
of parentheses, and somewhat helpful even for simple cases;
I've played with the patch a bit. First, let me say that I like
how it associates the curlies. I agree that it will be helpful.
There are other cases where highlighting mismatched or missing
tokens might be useful, such as for pairs of < and > in complex
template declarations.
Indeed; braces and parens seemed most useful; my plan is to leave
< > and [ ] for followups.
But I mainly experimented with it to see if I could get it to
manifest some of the same symptoms I described in bug 81269.
I'm not sure it does reproduce the exact same thing or if it's
a feature, so let me use this as an opportunity to ask. Given
something like
namespace {
enum { e
I see this output:
a.C:3:8: error: expected ‘}’ at end of input
enum { e
~ ^
a.C:3:8: error: expected unqualified-id at end of input
a.C:3:8: error: expected ‘}’ at end of input
a.C:1:11: note: to match this ‘{’
namespace {
^
I think this is an issue with how the diagnostics subsystem chooses
colors; it looks like it's time to rethink that.
Here's what's currently going on:
The color-selection is done in class colorizer in diagnostic-show
-locus.c; the exact colors are in diagnostic-colors.c
with the first open curly/caret in green,
This is range 1 within its rich_location, and so colorizer uses state
1, and hence uses the color named "range1", which is implemented as
COLOR_FG_GREEN aka "range1=32" in GCC_COLORS.
the 'e' in red, and
the last open curly/caret in cyan.
This is range 0 within its rich_location, and so colorizer uses the
state 0:
/* Make range 0 be the same color as the "kind" text
(error vs warning vs note). */
This diagnostic is an error, and hence it uses the color named "error",
which is bold red.
I see. Thanks for the detailed explanation! It cleared things
up for me.
Is the green color intended? And if yes, what is the intent of
distinguishing it from the red 'e'? I note that the caret is
red (and there are no other colors in the output) in this case:
namespace { enum {
but becomes green again when I add an enumerator:
namespace { enum { e
Is there an extra line here that you trimmed? Presumably in this case
the '{' is being underlined with a "~", and the "e" has the "^" (the
caret)?
The source file has just one line in both instances, but yes,
the underlining is as you described.
In this case, the red is used for that of the caret, and the green for
the secondary location.
Aha! I think I understand now. In the first case there is no
green because both the primary and the secondary location are
the unmatched open curly. That makes sense, though it's not
quite obvious (I haven't worked with secondary locations yet
or noticed them in other diagnostics).
If you don't completely rework things (and even if you do) it
would be helpful to document this in more detail in the manual.
E.g., describe what range1 and range2 (and other less obvious
elements) are used for and show a couple of examples. Or for
GCC developers, add some of this detail to one of the Wiki
pages.
I picked this system for coloring the source and annotations in gcc 6
(IIRC), but it seems garish to me now; I think I now favor emulating
what clang does, which is to *not* color the printed source lines, and
to use just one color (green) for underlines and carets.
I'm sure it's a matter of personal preference but overall I like
the GCC highlighting scheme better. It makes it clear what the
source of the problem is. Especially with fix-it hints in green,
it makes it clear what's "good" and what's "bad."
Can I use PR 81269 for tracking a refresh of how we do colorization in
diagnostics?
Sure, feel free to use it however you see fit.
I ask because in the test case in 81269, highlighting the different
tokens in the three colors seems especially confusing and I'd like
to better understand if it's intentional (and what it means).
Incidentally, I tried to make use of this feature in the middle
end (in gimple-ssa-sprintf.c)
Re: [Patch, fortran] PR34640 - ICE when assigning item of a derived-component to a pointer
Hi Jerry and Thomas,
As Thomas noted, the span field is added in the middle of the
descriptor because the caf token field makes the descriptor variable
length. This is reflected in the change in libgfortran.h.
It has crossed my mind in the last twenty four hours that I should add
some more fields, for example by expanding the dtype field, which
would then allow us to bump up the maximum number of dimensions for
example.
However, I seem, temporarily I hope, to be completely blown out of the
water. We had a rainstorm this afternoon, which caused a glitch in the
mains. Now, neither of my workstations seem to work any more. I have
tried everything but both remain totally unresponsive.
As to anything to do with lto, I am sorry but it is beyond my pay
grade. I got caught with lto in implementing the submodule patch. I
got lucky in that I found the fix more or less by trying things at
random. That said, I'll take a look at p68649 once my blood pressure
has dropped. It seems to me that the gurus have provided more than
enough clues.
Regards
Paul
On 11 July 2017 at 19:12, Thomas Koenig wrote:
> Am 11.07.2017 um 16:48 schrieb Jerry DeLisle:
>
>> Somewhere in the threads on this, there was mentioned ABI breakage/change.
>
>
> That was me.
>
>> Does it really do this?
>
>
> Yes. Look at this part:
>
> Index: libgfortran/libgfortran.h
> ===
> *** libgfortran/libgfortran.h(revision 250082)
> --- libgfortran/libgfortran.h(working copy)
> *** struct {\
> *** 339,344
> --- 339,345
> type *base_addr;\
> size_t offset;\
> index_type dtype;\
> + index_type span;\
> descriptor_dimension dim[r];\
> }
>
>> If the significant change is in the descriptor and you
>> just added the span on the end of the structure, I am not convinced this
>> is an
>> issue. (I have not studied the patch at all, I would rather not bump
>> library
>> version)
>
> Unless I am mistaken, we only build the required dimensions for
> an array descriptor. Putting it on the end would not work
> unless we changed that behavior.
>
> But we are doing something wrong with the array descriptors anyway. See
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68649#c7 for a description.
> Other comments in the same PR have some suggestions, but nothing that
> works (or so I think).
>
> So, if we do break the ABI, we could try to fix the remaining
> issues with the array descriptors - not with this patch, but
> before 8.1 is released. Flexible array members come to mind.
>
> Regards
>
> Thomas
--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Re: [PATCH, AArch64] Add RDMA support to Falkor.
Ping.
Jim
On Thu, Jun 29, 2017 at 1:53 PM, Jim Wilson wrote:
> Falkor is an ARMV8-A part, but also includes the RDMA extension from
> ARMV8.1-A.
> I'd like to enable support for the RDMA instructions when -mcpu=falkor is
> used,
> and also make the RDMA intrisics available. To do that, I need to add rdma
> as an architecture extension, and modify a few things to use it. Binutils
> already supports rdma as an architecture extension.
>
> I only did the aarch64 port, and not the arm port. There are no supported
> targets that have the RDMA instructions and also aarch32 support. There are
> also no aarch32 RDMA testcases. So there is no way to test it. It wasn't
> clear whether it was better to add something untested or leave it out. I
> chose
> to leave it out for now.
>
> I also needed a few testcase changes. There were redundant options being
> added for the RDMA tests that I had to remove as they are now wrong. Also
> the fact that I only did aarch64 means we need to check both armv8-a+rdma and
> armv8.1-a for the rdma support.
>
> This was tested with an aarch64 bootstrap and make check. There were no
> regressions.
>
> OK?
>
> Jim
>
> gcc/
> * config/aarch64/aarch64-cores.def (falkor): Add AARCH64_FL_RDMA.
> (qdf24xx): Likewise.
> * config/aarch64/aarch64-options-extensions.def (rdma); New.
> * config/aarch64/aarch64.h (AARCH64_FL_RDMA): New.
> (AARCH64_FL_V8_1): Renumber.
> (AARCH64_FL_FOR_ARCH8_1): Add AARCH64_FL_RDMA.
> (AARCH64_ISA_RDMA): Use AARCH64_FL_RDMA.
> * config/aarch64/arm_neon.h: Use +rdma instead of arch=armv8.1-a.
> * doc/invoke.texi (AArch64 Options): Mention +rmda in -march docs.
> Add
> rdma to feature modifiers list.
>
> gcc/testsuite/
> * lib/target-supports.exp (add_options_for_arm_v8_1a_neon): Delete
> redundant -march option.
> (check_effective_target_arm_v8_1a_neon_ok_nocache): Try armv8-a+rdma
> in addition to armv8.1-a.
> ---
> gcc/config/aarch64/aarch64-cores.def | 4 ++--
> gcc/config/aarch64/aarch64-option-extensions.def | 4
> gcc/config/aarch64/aarch64.h | 8 +---
> gcc/config/aarch64/arm_neon.h| 2 +-
> gcc/doc/invoke.texi | 5 -
> gcc/testsuite/lib/target-supports.exp| 18 ++
> 6 files changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64-cores.def
> b/gcc/config/aarch64/aarch64-cores.def
> index f8342ca..b8d0ba6 100644
> --- a/gcc/config/aarch64/aarch64-cores.def
> +++ b/gcc/config/aarch64/aarch64-cores.def
> @@ -65,8 +65,8 @@ AARCH64_CORE("thunderxt83", thunderxt83, thunderx, 8A,
> AARCH64_FL_FOR_ARCH
> AARCH64_CORE("xgene1", xgene1,xgene1,8A, AARCH64_FL_FOR_ARCH8,
> xgene1, 0x50, 0x000, -1)
>
> /* Qualcomm ('Q') cores. */
> -AARCH64_CORE("falkor", falkor,cortexa57, 8A, AARCH64_FL_FOR_ARCH8
> | AARCH64_FL_CRC | AARCH64_FL_CRYPTO, qdf24xx, 0x51, 0xC00, -1)
> -AARCH64_CORE("qdf24xx", qdf24xx, cortexa57, 8A, AARCH64_FL_FOR_ARCH8
> | AARCH64_FL_CRC | AARCH64_FL_CRYPTO, qdf24xx, 0x51, 0xC00, -1)
> +AARCH64_CORE("falkor", falkor,cortexa57, 8A, AARCH64_FL_FOR_ARCH8
> | AARCH64_FL_CRC | AARCH64_FL_CRYPTO | AARCH64_FL_RDMA, qdf24xx, 0x51,
> 0xC00, -1)
> +AARCH64_CORE("qdf24xx", qdf24xx, cortexa57, 8A, AARCH64_FL_FOR_ARCH8
> | AARCH64_FL_CRC | AARCH64_FL_CRYPTO | AARCH64_FL_RDMA, qdf24xx, 0x51,
> 0xC00, -1)
>
> /* Samsung ('S') cores. */
> AARCH64_CORE("exynos-m1", exynosm1, exynosm1, 8A, AARCH64_FL_FOR_ARCH8
> | AARCH64_FL_CRC | AARCH64_FL_CRYPTO, exynosm1, 0x53, 0x001, -1)
> diff --git a/gcc/config/aarch64/aarch64-option-extensions.def
> b/gcc/config/aarch64/aarch64-option-extensions.def
> index c0752ce..c4f059a 100644
> --- a/gcc/config/aarch64/aarch64-option-extensions.def
> +++ b/gcc/config/aarch64/aarch64-option-extensions.def
> @@ -63,4 +63,8 @@ AARCH64_OPT_EXTENSION("fp16", AARCH64_FL_F16,
> AARCH64_FL_FP, 0, "fphp asimdhp")
> /* Enabling or disabling "rcpc" only changes "rcpc". */
> AARCH64_OPT_EXTENSION("rcpc", AARCH64_FL_RCPC, 0, 0, "lrcpc")
>
> +/* Enabling "rdma" also enables "fp", "simd".
> + Disabling "rdma" just disables "rdma". */
> +AARCH64_OPT_EXTENSION("rdma", AARCH64_FL_RDMA, AARCH64_FL_FP |
> AARCH64_FL_SIMD, 0, "rdma")
> +
> #undef AARCH64_OPT_EXTENSION
> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index 106cf3a..7f91edb 100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -144,7 +144,8 @@ extern unsigned aarch64_architecture_version;
> #define AARCH64_FL_CRC(1 << 3) /* Has CRC. */
> /* ARMv8.1-A architecture extensions. */
> #define AARCH64_FL_LSE (1 << 4) /* Has Large System Extensions. */
> -#define AARCH64_FL_V8_1 (1 << 5) /* Has ARMv8.1-A extensions.
> */
> +#d
[PATCH][RFA/RFC] Stack clash mitigation 0/9
This patch series is designed to mitigate the problems exposed by the stack-clash exploits. As I've noted before, the way to address this class of problems is via a good stack probing strategy. This has taken much longer than expected to pull together for submission. Sorry about that. However, the delay has led to some clear improvements on ppc, aarch64 and s390 as well as tests which aren't eyeballed, but instead are part of the testsuite. This series introduces -fstack-check=clash which is a variant of -fstack-check designed to prevent "jumping the stack" as seen in the stack-clash exploits. The key ideas here: Individual stack allocations are never more than PROBE_INTERVAL in size (4k by default). Larger allocations are broken up into PROBE_INTERVAL chunks and each chunk is probed as it is allocated. No combination of stack allocations can exceed PROBE_INTERVAL bytes without probing. ie, if we have an allocation of 2k and a later allocation of 3k, then there must be a stack probe into the first 4k of allocated space that executes between the two allocations. We must consider an environment where code compiled without stack probing is linked statically or dynamically with code that is compiled with stack probing. That is actually the most likely scenario for an indefinite period of time. Thus we have to consider the possibility of a hostile caller in the call stack. We need not guarantee enough stack space to handle a signal if a probe hits the guard page. -- Probes come in two forms. They can be explicit or implicit. Explicit probes are emitted by prologue generation or dynamic stack allocation routines. These are net new code and avoiding them when it is safe to do so helps reduce the overhead of stack probing. Implicit probes are "probes" that occur as a natural side effect of the existing code or guarantees provided by the ABI. They are essentially free and may allow the compiler to avoid some explicit probes. Examples of implicit probes include 1. ISA which pushes the return address onto the stack in a call instruction (x86) 2. ABI mandates that *sp always contain a backchain pointer (ppc) 3. Prologue stores a register into the stack. We exploit this on aarch64 and s390. On s390 register saves go into the caller's stack frame, on aarch64 register saves hit newly allocated space in the callee's frame. We can exploit both to avoid some explicit probing. I've done implementations for x86, ppc, aarch64 and s390 and the included tests have been checked against those targets ($arch-unknown-linux). This patch does not change the probing insn itself. We've had various discussions on-list on a better probe insn for x86. I think the consensus is to avoid read-modify-write insns. A testb may ultimately be best. This is IMHO an independent implementation detail for each target and should be handled as a follow-up. But if folks insist, it's a trivial change to make as it doesn't fundamentally affect how all this stuff works. Other targets that have an existing -fstack-check=specific, but for which I have not added a -fstack-check=clash implementation get partial protection against stack clash as well. This is a side effect of keeping some of the early code we'd hoped to use to avoid writing a new probe implementation for each target. -- To get a sense of overhead, just 1.5% of routines in glibc need probing in their prologues (x86) in the testing I performed. IIRC each and every one of those routines needed just 1-4 inlined probes. Significantly more functions need alloca space probed (IIRC ~5%), but given the amazingly inefficient alloca code, I can't believe anyone will ever notice the probing overhead. -- Patch #1 contains the new option -fstack-check=clash and some dejagnu infrastructure (most of which is unused until later patches) Patch #2 adds the new style probing support to the alloca/vla area and indirects uses of STACK_CHECK_PROTECT through get_stack_check_protect. Patch #3 Add some generic dumping support for use by the target prologue expanders Patch #4 introduces the x86 specific bits Patch #5 addresses combine-stack-adjustments interactions with -fstack-check=clash Patch #6 adds PPC support Patch #7 adds aarch64 support Patch #8 adds s390 support The patch series has been bootstrapped and regression tested on x86_64-linux-gnu ppc64-linux-gnu ppc64le-linux-gnu aarch64-linux-gnu s390x-linux-gnu (another respin of this is still in-progress) Additionally, each target has been bootstrapped with -fstack-check=clash enabled by default, the testsuite run and checked for glaring errors. Earlier versions have also bootstrapped on 32bit PPC and 32bit s390. Earlier versions have also been used to build and regression test. glibc-2.17 with -fstack-check=clash enabled by default. The resulting x86 and x86_64 libraries also were scanned to verify proper probing. Similarly for x86_64 builds with the trunk glibc. An earlie
[PATCH][RFA/RFC] Stack clash mitigation patch 01/08
This is the first patch in the stack-clash mitigation patches.
It introduces a new style of stack probing -fstack-check=clash,
including documentation of the new option, how it differs from
-fstack-check=specific, etc.
FWIW -fstack-check=specific is dreadfully named. I haven't tried to
address that.
It also introduces some dejagnu bits that are later used in tests. The
idea was to introduce dejagnu functions which describe aspects of the
target and have the tests adjust their expectations based on those
dejagnu functions rather than on a target name.
Finally, this patch introduces one new test of note. Some targets have
call instructions that store a return pointer into the stack and we take
advantage of that ISA feature to avoid some explicit probes.
This optimization is restricted to cases where the caller does not have
a frame of its own (because there's no reasonable way to tear that frame
down on the return path).
However, a sufficiently smart compiler could realize that a call to a
noreturn function could be converted into a jump, even if the caller has
a frame because that frame need not be torn down. Thus it would be
possible for a function calling a noreturn function to advance the stack
into the guard without actually touching the guard page, which breaks
the assumption that the call instruction would touch the guard
triggering a fault for that case.
GCC doesn't currently optimize that case for various reasons, but it
seemed prudent to go ahead and explicitly verify that with a test.
Thoughts? Ok for the trunk?
* common.opt (-fstack-check=clash): New option.
* flag-types.h (enum stack_check_type): Improve comments.
(STACK_CLASH_BUILTIN_STACK_CHECK): New stack_check_type.
* opts.c (common_handle_option): Handle -fstack-check=clash.
* doc/invoke.texi (-fstack-check=clash): Document new option.
(-fstack-check): Note additional problem with -fstack-check=generic.
Note differences between "clash" and "specific", fallbacks
and recommendations based on expected use.
testsuite/
* gcc.dg/stack-check-2.c: New test.
* lib/target-supports.exp
(check_effective_target_stack_clash_protected): New function.
(check_effective_target_frame_pointer_for_non_leaf): Likewise.
(check_effective_target_caller_implicit_probes): Likewise.
commit 018fffb569512eccd6b77410e28caa159009df5d
Author: Jeff Law
Date: Thu Jun 29 16:36:21 2017 -0400
Recongize new option + test of noreturn functions
Dejagnu primitivies for stack probe checking
diff --git a/gcc/common.opt b/gcc/common.opt
index e81165c..8eec29f 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2300,7 +2300,7 @@ Apply variable expansion when loops are unrolled.
fstack-check=
Common Report RejectNegative Joined
--fstack-check=[no|generic|specific]Insert stack checking code into the
program.
+-fstack-check=[no|generic|clash|specific] Insert stack checking code into
the program.
fstack-check
Common Alias(fstack-check=, specific, no)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 3e5cee8..e72f3e9 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -11324,8 +11324,9 @@ generation of code to ensure that they see the stack
being extended.
You can additionally specify a string parameter: @samp{no} means no
checking, @samp{generic} means force the use of old-style checking,
-@samp{specific} means use the best checking method and is equivalent
-to bare @option{-fstack-check}.
+@samp{clash} means use a checking method designed to prevent stack clash
+style attacks, @samp{specific} means use target specific
+checking methods and is equivalent to bare @option{-fstack-check}.
Old-style checking is a generic mechanism that requires no specific
target support in the compiler but comes with the following drawbacks:
@@ -11333,7 +11334,8 @@ target support in the compiler but comes with the
following drawbacks:
@enumerate
@item
Modified allocation strategy for large objects: they are always
-allocated dynamically if their size exceeds a fixed threshold.
+allocated dynamically if their size exceeds a fixed threshold. Note this
+may change the semantics of some code.
@item
Fixed limit on the size of the static frame of functions: when it is
@@ -11345,8 +11347,25 @@ Inefficiency: because of both the modified allocation
strategy and the
generic implementation, code performance is hampered.
@end enumerate
-Note that old-style stack checking is also the fallback method for
-@samp{specific} if no target support has been added in the compiler.
+Note that old-style stack checking is the fallback method for @samp{clash}
+and @samp{specific} if no target support for either of those has been added
+in the compiler.
+
+Also note that @samp{clash} requires target dependent prologue sequences that
+are different than @samp{specific}. However, some targets may use
+@samp{specific} style prologues if the
[PATCH][RFA/RFC] Stack clash mitigation patch 03/08
One of the painful aspects of all this code is the amount of target
dependent bits that have to be written and tested.
I didn't want to be scanning assembly code or RTL for prologues. Each
target would have to have its own scanner which was too painful to
contemplate.
So instead I settled on having a routine that the target dependent
prologue expanders could call to dump information about what they were
doing.
This greatly simplifies the testing side of things by having a standard
way to dump decisions. When combined with the dejagnu routines from
patch #1 which describe key attributes of the target's prologue
generation I can write tests in a fairly generic way.
This will be used by every target dependent prologue expander in this
series.
Comments/Questions? OK for the trunk?
* function.c (dump_stack_clash_frame_info): New function.
* function.h (dump_stack_clash_frame_info): Prototype.
(enum stack_clash_probes): New enum.
commit 3b09af4e78f3fdb40a913fbf99197a31315a47bc
Author: root
Date: Thu Jul 6 04:34:45 2017 -0400
Generic logging routines
diff --git a/gcc/function.c b/gcc/function.c
index f625489..d78a266 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -5695,6 +5695,58 @@ get_arg_pointer_save_area (void)
return ret;
}
+
+/* If debugging dumps are requested, dump infomation about how the
+ target handled -fstack-check=clash for the prologue.
+
+ PROBES describes what if any probes were emitted.
+
+ RESIDUALS indicates if the prologue had any residual allocation
+ (ie total allocation was not a multiple of PROBE_INTERVAL. */
+
+void
+dump_stack_clash_frame_info (enum stack_clash_probes probes, bool residuals)
+{
+ if (!dump_file)
+return;
+
+ switch (probes)
+{
+case NO_PROBE_NO_FRAME:
+ fprintf (dump_file,
+ "Stack clash no probe no stack adjustment in prologue.\n");
+ break;
+case NO_PROBE_SMALL_FRAME:
+ fprintf (dump_file,
+ "Stack clash no probe small stack adjustment in prologue.\n");
+ break;
+case PROBE_INLINE:
+ fprintf (dump_file, "Stack clash inline probes in prologue.\n");
+ break;
+case PROBE_LOOP:
+ fprintf (dump_file, "Stack clash probe loop in prologue.\n");
+ break;
+}
+
+ if (residuals)
+fprintf (dump_file, "Stack clash residual allocation in prologue.\n");
+ else
+fprintf (dump_file, "Stack clash no residual allocation in prologue.\n");
+
+ if (frame_pointer_needed)
+fprintf (dump_file, "Stack clash frame pointer needed.\n");
+ else
+fprintf (dump_file, "Stack clash no frame pointer needed.\n");
+
+ if (TREE_THIS_VOLATILE (cfun->decl))
+fprintf (dump_file,
+"Stack clash noreturn prologue, assuming no implicit"
+" probes in caller.\n");
+ else
+fprintf (dump_file,
+"Stack clash not noreturn prologue.\n");
+}
+
/* Add a list of INSNS to the hash HASHP, possibly allocating HASHP
for the first time. */
diff --git a/gcc/function.h b/gcc/function.h
index 0f34bcd..87dac80 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -553,6 +553,14 @@ do {
\
((TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn) \
? MAX (FUNCTION_BOUNDARY, 2 * BITS_PER_UNIT) : FUNCTION_BOUNDARY)
+enum stack_clash_probes {
+ NO_PROBE_NO_FRAME,
+ NO_PROBE_SMALL_FRAME,
+ PROBE_INLINE,
+ PROBE_LOOP
+};
+
+extern void dump_stack_clash_frame_info (enum stack_clash_probes, bool);
extern void push_function_context (void);
[PATCH][RFA/RFC] Stack clash mitigation patch 02/08
The key goal in this patch is to introduce the new probing style for
dynamically allocated stack space and indirect uses of
STACK_CHECK_PROTECT via get_stack_check_protect().
Those two changes accomplish two things. First it gives most targets
protection of dynamically allocated space (exceptions are targets which
expanders to allocate dynamic stack space such as ppc).
Second, targets which are not covered by -fstack-check=clash prologues
later, but which are covered by -fstack-check=specific get a fair amount
of protection.
We essentially vector into a totally different routine to allocate/probe
the dynamic stack space when -fstack-check=clash is active. It differs
from the existing routine is that it allocates PROBE_INTERVAL chunks and
probes them as they are allocated. The existing code would allocate the
entire space as a single hunk, then probe PROBE_INTERVAL chunks within
the hunk.
That routine is never presented with constant allocations on x86, but is
presented with constant allocations on other architectures. It will
optimize cases when it knows it does not need the loop or the residual
allocation after the loop. It does not have an unrolled loop mode, but
one could be added -- it didn't seem worth the effort.
The test will check that the loop is avoided for one case where it makes
sense. It does not check for avoiding the residual allocation, but it
could probably be made to do so.
The indirection for STACK_CHECK_PROTECT via get_stack_protect is worth
some further discussion as well.
Early in the development of the stack-clash mitigation patches we
thought we could get away with re-using much of the existing target code
for -stack-check=specific.
Essentially that code starts a probing loop at STACK_CHECK_PROTECT and
probes 2-3 pages beyond the current function's needs. The problem was
that starting at STACK_CHECK_PROTECT would skip probes in the first
couple pages leaving the code vulnerable.
So the idea was to avoid using STACK_CHECK_PROTECT directly. Instead we
would indirect through a new function (get_stack_check_protect) which
would return either 0 or STACK_CHECK_PROTECT depending on whether or not
we wanted -fstack-check=clash or -fstack-check=specific respectively.
That scheme works reasonably well. Except that it will tend to allocate
a large (larger than PROBE_INTERVAL) chunk of memory at once, then go
back and probe regions of PROBE_INTERVAL size. That introduces an
unfortunate race condition with asynch signals and also crashes valgrind
on ppc and aarch64.
Rather than throw that code away, it may still be valuable to those
targets with -fstack-check=specific support, but without
-fstack-check=clash support. So I'm including it here.
Thoughts/comments? Ok for the trunk?
* defaults.h (STACK_CHECK_MOVING_SP): Enable with -fstack-check=clash
* explow.c (anti_adjust_stack_and_probe_stack_clash): New function.
(get_stack_check_protect): Likewise.
(allocate_dynamic_stack_space): Use new functions.
* rtl.h (get_stack_check_protect): Prototype.
* config/aarch64/aarch64.c (aarch64_expand_prologue): Use
get_stack_check_protect.
* config/alpha/alpha.c (alpha_expand_prologue): Likewise.
* config/arm/arm.c (arm_expand_prologue): Likewise.
* config/i386/i386.c (ix86_expand_prologue): Likewise.
* config/ia64/ia64.c (ia64_expand_prologue): Likewise.
* conifg/mips/mips.c (mips_expand_prologue): Likewise.
* config/powerpcspe/powerpcspe.c (rs6000_emit_prologue): Likewise.
* config/rs6000/rs6000.c (rs6000_emit_prologue): Likewise.
* config/sparc/sparc.c (sparc_expand_prologue): Likewise.
testsuite
* gcc.dg/stack-check-3.c: New test.
commit cddc77979e4183769e1817676c3b449d8c8cb202
Author: Jeff Law
Date: Wed Jun 28 14:02:16 2017 -0400
Use stack-clash probing if requested for alloca space
Use MOVING_SP by default for stack-clash probing
Simple test for dynamic allocations + probing
stack-check-3 fixes for improvements in dynamic area probe loop elision
use dg-requires... do not optimizing sibling calls
Indirect for STACK_CHECK_PROTECT
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index ef1b5a8..0a8b40a 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3676,12 +3676,14 @@ aarch64_expand_prologue (void)
{
if (crtl->is_leaf && !cfun->calls_alloca)
{
- if (frame_size > PROBE_INTERVAL && frame_size > STACK_CHECK_PROTECT)
- aarch64_emit_probe_stack_range (STACK_CHECK_PROTECT,
- frame_size - STACK_CHECK_PROTECT);
+ if (frame_size > PROBE_INTERVAL
+ && frame_size > get_stack_check_protect ())
+ aarch64_emit_probe_stack_range (get_stack_check_protect (),
+ (frame_size
+-
[PATCH][RFA/RFC] Stack clash mitigation patch 05/08
The prior patch introduced -fstack-check=clash prologues for the x86.
And yet we still saw large allocations in our testing.
It turns out combine-stack-adjustments would take
allocate PROBE_INTERVAL
probe
allocate PROBE_INTERVAL
probe
allocate PROBE_INTERVAL
probe
allocate RESIDUAL
And turn that into
allocate (3 * PROBE_INTERVAL) + residual
probe
probe
probe
Adjusting the address of the probes appropriately. Ugh.
This patch introduces a new note that the backend can attach to a stack
adjustment which essentially tells c-s-a to not merge it into other
adjustments. THere's an x86 specific test to verify behavior.
Comments/Questions? Ok for the trunk?
* combine-stack-adj.c (combine_stack_adjustments_for_block): Do
nothing for stack adjustments with REG_STACK_CHECK.
* config/i386/i386.c (pro_epilogue_adjust_stack): Return insn.
(ix86_adjust_satck_and_probe_stack_clash): Add REG_STACK_NOTEs.
* reg-notes.def (STACK_CHECK): New note.
testsuite/
* gcc.target/i386/stack-check-11.c: New test.
commit f363b876ccbbc584db85510cd24b80349fcd8260
Author: Jeff Law
Date: Wed Jun 28 12:36:49 2017 -0400
Don't combine adjustments for stck probing
diff --git a/gcc/combine-stack-adj.c b/gcc/combine-stack-adj.c
index 9ec14a3..82d6dba 100644
--- a/gcc/combine-stack-adj.c
+++ b/gcc/combine-stack-adj.c
@@ -508,6 +508,8 @@ combine_stack_adjustments_for_block (basic_block bb)
continue;
set = single_set_for_csa (insn);
+ if (set && find_reg_note (insn, REG_STACK_CHECK, NULL_RTX))
+ set = NULL_RTX;
if (set)
{
rtx dest = SET_DEST (set);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 7098f74..a737300 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -13405,7 +13405,7 @@ ix86_add_queued_cfa_restore_notes (rtx insn)
zero if %r11 register is live and cannot be freely used and positive
otherwise. */
-static void
+static rtx
pro_epilogue_adjust_stack (rtx dest, rtx src, rtx offset,
int style, bool set_cfa)
{
@@ -13496,6 +13496,7 @@ pro_epilogue_adjust_stack (rtx dest, rtx src, rtx
offset,
m->fs.sp_valid = valid;
m->fs.sp_realigned = realigned;
}
+ return insn;
}
/* Find an available register to be used as dynamic realign argument
@@ -13837,9 +13838,11 @@ ix86_adjust_stack_and_probe_stack_clash (const
HOST_WIDE_INT size)
for (i = PROBE_INTERVAL; i <= size; i += PROBE_INTERVAL)
{
/* Allocate PROBE_INTERVAL bytes. */
- pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
+ rtx insn
+ = pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
GEN_INT (-PROBE_INTERVAL), -1,
m->fs.cfa_reg == stack_pointer_rtx);
+ add_reg_note (insn, REG_STACK_CHECK, const0_rtx);
/* And probe at *sp. */
emit_stack_probe (stack_pointer_rtx);
diff --git a/gcc/reg-notes.def b/gcc/reg-notes.def
index 8734d26..18cf7e3 100644
--- a/gcc/reg-notes.def
+++ b/gcc/reg-notes.def
@@ -223,6 +223,10 @@ REG_NOTE (ARGS_SIZE)
pseudo reg. */
REG_NOTE (RETURNED)
+/* Indicates the instruction is a stack check probe that should not
+ be combined with other stack adjustments. */
+REG_NOTE (STACK_CHECK)
+
/* Used to mark a call with the function decl called by the call.
The decl might not be available in the call due to splitting of the call
insn. This note is a SYMBOL_REF. */
diff --git a/gcc/testsuite/gcc.target/i386/stack-check-11.c
b/gcc/testsuite/gcc.target/i386/stack-check-11.c
new file mode 100644
index 000..c17b8c6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/stack-check-11.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fstack-check=clash" } */
+
+extern void arf (unsigned long int *, unsigned long int *);
+void
+frob ()
+{
+ unsigned long int num[859];
+ unsigned long int den[859];
+ arf (den, num);
+}
+
+/* { dg-final { scan-assembler-times "subq" 4 } } */
+/* { dg-final { scan-assembler-times "orq" 3 } } */
+
[PATCH][RFA/RFC] Stack clash mitigation patch 04/08
This patch introduces x86 target specific bits to mitigate stack clash
attacks.
The key differences relative to the -fstack-check=specific expander are
that it never allocates more than PROBE_INTERVAL bytes at a time, it
probes into each allocated hunk immediately after allocation, and it
exploits the fact that a call instruction generates an implicit probe at
*sp for the callee and uses that fact to avoid many explicit probes.
The highlights:
1. If the size of the local frame is < PROBE_INTERVAL, then no
probing is needed.
2. For up to 4 * PROBE_INTERVAL sized frames the allocation and
probe are emitted inline.
3. Anything larger we implement as a allocate/probe loop where
each iteration handles a region of PROBE_INTERVAL size.
4. Residuals need not be probed.
5. CFIs should be correct, even for the loop case.
6. Introduces several new tests. Many of which are used unmodified
on ppc, aarch64 and s390 later.
This implementation should be very efficient for the most common cases.
Comments/Questions? OK for the trunk?
* config/i386/i386.c (ix86_adjust_stack_and_probe_stack_clash): New.
(ix86_expand_prologue): Dump stack clash info as needed.
Call ix86_adjust_stack_and_probe_stack_clash as needed.
testsuite/
* gcc.dg/stack-check-4.c: New test.
* gcc.dg/stack-check-5.c: New test.
* gcc.dg/stack-check-6.c: New test.
* gcc.dg/stack-check-7.c: New test.
* gcc.dg/stack-check-8.c: New test.
* gcc.dg/stack-check-9.c: New test.
* gcc.dg/stack-check-10.c: New test.
commit e8fae56a2362c2952f735ce515c84de970efc582
Author: Jeff Law
Date: Thu Jun 29 16:36:51 2017 -0400
X86 bits
and test for noreturn callee
Add more logging to x86 code
More tests
use dg-requires
do not optimizing sibling calls
Florian's tests
Add suitable regexps for targets that have implicit probes, but must
allocate
small amounts of stack in non-leaf functions.
Add -Wno-psabi to stack-check-8
Do not optimizing sibling calls
use dg-requires
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 0947b3c..7098f74 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -13779,6 +13779,140 @@ release_scratch_register_on_entry (struct scratch_reg
*sr)
#define PROBE_INTERVAL (1 << STACK_CHECK_PROBE_INTERVAL_EXP)
+/* Emit code to adjust the stack pointer by SIZE bytes while probing it.
+
+ This differs from the next routine in that it tries hard to prevent
+ attacks that jump the stack guard. Thus it is never allowed to allocate
+ more than PROBE_INTERVAL bytes of stack space without a suitable
+ probe. */
+
+static void
+ix86_adjust_stack_and_probe_stack_clash (const HOST_WIDE_INT size)
+{
+ struct machine_function *m = cfun->machine;
+
+ /* If this function does not statically allocate stack space, then
+ no probes are needed. */
+ if (!size)
+{
+ dump_stack_clash_frame_info (NO_PROBE_NO_FRAME, false);
+ return;
+}
+
+ /* If we are a noreturn function, then we have to consider the
+ possibility that we're called via a jump rather than a call.
+
+ Thus we don't have the implicit probe generated by saving the
+ return address into the stack at the call. Thus, the stack
+ pointer could be anywhere in the guard page. The safe thing
+ to do is emit a probe now.
+
+ ?!? This should be revamped to work like aarch64 and s390 where
+ we track the offset from the most recent probe. Normally that
+ offset would be zero. For a non-return function we would reset
+ it to PROBE_INTERVAL - (STACK_BOUNDARY / BITS_PER_UNIT). Then
+ we just probe when we cross PROBE_INTERVAL. */
+ if (TREE_THIS_VOLATILE (cfun->decl))
+emit_stack_probe (plus_constant (Pmode, stack_pointer_rtx,
+-GET_MODE_SIZE (word_mode)));
+
+ /* If we allocate less than PROBE_INTERVAL bytes statically,
+ then no probing is necessary, but we do need to allocate
+ the stack. */
+ if (size < PROBE_INTERVAL)
+{
+ pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
+GEN_INT (-size), -1,
+m->fs.cfa_reg == stack_pointer_rtx);
+ dump_stack_clash_frame_info (NO_PROBE_SMALL_FRAME, true);
+ return;
+}
+
+ /* We're allocating a large enough stack frame that we need to
+ emit probes. Either emit them inline or in a loop depending
+ on the size. */
+ if (size <= 4 * PROBE_INTERVAL)
+{
+ HOST_WIDE_INT i;
+ for (i = PROBE_INTERVAL; i <= size; i += PROBE_INTERVAL)
+ {
+ /* Allocate PROBE_INTERVAL bytes. */
+ pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
+GEN_INT (-PROBE_INTERVAL), -1,
+m->fs.cfa_reg == stack_pointer_rtx);
+
+
[PATCH] [RFA/RFC] Stack clash mitigation patch 06/08
This patch introduces -fstack-check=clash prologue support for the PPC.
PPC is interesting in that its ABIs requires *sp to always contain the
backchain. That implicit probe is very useful in eliminating many
explicit probes. In fact, from the standpoint of avoiding explicit
probes it's probably the most ideal situation.
We honor the requirement that store-with-base-register-modification
instructions are the only way to allocate stack in the new code. This
means we have to keep a copy of the backchain handy as a source operand
for that instruction. This implies a special case for a stack frame of
precisely PROBE_INTERVAL bytes, which need not copy the backchain into a
temporary.
I'm pretty sure the CFIs are not right for the loop case.
We select between two probing loop styles for the probe_stack_range insn.
The PPC port also has its own insn to allocate dynamic stack space. So
there's a chunk of code in that expander to handle -fstack-check=clash.
You might think there's refactoring opportunities in here. THe only
potential one would be the insn to allocate dynamic stack space and the
probe_stack_range insn. But it wasn't obvious to me how to refactor
them in a reasonable way.
You'll note there are no new tests -- the tests added in patch #5 are
used by the PPC port as-is.
Comments/Questions? Ok for the trunk?
* config/rs6000/rs6000-protos.h (output_probe_stack_range): Update
prototype for new argument.
* config/rs6000/rs6000.c (wrap_frame_mem): New function extracted
from rs6000_emit_allocate_stack.
(PROBE_INTERVAL): Define.
(handle_residual): New function.
(rs6000_emit_probe_stack_range_stack_clash): New function.
(rs6000_emit_allocate_stack): Use wrap_frame_mem.
Call rs6000_emit_probe_stack_range_stack_clash as needed.
(rs6000_emit_probe_stack_range): Add additional argument
to call to gen_probe_stack_range{si,di}.
(output_probe_stack_range): New.
(output_probe_stack_range_1): Renamed from output_probe_stack_range.
(output_probe_stack_range_stack_clash): New.
(rs6000_emit_prologue): Emit notes into dump file as requested.
* rs6000.md (allocate_stack): Handle -fstack-check=probe.
(probe_stack_range): Operand 0 is now early-clobbered.
Add additional operand and pass it to output_probe_stack_range.
commit 5d33b5b1b8a8f7b7f8d86cd8f5c27edaff9afb58
Author: root
Date: Thu Jul 6 05:08:52 2017 -0400
rs6000 stack probing
diff --git a/gcc/config/rs6000/rs6000-protos.h
b/gcc/config/rs6000/rs6000-protos.h
index aeec9b2..451c442 100644
--- a/gcc/config/rs6000/rs6000-protos.h
+++ b/gcc/config/rs6000/rs6000-protos.h
@@ -134,7 +134,7 @@ extern void rs6000_emit_sISEL (machine_mode, rtx[]);
extern void rs6000_emit_sCOND (machine_mode, rtx[]);
extern void rs6000_emit_cbranch (machine_mode, rtx[]);
extern char * output_cbranch (rtx, const char *, int, rtx_insn *);
-extern const char * output_probe_stack_range (rtx, rtx);
+extern const char * output_probe_stack_range (rtx, rtx, rtx);
extern void rs6000_emit_dot_insn (rtx dst, rtx src, int dot, rtx ccreg);
extern bool rs6000_emit_set_const (rtx, rtx);
extern int rs6000_emit_cmove (rtx, rtx, rtx, rtx);
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index aa70e30..1d0d254 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -25618,6 +25618,213 @@ rs6000_emit_stack_tie (rtx fp, bool hard_frame_needed)
emit_insn (gen_stack_tie (gen_rtx_PARALLEL (VOIDmode, p)));
}
+/* INSN allocates SIZE bytes on the stack (STACK_REG) using a store
+ with update style insn.
+
+ Set INSN's alias set/attributes and add suitable flags and notes
+ for the dwarf CFI machinery. */
+static void
+wrap_frame_mem (rtx insn, rtx stack_reg, HOST_WIDE_INT size)
+{
+ rtx par = PATTERN (insn);
+ gcc_assert (GET_CODE (par) == PARALLEL);
+ rtx set = XVECEXP (par, 0, 0);
+ gcc_assert (GET_CODE (set) == SET);
+ rtx mem = SET_DEST (set);
+ gcc_assert (MEM_P (mem));
+ MEM_NOTRAP_P (mem) = 1;
+ set_mem_alias_set (mem, get_frame_alias_set ());
+
+ RTX_FRAME_RELATED_P (insn) = 1;
+ add_reg_note (insn, REG_FRAME_RELATED_EXPR,
+ gen_rtx_SET (stack_reg, gen_rtx_PLUS (Pmode, stack_reg,
+ GEN_INT (-size;
+}
+
+#define PROBE_INTERVAL (1 << STACK_CHECK_PROBE_INTERVAL_EXP)
+
+#if PROBE_INTERVAL > 32768
+#error Cannot use indexed addressing mode for stack probing
+#endif
+
+/* Allocate ROUNDED_SIZE - ORIG_SIZE bytes on the stack, storing
+ ORIG_SP into *sp after the allocation.
+
+ ROUNDED_SIZE will be a multiple of PROBE_INTERVAL and
+ ORIG_SIZE - ROUNDED_SIZE will be less than PROBE_INTERVAL.
+
+ Return the insn that allocates the residual space. */
+static rtx_insn *
+handle_residual (HOST_WIDE_INT orig_size,
+HOST_WIDE_INT rounded_size,
+rtx orig_sp)
+{
+ /* Allocate (a
[PATCH][RFA/RFC] Stack clash mitigation patch 07/08
This patch introduces aarch64 -fstack-check=clash prologue support.
aarch64 is the first target that does not have any implicit probes in
the caller. Thus at prologue entry it must make conservative
assumptions about the offset of the most recent probed address relative
to the stack pointer.
However, it can and does use stores into the stack generated by the
prologue to avoid some explicit probes. It's not as efficient as x86 or
ppc, but it does try hard to be efficient.
We track the offset of the most recent probe relative to the stack
pointer. The initial state is PROBE_INTERVAL - (STACK_BOUNDARY /
BITS_PER_UNIT) bytes away. Allocations increase that value. Stores
decrease the value.
When the value crosses PROBE_INTERVAL we emit an explicit probe.
aarch64 prologues can allocate space in 3 different places. There's
INITIAL_ADJUST and FINAL_ADJUST which are simple adjustments and they're
trivial to handle. We just add the adjustment to the current offset and
if we cross PROBE_INTERVAL, we emit a probe before the stack adjustment.
Handling of CALLEE_ADJUST is more subtle because its two actions in one
instruction. The instruction both allocates space and stores into two
words at the lowest address in the allocated space.
Its easiest to mentally model the instruction as two distinct operations.
First the instruction adjusts the stack by CALLEE_ADJUST - 2 * WORD_SIZE
and thus increases the offset to the most recent probe. If the offset
to the most recent probe crosses PROBE_INTERVAL, then we must emit a
probe prior to the CALLEE_ADJUST instruction. Note that CALLEE_ADJUST
might be 0 or any multiple of 16. For 0/16 this step is a nop.
Second the CALLEE_ADJUST instruction adjusts the stack by 2 * WORD_SIZE
and stores into both just allocated words. We never need a probe for
this action. Furthermore, this action always zeros the offset to the
most recent probe which is very helpful in avoiding subsequent explicit
probes.
Between CALLEE_ADJUST and FINAL_ADJUST are other register saves. We
track the offsets for those and decrease the offset to the most recent
probe appropriately.
I'm pretty sure this implementation is about as good as it can get with
the ISA/ABI in place on aarch64.
I'm pretty sure that the loop to allocate large stacks does not update
CFIs correctly.
Again, you'll note that like ppc, this does not add any new tests. It
uses the tests that were added with the x86 support unmodifed.
Thoughts/Comments? OK for the trunk?
* config/aarch/aarch64.c (output_probe_stack_range): Handle
-fstack-check=clash probing too.
(aarch64_save_callee_saves): Return smallest offset from SP
that was written.
(aarch64_allocate_and_probe_stack_space): New function.
(aarch64_expand_prologue): Track distance between SP and most
recent probe. Use aarch64_allocate_and_probe_stack_space
when -fstack-check=clash rather than just adjusting sp.
Dump actions via dump_stack_clash_frame_info.
Handle probing and probe offset update for CALLEE_ADJUST space.
Use return value from aarch64_save_callee_saves to reduce
last_probe_offset.
commit 86871749daad396cf825b3982dee8e80eda96d8a
Author: root
Date: Fri Jul 7 11:17:06 2017 -0400
aarch64 support with dumping, adjust tests
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 0a8b40a..40bc183 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2830,6 +2830,9 @@ aarch64_output_probe_stack_range (rtx reg1, rtx reg2)
char loop_lab[32];
rtx xops[2];
+ if (flag_stack_check == STACK_CLASH_BUILTIN_STACK_CHECK)
+reg1 = stack_pointer_rtx;
+
ASM_GENERATE_INTERNAL_LABEL (loop_lab, "LPSRL", labelno++);
/* Loop. */
@@ -2841,7 +2844,14 @@ aarch64_output_probe_stack_range (rtx reg1, rtx reg2)
output_asm_insn ("sub\t%0, %0, %1", xops);
/* Probe at TEST_ADDR. */
- output_asm_insn ("str\txzr, [%0]", xops);
+ if (flag_stack_check == STACK_CLASH_BUILTIN_STACK_CHECK)
+{
+ gcc_assert (xops[0] == stack_pointer_rtx);
+ xops[1] = GEN_INT (PROBE_INTERVAL - 8);
+ output_asm_insn ("str\txzr, [%0, %1]", xops);
+}
+ else
+output_asm_insn ("str\txzr, [%0]", xops);
/* Test if TEST_ADDR == LAST_ADDR. */
xops[1] = reg2;
@@ -3245,9 +3255,11 @@ aarch64_return_address_signing_enabled (void)
/* Emit code to save the callee-saved registers from register number START
to LIMIT to the stack at the location starting at offset START_OFFSET,
- skipping any write-back candidates if SKIP_WB is true. */
+ skipping any write-back candidates if SKIP_WB is true.
-static void
+ Return the smallest offset from SP that is written. */
+
+static HOST_WIDE_INT
aarch64_save_callee_saves (machine_mode mode, HOST_WIDE_INT start_offset,
unsigned start, unsigned limit, bool skip_wb)
{
@@ -3257,6 +3269,8 @@ aarch64_save_callee_saves (machine_mod
[PATCH][RFA/RFC] Stack clash mitigation patch 08/08
This patch adds s390 support for stack-clash mitigation.
s390's most interesting property is that the caller allocates space for
the callee to save registers into.
So much like aarch64, we start with a very conservative assumption about
the offset between SP and the most recent stack probe. As we encounter
those register saves we may be able to decrease that offset. And like
aarch64 as we allocate space, the offset increases. If the offset
crosses PROBE_INTERVAL, we must emit probes.
Because the register saves hit the caller's frame s390 in some ways
generates code more like x86/ppc. Though if there aren't any register
saves, then the resulting code looks more like aarch64.
For large frames, I did not implement an allocate/probe in a loop.
Someone with a better understanding of the architecture is better suited
for that work. I'll note that you're going to need another scratch
register :-) This is the cause of the xfail of one test which expects
to see a prologue allocate/probe loop.
s390 has a -mbackchain option. I'm not sure where it's used, but we do
try to handle it in the initial offset computation. However, we don't
handle it in the actual allocations that occur when -fstack-check=clash
is enabled.
s390 does not have a -fstack-check=specific implementation. I have not
tried to add one. But I have defined STACK_CHECK_STATIC_BUILTIN. I
haven't investigated what side effects that might have.
Other than the xfail noted above, the s390 uses the same tests as the
x86, ppc and aarch64 ports.
I suspect we're going to need further iteration here.
Thoughts/Comments?
Jeff
* config/s390/s390.c (PROBE_INTERVAL): Define.
(allocate_stack_space): New function, partially extracted from
s390_emit_prologue.
(s390_emit_prologue): Track offset to most recent stack probe.
Code to allocate space moved into allocate_stack_space.
Dump actions when no stack is allocated.
* config/s390/s390.h (STACK_CHECK_STATIC_BUILTIN): Define.
testsuite/
* gcc.dg/stack-check-6.c: xfail for s390*-*-*.
commit 56523059d48f55991e7607dbde248f2aabe3e7e3
Author: Jeff Law
Date: Fri Jul 7 17:25:35 2017 +
S390 implementatoin
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 958ee3b..cddb393 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -10999,6 +10999,107 @@ pass_s390_early_mach::execute (function *fun)
} // anon namespace
+#define PROBE_INTERVAL (1 << STACK_CHECK_PROBE_INTERVAL_EXP)
+
+/* Allocate SIZE bytes of stack space, using TEMP_REG as a temporary
+ if necessary. LAST_PROBE_OFFSET contains the offset of the closest
+ probe relative to the stack pointer.
+
+ Note that SIZE is negative.
+
+ TMP_REG_IS_LIVE indicates that TEMP_REG actually holds a live
+ value and must be restored if we clobber it. */
+static void
+allocate_stack_space (rtx size, HOST_WIDE_INT last_probe_offset,
+ rtx temp_reg, bool tmp_reg_is_live)
+{
+ rtx insn;
+
+ /* If we are emitting stack probes and a SIZE allocation would cross
+ the PROBE_INTERVAL boundary, then we need significantly different
+ sequences to allocate and probe the stack. */
+ if (flag_stack_check == STACK_CLASH_BUILTIN_STACK_CHECK
+ && last_probe_offset + -INTVAL (size) < PROBE_INTERVAL)
+dump_stack_clash_frame_info (NO_PROBE_SMALL_FRAME, true);
+ else if (flag_stack_check == STACK_CLASH_BUILTIN_STACK_CHECK
+ && last_probe_offset + -INTVAL (size) >= PROBE_INTERVAL)
+{
+ rtx memref;
+
+ HOST_WIDE_INT rounded_size = -INTVAL (size) & -PROBE_INTERVAL;
+
+ emit_move_insn (temp_reg, GEN_INT (PROBE_INTERVAL - 8));
+
+ /* We really should have a runtime loop version as well. */
+ for (unsigned int i = 0; i < rounded_size; i += PROBE_INTERVAL)
+ {
+ insn = emit_insn (gen_add2_insn (stack_pointer_rtx,
+ GEN_INT (-PROBE_INTERVAL)));
+ RTX_FRAME_RELATED_P (insn);
+
+ /* We just allocated PROBE_INTERVAL bytes of stack space. Thus,
+a probe is mandatory here, but LAST_PROBE_OFFSET does not
+change. */
+ memref = gen_rtx_MEM (Pmode, gen_rtx_PLUS (Pmode, temp_reg,
+stack_pointer_rtx));
+ MEM_VOLATILE_P (memref);
+ emit_move_insn (memref, temp_reg);
+ }
+
+ /* Handle any residual allocation request. */
+ HOST_WIDE_INT residual = -INTVAL (size) - rounded_size;
+ insn = emit_insn (gen_add2_insn (stack_pointer_rtx,
+ GEN_INT (-residual)));
+ RTX_FRAME_RELATED_P (insn) = 1;
+ last_probe_offset += residual;
+ if (last_probe_offset >= PROBE_INTERVAL)
+ {
+ emit_move_insn (temp_reg, GEN_INT (residual
+- GET_MODE_SIZE (word_mode)));
+ memref = gen_rtx_MEM (Pmode, gen_rtx_PLUS
Re: A potential bug in lra-constraints.c for special_memory_constraint?
> we need to generate misaligned load/store insns ONLY for misaligned memory > access, therefore need a new constraints for misaligned address. Why? What happens exactly if the memory access turns out to be aligned? -- Eric Botcazou
Re: [PATCH] [RFA/RFC] Stack clash mitigation patch 06/08
Andrew, this seems like the sort of rs6000 patch likely to be relevant to the powerpcspe port. (Of course all rs6000 patches since the ports separated need to be monitored to spot such patches that need merging as they go in.) -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] [RFA/RFC] Stack clash mitigation patch 06/08
On 07/11/2017 03:51 PM, Joseph Myers wrote: > Andrew, this seems like the sort of rs6000 patch likely to be relevant to > the powerpcspe port. (Of course all rs6000 patches since the ports > separated need to be monitored to spot such patches that need merging as > they go in.) Yes. My hope would be that we get feedback from the ppc maintainers, iterate on the implementation and when satisfied Andrew could take them as-is and use them on spe. jeff
Backports to gcc 6.x
I would like to backport the following patches to the GCC 6 branch. PR9: Fix failure of gcc.dg/loop-8.c on Power https://gcc.gnu.org/ml/gcc-patches/2017-01/msg01788.html PR68972: g++.dg/cpp1y/vla-initlist1.C test case fails on power https://gcc.gnu.org/ml/gcc-patches/2017-02/msg00541.html Handle conflicting target options -mno-power9-vector and -mcpu=power9 https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01192.html PR80103: Fix ICE with cross compiler https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01335.html PR80101: Fix ICE in store_data_bypass_p https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00953.html Each of these patches has been bootstrapped and regression tested on the GCC 6 branch. In backport, patch PR80103 omits certain changes to existing comments that are not present in GCC6. Are these patches ok for backporting to GCC 6?
Re: A potential bug in lra-constraints.c for special_memory_constraint?
> On Jul 11, 2017, at 4:24 PM, Eric Botcazou wrote: > >> we need to generate misaligned load/store insns ONLY for misaligned memory >> access, therefore need a new constraints for misaligned address. > > Why? What happens exactly if the memory access turns out to be aligned? we add this new constraint as: ;; We need a special memory constraint for the misaligned memory access ;; This is only for TARGET_MISALIGN target (define_special_memory_constraint "B" "Memory reference whose address is misaligned" (and (match_code "mem") (match_test "TARGET_MISALIGN") (match_test "memory_is_misaligned (op, mode)”))) the routine “memory_is_misaligned” is a compile-time check to see whether the address is known to be misaligned or not. only for compile-time KNOWN misaligned memory access, we will use misaligned load/store insns provided by the new processor for the memory access. and then put this new constraints to sparc.md as: (define_insn "*movdi_insn_sp64" [(set (match_operand:DI 0 "nonimmediate_operand" "=r,r,r,r, B, m, r,*e,?*e,?*e,?W,b,b") (match_operand:DI 1 "input_operand""rI,N,B,m,rJ,rJ,*e, r, *e, W,*e,J,P"))] NOTE, the 4th constraints for this insn is “B, rJ”, if the operands match this constraint, then. misaligned store insns will be generated for the misaligned memory access instead of regular store. misaligned insns will NOT be used for memory access whose alignment cannot be decided to be misaligned during compilation time. Hope this is clear. If I still don’t answer your question, please let me know. Qing > > -- > Eric Botcazou
Re: A potential bug in lra-constraints.c for special_memory_constraint?
> we add this new constraint as: > > ;; We need a special memory constraint for the misaligned memory access > ;; This is only for TARGET_MISALIGN target > (define_special_memory_constraint "B" > "Memory reference whose address is misaligned" > (and (match_code "mem") > (match_test "TARGET_MISALIGN") > (match_test "memory_is_misaligned (op, mode)”))) > > the routine “memory_is_misaligned” is a compile-time check to see whether > the address is known to be misaligned or not. only for compile-time KNOWN > misaligned memory access, we will use misaligned load/store insns provided > by the new processor for the memory access. > > and then put this new constraints to sparc.md as: > > (define_insn "*movdi_insn_sp64" > [(set (match_operand:DI 0 "nonimmediate_operand" "=r,r,r,r, B, m, > r,*e,?*e,?*e,?W,b,b") (match_operand:DI 1 "input_operand" > "rI,N,B,m,rJ,rJ,*e, r, *e, W,*e,J,P"))] > > > NOTE, the 4th constraints for this insn is “B, rJ”, if the operands match > this constraint, then. misaligned store insns will be generated for the > misaligned memory access instead of regular store. OK, but what happens in the end? What's the failure mode? Internal compiler error, impossible reloading, wrong code, suboptimal code, etc? -- Eric Botcazou
[PATCH][RFA/RFC] Stack clash mitigation patch 07/08
Jeff Law wrote: > aarch64 is the first target that does not have any implicit probes in > the caller. Thus at prologue entry it must make conservative > assumptions about the offset of the most recent probed address relative > to the stack pointer. No - like I mentioned before that's not correct nor acceptable as it would imply that ~70% of functions need a probe at entry. I did a quick run across SPEC and found the outgoing argument size is > 1024 in just 9 functions out of 147000! Those 9 were odd special cases due to auto generated code to interface between C and Fortran. This is extremely unlikely to occur anywhere else. So even assuming an unchecked caller, large outgoing arguments are simply not a realistic threat. Therefore even when using a tiny 4K probe size we can safely adjust SP by 3KB before needing an explicit probe - now only 0.6% of functions need a probe. If we choose a proper minimum probe distance, say 64KB, explicit probes are basically non-existent (just 35 functions, or ~0.02% of all functions are > 64KB). Clearly inserting probes can be the default as the impact on code quality is negligible. With respect to implementation it is relatively easy to decide in aarch64_layout_frame which frames need probes and where. I don't think keeping a running offset of the last probe/store is useful, it'll just lead to inefficiencies and bugs. The patch doesn't deal with the delayed stores due to shrinkwrapping for example. Inserting probes before the prolog would be easier, eg. sub tmp, sp, 65536 str xzr, [tmp, 1024] // allow up to 1KB of outgoing arguments in callee sub tmp, sp, 131072 str xzr, [tmp, 1024] ... normal prolog for frame size 128-192KB Wilco
Re: A potential bug in lra-constraints.c for special_memory_constraint?
From: Eric Botcazou Date: Wed, 12 Jul 2017 01:19:03 +0200 >> we add this new constraint as: >> >> ;; We need a special memory constraint for the misaligned memory access >> ;; This is only for TARGET_MISALIGN target >> (define_special_memory_constraint "B" >> "Memory reference whose address is misaligned" >> (and (match_code "mem") >> (match_test "TARGET_MISALIGN") >> (match_test "memory_is_misaligned (op, mode)”))) >> >> the routine “memory_is_misaligned” is a compile-time check to see whether >> the address is known to be misaligned or not. only for compile-time KNOWN >> misaligned memory access, we will use misaligned load/store insns provided >> by the new processor for the memory access. >> >> and then put this new constraints to sparc.md as: >> >> (define_insn "*movdi_insn_sp64" >> [(set (match_operand:DI 0 "nonimmediate_operand" "=r,r,r,r, B, m, >> r,*e,?*e,?*e,?W,b,b") (match_operand:DI 1 "input_operand" >> "rI,N,B,m,rJ,rJ,*e, r, *e, W,*e,J,P"))] >> >> >> NOTE, the 4th constraints for this insn is “B, rJ”, if the operands match >> this constraint, then. misaligned store insns will be generated for the >> misaligned memory access instead of regular store. > > OK, but what happens in the end? What's the failure mode? Internal compiler > error, impossible reloading, wrong code, suboptimal code, etc? Yeah I'm still hopelessly confused what the problem is too. As far as I understand it, the unaligned loads and stores present in the M8 can be used just fine on aligned data. It's just not efficient (11 cycle latency instead of 3). Perhaps they are just trying to only match the constraints for the the unaligned loads and stores when absolutely necessary. If so, I really really wish they had said this from the beginning. :)
Re: [PATCH][RFA/RFC] Stack clash mitigation patch 07/08
On 07/11/2017 06:20 PM, Wilco Dijkstra wrote: > Jeff Law wrote: >> aarch64 is the first target that does not have any implicit probes in >> the caller. Thus at prologue entry it must make conservative >> assumptions about the offset of the most recent probed address relative >> to the stack pointer. > > No - like I mentioned before that's not correct nor acceptable as it would > imply > that ~70% of functions need a probe at entry. I did a quick run across SPEC > and > found the outgoing argument size is > 1024 in just 9 functions out of 147000! > Those 9 were odd special cases due to auto generated code to interface between > C and Fortran. This is extremely unlikely to occur anywhere else. So even > assuming > an unchecked caller, large outgoing arguments are simply not a realistic > threat. Whether or not such frames exist in SPEC isn't the question. THere's nothing in the ABI or ISA that allows us to avoid those probes without compromising security. Mixed code scenarios are going to be a fact of life, probably forever unless we can convince every ISV providing software that works on top of RHEL/*BSD/whatever to turn on probing (based on my experiences, that has exactly a zero chance of occurring). In a mixed code scenario a caller may have a large alloca or large outgoing argument area that pushes the stack pointer into the guard page without actually accessing the guard page. That requires a callee which is compiled with stack checking to make worst case assumptions at function entry to protect itself as much as possible from these attacks. THe aarch64 maintainers can certain nix what I've done or modify it in ways that suit them. That is their choice as port maintainers. However, Red Hat will have to evaluate the cost of reducing security for our customer base against the performance improvement of such changes. As I've said before, I do not know where that decision would fall. > > Therefore even when using a tiny 4K probe size we can safely adjust SP by 3KB > before needing an explicit probe - now only 0.6% of functions need a probe. > If we choose a proper minimum probe distance, say 64KB, explicit probes are > basically non-existent (just 35 functions, or ~0.02% of all functions are > > 64KB). > Clearly inserting probes can be the default as the impact on code quality is > negligible. Again, there's nothing that says 3k is safe. You're picking an arbitrary point that is safe in a codebase you've looked at. But I'm looking at this from a "what guarantees do I have from an ABI or ISA standpoint". The former may be more performant, but it's inherently less secure than the latter. > > With respect to implementation it is relatively easy to decide in > aarch64_layout_frame > which frames need probes and where. I don't think keeping a running offset of > the last > probe/store is useful, it'll just lead to inefficiencies and bugs. The patch > doesn't deal > with the delayed stores due to shrinkwrapping for example. Inserting probes > before > the prolog would be easier, eg. > > sub tmp, sp, 65536 > str xzr, [tmp, 1024] // allow up to 1KB of outgoing arguments in callee > sub tmp, sp, 131072 > str xzr, [tmp, 1024] > ... normal prolog for frame size 128-192KBIf you think you can do better > without compromising security, be my guest. Realistically, I'm pretty close to the limit of how much more time I can spend on the aarch64 target dependent issues. So I think the question I need answered from the aarch64 maintainers is Are they going to nak this code outright Are they going to keep basic structure but suggest tweaks I need to know the general direction so that I know whether or not to continue to carry the aarch64 changes in the patchkit. If y'all are going to nak, then I'll drop them from upstream except for the changes to indirect STACK_CHECK_PROTECT. jeff
[PATCH, rs6000] Add support for vec_revb builtin
GCC Maintainers:
The following patch adds support for the some additional vec_revb
builtin functions. It also adds a missing testcase. The
patch has been tested on powerpc64le-unknown-linux-gnu (Power 8 LE),
powerpc64-unknown-linux-gnu (Power 8 BE), powerpc64-unknown-linux-gnu
(Power 7).
Please let me know if the following patch is acceptable. Thanks.
Carl Love
gcc/ChangeLog:
2017-07-11 Carl Love
* config/rs6000/rs6000-c.c: Add support for built-in functions
vector bool char vec_revb (vector bool char);
vector bool short vec_revb (vector short char);
vector bool int vec_revb (vector bool int);
vector bool long long vec_revb (vector bool long long);
* doc/extend.texi: Update the built-in documentation file for the
new built-in functions.
gcc/testsuite/ChangeLog:
2017-07-11 Carl Love
* gcc.target/powerpc/p9-xxbr-1.c (rev_bool_char, rev_bool_short,
rev_bool_int): Add test cases for builtins.
* gcc.target/powerpc/p9-xxbr-2.c (rev_long_long, rev_ulong_ulong): Add
test cases for builtins.
---
gcc/config/rs6000/rs6000-c.c | 6 ++
gcc/doc/extend.texi | 4
gcc/testsuite/gcc.target/powerpc/p9-xxbr-1.c | 24 +---
gcc/testsuite/gcc.target/powerpc/p9-xxbr-2.c | 14 +-
4 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-c.c
index abe4479..c769442 100644
--- a/gcc/config/rs6000/rs6000-c.c
+++ b/gcc/config/rs6000/rs6000-c.c
@@ -5525,6 +5525,8 @@ const struct altivec_builtin_types
altivec_overloaded_builtins[] = {
{ P9V_BUILTIN_VEC_REVB, P9V_BUILTIN_XXBRQ_V16QI,
RS6000_BTI_unsigned_V16QI, RS6000_BTI_unsigned_V16QI, 0, 0 },
{ P9V_BUILTIN_VEC_REVB, P9V_BUILTIN_XXBRQ_V16QI,
+RS6000_BTI_bool_V16QI, RS6000_BTI_bool_V16QI, 0, 0 },
+ { P9V_BUILTIN_VEC_REVB, P9V_BUILTIN_XXBRQ_V16QI,
RS6000_BTI_V16QI, RS6000_BTI_V16QI, 0, 0 },
{ P9V_BUILTIN_VEC_REVB, P9V_BUILTIN_XXBRQ_V1TI,
RS6000_BTI_unsigned_V1TI, RS6000_BTI_unsigned_V1TI, 0, 0 },
@@ -5537,12 +5539,16 @@ const struct altivec_builtin_types
altivec_overloaded_builtins[] = {
{ P9V_BUILTIN_VEC_REVB, P9V_BUILTIN_XXBRD_V2DF,
RS6000_BTI_V2DF, RS6000_BTI_V2DF, 0, 0 },
{ P9V_BUILTIN_VEC_REVB, P9V_BUILTIN_XXBRW_V4SI,
+RS6000_BTI_bool_V4SI, RS6000_BTI_bool_V4SI, 0, 0 },
+ { P9V_BUILTIN_VEC_REVB, P9V_BUILTIN_XXBRW_V4SI,
RS6000_BTI_unsigned_V4SI, RS6000_BTI_unsigned_V4SI, 0, 0 },
{ P9V_BUILTIN_VEC_REVB, P9V_BUILTIN_XXBRW_V4SI,
RS6000_BTI_V4SI, RS6000_BTI_V4SI, 0, 0 },
{ P9V_BUILTIN_VEC_REVB, P9V_BUILTIN_XXBRW_V4SF,
RS6000_BTI_V4SF, RS6000_BTI_V4SF, 0, 0 },
{ P9V_BUILTIN_VEC_REVB, P9V_BUILTIN_XXBRH_V8HI,
+RS6000_BTI_bool_V8HI, RS6000_BTI_bool_V8HI, 0, 0 },
+ { P9V_BUILTIN_VEC_REVB, P9V_BUILTIN_XXBRH_V8HI,
RS6000_BTI_unsigned_V8HI, RS6000_BTI_unsigned_V8HI, 0, 0 },
{ P9V_BUILTIN_VEC_REVB, P9V_BUILTIN_XXBRH_V8HI,
RS6000_BTI_V8HI, RS6000_BTI_V8HI, 0, 0 },
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 07a953d..dc3d0d8 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -18483,13 +18483,17 @@ of each element.
If the ISA 3.0 instruction set additions (@option{-mcpu=power9})
are available:
@smallexample
+vector signed bool char vec_revb (vector signed char);
vector signed char vec_revb (vector signed char);
vector unsigned char vec_revb (vector unsigned char);
+vector bool short vec_revb (vector bool short);
vector short vec_revb (vector short);
vector unsigned short vec_revb (vector unsigned short);
+vector bool int vec_revb (vector bool int);
vector int vec_revb (vector int);
vector unsigned int vec_revb (vector unsigned int);
vector float vec_revb (vector float);
+vector bool long long vec_revb (vector bool long long);
vector long long vec_revb (vector long long);
vector unsigned long long vec_revb (vector unsigned long long);
vector double vec_revb (vector double);
diff --git a/gcc/testsuite/gcc.target/powerpc/p9-xxbr-1.c
b/gcc/testsuite/gcc.target/powerpc/p9-xxbr-1.c
index cd4ba73..164f11f 100644
--- a/gcc/testsuite/gcc.target/powerpc/p9-xxbr-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/p9-xxbr-1.c
@@ -13,6 +13,12 @@ rev_char (vector char a)
return vec_revb (a); /* XXBRQ. */
}
+vector bool char
+rev_bool_char (vector bool char a)
+{
+ return vec_revb (a); /* XXBRQ. */
+}
+
vector signed char
rev_schar (vector signed char a)
{
@@ -31,6 +37,12 @@ rev_short (vector short a)
return vec_revb (a); /* XXBRH. */
}
+vector bool short
+rev_bool_short (vector bool short a)
+{
+ return vec_revb (a); /* XXBRH. */
+}
+
vector unsigned short
rev_ushort (vector unsigned short a)
{
@@ -43,6 +55,12 @@ rev_int (vector int a)
return vec_revb (a); /* XXBRW. */
}
+vector bool int
+rev_bool_int
