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

Cross-compilation issues with compile_et, asn1compile #1162

Open
riastradh opened this issue Jun 19, 2023 · 20 comments
Open

Cross-compilation issues with compile_et, asn1compile #1162

riastradh opened this issue Jun 19, 2023 · 20 comments

Comments

@riastradh
Copy link

riastradh commented Jun 19, 2023

Heimdal provides two ways to compile error tables and ASN.1 schemas:

  • Use $(CC) to compile compile_et and asn1compile and run them out of the build tree.
    • This doesn't work when $(CC) is a cross-compiler.
    • An alternative convention is to use $(CC_FOR_BUILD), e.g. via AX_PROG_CC_FOR_BUILD, to invoke a native compiler even if $(CC) is a cross-compiler.
  • Run compile_et and asn1compile out of a directory specified with --with-cross-tools-dir.
    • Heimdal doesn't install compile_et or asn1compile anywhere -- they are listed in noinst_PROGRAMS -- so it's not obvious how one is supposed to create the directory to pass to --with-cross-tools-dir.
    • Simply installing an existing Heimdal package -- which contains the content of the installation prefix, but not of the source tree -- is not enough to do this because it won't contain the executables.

It would be nice if either:

  1. Heimdal's Makefile.am used $(CC_FOR_BUILD).
  2. Heimdal's Makefile.am installed asn1compile and compile_et somewhere (not necessarily in $PREFIX/bin but maybe in $PREFIX/libexec/heimdal7.8.0 or something).

NetBSD works around this by just not using Heimdal's build system, and instead having a shadow set of reachover makefiles to do this. But for cross-compilation in, e.g., pkgsrc, it would be much better to be able to either just pass CC_FOR_BUILD into Heimdal's build system, or add a self-dependency and use the native build of Heimdal to do a cross build.

@nicowilliams
Copy link
Contributor

nicowilliams commented Jun 20, 2023

Yes, we're aware that cross-compilation is currently broken because of in-tree compiler bootstrapping issues. We have four "compilers" in-tree (that I know of or remember at this time):

  • include/bits.c (which builds a program called bits that emits some typedefs and defines)
  • lib/asn1's asn1_compile (which compiles ASN.1 modules)
  • lib/sl's slc (which compiles command-line descriptions)
  • lib/com_err's compile_et (which compiles com_err error tables)

Of these four bits and asn1_compiled definitely need knowledge of the target ABI in order to emit correct output for the target ABI.

Heimdal provides two ways to compile error tables and ASN.1 schemas:

