Skip to content
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.

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

Console: Limit buffer size in ConsoleLogFromVM::Write #12034

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

chaoticgd
Copy link
Contributor

@chaoticgd chaoticgd commented Nov 25, 2024

Description of Changes

The buffer size for lines written to the EE/IOP consoles has been limited to 4096 characters. In addition, consecutive newline characters are no longer eaten.

Rationale behind Changes

Previously it was very easy to accidentally exhaust the host's memory. For example, if you made an infinite loop and forgot the newline character. Since there would be no output it wouldn't be immediately obvious what was wrong either.

Here's a small program that triggers this behaviour:

# ee-gcc exhaustmem.s -o exhaustmem.elf -nostdlib
	.set noat
	.set noreorder
	.set nomacro
	.global _start
_start:
print_loop:
	lui $a0, %hi(format_string)
	addiu $a0, $a0, %lo(format_string)
	addiu $v1, $zero, 117
	syscall
	b print_loop
	nop
	
	.section .rodata
format_string:
	.string "hello world"

Suggested Testing Steps

Here's the above program in its compiled form:
exhaustmem.zip

Since it's been intentionally written to use up lots of memory think twice before running it.

Note that the Qt console window has a similar quirk where it will just keep using more and more memory. This PR doesn't address that issue.

@lightningterror lightningterror merged commit 719063e into PCSX2:master Nov 26, 2024
12 checks passed
@JordanTheToaster
Copy link
Member

This breaks console output from games.

image

@chaoticgd chaoticgd deleted the logging_fix branch November 28, 2024 08:59
@chaoticgd
Copy link
Contributor Author

Oh, doh. The newline is a control character.

@knight-ryu12
Copy link
Contributor

knight-ryu12 commented Nov 28, 2024

probably that need to care if there is /r/n and /n instead to only check for /n

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants