Skip to content

Oci8 problem on 64-bit bigendian platforms, bug#73002 #7424

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Oci8 problem on 64-bit bigendian platforms, bug#73002 #7424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6769bd6
On 64-bit bigendian platforms pointers to 8-bytes field cannot be
lzsiga Aug 30, 2021
06cb6e3
Test-script to bug73002 (PR7427)
lzsiga Oct 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions ext/oci8/oci8_statement.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ typedef ub8 oci_phpsized_int;
typedef ub4 oci_phpsized_int;
#endif

#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
#define ZendLongPtr2UInt32Ptr(zendlongptr) ((uint32_t *)(zendlongptr))
#define ZendLongPtr2OciPhpsizedIntPtr(zendlongptr) ((oci_phpsized_int *)(zendlongptr))
#else
#define ZendLongPtr2UInt32Ptr(zendlongptr) \
((uint32_t *)((char *)(zendlongptr)+(sizeof(zend_long)-sizeof(uint32_t))))
#define ZendLongPtr2OciPhpsizedIntPtr(zendlongptr) \
((oci_phpsized_int *)((char *)(zendlongptr)+(sizeof(zend_long)-sizeof(oci_phpsized_int))))
#endif

Copy link
Member

@remicollet remicollet Oct 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO this does not looks like a proper fix

Moving the pointer is not the proper way when size differ, especially as other bytes are not initialized
Should use a temp storage of the proper size.

Copy link
Author

@lzsiga lzsiga Oct 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, using uint64_t * (or ub8 * in Oracle terms) pointers instead of uint32_t * (or ub4 *) pointers is not correct. This problematic behaviour wasn't created by my patch, it already existed; the patch only made it visible, as in big-endian platform it is different:

little-endian: ub4* shortintptr= (ub4 *)&ub8variable;
big-endian:    ub4* shortintptr= (ub4 *)((char *)&ub8variable+4);

Refactoring the code using temporary variables would be possible (they should put into struct php_oci_bind), but would require quite a bit of work (cf https://bugs.php.net/bug.php?id=73002#1473330938 )

/* {{{ php_oci_statement_create()
Create statemend handle and allocate necessary resources */
php_oci_statement *php_oci_statement_create(php_oci_connection *connection, char *query, int query_len)
Expand Down Expand Up @@ -1088,7 +1098,7 @@ int php_oci_bind_post_exec(zval *data)
/* This convetrsion is done on purpose (ext/oci8 uses LVAL as a temporary value) */
if (Z_LVAL_P(zv) == 0)
ZVAL_BOOL(zv, FALSE);
else if (Z_LVAL_P(zv) == 1)
else
ZVAL_BOOL(zv, TRUE);
}

Expand Down Expand Up @@ -1167,7 +1177,7 @@ int php_oci_bind_by_name(php_oci_statement *statement, char *name, size_t name_l
return 1;
}
convert_to_long(param);
bind_data = (oci_phpsized_int *)&Z_LVAL_P(param);
bind_data = ZendLongPtr2OciPhpsizedIntPtr(&Z_LVAL_P(param));
value_sz = sizeof(oci_phpsized_int);
mode = OCI_DEFAULT;
break;
Expand Down Expand Up @@ -1232,7 +1242,8 @@ int php_oci_bind_by_name(php_oci_statement *statement, char *name, size_t name_l
return 1;
}

value_sz = sizeof(zend_long);
bind_data = ZendLongPtr2UInt32Ptr (bind_data); /* relevant on bigendian 64-bit platforms */
value_sz = sizeof(int32_t);

mode = OCI_DEFAULT;
break;
Expand Down Expand Up @@ -1490,8 +1501,8 @@ sb4 php_oci_bind_out_callback(
ZVAL_STRINGL(val, NULL, Z_STRLEN(val) + 1);
#endif

/* XXX we assume that zend-zval len has 4 bytes */
*alenpp = (ub4*) &Z_STRLEN_P(val);
/* in 64-bit, length is 8 bytes long, Oracle expects 4 bytes */
*alenpp = ZendLongPtr2UInt32Ptr(&Z_STRLEN_P(val));
*bufpp = Z_STRVAL_P(val);
*piecep = OCI_ONE_PIECE;
*rcodepp = &phpbind->retcode;
Expand Down
28 changes: 28 additions & 0 deletions ext/oci8/tests/bigendian64_1.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
--TEST--
64-bit bigendian platform, PL/SQL, assign to bind-variable, string type
--SKIPIF--
<?php
if (!extension_loaded('oci8')) die ("skip no oci8 extension");
require(__DIR__.'/connect.inc');
?>
--FILE--
<?php

require(__DIR__.'/connect.inc');

// Run Test

echo "64-bit bigendian platform, PL/SQL, assign to bind-variable, string type\n";

$sql = "begin
:output1 := 'Teststr';
end;";

$s = oci_parse($c, $sql);
oci_bind_by_name($s, ':output1', $output1, 64);
oci_execute($s);
print $output1."\n"
?>
--EXPECT--
64-bit bigendian platform, PL/SQL, assign to bind-variable, string type
Teststr