terralang / terra

Terra is a low-level system programming language that is embedded in and meta-programmed by the Lua programming language.
terralang.org
Other
2.72k stars 201 forks source link

Terra Cannot Import Inlined C Functions #401

Closed ErikMcClure closed 5 years ago

ErikMcClure commented 5 years ago

As I discovered while trying to get the tests to work again in #400, Terra shouldn't actually be working on windows at all. This is because printf is defined as an inline function and has been since Visual Studio 2015. As a result, printf doesn't exist. You can force it to exist by including legacy_stdio_definitions.lib, but you must include this manually on every single project on windows that uses printf, without disrupting linux compilations, like so:

terralib.saveobj("benchmark_fannkuchredux", { main = main }, (jit.os == "Windows" and {"\\legacy_stdio_definitions.lib"} or nil) )

The larger issue here is that terra cannot actually compile any inline-only function because the function immediately ceases to exist. This also applies to things like compiler intrinsics that don't correspond to an actual function symbol.

The inline-only problem can be fixed by using llvm.used, as was done in #405, but this does not fix the stdio specific symbol resolution problem.

We could also consider trying to just manually include the stdio_legacy_definitions.lib on all windows compilations (plus our own library, if necessary) and therefore ensure that all of the standard library functions have symbols, at the very least. Whatever option is chosen, using the C standard library's printf() function should not result in random, confusing compiler errors that are incredibly hard to track down.

ErikMcClure commented 5 years ago

benchmark_fannkuchredux.t and benchmark_nbody.t are being temporarily skipped until we can consistently resolve this across visual studio versions.

ErikMcClure commented 5 years ago

The inline function issue has been resolved using llvm.used, but there is still a windows-specific stdio.h problem with duplicate symbols:

msvcrt.lib(default_local_stdio_options.obj) : error LNK2005: __local_stdio_printf_options already defined in terra-19860a.o
stdio.exe : fatal error LNK1169: one or more multiply defined symbols found

This function has the following definition:

// This function must not be inlined into callers to avoid ODR violations.  The
// static local variable has different names in C and in C++ translation units.
_Check_return_ _Ret_notnull_
__declspec(noinline) __inline unsigned __int64* __CRTDECL __local_stdio_printf_options(void)
{
    static unsigned __int64 _OptionsStorage;
    return &_OptionsStorage;
}

The linking problem will only occur if printf is called in a function reachable from main, and saveobj is used to save an executable. Clang itself does not run into this problem, but seems to declare this symbol as ExternallyAvailable:

$__local_stdio_printf_options = comdat any

@"??_C@_06FJFFADID@C?3?5?$CFi?6?$AA@" = linkonce_odr dso_local unnamed_addr constant [7 x i8] c"C: %i\0A\00", comdat, align 1
@__local_stdio_printf_options._OptionsStorage = internal global i64 0, align

The LLVM that Terra gets back from clang, however, is an actual function definition:

; Function Attrs: noinline norecurse nounwind
define linkonce_odr dso_local i64* @__local_stdio_printf_options() local_unnamed_addr #4 {
entry:
  ret i64* @__local_stdio_printf_options._OptionsStorage
}

It is possible clang has a substitute stdio.h file it uses to compensate for Windows being insane. It's not obvious how or if it's even possible to fix this problem, or if it will cause issues with other windows header files in the future. Manually setting this function to ExternallyAvailable actually solves the problem, which confirms that the primary issue here is that the function body shouldn't exist, yet it does anyway. How does clang know to avoid emitting the function body? Why does terra's invocation of LLVM insist on giving it one?

if(fn->getName().equals("__local_stdio_printf_options") || fn->getName().equals("__local_stdio_scanf_options"))
  fn->setLinkage(llvm::GlobalValue::AvailableExternallyLinkage);