Re: [PATCH 2/3] GDB: Add support for 24 bit addresses

[John Darrington]

On Fri, Aug 24, 2018 at 04:34:11PM -0400, Simon Marchi wrote:
 (CCing gcc-patches because of the change in include/dwarf2.h)
 
 This file is owned by GCC (see the MAINTAINERS file at the top of
 binutils-gdb).  To get a modification in it, you would need to provide a
 patch to gcc, then we can import the change in binutils-gdb.
 
 I am not too sure who is responsible for allocating these values, as they
 are not from the DWARF standard.  At the very least, there should be a
 comment to say what architecture uses this non-standard value.  Is there
 also a gcc port you are planning to upstream, that would use this value?

I might think about a gcc port when Gdb and Binutils are robust for this
arch.   But I haven't started any work on that so far.

J'
 

-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.

Re: PING [PATCH] warn for strlen of arrays with missing nul (PR 86552, 86711, 86714) )

[Jeff Law]

On 08/24/2018 11:26 AM, Bernd Edlinger wrote:
> On 08/24/18 18:51, Jeff Law wrote:
>>> Well, this is broken for wide character strings.
>>> but I hope we can get rid of STRING_CST which are
>>> not explicitly null terminated.
> 
> I am afraid that is not going to happen.
> Maybe we can get STRING_CST that are never longer
> than the TYPE_UNIT_SIZE, but c_strlen and c_getstr
> need to take care that the string is zero-terminated.
> 
> string_constant, should not promise the string is zero terminated.
> But instead it can promise that:
> 1) the STRING_CST is valid up to TREE_STRING_LENGTH
> 2) mem_size is >= TREE_STRING_LENGTH
> 3) memory between TREE_STRING_LENGTH and mem_size is ZERO.
> 
> It will not guarantee anything about zero termination any more.
Interesting because those conditions would be sufficient to deal with a
regression I stumbled over after fixing Martin's patch to not assume
that all STRING_CSTs are NUL terminated.

But I need to think about this a bit more.  Essentially the question
we'd need to ask is whether or not these are sufficient in general or
just in specific cases.

I tend to think they're not sufficient in general. If a string returned
by string_constant that didn't have a terminating NUL, but which did
pass the tests above were ultimately passed to the runtime's str*
routines, then the call may run off the end of the string.  We'd like to
be able to warn for that.

So ISTM those rules are only valid in contexts where we know the result
isn't going to be passed to str* and friends within the C library.

I do think they're sufficient to avoid problems with the
tree-ssa-forwprop code we've looked at.  So what may make the most sense
is to have that routine indicate it's willing to accept unterminated
strings, then check the conditions above before optimizing the code.

> 
> In the end, the best approach might be to either merge my patch
> with Martins, or step-wise, first fixing wrong code, and then
> implementing warnings without fixing wrong code.
Unsure at this time.  I've been working with both.  I suspect that if we
went with yours that we'd then turn around and layer Martin's on top of
it because of the desire to signal to callers that we have an
unterminated string and have the callers take appropriate action.  Which
begs the question of whether or not we just go with Martin's -- ie, is
there really any value in using both.  I haven't seen indications there
is value in that approach, but I'm still poking at things.

Jeff

Re: PING^1: [PATCH] Set start_location to 0 if we ran out of line map space

[H.J. Lu]

On Fri, Aug 24, 2018 at 4:05 PM, David Malcolm  wrote:
> On Wed, 2018-08-22 at 12:48 -0700, H.J. Lu wrote:
>> PING.
>
> [stripping out-of-date address for Tom Tromey from recipients]
>
> Sorry for the belated response.
>
>> -- Forwarded message --
>> From: H.J. Lu 
>> Date: Wed, Aug 15, 2018 at 4:33 AM
>> Subject: [PATCH] Set start_location to 0 if we ran out of line map
>> space
>> To: gcc-patches@gcc.gnu.org
>>
>>
>> With profiledbootstrap and --with-build-config=bootstrap-lto,
>> linemap_add
>> may create a macro map when we run out of line map space.  This patch
>> changes start_location to UNKNOWN_LOCATION (0) in this case.
>>
>> Tested with profiledbootstrap and --with-build-config=bootstrap-lto
>> on
>> Linux/x86-64.
>>
>> PR bootstrap/86872
>> * line-map.c (pure_location_p): Return true if linemap_lookup
>> returns NULL.
>> (linemap_add): Set start_location to 0 if we run out of line
>> map
>> space.
>> ---
>>  libcpp/line-map.c | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/libcpp/line-map.c b/libcpp/line-map.c
>> index 555cd129a9c..cafe42273eb 100644
>> --- a/libcpp/line-map.c
>> +++ b/libcpp/line-map.c
>> @@ -304,6 +304,8 @@ pure_location_p (line_maps *set, source_location
>> loc)
>>  return false;
>>
>>const line_map *map = linemap_lookup (set, loc);
>> +  if (map == NULL)
>> +return true;
>>const line_map_ordinary *ordmap = linemap_check_ordinary (map);
>
> I was wondering why this is needed, but it's presumably due to this
> clause:
>
>   if (set ==  NULL || line < RESERVED_LOCATION_COUNT)
> return NULL;
>
> in linemap_ordinary_map_lookup.

and

 /* Any ordinary locations ought to be "pure" at this point: no
 compressed ranges.  */
  linemap_assert (locus < RESERVED_LOCATION_COUNT
  || locus >= LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES
  || locus >= LINEMAPS_MACRO_LOWEST_LOCATION (set)
  || pure_location_p (set, locus));

  /* Consider short-range optimization.  */
  if (can_be_stored_compactly_p (set, locus, src_range, data))
{
  /* The low bits ought to be clear.  */
  linemap_assert (pure_location_p (set, locus));


> Normally, the ordinary_maps are monotonically increasing by start
> location, but with the next hunk to linemap_add there will be a map at
> the end for location 0 which thus isn't monotonic.
>
>>if (loc & ((1U << ordmap->m_range_bits) - 1))
>> @@ -492,6 +494,11 @@ linemap_add (struct line_maps *set, enum
>> lc_reason reason,
>>  }
>>
>>linemap_assert (reason != LC_ENTER_MACRO);
>> +
>> +  if (start_location >= LINE_MAP_MAX_LOCATION)
>> +/* We ran out of line map space.   */
>> +start_location = 0;
>> +
>
> If I'm reading this right, this means that we create an ordinary map
> that starts at location 0.  The caller, linemap_line_start should then
> bail out, returning location 0, and subsequent calls to
> linemap_line_start should return 0 without attempting to create another
> map.
>
> My first thought was that instead linemap_add ought to return NULL for
> this case, and linemap_line_start should check for NULL return and bail
> out, but I believe this approach would mean doing slightly more work
> per line.  Out of interest, did you try this other approach?

Yes, I did.  But it didn't work since callers of inemap_add expect
non-NULL return value for this case.

> I wonder if there's a sane way to reproduce this failure using
> gcc.dg/plugin/location_overflow_plugin.c ?
>
>>line_map_ordinary *map
>>  = linemap_check_ordinary (new_linemap (set, start_location));
>>map->reason = reason;
>> --
>> 2.17.1
>
> Given the testing this has had, and the awkwardness of reproducing,
> this is OK for trunk as-is.
>
> Thanks
> Dave
>

I will check it in.

Thanks.


-- 
H.J.

Re: ToT build problem on Aarch64 after cfg.h change

[Jeff Law]

On 08/24/2018 05:04 PM, Steve Ellcey wrote:
> Richard,
> 
> I am having a problem building GCC after this patch:
> 
> commit 2515797e5db67076d6cf7f3f185757c841f79edf
> Author: rguenth 
> Date:   Fri Aug 24 11:17:16 2018 +
> 
> 2018-08-24  Richard Biener  
>    
> * cfg.h (struct control_flow_graph): Add edge_flags_allocated and
> bb_flags_allocated members.
> (auto_flag): New RAII class for allocating flags.
> (auto_edge_flag): New RAII class for allocating edge flags.
> (auto_bb_flag): New RAII class for allocating bb flags.
> * cfgloop.c (verify_loop_structure): Allocate temporary edge
> flag dynamically.
> * cfganal.c (dfs_enumerate_from): Remove use of visited sbitmap
> in favor of temporarily allocated BB flag.
> * hsa-brig.c: Re-order includes.
> * hsa-dump.c: Likewise.
> * hsa-regalloc.c: Likewise.
> * print-rtl.c: Likewise.
> * profile-count.c: Likewise.
> 
> The failure is:
> 
> In file included from 
> /home/sellcey/gcc-m/src/gcc/gcc/config/aarch64/aarch64-speculation.cc:28:0:
> /home/sellcey/gcc-m/src/gcc/gcc/cfg.h: In constructor 
> ‘auto_edge_flag::auto_edge_flag(function*)’:
> /home/sellcey/gcc-m/src/gcc/gcc/cfg.h:172:22: error: invalid use of 
> incomplete type ‘struct function’
>  : auto_flag (>cfg->edge_flags_allocated) {}
> 
> It looks like this may be Aarch64 specific build problem since it is 
> compiling a platform specific file.  Is there just a missing include?
That would be my guess.
jeff

Re: PING^1: [PATCH] Set start_location to 0 if we ran out of line map space

[David Malcolm]

On Wed, 2018-08-22 at 12:48 -0700, H.J. Lu wrote:
> PING.

[stripping out-of-date address for Tom Tromey from recipients]

Sorry for the belated response.

> -- Forwarded message --
> From: H.J. Lu 
> Date: Wed, Aug 15, 2018 at 4:33 AM
> Subject: [PATCH] Set start_location to 0 if we ran out of line map
> space
> To: gcc-patches@gcc.gnu.org
> 
> 
> With profiledbootstrap and --with-build-config=bootstrap-lto,
> linemap_add
> may create a macro map when we run out of line map space.  This patch
> changes start_location to UNKNOWN_LOCATION (0) in this case.
> 
> Tested with profiledbootstrap and --with-build-config=bootstrap-lto
> on
> Linux/x86-64.
> 
> PR bootstrap/86872
> * line-map.c (pure_location_p): Return true if linemap_lookup
> returns NULL.
> (linemap_add): Set start_location to 0 if we run out of line
> map
> space.
> ---
>  libcpp/line-map.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/libcpp/line-map.c b/libcpp/line-map.c
> index 555cd129a9c..cafe42273eb 100644
> --- a/libcpp/line-map.c
> +++ b/libcpp/line-map.c
> @@ -304,6 +304,8 @@ pure_location_p (line_maps *set, source_location
> loc)
>  return false;
> 
>const line_map *map = linemap_lookup (set, loc);
> +  if (map == NULL)
> +return true;
>const line_map_ordinary *ordmap = linemap_check_ordinary (map);

I was wondering why this is needed, but it's presumably due to this
clause:

  if (set ==  NULL || line < RESERVED_LOCATION_COUNT)
return NULL;

in linemap_ordinary_map_lookup.

Normally, the ordinary_maps are monotonically increasing by start
location, but with the next hunk to linemap_add there will be a map at
the end for location 0 which thus isn't monotonic.

>if (loc & ((1U << ordmap->m_range_bits) - 1))
> @@ -492,6 +494,11 @@ linemap_add (struct line_maps *set, enum
> lc_reason reason,
>  }
> 
>linemap_assert (reason != LC_ENTER_MACRO);
> +
> +  if (start_location >= LINE_MAP_MAX_LOCATION)
> +/* We ran out of line map space.   */
> +start_location = 0;
> +

If I'm reading this right, this means that we create an ordinary map
that starts at location 0.  The caller, linemap_line_start should then
bail out, returning location 0, and subsequent calls to
linemap_line_start should return 0 without attempting to create another
map.

My first thought was that instead linemap_add ought to return NULL for
this case, and linemap_line_start should check for NULL return and bail
out, but I believe this approach would mean doing slightly more work
per line.  Out of interest, did you try this other approach?

I wonder if there's a sane way to reproduce this failure using 
gcc.dg/plugin/location_overflow_plugin.c ?

>line_map_ordinary *map
>  = linemap_check_ordinary (new_linemap (set, start_location));
>map->reason = reason;
> --
> 2.17.1

Given the testing this has had, and the awkwardness of reproducing,
this is OK for trunk as-is.

Thanks
Dave

ToT build problem on Aarch64 after cfg.h change

[Steve Ellcey]

Richard,

I am having a problem building GCC after this patch:

commit 2515797e5db67076d6cf7f3f185757c841f79edf
Author: rguenth 
Date:   Fri Aug 24 11:17:16 2018 +

2018-08-24  Richard Biener  
   
* cfg.h (struct control_flow_graph): Add edge_flags_allocated and
bb_flags_allocated members.
(auto_flag): New RAII class for allocating flags.
(auto_edge_flag): New RAII class for allocating edge flags.
(auto_bb_flag): New RAII class for allocating bb flags.
* cfgloop.c (verify_loop_structure): Allocate temporary edge
flag dynamically.
* cfganal.c (dfs_enumerate_from): Remove use of visited sbitmap
in favor of temporarily allocated BB flag.
* hsa-brig.c: Re-order includes.
* hsa-dump.c: Likewise.
* hsa-regalloc.c: Likewise.
* print-rtl.c: Likewise.
* profile-count.c: Likewise.

The failure is:

In file included from 
/home/sellcey/gcc-m/src/gcc/gcc/config/aarch64/aarch64-speculation.cc:28:0:
/home/sellcey/gcc-m/src/gcc/gcc/cfg.h: In constructor 
‘auto_edge_flag::auto_edge_flag(function*)’:
/home/sellcey/gcc-m/src/gcc/gcc/cfg.h:172:22: error: invalid use of incomplete 
type ‘struct function’
 : auto_flag (>cfg->edge_flags_allocated) {}

It looks like this may be Aarch64 specific build problem since it is 
compiling a platform specific file.  Is there just a missing include?

Steve Ellcey
sell...@cavium.com

Re: PING [PATCH] warn for strlen of arrays with missing nul (PR 86552, 86711, 86714) )

[Bernd Edlinger]

On 08/24/18 18:04, Jeff Law wrote:
> On 08/24/2018 06:27 AM, Bernd Edlinger wrote:
> [ Lots of snipping throughout ]
> 
> 

> +
>  if (TREE_CODE (src) == COND_EXPR
>  && (only_value || !TREE_SIDE_EFFECTS (TREE_OPERAND (src, 0
>{
> -  tree len1, len2;
> -
> -  len1 = c_strlen (TREE_OPERAND (src, 1), only_value);
> -  len2 = c_strlen (TREE_OPERAND (src, 2), only_value);
> +  tree len1 = c_strlen (TREE_OPERAND (src, 1), only_value, arrs);
> +  tree len2 = c_strlen (TREE_OPERAND (src, 2), only_value, arrs + 1);
>  if (tree_int_cst_equal (len1, len2))
> - return len1;
> + {

 funny, if called with NULL *arr and arrs[0] alias each other.
>>>

> +   *arr = arrs[0] ? arrs[0] : arrs[1];
> +   return len1;
> + }
>}
>>>
>>> And in this case it looks like your comment is before the code you're
>>> commenting about.  It was fairly obvious in this case because the code
>>> prior to your "funny, if called..." comment didn't reference arr or arrs
>>> at all.
>>>
>>> And more importantly, why are you concerned about the aliasing?
>>>
>>
>> It is just *arr = arrs[0] does nothing, but it looks like the author
>> was not aware of it.  It may be okay, but causes head-scratching.
>> If you don't have the context you will think this does something different.
> *arr = arrs[0] ? arrs[0] : arrs[1];
> 
> Yes, there's potentially a dead store when arr points to arrs[0] because
> of the earlier initialization when NULL was passed for arr.  But
> otherwise we'd be checking repeatedly that arr != NULL.
> 
> 
>  /* PTR can point to the byte representation of any string type, 
> including
> char* and wchar_t*.  */
>  const char *ptr = TREE_STRING_POINTER (src);
> @@ -650,7 +709,8 @@ c_strlen (tree src, int only_value)
>  offsave = fold_convert (ssizetype, offsave);
>  tree condexp = fold_build2_loc (loc, LE_EXPR, boolean_type_node, 
> offsave,
> build_int_cst (ssizetype, len * 
> eltsize));

 this computation is wrong, it computes not in units of eltsize,
 I am however not sure if it is really good that this function tries to
 compute strlen of wide character strings.
>>> Note that in the second version of Martin's patch eltsize will always be
>>> 1 when we get here.  Furthermore, the multiplication by eltsize is gone.
>>>It looks like you switched back to commenting after the code for that
>>> comment, but then immediately thereafter...
>>>
>>
>> Acutally I had no idea that the second patch did resolve some of my comments
>> and which those were.
> It happens.  Multiple long threads make it difficult to follow.
> 
>>
>> I had the impression that it is just splitting up of a large patch into
>> several smaller without reworking at the same time.
>>
>> Once again, a summary what was changed would be helpful.
> Agreed.
> 
> 
 What are you fixing here, I think that was another bug.
 If this fixes something then it should be in a different patch,
 just handling this.

>  unsigned len = string_length (ptr + eltoff * eltsize, eltsize,
> - maxelts - eltoff);
> + strelts - eltoff);
>
>  return ssize_int (len);
>}
>>> I'm guessing the comment in this case refers to the code after.
>>> Presumably questioning the change from maxelts to strelts.
>>>
>>>
>>
>> Yes.  I was thinking that could be a patch of its own.
> Funny.  I found myself in agreement and was going to extract this out
> and run it through the usual tests and get it onto the trunk
> immediately.  Then I realized you'd already fixed this in the patch that
> added the eltsize paramter to c_strlen which has already been committed
> :-)
> 
> 
> 
>>
>> Well, yes, although I changed my mind on strlen(L"abc") meanwhile.
>>
>> This may appear as if I contradict myself but, it is more like I learn.
>>
>> The installed patch for the ELTSIZE parameter was in part inspired by the
>> thought about "not folding invalid stuff" that was forming in my mind
>> at that time, even before I wrote it down and sent it to this list.
> ACK.  And I think there seems to be consensus forming around that
> concept which is good.
> 
> If we ultimately decide not to fold strlen of a wide character string,
> then that'll be an easy enough change to make.  In the mean time bring
> consistent with how the C library strlen works is a good thing IMHO.
> 
> 
> @@ -8255,19 +8333,30 @@ fold_builtin_classify_type (tree arg)
>  return build_int_cst (integer_type_node, type_to_class (TREE_TYPE 
> (arg)));
>}
>
> -/* Fold a call to __builtin_strlen with argument ARG.  */
> +/* Fold a strlen call to FNDECL of TYPE, and with argument ARG.  */
>
>static tree
> -fold_builtin_strlen (location_t loc, 

[PATCH] Adjust string_constant to new STRING_CST semantics

[Bernd Edlinger]

Hi,

I noticed that the latest patches to make STRING_CST
semantics for literals and for initializers consistent
also fixes PR 86711 and PR 86714 (but not PR 87053).

The code change in the string_constant does not change
any test case, the new test cases are already fixed
once we have the unified STRING_CST semantics.
Thus https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html
would be the pre-condition for this patch.

If the pre-condition patch is applied then my earlier patch
to fix PR 86711/86714 is no longer necessary.
Likely neither Martin's patch (at least that part about
PR 86711/86714).


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.
gcc:
2018-08-24  Bernd Edlinger  

	* expr.c (string_constant): Adjust function comment.
	Remove bogus check for zero termination.

testsuite:
2018-08-24  Bernd Edlinger  

	PR middle-end/86711
	PR middle-end/86714
	* gcc.c-torture/execute/pr86711.c: New test.
	* gcc.c-torture/execute/pr86714.c: New test.

diff -Npur gcc/expr.c gcc/expr.c
--- gcc/expr.c	2018-08-22 22:34:01.0 +0200
+++ gcc/expr.c	2018-08-24 20:24:21.592913041 +0200
@@ -11304,8 +11304,9 @@ is_aligning_offset (const_tree offset, c
if it doesn't.  If we return nonzero, set *PTR_OFFSET to the (possibly
non-constant) offset in bytes within the string that ARG is accessing.
The type of the offset is sizetype.  If MEM_SIZE is non-zero the storage
-   size of the memory is returned.  If MEM_SIZE is zero, the string is
-   only returned when it is properly zero terminated.  */
+   size of the memory is returned.  The returned STRING_CST object is
+   valid up to TREE_STRING_LENGTH.  Bytes between TREE_STRING_LENGTH
+   and MEM_SIZE are ZERO.  MEM_SIZE is at least TREE_STRING_LENGTH.  */
 
 tree
 string_constant (tree arg, tree *ptr_offset, tree *mem_size)
@@ -11404,6 +11405,8 @@ string_constant (tree arg, tree *ptr_off
   *ptr_offset = fold_convert (sizetype, offset);
   if (mem_size)
 	*mem_size = TYPE_SIZE_UNIT (TREE_TYPE (array));
+  gcc_checking_assert (tree_to_shwi (TYPE_SIZE_UNIT (TREE_TYPE (array)))
+			   >= TREE_STRING_LENGTH (array));
   return array;
 }
 
