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

[POC] Support calling global functions from constant expressions #12

Closed
wants to merge 126 commits into from

Conversation

TysonAndre
Copy link
Owner

Motivation: I'd want to be able to call many of php's functions such as count(), strlen(), array_flip(), sprintf(), etc. in a constant declaration.

  • Currently, this treats all functions as global functions.
    Function resolution is straightforward to implement,
    I just haven't implemented it for a proof of concept.

  • Argument unpacking isn't supported, but would be easy to implement.

  • It turns out parameter defaults evaluate some constants every time,
    depending on whether the PHP value is immutable.

    It's likely we want to change this for expressions containing
    function calls, which can be more expensive than evaluating a regular constant AST.
    (e.g. reading files)

  • This handles recursive definitions.

  • Static calls with known classes (other than static::)
    may also be easy to implement.

  • This also constrains function return values to be valid constants,
    (for what define() would accept)
    and throws an error otherwise.

    In the future, const X = [my_call()->x]; may be possible.

  • Variables aren't supported, because they depend on the declaration's scope,
    which may cease to exist.
    A way around this is to define a global helper function to use whatever global variables or static properties are needed, or just use define()

  • This throws an Error if a function call in a constant
    begins when it's still in progress.

Aside: https://github.com/TysonAndre/php-src/pull/10/files is an alternative
approach to a simple syntax to declaring constants with dynamic values,
which instead limits dynamic expressions of any type to static const.


It turns out that function calls can already be invoked when evaluating a
constant, e.g. from php's error handler.
So it seems viable for PHP's to support ordinary calls,
which this POC attempts to do.

(Imagine an error handler defining SOME_DYNAMIC_CONST123 to be a dynamic
value if a notice is emitted) for
const X = [[]['undefined_index'], SOME_DYNAMIC_CONST123][1];

@TysonAndre
Copy link
Owner Author

All of the tests seem to fail in NTS non-debug - local testing was done with NTS and --enable-debug.

========DIFF========
001+ Segmentation fault (core dumped)
001- X is Hello, World
002+ 
003+ Termsig=11
========DONE========
FAIL Can call internal functions from class constants [Zend/tests/call_in_const/class_const01.phpt]

Girgias and others added 7 commits January 25, 2020 12:22
Rewrote session.gc_probability and session.gc_divisor INI setting
description to be more succint.
* PHP-7.3:
  Fixed bug #79080 [ci skip]
* PHP-7.4:
  Fixed bug #79080 [ci skip]
Florian Smeets and others added 21 commits January 26, 2020 14:12
* PHP-7.3:
  Add CURLOPT CURLOPT_HTTP09_ALLOWED available since 7.64.0
* PHP-7.4:
  Add CURLOPT CURLOPT_HTTP09_ALLOWED available since 7.64.0
Set CLI exit code to 1 when invalid parameters are passed,
and print error to stderr.
* PHP-7.3:
  Fix bug #78323: Code 0 is returned on invalid options
* PHP-7.4:
  Fix bug #78323: Code 0 is returned on invalid options
- add ZipArchive::LIBZIP_VERSION
- skip bug53885.phpt with libzip 1.6.0 (empty file is no more valid archive)
* PHP-7.4:
  - bump zip extension version to 1.15.6 - add ZipArchive::LIBZIP_VERSION - skip bug53885.phpt with libzip 1.6.0 (empty file is no more valid archive)
And filter out echoes of the empty string (e.g. false/null)

Split out of php#5097 (on GitHub)

Closes phpGH-5118
Since this pattern is understood by compilers, not a real issue, but
certainly cleaner this way.
* PHP-7.4:
  Fix #79172: STRUCT_OFFSET() relies on undefined behavior
We actually have to decompress, when told to do so.
* PHP-7.3:
  Fix #76584: PharFileInfo::decompress not working
* PHP-7.4:
  Fix #76584: PharFileInfo::decompress not working
Always operate on copies of the functions, so we don't reference
temporary trait methods that have gone out of scope.

This could be more efficient, but doing an allocated copy only when
strictly necessary turned out to be somewhat tricky.
nikic and others added 26 commits January 30, 2020 15:31
The SplFixedArray API treats all elements as NULL, even if they
have not been explicitly initialized. Rather than initializing
to UNDEF an treating that specially in various circumstances,
directly initialize elements to NULL.

This also fixes an assertion failure in the attached test case.
The following commit introduces a cross-compilation failure:

   93c728b
  "Try to control ZEND_MM_ALIGNED_SIZE type"

