.area: Fix .byt:0000 entry being written to symfile if area is perfectly filled

Commands/CDirectiveArea.cpp

@@ -177,7 +177,10 @@ void CDirectiveArea::writeSymData(SymbolData& symData) const
 	if (fillExpression.isLoaded())
 	{
 		int64_t subAreaUsage = Allocations::getSubAreaUsage(fileID, position);
-		symData.addData(position+contentSize+subAreaUsage,areaSize-contentSize-subAreaUsage,SymbolData::Data8);
+		int64_t address = position+(contentSize+subAreaUsage);
+		size_t fillSize = areaSize-(contentSize+subAreaUsage);
+		if (fillSize != 0)
+			symData.addData(address,fillSize,SymbolData::Data8);
 	}
 }
 

Commands/CDirectiveFile.cpp

@@ -562,3 +562,162 @@ void DirectiveObjImport::writeSymData(SymbolData& symData) const
 
 	rel.writeSymbols(symData);
 }
+
+//
+// CDirectiveSymImport
+//
+std::string trimSymFileLine(std::string line)
+{
+	// Trim start of line
+	size_t lineStart = line.find_first_not_of(" \t");
+
+	// End line at comment (starting with ;)
+	// \x1A included here as well since No$gba .sym file ends with it
+	size_t lineEnd = line.find_first_of(";\x1A");
+
+	if (lineStart >= lineEnd)
+		return std::string(); // line consists of only whitespace
+
+	// Trim end of line
+	lineEnd = line.find_last_not_of(" \t",lineEnd-1);
+	// lineEnd now points to position of last char
+	return line.substr(lineStart,lineEnd-lineStart+1);
+}
+
+CDirectiveSymImport::CDirectiveSymImport(const fs::path& fileName)
+{
+	// We may be adding global labels, so start a new section
+	updateSection(++Global.Section);
+
+	this->fileName = getFullPathName(fileName);
+	if (!fs::exists(this->fileName))
+	{
+		Logger::printError(Logger::FatalError,"File %s not found",this->fileName.u8string());
+		return;
+	}
+
+	TextFile file;
+	file.open(this->fileName,TextFile::Read);
+	if (!file.isOpen())
+	{
+		Logger::printError(Logger::FatalError,"Could not open file %s",this->fileName.u8string());
+		return;
+	}
+
+	std::vector<std::string> lines = file.readAll();
+	size_t l = 0;
+	for (std::string line: lines)
+	{
+		l++;
+		line = trimSymFileLine(line);
+		if (line.empty())
+			continue;
+
+		// Parse symbol address
+		const char* addrStart = (char*)line.c_str();
+		char* addrEnd;
+		uint32_t symAddr = strtoul(addrStart,&addrEnd,16);
+		// Check: not 8 chars, not followed by space/tab
+		if (addrStart+8 != addrEnd || !strchr(" \t",*addrEnd))
+		{
+			Logger::printError(Logger::Warning,"Invalid symbol address on line %i of symbols file %s",l,this->fileName);
+			continue;
+		}
+
+		// Rest of the line is the symbol value
+		std::string symVal = std::string(addrEnd);
+		symVal = symVal.substr(symVal.find_first_not_of(" \t"));
+
+		std::string name = symVal;
+
+		// Get optional label size
+		size_t commaPos = symVal.find(',');
+		uint32_t funcSize = 0;
+		if (commaPos != std::string::npos)
+		{
+			// Parse size
+			const char* sizeStart = symVal.c_str()+commaPos+1;
+			char* sizeEnd;
+			funcSize = strtoul(sizeStart,&sizeEnd,16);
+			// Check: empty size, not in range, not followed by eol/space/tab/comma
+			if (sizeStart == sizeEnd || errno == ERANGE || !strchr("\0 \t,",*sizeEnd))
+			{
+				Logger::printError(Logger::Warning,"Invalid function size on line %i of symbols file %s",l,this->fileName);
+				funcSize = 0;
+				// We can still salvage this I guess
+			}
+			else {
+				// Got valid label size, so remove it from identifier (trim end in the process)
+				name = symVal.substr(0,symVal.find_first_of(" \t"));
+			}
+		}
+
+		// Create label for this symbol, if it's not a directive and it would be a global label
+		const Identifier identifier = Identifier(name);
+		if (name.find('.') != 0 && Global.symbolTable.isGlobalSymbol(identifier))
+		{
+			std::shared_ptr<Label> label = Global.symbolTable.getLabel(identifier,-1,-1);
+			if (label == nullptr)
+			{
+				// No$gba (supposedly...) allows pretty much any character, but armips doesn't
+				// We can't import this label, but if the user never references it, it's fine
+				// (If it IS referenced, that will eventually yield an error anyway)
+				Logger::printError(Logger::Warning,"Invalid label name \"%s\" on line %i of symbols file %s",name,l,this->fileName);
+				continue;
+			}
+			if (label->isDefined())
+			{
+				Logger::printError(Logger::Error,"Label \"%s\" already defined on line %i of symbols file %s",name,l,this->fileName);
+				continue;
+			}
+			// If already defined and not a global symbol, that's fine, we are in a dedicated section anyway
+			label->setOriginalName(identifier);
+			label->setValue(symAddr);
+			label->setInfo(funcSize);
+			label->setUpdateInfo(false);
+			label->setDefined(true);
+
+			labels.push_back(label);
+		} else {
+			// Store the symbol anyway since we want to merge everything into the output symfile
+			otherSymbols.push_back(std::pair(symAddr,symVal));
+		}
+	}
+}
+
+bool CDirectiveSymImport::Validate(const ValidateState& state)
+{
+	return false;
+}
+
+void CDirectiveSymImport::Encode() const
+{
+
+}
+
+void CDirectiveSymImport::writeTempData(TempData& tempData) const
+{
+	tempData.writeLine(-1,tfm::format(".importsym \"%s\"",fileName.u8string()));
+	for (const auto& label: labels)
+	{
+		tempData.writeLine(label->getValue(),tfm::format("%s",label->getName()));
+	}
+}
+
+void CDirectiveSymImport::writeSymData(SymbolData& symData) const
+{
+	for (const auto& label: labels)
+	{
+		if (label->getInfo())
+			symData.startFunction(label->getValue());
+
+		symData.addLabel(label->getValue(),label->getName().string());
+
+		if (label->getInfo())
+			symData.endFunction(label->getValue()+label->getInfo());
+	}
+	for (const auto& symbol: otherSymbols)
+	{
+		symData.addLabel(symbol.first,symbol.second);
+	}
+}

