diff mbox

Fix declaration of pthread-structs in s-osinte-rtems.ads

Message ID 8173705.y9mlaLCnmB@kubuntu
State New
Headers show

Commit Message

Jan Sommer Oct. 31, 2015, 3:47 p.m. UTC 
Hi,

This patch changes the Ada-declaration of the pthread-related structs such as pthread_attr_t from a field-equivalent declaration to just reserving the right amount of memory.
It is only rtems related and essentially copies the way how the types are defined in s-osinte-linux.ads. It makes the declarations independent of a particular newlib-version and fixes the bug I filed here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68169

CC are the rtems developers for discussion.

Best regards,

   Jan

Comments

Jan Sommer Oct. 31, 2015, 3:54 p.m. UTC | #1 
Am Saturday 31 October 2015, 16:47:35 schrieb Jan Sommer:
> Hi,
> 
> This patch changes the Ada-declaration of the pthread-related structs such as pthread_attr_t from a field-equivalent declaration to just reserving the right amount of memory.
> It is only rtems related and essentially copies the way how the types are defined in s-osinte-linux.ads. It makes the declarations independent of a particular newlib-version and fixes the bug I filed here:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68169
> 
> CC are the rtems developers for discussion.
> 

I forgot to add: The patch is based on the current gcc 4.9 branch
Arnaud Charlet Oct. 31, 2015, 5:11 p.m. UTC | #2 
> > This patch changes the Ada-declaration of the pthread-related structs
> > such as pthread_attr_t from a field-equivalent declaration to just
> > reserving the right amount of memory.
> > It is only rtems related and essentially copies the way how the types are
> > defined in s-osinte-linux.ads. It makes the declarations independent
> > of a particular newlib-version and fixes the bug I filed here:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68169
> > 
> > CC are the rtems developers for discussion.
> 
> I forgot to add: The patch is based on the current gcc 4.9 branch

Well you need to submit patches against trunk (and if needed backport them)
rather than the other way around.

Also, you need to provide a ChangeLog in the proper format.

Arno
Joel Sherrill Nov. 1, 2015, 4:26 p.m. UTC | #3 
On 10/31/2015 10:47 AM, Jan Sommer wrote:
> Hi,
>
> This patch changes the Ada-declaration of the pthread-related structs such as pthread_attr_t from a field-equivalent declaration to just reserving the right amount of memory.
> It is only rtems related and essentially copies the way how the types are defined in s-osinte-linux.ads. It makes the declarations independent of a particular newlib-version and fixes the bug I filed here:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68169
>
> CC are the rtems developers for discussion.
>
I am ok with this patch unless Arnaud says otherwise.

I would like to apply it to 4.9 and newer.

Comments?

> Best regards,
>
>     Jan
Jan Sommer Nov. 2, 2015, 10:57 a.m. UTC | #4 
Am Saturday 31 October 2015, 18:11:47 schrieb Arnaud Charlet:
> > > This patch changes the Ada-declaration of the pthread-related structs
> > > such as pthread_attr_t from a field-equivalent declaration to just
> > > reserving the right amount of memory.
> > > It is only rtems related and essentially copies the way how the types are
> > > defined in s-osinte-linux.ads. It makes the declarations independent
> > > of a particular newlib-version and fixes the bug I filed here:
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68169
> > > 
> > > CC are the rtems developers for discussion.
> > 
> > I forgot to add: The patch is based on the current gcc 4.9 branch
> 
> Well you need to submit patches against trunk (and if needed backport them)
> rather than the other way around.
> 
> Also, you need to provide a ChangeLog in the proper format.
> 

Ok, I don't have time today. I will make a patch against trunk and will try again with the correct format tomorrow.
How does the backporting work?
It's my first contribution to gcc, so bare with me ;-)

Best regards,

   Jan
Arnaud Charlet Nov. 2, 2015, 10:59 a.m. UTC | #5 
> Ok, I don't have time today. I will make a patch against trunk and will try
> again with the correct format tomorrow.
> How does the backporting work?
> It's my first contribution to gcc, so bare with me ;-)

See https://gcc.gnu.org/contribute.html for details.

