Bug 82384 - [8 Regression] s-taprop.adb failed to compile for x32
Summary: [8 Regression] s-taprop.adb failed to compile for x32
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: ada (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: 8.0
Assignee: Eric Botcazou
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-01 20:47 UTC by H.J. Lu
Modified: 2017-10-03 10:05 UTC (History)
2 users (show)

See Also:
Host:
Target: x86-64
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-10-02 00:00:00


Attachments
Tentative untested fix (228 bytes, patch)
2017-10-02 07:40 UTC, Pierre-Marie de Rodat
Details | Diff
I am testing this patch. (356 bytes, patch)
2017-10-02 10:37 UTC, H.J. Lu
Details | Diff
Better fix (545 bytes, patch)
2017-10-02 16:03 UTC, Eric Botcazou
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2017-10-01 20:47:38 UTC
s-taprop.adb failed to compile for x32:

s-taprop.adb:341:29: operator for type "System.Linux.time_t" is not directly vis
ible
s-taprop.adb:341:29: add with_clause and use_clause for "Linux"

which is caused by

2017-09-25  Doug Rupp  <rupp@adacore.com>

        * libgnarl/s-taprop__linux.adb (Base_Monotonic_Clock): New variable.
        (Compute_Base_Monotonic_Clock): New function.
        (Timed_Sleep): Adjust to use Base_Monotonic_Clock.
        (Timed_Delay): Likewise.
        (Monotonic_Clock): Likewise.
        * s-oscons-tmplt.c (CLOCK_MONOTONIC): Use on Linux.
Comment 1 Pierre-Marie de Rodat 2017-10-02 07:40:26 UTC
Created attachment 42273 [details]
Tentative untested fix

Hello,

Thank you for reporting this. Could you please check that the attached untested patch fixes the issue you are reporting? (or tell me how to reproduce on my x86_64-linux box :-)) Thank you in advance!
Comment 2 Eric Botcazou 2017-10-02 07:46:50 UTC
.
Comment 3 Eric Botcazou 2017-10-02 07:52:26 UTC
> Thank you for reporting this. Could you please check that the attached
> untested patch fixes the issue you are reporting? (or tell me how to
> reproduce on my x86_64-linux box :-)) Thank you in advance!

I have essentially the same patch locally:

Index: s-linux__x32.ads
===================================================================
--- s-linux__x32.ads    (revision 253315)
+++ s-linux__x32.ads    (working copy)
@@ -6,7 +6,7 @@
 --                                                                          --
 --                                  S p e c                                 --
 --                                                                          --
---             Copyright (C) 2013-2017, Free Software Foundation, Inc.      --
+--          Copyright (C) 2013-2017, Free Software Foundation, Inc.         --
 --
 --                                                                          --
 -- GNARL is free software; you can  redistribute it  and/or modify it under --
@@ -45,8 +45,9 @@ package System.Linux is
    -- Time --
    ----------
 
-   type time_t       is new Long_Long_Integer;
-   subtype clockid_t is Interfaces.C.int;
+   subtype suseconds_t is Long_Long_Integer;
+   subtype time_t      is Long_Long_Integer;
+   subtype clockid_t   is Interfaces.C.int;
 
    type timespec is record
       tv_sec  : time_t;
@@ -56,7 +57,7 @@ package System.Linux is
 
    type timeval is record
       tv_sec  : time_t;
-      tv_usec : Long_Long_Integer;
+      tv_usec : suseconds_t;
    end record;
    pragma Convention (C, timeval);
 
with additional tweaks to make it resemble s-linux.ads more closely.
Comment 4 H.J. Lu 2017-10-02 08:41:28 UTC
(In reply to Pierre-Marie de Rodat from comment #1)
> Created attachment 42273 [details]
> Tentative untested fix
> 
> Hello,
> 
> Thank you for reporting this. Could you please check that the attached

I am testing it now.

> untested patch fixes the issue you are reporting? (or tell me how to
> reproduce on my x86_64-linux box :-)) Thank you in advance!

You can install x32 run-time libraries on Ubuntu and configure GCC with

