Re: [PATCH v4] c++: DR2237, cdtor and template-id tweaks [PR107126]

2024-02-09 Thread Jason Merrill

On 2/9/24 18:46, Marek Polacek wrote:

On Fri, Feb 09, 2024 at 10:20:04AM -0500, Jason Merrill wrote:

On 2/8/24 16:26, Marek Polacek wrote:

This patch does *not* fix

where the C++20 diagnostic is missing altogether.


What would it take to fix that as well?


It's the "further DR2237 fix" below which is basically a one liner.
I'd thought it would be more involved than that.
  

* doc/invoke.texi: Document -Wdeprecated-template-id-cdtor.


Was it ever deprecated?  I'm not seeing that in

https://timsong-cpp.github.io/cppwp/n4659/#depr (C++17)


Aha, [diff] != [depr]...
  

Let's drop the word "deprecated" from the option name and documentation.


Done throughout.
  

@@ -32331,11 +32338,11 @@ cp_parser_constructor_declarator_p (cp_parser 
*parser, cp_parser_flags flags,
 if (next_token->type != CPP_NAME
 && next_token->type != CPP_SCOPE
 && next_token->type != CPP_NESTED_NAME_SPECIFIER
-  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
-declarator-id of a constructor or destructor.  */
-  && (next_token->type != CPP_TEMPLATE_ID || cxx_dialect >= cxx20))
+  && next_token->type != CPP_TEMPLATE_ID)
   return false;
+  const bool saw_template_id = (next_token->type == CPP_TEMPLATE_ID);


Please incorporate your "further DR2237 fix" patch into this one.


Patches squashed.
  