@@ -11446,27 +11449,10 @@ string_constant (tree arg, tree *ptr_off
   if (!init || TREE_CODE (init) != STRING_CST)
 return NULL_TREE;
 
-  tree array_size = DECL_SIZE_UNIT (array);
-  if (!array_size || TREE_CODE (array_size) != INTEGER_CST)
-return NULL_TREE;
-
-  /* Avoid returning a string that doesn't fit in the array
- it is stored in, like
- const char a[4] = "abcde";
- but do handle those that fit even if they have excess
- initializers, such as in
- const char a[4] = "abc\000\000";
- The excess elements contribute to TREE_STRING_LENGTH()
- but not to strlen().  */
-  unsigned HOST_WIDE_INT charsize
-= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (init;
-  unsigned HOST_WIDE_INT length = TREE_STRING_LENGTH (init);
-  length = string_length (TREE_STRING_POINTER (init), charsize,
-			  length / charsize);
   if (mem_size)
 *mem_size = TYPE_SIZE_UNIT (TREE_TYPE (init));
-  else if (compare_tree_int (array_size, length + 1) < 0)
-return NULL_TREE;
+  gcc_checking_assert (tree_to_shwi (TYPE_SIZE_UNIT (TREE_TYPE (init)))
+		   >= TREE_STRING_LENGTH (init));
 
   *ptr_offset = offset;
   return init;
diff -Npur gcc/testsuite/gcc.c-torture/execute/pr86711.c gcc/testsuite/gcc.c-torture/execute/pr86711.c
--- gcc/testsuite/gcc.c-torture/execute/pr86711.c	1970-01-01 01:00:00.0 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr86711.c	2018-08-24 20:29:15.960882045 +0200
@@ -0,0 +1,11 @@
+/* PR middle-end/86711 */
+
+static const char a[2][4] = { "1234", "5678" };
+
+int main ()
+{
+  void *p = __builtin_memchr (a, 0, 5);
+
+  if (p)
+__builtin_abort ();
+}
diff -Npur gcc/testsuite/gcc.c-torture/execute/pr86714.c gcc/testsuite/gcc.c-torture/execute/pr86714.c
--- gcc/testsuite/gcc.c-torture/execute/pr86714.c	1970-01-01 01:00:00.0 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr86714.c	2018-08-24 20:29:15.960882045 +0200
@@ -0,0 +1,12 @@
+/* PR middle-end/86714 */
+
+const char a[2][3] = { "1234", "xyz" };
+char b[6];
+
+int main ()
+{
+  __builtin_memcpy (b, a, 4);
+  __builtin_memset (b + 4, 'a', 2);
+  if (__builtin_memcmp (b, "123xaa", 6))
+__builtin_abort ();
+}

[committed] diagnostics: tweaks to line-spans vs line numbering (PR 87091)

[David Malcolm]

This patch tweaks how line numbers are printed for a diagnostic
containing multiple line spans.

With this patch, rather than printing span headers:

  ../x86_64-pc-linux-gnu/libstdc++-v3/include/vector:87:22: note: message
  ../x86_64-pc-linux-gnu/libstdc++-v3/include/vector:74:1:
  ++ |+#include 
  74 | #endif
  ../x86_64-pc-linux-gnu/libstdc++-v3/include/vector:87:22:
  87 |   using vector = std::vector<_Tp, polymorphic_allocator<_Tp>>;
 |  ^~~

we now print:

  ../x86_64-pc-linux-gnu/libstdc++-v3/include/vector:87:22: note: message
  +++ |+#include 
   74 | #endif
  
   87 |   using vector = std::vector<_Tp, polymorphic_allocator<_Tp>>;
  |  ^~~

and for sufficiently close lines, rather than print a gap:

  + |+#include 
  1 | test (int ch)
  ..
  3 |  putchar (ch);
|  ^~~

we print the line itself:

  + |+#include 
  1 | test (int ch)
  2 | {
  3 |  putchar (ch);
|  ^~~

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

Committed to trunk as r263843.

gcc/ChangeLog:
PR 87091
* diagnostic-show-locus.c (layout::layout): Ensure the margin is
wide enough for jumps in the line-numbering to be visible.
(layout::print_gap_in_line_numbering): New member function.
(layout::calculate_line_spans): When using line numbering, merge
line spans that are only 1 line apart.
(diagnostic_show_locus): When printing line numbers, show gaps in
line numbering directly, rather than printing headers.
(selftest::test_diagnostic_show_locus_fixit_lines): Add test of
line-numbering with multiple line spans.
(selftest::test_fixit_insert_containing_newline_2): Add test of
line-numbering, in which the spans are close enough to be merged.

gcc/testsuite/ChangeLog:
PR 87091
* gcc.dg/missing-header-fixit-3.c: Update for changes to how
line spans are printed with -fdiagnostics-show-line-numbers.
---
 gcc/diagnostic-show-locus.c   | 138 +-
 gcc/testsuite/gcc.dg/missing-header-fixit-3.c |  13 +--
 2 files changed, 116 insertions(+), 35 deletions(-)

diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index a759826..1e7f969 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -241,6 +241,7 @@ class layout
   int get_num_line_spans () const { return m_line_spans.length (); }
   const line_span *get_line_span (int idx) const { return _line_spans[idx]; }
 
+  void print_gap_in_line_numbering ();
   bool print_heading_for_line_span_index_p (int line_span_idx) const;
 
   expanded_location get_expanded_location (const line_span *) const;
@@ -923,6 +924,9 @@ layout::layout (diagnostic_context * context,
   if (highest_line < 0)
 highest_line = 0;
   m_linenum_width = num_digits (highest_line);
+  /* If we're showing jumps in the line-numbering, allow at least 3 chars.  */
+  if (m_line_spans.length () > 1)
+m_linenum_width = MAX (m_linenum_width, 3);
 
   /* Adjust m_x_offset.
  Center the primary caret to fit in max_width; all columns
@@ -1059,6 +1063,20 @@ layout::will_show_line_p (linenum_type row) const
   return false;
 }
 
+/* Print a line showing a gap in the line numbers, for showing the boundary
+   between two line spans.  */
+
+void
+layout::print_gap_in_line_numbering ()
+{
+  gcc_assert (m_show_line_numbers_p);
+
+  for (int i = 0; i < m_linenum_width + 1; i++)
+pp_character (m_pp, '.');
+
+  pp_newline (m_pp);
+}
+
 /* Return true iff we should print a heading when starting the
line span with the given index.  */
 
@@ -1156,21 +1174,34 @@ get_line_span_for_fixit_hint (const fixit_hint *hint)
This function populates m_line_spans with an ordered, disjoint list of
the line spans of interest.
 
-   For example, if the primary caret location is on line 7, with ranges
-   covering lines 5-6 and lines 9-12:
+   Printing a gap between line spans takes one line, so, when printing
+   line numbers, we allow a gap of up to one line between spans when
+   merging, since it makes more sense to print the source line rather than a
+   "gap-in-line-numbering" line.  When not printing line numbers, it's
+   better to be more explicit about what's going on, so keeping them as
+   separate spans is preferred.
+
+   For example, if the primary range is on lines 8-10, with secondary ranges
+   covering lines 5-6 and lines 13-15:
 
  004
- 005   |RANGE 0
- 006   |RANGE 0
- 007  |PRIMARY CARET
- 008
- 009|RANGE 1
- 010|RANGE 1
- 011|RANGE 1
- 012|RANGE 1
- 013
-
-   then we want two spans: lines 5-7 and lines 9-12.  */
+ 005   |RANGE 1
+ 006   |RANGE 1
+ 007
+ 008  |PRIMARY RANGE
+ 009  

C++ PATCH for c++/87080, ICE with -Wpessimizing-move

[Marek Polacek]

The problem in this testcase was that we were calling is_std_move_p from
template context, which breaks in cp_get_fndecl_from_callee.  This warning
is not meant to warn while parsing a template, so I think we should apply this.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-08-24  Marek Polacek  

PR c++/87080
* typeck.c (maybe_warn_pessimizing_move): Do nothing in a template.

* g++.dg/cpp0x/Wpessimizing-move5.C: New test.

diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 122d9dcd4b3..24647e29a55 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -9192,6 +9192,11 @@ maybe_warn_pessimizing_move (tree retval, tree functype)
   if (cxx_dialect < cxx11)
 return;
 
+  /* Wait until instantiation time, since we can't gauge if we should do
+ the NRVO until then.  */
+  if (processing_template_decl)
+return;
+
   /* This is only interesting for class types.  */
   if (!CLASS_TYPE_P (functype))
 return;
diff --git gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move5.C 
gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move5.C
index e69de29bb2d..02ad2113505 100644
--- gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move5.C
+++ gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move5.C
@@ -0,0 +1,14 @@
+// PR c++/87080
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wpessimizing-move" }
+
+struct a {
+  template a <<(b);
+};
+a c();
+template
+a fn2()
+{
+  int d = 42;
+  return c() << d;
+}

Re: [PATCH 2/3] GDB: Add support for 24 bit addresses

[Simon Marchi]

(CCing gcc-patches because of the change in include/dwarf2.h)

On 2018-08-23 13:35, John Darrington wrote:

* include/dwarf2.h (enum dwarf_unit_type)[DW_EH_PE_udata3]: New member.
* gdb/dwarf2-frame.c (encoding_for_size): Deal with case 3.
(read_encoded_value): Deal with case DW_EH_PE_udata3
---
 gdb/dwarf2-frame.c | 7 ++-
 include/dwarf2.h   | 5 -
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index f7dc820f4d..b329e34997 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -1527,12 +1527,14 @@ encoding_for_size (unsigned int size)
 {
 case 2:
   return DW_EH_PE_udata2;
+case 3:
+  return DW_EH_PE_udata3;
 case 4:
   return DW_EH_PE_udata4;
 case 8:
   return DW_EH_PE_udata8;
 default:
-  internal_error (__FILE__, __LINE__, _("Unsupported address 
size"));

+  internal_error (__FILE__, __LINE__, _("Unsupported address size
%d"), size);
 }
 }

@@ -1605,6 +1607,9 @@ read_encoded_value (struct comp_unit *unit,
gdb_byte encoding,
 case DW_EH_PE_udata2:
   *bytes_read_ptr += 2;
   return (base + bfd_get_16 (unit->abfd, (bfd_byte *) buf));
+case DW_EH_PE_udata3:
+  *bytes_read_ptr += 3;
+  return (base + bfd_get_24 (unit->abfd, (bfd_byte *) buf));
 case DW_EH_PE_udata4:
   *bytes_read_ptr += 4;
   return (base + bfd_get_32 (unit->abfd, (bfd_byte *) buf));
diff --git a/include/dwarf2.h b/include/dwarf2.h
index cf0039a92a..05c328057b 100644
--- a/include/dwarf2.h
+++ b/include/dwarf2.h
@@ -474,11 +474,14 @@ enum dwarf_unit_type
 #define DW_EH_PE_udata20x02
 #define DW_EH_PE_udata40x03
 #define DW_EH_PE_udata80x04
+
+#define DW_EH_PE_udata30x05
+
+#define DW_EH_PE_signed0x08
 #define DW_EH_PE_sleb128   0x09
 #define DW_EH_PE_sdata20x0A
 #define DW_EH_PE_sdata40x0B
 #define DW_EH_PE_sdata80x0C
-#define DW_EH_PE_signed0x08

 #define DW_EH_PE_pcrel 0x10
 #define DW_EH_PE_textrel   0x20


This file is owned by GCC (see the MAINTAINERS file at the top of 
binutils-gdb).  To get a modification in it, you would need to provide a 
patch to gcc, then we can import the change in binutils-gdb.


I am not too sure who is responsible for allocating these values, as 
they are not from the DWARF standard.  At the very least, there should 
be a comment to say what architecture uses this non-standard value.  Is 
there also a gcc port you are planning to upstream, that would use this 
value?


Simon

Re: [PATCH] Check the STRING_CSTs in varasm.c

[Bernd Edlinger]

Hi!


This is an alternative approach in making STRING_CST semantics
consistent.

This means it makes STRING_CST used for literals and for initializers
use exactly the same semantics.

It makes sure that all STRING_CST have a TYPE_SIZE_UNIT, and that it is
always larger than TREE_STRING_LENGTH, these and certain other properties
are checked in varasm.c, so the currently observed consistency issues
in the middle-end can be resolved.


It depends on the following pre-condition patches:

[PATCH] Use complete_array_type on flexible array member initializers
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01538.html

PATCHv2] Call braced_list_to_string after array size is fixed
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01565.html

[PATCHv2] Handle overlength strings in the C FE
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01566.html

[PATCHv2] Handle overlength strings in C++ FE
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01567.html

[PATCHv2] Handle overlength string literals in the fortan FE
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01568.html

The Ada and GO patches are no longer necessary.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.
2018-08-24  Bernd Edlinger  

	* varasm.c (compare_constant): Compare type size of STRING_CSTs.
	(get_constant_size): Don't make STRING_CSTs larger than they are.
	(check_string_literal): New check function for STRING_CSTs.
	(output_constant): Use it.

Index: gcc/varasm.c
===
--- gcc/varasm.c	(revision 263807)
+++ gcc/varasm.c	(working copy)
@@ -3146,7 +3146,9 @@ compare_constant (const tree t1, const tree t2)
   return FIXED_VALUES_IDENTICAL (TREE_FIXED_CST (t1), TREE_FIXED_CST (t2));
 
 case STRING_CST:
-  if (TYPE_MODE (TREE_TYPE (t1)) != TYPE_MODE (TREE_TYPE (t2)))
+  if (TYPE_MODE (TREE_TYPE (t1)) != TYPE_MODE (TREE_TYPE (t2))
+	  || int_size_in_bytes (TREE_TYPE (t1))
+	 != int_size_in_bytes (TREE_TYPE (t2)))
 	return 0;
 
   return (TREE_STRING_LENGTH (t1) == TREE_STRING_LENGTH (t2)
@@ -3303,8 +3305,9 @@ get_constant_size (tree exp)
   HOST_WIDE_INT size;
 
   size = int_size_in_bytes (TREE_TYPE (exp));
-  if (TREE_CODE (exp) == STRING_CST)
-size = MAX (TREE_STRING_LENGTH (exp), size);
+  gcc_checking_assert (size >= 0);
+  gcc_checking_assert (TREE_CODE (exp) != STRING_CST
+		   || size >= TREE_STRING_LENGTH (exp));
   return size;
 }
 
@@ -4774,6 +4777,30 @@ initializer_constant_valid_for_bitfield_p (tree va
   return false;
 }
 
+/* Check if a STRING_CST fits into the field.
+   Tolerate only the case when the NUL termination
+   does not fit into the field.   */
+
+static bool
+check_string_literal (tree string, unsigned HOST_WIDE_INT size)
+{
+  tree type = TREE_TYPE (string);
+  tree eltype = TREE_TYPE (type);
+  unsigned HOST_WIDE_INT elts = tree_to_uhwi (TYPE_SIZE_UNIT (eltype));
+  unsigned HOST_WIDE_INT mem_size = tree_to_uhwi (TYPE_SIZE_UNIT (type));
+  int len = TREE_STRING_LENGTH (string);
+
+  if (elts != 1 && elts != 2 && elts != 4)
+return false;
+  if (len < 0 || len % elts != 0)
+return false;
+  if (size < (unsigned)len)
+return false;
+  if (mem_size != size)
+return false;
+  return true;
+}
+
 /* output_constructor outer state of relevance in recursive calls, typically
for nested aggregate bitfields.  */
 
@@ -4942,6 +4969,7 @@ output_constant (tree exp, unsigned HOST_WIDE_INT
 	case STRING_CST:
 	  thissize
 	= MIN ((unsigned HOST_WIDE_INT)TREE_STRING_LENGTH (exp), size);
+	  gcc_checking_assert (check_string_literal (exp, size));
 	  assemble_string (TREE_STRING_POINTER (exp), thissize);
 	  break;
 	case VECTOR_CST:

[PATCHv2] Handle overlength string literals in the fortan FE

[Bernd Edlinger]

Hi!


This is an alternative approach to handle overlength strings in the Fortran FE.

The difference to the previous version is that overlength
STRING_CST never have a longer TREE_STRING_LENGTH than the TYPE_DOMAIN.
And those STRING_CSTs are thus no longer zero terminated.

And the requirement to have all sting constants internally zero-terminated
is dropped.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.
2018-08-01  Bernd Edlinger  

	* trans-array.c (gfc_conv_array_initializer): Remove excess precision
	from overlength string initializers.

Index: gcc/fortran/trans-array.c
===
--- gcc/fortran/trans-array.c	(revision 263807)
+++ gcc/fortran/trans-array.c	(working copy)
@@ -5964,6 +5964,26 @@ gfc_conv_array_initializer (tree type, gfc_expr *
 	{
 	case EXPR_CONSTANT:
 	  gfc_conv_constant (, c->expr);
+
+	  /* See gfortran.dg/charlen_15.f90 for instance.  */
+	  if (TREE_CODE (se.expr) == STRING_CST
+		  && TREE_CODE (type) == ARRAY_TYPE)
+		{
+		  tree atype = type;
+		  while (TREE_CODE (TREE_TYPE (atype)) == ARRAY_TYPE)
+		atype = TREE_TYPE (atype);
+		  if (TREE_CODE (TREE_TYPE (atype)) == INTEGER_TYPE
+		  && tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (se.expr)))
+			 > tree_to_uhwi (TYPE_SIZE_UNIT (atype)))
+		{
+		  unsigned HOST_WIDE_INT size
+			= tree_to_uhwi (TYPE_SIZE_UNIT (atype));
+		  const char *p = TREE_STRING_POINTER (se.expr);
+
+		  se.expr = build_string (size, p);
+		  TREE_TYPE (se.expr) = atype;
+		}
+		}
 	  break;
 
 	case EXPR_STRUCTURE:

[PATCHv2] Handle overlength strings in C++ FE

[Bernd Edlinger]

Hi!


This is an alternative approach to handle overlength strings in the C++ FE.

The difference to the previous version is that overlength
STRING_CST never have a longer TREE_STRING_LENGTH than the TYPE_DOMAIN.
And those STRING_CSTs are thus no longer zero terminated.

Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.
2018-08-01  Bernd Edlinger  

	* typeck2.c (digest_init_r): Fix overlength strings.
	* vtable-class-hierarchy.c (build_key_buffer_arg): Make string literal
	NUL terminated.

Index: gcc/cp/typeck2.c
===
--- gcc/cp/typeck2.c	(revision 263807)
+++ gcc/cp/typeck2.c	(working copy)
@@ -1116,8 +1116,13 @@ digest_init_r (tree type, tree init, int nested, i
 		 counted in the length of the constant, but in C++ this would
 		 be invalid.  */
 	  if (size < TREE_STRING_LENGTH (init))
-		permerror (loc, "initializer-string for array "
-			   "of chars is too long");
+		{
+		  permerror (loc, "initializer-string for array "
+			 "of chars is too long");
+
+		  init = build_string (size, TREE_STRING_POINTER (init));
+		  TREE_TYPE (init) = type;
+		}
 	}
 	  return init;
 	}
Index: gcc/cp/vtable-class-hierarchy.c
===
--- gcc/cp/vtable-class-hierarchy.c	(revision 263807)
+++ gcc/cp/vtable-class-hierarchy.c	(working copy)
@@ -738,7 +738,7 @@ build_key_buffer_arg (tree base_ptr_var_decl)
   tree ret_value;
 
   /* Set the len and hash for the string.  */
-  *value_ptr = len1;
+  *value_ptr = len1++;
   value_ptr++;
   *value_ptr = hash_value;
 

[PATCHv2] Handle overlength strings in the C FE

[Bernd Edlinger]

Hi!


This is an alternative approach handle overlength strings in the C FE.

The difference to the previous version is that overlength
STRING_CST never have a longer TREE_STRING_LENGTH than the TYPE_DOMAIN.
And those STRING_CSTs are thus no longer zero terminated.

Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.
2018-08-01  Bernd Edlinger  

	* c-typeck.c (digest_init): Shorten overlength strings.

Index: gcc/c/c-typeck.c
===
--- gcc/c/c-typeck.c	(revision 263807)
+++ gcc/c/c-typeck.c	(working copy)
@@ -7488,19 +7488,17 @@ digest_init (location_t init_loc, tree type, tree
 		}
 	}
 
-	  TREE_TYPE (inside_init) = type;
 	  if (TYPE_DOMAIN (type) != NULL_TREE
 	  && TYPE_SIZE (type) != NULL_TREE
 	  && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST)
 	{
 	  unsigned HOST_WIDE_INT len = TREE_STRING_LENGTH (inside_init);
+	  unsigned unit = TYPE_PRECISION (typ1) / BITS_PER_UNIT;
 
 	  /* Subtract the size of a single (possibly wide) character
 		 because it's ok to ignore the terminating null char
 		 that is counted in the length of the constant.  */
-	  if (compare_tree_int (TYPE_SIZE_UNIT (type),
-(len - (TYPE_PRECISION (typ1)
-	/ BITS_PER_UNIT))) < 0)
+	  if (compare_tree_int (TYPE_SIZE_UNIT (type), len - unit) < 0)
 		pedwarn_init (init_loc, 0,
 			  ("initializer-string for array of chars "
 			   "is too long"));
@@ -7509,8 +7507,17 @@ digest_init (location_t init_loc, tree type, tree
 		warning_at (init_loc, OPT_Wc___compat,
 			("initializer-string for array chars "
 			 "is too long for C++"));
+	  if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
+		{
+		  unsigned HOST_WIDE_INT size
+		= tree_to_uhwi (TYPE_SIZE_UNIT (type));
+		  const char *p = TREE_STRING_POINTER (inside_init);
+
+		  inside_init = build_string (size, p);
+		}
 	}
 
+	  TREE_TYPE (inside_init) = type;
 	  return inside_init;
 	}
   else if (INTEGRAL_TYPE_P (typ1))

[PATCHv2] Call braced_list_to_string after array size is fixed

[Bernd Edlinger]

Hi,

this updated patch fixes one regression with current trunk due
to a new test case.  Sorry for the confusion.

The change to the previous version is:
1) the check to avoid folding on empty char arrays is restored.
2) A null-termination character is added except when the string is full length.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.
c-family:
2018-08-24  Bernd Edlinger  

	* c-common.c (braced_list_to_string): Remove eval parameter.
	Add some more checks.  Always create zero-terminated STRING_CST.
	* c-common.h (braced_list_to_string): Adjust prototype.

c:
2018-08-24  Bernd Edlinger  

	* c-decl.c (finish_decl): Call braced_list_to_string here ...
	* c-parser.c (c_parser_declaration_or_fndef): ... instead of here.


cp:
2018-08-24  Bernd Edlinger  

	* decl.c (eval_check_narrowing): Remove.
	(check_initializer): Move call to braced_list_to_string from here ...
	* typeck2.c (store_init_value): ... to here.
	(digest_init_r): Remove handing of signed/unsigned char strings.

testsuite:
2018-08-24  Bernd Edlinger  

	* c-c++-common/array-init.c: New test.
	* g++.dg/init/string2.C: Remove xfail.

diff -Npur gcc/c/c-decl.c gcc/c/c-decl.c
--- gcc/c/c-decl.c	2018-08-21 08:17:41.0 +0200
+++ gcc/c/c-decl.c	2018-08-24 20:03:12.508230259 +0200
@@ -5030,6 +5030,17 @@ finish_decl (tree decl, location_t init_
   relayout_decl (decl);
 }
 
+  if (TREE_CODE (type) == ARRAY_TYPE
+  && DECL_INITIAL (decl)
+  && TREE_CODE (DECL_INITIAL (decl)) == CONSTRUCTOR)
+{
+  tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type));
+  if (typ1 == char_type_node
+	  || typ1 == signed_char_type_node
+	  || typ1 == unsigned_char_type_node)
+	DECL_INITIAL (decl) = braced_list_to_string (type, DECL_INITIAL (decl));
+}
+
   if (VAR_P (decl))
 {
   if (init && TREE_CODE (init) == CONSTRUCTOR)
diff -Npur gcc/c/c-parser.c gcc/c/c-parser.c
--- gcc/c/c-parser.c	2018-08-21 08:17:41.0 +0200
+++ gcc/c/c-parser.c	2018-08-24 20:03:12.511230218 +0200
@@ -2127,15 +2127,6 @@ c_parser_declaration_or_fndef (c_parser
 	  if (d != error_mark_node)
 		{
 		  maybe_warn_string_init (init_loc, TREE_TYPE (d), init);
-
-		  /* Try to convert a string CONSTRUCTOR into a STRING_CST.  */
-		  tree valtype = TREE_TYPE (init.value);
-		  if (TREE_CODE (init.value) == CONSTRUCTOR
-		  && TREE_CODE (valtype) == ARRAY_TYPE
-		  && TYPE_STRING_FLAG (TREE_TYPE (valtype)))
-		if (tree str = braced_list_to_string (valtype, init.value))
-		  init.value = str;
-
 		  finish_decl (d, init_loc, init.value,
 			   init.original_type, asm_name);
 		}
diff -Npur gcc/c-family/c-common.c gcc/c-family/c-common.c
--- gcc/c-family/c-common.c	2018-08-17 05:02:11.0 +0200
+++ gcc/c-family/c-common.c	2018-08-24 20:03:12.512230205 +0200
@@ -8511,39 +8511,28 @@ maybe_add_include_fixit (rich_location *
 }
 
 /* Attempt to convert a braced array initializer list CTOR for array
-   TYPE into a STRING_CST for convenience and efficiency.  When non-null,
-   use EVAL to attempt to evalue constants (used by C++).  Return
-   the converted string on success or null on failure.  */
+   TYPE into a STRING_CST for convenience and efficiency.  Return
+   the converted string on success or the original ctor on failure.  */
 
 tree
-braced_list_to_string (tree type, tree ctor, tree (*eval)(tree, tree))
+braced_list_to_string (tree type, tree ctor)
 {
-  unsigned HOST_WIDE_INT nelts = CONSTRUCTOR_NELTS (ctor);
+  if (!tree_fits_uhwi_p (TYPE_SIZE_UNIT (type)))
+return ctor;
 
   /* If the array has an explicit bound, use it to constrain the size
  of the string.  If it doesn't, be sure to create a string that's
  as long as implied by the index of the last zero specified via
  a designator, as in:
const char a[] = { [7] = 0 };  */
-  unsigned HOST_WIDE_INT maxelts = HOST_WIDE_INT_M1U;
-  if (tree size = TYPE_SIZE_UNIT (type))
-{
-  if (tree_fits_uhwi_p (size))
-	{
-	  maxelts = tree_to_uhwi (size);
-	  maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
+  unsigned HOST_WIDE_INT maxelts = tree_to_uhwi (TYPE_SIZE_UNIT (type));
+  maxelts /= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (type)));
 
-	  /* Avoid converting initializers for zero-length arrays.  */
-	  if (!maxelts)
-	return NULL_TREE;
-	}
-}
-  else if (!nelts)
-/* Avoid handling the undefined/erroneous case of an empty
-   initializer for an arrays with unspecified bound.  */
-return NULL_TREE;
+  /* Avoid converting initializers for zero-length arrays.  */
+  if (!maxelts)
+return ctor;
 
-  tree eltype = TREE_TYPE (type);
+  unsigned HOST_WIDE_INT nelts = CONSTRUCTOR_NELTS (ctor);
 
   auto_vec str;
   str.reserve (nelts + 1);
@@ -8553,19 +8542,21 @@ braced_list_to_string (tree type, tree c
 
   FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (ctor), i, index, value)
 {
-  unsigned HOST_WIDE_INT idx = index ? tree_to_uhwi (index) : i;
+  unsigned 

Re: C++ PATCH for c++/87029, Implement -Wredundant-move

[Marek Polacek]

On Fri, Aug 24, 2018 at 11:32:18PM +1000, Jason Merrill wrote:
> On Fri, Aug 24, 2018 at 12:53 AM, Marek Polacek  wrote:
> > On Thu, Aug 23, 2018 at 10:44:30AM -0400, Marek Polacek wrote:
> >> +T
> >> +fn3 (const T t)
> >> +{
> >> +  // t is const: will decay into move.
> >> +  return t;
> >> +}
> >> +
> >> +T
> >> +fn4 (const T t)
> >> +{
> >> +  // t is const: will decay into move despite std::move, so it's 
> >> redundant.
> >> +  return std::move (t); // { dg-warning "redundant move in return 
> >> statement" }
> >> +}
> >
> > This should read "decay into copy".  We can't move from a const object.  
> > Consider
> > it fixed.
> 
> Well, we'll do the overload resolution as though t were an rvalue,
> even though it's const; it's just unlikely to succeed, since a
> constructor taking a const rvalue reference doesn't make much sense.

Ah, true.   I guess that's only used in like std::reference_wrapper.

> It occurs to me that the std::move might not be redundant in some
> cases: for the implicit treatment as an rvalue, the return must select
> a constructor that takes an rvalue reference to the returned object's
> type.  With an explict std::move, that restriction doesn't apply.  So,
> for
> 
> struct C { };
> struct A {
>   operator C() &;
>   operator C() &&;
> };
> 
> C f(A a)
> {
>   return a; // calls operator C()&
>   return std::move(a); // calls operator C()&&
> }
> 
> ...though I see we currently get the first return wrong, and call the
> rvalue overload for both.  I think there was a recent core issue in
> this area, I'll try to find that later.
 
You're right.  Wow, I did not think of this.  I went looking for that DR but
I'm not finding it, please do let me know if you find it.  Does that mean that
treat_lvalue_as_rvalue_p will have to change?  Do you want me to open a PR?

> Anyway, I imagine this sort of example is rare enough that the warning
> is still worth having; people doing this can use static_cast instead
> or just turn off the warning.
> 
> BTW, Instead of using location_of (retval) in each diagnostic call,
> let's put at the top of the function
> 
>   location_t loc = cp_expr_loc_or_loc (retval, input_location);
> 
> and use 'loc' in the diagnostics.  This is the pattern I generally mean to 
> use.

Done.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-08-24  Marek Polacek  

PR c++/87029, Implement -Wredundant-move.
* c.opt (Wredundant-move): New option.

* typeck.c (treat_lvalue_as_rvalue_p): New function.
(maybe_warn_pessimizing_move): Call convert_from_reference.
Warn about redundant moves.

* doc/invoke.texi: Document -Wredundant-move.

* g++.dg/cpp0x/Wredundant-move1.C: New test.
* g++.dg/cpp0x/Wredundant-move2.C: New test.
* g++.dg/cpp0x/Wredundant-move3.C: New test.
* g++.dg/cpp0x/Wredundant-move4.C: New test.

diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 76840dd77ad..31a2b972919 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -985,6 +985,10 @@ Wredundant-decls
 C ObjC C++ ObjC++ Var(warn_redundant_decls) Warning
 Warn about multiple declarations of the same object.
 
+Wredundant-move
+C++ ObjC++ Var(warn_redundant_move) Warning LangEnabledBy(C++ ObjC++,Wextra)
+Warn about redundant calls to std::move.
+
 Wregister
 C++ ObjC++ Var(warn_register) Warning
 Warn about uses of register storage specifier.
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 122d9dcd4b3..cd1ee224883 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -9178,6 +9178,19 @@ can_do_nrvo_p (tree retval, tree functype)
  && !TYPE_VOLATILE (TREE_TYPE (retval)));
 }
 
+/* Returns true if we should treat RETVAL, an expression being returned,
+   as if it were designated by an rvalue.  See [class.copy.elision].  */
+
+static bool
+treat_lvalue_as_rvalue_p (tree retval)
+{
+  return ((cxx_dialect != cxx98)
+ && ((VAR_P (retval) && !DECL_HAS_VALUE_EXPR_P (retval))
+ || TREE_CODE (retval) == PARM_DECL)
+ && DECL_CONTEXT (retval) == current_function_decl
+ && !TREE_STATIC (retval));
+}
+
 /* Warn about wrong usage of std::move in a return statement.  RETVAL
is the expression we are returning; FUNCTYPE is the type the function
is declared to return.  */
@@ -9185,7 +9198,9 @@ can_do_nrvo_p (tree retval, tree functype)
 static void
 maybe_warn_pessimizing_move (tree retval, tree functype)
 {
-  if (!warn_pessimizing_move)
+  location_t loc = cp_expr_loc_or_loc (retval, input_location);
+
+  if (!(warn_pessimizing_move || warn_redundant_move))
 return;
 
   /* C++98 doesn't know move.  */
@@ -9207,14 +9222,24 @@ maybe_warn_pessimizing_move (tree retval, tree functype)
  STRIP_NOPS (arg);
  if (TREE_CODE (arg) == ADDR_EXPR)
arg = TREE_OPERAND (arg, 0);
+ arg = convert_from_reference (arg);
  /* Warn if we could do copy elision were it not for the move.  */
  if (can_do_nrvo_p (arg, functype))
{
 

[PATCH] convert MIN_EXPR operands to the same type (PR 87059)

[Martin Sebor]

PR 87059 points out an ICE in the recently enhanced VRP code
that was traced back to a MIN_EXPR built out of operands of
types with different sign by expand_builtin_strncmp().

The attached patch adjusts the function to make sure both
operands have the same type, and to make these mismatches
easier to detect, also adds an assertion to fold_binary_loc()
for these expressions.

Bootstrapped on x86_64-linux.

Martin

PS Aldy, I have not tested this on powerpc64le.
PR tree-optimization/87059 - internal compiler error: in set_value_range

gcc/ChangeLog:

	PR tree-optimization/87059
	* builtins.c (expand_builtin_strncmp): Convert MIN_EXPR operand
	to the same type as the other.
	* fold-const.c (fold_binary_loc): Assert expectation.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index b1a79f3..6a992bd 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -4759,7 +4759,10 @@ expand_builtin_strncmp (tree exp, ATTRIBUTE_UNUSED rtx target,
   /* If we are not using the given length, we must incorporate it here.
  The actual new length parameter will be MIN(len,arg3) in this case.  */
   if (len != len3)
-len = fold_build2_loc (loc, MIN_EXPR, TREE_TYPE (len), len, len3);
+{
+  len = fold_convert_loc (loc, sizetype, len);
+  len = fold_build2_loc (loc, MIN_EXPR, TREE_TYPE (len), len, len3);
+}
   rtx arg1_rtx = get_memory_rtx (arg1, len);
   rtx arg2_rtx = get_memory_rtx (arg2, len);
   rtx arg3_rtx = expand_normal (len);
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index b318fc77..1e44a24 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -9326,6 +9326,14 @@ fold_binary_loc (location_t loc, enum tree_code code, tree type,
 
   if (kind == tcc_comparison || code == MIN_EXPR || code == MAX_EXPR)
 {
+  if (code == MIN_EXPR || code == MAX_EXPR)
+	{
+	  tree typ0 = TREE_TYPE (arg0);
+	  tree typ1 = TREE_TYPE (arg1);
+	  gcc_assert (TYPE_SIGN (typ0) == TYPE_SIGN (typ1)
+		  && TYPE_MODE (typ0) == TYPE_MODE (typ1));
+	}
+
   STRIP_SIGN_NOPS (arg0);
   STRIP_SIGN_NOPS (arg1);
 }

Re: [PATCH]: Remove remaining traces of MPX bounded pointers

[Jakub Jelinek]

On Fri, Aug 24, 2018 at 03:56:27PM +0200, Uros Bizjak wrote:
> 2018-08-23  Uros Bizjak  
> 
> * emit-rtl.c (init_emit_once): Do not emit MODE_POINTER_BOUNDS RTXes.
> * emit-rtl.h (rtl_data): Remove return_bnd.
> * explow.c (trunc_int_for_mode): Do not handle POINTER_BOUNDS_MODE_P.
> * function.c (diddle_return_value): Do not handle crtl->return_bnd.
> * genmodes.c (complete_mode): Do not handle MODE_POINTER_BOUNDS.
> (POINTER_BOUNDS_MODE): Remove definition.
> (make_pointer_bounds_mode): Remove.
> (get_mode_class): Do not handle MODE_POINTER_BOUNDS.
> * machmode.h (POINTER_BOUNDS_MODE_P): Remove definition.
> (scalare_mode::includes_p): Do not handle MODE_POINTER_BOUNDS.
> * mode-classes.def: Do not define MODE_POINTER_BOUNDS.
> * stor-layout.c (int_mode_for_mode): Do not handle MODE_POINTER_BOUNDS.
> * tree-core.h (enum tree_index): Remove TI_POINTER_BOUNDS_TYPE.
> * varasm.c (output_constant_pool_2): Do not handle MODE_POINTER_BOUNDS.
> 
> * config/i386/i386-modes.def (BND32, BND64): Remove.
> * config/i386/i386.c (dbx_register_map): Remove bound registers.
> (dbx64_register_map): Ditto.
> (svr4_dbx_register_map): Ditto.
> (indirect_thunk_bnd_needed): Remove.
> (indirect_thunks_bnd_used): Ditto.
> (indirect_return_bnd_needed): Ditto.
> (indirect_return_via_cx_bnd): Ditto.
> (enum indirect_thunk_prefix): Remove indirect_thunk_prefix_bnd.
> (indirect_thunk_name): Remove handling of indirect_thunk_prefix_bnd.
> (output_indirect_thunk): Ditto.  Remove need_prefix argument.
> (output_indirect_thunk_function): Remove handling of
> indirect_return_bnd_needed, indirect_return_via_cx_bnd,
> indirect_thunk_bnd_needed and indirect_thunks_bnd_used variables.
> (ix86_save_reg): Remove handling of crtl->return_bnd.
> (ix86_legitimate_constant_p): Remove handling of POINTER_BOUNDS_MODE_P.
> (ix86_print_operand_address_as): Remove handling of UNSPEC_BNDMK_ADDR
> and UNSPEC_BNDLX_ADDR.
> (ix86_output_indirect_branch_via_reg): Remove handling of
> indirect_thunk_prefix_bnd.
> (ix86_output_indirect_branch_via_push): Ditto.
> (ix86_output_function_return): Ditto.
> (ix86_output_indirect_function_return): Ditto.
> (avoid_func_arg_motion): Do not handle UNSPEC_BNDSTX.
> * config/i386/i386.h (FIXED_REGISTERS): Remove bound registers.
> (CALL_USED_REGISTERS): Ditto.
> (REG_ALLOC_ORDER): Update for removal of bound registers.
> (HI_REGISTER_NAMES): Ditto.
> * config/i386/i386.md (UNSPEC_BNDMK, UNSPEC_BNDMK_ADDR, UNSPEC_BNDSTX)
> (UNSPEC_BNDLDX, UNSPEC_BNDLDX_ADDR, UNSPEC_BNDCL, UNSPEC_BNDCU)
> (UNSPEC_BNDCN, UNSPEC_MPX_FENCE): Remove.
> (BND0_REG, BND1_REG, BND2_REG, BND3_REG): Remove
> (FIRST_PSEUDO_REG): Update.
> (BND): Remove mode iterator.
> * config/i386/predicates.md (bnd_mem_operator): Remove.
> 
> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
> 
> OK for mainline?

OPk, thanks.

Jakub

Re: VRP: make range_includes_zero_p handle value_ranges

[Aldy Hernandez]

On 08/24/2018 05:08 AM, Richard Biener wrote:


@@ -1407,7 +1407,10 @@ extract_range_from_binary_expr_1 (value_range *vr,
&& code != POINTER_PLUS_EXPR
&& (vr0.type == VR_VARYING
   || vr1.type == VR_VARYING
- || vr0.type != vr1.type
+ || (vr0.type != vr1.type
+ /* We can handle POINTER_PLUS_EXPR(~[0,0], [x,y]) below,
+even though we have differing range kinds.  */
+ && code != POINTER_PLUS_EXPR)
   || symbolic_range_p ()
   || symbolic_range_p ()))
  {

is redundant now (spot the code != POINTER_PLUS_EXPR check at the
beginning of context)


Hey, no fair making my code irrelevant mid way through a review! :)



OK with this hunk removed.


Done.

Thanks.
Aldy

Go patch committed: Remove the dummy arg of getcallersp

[Ian Lance Taylor]

This patch changes the Go compiler and libgo runtime package to remove
the dummy argument of runtime.getcallersp.  This is a port of
https://golang.org/cl/109596 to the gofrontend, in preparation for
updating libgo to 1.11.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 263749)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-274c88df4d6f9360dcd657b6e069a3b5a1d37a90
+8deaafd14414bb5cbbdf3e2673f61b6d836d7d2a
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 263749)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -9635,13 +9635,9 @@ Call_expression::do_lower(Gogo* gogo, Na
"__builtin_return_address",
0);
}
- else if (this->args_ != NULL
-  && this->args_->size() == 1
+ else if ((this->args_ == NULL || this->args_->size() == 0)
   && n == "getcallersp")
{
- // The actual argument to getcallersp is always the
- // address of a parameter; we don't need that for the
- // GCC builtin function, so we just ignore it.
  static Named_object* builtin_frame_address;
  return this->lower_to_builtin(_frame_address,
"__builtin_frame_address",
Index: libgo/go/runtime/cgo_gccgo.go
===
--- libgo/go/runtime/cgo_gccgo.go   (revision 263749)
+++ libgo/go/runtime/cgo_gccgo.go   (working copy)
@@ -47,7 +47,7 @@ func Cgocall() {
mp := getg().m
mp.ncgocall++
mp.ncgo++
-   entersyscall(0)
+   entersyscall()
mp.incgo = true
 }
 
@@ -63,7 +63,7 @@ func CgocallDone() {
// If we are invoked because the C function called _cgo_panic,
// then _cgo_panic will already have exited syscall mode.
if readgstatus(gp)&^_Gscan == _Gsyscall {
-   exitsyscall(0)
+   exitsyscall()
}
 }
 
@@ -84,7 +84,7 @@ func CgocallBack() {
 
lockOSThread()
 
-   exitsyscall(0)
+   exitsyscall()
gp.m.incgo = false
 
if gp.m.ncgo == 0 {
@@ -134,7 +134,7 @@ func CgocallBackDone() {
}
 
gp.m.incgo = true
-   entersyscall(0)
+   entersyscall()
 
if drop {
mp.dropextram = false
@@ -144,7 +144,7 @@ func CgocallBackDone() {
 
 // _cgo_panic may be called by SWIG code to panic.
 func _cgo_panic(p *byte) {
-   exitsyscall(0)
+   exitsyscall()
panic(gostringnocopy(p))
 }
 
Index: libgo/go/runtime/lock_futex.go
===
--- libgo/go/runtime/lock_futex.go  (revision 263749)
+++ libgo/go/runtime/lock_futex.go  (working copy)
@@ -236,8 +236,8 @@ func notetsleepg(n *note, ns int64) bool
throw("notetsleepg on g0")
}
 
-   entersyscallblock(0)
+   entersyscallblock()
ok := notetsleep_internal(n, ns)
-   exitsyscall(0)
+   exitsyscall()
return ok
 }
Index: libgo/go/runtime/lock_sema.go
===
--- libgo/go/runtime/lock_sema.go   (revision 263749)
+++ libgo/go/runtime/lock_sema.go   (working copy)
@@ -289,8 +289,8 @@ func notetsleepg(n *note, ns int64) bool
throw("notetsleepg on g0")
}
semacreate(gp.m)
-   entersyscallblock(0)
+   entersyscallblock()
ok := notetsleep_internal(n, ns, nil, 0)
-   exitsyscall(0)
+   exitsyscall()
return ok
 }
Index: libgo/go/runtime/malloc.go
===
--- libgo/go/runtime/malloc.go  (revision 263749)
+++ libgo/go/runtime/malloc.go  (working copy)
@@ -621,7 +621,7 @@ func mallocgc(size uintptr, typ *_type,
// callback.
incallback := false
if gomcache() == nil && getg().m.ncgo > 0 {
-   exitsyscall(0)
+   exitsyscall()
incallback = true
}
 
@@ -709,7 +709,7 @@ func mallocgc(size uintptr, typ *_type,
mp.mallocing = 0
releasem(mp)
if incallback {
-   entersyscall(0)
+   entersyscall()
}
return x
}
@@ -835,7 +835,7 @@ func mallocgc(size uintptr, typ *_type,
}
 
if 

[PATCH] Forward declare debug containers so std::pmr aliases work

[Jonathan Wakely]

Prior to this change, including a  header when _GLIBCXX_DEBUG
is also defined would fail to compile in C++17 or later. The 
header would include the standard  header which defined
std::pmr::xxx as an alias for std::xxx. But in Debug Mode std::xxx
refers to std::__debug::xxx which has not been defined yet (because it
is in  after the inclusion of ).

This adds declarations of the debug containers before including the
non-Debug Mode  header, so that the std::pmr::xxx aliases work.

* include/debug/deque (std::__debug::deque): Declare.
* include/debug/forward_list (std::__debug::forward_list): Declare.
* include/debug/list (std::__debug::list): Declare.
* include/debug/map (std::__debug::map): Declare.
* include/debug/set (std::__debug::set): Declare.
* include/debug/unordered_map (std::__debug::unordered_map): Declare.
* include/debug/unordered_set (std::__debug::unordered_set): Declare.
* include/debug/vector (std::__debug::vector): Declare.
* testsuite/23_containers/deque/types/pmr_typedefs_debug.cc: New test.
* testsuite/23_containers/forward_list/pmr_typedefs_debug.cc: New
test.
* testsuite/23_containers/list/pmr_typedefs_debug.cc: New test.
* testsuite/23_containers/map/pmr_typedefs_debug.cc: New test.
* testsuite/23_containers/multimap/pmr_typedefs_debug.cc: New test.
* testsuite/23_containers/multiset/pmr_typedefs_debug.cc: New test.
* testsuite/23_containers/set/pmr_typedefs_debug.cc: New test.
* testsuite/23_containers/unordered_map/pmr_typedefs_debug.cc: New
test.
* testsuite/23_containers/unordered_multimap/pmr_typedefs_debug.cc:
New test.
* testsuite/23_containers/unordered_multiset/pmr_typedefs_debug.cc:
New test.
* testsuite/23_containers/unordered_set/pmr_typedefs_debug.cc: New
test.
* testsuite/23_containers/vector/cons/destructible_debug_neg.cc:
Adjust dg-error lineno.
* testsuite/23_containers/vector/types/pmr_typedefs_debug.cc: New
test.

Tested x86_64-linux, committed to trunk.


commit 9ad665dca4632d82a6d699886f728b3aa395f481
Author: Jonathan Wakely 
Date:   Fri Aug 24 17:47:32 2018 +0100

Forward declare debug containers so std::pmr aliases work

Prior to this change, including a  header when _GLIBCXX_DEBUG
is also defined would fail to compile in C++17 or later. The 
header would include the standard  header which defined
std::pmr::xxx as an alias for std::xxx. But in Debug Mode std::xxx
refers to std::__debug::xxx which has not been defined yet (because it
is in  after the inclusion of ).

This adds declarations of the debug containers before including the
non-Debug Mode  header, so that the std::pmr::xxx aliases work.

* include/debug/deque (std::__debug::deque): Declare.
* include/debug/forward_list (std::__debug::forward_list): Declare.
* include/debug/list (std::__debug::list): Declare.
* include/debug/map (std::__debug::map): Declare.
* include/debug/set (std::__debug::set): Declare.
* include/debug/unordered_map (std::__debug::unordered_map): 
Declare.
* include/debug/unordered_set (std::__debug::unordered_set): 
Declare.
* include/debug/vector (std::__debug::vector): Declare.
* testsuite/23_containers/deque/types/pmr_typedefs_debug.cc: New 
test.
* testsuite/23_containers/forward_list/pmr_typedefs_debug.cc: New
test.
* testsuite/23_containers/list/pmr_typedefs_debug.cc: New test.
* testsuite/23_containers/map/pmr_typedefs_debug.cc: New test.
* testsuite/23_containers/multimap/pmr_typedefs_debug.cc: New test.
* testsuite/23_containers/multiset/pmr_typedefs_debug.cc: New test.
* testsuite/23_containers/set/pmr_typedefs_debug.cc: New test.
* testsuite/23_containers/unordered_map/pmr_typedefs_debug.cc: New
test.
* testsuite/23_containers/unordered_multimap/pmr_typedefs_debug.cc:
New test.
* testsuite/23_containers/unordered_multiset/pmr_typedefs_debug.cc:
New test.
* testsuite/23_containers/unordered_set/pmr_typedefs_debug.cc: New
test.
* testsuite/23_containers/vector/cons/destructible_debug_neg.cc:
Adjust dg-error lineno.
* testsuite/23_containers/vector/types/pmr_typedefs_debug.cc: New
test.

diff --git a/libstdc++-v3/include/debug/deque b/libstdc++-v3/include/debug/deque
index a6047dbed1b..ad86b5c8f38 100644
--- a/libstdc++-v3/include/debug/deque
+++ b/libstdc++-v3/include/debug/deque
@@ -31,6 +31,11 @@
 
 #pragma GCC system_header
 
+#include 
+namespace std _GLIBCXX_VISIBILITY(default) { namespace __debug {
+  template class deque;
+} } // namespace std::__debug
+
 #include 
 

Re: lightweight class to wide int ranges in VRP and friends

[Aldy Hernandez]

On 08/24/2018 06:32 AM, Richard Biener wrote:

On Wed, Aug 15, 2018 at 7:31 PM Aldy Hernandez  wrote:


I kept seeing the same patterns over and over in all this re-factoring:

1. extract value_range constants into pairs of wide ints

2. mimic symbolics as [-MIN, +MAX] (most of the time :))

3. perform some wide int operation on the wide int range

4. convert back to a value range

So I've decided to clean this up even more.  Instead of passing a pair
of wide ints plus sign, precision, and god knows what to every
wide_int_range_* function, I've implemented a lighweight class
(wi_range) for a pair of wide ints.  It is not meant for long term
storage, but even so its footprint is minimal.

The only extra bits are another 64-bit word per pair for keeping
attributes such as precision, sign, overflow_wraps, and a few other
things we'll need shortly.  In reality we only need one set of
attributes for both sets of pairs, so we waste one 64-bit word.  I don't
care.  They're short lived, and the resulting code looks an awful lot
nicer.  For example, the shift code gets rewritten from this:

if (range_int_cst_p ()
   && !vrp_shift_undefined_p (vr1, prec))
 {
   if (code == RSHIFT_EXPR)
 {
   if (vr0.type != VR_RANGE || symbolic_range_p ())
 {
   vr0.type = type = VR_RANGE;
   vr0.min = vrp_val_min (expr_type);
   vr0.max = vrp_val_max (expr_type);
 }
   extract_range_from_multiplicative_op (vr, code, , );
   return;
 }
   else if (code == LSHIFT_EXPR
&& range_int_cst_p ())
 {
   wide_int res_lb, res_ub;
   if (wide_int_range_lshift (res_lb, res_ub, sign, prec,
  wi::to_wide (vr0.min),
  wi::to_wide (vr0.max),
  wi::to_wide (vr1.min),
  wi::to_wide (vr1.max),
  TYPE_OVERFLOW_UNDEFINED (expr_type),
  TYPE_OVERFLOW_WRAPS (expr_type)))
 {
   min = wide_int_to_tree (expr_type, res_lb);
   max = wide_int_to_tree (expr_type, res_ub);
   set_and_canonicalize_value_range (vr, VR_RANGE,
 min, max, NULL);
   return;
 }
 }
 }
set_value_range_to_varying (vr);

into this:

wi_range w0 (, expr_type);
wi_range w1 (, expr_type);
if (!range_int_cst_p ()
   || wi_range_shift_undefined_p (w1)
   || (code == LSHIFT_EXPR
   && !range_int_cst_p ()))
 {
   vr->set_varying ();
   return;
 }
bool success;
if (code == RSHIFT_EXPR)
 success = wi_range_multiplicative_op (res, code, w0, w1);
else
 success = wi_range_lshift (res, w0, w1);

if (success)
 vr->set_and_canonicalize (res, expr_type);
else
 vr->set_varying ();
return;


not sure whether I like the munging of lshift and right shift (what exactly gets
done is less clear in your version ...).  Having a light-weigth class for
the wi_range_ parameters is nice though.


No worries.  I'm not emotionally attached to munging them :).




I also munged together the maybe/mustbe nonzero_bits into one structure.

Finally, I've pontificated that wide_int_range_blah is too much typing.
Everyone's RSI will thank me for rewriting the methods as wi_range_blah.

I've tried to keep the functionality changes to a minimum.  However,
there are some slight changes where I treat symbolics and VR_VARYING as
[MIN, MAX] and let the constant code do its thing.  It is considerably
easier to read.

I am finally happy with the overall direction of this.  If this and the
last one are acceptable, I think I only need to clean MIN/MAX_EXPR and
ABS_EXPR and I'm basically done with what I need going forward.

Phew...

How does this look?


+struct wi_range
+{
+  wide_int min;
+  wide_int max;
+  /* This structure takes one additional 64-bit word apart from the
+ min/max bounds above.  It is laid out so that PRECISION and SIGN
+ can be accessed without any bit twiddling, since they're the most
+ commonly accessed fields.  */
+  unsigned short precision;
+  bool empty_p:1;
+  bool pointer_type_p:1;
+  bool overflow_wraps:1;
+  bool overflow_undefined:1;
+  signop sign;

isn't precision already available in min.get_precision () which is
required to be equal to max.get_precision ()?


Indeed.  Good point.



+  wi_range () { empty_p = false; }

true I suppose?  Better not allow default construction?


OK.



empty_p doesn't seem to be documented, nor is pointer_type_p
or why that is necessary - maybe simply 

Re: PING [PATCH] warn for strlen of arrays with missing nul (PR 86552, 86711, 86714) )

[Bernd Edlinger]

On 08/24/18 18:51, Jeff Law wrote:
> On 08/02/2018 07:26 AM, Bernd Edlinger wrote:
>>
>>> -  if (compare_tree_int (array_size, length + 1) < 0)
>>> +  if (nulterm)
>> but here you compare bytes with length which is measued un chars.
>>
>>> +*nulterm = array_elts > length;
>>> +  else if (array_elts <= length)
>>>   return NULL_TREE;
>>>   
>>> *ptr_offset = offset;
> Actually in the V2 patch length is measured by element size.  So I think
> this is a non-issue.
> 
> 
> 
>>> -  /* Compute and store the length of the substring at OFFSET.
>>> +  /* Compute and store the size of the substring at OFFSET.
>>>  All offsets past the initial length refer to null strings.  */
>>> -  if (offset <= string_length)
>>> -   *strlen = string_length - offset;
>> this should be offset < string_length.
>>
>>> +  if (offset <= string_size)
>>> +   *strsize = string_size - offset;
>>> else
>>> -   *strlen = 0;
>> this should be 1, you may access the NUL byte of "".
>>
>>> +   *strsize = 0;
>>>   }
> Agreed in both cases based on the defined behavior for the function (NUL
> is included).
> 
> 
>>>   
>>> -  const char *string = TREE_STRING_POINTER (src);
>>> -
>>> -  if (string_length == 0
>>> -  || offset >= string_size)
>>> +  if (string_size == 0
>>> +  || offset >= array_size)
>>>   return NULL;
>>>   
>>> -  if (strsize)
>>> -{
>>> -  /* Support even constant character arrays that aren't proper
>>> -NUL-terminated strings.  */
>>> -  *strsize = string_size;
>>> -}
>>> -  else if (string[string_length - 1] != '\0')
>> Well, this is broken for wide character strings.
>> but I hope we can get rid of STRING_CST which are
>> not explicitly null terminated.

I am afraid that is not going to happen.
Maybe we can get STRING_CST that are never longer
than the TYPE_UNIT_SIZE, but c_strlen and c_getstr
need to take care that the string is zero-terminated.

string_constant, should not promise the string is zero terminated.
But instead it can promise that:
1) the STRING_CST is valid up to TREE_STRING_LENGTH
2) mem_size is >= TREE_STRING_LENGTH
3) memory between TREE_STRING_LENGTH and mem_size is ZERO.

It will not guarantee anything about zero termination any more.

But that will again need an adjustment to the string_constant
and likely to my 86711/86714/87053 patch set.

Sorry for all this back and forth.

> This hunk is gone in the V2 patch.  So I'm not going to worry about it
> right now.
> 
> 

Hmm.  In my tree I wanted to drop this check first, but that
did not work right. So I skip the check if strsize is not NULL.
But if it is NULL, I check the element size is 1, if it is
not 1 return NULL, because it is no character string.

I had several iterations here, and it might still be dependent
on semantical properties of STRING_CST that may not be be granted
in the light of my recent discoveries.


In the end, the best approach might be to either merge my patch
with Martins, or step-wise, first fixing wrong code, and then
implementing warnings without fixing wrong code.

Anyway, the smaller the patch the better for the review.


Bernd.

Re: [PR 87073] [committed] fix go bootstrap

[Aldy Hernandez]

On 08/24/2018 12:56 PM, H.J. Lu wrote:

n Fri, Aug 24, 2018 at 1:02 AM Aldy Hernandez  wrote:


I have no idea how this passed bootstrap and tests in other languages.

The problem here is that wide_int_binop is overflowing on TRUNC_DIV_EXPR
and a range in Go.  This is causing us to use wmin/wmax uninitialized.

Serves me right for all my whining about Ada yesterday.  I think the VRP
changes are sufficiently all encompassing that I will start testing
--enable-languages=all instead of whatever our defaults are.  My bad.

Committed as obvious.


This patch also fixed:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87092

I am checking in this patch to add a testcase.



Thank you.

Re: [PR 87073] [committed] fix go bootstrap

[H.J. Lu]

n Fri, Aug 24, 2018 at 1:02 AM Aldy Hernandez  wrote:
>
> I have no idea how this passed bootstrap and tests in other languages.
>
> The problem here is that wide_int_binop is overflowing on TRUNC_DIV_EXPR
> and a range in Go.  This is causing us to use wmin/wmax uninitialized.
>
> Serves me right for all my whining about Ada yesterday.  I think the VRP
> changes are sufficiently all encompassing that I will start testing
> --enable-languages=all instead of whatever our defaults are.  My bad.
>
> Committed as obvious.

This patch also fixed:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87092

I am checking in this patch to add a testcase.

-- 
H.J.
From 89dabc2d6e95ebac507cdfe082085af6b3ec4701 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Fri, 24 Aug 2018 09:50:56 -0700
Subject: [PATCH] Add a testcase for PR middle-end/87092

	PR middle-end/87092
	* gcc.dg/pr87092.c: New.
---
 gcc/testsuite/gcc.dg/pr87092.c | 10 ++
 1 file changed, 10 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/pr87092.c

diff --git a/gcc/testsuite/gcc.dg/pr87092.c b/gcc/testsuite/gcc.dg/pr87092.c
new file mode 100644
index 000..4a6faebbd93
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr87092.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fwrapv" } */
+
+int a, b;
+
+void
+c(void) {
+  if (b)
+b = a / b;
+}
-- 
2.17.1

Re: PING [PATCH] warn for strlen of arrays with missing nul (PR 86552, 86711, 86714) )

[Jeff Law]

On 08/02/2018 07:26 AM, Bernd Edlinger wrote:
> 
>> -  if (compare_tree_int (array_size, length + 1) < 0)
>> +  if (nulterm)
> but here you compare bytes with length which is measued un chars.
> 
>> +*nulterm = array_elts > length;
>> +  else if (array_elts <= length)
>>  return NULL_TREE;
>>  
>>*ptr_offset = offset;
Actually in the V2 patch length is measured by element size.  So I think
this is a non-issue.



>> -  /* Compute and store the length of the substring at OFFSET.
>> +  /* Compute and store the size of the substring at OFFSET.
>>   All offsets past the initial length refer to null strings.  */
>> -  if (offset <= string_length)
>> -*strlen = string_length - offset;
> this should be offset < string_length.
> 
>> +  if (offset <= string_size)
>> +*strsize = string_size - offset;
>>else
>> -*strlen = 0;
> this should be 1, you may access the NUL byte of "".
> 
>> +*strsize = 0;
>>  }
Agreed in both cases based on the defined behavior for the function (NUL
is included).


