Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 159 additions & 0 deletions Commands/CDirectiveFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works for no$gba .sym files (or -sym output), but not for files generated by -sym2 (if .func was used, at least). Anything after a comma should also be removed from the identifier.

if (name.find('.') != 0 && Global.symbolTable.isGlobalSymbol(identifier))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Symbols can contain periods (see SymbolTable::isValidSymbolName) - but they can't contain colons, which .sym uses inside of its directives. So perhaps:

if (name.find(':') == std::string::npos && Global.symbolTable.isValidSymbolName(identifier) && Global.symbolTable.isGlobalSymbol(identifier))

This would make the nullptr check below redundant, however, so the check against isValidSymbolName could be used there instead too.

Copy link
Contributor Author

@Prof9 Prof9 May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should still prohibit symbols starting with a period. Not all symfile directives use colons; e.g. .arm, .thumb and .pool do not, and I don't think it makes sense to create a label named .pool. It's also easier to understand.

{
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);
}
}
14 changes: 14 additions & 0 deletions Commands/CDirectiveFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
2 changes: 1 addition & 1 deletion Core/ELF/ElfRelocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions Core/SymbolTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -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;
};

Expand Down
33 changes: 33 additions & 0 deletions Main/Tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
11 changes: 11 additions & 0 deletions Parser/DirectivesParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -792,6 +802,7 @@ const DirectiveMap directives = {

{ ".importobj", { &parseDirectiveObjImport, 0 } },
{ ".importlib", { &parseDirectiveObjImport, 0 } },
{ ".importsym", { &parseDirectiveSymImport, 0 } },

{ ".erroronwarning", { &parseDirectiveErrorWarning, 0 } },
{ ".relativeinclude", { &parseDirectiveRelativeInclude, 0 } },
Expand Down
8 changes: 8 additions & 0 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions Tests/.gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
output.bin
output.sym
output.txt
*.temp.txt
47 changes: 47 additions & 0 deletions Tests/Core/SymFileImport/SymFileImport.asm
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions Tests/Core/SymFileImport/commandLine.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0 -sym2 output.sym SymFileImport.asm
Binary file added Tests/Core/SymFileImport/expected.bin
Binary file not shown.
25 changes: 25 additions & 0 deletions Tests/Core/SymFileImport/expected.sym
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
00000000 0
00000000 .dbl:0008
00000008 .dbl:0008
00000010 .dbl:0008
00000018 .dbl:0008
00000020 .dbl:0008
00000028 .dbl:0008
00000034 .dbl:0004
00000034 .pool
004488CC Function,CC884400
01100110 ValidLabel
11223344 @StaticLabel
12345678 NewlyAddedLabel1
1234567C @NewlyAddedStaticLabel
12345680 @@NewlyAddedLocalLabel
23322332 ValidLabelWithSpaces
45544554 ValidLabelWithTabs
55667788 @StaticLabel
67766776 ValidLabelWithSpacesAndTabs
87654321 NewlyAddedLabel2
89988998 ValidLabelWithComment
99AABBCC @@LocalLabel
DDEEFF00 @@LocalLabel
FEDCBA98 .pool

17 changes: 17 additions & 0 deletions Tests/Core/SymFileImport/input.sym
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
; Line with comment

01100110 ValidLabel
23322332 ValidLabelWithSpaces
45544554 ValidLabelWithTabs
67766776 ValidLabelWithSpacesAndTabs
89988998 ValidLabelWithComment ; No$gba does allow this, if there is whitespace before the ;

11223344 @StaticLabel
55667788 @StaticLabel
99AABBCC @@LocalLabel
DDEEFF00 @@LocalLabel

004488CC Function , CC884400

FEDCBA98 .pool