Office Hours for the GNU Toolchain on 2024-07-25 at 11am EST5EDT.

2024-07-24 Thread Carlos O'Donell via Gcc
Office Hours for the GNU Toolchain on 2024-07-25 at 11am EST5EDT.

Agenda:
* https://gcc.gnu.org/wiki/OfficeHours#Next

Meeting Link:
* https://bbb.linuxfoundation.org/room/adm-xcb-for-sk6

--
Cheers,
Carlos.



Office Hours for the GNU Toolchain on 2024-06-27 at 11am EST5EDT.

2024-06-24 Thread Carlos O'Donell via Gcc
Office Hours for the GNU Toolchain on 2024-06-27 at 11am EST5EDT.

Agenda:
* https://gcc.gnu.org/wiki/OfficeHours#Next

Meeting Link:
* https://bbb.linuxfoundation.org/room/adm-xcb-for-sk6

--
Cheers,
Carlos.



Office Hours for the GNU Toolchain on 2024-05-30 at 11am EST5EDT.

2024-05-22 Thread Carlos O'Donell via Gcc
Office Hours for the GNU Toolchain on 2024-05-30 at 11am EST5EDT.

Agenda:
* https://gcc.gnu.org/wiki/OfficeHours#Next

Meeting Link:
* https://bbb.linuxfoundation.org/room/adm-xcb-for-sk6

--
Cheers,
Carlos.



Re: [PATCH] wwwdocs: contribute.html: Update consensus on patch content.

2024-05-01 Thread Carlos O'Donell
On 4/25/24 14:32, Richard Biener wrote:
> 
> 
>> Am 25.04.2024 um 17:44 schrieb Carlos O'Donell :
>>
>> Discussion is here:
>> https://inbox.sourceware.org/gcc/CAPS5khZeWkAD=v8ka9g5eecdnk3bdhfnzjumpvc+hedmkvj...@mail.gmail.com/
>>
>> Rough consensus from Jakub Jelinek, Richard Biener and others is
>> that maintainers are for the change.
> 
> Ok

Pushed.

Thanks for helping me move this forward.

I look forward to pre-commit CI across all the projects.

Helping Linaro make forward progress here is my primary goal.

-- 
Cheers,
Carlos.



gcc-wwwdocs branch master updated. 3e9f71f2c5ae175b2a4b209156f2241fa0646381

2024-05-01 Thread Carlos O'Donell via Gcc-cvs-wwwdocs
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gcc-wwwdocs".

