Refactor getSectionDescription in src/link/assign.cpp

since they may call `isatty` which can change `errno`

Fixes #1857

CMakeLists.txt

@@ -49,7 +49,7 @@ else()
                   -fsanitize=float-divide-by-zero)
     add_compile_options(${SAN_FLAGS})
     add_link_options(${SAN_FLAGS})
-    add_definitions(-D_GLIBCXX_ASSERTIONS)
+    add_definitions(-D_GLIBCXX_ASSERTIONS -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG)
     # A non-zero optimization level is desired in debug mode, but allow overriding it nonetheless
     set(CMAKE_CXX_FLAGS_DEBUG "-g -Og -fno-omit-frame-pointer -fno-optimize-sibling-calls ${CMAKE_CXX_FLAGS_DEBUG}"
                               CACHE STRING "" FORCE)

Makefile

@@ -222,8 +222,8 @@ develop:
 		-Wformat=2 -Wformat-overflow=2 -Wformat-truncation=1 \
 		-Wno-format-nonliteral -Wno-strict-overflow -Wno-unused-but-set-variable \
 		-Wno-type-limits -Wno-tautological-constant-out-of-range-compare -Wvla \
-		-D_GLIBCXX_ASSERTIONS -fsanitize=address -fsanitize=undefined \
-		-fsanitize=float-divide-by-zero" \
+		-D_GLIBCXX_ASSERTIONS -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG \
+		-fsanitize=address -fsanitize=undefined -fsanitize=float-divide-by-zero" \
 		CXXFLAGS="-ggdb3 -Og -fno-omit-frame-pointer -fno-optimize-sibling-calls"
 
 # Target used in development to debug with gdb.

include/verbosity.hpp

@@ -3,17 +3,14 @@
 #ifndef RGBDS_VERBOSITY_HPP
 #define RGBDS_VERBOSITY_HPP
 
+#include <stdarg.h>
 #include <stdio.h>
 
-#include "style.hpp"
-
 // This macro does not evaluate its arguments unless the condition is true.
 #define verbosePrint(level, ...) \
 	do { \
 		if (checkVerbosity(level)) { \
-			style_Set(stderr, STYLE_MAGENTA, false); \
-			fprintf(stderr, __VA_ARGS__); \
-			style_Reset(stderr); \
+			printVerbosely(__VA_ARGS__); \
 		} \
 	} while (0)
 
