From 9772201345ab47cc820fd6c08247c133605f8b84 Mon Sep 17 00:00:00 2001 From: RSDuck Date: Fri, 4 Sep 2020 20:37:14 +0200 Subject: remove some UB - savestates used to read a four bytes from a single byte value - a few unassigned variables - some other things - also make the ROR macro an inline function --- src/ARMJIT_x64/ARMJIT_ALU.cpp | 6 +++--- src/ARMJIT_x64/ARMJIT_Compiler.cpp | 2 +- src/ARMJIT_x64/ARMJIT_LoadStore.cpp | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) (limited to 'src/ARMJIT_x64') diff --git a/src/ARMJIT_x64/ARMJIT_ALU.cpp b/src/ARMJIT_x64/ARMJIT_ALU.cpp index 57a38c4..24d22ed 100644 --- a/src/ARMJIT_x64/ARMJIT_ALU.cpp +++ b/src/ARMJIT_x64/ARMJIT_ALU.cpp @@ -110,7 +110,7 @@ OpArg Compiler::A_Comp_GetALUOp2(bool S, bool& carryUsed) Comp_AddCycles_C(); u32 shift = (CurInstr.Instr >> 7) & 0x1E; - u32 imm = ROR(CurInstr.Instr & 0xFF, shift); + u32 imm = ::ROR(CurInstr.Instr & 0xFF, shift); carryUsed = false; if (S && shift) @@ -493,7 +493,7 @@ OpArg Compiler::Comp_RegShiftReg(int op, Gen::OpArg rs, Gen::OpArg rm, bool S, b { if (S) BT(32, R(RSCRATCH), Imm8(31)); - ROR_(32, R(RSCRATCH), R(ECX)); + ROR(32, R(RSCRATCH), R(ECX)); if (S) SETcc(CC_C, R(RSCRATCH2)); } @@ -555,7 +555,7 @@ OpArg Compiler::Comp_RegShiftImm(int op, int amount, OpArg rm, bool S, bool& car case 3: // ROR MOV(32, R(RSCRATCH), rm); if (amount > 0) - ROR_(32, R(RSCRATCH), Imm8(amount)); + ROR(32, R(RSCRATCH), Imm8(amount)); else { BT(32, R(RCPSR), Imm8(29)); diff --git a/src/ARMJIT_x64/ARMJIT_Compiler.cpp b/src/ARMJIT_x64/ARMJIT_Compiler.cpp index 1fdbaf8..c6419c9 100644 --- a/src/ARMJIT_x64/ARMJIT_Compiler.cpp +++ b/src/ARMJIT_x64/ARMJIT_Compiler.cpp @@ -106,7 +106,7 @@ void Compiler::A_Comp_MSR() Comp_AddCycles_C(); OpArg val = CurInstr.Instr & (1 << 25) - ? Imm32(ROR((CurInstr.Instr & 0xFF), ((CurInstr.Instr >> 7) & 0x1E))) + ? Imm32(::ROR((CurInstr.Instr & 0xFF), ((CurInstr.Instr >> 7) & 0x1E))) : MapReg(CurInstr.A_Reg(0)); u32 mask = 0; diff --git a/src/ARMJIT_x64/ARMJIT_LoadStore.cpp b/src/ARMJIT_x64/ARMJIT_LoadStore.cpp index 57d98cc..1be6608 100644 --- a/src/ARMJIT_x64/ARMJIT_LoadStore.cpp +++ b/src/ARMJIT_x64/ARMJIT_LoadStore.cpp @@ -73,7 +73,7 @@ bool Compiler::Comp_MemLoadLiteral(int size, bool signExtend, int rd, u32 addr) if (size == 32) { CurCPU->DataRead32(addr & ~0x3, &val); - val = ROR(val, (addr & 0x3) << 3); + val = ::ROR(val, (addr & 0x3) << 3); } else if (size == 16) { @@ -225,13 +225,13 @@ void Compiler::Comp_MemAccess(int rd, int rn, const Op2& op2, int size, int flag if (addrIsStatic) { if (staticAddress & 0x3) - ROR_(32, rdMapped, Imm8((staticAddress & 0x3) * 8)); + ROR(32, rdMapped, Imm8((staticAddress & 0x3) * 8)); } else { AND(32, R(RSCRATCH3), Imm8(0x3)); SHL(32, R(RSCRATCH3), Imm8(3)); - ROR_(32, rdMapped, R(RSCRATCH3)); + ROR(32, rdMapped, R(RSCRATCH3)); } } } @@ -270,7 +270,7 @@ void Compiler::Comp_MemAccess(int rd, int rn, const Op2& op2, int size, int flag { MOV(32, rdMapped, R(RSCRATCH)); if (staticAddress & 0x3) - ROR_(32, rdMapped, Imm8((staticAddress & 0x3) * 8)); + ROR(32, rdMapped, Imm8((staticAddress & 0x3) * 8)); } else { -- cgit v1.2.3 From 0d845c9e69e19046f351ce396da8eec150243387 Mon Sep 17 00:00:00 2001 From: Valeri Date: Thu, 1 Oct 2020 14:44:09 +0300 Subject: Random minor fixes (#757) * Fix incorrect/questionable assert() usage Originally reported by https://lgtm.com/projects/g/Arisotura/melonDS/?mode=tree&ruleFocus=2159000700, but also includes a bunch of other fixes. * Fix some `printf` warnings Rule https://lgtm.com/projects/g/Arisotura/melonDS/?mode=tree&ruleFocus=2160310550 * Remove useless check It is never passed thanks to `if (num_in < 1) {...; return}` before Rule https://lgtm.com/projects/g/Arisotura/melonDS/?mode=tree&ruleFocus=2154840804 * Add missing header guard, rename other to avoid conflicts Rule https://lgtm.com/projects/g/Arisotura/melonDS/?mode=tree&ruleFocus=2163210746 * Make DSi_SDDevice destructor virtual Rule https://lgtm.com/projects/g/Arisotura/melonDS/?mode=tree&ruleFocus=2158670642 * Use thread-safe localtime_r, assign `time` result directly Rule https://lgtm.com/projects/g/Arisotura/melonDS/?mode=tree&ruleFocus=2154840805 * Fix MinGW build It needs _POSIX_THREAD_SAFE_FUNCTIONS to export `localtime_r` --- src/ARMJIT_A64/ARMJIT_Compiler.h | 6 ++--- src/ARMJIT_A64/ARMJIT_LoadStore.cpp | 4 +-- src/ARMJIT_Compiler.h | 7 +++++- src/ARMJIT_RegisterCache.h | 3 ++- src/ARMJIT_x64/ARMJIT_ALU.cpp | 7 +++--- src/ARMJIT_x64/ARMJIT_Compiler.h | 6 ++--- src/ARMJIT_x64/ARMJIT_LoadStore.cpp | 4 +-- src/Config.cpp | 2 +- src/DSi_SD.h | 2 +- src/RTC.cpp | 41 ++++++++++++++++--------------- src/frontend/qt_sdl/InputConfigDialog.cpp | 2 +- src/frontend/qt_sdl/main.cpp | 1 - 12 files changed, 46 insertions(+), 39 deletions(-) (limited to 'src/ARMJIT_x64') diff --git a/src/ARMJIT_A64/ARMJIT_Compiler.h b/src/ARMJIT_A64/ARMJIT_Compiler.h index 0e7d54c..af7497a 100644 --- a/src/ARMJIT_A64/ARMJIT_Compiler.h +++ b/src/ARMJIT_A64/ARMJIT_Compiler.h @@ -1,5 +1,5 @@ -#ifndef ARMJIT_COMPILER_H -#define ARMJIT_COMPILER_H +#ifndef ARMJIT_A64_COMPILER_H +#define ARMJIT_A64_COMPILER_H #include "../ARM.h" #include "../ARMJIT.h" @@ -266,4 +266,4 @@ public: } -#endif \ No newline at end of file +#endif diff --git a/src/ARMJIT_A64/ARMJIT_LoadStore.cpp b/src/ARMJIT_A64/ARMJIT_LoadStore.cpp index 14aa847..86e257a 100644 --- a/src/ARMJIT_A64/ARMJIT_LoadStore.cpp +++ b/src/ARMJIT_A64/ARMJIT_LoadStore.cpp @@ -42,7 +42,7 @@ s64 Compiler::RewriteMemAccess(u64 pc) return patch.PatchOffset; } printf("this is a JIT bug! %08x\n", __builtin_bswap32(*(u32*)pc)); - assert(false); + abort(); } bool Compiler::Comp_MemLoadLiteral(int size, bool signExtend, int rd, u32 addr) @@ -807,4 +807,4 @@ void Compiler::T_Comp_LDMIA_STMIA() } } -} \ No newline at end of file +} diff --git a/src/ARMJIT_Compiler.h b/src/ARMJIT_Compiler.h index 513c103..25a0210 100644 --- a/src/ARMJIT_Compiler.h +++ b/src/ARMJIT_Compiler.h @@ -1,3 +1,6 @@ +#ifndef ARMJIT_COMPILER_H +#define ARMJIT_COMPILER_H + #if defined(__x86_64__) #include "ARMJIT_x64/ARMJIT_Compiler.h" #elif defined(__aarch64__) @@ -9,4 +12,6 @@ namespace ARMJIT { extern Compiler* JITCompiler; -} \ No newline at end of file +} + +#endif diff --git a/src/ARMJIT_RegisterCache.h b/src/ARMJIT_RegisterCache.h index feb2d35..b11255e 100644 --- a/src/ARMJIT_RegisterCache.h +++ b/src/ARMJIT_RegisterCache.h @@ -61,7 +61,8 @@ public: } } - assert("Welp!"); + printf("this is a JIT bug! LoadRegister failed\n"); + abort(); } void PutLiteral(int reg, u32 val) diff --git a/src/ARMJIT_x64/ARMJIT_ALU.cpp b/src/ARMJIT_x64/ARMJIT_ALU.cpp index 24d22ed..511b3b1 100644 --- a/src/ARMJIT_x64/ARMJIT_ALU.cpp +++ b/src/ARMJIT_x64/ARMJIT_ALU.cpp @@ -209,7 +209,8 @@ void Compiler::A_Comp_Arith() Comp_ArithTriOp(&Compiler::AND, rd, rn, op2, carryUsed, sFlag|opSymmetric|opInvertOp2); break; default: - assert("unimplemented"); + printf("this is a JIT bug! %04x\n", op); + abort(); } if (CurInstr.A_Reg(12) == 15) @@ -566,7 +567,7 @@ OpArg Compiler::Comp_RegShiftImm(int op, int amount, OpArg rm, bool S, bool& car return R(RSCRATCH); } - assert(false); + abort(); } void Compiler::T_Comp_ShiftImm() @@ -779,4 +780,4 @@ void Compiler::T_Comp_RelAddr() MOV(32, rd, Imm32((R15 & ~2) + offset)); } -} \ No newline at end of file +} diff --git a/src/ARMJIT_x64/ARMJIT_Compiler.h b/src/ARMJIT_x64/ARMJIT_Compiler.h index 9a64d09..3e900c3 100644 --- a/src/ARMJIT_x64/ARMJIT_Compiler.h +++ b/src/ARMJIT_x64/ARMJIT_Compiler.h @@ -1,5 +1,5 @@ -#ifndef ARMJIT_COMPILER_H -#define ARMJIT_COMPILER_H +#ifndef ARMJIT_X64_COMPILER_H +#define ARMJIT_X64_COMPILER_H #include "../dolphin/x64Emitter.h" @@ -252,4 +252,4 @@ public: } -#endif \ No newline at end of file +#endif diff --git a/src/ARMJIT_x64/ARMJIT_LoadStore.cpp b/src/ARMJIT_x64/ARMJIT_LoadStore.cpp index 1be6608..8b4e8fe 100644 --- a/src/ARMJIT_x64/ARMJIT_LoadStore.cpp +++ b/src/ARMJIT_x64/ARMJIT_LoadStore.cpp @@ -39,7 +39,7 @@ s32 Compiler::RewriteMemAccess(u64 pc) return patch.Offset; } - printf("this is a JIT bug %x\n", pc); + printf("this is a JIT bug %llx\n", pc); abort(); } @@ -819,4 +819,4 @@ void Compiler::T_Comp_LDMIA_STMIA() ADD(32, rb, Imm8(offset)); } -} \ No newline at end of file +} diff --git a/src/Config.cpp b/src/Config.cpp index 2c5fd2c..8bda9ae 100644 --- a/src/Config.cpp +++ b/src/Config.cpp @@ -169,7 +169,7 @@ void Save() if (entry->Type == 0) fprintf(f, "%s=%d\n", entry->Name, *(int*)entry->Value); else - fprintf(f, "%s=%s\n", entry->Name, entry->Value); + fprintf(f, "%s=%s\n", entry->Name, (char*)entry->Value); entry++; } diff --git a/src/DSi_SD.h b/src/DSi_SD.h index 30da6c7..43f5a98 100644 --- a/src/DSi_SD.h +++ b/src/DSi_SD.h @@ -103,7 +103,7 @@ class DSi_SDDevice { public: DSi_SDDevice(DSi_SDHost* host) { Host = host; IRQ = false; } - ~DSi_SDDevice() {} + virtual ~DSi_SDDevice() {} virtual void Reset() = 0; diff --git a/src/RTC.cpp b/src/RTC.cpp index ba51dff..aff3dd3 100644 --- a/src/RTC.cpp +++ b/src/RTC.cpp @@ -16,6 +16,9 @@ with melonDS. If not, see http://www.gnu.org/licenses/. */ +// Required by MinGW to enable localtime_r in time.h +#define _POSIX_THREAD_SAFE_FUNCTIONS + #include #include #include @@ -125,31 +128,29 @@ void ByteIn(u8 val) case 0x20: { - time_t timestamp; - struct tm* timedata; - time(×tamp); - timedata = localtime(×tamp); - - Output[0] = BCD(timedata->tm_year - 100); - Output[1] = BCD(timedata->tm_mon + 1); - Output[2] = BCD(timedata->tm_mday); - Output[3] = BCD(timedata->tm_wday); - Output[4] = BCD(timedata->tm_hour); - Output[5] = BCD(timedata->tm_min); - Output[6] = BCD(timedata->tm_sec); + time_t timestamp = time(NULL); + struct tm timedata; + localtime_r(×tamp, &timedata); + + Output[0] = BCD(timedata.tm_year - 100); + Output[1] = BCD(timedata.tm_mon + 1); + Output[2] = BCD(timedata.tm_mday); + Output[3] = BCD(timedata.tm_wday); + Output[4] = BCD(timedata.tm_hour); + Output[5] = BCD(timedata.tm_min); + Output[6] = BCD(timedata.tm_sec); } break; case 0x60: { - time_t timestamp; - struct tm* timedata; - time(×tamp); - timedata = localtime(×tamp); - - Output[0] = BCD(timedata->tm_hour); - Output[1] = BCD(timedata->tm_min); - Output[2] = BCD(timedata->tm_sec); + time_t timestamp = time(NULL); + struct tm timedata; + localtime_r(×tamp, &timedata); + + Output[0] = BCD(timedata.tm_hour); + Output[1] = BCD(timedata.tm_min); + Output[2] = BCD(timedata.tm_sec); } break; diff --git a/src/frontend/qt_sdl/InputConfigDialog.cpp b/src/frontend/qt_sdl/InputConfigDialog.cpp index 2292441..522b16c 100644 --- a/src/frontend/qt_sdl/InputConfigDialog.cpp +++ b/src/frontend/qt_sdl/InputConfigDialog.cpp @@ -228,7 +228,7 @@ void KeyMapButton::keyPressEvent(QKeyEvent* event) { if (!isChecked()) return QPushButton::keyPressEvent(event); - printf("KEY PRESSED = %08X %08X | %08X %08X %08X\n", event->key(), event->modifiers(), event->nativeVirtualKey(), event->nativeModifiers(), event->nativeScanCode()); + printf("KEY PRESSED = %08X %08X | %08X %08X %08X\n", event->key(), (int)event->modifiers(), event->nativeVirtualKey(), event->nativeModifiers(), event->nativeScanCode()); int key = event->key(); int mod = event->modifiers(); diff --git a/src/frontend/qt_sdl/main.cpp b/src/frontend/qt_sdl/main.cpp index e0c12e3..3a735fb 100644 --- a/src/frontend/qt_sdl/main.cpp +++ b/src/frontend/qt_sdl/main.cpp @@ -115,7 +115,6 @@ void audioCallback(void* data, Uint8* stream, int len) if (num_in < len_in-margin) { int last = num_in-1; - if (last < 0) last = 0; for (int i = num_in; i < len_in-margin; i++) ((u32*)buf_in)[i] = ((u32*)buf_in)[last]; -- cgit v1.2.3