Arno
Sebastian Huber Nov. 2, 2015, 11:39 a.m. UTC | #6 
On 31/10/15 16:47, Jan Sommer wrote:
> Hi,
>
> This patch changes the Ada-declaration of the pthread-related structs such as pthread_attr_t from a field-equivalent declaration to just reserving the right amount of memory.
> It is only rtems related and essentially copies the way how the types are defined in s-osinte-linux.ads. It makes the declarations independent of a particular newlib-version and fixes the bug I filed here:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68169

[...]

>      -------------
>      -- Signals --
> @@ -448,6 +450,7 @@ package System.OS_Interface is
>         ss_low_priority     : int;
>         ss_replenish_period : timespec;
>         ss_initial_budget   : timespec;
> +      sched_ss_max_repl   : int;
>      end record;
>      pragma Convention (C, struct_sched_param);

Why is this structure not changed to an opaque size + alignment type 
like the other structures?

>   
> @@ -621,43 +624,34 @@ private
>      end record;
>      pragma Convention (C, timespec);
>   
> -   CLOCK_REALTIME :  constant clockid_t := 1;
> -   CLOCK_MONOTONIC : constant clockid_t := 4;
> +   CLOCK_REALTIME :  constant clockid_t := System.OS_Constants.CLOCK_REALTIME;
> +   CLOCK_MONOTONIC : constant clockid_t := System.OS_Constants.CLOCK_MONOTONIC;
> +
> +   subtype char_array is Interfaces.C.char_array;
>   
>      type pthread_attr_t is record
> -      is_initialized  : int;
> -      stackaddr       : System.Address;
> -      stacksize       : int;
> -      contentionscope : int;
> -      inheritsched    : int;
> -      schedpolicy     : int;
> -      schedparam      : struct_sched_param;
> -      cputime_clocked_allowed : int;
> -      detatchstate    : int;
> +      Data : char_array (1 .. OS_Constants.PTHREAD_ATTR_SIZE);
>      end record;
>      pragma Convention (C, pthread_attr_t);
> +   for pthread_attr_t'Alignment use Interfaces.C.unsigned_long'Alignment;
>   
>      type pthread_condattr_t is record
> -      flags           : int;
> -      process_shared  : int;
> +      Data : char_array (1 .. OS_Constants.PTHREAD_CONDATTR_SIZE);
>      end record;
>      pragma Convention (C, pthread_condattr_t);
> +   for pthread_condattr_t'Alignment use Interfaces.C.int'Alignment;
>   
>      type pthread_mutexattr_t is record
> -      is_initialized  : int;
> -      process_shared  : int;
> -      prio_ceiling    : int;
> -      protocol        : int;
> -      mutex_type      : int;
> -      recursive       : int;
> -   end record;
> +      Data : char_array (1 .. OS_Constants.PTHREAD_MUTEXATTR_SIZE);
> +   end  record;
>      pragma Convention (C, pthread_mutexattr_t);
> +   for pthread_mutexattr_t'Alignment use Interfaces.C.int'Alignment;
[...]

The alignment is sometimes int and sometimes unsigned long. I would 
change this to long long or double throughout, e.g. if we change the CPU 
mask type to uint64_t, then the alignment specified here is no longer 
correct.
Jan Sommer Nov. 3, 2015, 11:36 a.m. UTC | #7 
Am Monday 02 November 2015, 12:39:57 schrieb Sebastian Huber:
> 
> On 31/10/15 16:47, Jan Sommer wrote:
> > Hi,
> >
> > This patch changes the Ada-declaration of the pthread-related structs such as pthread_attr_t from a field-equivalent declaration to just reserving the right amount of memory.
> > It is only rtems related and essentially copies the way how the types are defined in s-osinte-linux.ads. It makes the declarations independent of a particular newlib-version and fixes the bug I filed here:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68169
> 
> [...]
> 
> >      -------------
> >      -- Signals --
> > @@ -448,6 +450,7 @@ package System.OS_Interface is
> >         ss_low_priority     : int;
> >         ss_replenish_period : timespec;
> >         ss_initial_budget   : timespec;
> > +      sched_ss_max_repl   : int;
> >      end record;
> >      pragma Convention (C, struct_sched_param);
> 
> Why is this structure not changed to an opaque size + alignment type 
> like the other structures?
> 