@@ -30,6 +27,9 @@ enum Verbosity {
 void incrementVerbosity();
 bool checkVerbosity(Verbosity level);
 
+[[gnu::format(printf, 1, 2)]]
+void printVerbosely(char const *fmt, ...);
+
 void printVVVVVVerbosity();
 
 #endif // RGBDS_VERBOSITY_HPP

src/asm/section.cpp

@@ -120,9 +120,8 @@ Section *sect_FindSectionByName(std::string const &name) {
 		++nbSectErrors; \
 	} while (0)
 
-static unsigned int mergeSectUnion(
-    Section &sect, SectionType type, uint32_t org, uint8_t alignment, uint16_t alignOffset
-) {
+static unsigned int
+    mergeSectUnion(Section &sect, uint32_t org, uint8_t alignment, uint16_t alignOffset) {
 	unsigned int nbSectErrors = 0;
 
 	assume(alignment < 16); // Should be ensured by the caller
@@ -133,12 +132,6 @@ static unsigned int mergeSectUnion(
 	uint32_t sectAlignSize = 1u << sect.align;
 	uint32_t sectAlignMask = sectAlignSize - 1;
 
-	// Unionized sections only need "compatible" constraints, and they end up with the strictest
-	// combination of both.
-	if (sectTypeHasData(type)) {
-		sectError("Cannot declare ROM sections as `UNION`");
-	}
-
 	if (org != UINT32_MAX) {
 		// If both are fixed, they must be the same
 		if (sect.org != UINT32_MAX && sect.org != org) {
@@ -266,12 +259,10 @@ static void mergeSections(
 	} else {
 		switch (mod) {
 		case SECTION_UNION:
-		case SECTION_FRAGMENT:
-			nbSectErrors += mod == SECTION_UNION
-			                    ? mergeSectUnion(sect, type, org, alignment, alignOffset)
-			                    : mergeFragments(sect, org, alignment, alignOffset);
-
-			// Common checks
+		case SECTION_FRAGMENT: {
+			unsigned int (*merge)(Section &, uint32_t, uint8_t, uint16_t) =
+			    mod == SECTION_UNION ? mergeSectUnion : mergeFragments;
+			nbSectErrors += merge(sect, org, alignment, alignOffset);
 
 			// If the section's bank is unspecified, override it
 			if (sect.bank == UINT32_MAX) {
@@ -282,6 +273,7 @@ static void mergeSections(
 				sectError("Section already declared with different bank %" PRIu32, sect.bank);
 			}
 			break;
+		}
 
 		case SECTION_NORMAL:
 			errorNoTrace([&]() {
@@ -513,6 +505,11 @@ void sect_NewSection(
 		}
 	}
 
+	if (mod == SECTION_UNION && sectTypeHasData(type)) {
+		error("Cannot declare ROM sections as `UNION`");
+		return;
+	}
+
 	if (currentLoadSection) {
 		sect_EndLoadSection("SECTION");
 	}
@@ -1121,12 +1118,12 @@ std::string sect_PushSectionFragmentLiteral() {
 		);
 	}
 
+	// This section has data (ROM0 or ROMX), so it cannot be a UNION
+	assume(currentSection->modifier != SECTION_UNION);
+
 	if (currentLoadSection) {
 		fatal("`LOAD` blocks cannot contain fragment literals");
 	}
-	if (currentSection->modifier == SECTION_UNION) {
-		fatal("`SECTION UNION` cannot contain fragment literals");
-	}
 
 	// A section containing a fragment literal has to become a fragment too
 	currentSection->modifier = SECTION_FRAGMENT;

src/cli.cpp

@@ -21,10 +21,11 @@ static std::vector<size_t>
 
 	std::filebuf file;
 	if (!file.open(path, std::ios_base::in)) {
+		int errnum = errno;
 		style_Set(stderr, STYLE_RED, true);
 		fputs("FATAL: ", stderr);
 		style_Reset(stderr);
-		fprintf(stderr, "Failed to open at-file \"%s\": %s\n", path.c_str(), strerror(errno));
+		fprintf(stderr, "Failed to open at-file \"%s\": %s\n", path.c_str(), strerror(errnum));
 		usage.printAndExit(1);
 	}
 

src/link/assign.cpp

@@ -208,50 +208,37 @@ static std::optional<size_t> getPlacement(Section const &section, MemoryLocation
 }
 
 static std::string getSectionDescription(Section const &section) {
-	std::string where;
-
-	char bank[8], addr[8], mask[8], offset[8];
+	std::string description =
+	    "\"" + section.name + "\" (" + sectionTypeInfo[section.type].name + " section) ";
 	if (section.isBankFixed && sectTypeBanks(section.type) != 1) {
+		char bank[8];
 		snprintf(bank, sizeof(bank), "%02" PRIx32, section.bank);
-	}
 		if (section.isAddressFixed) {
+			char addr[8];
 			snprintf(addr, sizeof(addr), "%04" PRIx16, section.org);
+			description = description + "at $" + bank + ":" + addr;
+		} else if (section.isAlignFixed) {
+			char mask[8];
+			snprintf(mask, sizeof(mask), "%" PRIx16, static_cast<uint16_t>(~section.alignMask));
+			description = description + "in bank $" + bank + " with align mask $" + mask;
+		} else {
+			description = description + "in bank $" + bank;
 		}
-	if (section.isAlignFixed) {
+	} else {
+		if (section.isAddressFixed) {
+			char addr[8];
+			snprintf(addr, sizeof(addr), "%04" PRIx16, section.org);
+			description = description + "at address $" + addr;
+		} else if (section.isAlignFixed) {
+			char mask[8], offset[8];
 			snprintf(mask, sizeof(mask), "%" PRIx16, static_cast<uint16_t>(~section.alignMask));
 			snprintf(offset, sizeof(offset), "%" PRIx16, section.alignOfs);
-	}
-
-	if (section.isBankFixed && sectTypeBanks(section.type) != 1) {
-		if (section.isAddressFixed) {
-			where = "at $";
-			where += bank;
-			where += ":";
-			where += addr;
-		} else if (section.isAlignFixed) {
-			where = "in bank $";
-			where += bank;
-			where += " with align mask $";
-			where += mask;
+			description = description + "with align mask $" + mask + " and offset $" + offset;
 		} else {
-			where = "in bank $";
-			where += bank;
-		}
-	} else {
-		if (section.isAddressFixed) {
-			where = "at address $";
-			where += addr;
-		} else if (section.isAlignFixed) {
-			where = "with align mask $";
-			where += mask;
-			where += " and offset $";
-			where += offset;
-		} else {
-			where = "anywhere";
+			description = description + "anywhere";
 		}
 	}
-
-	return where;
+	return description;
 }
 
 // Places a section in a suitable location, or error out if it fails to.
@@ -310,19 +297,11 @@ static void placeSection(Section &section) {
 
 	if (!section.isBankFixed || !section.isAddressFixed) {
 		// If a section failed to go to several places, nothing we can report
-		fatal(
-		    "Unable to place \"%s\" (%s section) %s",
-		    section.name.c_str(),
-		    sectionTypeInfo[section.type].name.c_str(),
-		    getSectionDescription(section).c_str()
-		);
+		fatal("Unable to place %s", getSectionDescription(section).c_str());
 	} else if (section.org + section.size > sectTypeEndAddr(section.type) + 1) {
 		// If the section just can't fit the bank, report that
 		fatal(
-		    "Unable to place \"%s\" (%s section) %s: section runs past end of region ($%04x > "
-		    "$%04x)",
-		    section.name.c_str(),
-		    sectionTypeInfo[section.type].name.c_str(),
+		    "Unable to place %s: section runs past end of region ($%04x > $%04x)",
 		    getSectionDescription(section).c_str(),
 		    section.org + section.size,
 		    sectTypeEndAddr(section.type) + 1
@@ -330,9 +309,7 @@ static void placeSection(Section &section) {
 	} else {
 		// Otherwise there is overlap with another section
 		fatal(
-		    "Unable to place \"%s\" (%s section) %s: section overlaps with \"%s\"",
-		    section.name.c_str(),
-		    sectionTypeInfo[section.type].name.c_str(),
+		    "Unable to place %s: section overlaps with \"%s\"",
 		    getSectionDescription(section).c_str(),
 		    out_OverlappingSection(section)->name.c_str()
 		);

src/verbosity.cpp

@@ -24,6 +24,15 @@ void incrementVerbosity() {
 	}
 }
 
+void printVerbosely(char const *fmt, ...) {
+	va_list args;
+	style_Set(stderr, STYLE_MAGENTA, false);
+	va_start(args, fmt);
+	vfprintf(stderr, fmt, args);
+	va_end(args);
+	style_Reset(stderr);
+}
+
 void printVVVVVVerbosity() {
 	if (!checkVerbosity(VERB_VVVVVV)) {
 		return;

src/version.cpp

@@ -14,7 +14,13 @@
 #if !defined(NDEBUG) && defined(__SANITIZE_ADDRESS__) && !defined(__APPLE__)
 extern "C" {
 	char const *__asan_default_options(void) {
-		return "detect_leaks=1";
+		return "detect_leaks=1"
+		       ":detect_stack_use_after_return=1"
+		       ":detect_invalid_pointer_pairs=2"
+		       ":check_initialization_order=1"
+		       ":strict_init_order=1"
+		       ":strict_string_checks=1"
+		       ":print_legend=0";
 	}
 }
 #endif

test/asm/fragment-literal-in-union.err

@@ -1,2 +1,6 @@
-FATAL: `SECTION UNION` cannot contain fragment literals
+error: Cannot declare ROM sections as `UNION`
+    at fragment-literal-in-union.asm(1)
+error: Cannot output data outside of a `SECTION`
+    at fragment-literal-in-union.asm(2)
+FATAL: Cannot output fragment literals outside of a `SECTION`
     at fragment-literal-in-union.asm(3)

test/asm/section-union-data.asm

@@ -0,0 +1,2 @@
+SECTION UNION "wat", ROM0
+db 42

test/asm/section-union-data.err

@@ -0,0 +1,5 @@
+error: Cannot declare ROM sections as `UNION`
+    at section-union-data.asm(1)
+error: Cannot output data outside of a `SECTION`
+    at section-union-data.asm(2)
+Assembly aborted with 2 errors!

test/link/section-conflict/different-mod/a.asm

@@ -1,2 +1,2 @@
-section fragment "test", rom0
-db 1
+section fragment "test", wram0
+w1:: db

test/link/section-conflict/different-mod/b.asm

@@ -1,2 +1,2 @@
-section union "test", rom0
-db 2
+section union "test", wram0
+w2:: db

test/link/section-union/data-overlay.asm

@@ -1,10 +0,0 @@
-IF !DEF(SECOND)
-	def DATA equs "ds 4"
-ELSE
-	def DATA equs "db $aa, $bb, $cc, $dd"
-ENDC
-
-SECTION UNION "overlaid data", ROM0
-	{DATA}
-
-	PURGE DATA

test/link/section-union/data-overlay.out

@@ -1,7 +0,0 @@
-FATAL: Section "overlaid data" is of type `ROM0`, which cannot be `UNION`ized
-Linking aborted with 1 error
----
-error: Cannot declare ROM sections as `UNION`
-    at <stdin>(18)
-FATAL: Cannot create section "overlaid data" (1 error)
-    at <stdin>(18)

test/link/section-union/different-data.asm

@@ -1,8 +0,0 @@
-IF !DEF(SECOND)
-	def DATA = 1
-ELSE
-	def DATA = 2
-ENDC
-
-SECTION UNION "different data", ROM0
-	db DATA

test/link/section-union/different-data.out

@@ -1,7 +0,0 @@
-FATAL: Section "different data" is of type `ROM0`, which cannot be `UNION`ized
-Linking aborted with 1 error
----
-error: Cannot declare ROM sections as `UNION`
-    at <stdin>(16)
-FATAL: Cannot create section "different data" (1 error)
-    at <stdin>(16)

test/link/section-union/different-size.asm

@@ -1,8 +0,0 @@
-IF !DEF(SECOND)
-	def SIZE = 69
-ELSE
-	def SIZE = 420
-ENDC
-
-SECTION UNION "different section sizes", ROM0
-	ds SIZE

test/link/section-union/different-size.out

@@ -1,7 +0,0 @@
-FATAL: Section "different section sizes" is of type `ROM0`, which cannot be `UNION`ized
-Linking aborted with 1 error
----
-error: Cannot declare ROM sections as `UNION`
-    at <stdin>(16)
-FATAL: Cannot create section "different section sizes" (1 error)
-    at <stdin>(16)

test/link/section-union/different-syntaxes.asm

@@ -1,10 +0,0 @@
-IF !DEF(SECOND)
-	def INSTR equs "sbc a"
-ELSE
-	def INSTR equs "db $9f"
-ENDC
-
-SECTION UNION "different syntaxes", ROM0
-	{INSTR}
-
-	PURGE INSTR

test/link/section-union/different-syntaxes.out

@@ -1,7 +0,0 @@
-FATAL: Section "different syntaxes" is of type `ROM0`, which cannot be `UNION`ized
-Linking aborted with 1 error
----
-error: Cannot declare ROM sections as `UNION`
-    at <stdin>(18)
-FATAL: Cannot create section "different syntaxes" (1 error)
-    at <stdin>(18)

test/link/section-union/split-data.asm

@@ -1,10 +0,0 @@
-IF !DEF(SECOND)
-	def DATA equs "ds 1\ndb $aa"
-ELSE
-	def DATA equs "db $bb\nds 1"
-ENDC
-
-SECTION UNION "mutually-overlaid data", ROM0
-	{DATA}
-
-	PURGE DATA

test/link/section-union/split-data.out

@@ -1,7 +0,0 @@
-FATAL: Section "mutually-overlaid data" is of type `ROM0`, which cannot be `UNION`ized
-Linking aborted with 1 error
----
-error: Cannot declare ROM sections as `UNION`
-    at <stdin>(18)
-FATAL: Cannot create section "mutually-overlaid data" (1 error)
-    at <stdin>(18)