From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934511AbcJMR6a (ORCPT ); Thu, 13 Oct 2016 13:58:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45016 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934428AbcJMR6V (ORCPT ); Thu, 13 Oct 2016 13:58:21 -0400 Subject: Re: Another gcc corruption bug (was Re: [PATCH] [RFC] x86: avoid -mtune=atom for objtool warnings) To: Josh Poimboeuf , Arnd Bergmann References: <20161010125709.1870563-1-arnd@arndb.de> <20161011150541.opini6gbolmnpzy5@treble> <20161011155146.icyl3zewdvmms2h2@treble> <2252957.Vm1BYSSRqP@wuerfel> <20161013124636.6ze7ff4qncbmtonv@treble> Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, Emese Revfy From: Denys Vlasenko Message-ID: <1e64229d-4383-8211-ee4b-155d185abb30@redhat.com> Date: Thu, 13 Oct 2016 19:57:41 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20161013124636.6ze7ff4qncbmtonv@treble> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Thu, 13 Oct 2016 17:57:44 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/13/2016 02:46 PM, Josh Poimboeuf wrote: > On Tue, Oct 11, 2016 at 10:38:42PM +0200, Arnd Bergmann wrote: >> On Tuesday, October 11, 2016 10:51:46 AM CEST Josh Poimboeuf wrote: >>> Notice how it just falls off the end of the function. We had a similar >>> bug before: >>> >>> https://lkml.kernel.org/r/20160413033649.7r3msnmo3trtq47z@treble >> >> I remember that nightmare :( >> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 >>> >>> I'm not sure yet if this is the same gcc bug or a different one. Maybe >>> it's related to the new GCC_PLUGIN_SANCOV? >> >> I've reduced one of the test cases to this now: >> >> /* gcc-6 -O2 -fno-strict-aliasing -fno-reorder-blocks -fno-omit-frame-pointer -Wno-pointer-sign -fsanitize-coverage=trace-pc -Wall -Werror -c snic_res.c -o snic_res.o */ >> typedef int spinlock_t; >> extern unsigned int ioread32(void *); >> struct vnic_wq_ctrl { >> unsigned int error_status; >> }; >> struct vnic_wq { >> struct vnic_wq_ctrl *ctrl; >> } mempool_t; >> struct snic { >> unsigned int wq_count; >> __attribute__ ((__aligned__)) struct vnic_wq wq[1]; >> spinlock_t wq_lock[1]; >> }; >> unsigned int snic_log_q_error_err_status; >> void snic_log_q_error(struct snic *snic) >> { >> unsigned int i; >> for (i = 0; i < snic->wq_count; i++) >> snic_log_q_error_err_status = >> ioread32(&snic->wq[i].ctrl->error_status); >> } >> >> which gets compiled into >> >> 0000000000000000 : >> 0: 55 push %rbp >> 1: 48 89 e5 mov %rsp,%rbp >> 4: 53 push %rbx >> 5: 48 89 fb mov %rdi,%rbx >> 8: 48 83 ec 08 sub $0x8,%rsp >> c: e8 00 00 00 00 callq 11 >> d: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 >> 11: 8b 03 mov (%rbx),%eax >> 13: 85 c0 test %eax,%eax >> 15: 75 11 jne 28 >> 17: 48 83 c4 08 add $0x8,%rsp >> 1b: 5b pop %rbx >> 1c: 5d pop %rbp >> 1d: e9 00 00 00 00 jmpq 22 >> 1e: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 >> 22: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1) >> 28: e8 00 00 00 00 callq 2d >> 29: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 >> 2d: 48 8b 7b 10 mov 0x10(%rbx),%rdi >> 31: e8 00 00 00 00 callq 36 >> 32: R_X86_64_PC32 ioread32-0x4 >> 36: 89 05 00 00 00 00 mov %eax,0x0(%rip) # 3c >> 38: R_X86_64_PC32 snic_log_q_error_err_status-0x4 >> 3c: 83 3b 01 cmpl $0x1,(%rbx) >> 3f: 76 d6 jbe 17 >> 41: e8 00 00 00 00 callq 46 >> 42: R_X86_64_PC32 __sanitizer_cov_trace_pc-0x4 > > I opened a bug: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77966 > Surprisingly, it's really "not a bug". The only way you can end up in this branch is if you have a bug and run off the end of wq[1] array member: i.e. if snic->wq_count >= 2. (See gcc BZ for smaller example) It's debatable whether it's okay for gcc to just let buggy code to run off and execute something random. It is surely surprising, and not debug-friendly. An option to emit a crashing instruction (HLT, INT3, that sort of thing) instead of just stopping code generation might be useful.