[RFC] arm: atomics: ARMv7 doubleword atomicity

[mudrievskyjpetro via Gcc]

Dear maintainers of GCC arm port,

Can you share your thoughts on the email I've sent to the mailing list?
I've originally sent it to Will Deacon, gcc and linux mailing lists, but
no one is responding, so I'm pinging you directly.

https://gcc.gnu.org/pipermail/gcc/2023-April/241093.html

---
Peter

gcc-10-20230406 is now available

[GCC Administrator via Gcc]

Snapshot gcc-10-20230406 is now available on
  https://gcc.gnu.org/pub/gcc/snapshots/10-20230406/
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 a23627d8af71214e27798a2083a337b98b284f3e

You'll find:

 gcc-10-20230406.tar.xz   Complete GCC

  SHA256=c31397f723e0638ef2467f0e945c00cc58c6deb4524a67a56196f9185942d994
  SHA1=cd9f72a518bd5b3329a1eb9cc8b2d60daf8a56d8

Diffs from 10-20230330 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: -Wanalyzer-use-of-uninitialized-value always shadows -Wanalyzer-out-of-bounds

[David Malcolm via Gcc]

On Thu, 2023-04-06 at 13:02 +0200, Benjamin Priour wrote:
> Hi David,
> I haven't yet looked into your suggestions, probably won't have time
> until
> tomorrow actually :/
> Still, here are some updates
> 
> On Thu, Apr 6, 2023 at 2:32 AM David Malcolm 
> wrote:
> 
> > On Wed, 2023-04-05 at 19:50 +0200, Benjamin Priour wrote:
> > > Hi David,
> 

[...snip...]

> > There's quite a bit of duplication here.  My recollection is that
> > there's code in the analyzer that's meant to be eliminating some of
> > this e.g. we want to show the OOB when consecutive_oob_in_frame is
> > called directly; we *don't* want to show it when
> > consecutive_oob_in_frame is called by goo.  Perhaps this
> > deduplication
> > code isn't working?  Can you reproduce similar behavior with C, or
> > is
> > it specific to C++?
> > 
> > 
> Identical behavior both in C and C++. I will look at this code, any
> hint at
> where it starts ?
> Otherwise I would find it the good old way.

It's in diagnostic-manager.cc:
diagnostic_manager::emit_saved_diagnostics uses class dedupe_winners to
try to deduplicate the various saved_diagnostic instances, using
dedupe_key::operator== (and also its hash function).  One of the things
dedupe_key::operator== uses is saved_diagnostic::operator==, which does
the bulk of the work to decide "are these two really the 'same'
diagnotic?"


> > > 
> > > First, as the subject line reads, I get a
> > > -Wanalyzer-use-of-uninitialized-value for each -Wanalyzer-out-of-
> > > bounds. I
> > > feel it might be too much, as fixing the OOB would fix the
> > > former.
> > > So maybe only OOB could result in a warning here ?
> > 
> > Yes, that's a good point.  Can you file a bug about this in
> > bugzilla
> > please?  (and feel free to assign it to yourself if you want to
> > have a
> > go at fixing it)
> > 
> 
> Unfortunately the Assignee field is grayed out for me in both
> enter_bug.cgi
> and show_bug.cgi.
> I've also created a new tracker bug for out-of-bounds, as there is a
> number
> of related bugs.

I think you get more access rights in our bugzilla if you log in with a
gcc.gnu.org email address.  You can request one here:
  https://sourceware.org/cgi-bin/pdw/ps_form.cgi
and cite me as approving the request.


> > 
> > Maybe we could fix this by having region_model::check_region_bounds
> > return a bool that signifies if the access is valid, and
> > propagating
> > that value up through callers so that we can return a non-
> > poisoned_svalue at the point where we'd normally return an
> > "uninitialized" poisoned_svalue.
> > 
> > Alternatively, we could simply terminate any analysis path in which
> > an
> > OOB access is detected (by implementing the
> > pending_diagnostic::terminate_path_p virtual function for class
> > out_of_bounds).
> > 
> 
> I'm adding your suggestions as comment to the filed bugs so as to not
> forget them.

Thanks.


