Skip to content

Extend OOVPA Tests #154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
RadWolfie wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Extend OOVPA Tests #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f9ba3e1
remove outdated function since revisions no longer has xref bind
RadWolfie Nov 8, 2021
82effe1
add SymbolDatabaseVerifyContext_VerifyXRefJmp test as debug
RadWolfie Nov 8, 2021
10f3db6
fix check for unset xref from symbol's group
RadWolfie Nov 8, 2021
31024e6
add check for duplicate symbols, we want to reduce redundant much as …
RadWolfie Nov 8, 2021
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
5 changes: 4 additions & 1 deletion include/xref/list_xref.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
// ******************************************************************
#pragma once

#include <stdint.h>

// ******************************************************************
// * XRefDatabase
// ******************************************************************
Expand Down Expand Up @@ -418,7 +420,8 @@ typedef enum _XRefDatabase {
#include "xonline.def"
#undef XREF_SYMBOL

XREF_COUNT // XREF_COUNT must always be last.
XREF_COUNT, // XREF_COUNT must always be last.
XREF_UNDETERMINED = (uint16_t)-1
// Also, if XREF_COUNT > sizeof(uint16), enlarge struct OOVPA.XRefSaveIndex (and Value somehow)
} XRefDatabase;

Expand Down
2 changes: 1 addition & 1 deletion src/lib/internal_functions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ static bool CompareOOVPAToAddress(iXbSymbolContext* pContext, OOVPA* Oovpa, memp
// Return if the given (XRef'erenced) is not set yet.
static inline bool internal_IsXRefUnset(uint16_t XRef)
{
return (XRef == (uint16_t)XREF_ADDR_UNDETERMINED) || (XRef == XREF_ADDR_DERIVE);
return (XRef == XREF_UNDETERMINED) || (XRef <= XREF_KT_COUNT);
Copy link
Member

@PatrickvL PatrickvL Nov 10, 2021

Choose a reason for hiding this comment

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

Why is the second test changed?

Copy link
Member Author

@RadWolfie RadWolfie Nov 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

internal_IsXRefUnset is intended to check if xref is set for database's symbols that are grouped together. Plus cannot have kernel thunk references from the database's symbols. So, that's consider as not set. There's two difference between internal_IsXRefUnset and internal_IsXRefAddrUnset. internal_IsXRefAddrUnset is designed for check xref_database's array that is cached.

}

// Return if the given (XRef'erenced) address is not set yet.
Expand Down
47 changes: 34 additions & 13 deletions src/lib/internal_tests.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,12 +225,34 @@ static unsigned int SymbolDatabaseVerifyContext_VerifyOOVPA(SymbolDatabaseVerify
return error_count;
}

// This function will report as verbose purpose only.
static unsigned int SymbolDatabaseVerifyContext_VerifyXRefJmp(SymbolDatabaseVerifyContext* context, const OOVPATable* table, uint32_t symbol_index, uint32_t revision_index)
{
LOOVPA* loovpa = (LOOVPA*)table[symbol_index].revisions[revision_index].Oovpa;
// Check if signature has both xref and byte registered.
if (loovpa->Header.Count == 2 &&
loovpa->Header.XRefCount == 1) {
// XRefs are always stored at the top whilst byte pairs are after.
// Since we only check for one xref and one byte. We can safely use hardcode offset to 1.
// Plus we don't need to check the OOVPA's offset value is 0 or not.
if (loovpa->Lovp[1].value == 0xE9) {
output_message_format(&context->output, XB_OUTPUT_MESSAGE_DEBUG, "%s has one xref and one jump instruction", table[symbol_index].szFuncName);
}
}
return 0;
}

static unsigned int SymbolDatabaseVerifyContext_VerifyXRefSymbolVsRevision(SymbolDatabaseVerifyContext* context, const OOVPATable* table, uint32_t symbol_index, uint32_t revision_index)
static unsigned int SymbolDatabaseVerifyContext_VerifySymbolDuplicate(SymbolDatabaseVerifyContext* context, const OOVPATable* table, uint32_t symbol_index)
{
unsigned int error_count = 0;
if (!internal_IsXRefUnset(table[symbol_index].xref)) {
LOOVPA* loovpa = (LOOVPA*)table[symbol_index].revisions[revision_index].Oovpa;

if (&context->main.data->SymbolsTable[context->main.symbol_index] != &context->against.data->SymbolsTable[context->against.symbol_index] &&
context->main.revision_index == 0 && context->against.revision_index == 0) {

if (strcmp(table[symbol_index].szFuncName, context->main.data->SymbolsTable[context->main.symbol_index].szFuncName) == 0) {
SymbolDatabaseVerifyContext_OOVPAError(context, "Duplicate symbol detected");
error_count++;
}
}
return error_count;
}
Expand All @@ -242,11 +264,19 @@ static unsigned int SymbolDatabaseVerifyContext_VerifyEntry(SymbolDatabaseVerify
context->main.symbol_index = symbol_index;
context->main.revision_index = revision_index;

error_count += SymbolDatabaseVerifyContext_VerifyXRefSymbolVsRevision(context, table, symbol_index, revision_index);
error_count += SymbolDatabaseVerifyContext_VerifyXRefJmp(context, table, symbol_index, revision_index);

// For safety check purpose
if (internal_IsXRefUnset(table[symbol_index].xref)) {
output_message_format(&context->output, XB_OUTPUT_MESSAGE_ERROR, "%s cannot have unset xref.", table[symbol_index].szFuncName);
error_count++;
}
}
else {
context->against.symbol_index = symbol_index;
context->against.revision_index = revision_index;

error_count += SymbolDatabaseVerifyContext_VerifySymbolDuplicate(context, table, symbol_index);
}


Expand All @@ -269,15 +299,6 @@ static unsigned int SymbolDatabaseVerifyContext_VerifyDatabase(SymbolDatabaseVer

// Verify each entry in data's symbol table.
for (uint32_t s = 0; s < data->SymbolsTableCount; s++) {
// We only need to check from main, not against.
if (context->against.data == NULL) {
// For safety check purpose
if (internal_IsXRefUnset(data->SymbolsTable[s].xref)) {
output_message_format(&context->output, XB_OUTPUT_MESSAGE_ERROR, "%s cannot have unset xref.", data->SymbolsTable[s].szFuncName);
error_count++;
}
}

// Check each revision entry in a symbol.
for (uint32_t r = 0; r < data->SymbolsTable[s].count; r++) {
error_count += SymbolDatabaseVerifyContext_VerifyEntry(context, data->SymbolsTable, s, r);
Expand Down