[RFC] Move hash-table.h and related files to libiberty
Hello,
I would like to move hash-table.h, hash-map.h and related files
to libiberty, so that GDB can make use of it.
I see that gcc already has a C++ file in include/ (unique-ptr.h),
which I understand is libiberty.
However, this patch is not complete yet (for a start, it doesn't
compile). Before I go further down this road, is this acceptable
in principle to the gcc/libiberty maintainers?
(the bulk of the patch is including vec.h in a lot of files,
because hash-table.h previously included it. It doesn't
actually use it, and I didn't think it was necessary to
move that to libiberty as well, so I removed that include
and instead am adding it to all the files that now don't
compile.)
Thanks!
Christian
Index: Makefile.in
===
--- Makefile.in (revision 275695)
+++ Makefile.in (working copy)
@@ -1490,7 +1490,6 @@
spellcheck-tree.o \
sreal.o \
stack-ptr-mod.o \
- statistics.o \
stmt.o \
stor-layout.o \
store-motion.o \
Index: bitmap.c
===
--- bitmap.c(revision 275695)
+++ bitmap.c(working copy)
@@ -22,6 +22,7 @@
#include "coretypes.h"
#include "bitmap.h"
#include "selftest.h"
+#include "vec.h"
/* Memory allocation statistics purpose instance. */
mem_alloc_description bitmap_mem_desc;
Index: cfgloop.c
===
--- cfgloop.c (revision 275695)
+++ cfgloop.c (working copy)
@@ -1135,7 +1135,7 @@
gcc_assert (current_loops->exits == NULL);
current_loops->exits
-= hash_table::create_ggc (2 * number_of_loops (cfun));
+= hash_table_ggc::create_ggc (2 * number_of_loops
(cfun));
FOR_EACH_BB_FN (bb, cfun)
{
Index: cgraph.c
===
--- cgraph.c(revision 275695)
+++ cgraph.c(working copy)
@@ -183,7 +183,7 @@
version_info_node->this_node = this;
if (cgraph_fnver_htab == NULL)
-cgraph_fnver_htab = hash_table::create_ggc (2);
+cgraph_fnver_htab = hash_table_ggc::create_ggc
(2);
*cgraph_fnver_htab->find_slot (version_info_node, INSERT)
= version_info_node;
@@ -760,7 +760,7 @@
if (n > 100)
{
- call_site_hash = hash_table::create_ggc (120);
+ call_site_hash = hash_table_ggc::create_ggc (120);
for (e2 = callees; e2; e2 = e2->next_callee)
cgraph_add_edge_to_call_site_hash (e2);
for (e2 = indirect_calls; e2; e2 = e2->next_callee)
Index: common/common-target.h
===
--- common/common-target.h (revision 275695)
+++ common/common-target.h (working copy)
@@ -22,6 +22,7 @@
#ifndef GCC_COMMON_TARGET_H
#define GCC_COMMON_TARGET_H
+#include "vec.h"
/* Sets of optimization levels at which an option may be enabled by
default_options_optimization. */
Index: common/common-targhooks.h
===
--- common/common-targhooks.h (revision 275695)
+++ common/common-targhooks.h (working copy)
@@ -20,6 +20,8 @@
#ifndef GCC_COMMON_TARGHOOKS_H
#define GCC_COMMON_TARGHOOKS_H
+#include "vec.h"
+
extern enum unwind_info_type default_except_unwind_info (struct gcc_options *);
extern enum unwind_info_type dwarf2_except_unwind_info (struct gcc_options *);
extern enum unwind_info_type sjlj_except_unwind_info (struct gcc_options *);
Index: config/darwin.c
===
--- config/darwin.c (revision 275695)
+++ config/darwin.c (working copy)
@@ -572,7 +572,7 @@
sprintf (buffer, "&%s%c%s%s%s%s", quote, L_or_l, prefix, name, suffix,
quote);
if (!machopic_indirections)
-machopic_indirections = hash_table::create_ggc (37);
+machopic_indirections = hash_table_ggc::create_ggc
(37);
machopic_indirection **slot
= machopic_indirections->find_slot_with_hash (buffer,
@@ -3454,7 +3454,7 @@
rest_of_decl_compilation (cfstring_class_reference, 0, 0);
/* Initialize the hash table used to hold the constant CFString objects. */
- cfstring_htab = hash_table::create_ggc (31);
+ cfstring_htab = hash_table_ggc::create_ggc (31);
return cfstring_type_node;
}
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 275695)
+++ config/i386/i386.c (working copy)
@@ -95,6 +95,7 @@
#include "i386-builtins.h"
#include "i386-expand.h"
#include "i386-features.h"
+#include "hash-table-ggc.h"
/* This file should be included last. */
#include "target-def.h"
@@ -11048,7 +11049,7 @@
rtx rtl;
if (!dllimport_map)
-dllimport_map = hash_table::create_ggc (512);
+dllimport_map = hash_table_ggc::create_ggc (512);
in.hash = htab_hash_pointer (decl);
in.base.from = decl;
Index: cp/constexpr.c
Re: [RFC] Move hash-table.h and related files to libiberty
On September 21, 2019 11:12:38 AM GMT+02:00, Christian Biesinger via
gcc-patches wrote:
>Hello,
>
>I would like to move hash-table.h, hash-map.h and related files
>to libiberty, so that GDB can make use of it.
>
>I see that gcc already has a C++ file in include/ (unique-ptr.h),
>which I understand is libiberty.
>
>However, this patch is not complete yet (for a start, it doesn't
>compile). Before I go further down this road, is this acceptable
>in principle to the gcc/libiberty maintainers?
>
>(the bulk of the patch is including vec.h in a lot of files,
>because hash-table.h previously included it. It doesn't
>actually use it, and I didn't think it was necessary to
>move that to libiberty as well, so I removed that include
>and instead am adding it to all the files that now don't
>compile.)
The bulk seems to be hash_table to hash_table_ggc renaming. Can you explain?
Also we can then rename create_ggc back to create?
Richard.
>Thanks!
>Christian
>
>Index: Makefile.in
>===
>--- Makefile.in(revision 275695)
>+++ Makefile.in(working copy)
>@@ -1490,7 +1490,6 @@
> spellcheck-tree.o \
> sreal.o \
> stack-ptr-mod.o \
>- statistics.o \
> stmt.o \
> stor-layout.o \
> store-motion.o \
>Index: bitmap.c
>===
>--- bitmap.c (revision 275695)
>+++ bitmap.c (working copy)
>@@ -22,6 +22,7 @@
> #include "coretypes.h"
> #include "bitmap.h"
> #include "selftest.h"
>+#include "vec.h"
>
> /* Memory allocation statistics purpose instance. */
> mem_alloc_description bitmap_mem_desc;
>Index: cfgloop.c
>===
>--- cfgloop.c (revision 275695)
>+++ cfgloop.c (working copy)
>@@ -1135,7 +1135,7 @@
>
> gcc_assert (current_loops->exits == NULL);
> current_loops->exits
>-= hash_table::create_ggc (2 * number_of_loops
>(cfun));
>+= hash_table_ggc::create_ggc (2 *
>number_of_loops (cfun));
>
> FOR_EACH_BB_FN (bb, cfun)
> {
>Index: cgraph.c
>===
>--- cgraph.c (revision 275695)
>+++ cgraph.c (working copy)
>@@ -183,7 +183,7 @@
> version_info_node->this_node = this;
>
> if (cgraph_fnver_htab == NULL)
>-cgraph_fnver_htab =
>hash_table::create_ggc (2);
>+cgraph_fnver_htab =
>hash_table_ggc::create_ggc (2);
>
> *cgraph_fnver_htab->find_slot (version_info_node, INSERT)
> = version_info_node;
>@@ -760,7 +760,7 @@
>
> if (n > 100)
> {
>- call_site_hash = hash_table::create_ggc
>(120);
>+ call_site_hash = hash_table_ggc::create_ggc
>(120);
> for (e2 = callees; e2; e2 = e2->next_callee)
> cgraph_add_edge_to_call_site_hash (e2);
> for (e2 = indirect_calls; e2; e2 = e2->next_callee)
>Index: common/common-target.h
>===
>--- common/common-target.h (revision 275695)
>+++ common/common-target.h (working copy)
>@@ -22,6 +22,7 @@
> #ifndef GCC_COMMON_TARGET_H
> #define GCC_COMMON_TARGET_H
>
>+#include "vec.h"
>
> /* Sets of optimization levels at which an option may be enabled by
>default_options_optimization. */
>Index: common/common-targhooks.h
>===
>--- common/common-targhooks.h (revision 275695)
>+++ common/common-targhooks.h (working copy)
>@@ -20,6 +20,8 @@
> #ifndef GCC_COMMON_TARGHOOKS_H
> #define GCC_COMMON_TARGHOOKS_H
>
>+#include "vec.h"
>+
>extern enum unwind_info_type default_except_unwind_info (struct
>gcc_options *);
>extern enum unwind_info_type dwarf2_except_unwind_info (struct
>gcc_options *);
>extern enum unwind_info_type sjlj_except_unwind_info (struct
>gcc_options *);
>Index: config/darwin.c
>===
>--- config/darwin.c(revision 275695)
>+++ config/darwin.c(working copy)
>@@ -572,7 +572,7 @@
>sprintf (buffer, "&%s%c%s%s%s%s", quote, L_or_l, prefix, name, suffix,
>quote);
>
> if (!machopic_indirections)
>-machopic_indirections = hash_table::create_ggc
>(37);
>+machopic_indirections =
>hash_table_ggc::create_ggc (37);
>
> machopic_indirection **slot
> = machopic_indirections->find_slot_with_hash (buffer,
>@@ -3454,7 +3454,7 @@
> rest_of_decl_compilation (cfstring_class_reference, 0, 0);
>
>/* Initialize the hash table used to hold the constant CFString
>objects. */
>- cfstring_htab = hash_table::create_ggc (31);
>+ cfstring_htab = hash_table_ggc::create_ggc (31);
>
> return cfstring_type_node;
> }
>Index: config/i386/i386.c
>===
>--- config/i386/i386.c (revision 275695)
>+++ config/i386/i386.c (working copy)
>@@ -95,6 +95,7 @@
> #include "i386-builtins.h"
> #include "i386-expand.h"
> #include "i386-features.h"
>+#include "hash-table-ggc.h"
>
>
Re: [RFC] Move hash-table.h and related files to libiberty
On Sat, Sep 21, 2019 at 7:22 PM Richard Biener wrote: > > On September 21, 2019 11:12:38 AM GMT+02:00, Christian Biesinger via > gcc-patches wrote: > >Hello, > > > >I would like to move hash-table.h, hash-map.h and related files > >to libiberty, so that GDB can make use of it. > > > >I see that gcc already has a C++ file in include/ (unique-ptr.h), > >which I understand is libiberty. > > > >However, this patch is not complete yet (for a start, it doesn't > >compile). Before I go further down this road, is this acceptable > >in principle to the gcc/libiberty maintainers? > > > >(the bulk of the patch is including vec.h in a lot of files, > >because hash-table.h previously included it. It doesn't > >actually use it, and I didn't think it was necessary to > >move that to libiberty as well, so I removed that include > >and instead am adding it to all the files that now don't > >compile.) > > The bulk seems to be hash_table to hash_table_ggc renaming. Can you explain? Yeah, sure. If hash-table.h lives in libiberty, I wanted to reduce the dependencies on other headers. GCC's garbage collector seems like something that does not belong there, so I moved this create function to a separate header, which required renaming it since it now can't be part of the same class. (the other option would be some kind of #ifdef GCC thing, but that seemed ugly to me) > Also we can then rename create_ggc back to create? Sure, that'd work. Christian
[PATCH] Add new builtin __SANITIZE_UNDEFINED__ macros for fsanitize=undefined switch
GCC version of https://reviews.llvm.org/D52386
2019-09-21 Kamil Rytarowski
* cppbuiltin.c (define_builtin_macros_for_compilation_flags): Add new
builtin __SANITIZE_UNDEFINED__ macros for fsanitize=undefined switch.
* doc/cpp.texi: Document new macros.
* c-c++-common/lsan/sanitize-undefined-macro.c: New test.
---
gcc/ChangeLog| 6 ++
gcc/cppbuiltin.c | 3 +++
gcc/doc/cpp.texi | 3 +++
gcc/testsuite/ChangeLog | 4
.../c-c++-common/ubsan/sanitize-ubsan-macro.c| 12
5 files changed, 28 insertions(+)
create mode 100644 gcc/testsuite/c-c++-common/ubsan/sanitize-ubsan-macro.c
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 7b773167433..4f99190ca0c 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2019-09-21 Kamil Rytarowski
+
+ * cppbuiltin.c (define_builtin_macros_for_compilation_flags): Add new
+ builtin __SANITIZE_UNDEFINED__ macros for fsanitize=undefined switch.
+ * doc/cpp.texi: Document new macros.
+
2019-09-20 Jonas Pfeil
* config/microblaze/microblaze.h (ASM_OUTPUT_SKIP): Use
diff --git a/gcc/cppbuiltin.c b/gcc/cppbuiltin.c
index 60e5bedc366..221fb68b575 100644
--- a/gcc/cppbuiltin.c
+++ b/gcc/cppbuiltin.c
@@ -93,6 +93,9 @@ define_builtin_macros_for_compilation_flags (cpp_reader
*pfile)
if (flag_sanitize & SANITIZE_ADDRESS)
cpp_define (pfile, "__SANITIZE_ADDRESS__");
+ if (flag_sanitize & SANITIZE_UNDEFINED)
+cpp_define (pfile, "__SANITIZE_UNDEFINED__");
+
if (flag_sanitize & SANITIZE_THREAD)
cpp_define (pfile, "__SANITIZE_THREAD__");
diff --git a/gcc/doc/cpp.texi b/gcc/doc/cpp.texi
index f2de39a270c..3145d3ebab4 100644
--- a/gcc/doc/cpp.texi
+++ b/gcc/doc/cpp.texi
@@ -2359,6 +2359,9 @@ in use.
This macro is defined, with value 1, when @option{-fsanitize=address}
or @option{-fsanitize=kernel-address} are in use.
+@item __SANITIZE_UNDEFINED__
+This macro is defined, with value 1, when @option{-fsanitize=undefined} is in
use.
+
@item __SANITIZE_THREAD__
This macro is defined, with value 1, when @option{-fsanitize=thread} is in use.
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 05c25ee28ce..e35880b1b2f 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2019-09-21 Kamil Rytarowski
+
+ * c-c++-common/usan/sanitize-undefined-macro.c: New test.
+
2019-09-20 Iain Sandoe
* gcc.target/i386/naked-1.c: Alter options to use non-
diff --git a/gcc/testsuite/c-c++-common/ubsan/sanitize-ubsan-macro.c
b/gcc/testsuite/c-c++-common/ubsan/sanitize-ubsan-macro.c
new file mode 100644
index 000..ecca41da32e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ubsan/sanitize-ubsan-macro.c
@@ -0,0 +1,12 @@
+/* Check that -fsanitize=undefined options defines __SANITIZE_UNDEFINED__
macros. */
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "*" } { "-O0" } } */
+
+int
+main ()
+{
+#ifndef __SANITIZE_UNDEFINED__
+ bad construction
+#endif
+ return 0;
+}
--
2.23.0
Re: [RFC] Move hash-table.h and related files to libiberty
On September 21, 2019 12:28:57 PM GMT+02:00, Christian Biesinger wrote: >On Sat, Sep 21, 2019 at 7:22 PM Richard Biener > wrote: >> >> On September 21, 2019 11:12:38 AM GMT+02:00, Christian Biesinger via >gcc-patches wrote: >> >Hello, >> > >> >I would like to move hash-table.h, hash-map.h and related files >> >to libiberty, so that GDB can make use of it. >> > >> >I see that gcc already has a C++ file in include/ (unique-ptr.h), >> >which I understand is libiberty. >> > >> >However, this patch is not complete yet (for a start, it doesn't >> >compile). Before I go further down this road, is this acceptable >> >in principle to the gcc/libiberty maintainers? >> > >> >(the bulk of the patch is including vec.h in a lot of files, >> >because hash-table.h previously included it. It doesn't >> >actually use it, and I didn't think it was necessary to >> >move that to libiberty as well, so I removed that include >> >and instead am adding it to all the files that now don't >> >compile.) >> >> The bulk seems to be hash_table to hash_table_ggc renaming. Can you >explain? > >Yeah, sure. If hash-table.h lives in libiberty, I wanted to reduce the >dependencies on other headers. GCC's garbage collector seems like >something that does not belong there, so I moved this create function >to a separate header, which required renaming it since it now can't be >part of the same class. (the other option would be some kind of #ifdef >GCC thing, but that seemed ugly to me) As long as gengtype can still pick up everything correctly via the GTY annotations that's probably OK. >> Also we can then rename create_ggc back to create? > >Sure, that'd work. > >Christian
Re: [PATCH] PR fortran/91426: Colorize %L text to match diagnostic_show_locus
Hi David, I think technically I can self-approve this, but I'm not a day-to-day user of fortran; does this look sane? Very much so, I also find this more readable. I'd wait another day or so for comitting this, so that other people with different aesthetic sense can also chime in if they want to :-) Regards Thomas
C++ PATCH to add test for DR 2345
Jumping across initializers is forbidden, and this DR clarifies that
that applies to initializers in init-statements too. We already detect
this case, so I'm adding this test and marking the DR as resolved.
Tested on x86_64-linux, applying to trunk.
2019-09-21 Marek Polacek
DR 2345 - Jumping across initializers in init-statements and conditions.
* g++.dg/cpp1z/init-statement10.C: New test.
diff --git gcc/testsuite/g++.dg/cpp1z/init-statement10.C
gcc/testsuite/g++.dg/cpp1z/init-statement10.C
new file mode 100644
index 000..d13d135dab1
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp1z/init-statement10.C
@@ -0,0 +1,14 @@
+// DR 2345 - Jumping across initializers in init-statements and conditions.
+// { dg-do compile { target c++17 } }
+
+int
+fn ()
+{
+ goto X;
+ if (int i = 42; i == 42)
+{
+X: // { dg-error "jump to label" }
+ return i;
+}
+ return -1;
+}
Re: [PATCH 3/4] New IPA-SRA implementation
Hi,
Thanks for doing this.
Martin Jambor writes:
> +/* Analyze function body scan results stored in param_accesses and
> + param_accesses, detect possible transformations and store information of
> + those in function summary. NODE, FUN and IFS are all various structures
> + describing the currently analyzed function. */
> +
> +static void
> +process_scan_results (cgraph_node *node, struct function *fun,
> + isra_func_summary *ifs,
> + vec *param_descriptions)
> +{
> + bool check_pass_throughs = false;
> + bool dereferences_propagated = false;
> + tree parm = DECL_ARGUMENTS (node->decl);
> + unsigned param_count = param_descriptions->length();
> +
> + for (unsigned desc_index = 0;
> + desc_index < param_count;
> + desc_index++, parm = DECL_CHAIN (parm))
> +{
> + gensum_param_desc *desc = &(*param_descriptions)[desc_index];
> + if (!desc->locally_unused && !desc->split_candidate)
> + continue;
I'm jumping in the middle without working through the whole pass,
so this is probably a daft question sorry, but: what is this loop
required to do when:
!desc->split_candidate && desc->locally_unused
? AFAICT...
> +
> + if (flag_checking)
> + isra_verify_access_tree (desc->accesses);
> +
> + if (!dereferences_propagated
> + && desc->by_ref
> + && desc->accesses)
> + {
> + propagate_dereference_distances (fun);
> + dereferences_propagated = true;
> + }
> +
> + HOST_WIDE_INT nonarg_acc_size = 0;
> + bool only_calls = true;
> + bool check_failed = false;
> +
> + int entry_bb_index = ENTRY_BLOCK_PTR_FOR_FN (fun)->index;
> + for (gensum_param_access *acc = desc->accesses;
> +acc;
> +acc = acc->next_sibling)
> + if (check_gensum_access (parm, desc, acc, &nonarg_acc_size, &only_calls,
> + entry_bb_index))
> + {
> + check_failed = true;
> + break;
> + }
> + if (check_failed)
> + continue;
> +
> + if (only_calls)
> + desc->locally_unused = true;
> +
> + HOST_WIDE_INT cur_param_size
> + = tree_to_uhwi (TYPE_SIZE (TREE_TYPE (parm)));
> + HOST_WIDE_INT param_size_limit;
> + if (!desc->by_ref || optimize_function_for_size_p (fun))
> + param_size_limit = cur_param_size;
> + else
> + param_size_limit = (PARAM_VALUE (PARAM_IPA_SRA_PTR_GROWTH_FACTOR)
> +* cur_param_size);
> + if (nonarg_acc_size > param_size_limit
> + || (!desc->by_ref && nonarg_acc_size == param_size_limit))
> + {
> + disqualify_split_candidate (desc, "Would result into a too big set of"
> + "replacements.");
> + }
> + else
> + {
> + /* create_parameter_descriptors makes sure unit sizes of all
> + candidate parameters fit unsigned integers restricted to
> + ISRA_ARG_SIZE_LIMIT. */
> + desc->param_size_limit = param_size_limit / BITS_PER_UNIT;
> + desc->nonarg_acc_size = nonarg_acc_size / BITS_PER_UNIT;
> + if (desc->split_candidate && desc->ptr_pt_count)
> + {
> + gcc_assert (desc->by_ref);
> + check_pass_throughs = true;
> + }
> + }
> +}
...disqualify_split_candidate should be a no-op in that case,
because we've already disqualified the parameter for a different reason.
So it looks like the main effect is instead to set up param_size_limit
and nonarg_acc_size, the latter of which I assume is 0 when
desc->locally_unused.
The reason for asking is that the final "else" says that we've already
checked that param_size_limit is in range, but that's only true if
desc->split_candidate. In particular:
if (is_gimple_reg (parm)
&& !isra_track_scalar_param_local_uses (fun, node, parm, num,
&scalar_call_uses))
{
if (dump_file && (dump_flags & TDF_DETAILS))
fprintf (dump_file, " is a scalar with only %i call uses\n",
scalar_call_uses);
desc->locally_unused = true;
desc->call_uses = scalar_call_uses;
}
happens before:
if (type_size == 0
|| type_size >= ISRA_ARG_SIZE_LIMIT)
{
if (dump_file && (dump_flags & TDF_DETAILS))
fprintf (dump_file, " not a candidate, has zero or huge size\n");
continue;
}
An uninteresting case in which we truncate the value is:
typedef unsigned int big_vec __attribute__((vector_size (0x2)));
void foo (big_vec x) {}
and going further triggers an assert in the tree_to_uhwi (...):
typedef unsigned int omg_vec __attribute__((vector_size (1ULL << 61)));
void foo (omg_vec x) {}
but as you can guess, SVE is the real reason I'm asking. ;-)
FWIW, I tried changing it to:
gensum_param_desc *desc = &(*param_descriptions)[desc_index];
if (!desc->split_candidate)
Re: [PATCH] Add new builtin __SANITIZE_UNDEFINED__ macros for fsanitize=undefined switch
On Sat, Sep 21, 2019 at 01:31:10PM +0200, Kamil Rytarowski wrote: > GCC version of https://reviews.llvm.org/D52386 > > 2019-09-21 Kamil Rytarowski > > * cppbuiltin.c (define_builtin_macros_for_compilation_flags): Add new > builtin __SANITIZE_UNDEFINED__ macros for fsanitize=undefined switch. > * doc/cpp.texi: Document new macros. > > * c-c++-common/lsan/sanitize-undefined-macro.c: New test. I fail to see a good use case for this, it will just lead people to add workarounds for ubsan diagnostics without actually fixing it for real in their production code. Also, -fsanitize=undefined is a collection of many different sanitizers. Those that want such macros to say provide libubsan definitions on their own can do that in their build system, next to -fsanitize=undefined provide their own -Dwhatever that will make that happen. Jakub
Re: [PATCH] 2019-09-20 Kamil Rytarowski
On Fri, Sep 20, 2019 at 10:45:42PM +0200, Kamil Rytarowski wrote: > GCC version of https://reviews.llvm.org/D67719 > > From 422827582d84e078df2a8e303d807c830a155ab5 Mon Sep 17 00:00:00 2001 > From: Kamil Rytarowski > Date: Fri, 20 Sep 2019 22:02:09 +0200 > Subject: [PATCH] 2019-09-20 Kamil Rytarowski > > * cppbuiltin.c (define_builtin_macros_for_compilation_flags): Add new > builtin __SANITIZE_LEAK__ macros for fsanitize=leak switch. > * doc/cpp.texi: Document new macros. > > * c-c++-common/lsan/sanitize-leak-macro.c: New test. And this looks even more pointless. -fsanitize=leak generally isn't a compiler option, just a linker option, but a patch like this would make it a compiler option too. There are no code changes with -fsanitize=leak, just linking arranges a malloc etc. replacement that traces the leaks. Jakub
Re: [PATCH] Add new builtin __SANITIZE_UNDEFINED__ macros for fsanitize=undefined switch
On 21.09.2019 16:51, Jakub Jelinek wrote: > On Sat, Sep 21, 2019 at 01:31:10PM +0200, Kamil Rytarowski wrote: >> GCC version of https://reviews.llvm.org/D52386 >> >> 2019-09-21 Kamil Rytarowski >> >> * cppbuiltin.c (define_builtin_macros_for_compilation_flags): Add new >> builtin __SANITIZE_UNDEFINED__ macros for fsanitize=undefined switch. >> * doc/cpp.texi: Document new macros. >> >> * c-c++-common/lsan/sanitize-undefined-macro.c: New test. > > I fail to see a good use case for this, it will just lead people to add > workarounds for ubsan diagnostics without actually fixing it for real in > their production code. It's needed in real production code in the NetBSD kernel/userland as there are legitimate code paths that are not UBSan clean. There is also a UBSan specific header that can be picked with this ifdef. > Also, -fsanitize=undefined is a collection of many > different sanitizers. I wrote a full UBSan kerne&userland&in-libc runtime so I am aware about this, however I don't have a use case on fine-grained processor checks. > Those that want such macros to say provide libubsan > definitions on their own can do that in their build system, next to > -fsanitize=undefined provide their own -Dwhatever that will make that > happen. Do you see point in e.g. __SANITIZER_ADDRESS__ ? This macro is intended to be used in public NetBSD system headers (as we need it there) and you volunteer to patch scons, cmake, bmake, gmake, gn, autotools etc for us? > > Jakub > signature.asc Description: OpenPGP digital signature
Re: [PATCH] 2019-09-20 Kamil Rytarowski
On 21.09.2019 16:53, Jakub Jelinek wrote: > On Fri, Sep 20, 2019 at 10:45:42PM +0200, Kamil Rytarowski wrote: >> GCC version of https://reviews.llvm.org/D67719 >> >> From 422827582d84e078df2a8e303d807c830a155ab5 Mon Sep 17 00:00:00 2001 >> From: Kamil Rytarowski >> Date: Fri, 20 Sep 2019 22:02:09 +0200 >> Subject: [PATCH] 2019-09-20 Kamil Rytarowski >> >> * cppbuiltin.c (define_builtin_macros_for_compilation_flags): Add new >> builtin __SANITIZE_LEAK__ macros for fsanitize=leak switch. >> * doc/cpp.texi: Document new macros. >> >> * c-c++-common/lsan/sanitize-leak-macro.c: New test. > > And this looks even more pointless. -fsanitize=leak generally isn't a > compiler option, just a linker option, This is wrong. -fsanitize=leak IS a compiler option. > but a patch like this would make it a > compiler option too. There are no code changes with -fsanitize=leak, just > linking arranges a malloc etc. replacement that traces the leaks. > I am aware about its implementation as I ported LSan to NetBSD. > Jakub > The same argument applies here as in the UBSan patch. Here we have got a special use-case to use standalone LSan on NetBSD we we run whole userland sanitization and ASan is much more fragile (largely due to its incompleteness in interceptors). This ifdef is also scheduled to be included in public system headers. signature.asc Description: OpenPGP digital signature
Re: Monotonically increasing counter (was Re: [Contrib PATCH] Add scripts to convert GCC repo from SVN to Git)
On Thu, Sep 19, 2019 at 03:29:27PM -0400, Jason Merrill wrote: > I suppose we also need to decide what form we want to use for the > equivalent of gcc.gnu.org/rN. I figure it needs to be some prefix > followed by a "commit-ish" (hash, tag, etc.). Perhaps "g:" as the > prefix, so > > gcc.gnu.org/g:HEAD > gcc.gnu.org/g:gcc-9-branch > gcc.gnu.org/g:3cf0d8938a953e Hrm, but we should probably not allow everything here, some things can be expensive to compute (HEAD~123456 for example), and we don't want to expose the reflog on the server (if there even is one), etc. OTOH allowing pretty much everything here is a quite neat idea. What do we use for gitweb thing? That might have a solution for this already. Currently we seem to use plain gitweb, maybe cgit or similar would be nicer? It looks more modern, anyway :-P Segher
[Darwin, committed] Update machopic_legitimize_pic_address.
Some changes were missed here in the transition to LRA. The Darwin
archs are all using LRA now.
tested on i686-darwin9, x86_64-darwin16,
applied to mainline
thanks
Iain
gcc/ChangeLog:
2019-09-21 Iain Sandoe
* config/darwin.c (machopic_legitimize_pic_address): Check
for lra not reload.
diff --git a/gcc/config/darwin.c b/gcc/config/darwin.c
index e1017befaf..3e4bbffc92 100644
--- a/gcc/config/darwin.c
+++ b/gcc/config/darwin.c
@@ -842,7 +842,7 @@ machopic_legitimize_pic_address (rtx orig, machine_mode
mode, rtx reg)
{
if (reg == 0)
{
- gcc_assert (!reload_in_progress);
+ gcc_assert (!lra_in_progress);
reg = gen_reg_rtx (Pmode);
}
@@ -926,7 +926,7 @@ machopic_legitimize_pic_address (rtx orig, machine_mode
mode, rtx reg)
emit_use (gen_rtx_REG (Pmode, PIC_OFFSET_TABLE_REGNUM));
#endif
- if (reload_in_progress)
+ if (lra_in_progress)
df_set_regs_ever_live (REGNO (pic), true);
pic_ref = gen_rtx_PLUS (Pmode, pic,
machopic_gen_offset (XEXP (orig, 0)));
@@ -950,7 +950,7 @@ machopic_legitimize_pic_address (rtx orig, machine_mode
mode, rtx reg)
if (reg == 0)
{
- gcc_assert (!reload_in_progress);
+ gcc_assert (!lra_in_progress);
reg = gen_reg_rtx (Pmode);
}
@@ -996,7 +996,7 @@ machopic_legitimize_pic_address (rtx orig, machine_mode
mode, rtx reg)
#if 0
emit_use (pic_offset_table_rtx);
#endif
- if (reload_in_progress)
+ if (lra_in_progress)
df_set_regs_ever_live (REGNO (pic), true);
pic_ref = gen_rtx_PLUS (Pmode,
pic,
Re: Monotonically increasing counter (was Re: [Contrib PATCH] Add scripts to convert GCC repo from SVN to Git)
On 9/21/19 2:18 PM, Segher Boessenkool wrote: On Thu, Sep 19, 2019 at 03:29:27PM -0400, Jason Merrill wrote: I suppose we also need to decide what form we want to use for the equivalent of gcc.gnu.org/rN. I figure it needs to be some prefix followed by a "commit-ish" (hash, tag, etc.). Perhaps "g:" as the prefix, so gcc.gnu.org/g:HEAD gcc.gnu.org/g:gcc-9-branch gcc.gnu.org/g:3cf0d8938a953e Hrm, but we should probably not allow everything here, some things can be expensive to compute (HEAD~123456 for example), and we don't want to expose the reflog on the server (if there even is one), etc. OTOH allowing pretty much everything here is a quite neat idea. What do we use for gitweb thing? That might have a solution for this already. Currently we seem to use plain gitweb, maybe cgit or similar would be nicer? It looks more modern, anyway :-P Segher If I recall correctly using git branches based off tags is the preferred way. And to Seger's point after a server there is none after pulling down in git. Everything is off line unless he means something else. The biggest thing as I pointed out at Cauldron in terms of issues are: a) How much history do you need in terms of how far back for git bisect or rebasing and b. Branching after a major release or for other non trunk branches. How to allow other branches or how to set them up using tags e.t.c in git for this. Mostly the problem with git is getting in right for these two based on the project requirments, Nick
Re: Deprecating cc0 (and consequently cc0 targets)
> On Sep 20, 2019, at 1:45 PM, Jeff Law wrote: > > On 9/20/19 11:22 AM, Richard Biener wrote: >> ... >> It seems to be that for the goal to keep a target alive a variant #2 >> conversion (according to the wiki) should be closely mirror CC0 >> behavior and thus should be easier to achieve and with less patterns >> touched than the 'good' variant. Can you acknowledge that and would >> such 'simple' conversions be OK? > Unless the target has condition code preserving arithmetic a variant #2 > conversion is the only way to go. Yes. > Now if someone did a variant #2 without the optimization bits (ie, > adding appropriate _set_flags pattern variants), I think that should be > considered explicitly OK even though the code quality would potentially > suffer. Essentially it's a choice between dropping the port or keeping > the port, but with slightly less optimization. THe latter seems like a > better choice to me. True. Then again, the added work of creating the pattern pairs is modest. With define_subst, much of the work can be automated. For pdp11, I found this to be the case; the hard part was to learn what is needed, and for that the Wiki ends up a big help. Also, pdp11 is harder than most because it has two CC registers (one for float ops, one for the rest). I don't know all the others, but for example VAX only has one, and I'm pretty sure the same applies to m68k. paul
Re: Deprecating cc0 (and consequently cc0 targets)
On 9/21/19 2:48 PM, Paul Koning wrote: > > >> On Sep 20, 2019, at 1:45 PM, Jeff Law wrote: >> >> On 9/20/19 11:22 AM, Richard Biener wrote: >>> ... It seems to be that for the goal to keep a target alive a >>> variant #2 conversion (according to the wiki) should be closely >>> mirror CC0 behavior and thus should be easier to achieve and with >>> less patterns touched than the 'good' variant. Can you >>> acknowledge that and would such 'simple' conversions be OK? >> Unless the target has condition code preserving arithmetic a >> variant #2 conversion is the only way to go. > > Yes. > >> Now if someone did a variant #2 without the optimization bits (ie, >> adding appropriate _set_flags pattern variants), I think that >> should be considered explicitly OK even though the code quality >> would potentially suffer. Essentially it's a choice between >> dropping the port or keeping the port, but with slightly less >> optimization. THe latter seems like a better choice to me. > > True. Then again, the added work of creating the pattern pairs is > modest. With define_subst, much of the work can be automated. The patterns and support to handle optimization can be added after the basic conversion is done. In fact, that's precisely how I'd approach it. > > For pdp11, I found this to be the case; the hard part was to learn > what is needed, and for that the Wiki ends up a big help. Also, > pdp11 is harder than most because it has two CC registers (one for > float ops, one for the rest). I don't know all the others, but for > example VAX only has one, and I'm pretty sure the same applies to > m68k. m68k is like pdp11 in this regard. Two condition code registers, one in the main processor and another for the 68881 FP unit. jeff
Re: Monotonically increasing counter (was Re: [Contrib PATCH] Add scripts to convert GCC repo from SVN to Git)
On Sat, Sep 21, 2019 at 2:18 PM Segher Boessenkool
wrote:
>
> On Thu, Sep 19, 2019 at 03:29:27PM -0400, Jason Merrill wrote:
> > I suppose we also need to decide what form we want to use for the
> > equivalent of gcc.gnu.org/rN. I figure it needs to be some prefix
> > followed by a "commit-ish" (hash, tag, etc.). Perhaps "g:" as the
> > prefix, so
> >
> > gcc.gnu.org/g:HEAD
> > gcc.gnu.org/g:gcc-9-branch
> > gcc.gnu.org/g:3cf0d8938a953e
>
> Hrm, but we should probably not allow everything here, some things can
> be expensive to compute (HEAD~123456 for example), and we don't want to
> expose the reflog on the server (if there even is one), etc. OTOH
> allowing pretty much everything here is a quite neat idea.
>
> What do we use for gitweb thing? That might have a solution for this
> already. Currently we seem to use plain gitweb, maybe cgit or similar
> would be nicer? It looks more modern, anyway :-P
The current gitweb allows all of my examples with a bit longer URL, e.g.
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=3cf0d8938a953e
it doesn't seem to allow HEAD~1 or HEAD^. It does allow HEAD@{2}, but
I don't see what the problem with that would be.
I was figuring to just rewrite the /g: pattern into the appropriate
query URL for the git web interface.
I have no opinion about which web interface to use. Apparently there
are many:
https://git.wiki.kernel.org/index.php/Interfaces,_frontends,_and_tools#Web_Interfaces
Jason
Re: [C++ PATCH 4/4] PR c++/30277 - int-width bit-field promotion.
On Sat, Sep 21, 2019 at 2:23 AM Jakub Jelinek wrote:
>
> On Mon, Sep 16, 2019 at 12:33:28AM -0400, Jason Merrill wrote:
> > Here, if cp_perform_integral_promotions saw that the TREE_TYPE of a
> > bit-field reference was the same as the type it promotes to, it didn't do
> > anything. But then decay_conversion saw that the bit-field reference was
> > unchanged, and converted it to its declared type. So I needed to add
> > something to make it clear that promotion has been done. But then the 33819
> > change caused trouble by looking through the NOP_EXPR I just added. This
> > was the wrong fix for that bug; I've now fixed that better by recognizing in
> > cp_perform_integral_promotions that we won't promote a bit-field larger than
> > 32 bits, so we should use the declared type.
> >
> > Tested x86_64-pc-linux-gnu, applying to trunk.
> >
> > PR c++/33819 - long bit-field promotion.
> > * typeck.c (cp_perform_integral_promotions): Handle large bit-fields
> > properly. Handle 32-bit non-int bit-fields properly.
> > (is_bitfield_expr_with_lowered_type): Don't look through NOP_EXPR.
>
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/expr/bitfield14.C
> > @@ -0,0 +1,17 @@
> > +// PR c++/30277
> > +// { dg-do compile { target c++11 } }
> > +
> > +struct S
> > +{
> > + signed long l: 32;
> > +};
> > +
> > +void foo(long) = delete;
> > +void foo(int) {}
> > +
> > +int main()
> > +{
> > + S x = {1};
> > + foo(x.l+0);
> > + return 0;
> > +}
>
> This testcase fails on all targets where int and long have the same
> precision. Is that a bug on the compiler side, or should the testcase just
> use a type with a larger precision than int (i.e. long long), like following
> (tested on x86_64-linux and i686-linux):
It's a testcase bug.
> 2019-09-21 Jakub Jelinek
>
> PR c++/30277
> * g++.dg/expr/bitfield14.C (struct S): Use signed long long instead
> of signed long.
> (foo): Use long long instead of long.
OK, thanks.
Jason
[committed] avoid bogus warning on strcpy into nested member array (PR 91830)
The recent improvement to -Warray-bounds to detect past-the-end
accesses by string functions to member subobjects had a bug that
triggered a false positive in a Binutils build. In r276022 I've
committed the attached trivial fix to correct the bug.
Martin
Index: gcc/ChangeLog
===
Index: gcc/testsuite/gcc.dg/Warray-bounds-47.c
===
--- gcc/testsuite/gcc.dg/Warray-bounds-47.c (nonexistent)
+++ gcc/testsuite/gcc.dg/Warray-bounds-47.c (revision 276022)
@@ -0,0 +1,429 @@
+/* PR middle-end/91830 - Bogus -Warray-bounds on strcpy into a member
+ of a subobject compiling binutils
+ { dg-do compile }
+ { dg-options "-O2 -Wall -ftrack-macro-expansion=0" } */
+
+extern char* strcpy (char*, const char*);
+extern void sink (void*);
+
+#define S36 "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+
+#define S(N) (S36 + sizeof (S36) - N - 1)
+
+/* In the test macro, prevent the strcpy to memcpy transformation
+ by using a local array initialized with the string literal. Without
+ it, GCC transforms the strcpy call with memcpy which (unfortunately)
+ permits accesses that cross subobject boundaries. */
+#define T(obj, mem, n)\
+ do { \
+struct A *pa = &obj; \
+const char a[] = S36; \
+strcpy (pa->mem, a + sizeof a - n - 1); \
+sink (&obj);\
+ } while (0)
+
+
+struct A { char a[3]; char b[5]; };
+struct B { char c[7]; struct A a; struct A a2[2]; };
+
+extern struct B b[];
+
+void array_nowarn (int i)
+{
+ struct B *pb = b;
+
+ T (pb[0].a, a, 0); // { dg-bogus "\\\[-W" }
+ T (pb[0].a, a, 1); // { dg-bogus "\\\[-W" }
+ T (pb[0].a, a, 2); // { dg-bogus "\\\[-W" }
+
+ T (pb[1].a, a, 0); // { dg-bogus "\\\[-W" }
+ T (pb[1].a, a, 1); // { dg-bogus "\\\[-W" }
+ T (pb[1].a, a, 2); // { dg-bogus "\\\[-W" }
+
+ T (pb[123].a, a, 0);// { dg-bogus "\\\[-W" }
+ T (pb[123].a, a, 1);// { dg-bogus "\\\[-W" }
+ T (pb[123].a, a, 2);// { dg-bogus "\\\[-W" }
+
+ T (pb[i].a, a, 0);
+ T (pb[i].a, a, 1);
+ T (pb[i].a, a, 2);
+
+
+ T (pb[0].a, b, 0); // { dg-bogus "\\\[-W" }
+ T (pb[0].a, b, 1); // { dg-bogus "\\\[-W" }
+ T (pb[0].a, b, 2); // { dg-bogus "\\\[-W" }
+ T (pb[0].a, b, 3); // { dg-bogus "\\\[-W" }
+ T (pb[0].a, b, 4); // { dg-bogus "\\\[-W" }
+
+ T (pb[1].a, b, 0); // { dg-bogus "\\\[-W" }
+ T (pb[1].a, b, 1); // { dg-bogus "\\\[-W" }
+ T (pb[1].a, b, 2); // { dg-bogus "\\\[-W" }
+ T (pb[1].a, b, 3); // { dg-bogus "\\\[-W" }
+ T (pb[1].a, b, 4); // { dg-bogus "\\\[-W" }
+
+ T (pb[123].a, b, 0);// { dg-bogus "\\\[-W" }
+ T (pb[123].a, b, 1);// { dg-bogus "\\\[-W" }
+ T (pb[123].a, b, 2);// { dg-bogus "\\\[-W" }
+ T (pb[123].a, b, 3);// { dg-bogus "\\\[-W" }
+ T (pb[123].a, b, 4);// { dg-bogus "\\\[-W" }
+
+ T (pb[i].a, b, 0);
+ T (pb[i].a, b, 1);
+ T (pb[i].a, b, 2);
+ T (pb[i].a, b, 3);
+ T (pb[i].a, b, 4);
+
+
+ T (pb[0].a2[0], b, 0); // { dg-bogus "\\\[-W" }
+ T (pb[0].a2[0], b, 1); // { dg-bogus "\\\[-W" }
+ T (pb[0].a2[0], b, 2); // { dg-bogus "\\\[-W" }
+ T (pb[0].a2[0], b, 3); // { dg-bogus "\\\[-W" }
+ T (pb[0].a2[0], b, 4); // { dg-bogus "\\\[-W" }
+
+ T (pb[1].a2[0], b, 0); // { dg-bogus "\\\[-W" }
+ T (pb[1].a2[0], b, 1); // { dg-bogus "\\\[-W" }
+ T (pb[1].a2[0], b, 2); // { dg-bogus "\\\[-W" }
+ T (pb[1].a2[0], b, 3); // { dg-bogus "\\\[-W" }
+ T (pb[1].a2[0], b, 4); // { dg-bogus "\\\[-W" }
+
+ T (pb[123].a2[0], b, 0);// { dg-bogus "\\\[-W" }
+ T (pb[123].a2[0], b, 1);// { dg-bogus "\\\[-W" }
+ T (pb[123].a2[0], b, 2);// { dg-bogus "\\\[-W" }
+ T (pb[123].a2[0], b, 3);// { dg-bogus "\\\[-W" }
+ T (pb[123].a2[0], b, 4);// { dg-bogus "\\\[-W" }
+
+ T (pb[123].a2[1], b, 0);// { dg-bogus "\\\[-W" }
+ T (pb[123].a2[1], b, 1);// { dg-bogus "\\\[-W" }
+ T (pb[123].a2[1], b, 2);// { dg-bogus "\\\[-W" }
+ T (pb[123].a2[1], b, 3);// { dg-bogus "\\\[-W" }
+ T (pb[123].a2[1], b, 4);// { dg-bogus "\\\[-W" }
+
+ T (pb[i].a2[0], b, 0);
+ T (pb[i].a2[0], b, 1);
+ T (pb[i].a2[0], b, 2);
+ T (pb[i].a2[0], b, 3);
+ T (pb[i].a2[0], b, 4);
+
+ T (pb[i].a2[1], b, 0);
+ T (pb[i].a2[1], b, 1);
+ T (pb[i].a2[1], b, 2);
+ T (pb[i].a2[1], b, 3);
+ T (pb[i].a2[1], b, 4);
+}
+
+void array_warn (int i)
+{
+ struct B *pb = b;
+
+ T (pb[0].a, a, 3); // { dg-warning "\\\[-Warray-bounds" }
+ T (pb[0].a, a, 4); // { dg-warning "\\\[-Warray-bounds" }
+
+ T (pb[1].a, a, 5); // { dg-warning "\\\[-Warray-bounds" }
+ T (pb[1].a, a, 6); // { dg-warning "\\\[-Warray-bounds" }
+
+ T (pb[789].a, a, 5);// { dg-warning "\\\[-Warray-bounds" }
+ T (pb[789].a, a, 6);// { dg-warning "\\\[-Warr
Re: Deprecating cc0 (and consequently cc0 targets)
On Sat, Sep 21, 2019 at 03:04:26PM -0600, Jeff Law wrote: > On 9/21/19 2:48 PM, Paul Koning wrote: > >> On Sep 20, 2019, at 1:45 PM, Jeff Law wrote: > >> On 9/20/19 11:22 AM, Richard Biener wrote: > >> Now if someone did a variant #2 without the optimization bits (ie, > >> adding appropriate _set_flags pattern variants), I think that > >> should be considered explicitly OK even though the code quality > >> would potentially suffer. Essentially it's a choice between > >> dropping the port or keeping the port, but with slightly less > >> optimization. THe latter seems like a better choice to me. > > > > True. Then again, the added work of creating the pattern pairs is > > modest. With define_subst, much of the work can be automated. > The patterns and support to handle optimization can be added after the > basic conversion is done. In fact, that's precisely how I'd approach it. Yeah, but a type #2 conversion is more than that; it makes it harder to do a type #1 conversion later than if you started with doing just that. Of course it is better than totally dropping a target. Some coordination would be useful. OTOH, a type #2 conversion seems easy enough that maybe we can just pull that off for *all* targets for GCC 10, and actually remove CC0 already? > > For pdp11, I found this to be the case; the hard part was to learn > > what is needed, and for that the Wiki ends up a big help. Also, > > pdp11 is harder than most because it has two CC registers (one for > > float ops, one for the rest). I don't know all the others, but for > > example VAX only has one, and I'm pretty sure the same applies to > > m68k. > m68k is like pdp11 in this regard. Two condition code registers, one in > the main processor and another for the 68881 FP unit. I think the main difficulty with m68k is that it is a pretty big target. Segher
[PATCH] handle null and non-constant values in get_range_strlen_dynamic (PR 91570)
The new get_range_strlen_dynamic function has a couple of bugs
where it assumes that the length range bounds it gets back from
get_range_strlen are non-null integer constants. The attached
"quick and dirty" fix removes those assumptions. Since it's
apparently causing package failures in Jeff's GCC buildbot
I will commit the patch on Monday to get those builds to pass.
But I'm not too happy with how fragile this seems to be so I
will try to do some further cleanup here in the near future
to make it more robust.
Martin
PR tree-optimization/91570 - ICE in get_range_strlen_dynamic on a conditional of two strings
gcc/testsuite/ChangeLog:
PR tree-optimization/91570
* gcc.dg/pr91570.c: New test.
gcc/ChangeLog:
PR tree-optimization/91570
* tree-ssa-strlen.c (get_range_strlen_dynamic): Handle null and
non-constant maxlen and maxbound.
Index: gcc/testsuite/gcc.dg/pr91570.c
===
--- gcc/testsuite/gcc.dg/pr91570.c (nonexistent)
+++ gcc/testsuite/gcc.dg/pr91570.c (working copy)
@@ -0,0 +1,30 @@
+/* PR tree-optimization/91570 - ICE in get_range_strlen_dynamic on
+ a conditional of two strings
+ { dg-do compile }
+ { dg-options "-O2 -Wall" } */
+
+extern char a[], b[];
+
+/* Test case from comment #0 on the bug. */
+
+void comment_0 (int i)
+{
+ a[0] = 0;
+ b[0] = '1';
+
+ const char *p = i ? b : a;
+
+ if (__builtin_snprintf (0, 0, "%s", p) < 4)
+__builtin_abort ();
+}
+
+
+/* Test case from comment #2 on the bug. */
+
+void comment_2 (char *s)
+{
+ char *t = __builtin_strrchr (s, '/');
+ __builtin_strcat (s, ".SIF");
+ t = t ? t : s;
+ __builtin_printf ("%s", t);
+}
Index: gcc/tree-ssa-strlen.c
===
--- gcc/tree-ssa-strlen.c (revision 276021)
+++ gcc/tree-ssa-strlen.c (working copy)
@@ -896,7 +896,8 @@ get_range_strlen_dynamic (tree src, c_strlen_data
if (!argdata.minlen
|| (integer_zerop (argdata.minlen)
- && integer_all_onesp (argdata.maxbound)
+ && (!argdata.maxbound
+ || integer_all_onesp (argdata.maxbound))
&& integer_all_onesp (argdata.maxlen)))
{
/* Set the upper bound of the length to unbounded. */
@@ -910,11 +911,13 @@ get_range_strlen_dynamic (tree src, c_strlen_data
|| tree_int_cst_lt (argdata.minlen, pdata->minlen))
pdata->minlen = argdata.minlen;
if (!pdata->maxlen
- || tree_int_cst_lt (pdata->maxlen, argdata.maxlen))
+ || (argdata.maxlen
+ && tree_int_cst_lt (pdata->maxlen, argdata.maxlen)))
pdata->maxlen = argdata.maxlen;
if (!pdata->maxbound
- || (tree_int_cst_lt (pdata->maxbound,
- argdata.maxbound)
+ || (argdata.maxbound
+ && tree_int_cst_lt (pdata->maxbound,
+ argdata.maxbound)
&& !integer_all_onesp (argdata.maxbound)))
pdata->maxbound = argdata.maxbound;
}
@@ -1007,11 +1010,19 @@ get_range_strlen_dynamic (tree src, c_strlen_data
pdata->maxlen = build_all_ones_cst (size_type_node);
}
}
- else
+ else if (TREE_CODE (pdata->minlen) == INTEGER_CST)
{
pdata->maxlen = pdata->minlen;
pdata->maxbound = pdata->minlen;
}
+ else
+ {
+ /* For PDATA->MINLEN that's a non-constant expression such
+ as PLUS_EXPR whose value range is unknown, set the bounds
+ to zero and SIZE_MAX. */
+ pdata->minlen = build_zero_cst (size_type_node);
+ pdata->maxlen = build_all_ones_cst (size_type_node);
+ }
return true;
}
Re: Monotonically increasing counter (was Re: [Contrib PATCH] Add scripts to convert GCC repo from SVN to Git)
On Sat, Sep 21, 2019 at 05:32:03PM -0400, Jason Merrill wrote:
> On Sat, Sep 21, 2019 at 2:18 PM Segher Boessenkool
> wrote:
> >
> > On Thu, Sep 19, 2019 at 03:29:27PM -0400, Jason Merrill wrote:
> > > I suppose we also need to decide what form we want to use for the
> > > equivalent of gcc.gnu.org/rN. I figure it needs to be some prefix
> > > followed by a "commit-ish" (hash, tag, etc.). Perhaps "g:" as the
> > > prefix, so
> > >
> > > gcc.gnu.org/g:HEAD
> > > gcc.gnu.org/g:gcc-9-branch
> > > gcc.gnu.org/g:3cf0d8938a953e
> >
> > Hrm, but we should probably not allow everything here, some things can
> > be expensive to compute (HEAD~123456 for example), and we don't want to
> > expose the reflog on the server (if there even is one), etc. OTOH
> > allowing pretty much everything here is a quite neat idea.
> >
> > What do we use for gitweb thing? That might have a solution for this
> > already. Currently we seem to use plain gitweb, maybe cgit or similar
> > would be nicer? It looks more modern, anyway :-P
>
> The current gitweb allows all of my examples with a bit longer URL, e.g.
>
> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=3cf0d8938a953e
Yeah, they are all just refs, that should never be a problem.
> it doesn't seem to allow HEAD~1 or HEAD^. It does allow HEAD@{2}, but
> I don't see what the problem with that would be.
It is an O(n) amount of work for the web server. But not bigger than our
reflogs are, so that is fine if we run gc often enough (which we should
anyway, to keep a healthy fast repo).
This would be a much bigger problem for ~ because a) that isn't limited
in a similar way, and b) it is more expensive in any case (you have to
follow refs to get to the previous version, not simply go to the next
line in a file).
> I was figuring to just rewrite the /g: pattern into the appropriate
> query URL for the git web interface.
Yeah. I'm just cautious about the potential for DoS. But it looks to
be okay.
gitweb disallows some specific characters (and character sequences) here:
if ($input =~ m!(/\.|\.\.|[\000-\040\177 ~^:?*\[]|/$)!) {
return undef;
}
> I have no opinion about which web interface to use. Apparently there
> are many:
> https://git.wiki.kernel.org/index.php/Interfaces,_frontends,_and_tools#Web_Interfaces
Yes, and that is independent of the actual move to git; what we have now
will do for now, and OTOH the web thing can be changed already if we want
to do that.
I like that g: syntax btw.
Segher
Re: [PATCH] Microblaze: Type confusion in fprintf
On Fri, Sep 20, 2019 at 04:50:12PM -0600, Jeff Law wrote:
> On 9/12/19 7:33 AM, Jonas Pfeil wrote:
> > A type confusion leads to illegal (and nonsensical) assembly on certain host
> > architectures (e.g. ARM), where HOST_WIDE_INT (64 bit) and unsigned long (32
> > bit) have different alignments. So this has printed an uninitialized
> > register instead of the intended value. This fixes float constant
> > initialization on an ARM->Microblaze cross compiler.
> >
> > --- a/gcc/config/microblaze/microblaze.c
> > +++ b/gcc/config/microblaze/microblaze.c
> > @@ -2468,7 +2468,7 @@ print_operand (FILE * file, rtx op, int letter)
> > unsigned long value_long;
> > REAL_VALUE_TO_TARGET_SINGLE (*CONST_DOUBLE_REAL_VALUE (op),
> >value_long);
> > - fprintf (file, HOST_WIDE_INT_PRINT_HEX, value_long);
> > + fprintf (file, "%#lx", value_long);
> > }
> >else
> > {
> >
> But shouldn't HOST_WIDE_INT_PRINT_HEX here result in the same thing?
>
> >From hwint.h:
>
> #ifndef HAVE_INTTYPES_H
> #if INT64_T_IS_LONG
> # define GCC_PRI64 HOST_LONG_FORMAT
> #else
> # define GCC_PRI64 HOST_LONG_LONG_FORMAT
> #endif
>
> [ ... ]
>
> #undef PRIx64
> #define PRIx64 GCC_PRI64 "x"
> [ ... ]
>
> #define HOST_WIDE_INT_PRINT_HEX "%#" PRIx64
>
>
> If that's not giving us back %#lx, then it seems to me like something is
> wrong elsewhere.
This is a 32-bit target though, so INT64_T_IS_LONG is false.
Segher
Re: C++ PATCH for c++/91819 - ICE with operator++ and enum.
On 9/19/19 3:40 PM, Marek Polacek wrote: This is an ICE that started with the recent r275745. The problem here is that for a POSTINCREMENT_EXPR build_new_op_1 is called with null arg2, so arg2_type is also null after 5819 tree arg2_type = arg2 ? unlowered_expr_type (arg2) : NULL_TREE; but then we make arg2 nonnull 5887 if (code == POSTINCREMENT_EXPR || code == POSTDECREMENT_EXPR) 5888 arg2 = integer_zero_node; while arg2_type is still null and so 5940 else if (! arg2 || ! CLASS_TYPE_P (arg2_type)) crashes. Fixed by setting arg2_type in the ++/-- case. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-09-19 Marek Polacek PR c++/91819 - ICE with operator++ and enum. * call.c (build_new_op_1): Set arg2_type. * g++.dg/other/operator4.C: New test. OK, thanks. Jason
