[PATCH] tree-optimization: [PR102622]: wrong code due to signed one bit integer and "a?-1:0"
From: Andrew Pinski
So it turns out this is kinda of a latent bug but not really latent.
In GCC 9 and 10, phi-opt would transform a?-1:0 (even for signed 1-bit integer)
to -(type)a but the type is an one bit integer which means the negation is
undefined. GCC 11 fixed the problem by checking for a?pow2cst:0 transformation
before a?-1:0 transformation.
When I added the transformations to match.pd, I had swapped the order not paying
attention and I didn't expect anything of it. Because there was no testcase
failing
due to this.
Anyways this fixes the problem on the trunk by swapping the order in match.pd
and
adding a comment of why the order is this way.
I will try to come up with a patch for GCC 9 and 10 series later on which fixes
the problem there too.
Note I didn't include the original testcase which requires the vectorizer and
AVX-512f
as I can't figure out the right dg options to restrict it to avx-512f but I did
come up
with a testcase which shows the problem and even more shows the problem with
the 9/10
series as mentioned.
OK? Bootstrapped and tested on x86_64-linux-gnu.
PR tree-optimization/102622
gcc/ChangeLog:
* match.pd: Swap the order of a?pow2cst:0 and a?-1:0 transformations.
Swap the order of a?0:pow2cst and a?0:-1 transformations.
gcc/testsuite/ChangeLog:
* gcc.c-torture/execute/bitfld-10.c: New test.
---
gcc/match.pd | 26 ---
.../gcc.c-torture/execute/bitfld-10.c | 24 +
2 files changed, 41 insertions(+), 9 deletions(-)
create mode 100644 gcc/testsuite/gcc.c-torture/execute/bitfld-10.c
diff --git a/gcc/match.pd b/gcc/match.pd
index 9d7c1ac637f..c153e9a6e98 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3949,15 +3949,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* a ? 1 : 0 -> a if 0 and 1 are integral types. */
(if (integer_onep (@1))
(convert (convert:boolean_type_node @0)))
-/* a ? -1 : 0 -> -a. */
-(if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1))
- (negate (convert (convert:boolean_type_node @0
/* a ? powerof2cst : 0 -> a << (log2(powerof2cst)) */
(if (INTEGRAL_TYPE_P (type) && integer_pow2p (@1))
(with {
tree shift = build_int_cst (integer_type_node, tree_log2 (@1));
}
- (lshift (convert (convert:boolean_type_node @0)) { shift; })
+ (lshift (convert (convert:boolean_type_node @0)) { shift; })))
+/* a ? -1 : 0 -> -a. No need to check the TYPE_PRECISION not being 1
+ here as the powerof2cst case above will handle that case correctly. */
+(if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1))
+ (negate (convert (convert:boolean_type_node @0))
(if (integer_zerop (@1))
(with {
tree booltrue = constant_boolean_node (true, boolean_type_node);
@@ -3966,16 +3967,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* a ? 0 : 1 -> !a. */
(if (integer_onep (@2))
(convert (bit_xor (convert:boolean_type_node @0) { booltrue; } )))
- /* a ? -1 : 0 -> -(!a). */
- (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
- (negate (convert (bit_xor (convert:boolean_type_node @0) { booltrue; }
/* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */
- (if (INTEGRAL_TYPE_P (type) && integer_pow2p (@2))
+ (if (INTEGRAL_TYPE_P (type) && integer_pow2p (@2)
+ && TYPE_PRECISION (type) != 1)
(with {
tree shift = build_int_cst (integer_type_node, tree_log2 (@2));
}
(lshift (convert (bit_xor (convert:boolean_type_node @0) { booltrue; }
))
-{ shift; }
+{ shift; })))
+ /* a ? -1 : 0 -> -(!a). No need to check the TYPE_PRECISION not being 1
+ here as the powerof2cst case above will handle that case correctly. */
+ (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
+ (negate (convert (bit_xor (convert:boolean_type_node @0) { booltrue; }
+)
+ )
+ )
+ )
+)
#endif
/* Simplification moved from fold_cond_expr_with_comparison. It may also
diff --git a/gcc/testsuite/gcc.c-torture/execute/bitfld-10.c
b/gcc/testsuite/gcc.c-torture/execute/bitfld-10.c
new file mode 100644
index 000..bdbf5733ce7
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/bitfld-10.c
@@ -0,0 +1,24 @@
+/* PR tree-optimization/102622 */
+/* Wrong code introduced due to phi-opt
+ introducing undefined signed interger overflow
+ with one bit signed integer negation. */
+
+struct f{signed t:1;};
+int g(struct f *a, int t) __attribute__((noipa));
+int g(struct f *a, int t)
+{
+if (t)
+ a->t = -1;
+else
+ a->t = 0;
+int t1 = a->t;
+if (t1) return 1;
+return t1;
+}
+
+int main(void)
+{
+struct f a;
+if (!g(, 1)) __builtin_abort();
+return 0;
+}
--
2.17.1
Re: *PING* [PATCH] PR fortran/99348, 102521 - ICEs when initializing DT parameter arrays from scalar
This one looks OK. Sorry I missed it earlier. Thanks again for the patch! Jerry On 10/9/21 12:27 PM, Harald Anlauf via Fortran wrote: *Ping* Am 03.10.21 um 21:20 schrieb Harald Anlauf via Fortran: Dear Fortranners, when initializing parameter arrays from scalars, we did handle only the case init->expr_type == EXPR_CONSTANT, which misses the case of derived types. As a consequence the constructor for the r.h.s. was not set up, which later led to different ICEs. To solve this I looked at gfc_simplify_spread. I was contemplating whether to also copy the logic to make this initialization dependent on -fmax-array-constructor. I chose not to, because there is no sensible and simple fallback available to handle that case while allowing the access to array elements. We could instead make that a warning. Comments / opinions? Regtested on x86_64-pc-linux-gnu. OK for mainline? As this is an ICE on valid, potentially useful code, I'd like to backport this to 11-branch. Thanks, Harald
Re: [PATCH 0/2] libsanitizer: Merge with upstream commit fdf4c035225d
On Thu, 7 Oct 2021, H.J. Lu wrote: >> Thus breaking bootstrap on FreeBSD: >> >> GCC-HEAD/libsanitizer/sanitizer_common/sanitizer_platform_limits_freebsd.cpp:370:36: >> error: 'MD5_CTX' was not declared in this scope > 370 | const unsigned MD5_CTX_sz = sizeof(MD5_CTX); >> |^~~ >> GCC-HEAD/libsanitizer/sanitizer_common/sanitizer_platform_limits_freebsd.cpp:371:36: >> error: >> 'MD5_DIGEST_STRING_LENGTH' was not declared in this scope > compiler-rt sync brought in > > commit 18a7ebda99044473fdbce6376993714ff54e6690 > Author: David Carlier > Date: Wed Oct 6 06:01:50 2021 +0100 > > [Sanitizers] intercept md5 and sha* apis on FreeBSD. > > Reviewed By: vitalybuka > > Differential Revision: https://reviews.llvm.org/D110989 > > diff --git > a/libsanitizer/sanitizer_common/sanitizer_platform_limits_freebsd.cpp > b/libsanitizer/sanitizer_common/sanitizer_platform_limits_freebsd.cpp > index bfe3eea464d..64535805e40 100644 > --- a/libsanitizer/sanitizer_common/sanitizer_platform_limits_freebsd.cpp > +++ b/libsanitizer/sanitizer_common/sanitizer_platform_limits_freebsd.cpp > @@ -69,6 +69,11 @@ > #include > #include > #include > +#include Yep, and here is the problem: In the case of Clang this uses FreeBSD's . In the case of GCC apparently this uses our own, bare-bone md5.h - include/md5.h. Boom. Regression. Bootstrap failure on every platfor and version of FreeBSD. (FreeBSD 11.x is also broken by this patch, in a different way, alas went end of life a week ago.) I now filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102675 and sadly don't have an idea how to tackle this. Gerald
Re: [Patch] [v2] Fortran: Fix Bind(C) Array-Descriptor Conversion (Move to Front-End Code)
Re-sent with gzipped attachment as gcc-patches@ did reject it as being too
large.
Alternatively, you can find it at
https://gcc.gnu.org/pipermail/fortran/attachments/20211009/dc26f92d/attachment-0001.bin
as fortran@ contrary to gcc-patches@ did accept the patch ...
Tobias
On 09.10.21 23:48, Tobias Burnus wrote:
Hi all,
attached is the updated version. Changes:
* Handle noncontiguous arrays – with BIND(C), (g)Fortran needs to make it
contiguous in the caller but also handle noncontiguous in the callee.
* Fixes/handle 'character(len=*)' with BIND(C); those always use an
array descriptor - also with explicit-size and assumed-size arrays
* Fixed a bunch of bugs, found when writing extensive testcases.
* Fixed type(*) handling - those now pass properly type and elem_len
on when calling a new function (bind(C) or not).
Besides adding the type itself (which is rather straight forward),
this patch only had minor modifications – and then the two big
conversion functions.
While it looks intimidating, it should be comparably simple to
review as everything is on one place and hopefully sufficiently
well documented.
OK – for mainline? Other comments? More PRs which are fixed?
Issues not yet fixed (which are inside the scope of this patch)?
(If this patch is too long, I also have a nine-day old pending patch
at https://gcc.gnu.org/pipermail/gcc-patches/2021-October/580624.html )
Tobias
PS: The following still applies.
On 06.09.21 12:52, Tobias Burnus wrote:
gfortran's internal array descriptor (xgfc descriptor) and
the descriptor used with BIND(C) (CFI descriptor, ISO_Fortran_binding.h
of TS29113 / Fortran 2018) are different. Thus, when calling a BIND(C)
procedure the gfc descriptor has to be converted to cfi – and when a
BIND(C) procedure is implemented in Fortran, the argument has to be
converted back from CFI to gfc.
The current implementation handles part in the FE and part in
libgfortran,
but there were several issues, e.g. PR101635 failed due to alias issues,
debugging wasn't working well, uninitialized memory was used in some
cases
etc.
This patch now moves descriptor conversion handling to the FE – which
also
can make use of compile-time knowledge, useful both for diagnostic
and to
optimize the code.
Additionally:
- Some cases where TS29113 mandates that the array descriptor should be
used now use the array descriptor, in particular character scalars
with
'len=*' and allocatable/pointer scalars.
- While debugging the alias issue, I simplified 'select rank'. While
some
special case is needed for assumed-shape arrays, those cannot
appear when
the argument has the pointer or allocatable attribute. That's not
only a
missed optimization, pointer/allocatable arrays can also be NULL -
such
that accessing desc->dim.ubound[rank-1] can be uninitialized memory
...
OK? Comments? Suggestions?
* * *
For some more dumps, see the discussion about the alias issue at:
https://gcc.gnu.org/pipermail/gcc-patches/2021-August/578364.html
("[RFH] ME optimizes variable assignment away / Fortran bind(C)
descriptor conversion")
plus the original emails:
- https://gcc.gnu.org/pipermail/gcc-patches/2021-August/578271.html
- and (correct dump)
https://gcc.gnu.org/pipermail/gcc-patches/2021-August/578274.html
Debugging - not ideal but not too bad either. For
subroutine f(x) bind(C)
integer :: x(:)
with an uninitialized size-4 array as argument:
m::f (_x=...) at foo4.f90:3
3 subroutine f(x) bind(C)
(gdb) p x
Cannot access memory at address 0x38
(gdb) p _x
$6 = ( base_addr = 0x7fffe2c0, elem_len = 4, version = 1, rank =
1 '\001', attribute = 2 '\002', type = 1025, dim = (( lower_bound =
0, extent = 5, sm = 4 )) )
(gdb) s
5 x(1) = 5
(gdb) p x
$7 = (0, 0, 0, -670762413, 0)
Tobias
PS: This patch fixes but not necessarily fully the following PRs:
PR fortran/102086 - [F2008][TS29113] Accepts invalid scalar TYPE(*)
as actual argument to assumed-rank
PR fortran/92189 - Fortran-written bind(C) function with allocatable
argument does not update C descriptor on exit
PR fortran/92621 - Problems with memory handling with allocatable
intent(out) arrays with bind(c)
PR fortran/101308 - Bind(C): gfortran does not create C descriptors
for scalar pointer/allocatable arguments
PR fortran/101635 - FAIL: gfortran.dg/PR93963.f90 – alias-handling
issue with BIND(C)'s _gfortran_cfi_desc_to_gfc_desc
PR fortran/92482 - BIND(C) with array-descriptor mishandled for type
character
and possibly some more.
PPS: I should add some additional testcases – I try to do this as
Part 2 of this patch.
PPPS: Once the patch is in, some audit needs to be done which parts
of those PRs remain
as follow-up work. I think some still existing issues are covered by
José's pending
patches + for those which are now fixed, the testcase might still be
added.
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße
201, 80634 München; Gesellschaft mit beschränkter Haftung;
Geschäftsf
Re: [PATCH] Adjust more testcases for O2 vectorization enabling.
On Fri, Oct 8, 2021 at 9:55 PM liuhongt wrote: > > Pushed to trunk. > > libgomp/ChangeLog: > > * testsuite/libgomp.c++/scan-10.C: Add option -fvect-cost-model=cheap. > * testsuite/libgomp.c++/scan-11.C: Ditto. > * testsuite/libgomp.c++/scan-12.C: Ditto. > * testsuite/libgomp.c++/scan-13.C: Ditto. > * testsuite/libgomp.c++/scan-14.C: Ditto. > * testsuite/libgomp.c++/scan-15.C: Ditto. > * testsuite/libgomp.c++/scan-16.C: Ditto. > * testsuite/libgomp.c++/scan-9.C: Ditto. > * testsuite/libgomp.c-c++-common/lastprivate-conditional-7.c: Ditto. > * testsuite/libgomp.c-c++-common/lastprivate-conditional-8.c: Ditto. > * testsuite/libgomp.c/scan-11.c: Ditto. > * testsuite/libgomp.c/scan-12.c: Ditto. > * testsuite/libgomp.c/scan-13.c: Ditto. > * testsuite/libgomp.c/scan-14.c: Ditto. > * testsuite/libgomp.c/scan-15.c: Ditto. > * testsuite/libgomp.c/scan-16.c: Ditto. > * testsuite/libgomp.c/scan-17.c: Ditto. > * testsuite/libgomp.c/scan-18.c: Ditto. > * testsuite/libgomp.c/scan-19.c: Ditto. > * testsuite/libgomp.c/scan-20.c: Ditto. > * testsuite/libgomp.c/scan-21.c: Ditto. > * testsuite/libgomp.c/scan-22.c: Ditto. > > gcc/testsuite/ChangeLog: > > * g++.dg/tree-ssa/pr94403.C: Add -fno-tree-vectorize > * gcc.dg/optimize-bswapsi-5.c: Ditto. > * gcc.dg/optimize-bswapsi-6.c: Ditto. > * gcc.dg/Warray-bounds-51.c: Add -mtune=generic > * gcc.dg/Wstringop-overflow-14.c: libgomp.graphite/force-parallel-8.c also fails because of the -O2 change. -- H.J.
*PING* [PATCH] PR fortran/99348, 102521 - ICEs when initializing DT parameter arrays from scalar
*Ping* Am 03.10.21 um 21:20 schrieb Harald Anlauf via Fortran: Dear Fortranners, when initializing parameter arrays from scalars, we did handle only the case init->expr_type == EXPR_CONSTANT, which misses the case of derived types. As a consequence the constructor for the r.h.s. was not set up, which later led to different ICEs. To solve this I looked at gfc_simplify_spread. I was contemplating whether to also copy the logic to make this initialization dependent on -fmax-array-constructor. I chose not to, because there is no sensible and simple fallback available to handle that case while allowing the access to array elements. We could instead make that a warning. Comments / opinions? Regtested on x86_64-pc-linux-gnu. OK for mainline? As this is an ICE on valid, potentially useful code, I'd like to backport this to 11-branch. Thanks, Harald
Re: [PATCH] PR fortran/65454 - accept both old and new-style relational operators
Hi Jerry, > Gesendet: Samstag, 09. Oktober 2021 um 00:28 Uhr > Looks all good Harald, OK and thanks for the support! Thanks for the quick review! Harald
Re: [PATCH] Convert strlen pass from evrp to ranger.
We seem to be passing a lot of context around in the strlen code. I
certainly don't want to contribute to more.
Most of the handle_* functions are passing the gsi as well as either
ptr_qry or rvals. That looks a bit messy. May I suggest putting all
of that in the strlen pass object (well, the dom walker object, but we
can rename it to be less dom centric)?
Something like the attached (untested) patch could be the basis for
further cleanups.
Jakub, would this line of work interest you?
Aldy
On Fri, Oct 8, 2021 at 5:12 PM Aldy Hernandez wrote:
>
> The following patch converts the strlen pass from evrp to ranger,
> leaving DOM as the last remaining user.
>
> No additional cleanups have been done. For example, the strlen pass
> still has uses of VR_ANTI_RANGE, and the sprintf still passes around
> pairs of integers instead of using a proper range. Fixing this
> could further improve these passes.
>
> As a further enhancement, if the relevant maintainers deem useful,
> the domwalk could be removed from strlen. That is, unless the pass
> needs it for something else.
>
> With ranger we are now able to remove the range calculation from
> before_dom_children entirely. Just working with the ranger on-demand
> catches all the strlen and sprintf testcases with the exception of
> builtin-sprintf-warn-22.c which is due to a limitation of the sprintf
> code. I have XFAILed the test and documented what the problem is.
>
> It looks like the same problem in the sprintf test triggers a false
> positive in gimple-ssa-warn-access.cc so I have added
> -Wno-format-overflow until it can be fixed.
>
> I can expand on the false positive if necessary, but the gist is that
> this:
>
> _17 = strlen (_132);
> _18 = strlen (_136);
> _19 = _18 + _17;
> if (_19 > 75)
> goto ; [0.00%]
> else
> goto ; [100.00%]
>
> ...dominates the sprintf in BB61. This means that ranger can figure
> out that the _17 and _18 are [0, 75]. On the other hand, evrp
> returned a range of [0, 9223372036854775805] which presumably the
> sprintf code was ignoring as a false positive here:
>
> char sizstr[80];
> ...
> ...
> char *s1 = print_generic_expr_to_str (sizrng[1]);
> gcc_checking_assert (strlen (s0) + strlen (s1)
>< sizeof sizstr - 4);
> sprintf (sizstr, "[%s, %s]", s0, s1);
>
> The warning triggers with:
>
> gimple-ssa-warn-access.cc: In member function ‘void
> {anonymous}::pass_waccess::maybe_check_access_sizes(rdwr_map*, tree, tree,
> gimple*)’:
> gimple-ssa-warn-access.cc:2916:32: warning: ‘%s’ directive writing up to 75
> bytes into a region of size between 2 and 77 [-Wformat-overflow=]
> 2916 | sprintf (sizstr, "[%s, %s]", s0, s1);
> |^~
> gimple-ssa-warn-access.cc:2916:23: note: ‘sprintf’ output between 5 and 155
> bytes into a destination of size 80
> 2916 | sprintf (sizstr, "[%s, %s]", s0, s1);
> | ^~~~
>
> On a positive note, these changes found two possible sprintf overflow
> bugs in the C++ and Fortran front-ends which I have fixed below.
>
> Bootstrap and regtested on x86-64 Linux. I also ran it through our
> callgrind harness and there was no overall change in overall
> compilation time.
>
> OK?
>
> gcc/ChangeLog:
>
> * Makefile.in: Disable -Wformat-overflow for
> gimple-ssa-warn-access.o.
> * tree-ssa-strlen.c (compare_nonzero_chars): Pass statement
> context to ranger.
> (get_addr_stridx): Same.
> (get_stridx): Same.
> (get_range_strlen_dynamic): Same.
> (handle_builtin_strlen): Same.
> (handle_builtin_strchr): Same.
> (handle_builtin_strcpy): Same.
> (maybe_diag_stxncpy_trunc): Same.
> (handle_builtin_stxncpy_strncat):
> (handle_builtin_memcpy): Same.
> (handle_builtin_strcat): Same.
> (handle_alloc_call): Same.
> (handle_builtin_memset): Same.
> (handle_builtin_string_cmp): Same.
> (handle_pointer_plus): Same.
> (count_nonzero_bytes_addr): Same.
> (count_nonzero_bytes): Same.
> (handle_store): Same.
> (fold_strstr_to_strncmp): Same.
> (handle_integral_assign): Same.
> (check_and_optimize_stmt): Same.
> (class strlen_dom_walker): Replace evrp with ranger.
> (strlen_dom_walker::before_dom_children): Remove evrp.
> (strlen_dom_walker::after_dom_children): Remove evrp.
>
> gcc/cp/ChangeLog:
>
> * ptree.c (cxx_print_xnode): Add more space to pfx array.
>
> gcc/fortran/ChangeLog:
>
> * misc.c (gfc_dummy_typename): Make sure ts->kind is
> non-negative.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/builtin-sprintf-warn-22.c: XFAIL.
> ---
> gcc/Makefile.in | 1 +
>
[PATCH] check to see if null pointer is dereferenceable [PR102630]
When determining the size of an object compute_objsize_r() assumes
that addresses derived from null pointers are not derefernceable
because null pointers themselves are not, without calling
targetm.addr_space.zero_address_valid() to see if that assumption
is supported for the pointer on the target. The attached change
adds the missing call to avoid a spurious warning in Glibc that
uses named address spaces to implement TLS on x86_64.
Tested on x86_64-linux.
Martin
Check to see if null pointer is dereferenceable [PR102630].
Resolves:
PR middle-end/102630 - Spurious -Warray-bounds with named address space
gcc/ChangeLog:
PR middle-end/102630
* pointer-query.cc (compute_objsize_r): Handle named address spaces.
gcc/testsuite/ChangeLog:
PR middle-end/102630
* gcc.dg/Warray-bounds-90.c: New test.
diff --git a/gcc/pointer-query.cc b/gcc/pointer-query.cc
index 83b1f0fc866..910f452868e 100644
--- a/gcc/pointer-query.cc
+++ b/gcc/pointer-query.cc
@@ -41,6 +41,7 @@
#include "pointer-query.h"
#include "tree-pretty-print.h"
#include "tree-ssanames.h"
+#include "target.h"
static bool compute_objsize_r (tree, int, access_ref *, ssa_name_limit_t &,
pointer_query *);
@@ -1869,13 +1870,24 @@ compute_objsize_r (tree ptr, int ostype, access_ref *pref,
if (code == INTEGER_CST)
{
/* Pointer constants other than null are most likely the result
- of erroneous null pointer addition/subtraction. Set size to
- zero. For null pointers, set size to the maximum for now
- since those may be the result of jump threading. */
+ of erroneous null pointer addition/subtraction. Unless zero
+ is a valid address set size to zero. For null pointers, set
+ size to the maximum for now since those may be the result of
+ jump threading. */
if (integer_zerop (ptr))
pref->set_max_size_range ();
+ else if (POINTER_TYPE_P (TREE_TYPE (ptr)))
+ {
+ tree deref_type = TREE_TYPE (TREE_TYPE (ptr));
+ addr_space_t as = TYPE_ADDR_SPACE (deref_type);
+ if (targetm.addr_space.zero_address_valid (as))
+ pref->set_max_size_range ();
+ else
+ pref->sizrng[0] = pref->sizrng[1] = 0;
+ }
else
pref->sizrng[0] = pref->sizrng[1] = 0;
+
pref->ref = ptr;
return true;
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-90.c b/gcc/testsuite/gcc.dg/Warray-bounds-90.c
new file mode 100644
index 000..93461d4b238
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-90.c
@@ -0,0 +1,31 @@
+/* PR middle-end/102630 - spurious -Warray-bounds with named address space
+ { dg-do compile }
+ { dg-options "-O2 -Wall" } */
+
+void sink (void*, ...);
+#define sink(...) sink (0, __VA_ARGS__)
+
+
+void test_generic_null (void)
+{
+ // We want some warning here but -Warray-bounds may not be it.
+ *(char*)0 = 0; // { dg-warning "" "pr??" { xfail *-*-* } }
+}
+
+void test_generic_cstaddr (void)
+{
+ /* We get -Warray-bounds (-Wstringop-overflow in GCC 11) but some
+ other warning might be preferable. */
+ *(char*)1234 = 1; // { dg-warning "" }
+}
+
+
+void test_named_address_space_null (void)
+{
+ *(char __seg_fs*)0 = 0; // no warning (intentional)
+}
+
+void test_named_address_space_cstaddr (void)
+{
+ *(char __seg_fs*)1234 = 0; // { dg-bogus "-Warray-bounds" }
+}
Re: [PATCH] Convert strlen pass from evrp to ranger.
On 10/9/21 10:19 AM, Martin Sebor wrote: On 10/9/21 9:04 AM, Aldy Hernandez wrote: On Fri, Oct 8, 2021 at 6:52 PM Martin Sebor wrote: On 10/8/21 9:12 AM, Aldy Hernandez via Gcc-patches wrote: The following patch converts the strlen pass from evrp to ranger, leaving DOM as the last remaining user. Thanks for doing this. I know I said I'd work on it but I'm still bogged down in my stage 1 work that's not going so great :( I just have a few minor comments/questions on the strlen change (inline) but I am a bit concerned about the test failure. No additional cleanups have been done. For example, the strlen pass still has uses of VR_ANTI_RANGE, and the sprintf still passes around pairs of integers instead of using a proper range. Fixing this could further improve these passes. As a further enhancement, if the relevant maintainers deem useful, the domwalk could be removed from strlen. That is, unless the pass needs it for something else. With ranger we are now able to remove the range calculation from before_dom_children entirely. Just working with the ranger on-demand catches all the strlen and sprintf testcases with the exception of builtin-sprintf-warn-22.c which is due to a limitation of the sprintf code. I have XFAILed the test and documented what the problem is. builtin-sprintf-warn-22.c is a regression test for a false positive in Glibc. If it fails we'll have to deal with the Glibc failure again, which I would rather avoid. Have you checked to see if Glibc is affected by the change? Have you tested Glibc with the patch to see if the warning comes back? If it does there are other ways to suppress it, and I can take care of it. I just want us to avoid reintroducing it without doing our due diligence first. I've built Glibc with the latest GCC and your patch applied and the warning is back so we'll need to suppress it there. I opened Glibc bug 28439 and will submit a patch for it: https://sourceware.org/bugzilla/show_bug.cgi?id=28439 There are other Glibc warnings to deal with so I don't think your patch needs to wait until this one is resolved. I also missed the following in your patch: > gcc/ChangeLog: > >* Makefile.in: Disable -Wformat-overflow for >gimple-ssa-warn-access.o. Rather than disabling the warning for the whole file (or any others), unless suppressing it in the code is overly intrusive, doing it that way is preferable. For %s sprintf directives, specifying precision can be used to constrain the output. In this case where each %s output is the result of formatting an (at most) 64-bit integer, we know that the length of each %s argument is at most 21 bytes, so specifying a precision as big as 37 should both make sure the output fits and avoid the warning. I.e., this should (and in my tests does) work: char *s1 = print_generic_expr_to_str (sizrng[1]); sprintf (sizstr, "[%.37s, %.37s]", s0, s1); free (s1); Thanks Martin ... I think there still are quite a few calls to get_addr_stridx() and get_addr() that don't pass in rvals. I started doing this back in the GCC 11 cycle but didn't finish the job. Eventually, rvals should be passed to all their callers (or they made class members with a ranger member). I mention this in case it has an effect on the range propagation (since the pass also sets ranges). Definitely. You'll get even better ranges, though perhaps more false positives as discussed above :-/. We want better ranges and better analysis. Ultimately, it will lead to better quality warnings, as has been one of the goals behind Ranger. If along the way it means a few false positives in some edge cases, we'll deal with them. I don't see this one as a significant problem. Martin
Re: [PATCH] Convert strlen pass from evrp to ranger.
On 10/9/21 9:04 AM, Aldy Hernandez wrote: On Fri, Oct 8, 2021 at 6:52 PM Martin Sebor wrote: On 10/8/21 9:12 AM, Aldy Hernandez via Gcc-patches wrote: The following patch converts the strlen pass from evrp to ranger, leaving DOM as the last remaining user. Thanks for doing this. I know I said I'd work on it but I'm still bogged down in my stage 1 work that's not going so great :( I just have a few minor comments/questions on the strlen change (inline) but I am a bit concerned about the test failure. No additional cleanups have been done. For example, the strlen pass still has uses of VR_ANTI_RANGE, and the sprintf still passes around pairs of integers instead of using a proper range. Fixing this could further improve these passes. As a further enhancement, if the relevant maintainers deem useful, the domwalk could be removed from strlen. That is, unless the pass needs it for something else. With ranger we are now able to remove the range calculation from before_dom_children entirely. Just working with the ranger on-demand catches all the strlen and sprintf testcases with the exception of builtin-sprintf-warn-22.c which is due to a limitation of the sprintf code. I have XFAILed the test and documented what the problem is. builtin-sprintf-warn-22.c is a regression test for a false positive in Glibc. If it fails we'll have to deal with the Glibc failure again, which I would rather avoid. Have you checked to see if Glibc is affected by the change? Have you tested Glibc with the patch to see if the warning comes back? If it does there are other ways to suppress it, and I can take care of it. I just want us to avoid reintroducing it without doing our due diligence first. ... I think there still are quite a few calls to get_addr_stridx() and get_addr() that don't pass in rvals. I started doing this back in the GCC 11 cycle but didn't finish the job. Eventually, rvals should be passed to all their callers (or they made class members with a ranger member). I mention this in case it has an effect on the range propagation (since the pass also sets ranges). Definitely. You'll get even better ranges, though perhaps more false positives as discussed above :-/. We want better ranges and better analysis. Ultimately, it will lead to better quality warnings, as has been one of the goals behind Ranger. If along the way it means a few false positives in some edge cases, we'll deal with them. I don't see this one as a significant problem. Martin
Re: [PATCH] Convert strlen pass from evrp to ranger.
On Fri, Oct 8, 2021 at 6:52 PM Martin Sebor wrote:
>
> On 10/8/21 9:12 AM, Aldy Hernandez via Gcc-patches wrote:
> > The following patch converts the strlen pass from evrp to ranger,
> > leaving DOM as the last remaining user.
>
> Thanks for doing this. I know I said I'd work on it but I'm still
> bogged down in my stage 1 work that's not going so great :( I just
> have a few minor comments/questions on the strlen change (inline)
> but I am a bit concerned about the test failure.
>
> >
> > No additional cleanups have been done. For example, the strlen pass
> > still has uses of VR_ANTI_RANGE, and the sprintf still passes around
> > pairs of integers instead of using a proper range. Fixing this
> > could further improve these passes.
> >
> > As a further enhancement, if the relevant maintainers deem useful,
> > the domwalk could be removed from strlen. That is, unless the pass
> > needs it for something else.
> >
> > With ranger we are now able to remove the range calculation from
> > before_dom_children entirely. Just working with the ranger on-demand
> > catches all the strlen and sprintf testcases with the exception of
> > builtin-sprintf-warn-22.c which is due to a limitation of the sprintf
> > code. I have XFAILed the test and documented what the problem is.
>
> builtin-sprintf-warn-22.c is a regression test for a false positive
> in Glibc. If it fails we'll have to deal with the Glibc failure
> again, which I would rather avoid. Have you checked to see if
> Glibc is affected by the change?
>
> >
> > It looks like the same problem in the sprintf test triggers a false
> > positive in gimple-ssa-warn-access.cc so I have added
> > -Wno-format-overflow until it can be fixed.
> >
> > I can expand on the false positive if necessary, but the gist is that
> > this:
> >
> > _17 = strlen (_132);
> > _18 = strlen (_136);
> > _19 = _18 + _17;
> > if (_19 > 75)
> >goto ; [0.00%]
> > else
> >goto ; [100.00%]
> >
> > ...dominates the sprintf in BB61. This means that ranger can figure
> > out that the _17 and _18 are [0, 75]. On the other hand, evrp
> > returned a range of [0, 9223372036854775805] which presumably the
> > sprintf code was ignoring as a false positive here:
>
> This is a feature designed to avoid false positives when the sprintf
> pass doesn't know anything about the strings (i.e., their lengths
> are unconstrained by either the sizes of the arrays they're stored
> in or any expressions like asserts involving their lengths).
>
> It sounds like the strlen/ranger improvement partially propagates
> constraints from subsequent expressions into the strlen results
> but it doesn't go far enough for them to actually fully satisfy
> the constraint, which is what in turn triggers the warning.
>
> I.e., in the test:
>
> void g (char *s1, char *s2)
> {
>char b[1025];
>size_t n = __builtin_strlen (s1), d = __builtin_strlen (s2);
>if (n + d + 1 >= 1025)
> return;
>
>sprintf (b, "%s.%s", s1, s2); // { dg-bogus "\\\[-Wformat-overflow" }
>
> the range of n and d is [0, INF] and so the sprintf call doesn't
> trigger a warning. With your change, because their range is
> [0, 1023] each (and there's no way to express that their sum
> is less than 1025), the warning triggers because it considers
> the worst case scenario (the upper bounds of both).
I agree with Andrew. If this doesn't work with more than 1 string we
shouldn't even attempt it, as it's bound to be riddled with false
positives, which you'll get more of with enhanced ranges at this
point.
> > @@ -269,7 +270,7 @@ compare_nonzero_chars (strinfo *si, unsigned
> > HOST_WIDE_INT off,
> > return -1;
> >
> > value_range vr;
> > - if (!rvals->range_of_expr (vr, si->nonzero_chars, si->stmt))
> > + if (!rvals->range_of_expr (vr, si->nonzero_chars, stmt))
>
> We need stmt rather than si->stmt because the latter may be different
> or null when the former will always refer to the current statement,
> correct?
Yes.
> I think there still are quite a few calls to get_addr_stridx() and
> get_addr() that don't pass in rvals. I started doing this back in
> the GCC 11 cycle but didn't finish the job. Eventually, rvals
> should be passed to all their callers (or they made class members
> with a ranger member). I mention this in case it has an effect on
> the range propagation (since the pass also sets ranges).
Definitely. You'll get even better ranges, though perhaps more false
positives as discussed above :-/.
Aldy
Re: [PATCH] var-tracking: Fix a wrong-debug issue caused by my r10-7665 var-tracking change [PR102441]
On October 9, 2021 5:26:17 AM GMT+02:00, Alexandre Oliva wrote: >Hello, Jakub, > >On Oct 4, 2021, Jakub Jelinek wrote: > >> Finally, patch2, the shortest patch, uses MO_VAL_SET whenever destination >> is not sp and otherwise drops the micro-operation on the floor. > >That sounds quite reasonable to me, and it is indeed my favorite of the >3 proposed patches, because the mapping of locations to values is kept >most accurate. What I thought as well. Thus, OK. Richard. >Thanks! >
[PATCH] doc: Fix typos in alloc_size documentation
2021-10-09 Daniel Le
* doc/extend.texi (Common Variable Attributes): Fix typos in
alloc_size documentation.
---
gcc/doc/extend.texi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 133b82eef38..10d466fae9a 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -7359,10 +7359,10 @@ The @code{warn_if_not_aligned} attribute can
also be used for types
The @code{alloc_size} variable attribute may be applied to the declaration
of a pointer to a function that returns a pointer and takes at least one
argument of an integer type. It indicates that the returned pointer points
-to an object whose size is given by the function argument at @var{position-1},
+to an object whose size is given by the function argument at @var{position},
or by the product of the arguments at @var{position-1} and @var{position-2}.
Meaningful sizes are positive values less than @code{PTRDIFF_MAX}. Other
-sizes are disagnosed when detected. GCC uses this information to improve
+sizes are diagnosed when detected. GCC uses this information to improve
the results of @code{__builtin_object_size}.
For instance, the following declarations
--
2.25.1
--
Regards,
Daniel
Re: [PATCH] testsuite: Add missing comment for some dg-warning
On Sat, Oct 9, 2021 at 3:51 PM Kewen.Lin via Gcc-patches
wrote:
>
> Hi,
>
> This patch fixes the typos introduced by commit r12-4240.
>
> The dg-warning format looks like:
>
> { dg-warning regexp [comment [{ target/xfail selector } [line] ]] }
>
> Some dg-warnings such as:
>
> { dg-warning "\\\[-Wstringop-overflow" { target { i?86-*-* x86_64-*-* } } }
>
> miss the comment field, it makes target selector not take effect.
>
> For targets which are not { i?86-*-* x86_64-*-* }, this kind of cases
> fail or pass unexpectedly.
>
> Is it ok for trunk?
LGTM, thanks.
>
> BR,
> Kewen
> ---
> gcc/testsuite/ChangeLog:
>
> * c-c++-common/Wstringop-overflow-2.c: Add missing comment.
> * gcc.dg/Warray-bounds-51.c: Likewise.
> * gcc.dg/Warray-parameter-3.c: Likewise.
> * gcc.dg/Wstringop-overflow-14.c: Likewise.
> * gcc.dg/Wstringop-overflow-21.c: Likewise.
> * gcc.dg/Wstringop-overflow-76.c: Likewise.
>
> -
> diff --git a/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
> b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
> index 7e9da8a02cb..7d29b5f48c7 100644
> --- a/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
> +++ b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
> @@ -221,7 +221,7 @@ void ga1_1 (void)
>a1_1.a[1] = 1;// { dg-warning "\\\[-Wstringop-overflow" }
>a1_1.a[2] = 2;// { dg-warning "\\\[-Wstringop-overflow" }
>
> - struct A1 a = { 0, { 1 } }; // { dg-warning "\\\[-Wstringop-overflow" {
> target { i?86-*-* x86_64-*-* } } }
> + struct A1 a = { 0, { 1 } }; // { dg-warning "\\\[-Wstringop-overflow" ""
> { target { i?86-*-* x86_64-*-* } } }
>a.a[0] = 0;
>a.a[1] = 1; // { dg-warning "\\\[-Wstringop-overflow" ""
> { xfail { i?86-*-* x86_64-*-* } } }
>a.a[2] = 2; // { dg-warning "\\\[-Wstringop-overflow" ""
> { xfail { i?86-*-* x86_64-*-* } } }
> @@ -320,7 +320,7 @@ void ga1i_1 (void)
>a1i_1.a[1] = 1; // { dg-warning "\\\[-Wstringop-overflow" }
>a1i_1.a[2] = 2; // { dg-warning "\\\[-Wstringop-overflow" }
>
> - struct A1 a = { 0, { 1 } }; // { dg-warning "\\\[-Wstringop-overflow" {
> target { i?86-*-* x86_64-*-* } } }
> + struct A1 a = { 0, { 1 } }; // { dg-warning "\\\[-Wstringop-overflow" ""
> { target { i?86-*-* x86_64-*-* } } }
>a.a[0] = 1;
>a.a[1] = 2; // { dg-warning "\\\[-Wstringop-overflow" ""
> { xfail { i?86-*-* x86_64-*-* } } }
>a.a[2] = 3; // { dg-warning "\\\[-Wstringop-overflow" ""
> { xfail { i?86-*-* x86_64-*-* } } }
> diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-51.c
> b/gcc/testsuite/gcc.dg/Warray-bounds-51.c
> index b0b8bdb7938..c12f1407385 100644
> --- a/gcc/testsuite/gcc.dg/Warray-bounds-51.c
> +++ b/gcc/testsuite/gcc.dg/Warray-bounds-51.c
> @@ -38,7 +38,7 @@ void test_struct_char_vla_location (void)
>} s;
>
>s.cvla[0] = __LINE__;
> - s.cvla[nelts - 1] = 0; // { dg-warning "\\\[-Wstringop-overflow" { target
> { i?86-*-* x86_64-*-* } } }
> + s.cvla[nelts - 1] = 0; // { dg-warning "\\\[-Wstringop-overflow" "" {
> target { i?86-*-* x86_64-*-* } } }
>s.cvla[nelts] = 0; // { dg-warning "\\\[-Warray-bounds" }
>
>sink ();
> diff --git a/gcc/testsuite/gcc.dg/Warray-parameter-3.c
> b/gcc/testsuite/gcc.dg/Warray-parameter-3.c
> index e2c47e1ed36..e8a269c85c6 100644
> --- a/gcc/testsuite/gcc.dg/Warray-parameter-3.c
> +++ b/gcc/testsuite/gcc.dg/Warray-parameter-3.c
> @@ -77,7 +77,7 @@ gia3 (int a[3])
> __attribute__ ((noipa)) void
> gcas3 (char a[static 3])
> {
> - a[0] = 0; a[1] = 1; a[2] = 2; // { dg-warning "\\\[-Wstringop-overflow" {
> target { i?86-*-* x86_64-*-* } } }
> + a[0] = 0; a[1] = 1; a[2] = 2; // { dg-warning "\\\[-Wstringop-overflow" ""
> { target { i?86-*-* x86_64-*-* } } }
>a[3] = 3; // { dg-warning "\\\[-Warray-bounds" }
> }
>
> diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-14.c
> b/gcc/testsuite/gcc.dg/Wstringop-overflow-14.c
> index b648f5b41b1..7ac0154fc25 100644
> --- a/gcc/testsuite/gcc.dg/Wstringop-overflow-14.c
> +++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-14.c
> @@ -35,7 +35,7 @@ void test_memcpy_cond (int i)
> void test_int16 (void)
> {
>char *p = a4 + 1;
> - *(int16_t*)p = 0;// { dg-warning "writing 4 bytes into a region of
> size 3" { target { i?86-*-* x86_64-*-* } } }
> + *(int16_t*)p = 0;// { dg-warning "writing 4 bytes into a region of
> size 3" "" { target { i?86-*-* x86_64-*-* } } }
>*(int16_t*)(p + 2) = 0; // { dg-warning "writing 2 bytes into a region
> of size 1" "" { xfail { i?86-*-* x86_64-*-* } } }
> }
>
> diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-21.c
> b/gcc/testsuite/gcc.dg/Wstringop-overflow-21.c
> index e88f7b47894..d88bde9c740 100644
> --- a/gcc/testsuite/gcc.dg/Wstringop-overflow-21.c
> +++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-21.c
> @@ -23,7 +23,7 @@ void test_store_zero_length (int i)
> {
>char
[PATCH] testsuite: Add missing comment for some dg-warning
Hi,
This patch fixes the typos introduced by commit r12-4240.
The dg-warning format looks like:
{ dg-warning regexp [comment [{ target/xfail selector } [line] ]] }
Some dg-warnings such as:
{ dg-warning "\\\[-Wstringop-overflow" { target { i?86-*-* x86_64-*-* } } }
miss the comment field, it makes target selector not take effect.
For targets which are not { i?86-*-* x86_64-*-* }, this kind of cases
fail or pass unexpectedly.
Is it ok for trunk?
BR,
Kewen
---
gcc/testsuite/ChangeLog:
* c-c++-common/Wstringop-overflow-2.c: Add missing comment.
* gcc.dg/Warray-bounds-51.c: Likewise.
* gcc.dg/Warray-parameter-3.c: Likewise.
* gcc.dg/Wstringop-overflow-14.c: Likewise.
* gcc.dg/Wstringop-overflow-21.c: Likewise.
* gcc.dg/Wstringop-overflow-76.c: Likewise.
-
diff --git a/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
index 7e9da8a02cb..7d29b5f48c7 100644
--- a/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
+++ b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
@@ -221,7 +221,7 @@ void ga1_1 (void)
a1_1.a[1] = 1;// { dg-warning "\\\[-Wstringop-overflow" }
a1_1.a[2] = 2;// { dg-warning "\\\[-Wstringop-overflow" }
- struct A1 a = { 0, { 1 } }; // { dg-warning "\\\[-Wstringop-overflow" {
target { i?86-*-* x86_64-*-* } } }
+ struct A1 a = { 0, { 1 } }; // { dg-warning "\\\[-Wstringop-overflow" "" {
target { i?86-*-* x86_64-*-* } } }
a.a[0] = 0;
a.a[1] = 1; // { dg-warning "\\\[-Wstringop-overflow" "" {
xfail { i?86-*-* x86_64-*-* } } }
a.a[2] = 2; // { dg-warning "\\\[-Wstringop-overflow" "" {
xfail { i?86-*-* x86_64-*-* } } }
@@ -320,7 +320,7 @@ void ga1i_1 (void)
a1i_1.a[1] = 1; // { dg-warning "\\\[-Wstringop-overflow" }
a1i_1.a[2] = 2; // { dg-warning "\\\[-Wstringop-overflow" }
- struct A1 a = { 0, { 1 } }; // { dg-warning "\\\[-Wstringop-overflow" {
target { i?86-*-* x86_64-*-* } } }
+ struct A1 a = { 0, { 1 } }; // { dg-warning "\\\[-Wstringop-overflow" "" {
target { i?86-*-* x86_64-*-* } } }
a.a[0] = 1;
a.a[1] = 2; // { dg-warning "\\\[-Wstringop-overflow" "" {
xfail { i?86-*-* x86_64-*-* } } }
a.a[2] = 3; // { dg-warning "\\\[-Wstringop-overflow" "" {
xfail { i?86-*-* x86_64-*-* } } }
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-51.c
b/gcc/testsuite/gcc.dg/Warray-bounds-51.c
index b0b8bdb7938..c12f1407385 100644
--- a/gcc/testsuite/gcc.dg/Warray-bounds-51.c
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-51.c
@@ -38,7 +38,7 @@ void test_struct_char_vla_location (void)
} s;
s.cvla[0] = __LINE__;
- s.cvla[nelts - 1] = 0; // { dg-warning "\\\[-Wstringop-overflow" { target {
i?86-*-* x86_64-*-* } } }
+ s.cvla[nelts - 1] = 0; // { dg-warning "\\\[-Wstringop-overflow" "" { target
{ i?86-*-* x86_64-*-* } } }
s.cvla[nelts] = 0; // { dg-warning "\\\[-Warray-bounds" }
sink ();
diff --git a/gcc/testsuite/gcc.dg/Warray-parameter-3.c
b/gcc/testsuite/gcc.dg/Warray-parameter-3.c
index e2c47e1ed36..e8a269c85c6 100644
--- a/gcc/testsuite/gcc.dg/Warray-parameter-3.c
+++ b/gcc/testsuite/gcc.dg/Warray-parameter-3.c
@@ -77,7 +77,7 @@ gia3 (int a[3])
__attribute__ ((noipa)) void
gcas3 (char a[static 3])
{
- a[0] = 0; a[1] = 1; a[2] = 2; // { dg-warning "\\\[-Wstringop-overflow" {
target { i?86-*-* x86_64-*-* } } }
+ a[0] = 0; a[1] = 1; a[2] = 2; // { dg-warning "\\\[-Wstringop-overflow" "" {
target { i?86-*-* x86_64-*-* } } }
a[3] = 3; // { dg-warning "\\\[-Warray-bounds" }
}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-14.c
b/gcc/testsuite/gcc.dg/Wstringop-overflow-14.c
index b648f5b41b1..7ac0154fc25 100644
--- a/gcc/testsuite/gcc.dg/Wstringop-overflow-14.c
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-14.c
@@ -35,7 +35,7 @@ void test_memcpy_cond (int i)
void test_int16 (void)
{
char *p = a4 + 1;
- *(int16_t*)p = 0;// { dg-warning "writing 4 bytes into a region of size
3" { target { i?86-*-* x86_64-*-* } } }
+ *(int16_t*)p = 0;// { dg-warning "writing 4 bytes into a region of size
3" "" { target { i?86-*-* x86_64-*-* } } }
*(int16_t*)(p + 2) = 0; // { dg-warning "writing 2 bytes into a region of
size 1" "" { xfail { i?86-*-* x86_64-*-* } } }
}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-21.c
b/gcc/testsuite/gcc.dg/Wstringop-overflow-21.c
index e88f7b47894..d88bde9c740 100644
--- a/gcc/testsuite/gcc.dg/Wstringop-overflow-21.c
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-21.c
@@ -23,7 +23,7 @@ void test_store_zero_length (int i)
{
char a[3];
struct S0 *p = (struct S0*)a;
- p->a = 0; // { dg-warning "\\\[-Wstringop-overflow"
{ target { i?86-*-* x86_64-*-* } } }
+ p->a = 0; // { dg-warning "\\\[-Wstringop-overflow"
"" { target { i?86-*-* x86_64-*-* } } }
p->b[0] = 0;
p->b[1] =
Re: Add dg-require-wchars to libstdc++ testsuite
On Fri, 15 Jan 2021, 19:47 Jonathan Wakely, wrote: > > > On Fri, 15 Jan 2021, 16:19 Alexandre Oliva, wrote: > >> On Jan 15, 2021, Jonathan Wakely wrote: >> >> > On Thu, 14 Jan 2021, 22:22 Alexandre Oliva, wrote: >> >> ... it is definitely the case that the target currently defines >> wchar_t, >> >> and it even offers wchar.h and a lot of (maybe all?) wcs* functions. >> >> This was likely not the case when the patch was first written. >> >> >> >> I'll double check whether any of the patch is still needed for current >> >> versions. >> >> With the tests I've run since yesterday, I've determined that: >> >> - the wchar-related patches for the libstdc++ testsuite, that I had >> proposed last year, are no longer needed >> >> - your two patchlets did not bring about any regressions to test >> results, not in mainline x86_64-linux-gnu native, not with the trivial >> backports to the gcc-10 tree for x-arm-vxw7r2 that was the focus of my >> immediate attention. >> >> So, I withdraw my submissions of the testsuite patches, and I encourage >> you to proceed with the two changes you proposed. >> > > Great, I'll save them in a git branch to be revisited in stage 1. > I've committed a series of 8 patches to enable partial wchar_t support even without a working in libc. std::wstring and std::wstring_view should work fine now (albeit slower than if libc provides optimised wcslen etc.) Wide character I/O and charset conversions still require libc support, so are disabled when _GLIBCXX_USE_WCHAR_T is not defined. I would appreciate confirmation that this hasn't caused any problems for vxworks (understanding that vxworks does have wide character support in libc now, as discussed back in January). > > >> However, for avoidance of any doubt, I'll restate that I cannot vow for >> whether they're enough to fix the issues we'd run into back when >> wchar/wcs* were not supported in the target system, because now they >> are, so the changes do not bring any visible improvements to our results >> either. >> > > Understood, thanks for checking them though. > > >>
Re: [PATCH] hardened conditionals
On October 9, 2021 5:30:04 AM GMT+02:00, Alexandre Oliva via Gcc-patches wrote: > >This patch introduces optional passes to harden conditionals used in >branches, and in computing boolean expressions, by adding redundant >tests of the reversed conditions, and trapping in case of unexpected >results. Though in abstract machines the redundant tests should never >fail, CPUs may be led to misbehave under certain kinds of attacks, >such as of power deprivation, and these tests reduce the likelihood of >going too far down an unexpected execution path. > >This patch was regstrapped on x86_64-linux-gnu. It was also >bootstrapped along with an extra common.opt that enables both passes >unconditionally. Ok to install? Why two passes (and two IL traverses?) How do you prevent RTL optimizers (jump threading) from removing the redundant tests? I'd have expected such hardening to occur very late in the RTL pipeline. Richard. > >for gcc/ChangeLog > > * common.opt (fharden-compares): New. > (fharden-conditional-branches): New. > * doc/invoke.texi: Document new options. > * gimple-harden-conditionals.cc: New. > * passes.def: Add new passes. > * tree-pass.h (make_pass_harden_compares): Declare. > (make_pass_harden_conditional_branches): Declare. > >for gcc/ada/ChangeLog > > * doc/gnat_rm/security_hardening_features.rst > (Hardened Conditionals): New. > >for gcc/testsuite/ChangeLog > > * c-c++-common/torture/harden-comp.c: New. > * c-c++-common/torture/harden-cond.c: New. >--- > gcc/Makefile.in|1 > .../doc/gnat_rm/security_hardening_features.rst| 40 ++ > gcc/common.opt |8 > gcc/doc/invoke.texi| 19 + > gcc/gimple-harden-conditionals.cc | 379 > gcc/passes.def |2 > gcc/testsuite/c-c++-common/torture/harden-comp.c | 14 + > gcc/testsuite/c-c++-common/torture/harden-cond.c | 18 + > gcc/tree-pass.h|3 > 9 files changed, 484 insertions(+) > create mode 100644 gcc/gimple-harden-conditionals.cc > create mode 100644 gcc/testsuite/c-c++-common/torture/harden-comp.c > create mode 100644 gcc/testsuite/c-c++-common/torture/harden-cond.c > >diff --git a/gcc/Makefile.in b/gcc/Makefile.in >index 64252697573a7..7209ed117d09d 100644 >--- a/gcc/Makefile.in >+++ b/gcc/Makefile.in >@@ -1389,6 +1389,7 @@ OBJS = \ > gimple-if-to-switch.o \ > gimple-iterator.o \ > gimple-fold.o \ >+ gimple-harden-conditionals.o \ > gimple-laddress.o \ > gimple-loop-interchange.o \ > gimple-loop-jam.o \ >diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst >b/gcc/ada/doc/gnat_rm/security_hardening_features.rst >index 1c46e3a4c7b88..52240d7e3dd54 100644 >--- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst >+++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst >@@ -87,3 +87,43 @@ types and subtypes, may be silently ignored. Specifically, >it is not > currently recommended to rely on any effects this pragma might be > expected to have when calling subprograms through access-to-subprogram > variables. >+ >+ >+.. Hardened Conditionals: >+ >+Hardened Conditionals >+= >+ >+GNAT can harden conditionals to protect against control flow attacks. >+ >+This is accomplished by two complementary transformations, each >+activated by a separate command-line option. >+ >+The option *-fharden-compares* enables hardening of compares that >+compute results stored in variables, adding verification that the >+reversed compare yields the opposite result. >+ >+The option *-fharden-conditional-branches* enables hardening of >+compares that guard conditional branches, adding verification of the >+reversed compare to both execution paths. >+ >+These transformations are introduced late in the compilation pipeline, >+long after boolean expressions are decomposed into separate compares, >+each one turned into either a conditional branch or a compare whose >+result is stored in a boolean variable or temporary. Compiler >+optimizations, if enabled, may also turn conditional branches into >+stored compares, and vice-versa. Conditionals may also be optimized >+out entirely, if their value can be determined at compile time, and >+occasionally multiple compares can be combined into one. >+ >+It is thus difficult to predict which of these two options will affect >+a specific compare operation expressed in source code. Using both >+options ensures that every compare that is not optimized out will be >+hardened. >+ >+The addition of reversed compares can be observed by enabling the dump >+files of the corresponding passes, through command line options >+*-fdump-tree-hardcmp* and *-fdump-tree-hardcbr*, respectively. >+ >+They are separate options, however, because of the significantly
