i#7055: Migrate actions runner image from macOS 12 to macOS 13. #7094
Conversation
| @@ -3947,6 +3954,10 @@ set_selfmod_sandbox_offsets(dcontext_t *dcontext) | |||
| } | |||
| len = encode_with_patch_list(dcontext, &patch, &ilist, buf); | |||
| ASSERT(len < BUFFER_SIZE_BYTES(buf)); | |||
| if (len >= BUFFER_SIZE_BYTES(buf)) { | |||
| LOG(THREAD, LOG_EMIT, 3, "len: %d >= buffer size: %d\n", len, | |||
There was a problem hiding this comment.
But LOG is debug-only: so this seems like wasted code.
Instead, do this above, which is what we do elsewhere in the code for this kind of thing:
IF_DEBUG(len =) ...
| @@ -64,8 +64,14 @@ run_analyzer(int argc, const char *args[]) | |||
| assert(!!analyzer); | |||
| bool res = analyzer.run(); | |||
| assert(res); | |||
| if (!res) { | |||
There was a problem hiding this comment.
Generally what we do is instead wrap the var in IF_DEBUG: so IF_DEBUG(bool res =) up above.
Ditto below.
| target = opnd_get_pc(instr_get_target(instr)); | ||
| /* FIXME case 6962: we ignore the segment and assume it matches current cs */ | ||
| #endif |
There was a problem hiding this comment.
Can we instead move this whole if..else chain inside the existing ifdef below, along with the declaration of the target variable?
| "new_thread_setup: thread " TIDFMT ", dstack " PFX " clone record " PFX "\n", | ||
| d_r_get_thread_id(), get_clone_record_dstack(crec), crec); | ||
|
|
||
| rc = dynamo_thread_init(get_clone_record_dstack(crec), mc, crec, false); | ||
| ASSERT(rc != -1); /* this better be a new thread */ | ||
| if (rc == -1) |
There was a problem hiding this comment.
Use IF_DEBUG(rc =) above.
| @@ -100,6 +100,9 @@ instr_create(void *drcontext) | |||
| is_instr_isa_mode_set = instr_set_isa_mode(instr, dr_get_isa_mode(dcontext)); | |||
| #endif | |||
| CLIENT_ASSERT(is_instr_isa_mode_set, "setting instruction ISA mode unsuccessful"); | |||
| if (!is_instr_isa_mode_set) { | |||
There was a problem hiding this comment.
Instead, put DODEBUG({}); around the lines related to is_instr_* (and move its decl down: it's up top b/c older Visual Studio required decls at top but none of our supported compilers require that anymore).
| @@ -797,6 +797,10 @@ read_evex(byte *pc, decode_info_t *di, byte instr_byte, | |||
| info = *ret_info; | |||
|
|
|||
| CLIENT_ASSERT(info->type == EVEX_PREFIX_EXT, "internal evex decoding error"); | |||
| if (info->type != EVEX_PREFIX_EXT) { | |||
There was a problem hiding this comment.
Instead of all these if..LOG which won't actually log anything b/c they won't be reached in debug and in release LOG is a nop, use IF_DEBUG(info = *ret_info) (or DEBUG_DECLARE(const instr_info_t *info = *ret_Info).
The macOS 12 Actions runner image will begin deprecation on 10/7/24 and will be fully unsupported by 12/3/24 for GitHub and ADO.
Please refer to actions/runner-images#10721 for details.
Updating the runner to macOS13 and DEVELOPER_DIR triggers unused-but-set-variable warnings.
Changed code to address unused-but-set-variable warnings by moving code under #ifdef, adding LOGs following CLIENT_ASSERT.
Fixes: #7055