br-arm-full/build/php-7.4.2/Zend/zend_alloc.h:30:38:
error: missing binary operator before token "8"
                                              ^
br-arm-full/build/php-7.4.2/ext/opcache/ZendAccelerator.c:1380:7:
note: in expansion of macro ‘ZEND_MM_ALIGNMENT’

Closes phpGH-5128.
* PHP-7.4:
  fix cross compilation failure due to size_t typecast in define
This reverts commit b0d7b12.

This change wasn't quite right: I noticed only now that the
RSHUTDOWN function is #ifdef PHP_WIN32 and I'm not fully convinced
that ifdef can be removed: syslog might also be used by error
logging in FPM for example, in which case we probably shouldn't
be closing the log here.

Generally it's unclear how the openlog() functionality exposed
by PHP is supposed to interact with openlog() uses by SAPIs.

For now I'll just revert this change and let it leak across
requests.
* PHP-7.4:
  Fixed bug #79094 (Crashing when running recursion function)
change for Windows (which have libzip 1.4 by default)
update NEWS
The annotation %empty is properly enforced: warnings when it's
missing, and errors when it's inappropriate.  Support for %empty was
introduced in Bison 3.0.

Pass -Wempty-rule to Bison.

Closes phpGH-5134
Unlink the current stack frame before freeing CVs or extra args.
This means it will no longer show up in back traces that are
generated during CV destruction.

We already did this prior to destructing the object/closure,
presumably for the same reason.
* PHP-7.3:
  Fix bug #76047
* PHP-7.4:
  Fix bug #76047
When compiling with "-Wall -Werror" gcc emits two errors:

../src/pcre2_jit_neon_inc.h:211:8: error: ‘cmp1b’ is used uninitialized in this function [-Werror=uninitialized]
  211 | data = fast_forward_char_pair_compare(compare1_type, data, cmp1a, cmp1b);
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/pcre2_jit_neon_inc.h:212:9: error: ‘cmp2b’ is used uninitialized in this function [-Werror=uninitialized]
  212 | data2 = fast_forward_char_pair_compare(compare2_type, data2, cmp2a, cmp2b);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The compiler emits the error message before inlining.
Because the warning is based on an intra-procedural data
flow analysis, the compiler does not see that cmp1b and
cmp2b are not used when they are not initialized.

Here is the code of that function, cmp2 is only used
when ctype is compare_match2 or compare_match1i,
and not when ctype is compare_match1:

static inline vect_t fast_forward_char_pair_compare(compare_type ctype, vect_t dst, vect_t cmp1, vect_t cmp2)
{
if (ctype == compare_match2)
 {
 vect_t tmp = dst;
 dst = VCEQQ(dst, cmp1);
 tmp = VCEQQ(tmp, cmp2);
 dst = VORRQ(dst, tmp);
 return dst;
 }

if (ctype == compare_match1i)
  dst = VORRQ(dst, cmp2);
dst = VCEQQ(dst, cmp1);
return dst;
}

The patch inlines by hand the case of compare_match1 such that the
code avoids referring to cmp1b and cmp2b.

Tested on aarch64-linux with `make check`.
Extensions are not loaded by default (commented out)

Closes phpGH-5136
First, fix 5547d36: the definition of
YFLAGS was not passed into the Makefile: AC_SUBST does not suffice, we
need PHP_SUBST_OLD.  While at it, allow to pass variable and value at
the same time.

Then pass -Wall to bison, rather than only -Wempty-rules.

Use %precedence where associativity is useless.

Remove useless %precedence.
phpGH-5138
Prefer '%define api.value.type' to '#define YYSTYPE', so that Bison
know the type.

Use '%code requires' to declare what is needed to define the api.value.type
(that code is output in the generated header before the generated
definition of YYSTYPE).

Prefer '%define api.prefix' inside the grammar file to '-p' outside,
as anyway the functions defined in the file actually use this prefix.

Prefer `%param` to both `%parse-param` and `%lex-param`.

Closes phpGH-5138
I can think of two main approaches to adding function call
support to PHP. This implements the latter.

1. Only allow functions that are actually deterministic and don't depend on ini
   settings to be used in constant declarations.

   For example, allow `\count()`, `\strlen()`, `\array_merge()`, and `\in_array()`,
   but possibly don't allow functions such as
   `strtolower()` (different in Turkish locale),
   sprintf() (Depends on `ini_get('precision')`, but so does `(string)EXPR`),
   `json_encode()` (The `json` extension can be disabled),
   or calls which aren't unambiguously resolved with `\` or `use function`.