>>  
>> -  const char *string = TREE_STRING_POINTER (src);
>> -
>> -  if (string_length == 0
>> -  || offset >= string_size)
>> +  if (string_size == 0
>> +  || offset >= array_size)
>>  return NULL;
>>  
>> -  if (strsize)
>> -{
>> -  /* Support even constant character arrays that aren't proper
>> - NUL-terminated strings.  */
>> -  *strsize = string_size;
>> -}
>> -  else if (string[string_length - 1] != '\0')
> Well, this is broken for wide character strings.
> but I hope we can get rid of STRING_CST which are
> not explicitly null terminated.
This hunk is gone in the V2 patch.  So I'm not going to worry about it
right now.



> 
>> +  if (!nulterm && string[string_size - 1] != '\0')
>>  {
>> -  /* Support only properly NUL-terminated strings but handle
>> - consecutive strings within the same array, such as the six
>> - substrings in "1\0002\0003".  */
>> +  /* When NULTERM is null, support only properly nul-terminated
>> + strings but handle consecutive strings within the same array,
>> + such as the six substrings in "1\0002\0003".  Otherwise, let
>> + the caller deal with non-nul-terminated arrays.  */
>>return NULL;
>>  }
>>  
>> -  return offset <= string_length ? string + offset : "";
> this should be offset < string_size.
> 
>> +  return offset <= string_size ? string + offset : "";
Agreed.

