thirdparty/ppsspp
2
0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Avoid returning points from the symbol map.

Browse source 
Now that it uses a lookup, this is even more dangerous.  But, the maps
could be reordered while it's trying to print the pointer and cause that
data to become invalid.

This should be safe from race conditions.
This commit is contained in:
Unknown W. Brackets 2014-01-25 21:40:23 -08:00
parent b1af4f4911
commit 76afb2a8d5
11 changed files with 69 additions and 50 deletions
Download patch file Download diff file Expand all files Collapse all files

2
Core/Debugger/Breakpoints.cpp
View file

@ -41,7 +41,7 @@ void MemCheck::Action(u32 addr, bool write, int size, u32 pc)
++numHits;
if (result & MEMCHECK_LOG)
NOTICE_LOG(MEMMAP, "CHK %s%i at %08x (%s), PC=%08x (%s)", write ? "Write" : "Read", size * 8, addr, symbolMap.GetDescription(addr), pc, symbolMap.GetDescription(pc));
NOTICE_LOG(MEMMAP, "CHK %s%i at %08x (%s), PC=%08x (%s)", write ? "Write" : "Read", size * 8, addr, symbolMap.GetDescription(addr).c_str(), pc, symbolMap.GetDescription(pc));
if (result & MEMCHECK_BREAK)
{
Core_EnableStepping(true);

3
Core/Debugger/DebugInterface.h
View file

@ -16,6 +16,7 @@
// https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
#pragma once
#include <string>
#include <cstdio>
#include "Common/CommonTypes.h"
#include "native/math/expression_parser.h"
@ -45,7 +46,7 @@ public:
virtual void step() {}
virtual void runToBreakpoint() {}
virtual int getColor(unsigned int address){return 0xFFFFFFFF;}
virtual const char *getDescription(unsigned int address) {return "";}
virtual std::string getDescription(unsigned int address) {return "";}
virtual bool initExpression(const char* exp, PostfixExpression& dest) { return false; };
virtual bool parseExpression(PostfixExpression& exp, u32& dest) { return false; };

28
Core/Debugger/DisassemblyManager.cpp
View file

@ -15,7 +15,7 @@
// Official git repository and contact information can be found at
// https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
#include <string>
#include <algorithm>
#include <map>
@ -70,10 +70,10 @@ void parseDisasm(const char* disasm, char* opcode, char* arguments, bool insertS
u32 branchTarget;
sscanf(disasm+3,"%08x",&branchTarget);
const char* addressSymbol = symbolMap.GetLabelName(branchTarget);
if (addressSymbol != NULL && insertSymbols)
const std::string addressSymbol = symbolMap.GetLabelString(branchTarget);
if (!addressSymbol.empty() && insertSymbols)
{
arguments += sprintf(arguments,"%s",addressSymbol);
arguments += sprintf(arguments,"%s",addressSymbol.c_str());
} else {
arguments += sprintf(arguments,"0x%08X",branchTarget);
}
@ -740,16 +740,16 @@ bool DisassemblyMacro::disassemble(u32 address, DisassemblyLineInfo& dest, bool
dest.type = DISTYPE_MACRO;
dest.info = MIPSAnalyst::GetOpcodeInfo(DisassemblyManager::getCpu(),address);
const char* addressSymbol;
std::string addressSymbol;
switch (type)
{
case MACRO_LI:
dest.name = name;
addressSymbol = symbolMap.GetLabelName(immediate);
if (addressSymbol != NULL && insertSymbols)
addressSymbol = symbolMap.GetLabelString(immediate);
if (!addressSymbol.empty() && insertSymbols)
{
sprintf(buffer,"%s,%s",DisassemblyManager::getCpu()->GetRegName(0,rt),addressSymbol);
sprintf(buffer,"%s,%s",DisassemblyManager::getCpu()->GetRegName(0,rt),addressSymbol.c_str());
} else {
sprintf(buffer,"%s,0x%08X",DisassemblyManager::getCpu()->GetRegName(0,rt),immediate);
}
@ -762,10 +762,10 @@ bool DisassemblyMacro::disassemble(u32 address, DisassemblyLineInfo& dest, bool
case MACRO_MEMORYIMM:
dest.name = name;
addressSymbol = symbolMap.GetLabelName(immediate);
if (addressSymbol != NULL && insertSymbols)
addressSymbol = symbolMap.GetLabelString(immediate);
if (!addressSymbol.empty() && insertSymbols)
{
sprintf(buffer,"%s,%s",DisassemblyManager::getCpu()->GetRegName(0,rt),addressSymbol);
sprintf(buffer,"%s,%s",DisassemblyManager::getCpu()->GetRegName(0,rt),addressSymbol.c_str());
} else {
sprintf(buffer,"%s,0x%08X",DisassemblyManager::getCpu()->GetRegName(0,rt),immediate);
}
@ -956,9 +956,9 @@ void DisassemblyData::createLines()
case DATATYPE_WORD:
{
value = Memory::Read_U32(pos);
const char* label = symbolMap.GetLabelName(value);
if (label != NULL)
sprintf(buffer,"%s",label);
const std::string label = symbolMap.GetLabelString(value);
if (!label.empty())
sprintf(buffer,"%s",label.c_str());
else
sprintf(buffer,"0x%08X",value);
pos += 4;

13
Core/Debugger/SymbolMap.cpp
View file

@ -333,9 +333,7 @@ u32 SymbolMap::GetNextSymbolAddress(u32 address, SymbolType symmask) {
return dataAddress;
}
static char descriptionTemp[256];
const char *SymbolMap::GetDescription(unsigned int address) const {
std::string SymbolMap::GetDescription(unsigned int address) const {
lock_guard guard(lock_);
const char* labelName = NULL;
@ -351,6 +349,7 @@ const char *SymbolMap::GetDescription(unsigned int address) const {
if (labelName != NULL)
return labelName;
char descriptionTemp[256];
sprintf(descriptionTemp, "(%08x)", address);
return descriptionTemp;
}
@ -742,6 +741,14 @@ const char *SymbolMap::GetLabelNameRel(u32 relAddress, int moduleIndex) const {
return it->second.name;
}
std::string SymbolMap::GetLabelString(u32 address) const {
lock_guard guard(lock_);
const char *label = GetLabelName(address);
if (label == NULL)
return "";
return label;
}
bool SymbolMap::GetLabelValue(const char* name, u32& dest) {
lock_guard guard(lock_);
for (auto it = activeLabels.begin(); it != activeLabels.end(); it++) {

7
Core/Debugger/SymbolMap.h
View file

@ -67,7 +67,7 @@ public:
SymbolType GetSymbolType(u32 address) const;
bool GetSymbolInfo(SymbolInfo *info, u32 address, SymbolType symmask = ST_FUNCTION) const;
u32 GetNextSymbolAddress(u32 address, SymbolType symmask);
const char *GetDescription(unsigned int address) const;
std::string GetDescription(unsigned int address) const;
std::vector<SymbolEntry> GetAllSymbols(SymbolType symmask);
#ifdef _WIN32
@ -89,9 +89,8 @@ public:
bool RemoveFunction(u32 startAddress, bool removeName);
void AddLabel(const char* name, u32 address, int moduleIndex = -1);
std::string GetLabelString(u32 address) const;
void SetLabelName(const char* name, u32 address);
const char *GetLabelName(u32 address) const;
const char *GetLabelNameRel(u32 relAddress, int moduleIndex) const;
bool GetLabelValue(const char* name, u32& dest);
void AddData(u32 address, u32 size, DataType type, int moduleIndex = -1);
@ -104,6 +103,8 @@ public:
private:
void AssignFunctionIndices();
void UpdateActiveSymbols();
const char *GetLabelName(u32 address) const;
const char *GetLabelNameRel(u32 relAddress, int moduleIndex) const;
struct FunctionEntry {
u32 start;

17
Core/MIPS/MIPSAnalyst.cpp
View file

@ -264,6 +264,15 @@ skip:
return !strncmp(name, "z_un_", strlen("z_un_")) || !strncmp(name, "u_un_", strlen("u_un_"));
}
static bool IsDefaultFunction(const std::string &name) {
if (name.empty()) {
// Must be I guess?
return true;
}
return IsDefaultFunction(name.c_str());
}
static u32 ScanAheadForJumpback(u32 fromAddr, u32 knownStart, u32 knownEnd) {
static const u32 MAX_AHEAD_SCAN = 0x1000;
// Maybe a bit high... just to make sure we don't get confused by recursive tail recursion.
@ -516,13 +525,13 @@ skip:
continue;
}
// Functions with default names aren't very interesting either.
const char *name = symbolMap.GetLabelName(f.start);
const std::string name = symbolMap.GetLabelString(f.start);
if (IsDefaultFunction(name)) {
continue;
}
HashMapFunc mf = { "", f.hash, f.size };
strncpy(mf.name, name, sizeof(mf.name) - 1);
strncpy(mf.name, name.c_str(), sizeof(mf.name) - 1);
hashMap.insert(mf);
}
}
@ -584,10 +593,10 @@ skip:
if (f.hash == mf->hash && f.size == mf->size) {
strncpy(f.name, mf->name, sizeof(mf->name) - 1);
const char *existingLabel = symbolMap.GetLabelName(f.start);
std::string existingLabel = symbolMap.GetLabelString(f.start);
char defaultLabel[256];
// If it was renamed, keep it. Only change the name if it's still the default.
if (existingLabel == NULL || !strcmp(existingLabel, DefaultFunctionName(defaultLabel, f.start))) {
if (existingLabel.empty() || !strcmp(existingLabel.c_str(), DefaultFunctionName(defaultLabel, f.start))) {
symbolMap.SetLabelName(mf->name, f.start);
}
}

2
Core/MIPS/MIPSDebugInterface.cpp
View file

@ -223,7 +223,7 @@ int MIPSDebugInterface::getColor(unsigned int address)
if (n==-1) return 0xFFFFFF;
return colors[n%6];
}
const char *MIPSDebugInterface::getDescription(unsigned int address)
std::string MIPSDebugInterface::getDescription(unsigned int address)
{
return symbolMap.GetDescription(address);
}

3
Core/MIPS/MIPSDebugInterface.h
View file

@ -17,6 +17,7 @@
#pragma once
#include <string>
#include "MIPS.h"
#include "Core/Debugger/DebugInterface.h"
@ -39,7 +40,7 @@ public:
virtual void step() {}
virtual void runToBreakpoint();
virtual int getColor(unsigned int address);
virtual const char *getDescription(unsigned int address);
virtual std::string getDescription(unsigned int address);
virtual bool initExpression(const char* exp, PostfixExpression& dest);
virtual bool parseExpression(PostfixExpression& exp, u32& dest);

6
Core/MIPS/x86/Jit.cpp
View file

@ -374,9 +374,9 @@ bool Jit::DescribeCodePtr(const u8 *ptr, std::string &name)
else if (jitAddr != (u32)-1)
{
char temp[1024];
const char *label = symbolMap.GetDescription(jitAddr);
if (label != NULL)
snprintf(temp, sizeof(temp), "%08x_%s", jitAddr, label);
const std::string label = symbolMap.GetDescription(jitAddr);
if (!label.empty())
snprintf(temp, sizeof(temp), "%08x_%s", jitAddr, label.c_str());
else
snprintf(temp, sizeof(temp), "%08x", jitAddr);
name = temp;

30
Windows/Debugger/CtrlDisAsmView.cpp
View file

@ -205,8 +205,8 @@ bool CtrlDisAsmView::getDisasmAddressText(u32 address, char* dest, bool abbrevia
{
if (displaySymbols)
{
const char* addressSymbol = symbolMap.GetLabelName(address);
if (addressSymbol != NULL)
const std::string addressSymbol = symbolMap.GetLabelString(address);
if (!addressSymbol.empty())
{
for (int k = 0; addressSymbol[k] != 0; k++)
{
@ -915,7 +915,7 @@ void CtrlDisAsmView::onMouseUp(WPARAM wParam, LPARAM lParam, int button)
{
char name[256];
std::string newname;
strncpy_s(name, symbolMap.GetLabelName(funcBegin),_TRUNCATE);
strncpy_s(name, symbolMap.GetLabelString(funcBegin).c_str(),_TRUNCATE);
if (InputBox_GetString(MainWindow::GetHInstance(), MainWindow::GetHWND(), L"New function name", name, newname)) {
symbolMap.SetLabelName(newname.c_str(),funcBegin);
u32 funcSize = symbolMap.GetFunctionSize(curAddress);
@ -1048,10 +1048,10 @@ void CtrlDisAsmView::updateStatusBarText()
// TODO: Could also be a float...
{
u32 data = Memory::Read_U32(line.info.dataAddress);
const char* addressSymbol = symbolMap.GetLabelName(data);
if (addressSymbol)
const std::string addressSymbol = symbolMap.GetLabelString(data);
if (!addressSymbol.empty())
{
sprintf(text,"[%08X] = %s (%08X)",line.info.dataAddress,addressSymbol,data);
sprintf(text,"[%08X] = %s (%08X)",line.info.dataAddress,addressSymbol.c_str(),data);
} else {
sprintf(text,"[%08X] = %08X",line.info.dataAddress,data);
}
@ -1066,12 +1066,12 @@ void CtrlDisAsmView::updateStatusBarText()
if (line.info.isBranch)
{
const char* addressSymbol = symbolMap.GetLabelName(line.info.branchTarget);
if (addressSymbol == NULL)
const std::string addressSymbol = symbolMap.GetLabelString(line.info.branchTarget);
if (addressSymbol.empty())
{
sprintf(text,"%08X",line.info.branchTarget);
} else {
sprintf(text,"%08X = %s",line.info.branchTarget,addressSymbol);
sprintf(text,"%08X = %s",line.info.branchTarget,addressSymbol.c_str());
}
}
} else if (line.type == DISTYPE_DATA)
@ -1081,14 +1081,14 @@ void CtrlDisAsmView::updateStatusBarText()
start = curAddress;
u32 diff = curAddress-start;
const char* label = symbolMap.GetLabelName(start);
const std::string label = symbolMap.GetLabelString(start);
if (label != NULL)
if (!label.empty())
{
if (diff != 0)
sprintf(text,"%08X (%s) + %08X",start,label,diff);
sprintf(text,"%08X (%s) + %08X",start,label.c_str(),diff);
else
sprintf(text,"%08X (%s)",start,label);
sprintf(text,"%08X (%s)",start,label.c_str());
} else {
if (diff != 0)
sprintf(text,"%08X + %08X",start,diff);
@ -1204,7 +1204,7 @@ std::string CtrlDisAsmView::disassembleRange(u32 start, u32 size)
{
MIPSAnalyst::MipsOpcodeInfo info = MIPSAnalyst::GetOpcodeInfo(debugger,start+i);
if (info.isBranch && symbolMap.GetLabelName(info.branchTarget) == NULL)
if (info.isBranch && symbolMap.GetLabelString(info.branchTarget).empty())
{
if (branchAddresses.find(info.branchTarget) == branchAddresses.end())
{
@ -1236,7 +1236,7 @@ std::string CtrlDisAsmView::disassembleRange(u32 start, u32 size)
}
if (line.info.isBranch && !line.info.isBranchToRegister
&& symbolMap.GetLabelName(line.info.branchTarget) == NULL
&& symbolMap.GetLabelString(line.info.branchTarget).empty()
&& branchAddresses.find(line.info.branchTarget) != branchAddresses.end())
{
sprintf(buffer,"pos_%08X",line.info.branchTarget);

8
Windows/Debugger/Debugger_Lists.cpp
View file

@ -463,8 +463,8 @@ void CtrlBreakpointList::GetColumnText(wchar_t* dest, int row, int col)
else
wsprintf(dest,L"0x%08X",mc.end-mc.start);
} else {
const char* sym = symbolMap.GetLabelName(displayedBreakPoints_[index].addr);
if (sym != NULL)
const std::string sym = symbolMap.GetLabelString(displayedBreakPoints_[index].addr);
if (!sym.empty())
{
std::wstring s = ConvertUTF8ToWString(sym);
wcscpy(dest,s.c_str());
@ -635,8 +635,8 @@ void CtrlStackTraceView::GetColumnText(wchar_t* dest, int row, int col)
break;
case SF_ENTRYNAME:
{
const char* sym = symbolMap.GetLabelName(frames[row].entry);
if (sym != NULL) {
const std::string sym = symbolMap.GetLabelString(frames[row].entry);
if (!sym.empty()) {
wcscpy(dest, ConvertUTF8ToWString(sym).c_str());
} else {
wcscpy(dest,L"-");