Commands/CDirectiveFile.h

@@ -134,3 +134,17 @@ class DirectiveObjImport: public CAssemblerCommand
 	ElfRelocator rel;
 	std::unique_ptr<CAssemblerCommand> ctor;
 };
+
+class CDirectiveSymImport : public CAssemblerCommand
+{
+public:
+	CDirectiveSymImport(const fs::path& fileName);
+	bool Validate(const ValidateState& state) override;
+	void Encode() const override;
+	void writeTempData(TempData& tempData) const override;
+	void writeSymData(SymbolData& symData) const override;
+private:
+	fs::path fileName;
+	std::vector<std::shared_ptr<Label>> labels;
+	std::vector<std::pair<uint64_t, std::string>> otherSymbols;
+};

Core/ELF/ElfRelocator.cpp

@@ -384,7 +384,7 @@ bool ElfRelocator::relocateFile(ElfRelocatorFile& file, int64_t& relocationAddre
 
 					relData.relocationBase = (unsigned int) label->getValue();
 					relData.targetSymbolType = label->isData() ? STT_OBJECT : STT_FUNC;
-					relData.targetSymbolInfo = label->getInfo();
+					relData.targetSymbolInfo = (int)label->getInfo();
 				} else {
 					relData.relocationBase = relocationOffsets[symSection]+relData.symbolAddress;
 				}

Core/SymbolTable.h

@@ -44,8 +44,8 @@ class Label
 	void setDefined(bool b) { defined = b; };
 	bool isData() { return data; };
 	void setIsData(bool b) { data = b; };
-	void setInfo(int inf) { info = inf; };
-	int getInfo() { return info; };
+	void setInfo(int64_t inf) { info = inf; };
+	int64_t getInfo() { return info; };
 	void setUpdateInfo(bool b) { updateInfo = b; };
 	bool getUpdateInfo() { return updateInfo; };
 	void setSection(int num) { section = num; }
@@ -59,7 +59,7 @@ class Label
 	bool defined = false;
 	bool data = false;
 	bool updateInfo = true;
-	int info = 0;
+	int64_t info = 0;
 	int section = 0;
 };
 

Main/Tests.cpp

@@ -75,6 +75,17 @@ bool TestRunner::executeTest(const fs::path& dir, const std::string& testName, s
 	fs::path oldDir = fs::current_path();
 	fs::current_path(dir);
 
+	// Remove previous output files
+	for (const auto& file : fs::directory_iterator(fs::current_path()))
+	{
+		std::string fileName = file.path().filename().u8string();
+		if (fileName.rfind("output",0) == 0)
+		{
+			// If filename starts with output, delete the file
+			fs::remove(file.path());
+		}
+	}
+
 	ArmipsArguments settings;
 	std::vector<std::string> errors;
 	int expectedRetVal = 0;
@@ -169,6 +180,28 @@ bool TestRunner::executeTest(const fs::path& dir, const std::string& testName, s
 		}
 	}
 
