Skip to content

Conversation

durban
Copy link
Contributor

@durban durban commented Oct 16, 2025

WIP

#include <stddef.h>

// we'll need POSIX for `sigaction`:
#define _POSIX_C_SOURCE 1

This comment was marked as resolved.

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It fails (on 0.5.9) with the same "incomplete type" and "implicit declaration" errors.

Copy link
Contributor

@s5bug s5bug Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, conversation in #scala-native I realized that. Technically this should be defined before the inclusion of any other standard headers (so before stddef.h) but I doubt it matters.

It would be an issue if i.e. sigaction depended on something in stddef.h that stddef.h only defines with _POSIX_C_SOURCE active.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it should be defined before the inclusion of practically anything anywhere (but we can't do that, it's not up to us). So it's not bulletproof, but I don't have a better idea.

Copy link
Contributor

@s5bug s5bug Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This being a .c file, the only earlier you can get is if someone passes -include to the compiler. (This is related to why you only define _POSIX_C_SOURCE in source files and not in header files: you can't be sure that your header will be included first.) TLDR I don't see why it's not bulletproof 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this should be defined before the inclusion of any other standard headers

That is standards speak for "You will regret it if you do it any other way" ;-)

@durban durban linked an issue Oct 16, 2025 that may be closed by this pull request
@durban
Copy link
Contributor Author

durban commented Oct 16, 2025

@ekrich
Copy link

ekrich commented Oct 16, 2025

You should be able to just do the following:

https://github.com/scala-native/scala-native/blob/3f89e6f4ad8b657d5475dbaec65dca437168ae1e/javalib/src/main/resources/scala-native/scala-native.properties

Or conversely like this:

https://github.com/scala-native/scala-native/pull/4523/files

@LeeTibbert
Copy link
Contributor

LeeTibbert commented Oct 16, 2025

Thank you for this PR. You know the CE build environment and there are plenty of log files.
I would have gotten lost.

The code in https://github.com/scala-native/scala-native/pull/4523/files Eric linked above
should give you exactly what CE had with SN 0.5.8 and before.

By my understanding "-std=c11" gives one standard C ISO C.

Defining "_POSIX_SOURCE" adds various elements marked by the Open Group (a.k.a POSIX)
as extensions to ISO C.

Defining "_GNU_SOURCE" adds more magic to the soup.

Starting from a clean sheet of paper, "_POSIX_SOURCE" is preferable to most/many/well_a_few people.
It is well documented, the documentation is readily available and the extensions are, for the most part,
reasonable. It is also more likely to port to "unix-like" but not strictly unix systems. That is, other
POSIX compliant or near compliant systems. Of course, here we are talking about signals, so
all bets are off to begin with.

In this case, we are dealing with existing code which used, effectively, _GNU_SOURCE. I believe
there are some slight differences in how some things are defined. This is Fear, Uncertainty, & Doubt
and a wish to get to dinner before midnight. If I were betting my paycheck or tranquility over the
next few weeks, I hold my nose, honor existing practice (stare decisis) and go with _GNU_SOURCE.
Of course, that is announcing to the world that I have few scruples when maintaining code.

(I compared the file using -std=11, _POSIX_SOURCE, and _GNU_SOURCE using the clang/gcc -E
option to run just the pre-processor. I did not look at the slight differences between the latter two.)

Let me know if & how I can help.

#ifdef CATS_EFFECT_SIGNAL_HELPER

// we'll need POSIX for `sigaction`:
#define _POSIX_C_SOURCE 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The macro with which I am familiar is "_POSIX_SOURCE". Is "_POSIX_C_SOURCE" here an alias for the former? If not, that may explain some lack of effect.

I just made a change in SN where I added guard code
so that the definition is idempotent.
Roughly (not compiled):

#if !defined(_GNU_SOURCE)
#define _GNU_SOURCE 1
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we'll need the #if here too.

Apprently _POSIX_SOURCE is obsolete, and equivalent to _POSIX_C_SOURCE 1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LeeTibbert you shouldn't need to guard it: https://godbolt.org/z/3WssGsE1r and my argument is that you shouldn't want to guard it. Getting an error when it's defined twice is good because it tells you someone is using it wrong (i.e. the only place it should be defined is at the very beginning of a source file ["translation unit"])

Copy link
Contributor

@LeeTibbert LeeTibbert Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good discussion & good points.

People, especially in large projects with build files, specify compiler options
on the command line quite frequently. The Gell-Mann concept of "If it can be done
it will be done" applies here, I think.

Guarding it or not is, I believe, one of those points where it depends upon what one
wants/intends/can_live_with. Kind of like the reason that people bet on whether
a coin flip will be heads or tails. Personal preference & keeps things interesting.

Over time I have become much more defensive when writing code which gets compiled
anywhere off my machine. I have given up the idea that I control anything, even on that/those
machine(s).

I have become both reluctant to break user code and to give up a week of my time to an
interruption when I do. The determining question here is "Can a person compiling this
code possibly supply a value which conflicts?"

That is, if the code requires a methods introduced in an Open Group version 6, what happens if they
define a version, say 1, on the command line? In that case you are correct that the conflict should
be reported.

In this case, by my possibly mistaken reading, I believe that there is no current conflicting value,
even an empty replacement or version 1. Providing the guard will prevent a complaint in the
"innocent" situation of a user specifying a value. If at all possible and if it maintains correctness
of the code, user specified values should be respected.

I believe this is more important when the code specifies a value such as 1 and a user
wants to compile with a higher Open Group version, say 8, in order to gain a V8 definition
in another file. Why annoy the user by breaking then?


For me, I am much more concerned with the case where a user can specify a conflicting definition. Say they compile their entire project with "-std=gnu17". One would then have "_GNU_SOURCE" (and/or possibly a newer macro" defined at the same time as this file defining "_POSIX_C_SOURCE".

I believe, on hasty reading, that the <signal.h> file included by signal_helper.c will messily
detect and report this situation by having multiple definitions for some entry_points/functions.


I'm off to a another lesson in humility by chasing a Scala 2 `NullPointerException` in `ConcurrentHashMap`. I fear Cats Effects is probably affected by this bug. This discussion is much more interesting that that lesson ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@s5bug I this case I mostly agree with @LeeTibbert. Specifically, the scenario I'm guarding against is this: the user specifies -D_POSIX_C_SOURCE=2 (or later) when compiling; if we don't have the #ifndef, then that causes a warning. However, we're actually fine with 2 (or later). As far as I understand, for that file, all we need is that we have some _POSIX_C_SOURCE defined.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The determining question here is "Can a person compiling this code possibly supply a value which conflicts?"

My view is it is more useful to tell a user "hey you shouldn't be doing this" than let it go through. If SN wants to support people doing strange things with compiler flags, that's fine by me, but for cases outside of this one that require a _POSIX_C_SOURCE other than 1, at the very least rather than

#ifndef _POSIX_C_SOURCE
#define _POSIX_C_SOURCE 123456L
#endif

could we recommend

#if defined(_POSIX_C_SOURCE)
#if _POSIX_C_SOURCE < 123456L
#error ...
#endif
#else
#define _POSIX_C_SOURCE 123456L
#endif

somewhere?

Say they compile their entire project with "-std=gnu17"

My godbolt link shows this is not an issue: a warning is only thrown when a double-define occurs (GCC's private features.h header does guard, even when -std=gnu... is set).

Specifically, the scenario I'm guarding against is this: the user specifies -D_POSIX_C_SOURCE=2 (or later) when compiling

I want to reiterate that the user should not be doing this. _POSIX_C_SOURCE should be specified on a source-file level, not broadly across a whole compilation. If SN/CE wants to support this whole-compilation case, that's fine, I just want that decision to be made with full knowledge.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to reiterate that the user should not be doing this. _POSIX_C_SOURCE should be specified on a source-file level, not broadly across a whole compilation.

I think it immediately says the developer there is POSIX inside. Something like the internal Scala Native posixlib it makes sense for that compilation unit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@s5bug

I want to reiterate that the user should not be doing this. _POSIX_C_SOURCE should be specified on a source-file level, not broadly across a whole compilation.

I'm not convinced this is the case; man feature_test_macros says this to me:

NOTE: In order to be effective, a feature test macro must be defined before including any header files. This can be done either in the compilation command (cc -DMACRO=value) or by defining the macro within the source code before including any headers.

It then goes on detailing various macros, including _POSIX_C_SOURCE. So it seems fine to me.

Of course it's possible that there exists some other documentation or "best practices" which says otherwise. If so, I'd be happy to read that...

@LeeTibbert
Copy link
Contributor

Do you think the errors in the log file have to do with 0.5.9 or with _Mumble_SOURCE?

If the former, I will give things a closer look. I took a fast run thru the logs and could not
make sense of them, limitation at my end.

@durban
Copy link
Contributor Author

durban commented Oct 16, 2025

Do you think the errors in the log file have to do with 0.5.9 or with _Mumble_SOURCE?

Which errors, in which log file?

@LeeTibbert
Copy link
Contributor

LeeTibbert commented Oct 16, 2025

Stacktraces broke at least on ARM Linux (scala-native; 2.13 and 3); 2 examples:

* https://github.com/typelevel/cats-effect/actions/runs/18574362636/job/52958750712#step:22:1975

* https://github.com/typelevel/cats-effect/actions/runs/18574362636/job/52956834671#step:22:3670

Yes, there were some stack trace changes in 0.5.9. I believe that they had to do with "lazy loading".

I do not want to be spreading falsehoods do to my not being up to date, but I thought stack traces were broken
in ARM Linux for a very long time. Perhaps different breakage.

I looked at one of the cited log file and did not see a stack trace where the log said there was a failure.
I do not read CE, sorry, so what was the failure? Do you have any idea?

You might try again with _GNU_SOURCE to see if anything improves. I suspect you are seeing
an unrelated & real problem.

#ifdef CATS_EFFECT_SIGNAL_HELPER

// we'll need POSIX for `sigaction`:
#ifndef _POSIX_C_SOURCE
Copy link
Contributor

@LeeTibbert LeeTibbert Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK _POSIX_C_SOURCE is newer than _POSIX_SOURCE. I believe that it
is supposed to have a value defined in the Open Group Standard. Something
much higher than 1.

Later: see version suggestion below and related discussion in CE Issue #4509

All the .h file code I was looking for used _POSIX_SOURCE, and not the former.
I guess the compiler will tell you/us if it can not find the symbols revealed by the
correct macro.

@durban
Copy link
Contributor Author

durban commented Oct 16, 2025

Yeah, I'm looking at scala-native/scala-native#4512, it seems possibly related. Some of our unittests fail at least on ARM Linux. All the failing tests depend on getting stacktraces. These same tests passed on ARM Linux with SN 0.5.8. I do not know yet the exact nature of the failures.

@LeeTibbert
Copy link
Contributor

A suggestion following up on a suggestion by @ekrich in CE Issue #4509.

#define _POSIX_C_SOURCE 202405L // Open Group 2024 Issue 8

@durban
Copy link
Contributor Author

durban commented Oct 17, 2025

The stacktrace thing is this: scala-native/scala-native#4526.

@LeeTibbert
Copy link
Contributor

The stacktrace thing is this: scala-native/scala-native#4526.

Thank you for the SN Issue. Wojciech will see the discussion there.

Do you happen to know what happens if you toggle .generateFunctionSourcePositions(false)
back to true? Still SEGFAULT? Not trying to suck you down into a days long rat hole.

@durban
Copy link
Contributor Author

durban commented Oct 17, 2025

@LeeTibbert We'll see: e40493b.

@durban durban marked this pull request as ready for review October 17, 2025 12:24
@durban durban added this to the v3.7.0 milestone Oct 17, 2025
@LeeTibbert
Copy link
Contributor

e40493b seemed to do the trick & move in a desired direction to boot.

I think I found the SN NullPointerException which was killing skunk (I think that is another CE project).

As always, yell if SN is misbehaving for CE. I may not be able to fix it, but I can listen & investigate.

@LeeTibbert
Copy link
Contributor

Do you think the errors in the log file have to do with 0.5.9 or with _Mumble_SOURCE?

Which errors, in which log file?

Overcome by events, logs for current commit are all green.

@LeeTibbert
Copy link
Contributor

s5bug:

I like block you suggested about incompatible prior POSIX/Open_Group versions being specified.
Thank you, I'll have to remember it.

#if defined(_POSIX_C_SOURCE)
#if _POSIX_C_SOURCE < 123456L
#error ...
#endif
#else
#define _POSIX_C_SOURCE 123456L
#endif

People can and do specify POSIX versions outside the compilation unit: say on the command line by a build
file. I do not want to take on the responsibility for telling them that they are wrong for using a time honored practice.

IMO, the basic problem is a project shipping files which have to be compiled outside of a known environment, say
CI. There are just too many variants and time honored practices for a small devo team to reliably cover.
For the while, it is probably easier to achieve than using Dockerfiles or shipping os specific archive (.a) or
libraries (.so). I can still grumble about the development cost of "glue" files and use them if I must.

Hope all your coding is going well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3.7.0-RC1 broken with SN 0.5.9

4 participants