@@ -32552,6 +32559,19 @@ cp_parser_constructor_declarator_p (cp_parser *parser, 
cp_parser_flags flags,
 /* We did not really want to consume any tokens.  */
 cp_parser_abort_tentative_parse (parser);
+  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
+ declarator-id of a constructor or destructor.  */
+  if (constructor_p && saw_template_id)
+{
+  gcc_checking_assert
+   (!cp_parser_uncommitted_to_tentative_parse_p (parser));


Now I see the abort_ just above, so this seems unnecessary after all.


Done, thanks.

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


OK, thanks.


-- >8 --
Since my r11-532 changes to implement DR2237, for this test:

   template
   struct S {
 S();
   };

in C++20 we emit the ugly:

q.C:3:8: error: expected unqualified-id before ')' token
 3 |   S();

which doesn't explain what the problem is.  This patch improves that
diagnostic, reduces the error to a pedwarn, and adds a -Wc++20-compat
diagnostic.  We now say:

q.C:3:7: warning: template-id not allowed for constructor in C++20 
[-Wtemplate-id-cdtor]
 3 |   S();
q.C:3:7: note: remove the '< >'

This patch also fixes

where the C++20 diagnostic was missing altogether: The problem was that I 
checked
for CPP_TEMPLATE_ID too early, at a point at which cp_parser_template_id may not
have been called yet.  So let's check for it at the end of the function, after
the tentative parse and rollback.

-Wc++20-compat triggered in libitm/; I sent a patch for that.

DR 2237
PR c++/107126
PR c++/97202

gcc/c-family/ChangeLog:

* c-opts.cc (c_common_post_options): In C++20 or with -Wc++20-compat,
turn on -Wtemplate-id-cdtor.
* c.opt (Wtemplate-id-cdtor): New.

gcc/cp/ChangeLog:

* parser.cc (cp_parser_unqualified_id): Downgrade the DR2237 error to
a pedwarn.
(cp_parser_constructor_declarator_p): Likewise.

gcc/ChangeLog:

* doc/invoke.texi: Document -Wtemplate-id-cdtor.

gcc/testsuite/ChangeLog:

* g++.dg/DRs/dr2237.C: Adjust dg-error.
* g++.dg/parse/constructor2.C: Likewise.
* g++.dg/template/error34.C: Likewise.
* g++.old-deja/g++.pt/ctor2.C: Likewise.
* g++.dg/DRs/dr2237-2.C: New test.
* g++.dg/DRs/dr2237-3.C: New test.
* g++.dg/DRs/dr2237-4.C: New test.
* g++.dg/DRs/dr2237-5.C: New test.
* g++.dg/warn/Wtemplate-id-cdtor-1.C: New test.
* g++.dg/warn/Wtemplate-id-cdtor-2.C: New test.
* g++.dg/warn/Wtemplate-id-cdtor-3.C: New test.
* g++.dg/warn/Wtemplate-id-cdtor-4.C: New test.
---
  gcc/c-family/c-opts.cc|  5 +++
  gcc/c-family/c.opt|  4 +++
  gcc/cp/parser.cc  | 33 ++-
  gcc/doc/invoke.texi   | 19 ++-
  gcc/testsuite/g++.dg/DRs/dr2237-2.C   |  9 +
  gcc/testsuite/g++.dg/DRs/dr2237-3.C   | 16 +
  gcc/testsuite/g++.dg/DRs/dr2237-4.C   | 11 +++
  gcc/testsuite/g++.dg/DRs/dr2237-5.C   |  7 
  gcc/testsuite/g++.dg/DRs/dr2237.C |  2 +-
  gcc/testsuite/g++.dg/parse/constructor2.C | 16 -
  gcc/testsuite/g++.dg/template/error34.C   | 10 +++---
  .../g++.dg/warn/Wtemplate-id-cdtor-1.C|  9 +
  .../g++.dg/warn/Wtemplate-id-cdtor-2.C|  9 +
  .../g++.dg/warn/Wtemplate-id-cdtor-3.C|  9 +
  

[PATCH v4] c++: DR2237, cdtor and template-id tweaks [PR107126]

2024-02-09 Thread Marek Polacek
On Fri, Feb 09, 2024 at 10:20:04AM -0500, Jason Merrill wrote:
> On 2/8/24 16:26, Marek Polacek wrote:
> > This patch does *not* fix
> > 
> > where the C++20 diagnostic is missing altogether.
> 
> What would it take to fix that as well?

It's the "further DR2237 fix" below which is basically a one liner.
I'd thought it would be more involved than that.
 
> > * doc/invoke.texi: Document -Wdeprecated-template-id-cdtor.
> 
> Was it ever deprecated?  I'm not seeing that in
> 
> https://timsong-cpp.github.io/cppwp/n4659/#depr (C++17)

Aha, [diff] != [depr]...
 
> Let's drop the word "deprecated" from the option name and documentation.

Done throughout.
 
> > @@ -32331,11 +32338,11 @@ cp_parser_constructor_declarator_p (cp_parser 
> > *parser, cp_parser_flags flags,
> > if (next_token->type != CPP_NAME
> > && next_token->type != CPP_SCOPE
> > && next_token->type != CPP_NESTED_NAME_SPECIFIER
> > -  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as 
> > the
> > -declarator-id of a constructor or destructor.  */
> > -  && (next_token->type != CPP_TEMPLATE_ID || cxx_dialect >= cxx20))
> > +  && next_token->type != CPP_TEMPLATE_ID)
> >   return false;
> > +  const bool saw_template_id = (next_token->type == CPP_TEMPLATE_ID);
> 
> Please incorporate your "further DR2237 fix" patch into this one.

Patches squashed.
 
> > @@ -32552,6 +32559,19 @@ cp_parser_constructor_declarator_p (cp_parser 
> > *parser, cp_parser_flags flags,
> > /* We did not really want to consume any tokens.  */
> > cp_parser_abort_tentative_parse (parser);
> > +  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
> > + declarator-id of a constructor or destructor.  */
> > +  if (constructor_p && saw_template_id)
> > +{
> > +  gcc_checking_assert
> > +   (!cp_parser_uncommitted_to_tentative_parse_p (parser));
> 
> Now I see the abort_ just above, so this seems unnecessary after all.

Done, thanks.

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

-- >8 --
Since my r11-532 changes to implement DR2237, for this test:

  template
  struct S {
S();
  };

in C++20 we emit the ugly:

q.C:3:8: error: expected unqualified-id before ')' token
3 |   S();

which doesn't explain what the problem is.  This patch improves that
diagnostic, reduces the error to a pedwarn, and adds a -Wc++20-compat
diagnostic.  We now say:

q.C:3:7: warning: template-id not allowed for constructor in C++20 
[-Wtemplate-id-cdtor]
3 |   S();
q.C:3:7: note: remove the '< >'

This patch also fixes

where the C++20 diagnostic was missing altogether: The problem was that I 
checked
for CPP_TEMPLATE_ID too early, at a point at which cp_parser_template_id may not
have been called yet.  So let's check for it at the end of the function, after
the tentative parse and rollback.

-Wc++20-compat triggered in libitm/; I sent a patch for that.

DR 2237
PR c++/107126
PR c++/97202

gcc/c-family/ChangeLog:

* c-opts.cc (c_common_post_options): In C++20 or with -Wc++20-compat,
turn on -Wtemplate-id-cdtor.
* c.opt (Wtemplate-id-cdtor): New.

gcc/cp/ChangeLog:

* parser.cc (cp_parser_unqualified_id): Downgrade the DR2237 error to
a pedwarn.
(cp_parser_constructor_declarator_p): Likewise.

gcc/ChangeLog:

* doc/invoke.texi: Document -Wtemplate-id-cdtor.

gcc/testsuite/ChangeLog:

* g++.dg/DRs/dr2237.C: Adjust dg-error.
* g++.dg/parse/constructor2.C: Likewise.
* g++.dg/template/error34.C: Likewise.
* g++.old-deja/g++.pt/ctor2.C: Likewise.
* g++.dg/DRs/dr2237-2.C: New test.
* g++.dg/DRs/dr2237-3.C: New test.
* g++.dg/DRs/dr2237-4.C: New test.
* g++.dg/DRs/dr2237-5.C: New test.
* g++.dg/warn/Wtemplate-id-cdtor-1.C: New test.
* g++.dg/warn/Wtemplate-id-cdtor-2.C: New test.
* g++.dg/warn/Wtemplate-id-cdtor-3.C: New test.
* g++.dg/warn/Wtemplate-id-cdtor-4.C: New test.
---
 gcc/c-family/c-opts.cc|  5 +++
 gcc/c-family/c.opt|  4 +++
 gcc/cp/parser.cc  | 33 ++-
 gcc/doc/invoke.texi   | 19 ++-
 gcc/testsuite/g++.dg/DRs/dr2237-2.C   |  9 +
 gcc/testsuite/g++.dg/DRs/dr2237-3.C   | 16 +
 gcc/testsuite/g++.dg/DRs/dr2237-4.C   | 11 +++
 gcc/testsuite/g++.dg/DRs/dr2237-5.C   |  7 
 gcc/testsuite/g++.dg/DRs/dr2237.C |  2 +-
 gcc/testsuite/g++.dg/parse/constructor2.C | 16 -
 gcc/testsuite/g++.dg/template/error34.C   | 10 +++---
 .../g++.dg/warn/Wtemplate-id-cdtor-1.C|  9 +
 .../g++.dg/warn/Wtemplate-id-cdtor-2.C|  9 +
 .../g++.dg/warn/Wtemplate-id-cdtor-3.C|  9 +
 

Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-09 Thread Iain Sandoe



> On 9 Feb 2024, at 10:56, Iain Sandoe  wrote:
>> On 8 Feb 2024, at 21:44, Jason Merrill  wrote:
>> 
>> On 2/8/24 12:55, Paolo Bonzini wrote:
>>> On 2/8/24 18:16, Jason Merrill wrote:
>> 
> 
> Hmm.  In stage 1, when we build with the system gcc, I'd think we want 
> the just-built gnat1 to find the system libgcc.
> 
> In stage 2, when we build with the stage 1 gcc, we want the just-built 
> gnat1 to find the stage 1 libgcc.
> 
> In neither case do we want it to find the libgcc from the current stage.
> 
> So it seems to me that what we want is for stage2+ LD_LIBRARY_PATH to 
> include the TARGET_LIB_PATH from the previous stage.  Something like the 
> below, on top of the earlier patch.
> 
> Does this make sense?  Does it work on Darwin?
 
 Oops, that was broken, please consider this one instead:
>>> Yes, this one makes sense (and the current code would not work since it 
>>> lacks the prev- prefix on TARGET_LIB_PATH).
>> 
>> Indeed, that seems like evidence that the only element of TARGET_LIB_PATH 
>> that has been useful in HOST_EXPORTS is the prev- part of HOST_LIB_PATH_gcc.
>> 
>> So, here's another patch that just includes that for post-stage1:
>> <0001-build-drop-target-libs-from-LD_LIBRARY_PATH-PR105688.patch>
> 
> Hmm this still fails for me with gnat1 being unable to find libgcc_s.
> It seems I have to add the PREV_HOST_LIB_PATH_gcc to HOST_LIB_PATH for it to 
> succeed so,
> presumably, the post stage1 exports are not being forwarded to that build.  
> I’ll try to analyze what
> exactly is failing.

The fail is occurring in the target libada build; so, I suppose, one might say 
it’s reasonable that it
requires this host path to be added to the target exports since it’s a host 
library used during target
builds (or do folks expect the host exports to be made for target lib builds as 
well?)

Appending the prev-gcc dirctory to the HOST_LIB_PATH fixes this because 
HOST_LIB_PATH is
appended to the TARGET_EXPORTS (we do not seem to make those depend on the 
stage).

Iain




Re: [PATCH] Notes on the warnings-as-errors change in GCC 14

2024-02-09 Thread Gerald Pfeifer
On Fri, 2 Feb 2024, Florian Weimer wrote:
> +Certain warnings are now errors

That's quite a nice description, thank you, Florian!

> +The initial ISO C standard and its 1999 revision removed support for

May I suggest to wrap paragraphs in ...? Not strictly necessary any 
more, now that we switched to HTML 5, though more consistent and explicit.

> [For backwards]
> +compatibility, GCC 13 and earlier compiler versions diagnosed use of
> +these features as warnings only.  Although these warnings have been
> +enabled by default for many releases, experience shows that these
> +warnings are easily ignored, resulting in difficult-to-diagnose bugs.

Can we just say "GCC 13 and earlier" (or "GCC 13 and earlier versions")?

And "...shows that they are easily ignored, resulting in difficult to 
diagnose bugs"?

> +Several GNU/Linux distributions have shared patches from their porting
> +efforts on relevant upstream mailing lists and bug trackers, but of
> +course there is a strong overlap between programs that have these
> +historic C compatibility issues and are dormant today.

At first I thought something is missing here, then I realized it may be a 
difference between German (which also is my mother tongue) and English.

One way to address it would be saying "and those that are dormant", though
I believe something like

  ...but of course many programs that exhibit these historic C 
  compatibility issues are dormant today.

> +In most cases, simply adding the missing int keyword
> +addresses the error.  For example, a flag like
> +
> +
> +  static initialized;
> +
> +
> +might turn into:
> +
> +
> +  static int initialized;
> +

How about "For example, a variable like ... becomes"?

> +If the return type of a function is omitted, the correct type to
> +add could be int or void, depending on
> +whether the function returns a value or not.

"can be"

> +In some cases, the previously implied int type
> +was not correct.

I'd omit the comma here.

>  This mostly happens in function definitions
> +that are not prototypes

Naive questions: Can definitions really be prototypes (in C)?

> +declared outside the parameter list.  Using the correct
> +type maybe required to avoid int-conversion errors (see below).

Something feels odd with this sentence?

> +
> +  void
> +  write_string (int fd, const char *s)

const char *s) as well?

> +Some programs are built with -std=c11 or
> +similar -std options that do not contain the
> +string gnu, but these programs still use POSIX or other

...but still use... (omit "these programs")

> +If undeclared functions from the same project are called and there is
> +yet no suitable shared header file, you should add a declaration to a

"no suitable shared header file yet, you can add..."

> +However, this is an obsolete C feature that will change meaning in C23
> +(declaration a function with a prototype and accepting no arguments,
> +similar to C++).

"declaration of a function", or some other change here?

> +Incorrectly spelled type names in function declarations are treated as
> +errors in more cases, under a
> +new -Wdeclaration-missing-parameter-type warning.  The
> +second line in the following example is now treated as an error
> +(previously this resulted in an unnamed warning):

What is an "unnamed" warning? Can we simply omit "unnamed" here?

> +GCC will type-check function arguments after that, potentially
> +requiring further changes.  (Previously, the function declaration was
> +treated as not having no prototype.)

That second sentence uses double negation, which logically is the same as 
just the original statement.

> +
> +By default, GCC still accepts returning an expression of
> +type void from within a function that itself
> +returns void, as a GNU extension that matches C++ rules
> +in this area.

Does the GNU extension match C++ (standard rules)?


Wee, this is a patch. All the more kudos to you.. 

I'll be looking into the second half of the patch tomorrow; feel free to 
commit the aspects covered above (after considering my review).

Cheers,




Re: [PATCH 2/2] c++/modules: ICE with modular fmtlib

2024-02-09 Thread Patrick Palka
On Fri, 9 Feb 2024, Patrick Palka wrote:

> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> OK for trunk?
> 
> I'll try to reduce and add testcases overnight for these separate bugs
> before pushing.
> 
> -- >8 --
> 
> Building modular fmtlib triggered two small modules bugs in C++23 and
> C++26 mode respectively (due to libstdc++ header differences).
> 
> The first is that a TEMPLATE_DECL having DECL_LANG_SPECIFIC doesn't
> necessarily imply that its DECL_TEMPLATE_RESULT has DECL_LANG_SPECIFIC.
> So we need to use STRIP_TEMPLATE consistently; this is a follow-up to
> r12-7187-gdb84f382ae3dc2.
> 
> The second is that get_originating_module_decl was ICEing on class-scope
> enumerators injected via using-enum.
> 
> gcc/cp/ChangeLog:
> 
>   * module.cc (depset::hash::add_specializations): Use
>   STRIP_TEMPLATE consistently.
>   (get_originating_module_decl): Handle class-scope CONST_DECL.
> ---
>  gcc/cp/module.cc | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 3c2fef0e3f4..659fa03dae1 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -13248,7 +13248,7 @@ depset::hash::add_specializations (bool decl_p)
>if (use_tpl == 1)
>   /* Implicit instantiations only walked if we reach them.  */
>   needs_reaching = true;
> -  else if (!DECL_LANG_SPECIFIC (spec)
> +  else if (!DECL_LANG_SPECIFIC (STRIP_TEMPLATE (spec))
>  || !DECL_MODULE_PURVIEW_P (STRIP_TEMPLATE (spec)))
>   /* Likewise, GMF explicit or partial specializations.  */
>   needs_reaching = true;
> @@ -18708,7 +18708,8 @@ get_originating_module_decl (tree decl)
>&& (TREE_CODE (DECL_CONTEXT (decl)) == ENUMERAL_TYPE))
>  decl = TYPE_NAME (DECL_CONTEXT (decl));
>else if (TREE_CODE (decl) == FIELD_DECL
> -|| TREE_CODE (decl) == USING_DECL)
> +|| TREE_CODE (decl) == USING_DECL
> +|| TREE_CODE (decl) == CONST_DECL)

On second thought maybe we should test for CONST_DECL_USING_P (decl)
specifically.  In other contexts a CONST_DECL could be a template
parameter, so using CONST_DECL_USING_P makes this code more readable
arguably.

>  {
>decl = DECL_CONTEXT (decl);
>if (TREE_CODE (decl) != FUNCTION_DECL)
> -- 
> 2.43.0.561.g235986be82
> 
> 



Re: [PATCH] c++: avoid name lookup during mangling

2024-02-09 Thread Patrick Palka
On Fri, 9 Feb 2024, Jason Merrill wrote:

> On 2/9/24 11:55, Patrick Palka wrote:
> > On Fri, 9 Feb 2024, Jason Merrill wrote:
> > 
> > > On 2/9/24 10:51, Patrick Palka wrote:
> > > > It turns out that with modules we can call mangle_decl recursively,
> > > > which is a problem because the global mangling state isn't recursion
> > > > aware.  The recursion happens from write_closure_type_name, which calls
> > > > lambda function, which performs name lookup, which can trigger lazy
> > > > loading,
> > > 
> > > Hmm, why would looking for a member of a closure trigger lazy loading?
> > 
> > No idea..  We could sidestep lazy loading here by using
> > get_class_binding_direct instead of lookup_member in lambda_function,
> > but I don't know if if that would have unintended consequences.
> 
> Seems worth trying; the lazy load should still get triggered by some other
> name lookup.

Makes sense.

> 
> > FWIW the closure for which op() lookup triggers lazy loading is for a
> > non-generic lambda from an instantiated function template scope and
> > captures another closure object that captures a dependent function
> > parameter (_overwrite in libstdc++'s  line 1607).  So I'd guess
> > the entity dependency graph is pretty complicated..
> > 
> > And it seems lazy loading from maybe_lazily_declare can be quite
> > recursive, I wonder if that's expected?  I attached the full backtrace
> > that leads mangle_decl recursion, and there's about 15 recursive calls
> > to maybe_lazily_declare in this backtrace which seem to go through
> > check_local_shadow.  Maybe we can reduce this somehow?
> 
> I think we shouldn't bother with check_local_shadow in a clone or
> instantiation, only when actually parsing.

Ah, this idea seems to work very nicely.  I posted a new patch
implementing them at
https://gcc.gnu.org/pipermail/gcc-patches/2024-February/645307.html (and
a follow-up patch fixing two small bugs caught by testing fmtlib with
newer -std).



[PATCH 2/2] c++/modules: ICE with modular fmtlib

2024-02-09 Thread Patrick Palka
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for trunk?

I'll try to reduce and add testcases overnight for these separate bugs
before pushing.

-- >8 --

Building modular fmtlib triggered two small modules bugs in C++23 and
C++26 mode respectively (due to libstdc++ header differences).

The first is that a TEMPLATE_DECL having DECL_LANG_SPECIFIC doesn't
necessarily imply that its DECL_TEMPLATE_RESULT has DECL_LANG_SPECIFIC.
So we need to use STRIP_TEMPLATE consistently; this is a follow-up to
r12-7187-gdb84f382ae3dc2.

The second is that get_originating_module_decl was ICEing on class-scope
enumerators injected via using-enum.

gcc/cp/ChangeLog:

* module.cc (depset::hash::add_specializations): Use
STRIP_TEMPLATE consistently.
(get_originating_module_decl): Handle class-scope CONST_DECL.
---
 gcc/cp/module.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 3c2fef0e3f4..659fa03dae1 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -13248,7 +13248,7 @@ depset::hash::add_specializations (bool decl_p)
   if (use_tpl == 1)
/* Implicit instantiations only walked if we reach them.  */
needs_reaching = true;
-  else if (!DECL_LANG_SPECIFIC (spec)
+  else if (!DECL_LANG_SPECIFIC (STRIP_TEMPLATE (spec))
   || !DECL_MODULE_PURVIEW_P (STRIP_TEMPLATE (spec)))
/* Likewise, GMF explicit or partial specializations.  */
needs_reaching = true;
@@ -18708,7 +18708,8 @@ get_originating_module_decl (tree decl)
   && (TREE_CODE (DECL_CONTEXT (decl)) == ENUMERAL_TYPE))
 decl = TYPE_NAME (DECL_CONTEXT (decl));
   else if (TREE_CODE (decl) == FIELD_DECL
-  || TREE_CODE (decl) == USING_DECL)
+  || TREE_CODE (decl) == USING_DECL
+  || TREE_CODE (decl) == CONST_DECL)
 {
   decl = DECL_CONTEXT (decl);
   if (TREE_CODE (decl) != FUNCTION_DECL)
-- 
2.43.0.561.g235986be82



[PATCH 1/2] c++/modules: reduce lazy loading recursion

2024-02-09 Thread Patrick Palka
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for trunk

-- >8 --

It turns out that with modules we can call mangle_decl recursively,
which is a problem because the global mangling state isn't recursion
aware.  The recursion happens from write_closure_type_name, which calls
lambda_function, which performs name lookup, which can trigger lazy
loading, which can call maybe_clone_body for a newly loaded cdtor, which
can inspect DECL_ASSEMBLER_NAME, which enters mangling.  This was observed
when using fmtlib as a module with trunk and it leads to a bogus
"mangling conflicts with a previous mangle error" followed by an ICE
from cdtor_comdat_group due to a mangling mismatch.

This patch fixes this by sidestepping lazy loading when performing the
op() lookup in lambda_function, so that we don't accidentally end up
entering mangling recursively.  This should be safe since the lazy load
should still get triggered by some other name lookup.

In passing it was noticed that lazy loading can get quite recursive
ultimately due to the name lookups from check_local_shadow, which may
perform lazy loading, which cause us to instantiate/clone things, which
end up calling check_local_shadow.  This patch fixes this by
implementating an optimization suggested by Jason:

> I think we shouldn't bother with check_local_shadow in a clone or
> instantiation, only when actually parsing.

This reduces the maximum depth of lazy loading recursion for a simple
modular Hello World from ~115 to ~12.

gcc/cp/ChangeLog:

* lambda.cc (lambda_function): Call get_class_binding_direct
instead of lookup_member to sidestep lazy loading.
* name-lookup.cc (check_local_shadow): Punt if we're in a
function context that's definitely not actual parsing.
---
 gcc/cp/lambda.cc  |  4 +---
 gcc/cp/name-lookup.cc | 17 +
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
index 1d37e5a52b9..4b1f9391fee 100644
--- a/gcc/cp/lambda.cc
+++ b/gcc/cp/lambda.cc
@@ -175,9 +175,7 @@ lambda_function (tree lambda)
   if (CLASSTYPE_TEMPLATE_INSTANTIATION (type)
   && !COMPLETE_OR_OPEN_TYPE_P (type))
 return NULL_TREE;
-  lambda = lookup_member (type, call_op_identifier,
- /*protect=*/0, /*want_type=*/false,
- tf_warning_or_error);
+  lambda = get_class_binding_direct (type, call_op_identifier);
   if (lambda)
 lambda = STRIP_TEMPLATE (get_first_fn (lambda));
   return lambda;
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index e58f3b5cb4d..ca5ba87bc76 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -3275,6 +3275,23 @@ check_local_shadow (tree decl)
   if (TREE_CODE (decl) == PARM_DECL && !DECL_CONTEXT (decl))
 return NULL_TREE;
 
+  if (DECL_FUNCTION_SCOPE_P (decl))
+{
+  tree ctx = DECL_CONTEXT (decl);
+  if (DECL_CLONED_FUNCTION_P (ctx)
+ || DECL_TEMPLATE_INSTANTIATED (ctx)
+ || (DECL_LANG_SPECIFIC (ctx)
+ && DECL_DEFAULTED_FN (ctx))
+ || (LAMBDA_FUNCTION_P (ctx)
+ && LAMBDA_EXPR_REGEN_INFO (CLASSTYPE_LAMBDA_EXPR
+(DECL_CONTEXT (ctx)
+   /* It suffices to check shadowing only when actually parsing.
+  So punt for clones, instantiations, defaulted functions and
+  regenerated lambdas.  This optimization helps lazy loading
+  cascades with modules.  */
+   return NULL_TREE;
+}
+
   tree old = NULL_TREE;
   cp_binding_level *old_scope = NULL;
   if (cxx_binding *binding = outer_binding (DECL_NAME (decl), NULL, true))
-- 
2.43.0.561.g235986be82



Re: [PATCH] call maybe_return_this in build_clone

2024-02-09 Thread Torbjorn SVENSSON




On 2024-02-09 19:57, Jason Merrill wrote:

On 2/7/24 11:24, Torbjorn SVENSSON wrote:

Hi,

Is it okay to backport 0d24289d129639efdc79338a64188d6d404375e8 to 
releases/gcc-13?


OK.


Pushed as 583bd84075d23cde5fd489ab726093060870d773.


Re: [RFC] GCC Security policy

2024-02-09 Thread Joseph Myers
On Fri, 9 Feb 2024, Siddhesh Poyarekar wrote:

> > I think disallowing running as root would be a big problem in practice -
> > the typical problem case is when people build software as non-root and run
> > "make install" as root, and for some reason "make install" wants to
> > (re)build or (re)link something.
> 
> Isn't that a problematic practice though?  Or maybe have those invocations be
> separated out as CC_ROOT?

Ideally dependencies would be properly set up so that everything is built 
in the original build, and ideally there would be no need to relink at 
install time (I'm not sure of the exact circumstances in which it might be 
needed, or on what OSes to e.g. encode the right library paths in final 
installed executables).  In practice I think it's common for some building 
to take place at install time.

There is a more general principle here of composability: it's not helpful 
for being able to write scripts or makefiles combining invocations of 
different utilities and have them behave predictably if some of those 
utilities start making judgements about whether it's a good idea to run 
them in a particular environment rather than just doing their job 
independent of irrelevant aspects of the environment.  The semantics of 
invoking "gcc" have nothing to do with whether it's run as root; it should 
never need to look up what user it's running as at all.  (And it's 
probably also a bad idea for lots of separate utilities to gain their own 
ways to run in a restricted environment, for similar reasons; rather than 
teaching "gcc" a way to create a restricted environment itself, ensure 
there are easy-to-use more general utilities for running arbitrary 
programs on untrusted input in a contained environment.)

-- 
Joseph S. Myers
josmy...@redhat.com



Re: [PATCH] call maybe_return_this in build_clone

2024-02-09 Thread Jason Merrill

On 2/7/24 11:24, Torbjorn SVENSSON wrote:

Hi,

Is it okay to backport 0d24289d129639efdc79338a64188d6d404375e8 to 
releases/gcc-13?


OK.


Without this backport, I see these failures on arm-none-eabi:

FAIL: g++.dg/warn/Wuse-after-free3.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/warn/Wuse-after-free3.C  -std=gnu++17 (test for excess errors)
FAIL: g++.dg/warn/Wuse-after-free3.C  -std=gnu++20 (test for excess errors)

Kind regards,
Torbjörn

On 2023-11-22 09:26, Alexandre Oliva wrote:

On Nov 20, 2023, Jason Merrill  wrote:


So it only passed on that platform before because of the bug?


I'm not sure it passed (we've had patches for that testcase before), but
I didn't look closely enough into their history to tell.  I suspected
the warning suppression machinery changed, or details on cloning did, or
something.  It's been fragile historically.  But yeah, recently, the
test for the warning was only passing because of the bug.  But we were
also getting excess warnings, so it wasn't fully passing.


Thanks for the reviews!







Re: [PATCH] c++: fix ICE with __type_pack_element [PR113834]

2024-02-09 Thread Jason Merrill

On 2/9/24 13:32, Marek Polacek wrote:

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


OK.


-- >8 --
Here we crash on this invalid code because we seem to infinitely recurse
and end up with __type_pack_element with index that doesn't tree_fits_shwi_p
which then crashes on tree_to_shwi.

Thanks to Jakub for suggesting a nicer fix than my original one.

PR c++/113834

gcc/cp/ChangeLog:

* semantics.cc (finish_type_pack_element): Perform range checking
before tree_to_shwi.

gcc/testsuite/ChangeLog:

* g++.dg/ext/type_pack_element4.C: New test.
---
  gcc/cp/semantics.cc   |  7 +++
  gcc/testsuite/g++.dg/ext/type_pack_element4.C | 17 +
  2 files changed, 20 insertions(+), 4 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/ext/type_pack_element4.C

diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 3299e270446..57840176863 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -4650,20 +4650,19 @@ finish_type_pack_element (tree idx, tree types, 
tsubst_flags_t complain)
error ("%<__type_pack_element%> index is not an integral constant");
return error_mark_node;
  }
-  HOST_WIDE_INT val = tree_to_shwi (idx);
-  if (val < 0)
+  if (tree_int_cst_sgn (idx) < 0)
  {
if (complain & tf_error)
error ("%<__type_pack_element%> index is negative");
return error_mark_node;
  }
-  if (val >= TREE_VEC_LENGTH (types))
+  if (wi::to_widest (idx) >= TREE_VEC_LENGTH (types))
  {
if (complain & tf_error)
error ("%<__type_pack_element%> index is out of range");
return error_mark_node;
  }
-  return TREE_VEC_ELT (types, val);
+  return TREE_VEC_ELT (types, tree_to_shwi (idx));
  }
  
  /* Implement the __direct_bases keyword: Return the direct base classes

diff --git a/gcc/testsuite/g++.dg/ext/type_pack_element4.C 
b/gcc/testsuite/g++.dg/ext/type_pack_element4.C
new file mode 100644
index 000..aa508c79090
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/type_pack_element4.C
@@ -0,0 +1,17 @@
+// PR c++/113834
+// { dg-do compile { target c++17 } }
+
+template  class tuple{};
+template 
+__type_pack_element<__i, _Elements...> (tuple<_Elements...> &__t) noexcept; // { 
dg-error "index is out of range" }
+tuple data;
+template 
+unsigned take_impl(unsigned idx) {
+  if constexpr (Level != -1){
+return take_impl(get(data)); // { dg-error "" }
+  }
+  return 0;
+}
+int main() {
+  take_impl<2>(0);
+}

base-commit: c9bdcb0c3433ce09f5bb713a51a14130858578a2




Re: [PATCH v3] c++: make build_throw SFINAE-friendly [PR98388]

2024-02-09 Thread Jason Merrill

On 2/9/24 13:31, Marek Polacek wrote:

On Fri, Feb 09, 2024 at 10:07:33AM -0500, Jason Merrill wrote:

On 2/8/24 17:20, Marek Polacek wrote:

On Thu, Feb 08, 2024 at 04:53:45PM -0500, Jason Merrill wrote:

On 2/8/24 11:51, Marek Polacek wrote:

On Thu, Feb 08, 2024 at 08:49:28AM -0500, Patrick Palka wrote:

On Wed, 7 Feb 2024, Marek Polacek wrote:


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

-- >8 --
Here the problem is that we give hard errors while substituting
template parameters during overload resolution of is_throwable
which has an invalid throw in decltype.

The backtrace shows that fn_type_unification -> instantiate_template
-> tsubst* passes complain=0 as expected, but build_throw doesn't
have a complain parameter.  So let's add one.  Also remove a redundant
local variable which I should have removed in my P2266 patch.

But there's still something not quite clear to me.  I claim that 'b'
in the testcase should evaluate to false since the first overload ought
to have been discarded.  EDG 6.6 agrees, but clang++, msvc, and icx evaluate
it to true.  Who's right?


I think it should be true since P1155, which we implement in C++20 mode and
above (or rather, we implement the sequel P2266); since then we implicitly
move from the function parameter.

The patch looks good except that we should test this expected value.


I could add
#if __cplusplus >= 202002L
static_assert (b, "move from the function parameter");
#else
static_assert (!b, "no move from the function parameter");
#endif

but that's going to fail for C++20 and above.


Ah, because treat_lvalue_as_rvalue_p doesn't recognize t as being from the
current scope in the trailing return type.  But that shouldn't be necessary:

https://eel.is/c++draft/expr.prim.id.unqual#4.2 says it's move-eligible
"if the id-expression (possibly parenthesized) is the operand of a
throw-expression ([expr.throw]), and names an implicitly movable entity that
belongs to a scope that does not contain the compound-statement of the
innermost lambda-expression, try-block, or function-try-block (if any) whose
compound-statement or ctor-initializer contains the throw-expression."

here there is no enclosing lambda or try, so it seems move-eligible.


...and as a result, 't' will be an xvalue, therefore moveonly(moveonly&&)
should be used, therefore the first overload should be used, which means 'b'
is true.


Yep.


I wonder if this is the
second half of the problem in 113789?

I could comment the first static_assert and add a FIXME if that sounds good?


dg-bogus would be better than commenting it out.


Ok, in this patch the only change I made is:

+#if __cplusplus >= 202002L
+static_assert (b, "move from the function parameter"); // { dg-bogus "" 
"PR113853" { xfail c++20 } }
+#else
+static_assert (!b, "no move from the function parameter");
+#endif


OK.


Will you also look into fixing the treat_ bug?  That can be a separate
patch.


I'd like to.  I opened .

-- >8 --
Here the problem is that we give hard errors while substituting
template parameters during overload resolution of is_throwable
which has an invalid throw in decltype.

The backtrace shows that fn_type_unification -> instantiate_template
-> tsubst* passes complain=0 as expected, but build_throw doesn't
have a complain parameter.  So let's add one.  Also remove a redundant
local variable which I should have removed in my P2266 patch.

There's still one problem for which I opened .
We need to patch up treat_lvalue_as_rvalue_p and remove the dg-bogus.

Thanks to Patrick for notifying me of this PR.  This doesn't fully fix
113789; there I think I'll have to figure our why a candidate wasn't
discarded from the overload set.

gcc/cp/ChangeLog:

* coroutines.cc (coro_rewrite_function_body): Pass tf_warning_or_error
to build_throw.
(morph_fn_to_coro): Likewise.
* cp-tree.h (build_throw): Adjust.
* except.cc (expand_end_catch_block): Pass tf_warning_or_error to
build_throw.
(build_throw): Add a tsubst_flags_t parameter.  Use it.  Remove
redundant variable.  Guard an inform call.
* parser.cc (cp_parser_throw_expression): Pass tf_warning_or_error
to build_throw.
* pt.cc (tsubst_expr) : Pass complain to build_throw.

libcc1/ChangeLog:

* libcp1plugin.cc (plugin_build_unary_expr): Pass tf_error to
build_throw.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/sfinae69.C: New test.
---
  gcc/cp/coroutines.cc  |  4 ++--
  gcc/cp/cp-tree.h  |  3 ++-
  gcc/cp/except.cc  | 33 ---
  gcc/cp/parser.cc  |  2 +-
  gcc/cp/pt.cc  |  2 +-
  gcc/testsuite/g++.dg/cpp0x/sfinae69.C | 21 +
  libcc1/libcp1plugin.cc|  4 ++--
  7 files changed, 43 insertions(+), 26 deletions(-)
  create mode 100644 

[PATCH] c++: fix ICE with __type_pack_element [PR113834]

2024-02-09 Thread Marek Polacek
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
Here we crash on this invalid code because we seem to infinitely recurse
and end up with __type_pack_element with index that doesn't tree_fits_shwi_p
which then crashes on tree_to_shwi.

Thanks to Jakub for suggesting a nicer fix than my original one.

PR c++/113834

gcc/cp/ChangeLog:

* semantics.cc (finish_type_pack_element): Perform range checking
before tree_to_shwi.

gcc/testsuite/ChangeLog:

* g++.dg/ext/type_pack_element4.C: New test.
---
 gcc/cp/semantics.cc   |  7 +++
 gcc/testsuite/g++.dg/ext/type_pack_element4.C | 17 +
 2 files changed, 20 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ext/type_pack_element4.C

diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 3299e270446..57840176863 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -4650,20 +4650,19 @@ finish_type_pack_element (tree idx, tree types, 
tsubst_flags_t complain)
error ("%<__type_pack_element%> index is not an integral constant");
   return error_mark_node;
 }
-  HOST_WIDE_INT val = tree_to_shwi (idx);
-  if (val < 0)
+  if (tree_int_cst_sgn (idx) < 0)
 {
   if (complain & tf_error)
error ("%<__type_pack_element%> index is negative");
   return error_mark_node;
 }
-  if (val >= TREE_VEC_LENGTH (types))
+  if (wi::to_widest (idx) >= TREE_VEC_LENGTH (types))
 {
   if (complain & tf_error)
error ("%<__type_pack_element%> index is out of range");
   return error_mark_node;
 }
-  return TREE_VEC_ELT (types, val);
+  return TREE_VEC_ELT (types, tree_to_shwi (idx));
 }
 
 /* Implement the __direct_bases keyword: Return the direct base classes
diff --git a/gcc/testsuite/g++.dg/ext/type_pack_element4.C 
b/gcc/testsuite/g++.dg/ext/type_pack_element4.C
new file mode 100644
index 000..aa508c79090
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/type_pack_element4.C
@@ -0,0 +1,17 @@
+// PR c++/113834
+// { dg-do compile { target c++17 } }
+
+template  class tuple{};
+template 
+__type_pack_element<__i, _Elements...> (tuple<_Elements...> &__t) 
noexcept; // { dg-error "index is out of range" }
+tuple data;
+template 
+unsigned take_impl(unsigned idx) {
+  if constexpr (Level != -1){
+return take_impl(get(data)); // { dg-error "" }
+  }
+  return 0;
+}
+int main() {
+  take_impl<2>(0);
+}

base-commit: c9bdcb0c3433ce09f5bb713a51a14130858578a2
-- 
2.43.0



[PATCH v3] c++: make build_throw SFINAE-friendly [PR98388]

2024-02-09 Thread Marek Polacek
On Fri, Feb 09, 2024 at 10:07:33AM -0500, Jason Merrill wrote:
> On 2/8/24 17:20, Marek Polacek wrote:
> > On Thu, Feb 08, 2024 at 04:53:45PM -0500, Jason Merrill wrote:
> > > On 2/8/24 11:51, Marek Polacek wrote:
> > > > On Thu, Feb 08, 2024 at 08:49:28AM -0500, Patrick Palka wrote:
> > > > > On Wed, 7 Feb 2024, Marek Polacek wrote:
> > > > > 
> > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > > > 
> > > > > > -- >8 --
> > > > > > Here the problem is that we give hard errors while substituting
> > > > > > template parameters during overload resolution of is_throwable
> > > > > > which has an invalid throw in decltype.
> > > > > > 
> > > > > > The backtrace shows that fn_type_unification -> instantiate_template
> > > > > > -> tsubst* passes complain=0 as expected, but build_throw doesn't
> > > > > > have a complain parameter.  So let's add one.  Also remove a 
> > > > > > redundant
> > > > > > local variable which I should have removed in my P2266 patch.
> > > > > > 
> > > > > > But there's still something not quite clear to me.  I claim that 'b'
> > > > > > in the testcase should evaluate to false since the first overload 
> > > > > > ought
> > > > > > to have been discarded.  EDG 6.6 agrees, but clang++, msvc, and icx 
> > > > > > evaluate
> > > > > > it to true.  Who's right?
> > > 
> > > I think it should be true since P1155, which we implement in C++20 mode 
> > > and
> > > above (or rather, we implement the sequel P2266); since then we implicitly
> > > move from the function parameter.
> > > 
> > > The patch looks good except that we should test this expected value.
> > 
> > I could add
> > #if __cplusplus >= 202002L
> > static_assert (b, "move from the function parameter");
> > #else
> > static_assert (!b, "no move from the function parameter");
> > #endif
> > 
> > but that's going to fail for C++20 and above.
> 
> Ah, because treat_lvalue_as_rvalue_p doesn't recognize t as being from the
> current scope in the trailing return type.  But that shouldn't be necessary:
> 
> https://eel.is/c++draft/expr.prim.id.unqual#4.2 says it's move-eligible
> "if the id-expression (possibly parenthesized) is the operand of a
> throw-expression ([expr.throw]), and names an implicitly movable entity that
> belongs to a scope that does not contain the compound-statement of the
> innermost lambda-expression, try-block, or function-try-block (if any) whose
> compound-statement or ctor-initializer contains the throw-expression."
> 
> here there is no enclosing lambda or try, so it seems move-eligible.

...and as a result, 't' will be an xvalue, therefore moveonly(moveonly&&)
should be used, therefore the first overload should be used, which means 'b'
is true.

> > I wonder if this is the
> > second half of the problem in 113789?
> > 
> > I could comment the first static_assert and add a FIXME if that sounds good?
> 
> dg-bogus would be better than commenting it out.

Ok, in this patch the only change I made is:

+#if __cplusplus >= 202002L
+static_assert (b, "move from the function parameter"); // { dg-bogus "" 
"PR113853" { xfail c++20 } }
+#else
+static_assert (!b, "no move from the function parameter");
+#endif
 
> Will you also look into fixing the treat_ bug?  That can be a separate
> patch.

I'd like to.  I opened .

-- >8 --
Here the problem is that we give hard errors while substituting
template parameters during overload resolution of is_throwable
which has an invalid throw in decltype.

The backtrace shows that fn_type_unification -> instantiate_template
-> tsubst* passes complain=0 as expected, but build_throw doesn't
have a complain parameter.  So let's add one.  Also remove a redundant
local variable which I should have removed in my P2266 patch.

There's still one problem for which I opened .
We need to patch up treat_lvalue_as_rvalue_p and remove the dg-bogus.

Thanks to Patrick for notifying me of this PR.  This doesn't fully fix
113789; there I think I'll have to figure our why a candidate wasn't
discarded from the overload set.

gcc/cp/ChangeLog:

* coroutines.cc (coro_rewrite_function_body): Pass tf_warning_or_error
to build_throw.
(morph_fn_to_coro): Likewise.
* cp-tree.h (build_throw): Adjust.
* except.cc (expand_end_catch_block): Pass tf_warning_or_error to
build_throw.
(build_throw): Add a tsubst_flags_t parameter.  Use it.  Remove
redundant variable.  Guard an inform call.
* parser.cc (cp_parser_throw_expression): Pass tf_warning_or_error
to build_throw.
* pt.cc (tsubst_expr) : Pass complain to build_throw.

libcc1/ChangeLog:

* libcp1plugin.cc (plugin_build_unary_expr): Pass tf_error to
build_throw.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/sfinae69.C: New test.
---
 gcc/cp/coroutines.cc  |  4 ++--
 gcc/cp/cp-tree.h  |  3 ++-
 gcc/cp/except.cc

Re: [PATCH v4]: testcases for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

2024-02-09 Thread Jason Merrill

On 2/9/24 11:02, Hans-Peter Nilsson wrote:

Oops, I managed to send a version that only added a comment,
but still had a dg-do run.  Anyway, here's v4: actually
change the "dg-do run", not just adding a comment.  Sending
as a self-contained fresh patch for the benefit of
aforementioned CI.  See v2 and v3 for more.  Sorry!

Ok to commit?


OK.


-- >8 --

Test-cases, with constexpr-reinterpret3.C dg-ice:ing the PR c++/113545 bug.

Regarding the request in the comment, A dg-do run when there's an ICE
will cause some CI's to signal an error for the run being "UNRESOLVED"
(compilation failed to produce executable).  Note that dejagnu (1.6.3)
itself doesn't consider this an error.

gcc/testsuite:
PR c++/113545
* g++.dg/cpp1y/constexpr-reinterpret3.C,
g++.dg/cpp1y/constexpr-reinterpret4.C: New tests.
---
  .../g++.dg/cpp1y/constexpr-reinterpret3.C | 56 +++
  .../g++.dg/cpp1y/constexpr-reinterpret4.C | 54 ++
  2 files changed, 110 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C

diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
new file mode 100644
index ..51feb2e558e7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
@@ -0,0 +1,56 @@
+// PR c++/113545
+// { dg-do compile { target c++14 } }
+// Please change the above "dg-do compile" to "dg-do run" when the ICE is 
resolved.
+// { dg-ice "PR112545 - constexpr function with switch called for 
reinterpret_cast" }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned const char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(c);
+  unsigned const char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(d);
+  unsigned const char e = swbar((__UINTPTR_TYPE__) );
+  xyzzy(e);
+  unsigned const char f = ifbar((__UINTPTR_TYPE__) );
+  xyzzy(f);
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
new file mode 100644
index ..9aaa6e463bc6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
@@ -0,0 +1,54 @@
+// PR c++/113545
+// { dg-do compile { target c++14 } }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>()); // { 
dg-error "conversion from pointer type" }
+  xyzzy(c);
+  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>()); // { 
dg-error "conversion from pointer type" }
+  xyzzy(d);
+  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) ); // { dg-error 
"conversion from pointer type" }
+  xyzzy(e);
+  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) ); // { dg-error 
"conversion from pointer type" }
+  xyzzy(f);
+}

1



Re: [PATCH] c++: avoid name lookup during mangling

2024-02-09 Thread Jason Merrill

On 2/9/24 11:55, Patrick Palka wrote:

On Fri, 9 Feb 2024, Jason Merrill wrote:


On 2/9/24 10:51, Patrick Palka wrote:

It turns out that with modules we can call mangle_decl recursively,
which is a problem because the global mangling state isn't recursion
aware.  The recursion happens from write_closure_type_name, which calls
lambda function, which performs name lookup, which can trigger lazy
loading,


Hmm, why would looking for a member of a closure trigger lazy loading?


No idea..  We could sidestep lazy loading here by using
get_class_binding_direct instead of lookup_member in lambda_function,
but I don't know if if that would have unintended consequences.


Seems worth trying; the lazy load should still get triggered by some 
other name lookup.



FWIW the closure for which op() lookup triggers lazy loading is for a
non-generic lambda from an instantiated function template scope and
captures another closure object that captures a dependent function
parameter (_overwrite in libstdc++'s  line 1607).  So I'd guess
the entity dependency graph is pretty complicated..

And it seems lazy loading from maybe_lazily_declare can be quite
recursive, I wonder if that's expected?  I attached the full backtrace
that leads mangle_decl recursion, and there's about 15 recursive calls
to maybe_lazily_declare in this backtrace which seem to go through
check_local_shadow.  Maybe we can reduce this somehow?


I think we shouldn't bother with check_local_shadow in a clone or 
instantiation, only when actually parsing.


Jason



Re: [PATCH] libstdc++: Use _GLIBCXX_USE_BUILTIN_TRAIT for is_same

2024-02-09 Thread Jonathan Wakely
On Fri, 9 Feb 2024 at 16:02, Patrick Palka  wrote:

> On Thu, 8 Feb 2024, Ken Matsui wrote:
>
> > Since is_same has a fallback native implementation, and
> > _GLIBCXX_HAVE_BUILTIN_IS_SAME does not support toggling which
> > implementation to use, we remove the _GLIBCXX_HAVE_BUILTIN_IS_SAME
> > definition and use _GLIBCXX_USE_BUILTIN_TRAIT instead.
> >
> > libstdc++-v3/ChangeLog:
> >
> >   * include/bits/c++config (_GLIBCXX_HAVE_BUILTIN_IS_SAME):
> >   Removed.
> >   * include/std/type_traits (is_same): Use
> >   _GLIBCXX_USE_BUILTIN_TRAIT instead of
> >   _GLIBCXX_HAVE_BUILTIN_IS_SAME.
> >   (is_same_v): Likewise.
>
> LGTM
>

Me too, OK for trunk, thanks.

+Reviewed-by: Jonathan Wakely 



>
> >
> > Signed-off-by: Ken Matsui 
> > ---
> >  libstdc++-v3/include/bits/c++config  | 4 
> >  libstdc++-v3/include/std/type_traits | 9 +
> >  2 files changed, 5 insertions(+), 8 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/bits/c++config
> b/libstdc++-v3/include/bits/c++config
> > index ad07ce92d5f..b57e3f338e9 100644
> > --- a/libstdc++-v3/include/bits/c++config
> > +++ b/libstdc++-v3/include/bits/c++config
> > @@ -845,10 +845,6 @@ namespace __gnu_cxx
> >  # define _GLIBCXX_HAVE_BUILTIN_IS_AGGREGATE 1
> >  #endif
> >
> > -#if _GLIBCXX_HAS_BUILTIN(__is_same)
> > -#  define _GLIBCXX_HAVE_BUILTIN_IS_SAME 1
> > -#endif
> > -
> >  #if _GLIBCXX_HAS_BUILTIN(__builtin_launder)
> >  # define _GLIBCXX_HAVE_BUILTIN_LAUNDER 1
> >  #endif
> > diff --git a/libstdc++-v3/include/std/type_traits
> b/libstdc++-v3/include/std/type_traits
> > index a9bb2806ca9..21402fd8c13 100644
> > --- a/libstdc++-v3/include/std/type_traits
> > +++ b/libstdc++-v3/include/std/type_traits
> > @@ -1470,16 +1470,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >// Type relations.
> >
> >/// is_same
> > +#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_same)
> >template
> >  struct is_same
> > -#ifdef _GLIBCXX_HAVE_BUILTIN_IS_SAME
> >  : public __bool_constant<__is_same(_Tp, _Up)>
> > +{ };
> >  #else
> > +  template
> > +struct is_same
> >  : public false_type
> > -#endif
> >  { };
> >
> > -#ifndef _GLIBCXX_HAVE_BUILTIN_IS_SAME
> >template
> >  struct is_same<_Tp, _Tp>
> >  : public true_type
> > @@ -3496,7 +3497,7 @@ template 
> >  template 
> >inline constexpr size_t extent_v<_Tp[], _Idx> = extent_v<_Tp, _Idx -
> 1>;
> >
> > -#ifdef _GLIBCXX_HAVE_BUILTIN_IS_SAME
> > +#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_same)
> >  template 
> >inline constexpr bool is_same_v = __is_same(_Tp, _Up);
> >  #else
> > --
> > 2.43.0
> >
> >
>
>


Re: [PATCH] c++: Don't advertise cxx_constexpr_string_builtins [PR113658]

2024-02-09 Thread Jason Merrill

On 2/2/24 10:45, Alex Coplan wrote:

On 02/02/2024 09:34, Marek Polacek wrote:

On Fri, Feb 02, 2024 at 10:27:23AM +, Alex Coplan wrote:

Bootstrapped/regtested on x86_64-apple-darwin, OK for trunk?

Thanks,
Alex

-- >8 --

When __has_feature was introduced for GCC 14, I included the feature
cxx_constexpr_string_builtins, since of the relevant string builtins
that GCC implements, it seems to support constexpr evaluation of those
builtins.

However, as the PR shows, GCC doesn't implement the full list of
builtins in the clang documentation.  After enumerating the builtins,
the clang docs [1] say:


Support for constant expression evaluation for the above builtins can
be detected with __has_feature(cxx_constexpr_string_builtins).


and a strict reading of this would suggest we can't really support
constexpr evaluation of a builtin if we don't implement the builtin in
the first place.

So the conservatively correct thing to do seems to be to stop
advertising the feature altogether to avoid failing to build code which
assumes the presence of this feature implies the presence of all the
builtins listed in the clang documentation.

[1] : https://clang.llvm.org/docs/LanguageExtensions.html#string-builtins

gcc/cp/ChangeLog:

PR c++/113658
* cp-objcp-common.cc (cp_feature_table): Remove entry for
cxx_constexpr_string_builtins.

gcc/testsuite/ChangeLog:

PR c++/113658
* g++.dg/ext/pr113658.C: New test.



diff --git a/gcc/cp/cp-objcp-common.cc b/gcc/cp/cp-objcp-common.cc
index f06edf04ef0..85dde0459fa 100644
--- a/gcc/cp/cp-objcp-common.cc
+++ b/gcc/cp/cp-objcp-common.cc
@@ -110,7 +110,6 @@ static constexpr cp_feature_info cp_feature_table[] =
{ "cxx_alignof", cxx11 },
{ "cxx_attributes", cxx11 },
{ "cxx_constexpr", cxx11 },
-  { "cxx_constexpr_string_builtins", cxx11 },
{ "cxx_decltype", cxx11 },
{ "cxx_decltype_incomplete_return_types", cxx11 },
{ "cxx_default_function_template_args", cxx11 },
diff --git a/gcc/testsuite/g++.dg/ext/pr113658.C 
b/gcc/testsuite/g++.dg/ext/pr113658.C
new file mode 100644
index 000..f4a34888f28
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/pr113658.C


Might be better to name this has-feature2.C

Please include
// PR c++/113658


Can do.


OK with those two testcase adjustments.

Jason



Re: [RFC] GCC Security policy

2024-02-09 Thread Siddhesh Poyarekar

On 2024-02-09 12:14, Joseph Myers wrote:

On Fri, 9 Feb 2024, Siddhesh Poyarekar wrote:


For privilege management we could add a --allow-root driver flag that allows
gcc to run as root.  Without the flag one could either outright refuse to run
or drop privileges and run.  Dropping privileges will be a bit tricky to
implement because it would need a user to drop privileges to and then there
would be the question of how to manage file access to read the compiler input
and write out the compiler output.  If there's no such user, gcc could refuse
to run as root by default.  I wonder though if from a security posture
perspective it makes sense to simply discourage running as root all the time
and not bother trying to make it work with dropped privileges and all that.
Of course it would mean that this would be less of a "project"; it'll be a
simple enough patch to refuse to run until --allow-root is specified.


I think disallowing running as root would be a big problem in practice -
the typical problem case is when people build software as non-root and run
"make install" as root, and for some reason "make install" wants to
(re)build or (re)link something.


Isn't that a problematic practice though?  Or maybe have those 
invocations be separated out as CC_ROOT?


Thanks,
Sid


Re: [RFC] GCC Security policy

2024-02-09 Thread Joseph Myers
On Fri, 9 Feb 2024, Siddhesh Poyarekar wrote:

> For privilege management we could add a --allow-root driver flag that allows
> gcc to run as root.  Without the flag one could either outright refuse to run
> or drop privileges and run.  Dropping privileges will be a bit tricky to
> implement because it would need a user to drop privileges to and then there
> would be the question of how to manage file access to read the compiler input
> and write out the compiler output.  If there's no such user, gcc could refuse
> to run as root by default.  I wonder though if from a security posture
> perspective it makes sense to simply discourage running as root all the time
> and not bother trying to make it work with dropped privileges and all that.
> Of course it would mean that this would be less of a "project"; it'll be a
> simple enough patch to refuse to run until --allow-root is specified.

I think disallowing running as root would be a big problem in practice - 
the typical problem case is when people build software as non-root and run 
"make install" as root, and for some reason "make install" wants to 
(re)build or (re)link something.

-- 
Joseph S. Myers
josmy...@redhat.com



Re: [PATCH] c++: avoid name lookup during mangling

2024-02-09 Thread Patrick Palka
On Fri, 9 Feb 2024, Jason Merrill wrote:

> On 2/9/24 10:51, Patrick Palka wrote:
> > It turns out that with modules we can call mangle_decl recursively,
> > which is a problem because the global mangling state isn't recursion
> > aware.  The recursion happens from write_closure_type_name, which calls
> > lambda function, which performs name lookup, which can trigger lazy
> > loading,
> 
> Hmm, why would looking for a member of a closure trigger lazy loading?

No idea..  We could sidestep lazy loading here by using
get_class_binding_direct instead of lookup_member in lambda_function,
but I don't know if if that would have unintended consequences.

FWIW the closure for which op() lookup triggers lazy loading is for a
non-generic lambda from an instantiated function template scope and
captures another closure object that captures a dependent function
parameter (_overwrite in libstdc++'s  line 1607).  So I'd guess
the entity dependency graph is pretty complicated..

And it seems lazy loading from maybe_lazily_declare can be quite
recursive, I wonder if that's expected?  I attached the full backtrace
that leads mangle_decl recursion, and there's about 15 recursive calls
to maybe_lazily_declare in this backtrace which seem to go through
check_local_shadow.  Maybe we can reduce this somehow?

This is just a naive experiment, but interestingly only two modules
tests fail if we remove the call to lazy_load_pendings from
maybe_lazily_declare:

  gcc/testsuite/g++/g++.sum:FAIL: g++.dg/modules/member-def-1_c.C -std=c++17  
scan-lang-dump module "Reading 1 pending entities keyed to '::frob'"
  gcc/testsuite/g++/g++.sum:FAIL: g++.dg/modules/member-def-1_c.C -std=c++2a  
scan-lang-dump module "Reading 1 pending entities keyed to '::frob'"
  gcc/testsuite/g++/g++.sum:FAIL: g++.dg/modules/member-def-1_c.C -std=c++2b  
scan-lang-dump module "Reading 1 pending entities keyed to '::frob'"
  gcc/testsuite/g++/g++.sum:FAIL: g++.dg/modules/part-6 -std=c++17 link
  gcc/testsuite/g++/g++.sum:FAIL: g++.dg/modules/part-6 -std=c++2a link
  gcc/testsuite/g++/g++.sum:FAIL: g++.dg/modules/part-6 -std=c++2b link

> 
> Jason
> 
> #0  start_mangling (entity=) at 
/home/patrick/code/gcc/gcc/cp/mangle.cc:4315
#1  mangle_decl_string (decl=decl@entry=) at /home/patrick/code/gcc/gcc/cp/mangle.cc:4415
#2  0x00b29e3b in get_mangled_id (decl=) at /home/patrick/code/gcc/gcc/cp/mangle.cc:4441
#3  mangle_decl (decl=) at 
/home/patrick/code/gcc/gcc/cp/mangle.cc:4479
#4  0x015e9d5e in decl_assembler_name (decl=) at /home/patrick/code/gcc/gcc/tree.cc:719
#5  0x00b8798f in cdtor_comdat_group (complete=, 
base=base@entry=) at 
/home/patrick/code/gcc/gcc/cp/optimize.cc:177
#6  0x00b89ec3 in maybe_clone_body (fn=fn@entry=) at /home/patrick/code/gcc/gcc/cp/optimize.cc:597
#7  0x00b40486 in post_load_processing () at 
/home/patrick/code/gcc/gcc/cp/module.cc:17548
#8  0x00b6b44f in lazy_load_pendings (decl=) at 
/home/patrick/code/gcc/gcc/cp/module.cc:19281
#9  0x00b75059 in maybe_lazily_declare (name=, klass=) at 
/home/patrick/code/gcc/gcc/cp/name-lookup.cc:2002
#10 get_class_binding (klass=, 
name=, want_type=) at 
/home/patrick/code/gcc/gcc/cp/name-lookup.cc:2038
#11 0x00c5313f in lookup_field_r (binfo=binfo@entry=, data=data@entry=0x7fff7660) at 
/home/patrick/code/gcc/gcc/cp/search.cc:1061
#12 0x00c533d7 in dfs_walk_all (data=0x7fff7660, post_fn=0x0, 
pre_fn=0xc530e0 , binfo=) at /home/patrick/code/gcc/gcc/cp/search.cc:1507
#13 lookup_member (xbasetype=, xbasetype@entry=, name=name@entry=, protect=protect@entry=2,
want_type=want_type@entry=true, complain=complain@entry=3, 
afi=afi@entry=0x0) at /home/patrick/code/gcc/gcc/cp/search.cc:1216
#14 0x00b70340 in get_class_binding (name=name@entry=, scope=scope@entry=0x7fffeee18a20) at 
/home/patrick/code/gcc/gcc/cp/name-lookup.cc:5615
#15 0x00b7a160 in outer_binding (name=, binding=binding@entry=0x0, class_p=class_p@entry=true) at 
/home/patrick/code/gcc/gcc/cp/name-lookup.cc:7715
#16 0x00b7ef2d in check_local_shadow (decl=decl@entry=) at /home/patrick/code/gcc/gcc/cp/name-lookup.cc:3280
#17 0x00b8493f in pushdecl (decl=, 
hiding=hiding@entry=false) at /home/patrick/code/gcc/gcc/cp/name-lookup.cc:4007
#18 0x00ab6791 in do_push_parm_decls (decl=decl@entry=, args=, 
nonparms=nonparms@entry=0x7fff7868)
at /home/patrick/code/gcc/gcc/cp/decl.cc:18214
#19 0x00ab6813 in store_parm_decls 
(current_function_parms=current_function_parms@entry=) at /home/patrick/code/gcc/gcc/cp/decl.cc:18259
#20 0x00ab7311 in start_preparsed_function 
(decl1=decl1@entry=, 
attrs=attrs@entry=, flags=flags@entry=1) at 
/home/patrick/code/gcc/gcc/cp/decl.cc:18115
#21 0x00b894e8 in maybe_clone_body (fn=fn@entry=) at /home/patrick/code/gcc/gcc/cp/optimize.cc:581
#22 0x00b40486 in post_load_processing () at 
/home/patrick/code/gcc/gcc/cp/module.cc:17548
#23 0x00b6b44f in 

[PATCH] RISC-V: Point our Python scripts at python3

2024-02-09 Thread Palmer Dabbelt
This builds for me, and I frequently have python-is-python3 type
packages installed so I think I've been implicitly testing it for a
while.  Looks like Kito's tested similar configurations, and the
bugzilla indicates we should be moving over.

gcc/ChangeLog:

PR 109668
* config/riscv/arch-canonicalize: Move to python3
* config/riscv/multilib-generator: Likewise
---
I am in no way a Python expert, but I think this is functionally a NOP
for the configurations I've been building/testing.  It's passing a
simple cross build.

---
 gcc/config/riscv/arch-canonicalize  | 2 +-
 gcc/config/riscv/multilib-generator | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/config/riscv/arch-canonicalize 
b/gcc/config/riscv/arch-canonicalize
index 629bed85347..8f7d040cdeb 100755
--- a/gcc/config/riscv/arch-canonicalize
+++ b/gcc/config/riscv/arch-canonicalize
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 
 # Tool for canonical RISC-V architecture string.
 # Copyright (C) 2011-2024 Free Software Foundation, Inc.
diff --git a/gcc/config/riscv/multilib-generator 
b/gcc/config/riscv/multilib-generator
index 1a957878d0c..25cb6762ea7 100755
--- a/gcc/config/riscv/multilib-generator
+++ b/gcc/config/riscv/multilib-generator
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 
 # RISC-V multilib list generator.
 # Copyright (C) 2011-2024 Free Software Foundation, Inc.
-- 
2.43.0



Re: [pushed] diagnostics, analyzer: add optional per-diagnostic property bags to SARIF

2024-02-09 Thread Joseph Myers
On Fri, 1 Dec 2023, David Malcolm wrote:

>   * diagnostic-core.h (emit_diagnostic_valist): New overload decl.

This has broken regeneration of gcc.pot (overloads can't have the message 
extracted for translation in different argument positions).

emit_diagnostic_valist used incompatibly as both 
--keyword=emit_diagnostic_valist:4
--flag=emit_diagnostic_valist:4:gcc-internal-format and 
--keyword=emit_diagnostic_valist:5
--flag=emit_diagnostic_valist:5:gcc-internal-format

See commit 6c8e584430bc5dc01b4438f3c38a2a10fcba7685 for previous fixes for 
this involving the same function, or 
40fecdd62f7d293a214dd71b81de5e0f1099bba7 and 
db30e21cbff7b9b2acd13ab83e25e3bf52f9696f for older fixes for similar 
issues.

-- 
Joseph S. Myers
josmy...@redhat.com



Re: [PATCH] c++/modules: anon union member of as-base class [PR112580]

2024-02-09 Thread Jason Merrill

On 2/1/24 12:18, Patrick Palka wrote:

Tested on x86_64-pc-linux-gnu, does this look OK for trunk?


OK.


-- >8 --

Here when streaming in the fields of the as-base version of
_Formatting_scanner we end up clobbering ANON_AGGR_TYPE_FIELD
of the anonymous union type, since it turns out this type is shared
between the original FIELD_DECL and the as-base FIELD_DECL copy (copied
during layout_class_type).  ANON_AGGR_TYPE_FIELD first gets properly set
to the original FIELD_DECL when streaming in the canonical definition of
_Formatting_scanner, and then gets overwritten to the as-base
FIELD_DECL when streaming in the the as-base definition.  This leads to
lookup_anon_field giving the wrong answer when resolving the _M_values
use.

This patch makes us avoid overwriting ANON_AGGR_TYPE_FIELD when streaming
in an as-base class definition, it should already be properly set at that
point.

PR c++/112580

gcc/cp/ChangeLog:

* module.cc (trees_in::read_class_def): When streaming in
an anonymous union field of an as-base class, don't clobber
ANON_AGGR_TYPE_FIELD.

gcc/testsuite/ChangeLog:

* g++.dg/modules/anon-3_a.H: New test.
* g++.dg/modules/anon-3_b.C: New test.
---
  gcc/cp/module.cc| 12 ++--
  gcc/testsuite/g++.dg/modules/anon-3_a.H | 21 +
  gcc/testsuite/g++.dg/modules/anon-3_b.C |  8 
  3 files changed, 39 insertions(+), 2 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/modules/anon-3_a.H
  create mode 100644 gcc/testsuite/g++.dg/modules/anon-3_b.C

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 3c2fef0e3f4..d36ba2eeeb3 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -12178,8 +12178,16 @@ trees_in::read_class_def (tree defn, tree 
maybe_template)
  
  	  if (TREE_CODE (decl) == FIELD_DECL

  && ANON_AGGR_TYPE_P (TREE_TYPE (decl)))
-   ANON_AGGR_TYPE_FIELD
- (TYPE_MAIN_VARIANT (TREE_TYPE (decl))) = decl;
+   {
+ tree anon_type = TYPE_MAIN_VARIANT (TREE_TYPE (decl));
+ if (DECL_NAME (defn) == as_base_identifier)
+   /* ANON_AGGR_TYPE_FIELD should already point to the
+  original FIELD_DECL, don't overwrite it to point
+  to the as-base FIELD_DECL copy.  */
+   gcc_checking_assert (ANON_AGGR_TYPE_FIELD (anon_type));
+ else
+   ANON_AGGR_TYPE_FIELD (anon_type) = decl;
+   }
  
  	  if (TREE_CODE (decl) == USING_DECL

  && TREE_CODE (USING_DECL_SCOPE (decl)) == RECORD_TYPE)
diff --git a/gcc/testsuite/g++.dg/modules/anon-3_a.H 
b/gcc/testsuite/g++.dg/modules/anon-3_a.H
new file mode 100644
index 000..64a6aab51dd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/anon-3_a.H
@@ -0,0 +1,21 @@
+// PR c++/112580
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+template 
+struct _Formatting_scanner {
+  union {
+int _M_values = 42;
+  };
+  virtual int _M_format_arg() { return _M_values; }
+};
+
+template 
+auto __do_vformat_to() {
+  _Formatting_scanner s;
+  return s;
+}
+
+inline auto vformat() {
+  return __do_vformat_to();
+}
diff --git a/gcc/testsuite/g++.dg/modules/anon-3_b.C 
b/gcc/testsuite/g++.dg/modules/anon-3_b.C
new file mode 100644
index 000..d676fe47536
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/anon-3_b.C
@@ -0,0 +1,8 @@
+// PR c++/112580
+// { dg-additional-options "-fmodules-ts" }
+
+import "anon-3_a.H";
+
+int main() {
+  vformat()._M_format_arg();
+}




Re: [PATCH] c++: avoid name lookup during mangling

2024-02-09 Thread Jason Merrill

On 2/9/24 10:51, Patrick Palka wrote:

It turns out that with modules we can call mangle_decl recursively,
which is a problem because the global mangling state isn't recursion
aware.  The recursion happens from write_closure_type_name, which calls
lambda function, which performs name lookup, which can trigger lazy
loading,


Hmm, why would looking for a member of a closure trigger lazy loading?

Jason



Re: [PATCH v4]: testcases for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

2024-02-09 Thread Hans-Peter Nilsson
Oops, I managed to send a version that only added a comment,
but still had a dg-do run.  Anyway, here's v4: actually
change the "dg-do run", not just adding a comment.  Sending
as a self-contained fresh patch for the benefit of
aforementioned CI.  See v2 and v3 for more.  Sorry!

Ok to commit?

-- >8 --

Test-cases, with constexpr-reinterpret3.C dg-ice:ing the PR c++/113545 bug.

Regarding the request in the comment, A dg-do run when there's an ICE
will cause some CI's to signal an error for the run being "UNRESOLVED"
(compilation failed to produce executable).  Note that dejagnu (1.6.3)
itself doesn't consider this an error.

gcc/testsuite:
PR c++/113545
* g++.dg/cpp1y/constexpr-reinterpret3.C,
g++.dg/cpp1y/constexpr-reinterpret4.C: New tests.
---
 .../g++.dg/cpp1y/constexpr-reinterpret3.C | 56 +++
 .../g++.dg/cpp1y/constexpr-reinterpret4.C | 54 ++
 2 files changed, 110 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C

diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
new file mode 100644
index ..51feb2e558e7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
@@ -0,0 +1,56 @@
+// PR c++/113545
+// { dg-do compile { target c++14 } }
+// Please change the above "dg-do compile" to "dg-do run" when the ICE is 
resolved.
+// { dg-ice "PR112545 - constexpr function with switch called for 
reinterpret_cast" }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned const char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(c);
+  unsigned const char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(d);
+  unsigned const char e = swbar((__UINTPTR_TYPE__) );
+  xyzzy(e);
+  unsigned const char f = ifbar((__UINTPTR_TYPE__) );
+  xyzzy(f);
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
new file mode 100644
index ..9aaa6e463bc6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
@@ -0,0 +1,54 @@
+// PR c++/113545
+// { dg-do compile { target c++14 } }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>()); 
// { dg-error "conversion from pointer type" }
+  xyzzy(c);
+  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>()); 
// { dg-error "conversion from pointer type" }
+  xyzzy(d);
+  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) ); // { dg-error 
"conversion from pointer type" }
+  xyzzy(e);
+  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) ); // { dg-error 
"conversion from pointer type" }
+  xyzzy(f);
+}
-- 
2.30.2


