From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x243.google.com (mail-wm0-x243.google.com [IPv6:2a00:1450:400c:c09::243]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D0F6C21D046A2 for ; Tue, 19 Sep 2017 09:59:37 -0700 (PDT) Received: by mail-wm0-x243.google.com with SMTP id m127so226821wmm.0 for ; Tue, 19 Sep 2017 10:02:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=qyEAKOnwRDy8e7GljpdrokyvE1q2cLNIPg/L9TQmN8Y=; b=G4MpiADtZb6B3CarJUFaIRE6pFfyNYfu2yOInNUDpwBJyrcncmI9Y5xpEI1NgDpDyz fN0YVxVdU0ofBb4WxTM5DSC2hPZDAYdKL5Q3yAPFcwAzNyApgh/DJmX8cQW76at9abJ3 blv0vfUrrB9Cu2I3YQ5Nrljorng8osKlpGwexddl08wB8j96iKAB4maRn2KtA0PpMJ1R l4rYKWFPreXDNCmCTNEEcV7vVlOL+/Zxgb6jAqjOpDS0nUtspjaTrT64akd7JvUQ08mI HPi/KS4aqN+/TTxvDez80U+bGN3j6ulR/hEAJ3E81kNqpAor9DxM2XUhz6auBI/qvl0a k3+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:subject:to:cc:references:from:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=qyEAKOnwRDy8e7GljpdrokyvE1q2cLNIPg/L9TQmN8Y=; b=drQa8J9kNckuhsev4YyM3wofRBCPjWE7nzgdMJ0NGBQGZ03jtE3Y2NsHb2TuQq7N6H SJY8gaLmeUVUQIA0r/uXhoq9eNoyBmCnRN8b4IAFHYF9ToxpijEHkn2QZyHLK4ZE/HIk gxTTqwuc5o0x2N4d3mrX6y99GNYQtGTcWsaxipoeBjscvoMclUYIiGU1tSI2k+xFk9CK jU2yNJJ9UY9OVCrOpMJMm7PPEnatzt/e7e053+s4qJ+6kq2fKgQS5K708s9fNESZ0NGd 3MjQzXWz6zGvn/d/Rfa2XPtkVxQKL8SsXlfFKllxZ/SN8xK3eYPS1ZmwUucPpL9neFhp dNqw== X-Gm-Message-State: AHPjjUglyU7bRqKxicG6nVRv2Y2RxdnbGvhNobX0hcdZqBrBvs5mfhf2 aDlzN9lUJ9ByVindSPayGaA= X-Google-Smtp-Source: AOwi7QCgIXEBSCTAfTqMavRcAy55hiukxTs7cCz5xMv/ju6CBRYdIjDsc1bInW47aM95525gzxzLWA== X-Received: by 10.28.232.6 with SMTP id f6mr1732643wmh.153.1505840560860; Tue, 19 Sep 2017 10:02:40 -0700 (PDT) Received: from [192.168.10.165] (dynamic-adsl-78-12-246-117.clienti.tiscali.it. [78.12.246.117]) by smtp.googlemail.com with ESMTPSA id r21sm2544865wmd.26.2017.09.19.10.02.39 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 19 Sep 2017 10:02:40 -0700 (PDT) Sender: Paolo Bonzini To: Hao Wu , edk2-devel@lists.01.org Cc: Eric Dong , Star Zeng References: <20170919114351.18448-1-hao.a.wu@intel.com> <20170919114351.18448-4-hao.a.wu@intel.com> From: Paolo Bonzini Message-ID: <55cbb690-9ae1-56ed-d5a5-e100d9dd98da@redhat.com> Date: Tue, 19 Sep 2017 19:02:39 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170919114351.18448-4-hao.a.wu@intel.com> Subject: Re: [PATCH 3/6] MdeModulePkg/Tpl: Fix negative value left shift X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Sep 2017 16:59:38 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 19/09/2017 13:43, Hao Wu wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=695 > > Within function CoreRestoreTpl(), left shift a negative value -2 is used > in: > "while (((-2 << NewTpl) & gEventPending) != 0) {" > > which involves undefined behavior. > > According to the C11 spec, Section 6.5.7: >> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated >> bits are filled with zeros. If E1 has an unsigned type, the value >> of the result is E1 * 2^E2 , reduced modulo one more than the >> maximum value representable in the result type. If E1 has a signed >> type and nonnegative value, and E1 * 2^E2 is representable in the >> result type, then that is the resulting value; otherwise, the >> behavior is undefined. > > This commit explicitly cast '-2' with UINTN to resolve this issue. > > Cc: Steven Shi > Cc: Star Zeng > Cc: Eric Dong > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Hao Wu > --- > MdeModulePkg/Core/Dxe/Event/Tpl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Core/Dxe/Event/Tpl.c b/MdeModulePkg/Core/Dxe/Event/Tpl.c > index 8ad0a33701..8c50f61117 100644 > --- a/MdeModulePkg/Core/Dxe/Event/Tpl.c > +++ b/MdeModulePkg/Core/Dxe/Event/Tpl.c > @@ -123,7 +123,7 @@ CoreRestoreTpl ( > // > // Dispatch any pending events > // > - while (((-2 << NewTpl) & gEventPending) != 0) { > + while (((((UINTN)-2) << NewTpl) & gEventPending) != 0) { > gEfiCurrentTpl = (UINTN) HighBitSet64 (gEventPending); > if (gEfiCurrentTpl < TPL_HIGH_LEVEL) { > CoreSetInterruptState (TRUE); > Maybe: for (;;) { PendingTpl = (UINTN) HighBitSet64 (gEventPending); if (NewTpl >= PendingTpl) { break; } gEfiCurrentTpl = PendingTpl; } This is much more readable, and HighBitSet64 should be efficient on most modern processors. Paolo