The branch, master has been updated
   via  3e9f71f2c5ae175b2a4b209156f2241fa0646381 (commit)
  from  b4efd5049c4fee5c541f4116db064958e5391bba (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -
commit 3e9f71f2c5ae175b2a4b209156f2241fa0646381
Author: Carlos O'Donell 
Date:   Thu Apr 25 11:29:02 2024 -0400

wwwdocs: contribute.html: Update consensus on patch content.

Discussion is here:

https://inbox.sourceware.org/gcc/CAPS5khZeWkAD=v8ka9g5eecdnk3bdhfnzjumpvc+hedmkvj...@mail.gmail.com/

Rough consensus from Jakub Jelinek, Richard Biener and others is
that maintainers are for the change.

This changes the contribution notes to allow it.

diff --git a/htdocs/contribute.html b/htdocs/contribute.html
index 7c1ae323..e8137edc 100644
--- a/htdocs/contribute.html
+++ b/htdocs/contribute.html
@@ -195,8 +195,9 @@ of your testing.
 
 The patch itself
 
-Do not include generated files as part of the patch, just mention
-them in the ChangeLog (e.g., "* configure: Regenerate."). 
+The patch should include everything you are changing (including
+regenerated files which should be noted in the ChangeLog e.g.
+"* configure: Regenerate.").
 
 
 

---

Summary of changes:
 htdocs/contribute.html | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


hooks/post-receive
-- 
gcc-wwwdocs


[PATCH] wwwdocs: contribute.html: Update consensus on patch content.

2024-04-25 Thread Carlos O'Donell
Discussion is here:
https://inbox.sourceware.org/gcc/CAPS5khZeWkAD=v8ka9g5eecdnk3bdhfnzjumpvc+hedmkvj...@mail.gmail.com/

Rough consensus from Jakub Jelinek, Richard Biener and others is
that maintainers are for the change.

This changes the contribution notes to allow it.
---
 htdocs/contribute.html | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/htdocs/contribute.html b/htdocs/contribute.html
index 7c1ae323..e8137edc 100644
--- a/htdocs/contribute.html
+++ b/htdocs/contribute.html
@@ -195,8 +195,9 @@ of your testing.
 
 The patch itself
 
-Do not include generated files as part of the patch, just mention
-them in the ChangeLog (e.g., "* configure: Regenerate."). 
+The patch should include everything you are changing (including
+regenerated files which should be noted in the ChangeLog e.g.
+"* configure: Regenerate.").
 
 
 
-- 
2.44.0



Invitation: Office Hours for the GNU Toolchain @ Thu May 30, 2024 11am - 12pm (EDT) (gcc@gcc.gnu.org)

2024-04-17 Thread Carlos O'Donell via Gcc

Office Hours for the GNU Toolchain
Thursday May 30, 2024 ⋅ 11am – 12pm
Eastern Time - New York

Location
https://bbb.linuxfoundation.org/room/adm-xcb-for-sk6
https://www.google.com/url?q=https%3A%2F%2Fbbb.linuxfoundation.org%2Froom%2Fadm-xcb-for-sk6=D=17137887=AOvVaw2V18NEONPlH-coE-TT_R5q



Office hours for the GNU  
Toolchain:https://gcc.gnu.org/wiki/OfficeHoursMeeting  
link:https://bbb.linuxfoundation.org/room/adm-xcb-for-sk6


Organizer
Carlos O'Donell
codon...@redhat.com

Guests
Carlos O'Donell - organizer
Nick Clifton
Jakub Jelinek
David Edelsohn
Siddhesh Poyarekar
Jason Merrill
j...@polyomino.org.uk
richard.earns...@arm.com
girish...@gmail.com
adhemerval.zane...@linaro.org
Florian Weimer
christophe.l...@linaro.org
berg...@linux.ibm.com
Peter Bergner
libc-al...@sourceware.org
dilfri...@gentoo.org
binut...@sourceware.org
gcc@gcc.gnu.org
g...@sourceware.org
View all guest info  
https://calendar.google.com/calendar/event?action=VIEW=MjVqczBqZ2x2aWFkZ2QyczNzYTJoMmdwb2VfMjAyNDA1MzBUMTUwMDAwWiBnY2NAZ2NjLmdudS5vcmc=MTkjY29kb25lbGxAcmVkaGF0LmNvbTA1YWM1ODFlZjQwZjU1YjhkN2ZjMGJlMDZlYzkyNDFiMTkxNDQ1Yjc=America%2FNew_York=en=0


Reply for gcc@gcc.gnu.org and view more details  
https://calendar.google.com/calendar/event?action=VIEW=MjVqczBqZ2x2aWFkZ2QyczNzYTJoMmdwb2VfMjAyNDA1MzBUMTUwMDAwWiBnY2NAZ2NjLmdudS5vcmc=MTkjY29kb25lbGxAcmVkaGF0LmNvbTA1YWM1ODFlZjQwZjU1YjhkN2ZjMGJlMDZlYzkyNDFiMTkxNDQ1Yjc=America%2FNew_York=en=0

Your attendance is optional.

~~//~~
Invitation from Google Calendar: https://calendar.google.com/calendar/

You are receiving this email because you are an attendee on the event. To  
stop receiving future updates for this event, decline this event.


Forwarding this invitation could allow any recipient to send a response to  
the organizer, be added to the guest list, invite others regardless of  
their own invitation status, or modify your RSVP.


Learn more https://support.google.com/calendar/answer/37135#forwarding


invite.ics
Description: application/ics


Invitation: Office Hours for the GNU Toolchain @ Monthly from 11am to 12pm on the last Thursday (EDT) (gcc@gcc.gnu.org)

2024-04-17 Thread Carlos O'Donell via Gcc

Office Hours for the GNU Toolchain
Monthly from 11am to 12pm on the last Thursday
Eastern Time - New York

Location
https://bbb.linuxfoundation.org/room/adm-xcb-for-sk6
https://www.google.com/url?q=https%3A%2F%2Fbbb.linuxfoundation.org%2Froom%2Fadm-xcb-for-sk6=D=17137887=AOvVaw2V18NEONPlH-coE-TT_R5q



Office hours for the GNU  
Toolchain:https://gcc.gnu.org/wiki/OfficeHoursMeeting  
link:https://bbb.linuxfoundation.org/room/adm-xcb-for-sk6


Organizer
Carlos O'Donell
codon...@redhat.com

Guests
Carlos O'Donell - organizer
Nick Clifton
Jakub Jelinek
David Edelsohn
Siddhesh Poyarekar
Jason Merrill
j...@polyomino.org.uk
richard.earns...@arm.com
girish...@gmail.com
adhemerval.zane...@linaro.org
Florian Weimer
berg...@linux.ibm.com
Peter Bergner
libc-al...@sourceware.org
dilfri...@gentoo.org
binut...@sourceware.org
christophe.l...@linaro.org
gcc@gcc.gnu.org
g...@sourceware.org
View all guest info  
https://calendar.google.com/calendar/event?action=VIEW=MjVqczBqZ2x2aWFkZ2QyczNzYTJoMmdwb2VfUjIwMjQwNDI1VDE1MDAwMCBnY2NAZ2NjLmdudS5vcmc=MTkjY29kb25lbGxAcmVkaGF0LmNvbTc2YTJkODA5YmFjODAwMzBiZmVlYTZlYjRmMTkyODAyYmM1Yjk2M2E=America%2FNew_York=en=0


Reply for gcc@gcc.gnu.org and view more details  
https://calendar.google.com/calendar/event?action=VIEW=MjVqczBqZ2x2aWFkZ2QyczNzYTJoMmdwb2VfUjIwMjQwNDI1VDE1MDAwMCBnY2NAZ2NjLmdudS5vcmc=MTkjY29kb25lbGxAcmVkaGF0LmNvbTc2YTJkODA5YmFjODAwMzBiZmVlYTZlYjRmMTkyODAyYmM1Yjk2M2E=America%2FNew_York=en=0

Your attendance is optional.

~~//~~
Invitation from Google Calendar: https://calendar.google.com/calendar/

You are receiving this email because you are an attendee on the event. To  
stop receiving future updates for this event, decline this event.


Forwarding this invitation could allow any recipient to send a response to  
the organizer, be added to the guest list, invite others regardless of  
their own invitation status, or modify your RSVP.


Learn more https://support.google.com/calendar/answer/37135#forwarding


invite.ics
Description: application/ics


Office Hours for the GNU Toolchain on 2024-04-25 at 11am EST5EDT.

2024-04-16 Thread Carlos O'Donell via Gcc
Office Hours for the GNU Toolchain on 2024-04-25 at 11am EST5EDT.

Agenda:
* https://gcc.gnu.org/wiki/OfficeHours#Next

Meeting Link:
* https://bbb.linuxfoundation.org/room/adm-xcb-for-sk6

--
Cheers,
Carlos.



Office Hours for the GNU Toolchain on 2024-03-28 at 11am EST5EDT.

2024-03-19 Thread Carlos O'Donell via Gcc
Office Hours for the GNU Toolchain on 2024-03-28 at 11am EST5EDT.

Agenda:
* https://gcc.gnu.org/wiki/OfficeHours#Next

Meeting Link:
* https://bbb.linuxfoundation.org/room/adm-xcb-for-sk6

--
Cheers,
Carlos.



Office Hours for the GNU Toolchain on 2024-02-29 at 11am EST5EDT.

2024-02-19 Thread Carlos O'Donell via Gcc
Office Hours for the GNU Toolchain on 2024-02-29 at 11am EST5EDT.

Agenda:
* https://gcc.gnu.org/wiki/OfficeHours#Next

Meeting Link:
* https://bbb.linuxfoundation.org/room/adm-xcb-for-sk6

--
Cheers,
Carlos.



Re: New TLS usage in libgcc_s.so.1, compatibility impact

2024-01-15 Thread Carlos O'Donell via Gcc
On 1/15/24 08:55, Adhemerval Zanella Netto wrote:
> 
> 
> On 15/01/24 09:46, Szabolcs Nagy wrote:
>> The 01/13/2024 13:49, Florian Weimer wrote:
>>> This commit
>>>
>>> commit 8abddb187b33480d8827f44ec655f45734a1749d
>>> Author: Andrew Burgess 
>>> Date:   Sat Aug 5 14:31:06 2023 +0200
>>>
>>> libgcc: support heap-based trampolines
>>> 
>>> Add support for heap-based trampolines on x86_64-linux, aarch64-linux,
>>> and x86_64-darwin. Implement the __builtin_nested_func_ptr_created and
>>> __builtin_nested_func_ptr_deleted functions for these targets.
>>> 
>>> Co-Authored-By: Maxim Blinov 
>>> Co-Authored-By: Iain Sandoe 
>>> Co-Authored-By: Francois-Xavier Coudert 
>>>
>>> added TLS usage to libgcc_s.so.1.  The way that libgcc_s is currently
>>> built, it ends up using a dynamic TLS variant on the Linux targets.
>>> This means that there is no up-front TLS allocation with glibc (but
>>> there would be one with musl).
>>>
>>> There is still a compatibility impact because glibc assigns a TLS module
>>> ID upfront.  This seems to be what causes the
>>> ust/libc-wrapper/test_libc-wrapper test in lttng-tools to fail.  We end
>>> up with an infinite regress during process termination because
>>> libgcc_s.so.1 has been loaded, resulting in a DTV update.  When this
>>> happens, the bottom of the stack looks like this:
>>>
>>> #4447 0x77f288f0 in free () from 
>>> /lib64/liblttng-ust-libc-wrapper.so.1
>>> #4448 0x77fdb142 in free (ptr=)
>>> at ../include/rtld-malloc.h:50
>>> #4449 _dl_update_slotinfo (req_modid=3, new_gen=2) at ../elf/dl-tls.c:822
>>> #4450 0x77fdb214 in update_get_addr (ti=0x77f2bfc0, 
>>> gen=) at ../elf/dl-tls.c:916
>>> #4451 0x77fddccc in __tls_get_addr ()
>>> at ../sysdeps/x86_64/tls_get_addr.S:55
>>> #4452 0x77f288f0 in free () from 
>>> /lib64/liblttng-ust-libc-wrapper.so.1
>>> #4453 0x77fdb142 in free (ptr=)
>>> at ../include/rtld-malloc.h:50
>>> #4454 _dl_update_slotinfo (req_modid=2, new_gen=2) at ../elf/dl-tls.c:822
>>> #4455 0x77fdb214 in update_get_addr (ti=0x77f39fa0, 
>>> gen=) at ../elf/dl-tls.c:916
>>> #4456 0x77fddccc in __tls_get_addr ()
>>> at ../sysdeps/x86_64/tls_get_addr.S:55
>>> #4457 0x77f36113 in lttng_ust_cancelstate_disable_push ()
>>>from /lib64/liblttng-ust-common.so.1
>>> #4458 0x77f4c2e8 in ust_lock_nocheck () from 
>>> /lib64/liblttng-ust.so.1
>>> #4459 0x77f5175a in lttng_ust_cleanup () from 
>>> /lib64/liblttng-ust.so.1
>>> #4460 0x77fca0f2 in _dl_call_fini (
>>> closure_map=closure_map@entry=0x77fbe000) at dl-call_fini.c:43
>>> #4461 0x77fce06e in _dl_fini () at dl-fini.c:114
>>> #4462 0x77d82fe6 in __run_exit_handlers () from /lib64/libc.so.6
>>>
>>> Cc:ing  for awareness.
>>>
>>> The issue also requires a recent glibc with changes to DTV management:
>>> commit d2123d68275acc0f061e73d5f86ca504e0d5a344 ("elf: Fix slow tls
>>> access after dlopen [BZ #19924]").  If I understand things correctly,
>>> before this glibc change, we didn't deallocate the old DTV, so there was
>>> no call to the free function.
>>
>> with 19924 fixed, after a dlopen or dlclose every thread updates
>> its dtv on the next dynamic tls access.
>>
>> before that, dtv was only updated up to the generation of the
>> module being accessed for a particular tls access.
>>
>> so hitting the free in the dtv update path is now more likely
>> but the free is not new, it was there before.
>>
>> also note that this is unlikely to happen on aarch64 since
>> tlsdesc only does dynamic tls access after a 512byte static
>> tls reservation runs out.
>>
>>>
>>> On the glibc side, we should recommend that intercepting mallocs and its
>>> dependencies use initial-exec TLS because that kind of TLS does not use
>>> malloc.  If intercepting mallocs using dynamic TLS work at all, that's
>>> totally by accident, and was in the past helped by glibc bug 19924.  (I
>>
>> right.
>>
>>> don't think there is anything special about libgcc_s.so.1 that triggers
>>> the test failure above, it is just an object with dynamic TLS that is
>>> implicitly loaded via dlopen at the right stage of the test.)  In this
>>> particular case, we can also paper over the test failure in glibc by not
>>> call free at all because the argument is a null pointer:
>>>
>>> diff --git a/elf/dl-tls.c b/elf/dl-tls.c
>>> index 7b3dd9ab60..14c71cbd06 100644
>>> --- a/elf/dl-tls.c
>>> +++ b/elf/dl-tls.c
>>> @@ -819,7 +819,8 @@ _dl_update_slotinfo (unsigned long int req_modid, 
>>> size_t new_gen)
>>>  dtv entry free it.  Note: this is not AS-safe.  */
>>>   /* XXX Ideally we will at some point create a memory
>>>  pool.  */
>>> - free (dtv[modid].pointer.to_free);
>>> + if (dtv[modid].pointer.to_free != NULL)
>>> +   free (dtv[modid].pointer.to_free);
>>>   dtv[modid].pointer.val = TLS_DTV_UNALLOCATED;

Office Hours for the GNU Toolchain on 2024-01-25 at 11am EST5EDT.

2024-01-15 Thread Carlos O'Donell via Gcc
Office Hours for the GNU Toolchain on 2024-01-25 at 11am EST5EDT.

Agenda:
* https://gcc.gnu.org/wiki/OfficeHours#Next

Meeting Link:
* https://bbb.linuxfoundation.org/room/adm-xcb-for-sk6

-- 
Cheers,
Carlos.



Office Hours for the GNU Toolchain on 2023-11-30 at 11am EST5EDT.

2023-11-24 Thread Carlos O'Donell via Gcc
Office Hours for the GNU Toolchain on 2023-11-30 at 11am EST5EDT.

Agenda: 
 * https://gcc.gnu.org/wiki/OfficeHours#Next

Meeting Link:
 * https://bbb.linuxfoundation.org/room/adm-xcb-for-sk6

-- 
Cheers,
Carlos.



Re: Checks that autotools generated files were re-generated correctly

2023-11-07 Thread Carlos O'Donell via Gcc
On 11/7/23 02:38, Maxim Kuvyrkov wrote:
>> On Nov 6, 2023, at 21:19, Christophe Lyon
>>  wrote:
>> 
>> Hi!
>> 
>> On Mon, 6 Nov 2023 at 18:05, Martin Jambor 
>> wrote:
>>> 
>>> Hello,
>>> 
>>> I have inherited Martin Liška's buildbot script that checks that
>>> all sorts of autotools generated files, mainly configure scripts,
>>> were re-generated correctly when appropriate.  While the checks
>>> are hopefully useful, they report issues surprisingly often and
>>> reporting them feels especially unproductive.
>>> 
>>> Could such checks be added to our server side push hooks so that
>>> commits introducing these breakages would get refused
>>> automatically.  While the check might be a bit expensive, it only
>>> needs to be run on files touching the generated files and/or the
>>> files these are generated from.

$0.02.

We should move in a direction where all server side push hooks removed.

Removing the hooks allows for easy repo replication, and sharing load.

Such checks should all be moved IMO into pre-commit CI, or post-commit CI.

>>> Alternatively, Maxim, you seem to have an infrastructure that is
>>> capable of sending email.  Would you consider adding the check to
>>> your buildbot instance and report issues automatically?  The
>>> level of totally
>> 
>> After the discussions we had during Cauldron, I actually thought
>> we should add such a bot.
>> 
>> Initially I was thinking about adding this as a "precommit" check,
>> to make sure the autogenerated files were submitted correctly, but
>> I realized that the policy is actually not to send autogenerated
>> files as part of the patch (thus making pre-commit check
>> impracticable in such cases, unless we autogenerate those files
>> after applying the patch)
>> 
>> I understand you mean to run this as a post-commit bot, meaning we 
>> would continue to "accept" broken commits, but now automatically
>> send a notification, asking for a prompt fix?
>> 
>> We can probably implement that, indeed. Is that the general
>> agreement?
> 
> [CC: Siddhesh, Carlos]
> 
> Hi Martin,
> 
> I agree with Christophe, and we can add various source-level checks
> and wrap them up as a post-commit job.  The job will then send out
> email reports to developers whose patches failed it.

This is a great way to handle this until we have more consensus around other
kinds of worfklows.

> Where the current script is located?  These checks would be useful
> for all GNU Toolchain projects -- binutils/GDB, GCC, Glibc and,
> maybe, Newlib -- so it would be useful to put it in a separate
> "gnutools" repo.  I think Siddhesh and Carlos are looking into
> creating such a repo on gitlab?

I can make any repo we want here:

https://gitlab.com/gnutools

-- 
Cheers,
Carlos.



Office Hours for the GNU Toolchain on October 17th at 11am EDT.

2023-10-06 Thread Carlos O'Donell via Gcc
Office Hours for the GNU Toolchain on October 17th at 11am EDT.

Agenda: 
 * First Office Hours and test of the meeting system.
 * No specific agenda will be presented, but feel free to attend.

Meeting Link:
 * https://bbb.linuxfoundation.org/room/adm-xcb-for-sk6

-- 
Cheers,
Carlos.



Re: [RFC] GCC Security policy

2023-08-08 Thread Carlos O'Donell via Gcc-patches
On 8/8/23 13:46, David Edelsohn wrote:
> I believe that upstream projects for components that are imported
> into GCC should be responsible for their security policy, including
> libgo, gofrontend, libsanitizer (other than local patches), zlib,
> libtool, libphobos, libcody, libffi, eventually Rust libcore, etc.

I agree completely.

We can reference the upstream and direct people to follow upstream security
policy for these bundled components.

Any other policy risks having conflicting guidance between the projects,
which is not useful for security policy.

There might be exceptions to this rule, particularly when the downstream
wants to accept particular risks while upstream does not; but none of these
components are in that case IMO.

-- 
Cheers,
Carlos.



Re: git out-of-order commit (was Re: [PATCH] Fortran: Remove unused declaration)

2023-01-20 Thread Carlos O'Donell via Gcc-patches
On 1/19/23 23:26, Bernhard Reutner-Fischer wrote:
> On 19 January 2023 20:39:08 CET, Jason Merrill  wrote:
>> On Sat, Nov 12, 2022 at 4:24 PM Harald Anlauf via Gcc-patches
>>  wrote:
>>>
>>> Am 12.11.22 um 22:05 schrieb Bernhard Reutner-Fischer via Gcc-patches:
 This function definition was removed years ago, remove it's prototype.

 gcc/fortran/ChangeLog:

   * gfortran.h (gfc_check_include): Remove declaration.
 ---
   gcc/fortran/gfortran.h | 1 -
   1 file changed, 1 deletion(-)
 ---
 Regtests cleanly, ok for trunk?

 diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
 index c4deec0d5b8..ce3ad61bb52 100644
 --- a/gcc/fortran/gfortran.h
 +++ b/gcc/fortran/gfortran.h
 @@ -3208,7 +3208,6 @@ int gfc_at_eof (void);
   int gfc_at_bol (void);
   int gfc_at_eol (void);
   void gfc_advance_line (void);
 -int gfc_check_include (void);
   int gfc_define_undef_line (void);

   int gfc_wide_is_printable (gfc_char_t);
>>>
>>> OK, thanks.
>>
>> Somehow this was applied with a CommitDate in 2021, breaking scripts
>> that assume monotonically increasing CommitDate.  Anyone know how that
>> could have happened?
> 
> Sorry for that.
> I think i cherry-picked this commit to master before pushing it, not 100% 
> sure though.
> What shall we do now?

I doubt a cherry-pick did this, we cherry pick often in glibc and the
commit is added to the top of checkout and the commit date updated.

There isn't anything we can do now.

I was recently made aware that --since-as-filter= was added specifically to 
address this issue.

https://patchwork.kernel.org/project/git/patch/YlnYDgZRzDI87b/z...@vmiklos.hu/
~~~
This is similar to --since, but it will filter out not matching commits,
rather than stopping at the first not matching commit.

This is useful if you e.g. want to list the commits from the last year,
but one odd commit has a bad commit date and that would hide lots of
earlier commits in that range.

The behavior of --since is left unchanged, since it's valid to depend on
its current behavior.

Signed-off-by: Miklos Vajna 
~~~

"but one odd commit has a bad commit date" :-)

We should try to avoid commits like this because they really complicate
any date-based analysis tooling, and --since-as-filter is fairly new.

-- 
Cheers,
Carlos.



RFP is open for the Toolchains and Kernel Track at LPC 2022

2022-04-26 Thread Carlos O'Donell via Gcc
The Linux Plumbers Conference 2022 (https://lpc.events) will be held
from 12 to 14 of September 2022 in Dublin.

As part of the conference we will be having a Toolchains and Kernel
track that will focus on topics of interest related to building the
Linux kernel, and kernel development in general. The goal is to get
kernel developers and toolchain developers together to discuss
outstanding or upcoming issues, feature requests, and further
collaboration.

Here we are calling for activity proposals for the track.

Note that LPC "activities" are not quite the same than regular talks as
they are found in most other free software events.  The activities shall
be related to the Linux kernel and should focus on some particular
toolchain problem, or issue, or proposed enhancement, that require or
would benefit from the input of kernel hackers.  The pursued outcome of
the activity shall not be some vague directions or promises of future
work and/or collaboration: you should aim to discuss and agree on the
spot using the fact the kernel hackers will be present.  The duration of
each activity depends on its nature and on how it actually develops:
some discussions will require just a few minutes, while others may
require more time.  However, it would be useful if you specify an
estimation of how much time you expect your activity will require; that
will help us in the scheduling. Slides are actually discouraged, so
please try to keep them at a minimum, the ideal is just one or two
slides to establish the context for the discussion.

As a reference, these are some of the topics that we would like to cover
this year:

Upstreaming Rust Support
Toolchain security feature requests
BPF/BTF support in the GNU Toolchain
Kernel Control Flow Integrity
PGO, AutoFDO, gprofng, perf
sysroots on kernel.org
hardware mitigation post mortems
Kernel debugging with drgn and poke+gdb
ABI analysis
GCC -fanalyzer
Fast Kernel Headers

If you are interested in proposing an activity, please use the online
form at the event website: https://lpc.events/event/16/abstracts/

Please make sure you use the submission form for the Toolchains track.

The CFP closes the 15th of August.  We will announce the approved
activities and publish the schedule of the track shortly after that.

[Thanks to Jose Marchesi and Nick Nick Desaulniers for organizing!]
-- 
Cheers,
Carlos.



Re: https://patchwork.sourceware.org/project/gcc/

2021-09-29 Thread Carlos O'Donell via Gcc
On 9/29/21 07:22, Jonathan Wakely wrote:
> On Wed, 29 Sept 2021 at 11:04, Thomas Schwinge  
> wrote:
>> Also, we'll need some user guide: web page, or wiki page, or,
>> preferably?, on  itself.
>> How are we using the different states, archived, bundles, etc.
> 
> Good idea ... care to write it? :-)
> 
>> I bet Carlos and team have all this sorted out for glibc already, so we
>> may "just" copy that for GCC?  ;-P
 
Quick reply.

1. Thomas now has gcc project permission.

2. See glibc's notes:
https://sourceware.org/glibc/wiki/Patch%20Review%20Workflow


-- 
Cheers,
Carlos.



The official glibc IRC channel is now on OFTC as #glibc.

2021-06-16 Thread Carlos O'Donell via Gcc
Community,

The official glibc IRC channel is now on OFTC as #glibc.

gcc developers, please feel free to visit #glibc :-)

We also have an unofficial IRC channel #glibc on Libera.Chat

-- 
Cheers,
Carlos.



Re: New x86-64 micro-architecture levels

2020-07-31 Thread Carlos O'Donell via Gcc
On 7/22/20 6:34 AM, Florian Weimer wrote:
> * Jan Beulich:
> 
>> On 21.07.2020 20:04, Florian Weimer wrote:
>>> * Premachandra Mallappa:
>>> 
 [AMD Public Use]
 
 Hi Floarian,
 
> I'm including a proposal for the levels below.  I use single
> letters for them, but I expect that the concrete
> implementation of this proposal will use names like
> “x86-100”, “x86-101”, like in the glibc patch referenced
> above.  (But we can discuss other approaches.)
 
 Personally I am not a big fan of this, for 2 reasons 1. uses
 just x86 in name on x86_64 as well
>>> 
>>> That's deliberate, so that we can use the same x86-* names for
>>> 32-bit library selection (once we define matching
>>> micro-architecture levels there).
>> 
>> While indeed I did understand it to be deliberate, in the light of 
>> 64-bit only ISA extensions (like AMX, and I suspect we're going to 
>> see more) I nevertheless think Premachandra has a point here.
> 
> Let me explain how I ended up there.  Maybe I'm wrong.

I did a review of your analysis, and it is my opinion that your
conclusion is correct.

> Previously, I observed that it is difficult to set LD_PRELOAD and 
> LD_LIBRARY_PATH on combined x86-64/i386 systems, so that the right 
> libraries are loaded for both variants, and users aren't confused by 
> dynamic linker warning messages.  On some systems, it is possible to
> use dynamic string tokens ($LIB), but not all.

The case of LD_PRELOAD is the most difficult because it is a direct request
to the dynamic loader to load a particular library. If the library to be
loaded is an absolute path then you'll always get warning messages if you
need to execute child processes that inherited LD_PRELOAD for an architecture
that doesn't match the architecture to be executed.

The case of LD_LIBRARY_PATH is generally less troublesome because you are
adding search paths, and the library loading can be suppressed by other
mechanisms that include search path pruning.

It is also possible that $LIB does not match what is actually required
for the system to operate correctly and it depends on /etc/ld.so.conf
(and included files) for correctness (despite it being a cache, see
glibc bug 22359). This is an ISV problem that the ISV can solve.

> Eventually, it will be possible to add and restrict glibc-hwcaps 
> subdirectories by setting an environment variable.  The original
> patch series only contains ld.so command line options because I
> wanted to avoid a discussion about the precise mechanism for setting
> the environment variable (current glibc has two approaches).  But the
> desire to provide this functionality is there: for adding additional 
> glibc-hwcaps subdirectories to be searched first, and for
> restricting selection to a subset of the built-in
> (automatically-selected) subdirectories.

If you allow the addition of subdirectories, those subdirectories
can then be processed as directories are normally processed and we
can indeed avoid emitting an error message. The addition of directories
is not a direct request to the loader to load a specific shared object.

> I was worried that we would run into the same problem as with 
> LD_PRELOAD, where x86-64 and i386 binaries may have different 
> requirements.  I wanted to minimize the conflict by sharing the
> names (eventually, once we have 32-bit variants).

Right, this would make it easier to deploy from the ISV side.

> But thinking about this again, I'm not sure if my worry is
> warranted. The main selection criteria is still the library load
> path, and that is already provided by some different means (e.g.
> $LIB).  Within the library path, there is the glibc-hwcaps
> subdirectory, but since it is nested under a specific library path
> subdirectory (determined by the architecture), adding subdirectories
> to be searched which do not exist on the file system, or surpressing
> directories which would not be searched in the first place, is not a
> problem.  The situation is completely benign and would not warrant
> any error message from the dynamic loader.

I agree completely.
 
> If this analysis is correct, there is no reason to share the 
> subdirectory names between x86-64 and i386 binaries, and we can put
> “64” somewhere in the x86-64 strings.

We can choose not to share the paths. In fact it may make it easier to
explain to users that they are distinct.

In summary:

The conclusion is that x86-64 and i386 shared objects can use different
directories because they are just search paths, and such search paths
have different semantics from explicit load requests like LD_PRELOAD,
therefore they can be suppressed at runtime without the need to issue
an error or warning diagnostic.

Notes:
- We may wish to have an LD_DEBUG settings that helps catch issues
  with various paths, but that's a diagnostic settings whose semantics
  we can iron out as we discover developers making bad choices.

-- 
Cheers,
Carlos.



Re: New x86-64 micro-architecture levels

2020-07-31 Thread Carlos O'Donell via Gcc
On 7/22/20 5:26 AM, Richard Biener via Libc-alpha wrote:
> So for the bike-shedding I indeed think x86-10{0,1,2,3}
> or x86-{A,B,C,..}, eventually duplicating as x86_64- as
> suggested by Jan is better than x86-2014 or x86-avx2.

Agreed. If we really want to be clear, call it a "level"
or something else that has a direct English meaning
e.g. x86-level-101. This makes it unambiguously clear
that this is some kind of step at which some kind of
features are enabled.

-- 
Cheers,
Carlos.



Re: Separate commit mailing lists for trunk/branches possible?

2020-07-17 Thread Carlos O'Donell via Gcc
On 7/17/20 3:41 PM, Florian Weimer wrote:
> * Carlos O'Donell via Gcc:
> 
>> FYI, for glibc we use the AdaCore git commit hooks (like gdb).
>>
>> There we use this configuration:
>>
>> # Only send emails for master and release branches.
>> no-emails = refs/heads/(?!master|release.*)
>>
>> This way you don't get vendor branch commit emails.
> 
> I think this only affects the Bugzilla gateway (which is broken anyway
> because it doesn't extract bug numbers from the commit subject).

I thought it was affecting everything. It sure looks like the infrastructure
intends it to be that way? See hooks/updates/__init__.py 
(send_email_notification)
which is used by post_receive.py driven by post-receive.

However you are right, even since April 15th 2020 when you and I switched
to the AdaCore git commit hooks it appears to have continued to allow
commits to vendor branches to get posted to glibc-cvs.

The IRC irker has a clear filter: refs/heads/master|refs/heads/release/*,
but the irker doesn't seem to be working either.
 
> Anyway, there are a ton of commit notifications for personal branches:
> 
>   <https://sourceware.org/pipermail/glibc-cvs/2020q3/thread.html>
 
Let me raise this on the list.

-- 
Cheers,
Carlos.



Re: Separate commit mailing lists for trunk/branches possible?

2020-07-17 Thread Carlos O'Donell via Gcc
On 7/17/20 2:11 PM, Frank Ch. Eigler via Overseers wrote:
> Hi -
> 
>> Would it be reasonable to have the mailing list split into more than
>> one, that is at least the original covering the trunk, and then one
>> or more for branches?  [...]
> 
> (This matter is for the gcc community to decide.  Overseers do not
> control git/mailing list traffic policy.)
> 
> 
> - FChE
> 

FYI, for glibc we use the AdaCore git commit hooks (like gdb).

There we use this configuration:

# Only send emails for master and release branches.
no-emails = refs/heads/(?!master|release.*)

This way you don't get vendor branch commit emails.

-- 
Cheers,
Carlos.



Feedback on the GNU Social contract and new wiki.gnu.tools.

2020-01-28 Thread Carlos O'Donell
Myself and several other GNU Maintainers have been publicly discussing
a GNU Social contract on gnu-misc-disc...@gnu.org and what it means to
be a GNU Project volunteer.

We are continuing that discussion by reaching out (by email) to all
GNU Maintainers. We look forward to your feedback! Please have a look
at the email you get since it has some important dates.

To assist in this discussion we have created a new wiki for use by GNU
Maintainers.

https://wiki.gnu.tools

The wiki is designed to meet the needs of GNU Maintainers who have
asked for a wiki.  Given the feedback it is git based with markdown
support, and is an actively maintained wiki that is free software with
good internationalization.

Thank you for your support in making a positive change in the GNU Project.

Cheers,
Carlos.


Setting up a wiki for GNU Project volunteers?

2019-12-15 Thread Carlos O'Donell
A wiki for GNU Project volunteers would provide a central place
for cross-project collaboration, including collaboration between
gcc, glibc, gdb, and binutils.

This is an invitation to discuss having a high-level wiki for
GNU Project volunteers.

The conversation is on gnu-misc-discuss:
https://lists.gnu.org/archive/html/gnu-misc-discuss/2019-12/msg2.html

Please feel free to send me your input directly if you like.

Thank you for your input.

-- 
Cheers,
Carlos.



Public discussions on GNU Project governance.

2019-10-23 Thread Carlos O'Donell
GNU Maintainers, developers, volunteers, etc.,

This relates to all of our projects and how they operate. Please take
a minute and look over this email.

This is an invitation to a public discussion about GNU Project governance 
by GNU maintainers, developers, volunteers, etc.

Discussions will take place on a moderated mailing list gnu-misc-discuss:
https://lists.gnu.org/mailman/listinfo/gnu-misc-discuss

Mark Wielaard has kicked this off with a post about his ideas:
https://lists.gnu.org/archive/html/gnu-misc-discuss/2019-10/msg2.html

Please feel free to post your own ideas and opinions about governance
and how it relates to the GNU Project.

Please keep the conversations kind [1] and strictly about governance and
not about specific people and their respective abilities. If you need an 
example: "Carlos O'Donell is an aweful leader and speller" is off-topic,
but "I would like an autocratic model where we have one leader for life
who passes this on to the next leader" is on-topic.

If you have any questions please don't hesitate to ask me.

-- 
Cheers,
Carlos.

[1] https://www.gnu.org/philosophy/kind-communication.html



Re: glibc-2.2{8,9} multiarch build failure on x86_64 with gcc 9

2019-05-01 Thread Carlos O'Donell

On 5/1/19 2:24 PM, Arvind Sankar wrote:

gcc 9 when configured for fortran installs ISO_Fortran_Binding.h in
gfor_cdir = 
$(libdir)/gcc/$(target_alias)/$(gcc_version)$(MULTISUBDIR)/include
For x86_64's 32-bit architecture support, this creates a subdirectory named 
'include'
inside $(libsubdir)/32 which didn't use to exist in gcc 8.

This doesn't seem correct.

I would have expected the header to exist under the target name, for example:

/usr/lib/gcc/i686-redhat-linux/9/include/ISO_Fortran_binding.h

This way it doesn't conflict with other uses.

Perhaps there is enough variability in the way you build, package, and install 
this
that it can break in some configurations.

I think the gcc community needs to comment on this.

--
Cheers,
Carlos.


Re: GNU Tools Cauldron 2017 follow up: "Reviewed-by" etc.

2017-10-19 Thread Carlos O'Donell
On 10/19/2017 09:45 AM, Joseph Myers wrote:
> On Thu, 19 Oct 2017, Thomas Schwinge wrote:
> 
>> Hi!
>>
>> Still waiting for any kind of reaction -- general process-change inertia,
>> chicken-and-egg problem, I suppose.  ;-/
>>
>> I have now put the proposed text onto a wiki page, so that those
>> interested have a convenient handle to use,
>> .
> 
> That wiki page refers to Reviewed-by as being about crediting reviewers.  
> But the specification appears to be oriented to something else entirely 
> (i.e. convincing a committer - in a Linux-kernel-like context with a very 
> limited set of committers to a particular tree, much smaller than the set 
> of reviewers - that a patch is worthy of commit).  It doesn't cover 
> reviews that request changes, or only relate to part of a patch, or relate 
> to a previous version of a patch - only the limited special case of a 
> review approving the entirety of a patch as posted.  If the aim is credit, 
> a substantially different specification is needed.
 
If a person is requesting changes, they should after accepting the changes,
submit a 'Reviewed-by:' tag or 'Acked-by:' tag to indicate they are happy
with the results?

-- 
Cheers,
Carlos.


Re: GNU Tools Cauldron 2017 follow up: "Reviewed-by" etc.

2017-10-19 Thread Carlos O'Donell
On 10/19/2017 09:45 AM, Joseph Myers wrote:
> On Thu, 19 Oct 2017, Thomas Schwinge wrote:
> 
>> Hi!
>>
>> Still waiting for any kind of reaction -- general process-change inertia,
>> chicken-and-egg problem, I suppose.  ;-/
>>
>> I have now put the proposed text onto a wiki page, so that those
>> interested have a convenient handle to use,
>> .
> 
> That wiki page refers to Reviewed-by as being about crediting reviewers.  
> But the specification appears to be oriented to something else entirely 
> (i.e. convincing a committer - in a Linux-kernel-like context with a very 
> limited set of committers to a particular tree, much smaller than the set 
> of reviewers - that a patch is worthy of commit).  It doesn't cover 
> reviews that request changes, or only relate to part of a patch, or relate 
> to a previous version of a patch - only the limited special case of a 
> review approving the entirety of a patch as posted.  If the aim is credit, 
> a substantially different specification is needed.
 
This is the purpose of Acked-by: ...

Which we could also include.

linux/Documentation/process/submitting-patches.rst
...

Acked-by: does not necessarily indicate acknowledgement of the entire patch.
For example, if a patch affects multiple subsystems and has an Acked-by: from
one subsystem maintainer then this usually indicates acknowledgement of just
the part which affects that maintainer's code.  Judgement should be used here.
When in doubt people should refer to the original discussion in the mailing
list archives.

...

-- 
Cheers,
Carlos.


Re: GNU Tools Cauldron 2017 follow up: "Reviewed-by" etc.

2017-10-19 Thread Carlos O'Donell
On 10/19/2017 08:57 AM, Thomas Schwinge wrote:
> Hi!
> 
> Still waiting for any kind of reaction -- general process-change inertia,
> chicken-and-egg problem, I suppose.  ;-/
> 
> I have now put the proposed text onto a wiki page, so that those
> interested have a convenient handle to use,
> <https://gcc.gnu.org/wiki/Reviewed-by>.

I've started using Reviewed-by: Carlos O'Donell <car...@redhat.com>
and Signed-off-by: Carlos O'Donell <car...@redhat.com> in all my
glibc reviews.

Since then I've seen 5 such items go into the git commit messages.

Progress? :-)

-- 
Cheers,
Carlos.


Re: GNU Tools Cauldron 2017 follow up: "Reviewed-by" etc.

2017-09-21 Thread Carlos O'Donell
On 09/21/2017 12:38 PM, Richard Biener wrote:
> On September 21, 2017 8:18:39 PM GMT+02:00, Carlos O'Donell 
> <car...@redhat.com> wrote:
>> On 09/21/2017 11:56 AM, Richard Biener wrote:
>>>> Not yet.
>>>
>>> I think given an OK from an official reviewer entitles you to commit
>>> it indeed IS matching the formal statement. It better does...
>>
>> Isn't it better to be explicit about this; rather than assuming?
>>
>>>> All of this is nothing compared to the work of doing the review.
>>>
>>> Depends on the complexity of the patch...  
>>
>> ... depends on your ability to create quick paste hot keys :}
> 
> Indeed. Any idea how to do that with K9 / SwiftKey? 

No idea. Right now I use Thunderbird+Clippings, and hotkey my responses
via ctrl+alt+v,r and ctrl+alt+v,s for Reviewed-by/Signed-off-by. Then
with mutt+vim I use recorded macros.

-- 
Cheers,
Carlos.


Re: GNU Tools Cauldron 2017 follow up: "Reviewed-by" etc.

2017-09-21 Thread Carlos O'Donell
On 09/21/2017 11:56 AM, Richard Biener wrote:
>> Not yet.
> 
> I think given an OK from an official reviewer entitles you to commit
> it indeed IS matching the formal statement. It better does...

Isn't it better to be explicit about this; rather than assuming?

>> All of this is nothing compared to the work of doing the review.
> 
> Depends on the complexity of the patch...  

... depends on your ability to create quick paste hot keys :}

-- 
Cheers,
Carlos.


Re: GNU Tools Cauldron 2017 follow up: "Reviewed-by" etc.

2017-09-21 Thread Carlos O'Donell
On 09/21/2017 10:50 AM, Thomas Schwinge wrote:
> So my question is, if I've gotten a patch reviewed by someone who is not
> yet ;-) familiar with that new process, and I nevertheless want to
> acknowledge their time invested in review by putting "Reviewed-by" into
> the commit log, is it fine to do that if the reviewer just answered with
> "OK" (or similar) instead of an explicit "Reviewed-by: NAME "
> statement?
You should instead ask the author to give their "Reviewed-by:" and point
out what the Reviewed-by statement means.

> That is, is it fine to assume that our current patch review's standard
> "OK" (or similar) answer matches the more formal "Reviewer's statement of
> oversight"?

Not yet.

> Maybe in the future, reviewers will then switch over to explicitly
> stating "Reviewed-by: NAME " -- or maybe not, because "OK" is just
> so much easier to type...

All of this is nothing compared to the work of doing the review.

It will be your own personal comments, your reminder, your leading by 
example, that will change behaviours.

-- 
Cheers,
Carlos.


Re: RFC: Add ___tls_get_addr

2017-07-06 Thread Carlos O'Donell
On 07/05/2017 12:02 PM, Rich Felker wrote:
> Note that if you make the change and have gcc generate calls to the
> new ___tls_get_addr symbol, it's going to be problematic for people
> trying to link to older glibc versions that don't have it.

This is a normal problem to have, and there are solutions for this.

As you ask, the real question is: Is it necessary?

-- 
Cheers,
Carlos.


Re: RFC: Add ___tls_get_addr

2017-07-05 Thread Carlos O'Donell
On 07/05/2017 11:38 AM, H.J. Lu wrote:
> On x86-64, __tls_get_addr has to realigns stack so that binaries compiled by
> GCCs older than GCC 4.9.4:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58066
> 
> continue to work even if vector instructions are used by functions called
> from __tls_get_addr, which assumes 16-byte stack alignment as specified
> by x86-64 psABI.
> 
> We are considering to add an alternative interface, ___tls_get_addr, to
> glibc, which doesn't realign stack.  Compilers, which properly align stack
> for TLS, call generate call to ___tls_get_addr, instead of __tls_get_addr,
> if ___tls_get_addr is available.
> 
> Any comments?

Overall I agree with the idea.

However, the triple underscore will lead to eventual human error, can we
avoid that error by changing the name in an meaningful way? I don't care
what you choose really.

-- 
Cheers,
Carlos.


Re: [PATCH] Implement new hook for max_align_t_align

2016-10-12 Thread Carlos O'Donell
On 10/12/2016 10:17 AM, John David Anglin wrote:
> On 2016-10-12 9:48 AM, Jason Merrill wrote:
>> On Wed, Oct 12, 2016 at 4:02 AM, Jakub Jelinek  wrote:
>>> On Wed, Oct 12, 2016 at 09:52:04AM +0200, Florian Weimer wrote:
 dropping the alignment means that the padding before the lock member
 vanishes.  Consequently, we have just created a silent ABI change in
 application code, which is a big no-no.
>>> Sure, it would be an ABI change, but how many users would it affect?
>>>
 Since this is PA-RISC, which is essentially dead (neither HPE nor Debian
 ship it anymore), I stand by my suggestion to bump the fundamental 
 alignment
>>> Or just drop support for a dead arch?
>>>
 instead.  Sure, it is a bit inefficient, but this will only affect PA-RISC
 users.  It does not even cause work for PA-RISC porters. Conversely, if we
 work on this to come up with a different fix, many more people will be
 affected (because they don't get all the nice things we could work on
 instead), and we may need to maintain a special GCC kludge for the
 alternative solution, impacting GCC developers in particular.
>>> But sure, bumping malloc alignment is probably easiest.  And people who want
>>> performance have better options than to stay on 32-bit PA-RISC anyway.
>> Or we could do nothing and tell people to ignore the harmless warning.
> The warning is an issue because of -Werror.  However, it appears easy to 
> suppress it in the PA
> backend.  I have a patch that I'm testing.
> 
> We are discussing offline regarding the glibc issue.  It easy to bump the 
> alignment of malloc
> but I take Jakub's point and maybe we should break the ABI.  Debian unstable 
> churns
> quickly, and I think we would be better off being consistent with the current 
> max_align_t
> and 8-byte aligned malloc.

I am against breaking the ABI.

I would rather see us bump malloc alignment up to 16-bytes.

The last time I changed this alignment it _immediately_ broken libstdc++ 
boostrap
because it's using exactly the kind of embedded pthread_mutext_t we're talking 
about
breaking.

So in that case the debian builds in unstable broke right away, and I had to 
revert
the change. We'd have to BinNMU a bunch of things to get this working again.

Again I think our two options are, in my order of preference:

- Disable the warning via a PA backend change.
- Bump malloc alignment.

I am sensitive to the first change being something that carries with it extra
maintenance burden, so I'm happy to see the second solution chosen if that's 
what
everyone wants (Florian's suggestion).

-- 
Cheers,
Carlos.


Re: [PATCH] Implement new hook for max_align_t_align

2016-10-12 Thread Carlos O'Donell
On 10/11/2016 04:04 PM, John David Anglin wrote:
> On 2016-10-11 2:50 PM, Jason Merrill wrote:
>> /* Alignment, in bits, a C conformant malloc implementation has to
>> provide.
>> The HP-UX malloc implementation provides a default alignment of 8
>> bytes.
>> This can be increased with mallopt.  The glibc implementation also
>> provides
>> 8-byte alignment.  Note that this isn't enough for various POSIX
>> types such
>> as pthread_mutex_t.  However, since we no longer need the 16-byte
>> alignment
>> for atomic operations, we ignore the nominal alignment specified
>> for these
>> types.  The same is true for long double on 64-bit HP-UX.  */
>>
>> If PA malloc doesn't actually provide 16-byte alignment, this change
>> seems problematic; it will mean any type that wants 16-byte alignment
>> will silently get 8-byte alignment instead.
> 
> I agree the situation is something of a mess.  On linux, we could bump the 
> alignment
> of malloc to 16-bytes.  However, Carlos argued that we don't need to and I 
> think doing
> so would be detrimental to performance.

Correct, we do not need a 16-byte alignment at runtime.

> The 16-byte alignment was used originally because the ldcw instruction used 
> for atomic
> operations in linux threads needs 16-byte alignment.  However, the nptl 
> pthread
> implementation now uses a kernel helper for atomic operations.  It only needs
> 4-byte alignment.  The largest alignment actually needed is for long double 
> (8 bytes).
> However, we can't change the 16-byte alignment without affecting the layout 
> of various
> structures.

Correct, the structure padding needs to continue to be there to match the 
original ABI.

> The same is true for long double on HPUX.  Originally, it was planned to 
> implement it
> in hardware and that would have required 16-byte alignment.  It was only 
> implemented
> in software with an 8-byte alignment requirement.  Somehow, it ended up with 
> 8 and
> 16-byte alignment in the HP 32 and 64-bit compilers, respectively. In both 
> cases, malloc
> has 8-byte alignment.  It is possible to increase the "grain" size of HP 
> malloc to 16 bytes.
> 
> Thus, I don't think the silent reduction to 8-byte alignment matters.  
> Without the change,
> we are faced with spurious warnings from new.

I suggested disabling the warnings.
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01445.html

Which is what Jason suggests here:
https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00786.html

Though Florian Weimer suggests just bumping malloc to return 16-byte aligned 
objects and
raising max_align_t, since conceptually that's simple (but will impact 
performance):
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01446.html

I think the case where a user specifically requests a larger aligment is still 
always
bound to fail if they exceed malloc's aligment. So removing the warning just 
leaves
hppa where it is today. No warning. Working with existing malloc alignment. But 
unable
to warn users of legitimate cases where this might matter?

I still suggest disabling the warning.

-- 
Cheers,
Carlos.


Re: [gofrontend-dev] Go patch committed: Avoid warning by using a local var for std::ofstream

2016-09-21 Thread Carlos O'Donell
On 09/21/2016 10:56 AM, Florian Weimer wrote:
> * John David Anglin:
> 
>> On Tue, Sep 20, 2016 at 09:27:17PM +0200, Florian Weimer wrote:
>>> * Ian Lance Taylor:
>>>
 GCC PR 77625 is about a warning while compiling the Go frontend.  The
 Go frontend called `new std::ofstream()`, and the warning is "error:
 `new' of type `std::ofstream {aka std::basic_ofstream}' with
 extended alignment 16".  Frankly I'm not sure how this supposed to
 work: shouldn't it be possible to write new std::ofstream()?  But the
 problem is easy to avoid, since in this case the std::ofstream can be
 a local variable, and that is a better approach anyhow.  This patch
 was bootstrapped on x86_64-pc-linux-gnu.  Committed to mainline.
>>>
>>> This happens only on hppa, right?  It looks as if libstdc++ is
>>> seriously broken there.
>>
>> I'm not sure I understand the comment.  I created a short testcase.
>> I agree that the issue is hppa specific but the extended alignment
>> is coming from the __lock field in pthread_mutex_t.  I'm not sure
>> why this affects new std::ofstream().
> 
> Ahh, thanks for the explanation.  The C++ streams probably have some
> internal locks (hidden behind libstdc++ abstractions).

The alignment is a historical ABI accident that we inherited from
Linuxthreads.

>> The alignment of 16 arises in code that used the ldcw instruction.
>> Although this could be avoided in glibc there are numerous other
>> packages with objects requiring 16-byte alignment.  So, I'm tending
>> to think the typedef for max_align_t should be adjusted on hppa-linux
>> so that it has 16-byte alignment.  Is that the correct approach?
> 
> Yes, and for the warning to go away, GCC's MALLOC_ABI_ALIGNMENT needs
> to be increased as well, I think.

This is not required.

The __attribute__((__aligned__(16))); is not required for pthread locks
to work correctly, it is simply there to ensure structure padding and
layout matches the pre-NPTL ABI to ensure we don't break ABI.

In the original Linuxthreads code there was indeed a singular lock word
that needed 16-byte alignment because the hppa 'load and clear word'
atomic operation needed that alignment.

In NPTL the alignment restrictions are lifted and we use a light-weight
kernel helper (like ARM and m68k) which does compare-and-swap atomically
using kernel-side locks (synchronized with futex operations in the
kernel).

So the alignment is only needed for structure layout.

Malloc can return word aligned memory for a pthread_mutex_t and it would
work just fine.

The broader question is: How do we get rid of the warning?

> Nowadays, we can change the glibc malloc alignment fairly easily, but
> interposed mallocs won't be so nice.  But it is unlikely that any of
> this matters in practice because the issue is not new (only the
> warning is).

The issue is not new, but changing glibc's malloc alignment is not
required.

> I have Cc:ed a few people who might have ideas how to deal with this.
> 
> Torvald, would it be possible to align mutexes internally on hppa, to
> avoid the 16-byte alignment of the entire struct (that is, store a
> pointer to the actual mutex object, which points to a sub-region of
> the struct which is suitably aligned)?

The attribute aligned is used for laying out larger structures where the
pthread structures are embedded in them. Therefore removing attribute
aligned would result in an ABI break for all such embedded structures.
The attribute aligned must remain for such structures to continue to be
laid out the same way they were with Linuxthreads, and now NPTL.

As I mentioned before, the alignment is not required for the correct
operation of the locks.

> We probably could add libstdc++ compile-time tests which make sure
> that all relevant C++ types can be allocated with the global operator
> new.

So how do we get rid of the warning?

We need to keep __attribute__((__aligned(16))); because of structure
layout ABI compatibility, but we don't actually need the alignment at
runtime anymore.

-- 
Cheers,
Carlos.


Re: [PATCH] Support x86-64 TLS code sequences without PLT

2016-06-06 Thread Carlos O'Donell
On 06/03/2016 05:21 PM, H.J. Lu wrote:
> We can generate x86-64 TLS code sequences for general and local dynamic
> models without PLT, which uses indirect call via GOT:
> 
> call *__tls_get_addr@GOTPCREL(%rip)
> 
> instead of direct call:
> 
> call __tls_get_addr[@PLT]

What are the actual pros and cons of this change?

Does this improve security? Performance?

The __tls_get_addr symbol, on x86_64, lives in ld.so, which generally
means that all shared objects (GD usage) indirect through their PLT/GOT
to make the call. In this model, and because of lazy linking, the
PLT-related GOT entries are left read-write to be updated after resolution
(ignore the BIND_NOW + RELRO case since in that case we do all of this
up front).

After your change, without a PLT entry, these symbols can no longer be 
interposed? The static linker would generate a binding (a got reloc for
the symbol which is resolved by the dynamic loader) that cannot be changed,
becomes RO after RELRO?

Is the security benefit worth the loss of interposition for this symbol?

Is there any performance gains?

-- 
Cheers,
Carlos.


Re: An abridged "Writing C" for the gcc web pages

2016-05-03 Thread Carlos O'Donell
On 05/03/2016 06:39 PM, Bernd Schmidt wrote:
> On 05/03/2016 09:59 PM, Richard Sandiford wrote:
>> 
>> And sometimes there are multiple acceptable ways of writing the
>> same code. Which wins is an aesthetic choice, which tools tend to
>> be bad at handling. E.g. if a parameter list is too long for a
>> line, there's often a choice of how to balance the parameters
>> across multiple lines, with some being more readable than others.
>> I wouldn't want the one chosen by a formatting tool to be the only
>> acceptable one.
> 
> I have all the same reservations about using tools for this. We don't
> call it "style" for nothing, it is not purely mechanical and until we
> have general-purpose AI I don't believe computers can do a
> satisfactory job. I've seen attempts to do so in the past, and they
> have failed - you invariably end up with humans contorting their code
> to have it pass the checker, which is entirely counterproductive.

This would be silly. I'd never impose the tool as a commit blocker.
We have strayed from the initial goal: Help new developers submit
well formatted patches, and learn by example using a tool.

-- 
Cheers,
Carlos.


Re: An abridged "Writing C" for the gcc web pages

2016-05-03 Thread Carlos O'Donell
On 05/03/2016 03:59 PM, Richard Sandiford wrote:
> Jeff Law <l...@redhat.com> writes:
>> On 05/02/2016 02:40 PM, Carlos O'Donell wrote:
>>>
>>> However, in the end, I think that yet-another-document is not the
>>> solution we want. What we actually need is a program that just formats
>>> your source according to the GNU Coding Style and that way you can always
>>> tell your users "Run indent" and be done with it. The output of such a
>>> program should always be considered correct, and if it's not, we should
>>> fix it immediately.
>>>
>>> Why can't we use indent?
>> Sadly, "indent" simply breaks c++ code.
>>
>> I think the solution here is clang-format with a suitable clang-format 
>> configuration file.  One has been started (gcc/contrib/clang-format), 
>> but it's not yet complete.
> 
> How far are you intending to go with that though?

As far as it takes to make the process of submitting patches less
painful for new contributors and sensible for maintainers?

If we end up with one tool per branch then we did it wrong. In glibc
I'm happy to retroactively say the newest formatting tool is always
right for all branches barring expert intervention.

> And if the tool isn't the final authority, and it does still remain
> a human choice, then...

We *are* the community, so yes, there is a final line in the sand where
a sensible expert maintainer may tell you "yeah but" and ask you to change
the code to make it more legible. However, 99% of the changes are now just
going to be automatic.
 
>>> At the end of the day I never want to see another comment about code
>>> formatting because the user used X and X always formats code correctly.
>> Amen.
> 
> ...this kind of comment is still going to occur.  And clear documentation
> should help when it does.

Yes, it will occur, but the frequency should be much reduced. I also agree
that clear documentation helps, so I don't disparage Bernd's work here,
I think it's great. I just think that the most important next step here
is a submission code formatting tool.

> Also, coding standards and the scope of Bernd's document are more
> than formatting.  E.g. take your comment in a recent thread about not
> adding "()" when referring to a function in comments.  Even clang-format
> is unlikely to be smart enough to know whether a particular () is
> acceptable or not.  (It would be acceptable in a quoted code snippet
> but not in a normal sentence.)  Also, the tool wouldn't know when
> argument names need to be capitalised in comments, etc.  Sometimes
> argument names are English words and comments contain a mixture of
> both uses.  Bernd's document talked about those kinds of details too.

Agreed. That's minor.

> Sorry for the rant :-)  Maybe I'm just jaded by past experience with
> automatic formatting tools.  E.g. we internally use(d?) check_GNU_style.sh
> to check patches before submission and it seemed to produce far more
> false positives than true positives.  (I'm not sure it ever produced
> what I'd consider a true positive for me, although I did sometimes do
> what it said even if I disagreed, as the path of least resistance.)
> That's not to say the script is bad, just that it shouldn't always be
> taken literally.