* Use $(CC) to compile compile_et and asn1compile and run them out of the build tree.
  
  * This doesn't work when $(CC) is a cross-compiler.
  * An alternative convention is to use $(CC_FOR_BUILD), e.g. via [AX_PROG_CC_FOR_BUILD](https://www.gnu.org/software/autoconf-archive/ax_prog_cc_for_build.html), to invoke a native compiler even if $(CC) is a cross-compiler.

Indeed, running in-tree compilers from the build tree either can't work when cross-compiling (if we use $(CC)) or may produce broken results because the local compiler doesn't have knowledge of the target ABI (if we use $(CC_FOR_BUILD)).

* Run compile_et and asn1compile out of a directory specified with --with-cross-tools-dir.
  
  * Heimdal doesn't install compile_et or asn1compile anywhere -- they are listed in noinst_PROGRAMS -- so it's not obvious how one is supposed to create the directory to pass to --with-cross-tools-dir.

Great point! We should definitely install at least asn1_compile and bits, especially given that asn1_compile has gotten more generally useful.

I believe the other two in-tree compilers, slc, and compile_et do not need knowledge of the target ABI, so we can always build them in-tree for the local ABI then run them, and we don't need to install them, so we can just build them with $(CC_FOR_BUILD).

  * Simply installing an existing Heimdal package -- which contains the content of the installation prefix, but not of the source tree -- is not enough to do this because it won't contain the executables.

Because we'd have a chicken-and-egg problem until a) we make an 8.0 release, and b) all the distributions upgrade their Heimdal pkgs to 8.0, we'll have to go with --with-cross-tools-dir. But that doesn't solve the need for our in-tree compilers to have knowledge of the target architecture.

What we probably need is a description of the target architecture that asn1_compile and bits can consume.

(The slc and compile_et programs don't need to know anything about the target architecture, but asn1_compile does.)

Perhaps we could require that one provide bits' output in order to cross-compile, and make sure that emits all the defines that asn1_compile would need. As it happens asn1_compile does have an argument for this:

--type-file=FILE            Name of a C header file to generate includes of for base types

That might be sufficient... I'd need to check carefully that asn1_compile doesn't need knowledge of the target ABI, that --type-file=FILE is sufficient, and if so, besides the use of $(CC_FOR_BUILD) to build in-tree compilers we'd need a ./configure option for specifying a krb5-types.h file to use instead of running bits to generate it.

@nicowilliams
Copy link
Contributor

I think in fact asn1_compile may indeed not need knowledge of the target ABI, just a --type-file=FILE argument. We'd have to use $(CC_FOR_BUILD) to build asn1_compile itself, and $(CC) to build its outputs.

Thank you for this issue. As I never cross-compile Heimdal, I've not stopped to think about it, but your issue clarified a bunch of things in my mind.

@riastradh
Copy link
Author

Of these four bits and asn1_compiled definitely need knowledge of the target ABI in order to emit correct output for the target ABI.

What knowledge of the target ABI do they require?

This could be passed through CPPFLAGS_FOR_BUILD, perhaps? Or, in some cases like sizeof various types, discovered by autoconf (through compile-tests rather than run-tests).

@nicowilliams
Copy link
Contributor

Ah, we don't even need to tell asn1_compile what krb5-types.h file to use -- as long as the one found during the build is the one given to ./configure. So I think all in all we only need to:

  • add a ./configure option by which to supply a krb5-types.h file instead of building and running include/bits
  • use $(CC_FOR_BUILD) to build lib/sl/slc, lib/com_err/compile_et, and lib/asn1/asn1_compile

though we should also

  • check that indeed lib/asn1/asn1_compile does not need knowledge of the target ABI, that the krb5-types.h file is sufficient

@nicowilliams
Copy link
Contributor

What knowledge of the target ABI do they require?

include/bits emits output like:

#ifndef __krb5_types_h__
#define __krb5_types_h__

#include <inttypes.h>
#include <sys/types.h>
#include <sys/bitypes.h>
#include <sys/socket.h>


typedef socklen_t krb5_socklen_t;
#include <unistd.h>
typedef ssize_t krb5_ssize_t;

typedef int krb5_socket_t;

#if !defined(__has_extension)
#define __has_extension(x) 0
#endif

#ifndef KRB5TYPES_REQUIRE_GNUC
#define KRB5TYPES_REQUIRE_GNUC(m,n,p) \
    (((__GNUC__ * 10000) + (__GNUC_MINOR__ * 100) + __GNUC_PATCHLEVEL__) >= \
     (((m) * 10000) + ((n) * 100) + (p)))
#endif

#ifndef HEIMDAL_DEPRECATED
#if __has_extension(deprecated) || KRB5TYPES_REQUIRE_GNUC(3,1,0)
#define HEIMDAL_DEPRECATED __attribute__ ((__deprecated__))
#elif defined(_MSC_VER) && (_MSC_VER>1200)
#define HEIMDAL_DEPRECATED __declspec(deprecated)
#else
#define HEIMDAL_DEPRECATED
#endif
#endif

#ifndef HEIMDAL_PRINTF_ATTRIBUTE
#if __has_extension(format) || KRB5TYPES_REQUIRE_GNUC(3,1,0)
#define HEIMDAL_PRINTF_ATTRIBUTE(x) __attribute__ ((__format__ x))
#else
#define HEIMDAL_PRINTF_ATTRIBUTE(x)
#endif
#endif

#ifndef HEIMDAL_NORETURN_ATTRIBUTE
#if __has_extension(noreturn) || KRB5TYPES_REQUIRE_GNUC(3,1,0)
#define HEIMDAL_NORETURN_ATTRIBUTE __attribute__ ((__noreturn__))
#else
#define HEIMDAL_NORETURN_ATTRIBUTE
#endif
#endif

#ifndef HEIMDAL_UNUSED_ATTRIBUTE
#if __has_extension(unused) || KRB5TYPES_REQUIRE_GNUC(3,1,0)
#define HEIMDAL_UNUSED_ATTRIBUTE __attribute__ ((__unused__))
#else
#define HEIMDAL_UNUSED_ATTRIBUTE
#endif
#endif

#ifndef HEIMDAL_WARN_UNUSED_RESULT_ATTRIBUTE
#if __has_extension(warn_unused_result) || KRB5TYPES_REQUIRE_GNUC(3,3,0)
#define HEIMDAL_WARN_UNUSED_RESULT_ATTRIBUTE __attribute__ ((__warn_unused_result__))
#else
#define HEIMDAL_WARN_UNUSED_RESULT_ATTRIBUTE
#endif
#endif

#endif /* __krb5_types_h__ */

asn1_compile emits output that includes sizeof() expressions, but those are fine because those outputs should be compiled with $(CC), so I think asn1_compile needs no changes, and the only changes needed in lib/asn1 are in its Makefile.am to use $(CC_FOR_BUILD) to build the compiler.

@riastradh
Copy link
Author

riastradh commented Jun 21, 2023

Because we'd have a chicken-and-egg problem until a) we make an 8.0 release, and b) all the distributions upgrade their Heimdal pkgs to 8.0, we'll have to go with --with-cross-tools-dir. But that doesn't solve the need for our in-tree compilers to have knowledge of the target architecture.

This doesn't really pose a chicken-and-egg problem, as long as the asn1_compile and compile_et executables are themselves target-independent -- and even then it's a matter of convenience. It is perfectly reasonable for the build of a cross-built Heimdal to depend on a natively-built Heimdal of the same version. Several packages in pkgsrc already do this.

If the asn1_compile and/or compile_et executables are target-dependent, it is a little more inconvenient, but in pkgsrc, for example, we could have the normal heimdal-8.0.0, which installs, e.g., /usr/pkg/bin/kinit and /usr/pkg/bin/asn1_compile, live side-by-side with heimdal-aarch64--netbsd-8.0.0, which installs, e.g., /usr/pkg/aarch64--netbsd/bin/asn1_compile instead. Then in order to cross-build heimdal-8.0.0 for aarch64--netbsd, the builder would first need to install a native build of heimdal-aarch64--netbsd-8.0.0. (A cross build requiring a native build of some toolchain package other than gcc or build-essential is very common, of course.)

But it would be nicer if they were target-independent executables so we wouldn't have to do this.

@nicowilliams
Copy link
Contributor

I'm now pretty sure that the in-tree code generators other than include/bits.c are target-independent, but we do need to add a ./configure option by which to provide a krb5-types.h file.

@nicowilliams
Copy link
Contributor

nicowilliams commented Jun 22, 2023

So let's start with this:

diff --git a/configure.ac b/configure.ac
index cedb4c01f67..04f0722b991 100644
--- a/configure.ac
+++ b/configure.ac
@@ -18,6 +18,7 @@ dnl Checks for programs.
 AC_PROG_CC
 AM_PROG_CC_C_O
 AC_PROG_CPP
+AX_PROG_CC_FOR_BUILD
 AM_PATH_PYTHON
 AC_CHECK_PROG(CLANG_FORMAT, clang-format, [clang-format], [no])
 test "$CLANG_FORMAT" = no && CLANG_FORMAT=true

which is obvious enough, and then, sure, how about compile_et

diff --git a/lib/com_err/Makefile.am b/lib/com_err/Makefile.am
index 14e8e66fcdc..17dc5f10b13 100644
--- a/lib/com_err/Makefile.am
+++ b/lib/com_err/Makefile.am
@@ -31,6 +31,7 @@ libcom_err_la_DEPENDENCIES = version-script.map

 $(compile_et_OBJECTS): parse.h parse.c ## XXX broken automake 1.4s

+compile_et : CC=$(CC_FOR_BUILD)
 compile_et_LDADD = \
        libcom_err.la \
        $(LIB_roken) \

that immediately fails with:

make
gcc -O0 -ggdb3  -I/usr/include/et  -D_LARGE_FILES= -L/tmp/ossl/lib -Wl,-rpath,/tmp/ossl/lib  /home/nico/ws/heimdal/lib/com_err/compile_et.c   -o compile_et
/home/nico/ws/heimdal/lib/com_err/compile_et.c:36:10: fatal error: config.h: No such file or directory
   36 | #include <config.h>
      |          ^~~~~~~~~~
compilation terminated.
make: *** [<builtin>: compile_et] Error 1

OK, well, we shouldn't need config.h, but why isn't the compiler finding it? My make-fu is failing me right now. I tried using target-specific macro assignments to set COMPILE for compile_et so it has the right value, but that didn't work either.

But anyways, we see that config.h is going to be a problem, and so is roken.h, so that's annoying. How is one supposed to get a config.h for the local host rather than the target?

@nicowilliams
Copy link
Contributor

Ah, let's forget $(CC_FOR_BUILD). Instead lets install slc, compile_et, and asn1_compile, which we'll get from --with-cross-tools=DIR, which is an existing ./configure option, and we'll fix the various Makefile.am files to use the correct macros to invoke the cross-compilation tools when cross-compiling.

@riastradh
Copy link
Author

OK, well, we shouldn't need config.h, but why isn't the compiler finding it? My make-fu is failing me right now.

config.h would be wrong in CC_FOR_BUILD, because it describes the system that you're building for, not the system that you're building and running compile_et and asn1_compile on.

I tried using target-specific macro assignments to set COMPILE for compile_et so it has the right value, but that didn't work either.

I think you are probably supposed to set compile_et_COMPILE (target-specific macro assignments are a gmakeism anyway).

But anyways, we see that config.h is going to be a problem, and so is roken.h, so that's annoying. How is one supposed to get a config.h for the local host rather than the target?

You aren't, really. CC_FOR_BUILD is usually pretty limited, and it would be reasonable to just assume a standard C environment without much more; if you have to get more out of the host system, probably better to go the route of installing the tools and having a cross->native self-dependency.

@riastradh
Copy link
Author

I tried using target-specific macro assignments to set COMPILE for compile_et so it has the right value, but that didn't work either.

I think you are probably supposed to set compile_et_COMPILE (target-specific macro assignments are a gmakeism anyway).

Never mind, that doesn't work, as far as I can tell; I think it is necessary after all to write explicit rules invoking $(CC_FOR_BUILD) to compile and link the components of compile_et.

@nicowilliams
Copy link
Contributor

We do not have to build native versions of compile_et, slc, and asn1_compile -- just require that the user provide previously-built ones with --with-cross-tools=DIR. All we have to do is add a --with-krb5-types-header=FILE argument and fix up the --with-cross-tools=DIR code and the makefiles that invoke compile_et, slc, and asn1_compile to use those, and include/Makefile.am to use the header file provided with --with-krb5-types-header=FILE instead of building and running the bits program.

Or, at any rate, that seems to be the design for cross-compilation that we've inherited -- it just isn't complete.

Building with $(CC_FOR_BUILD) presents severe difficulties since we'd have to build native and target versions of lib/roken and lib/base, and we'd need native and target versions of config.h -- that's just ETOOHARD. First building natively to get the cross compilation tools, then cross-compiling with --with-cross-tools=DIR should be enough. We could even arrange to do a full native build first so the user doesn't have to, but I think that's overkill.

@nicowilliams
Copy link
Contributor

We might want to review every use of AC_RUN_IFELSE. Maybe for each of them we'd want to provide a ./configure option for specifying the behavior on the target, or -much easier- maybe the user would be expected to hand-edit config.h (I think that's just fine).

@nicowilliams
Copy link
Contributor

nicowilliams commented Jun 22, 2023

Also, regarding include/bits.c, the parts that are target ABI dependent are about printing typedefs for [u]int[8,16,32,64]_t, and if we demand that the target platform provide those via <stdint.h>, then we could just remove that code. Or... maybe we could use AX_COMPILE_CHECK_SIZEOF to check do sizeof checks when cross-compiling, and AC_C_CHAR_UNSIGNED, and... it looks like AX_CHECK_SIGN should work when cross-compiling because it doesn't run the program it compiles -I think-, so with a bit of work we can make include/bits.c possible to build with $(CC_FOR_BUILD) and then we wouldn't need to have the user provide a krb5-types.h file.

@nicowilliams
Copy link
Contributor

nicowilliams commented Jun 22, 2023

We don't even need include/bits with these macros... We can just edit include/krb5-types.cross to have all the #ifndef HAVE_INT8 and such, rename it to include/krb5-types.h, then delete include/bits.c!

@riastradh
Copy link
Author

We might want to review every use of AC_RUN_IFELSE. Maybe for each of them we'd want to provide a ./configure option for specifying the behavior on the target, or -much easier- maybe the user would be expected to hand-edit config.h (I think that's just fine).

