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

unsafe can modify pub const strings and arrays in other modules: is not good. #23582

Open
jorgeluismireles opened this issue Jan 25, 2025 · 4 comments · May be fixed by #23588
Open

unsafe can modify pub const strings and arrays in other modules: is not good. #23582

jorgeluismireles opened this issue Jan 25, 2025 · 4 comments · May be fixed by #23588
Assignees
Labels
Bug This tag is applied to issues which reports bugs. Unit: Checker Bugs/feature requests, that are related to the type checker.

Comments

@jorgeluismireles
Copy link

jorgeluismireles commented Jan 25, 2025

Describe the bug

unsafe code can modify others modules' pub const (variables in fact), including strings and fixed arrays. Don't know if is a bug or feature, but actual code can modify even the V library as important as time and others. Other's people libraries in purpose or by mistake can modify expectations in V library or other libraries. We don't want pi constant changes!

Reproduction Steps

Next program shows how time module "constants" are modified and the results before and after for the same time calls give different results.

import time

fn call_code_you_didnt_write() {
	unsafe{
		time.months_string = 'xxx'
		for i in 0 .. 12 { time.month_days[i] = 0 }
	}
}

fn main() {
	now := time.now()
	for i in ['original', ' updated'] {
		println('${i} months        ${time.months_string}')
		println('${i} days          ${time.month_days}')
		println('${i} January days  ${time.days_in_month(1, 2025)!}')
		println('${i} this month is ${now.smonth()}')
		
		call_code_you_didnt_write()
	}
}

Produces

original months        JanFebMarAprMayJunJulAugSepOctNovDec
original days          [31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31]
original January days  31
original this month is Jan
 updated months        xxx
 updated days          [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
 updated January days  0
 updated this month is xxx

Expected Behavior

Prevent unsafe code can modify const values.

As a comparison, currently unsafe can't modify private nor immutable fields of other modules nor the size of arrays. Though unsafe can modify local variables declared not mut.

Current Behavior

unsafe code can modify constants defined in the V library:

module time

pub const days_string = 'MonTueWedThuFriSatSun'
pub const long_days = ['Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday', 'Sunday']!
pub const month_days = [31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31]!
...

Possible Solution

  • Prevent unsafe modify the constants.
  • Change important library constants to be not public.

Additional Information/Context

No response

V version

V 0.4.9 c225e04

Environment details (OS name and version, etc.)

https://play.vlang.io/p/50eb44d1e0

Note

You can use the 👍 reaction to increase the issue's priority for developers.

Please note that only the 👍 reaction to the issue itself counts as a vote.
Other reactions and those to comments will not be taken into account.

@jorgeluismireles jorgeluismireles added the Bug This tag is applied to issues which reports bugs. label Jan 25, 2025
Copy link

Connected to Huly®: V_0.6-22008

@jorgeluismireles
Copy link
Author

import math

fn main() {
	println('${math.pi_2}')
	unsafe{
		math.pi_2 = 0
	}
	println('${math.pi_2}')
}
1.5707963267948966
0.0

@JalonSolov
Copy link
Contributor

The whole point of having unsafe is to be able to do whatever you need to do. One might consider it a bug if you can't modify everything.

If people use it unwisely, that is their own fault.

@felipensp felipensp self-assigned this Jan 26, 2025
@felipensp felipensp added the Unit: Checker Bugs/feature requests, that are related to the type checker. label Jan 26, 2025
@jorgeluismireles
Copy link
Author

unsafe can't modify simple (int, f64...) constants like math.pi but can modify post-calculated constants like math.pi_2 mentioned above.

module math
pub const pi = 3.14159265358979323846264338327950288419716939937510582097494459
pub const pi_2 = pi / 2.0

Next code tries to modify pi and fails:

import math

fn main() {
	println('${math.pi}')
	unsafe{
		math.pi = 0
	}
	println('${math.pi}')
}

produces

/tmp/v_60000/../../../../../../box/code.v:6: at main__main: RUNTIME ERROR: invalid memory access
/tmp/v_60000/../../../../../../tmp/v_60000/code.01JJM6699HAQC390GX4ZHRV6BE.tmp.c:13888: by main
3.141592653589793
Exited with error status 255

IMHO I think we need documented what things can and can't do unsafe. I understand const is regarded also as a global since can be defined outside any struct and maybe someone would need the capacity to modify some globals then the possibility unsafe can paint outside the box. But the term constant invites us to think in something written in stone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This tag is applied to issues which reports bugs. Unit: Checker Bugs/feature requests, that are related to the type checker.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants