From mboxrd@z Thu Jan 1 00:00:00 1970 From: Doug Goldstein Subject: Re: [PATCH 2/2] tools: detect appropriate debug optimization level Date: Tue, 29 Mar 2016 12:21:48 -0500 Message-ID: <56FAB9AC.5040301@cardoe.com> References: <1457403820-2949-1-git-send-email-cardoe@cardoe.com> <1457403820-2949-2-git-send-email-cardoe@cardoe.com> <20160308153847.GF31271@citrix.com> <56DEFF22.80004@cardoe.com> <20160308165040.GR31271@citrix.com> <56E9B09A.3020808@cardoe.com> <56F9473C.9090508@cardoe.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6002995481428518183==" Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: George Dunlap Cc: Euan Harris , "xen-devel@lists.xen.org" , Wei Liu , Ian Jackson , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --===============6002995481428518183== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="5wogRi600HRpORcCF1u2CefTR8FQvgfBa" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --5wogRi600HRpORcCF1u2CefTR8FQvgfBa Content-Type: multipart/mixed; boundary="j0uWFjwxlLIuxfmwuCC5ItS1BI6xKHHtm" From: Doug Goldstein To: George Dunlap Cc: Wei Liu , Euan Harris , Stefano Stabellini , Ian Jackson , "xen-devel@lists.xen.org" Message-ID: <56FAB9AC.5040301@cardoe.com> Subject: Re: [Xen-devel] [PATCH 2/2] tools: detect appropriate debug optimization level References: <1457403820-2949-1-git-send-email-cardoe@cardoe.com> <1457403820-2949-2-git-send-email-cardoe@cardoe.com> <20160308153847.GF31271@citrix.com> <56DEFF22.80004@cardoe.com> <20160308165040.GR31271@citrix.com> <56E9B09A.3020808@cardoe.com> <56F9473C.9090508@cardoe.com> In-Reply-To: --j0uWFjwxlLIuxfmwuCC5ItS1BI6xKHHtm Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 3/29/16 6:44 AM, George Dunlap wrote: > On Mon, Mar 28, 2016 at 4:01 PM, Doug Goldstein wro= te: >> On 3/16/16 2:14 PM, Doug Goldstein wrote: >>> On 3/8/16 10:50 AM, Wei Liu wrote: >>>> On Tue, Mar 08, 2016 at 10:34:42AM -0600, Doug Goldstein wrote: >>>>> On 3/8/16 9:38 AM, Wei Liu wrote: >>>>>> On Mon, Mar 07, 2016 at 08:23:40PM -0600, Doug Goldstein wrote: >>>>>>> The build should not use -O0 as that results in miscompilations. = There >>>>>> >>>>>> This needs some (concrete) references. Is that a known issue in gc= c? If >>>>>> so can you reference the bug number? >>>>> >>>>> So its not really a bug in GCC but just the complete lack of >>>>> optimizations in play. inlines aren't inlined. dead code eliminatio= n >>>>> isn't run so things are much bigger. structures aren't padded the s= ame way. >>>>> >>>> >>>> Urgh... >>>> >>>>> This came about from reading reports on the -devel and -user's ML t= hat >>>>> were solved by building Xen with debug=3Dn. I was also striving to = reduce >>>>> the duplication of CFLAGS that are passed on the command line of bu= ilds. >>>>> >>>> >>>> I agree this is a good idea. >>>> >>>>>> >>>>>>> have been a few instances on the ML where users were told to swit= ch >>>>>>> from -O0 to -O1 or -O2 or to set debug=3Dn and their issue went a= way. The >>>>>>> preferred route should be to use -Og if its available, otherwise = use >>>>>>> -O1 which is the default. This change undoes the change from -O1 = to -O0 >>>>>> >>>>>> gcc manual says -O0 is the default. >>>>> >>>>> I wasn't clear about where the 'the default' came from. That's the >>>>> default in the Xen tree (see: config/StdGNU.mk for example but ever= y >>>>> platform has -O1 set). >>>>> >>>> >>>> OK. I thought you're talking about something in the manual. >>>> >>>>>> >>>>>> Not that I disagree with this patch in general, but the commit mes= sage >>>>>> seems a bit misleading. >>>>> >>>>> I can rewrite it. I'd also be willing to change the patch to prefer= -Og >>>>> if its available and use -O0 if its not. >>>>> >>>> >>>> No need to do it now because ... >>>> >>>>>> >>>>>>> in 1166ecf781b1016eaa61f8d5ba4fb1fde9d599b6. >>>>>>> >>>>>> >>>>>> And I have no idea why -O1 confuses the debugger so I've CC'ed Eua= n for >>>>>> more input. >>>>> >>>>> -O1 can optimize things out when you look at them with gdb but -Og = is >>>>> suppose to do the right thing. >>>>> >>>> >>>> .. I don't know much about gcc so I would like to wait for Ian to gi= ve >>>> some input. >>>> >>>> Wei. >>>> >>>>>> >>>>>>> Signed-off-by: Doug Goldstein >>>>>>> --- >>>>>>> CC: Ian Jackson >>>>>>> CC: Stefano Stabellini >>>>>>> CC: Wei Liu >>>>>>> --- >>>>>>> tools/Rules.mk | 3 ++- >>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/tools/Rules.mk b/tools/Rules.mk >>>>>>> index 9ef0b47..ae6b01f 100644 >>>>>>> --- a/tools/Rules.mk >>>>>>> +++ b/tools/Rules.mk >>>>>>> @@ -137,7 +137,8 @@ SHLIB_libxenvchan =3D $(SHDEPS_libxenvchan) = -Wl,-rpath-link=3D$(XEN_LIBVCHAN) >>>>>>> >>>>>>> ifeq ($(debug),y) >>>>>>> # Disable optimizations and enable debugging information for mac= ros >>>>>>> -CFLAGS +=3D -O0 -g3 >>>>>>> +$(call cc-option-add,CFLAGS,CC,-Og) >>>>>>> +CFLAGS +=3D -g3 >>>>>>> # But allow an override to -O0 in case Python enforces -D_FORTIF= Y_SOURCE=3D. >>>>>>> PY_CFLAGS +=3D $(PY_NOOPT_CFLAGS) >>>>>>> endif >>>>>>> -- >>>>>>> 2.4.10 >>>>>>> >>>>> >>>>> >>>>> -- >>>>> Doug Goldstein >>>>> >>>> >>>> >>>> >>> >>> ping? >>> >>> >> >> ping the ping? >=20 > So just reading through the history -- I'm a bit confused why, if -Og > is supposed to "do the right thing", why you didn't add that in this > patch? I did. +$(call cc-option-add,CFLAGS,CC,-Og) That tests to see if the version of GCC you're using supports that flag and if it does adds it. Otherwise it does nothing. Per the README we support down to GCC 4.1.2 and -Og was added in 4.7 I believe. For versions of GCC older than 4.7 this uses the debug=3Dy default of -O1 instead of using -O0 which is known to break in some cases. >=20 > In any case, having debug=3Dy *functioning* should take priority over > having gdb working, so I'm inclined to say that we should take a patch > like this. Exactly why I'd like to see this land. >=20 > -George >=20 --=20 Doug Goldstein --j0uWFjwxlLIuxfmwuCC5ItS1BI6xKHHtm-- --5wogRi600HRpORcCF1u2CefTR8FQvgfBa Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0 iQJ8BAEBCgBmBQJW+rmvXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRBNTM5MEQ2RTNFMTkyNzlCNzVDMzIwOTVB MkJDMDNEQzg3RUQxQkQ0AAoJEKK8A9yH7RvUp1QP/AiTISrMHCcGc1dndqoYwaEW tl5JqNcUCBVu8sgWV5p0x0W2iW5lvlHE8tV6zddH/6SvnBSWLoJ3wo7KJf4R/bMp lUjW0fu0dmfrEgFm41kbCs6uRGkkRGBDUIgS6GzKBQluvVU+faBaiIhzRXkaWS9W YISsnaaMCjA+x58MyCLvxNoBd29Zvra3XVC1XyRSpUL3PezV4nIWZHM5iXnK8Abo Wegph1IMqL5VjHCC66bcge9LF4h/noFXcKuR1iZOy5KgHGICkEKK7Tv5rLJ4wrRW jwsVjR1uhfEQLYS+TmHFOHRx97+z848et/LJHqr1E6ATU2c1ev0gIDDWsAAkcAvv 3qjxwNzRW3t6PejWZ7sYMp8s7d1eXQyfkLhn71P+K9+VFlfDKxcxLVGCTdN8WrCw 2sd+7/Fu0yY3bNC4c9SZmT6G2f4LelTXaA3ARZZjFw+qSyTQ4ccMvwj04SaGzsic qz84qoZL1qGWS4Klge28MngSMfMg9VjZjJ8dGjshblhZuIQLvFq91qAq2UrGf143 Gq4TeFTjOF5atGsXV/qMkdmMbCoNcNMza/VHOOUUnsDDCCcGoz9GV5PiRkK7l07j x2n89EF32XU2BH1AmusF+ulBR2KxXK7Oiokd5T4zpk6CnIZ135NEAsMZ0eQ9V9/c Hb41Yo+yjow8jjjtf0PK =RWuO -----END PGP SIGNATURE----- --5wogRi600HRpORcCF1u2CefTR8FQvgfBa-- --===============6002995481428518183== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============6002995481428518183==--