Skip to content

Commit c6fe10d

Browse files
committed
arm_adi_v5: fix SIGSEGV due to failing re-examine
Commit 35a503b ("arm_adi_v5: add ap refcount and add get/put around ap use") modifies the examine functions of mem_ap, cortex_m, cortex_a and aarch64 by calling dap_put_ap() and then looking again for the mem-ap and calling dap_get_ap(). This causes an issue if the system is irresponsive and the examine fails and left the AP pointer to NULL. If the system was already examined the NULL pointer will cause a SIGSEGV. Commit b6dad91 ("target/cortex_m: prevent segmentation fault in cortex_m_poll()") proposes a fix for one specific case and only on cortex_m. Modify all the examine functions by skipping look-up for the AP if it was already set in a previous examine; the target's AP is not supposed to change during runtime. Remove the partial fix for cortex_m as it is not needed anymore. Change-Id: I806ec3b1b02fcc76e141c8dd3a65044febbf0a8c Signed-off-by: Antonio Borneo <[email protected]> Fixes: 35a503b ("arm_adi_v5: add ap refcount and add get/put around ap use") Reviewed-on: https://review.openocd.org/c/openocd/+/7392 Tested-by: jenkins Reviewed-by: Tomas Vanek <[email protected]>
1 parent 2278878 commit c6fe10d

File tree

4 files changed

+47
-69
lines changed

4 files changed

+47
-69
lines changed

src/target/aarch64.c

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2546,23 +2546,20 @@ static int aarch64_examine_first(struct target *target)
25462546
if (!pc)
25472547
return ERROR_FAIL;
25482548

