Skip to content

Commit

Permalink
Fix mismatch 'rz_cons_break_push' and 'rz_cons_break_pop' calls. (#4289)
Browse files Browse the repository at this point in the history
If a function calls 'rz_cons_break_push()' but never calls 'rz_cons_break_pop()' before return,
the stack count of 'RzConsContext->break_stack' contains too many elements (each time one too much).

This in turn will lead to not resetting 'RzConsContext->breaked' flag.
Because the flag is only set to false, if 'rz_stack_is_empty(context->break_stack) == true'
(in 'rz_cons_context_break_push()').

This wasn't a problem so far, because 'RzConsContext->breaked' is simply never set to true
(exceptions are some timeout cases as far as I can see).
Also these cases when 'rz_cons_break_pop()' was forgetten to be called, were edge error cases.
So not often hit.

But if Rizin is usd by Cutter 'RzConsContext->breaked' is set to 'true',
if an `AnalysisTask` interrupt is handled (in 'AnalysisTask::interrupt()').
This interrupt is triggered for example, when the introduction dialog is closed
and the main Cutter window opens (after the optional 'aaa').

Now, if the binary file was analysed with 'aaa', and a lot of error cases were hit,
those error cases sometimes never called 'rz_cons_break_pop()' before returning from their function.
Although, of course, they should have to the `RzConsContext->break_stack` is in a proper state.

This means, when the main Cutter window opens binary files which trigger many error edge cases,
the `RzConsContext->break_stack` is not empty
(because of the not executed 'rz_cons_break_pop()').

This also means, that the last thing done, was setting 'RzConsContext->breaked = true'
(by 'AnalysisTask::interrupt()').

If Cutter wants to show some disassembly, it calls 'rz_core_print_disasm()' which checks
'RzConsContext == false' via 'rz_cons_is_breaked()'. This condition is never true, because
the flag was not reset to `false` because the stack was never empty.
So it returns before anything was disassembled.

Hence Cutter gets no disassembly text.

Fixes rizinorg/cutter#2552
Fixes rizinorg/cutter#3275
  • Loading branch information
Rot127 authored Feb 23, 2024
1 parent 7959b4c commit a6c7864
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 7 deletions.
1 change: 1 addition & 0 deletions librz/analysis/reflines.c
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ RZ_API RzList /*<RzAnalysisRefline *>*/ *rz_analysis_reflines_get(RzAnalysis *an

sten_err:
list_err:
rz_cons_break_pop();
rz_list_free(sten);
rz_list_free(list);
return NULL;
Expand Down
1 change: 1 addition & 0 deletions librz/analysis/rtti_msvc.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ static RZ_OWN RzList /*<rtti_base_class_descriptor *>*/ *rtti_msvc_read_base_cla
ut8 tmp[4] = { 0 };
if (!context->analysis->iob.read_at(context->analysis->iob.io, addr, tmp, 4)) {
rz_list_free(ret);
rz_cons_break_pop();
return NULL;
}
ut32 (*read_32)(const void *src) = context->analysis->big_endian ? rz_read_be32 : rz_read_le32;
Expand Down
7 changes: 5 additions & 2 deletions librz/core/canalysis.c
Original file line number Diff line number Diff line change
Expand Up @@ -2801,6 +2801,7 @@ RZ_API RzList /*<RzAnalysisCycleHook *>*/ *rz_core_analysis_cycles(RzCore *core,
if (!ch) {
rz_analysis_cycle_frame_free(cf);
rz_list_free(hooks);
rz_cons_break_pop();
return NULL;
}
ch->addr = addr;
Expand Down Expand Up @@ -2986,7 +2987,8 @@ RZ_API int rz_core_search_value_in_range(RzCore *core, RzInterval search_itv, ut
rz_cons_break_push(NULL, NULL);

if (!rz_io_is_valid_offset(core->io, from, 0)) {
return -1;
hitctr = -1;
goto beach;
}
while (from < to) {
size = RZ_MIN(to - from, sizeof(buf));
Expand Down Expand Up @@ -3045,7 +3047,8 @@ RZ_API int rz_core_search_value_in_range(RzCore *core, RzInterval search_itv, ut
break;
default:
RZ_LOG_ERROR("core: unknown vsize %d (supported only 1,2,4,8)\n", vsize);
return -1;
hitctr = -1;
goto beach;
}
if (match && !vinfun) {
if (vinfunr) {
Expand Down
2 changes: 1 addition & 1 deletion librz/core/cesil.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ RZ_API int rz_core_esil_step(RzCore *core, ut64 until_addr, const char *until_ex
} else {
rz_reg_setv(core->analysis->reg, "PC", op.addr + op.size);
}
return 1;
return_tail(1);
}
}
rz_reg_setv(core->analysis->reg, name, addr + op.size);
Expand Down
4 changes: 2 additions & 2 deletions librz/core/cmd/cmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1365,8 +1365,6 @@ static int rz_core_cmd_subst(RzCore *core, char *cmd) {
free(cr);
}

rz_cons_break_pop();

if (tmpseek) {
rz_core_seek(core, orig_offset, true);
core->tmpseek = original_tmpseek;
Expand All @@ -1385,6 +1383,7 @@ static int rz_core_cmd_subst(RzCore *core, char *cmd) {
}
}
beach:
rz_cons_break_pop();
free(icmd);
return ret;
}
Expand Down Expand Up @@ -2865,6 +2864,7 @@ RZ_API int rz_core_cmd_foreach(RzCore *core, const char *cmd, char *each) {
free(cmdhit);
}
free(ostr);
rz_cons_break_pop();
return 0;
case '?': // "@@?"
rz_core_cmd_help(core, help_msg_at_at);
Expand Down
2 changes: 1 addition & 1 deletion librz/core/cmd/cmd_debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -2382,6 +2382,7 @@ RZ_IPI RzCmdStatus rz_cmd_debug_continue_back_handler(RzCore *core, int argc, co

if (!rz_debug_continue_back(core->dbg)) {
RZ_LOG_ERROR("core: cannot continue back\n");
rz_cons_break_pop();
return RZ_CMD_STATUS_ERROR;
}

Expand Down Expand Up @@ -2468,7 +2469,6 @@ RZ_IPI RzCmdStatus rz_cmd_debug_continue_send_signal_handler(RzCore *core, int a
// dcp
RZ_IPI RzCmdStatus rz_cmd_debug_continue_mapped_io_handler(RzCore *core, int argc, const char **argv) {
CMD_CHECK_DEBUG_DEAD(core);
rz_cons_break_push(rz_core_static_debug_stop, core->dbg);
RzIOMap *s;
ut64 pc;
int n = 0;
Expand Down
4 changes: 3 additions & 1 deletion librz/core/cmd/cmd_search.c
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ static int search_hash(RzCore *core, const char *hashname, const char *hashstr,
hashname, hashstr, from + i);
free(s);
free(buf);
rz_cons_break_pop();
return 1;
}
free(s);
Expand All @@ -241,6 +242,7 @@ static int search_hash(RzCore *core, const char *hashname, const char *hashstr,
eprintf("No hashes found\n");
return 0;
fail:
rz_cons_break_pop();
return -1;
}

Expand Down Expand Up @@ -1557,12 +1559,12 @@ static int rz_core_search_rop(RzCore *core, RzInterval search_itv, int opt, cons
if (rz_cons_is_breaked()) {
eprintf("\n");
}
rz_cons_break_pop();

if (param->outmode == RZ_MODE_JSON) {
pj_end(param->pj);
}
bad:
rz_cons_break_pop();
rz_list_free(rx_list);
rz_list_free(end_list);
free(grep_arg);
Expand Down
1 change: 1 addition & 0 deletions librz/debug/p/native/windows/windows_debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,7 @@ int w32_dbg_wait(RzDebug *dbg, int pid) {
do {
/* do not continue when already exited but still open for examination */
if (exited_already == pid) {
rz_cons_break_pop();
return RZ_DEBUG_REASON_DEAD;
}
memset(&de, 0, sizeof(DEBUG_EVENT));
Expand Down

0 comments on commit a6c7864

Please sign in to comment.