diff options
author | Evan Green <evgreen@chromium.org> | 2019-08-01 16:04:42 -0700 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2019-10-05 00:47:33 +0000 |
commit | a41fea6b7df1c83072140c0da9e07f09a8b032e4 (patch) | |
tree | 283880db1385c93420de304b410a3bccafa90971 | |
parent | 4a29d2ecf4392cb995548e8f174cc85bac8361c1 (diff) | |
download | chrome-ec-a41fea6b7df1c83072140c0da9e07f09a8b032e4.tar.gz |
printf: Fix up %p to %pP
In order to avoid landmines later with future extensions to %p,
disallow %p by itself. The danger is that we'll have something
like: printf("%pFOO", myptr), and then later will add a %pF
extension, but miss this printf (maybe the string is split, maybe
it's just missed).
Missing a conversion during extension is worse than just seeing a
print like <ptr_val>OO, since %pF likely reaches through the
pointer and interprets its contents according to whatever F means.
Convert existing uses of %p to %pP, so they're explicitly printing
a pointer value, giving us flexibility to extend in the future.
BUG=chromium:984041
TEST=make -j buildall
BRANCH=None
Cq-Depend:chrome-internal:1560879
Change-Id: I36a4bee8d41cb9a6139171f8de0d8f2f19468132
Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1730604
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
-rw-r--r-- | chip/g/dcrypto/app_cipher.c | 2 | ||||
-rw-r--r-- | chip/g/flash.c | 2 | ||||
-rw-r--r-- | chip/g/system.c | 2 | ||||
-rw-r--r-- | chip/nrf51/bluetooth_le.c | 4 | ||||
-rw-r--r-- | common/bluetooth_le.c | 4 | ||||
-rw-r--r-- | common/btle_hci_controller.c | 6 | ||||
-rw-r--r-- | common/btle_ll.c | 2 | ||||
-rw-r--r-- | common/hooks.c | 2 | ||||
-rw-r--r-- | common/memory_commands.c | 12 | ||||
-rw-r--r-- | common/new_nvmem.c | 2 | ||||
-rw-r--r-- | core/host/stack_trace.c | 1 | ||||
-rw-r--r-- | include/printf.h | 3 | ||||
-rw-r--r-- | test/nvmem.c | 6 | ||||
-rw-r--r-- | test/nvmem_tpm2_mock.c | 4 | ||||
-rw-r--r-- | test/printf.c | 14 | ||||
-rw-r--r-- | test/shmalloc.c | 8 |
16 files changed, 44 insertions, 30 deletions
diff --git a/chip/g/dcrypto/app_cipher.c b/chip/g/dcrypto/app_cipher.c index bc55705f32..2d95173c8f 100644 --- a/chip/g/dcrypto/app_cipher.c +++ b/chip/g/dcrypto/app_cipher.c @@ -260,7 +260,7 @@ static int prepare_running(struct test_info *pinfo) pinfo->test_blob_size |= 7; ccprintf("running %d iterations\n", number_of_iterations); - ccprintf("blob size %d at %p\n", pinfo->test_blob_size, pinfo->p); + ccprintf("blob size %d at %pP\n", pinfo->test_blob_size, pinfo->p); init_stats(&(pinfo->enc_stats)); init_stats(&(pinfo->dec_stats)); diff --git a/chip/g/flash.c b/chip/g/flash.c index fbf1af83a7..4829c986c8 100644 --- a/chip/g/flash.c +++ b/chip/g/flash.c @@ -270,7 +270,7 @@ static int do_flash_op(enum flash_op op, int is_info_bank, if (errors && (errors != prev_error)) { prev_error = errors; - CPRINTF("%s:%d errors %x fsh_pe_control %p\n", + CPRINTF("%s:%d errors %x fsh_pe_control %pP\n", __func__, __LINE__, errors, fsh_pe_control); } /* Error status is self-clearing. Read it until it does diff --git a/chip/g/system.c b/chip/g/system.c index c2ee5bdabb..1b0c285473 100644 --- a/chip/g/system.c +++ b/chip/g/system.c @@ -476,7 +476,7 @@ static int corrupt_header(volatile struct SignedHeader *header) GWRITE_FIELD(GLOBALSEC, FLASH_REGION6_CTRL, RD_EN, 1); GWRITE_FIELD(GLOBALSEC, FLASH_REGION6_CTRL, WR_EN, 1); - CPRINTS("%s: RW fallback must have happened, magic at %p before: %x", + CPRINTS("%s: RW fallback must have happened, magic at %pP before: %x", __func__, &header->magic, header->magic); rv = flash_physical_write((intptr_t)&header->magic - diff --git a/chip/nrf51/bluetooth_le.c b/chip/nrf51/bluetooth_le.c index 1c18a8a84f..6fe123ac4e 100644 --- a/chip/nrf51/bluetooth_le.c +++ b/chip/nrf51/bluetooth_le.c @@ -453,7 +453,7 @@ static int command_ble_adv(int argc, char **argv) rv = ble_radio_init(BLE_ADV_ACCESS_ADDRESS, BLE_ADV_CRCINIT); - CPRINTS("ADV @%p", &adv_packet); + CPRINTS("ADV @%pP", &adv_packet); ((uint32_t *)&addr)[0] = 0xA3A2A1A0 | type; ((uint32_t *)&addr)[1] = BLE_RANDOM_ADDR_MSBS_STATIC << 8 | 0x5A4; @@ -527,7 +527,7 @@ static int command_ble_adv_scan(int argc, char **argv) rv = radio_disable(); - CPRINTS("on_air payload rcvd %p", &rx_packet); + CPRINTS("on_air payload rcvd %pP", &rx_packet); return rv; } diff --git a/common/bluetooth_le.c b/common/bluetooth_le.c index bbc035a49d..5e611d1dd1 100644 --- a/common/bluetooth_le.c +++ b/common/bluetooth_le.c @@ -160,7 +160,7 @@ void dump_ble_packet(struct ble_pdu *ble_p) int curr_offs; if (ble_p->header_type_adv) { - CPRINTF("BLE packet @ %p: type %d, len %d, %s %s\n", + CPRINTF("BLE packet @ %pP: type %d, len %d, %s %s\n", ble_p, ble_p->header.adv.type, ble_p->header.adv.length, (ble_p->header.adv.txaddr ? " TXADDR" : ""), (ble_p->header.adv.rxaddr ? " RXADDR" : "")); @@ -187,7 +187,7 @@ void dump_ble_packet(struct ble_pdu *ble_p) mem_dump(ble_p->payload + curr_offs, ble_p->header.adv.length - curr_offs); } else { /* Data PDUs */ - CPRINTF("BLE data packet @%p: LLID %d," + CPRINTF("BLE data packet @%pP: LLID %d," " nesn %d, sn %d, md %d, length %d\n", ble_p, ble_p->header.data.llid, ble_p->header.data.nesn, ble_p->header.data.sn, ble_p->header.data.md, diff --git a/common/btle_hci_controller.c b/common/btle_hci_controller.c index 3d4076b073..903e9d5624 100644 --- a/common/btle_hci_controller.c +++ b/common/btle_hci_controller.c @@ -503,7 +503,7 @@ static int command_ble_hci_cmd(int argc, char **argv) hci_cmd(hci_buf); - CPRINTS("hci cmd @%p", hci_buf); + CPRINTS("hci cmd @%pP", hci_buf); return EC_SUCCESS; } @@ -543,7 +543,7 @@ static int command_hcitool(int argc, char **argv) hci_cmd(hci_buf); - CPRINTS("hci cmd @%p", hci_buf); + CPRINTS("hci cmd @%pP", hci_buf); return EC_SUCCESS; } @@ -588,7 +588,7 @@ static int command_ble_hci_acl(int argc, char **argv) hci_cmd(hci_buf); - CPRINTS("hci acl @%p", hci_buf); + CPRINTS("hci acl @%pP", hci_buf); return EC_SUCCESS; } diff --git a/common/btle_ll.c b/common/btle_ll.c index d05813d95c..3557c9710d 100644 --- a/common/btle_ll.c +++ b/common/btle_ll.c @@ -766,7 +766,7 @@ void bluetooth_ll_task(void) case ADVERTISING: if (deadline.val == 0) { - CPRINTS("ADV @%p", &ll_adv_pdu); + CPRINTS("ADV @%pP", &ll_adv_pdu); deadline.val = get_time().val + (uint32_t)ll_adv_timeout_us; ll_adv_events = 0; diff --git a/common/hooks.c b/common/hooks.c index ce8712eca2..dcdb4d8ee4 100644 --- a/common/hooks.c +++ b/common/hooks.c @@ -190,7 +190,7 @@ void hook_task(void *u) /* Handle deferred routines */ for (i = 0; i < DEFERRED_FUNCS_COUNT; i++) { if (__deferred_until[i] && __deferred_until[i] < t) { - CPRINTS("hook call deferred 0x%p", + CPRINTS("hook call deferred 0x%pP", __deferred_funcs[i].routine); /* * Call deferred function. Clear timer first, diff --git a/common/memory_commands.c b/common/memory_commands.c index d946ad6e24..6a6455acdc 100644 --- a/common/memory_commands.c +++ b/common/memory_commands.c @@ -147,16 +147,16 @@ static int command_read_word(int argc, char **argv) if ((argc - argc_offs) < 3) { switch (access_size) { case 1: - ccprintf("read 0x%p = 0x%02x\n", + ccprintf("read 0x%pP = 0x%02x\n", address, *((uint8_t *)address)); break; case 2: - ccprintf("read 0x%p = 0x%04x\n", + ccprintf("read 0x%pP = 0x%04x\n", address, *((uint16_t *)address)); break; default: - ccprintf("read 0x%p = 0x%08x\n", address, *address); + ccprintf("read 0x%pP = 0x%08x\n", address, *address); break; } return EC_SUCCESS; @@ -169,17 +169,17 @@ static int command_read_word(int argc, char **argv) switch (access_size) { case 1: - ccprintf("write 0x%p = 0x%02x\n", address, (uint8_t)value); + ccprintf("write 0x%pP = 0x%02x\n", address, (uint8_t)value); cflush(); /* Flush before writing in case this crashes */ *((uint8_t *)address) = (uint8_t)value; break; case 2: - ccprintf("write 0x%p = 0x%04x\n", address, (uint16_t)value); + ccprintf("write 0x%pP = 0x%04x\n", address, (uint16_t)value); cflush(); *((uint16_t *)address) = (uint16_t)value; break; default: - ccprintf("write 0x%p = 0x%02x\n", address, value); + ccprintf("write 0x%pP = 0x%02x\n", address, value); cflush(); *address = value; break; diff --git a/common/new_nvmem.c b/common/new_nvmem.c index e9fa2c1a0c..cd333520fa 100644 --- a/common/new_nvmem.c +++ b/common/new_nvmem.c @@ -1602,7 +1602,7 @@ static void verify_empty_page(void *ph) for (i = 0; i < (CONFIG_FLASH_BANK_SIZE / sizeof(*word_p)); i++) { if (word_p[i] != (uint32_t)~0) { - CPRINTS("%s: corrupted page at %p!", __func__, word_p); + CPRINTS("%s: corrupted page at %pP!", __func__, word_p); flash_physical_erase((uintptr_t)word_p - CONFIG_PROGRAM_MEMORY_BASE, CONFIG_FLASH_BANK_SIZE); diff --git a/core/host/stack_trace.c b/core/host/stack_trace.c index ea08aa469b..adef66dd44 100644 --- a/core/host/stack_trace.c +++ b/core/host/stack_trace.c @@ -44,6 +44,7 @@ static void __attribute__((noinline)) _task_dump_trace_impl(int offset) for (i = 0; i < sz - offset; ++i) { fprintf(stderr, "#%-2d %s\n", i, messages[i]); + /* %p is correct (as opposed to %pP) since this is the host */ sprintf(buf, "addr2line %p -e %s", trace[i + offset], __get_prog_name()); file = popen(buf, "r"); diff --git a/include/printf.h b/include/printf.h index ea1445a526..6162e98bd2 100644 --- a/include/printf.h +++ b/include/printf.h @@ -42,7 +42,8 @@ * - 's' - null-terminated ASCII string * - 'h' - binary data, print as hex; precision is length of data in bytes. * So "%.8h" prints 8 bytes of binary data - * - 'p' - pointer + * - 'pP'- raw pointer. %p followed by another character represents a %p + * extension. * - 'd' - signed integer * - 'i' - signed integer if CONFIG_PRINTF_LEGACY_LI_FORMAT is set (ignore l) * - 'u' - unsigned integer diff --git a/test/nvmem.c b/test/nvmem.c index df5457273b..8eaad92f23 100644 --- a/test/nvmem.c +++ b/test/nvmem.c @@ -211,8 +211,8 @@ static int iterate_over_flash(void) return EC_SUCCESS; } } - ccprintf("%s:%d bad delimiter location: ph %p, " - "dt.ph %p, offset %d, delim offset %d\n", + ccprintf("%s:%d bad delimiter location: ph %pP, " + "dt.ph %pP, offset %d, delim offset %d\n", __func__, __LINE__, at.mt.ph, at.dt.ph, at.mt.data_offset, at.dt.data_offset); @@ -654,7 +654,7 @@ static size_t fill_obj_offsets(uint16_t *offsets, size_t max_objects) uint32_t *op; op = evictable_offs_to_addr(offsets[i]); - ccprintf("offs %04x:%08x:%08x:%08x addr %p size %d\n", + ccprintf("offs %04x:%08x:%08x:%08x addr %pP size %d\n", offsets[i], op[-1], op[0], op[1], op, (uintptr_t)nvmem_cache_base(NVMEM_TPM) + op[-1] - (uintptr_t)op); diff --git a/test/nvmem_tpm2_mock.c b/test/nvmem_tpm2_mock.c index 070525406d..3b2e5340ad 100644 --- a/test/nvmem_tpm2_mock.c +++ b/test/nvmem_tpm2_mock.c @@ -331,8 +331,8 @@ void drop_evictable_obj(void *obj) obj_addr = (uintptr_t)obj - (uintptr_t)nvmem_cache_base(NVMEM_TPM); read_from_cache(obj_addr - sizeof(next_addr), sizeof(next_addr), &next_addr); - ccprintf("%s:%d dropping obj at cache addr %x, offset %x, addr %p next " - "addr %x aka %x (off s_evictNvStart)\n", + ccprintf("%s:%d dropping obj at cache addr %x, offset %x, addr %pP " + "next addr %x aka %x (off s_evictNvStart)\n", __func__, __LINE__, obj_addr - s_evictNvStart, obj_addr, obj, next_addr, next_addr - s_evictNvStart); diff --git a/test/printf.c b/test/printf.c index 7e857dd55c..cf20be7b31 100644 --- a/test/printf.c +++ b/test/printf.c @@ -196,8 +196,20 @@ test_static int test_vsnprintf_pointers(void) { void *ptr = (void *)0x55005E00; - T(expect_success("55005e00", "%p", ptr)); + T(expect_success("55005e00", "%pP", ptr)); T(expect_success(err_str, "%P", ptr)); + /* %p by itself is invalid */ + T(expect(EC_ERROR_INVAL, NO_BYTES_TOUCHED, + /* given -1 as output size limit */ + false, -1, "%p")); + /* %p with an unknown suffix is invalid */ + T(expect(EC_ERROR_INVAL, NO_BYTES_TOUCHED, + /* given -1 as output size limit */ + false, -1, "%p ")); + /* %p with an unknown suffix is invalid */ + T(expect(EC_ERROR_INVAL, NO_BYTES_TOUCHED, + /* given -1 as output size limit */ + false, -1, "%pQ")); return EC_SUCCESS; } diff --git a/test/shmalloc.c b/test/shmalloc.c index d96579c275..b841d7ae1e 100644 --- a/test/shmalloc.c +++ b/test/shmalloc.c @@ -112,7 +112,7 @@ static int check_for_overlaps(void) } } if (!allocation_match) { - ccprintf("missing match %p!\n", allocations[i].buf); + ccprintf("missing match %pP!\n", allocations[i].buf); return 0; } } @@ -137,7 +137,7 @@ static int shmem_is_ok(int line) struct shm_buffer *pbuf = free_buf_chain; if (pbuf && pbuf->prev_buffer) { - ccprintf("Bad free buffer list start %p\n", pbuf); + ccprintf("Bad free buffer list start %pP\n", pbuf); goto bailout; } @@ -153,13 +153,13 @@ static int shmem_is_ok(int line) if (pbuf->next_buffer) { if (top >= pbuf->next_buffer) { ccprintf("%s:%d" - " - inconsistent buffer size at %p\n", + " - inconsistent buffer size at %pP\n", __func__, __LINE__, pbuf); goto bailout; } if (pbuf->next_buffer->prev_buffer != pbuf) { ccprintf("%s:%d" - " - inconsistent next buffer at %p\n", + " - inconsistent next buffer at %pP\n", __func__, __LINE__, pbuf); goto bailout; } |