Re: [libc-coord] Add new ABI '__memcmpeq()' to libc

2021-09-17 Thread Richard Biener via Gcc
On Thu, Sep 16, 2021 at 10:36 PM Joseph Myers  wrote:
>
> On Thu, 16 Sep 2021, Chris Kennelly wrote:
>
> > In terms of relying on the feature:  If __memcmpeq is ever exposed as an a
> > simple alias for memcmp (since the notes mention that it's a valid
> > implementation), does that open up the possibility of depending on the
> > bcmp-like behavior that we were trying to escape?
>
> The proposal is as an ABI only (compilers would generate calls to
> __memcmpeq from boolean uses of memcmp, users wouldn't write calls to
> __memcmpeq directly, __memcmpeq wouldn't be declared in installed libc
> headers).  If such dependence arises, that would suggest a compiler bug
> wrongly generating such calls for non-boolean memcmp uses.

So the compiler would emit a call to __memcmpeq and at the same time
emit a weak alias of __memcmpeq to memcmp so the program links
when the libc version targeted does not provide __memcmpeq?  Or would
glibc through  magically communicate the availability of the new ABI
without actually declaring the function?
(I'm not sure whether a GCC build-time decision via configure is the
very best idea)

Richard.

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


Re: [libc-coord] Add new ABI '__memcmpeq()' to libc

2021-09-17 Thread Florian Weimer via Gcc
* Richard Biener via Gcc:

> On Thu, Sep 16, 2021 at 10:36 PM Joseph Myers  wrote:
>>
>> On Thu, 16 Sep 2021, Chris Kennelly wrote:
>>
>> > In terms of relying on the feature:  If __memcmpeq is ever exposed as an a
>> > simple alias for memcmp (since the notes mention that it's a valid
>> > implementation), does that open up the possibility of depending on the
>> > bcmp-like behavior that we were trying to escape?
>>
>> The proposal is as an ABI only (compilers would generate calls to
>> __memcmpeq from boolean uses of memcmp, users wouldn't write calls to
>> __memcmpeq directly, __memcmpeq wouldn't be declared in installed libc
>> headers).  If such dependence arises, that would suggest a compiler bug
>> wrongly generating such calls for non-boolean memcmp uses.
>
> So the compiler would emit a call to __memcmpeq and at the same time
> emit a weak alias of __memcmpeq to memcmp so the program links
> when the libc version targeted does not provide __memcmpeq?  Or would
> glibc through  magically communicate the availability of the new ABI
> without actually declaring the function?

I do not think ELF provides that capability.

We can add a declaration to  to communicate the availability.
I think this is how glibc (and other libcs) communicate the availability
of non-standard interfaces to GCC.

> (I'm not sure whether a GCC build-time decision via configure is the
> very best idea)

If libstdc++ or libgcc_s have a symbol dependency on glibc 2.35 for
other (unrelated) reasons, would the build-time dependency be less of a
concern?  Because another such dependency exists?

Thanks,
Florian



Re: [libc-coord] Add new ABI '__memcmpeq()' to libc

2021-09-17 Thread Richard Biener via Gcc
On Fri, Sep 17, 2021 at 10:08 AM Florian Weimer  wrote:
>
> * Richard Biener via Gcc:
>
> > On Thu, Sep 16, 2021 at 10:36 PM Joseph Myers  
> > wrote:
> >>
> >> On Thu, 16 Sep 2021, Chris Kennelly wrote:
> >>
> >> > In terms of relying on the feature:  If __memcmpeq is ever exposed as an 
> >> > a
> >> > simple alias for memcmp (since the notes mention that it's a valid
> >> > implementation), does that open up the possibility of depending on the
> >> > bcmp-like behavior that we were trying to escape?
> >>
> >> The proposal is as an ABI only (compilers would generate calls to
> >> __memcmpeq from boolean uses of memcmp, users wouldn't write calls to
> >> __memcmpeq directly, __memcmpeq wouldn't be declared in installed libc
> >> headers).  If such dependence arises, that would suggest a compiler bug
> >> wrongly generating such calls for non-boolean memcmp uses.
> >
> > So the compiler would emit a call to __memcmpeq and at the same time
> > emit a weak alias of __memcmpeq to memcmp so the program links
> > when the libc version targeted does not provide __memcmpeq?  Or would
> > glibc through  magically communicate the availability of the new 
> > ABI
> > without actually declaring the function?
>
> I do not think ELF provides that capability.

I guess a weak forwarder should do the trick at the cost of a jmp.

> We can add a declaration to  to communicate the availability.
> I think this is how glibc (and other libcs) communicate the availability
> of non-standard interfaces to GCC.

OK, I guess that's fine.

> > (I'm not sure whether a GCC build-time decision via configure is the
> > very best idea)
>
> If libstdc++ or libgcc_s have a symbol dependency on glibc 2.35 for
> other (unrelated) reasons, would the build-time dependency be less of a
> concern?  Because another such dependency exists?

Not sure, I was thinking that we'd need to re-compile GCC when we
upgrade glibc to make use of the feature.

But then being able to run an executable on a system that does not
provide the ABI but a compatible one (memcmp) might be a nice
thing.

Richard.

> Thanks,
> Florian
>


Re: [libc-coord] Add new ABI '__memcmpeq()' to libc

2021-09-17 Thread Florian Weimer via Gcc
* Richard Biener:

> On Fri, Sep 17, 2021 at 10:08 AM Florian Weimer  wrote:
>>
>> * Richard Biener via Gcc:
>>
>> > On Thu, Sep 16, 2021 at 10:36 PM Joseph Myers  
>> > wrote:
>> >>
>> >> On Thu, 16 Sep 2021, Chris Kennelly wrote:
>> >>
>> >> > In terms of relying on the feature:  If __memcmpeq is ever exposed as 
>> >> > an a
>> >> > simple alias for memcmp (since the notes mention that it's a valid
>> >> > implementation), does that open up the possibility of depending on the
>> >> > bcmp-like behavior that we were trying to escape?
>> >>
>> >> The proposal is as an ABI only (compilers would generate calls to
>> >> __memcmpeq from boolean uses of memcmp, users wouldn't write calls to
>> >> __memcmpeq directly, __memcmpeq wouldn't be declared in installed libc
>> >> headers).  If such dependence arises, that would suggest a compiler bug
>> >> wrongly generating such calls for non-boolean memcmp uses.
>> >
>> > So the compiler would emit a call to __memcmpeq and at the same time
>> > emit a weak alias of __memcmpeq to memcmp so the program links
>> > when the libc version targeted does not provide __memcmpeq?  Or would
>> > glibc through  magically communicate the availability of the new 
>> > ABI
>> > without actually declaring the function?
>>
>> I do not think ELF provides that capability.
>
> I guess a weak forwarder should do the trick at the cost of a jmp.

How would this look like in practice.

The GNU tools do not support weak symbol versions, so if you have a weak
reference to __memcmpeq@GLIBC_2.35, that's still a reference to the
GLIBC_2.35 symbol version.  The glibc 2.34 dynamic loader notes that
version and rejects the binary because GLIBC_2.35 does not exist.

(We should probably stop Cc:ing libc-coord because this is so very
GNU-specific at this point.)

Thanks,
Florian



Re: [libc-coord] Add new ABI '__memcmpeq()' to libc

2021-09-17 Thread Jakub Jelinek via Gcc
On Fri, Sep 17, 2021 at 10:08:34AM +0200, Florian Weimer via Gcc wrote:
> > So the compiler would emit a call to __memcmpeq and at the same time
> > emit a weak alias of __memcmpeq to memcmp so the program links
> > when the libc version targeted does not provide __memcmpeq?  Or would
> > glibc through  magically communicate the availability of the new 
> > ABI
> > without actually declaring the function?
> 
> I do not think ELF provides that capability.
> 
> We can add a declaration to  to communicate the availability.
> I think this is how glibc (and other libcs) communicate the availability
> of non-standard interfaces to GCC.

