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

Difficulties building 4.04 on Solaris with the SUNWSPro C compiler #7407

Closed
vicuna opened this issue Nov 11, 2016 · 5 comments

Comments

Projects
None yet
1 participant
@vicuna
Copy link

commented Nov 11, 2016

Original bug ID: 7407
Reporter: shayne_fletcher
Status: resolved (set by @xavierleroy on 2017-02-23T15:46:22Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.04.0
Fixed in version: 4.05.0 +dev/beta1/beta2/beta3/rc1
Category: ~DO NOT USE (was: OCaml general)

Bug description

cc -V gives, "cc: Sun C 5.12 SunOS_sparc Patch 148917-07 2013/10/18"
which according to
http://www.oracle.com/technetwork/server-storage/solarisstudio/training/index-jsp-141991.html
seems to indicate, Oracle Solaris Studio 12.3, C Compiler 5.12.

(1) C99 required:

  • Resolution:
    • Add -xc99=all to the compiler flags

(2) Problems with the newly added CAML_SYS_EXIT macro:

  • Defined in 'ocaml/byterun/caml/misc.h'

  • The problem is:

    #define CAML_SYS_EXIT(retcode) \
      CAML_SYS_PRIM_1(CAML_CPLUGINS_EXIT,exit,retcode)
    

    ultimately reduces to something like,

    (caml_cplugins_prim == NULL) ? prim(arg1) :
       caml_cplugins_prim(code,(intnat) (arg1),0,0)
    

    Where exit is concerned, the 'then' part of the conditional evaluates to void, the 'else' part of the conditional evaluates to long. This is a hard error for this compiler.

  • Resolution:

    • Modify 'ocaml/byterun/sys.c' and 'ocaml/byterun/printexc.c', replacing occurences of CAML_SYS_EXIT(2) with
    #if defined(__SUNPRO_C)
      exit (2);
    #else
      CAML_SYS_EXIT(2);
    #endif /*defined __SUNPRO_C*/
    

(3) Unresolved symbols linking 'objinfo_helper'

  • The problem here is due to newly added code to 'caml/alloc.h':

    static inline value caml_alloc_boxed (value arg) {
      value result = caml_alloc_small (1, 0);
       ...
    }
    

    Despite not being called from 'objinfo_helper.c', this morally induces a link dependency of objinfo_helperon 'libcamlrun.a' which in turn obligates a dependency on prims.o.

  • Resolution:

    • Add -Xlinker -znodefs to the compiler flags to permit unresolved dependencies (note : not a recommended practice, maybe better to not have these static inline constructions in the header at all).
@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 11, 2016

Comment author: @lefessan

For CAML_SYS_EXIT, I would prefer to define a new function int_exit(retcode) that would call exit, but have a type compatible with the macro :

#include <stdlib.h>
int int_exit(int retcode)
{
  exit(retcode);
  return retcode;
}

and then use int_exit instead of exit in CAML_SYS_EXIT ?

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 11, 2016

Comment author: @gasche

As far as I understand from looking at the README and the ./configure script, building OCaml on Solaris is usually done (and tested) using gcc. Do you have a particular reason to use the system compiler instead?

In any case it would be interesting to resolve any reasonable issue found by a more stringent compiler.

In the case of objinfo_helper.c, I don't see why we actually need caml/alloc.h -- and it builds fine if I remove the include. Could someone that knows better comment on whether removing the #include would be safe?

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 11, 2016

Comment author: @xavierleroy

I agree "a ? b : c" should never be used when "b" and "c" have incompatible types: even though GCC and probably Clang tolerate the case where one of "b" and "c" has type "void" and the other doesn't, this is not conformant ISO C code and will get rejected (rightly so) by other compilers.

I'd suggest to special-case CAML_SYS_EXIT and just add a cast to (void) in the "else" branch.

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 11, 2016

Comment author: shayne_fletcher

@gasche:

Do you have a particular reason to use the system compiler instead?

Organizational policy I'm afraid; regardless of my feelings on the matter this kind of constraint usually can't be worked around here. That said, in this particular instance I might be able to get away with switching to gcc and we're trying that now.

@vicuna

This comment has been minimized.

Copy link
Author

commented Feb 23, 2017

Comment author: @xavierleroy

Item 2 is a type error indeed, even though gcc and clang don't report it, and was fixed in commit 6d80f59.

Item 3 is subtle but it turns out that objinfo_helper doesn't need to include headers from the runtime system, so the fix is trivial (commit cfb0715).

Item 1 is the responsibility of the user.

@vicuna vicuna closed this Feb 23, 2017

@vicuna vicuna added the bug label Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.