Re: GCC 8.4 Status Report (2020-02-17)
> It has been almost a year since GCC 8.3 has been released and GCC 8.4 > release should have been released already, so we should concentrate on > getting it out soon. Unfortunately we have two P1s, one of them is > waiting for reporter's input, so we might as well just ignore it unless > the input is provided, but the other, C++ FE one, looks something that > should be fixed. If we get rid of the P1s, I'd like to create > 8.4-rc1 on Wednesday, Feb 26th and release 8.4 the week afterwards. > If you have any queued backports, please commit them to 8 branch > (and 9 branch too, we'd like to release 9.3 soon too). Hi Jakub, it just occurred to me that my patch here is a kind of security relevant one: https://gcc.gnu.org/ml/gcc-patches/2020-02/msg01060.html since every time the collect2 process is interrupted via a signal it can delete random files from the hard drive, since the signal handler may be using the path name, and passes it to the unlink function before it is initialized. The patch doe not apply cleanly to gcc-8, but just fixing that issue, without tackling the cleanup at the same time should be feasible, if you like that for this version? Thanks Bernd.
Re: GCC 8.4 Status Report (2020-02-17)
On 2/19/20 10:24 AM, Richard Biener wrote: > On Tue, Feb 18, 2020 at 8:37 PM Bernd Edlinger > wrote: >> >>> It has been almost a year since GCC 8.3 has been released and GCC 8.4 >>> release should have been released already, so we should concentrate on >>> getting it out soon. Unfortunately we have two P1s, one of them is >>> waiting for reporter's input, so we might as well just ignore it unless >>> the input is provided, but the other, C++ FE one, looks something that >>> should be fixed. If we get rid of the P1s, I'd like to create >>> 8.4-rc1 on Wednesday, Feb 26th and release 8.4 the week afterwards. >>> If you have any queued backports, please commit them to 8 branch >>> (and 9 branch too, we'd like to release 9.3 soon too). >> >> Hi Jakub, >> >> it just occurred to me that my patch here is a kind of >> security relevant one: >> >> https://gcc.gnu.org/ml/gcc-patches/2020-02/msg01060.html >> >> since every time the collect2 process is interrupted via a signal >> it can delete random files from the hard drive, since the >> signal handler may be using the path name, and passes it to the unlink >> function before it is initialized. >> >> The patch doe not apply cleanly to gcc-8, but just fixing >> that issue, without tackling the cleanup at the same time should >> be feasible, if you like that for this version? > > Sure, these kind of fixes are always welcome, but please post them > for review. > OK, posted here: https://gcc.gnu.org/ml/gcc-patches/2020-02/msg01104.html Bernd. > Richard. > >> >> Thanks >> Bernd.
Can we please have the old mailing list back
Hi, I do not want to start a flame war. I just am curious what was the reason why the old system cannot be used any more? Would there be a possibility to get the old look-and-feel back? Thanks Bernd.
Re: Can we please have the old mailing list back
On 3/25/20 8:32 AM, Jonathan Wakely wrote: > On Wed, 25 Mar 2020 at 04:48, Bernd Edlinger wrote: >> >> Hi, >> >> I do not want to start a flame war. >> >> I just am curious what was the reason why >> the old system cannot be used any more? > > The software it ran on hasn't been maintained for years. > >> Would there be a possibility to get the old look-and-feel back? > > There are at least two existing threads on this topic. > Sigh, yes, but it needs much more clicks than before to get an overview of the messages, so I did assume that was already discussed, but frankly I not even know which threads those were. Bernd.
Re: Can we please have the old mailing list back
On 3/25/20 8:58 AM, Bernd Edlinger wrote: > On 3/25/20 8:32 AM, Jonathan Wakely wrote: >> On Wed, 25 Mar 2020 at 04:48, Bernd Edlinger wrote: >>> >>> Hi, >>> >>> I do not want to start a flame war. >>> >>> I just am curious what was the reason why >>> the old system cannot be used any more? >> >> The software it ran on hasn't been maintained for years. >> >>> Would there be a possibility to get the old look-and-feel back? >> >> There are at least two existing threads on this topic. >> > > Sigh, yes, but it needs much more clicks than before to get > an overview of the messages, so I did assume that was already > discussed, but frankly I not even know which threads those were. > A different approach would be this: what do we have to do to get our mailing list threads to marc info, and spinics, where all the linux stuff is hosted, that is doing way better than this? I know gcc-patches is at marc.info but gdb-patches is not and gcc probably also not. Bernd.
Re: Can we please have the old mailing list back
-On 3/25/20 7:55 PM, Christopher Faylor wrote: > On Wed, Mar 25, 2020 at 04:23:02PM +0700, Arseny Solokha wrote: >> I believe the canonical place for the "Linux suff" mailing lists these days >> is >> lore.kernel.org, powered by public-inbox[1]. ISTM that software can address >> most >> if not all needs of those involved in GCC development and even has NNTP >> support, >> though I've no idea whether it could be an acceptable solution from the >> overseers' perspective. >> >> [1] https://public-inbox.org/public-inbox.git > > The overseers are trying hard to use only software that can be installed > via the RHEL packaging system so as not to duplicate the mistake that > kept us dependent on poorly supported mail software. Is there a > public-inbox rpm package for RHEL or CentOS? > > FWIW, this particular overseer is is also pretty exhausted from the > effort of moving sourceware to a new system + new software and would not > relish the effort involved in getting all of this moved to new software. > Honestly this is not about blaming you at all. I do not quite understand what is the exact software which was used previously? what is the exact problem that prevents it from being used any longer? Which software is being used now? Why is gcc-patc...@gcc.gnu.org, gcc@gcc.gnu.org. and fort...@gcc.gnu.org even this e-mail thread visible on marc.info: https://marc.info/?l=gcc&m=158512515107459&w=2 but not gdb-patches ? Could you add a link to https://marc.info/?l=gcc-patches https://marc.info/?l=gcc https://marc.info/?l=gcc-fortran note the unsystematic name gcc-fortran, the list is fort...@gcc.gnu.org There is no gcc-help on marc.info There is https://marc.info/?l=gcc but there is no gdb-patches what needs to be done to host those lists on marc.info as well? What needs to be done to host these lists on spinics for instance, or what else exists that can be used to search the messages? Bernd.
Re: Can we please have the old mailing list back
On 3/25/20 9:29 PM, Bernd Edlinger wrote: > Could you add a link to https://marc.info/?l=gcc-patches Why is above link no longer updating this is the last message there: 1. 2020-03-07 [5] [PATCH] c++: Fix ABI issue with alignas on armv7hl [P gcc-patch Jason Merrill is this a push or a pull why is this no longer active? > https://marc.info/?l=gcc also this one: 1. 2020-03-06 [1] gcc-8-20200306 is now available gcc gccadmin but if I look for mails from bernd.edlinger at marc.info I find this thread, and pretty much everything I do except my mails on gdb-patches, how can that be? Do you have any explanation for that Thanks Bernd.
Re: Can we please have the old mailing list back
On 3/26/20 4:16 PM, Christopher Faylor wrote: > On Wed, Mar 25, 2020 at 09:29:03PM +0100, Bernd Edlinger wrote: >> -On 3/25/20 7:55 PM, Christopher Faylor wrote: >>> On Wed, Mar 25, 2020 at 04:23:02PM +0700, Arseny Solokha wrote: >>>> I believe the canonical place for the "Linux suff" mailing lists these >>>> days is >>>> lore.kernel.org, powered by public-inbox[1]. ISTM that software can >>>> address most >>>> if not all needs of those involved in GCC development and even has NNTP >>>> support, >>>> though I've no idea whether it could be an acceptable solution from the >>>> overseers' perspective. >>>> >>>> [1] https://public-inbox.org/public-inbox.git >>> >>> The overseers are trying hard to use only software that can be installed >>> via the RHEL packaging system so as not to duplicate the mistake that >>> kept us dependent on poorly supported mail software. Is there a >>> public-inbox rpm package for RHEL or CentOS? >>> >>> FWIW, this particular overseer is is also pretty exhausted from the >>> effort of moving sourceware to a new system + new software and would not >>> relish the effort involved in getting all of this moved to new software. >>> >> >> Honestly this is not about blaming you at all. >> >> I do not quite understand what is the exact software which >> was used previously? >> >> what is the exact problem that prevents it from being used any longer? >> >> Which software is being used now? >> >> Why is gcc-patc...@gcc.gnu.org, gcc@gcc.gnu.org. and fort...@gcc.gnu.org >> even this e-mail thread visible >> on marc.info: https://marc.info/?l=gcc&m=158512515107459&w=2 >> but not gdb-patches ? >> >> Could you add a link to https://marc.info/?l=gcc-patches >> https://marc.info/?l=gcc >> https://marc.info/?l=gcc-fortran >> note the unsystematic name gcc-fortran, the list is fort...@gcc.gnu.org >> >> There is no gcc-help on marc.info >> There is https://marc.info/?l=gcc >> but there is no gdb-patches >> >> what needs to be done to host those lists on marc.info as well? >> >> What needs to be done to host these lists on spinics for instance, >> or what else exists that can be used to search the messages? > > marc.info is an independent site that is not associated with > sourceware.org. We don't control it. If you have questions about their > site then ask them. > > The mailing list software is all easily discernible by investigating > email headers and via google but someone else answered your questions > later in this thread. > But don't you think that we change something in 6.3 to make them break. like no longer sending updates, or something? Don't you have any idea what changed on our side? I mean what should I tell them they should do to fix that? Thanks Bernd.
Re: Can we please have the old mailing list back
On 3/25/20 10:07 PM, Jakub Jelinek wrote: > On Wed, Mar 25, 2020 at 09:03:15PM +, Jonathan Wakely via Gcc wrote: >> See the link at the bottom of every page in the old archive: >> http://www.mhonarc.org/ >> >>> what is the exact problem that prevents it from being used any longer? >> >> It's not packaged for RHEL 8. > > It is in EPEL8: > https://dl.fedoraproject.org/pub/epel/8/Everything/x86_64/Packages/m/mhonarc-2.6.19-17.el8.noarch.rpm > So, again, what is the reason why you cannot use this package? Thanks Bernd. PS: I would CC you, Christopher Faylor, but your email address is "cgf-use-the-mailinglist-ple...@gnu.org", so whatever I send there would not reach you.
Re: Can we please have the old mailing list back
On 3/25/20 7:55 PM, Christopher Faylor wrote: > > FWIW, this particular overseer is is also pretty exhausted from the > effort of moving sourceware to a new system + new software and would not > relish the effort involved in getting all of this moved to new software. > I am sorry, to hear that. Of course you can take a few days off. I do not think that it is the end of the world, when we solve the mailing list problems in a week or two for instance. Thanks Bernd. > cgf >
Re: Can we please have the old mailing list back
On 3/26/20 4:27 PM, Bernd Edlinger wrote: > On 3/26/20 4:16 PM, Christopher Faylor wrote: >> On Wed, Mar 25, 2020 at 09:29:03PM +0100, Bernd Edlinger wrote: >>> -On 3/25/20 7:55 PM, Christopher Faylor wrote: >>>> On Wed, Mar 25, 2020 at 04:23:02PM +0700, Arseny Solokha wrote: >>>>> I believe the canonical place for the "Linux suff" mailing lists these >>>>> days is >>>>> lore.kernel.org, powered by public-inbox[1]. ISTM that software can >>>>> address most >>>>> if not all needs of those involved in GCC development and even has NNTP >>>>> support, >>>>> though I've no idea whether it could be an acceptable solution from the >>>>> overseers' perspective. >>>>> >>>>> [1] https://public-inbox.org/public-inbox.git >>>> >>>> The overseers are trying hard to use only software that can be installed >>>> via the RHEL packaging system so as not to duplicate the mistake that >>>> kept us dependent on poorly supported mail software. Is there a >>>> public-inbox rpm package for RHEL or CentOS? >>>> >>>> FWIW, this particular overseer is is also pretty exhausted from the >>>> effort of moving sourceware to a new system + new software and would not >>>> relish the effort involved in getting all of this moved to new software. >>>> >>> >>> Honestly this is not about blaming you at all. >>> >>> I do not quite understand what is the exact software which >>> was used previously? >>> >>> what is the exact problem that prevents it from being used any longer? >>> >>> Which software is being used now? >>> >>> Why is gcc-patc...@gcc.gnu.org, gcc@gcc.gnu.org. and fort...@gcc.gnu.org >>> even this e-mail thread visible >>> on marc.info: https://marc.info/?l=gcc&m=158512515107459&w=2 >>> but not gdb-patches ? >>> >>> Could you add a link to https://marc.info/?l=gcc-patches >>> https://marc.info/?l=gcc >>> https://marc.info/?l=gcc-fortran >>> note the unsystematic name gcc-fortran, the list is fort...@gcc.gnu.org >>> >>> There is no gcc-help on marc.info >>> There is https://marc.info/?l=gcc >>> but there is no gdb-patches >>> >>> what needs to be done to host those lists on marc.info as well? >>> >>> What needs to be done to host these lists on spinics for instance, >>> or what else exists that can be used to search the messages? >> >> marc.info is an independent site that is not associated with >> sourceware.org. We don't control it. If you have questions about their >> site then ask them. >> >> The mailing list software is all easily discernible by investigating >> email headers and via google but someone else answered your questions >> later in this thread. >> > > But don't you think that we change something in 6.3 to make them break. > like no longer sending updates, or something? > > Don't you have any idea what changed on our side? > > I mean what should I tell them they should do to fix that? > > Ah, marc.info is fixed, it turned out that the messages were just Quarantined because due to the change in the ip adresses, mailing software etc. marc.info was under the impression that all these messages were just spam. That is what they told me: "For lists that often get spammed, we set up some silent header-checks so that mails that don't look like they came from the real listserver get quarrantined, and don't appear when viewing that list. Well, that can break when mailing list software changes - like when they switched away from ezmlm to Mailman. I've updated our filter check and un-quarrantined about 4500 mails to various gcc- lists that landed there this month." So indeed all our mailing list message are again on marc.info, I think when it can handle lkml it can handle gcc-patches as well. Many Thanks go to Hank Leininger who does a gread job with marc.info. Bernd.
Re: Can we please have the old mailing list back
On 4/1/20 8:51 AM, Bernd Edlinger wrote: > On 3/26/20 4:27 PM, Bernd Edlinger wrote: >> On 3/26/20 4:16 PM, Christopher Faylor wrote: >>> >>> marc.info is an independent site that is not associated with >>> sourceware.org. We don't control it. If you have questions about their >>> site then ask them. >>> >>> The mailing list software is all easily discernible by investigating >>> email headers and via google but someone else answered your questions >>> later in this thread. >>> >> >> But don't you think that we change something in 6.3 to make them break. >> like no longer sending updates, or something? >> >> Don't you have any idea what changed on our side? >> >> I mean what should I tell them they should do to fix that? >> >> > > Ah, marc.info is fixed, it turned out that the messages were just Quarantined > because due to the change in the ip adresses, mailing software etc. > marc.info was under the impression that all these messages were just spam. > > That is what they told me: > > "For lists that often get spammed, we set up some silent header-checks > so that mails that don't look like they came from the real listserver > get quarrantined, and don't appear when viewing that list. > > Well, that can break when mailing list software changes - like when they > switched away from ezmlm to Mailman. > > I've updated our filter check and un-quarrantined about 4500 mails to > various gcc- lists that landed there this month." > > So indeed all our mailing list message are again on marc.info, > I think when it can handle lkml it can handle gcc-patches as well. > > Many Thanks go to Hank Leininger who does a gread job with marc.info. > > > Bernd. > PS: I have a discovered a very serious problem with the mailing lists that must be fixed by our overseers. That is the scubbed attachments. As an example please look at this one: https://marc.info/?l=gdb-patches&m=158571308379946&w=2 you see this: -- next part -- A non-text attachment was scrubbed... Name: 0001-Fix-range-end-handling-of-inlined-subroutines.patch Type: text/x-patch Size: 10992 bytes Desc: not available URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20200313/5158bb87/attachment.bin> So there are two serious problems here: 1. there is a single point of failure, if sourceware.org goes down the attachment is lost. 2. since the url is http: a man in the middle can impersonate sourceware.org and give you a virus instead of my patch file. It does not help that sourceware.org redirects the download to https://sourceware.org/pipermail/gdb-patches/attachments/20200313/5158bb87/attachment.bin an attacker will not be so polite to do that. @overseeers: PLEASE STOP IMMEDIATELY THAT SCRUBBING can you act now, or do you need a CVE number first ? Thanks Bernd.
Re: Can we please have the old mailing list back
On 4/2/20 12:54 AM, Joseph Myers wrote: > On Wed, 1 Apr 2020, Bernd Edlinger wrote: > >> PS: I have a discovered a very serious problem with the mailing lists >> that must be fixed by our overseers. >> >> That is the scubbed attachments. >> >> As an example please look at this one: >> https://marc.info/?l=gdb-patches&m=158571308379946&w=2 > > The copy of that message I received from gdb-patches by email appears to > have the attachment intact. I don't know where marc.info got the message > from, but it doesn't seem to be by email from the list. > I don't know for sure, but I suppose one of the subscribers will be Hank Leininger from marc.info. Can you check the raw message format from you inbox, so you mail software is not playing tricks with you? Thanks Bernd.
Re: Can we please have the old mailing list back
On 4/2/20 1:41 AM, Jonathan Wakely wrote: > On Wed, 1 Apr 2020 at 23:54, Joseph Myers wrote: >> >> On Wed, 1 Apr 2020, Bernd Edlinger wrote: >> >>> PS: I have a discovered a very serious problem with the mailing lists >>> that must be fixed by our overseers. >>> >>> That is the scubbed attachments. >>> >>> As an example please look at this one: >>> https://marc.info/?l=gdb-patches&m=158571308379946&w=2 >> >> The copy of that message I received from gdb-patches by email appears to >> have the attachment intact. I don't know where marc.info got the message >> from, but it doesn't seem to be by email from the list. > > The sourceware.org archive also has an HTTP (not HTTPS) link for that > attachment: > > https://sourceware.org/pipermail/gdb-patches/2020-March/166601.html > Yes, exactly. But when I serach "bernd.edlinger" in marc.info I see *every* message I sent to the gcc-patches since 2013 and each of them has an attachment, and it is available per https from marc.info, not sourceware.org, that is insane to assume that marc.info would store attachments on sourceware.org, the only logical explanation is that sourceware.org changed the e-mail and removed the attachment and forwarded a different version of the e-mail to marc.info. Regarding the overseers, they repeatedly spoke up on this list, but all the time they use an e-mail that bounces. I'd call that impolite. If you know how to reach them, please make them aware of this issue, because it is a security relevant issue. Seriously. Bernd.
Re: Can we please have the old mailing list back
On 4/2/20 7:13 AM, Christopher Faylor wrote: > On Wed, Apr 01, 2020 at 09:57:05PM -0700, Unidef Defshrizzle wrote: >> We can setup a command line interface, maybe using CURSES, I mean GUIs are >> fun, but Jesus Christ they’re full of surprises > > Command line interface to what? > > You can read email using whatever interface your want. Archives are > obviously going to be web-formatted. > What happens to the e-mails when they are not archive, but forwarded to the subscribers, like mark.info who just subscribes the mails, and archived them they have a lot of hard disks for that and can handle attachments quite well. The point is previously the attachment were stored on marc.info, but now they are no longer reaching mark.info we mangle the e-mails that we do not archive. Which software is responsible for that? ezlm or what? Bernd.
Re: Can we please have the old mailing list back
On 4/2/20 11:01 AM, Richard Sandiford wrote: > Bernd Edlinger writes: >> On 4/1/20 8:51 AM, Bernd Edlinger wrote: >>> On 3/26/20 4:27 PM, Bernd Edlinger wrote: >>>> On 3/26/20 4:16 PM, Christopher Faylor wrote: >>>>> >>>>> marc.info is an independent site that is not associated with >>>>> sourceware.org. We don't control it. If you have questions about their >>>>> site then ask them. >>>>> >>>>> The mailing list software is all easily discernible by investigating >>>>> email headers and via google but someone else answered your questions >>>>> later in this thread. >>>>> >>>> >>>> But don't you think that we change something in 6.3 to make them break. >>>> like no longer sending updates, or something? >>>> >>>> Don't you have any idea what changed on our side? >>>> >>>> I mean what should I tell them they should do to fix that? >>>> >>>> >>> >>> Ah, marc.info is fixed, it turned out that the messages were just >>> Quarantined >>> because due to the change in the ip adresses, mailing software etc. >>> marc.info was under the impression that all these messages were just spam. >>> >>> That is what they told me: >>> >>> "For lists that often get spammed, we set up some silent header-checks >>> so that mails that don't look like they came from the real listserver >>> get quarrantined, and don't appear when viewing that list. >>> >>> Well, that can break when mailing list software changes - like when they >>> switched away from ezmlm to Mailman. >>> >>> I've updated our filter check and un-quarrantined about 4500 mails to >>> various gcc- lists that landed there this month." >>> >>> So indeed all our mailing list message are again on marc.info, >>> I think when it can handle lkml it can handle gcc-patches as well. >>> >>> Many Thanks go to Hank Leininger who does a gread job with marc.info. >>> >>> >>> Bernd. >>> >> >> PS: I have a discovered a very serious problem with the mailing lists >> that must be fixed by our overseers. >> >> That is the scubbed attachments. >> >> As an example please look at this one: >> https://marc.info/?l=gdb-patches&m=158571308379946&w=2 >> >> >> you see this: >> >> -- next part -- >> A non-text attachment was scrubbed... >> Name: 0001-Fix-range-end-handling-of-inlined-subroutines.patch >> Type: text/x-patch >> Size: 10992 bytes >> Desc: not available >> URL: >> <http://sourceware.org/pipermail/gdb-patches/attachments/20200313/5158bb87/attachment.bin> >> >> So there are two serious problems here: >> >> 1. there is a single point of failure, if sourceware.org goes down the >> attachment is lost. >> >> 2. since the url is http: a man in the middle can impersonate sourceware.org >> and give you a >> virus instead of my patch file. >> It does not help that sourceware.org redirects the download to >> https://sourceware.org/pipermail/gdb-patches/attachments/20200313/5158bb87/attachment.bin >> an attacker will not be so polite to do that. >> >> >> @overseeers: PLEASE STOP IMMEDIATELY THAT SCRUBBING >> >> can you act now, or do you need a CVE number first ? > > The overseers are reachable on: > > https://sourceware.org/mailman/listinfo/overseers > > Please keep the tone civil. I hope we never see the day where the GCC/ > sourceware lists have to have a code of conduct, but if we did, I think > some of the messages on this thread would have breached it. > > Thanks, > Richard > Thanks, for reminding me. I do personally full-heatedly apologize, and regret what I said above. I am sorry if I made you feel bad. That was not the true intention of what I said. I asked Hank Leininger for clarification how mark.info subscribes the mails, and what data he gets exactly from us. I am still waiting for his response, and let you know what he says. In the meantime, culd you please change http: to https: Thanks Bernd.
Re: Can we please have the old mailing list back
On 4/2/20 11:09 AM, Jonathan Wakely wrote: > On Thu, 2 Apr 2020 at 04:35, Bernd Edlinger wrote: >> Regarding the overseers, they repeatedly spoke up on this list, >> but all the time they use an e-mail that bounces. > > No. ONE of the overseers replies from a personal email address that > bounces, because as the address clearly says, you should contact him > via the list, not his personal address. And the relevant list is the > overseers one, not this one. > >> I'd call that impolite. > > No, it's impolite to ignore his request and try to reach him > personally, rather than via the list. It's perfectly reasonable to say > "please use the mailing list instead of emailing me personally". I do > that all the time when people reply to me off-list after looking for > help on gcc-help. > Ah, okay, I did not look at it this way. There are a few people that send me mails where I do not even do that. I chose to ignore them But that excludes everybody on this list of course :-) Bernd.
Re: Can we please have the old mailing list back
On 4/2/20 11:48 AM, Richard Sandiford wrote: > Bernd Edlinger writes: >> On 4/2/20 11:01 AM, Richard Sandiford wrote: >>> Bernd Edlinger writes: >>>> On 4/1/20 8:51 AM, Bernd Edlinger wrote: >>>>> On 3/26/20 4:27 PM, Bernd Edlinger wrote: >>>>>> On 3/26/20 4:16 PM, Christopher Faylor wrote: >>>>>>> >>>>>>> marc.info is an independent site that is not associated with >>>>>>> sourceware.org. We don't control it. If you have questions about their >>>>>>> site then ask them. >>>>>>> >>>>>>> The mailing list software is all easily discernible by investigating >>>>>>> email headers and via google but someone else answered your questions >>>>>>> later in this thread. >>>>>>> >>>>>> >>>>>> But don't you think that we change something in 6.3 to make them break. >>>>>> like no longer sending updates, or something? >>>>>> >>>>>> Don't you have any idea what changed on our side? >>>>>> >>>>>> I mean what should I tell them they should do to fix that? >>>>>> >>>>>> >>>>> >>>>> Ah, marc.info is fixed, it turned out that the messages were just >>>>> Quarantined >>>>> because due to the change in the ip adresses, mailing software etc. >>>>> marc.info was under the impression that all these messages were just spam. >>>>> >>>>> That is what they told me: >>>>> >>>>> "For lists that often get spammed, we set up some silent header-checks >>>>> so that mails that don't look like they came from the real listserver >>>>> get quarrantined, and don't appear when viewing that list. >>>>> >>>>> Well, that can break when mailing list software changes - like when they >>>>> switched away from ezmlm to Mailman. >>>>> >>>>> I've updated our filter check and un-quarrantined about 4500 mails to >>>>> various gcc- lists that landed there this month." >>>>> >>>>> So indeed all our mailing list message are again on marc.info, >>>>> I think when it can handle lkml it can handle gcc-patches as well. >>>>> >>>>> Many Thanks go to Hank Leininger who does a gread job with marc.info. >>>>> >>>>> >>>>> Bernd. >>>>> >>>> >>>> PS: I have a discovered a very serious problem with the mailing lists >>>> that must be fixed by our overseers. >>>> >>>> That is the scubbed attachments. >>>> >>>> As an example please look at this one: >>>> https://marc.info/?l=gdb-patches&m=158571308379946&w=2 >>>> >>>> >>>> you see this: >>>> >>>> -- next part -- >>>> A non-text attachment was scrubbed... >>>> Name: 0001-Fix-range-end-handling-of-inlined-subroutines.patch >>>> Type: text/x-patch >>>> Size: 10992 bytes >>>> Desc: not available >>>> URL: >>>> <http://sourceware.org/pipermail/gdb-patches/attachments/20200313/5158bb87/attachment.bin> >>>> >>>> So there are two serious problems here: >>>> >>>> 1. there is a single point of failure, if sourceware.org goes down the >>>> attachment is lost. >>>> >>>> 2. since the url is http: a man in the middle can impersonate >>>> sourceware.org and give you a >>>> virus instead of my patch file. >>>> It does not help that sourceware.org redirects the download to >>>> https://sourceware.org/pipermail/gdb-patches/attachments/20200313/5158bb87/attachment.bin >>>> an attacker will not be so polite to do that. >>>> >>>> >>>> @overseeers: PLEASE STOP IMMEDIATELY THAT SCRUBBING >>>> >>>> can you act now, or do you need a CVE number first ? >>> >>> The overseers are reachable on: >>> >>> https://sourceware.org/mailman/listinfo/overseers >>> >>> Please keep the tone civil. I hope we never see the day where the GCC/ >>> sourceware lists have to have a code of conduct, but if we did, I think >>> some of the messages on this thread would have breached
Question about the testresults mailing list
Hi, I have a question about the gcc-testresults mailing list, that is I see everyone using: [releases/gcc-9 revision 02a201f71:0f58d3b54:0e66150084aa217811a5c45fb15e98d7ed3e8839] or [master revision 63b2923dc6f:0c89e976db9:1c16f7fc903c1c1c912faf7889b69d83429b7b2e what is the first 2 hashes, I cant make sens out of it? Thanks Bernd.
Re: Question about the testresults mailing list
On 4/3/20 7:50 PM, Jonathan Wakely wrote: > On Fri, 3 Apr 2020 at 18:45, Bernd Edlinger wrote: >> >> Hi, >> >> I have a question about the gcc-testresults mailing list, >> that is I see everyone using: >> >> [releases/gcc-9 revision >> 02a201f71:0f58d3b54:0e66150084aa217811a5c45fb15e98d7ed3e8839] >> >> or >> >> [master revision >> 63b2923dc6f:0c89e976db9:1c16f7fc903c1c1c912faf7889b69d83429b7b2e >> >> what is the first 2 hashes, I cant make sens out of it? > > See contrib/gcc_update, which sets that string: > > revision=`$GCC_GIT log -n1 --pretty=tformat:%p:%t:%H` > ah, you mean my workflow was wrong, git pull configure make test gcc_testresults? what is your workflow? Thanks Bernd.
Re: Question about the testresults mailing list
On 4/3/20 7:50 PM, Jonathan Wakely wrote: > On Fri, 3 Apr 2020 at 18:45, Bernd Edlinger wrote: >> >> Hi, >> >> I have a question about the gcc-testresults mailing list, >> that is I see everyone using: >> >> [releases/gcc-9 revision >> 02a201f71:0f58d3b54:0e66150084aa217811a5c45fb15e98d7ed3e8839] >> >> or >> >> [master revision >> 63b2923dc6f:0c89e976db9:1c16f7fc903c1c1c912faf7889b69d83429b7b2e >> >> what is the first 2 hashes, I cant make sens out of it? > > See contrib/gcc_update, which sets that string: > > revision=`$GCC_GIT log -n1 --pretty=tformat:%p:%t:%H` > and for dummies like me, what is %p %t and %H ?
Re: Question about the testresults mailing list
On 4/3/20 7:57 PM, Jonathan Wakely wrote: > On Fri, 3 Apr 2020 at 18:54, Bernd Edlinger wrote: >> >> On 4/3/20 7:50 PM, Jonathan Wakely wrote: >>> On Fri, 3 Apr 2020 at 18:45, Bernd Edlinger wrote: >>>> >>>> Hi, >>>> >>>> I have a question about the gcc-testresults mailing list, >>>> that is I see everyone using: >>>> >>>> [releases/gcc-9 revision >>>> 02a201f71:0f58d3b54:0e66150084aa217811a5c45fb15e98d7ed3e8839] >>>> >>>> or >>>> >>>> [master revision >>>> 63b2923dc6f:0c89e976db9:1c16f7fc903c1c1c912faf7889b69d83429b7b2e >>>> >>>> what is the first 2 hashes, I cant make sens out of it? >>> >>> See contrib/gcc_update, which sets that string: >>> >>> revision=`$GCC_GIT log -n1 --pretty=tformat:%p:%t:%H` >>> >> >> ah, you mean my workflow was wrong, >> git pull >> configure >> make >> test >> gcc_testresults? >> >> what is your workflow? > > Using contrib/gcc_update isn't required. > I wonder if I should return to using the snapshot files. I am currently the only one testing armv7-gnueabihf modulo I'm using cortex-a9 I use a very slow custom device, it uses 5 days for bootstrap®testing. But it works, and it is stress-testing the linux as well as producing test results. So in order to make the test results comparable I publish with others, would it be good have different archs use the same revision, by default, which would be the one picked once a week for the snapshots? or does anyone test the snapshots and publish the results for differnt archs using the snapshot instead of arbitrary versions ? Of course using git it is rather simple to update to any version. What do you think? Bernd.
Re: Question about the testresults mailing list
On 4/3/20 7:56 PM, Jonathan Wakely wrote: > On Fri, 3 Apr 2020 at 18:55, Bernd Edlinger wrote: >> >> >> >> On 4/3/20 7:50 PM, Jonathan Wakely wrote: >>> On Fri, 3 Apr 2020 at 18:45, Bernd Edlinger wrote: >>>> >>>> Hi, >>>> >>>> I have a question about the gcc-testresults mailing list, >>>> that is I see everyone using: >>>> >>>> [releases/gcc-9 revision >>>> 02a201f71:0f58d3b54:0e66150084aa217811a5c45fb15e98d7ed3e8839] >>>> >>>> or >>>> >>>> [master revision >>>> 63b2923dc6f:0c89e976db9:1c16f7fc903c1c1c912faf7889b69d83429b7b2e >>>> >>>> what is the first 2 hashes, I cant make sens out of it? >>> >>> See contrib/gcc_update, which sets that string: >>> >>> revision=`$GCC_GIT log -n1 --pretty=tformat:%p:%t:%H` >>> >> >> and for dummies like me, >> >> what is %p %t and %H ? > > git help log > done. But why do we use parent hash and tree hash, that does just look confusing? Bernd.
Re: Can we please have the old mailing list back
On 4/2/20 10:06 AM, Andreas Schwab wrote: > On Apr 02 2020, Bernd Edlinger wrote: > >> What happens to the e-mails when they are not archive, but forwarded >> to the subscribers, like mark.info who just subscribes the mails, >> and archived them they have a lot of hard disks for that and can >> handle attachments quite well. The point is previously the attachment >> were stored on marc.info, but now they are no longer reaching mark.info >> we mangle the e-mails that we do not archive. > > You should reach out to the maintainers of mark.info and ask them how > they receive the messages. On gmane.io, all mails were received in > their complete, unmangled form. > Interesting point with gmane.io, do they have a web-interface? I do not want to do the NNTP blues, you know ;-) Thanks Bernd. > Andreas. >
Re: Can we please have the old mailing list back
On 4/10/20 9:06 AM, Andreas Schwab wrote: > On Apr 10 2020, Bernd Edlinger wrote: > >> Interesting point with gmane.io, do they have a web-interface? > > No. > Hmm. Not good. How do you access that data base ? NNTP ? Bernd. > Andreas. >
AW: basic asm and memory clobbers - Proposed solution
Hi,
I just found this in the docs:
The compiler copies the assembler instructions in a basic @code{asm}
verbatim to the assembly language output file, without
processing dialects or any of the @samp{%} operators that are available with
extended @code{asm}. This results in minor differences between basic
@code{asm} strings and extended @code{asm} templates. For example, to refer to
registers you might use @samp{%eax} in basic @code{asm} and
@samp{%%eax} in extended @code{asm}.
So it might be good to warn about % in asm statements too, because changing
anything on the asm syntax can be is quite dangerous.
And I wonder what the exact equivalence of asm("") is
in extended asm ("":::) or asm volatile ("":::) ?
Well, I start to think that Jeff is right, and we should treat a asm ("") as if
it
were asm volatile ("" ::: ) but if the asm ("nonempty with optional %") we
should
treat it as asm volatile ("nonempty with optional %%" ::: "memory").
Our docs should say that explicitly, and the implementation should follow that
direction.
Bernd.
AW: basic asm and memory clobbers - Proposed solution
Hi,
I just found this in the docs:
The compiler copies the assembler instructions in a basic @code{asm}
verbatim to the assembly language output file, without
processing dialects or any of the @samp{%} operators that are available with
extended @code{asm}. This results in minor differences between basic
@code{asm} strings and extended @code{asm} templates. For example, to refer to
registers you might use @samp{%eax} in basic @code{asm} and
@samp{%%eax} in extended @code{asm}.
So it might be good to warn about % in asm statements too, because changing
anything on the asm syntax can be is quite dangerous.
And I wonder what the exact equivalence of asm("") is
in extended asm ("":::) or asm volatile ("":::) ?
Well, I start to think that Jeff is right, and we should treat a asm ("") as if
it
were asm volatile ("" ::: ) but if the asm ("nonempty with optional %") we
should
treat it as asm volatile ("nonempty with optional %%" ::: "memory").
Our docs should say that explicitly, and the implementation should follow that
direction.
Bernd.
RE: basic asm and memory clobbers - Proposed solution
Hi,
> Well, I start to think that Jeff is right, and we should treat a asm ("") as
> if it
> were asm volatile ("" ::: )
> but if the asm ("nonempty with optional %") we should
> treat it as asm volatile ("nonempty with optional %%" ::: "memory").
> Our docs should say that explicitly, and the implementation should follow that
> direction.
>
attached is a patch that implements the above proposal.
I tried with this simple test case:
int x;
int main()
{
x = 1;
asm("test %eax,2");
x = 2;
}
gcc -O3 -S test.c
main:
.LFB0:
.cfi_startproc
movl$1, x(%rip)
#APP
# 5 "test.c" 1
test %eax,2
# 0 "" 2
#NO_APP
movl$2, x(%rip)
xorl%eax, %eax
ret
.cfi_endproc
What do you guys think?
Bernd.Index: gimple.c
===
--- gimple.c (Revision 230815)
+++ gimple.c (Arbeitskopie)
@@ -2567,6 +2567,10 @@
return true;
}
+ /* Non-empty basic ASM implicitly clobbers memory. */
+ if (gimple_asm_input_p (stmt) && strlen (gimple_asm_string (stmt)) != 0)
+return true;
+
return false;
}
Index: cfgexpand.c
===
--- cfgexpand.c (Revision 230815)
+++ cfgexpand.c (Arbeitskopie)
@@ -2650,9 +2650,23 @@
{
rtx body;
- if (TREE_CODE (string) == ADDR_EXPR)
-string = TREE_OPERAND (string, 0);
+ /* Non-empty basic ASM implicitly clobbers memory. */
+ if (TREE_STRING_LENGTH (string) != 0)
+{
+ rtx asm_op, clob;
+ asm_op = gen_rtx_ASM_OPERANDS (VOIDmode, empty_string, empty_string, 0,
+rtvec_alloc (0), rtvec_alloc (0),
+rtvec_alloc (0), locus);
+ MEM_VOLATILE_P (asm_op) = 1;
+
+ clob = gen_rtx_SCRATCH (VOIDmode);
+ clob = gen_rtx_MEM (BLKmode, clob);
+ clob = gen_rtx_CLOBBER (VOIDmode, clob);
+
+ emit_insn (gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, asm_op, clob)));
+}
+
body = gen_rtx_ASM_INPUT_loc (VOIDmode,
ggc_strdup (TREE_STRING_POINTER (string)),
locus);
Re: Solaris vtv port breaks x32 build
Hi, ~/gnu/gcc/configure --prefix=/usr --enable-bootstrap --enable-shared --enable-host-shared --enable-threads=posix --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --with-tune=haswell --with-multilib-list=m32,m64,mx32 --build=x86_64-redhat-linux build_alias=x86_64-redhat-linux --enable-offload-targets=nvptx-none --with-cuda-driver-include=/usr/local/cuda/include --with-cuda-driver-lib=/usr/local/cuda/lib64 --host=x86_64-redhat-linux host_alias=x86_64-redhat-linux --enable-languages=c,c++,fortran,jit,lto --enable-libmpx --enable-gnu-indirect-function --with-system-zlib --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --enable-vtable-verify Your host_alias looks wrong: isn't it equal to your build_alias ? Bernd.
Re: basic asm and memory clobbers - Proposed solution
On 01/12/2015 17:08, Richard Earnshaw wrote:
> On 28/11/15 04:05, David Wohlferd wrote:
>> On 11/24/2015 6:50 PM, David Wohlferd wrote:
>>> I have solved the problem with my previous patch. Here's the update
>>> (feedback welcome): http://www.LimeGreenSocks.com/gcc/24414g.zip
>>>
>>> Based on my understanding from the previous thread, this patch now
>>> does what it needs to do (code-wise) to resolve this "basic asm and
>>> memory clobbers" issue. As mentioned previously, this patch
>>> introduces a new warning (-Wonly-top-basic-asm), which is disabled by
>>> default. When enabled, it triggers a warning for any basic asm inside
>>> a function, unless the function has the "naked" attribute.
>>>
>>> An argument can be made that the default for this warning should be
>>> 'enabled.' Yes, this will break builds that use basic asm and
>>> -Werror, but it can easily be disabled with -Wno-only-top-basic-asm.
>>> And if we don't enable it, no one is going to know the check is even
>>> available. Then hidden problems like the one Paul was just describing
>>> still won't be found, and optimizations will continue to have
>>> unexpected side effects. OTOH, I can also see changing this to
>>> 'enabled' as more appropriate in the next phase 1.
>>>
>>> Now that I'm done with the code fix, I'm working on an update to the
>>> docs. Obviously they should be checked in as part of the code fix.
>>> I'm planning to actually use the word "deprecated" when describing the
>>> use of basic asm within functions. Seems like a big step.
>>>
>>> But there's no point in my proceeding any further until someone in
>>> authority agrees that this is the desired solution. I'm not actually
>>> sure who that is, but further work is a waste of time if no one is
>>> prepared to approve it.
>>>
>>> If you are that person, the questions to be answered are:
>>>
>>> 1) Is the idea of changing basic asm to "clobber everything" dead?
>>> 2) Is creating a warning for the use of "basic asm inside a function"
>>> the solution for this issue?
>>> 3) Should the warning be enabled by default in v6?
>>> 4) Should the warning be enabled by Wall or Wextra?
>>> 5) Should the v6 docs explicitly describe using "basic asm inside a
>>> function" as deprecated?
>>>
>>> If you're looking for my recommendations, I would say: Yes, Yes,
>>> (reluctantly) No, No and Yes.
>>>
>>> With this information in hand, I'll take a shot at finishing this off.
>>>
>>> For something that started out as a simple 3 sentence doc patch, this
>>> sure has turned into a project...
>> I have incorporated the feedback from Hans-Peter Nilsson, who pointed
>> out that the 'noinline' function attribute explicitly documents behavior
>> related to using asm(""). The patch now includes an exception for
>> this. Given how often this bit of code is used in various projects,
>> allowing this exception will surely ease the transition.
>>
>> I'm told that some people won't review patches unless they are included
>> as attachments, so this time the patch is attached.
>>
>> The patch includes first cut docs for the warning message, but I'm still
>> hoping to hear from someone before trying to update the basic asm docs.
>>
>> dw
>>
>> 24414h.patch
>>
>>
>> Index: gcc/c-family/c.opt
>> ===
>> --- gcc/c-family/c.opt (revision 230734)
>> +++ gcc/c-family/c.opt (working copy)
>> @@ -585,6 +585,10 @@
>> C++ ObjC++ Var(warn_namespaces) Warning
>> Warn on namespace definition.
>>
>> +Wonly-top-basic-asm
>> +C C++ Var(warn_only_top_basic_asm) Warning
>> +Warn on unsafe uses of basic asm.
>> +
>> Wsized-deallocation
>> C++ ObjC++ Var(warn_sized_deallocation) Warning EnabledBy(Wextra)
>> Warn about missing sized deallocation functions.
>> Index: gcc/c/c-parser.c
>> ===
>> --- gcc/c/c-parser.c (revision 230734)
>> +++ gcc/c/c-parser.c (working copy)
>> @@ -5880,7 +5880,18 @@
>> labels = NULL_TREE;
>>
>> if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN) && !is_goto)
>> + {
>> +/* Warn on basic asm used inside of functions,
>> + EXCEPT when in naked functions. Also allow asm(""). */
>> +if (warn_only_top_basic_asm && (TREE_STRING_LENGTH (str) != 1) )
>> + if (lookup_attribute ("naked",
>> +DECL_ATTRIBUTES (current_function_decl))
>> +== NULL_TREE)
> Formatting nit: the '== NULL_TREE)' should line up with the start of
> 'lookup_attribute'.
>
>
>> +warning_at(asm_loc, OPT_Wonly_top_basic_asm,
>> + "asm statement in function does not use extended
>> syntax");
>> +
>> goto done_asm;
>> + }
>>
>> /* Parse each colon-delimited section of operands. */
>> nsections = 3 + is_goto;
>> Index: gcc/cp/parser.c
>> ===
>> --- gcc/cp/parser.c (revision 230734)
>> +++ gcc/cp/p
AW: basic asm and memory clobbers - Proposed solution
On 1.12.2015, David Wohlferd wrote:
On 12/1/2015 10:10 AM, Bernd Edlinger wrote:
> > But IMHO asm("bla":) isn't any better than asm("bla").
> > I think _any_ asm with non-empty assembler string, that
> > claims to clobber _nothing_ is highly suspicious, and worth to be
> > warned about. I don't see any exceptions from this rule.
>
> There's one right now in the basic asm docs: asm("int $3");
>
> And I've seen others: asm volatile ("nop"), asm(".byte 0xf1\n"). I've
> seen a bunch more, but you get the idea.
I disagree.
Consider this example where "nop" should be injected
to guarantee a minimum pulse length:
int x;
x = 1;
asm volatile ("nop");
x = 0;
resulting code at -O2 is something like this:
nop
movl $0,x
That is because this asm statement allows
gcc to move the assembler code anywhere.
It is only guaranteed to be executed once,
but it not guaranteed to be executed between
the two assignments. To avoid that we have
to use a "memory" clobber.
Bernd.
Re: AW: basic asm and memory clobbers - Proposed solution
Hi,
> Surely in code like that, you would make "x" volatile? Memory clobbers
> are not a substitute for correct use of volatile accesses.
No,
It is as I wrote, a memory clobber is the only way to guarantee that
the asm statement is not move somewhere else.
I changed the example to use volatile and compiled it
with gcc-Version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5)
volatile int x;
void
test()
{
x = 1;
asm volatile("nop");
x = 0;
}
gcc -S -O2 test.c gives:
test:
.LFB0:
.cfi_startproc
movl$1, x(%rip)
movl$0, x(%rip)
#APP
# 6 "test.c" 1
nop
# 0 "" 2
#NO_APP
ret
.cfi_endproc
While it works with asm volatile("nop" ::: "memory").
Likewise for "cli" and "sti" if you try to implement critical sections.
Although, these instructions do not touch any memory, we
need the memory clobber to prevent code motion.
Bernd.
Re: Solaris vtv port breaks x32 build
On Tue, 1 Dec 2015 09:17:48, Ulrich Drepper wrote: > On Tue, Dec 1, 2015 at 2:39 AM, Matthias Klose wrote: > > that might be another instance of > > https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02064.html > > Does something like this help? > > No, same problem as before. This macro doesn't actually generate any > code in configure. Hmm, did you forget to run autoconf?
Re: basic asm and memory clobbers - Proposed solution
On 03.12.2015 00:27 David Wohlferd wrote:
> On 12/2/2015 3:34 AM, Bernd Edlinger wrote:
>> Hi,
>>
>>> Surely in code like that, you would make "x" volatile? Memory clobbers
>>> are not a substitute for correct use of volatile accesses.
>> No,
>>
>> It is as I wrote, a memory clobber is the only way to guarantee that
>> the asm statement is not move somewhere else.
>>
>> I changed the example to use volatile and compiled it
>> with gcc-Version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5)
>>
>> volatile int x;
>> void
>> test()
>> {
>> x = 1;
>> asm volatile("nop");
>> x = 0;
>> }
>>
>> gcc -S -O2 test.c gives:
>>
>>
>> test:
>> .LFB0:
>> .cfi_startproc
>> movl$1, x(%rip)
>> movl$0, x(%rip)
>> #APP
>> # 6 "test.c" 1
>> nop
>> # 0 "" 2
>> #NO_APP
>> ret
>> .cfi_endproc
>>
>>
>> While it works with asm volatile("nop" ::: "memory").
>> Likewise for "cli" and "sti" if you try to implement critical sections.
>> Although, these instructions do not touch any memory, we
>> need the memory clobber to prevent code motion.
>
> If the goal is to order things wrt x, why wouldn't you just reference x?
>
>x = 1;
>asm volatile("nop":"+m"(x));
>x = 0;
>
Exactly, that is what I mean. Either the asm can use memory clobber
or it can use output and/or input clobbers or any combination of that.
The problem with basic asm is not that it is basic, but that it does not
have any outputs nor any inputs and it has no way to specify what it
might clobber.
Therefore I think the condition for the warning should not be that the
asm is "basic", but that has zero outputs, zero inputs and zero clobbers.
That would make more sense to me.
Bernd.
> If you have a dependency, stating it explicitly seems a much better
> approach than hoping that the implied semantics of a memory clobber
> might get you what you want. Not only is it crystal clear for the
> optimizers what the ordering needs to be, people maintaining the code
> can better understand your intent as well.
>
> In summary: Yes inline asm can float. Clobbers might help. But I
> don't believe this is related to the "remove basic asm in functions"
> work. If a warning here is merited (and I'm not yet convinced), a
> separate case should be made for it.
>
> However it does seem like a good fit for a section in a "How to
> convert basic asm to extended asm" doc. The extended docs don't
> mention this need or how extended can be used to address it. It's a
> good reason for basic asm users to switch.
>
> dw
Re: basic asm and memory clobbers - Proposed solution
Am 03.12.2015 um 16:24 schrieb paul_kon...@dell.com:
> On the other hand, asm volatile ("foo":::) has a different meaning.
> That specifically says that "foo" doesn't clobber anything.
Well, not exactly, see the md_asm_adjust target callback.
On i386, rs6000, visium, cris and mn10300 targets asm volatile ("foo":::)
implicitly clobbers the "cc" register, even if that is not in the
clobber list
Bernd.
Re: basic asm and memory clobbers - Proposed solution
Hi,
On 12.12.2015 10:51, Andrew Haley wrote:
> On 11/12/15 22:18, David Wohlferd wrote:
>
>> And here are the three solutions that have been proposed:
>>
>> Solution 1:
>> Just document the current behavior of basic asm.
>>
>> People who have written incorrect code can be pointed at the text and
>> told to fix their code. People whose code is damaged by optimizations
>> can rewrite using extended to add dependencies (possibly doc this?).
>>
>> Solution 2:
>> Change the docs to say that basic asm clobbers everything (memory, all
>> registers, etc) or perhaps just memory (some debate here), but that due
>> to bugs that will eventually be addressed, it doesn't currently work
>> this way.
> It's not just bugs which make clobbering all registers unwise and/or
> impossible.
>
>> Eventually (presumably in a future phase 1) modify the code to
>> implement this.
>>
>> People who have written their code incorrectly may have some
>> hard-to-find problems solved for them. This is particularly valuable
>> for projects that are no longer being maintained. And while clobbers
>> aren't the best solution to dependencies, they can help.
>>
>> Solution 3:
>> Deprecate (and eventually disallow) the use of basic asm within
>> functions (perhaps excluding asm("") and naked functions). Do this by
>> emitting warnings (and eventually fatal errors). Doc this plan.
> You've missed the most practical solution, which meets most common
> usage: clobber memory, but not registers. That allows most of the
> effects that people intuitively want and expect, but avoids the
> breakage of register clobbers. It allows basic asm to be used in a
> sensible way by pushing and popping all used registers.
>
> Andrew.
Yes, implicitly clobber memory and cc-flags, on targets where that is
also automatically added to the clobber list, even when the user does
not mention that in the extended asm syntax.
That is also my preferred solution, I prepared a patch for next stage1,
the details are still under discussion, but it is certainly do-able with
not too much effort.
See https://gcc.gnu.org/ml/gcc-patches/2015-12/msg00938.html
The rationale for this is: there are lots of ways to write basic asm
statements, that may appear to work, if they clobber memory. Even if
they clobber CC that will also work most of the time.
And because you can use all global values simply by name, users will
expect that to be supported.
And, basic asm is a instruction scheduling barrier, that is similar to a
memory barrier. See sched-deps.c:
case ASM_INPUT:
{
/* Traditional and volatile asm instructions must be considered
to use
and clobber all hard registers, all pseudo-registers and all of
memory. So must TRAP_IF and UNSPEC_VOLATILE operations.
Consider for instance a volatile asm that changes the fpu
rounding
mode. An insn should not be moved across this even if it
only uses
pseudo-regs because it might give an incorrectly rounded
result. */
I think, this comment was written by Jeff Law in 1997, and in simple
cases (w/o loops and redundant assignments) the code below does 99%
exactly what the user expects. The problematic optimizations only
happen because of we did not have the implicit memory barriers yet,
which adds only the missing 1%.
My personal gut feeling on that warning is a bit mixed...
If we have a -Wall-enabled warning on asm("..."), people who know next
to nothing about assembler will be encouraged to "fix" this warning in a
part of the code which they probably do not understand at all. This
frightens me a bit.
Because I know they will soon find out, that adding a few colons fixes
the warning, but asm("...":::) is just more ugly, and will not be any
better IMHO.
For me, it is just very very unlikely that any piece of assembler really
clobbers nothing and has no inputs and no outputs at the same time, even
it it looks so at first sight...
It is much more likely that someone forgot to fill in the clobber section.
So for me it would also be good to warn on asm("...":::) and require
that, if they want to fix this warning, they must at least write
something in the clobber section, like asm ("...":::"nothing"); that
would be a new clobber name which can only stand alone and, which can
get stripped after the warning processing took place in the FE.
If the warning would warn on basic asm and extended asm with zero-input,
zero-output and zero-clobbers, only then I would regard that an
improvement. And for those few examples where we really have invented
something that does not clobber anything, I would say then you should
also write "nothing" in the clobber section.
Bernd.
AW: basic asm and memory clobbers - Proposed solution
Hi,
On 12/15/2015 13:52, Bernd Schmidt wrote:
>
> On 12/14/2015 09:10 AM, Segher Boessenkool wrote:
> > That, and adding a memory clobber degrades performance for a lot of
> > existing basic asm that does not expect the clobber, e.g. asm(""),
> > asm("#"), asm("nop"), ...
>
> I wonder about this. People keep bringing up "a lot of existing basic
> asm" in general, but are there any known examples in real software?
Yes, there are, I see a few in our own tree:
./libatomic/config/x86/fenv.c: asm volatile ("fwait");
./libcilkrts/runtime/config/x86/os-fence.h:# define __cilkrts_fence() __asm__
volatile ("lock addl $0,(%rsp)")
./libsanitizer/sanitizer_common/sanitizer_atomic_clang_x86.h:__asm__
__volatile__("pause");
./libgfortran/config/fpu-387.h: __asm__ __volatile__ ("fwait");
./libgcc/unwind-dw2.c: asm ("");
./libgcc/config/i386/sfp-exceptions.c: asm volatile ("fwait");
./libgcc/config/lm32/_udivsi3.c: __asm__ __volatile__ ("mv ea, ra");
./boehm-gc/include/private/gc_locks.h:#define GC_clear(addr) { asm("mb");
*(volatile unsigned *)addr = 0; }
!! this one really needs memory clobber !!
./boehm-gc/include/private/gc_locks.h:# define GC_memory_barrier() asm("mb")
./boehm-gc/mach_dep.c:asm("pushl r11"); asm("calls $1,_GC_push_one");
Bernd.
AW: basic asm and memory clobbers - Proposed solution
Hi,
On 15. Dezember 2015 23:43, Joseph Myers wrote:
> I think the typical use of basic asm is: you want to manipulate I/O
> registers or other such state unknown to the compiler (not any registers
> the compiler might use itself), and you want to do it in a way that is
> maximally compatible with as many compilers as possible (hence limiting
> yourself to the syntax subset that's in the C++ standard, for example).
> Compatibility with a wide range of other compilers is the critical thing
> here; this is not a GCC-invented feature, and considerations for
> deprecating an externally defined feature are completely different from
> considerations for GCC-invented features. Do you have evidence that it is
> now unusual for compilers to support basic asm without supporting
> GCC-compatible extended asm, or that other compiler providers generally
> consider basic asm deprecated?
I totally agree, and I know of another compiler which supports basic asm,
but no extended asm:
The windriver diab compiler does apparently not support extended asm,
instead they offer either "asm string statements" or "asm macros".
This is quoted from http://www.vxdev.com/docs/vx55man/diab5.0ppc/c-embed_.htm
"asm string statements are primarily useful for manipulating data in static
variables and special registers, changing processor status, etc., and are
subject to several restrictions: no assumption can be made about register
usage, non-scratch registers must be preserved, values may not be returned,
some optimizations are disabled, and more. asm macro functions described above
are recommended instead. See Notes and restrictions below.
An asm string statement provides a simple way to embed instructions in the
assembly code generated by the compiler. Its syntax is
asm[ volatile] ("string"[ ! register-list]);
where string is an ordinary string constant following the usual rules (adjacent
strings are pasted together, a `\' at the end of the line is removed, and the
next line is concatenated) and register-list is a list of scratch registers
(see Register-list line, p.154). The optional volatile keyword prevents
instructions from being moved before or after the string statement.
An asm string statement can be used wherever a statement or an external
declaration is allowed. string will be output as a line in the assembly code at
the point in a function at which the statement is encountered, and so must be a
valid assembly language statement.
If several assembly language statements are to be generated, they may either be
written as successive asm string statements, or by using `\n' within the string
to end each embedded assembly language statement. The compiler will not insert
any code between successive asm string statements.
If an asm string statement contains a label, and the function containing the
asm string is inlined more than once in some other function, a duplicate label
error will occur. Use an asm macro with a storage mode line containing a lab
clause for this case. See asm macros, p.151. "
asm macros on the other hand are very sophisticated, but totally incompatible
with gcc
Also from there:
Examples of asm macros
In this example, standard PowerPC techniques are used to wait on and then seize
a semaphore when it becomes free.
asm void semaphore_seize (volatile int *semaphore_p)
{
% reg semaphore_p; lab loop
! "r4", "r5"/* scratch registers used */
addir4,r0,1 # token for semaphore
loop: # label replaced by compiler
lwarx r5,r0,semaphore_p # semaphore will be in register
cmpicr0,0,r5,0 # semaphore free?
bne loop# branch if no
stwcx. r4,r0,semaphore_p # store token in semaphore
bne loop# in case interrupted before stwcz.
}
So, yes, you are right, the only globally understood form of assembler
statements in C is basic asm.
Bernd.
AW: basic asm and memory clobbers - Proposed solution
Hi,
On Thu, 17 Dec 2015 15:13:07, Bernd Schmidt wrote:
> > What's your take on making -Wonly-top-basic-asm a default (either now or
> > v7)? Is making it a non-default a waste of time because no one will
> > ever see it? Or is making it a default too aggressive? What about
> > adding it to -Wall?
>
>
> Depends if anyone has one in system headers I guess. We could try to add it
> to -Wall.
Sorry, but if I have to object.
Adding this warning to -Wall is too quickly and will bring the ia64, tilegx and
mep ports into trouble.
Each of them invokes some special semantics for basic asm:
mep: mep_interrupt_saved_reg looks for ASM_INPUT in the body, and saves
different registers if found.
tilegx: They never wrap {} around inline asm. But extended asm, are handled
differently, probably automatically surrounded by braces.
ia64: ASM_INPUT emits stop-bits. extended asm does not the same. That was
already mentioned by Segher.
Bernd.
Re: basic asm and memory clobbers - Proposed solution
On 18.12.2015 10:27, David Wohlferd wrote:
> On 12/17/2015 11:30 AM, Bernd Edlinger wrote:
>> On Thu, 17 Dec 2015 15:13:07, Bernd Schmidt wrote:
>>>>What's your take on making -Wonly-top-basic-asm a default
>>>> (either now or
>>>>v7)? Is making it a non-default a waste of time because no one
>>>> will
>>>>ever see it? Or is making it a default too aggressive? What about
>>>>adding it to -Wall?
>>> Depends if anyone has one in system headers I guess. We could try to
>>> add it to -Wall.
>> Sorry, but if I have to object.
>>
>> Adding this warning to -Wall is too quickly and will bring the ia64,
>> tilegx and mep ports into trouble.
>
> It doesn't look to me like adding the warnings will affect gcc
> itself. But I do see how it could have an impact on people using gcc
> on those platforms, if the warning causes them to convert to extended
> asm.
>
At least we should not start a panic until we have really understood all
the details, how to do that.
>> Each of them invokes some special semantics for basic asm:
>
> I'm collecting these for my "How to convert basic asm to extended asm"
> document. This may need to go in the gcc wiki instead of the user
> guide, since people may find important conversion tips like these
> asynchronous to gcc's releases.
>
>> mep: mep_interrupt_saved_reg looks for ASM_INPUT in the body, and
>> saves different registers if found.
>
> I'm trying to follow this code. A real challenge since I know nothing
> about mep. But what I see is:
>
> - This routine only applies to functions marked as
> __attribute__((interrupt)).
> - To correctly generate entry/exit, it walks thru each register (up to
> FIRST_PSEUDO_REGISTER) to see if it is used by the routine. If there
> is any basic asm within the routine (regardless of its contents), the
> register is considered 'in use.'
>
> The net result is that every register gets saved/restored by the
> entry/exit of this routine if it contains basic asm. The reason this
> is important is that if someone just adds a colon, it would suddenly
> *not* save/restore all the registers. Depending on what the asm does,
> this could be bad.
>
> Does that sound about right?
>
Yes.
> This is certainly worth mentioning in the 'convert' doc.
>
> I wonder how often this 'auto-clobber' feature is used, though. I
> don't see it mentioned in the 'interrupt' attribute docs for mep, and
> it's not in the basic asm docs either. If your interrupt doesn't need
> many registers, it seems like you'd want to know this and possibly use
> extended. And you'd really want to know if you are doing a
> (redundant) push/pop in your interrupt.
>
>> tilegx: They never wrap {} around inline asm. But extended asm, are
>> handled differently, probably automatically surrounded by braces.
>
> I know nothing about tilegx either. I've tried to read the code, and
> it seems like basic asm does not get 'bundled' while extended might be.
>
> Bundling for tilegx (as I understand it) is when you explicitly fill
> multiple pipelines by doing something like this:
>
>{ add r3,r4,r5 ; add r7,r8,r9 ; lw r10,r11 }
>
> So if you have a basic asm statement, you wouldn't want it
> automatically bundled by the compiler, since your asm could be more
> than 3 statements (the max?). Or your asm may do its own bundling. So
> it makes sense to never output braces when outputting basic asm.
>
> I know I'm guessing about what this means, but doesn't it seem like
> those same limitations would apply to extended? I wonder if this is a
> bug. I don't see any discussion of bundling (of this sort) in the docs.
>
I wold like to build a cross compiler, but currently that target seems
to be broken. I have to check
that target anyways, because of my other patch with the memory clobbers.
I see in tilegx_asm_output_opcode, that they do somehow automatically
place braces.
An asm("pseudo":) has a special meaning, and can be replaced with "" or "}".
However the static buf[100] means that any extended asm string > 95
characters, invokes
the gcc_assert in line 5487. In the moment I would _not_ recommend
changing any asm
statements without very careful testing.
>> ia64: ASM_INPUT emits stop-bits. extended asm does not the same.
>> That was already mentioned by Segher.
>
> I already had this one from Segher's email.
>
>
> Given all this, I'm more convinced than ever of the value of
> -Wonly-t
Re: basic asm and memory clobbers - Proposed solution
Hi,
On 19.12.2015 19:54, David Wohlferd wrote:
>
mep: mep_interrupt_saved_reg looks for ASM_INPUT in the body, and
saves different registers if found.
>>> I'm trying to follow this code. A real challenge since I know nothing
>>> about mep. But what I see is:
>>>
>>> - This routine only applies to functions marked as
>>> __attribute__((interrupt)).
>>> - To correctly generate entry/exit, it walks thru each register (up to
>>> FIRST_PSEUDO_REGISTER) to see if it is used by the routine. If there
>>> is any basic asm within the routine (regardless of its contents), the
>>> register is considered 'in use.'
>>>
>>> The net result is that every register gets saved/restored by the
>>> entry/exit of this routine if it contains basic asm. The reason this
>>> is important is that if someone just adds a colon, it would suddenly
>>> *not* save/restore all the registers. Depending on what the asm does,
>>> this could be bad.
>>>
>>> Does that sound about right?
>>>
>> Yes.
>
> Seems like a doc update would be appropriate here too then, if anyone
> wanted to volunteer...
>
To confirm this, I built a cross-compiler, but It was difficult, because
of pr64402.
Yes, a function with __attribute__((interrupt)) spills lots of
registers, when it just
contains asm(""); but almost nothing, if asm("":); is used. That is
remarkable.
>>> This is certainly worth mentioning in the 'convert' doc.
>>>
>>> I wonder how often this 'auto-clobber' feature is used, though. I
>>> don't see it mentioned in the 'interrupt' attribute docs for mep, and
>>> it's not in the basic asm docs either. If your interrupt doesn't need
>>> many registers, it seems like you'd want to know this and possibly use
>>> extended. And you'd really want to know if you are doing a
>>> (redundant) push/pop in your interrupt.
>>>
tilegx: They never wrap {} around inline asm. But extended asm, are
handled differently, probably automatically surrounded by braces.
>>> I know nothing about tilegx either. I've tried to read the code, and
>>> it seems like basic asm does not get 'bundled' while extended might be.
>>>
>>> Bundling for tilegx (as I understand it) is when you explicitly fill
>>> multiple pipelines by doing something like this:
>>>
>>> { add r3,r4,r5 ; add r7,r8,r9 ; lw r10,r11 }
>>>
>>> So if you have a basic asm statement, you wouldn't want it
>>> automatically bundled by the compiler, since your asm could be more
>>> than 3 statements (the max?). Or your asm may do its own bundling. So
>>> it makes sense to never output braces when outputting basic asm.
>>>
>>> I know I'm guessing about what this means, but doesn't it seem like
>>> those same limitations would apply to extended? I wonder if this is a
>>> bug. I don't see any discussion of bundling (of this sort) in the
>>> docs.
>>>
>> I wold like to build a cross compiler, but currently that target seems
>> to be broken. I have to check
>> that target anyways, because of my other patch with the memory clobbers.
>>
>> I see in tilegx_asm_output_opcode, that they do somehow automatically
>> place braces.
>> An asm("pseudo":) has a special meaning, and can be replaced with ""
>> or "}".
>> However the static buf[100] means that any extended asm string > 95
>> characters, invokes
>> the gcc_assert in line 5487. In the moment I would _not_ recommend
>> changing any asm
>> statements without very careful testing.
>
> Yes, the handling of extended asm there is very strange.
>
> If bundles can only be 3 instructions, then appending an entire
> extended asm in a single (already in-use) bundle seems odd. But maybe
> that's just because I don't understand tilegx.
>
> I'm not sure it's just changing basic asm to extended I would be
> concerned about. I'd worry about using extended asm at all...
>
I also built a tilegx cross compiler, but that was even more difficult,
because
of pr68917, which hit me when building the libgcc.
I tried a while, but was unable to find any example of an extended asm
that gets
auto-braces, or which can trigger the mentioned gcc_assert. It looks
like, in all
cases the bundles are closed already before the extended asm gets scheduled.
Probably the middle end already knows that it can't parse the asm string.
I see examples of extended asm in the linux /arch/tile tree, and some have
braces, some not. But as it seems, they are completely left alone by
the expansion.
You do not have to escape the { and } for extended asm, on this target,
using
%{ produces even an error. In deed asm ("pseudo":) expands to nothing,
it is
probably used as a memory barrier in the .md files. But asm ("pseudo")
feeds
"pseudo" to the assembler, which doesn't like it.
BTW: The tilepro target is a32-bit variant of tilegx and it has exactly
the same logic.
ia64: ASM_INPUT emits stop-bits. extended asm does not the same.
That was already mentioned by Segher.
>>> I already had this one from Segher's email.
>>>
>>>
>>> Given all th
Re: basic asm and memory clobbers - Proposed solution
On 20.12.2015 23:53, David Wohlferd wrote:
> On 12/20/2015 10:26 AM, Bernd Edlinger wrote:
>> On 19.12.2015 19:54, David Wohlferd wrote:
>>>>>> mep: mep_interrupt_saved_reg looks for ASM_INPUT in the body, and
>>>>>> saves different registers if found.
>>>>> I'm trying to follow this code. A real challenge since I know
>>>>> nothing
>>>>> about mep. But what I see is:
>>>>>
>>>>> - This routine only applies to functions marked as
>>>>> __attribute__((interrupt)).
>>>>> - To correctly generate entry/exit, it walks thru each register
>>>>> (up to
>>>>> FIRST_PSEUDO_REGISTER) to see if it is used by the routine. If there
>>>>> is any basic asm within the routine (regardless of its contents), the
>>>>> register is considered 'in use.'
>>>>>
>>>>> The net result is that every register gets saved/restored by the
>>>>> entry/exit of this routine if it contains basic asm. The reason this
>>>>> is important is that if someone just adds a colon, it would suddenly
>>>>> *not* save/restore all the registers. Depending on what the asm
>>>>> does,
>>>>> this could be bad.
>>>>>
>>>>> Does that sound about right?
>>>>>
>>>> Yes.
>>> Seems like a doc update would be appropriate here too then, if anyone
>>> wanted to volunteer...
>> To confirm this, I built a cross-compiler, but It was difficult, because
>> of pr64402.
>> Yes, a function with __attribute__((interrupt)) spills lots of
>> registers, when it just
>> contains asm(""); but almost nothing, if asm("":); is used. That is
>> remarkable.
>
> So if you use extended and clobber a couple registers asm("":::"r0",
> "r1"), does it spill just those specific registers? That would be cool.
>
Yes, the register names are "$0", "$1" and so on.
I tried
void __attribute__((interrupt))
intf(void)
{
asm("":::"$0", "$1");
}
$0 and $1 got saved, but only $1 got restored !
this is probably a bug. The loop in mep_expand_epilogue
is wrong, and does not restore $0, fixed that and $0 also
got restored.
> I don't write interrupt handlers, and certainly not on mep. But the
> little I know about them says that performance is an important (and
> sometimes critical) characteristic. There would be risk in changing
> this to extended (if you used a register but forgot to clobber it),
> but depending on the interrupt, it could be a nice performance 'win.'
>
> If no one else is prepared to step up to write this, I can. I'm just
> uncomfortable doing so since I can't try it myself. And I feel weird
> writing a patch for mep given that I know nothing about it.
>
> But since Bernd has tried it, maybe something like this added to the
> 'interrupt' attribute on
> https://gcc.gnu.org/onlinedocs/gcc/MeP-Function-Attributes.html
> -
> Be aware that if the function contains any basic @code{asm}
> (@pxref{Basic Asm}), all registers (whether referenced in the asm or
> not) will be preserved upon entry and restored upon exiting the
> interrupt. More efficient code can be generated by using extended
> @code{asm} (@pxref{Extended Asm}) and explicitly listing only the
> specific registers that need to be preserved (or none if your asm
> preserves any registers it uses).
> -
>
>>>>>> tilegx: They never wrap {} around inline asm. But extended asm, are
>>>>>> handled differently, probably automatically surrounded by braces.
>>>>> I know nothing about tilegx either. I've tried to read the code, and
>>>>> it seems like basic asm does not get 'bundled' while extended
>>>>> might be.
>>>>>
>>>>> Bundling for tilegx (as I understand it) is when you explicitly fill
>>>>> multiple pipelines by doing something like this:
>>>>>
>>>>> { add r3,r4,r5 ; add r7,r8,r9 ; lw r10,r11 }
>>>>>
>>>>> So if you have a basic asm statement, you wouldn't want it
>>>>> automatically bundled by the compiler, since your asm could be more
>>>>> than 3 statements (the max?). Or your asm may do its own
>>>>> bundling. So
>>>>> it makes sense to never output braces when outputting basic asm.
>>>>>
>>>>> I know I'm guessing about what this m
Re: Manipulating bit fields is behaving inconsistently
Hi,
> struct fields {
> long long unsigned f0:12;
> long long unsigned f1:52;
> } __attribute__((__packed__));
the C99 standard ISO/IEC 9899 forbids this type:
6.7.2.1 Structure and union specifiers
4 A bit-field shall have a type that is a qualified or unqualified version of
_Bool, signed int,
unsigned int, or some other implementation-defined type.
The C standard simply does not promote the bit-field value to any type larger
than int or
unsigned int.
GCC chooses to do the larger than int computations in an artificial 52-bit
type, but it is a
non-standard extension.
And if you compile your example with -Wall you'll see the problem:
gcc -m32 -O3 -Wall test.c
test.c: In function 'main':
test.c:17:21: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 2 has type 'int' [-Wformat=]
printf("x.f0=0x%llx\n", x.f0);
^
test.c:19:21: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 2 has type 'long long unsigned int:52' [-Wformat=]
printf("x.f1=0x%llx\n", x.f1);
^
so especially the first warning is no joke:
./a.out
x.f0=0x80497b40fff
g0=0x1ffe expect 0x1ffe
x.f1=0xf
g1=0xe expect 0x1e
OTOH that is perfectly OK for C++:
gcc -x c++ -m32 -O3 -Wall test.c
./a.out
x.f0=0xfff
g0=0x1ffe expect 0x1ffe
x.f1=0xf
g1=0x1e expect 0x1e
Regards
Bernd.
Re: _Bool and trap representations
Hi,
I modified Aexander's test case a bit, and found something
unexpected, which looks like a GCC-BUG to me:
cat test.c
#include
#include
#include
int main()
{
long double d0, d;
memcpy(&d0,
"\x00\x00\x00\x00\x00\x00\x00\x00\xff\x3f\x00\x00\x00\x00\x00\x00", sizeof d0);
// d = d0;
memcpy(&d, &d0, sizeof d0);
// if (memcmp(&d, &d0, sizeof d))
// abort();
printf("d = %Lf\n", d);
for (unsigned char *p = (unsigned char *)&d + sizeof d; p > (unsigned char
*)&d;)
printf("%02x ", *--p);
printf("\n");
}
// EOF
gcc -O3 test.c && ./a.out
d = 0.00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
but, when I un-comment the memcmp the test case shows the expected
result:
d = 0.00
00 00 00 00 00 00 3f ff 00 00 00 00 00 00 00 00
gcc-Version 7.0.0 20160613 (experimental) (GCC)
Would'nt you agree, that if we use memcpy it should be possible
to preserve denormalized or otherwise invalid bit patterns?
And why should the memcmp have an influence on the memcpy?
Bernd.
Re: Deprecating basic asm in a function - What now?
On 21/06/2016 17:53, Andrew Haley wrote: > On 21/06/16 17:43, Jeff Law wrote: > > I think there's enough resistance to deprecating basic asms within a > > function that we should probably punt that idea. > > > > I do think we should look to stomp out our own uses of basic asms > > within functions just from a long term maintenance standpoint. > > > > Finally I think we should continue to bring the implementation of > > basic asms more in-line with expectations and future proofing them > > since I'm having a hard time seeing a reasonable path to deprecating > > their use. > > Me too. I wonder if there's anything else we can do to make basic asm > in a function a bit less of a time bomb. > > Andrew. I do not like the idea to deprecate the basic asm at all, I must admit, but I think if we added a warning, that just contains a positive information, like "warning: basic asm semantic has been changed to implicitly clobber memory, if you have a problem with that, please convert this asm statement to extended asm syntax." Then that would possibly be acceptable for everybody here. We could still discuss, if that warning should be enabled with -Wall, -Wextra or only on request. Bernd.
cxx_fundamental_alignment vs. __float128
Hi Joseph,
I've just stumbled over this function in gcc/c-family/c-common.c,
which might need adjustment for __float128:
/* Return true iff ALIGN is an integral constant that is a fundamental
alignment, as defined by [basic.align] in the c++-11
specifications.
That is:
[A fundamental alignment is represented by an alignment less than or
equal to the greatest alignment supported by the implementation
in all contexts, which is equal to
alignof(max_align_t)]. */
bool
cxx_fundamental_alignment_p (unsigned align)
{
return (align <= MAX (TYPE_ALIGN (long_long_integer_type_node),
TYPE_ALIGN (long_double_type_node)));
}
Which might need adjustment, if you want to change alignof(max_align_t).
Bernd.
Message missing from gcc-patches
Hi,
this message did not get listed on the gcc-patches archive.
I've got no bounce, and it just vanished, several times.
Any idea what is wrong?
Bernd.
Forwarded Message
Subject: [PATCHv3, resent] Add a warning for suspicious use of
conditional expressions in boolean context
Date: Thu, 15 Sep 2016 11:19:32 +0200
From: Bernd Edlinger
To: GCC Patches
CC: Jason Merrill , Jeff Law , Joseph
Myers
Hi,
I send the latest version of the warning patch again, because I don't
see the previous patch submission on the gcc-patches list. It dropped
out silently, twice :(
Don't know what went wrong, so please excuse me if this e-mail arrives
duplicate.
On 09/14/16 20:11, Jason Merrill wrote:
>>
>> Yes. The reasoning I initially had was that it is completely
>> pointless to have something of the form "if (x ? 1 : 2)" or
>> "if (x ? 0 : 0)" because the result does not even depend on x
>> in this case. But something like "if (x ? 4999 : 0)" looks
>> bogus but does at least not ignore x.
>>
>> If the false-positives are becoming too much of a problem here,
>> then I should of course revert to the previous heuristic again.
>
> I think we could have both, where the weaker form is part of -Wall and
> people can explicitly select the stronger form.
>
Yes, agreed. So here is what I would think will be the first version.
It can later be extended to cover the more pedantic cases which
will not be enabled by -Wall.
I would like to send a follow-up patch for the warning on
signed-integer shift left in boolean context, which I think
should also be good for Wall.
(I already had that feature in patch version 2 but that's meanwhile
outdated).
Bootstrap and reg-testing on x86_64-pc-linux-gnu.
Is it OK for trunk?
Thanks
Bernd.
gcc:
2016-09-14 Bernd Edlinger
PR c++/77434
* doc/invoke.texi: Document -Wcond-in-bool-context.
PR middle-end/77421
* dwarf2out.c (output_loc_operands): Fix an assertion.
c-family:
2016-09-14 Bernd Edlinger
PR c++/77434
* c.opt (Wcond-in-bool-context): New warning.
* c-common.c (c_common_truthvalue_conversion): Warn on integer
constants in boolean context.
cp:
2016-09-14 Bernd Edlinger
PR c++/77434
* cvt.c (cp_convert_and_check): Suppress Wint-in-bool-context here.
testsuite:
2016-09-14 Bernd Edlinger
PR c++/77434
* c-c++-common/Wcond-in-bool-context.c: New test.
Index: gcc/builtins.c
===
--- gcc/builtins.c (revision 240135)
+++ gcc/builtins.c (working copy)
@@ -7887,15 +7887,18 @@ fold_builtin_classify (location_t loc, tree fndecl
tree isinf_call = build_call_expr_loc (loc, isinf_fn, 1, arg);
signbit_call = fold_build2_loc (loc, NE_EXPR, integer_type_node,
- signbit_call, integer_zero_node);
+ signbit_call, integer_zero_node);
isinf_call = fold_build2_loc (loc, NE_EXPR, integer_type_node,
- isinf_call, integer_zero_node);
+ isinf_call, integer_zero_node);
- tmp = fold_build3_loc (loc, COND_EXPR, integer_type_node, signbit_call,
- integer_minus_one_node, integer_one_node);
tmp = fold_build3_loc (loc, COND_EXPR, integer_type_node,
- isinf_call, tmp,
- integer_zero_node);
+ signbit_call, integer_minus_one_node,
+ integer_one_node);
+ /* Avoid a possible -Wint-in-bool-context warning in C. */
+ tmp = fold_build2_loc (loc, PLUS_EXPR, integer_type_node, tmp,
+ integer_zero_node);
+ tmp = fold_build3_loc (loc, COND_EXPR, integer_type_node,
+ isinf_call, tmp, integer_zero_node);
}
return tmp;
Index: gcc/c-family/c-common.c
===
--- gcc/c-family/c-common.c (revision 240135)
+++ gcc/c-family/c-common.c (working copy)
@@ -4652,6 +4652,19 @@ c_common_truthvalue_conversion (location_t locatio
TREE_OPERAND (expr, 0));
case COND_EXPR:
+ if (warn_int_in_bool_context)
+ {
+ tree val1 = fold_for_warn (TREE_OPERAND (expr, 1));
+ tree val2 = fold_for_warn (TREE_OPERAND (expr, 2));
+ if (TREE_CODE (val1) == INTEGER_CST
+ && TREE_CODE (val2) == INTEGER_CST
+ && !integer_zerop (val1)
+ && !integer_zerop (val2)
+ && (!integer_onep (val1)
+ || !integer_onep (val2)))
+ warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context,
+ "?: using integer constants in boolean context");
+ }
/* Distribute the conversion into the arms of a COND_EXPR. */
if (c_dialect_cxx ())
{
Index: gcc/c-family/c.opt
===
--- gcc/c-family/c.opt (revision 240135)
+++ gcc/c-family/c.opt (working copy)
@@ -545,6 +545,10 @@ Wint-conversion
C ObjC Var
sprintf warning on overlapping output
Hi Martin, in the past I have seen (and fixed) code like sprintf(buf, "%s %d", buf, x); that may possibly work by chance, but usually produces undefined results. Do you see a way to enhance the warning for cases where the output buffer overlaps an input buffer? Thanks Bernd.
Translated strings with sprintf %-directives
Hi Joseph, I just noticed that translated strings might have different sprintf arguments than the original message: $ LANG=de_DE.UTF-8 gcc -v --help|&grep shadow -Wintrinsic-shadow Warnen, wenn eine Benutzer-Prozedur denselben Namen wie ein Intrinsic hat. -Wshadow-ivar Warnen, wenn eine lokale Deklaration von %qE eine Instanzvariable verdeckt. -WshadowWarnen, wenn eine Variable eine andere überdeckt. Entspricht -Wshadow=global. -Wshadow-compatible-local Identisch mit -Wshadow=compatible-local. Verwenden Sie daher bitte diese Option. -Wshadow-local Identisch mit -Wshadow=local. Verwenden Sie daher bitte diese Option. -Wshadow=compatible-local Warnen, wenn eine lokale Variable eine andere lokale Variable oder einen Parameter mit gleichem Typ überdeckt. -Wshadow=global Warn when one variable shadows another (globally). ist identisch mit -Wshadow. -Wshadow=local Warnen, wenn eine lokale Variable eine andere lokale Variable oder einen Parameter überdeckt. -fasan-shadow-offset= Spezifischen Offset für Schattenspeicher verwenden. while $ LANG=C gcc -v --help|&grep shadow -Wintrinsic-shadow Warn if a user-procedure has the same name as an intrinsic. -Wshadow-ivar Warn if a local declaration hides an instance variable. -WshadowWarn when one variable shadows another. Same as -Wshadow=global. -Wshadow-compatible-local Same as -Wshadow=compatible-local. Use the latter option instead. -Wshadow-local Same as -Wshadow=local. Use the latter option instead. -Wshadow=compatible-local Warn when one local variable shadows another local variable or parameter of compatible type. -Wshadow=global Warn when one variable shadows another (globally). Same as -Wshadow. -Wshadow=local Warn when one local variable shadows another local variable or parameter. -mshstk Enable shadow stack built-in functions from Control-flow Enforcement Technology (CET). -fasan-shadow-offset= Use custom shadow memory offset. so the translated string value of -Wshadow-ivar has a sprintf format directive, while the original string does not. In this case it is not used with with sprintf so nothing happens with the string, but that is probably not always the case. But I wonder if that would be a kind of a security concern otherwise. Shouldn't there be an automatic check that the %-directives are given in the original and the translated message are exactly the same? Bernd.
Adding -Wshadow=local to gcc build rules
Hi, I'm currently trying to add -Wshadow=local to the gcc build rules. I started with -Wshadow, but gave up that idea immediately. As you could expect the current code base has plenty of shadowed local variables. Most are trivial to resolve, some are less trivial. I am not finished yet, but it is clear that it will be a rather big patch. I would like to ask you if you agree that would be a desirable step, in improving code quality in the gcc tree. Thanks Bernd.
Re: Adding -Wshadow=local to gcc build rules
On 9/19/19 12:19 PM, Richard Biener wrote:
> On Wed, Sep 18, 2019 at 3:09 PM Bernd Edlinger
> wrote:
>>
>> Hi,
>>
>> I'm currently trying to add -Wshadow=local to the gcc build rules.
>> I started with -Wshadow, but gave up that idea immediately.
>>
>> As you could expect the current code base has plenty of shadowed
>> local variables. Most are trivial to resolve, some are less trivial.
>> I am not finished yet, but it is clear that it will be a rather big
>> patch.
>>
>> I would like to ask you if you agree that would be a desirable step,
>> in improving code quality in the gcc tree.
>
> I wonder if -Wshadow=compatible-local is easier to achieve?
>
I tried that and it does not make a big difference, while
it misses for instance:
* gcc/c-family/c-ada-spec.c (dump_ada_macros)
unsigned char *s, shadowed by const unsigned char *s. :-/
On the other hand, -Wshadow=global is a lot more difficult,
because we have lots of globals, for instance:
context.h:
/* The global singleton context aka "g".
(the name is chosen to be easy to type in a debugger). */
extern gcc::context *g;
But unfortunately Wshadow=local does not find class members
shadowed by local variable, which happens for instance in
wide-int
With -Wshadow, I had to change this in wide-int.h:
Index: gcc/wide-int.h
===
--- gcc/wide-int.h (revision 275545)
+++ gcc/wide-int.h (working copy)
@@ -648,9 +648,9 @@ namespace wi
storage_ref () {}
storage_ref (const HOST_WIDE_INT *, unsigned int, unsigned int);
-const HOST_WIDE_INT *val;
-unsigned int len;
-unsigned int precision;
+const HOST_WIDE_INT *m_val;
+unsigned int m_len;
+unsigned int m_precision;
So this change was not necessary for -Wshadow=local, although
I would think that, shadowing class members is not much different from
shadowing a local scope.
Not sure if shadowing class members should be part of -Wshadow=local
instead of -Wshadow=global.
Bernd.
Re: Adding -Wshadow=local to gcc build rules
On 9/18/19 3:08 PM, Bernd Edlinger wrote: > Hi, > > I'm currently trying to add -Wshadow=local to the gcc build rules. > I started with -Wshadow, but gave up that idea immediately. > > As you could expect the current code base has plenty of shadowed > local variables. Most are trivial to resolve, some are less trivial. > I am not finished yet, but it is clear that it will be a rather big > patch. > > I would like to ask you if you agree that would be a desirable step, > in improving code quality in the gcc tree. > > > Thanks > Bernd. > Hurray!!! The build is now working with -Wshadow=local and I need to sort out the changes. I hope you are gonna like it :-) Currently it is a 600K patch file all together. Some parts are not trivial at all, but I hope it will pay off in more easy to follow code in various places and less errors in new code. Also one or two real bugs were hiding near the shadowed variable. Most of which are of the obvious kind where, one variable is shadowed by another one, and either the inner or the outer variable need to be renamed from name to name1, or i to j/k, or p to q, or x to y. Sometimes also to name2 or name3, etc. If it is a location_t I follow common practice that loc is renamed to loc0. I avoid renaming a parameter, unless it causes much more changes. Also when a renamed variable appears to be named in a comment I also update the comment. I tried to make the change smaller if possible, rather often the shadowed variable is of the exact same type, and no longer used at the place where the variable is declared again. I consider "unsigned" and "unsigned int" as too different to re-use the variable (or parameter). This means I did consider the control flow and data flow around each shadowed variable if avoiding the duplicate variable is safe. Unless there is disagreement I will go ahead and commit trivial parts after careful regression-testing under the obvious rule. I did also try to build a lot of cross compilers to make sure that the targets do not break when the warning is enabled. But it is nevertheless rather likely that you will see some fall-out once the warning will be enabled. So far each target (except tilepro) needed at least a few changes. This is the list of changed files: M ada/gcc-interface/decl.c M ada/gcc-interface/trans.c M asan.c M attribs.c M auto-inc-dec.c M bb-reorder.c M brig/brigfrontend/brig-basic-inst-handler.cc M brig/brigfrontend/brig-code-entry-handler.cc M brig/brigfrontend/brig-function-handler.cc M brig/brigfrontend/brig-util.cc M builtins.c M c/c-decl.c M c/c-parser.c M c/c-typeck.c M c/gimple-parser.c M c-family/c-ada-spec.c M c-family/c-common.c M c-family/c-cppbuiltin.c M c-family/c-lex.c M c-family/c-omp.c M caller-save.c M calls.c M cfg.c M cfgbuild.c M cfgcleanup.c M cfgexpand.c M cfgloopmanip.c M cfgrtl.c M cgraph.c M cgraph.h M cgraphbuild.c M cgraphclones.c M cgraphunit.c M combine.c M config/aarch64/aarch64.c M config/aarch64/cortex-a57-fma-steering.c M config/aarch64/falkor-tag-collision-avoidance.c M config/alpha/alpha.c M config/arm/arm.c M config/csky/csky.c M config/elfos.h M config/i386/i386-expand.c M config/i386/i386-features.c M config/i386/i386-options.c M config/i386/i386.c M config/i386/x86-tune-sched.c M config/ia64/ia64.c M config/m68k/m68k.c M config/m68k/m68k.md M config/microblaze/microblaze.c M config/mips/mips.c M config/nios2/nios2.c M config/pa/pa.c M config/pa/pa.md M config/rs6000/rs6000-call.c M config/rs6000/rs6000-logue.c M config/rs6000/rs6000-p8swap.c M config/rs6000/rs6000.c M config/rs6000/rs6000.md M config/s390/s390.c M config/sh/sh.c M config/sparc/sparc.c M config/xtensa/xtensa.c M configure M configure.ac M cp/call.c M cp/class.c M cp/constexpr.c M cp/constraint.cc M cp/cp-gimplify.c M cp/decl.c M cp/decl2.c M cp/error.c M cp/friend.c M cp/init.c M cp/method.c M cp/name-lookup.c M cp/parser.c M cp/pt.c M cp/rtti.c M cp/semantics.c M cp/typeck.c M cp/typeck2.c M cprop.c M cse.c M cselib.c M d/d-codegen.cc M d/decl.cc M d/dmd/doc.c M d/dmd/dscope.c M d/dmd/initsem.c M d/expr.cc M defaults.h M df-problems.c M df-scan.c M diagnostic-show-locus.c M doc/invoke.texi M dse.c M dumpfile.h M dwarf2cfi.c M dwarf2out.c M emit-rtl.c M expmed.c M expr.c M final.c M fold-const.c M fo
Re: Getting spurious FAILS in testsuite?
Hi,
I see this now as well on Ubuntu 16.04, but I doubt that the Kernel is
to blame.
I am able to reproduce this in debug-mode as follows:
strace -s 4096 -o strace.txt expect -- /usr/share/dejagnu/runtest.exp
--debug -v --tool gcc ubsan.exp=*
So I have now a dbg.out and a strace log file showing what's going on
when the bug happens.
One test case that reliably Fails is c-c++-common/ubsan/overflow-mul-4.c
strace sees the complete Output read in several 4K sized blocks, then a
half-full block
and then eof, so the complete message arrives but then a race condition
seems to happen.
so the following shows what happens while the spawn, two childs (test
case&/bin/cat) die but the fifo
(fileno 10) still has data to read out which works as expected but when
the EOF is found the last part
gets written to the log file (fileno 4), but is not used as test output.
fcntl(10, F_GETFL) = 0 (flags O_RDONLY)
fcntl(10, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
fcntl(10, F_GETFL) = 0x800 (flags O_RDONLY|O_NONBLOCK)
write(4, "spawn: returns {0}\r\n", 20) = 20
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16141,
si_uid=1000, si_status=0, si_utime=0, si_stime=0} ---
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16142,
si_uid=1000, si_status=0, si_utime=0, si_stime=0} ---
write(4, "Gate keeper glob pattern for '.+'", 33) = 33
write(4, " is ''. Not usable, disabling the", 33) = 33
write(4, " performance booster.\n", 22) = 22
write(4, "\r\nexpect: does \"", 16) = 16
write(4, "\" (spawn_id exp10) match regular expression ", 44) = 44
write(4, "\"", 1) = 1
write(4, ".+", 2) = 2
write(4, "\"? ", 3) = 3
write(4, "(No Gate, RE only) gate=", 24) = 24
write(4, "yes re=", 7) = 7
write(4, "no\r\n", 4) = 4
write(9, "\0", 1) = 1
futex(0x7d08cc, FUTEX_WAIT_BITSET_PRIVATE|FUTEX_CLOCK_REALTIME, 3029,
{1499793457, 287025000}, ) = 0
futex(0x7f2d85ae38e0, FUTEX_WAKE_PRIVATE, 1) = 0
read(10,
"/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:20:3:
runtime error: signed integer overflow: 1537228672809129302 * 6 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:21:3:
runtime error: signed integer overflow: -1537228672809129302 * -6 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:22:3:
runtime error: signed integer overflow: 1537228672809129302 * -6 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:23:3:
runtime error: signed integer overflow: -1537228672809129302 * 6 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:24:3:
runtime error: signed integer overflow: 2166572392 * 4257126175 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:25:3:
runtime error: signed integer overflow: -2166572392 * -4257126175 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:26:3:
runtime error: signed integer overflow: 2166572392 * -4257126175 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:27:3:
runtime error: signed integer overflow: -2166572392 * 4257126175 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:28:3:
runtime error: signed integer overflow: 1537228672809129301 * 7 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:29:3:
runtime error: signed integer overflow: -1537228672809129301 * -7 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:30:3:
runtime error: signed integer overflow: 1537228672809129301 * -7 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:31:3:
runtime error: signed integer overflow: -1537228672809129301 * 7 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:32:3:
runtime error: signed integer overflow: 2166572391 * 4257126176 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:33:3:
runtime error: signed integer overflow: -2166572391 * -4257126176 cannot
be represented in type 'long long
int'\n/home/ed/gnu/gcc-8-20170709/gcc/testsuite/c-c++-common/ubsan/overflow-mul-4.c:34:3:
runtime error: signed
Re: Getting spurious FAILS in testsuite?
On 07/11/17 21:42, Andrew Pinski wrote:
> On Tue, Jul 11, 2017 at 12:31 PM, Andrew Pinski wrote:
>> On Tue, Jul 11, 2017 at 12:27 PM, Andrew Pinski wrote:
>>> On Tue, Jul 11, 2017 at 12:15 PM, Bernd Edlinger
>>> wrote:
>>>> Hi,
>>>>
>>>> I see this now as well on Ubuntu 16.04, but I doubt that the Kernel is
>>>> to blame.
>>>
>>> I don't see these failures when I use a 4.11 kernel. Only with a 4.4
>>> kernel.
>>> Also the guality testsuite does not run at all with a 4.4 kernel, it
>>> does run when using a 4.11 kernel; I suspect this is the same symptom
>>> of the bug.
>>
>>
>> 4.11 kernel results (with CentOS 7.4 glibc):
>> https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg00998.html
>>
>> 4.4 kernel results (with ubuntu 1604 glibc):
>> https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg01009.html
>>
>> Note the glibc does not matter here as I get a similar testsuite
>> results as the CentOS 7.4 glibc using the Ubuntu 1604 glibc but with
>> the 4.11 kernel.
>>
>> Also note I get a similar results with a plain 4.11 kernel compared to
>> the above kernel.
>
>
> Maybe commit 6a7e6f78c235975cc14d4e141fa088afffe7062c or
> 0f40fbbcc34e093255a2b2d70b6b0fb48c3f39aa to the kernel fixes both
> issues.
>
As Georg-Johann points out here:
https://gcc.gnu.org/ml/gcc/2017-07/msg00028.html
updating the kernel alone does not fix the issue.
These patches do all circle around pty and tty devices,
but in the strace file I see a pipe(2) command
creating the fileno 10 and the sequence of events is
as follows:
pipe([7, 10]) = 0 (not in my previous e-mail)
clone() = 16141
clone() = 16142
write(4, "spawn: returns {0}\r\n", 20) = 20
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16141,
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16142,
read(10,...,4096) = 4096
read(10,...,4096) = 2144
read(10, "", 4096) = 0
close(10) = 0
wait4(16141, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16141
wait4(16142, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16142
All data arrive at the expect process, but apparently too quickly.
Thanks
Bernd.
Re: Getting spurious FAILS in testsuite?
On 07/11/17 22:28, Bernd Edlinger wrote:
> On 07/11/17 21:42, Andrew Pinski wrote:
>> On Tue, Jul 11, 2017 at 12:31 PM, Andrew Pinski
>> wrote:
>>> On Tue, Jul 11, 2017 at 12:27 PM, Andrew Pinski
>>> wrote:
>>>> On Tue, Jul 11, 2017 at 12:15 PM, Bernd Edlinger
>>>> wrote:
>>>>> Hi,
>>>>>
>>>>> I see this now as well on Ubuntu 16.04, but I doubt that the Kernel is
>>>>> to blame.
>>>>
>>>> I don't see these failures when I use a 4.11 kernel. Only with a
>>>> 4.4 kernel.
>>>> Also the guality testsuite does not run at all with a 4.4 kernel, it
>>>> does run when using a 4.11 kernel; I suspect this is the same symptom
>>>> of the bug.
>>>
>>>
>>> 4.11 kernel results (with CentOS 7.4 glibc):
>>> https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg00998.html
>>>
>>> 4.4 kernel results (with ubuntu 1604 glibc):
>>> https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg01009.html
>>>
>>> Note the glibc does not matter here as I get a similar testsuite
>>> results as the CentOS 7.4 glibc using the Ubuntu 1604 glibc but with
>>> the 4.11 kernel.
>>>
>>> Also note I get a similar results with a plain 4.11 kernel compared to
>>> the above kernel.
>>
>>
>> Maybe commit 6a7e6f78c235975cc14d4e141fa088afffe7062c or
>> 0f40fbbcc34e093255a2b2d70b6b0fb48c3f39aa to the kernel fixes both
>> issues.
>>
>
> As Georg-Johann points out here:
> https://gcc.gnu.org/ml/gcc/2017-07/msg00028.html
> updating the kernel alone does not fix the issue.
>
> These patches do all circle around pty and tty devices,
> but in the strace file I see a pipe(2) command
> creating the fileno 10 and the sequence of events is
> as follows:
>
> pipe([7, 10]) = 0 (not in my previous e-mail)
> clone() = 16141
> clone() = 16142
> write(4, "spawn: returns {0}\r\n", 20) = 20
> --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16141,
> --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16142,
> read(10,...,4096) = 4096
> read(10,...,4096) = 2144
> read(10, "", 4096) = 0
> close(10) = 0
> wait4(16141, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16141
> wait4(16142, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16142
>
> All data arrive at the expect process, but apparently too quickly.
>
>
> Thanks
> Bernd.
Oh, I think the true reason is this patch:
Author: Upstream
Description: Report and fix both by Nils Carlson.
Replaced a cc==0 check with proper Tcl_Eof() check.
Last-Modified: Fri, 23 Oct 2015 11:09:07 +0300
Bug: http://sourceforge.net/p/expect/patches/18/
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=799301
--- a/expect.c
+++ b/expect.c
@@ -1860,19 +1860,11 @@
if (cc == EXP_DATA_NEW) {
/* try to read it */
cc = expIRead(interp,esPtr,timeout,tcl_set_flags);
-
- /* the meaning of 0 from i_read means eof. Muck with it a */
- /* little, so that from now on it means "no new data arrived */
- /* but it should be looked at again anyway". */
- if (cc == 0) {
+
+ if (Tcl_Eof(esPtr->channel)) {
cc = EXP_EOF;
- } else if (cc > 0) {
- /* successfully read data */
- } else {
- /* failed to read data - some sort of error was encountered such as
-* an interrupt with that forced an error return
-*/
}
+
} else if (cc == EXP_DATA_OLD) {
cc = 0;
} else if (cc == EXP_RECONFIGURE) {
The correct way to fix this would be something like this:
if (cc == 0 && Tcl_Eof(esPtr->channel)) {
cc = EXP_EOF;
}
What happens for me is cc > 0 AND Tcl_Eof at the same time.
That makes dejagnu ignore the last few lines, because it does
not expect EOF and data at the same time.
Apparently tcl starts to read ahead because the default match
buffer size is unreasonably small like 2000 bytes.
I can work around it by increasing the buffer size like this:
ndex: gcc/testsuite/lib/gcc-dg.exp
===
--- gcc/testsuite/lib/gcc-dg.exp(revision 250150)
+++ gcc/testsuite/lib/gcc-dg.exp(working copy)
@@ -43,6 +43,9 @@
setenv LANG C.ASCII
}
+# Avoid sporadic data-losses with expect
+match_max -d 1
+
# Ensure GCC_COLORS is unset, for the rare testcases that verify
# how output is colorized.
if [info exists ::env(GCC_COLORS) ] {
What do you think?
Can debian/ubuntu people reproduce and fix this?
Should we increase the match buffer size on the gcc side?
Thanks
Bernd.
Re: Getting spurious FAILS in testsuite?
On 07/13/17 16:31, Georg-Johann Lay wrote:
> On 12.07.2017 15:40, Bernd Edlinger wrote:
>> On 07/11/17 22:28, Bernd Edlinger wrote:
>>> On 07/11/17 21:42, Andrew Pinski wrote:
>>>> On Tue, Jul 11, 2017 at 12:31 PM, Andrew Pinski
>>>> wrote:
>>>>> On Tue, Jul 11, 2017 at 12:27 PM, Andrew Pinski
>>>>> wrote:
>>>>>> On Tue, Jul 11, 2017 at 12:15 PM, Bernd Edlinger
>>>>>> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I see this now as well on Ubuntu 16.04, but I doubt that the
>>>>>>> Kernel is
>>>>>>> to blame.
>>>>>>
>>>>>> I don't see these failures when I use a 4.11 kernel. Only with a
>>>>>> 4.4 kernel.
>>>>>> Also the guality testsuite does not run at all with a 4.4 kernel, it
>>>>>> does run when using a 4.11 kernel; I suspect this is the same symptom
>>>>>> of the bug.
>>>>>
>>>>>
>>>>> 4.11 kernel results (with CentOS 7.4 glibc):
>>>>> https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg00998.html
>>>>>
>>>>> 4.4 kernel results (with ubuntu 1604 glibc):
>>>>> https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg01009.html
>>>>>
>>>>> Note the glibc does not matter here as I get a similar testsuite
>>>>> results as the CentOS 7.4 glibc using the Ubuntu 1604 glibc but with
>>>>> the 4.11 kernel.
>>>>>
>>>>> Also note I get a similar results with a plain 4.11 kernel compared to
>>>>> the above kernel.
>>>>
>>>>
>>>> Maybe commit 6a7e6f78c235975cc14d4e141fa088afffe7062c or
>>>> 0f40fbbcc34e093255a2b2d70b6b0fb48c3f39aa to the kernel fixes both
>>>> issues.
>>>>
>>>
>>> As Georg-Johann points out here:
>>> https://gcc.gnu.org/ml/gcc/2017-07/msg00028.html
>>> updating the kernel alone does not fix the issue.
>>>
>>> These patches do all circle around pty and tty devices,
>>> but in the strace file I see a pipe(2) command
>>> creating the fileno 10 and the sequence of events is
>>> as follows:
>>>
>>> pipe([7, 10]) = 0 (not in my previous e-mail)
>>> clone() = 16141
>>> clone() = 16142
>>> write(4, "spawn: returns {0}\r\n", 20) = 20
>>> --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16141,
>>> --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16142,
>>> read(10,...,4096) = 4096
>>> read(10,...,4096) = 2144
>>> read(10, "", 4096) = 0
>>> close(10) = 0
>>> wait4(16141, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16141
>>> wait4(16142, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16142
>>>
>>> All data arrive at the expect process, but apparently too quickly.
>>>
>>>
>>> Thanks
>>> Bernd.
>>
>>
>>
>> Oh, I think the true reason is this patch:
>> Author: Upstream
>> Description: Report and fix both by Nils Carlson.
>>Replaced a cc==0 check with proper Tcl_Eof() check.
>> Last-Modified: Fri, 23 Oct 2015 11:09:07 +0300
>> Bug: http://sourceforge.net/p/expect/patches/18/
>> Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=799301
>>
>> --- a/expect.c
>> +++ b/expect.c
>> @@ -1860,19 +1860,11 @@
>>if (cc == EXP_DATA_NEW) {
>>/* try to read it */
>>cc = expIRead(interp,esPtr,timeout,tcl_set_flags);
>> -
>> -/* the meaning of 0 from i_read means eof. Muck with it a */
>> -/* little, so that from now on it means "no new data arrived */
>> -/* but it should be looked at again anyway". */
>> -if (cc == 0) {
>> +
>> +if (Tcl_Eof(esPtr->channel)) {
>>cc = EXP_EOF;
>> -} else if (cc > 0) {
>> -/* successfully read data */
>> -} else {
>> -/* failed to read data - some sort of error was encountered
>> such as
>> - * an interrupt with that forced an error return
>> - */
>>}
>> +
>>} else if (cc == EXP_DATA_OLD) {
>>cc = 0;
>>} else if (cc == EXP_RECONFIGURE) {
>>
>>
>> The correct way to fix this would be something like this:
>>
>> if (cc == 0 && Tcl_Eof(esPtr->channel)) {
>>
Re: Getting spurious FAILS in testsuite?
On 07/14/17 13:03, Georg-Johann Lay wrote:
> On 13.07.2017 18:47, Bernd Edlinger wrote:
>> On 07/13/17 16:31, Georg-Johann Lay wrote:
>>> On 12.07.2017 15:40, Bernd Edlinger wrote:
>>>> On 07/11/17 22:28, Bernd Edlinger wrote:
>>>>> On 07/11/17 21:42, Andrew Pinski wrote:
>>>>>> On Tue, Jul 11, 2017 at 12:31 PM, Andrew Pinski
>>>>>> wrote:
>>>>>>> On Tue, Jul 11, 2017 at 12:27 PM, Andrew Pinski
>>>>>>> wrote:
>>>>>>>> On Tue, Jul 11, 2017 at 12:15 PM, Bernd Edlinger
>>>>>>>> wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I see this now as well on Ubuntu 16.04, but I doubt that the
>>>>>>>>> Kernel is
>>>>>>>>> to blame.
>>>>>>>>
>>>>>>>> I don't see these failures when I use a 4.11 kernel. Only with a
>>>>>>>> 4.4 kernel.
>>>>>>>> Also the guality testsuite does not run at all with a 4.4
>>>>>>>> kernel, it
>>>>>>>> does run when using a 4.11 kernel; I suspect this is the same
>>>>>>>> symptom
>>>>>>>> of the bug.
>>>>>>>
>>>>>>>
>>>>>>> 4.11 kernel results (with CentOS 7.4 glibc):
>>>>>>> https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg00998.html
>>>>>>>
>>>>>>> 4.4 kernel results (with ubuntu 1604 glibc):
>>>>>>> https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg01009.html
>>>>>>>
>>>>>>> Note the glibc does not matter here as I get a similar testsuite
>>>>>>> results as the CentOS 7.4 glibc using the Ubuntu 1604 glibc but with
>>>>>>> the 4.11 kernel.
>>>>>>>
>>>>>>> Also note I get a similar results with a plain 4.11 kernel
>>>>>>> compared to
>>>>>>> the above kernel.
>>>>>>
>>>>>>
>>>>>> Maybe commit 6a7e6f78c235975cc14d4e141fa088afffe7062c or
>>>>>> 0f40fbbcc34e093255a2b2d70b6b0fb48c3f39aa to the kernel fixes both
>>>>>> issues.
>>>>>>
>>>>>
>>>>> As Georg-Johann points out here:
>>>>> https://gcc.gnu.org/ml/gcc/2017-07/msg00028.html
>>>>> updating the kernel alone does not fix the issue.
>>>>>
>>>>> These patches do all circle around pty and tty devices,
>>>>> but in the strace file I see a pipe(2) command
>>>>> creating the fileno 10 and the sequence of events is
>>>>> as follows:
>>>>>
>>>>> pipe([7, 10]) = 0 (not in my previous e-mail)
>>>>> clone() = 16141
>>>>> clone() = 16142
>>>>> write(4, "spawn: returns {0}\r\n", 20) = 20
>>>>> --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16141,
>>>>> --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16142,
>>>>> read(10,...,4096) = 4096
>>>>> read(10,...,4096) = 2144
>>>>> read(10, "", 4096) = 0
>>>>> close(10) = 0
>>>>> wait4(16141, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16141
>>>>> wait4(16142, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16142
>>>>>
>>>>> All data arrive at the expect process, but apparently too quickly.
>>>>>
>>>>>
>>>>> Thanks
>>>>> Bernd.
>>>>
>>>>
>>>>
>>>> Oh, I think the true reason is this patch:
>>>> Author: Upstream
>>>> Description: Report and fix both by Nils Carlson.
>>>> Replaced a cc==0 check with proper Tcl_Eof() check.
>>>> Last-Modified: Fri, 23 Oct 2015 11:09:07 +0300
>>>> Bug: http://sourceforge.net/p/expect/patches/18/
>>>> Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=799301
>>>>
>>>> --- a/expect.c
>>>> +++ b/expect.c
>>>> @@ -1860,19 +1860,11 @@
>>>> if (cc == EXP_DATA_NEW) {
>>>> /* try to read it */
>>>> cc = expIRead(interp,esPtr,timeout,tcl_set_flags);
>>>> -
>>>> -/* the meaning of 0 from i_read means eof. Muck with it a */
>>&
Re: Inefficient code
You can get much better code if you make xrci a bit field.
so the entire bit filed region can be accessed word-wise:
#include
struct Xrb
{
uint16_t xrlen; /* Length of I/O buffer in bytes */
uint16_t xrbc; /* Byte count for transfer */
void * xrloc; /* Pointer to I/O buffer */
uint8_t xrci:8; /* Channel number times 2 for transfer */
uint32_t xrblk:24; /* Random access block number */
uint16_t xrtime;/* Wait time for terminal input */
uint16_t xrmod; /* Modifiers */
};
void test(struct Xrb *XRB)
{
XRB->xrblk = 5;
}
Bernd.
Re: gcc 11.1.0 mpfr
On 5/14/21 10:20 PM, Andrew Pinski via Gcc wrote: > On Fri, May 14, 2021 at 3:27 AM Richard Biener via Gcc > wrote: >> >> On May 14, 2021 10:53:21 AM GMT+02:00, "Martin Liška" wrote: >>> On 5/12/21 10:51 AM, Richard Biener via Gcc wrote: On Tue, May 11, 2021 at 6:34 PM Serge Belyshev via Gcc >>> wrote: > > >> $ egrep "mpfr\.h" log/cfg/cfg.gcc-11.1.0.log >> checking for the correct version of mpfr.h... buggy but acceptable >> >> It appears "gcc-11.1.0/contrib/download_prerequisites" >> specifies "mpfr-3.1.4.tar.bz2" whereas top level 'configure' >> has "MPFR_VERSION_NUM(3,1,6)". >> >> Is there a reason mpfr 3.1.6 isn't being used? > > No good reason: that script was not updated with new versions. >>> GCC-11 is > also known to work fine with the most recent mpfr version 4.1.0. Yes, the update of the minimum version and the buggy check was done by Janne w/o adjusting the download script. Richard. >>> >>> Should I adjust download_prerequisites accordingly? >> >> Yes. > > It looks like the adjustment was made but the file was not added uploaded: > [apinski@xeond2 gcc]$ ./contrib/download_prerequisites > 2021-05-14 13:18:41 > URL:http://gcc.gnu.org/pub/gcc/infrastructure/gmp-6.1.0.tar.bz2 > [2383840/2383840] -> "gmp-6.1.0.tar.bz2" [1] > http://gcc.gnu.org/pub/gcc/infrastructure/mpfr-4.1.0.tar.bz2: > 2021-05-14 13:18:42 ERROR 404: Not Found. > error: Cannot download mpfr-4.1.0.tar.bz2 from > http://gcc.gnu.org/pub/gcc/infrastructure/ > Yes, indeed, and please do not forget to update also the hashes in contrib/prerequisites.md5 and contrib/prerequisites.sha512 . However, I wonder why we change only the default mpfr version to latest available version but keep gmp and mpc versions from 2015? Thanks Bernd. > > Thanks, > Andrew Pinski > > >> >> Richard. >> >>> Martin >> >
RE: Still fails with strict-volatile-bitfields
Hi,
On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote:
>
> Sandra, Bernd,
>
> Can you take a look at
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734
>
> It seems a siimple case still doesn't work as expected. Did I miss anything?
>
> Thanks,
> Joey
Yes,
this is a major case where the C++ memory model is
in conflict with AAPCS.
You can get the expected code, by changing the struct
like this:
struct str {
volatile unsigned f1: 8;
unsigned dummy:24;
};
If it is written this way the C++ memory model allows
QImode, HImode, SImode. And -fstrict-volatile-bitfields
demands SImode, so the conflict is resolved. This dummy
member makes only a difference on the C level, and is
completely invisible in the generated code.
If -fstrict-volatile-bitfields is now given, we use SImode,
if -fno-strict-volatile-bitfields is given, we give GCC the
freedom to choose the access mode, maybe QImode if that is
faster.
In the _very_ difficult process to find an solution
that seems to be acceptable to all maintainers, we came to
the solution, that we need to adhere to the C++ memory
model by default. And we may not change the default
setting of -fstruct-volatile-bitfields at the same time!
As a future extension we discussed the possibility
to add a new option -fstrict-volatile-bitfields=aapcs
that explicitly allows us to break the C++ memory model.
But I did not yet try to implement this, as I feel that
would certainly not be accepted as we are in Phase3 now.
As another future extension there was the discussion
about the -Wportable-volatility warning, which I see now
as a warning that analyzes the structure layout and
warns about any structures that are not "well-formed",
in the sense, that a bit-field fails to define all
bits of the container.
Those people that do use bit-fields to access device-registers
do always define all bits, and of course in the same mode.
It would be good to have a warning, when some bits are missing.
They currently have to use great care to check their structures
manually.
I had a proposal for that warning but that concentrated
only on the volatile attribute, but I will have to re-write
that completely so that it can be done in stor-layout.c:
It should warn independent of optimization levels or actual
bitfield member references, thus, be implemented entirely at
the time we lay out the structure. The well-formed-ness of
a bit-field makes that possible.
But that will come too late for Phase3 as well.
Regards
Bernd.
RE: Still fails with strict-volatile-bitfields
On Thu, 9 Jan 2014 16:22:33, Richard Earnshaw wrote:
>
> On 09/01/14 08:26, Bernd Edlinger wrote:
>> Hi,
>>
>> On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote:
>>>
>>> Sandra, Bernd,
>>>
>>> Can you take a look at
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734
>>>
>>> It seems a siimple case still doesn't work as expected. Did I miss anything?
>>>
>>> Thanks,
>>> Joey
>>
>> Yes,
>>
>> this is a major case where the C++ memory model is
>> in conflict with AAPCS.
>>
>
> Does the compiler warn about this? And if so, is the warning on by
> default? I think it's essential that we don't have quiet changes in
> behaviour without such warnings.
>
> R.
No. This example was working in 4.6 and broken in 4.7 and 4.8.
Well, 4.7 should have warned about that.
4.9 does simply not change that, because it would violate the C++
memory model. Maybe that should go into release notes.
At the same time we had all kinds of invalid code generation,
starting at 4.6, especially with packed structures in C and Ada(!),
(writing not all bits, and using unaligned accesses)
and that is fixed in 4.9.
Bernd.
>
>> You can get the expected code, by changing the struct
>> like this:
>>
>> struct str {
>> volatile unsigned f1: 8;
>> unsigned dummy:24;
>> };
>>
>> If it is written this way the C++ memory model allows
>> QImode, HImode, SImode. And -fstrict-volatile-bitfields
>> demands SImode, so the conflict is resolved. This dummy
>> member makes only a difference on the C level, and is
>> completely invisible in the generated code.
>>
>> If -fstrict-volatile-bitfields is now given, we use SImode,
>> if -fno-strict-volatile-bitfields is given, we give GCC the
>> freedom to choose the access mode, maybe QImode if that is
>> faster.
>>
>> In the _very_ difficult process to find an solution
>> that seems to be acceptable to all maintainers, we came to
>> the solution, that we need to adhere to the C++ memory
>> model by default. And we may not change the default
>> setting of -fstruct-volatile-bitfields at the same time!
>>
>> As a future extension we discussed the possibility
>> to add a new option -fstrict-volatile-bitfields=aapcs
>> that explicitly allows us to break the C++ memory model.
>>
>> But I did not yet try to implement this, as I feel that
>> would certainly not be accepted as we are in Phase3 now.
>>
>> As another future extension there was the discussion
>> about the -Wportable-volatility warning, which I see now
>> as a warning that analyzes the structure layout and
>> warns about any structures that are not "well-formed",
>> in the sense, that a bit-field fails to define all
>> bits of the container.
>>
>> Those people that do use bit-fields to access device-registers
>> do always define all bits, and of course in the same mode.
>>
>> It would be good to have a warning, when some bits are missing.
>> They currently have to use great care to check their structures
>> manually.
>>
>> I had a proposal for that warning but that concentrated
>> only on the volatile attribute, but I will have to re-write
>> that completely so that it can be done in stor-layout.c:
>>
>> It should warn independent of optimization levels or actual
>> bitfield member references, thus, be implemented entirely at
>> the time we lay out the structure. The well-formed-ness of
>> a bit-field makes that possible.
>>
>> But that will come too late for Phase3 as well.
>>
>>
>> Regards
>> Bernd.
>>
>>
>
>
RE: Still fails with strict-volatile-bitfields
On, Fri, 10 Jan 2014 09:41:06, Richard Earnshaw wrote:
>
> On 10/01/14 08:49, Bernd Edlinger wrote:
>> On Thu, 9 Jan 2014 16:22:33, Richard Earnshaw wrote:
>>>
>>> On 09/01/14 08:26, Bernd Edlinger wrote:
>>>> Hi,
>>>>
>>>> On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote:
>>>>>
>>>>> Sandra, Bernd,
>>>>>
>>>>> Can you take a look at
>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734
>>>>>
>>>>> It seems a siimple case still doesn't work as expected. Did I miss
>>>>> anything?
>>>>>
>>>>> Thanks,
>>>>> Joey
>>>>
>>>> Yes,
>>>>
>>>> this is a major case where the C++ memory model is
>>>> in conflict with AAPCS.
>>>>
>>>
>>> Does the compiler warn about this? And if so, is the warning on by
>>> default? I think it's essential that we don't have quiet changes in
>>> behaviour without such warnings.
>>>
>>> R.
>>
>> No. This example was working in 4.6 and broken in 4.7 and 4.8.
>> Well, 4.7 should have warned about that.
>>
>> 4.9 does simply not change that, because it would violate the C++
>> memory model. Maybe that should go into release notes.
>
> That's no excuse for not generating a warning at compile time when the
> situation is encountered. Users of this feature are experiencing a
> quiet change of behaviour and that's unacceptable, even if the compiler
> is doing what it was intended to do; that doesn't necessarily match the
> user's expectations. There should always be a way to rewrite the C11
> intended semantics in a way that doesn't violate the strict volatile
> bitfields semantics.
>
Hmm...
You mean we should have a (ugly) warning enabled by default in 4.9 (at expmed.c)
if a bit-field access would be perfectly aligned and so, but is in conflict
with the
C++ memory model, and -fstrict-volatile-bitfields is in effect.
Maybe only once per compilation?
>>
>> At the same time we had all kinds of invalid code generation,
>> starting at 4.6, especially with packed structures in C and Ada(!),
>> (writing not all bits, and using unaligned accesses)
>> and that is fixed in 4.9.
>>
>
> I'm not worried about packed structures. You can't sensibly implement
> the strict volatile bitfields rules when things aren't aligned.
>
> R.
>
>>
>> Bernd.
>>
>>>
>>>> You can get the expected code, by changing the struct
>>>> like this:
>>>>
>>>> struct str {
>>>> volatile unsigned f1: 8;
>>>> unsigned dummy:24;
>>>> };
>>>>
>>>> If it is written this way the C++ memory model allows
>>>> QImode, HImode, SImode. And -fstrict-volatile-bitfields
>>>> demands SImode, so the conflict is resolved. This dummy
>>>> member makes only a difference on the C level, and is
>>>> completely invisible in the generated code.
>>>>
>>>> If -fstrict-volatile-bitfields is now given, we use SImode,
>>>> if -fno-strict-volatile-bitfields is given, we give GCC the
>>>> freedom to choose the access mode, maybe QImode if that is
>>>> faster.
>>>>
>>>> In the _very_ difficult process to find an solution
>>>> that seems to be acceptable to all maintainers, we came to
>>>> the solution, that we need to adhere to the C++ memory
>>>> model by default. And we may not change the default
>>>> setting of -fstruct-volatile-bitfields at the same time!
>>>>
>>>> As a future extension we discussed the possibility
>>>> to add a new option -fstrict-volatile-bitfields=aapcs
>>>> that explicitly allows us to break the C++ memory model.
>>>>
>>>> But I did not yet try to implement this, as I feel that
>>>> would certainly not be accepted as we are in Phase3 now.
>>>>
>>>> As another future extension there was the discussion
>>>> about the -Wportable-volatility warning, which I see now
>>>> as a warning that analyzes the structure layout and
>>>> warns about any structures that are not "well-formed",
>>>> in the sense, that a bit-field fails to define all
>>>> bits of the container.
>>>>
>>>> Those people that do use bit-fields to access device-registers
>>>> do always define all bits, and of course in the same mode.
>>>>
>>>> It would be good to have a warning, when some bits are missing.
>>>> They currently have to use great care to check their structures
>>>> manually.
>>>>
>>>> I had a proposal for that warning but that concentrated
>>>> only on the volatile attribute, but I will have to re-write
>>>> that completely so that it can be done in stor-layout.c:
>>>>
>>>> It should warn independent of optimization levels or actual
>>>> bitfield member references, thus, be implemented entirely at
>>>> the time we lay out the structure. The well-formed-ness of
>>>> a bit-field makes that possible.
>>>>
>>>> But that will come too late for Phase3 as well.
>>>>
>>>>
>>>> Regards
>>>> Bernd.
>>>>
>>>>
>>>
>>>
>>
>
>
RE: Still fails with strict-volatile-bitfields
On Fri, 10 Jan 2014 14:45:12, Richard Biener wrote:
>
> On Fri, Jan 10, 2014 at 2:30 PM, Bernd Edlinger
> wrote:
>> On, Fri, 10 Jan 2014 09:41:06, Richard Earnshaw wrote:
>>>
>>> On 10/01/14 08:49, Bernd Edlinger wrote:
>>>> On Thu, 9 Jan 2014 16:22:33, Richard Earnshaw wrote:
>>>>>
>>>>> On 09/01/14 08:26, Bernd Edlinger wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote:
>>>>>>>
>>>>>>> Sandra, Bernd,
>>>>>>>
>>>>>>> Can you take a look at
>>>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734
>>>>>>>
>>>>>>> It seems a siimple case still doesn't work as expected. Did I miss
>>>>>>> anything?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Joey
>>>>>>
>>>>>> Yes,
>>>>>>
>>>>>> this is a major case where the C++ memory model is
>>>>>> in conflict with AAPCS.
>>>>>>
>>>>>
>>>>> Does the compiler warn about this? And if so, is the warning on by
>>>>> default? I think it's essential that we don't have quiet changes in
>>>>> behaviour without such warnings.
>>>>>
>>>>> R.
>>>>
>>>> No. This example was working in 4.6 and broken in 4.7 and 4.8.
>>>> Well, 4.7 should have warned about that.
>>>>
>>>> 4.9 does simply not change that, because it would violate the C++
>>>> memory model. Maybe that should go into release notes.
>>>
>>> That's no excuse for not generating a warning at compile time when the
>>> situation is encountered. Users of this feature are experiencing a
>>> quiet change of behaviour and that's unacceptable, even if the compiler
>>> is doing what it was intended to do; that doesn't necessarily match the
>>> user's expectations. There should always be a way to rewrite the C11
>>> intended semantics in a way that doesn't violate the strict volatile
>>> bitfields semantics.
>>>
>>
>> Hmm...
>> You mean we should have a (ugly) warning enabled by default in 4.9 (at
>> expmed.c)
>> if a bit-field access would be perfectly aligned and so, but is in conflict
>> with the
>> C++ memory model, and -fstrict-volatile-bitfields is in effect.
>> Maybe only once per compilation?
>
> I'd say you want a warning for the structure declaration instead
> "Accesses to XYZ will not follow AACPS due to C++ memory model
> constraints".
>
> Richard.
>
Yes, that would be the way how we wanted to implement the
-Wportable-volatility warning, except that it would be on by default
if -fstrict-volatile-bitfields is in effect.
And it would not only warn if the member is declared volatile,
because the structure can be declared volatile later.
Except that I did not even start to implement it that way, that
would be quite late for 4.9 now?
Bernd.
>>
>>>>
>>>> At the same time we had all kinds of invalid code generation,
>>>> starting at 4.6, especially with packed structures in C and Ada(!),
>>>> (writing not all bits, and using unaligned accesses)
>>>> and that is fixed in 4.9.
>>>>
>>>
>>> I'm not worried about packed structures. You can't sensibly implement
>>> the strict volatile bitfields rules when things aren't aligned.
>>>
>>> R.
>>>
>>>>
>>>> Bernd.
>>>>
>>>>>
>>>>>> You can get the expected code, by changing the struct
>>>>>> like this:
>>>>>>
>>>>>> struct str {
>>>>>> volatile unsigned f1: 8;
>>>>>> unsigned dummy:24;
>>>>>> };
>>>>>>
>>>>>> If it is written this way the C++ memory model allows
>>>>>> QImode, HImode, SImode. And -fstrict-volatile-bitfields
>>>>>> demands SImode, so the conflict is resolved. This dummy
>>>>>> member makes only a difference on the C level, and is
>>>>>> completely invisible in the generated code.
>>>>>>
>>>>>> If -fstrict-volatile-bitfields is now given, we use SImode,
>>>>>> if -fno-strict-volatile-bitfields is given, we give GCC the
>>>>>> freedom
RE: Still fails with strict-volatile-bitfields
Hi, On Mon, 13 Jan 2014 10:26:03, Joey Ye wrote: > > Bernd, > > If that's the case, can you please firstly fix invoke.texi where the > behavior of strict-volatile-bitfields is described? At least my > interpretation of current doc doesn't explain the behavior of the case > we are discussing. Also it should be a generic definition rather than > target specific one. > Yes, if nobody objects I'd add a note like this to the documentation regarding the -fstrict-volatile-bitfields: Index: invoke.texi === --- invoke.texi (revision 207223) +++ invoke.texi (working copy) @@ -22456,6 +22456,10 @@ case GCC falls back to generating multiple accesses rather than code that will fault or truncate the result at run time. +Note: Due to restrictions of the C/C++11 memory model, write accesses are +not allowed to touch non bit-field members. It is therefore recommended +to define all bits of the field's type as bit-field members. + The default value of this option is determined by the application binary interface for the target processor. Thanks Bernd. > A related issue is that how would we expect users to build their > original code with volatile bitfields usage? From the approach I > understand they have to explicitly add -fstrict-volatile-bitfields for > 4.9 (by default it is disabled now), and probably > -fstrict-volatile-bitfields=aapcs (to allow C++ memory model conflict) > later. Neither of them are obvious to users. How about a configure > option to set default? > > Thanks, > Joey > > On Fri, Jan 10, 2014 at 10:20 PM, Bernd Edlinger > wrote: >> On Fri, 10 Jan 2014 14:45:12, Richard Biener wrote: >>> >>> On Fri, Jan 10, 2014 at 2:30 PM, Bernd Edlinger >>> wrote: >>>> On, Fri, 10 Jan 2014 09:41:06, Richard Earnshaw wrote: >>>>> >>>>> On 10/01/14 08:49, Bernd Edlinger wrote: >>>>>> On Thu, 9 Jan 2014 16:22:33, Richard Earnshaw wrote: >>>>>>> >>>>>>> On 09/01/14 08:26, Bernd Edlinger wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> On Thu, 9 Jan 2014 15:01:54, Yoey Ye wrote: >>>>>>>>> >>>>>>>>> Sandra, Bernd, >>>>>>>>> >>>>>>>>> Can you take a look at >>>>>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59734 >>>>>>>>> >>>>>>>>> It seems a siimple case still doesn't work as expected. Did I miss >>>>>>>>> anything? >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Joey >>>>>>>> >>>>>>>> Yes, >>>>>>>> >>>>>>>> this is a major case where the C++ memory model is >>>>>>>> in conflict with AAPCS. >>>>>>>> >>>>>>> >>>>>>> Does the compiler warn about this? And if so, is the warning on by >>>>>>> default? I think it's essential that we don't have quiet changes in >>>>>>> behaviour without such warnings. >>>>>>> >>>>>>> R. >>>>>> >>>>>> No. This example was working in 4.6 and broken in 4.7 and 4.8. >>>>>> Well, 4.7 should have warned about that. >>>>>> >>>>>> 4.9 does simply not change that, because it would violate the C++ >>>>>> memory model. Maybe that should go into release notes. >>>>> >>>>> That's no excuse for not generating a warning at compile time when the >>>>> situation is encountered. Users of this feature are experiencing a >>>>> quiet change of behaviour and that's unacceptable, even if the compiler >>>>> is doing what it was intended to do; that doesn't necessarily match the >>>>> user's expectations. There should always be a way to rewrite the C11 >>>>> intended semantics in a way that doesn't violate the strict volatile >>>>> bitfields semantics. >>>>> >>>> >>>> Hmm... >>>> You mean we should have a (ugly) warning enabled by default in 4.9 (at >>>> expmed.c) >>>> if a bit-field access would be perfectly aligned and so, but is in >>>> conflict with the >>>> C++ memory model, and -fstrict-volatile-bitfields is in effect. >>>> Maybe only