The best way, if AC_RUN_IFELSE is unavoidable, is to wrap it in AC_CACHE_CHECK. Then:

  1. The result can be cached, speeding up repeated configure checks when rerunning configure.
  2. The result can be overridden by heim_cv_mumble_frotz=yes or heim_cv_mumble_frotz=no on the configure command line or environment, which we use extensively in pkgsrc for cross-compiling based on prior knowledge of the platform.

This also saves effort on keeping the AC_MSG_CHECKING/RESULT business consistent.

This is much nicer to work with than patching config.h.

Also, regarding include/bits.c, the parts that are target ABI dependent are about printing typedefs for [u]int[8,16,32,64]_t, and if we demand that the target platform provide those via <stdint.h>, then we could just remove that code.

It's 2023. <stdint.h> was standardized in 1999, 24 years ago. I think it is reasonable to ask the target platform to provide its own uint32_t. If you really want to keep that legacy goo around, you can put it under #ifdef HAVE_CONFIG_H and in the other case (!HAVE_CONFIG_H) just use the standard types. This won't be a regression.

@nicowilliams
Copy link
Contributor

It's 2023. <stdint.h> was standardized in 1999, 24 years ago. I think it is reasonable to ask the target platform to provide its own uint32_t. If you really want to keep that legacy goo around, you can put it under #ifdef HAVE_CONFIG_H and in the other case (!HAVE_CONFIG_H) just use the standard types. This won't be a regression.

