From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on gnuweeb.org X-Spam-Level: X-Spam-Status: No, score=-1.8 required=5.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,NO_DNS_FOR_FROM, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from [192.168.148.80] (unknown [182.2.43.220]) by gnuweeb.org (Postfix) with ESMTPSA id AB1367E312; Sun, 3 Apr 2022 17:11:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gnuweeb.org; s=default; t=1649005901; bh=1rC6UHt0mHextJbrBjiOVj+mspQiXUc+hY7TYceWF9w=; h=Date:To:Cc:References:From:Subject:In-Reply-To:From; b=SUYqG/yl9vJAQyJZf1pDM93sT7CGNqHRaq/BR0J9H350N4KFqlF4GeIq89LxMjZv7 4vAxcSqQtFTBDyR5Ja+2TxP7xLYknukDns160e8kQUbOMWvW/rwm/F9Tg1K0j4sQhY x3QqFyuD4ZDm7xTSLSZWLCvTQaT5A2SG7yzX5/Ocz9jxapf857FMkgcRKQqwDkUI6j oTfbb3iqdpzTQ7T91/nMO+NfQkuLKwgfMjj3MUlAolNnAK63On/JEpgHDCjN9TY+Tn wVEntNBZjd521CNnEg1/iq58GSBZMmgl2+m0wUBS+kN6bN7WXPsDudumeFV3zO+Kkf DIcOpuZ9o3WRQ== Message-ID: <3ceb39a4-f592-68b6-b5e7-a33a2b33a402@gnuweeb.org> Date: Mon, 4 Apr 2022 00:11:31 +0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Content-Language: en-US To: Thomas Gleixner , Borislav Petkov Cc: Alviro Iskandar Setiawan , Dave Hansen , "H. Peter Anvin" , Ingo Molnar , Tony Luck , Yazen Ghannam , Linux Edac Mailing List , Linux Kernel Mailing List , Stable Kernel , GNU/Weeb Mailing List , x86 Mailing List , David Laight , Jiri Hladky References: <20220329104705.65256-1-ammarfaizi2@gnuweeb.org> <20220329104705.65256-2-ammarfaizi2@gnuweeb.org> <87zgl2ksu3.ffs@tglx> From: Ammar Faizi Subject: Re: [PATCH v6 1/2] x86/delay: Fix the wrong asm constraint in `delay_loop()` In-Reply-To: <87zgl2ksu3.ffs@tglx> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: On 4/3/22 11:57 PM, Thomas Gleixner wrote: > On Tue, Mar 29 2022 at 17:47, Ammar Faizi wrote: >> The asm constraint does not reflect that the asm statement can modify >> the value of @loops. But the asm statement in delay_loop() does modify >> the @loops. >> >> Specifiying the wrong constraint may lead to undefined behavior, it may >> clobber random stuff (e.g. local variable, important temporary value in >> regs, etc.). This is especially dangerous when the compiler decides to >> inline the function and since it doesn't know that the value gets >> modified, it might decide to use it from a register directly without >> reloading it. >> >> Fix this by changing the constraint from "a" (as an input) to "+a" (as >> an input and output). > > This analysis is plain wrong. The assembly code operates on a register > and not on memory: > asm volatile( > " test %0,%0 \n" > " jz 3f \n" > " jmp 1f \n" > > ".align 16 \n" > "1: jmp 2f \n" > > ".align 16 \n" > "2: dec %0 \n" > " jnz 2b \n" > "3: dec %0 \n" > > : /* we don't need output */ > ----> :"a" (loops) > > This tells the compiler to use [RE]AX and initialize it from the > variable 'loops'. It's never written back because all '%0' in the above > assembly are substituted with [RE]AX. This also tells the compiler that > the inline assembly clobbers [RE]AX and that's all it needs to know. Hi Thomas, Thanks for taking a look. I doubt about your sentence "This also tells the compiler that the inline assembly clobbers [RE]AX". How come it tells the compiler that the inline ASM clobbers [RE]AX? That's an input constraint. Doesn't that mean it is read-only for the ASM statement? That means the compiler is allowed to assume [RE]AX doesn't change inside the ASM statement. Those `dec`s do really change the [RE]AX. Please review this again. Thanks! -- Ammar Faizi