More MSAN features.

[nicolasnoble]

• DMA writes now properly flag memory as initialized.
• GDB now supports the memory region properly.

[coderabbitai]

Walkthrough

This pull request integrates memory sanitization into various DMA operations and memory handling routines throughout the codebase. It adds calls to a new msanDmaWrite method in several DMA functions (in cdrom, GPU, MDEC, and PSXDMA modules), updates memory mapping to include an MSAN region, refines memory block access logic with MSAN checks, and renames an initializer function. These modifications enhance memory safety tracking without altering existing control flow or error handling.

Changes

File(s)	Change Summary
src/core/cdrom.cc, src/core/gpu.cc, src/core/mdec.cc, src/core/psxdma.cc	Added calls to msanDmaWrite in DMA routines after data transfers to mark memory regions for sanitization. In psxdma.cc, added calls with adjusted sizes in both dma4 and dma6.
src/core/gdb-server.cc	Added a new XML memory mapping entry for the "MSAN" region with a designated address and length, wrapped in XML comments.
src/core/psxmem.cc, src/core/psxmem.h	Updated parameter names in readBlock and writeBlock for clarity; enhanced these methods with MSAN checks when memory is unmapped; added the new method msanDmaWrite to update the sanitizer's bitmap; standardized hexadecimal formatting.
src/mips/common/crt0/cxxglue.c	Renamed function call from pcsx_msanInit() to pcsx_initMsan() in the cxxmain function, reflecting a change in the initialization naming convention.

Sequence Diagram(s)

sequenceDiagram
    participant DMA_Function as DMA Function
    participant Memory_Module as Memory
    participant MSAN as MSAN Sanitizer

    DMA_Function ->> Memory_Module: Execute DMA transfer (read/write)
    Memory_Module ->> MSAN: Call msanDmaWrite(address, size)
    MSAN -->> Memory_Module: Update MSAN bitmap
    Memory_Module -->> DMA_Function: Return control after DMA operation

Loading

Possibly related PRs

• Adding "msan"-like feature. #1855: Introduces memory sanitization in DMA functions with msanDmaWrite, aligning closely with the changes in this PR.
• Implement DMA-msan compatibility #1866: Implements msan integration within DMA operations, particularly affecting the GPU and cdrom components.
• Several fixes and improvements to the typed debugger #1865: Focuses on memory handling and sanitization in DMA operations, complementing the TypedDebugger adjustments present in this PR.

Poem

I'm a bunny on the run,
Hopping through code under the sun,
Sanitizing bytes with every hop,
MSAN marks each block non-stop,
In DMA streams, I dance with glee –
A furry coder, wild and free!
🐇✨

---

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e74436 and 3e50878.

📒 Files selected for processing (8)

• src/core/cdrom.cc (1 hunks)
• src/core/gdb-server.cc (1 hunks)
• src/core/gpu.cc (1 hunks)
• src/core/mdec.cc (1 hunks)
• src/core/psxdma.cc (2 hunks)
• src/core/psxmem.cc (1 hunks)
• src/core/psxmem.h (3 hunks)
• src/mips/common/crt0/cxxglue.c (1 hunks)

🧰 Additional context used

🧬 Code Definitions (2)

src/core/psxdma.cc (2)

src/core/cdrom.cc (4)

• madr (1422-1490)
• madr (1422-1422)
• size (203-207)
• size (203-203)

src/core/psxmem.h (3)

• size (88-88)
• size (291-291)
• size (291-291)

src/core/gpu.cc (3)

src/core/gpu.h (3)

• madr (57-57)
• size (210-213)
• size (210-210)

src/core/cdrom.cc (4)

• madr (1422-1490)
• madr (1422-1422)
• size (203-207)
• size (203-203)

src/core/psxmem.h (3)

• size (88-88)
• size (291-291)
• size (291-291)

🪛 GitHub Check: CodeScene Cloud Delta Analysis (main)

src/core/psxmem.cc

[warning] 794-826: ❌ Getting worse: Code Duplication
introduced similar code in: PCSX::Memory::MemoryAsFile::readBlock,PCSX::Memory::MemoryAsFile::writeBlock. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

---

[warning] 794-806: ❌ New issue: Deep, Nested Complexity
PCSX::Memory::MemoryAsFile::readBlock has a nested complexity depth of 4, threshold = 4. This function contains deeply nested logic such as if statements and/or loops. The deeper the nesting, the lower the code health.

---

[warning] 814-826: ❌ New issue: Deep, Nested Complexity
PCSX::Memory::MemoryAsFile::writeBlock has a nested complexity depth of 4, threshold = 4. This function contains deeply nested logic such as if statements and/or loops. The deeper the nesting, the lower the code health.

⏰ Context from checks skipped due to timeout of 90000ms (11)

• GitHub Check: pcsx-redux (x86_64-linux)
• GitHub Check: pcsx-redux (aarch64-linux)
• GitHub Check: Codacy Static Code Analysis
• GitHub Check: build
• GitHub Check: cross-arm64
• GitHub Check: asan
• GitHub Check: coverage
• GitHub Check: macos-build-and-test-toolchain
• GitHub Check: build-openbios
• GitHub Check: aur-build
• GitHub Check: toolchain

🔇 Additional comments (12)

src/core/gdb-server.cc (1)

273-275: MSAN memory region added to GDB memory map

This addition properly maps the MSAN (Memory Sanitizer) region to the GDB memory map, enabling the debugger to have visibility into this memory area. This aligns with the PR objective of improving GDB support for memory regions.

