Hi,
recently, I noticed I was getting "Invalid pointer" and "4 bytes overflown"
messages executing PHP scripts connecting to sybase:
[...]php5/ext/sybase_ct/php_sybase_ct.c(818) : Block 0x09091060 status:
Invalid pointer: ((size=0x000000B6) != (next.prev=0x08410004))
Invalid pointer: ((prev=0x00000000) != (prev.size=0x000000B6))
[...]php5/Zend/zend_variables.c(175) : Block 0x09091050 status:
[...]php5/Zend/zend_execute.h(70) : Actual location (location was relayed)
Beginning: OK (allocated on [...]php5/Zend/zend_vm_execute.h:182, 16
bytes)
Start: OK
End: Overflown (magic=0x0000007C instead of 0xF822F907)
At least 4 bytes overflown
Trying to track down to what this resulted from I tried the most simple form
of a test script:
$ php -r 'sybase_connect("server", "user", "password");'
...which resulted in a segmentation fault:
0x082aaefd in vspprintf (pbuf=0x1058, max_len=0, format=0x83f3ed5
"sybase_%s_%s_%s__", ap=0xbfbff5fc "À\037F\bô\037F\bh
F\blö¿¿hö¿¿´0E\b\030ú¿¿
(ú¿¿\"P-\bs F\bè`D\b\004")
at /home/thekid/devel/php/php5/main/spprintf.c:753
753 *pbuf = xbuf.c;
The fix is quite easy, spprintf wants a char**, but was given a char*. The
commits leading to the breakage read "avoid sprintf" and "simplify code"
(without further explanation).
Now I do understand that simplifying sourcecode may be a great idea, and
that in this case it really does make things easier, but I was wondering
why:
1) These changes are necessary. I don't see a bug ID in the
CVS commit log anywhere.
2) Although phpt-tests *are* provided, these most probably
weren't run after committing these changes? They would
have uncovered these problems right away.
3) While I can imagine the committers maybe don't have a sybase
server at hand to test with, and that that's the reason for
not running the tests, why don't they simply send the
maintainers, who usually have things like this at hand to test
with, a patch with the changes? Or at least send it to the
list...
4) When this new guideline of "avoiding sprintf" was introduced
(must've been a couple of weeks ago, according to the commits)
and why it was not announced - IIRC - to php-internals? Is it
some kind of secret? Should I be on IRC or at conferences to
know stuff like this?
5) If I had not - by chance - recompiled PHP this would've probably
been in 5.2.2-release, right? For code cleanliness and undocumented
anti-sprintf reasons you risk breaking existing functionality...
I thought I'd bring this to the list because maybe these kind of careless
commits have happened to other extensions too. If you're an extension autor
or maintainer, you might want to check if your source was "simplified", too.
- Timm
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
