Re: [PATCH] avoid false positives due to compute_objsize (PR 95353)

[Martin Sebor via Gcc-patches]

On 6/13/20 3:50 PM, Sandra Loosemore wrote:

On 6/2/20 6:12 PM, Martin Sebor via Gcc-patches wrote:

The compute_objsize() function started out as a thin wrapper around
compute_builtin_object_size(), but over time developed its own
features to compensate for the other function's limitations (such
as its inability to work with ranges).  The interaction of these
features and the limitations has started to become increasingly
problematic as the former function is used in more contexts.

A complete "fix" for all the problems (as well as some other
limitations) that I'm working on will be more extensive and won't
be appropriate for backports.  Until then, the attached patch
cleans up the extensions compute_objsize() has accumulated over
the years to avoid a class of false positives.

To make the warnings issued based on the results of the function
easier to understand and fix, the patch also adds an informative
message to many instances of -Wstringop-overflow to point to
the object to which the warning refers.  This is especially
helpful when the object is referenced by a series of pointer
operations.

Tested by boostrapping on x86_64-linux and building Binutils/GDB,
Glibc, and the Linux kernel with no new warnings.

Besides applying it to trunk I'm looking to backport the fix to
GCC 10.


This patch (commit a2c2cee92e5defff9bf23d3b1184ee96e57e5fdd) has broken 
glibc builds on nios2-linux-gnu, when building sysdeps/posix/getaddrinfo.c:


../sysdeps/posix/getaddrinfo.c: In function 'gaih_inet.constprop':
../sysdeps/posix/getaddrinfo.c:1081:3: error: 'memcpy' writing 16 bytes 
into a region of size 8 overflows the destination 
[-Werror=stringop-overflow=]

  1081 |   memcpy (>sin6_addr,
   |   ^~
  1082 |    at2->addr, sizeof (struct in6_addr));
   |    
In file included from ../include/netinet/in.h:3,
  from ../resolv/bits/types/res_state.h:5,
  from ../include/bits/types/res_state.h:1,
  from ../nptl/descr.h:35,
  from ../sysdeps/nios2/nptl/tls.h:45,
  from ../sysdeps/generic/libc-tsd.h:44,
  from ../include/../locale/localeinfo.h:224,
  from ../include/ctype.h:26,
  from ../sysdeps/posix/getaddrinfo.c:57:
../inet/netinet/in.h:249:19: note: destination object 'sin_zero'
   249 | unsigned char sin_zero[sizeof (struct sockaddr)
   |   ^~~~


I have to say that I don't understand the "note" diagnostic here at all. 
  :-(  Why does it think the destination object is a field of struct 
sockaddr_in, while this memcpy is filling in a field of struct 
sockaddr_in6?  (And, the sin6_addr field is indeed of type struct 
in6_addr, matching the sizeof expression.)


Most likely because some earlier pass (from my exchange with Jeff
about this instance of the warning I suspect it's PRE) substitutes
one member for the other in the IL when offsets into them happen
to evaluate to the same offset from the start of the enclosing
object.  The Glibc code does this:

struct sockaddr_in6 *sin6p =
  (struct sockaddr_in6 *) ai->ai_addr;

sin6p->sin6_port = st2->port;
sin6p->sin6_flowinfo = 0;
memcpy (>sin6_addr,
at2->addr, sizeof (struct in6_addr));

and the warning doesn't see sin6p->sin6_addr as the destination
but something like

[(void *)ai_10 + 4B].sin_zero;

The details in this and all other middle end warnings are only as
reliable as the IL they work with.  If the IL that doesn't correspond
to the original source code they're going to be confusing (and may
trigger false positives).

Instead of substituting one member for another in the COMPONENT_REF
when both happen to be accessed at the same offset, using a MEM_REF
alone into the enclosing struct or union plus the offset of
the members would avoid the problem.  Something like this:

[(void *)ai_10 + 4B + 8B];

(or whatever the final offset of the member is).

As for the warning itself, GCC tries to avoid warning for calls
to memcpy that write into multiple members at the same time, up
to the size of the complete object.  This is done because
the Linux kernel does these things in a few places.  As dangerous
as the practice is, the change I committed tries to preserve this
special exception for now (in the future I'd like to add a new
warning option to control it).  As I mentioned above I tested
the change with the kernel (as well as Glibc) and didn't see any
new warnings but it looks like this one slipped through (funny
that with all this testing it only triggers on such an uncommon
target).  I opened 95667 to fix it.  (The warning will most likely
come back once using memcpy to write across multiple members starts
getting diagnosed.)

Martin

Re: [PATCH] avoid false positives due to compute_objsize (PR 95353)

[Sandra Loosemore]

On 6/2/20 6:12 PM, Martin Sebor via Gcc-patches wrote:

The compute_objsize() function started out as a thin wrapper around
compute_builtin_object_size(), but over time developed its own
features to compensate for the other function's limitations (such
as its inability to work with ranges).  The interaction of these
features and the limitations has started to become increasingly
problematic as the former function is used in more contexts.

A complete "fix" for all the problems (as well as some other
limitations) that I'm working on will be more extensive and won't
be appropriate for backports.  Until then, the attached patch
cleans up the extensions compute_objsize() has accumulated over
the years to avoid a class of false positives.

To make the warnings issued based on the results of the function
easier to understand and fix, the patch also adds an informative
message to many instances of -Wstringop-overflow to point to
the object to which the warning refers.  This is especially
helpful when the object is referenced by a series of pointer
operations.

Tested by boostrapping on x86_64-linux and building Binutils/GDB,
Glibc, and the Linux kernel with no new warnings.

Besides applying it to trunk I'm looking to backport the fix to
GCC 10.


This patch (commit a2c2cee92e5defff9bf23d3b1184ee96e57e5fdd) has broken 
glibc builds on nios2-linux-gnu, when building sysdeps/posix/getaddrinfo.c:


../sysdeps/posix/getaddrinfo.c: In function 'gaih_inet.constprop':
../sysdeps/posix/getaddrinfo.c:1081:3: error: 'memcpy' writing 16 bytes 
into a region of size 8 overflows the destination 
[-Werror=stringop-overflow=]

 1081 |   memcpy (>sin6_addr,
  |   ^~
 1082 |at2->addr, sizeof (struct in6_addr));
  |
In file included from ../include/netinet/in.h:3,
 from ../resolv/bits/types/res_state.h:5,
 from ../include/bits/types/res_state.h:1,
 from ../nptl/descr.h:35,
 from ../sysdeps/nios2/nptl/tls.h:45,
 from ../sysdeps/generic/libc-tsd.h:44,
 from ../include/../locale/localeinfo.h:224,
 from ../include/ctype.h:26,
 from ../sysdeps/posix/getaddrinfo.c:57:
../inet/netinet/in.h:249:19: note: destination object 'sin_zero'
  249 | unsigned char sin_zero[sizeof (struct sockaddr)
  |   ^~~~


I have to say that I don't understand the "note" diagnostic here at all. 
 :-(  Why does it think the destination object is a field of struct 
sockaddr_in, while this memcpy is filling in a field of struct 
sockaddr_in6?  (And, the sin6_addr field is indeed of type struct 
in6_addr, matching the sizeof expression.)


-Sandra the confused

Re: [PATCH] avoid false positives due to compute_objsize (PR 95353)

[Martin Sebor via Gcc-patches]

On 6/11/20 2:37 PM, Rainer Orth wrote:

Hi Martin,


The compute_objsize() function started out as a thin wrapper around
compute_builtin_object_size(), but over time developed its own
features to compensate for the other function's limitations (such
as its inability to work with ranges).  The interaction of these
features and the limitations has started to become increasingly
problematic as the former function is used in more contexts.

A complete "fix" for all the problems (as well as some other
limitations) that I'm working on will be more extensive and won't
be appropriate for backports.  Until then, the attached patch
cleans up the extensions compute_objsize() has accumulated over
the years to avoid a class of false positives.

To make the warnings issued based on the results of the function
easier to understand and fix, the patch also adds an informative
message to many instances of -Wstringop-overflow to point to
the object to which the warning refers.  This is especially
helpful when the object is referenced by a series of pointer
operations.

Tested by boostrapping on x86_64-linux and building Binutils/GDB,
Glibc, and the Linux kernel with no new warnings.

Besides applying it to trunk I'm looking to backport the fix to
GCC 10.


it seems you were over-eager in removing xfail's from
gcc.dg/builtin-stringop-chk-5.c: on both Solaris/SPARC and x86 (32-bit
only) I see


Thanks fpr the heads up.  The test just needed adjusting to avoid
unintended assumptions about the data model.  I've fixed it in
r11-1292.

Martin



+FAIL: gcc.dg/builtin-stringop-chk-5.c (test for excess errors)
+FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for 
warnings, line 136)
+FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for 
warnings, line 139)
+FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for 
warnings, line 142)
+FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for 
warnings, line 145)
+FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for 
warnings, line 148)
+FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for 
warnings, line 151)

