Iain-san,

Thank you for finally working on GDC Darwin support.

T. Yamada

2020年11月30日(月) 7:28 <gcc-patches-requ...@gcc.gnu.org>:
>
> Send Gcc-patches mailing list submissions to
>         gcc-patches@gcc.gnu.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
>         https://gcc.gnu.org/mailman/listinfo/gcc-patches
> or, via email, send a message with subject or body 'help' to
>         gcc-patches-requ...@gcc.gnu.org
>
> You can reach the person managing the list at
>         gcc-patches-ow...@gcc.gnu.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Gcc-patches digest..."
> Today's Topics:
>
>    1. Re: [PATCH] configure: Support building D front-end on
>       *-*-darwin* (Iain Buclaw)
>    2. Re: [PATCH] d: Add darwin support for D language front-end
>       (Iain Buclaw)
>    3. Fix freeing of thunk_info (Jan Hubicka)
>    4. Re: [PATCH] handle conditionals in -Wstringop-overflow et al.
>       (PR 92936) (Martin Sebor)
>
>
>
> ---------- Forwarded message ----------
> From: Iain Buclaw <ibuc...@gdcproject.org>
> To: Iain Sandoe <i...@sandoe.co.uk>
> Cc: gcc-patches@gcc.gnu.org
> Bcc:
> Date: Sun, 29 Nov 2020 22:13:58 +0100
> Subject: Re: [PATCH] configure: Support building D front-end on *-*-darwin*
> Excerpts from Iain Sandoe's message of November 29, 2020 10:35 am:
> > Hi Iain
> >
> > Iain Buclaw <ibuc...@gdcproject.org> wrote:
> >
> >> The bootstrap has been succeeding for some time now, there's no need to
> >> set it as an unsupported language.
> >>
> >> OK for mainline?
> >
> > At present, this breaks bootstrap on 32 bit Darwin hosts.
> >
> > Once that’s resolved, then it seems a reasonable to make sure that the D
> > FE is built and tested (even though there is still work to do on the library
> > support).
> >
> > So, OK once all the currently tested Darwin hosts bootstrap with D enabled.
> >
>
> As confirmed off list, powerpc and i686 darwin9 bootstrapped OK, so I've
> committed it as r11-5521.
>
> Iain.
>
>
>
>
> ---------- Forwarded message ----------
> From: Iain Buclaw <ibuc...@gdcproject.org>
> To: Iain Sandoe <i...@sandoe.co.uk>
> Cc: gcc-patches@gcc.gnu.org
> Bcc:
> Date: Sun, 29 Nov 2020 22:21:26 +0100
> Subject: Re: [PATCH] d: Add darwin support for D language front-end
> Excerpts from Iain Sandoe's message of November 29, 2020 1:49 pm:
> > Hi Iain
> >
> > Iain Buclaw <ibuc...@gdcproject.org> wrote:
> >
> >> This patch adds necessary predefined version symbols and support for
> >> moduleinfo sections for darwin to allow testing libphobos support.
> >>
> >> OK for mainline?
> >
> > As we discussed off-list, this sets ABI (the section names are visible and
> > part
> > of the contract with the runtime).
> >
> > It’s highly desirable that we can interoperate with D objects built by
> > other impls.
> > so the section naming convention should be agreed.
> >
> > As far as I understand things, the names you have here agree with those used
> > by “upstream” (but current the LLVM D variant uses something different).
> >
> > The Darwin parts are OK, with a note to (re-)check on the section names
> > before
> > the first release*.
> >
> > thanks
> > Iain
> >
> > * perhaps it might be worth pinging the relevant upstream and LLV folks long
> > before that - since the release cycles there mean that there will likely be
> > another
> > one before GCC11 ships.
> >
>
> I've sent Martin a message, so hoping for a response within the week
> which name he'd prefer I go.  I suspect it'd be better to align with
> LLVM instead of the reference DMD compiler.  As the compiler/run-time
> integration of modules uses the same machinery, and we both want to
> support loading shared DSOs (DMD is static only on OSX).
>
> Adjusted the patch with some copy/paste fixes (missing tabs, Glibc gets
> a mention).  Committed the following as r11-5522.
>
> Regards,
> Iain
>
> ---
> gcc/ChangeLog:
>
>         * config.gcc (*-*-darwin*): Set d_target_objs and target_has_targetdm.
>         * config/elfos.h (TARGET_D_MINFO_SECTION): New macro.
>         (TARGET_D_MINFO_START_NAME): New macro.
>         (TARGET_D_MINFO_END_NAME): New macro.
>         * config/t-darwin: Add darwin-d.o.
>         * doc/tm.texi: Regenerate.
>         * doc/tm.texi.in (D language and ABI): Add @hook for
>         TARGET_D_MINFO_SECTION, TARGET_D_MINFO_START_NAME, and
>         TARGET_D_MINFO_END_NAME.
>         * config/darwin-d.c: New file.
>
> gcc/d/ChangeLog:
>
>         * d-target.def (d_minfo_section): New hook.
>         (d_minfo_start_name): New hook.
>         (d_minfo_end_name): New hook.
>         * modules.cc: Include d-target.h.
>         (register_moduleinfo): Update to use new targetdm hooks.
> ---
>  gcc/config.gcc        |  2 ++
>  gcc/config/darwin-d.c | 49 +++++++++++++++++++++++++++++++++++++++++++
>  gcc/config/elfos.h    |  6 ++++++
>  gcc/config/t-darwin   |  3 +++
>  gcc/d/d-target.def    | 25 ++++++++++++++++++++++
>  gcc/d/modules.cc      | 14 ++++++++++---
>  gcc/doc/tm.texi       | 20 ++++++++++++++++++
>  gcc/doc/tm.texi.in    |  6 ++++++
>  8 files changed, 122 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/config/darwin-d.c
>
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index 0ae58482657..aacab0d9133 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -702,8 +702,10 @@ case ${target} in
>    extra_options="${extra_options} darwin.opt"
>    c_target_objs="${c_target_objs} darwin-c.o"
>    cxx_target_objs="${cxx_target_objs} darwin-c.o"
> +  d_target_objs="${d_target_objs} darwin-d.o"
>    fortran_target_objs="darwin-f.o"
>    target_has_targetcm=yes
> +  target_has_targetdm=yes
>    extra_objs="${extra_objs} darwin.o"
>    extra_gcc_objs="darwin-driver.o"
>    default_use_cxa_atexit=yes
> diff --git a/gcc/config/darwin-d.c b/gcc/config/darwin-d.c
> new file mode 100644
> index 00000000000..ced07ce006a
> --- /dev/null
> +++ b/gcc/config/darwin-d.c
> @@ -0,0 +1,49 @@
> +/* Darwin support needed only by D front-end.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +
> +GCC 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.
> +
> +GCC 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.
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3.  If not see
> +<http://www.gnu.org/licenses/>.  */
> +
> +#include "config.h"
> +#include "system.h"
> +#include "coretypes.h"
> +#include "tm_d.h"
> +#include "d/d-target.h"
> +#include "d/d-target-def.h"
> +
> +/* Implement TARGET_D_OS_VERSIONS for Darwin targets.  */
> +
> +static void
> +darwin_d_os_builtins (void)
> +{
> +  d_add_builtin_version ("Posix");
> +  d_add_builtin_version ("OSX");
> +  d_add_builtin_version ("darwin");
> +}
> +
> +#undef TARGET_D_OS_VERSIONS
> +#define TARGET_D_OS_VERSIONS darwin_d_os_builtins
> +
> +/* Define TARGET_D_MINFO_SECTION for Darwin targets.  */
> +
> +#undef TARGET_D_MINFO_SECTION
> +#define TARGET_D_MINFO_SECTION "__DATA,__minfodata"
> +
> +#undef TARGET_D_MINFO_START_NAME
> +#define TARGET_D_MINFO_START_NAME "*section$start$__DATA$__minfodata"
> +
> +#undef TARGET_D_MINFO_END_NAME
> +#define TARGET_D_MINFO_END_NAME "*section$end$__DATA$__minfodata"
> +
> +struct gcc_targetdm targetdm = TARGETDM_INITIALIZER;
> diff --git a/gcc/config/elfos.h b/gcc/config/elfos.h
> index 74a3eafda6b..d8f169f54df 100644
> --- a/gcc/config/elfos.h
> +++ b/gcc/config/elfos.h
> @@ -474,3 +474,9 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
> If not, see
>
>  #undef TARGET_LIBC_HAS_FUNCTION
>  #define TARGET_LIBC_HAS_FUNCTION no_c99_libc_has_function
> +
> +/* ELF support needed only by D front-end.  */
> +
> +#define TARGET_D_MINFO_SECTION "minfo"
> +#define TARGET_D_MINFO_START_NAME "__start_minfo"
> +#define TARGET_D_MINFO_END_NAME "__stop_minfo"
> diff --git a/gcc/config/t-darwin b/gcc/config/t-darwin
> index 7f2ac282bfc..355389c60b4 100644
> --- a/gcc/config/t-darwin
> +++ b/gcc/config/t-darwin
> @@ -26,6 +26,9 @@ darwin-c.o: $(srcdir)/config/darwin-c.c
>         $(COMPILE) $(PREPROCESSOR_DEFINES) $<
>         $(POSTCOMPILE)
>
> +darwin-d.o: $(srcdir)/config/darwin-d.c
> +       $(COMPILE) $<
> +       $(POSTCOMPILE)
>
>  darwin-f.o: $(srcdir)/config/darwin-f.c
>         $(COMPILE) $<
> diff --git a/gcc/d/d-target.def b/gcc/d/d-target.def
> index 41b31723188..728cba70335 100644
> --- a/gcc/d/d-target.def
> +++ b/gcc/d/d-target.def
> @@ -46,5 +46,30 @@ relating to the target operating system.",
>   void, (void),
>   hook_void_void)
>
> +/* ModuleInfo section name and brackets.  */
> +DEFHOOKPOD
> +(d_minfo_section,
> + "Contains the name of the section in which module info references should 
> be\n\
> +placed.  This section is expected to be bracketed by two symbols to 
> indicate\n\
> +the start and end address of the section, so that the runtime library can\n\
> +collect all modules for each loaded shared library and executable.  The\n\
> +default value of @code{NULL} disables the use of sections altogether.",
> + const char *, NULL)
> +
> +DEFHOOKPOD
> +(d_minfo_start_name,
> + "If @code{TARGET_D_MINFO_SECTION} is defined, then this must also be 
> defined\n\
> +as the name of the symbol indicating the start address of the module info\n\
> +section",
> + const char *, NULL)
> +
> +/* The name of the ModuleInfo section.  */
> +DEFHOOKPOD
> +(d_minfo_end_name,
> + "If @code{TARGET_D_MINFO_SECTION} is defined, then this must also be 
> defined\n\
> +as the name of the symbol indicating the end address of the module info\n\
> +section",
> + const char *, NULL)
> +
>  /* Close the 'struct gcc_targetdm' definition.  */
>  HOOK_VECTOR_END (C90_EMPTY_HACK)
> diff --git a/gcc/d/modules.cc b/gcc/d/modules.cc
> index 4b48c19a90e..742a24ff0bb 100644
> --- a/gcc/d/modules.cc
> +++ b/gcc/d/modules.cc
> @@ -36,6 +36,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "stringpool.h"
>
>  #include "d-tree.h"
> +#include "d-target.h"
>
>
>  /* D generates module information to inform the runtime library which modules
> @@ -405,6 +406,10 @@ build_dso_registry_var (const char * name, tree type)
>  static void
>  register_moduleinfo (Module *decl, tree minfo)
>  {
> +  /* No defined minfo section for target.  */
> +  if (targetdm.d_minfo_section == NULL)
> +    return;
> +
>    if (!targetm_common.have_named_sections)
>      sorry ("%<-fmoduleinfo%> is not supported on this target");
>
> @@ -420,7 +425,8 @@ register_moduleinfo (Module *decl, tree minfo)
>    DECL_EXTERNAL (mref) = 0;
>    DECL_PRESERVE_P (mref) = 1;
>
> -  set_decl_section_name (mref, "minfo");
> +  set_decl_section_name (mref, targetdm.d_minfo_section);
> +  symtab_node::get (mref)->implicit_section = true;
>    d_pushdecl (mref);
>    rest_of_decl_compilation (mref, 1, 0);
>
> @@ -431,10 +437,12 @@ register_moduleinfo (Module *decl, tree minfo)
>    if (!first_module)
>      return;
>
> -  start_minfo_node = build_dso_registry_var ("__start_minfo", ptr_type_node);
> +  start_minfo_node = build_dso_registry_var (targetdm.d_minfo_start_name,
> +                                            ptr_type_node);
>    rest_of_decl_compilation (start_minfo_node, 1, 0);
>
> -  stop_minfo_node = build_dso_registry_var ("__stop_minfo", ptr_type_node);
> +  stop_minfo_node = build_dso_registry_var (targetdm.d_minfo_end_name,
> +                                           ptr_type_node);
>    rest_of_decl_compilation (stop_minfo_node, 1, 0);
>
>    /* Declare dso_slot and dso_initialized.  */
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index d9502c2c607..9f700b1c774 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -10822,6 +10822,26 @@ Similarly to @code{TARGET_D_CPU_VERSIONS}, but is 
> used for versions
>  relating to the target operating system.
>  @end deftypefn
>
> +@deftypevr {D Target Hook} {const char *} TARGET_D_MINFO_SECTION
> +Contains the name of the section in which module info references should be
> +placed.  This section is expected to be bracketed by two symbols to indicate
> +the start and end address of the section, so that the runtime library can
> +collect all modules for each loaded shared library and executable.  The
> +default value of @code{NULL} disables the use of sections altogether.
> +@end deftypevr
> +
> +@deftypevr {D Target Hook} {const char *} TARGET_D_MINFO_START_NAME
> +If @code{TARGET_D_MINFO_SECTION} is defined, then this must also be defined
> +as the name of the symbol indicating the start address of the module info
> +section
> +@end deftypevr
> +
> +@deftypevr {D Target Hook} {const char *} TARGET_D_MINFO_END_NAME
> +If @code{TARGET_D_MINFO_SECTION} is defined, then this must also be defined
> +as the name of the symbol indicating the end address of the module info
> +section
> +@end deftypevr
> +
>  @node Named Address Spaces
>  @section Adding support for named address spaces
>  @cindex named address spaces
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index b08923c8f28..012cb1c53f0 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -7353,6 +7353,12 @@ floating-point support; they are not included in this 
> mechanism.
>
>  @hook TARGET_D_OS_VERSIONS
>
> +@hook TARGET_D_MINFO_SECTION
> +
> +@hook TARGET_D_MINFO_START_NAME
> +
> +@hook TARGET_D_MINFO_END_NAME
> +
>  @node Named Address Spaces
>  @section Adding support for named address spaces
>  @cindex named address spaces
> --
> 2.27.0
>
>
>
>
>
> ---------- Forwarded message ----------
> From: Jan Hubicka <hubi...@ucw.cz>
> To: gcc-patches@gcc.gnu.org
> Cc:
> Bcc:
> Date: Sun, 29 Nov 2020 23:23:01 +0100
> Subject: Fix freeing of thunk_info
> Hi,
> thunk_info should be freed with ggc_delete since it is ggc allocated.
>
> Bootstrapped/regtested x86_64-linux,
> Honza
>         PR jit/97867
>         * symtab-thunks.h (thunk_info::release): Use ggc_delete.
> diff --git a/gcc/symtab-thunks.h b/gcc/symtab-thunks.h
> index 41a684995b3..0dba2217793 100644
> --- a/gcc/symtab-thunks.h
> +++ b/gcc/symtab-thunks.h
> @@ -167,7 +167,7 @@ inline void
>  thunk_info::release ()
>  {
>    if (symtab->m_thunks)
> -    delete (symtab->m_thunks);
> +    ggc_delete (symtab->m_thunks);
>    symtab->m_thunks = NULL;
>  }
>  #endif  /* GCC_SYMTAB_THUNKS_H  */
>
>
>
>
> ---------- Forwarded message ----------
> From: Martin Sebor <mse...@gmail.com>
> To: Jeff Law <l...@redhat.com>, gcc-patches <gcc-patches@gcc.gnu.org>
> Cc:
> Bcc:
> Date: Sun, 29 Nov 2020 15:27:39 -0700
> Subject: Re: [PATCH] handle conditionals in -Wstringop-overflow et al. (PR 
> 92936)
> On 11/13/20 2:34 PM, Jeff Law wrote:
> >
> > On 11/2/20 7:24 PM, Martin Sebor wrote:
> >> The attached patch extends compute_objsize() to handle conditional
> >> expressions represented either as PHIs or MIN_EXPR and MAX_EXPR.
> >>
> >> To simplify the handling of the -Wstringop-overflow/-overread
> >> warnings the change factors this code out of tree-ssa-strlen.c
> >> and into inform_access() in builtins.c, making it a member of
> >> access_ref.  Besides eliminating a decent amount of code
> >> duplication this also improves the consistency of the warnings.
> >>
> >> Finally, the change introduces a distinction between the definite
> >> kinds of -Wstringop-overflow (and -Wstringop-overread) warnings
> >> and the maybe kind.  The latter are currently only being issued
> >> for function array parameters but I expect to make use of them
> >> more extensively in the future.
> >>
> >> Besides the usual GCC bootstrap/regtest I have tested the change
> >> with Binutils/GDB and Glibc and verified that it doesn't introduce
> >> any false positives.
> >>
> >> Martin
> >>
> >> gcc-92936.diff
> >>
> >> PR middle-end/92936 - missing warning on a past-the-end store to a PHI
> >> PR middle-end/92940 - incorrect offset and size in -Wstringop-overflow for 
> >> out-of-bounds store into VLA and two offset ranges
> >> PR middle-end/89428 - missing -Wstringop-overflow on a PHI with variable 
> >> offset
> >>
> >> gcc/ChangeLog:
> >>
> >>      PR middle-end/92936
> >>      PR middle-end/92940
> >>      PR middle-end/89428
> >>      * builtins.c (access_ref::access_ref): Initialize member.
> >>      (access_ref::phi): New function.
> >>      (access_ref::get_ref): New function.
> >>      (access_ref::add_offset): Remove duplicate assignment.
> >>      (maybe_warn_for_bound): Add "maybe" kind of warning messages.
> >>      (warn_for_access): Same.
> >>      (inform_access): Rename...
> >>      (access_ref::inform_access): ...to this.  Print PHI arguments.  Format
> >>      offset the same as size and simplify.  Improve printing of allocation
> >>      functions and VLAs.
> >>      (check_access): Adjust to the above.
> >>      (gimple_parm_array_size): Change argument.
> >>      (handle_min_max_size): New function.
> >>      * builtins.h (struct access_ref): Declare new members.
> >>      (gimple_parm_array_size): Change argument.
> >>      * tree-ssa-strlen.c (maybe_warn_overflow): Use access_ref and 
> >> simplify.
> >>      (handle_builtin_memcpy): Correct argument passed to 
> >> maybe_warn_overflow.
> >>      (handle_builtin_memset): Same.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >>      PR middle-end/92936
> >>      PR middle-end/92940
> >>      PR middle-end/89428
> >>      * c-c++-common/Wstringop-overflow-2.c: Adjust text of expected
> >>      informational notes.
> >>      * gcc.dg/Wstringop-overflow-11.c: Remove xfails.
> >>      * gcc.dg/Wstringop-overflow-12.c: Same.
> >>      * gcc.dg/Wstringop-overflow-17.c: Adjust text of expected messages.
> >>      * gcc.dg/Wstringop-overflow-27.c: Same.  Remove xfails.
> >>      * gcc.dg/Wstringop-overflow-28.c: Adjust text of expected messages.
> >>      * gcc.dg/Wstringop-overflow-29.c: Same.
> >>      * gcc.dg/Wstringop-overflow-37.c: Same.
> >>      * gcc.dg/Wstringop-overflow-46.c: Same.
> >>      * gcc.dg/Wstringop-overflow-47.c: Same.
> >>      * gcc.dg/Wstringop-overflow-54.c: Same.
> >>      * gcc.dg/warn-strnlen-no-nul.c: Add expected warning.
> >>      * gcc.dg/Wstringop-overflow-58.c: New test.
> >>      * gcc.dg/Wstringop-overflow-59.c: New test.
> >>      * gcc.dg/Wstringop-overflow-60.c: New test.
> >>      * gcc.dg/Wstringop-overflow-61.c: New test.
> >>      * gcc.dg/Wstringop-overflow-62.c: New test.
> >
> > So my only significant concern here is the recursive nature and the lack
> > of a limiter for pathological cases.  We certainly run into cases with
> > thousands of PHI arguments and deep chains of PHIs feeding other PHIs.
> > Can you put in a PARAM to limit the amount of recursion and and PHI
> > arguments you look at?  With that I think this is fine -- I consider it
> > unlikely this patch is the root cause of the ICEs I sent you earlier
> > today from the tester since those failures are in the array bounds
> > checking bits.
>
> I've reused the same approach/class as in tree-ssa-strlen.c and
> after adjusting things that need to be adjusted and retesting
> with Binutils/GDB and Glibc committed the attached patch in
> r11-5523.
>
> That said, although the recursion hasn't been a problem (there's
> still code elsewhere that does this sort of traversal of the use-
> def chains that doesn't use the parameter), the subsequent patch
> that adds the cache makes it possible to reduce it to just trees
> (when the function is called in a GIMPLE pass to cache each
> pointer assignment).
>
> Martin

Reply via email to