That's a shame and a clear indicator the script is wrong or the technology
wasn't around for us to do the kinds of things we wanted.

> Obviously clang-format is smarter than check_GNU_style.sh but I think
> the same principle applies.

If we get one false positive out of the new checker we've done something
wrong. It should strive to give output that everyone agrees is 100%
correct barring optinal aesthetic choices. This way the tool is run, a
reviewer looks at the patch, and final comments are made on the style.

That's what I'd like to see for glibc. $0.02.

-- 
Cheers,
Carlos.


Re: An abridged "Writing C" for the gcc web pages

2016-05-02 Thread Carlos O'Donell
On 04/22/2016 12:21 PM, Bernd Schmidt wrote:
> (Apologies if you get this twice, the mailing list didn't like the
> html attachment in the first attempt).
> 
> We frequently get malformatted patches, and it's been brought to my
> attention that some people don't even make the effort to read the GNU
> coding standards before trying to contribute code. TL;DR seems to be
> the excuse, and while I find that attitude inappropriate, we could
> probably improve the situation by spelling out the most basic rules
> in an abridged document on our webpages. Below is a draft I came up
> with. Thoughts?

Despite the comments downthread, I think that abridged versions of
longish standards documents are always a good idea. They need to be
maintained, usually reactively, and that's fine. It gives a new
contributor something to read that's short enough to provide the salient
points for an initial patch/review cycle. The point is to get better
incrementally. Nobody is ever perfect the first time.