There is no corresponding size constant in s-oscons.ads. The linux version of s-osinte.ads uses a record declaration too.

> >   
> > @@ -621,43 +624,34 @@ private
> >      end record;
> >      pragma Convention (C, timespec);
> >   
> > -   CLOCK_REALTIME :  constant clockid_t := 1;
> > -   CLOCK_MONOTONIC : constant clockid_t := 4;
> > +   CLOCK_REALTIME :  constant clockid_t := System.OS_Constants.CLOCK_REALTIME;
> > +   CLOCK_MONOTONIC : constant clockid_t := System.OS_Constants.CLOCK_MONOTONIC;
> > +
> > +   subtype char_array is Interfaces.C.char_array;
> >   
> >      type pthread_attr_t is record
> > -      is_initialized  : int;
> > -      stackaddr       : System.Address;
> > -      stacksize       : int;
> > -      contentionscope : int;
> > -      inheritsched    : int;
> > -      schedpolicy     : int;
> > -      schedparam      : struct_sched_param;
> > -      cputime_clocked_allowed : int;
> > -      detatchstate    : int;
> > +      Data : char_array (1 .. OS_Constants.PTHREAD_ATTR_SIZE);
> >      end record;
> >      pragma Convention (C, pthread_attr_t);
> > +   for pthread_attr_t'Alignment use Interfaces.C.unsigned_long'Alignment;
> >   
> >      type pthread_condattr_t is record
> > -      flags           : int;
> > -      process_shared  : int;
> > +      Data : char_array (1 .. OS_Constants.PTHREAD_CONDATTR_SIZE);
> >      end record;
> >      pragma Convention (C, pthread_condattr_t);
> > +   for pthread_condattr_t'Alignment use Interfaces.C.int'Alignment;
> >   
> >      type pthread_mutexattr_t is record
> > -      is_initialized  : int;
> > -      process_shared  : int;
> > -      prio_ceiling    : int;
> > -      protocol        : int;
> > -      mutex_type      : int;
> > -      recursive       : int;
> > -   end record;
> > +      Data : char_array (1 .. OS_Constants.PTHREAD_MUTEXATTR_SIZE);
> > +   end  record;
> >      pragma Convention (C, pthread_mutexattr_t);
> > +   for pthread_mutexattr_t'Alignment use Interfaces.C.int'Alignment;
> [...]
> 
> The alignment is sometimes int and sometimes unsigned long. I would 
> change this to long long or double throughout, e.g. if we change the CPU 
> mask type to uint64_t, then the alignment specified here is no longer 
> correct.
> 

Thanks for the tip. I will change that.

Best regards,

   Jan
diff mbox

Patch

From 6b445bc37bf59641f46c01ec64a248efb5ec40f4 Mon Sep 17 00:00:00 2001
From: Jan Sommer <soja-lists@aries.uberspace.de>
Date: Sat, 31 Oct 2015 16:25:14 +0100
Subject: [PATCH 2/2] Update type definitions to use the constant values from
 s-oscons.ads

Currently the pthread type definitions list all fields of the corresponding c-struct in Ada.
However, at least pthread_attr_t has more fields in current newlib than in the Ada declaration here.

This change will declare the pthread-related structs in the same way as in s-osinte-linux.ads by using fixed length char-arrays with the length obtained from s-oscons.ads.
This way the declaration is independent of a certain changes in newlib.

It also replaces some hardcoded errnos with the corresponding constants from s-oscons.ads

Fixes bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68169
---
 gcc/ada/s-osinte-rtems.ads | 50 ++++++++++++++++++++--------------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/gcc/ada/s-osinte-rtems.ads b/gcc/ada/s-osinte-rtems.ads
index 8b9ae12..992254f 100644
--- a/gcc/ada/s-osinte-rtems.ads
+++ b/gcc/ada/s-osinte-rtems.ads
@@ -51,6 +51,8 @@ 
 --  It is designed to be a bottom-level (leaf) package.
 
 with Interfaces.C;
+with System.OS_Constants;
+
 package System.OS_Interface is
    pragma Preelaborate;
 