Excess errors:
/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:136:3:
 warning: 'memset' writing 4 bytes into a region of size 1 overflows the 
destination [-Wstringop-overflow=]
/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:139:3:
 warning: 'memset' writing 4 bytes into a region of size 1 overflows the 
destination [-Wstringop-overflow=]
/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:142:3:
 warning: 'memset' writing 3 bytes into a region of size 1 overflows the 
destination [-Wstringop-overflow=]
/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:145:3:
 warning: 'memset' writing 3 bytes into a region of size 1 overflows the 
destination [-Wstringop-overflow=]
/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:148:3:
 warning: 'memset' writing 2 bytes into a region of size 1 overflows the 
destination [-Wstringop-overflow=]
/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:151:3:
 warning: 'memset' writing 2 bytes into a region of size 1 overflows the 
destination [-Wstringop-overflow=]

Rainer

[PATCH] libgomp: added partial omp-tools.h for OMPD.

[y2s1982 via Gcc-patches]

This patch adds a partial omp-tools.h from OpenMP which describes function
prototypes and data types used for OMPD.

All feedback has been addressed.

2020-06-13  Tony Sim  

libgomp/ChangeLog:

* Makefile.am (nodist_libsubinclude_HEADERS): Add omp-tools.h.
* Makefile.in: Regenerate.
* omp-tools.h: New file.

---
 libgomp/Makefile.am |   2 +-
 libgomp/Makefile.in |   8 +-
 libgomp/omp-tools.h | 295 
 3 files changed, 300 insertions(+), 5 deletions(-)
 create mode 100644 libgomp/omp-tools.h

diff --git a/libgomp/Makefile.am b/libgomp/Makefile.am
index b84156291e8..217e6728d1c 100644
--- a/libgomp/Makefile.am
+++ b/libgomp/Makefile.am
@@ -74,7 +74,7 @@ libgomp_la_SOURCES += openacc.f90
 endif
 
 nodist_noinst_HEADERS = libgomp_f.h
-nodist_libsubinclude_HEADERS = omp.h openacc.h acc_prof.h
+nodist_libsubinclude_HEADERS = omp.h omp-tools.h openacc.h acc_prof.h
 if USE_FORTRAN
 nodist_finclude_HEADERS = omp_lib.h omp_lib.f90 omp_lib.mod omp_lib_kinds.mod \
openacc_lib.h openacc.f90 openacc.mod openacc_kinds.mod
diff --git a/libgomp/Makefile.in b/libgomp/Makefile.in
index 5ff2ac14db9..c844afd7f0d 100644
--- a/libgomp/Makefile.in
+++ b/libgomp/Makefile.in
@@ -16,7 +16,7 @@
 
 # Plugins for offload execution, Makefile.am fragment.
 #