I especially like that Jason caught on that your document is actually
*more* than just the GNU Coding Standard, and that where the standard
is underspecified the projects have their own conventions.

For examples just look at:
https://sourceware.org/glibc/wiki/Style_and_Conventions

However, in the end, I think that yet-another-document is not the
solution we want. What we actually need is a program that just formats
your source according to the GNU Coding Style and that way you can always
tell your users "Run indent" and be done with it. The output of such a
program should always be considered correct, and if it's not, we should
fix it immediately.

Why can't we use indent?

If we can't use indent, what do we need to solve this problem?

At the end of the day I never want to see another comment about code
formatting because the user used X and X always formats code correctly.

-- 
Cheers,
Carlos.


Re: [PATCH] extend.texi: Expand on the perils of using the 'leaf' attribute.

2016-03-15 Thread Carlos O'Donell
On 03/14/2016 06:15 PM, Sandra Loosemore wrote:
> On 03/14/2016 12:40 PM, Carlos O'Donell wrote:
>> Using the 'leaf' attribute is difficult in certain use cases, and
>> the documentation rightly points out that signals is one such
>> problem.
>> 
>> We should additionally document the following caveats:
>> 
>> * Indirect function resolvers (thanks to Florian Weimer for
>> catching this). * Indirect function implementations * ELF symbol
>> interposition.
>> 
>> [snip]
>> 
>> gcc/ 2016-03-14  Carlos O'Donell  <car...@redhat.com>
>> 
>> * doc/extend.texi (Common Function Attributes): Describe ifunc
>> impact on leaf attribute.
>> 
> 
> H.  Both your patch and the original text really need some
> copy-editing to fix noun/verb agreement, punctuation, etc.  How about
> something like the attached patch?  I just threw this together and
> haven't tested this in any way, but you confirm that it builds and it
> looks OK to you, feel free to check it in.