jeff

[PR 87059] kludge over MIN_EXPR problem causing VRP failure in value ranges

[Aldy Hernandez]

As discussed in the PR, the MIN_EXPR being passed to VRP has 
incompatible signs.  I expect MIN_EXPR to have the same type for all 
arguments plus the MIN_EXPR node itself, but this is not the case.


The culprit on PPC is expand_builtin_strncmp, but fixing it there causes 
other problems on x86-64 (see PR).  I believe Martin Sebor had some 
questions related to the x86 fallout 
(https://gcc.gnu.org/ml/gcc/2018-08/msg00164.html).


Since it seems this has been broken for a while, and I'd like to unbreak 
PPC without having to take my patch out, I suggest (for now) just 
passing the sign of the first argument as VRP had been doing all along 
(through int_const_binop):


int_const_binop():
...
  tree type = TREE_TYPE (arg1);
...
  if (TREE_CODE (arg1) == INTEGER_CST && TREE_CODE (arg2) == INTEGER_CST)
{
  wide_int warg1 = wi::to_wide (arg1), res;
  wide_int warg2 = wi::to_wide (arg2, TYPE_PRECISION (type));
  success = wide_int_binop (res, code, warg1, warg2, sign, );
  poly_res = res;
}

At some point later, if someone is sufficiently vexed by broken 
MIN_EXPR, we could fix it and the x86 fall out.


OK pending tests?

gcc/

	PR 87059/tree-optimization
	* tree-vrp.c (extract_range_from_binary_expr_1): Pass sign of
	first argument to wide_int_range_min_max.

diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 735b3646e81..7373011d901 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -1566,6 +1566,15 @@ extract_range_from_binary_expr_1 (value_range *vr,
   wide_int vr1_min, vr1_max;
   extract_range_into_wide_ints (, sign, prec, vr0_min, vr0_max);
   extract_range_into_wide_ints (, sign, prec, vr1_min, vr1_max);
+
+  /* FIXME: Currently the sign of MIN_EXPR and the sign of its
+	 arguments are not always the same.  Fixing the creators of
+	 these faulty nodes caused other problems.  For now use the
+	 sign of the first argument as VRP was previously doing.  See
+	 PR87059.  */
+  if (vr0.min)
+	sign = TYPE_SIGN (TREE_TYPE (vr0.min));
+
   if (wide_int_range_min_max (wmin, wmax, code, sign, prec,
   vr0_min, vr0_max, vr1_min, vr1_max))
 	set_value_range (vr, VR_RANGE,

Re: new(nothrow) is malloc-like

[Jonathan Wakely]

On 24/08/18 18:25 +0200, Marc Glisse wrote:

Hello,

this makes the throwing and non-throwing versions of operator new more 
consistent with respect to __attribute__((malloc)). The throwing 
versions are already unconditionally declared with DECL_IS_MALLOC = 1 
in the front-end.


Bootstrap+regtest on powerpc64le-unknown-linux-gnu. I manually checked 
that the attribute has an effect.


2018-08-25  Marc Glisse  

PR libstdc++/86822
* libsupc++/new (operator new(size_t, nothrow_t), operator
new[](size_t, nothrow_t), operator new(size_t, align_val_t, nothrow_t),
operator new[](size_t, align_val_t, nothrow_t)): Add malloc attribute.


OK, thanks.

new(nothrow) is malloc-like

[Marc Glisse]

Hello,

this makes the throwing and non-throwing versions of operator new more 
consistent with respect to __attribute__((malloc)). The throwing versions 
are already unconditionally declared with DECL_IS_MALLOC = 1 in the 
front-end.


Bootstrap+regtest on powerpc64le-unknown-linux-gnu. I manually checked 
that the attribute has an effect.


2018-08-25  Marc Glisse  

PR libstdc++/86822
* libsupc++/new (operator new(size_t, nothrow_t), operator
new[](size_t, nothrow_t), operator new(size_t, align_val_t, nothrow_t),
operator new[](size_t, align_val_t, nothrow_t)): Add malloc attribute.

--
Marc GlisseIndex: libstdc++-v3/libsupc++/new
===
--- libstdc++-v3/libsupc++/new	(revision 263834)
+++ libstdc++-v3/libsupc++/new	(working copy)
@@ -130,40 +130,40 @@ void operator delete(void*) _GLIBCXX_USE
   __attribute__((__externally_visible__));
 void operator delete[](void*) _GLIBCXX_USE_NOEXCEPT
   __attribute__((__externally_visible__));
 #if __cpp_sized_deallocation
 void operator delete(void*, std::size_t) _GLIBCXX_USE_NOEXCEPT
   __attribute__((__externally_visible__));
 void operator delete[](void*, std::size_t) _GLIBCXX_USE_NOEXCEPT
   __attribute__((__externally_visible__));
 #endif
 void* operator new(std::size_t, const std::nothrow_t&) _GLIBCXX_USE_NOEXCEPT
-  __attribute__((__externally_visible__));
+  __attribute__((__externally_visible__, __malloc__));
 void* operator new[](std::size_t, const std::nothrow_t&) _GLIBCXX_USE_NOEXCEPT
-  __attribute__((__externally_visible__));
+  __attribute__((__externally_visible__, __malloc__));
 void operator delete(void*, const std::nothrow_t&) _GLIBCXX_USE_NOEXCEPT
   __attribute__((__externally_visible__));
 void operator delete[](void*, const std::nothrow_t&) _GLIBCXX_USE_NOEXCEPT
   __attribute__((__externally_visible__));
 #if __cpp_aligned_new
 void* operator new(std::size_t, std::align_val_t)
   __attribute__((__externally_visible__));
 void* operator new(std::size_t, std::align_val_t, const std::nothrow_t&)
-  _GLIBCXX_USE_NOEXCEPT __attribute__((__externally_visible__));
+  _GLIBCXX_USE_NOEXCEPT __attribute__((__externally_visible__, __malloc__));
 void operator delete(void*, std::align_val_t)
   _GLIBCXX_USE_NOEXCEPT __attribute__((__externally_visible__));
 void operator delete(void*, std::align_val_t, const std::nothrow_t&)
   _GLIBCXX_USE_NOEXCEPT __attribute__((__externally_visible__));
 void* operator new[](std::size_t, std::align_val_t)
   __attribute__((__externally_visible__));
 void* operator new[](std::size_t, std::align_val_t, const std::nothrow_t&)
-  _GLIBCXX_USE_NOEXCEPT __attribute__((__externally_visible__));
+  _GLIBCXX_USE_NOEXCEPT __attribute__((__externally_visible__, __malloc__));
 void operator delete[](void*, std::align_val_t)
   _GLIBCXX_USE_NOEXCEPT __attribute__((__externally_visible__));
 void operator delete[](void*, std::align_val_t, const std::nothrow_t&)
   _GLIBCXX_USE_NOEXCEPT __attribute__((__externally_visible__));
 #if __cpp_sized_deallocation
 void operator delete(void*, std::size_t, std::align_val_t)
   _GLIBCXX_USE_NOEXCEPT __attribute__((__externally_visible__));
 void operator delete[](void*, std::size_t, std::align_val_t)
   _GLIBCXX_USE_NOEXCEPT __attribute__((__externally_visible__));
 #endif // __cpp_sized_deallocation

Re: PING [PATCH] warn for strlen of arrays with missing nul (PR 86552, 86711, 86714) )

[Jeff Law]

On 08/24/2018 06:27 AM, Bernd Edlinger wrote:
[ Lots of snipping throughout ]


>>>
 +
 if (TREE_CODE (src) == COND_EXPR
 && (only_value || !TREE_SIDE_EFFECTS (TREE_OPERAND (src, 0
   {
 -  tree len1, len2;
 -
 -  len1 = c_strlen (TREE_OPERAND (src, 1), only_value);
 -  len2 = c_strlen (TREE_OPERAND (src, 2), only_value);
 +  tree len1 = c_strlen (TREE_OPERAND (src, 1), only_value, arrs);
 +  tree len2 = c_strlen (TREE_OPERAND (src, 2), only_value, arrs + 1);
 if (tree_int_cst_equal (len1, len2))
 -  return len1;
 +  {
>>>
>>> funny, if called with NULL *arr and arrs[0] alias each other.
>>
>>>
 +*arr = arrs[0] ? arrs[0] : arrs[1];
 +return len1;
 +  }
   }
>>
>> And in this case it looks like your comment is before the code you're
>> commenting about.  It was fairly obvious in this case because the code
>> prior to your "funny, if called..." comment didn't reference arr or arrs
>> at all.
>>
>> And more importantly, why are you concerned about the aliasing?
>>
> 
> It is just *arr = arrs[0] does nothing, but it looks like the author
> was not aware of it.  It may be okay, but causes head-scratching.
> If you don't have the context you will think this does something different.
*arr = arrs[0] ? arrs[0] : arrs[1];

Yes, there's potentially a dead store when arr points to arrs[0] because
of the earlier initialization when NULL was passed for arr.  But
otherwise we'd be checking repeatedly that arr != NULL.


 /* PTR can point to the byte representation of any string type, 
 including
char* and wchar_t*.  */
 const char *ptr = TREE_STRING_POINTER (src);
 @@ -650,7 +709,8 @@ c_strlen (tree src, int only_value)
 offsave = fold_convert (ssizetype, offsave);
 tree condexp = fold_build2_loc (loc, LE_EXPR, boolean_type_node, 
 offsave,
  build_int_cst (ssizetype, len * eltsize));
>>>
>>> this computation is wrong, it computes not in units of eltsize,
>>> I am however not sure if it is really good that this function tries to
>>> compute strlen of wide character strings.
>> Note that in the second version of Martin's patch eltsize will always be
>> 1 when we get here.  Furthermore, the multiplication by eltsize is gone.
>>   It looks like you switched back to commenting after the code for that
>> comment, but then immediately thereafter...
>>
> 
> Acutally I had no idea that the second patch did resolve some of my comments
> and which those were.
It happens.  Multiple long threads make it difficult to follow.

> 
> I had the impression that it is just splitting up of a large patch into
> several smaller without reworking at the same time.
> 
> Once again, a summary what was changed would be helpful.
Agreed.


>>> What are you fixing here, I think that was another bug.
>>> If this fixes something then it should be in a different patch,
>>> just handling this.
>>>
 unsigned len = string_length (ptr + eltoff * eltsize, eltsize,
 -  maxelts - eltoff);
 +  strelts - eltoff);
   
 return ssize_int (len);
   }
>> I'm guessing the comment in this case refers to the code after.
>> Presumably questioning the change from maxelts to strelts.
>>
>>
> 
> Yes.  I was thinking that could be a patch of its own.
Funny.  I found myself in agreement and was going to extract this out
and run it through the usual tests and get it onto the trunk
immediately.  Then I realized you'd already fixed this in the patch that
added the eltsize paramter to c_strlen which has already been committed
:-)



> 
> Well, yes, although I changed my mind on strlen(L"abc") meanwhile.
> 
> This may appear as if I contradict myself but, it is more like I learn.
> 
> The installed patch for the ELTSIZE parameter was in part inspired by the
> thought about "not folding invalid stuff" that was forming in my mind
> at that time, even before I wrote it down and sent it to this list.
ACK.  And I think there seems to be consensus forming around that
concept which is good.

If we ultimately decide not to fold strlen of a wide character string,
then that'll be an easy enough change to make.  In the mean time bring
consistent with how the C library strlen works is a good thing IMHO.


 @@ -8255,19 +8333,30 @@ fold_builtin_classify_type (tree arg)
 return build_int_cst (integer_type_node, type_to_class (TREE_TYPE 
 (arg)));
   }
   
 -/* Fold a call to __builtin_strlen with argument ARG.  */
 +/* Fold a strlen call to FNDECL of TYPE, and with argument ARG.  */
   
   static tree
 -fold_builtin_strlen (location_t loc, tree type, tree arg)
 +fold_builtin_strlen (location_t loc, tree fndecl, tree type, tree arg)
   {
 if (!validate_arg (arg, POINTER_TYPE))
   return NULL_TREE;
 else
   {

[PATCH] avoid warning on constant strncpy until next statement is reachable (PR 87028)

[Martin Sebor]

The warning suppression for -Wstringop-truncation looks for
the next statement after a truncating strncpy to see if it
adds a terminating nul.  This only works when the next
statement can be reached using the Gimple statement iterator
which isn't until after gimplification.  As a result, strncpy
calls that truncate their constant argument that are being
folded to memcpy this early get diagnosed even if they are
followed by the nul assignment:

  const char s[] = "12345";
  char d[3];

  void f (void)
  {
strncpy (d, s, sizeof d - 1);   // -Wstringop-truncation
d[sizeof d - 1] = 0;
  }

To avoid the warning I propose to defer folding strncpy to
memcpy until the pointer to the basic block the strnpy call
is in can be used to try to reach the next statement (this
happens as early as ccp1).  I'm aware of the preference to
fold things early but in the case of strncpy (a relatively
rarely used function that is often misused), getting
the warning right while folding a bit later but still fairly
early on seems like a reasonable compromise.  I fear that
otherwise, the false positives will drive users to adopt
other unsafe solutions (like memcpy) where these kinds of
bugs cannot be as readily detected.

Tested on x86_64-linux.

Martin

PS There still are outstanding cases where the warning can
be avoided.  I xfailed them in the test for now but will
still try to get them to work for GCC 9.
PR tree-optimization/87028 - false positive -Wstringop-truncation strncpy with global variable source string
gcc/ChangeLog:

	PR tree-optimization/87028
	* gimple-fold.c (gimple_fold_builtin_strncpy): Avoid folding when
	statement doesn't belong to a basic block.
	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Handle MEM_REF on
	the left hand side of assignment.

gcc/testsuite/ChangeLog:

	PR tree-optimization/87028
	* c-c++-common/Wstringop-truncation.c: Remove xfails.
	* gcc.dg/Wstringop-truncation-5.c: New test.

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 07341eb..284c2fb 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -1702,6 +1702,11 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi,
   if (tree_int_cst_lt (ssize, len))
 return false;
 
+  /* Defer warning (and folding) until the next statement in the basic
+ block is reachable.  */
+  if (!gimple_bb (stmt))
+return false;
+
   /* Diagnose truncation that leaves the copy unterminated.  */
   maybe_diag_stxncpy_trunc (*gsi, src, len);
 
diff --git a/gcc/testsuite/c-c++-common/Wstringop-truncation.c b/gcc/testsuite/c-c++-common/Wstringop-truncation.c
index e78e85e..4ddb9bd 100644
--- a/gcc/testsuite/c-c++-common/Wstringop-truncation.c
+++ b/gcc/testsuite/c-c++-common/Wstringop-truncation.c
@@ -329,9 +329,8 @@ void test_strncpy_array (Dest *pd, int i, const char* s)
  of the array to NUL is not diagnosed.  */
   {
 /* This might be better written using memcpy() but it's safe so
-   it probably shouldn't be diagnosed.  It currently triggers
-   a warning because of bug 81704.  */
-strncpy (dst7, "0123456", sizeof dst7);   /* { dg-bogus "\\\[-Wstringop-truncation]" "bug 81704" { xfail *-*-* } } */
+   it isn't diagnosed.  See pr81704 and pr87028.  */
+strncpy (dst7, "0123456", sizeof dst7);   /* { dg-bogus "\\\[-Wstringop-truncation]" } */
 dst7[sizeof dst7 - 1] = '\0';
 sink (dst7);
   }
@@ -350,7 +349,7 @@ void test_strncpy_array (Dest *pd, int i, const char* s)
   }
 
   {
-strncpy (pd->a5, "01234", sizeof pd->a5);   /* { dg-bogus "\\\[-Wstringop-truncation]" "bug 81704" { xfail *-*-* } } */
+strncpy (pd->a5, "01234", sizeof pd->a5);   /* { dg-bogus "\\\[-Wstringop-truncation]" } */
 pd->a5[sizeof pd->a5 - 1] = '\0';
 sink (pd);
   }
diff --git a/gcc/testsuite/gcc.dg/Wstringop-truncation-5.c b/gcc/testsuite/gcc.dg/Wstringop-truncation-5.c
new file mode 100644
index 000..03bcba9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-truncation-5.c
@@ -0,0 +1,111 @@
+/* PR tree-optimization/87028 - false positive -Wstringop-truncation
+   strncpy with global variable source string
+   { dg-do compile }
+   { dg-options "-O2 -Wstringop-truncation" } */
+
+char *strncpy (char *, const char *, __SIZE_TYPE__);
+
+void sink (char*);
+
+#define STR   "1234567890"
+
+struct S
+{
+  char a[5], b[5], *p;
+};
+
+const char arr[] = STR;
+const char arr2[][10] = { "123", STR };
+
+const char* const ptr = STR;
+
+char a[5];
+
+void test_literal (char *d, struct S *s)
+{
+  strncpy (d, STR, 3);  /* { dg-bogus "\\\[-Wstringop-truncation]" } */
+  d[3] = '\0';
+
+  strncpy (a, STR, sizeof a - 1);   /* { dg-bogus "\\\[-Wstringop-truncation]" } */
+  a[sizeof a - 1] = '\0';
+
+  strncpy (s->a, STR, sizeof s->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */
+  s->a[sizeof s->a - 1] = '\0';
+
+  strncpy (>b[0], STR, sizeof s->b - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */
+  s->b[sizeof s->b - 1] = '\0';
+
+  strncpy (s->p, STR, 4);

Re: [PATCH,Darwin, config] Fix PR70694; don't force visibility on inlines for Darwin > 8

[Jonathan Wakely]

On 24/08/18 16:09 +0100, Iain Sandoe wrote:

At some point, with the toolchain components available to Darwin8, it was
preferable to force everything generated by headers to be hidden.
However, the work-around wasn’t limited and it has caused Darwin x86/x86_64
to have a blanket application of  "-fvisibility-inlines-hidden” ever since.

Since, in libstdc++, the visibilities are specified explicitly and the flag
causes code to be hidden that is intended to be visible.  This causes a lot of
warnings when the library exports are made (and results in the PR).

In the patch, I’ve left Darwin4-7 as is (corresponding to OS X 10.0-10.4) and 
work-
around in place for Darwin8 (since I’ve not been able to test the change with 
the
final xcode tools available there).

Fix confirmed on Darwin10, with current trunk (this has been bootstrapped many
times while sitting in my queue).

OK for trunk?


OK, thanks.

IIUC libc++ puts explicit visibility("hidden") attributes on
individual inline functions, but I don't know what their reason for
that is. It might not be a darwin-specific thing.

[PATCH][debug] Add -gdescribe-dies

[Tom de Vries]

[ was: Re: [PATCH][debug] Add -gdescriptive-dies ]
On Fri, Aug 24, 2018 at 12:44:38PM +0200, Richard Biener wrote:
> On Wed, 22 Aug 2018, Tom de Vries wrote:
> 
> > [ was: Re: [PATCH][debug] Add -gforce-named-dies ]
> > 
> > On 08/22/2018 11:46 AM, Tom de Vries wrote:
> > > On 08/22/2018 08:56 AM, Tom de Vries wrote:
> > >> This is an undocumented developer-only option, because using this option 
> > >> may
> > >> change behaviour of dwarf consumers, f.i., gdb shows the artificial 
> > >> variables:
> > >> ...
> > >> (gdb) info locals
> > >> a = 0x7fffda90 "\005"
> > >> D.4278 = 
> > >> ...
> > > I just found in the dwarf 5 spec the attribute DW_AT_description
> > > (present since version 3):
> > > ...
> > > 2.20 Entity Descriptions
> > > Some debugging information entries may describe entities in the program
> > > that are artificial, or which otherwise have a “name” that is not a
> > > valid identifier in the programming language.
> > > 
> > > This attribute provides a means for the producer to indicate the purpose
> > > or usage of the containing debugging infor
> > > 
> > > Generally, any debugging information entry that has, or may have, a
> > > DW_AT_name attribute, may also have a DW_AT_description attribute whose
> > > value is a null-terminated string providing a description of the entity.
> > > 
> > > It is expected that a debugger will display these descriptions as part
> > > of displaying other properties of an entity.
> > > ...
> > > 
> > > AFAICT, gdb currently does not explicitly handle this attribute, which I
> > > think means it's ignored.
> > > 
> > > It seems applicable to use DW_AT_description at least for the artificial
> > > decls.
> > > 
> > > Perhaps even for all cases that are added by the patch?
> > > 
> > 
> > I've chosen for this option. Using DW_AT_desciption instead of
> > DW_AT_name should minimize difference in gdb behaviour with and without
> > -gdescriptive-dies.
> 
> -gdescribe-dies?
>

Done.

> > > I'll rewrite the patch.
> > 
> > OK for trunk?
> 
> Few comments:
> 
> +static void
> +add_desc_attribute (dw_die_ref die, tree decl)
> +{
> +  tree decl_name;
> +
> +  if (!flag_descriptive_dies || dwarf_version < 3)
> +return;
> +
> 
> you said this is a DWARF5 "feature",

No, it's a DWARF3 "feature". I copied the text from the DWARF5 spec.

> I'd suggest changing the
> check to
> 
>   if (!flag_desctiprive_dies || (dwarf_version < 5 && dwarf_strict))
> 
> given -gdescribe-dies is enough of a user request to enable the
> feature.

Done.

> Not sure if we should warn about -gstrict-dwarf
> combination with it and -gdwarf-N < 5.
>

Not sure either, left it out.

> +  else if (TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) == 
> CONST_DECL)
> +{
> +  char buf[32];
> +  char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
> +  sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
> +  add_desc_attribute (die, buf);
> +}
> 
> I wondered before if we can use pretty-printing of decl here.  At
> least I wouldn't restrict it to VAR_DECLs, FUNCTION_DECLs can
> certainly appear here I think.
>

Done.

> +@item -gdescriptive-dies
> +@opindex gdescriptive-dies
> +Add description attribute to DWARF DIEs that have no name attribute.
> +
> 
> Either "description attributes" or "a description attribute", not
> 100% sure being a non-native speaker.
>

Went for "description attributes".

> Otherwise the patch looks OK to me but please leave Jason time
> to comment.
>

Will do.

Untested patch attached.

Thanks,
- Tom

[debug] Add -gdescribe-dies

This patch adds option -gdescribe-dies.  It sets the DW_AT_description
attribute of dies that do not get a DW_AT_name attribute, to make it easier to
figure out what the die is describing.

The option exports the names of artificial variables:
...
 DIE0: DW_TAG_variable (0x7fa934dd54b0)
+  DW_AT_description: "D.1922"
   DW_AT_type: die -> 0 (0x7fa934dd0d70)
   DW_AT_artificial: 1

...
which can be traced back to gimple dumps:
...
  char a[0:D.1922] [value-expr: *a.0];
...

Furthermore, it adds names to external references:
...
 DIE0: DW_TAG_subprogram (0x7fa88b9650f0)
+DW_AT_description: "main"
 DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 29 (0x7fa88b965140)
...

2018-08-15  Tom de Vries  

* common.opt (gdescribe-dies): Add option.
* dwarf2out.c (add_name_and_src_coords_attributes): Add description
attribute for artifical and nameless decls.
(dwarf2out_register_external_die): Add description attribute to
external reference die.
(add_desc_attribute): New functions.
(gen_subprogram_die): Add description attribute to
DW_TAG_call_site_parameter.
* tree-pretty-print.c (print_generic_expr_to_str): New function.
* tree-pretty-print.h (print_generic_expr_to_str): Declare.
* doc/invoke.texi (@item Debugging Options): Add -gdescribe-dies and
-gno-describe-dies.
(@item -gdescribe-dies): Add.

Re: [PATCH] treat -Wxxx-larger-than=HWI_MAX special (PR 86631)

[Martin Sebor]

On 08/24/2018 03:36 AM, Richard Biener wrote:

On Fri, Aug 24, 2018 at 12:35 AM Martin Sebor  wrote:


On 08/23/2018 07:18 AM, Richard Biener wrote:

On Thu, Aug 23, 2018 at 12:20 AM Martin Sebor  wrote:


On 08/20/2018 06:14 AM, Richard Biener wrote:

On Thu, Jul 26, 2018 at 10:52 PM Martin Sebor  wrote:


On 07/26/2018 08:58 AM, Martin Sebor wrote:

On 07/26/2018 02:38 AM, Richard Biener wrote:

On Wed, Jul 25, 2018 at 5:54 PM Martin Sebor  wrote:


On 07/25/2018 08:57 AM, Jakub Jelinek wrote:

On Wed, Jul 25, 2018 at 08:54:13AM -0600, Martin Sebor wrote:

I don't mean for the special value to be used except internally
for the defaults.  Otherwise, users wanting to override the default
will choose a value other than it.  I'm happy to document it in
the .opt file for internal users though.

-1 has the documented effect of disabling the warnings altogether
(-1 is SIZE_MAX) so while I agree that -1 looks better it doesn't
work.  (It would need more significant changes.)


The variable is signed, so -1 is not SIZE_MAX.  Even if -1 disables
it, you
could use e.g. -2 or other negative value for the other special case.


The -Wxxx-larger-than=N distinguish three ranges of argument
values (treated as unsigned):

   1.  [0, HOST_WIDE_INT_MAX)
   2.  HOST_WIDE_INT_MAX
   3.  [HOST_WIDE_INT_MAX + 1, Infinity)


But it doesn't make sense for those to be host dependent.


It isn't when the values are handled by each warning.  That's
also the point of this patch: to remove this (unintended)
dependency.


I think numerical user input should be limited to [0, ptrdiff_max]
and cases (1) and (2) should be simply merged, I see no value
in distinguishing them.  -Wxxx-larger-than should be aliased
to [0, ptrdiff_max], case (3) is achieved by -Wno-xxx-larger-than.


To be clear: this is also close to what this patch does.

The only wrinkle is that we don't know the value of PTRDIFF_MAX
either at the time the option initial value is set in the .opt
file or when the option is processed when it's specified either
on the command line or as an alias in the .opt file (as all
-Wno-xxx-larger-than options are).


But then why not make that special value accessible and handle
it as PTRDIFF_MAX when that is available (at users of the params)?

That is,

Index: gcc/calls.c
===
--- gcc/calls.c (revision 262951)
+++ gcc/calls.c (working copy)
@@ -1222,9 +1222,12 @@ alloc_max_size (void)
   if (alloc_object_size_limit)
 return alloc_object_size_limit;

-  alloc_object_size_limit
-= build_int_cst (size_type_node, warn_alloc_size_limit);
+  HOST_WIDE_INT limit = warn_alloc_size_limit;
+  if (limit == HOST_WIDE_INT_MAX)
+limit = tree_to_shwi (TYPE_MAX_VALUE (ptrdiff_type_node));

+  alloc_object_size_limit = build_int_cst (size_type_node, limit);
+
   return alloc_object_size_limit;
 }

use sth like

 if (warn_alloc_size_limit == -1)
   alloc_object_size_limit = fold_convert (size_type_node,
TYPE_MAX_VALUE (ptrdiff_type_node));
 else
   alloc_object_size_limit = size_int (warn_alloc_size_limit);

?  Also removing the need to have > int params values.


Not sure I understand this last part.  Remove the enhancement?
(We do need to handle option arguments in excess of INT_MAX.)


I see.



It's that HOST_WIDE_INT_MAX use that is problematic IMHO.  Why not use -1?


-1 is a valid/documented value of the argument of all these
options because it's treated as unsigned HWI:

   Warnings controlled by the option can be disabled either
   by specifying byte-size of ‘SIZE_MAX’

It has an intuitive meaning: warning for any size larger than
the maximum means not warning at all.  Treating -1 as special
instead of HOST_WIDE_INT_MAX would replace that meaning with
"warn on any size in excess of PTRDIFF_MAX."

A reasonable way to disable the warning is like so:

   gcc -Walloc-size-larger-than=$(getconf ULONG_MAX) ...

That would not work anymore.

Treating HOST_WIDE_INT_MAX as PTRDIFF_MAX is the natural choice
on LP64: they have the same value.  It's only less than perfectly
natural in ILP32 and even there it's not a problem in practice
because it's either far out of the range of valid values [0, 4GB]
(i.e., where HWI is a 64-bit long long), or it's also equal to
PTRDIFF_MAX (on hosts with no 64-bit type, if GCC even supports
any).

I'm not trying to be stubborn here but I just don't see why
you think that setting aside HOST_WIDE_INT_MAX is problematic.
Anything else is worse from a user-interface POV.  It makes
little difference inside GCC as long as we want to let users
choose to warn for allocation sizes over some value in
[PTRDIFF_MAX, SIZE_MAX] -- e.g., for malloc() over 3GB but
not for less.  There's also the -Walloca-size-larger-than=
case where PTRDIFF_MAX means only warn for known sizes over
than but not for unknown sizes.


Well I understand all of the above.  Alternatively you can omit
the initializer for the option and use

  if 

[PATCH,Darwin, config] Fix PR70694; don't force visibility on inlines for Darwin > 8

[Iain Sandoe]

At some point, with the toolchain components available to Darwin8, it was
preferable to force everything generated by headers to be hidden.
However, the work-around wasn’t limited and it has caused Darwin x86/x86_64
to have a blanket application of  "-fvisibility-inlines-hidden” ever since.

Since, in libstdc++, the visibilities are specified explicitly and the flag
causes code to be hidden that is intended to be visible.  This causes a lot of
warnings when the library exports are made (and results in the PR).

In the patch, I’ve left Darwin4-7 as is (corresponding to OS X 10.0-10.4) and 
work-
around in place for Darwin8 (since I’ve not been able to test the change with 
the
final xcode tools available there).

Fix confirmed on Darwin10, with current trunk (this has been bootstrapped many
times while sitting in my queue).

OK for trunk?
Iain

libstdc++/

PR libstdc++/70694
* configure.host (OPT_LDFLAGS): Don't append
-fvisibility-inlines-hidden for newer Darwin.
---
 libstdc++-v3/configure.host | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/libstdc++-v3/configure.host b/libstdc++-v3/configure.host
index caea9de9c7..155a3cdea1 100644
--- a/libstdc++-v3/configure.host
+++ b/libstdc++-v3/configure.host
@@ -230,16 +230,15 @@ case "${host_os}" in
 os_include_dir="os/newlib"
 OPT_LDFLAGS="${OPT_LDFLAGS} \$(lt_host_flags)"
 ;;
-  darwin | darwin[1-7] | darwin[1-7].*)
-# On Darwin, performance is improved if libstdc++ is single-module.
-# Up to at least 10.3.7, -flat_namespace is required for proper
-# treatment of coalesced symbols.
+  darwin[4-7] | darwin[4-7].*)
+# For earlier Darwin, performance is improved if libstdc++ is
+# single-module. Up to at least 10.3.7, -flat_namespace is required
+# for proper treatment of coalesced symbols.
 OPT_LDFLAGS="${OPT_LDFLAGS} -Wl,-single_module -Wl,-flat_namespace"
 os_include_dir="os/bsd/darwin"
 ;;
-  darwin[89] | darwin[89].* | darwin[1-9][0-9]* )
-# On Darwin, performance is improved if libstdc++ is single-module,
-# and on 8+ compatibility is better if not -flat_namespace.
+  darwin8 | darwin8.* )
+# For 8+ compatibility is better if not -flat_namespace.
 OPT_LDFLAGS="${OPT_LDFLAGS} -Wl,-single_module"
 case "${host_cpu}" in
   i[34567]86 | x86_64)
