Re: [PATCH] c++, v2: Fix get_member_function_from_ptrfunc with -fsanitize=bounds [PR116449]
Am 2024-09-04 um 19:12 schrieb Jakub Jelinek: On Wed, Sep 04, 2024 at 12:34:04PM -0400, Jason Merrill wrote: So, one possibility would be to call save_expr unconditionally in get_member_function_from_ptrfunc as well. Or build a TARGET_EXPR (force_target_expr or similar). Yes. I don't have a strong preference between the two. Here is a patch that uses save_expr but uses it still conditionally, doesn't make sense to use it for the common case of just decls, there is nothing to unshare in that case. Passed the test, ok if it passes full bootstrap/regtest? 2024-09-04 Jakub Jelinek PR c++/116449 * typeck.cc (get_member_function_from_ptrfunc): Use save_expr on instance_ptr and function even if it doesn't have side-effects, as long as it isn't a decl. * g++.dg/ubsan/pr116449.C: New test. --- gcc/cp/typeck.cc.jj 2024-09-02 17:07:30.115098114 +0200 +++ gcc/cp/typeck.cc2024-09-04 19:08:24.127490242 +0200 @@ -4188,10 +4188,21 @@ get_member_function_from_ptrfunc (tree * if (!nonvirtual && is_dummy_object (instance_ptr)) nonvirtual = true; - if (TREE_SIDE_EFFECTS (instance_ptr)) - instance_ptr = instance_save_expr = save_expr (instance_ptr); + /* Use save_expr even when instance_ptr doesn't have side-effects, +unless it is a simple decl (save_expr won't do anything on +constants), so that we don't ubsan instrument the expression +multiple times. See PR116449. */ + if (TREE_SIDE_EFFECTS (instance_ptr) || !DECL_P (instance_ptr)) + { + instance_save_expr = save_expr (instance_ptr); + if (instance_save_expr == instance_ptr) + instance_save_expr = NULL_TREE; + else + instance_ptr = instance_save_expr; + } - if (TREE_SIDE_EFFECTS (function)) + /* See above comment. */ + if (TREE_SIDE_EFFECTS (function) || !DECL_P (function)) function = save_expr (function); Hmm, it just occured to me, how about adding !NONVIRTUAL here? When NONVIRTUAL is true, there is no conditional stmt at all, or? Franz
Re: [PATCH] Make Warray-bounds alias to Warray-bounds= [PR107787]
Am 2022-11-23 um 21:11 schrieb Richard Biener via Gcc-patches: On Wed, Nov 23, 2022 at 3:08 PM Iskander Shakirzyanov via Gcc-patches wrote: Hi! Sorry for the initially missing description. The following patch changes the definition of -Warray-bounds to an Alias to -Warray-bounds=1. This is necessary for the correct use of -Werror=array-bounds=X, for more information see bug report 107787 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107787) As I understand, this happens because -Warray-bounds and -Warray-bounds= are declared as 2 different options in common.opt, so when diagnostic_classify_diagnostic() (opts-common.cc:1880) is called, DK_ERROR is set for the -Warray-bounds= option, but in diagnostic_report_diagnostic() (diagnostic.cc:1446) through warning_at() passes opt_index of -Warray-bounds, so information about DK_ERROR is lost. How did you test the patch? If you bootstrapped it and ran the testsuite then it's OK. Hi, I'm pretty sure the testsuite will have regressions, as I have a very similar patch lying around that needs these testsuite changes. I also added some modified tests that check that -Werror=array-bounds=X works as expected (which it didn't before). This also shows nicely why I don't like warnings with levels, what if I want -Werror=array-bounds=2 + -Warray-bounds=1? The reason (besides lack of time) I didn't submit that patch yet, is that I wanted to check if the tool to process common.opt can be changed to detect warnings with duplicated warning variables. Because I think at least -Wuse-after-free= and Wattributes= have the same problem. BTW, is the duplicated warning description "Warn if an array is accessed out of bounds." needed or not with Alias()? There are examples either way in common.opt. I've attached my patch, feel free to integrate the testsuite changes. Franz From 9bfefe2082f55f2ad2cd19beedbfeaf9bd20fb4a Mon Sep 17 00:00:00 2001 From: Franz Sirl Date: Thu, 8 Jul 2021 10:25:00 +0200 Subject: [PATCH 08/11] Unify -Warray-bounds/-Warray-bounds= warning variables. --- gcc/builtins.cc | 6 +- gcc/c-family/c-common.cc| 6 +- gcc/common.opt | 3 +- gcc/diagnostic-spec.cc | 1 - gcc/gimple-array-bounds.cc | 38 +++--- gcc/gimple-ssa-warn-restrict.cc | 2 +- gcc/testsuite/gcc.dg/Warray-bounds-34.c | 2 +- gcc/testsuite/gcc.dg/Warray-bounds-43.c | 6 +- gcc/testsuite/gcc.dg/Warray-bounds-93.c | 103 gcc/testsuite/gcc.dg/Warray-bounds-94.c | 149 10 files changed, 283 insertions(+), 33 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/Warray-bounds-93.c create mode 100644 gcc/testsuite/gcc.dg/Warray-bounds-94.c diff --git a/gcc/builtins.cc b/gcc/builtins.cc index 4dc1ca672b2..02c4fefa86f 100644 --- a/gcc/builtins.cc +++ b/gcc/builtins.cc @@ -696,14 +696,14 @@ c_strlen (tree arg, int only_value, c_strlen_data *data, unsigned eltsize) { /* Suppress multiple warnings for propagated constant strings. */ if (only_value != 2 - && !warning_suppressed_p (arg, OPT_Warray_bounds) - && warning_at (loc, OPT_Warray_bounds, + && !warning_suppressed_p (arg, OPT_Warray_bounds_) + && warning_at (loc, OPT_Warray_bounds_, "offset %qwi outside bounds of constant string", eltoff)) { if (decl) inform (DECL_SOURCE_LOCATION (decl), "%qE declared here", decl); - suppress_warning (arg, OPT_Warray_bounds); + suppress_warning (arg, OPT_Warray_bounds_); } return NULL_TREE; } diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc index 71507d4cb0a..cf423d384f6 100644 --- a/gcc/c-family/c-common.cc +++ b/gcc/c-family/c-common.cc @@ -6811,7 +6811,7 @@ fold_offsetof (tree expr, tree type, enum tree_code ctx) definition thereof. */ if (TREE_CODE (v) == ARRAY_REF || TREE_CODE (v) == COMPONENT_REF) - warning (OPT_Warray_bounds, + warning (OPT_Warray_bounds_, "index %E denotes an offset " "greater than size of %qT", t, TREE_TYPE (TREE_OPERAND (expr, 0))); @@ -8530,9 +8530,9 @@ convert_vector_to_array_for_subscript (location_t loc, index = fold_for_warn (index); if (TREE_CODE (index) == INTEGER_CST) -if (!tree_fits_uhwi_p (index) + if (!tree_fits_uhwi_p (index) || maybe_ge (tree_to_uhwi (index), TYPE_VECTOR_SUBPARTS (type))) - warning_at (loc, OPT_Warray_bounds, "index value is out of bound"); + warning_at (loc, OPT_Warray_bounds_, "index value is out of bound");
Re: [RFA] Minor improvement to coremark, avoid unconditional jump to return
Am 2022-10-07 um 16:13 schrieb Jeff Law: On 10/7/22 04:51, Franz Sirl wrote: Am 2022-09-25 um 18:28 schrieb Jeff Law: This is a minor improvement for the core_list_find routine in coremark. Basically for riscv, and likely other targets, we can end up with an unconditional jump to a return statement. This is a result of compensation code created by bb-reorder, and no jump optimization pass runs after bb-reorder to clean this stuff up. This patch utilizes preexisting code to identify suitable branch targets as well as preexisting code to emit a suitable return, so it's pretty simple. Note that when we arrange to do this optimization, the original return block may become unreachable. So we conditionally call delete_unreachable_blocks to fix that up. This triggers ~160 times during an x86_64 bootstrap. Naturally it bootstraps and regression tests on x86_64. I've also bootstrapped this on riscv64, regression testing with qemu shows some regressions, but AFAICT they're actually qemu bugs with signal handling/delivery -- qemu user mode emulation is not consistently calling user defined signal handlers. Given the same binary, sometimes they'll get called and the test passes, other times the handler isn't called and the test (of course) fails. I'll probably spend some time to try and chase this down for the sake of making testing easier. OK for the trunk? Hello Jeff, I've bisected this change to break a "profiledbootstrap" on x86_64 like that: make[3]: Entering directory '/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/gcc' /home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/./prev-gcc/xg++ -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/./prev-gcc/ -B/usr/x86_64-suse-linux/ bin/ -nostdinc++ -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/src/.libs -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64 -suse-linux/prev-x86_64-suse-linux/libstdc++-v3/libsupc++/.libs -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/include/x86_64-suse-linu x -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/include -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/libstdc++-v3/libsupc++ -L /home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/src/.libs -L/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x8 6_64-suse-linux/libstdc++-v3/libsupc++/.libs -fno-PIE -c -O2 -g -fmessage-length=0 -D_FORTIFY_SOURCE=2 -funwind-tables -fasynchronous-unwind-tables -U_FORTIFY_SOURCE -fprofile-use -fprofile-reprod ucible=parallel-runs -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -I. -I. -I../../gcc -I../../gcc/. -I../../gcc/../include -I../../gcc/../libcpp/include -I../../gcc/../libcody -I../../gcc/../libdecnumber -I../../gcc/../libdecnumber/bid -I../libdecnumber -I../../gcc/../libbacktrace -o cgraph.o -MT cgraph.o -MMD -MP -MF ./.deps/cgraph.TPo ../../gcc/cgraph.cc ../../gcc/cgraph.cc: In member function 'cgraph_edge* cgraph_edge::first_speculative_call_target()': ../../gcc/cgraph.cc:1166:1: error: EDGE_CROSSING incorrectly set across same section 1166 | } | ^ ../../gcc/cgraph.cc:1166:1: error: No region crossing jump at section boundary in bb 19 during RTL pass: bbro ../../gcc/cgraph.cc:1166:1: internal compiler error: verify_flow_info failed 0xa7116e verify_flow_info() ../../gcc/cfghooks.cc:284 0x1c64958 execute ../../gcc/bb-reorder.cc:2663 In such a case, what do you need to reproduce it? I'm a mere user of the Suse RPM builds here, no idea if profiling needs any extra data to reproduce to bug. Franz, it's been a long time. Good to hear from you. I'll take a look. I'll start with a profiledbootstrap and if that doesn't reproduce I'll get in contact Yes, long time... Things happened :-) But I'm still following GCC and report bugs as time permits. I've opened 107182 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107182 with the -freport-bug file and cgraph.gcda. I hope this is enough to reproduce. Franz.
Re: [RFA] Minor improvement to coremark, avoid unconditional jump to return
Am 2022-09-25 um 18:28 schrieb Jeff Law: This is a minor improvement for the core_list_find routine in coremark. Basically for riscv, and likely other targets, we can end up with an unconditional jump to a return statement. This is a result of compensation code created by bb-reorder, and no jump optimization pass runs after bb-reorder to clean this stuff up. This patch utilizes preexisting code to identify suitable branch targets as well as preexisting code to emit a suitable return, so it's pretty simple. Note that when we arrange to do this optimization, the original return block may become unreachable. So we conditionally call delete_unreachable_blocks to fix that up. This triggers ~160 times during an x86_64 bootstrap. Naturally it bootstraps and regression tests on x86_64. I've also bootstrapped this on riscv64, regression testing with qemu shows some regressions, but AFAICT they're actually qemu bugs with signal handling/delivery -- qemu user mode emulation is not consistently calling user defined signal handlers. Given the same binary, sometimes they'll get called and the test passes, other times the handler isn't called and the test (of course) fails. I'll probably spend some time to try and chase this down for the sake of making testing easier. OK for the trunk? Hello Jeff, I've bisected this change to break a "profiledbootstrap" on x86_64 like that: make[3]: Entering directory '/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/gcc' /home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/./prev-gcc/xg++ -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/./prev-gcc/ -B/usr/x86_64-suse-linux/ bin/ -nostdinc++ -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/src/.libs -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64 -suse-linux/prev-x86_64-suse-linux/libstdc++-v3/libsupc++/.libs -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/include/x86_64-suse-linu x -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/include -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/libstdc++-v3/libsupc++ -L /home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/src/.libs -L/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x8 6_64-suse-linux/libstdc++-v3/libsupc++/.libs -fno-PIE -c -O2 -g -fmessage-length=0 -D_FORTIFY_SOURCE=2 -funwind-tables -fasynchronous-unwind-tables -U_FORTIFY_SOURCE -fprofile-use -fprofile-reprod ucible=parallel-runs -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -I. -I. -I../../gcc -I../../gcc/. -I../../gcc/../include -I../../gcc/../libcpp/include -I../../gcc/../libcody -I../../gcc/../libdecnumber -I../../gcc/../libdecnumber/bid -I../libdecnumber -I../../gcc/../libbacktrace -o cgraph.o -MT cgraph.o -MMD -MP -MF ./.deps/cgraph.TPo ../../gcc/cgraph.cc ../../gcc/cgraph.cc: In member function 'cgraph_edge* cgraph_edge::first_speculative_call_target()': ../../gcc/cgraph.cc:1166:1: error: EDGE_CROSSING incorrectly set across same section 1166 | } | ^ ../../gcc/cgraph.cc:1166:1: error: No region crossing jump at section boundary in bb 19 during RTL pass: bbro ../../gcc/cgraph.cc:1166:1: internal compiler error: verify_flow_info failed 0xa7116e verify_flow_info() ../../gcc/cfghooks.cc:284 0x1c64958 execute ../../gcc/bb-reorder.cc:2663 In such a case, what do you need to reproduce it? I'm a mere user of the Suse RPM builds here, no idea if profiling needs any extra data to reproduce to bug. Franz.
Re: [PATCH] libgo: Recognize off64_t / loff_t type definition of musl libc
Am 2022-06-21 um 09:34 schrieb Sören Tempel via Gcc-patches: Hi, The problem is: glibc defines loff_t in sys/types.h, not fcntl.h (where musl defines it). I falsely assumed that the newly committed AC_CHECK_TYPES invocation would include fcntl.h *in addition to* AC_INCLUDES_DEFAULT. However, as it turns out specifying includes for AC_CHECK_TYPES overwrites the default instead of appending to it. The patch below should fix this by appending to AC_INCLUDES_DEFAULT explicitly. Alternatively, we could try to add fcntl.h to AC_INCLUDES_DEFAULT, though my autotools knowledge is severely limited and hence I am not sure how this would be achieved. diff --git a/libgo/configure b/libgo/configure index b7ff9b3..273af1d 100755 --- a/libgo/configure +++ b/libgo/configure @@ -15549,8 +15549,10 @@ fi CFLAGS_hold="$CFLAGS" CFLAGS="$OSCFLAGS $CFLAGS" -ac_fn_c_check_type "$LINENO" "loff_t" "ac_cv_type_loff_t" "#include -" +ac_fn_c_check_type "$LINENO" "loff_t" "ac_cv_type_loff_t" " +$ac_includes_default +#include + " if test "x$ac_cv_type_loff_t" = xyes; then : cat >>confdefs.h <<_ACEOF diff --git a/libgo/configure.ac b/libgo/configure.ac index bac58b0..b237392 100644 --- a/libgo/configure.ac +++ b/libgo/configure.ac @@ -604,7 +604,9 @@ AC_TYPE_OFF_T CFLAGS_hold="$CFLAGS" CFLAGS="$OSCFLAGS $CFLAGS" -AC_CHECK_TYPES([loff_t], [], [], [[#include ]]) +AC_CHECK_TYPES([loff_t], [], [], [ +AC_INCLUDES_DEFAULT +#include ]) CFLAGS="$CFLAGS_hold" LIBS_hold="$LIBS" Hi, the patch restores bootstrap for me on x86_64-suse-linux. Franz.
Re: [PATCH v2] c++: Check attributes on friend declarations [PR99032]
Am 2021-05-14 um 00:08 schrieb Marek Polacek via Gcc-patches: On Wed, May 12, 2021 at 08:27:18PM -0400, Jason Merrill wrote: On 5/12/21 8:03 PM, Marek Polacek wrote: diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c index 89f874a32cc..2bcefb619aa 100644 --- a/gcc/cp/decl2.c +++ b/gcc/cp/decl2.c @@ -1331,6 +1331,20 @@ any_dependent_type_attributes_p (tree attrs) return false; } +/* True if ATTRS contains any attribute that requires a type. */ Let's invert this to check if ATTRS contains any attribute that does *not* require a type, and would therefore apply to the decl. Sounds good, done. Now I don't need to check *attrlist. I've also fixed up the xfail thing in my new test. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- This patch implements [dcl.attr.grammar]/5: "If an attribute-specifier-seq appertains to a friend declaration ([class.friend]), that declaration shall be a definition." This restriction applies to C++11-style attributes as well as GNU attributes with the exception that we allow GNU attributes that require a type, such as vector_size to continue accepting code as in attrib63.C. There are various forms of friend declarations, we have friend templates, C++11 extended friend declarations, and so on. In some cases we already ignore the attribute and warn that it was ignored. But certain cases weren't diagnosed, and with this patch we'll give a hard error. I tried hard not to emit both a warning and error and I think it worked out. Jason provided the cp_parser_decl_specifier_seq hunk to detect using standard attributes in the middle of decl-specifiers, which is invalid. Co-authored-by: Jason Merrill gcc/cp/ChangeLog: PR c++/99032 * cp-tree.h (any_non_type_attribute_p): Declare. * decl.c (grokdeclarator): Diagnose when an attribute appertains to a friend declaration that is not a definition. * decl2.c (any_non_type_attribute_p): New. * parser.c (cp_parser_decl_specifier_seq): Diagnose standard attributes in the middle of decl-specifiers. (cp_parser_elaborated_type_specifier): Diagnose when an attribute appertains to a friend declaration that is not a definition. (cp_parser_member_declaration): Likewise. Hi, I haven't investigated it in detail yet, but it seems this change breaks building Qt-based (tested with Qt-5.12.7) applications. Sample error output with trunk@r12+876 -std=gnu++14: /usr/include/qt5/QtCore/qvariant.h:470:33: error: attribute appertains to a friend declaration that is not a definition friend Q_CORE_EXPORT QDebug operator<<(QDebug, const QVariant &); ^~~~ For GCC Q_CORE_EXPORT is defined (via Q_DECL_EXPORT) to __attribute__((visibility("default"))) AFAICS. As this error seemingly cannot be turned into a warning, it's probably quite a problem for many people. regards, Franz
Re: [PATCH] run -Wnonnull later (PR 87489)
Am 2021-02-01 um 01:31 schrieb Martin Sebor via Gcc-patches: The initial -Wnonnull implementation in the middle end took place too late in the pipeline (just before expansion), and as a result was prone to false positives (bug 78817). In an attempt to avoid the worst of those, the warning was moved to the ccp2 pass in r243874. However, as the test case in PR 87489 shows, this is in turn too early and causes other false positives as a result. A few experiments with running the warning later suggest that just before the mergephi2 pass is a good point to avoid this class of false positives without causing any regressions or introducing any new warnings either in Glibc or in Binutils/GDB. Since PR 87489 is a GCC 8-11 regression I propose to make this change on the trunk as well as on the release branches. Hi Martin, I tested your patch and it showed also one more warning for this testcase with -O2 -Wnonnull: class b { public: long c(); }; class B { public: B() : f() {} b *f; }; long d, e; class g : B { public: void h() { long a = f->c(); d = e = a; } }; class j { j(); g i; }; j::j() { i.h(); } I hope cvise didn't minimize too much, but at least in the original much larger code the warning looks reasonable too. Franz
Re: [committed] i386: Use lock prefixed insn instead of MFENCE [PR95750]
Am 2020-07-20 um 20:39 schrieb Uros Bizjak via Gcc-patches: Currently, __atomic_thread_fence(seq_cst) on x86 and x86-64 generates mfence instruction. A dummy atomic instruction (a lock-prefixed instruction or xchg with a memory operand) would provide the same sequential consistency guarantees while being more efficient on most current CPUs. The mfence instruction additionally orders non-temporal stores, which is not relevant for atomic operations and are not ordered by seq_cst atomic operations anyway. 2020-07-20 Uroš Bizjak gcc/ChangeLog: PR target/95750 * config/i386/i386.h (TARGET_AVOID_MFENCE): Rename from TARGET_USE_XCHG_FOR_ATOMIC_STORE. * config/i386/sync.md (mfence_sse2): Disable for TARGET_AVOID_MFENCE. (mfence_nosse): Enable also for TARGET_AVOID_MFENCE. Emit stack referred memory in word_mode. (mem_thread_fence): Do not generate mfence_sse2 pattern when TARGET_AVOID_MFENCE is true. (atomic_store): Update for rename. * config/i386/x86-tune.def (X86_TUNE_AVOID_MFENCE): Rename from X86_TUNE_USE_XCHG_FOR_ATOMIC_STORE. gcc/testsuite/ChangeLog: PR target/95750 * gcc.target/i386/pr95750.c: New test. Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. Uros. Hi, I didn't bisect it, but I see a profiledbootstrap ICE that may be related: libtool: compile: /home/fsirl/rpmbuild/BUILD/gcc-11.0.0+gitr11+2246/obj-x86_64-suse-linux/./gcc/xgcc -B/home/fsirl/rpmbuild/BUILD/gcc-11.0.0+gitr11+2246/obj-x86_64-suse-linux/./gcc/ -B/usr/x86_64-su se-linux/bin/ -B/usr/x86_64-suse-linux/lib/ -isystem /usr/x86_64-suse-linux/include -isystem /usr/x86_64-suse-linux/sys-include -m32 -DHAVE_CONFIG_H -I. -I../../../../libgo -I ../../../../libgo/runti me -I../../../../libgo/../libffi/include -I../libffi/include -pthread -L../libatomic/.libs -fexceptions -fnon-call-exceptions -fsplit-stack -Wall -Wextra -Wwrite-strings -Wcast-qual -minline-all-stri ngops -D_GNU_SOURCE -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I ../../../../libgo/../libgcc -I ../../../../libgo/../libbacktrace -I ../../../gcc/include -O2 -g -fmessage-length=0 -D_FORTIFY_SOURCE= 2 -funwind-tables -fasynchronous-unwind-tables -U_FORTIFY_SOURCE -m32 -MT runtime/runtime_c.lo -MD -MP -MF runtime/.deps/runtime_c.Tpo -c ../../../../libgo/runtime/runtime_c.c -fPIC -DPIC -o runtime /.libs/runtime_c.o ../../../../libgo/runtime/runtime_c.c: In function ?runtime_cputicks?: ../../../../libgo/runtime/runtime_c.c:102:1: error: unrecognizable insn: 102 | } | ^ (insn 20 19 21 6 (set (mem/v:BLK (scratch:SI) [0 A8]) (unspec:BLK [ (mem/v:BLK (scratch:SI) [0 A8]) ] UNSPEC_MFENCE)) "../../../../libgo/runtime/runtime_c.c":84:7 -1 (nil)) during RTL pass: vregs ../../../../libgo/runtime/runtime_c.c:102:1: internal compiler error: in extract_insn, at recog.c:2294 This is on a Xeon X5650 machine. Franz
Re: [PATCH] Add missing store in emission of asan_stack_free.
Am 2020-05-19 um 21:05 schrieb Martin Liška: Hi. We make direct emission for asan_emit_stack_protection for smaller stacks. That's fine but we're missing the piece that marks the stack as released and we run out of pre-allocated stacks. I also included some stack-related constants that were used in asan.c. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin gcc/ChangeLog: 2020-05-19 Martin Liska PR sanitizer/94910 * asan.c (asan_emit_stack_protection): Emit also **SavedFlagPtr(FakeStack) = 0 in order to release a stack frame. * asan.h (ASAN_MIN_STACK_FRAME_SIZE_LOG): New. (ASAN_MAX_STACK_FRAME_SIZE_LOG): Likewise. (ASAN_MIN_STACK_FRAME_SIZE): Likewise. (ASAN_MAX_STACK_FRAME_SIZE): Likewise. --- gcc/asan.c | 26 ++ gcc/asan.h | 8 2 files changed, 30 insertions(+), 4 deletions(-) >- if (asan_frame_size > 32 && asan_frame_size <= 65536 && pbase >+ if (asan_frame_size >= ASAN_MIN_STACK_FRAME_SIZE Hi, is the change from > to >= and from 32 to 64 for ASAN_MIN_STACK_FRAME_SIZE intentional? Just asking because it doesn't look obvious from Changelog or patch. Also a few lines below the "5" in use_after_return_class = floor_log2 (asan_frame_size - 1) - 5; looks like it may be related to ASAN_MIN_STACK_FRAME_SIZE_LOG. regards, Franz
[PATCH] Alias -Warray-bounds to -Warray-bounds=1
Hi, as discussed with Martin, this patch consolidates -Warray-bounds into an alias of -Warray-bounds=1. Bootstrapped on x86_64-linux, no regressions. Please apply if it's OK. Franz. gcc/ChangeLog: 2018-07-25 Franz Sirl * common.opt: Alias -Warray-bounds to -Warray-bounds=1. * builtins.c (c_strlen): Use OPT_Warray_bounds_. * gimple-ssa-warn-restrict.c (maybe_diag_offset_bounds): Likewise. * tree-vrp.c (vrp_prop::check_array_ref, vrp_prop::check_mem_ref, vrp_prop::search_for_addr_array): Likewise. gcc/c-family/ChangeLog: 2018-07-25 Franz Sirl * c.opt: Remove -Warray-bounds. * c-common.c (fold_offsetof, convert_vector_to_array_for_subscript): Use OPT_Warray_bounds_. Index: gcc/builtins.c === --- gcc/builtins.c (revision 262966) +++ gcc/builtins.c (working copy) @@ -675,7 +675,7 @@ c_strlen (tree src, int only_value) if (only_value != 2 && !TREE_NO_WARNING (src)) { - warning_at (loc, OPT_Warray_bounds, + warning_at (loc, OPT_Warray_bounds_, "offset %qwi outside bounds of constant string", eltoff); TREE_NO_WARNING (src) = 1; Index: gcc/c-family/c-common.c === --- gcc/c-family/c-common.c (revision 262966) +++ gcc/c-family/c-common.c (working copy) @@ -6257,7 +6257,7 @@ fold_offsetof (tree expr, tree type, enum tree_cod definition thereof. */ if (TREE_CODE (v) == ARRAY_REF || TREE_CODE (v) == COMPONENT_REF) - warning (OPT_Warray_bounds, + warning (OPT_Warray_bounds_, "index %E denotes an offset " "greater than size of %qT", t, TREE_TYPE (TREE_OPERAND (expr, 0))); @@ -7662,7 +7662,7 @@ convert_vector_to_array_for_subscript (location_t if (TREE_CODE (index) == INTEGER_CST) if (!tree_fits_uhwi_p (index) || maybe_ge (tree_to_uhwi (index), TYPE_VECTOR_SUBPARTS (type))) - warning_at (loc, OPT_Warray_bounds, "index value is out of bound"); + warning_at (loc, OPT_Warray_bounds_, "index value is out of bound"); /* We are building an ARRAY_REF so mark the vector as addressable to not run into the gimplifiers premature setting of DECL_GIMPLE_REG_P Index: gcc/c-family/c.opt === --- gcc/c-family/c.opt (revision 262966) +++ gcc/c-family/c.opt (working copy) @@ -326,10 +326,6 @@ Wno-alloca-larger-than C ObjC C++ LTO ObjC++ Alias(Walloca-larger-than=,18446744073709551615EiB,none) Warning -Wno-alloca-larger-than Disable Walloca-larger-than= warning. Equivalent to Walloca-larger-than= or larger. -Warray-bounds -LangEnabledBy(C ObjC C++ LTO ObjC++) -; in common.opt - Warray-bounds= LangEnabledBy(C ObjC C++ LTO ObjC++,Wall,1,0) ; in common.opt Index: gcc/common.opt === --- gcc/common.opt (revision 262966) +++ gcc/common.opt (working copy) @@ -539,8 +539,7 @@ Common Var(warn_aggressive_loop_optimizations) Ini Warn if a loop with constant number of iterations triggers undefined behavior. Warray-bounds -Common Var(warn_array_bounds) Warning -Warn if an array is accessed out of bounds. +Common Warning Alias(Warray-bounds=,1,0) Warray-bounds= Common Joined RejectNegative UInteger Var(warn_array_bounds) Warning IntegerRange(0, 2) Index: gcc/gimple-ssa-warn-restrict.c === --- gcc/gimple-ssa-warn-restrict.c (revision 262966) +++ gcc/gimple-ssa-warn-restrict.c (working copy) @@ -1619,7 +1619,7 @@ maybe_diag_offset_bounds (location_t loc, gcall *c if (DECL_P (ref.base) && TREE_CODE (type = TREE_TYPE (ref.base)) == ARRAY_TYPE) { - if (warning_at (loc, OPT_Warray_bounds, + if (warning_at (loc, OPT_Warray_bounds_, "%G%qD pointer overflow between offset %s " "and size %s accessing array %qD with type %qT", call, func, rangestr[0], rangestr[1], ref.base, type)) @@ -1629,13 +1629,13 @@ maybe_diag_offset_bounds (location_t loc, gcall *c warned = true; } else - warned = warning_at (loc, OPT_Warray_bounds, + warned = warning_at (loc, OPT_Warray_bounds_, "%G%qD pointer overflow between offset %s " "and size %s", call, func, rangestr[0], rangestr[1]); }
Re: committed: remove redundant -Wall from -Warray-bounds (PR 82063)
Am 2018-07-24 um 17:35 schrieb Martin Sebor: On 07/24/2018 03:24 AM, Franz Sirl wrote: Am 2018-07-20 um 23:22 schrieb Martin Sebor: As the last observation in PR 82063 Jim points out that Both -Warray-bounds and -Warray-bounds= are listed in the c.opt file as being enabled by -Wall, but they are the same option, and it causes this one option to be processed twice in the C_handle_option_auto function in the generated options.c file. It gets set to the same value twice, so it does work as intended, but this is wasteful. I have removed the redundant -Wall from the first option and committed the change as obvious in r262912. Hi Martin, this looks related to PR 68845 and my patch in there. I never posted it to gcc-patches because I couldn't find a definitive answer on how options duplicated between common.opt and c-family/c.opt are supposed to be handled. For example, Warray-bounds in common.opt is a separate option (not an alias to Warray-bounds=), leading to separate enums for them. Is this intended? Warray-bounds seemed to be the only option with an equal sign doing it like that at that time. Now Wcast-align is doing the same... Can you shed some light on this? -Warray-bounds= (the form that takes an argument) was added in r219577. Before then, only the plain form existed. If I had to guess, the interplay between the two options (as opposed to making the latter an alias for the new option) wasn't considered. I didn't think of it until now either. Your patch seems like the right solution to me. Let me know if you will submit it. If not, I posted the patch below that touches this area and that will likely need updating so I can roll your change into it: https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01286.html I'll post a patch tomorrow, since I already have all the changes available and tested here. Note that one minor change with this patch is that with -fdiagnostics-show-option the message will show -Warray-bounds= (equal sign added) instead of -Warray-bounds. Franz
Re: committed: remove redundant -Wall from -Warray-bounds (PR 82063)
Am 2018-07-20 um 23:22 schrieb Martin Sebor: As the last observation in PR 82063 Jim points out that Both -Warray-bounds and -Warray-bounds= are listed in the c.opt file as being enabled by -Wall, but they are the same option, and it causes this one option to be processed twice in the C_handle_option_auto function in the generated options.c file. It gets set to the same value twice, so it does work as intended, but this is wasteful. I have removed the redundant -Wall from the first option and committed the change as obvious in r262912. Hi Martin, this looks related to PR 68845 and my patch in there. I never posted it to gcc-patches because I couldn't find a definitive answer on how options duplicated between common.opt and c-family/c.opt are supposed to be handled. For example, Warray-bounds in common.opt is a separate option (not an alias to Warray-bounds=), leading to separate enums for them. Is this intended? Warray-bounds seemed to be the only option with an equal sign doing it like that at that time. Now Wcast-align is doing the same... Can you shed some light on this? Franz
Re: backporting fix for 85602 to GCC 8
Am 2018-07-18 um 01:50 schrieb Martin Sebor: If there are no objections I'd like to backport the solution for PR 85602 to avoid a class of unnecessary warnings for safe uses of nonstring arrays. With the release coming up later this week I'll go ahead and commit the patch tomorrow. https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=261718 Hi Martin, and please remember the follow-up fix https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=261751 The patch for PR 85602 makes the extended and enabled-by-Wall string warnings (which I like!) complete. There's a warning for the majority of cases and for the char-array-without-NUL cases there is the nonstring attribute describing it nicely, much better than to turn off the warning around such code. I know that probably not too many codebases will be affected, but for anyone affected the nonstring attribute is a much better way to avoid the warnings than to turn it off (and if they turn off the warnings for gcc-8 they often won't turn it on again for gcc-9+). The nonstring attribute is also the documented way to silence the warnings. BTW, while re-reading the documentation I noticed some minor omissions, I attached a patch (untested). Feel free to commit it (I have no access) if you think it's correct. Franz. 2018-07-12 Franz Sirl * invoke.texi (Wstringop-overflow, Wstringop-truncation): Mention enabling via -Wall. (Wall): Add -Wstringop-overflow02 and -Wstringop-truncation. Index: invoke.texi === diff --git a/trunk/gcc/doc/invoke.texi b/trunk/gcc/doc/invoke.texi --- a/trunk/gcc/doc/invoke.texi (revision 262850) +++ b/trunk/gcc/doc/invoke.texi (working copy) @@ -3992,6 +3992,8 @@ -Wsizeof-pointer-memaccess @gol -Wstrict-aliasing @gol -Wstrict-overflow=1 @gol +-Wstringop-overflow=2 @gol +-Wstringop-truncation @gol -Wswitch @gol -Wtautological-compare @gol -Wtrigraphs @gol @@ -5318,7 +5320,7 @@ @} @end smallexample -Option @option{-Wstringop-overflow=2} is enabled by default. +Option @option{-Wstringop-overflow=2} is enabled by @option{-Wall}. @table @gcctabopt @item -Wstringop-overflow @@ -5416,6 +5418,8 @@ such arrays GCC issues warnings unless it can prove that the use is safe. @xref{Common Variable Attributes}. +Option @option{-Wstringop-truncation} is enabled by @option{-Wall}. + @item -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{|}cold@r{|}malloc@r{]} @opindex Wsuggest-attribute= @opindex Wno-suggest-attribute=
Re: Have g++ define _FILE_OFFSET_BITS=64 on Solaris
Am 2018-06-25 um 15:57 schrieb Rainer Orth: Hi Franz, so you are supposed to use "-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64", but at least a quick glance at the Sol10 headers shows that the additional -D_LARGEFILE_SOURCE only makes a difference for fseeko/ftello. That still right, that's also explained in lfcompile(7). doesn't explain -D_LARGEFILE64_SOURCE, does libstdc++ really need to use _LARGEFILE64_SOURCE functions? Honestly, I hadn't checked, but wondered the same thing. However, I'm a bit wary to simply remove them after years for fear of breaking user existing user code. But adding _FILE_OFFSET_BITS=64 is the far bigger change for the user. Now suddenly (for 32-bit applications) off_t changes size and thus many applications with mixed C/C++-code simply might break. The reason is that now (if they didn't take care of _LARGEFILE_SOURCE themselves), for example fread() really does a fread64() in the C++ parts and a fread() (the 32-bit version) in the C parts. This situation was avoided before by enabling _LARGEFILE_SOURCE without _FILE_OFFSET_BITS=64. Re-reading lfcompile(7) again shows that you can use either "-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64" (for portable applications) or only "-D_FILE_OFFSET_BITS=64". But in the GCC case we only need it for C++/libstdc++ so it seems "-D_FILE_OFFSET_BITS=64" should be enough. The rest is up to the users application, or? One might argue that way, but again it's a bit late to change this now for no compelling reason. The compelling reason is that with _FILE_OFFSET_BITS=64 all bets are off anyway IMO, see above. My guess is that without defining _LARGEFILE_SOURCE and _LARGEFILE64_SOURCE the configure check in libstdc++-v3/acinclude.m4 just won't define _GLIBCXX_USE_LFS and everything will fall in place. This would leave HPUX as the last user of _GLIBCXX_USE_LFS. I don't know about HP-UX, but _GLIBC_USE_LFS is defined on Linux/x86_64, too. To me, the meaning seems a bit confused: 27_io/fpos/14775.cc suggests that it denotes all system with largefile support, while acinclude.m4 (GLIBCXX_CHECK_LFS) only tests for the transitional functions (enabled by _LARGEFILE64_SOURCE on Solaris) while ignoring the `transparent' largefile support from _LARGEFILE_SOURCE _FILE_OFFSET_BITS=64. I'd rather not mess with this stuff. That I can fully understand ;-). Maybe a solution is to define the macros also for C, not only for C++. And to conditionalize _LARGEFILE_SOURCE on 32-bit compile and _LARGEFILE64_SOURCE on 64-bit compile to make it at least less confusing. Franz
Re: Have g++ define _FILE_OFFSET_BITS=64 on Solaris
Am 2018-06-22 um 09:51 schrieb Rainer Orth: Hi Franz, No idea about possible problems, but isn't it usually recommended to use either _FILE_OFFSET_BITS=64 or _LARGEFILE{64}_SOURCE=1, not both at the same time? quite the contrary: for regular largefile support, you're supposed to use `getconf LFS_CFLAGS', i.e. -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64, while `getconf LFS64_CFLAGS' (-D_LARGEFILE64_SOURCE) enables the transitional largefile interfaces (e.g. explicit stat64 calls and struct stat64 instead of making stat and struct stat largefile-aware). For all the gory details, see the lfcompile(7), lfcompile64(7), and lf64(7) man pages: https://docs.oracle.com/cd/E88353_01/html/E37853/index.html Rainer Hi Rainer, so you are supposed to use "-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64", but at least a quick glance at the Sol10 headers shows that the additional -D_LARGEFILE_SOURCE only makes a difference for fseeko/ftello. That still doesn't explain -D_LARGEFILE64_SOURCE, does libstdc++ really need to use _LARGEFILE64_SOURCE functions? Re-reading lfcompile(7) again shows that you can use either "-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64" (for portable applications) or only "-D_FILE_OFFSET_BITS=64". But in the GCC case we only need it for C++/libstdc++ so it seems "-D_FILE_OFFSET_BITS=64" should be enough. The rest is up to the users application, or? My guess is that without defining _LARGEFILE_SOURCE and _LARGEFILE64_SOURCE the configure check in libstdc++-v3/acinclude.m4 just won't define _GLIBCXX_USE_LFS and everything will fall in place. This would leave HPUX as the last user of _GLIBCXX_USE_LFS. Franz
Re: Have g++ define _FILE_OFFSET_BITS=64 on Solaris
Am 2018-06-21 um 16:17 schrieb Rainer Orth: I recently found two libstdc++ testcases failing on some Solaris hosts for 32-bit only: FAIL: 27_io/filesystem/operations/space.cc execution test FAIL: experimental/filesystem/operations/space.cc execution test Both file in the same way: terminate called after throwing an instance of 'std::filesystem::__cxx11::filesystem_error' what(): filesystem error: cannot get free space: Value too large for defined data type [.] However, the test PASSes just fine on other systems. It turns out that the tests FAIL with statvfs(".", 0xFEFFDB64)Err#79 EOVERFLOW On the failing system, the build filesystem is 3.4 TB, thus the EOVERFLOW. It seems g++ on Solaris doesn't fully enable largefile support: it has -D_LARGEFILE_SOURCE=1 in gcc/config/sol2.h (TARGET_OS_CPP_BUILTINS), but lacks -D_FILE_OFFSET_BITS=64 which is required to get the largefile-aware functions (statvfs64 in this case). The following patch adds that, fixing the two failures. Bootstrapped without regressions on i386-pc-solaris2.1[01] and sparc-sun-solaris2.1[01]. Unless someone has an idea why this might cause problems, I'll install the patch on mainline and backport to the gcc-7 and gcc-8 branches. No idea about possible problems, but isn't it usually recommended to use either _FILE_OFFSET_BITS=64 or _LARGEFILE{64}_SOURCE=1, not both at the same time? Franz
Re: [RFC][PATCH] Stabilize a few qsort comparison functions
Am 2018-06-12 um 23:49 schrieb Jeff Law: On 02/07/2018 09:58 AM, Franz Sirl wrote: Hi, this is the result of an attempt to minimize the differences between the compile results of a Linux-based and a Cygwin64-based powerpc-eabi cross toolchain. The method used was: - find the -fverbose-asm assembler files that differ - compile that file again on both platforms with -O2 -g3 -fdump-tree-all-all -fdump-rtl-all -fdump-noaddr - look for the first dump file with differences and check that pass for qsort's - stabilize the compare functions With some help on IRC to better understand the passes and some serious debugging of GCC I came up with this patch. On the tested codebase the differences in the assembler sources are now down to 0. If the various pass maintainers have better ideas on how to stabilize the compare functions, I'll be happy to verify them on the codebase. For the SRA patch I already have an alternate version with an additional ID member. Comments? Bootstrapped on linux-x86_64, no testsuite regressions. Franz Sirl 2018-02-07 Franz Sirl * ira-build.c (object_range_compare_func): Stabilize sort. * tree-sra.c (compare_access_positions): Likewise. * varasm.c (output_object_block_compare): Likewise. * tree-ssa-loop-ivopts.c (group_compare_offset): Likewise. (struct iv_common_cand): New member. (record_common_cand): Initialize new member. (common_cand_cmp): Use new member to stabilize sort. * tree-vrp.c (struct assert_locus): New member. (register_new_assert_for): Initialize new member. (compare_assert_loc): Use new member to stabilize sort. This looks pretty reasonable. I don't think you've contributed much recently, do you still have write access to the repository? Hi Jeff, after Alexander Monakov's gcc_qsort changes, this patch is not necessary anymore. I've verified that with a backport (the 2 patches r260216 and r260222 applied cleanly) of gcc_qsort to the gcc-8-branch. The resulting powerpc-eabi crosscompilers produce no more unexpected differences between a Linux and a Cygwin host. Tested (same like with my patch) by comparing the -fverbose-asm assembly output on a complete rebuild of the software here. So, unless someone thinks one of the changes makes sense anyway, this patch is obsolete. On the repository write access, yes, I don't have one anymore. But before reactivating that I need to do the legal paperwork, because unless before when GCC was a strictly private pet project for me, it now is work related. I already got permission from my company for that, just need to find some spare time to start the legal stuff. Franz
Re: [PATCH] use string length to relax -Wstringop-overflow for nonstrings (PR 85623)
Am 2018-05-10 um 21:26 schrieb Martin Sebor: GCC 8.1 warns for unbounded (and some bounded) string comparisons involving arrays declared attribute nonstring (i.e., char arrays that need not be nul-terminated). For instance: extern __attribute__((nonstring)) char a[4]; int f (void) { return strncmp (a, "123", sizeof a); } warning: ‘strcmp’ argument 1 declared attribute ‘nonstring’ Note that the warning refers to strcmp even though the call in the source is to strncmp, because prior passes transform one to the other. The warning above is unnecessary (for strcmp) and incorrect for strncmp because the call reads exactly four bytes from the non- string array a regardless of the bound and so there is no risk that it will read past the end of the array. The attached change enhances the warning to use the length of the string argument to suppress some of these needless warnings for both bounded and unbounded string comparison functions. When the length of the string is unknown, the warning uses its size (when possible) as the upper bound on the number of accessed bytes. The change adds no new warnings. I'm looking for approval to commit it to both trunk and 8-branch. Hi Martin, this patch is a nice improvement and makes "nonstring" a lot more useable. But shouldn't the attribute also handle these cases (similar to PR 85602): #include typedef struct { char segname[16] __attribute__((__nonstring__)); char sectname[16] __attribute__((__nonstring__)); } MachO_header; const char *get_seg_sect(MachO_header *hdr) { static char segname_sectname[sizeof(hdr->segname) + sizeof(hdr->sectname) + 2]; memset(segname_sectname, 0, sizeof(segname_sectname)); strncpy(segname_sectname, hdr->segname, sizeof(hdr->segname)); strcat(segname_sectname, "@"); strncat(segname_sectname, hdr->sectname, sizeof(hdr->sectname)); return segname_sectname; } $ gcc-8 -c -O2 -W -Wall test-macho.c In file included from /usr/include/string.h:630, from test-macho.c:1: test-macho.c: In function 'get_seg_sect': test-macho.c:14:48: warning: argument to 'sizeof' in '__builtin_strncpy' call is the same expression as the source; did you mean to use the size of the destination? [-Wsizeof-pointer-memaccess] strncpy(segname_sectname, hdr->segname, sizeof(hdr->segname)); ^ test-macho.c:16:49: warning: argument to 'sizeof' in '__builtin_strncat' call is the same expression as the source; did you mean to use the size of the destination? [-Wsizeof-pointer-memaccess] strncat(segname_sectname, hdr->sectname, sizeof(hdr->sectname)); ^ As you can see __attribute__((__nonstring__)) doesn't silence the warning. Franz.
[RFC][PATCH] Stabilize a few qsort comparison functions
Hi, this is the result of an attempt to minimize the differences between the compile results of a Linux-based and a Cygwin64-based powerpc-eabi cross toolchain. The method used was: - find the -fverbose-asm assembler files that differ - compile that file again on both platforms with -O2 -g3 -fdump-tree-all-all -fdump-rtl-all -fdump-noaddr - look for the first dump file with differences and check that pass for qsort's - stabilize the compare functions With some help on IRC to better understand the passes and some serious debugging of GCC I came up with this patch. On the tested codebase the differences in the assembler sources are now down to 0. If the various pass maintainers have better ideas on how to stabilize the compare functions, I'll be happy to verify them on the codebase. For the SRA patch I already have an alternate version with an additional ID member. Comments? Bootstrapped on linux-x86_64, no testsuite regressions. Franz Sirl 2018-02-07 Franz Sirl * ira-build.c (object_range_compare_func): Stabilize sort. * tree-sra.c (compare_access_positions): Likewise. * varasm.c (output_object_block_compare): Likewise. * tree-ssa-loop-ivopts.c (group_compare_offset): Likewise. (struct iv_common_cand): New member. (record_common_cand): Initialize new member. (common_cand_cmp): Use new member to stabilize sort. * tree-vrp.c (struct assert_locus): New member. (register_new_assert_for): Initialize new member. (compare_assert_loc): Use new member to stabilize sort. Index: gcc/ira-build.c === --- gcc/ira-build.c (revision 257438) +++ gcc/ira-build.c (working copy) @@ -2787,8 +2787,10 @@ object_range_compare_func (const void *v1p, const if ((diff = OBJECT_MIN (obj1) - OBJECT_MIN (obj2)) != 0) return diff; if ((diff = OBJECT_MAX (obj1) - OBJECT_MAX (obj2)) != 0) - return diff; - return ALLOCNO_NUM (a1) - ALLOCNO_NUM (a2); +return diff; + if ((diff = ALLOCNO_NUM (a1) - ALLOCNO_NUM (a2)) != 0) +return diff; + return OBJECT_SUBWORD (obj1) - OBJECT_SUBWORD (obj2); } /* Sort ira_object_id_map and set up conflict id of allocnos. */ Index: gcc/tree-sra.c === --- gcc/tree-sra.c (revision 257438) +++ gcc/tree-sra.c (working copy) @@ -1567,7 +1567,13 @@ compare_access_positions (const void *a, const voi if (f1->size == f2->size) { if (f1->type == f2->type) - return 0; + { + if (f1->stmt == f2->stmt) + return 0; + if (f1->stmt && f2->stmt) + return gimple_uid (f1->stmt) - gimple_uid (f2->stmt); + return f1->stmt ? -1 : 1; + } /* Put any non-aggregate type before any aggregate type. */ else if (!is_gimple_reg_type (f1->type) && is_gimple_reg_type (f2->type)) Index: gcc/tree-ssa-loop-ivopts.c === --- gcc/tree-ssa-loop-ivopts.c (revision 257438) +++ gcc/tree-ssa-loop-ivopts.c (working copy) @@ -443,6 +443,7 @@ struct iv_common_cand /* IV uses from which this common candidate is derived. */ auto_vec uses; hashval_t hash; + unsigned id; }; /* Hashtable helpers. */ @@ -2617,8 +2618,11 @@ group_compare_offset (const void *a, const void *b { const struct iv_use *const *u1 = (const struct iv_use *const *) a; const struct iv_use *const *u2 = (const struct iv_use *const *) b; + int diff; - return compare_sizes_for_sort ((*u1)->addr_offset, (*u2)->addr_offset); + if ((diff = compare_sizes_for_sort ((*u1)->addr_offset, (*u2)->addr_offset)) != 0) +return diff; + return (*u1)->id - (*u2)->id; } /* Check if small groups should be split. Return true if no group @@ -3378,6 +3382,7 @@ record_common_cand (struct ivopts_data *data, tree struct iv_common_cand ent; struct iv_common_cand **slot; + ent.id = data->iv_common_cands.length (); ent.base = base; ent.step = step; ent.hash = iterative_hash_expr (base, 0); @@ -3387,6 +3392,7 @@ record_common_cand (struct ivopts_data *data, tree if (*slot == NULL) { *slot = new iv_common_cand (); + (*slot)->id = ent.id; (*slot)->base = base; (*slot)->step = step; (*slot)->uses.create (8); @@ -3412,6 +3418,8 @@ common_cand_cmp (const void *p1, const void *p2) n1 = (*ccand1)->uses.length (); n2 = (*ccand2)->uses.length (); + if (n1 == n2) +return (*ccand1)->id - (*ccand2)->id; return n2 - n1; } Index: gcc/tree-vrp.c === --- gcc/tree-vrp.c (revision 257438) +++ gcc/tree-vrp.c (working copy) @@ -101,6 +101,8 @@ struct assert_locus /* Predicate code for the ASSERT_EXPR. Must be COMPARISON_CLA
[PATCH] Minor warning option sync with clang
Hi, a mail in the gcc-list https://gcc.gnu.org/ml/gcc/2018-01/msg00144.html reminded me about this minor stuff I have (so it can be controlled by -Werror), but never managed to write testcases. I'm posting it here in case it's good enough or someone wants to take over from here. Franz c/ChangeLog; 2018-01-22 Franz Sirl * c-decl.c (grokdeclarator): Use OPT_Wextern_initializer. * c-typeck.c (build_binary_op): Use OPT_Wcompare_distinct_pointer_types. c-family/ChangeLog: 2018-01-22 Franz Sirl * c.opt (-Wcompare-distinct-pointer-types): New option. (-Wextern-initializer): Likewise. cp/ChangeLog: 2018-01-22 Franz Sirl * decl.c (grokdeclarator): Use OPT_Wextern_initializer. * typeck.c (composite_pointer_error): Use OPT_Wcompare_distinct_pointer_types. Index: gcc-trunk/gcc/c-family/c.opt === --- gcc-trunk/gcc/c-family/c.opt(revision 256939) +++ gcc-trunk/gcc/c-family/c.opt(working copy) @@ -318,10 +318,6 @@ alloca, and on bounded uses of alloca whose bound can be larger than bytes. -Warray-bounds -LangEnabledBy(C ObjC C++ ObjC++,Wall) -; in common.opt - Warray-bounds= LangEnabledBy(C ObjC C++ ObjC++,Wall,1,0) ; in common.opt @@ -420,6 +416,10 @@ C ObjC C++ ObjC++ Warning Alias(Wcomment) Synonym for -Wcomment. +Wcompare-distinct-pointer-types +C ObjC C++ ObjC++ Var(warn_distinct_pointer_types) Init(1) Warning +Warn for comparison of distinct pointer types. + Wconditionally-supported C++ ObjC++ Var(warn_conditionally_supported) Warning Warn for conditionally-supported constructs. @@ -512,6 +512,10 @@ C ObjC RejectNegative Warning Alias(Werror=, implicit-function-declaration) This switch is deprecated; use -Werror=implicit-function-declaration instead. +Wextern-initializer +C ObjC C++ ObjC++ Var(warn_extern_initializer) Init(1) Warning +Warn about extern declarations with an initializer. + Wextra C ObjC C++ ObjC++ Warning ; in common.opt @@ -694,6 +698,10 @@ C C++ Common Var(warn_misleading_indentation) Warning LangEnabledBy(C C++,Wall) Warn when the indentation of the code does not reflect the block structure. +Wmisleading-indentation-ltb +C C++ Common Var(warn_misleading_indentation_ltb) Warning +Warn when the indentation of the code does not reflect the block structure. + Wmissing-braces C ObjC C++ ObjC++ Var(warn_missing_braces) Warning LangEnabledBy(C ObjC,Wall) Warn about possibly missing braces around initializers. Index: gcc-trunk/gcc/c/c-decl.c === --- gcc-trunk/gcc/c/c-decl.c(revision 256939) +++ gcc-trunk/gcc/c/c-decl.c(working copy) @@ -5888,8 +5888,8 @@ /* It is fine to have 'extern const' when compiling at C and C++ intersection. */ if (!(warn_cxx_compat && constp)) - warning_at (loc, 0, "%qE initialized and declared %", -name); + warning_at (loc, OPT_Wextern_initializer, +"%qE initialized and declared %", name); } else error_at (loc, "%qE has both % and initializer", name); Index: gcc-trunk/gcc/cp/decl.c === --- gcc-trunk/gcc/cp/decl.c (revision 256939) +++ gcc-trunk/gcc/cp/decl.c (working copy) @@ -12408,7 +12408,8 @@ /* It's common practice (and completely valid) to have a const be initialized and declared extern. */ if (!(type_quals & TYPE_QUAL_CONST)) - warning (0, "%qs initialized and declared %", name); + warning_at (input_location, OPT_Wextern_initializer, + "%qs initialized and declared %", name); } else { Index: gcc-trunk/gcc/c/c-typeck.c === --- gcc-trunk/gcc/c/c-typeck.c (revision 256939) +++ gcc-trunk/gcc/c/c-typeck.c (working copy) @@ -11599,8 +11599,9 @@ else /* Avoid warning about the volatile ObjC EH puts on decls. */ if (!objc_ok) - pedwarn (location, 0, - "comparison of distinct pointer types lacks a cast"); + pedwarn (location, OPT_Wcompare_distinct_pointer_types, + "comparison of distinct pointer types " + "%qT and %qT lacks a cast", type0, type1); if (result_type == NULL_TREE) { @@ -11711,8 +11712,9 @@ int qual = ENCODE_QUAL_ADDR_SPACE (as_common); result_type = build_pointer_type (build_qualified_type (void_type_node, qual)); - pedwarn (location, 0, - "comparison of distinct pointer types lacks a cast"
Re: [PATCH v2] New C++ warning option '-Waccess-specifiers'
Am 2017-07-24 um 00:19 schrieb Volker Reichelt: On 23 Jul, Eric Gallager wrote: On 7/23/17, Volker Reichelt wrote: Hi again, here is an updated patch for a new warning about redundant access-specifiers. It takes Dave's various comments into account. The main changes w.r.t. to the previous versions are: * The warning is now a two-level warning with a slightly shorter name: -Waccess-specifiers=1, -Waccess-specifiers=2 with -Waccess-specifiers defaulting to -Waccess-specifiers=1. Just a more generalized comment as a user, but I don't really like this trend that new warning options are so often given numeric levels these days. A warning option with different levels requires special handling in configure scripts or Makefiles, which is harder than just toggling different names (i.e. how things work without numeric levels). Fair point. Another point is the handling of -Werror=. AFAIK it would be impossible right now to have "-Werror=access-specifiers=1 -Waccess-specifiers=2", with a combined meaning of "error for level 1 + warning for level 2". Actually, are the intended semantics for the existing cases (eg. -Warray-bounds=) vs. -Werror= even documented somewhere? Franz
Re: [PATCH] Use secure_getenv for GOMP_DEBUG
Am 27.06.17 um 13:10 schrieb Tom de Vries: --- a/libgomp/plugin/plugin-hsa.c +++ b/libgomp/plugin/plugin-hsa.c @@ -39,32 +39,7 @@ #include #include "libgomp-plugin.h" #include "gomp-constants.h" - -/* Secure getenv() which returns NULL if running as SUID/SGID. */ -#ifndef HAVE_SECURE_GETENV -#ifdef HAVE___SECURE_GETENV -#define secure_getenv __secure_getenv -#elif defined (HAVE_UNISTD_H) && defined(HAVE_GETUID) && defined(HAVE_GETEUID) \ - && defined(HAVE_GETGID) && defined(HAVE_GETEGID) - -#include - -/* Implementation of secure_getenv() for targets where it is not provided but - we have at least means to test real and effective IDs. */ - -static char * -secure_getenv (const char *name) -{ - if ((getuid () == geteuid ()) && (getgid () == getegid ())) -return getenv (name); - else -return NULL; -} - -#else -#define secure_getenv getenv -#endif -#endif +#include "secure-getenv.h" Hi, that should be secure_getenv.h (underscore instead of dash). Franz
Re: [PATCH] Introduce --with-gcc-major-version-only configure option (take 2)
Am 2017-01-12 um 21:16 schrieb Jakub Jelinek: libmpx/ * configure.ac: Add GCC_BASE_VER. * Makefile.am (gcc_version): Use @get_gcc_base_ver@ instead of cat to get version from BASE-VER file. * configure: Regenerated. Hi, it seems libmpx/configure.ac is missing the acx.m4 include, because there is now a bare GCC_BASE_VER in the regenerated libmpx/configure. The attached patch seem to fix it, but I'm not good with autoconf. Franz Index: libmpx/configure.ac === --- libmpx/configure.ac (revision 244613) +++ libmpx/configure.ac (working copy) @@ -1,6 +1,8 @@ # -*- Autoconf -*- # Process this file with autoconf to produce a configure script. +sinclude(../config/acx.m4) + AC_PREREQ([2.64]) AC_INIT(package-unused, version-unused, libmpx)
Re: Remove mode argument from gen_rtx_SET
Am 2015-05-08 um 13:57 schrieb Segher Boessenkool: On Fri, May 08, 2015 at 12:32:30PM +0200, Franz Sirl wrote: this patch (r222882 is fine, r222883 fails) breaks bootstrap for me on x86_64-linux-gnu: i386.md has "set:BND" twice; replace that with just "set", and all should be fine. Maybe gen* should warn on this; maybe it already does. I didn't see a warning in the logs at least. But your suggestion fixes the bootstrap for me. Franz. Index: gcc/config/i386/i386.md === --- gcc/config/i386/i386.md (revision 222909) +++ gcc/config/i386/i386.md (working copy) @@ -18879,7 +18879,7 @@ [(set_attr "type" "mpxchk")]) (define_expand "_ldx" - [(parallel [(set:BND (match_operand:BND 0 "register_operand") + [(parallel [(set (match_operand:BND 0 "register_operand") (unspec:BND [(mem: (match_par_dup 3 @@ -18909,7 +18909,7 @@ }) (define_insn "*_ldx" - [(parallel [(set:BND (match_operand:BND 0 "register_operand" "=w") + [(parallel [(set (match_operand:BND 0 "register_operand" "=w") (unspec:BND [(match_operator: 3 "bnd_mem_operator" [(unspec:
Re: Remove mode argument from gen_rtx_SET
Am 2015-05-07 um 13:37 schrieb Richard Sandiford: One problem with the automatically-generated gen_rtx_FOO () macros is that they always have a mode parameter, even for codes like SET where the mode should always be VOIDmode. This inevitably leads to cases where a caller accidentally passes something other than VOIDmode. E.g. when expanding an SImode move, the temptation is to make everything SImode, even the SETs. This in turn can cause two instructions to appear different simply because their SETs have different modes, even though the SET_DEST and SET_SRC are identical. E.g. for gcc/testsuite/g++.dg/torture/pr34651.C on lm32-elf we have the following before jump2: (jump_insn 42 191 43 5 (set (pc) (if_then_else (eq:SI (reg:SI 13 r13 [orig:43 inHotKey$4+-3 ] [43]) (const_int 0 [0])) (label_ref 53) (pc))) gcc/testsuite/g++.dg/torture/pr34651.C:22 22 {*beq} (expr_list:REG_DEAD (reg:SI 13 r13 [orig:43 inHotKey$4+-3 ] [43]) (int_list:REG_BR_PROB 5000 (nil))) -> 53) (note 43 42 48 6 [bb 6] NOTE_INSN_BASIC_BLOCK) (insn 48 43 47 6 (set (reg:SI 2 r2) (mem/u/c:SI (reg:SI 1 r1) [4 S4 A32])) gcc/testsuite/g++.dg/torture/pr34651.C:22 7 {movsi_insn} (expr_list:REG_DEAD (reg:SI 1 r1) (nil))) [...] (code_label 53 169 54 7 6 "" [1 uses]) (note 54 53 12 7 [bb 7] NOTE_INSN_BASIC_BLOCK) (insn 12 54 57 7 (set:SI (reg/f:SI 2 r2 [orig:46 D.2050 ] [46]) (mem/u/c:SI (reg:SI 1 r1) [4 S4 A32])) gcc/testsuite/g++.dg/torture/pr34651.C:22 7 {movsi_insn} (expr_list:REG_DEAD (reg:SI 1 r1) (expr_list:REG_EQUAL (symbol_ref/f:SI ("*.LC3") [flags 0x2] ) (nil where insns 12 and 48 are identical except for the :SI on the SET. This difference prevents us from merging the heads of the two blocks; after removing it we replace the two loads with a single load before the branch. This patch removes the mode argument from gen_rtx_SET and updates all callers. I used a script to (try to) make sure that all callers really had been caught. I also built one target per CPU just in case. There were some changes in gcc.dg, g++.dg and gcc.c-torture assembly code for c6x-elf, lm32-elf and v850-elf, but all of them seemed to be code improvements from removing duplicated instructions. (Other ports also passed spurious modes but apparently not in a way that affects the tests I'd tried.) Also tested on x86_64-linux-gnu. OK to install? BTW, I've split the patch up into two, the last bit being a mechanical removal of modes. (I did it by hand though to try to keep things properly formatted.) Thanks, Richard gcc/ * rtl.h (always_void_p): New function. * gengenrtl.c (always_void_p(: Likewise. (genmacro): Don't add a mode parameter to gen_rtx_foo if rtxes with code foo are always VOIDmode. * genemit.c (gen_exp): Update gen_rtx_foo calls accordingly. * builtins.c, caller-save.c, calls.c, cfgexpand.c, combine.c, compare-elim.c, config/aarch64/aarch64.c, config/aarch64/aarch64.md, config/alpha/alpha.c, config/alpha/alpha.md, config/arc/arc.c, config/arc/arc.md, config/arm/arm-fixed.md, config/arm/arm.c, config/arm/arm.md, config/arm/ldrdstrd.md, config/arm/thumb2.md, config/arm/vfp.md, config/avr/avr.c, config/bfin/bfin.c, config/c6x/c6x.c, config/c6x/c6x.md, config/cr16/cr16.c, config/cris/cris.c, config/cris/cris.md, config/darwin.c, config/epiphany/epiphany.c, config/epiphany/epiphany.md, config/fr30/fr30.c, config/frv/frv.c, config/frv/frv.md, config/h8300/h8300.c, config/i386/i386.c, config/i386/i386.md, config/i386/sse.md, config/ia64/ia64.c, config/ia64/vect.md, config/iq2000/iq2000.c, config/iq2000/iq2000.md, config/lm32/lm32.c, config/lm32/lm32.md, config/m32c/m32c.c, config/m32r/m32r.c, config/m68k/m68k.c, config/m68k/m68k.md, config/mcore/mcore.c, config/mcore/mcore.md, config/mep/mep.c, config/microblaze/microblaze.c, config/mips/mips.c, config/mips/mips.md, config/mmix/mmix.c, config/mn10300/mn10300.c, config/msp430/msp430.c, config/nds32/nds32-memory-manipulation.c, config/nds32/nds32.c, config/nds32/nds32.md, config/nios2/nios2.c, config/nvptx/nvptx.c, config/pa/pa.c, config/pa/pa.md, config/rl78/rl78.c, config/rs6000/altivec.md, config/rs6000/rs6000.c, config/rs6000/rs6000.md, config/rs6000/vector.md, config/rs6000/vsx.md, config/rx/rx.c, config/rx/rx.md, config/s390/s390.c, config/s390/s390.md, config/sh/sh.c, config/sh/sh.md, config/sh/sh_treg_combine.cc, config/sparc/sparc.c, config/sparc/sparc.md, config/spu/spu.c, config/spu/spu.md, config/stormy16/stormy16.c, config/tilegx/tilegx.c, config/tilegx/tilegx.md, config/tilepro/tilepro.c, config/tilepro/tilepro.md, config/v
Re: [PATCH] Put all constants last in tree_swap_operands_p, remove odd -Os check
Am 15.08.2014 um 11:32 schrieb Manuel López-Ibáñez: > On 15 August 2014 11:07, Richard Biener wrote: >> - if (TREE_CODE (arg1) == INTEGER_CST) >> + if (CONSTANT_CLASS_P (arg1) == INTEGER_CST) > > Huh? > > /* Nonzero if NODE represents a constant. */ > > #define CONSTANT_CLASS_P(NODE)\ > (TREE_CODE_CLASS (TREE_CODE (NODE)) == tcc_constant) > > Sadly, we don't have a warning for this, but clang++ has one: > > test.c:4:16: warning: comparison of constant 2 with expression of type > 'bool' is always false [-Wtautological-constant-out-of-range-compare] > if ((a == 1) == 2) { > ^ ~ > > I'll open a PR See also PR 44077 Franz
Re: Use "[warning enabled by default]" for default warnings
Am 2014-02-11 15:36, schrieb Richard Sandiford: I thought the trend these days was to move towards -Werror, so that for many people the expected output is to get no warnings at all. And bear in mind that the kind of warnings that are not under -W control tend to be those that are so likely to be a mistake that no-one has ever had an incentive to make them optional. I find it hard to believe that significant numbers of users are not fixing the sources of those warnings and are instead requiring every release of GCC to produce warnings with a particular wording. Hi, actually at my site we turn on more and more warnings into errors, but we do it warning by warning with more -Werror=..., so the fine-grained warning changes are really nice for us. The problem we face with "[enabled by default]" warnings is not that there are no options to turn these warnings off (we _want_ these warnings), but this also means there are no corresponding -Werror= options (and also no -Werror=enabled-by-default or -Werror=default-warnings). And pure -Werror turns all other warnings we want to see into errors too :(. regards, Franz