Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

musl compatibility #130

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

musl compatibility #130

wants to merge 3 commits into from

Conversation

v01dXYZ
Copy link

@v01dXYZ v01dXYZ commented Jun 4, 2024

musl has some discrepancies with glibc that causes some issues at compilation/run:

  • headers that don't provide the same set of symbols (time.h)
  • usage of routines that depends on behaviour left to the implementation by the spec (random). At the end, the program generates very different outputs.

Feel free to suggest the right tags for the commits ([MultiSource], [CLAMR], ...).

v01dxyz added 2 commits June 4, 2024 16:57
With musl, mesh.cpp compilation fails as struct timeval is not defined
although <time.h> is included before "timer.h" which uses this data
type.

The reason is with musl contrary to glibc, <time.h> does not include
<sys/time.h> (which provides gettimeofday, struct timeval).

Fixes llvm/llvm-project#94320
musl implements random() differently than glibc.

The output of the program is thus dependent on this implementation
detail and could differ between platforms. To remediate that, we
replace it by a very simple PRNG (ANSI C LCG).

Warning: [^1] indicates this PRNG have quite a poor LSB randomness.

[^1]: Entacher, Karl. (1999). A Collection of Selected Pseudorandom
Number Generators With Linear Structures.

Fixes llvm/llvm-project#94048
@v01dXYZ v01dXYZ force-pushed the musl-compatibility branch 2 times, most recently from d67f3e4 to d5d46bb Compare June 5, 2024 18:35
Compatible with musl in particular. <unistd.h> is already included in
"config.h".

It supposes the system does not "lie" by providing only a partial POSIX
support.

FreeBSD/NetBSD/OpenBSD/Apple (XNU) provides the macro too but are left
untouched as I don't have the means to test it.

Duplicate of an abandoned patch from 2015: https://reviews.llvm.org/D13938
@v01dXYZ v01dXYZ force-pushed the musl-compatibility branch from d5d46bb to 551187d Compare June 5, 2024 18:39
@v01dXYZ
Copy link
Author

v01dXYZ commented Jun 6, 2024

I found a discrepancy related to the way buffered IO is implemented by libc.

#include <stdio.h>
#include <unistd.h>

#define WRITE(fd, str) \
  do { \
  char s[] = str;                               \
  write(fd, s, sizeof(s)); \
  } while (0)

int main(int argc, char **argv)
{
  if(argc < 2) {
    fprintf(stdout, "stdout\n");
    fprintf(stderr, "stderr\n");
  } else {
    WRITE(STDOUT_FILENO, "stdout\n");
    WRITE(STDERR_FILENO, "stderr\n");
  }

  return 0;
}

When you use the following command line, with glibc we have

$ ./buff_io_discrepancy 2>&1 | cat # no argument  -> buffered IO
stderr
stdout
$ ./buff_io_discrepancy direct 2>&1 | cat # argument -> direct IO
stdout
stderr

musl always outputs the same (in this very case at least, I don't know if it holds always true).

This problem appears too with a >out 2>&1.
I don't know how to solves this problem, except by considering using separate reference files for stdout/stderr.

@androm3da androm3da self-requested a review February 26, 2025 17:56
@androm3da
Copy link
Member

These changes LGTM but is there someone who should approve them?

Copy link
Contributor

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

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

  1. This PR has been dormant for a long time, so I want to be careful handling it.
  2. For some imported test suites our policy is not to modify the source of the test but to disable it, I don't know how that policy would change with an application. Perhaps we would ideally fix it upstream, if the application is imported from elsewhere.

This is something we should document if we haven't already. @lenary has dealt with this for a test suite before, did we document that decision anywhere? (I would expect it to be in https://llvm.org/docs/TestSuiteGuide.html)

At least we can make that policy clear, even if this PR does not move forward.

Edit: for example obsequi does have a non-CMake change 24139ac, but the majority are CMake changes.

@DavidSpickett
Copy link
Contributor

Just to be clear: being compatible with musl seems like a fine goal if there is a need for it.

@lenary
Copy link
Member

lenary commented Feb 26, 2025

Yeah, documenting "what can be changed" vs "what cannot" has not really been done, and I think some changes have crept in to what is effectively vendored code.

I can maybe write a quick note for the docs this afternoon, if that would help.

@androm3da
Copy link
Member

Perhaps we would ideally fix it upstream, if the application is imported from elsewhere.

That makes the most sense.

@lenary
Copy link
Member

lenary commented Feb 26, 2025

I opened llvm/llvm-project#128937 which I hope adds some clarity about our intentions originally.

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.

4 participants