The infinite loop check seems to work pretty well. I've attached the
patches and the results from an allmodconfig on linux-next.
If we have code like:
if (readl() & FOO) {
/* we can reach here if readl() succeeds or it fails */
} else {
/* we can only reach here if readl() succeeds.
}
The test records if we rely on readl() to reach the start of a loop.
The it records if we rely on readl() to exit from the loop.
If we didn't need readl() to reach the start of the loop, but we need it
to exit the loop then print a warning.
If we hit a return statement then don't print any more warnings until
we have exited from all the loops.
The one false positive that I saw was code like this:
while (readl() & BAR) {
frob();
if (x)
goto end;
}
<-- can only reach here if readl() succeeds.
frob();
end:
wow_much_frob();
It's not a forever loop because of the goto.
regards,
dan carpenter
>From a05392caf5ea7444aa1d43b6d6a140a08fad4079 Mon Sep 17 00:00:00 2001
From: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Date: Tue, 28 Jan 2014 03:13:47 +0300
Subject: [PATCH 1/2] flow: add a STMT_HOOK_AFTER hook
It's like the other STMT_HOOK but it is called after a statement has been
processed instead of before.
Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
---
smatch.h | 1 +
smatch_flow.c | 1 +
smatch_hooks.c | 3 +++
3 files changed, 5 insertions(+)
diff --git a/smatch.h b/smatch.h
index 717612ac7cb3..a4e88b217dba 100644
--- a/smatch.h
+++ b/smatch.h
@@ -81,6 +81,7 @@ DECLARE_PTR_LIST(tracker_list, struct tracker);
enum hook_type {
EXPR_HOOK,
STMT_HOOK,
+ STMT_HOOK_AFTER,
SYM_HOOK,
STRING_HOOK,
DECLARATION_HOOK,
diff --git a/smatch_flow.c b/smatch_flow.c
index 3705a9415c55..bfe06ad58b5b 100644
--- a/smatch_flow.c
+++ b/smatch_flow.c
@@ -782,6 +782,7 @@ void __split_stmt(struct statement *stmt)
__split_expr(stmt->range_high);
break;
}
+ __pass_to_client(stmt, STMT_HOOK_AFTER);
out:
__process_post_op_stack();
}
diff --git a/smatch_hooks.c b/smatch_hooks.c
index c471d75ee914..57e39826a1e9 100644
--- a/smatch_hooks.c
+++ b/smatch_hooks.c
@@ -48,6 +48,9 @@ void add_hook(void *func, enum hook_type type)
case STMT_HOOK:
container->data_type = STMT_PTR;
break;
+ case STMT_HOOK_AFTER:
+ container->data_type = STMT_PTR;
+ break;
case SYM_HOOK:
container->data_type = EXPR_PTR;
break;
--
1.8.0.rc0.18.gf84667d.dirty
>From 867696d037e997ffa0b07ed84cb825e0479dcf7e Mon Sep 17 00:00:00 2001
From: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Date: Tue, 28 Jan 2014 03:19:36 +0300
Subject: [PATCH 2/2] *new* readl_infinite_loops: check for hotplug bugs which
could cause hangs
I've a device is pulled then readl() returns ((uint)-1). A lot of code
doesn't check for that and does:
while (readl() & 0x80)
;
If the device has been pulled then that will loop forever. Thanks to
Joe Lawrence, for suggesting this.
Reported-by: Joe Lawrence <joe.lawrence@xxxxxxxxxxx>
Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
---
check_list.h | 1 +
check_readl_infinite_loops.c | 157 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 158 insertions(+)
create mode 100644 check_readl_infinite_loops.c
diff --git a/check_list.h b/check_list.h
index 5983330ada26..fd4d5b4482a1 100644
--- a/check_list.h
+++ b/check_list.h
@@ -96,6 +96,7 @@ CK(check_missing_break)
CK(check_array_condition)
CK(check_struct_type)
CK(check_cast_assign)
+CK(check_readl_infinite_loops)
/* <- your test goes here */
/* CK(register_template) */
diff --git a/check_readl_infinite_loops.c b/check_readl_infinite_loops.c
new file mode 100644
index 000000000000..7915330caad0
--- /dev/null
+++ b/check_readl_infinite_loops.c
@@ -0,0 +1,157 @@
+/*
+ * smatch/check_readl_hotplug.c
+ *
+ * Copyright (C) 2014 Oracle.
+ *
+ * Licensed under the Open Software License version 1.1
+ *
+ */
+
+#include "smatch.h"
+#include "smatch_extra.h"
+
+static int my_id;
+
+STATE(readl);
+STATE(readl_ff);
+STATE(readl_00);
+
+DECLARE_PTR_LIST(state_stack, struct smatch_state);
+struct state_stack *state_at_start;
+
+static int readl_has_been_called;
+static int returned;
+
+static int is_readl_call(struct expression *expr)
+{
+ struct symbol *sym;
+
+ expr = strip_expr(expr);
+ if (expr->type != EXPR_CALL)
+ return 0;
+ if (expr->fn->type != EXPR_SYMBOL)
+ return 0;
+ sym = expr->fn->symbol;
+ if (!sym || !sym->ident)
+ return 0;
+ if (strcmp(sym->ident->name, "readl") != 0)
+ return 0;
+ return 1;
+}
+
+static int is_readl(struct expression *expr)
+{
+ if (is_readl_call(expr))
+ return 1;
+ if (get_state_expr(my_id, expr) == &readl)
+ return 1;
+ return 0;
+}
+
+static void match_assign(struct expression *expr)
+{
+ if (is_readl(expr->right))
+ set_state_expr(my_id, expr->left, &readl);
+ else if (get_state_expr(my_id, expr->left))
+ set_state_expr(my_id, expr->left, &undefined);
+}
+
+static int condition_depends_on_readl(struct expression *expr)
+{
+ if (expr->type == EXPR_BINOP) {
+ if (condition_depends_on_readl(expr->left))
+ return 1;
+ if (condition_depends_on_readl(expr->right))
+ return 1;
+ return 0;
+ }
+ if (is_readl(expr))
+ return 1;
+ return 0;
+}
+
+static void check_condition(struct expression *expr)
+{
+ if (expr->op != '&')
+ return;
+ if (!condition_depends_on_readl(expr))
+ return;
+ readl_has_been_called = 1;
+ set_true_false_states(my_id, "depends on", NULL, &readl_ff, &readl_00);
+}
+
+static void match_return(struct expression *expr)
+{
+
+ if (__inline_fn)
+ return;
+ returned = 1;
+#if 0
+ struct smatch_state *tmp;
+
+ if (!readl_has_been_called)
+ return;
+
+ FOR_EACH_PTR(state_at_start, tmp) {
+ REPLACE_CURRENT_PTR(tmp, NULL);
+ }
+#endif
+}
+
+static void push_state_at_start(struct smatch_state *state)
+{
+ add_ptr_list(&state_at_start, state);
+}
+
+static struct smatch_state *pop_state_at_start(void)
+{
+ struct smatch_state *state;
+
+ state = last_ptr_list((struct ptr_list *)state_at_start);
+ delete_ptr_list_last((struct ptr_list **)&state_at_start);
+ return state;
+}
+
+static void before_loop(struct statement *stmt)
+{
+ struct smatch_state *state;
+
+ if (!stmt || stmt->type != STMT_ITERATOR)
+ return;
+ if (ptr_list_empty(state_at_start))
+ returned = 0;
+ state = get_state(my_id, "depends on", NULL);
+ push_state_at_start(state);
+}
+
+static void after_loop(struct statement *stmt)
+{
+ struct smatch_state *old_state;
+
+ if (!stmt || stmt->type != STMT_ITERATOR)
+ return;
+ old_state = pop_state_at_start();
+ if (old_state == &readl_00)
+ return;
+ if (returned)
+ return;
+ if (get_state(my_id, "depends on", NULL) != &readl_00)
+ return;
+ sm_msg("warn: this loop depends on readl() succeeding");
+}
+
+void check_readl_infinite_loops(int id)
+{
+ if (option_project != PROJ_KERNEL)
+ return;
+
+ my_id = id;
+
+ add_hook(match_assign, ASSIGNMENT_HOOK);
+ add_hook(check_condition, CONDITION_HOOK);
+
+ add_hook(&match_return, RETURN_HOOK);
+
+ add_hook(before_loop, STMT_HOOK);
+ add_hook(after_loop, STMT_HOOK_AFTER);
+}
--
1.8.0.rc0.18.gf84667d.dirty
drivers/iommu/amd_iommu.c:802 amd_iommu_int_thread() warn: this loop depends on readl() succeeding
drivers/iommu/dmar.c:1004 dmar_disable_qi() warn: this loop depends on readl() succeeding
drivers/iommu/dmar.c:1004 dmar_disable_qi() warn: this loop depends on readl() succeeding
drivers/iommu/dmar.c:1272 dmar_fault() warn: this loop depends on readl() succeeding
drivers/iommu/intel-iommu.c:975 iommu_flush_write_buffer() warn: this loop depends on readl() succeeding
drivers/iommu/intel-iommu.c:975 iommu_flush_write_buffer() warn: this loop depends on readl() succeeding
drivers/iommu/intel-iommu.c:1183 iommu_disable_protect_mem_regions() warn: this loop depends on readl() succeeding
drivers/iommu/intel-iommu.c:1183 iommu_disable_protect_mem_regions() warn: this loop depends on readl() succeeding
drivers/iommu/intel-iommu.c:1216 iommu_disable_translation() warn: this loop depends on readl() succeeding
drivers/iommu/intel-iommu.c:1216 iommu_disable_translation() warn: this loop depends on readl() succeeding
drivers/iommu/intel_irq_remapping.c:499 iommu_disable_irq_remapping() warn: this loop depends on readl() succeeding
drivers/iommu/intel_irq_remapping.c:499 iommu_disable_irq_remapping() warn: this loop depends on readl() succeeding
drivers/usb/gadget/mv_udc_core.c:196 process_ep_req() warn: this loop depends on readl() succeeding
drivers/usb/early/ehci-dbgp.c:1072 kgdbdbgp_reader_thread() warn: this loop depends on readl() succeeding
drivers/net/ethernet/intel/ixgb/ixgb_hw.c:777 ixgb_read_phy_reg() warn: this loop depends on readl() succeeding
drivers/net/ethernet/intel/ixgb/ixgb_hw.c:804 ixgb_read_phy_reg() warn: this loop depends on readl() succeeding
drivers/net/ethernet/intel/ixgb/ixgb_hw.c:873 ixgb_write_phy_reg() warn: this loop depends on readl() succeeding
drivers/net/ethernet/intel/ixgb/ixgb_hw.c:900 ixgb_write_phy_reg() warn: this loop depends on readl() succeeding
drivers/net/ethernet/natsemi/ns83820.c:1532 ns83820_do_reset() warn: this loop depends on readl() succeeding
drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c:219 dwmac_dma_flush_tx_fifo() warn: this loop depends on readl() succeeding
drivers/net/ethernet/amd/amd8111e.c:129 amd8111e_read_phy() warn: this loop depends on readl() succeeding
drivers/net/ethernet/amd/amd8111e.c:159 amd8111e_write_phy() warn: this loop depends on readl() succeeding
drivers/net/ethernet/amd/amd8111e.c:818 amd8111e_rx_poll() warn: this loop depends on readl() succeeding
drivers/net/ethernet/brocade/bna/bfa_ioc_ct.c:882 bfa_ioc_ct2_pll_init() warn: this loop depends on readl() succeeding
drivers/net/ethernet/brocade/bna/bfa_ioc.c:1176 bfa_ioc_hw_sem_init() warn: this loop depends on readl() succeeding
drivers/net/ethernet/sun/cassini.c:3508 cas_start_dma() warn: this loop depends on readl() succeeding
drivers/net/ethernet/sun/sunhme.c:650 set_happy_link_modes() warn: this loop depends on readl() succeeding
drivers/net/ethernet/adaptec/starfire.c:1541 netdev_poll() warn: this loop depends on readl() succeeding
drivers/dma/dw/core.c:207 dwc_chan_disable() warn: this loop depends on readl() succeeding
drivers/dma/dw/core.c:1176 dwc_free_chan_resources() warn: this loop depends on readl() succeeding
drivers/dma/dw/core.c:1471 dw_dma_off() warn: this loop depends on readl() succeeding
drivers/input/serio/altera_ps2.c:66 altera_ps2_open() warn: this loop depends on readl() succeeding
drivers/media/pci/ddbridge/ddbridge-core.c:1011 input_tasklet() warn: this loop depends on readl() succeeding
drivers/media/pci/ddbridge/ddbridge-core.c:1356 flashio() warn: this loop depends on readl() succeeding
drivers/media/pci/ddbridge/ddbridge-core.c:1376 flashio() warn: this loop depends on readl() succeeding
drivers/spi/spi-imx.c:421 mx31_reset() warn: this loop depends on readl() succeeding
drivers/video/neofb.c:485 neo2200_sync() warn: this loop depends on readl() succeeding
drivers/infiniband/hw/amso1100/c2_qp.c:737 c2_activity() warn: this loop depends on readl() succeeding
drivers/infiniband/hw/nes/nes_utils.c:403 nes_read16_eeprom() warn: this loop depends on readl() succeeding
drivers/isdn/hisax/telespci.c:130 write_fifo_isac() warn: this loop depends on readl() succeeding
drivers/isdn/hisax/telespci.c:47 readisac() warn: this loop depends on readl() succeeding
drivers/isdn/hisax/telespci.c:64 writeisac() warn: this loop depends on readl() succeeding
drivers/isdn/hisax/telespci.c:112 read_fifo_isac() warn: this loop depends on readl() succeeding
drivers/isdn/hisax/telespci.c:80 readhscx() warn: this loop depends on readl() succeeding
drivers/isdn/hisax/telespci.c:96 writehscx() warn: this loop depends on readl() succeeding
drivers/isdn/hisax/telespci.c:147 read_fifo_hscx() warn: this loop depends on readl() succeeding
drivers/isdn/hisax/telespci.c:165 write_fifo_hscx() warn: this loop depends on readl() succeeding
drivers/scsi/3w-sas.c:1200 twl_interrupt() warn: this loop depends on readl() succeeding
drivers/scsi/3w-sas.c:1349 twl_reset_sequence() warn: this loop depends on readl() succeeding
drivers/scsi/dpt_i2o.c:2137 adpt_isr() warn: this loop depends on readl() succeeding
drivers/scsi/3w-9xxx.c:1516 twa_poll_status_gone() warn: this loop depends on readl() succeeding
drivers/scsi/bfa/bfa_ioc.c:718 bfa_iocpf_sm_fwcheck_entry() warn: this loop depends on readl() succeeding
drivers/scsi/megaraid.c:1233 issue_scb_block() warn: this loop depends on readl() succeeding
drivers/scsi/megaraid.c:1391 megaraid_isr_memmapped() warn: this loop depends on readl() succeeding
drivers/scsi/arcmsr/arcmsr_hba.c:2474 arcmsr_polling_hbc_ccbdone() warn: this loop depends on readl() succeeding
drivers/atm/idt77252.c:166 waitfor_idle() warn: this loop depends on readl() succeeding
drivers/atm/nicstarmac.c:171 read_eprom_byte() warn: this loop depends on readl() succeeding
drivers/atm/nicstarmac.c:170 read_eprom_byte() warn: this loop depends on readl() succeeding
drivers/atm/nicstarmac.c:215 nicstar_init_eprom() warn: this loop depends on readl() succeeding
drivers/atm/nicstar.c:324 ns_read_sram() warn: this loop depends on readl() succeeding
drivers/atm/nicstar.c:341 ns_write_sram() warn: this loop depends on readl() succeeding
drivers/atm/nicstar.c:452 ns_init_card() warn: this loop depends on readl() succeeding
drivers/atm/nicstar.c:1036 push_rxbufs() warn: this loop depends on readl() succeeding
drivers/atm/nicstar.c:1438 ns_close() warn: this loop depends on readl() succeeding
drivers/atm/nicstar.c:2812 ns_phy_put() warn: this loop depends on readl() succeeding
drivers/atm/nicstar.c:2827 ns_phy_get() warn: this loop depends on readl() succeeding
drivers/atm/he.c:204 he_readl_internal() warn: this loop depends on readl() succeeding
drivers/atm/he.c:188 he_writel_internal() warn: this loop depends on readl() succeeding
drivers/atm/he.c:2357 he_close() warn: this loop depends on readl() succeeding