2549-
if (armv8->debug_ap) {
2550-
dap_put_ap(armv8->debug_ap);
2551-
armv8->debug_ap = NULL;
2552-
}
2553-
2554-
if (pc->adiv5_config.ap_num == DP_APSEL_INVALID) {
2555-
/* Search for the APB-AB */
2556-
retval = dap_find_get_ap(swjdp, AP_TYPE_APB_AP, &armv8->debug_ap);
2557-
if (retval != ERROR_OK) {
2558-
LOG_ERROR("Could not find APB-AP for debug access");
2559-
return retval;
2560-
}
2561-
} else {
2562-
armv8->debug_ap = dap_get_ap(swjdp, pc->adiv5_config.ap_num);
2563-
if (!armv8->debug_ap) {
2564-
LOG_ERROR("Cannot get AP");
2565-
return ERROR_FAIL;
2549+
if (!armv8->debug_ap) {
2550+
if (pc->adiv5_config.ap_num == DP_APSEL_INVALID) {
2551+
/* Search for the APB-AB */
2552+
retval = dap_find_get_ap(swjdp, AP_TYPE_APB_AP, &armv8->debug_ap);
2553+
if (retval != ERROR_OK) {
2554+
LOG_ERROR("Could not find APB-AP for debug access");
2555+
return retval;
2556+
}
2557+
} else {
2558+
armv8->debug_ap = dap_get_ap(swjdp, pc->adiv5_config.ap_num);
2559+
if (!armv8->debug_ap) {
2560+
LOG_ERROR("Cannot get AP");
2561+
return ERROR_FAIL;
2562+
}
25662563
}
25672564
}
25682565

src/target/cortex_a.c

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2874,23 +2874,20 @@ static int cortex_a_examine_first(struct target *target)
28742874
int retval = ERROR_OK;
28752875
uint32_t didr, cpuid, dbg_osreg, dbg_idpfr1;
28762876

2877-
if (armv7a->debug_ap) {
2878-
dap_put_ap(armv7a->debug_ap);
2879-
armv7a->debug_ap = NULL;
2880-
}
2881-
2882-
if (pc->ap_num == DP_APSEL_INVALID) {
2883-
/* Search for the APB-AP - it is needed for access to debug registers */
2884-
retval = dap_find_get_ap(swjdp, AP_TYPE_APB_AP, &armv7a->debug_ap);
2885-
if (retval != ERROR_OK) {
2886-
LOG_ERROR("Could not find APB-AP for debug access");
2887-
return retval;
2888-
}
2889-
} else {
2890-
armv7a->debug_ap = dap_get_ap(swjdp, pc->ap_num);
2891-
if (!armv7a->debug_ap) {
2892-
LOG_ERROR("Cannot get AP");
2893-
return ERROR_FAIL;
2877+
if (!armv7a->debug_ap) {
2878+
if (pc->ap_num == DP_APSEL_INVALID) {
2879+
/* Search for the APB-AP - it is needed for access to debug registers */
2880+
retval = dap_find_get_ap(swjdp, AP_TYPE_APB_AP, &armv7a->debug_ap);
2881+
if (retval != ERROR_OK) {
2882+
LOG_ERROR("Could not find APB-AP for debug access");
2883+
return retval;
2884+
}
2885+
} else {
2886+
armv7a->debug_ap = dap_get_ap(swjdp, pc->ap_num);
2887+
if (!armv7a->debug_ap) {
2888+
LOG_ERROR("Cannot get AP");
2889+
return ERROR_FAIL;
2890+
}
28942891
}
28952892
}
28962893

src/target/cortex_m.c

Lines changed: 14 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -879,16 +879,6 @@ static int cortex_m_poll(struct target *target)
879879
struct cortex_m_common *cortex_m = target_to_cm(target);
880880
struct armv7m_common *armv7m = &cortex_m->armv7m;
881881

882-
/* Check if debug_ap is available to prevent segmentation fault.
883-
* If the re-examination after an error does not find a MEM-AP
884-
* (e.g. the target stopped communicating), debug_ap pointer
885-
* can suddenly become NULL.
886-
*/
887-
if (!armv7m->debug_ap) {
888-
target->state = TARGET_UNKNOWN;
889-
return ERROR_TARGET_NOT_EXAMINED;
890-
}
891-
892882
/* Read from Debug Halting Control and Status Register */
893883
retval = cortex_m_read_dhcsr_atomic_sticky(target);
894884
if (retval != ERROR_OK) {
@@ -2311,23 +2301,20 @@ int cortex_m_examine(struct target *target)
23112301
/* hla_target shares the examine handler but does not support
23122302
* all its calls */
23132303
if (!armv7m->is_hla_target) {
2314-
if (armv7m->debug_ap) {
2315-
dap_put_ap(armv7m->debug_ap);
2316-
armv7m->debug_ap = NULL;
2317-
}
2318-
2319-
if (cortex_m->apsel == DP_APSEL_INVALID) {
2320-
/* Search for the MEM-AP */
2321-
retval = cortex_m_find_mem_ap(swjdp, &armv7m->debug_ap);
2322-
if (retval != ERROR_OK) {
2323-
LOG_TARGET_ERROR(target, "Could not find MEM-AP to control the core");
2324-
return retval;
2325-
}
2326-
} else {
2327-
armv7m->debug_ap = dap_get_ap(swjdp, cortex_m->apsel);
2328-
if (!armv7m->debug_ap) {
2329-
LOG_ERROR("Cannot get AP");
2330-
return ERROR_FAIL;
2304+
if (!armv7m->debug_ap) {
2305+
if (cortex_m->apsel == DP_APSEL_INVALID) {
2306+
/* Search for the MEM-AP */
2307+
retval = cortex_m_find_mem_ap(swjdp, &armv7m->debug_ap);
2308+
if (retval != ERROR_OK) {
2309+
LOG_TARGET_ERROR(target, "Could not find MEM-AP to control the core");
2310+
return retval;
2311+
}
2312+
} else {
2313+
armv7m->debug_ap = dap_get_ap(swjdp, cortex_m->apsel);
2314+
if (!armv7m->debug_ap) {
2315+
LOG_ERROR("Cannot get AP");
2316+
return ERROR_FAIL;
2317+
}
23312318
}
23322319
}
23332320

src/target/mem_ap.c

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -136,15 +136,12 @@ static int mem_ap_examine(struct target *target)
136136
struct mem_ap *mem_ap = target->arch_info;
137137

138138
if (!target_was_examined(target)) {
139-
if (mem_ap->ap) {
140-
dap_put_ap(mem_ap->ap);
141-
mem_ap->ap = NULL;
142-
}
143-
144-
mem_ap->ap = dap_get_ap(mem_ap->dap, mem_ap->ap_num);
145139
if (!mem_ap->ap) {
146-
LOG_ERROR("Cannot get AP");
147-
return ERROR_FAIL;
140+
mem_ap->ap = dap_get_ap(mem_ap->dap, mem_ap->ap_num);
141+
if (!mem_ap->ap) {
142+
LOG_ERROR("Cannot get AP");
143+
return ERROR_FAIL;
144+
}
148145
}
149146
target_set_examined(target);
150147
target->state = TARGET_UNKNOWN;

0 commit comments

Comments
 (0)