Yeah, that is what we've done in the past, e.g. in case of stpcpy.

Jakub



Re: [libc-coord] Add new ABI '__memcmpeq()' to libc

2021-09-17 Thread Richard Biener via Gcc
On Fri, Sep 17, 2021 at 10:37 AM Florian Weimer  wrote:
>
> * Richard Biener:
>
> > On Fri, Sep 17, 2021 at 10:08 AM Florian Weimer  wrote:
> >>
> >> * Richard Biener via Gcc:
> >>
> >> > On Thu, Sep 16, 2021 at 10:36 PM Joseph Myers  
> >> > wrote:
> >> >>
> >> >> On Thu, 16 Sep 2021, Chris Kennelly wrote:
> >> >>
> >> >> > In terms of relying on the feature:  If __memcmpeq is ever exposed as 
> >> >> > an a
> >> >> > simple alias for memcmp (since the notes mention that it's a valid
> >> >> > implementation), does that open up the possibility of depending on the
> >> >> > bcmp-like behavior that we were trying to escape?
> >> >>
> >> >> The proposal is as an ABI only (compilers would generate calls to
> >> >> __memcmpeq from boolean uses of memcmp, users wouldn't write calls to
> >> >> __memcmpeq directly, __memcmpeq wouldn't be declared in installed libc
> >> >> headers).  If such dependence arises, that would suggest a compiler bug
> >> >> wrongly generating such calls for non-boolean memcmp uses.
> >> >
> >> > So the compiler would emit a call to __memcmpeq and at the same time
> >> > emit a weak alias of __memcmpeq to memcmp so the program links
> >> > when the libc version targeted does not provide __memcmpeq?  Or would
> >> > glibc through  magically communicate the availability of the 
> >> > new ABI
> >> > without actually declaring the function?
> >>
> >> I do not think ELF provides that capability.
> >
> > I guess a weak forwarder should do the trick at the cost of a jmp.
>
> How would this look like in practice.
>
> The GNU tools do not support weak symbol versions, so if you have a weak
> reference to __memcmpeq@GLIBC_2.35, that's still a reference to the
> GLIBC_2.35 symbol version.  The glibc 2.34 dynamic loader notes that
> version and rejects the binary because GLIBC_2.35 does not exist.

Aww, symbol versions. Yeah, that makes it difficult ...

Anyway, with a declaration available it's good enough I think.

Richard.

> (We should probably stop Cc:ing libc-coord because this is so very
> GNU-specific at this point.)
>
> Thanks,
> Florian
>


ARM32 configury changes, with no FPU as a default

2021-09-17 Thread Matthias Klose
Starting with GCC 8, the configury allows to encode extra features into the
architecture string. Debian and Ubuntu's armhf (hard float) architecture is
configured with

  --with-arch=armv7-a --with-fpu=vfpv3-d16

and now should be configured with

  --with-arch=armv7-a+fp

The --with-fpu configure option is deprecated.  The problem with this approach
is that there is no default for the fpu setting, while old compilers silently
pick up the -mfpu from the configured compiler.

This breaks software which explicitly configures things like -march=armv7-a, or
where the architecture string is embedded in the source as an attribute.  So
going from one place in the compiler about configuring the ABI for a distro
arch, this config now moves to some dozen places in different packages.  Not the
thing I would expect.

Richard tells me that the --with-fpu option goes away in some future GCC
version, just asking here how others solved this issue, or are we the only ones
still building for ARM 32bit?

Thanks, Matthias


Re: ARM32 configury changes, with no FPU as a default

2021-09-17 Thread Florian Weimer via Gcc
* Matthias Klose:

> Starting with GCC 8, the configury allows to encode extra features into the
> architecture string. Debian and Ubuntu's armhf (hard float) architecture is
> configured with
>
>   --with-arch=armv7-a --with-fpu=vfpv3-d16
>
> and now should be configured with
>
>   --with-arch=armv7-a+fp
>
> The --with-fpu configure option is deprecated.  The problem with this approach
> is that there is no default for the fpu setting, while old compilers silently
> pick up the -mfpu from the configured compiler.

FWIW, Fedora uses:

--with-tune=generic-armv7-a --with-arch=armv7-a \
--with-float=hard --with-fpu=vfpv3-d16 --with-abi=aapcs-linux \

Not sure how it is impacted by this change.

> This breaks software which explicitly configures things like
> -march=armv7-a, or where the architecture string is embedded in the
> source as an attribute.  So going from one place in the compiler about
> configuring the ABI for a distro arch, this config now moves to some
> dozen places in different packages.  Not the thing I would expect.

I don't know if we have seen such problems in Fedora.  I don't remember
any reports.

Thanks,
Florian



[PING^2] Re: Fix 'hash_table::expand' to destruct stale Value objects

2021-09-17 Thread Thomas Schwinge
Hi!

On 2021-09-10T10:00:25+0200, I wrote:
> On 2021-09-01T19:31:19-0600, Martin Sebor via Gcc-patches 
>  wrote:
>> On 8/30/21 4:46 AM, Thomas Schwinge wrote:
>>> Ping -- we still need to plug the memory leak; see patch attached, and/or
>>> long discussion here:
>>
>> Thanks for answering my questions.  I have no concerns with going
>> forward with the patch as is.
>
> Thanks, Martin.  Ping for formal approval (and review for using proper
> C++ terminology in the 'gcc/hash-table.h:hash_table::expand' source code
> comment that I'm adding).  Patch again attached, for easy reference.

Ping, once again.


Grüße
 Thomas


>> Just a suggestion/request: unless
>> this patch fixes all the outstanding problems you know of or suspect
>> in this area (leaks/missing dtor calls) and unless you plan to work
>> on those in the near future, please open a bug for them with a brain
>> dump of what you learned.  That should save us time when the day
>> comes to tackle those.
>
> ACK.  I'm not aware of any additional known problems.  (In our email
> discussion, we did have some "vague ideas" for opportunities of
> clarification/clean-up, but these aren't worth filing PRs for; needs
> someone to gain understanding, taking a look.)
>
>
> Grüße
>  Thomas
>
>
>>> On 2021-08-16T14:10:00-0600, Martin Sebor  wrote:
 On 8/16/21 6:44 AM, Thomas Schwinge wrote:
