[PATCH] Fix PR ada/111813 (Inconsistent limit in Ada.Calendar.Formatting)
The description of the second Value function (returning Duration) (ARM
9.6.1(87)
doesn't place any limitation on the Elapsed_Time parameter's value, beyond
"Constraint_Error is raised if the string is not formatted as described for
Image, or
the function cannot interpret the given string as a Duration value".
It would seem reasonable that Value and Image should be consistent, in that any
string produced by Image should be accepted by Value. Since Image must produce
a two-digit representation of the Hours, there's an implication that its
Elapsed_Time parameter should be less than 100.0 hours (the ARM merely says
that in that case the result is implementation-defined).
The current implementation of Value raises Constraint_Error if the Elapsed_Time
parameter is greater than or equal to 24 hours.
This patch removes the restriction, so that the Elapsed_Time parameter must only
be less than 100.0 hours.
gcc/ada/Changelog:
2023-10-15 Simon Wright
PR ada/111813
* gcc/ada/libgnat/a-calfor.adb (Value (2)): Allow values of parameter
Elapsed_Time greater than or equal to 24 hours, by doing the
hour calculations in Natural rather than Hour_Number (0 .. 23).
Calculate the result directly rather than by using Seconds_Of
(whose Hour parameter is of type Hour_Number).
If an exception occurs of type Constraint_Error, re-raise it
rather than raising a new CE.
gcc/testsuite/Changelog:
2023-10-15 Simon Wright
PR ada/111813
* gcc/testsuite/gnat.dg/calendar_format_value.adb: New test.
---
gcc/ada/libgnat/a-calfor.adb | 11 +---
.../gnat.dg/calendar_format_value.adb | 26 +++
2 files changed, 34 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gnat.dg/calendar_format_value.adb
diff --git a/gcc/ada/libgnat/a-calfor.adb b/gcc/ada/libgnat/a-calfor.adb
index 18f4e7388df..493728b490e 100644
--- a/gcc/ada/libgnat/a-calfor.adb
+++ b/gcc/ada/libgnat/a-calfor.adb
@@ -777,7 +777,7 @@ package body Ada.Calendar.Formatting is
function Value (Elapsed_Time : String) return Duration is
D : String (1 .. 11);
- Hour : Hour_Number;
+ Hour : Natural;
Minute : Minute_Number;
Second : Second_Number;
Sub_Second : Second_Duration := 0.0;
@@ -817,7 +817,7 @@ package body Ada.Calendar.Formatting is
-- Value extraction
- Hour := Hour_Number (Hour_Number'Value (D (1 .. 2)));
+ Hour := Natural (Natural'Value (D (1 .. 2)));
Minute := Minute_Number (Minute_Number'Value (D (4 .. 5)));
Second := Second_Number (Second_Number'Value (D (7 .. 8)));
@@ -837,9 +837,14 @@ package body Ada.Calendar.Formatting is
raise Constraint_Error;
end if;
- return Seconds_Of (Hour, Minute, Second, Sub_Second);
+ return Duration (Hour * 3600)
++ Duration (Minute * 60)
++ Duration (Second)
++ Sub_Second;
exception
+ -- CE is mandated, but preserve trace if CE already.
+ when Constraint_Error => raise;
when others => raise Constraint_Error;
end Value;
diff --git a/gcc/testsuite/gnat.dg/calendar_format_value.adb
b/gcc/testsuite/gnat.dg/calendar_format_value.adb
new file mode 100644
index 000..e98e496fd3b
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/calendar_format_value.adb
@@ -0,0 +1,26 @@
+-- { dg-do run }
+-- { dg-options "-O2" }
+
+with Ada.Calendar.Formatting;
+
+procedure Calendar_Format_Value is
+ Limit : constant Duration
+ := 99 * 3600.0 + 59 * 60.0 + 59.0;
+begin
+ declare
+ Image : constant String := Ada.Calendar.Formatting .Image (Limit);
+ Image_Error : exception;
+ begin
+ if Image /= "99:59:59" then
+ raise Image_Error with "image: " & Image;
+ end if;
+ declare
+ Value : constant Duration := Ada.Calendar.Formatting.Value (Image);
+ Value_Error : exception;
+ begin
+ if Value /= Limit then
+raise Value_Error with "duration: " & Value'Image;
+ end if;
+ end;
+ end;
+end Calendar_Format_Value;
--
2.39.3 (Apple Git-145)
Re: [PATCH] Fix PR ada/111813 (Inconsistent limit in Ada.Calendar.Formatting)
Pierre-Marie, I’ve CC’d you hoping you’re an appropriate person to ping on this
one.
If not, who would be for this sort of change?
I should have said, tested by
- add test case, make -C gcc check-gnat: error reported
- make -C gcc gnatlib_and_tools; make install
- make -C gcc check-gnat: no error reported
FSF copyright assignment RT:1016382
—S
> On 16 Oct 2023, at 14:32, Simon Wright wrote:
>
> The description of the second Value function (returning Duration) (ARM
> 9.6.1(87)
> doesn't place any limitation on the Elapsed_Time parameter's value, beyond
> "Constraint_Error is raised if the string is not formatted as described for
> Image, or
> the function cannot interpret the given string as a Duration value".
>
> It would seem reasonable that Value and Image should be consistent, in that
> any
> string produced by Image should be accepted by Value. Since Image must produce
> a two-digit representation of the Hours, there's an implication that its
> Elapsed_Time parameter should be less than 100.0 hours (the ARM merely says
> that in that case the result is implementation-defined).
>
> The current implementation of Value raises Constraint_Error if the
> Elapsed_Time
> parameter is greater than or equal to 24 hours.
>
> This patch removes the restriction, so that the Elapsed_Time parameter must
> only
> be less than 100.0 hours.
>
> gcc/ada/Changelog:
>
> 2023-10-15 Simon Wright
>
> PR ada/111813
>
> * gcc/ada/libgnat/a-calfor.adb (Value (2)): Allow values of parameter
> Elapsed_Time greater than or equal to 24 hours, by doing the
> hour calculations in Natural rather than Hour_Number (0 .. 23).
> Calculate the result directly rather than by using Seconds_Of
> (whose Hour parameter is of type Hour_Number).
>
> If an exception occurs of type Constraint_Error, re-raise it
> rather than raising a new CE.
>
> gcc/testsuite/Changelog:
>
> 2023-10-15 Simon Wright
>
> PR ada/111813
>
> * gcc/testsuite/gnat.dg/calendar_format_value.adb: New test.
>
> ---
> gcc/ada/libgnat/a-calfor.adb | 11 +---
> .../gnat.dg/calendar_format_value.adb | 26 +++
> 2 files changed, 34 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/gnat.dg/calendar_format_value.adb
>
> diff --git a/gcc/ada/libgnat/a-calfor.adb b/gcc/ada/libgnat/a-calfor.adb
> index 18f4e7388df..493728b490e 100644
> --- a/gcc/ada/libgnat/a-calfor.adb
> +++ b/gcc/ada/libgnat/a-calfor.adb
> @@ -777,7 +777,7 @@ package body Ada.Calendar.Formatting is
>
>function Value (Elapsed_Time : String) return Duration is
> D : String (1 .. 11);
> - Hour : Hour_Number;
> + Hour : Natural;
> Minute : Minute_Number;
> Second : Second_Number;
> Sub_Second : Second_Duration := 0.0;
> @@ -817,7 +817,7 @@ package body Ada.Calendar.Formatting is
>
> -- Value extraction
>
> - Hour := Hour_Number (Hour_Number'Value (D (1 .. 2)));
> + Hour := Natural (Natural'Value (D (1 .. 2)));
> Minute := Minute_Number (Minute_Number'Value (D (4 .. 5)));
> Second := Second_Number (Second_Number'Value (D (7 .. 8)));
>
> @@ -837,9 +837,14 @@ package body Ada.Calendar.Formatting is
> raise Constraint_Error;
> end if;
>
> - return Seconds_Of (Hour, Minute, Second, Sub_Second);
> + return Duration (Hour * 3600)
> ++ Duration (Minute * 60)
> ++ Duration (Second)
> ++ Sub_Second;
>
>exception
> + -- CE is mandated, but preserve trace if CE already.
> + when Constraint_Error => raise;
> when others => raise Constraint_Error;
>end Value;
>
> diff --git a/gcc/testsuite/gnat.dg/calendar_format_value.adb
> b/gcc/testsuite/gnat.dg/calendar_format_value.adb
> new file mode 100644
> index 000..e98e496fd3b
> --- /dev/null
> +++ b/gcc/testsuite/gnat.dg/calendar_format_value.adb
> @@ -0,0 +1,26 @@
> +-- { dg-do run }
> +-- { dg-options "-O2" }
> +
> +with Ada.Calendar.Formatting;
> +
> +procedure Calendar_Format_Value is
> + Limit : constant Duration
> + := 99 * 3600.0 + 59 * 60.0 + 59.0;
> +begin
> + declare
> + Image : constant String := Ada.Calendar.Formatting .Image (Limit);
> + Image_Error : exception;
> + begin
> + if Image /= "99:59:59" then
> + raise Image_Error with "image: " & Image;
> + end if;
> + declare
> + Value : constant Duration := Ada.Calendar.Formatting.Value (Image);
> + Value_Error : exception;
> + begin
> + if Value /= Limit then
> +raise Value_Error with "duration: " & Value'Image;
> + end if;
> + end;
> + end;
> +end Calendar_Format_Value;
> --
> 2.39.3 (Apple Git-145)
>
[PATCH] Fix PR ada/111909 On Darwin, determine filesystem case sensitivity at runtime
This change affects only Ada.
In gcc/ada/adaint.c(__gnat_get_file_names_case_sensitive), the
assumption for __APPLE__ is that file names are case-insensitive
unless __arm__ or __arm64__ are defined, in which case file names
are declared case-sensitive.
The associated comment is
"By default, we suppose filesystems aren't case sensitive on
Windows and Darwin (but they are on arm-darwin)."
This means that on aarch64-apple-darwin, file names are declared
case-sensitive, which is not normally the case (but users can set
up case-sensitive volumes).
It's understood that GCC does not currently support iOS/tvOS/watchOS,
so we assume macOS.
Bootstrapped on x86_64-apple-darwin with languages c,c++,ada and regression
tested (check-gnat).
Also, tested with the example from PR ada/81114, extracted into 4 volumes
(APFS, APFS-case-sensitive,
HFS, HFS-case-sensitive; the example code built successfully on the
case-sensitive volumes.
Setting GNAT_FILE_NAME_CASE_SENSITIVE successfully overrode the choices made by
the
new code.
gcc/ada/Changelog:
2023-10-29 Simon Wright
PR ada/111909
* gcc/ada/adaint.c
(__gnat_get_file_names_case_sensitive): Remove the checks for
__arm__, __arm64__.
Split out the check for __APPLE__; remove the checks for __arm__,
__arm64__, and use getattrlist(2) to determine whether the current
working directory is on a case-sensitive filesystem.
Signed-off-by: Simon Wright
---
gcc/ada/adaint.c | 46 ++
1 file changed, 42 insertions(+), 4 deletions(-)
diff --git a/gcc/ada/adaint.c b/gcc/ada/adaint.c
index 2a193efc002..43d166824b0 100644
--- a/gcc/ada/adaint.c
+++ b/gcc/ada/adaint.c
@@ -85,6 +85,7 @@
#if defined (__APPLE__)
#include
+#include
#endif
#if defined (__hpux__)
@@ -613,11 +614,48 @@ __gnat_get_file_names_case_sensitive (void)
else
{
/* By default, we suppose filesystems aren't case sensitive on
-Windows and Darwin (but they are on arm-darwin). */
-#if defined (WINNT) || defined (__DJGPP__) \
- || (defined (__APPLE__) && !(defined (__arm__) || defined (__arm64__)))
+Windows or DOS. */
+#if defined (WINNT) || defined (__DJGPP__)
file_names_case_sensitive_cache = 0;
-#else
+#elif defined (__APPLE__)
+ /* Determine whether the current volume is case-sensitive. */
+ {
+ /* Formulate a query for the volume capabilities. */
+ struct attrlist attrList
+ = {ATTR_BIT_MAP_COUNT,
+0, /* reserved. */
+0, /* commonattr. */
+ATTR_VOL_INFO | ATTR_VOL_CAPABILITIES, /* volattr. */
+0, /* dirattr. */
+0, /* fileattr. */
+0/* forkattr. */
+ };
+
+ /* A buffer to contain just the volume capabilities. */
+ struct returnBuf {
+ u_int32_t length;
+ vol_capabilities_attr_t caps;
+ } __attribute__ ((aligned (4), packed)) retBuf;
+
+ /* Default to case-insensitive. */
+ file_names_case_sensitive_cache = 0;
+
+ /* Query the current working directory. */
+ if (getattrlist (".",
+&attrList,
+&retBuf,
+sizeof (retBuf),
+0) == 0)
+ /* The call succeeded. */
+ if ((retBuf.caps.valid[VOL_CAPABILITIES_FORMAT]
+ & VOL_CAP_FMT_CASE_SENSITIVE))
+ /* The volume could be case-sensitive. */
+ if (retBuf.caps.capabilities[VOL_CAPABILITIES_FORMAT]
+ & VOL_CAP_FMT_CASE_SENSITIVE)
+ /* The volume is case-sensitive. */
+ file_names_case_sensitive_cache = 1;
+ }
+#else /* Neither Windows nor Apple. */
file_names_case_sensitive_cache = 1;
#endif
}
--
2.39.3 (Apple Git-145)
Re: [PATCH] Fix PR ada/111909 On Darwin, determine filesystem case sensitivity at runtime
On 3 Nov 2023, at 08:39, Arnaud Charlet wrote: > In addition to the non portable issues already mentioned, this change isn't > OK also > for other reasons. > > Basically this function is global and decides once for all on the case > sensitivity, while > the case sensitiviy is on a per filsystem basis as you noted. Well, the current code does exactly what you describe, with less relationship to the actual environment than this proposal. > So without changing fundamentally the model, you can't decide dynamically for > the whole > system. Making the choice based on the current directory is pretty random, > since the current > directory isn't well defined at program's start up and could be pretty much > any filesystem. I’d imagine that projects spread over more than one differently-case-sensitive filesystem would be rare. As to the current directory at compiler startup, with GPRbuild it’s the object directory, so likely to be somewhere near the project’s source tree. > Note that the current setting on arm is actually for iOS, which we did > support at AdaCore > at some point (and could revive in the future, who knows). Wouldn’t it be more natural to go via LLVM? I understand from Iain that iOS isn’t currently supported by GCC. > So it would be fine to refine the test to differentiate between macOS and > embedded iOS and co, > that would be a better change here. There didn’t seem to be a way to do that. But anyway, I find myself puzzled by the casing issue. It seems to me that on a CS filesystem users would be well advised to stick to lower-case filenames, or the compiler won’t be able to resolve 'with' statements; whereas on a non-CS system, there’d be no such constraint. Indeed, when compiling a file with a mixed-case name in a CS environment, the compiler warns: $ GNAT_FILE_NAME_CASE_SENSITIVE=1 gcc -c -u -f WTF.adb WTF.adb:1:11: warning: file name does not match unit name, should be "wtf.adb" [enabled by default] Also, there’ve been about 80 downloads of GCC 13.1.0 for aarch64-apple-darwin, and no case-sensitivity issues have been reported. So, Iain, do we want to pursue this? > >> This change affects only Ada. >> >> In gcc/ada/adaint.c(__gnat_get_file_names_case_sensitive), the >> assumption for __APPLE__ is that file names are case-insensitive >> unless __arm__ or __arm64__ are defined, in which case file names >> are declared case-sensitive. >> >> The associated comment is >> "By default, we suppose filesystems aren't case sensitive on >> Windows and Darwin (but they are on arm-darwin)." >> >> This means that on aarch64-apple-darwin, file names are declared >> case-sensitive, which is not normally the case (but users can set >> up case-sensitive volumes). >> >> It's understood that GCC does not currently support iOS/tvOS/watchOS, >> so we assume macOS. >> >> Bootstrapped on x86_64-apple-darwin with languages c,c++,ada and regression >> tested (check-gnat). >> Also, tested with the example from PR ada/81114, extracted into 4 volumes >> (APFS, APFS-case-sensitive, >> HFS, HFS-case-sensitive; the example code built successfully on the >> case-sensitive volumes. >> Setting GNAT_FILE_NAME_CASE_SENSITIVE successfully overrode the choices made >> by the >> new code. >> >> gcc/ada/Changelog: >> >> 2023-10-29 Simon Wright >> >> PR ada/111909 >> >> * gcc/ada/adaint.c >> (__gnat_get_file_names_case_sensitive): Remove the checks for >> __arm__, __arm64__. >> Split out the check for __APPLE__; remove the checks for __arm__, >> __arm64__, and use getattrlist(2) to determine whether the current >> working directory is on a case-sensitive filesystem. >> >> Signed-off-by: Simon Wright
Re: [PATCH] Fix PR ada/111813 (Inconsistent limit in Ada.Calendar.Formatting)
On 24 Oct 2023, at 10:49, Arnaud Charlet wrote: > > This change is OK, thank you. Can it be committed, then, please? >> The description of the second Value function (returning Duration) (ARM >> 9.6.1(87) >> doesn't place any limitation on the Elapsed_Time parameter's value, beyond >> "Constraint_Error is raised if the string is not formatted as described for >> Image, or >> the function cannot interpret the given string as a Duration value". >> >> It would seem reasonable that Value and Image should be consistent, in that >> any >> string produced by Image should be accepted by Value. Since Image must >> produce >> a two-digit representation of the Hours, there's an implication that its >> Elapsed_Time parameter should be less than 100.0 hours (the ARM merely says >> that in that case the result is implementation-defined). >> >> The current implementation of Value raises Constraint_Error if the >> Elapsed_Time >> parameter is greater than or equal to 24 hours. >> >> This patch removes the restriction, so that the Elapsed_Time parameter must >> only >> be less than 100.0 hours. >> >> gcc/ada/Changelog: >> >> 2023-10-15 Simon Wright >> >> PR ada/111813 >> >> * gcc/ada/libgnat/a-calfor.adb (Value (2)): Allow values of parameter >> Elapsed_Time greater than or equal to 24 hours, by doing the >> hour calculations in Natural rather than Hour_Number (0 .. 23). >> Calculate the result directly rather than by using Seconds_Of >> (whose Hour parameter is of type Hour_Number). >> >> If an exception occurs of type Constraint_Error, re-raise it >> rather than raising a new CE. >> >> gcc/testsuite/Changelog: >> >> 2023-10-15 Simon Wright >> >> PR ada/111813 >> >> * gcc/testsuite/gnat.dg/calendar_format_value.adb: New test.
Re: [PATCH] Fix PR ada/111909 On Darwin, determine filesystem case sensitivity at runtime
On 6 Nov 2023, at 08:36, Arnaud Charlet wrote: > >>> So without changing fundamentally the model, you can't decide dynamically >>> for the whole >>> system. Making the choice based on the current directory is pretty random, >>> since the current >>> directory isn't well defined at program's start up and could be pretty much >>> any filesystem. >> >> I’d imagine that projects spread over more than one >> differently-case-sensitive filesystem would >> be rare. As to the current directory at compiler startup, with GPRbuild it’s >> the object directory, so >> likely to be somewhere near the project’s source tree. > > I am not talking about the current directory when the compiler runs, I am > talking about the > current directory where the target program runs, which can be pretty much > anywhere. > > In other words, you are modifying a runtime file (adaint.c) which is used > both by the host compiler > and by the target applications. My comment worries about the target > applications while yours > applies to the host compiler only. I don’t understand? The change works out whether the filesystem of the current working directory is CS, whether it’s the compiler or some user program that’s running it (it looks like that would have to be via some higher-level compiler package, I found only GNAT.Command_Line and GNAT.Directory_Operations). I can see that might not be what the user program wants, but if they actually care the current situation isn’t great anyway; the compiler definitely makes the wrong choice for new Macs. >>> Note that the current setting on arm is actually for iOS, which we did >>> support at AdaCore >>> at some point (and could revive in the future, who knows). >> >> Wouldn’t it be more natural to go via LLVM? I understand from Iain that iOS >> isn’t currently >> supported by GCC. > > That's another option. We'd like to keep both options on the table, since > both options have > pros and cons. > >>> So it would be fine to refine the test to differentiate between macOS and >>> embedded iOS and co, >>> that would be a better change here. >> >> There didn’t seem to be a way to do that. > > OK, I thought there would be some defines that we could use for that, too bad > if there isn't > and indeed we might need to perform another runtime check then as suggested > by Iain. I can see a possible interface, operatingSystemVersion in NSProcessInfo.h - Objective C needed, I think > Arno
Re: [PATCH] Fix PR ada/111909 On Darwin, determine filesystem case sensitivity at runtime
On 11 Nov 2023, at 18:10, Iain Sandoe wrote: > >> On 11 Nov 2023, at 07:47, Simon Wright wrote: >> >> On 6 Nov 2023, at 08:36, Arnaud Charlet wrote: >>> >>>>> So without changing fundamentally the model, you can't decide dynamically >>>>> for the whole >>>>> system. Making the choice based on the current directory is pretty >>>>> random, since the current >>>>> directory isn't well defined at program's start up and could be pretty >>>>> much any filesystem. >>>> >>>> I’d imagine that projects spread over more than one >>>> differently-case-sensitive filesystem would >>>> be rare. As to the current directory at compiler startup, with GPRbuild >>>> it’s the object directory, so >>>> likely to be somewhere near the project’s source tree. >>> >>> I am not talking about the current directory when the compiler runs, I am >>> talking about the >>> current directory where the target program runs, which can be pretty much >>> anywhere. >>> >>> In other words, you are modifying a runtime file (adaint.c) which is used >>> both by the host compiler >>> and by the target applications. My comment worries about the target >>> applications while yours >>> applies to the host compiler only. >> >> I don’t understand? >> >> The change works out whether the filesystem of the current working directory >> is CS, whether >> it’s the compiler or some user program that’s running it (it looks like that >> would have to be via >> some higher-level compiler package, I found only GNAT.Command_Line and >> GNAT.Directory_Operations). >> >> I can see that might not be what the user program wants, but if they >> actually care the current >> situation isn’t great anyway; the compiler definitely makes the wrong choice >> for new Macs. >> >>>>> Note that the current setting on arm is actually for iOS, which we did >>>>> support at AdaCore >>>>> at some point (and could revive in the future, who knows). >>>> >>>> Wouldn’t it be more natural to go via LLVM? I understand from Iain that >>>> iOS isn’t currently >>>> supported by GCC. >>> >>> That's another option. We'd like to keep both options on the table, since >>> both options have >>> pros and cons. >>> >>>>> So it would be fine to refine the test to differentiate between macOS and >>>>> embedded iOS and co, >>>>> that would be a better change here. >>>> >>>> There didn’t seem to be a way to do that. >>> >>> OK, I thought there would be some defines that we could use for that, too >>> bad if there isn't >>> and indeed we might need to perform another runtime check then as suggested >>> by Iain. >> >> I can see a possible interface, operatingSystemVersion in NSProcessInfo.h - >> Objective C >> needed, I think > > Some of the NS interfaces are available to regular C (e.g. stuff in > CoreFoundation), and I am > fairly/very sure that we will be able to find a machanism that does not > involve introducing an > ObjC dep. [I am obvioulsy not in any way against ObjC - since i’m the > maintainer ;) .. but it > seems heavyweight for solving this issue]. It certainly would be heavyweight, since TargetConditionals.h includes TARGET_OS_OSX, which is 1 if we’re compiling for macOS and 0 otherwise (there’s a useful chart at :83 in the MacOSX13.1 SDK). Two ways ahead here: (1) just replace the current __arm__, __arm64__ test with this (2) as 1, but implement the runtime test for case sensitivity only for macOS Whether (2) is acceptable depends, I suppose, on what issues Iain encountered on Darwin 9 & Darwin 17. I’ll be content to go with (1).
Re: [PATCH] Fix PR ada/111909 On Darwin, determine filesystem case sensitivity at runtime
On 13 Nov 2023, at 16:18, Arnaud Charlet wrote:
>
>>>>> OK, I thought there would be some defines that we could use for that, too
>>>>> bad if there isn't
>>>>> and indeed we might need to perform another runtime check then as
>>>>> suggested by Iain.
>>>>
>>>> I can see a possible interface, operatingSystemVersion in NSProcessInfo.h
>>>> - Objective C
>>>> needed, I think
>>>
>>> Some of the NS interfaces are available to regular C (e.g. stuff in
>>> CoreFoundation), and I am
>>> fairly/very sure that we will be able to find a machanism that does not
>>> involve introducing an
>>> ObjC dep. [I am obvioulsy not in any way against ObjC - since i’m the
>>> maintainer ;) .. but it
>>> seems heavyweight for solving this issue].
>>
>> It certainly would be heavyweight, since TargetConditionals.h includes
>> TARGET_OS_OSX,
>> which is 1 if we’re compiling for macOS and 0 otherwise (there’s a useful
>> chart at :83 in the
>> MacOSX13.1 SDK).
>>
>> Two ways ahead here:
>> (1) just replace the current __arm__, __arm64__ test with this
>
> That would be fine here (replace refs to *arm* by TARGET_OS_OSX), since this
> was my original
> suggestion (copied at the top of this email).
>
>> (2) as 1, but implement the runtime test for case sensitivity only for macOS
>>
>> Whether (2) is acceptable depends, I suppose, on what issues Iain
>> encountered on Darwin 9
>> & Darwin 17. I’ll be content to go with (1).
I'm not sure whether this should have been a new [PATCH V2] thread?
Also, should the test code below (between %%%) be included in the
testsuite?
--8<--
In gcc/ada/adaint.c(__gnat_get_file_names_case_sensitive), the
current assumption for __APPLE__ is that file names are
case-insensitive unless __arm__ or __arm64__ are defined, in which
case file names are declared case-sensitive.
The associated comment is
"By default, we suppose filesystems aren't case sensitive on
Windows and Darwin (but they are on arm-darwin)."
This means that on aarch64-apple-darwin, file names are treated as
case-sensitive, which is not the default case.
Apple provide a header file which permits a
compile-time check for the compiler target (e.g. OSX vs IOS). At Darwin
10.5 (Xcode 3) iOS wasn't supported, so it was adequate to check
TARGET_OS_MAC; nowadays, that covers many variants including macOS
and iOS, so one needs to check whether TARGET_OS_OSX is defined, and
if so whether it's set.
Bootstrapped on x86_64-apple-darwin with languages c,c++,ada and regression
tested (check-ada).
Likewise bootstrapped on aarch64-apple-darwin from the Github sources
corresponding to GCC 2023-11-05.
__gnat_get_file_names_case_sensitive() isn't exported to user code, so
implemented check code as below: each compiler (x86_64-apple-darwin and
aarch64-apple-darwin) reported that file names were not case sensitive.
%%%
with Ada.Text_IO;
with Interfaces.C;
procedure Check_Case_Sensitivity is
type C_Boolean is (False, True)
with Convention => C;
function Get_File_Names_Case_Sensitive return C_Boolean
with
Import,
Convention => C,
External_Name => "__gnat_get_file_names_case_sensitive";
begin
Ada.Text_IO.Put_Line ("GNAT thinks file names are " &
(case Get_File_Names_Case_Sensitive is
when False => "not case sensitive",
when True => "case sensitive"));
end Check_Case_Sensitivity;
%%%
gcc/ada/Changelog:
2023-11-16 Simon Wright
* gcc/ada/adaint.c
(__gnat_get_file_names_case_sensitive): Split out the __APPLE__
check and remove the checks for __arm__, __arm64__.
File names are by default case sensitive unless TARGET_OS_OSX
(or if this is an older OS release, in which case TARGET_OS_OSX
is undefined, TARGET_OS_MAC) is set.
Signed-off-by: Simon Wright
---
gcc/ada/adaint.c | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/gcc/ada/adaint.c b/gcc/ada/adaint.c
index bb4ed2607e5..1ef529ec20b 100644
--- a/gcc/ada/adaint.c
+++ b/gcc/ada/adaint.c
@@ -84,7 +84,7 @@
#endif /* VxWorks */
#if defined (__APPLE__)
-#include
+#include
#endif
#if defined (__hpux__)
@@ -613,12 +613,25 @@ __gnat_get_file_names_case_sensitive (void)
else
{
/* By default, we suppose filesystems aren't case sensitive on
-Windows and Darwin (but they are on arm-darwin). */
-#if defined (WINNT) || defined (__DJGPP__) \
- || (defined (__APPLE__) && !(defined (__arm__) || defined (__arm64__)))
+Windows or DOS. */
+#if defined (WINNT) || defin
Re: [PATCH] Fix PR ada/111909 On Darwin, determine filesystem case sensitivity at runtime
On 17 Nov 2023, at 08:37, Arnaud Charlet wrote:
>
>> Also, should the test code below (between %%%) be included in the
>> testsuite?
>
> It would be good but tests shouldn't output anything, they should be self
> testing,
> and you will need to deal with making the test portable to all targets.
>
> Given that the compiler itself uses this feature, I don't think this is worth
> the trouble.
OK
>> @@ -613,12 +613,25 @@ __gnat_get_file_names_case_sensitive (void)
>> else
>> {
>>/* By default, we suppose filesystems aren't case sensitive on
>> - Windows and Darwin (but they are on arm-darwin). */
>> -#if defined (WINNT) || defined (__DJGPP__) \
>> - || (defined (__APPLE__) && !(defined (__arm__) || defined (__arm64__)))
>> + Windows or DOS. */
>> +#if defined (WINNT) || defined (__DJGPP__)
>> + file_names_case_sensitive_cache = 0;
>> +#elif defined (__APPLE__)
>> + /* By default, macOS volumes are case-insensitive, iOS
>> + volumes are case-sensitive. */
>> +#if defined (TARGET_OS_OSX) /* In recent SDK. */
>> +#if TARGET_OS_OSX/* macOS. */
>>file_names_case_sensitive_cache = 0;
>> #else
>>file_names_case_sensitive_cache = 1;
>> +#endif
>> +#elif TARGET_OS_MAC/* macOS, in older SDK. */
>> + file_names_case_sensitive_cache = 0;
>> +#else
>> + file_names_case_sensitive_cache = 1;
>> +#endif
>> +#else /* Neither Windows nor Apple. */
>> + file_names_case_sensitive_cache = 1;
>> #endif
>
> Please simplify the above to (untested):
>
> #elif defined (__APPLE__)
>/* By default, macOS volumes are case-insensitive, iOS
> volumes are case-sensitive. */
> #if TARGET_OS_MAC/* macOS, in older SDK. */
> file_names_case_sensitive_cache = 0;
> #elif TARGET_OS_OSX /* macOS, in recent SDK. */
> file_names_case_sensitive_cache = 0;
> #else/* assume iOS. */
> file_names_case_sensitive_cache = 1;
> #endif
> #else /* Neither Windows nor Apple. */
>file_names_case_sensitive_cache = 1;
> #endif
>
> which is simpler and more readable and should be equivalent AFAICT.
>
> OK with the above change.
>
> Arno
Sorry, but that wouldn’t work.
TargetConditionals.h is created by Apple as part of SDK construction, so the
TARGET_* macros are defined directly (#define TARGET_OS_OSX 1),
In a newer macOS SDK, both TARGET_OS_MAC and TARGET_OS_OSX are defined and set
to 1, and TARGET_OS_MAC covers OSX (macOS), IOS, TV, WATCH and others.
In an older macOS SDK, TARGET_OS_MAC is defined and set to 1, and none of the
others are defined at all.
This is from the current TargetConditionals.h:
*
+---+
*| TARGET_OS_MAC
|
*| +-+ +-+
+---+ |
*| | | | TARGET_OS_IPHONE | |
| |
*| | | | +-+ ++ +---+ ++ | |
| |
*| | | | | IOS | || | | || | |
| |
*| | OSX | | | +-+ | | TV | | WATCH | | BRIDGE | | | DRIVERKIT
| |
*| | | | | | MACCATALYST | | || | | || | |
| |
*| | | | | +-+ | || | | || | |
| |
*| | | | +-+ ++ +---+ ++ | |
| |
*| +-+ +-+
+---+ |
*
+---+
Re: [PATCH] Fix PR ada/111909 On Darwin, determine filesystem case sensitivity at runtime
>>> Please simplify the above to (untested): >>> >>> #elif defined (__APPLE__) >>> /* By default, macOS volumes are case-insensitive, iOS >>> volumes are case-sensitive. */ >>> #if TARGET_OS_MAC/* macOS, in older SDK. */ >>>file_names_case_sensitive_cache = 0; >>> #elif TARGET_OS_OSX /* macOS, in recent SDK. */ >>>file_names_case_sensitive_cache = 0; >>> #else/* assume iOS. */ >>>file_names_case_sensitive_cache = 1; >>> #endif >>> #else /* Neither Windows nor Apple. */ >>> file_names_case_sensitive_cache = 1; >>> #endif >>> >>> which is simpler and more readable and should be equivalent AFAICT. >>> >>> OK with the above change. >>> >>> Arno >> >> Sorry, but that wouldn’t work. > > Then invert the two first tests, that doesn't change the gist of my > suggestion to simplify the > tests. Apple’s naming is definitely confusing in this area! In current SDKs, TARGET_OS_MAC means code is being generated for a Mac OS X variant, which covers OSX, IOS, Watch … ; to determine which kind of device, you have to check the specific define for that device - OSX corresponds to macOS, i.e. laptops, desktops. In older SDKs (specifically Xcode 3, for macOS Leopard (darwin 9) as mentioned by Iain) TARGET_OS_MAC means code is being generated for "Mac OS", i.e. laptops, desktops as above; TARGET_OS_OSX is undefined (as are TARGET_OS_IOS etc). If we are compiling for macOS, using a current macOS SDK, then TARGET_OS_MAC is set to 1 and TARGET_OS_OSX is set to 1. If we were compiling for iOS, using a current iOS SDK as supplied with current Xcode, then TARGET_OS_MAC would be set to 1, TARGET_OS_OSX would be set to 0, and TARGET_OS_IOS would be set to 1. If TARGET_OS_OSX is defined then — we’re generating code for a recent Apple device, TARGET_OS_MAC could mean — macOS, iOS, iWatch etc, so we can’t use it. if TARGET_OS_OSX is 1 then — we’re generating code for macOS, file names are case-insensitive. else — we’re trying to generate code for a device which GCC doesn’t support at — the moment, e.g. iOS; let’s assume file names are case-sensitive.. end if else if TARGET_OS_MAC is 1 then — we’re generating code for macOS, file names are case-insensitve. else — let’s assume file names are case-sensitive. end if end if What we’re doing here is providing a default behaviour; it’s certainly the case that Apple filesystems are by default case-insensitive. If a user has code on case-sensitive file systems (Apple or other, e.g. unix-over-NFS) it’s up to them to use GNAT_FILE_NAME_CASE_SENSITIVE. >> TargetConditionals.h is created by Apple as part of SDK construction, so the >> TARGET_* macros are defined directly (#define TARGET_OS_OSX 1), >> >> In a newer macOS SDK, both TARGET_OS_MAC and TARGET_OS_OSX are defined and >> set to 1, and TARGET_OS_MAC covers OSX (macOS), IOS, TV, WATCH and others. >> In an older macOS SDK, TARGET_OS_MAC is defined and set to 1, and none of >> the others are defined at all.
Re: [PATCH] Fix PR ada/111909 On Darwin, determine filesystem case sensitivity at runtime
> >> Apple’s naming is definitely confusing in this area! >> >> In current SDKs, TARGET_OS_MAC means code is being generated for a Mac OS X >> variant, >> which covers OSX, IOS, Watch … ; to determine which kind of device, you have >> to check the >> specific define for that device - OSX corresponds to macOS, i.e. laptops, >> desktops. >> >> In older SDKs (specifically Xcode 3, for macOS Leopard (darwin 9) as >> mentioned by Iain) >> TARGET_OS_MAC means code is being generated for "Mac OS", i.e. laptops, >> desktops as >> above; TARGET_OS_OSX is undefined (as are TARGET_OS_IOS etc). >> >> If we are compiling for macOS, using a current macOS SDK, then TARGET_OS_MAC >> is >> set to 1 and TARGET_OS_OSX is set to 1. >> >> If we were compiling for iOS, using a current iOS SDK as supplied with >> current Xcode, then >> TARGET_OS_MAC would be set to 1, TARGET_OS_OSX would be set to 0, and >> TARGET_OS_IOS would be set to 1. > > OK so then the following is sufficient for our needs: > > #elif defined (__APPLE__) > /* By default, macOS volumes are case-insensitive, iOS > volumes are case-sensitive. */ > #if TARGET_OS_IOS >file_names_case_sensitive_cache = 1; > #else >file_names_case_sensitive_cache = 0; > #endif > #else /* Neither Windows nor Apple. */ >file_names_case_sensitive_cache = 1; > #endif > > We want the default to be 0, and we only care about setting it to 1 on iOS > for recent > SDKs, the case of an old SDK and iOS isn't interesting at this stage, so it's > fine if we set > the var to 0 in this scenario. I can’t speak for Darwin maintainers, so I’ll leave it to Iain to comment on this suggestion.
Re: [PATCH] Fix PR ada/111909 On Darwin, determine filesystem case sensitivity at runtime
On 21 Nov 2023, at 11:22, Iain Sandoe wrote: > > Hello Simon, Arno, > >> On 17 Nov 2023, at 13:43, Simon Wright wrote: >> >>> >>>> Apple’s naming is definitely confusing in this area! >>>> >>>> In current SDKs, TARGET_OS_MAC means code is being generated for a Mac OS >>>> X variant, >>>> which covers OSX, IOS, Watch … ; to determine which kind of device, you >>>> have to check the >>>> specific define for that device - OSX corresponds to macOS, i.e. laptops, >>>> desktops. >>>> >>>> In older SDKs (specifically Xcode 3, for macOS Leopard (darwin 9) as >>>> mentioned by Iain) >>>> TARGET_OS_MAC means code is being generated for "Mac OS", i.e. laptops, >>>> desktops as >>>> above; TARGET_OS_OSX is undefined (as are TARGET_OS_IOS etc). >>>> >>>> If we are compiling for macOS, using a current macOS SDK, then >>>> TARGET_OS_MAC is >>>> set to 1 and TARGET_OS_OSX is set to 1. >>>> >>>> If we were compiling for iOS, using a current iOS SDK as supplied with >>>> current Xcode, then >>>> TARGET_OS_MAC would be set to 1, TARGET_OS_OSX would be set to 0, and >>>> TARGET_OS_IOS would be set to 1. >>> >>> OK so then the following is sufficient for our needs: >>> >>> #elif defined (__APPLE__) >>> /* By default, macOS volumes are case-insensitive, iOS >>>volumes are case-sensitive. */ >>> #if TARGET_OS_IOS >>> file_names_case_sensitive_cache = 1; >>> #else >>> file_names_case_sensitive_cache = 0; >>> #endif >>> #else /* Neither Windows nor Apple. */ >>> file_names_case_sensitive_cache = 1; >>> #endif >>> >>> We want the default to be 0, and we only care about setting it to 1 on iOS >>> for recent >>> SDKs, the case of an old SDK and iOS isn't interesting at this stage, so >>> it's fine if we set >>> the var to 0 in this scenario. >> >> I can’t speak for Darwin maintainers, so I’ll leave it to Iain to comment on >> this suggestion. > > * We are far away from having support for watchOS (32b Arm64) so I think that > is a bridge > that can be crossed later. > > * It seems to me that the proposed solution is better matched to the defaults > on macOS/iOS. > > * It would be better to have an automatic solution for folks (like me) who do > use case- > sensitive file systems on macOS, but we do not have the resources right now > to figure > out what is not working on the earlier systems. I looked briefly, and found > that the libcalls > are thin wrappers on a syscall, so that the different behaviours we are > seeing on earlier > OS versions reflects the kernel’s handling of the provided path, rather than > some improvement > in newer library functions. That suggests to me that we will need to wrap > the call in some more > complex logic to obtain the correct response. > > So, I think that (with a test across the range of supported OS versions) the > proposed > solution is an incremental improvement and we should take it. > > When there’s a final proposed patch, I can add it into my testing across the > systems. > > Iain Herewith my proposed patch (still in thread, though the subject of the thread isn’t still appropriate): In gcc/ada/adaint.c(__gnat_get_file_names_case_sensitive), the current assumption for __APPLE__ is that file names are case-insensitive unless __arm__ or __arm64__ are defined, in which case file names are declared case-sensitive. The associated comment is "By default, we suppose filesystems aren't case sensitive on Windows and Darwin (but they are on arm-darwin)." This means that on aarch64-apple-darwin, file names are treated as case-sensitive, which is not the default case. The true default position is that macOS file systems are case-insensitive, iOS file systems are case-sensitive. Apple provide a header file which permits a compile-time check for the compiler target (e.g. OSX vs IOS); if TARGET_OS_IOS is defined as 1, this is a build for iOS. gcc/ada/Changelog: 2023-11-21 Simon Wright mailto:si...@pushface.org>> * gcc/ada/adaint.c (__gnat_get_file_names_case_sensitive): Split out the __APPLE__ check and remove the checks for __arm__, __arm64__. For Apple, file names are by default case-insensitive unless TARGET_OS_IOS is set. Signed-off-by: Simon Wright --- gcc/ada/adaint.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/gcc/ada/adaint.c b/g
Re: [PATCH] Fix PR ada/111909 On Darwin, determine filesystem case sensitivity at runtime
On 21 Nov 2023, at 23:13, Iain Sandoe wrote:
>> #if defined (__APPLE__)
>> -#include
>
> If removing unistd.h is intentional (i.e. you determined that it’s no longer
> needed for Darwin), then we should make that a separate patch.
I thought that I’d had to include unistd.h for the first patch in this thread;
clearly not!
What I hope will be the final version:
——— 8< .———
In gcc/ada/adaint.c(__gnat_get_file_names_case_sensitive), the current
assumption for __APPLE__ is that file names are case-insensitive
unless __arm__ or __arm64__ are defined, in which case file names are
declared case-sensitive.
The associated comment is
"By default, we suppose filesystems aren't case sensitive on
Windows and Darwin (but they are on arm-darwin)."
This means that on aarch64-apple-darwin, file names are treated as
case-sensitive, which is not the default case.
The true default position is that macOS file systems are
case-insensitive, iOS file systems are case-sensitive.
Apple provide a header file which permits a
compile-time check for the compiler target (e.g. OSX vs IOS); if
TARGET_OS_IOS is defined as 1, this is a build for iOS.
* gcc/ada/adaint.c
(__gnat_get_file_names_case_sensitive): Split out the __APPLE__
check and remove the checks for __arm__, __arm64__.
For Apple, file names are by default case-insensitive unless
TARGET_OS_IOS is set.
Signed-off-by: Simon Wright
---
gcc/ada/adaint.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/gcc/ada/adaint.c b/gcc/ada/adaint.c
index bb4ed2607e5..2e9c59ae958 100644
--- a/gcc/ada/adaint.c
+++ b/gcc/ada/adaint.c
@@ -85,6 +85,7 @@
#if defined (__APPLE__)
#include
+#include
#endif
#if defined (__hpux__)
@@ -613,11 +614,18 @@ __gnat_get_file_names_case_sensitive (void)
else
{
/* By default, we suppose filesystems aren't case sensitive on
-Windows and Darwin (but they are on arm-darwin). */
-#if defined (WINNT) || defined (__DJGPP__) \
- || (defined (__APPLE__) && !(defined (__arm__) || defined (__arm64__)))
+Windows or DOS. */
+#if defined (WINNT) || defined (__DJGPP__)
file_names_case_sensitive_cache = 0;
+#elif defined (__APPLE__)
+ /* By default, macOS volumes are case-insensitive, iOS
+volumes are case-sensitive. */
+#if TARGET_OS_IOS
+ file_names_case_sensitive_cache = 1;
#else
+ file_names_case_sensitive_cache = 0;
+#endif
+#else /* Neither Windows nor Apple. */
file_names_case_sensitive_cache = 1;
#endif
}
--
2.37.1 (Apple Git-137.1)
Re: [PATCH] Fix PR ada/111909 On Darwin, determine filesystem case sensitivity at runtime
> On 22 Nov 2023, at 15:03, Iain Sandoe wrote: > > > >> On 22 Nov 2023, at 14:48, Iain Sandoe wrote: >> >> >> >>> On 22 Nov 2023, at 13:55, Arnaud Charlet wrote: >>> >>>>>> #if defined (__APPLE__) >>>>>> -#include >>>>> >>>>> If removing unistd.h is intentional (i.e. you determined that it’s no >>>>> longer >>>>> needed for Darwin), then we should make that a separate patch. >>>> >>>> I thought that I’d had to include unistd.h for the first patch in this >>>> thread; clearly not! >>>> >>>> What I hope will be the final version: >>> >>> OK here. >> >> also OK here, thanks > > I think this fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111909 ? > if you agree then please add that to the commit. > Iain git format-patch does so much, I forgot this, sorry: gcc/ada/Changelog: 2023-11-22 Simon Wright mailto:si...@pushface.org>> PR ada/111909 > >> Iain >> >>> >>>> ——— 8< .——— >>>> >>>> In gcc/ada/adaint.c(__gnat_get_file_names_case_sensitive), the current >>>> assumption for __APPLE__ is that file names are case-insensitive >>>> unless __arm__ or __arm64__ are defined, in which case file names are >>>> declared case-sensitive. >>>> >>>> The associated comment is >>>> "By default, we suppose filesystems aren't case sensitive on >>>> Windows and Darwin (but they are on arm-darwin)." >>>> >>>> This means that on aarch64-apple-darwin, file names are treated as >>>> case-sensitive, which is not the default case. >>>> >>>> The true default position is that macOS file systems are >>>> case-insensitive, iOS file systems are case-sensitive. >>>> >>>> Apple provide a header file which permits a >>>> compile-time check for the compiler target (e.g. OSX vs IOS); if >>>> TARGET_OS_IOS is defined as 1, this is a build for iOS. >>>> >>>> * gcc/ada/adaint.c >>>> (__gnat_get_file_names_case_sensitive): Split out the __APPLE__ >>>> check and remove the checks for __arm__, __arm64__. >>>> For Apple, file names are by default case-insensitive unless >>>> TARGET_OS_IOS is set. >>>> >>>> Signed-off-by: Simon Wright >
Re: [PATCH] Fix PR ada/111909 On Darwin, determine filesystem case sensitivity at runtime
> On 22 Nov 2023, at 15:13, Simon Wright wrote: > > > >> On 22 Nov 2023, at 15:03, Iain Sandoe wrote: >> >> >> >>> On 22 Nov 2023, at 14:48, Iain Sandoe wrote: >>> >>> >>> >>>> On 22 Nov 2023, at 13:55, Arnaud Charlet wrote: >>>> >>>>>>> #if defined (__APPLE__) >>>>>>> -#include >>>>>> >>>>>> If removing unistd.h is intentional (i.e. you determined that it’s no >>>>>> longer >>>>>> needed for Darwin), then we should make that a separate patch. >>>>> >>>>> I thought that I’d had to include unistd.h for the first patch in this >>>>> thread; clearly not! >>>>> >>>>> What I hope will be the final version: >>>> >>>> OK here. >>> >>> also OK here, thanks >> >> I think this fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111909 ? >> if you agree then please add that to the commit. >> Iain > > git format-patch does so much, I forgot this, sorry: > > gcc/ada/Changelog: > > 2023-11-22 Simon Wright mailto:si...@pushface.org>> > > PR ada/111909 Can we commit this one now, please? —S > >> >>> Iain >>> >>>> >>>>> ——— 8< .——— >>>>> >>>>> In gcc/ada/adaint.c(__gnat_get_file_names_case_sensitive), the current >>>>> assumption for __APPLE__ is that file names are case-insensitive >>>>> unless __arm__ or __arm64__ are defined, in which case file names are >>>>> declared case-sensitive. >>>>> >>>>> The associated comment is >>>>> "By default, we suppose filesystems aren't case sensitive on >>>>> Windows and Darwin (but they are on arm-darwin)." >>>>> >>>>> This means that on aarch64-apple-darwin, file names are treated as >>>>> case-sensitive, which is not the default case. >>>>> >>>>> The true default position is that macOS file systems are >>>>> case-insensitive, iOS file systems are case-sensitive. >>>>> >>>>> Apple provide a header file which permits a >>>>> compile-time check for the compiler target (e.g. OSX vs IOS); if >>>>> TARGET_OS_IOS is defined as 1, this is a build for iOS. >>>>> >>>>> * gcc/ada/adaint.c >>>>> (__gnat_get_file_names_case_sensitive): Split out the __APPLE__ >>>>> check and remove the checks for __arm__, __arm64__. >>>>> For Apple, file names are by default case-insensitive unless >>>>> TARGET_OS_IOS is set. >>>>> >>>>> Signed-off-by: Simon Wright >> >
Re: [PATCH] PR ada/66205 gnatbind generates invalid code when finalization is enabled in restricted runtime
I’ve updated the original patch, which was built against 5.1.0 on x64_64-apple-darwin13, this patch is against 6.0.0-20151101 on x86_64-apple-darwin15. -- If the RTS in use is "configurable" (I believe this is the same in this context as "restricted") and includes finalization, gnatbind generates binder code that won't compile. This situation arises, for example, with an embedded RTS that incorporates the Ada 2012 generalized container iterators. I note that in the last 3 hunks of the attached patch there may be overkill; the original code checks whether the No_Finalization restriction doesn’t occur, and I’ve added a check that Configurable_Run_Time_On_Target isn’t set; I suspect, given other areas of the code, that the No_Finalization check is actually intended as a way of determining that this is a restricted runtime, and that the Configurable_Run_Time_On_Target check could replace it. The attached patch was bootstrapped/regression tested (make check-ada) against 6.0.0 on x86_64-apple-darwin15 (which confirms that the patch hasn't broken builds against the standard RTS). arm-eabi-gnatbind was successful against both an RTS with finalization and one without. gcc/ada/Changelog: 2015-11-11 Simon Wright PR ada/66205 * bindgen.adb (Gen_Adafinal): if Configurable_Run_Time_On_Target is true, generate a null body. (Gen_Main): if Configurable_Run_Time_On_Target is true, then - don't import __gnat_initialize or __gnat_finalize (as Initialize, Finalize rsp). - don't call Initialize or Finalize. gcc-6.0.0-20151101-gcc-ada-bindgen.adb.diff Description: Binary data
Re: [PATCH] PR ada/66205 gnatbind generates invalid code when finalization is enabled in restricted runtime
On 11 Nov 2015, at 19:43, Simon Wright wrote: > This situation arises, for example, with an embedded RTS that incorporates the > Ada 2012 generalized container iterators. I should add, this PR is the “other half” of PR ada/66242, which is fixed in GCC 6; so please can it be reviewed? I didn’t make it plain that the comment I’ve put in the first hunk, -- For restricted run-time libraries (ZFP and Ravenscar) tasks -- are non-terminating, so we do not want finalization. is lifted from the unpatched code at line 480, where it relates to the use of Configurable_Run_Time_On_Target for this purpose.
[PATCH] Fix PR target/104871 (macosx-version-min wrong for macOS >= Big Sur (darwin20))
This is the same sort of problem as in PR80204: at present, GCC 11 & 12 assume
that if the
OS version is >= 20, the compiler should see --mmacosx-version-min={major -
9}.{minor -1}.0,
e.g. for OS version 21.3.0 that would be 12.2.0 (the linker sees
-macosx-version-min, same
arguments).
However, the native compiler clang treats 21.3.0 as 12.0.0: the compiler sees
-triple x86_64-apple-macosx12.0.0
and the linker sees
-platform_version macos 12.0.0
the result of which is that linking an object file built with clang and one
built with gcc gives e.g.
ld: warning: object file (null.o) was built for newer macOS version (12.2)
than being linked (12.0)
I propose the following patch, which works fine for me (darwin 21.3.0).
gcc/ChangeLog:
2022-06-02 Simon Wright
PR target/104871
* config/darwin-driver.cc (darwin_find_version_from_kernel): if the OS
version is
20 (macOS 11) or greater, report the minor version and the patch
level as 0
to match Apple clang’s behaviour.
pr104871.diff
Description: Binary data
[PATCH] Fix PR target/104871 (macosx-version-min wrong for macOS >= Big Sur (darwin20))
(resent with commit message format update)
This is the same sort of problem as in PR80204: at present, GCC 11 & 12 assume
that if the
OS version is >= 20, the compiler should see --mmacosx-version-min={major -
9}.{minor -1}.0,
e.g. for OS version 21.3.0 that would be 12.2.0 (the linker sees
-macosx-version-min, same
arguments).
However, the native compiler clang treats 21.3.0 as 12.0.0: the compiler sees
-triple x86_64-apple-macosx12.0.0
and the linker sees
-platform_version macos 12.0.0
the result of which is that linking an object file built with clang and one
built with gcc gives e.g.
ld: warning: object file (null.o) was built for newer macOS version (12.2)
than being linked (12.0)
I propose the following patch, which works fine for me (darwin 21.3.0).
gcc/ChangeLog:
2022-06-02 Simon Wright
PR target/104871
* config/darwin-driver.cc (darwin_find_version_from_kernel): If the OS
version is
20 (macOS 11) or greater, report the minor version and the patch
level as 0
to match Apple clang’s behaviour.
pr104871.diff
Description: Binary data
[PATCH] Fix PR target/104871 (macosx-version-min wrong for macOS >= Big Sur (darwin20))
(resent with correct address for Iain)
This is the same sort of problem as in PR80204: at present, GCC 11 & 12 assume
that if the
OS version is >= 20, the compiler should see --mmacosx-version-min={major -
9}.{minor -1}.0,
e.g. for OS version 21.3.0 that would be 12.2.0 (the linker sees
-macosx-version-min, same
arguments).
However, the native compiler clang treats 21.3.0 as 12.0.0: the compiler sees
-triple x86_64-apple-macosx12.0.0
and the linker sees
-platform_version macos 12.0.0
the result of which is that linking an object file built with clang and one
built with gcc gives e.g.
ld: warning: object file (null.o) was built for newer macOS version (12.2)
than being linked (12.0)
I propose the following patch, which works fine for me (darwin 21.3.0).
gcc/ChangeLog:
2022-06-02 Simon Wright
PR target/104871
* config/darwin-driver.cc (darwin_find_version_from_kernel): If the OS
version is
20 (macOS 11) or greater, report the minor version and the patch
level as 0
to match Apple clang’s behaviour.
pr104871.diff
Description: Binary data
Re: [PATCH] Fix PR target/104871 (macosx-version-min wrong for macOS >= Big Sur (darwin20))
On 11 Jun 2022, at 11:37, Iain Sandoe wrote:
>
> Hi Simon,
>
> thanks for the patch.
>
>> On 11 Jun 2022, at 10:17, Simon Wright wrote:
>>
>> (resent with correct address for Iain)
>>
>> This is the same sort of problem as in PR80204: at present, GCC 11 & 12
>> assume that if the
>> OS version is >= 20, the compiler should see --mmacosx-version-min={major -
>> 9}.{minor -1}.0,
>> e.g. for OS version 21.3.0 that would be 12.2.0 (the linker sees
>> -macosx-version-min, same
>> arguments).
>>
>> However, the native compiler clang treats 21.3.0 as 12.0.0: the compiler sees
>> -triple x86_64-apple-macosx12.0.0
>> and the linker sees
>> -platform_version macos 12.0.0
>> the result of which is that linking an object file built with clang and one
>> built with gcc gives e.g.
>>
>> ld: warning: object file (null.o) was built for newer macOS version (12.2)
>> than being linked (12.0)
>>
>> I propose the following patch, which works fine for me (darwin 21.3.0).
>
> this LGTM - just need to sort out a couple of nits and an admin point.
>
> FWIW; the following are honoured in preserving the minor version (so we still
> have scope for
> mismatches if some objects are built this way and others picking up the
> kernel version) ..
>
> clang -target x86_64-apple-macosx11.3 …
> clang -mmacosx-version-min=11.3 …
> MACOSX_DEPLOYMENT_TARGET=11.3 clang … (although this seems on at least one
> version
> of xcodem to pass 12.3 to the linker.. hmmm).
Something on the lines of "the native compiler clang treats 21.3.0 as 12.0.0
(unless overridden by e.g.
MACOSX_DEPLOYMENT_TARGET=11.3 )"?
I did see in the otool -l report on a gcc 12.1.0 executable generated as above
(Darwin 21.5.0)
cmd LC_BUILD_VERSION
cmdsize 32
platform 1
minos 11.3
sdk 10.17
— don’t know where the 10.17 comes from, still there even without
MACOSX_DEPLOYMENT_TARGET.
The SDK was Xcode 13.4.1. I also have CLT 13.4.0.0.1.1651278267. clang tells ld
"-platform_version macos 11.3.0 12.3", gcc just says "-macosx_version_min 11.3".
> I guess you do not have commit access?
> if you do not have an FSF assignment for copyright, are you OK to sign this
> off using the DCO?
>
> https://gcc.gnu.org/dco.html <https://gcc.gnu.org/dco.html>
No commit access, but FSF assignment RT:1016382. Do I need to say this in the
patch email somewhere?
> for furture reference, please check that patches conform to GCC coding style
> (this one has some
> whitespace glitches)
I don’t see the whitespace glitches? (I have missed a period after ld in the
second comment)
>
> thanks,
> Iain
>
>
>> gcc/ChangeLog:
>>
>> 2022-06-02 Simon Wright mailto:si...@pushface.org>>
>>
>> PR target/104871
>> * config/darwin-driver.cc <http://darwin-driver.cc/>
>> (darwin_find_version_from_kernel): If the OS version is
>> 20 (macOS 11) or greater, report the minor version and the patch level as 0
>> to match Apple clang’s behaviour.
>>
>>
[PATCH] PR target/80556
This PR was raised because of a bootstrap failure on Darwin. The cause was a mishandled exception raised by Rtsfind.Load_Fail while processing g-exptty.adb. g-exptty.adb had been updated on 2017-04-25 (but so had a lot of other parts of GNAT). Since a lot of other compilations had been performed successfully by this point in the build, one may assume that this was the first that actually caused an exception. The exception was mishandled because the default ldflags call up -static-libgcc, and (on Darwin; not, it seems, on Debian [jessie]), the notes on -static-libgcc/-shared-libgcc at https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html which say However, if a library or main executable is supposed to throw or catch exceptions, you must link it using the G++ driver, as appropriate for the languages used in the program, or using the option -shared-libgcc, such that it is linked with the shared libgcc. do actually apply (this affects all the Ada executables, in this case specifically gnat1). The result of the mishandling of the exception is that gnat1 crashes with SIGILL. The change proposed here is to include -lSystem in such a way that it's called in before the (static) libgcc, and thus supplies the required exception unwinders. Bootstrapped on Darwin 16.6.0 and on Debian Jessie. A question: I've checked for x86_64-apple-darwin*, is this OK or should it be more restrictive? Changelog: 2017-06-09 Simon Wright PR target/80556 * configure.ac (stage1_ldflags): For Darwin, include -lSystem. (poststage1_ldflags): likewise. * configure: regenerated. 80556-3.diff Description: Binary data
Re: [PATCH] fix PR ada/80888
Ping If OK, can it be applied please? (patch applies cleanly to current sources) > On 27 May 2017, at 16:58, Simon Wright wrote: > > The GNAT reference manual says in 11.6 Wide_Text_IO > <https://gcc.gnu.org/onlinedocs/gnat_rm/Wide_005fText_005fIO.html>, > > "The default encoding method for the standard files, and for opened > files for which no WCEM parameter is given in the FORM string matches > the wide character encoding specified for the main program (the > default being brackets encoding if no coding method was specified with > -gnatW)." > > This is not true; the default is brackets encoding regardless of the > coding method specified with -gnatW. > > The attached patch (to 7.1.0) corrects this. Tested on > x86_84-apple-darwin15 by rebuilding the library (cd gcc; make gnatlib > gnatlib-shared) and make -j4 check-ada, > > === acats Summary === > # of expected passes 2320 > # of unexpected failures 0 > /Volumes/Miscellaneous/tmp/gcc-7.1.0/gcc/testsuite/ada/acats/run_all.sh > completed at Fri 26 May 2017 15:44:52 BST > > === gnat Summary === > > # of expected passes 2569 > # of expected failures24 > # of unsupported tests7 > /Volumes/Miscellaneous/tmp/gcc-7.1.0-build/gcc/gnatmake version 7.1.0 > > gcc/ada/Changelog: > > 2017-05-27 Simon Wright > > PR ada/80888 > * a-textio.adb (Set_WCEM): default the file's wide character encoding > method to Default_WCEM, not WCEM_Brackets. > * a-witeio.adb: likewise. > * a-ztexio.adb: likewise. > >
Re: [PATCH] PR target/80556
On 28 Jun 2017, at 18:40, Jeff Law wrote:
>
> On 06/09/2017 07:57 AM, Simon Wright wrote:
>> 2017-06-09 Simon Wright
>>
>>PR target/80556
>>* configure.ac (stage1_ldflags): For Darwin, include -lSystem.
>> (poststage1_ldflags): likewise.
>>* configure: regenerated.
> I'm a bit confused here. Isn't -lSystem included in darwin's LIB_SPEC
> in which case the right things ought to already be happening, shouldn't it?
The specs that involve -lSystem are
*link_gcc_c_sequence:
%:version-compare(>= 10.6 mmacosx-version-min= -no_compact_unwind)
%{!static:%{!static-libgcc: %:version-compare(>= 10.6
mmacosx-version-min= -lSystem) } }
%{fno-pic|fno-PIC|fno-pie|fno-PIE|fapple-kext|mkernel|static|mdynamic-no-pic:
%:version-compare(>= 10.7 mmacosx-version-min= -no_pie) } %G %L
*lib:
%{!static:-lSystem}
but I also see
*libgcc:
%{static-libgcc|static: -lgcc_eh -lgcc;
which might be the root of the problem?
Looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80556#c39, I report that
$ gnatmake raiser -largs -static-libgcc -static-libstdc++
resulted in the link command
/usr/bin/ld -dynamic -arch x86_64 -macosx_version_min 10.12.0
-weak_reference_mismatches non-weak -o raiser -L./
-L/opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0/adalib/
-L/opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0
-L/opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0/../../.. b~raiser.o
./raiser.o -v
/opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0/adalib/libgnat.a
-no_compact_unwind -lgcc_eh -lgcc -lSystem
i.e. -lSystem is *after* -lgcc, so that its exception handling won't be invoked.
I don't know what -lgcc_eh does, but my patch would be pretty much equivalent
to changing the libgcc spec above to
*libgcc:
%{static-libgcc|static: -lSystem -lgcc_eh -lgcc;
and if that would be OK it would obviously be much better.
I've rebuilt gcc-8-20170528 with this change alone (i.e. not the patch
currently posted here), successfully.
If I propose this alternative patch, should it be a new post, or should I
continue this thread?
Re: [PATCH] PR ada/66205 gnatbind generates invalid code when finalization is enabled in restricted runtime
On 7 Mar 2017, at 16:20, Simon Wright wrote: > > On 19 Dec 2015, at 22:05, Simon Wright wrote: >> >> On 12 Nov 2015, at 10:02, Arnaud Charlet wrote: >>> >>>>> This situation arises, for example, with an embedded RTS that >>>>> incorporates the >>>>> Ada 2012 generalized container iterators. >>>> >>>> I should add, this PR is the ???other half??? of PR ada/66242, which is >>>> fixed >>>> in GCC 6; so please can it be reviewed? >>> >>> The proper patch for PR ada/66242 hasn't been committed yet (it's pending), >>> so I'd rather review the situation once PR ada/66242 is dealt with. >>> >>> I'm not convinced at all that your patch is the way to go, so I'd rather >>> consider it only after PR ada/66242 is solved properly. >> >> Looks as though PR ada/66242 has been sorted out. >> >> Since we can now *compile* code that is built with finalization enabled in a >> restricted runtime, but we can't *bind* it, could we take another look at >> this? the patch I provided in this thread still applies at snapshot 20151213 >> with minor offsets (8). > > Same problem exists in gcc version 7.0.1 20170302 (experimental) (GCC). and with gcc 8.0.0 20171102 (experimental) (r254339); and there's been no change to the affected file (bindgen.adb) since then. I've come up with a considerably simpler patch, which merely causes the procedure adafinal to be generated with a null body if the restriction No_Task_Termination is set (note, this restriction is part of the Ravenscar profile; if tasks can't terminate, program level finalization can't happen [ARM 10.2(25), "When the environment task completes (normally or abnormally), it waits for the termination of all such tasks, and then finalizes any remaining objects of the partition."] I've bootstrapped the compiler (x86_64-apple-darwin15), and "make check-ada" produces the same results with and without the patch (the same 3 FAILs occur in both, in gnat.sum). For the arm-eabi compiler, I successfully make and run builds for an STM32F4 target without exception propagation but with and without finalization. gcc/ada/Changelog: 2017-12-05 Simon Wright PR ada/66205 * bindgen.adb (Gen_AdaFinal): If the restriction No_Task_Termination is present, generate a null body. bindgen.adb.diff Description: Binary data
[PATCH] PR ada/66242 Front-end error if exception propagation disabled
There are two issues in code expansion related to Ada.Finalization that aren’t properly handled in the presence of pragma Restriction (No_Exception_Propagation). The expanded code attempts to catch any exception propagated during Finalize and record that it happened, and later, if there was a caught exception, raise Program_Error instead. Given No_Exception_Propagation, this fails because no exception could have been propagated from Finalize in the first place, so Exp_Ch7.Build_Object_Declarations detects that No_Exception_Propagation is in force and omits the creation of the temporary variable which would have recorded the propagation. The attached patch modifies the two compiler source files affected and adds a new test case. Patched 5.1.0 code, bootstrap and regression test on x86_64-apple-darwin13. The patches apply to trunk with minor offsets. gcc/ada/Changelog: 2015–6-14 Simon Wright pr66242.diff Description: Binary data
[PATCH] PR ada/66205 gnatbind generates invalid code when finalization is enabled in restricted runtime
If the RTS in use is "configurable" (I believe this is the same in this context as "restricted") and includes finalization, gnatbind generates binder code that won't compile. This situation arises, for example, with an embedded RTS that incorporates the Ada 2012 generalized container iterators. The attached patch was bootstrapped/regression tested (make check-ada) against 5.1.0 on x86_64-apple-darwin13 (which confirms that the patch hasn't broken builds against the standard RTS). arm-eabi-gnatbind was successful against both an RTS with finalization and one without. The patch applies with no offset to the trunk. gcc/ada/Changelog: 2015-6-15 Simon Wright PR ada/66205 * bindgen.adb (Gen_Adafinal): if Configurable_Run_Time_On_Target is true, generate a null body. (Gen_Main): if Configurable_Run_Time_On_Target is true, then - don't import __gnat_initialize or __gnat_finalize (as Initialize, Finalize rsp). - don't call Initialize or Finalize. pr66205.diff Description: Binary data
[PATCH] PR ada/89583, GNAT.Sockets.Bind_Socket fails with IPv4 address
With GCC9, GNAT.Sockets includes support for IPv6. Sockaddr is an Unchecked_Union, which now includes IPv6 fields, bringing the total possible size to 28 bytes. The code in Bind_Socket currently calculates the length of the struct sockaddr to be passed to bind(2) as this size, which (at any rate on Darwin x86_64) results in failure (EINVAL). This patch provides the required length explicitly from the socket's family. Tested by rebuilding the compiler with --disable-bootstrap and re-running the reproducer. gcc/ada/Changelog: 2019-03-04 Simon Wright PR ada/89583 * libgnat/g-socket.adb (Bind_Socket): Calculate Len (the significant length of the Sockaddr) using the Family of the Address parameter. pr89583.diff Description: Binary data
Re: [PATCH] PR ada/89583, GNAT.Sockets.Bind_Socket fails with IPv4 address
On 4 Mar 2019, at 19:48, Simon Wright wrote: > With GCC9, GNAT.Sockets includes support for IPv6. Sockaddr is an > Unchecked_Union, which now includes IPv6 fields, bringing the total possible > size to 28 bytes. The code in Bind_Socket currently calculates the length of > the struct sockaddr to be passed to bind(2) as this size, which (at any rate > on Darwin x86_64) results in failure (EINVAL). My previous version of this patch calculated the required length explicitly, rather than re-using the same calculation already made in GNAT.Sockets.Thin_Common.Lengths. Also added new testcase. Tested by rebuilding gnatlib (make -C gcc gnatlib) and running make check-gnat: === gnat Summary === # of expected passes2961 # of expected failures 23 # of unsupported tests 10 gcc/ada/Changelog: 2019-03-05 Simon Wright PR ada/89583 * libgnat/g-socket.adb (Bind_Socket): Calculate Len (the significant length of the Sockaddr) using GNAT.Sockets.Thin_Common.Lengths. gcc/testsuite/Changelog: 2019-03-05 Simon Wright PR ada/89583 * gnat.dg/socket2.adb: New. pr89583.diff Description: Binary data
Re: [PATCH] PR ada/89583, GNAT.Sockets.Bind_Socket fails with IPv4 address
Arno, OK. --S > On 5 Mar 2019, at 20:38, Arnaud Charlet wrote: > > Simon, > > Thanks for the patch. > We already have a fix pending for that in our tree that we will merge. > > Arno > >> On 4 Mar 2019, at 20:48, Simon Wright wrote: >> >> With GCC9, GNAT.Sockets includes support for IPv6. Sockaddr is an >> Unchecked_Union, which now includes IPv6 fields, bringing the total possible >> size to 28 bytes. The code in Bind_Socket currently calculates the length of >> the struct sockaddr to be passed to bind(2) as this size, which (at any rate >> on Darwin x86_64) results in failure (EINVAL). >> >> This patch provides the required length explicitly from the socket's family. >> >> Tested by rebuilding the compiler with --disable-bootstrap and re-running >> the reproducer. >> >> gcc/ada/Changelog: >> >> 2019-03-04 Simon Wright >> >> PR ada/89583 >> * libgnat/g-socket.adb (Bind_Socket): Calculate Len (the significant >> length of >> the Sockaddr) using the Family of the Address parameter. >> >> >> >
Re: [PATCH] PR ada/89583, GNAT.Sockets.Bind_Socket fails with IPv4 address
Ping? > On 5 Mar 2019, at 20:43, Simon Wright wrote: > > Arno, > > OK. > > --S > >> On 5 Mar 2019, at 20:38, Arnaud Charlet wrote: >> >> Simon, >> >> Thanks for the patch. >> We already have a fix pending for that in our tree that we will merge. >> >> Arno >> >>> On 4 Mar 2019, at 20:48, Simon Wright wrote: >>> >>> With GCC9, GNAT.Sockets includes support for IPv6. Sockaddr is an >>> Unchecked_Union, which now includes IPv6 fields, bringing the total >>> possible size to 28 bytes. The code in Bind_Socket currently calculates the >>> length of the struct sockaddr to be passed to bind(2) as this size, which >>> (at any rate on Darwin x86_64) results in failure (EINVAL). >>> >>> This patch provides the required length explicitly from the socket's family. >>> >>> Tested by rebuilding the compiler with --disable-bootstrap and re-running >>> the reproducer. >>> >>> gcc/ada/Changelog: >>> >>> 2019-03-04 Simon Wright >>> >>> PR ada/89583 >>> * libgnat/g-socket.adb (Bind_Socket): Calculate Len (the significant >>> length of >>>the Sockaddr) using the Family of the Address parameter. >>> >>> >>> >> >
Re: [PATCH] PR ada/89583, GNAT.Sockets.Bind_Socket fails with IPv4 address
Thanks, Pierre-Marie: it'd be a shame if 9.1 couldn't handle IPv4. --S > On 20 Mar 2019, at 13:31, Pierre-Marie de Rodat wrote: > > Hello Simon, > > On 3/19/19 5:02 PM, Simon Wright wrote: >> Ping? > > Sorry for the delay! Thank you for the notice; I’ll try to get our fix ported > as soon as possible (hopefully before the end of the week). > > Cheers, > > -- > Pierre-Marie de Rodat
[PATCH] Fix PR ada/71358
This fixes a minor problem where GNAT.Command_Line.Getopt raises CE if there are in fact no program-specified options (only the internally-supplied -h, --help are meant to be available). Tested on GCC 6.1.0, x86_64-apple-darwin15. If OK, can someone commit it (I can't). gcc/ada/Changelog: 2016-05-31 Simon Wright PR ada/71358 * g-comlin.adb: bump copyright year. (Display_Section_Help): don't deference Config.Switches if it's null. (Getopt): likewise. g-comlin.adb.diff Description: Binary data
Re: [PATCH] PR target/80556
On 29 Jun 2017, at 21:41, Simon Wright wrote:
>
> On 28 Jun 2017, at 18:40, Jeff Law wrote:
>>
>> On 06/09/2017 07:57 AM, Simon Wright wrote:
>>> 2017-06-09 Simon Wright
>>>
>>> PR target/80556
>>> * configure.ac (stage1_ldflags): For Darwin, include -lSystem.
>>> (poststage1_ldflags): likewise.
>>> * configure: regenerated.
>> I'm a bit confused here. Isn't -lSystem included in darwin's LIB_SPEC
>> in which case the right things ought to already be happening, shouldn't it?
>
> The specs that involve -lSystem are
>
> *link_gcc_c_sequence:
> %:version-compare(>= 10.6 mmacosx-version-min= -no_compact_unwind)
> %{!static:%{!static-libgcc: %:version-compare(>= 10.6
> mmacosx-version-min= -lSystem) } }
> %{fno-pic|fno-PIC|fno-pie|fno-PIE|fapple-kext|mkernel|static|mdynamic-no-pic:
> %:version-compare(>= 10.7 mmacosx-version-min= -no_pie) } %G %L
>
> *lib:
> %{!static:-lSystem}
>
> but I also see
>
> *libgcc:
> %{static-libgcc|static: -lgcc_eh -lgcc;
>
> which might be the root of the problem?
>
> Looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80556#c39, I report
> that
>
> $ gnatmake raiser -largs -static-libgcc -static-libstdc++
>
> resulted in the link command
>
> /usr/bin/ld -dynamic -arch x86_64 -macosx_version_min 10.12.0
> -weak_reference_mismatches non-weak -o raiser -L./
> -L/opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0/adalib/
> -L/opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0
> -L/opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0/../../.. b~raiser.o
> ./raiser.o -v
> /opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0/adalib/libgnat.a
> -no_compact_unwind -lgcc_eh -lgcc -lSystem
>
> i.e. -lSystem is *after* -lgcc, so that its exception handling won't be
> invoked.
>
> I don't know what -lgcc_eh does, but my patch would be pretty much equivalent
> to changing the libgcc spec above to
>
> *libgcc:
> %{static-libgcc|static: -lSystem -lgcc_eh -lgcc;
>
> and if that would be OK it would obviously be much better.
>
> I've rebuilt gcc-8-20170528 with this change alone (i.e. not the patch
> currently posted here), successfully.
I've rebuilt and tested gcc-8-20170820 with this change, successfully.
gcc/Changelog:
2017-09-01 Simon Wright
PR target/80556
* config/darwin.h (REAL_LIBGCC_SPEC): for static-libgcc|static, include
-lSystem first.
80556-darwin.h.diff
Description: Binary data
[PATCH]: PR target/80204 (Darwin macosx-version-min problem)
In gcc/config/darwin-driver.c, darwin_find_version_from_kernel() assumes
that the minor version in the Darwin kernel version (16.7.0, => minor
version 7) is equal to the bugfix component of the macOS version, so that
the compiler receives -mmacosx-version-min=10.12.7 and the linker receives
-macosx_version_min 10.12.7.
Unfortunately, Apple don’t apply this algorithm; the macOS version is
actually 10.12.6.
Getting this wrong means that it’s impossible to run an executable from
within a bundle: Sierra complains "You have macOS 10.12.6. The application
requires macOS 10.12.7 or later".
A workround would perhaps be to link the executable with
-Wl,-macosx_version_min,`sw_vers -productVersion` (I assume that it’s only
the linker phase that matters?)
I see that Apple’s gcc (Apple LLVM version 8.0.0
(clang-800.0.42.1)) specifies - only at link time -
-macosx_version_min 10.12.0
This patch does the same.
gcc/Changelog:
2017-09-01 Simon Wright
PR target/80204
* config/darwin-driver.c (darwin_find_version_from_kernel): eliminate
calculation of the
minor version, always output as 0.
80204-darwin-driver.c.diff
Description: Binary data
[PATCH]: PR target/80204 (Darwin macosx-version-min problem)
In gcc/config/darwin-driver.c, darwin_find_version_from_kernel() assumes
that the minor version in the Darwin kernel version (16.7.0, => minor
version 7) is equal to the bugfix component of the macOS version, so that
the compiler receives -mmacosx-version-min=10.12.7 and the linker receives
-macosx_version_min 10.12.7.
Unfortunately, Apple don’t apply this algorithm; the macOS version is
actually 10.12.6.
Getting this wrong means that it’s impossible to run an executable from
within a bundle: Sierra complains "You have macOS 10.12.6. The application
requires macOS 10.12.7 or later".
A workround would perhaps be to link the executable with
-Wl,-macosx_version_min,`sw_vers -productVersion` (I assume that it’s only
the linker phase that matters?)
I see that Apple’s gcc (Apple LLVM version 8.0.0
(clang-800.0.42.1)) specifies - only at link time -
-macosx_version_min 10.12.0
This patch does the same.
gcc/Changelog:
2017-09-01 Simon Wright
PR target/80204
* config/darwin-driver.c (darwin_find_version_from_kernel): eliminate
calculation of the
minor version, always output as 0.
80204-darwin-driver.c.diff
Description: Binary data
Re: [PATCH]: PR target/80204 (Darwin macosx-version-min problem)
(Apologies if this is a duplicate: Mac Mail vs the list's requirement for plain text) On 4 Sep 2017, at 07:09, Jeff Law wrote: > > On 09/01/2017 04:05 PM, Simon Wright wrote: >> In gcc/config/darwin-driver.c, darwin_find_version_from_kernel() assumes >> that the minor version in the Darwin kernel version (16.7.0, => minor >> version 7) is equal to the bugfix component of the macOS version, so that >> the compiler receives -mmacosx-version-min=10.12.7 and the linker receives >> -macosx_version_min 10.12.7. >> >> Unfortunately, Apple don’t apply this algorithm; the macOS version is >> actually 10.12.6. >> >> Getting this wrong means that it’s impossible to run an executable from >> within a bundle: Sierra complains "You have macOS 10.12.6. The application >> requires macOS 10.12.7 or later". >> >> A workround would perhaps be to link the executable with >> -Wl,-macosx_version_min,`sw_vers -productVersion` (I assume that it’s only >> the linker phase that matters?) >> >> I see that Apple’s gcc (Apple LLVM version 8.0.0 >> (clang-800.0.42.1)) specifies - only at link time - >> -macosx_version_min 10.12.0 >> >> This patch does the same. >> >> gcc/Changelog: >> >> 2017-09-01 Simon Wright >> >> PR target/80204 >> * config/darwin-driver.c (darwin_find_version_from_kernel): eliminate >> calculation of the >>minor version, always output as 0. >> > OK > jeff Great! Can it be applied, please, when convenient --S
Re: [PATCH]: PR target/80204 (Darwin macosx-version-min problem)
On 1 Sep 2017, at 23:05, Simon Wright wrote: > > 2017-09-01 Simon Wright > > PR target/80204 > * config/darwin-driver.c (darwin_find_version_from_kernel): eliminate > calculation of the > minor version, always output as 0. Like this? Do you need the patch to be resubmitted as well? gcc/Changelog 2017-09-01 Simon Wright PR target/80204 * config/darwin-driver.c (darwin_find_version_from_kernel): Eliminate calculation of the minor version, always output as 0.
[PATCH] fix PR ada/80888
The GNAT reference manual says in 11.6 Wide_Text_IO
<https://gcc.gnu.org/onlinedocs/gnat_rm/Wide_005fText_005fIO.html>,
"The default encoding method for the standard files, and for opened
files for which no WCEM parameter is given in the FORM string matches
the wide character encoding specified for the main program (the
default being brackets encoding if no coding method was specified with
-gnatW)."
This is not true; the default is brackets encoding regardless of the
coding method specified with -gnatW.
The attached patch (to 7.1.0) corrects this. Tested on
x86_84-apple-darwin15 by rebuilding the library (cd gcc; make gnatlib
gnatlib-shared) and make -j4 check-ada,
=== acats Summary ===
# of expected passes2320
# of unexpected failures0
/Volumes/Miscellaneous/tmp/gcc-7.1.0/gcc/testsuite/ada/acats/run_all.sh
completed at Fri 26 May 2017 15:44:52 BST
=== gnat Summary ===
# of expected passes2569
# of expected failures 24
# of unsupported tests 7
/Volumes/Miscellaneous/tmp/gcc-7.1.0-build/gcc/gnatmake version 7.1.0
gcc/ada/Changelog:
2017-05-27 Simon Wright
PR ada/80888
* a-textio.adb (Set_WCEM): default the file's wide character encoding
method to Default_WCEM, not WCEM_Brackets.
* a-witeio.adb: likewise.
* a-ztexio.adb: likewise.
wcem-fix.diff
Description: Binary data
Re: [Ada] Fix PR ada/78845
On 21 Dec 2016, at 14:52, Arnaud Charlet wrote: > >>> Yes, please resend an updated patch. >> >> The function Ada.Numerics.Generic_Real_Arrays.Inverse is required >> (ARM G.3.1(72)) to >> return a matrix with the bounds of the dimension indices swapped, i.e. >> result'Range(1) == >> input'Range(2) and vice versa. The present code gets result'Range(1) >> correct, but >> result'Range(2) always starts at 1. >> >> Of course, many users would always use ranges starting at 1 and wouldn't see >> a >> problem. >> >> The same applies to Ada.Numerics.Complex_Real_Arrays.Inverse (ARM >> G.3.2(140)). > > Updated patch OK as well. > >> 2016-12-21 Simon Wright > <mailto:si...@pushface.org>> >> >> PR ada/78845 >> * a-ngcoar.adb (Inverse): call Unit_Matrix with First_1 set to >> A'First(2) >>and vice versa. >> * a-ngrear.adb (Inverse): likewise. Can the patch be committed, please, when convenient? (patch reposted below for convenience) pr78845.diff Description: Binary data
Re: [PATCH] PR target/80556
On 18 Sep 2017, at 21:09, Iain Sandoe wrote: > > Hi Simon, > >> On 29 Jun 2017, at 21:41, Simon Wright wrote: >> >> On 28 Jun 2017, at 18:40, Jeff Law wrote: >>> >>> On 06/09/2017 07:57 AM, Simon Wright wrote: >>>> 2017-06-09 Simon Wright >>>> >>>> PR target/80556 >>>> * configure.ac (stage1_ldflags): For Darwin, include -lSystem. >>>>(poststage1_ldflags): likewise. >>>> * configure: regenerated. >>> I'm a bit confused here. Isn't -lSystem included in darwin's LIB_SPEC >>> in which case the right things ought to already be happening, shouldn't it? >> >> The specs that involve -lSystem are > >> I've rebuilt gcc-8-20170528 with this change alone (i.e. not the patch >> currently posted here), successfully. >> >> If I propose this alternative patch, should it be a new post, or should I >> continue this thread? > > thanks for the patch. > > The basic idea seems sound - as a workaround (as noted in comment #20 in the > PR, we should really rationalise the libgcc/crts stuff to reflect the modern > world, but these things take time...). > > The patch as you have it would apply to every version of Darwin. > > AFAICT from the published sources, i386 Darwin should be able to work with > the libgcc unwinder (and all earlier Darwin *have* to) - so I’ve proposed a > modified patch in the PR that makes the changes specific to m64 x86 and > doesn’t make any alteration for PPC and/or Darwin < 10. That sounds like the right thing to do. I hadn't considered the older hardware/os issues (I only have kit back to macOS 10.11, Darwin 15).
Re: [PATCH] PR ada/66205 gnatbind generates invalid code when finalization is enabled in restricted runtime
On 19 Dec 2015, at 22:05, Simon Wright wrote: > > On 12 Nov 2015, at 10:02, Arnaud Charlet wrote: >> >>>> This situation arises, for example, with an embedded RTS that >>>> incorporates the >>>> Ada 2012 generalized container iterators. >>> >>> I should add, this PR is the ???other half??? of PR ada/66242, which is >>> fixed >>> in GCC 6; so please can it be reviewed? >> >> The proper patch for PR ada/66242 hasn't been committed yet (it's pending), >> so I'd rather review the situation once PR ada/66242 is dealt with. >> >> I'm not convinced at all that your patch is the way to go, so I'd rather >> consider it only after PR ada/66242 is solved properly. > > Looks as though PR ada/66242 has been sorted out. > > Since we can now *compile* code that is built with finalization enabled in a > restricted runtime, but we can't *bind* it, could we take another look at > this? the patch I provided in this thread still applies at snapshot 20151213 > with minor offsets (8). Same problem exists in gcc version 7.0.1 20170302 (experimental) (GCC). Note, what may not have been clear before and is now relevant given AdaCore's -full- embedded runtimes, the problem occurs when the runtime doesn't support exception propagation but does support finalization. As far as gnatbind is concerned, "runtime does not support exception propagation" appears to be indicated by "Suppress_Standard_Library_On_Target" rather than "Cumulative_Restrictions.Set (No_Exception_Propagation)" - the latter is partition-wide, so could be used instead maybe? I attach an updated patch. I've tested the new gnatbind against gcc version 7.0.1 20170302 (experimental) (GCC) on native x86_64-apple-darwin16 by running 'make check-ada', and for --target=arm-eabi by successfully making and running builds for an STM32F4 target without exception propagation but with and without finalization. I used the same gnatbind for both targets - renamed to arm-eabi-gnatbind for those builds - seeing there's no target dependence in gnatbind itself. gcc/ada/Changelog: 2017-03-07 Simon Wright PR ada/66205 * bindgen.adb: If the restriction No_Finalization is absent (i.e. finalization is supported) but Suppress_Standard_Library_On_Target is true, then - don't import __gnat_initialize or __gnat_finalize (as Initialize, Finalize rsp). - don't call Initialize or Finalize. - don't generate or call adafinal. gcc-ada-bindgen.adb.diff Description: Binary data
[Ada] Fix PR ada/78845
The function Ada.Numerics.Generic_Real_Arrays.Inverse is required (ARM G.3.1(72)) to return a matrix with the bounds of the dimension indices swapped, i.e. result'Range(1) == input'Range(2) and vice versa. The present code gets result'Range(1) correct, but result'Range(2) always starts at 1. Of course, many users would always use ranges starting at 1 and wouldn't see a problem. 2016-12-17 Simon Wright PR ada/78845 * a-ngrear.adb (Inverse): call Unit_Matrix with First_1 set to A'First(2) and vice versa. a-ngrear.adb.diff Description: Binary data
Re: [PATCH] PR ada/66205 gnatbind generates invalid code when finalization is enabled in restricted runtime
On 12 Nov 2015, at 10:02, Arnaud Charlet wrote: > >>> This situation arises, for example, with an embedded RTS that >>> incorporates the >>> Ada 2012 generalized container iterators. >> >> I should add, this PR is the ???other half??? of PR ada/66242, which is fixed >> in GCC 6; so please can it be reviewed? > > The proper patch for PR ada/66242 hasn't been committed yet (it's pending), > so I'd rather review the situation once PR ada/66242 is dealt with. > > I'm not convinced at all that your patch is the way to go, so I'd rather > consider it only after PR ada/66242 is solved properly. Looks as though PR ada/66242 has been sorted out. Since we can now *compile* code that is built with finalization enabled in a restricted runtime, but we can't *bind* it, could we take another look at this? the patch I provided in this thread still applies at snapshot 20151213 with minor offsets (8).
Re: [PATCH] fix PR ada/42978
On 8 Feb 2010, at 14:18, Arnaud Charlet wrote: >> OK, second try. >> >> Tested on gcc version 4.5.0 20100207 (experimental) [trunk revision 156574] >> (GCC). >> >> 2010-02-07 Simon Wright >> >> PR ada/42978 >> * mlib-utl.adb (Ar): Output ranlib options if verbose. > > This is OK, thanks. Can this be applied, please? The patch still applies at r178911. 2010-02-07 Simon Wright PR ada/42978 * mlib-utl.adb (Ar): Output ranlib options if verbose. gcc-ada-mlib-utl.adb.diff Description: Binary data
[PATCH] ada: PR target/117538 Traceback includes load address if executable is PIE.
If s-trasym.adb (System.Traceback.Symbolic, used as a renaming by
GNAT.Traceback.Symbolic) is given a traceback from a
position-independent executable, it does not include the executable's
load address in the report. This is necessary in order to decode the
traceback report.
Note, this has already been done for s-trasym__dwarf.adb, which really
does produce a symbolic traceback; s-trasym.adb is the version used in
systems which don't actually support symbolication.
Bootstrapped and regtested (ada onlyj) on x86_64-apple-darwin.
* gcc/ada/libgnat/s-trasym.adb: Returns the traceback in the required
form. Note that leading zeros are trimmed from hexadecimal strings.
(Symbolic_Traceback): Import Executable_Load_Address.
(Trim_Hex): New internal function to trim leading '0' characters
from a hexadecimal string.
(Load_Address): New, from call to Executable_Load_Address.
(One_If_Executable_Is_PI): New, 0 if Load_Address is null, 1 if
not.
(Max_Image_Length): New, found by calling System.Address_Image on
the first address in the traceback. NB, doesn't include "0x".
(Load_Address_Prefix): New, String containing the required value.
(Max_Length_Needed): New, computed using the number of elements
in the traceback plus the load address, if the executable is PIE.
(Result): New String of the required length (which will be an
overestimate).
2024-11-13 Simon Wright
gcc/ada/Changelog:
PR target/117538
* libgnat/s-trasym.adb: Returns the traceback in the required
form. Note that leading zeros are trimmed from hexadecimal strings.
—
diff --git a/gcc/ada/libgnat/s-trasym.adb b/gcc/ada/libgnat/s-trasym.adb
index 894fcf37ffd..7172214453f 100644
--- a/gcc/ada/libgnat/s-trasym.adb
+++ b/gcc/ada/libgnat/s-trasym.adb
@@ -53,19 +53,75 @@ package body System.Traceback.Symbolic is
else
declare
-Img : String := System.Address_Image (Traceback (Traceback'First));
-
-Result : String (1 .. (Img'Length + 3) * Traceback'Length);
-Last : Natural := 0;
+function Executable_Load_Address return System.Address;
+pragma Import
+ (C, Executable_Load_Address,
+ "__gnat_get_executable_load_address");
+
+function Trim_Hex (S : String) return String;
+function Trim_Hex (S : String) return String is
+ Non_0 : Positive;
+begin
+ for J in S'Range loop
+ if S (J) /= '0' or else J = S'Last then
+ Non_0 := J;
+ exit;
+ end if;
+ end loop;
+ return S (Non_0 .. S'Last);
+end Trim_Hex;
+
+Load_Address : constant System.Address :=
+ Executable_Load_Address;
+One_If_Executable_Is_PI : constant Natural :=
+ Boolean'Pos (Load_Address /= Null_Address);
+
+-- How long is an Address_Image?
+Max_Image_Length : constant Natural :=
+ System.Address_Image (Traceback (Traceback'First))'
+Length;
+
+Load_Address_Prefix : constant String :=
+ "Load address: ";
+
+Max_Length_Needed : constant Positive :=
+ (Load_Address_Prefix'Length *
+ One_If_Executable_Is_PI) +
+ (Max_Image_Length + 3) *
+(Traceback'Length + One_If_Executable_Is_PI) +
+ 2;
+
+Result : String (1 .. Max_Length_Needed);
+
+Last : Natural := 0;
begin
+
+if One_If_Executable_Is_PI /= 0 then
+ declare
+ item : constant String :=
+Load_Address_Prefix & "0x" &
+Trim_Hex
+ (System.Address_Image (Load_Address)) &
+ASCII.LF;
+ begin
+ Last := item'Length;
+ Result (1 .. Last) := item;
+ end;
+end if;
+
for J in Traceback'Range loop
- Img := System.Address_Image (Traceback (J));
- Result (Last + 1 .. Last + 2) := "0x";
- Last := Last + 2;
- Result (Last + 1 .. Last + Img'Length) := Img;
- Last := Last + Img'Length + 1;
- Result (Last) := ' ';
+ declare
+ Img : constant String :=
+Trim_Hex
+ (System.Address_Image (Traceback (J)));
+ begin
+ Result (Last + 1 .. Last + 2) := "0x";
+ Last := Last + 2;
+ Result (Last
[PATCH v2] ada: PR target/117538 Traceback includes load address if executable is PIE.
If s-trasym.adb (System.Traceback.Symbolic, used as a renaming by GNAT.Traceback.Symbolic) is given a traceback from a position-independent executable, it does not include the executable's load address in the report. This is necessary in order to decode the traceback report. Note, this has already been done for s-trasym__dwarf.adb, which really does produce a symbolic traceback; s-trasym.adb is the version used in systems which don't actually support symbolication. Bootstrapped and regtested (ada onlyj) on x86_64-apple-darwin. * gcc/ada/libgnat/s-trasym.adb: Returns the traceback in the required form. Note that leading zeros are trimmed from hexadecimal strings. (Symbolic_Traceback): Import Executable_Load_Address. (Load_Address): New, from call to Executable_Load_Address. (One_If_Executable_Is_PI): New, 0 if Load_Address is null, 1 if not. (Image_Length): New, found by calling System.Address_Image on the first address in the traceback. NB, doesn't include "0x". (Load_Address_Prefix): New, String containing the required value. (Length_Needed): New, computed using the number of elements in the traceback, plus the load address if the executable is PIE. (Result): New String of the required length (which will be an overestimate). 2024-11-24 Simon Wright gcc/ada/Changelog: PR target/117538 * libgnat/s-trasym.adb: Returns the traceback, with the program load address if applicable. --- diff --git a/gcc/ada/libgnat/s-trasym.adb b/gcc/ada/libgnat/s-trasym.adb index 894fcf37ffd..5351f6fda9b 100644 --- a/gcc/ada/libgnat/s-trasym.adb +++ b/gcc/ada/libgnat/s-trasym.adb @@ -53,19 +53,63 @@ package body System.Traceback.Symbolic is else declare -Img : String := System.Address_Image (Traceback (Traceback'First)); - -Result : String (1 .. (Img'Length + 3) * Traceback'Length); -Last : Natural := 0; +function Executable_Load_Address return System.Address; +pragma Import + (C, Executable_Load_Address, + "__gnat_get_executable_load_address"); + +Load_Address : constant System.Address := + Executable_Load_Address; +One_If_Executable_Is_PI : constant Natural := + Boolean'Pos (Load_Address /= Null_Address); + +-- How long is an Address_Image? (hex digits only). +Image_Length : constant Natural := + System.Address_Image (Traceback (Traceback'First))' +Length; + +Load_Address_Prefix : constant String := + "Load address: "; + +-- For each address to be output, we need the preceding "%x" +-- and a trailing space, making 3 additional characters. +-- There are 2 additional LFs. +Length_Needed : constant Positive := + (Load_Address_Prefix'Length * + One_If_Executable_Is_PI) + + (Image_Length + 3) * +(Traceback'Length + One_If_Executable_Is_PI) + + 2; + +Result : String (1 .. Length_Needed); + +Last : Natural := 0; begin + +if One_If_Executable_Is_PI /= 0 then + declare + Item : constant String := +Load_Address_Prefix & "0x" & +System.Address_Image (Load_Address) & +ASCII.LF; + begin + Last := Item'Length; + Result (1 .. Last) := Item; + end; +end if; + for J in Traceback'Range loop - Img := System.Address_Image (Traceback (J)); - Result (Last + 1 .. Last + 2) := "0x"; - Last := Last + 2; - Result (Last + 1 .. Last + Img'Length) := Img; - Last := Last + Img'Length + 1; - Result (Last) := ' '; + declare + Img : constant String := +System.Address_Image (Traceback (J)); + begin + Result (Last + 1 .. Last + 2) := "0x"; + Last := Last + 2; + Result (Last + 1 .. Last + Img'Length) := Img; + Last := Last + Img'Length + 1; + Result (Last) := ' '; + end; end loop; Result (Last) := ASCII.LF;
Re: [PATCH v2] ada: PR target/117538 Traceback includes load address if executable is PIE.
I wish to retract this version of the patch and request that the first version (posted on 22/11/2024) be considered instead. > On 24 Nov 2024, at 11:23, Simon Wright wrote: > > If s-trasym.adb (System.Traceback.Symbolic, used as a renaming by > GNAT.Traceback.Symbolic) is given a traceback from a > position-independent executable, it does not include the executable's > load address in the report. This is necessary in order to decode the > traceback report. > > Note, this has already been done for s-trasym__dwarf.adb, which really > does produce a symbolic traceback; s-trasym.adb is the version used in > systems which don't actually support symbolication. > > Bootstrapped and regtested (ada onlyj) on x86_64-apple-darwin. > > * gcc/ada/libgnat/s-trasym.adb: Returns the traceback in the required > form. Note that leading zeros are trimmed from hexadecimal strings. > (Symbolic_Traceback): Import Executable_Load_Address. > (Load_Address): New, from call to Executable_Load_Address. > (One_If_Executable_Is_PI): New, 0 if Load_Address is null, 1 if > not. > (Image_Length): New, found by calling System.Address_Image on > the first address in the traceback. NB, doesn't include "0x". > (Load_Address_Prefix): New, String containing the required value. > (Length_Needed): New, computed using the number of elements > in the traceback, plus the load address if the executable is PIE. > (Result): New String of the required length (which will be an > overestimate). > > 2024-11-24 Simon Wright > > gcc/ada/Changelog: > > PR target/117538 > * libgnat/s-trasym.adb: Returns the traceback, with the program load > address if applicable. > > --- > diff --git a/gcc/ada/libgnat/s-trasym.adb b/gcc/ada/libgnat/s-trasym.adb > index 894fcf37ffd..5351f6fda9b 100644 > --- a/gcc/ada/libgnat/s-trasym.adb > +++ b/gcc/ada/libgnat/s-trasym.adb > @@ -53,19 +53,63 @@ package body System.Traceback.Symbolic is > > else > declare > -Img : String := System.Address_Image (Traceback > (Traceback'First)); > - > -Result : String (1 .. (Img'Length + 3) * Traceback'Length); > -Last : Natural := 0; > +function Executable_Load_Address return System.Address; > +pragma Import > + (C, Executable_Load_Address, > + "__gnat_get_executable_load_address"); > + > +Load_Address : constant System.Address := > + Executable_Load_Address; > +One_If_Executable_Is_PI : constant Natural := > + Boolean'Pos (Load_Address /= Null_Address); > + > +-- How long is an Address_Image? (hex digits only). > +Image_Length : constant Natural := > + System.Address_Image (Traceback (Traceback'First))' > +Length; > + > +Load_Address_Prefix : constant String := > + "Load address: "; > + > +-- For each address to be output, we need the preceding "%x" > +-- and a trailing space, making 3 additional characters. > +-- There are 2 additional LFs. > +Length_Needed : constant Positive := > + (Load_Address_Prefix'Length * > + One_If_Executable_Is_PI) + > + (Image_Length + 3) * > +(Traceback'Length + One_If_Executable_Is_PI) + > + 2; > + > +Result : String (1 .. Length_Needed); > + > +Last : Natural := 0; > > begin > + > +if One_If_Executable_Is_PI /= 0 then > + declare > + Item : constant String := > +Load_Address_Prefix & "0x" & > +System.Address_Image (Load_Address) & > +ASCII.LF; > + begin > + Last := Item'Length; > + Result (1 .. Last) := Item; > + end; > +end if; > + > for J in Traceback'Range loop > - Img := System.Address_Image (Traceback (J)); > - Result (Last + 1 .. Last + 2) := "0x"; > - Last := Last + 2; > - Result (Last + 1 .. Last + Img'Length) := Img; > - Last := Last + Img'Length + > 1; > - Result (Last) := ' '; > + declare > + Img : constant String := > +System.Address_Image (Traceback (J)); > + begin > + Result (Last + 1 .. Last + 2) := "0x"; > + Last := Last + 2; > + Result (Last + 1 .. Last + Img'Length) := Img; > + Last := Last + Img'Length + 1; > + Result (Last) := ' '; > + end; > end loop; > > Result (Last) := ASCII.LF; >
[PATCH v3] ada: PR target/117538 Traceback includes load address if executable is PIE.
If s-trasym.adb (System.Traceback.Symbolic, used as a renaming by GNAT.Traceback.Symbolic) is given a traceback from a position-independent executable, it does not include the executable's load address in the report. This is necessary in order to decode the traceback report. This version of the patch is considerably simplified, as requested. Bootstrapped and regtested (ada onlyj) on x86_64-apple-darwin. * gcc/ada/libgnat/s-trasym.adb: Returns the traceback in the required form, using __gnat_get_executable_load_address to get the address (or null, if not present). 2024-12-17 Simon Wright gcc/ada/Changelog: PR target/117538 * libgnat/s-trasym.adb: Returns the traceback, with the program load address if available. — diff --git a/gcc/ada/libgnat/s-trasym.adb b/gcc/ada/libgnat/s-trasym.adb index 894fcf37ffd..a83d60f 100644 --- a/gcc/ada/libgnat/s-trasym.adb +++ b/gcc/ada/libgnat/s-trasym.adb @@ -69,7 +69,24 @@ package body System.Traceback.Symbolic is end loop; Result (Last) := ASCII.LF; -return Result (1 .. Last); +declare + function Executable_Load_Address return System.Address; + pragma Import + (C, Executable_Load_Address, + "__gnat_get_executable_load_address"); + + Load_Address : constant System.Address := + Executable_Load_Address; +begin + if Load_Address = System.Null_Address then + return Result (1 .. Last); + else + return "Load address: 0x" +& System.Address_Image (Load_Address) + & ASCII.LF + & Result (1 .. Last); + end if; +end; end; end if; end Symbolic_Traceback;
Re: [PATCH v3] ada: PR target/117538 Traceback includes load address if executable is PIE.
On 18 Dec 2024, at 20:33, Eric Botcazou wrote: > >> Bootstrapped and regtested (ada onlyj) on x86_64-apple-darwin. >> >> * gcc/ada/libgnat/s-trasym.adb: Returns the traceback in the required >> form, using __gnat_get_executable_load_address to get the address >> (or null, if not present). >> >> 2024-12-17 Simon Wright >> >> gcc/ada/Changelog: >> >> PR target/117538 >> * libgnat/s-trasym.adb: Returns the traceback, with the program load address >> if available. > > This is OK, thanks. Do you want me to apply it? Yes, thank you. > > -- > Eric Botcazou > >
Ping [PATCH] ada: PR target/117538 Traceback includes load address if executable is PIE.
What I didn’t say before is that, if for example an exception is raised in Ada Language Server and the code uses s-trasym to output a report, what you get on macOS is, for example, ALS.MAIN] in GNATformat Format [ALS.MAIN] raised CONSTRAINT_ERROR : erroneous memory access _ALS.MAIN_ 0x000105847D68 0x000105847DA0 0x0001058EA6AC 0x0001058EB5F4 0x000104B292C5 0x000104B36C5C 0x000104A8C0A0 0x000104A3C29C 0x0001049DB52C 0x000104A03844 0x000104A02DE8 0x000104A03058 0x000104A03C0C 0x000104A03240 0x000104A03854 0x000104A03884 0x000104A02DE8 0x000104A04F8C 0x0001049996E8 0x0001035BD344 0x0001035BD620 0x0001034AFBC8 0x0001034962DC 0x00010354930C 0x00010353D33C 0x00010252074C 0x00010252D25C 0x00010350A53C 0x000105831980 0x00019EE8F2E0 which is useless without the program load address. (A symbolic traceback would be even better, but that would be a different and more difficult project). What should I do to get this change pushed? > On 13 Nov 2024, at 17:15, Simon Wright wrote: > > If s-trasym.adb (System.Traceback.Symbolic, used as a renaming by > GNAT.Traceback.Symbolic) is given a traceback from a > position-independent executable, it does not include the executable's > load address in the report. This is necessary in order to decode the > traceback report. > > Note, this has already been done for s-trasym__dwarf.adb, which really > does produce a symbolic traceback; s-trasym.adb is the version used in > systems which don't actually support symbolication. > > Bootstrapped and regtested (ada onlyj) on x86_64-apple-darwin. > > * gcc/ada/libgnat/s-trasym.adb: Returns the traceback in the required >form. Note that leading zeros are trimmed from hexadecimal strings. > (Symbolic_Traceback): Import Executable_Load_Address. > (Trim_Hex): New internal function to trim leading '0' characters >from a hexadecimal string. > (Load_Address): New, from call to Executable_Load_Address. > (One_If_Executable_Is_PI): New, 0 if Load_Address is null, 1 if >not. > (Max_Image_Length): New, found by calling System.Address_Image on >the first address in the traceback. NB, doesn't include "0x". > (Load_Address_Prefix): New, String containing the required value. > (Max_Length_Needed): New, computed using the number of elements >in the traceback plus the load address, if the executable is PIE. > (Result): New String of the required length (which will be an >overestimate). > > 2024-11-13 Simon Wright > > gcc/ada/Changelog: > > PR target/117538 > * libgnat/s-trasym.adb: Returns the traceback in the required > form. Note that leading zeros are trimmed from hexadecimal strings. > > — > diff --git a/gcc/ada/libgnat/s-trasym.adb b/gcc/ada/libgnat/s-trasym.adb > index 894fcf37ffd..7172214453f 100644 > --- a/gcc/ada/libgnat/s-trasym.adb > +++ b/gcc/ada/libgnat/s-trasym.adb > @@ -53,19 +53,75 @@ package body System.Traceback.Symbolic is > > else > declare > -Img : String := System.Address_Image (Traceback > (Traceback'First)); > - > -Result : String (1 .. (Img'Length + 3) * Traceback'Length); > -Last : Natural := 0; > +function Executable_Load_Address return System.Address; > +pragma Import > + (C, Executable_Load_Address, > + "__gnat_get_executable_load_address"); > + > +function Trim_Hex (S : String) return String; > +function Trim_Hex (S : String) return String is > + Non_0 : Positive; > +begin > + for J in S'Range loop > + if S (J) /= '0' or else J = S'Last then > + Non_0 := J; > + exit; > + end if; > + end loop; > + return S (Non_0 .. S'Last); > +end Trim_Hex; > + > +Load_Address : constant System.Address := > + Executable_Load_Address; > +One_If_Executable_Is_PI : constant Natural := > + Boolean'Pos (Load_Address /= Null_Address); > + > +-- How long is an Address_Image? > +Max_Image_Length : constant Natural := > + System.Address_Image (Traceback (Traceback'First))' > +Length; > + > +Load_Address_Prefix : constant String := > + "Load address: "; > + > +Max_Length_Needed : constant Positive := > + (Load_Address_Prefix'Length * > + One_If_Executable_Is_PI) + &