PDF looks good.

Committed as r234247.

2016-03-16  Carlos O'Donell  <car...@redhat.com>
Sandra Loosemore  <san...@codesourcery.com>

* doc/extend.texi (Common Function Attributes): Describe ifunc impact
on leaf attribute. Mention ELF interposition problems.

Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi (revision 234236)
+++ gcc/doc/extend.texi (revision 234247)
@@ -2772,30 +2772,41 @@
 
 @item leaf
 @cindex @code{leaf} function attribute
-Calls to external functions with this attribute must return to the current
-compilation unit only by return or by exception handling.  In particular, leaf
-functions are not allowed to call callback function passed to it from the 
current
-compilation unit or directly call functions exported by the unit or longjmp
-into the unit.  Leaf function might still call functions from other compilation
-units and thus they are not necessarily leaf in the sense that they contain no
-function calls at all.
+Calls to external functions with this attribute must return to the
+current compilation unit only by return or by exception handling.  In
+particular, a leaf function is not allowed to invoke callback functions
+passed to it from the current compilation unit, directly call functions
+exported by the unit, or @code{longjmp} into the unit.  Leaf functions
+might still call functions from other compilation units and thus they
+are not necessarily leaf in the sense that they contain no function
+calls at all.
 
-The attribute is intended for library functions to improve dataflow analysis.
-The compiler takes the hint that any data not escaping the current compilation 
unit can
-not be used or modified by the leaf function.  For example, the @code{sin} 
function
-is a leaf function, but @code{qsort} is not.
+The attribute is intended for library functions to improve dataflow
+analysis.  The compiler takes the hint that any data not escaping the
+current compilation unit cannot be used or modified by the leaf
+function.  For example, the @code{sin} function is a leaf function, but
+@code{qsort} is not.
 
-Note that leaf functions might invoke signals and signal handlers might be
-defined in the current compilation unit and use static variables.  The only
-compliant way to write such a signal handler is to declare such variables
-@code{volatile}.
+Note that leaf functions might indirectly run a signal handler defined
+in the current compilation unit that uses static variables.  Similarly,
+when lazy symbol resolution is in effect, leaf functions might invoke
+indirect functions whose resolver function or implementation function is
+defined in the current compilation unit and uses static variables.  There
+is no standard-compliant way to write such a signal handler, resolver
+function, or implementation function, and the best that you can do is to
+remove the @code{leaf} attribute or mark all such static variables
+@code{volatile}.  Lastly, for ELF-based systems that support symbol
+interposition, care should be taken that functions defined in the
+current compilation unit do not unexpectedly interpose other symbols
+based on the defined standards mode and defined feature test macros;
+otherwise an inadvertent callback would be added.
 
-The attribute has no effect on functions defined within the current compilation
-unit.  This is to allow easy merging of multiple compilation units into one,
-for example, by using the link-time optimization.  For this reason the
-attribute is not allowed on types to annotate indirect calls.
+The attribute has no effect on functions defined within the current
+compilation unit.  This is to allow easy merging of multiple compilation
+units into one, for example, by using the link-time optimization.  For
+this reason the attribute is not allowed on types to annotate indirect
+calls.
 
-
 @item malloc
 @cindex @code{malloc} function attribute
 @cindex functions that behave like malloc
-- 
Cheers,
Carlos.


Re: [PATCH] extend.texi: Expand on the perils of using the 'leaf' attribute.

2016-03-15 Thread Carlos O'Donell
On 03/14/2016 06:15 PM, Sandra Loosemore wrote:
> On 03/14/2016 12:40 PM, Carlos O'Donell wrote:
>> Using the 'leaf' attribute is difficult in certain use cases, and
>> the documentation rightly points out that signals is one such
>> problem.
>> 
>> We should additionally document the following caveats:
>> 
>> * Indirect function resolvers (thanks to Florian Weimer for
>> catching this). * Indirect function implementations * ELF symbol
>> interposition.
>> 
>> [snip]
>> 
>> gcc/ 2016-03-14  Carlos O'Donell  <car...@redhat.com>
>> 
>> * doc/extend.texi (Common Function Attributes): Describe ifunc
>> impact on leaf attribute.
>> 
> 
> H.  Both your patch and the original text really need some
> copy-editing to fix noun/verb agreement, punctuation, etc.  How about
> something like the attached patch?  I just threw this together and
> haven't tested this in any way, but you confirm that it builds and it
> looks OK to you, feel free to check it in.

Hey Sandra! :-)

Testing right now. I like your text better. I'll commit once I make
sure I haven't made a mistake in the formatting.

-- 
Cheers,
Carlos.


[PATCH] extend.texi: Expand on the perils of using the 'leaf' attribute.

2016-03-14 Thread Carlos O'Donell
Using the 'leaf' attribute is difficult in certain use cases, and the
documentation rightly points out that signals is one such problem.

We should additionally document the following caveats:

* Indirect function resolvers (thanks to Florian Weimer for catching this).
* Indirect function implementations
* ELF symbol interposition.

Note that neither the C nor C++ standards talks at all about how
memory is synchronized between the current execution context and that
of a signal handler. Therefore this patch rewords the text to say
"There is no standards compliant way..." although in practice is just
works and one would expect the standards (POSIX) to adopt such language
that existing practice works.