--with-multilib-list=m32,m64,mx32
Comment 5 H.J. Lu 2017-10-02 10:10:12 UTC
(In reply to H.J. Lu from comment #4)
> (In reply to Pierre-Marie de Rodat from comment #1)
> > Created attachment 42273 [details]
> > Tentative untested fix
> > 
> > Hello,
> > 
> > Thank you for reporting this. Could you please check that the attached
> 
> I am testing it now.

It doesn't work:

[hjl@gnu-efi-2 rts_x32]$ /export/build/gnu/gcc-x32/build-x86_64-linux/./gcc/xgcc -B/export/build/gnu/gcc-x32/build-x86_64-linux/./gcc/ -B/usr/gcc-8.0.0-x32/x86_64-pc-linux-gnu/bin/ -B/usr/gcc-8.0.0-x32/x86_64-pc-linux-gnu/lib/ -isystem /usr/gcc-8.0.0-x32/x86_64-pc-linux-gnu/include -isystem /usr/gcc-8.0.0-x32/x86_64-pc-linux-gnu/sys-include    -c -g -O2 -mx32 -fpic  -W -Wall -gnatpg -nostdinc -mx32  s-osinte.adb -o s-osinte.o
s-osinte.adb:93:07: warning: ineffective use clause for type "System.Linux.time_t"
[hjl@gnu-efi-2 rts_x32]$
Comment 6 H.J. Lu 2017-10-02 10:10:33 UTC
(In reply to Eric Botcazou from comment #3)
> > Thank you for reporting this. Could you please check that the attached
> > untested patch fixes the issue you are reporting? (or tell me how to
> > reproduce on my x86_64-linux box :-)) Thank you in advance!
> 
> I have essentially the same patch locally:
> 
> Index: s-linux__x32.ads
> ===================================================================
> --- s-linux__x32.ads    (revision 253315)
> +++ s-linux__x32.ads    (working copy)
> @@ -6,7 +6,7 @@
>  --                                                                         
> --
>  --                                  S p e c                                
> --
>  --                                                                         
> --
> ---             Copyright (C) 2013-2017, Free Software Foundation, Inc.     
> --
> +--          Copyright (C) 2013-2017, Free Software Foundation, Inc.        
> --
>  --
>  --                                                                         
> --
>  -- GNARL is free software; you can  redistribute it  and/or modify it under
> --
> @@ -45,8 +45,9 @@ package System.Linux is
>     -- Time --
>     ----------
>  
> -   type time_t       is new Long_Long_Integer;
> -   subtype clockid_t is Interfaces.C.int;
> +   subtype suseconds_t is Long_Long_Integer;
> +   subtype time_t      is Long_Long_Integer;
> +   subtype clockid_t   is Interfaces.C.int;
>  
>     type timespec is record
>        tv_sec  : time_t;
> @@ -56,7 +57,7 @@ package System.Linux is
>  
>     type timeval is record
>        tv_sec  : time_t;
> -      tv_usec : Long_Long_Integer;
> +      tv_usec : suseconds_t;
>     end record;
>     pragma Convention (C, timeval);
>  
> with additional tweaks to make it resemble s-linux.ads more closely.

I am testing this now.
Comment 7 Pierre-Marie de Rodat 2017-10-02 10:35:19 UTC
I think your second test will also fail for the same reason. I suggest you also add the following patchlet:

diff --git a/gcc/ada/libgnarl/s-osinte__x32.adb b/gcc/ada/libgnarl/s-osinte__x32.adb
index a2874be3d69..477abcc4176 100644
--- a/gcc/ada/libgnarl/s-osinte__x32.adb
+++ b/gcc/ada/libgnarl/s-osinte__x32.adb
@@ -89,8 +89,6 @@ package body System.OS_Interface is
    function To_Timespec (D : Duration) return timespec is
       S : time_t;
       F : Duration;
-
-      use type System.Linux.time_t;
    begin
       S := time_t (Long_Long_Integer (D));
       F := D - Duration (S);

Unfortunately my Linux distrib does not package a runtime package for x32, so testing will require me to install a VM and so on. I hope you’ll understand that I’ll do that only if things get too complex on your side. Thanks again for your help!
Comment 8 H.J. Lu 2017-10-02 10:37:33 UTC
Created attachment 42277 [details]
I am testing this patch.
Comment 9 Eric Botcazou 2017-10-02 16:03:37 UTC
Created attachment 42288 [details]
Better fix

Let's use this one instead to avoid future problems.
Comment 10 Eric Botcazou 2017-10-02 19:38:38 UTC
Author: ebotcazou
Date: Mon Oct  2 19:38:06 2017
New Revision: 253366

URL: https://gcc.gnu.org/viewcvs?rev=253366&root=gcc&view=rev
Log:
	PR ada/82384
	* libgnarl/s-linux__x32.ads (suseconds_t): New subtype.
	(time_t): Change from derived type to subtype.
	(timeval): Use suseconds_t for tv_usec.
	* libgnarl/s-osinte__x32.adb (To_Timespec): Remove use type clause.

Modified:
    trunk/gcc/ada/ChangeLog
    trunk/gcc/ada/libgnarl/s-linux__x32.ads
    trunk/gcc/ada/libgnarl/s-osinte__x32.adb
Comment 11 Eric Botcazou 2017-10-02 19:40:54 UTC
Thanks for reporting the problem.
Comment 12 Eric Botcazou 2017-10-02 19:41:20 UTC
.
Comment 13 Pierre-Marie de Rodat 2017-10-03 10:05:15 UTC
Yes, and thank you Eric for checking the fix in. :-)