> From: Hans-Peter Nilsson 
> CC: , 
> Content-Type: text/plain; charset="iso-8859-1"
> Date: Fri, 9 Feb 2024 16:30:43 +0100
> 
> Bah.  Linaro's CI didn't like that there were UNRESOLVEDs
> due to this patch.  Running it "as usual" didn't show
> anything suspicious.  Sure, there were "# of unresolved
> testcases 3" in the summary (see v2), but no error or other
> special message from dejagnu.  Perhaps there could be a way
> to have dg-ice automatically downgrade a dg-do run that
> failed due to the ICE to a dg-do compile (or just not emit
> the UNRESOLVED note), but pragmatically, this is a rare
> enough case to not bother.  It looks like these were the
> only UNRESOLVEDs in that CI run, so I'm just going to make a
> mental note and move on.
> 
> For 

Re: [PATCH] libstdc++: Use _GLIBCXX_USE_BUILTIN_TRAIT for is_same

2024-02-09 Thread Patrick Palka
On Thu, 8 Feb 2024, Ken Matsui wrote:

> Since is_same has a fallback native implementation, and
> _GLIBCXX_HAVE_BUILTIN_IS_SAME does not support toggling which
> implementation to use, we remove the _GLIBCXX_HAVE_BUILTIN_IS_SAME
> definition and use _GLIBCXX_USE_BUILTIN_TRAIT instead.
> 
> libstdc++-v3/ChangeLog:
> 
>   * include/bits/c++config (_GLIBCXX_HAVE_BUILTIN_IS_SAME):
>   Removed.
>   * include/std/type_traits (is_same): Use
>   _GLIBCXX_USE_BUILTIN_TRAIT instead of
>   _GLIBCXX_HAVE_BUILTIN_IS_SAME.
>   (is_same_v): Likewise.

LGTM

> 
> Signed-off-by: Ken Matsui 
> ---
>  libstdc++-v3/include/bits/c++config  | 4 
>  libstdc++-v3/include/std/type_traits | 9 +
>  2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/libstdc++-v3/include/bits/c++config 
> b/libstdc++-v3/include/bits/c++config
> index ad07ce92d5f..b57e3f338e9 100644
> --- a/libstdc++-v3/include/bits/c++config
> +++ b/libstdc++-v3/include/bits/c++config
> @@ -845,10 +845,6 @@ namespace __gnu_cxx
>  # define _GLIBCXX_HAVE_BUILTIN_IS_AGGREGATE 1
>  #endif
>  
> -#if _GLIBCXX_HAS_BUILTIN(__is_same)
> -#  define _GLIBCXX_HAVE_BUILTIN_IS_SAME 1
> -#endif
> -
>  #if _GLIBCXX_HAS_BUILTIN(__builtin_launder)
>  # define _GLIBCXX_HAVE_BUILTIN_LAUNDER 1
>  #endif
> diff --git a/libstdc++-v3/include/std/type_traits 
> b/libstdc++-v3/include/std/type_traits
> index a9bb2806ca9..21402fd8c13 100644
> --- a/libstdc++-v3/include/std/type_traits
> +++ b/libstdc++-v3/include/std/type_traits
> @@ -1470,16 +1470,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>// Type relations.
>  
>/// is_same
> +#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_same)
>template
>  struct is_same
> -#ifdef _GLIBCXX_HAVE_BUILTIN_IS_SAME
>  : public __bool_constant<__is_same(_Tp, _Up)>
> +{ };
>  #else
> +  template
> +struct is_same
>  : public false_type
> -#endif
>  { };
>  
> -#ifndef _GLIBCXX_HAVE_BUILTIN_IS_SAME
>template
>  struct is_same<_Tp, _Tp>
>  : public true_type
> @@ -3496,7 +3497,7 @@ template 
>  template 
>inline constexpr size_t extent_v<_Tp[], _Idx> = extent_v<_Tp, _Idx - 1>;
>  
> -#ifdef _GLIBCXX_HAVE_BUILTIN_IS_SAME
> +#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_same)
>  template 
>inline constexpr bool is_same_v = __is_same(_Tp, _Up);
>  #else
> -- 
> 2.43.0
> 
> 



Re: [RFC] GCC Security policy

2024-02-09 Thread Siddhesh Poyarekar

On 2024-02-09 10:38, Martin Jambor wrote:

If anyone is interested in scoping this and then mentoring this as a
Google Summer of Code project this year then now is the right time to
speak up!


I can help with mentoring and reviews, although I'll need someone to 
assist with actual approvals.


There are two distinct sets of ideas to explore, one is privilege 
management and the other sandboxing.


For privilege management we could add a --allow-root driver flag that 
allows gcc to run as root.  Without the flag one could either outright 
refuse to run or drop privileges and run.  Dropping privileges will be a 
bit tricky to implement because it would need a user to drop privileges 
to and then there would be the question of how to manage file access to 
read the compiler input and write out the compiler output.  If there's 
no such user, gcc could refuse to run as root by default.  I wonder 
though if from a security posture perspective it makes sense to simply 
discourage running as root all the time and not bother trying to make it 
work with dropped privileges and all that.  Of course it would mean that 
this would be less of a "project"; it'll be a simple enough patch to 
refuse to run until --allow-root is specified.


This probably ties in somewhat with an idea David Malcolm had riffed on 
with me earlier, of caching files for diagnostics.  If we could unify 
file accesses somehow, we could make this happen, i.e. open/read files 
as root and then do all execution as non-root.


Sandboxing will have similar requirements, i.e. map in input files and 
an output file handle upfront and then unshare() into a sandbox to do 
the actual compilation.  This will make sure that at least the 
processing of inputs does not affect the system on which the compilation 
is being run.


Sid


[PATCH v5 4/4] Use the .ACCESS_WITH_SIZE in bound sanitizer.

2024-02-09 Thread Qing Zhao
gcc/c-family/ChangeLog:

* c-ubsan.cc (get_bound_from_access_with_size): New function.
(ubsan_instrument_bounds): Handle call to .ACCESS_WITH_SIZE.

gcc/testsuite/ChangeLog:

* gcc.dg/ubsan/flex-array-counted-by-bounds-2.c: New test.
* gcc.dg/ubsan/flex-array-counted-by-bounds-3.c: New test.
* gcc.dg/ubsan/flex-array-counted-by-bounds.c: New test.
---
 gcc/c-family/c-ubsan.cc   | 42 +
 .../ubsan/flex-array-counted-by-bounds-2.c| 45 ++
 .../ubsan/flex-array-counted-by-bounds-3.c| 34 ++
 .../ubsan/flex-array-counted-by-bounds.c  | 46 +++
 4 files changed, 167 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c
 create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-3.c
 create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c

diff --git a/gcc/c-family/c-ubsan.cc b/gcc/c-family/c-ubsan.cc
index 940982819ddf..164b29845b3a 100644
--- a/gcc/c-family/c-ubsan.cc
+++ b/gcc/c-family/c-ubsan.cc
@@ -376,6 +376,40 @@ ubsan_instrument_return (location_t loc)
   return build_call_expr_loc (loc, t, 1, build_fold_addr_expr_loc (loc, data));
 }
 