+	if (fs::exists("expected.sym"))
+	{
+		TextFile expectedFile;
+		TextFile actualFile;
+		expectedFile.open("expected.sym",TextFile::Read);
+		actualFile.open("output.sym",TextFile::Read);
+		if (actualFile.isOpen())
+		{
+			std::vector<std::string> expected = expectedFile.readAll();
+			std::vector<std::string> actual = actualFile.readAll();
+
+			if (expected != actual)
+			{
+				errorString += tfm::format("Symbols file does not match\n");
+				result = false;
+			}
+		} else {
+			errorString += tfm::format("Symbols file not produced\n");
+			result = false;
+		}
+	}
+
 	fs::current_path(oldDir);
 	return result;
 }

Parser/DirectivesParser.cpp

@@ -190,6 +190,16 @@ std::unique_ptr<CAssemblerCommand> parseDirectiveObjImport(Parser& parser, int f
 	return std::make_unique<DirectiveObjImport>(fileName.path());
 }
 
+std::unique_ptr<CAssemblerCommand> parseDirectiveSymImport(Parser& parser, int flags)
+{
+	Expression exp = parser.parseExpression();
+	StringLiteral fileName;
+	if (!exp.evaluateString(fileName,true))
+		return nullptr;
+
+	return std::make_unique<CDirectiveSymImport>(fileName.path());
+}
+
 std::unique_ptr<CAssemblerCommand> parseDirectiveConditional(Parser& parser, int flags)
 {
 	ConditionType type;
@@ -792,6 +802,7 @@ const DirectiveMap directives = {
 
 	{ ".importobj",       { &parseDirectiveObjImport,       0 } },
 	{ ".importlib",       { &parseDirectiveObjImport,       0 } },
+	{ ".importsym",       { &parseDirectiveSymImport,       0 } },
 
 	{ ".erroronwarning",  { &parseDirectiveErrorWarning,    0 } },
 	{ ".relativeinclude", { &parseDirectiveRelativeInclude, 0 } },

Readme.md

@@ -816,6 +816,14 @@ By specifying `.nocash on`, No$gba semantics will be enabled for data directives
 
 By specifying `.sym off`, any symbols (e.g. labels) defined after it will not be written to the symfile (if specified with the `-sym`/`-sym2` command line flag). This can be useful when using labels to define enum values that should not be interpreted as memory addresses. Writing to the symfile can be enabled again with `.sym on`. By default, this feature is on.
 
+### Import symfile
+
+```
+.importsym SymFile
+```
+
+Imports all symbols from the existing symfile specified by `SymFile`. For any symbols in `SymFile` not starting with `.`, `@` or `@@`, a global label will be created, which can be used like any other label. Regardless of prefix, all symbols and directives in `SymFile` are also written to the main output symfile for this run of armips, if symfile writing is enabled. This directive terminates the scope for local labels and `equ`s.
+
 ## 5.2 MIPS directives
 
 ### Load delay

Tests/.gitignore

@@ -1,3 +1,4 @@
 output.bin
+output.sym
 output.txt
 *.temp.txt

Tests/Area/Fill/Fill.asm

@@ -12,4 +12,9 @@ end:
 	.error "Area content not filled"
 .endif
 
+// Perfectly filled area
+.area 20h,2
+	.fill 20h
+.endarea
+
 .close

Tests/Area/Fill/commandLine.txt

@@ -0,0 +1 @@
+0 -sym2 output.sym Fill.asm

Tests/Area/Fill/expected.sym

@@ -0,0 +1,7 @@
+00000000 0
+00000000 .dbl:0004
+00000000 start
+00000004 .byt:001C
+00000020 .byt:0020
+00000020 end
+

Tests/Core/SymFileImport/SymFileImport.asm

@@ -0,0 +1,47 @@
+.nds
+.create "output.bin",0
+
+.if defined(.pool)
+	.error ".pool should not be accessible here"
+.endif
+.if defined(@@LocalLabel)
+	.error "@@LocalLabel should not be accessible here"
+.endif
+.if defined(@StaticLabel)
+	.error "@StaticLabel should not be accessible here"
+.endif
+
+.importsym "input.sym"
+
+.if defined(.pool)
+	.error ".pool should not be accessible here"
+.endif
+.if defined(@@LocalLabel)
+	.error "@@LocalLabel should not be accessible here"
+.endif
+.if defined(@StaticLabel)
+	.error "@StaticLabel should not be accessible here"
+.endif
+
+.dd	ValidLabel
+.dd	ValidLabelWithSpaces
+.dd	ValidLabelWithTabs
+.dd	ValidLabelWithSpacesAndTabs
+.dd	ValidLabelWithComment
+.dd	Function
+
+ldr	r0,=0xABCDABCD
+.pool
+
+.org 0x12345678
+NewlyAddedLabel1:
+
+.definelabel NewlyAddedLabel2, 0x87654321
+
+.skip 4
+@NewlyAddedStaticLabel:
+
+.skip 4
+@@NewlyAddedLocalLabel:
+
+.close

Tests/Core/SymFileImport/commandLine.txt

@@ -0,0 +1 @@
+0 -sym2 output.sym SymFileImport.asm