> > > Second, it seems that if a frame was a cause for a OOB (either by
> > > containing the spurious code or by being a caller to such code),
> > > it
> > > will
> > > only emit one set of warning, rather than at each unique
> > > compromising
> > > statements.
> > 
> > Maybe.  There's a pending_diagnostic::supercedes_p virtual function
> > that perhaps we could implement for out_of_bounds (or its
> > subclasses).
> > 
> > 
> 
> > > 
> > > Finally, I think the diagnostic path should only go at deep as
> > > the
> > > declaration of the injurious index.
> > 
> > I'm not quite sure what you mean by this, sorry.
> > 
> > 
> Indeed not the best explanation so far. I was actually sort of
> suggesting
> to only emit OOB only on direct call sites,
> you did too, so in a way you have answered me on this.
> 
> Just an addition though: if there is an OOB independent of its
> enclosing
> function's parameters, I think
> it might make sense to not emit for this particular OOB outside the
> function definition itself.
> Meaning that no OOB should be emitted on call sites to this function
> for
> this particular array access.
> (Typically, consecutive_oob_in_frame () shouldn't have resulted in
> more
> than one warning, since the OOB within is independent of its
> parameters).

Yes; I think we're in agreement here.

> 
> I believe right now the expected behavior is to issue warnings only
> on
> actual function calls, so that a function never called
> won't result in warnings. 

IIRC that was the case in GCC 10, but I changed it in
8a2750086d57d1a2251d9239fa4e6c2dc9ec3a86.

> As a result, the initial analysis of each
> functions should never results in warnings -actually the case for
> malloc-leak,
> not for OOB though-.
> Thus we would need to tweak this into actually diagnosing the issues
> on
> initial analysis -those that can be at least-, so that they are saved
> for a
> later
> use whenever the function is actually called. Then we would emit them
> once,
> and only once, because by nature these diagnostics are parameters
> independent.
> I hope I made it clearer, not more convoluted.
> 

Re: [PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[Alejandro Colomar via Gcc]

Hi Eric,

On 4/6/23 18:24, Eric Blake wrote:
> On Wed, Apr 05, 2023 at 02:42:04AM +0200, Alejandro Colomar wrote:
>> Hi Eric,
>>
>> I'm going to reply both your emails here so that GCC is CCed, and they can
>> suggest better stuff.  I'm worried about sending something to POSIX without
>> enough eyes checking it.  So this will be a long email.
> 
> Because your mail landed in a publicly archived mailing list, the
> POSIX folks saw it anyways ;)

:)

> 
> ...
>>>
>>> Whether gcc already has all the attributes you need is not my area of
>>> expertise.  In my skim of the glibc list conversation, I saw mention
>>> of attribute [[gnu:transparent_union]] rather than [[__may_alias__]] -
>>> if that's a better implementation-defined extension that does what we
>>> need, then use it.  The standard developers were a bit uncomfortable
>>> directly putting [[gnu:transparent_union]] in the standard, but
>>> [[__may_alias__]] was noncontroversial (it's in the namespace reserved
>>> for the implementation)
>>
>> Not really; implementation-defined attributes are required to use an
>> implementation-defined prefix like 'gnu::'.  So [[__may_alias__]] is
>> reserved by ISO C, AFAIR.  Maybe it would be better to just mention
>> attributes without any specific attribute name; being fuzzy about it
>> would help avoid making promises that we can't hold.
> 
> On this point, the group agreed, and we intentionally loosened to
> wording to just mention an implementation-defined extension, rather
> than giving any specific attribute name.
> 
> ...
>>
>> I would just make it more fuzzy about which standard version did what.
>> How about this?:
>>
>> [[
>> Note that defining the sockaddr_storage and sockaddr structures using
>> only mechanisms defined in editions of the ISO C standard may produce
>> aliasing diagnostics.  Because of the large body of existing code
>> utilizing sockets in a way that could trigger undefined behavior due
>> to strict aliasing rules, this standard mandates that the various socket
>> address structures can alias each other for accessing their first member,
> 
> The sa_family_t member is not necessarily the first member on all
> platforms (it happens to be first in Linux, but as a counter-example,
> https://man.freebsd.org/cgi/man.cgi?query=unix&sektion=4 shows
> sun_family as the second one-byte field in struct sockaddr_un).  The
> emphasis is on derefencing the family member (whatever offset it is
> at) to learn what cast to use to then safely access the rest of the
> storage.
> 
> As such, here's the updated wording that the Austin Group tried today
> (and we plan on starting a 30-day interpretation feedback window if
> there are still adjustments to be made to the POSIX wording):
> 
> https://austingroupbugs.net/view.php?id=1641#c6255

Thanks!  That wording (both paragraphs) LGTM.

Cheers,
Alex

-- 

GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH] sockaddr.3type: Document that sockaddr_storage is the API to be used

[Eric Blake via Gcc]

On Wed, Apr 05, 2023 at 02:42:04AM +0200, Alejandro Colomar wrote:
> Hi Eric,
> 
> I'm going to reply both your emails here so that GCC is CCed, and they can
> suggest better stuff.  I'm worried about sending something to POSIX without
> enough eyes checking it.  So this will be a long email.

Because your mail landed in a publicly archived mailing list, the
POSIX folks saw it anyways ;)

