On 10/17/2016 01:11 PM, Jeff Law wrote:
On 10/02/2016 02:10 PM, Martin Sebor wrote:
The attached patch fixes a number of outstanding test failures
and ILP32-related bugs in the gimple-ssa-sprintf pattch pointed
out in bug 77676 and 77735). The patch also fixes c_strlen to
correctly handle wide strings (previously it accepted them but
treated them as nul-terminated byte sequences), and adjusts the
handling of "%a" to avoid assuming a specific number of decimal
digits (this is likely a defect in C11 that I'm pursuing with
WG14).
Tested on powerpc64le, i386, and x86_64.
There is one outstanding failure in the builtin-sprintf-warn-1.c
test on powerpc64le that looks like it might be due to the
printf_pointer_format target hook not having been set up entirely
correctly. I'll look into that separately, along with pr77819.
Martin
gcc-77735.diff
PR middle-end/77735 - FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
gcc/ChangeLog:
2016-10-02 Martin Sebor <msebor@redhat.com>
PR middle-end/77735
* builtins.c (string_length): New function.
(c_strlen): Use string_length. Correctly handle wide strings.
* gimple-ssa-sprintf.c (target_max_value, target_size_max): New
functions.
(target_int_max): Call target_max_value.
(format_result::knownrange): New data member.
(fmtresult::fmtresult): Define default constructor.
(format_integer): Use it and set format_result::knownrange.
Handle global constants.
(format_floating_max): Add third argument.
(format_floating): Recompute maximum value for %a for each argument.
(get_string_length): Use fmtresult default ctor.
(format_string): Set format_result::knownrange.
(format_directive): Check format_result::knownrange.
(add_bytes): Same. Correct caret placement in diagnostics.
(pass_sprintf_length::compute_format_length): Set
format_result::knownrange.
(pass_sprintf_length::handle_gimple_call): Use target_size_max.
gcc/testsuite/ChangeLog:
2016-10-02 Martin Sebor <msebor@redhat.com>
PR middle-end/77735
* gcc.dg/tree-ssa/builtin-sprintf-2.c: Add test cases.
* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Same.
* gcc.dg/tree-ssa/builtin-sprintf-warn-2.c: Same.
* gcc.dg/tree-ssa/builtin-sprintf-warn-3.c: Adjust/relax.
* gcc.dg/tree-ssa/builtin-sprintf-warn-4.c: Add test cases.
* gcc.dg/tree-ssa/builtin-sprintf-warn-6.c: XFAIL for LP64 only.
* gcc.dg/tree-ssa/builtin-sprintf.c: Add test cases.
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 92f939e..410bbc3 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -1281,9 +1330,9 @@ format_floating (const conversion_spec &spec,
int width, int prec)
res.range.max = width;
}
- /* The argument is only considered bounded when the range of output
- bytes is exact. */
- res.bounded = res.range.min == res.range.max;
+ /* The argument is considered bounded when the output of a directive
+ is fully specified and the range of output bytes is exact. */
+ // res.bounded &= res.range.min == res.range.max;
Did you mean to leave this commented out?
I'm pretty sure I didn't mean to leave the code commented out like
that. I suspect I commented it out to confirm that it's not needed,
and forgot to remove it after it passed my testing. Let me remove
it (and thanks for pointing it out!)
It looks like you're defining a constructor for "struct fmtresult".
Normally once we define a constructor we turn the underlying object into
a class. It appears that the constructor leaves argmin, argmax,
knownrange, bounded & constant uninitialized? Am I missing something
here?
I added the default ctor to avoid having to explicitly clear the
members. The member() syntax means to default-initialize them by
setting them to zero or null:
struct fmtresult
{
+ fmtresult ()
+ : argmin (), argmax (), knownrange (), bounded (), constant ()
+ {
+ range.min = range.max = HOST_WIDE_INT_MAX;
+ }
+
Or did you have something else in mind? (I want to leave the members
public so they can conveniently accessed.)