+/* Get the tree that represented the number of counted_by, i.e, the maximum
+   number of the elements of the object that the call to .ACCESS_WITH_SIZE
+   points to, this number will be the bound of the corresponding array.  */
+static tree
+get_bound_from_access_with_size (tree call)
+{
+  if (!is_access_with_size_p (call))
+return NULL_TREE;
+
+  tree ref_to_size = CALL_EXPR_ARG (call, 1);
+  unsigned int type_of_size = TREE_INT_CST_LOW (CALL_EXPR_ARG (call, 2));
+  tree type = TREE_TYPE (CALL_EXPR_ARG (call, 3));
+  tree size = fold_build2 (MEM_REF, type, unshare_expr (ref_to_size),
+  build_int_cst (ptr_type_node, 0));
+  /* If size is negative value, treat it as zero.  */
+  if (!TYPE_UNSIGNED (type))
+  {
+tree cond = fold_build2 (LT_EXPR, boolean_type_node,
+unshare_expr (size), build_zero_cst (type));
+size = fold_build3 (COND_EXPR, type, cond,
+   build_zero_cst (type), size);
+  }
+
+  /* Only when type_of_size is 1,i.e, the number of the elements of
+ the object type, return the size.  */
+  if (type_of_size != 1)
+return NULL_TREE;
+  else
+size = fold_convert (sizetype, size);
+
+  return size;
+}
+
+
 /* Instrument array bounds for ARRAY_REFs.  We create special builtin,
that gets expanded in the sanopt pass, and make an array dimension
of it.  ARRAY is the array, *INDEX is an index to the array.
@@ -401,6 +435,14 @@ ubsan_instrument_bounds (location_t loc, tree array, tree 
*index,
  && COMPLETE_TYPE_P (type)
  && integer_zerop (TYPE_SIZE (type)))
bound = build_int_cst (TREE_TYPE (TYPE_MIN_VALUE (domain)), -1);
+  else if (INDIRECT_REF_P (array)
+  && is_access_with_size_p ((TREE_OPERAND (array, 0
+   {
+ bound = get_bound_from_access_with_size ((TREE_OPERAND (array, 0)));
+ bound = fold_build2 (MINUS_EXPR, TREE_TYPE (bound),
+  bound,
+  build_int_cst (TREE_TYPE (bound), 1));
+   }
   else
return NULL_TREE;
 }
diff --git a/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c 
b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c
new file mode 100644
index ..148934975ee5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c
@@ -0,0 +1,45 @@
+/* test the attribute counted_by and its usage in
+   bounds sanitizer combined with VLA.  */
+/* { dg-do run } */
+/* { dg-options "-fsanitize=bounds" } */
+/* { dg-output "index 11 out of bounds for type 'int 
\\\[\\\*\\\]\\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*index 20 out of bounds for type 'int 
\\\[\\\*\\\]\\\[\\\*\\\]\\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*index 11 out of bounds for type 'int 
\\\[\\\*\\\]\\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*index 10 out of bounds for type 'int 
\\\[\\\*\\\]'\[^\n\r]*(\n|\r\n|\r)" } */
+
+
+#include 
+
+void __attribute__((__noinline__)) setup_and_test_vla (int n, int m)
+{
+   struct foo {
+   int n;
+   int p[][n] __attribute__((counted_by(n)));
+   } *f;
+
+   f = (struct foo *) malloc (sizeof(struct foo) + m*sizeof(int[n]));
+   f->n = m;
+   f->p[m][n-1]=1;
+   return;
+}
+
+void __attribute__((__noinline__)) setup_and_test_vla_1 (int n1, int n2, int m)
+{
+  struct foo {
+int n;
+int p[][n2][n1] __attribute__((counted_by(n)));
+  } *f;
+
+  f = (struct foo *) malloc (sizeof(struct foo) + m*sizeof(int[n2][n1]));
+  f->n = m;
+  f->p[m][n2][n1]=1;
+  return;
+}
+
+int main(int argc, char *argv[])
+{
+  setup_and_test_vla (10, 11);
+  setup_and_test_vla_1 (10, 11, 20);
+  return 0;

[PATCH v5 2/4] Convert references with "counted_by" attributes to/from .ACCESS_WITH_SIZE.

2024-02-09 Thread Qing Zhao
Including the following changes:
* The definition of the new internal function .ACCESS_WITH_SIZE
  in internal-fn.def.
* C FE converts every reference to a FAM with a "counted_by" attribute
  to a call to the internal function .ACCESS_WITH_SIZE.
  (build_component_ref in c_typeck.cc)

  This includes the case when the object is statically allocated and
  initialized.
  In order to make this working, the routines initializer_constant_valid_p_1
  and output_constant in varasm.cc are updated to handle calls to
  .ACCESS_WITH_SIZE.
  (initializer_constant_valid_p_1 and output_constant in varasm.c)

  However, for the reference inside "offsetof", the "counted_by" attribute is
  ignored since it's not useful at all.
  (c_parser_postfix_expression in c/c-parser.cc)

  In addtion to "offsetof", for the reference inside operator "typeof" and
  "alignof", we ignore counted_by attribute too.

  When building ADDR_EXPR for the .ACCESS_WITH_SIZE in C FE,
  replace the call with its first argument.

* Convert every call to .ACCESS_WITH_SIZE to its first argument.
  (expand_ACCESS_WITH_SIZE in internal-fn.cc)
* Adjust alias analysis to exclude the new internal from clobbering anything.
  (ref_maybe_used_by_call_p_1 and call_may_clobber_ref_p_1 in tree-ssa-alias.cc)
* Adjust dead code elimination to eliminate the call to .ACCESS_WITH_SIZE when
  it's LHS is eliminated as dead code.
  (eliminate_unnecessary_stmts in tree-ssa-dce.cc)
* Provide the utility routines to check the call is .ACCESS_WITH_SIZE and
  get the reference from the call to .ACCESS_WITH_SIZE.
  (is_access_with_size_p and get_ref_from_access_with_size in tree.cc)

gcc/c/ChangeLog:

* c-parser.cc (c_parser_postfix_expression): Ignore the counted-by
attribute when build_component_ref inside offsetof operator.
* c-tree.h (build_component_ref): Add one more parameter.
* c-typeck.cc (build_counted_by_ref): New function.
(build_access_with_size_for_counted_by): New function.
(build_component_ref): Check the counted-by attribute and build
call to .ACCESS_WITH_SIZE.
(build_unary_op): When building ADDR_EXPR for
.ACCESS_WITH_SIZE, use its first argument.
(lvalue_p): Accept call to .ACCESS_WITH_SIZE.

gcc/ChangeLog:

* internal-fn.cc (expand_ACCESS_WITH_SIZE): New function.
* internal-fn.def (ACCESS_WITH_SIZE): New internal function.
* tree-ssa-alias.cc (ref_maybe_used_by_call_p_1): Special case
IFN_ACCESS_WITH_SIZE.
(call_may_clobber_ref_p_1): Special case IFN_ACCESS_WITH_SIZE.
* tree-ssa-dce.cc (eliminate_unnecessary_stmts): Eliminate the call
to .ACCESS_WITH_SIZE when its LHS is dead.
* tree.cc (process_call_operands): Adjust side effect for function
.ACCESS_WITH_SIZE.
(is_access_with_size_p): New function.
(get_ref_from_access_with_size): New function.
* tree.h (is_access_with_size_p): New prototype.
(get_ref_from_access_with_size): New prototype.
* varasm.cc (initializer_constant_valid_p_1): Handle call to
.ACCESS_WITH_SIZE.
(output_constant): Handle call to .ACCESS_WITH_SIZE.

gcc/testsuite/ChangeLog:

* gcc.dg/flex-array-counted-by-2.c: New test.
---
 gcc/c/c-parser.cc |  10 +-
 gcc/c/c-tree.h|   2 +-
 gcc/c/c-typeck.cc | 128 +-
 gcc/internal-fn.cc|  36 +
 gcc/internal-fn.def   |   4 +
 .../gcc.dg/flex-array-counted-by-2.c  | 112 +++
 gcc/tree-ssa-alias.cc |   2 +
 gcc/tree-ssa-dce.cc   |   5 +-
 gcc/tree.cc   |  25 +++-
 gcc/tree.h|   8 ++
 gcc/varasm.cc |  10 ++
 11 files changed, 332 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-2.c

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index c31349dae2ff..a6ed5ac43bb1 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -10850,9 +10850,12 @@ c_parser_postfix_expression (c_parser *parser)
if (c_parser_next_token_is (parser, CPP_NAME))
  {
c_token *comp_tok = c_parser_peek_token (parser);
+   /* Ignore the counted_by attribute for reference inside
+  offsetof since the information is not useful at all.  */
offsetof_ref
  = build_component_ref (loc, offsetof_ref, comp_tok->value,
-comp_tok->location, UNKNOWN_LOCATION);
+comp_tok->location, UNKNOWN_LOCATION,
+false);
c_parser_consume_token (parser);
while (c_parser_next_token_is (parser, CPP_DOT)
   || 

[PATCH v5 3/4] Use the .ACCESS_WITH_SIZE in builtin object size.

2024-02-09 Thread Qing Zhao
gcc/ChangeLog:

* tree-object-size.cc (access_with_size_object_size): New function.
(call_object_size): Call the new function.

gcc/testsuite/ChangeLog:

* gcc.dg/builtin-object-size-common.h: Add a new macro EXPECT.
* gcc.dg/flex-array-counted-by-3.c: New test.
* gcc.dg/flex-array-counted-by-4.c: New test.
* gcc.dg/flex-array-counted-by-5.c: New test.
---
 .../gcc.dg/builtin-object-size-common.h   |  11 ++
 .../gcc.dg/flex-array-counted-by-3.c  |  63 +++
 .../gcc.dg/flex-array-counted-by-4.c  | 178 ++
 .../gcc.dg/flex-array-counted-by-5.c  |  48 +
 gcc/tree-object-size.cc   |  59 ++
 5 files changed, 359 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-5.c

diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-common.h 
b/gcc/testsuite/gcc.dg/builtin-object-size-common.h
index 66ff7cdd953a..b677067c6e6b 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-common.h
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-common.h
@@ -30,3 +30,14 @@ unsigned nfails = 0;
   __builtin_abort ();\
 return 0;\
   } while (0)
+
+#define EXPECT(p, _v) do {   \
+  size_t v = _v; \
+  if (p == v)\
+__builtin_printf ("ok:  %s == %zd\n", #p, p);\
+  else   \
+{\
+  __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v);\
+  FAIL ();   \
+}\
+} while (0);
diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c 
b/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
new file mode 100644
index ..0066c32ca808
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
@@ -0,0 +1,63 @@
+/* test the attribute counted_by and its usage in
+ * __builtin_dynamic_object_size.  */ 
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include "builtin-object-size-common.h"
+
+struct flex {
+  int b;
+  int c[];
+} *array_flex;
+
+struct annotated {
+  int b;
+  int c[] __attribute__ ((counted_by (b)));
+} *array_annotated;
+
+struct nested_annotated {
+  struct {
+union {
+  int b;
+  float f; 
+};
+int n;
+  };
+  int c[] __attribute__ ((counted_by (b)));
+} *array_nested_annotated;
+
+void __attribute__((__noinline__)) setup (int normal_count, int attr_count)
+{
+  array_flex
+= (struct flex *)malloc (sizeof (struct flex)
++ normal_count *  sizeof (int));
+  array_flex->b = normal_count;
+
+  array_annotated
+= (struct annotated *)malloc (sizeof (struct annotated)
+ + attr_count *  sizeof (int));
+  array_annotated->b = attr_count;
+
+  array_nested_annotated
+= (struct nested_annotated *)malloc (sizeof (struct nested_annotated)
++ attr_count *  sizeof (int));
+  array_nested_annotated->b = attr_count;
+
+  return;
+}
+
+void __attribute__((__noinline__)) test ()
+{
+EXPECT(__builtin_dynamic_object_size(array_flex->c, 1), -1);
+EXPECT(__builtin_dynamic_object_size(array_annotated->c, 1),
+  array_annotated->b * sizeof (int));
+EXPECT(__builtin_dynamic_object_size(array_nested_annotated->c, 1),
+  array_nested_annotated->b * sizeof (int));
+}
+
+int main(int argc, char *argv[])
+{
+  setup (10,10);   
+  test ();
+  DONE ();
+}
diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c 
b/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
new file mode 100644
index ..3ce7f3545549
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-4.c
@@ -0,0 +1,178 @@
+/* test the attribute counted_by and its usage in
+__builtin_dynamic_object_size: what's the correct behavior when the
+allocation size mismatched with the value of counted_by attribute?
+we should always use the latest value that is hold by the counted_by
+field.  */
+/* { dg-do run } */
+/* { dg-options "-O -fstrict-flex-arrays=3" } */
+
+#include "builtin-object-size-common.h"
+
+struct annotated {
+  size_t foo;
+  char others;
+  char array[] __attribute__((counted_by (foo)));
+};
+
+#define noinline __attribute__((__noinline__))
+#define SIZE_BUMP 10 
+#define MAX(a, b) ((a) > (b) ? (a) : (b))
+
+/* In general, Due to type casting, the type for the pointee of a pointer
+   does not say 

[PATCH v5 0/4] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2024-02-09 Thread Qing Zhao
Hi,

This is the 5th version of the patch.

compare with the 4th version, the major difference are:

1. Change the return type of the routine .ACCESS_WITH_SIZE 
   FROM:
 Pointer to the type of the element of the flexible array;
   TO:
 Pointer to the type of the flexible array;
And then wrap the call with an indirection reference. 

2. Adjust all other parts with this change, (this will simplify the bound 
sanitizer instrument);

3. Add the fixes to the kernel building failures, which include:
A. The operator ???typeof??? cannot return correct type for a->array; 
B. The operator ???&??? cannot return correct address for a->array;

4. Correctly handle the case when the value of ???counted-by??? is zero or 
negative as following
   4.1. Update the counted-by doc with the following:
When the counted-by field is assigned a negative integer value, the 
compiler will treat the value as zero. 
   4.2. Adjust __bdos and array bound sanitizer to handle correctly when 
???counted-by??? is zero. 


It based on the following proposal:

https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635884.html
Represent the missing dependence for the "counted_by" attribute and its 
consumers

**The summary of the proposal is:

* Add a new internal function ".ACCESS_WITH_SIZE" to carry the size information 
for every reference to a FAM field;
* In C FE, Replace every reference to a FAM field whose TYPE has the 
"counted_by" attribute with the new internal function ".ACCESS_WITH_SIZE";
* In every consumer of the size information, for example, BDOS or array bound 
sanitizer, query the size information or ACCESS_MODE information from the new 
internal function;
* When expansing to RTL, replace the internal function with the actual 
reference to the FAM field;
* Some adjustment to ipa alias analysis, and other SSA passes to mitigate the 
impact to the optimizer and code generation.


**The new internal function

  .ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE, TYPE_OF_SIZE, 
ACCESS_MODE)

INTERNAL_FN (ACCESS_WITH_SIZE, ECF_LEAF | ECF_NOTHROW, NULL)

which returns the "REF_TO_OBJ" same as the 1st argument;

Both the return type and the type of the first argument of this function have 
been converted from the incomplete array type to the corresponding pointer type.

The call to .ACCESS_WITH_SIZE is wrapped with an INDIRECT_REF, whose type is 
the original imcomplete array type.

Please see the following link for why:
https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638793.html
https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639605.html

1st argument "REF_TO_OBJ": The reference to the object;
2nd argument "REF_TO_SIZE": The reference to the size of the object,
3rd argument "CLASS_OF_SIZE": The size referenced by the REF_TO_SIZE represents
   0: unknown;
   1: the number of the elements of the object type;
   2: the number of bytes;
4th argument TYPE_OF_SIZE: A constant 0 with the TYPE of the object
  refed by REF_TO_SIZE
5th argument "ACCESS_MODE":
  -1: Unknown access semantics
   0: none
   1: read_only
   2: write_only
   3: read_write

** The Patch sets included:

1. Provide counted_by attribute to flexible array member field;
  which includes:
  * "counted_by" attribute documentation;
  * C FE handling of the new attribute;
syntax checking, error reporting;
  * testing cases;

2. Convert "counted_by" attribute to/from .ACCESS_WITH_SIZE.
  which includes:
  * The definition of the new internal function .ACCESS_WITH_SIZE in 
internal-fn.def.
  * C FE converts every reference to a FAM with "counted_by" attribute to a 
call to the internal function .ACCESS_WITH_SIZE.
(build_component_ref in c_typeck.cc)
This includes the case when the object is statically allocated and 
initialized.
In order to make this working, we should update 
initializer_constant_valid_p_1 and output_constant in varasm.cc to include 
calls to .ACCESS_WITH_SIZE.

However, for the reference inside "offsetof", ignore the "counted_by" 
attribute since it's not useful at all. (c_parser_postfix_expression in 
c/c-parser.cc)
In addtion to "offsetof", for the reference inside operator "typeof" and
  "alignof", we ignore counted_by attribute too.
When building ADDR_EXPR for the .ACCESS_WITH_SIZE in C FE,
  replace the call with its first argument.

  * Convert every call to .ACCESS_WITH_SIZE to its first argument.
(expand_ACCESS_WITH_SIZE in internal-fn.cc)
  * adjust alias analysis to exclude the new internal from clobbering 
anything.
(ref_maybe_used_by_call_p_1 and call_may_clobber_ref_p_1 in 
tree-ssa-alias.cc)
  * adjust dead code elimination to eliminate the call to .ACCESS_WITH_SIZE 
when
it's LHS is eliminated as dead code.
(eliminate_unnecessary_stmts in tree-ssa-dce.cc)
  * Provide the utility routines to check the call is .ACCESS_WITH_SIZE and
get the 

[PATCH v5 1/4] Provide counted_by attribute to flexible array member field (PR108896)

2024-02-09 Thread Qing Zhao
'counted_by (COUNT)'
 The 'counted_by' attribute may be attached to the C99 flexible
 array member of a structure.  It indicates that the number of the
 elements of the array is given by the field named "COUNT" in the
 same structure as the flexible array member.  GCC uses this
 information to improve the results of the array bound sanitizer and
 the '__builtin_dynamic_object_size'.

 For instance, the following code:

  struct P {
size_t count;
char other;
char array[] __attribute__ ((counted_by (count)));
  } *p;

 specifies that the 'array' is a flexible array member whose number
 of elements is given by the field 'count' in the same structure.

 The field that represents the number of the elements should have an
 integer type.  Otherwise, the compiler will report a warning and
 ignore the attribute.

 When the field that represents the number of the elements is assigned a
 negative integer value, the compiler will treat the value as zero.

 An explicit 'counted_by' annotation defines a relationship between
 two objects, 'p->array' and 'p->count', and there are the following
 requirementthat on the relationship between this pair:

* 'p->count' should be initialized before the first reference to
  'p->array';

* 'p->array' has _at least_ 'p->count' number of elements
  available all the time.  This relationship must hold even
  after any of these related objects are updated during the
  program.

 It's the user's responsibility to make sure the above requirements
 to be kept all the time.  Otherwise the compiler will report
 warnings, at the same time, the results of the array bound
 sanitizer and the '__builtin_dynamic_object_size' is undefined.

 One important feature of the attribute is, a reference to the
 flexible array member field will use the latest value assigned to
 the field that represents the number of the elements before that
 reference.  For example,

p->count = val1;
p->array[20] = 0;  // ref1 to p->array
p->count = val2;
p->array[30] = 0;  // ref2 to p->array

 in the above, 'ref1' will use 'val1' as the number of the elements
 in 'p->array', and 'ref2' will use 'val2' as the number of elements
 in 'p->array'.

gcc/c-family/ChangeLog:

PR C/108896
* c-attribs.cc (handle_counted_by_attribute): New function.
(attribute_takes_identifier_p): Add counted_by attribute to the list.
* c-common.cc (c_flexible_array_member_type_p): ...To this.
* c-common.h (c_flexible_array_member_type_p): New prototype.

gcc/c/ChangeLog:

PR C/108896
* c-decl.cc (flexible_array_member_type_p): Renamed and moved to...
(add_flexible_array_elts_to_size): Use renamed function.
(is_flexible_array_member_p): Use renamed function.
(verify_counted_by_attribute): New function.
(finish_struct): Use renamed function and verify counted_by
attribute.
* c-tree.h (lookup_field): New prototype.
* c-typeck.cc (lookup_field): Expose as extern function.

gcc/ChangeLog:

PR C/108896
* doc/extend.texi: Document attribute counted_by.

gcc/testsuite/ChangeLog:

PR C/108896
* gcc.dg/flex-array-counted-by.c: New test.
---
 gcc/c-family/c-attribs.cc| 54 -
 gcc/c-family/c-common.cc | 13 +++
 gcc/c-family/c-common.h  |  1 +
 gcc/c/c-decl.cc  | 85 
 gcc/c/c-tree.h   |  1 +
 gcc/c/c-typeck.cc|  3 +-
 gcc/doc/extend.texi  | 64 +++
 gcc/testsuite/gcc.dg/flex-array-counted-by.c | 40 +
 8 files changed, 241 insertions(+), 20 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by.c

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 40a0cf90295d..4395c0656b14 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -105,6 +105,8 @@ static tree handle_warn_if_not_aligned_attribute (tree *, 
tree, tree,
  int, bool *);
 static tree handle_strict_flex_array_attribute (tree *, tree, tree,
 int, bool *);
+static tree handle_counted_by_attribute (tree *, tree, tree,
+  int, bool *);
 static tree handle_weak_attribute (tree *, tree, tree, int, bool *) ;
 static tree handle_noplt_attribute (tree *, tree, tree, int, bool *) ;
 static tree handle_alias_ifunc_attribute (bool, tree *, tree, tree, bool *);
@@ -412,6 +414,8 @@ const struct attribute_spec c_common_gnu_attributes[] =
  handle_warn_if_not_aligned_attribute, NULL },

[PATCH] c++: avoid name lookup during mangling

2024-02-09 Thread Patrick Palka
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
reasonable?

-- >8 --

It turns out that with modules we can call mangle_decl recursively,
which is a problem because the global mangling state isn't recursion
aware.  The recursion happens from write_closure_type_name, which calls
lambda function, which performs name lookup, which can trigger lazy
loading, which can call maybe_clone_body for a newly loaded function,
which can inspect DECL_ASSEMBLER_NAME, which enters mangling.  This was
observed when using fmtlib as a module with trunk, and leads to a bogus
"mangling conflicts with a previous mangle error" followed by an ICE
from cdtor_comdat_group due to a mangling mismatch.

I considered making our mangling state recursion-aware, but it seems
this lambda_function call is the only source of name lookup during
mangling, and in turn the only source of potential recursion with
modules so perhaps it's better to just address that.  To that end, this
patch adds a new field to LAMBDA_EXPR holding the op() of the closure
type, so that lambda_function can just inspect this field rather than
having to perform name lookup.  With this patch fmtlib can be
successfully imported as a module (with a few minor source tweaks).

In passing this patch adds streaming of LAMBDA_EXPR_REGEN_INFO which I
noticed is missing.

gcc/cp/ChangeLog:

* cp-tree.h (LAMBDA_EXPR_FUNCTION): Define.
(tree_lambda_expr::function): New member.
* lambda.cc (lambda_function): Use LAMBDA_EXPR_FUNCTION instead
of performing name lookup.
* module.cc (trees_out::core_vals): Stream LAMBDA_EXPR_REGEN_INFO
and LAMBDA_EXPR_FUNCTION.
(trees_in::core_vals): Likewise.
* parser.cc (cp_parser_lambda_declarator_opt): Set
LAMBDA_EXPR_FUNCTION.
* pt.cc (tsubst_lambda_expr): Likewise.
---
 gcc/cp/cp-tree.h |  5 +
 gcc/cp/lambda.cc | 20 +---
 gcc/cp/module.cc |  4 
 gcc/cp/parser.cc |  1 +
 gcc/cp/pt.cc |  1 +
 5 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 969c7239c97..3bef35fd90c 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1543,6 +1543,10 @@ enum cp_lambda_default_capture_mode_type {
 #define LAMBDA_EXPR_CLOSURE(NODE) \
   (TREE_TYPE (LAMBDA_EXPR_CHECK (NODE)))
 
+/* The op() of the lambda closure type as would be found by name lookup.  */
+#define LAMBDA_EXPR_FUNCTION(NODE) \
+  (((struct tree_lambda_expr *)LAMBDA_EXPR_CHECK (NODE))->function)
+
 struct GTY (()) tree_lambda_expr
 {
   struct tree_typed typed;
@@ -1550,6 +1554,7 @@ struct GTY (()) tree_lambda_expr
   tree this_capture;
   tree extra_scope;
   tree regen_info;
+  tree function;
   vec *pending_proxies;
   location_t locus;
   enum cp_lambda_default_capture_mode_type default_capture_mode : 2;
diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
index 1d37e5a52b9..c3ec9381c19 100644
--- a/gcc/cp/lambda.cc
+++ b/gcc/cp/lambda.cc
@@ -165,22 +165,12 @@ begin_lambda_type (tree lambda)
 tree
 lambda_function (tree lambda)
 {
-  tree type;
-  if (TREE_CODE (lambda) == LAMBDA_EXPR)
-type = LAMBDA_EXPR_CLOSURE (lambda);
-  else
-type = lambda;
-  gcc_assert (LAMBDA_TYPE_P (type));
-  /* Don't let debug_tree cause instantiation.  */
-  if (CLASSTYPE_TEMPLATE_INSTANTIATION (type)
-  && !COMPLETE_OR_OPEN_TYPE_P (type))
+  if (CLASS_TYPE_P (lambda))
+lambda = CLASSTYPE_LAMBDA_EXPR (lambda);
+  tree callop = LAMBDA_EXPR_FUNCTION (lambda);
+  if (!callop)
 return NULL_TREE;
-  lambda = lookup_member (type, call_op_identifier,
- /*protect=*/0, /*want_type=*/false,
- tf_warning_or_error);
-  if (lambda)
-lambda = STRIP_TEMPLATE (get_first_fn (lambda));
-  return lambda;
+  return STRIP_TEMPLATE (callop);
 }
 
 /* True if EXPR is an expression whose type can be used directly in lambda
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 3c2fef0e3f4..7ae965d38e4 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -6317,6 +6317,8 @@ trees_out::core_vals (tree t)
   WT (((lang_tree_node *)t)->lambda_expression.capture_list);
   WT (((lang_tree_node *)t)->lambda_expression.this_capture);
   WT (((lang_tree_node *)t)->lambda_expression.extra_scope);
+  WT (((lang_tree_node *)t)->lambda_expression.regen_info);
+  WT (((lang_tree_node *)t)->lambda_expression.function);
   /* pending_proxies is a parse-time thing.  */
   gcc_assert (!((lang_tree_node *)t)->lambda_expression.pending_proxies);
   if (state)
@@ -6818,6 +6820,8 @@ trees_in::core_vals (tree t)
   RT (((lang_tree_node *)t)->lambda_expression.capture_list);
   RT (((lang_tree_node *)t)->lambda_expression.this_capture);
   RT (((lang_tree_node *)t)->lambda_expression.extra_scope);
+  RT (((lang_tree_node *)t)->lambda_expression.regen_info);
+  RT (((lang_tree_node *)t)->lambda_expression.function);
   /* lambda_expression.pending_proxies is 

Re: [RFC] GCC Security policy

2024-02-09 Thread Martin Jambor
Hi,

On Tue, Aug 08 2023, Richard Biener via Gcc-patches wrote:
> On Tue, Aug 8, 2023 at 2:33 PM Siddhesh Poyarekar  wrote:
>>
>> On 2023-08-08 04:16, Richard Biener wrote:
>> > On Mon, Aug 7, 2023 at 7:30 PM David Edelsohn via Gcc-patches
>> >  wrote:
>> >>
>> >> FOSS Best Practices recommends that projects have an official Security
>> >> policy stated in a SECURITY.md or SECURITY.txt file at the root of the
>> >> repository.  GLIBC and Binutils have added such documents.
>> >>
>> >> Appended is a prototype for a Security policy file for GCC based on the
>> >> Binutils document because GCC seems to have more affinity with Binutils as
>> >> a tool. Do the runtime libraries distributed with GCC, especially libgcc,
>> >> require additional security policies?
>> >>
>> >> [ ] Is it appropriate to use the Binutils SECURITY.txt as the starting
>> >> point or should GCC use GLIBC SECURITY.md as the starting point for the 
>> >> GCC
>> >> Security policy?
>> >>
>> >> [ ] Does GCC, or some components of GCC, require additional care because 
>> >> of
>> >> runtime libraries like libgcc and libstdc++, and because of gcov and
>> >> profile-directed feedback?
>> >
>> > I do think that the runtime libraries should at least be explicitly 
>> > mentioned
>> > because they fall into the "generated output" category and bugs in the
>> > runtime are usually more severe as affecting a wider class of inputs.
>>
>> Ack, I'd expect libstdc++ and libgcc to be aligned with glibc's
>> policies.  libiberty and others on the other hand, would probably be
>> more suitably aligned with binutils libbfd, where we assume trusted input.
>>
>> >> Thoughts?
>> >>
>> >> Thanks, David
>> >>
>> >> GCC Security Process
>> >> 
>> >>
>> >> What is a GCC security bug?
>> >> ===
>> >>
>> >>  A security bug is one that threatens the security of a system or
>> >>  network, or might compromise the security of data stored on it.
>> >>  In the context of GCC there are two ways in which such
>> >>  bugs might occur.  In the first, the programs themselves might be
>> >>  tricked into a direct compromise of security.  In the second, the
>> >>  tools might introduce a vulnerability in the generated output that
>> >>  was not already present in the files used as input.
>> >>
>> >>  Other than that, all other bugs will be treated as non-security
>> >>  issues.  This does not mean that they will be ignored, just that
>> >>  they will not be given the priority that is given to security bugs.
>> >>
>> >>  This stance applies to the creation tools in the GCC (e.g.,
>> >>  gcc, g++, gfortran, gccgo, gccrs, gnat, cpp, gcov, etc.) and the
>> >>  libraries that they use.
>> >>
>> >> Notes:
>> >> ==
>> >>
>> >>  None of the programs in GCC need elevated privileges to operate and
>> >>  it is recommended that users do not use them from accounts where such
>> >>  privileges are automatically available.
>> >
>> > I'll note that we could ourselves mitigate some of that by handling 
>> > privileged
>> > invocation of the driver specially, dropping privs on exec of the sibling 
>> > tools
>> > and possibly using temporary files or pipes to do the parts of the I/O that
>> > need to be privileged.
>>
>> It's not a bad idea, but it ends up giving legitimizing running the
>> compiler as root, pushing the responsibility of privilege management to
>> the driver.  How about rejecting invocation as root altogether by
>> default, bypassed with a --run-as-root flag instead?
>>
>> I've also been thinking about a --sandbox flag that isolates the build
>> process (for gcc as well as binutils) into a separate namespace so that
>> it's usable in a restricted mode on untrusted sources without exposing
>> the rest of the system to it.
>
> There's probably external tools to do this, not sure if we should replicate
> things in the driver for this.
>
> But sure, I think the driver is the proper point to address any of such
> issues - iff we want to address them at all.  Maybe a nice little
> google summer-of-code project ;)
>

If anyone is interested in scoping this and then mentoring this as a
Google Summer of Code project this year then now is the right time to
speak up!

Thanks,

Martin


Re: [PATCH][PUSHED] hwasan: support new dg-output format.

2024-02-09 Thread Alex Coplan
Hi,

On 04/05/2022 09:59, Martin Liška wrote:
> Supports change in libsanitizer where it newly reports:
> READ of size 4 at 0xc3d4 tags: 02/01(00) (ptr/mem) in thread T0
> 
> So the 'tags' contains now 3 entries compared to 2 entries.
> 
> gcc/testsuite/ChangeLog:
> 
>   * c-c++-common/hwasan/alloca-outside-caught.c: Update dg-output.
>   * c-c++-common/hwasan/heap-overflow.c: Likewise.
>   * c-c++-common/hwasan/hwasan-thread-access-parent.c: Likewise.
>   * c-c++-common/hwasan/large-aligned-1.c: Likewise.

I noticed the above test (large-aligned-1.c) failing on the GCC 12
branch due to the change in output format mentioned above.  This patch
(committed as r13-100-g3771486daa1e904ceae6f3e135b28e58af33849f) seems
to apply cleanly on the GCC 12 branch too, is it OK to backport to GCC 12?

Thanks,
Alex

>   * c-c++-common/hwasan/stack-tagging-basic-1.c: Likewise.
> ---
>  gcc/testsuite/c-c++-common/hwasan/alloca-outside-caught.c   | 2 +-
>  gcc/testsuite/c-c++-common/hwasan/heap-overflow.c   | 2 +-
>  gcc/testsuite/c-c++-common/hwasan/hwasan-thread-access-parent.c | 2 +-
>  gcc/testsuite/c-c++-common/hwasan/large-aligned-1.c | 2 +-
>  gcc/testsuite/c-c++-common/hwasan/stack-tagging-basic-1.c   | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/gcc/testsuite/c-c++-common/hwasan/alloca-outside-caught.c 
> b/gcc/testsuite/c-c++-common/hwasan/alloca-outside-caught.c
> index 60d7a9a874f..6f3825bee7c 100644
> --- a/gcc/testsuite/c-c++-common/hwasan/alloca-outside-caught.c
> +++ b/gcc/testsuite/c-c++-common/hwasan/alloca-outside-caught.c
> @@ -20,6 +20,6 @@ main ()
>  }
>  
>  /* { dg-output "HWAddressSanitizer: tag-mismatch on address 0x\[0-9a-f\]*.*" 
> } */
> -/* { dg-output "READ of size 4 at 0x\[0-9a-f\]* tags: 
> \[\[:xdigit:\]\]\[\[:xdigit:\]\]/00 \\(ptr/mem\\) in thread T0.*" } */
> +/* { dg-output "READ of size 4 at 0x\[0-9a-f\]* tags: 
> \[\[:xdigit:\]\]\[\[:xdigit:\]\]/00.* \\(ptr/mem\\) in thread T0.*" } */
>  /* { dg-output "Address 0x\[0-9a-f\]* is located in stack of thread T0.*" } 
> */
>  /* { dg-output "SUMMARY: HWAddressSanitizer: tag-mismatch \[^\n\]*.*" } */
> diff --git a/gcc/testsuite/c-c++-common/hwasan/heap-overflow.c 
> b/gcc/testsuite/c-c++-common/hwasan/heap-overflow.c
> index 137466800de..bddb38c81f1 100644
> --- a/gcc/testsuite/c-c++-common/hwasan/heap-overflow.c
> +++ b/gcc/testsuite/c-c++-common/hwasan/heap-overflow.c
> @@ -23,7 +23,7 @@ int main(int argc, char **argv) {
>  }
>  
>  /* { dg-output "HWAddressSanitizer: tag-mismatch on address 0x\[0-9a-f\]*.*" 
> } */
> -/* { dg-output "READ of size 1 at 0x\[0-9a-f\]* tags: 
> \[\[:xdigit:\]\]\[\[:xdigit:\]\]/\[\[:xdigit:\]\]\[\[:xdigit:\]\] 
> \\(ptr/mem\\) in thread T0.*" } */
> +/* { dg-output "READ of size 1 at 0x\[0-9a-f\]* tags: 
> \[\[:xdigit:\]\]\[\[:xdigit:\]\]/\[\[:xdigit:\]\]\[\[:xdigit:\]\].* 
> \\(ptr/mem\\) in thread T0.*" } */
>  /* { dg-output "located 0 bytes to the right of 10-byte region.*" } */
>  /* { dg-output "allocated here:.*" } */
>  /* { dg-output "#1 0x\[0-9a-f\]+ +in _*main \[^\n\r]*heap-overflow.c:18" } */
> diff --git a/gcc/testsuite/c-c++-common/hwasan/hwasan-thread-access-parent.c 
> b/gcc/testsuite/c-c++-common/hwasan/hwasan-thread-access-parent.c
> index 828909d3b3b..eca27c8cd2c 100644
> --- a/gcc/testsuite/c-c++-common/hwasan/hwasan-thread-access-parent.c
> +++ b/gcc/testsuite/c-c++-common/hwasan/hwasan-thread-access-parent.c
> @@ -46,6 +46,6 @@ main (int argc, char **argv)
>  }
>  
>  /* { dg-output "HWAddressSanitizer: tag-mismatch on address 0x\[0-9a-f\]*.*" 
> } */
> -/* { dg-output "READ of size 4 at 0x\[0-9a-f\]* tags: 
> 00/\[\[:xdigit:\]\]\[\[:xdigit:\]\] \\(ptr/mem\\) in thread T1.*" } */
> +/* { dg-output "READ of size 4 at 0x\[0-9a-f\]* tags: 
> 00/\[\[:xdigit:\]\]\[\[:xdigit:\]\].* \\(ptr/mem\\) in thread T1.*" } */
>  /* { dg-output "Address 0x\[0-9a-f\]* is located in stack of thread T0.*" } 
> */
>  /* { dg-output "SUMMARY: HWAddressSanitizer: tag-mismatch \[^\n\]*.*" } */
> diff --git a/gcc/testsuite/c-c++-common/hwasan/large-aligned-1.c 
> b/gcc/testsuite/c-c++-common/hwasan/large-aligned-1.c
> index 1aa13032396..6158ba4bdbc 100644
> --- a/gcc/testsuite/c-c++-common/hwasan/large-aligned-1.c
> +++ b/gcc/testsuite/c-c++-common/hwasan/large-aligned-1.c
> @@ -9,6 +9,6 @@
>  /* { dg-output "HWAddressSanitizer: tag-mismatch on address 0x\[0-9a-f\]*.*" 
> } */
>  /* NOTE: This assumes the current tagging mechanism (one at a time from the
> base and large aligned variables being handled first).  */
> -/* { dg-output "READ of size 4 at 0x\[0-9a-f\]* tags: 
> \[\[:xdigit:\]\]\[\[:xdigit:\]\]/\[\[:xdigit:\]\]\[\[:xdigit:\]\] 
> \\(ptr/mem\\) in thread T0.*" } */
> +/* { dg-output "READ of size 4 at 0x\[0-9a-f\]* tags: 
> \[\[:xdigit:\]\]\[\[:xdigit:\]\]/\[\[:xdigit:\]\]\[\[:xdigit:\]\].* 
> \\(ptr/mem\\) in thread T0.*" } */
>  /* { dg-output "Address 0x\[0-9a-f\]* is located in stack of thread T0.*" } 
> 

[PATCH v3]: testcases for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

2024-02-09 Thread Hans-Peter Nilsson
Bah.  Linaro's CI didn't like that there were UNRESOLVEDs
due to this patch.  Running it "as usual" didn't show
anything suspicious.  Sure, there were "# of unresolved
testcases 3" in the summary (see v2), but no error or other
special message from dejagnu.  Perhaps there could be a way
to have dg-ice automatically downgrade a dg-do run that
failed due to the ICE to a dg-do compile (or just not emit
the UNRESOLVED note), but pragmatically, this is a rare
enough case to not bother.  It looks like these were the
only UNRESOLVEDs in that CI run, so I'm just going to make a
mental note and move on.

For more comments, please see v2 of this patch.

v3: Change dg-do run to dg-do compile to avoid an UNRESOLVED.
Tested as with v2.  Ok to commit?

-- >8 --

Test-cases, with constexpr-reinterpret3.C dg-ice:ing the PR c++/113545 bug.

Regarding the request in the comment, a dg-do run when there's an ICE
will cause some CI's to signal an error for the run being "UNRESOLVED"
(compilation failed to produce executable).  Note that dejagnu (1.6.3)
itself doesn't consider this an error.

gcc/testsuite:
PR c++/113545
* g++.dg/cpp1y/constexpr-reinterpret3.C,
g++.dg/cpp1y/constexpr-reinterpret4.C: New tests.
---
 .../g++.dg/cpp1y/constexpr-reinterpret3.C | 56 +++
 .../g++.dg/cpp1y/constexpr-reinterpret4.C | 54 ++
 2 files changed, 110 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C

diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
new file mode 100644
index ..6c396bff72b6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
@@ -0,0 +1,56 @@
+// PR c++/113545
+// { dg-do run { target c++14 } }
+// Please change the above "dg-do compile" to "dg-do run" when the ICE is 
resolved.
+// { dg-ice "PR112545 - constexpr function with switch called for 
reinterpret_cast" }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned const char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(c);
+  unsigned const char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(d);
+  unsigned const char e = swbar((__UINTPTR_TYPE__) );
+  xyzzy(e);
+  unsigned const char f = ifbar((__UINTPTR_TYPE__) );
+  xyzzy(f);
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
new file mode 100644
index ..9aaa6e463bc6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
@@ -0,0 +1,54 @@
+// PR c++/113545
+// { dg-do compile { target c++14 } }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>()); 
// { dg-error "conversion from pointer type" }
+  xyzzy(c);
+  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>()); 
// { dg-error "conversion from pointer type" }
+  xyzzy(d);
+  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) ); // { dg-error 
"conversion from pointer type" }
+  xyzzy(e);
+  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) ); // { dg-error 
"conversion from pointer type" }
+  xyzzy(f);
+}
-- 
2.30.2



Re: [PATCH v3] c++: DR2237, cdtor and template-id tweaks [PR107126]

2024-02-09 Thread Jason Merrill

On 2/8/24 16:26, Marek Polacek wrote:

This patch does *not* fix

where the C++20 diagnostic is missing altogether.


What would it take to fix that as well?


* doc/invoke.texi: Document -Wdeprecated-template-id-cdtor.


Was it ever deprecated?  I'm not seeing that in

https://timsong-cpp.github.io/cppwp/n4659/#depr (C++17)

Let's drop the word "deprecated" from the option name and documentation.


@@ -32331,11 +32338,11 @@ cp_parser_constructor_declarator_p (cp_parser 
*parser, cp_parser_flags flags,
if (next_token->type != CPP_NAME
&& next_token->type != CPP_SCOPE
&& next_token->type != CPP_NESTED_NAME_SPECIFIER
-  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the
-declarator-id of a constructor or destructor.  */
-  && (next_token->type != CPP_TEMPLATE_ID || cxx_dialect >= cxx20))
+  && next_token->type != CPP_TEMPLATE_ID)
  return false;
  