> On 2021-08-12T17:15:44-0600, Martin Sebor via Gcc  wrote:
>> On 8/6/21 10:57 AM, Thomas Schwinge wrote:
>>> So I'm trying to do some C++...  ;-)
>>>
>>> Given:
>>>
>>>/* A map from SSA names or var decls to record fields.  */
>>>typedef hash_map field_map_t;
>>>
>>>/* For each propagation record type, this is a map from SSA 
>>> names or var decls
>>>   to propagate, to the field in the record type that should be 
>>> used for
>>>   transmission and reception.  */
>>>typedef hash_map record_field_map_t;
>>>
>>> Thus, that's a 'hash_map>'.  (I may do that,
>>> right?)  Looking through GCC implementation files, very most of all uses
>>> of 'hash_map' boil down to pointer key ('tree', for example) and
>>> pointer/integer value.
>>
>> Right.  Because most GCC containers rely exclusively on GCC's own
>> uses for testing, if your use case is novel in some way, chances
>> are it might not work as intended in all circumstances.
>>
>> I've wrestled with hash_map a number of times.  A use case that's
>> close to yours (i.e., a non-trivial value type) is in cp/parser.c:
>> see class_to_loc_map_t.
>
> Indeed, at the time you sent this email, I already had started looking
> into that one!  (The Fortran test cases that I originally analyzed, which
> triggered other cases of non-POD/non-trivial destructor, all didn't
> result in a memory leak, because the non-trivial constructor doesn't
> actually allocate any resources dynamically -- that's indeed different in
> this case here.)  ..., and indeed:
>
>> (I don't remember if I tested it for leaks
>> though.  It's used to implement -Wmismatched-tags so compiling
>> a few tests under Valgrind should show if it does leak.)
>
> ... it does leak memory at present.  :-| (See attached commit log for
> details for one example.)
>>>
>>> (Attached "Fix 'hash_table::expand' to destruct stale Value objects"
>>> again.)
>>>
> To that effect, to document the current behavior, I propose to
> "Add more self-tests for 'hash_map' with Value type with non-trivial
> constructor/destructor"
>>>
>>> (We've done that in commit e4f16e9f357a38ec702fb69a0ffab9d292a6af9b
>>> "Add more self-tests for 'hash_map' with Value type with non-trivial
>>> constructor/destructor", quickly followed by bug fix
>>> commit bb04a03c6f9bacc890118b9e12b657503093c2f8
>>> "Make 'gcc/hash-map-tests.c:test_map_of_type_with_ctor_and_dtor_expand'
>>> work on 32-bit architectures [PR101959]".
>>>
> (Also cherry-pick into release branches, eventually?)
>>>
>>> Then:
>>>
>>>record_field_map_t field_map ([...]); // see below
>>>for ([...])
>>>  {
>>>tree record_type = [...];
>>>[...]
>>>bool existed;
>>>field_map_t &fields
>>>  = field_map.get_or_insert (record_type, &existed);
>>>gcc_checking_assert (!existed);
>>>[...]
>>>for ([...])
>>>  fields.put ([...], [...]);
>>>[...]
>>>  }
>>>[stuff that looks up elements from 'field_map']
>>>field_map.empty ();
>>>
>>> This generally works.
>>>
>>> If I instantiate 'record_field_map_t field_map (40);', Valgrind is 
>>> happy.
>>> If however I instantiate 'record_field_map_t field_map (13);' (where 
>>> '13'
>>> would be the default for 'hash_map'), Val

sanitizers support for windows??

2021-09-17 Thread unlvsur unlvsur via Gcc
I find that clang can build sanitizers correctly but libstdc++ does not work 
with sanitizers since it does not support related functionalities with 
sanitizers on windows.

Can we start to add support for sanitizers for windows?? I honestly do not 
think that would take a huge amount of time.

Sent from Mail for Windows



Guidance about how to start Contributing

2021-09-17 Thread Sarthak Bhatnagar via Gcc
Hello Mentors

I am currently in my 3rd year of engineering . I have developed skills in
c++ ,data structures and algorithms and web development too.I have been
working on several projects.Now I am interested in contributing and need
guidance. Please guide me so that I can contribute to GSOC 2022.

Thanks and Regards
Sarthak Bhatnagar


Re: ARM32 configury changes, with no FPU as a default

2021-09-17 Thread Richard Earnshaw via Gcc




On 17/09/2021 11:23, Florian Weimer via Gcc wrote:

* Matthias Klose:


Starting with GCC 8, the configury allows to encode extra features into the
architecture string. Debian and Ubuntu's armhf (hard float) architecture is
configured with

   --with-arch=armv7-a --with-fpu=vfpv3-d16

and now should be configured with

   --with-arch=armv7-a+fp

The --with-fpu configure option is deprecated.  The problem with this approach
is that there is no default for the fpu setting, while old compilers silently
pick up the -mfpu from the configured compiler.


FWIW, Fedora uses:

 --with-tune=generic-armv7-a --with-arch=armv7-a \
--with-float=hard --with-fpu=vfpv3-d16 --with-abi=aapcs-linux \

Not sure how it is impacted by this change.


This breaks software which explicitly configures things like
-march=armv7-a, or where the architecture string is embedded in the
source as an attribute.  So going from one place in the compiler about
configuring the ABI for a distro arch, this config now moves to some
dozen places in different packages.  Not the thing I would expect.


I don't know if we have seen such problems in Fedora.  I don't remember
any reports.



That's still using the now-deprecated --with-fpu option.  I want to 
remove that from GCC eventually in favour of the new way of adding the 
FP configuration as part of the architecture.  Recent versions of the 
Arm architecture do not document separate FPU versions, but just add 
features to the main architecture, so we need to move away from the old 
approach.


R.


Thanks,
Florian



Re: sanitizers support for windows??

2021-09-17 Thread Stephen M. Webb
On 2021-09-17 07:52, unlvsur unlvsur via Libstdc++ wrote:
> I find that clang can build sanitizers correctly but libstdc++ does not work 
> with sanitizers since it does not support related functionalities with 
> sanitizers on windows.
> 
> Can we start to add support for sanitizers for windows?? I honestly do not 
> think that would take a huge amount of time.

Our experience at QNX is that other than ubsan, the santizers require 
considerable interworking with the GNU libc and
are not trivial to port to an OS that uses a different libc. Since these are 
actually third-party software any porting
effort or contribution should probably go through the upstream LLVM project to 
avoid forking.

Don't let that stop you from developing a port though.

-- 
Stephen M. Webb  


Re: [PING^2] Re: Fix 'hash_table::expand' to destruct stale Value objects

2021-09-17 Thread Richard Biener via Gcc
On Fri, Sep 17, 2021 at 1:17 PM Thomas Schwinge  wrote:
>
> Hi!
>
> On 2021-09-10T10:00:25+0200, I wrote:
> > On 2021-09-01T19:31:19-0600, Martin Sebor via Gcc-patches 
> >  wrote:
> >> On 8/30/21 4:46 AM, Thomas Schwinge wrote:
> >>> Ping -- we still need to plug the memory leak; see patch attached, and/or
> >>> long discussion here:
> >>
> >> Thanks for answering my questions.  I have no concerns with going
> >> forward with the patch as is.
> >
> > Thanks, Martin.  Ping for formal approval (and review for using proper
> > C++ terminology in the 'gcc/hash-table.h:hash_table::expand' source code
> > comment that I'm adding).  Patch again attached, for easy reference.
>
> Ping, once again.

I'm happy when a C++ literate approves the main change which I quote as

  new ((void*) q) value_type (std::move (x));
+
+ /* Manually invoke destructor of original object, to counterbalance
+object constructed via placement new.  */
+ x.~value_type ();

but I had the impression that std::move already "moves away" from the source?
That said, the dance above looks iffy, there must be a nicer way to "move"
an object in C++?

What happens if the dtor is deleted btw?  Shouldn't you use sth
like a placement 'delete' instead of invoking a DTOR?

But the above clearly shows I know nothing of C++ :P

Richard.

>
> Grüße
>  Thomas
>
>
> >> Just a suggestion/request: unless
> >> this patch fixes all the outstanding problems you know of or suspect
> >> in this area (leaks/missing dtor calls) and unless you plan to work
> >> on those in the near future, please open a bug for them with a brain
> >> dump of what you learned.  That should save us time when the day
> >> comes to tackle those.
> >
> > ACK.  I'm not aware of any additional known problems.  (In our email
> > discussion, we did have some "vague ideas" for opportunities of
> > clarification/clean-up, but these aren't worth filing PRs for; needs
> > someone to gain understanding, taking a look.)
> >
> >
> > Grüße
> >  Thomas
> >
> >
> >>> On 2021-08-16T14:10:00-0600, Martin Sebor  wrote:
>  On 8/16/21 6:44 AM, Thomas Schwinge wrote:
> > On 2021-08-12T17:15:44-0600, Martin Sebor via Gcc  
> > wrote:
> >> On 8/6/21 10:57 AM, Thomas Schwinge wrote:
> >>> So I'm trying to do some C++...  ;-)
> >>>
> >>> Given:
> >>>
> >>>/* A map from SSA names or var decls to record fields.  */
> >>>typedef hash_map field_map_t;
> >>>
> >>>/* For each propagation record type, this is a map from SSA 
> >>> names or var decls
> >>>   to propagate, to the field in the record type that should 
> >>> be used for
> >>>   transmission and reception.  */
> >>>typedef hash_map record_field_map_t;
> >>>
> >>> Thus, that's a 'hash_map>'.  (I may do 
> >>> that,
> >>> right?)  Looking through GCC implementation files, very most of all 
> >>> uses
> >>> of 'hash_map' boil down to pointer key ('tree', for example) and
> >>> pointer/integer value.
> >>
> >> Right.  Because most GCC containers rely exclusively on GCC's own
> >> uses for testing, if your use case is novel in some way, chances
> >> are it might not work as intended in all circumstances.
> >>
> >> I've wrestled with hash_map a number of times.  A use case that's
> >> close to yours (i.e., a non-trivial value type) is in cp/parser.c:
> >> see class_to_loc_map_t.
> >
> > Indeed, at the time you sent this email, I already had started looking
> > into that one!  (The Fortran test cases that I originally analyzed, 
> > which
> > triggered other cases of non-POD/non-trivial destructor, all didn't
> > result in a memory leak, because the non-trivial constructor doesn't
> > actually allocate any resources dynamically -- that's indeed different 
> > in
> > this case here.)  ..., and indeed:
> >
> >> (I don't remember if I tested it for leaks
> >> though.  It's used to implement -Wmismatched-tags so compiling
> >> a few tests under Valgrind should show if it does leak.)
> >
> > ... it does leak memory at present.  :-| (See attached commit log for
> > details for one example.)
> >>>
> >>> (Attached "Fix 'hash_table::expand' to destruct stale Value objects"
> >>> again.)
> >>>
> > To that effect, to document the current behavior, I propose to
> > "Add more self-tests for 'hash_map' with Value type with non-trivial
> > constructor/destructor"
> >>>
> >>> (We've done that in commit e4f16e9f357a38ec702fb69a0ffab9d292a6af9b
> >>> "Add more self-tests for 'hash_map' with Value type with non-trivial
> >>> constructor/destructor", quickly followed by bug fix
> >>> commit bb04a03c6f9bacc890118b9e12b657503093c2f8
> >>> "Make 'gcc/hash-map-tests.c:test_map_of_type_with_ctor_and_dtor_expand'
> >>> work on 32-bit architectures [PR101959]".
> >>>
> > (Also cherry-pick into release bra

Re: [PING^2] Re: Fix 'hash_table::expand' to destruct stale Value objects

2021-09-17 Thread Jonathan Wakely via Gcc
On Fri, 17 Sept 2021 at 13:08, Richard Biener
 wrote:
>
> On Fri, Sep 17, 2021 at 1:17 PM Thomas Schwinge  
> wrote:
> >
> > Hi!
> >
> > On 2021-09-10T10:00:25+0200, I wrote:
> > > On 2021-09-01T19:31:19-0600, Martin Sebor via Gcc-patches 
> > >  wrote:
> > >> On 8/30/21 4:46 AM, Thomas Schwinge wrote:
> > >>> Ping -- we still need to plug the memory leak; see patch attached, 
> > >>> and/or
> > >>> long discussion here:
> > >>
> > >> Thanks for answering my questions.  I have no concerns with going
> > >> forward with the patch as is.
> > >
> > > Thanks, Martin.  Ping for formal approval (and review for using proper
> > > C++ terminology in the 'gcc/hash-table.h:hash_table::expand' source code
> > > comment that I'm adding).  Patch again attached, for easy reference.
> >
> > Ping, once again.
>
> I'm happy when a C++ literate approves the main change which I quote as
>
>   new ((void*) q) value_type (std::move (x));
> +
> + /* Manually invoke destructor of original object, to counterbalance
> +object constructed via placement new.  */
> + x.~value_type ();
>
> but I had the impression that std::move already "moves away" from the source?

It just casts the argument to an rvalue reference, which allows the
value_type constructor to steal its guts.

> That said, the dance above looks iffy, there must be a nicer way to "move"
> an object in C++?

The code above is doing two things: transfer the resources from x to a
new object at location *q, and then destroy x.

The first part (moving its resources) has nothing to do with
destruction. An object still needs to be destroyed, even if its guts
have been moved to another object.

The second part is destroying the object, to end its lifetime. You
wouldn't usually call a destructor explicitly, because it would be
done automatically at the end of scope for objects on the stack, or
done by delete when you free obejcts on the heap. This is a special
case where the object's lifetime is manually managed in storage that
is manually managed.

>
> What happens if the dtor is deleted btw?

If the destructor is deleted you have created an unusable type that
cannot be stored in containers. It can only be created using new, and
then never destroyed. If you play stupid games, you win stupid prizes.
Don't do that.

> Shouldn't you use sth
> like a placement 'delete' instead of invoking a DTOR?

No, there is no placement delete. This is exactly the right way to
destroy an object in-place.

I haven't read the rest of the patch, but the snippet above looks fine.


Re: [PING^2] Re: Fix 'hash_table::expand' to destruct stale Value objects

2021-09-17 Thread Richard Biener via Gcc
On Fri, Sep 17, 2021 at 2:39 PM Jonathan Wakely  wrote:
>
> On Fri, 17 Sept 2021 at 13:08, Richard Biener
>  wrote:
> >
> > On Fri, Sep 17, 2021 at 1:17 PM Thomas Schwinge  
> > wrote:
> > >
> > > Hi!
> > >
> > > On 2021-09-10T10:00:25+0200, I wrote:
> > > > On 2021-09-01T19:31:19-0600, Martin Sebor via Gcc-patches 
> > > >  wrote:
> > > >> On 8/30/21 4:46 AM, Thomas Schwinge wrote:
> > > >>> Ping -- we still need to plug the memory leak; see patch attached, 
> > > >>> and/or
> > > >>> long discussion here:
> > > >>
> > > >> Thanks for answering my questions.  I have no concerns with going
> > > >> forward with the patch as is.
> > > >
> > > > Thanks, Martin.  Ping for formal approval (and review for using proper
> > > > C++ terminology in the 'gcc/hash-table.h:hash_table::expand' source code
> > > > comment that I'm adding).  Patch again attached, for easy reference.
> > >
> > > Ping, once again.
> >
> > I'm happy when a C++ literate approves the main change which I quote as
> >
> >   new ((void*) q) value_type (std::move (x));
> > +
> > + /* Manually invoke destructor of original object, to 
> > counterbalance
> > +object constructed via placement new.  */
> > + x.~value_type ();
> >
> > but I had the impression that std::move already "moves away" from the 
> > source?
>
> It just casts the argument to an rvalue reference, which allows the
> value_type constructor to steal its guts.
>
> > That said, the dance above looks iffy, there must be a nicer way to "move"
> > an object in C++?
>
> The code above is doing two things: transfer the resources from x to a
> new object at location *q, and then destroy x.
>
> The first part (moving its resources) has nothing to do with
> destruction. An object still needs to be destroyed, even if its guts
> have been moved to another object.
>
> The second part is destroying the object, to end its lifetime. You
> wouldn't usually call a destructor explicitly, because it would be
> done automatically at the end of scope for objects on the stack, or
> done by delete when you free obejcts on the heap. This is a special
> case where the object's lifetime is manually managed in storage that
> is manually managed.
>
> >
> > What happens if the dtor is deleted btw?
>
> If the destructor is deleted you have created an unusable type that
> cannot be stored in containers. It can only be created using new, and
> then never destroyed. If you play stupid games, you win stupid prizes.
> Don't do that.
>
> > Shouldn't you use sth
> > like a placement 'delete' instead of invoking a DTOR?
>
> No, there is no placement delete. This is exactly the right way to
> destroy an object in-place.
>
> I haven't read the rest of the patch, but the snippet above looks fine.

OK, thanks for clarifying.

The patch is OK then.

Thanks,
Richard.


Re: Guidance about how to start Contributing

2021-09-17 Thread Martin Jambor
Hi,

On Fri, Sep 17 2021, Sarthak Bhatnagar via Gcc wrote:
> Hello Mentors
>
> I am currently in my 3rd year of engineering . I have developed skills in
> c++ ,data structures and algorithms and web development too.I have been
> working on several projects.Now I am interested in contributing and need
> guidance. Please guide me so that I can contribute to GSOC 2022.


We are delighted you found contributing to GCC interesting.  At this
time I can only point you to https://gcc.gnu.org/wiki/SummerOfCode and
specifically to https://gcc.gnu.org/wiki/SummerOfCode#Before_you_apply
which explains some of the steps you can take to familiarize yourself
with our code base.

Apart from what is described there, any further advice would depend on
what part of GCC you would like to contribute to.   Most of the projects
listed at the top of the page linked above will still be relevant in
2022.

Good luck,

Martin




Re: [libc-coord] Add new ABI '__memcmpeq()' to libc

2021-09-17 Thread Joseph Myers
On Fri, 17 Sep 2021, Richard Biener via Gcc wrote:

> when the libc version targeted does not provide __memcmpeq?  Or would
> glibc through  magically communicate the availability of the new ABI
> without actually declaring the function?
> (I'm not sure whether a GCC build-time decision via configure is the
> very best idea)

I was supposing a build-time decision (using GCC_GLIBC_VERSION_GTE_IFELSE 
to know if the glibc version on the target definitely has this function).  
But if we add a header declaration, you could check for __memcmpeq being 
declared (and so cover arbitrary C libraries, not just glibc, and avoid 
issues of needing to disable this logic for freestanding compilations, 
which would otherwise be an issue if a glibc-target toolchain is used for 
a freestanding kernel compilation).  The case of people calling 
__builtin_memcmp (or declaring memcmp themselves) without string.h 
included probably isn't one it's important to optimize.

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


Re: [libc-coord] Add new ABI '__memcmpeq()' to libc

2021-09-17 Thread Florian Weimer via Gcc
* Joseph Myers:

> I was supposing a build-time decision (using GCC_GLIBC_VERSION_GTE_IFELSE 
> to know if the glibc version on the target definitely has this function).  
> But if we add a header declaration, you could check for __memcmpeq being 
> declared (and so cover arbitrary C libraries, not just glibc, and avoid 
> issues of needing to disable this logic for freestanding compilations, 
> which would otherwise be an issue if a glibc-target toolchain is used for 
> a freestanding kernel compilation).  The case of people calling 
> __builtin_memcmp (or declaring memcmp themselves) without string.h 
> included probably isn't one it's important to optimize.

The header-less case looks relevant to C++ and other language front
ends, though.  So a GCC_GLIBC_VERSION_GTE_IFELSE check could still make
sense for them.

(Dropping libc-coord.)

Thanks,
Florian



Re: [FYI] bugzilla cleanup

2021-09-17 Thread Richard Earnshaw via Gcc




On 16/09/2021 16:44, Martin Sebor via Gcc wrote:

On 9/14/21 2:10 AM, Andrew Pinski via Gcc wrote:

Hi all,
   I am doing some bugzilla cleanup.  This includes disabling some
components and some versions for new bugs.
So far I have disabled versions before GCC 4 because we have not had a
report from someone for those versions in over 7 years.  I disabled
some versions which are about developmental branches which are
inactive too.
I also disabled the java, libgcj, fastjar, libmudflap, treelang and
libf2c components.

I am in the process of moving away from having an inline-asm component
to an inline-asm keyword instead; this was suggested on IRC and I
agree.  After the current open bugs have moved away from the
inline-asm component, I will disable it also.

If anyone else has any other suggestions that should be done, please
let me know and I will look into doing it.


Re: Keywords: I find it useful to differentiate between two kinds of
diagnostic bugs: false positives and false negatives (the latter for
existing warnings that don't trigger when intended, as opposed to
requests to enhance existing warnings or add new ones). I've been
using Personal Tags for this but it might be useful to others as
well.  If you agree and add the corresponding new keywords
(false-positive and false-negative) I'll set them based on my Tags.

One other suggestion: every once in a while someone asks if
ice-on-invalid-code bugs apply to syntactically well-formed code that
has undefined behavior (I don't believe it does).  It would help to
clarify the Description for this Keyword (and, correspondingly, for
ice-on-valid).  E.g., something like

ice-on-invalid-code: ICE on code that is not syntactically valid.
ice-on-valid-code: ICE on code that is syntactically valid.



What about syntactically valid but semantically invalid code?  I'd call 
that ICE-on-invalid as well.


R.


Martin


Re: [PING^2] Re: Fix 'hash_table::expand' to destruct stale Value objects

2021-09-17 Thread Thomas Schwinge
Hi!

On 2021-09-17T15:03:18+0200, Richard Biener  wrote:
> On Fri, Sep 17, 2021 at 2:39 PM Jonathan Wakely  wrote:
>> On Fri, 17 Sept 2021 at 13:08, Richard Biener
>>  wrote:
>> > On Fri, Sep 17, 2021 at 1:17 PM Thomas Schwinge  
>> > wrote:
>> > > On 2021-09-10T10:00:25+0200, I wrote:
>> > > > On 2021-09-01T19:31:19-0600, Martin Sebor via Gcc-patches 
>> > > >  wrote:
>> > > >> On 8/30/21 4:46 AM, Thomas Schwinge wrote:
>> > > >>> Ping -- we still need to plug the memory leak; see patch attached, 
>> > > >>> [...]

>> > > > Ping for formal approval (and review for using proper
>> > > > C++ terminology in the 'gcc/hash-table.h:hash_table::expand' source 
>> > > > code
>> > > > comment that I'm adding).  Patch again attached, for easy reference.

>> > I'm happy when a C++ literate approves the main change which I quote as
>> >
>> >   new ((void*) q) value_type (std::move (x));
>> > +
>> > + /* Manually invoke destructor of original object, to 
>> > counterbalance
>> > +object constructed via placement new.  */
>> > + x.~value_type ();
>> >
>> > but I had the impression that std::move already "moves away" from the 
>> > source?
>>
>> It just casts the argument to an rvalue reference, which allows the
>> value_type constructor to steal its guts.
>>
>> > That said, the dance above looks iffy, there must be a nicer way to "move"
>> > an object in C++?
>>
>> The code above is doing two things: transfer the resources from x to a
>> new object at location *q, and then destroy x.
>>
>> The first part (moving its resources) has nothing to do with
>> destruction. An object still needs to be destroyed, even if its guts
>> have been moved to another object.
>>
>> The second part is destroying the object, to end its lifetime. You
>> wouldn't usually call a destructor explicitly, because it would be
>> done automatically at the end of scope for objects on the stack, or
>> done by delete when you free obejcts on the heap. This is a special
>> case where the object's lifetime is manually managed in storage that
>> is manually managed.

ACK, and happy that I understood this correctly.

And, thanks for providing some proper C++-esque wording, which I
summarized to replace my original source code comment.

>> > What happens if the dtor is deleted btw?
>>
>> If the destructor is deleted you have created an unusable type that
>> cannot be stored in containers. It can only be created using new, and
>> then never destroyed. If you play stupid games, you win stupid prizes.
>> Don't do that.

Haha!  ;-)

And, by the way, as I understood this: if the destructor is "trivial"
(which includes POD types, for example), the explicit destructor call
here is a no-op.

>> I haven't read the rest of the patch, but the snippet above looks fine.
>
> OK, thanks for clarifying.
>
> The patch is OK then.

Thanks, pushed to master branch
commit 89be17a1b231ade643f28fbe616d53377e069da8
"Fix 'hash_table::expand' to destruct stale Value objects".

Should this be backported to release branches, after a while?


Grüße
 Thomas


-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
>From 89be17a1b231ade643f28fbe616d53377e069da8 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Fri, 13 Aug 2021 18:03:38 +0200
Subject: [PATCH] Fix 'hash_table::expand' to destruct stale Value objects

Thus plugging potentional memory leaks if these have non-trivial
constructor/destructor.

See

and others.

As one example, compilation of 'g++.dg/warn/Wmismatched-tags.C' per
'valgrind --leak-check=full' improves as follows:

 [...]
-
-104 bytes in 1 blocks are definitely lost in loss record 399 of 519
-   at 0x483DFAF: realloc (vg_replace_malloc.c:836)
-   by 0x223B62C: xrealloc (xmalloc.c:179)
-   by 0xA8D848: void va_heap::reserve(vec*&, unsigned int, bool) (vec.h:290)
-   by 0xA8B373: vec::reserve(unsigned int, bool) (vec.h:1858)
-   by 0xA8B277: vec::safe_push(class_decl_loc_t::class_key_loc_t const&) (vec.h:1967)
-   by 0xA57481: class_decl_loc_t::add_or_diag_mismatched_tag(tree_node*, tag_types, bool, bool) (parser.c:32967)
-   by 0xA573E1: class_decl_loc_t::add(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32941)
-   by 0xA56C52: cp_parser_check_class_key(cp_parser*, unsigned int, tag_types, tree_node*, bool, bool) (parser.c:32819)
-   by 0xA3AD12: cp_parser_elaborated_type_specifier(cp_parser*, bool, bool) (parser.c:20227)
-   by 0xA37EF2: cp_parser_type_specifier(cp_parser*, int, cp_decl_specifier_seq*, bool, int*, bool*) (parser.c:18942)
-   by 0xA31CDD: cp_parser_decl_specifier_seq(cp_parser*, int, cp_decl_specifier_seq*, int*) (parser.c

Re: [FYI] bugzilla cleanup

2021-09-17 Thread Martin Sebor via Gcc

On 9/17/21 8:54 AM, Richard Earnshaw wrote:



On 16/09/2021 16:44, Martin Sebor via Gcc wrote:

On 9/14/21 2:10 AM, Andrew Pinski via Gcc wrote:

Hi all,
   I am doing some bugzilla cleanup.  This includes disabling some
components and some versions for new bugs.
So far I have disabled versions before GCC 4 because we have not had a
report from someone for those versions in over 7 years.  I disabled
some versions which are about developmental branches which are
inactive too.
I also disabled the java, libgcj, fastjar, libmudflap, treelang and
libf2c components.

I am in the process of moving away from having an inline-asm component
to an inline-asm keyword instead; this was suggested on IRC and I
agree.  After the current open bugs have moved away from the
inline-asm component, I will disable it also.

If anyone else has any other suggestions that should be done, please
let me know and I will look into doing it.


Re: Keywords: I find it useful to differentiate between two kinds of
diagnostic bugs: false positives and false negatives (the latter for
existing warnings that don't trigger when intended, as opposed to
requests to enhance existing warnings or add new ones). I've been
using Personal Tags for this but it might be useful to others as
well.  If you agree and add the corresponding new keywords
(false-positive and false-negative) I'll set them based on my Tags.

One other suggestion: every once in a while someone asks if
ice-on-invalid-code bugs apply to syntactically well-formed code that
has undefined behavior (I don't believe it does).  It would help to
clarify the Description for this Keyword (and, correspondingly, for
ice-on-valid).  E.g., something like

ice-on-invalid-code: ICE on code that is not syntactically valid.
ice-on-valid-code: ICE on code that is syntactically valid.



What about syntactically valid but semantically invalid code?  I'd call 
that ICE-on-invalid as well.


My understanding of the "valid" in the keyword is that it refers
only to syntactic validity AKA well-formedness.

But I only set the keyword to help with bug triage, I don't actually
rely on it for anything myself.  The interpretation should be based
on what its main consumers use it for.  I expect Richard Biener (CC'd)
might be using it for his release planning activities and so might
want to weigh in on this.

Martin

PS In bugs reported for my code in the middle end (often having to
do with detecting semantically invalid AKA undefined code), I use
ice-on-valid for well-formed test cases with undefined behavior.
I set ice-on-invalid only for the less common cases of syntactically
invalid code that somehow makes it into the middle-end or that's
analyzed in the front end.  If this isn't right I'd like to know.


Re: [libc-coord] Add new ABI '__memcmpeq()' to libc

2021-09-17 Thread Martin Sebor via Gcc

On 9/17/21 3:12 AM, Jakub Jelinek via Gcc wrote:

On Fri, Sep 17, 2021 at 10:08:34AM +0200, Florian Weimer via Gcc wrote:

So the compiler would emit a call to __memcmpeq and at the same time
emit a weak alias of __memcmpeq to memcmp so the program links
when the libc version targeted does not provide __memcmpeq?  Or would
glibc through  magically communicate the availability of the new ABI
without actually declaring the function?


I do not think ELF provides that capability.

We can add a declaration to  to communicate the availability.
I think this is how glibc (and other libcs) communicate the availability
of non-standard interfaces to GCC.


Yeah, that is what we've done in the past, e.g. in case of stpcpy.


The stpcpy hack has been a cause of mysterious bugs as discussed
in pr82429.  It means that programs that declare standard functions
without #including  or use GCC built-ins directly are
enalized by ending up with suboptimal code than those that do
include the header.

It's also a subtlety that not all of us keep in mind when working
on GCC, that can lead to wasted time trying to understand why
an expected optimization isn't working in one test case when it
does work in an apparently equivalent one.

Martin


Re: [libc-coord] Add new ABI '__memcmpeq()' to libc

2021-09-17 Thread Noah Goldstein via Gcc
On Fri, Sep 17, 2021 at 3:32 AM Richard Biener via Gcc 
wrote:

> On Fri, Sep 17, 2021 at 10:08 AM Florian Weimer 
> wrote:
> >
> > * Richard Biener via Gcc:
> >
> > > On Thu, Sep 16, 2021 at 10:36 PM Joseph Myers 
> wrote:
> > >>
> > >> On Thu, 16 Sep 2021, Chris Kennelly wrote:
> > >>
> > >> > In terms of relying on the feature:  If __memcmpeq is ever exposed
> as an a
> > >> > simple alias for memcmp (since the notes mention that it's a valid
> > >> > implementation), does that open up the possibility of depending on
> the
> > >> > bcmp-like behavior that we were trying to escape?
> > >>
> > >> The proposal is as an ABI only (compilers would generate calls to
> > >> __memcmpeq from boolean uses of memcmp, users wouldn't write calls to
> > >> __memcmpeq directly, __memcmpeq wouldn't be declared in installed libc
> > >> headers).  If such dependence arises, that would suggest a compiler
> bug
> > >> wrongly generating such calls for non-boolean memcmp uses.
> > >
> > > So the compiler would emit a call to __memcmpeq and at the same time
> > > emit a weak alias of __memcmpeq to memcmp so the program links
> > > when the libc version targeted does not provide __memcmpeq?  Or would
> > > glibc through  magically communicate the availability of the
> new ABI
> > > without actually declaring the function?
> >
> > I do not think ELF provides that capability.
>
> I guess a weak forwarder should do the trick at the cost of a jmp.
>

Where would the jmp be and under what conditions? Is there no way to achieve
this with zero overhead?


>
> > We can add a declaration to  to communicate the availability.
> > I think this is how glibc (and other libcs) communicate the availability
> > of non-standard interfaces to GCC.
>
> OK, I guess that's fine.
>
> > > (I'm not sure whether a GCC build-time decision via configure is the
> > > very best idea)
> >
> > If libstdc++ or libgcc_s have a symbol dependency on glibc 2.35 for
> > other (unrelated) reasons, would the build-time dependency be less of a
> > concern?  Because another such dependency exists?
>
> Not sure, I was thinking that we'd need to re-compile GCC when we
> upgrade glibc to make use of the feature.
>
> But then being able to run an executable on a system that does not
> provide the ABI but a compatible one (memcmp) might be a nice
> thing.
>
> Richard.
>
> > Thanks,
> > Florian
> >
>


Re: [FYI] bugzilla cleanup

2021-09-17 Thread Richard Biener via Gcc
On September 17, 2021 6:30:21 PM GMT+02:00, Martin Sebor  
wrote:
>On 9/17/21 8:54 AM, Richard Earnshaw wrote:
>> 
>> 
>> On 16/09/2021 16:44, Martin Sebor via Gcc wrote:
>>> On 9/14/21 2:10 AM, Andrew Pinski via Gcc wrote:
 Hi all,
    I am doing some bugzilla cleanup.  This includes disabling some
 components and some versions for new bugs.
 So far I have disabled versions before GCC 4 because we have not had a
 report from someone for those versions in over 7 years.  I disabled
 some versions which are about developmental branches which are
 inactive too.
 I also disabled the java, libgcj, fastjar, libmudflap, treelang and
 libf2c components.

 I am in the process of moving away from having an inline-asm component
 to an inline-asm keyword instead; this was suggested on IRC and I
 agree.  After the current open bugs have moved away from the
 inline-asm component, I will disable it also.

 If anyone else has any other suggestions that should be done, please
 let me know and I will look into doing it.
>>>
>>> Re: Keywords: I find it useful to differentiate between two kinds of
>>> diagnostic bugs: false positives and false negatives (the latter for
>>> existing warnings that don't trigger when intended, as opposed to
>>> requests to enhance existing warnings or add new ones). I've been
>>> using Personal Tags for this but it might be useful to others as
>>> well.  If you agree and add the corresponding new keywords
>>> (false-positive and false-negative) I'll set them based on my Tags.
>>>
>>> One other suggestion: every once in a while someone asks if
>>> ice-on-invalid-code bugs apply to syntactically well-formed code that
>>> has undefined behavior (I don't believe it does).  It would help to
>>> clarify the Description for this Keyword (and, correspondingly, for
>>> ice-on-valid).  E.g., something like
>>>
>>> ice-on-invalid-code: ICE on code that is not syntactically valid.
>>> ice-on-valid-code: ICE on code that is syntactically valid.
>>>
>> 
>> What about syntactically valid but semantically invalid code?  I'd call 
>> that ICE-on-invalid as well.
>
>My understanding of the "valid" in the keyword is that it refers
>only to syntactic validity AKA well-formedness.

Yes. 

>But I only set the keyword to help with bug triage, I don't actually
>rely on it for anything myself.  The interpretation should be based
>on what its main consumers use it for.  I expect Richard Biener (CC'd)
>might be using it for his release planning activities and so might
>want to weigh in on this.

In fact syntactically valid code should not ICE and that's as important as 
semantically valid code not ICE. 

>Martin
>
>PS In bugs reported for my code in the middle end (often having to
>do with detecting semantically invalid AKA undefined code), I use
>ice-on-valid for well-formed test cases with undefined behavior.
>I set ice-on-invalid only for the less common cases of syntactically
>invalid code that somehow makes it into the middle-end or that's
>analyzed in the front end.  If this isn't right I'd like to know.

That's correct. 

Richard. 



Re: [PING^2] Re: Fix 'hash_table::expand' to destruct stale Value objects

2021-09-17 Thread Jonathan Wakely via Gcc
On Fri, 17 Sep 2021, 16:52 Thomas Schwinge,  wrote:

> Hi!
>
> On 2021-09-17T15:03:18+0200, Richard Biener 
> wrote:
> > On Fri, Sep 17, 2021 at 2:39 PM Jonathan Wakely 
> wrote:
> >> On Fri, 17 Sept 2021 at 13:08, Richard Biener
> >>  wrote:
> >> > On Fri, Sep 17, 2021 at 1:17 PM Thomas Schwinge <
> tho...@codesourcery.com> wrote:
> >> > > On 2021-09-10T10:00:25+0200, I wrote:
> >> > > > On 2021-09-01T19:31:19-0600, Martin Sebor via Gcc-patches <
> gcc-patc...@gcc.gnu.org> wrote:
> >> > > >> On 8/30/21 4:46 AM, Thomas Schwinge wrote:
> >> > > >>> Ping -- we still need to plug the memory leak; see patch
> attached, [...]
>
> >> > > > Ping for formal approval (and review for using proper
> >> > > > C++ terminology in the 'gcc/hash-table.h:hash_table::expand'
> source code
> >> > > > comment that I'm adding).  Patch again attached, for easy
> reference.
>
> >> > I'm happy when a C++ literate approves the main change which I quote
> as
> >> >
> >> >   new ((void*) q) value_type (std::move (x));
> >> > +
> >> > + /* Manually invoke destructor of original object, to
> counterbalance
> >> > +object constructed via placement new.  */
> >> > + x.~value_type ();
> >> >
> >> > but I had the impression that std::move already "moves away" from the
> source?
> >>
> >> It just casts the argument to an rvalue reference, which allows the
> >> value_type constructor to steal its guts.
> >>
> >> > That said, the dance above looks iffy, there must be a nicer way to
> "move"
> >> > an object in C++?
> >>
> >> The code above is doing two things: transfer the resources from x to a
> >> new object at location *q, and then destroy x.
> >>
> >> The first part (moving its resources) has nothing to do with
> >> destruction. An object still needs to be destroyed, even if its guts
> >> have been moved to another object.
> >>
> >> The second part is destroying the object, to end its lifetime. You
> >> wouldn't usually call a destructor explicitly, because it would be
> >> done automatically at the end of scope for objects on the stack, or
> >> done by delete when you free obejcts on the heap. This is a special
> >> case where the object's lifetime is manually managed in storage that
> >> is manually managed.
>
> ACK, and happy that I understood this correctly.
>
> And, thanks for providing some proper C++-esque wording, which I
> summarized to replace my original source code comment.
>
> >> > What happens if the dtor is deleted btw?
> >>
> >> If the destructor is deleted you have created an unusable type that
> >> cannot be stored in containers. It can only be created using new, and
> >> then never destroyed. If you play stupid games, you win stupid prizes.
> >> Don't do that.
>
> Haha!  ;-)
>
> And, by the way, as I understood this: if the destructor is "trivial"
> (which includes POD types, for example), the explicit destructor call
> here is a no-op.
>

Right.

And you can even do x.~value_type(); for things which aren't classes and
don't have any destructor at all, not even a trivial one. So in a template
function, if the template argument T is int or char or long*, it's ok to do
t.~T(). This is called a pseudo-destructor call (because scalar types like
int don't actually have a destructor). This will also be a no-op.

This allows you to write the same template code for any types* and it will
correctly destroy them, whether they have a non-trivial destructor that
does real work, or a trivial one, or if they are not even classes and have
no destructor at all.

* Well, nearly any types... You can't do it if the destructor is deleted,
as Richi asked about, or private, and you can't do it for non-object types
(references, functions, void) but that's ok because you can't store them in
a container anyway.


Re: nvtpx: error: array subscript -1 is below array bounds of 'short int [2][16]'

2021-09-17 Thread Martin Sebor via Gcc

On 9/4/21 1:10 PM, Jan-Benedict Glaw wrote:

Hi!

Running automated tests again, I found that when building current
(2fcfc03459a907c0237ea6e2c6e4ce4871034bed) GCC with a recent GCC, a
build (make all-gcc) when ./configure'ed for -target=nvptx-none
--enable-werror-always --enable-languages=all --disable-gcov
--disable-shared --disable-threads --without-headers fails with:


I was able to reproduce the warning.  The IL that triggers it is
below:

   [local count: 64737]:
  _1504 = MEM[(const struct rtx_def *)dreg_1461].u.reg.regno;
  dregno_1505 = (int) _1504;
  _1506 = (long unsigned int) dregno_1505;
  _1507 = _1506 * 2;
  _1508 = reg_renumber.845_1499 + _1507;
  _1509 = default_target_ira.x_ira_class_hard_regs[dclass.380_1481][0];

dclass.380_1481 is an SSA_NAME in the range int [-INF, -1], so
the warning is working as designed.

dclass' type is enum reg_class which is defined like so:

  emum enum reg_class { NO_REGS, ALL_REGS, LIM_REG_CLASSES };

GCC treats it as signed despite it having no negative enumerators,
and somehow infers its range as negative.  Since (if) none of its
values can be negative as I suspect, defining it as unsigned should
help:

  emum enum reg_class: unsigned { NO_REGS, ALL_REGS, LIM_REG_CLASSES };

And, in fact, it does avoid the warning.  I haven't done any other
testing with this change but I'd expect it to also improve codegen
by letting GCC eliminate what's probably unreachable code (which
the warning might then be issued for).

You didn't say what you're looking for but I would suggest to start
by opening a bug in Bugzilla, attaching the preprocessed translation
unit for lra-constraints.c, and referencing this discussion.  To get
it fixed quickly I'd try testing (and, if successful, submitting)
the enum change above.  Another, more targeted solution, the change
above isn't safe, is to add asserting above the uses of the dclass
and sclass variables as indices that they're not negative, e.g.,
like so:

  if (dclass < 0)
gcc_unreachable ();

Martin



[all 2021-09-04 16:33:59] /usr/lib/gcc-snapshot/bin/g++  -fno-PIE -c   -g -O2 
-DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wno-error=format-diag -Wmissing-format-attribute 
-Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros 
-Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -I. 
-I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include 
-I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libcody  
-I../../gcc/gcc/../libdecnumber -I../../gcc/gcc/../libdecnumber/dpd 
-I../libdecnumber -I../../gcc/gcc/../libbacktrace   -o lra-constraints.o -MT 
lra-constraints.o -MMD -MP -MF ./.deps/lra-constraints.TPo 
../../gcc/gcc/lra-constraints.c
[all 2021-09-04 16:34:07] In function 'bool check_and_process_move(bool*, 
bool*)',
[all 2021-09-04 16:34:07] inlined from 'bool curr_insn_transform(bool)' at 
../../gcc/gcc/lra-constraints.c:4092:33:
[all 2021-09-04 16:34:07] ../../gcc/gcc/lra-constraints.c:1317:56: error: array 
subscript -1 is below array bounds of 'short int [2][16]' [-Werror=array-bounds]
[all 2021-09-04 16:34:07]  1317 |   reg_renumber[dregno] = 
ira_class_hard_regs[dclass][0];
[all 2021-09-04 16:34:07] In file included from 
../../gcc/gcc/lra-constraints.c:123:
[all 2021-09-04 16:34:07] ../../gcc/gcc/ira.h: In function 'bool 
curr_insn_transform(bool)':
[all 2021-09-04 16:34:07] ../../gcc/gcc/ira.h:85:9: note: while referencing 
'target_ira::x_ira_class_hard_regs'
[all 2021-09-04 16:34:07]85 |   short 
x_ira_class_hard_regs[N_REG_CLASSES][FIRST_PSEUDO_REGISTER];
[all 2021-09-04 16:34:07]   | ^
[all 2021-09-04 16:34:07] In function 'bool check_and_process_move(bool*, 
bool*)',
[all 2021-09-04 16:34:07] inlined from 'bool curr_insn_transform(bool)' at 
../../gcc/gcc/lra-constraints.c:4092:33:
[all 2021-09-04 16:34:07] ../../gcc/gcc/lra-constraints.c:1324:56: error: array 
subscript -1 is below array bounds of 'short int [2][16]' [-Werror=array-bounds]
[all 2021-09-04 16:34:07]  1324 |   reg_renumber[sregno] = 
ira_class_hard_regs[sclass][0];
[all 2021-09-04 16:34:07] In file included from 
../../gcc/gcc/lra-constraints.c:123:
[all 2021-09-04 16:34:07] ../../gcc/gcc/ira.h: In function 'bool 
curr_insn_transform(bool)':
[all 2021-09-04 16:34:07] ../../gcc/gcc/ira.h:85:9: note: while referencing 
'target_ira::x_ira_class_hard_regs'
[all 2021-09-04 16:34:07]85 |   short 
x_ira_class_hard_regs[N_REG_CLASSES][FIRST_PSEUDO_REGISTER];
[all 2021-09-04 16:34:07]   | ^
[all 2021-09-04 16:34:13] cc1plus: all warnings being treated as errors
[all 2021-09-04 16:34:13] make[1]: *** [Makefile:1142: lra-constraints.o] Error 
1
[all 2021-09-04 16:34:13] make[1]: Leaving directory 
'/var/lib/laminar/run/gcc-nvptx-none/5/toolchain-build/gcc'
[all 2021-09-04 16:34:13] make: *** [Makefile:4407: all-gcc] Error 2

Thanks,
   

gcc-10-20210917 is now available

2021-09-17 Thread GCC Administrator via Gcc
Snapshot gcc-10-20210917 is now available on
  https://gcc.gnu.org/pub/gcc/snapshots/10-20210917/
and on various mirrors, see http://gcc.gnu.org/mirrors.html for details.

This snapshot has been generated from the GCC 10 git branch
with the following options: git://gcc.gnu.org/git/gcc.git branch 
releases/gcc-10 revision 0c1af84f181237b8a90ae391152dbdcb9c6ab064

You'll find:

 gcc-10-20210917.tar.xz   Complete GCC

  SHA256=b5178e4d3395ec57830a37ab64c75d1b6f37e6854b8759cb873dd303414c8861
  SHA1=9ff1038d905a188bb0d9da419b22e180e56a33fc

Diffs from 10-20210910 are available in the diffs/ subdirectory.

When a particular snapshot is ready for public consumption the LATEST-10
link is updated and a message is sent to the gcc list.  Please do not use
a snapshot before it has been announced that way.


Re: [libc-coord] Add new ABI '__memcmpeq()' to libc

2021-09-17 Thread James Y Knight via Gcc
On Thu, Sep 16, 2021 at 5:50 PM enh  wrote:

> plus testing for _equality_ can (as mentioned earlier) have slightly
> different properties from the three-way comparator behavior of
> bcmp()/memcmp().
>

Per spec, bcmp is not a three-way comparison, it is an equality comparison
with exactly the same semantics as proposed for __memcmpeq.

Glibc currently implements bcmp as an alias to memcmp -- which is valid,
but provides more than just the boolean equality semantics. There was
concern raised that modifying that might break existing binaries. However,
this concern can be easily addressed in the same way glibc typically
addresses such compatibility concerns: creating a new symbol-version for
the new bcmp implementation, with the previous bcmp symbol-version
remaining as a memcmp alias.



> On Thu, Sep 16, 2021 at 2:43 PM Joseph Myers 
> wrote:
>
>> On Thu, 16 Sep 2021, James Y Knight wrote:
>>
>> > Wouldn't it be far simpler to just un-deprecate bcmp?
>>
>> The aim is to have something to which calls can be generated in all
>> standards modes.  bcmp has never been part of ISO C; there's nothing to
>> undeprecate there.
>
>
OK, I can see that. I suppose that strict ISO C compatibility could be
important to someone. :)