@@ -248,6 +247,10 @@ case "${host_os}" in
 esac
 os_include_dir="os/bsd/darwin"
 ;;
+  darwin*)
+# Post Darwin8, defaults should be sufficient.
+os_include_dir="os/bsd/darwin"
+;;
   *djgpp*)  # leading * picks up "msdosdjgpp"
 os_include_dir="os/djgpp"
 error_constants_dir="os/djgpp"
-- 
2.17.1

Re: OpenRISC Port Submission

[Richard Henderson]

On 08/23/2018 06:04 PM, David Edelsohn wrote:
> Stafford,
> 
> Thanks for rewriting and contributing the new OpenRISC port for GCC. I have
> forwarded your request to the GCC Steering Committee to formally accept the
> contribution.
> 
> The port still requires a technical review and approval from a GCC Global
> Reviewer.  You mentioned Richard Henderson as co-author.  Has he reviewed
> the entire port?  If Richard signs off on the port, approval could be
> relatively quick.

I've looked through the port enough to know there's nothing weird.
There may well be nits to be picked.  Also, I am a couple of years
out of date and there may well be new preferred practices that I
have missed.


r~

Re: [PATCH]: Remove remaining traces of MPX bounded pointers

[Jeff Law]

On 08/24/2018 07:56 AM, Uros Bizjak wrote:
> 2018-08-23  Uros Bizjak  
> 
> * emit-rtl.c (init_emit_once): Do not emit MODE_POINTER_BOUNDS RTXes.
> * emit-rtl.h (rtl_data): Remove return_bnd.
> * explow.c (trunc_int_for_mode): Do not handle POINTER_BOUNDS_MODE_P.
> * function.c (diddle_return_value): Do not handle crtl->return_bnd.
> * genmodes.c (complete_mode): Do not handle MODE_POINTER_BOUNDS.
> (POINTER_BOUNDS_MODE): Remove definition.
> (make_pointer_bounds_mode): Remove.
> (get_mode_class): Do not handle MODE_POINTER_BOUNDS.
> * machmode.h (POINTER_BOUNDS_MODE_P): Remove definition.
> (scalare_mode::includes_p): Do not handle MODE_POINTER_BOUNDS.
> * mode-classes.def: Do not define MODE_POINTER_BOUNDS.
> * stor-layout.c (int_mode_for_mode): Do not handle MODE_POINTER_BOUNDS.
> * tree-core.h (enum tree_index): Remove TI_POINTER_BOUNDS_TYPE.
> * varasm.c (output_constant_pool_2): Do not handle MODE_POINTER_BOUNDS.
> 
> * config/i386/i386-modes.def (BND32, BND64): Remove.
> * config/i386/i386.c (dbx_register_map): Remove bound registers.
> (dbx64_register_map): Ditto.
> (svr4_dbx_register_map): Ditto.
> (indirect_thunk_bnd_needed): Remove.
> (indirect_thunks_bnd_used): Ditto.
> (indirect_return_bnd_needed): Ditto.
> (indirect_return_via_cx_bnd): Ditto.
> (enum indirect_thunk_prefix): Remove indirect_thunk_prefix_bnd.
> (indirect_thunk_name): Remove handling of indirect_thunk_prefix_bnd.
> (output_indirect_thunk): Ditto.  Remove need_prefix argument.
> (output_indirect_thunk_function): Remove handling of
> indirect_return_bnd_needed, indirect_return_via_cx_bnd,
> indirect_thunk_bnd_needed and indirect_thunks_bnd_used variables.
> (ix86_save_reg): Remove handling of crtl->return_bnd.
> (ix86_legitimate_constant_p): Remove handling of POINTER_BOUNDS_MODE_P.
> (ix86_print_operand_address_as): Remove handling of UNSPEC_BNDMK_ADDR
> and UNSPEC_BNDLX_ADDR.
> (ix86_output_indirect_branch_via_reg): Remove handling of
> indirect_thunk_prefix_bnd.
> (ix86_output_indirect_branch_via_push): Ditto.
> (ix86_output_function_return): Ditto.
> (ix86_output_indirect_function_return): Ditto.
> (avoid_func_arg_motion): Do not handle UNSPEC_BNDSTX.
> * config/i386/i386.h (FIXED_REGISTERS): Remove bound registers.
> (CALL_USED_REGISTERS): Ditto.
> (REG_ALLOC_ORDER): Update for removal of bound registers.
> (HI_REGISTER_NAMES): Ditto.
> * config/i386/i386.md (UNSPEC_BNDMK, UNSPEC_BNDMK_ADDR, UNSPEC_BNDSTX)
> (UNSPEC_BNDLDX, UNSPEC_BNDLDX_ADDR, UNSPEC_BNDCL, UNSPEC_BNDCU)
> (UNSPEC_BNDCN, UNSPEC_MPX_FENCE): Remove.
> (BND0_REG, BND1_REG, BND2_REG, BND3_REG): Remove
> (FIRST_PSEUDO_REG): Update.
> (BND): Remove mode iterator.
> * config/i386/predicates.md (bnd_mem_operator): Remove.
> 
> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
Oh how I wish we'd done all this back in 2017.  It would have made the
spectrev2 work easier to backport to the old releases which didn't have
all the BND stuff.  Such is life.

> 
> OK for mainline?
OK.

Jeff

[PATCH] Fix overeager spelling corrections (PR c/82967)

[David Malcolm]

This patch tunes class best_match's cutoff for rejecting meaningless
spelling suggestions.

Previously, we allowed an edit distance of up to half of the length of the
longer of the goal string and closest candidate strings, rounded down.

With this patch, we now allow only up to a third - with some tuning of
rounding (and for very short strings), to ensure that:
(a) everything that worked before still works (with the removal of a
couple of cases that shouldn't), and that
(b) the new threshold is always at least as conservative as the old
threshold and thus shouldn't offer new nonsensical suggestions (with
the possible exception of cases where transposition has helped; see
r261521 aka Damerau-Levenshtein; PR other/69968).

In particular, all of the bogus suggestions from PR c/82967 are now
no longer offered.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
adds 4 PASS results to gcc.sum.

OK for trunk?

