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

util.c: Perl_my_cxt_init() dont round up/over allocate callers' structs #22879

Open
wants to merge 1 commit into
base: blead
Choose a base branch
from

Conversation

bulk88
Copy link
Contributor

@bulk88 bulk88 commented Jan 1, 2025

extra thoughts

perl5/sv.c

Line 5247 in a73cdae

PERL_STRLEN_ROUNDUP(len + 1);

+(PTRSIZE-1) &0x7 trick

perl5/sv.c

Line 5265 in a73cdae

SvLEN_set(sv, Perl_safesysmalloc_size(ptr));

msize? IDK, but does Perl_safesysmalloc_size()/LIBC backend, in-place stretch the input string alloc block, to in-place realloc to "MAX", whatever MAX is for that OS/libc version/CPU/malloc algo.

sv_usepvn() seems clunky vs Newx(); Also sv_usepvn() SVt_PV then SvUPGRADE() SVt_PVIV is 2 pass thru sv_upgrade(). Newx and SVPV_set() are 1 pass.

PPPort backport needs a little bit more eyes, since it faces, IDK realistic or not, but MY_CXT_INIT SV* in ppport are slight easier to corrupt from PP bc they are PP visible in ancient perls.

IDK if to be paranoid with SvTHINKFIRST() or nothing PP or core will fill-in ahead the MCT SV leaving it POK_on, CUR !=0 MAX !=0 ? PPPort version might be fine as is tho.


-an old bug fixed long ago, its copied here, has come back.

/* newSV() allocates one more than needed */
 p = (void*)SvPVX(newSV(size-1));

I saw an XS module passed size 0x10 to my_cxt_init(), my_cxt_init() lowered it to 0xF, and passed it to newSV(). Eventually value 0xF reaches sv_grow_fresh(), inside sv_grow_fresh() this code,

#ifdef PERL_COPY_ON_WRITE
/* COW ... uses SvPVX(sv)[SvLEN(sv)-1] to store the COW count. ....
 allocate one more byte than asked for */

if (newlen != MEM_SIZE_MAX) newlen++;

bumped 0x10 to 0x11. The final value, that was passed to malloc() inside MS CRT/Libc, was 0x11. On 64b, and value "0x11" (bytes long) in arg size. 7/15/23/31 bytes have just been wasted. MY_CXT API exclusively only gets aligned const literal ~"powers of 2" from XS modules. Risks of off by 1 or 2, string overflow bad writes, or finding buggy XS causing CUR==MAX unterminated buffer SVPVs, will never happen with MY_CXT.

sv_usepvn_flags() was not used in this commit, since that fnc is very large and "heavy" (src comparison, not benchmark), compared to newSV_type(). sv_usepvn_flags() has no newSVusepvn() analog.

Add some comments about this source code. Internally document what happens to the new SV. Newxz() vs Zero(), Newxz() has a tiny chance being faster, if malloc() skips a user mode memset() because its a new VM OS page, and its gurenteed nulled by an OS kernel.

The "size -1" constant was a bug before. Its a bug now. Changing the constant to -2, will inevitably be broken again, either with OS/CPU specific code, or perl getting general PV/String performance tweaks. Calling Newx() directly, fixes this permanently.


  • This set of changes requires a perldelta entry, and it is included.

^^^^^ I dont think its needed, but I wrote a perldelta anyways.

-an old bug fixed long ago, its copied here, has come back.

/* newSV() allocates one more than needed */
 p = (void*)SvPVX(newSV(size-1));

I saw an XS module passed size 0x10 to my_cxt_init(), my_cxt_init()
lowered it to 0xF, and passed it to newSV(). Eventually value 0xF
reaches sv_grow_fresh(), inside sv_grow_fresh() this code,

#ifdef PERL_COPY_ON_WRITE
/* COW ... uses SvPVX(sv)[SvLEN(sv)-1] to store the COW count. ....
 allocate one more byte than asked for */

if (newlen != MEM_SIZE_MAX) newlen++;

bumped 0x10 to 0x11. The final value, that was passed to malloc() inside
MS CRT/Libc, was 0x11. On 64b, and value "0x11" (bytes long) in
arg size. 7/15/23/31 bytes have just been wasted. MY_CXT API exclusively
only gets aligned const literal ~"powers of 2" from XS modules. Risks of
off by 1 or 2, string overflow bad writes, or finding buggy XS causing
CUR==MAX unterminated buffer SVPVs, will never happen with MY_CXT.

sv_usepvn_flags() was not used in this commit, since that fnc is very
large and "heavy" (src comparison, not benchmark), compared to
newSV_type(). sv_usepvn_flags() has no newSVusepvn() analog.

Add some comments about this source code. Internally document what happens
to the new SV. Newxz() vs Zero(), Newxz() has a tiny chance being faster,
if malloc() skips a user mode memset() because its a new VM OS page, and
its gurenteed nulled by an OS kernel.

The "size -1" constant was a bug before. Its a bug now. Changing the
constant to -2, will inevitably be broken again, either with OS/CPU
specific code, or perl getting general PV/String performance tweaks.
Calling Newx() directly, fixes this permanently.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant