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

POSIX: check if strtod() can handle hex #319

Closed
wants to merge 1 commit into from
Closed

Conversation

gperciva
Copy link
Member

No description provided.

@gperciva
Copy link
Member Author

I'm not certain we want this solution; it doesn't help with clang. See #320 for discussion.

@gperciva gperciva closed this Oct 24, 2020
Hex support for strtod() was added to the C standard in C99.
On Solaris, the documented default behaviour is not to support it [1].
(That said, `cc` and `c99` on Solaris 11.4.0.15.0 have no problem with it?)

This commit allows gcc on Solaris to recognize that it needs to use -std=c99.

[1] $ man 3 strtod
    In default mode for strtod(), only decimal, INF/INFINITY, and
    NAN/NAN(n-char-sequence) forms are recognized. In C99/SUSv3 mode,
    hexadecimal strings are also recognized.
    https://docs.oracle.com/cd/E88353_01/html/E37843/strtod-3c.html
@cperciva
Copy link
Member

I don't like this:

  1. I don't want to run binaries we compiled unless absolutely necessary; it breaks cross-compiling.

  2. If some tests fail on Solaris because Solaris is broken, well... that seems like how things should be?

@gperciva
Copy link
Member Author

Hmm. There's two sorts of run-time problems:

  1. runtime broken systems which we can work around at compile-time.
    (e.g., OSX 10.11 with XCode 8: we can avoid a run-time error by compiling with -DPOSIXFAIL_CLOCK_GETTIME; this is currently checked in posix-cflags.sh).
  2. runtime broken systems which we cannot work around.
    (e.g., solaris + clang 6 with any ANSI-C vs. C99 function)

In the case of 1, I think that it would be nice to continue to detect & use the workarounds. I'd be happy to move that code to posix-cflags-runtime.sh for clarity, if that would help?

Alternatively, we could add a tests/POSIX/check-runtime.sh, which would be added to all software which uses libcperciva? If there's an error of type 1, we could output a message to the user saying

your platform is broken. Please recompile after manually setting
CFLAGS="-DPOSIXFAIL_CLOCK_GETTIME"

and if there's an error of type 2, the message could just be

your platform is broken, period.

Asking for somebody to set a compiler flag manually would be a bit weird, but it would satisfy the concern about cross-compiling? Maybe? (Note that this would apply to tarsnap as well. I'm really not a fan of asking users to manually set CFLAGS if we can avoid it.)

The status quo which I want to avoid is this: the libcperciva tests fail on solaris + gcc, we don't change anything in kivaloo, we forget about this, then two years from now we get a confused bug report about kivaloo failing in a weird way, and after a few hours of debugging we figure out that strtod() can't handle hex if it's on solaris + gcc unless we link it with -std=c99.

Also, remember that there's a number of other functions whose behaviour differs between ANSI-C and C99. As far as I know, the only one that we use is strtod, but there could be others. (I've only spent 2 minutes reading the ANSI-C standard, to double-check the difference with strtod.)

@cperciva
Copy link
Member

I'm ok with the existing OS X hack. It's ugly but it's necessary for a widely used platform. (Hopefully at some point we can remove it though? I don't know how common that version is these days.)

For the case of "Solaris and clang 6 a broken combination", I would probably just document in BUILDING.

@gperciva
Copy link
Member Author

What about "solaris and gcc", which is a broken-but-fixable combination? Likewise just document it in BUILDING?

@cperciva
Copy link
Member

I'd be inclined to just document that. I mean, the official way of building is make, which should use a C99-compatible compiler by default; if someone goes out of their way to select a non-default compiler, it's up to them to make sure their selection is C99-compatible.

@gperciva
Copy link
Member Author

gperciva commented Oct 29, 2020

That works for our POSIX Makefiles, but do you feel like taking a wild guess as to which compiler is the first choice of GNU autotools? :(

I see three possibilities to allow tarsnap and scrypt to compile safely with a simple ./configure && make:

  1. change the order of compilers it checks on Solaris, to prioritize cc and c99 over gcc. So far I've failed to do this. I suspect that AC_PROG_CC can't appear in a conditional context, because configure only contained code arising from the first instance of that command.

  2. change the default order of compilers for all platforms. (That is a much more invasive change than seems warrented.)

  3. add this check to autoconf, i.e. Fix solaris linking to a c89 library scrypt#285. (That's a draft PR, but it only needs a little bit of cleaning up.) Note that AC_TRY_RUN has a built-in capability to disable itself if you're doing a cross-compile.

Of course, there's also:

  1. document that we cannot build scrypt and tarsnap safely on Solaris unless the user manually does ./configure CC=cc or ./configure CC=c99 or ./configure LDFLAGS=-std=c99.

@gperciva gperciva closed this Nov 3, 2020
@gperciva gperciva deleted the fix-solaris-gcc branch November 15, 2020 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants