Selaa lähdekoodia

linker improvements

`path_to_fullpath` did different things on Windows&Unix, an attempt to
bring them closer together was made here.
This was prompted by the compiler completely ignoring
`foreign import "foo.dylib"` when `foo.dylib` does not exist (because
`path_to_fullpath` returns an empty string).
Causing just unresolved symbol errors, when on Windows it would pass
along the path to the linker and actually say it doesn't exist, which
is now also the case for Unix.
This also fixes some checker errors that relied on the Windows
behaviour, for example: `Error: File name, , cannot be as a library name
as it is not a valid identifier`.

Made `-no-crt` require either `-default-to-nil-allocator` or
`-default-to-panic-allocator` on Unix, the current default allocators
rely on libc and this was not obvious and would immediately bring up
unresolved symbol errors for the linked memory management functions, or
just link with libc anyways because it was foreign imported.

Added a suggestion to install `nasm` with the user's package manager
when assembling using `nasm` fails on Unix, I saw some confusion about
it in the Discord.

Ignore explicit foreign imports of libc. It is already linked in later
on in the linking process and would otherwise (at least on macOS) cause
linker warnings for duplicate libraries. This also makes it so when
using `-no-crt` and importing something that requires libc, linker
errors are given (like I would expect), instead of silently still
linking with libc because it was foreign imported.
Laytan Laats 1 vuosi sitten
vanhempi
commit
a555862522
2 muutettua tiedostoa jossa 36 lisäystä ja 5 poistoa
  1. 25 2
      src/build_settings.cpp
  2. 11 3
      src/linker.cpp

+ 25 - 2
src/build_settings.cpp

@@ -1190,13 +1190,24 @@ gb_internal String path_to_fullpath(gbAllocator a, String s, bool *ok_) {
 	char *p;
 	mutex_lock(&fullpath_mutex);
 	p = realpath(cast(char *)s.text, 0);
+	defer (free(p));
 	mutex_unlock(&fullpath_mutex);
 	if(p == nullptr) {
 		if (ok_) *ok_ = false;
-		return String{};
+
+		// Path doesn't exist or is malformed, Windows's `GetFullPathNameW` does not check for
+		// existence of the file where `realpath` does, which causes different behaviour between platforms.
+		// Two things could be done here:
+		// 1. clean the path and resolve it manually, just like the Windows function does,
+		//    probably requires porting `filepath.clean` from Odin and doing some more processing.
+		// 2. just return a copy of the original path.
+		//
+		// I have opted for 2 because it is much simpler + we already return `ok = false` + further
+		// checks and processes will use the path and cause errors (which we want).
+		return copy_string(a, s);
 	}
 	if (ok_) *ok_ = true;
-	return make_string_c(p);
+	return copy_string(a, make_string_c(p));
 }
 #else
 #error Implement system
@@ -1939,6 +1950,18 @@ gb_internal bool init_build_paths(String init_filename) {
 		}
 	}
 
+	if (build_context.no_crt && !build_context.ODIN_DEFAULT_TO_NIL_ALLOCATOR && !build_context.ODIN_DEFAULT_TO_PANIC_ALLOCATOR) {
+		switch (build_context.metrics.os) {
+		case TargetOs_linux:
+		case TargetOs_darwin:
+		case TargetOs_essence:
+		case TargetOs_freebsd:
+		case TargetOs_openbsd:
+		case TargetOs_haiku:
+			gb_printf_err("-no-crt on unix systems requires either -default-to-nil-allocator or -default-to-panic-allocator to also be present because the default allocator requires crt\n");
+			return false;
+		}
+	}
 
 	if (bc->target_features_string.len != 0) {
 		enable_target_feature({}, bc->target_features_string);

+ 11 - 3
src/linker.cpp

@@ -376,6 +376,10 @@ gb_internal i32 linker_stage(LinkerData *gen) {
 								LIT(obj_file),
 								LIT(build_context.extra_assembler_flags)
 							);						
+							if (!result) {
+								gb_printf_err("executing `nasm` to assemble foreing import of %.*s failed.\n\tSuggestion: `nasm` does not ship with the compiler and should be installed with your system's package manager.\n", LIT(asm_file));
+								return result;
+							}
 						}
 						array_add(&gen->output_object_paths, obj_file);
 					} else {
@@ -383,9 +387,13 @@ gb_internal i32 linker_stage(LinkerData *gen) {
 							continue;
 						}
 
-						// NOTE(zangent): Sometimes, you have to use -framework on MacOS.
-						//   This allows you to specify '-f' in a #foreign_system_library,
-						//   without having to implement any new syntax specifically for MacOS.
+						// Do not add libc again, this is added later already, and omitted with
+						// the `-no-crt` flag, not skipping here would cause duplicate library
+						// warnings when linking on darwin and might link libc silently even with `-no-crt`.
+						if (lib == str_lit("System.framework") || lib == str_lit("c")) {
+							continue;
+						}
+
 						if (build_context.metrics.os == TargetOs_darwin) {
 							if (string_ends_with(lib, str_lit(".framework"))) {
 								// framework thingie