capstone-engine / capstone

Capstone disassembly/disassembler framework for ARM, ARM64 (ARMv8), Alpha, BPF, Ethereum VM, HPPA, LoongArch, M68K, M680X, Mips, MOS65XX, PPC, RISC-V(rv32G/rv64G), SH, Sparc, SystemZ, TMS320C64X, TriCore, Webassembly, XCore and X86.
http://www.capstone-engine.org
7.18k stars 1.52k forks source link

SH: Use bitwise OR with mask for sign extension #2371

Closed lhsazevedo closed 3 weeks ago

lhsazevedo commented 1 month ago

Sign extend using bitwise OR with mask, instead of unary minus. Fixes error when building for UWP with Security Development Lifecycle (SDL) checks:

capstone\arch\SH\SHDisassembler.c(1463,11): error C4146: unary minus operator applied to unsigned type, result still unsigned [capstone\capstone.vcxproj]
capstone\arch\SH\SHDisassembler.c(1468,11): error C4146: unary minus operator applied to unsigned type, result still unsigned [capstone\capstone.vcxproj]

See https://learn.microsoft.com/en-us/cpp/build/reference/sdl-enable-additional-security-checks?view=msvc-170

Also inline with the rest of the codebase, which uses bitwise OR and mask for sign extension:

$ grep -r '|= ~' .
./arch/TMS320C64x/TMS320C64xDisassembler.c:     imm |= ~((1 << 5) - 1);
./arch/TMS320C64x/TMS320C64xDisassembler.c:     imm |= ~((1 << 16) - 1);
./arch/TMS320C64x/TMS320C64xDisassembler.c:     imm |= ~((1 << 7) - 1);
./arch/TMS320C64x/TMS320C64xDisassembler.c:     imm |= ~((1 << 10) - 1);
./arch/TMS320C64x/TMS320C64xDisassembler.c:     imm |= ~((1 << 12) - 1);
./arch/TMS320C64x/TMS320C64xDisassembler.c:     imm |= ~((1 << 21) - 1);
./arch/X86/X86Disassembler.c:                           immediate |= ~(0xffull);
./arch/X86/X86Disassembler.c:                           immediate |= ~(0xffffull);
./arch/X86/X86Disassembler.c:                           immediate |= ~(0xffffffffull);
./arch/X86/X86Disassembler.c:                   immediate |= ~(0xffull);
./arch/X86/X86Disassembler.c:                   immediate |= ~(0xffffull);
./arch/X86/X86Disassembler.c:                   immediate |= ~(0xffffffffull);
./arch/X86/X86Disassembler.c:                   immediate |= ~(0xffull);
./arch/X86/X86Disassembler.c:                   immediate |= ~(0xffffull);
./arch/X86/X86Disassembler.c:                   immediate |= ~(0xffffffffull);
./arch/AArch64/AArch64Disassembler.c:       ImmVal |= ~((1LL << 19) - 1);
./arch/AArch64/AArch64Disassembler.c:       offset |= ~((1LL << 9) - 1);
./arch/AArch64/AArch64Disassembler.c:       offset |= ~((1LL << 7) - 1);
./arch/AArch64/AArch64Disassembler.c:       imm |= ~((1LL << 21) - 1);
./arch/AArch64/AArch64Disassembler.c:       imm |= ~((1LL << 26) - 1);
./arch/AArch64/AArch64Disassembler.c:       dst |= ~((1LL << 14) - 1);
./arch/AArch64/AArch64Disassembler.c:           Imm |= ~((1LL << Bits) - 1);                                       \
./arch/AArch64/AArch64AddressingModes.h:        Imm |= ~Mask;
./arch/SH/SHDisassembler.c:             imm |= ~((1 << 28) - 1);
./arch/SH/SHDisassembler.c:             imm |= ~((1 << 20) - 1);
$ grep -r '= -(' .
./MathExtras.h: Value &= -((int8_t)Value);
./suite/synctools/tablegen/include/llvm/CodeGen/ReachingDefAnalysis.h:  const int ReachingDefDefaultVal = -(1 << 20);
./suite/synctools/tablegen/include/llvm/CodeGen/SelectionDAGNodes.h:      NId = -(NId + 1);

Suggested reviewer: @ysat0

XVilka commented 1 month ago

Should be also cherry-picked to next

XVilka commented 1 month ago

@kabeor you forgot this one 😄

XVilka commented 3 weeks ago

@kabeor @aquynh

XVilka commented 3 weeks ago

@kabeor

aquynh commented 3 weeks ago

merged, thanks!

please can you also make a PR for the next branch?

lhsazevedo commented 2 weeks ago

Sure!