No doubt. If it were up to me we'd be using C11. But @jaltman has to support oddball builds on Windows versions that are no longer supported by MSFT. That said, we did change the Windows build to fake-up a stdint.h, so you're right. In any case, we can get rid of include/bits.c now I think.

@riastradh
Copy link
Author

riastradh commented Jun 22, 2023

But @jaltman has to support oddball builds on Windows versions that are no longer supported by MSFT.

Do those function as cross builds or as native builds?

As long as the native build can rely on config.h under #ifdef HAVE_CONFIG_H, whatever the native build of tool like compile_et does for a cross build does under #ifndef HAVE_CONFIG_H won't affect it.

@nicowilliams
Copy link
Contributor

But @jaltman has to support oddball builds on Windows versions that are no longer supported by MSFT.

Do those function as cross builds or as native builds?

Native, and they don't use autoconf.

As long as the native build can rely on config.h under #ifdef HAVE_CONFIG_H, whatever the native build of tool like compile_et does for a cross build does under #ifndef HAVE_CONFIG_H won't affect it.

Building slc, compile_et, and asn1_compile natively in the same build as cross building is ETOOHARD (see above). Instead the thing to do is to first build natively, install (and for now, manually install those three programs), then run a cross-build with --with-cross-tools=/path/to/dir/containing/those/programs. The upshot is that we'll not need to have any uses of $(CC_FOR_BUILD) and we'll not need to run any of the target host build executables.

Installing slc, compile_et, and asn1_compile would be nice, but we should probably give the first two a better name, something like heim_slc and heim_compile_et.

@nicowilliams
Copy link
Contributor

I'm putting a PR together, FYI. The thought of cross-builds w/ MingW for Windows builds is appealing!

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

No branches or pull requests

2 participants