On 10/31/2016 12:11 PM, Richard Biener wrote:
> On Mon, Oct 31, 2016 at 11:18 AM, Richard Sandiford
> <richard.sandif...@arm.com> wrote:
>> Richard Biener <richard.guent...@gmail.com> writes:
>>> On Mon, Oct 31, 2016 at 10:58 AM, Richard Sandiford
>>> <richard.sandif...@arm.com> wrote:
>>>> Richard Biener <richard.guent...@gmail.com> writes:
>>>>> On Mon, Oct 31, 2016 at 10:10 AM, Martin Liška <mli...@suse.cz> wrote:
>>>>>> On 10/31/2016 01:12 AM, Richard Sandiford wrote:
>>>>>>> Richard Biener <richard.guent...@gmail.com> writes:
>>>>>>>> On Thu, Oct 27, 2016 at 5:06 PM, Martin Liška <mli...@suse.cz> wrote:
>>>>>>>>> On 10/27/2016 03:35 PM, Richard Biener wrote:
>>>>>>>>>> On Thu, Oct 27, 2016 at 9:41 AM, Martin Liška <mli...@suse.cz> wrote:
>>>>>>>>>>> Running simple test-case w/o the proper header file causes ICE:
>>>>>>>>>>> strncmp ("a", "b", -1);
>>>>>>>>>>>
>>>>>>>>>>> 0xe74462 tree_to_uhwi(tree_node const*)
>>>>>>>>>>> ../../gcc/tree.c:7324
>>>>>>>>>>> 0x90a23f host_size_t_cst_p
>>>>>>>>>>> ../../gcc/fold-const-call.c:63
>>>>>>>>>>> 0x90a23f fold_const_call(combined_fn, tree_node*, tree_node*,
>>>>>>>>>>> tree_node*, tree_node*)
>>>>>>>>>>> ../../gcc/fold-const-call.c:1512
>>>>>>>>>>> 0x787b01 fold_builtin_3
>>>>>>>>>>> ../../gcc/builtins.c:8385
>>>>>>>>>>> 0x787b01 fold_builtin_n(unsigned int, tree_node*, tree_node**,
>>>>>>>>>>> int, bool)
>>>>>>>>>>> ../../gcc/builtins.c:8465
>>>>>>>>>>> 0x9052b1 fold(tree_node*)
>>>>>>>>>>> ../../gcc/fold-const.c:11919
>>>>>>>>>>> 0x6de2bb c_fully_fold_internal
>>>>>>>>>>> ../../gcc/c/c-fold.c:185
>>>>>>>>>>> 0x6e1f6b c_fully_fold(tree_node*, bool, bool*)
>>>>>>>>>>> ../../gcc/c/c-fold.c:90
>>>>>>>>>>> 0x67cbbf c_process_expr_stmt(unsigned int, tree_node*)
>>>>>>>>>>> ../../gcc/c/c-typeck.c:10369
>>>>>>>>>>> 0x67cfbd c_finish_expr_stmt(unsigned int, tree_node*)
>>>>>>>>>>> ../../gcc/c/c-typeck.c:10414
>>>>>>>>>>> 0x6cb578 c_parser_statement_after_labels
>>>>>>>>>>> ../../gcc/c/c-parser.c:5430
>>>>>>>>>>> 0x6cd333 c_parser_compound_statement_nostart
>>>>>>>>>>> ../../gcc/c/c-parser.c:4944
>>>>>>>>>>> 0x6cdbde c_parser_compound_statement
>>>>>>>>>>> ../../gcc/c/c-parser.c:4777
>>>>>>>>>>> 0x6c93ac c_parser_declaration_or_fndef
>>>>>>>>>>> ../../gcc/c/c-parser.c:2176
>>>>>>>>>>> 0x6d51ab c_parser_external_declaration
>>>>>>>>>>> ../../gcc/c/c-parser.c:1574
>>>>>>>>>>> 0x6d5c09 c_parser_translation_unit
>>>>>>>>>>> ../../gcc/c/c-parser.c:1454
>>>>>>>>>>> 0x6d5c09 c_parse_file()
>>>>>>>>>>> ../../gcc/c/c-parser.c:18173
>>>>>>>>>>> 0x72ffd2 c_common_parse_file()
>>>>>>>>>>> ../../gcc/c-family/c-opts.c:1087
>>>>>>>>>>>
>>>>>>>>>>> Following patch improves the host_size_t_cst_p predicate.
>>>>>>>>>>>
>>>>>>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives
>>>>>>>>>>> regression tests.
>>>>>>>>>>>
>>>>>>>>>>> Ready to be installed?
>>>>>>>>>>
>>>>>>>>>> I believe the wi::min_precision (t, UNSIGNED) <= sizeof (size_t) *
>>>>>>>>>> CHAR_BIT test is now redundant.
>>>>>>>>>>
>>>>>>>>>> OTOH it was probably desired to allow -1 here? A little looking back
>>>>>>>>>> in time should tell.
>>>>>>>>>
>>>>>>>>> Ok, it started with r229922, where it was changed from:
>>>>>>>>>
>>>>>>>>> if (tree_fits_uhwi_p (len) && p1 && p2)
>>>>>>>>> {
>>>>>>>>> const int i = strncmp (p1, p2, tree_to_uhwi (len));
>>>>>>>>> ...
>>>>>>>>>
>>>>>>>>> to current version:
>>>>>>>>>
>>>>>>>>> case CFN_BUILT_IN_STRNCMP:
>>>>>>>>> {
>>>>>>>>> bool const_size_p = host_size_t_cst_p (arg2, &s2);
>>>>>>>>>
>>>>>>>>> Thus I'm suggesting to change to back to it.
>>>>>>>>>
>>>>>>>>> Ready to be installed?
>>>>>>>>
>>>>>>>> Let's ask Richard.
>>>>>>>
>>>>>>> The idea with the:
>>>>>>>
>>>>>>> wi::min_precision (t, UNSIGNED) <= sizeof (size_t) * CHAR_BIT
>>>>>>>
>>>>>>> test was to stop us attempting 64-bit size_t operations on ILP32 hosts.
>>>>>>> I think we still want that.
>>>>>>
>>>>>> OK, so is the consensus to add tree_fits_uhwi_p predicate to the current
>>>>>> wi::min_precision check, right?
>>>>>
>>>>> Not sure. If we have host_size_t_cst_p then we should have a
>>>>> corresponding
>>>>> size_t host_size_t (const_tree) and should use those in pairs. Not sure
>>>>> why we have sth satisfying host_size_t_cst_p but not tree_fits_uhwi_p.
>>>>
>>>> It's the other way around: something can satisfy tree_fits_uhwi_p
>>>> (i.e. fit within a uint64_t) but not fit within the host's size_t.
>>>> The kind of case I'm thinking of is:
>>>>
>>>> strncmp ("fi", "fo", (1L << 32) + 1)
>>>>
>>>> for an ILP32 host and LP64 target. There's a danger that by passing
>>>> the uint64_t value (1L << 32) + 1 to the host's strncmp that we'd
>>>> truncate it to 1, giving the wrong result.
>>>
>>> Yes, but if it passes host_size_t_cst_p why does tree_to_uhwi ICE then?
>>> (unless we have a > 64bit host size_t).
>>
>> Because in Martin's test case the length has a signed type.
>> tree_to_uhwi then treats the argument as -1 to infinite precision
>> rather than ~(size_t) 0 to infinite precision.
>
> Indeed. So the bug is kind-of in the caller then. OTOH we could simply
> re-interpret the input as target size_t before doing the range check /
> conversion.
>
> I believe fold_const_call has the general issue of not verifying argument
> types
> before recognizing things as BUILT_IN_* (or the FE is at fault - but that's an
> old discussion...)
>
> Richard.
Updated and tested version of the patch that add types_compatible_p check
to host_size_t_cst_p.
Ready to be installed?
Martin
>
>> Thanks,
>> Richard
>From 0b0b36dc062fd0e9ffc7432d9d17bf4c6499b879 Mon Sep 17 00:00:00 2001
From: marxin <mli...@suse.cz>
Date: Wed, 26 Oct 2016 13:48:39 +0200
Subject: [PATCH] Fix host_size_t_cst_p predicate
gcc/ChangeLog:
2016-10-26 Martin Liska <mli...@suse.cz>
* fold-const-call.c (host_size_t_cst_p): Test whether
t is convertible to size_t.
gcc/testsuite/ChangeLog:
2016-10-26 Martin Liska <mli...@suse.cz>
* gcc.dg/tree-ssa/builtins-folding-gimple-ub.c (main): Add
test case.
---
gcc/fold-const-call.c | 4 +++-
gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-ub.c | 4 ++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/gcc/fold-const-call.c b/gcc/fold-const-call.c
index 05a15f9..1b3a755 100644
--- a/gcc/fold-const-call.c
+++ b/gcc/fold-const-call.c
@@ -29,6 +29,7 @@ along with GCC; see the file COPYING3. If not see
#include "case-cfn-macros.h"
#include "tm.h" /* For C[LT]Z_DEFINED_AT_ZERO. */
#include "builtins.h"
+#include "gimple-expr.h"
/* Functions that test for certain constant types, abstracting away the
decision about whether to check for overflow. */
@@ -57,7 +58,8 @@ complex_cst_p (tree t)
static inline bool
host_size_t_cst_p (tree t, size_t *size_out)
{
- if (integer_cst_p (t)
+ if (types_compatible_p (size_type_node, TREE_TYPE (t))
+ && integer_cst_p (t)
&& wi::min_precision (t, UNSIGNED) <= sizeof (size_t) * CHAR_BIT)
{
*size_out = tree_to_uhwi (t);
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-ub.c b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-ub.c
index df0ede2..e1658d1 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-ub.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-ub.c
@@ -17,6 +17,10 @@ main (void)
if (__builtin_memchr (foo1, 'x', 1000)) /* Not folded away. */
__builtin_abort ();
+ /* STRNCMP. */
+ if (strncmp ("a", "b", -1)) /* { dg-warning "implicit declaration of function" } */
+ __builtin_abort ();
+
return 0;
}
--
2.10.1