Lastly, we mention that the 'leaf' attribute might simply be removed
if that is the easiest option.

OK to checkin?

For completeness the motivating example from a user was like this:
cat >> leaf.c <
#include 
#include 
#include 

static int tst;

void my_h(int sig)
{
  if (tst == 1)
_exit (0);
  _exit (1);
}

int main()
{
  signal(SIGUSR1, my_h);
  tst++;
  pthread_kill(pthread_self(), SIGUSR1);
  tst--;
  return 2;
}
EOF
gcc -g3 -O3 -o leaf leaf.c -lpthread
./test; echo $?
1

Where the global write of tst is elided by the compiler because
pthread_kill is marked __THROW (includes leaf). It's an open
question if pthread_kill should or should not use __THROWNL.
Even if we fix that in glibc, the changes below to the docs are
still important clarifications.

gcc/
2016-03-14  Carlos O'Donell  <car...@redhat.com>

* doc/extend.texi (Common Function Attributes): Describe ifunc impact
on leaf attribute.

Index: extend.texi
===
--- extend.texi (revision 234183)
+++ extend.texi (working copy)
@@ -2786,9 +2786,17 @@
 is a leaf function, but @code{qsort} is not.
 
 Note that leaf functions might invoke signals and signal handlers might be
-defined in the current compilation unit and use static variables.  The only
-compliant way to write such a signal handler is to declare such variables
-@code{volatile}.
+defined in the current compilation unit and use static variables.  Similarly,
+when lazy symbol resolution is in effect, leaf functions might invoke indirect
+functions whose resolver function or implementation function might be defined
+in the current compilation unit and use static variables. There is no standards
+compliant way to write such a signal handler, resolver function, or
+implementation function, and the best that you can do is to remove the leaf
+attribute or mark all such variables @code{volatile}.  Lastly, for ELF-based
+systems which support symbol interposition one should take care that functions
+defined in the current compilation unit do not unexpectedly interpose other
+symbols based on the defined standards mode otherwise an inadvertent callback
+would be added.
 
 The attribute has no effect on functions defined within the current compilation
 unit.  This is to allow easy merging of multiple compilation units into one,
--
Cheers,
Carlos.


Re: [RFC PATCH] -Wmemset-transposed-args (PR middle-end/61294)

2014-07-08 Thread Carlos O'Donell
On 07/08/2014 08:50 AM, Jakub Jelinek wrote:
 Hi!
 
 This is an attempt to move the warning about transposed memset arguments
 from the glibc headers to gcc FEs.  The problem with the warning in glibc
 is that it uses __builtin_constant_p and e.g. jump threading very often
 makes the warning trigger even on code where it is very unlikely a user
 swapped arguments.  See e.g.
 https://gcc.gnu.org/PR51744
 https://gcc.gnu.org/PR56977
 https://gcc.gnu.org/PR61294
 https://bugzilla.redhat.com/452219
 https://bugs.kde.org/show_bug.cgi?id=311098
 https://bugzilla.mozilla.org/show_bug.cgi?id=581227
 and many others.  Thus, I'd like to warn in the FEs instead, and
 once we have a GCC release with that warning in, disable the glibc
 bits/string3.h:
   if (__builtin_constant_p (__len)  __len == 0
(!__builtin_constant_p (__ch) || __ch != 0))
 {
   __warn_memset_zero_len ();
   return __dest;
 }
 warning for GCC versions with that new warning in.
 
 Any thoughts on this?
 
 If you are ok with it, either we can add it only for 4.10/5.0 and
 later only, or perhaps 4.9.2 too, or even 4.9.1.  For -D_FORTIFY_SOURCE=2
 built code with glibc it shouldn't make a difference (other than having
 fewer false positives), but for other non-fortified -Wall compilation
 it would make a difference (introducing new warnings), so perhaps
 doing it only for 4.10/5.0+ is best.

This seems like a sensible solution to fixing the false positives
caused by jump threading (I haven't looked into that in detail,
just briefly).

I would prefer we enable this for 4.10/5.0+ if only to avoid the
fallout (new warnings) that would happen for the distributions.

We can fix the glibc header once the fix is in gcc, unless someone
objects to the fix itself.

Cheers,
Carlos.



Re: [RFC PATCH] -Wmemset-transposed-args (PR middle-end/61294)

2014-07-08 Thread Carlos O'Donell
On 07/08/2014 03:24 PM, Jason Merrill wrote:
 I don't think we want to warn about e.g. 1-1, only about literal 0.

What rationale would you give for not warning on 1-1?

Cheers,
Carlos.



Re: RFC: Doc update for attribute

2014-05-20 Thread Carlos O'Donell
On 05/20/2014 03:02 AM, David Wohlferd wrote:
 After thinking about this some more, I believe I have some better
 text. Previously I used the word discouraged to describe this
 practice. The existing docs use the term avoid. I believe what you
 want is something more like the attached. Direct and clear, just like
 docs should be.

David, Thanks for the new patch.

 If you are ok with this, I'll send it to gcc-patches.

Looks good to me.

Cheers,
Carlos.

 Index: extend.texi
 ===
 --- extend.texi   (revision 210624)
 +++ extend.texi   (working copy)
 @@ -3332,16 +3332,15 @@
  
  @item naked
  @cindex function without a prologue/epilogue code
 -Use this attribute on the ARM, AVR, MCORE, MSP430, NDS32, RL78, RX and SPU
 -ports to indicate that the specified function does not need prologue/epilogue
 -sequences generated by the compiler.
 -It is up to the programmer to provide these sequences. The
 -only statements that can be safely included in naked functions are
 -@code{asm} statements that do not have operands.  All other statements,
 -including declarations of local variables, @code{if} statements, and so
 -forth, should be avoided.  Naked functions should be used to implement the
 -body of an assembly function, while allowing the compiler to construct
 -the requisite function declaration for the assembler.
 +This attribute is available on the ARM, AVR, MCORE, MSP430, NDS32,
 +RL78, RX and SPU ports.  It allows the compiler to construct the
 +requisite function declaration, while allowing the body of the
 +function to be assembly code. The specified function will not have
 +prologue/epilogue sequences generated by the compiler. Only Basic
 +@code{asm} statements can safely be included in naked functions
 +(@pxref{Basic Asm}). While using Extended @code{asm} or a mixture of
 +Basic @code{asm} and ``C'' code may appear to work, they cannot be
 +depended upon to work reliably and are not supported.
  
  @item near
  @cindex functions that do not handle memory bank switching on 68HC11/68HC12
 @@ -6269,6 +6268,8 @@
  efficient code, and in most cases it is a better solution. When writing 
  inline assembly language outside of C functions, however, you must use Basic 
  @code{asm}. Extended @code{asm} statements have to be inside a C function.
 +Functions declared with the @code{naked} attribute also require Basic 
 +@code{asm} (@pxref{Function Attributes}).
  
  Under certain circumstances, GCC may duplicate (or remove duplicates of) 
 your 
  assembly code when optimizing. This can lead to unexpected duplicate 
 @@ -6388,6 +6389,8 @@
  
  Note that Extended @code{asm} statements must be inside a function. Only 
  Basic @code{asm} may be outside functions (@pxref{Basic Asm}).
 +Functions declared with the @code{naked} attribute also require Basic 
 +@code{asm} (@pxref{Function Attributes}).
  
  While the uses of @code{asm} are many and varied, it may help to think of an 
  @code{asm} statement as a series of low-level instructions that convert 
 input 



Re: RFC: Doc update for attribute

2014-05-20 Thread Carlos O'Donell
On 05/20/2014 03:59 AM, Georg-Johann Lay wrote:
 Am 05/16/2014 07:16 PM, schrieb Carlos O'Donell:
 On 05/12/2014 11:13 PM, David Wohlferd wrote:
 After updating gcc's docs about inline asm, I'm trying to
 improve some of the related sections. One that I feel has
 problems with clarity is __attribute__ naked.
 
 I have attached my proposed update. Comments/corrections are 
 welcome.
 
 In a related question:
 
 To better understand how this attribute is used, I looked at the 
 Linux kernel. While the existing docs say only ... asm
 statements that do not have operands can safely be used, Linux
 routinely uses asm WITH operands.
 
 That's a bug. Period. You must not use naked with an asm that has 
 operands. Any kind of operand might inadvertently cause the
 compiler to generate code and that would violate the requirements
 of the attribute and potentially generate an ICE.
 
 There is target hook TARGET_ALLOCATE_STACK_SLOTS_FOR_ARGS that is
 intended to cater that case.  For example, the documentation
 indicates it only works with optimization turned off.  But I don't
 know how reliable it is in general.  For avr target it works as
 expected.
 
 https://gcc.gnu.org/onlinedocs/gccint/Misc.html#index-TARGET_005fALLOCATE_005fSTACK_005fSLOTS_005fFOR_005fARGS-4969

It's still a bug for now. That hook is there because we've allowed 
bad code to exist for so long that at this point we must for legacy
reasons allow some type of input arguments in the asm. However, that
doesn't mean we should actively promote this feature or let users
use it (until we fix it).

Ideally you do want to use the named input arguments as r types to
avoid needing to know the exact registers used in the call sequence.
Referencing the variables by name and letting gcc emit the right
register is useful, but only if it works consistently and today it
doesn't.

Features that fail to work depending on the optimization level should
not be promoted in the documentation. We should document what works
and file bugs or fix what doesn't work.

Cheers,
Carlos.


Re: RFC: Doc update for attribute

2014-05-16 Thread Carlos O'Donell
On 05/12/2014 11:13 PM, David Wohlferd wrote:
 After updating gcc's docs about inline asm, I'm trying to improve
 some of the related sections. One that I feel has problems with
 clarity is __attribute__ naked.
 
 I have attached my proposed update. Comments/corrections are
 welcome.
 
 In a related question:
 
 To better understand how this attribute is used, I looked at the
 Linux kernel. While the existing docs say only ... asm statements
 that do not have operands can safely be used, Linux routinely uses
 asm WITH operands.

That's a bug. Period. You must not use naked with an asm that has
operands. Any kind of operand might inadvertently cause the compiler
to generate code and that would violate the requirements of the
attribute and potentially generate an ICE.

The correct solution, and we've talked about this in the past, is to
have the compiler generate a hard error if you use an asm statement
with operands and naked. I don't know what anyone ever got around to it.

 Some examples:
 
 memory clobber operand:
 http://lxr.free-electrons.com/source/arch/arm/kernel/kprobes.c#L377 

Is this needed?

 Input arguments:
 http://lxr.free-electrons.com/source/arch/arm/mm/copypage-feroceon.c#L17

This is a bug and it's wrong. The naked asm can just assume the use of
first and second arguments as per AAPCS.

 Since I don't know why asm with operands was excluded from the
 existing docs, I'm not sure whether what Linux does here is supported
 or not (maybe with some limitations?). If someone can clarify, I'll
 add it to this text.

The asm with operands was excluded because to allow them in the
implementation would require gcc to potentially copy the argumnents
to temporary storage depending on their type. There is no prologue so
the compiler has no stack in which to place the arguments, therefore
the result is an impossible to satisfy constraint which usually results
in an ICE or compiler error.

Even if you said it was OK to use the incoming arguments with r type
operands the optimization level of the compile might inadvertently
try to force those values to the stack and that again is an impossible
to satisfy condition with a naked function.

 Even without discussing asm with operands, I believe this text is 
 an improvement.
 Thanks in advance,
 dw
 
 extend.texi.patch
 
 
 Index: extend.texi
 ===
 --- extend.texi   (revision 210349)
 +++ extend.texi   (working copy)
 @@ -3330,16 +3330,15 @@
  
  @item naked
  @cindex function without a prologue/epilogue code
 -Use this attribute on the ARM, AVR, MCORE, MSP430, NDS32, RL78, RX and SPU
 -ports to indicate that the specified function does not need prologue/epilogue
 -sequences generated by the compiler.
 -It is up to the programmer to provide these sequences. The
 -only statements that can be safely included in naked functions are
 -@code{asm} statements that do not have operands.  All other statements,
 -including declarations of local variables, @code{if} statements, and so
 -forth, should be avoided.  Naked functions should be used to implement the
 -body of an assembly function, while allowing the compiler to construct
 -the requisite function declaration for the assembler.
 +This attribute is available on the ARM, AVR, MCORE, MSP430, NDS32, RL78, RX 
 +and SPU ports.  It allows the compiler to construct the requisite function 
 +declaration, while allowing the body of the function to be assembly code. 
 +The specified function will not have prologue/epilogue sequences generated 
 +by the compiler; it is up to the programmer to provide these sequences if 
 +the function requires them. The expectation is that only Basic @code{asm} 
 +statements will be included in naked functions (@pxref{Basic Asm}). While it 
 +is discouraged, it is possible to write your own prologue/epilogue code 
 +using asm and use ``C'' code in the middle.

