Add c warning checks in device binary programs
Enable `-Wall` by default to catch coding errors in compilation time.
Clean up the code base on unused variables and uninitialzed values.
Change-Id: I010ad1d690631116f0de03215ffde59ed5e18ac1
diff --git a/rules/matcha.bzl b/rules/matcha.bzl
index 9418dbf..8caf9aa 100644
--- a/rules/matcha.bzl
+++ b/rules/matcha.bzl
@@ -53,6 +53,12 @@
"smc": "//sw/device/lib/arch:smc_asic",
}
+MATCHA_COPTS = [
+ "-Werror",
+ "-Wall",
+ "-Wno-unused-function",
+]
+
# This helper function generates a dictionary of per-device dependencies which is used to
# generate slightly different binaries for each hardware target, including two
# simulation platforms (DV and Verilator), and one FPGA platform (Nexus),
@@ -142,6 +148,11 @@
Containing all targets across all devices for the above generated rules.
"""
deps = kwargs.pop("deps", [])
+
+ # Place the default copts first to allow per-target override.
+ copts = MATCHA_COPTS + kwargs.pop("copts", [])
+ kwargs.update({"copts": copts})
+
all_targets = []
for (device, dev_deps) in per_device_deps.items():
if device not in device_deps("secure_core"):
@@ -249,6 +260,10 @@
Containing all targets across all devices for the above generated rules.
"""
deps = kwargs.pop("deps", [])
+
+ # Place the default copts first to allow per-target override.
+ copts = MATCHA_COPTS + kwargs.pop("copts", [])
+ kwargs.update({"copts": copts})
all_targets = []
for (device, dev_deps) in per_device_deps.items():
if device not in device_deps("secure_core"):
diff --git a/sw/device/examples/demo_hps_live/hps_demo_smc.c b/sw/device/examples/demo_hps_live/hps_demo_smc.c
index 448f901..c397097 100644
--- a/sw/device/examples/demo_hps_live/hps_demo_smc.c
+++ b/sw/device/examples/demo_hps_live/hps_demo_smc.c
@@ -114,9 +114,8 @@
// Render an RGB565 buffer with the score text.
memset(paint_ctx.Image, 0, LCD_WIDTH * font->Height * sizeof(uint16_t));
memset(paint_str, 0, sizeof(paint_str));
- size_t len =
- base_snprintf(paint_str, sizeof(paint_str), "Score 0: %d, Score 1: %d",
- model_out_val0, model_out_val1);
+ base_snprintf(paint_str, sizeof(paint_str), "Score 0: %d, Score 1: %d",
+ model_out_val0, model_out_val1);
Paint_DrawString_EN(&paint_ctx, 0, 0, paint_str, font, BLACK, WHITE);
LCD_ClearToBufferWindow(&spi_display, paint_ctx.Image, 0, 0, LCD_WIDTH,
font->Height);
diff --git a/sw/device/examples/demo_isp/isp_live_cam_test_smc.c b/sw/device/examples/demo_isp/isp_live_cam_test_smc.c
index 554f80d..48bf207 100644
--- a/sw/device/examples/demo_isp/isp_live_cam_test_smc.c
+++ b/sw/device/examples/demo_isp/isp_live_cam_test_smc.c
@@ -48,9 +48,6 @@
static volatile uint32_t isp_wrapper_mi_handled;
static void handle_isp_wrapper_isp_irq() {
- mmio_region_t ml_dmem_base_addr =
- mmio_region_from_addr(TOP_MATCHA_ML_TOP_DMEM_BASE_ADDR);
- uint32_t mem_val;
LOG_INFO("Start of a frame.");
// Print the frame to uart.
size_t bytes_remaining = 320 * 240;
@@ -69,11 +66,6 @@
static void handle_isp_wrapper_isrs(const dif_rv_plic_irq_id_t interrupt_id) {
// NOTE: This initialization is superfluous, since the `default` case below
// is effectively noreturn, but the compiler is unable to prove this.
- // dif_isp_wrapper_irq_t isp_ctrl_irq = 0;
- uint32_t isp_status = mmio_region_read32(isp_wrapper.base_addr, ISP_MIS);
- uint32_t mi_status =
- mmio_region_read32(isp_wrapper.base_addr, MI_MIS);
-
switch (interrupt_id) {
case kTopMatchaPlicIrqIdIspWrapperIsp:
isp_wrapper_isp_handled++;
diff --git a/sw/device/examples/spi_display/spi_display_smc.c b/sw/device/examples/spi_display/spi_display_smc.c
index 7a4fe35..899133a 100644
--- a/sw/device/examples/spi_display/spi_display_smc.c
+++ b/sw/device/examples/spi_display/spi_display_smc.c
@@ -49,7 +49,6 @@
static volatile bool isp_frame_done_irq_seen = false;
void ottf_external_isr(void) {
- uint32_t tx, rx;
dif_rv_plic_irq_id_t plic_irq_id;
// Clear IRQ from the tlul_mailbox and claim it from the PLIC.
diff --git a/sw/device/examples/spi_passthrough/spi_passthrough.c b/sw/device/examples/spi_passthrough/spi_passthrough.c
index 9c57d1f..8af4c11 100644
--- a/sw/device/examples/spi_passthrough/spi_passthrough.c
+++ b/sw/device/examples/spi_passthrough/spi_passthrough.c
@@ -82,12 +82,6 @@
kSpiDeviceWriteCommandSlotBase = 11,
};
-static dif_irq_type_t irq_types[] = {
- kDifIrqTypeEvent, kDifIrqTypeEvent, kDifIrqTypeEvent, kDifIrqTypeEvent,
- kDifIrqTypeEvent, kDifIrqTypeEvent, kDifIrqTypeEvent, kDifIrqTypeEvent,
- kDifIrqTypeEvent, kDifIrqTypeEvent, kDifIrqTypeEvent, kDifIrqTypeStatus,
-};
-
#define FULL_PAGE_SIZE 512
static const int kHalfPageSize = 256;
static const int kFullPageSize = FULL_PAGE_SIZE;
@@ -391,7 +385,6 @@
memcpy(full_page + kHalfPageSize, payload, kHalfPageSize);
}
- uint32_t *full_page_u32 = (uint32_t *)full_page;
dif_toggle_t addr4b_enabled;
CHECK_DIF_OK(
dif_spi_device_get_4b_address_mode(&spi_device, &addr4b_enabled));
diff --git a/sw/device/examples/spi_to_host/spi_to_host_sc.c b/sw/device/examples/spi_to_host/spi_to_host_sc.c
index fdc8ae0..504f290 100644
--- a/sw/device/examples/spi_to_host/spi_to_host_sc.c
+++ b/sw/device/examples/spi_to_host/spi_to_host_sc.c
@@ -44,7 +44,6 @@
#define kSwStrap0Pin (22)
void ottf_external_isr(void) {
- uint32_t rx;
dif_rv_plic_irq_id_t plic_irq_id;
CHECK_DIF_OK(dif_rv_plic_irq_claim(&plic_sec, kTopMatchaPlicTargetIbex0,
diff --git a/sw/device/examples/spi_to_host/spi_to_host_smc.c b/sw/device/examples/spi_to_host/spi_to_host_smc.c
index 9b062d0..91efff6 100644
--- a/sw/device/examples/spi_to_host/spi_to_host_smc.c
+++ b/sw/device/examples/spi_to_host/spi_to_host_smc.c
@@ -35,7 +35,6 @@
static dif_rv_plic_t plic_smc;
void ottf_external_isr(void) {
- uint32_t tx, rx;
dif_rv_plic_irq_id_t plic_irq_id;
CHECK_DIF_OK(dif_rv_plic_irq_claim(&plic_smc, kTopMatchaPlicTargetIbex0Smc,
diff --git a/sw/device/tests/cam_ctrl_test.c b/sw/device/tests/cam_ctrl_test.c
index 45675e3..051e5ef 100644
--- a/sw/device/tests/cam_ctrl_test.c
+++ b/sw/device/tests/cam_ctrl_test.c
@@ -33,7 +33,7 @@
static dif_cam_ctrl_t cam_ctrl;
void _ottf_main(void) {
- uint32_t en_result, regwen_result;
+ uint32_t en_result;
// Initialize the UART to enable logging for non-DV simulation platforms.
if (kDeviceType != kDeviceSimDV) {
diff --git a/sw/device/tests/dma_main.c b/sw/device/tests/dma_main.c
index e9bb3f6..654e8da 100644
--- a/sw/device/tests/dma_main.c
+++ b/sw/device/tests/dma_main.c
@@ -54,11 +54,10 @@
void ottf_external_isr(void) {
LOG_INFO("Writer interrupt received!");
- uint32_t ram_main_data[LINE_COUNT], ram_smc_data[LINE_COUNT];
+ uint32_t ram_smc_data[LINE_COUNT];
dif_rv_plic_irq_id_t plic_irq_id;
dif_dma_irq_state_snapshot_t intr_state;
- mmio_region_t reader_start_addr = mmio_region_from_addr(READER_START_ADDR);
mmio_region_t writer_start_addr = mmio_region_from_addr(WRITER_START_ADDR);
// Claim PLIC IRQ, perform DMA interrupt reg checks after
diff --git a/sw/device/tests/isp_wrapper_test.c b/sw/device/tests/isp_wrapper_test.c
index af14dde..1c7ca1b 100644
--- a/sw/device/tests/isp_wrapper_test.c
+++ b/sw/device/tests/isp_wrapper_test.c
@@ -33,8 +33,6 @@
static dif_isp_wrapper_t isp_wrapper;
void _ottf_main(void) {
- uint32_t en_result, regwen_result;
-
// Initialize the UART to enable logging for non-DV simulation platforms.
if (kDeviceType != kDeviceSimDV) {
init_uart(TOP_MATCHA_UART0_BASE_ADDR, &uart);
diff --git a/sw/device/tests/rstmgr_alert_info_test.c b/sw/device/tests/rstmgr_alert_info_test.c
index daa61b4..9b5515d 100644
--- a/sw/device/tests/rstmgr_alert_info_test.c
+++ b/sw/device/tests/rstmgr_alert_info_test.c
@@ -481,9 +481,6 @@
*/
static void prgm_alert_handler_round4(void) {
- dif_alert_handler_alert_t alerts[ALERT_HANDLER_PARAM_N_ALERTS];
- dif_alert_handler_class_t alert_classes[ALERT_HANDLER_PARAM_N_ALERTS];
-
// Enable all alerts to enable ping associate with them.
for (int i = 0; i < ALERT_HANDLER_PARAM_N_ALERTS; ++i) {
CHECK_DIF_OK(dif_alert_handler_configure_alert(
diff --git a/sw/device/tests/sim_dv/BUILD b/sw/device/tests/sim_dv/BUILD
index 8b4c63b..7e707b0 100644
--- a/sw/device/tests/sim_dv/BUILD
+++ b/sw/device/tests/sim_dv/BUILD
@@ -363,6 +363,10 @@
matcha_dv_test(
name = "data_integrity_escalation_reset_test",
srcs = ["@lowrisc_opentitan//sw/device/tests/sim_dv:data_integrity_escalation_reset_test.c"],
+ copts = [
+ # Mask the code error from OT source
+ "-Wno-unused-variable",
+ ],
deps = [
"//sw/device/lib/dif:alert_handler",
"//sw/device/lib/dif:rstmgr",
@@ -447,6 +451,10 @@
matcha_dv_test(
name = "alert_handler_escalation_test",
srcs = ["@lowrisc_opentitan//sw/device/tests/sim_dv:alert_handler_escalation.c"],
+ copts = [
+ # Mask the code error from OT source
+ "-Wno-unused-variable",
+ ],
deps = [
"//hw/top_matcha:alert_handler_regs",
"//sw/device/lib/arch:device",
@@ -489,6 +497,10 @@
matcha_dv_test(
name = "all_escalation_resets_test",
srcs = ["@lowrisc_opentitan//sw/device/tests/sim_dv:all_escalation_resets_test.c"],
+ copts = [
+ # Mask the code error from OT source
+ "-Wno-unused-variable",
+ ],
deps = [
"//sw/device/lib/dif:alert_handler",
"//sw/device/lib/dif:clkmgr",
diff --git a/sw/device/tests/sim_dv/ast_clk_rst_inputs.c b/sw/device/tests/sim_dv/ast_clk_rst_inputs.c
index ccc8972..72e9a00 100644
--- a/sw/device/tests/sim_dv/ast_clk_rst_inputs.c
+++ b/sw/device/tests/sim_dv/ast_clk_rst_inputs.c
@@ -61,8 +61,6 @@
static volatile const uint8_t kChannel1MinLowByte;
static volatile const uint8_t kChannel1MinHighByte;
-static const uint32_t kPlicTarget = kTopMatchaPlicTargetIbex0;
-
static dif_sensor_ctrl_t sensor_ctrl;
static dif_alert_handler_t alert_handler;
static dif_aon_timer_t aon_timer;
@@ -71,7 +69,6 @@
static dif_pwrmgr_t pwrmgr;
static dif_rstmgr_t rstmgr;
static dif_entropy_src_t entropy_src;
-static dif_pwrmgr_wakeup_reason_t wakeup_reason;
static dif_clkmgr_t clkmgr;
static dif_adc_ctrl_t adc_ctrl;
@@ -79,15 +76,6 @@
static volatile bool interrupt_serviced = false;
static bool first_adc_setup = true;
-static plic_isr_ctx_t plic_ctx = {.rv_plic = &plic,
- .hart_id = kTopMatchaPlicTargetIbex0};
-
-static pwrmgr_isr_ctx_t pwrmgr_isr_ctx = {
- .pwrmgr = &pwrmgr,
- .plic_pwrmgr_start_irq_id = kTopMatchaPlicIrqIdPwrmgrAonWakeup,
- .expected_irq = kDifPwrmgrIrqWakeup,
- .is_only_irq = true};
-
enum {
/**
* The size of the buffer used in firmware to process the entropy bits in
@@ -326,7 +314,6 @@
dif_entropy_src_config_t entropy_src_config, uint32_t alert_idx) {
bool deepsleep;
int read_fifo_depth_val;
- bool clock_check_status;
uint32_t unhealthy_fifos, errors, alerts;
const dif_edn_t edn0 = {.base_addr =
@@ -535,11 +522,6 @@
}
bool test_main() {
- uint32_t event_idx;
- int idx = 1;
- int read_fifo_depth_val;
-
- bool deepsleep;
dif_pwrmgr_domain_config_t pwrmgr_config;
const dif_entropy_src_config_t entropy_src_config = {
diff --git a/sw/device/tests/sim_dv/i2c_device_tx_rx_test.c b/sw/device/tests/sim_dv/i2c_device_tx_rx_test.c
index 19154e3..2771190 100644
--- a/sw/device/tests/sim_dv/i2c_device_tx_rx_test.c
+++ b/sw/device/tests/sim_dv/i2c_device_tx_rx_test.c
@@ -127,8 +127,6 @@
* This function overrides the default OTTF external ISR.
*/
void ottf_external_isr(void) {
- uint32_t i2c_irq_fmt_threshold_id;
-
plic_isr_ctx_t plic_ctx = {.rv_plic = &plic,
.hart_id = kTopMatchaPlicTargetIbex0};
diff --git a/sw/device/tests/sim_dv/i2c_host_tx_rx_test.c b/sw/device/tests/sim_dv/i2c_host_tx_rx_test.c
index 82c9bcb..00ec85d 100644
--- a/sw/device/tests/sim_dv/i2c_host_tx_rx_test.c
+++ b/sw/device/tests/sim_dv/i2c_host_tx_rx_test.c
@@ -41,12 +41,6 @@
static dif_pinmux_t pinmux;
static dif_rv_plic_t plic;
-static const dif_i2c_fmt_flags_t default_flags = {.start = false,
- .stop = false,
- .read = false,
- .read_cont = false,
- .suppress_nak_irq = false};
-
OTTF_DEFINE_TEST_CONFIG();
/**
diff --git a/sw/device/tests/sim_dv/rv_dm_access_after_wakeup.c b/sw/device/tests/sim_dv/rv_dm_access_after_wakeup.c
index 7703621..68672b3 100644
--- a/sw/device/tests/sim_dv/rv_dm_access_after_wakeup.c
+++ b/sw/device/tests/sim_dv/rv_dm_access_after_wakeup.c
@@ -69,9 +69,9 @@
static void put_to_sleep(dif_pwrmgr_t *pwrmgr, bool deep_sleep) {
dif_pwrmgr_domain_config_t cfg;
CHECK_DIF_OK(dif_pwrmgr_get_domain_config(pwrmgr, &cfg));
- cfg = cfg & (kDifPwrmgrDomainOptionIoClockInLowPower |
- kDifPwrmgrDomainOptionUsbClockInLowPower |
- kDifPwrmgrDomainOptionUsbClockInActivePower) |
+ cfg = (cfg & (kDifPwrmgrDomainOptionIoClockInLowPower |
+ kDifPwrmgrDomainOptionUsbClockInLowPower |
+ kDifPwrmgrDomainOptionUsbClockInActivePower)) |
(!deep_sleep ? kDifPwrmgrDomainOptionMainPowerInLowPower : 0);
pwrmgr_testutils_enable_low_power(pwrmgr, kDifPwrmgrWakeupRequestSourceOne,
diff --git a/sw/device/tests/sim_dv/sleep_pin_mio_dio_val_test.c b/sw/device/tests/sim_dv/sleep_pin_mio_dio_val_test.c
index 7c85284..36b7dde 100644
--- a/sw/device/tests/sim_dv/sleep_pin_mio_dio_val_test.c
+++ b/sw/device/tests/sim_dv/sleep_pin_mio_dio_val_test.c
@@ -119,7 +119,6 @@
static uint8_t kDioPads[NUM_DIO_PADS] = {0};
// PLIC structures
-static const uint32_t kPlicTarget = kTopMatchaPlicTargetIbex0;
static dif_pwrmgr_t pwrmgr;
static dif_pinmux_t pinmux;
static dif_rv_plic_t plic;
diff --git a/sw/device/tests/sim_dv/sleep_pin_retention_test.c b/sw/device/tests/sim_dv/sleep_pin_retention_test.c
index 49dfb49..3878daf 100644
--- a/sw/device/tests/sim_dv/sleep_pin_retention_test.c
+++ b/sw/device/tests/sim_dv/sleep_pin_retention_test.c
@@ -52,11 +52,6 @@
// variable via backdoor_overwrite.
static volatile const uint8_t kRounds = 2;
-// SW testcode randomly chooses the value for 8 GPIO pins (pins are fixed) then
-// invert the value for retention. SW will sends the value to SV testbench via
-// LOG_INFO().
-static const uint8_t kGpioVal = 0x00;
-
// To wakeup and maintain GPIO, for now test enters to normal sleep only.
static const bool deepPowerdown = false;
@@ -171,7 +166,6 @@
bool test_main(void) {
bool result = true;
- dif_pinmux_index_t detector;
dif_pinmux_wakeup_config_t wakeup_cfg;
// Default Deep Power Down
diff --git a/sw/device/tests/sim_dv/sleep_pin_wake_test.c b/sw/device/tests/sim_dv/sleep_pin_wake_test.c
index 16e6c6e..b3fecd6 100644
--- a/sw/device/tests/sim_dv/sleep_pin_wake_test.c
+++ b/sw/device/tests/sim_dv/sleep_pin_wake_test.c
@@ -34,7 +34,6 @@
OTTF_DEFINE_TEST_CONFIG();
// PLIC structures
-static const uint32_t kPlicTarget = kTopMatchaPlicTargetIbex0;
static dif_pwrmgr_t pwrmgr;
static dif_pinmux_t pinmux;
static dif_rv_plic_t plic;
@@ -71,7 +70,6 @@
}
bool test_main(void) {
- dif_pinmux_index_t detector;
dif_pinmux_wakeup_config_t wakeup_cfg;
// Default Deep Power Down
diff --git a/sw/device/tests/sim_dv/spi_host_tx_rx_test.c b/sw/device/tests/sim_dv/spi_host_tx_rx_test.c
index 9160a23..738a7f0 100644
--- a/sw/device/tests/sim_dv/spi_host_tx_rx_test.c
+++ b/sw/device/tests/sim_dv/spi_host_tx_rx_test.c
@@ -220,8 +220,8 @@
// Setup spi host configuration
LOG_INFO("Testing spi_host%0d", kSPIHostIdx);
- uintptr_t base_addr;
- uint64_t clkHz;
+ uintptr_t base_addr = 0;
+ uint64_t clkHz = 0;
switch (kSPIHostIdx) {
case 0: {
base_addr = TOP_MATCHA_SPI_HOST0_BASE_ADDR;
@@ -235,6 +235,7 @@
}
default:
LOG_FATAL("Invalid kSPIHostIdx: %d", kSPIHostIdx);
+ abort();
}
CHECK_DIF_OK(dif_spi_host_init(mmio_region_from_addr(base_addr), &spi_host));
init_spi_host(&spi_host, (uint32_t)clkHz);
diff --git a/sw/device/tests/sim_dv/sysrst_ctrl_in_irq_test.c b/sw/device/tests/sim_dv/sysrst_ctrl_in_irq_test.c
index 5b73940..0804ca1 100644
--- a/sw/device/tests/sim_dv/sysrst_ctrl_in_irq_test.c
+++ b/sw/device/tests/sim_dv/sysrst_ctrl_in_irq_test.c
@@ -66,13 +66,6 @@
kTopMatchaPinmuxInselIoc7, kTopMatchaPinmuxInselIoc9,
};
-static const dif_sysrst_ctrl_pin_t kSysrstCtrlInputs[] = {
- kDifSysrstCtrlPinKey0In, kDifSysrstCtrlPinKey1In,
- kDifSysrstCtrlPinKey2In, kDifSysrstCtrlPinPowerButtonIn,
- kDifSysrstCtrlPinAcPowerPresentIn, kDifSysrstCtrlPinLidOpenIn,
- kDifSysrstCtrlPinEcResetInOut, kDifSysrstCtrlPinFlashWriteProtectInOut,
-};
-
void test_phase_sync() {
test_status_set(kTestStatusInTest);
test_status_set(kTestStatusInWfi);
diff --git a/sw/device/tests/smc/smc_cam_i2c_test.c b/sw/device/tests/smc/smc_cam_i2c_test.c
index 6f3c861..64d27d2 100644
--- a/sw/device/tests/smc/smc_cam_i2c_test.c
+++ b/sw/device/tests/smc/smc_cam_i2c_test.c
@@ -41,7 +41,6 @@
static volatile bool isp_frame_done_irq_seen = false;
void ottf_external_isr(void) {
- top_matcha_plic_peripheral_smc_t peripheral;
dif_rv_plic_irq_id_t interrupt_id;
CHECK_DIF_OK(dif_rv_plic_irq_claim(&plic, kTopMatchaPlicTargetIbex0Smc,
&interrupt_id));