...
> > 
> > Whether gcc already has all the attributes you need is not my area of
> > expertise.  In my skim of the glibc list conversation, I saw mention
> > of attribute [[gnu:transparent_union]] rather than [[__may_alias__]] -
> > if that's a better implementation-defined extension that does what we
> > need, then use it.  The standard developers were a bit uncomfortable
> > directly putting [[gnu:transparent_union]] in the standard, but
> > [[__may_alias__]] was noncontroversial (it's in the namespace reserved
> > for the implementation)
> 
> Not really; implementation-defined attributes are required to use an
> implementation-defined prefix like 'gnu::'.  So [[__may_alias__]] is
> reserved by ISO C, AFAIR.  Maybe it would be better to just mention
> attributes without any specific attribute name; being fuzzy about it
> would help avoid making promises that we can't hold.

On this point, the group agreed, and we intentionally loosened to
wording to just mention an implementation-defined extension, rather
than giving any specific attribute name.

...
> 
> I would just make it more fuzzy about which standard version did what.
> How about this?:
> 
> [[
> Note that defining the sockaddr_storage and sockaddr structures using
> only mechanisms defined in editions of the ISO C standard may produce
> aliasing diagnostics.  Because of the large body of existing code
> utilizing sockets in a way that could trigger undefined behavior due
> to strict aliasing rules, this standard mandates that the various socket
> address structures can alias each other for accessing their first member,

The sa_family_t member is not necessarily the first member on all
platforms (it happens to be first in Linux, but as a counter-example,
https://man.freebsd.org/cgi/man.cgi?query=unix&sektion=4 shows
sun_family as the second one-byte field in struct sockaddr_un).  The
emphasis is on derefencing the family member (whatever offset it is
at) to learn what cast to use to then safely access the rest of the
storage.

As such, here's the updated wording that the Austin Group tried today
(and we plan on starting a 30-day interpretation feedback window if
there are still adjustments to be made to the POSIX wording):

https://austingroupbugs.net/view.php?id=1641#c6255

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [RFC PATCH] driver: unfilter default library path [PR 104707]

[Michael Matz via Gcc]

Hello,

On Thu, 6 Apr 2023, Shiqi Zhang wrote:

> Currently, gcc delibrately filters out default library paths "/lib/" and
> "/usr/lib/", causing some linkers like mold fails to find libraries.

If linkers claim to be a compatible replacement for other linkers then 
they certainly should behave in a similar way.  In this case: look into 
/lib and /usr/lib when something isn't found till then.

> This behavior was introduced at least 31 years ago in the initial
> revision of the git repo, personally I think it's obsolete because:
>  1. The less than 20 bytes of saving is negligible compares to the command
> line argument space of most hosts today.

That's not the issue that is solved by ignoring these paths in the driver 
for %D/%I directives.  The issue is (traditionally) that even if the 
startfiles sit in /usr/lib (say), you don't want to add -L/usr/lib to the 
linker command line because the user might have added -L/usr/local/lib 
explicitely into her link command and depending on order of spec file 
entries the -L/usr/lib would be added in front interacting with the 
expectations of where libraries are found.

Hence: never add something in (essentially) random places that is default 
fallback anyway.  (Obviously the above problem could be solved in a 
different, more complicated, way.  But this is the way it was solved since 
about forever).

If mold doesn't look into {,/usr}/lib{,64} (as appropriate) by default 
then that's the problem of mold.


Ciao,
Michael.

Re: -Wanalyzer-use-of-uninitialized-value always shadows -Wanalyzer-out-of-bounds

[Benjamin Priour via Gcc]

Hi David,
I haven't yet looked into your suggestions, probably won't have time until
tomorrow actually :/
Still, here are some updates

On Thu, Apr 6, 2023 at 2:32 AM David Malcolm  wrote:

> On Wed, 2023-04-05 at 19:50 +0200, Benjamin Priour wrote:
> > Hi David,
> >
> > I used the below code snippet to experiment with out-of-bounds (OOB)
> > on
> > trunk. Three things occurred that I believe could see some
> > improvement. See
> > https://godbolt.org/z/57n459cEs for the warnings.
> >
> > int consecutive_oob_in_frame ()
> > {
> > int arr[] = {1,2,3,4,5,6,7};
> > int y1 = arr[9]; // only  this one get warnings (3*2 actually),
> > expect
> > only 1 OOB though
> > int y2 = arr[10]; // expect a warning too, despite fooling with
> > asm
> > int y3 = arr[50]; // expect a warning for that one too (see asm)
> > return (y1+y2+y3);
> > }
> >
> > int goo () {
> > int x = consecutive_oob_in_frame (); // causes another pair of
> > warnings
> > return 2 * x;
> > }
> >
> > int main () {
> > goo (); // causes another pair of warning
> > consecutive_oob_in_frame (); // silent
> > int x [] = {1,2};
> > x[5]; /* silent, probably because another set of OOB warnings
> > has already been issued with this frame being the source */
> > return 0;
> > }
>
>

> There's quite a bit of duplication here.  My recollection is that
> there's code in the analyzer that's meant to be eliminating some of
> this e.g. we want to show the OOB when consecutive_oob_in_frame is
> called directly; we *don't* want to show it when
> consecutive_oob_in_frame is called by goo.  Perhaps this deduplication
> code isn't working?  Can you reproduce similar behavior with C, or is
> it specific to C++?
>
>
Identical behavior both in C and C++. I will look at this code, any hint at
where it starts ?
Otherwise I would find it the good old way.


>
> >
> > First, as the subject line reads, I get a
> > -Wanalyzer-use-of-uninitialized-value for each -Wanalyzer-out-of-
> > bounds. I
> > feel it might be too much, as fixing the OOB would fix the former.
> > So maybe only OOB could result in a warning here ?
>
> Yes, that's a good point.  Can you file a bug about this in bugzilla
> please?  (and feel free to assign it to yourself if you want to have a
> go at fixing it)
>

Unfortunately the Assignee field is grayed out for me in both enter_bug.cgi
and show_bug.cgi.
I've also created a new tracker bug for out-of-bounds, as there is a number
of related bugs.


>
> Maybe we could fix this by having region_model::check_region_bounds
> return a bool that signifies if the access is valid, and propagating
> that value up through callers so that we can return a non-
> poisoned_svalue at the point where we'd normally return an
> "uninitialized" poisoned_svalue.
>
> Alternatively, we could simply terminate any analysis path in which an
> OOB access is detected (by implementing the
> pending_diagnostic::terminate_path_p virtual function for class
> out_of_bounds).
>

I'm adding your suggestions as comment to the filed bugs so as to not
forget them.


>
> >
> > Second, it seems that if a frame was a cause for a OOB (either by
> > containing the spurious code or by being a caller to such code), it
> > will
> > only emit one set of warning, rather than at each unique compromising
> > statements.
>
> Maybe.  There's a pending_diagnostic::supercedes_p virtual function
> that perhaps we could implement for out_of_bounds (or its subclasses).
>
>

> >
> > Finally, I think the diagnostic path should only go at deep as the
> > declaration of the injurious index.
>
> I'm not quite sure what you mean by this, sorry.
>
>
Indeed not the best explanation so far. I was actually sort of suggesting
to only emit OOB only on direct call sites,
you did too, so in a way you have answered me on this.

Just an addition though: if there is an OOB independent of its enclosing
function's parameters, I think
it might make sense to not emit for this particular OOB outside the
function definition itself.
Meaning that no OOB should be emitted on call sites to this function for
this particular array access.
(Typically, consecutive_oob_in_frame () shouldn't have resulted in more
than one warning, since the OOB within is independent of its parameters).

I believe right now the expected behavior is to issue warnings only on
actual function calls, so that a function never called
won't result in warnings. As a result, the initial analysis of each
functions should never results in warnings -actually the case for
malloc-leak,
not for OOB though-.
Thus we would need to tweak this into actually diagnosing the issues on
initial analysis -those that can be at least-, so that they are saved for a
later
use whenever the function is actually called. Then we would emit them once,
and only once, because by nature these diagnostics are parameters
independent.
I hope I made it clearer, not more convoluted.

>
> > Also, have you considered extending the current call summaries