src/core/mdec.cc (1)

483-483: Memory sanitization correctly added for MDEC DMA operations

This line correctly marks the memory region affected by DMA transfers as initialized in the Memory Sanitizer's tracking system. This is consistent with the PR objective of modifying DMA writes to properly flag memory as initialized.

src/mips/common/crt0/cxxglue.c (1)

52-52:

✅ Verification successful

Function call renamed for consistency

Renamed function call from pcsx_msanInit() to pcsx_initMsan(), which provides a more consistent naming convention by following the pattern of verb+noun rather than noun+verb.

---

🏁 Script executed:

#!/bin/bash
# Verify the definition of the new function name exists
grep -r "pcsx_initMsan" --include="*.h" --include="*.c" --include="*.cc" --include="*.cpp" .

Length of output: 266

---

Function call renaming verified

The renaming of pcsx_msanInit() to pcsx_initMsan() has been confirmed. The function definition is located in src/mips/common/hardware/pcsxhw.h and its usage in src/mips/common/crt0/cxxglue.c aligns with the consistent verb+noun naming convention. No additional changes are required.

src/core/psxdma.cc (2)

64-64: Memory sanitization added for SPU DMA read operations

This addition correctly marks memory affected by SPU DMA operations as initialized in the Memory Sanitizer's tracking system. The multiplication by 2 properly accounts for the size of uint16_t elements being transferred, ensuring accurate memory tracking.

---

120-120: Memory sanitization added for GPU OT DMA operations

This addition correctly marks memory affected by GPU Ordering Table DMA operations as initialized in the Memory Sanitizer's tracking system. The multiplication by 4 properly accounts for the size of uint32_t elements, ensuring the entire affected memory region is correctly tracked.

src/core/cdrom.cc (1)

1469-1469: Good addition for memory sanitization tracking

This call correctly marks memory regions written to by the CDROM DMA operation as initialized in the memory sanitizer, which prevents false positive "uninitialized memory read" warnings or errors when this memory is accessed later.

src/core/gpu.cc (1)

517-517: Good MSAN integration for GPU DMA reads

This line correctly marks memory regions filled by the GPU DMA operation (VRAM to memory) as initialized in the memory sanitizer. The size multiplication by 4 accounts for the 32-bit word size, ensuring accurate tracking of the entire memory range affected by the operation.

src/core/psxmem.h (3)

106-107: Style consistency improvement

Changed uppercase 0xFF to lowercase 0xff for consistent hexadecimal notation.

---

127-128: Style consistency improvement

Changed uppercase 0xFF to lowercase 0xff for consistent hexadecimal notation.

---

136-142: Well-designed MSAN DMA write tracking function

This new method provides a clean interface for hardware components to mark memory regions as initialized after DMA writes. It has the appropriate checks and efficiently marks each byte in the specified range.

src/core/psxmem.cc (2)

794-806: Improved MSAN integration for memory reads

Enhanced the readBlock method to handle reads from unmapped memory regions that are tracked by MSAN. This ensures that memory operations through the file interface correctly respect MSAN's initialization tracking, preventing invalid reads while allowing access to properly initialized MSAN memory.

🧰 Tools

🪛 GitHub Check: CodeScene Cloud Delta Analysis (main)

[warning] 794-826: ❌ Getting worse: Code Duplication
introduced similar code in: PCSX::Memory::MemoryAsFile::readBlock,PCSX::Memory::MemoryAsFile::writeBlock. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

---

[warning] 794-806: ❌ New issue: Deep, Nested Complexity
PCSX::Memory::MemoryAsFile::readBlock has a nested complexity depth of 4, threshold = 4. This function contains deeply nested logic such as if statements and/or loops. The deeper the nesting, the lower the code health.

---

814-826: Improved MSAN integration for memory writes

Enhanced the writeBlock method to handle writes to unmapped memory regions that are tracked by MSAN. This ensures that memory operations through the file interface correctly update MSAN's tracked memory, maintaining consistent memory state even for regions that don't map to actual hardware.

🧰 Tools

🪛 GitHub Check: CodeScene Cloud Delta Analysis (main)

[warning] 794-826: ❌ Getting worse: Code Duplication
introduced similar code in: PCSX::Memory::MemoryAsFile::readBlock,PCSX::Memory::MemoryAsFile::writeBlock. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

---

[warning] 814-826: ❌ New issue: Deep, Nested Complexity
PCSX::Memory::MemoryAsFile::writeBlock has a nested complexity depth of 4, threshold = 4. This function contains deeply nested logic such as if statements and/or loops. The deeper the nesting, the lower the code health.

✨ Finishing Touches

• 📝 Generate Docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai plan to trigger planning for file edits and PR creation.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[Copilot]

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (3)

src/core/psxmem.h:136

• The new msanDmaWrite function lacks dedicated tests. Consider adding tests to verify that DMA writes correctly update the MSAN initialized bitmap.

void msanDmaWrite(uint32_t addr, uint32_t size) {

src/core/psxmem.cc:794

• [nitpick] Consider using a more descriptive name for the parameter 'dest_' (e.g. 'outputBuffer') to improve code clarity.

void PCSX::Memory::MemoryAsFile::readBlock(void *dest_, size_t size, size_t ptr) {

src/core/psxmem.cc:814

• [nitpick] Consider renaming the parameter 'src_' to a more descriptive name such as 'inputBuffer' to improve clarity.

void PCSX::Memory::MemoryAsFile::writeBlock(const void *src_, size_t size, size_t ptr) {