@@ -60,6 +62,7 @@  package System.OS_Interface is
    subtype rtems_id       is Interfaces.C.unsigned;
 
    subtype int            is Interfaces.C.int;
+   subtype char           is Interfaces.C.char;
    subtype short          is Interfaces.C.short;
    subtype long           is Interfaces.C.long;
    subtype unsigned       is Interfaces.C.unsigned;
@@ -68,7 +71,6 @@  package System.OS_Interface is
    subtype unsigned_char  is Interfaces.C.unsigned_char;
    subtype plain_char     is Interfaces.C.plain_char;
    subtype size_t         is Interfaces.C.size_t;
-
    -----------
    -- Errno --
    -----------
@@ -76,11 +78,11 @@  package System.OS_Interface is
    function errno return int;
    pragma Import (C, errno, "__get_errno");
 
-   EAGAIN    : constant := 11;
-   EINTR     : constant := 4;
-   EINVAL    : constant := 22;
-   ENOMEM    : constant := 12;
-   ETIMEDOUT : constant := 116;
+   EAGAIN    : constant := System.OS_Constants.EAGAIN;
+   EINTR     : constant := System.OS_Constants.EINTR;
+   EINVAL    : constant := System.OS_Constants.EINVAL;
+   ENOMEM    : constant := System.OS_Constants.ENOMEM;
+   ETIMEDOUT : constant := System.OS_Constants.ETIMEDOUT;
 
    -------------
    -- Signals --
@@ -448,6 +450,7 @@  package System.OS_Interface is
       ss_low_priority     : int;
       ss_replenish_period : timespec;
       ss_initial_budget   : timespec;
+      sched_ss_max_repl   : int;
    end record;
    pragma Convention (C, struct_sched_param);
 
@@ -621,43 +624,34 @@  private
    end record;
    pragma Convention (C, timespec);
 
-   CLOCK_REALTIME :  constant clockid_t := 1;
-   CLOCK_MONOTONIC : constant clockid_t := 4;
+   CLOCK_REALTIME :  constant clockid_t := System.OS_Constants.CLOCK_REALTIME;
+   CLOCK_MONOTONIC : constant clockid_t := System.OS_Constants.CLOCK_MONOTONIC;
+
+   subtype char_array is Interfaces.C.char_array;
 
    type pthread_attr_t is record
-      is_initialized  : int;
-      stackaddr       : System.Address;
-      stacksize       : int;
-      contentionscope : int;
-      inheritsched    : int;
-      schedpolicy     : int;
-      schedparam      : struct_sched_param;
-      cputime_clocked_allowed : int;
-      detatchstate    : int;
+      Data : char_array (1 .. OS_Constants.PTHREAD_ATTR_SIZE);
    end record;
    pragma Convention (C, pthread_attr_t);
+   for pthread_attr_t'Alignment use Interfaces.C.unsigned_long'Alignment;
 
    type pthread_condattr_t is record
-      flags           : int;
-      process_shared  : int;
+      Data : char_array (1 .. OS_Constants.PTHREAD_CONDATTR_SIZE);
    end record;
    pragma Convention (C, pthread_condattr_t);
+   for pthread_condattr_t'Alignment use Interfaces.C.int'Alignment;
 
    type pthread_mutexattr_t is record
-      is_initialized  : int;
-      process_shared  : int;
-      prio_ceiling    : int;
-      protocol        : int;
-      mutex_type      : int;
-      recursive       : int;
-   end record;
+      Data : char_array (1 .. OS_Constants.PTHREAD_MUTEXATTR_SIZE);
+   end  record;
    pragma Convention (C, pthread_mutexattr_t);
+   for pthread_mutexattr_t'Alignment use Interfaces.C.int'Alignment;
 
    type pthread_rwlockattr_t is record
-      is_initialized  : int;
-      process_shared  : int;
+      Data : char_array (1 .. OS_Constants.PTHREAD_RWLOCKATTR_SIZE);
    end record;
    pragma Convention (C, pthread_rwlockattr_t);
+   for pthread_rwlockattr_t'Alignment use Interfaces.C.unsigned_long'Alignment;
 
    type pthread_t is new rtems_id;
 
-- 
2.5.0