OK for backporting to gcc 8? (with the caveat that gcc 8's
edit distance doesn't do transpositions)

gcc/ChangeLog:
PR c/82967
* spellcheck.c (get_edit_distance_cutoff): New function.
(selftest::test_edit_distance_unit_test_oneway): Rename to...
(selftest::test_get_edit_distance_one_way): ...this.
(selftest::test_get_edit_distance_unit): Rename to...
(selftest::test_get_edit_distance_both_ways): ...this.
(selftest::test_edit_distances): Move tests to this new function,
and test some more pairs of strings.  Update for above renaming.
(selftest::get_old_cutoff): New function.
(selftest::test_get_edit_distance_cutoff): New function.
(selftest::assert_suggested_for): New function.
(ASSERT_SUGGESTED_FOR): New macro.
(selftest::assert_not_suggested_for): New function.
(ASSERT_NOT_SUGGESTED_FOR): New macro.
(selftest::test_suggestions): New function.
(selftest::spellcheck_c_tests): Move test_get_edit_distance_unit
tests to selftest::test_edit_distances and call it.  Add calls to
selftest::test_get_edit_distance_cutoff and
selftest::test_suggestions.
* spellcheck.h (get_edit_distance_cutoff): New function declaration.
(best_match::consider): Replace hard-coded cutoff calculation with
a call to...
(best_match::get_cutoff): New declaration.
(best_match::get_best_meaningful_candidate): Likewise.

gcc/testsuite/ChangeLog:
PR c/82967
* c-c++-common/attributes-1.c: Remove bogus suggestion from
dg-prune-output.
* gcc.dg/diagnostic-token-ranges.c (undeclared_identifier): Remove
bogus suggestion.
* gcc.dg/spellcheck-identifiers-4.c: New test.
---
 gcc/spellcheck.c| 231 
 gcc/spellcheck.h|  19 +-
 gcc/testsuite/c-c++-common/attributes-1.c   |   2 +-
 gcc/testsuite/gcc.dg/diagnostic-token-ranges.c  |   3 +-
 gcc/testsuite/gcc.dg/spellcheck-identifiers-4.c |  10 +
 5 files changed, 224 insertions(+), 41 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-identifiers-4.c

diff --git a/gcc/spellcheck.c b/gcc/spellcheck.c
index e2a8b92..690e6fa 100644
--- a/gcc/spellcheck.c
+++ b/gcc/spellcheck.c
@@ -162,6 +162,36 @@ find_closest_string (const char *target,
   return bm.get_best_meaningful_candidate ();
 }
 
+/* Generate the maximum edit distance for which we consider a suggestion
+   to be meaningful, given a goal of length GOAL_LEN and a candidate of
+   length CANDIDATE_LEN.
+
+   This is a third of the the length of the candidate or of the goal,
+   whichever is bigger.  */
+
+edit_distance_t
+get_edit_distance_cutoff (size_t goal_len, size_t candidate_len)
+{
+  size_t max_length = MAX (goal_len, candidate_len);
+  size_t min_length = MIN (goal_len, candidate_len);
+
+  gcc_assert (max_length >= min_length);
+
+  /* Special case: don't offer suggestions for a pair of
+ length == 1 strings (or empty strings).  */
+  if (max_length <= 1)
+return 0;
+
+  /* If the lengths are close, then round down.  */
+  if (max_length - min_length <= 1)
+/* ...but allow an edit distance of at least 1.  */
+return MAX (max_length / 3, 1);
+
+  /* Otherwise, round up (thus giving a little extra leeway to some cases
+ involving insertions/deletions).  */
+  return (max_length + 2) / 3;
+}
+
 #if CHECKING_P
 
 namespace selftest {
@@ -171,8 +201,8 @@ namespace selftest {
 /* Verify that get_edit_distance (A, B) equals the expected value.  */
 
 static void
-test_edit_distance_unit_test_oneway (const char *a, const char *b,
-   edit_distance_t expected)
+test_get_edit_distance_one_way (const char *a, const char *b,
+   edit_distance_t expected)
 {
   edit_distance_t actual = get_edit_distance (a, b);
   ASSERT_EQ (actual, expected);
@@ -185,11 +215,169 @@ test_edit_distance_unit_test_oneway (const char *a, 
const 

Optimize sreal normalization

[Jan Hubicka]

Hi,
this patch makes new exp and sig explicit parameter of normalize function.
This is better for inline analysis to track when the result is going to be 
constant
and make sus to inline sreal constructor for constants.

My main motivation is however to change memory representation of sreal so we do
not need 64bit sig + 32bit exponent.  This is needed only temporarily because
we always normalize to 33bit sig (32bit value + sign).  This needs bit more
massaging to get rid of the extra bit so I will send it as followup.

Bootstrapped/regtested x86_64-linux OK?

Honza

* sreal.h (normalize, normalize_up, normalize_down): Add new_sig/new_exp
parameters.
(sreal constructor): Update.
* sreal.c (sreal:operator+, sreal:operator-, sreal:operator*,
sreal:operator/): Update.
Index: sreal.h
===
--- sreal.h (revision 263834)
+++ sreal.h (working copy)
@@ -45,9 +45,9 @@ public:
   sreal () : m_sig (-1), m_exp (-1) {}
 
   /* Construct a sreal.  */
-  sreal (int64_t sig, int exp = 0) : m_sig (sig), m_exp (exp)
+  sreal (int64_t sig, int exp = 0)
   {
-normalize ();
+normalize (sig, exp);
   }
 
   void dump (FILE *) const;
@@ -130,9 +130,9 @@ public:
   }
 
 private:
-  inline void normalize ();
-  inline void normalize_up ();
-  inline void normalize_down ();
+  inline void normalize (int64_t new_sig, signed int new_exp);
+  inline void normalize_up (int64_t new_sig, signed int new_exp);
+  inline void normalize_down (int64_t new_sig, signed int new_exp);
   void shift_right (int amount);
   static sreal signedless_plus (const sreal , const sreal , bool negative);
   static sreal signedless_minus (const sreal , const sreal , bool 
negative);
@@ -199,23 +199,24 @@ inline sreal operator>> (const sreal ,
Make this separate method so inliner can handle hot path better.  */
 
 inline void
-sreal::normalize_up ()
+sreal::normalize_up (int64_t new_sig, signed int new_exp)
 {
-  unsigned HOST_WIDE_INT sig = absu_hwi (m_sig);
+  unsigned HOST_WIDE_INT sig = absu_hwi (new_sig);
   int shift = SREAL_PART_BITS - 2 - floor_log2 (sig);
 
   gcc_checking_assert (shift > 0);
   sig <<= shift;
-  m_exp -= shift;
+  new_exp -= shift;
   gcc_checking_assert (sig <= SREAL_MAX_SIG && sig >= SREAL_MIN_SIG);
 
   /* Check underflow.  */
-  if (m_exp < -SREAL_MAX_EXP)
+  if (new_exp < -SREAL_MAX_EXP)
 {
-  m_exp = -SREAL_MAX_EXP;
+  new_exp = -SREAL_MAX_EXP;
   sig = 0;
 }
-  if (SREAL_SIGN (m_sig) == -1)
+  m_exp = new_exp;
+  if (SREAL_SIGN (new_sig) == -1)
 m_sig = -sig;
   else
 m_sig = sig;
@@ -226,16 +227,16 @@ sreal::normalize_up ()
Make this separate method so inliner can handle hot path better.  */
 
 inline void
-sreal::normalize_down ()
+sreal::normalize_down (int64_t new_sig, signed int new_exp)
 {
   int last_bit;
-  unsigned HOST_WIDE_INT sig = absu_hwi (m_sig);
+  unsigned HOST_WIDE_INT sig = absu_hwi (new_sig);
   int shift = floor_log2 (sig) - SREAL_PART_BITS + 2;
 
   gcc_checking_assert (shift > 0);
   last_bit = (sig >> (shift-1)) & 1;
   sig >>= shift;
-  m_exp += shift;
+  new_exp += shift;
   gcc_checking_assert (sig <= SREAL_MAX_SIG && sig >= SREAL_MIN_SIG);
 
   /* Round the number.  */
@@ -243,16 +244,17 @@ sreal::normalize_down ()
   if (sig > SREAL_MAX_SIG)
 {
   sig >>= 1;
-  m_exp++;
+  new_exp++;
 }
 
   /* Check overflow.  */
-  if (m_exp > SREAL_MAX_EXP)
+  if (new_exp > SREAL_MAX_EXP)
 {
-  m_exp = SREAL_MAX_EXP;
+  new_exp = SREAL_MAX_EXP;
   sig = SREAL_MAX_SIG;
 }
-  if (SREAL_SIGN (m_sig) == -1)
+  m_exp = new_exp;
+  if (SREAL_SIGN (new_sig) == -1)
 m_sig = -sig;
   else
 m_sig = sig;
@@ -261,16 +263,24 @@ sreal::normalize_down ()
 /* Normalize *this; the hot path.  */
 
 inline void
-sreal::normalize ()
+sreal::normalize (int64_t new_sig, signed int new_exp)
 {
-  unsigned HOST_WIDE_INT sig = absu_hwi (m_sig);
+  unsigned HOST_WIDE_INT sig = absu_hwi (new_sig);
 
   if (sig == 0)
-m_exp = -SREAL_MAX_EXP;
+{
+  m_sig = 0;
+  m_exp = -SREAL_MAX_EXP;
+}
   else if (sig > SREAL_MAX_SIG)
-normalize_down ();
+normalize_down (new_sig, new_exp);
   else if (sig < SREAL_MIN_SIG)
-normalize_up ();
+normalize_up (new_sig, new_exp);
+  else
+{
+  m_sig = new_sig;
+  m_exp = new_exp;
+}
 }
 
 #endif
Index: sreal.c
===
--- sreal.c (revision 263834)
+++ sreal.c (working copy)
@@ -138,7 +138,8 @@ sreal
 sreal::operator+ (const sreal ) const
 {
   int dexp;
-  sreal tmp, r;
+  sreal tmp;
+  int64_t r_sig, r_exp;
 
   const sreal *a_p = this, *b_p = , *bb;
 
@@ -146,10 +147,14 @@ sreal::operator+ (const sreal ) co
 std::swap (a_p, b_p);
 
   dexp = a_p->m_exp - b_p->m_exp;
-  r.m_exp = a_p->m_exp;
+  r_exp = a_p->m_exp;
   if (dexp > SREAL_BITS)
 {
-  r.m_sig = a_p->m_sig;
+  r_sig = 

[PATCH]: Remove remaining traces of MPX bounded pointers

[Uros Bizjak]

2018-08-23  Uros Bizjak  

* emit-rtl.c (init_emit_once): Do not emit MODE_POINTER_BOUNDS RTXes.
* emit-rtl.h (rtl_data): Remove return_bnd.
* explow.c (trunc_int_for_mode): Do not handle POINTER_BOUNDS_MODE_P.
* function.c (diddle_return_value): Do not handle crtl->return_bnd.
* genmodes.c (complete_mode): Do not handle MODE_POINTER_BOUNDS.
(POINTER_BOUNDS_MODE): Remove definition.
(make_pointer_bounds_mode): Remove.
(get_mode_class): Do not handle MODE_POINTER_BOUNDS.
* machmode.h (POINTER_BOUNDS_MODE_P): Remove definition.
(scalare_mode::includes_p): Do not handle MODE_POINTER_BOUNDS.
* mode-classes.def: Do not define MODE_POINTER_BOUNDS.
* stor-layout.c (int_mode_for_mode): Do not handle MODE_POINTER_BOUNDS.
* tree-core.h (enum tree_index): Remove TI_POINTER_BOUNDS_TYPE.
* varasm.c (output_constant_pool_2): Do not handle MODE_POINTER_BOUNDS.

* config/i386/i386-modes.def (BND32, BND64): Remove.
* config/i386/i386.c (dbx_register_map): Remove bound registers.
(dbx64_register_map): Ditto.
(svr4_dbx_register_map): Ditto.
(indirect_thunk_bnd_needed): Remove.
(indirect_thunks_bnd_used): Ditto.
(indirect_return_bnd_needed): Ditto.
(indirect_return_via_cx_bnd): Ditto.
(enum indirect_thunk_prefix): Remove indirect_thunk_prefix_bnd.
(indirect_thunk_name): Remove handling of indirect_thunk_prefix_bnd.
(output_indirect_thunk): Ditto.  Remove need_prefix argument.
(output_indirect_thunk_function): Remove handling of
indirect_return_bnd_needed, indirect_return_via_cx_bnd,
indirect_thunk_bnd_needed and indirect_thunks_bnd_used variables.
(ix86_save_reg): Remove handling of crtl->return_bnd.
(ix86_legitimate_constant_p): Remove handling of POINTER_BOUNDS_MODE_P.
(ix86_print_operand_address_as): Remove handling of UNSPEC_BNDMK_ADDR
and UNSPEC_BNDLX_ADDR.
(ix86_output_indirect_branch_via_reg): Remove handling of
indirect_thunk_prefix_bnd.
(ix86_output_indirect_branch_via_push): Ditto.
(ix86_output_function_return): Ditto.
(ix86_output_indirect_function_return): Ditto.
(avoid_func_arg_motion): Do not handle UNSPEC_BNDSTX.
* config/i386/i386.h (FIXED_REGISTERS): Remove bound registers.
(CALL_USED_REGISTERS): Ditto.
(REG_ALLOC_ORDER): Update for removal of bound registers.
(HI_REGISTER_NAMES): Ditto.
* config/i386/i386.md (UNSPEC_BNDMK, UNSPEC_BNDMK_ADDR, UNSPEC_BNDSTX)
(UNSPEC_BNDLDX, UNSPEC_BNDLDX_ADDR, UNSPEC_BNDCL, UNSPEC_BNDCU)
(UNSPEC_BNDCN, UNSPEC_MPX_FENCE): Remove.
(BND0_REG, BND1_REG, BND2_REG, BND3_REG): Remove
(FIRST_PSEUDO_REG): Update.
(BND): Remove mode iterator.
* config/i386/predicates.md (bnd_mem_operator): Remove.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

OK for mainline?

Uros.
diff --git a/gcc/config/i386/i386-modes.def b/gcc/config/i386/i386-modes.def
index 08c79a5df4e9..12c17ce7dfc2 100644
--- a/gcc/config/i386/i386-modes.def
+++ b/gcc/config/i386/i386-modes.def
@@ -98,9 +98,6 @@ VECTOR_MODE (INT, QI, 14);/*  V14QI */
 VECTOR_MODE (INT, HI, 6); /*   V6HI */
 VECTOR_MODE (INT, SI, 64);/* V64SI */
 
-POINTER_BOUNDS_MODE (BND32, 8);
-POINTER_BOUNDS_MODE (BND64, 16);
-
 INT_MODE (OI, 32);
 INT_MODE (XI, 64);
 
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 03118102319b..98677386a2b9 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -262,7 +262,7 @@ enum reg_class const regclass_map[FIRST_PSEUDO_REGISTER] =
   EVEX_SSE_REGS, EVEX_SSE_REGS, EVEX_SSE_REGS, EVEX_SSE_REGS,
   /* Mask registers.  */
   MASK_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS,
-  MASK_EVEX_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS,
+  MASK_EVEX_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS, MASK_EVEX_REGS
 };
 
 /* The "default" register map used in 32bit mode.  */
@@ -278,8 +278,7 @@ int const dbx_register_map[FIRST_PSEUDO_REGISTER] =
   -1, -1, -1, -1, -1, -1, -1, -1,  /* extended SSE registers */
   -1, -1, -1, -1, -1, -1, -1, -1,   /* AVX-512 registers 16-23*/
   -1, -1, -1, -1, -1, -1, -1, -1,   /* AVX-512 registers 24-31*/
-  93, 94, 95, 96, 97, 98, 99, 100,  /* Mask registers */
-  101, 102, 103, 104,  /* bound registers */
+  93, 94, 95, 96, 97, 98, 99, 100   /* Mask registers */
 };
 
 /* The "default" register map used in 64bit mode.  */
@@ -295,8 +294,7 @@ int const dbx64_register_map[FIRST_PSEUDO_REGISTER] =
   25, 26, 27, 28, 29, 30, 31, 32,  /* extended SSE registers */
   67, 68, 69, 70, 71, 72, 73, 74,   /* AVX-512 registers 16-23 */
   75, 76, 77, 78, 79, 80, 81, 82,   /* AVX-512 registers 24-31 */
-  118, 119, 120, 121, 122, 123, 124, 125, /* Mask registers */
-  126, 127, 128, 129,  /* bound registers */
+  118, 119, 120, 121, 122, 123, 124, 125 /* Mask registers */
 };
 
 /* Define the register numbers to be used in Dwarf 

Re: C++ PATCH for c++/87029, Implement -Wredundant-move

[Jason Merrill]

On Fri, Aug 24, 2018 at 12:53 AM, Marek Polacek  wrote:
> On Thu, Aug 23, 2018 at 10:44:30AM -0400, Marek Polacek wrote:
>> +T
>> +fn3 (const T t)
>> +{
>> +  // t is const: will decay into move.
>> +  return t;
>> +}
>> +
>> +T
>> +fn4 (const T t)
>> +{
>> +  // t is const: will decay into move despite std::move, so it's redundant.
>> +  return std::move (t); // { dg-warning "redundant move in return 
>> statement" }
>> +}
>
> This should read "decay into copy".  We can't move from a const object.  
> Consider
> it fixed.

Well, we'll do the overload resolution as though t were an rvalue,
even though it's const; it's just unlikely to succeed, since a
constructor taking a const rvalue reference doesn't make much sense.

It occurs to me that the std::move might not be redundant in some
cases: for the implicit treatment as an rvalue, the return must select
a constructor that takes an rvalue reference to the returned object's
type.  With an explict std::move, that restriction doesn't apply.  So,
for

struct C { };
struct A {
  operator C() &;
  operator C() &&;
};

C f(A a)
{
  return a; // calls operator C()&
  return std::move(a); // calls operator C()&&
}

...though I see we currently get the first return wrong, and call the
rvalue overload for both.  I think there was a recent core issue in
this area, I'll try to find that later.

Anyway, I imagine this sort of example is rare enough that the warning
is still worth having; people doing this can use static_cast instead
or just turn off the warning.

BTW, Instead of using location_of (retval) in each diagnostic call,
let's put at the top of the function

  location_t loc = cp_expr_loc_or_loc (retval, input_location);

and use 'loc' in the diagnostics.  This is the pattern I generally mean to use.

Jason

[PATCH] Use complete_array_type on flexible array member initializers

[Bernd Edlinger]

Hi!


This patch prevents init values of STRING_CST and braced
array initializers to reach the middle-end with incomplete
type.

This will allow further simplifications in the middle-end,
and address existing issues with STRING_CST in a correct
way.



Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.
gcc:
2018-08-24  Bernd Edlinger  

	* varasm.c (output_constructor_regular_field): Check TYPE_SIZE_UNIT of
	the init value.

c-family:
2018-08-24  Bernd Edlinger  

	* c-common.c (complete_flexible_array_elts): New helper function.
	* c-common.h (complete_flexible_array_elts): Declare.

c:
2018-08-24  Bernd Edlinger  

	* c-decl.c (finish_decl): Call complete_flexible_array_elts.

cp:
2018-08-24  Bernd Edlinger  

	* decl.c (check_initializer): Call complete_flexible_array_elts.


diff -Npur gcc/c/c-decl.c gcc/c/c-decl.c
--- gcc/c/c-decl.c	2018-08-21 08:17:41.0 +0200
+++ gcc/c/c-decl.c	2018-08-24 12:06:21.374892294 +0200
@@ -5035,6 +5035,8 @@ finish_decl (tree decl, location_t init_
   if (init && TREE_CODE (init) == CONSTRUCTOR)
 	add_flexible_array_elts_to_size (decl, init);
 
+  complete_flexible_array_elts (DECL_INITIAL (decl));
+
   if (DECL_SIZE (decl) == NULL_TREE && TREE_TYPE (decl) != error_mark_node
 	  && COMPLETE_TYPE_P (TREE_TYPE (decl)))
 	layout_decl (decl, 0);
diff -Npur gcc/c-family/c-common.c gcc/c-family/c-common.c
--- gcc/c-family/c-common.c	2018-08-17 05:02:11.0 +0200
+++ gcc/c-family/c-common.c	2018-08-24 12:45:56.559011703 +0200
@@ -6427,6 +6427,28 @@ complete_array_type (tree *ptype, tree i
   return failure;
 }
 
+/* INIT is an constructor of a structure with a flexible array member.
+   Complete the flexible array member with a domain based on it's value.  */
+void
+complete_flexible_array_elts (tree init)
+{
+  tree elt, type;
+
+  if (init == NULL_TREE || TREE_CODE (init) != CONSTRUCTOR)
+return;
+
+  if (vec_safe_is_empty (CONSTRUCTOR_ELTS (init)))
+return;
+
+  elt = CONSTRUCTOR_ELTS (init)->last ().value;
+  type = TREE_TYPE (elt);
+  if (TREE_CODE (type) == ARRAY_TYPE
+  && TYPE_SIZE (type) == NULL_TREE)
+complete_array_type (_TYPE (elt), elt, false);
+  else
+complete_flexible_array_elts (elt);
+}
+
 /* Like c_mark_addressable but don't check register qualifier.  */
 void 
 c_common_mark_addressable_vec (tree t)
diff -Npur gcc/c-family/c-common.h gcc/c-family/c-common.h
--- gcc/c-family/c-common.h	2018-08-17 05:02:11.0 +0200
+++ gcc/c-family/c-common.h	2018-08-24 12:06:21.375892280 +0200
@@ -1038,6 +1038,7 @@ extern tree fold_offsetof (tree, tree =
 			   tree_code ctx = ERROR_MARK);
 
 extern int complete_array_type (tree *, tree, bool);
+extern void complete_flexible_array_elts (tree);
 
 extern tree builtin_type_for_size (int, bool);
 
diff -Npur gcc/cp/decl.c gcc/cp/decl.c
--- gcc/cp/decl.c	2018-08-22 22:35:38.0 +0200
+++ gcc/cp/decl.c	2018-08-24 12:06:21.377892252 +0200
@@ -6528,6 +6528,8 @@ check_initializer (tree decl, tree init,
 
 	  init_code = store_init_value (decl, init, cleanups, flags);
 
+	  complete_flexible_array_elts (DECL_INITIAL (decl));
+
 	  if (pedantic && TREE_CODE (type) == ARRAY_TYPE
 	  && DECL_INITIAL (decl)
 	  && TREE_CODE (DECL_INITIAL (decl)) == STRING_CST
diff -Npur gcc/varasm.c gcc/varasm.c
--- gcc/varasm.c	2018-08-16 17:28:11.0 +0200
+++ gcc/varasm.c	2018-08-24 12:06:21.378892238 +0200
@@ -5161,6 +5161,8 @@ output_constructor_regular_field (oc_loc
 	 on the chain is a TYPE_DECL of the enclosing struct.  */
 	  const_tree next = DECL_CHAIN (local->field);
 	  gcc_assert (!fieldsize || !next || TREE_CODE (next) != FIELD_DECL);
+	  tree size = TYPE_SIZE_UNIT (TREE_TYPE (local->val));
+	  gcc_checking_assert (compare_tree_int (size, fieldsize) == 0);
 	}
   else
 	fieldsize = tree_to_uhwi (DECL_SIZE_UNIT (local->field));

Re: Avoid is_constant calls in vectorizable_bswap

[Richard Biener]

On Thu, Aug 23, 2018 at 11:09 AM Richard Sandiford
 wrote:
>
> The "new" VEC_PERM_EXPR handling makes it easy to support bswap
> for variable-length vectors.
>
> Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf
> and x86_64-linux-gnu.  OK to install?

OK.

Richard.

> Richard
>
>
> 2018-08-23  Richard Sandiford  
>
> gcc/
> * tree-vect-stmts.c (vectorizable_bswap): Handle variable-length
> vectors.
>
> gcc/testsuite/
> * gcc.target/aarch64/sve/bswap_1.c: New test.
> * gcc.target/aarch64/sve/bswap_2.c: Likewise.
> * gcc.target/aarch64/sve/bswap_3.c: Likewise.
>
> Index: gcc/tree-vect-stmts.c
> ===
> --- gcc/tree-vect-stmts.c   2018-08-23 09:59:35.245682525 +0100
> +++ gcc/tree-vect-stmts.c   2018-08-23 10:07:30.233601466 +0100
> @@ -2961,13 +2961,10 @@ vectorizable_bswap (stmt_vec_info stmt_i
>vec_info *vinfo = stmt_info->vinfo;
>loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
>unsigned ncopies;
> -  unsigned HOST_WIDE_INT nunits, num_bytes;
>
>op = gimple_call_arg (stmt, 0);
>vectype = STMT_VINFO_VECTYPE (stmt_info);
> -
> -  if (!TYPE_VECTOR_SUBPARTS (vectype).is_constant ())
> -return false;
> +  poly_uint64 nunits = TYPE_VECTOR_SUBPARTS (vectype);
>
>/* Multiple types in SLP are handled by creating the appropriate number of
>   vectorized stmts for each SLP node.  Hence, NCOPIES is always 1 in
> @@ -2983,11 +2980,11 @@ vectorizable_bswap (stmt_vec_info stmt_i
>if (! char_vectype)
>  return false;
>
> -  if (!TYPE_VECTOR_SUBPARTS (char_vectype).is_constant (_bytes))
> +  poly_uint64 num_bytes = TYPE_VECTOR_SUBPARTS (char_vectype);
> +  unsigned word_bytes;
> +  if (!constant_multiple_p (num_bytes, nunits, _bytes))
>  return false;
>
> -  unsigned word_bytes = num_bytes / nunits;
> -
>/* The encoding uses one stepped pattern for each byte in the word.  */
>vec_perm_builder elts (num_bytes, word_bytes, 3);
>for (unsigned i = 0; i < 3; ++i)
> Index: gcc/testsuite/gcc.target/aarch64/sve/bswap_1.c
> ===
> --- /dev/null   2018-07-26 10:26:13.137955424 +0100
> +++ gcc/testsuite/gcc.target/aarch64/sve/bswap_1.c  2018-08-23 
> 10:07:30.233601466 +0100
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -ftree-vectorize" } */
> +
> +#include 
> +
> +void
> +f (uint16_t *a, uint16_t *b)
> +{
> +  for (int i = 0; i < 100; ++i)
> +a[i] = __builtin_bswap16 (b[i]);
> +}
> +
> +/* { dg-final { scan-assembler-times {\trevb\tz[0-9]+\.h, p[0-7]/m, 
> z[0-9]+\.h\n} 1 { xfail aarch64_big_endian } } } */
> Index: gcc/testsuite/gcc.target/aarch64/sve/bswap_2.c
> ===
> --- /dev/null   2018-07-26 10:26:13.137955424 +0100
> +++ gcc/testsuite/gcc.target/aarch64/sve/bswap_2.c  2018-08-23 
> 10:07:30.233601466 +0100
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -ftree-vectorize" } */
> +
> +#include 
> +
> +void
> +f (uint32_t *a, uint32_t *b)
> +{
> +  for (int i = 0; i < 100; ++i)
> +a[i] = __builtin_bswap32 (b[i]);
> +}
> +
> +/* { dg-final { scan-assembler-times {\trevb\tz[0-9]+\.s, p[0-7]/m, 
> z[0-9]+\.s\n} 1 { xfail aarch64_big_endian } } } */
> Index: gcc/testsuite/gcc.target/aarch64/sve/bswap_3.c
> ===
> --- /dev/null   2018-07-26 10:26:13.137955424 +0100
> +++ gcc/testsuite/gcc.target/aarch64/sve/bswap_3.c  2018-08-23 
> 10:07:30.233601466 +0100
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -ftree-vectorize" } */
> +
> +#include 
> +
> +void
> +f (uint64_t *a, uint64_t *b)
> +{
> +  for (int i = 0; i < 100; ++i)
> +a[i] = __builtin_bswap64 (b[i]);
> +}
> +
> +/* { dg-final { scan-assembler-times {\trevb\tz[0-9]+\.d, p[0-7]/m, 
> z[0-9]+\.d\n} 1 { xfail aarch64_big_endian } } } */

Re: Handle SLP permutations for variable-length vectors

[Richard Biener]

On Thu, Aug 23, 2018 at 11:08 AM Richard Sandiford
 wrote:
>
> The SLP code currently punts for all variable-length permutes.
> This patch makes it handle the easy case of N->N permutes in which
> the number of vector lanes is a multiple of N.  Every permute then
> uses the same mask, and that mask repeats (with a stride) every
> N elements.
>
> The patch uses the same path for constant-length vectors,
> since it should be slightly cheaper in terms of compile time.
>
> Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf
> and x86_64-linux-gnu.  OK to install?

OK.

Thanks,
Richard.

> Richard
>
>
> 2018-08-23  Richard Sandiford  
>
> gcc/
> * tree-vect-slp.c (vect_transform_slp_perm_load): Separate out
> the case in which the permute needs only a single element and
> repeats for every vector of the result.  Extend that case to
> handle variable-length vectors.
> * tree-vect-stmts.c (vectorizable_load): Update accordingly.
>
> gcc/testsuite/
> * gcc.target/aarch64/sve/slp_perm_1.c: New test.
> * gcc.target/aarch64/sve/slp_perm_2.c: Likewise.
> * gcc.target/aarch64/sve/slp_perm_3.c: Likewise.
> * gcc.target/aarch64/sve/slp_perm_4.c: Likewise.
> * gcc.target/aarch64/sve/slp_perm_5.c: Likewise.
> * gcc.target/aarch64/sve/slp_perm_6.c: Likewise.
> * gcc.target/aarch64/sve/slp_perm_7.c: Likewise.
>
> Index: gcc/tree-vect-slp.c
> ===
> *** gcc/tree-vect-slp.c 2018-08-21 14:47:08.339163256 +0100
> --- gcc/tree-vect-slp.c 2018-08-23 09:59:35.245682525 +0100
> *** vect_transform_slp_perm_load (slp_tree n
> *** 3606,3618 
>   {
> stmt_vec_info stmt_info = SLP_TREE_SCALAR_STMTS (node)[0];
> vec_info *vinfo = stmt_info->vinfo;
> -   tree mask_element_type = NULL_TREE, mask_type;
> int vec_index = 0;
> tree vectype = STMT_VINFO_VECTYPE (stmt_info);
> !   int group_size = SLP_INSTANCE_GROUP_SIZE (slp_node_instance);
> unsigned int mask_element;
> machine_mode mode;
> -   unsigned HOST_WIDE_INT nunits, const_vf;
>
> if (!STMT_VINFO_GROUPED_ACCESS (stmt_info))
>   return false;
> --- 3606,3616 
>   {
> stmt_vec_info stmt_info = SLP_TREE_SCALAR_STMTS (node)[0];
> vec_info *vinfo = stmt_info->vinfo;
> int vec_index = 0;
> tree vectype = STMT_VINFO_VECTYPE (stmt_info);
> !   unsigned int group_size = SLP_INSTANCE_GROUP_SIZE (slp_node_instance);
> unsigned int mask_element;
> machine_mode mode;
>
> if (!STMT_VINFO_GROUPED_ACCESS (stmt_info))
>   return false;
> *** vect_transform_slp_perm_load (slp_tree n
> *** 3620,3641 
> stmt_info = DR_GROUP_FIRST_ELEMENT (stmt_info);
>
> mode = TYPE_MODE (vectype);
> !
> !   /* At the moment, all permutations are represented using per-element
> !  indices, so we can't cope with variable vector lengths or
> !  vectorization factors.  */
> !   if (!TYPE_VECTOR_SUBPARTS (vectype).is_constant ()
> !   || !vf.is_constant (_vf))
> ! return false;
> !
> !   /* The generic VEC_PERM_EXPR code always uses an integral type of the
> !  same size as the vector element being permuted.  */
> !   mask_element_type = lang_hooks.types.type_for_mode
> ! (int_mode_for_mode (TYPE_MODE (TREE_TYPE (vectype))).require (), 1);
> !   mask_type = get_vectype_for_scalar_type (mask_element_type);
> !   vec_perm_builder mask (nunits, nunits, 1);
> !   mask.quick_grow (nunits);
> !   vec_perm_indices indices;
>
> /* Initialize the vect stmts of NODE to properly insert the generated
>stmts later.  */
> --- 3618,3624 
> stmt_info = DR_GROUP_FIRST_ELEMENT (stmt_info);
>
> mode = TYPE_MODE (vectype);
> !   poly_uint64 nunits = TYPE_VECTOR_SUBPARTS (vectype);
>
> /* Initialize the vect stmts of NODE to properly insert the generated
>stmts later.  */
> *** vect_transform_slp_perm_load (slp_tree n
> *** 3669,3682 
> bool noop_p = true;
> *n_perms = 0;
>
> !   for (unsigned int j = 0; j < const_vf; j++)
>   {
> !   for (int k = 0; k < group_size; k++)
> {
> ! unsigned int i = (SLP_TREE_LOAD_PERMUTATION (node)[k]
> !   + j * DR_GROUP_SIZE (stmt_info));
> ! vec_index = i / nunits;
> ! mask_element = i % nunits;
>   if (vec_index == first_vec_index
>   || first_vec_index == -1)
> {
> --- 3652,3704 
> bool noop_p = true;
> *n_perms = 0;
>
> !   vec_perm_builder mask;
> !   unsigned int nelts_to_build;
> !   unsigned int nvectors_per_build;
> !   bool repeating_p = (group_size == DR_GROUP_SIZE (stmt_info)
> ! && multiple_p (nunits, group_size));
> !   if (repeating_p)
> ! {
> !   /* A single vector contains a whole number of copies of the node, so:
> !(a) all permutes can use the same mask; and
> !(b) the permutes only 

Re: C++ PATCH for c++/67012, c++/86942, detect invalid cases with function return type deduction

[Jason Merrill]

On Fri, Aug 24, 2018 at 3:01 AM, Marek Polacek  wrote:
> On Tue, Aug 21, 2018 at 11:59:06PM +1200, Jason Merrill wrote:
>> On Fri, Aug 17, 2018 at 2:17 PM, Marek Polacek  wrote:
>> > As I promised in 
>> > ,
>> > this patch fixes a couple of invalid cases we weren't detecting.  It's got
>> > testcases from two PRs and another case I found out; they're intertwined so
>> > I think it makes sense to fix them in one go.
>> >
>> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>> >
>> > 2018-08-16  Marek Polacek  
>> >
>> > PR c++/86942
>> > PR c++/67012
>> > * decl.c (grokdeclarator): Disallow functions with trailing return
>> > type with decltype(auto) as its type.  Also check the function if
>> > it's inner declarator doesn't exist.
>> >
>> > * g++.dg/cpp0x/auto52.C: New test.
>> > * g++.dg/cpp1y/auto-fn52.C: New test.
>> > * g++.dg/cpp1y/auto-fn53.C: New test.
>> > * g++.dg/cpp1y/auto-fn54.C: New test.
>> >
>> > diff --git gcc/cp/decl.c gcc/cp/decl.c
>> > index fa58bc4d2b3..8261f8e30e5 100644
>> > --- gcc/cp/decl.c
>> > +++ gcc/cp/decl.c
>> > @@ -11238,7 +11238,10 @@ grokdeclarator (const cp_declarator *declarator,
>> >
>> > /* Handle a late-specified return type.  */
>> > tree late_return_type = 
>> > declarator->u.function.late_return_type;
>> > -   if (funcdecl_p)
>> > +   if (funcdecl_p
>> > +   /* This is the case e.g. for
>> > +  using T = auto () -> int.  */
>> > +   || inner_declarator == NULL)
>>
>> Hmm, checking funcdecl_p here seems just wrong; these errors should be
>> the same regardless of whether this is declaring a function.  What
>> breaks if we just remove this condition?  The deduction guide errors
>> will need to be adjusted to handle the abstract declarator case, but
>> that looks like the only spot that would need fixing.
>
> That was my first idea but it breaks with pointers to functions and pointers
> to member functions as in auto2.C:
>
> auto (*fp)() = f;
> where the declarators are: cdk_function -> cdk_pointer -> cdk_id
> auto (A::*pmf)() = ::f;
> where the declarators are: cdk_function -> cdk_ptrmem -> cdk_id
>
> it complains that a function uses 'auto' type specifier without trailing 
> return type

Ah, right.

>> >   {
>> > if (tree auto_node = type_uses_auto (type))
>> >   {
>> > @@ -11270,6 +11273,18 @@ grokdeclarator (const cp_declarator *declarator,
>> >name, type);
>> > return error_mark_node;
>> >   }
>> > +   else if (is_auto (type)
>> > +&& (TYPE_IDENTIFIER (type)
>> > +== decltype_auto_identifier))
>>
>> I think you want AUTO_IS_DECLTYPE here.
>
> Ah, nice.

OK with this change, then.

Jason

Re: [PATCH] Improve checks in c_strlen (PR 87053)

[Bernd Edlinger]

On 08/24/18 07:58, Jeff Law wrote:
> On 08/23/2018 03:27 AM, Bernd Edlinger wrote:
>> On 08/22/18 18:28, Martin Sebor wrote:
>>> On 08/22/2018 08:41 AM, Bernd Edlinger wrote:
 Hi!


 This patch adds some more checks to c_getstr to fix PR middle-end/87053
 wrong code bug.

 Unfortunately this patch alone is not sufficient to fix the problem,
 but also the patch for PR 86714 that hardens c_getstr is necessary
 to prevent the wrong folding.


 Bootstrapped and reg-tested on top of my PR 86711/86714 patch.
 Is it OK for trunk?
>>>
>>> This case is also the subject of the patch I submitted back in
>>> July for 86711/86714 and 86552.  With it, GCC avoid folding
>>> the strlen call early and warns for the missing nul:
>>>
>>> warning: ‘__builtin_strlen’ argument missing terminating nul 
>>> [-Wstringop-overflow=]
>>>      if (__builtin_strlen (u.z) != 7)
>>>      ^~~~
>>>
>>> The patch doesn't doesn't prevent all such strings from being
>>> folded and it eventually lets fold_builtin_strlen() do its thing:
>>>
>>>     /* To avoid warning multiple times about unterminated
>>>    arrays only warn if its length has been determined
>>>    and is being folded to a constant.  */
>>>     if (nonstr)
>>>       warn_string_no_nul (loc, NULL_TREE, fndecl, nonstr);
>>>
>>>     return fold_convert_loc (loc, type, len);
>>>
>>> Handling this case is a matter of avoiding the folding here as
>>> well and moving the warning later.
>>>
>>> Since my patch is still in the review queue and does much more
>>> than just prevent folding of non-nul terminated arrays it should
>>> be reviewed first.
>>>
>>
>> Hmmm, now you made me curious.
>>
>> So I tried to install your patch (I did this on r263508
>> since it does not apply to trunk, one thing I noted is
>> that part 4 and part 3 seem to create 
>> gcc/testsuite/gcc.dg/warn-strcpy-no-nul.c
>> I did not check if they are identical or not).
>>
>> So I tried the test case from this PR on the compiler built with your patch:
>>
>> $ cat cat pr87053.c
>> /* PR middle-end/87053 */
>>
>> const union
>> { struct {
>>   char x[4];
>>   char y[4];
>> };
>> struct {
>>   char z[8];
>> };
>> } u = {{"1234", "567"}};
>>
>> int main ()
>> {
>> if (__builtin_strlen (u.z) != 7)
>>   __builtin_abort ();
>> }
>> $ gcc -S pr87053.c
>> pr87053.c: In function 'main':
>> pr87053.c:15:7: warning: '__builtin_strlen' argument missing terminating nul 
>> [-Wstringop-overflow=]
>> 15 |   if (__builtin_strlen (u.z) != 7)
>>  |   ^~~~
>> pr87053.c:11:3: note: referenced argument declared here
>> 11 | } u = {{"1234", "567"}};
>>  |   ^
>> $ cat pr87053.s
>>  .file   "pr87053.c"
>>  .text
>>  .globl  u
>>  .section.rodata
>>  .align 8
>>  .type   u, @object
>>  .size   u, 8
>> u:
>>  .ascii  "1234"
>>  .string "567"
>>  .text
>>  .globl  main
>>  .type   main, @function
>> main:
>> .LFB0:
>>  .cfi_startproc
>>  pushq   %rbp
>>  .cfi_def_cfa_offset 16
>>  .cfi_offset 6, -16
>>  movq%rsp, %rbp
>>  .cfi_def_cfa_register 6
>>  callabort
>>  .cfi_endproc
>> .LFE0:
>>  .size   main, .-main
>>  .ident  "GCC: (GNU) 9.0.0 20180813 (experimental)"
>>  .section.note.GNU-stack,"",@progbits
>>
>>
>> So we get a warning, and still wrong code.
>>
>> That is the reason why I think this patch of yours adds
>> confusion by trying to fix everything in one step.
>>
>> And I would like you to think of ways how to solve
>> a problem step by step.
>>
>> And at this time, sorry, we should restore correctness issues.
>> And fix wrong-code issues.
>> If possible without breaking existing warnings, yes.
>> But no new warnings, sorry again.
> Just a note, Martin's most fix for 86711/86714 fixes codegen issues
> without breaking existing warnings or adding new warnings.  The new
> warnings were broken out into follow-up patches.
> 

BTW: the warning about u.z not null terminated is bogus.

There are middle-end consistency and correctness issues all over.
They have IMO precedence even over wrong-code issues.


Bernd.

Re: PING [PATCH] warn for strlen of arrays with missing nul (PR 86552, 86711, 86714) )

[Bernd Edlinger]

On 08/24/18 08:36, Jeff Law wrote:
> On 08/02/2018 07:26 AM, Bernd Edlinger wrote:
>> On 08/02/18 04:44, Martin Sebor wrote:
>>> Since the foundation of the patch is detecting and avoiding
>>> the overly aggressive folding of unterminated char arrays,
>>> besides issuing a warning for such arguments to strlen,
>>> the patch also fixes pr86711 - wrong folding of memchr, and
>>> pr86714 - tree-ssa-forwprop.c confused by too long initializer.
>>>
>>> The substance of the attached updated patch is unchanged,
>>> I have just added test cases for the two additional bugs.
>>>
>>> Bernd, as I mentioned Wednesday, the patch supersedes
>>> yours here:
>>> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01800.html
>>>
>>
>> No problem, but I hope you understand, that I still uphold
>> my patch.
>>
>> So we have two patches now:
>> - mine, fixing a wrong code bug,
>> - yours, implementing a new warning and fixing a wrong
>> code bug at the same time.
>>
>> I will add a few comments to your patch below.
> [ ... ]
> 
> So a lot of the comments are out of date, presumably because Martin
> fixed the issues you pointed out in his second version of the patch.
> But there's still some useful nuggets in your comments that are still
> relevant.
> 

Yes, possible, and therefore I would like updated patches summarize
the changes.

> FYI it appears that sometimes you comment above a chunk of code, and
> other times below.  That makes it exceedingly difficult to figure out
> the issue you're trying to raise.
> 

Okydoky.


> 
>>
>>> Martin
>>>
>>> On 07/30/2018 01:17 PM, Martin Sebor wrote:
 Attached is an updated version of the patch that handles more
 instances of calling strlen() on a constant array that is not
 a nul-terminated string.

 No other functions except strlen are explicitly handled yet,
 and neither are constant arrays with braced-initializer lists
 like const char a[] = { 'a', 'b', 'c' };  I am testing
 an independent solution for those (bug 86552).  Once those
 are handled the warning will be able to detect those as well.

 Tested on x86_64-linux.

 On 07/25/2018 05:38 PM, Martin Sebor wrote:
> Ping: https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01124.html
>
> The fix for bug 86532 has been checked in so this enhancement
> can now be applied on top of it (with only minor adjustments).
>
> On 07/19/2018 02:08 PM, Martin Sebor wrote:
>> In the discussion of my patch for pr86532 Bernd noted that
>> GCC silently accepts constant character arrays with no
>> terminating nul as arguments to strlen (and other string
>> functions).
>>
>> The attached patch is a first step in detecting these kinds
>> of bugs in strlen calls by issuing -Wstringop-overflow.
>> The next step is to modify all other handlers of built-in
>> functions to detect the same problem (not part of this patch).
>> Yet another step is to detect these problems in arguments
>> initialized using the non-string form:
>>
>>    const char a[] = { 'a', 'b', 'c' };
>>
>> This patch is meant to apply on top of the one for bug 86532
>> (I tested it with an earlier version of that patch so there
>> is code in the context that does not appear in the latest
>> version of the other diff).
>>
>> Martin
>>
>

>>>
>>> PR tree-optimization/86714 - tree-ssa-forwprop.c confused by too long 
>>> initializer
>>> PR tree-optimization/86711 - wrong folding of memchr
>>> PR tree-optimization/86552 - missing warning for reading past the end of 
>>> non-string arrays
>>>
>>> gcc/ChangeLog:
>>>
>>> PR tree-optimization/86714
>>> PR tree-optimization/86711
>>> PR tree-optimization/86552
>>> * builtins.h (warn_string_no_nul): Declare..
>>> (c_strlen): Add argument.
>>> * builtins.c (warn_string_no_nul): New function.
>>> (fold_builtin_strlen): Add argument.  Detect missing nul.
>>> (fold_builtin_1): Adjust.
>>> (string_length): Add argument and use it.
>>> (c_strlen): Same.
>>> (expand_builtin_strlen): Detect missing nul.
>>> * expr.c (string_constant): Add arguments.  Detect missing nul
>>> terminator and outermost declaration it's missing in.
>>> * expr.h (string_constant): Add argument.
>>> * fold-const.c (c_getstr): Change argument to bool*, rename
>>> other arguments.
>>> * fold-const-call.c (fold_const_call): Detect missing nul.
>>> * gimple-fold.c (get_range_strlen): Add argument.
>>> (get_maxval_strlen): Adjust.
>>> * gimple-fold.h (get_range_strlen): Add argument.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> PR tree-optimization/86714
>>> PR tree-optimization/86711
>>> PR tree-optimization/86552
>>> * gcc.c-torture/execute/memchr-1.c: New test.
>>> * gcc.c-torture/execute/pr86714.c: New test.
>>> * gcc.dg/warn-strlen-no-nul.c: New test.
>>>
>>> diff --git a/gcc/builtins.c b/gcc/builtins.c
>>> index aa3e0d8..f4924d5 

Re: [PATCH] Print default options selection for -march,-mcpu and -mtune for aarch64 (PR driver/83193).

[Martin Liška]

On 08/23/2018 04:46 PM, Richard Earnshaw (lists) wrote:
> On 02/08/18 10:46, Martin Liška wrote:
>> On 08/02/2018 11:39 AM, Richard Earnshaw (lists) wrote:
>>> On 18/07/18 16:48, Martin Liška wrote:
 Hi.

 This is aarch64 fix for PR83193. It's about setting of default options
 so that --help=target -Q prints proper numbers:

 Now this is seen on my cross-compiler:

 --- /home/marxin/Downloads/options-2-before.txt2018-07-18 
 14:53:11.658146543 +0200
 +++ /home/marxin/Downloads/options-2.txt   2018-07-18 14:52:30.113274284 
 +0200
 @@ -1,10 +1,10 @@
  The following options are target specific:
-mabi=ABI   lp64
 -  -march=ARCH 
 +  -march= armv8-a
>>>
>>> So we have
>>>
>>> -mabi=ABI   lp64
>>>
>>> but
>>>
>>> -march= armv8-a
>>>^ blank
>>>
>>> Isn't that inconsistent?
>>
>> It is probably, in this case I would remove 'ABI' from -mabi option. It's 
>> explained bellow
>> what are possible options:
>>
>>   Known AArch64 ABIs (for use with the -mabi= option):
>> ilp32 lp64
>>
>> Similarly for:
>>   -moverride=STRING   Power users only! Override CPU optimization 
>> parameters.
>>   -msve-vector-bits=N Set the number of bits in an SVE vector 
>> register to N.
>>
>> It's more common to  notation, there are some samples from 
>> --help=common:
>>
>>   -fmax-errors=   Maximum number of errors to report.
>>   -fmessage-length=   Limit diagnostics to  characters per 
>> line.  0 suppresses line-wrapping.
>>   -fira-region=[one|all|mixed] Set regions for IRA.
>>   -fira-verbose=  Control IRA's level of diagnostic messages.
>>   -flifetime-dse=<0,2>This option lacks documentation.
>>   -fstack-limit-register= Trap if the stack goes past .
>>   -fstack-limit-symbol= Trap if the stack goes past symbol .
>>
>> Are you fine with the suggested approach?
> 
> Yes, those look sensible.
> 
> R.
> 
>>
>> Martin
>>
>>>
>>> R.
>>>
-mbig-endian[disabled]
-mbionic[disabled]
-mcmodel=   small
 -  -mcpu=CPU   
 +  -mcpu=  generic
-mfix-cortex-a53-835769 [enabled]
-mfix-cortex-a53-843419 [enabled]
-mgeneral-regs-only [disabled]
 @@ -19,7 +19,7 @@
-msve-vector-bits=N scalable
-mtls-dialect=  desc
-mtls-size= 24
 -  -mtune=CPU  
 +  -mtune= generic
-muclibc[disabled]

 May I please ask ARM folks to test the patch?
 Thanks,
 Martin

 gcc/ChangeLog:

 2018-07-18  Martin Liska  

 PR driver/83193
* config/aarch64/aarch64.c (aarch64_override_options_internal):
 Set default values for x_aarch64_*_string strings.
* config/aarch64/aarch64.opt: Remove --{march,mcpu,mtune}==
 prefix.
 ---
  gcc/config/aarch64/aarch64.c   | 7 +++
  gcc/config/aarch64/aarch64.opt | 6 +++---
  2 files changed, 10 insertions(+), 3 deletions(-)



 0001-Print-default-options-selection-for-march-mcpu-and-m.patch


 diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
 index 6fa03e4b091..d48e6278efa 100644
 --- a/gcc/config/aarch64/aarch64.c
 +++ b/gcc/config/aarch64/aarch64.c
 @@ -10713,6 +10713,13 @@ aarch64_override_options_internal (struct 
 gcc_options *opts)
&& opts->x_optimize >= 
 aarch64_tune_params.prefetch->default_opt_level)
  opts->x_flag_prefetch_loop_arrays = 1;
  
 +  if (opts->x_aarch64_arch_string == NULL)
 +opts->x_aarch64_arch_string = selected_arch->name;
 +  if (opts->x_aarch64_cpu_string == NULL)
 +opts->x_aarch64_cpu_string = selected_cpu->name;
 +  if (opts->x_aarch64_tune_string == NULL)
 +opts->x_aarch64_tune_string = selected_tune->name;
 +
aarch64_override_options_after_change_1 (opts);
  }
  
 diff --git a/gcc/config/aarch64/aarch64.opt 
 b/gcc/config/aarch64/aarch64.opt
 index 1426b45ff0f..7f0b65de37b 100644
 --- a/gcc/config/aarch64/aarch64.opt
 +++ b/gcc/config/aarch64/aarch64.opt
 @@ -117,15 +117,15 @@ Enum(aarch64_tls_size) String(48) Value(48)
  
  march=
  Target RejectNegative ToLower Joined Var(aarch64_arch_string)
 --march=ARCH   Use features of architecture ARCH.
 +Use features of architecture ARCH.
  
  mcpu=
  Target RejectNegative ToLower Joined Var(aarch64_cpu_string)
 

Re: [PATCH] Make GO string literals properly NUL terminated

[Bernd Edlinger]

On 08/24/18 12:52, Richard Biener wrote:
> On Wed, Aug 22, 2018 at 6:57 AM Bernd Edlinger
>  wrote:
>>
>> On 08/21/18 10:33, Richard Biener wrote:
>>> On Mon, 20 Aug 2018, Bernd Edlinger wrote:
>>>
 On 08/20/18 15:19, Richard Biener wrote:
> On Mon, 20 Aug 2018, Bernd Edlinger wrote:
>
>> On 08/20/18 13:01, Richard Biener wrote:
>>> On Wed, Aug 1, 2018 at 3:05 PM Bernd Edlinger 
>>>  wrote:



 On 08/01/18 11:29, Richard Biener wrote:
>
> Hmm.  I think it would be nice if TREE_STRING_LENGTH would
> match char[2] and TYPE_SIZE_UNIT even if that is inconvenient
> for your check above.  Because the '\0' doesn't belong to the
> string.  Then build_string internally appends a '\0' outside
> of TREE_STRING_LENGTH.
>

 Hmm. Yes, but the outside-0 byte is just one byte, not a wide
 character.
>>>
>>> That could be fixed though (a wide 0 is just N 0s).  Add a elsz = 1
>>> parameter to build_string and allocate as many extra 0s as needed.
>>>
>>>   There are STRING_CSTs which are not string literals,
 for instance attribute tags, Pragmas, asm constrants, etc.
 They use the '\0' outside, and have probably no TREE_TYPE.

>
>> So I would like to be able to assume that the STRING_CST objects
>> are internally always generated properly by the front end.
>
> Yeah, I guess we need to define what "properly" is ;)
>
 Yes.

>> And that the ARRAY_TYPE of the string literal either has the
>> same length than the TREE_STRING_LENGTH or if it is shorter,
>> this is always exactly one (wide) character size less than 
>> TREE_STRING_LENGTH
>
> I think it should be always the same...
>

 One could not differentiate between "\0" without zero-termination
 and "" with zero-termination, theoretically.
>>>
>>> Is that important?  Doesn't the C standard say how to parse string 
>>> literals?
>>>
 We also have char x[100] = "ab";
 that is TREE_STRING_LENGTH=3, and TYPE_SIZE_UNIT(TREE_TYPE(x)) = 100.
 Of course one could create it with a TREE_STRING_LENGTH = 100,
 but imagine char x[1000] = "ab"
>>>
>>> The question is more about TYPE_SIZE_UNIT (TREE_TYPE ("ab")) which I
>>> hope matches "ab" and not 'x'.  If it matches 'x' then I'd rather have 
>>> it
>>> unconditionally be [], thus incomplete (because the literals "size" 
>>> depends
>>> on the context/LHS it is used on).
>>>
>>
>> Sorry, but I must say, it is not at all like that.
>>
>> If I compile x.c:
>> const char x[100] = "ab";
>>
>> and set a breakpoint at output_constant:
>>
>> Breakpoint 1, output_constant (exp=0x76ff9dc8, size=100, align=256,
>> reverse=false) at ../../gcc-trunk/gcc/varasm.c:4821
>> 4821 if (size == 0 || flag_syntax_only)
>> (gdb) p size
>> $1 = 100
>> (gdb) call debug(exp)
>> "ab"
>> (gdb) p *exp
>> $2 = {base = {code = STRING_CST, side_effects_flag = 0, constant_flag = 
>> 1,
>> (gdb) p exp->typed.type->type_common.size_unit
>> $5 = (tree) 0x76ff9d80
>> (gdb) call debug(exp->typed.type->type_common.size_unit)
>> 100
>> (gdb) p exp->string.length
>> $6 = 3
>> (gdb) p exp->string.str[0]
>> $8 = 97 'a'
>> (gdb) p exp->string.str[1]
>> $9 = 98 'b'
>> (gdb) p exp->string.str[2]
>> $10 = 0 '\000'
>> (gdb) p exp->string.str[3]
>> $11 = 0 '\000'
>>
>>
>> This is an important property of string_cst objects, that is used in 
>> c_strlen:
>>
>> It folds c_strlen([4]) directly to 0, because every byte beyond 
>> TREE_STRING_LENGTH
>> is guaranteed to be zero up to the type size.
>>
>> I would not have spent one thought on implementing an optimization like 
>> that,
>> but that's how it is right now.
>
> Huh.  So somebody interpreted STRING_CSTs similar to CONSTRUCTORs aka
> they have zero-padding up to its type size.  I don't see this documented
> anywhere and it would suggest to "optimize" "ab\0\0\0\0" to "ab\0"
> with appropriate TYPE_SIZE.
>
> This is also a relatively new thing on trunk (coming out of the added
> mem_size parameter of string_constant).  That this looks at the STRING_CST
> type like
>
>  if (TREE_CODE (array) == STRING_CST)
>{
>  *ptr_offset = fold_convert (sizetype, offset);
>  if (mem_size)
>*mem_size = TYPE_SIZE_UNIT (TREE_TYPE (array));
>  return array;
>
> I'd call simply a bug.  As said, interpretation of STRING_CSTs should
> depend on the context.  And for
>

 This use of the TYPE_SIZE_UNIT was pre-existing 

Re: [PATCH] Add -Wabsolute-value

[Joseph Myers]

On Fri, 24 Aug 2018, Martin Jambor wrote:

> +/* Assuming we have encountered a call to a probably wrong kind of abs, 
> issue a
> +   warning.  LOC is the location of the call, FNKIND is a string 
> characterizing
> +   the class of the used abs function, FNDEC is the actual function 
> declaration
> +   and ATYPE is type of the supplied actual argument.  */

For proper i18n, you have to use an enumeration here, not English string 
fragments.

> +  warning_at (loc, OPT_Wabsolute_value,
> +   "using %s absolute value function %qD when argument "
> +   "is of %s type %qT", fnkind, fndecl, act, atype);

And then have all the possible combinations as complete sentences, in 
separate warning_at calls in appropriate switch statments or marked up 
with G_() if you put more than one in a single warning_at call using ?:, 
for translation purposes.  (Any cases that are impossible combinations for 
the warning - you don't want translators to have to produce useless 
translations where e.g. both argument and call are of the same kind and so 
the warning shouldn't occur - should use gcc_unreachable ().)

> +case BUILT_IN_FABS:
> +case BUILT_IN_FABSF:
> +case BUILT_IN_FABSL:
> +  if (!SCALAR_FLOAT_TYPE_P (atype)
> +   || DECIMAL_FLOAT_MODE_P (TYPE_MODE (atype)))

Should include the _FloatN / _FloatNx built-in functions here (CASE_FLT_FN 
together with CASE_FLT_FN_FLOATN_NX).

> +case BUILT_IN_CABS:
> +case BUILT_IN_CABSF:
> +case BUILT_IN_CABSL:

And use CASE_FLT_FN here rather than hardcoding the list (but we don't yet 
have _FloatN / _FloatNx built-in variants of cabs).

> +  tree ftype = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
> +  if (tree_fits_uhwi_p (TYPE_SIZE (atype))
> +  && tree_to_uhwi (TYPE_SIZE (atype)) > tree_to_uhwi (TYPE_SIZE (ftype)))
> +warning_at (loc, OPT_Wabsolute_value,
> + "absolute value function %qD given an argument of type %qT "
> + "but has parameter of type %qT which may cause truncation "
> + "of value ", fndecl, atype, ftype);

Should not have space at end of warning text.  I don't think TYPE_SIZE is 
the right thing to use in general; for example, on x86_64, you should warn 
for passing a _Float128 value to fabsl, but both long double and _Float128 
are 16-byte types (with only 10 bytes non-padding in long double).

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH][0/4][RFC] RPO style value-numbering

[Richard Biener]

On Wed, 1 Aug 2018, Richard Biener wrote:

> 
> This rewrites the value-numbering algorithm used for FRE and PRE from
> SSA SCC based to RPO based, thus switching from an algorithm that
> handles SSA SCCs optimistically to one that handles CFG SCCs 
> optimistically.
> 
> The main motivation for this besides being more optimistic was that
> adding CFG context sensitive info is easier in RPO style.  Also
> tracking availability and thus making expression simplification not
> based on values like with SCCVN is possible which allows us to remove
> all the warts that scrap side-info we store on SSA names.  It also
> fixes PR86554 which is another manifestation of the same issue.
> 
> Another motivation was that we're in the need of applying value-numbering
> on regions like when unrolling loops or as part of cleanup on code
> generated by other passes like the vectorizer.  Thus this rewrite
> makes sure that the value-numbering works efficiently on regions
> (though in a non-iterative mode), avoiding work and space that is
> on the order of the function size rather than the region size to work on.
> Sofar the GIMPLE unroller makes use of this, scrapping its own
> simple constant propagation engine.  I expect that DOM could get rid of
> its value-numbering and instead use a non-iterative RPO-VN run as well.
> 
> The patch adds something called predication but it just implements
> what I put on top of SCCVN to not regress in that area.
> 
> With more optimistic handling comes compile-time regressions and
> without limiting I can observe for example a 8% compile-time regression
> on 416.gamess which contains loop depths exceeding 8.  The patch now
> contains heuristics to selectively value-number backedges optimistically
> or not and chooses to do so for the innermost 3 and the outermost loop
> of a nest (controlled by --param rpo-vn-max-loop-depth).  I have not
> yet played with other values of the param nor re-measured compile-time
> for SPEC 2k6.
> 
> I've bootstrapped and tested the series on x86_64-unknown-linux-gnu
> with bootstrap-O1 and regular bootstrap.
> 
> I plan to go forward with this for GCC 9.

This.  I'm momentarily installing 1/4, will re-post 4/4 and install
that on Monday.

Richard.

Re: [PATCH RFC] add generic expansion for MULT_HIGHPART_EXP

[Pekka Jääskeläinen]

Hi,

On Mon, Aug 20, 2018 at 1:46 PM Alexander Monakov  wrote:
> > So - how difficult is it to fix BRIG to not use MULT_HIGHPART_EXPR if
> > not supported?
>
> Pekka, can you comment? I think you have fallback paths for vector types
> only at the moment?

I think it involves pretty much moving the code of your patch to the
BRIG frontend.
To me it'd look a bit wrong in terms of "division of responsibilities"
as this is not really
frontend-specific as far as I understand (even if BRIG/HSAIL happened
to be the only language
supporting them currently).

> Does BRIG have mult-highpart for 64-bit integers? On 32-bit targets we
> won't be able to easily expand them (but on 64-bit targets it is fine).

Yes it does. 32b targets are not high priority for BRIG FE at this
point, so I wouldn't
worry about this as we assume HSA's "large" model is used, so this is likely not
the only problem when trying to generate for 32b machines.

> For scalar types I think we should prefer to implement a generic expansion
> rather than have the frontend query the backend. For vector types I am not
> sure.

In my relatively gcc-uneducated humble opinion these both belong more
naturally to a
target-specific expansion or "legalization" pass, which tries to
convert tree nodes to what the target
can support.

BR,
Pekka

Re: [PATCH] Add -Wabsolute-value

[Martin Jambor]

Hi

On Wed, Aug 15 2018, Eric Gallager wrote:
> On 8/14/18, Joseph Myers  wrote:
>> On Tue, 14 Aug 2018, Martin Jambor wrote:
>>
>>> when you try compiling a call to function abs and provide an unsigned
>>> int in the argument in C++, you will get an error about ambiguous
>>> overload.  In C however, it will pass without silently.  The following
>>> patch adds a warning for these cases, because I think it is likely that
>>> such code does not do what the author intended.
>>
>> abs of unsigned short (promoted to int) seems harmless; I don't see any
>> tests to make sure it doesn't warn.  Really the issue seems more like abs
>> (or labs / llabs / imaxabs) of an argument whose value might be changed by
>> the conversion to int.

That actually wasn't my motivation.  I believe that when someone
computes absolute value of something, they expect that the something can
have negative values, which however is not true if its type is unsigned
and so they might want to go and re-check the code and/or data
structures.  Therefore, I'd prefer to warn in all such cases.

>>> + else if (DECL_FUNCTION_CODE (expr.value) == BUILT_IN_ABS
>>
>> This looks like it would only handle abs, not labs / llabs / imaxabs.

Right, I originally only planned to send the patch only as an RFC and
forgot to add them when I made it more complete.  Sorry.

>>
>>> +@code{<} or @code{>=}.  When compiling C, also warn when calculating
>>> +an absolute value from an unsigned type.  This warning is also enabled
>>
>> But this would suggest any absolute value function, not just abs.
>
> clang has a -Wabsolute-value warning flag that might be looking at
> here for comparison; see bug 63886 for an example:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63886

Interesting.  If that's the case, maybe we can have an extra option too.
The following patch adds it, with most of Clang's functionality, the
missing bits (effectively passing a pointer to abs) are already errors
or dealt with -Wint-conversion which is in -Wall.  It:

1) warns if an unsigned integer type is passed,

2) divides abs functions into integer, floating-point, complex and
   decimal-floating-point classes and warns if a wrong one is selected,
   and

3) warns if an absolute value function is passed a bigger actual
   argument than its formal parameter, e.g. if abs() is used where
   labs() seems the appropriate choice

The 2) and 3) functionality overlaps quite a bit with
-Wfloat-conversion, but they also warn on situations not covered there
(abs() instead of labs() is probably the best example).  I can remove
those bits if it was deemed a problem, but I thought that symmetrical
behavior is better, especially because -Wfloat-conversion is not even in
-Wextra.

The warning is part of -Wextra.  Like before, all warnings can be
suppressed with an explicit type cast.  Therefore I still warn early,
not attempting any use of VRP as suggested by Martin Sebor.

What do you think, is this potentially useful?  Should I remove some
bits?  Any other suggestion/comments?  For the record, the patch passes
bootstrap and testing on x86_64-linux.

Thanks,

Martin


2018-08-23  Martin Jambor  

gcc/
* common.opt (Wabsolute-value): New.
* doc/invoke.texi (Warning Options): Likewise.

gcc/c/
* c/c-parser.c (construct_wrong_abs_kind_warning): New function.
(warn_for_abs): Likewise.
(c_parser_postfix_expression_after_primary): Call it.

testsuite/
* gcc.dg/warn-abs-1.c: New test.
* gcc.dg/dfp/warn-abs-2.c: Likewise.
---
 gcc/c/c-parser.c  | 132 --
 gcc/common.opt|   4 +
 gcc/doc/invoke.texi   |   8 ++
 gcc/testsuite/gcc.dg/dfp/warn-abs-2.c |  21 
 gcc/testsuite/gcc.dg/warn-abs-1.c |  66 +
 5 files changed, 225 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/dfp/warn-abs-2.c
 create mode 100644 gcc/testsuite/gcc.dg/warn-abs-1.c

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 5ad4f57a4fe..46adace69c5 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -9104,6 +9104,121 @@ sizeof_ptr_memacc_comptypes (tree type1, tree type2)
   return comptypes (type1, type2) == 1;
 }
 
+/* Assuming we have encountered a call to a probably wrong kind of abs, issue a
+   warning.  LOC is the location of the call, FNKIND is a string characterizing
+   the class of the used abs function, FNDEC is the actual function declaration
+   and ATYPE is type of the supplied actual argument.  */
+
+static void
+construct_wrong_abs_kind_warning (location_t loc, const char *fnkind,
+ tree fndecl, tree atype)
+{
+  const char *act;
+
+  if (INTEGRAL_TYPE_P (atype))
+act = "integer";
+  else if (SCALAR_FLOAT_TYPE_P (atype))
+{
+  if (DECIMAL_FLOAT_MODE_P (TYPE_MODE (atype)))
+   act = "decimal floating point";
+  else
+   act = "floating point";
+}
+  else if 

Re: [PATCH][debug] Add -gdescriptive-dies

[Richard Biener]

On Wed, 22 Aug 2018, Tom de Vries wrote:

> [ was: Re: [PATCH][debug] Add -gforce-named-dies ]
> 
> On 08/22/2018 11:46 AM, Tom de Vries wrote:
> > On 08/22/2018 08:56 AM, Tom de Vries wrote:
> >> This is an undocumented developer-only option, because using this option 
> >> may
> >> change behaviour of dwarf consumers, f.i., gdb shows the artificial 
> >> variables:
> >> ...
> >> (gdb) info locals
> >> a = 0x7fffda90 "\005"
> >> D.4278 = 
> >> ...
> > I just found in the dwarf 5 spec the attribute DW_AT_description
> > (present since version 3):
> > ...
> > 2.20 Entity Descriptions
> > Some debugging information entries may describe entities in the program
> > that are artificial, or which otherwise have a “name” that is not a
> > valid identifier in the programming language.
> > 
> > This attribute provides a means for the producer to indicate the purpose
> > or usage of the containing debugging infor
> > 
> > Generally, any debugging information entry that has, or may have, a
> > DW_AT_name attribute, may also have a DW_AT_description attribute whose
> > value is a null-terminated string providing a description of the entity.
> > 
> > It is expected that a debugger will display these descriptions as part
> > of displaying other properties of an entity.
> > ...
> > 
> > AFAICT, gdb currently does not explicitly handle this attribute, which I
> > think means it's ignored.
> > 
> > It seems applicable to use DW_AT_description at least for the artificial
> > decls.
> > 
> > Perhaps even for all cases that are added by the patch?
> > 
> 
> I've chosen for this option. Using DW_AT_desciption instead of
> DW_AT_name should minimize difference in gdb behaviour with and without
> -gdescriptive-dies.

-gdescribe-dies?

> > I'll rewrite the patch.
> 
> OK for trunk?

Few comments:

+static void
+add_desc_attribute (dw_die_ref die, tree decl)
+{
+  tree decl_name;
+
+  if (!flag_descriptive_dies || dwarf_version < 3)
+return;
+

you said this is a DWARF5 "feature", I'd suggest changing the
check to

  if (!flag_desctiprive_dies || (dwarf_version < 5 && dwarf_strict))

given -gdescribe-dies is enough of a user request to enable the
feature.  Not sure if we should warn about -gstrict-dwarf
combination with it and -gdwarf-N < 5.

+  else if (TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) == 
CONST_DECL)
+{
+  char buf[32];
+  char decl_letter = TREE_CODE (decl) == CONST_DECL ? 'C' : 'D';
+  sprintf (buf, "%c.%u", decl_letter, DECL_UID (decl));
+  add_desc_attribute (die, buf);
+}

I wondered before if we can use pretty-printing of decl here.  At
least I wouldn't restrict it to VAR_DECLs, FUNCTION_DECLs can
certainly appear here I think.

+@item -gdescriptive-dies
+@opindex gdescriptive-dies
+Add description attribute to DWARF DIEs that have no name attribute.
+

Either "description attributes" or "a description attribute", not
100% sure being a non-native speaker.

Otherwise the patch looks OK to me but please leave Jason time
to comment.

Richard.

[PATCH] (PR86989)

[Segher Boessenkool]

There currently is nothing that prevents replacing the TOC_REGISTER in
a TOCREL unspec with something else, like a pseudo, or a memory ref.
This of course does not work.  Fix that.

Tested on powerpc64-linux {-m32,-m64}; committing.


Segher


2018-08-24  Segher Boessenkool  

PR target/86989
* config/rs6000/rs6000.c (toc_relative_expr_p): Check that the base is
the TOC register.

---
 gcc/config/rs6000/rs6000.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index a967912..ed33912 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -7932,7 +7932,9 @@ toc_relative_expr_p (const_rtx op, bool strict, const_rtx 
*tocrel_base_ret,
 *tocrel_offset_ret = tocrel_offset;
 
   return (GET_CODE (tocrel_base) == UNSPEC
- && XINT (tocrel_base, 1) == UNSPEC_TOCREL);
+ && XINT (tocrel_base, 1) == UNSPEC_TOCREL
+ && REG_P (XVECEXP (tocrel_base, 0, 1))
+ && REGNO (XVECEXP (tocrel_base, 0, 1)) == TOC_REGISTER);
 }
 
 /* Return true if X is a constant pool address, and also for cmodel=medium
-- 
1.8.3.1

Re: lightweight class to wide int ranges in VRP and friends

[Richard Biener]

On Wed, Aug 15, 2018 at 7:31 PM Aldy Hernandez  wrote:
>
> I kept seeing the same patterns over and over in all this re-factoring:
>
> 1. extract value_range constants into pairs of wide ints
>
> 2. mimic symbolics as [-MIN, +MAX] (most of the time :))
>
> 3. perform some wide int operation on the wide int range
>
> 4. convert back to a value range
>
> So I've decided to clean this up even more.  Instead of passing a pair
> of wide ints plus sign, precision, and god knows what to every
> wide_int_range_* function, I've implemented a lighweight class
> (wi_range) for a pair of wide ints.  It is not meant for long term
> storage, but even so its footprint is minimal.
>
> The only extra bits are another 64-bit word per pair for keeping
> attributes such as precision, sign, overflow_wraps, and a few other
> things we'll need shortly.  In reality we only need one set of
> attributes for both sets of pairs, so we waste one 64-bit word.  I don't
> care.  They're short lived, and the resulting code looks an awful lot
> nicer.  For example, the shift code gets rewritten from this:
>
>if (range_int_cst_p ()
>   && !vrp_shift_undefined_p (vr1, prec))
> {
>   if (code == RSHIFT_EXPR)
> {
>   if (vr0.type != VR_RANGE || symbolic_range_p ())
> {
>   vr0.type = type = VR_RANGE;
>   vr0.min = vrp_val_min (expr_type);
>   vr0.max = vrp_val_max (expr_type);
> }
>   extract_range_from_multiplicative_op (vr, code, , );
>   return;
> }
>   else if (code == LSHIFT_EXPR
>&& range_int_cst_p ())
> {
>   wide_int res_lb, res_ub;
>   if (wide_int_range_lshift (res_lb, res_ub, sign, prec,
>  wi::to_wide (vr0.min),
>  wi::to_wide (vr0.max),
>  wi::to_wide (vr1.min),
>  wi::to_wide (vr1.max),
>  TYPE_OVERFLOW_UNDEFINED (expr_type),
>  TYPE_OVERFLOW_WRAPS (expr_type)))
> {
>   min = wide_int_to_tree (expr_type, res_lb);
>   max = wide_int_to_tree (expr_type, res_ub);
>   set_and_canonicalize_value_range (vr, VR_RANGE,
> min, max, NULL);
>   return;
> }
> }
> }
>set_value_range_to_varying (vr);
>
> into this:
>
>wi_range w0 (, expr_type);
>wi_range w1 (, expr_type);
>if (!range_int_cst_p ()
>   || wi_range_shift_undefined_p (w1)
>   || (code == LSHIFT_EXPR
>   && !range_int_cst_p ()))
> {
>   vr->set_varying ();
>   return;
> }
>bool success;
>if (code == RSHIFT_EXPR)
> success = wi_range_multiplicative_op (res, code, w0, w1);
>else
> success = wi_range_lshift (res, w0, w1);
>
>if (success)
> vr->set_and_canonicalize (res, expr_type);
>else
> vr->set_varying ();
>return;

not sure whether I like the munging of lshift and right shift (what exactly gets
done is less clear in your version ...).  Having a light-weigth class for
the wi_range_ parameters is nice though.

> I also munged together the maybe/mustbe nonzero_bits into one structure.
>
> Finally, I've pontificated that wide_int_range_blah is too much typing.
> Everyone's RSI will thank me for rewriting the methods as wi_range_blah.
>
> I've tried to keep the functionality changes to a minimum.  However,
> there are some slight changes where I treat symbolics and VR_VARYING as
> [MIN, MAX] and let the constant code do its thing.  It is considerably
> easier to read.
>
> I am finally happy with the overall direction of this.  If this and the
> last one are acceptable, I think I only need to clean MIN/MAX_EXPR and
> ABS_EXPR and I'm basically done with what I need going forward.
>
> Phew...
>
> How does this look?

+struct wi_range
+{
+  wide_int min;
+  wide_int max;
+  /* This structure takes one additional 64-bit word apart from the
+ min/max bounds above.  It is laid out so that PRECISION and SIGN
+ can be accessed without any bit twiddling, since they're the most
+ commonly accessed fields.  */
+  unsigned short precision;
+  bool empty_p:1;
+  bool pointer_type_p:1;
+  bool overflow_wraps:1;
+  bool overflow_undefined:1;
+  signop sign;

isn't precision already available in min.get_precision () which is
required to be equal to max.get_precision ()?

+  wi_range () { empty_p = false; }

true I suppose?  Better not allow default construction?

empty_p doesn't seem to be documented, nor is pointer_type_p
or why that is necessary - maybe simply store a tree type
instead 

Re: [PATCH 0/6] improve handling of char arrays with missing nul (PR 86552, 86711, 86714)

[Richard Biener]

On Wed, Aug 15, 2018 at 5:42 PM Jeff Law  wrote:
>
> On 08/15/2018 08:47 AM, Martin Sebor wrote:
> > On 08/15/2018 12:02 AM, Jeff Law wrote:
> >> On 08/13/2018 03:23 PM, Martin Sebor wrote:
> >>> To make reviewing the changes easier I've split up the patch
> >>> into a series:
> >> [ ... ]
> >> I'm about done for the night and thus won't get into the series (and as
> >> you know Bernd has a competing patch in this space).  But I did want to
> >> chime in on two things...
> >>
> >>>
> >>> There are many more string functions where unterminated (constant
> >>> or otherwise) should be diagnosed.  I plan to continue to work on
> >>> those (with the constant ones first)  but I want to post this
> >>> updated patch for review now, mainly so that the wrong code bug
> >>> (PR 86711) can be resolved and the basic detection infrastructure
> >>> agreed on.
> >> Yes, I think we definitely want to focus on the wrong code bug first.
> >>
> >>>
> >>> An open question in my mind is what should GCC do with such calls
> >>> after issuing a warning: replace them with traps?  Fold them into
> >>> constants?  Or continue to pass them through to the corresponding
> >>> library functions?
> >> My personal preference is to turn them into traps.  I don't think we
> >> have to preserve the call itself in this case.   I think the sequencing
> >> is to insert the trap before the call point, split the block after the
> >> trap, remove the outgoing edges, let DCE clean up the rest.  At least I
> >> think that's the sequencing.
> >
> > That sounds fine to me.  It would be close in its effects to
> > what _FORTIFY_SOURCE does.
> The bad guys are exceedingly resourceful in how they exploit undefined
> behavior.  By trapping immediately they don't have any window to do
> anything nefarious.
>
> >
> > It would be helpful to get a broader consensus on this and start
> > adopting the same consistent solution in all contexts.  The question
> > has come up a few times, most recently also in PR 86519 (folding
> > memcmp(a, "a", 3)) where GCC ends up calling the library function.
> Yup.  We've got a mish-mash of strategies here.

Folding cannot easily make sth "regular" as memcmp a noreturn thing.
At least not all callers expect that to happen.  So what you'd need to
do is ensure GF_CALL_CTRL_ALTERING is not set on the replacement
trap().  The next fixup_cfg () pass will fix things for you then.

> >
> > FWIW, if there are other preferences it might be worthwhile to
> > consider providing an option to control the behavior in these
> > cases.  There may also be interactions with or implications for
> > the sanitizers to consider.
> There's some (Marc Glisse IIRC) that would prefer to see the control
> path to the undefined behavior zapped entirely.  We didn't initially do
> that because the path my have other observable side effects.  However,
> there may be cases where it makes sense.

You can't remove observable side-effects and given that there exist
things like signal handlers for SIGSEGV even changing a memcmp
to __builtin_trap() may change observable behavior.

This is why some places in GCC simply refuse to optimize "broken"
cases but keep calling the library.

Richard.

> >
> > Once there is agreement on what the solution should be I can look
> > into implementing it at some point in the future.
> ACK.  Certainly lower priority than the stuff in flight right now.
>
> jeff

Re: [PATCH] treat -Wxxx-larger-than=HWI_MAX special (PR 86631)

[Richard Biener]

On Fri, Aug 24, 2018 at 12:35 AM Martin Sebor  wrote:
>
> On 08/23/2018 07:18 AM, Richard Biener wrote:
> > On Thu, Aug 23, 2018 at 12:20 AM Martin Sebor  wrote:
> >>
> >> On 08/20/2018 06:14 AM, Richard Biener wrote:
> >>> On Thu, Jul 26, 2018 at 10:52 PM Martin Sebor  wrote:
> 
>  On 07/26/2018 08:58 AM, Martin Sebor wrote:
> > On 07/26/2018 02:38 AM, Richard Biener wrote:
> >> On Wed, Jul 25, 2018 at 5:54 PM Martin Sebor  wrote:
> >>>
> >>> On 07/25/2018 08:57 AM, Jakub Jelinek wrote:
>  On Wed, Jul 25, 2018 at 08:54:13AM -0600, Martin Sebor wrote:
> > I don't mean for the special value to be used except internally
> > for the defaults.  Otherwise, users wanting to override the default
> > will choose a value other than it.  I'm happy to document it in
> > the .opt file for internal users though.
> >
> > -1 has the documented effect of disabling the warnings altogether
> > (-1 is SIZE_MAX) so while I agree that -1 looks better it doesn't
> > work.  (It would need more significant changes.)
> 
>  The variable is signed, so -1 is not SIZE_MAX.  Even if -1 disables
>  it, you
>  could use e.g. -2 or other negative value for the other special case.
> >>>
> >>> The -Wxxx-larger-than=N distinguish three ranges of argument
> >>> values (treated as unsigned):
> >>>
> >>>1.  [0, HOST_WIDE_INT_MAX)
> >>>2.  HOST_WIDE_INT_MAX
> >>>3.  [HOST_WIDE_INT_MAX + 1, Infinity)
> >>
> >> But it doesn't make sense for those to be host dependent.
> >
> > It isn't when the values are handled by each warning.  That's
> > also the point of this patch: to remove this (unintended)
> > dependency.
> >
> >> I think numerical user input should be limited to [0, ptrdiff_max]
> >> and cases (1) and (2) should be simply merged, I see no value
> >> in distinguishing them.  -Wxxx-larger-than should be aliased
> >> to [0, ptrdiff_max], case (3) is achieved by -Wno-xxx-larger-than.
> 
>  To be clear: this is also close to what this patch does.
> 
>  The only wrinkle is that we don't know the value of PTRDIFF_MAX
>  either at the time the option initial value is set in the .opt
>  file or when the option is processed when it's specified either
>  on the command line or as an alias in the .opt file (as all
>  -Wno-xxx-larger-than options are).
> >>>
> >>> But then why not make that special value accessible and handle
> >>> it as PTRDIFF_MAX when that is available (at users of the params)?
> >>>
> >>> That is,
> >>>
> >>> Index: gcc/calls.c
> >>> ===
> >>> --- gcc/calls.c (revision 262951)
> >>> +++ gcc/calls.c (working copy)
> >>> @@ -1222,9 +1222,12 @@ alloc_max_size (void)
> >>>if (alloc_object_size_limit)
> >>>  return alloc_object_size_limit;
> >>>
> >>> -  alloc_object_size_limit
> >>> -= build_int_cst (size_type_node, warn_alloc_size_limit);
> >>> +  HOST_WIDE_INT limit = warn_alloc_size_limit;
> >>> +  if (limit == HOST_WIDE_INT_MAX)
> >>> +limit = tree_to_shwi (TYPE_MAX_VALUE (ptrdiff_type_node));
> >>>
> >>> +  alloc_object_size_limit = build_int_cst (size_type_node, limit);
> >>> +
> >>>return alloc_object_size_limit;
> >>>  }
> >>>
> >>> use sth like
> >>>
> >>>  if (warn_alloc_size_limit == -1)
> >>>alloc_object_size_limit = fold_convert (size_type_node,
> >>> TYPE_MAX_VALUE (ptrdiff_type_node));
> >>>  else
> >>>alloc_object_size_limit = size_int (warn_alloc_size_limit);
> >>>
> >>> ?  Also removing the need to have > int params values.
> >>
> >> Not sure I understand this last part.  Remove the enhancement?
> >> (We do need to handle option arguments in excess of INT_MAX.)
> >
> > I see.
> >
> >>>
> >>> It's that HOST_WIDE_INT_MAX use that is problematic IMHO.  Why not use -1?
> >>
> >> -1 is a valid/documented value of the argument of all these
> >> options because it's treated as unsigned HWI:
> >>
> >>Warnings controlled by the option can be disabled either
> >>by specifying byte-size of ‘SIZE_MAX’
> >>
> >> It has an intuitive meaning: warning for any size larger than
> >> the maximum means not warning at all.  Treating -1 as special
> >> instead of HOST_WIDE_INT_MAX would replace that meaning with
> >> "warn on any size in excess of PTRDIFF_MAX."
> >>
> >> A reasonable way to disable the warning is like so:
> >>
> >>gcc -Walloc-size-larger-than=$(getconf ULONG_MAX) ...
> >>
> >> That would not work anymore.
> >>
> >> Treating HOST_WIDE_INT_MAX as PTRDIFF_MAX is the natural choice
> >> on LP64: they have the same value.  It's only less than perfectly
> >> natural in ILP32 and even there it's not a problem in practice
> >> because it's either far out of the range of valid values [0, 4GB]
> >> (i.e., where HWI is a 64-bit long long), or it's also equal to
> >> PTRDIFF_MAX (on hosts 

Re: [PATCH] print full STRING_CST in Gimple dumps (PR 87052)

[Richard Biener]

On Thu, Aug 23, 2018 at 5:13 PM Martin Sebor  wrote:
>
> On 08/23/2018 08:07 AM, Michael Matz wrote:
> > Hi,
> >
> > On Thu, 23 Aug 2018, Richard Biener wrote:
> >
>  Can you write a not \0 terminated string literal in C?
> >>>
> >>> Yes: char a[2] = "12";
> >>
> >> I thought they are fully defined in translation phase #1 ...
> >
> > No, you can't write a string literal which is not zero terminated, because
> > in translation phase 7 a zero code is appended to all character sequences
> > resulting from string literals, which is then used to allocate and
> > initialize a static (wide) character array of just the right size,
> > including the zero code.
> >
> > The above construct uses that static char[3] array from the string literal
> > to initialize a char[2] array (which is explicitely allowed), and _that_
> > one is not zero terminated.  But it's also no string literal.
>
> Yes, you're right.  I misinterpreted Richard's question as
> asking if one could construct an unterminated array of chars
> using a string literal.
>
> The distinction that I thought would be useful to capture in
> Gimple is between "12" that initializes an array of two elements
> (and where the nul doesn't fit) vs "12" that initializes one of
> three elements (and where the nul does fit).  It's probably not
> terribly important when the array type also appears in Gimple
> like in the case above but there are (perhaps corner) cases
> where it doesn't, as in the second one below:
>
>char a[2] = "12";
>char *p = (char[2]){ "12" };
>
> which ends up represented as:
>
>char a[2];
>char * p;
>char D.1910[2];
>
>try
>  {
>a = "12";
>D.1910 = "12";
>p = 
>  }

Yeah, we currently require useless_type_conversion_p between the
LHS type and the RHS type (type of a vs. type of "12") which implies
TYPE_SIZE matches (if constant).  That means GIMPLE expects
at least the type of "12" to not include the NUL.

Richard.

> Martin

Re: VRP: make range_includes_zero_p handle value_ranges

[Richard Biener]

On Thu, Aug 23, 2018 at 4:50 PM Aldy Hernandez  wrote:
>
>
>
> On 08/23/2018 05:59 AM, Richard Biener wrote:
> > On Wed, Aug 22, 2018 at 11:31 AM Aldy Hernandez  wrote:
> >>
> >>
> >>
> >> On 08/21/2018 05:46 AM, Richard Biener wrote:
> >>> On Wed, Aug 15, 2018 at 3:33 AM Aldy Hernandez  wrote:
> >>
>  Finally, my apologies for including a tiny change to the
>  POINTER_PLUS_EXPR handling code as well.  It came about the same set of
>  auditing tests.
> >>>
> >>> Bah, please split up things here ;)  I've done a related change there
> >>> yesterday...
> >>>
> 
>  It turns out we can handle POINTER_PLUS_EXPR(~[0,0], [X,Y]) without
>  bailing as VR_VARYING in extract_range_from_binary_expr_1.  In doing so,
>  I also noticed that ~[0,0] is not the only non-null.  We could also have
>  ~[0,2] and still know that the pointer is not zero.  I have adjusted
>  range_is_nonnull accordingly.
> >>>
> >>> But there are other consumers and it would have been better to
> >>> change range_includes_zero_p to do the natural thing (get a VR) and
> >>> then remove range_is_nonnull as redundant if possible.
> >>
> >> Indeed.  Cleaning up range_includes_zero_p makes VRP and friends a lot
> >> cleaner.  Thanks for the suggestion.
> >>
> >> I lazily avoided cleaning up the division code affected in this patch
> >> too much, since it's going to be superseded by my division changes in
> >> the other patch.
> >>
> >> OK pending tests?
> >
> > -/* Return 1 if [MIN, MAX] includes the value zero, 0 if it does not
> > -   include the value zero, -2 if we cannot tell.  */
> > +/* Return 1 if *VR includes the value zero, 0 if it does not include
> > +   the value zero, or -2 if we cannot tell.  */
> >
> >   int
> > -range_includes_zero_p (tree min, tree max)
> > +range_includes_zero_p (const value_range *vr)
> >   {
> > -  tree zero = build_int_cst (TREE_TYPE (min), 0);
> > -  return value_inside_range (zero, min, max);
> > +  if (vr->type == VR_UNDEFINED || vr->type == VR_VARYING)
> > +return -2;
> > +
> > +  tree zero = build_int_cst (TREE_TYPE (vr->min), 0);
> > +  if (vr->type == VR_RANGE)
> > +return value_inside_range (zero, vr->min, vr->max);
> > +  else
> > +return !value_inside_range (zero, vr->min, vr->max);
> >   }
> >
> > please make it return a bool.  VR_VARYING means the range does
> > include zero.  For VR_UNDEFINED we could say it doesn't or we choose
> > to not possibly optimize and thus return true (just do that for now).
> > That's because VR_VARYING is [-INF, INF] and VR_UNDEFINED is
> > an empty range.
>
> Done.
>
> >
> > I suppose the -2 for we cannot tell was for the case of symbolic
> > ranges where again we can conservatively return true
> > (and your return !value_inside_range for VR_ANTI_RANGE botched
> > the tri-state return value anyways...).
>
> Bah!  I sure botched up the anti range return.  That was the last
> bootstrap failure I was investigating.  You'd be amazed how many things
> mysteriously fail when things like ~[SYMBOL, SYMBOL] are assumed to be
> non-zero.
>
> Thanks for spotting this.  Bootstrap is happy now :-P.
>
> >
> > So, can you please rework that?
>
> Sure can!

@@ -1407,7 +1407,10 @@ extract_range_from_binary_expr_1 (value_range *vr,
   && code != POINTER_PLUS_EXPR
   && (vr0.type == VR_VARYING
  || vr1.type == VR_VARYING
- || vr0.type != vr1.type
+ || (vr0.type != vr1.type
+ /* We can handle POINTER_PLUS_EXPR(~[0,0], [x,y]) below,
+even though we have differing range kinds.  */
+ && code != POINTER_PLUS_EXPR)
  || symbolic_range_p ()
  || symbolic_range_p ()))
 {

is redundant now (spot the code != POINTER_PLUS_EXPR check at the
beginning of context)

OK with this hunk removed.

Thanks,
Richard.

> Attached.
>

Re: [PATCH] DWARF: Call set_indirect_string on DW_MACINFO_start_file

[Richard Biener]

On Thu, Aug 23, 2018 at 4:38 PM H.J. Lu  wrote:
>
> On Thu, Aug 23, 2018 at 5:56 AM, Richard Biener
>  wrote:
> > On Wed, Aug 22, 2018 at 9:36 PM H.J. Lu  wrote:
> >>
> >> Since -gsplit-dwarf -g3 will output filename as indirect string, call
> >> set_indirect_string on DW_MACINFO_start_file for -gsplit-dwarf -g3.
> >>
> >> OK for trunk?
> >
> > Can you add a testcase?
> >
> >
>
> Here is the updated patch with a testcase.  Before my fix:
>
> /export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/xgcc
> -B/export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/ -m32
> -O2 -fno-var-tracking-assignments -gsplit-dwarf -g3 -march=skylake
> -mrtm -mabm -S x.c
> x.c:14:1: internal compiler error: in output_index_string, at 
> dwarf2out.c:28780
> 14 | }
>| ^
> 0xb090e7 output_index_string(indirect_string_node**, unsigned int*)
> /export/gnu/import/git/sources/gcc/gcc/dwarf2out.c:28780
> 0xb09370 output_indirect_strings
> /export/gnu/import/git/sources/gcc/gcc/dwarf2out.c:28868
> 0xb0ed85 dwarf2out_finish
> /export/gnu/import/git/sources/gcc/gcc/dwarf2out.c:31563
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See  for instructions.
>
> After:
>
> /export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/xgcc
> -B/export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/ -m32
> -O2 -fno-var-tracking-assignments -gsplit-dwarf -g3 -march=skylake
> -mrtm -mabm -S x.c
>
> OK for trunk?

OK.

Richard.

> Thanks.
>
> --
> H.J.

[PR 87073] [committed] fix go bootstrap

[Aldy Hernandez]

I have no idea how this passed bootstrap and tests in other languages.

The problem here is that wide_int_binop is overflowing on TRUNC_DIV_EXPR 
and a range in Go.  This is causing us to use wmin/wmax uninitialized.


Serves me right for all my whining about Ada yesterday.  I think the VRP 
changes are sufficiently all encompassing that I will start testing 
--enable-languages=all instead of whatever our defaults are.  My bad.


Committed as obvious.

Aldy
gcc/

	PR 87073/bootstrap
	* wide-int-range.cc (wide_int_range_div): Do not ignore result
	from wide_int_range_multiplicative_op.

diff --git a/gcc/wide-int-range.cc b/gcc/wide-int-range.cc
index cbc71c25cfe..3cdcede04cd 100644
--- a/gcc/wide-int-range.cc
+++ b/gcc/wide-int-range.cc
@@ -687,14 +687,11 @@ wide_int_range_div (wide_int , wide_int ,
 
   /* If we know we won't divide by zero, just do the division.  */
   if (!wide_int_range_includes_zero_p (divisor_min, divisor_max, sign))
-{
-  wide_int_range_multiplicative_op (wmin, wmax, code, sign, prec,
-	dividend_min, dividend_max,
-	divisor_min, divisor_max,
-	overflow_undefined,
-	overflow_wraps);
-  return true;
-}
+return wide_int_range_multiplicative_op (wmin, wmax, code, sign, prec,
+	 dividend_min, dividend_max,
+	 divisor_min, divisor_max,
+	 overflow_undefined,
+	 overflow_wraps);
 
   /* If flag_non_call_exceptions, we must not eliminate a division
  by zero.  */

Re: PING [PATCH] warn for strlen of arrays with missing nul (PR 86552, 86711, 86714) )

[Jeff Law]

On 08/02/2018 07:26 AM, Bernd Edlinger wrote:
> On 08/02/18 04:44, Martin Sebor wrote:
>> Since the foundation of the patch is detecting and avoiding
>> the overly aggressive folding of unterminated char arrays,
>> besides issuing a warning for such arguments to strlen,
>> the patch also fixes pr86711 - wrong folding of memchr, and
>> pr86714 - tree-ssa-forwprop.c confused by too long initializer.
>>
>> The substance of the attached updated patch is unchanged,
>> I have just added test cases for the two additional bugs.
>>
>> Bernd, as I mentioned Wednesday, the patch supersedes
>> yours here:
>> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01800.html
>>
> 
> No problem, but I hope you understand, that I still uphold
> my patch.
> 
> So we have two patches now:
> - mine, fixing a wrong code bug,
> - yours, implementing a new warning and fixing a wrong
> code bug at the same time.
> 
> I will add a few comments to your patch below.
[ ... ]

So a lot of the comments are out of date, presumably because Martin
fixed the issues you pointed out in his second version of the patch.
But there's still some useful nuggets in your comments that are still
relevant.

FYI it appears that sometimes you comment above a chunk of code, and
other times below.  That makes it exceedingly difficult to figure out
the issue you're trying to raise.


> 
>> Martin
>>
>> On 07/30/2018 01:17 PM, Martin Sebor wrote:
>>> Attached is an updated version of the patch that handles more
>>> instances of calling strlen() on a constant array that is not
>>> a nul-terminated string.
>>>
>>> No other functions except strlen are explicitly handled yet,
>>> and neither are constant arrays with braced-initializer lists
>>> like const char a[] = { 'a', 'b', 'c' };  I am testing
>>> an independent solution for those (bug 86552).  Once those
>>> are handled the warning will be able to detect those as well.
>>>
>>> Tested on x86_64-linux.
>>>
>>> On 07/25/2018 05:38 PM, Martin Sebor wrote:
 Ping: https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01124.html

 The fix for bug 86532 has been checked in so this enhancement
 can now be applied on top of it (with only minor adjustments).

 On 07/19/2018 02:08 PM, Martin Sebor wrote:
> In the discussion of my patch for pr86532 Bernd noted that
> GCC silently accepts constant character arrays with no
> terminating nul as arguments to strlen (and other string
> functions).
>
> The attached patch is a first step in detecting these kinds
> of bugs in strlen calls by issuing -Wstringop-overflow.
> The next step is to modify all other handlers of built-in
> functions to detect the same problem (not part of this patch).
> Yet another step is to detect these problems in arguments
> initialized using the non-string form:
>
>   const char a[] = { 'a', 'b', 'c' };
>
> This patch is meant to apply on top of the one for bug 86532
> (I tested it with an earlier version of that patch so there
> is code in the context that does not appear in the latest
> version of the other diff).
>
> Martin
>

>>>
>>
>> PR tree-optimization/86714 - tree-ssa-forwprop.c confused by too long 
>> initializer
>> PR tree-optimization/86711 - wrong folding of memchr
>> PR tree-optimization/86552 - missing warning for reading past the end of 
>> non-string arrays
>>
>> gcc/ChangeLog:
>>
>>  PR tree-optimization/86714
>>  PR tree-optimization/86711
>>  PR tree-optimization/86552
>>  * builtins.h (warn_string_no_nul): Declare..
>>  (c_strlen): Add argument.
>>  * builtins.c (warn_string_no_nul): New function.
>>  (fold_builtin_strlen): Add argument.  Detect missing nul.
>>  (fold_builtin_1): Adjust.
>>  (string_length): Add argument and use it.
>>  (c_strlen): Same.
>>  (expand_builtin_strlen): Detect missing nul.
>>  * expr.c (string_constant): Add arguments.  Detect missing nul
>>  terminator and outermost declaration it's missing in.
>>  * expr.h (string_constant): Add argument.
>>  * fold-const.c (c_getstr): Change argument to bool*, rename
>>  other arguments.
>>  * fold-const-call.c (fold_const_call): Detect missing nul.
>>  * gimple-fold.c (get_range_strlen): Add argument.
>>  (get_maxval_strlen): Adjust.
>>  * gimple-fold.h (get_range_strlen): Add argument.
>>
>> gcc/testsuite/ChangeLog:
>>
>>  PR tree-optimization/86714
>>  PR tree-optimization/86711
>>  PR tree-optimization/86552
>>  * gcc.c-torture/execute/memchr-1.c: New test.
>>  * gcc.c-torture/execute/pr86714.c: New test.
>>  * gcc.dg/warn-strlen-no-nul.c: New test.
>>
>> diff --git a/gcc/builtins.c b/gcc/builtins.c
>> index aa3e0d8..f4924d5 100644
>> --- a/gcc/builtins.c
>> +++ b/gcc/builtins.c
>> @@ -150,7 +150,7 @@ static tree stabilize_va_list_loc (location_t, tree, 
>> int);
>>  static rtx expand_builtin_expect (tree, rtx);
>>  static tree fold_builtin_constant_p