Обсуждение: Solaris compiler status
I've been trying out the Solaris compiler (Oracle Developer Studio 12.6, on Solaris), mainly to test out some C11 support details. Observations: - That compiler has a certain known idiosyncratic way of dealing with some inline functions, which lead to commits such as 3e9ca526012 and discussions such as [0] (ironically the first thread you find when searching for "solaris compiler dead"). So people did use this at some point. But this has been broken again since commit 4908c5872059. So apparently, no one has used this compiler successfully since at least PG17. (This issue is easy to fix with some #ifndef FRONTEND, as before.) (The 32-bit build appears to have been uncompilable since commit c8b2ef05f48 in PG16. This is fixed by 8-byte-datums-everywhere in master.) [0]: https://www.postgresql.org/message-id/20180928150700.h4ikpnh2fibib2nn@alap3.anarazel.de - At least in my installation (which is supposed to have all the latest versions), the compiler runs out memory when trying to compile gram.c, even without optimizations on. (You can get past this by compiling that one file with gcc.) - This compiler is not supported in Meson at all. (meson setup run fails very early.) I also don't see any recent relevant attempts in the Meson issue tracker. - No buildfarm coverage since who knows when (possibly since [0]?). - Hundreds of compiler warnings (They are not necessarily wrong, but it shows that no one has taken care to tune the warnings or the code in any way recently.) - Shaky C11 support: * I was trying to test the ICE_P() macro discovered by David Rowley [1] -- I doesn't work. (It returns zero even for constants, so at least it fails in the right direction.) [1]: https://www.postgresql.org/message-id/CAApHDvoOnd4PBNV0qyJVLmbWvWUjztzcMH5xY2AGp5Vov6XU3Q%40mail.gmail.com * Strangely, the compiler claims to support __builtin_constant_p(), but our configure test for that fails for other mysterious weird reasons. * This compiler rejects _Alignas() inside structure definitions, even though it appears to support pretty much everything else, so it seems kind of incomplete. * It gives "typedef redeclared" warnings for my patch set [2], but I suppose there would be a way to turn those off via warning toggling options. [2]: https://www.postgresql.org/message-id/flat/10d32190-f31b-40a5-b177-11db55597355@eisentraut.org I mean, at least the AIX enthusiasts popped up right away when we broke their platform. Here, it seems, truly no one cares. So, can we declare that we don't support this compiler anymore? (gcc is readily available and seems to work alright, so I'm not saying the OS is no longer viable.)
On 2025-09-03 We 1:47 AM, Peter Eisentraut wrote: > I've been trying out the Solaris compiler (Oracle Developer Studio > 12.6, on Solaris), mainly to test out some C11 support details. > Observations: > [ various issues] > > I mean, at least the AIX enthusiasts popped up right away when we > broke their platform. Here, it seems, truly no one cares. > > So, can we declare that we don't support this compiler anymore? > > (gcc is readily available and seems to work alright, so I'm not saying > the OS is no longer viable.) > > > Seems reasonable. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
> On 3 Sep 2025, at 07:47, Peter Eisentraut <peter@eisentraut.org> wrote: > - Hundreds of compiler warnings (They are not necessarily wrong, but it shows that no one has taken care to tune the warningsor the code in any way recently.) Should these be posted somewhere before they are lost in case anyone is keen on digging through the list for any relevant fixes? > So, can we declare that we don't support this compiler anymore? > > (gcc is readily available and seems to work alright, so I'm not saying the OS is no longer viable.) +1 based on the findings above. -- Daniel Gustafsson
Daniel Gustafsson <daniel@yesql.se> writes:
> On 3 Sep 2025, at 07:47, Peter Eisentraut <peter@eisentraut.org> wrote:
>> So, can we declare that we don't support this compiler anymore?
>> (gcc is readily available and seems to work alright, so I'm not saying the OS is no longer viable.)
> +1 based on the findings above.
+1, given the lack of complaints it certainly appears that
nobody cares.
regards, tom lane
On 03.09.25 13:42, Daniel Gustafsson wrote:
>> On 3 Sep 2025, at 07:47, Peter Eisentraut <peter@eisentraut.org> wrote:
>
>> - Hundreds of compiler warnings (They are not necessarily wrong, but it shows that no one has taken care to tune the
warningsor the code in any way recently.)
>
> Should these be posted somewhere before they are lost in case anyone is keen on
> digging through the list for any relevant fixes?
That would require me to figure out how to export files from this VM.
;-) But I did figure out how to copy and paste, so here is at least a
summary:
1 (E_ARG_INCOMPATIBLE_WITH_ARG_L)
16 (E_ASSIGNMENT_TYPE_MISMATCH)
1 (E_CONST_OBJ_SHOULD_HAVE_INITIZR)
24 (E_CONST_PROMOTED_LONG_LONG)
1 (E_EMPTY_DECLARATION)
1899 (E_END_OF_LOOP_CODE_NOT_REACHED)
22 (E_FUNC_NO_RET_RET)
2 (E_INTEGER_OVERFLOW_DETECTED)
23 (E_L_OPERAND_DOT_NOT_LVALUE_NOW)
2 (E_OPERANDS_INCOMPATIBLE_TYPES)
1 (E_REF_STATIC_EXTERN_INLINE)
714 (E_STATEMENT_NOT_REACHED)
435 (E_STRUCT_DERIVED_FROM_FLEX_MBR)
Most of these as you can see are from lack of understanding about flow
control annotations or conventions.
But there are a couple of interesting ones:
1 (E_EMPTY_DECLARATION)
This is actually a real find, albeit harmless:
```
diff --git a/src/backend/replication/logical/slotsync.c
b/src/backend/replication/logical/slotsync.c
index 9d0072a49ed..8c061d55bdb 100644
--- a/src/backend/replication/logical/slotsync.c
+++ b/src/backend/replication/logical/slotsync.c
@@ -1337,7 +1337,7 @@ reset_syncing_flag()
SpinLockRelease(&SlotSyncCtx->mutex);
syncing_slots = false;
-};
+}
/*
* The main loop of our worker process.
```
2 (E_INTEGER_OVERFLOW_DETECTED)
This complains about lwlock.c
StaticAssertDecl((MAX_BACKENDS & LW_FLAG_MASK) == 0,
"MAX_BACKENDS and LW_FLAG_MASK overlap");
StaticAssertDecl((LW_VAL_EXCLUSIVE & LW_FLAG_MASK) == 0,
"LW_VAL_EXCLUSIVE and LW_FLAG_MASK overlap");
Here, MAX_BACKENDS is of type unsigned int and LW_FLAG_MASK is of type
signed int. (LW_VAL_EXCLUSIVE is computed from MAX_BACKENDS and is also
unsigned int.) The & operation promotes both to unsigned int, and so it
thinks you might get an overflow of the LW_FLAG_MASK value?
I can make the warning go away with this:
diff --git a/src/backend/storage/lmgr/lwlock.c
b/src/backend/storage/lmgr/lwlock.c
index a4aecd1fbc3..667ed4cf699 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -95,7 +95,7 @@
#define LW_FLAG_RELEASE_OK ((uint32) 1 << 30)
#define LW_FLAG_LOCKED ((uint32) 1 << 29)
#define LW_FLAG_BITS 3
-#define LW_FLAG_MASK
(((1<<LW_FLAG_BITS)-1)<<(32-LW_FLAG_BITS))
+#define LW_FLAG_MASK
(((1U<<LW_FLAG_BITS)-1)<<(32-LW_FLAG_BITS))
/* assumes MAX_BACKENDS is a (power of 2) - 1, checked below */
#define LW_VAL_EXCLUSIVE (MAX_BACKENDS + 1)
I can't reproduce this with gcc by removing -fwrapv and dialing up the
-Wstrict-overflow= levels. And I also don't want to think about what
this double-shift expression even means. :-/
But intuitively, it seems better if a "mask" value has an unsigned type.
I don't know to what extent PostgreSQL can even survive in a non-fwrapv
environment. That's a larger philosophical question.
On Thu, Sep 4, 2025 at 2:42 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Daniel Gustafsson <daniel@yesql.se> writes: > > On 3 Sep 2025, at 07:47, Peter Eisentraut <peter@eisentraut.org> wrote: > >> So, can we declare that we don't support this compiler anymore? > >> (gcc is readily available and seems to work alright, so I'm not saying the OS is no longer viable.) > > > +1 based on the findings above. > > +1, given the lack of complaints it certainly appears that > nobody cares. +1. Last release 2017, and development had ended by 2020 according to a well known Sun/Oracle engineer: https://forums.oracle.com/ords/apexds/post/is-developer-studio-abandoned-and-discontinued-by-oracle-3285
On 2025-09-03 07:47:37 +0200, Peter Eisentraut wrote: > So, can we declare that we don't support this compiler anymore? +1
On 03.09.25 07:47, Peter Eisentraut wrote: > So, can we declare that we don't support this compiler anymore? Ok, looks like we're calling it. Here is a patch set to remove what turns out to be a significant amount of code and documentation to support this compiler. I also added in one tiny cosmetic bug fix that this compiler found, as its parting gift. In passing, I also removed some version checks for no longer supported old GCC versions, since they touched some of the same #ifdef etc. lines. In src/include/storage/s_lock.h, there was a comment to look in src/backend/port/tas/sunstudio_sparc.s for some explanation. But the latter is to be removed, so I copied the comment over and tried to fit it in. But I don't understand it much, and it makes some claims about gcc support for sparc, so it could be that some more code for newer sparc variants is dead, or maybe gcc has caught up in the meantime. Also, that cvs.opensolaris.org link no longer works. But I think this all could be a separate project to sort out.
Вложения
Hi, Generally looks sane. A few minor remarks: 0003 leaves building of tas.o in place, but afaict there's nothing using the infrastructure anymore - and it seems rather unlikely that we'll use it again. 0003 changes solaris support from "well-supported" to "supported" - that imo seems independent of the rest. Greetings, Andres Freund
Peter Eisentraut <peter@eisentraut.org> writes:
> Here is a patch set to remove what turns out to be a significant amount
> of code and documentation to support this compiler.
Looks generally sane by eyeball -- I did not grep for anything that
you missed. I concur with Andres that tas.o is probably never going
to be used again, and if it is we can put back support for it.
> In src/include/storage/s_lock.h, there was a comment to look in
> src/backend/port/tas/sunstudio_sparc.s for some explanation. But the
> latter is to be removed, so I copied the comment over and tried to fit
> it in. But I don't understand it much, and it makes some claims about
> gcc support for sparc, so it could be that some more code for newer
> sparc variants is dead, or maybe gcc has caught up in the meantime.
I would reduce the comment in s_lock.h to something along the lines of
/*
* "cas" would be better than "ldstub", but it is only present on
* sparcv8plus and later, while some platforms still support sparcv7
* or sparcv8. Also, "cas" requires that the system be running in
* TSO mode.
*/
There's a comment a few lines above explaining TSO, so we don't need
more than that here.
Possibly at some point somebody will be motivated to improve the
s_lock.h code beyond its current state, but I agree that that's
material for future work.
regards, tom lane
On 04.09.25 18:29, Tom Lane wrote: > Peter Eisentraut <peter@eisentraut.org> writes: >> Here is a patch set to remove what turns out to be a significant amount >> of code and documentation to support this compiler. > > Looks generally sane by eyeball -- I did not grep for anything that > you missed. I concur with Andres that tas.o is probably never going > to be used again, and if it is we can put back support for it. > >> In src/include/storage/s_lock.h, there was a comment to look in >> src/backend/port/tas/sunstudio_sparc.s for some explanation. But the >> latter is to be removed, so I copied the comment over and tried to fit >> it in. But I don't understand it much, and it makes some claims about >> gcc support for sparc, so it could be that some more code for newer >> sparc variants is dead, or maybe gcc has caught up in the meantime. > > I would reduce the comment in s_lock.h to something along the lines of > > /* > * "cas" would be better than "ldstub", but it is only present on > * sparcv8plus and later, while some platforms still support sparcv7 > * or sparcv8. Also, "cas" requires that the system be running in > * TSO mode. > */ > > There's a comment a few lines above explaining TSO, so we don't need > more than that here. > > Possibly at some point somebody will be motivated to improve the > s_lock.h code beyond its current state, but I agree that that's > material for future work. This has been committed, with these adjustments.