I wouldn't remove the last sentence since IMO it's not the intent of the feature
to ever support that and the compiler doesn't guarantee it and may result
in wrong code given that `naked' is a fragile low-level feature.

  
  @item near
  @cindex functions that do not handle memory bank switching on 68HC11/68HC12

Cheers,
Carlos.


[PATCH] MAINTAINERS: Update email address.

2013-08-09 Thread Carlos O'Donell
I'm working at Red Hat now as the glibc team lead within the tools group.

Please feel free to reach out to me if you have any glibc related questions.

Still having fun working on GNU tools :-)

Committed.

2013-08-09  Carlos O'Donell  car...@redhat.com

* MAINTAINERS (Write After Approval): Update email.

Index: MAINTAINERS
===
--- MAINTAINERS (revision 201643)
+++ MAINTAINERS (working copy)
@@ -478,7 +478,7 @@
 Dan Nicolaescu d...@ics.uci.edu
 Dorit Nuzman   do...@il.ibm.com
 David O'Brien  obr...@freebsd.org
-Carlos O'Donellcar...@codesourcery.com
+Carlos O'Donellcar...@redhat.com
 Peter O'Gorman po...@thewrittenword.com
 Andrea Ornsteinandrea.ornst...@st.com
 Seongbae Park  seongbae.p...@gmail.com
---

Cheers,
Carlos.


Re: [PATCH, libstdc++] Fix 22_locale/time_get/get_weekday/char/38081-[12].cc tests for glibc 2.17

2013-02-11 Thread Carlos O'Donell
On 02/11/2013 12:28 PM, H.J. Lu wrote:
 On Mon, Feb 11, 2013 at 9:18 AM, Paolo Carlini paolo.carl...@oracle.com 
 wrote:
 On 02/11/2013 04:33 PM, Julian Brown wrote:

 Hi,

 It seems that glibc 2.17 changes the abbreviated names of weekdays for
 ru_RU locales by removing an extraneous ., as described in:

 http://sourceware.org/bugzilla/show_bug.cgi?id=10873

 An earlier patch (circa glibc 2.14) changed (IIUC!) archaic/unusual
 three-letter abbreviations to more-common two-letter abbreviations, but
 included dots after each weekday name, which was apparently still wrong.
 But, the two tests of this feature in the libstdc++ testsuite expect
 those dots to be present, so they fail.

 So, the attached patch simply removes the expectation that dots are
 present in the abbreviated names from the libstdc++ tests in question,
 if the glibc version in use is recent enough.

 The tests pass (with a current gcc, trunk eglibc) with the attached
 patch, and fail (for me) without it (cross-testing to ARM Linux, for
 no particular reason). OK to apply?

 I think it's Ok, yes. Thanks. However, I would appreciate if somebody with a
 glibc 2.17 system at hand could double check. Maybe HJ?

 
 I am not familiar with locale.  CC to glibc mailing list.

Paolo,

glibc 2.17 and eglibc 2.17 are so close that it should be fine
to test just one. I'd think the change should be fine for libstdc++.

Upstream will be working to reduce the differences between eglibc
and glibc so eventually these reports will just say glibc in
their testing notes.

Does that make sense?

Cheers,
Carlos.


Re: PATCH to libstdc++ to use __cxa_thread_atexit_impl if available

2013-01-22 Thread Carlos O'Donell
On 01/22/2013 07:24 PM, Siddhesh Poyarekar wrote:
 Cc'ing Carlos on this so that he's aware of it.
 
 Siddhesh
 
 Jakub Jelinek ja...@redhat.com wrote:
 
 On Sat, Jan 19, 2013 at 12:22:23PM -0500, Jason Merrill wrote:
 Siddhesh has a patch to implement the thread atexit functionality in
 glibc in order to integrate better with the dynamic loader and run
 the cleanups in the correct order.  Once it's available there, this
 patch will make the copy in libsupc++ use it.  The main
 __cxa_thread_atexit function will always live in libsupc++, however,
 in order to maintain ABI compatibility between releases of
 libstdc++.

 Does this configure change look right, or should it go in linkage.m4
 somewhere?

 I think I'll hold off checking this in until Siddhesh's patch goes
 into glibc.
 
 Yeah, it should make it into glibc first (no idea why is it taking so long),
 otherwise we can't be sure it won't be implemented differently in glibc.
 
   Jakub
 

If there is a patch that needs to be reviewed immediately
then Siddhesh should ping that patch and CC me so I can
do a timely review. I'll look into this patch tomorrow.

In general we lack qualified reviewers in glibc.
We try our best to get through the incoming patches,
but given the new friendliness of the community
we're flooded with patches, but not with reviewers.

Please have patience has the community grows up.

Cheers,
Carlos.


Re: [PATCH v2] ARM: Use different linker path for hardfloat ABI

2012-04-26 Thread Carlos O'Donell
On Mon, Apr 23, 2012 at 5:36 PM, Michael Hope michael.h...@linaro.org wrote:
 2012-04-24  Michael Hope  michael.h...@linaro.org
            Richard Earnshaw  rearn...@arm.com

        * config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER_SOFT_FLOAT): Define.
        (GLIBC_DYNAMIC_LINKER_HARD_FLOAT): Define.
        (GLIBC_DYNAMIC_LINKER_DEFAULT): Define.
        (GLIBC_DYNAMIC_LINKER): Redefine to use the hard float path.

 diff --git a/gcc/config/arm/linux-eabi.h b/gcc/config/arm/linux-eabi.h
 index 80bd825..2ace6f0 100644
 --- a/gcc/config/arm/linux-eabi.h
 +++ b/gcc/config/arm/linux-eabi.h
 @@ -62,7 +62,17 @@
  /* Use ld-linux.so.3 so that it will be possible to run classic
    GNU/Linux binaries on an EABI system.  */
  #undef  GLIBC_DYNAMIC_LINKER
 -#define GLIBC_DYNAMIC_LINKER /lib/ld-linux.so.3
 +#define GLIBC_DYNAMIC_LINKER_SOFT_FLOAT /lib/ld-linux.so.3
 +#define GLIBC_DYNAMIC_LINKER_HARD_FLOAT /lib/ld-linux-armhf.so.3
 +#if TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_HARD
 +#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_HARD_FLOAT
 +#else
 +#define GLIBC_DYNAMIC_LINKER_DEFAULT GLIBC_DYNAMIC_LINKER_SOFT_FLOAT
 +#endif
 +#define GLIBC_DYNAMIC_LINKER \
 +   %{mfloat-abi=hard: GLIBC_DYNAMIC_LINKER_HARD_FLOAT } \
 +    %{mfloat-abi=soft*: GLIBC_DYNAMIC_LINKER_SOFT_FLOAT } \
 +    %{!mfloat-abi=*: GLIBC_DYNAMIC_LINKER_DEFAULT }

  /* At this point, bpabi.h will have clobbered LINK_SPEC.  We want to
    use the GNU/Linux version, not the generic BPABI version.  */

This patch is broken. Please fix this.

You can't use a named enumeration in cpp equality.

The type ARM_FLOAT_ABI_HARD is a named enumeration and evaluates to 0
as an unknown identifier.

Therefore #if TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_HARD
evaluates to #if 0 == 0 and is always true.

Watch out that #define ARM_FLOAT_ABI_HARD ARM_FLOAT_ABI_HARD for
such enums is not conforming C99/C11.

I suggest you define the types as macros and then set the named enum
to those values, then use the macros in the header equality checks.

e.g.
#define VAL1 0 then enum FOO { RVAL1 = VAL1, ... }

Look at arm.h for the enum definition.

Cheers,
Carlos.


Re: [PATCH v2] ARM: Use different linker path for hardfloat ABI

2012-04-23 Thread Carlos O'Donell
On Sun, Apr 22, 2012 at 6:20 PM, Michael Hope michael.h...@linaro.org wrote:
 Change the dynamic linker path for ARM hard float executables.
 Matches the path discussed and agreed on last week[1].  Carlos will
 follow up with the matching patch to GLIBC[2].  I'm happy to if he's
 busy.

I'm testing a glibc patch with Richard's alternate version.

If all goes well I'll post it tomorrow for review on libc-po...@sourceware.org.

Cheers,
Carlos.


Re: [PATCH] ARM: Use different linker path for hardfloat ABI

2012-04-10 Thread Carlos O'Donell
On Tue, Apr 3, 2012 at 6:56 PM, Joseph S. Myers jos...@codesourcery.com wrote:
 (e) Existing practice for cases that do use different dynamic linkers is
 to use a separate library directory, not just dynamic linker name, as in
 lib32 and lib64 for MIPS or libx32 for x32; it's certainly a lot easier to
 make two sets of libraries work in parallel if you have separate library
 directories like that.  So it would seem more appropriate to define a
 directory libhf for ARM (meaning you need a binutils patch as well to
 handle that directory, I think), and these different Debian-style names
 could be implemented separately in a multiarch patch if someone submits
 one that properly accounts for my review comments on previous patch
 versions (failure to produce such a fixed patch being why Debian multiarch
 directory support has not got into GCC so far).

The thread doesn't seem to be wrapping up, instead it appears to go in
circles :-)

As a glibc maintainer, when it comes to this issue, I am guided by:

(1) This is a distribution problem and the solution needs to come from
a consensus among the distributions.

(2) The gcc/glibc community should listen to the distributions.

The distributions have the most experience when it comes to
whole-system issues. I certainly don't have that experience.
Unfortunately *I* give the distributions a B- or C+ for communication.
Please make it easy for me to give you an A. It is exceedingly
difficult for me to review solutions that span multiple patches,
emails, mailing lists, and communities. The best way to avoid
rehashing old problems is to document the design and get sign off from
the interested parties.

If I see uncoordinated and conflicting responses from the
distributions... I get worried.

Is there a proposal on a wiki that begins to summarize the suggestions
and solution?

Cheers,
Carlos.


Re: struct siginfo vs. siginfo_t (was: GNU C Library master sources branch, master, updated. glibc-2.15-229-g4efeffc)

2012-03-15 Thread Carlos O'Donell
On Thu, Mar 15, 2012 at 11:05 AM, Thomas Schwinge
tho...@codesourcery.com wrote:
 Hi!

 On 26 Feb 2012 18:17:52 -, drep...@sourceware.org wrote:
 http://sources.redhat.com/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=4efeffc1d583597e4f52985b9747269e47b754e2

 commit 4efeffc1d583597e4f52985b9747269e47b754e2
 Author: Ulrich Drepper drep...@gmail.com
 Date:   Sun Feb 26 13:17:27 2012 -0500

     Fix up POSIX testing in conformtest

 [...]
 +     * sysdeps/unix/sysv/linux/bits/siginfo.h: Don't name siginfo_t
 +     struct.  [...]
 [...]

 diff --git a/sysdeps/unix/sysv/linux/bits/siginfo.h 
 b/sysdeps/unix/sysv/linux/bits/siginfo.h
 index ecef39d..0635e2f 100644
 --- a/sysdeps/unix/sysv/linux/bits/siginfo.h
 +++ b/sysdeps/unix/sysv/linux/bits/siginfo.h
 [...]
 @@ -47,7 +47,7 @@ typedef union sigval
  #  define __SI_PAD_SIZE     ((__SI_MAX_SIZE / sizeof (int)) - 3)
  # endif

 -typedef struct siginfo
 +typedef struct
    {
      int si_signo;            /* Signal number.  */
      int si_errno;            /* If non-zero, an errno value associated with
 [...]

 This change breaks GCC:

    In file included from 
 /scratch/tschwing/FM_sh-linux-gnu-mk2/src/gcc-mainline/libgcc/unwind-dw2.c:377:0:
    ./md-unwind-support.h: In function 'sh_fallback_frame_state':
    ./md-unwind-support.h:182:17: error: field 'info' has incomplete type

 In my case, this is really libgcc/config/sh/linux-unwind.h:

    [...]
       181            struct rt_sigframe {
       182              struct siginfo info;
       183              struct ucontext uc;
       184            } *rt_ = context-cfa;
    [...]

POSIX says you get siginto_t *not* struct siginfo, please fix the code.

 (Is there really nobody doing nightly testing of GCC against glibc master
 branch on x86, which would have shown this earlier?)

I don't test building GCC against glibc master unless I'm making a
potentially ABI breaking change.

We should be rebuilding *all* of userspace when glibc changes. It
would be nice if we setup an OpenEmbedded system to rebuild as much of
x86-64 userspace as possible against a new glibc and check for
regressions.

 Replacing struct siginfo with siginfo_t in sh/linux-unwind.h makes the
 build pass.  Is this the desired approach; shall we apply this for all
 cases listed above (and submit to Boehm GC upstream)?  I wonder -- if GCC
 breaks, how much software out in the wild is going to break once this
 glibc change ripples through?

You can't know until you try building a full distribution.

Cheers,
Carlos.


Re: [Patches] C++ linking problem when locale are disabled in eglic.

2012-03-13 Thread Carlos O'Donell
On Tue, Mar 13, 2012 at 6:43 AM, Jonathan Wakely jwakely@gmail.com wrote:
 This should have been CC'd to the libstdc++ list too.

 C++ programs don't link anymore with gcc  4.5.3 and eglibc 2.15 with 
 OPTION_EGLIBC_LOCALE_CODE=n.

 It seems that gcc/libstdc++-v3/acinclude.m4 doesn't check for bugs in early 
 glibc-2.2.x series anymore. So, it doesn't detect the lack of newlocale, 
 duplocale, freelocale, nl_langinfo_l and uselocale. On the other hand, 
 there is a new test for the presence of uclibc (!defined(__UCLIBC__)).

 I solved the problem with the '--enable-clocale=generic' option when 
 configuring gcc.

 So my question to the gcc team is : should gcc detect the lack of these 
 functions with eglibc , or using --enable-clocale=generic is the good 
 practice ?

 Ideally, yes.

 That wasn't very clear, sorry. I meant that ideally libstdc++ would
 detect the lack of the functions.  In the absence of that detection
 --enable-clocale=generic is a reasonable workaround.

As a glibc maintainer I agree with Jonathan's general statements.
We need the help of the developer community to test the various
configurations and patches are always welcome.

Cheers,
Carlos.


[RFC] Implementation and documentation of -iwithprefix do not match?

2008-09-10 Thread Carlos O'Donell

The GCC option -iwithprefix does not match the documentation.

Is there any reason for this discrepancy?

The GCC option -iwithprefix is documented in gcc/doc/cppopts.texi

@item -iwithprefix @var{dir}
@itemx -iwithprefixbefore @var{dir}
@opindex iwithprefix
@opindex iwithprefixbefore
Append @var{dir} to the prefix specified previously with
@option{-iprefix}, and add the resulting directory to the include search
path.  @option{-iwithprefixbefore} puts it in the same place @option{-I}
would; @option{-iwithprefix} puts it where @option{-idirafter} would.

However, -iwithprefix does not put the search path where -idirafter 
would, instead it puts the search path at the top of the SYSTEM chain.


See gcc/c-opts.c:
...
case OPT_idirafter:
  add_path (xstrdup (arg), AFTER, 0, true);
  break;
...
case OPT_iwithprefix:
  add_prefixed_path (arg, SYSTEM);
  break;

The order of the compilers SYSTEM paths is important to the correct 
processing of C++ header #include_next macros.


If you use -iwithprefix to modify the compilers -iprefix path to define 
a header that matches a SYSTEM header, the compiler removes the 
redundant SYSTEM path definition and leaves the one generated by 
-iwithprefix (it's the same path just in a different search order). 
However, since -iwithprefix moved the header definition to the *top* of 
the search list, when #include_next is used, it may result in a missing 
header.


If -iwithprefix implemented what was documented, then it appears that 
there would be no issue.


Cheers,
Carlos.
--
Carlos O'Donell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x716


Re: Possible GCC 4.3 driver regression caused by your patch

2008-03-02 Thread Carlos O'Donell

Greg Schafer wrote:

Hi Carlos and Mark,

Your Relocated compiler should not look in $prefix patch here:

http://gcc.gnu.org/ml/gcc/2006-10/msg00280.html

appears to have caused a regression in my GCC 4.3 testing.

In summary, there is a small window *during the GCC build itself* where GCC
does not pick up the correct startfiles. For example, when GCC_FOR_TARGET is
called to build the target libraries, the startfiles in $prefix/lib are not
used. Instead, the startfiles from the host's /usr/lib are used which breaks
my build. Note that the problem seems to rectify itself once the just-built
GCC is installed into $prefix.

Here's the scenario:

 - Native build
 - i686-pc-linux-gnu
 - --prefix=/temptools
 - Glibc already installed in /temptools/lib


What options did you use to configure the compiler? Could you double 
check your host system is actually i686-pc-linux-gnu? When you say 
breaks my build, what error are you seeing?


Cheers,
Carlos.
--
Carlos O'Donell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x716


Re: History of m68k ABI and `-malign-int'?

