Skip to content

Zend/zend_compile.h: use uint32_t type for zend_op_array.last_var #21543

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

Open
Girgias wants to merge 1 commit into
base: master
Choose a base branch
from
+18 −22
Open

Zend/zend_compile.h: use uint32_t type for zend_op_array.last_var #21543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion Zend/Optimizer/compact_vars.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
/* This pass removes all CVs and temporaries that are completely unused. It does *not* merge any CVs or TMPs.
* This pass does not operate on SSA form anymore. */
void zend_optimizer_compact_vars(zend_op_array *op_array) {
int i;
uint32_t i;

ALLOCA_FLAG(use_heap1);
ALLOCA_FLAG(use_heap2);
Expand Down
2 changes: 1 addition & 1 deletion Zend/Optimizer/optimize_temp_vars_5.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
void zend_optimize_temporary_variables(zend_op_array *op_array, zend_optimizer_ctx *ctx)
{
uint32_t T = op_array->T;
int offset = op_array->last_var;
uint32_t offset = op_array->last_var;
uint32_t bitset_len;
zend_bitset taken_T; /* T index in use */
zend_op **start_of_T; /* opline where T is first used */
Expand Down
2 changes: 1 addition & 1 deletion Zend/Optimizer/sccp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2381,7 +2381,7 @@ static uint32_t replace_constant_operands(sccp_ctx *ctx) {

/* We iterate the variables backwards, so we can eliminate sequences like INIT_ROPE
* and INIT_ARRAY. */
for (i = ssa->vars_count - 1; i >= op_array->last_var; i--) {
for (i = ssa->vars_count - 1; i >= (int)op_array->last_var; i--) {
Copy link
Copy Markdown
Member

@iluuu1994 iluuu1994 Mar 27, 2026

Choose a reason for hiding this comment

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

😕 This this warn? That's pretty error prone.

Copy link
Copy Markdown
Member Author

@Girgias Girgias Mar 27, 2026

Choose a reason for hiding this comment

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

Was breaking opcache, not sure why.

zend_ssa_var *var = &ssa->vars[i];
zval *value;
int use;
Expand Down
2 changes: 1 addition & 1 deletion Zend/Optimizer/zend_inference.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4885,7 +4885,7 @@ static void zend_mark_cv_references(const zend_op_array *op_array, const zend_sc
ZEND_API zend_result zend_ssa_inference(zend_arena **arena, const zend_op_array *op_array, const zend_script *script, zend_ssa *ssa, zend_long optimization_level) /* {{{ */
{
zend_ssa_var_info *ssa_var_info;
int i;
uint32_t i;

if (!ssa->var_info) {
ssa->var_info = zend_arena_calloc(arena, ssa->vars_count, sizeof(zend_ssa_var_info));
Expand Down
3 changes: 2 additions & 1 deletion Zend/Optimizer/zend_ssa.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,8 @@ ZEND_API zend_result zend_build_ssa(zend_arena **arena, const zend_script *scrip
uint32_t set_size;
zend_bitset def, in, phi;
int *var = NULL;
int i, j, k, changed;
int i, k, changed;
uint32_t j;
zend_dfg dfg;
ALLOCA_FLAG(dfg_use_heap)
ALLOCA_FLAG(var_use_heap)
Expand Down
7 changes: 3 additions & 4 deletions Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ static zend_always_inline uint32_t get_temporary_variable(void) /* {{{ */

static uint32_t lookup_cv(zend_string *name) /* {{{ */{
zend_op_array *op_array = CG(active_op_array);
int i = 0;
uint32_t i = 0;
zend_ulong hash_value = zend_string_hash_val(name);

while (i < op_array->last_var) {
Expand Down Expand Up @@ -8486,9 +8486,8 @@ static void zend_compile_closure_uses(zend_ast *ast) /* {{{ */
ZVAL_NULL(&zv);

{
int i;
for (i = 0; i < op_array->last_var; i++) {
if (zend_string_equals(op_array->vars[i], var_name)) {
for (uint32_t j = 0; j < op_array->last_var; j++) {
Copy link
Copy Markdown
Member

@iluuu1994 iluuu1994 Mar 27, 2026

Choose a reason for hiding this comment

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

I don't understand the switch in variable name here. There's no surrounding i?

Copy link
Copy Markdown
Member Author

@Girgias Girgias Mar 27, 2026

Choose a reason for hiding this comment

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

I was getting a shadowing warning. I'll double check.

if (zend_string_equals(op_array->vars[j], var_name)) {
zend_error_noreturn_unchecked(E_COMPILE_ERROR,
"Cannot use lexical variable $%S as a parameter name", var_name);
}
Expand Down
2 changes: 1 addition & 1 deletion Zend/zend_compile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ struct _zend_op_array {
/* END of common elements */

uint32_t cache_size; /* number of run_time_cache_slots * sizeof(void*) */
int last_var; /* number of CV variables */
uint32_t last_var; /* number of CV variables */
uint32_t last; /* number of opcodes */

zend_op *opcodes;
Expand Down
2 changes: 1 addition & 1 deletion Zend/zend_execute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4278,7 +4278,7 @@ ZEND_API void zend_clean_and_cache_symbol_table(zend_array *symbol_table) /* {{{
static zend_always_inline void i_free_compiled_variables(zend_execute_data *execute_data) /* {{{ */
{
zval *cv = EX_VAR_NUM(0);
int count = EX(func)->op_array.last_var;
uint32_t count = EX(func)->op_array.last_var;
while (EXPECTED(count != 0)) {
i_zval_ptr_dtor(cv);
cv++;
Expand Down
4 changes: 1 addition & 3 deletions ext/opcache/jit/zend_jit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2915,9 +2915,7 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
left_frame = true;
}
if (!left_frame) {
int j;

for (j = 0 ; j < op_array->last_var; j++) {
for (uint32_t j = 0 ; j < op_array->last_var; j++) {
uint32_t info = zend_ssa_cv_info(op_array, ssa, j);

if (info & (MAY_BE_STRING|MAY_BE_ARRAY|MAY_BE_OBJECT|MAY_BE_RESOURCE|MAY_BE_REF)) {
Expand Down
7 changes: 4 additions & 3 deletions ext/opcache/jit/zend_jit_trace.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ static int zend_jit_trace_add_phis(zend_jit_trace_rec *trace_buffer, uint32_t ss
{
const zend_op_array *op_array;
zend_jit_trace_rec *p;
int k, vars_count;
uint32_t vars_count;
zend_bitset use, def;
uint32_t build_flags = ZEND_SSA_RC_INFERENCE | ZEND_SSA_USE_CV_RESULTS;
uint32_t set_size;
Expand Down Expand Up @@ -712,7 +712,7 @@ static int zend_jit_trace_add_phis(zend_jit_trace_rec *trace_buffer, uint32_t ss
} else {
vars_count = op_array->last_var + op_array->T;
}
for (k = 0; k < vars_count; k++) {
for (uint32_t k = 0; k < vars_count; k++) {
if (zend_bitset_in(use, k)) {
zend_ssa_phi *phi = zend_arena_calloc(&CG(arena), 1,
ZEND_MM_ALIGNED_SIZE(sizeof(zend_ssa_phi)) +
Expand Down Expand Up @@ -7814,7 +7814,8 @@ static void zend_jit_dump_trace(zend_jit_trace_rec *trace_buffer, zend_ssa *tssa
const zend_op_array *op_array;
const zend_op *opline;
uint32_t level = 1 + trace_buffer[0].level;
int idx, len, i, v, vars_count, call_level;
int len, v;
uint32_t idx, i, vars_count, call_level;
Copy link
Copy Markdown
Member

@iluuu1994 iluuu1994 Mar 27, 2026

Choose a reason for hiding this comment

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

Seems quite inconsistent, the same variables are declared many times in the JIT files, and i could be local for pretty much all of them.

Copy link
Copy Markdown
Member Author

@Girgias Girgias Mar 27, 2026

Choose a reason for hiding this comment

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

I'm happy to make the i variable local, wasn't sure if I should or not.


ZEND_ASSERT(p->op == ZEND_JIT_TRACE_START);
op_array = p->op_array;
Expand Down
3 changes: 1 addition & 2 deletions ext/opcache/zend_persist.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -681,9 +681,8 @@ static void zend_persist_op_array_ex(zend_op_array *op_array, zend_persistent_sc
}

if (op_array->vars) {
int i;
op_array->vars = zend_shared_memdup_put_free(op_array->vars, sizeof(zend_string*) * op_array->last_var);
for (i = 0; i < op_array->last_var; i++) {
for (uint32_t i = 0; i < op_array->last_var; i++) {
zend_accel_store_interned_string(op_array->vars[i]);
}
}
Expand Down
4 changes: 1 addition & 3 deletions ext/opcache/zend_persist_calc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,8 @@ static void zend_persist_op_array_calc_ex(zend_op_array *op_array)
}

if (op_array->vars) {
int i;

ADD_SIZE(sizeof(zend_string*) * op_array->last_var);
for (i = 0; i < op_array->last_var; i++) {
for (uint32_t i = 0; i < op_array->last_var; i++) {
ADD_INTERNED_STRING(op_array->vars[i]);
}
}
Expand Down
Loading