From cc80db955d0551c2456692da6176530dd27e08ed Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Fri, 1 Oct 2021 16:01:47 +0200 Subject: executor: check for single-line compound statements Historically the code base does not use single-line compound statements ({} around single-line blocks). But there are few precedents creeped into already. Add a check to keep the code base consistent. --- executor/common.h | 6 ++---- executor/common_kvm_ppc64.h | 3 +-- executor/common_linux.h | 3 +-- executor/common_usb.h | 3 +-- executor/common_usb_netbsd.h | 3 +-- executor/executor.cc | 3 +-- executor/style_test.go | 26 ++++++++++++++++++++++++-- pkg/csource/generated.go | 18 ++++++------------ 8 files changed, 37 insertions(+), 28 deletions(-) diff --git a/executor/common.h b/executor/common.h index 3c817a7e8..d9367fd38 100644 --- a/executor/common.h +++ b/executor/common.h @@ -102,9 +102,8 @@ static void segv_handler(int sig, siginfo_t* info, void* ctx) // address of the faulting instruction rather than zero as other // operating systems seem to do. However, such faults should always be // ignored. - if (sig == SIGBUS) { + if (sig == SIGBUS) valid = 1; - } #endif if (skip && valid) { debug("SIGSEGV on %p, skipping\n", (void*)addr); @@ -689,9 +688,8 @@ static void loop(void) if (current_time_ms() - start < program_timeout_ms) continue; #else - if (current_time_ms() - start < /*{{{PROGRAM_TIMEOUT_MS}}}*/) { + if (current_time_ms() - start < /*{{{PROGRAM_TIMEOUT_MS}}}*/) continue; - } #endif debug("killing hanging pid %d\n", pid); kill_and_wait(pid, &status); diff --git a/executor/common_kvm_ppc64.h b/executor/common_kvm_ppc64.h index 1063e587b..82d781772 100644 --- a/executor/common_kvm_ppc64.h +++ b/executor/common_kvm_ppc64.h @@ -195,9 +195,8 @@ static volatile long syz_kvm_setup_cpu(volatile long a0, volatile long a1, volat memreg.guest_phys_addr = i * page_size; memreg.memory_size = page_size; memreg.userspace_addr = (uintptr_t)host_mem + i * page_size; - if (ioctl(vmfd, KVM_SET_USER_MEMORY_REGION, &memreg)) { + if (ioctl(vmfd, KVM_SET_USER_MEMORY_REGION, &memreg)) return -1; - } } struct kvm_regs regs; diff --git a/executor/common_linux.h b/executor/common_linux.h index d80883729..972376165 100644 --- a/executor/common_linux.h +++ b/executor/common_linux.h @@ -2512,9 +2512,8 @@ static bool process_command_pkt(int fd, char* buf, ssize_t buf_size) { struct hci_command_hdr* hdr = (struct hci_command_hdr*)buf; if (buf_size < (ssize_t)sizeof(struct hci_command_hdr) || - hdr->plen != buf_size - sizeof(struct hci_command_hdr)) { + hdr->plen != buf_size - sizeof(struct hci_command_hdr)) failmsg("process_command_pkt: invalid size", "suze=%zx", buf_size); - } switch (hdr->opcode) { case HCI_OP_WRITE_SCAN_ENABLE: { diff --git a/executor/common_usb.h b/executor/common_usb.h index 35793f4d7..361605b0e 100644 --- a/executor/common_usb.h +++ b/executor/common_usb.h @@ -117,9 +117,8 @@ static struct usb_device_index* add_usb_index(int fd, const char* dev, size_t de static struct usb_device_index* lookup_usb_index(int fd) { for (int i = 0; i < USB_MAX_FDS; i++) { - if (__atomic_load_n(&usb_devices[i].fd, __ATOMIC_ACQUIRE) == fd) { + if (__atomic_load_n(&usb_devices[i].fd, __ATOMIC_ACQUIRE) == fd) return &usb_devices[i].index; - } } return NULL; } diff --git a/executor/common_usb_netbsd.h b/executor/common_usb_netbsd.h index 138b50557..986555321 100644 --- a/executor/common_usb_netbsd.h +++ b/executor/common_usb_netbsd.h @@ -268,8 +268,7 @@ static volatile long syz_usb_connect_impl(int fd, uint64 speed, uint64 dev_len, if ((req.u.ctrl.bmRequestType & USB_TYPE_MASK) == USB_TYPE_STANDARD && req.u.ctrl.bRequest == USB_REQ_SET_CONFIGURATION) { - // TODO: possibly revisit. - } + } // TODO: possibly revisit. if (response_length > sizeof(data)) response_length = 0; diff --git a/executor/executor.cc b/executor/executor.cc index 6e3214925..25e3be69d 100644 --- a/executor/executor.cc +++ b/executor/executor.cc @@ -1184,9 +1184,8 @@ void execute_call(thread_t* th) } th->fault_injected = false; - if (th->call_props.fail_nth > 0) { + if (th->call_props.fail_nth > 0) th->fault_injected = fault_injected(fail_fd); - } debug("#%d [%llums] <- %s=0x%llx errno=%d ", th->id, current_time_ms() - start_time_ms, call->name, (uint64)th->res, th->reserrno); diff --git a/executor/style_test.go b/executor/style_test.go index 9c83f2536..5c393ea3d 100644 --- a/executor/style_test.go +++ b/executor/style_test.go @@ -32,6 +32,22 @@ if (foo) `, ` if (x + y) debug_dump_data(data, len); +`, + }, + }, + { + pattern: `\) {\n[^\n}]+?\n\t*}\n`, + suppression: "debug|__except", + message: "Don't use single-line compound statements (remove {})", + tests: []string{ + ` +if (foo) { + bar(); +} +`, ` + while (x + y) { + foo(a, y); + } `, }, }, @@ -146,18 +162,24 @@ if (foo) re := regexp.MustCompile(check.pattern) supp := regexp.MustCompile(check.suppression) for _, match := range re.FindAllIndex(data, -1) { - // Locate the last line of the match, that's where we assume the error is. end := match[1] - 1 for end != len(data) && data[end] != '\n' { end++ } - start := end - 1 + // Match suppressions against all lines of the match. + start := match[0] - 1 for start != 0 && data[start-1] != '\n' { start-- } if check.suppression != "" && supp.Match(data[start:end]) { continue } + // Locate the last line of the match, that's where we assume the error is. + start = end - 1 + for start != 0 && data[start-1] != '\n' { + start-- + } + line := bytes.Count(data[:start], []byte{'\n'}) + 1 t.Errorf("\nexecutor/%v:%v: %v\n%s\n", file, line, check.message, data[start:end]) } diff --git a/pkg/csource/generated.go b/pkg/csource/generated.go index 27a7bc132..40d1c1963 100644 --- a/pkg/csource/generated.go +++ b/pkg/csource/generated.go @@ -85,9 +85,8 @@ static void segv_handler(int sig, siginfo_t* info, void* ctx) int skip = __atomic_load_n(&skip_segv, __ATOMIC_RELAXED) != 0; int valid = addr < prog_start || addr > prog_end; #if GOOS_freebsd || (GOOS_test && HOSTGOOS_freebsd) - if (sig == SIGBUS) { + if (sig == SIGBUS) valid = 1; - } #endif if (skip && valid) { debug("SIGSEGV on %p, skipping\n", (void*)addr); @@ -693,9 +692,8 @@ static struct usb_device_index* add_usb_index(int fd, const char* dev, size_t de static struct usb_device_index* lookup_usb_index(int fd) { for (int i = 0; i < USB_MAX_FDS; i++) { - if (__atomic_load_n(&usb_devices[i].fd, __ATOMIC_ACQUIRE) == fd) { + if (__atomic_load_n(&usb_devices[i].fd, __ATOMIC_ACQUIRE) == fd) return &usb_devices[i].index; - } } return NULL; } @@ -4375,9 +4373,8 @@ static struct usb_device_index* add_usb_index(int fd, const char* dev, size_t de static struct usb_device_index* lookup_usb_index(int fd) { for (int i = 0; i < USB_MAX_FDS; i++) { - if (__atomic_load_n(&usb_devices[i].fd, __ATOMIC_ACQUIRE) == fd) { + if (__atomic_load_n(&usb_devices[i].fd, __ATOMIC_ACQUIRE) == fd) return &usb_devices[i].index; - } } return NULL; } @@ -5911,9 +5908,8 @@ static bool process_command_pkt(int fd, char* buf, ssize_t buf_size) { struct hci_command_hdr* hdr = (struct hci_command_hdr*)buf; if (buf_size < (ssize_t)sizeof(struct hci_command_hdr) || - hdr->plen != buf_size - sizeof(struct hci_command_hdr)) { + hdr->plen != buf_size - sizeof(struct hci_command_hdr)) failmsg("process_command_pkt: invalid size", "suze=%zx", buf_size); - } switch (hdr->opcode) { case HCI_OP_WRITE_SCAN_ENABLE: { @@ -7433,9 +7429,8 @@ static volatile long syz_kvm_setup_cpu(volatile long a0, volatile long a1, volat memreg.guest_phys_addr = i * page_size; memreg.memory_size = page_size; memreg.userspace_addr = (uintptr_t)host_mem + i * page_size; - if (ioctl(vmfd, KVM_SET_USER_MEMORY_REGION, &memreg)) { + if (ioctl(vmfd, KVM_SET_USER_MEMORY_REGION, &memreg)) return -1; - } } struct kvm_regs regs; @@ -10573,9 +10568,8 @@ static void loop(void) if (current_time_ms() - start < program_timeout_ms) continue; #else - if (current_time_ms() - start < /*{{{PROGRAM_TIMEOUT_MS}}}*/) { + if (current_time_ms() - start < /*{{{PROGRAM_TIMEOUT_MS}}}*/) continue; - } #endif debug("killing hanging pid %d\n", pid); kill_and_wait(pid, &status); -- cgit mrf-deployment