+  const bool saw_template_id = (next_token->type == CPP_TEMPLATE_ID);


Please incorporate your "further DR2237 fix" patch into this one.


@@ -32552,6 +32559,19 @@ cp_parser_constructor_declarator_p (cp_parser *parser, 
cp_parser_flags flags,
/* We did not really want to consume any tokens.  */
cp_parser_abort_tentative_parse (parser);
  
+  /* DR 2237 (C++20 only): A simple-template-id is no longer valid as the

+ declarator-id of a constructor or destructor.  */
+  if (constructor_p && saw_template_id)
+{
+  gcc_checking_assert
+   (!cp_parser_uncommitted_to_tentative_parse_p (parser));


Now I see the abort_ just above, so this seems unnecessary after all.

Thanks,
Jason



Re: [PATCH v2] c++: make build_throw SFINAE-friendly [PR98388]

2024-02-09 Thread Jason Merrill

On 2/8/24 17:20, Marek Polacek wrote:

On Thu, Feb 08, 2024 at 04:53:45PM -0500, Jason Merrill wrote:

On 2/8/24 11:51, Marek Polacek wrote:

On Thu, Feb 08, 2024 at 08:49:28AM -0500, Patrick Palka wrote:

On Wed, 7 Feb 2024, Marek Polacek wrote:


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

-- >8 --
Here the problem is that we give hard errors while substituting
template parameters during overload resolution of is_throwable
which has an invalid throw in decltype.

The backtrace shows that fn_type_unification -> instantiate_template
-> tsubst* passes complain=0 as expected, but build_throw doesn't
have a complain parameter.  So let's add one.  Also remove a redundant
local variable which I should have removed in my P2266 patch.

But there's still something not quite clear to me.  I claim that 'b'
in the testcase should evaluate to false since the first overload ought
to have been discarded.  EDG 6.6 agrees, but clang++, msvc, and icx evaluate
it to true.  Who's right?


I think it should be true since P1155, which we implement in C++20 mode and
above (or rather, we implement the sequel P2266); since then we implicitly
move from the function parameter.

The patch looks good except that we should test this expected value.


I could add
#if __cplusplus >= 202002L
static_assert (b, "move from the function parameter");
#else
static_assert (!b, "no move from the function parameter");
#endif

but that's going to fail for C++20 and above.


Ah, because treat_lvalue_as_rvalue_p doesn't recognize t as being from 
the current scope in the trailing return type.  But that shouldn't be 
necessary:


https://eel.is/c++draft/expr.prim.id.unqual#4.2 says it's move-eligible
"if the id-expression (possibly parenthesized) is the operand of a 
throw-expression ([expr.throw]), and names an implicitly movable entity 
that belongs to a scope that does not contain the compound-statement of 
the innermost lambda-expression, try-block, or function-try-block (if 
any) whose compound-statement or ctor-initializer contains the 
throw-expression."


here there is no enclosing lambda or try, so it seems move-eligible.


I wonder if this is the
second half of the problem in 113789?

I could comment the first static_assert and add a FIXME if that sounds good?


dg-bogus would be better than commenting it out.

Will you also look into fixing the treat_ bug?  That can be a separate 
patch.


Jason



Re: [PATCH] testsuite, arm: Fix testcase arm/pr112337.c to check for the options first

2024-02-09 Thread Richard Earnshaw (lists)
On 30/01/2024 17:07, Saurabh Jha wrote:
> Hey,
> 
> Previously, this test was added to fix this bug: 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112337. However, it did not 
> check the compilation options before using them, leading to errors.
> 
> This patch fixes the test by first checking whether it can use the options 
> before using them.
> 
> Tested for arm-none-eabi and found no regressions. The output of check-gcc 
> with RUNTESTFLAGS="arm.exp=*" changed like this:
> 
> Before:
> # of expected passes  5963
> # of unexpected failures  64
> 
> After:
> # of expected passes  5964
> # of unexpected failures  63
> 
> Ok for master?
> 
> Regards,
> Saurabh
> 
> gcc/testsuite/ChangeLog:
> 
> * gcc.target/arm/pr112337.c: Check whether we can use the compilation 
> options before using them.

My apologies for missing this earlier.  It didn't show up in patchwork. That's 
most likely because the attachment is a binary blob instead of text/plain.  
That also means that the Linaro CI system hasn't seen this patch either.  
Please can you fix your mailer to add plain text patch files.

-/* { dg-options "-O2 -march=armv8.1-m.main+fp.dp+mve.fp -mfloat-abi=hard" } */
+/* { dg-require-effective-target arm_hard_ok } */
+/* { dg-require-effective-target arm_v8_1m_mve_ok } */
+/* { dg-options "-O2 -mfloat-abi=hard" } */
+/* { dg-add-options arm_v8_1m_mve } */

This is moving in the right direction, but it adds more than necessary now: 
checking for, and adding -mfloat-abi=hard is not necessary any more as 
arm_v8_1m_mve_ok will work out what float-abi flags are needed to make the 
options work. (What's more, it will prevent the test from running if the base 
configuration of the compiler is incompatible with the hard float ABI, which is 
more than we need.).

So please can you re-spin removing the hard-float check and removing that from 
dg-options.

Thanks,
R.


[PATCH v2] libstdc++: add ARM SVE support to std::experimental::simd

2024-02-09 Thread Srinivas Yadav Singanaboina
Hi,

Thanks for review @Richard!. I have tried to address most of your comments in 
this patch.
The major updates include optimizing operator[] for masks, find_first_set and 
find_last_set.

My further comments on some of the pointed out issues are
a. regarding the coverage of types supported for sve : Yes, all the types are 
covered by 
mapping any type using simple two rules : the size of the type and signedness 
of it.
b. all the operator overloads now use infix operators. For division and 
remainder, 
the inactive elements are padded with 1 to avoid undefined behavior.
c. isnan is optimized to have only two cases i.e finite_math_only case or case 
where svcmpuo is used.
d. _S_load for masks (bool) now uses svld1 by reinterpret_casting the pointer 
to uint8_t pointer and then performing a svunpklo.
The same optimization is not done for masked_load and stores, as conversion of 
mask from a higher size type to lower size type is not optimal (sequential).
e. _S_unary_minus could not use svneg_x because it does not support unsigned 
types.
f. added specializations for reductions.
g. find_first_set and find_last_set are optimized using svclastb.


libstdc++-v3/ChangeLog:

* include/Makefile.am: Add simd_sve.h.
* include/Makefile.in: Add simd_sve.h.
* include/experimental/bits/simd.h: Add new SveAbi.
* include/experimental/bits/simd_builtin.h: Use
  __no_sve_deduce_t to support existing Neon Abi.
* include/experimental/bits/simd_converter.h: Convert
  sequentially when sve is available.
* include/experimental/bits/simd_detail.h: Define sve
  specific macro.
* include/experimental/bits/simd_math.h: Fallback frexp
  to execute sequntially when sve is available, to handle
  fixed_size_simd return type that always uses sve.
* include/experimental/simd: Include bits/simd_sve.h.
* testsuite/experimental/simd/tests/bits/main.h: Enable
  testing for sve128, sve256, sve512.
* include/experimental/bits/simd_sve.h: New file.

 Signed-off-by: Srinivas Yadav Singanaboina
 vasu.srinivasvasu...@gmail.com
---
 libstdc++-v3/include/Makefile.am  |1 +
 libstdc++-v3/include/Makefile.in  |1 +
 libstdc++-v3/include/experimental/bits/simd.h |  131 +-
 .../include/experimental/bits/simd_builtin.h  |   35 +-
 .../experimental/bits/simd_converter.h|   57 +-
 .../include/experimental/bits/simd_detail.h   |7 +-
 .../include/experimental/bits/simd_math.h |   14 +-
 .../include/experimental/bits/simd_sve.h  | 1863 +
 libstdc++-v3/include/experimental/simd|3 +
 .../experimental/simd/tests/bits/main.h   |3 +
 10 files changed, 2084 insertions(+), 31 deletions(-)
 create mode 100644 libstdc++-v3/include/experimental/bits/simd_sve.h

diff --git a/libstdc++-v3/include/Makefile.am b/libstdc++-v3/include/Makefile.am
index 6209f390e08..1170cb047a6 100644
--- a/libstdc++-v3/include/Makefile.am
+++ b/libstdc++-v3/include/Makefile.am
@@ -826,6 +826,7 @@ experimental_bits_headers = \
${experimental_bits_srcdir}/simd_neon.h \
${experimental_bits_srcdir}/simd_ppc.h \
${experimental_bits_srcdir}/simd_scalar.h \
+   ${experimental_bits_srcdir}/simd_sve.h \
${experimental_bits_srcdir}/simd_x86.h \
${experimental_bits_srcdir}/simd_x86_conversions.h \
${experimental_bits_srcdir}/string_view.tcc \
diff --git a/libstdc++-v3/include/Makefile.in b/libstdc++-v3/include/Makefile.in
index 596fa0d2390..bc44582a2da 100644
--- a/libstdc++-v3/include/Makefile.in
+++ b/libstdc++-v3/include/Makefile.in
@@ -1172,6 +1172,7 @@ experimental_bits_headers = \
${experimental_bits_srcdir}/simd_neon.h \
${experimental_bits_srcdir}/simd_ppc.h \
${experimental_bits_srcdir}/simd_scalar.h \
+   ${experimental_bits_srcdir}/simd_sve.h \
${experimental_bits_srcdir}/simd_x86.h \
${experimental_bits_srcdir}/simd_x86_conversions.h \
${experimental_bits_srcdir}/string_view.tcc \
diff --git a/libstdc++-v3/include/experimental/bits/simd.h 
b/libstdc++-v3/include/experimental/bits/simd.h
index 90523ea57dc..d274cd740fe 100644
--- a/libstdc++-v3/include/experimental/bits/simd.h
+++ b/libstdc++-v3/include/experimental/bits/simd.h
@@ -39,12 +39,16 @@
 #include 
 #include 
 #include 
+#include 
 
 #if _GLIBCXX_SIMD_X86INTRIN
 #include 
 #elif _GLIBCXX_SIMD_HAVE_NEON
 #include 
 #endif
+#if _GLIBCXX_SIMD_HAVE_SVE
+#include 
+#endif
 
 /** @ingroup ts_simd
  * @{
@@ -83,6 +87,12 @@ using __m512d [[__gnu__::__vector_size__(64)]] = double;
 using __m512i [[__gnu__::__vector_size__(64)]] = long long;
 #endif
 
+#if _GLIBCXX_SIMD_HAVE_SVE
+constexpr inline int __sve_vectorized_size_bytes = __ARM_FEATURE_SVE_BITS / 8;
+#else
+constexpr inline int __sve_vectorized_size_bytes = 0;
+#endif 
+
 namespace simd_abi {
 // simd_abi forward declarations {{{
 // implementation details:
@@ -108,6 

[PATCH] libgcc: fix Win32 CV abnormal spurious wakeups in timed wait [PR113850]

2024-02-09 Thread Matteo Italia
The Win32 threading model uses __gthr_win32_abs_to_rel_time to convert
the timespec used in gthreads to specify the absolute time for end of
the condition variables timed wait to a milliseconds value relative to
"now" to pass to the Win32 SleepConditionVariableCS function.

Unfortunately, the conversion is incorrect, as, due to a typo, it
returns the relative time _in seconds_, so SleepConditionVariableCS
receives a timeout value 1000 times shorter than it should be, resulting
in a huge amount of spurious wakeups in calls such as
std::condition_variable::wait_for or wait_until.

This can be demonstrated easily by this sample program:

```

int main() {
std::condition_variable cv;
std::mutex mx;
bool pass = false;

auto thread_fn = [&](bool timed) {
int wakeups = 0;
using sc = std::chrono::system_clock;
auto before = sc::now();
std::unique_lock ml(mx);
if (timed) {
cv.wait_for(ml, std::chrono::seconds(2), [&]{
++wakeups;
return pass;
});
} else {
cv.wait(ml, [&]{
++wakeups;
return pass;
});
}
printf("pass: %d; wakeups: %d; elapsed: %d ms\n", pass, wakeups,
int((sc::now() - before) / std::chrono::milliseconds(1)));
pass = false;
};

{
// timed wait, let expire
std::thread t(thread_fn, true);
t.join();
}

{
// timed wait, wake up explicitly after 1 second
std::thread t(thread_fn, true);
std::this_thread::sleep_for(std::chrono::seconds(1));
{
std::unique_lock ml(mx);
pass = true;
}
cv.notify_all();
t.join();
}

{
// non-timed wait, wake up explicitly after 1 second
std::thread t(thread_fn, false);
std::this_thread::sleep_for(std::chrono::seconds(1));
{
std::unique_lock ml(mx);
pass = true;
}
cv.notify_all();
t.join();
}
return 0;
}
```

On builds that other threading models (e.g. POSIX on Linux, or
winpthreads or MCF on Win32) the output is something like
```
pass: 0; wakeups: 2; elapsed: 2000 ms
pass: 1; wakeups: 2; elapsed: 991 ms
pass: 1; wakeups: 2; elapsed: 996 ms
```

while on the affected builds it's more like
```
pass: 0; wakeups: 1418; elapsed: 2000 ms
pass: 1; wakeups: 479; elapsed: 988 ms
pass: 1; wakeups: 2; elapsed: 992 ms
```
(notice the huge number of wakeups).

This commit fixes the conversion, adjusting the final division by
NSEC100_PER_SEC to use NSEC100_PER_MSEC instead (already defined in the
file and not used in any other place, so the problem here was probably a
typo or some rebase/refactoring mishap).

libgcc/ChangeLog:

* config/i386/gthr-win32-cond.c (__gthr_win32_abs_to_rel_time):
  fix absolute timespec to relative milliseconds count
  conversion (it incorrectly returned seconds instead of
  milliseconds); this avoids spurious wakeups in
  __gthr_win32_cond_timedwait
---
 libgcc/config/i386/gthr-win32-cond.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libgcc/config/i386/gthr-win32-cond.c 
b/libgcc/config/i386/gthr-win32-cond.c
index ecb007a54b2..650c448f286 100644
--- a/libgcc/config/i386/gthr-win32-cond.c
+++ b/libgcc/config/i386/gthr-win32-cond.c
@@ -78,7 +78,7 @@ __gthr_win32_abs_to_rel_time (const __gthread_time_t 
*abs_time)
   if (abs_time_nsec100 < now.nsec100)
 return 0;
 
-  return (DWORD) CEIL_DIV (abs_time_nsec100 - now.nsec100, NSEC100_PER_SEC);
+  return (DWORD) CEIL_DIV (abs_time_nsec100 - now.nsec100, NSEC100_PER_MSEC);
 }
 
 /* Check the sizes of the local version of the Win32 types.  */
-- 
2.34.1



Re: [PATCH] bitint: Fix handling of VIEW_CONVERT_EXPRs to minimally supported huge INTEGER_TYPEs [PR113783]

2024-02-09 Thread Richard Biener
On Fri, 9 Feb 2024, Jakub Jelinek wrote:

> Hi!
> 
> On the following testcases memcpy lowering folds the calls to
> reading and writing of MEM_REFs with huge INTEGER_TYPEs - uint256_t
> with OImode or uint512_t with XImode.  Further optimization turn
> the load from MEM_REF from the large/huge _BitInt var into VIEW_CONVERT_EXPR
> from it to the uint256_t/uint512_t.  The backend doesn't really
> support those except for "movoi"/"movxi" insns, so it isn't possible
> to handle it like casts to supportable INTEGER_TYPEs where we can
> construct those from individual limbs - there are no OImode/XImode shifts
> and the like we can use.
> So, the following patch makes sure for such VCEs that the SSA_NAME operand
> of the VCE lives in memory and then turns it into a VIEW_CONVERT_EXPR so
> that we actually load the OImode/XImode integer from memory (i.e. a mov).
> We need to make sure those aren't merged with other
> operations in the gimple_lower_bitint hunks.
> For SSA_NAMEs which have underlying VAR_DECLs that is all we need, those
> VAR_DECL have ARRAY_TYPEs.
> For SSA_NAMEs which have underlying PARM_DECLs or RESULT_DECLs those have
> BITINT_TYPE and I had to tweak expand_expr_real_1 for that so that it
> doesn't try convert_modes on those when one of the modes is BLKmode - we
> want to fall through into the adjust_address on the MEM.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2024-02-09  Jakub Jelinek  
> 
>   PR tree-optimization/113783
>   * gimple-lower-bitint.cc (bitint_large_huge::lower_stmt): Look
>   through VIEW_CONVERT_EXPR for final cast checks.  Handle
>   VIEW_CONVERT_EXPRs from large/huge _BitInt to > MAX_FIXED_MODE_SIZE
>   INTEGER_TYPEs.
>   (gimple_lower_bitint): Don't merge mergeable operations or other
>   casts with VIEW_CONVERT_EXPRs to > MAX_FIXED_MODE_SIZE INTEGER_TYPEs.
>   * expr.cc (expand_expr_real_1): Don't use convert_modes if either
>   mode is BLKmode.
> 
>   * gcc.dg/bitint-88.c: New test.
> 
> --- gcc/gimple-lower-bitint.cc.jj 2024-02-06 12:58:48.296021497 +0100
> +++ gcc/gimple-lower-bitint.cc2024-02-08 12:49:40.435313811 +0100
> @@ -5263,6 +5263,8 @@ bitint_large_huge::lower_stmt (gimple *s
>  {
>lhs = gimple_assign_lhs (stmt);
>tree rhs1 = gimple_assign_rhs1 (stmt);
> +  if (TREE_CODE (rhs1) == VIEW_CONVERT_EXPR)
> + rhs1 = TREE_OPERAND (rhs1, 0);
>if (TREE_CODE (TREE_TYPE (lhs)) == BITINT_TYPE
> && bitint_precision_kind (TREE_TYPE (lhs)) >= bitint_prec_large
> && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
> @@ -5273,6 +5275,44 @@ bitint_large_huge::lower_stmt (gimple *s
>  || POINTER_TYPE_P (TREE_TYPE (lhs
>   {
> final_cast_p = true;
> +   if (TREE_CODE (TREE_TYPE (lhs)) == INTEGER_TYPE
> +   && TYPE_PRECISION (TREE_TYPE (lhs)) > MAX_FIXED_MODE_SIZE
> +   && gimple_assign_rhs_code (stmt) == VIEW_CONVERT_EXPR)
> + {
> +   /* Handle VIEW_CONVERT_EXPRs to not generally supported
> +  huge INTEGER_TYPEs like uint256_t or uint512_t.  These
> +  are usually emitted from memcpy folding and backends
> +  support moves with them but that is usually it.  */
> +   if (TREE_CODE (rhs1) == INTEGER_CST)
> + {
> +   rhs1 = fold_unary (VIEW_CONVERT_EXPR, TREE_TYPE (lhs),
> +  rhs1);
> +   gcc_assert (rhs1 && TREE_CODE (rhs1) == INTEGER_CST);
> +   gimple_assign_set_rhs1 (stmt, rhs1);
> +   gimple_assign_set_rhs_code (stmt, INTEGER_CST);
> +   update_stmt (stmt);
> +   return;
> + }
> +   gcc_assert (TREE_CODE (rhs1) == SSA_NAME);
> +   if (SSA_NAME_IS_DEFAULT_DEF (rhs1)
> +   && (!SSA_NAME_VAR (rhs1) || VAR_P (SSA_NAME_VAR (rhs1
> + {
> +   tree var = create_tmp_reg (TREE_TYPE (lhs));
> +   rhs1 = get_or_create_ssa_default_def (cfun, var);
> +   gimple_assign_set_rhs1 (stmt, rhs1);
> +   gimple_assign_set_rhs_code (stmt, SSA_NAME);
> + }
> +   else
> + {
> +   int part = var_to_partition (m_map, rhs1);
> +   gcc_assert (m_vars[part] != NULL_TREE);
> +   rhs1 = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (lhs),
> +  m_vars[part]);
> +   gimple_assign_set_rhs1 (stmt, rhs1);
> + }
> +   update_stmt (stmt);
> +   return;
> + }
> if (TREE_CODE (rhs1) == SSA_NAME
> && (m_names == NULL
> || !bitmap_bit_p (m_names, SSA_NAME_VERSION (rhs1
> @@ -6103,7 +6143,13 @@ gimple_lower_bitint (void)
> if (gimple_assign_cast_p (use_stmt))
>   {
> tree lhs = gimple_assign_lhs (use_stmt);
> -

Re: [PATCH] testsuite, arm: Fix testcase arm/pr112337.c to check for the options first

2024-02-09 Thread Saurabh Jha
Ping. I also don't have commit access so can someone please commit on my behalf.


From: Saurabh Jha
Sent: Tuesday, January 30, 2024 5:07 PM
To: Richard Sandiford via Gcc-patches; Richard Sandiford; Kyrylo Tkachov; 
Richard Earnshaw
Subject: [PATCH] testsuite, arm: Fix testcase arm/pr112337.c to check for the 
options first

Hey,

Previously, this test was added to fix this bug: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112337. However, it did not check 
the compilation options before using them, leading to errors.

This patch fixes the test by first checking whether it can use the options 
before using them.

Tested for arm-none-eabi and found no regressions. The output of check-gcc with 
RUNTESTFLAGS="arm.exp=*" changed like this:

Before:
# of expected passes  5963
# of unexpected failures  64

After:
# of expected passes  5964
# of unexpected failures  63

Ok for master?

Regards,
Saurabh

gcc/testsuite/ChangeLog:

* gcc.target/arm/pr112337.c: Check whether we can use the compilation 
options before using them.


Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-09 Thread Iain Sandoe



> On 8 Feb 2024, at 21:44, Jason Merrill  wrote:
> 
> On 2/8/24 12:55, Paolo Bonzini wrote:
>> On 2/8/24 18:16, Jason Merrill wrote:
> 
 
 Hmm.  In stage 1, when we build with the system gcc, I'd think we want the 
 just-built gnat1 to find the system libgcc.
 
 In stage 2, when we build with the stage 1 gcc, we want the just-built 
 gnat1 to find the stage 1 libgcc.
 
 In neither case do we want it to find the libgcc from the current stage.
 
 So it seems to me that what we want is for stage2+ LD_LIBRARY_PATH to 
 include the TARGET_LIB_PATH from the previous stage.  Something like the 
 below, on top of the earlier patch.
 
 Does this make sense?  Does it work on Darwin?
>>> 
>>> Oops, that was broken, please consider this one instead:
>> Yes, this one makes sense (and the current code would not work since it 
>> lacks the prev- prefix on TARGET_LIB_PATH).
> 
> Indeed, that seems like evidence that the only element of TARGET_LIB_PATH 
> that has been useful in HOST_EXPORTS is the prev- part of HOST_LIB_PATH_gcc.
> 
> So, here's another patch that just includes that for post-stage1:
> <0001-build-drop-target-libs-from-LD_LIBRARY_PATH-PR105688.patch>

Hmm this still fails for me with gnat1 being unable to find libgcc_s.
It seems I have to add the PREV_HOST_LIB_PATH_gcc to HOST_LIB_PATH for it to 
succeed so,
presumably, the post stage1 exports are not being forwarded to that build.  
I’ll try to analyze what
exactly is failing.
Iain



Re: [PATCH] [testsuite] tsvc: skip include malloc.h when unavailable

2024-02-09 Thread Torbjorn SVENSSON




On 2024-02-09 11:34, Richard Biener wrote:

On Fri, Feb 9, 2024 at 11:33 AM Torbjorn SVENSSON
 wrote:


Hi,

Is it okay to backport 2f20d6296087cae51f55eeecb3efefe786191fd6 to
releases/gcc-13?


Yes.


Pushed as 5b3dcff46780192a2e526bc434d61c8626898050.


Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

2024-02-09 Thread Richard Biener
On Thu, Feb 1, 2024 at 4:26 PM Jakub Jelinek  wrote:
>
> On Thu, Feb 01, 2024 at 03:55:51PM +0100, Jakub Jelinek wrote:
> > No, besides the formatting being incorrect both in ChangeLog and in the
> > patch, this pessimizes ILP32 hosts unnecessarily.
>
> So like this instead?

OK.

Thanks,
Richard.

> 2024-02-01  Jakub Jelinek  
>
> * hwint.h (GCC_PRISZ, fmt_size_t, HOST_SIZE_T_PRINT_DEC,
> HOST_SIZE_T_PRINT_UNSIGNED, HOST_SIZE_T_PRINT_HEX,
> HOST_SIZE_T_PRINT_HEX_PURE): Define.
> * ira-conflicts.cc (build_conflict_bit_table): Use it.  Formatting
> fixes.
>
> --- gcc/hwint.h.jj  2024-01-03 11:51:32.676715299 +0100
> +++ gcc/hwint.h 2024-02-01 16:22:53.037013522 +0100
> @@ -115,6 +115,27 @@ typedef HOST_WIDE_INT __gcc_host_wide_in
>  #define HOST_WIDE_INT_PRINT_DOUBLE_HEX "0x%" PRIx64 "%016" PRIx64
>  #define HOST_WIDE_INT_PRINT_PADDED_HEX "%016" PRIx64
>
> +/* Similarly format modifier for printing size_t.  As not all hosts support
> +   z modifier in printf, use GCC_PRISZ and cast argument to fmt_size_t.
> +   So, instead of doing fprintf ("%zu\n", sizeof (x) * y); use
> +   fprintf (HOST_SIZE_T_PRINT_UNSIGNED "\n",
> +   (fmt_size_t) (sizeof (x) * y));  */
> +#if SIZE_MAX <= INT_MAX
> +# define GCC_PRISZ ""
> +# define fmt_size_t unsigned int
> +#elif SIZE_MAX <= LONG_MAX
> +# define GCC_PRISZ HOST_LONG_FORMAT
> +# define fmt_size_t unsigned long int
> +#else
> +# define GCC_PRISZ HOST_LONG_LONG_FORMAT
> +# define fmt_size_t unsigned long long int
> +#endif
> +
> +#define HOST_SIZE_T_PRINT_DEC "%" GCC_PRISZ "d"
> +#define HOST_SIZE_T_PRINT_UNSIGNED "%" GCC_PRISZ "u"
> +#define HOST_SIZE_T_PRINT_HEX "%#" GCC_PRISZ "x"
> +#define HOST_SIZE_T_PRINT_HEX_PURE "%" GCC_PRISZ "x"
> +
>  /* Define HOST_WIDEST_FAST_INT to the widest integer type supported
> efficiently in hardware.  (That is, the widest integer type that fits
> in a hardware register.)  Normally this is "long" but on some hosts it
> --- gcc/ira-conflicts.cc.jj 2024-01-03 11:51:31.748728178 +0100
> +++ gcc/ira-conflicts.cc2024-02-01 16:12:02.156137005 +0100
> @@ -115,10 +115,10 @@ build_conflict_bit_table (void)
> > (uint64_t) param_ira_max_conflict_table_size * 1024 * 1024)
>   {
> if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
> - fprintf
> -   (ira_dump_file,
> -"+++Conflict table will be too big(>%dMB) -- don't use it\n",
> -param_ira_max_conflict_table_size);
> + fprintf (ira_dump_file,
> +  "+++Conflict table will be too big(>%dMB) "
> +  "-- don't use it\n",
> +  param_ira_max_conflict_table_size);
> return false;
>   }
>}
> @@ -148,11 +148,13 @@ build_conflict_bit_table (void)
>
>object_set_words = (ira_objects_num + IRA_INT_BITS - 1) / IRA_INT_BITS;
>if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
> -fprintf
> -  (ira_dump_file,
> -   "+++Allocating %ld bytes for conflict table (uncompressed size 
> %ld)\n",
> -   (long) allocated_words_num * sizeof (IRA_INT_TYPE),
> -   (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
> +fprintf (ira_dump_file,
> +"+++Allocating " HOST_SIZE_T_PRINT_UNSIGNED
> +" bytes for conflict table (uncompressed size "
> +HOST_SIZE_T_PRINT_UNSIGNED ")\n",
> +(fmt_size_t) (sizeof (IRA_INT_TYPE) * allocated_words_num),
> +(fmt_size_t) (sizeof (IRA_INT_TYPE) * object_set_words
> +  * ira_objects_num));
>
>objects_live = sparseset_alloc (ira_objects_num);
>for (i = 0; i < ira_max_point; i++)
>
>
> Jakub
>


Re: [PATCH] [testsuite] tsvc: skip include malloc.h when unavailable

2024-02-09 Thread Richard Biener
On Fri, Feb 9, 2024 at 11:33 AM Torbjorn SVENSSON
 wrote:
>
> Hi,
>
> Is it okay to backport 2f20d6296087cae51f55eeecb3efefe786191fd6 to
> releases/gcc-13?

Yes.

> Without this backport, I see about 150 failures on arm-none-eabi, an
> example of them is:
>
> FAIL: gcc.dg/vect/tsvc/vect-tsvc-s000.c (test for excess errors)
>
>
> Kind regards,
> Torbjörn
>
> On 2023-05-24 11:02, Richard Biener via Gcc-patches wrote:
> > On Wed, May 24, 2023 at 7:17 AM Alexandre Oliva via Gcc-patches
> >  wrote:
> >>
> >>
> >> tsvc tests all fail on systems that don't offer a malloc.h, other than
> >> those that explicitly rule that out.  Use the preprocessor to test for
> >> malloc.h's availability.
> >>
> >> tsvc.h also expects a definition for struct timeval, but it doesn't
> >> include sys/time.h.  Add a conditional include thereof.
> >>
> >> Bootstrapped on x86_64-linux-gnu.  Also tested on ppc- and x86-vx7r2
> >> with gcc-12.
> >
> > OK.
> >
> >>
> >> for  gcc/testsuite/ChangeLog
> >>
> >>  * gcc.dg/vect/tsvc/tsvc.h: Test for and conditionally include
> >>  malloc.h and sys/time.h.
> >>
> >> ---
> >>   gcc/testsuite/gcc.dg/vect/tsvc/tsvc.h |5 -
> >>   1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/gcc/testsuite/gcc.dg/vect/tsvc/tsvc.h 
> >> b/gcc/testsuite/gcc.dg/vect/tsvc/tsvc.h
> >> index 75494c24cfa62..cd39c041903dd 100644
> >> --- a/gcc/testsuite/gcc.dg/vect/tsvc/tsvc.h
> >> +++ b/gcc/testsuite/gcc.dg/vect/tsvc/tsvc.h
> >> @@ -11,9 +11,12 @@
> >>
> >>   #include 
> >>   #include 
> >> -#if !defined(__APPLE__) && !defined(__DragonFly__)
> >> +#if __has_include()
> >>   #include 
> >>   #endif
> >> +#if __has_include()
> >> +#include 
> >> +#endif
> >>   #include 
> >>   #include 
> >>
> >>
> >> --
> >> Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
> >> Free Software Activist   GNU Toolchain Engineer
> >> Disinformation flourishes because many people care deeply about injustice
> >> but very few check the facts.  Ask me about 


Re: [PATCH] [testsuite] tsvc: skip include malloc.h when unavailable

2024-02-09 Thread Torbjorn SVENSSON

Hi,

Is it okay to backport 2f20d6296087cae51f55eeecb3efefe786191fd6 to 
releases/gcc-13?


Without this backport, I see about 150 failures on arm-none-eabi, an 
example of them is:


FAIL: gcc.dg/vect/tsvc/vect-tsvc-s000.c (test for excess errors)


Kind regards,
Torbjörn

On 2023-05-24 11:02, Richard Biener via Gcc-patches wrote:

On Wed, May 24, 2023 at 7:17 AM Alexandre Oliva via Gcc-patches
 wrote:



tsvc tests all fail on systems that don't offer a malloc.h, other than
those that explicitly rule that out.  Use the preprocessor to test for
malloc.h's availability.

tsvc.h also expects a definition for struct timeval, but it doesn't
include sys/time.h.  Add a conditional include thereof.

Bootstrapped on x86_64-linux-gnu.  Also tested on ppc- and x86-vx7r2
with gcc-12.


OK.



for  gcc/testsuite/ChangeLog

 * gcc.dg/vect/tsvc/tsvc.h: Test for and conditionally include
 malloc.h and sys/time.h.

---
  gcc/testsuite/gcc.dg/vect/tsvc/tsvc.h |5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/vect/tsvc/tsvc.h 
b/gcc/testsuite/gcc.dg/vect/tsvc/tsvc.h
index 75494c24cfa62..cd39c041903dd 100644
--- a/gcc/testsuite/gcc.dg/vect/tsvc/tsvc.h
+++ b/gcc/testsuite/gcc.dg/vect/tsvc/tsvc.h
@@ -11,9 +11,12 @@

  #include 
  #include 
-#if !defined(__APPLE__) && !defined(__DragonFly__)
+#if __has_include()
  #include 
  #endif
+#if __has_include()
+#include 
+#endif
  #include 
  #include 


--
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about 


[pushed] libgcc, Darwin: Update symbol exports to include bitint and bf.

2024-02-09 Thread Iain Sandoe
Tested on i686-darwin8, x86_64-darwin{14,17,19,21,23} pushed to trunk.
thanks,
Iain

--- 8< ---

Some exports were missed from the GCC-13 cycle, these are added here
along with the bitint-related ones added in GCC-14.

libgcc/ChangeLog:

* config/i386/libgcc-darwin.ver: Export bf and bitint-related
synbols.
---
 libgcc/config/i386/libgcc-darwin.ver | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/libgcc/config/i386/libgcc-darwin.ver 
b/libgcc/config/i386/libgcc-darwin.ver
index c97dae73855..06560d6b47f 100644
--- a/libgcc/config/i386/libgcc-darwin.ver
+++ b/libgcc/config/i386/libgcc-darwin.ver
@@ -1,9 +1,12 @@
+
+# NOTE: Darwin does not version individual symbols, the grouping is
+# preserved here to document at which GCC revision they were introduced.
+
 GCC_4.8.0 {
   __cpu_model
   __cpu_indicator_init
 }
 
-%inherit GCC_12.0.0 GCC_7.0.0
 GCC_12.0.0 {
   __divhc3
   __mulhc3
@@ -22,3 +25,22 @@ GCC_12.0.0 {
   __trunctfhf2
   __truncxfhf2
 }
+
+GCC_14.0.0 {
+  # Added to GCC_13.0.0 in i386/libgcc-glibc.ver.
+  __extendbfsf2
+  __floattibf
+  __floatuntibf
+  __truncdfbf2
+  __truncsfbf2
+  __trunctfbf2
+  __truncxfbf2
+  __trunchfbf2
+  # Added to GCC_14.0.0 in i386/libgcc-glibc.ver.
+  __fixxfbitint
+  __fixtfbitint
+  __floatbitintbf
+  __floatbitinthf
+  __floatbitintxf
+  __floatbitinttf
+}
-- 
2.39.2 (Apple Git-143)



[PATCH 3/3][RFC] Initial MEM_BASE population

2024-02-09 Thread Richard Biener
The following attempts to set a MEM_BASE from set_mem_attributes_minus_bitpos
which is often first called on the expanded base of a complicated memory
reference.  For now simply preserve a SYMBOL_REF (fancy unwrapping to be
implemented).

The commented code would transfer some points-to info.

* emit-rtl.cc (set_mem_attributes_minus_bitpos): If ref
is suitable as base, record it.
---
 gcc/emit-rtl.cc | 24 
 1 file changed, 24 insertions(+)

diff --git a/gcc/emit-rtl.cc b/gcc/emit-rtl.cc
index 1cf238d9571..3fb52a87e08 100644
--- a/gcc/emit-rtl.cc
+++ b/gcc/emit-rtl.cc
@@ -2027,6 +2027,12 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int 
objectp,
   if (objectp || TREE_CODE (t) == INDIRECT_REF)
 attrs.align = MAX (attrs.align, TYPE_ALIGN (type));
 
+  /* We usually expand the base of a complicated ref first, so record any
+ base value we can determine.  ???  Probably should use a simplified
+ find_base_value here to unwrap a contained SYMBOL_REF.  */
+  if (GET_CODE (ref) == SYMBOL_REF)
+attrs.base = copy_rtx (ref);
+
   /* If the size is known, we can set that.  */
   tree new_size = TYPE_SIZE_UNIT (type);
 
@@ -2074,6 +2080,24 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int 
objectp,
  0;
  else
as = TYPE_ADDR_SPACE (TREE_TYPE (base));
+
+#if 0
+ /* Set the base RTX based on points-to info.  */
+ if (!attrs.base
+ && (TREE_CODE (base) == MEM_REF
+ || TREE_CODE (base) == TARGET_MEM_REF)
+ && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME
+ && SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0)))
+   {
+ auto pt = _NAME_PTR_INFO (TREE_OPERAND (base, 0))->pt;
+ if (pt->nonlocal
+ && !pt->anything
+ && !pt->escaped
+ && !pt->ipa_escaped
+ && bitmap_empty_p (pt->vars))
+   attrs.base = arg_base_value;
+   }
+#endif
}
 
   /* If this expression uses it's parent's alias set, mark it such
-- 
2.35.3


[PATCH 2/3] Remove get_reg_base_value

2024-02-09 Thread Richard Biener
This makes recorded reg base values private to alias.cc, its computation
should go away but find_base_term can be used to cross-check that
we don't regress when only having MEM_BASE eventually.

Selective scheduling wouldn't need to avoid renaming regs with a base
value anymore, so this removes this only outside use.

* alias.cc (get_reg_base_value): Make static.
* rtl.h (get_reg_base_value): Remove.
* sel-sched.cc (init_regs_for_mode): Do not worry about
REG_BASE_VALUE when recording regs for renaming.
---
 gcc/alias.cc | 2 +-
 gcc/rtl.h| 1 -
 gcc/sel-sched.cc | 4 
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/gcc/alias.cc b/gcc/alias.cc
index 48633aff699..541994c5047 100644
--- a/gcc/alias.cc
+++ b/gcc/alias.cc
@@ -1656,7 +1656,7 @@ record_set (rtx dest, const_rtx set, void *data 
ATTRIBUTE_UNUSED)
 
 /* Return REG_BASE_VALUE for REGNO.  Selective scheduler uses this to avoid
using hard registers with non-null REG_BASE_VALUE for renaming.  */
-rtx
+static rtx
 get_reg_base_value (unsigned int regno)
 {
   return (*reg_base_value)[regno];
diff --git a/gcc/rtl.h b/gcc/rtl.h
index c84334cb945..3fcaaec49e4 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -4496,7 +4496,6 @@ extern bool may_be_sp_based_p (rtx);
 extern rtx gen_hard_reg_clobber (machine_mode, unsigned int);
 extern rtx get_reg_known_value (unsigned int);
 extern bool get_reg_known_equiv_p (unsigned int);
-extern rtx get_reg_base_value (unsigned int);
 extern rtx extract_mem_from_operand (rtx);
 
 #ifdef STACK_REGS
diff --git a/gcc/sel-sched.cc b/gcc/sel-sched.cc
index 17b71127960..77ba100023d 100644
--- a/gcc/sel-sched.cc
+++ b/gcc/sel-sched.cc
@@ -1082,9 +1082,13 @@ init_regs_for_mode (machine_mode mode)
 /* Can't use regs which aren't saved by
the prologue.  */
 || !TEST_HARD_REG_BIT (sel_hrd.regs_ever_used, cur_reg + i)
+   /* It's fine to elide this since REG_BASE_VALUE will go away
+  and the info is in the MEMs itself.  */
+#if 0
/* Can't use regs with non-null REG_BASE_VALUE, because adjusting
   it affects aliasing globally and invalidates all AV sets.  */
|| get_reg_base_value (cur_reg + i)
+#endif
 #ifdef LEAF_REGISTERS
 /* We can't use a non-leaf register if we're in a
leaf function.  */
-- 
2.35.3



[PATCH 1/3][RFC] Add MEM_BASE

2024-02-09 Thread Richard Biener
The following adds a 'base' member to the RTL memory attributes,
recording the base value as base_alias_check expects and currently
find_base_{value,term} compute.  It is expected to either contain
a SYMBOL_REF or a special ADDRESS, see base_alias_check for reference.

With this patch nothing sets MEM_BASE but RTL alias analysis would
prefer any recorded MEM_BASE over dynamically computing it.

* rtl.h (mem_attrs::base): New member.
(MEM_BASE): New.
* emit-rtl.h (set_mem_base): Declare.
* emit-rtl.cc (mem_attrs_eq_p): Handle MEM_BASE.
(mem_attrs::mem_attrs): Initialize it.
(set_mem_attributes_minus_bitpos): Preserve it.
(set_mem_base): New function.
* alias.cc (true_dependence_1): Prefer MEM_BASE over
using find_base_term.
(write_dependence_p): Likewise.
(may_alias_p): Likewise.
* print-rtl.cc (rtx_writer::print_rtx): Print MEM_BASE.
---
 gcc/alias.cc | 26 ++
 gcc/emit-rtl.cc  | 18 +-
 gcc/emit-rtl.h   |  3 +++
 gcc/print-rtl.cc |  6 ++
 gcc/rtl.h|  7 +++
 5 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/gcc/alias.cc b/gcc/alias.cc
index 808e2095d9b..48633aff699 100644
--- a/gcc/alias.cc
+++ b/gcc/alias.cc
@@ -2934,7 +2934,6 @@ true_dependence_1 (const_rtx mem, machine_mode mem_mode, 
rtx mem_addr,
   const_rtx x, rtx x_addr, bool mem_canonicalized)
 {
   rtx true_mem_addr;
-  rtx base;
   int ret;
 
   gcc_checking_assert (mem_canonicalized ? (mem_addr != NULL_RTX)
@@ -2981,13 +2980,17 @@ true_dependence_1 (const_rtx mem, machine_mode 
mem_mode, rtx mem_addr,
   if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
 return true;
 
-  base = find_base_term (x_addr);
+  rtx base = MEM_BASE (x);
+  if (!base)
+base = find_base_term (x_addr);
   if (base && (GET_CODE (base) == LABEL_REF
   || (GET_CODE (base) == SYMBOL_REF
   && CONSTANT_POOL_ADDRESS_P (base
 return false;
 
-  rtx mem_base = find_base_term (true_mem_addr);
+  rtx mem_base = MEM_BASE (mem);
+  if (!mem_base)
+mem_base = find_base_term (true_mem_addr);
   if (! base_alias_check (x_addr, base, true_mem_addr, mem_base,
  GET_MODE (x), mem_mode))
 return false;
@@ -3045,7 +3048,6 @@ write_dependence_p (const_rtx mem,
 {
   rtx mem_addr;
   rtx true_mem_addr, true_x_addr;
-  rtx base;
   int ret;
 
   gcc_checking_assert (x_canonicalized
@@ -3088,7 +3090,9 @@ write_dependence_p (const_rtx mem,
   if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
 return true;
 
-  base = find_base_term (true_mem_addr);
+  rtx base = MEM_BASE (mem);
+  if (!base)
+base = find_base_term (true_mem_addr);
   if (! writep
   && base
   && (GET_CODE (base) == LABEL_REF
@@ -3096,7 +3100,9 @@ write_dependence_p (const_rtx mem,
  && CONSTANT_POOL_ADDRESS_P (base
 return false;
 
-  rtx x_base = find_base_term (true_x_addr);
+  rtx x_base = MEM_BASE (x);
+  if (!x_base)
+x_base = find_base_term (true_x_addr);
   if (! base_alias_check (true_x_addr, x_base, true_mem_addr, base,
  GET_MODE (x), GET_MODE (mem)))
 return false;
@@ -3211,8 +3217,12 @@ may_alias_p (const_rtx mem, const_rtx x)
   if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
 return true;
 
-  rtx x_base = find_base_term (x_addr);
-  rtx mem_base = find_base_term (mem_addr);
+  rtx x_base = MEM_BASE (x);
+  if (!x_base)
+x_base = find_base_term (x_addr);
+  rtx mem_base = MEM_BASE (mem);
+  if (!mem_base)
+mem_base = find_base_term (mem_addr);
   if (! base_alias_check (x_addr, x_base, mem_addr, mem_base,
  GET_MODE (x), GET_MODE (mem_addr)))
 return false;
diff --git a/gcc/emit-rtl.cc b/gcc/emit-rtl.cc
index 1856fa4884f..1cf238d9571 100644
--- a/gcc/emit-rtl.cc
+++ b/gcc/emit-rtl.cc
@@ -368,7 +368,10 @@ mem_attrs_eq_p (const class mem_attrs *p, const class 
mem_attrs *q)
  && p->addrspace == q->addrspace
  && (p->expr == q->expr
  || (p->expr != NULL_TREE && q->expr != NULL_TREE
- && operand_equal_p (p->expr, q->expr, 0;
+ && operand_equal_p (p->expr, q->expr, 0)))
+ && (p->base == q->base
+ || (p->base != NULL_RTX && q->base != NULL_RTX
+ && rtx_equal_p (p->base, q->base;
 }
 
 /* Set MEM's memory attributes so that they are the same as ATTRS.  */
@@ -1828,6 +1831,7 @@ operand_subword_force (rtx op, poly_uint64 offset, 
machine_mode mode)
 
 mem_attrs::mem_attrs ()
   : expr (NULL_TREE),
+base (NULL_RTX),
 offset (0),
 size (0),
 alias (0),
@@ -1985,6 +1989,7 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int 
objectp,
   /* ??? Can this ever happen?  Calling this routine on a MEM that
 already carries memory attributes should probably be invalid.  */
   attrs.expr = refattrs->expr;
+  attrs.base = 

[PATCH] rtl-optimization/113597 - recover base term for argument pointers

2024-02-09 Thread Richard Biener
The following allows a base term to be derived from an existing
MEM_EXPR, notably the points-to set of a MEM_REF base.  For the
testcase in the PR this helps RTL DSE elide stores to a stack
temporary.  This covers pointers to NONLOCAL which can be mapped
to arg_base_value, helping to disambiguate against other special
bases (ADDRESS) as well as PARM_DECL accesses.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

This is an attempt to recover some of the losses from dumbing down
find_base_{term,value}.  I did give my ideas how to properly do
this during stage1 a start, I will post a short incomplete RFC series
later today.

OK for trunk?

I've included all languages in testing and also tested with -m32 but
details of RTL alias analysis might escape me ...

Thanks,
Richard.

PR rtl-optimization/113597
* alias.cc (find_base_term): Add argument for the whole mem
and derive a base term from its MEM_EXPR.
(true_dependence_1): Pass down the MEMs to find_base_term.
(write_dependence_p): Likewise.
(may_alias_p): Likewise.
---
 gcc/alias.cc | 43 ---
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/gcc/alias.cc b/gcc/alias.cc
index 6fad4b29d31..e33c56b0e80 100644
--- a/gcc/alias.cc
+++ b/gcc/alias.cc
@@ -40,6 +40,9 @@ along with GCC; see the file COPYING3.  If not see
 #include "rtl-iter.h"
 #include "cgraph.h"
 #include "ipa-utils.h"
+#include "stringpool.h"
+#include "value-range.h"
+#include "tree-ssanames.h"
 
 /* The aliasing API provided here solves related but different problems:
 
@@ -190,6 +193,10 @@ static struct {
arguments, since we do not know at this level whether accesses
based on different arguments can alias.  The ADDRESS has id 0.
 
+   This is solely useful to disambiguate against other ADDRESS
+   bases as we know incoming pointers cannot point to local
+   stack, frame or argument space.
+
  2. stack_pointer_rtx, frame_pointer_rtx, hard_frame_pointer_rtx
(if distinct from frame_pointer_rtx) and arg_pointer_rtx.
Each of these rtxes has a separate ADDRESS associated with it,
@@ -2113,12 +2120,34 @@ find_base_term (rtx x, vec, 32> visited_vals;
   rtx res = find_base_term (x, visited_vals);
   for (unsigned i = 0; i < visited_vals.length (); ++i)
 visited_vals[i].first->locs = visited_vals[i].second;
+  if (!res && mem && MEM_EXPR (mem))
+{
+  tree base = get_base_address (MEM_EXPR (mem));
+  if (TREE_CODE (base) == PARM_DECL
+ && DECL_RTL_SET_P (base))
+   /* We need to look at how we expanded a PARM_DECL.  It might be in
+  the argument space (UNIQUE_BASE_VALUE_ARGP) or it might
+  be spilled (UNIQUE_BASE_VALUE_FP/UNIQUE_BASE_VALUE_HFP).  */
+   res = find_base_term (DECL_RTL (base));
+  else if (TREE_CODE (base) == MEM_REF
+  && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME
+  && SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0)))
+   {
+ auto pt = _NAME_PTR_INFO (TREE_OPERAND (base, 0))->pt;
+ if (pt->nonlocal
+ && !pt->anything
+ && !pt->escaped
+ && !pt->ipa_escaped
+ && bitmap_empty_p (pt->vars))
+   res = arg_base_value;
+   }
+}
   return res;
 }
 
@@ -3035,13 +3064,13 @@ true_dependence_1 (const_rtx mem, machine_mode 
mem_mode, rtx mem_addr,
   if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
 return true;
 
-  base = find_base_term (x_addr);
+  base = find_base_term (x_addr, x);
   if (base && (GET_CODE (base) == LABEL_REF
   || (GET_CODE (base) == SYMBOL_REF
   && CONSTANT_POOL_ADDRESS_P (base
 return false;
 
-  rtx mem_base = find_base_term (true_mem_addr);
+  rtx mem_base = find_base_term (true_mem_addr, mem);
   if (! base_alias_check (x_addr, base, true_mem_addr, mem_base,
  GET_MODE (x), mem_mode))
 return false;
@@ -3142,7 +3171,7 @@ write_dependence_p (const_rtx mem,
   if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
 return true;
 
-  base = find_base_term (true_mem_addr);
+  base = find_base_term (true_mem_addr, mem);
   if (! writep
   && base
   && (GET_CODE (base) == LABEL_REF
@@ -3150,7 +3179,7 @@ write_dependence_p (const_rtx mem,
  && CONSTANT_POOL_ADDRESS_P (base
 return false;
 
-  rtx x_base = find_base_term (true_x_addr);
+  rtx x_base = find_base_term (true_x_addr, x);
   if (! base_alias_check (true_x_addr, x_base, true_mem_addr, base,
  GET_MODE (x), GET_MODE (mem)))
 return false;
@@ -3265,8 +3294,8 @@ may_alias_p (const_rtx mem, const_rtx x)
   if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
 return true;
 
-  rtx x_base = find_base_term (x_addr);
-  rtx mem_base = find_base_term (mem_addr);
+  rtx x_base = find_base_term (x_addr, x);
+  rtx mem_base = find_base_term (mem_addr, mem);
   if (! base_alias_check (x_addr, x_base, 

Re: [PATCH] middle-end/113576 - zero padding of vector bools when expanding compares

2024-02-09 Thread Jakub Jelinek
On Fri, Feb 09, 2024 at 10:24:25AM +0100, Richard Biener wrote:
> The following zeros paddings of vector bools when expanding compares
> and the mode used for the compare is an integer mode.  In that case
> targets cannot distinguish between a 4 element and 8 element vector
> compare (both get to the QImode compare optab) so we have to do the
> job in the middle-end.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> 
> OK?
> 
> Thanks,
> Richard.
> 
>   PR middle-end/113576
>   * expr.cc (do_store_flag): For vector bool compares of vectors
>   with padding zero that.
>   * dojump.cc (do_compare_and_jump): Likewise.

LGTM, but please give Richard time to chime in.

Jakub



Re: [COMMITTED 13/25] gccrs: remove old generics hack to reuse generic symbols from previous seg

2024-02-09 Thread Jakub Jelinek
On Wed, Feb 07, 2024 at 12:43:59PM +0100, arthur.co...@embecosm.com wrote:
> From: Philip Herron 
> 
> This patch introduces one regression because generics are getting better
> understood over time. The code here used to apply generics with the same
> symbol from previous segments which was a bit of a hack with out limited
> inference variable support. The regression looks like it will be related
> to another issue which needs to default integer inference variables much
> more aggresivly to default integer.
> 
> Fixes #2723
> 
> gcc/rust/ChangeLog:
> 
>   * typecheck/rust-hir-type-check-path.cc 
> (TypeCheckExpr::resolve_segments): remove hack
> 
> gcc/testsuite/ChangeLog:
> 
>   * rust/compile/issue-1773.rs: Moved to...
>   * rust/compile/issue-1773.rs.bak: ...here.

Please don't use such suffixes in the testsuite.
Either delete the testcase, or xfail it somehow until the bug is fixed.

Jakub



Patch ping

2024-02-09 Thread Jakub Jelinek
Hi!

I'd like to ping 2 patches:

https://gcc.gnu.org/pipermail/gcc-patches/2024-January/644580.html
PR113617 P1 - Handle private COMDAT function symbol reference in readonly data 
section

More details in the
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/thread.html#644121
and
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/thread.html#644486
threads.

and

https://gcc.gnu.org/pipermail/gcc-patches/2024-February/644701.html
Introduce HOST_SIZE_T_PRINT_UNSIGNED etc. macros to fix LLP64 host build issue

Both have been successfully bootstrapped/regtested on x86_64-linux and
i686-linux, the latter has been tested by Jonathan on Windows too.
The alternative is start using %zu (AFAIK we only do that in libgccjit which
isn't supported everywhere and while it is C99, not sure if all supported
host C libraries support it), or change ira-conflicts.cc to
--- gcc/ira-conflicts.cc2024-02-01 21:03:57.339193085 +0100
+++ gcc/ira-conflicts.cc2024-02-09 10:41:39.201150644 +0100
@@ -151,8 +151,8 @@ build_conflict_bit_table (void)
 fprintf
   (ira_dump_file,
"+++Allocating %ld bytes for conflict table (uncompressed size %ld)\n",
-   (long) allocated_words_num * sizeof (IRA_INT_TYPE),
-   (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
+   (long) (allocated_words_num * sizeof (IRA_INT_TYPE)),
+   (long) (object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE)));
 
   objects_live = sparseset_alloc (ira_objects_num);
   for (i = 0; i < ira_max_point; i++)
Note, we have many more cases where we use %ld or %lu to print
size_t values (ideally %zd/%zu if we can assume it on all hosts, or
with the above introduced HOST_SIZE_T_PRINT*, the problem with the
%ld/%lu and casts is that it truncates the values on LLP64 hosts (aka
%Windows).

Jakub



Re: [PATCH] expand: Fix asm goto expansion [PR113415]

2024-02-09 Thread Richard Biener
On Fri, 9 Feb 2024, Jakub Jelinek wrote:

> Hi!
> 
> The asm goto expansion ICEs on the following testcase (which normally
> is rejected later), because expand_asm_stmt emits the code to copy
> the large var out of the out operand to its memory location into
> after_rtl_seq ... after_rtl_end sequence and because it is asm goto,
> it duplicates the sequence on each successor edge of the asm goto.
> The problem is that with -mstringop-strategy=byte_loop that sequence
> contains loops, so CODE_LABELs, JUMP_INSNs, with other strategies
> could contain CALL_INSNs etc.
> But the copying is done using a loop doing
> emit_insn (copy_insn (PATTERN (curr)));
> which does the right thing solely for INSNs, it will do the wrong thing
> for JUMP_INSNs, CALL_INSNs, CODE_LABELs (with RTL checking even ICE on
> them), BARRIERs and the like.
> 
> The following patch partially fixes it (with the hope that such stuff only
> occurs in asms that really can't be accepted; if one uses say "=rm" or
> "=g" constraint then the operand uses the memory directly and nothing is
> copied) by using the
> duplicate_insn_chain function which is used e.g. in RTL loop unrolling and
> which can handle JUMP_INSNs, CALL_INSNs, BARRIERs etc.
> As it is meant to operate on sequences inside of basic blocks, it doesn't
> handle CODE_LABELs (well, it skips them), so if we need a solution that
> will be correct at runtime here for those cases, we'd need to do further
> work (e.g. still use duplicate_insn_chain, but if we notice any CODE_LABELs,
> walk the sequence again, add copies of the CODE_LABELs and then remap
> references to the old CODE_LABELs in the copied sequence to the new ones).
> Because as is now, if the code in one of the sequence copies (where the
> CODE_LABELs have been left out) decides to jump to such a CODE_LABEL, it
> will jump to the CODE_LABEL which has been in the original sequence (which
> the code emits on the last edge, after all, duplicating the sequence
> EDGE_COUNT times and throwing away the original was wasteful, compared to
> doing that just EDGE_COUNT - 1 times and using the original.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

> Or do need to handle even CODE_LABELs?

I guess we'll do when we run into it?  I'm not sure we can constrain
asm gotos in a way such complicated "after" sequences never happen,
right?  A sorry () might be a possibility for cases we don't handle.

Richard.

> 2024-02-09  Jakub Jelinek  
> 
>   PR middle-end/113415
>   * cfgexpand.cc (expand_asm_stmt): For asm goto, use
>   duplicate_insn_chain to duplicate after_rtl_seq sequence instead
>   of hand written loop with emit_insn of copy_insn and emit original
>   after_rtl_seq on the last edge.
> 
>   * gcc.target/i386/pr113415.c: New test.
> 
> --- gcc/cfgexpand.cc.jj   2024-01-24 13:11:21.011469855 +0100
> +++ gcc/cfgexpand.cc  2024-02-08 18:22:04.699621085 +0100
> @@ -3671,16 +3671,21 @@ expand_asm_stmt (gasm *stmt)
>   {
> edge e;
> edge_iterator ei;
> -   
> +   unsigned int cnt = EDGE_COUNT (gimple_bb (stmt)->succs);
> +
> FOR_EACH_EDGE (e, ei, gimple_bb (stmt)->succs)
>   {
> -   start_sequence ();
> -   for (rtx_insn *curr = after_rtl_seq;
> -curr != NULL_RTX;
> -curr = NEXT_INSN (curr))
> - emit_insn (copy_insn (PATTERN (curr)));
> -   rtx_insn *copy = get_insns ();
> -   end_sequence ();
> +   rtx_insn *copy;
> +   if (--cnt == 0)
> + copy = after_rtl_seq;
> +   else
> + {
> +   start_sequence ();
> +   duplicate_insn_chain (after_rtl_seq, after_rtl_end,
> + NULL, NULL);
> +   copy = get_insns ();
> +   end_sequence ();
> + }
> insert_insn_on_edge (copy, e);
>   }
>   }
> --- gcc/testsuite/gcc.target/i386/pr113415.c.jj   2024-02-08 
> 18:26:27.622966847 +0100
> +++ gcc/testsuite/gcc.target/i386/pr113415.c  2024-02-08 18:26:11.336193222 
> +0100
> @@ -0,0 +1,11 @@
> +/* PR middle-end/113415 */
> +/* { dg-do compile } */
> +/* { dg-options "-mstringop-strategy=byte_loop" } */
> +
> +void
> +foo (void)
> +{
> +  unsigned long arr[64];
> +lab:
> +  __asm__ goto ("" : "=r" (arr) : : : lab);  /* { dg-error "impossible 
> constraint in 'asm'" } */
> +}
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)


Re: [PATCH] lower-bitint: Fix up additions of EH edges [PR113818]

2024-02-09 Thread Richard Biener
On Fri, 9 Feb 2024, Jakub Jelinek wrote:

> Hi!
> 
> Due to -fnon-call-exceptions the bitint lowering adds new EH edges
> in various places, so that the EH edge points from handling (e.g. load or
> store) of each of the limbs.  The problem is that the EH edge destination
> as shown in the testcase can have some PHIs.  If it is just a virtual
> PHI, no big deal, the pass uses TODO_update_ssa_only_virtuals, but if
> it has other PHIs, I think we need to copy the values from the preexisting
> corresponding EH edge (which is from the original stmt to the EH pad)
> to the newly added EH edge, so that the PHI arguments are the same rather
> than missing (which ICEs during checking at the end of the pass).
> 
> This patch adds a function to do that and uses it whenever adding EH edges.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2024-02-09  Jakub Jelinek  
> 
>   PR tree-optimization/113818
>   * gimple-lower-bitint.cc (add_eh_edge): New function.
>   (bitint_large_huge::handle_load,
>   bitint_large_huge::lower_mergeable_stmt,
>   bitint_large_huge::lower_muldiv_stmt): Use it.
> 
>   * gcc.dg/bitint-89.c: New test.
> 
> --- gcc/gimple-lower-bitint.cc.jj 2024-02-08 14:33:36.033220098 +0100
> +++ gcc/gimple-lower-bitint.cc2024-02-08 17:29:38.182386934 +0100
> @@ -1681,6 +1681,27 @@ bitint_large_huge::handle_cast (tree lhs
>return NULL_TREE;
>  }
>  
> +/* Add a new EH edge from SRC to EH_EDGE->dest, where EH_EDGE
> +   is an older EH edge, and except for virtual PHIs duplicate the
> +   PHI argument from the EH_EDGE to the new EH edge.  */
> +
> +static void
> +add_eh_edge (basic_block src, edge eh_edge)
> +{
> +  edge e = make_edge (src, eh_edge->dest, EDGE_EH);
> +  e->probability = profile_probability::very_unlikely ();
> +  for (gphi_iterator gsi = gsi_start_phis (eh_edge->dest);
> +   !gsi_end_p (gsi); gsi_next ())
> +{
> +  gphi *phi = gsi.phi ();
> +  tree lhs = gimple_phi_result (phi);
> +  if (virtual_operand_p (lhs))
> + continue;
> +  const phi_arg_d *arg = gimple_phi_arg (phi, eh_edge->dest_idx);
> +  add_phi_arg (phi, arg->def, e, arg->locus);
> +}
> +}
> +
>  /* Helper function for handle_stmt method, handle a load from memory.  */
>  
>  tree
> @@ -1756,8 +1777,7 @@ bitint_large_huge::handle_load (gimple *
> if (eh_edge)
>   {
> edge e = split_block (gsi_bb (m_gsi), g);
> -   make_edge (e->src, eh_edge->dest, EDGE_EH)->probability
> - = profile_probability::very_unlikely ();
> +   add_eh_edge (e->src, eh_edge);
> m_gsi = gsi_after_labels (e->dest);
> if (gsi_bb (save_gsi) == e->src)
>   {
> @@ -1876,8 +1896,7 @@ bitint_large_huge::handle_load (gimple *
>   {
> edge e = split_block (gsi_bb (m_gsi), g);
> m_gsi = gsi_after_labels (e->dest);
> -   make_edge (e->src, eh_edge->dest, EDGE_EH)->probability
> - = profile_probability::very_unlikely ();
> +   add_eh_edge (e->src, eh_edge);
>   }
>   }
> if (conditional)
> @@ -1934,8 +1953,7 @@ normal_load:
>   {
> edge e = split_block (gsi_bb (m_gsi), g);
> m_gsi = gsi_after_labels (e->dest);
> -   make_edge (e->src, eh_edge->dest, EDGE_EH)->probability
> - = profile_probability::very_unlikely ();
> +   add_eh_edge (e->src, eh_edge);
>   }
>if (tree_fits_uhwi_p (idx))
>   {
> @@ -2554,8 +2572,8 @@ bitint_large_huge::lower_mergeable_stmt
>   {
> edge e = split_block (gsi_bb (m_gsi), g);
> m_gsi = gsi_after_labels (e->dest);
> -   make_edge (e->src, eh_pad, EDGE_EH)->probability
> - = profile_probability::very_unlikely ();
> +   add_eh_edge (e->src,
> +find_edge (gimple_bb (stmt), eh_pad));
>   }
>   }
> if (kind == bitint_prec_large)
> @@ -2633,8 +2651,8 @@ bitint_large_huge::lower_mergeable_stmt
>   {
> edge e = split_block (gsi_bb (m_gsi), g);
> m_gsi = gsi_after_labels (e->dest);
> -   make_edge (e->src, eh_pad, EDGE_EH)->probability
> - = profile_probability::very_unlikely ();
> +   add_eh_edge (e->src,
> +find_edge (gimple_bb (stmt), eh_pad));
>   }
>   }
> if (new_bb)
> @@ -2777,8 +2795,7 @@ bitint_large_huge::lower_mergeable_stmt
>   {
> edge e = split_block (gsi_bb (m_gsi), g);
> m_gsi = gsi_after_labels (e->dest);
> -   make_edge (e->src, eh_pad, 

[PATCH] expand: Fix asm goto expansion [PR113415]

2024-02-09 Thread Jakub Jelinek
Hi!

The asm goto expansion ICEs on the following testcase (which normally
is rejected later), because expand_asm_stmt emits the code to copy
the large var out of the out operand to its memory location into
after_rtl_seq ... after_rtl_end sequence and because it is asm goto,
it duplicates the sequence on each successor edge of the asm goto.
The problem is that with -mstringop-strategy=byte_loop that sequence
contains loops, so CODE_LABELs, JUMP_INSNs, with other strategies
could contain CALL_INSNs etc.
But the copying is done using a loop doing
emit_insn (copy_insn (PATTERN (curr)));
which does the right thing solely for INSNs, it will do the wrong thing
for JUMP_INSNs, CALL_INSNs, CODE_LABELs (with RTL checking even ICE on
them), BARRIERs and the like.

The following patch partially fixes it (with the hope that such stuff only
occurs in asms that really can't be accepted; if one uses say "=rm" or
"=g" constraint then the operand uses the memory directly and nothing is
copied) by using the
duplicate_insn_chain function which is used e.g. in RTL loop unrolling and
which can handle JUMP_INSNs, CALL_INSNs, BARRIERs etc.
As it is meant to operate on sequences inside of basic blocks, it doesn't
handle CODE_LABELs (well, it skips them), so if we need a solution that
will be correct at runtime here for those cases, we'd need to do further
work (e.g. still use duplicate_insn_chain, but if we notice any CODE_LABELs,
walk the sequence again, add copies of the CODE_LABELs and then remap
references to the old CODE_LABELs in the copied sequence to the new ones).
Because as is now, if the code in one of the sequence copies (where the
CODE_LABELs have been left out) decides to jump to such a CODE_LABEL, it
will jump to the CODE_LABEL which has been in the original sequence (which
the code emits on the last edge, after all, duplicating the sequence
EDGE_COUNT times and throwing away the original was wasteful, compared to
doing that just EDGE_COUNT - 1 times and using the original.

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

Or do need to handle even CODE_LABELs?

2024-02-09  Jakub Jelinek  

PR middle-end/113415
* cfgexpand.cc (expand_asm_stmt): For asm goto, use
duplicate_insn_chain to duplicate after_rtl_seq sequence instead
of hand written loop with emit_insn of copy_insn and emit original
after_rtl_seq on the last edge.

* gcc.target/i386/pr113415.c: New test.

--- gcc/cfgexpand.cc.jj 2024-01-24 13:11:21.011469855 +0100
+++ gcc/cfgexpand.cc2024-02-08 18:22:04.699621085 +0100
@@ -3671,16 +3671,21 @@ expand_asm_stmt (gasm *stmt)
{
  edge e;
  edge_iterator ei;
- 
+ unsigned int cnt = EDGE_COUNT (gimple_bb (stmt)->succs);
+
  FOR_EACH_EDGE (e, ei, gimple_bb (stmt)->succs)
{
- start_sequence ();
- for (rtx_insn *curr = after_rtl_seq;
-  curr != NULL_RTX;
-  curr = NEXT_INSN (curr))
-   emit_insn (copy_insn (PATTERN (curr)));
- rtx_insn *copy = get_insns ();
- end_sequence ();
+ rtx_insn *copy;
+ if (--cnt == 0)
+   copy = after_rtl_seq;
+ else
+   {
+ start_sequence ();
+ duplicate_insn_chain (after_rtl_seq, after_rtl_end,
+   NULL, NULL);
+ copy = get_insns ();
+ end_sequence ();
+   }
  insert_insn_on_edge (copy, e);
}
}
--- gcc/testsuite/gcc.target/i386/pr113415.c.jj 2024-02-08 18:26:27.622966847 
+0100
+++ gcc/testsuite/gcc.target/i386/pr113415.c2024-02-08 18:26:11.336193222 
+0100
@@ -0,0 +1,11 @@
+/* PR middle-end/113415 */
+/* { dg-do compile } */
+/* { dg-options "-mstringop-strategy=byte_loop" } */
+
+void
+foo (void)
+{
+  unsigned long arr[64];
+lab:
+  __asm__ goto ("" : "=r" (arr) : : : lab);/* { dg-error "impossible 
constraint in 'asm'" } */
+}

Jakub



Re: [PATCH] lower-bitint: Attempt not to emit always true conditions in handle_cast [PR113774]

2024-02-09 Thread Richard Biener
On Fri, 9 Feb 2024, Jakub Jelinek wrote:

> Hi!
> 
> The following patch is the optimization part of PR113774, where in
> handle_cast we emit some conditionals which are always true and presumably
> VRP would figure that out later and clean it up, except that instead
> thread1 is invoked and threads everything through the conditions, so we end
> up with really ugly code which is hard to be cleaned up later and then
> run into PR113831 VN bug and miscompile stuff.
> 
> handle_cast computes low and high as limb indexes, where idx < low
> doesn't need any special treatment, just uses the operand's limb,
> idx >= high cases all the bits in the limb are an extension (so, for
> unsigned widening cast all those bits are 0, for signed widening cast
> all those bits are equal to the in earlier code computed sign mask,
> narrowing cast don't trigger this code) and then the idx == low && idx <
> high case if it exists need special treatment (some bits are copied, others
> extended, or all bits are copied but sign mask needs to be computed).
> 
> The code already attempted to optimize away some unneeded casts, in the
> first hunk below e.g. for the case like 257 -> 321 bit extension, where
> low is 4 and high 5 and we use a loop handling the first 4 limbs (2
> iterations) with m_upwards_2limb 4 - no special handling is needed in the
> loop, and the special handling is done on the first limb after the loop
> and then the last limb after the loop gets the extension only, or
> in the second hunk where can emit a single comparison instead of
> 2 e.g. for the low == high case - that must be a zero extension from
> multiple of limb bits, say 192 -> 328, or for the case where we know
> the idx == low case happens in the other limb processed in the loop, not
> the current one.
> 
> But the testcase shows further cases where we always know some of the
> comparisons can be folded to true/false, in particular there is
> 255 -> 257 bit zero extension, so low 3, high 4, m_upwards_2limb 4.
> The loop handles 2 limbs at the time and for the first limb we were
> emitting idx < low ? operand[idx] : 0; but because idx goes from 0
> with step 2 2 iterations, idx < 3 is always true, so we can just
> emit operand[idx].  This is handled in the first hunk.  In addition
> to fixing it (that is the " - m_first" part in there) I've rewritten
> it using low to make it more readable.
> 
> Similarly, in the other limb we were emitting
> idx + 1 <= low ? (idx + 1 == low ? operand[idx] & 0x7ffff : operand[idx]) 
> : 0
> but idx + 1 <= 3 is always true in the loop, so all we should emit is
> idx + 1 == low ? operand[idx] & 0x7ffff : operand[idx],
> Unfortunately for the latter, when single_comparison is true, we emit
> just one comparison, but the code which fills the branches will fill it
> with the operand[idx] and 0 cases (for zero extension, for sign extension
> similarly), not the operand[idx] (aka copy) and operand[idx] & 0x7ffff
> (aka most significant limb of the narrower precision) cases.  Instead
> of making the code less readable by using single_comparison for that and
> handling it in the code later differently I've chosen to just emit
> a condition which will be always true and let cfg cleanup clean it up.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2024-02-09  Jakub Jelinek  
> 
>   PR tree-optimization/113774
>   * gimple-lower-bitint.cc (bitint_large_huge::handle_cast): Don't
>   emit any comparison if m_first and low + 1 is equal to
>   m_upwards_2limb, simplify condition for that.  If not
>   single_comparison, not m_first and we can prove that the idx <= low
>   comparison will be always true, emit instead of idx <= low
>   comparison low <= low such that cfg cleanup will optimize it at
>   the end of the pass.
> 
>   * gcc.dg/torture/bitint-57.c: New test.
> 
> --- gcc/gimple-lower-bitint.cc.jj 2024-02-08 12:49:40.435313811 +0100
> +++ gcc/gimple-lower-bitint.cc2024-02-08 14:33:36.033220098 +0100
> @@ -1350,9 +1350,7 @@ bitint_large_huge::handle_cast (tree lhs
>if (!tree_fits_uhwi_p (idx))
>   {
> if (m_upwards_2limb
> -   && (m_upwards_2limb * limb_prec
> -   <= ((unsigned) TYPE_PRECISION (rhs_type)
> -   - !TYPE_UNSIGNED (rhs_type
> +   && low >= m_upwards_2limb - m_first)
>   {
> rhs1 = handle_operand (rhs1, idx);
> if (m_first)
> @@ -1363,8 +1361,21 @@ bitint_large_huge::handle_cast (tree lhs
>   }
> bool single_comparison
>   = low == high || (m_upwards_2limb && (low & 1) == m_first);
> +   tree idxc = idx;
> +   if (!single_comparison
> +   && m_upwards_2limb
> +   && !m_first
> +   && low + 1 == m_upwards_2limb)
> + /* In this case we know that idx <= low always,
> +so effectively we just needs a single comparison,
> +idx < low or 

[PATCH] middle-end/113576 - zero padding of vector bools when expanding compares

2024-02-09 Thread Richard Biener
The following zeros paddings of vector bools when expanding compares
and the mode used for the compare is an integer mode.  In that case
targets cannot distinguish between a 4 element and 8 element vector
compare (both get to the QImode compare optab) so we have to do the
job in the middle-end.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

OK?

Thanks,
Richard.

PR middle-end/113576
* expr.cc (do_store_flag): For vector bool compares of vectors
with padding zero that.
* dojump.cc (do_compare_and_jump): Likewise.
---
 gcc/dojump.cc | 16 
 gcc/expr.cc   | 17 +
 2 files changed, 33 insertions(+)

diff --git a/gcc/dojump.cc b/gcc/dojump.cc
index e2d2b3cb111..ec2a365e488 100644
--- a/gcc/dojump.cc
+++ b/gcc/dojump.cc
@@ -1266,6 +1266,7 @@ do_compare_and_jump (tree treeop0, tree treeop1, enum 
rtx_code signed_code,
   machine_mode mode;
   int unsignedp;
   enum rtx_code code;
+  unsigned HOST_WIDE_INT nunits;
 
   /* Don't crash if the comparison was erroneous.  */
   op0 = expand_normal (treeop0);
@@ -1308,6 +1309,21 @@ do_compare_and_jump (tree treeop0, tree treeop1, enum 
rtx_code signed_code,
   emit_insn (targetm.gen_canonicalize_funcptr_for_compare (new_op1, op1));
   op1 = new_op1;
 }
+  /* For boolean vectors with less than mode precision precision
+ make sure to fill padding with consistent values.  */
+  else if (VECTOR_BOOLEAN_TYPE_P (type)
+  && SCALAR_INT_MODE_P (mode)
+  && TYPE_VECTOR_SUBPARTS (type).is_constant ()
+  && maybe_ne (GET_MODE_PRECISION (mode), nunits))
+{
+  gcc_assert (code == EQ || code == NE);
+  op0 = expand_binop (mode, and_optab, op0,
+ GEN_INT ((1 << nunits) - 1), NULL_RTX,
+ true, OPTAB_WIDEN);
+  op1 = expand_binop (mode, and_optab, op1,
+ GEN_INT ((1 << nunits) - 1), NULL_RTX,
+ true, OPTAB_WIDEN);
+}
 
   do_compare_rtx_and_jump (op0, op1, code, unsignedp, treeop0, mode,
   ((mode == BLKmode)
diff --git a/gcc/expr.cc b/gcc/expr.cc
index fc5e998e329..096081fdc53 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -13502,6 +13502,7 @@ do_store_flag (sepops ops, rtx target, machine_mode 
mode)
   rtx op0, op1;
   rtx subtarget = target;
   location_t loc = ops->location;
+  unsigned HOST_WIDE_INT nunits;
 
   arg0 = ops->op0;
   arg1 = ops->op1;
@@ -13694,6 +13695,22 @@ do_store_flag (sepops ops, rtx target, machine_mode 
mode)
 
   expand_operands (arg0, arg1, subtarget, , , EXPAND_NORMAL);
 
+  /* For boolean vectors with less than mode precision precision
+ make sure to fill padding with consistent values.  */
+  if (VECTOR_BOOLEAN_TYPE_P (type)
+  && SCALAR_INT_MODE_P (operand_mode)
+  && TYPE_VECTOR_SUBPARTS (type).is_constant ()
+  && maybe_ne (GET_MODE_PRECISION (operand_mode), nunits))
+{
+  gcc_assert (code == EQ || code == NE);
+  op0 = expand_binop (mode, and_optab, op0,
+ GEN_INT ((1 << nunits) - 1), NULL_RTX,
+ true, OPTAB_WIDEN);
+  op1 = expand_binop (mode, and_optab, op1,
+ GEN_INT ((1 << nunits) - 1), NULL_RTX,
+ true, OPTAB_WIDEN);
+}
+
   if (target == 0)
 target = gen_reg_rtx (mode);
 
-- 
2.35.3


[PATCH] lower-bitint: Fix up additions of EH edges [PR113818]

2024-02-09 Thread Jakub Jelinek
Hi!

Due to -fnon-call-exceptions the bitint lowering adds new EH edges
in various places, so that the EH edge points from handling (e.g. load or
store) of each of the limbs.  The problem is that the EH edge destination
as shown in the testcase can have some PHIs.  If it is just a virtual
PHI, no big deal, the pass uses TODO_update_ssa_only_virtuals, but if
it has other PHIs, I think we need to copy the values from the preexisting
corresponding EH edge (which is from the original stmt to the EH pad)
to the newly added EH edge, so that the PHI arguments are the same rather
than missing (which ICEs during checking at the end of the pass).

This patch adds a function to do that and uses it whenever adding EH edges.

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

2024-02-09  Jakub Jelinek  

PR tree-optimization/113818
* gimple-lower-bitint.cc (add_eh_edge): New function.
(bitint_large_huge::handle_load,
bitint_large_huge::lower_mergeable_stmt,
bitint_large_huge::lower_muldiv_stmt): Use it.

* gcc.dg/bitint-89.c: New test.

--- gcc/gimple-lower-bitint.cc.jj   2024-02-08 14:33:36.033220098 +0100
+++ gcc/gimple-lower-bitint.cc  2024-02-08 17:29:38.182386934 +0100
@@ -1681,6 +1681,27 @@ bitint_large_huge::handle_cast (tree lhs
   return NULL_TREE;
 }
 
+/* Add a new EH edge from SRC to EH_EDGE->dest, where EH_EDGE
+   is an older EH edge, and except for virtual PHIs duplicate the
+   PHI argument from the EH_EDGE to the new EH edge.  */
+
+static void
+add_eh_edge (basic_block src, edge eh_edge)
+{
+  edge e = make_edge (src, eh_edge->dest, EDGE_EH);
+  e->probability = profile_probability::very_unlikely ();
+  for (gphi_iterator gsi = gsi_start_phis (eh_edge->dest);
+   !gsi_end_p (gsi); gsi_next ())
+{
+  gphi *phi = gsi.phi ();
+  tree lhs = gimple_phi_result (phi);
+  if (virtual_operand_p (lhs))
+   continue;
+  const phi_arg_d *arg = gimple_phi_arg (phi, eh_edge->dest_idx);
+  add_phi_arg (phi, arg->def, e, arg->locus);
+}
+}
+
 /* Helper function for handle_stmt method, handle a load from memory.  */
 
 tree
@@ -1756,8 +1777,7 @@ bitint_large_huge::handle_load (gimple *
  if (eh_edge)
{
  edge e = split_block (gsi_bb (m_gsi), g);
- make_edge (e->src, eh_edge->dest, EDGE_EH)->probability
-   = profile_probability::very_unlikely ();
+ add_eh_edge (e->src, eh_edge);
  m_gsi = gsi_after_labels (e->dest);
  if (gsi_bb (save_gsi) == e->src)
{
@@ -1876,8 +1896,7 @@ bitint_large_huge::handle_load (gimple *
{
  edge e = split_block (gsi_bb (m_gsi), g);
  m_gsi = gsi_after_labels (e->dest);
- make_edge (e->src, eh_edge->dest, EDGE_EH)->probability
-   = profile_probability::very_unlikely ();
+ add_eh_edge (e->src, eh_edge);
}
}
  if (conditional)
@@ -1934,8 +1953,7 @@ normal_load:
{
  edge e = split_block (gsi_bb (m_gsi), g);
  m_gsi = gsi_after_labels (e->dest);
- make_edge (e->src, eh_edge->dest, EDGE_EH)->probability
-   = profile_probability::very_unlikely ();
+ add_eh_edge (e->src, eh_edge);
}
   if (tree_fits_uhwi_p (idx))
{
@@ -2554,8 +2572,8 @@ bitint_large_huge::lower_mergeable_stmt
{
  edge e = split_block (gsi_bb (m_gsi), g);
  m_gsi = gsi_after_labels (e->dest);
- make_edge (e->src, eh_pad, EDGE_EH)->probability
-   = profile_probability::very_unlikely ();
+ add_eh_edge (e->src,
+  find_edge (gimple_bb (stmt), eh_pad));
}
}
  if (kind == bitint_prec_large)
@@ -2633,8 +2651,8 @@ bitint_large_huge::lower_mergeable_stmt
{
  edge e = split_block (gsi_bb (m_gsi), g);
  m_gsi = gsi_after_labels (e->dest);
- make_edge (e->src, eh_pad, EDGE_EH)->probability
-   = profile_probability::very_unlikely ();
+ add_eh_edge (e->src,
+  find_edge (gimple_bb (stmt), eh_pad));
}
}
  if (new_bb)
@@ -2777,8 +2795,7 @@ bitint_large_huge::lower_mergeable_stmt
{
  edge e = split_block (gsi_bb (m_gsi), g);
  m_gsi = gsi_after_labels (e->dest);
- make_edge (e->src, eh_pad, EDGE_EH)->probability
-   = profile_probability::very_unlikely ();
+ add_eh_edge (e->src, find_edge (gimple_bb (stmt), eh_pad));
}
  

[PATCH] lower-bitint: Attempt not to emit always true conditions in handle_cast [PR113774]

2024-02-09 Thread Jakub Jelinek
Hi!

The following patch is the optimization part of PR113774, where in
handle_cast we emit some conditionals which are always true and presumably
VRP would figure that out later and clean it up, except that instead
thread1 is invoked and threads everything through the conditions, so we end
up with really ugly code which is hard to be cleaned up later and then
run into PR113831 VN bug and miscompile stuff.

handle_cast computes low and high as limb indexes, where idx < low
doesn't need any special treatment, just uses the operand's limb,
idx >= high cases all the bits in the limb are an extension (so, for
unsigned widening cast all those bits are 0, for signed widening cast
all those bits are equal to the in earlier code computed sign mask,
narrowing cast don't trigger this code) and then the idx == low && idx <
high case if it exists need special treatment (some bits are copied, others
extended, or all bits are copied but sign mask needs to be computed).

The code already attempted to optimize away some unneeded casts, in the
first hunk below e.g. for the case like 257 -> 321 bit extension, where
low is 4 and high 5 and we use a loop handling the first 4 limbs (2
iterations) with m_upwards_2limb 4 - no special handling is needed in the
loop, and the special handling is done on the first limb after the loop
and then the last limb after the loop gets the extension only, or
in the second hunk where can emit a single comparison instead of
2 e.g. for the low == high case - that must be a zero extension from
multiple of limb bits, say 192 -> 328, or for the case where we know
the idx == low case happens in the other limb processed in the loop, not
the current one.

But the testcase shows further cases where we always know some of the
comparisons can be folded to true/false, in particular there is
255 -> 257 bit zero extension, so low 3, high 4, m_upwards_2limb 4.
The loop handles 2 limbs at the time and for the first limb we were
emitting idx < low ? operand[idx] : 0; but because idx goes from 0
with step 2 2 iterations, idx < 3 is always true, so we can just
emit operand[idx].  This is handled in the first hunk.  In addition
to fixing it (that is the " - m_first" part in there) I've rewritten
it using low to make it more readable.

Similarly, in the other limb we were emitting
idx + 1 <= low ? (idx + 1 == low ? operand[idx] & 0x7ffff : operand[idx]) : 0
but idx + 1 <= 3 is always true in the loop, so all we should emit is
idx + 1 == low ? operand[idx] & 0x7ffff : operand[idx],
Unfortunately for the latter, when single_comparison is true, we emit
just one comparison, but the code which fills the branches will fill it
with the operand[idx] and 0 cases (for zero extension, for sign extension
similarly), not the operand[idx] (aka copy) and operand[idx] & 0x7ffff
(aka most significant limb of the narrower precision) cases.  Instead
of making the code less readable by using single_comparison for that and
handling it in the code later differently I've chosen to just emit
a condition which will be always true and let cfg cleanup clean it up.

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

2024-02-09  Jakub Jelinek  

PR tree-optimization/113774
* gimple-lower-bitint.cc (bitint_large_huge::handle_cast): Don't
emit any comparison if m_first and low + 1 is equal to
m_upwards_2limb, simplify condition for that.  If not
single_comparison, not m_first and we can prove that the idx <= low
comparison will be always true, emit instead of idx <= low
comparison low <= low such that cfg cleanup will optimize it at
the end of the pass.

* gcc.dg/torture/bitint-57.c: New test.

--- gcc/gimple-lower-bitint.cc.jj   2024-02-08 12:49:40.435313811 +0100
+++ gcc/gimple-lower-bitint.cc  2024-02-08 14:33:36.033220098 +0100
@@ -1350,9 +1350,7 @@ bitint_large_huge::handle_cast (tree lhs
   if (!tree_fits_uhwi_p (idx))
{
  if (m_upwards_2limb
- && (m_upwards_2limb * limb_prec
- <= ((unsigned) TYPE_PRECISION (rhs_type)
- - !TYPE_UNSIGNED (rhs_type
+ && low >= m_upwards_2limb - m_first)
{
  rhs1 = handle_operand (rhs1, idx);
  if (m_first)
@@ -1363,8 +1361,21 @@ bitint_large_huge::handle_cast (tree lhs
}
  bool single_comparison
= low == high || (m_upwards_2limb && (low & 1) == m_first);
+ tree idxc = idx;
+ if (!single_comparison
+ && m_upwards_2limb
+ && !m_first
+ && low + 1 == m_upwards_2limb)
+   /* In this case we know that idx <= low always,
+  so effectively we just needs a single comparison,
+  idx < low or idx == low, but we'd need to emit different
+  code for the 2 branches than single_comparison normally
+  emits.  So, instead of special-casing that, emit a
+ 

Re: [PATCH v2] libstdc++: optimize bit iterators assuming normalization [PR110807]

2024-02-09 Thread Torbjorn SVENSSON




On 2024-02-07 17:36, Jonathan Wakely wrote:



On Wed, 7 Feb 2024 at 16:25, Torbjorn SVENSSON 
mailto:torbjorn.svens...@foss.st.com>> 
wrote:


Hi,

Is it okay to backport e39b3e02c27bd771a07e385f9672ecf1a45ced77 to
releases/gcc-13?


It would also need 807f47497f17ed50be91f0f879308cb6fa063966

Please test with that as well, and OK for both if all goes well.


Test was passed on arm-none-eabi for both these.
Pushed as 5d684a5f7f82b1425cac5eaa11dccc3b51a62d44 and 
adef1e0ebeb5055ed4e8776fd3f73c9d84821eaa.


Re: [PATCH v2] testsuite: Pattern does not match when using --specs=nano.specs

2024-02-09 Thread Torbjorn SVENSSON




On 2024-02-09 01:07, Mike Stump wrote:

On Feb 8, 2024, at 9:44 AM, Torbjörn SVENSSON  
wrote:


Changes since v1:
- Replaced .* with [^\r\n]* to avoid matching newline.

Ok for trunk and releases/gcc-13?


Ok.


Pushed as 1175d1b35ce7bf8ee7c9b37b334370f01eb95335 and 
810b0b3f75c454da3f6b5722870716796d2d7a83.


Re: [PATCH] c++: for contracts, cdtors never return this

2024-02-09 Thread Torbjorn SVENSSON




On 2024-02-07 20:46, Jason Merrill wrote:

On 2/7/24 11:22, Torbjorn SVENSSON wrote:

Hi,

Is it okay to backport 71804526d3a71a8c0f189a89ce3aa615784bfd8b to 
releases/gcc-13?


OK.


Pushed as eae51472f68d9f922aa3a1a636f81467bfdae87a.


[PATCH] bitint: Fix handling of VIEW_CONVERT_EXPRs to minimally supported huge INTEGER_TYPEs [PR113783]

2024-02-09 Thread Jakub Jelinek
Hi!

On the following testcases memcpy lowering folds the calls to
reading and writing of MEM_REFs with huge INTEGER_TYPEs - uint256_t
with OImode or uint512_t with XImode.  Further optimization turn
the load from MEM_REF from the large/huge _BitInt var into VIEW_CONVERT_EXPR
from it to the uint256_t/uint512_t.  The backend doesn't really
support those except for "movoi"/"movxi" insns, so it isn't possible
to handle it like casts to supportable INTEGER_TYPEs where we can
construct those from individual limbs - there are no OImode/XImode shifts
and the like we can use.
So, the following patch makes sure for such VCEs that the SSA_NAME operand
of the VCE lives in memory and then turns it into a VIEW_CONVERT_EXPR so
that we actually load the OImode/XImode integer from memory (i.e. a mov).
We need to make sure those aren't merged with other
operations in the gimple_lower_bitint hunks.
For SSA_NAMEs which have underlying VAR_DECLs that is all we need, those
VAR_DECL have ARRAY_TYPEs.
For SSA_NAMEs which have underlying PARM_DECLs or RESULT_DECLs those have
BITINT_TYPE and I had to tweak expand_expr_real_1 for that so that it
doesn't try convert_modes on those when one of the modes is BLKmode - we
want to fall through into the adjust_address on the MEM.

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

2024-02-09  Jakub Jelinek  

PR tree-optimization/113783
* gimple-lower-bitint.cc (bitint_large_huge::lower_stmt): Look
through VIEW_CONVERT_EXPR for final cast checks.  Handle
VIEW_CONVERT_EXPRs from large/huge _BitInt to > MAX_FIXED_MODE_SIZE
INTEGER_TYPEs.
(gimple_lower_bitint): Don't merge mergeable operations or other
casts with VIEW_CONVERT_EXPRs to > MAX_FIXED_MODE_SIZE INTEGER_TYPEs.
* expr.cc (expand_expr_real_1): Don't use convert_modes if either
mode is BLKmode.

* gcc.dg/bitint-88.c: New test.

--- gcc/gimple-lower-bitint.cc.jj   2024-02-06 12:58:48.296021497 +0100
+++ gcc/gimple-lower-bitint.cc  2024-02-08 12:49:40.435313811 +0100
@@ -5263,6 +5263,8 @@ bitint_large_huge::lower_stmt (gimple *s
 {
   lhs = gimple_assign_lhs (stmt);
   tree rhs1 = gimple_assign_rhs1 (stmt);
+  if (TREE_CODE (rhs1) == VIEW_CONVERT_EXPR)
+   rhs1 = TREE_OPERAND (rhs1, 0);
   if (TREE_CODE (TREE_TYPE (lhs)) == BITINT_TYPE
  && bitint_precision_kind (TREE_TYPE (lhs)) >= bitint_prec_large
  && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
@@ -5273,6 +5275,44 @@ bitint_large_huge::lower_stmt (gimple *s
   || POINTER_TYPE_P (TREE_TYPE (lhs
{
  final_cast_p = true;
+ if (TREE_CODE (TREE_TYPE (lhs)) == INTEGER_TYPE
+ && TYPE_PRECISION (TREE_TYPE (lhs)) > MAX_FIXED_MODE_SIZE
+ && gimple_assign_rhs_code (stmt) == VIEW_CONVERT_EXPR)
+   {
+ /* Handle VIEW_CONVERT_EXPRs to not generally supported
+huge INTEGER_TYPEs like uint256_t or uint512_t.  These
+are usually emitted from memcpy folding and backends
+support moves with them but that is usually it.  */
+ if (TREE_CODE (rhs1) == INTEGER_CST)
+   {
+ rhs1 = fold_unary (VIEW_CONVERT_EXPR, TREE_TYPE (lhs),
+rhs1);
+ gcc_assert (rhs1 && TREE_CODE (rhs1) == INTEGER_CST);
+ gimple_assign_set_rhs1 (stmt, rhs1);
+ gimple_assign_set_rhs_code (stmt, INTEGER_CST);
+ update_stmt (stmt);
+ return;
+   }
+ gcc_assert (TREE_CODE (rhs1) == SSA_NAME);
+ if (SSA_NAME_IS_DEFAULT_DEF (rhs1)
+ && (!SSA_NAME_VAR (rhs1) || VAR_P (SSA_NAME_VAR (rhs1
+   {
+ tree var = create_tmp_reg (TREE_TYPE (lhs));
+ rhs1 = get_or_create_ssa_default_def (cfun, var);
+ gimple_assign_set_rhs1 (stmt, rhs1);
+ gimple_assign_set_rhs_code (stmt, SSA_NAME);
+   }
+ else
+   {
+ int part = var_to_partition (m_map, rhs1);
+ gcc_assert (m_vars[part] != NULL_TREE);
+ rhs1 = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (lhs),
+m_vars[part]);
+ gimple_assign_set_rhs1 (stmt, rhs1);
+   }
+ update_stmt (stmt);
+ return;
+   }
  if (TREE_CODE (rhs1) == SSA_NAME
  && (m_names == NULL
  || !bitmap_bit_p (m_names, SSA_NAME_VERSION (rhs1
@@ -6103,7 +6143,13 @@ gimple_lower_bitint (void)
  if (gimple_assign_cast_p (use_stmt))
{
  tree lhs = gimple_assign_lhs (use_stmt);
- if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
+ if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
+ /* Don't merge 

Re: LoongArch: Backport r14-4674 "LoongArch: Delete macro definition ASM_OUTPUT_ALIGN_WITH_NOP."?

2024-02-09 Thread Xi Ruoyao
On Fri, 2024-02-09 at 00:02 +0800, chenglulu wrote:
> 
> 在 2024/2/7 上午12:23, Xi Ruoyao 写道:
> > Hi Lulu,
> > 
> > I'm proposing to backport r14-4674 "LoongArch: Delete macro definition
> > ASM_OUTPUT_ALIGN_WITH_NOP." to releases/gcc-12 and releases/gcc-13.  The
> > reasons:
> > 
> > 1. Strictly speaking, the old ASM_OUTPUT_ALIGN_WITH_NOP macro may cause
> > a correctness issue.  For example, a developer may use -falign-
> > functions=16 and then use the low 4 bits of a function pointer to encode
> > some metainfo.  Then ASM_OUTPUT_ALIGN_WITH_NOP causes the functions not
> > really aligned to a 16 bytes boundary, causing some breakage.
> > 
> > 2. With Binutils-2.42,  ASM_OUTPUT_ALIGN_WITH_NOP can cause illegal
> > opcodes.  For example:
> > 
> > .globl _start
> > _start:
> > .balign 32
> > nop
> > nop
> > nop
> > addi.d $a0, $r0, 1
> > .balign 16,54525952,4
> > addi.d $a0, $a0, 1
> > 
> > is assembled and linked to:
> > 
> > 0220 <_start>:
> >   220:  0340    nop
> >   224:  0340    nop
> >   228:  0340    nop
> >   22c:  02c00404    li.d$a0, 1
> >   230:      .word   0x   # <== OOPS!
> >   234:  02c00484    addi.d  $a0, $a0, 1
> > 
> > Arguably this is a bug in GAS (it should at least error out for the
> > unsupported case where .balign 16,54525952,4 appears with -mrelax; I'd
> > prefer it to support the 3-operand .align directive even -mrelax for
> > reasons I've given in [1]).  But we can at least work it around by
> > removing ASM_OUTPUT_ALIGN_WITH_NOP to allow using GCC 13.3 with Binutils
> > 2.42.
> > 
> > 3. Without ASM_OUTPUT_ALIGN_WITH_NOP, GCC just outputs something like
> > ".align 5" which works as expected since Binutils-2.38.
> > 
> > 4. GCC < 14 does not have a default setting of -falign-*, so changing
> > this won't affect anyone who do not specify -falign-* explicitly.
> > 
> > [1]:https://github.com/loongson-community/discussions/issues/41#issuecomment-1925872603
> > 
> > Is it OK to backport r14-4674 into releases/gcc-12 and releases/gcc-13
> > then?
> > 
> Ok, I agree with you.
> 
> Thanks!

Oops, with Binutils-2.41 GAS will fail to assemble some conditional
branches if we do this :(.

Not sure what to do (maybe backporting both this and a simplified
version of PR112330 fix?)  Let's reconsider after the holiday...

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University