2. Allow any function (user-defined or internal) to be called,
   leave it to coding practice guidelines to assert that constants are only
   used in safe ways.

-------

- This POC can handle fully qualified, namespace relative, and not-FQ calls.
- Argument unpacking is supported
- It turns out parameter defaults evaluate some constants every time,
  depending on whether the PHP value is immutable.

  Possible solutions:

  1. Permanently resolve parameter defaults for expressions containing
     function calls, which can be more expensive than evaluating a regular constant AST.
  2. Permanently resolve expressions in parameter defaults
  3. Don't start allowing calls in parameter defaults
- This handles recursive definitions.
  It throws if a function call being evaluated ends up reaching the same call.
- Static calls with known classes (other than static::)
  are probably easy to implement.
- This also constrains function return values to be valid constants,
  (i.e. they can be the same values that define() would accept)
  and throws an error otherwise.

  In the future, `const X = [my_call()->x];` may be possible.
- Variables aren't supported, because they depend on the declaration's scope,
  which may cease to exist.
- TODO: Forbid backtick string syntax for shell_exec, which gets converted to
  a ZEND_AST_CALL node.

-------

It turns out that function calls can already be invoked when evaluating a
constant, e.g. from php's error handler.
So it seems viable for PHP's engine to support regular calls,
which this POC attempts to do.

(Imagine an error handler defining SOME_DYNAMIC_CONST123 to be a dynamic
value if a notice is emitted) for the expression
`const X = [[]['undefined_index'], SOME_DYNAMIC_CONST123][1];`
@TysonAndre TysonAndre closed this Feb 1, 2020
TysonAndre pushed a commit that referenced this pull request May 31, 2021
The following opcodes would be generated:

  ...
  BB1:
  0003 JMP BB3

  BB2:
  0004 INIT_FCALL 1 96 string("chr")
  0005 #10.T3 [long] = SR #3.CV0($int) [long] #7.CV2($i) ...
  0006 #11.T4 [long] RANGE[0..127] = BW_AND #10.T3 [long] ...
  0007 #12.T3 [long] RANGE[128..255] = BW_OR #11.T4 [long] ...
  0008 SEND_VAL #12.T3 [long] RANGE[128..255] 1
  0009 #13.V3 [ref, rc1, rcn, any] = DO_ICALL
  0010 ASSIGN_OP (CONCAT) #6.CV1($out) [rc1, rcn, string]
  0011 ADD #7.CV2($i)... int(7) #7.CV2($i) ... -> #15.CV2($i) ...

  BB3:
  0012 #8.T4 [long] = SR #3.CV0($int) #7.CV2($i) [long, double]
  0013 #9.T3 [bool] RANGE[0..1] = IS_SMALLER int(128) #8.T4
  0014 JMPNZ #9.T3 [bool] RANGE[0..1] BB2
  ...

Main changes are:
1. SR opcode covers new path in function zend_jit_long_math_helper().
2. BW_AND and BW_OR opcodes are supported. See macro LONG_OP.
3. Function zend_jit_concat_helper() is added to support ASSIGN_OP
opcode. Speficically, CONCAT and FAST_CONCAT is supported for statements
"$out .= ...".
4. New path is covered in function zend_jit_cmp_long_long() by
IS_SMALLER opcode.
5. New path is covered in macros ZVAL_PTR_DTOR and ZVAL_DTOR_FUNC when
leaving.
TysonAndre pushed a commit that referenced this pull request Jun 26, 2021
* JIT/AArch64: Support shifted immediate

As pointed out by MikePall in [1], shifted immediate value is supported.
See [2]. For example, `add x0, x1, php#4096` would be encoded by DynASM
into `add x0, x1, #1, lsl #12` directly.

In this patch, a helper is added to check whether an immediate value is
in the two allowed ranges: (1) 0 to 4095, and (2) LSL #12 on all the
values from the first range.

Note that this helper works for add/adds/sub/subs/cmp/cmn instructions.

[1] LuaJIT/LuaJIT#718
[2]
https://github.com/LuaJIT/LuaJIT/blob/v2.1/dynasm/dasm_arm64.lua#L342

Change-Id: I4870048b9b8e6c429b73a4803af2a3b2d5ec0fbb

* Deprecatd CMP_IMM/ADD_SUB_IMM and add test cases

Macros CMP_IMM and ADD_SUB_IMM are deprecated and instead we use
this helper to guard the immediate encoding.

Add two 64-bit only test cases, since 64-bit integers are used
and tested inside.

Change-Id: I0b42d4617b40372e2f4ce5b6ad31a4ddb7d89e49
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.