-# Copyright (C) 2014-2019 Free Software Foundation, Inc.
+# Copyright (C) 2014-2020 Free Software Foundation, Inc.
 #
 # Contributed by Mentor Embedded.
 #
@@ -573,8 +573,8 @@ libgomp_la_SOURCES = alloc.c atomic.c barrier.c critical.c 
env.c \
affinity.c target.c splay-tree.c libgomp-plugin.c \
oacc-parallel.c oacc-host.c oacc-init.c oacc-mem.c \
oacc-async.c oacc-plugin.c oacc-cuda.c priority_queue.c \
-   affinity-fmt.c teams.c allocator.c oacc-profiling.c oacc-target.c \
-   $(am__append_4)
+   affinity-fmt.c teams.c allocator.c oacc-profiling.c \
+   oacc-target.c $(am__append_4)
 
 # Nvidia PTX OpenACC plugin.
 @PLUGIN_NVPTX_TRUE@libgomp_plugin_nvptx_version_info = -version-info 
$(libtool_VERSION)
@@ -610,7 +610,7 @@ libgomp_la_SOURCES = alloc.c atomic.c barrier.c critical.c 
env.c \
 @PLUGIN_GCN_TRUE@libgomp_plugin_gcn_la_LIBADD = libgomp.la $(PLUGIN_GCN_LIBS)
 @PLUGIN_GCN_TRUE@libgomp_plugin_gcn_la_LIBTOOLFLAGS = --tag=disable-static
 nodist_noinst_HEADERS = libgomp_f.h
-nodist_libsubinclude_HEADERS = omp.h openacc.h acc_prof.h
+nodist_libsubinclude_HEADERS = omp.h omp-tools.h openacc.h acc_prof.h
 @USE_FORTRAN_TRUE@nodist_finclude_HEADERS = omp_lib.h omp_lib.f90 omp_lib.mod 
omp_lib_kinds.mod \
 @USE_FORTRAN_TRUE@ openacc_lib.h openacc.f90 openacc.mod openacc_kinds.mod
 
diff --git a/libgomp/omp-tools.h b/libgomp/omp-tools.h
new file mode 100644
index 000..394c33e40dd
--- /dev/null
+++ b/libgomp/omp-tools.h
@@ -0,0 +1,295 @@
+/* Copyright (C) 2020 Free Software Foundation, Inc.
+   Contributed by Yoosuk Sim .
+
+   This file is part of the GNU Offloading and Multi Processing Library
+   (libgomp).
+
+   Libgomp is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   Libgomp is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   .  */
+
+/* This file contains prototypes of functions and data types defined
+   in the OMPD standard.  */
+
+#ifndef _OMP_TOOLS_H
+#define _OMP_TOOLS_H
+
+#ifdef __cplusplus
+extern "C" {
+# define __GOMPD_NOTHROW throw ()
+#else
+# define __GOMPD_NOTHROW __attribute__((__nothrow__))
+#endif
+
+typedef __UINT64_TYPE__ ompd_size_t;
+
+typedef __UINT64_TYPE__ ompd_wait_id_t;
+
+typedef __UINT64_TYPE__ ompd_addr_t;
+typedef __INT64_TYPE__ ompd_word_t;
+typedef __UINT64_TYPE__ ompd_seg_t;
+
+typedef __UINT64_TYPE__ ompd_device_t;
+
+typedef __UINT64_TYPE__ ompd_thread_id_t;
+
+typedef enum ompd_scope_t {
+  ompd_scope_global = 1,
+  ompd_scope_address_space = 2,
+  ompd_scope_thread = 3,
+  ompd_scope_parallel = 4,
+  ompd_scope_implicit_task = 5,
+  ompd_scope_task = 6
+} ompd_scope_t;
+
+typedef __UINT64_TYPE__ ompd_icv_id_t;
+
+typedef enum ompd_rc_t {
+  ompd_rc_ok = 0,
+  ompd_rc_unavailable = 1,
+  ompd_rc_stale_handle = 2,
+  ompd_rc_bad_input = 3,
+  ompd_rc_error = 4,
+  ompd_rc_unsupported = 5,
+  ompd_rc_needs_state_tracking = 

Re: [PATCH] libgomp: added a partial omp-tools.h for OMPD

[y2s1982 . via Gcc-patches]

Hi Jakub,

When I compiled the header after adding the missing typedefs, I got
warnings about nothrow attribute being ignored on typedef function
declarations.
Should I remove them from typedef function declarations?

Cheers,

Tony Sim


On Sat, Jun 13, 2020 at 10:15 AM y2s1982 .  wrote:

> Hi Jakub,
>
> Thank you for the valuable feedback. I especially liked the tip on how to
> compile the header.
> I will make a new patch and submit it.
>
> Cheers,
>
> Tony Sim
>
> On Sat, Jun 13, 2020 at 5:13 AM Jakub Jelinek  wrote:
>
>> On Sat, Jun 13, 2020 at 11:06:52AM +0200, Jakub Jelinek via Gcc-patches
>> wrote:
>> > On Fri, Jun 12, 2020 at 07:51:32PM -0400, y2s1982 wrote:
>> > > This patch adds a partial omp-tools.h from OpenMP project which
>> > > declares function prototypes and typedefs used in OMPD.
>> > >
>> > > This patch also addressed all feedback.
>> > >
>> > > 2020-06-12  Tony Sim  
>> > >
>> > > libgomp/ChangeLog:
>> > >
>> > > * Makefile.am: Added new header.
>> >
>> > This should give more details, like:
>> >   * Makefile.am (nodist_libsubinclude_HEADERS): Add omp-tools.h.
>> >
>> > > * Makefile.in: Regenerate.
>> > > * omp-tools.h: New file.
>> >
>> > Otherwise LGTM, but as we discussed, for now please push it to your
>> > stable repository and we'll put it into GCC mainline when it is complete
>> > or at least mostly complete.
>>
>> Actually, have you tried to compile the header?
>> echo '#include "omp-tools.h"' | gcc -S -xc - -o /tmp/omp-tools.s
>> ?  I think you are missing some typedefs.
>>
>> E.g. in OpenMP/sources I see
>> typedef uint64_t ompd_size_t;
>>
>> typedef uint64_t ompd_wait_id_t;
>>
>> typedef uint64_t ompd_addr_t;
>> typedef int64_t ompd_word_t;
>> typedef uint64_t ompd_seg_t;
>>
>> typedef uint64_t ompd_device_t;
>> and I think from these you only have ompd_wait_id_t...
>> For int64_t replacement use __INT64_TYPE__...
>>
>> Jakub
>>
>>

Re: [PATCH] libgomp: added a partial omp-tools.h for OMPD

[y2s1982 . via Gcc-patches]

Hi Jakub,

Thank you for the valuable feedback. I especially liked the tip on how to
compile the header.
I will make a new patch and submit it.

Cheers,

Tony Sim

On Sat, Jun 13, 2020 at 5:13 AM Jakub Jelinek  wrote:

> On Sat, Jun 13, 2020 at 11:06:52AM +0200, Jakub Jelinek via Gcc-patches
> wrote:
> > On Fri, Jun 12, 2020 at 07:51:32PM -0400, y2s1982 wrote:
> > > This patch adds a partial omp-tools.h from OpenMP project which
> > > declares function prototypes and typedefs used in OMPD.
> > >
> > > This patch also addressed all feedback.
> > >
> > > 2020-06-12  Tony Sim  
> > >
> > > libgomp/ChangeLog:
> > >
> > > * Makefile.am: Added new header.
> >
> > This should give more details, like:
> >   * Makefile.am (nodist_libsubinclude_HEADERS): Add omp-tools.h.
> >
> > > * Makefile.in: Regenerate.
> > > * omp-tools.h: New file.
> >
> > Otherwise LGTM, but as we discussed, for now please push it to your
> > stable repository and we'll put it into GCC mainline when it is complete
> > or at least mostly complete.
>
> Actually, have you tried to compile the header?
> echo '#include "omp-tools.h"' | gcc -S -xc - -o /tmp/omp-tools.s
> ?  I think you are missing some typedefs.
>
> E.g. in OpenMP/sources I see
> typedef uint64_t ompd_size_t;
>
> typedef uint64_t ompd_wait_id_t;
>
> typedef uint64_t ompd_addr_t;
> typedef int64_t ompd_word_t;
> typedef uint64_t ompd_seg_t;
>
> typedef uint64_t ompd_device_t;
> and I think from these you only have ompd_wait_id_t...
> For int64_t replacement use __INT64_TYPE__...
>
> Jakub
>
>

Re: [PATCH v1 1/2][PPC64] [PR88877]

[kamlesh kumar via Gcc-patches]

Thank you all for the suggestions.
This is first patch where I have just defined a struct libcall_arg_t
which contains
three member rtx, machine_mode and a boolean unsigned_p and will be
used in passing args in
emit_library_[call/value] functions.

Once this patch is approved then i will create second patch in which
arg type libcall_arg_t will be
propogated on all instances needed.
Then final patch which will fix the actual problem reported in PR88877.

ChangeLog Entry:

2020-06-13  Kamlesh Kumar  

   * rtl.h (libcall_arg_t): Defined.


---
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 0872cc4..c023ff0 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2238,6 +2238,18 @@ struct address_info {
   enum rtx_code base_outer_code;
 };

+/* This is used for passing args in emit_library_* functions */
+typedef struct libcall_arg {
+  rtx value;
+  machine_mode mode;
+  bool unsigned_p;
+  libcall_arg (rtx v, machine_mode m, bool u) {
+value = v;
+mode = m;
+unsigned_p = u;
+  }
+} libcall_arg_t;
+
 /* This is used to bundle an rtx and a mode together so that the pair
can be used with the wi:: routines.  If we ever put modes into rtx
integer constants, this should go away and then just pass an rtx in.  */
--
2.7.4

On Fri, Jun 12, 2020 at 4:43 AM Segher Boessenkool
 wrote:
>
> On Tue, Jun 09, 2020 at 02:29:13PM -0600, Jeff Law wrote:
> > On Sun, 2020-05-24 at 11:22 -0500, Segher Boessenkool wrote:
> > > OTOH, you don't need to name Tuple at all...  It should not *have* a
> > > constructor, since you declared it as class...  But you can just use
> > > std::tuple here?
> > >
> > > > (emit_library_call): Added default arg unsigned_p.
> > > > (emit_library_call_value): Added default arg unsigned_p.
> > >
> > > Yeah, eww.  Default arguments have all the problems you had before,
> > > except now it is hidden and much more surprising.
> > >
> > > Those functions really should take rtx_mode_t arguments?
> > >
> > > Thanks again for working on this,
> > ISTM that using std::tuple would be better than defining our own types.
>
> Yeah.  But as Jakub an Iain said, not using a container type (but a more
> concrete type, instead) is much better anyway :-)
>
> > I'd rather see the argument be explicit rather than using default arguments 
> > too.
> > While I have ack'd some patches with default arguments, I still don't like 
> > 'em.
>
> Default arguments have their place (but it's not here :-) )
>
> > I do like the approach of getting the infrastructure in place without 
> > changing
> > behavior, then having the behavior fix as a distinct change.
>
> With Git, commits are easy and cheap, and massaging a patch series into
> shape is easy and cheap as well.  If you develop using Git in the first
> place (and you should!), you should naturally end up with many patches
> in your series, and the preparatory patches first (after you reshuffle
> things a bit, if you are like me and your foresight is severly limited).
>
> So you have this separate *anyway* (or should have).  Since it helps
> reviewing a lot, and also later bisecting, it is good to keep it.
>
>
> Segher

[PATCH] Defined libcall_arg_t

[Kamlesh Kumar via Gcc-patches]

This is first patch where I have just defined a struct libcall_arg_t which 
contains
three member rtx, machine_mode and a boolean unsigned_p and will be used in 
passing args in
emit_library_[call/value] functions.

Once this patch is approved then i will create second patch in which arg type 
libcall_arg_t will be
propogated on all instances needed.
Then final patch which will fix the actual problem reported in PR88877.

ChangeLog Entry:

2020-06-13  Kamlesh Kumar  

   * rtl.h (libcall_arg_t): Defined.


---
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 0872cc4..c023ff0 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2238,6 +2238,18 @@ struct address_info {
   enum rtx_code base_outer_code;
 };
 
+/* This is used for passing args in emit_library_* functions */
+typedef struct libcall_arg {
+  rtx value;
+  machine_mode mode;
+  bool unsigned_p;
+  libcall_arg (rtx v, machine_mode m, bool u) {
+value = v;
+mode = m;
+unsigned_p = u;
+  }
+} libcall_arg_t;
+
 /* This is used to bundle an rtx and a mode together so that the pair
can be used with the wi:: routines.  If we ever put modes into rtx
integer constants, this should go away and then just pass an rtx in.  */
-- 
2.7.4

[PATCH] Defined libcall_arg_t

[Kamlesh Kumar via Gcc-patches]

This is first patch where I have just defined a struct libcall_arg_t which 
contains
three member rtx, machine_mode and a boolean unsigned_p and will be used in 
passing args in
emit_library_[call/value] functions.

Once this patch is approved then i will create second patch in which arg type 
libcall_arg_t will be
propogated on all instances needed.
Then final patch which will fix the actual problem reported in PR88877.

ChangeLog Entry:

2020-06-13  Kamlesh Kumar  

   * rtl.h (libcall_arg_t): Defined.


---
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 0872cc4..c023ff0 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2238,6 +2238,18 @@ struct address_info {
   enum rtx_code base_outer_code;
 };
 
+/* This is used for passing args in emit_library_* functions */
+typedef struct libcall_arg {
+  rtx value;
+  machine_mode mode;
+  bool unsigned_p;
+  libcall_arg (rtx v, machine_mode m, bool u) {
+value = v;
+mode = m;
+unsigned_p = u;
+  }
+} libcall_arg_t;
+
 /* This is used to bundle an rtx and a mode together so that the pair
can be used with the wi:: routines.  If we ever put modes into rtx
integer constants, this should go away and then just pass an rtx in.  */
-- 
2.7.4

Re: [PATCH] libgomp: added a partial omp-tools.h for OMPD

[Jakub Jelinek via Gcc-patches]

On Sat, Jun 13, 2020 at 11:06:52AM +0200, Jakub Jelinek via Gcc-patches wrote:
> On Fri, Jun 12, 2020 at 07:51:32PM -0400, y2s1982 wrote:
> > This patch adds a partial omp-tools.h from OpenMP project which
> > declares function prototypes and typedefs used in OMPD.
> > 
> > This patch also addressed all feedback.
> > 
> > 2020-06-12  Tony Sim  
> > 
> > libgomp/ChangeLog:
> > 
> > * Makefile.am: Added new header.
> 
> This should give more details, like:
>   * Makefile.am (nodist_libsubinclude_HEADERS): Add omp-tools.h.
> 
> > * Makefile.in: Regenerate.
> > * omp-tools.h: New file.
> 
> Otherwise LGTM, but as we discussed, for now please push it to your
> stable repository and we'll put it into GCC mainline when it is complete
> or at least mostly complete.

Actually, have you tried to compile the header?
echo '#include "omp-tools.h"' | gcc -S -xc - -o /tmp/omp-tools.s
?  I think you are missing some typedefs.

E.g. in OpenMP/sources I see
typedef uint64_t ompd_size_t;

typedef uint64_t ompd_wait_id_t;

typedef uint64_t ompd_addr_t;
typedef int64_t ompd_word_t;
typedef uint64_t ompd_seg_t;

typedef uint64_t ompd_device_t;
and I think from these you only have ompd_wait_id_t...
For int64_t replacement use __INT64_TYPE__...

Jakub

Re: [PATCH] libgomp: added a partial omp-tools.h for OMPD

[Jakub Jelinek via Gcc-patches]

On Fri, Jun 12, 2020 at 07:51:32PM -0400, y2s1982 wrote:
> This patch adds a partial omp-tools.h from OpenMP project which
> declares function prototypes and typedefs used in OMPD.
> 
> This patch also addressed all feedback.
> 
> 2020-06-12  Tony Sim  
> 
> libgomp/ChangeLog:
> 
>   * Makefile.am: Added new header.

This should give more details, like:
* Makefile.am (nodist_libsubinclude_HEADERS): Add omp-tools.h.

>   * Makefile.in: Regenerate.
>   * omp-tools.h: New file.

Otherwise LGTM, but as we discussed, for now please push it to your
stable repository and we'll put it into GCC mainline when it is complete
or at least mostly complete.

Jakub

Re: [PATCH 1/2] RISC-V: Describe correct USEs for gpr_save pattern [PR95252]

[Andreas Schwab]

On Jun 10 2020, Kito Cheng wrote:

>  - Verified on rv32emc/rv32gc/rv64gc bare-metal target and rv32gc/rv64gc
>linux target with qemu.

Obviously not:

../../gcc/config/riscv/riscv.c:5190:21: error: comparison of integer 
expressions of different signedness: 'int' and 'unsigned int' 
[-Werror=sign-compare]
 5190 |   for (int i = 1; i < veclen; ++i)
  |   ~~^~~~
In file included from ../../gcc/config/riscv/riscv.c:26:
../../gcc/config/riscv/riscv.c: In function 'bool 
riscv_gpr_save_operation_p(rtx)':
../../gcc/config/riscv/riscv.c:5219:19: error: comparison of integer 
expressions of different signedness: 'long int' and 'long unsigned int' 
[-Werror=sign-compare]
 5219 |   gcc_assert (len <= ARRAY_SIZE (gpr_save_reg_order));
../../gcc/system.h:748:14: note: in definition of macro 'gcc_assert'
  748 |((void)(!(EXPR) ? fancy_abort (__FILE__, __LINE__, __FUNCTION__), 0 
: 0))
  |  ^~~~

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

[patch, libfortran, committed] Suppress -Wstringop-overflow warning

[Thomas Koenig via Gcc-patches]

Hi,

I just committed as obvious the patch below.

Disable -Wstringop-overflow warning after checking code path of caller.

The warning that is disabled, only on this single line, has been
inspected and found to be not applicable; it is known that the size
of the buffer is safe.

libgfortran/ChangeLog:

2020-06-13  Thomas Koenig  

PR libfortran/95313
* io/write.c (ztoa_big): Disable -Wstringop-overflow for one
line.
diff --git a/libgfortran/io/write.c b/libgfortran/io/write.c
index 9f02683a25c..346615ed597 100644
--- a/libgfortran/io/write.c
+++ b/libgfortran/io/write.c
@@ -1178,7 +1178,15 @@ ztoa_big (const char *s, char *buffer, int len, GFC_UINTEGER_LARGEST *n)
 	}
 }
 
+  /* write_z, which calls ztoa_big, is called from transfer.c,
+ formatted_transfer_scalar_write.  There it is passed the kind as
+ argument, which means a maximum of 16.  The buffer is large
+ enough, but the compiler does not know that, so shut up the
+ warning here.  */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstringop-overflow"
   *q = '\0';
+#pragma GCC diagnostic pop
 
   if (*n == 0)
 return "0";