From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66]) by mx.groups.io with SMTP id smtpd.web10.8089.1637649686641789137 for ; Mon, 22 Nov 2021 22:41:27 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@posteo.de header.s=2017 header.b=jcHaSkN2; spf=pass (domain: posteo.de, ip: 185.67.36.66, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [89.146.220.130]) by mout02.posteo.de (Postfix) with ESMTPS id 70F76240106 for ; Tue, 23 Nov 2021 07:41:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1637649683; bh=py3lAokZb4NsQthLd1K2TXTFx4gw8lzhVoqpjtvpX0g=; h=Date:From:To:Cc:Subject:From; b=jcHaSkN2aNeHfBF6SqndG4CdhVZMpE2oRUutJdBeFFplKXX2+SxLRLzJHI5kbreew mPEkLHGLEkDxo0YyD0cTqTWy07HQYY1T+mtq8HaRNS4jfQgsRzCwghbquBRIWzBG6Y yzFCHO27WkTIXj3jiWgsblvP67ZDsrlcIN6BSYa0i9ZRjFLqDmG/xxT3Bf3ZWPbJVT mUXVOVDlwaFh+8BHWBbOdMGtF+GzmoLa1fqQsJ62y9nNCUhSkoTKldhourfHbAm8Eu 88b5a7Aj+kR3hxzRGQPxe8DJ6ofhCN3FPZrsy6bAtTedQVn4FND4+P0xQWHiJMC9n+ RJYqcJe9FC/vQ== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4Hyvg00PVmz9rxG; Tue, 23 Nov 2021 07:41:15 +0100 (CET) Date: Tue, 23 Nov 2021 06:41:14 +0000 (UTC) From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= To: devel@edk2.groups.io, mcb30@ipxe.org Cc: michael.d.kinney@intel.com, krichanov@ispras.ru, gaoliming@byosoft.com.cn, "Liu, Zhiguang" , vit9696@protonmail.com Message-ID: In-Reply-To: <8d9101ac-fbda-eecf-950e-692fa375c3f9@ipxe.org> References: <20211119090529.2865-1-krichanov@ispras.ru> <8d9101ac-fbda-eecf-950e-692fa375c3f9@ipxe.org> Subject: Re: [edk2-devel] [PATCH] MdePkg: DebugLib: Compilation fix for clang-13. MIME-Version: 1.0 X-Correlation-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 23.11.2021 00:17:30 Michael Brown : > On 22/11/2021 16:42, Michael D Kinney wrote: >> You are also modifying the DebugLib in the paths where ASSERT() macros >> are disabled.=C2=A0 When they are disabled, we want all code/data associ= ated >> with ASSERT() to be removed by the optimizing compiler/linker.=C2=A0 The= source >> code change appears to force a reference to a variable/expression.=C2=A0= Does >> this have any size impact to any of the toolchains when ASSERT() is >> disabled?=C2=A0 Can you provide the size comparison before and after thi= s >> change? > > I would very strongly recommend having the non-debug version of the macro= use something like: > > #define ASSERT(Expression) do {=C2=A0=C2=A0 \ > =C2=A0=C2=A0=C2=A0=C2=A0 if (FALSE) {=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 \ > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (VOID) (Expression);=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 \ > =C2=A0=C2=A0=C2=A0=C2=A0 }=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 \ > =C2=A0=C2=A0 } while (FALSE) > > Without the "if (FALSE)", you will find that an expression that may have = side-effects (e.g. by calling an external function) will result in executab= le code being generated. In theory +1, but don't some compilers generate "unreachable code" warnings= for this? I unfortunately cannot check them all right now. Maybe guards pu= sh/pop'ing the warning for that need to be defined, that are only allowed t= o be used with explicit documentation why they are necessary? Best regards, Marvin > > Michael > > >=20