2008-01-17 Thread Carlos O'Donell
On Thu, Jan 17, 2008 at 01:08:16AM +0100, Andreas Schwab wrote:
 Carlos O'Donell [EMAIL PROTECTED] writes:
 
  Why is 16-bit int alignment the default for m68k in gcc?
 
  Which ABIs influenced this decision?
 
 The original ABI was defined by Sun PCC.  Note that the SysV ABI
 actually uses natural alignment for all types, but that came only much
 later.
 
 You can find much of the history in the change logs of gcc 1.42.

Excellent, that scratches my itch, thanks for providing a history lesson
and a good pointer.

Cheers,
Carlos.
-- 
Carlos O'Donell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x716


History of m68k ABI and `-malign-int'?

2008-01-16 Thread Carlos O'Donell

Andreas, Jeff,

The documentation for `-malign-int' on m68k hints that 16-bit int
alignment was the prominent alignment chosen by m68k ABI's of the era.

Why is 16-bit int alignment the default for m68k in gcc?

Which ABIs influenced this decision?

Should Coldfire targets default to 16 or 32-bit int alignment?

Cheers,
Carlos.
-- 
Carlos O'Donell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x716


Re: [PATCH] Relocated compiler should not look in $prefix.

2006-11-13 Thread Carlos O'Donell
On Mon, Oct 16, 2006 at 03:32:27PM -0700, Mark Mitchell wrote:
 Ian Lance Taylor wrote:
 Carlos O'Donell [EMAIL PROTECTED] writes:
 
 A relocated compiler should not look in $prefix.
 
 I agree.
 
 I can't approve your patches, though.
 
 This patch is OK, once we reach Stage 1.
 
Checked into mainline.
Bootstrapped with no regressions on i686-pc-linux-gnu.

Cheers,
Carlos.
-- 
Carlos O'Donell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x716


Re: pr27650 - dllimport of virtual methods broken.

2006-09-20 Thread Carlos O'Donell
On Tue, Sep 19, 2006 at 10:09:49AM +1200, Danny Smith wrote:
 Revised patch. 

Thanks Danny!
 
 Tested on i686-pc-mingw32
 
 Danny
 
 cp/ChangeLog
 
   PR target/27650
   * class.c (check_for_override): Remove dllimport from virtual
   methods.
 
 testsuite/Changelog
   PR target/27650
   * g++.dg/ext/dllimport12.C: New file.

Mark,

What is your opinion on this patch?

Cheers,
Carlos.
-- 
Carlos O'Donell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x716


pr27650 - dllimport of virtual methods broken.

2006-09-13 Thread Carlos O'Donell

Is any of you able to give some comments on pr27650

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27650

In particular I am interested in an opinion of Danny's fix.

http://gcc.gnu.org/ml/gcc-patches/2006-05/msg01504.html

I definately don't know enough about attributes and dllimport to
comment. The fix works, but is it correct?

Cheers,
Carlos.
-- 
Carlos O'Donell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x716


Re: Imported GNU Classpath 0.92

2006-08-23 Thread Carlos O'Donell
On Wed, Aug 23, 2006 at 08:47:32AM +0200, Mark Wielaard wrote:
 On Tue, 2006-08-22 at 16:33 -0400, Carlos O'Donell wrote:
  Has the 'make html' target been fixed? I would like to enable this
  target so that html support doesn't bitrot.
 
 No sorry. I didn't know it was broken. I see that it only works when
 configuring with --with-gjdoc. If there is already a bug report about
 this could you assign it to me?

There isn't a pr about this, I'll file one now. Thanks Mark.

Filed as pr28822.
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28822

Cheers,
Carlos.
-- 
Carlos O'Donell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x716


Re: Imported GNU Classpath 0.92

2006-08-22 Thread Carlos O'Donell
On Tue, Aug 15, 2006 at 01:18:55AM +0200, Mark Wielaard wrote:
 GNU Classpath 0.92 was released last week. It contains a lot of new
 standard library classes and bug fixes. See
 http://savannah.gnu.org/forum/forum.php?forum_id=4573
 And the list of fixed bugs:
 http://gcc.gnu.org/bugzilla/buglist.cgi?product=classpathtarget_milestone=0.92
 
 This version has been imported now into the libjava directory on the
 trunk. It has been tested on a variety of platforms. But if we missed
 something please let us know ([EMAIL PROTECTED]).

Has the 'make html' target been fixed? I would like to enable this
target so that html support doesn't bitrot.

Cheers,
Carlos.
-- 
Carlos O'Donell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x716


Re: Bootstrap broken on ppc-darwin

2006-07-17 Thread Carlos O'Donell
On Sun, Jul 16, 2006 at 06:05:32PM -0700, Ian Lance Taylor wrote:
 Andrew Pinski [EMAIL PROTECTED] writes:
 
  Here we have the same scope_labelno.  The first dbxout_begin_prologue
  comes from calling rs6000_output_mi_thunk.  The normal way
  scope_labelno gets incremented is via the
  call to debug_hooks-function_decl in rest_of_handle_final which is
  not done for thunks.
  I don't know if we should call debug_hooks-function_decl for thunks
  or not.
 
 We shouldn't.  It doesn't make sense, since there is no proper
 current_function_decl for a thunk.

OK.
 
 Previously, scope_labelno was referenced in dbxout_block and
 incremented in dbxout_function_end.  Both functions are called only by
 dbxout_function_decl (a debug hook).  So it was always consistent.

Yes, that's correct.

 Now scope_labelno is used by dbxout_begin_prologue and
 dbxout_source_line.  Those are both debug hooks themselves.  So this
 patch has introduced a dependency which was not previously there,
 which has led to a bug.

Sorry about that, I tried to make sure that scope_labelno was
consistent.

 There are several ways to fix this, of course.  I think the simplest
 is going to be to always preincrement scope_labelno in
 dbxout_begin_prologue, rather than postincrementing it in
 dbxout_function_end.  In cases where that fails, we are already in
 trouble.

I'm testing a patch for this right now.

 Note that scope_labelno is now used for two different things: for the
 LFBB symbol, and for the Lscope symbol.  It does not have to be used
 for both, although as far as I can see it does no harm.

Using scope_labelno for both cases made the patch clearer. I wanted the
patch to be as simple as possible.

Sorry I missed thinking about the thunks.

I'll get back ASAP when my current patch finishes testing.

Cheers,
Carlos.
-- 
Carlos O'Donell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x716


Re: Bootstrap broken on ppc-darwin

2006-07-17 Thread Carlos O'Donell
On Sun, Jul 16, 2006 at 06:05:32PM -0700, Ian Lance Taylor wrote:
 Previously, scope_labelno was referenced in dbxout_block and
 incremented in dbxout_function_end.  Both functions are called only by
 dbxout_function_decl (a debug hook).  So it was always consistent.
 
 Now scope_labelno is used by dbxout_begin_prologue and
 dbxout_source_line.  Those are both debug hooks themselves.  So this
 patch has introduced a dependency which was not previously there,
 which has led to a bug.
 
 There are several ways to fix this, of course.  I think the simplest
 is going to be to always preincrement scope_labelno in
 dbxout_begin_prologue, rather than postincrementing it in
 dbxout_function_end.  In cases where that fails, we are already in
 trouble.
 
 Note that scope_labelno is now used for two different things: for the
 LFBB symbol, and for the Lscope symbol.  It does not have to be used
 for both, although as far as I can see it does no harm.

The following patch does a pre-increment of scope_labelno in
dbxout_begin_prologue.

No regressions on arm-none-eabi, and I verified the output stabs by
hand. The .Lscope and .LFBB labels start at 1, and increment for each
call to dbxout_begin_prologue.

Andrew, I built a powerpc-darwin cross compiler and manually verified
the output assembly from your testcase. It looks like it should fix your
problem. I don't see any repeated LFBB symbols, where previously I saw
repeated LFBB1 and LFBB4 due to the thunks.

OK to commit? 

Cheers,
Carlos.
-- 
Carlos O'Donell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x716

2006-07-17  Carlos O'Donell  [EMAIL PROTECTED]

* dbxout.c (dbxout_function_end): Do not increment scope_labelno.
(dbxout_begin_prologue): Increment scope_labelno.

Index: gcc/dbxout.c
===
--- gcc/dbxout.c(revision 115532)
+++ gcc/dbxout.c(working copy)
@@ -905,7 +905,6 @@ static void
 dbxout_function_end (tree decl)
 {
   char lscope_label_name[100];
-  int lscope_labelno = scope_labelno++;
 
   /* The Lscope label must be emitted even if we aren't doing anything
  else; dbxout_block needs it.  */
@@ -914,8 +913,8 @@ dbxout_function_end (tree decl)
   /* Convert Lscope into the appropriate format for local labels in case
  the system doesn't insert underscores in front of user generated
  labels.  */
-  ASM_GENERATE_INTERNAL_LABEL (lscope_label_name, Lscope, lscope_labelno);
-  targetm.asm_out.internal_label (asm_out_file, Lscope, lscope_labelno);
+  ASM_GENERATE_INTERNAL_LABEL (lscope_label_name, Lscope, scope_labelno);
+  targetm.asm_out.internal_label (asm_out_file, Lscope, scope_labelno);
 
   /* The N_FUN tag at the end of the function is a GNU extension,
  which may be undesirable, and is unnecessary if we do not have
@@ -941,7 +940,7 @@ dbxout_function_end (tree decl)
 {
   char begin_label[20];
   /* Reference current function start using LFBB.  */
-  ASM_GENERATE_INTERNAL_LABEL (begin_label, LFBB, lscope_labelno);
+  ASM_GENERATE_INTERNAL_LABEL (begin_label, LFBB, scope_labelno);
   dbxout_begin_empty_stabs (N_FUN);
   dbxout_stab_value_label_diff (lscope_label_name, begin_label);
 }
@@ -1249,6 +1248,9 @@ dbxout_begin_prologue (unsigned int line
!flag_debug_only_used_symbols)
 dbxout_stabd (N_BNSYM, 0);
 
+  /* pre-increment the scope counter */
+  scope_labelno++;
+
   dbxout_source_line (lineno, filename);
   /* Output function begin block at function scope, referenced 
  by dbxout_block, dbxout_source_line and dbxout_function_end.  */


Searching configured and relocated prefix.

2006-07-14 Thread Carlos O'Donell

GCC,

We currently search both the relocated compilers prefix and the
originally configured prefix. Should a relocated compiler be searching
both directories?

Does anyone see a need to search *both* the configured prefix and the
relocated prefix?  You can end up finding things you don't mean to find,
you can get NFS time-outs, etc.

However, the strategy above will break situations where people expect to
find some of their bits in a directory indicated by GCC_EXEC_PREFIX, and
the rest in the configured prefix.  

Cheers,
Carlos.
-- 
Carlos O'Donell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x716


Examples of callee returning struct, but caller allocating space?

2005-12-23 Thread Carlos O'Donell

The SPARC psABI defines that a caller must allocate the space for a
structure returned from the callee. If the callee sees the size marker
in the allocation matches the size of the return then it fills the slot.
If the size matches we return, if it doesn't match we return anyway but
add a fixed offset to the return (like a non-local goto).

There are some optimiations that can be done...

The caller, knowing that nobody uses the result can set the size marker
to zero. The callee should check for the size marker and write nothing
to the slot, returning to the normal location plus the offset.

The callee can assume the caller allocated the space and always write to
the slot (no size check) and return to the correct position in the
caller.

Is there any example of this type of behaviour in any other ports?
I'm just looking for implementation examples.

The root cause of this research is that code like this:

#include stdlib.h

div_t
div(int num, int denom)
{
div_t r;

r.quot = num / denom;
r.rem = num % denom;
return (r);
}

Produces asm that doesn't check the callers size marker in the allocated
return slot. If the caller and callee are compiled separately then an
optimization in the caller could cause a problem. Rightly though the
caller can't optimize without breaking ABI. Rightly the callee can't
opimize away the size check without breaking ABI. Without coordination
we can't safely remove the checks.

Cheers,
Carlos.
-- 
Carlos O'Donell 
 
CodeSourcery, LLC   

[EMAIL PROTECTED]