| Message ID | 20191031102306.15433-1-fontaine.fabrice@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] package/domoticz: fix build with RELRO | expand |
Hello Fabrice, As usual, thanks for working on build issues. On Thu, 31 Oct 2019 11:23:06 +0100 Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: > diff --git a/package/domoticz/0003-CMakeLists.txt-Add-USE_PRECOMPILED_HEADER-option.patch b/package/domoticz/0003-CMakeLists.txt-Add-USE_PRECOMPILED_HEADER-option.patch > new file mode 100644 > index 0000000000..831e4816e1 > --- /dev/null > +++ b/package/domoticz/0003-CMakeLists.txt-Add-USE_PRECOMPILED_HEADER-option.patch > @@ -0,0 +1,35 @@ > +From e2dfb2ece19748ba99ec8199fc902c0c9daff325 Mon Sep 17 00:00:00 2001 > +From: Fabrice Fontaine <fontaine.fabrice@gmail.com> > +Date: Wed, 30 Oct 2019 18:55:48 +0100 > +Subject: [PATCH] CMakeLists.txt: Add USE_PRECOMPILED_HEADER option > + > +Add USE_PRECOMPILED_HEADER to allow the user to disable precompiled > +header feature. Thanks to this, domoticz will be able to be built with > +RELRO on buildroot What is the relationship between using precompiled headers and relro build problems ? Thomas
Dear Thomas, Le jeu. 31 oct. 2019 à 13:43, Thomas Petazzoni <thomas.petazzoni@bootlin.com> a écrit : > > Hello Fabrice, > > As usual, thanks for working on build issues. > > On Thu, 31 Oct 2019 11:23:06 +0100 > Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: > > > diff --git a/package/domoticz/0003-CMakeLists.txt-Add-USE_PRECOMPILED_HEADER-option.patch b/package/domoticz/0003-CMakeLists.txt-Add-USE_PRECOMPILED_HEADER-option.patch > > new file mode 100644 > > index 0000000000..831e4816e1 > > --- /dev/null > > +++ b/package/domoticz/0003-CMakeLists.txt-Add-USE_PRECOMPILED_HEADER-option.patch > > @@ -0,0 +1,35 @@ > > +From e2dfb2ece19748ba99ec8199fc902c0c9daff325 Mon Sep 17 00:00:00 2001 > > +From: Fabrice Fontaine <fontaine.fabrice@gmail.com> > > +Date: Wed, 30 Oct 2019 18:55:48 +0100 > > +Subject: [PATCH] CMakeLists.txt: Add USE_PRECOMPILED_HEADER option > > + > > +Add USE_PRECOMPILED_HEADER to allow the user to disable precompiled > > +header feature. Thanks to this, domoticz will be able to be built with > > +RELRO on buildroot > > What is the relationship between using precompiled headers and relro > build problems ? I took me some time to understand the issue. It seems that linker flag such as -Wl,-z,relro should not be used when building a precompiled header. I discovered this through this domoticz commit message: https://github.com/domoticz/domoticz/commit/68698e7a5dce80dea5a9b997d7e171b80bf566ac However, this commit does not fix the issue on buildroot as -Wl,-z,relro is not passed through CXXFLAGS but through our toolchain-wrapper. Updating toolchain/toolchain-wrapper.c to add the logic to manage precompiled header seems more complicated than adding an option to disable this feature from domoticz. > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com Best Regards, Fabrice
Hello, I'm adding a few more people in Cc: Matt, Yann, Arnout, Peter. On Thu, 31 Oct 2019 14:01:33 +0100 Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: > > What is the relationship between using precompiled headers and relro > > build problems ? > I took me some time to understand the issue. > It seems that linker flag such as -Wl,-z,relro should not be used when > building a precompiled header. > I discovered this through this domoticz commit message: > https://github.com/domoticz/domoticz/commit/68698e7a5dce80dea5a9b997d7e171b80bf566ac > > However, this commit does not fix the issue on buildroot as > -Wl,-z,relro is not passed through CXXFLAGS but through our > toolchain-wrapper. > Updating toolchain/toolchain-wrapper.c to add the logic to manage > precompiled header seems more complicated than adding an option to > disable this feature from domoticz. So, I looked into this, found https://lists.fedoraproject.org/pipermail/devel/2011-July/153664.html and https://bugzilla.redhat.com/show_bug.cgi?id=718719. So it really seems like -Wl,-z,relzo should not be passed when compiling. But our compiler wrapper passes it unconditionally when RELRO is enabled. Do we have a way to detect that the wrapper is called to compile or to link ? I don't think we do. So is Fabrice's patch an acceptable work-around, or do we want to fix this more globally ? Thanks for your feedback, Thomas
Thomas, On Mon, Nov 4, 2019 at 4:12 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello, > > I'm adding a few more people in Cc: Matt, Yann, Arnout, Peter. > > On Thu, 31 Oct 2019 14:01:33 +0100 > Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: > > > > What is the relationship between using precompiled headers and relro > > > build problems ? > > I took me some time to understand the issue. > > It seems that linker flag such as -Wl,-z,relro should not be used when > > building a precompiled header. > > I discovered this through this domoticz commit message: > > https://github.com/domoticz/domoticz/commit/68698e7a5dce80dea5a9b997d7e171b80bf566ac > > > > However, this commit does not fix the issue on buildroot as > > -Wl,-z,relro is not passed through CXXFLAGS but through our > > toolchain-wrapper. > > Updating toolchain/toolchain-wrapper.c to add the logic to manage > > precompiled header seems more complicated than adding an option to > > disable this feature from domoticz. > > So, I looked into this, found > https://lists.fedoraproject.org/pipermail/devel/2011-July/153664.html > and https://bugzilla.redhat.com/show_bug.cgi?id=718719. > > So it really seems like -Wl,-z,relzo should not be passed when > compiling. But our compiler wrapper passes it unconditionally when > RELRO is enabled. > > Do we have a way to detect that the wrapper is called to compile or to > link ? I don't think we do. Looks like we can just key on -xc-header or -xc++-header and selectively enable the RELRO flags. I'll do some builds and then send a patch if that looks ok. https://pastebin.com/XGc8bFxh Matt
On 05/11/2019 21:19, Matthew Weber wrote: > Thomas, > > On Mon, Nov 4, 2019 at 4:12 PM Thomas Petazzoni > <thomas.petazzoni@bootlin.com> wrote: >> >> Hello, >> >> I'm adding a few more people in Cc: Matt, Yann, Arnout, Peter. >> >> On Thu, 31 Oct 2019 14:01:33 +0100 >> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: >> >>>> What is the relationship between using precompiled headers and relro >>>> build problems ? >>> I took me some time to understand the issue. >>> It seems that linker flag such as -Wl,-z,relro should not be used when >>> building a precompiled header. >>> I discovered this through this domoticz commit message: >>> https://github.com/domoticz/domoticz/commit/68698e7a5dce80dea5a9b997d7e171b80bf566ac >>> >>> However, this commit does not fix the issue on buildroot as >>> -Wl,-z,relro is not passed through CXXFLAGS but through our >>> toolchain-wrapper. >>> Updating toolchain/toolchain-wrapper.c to add the logic to manage >>> precompiled header seems more complicated than adding an option to >>> disable this feature from domoticz. >> >> So, I looked into this, found >> https://lists.fedoraproject.org/pipermail/devel/2011-July/153664.html >> and https://bugzilla.redhat.com/show_bug.cgi?id=718719. >> >> So it really seems like -Wl,-z,relzo should not be passed when >> compiling. But our compiler wrapper passes it unconditionally when >> RELRO is enabled. >> >> Do we have a way to detect that the wrapper is called to compile or to >> link ? I don't think we do. > > Looks like we can just key on -xc-header or -xc++-header and > selectively enable the RELRO flags. I'll do some builds and then send > a patch if that looks ok. > > https://pastebin.com/XGc8bFxh I'm not so sure if that is a generic solution... As explained in [1]: $ gcc -o /tmp/stdio.gch /usr/include/stdio.h $ gcc -c -o /tmp/stdio.gch /usr/include/stdio.h -Wl,-z,relro $ gcc -o /tmp/stdio.gch /usr/include/stdio.h -Wl,-z,relro /usr/bin/ld: /usr/lib/gcc/x86_64-redhat-linux/9/../../../../lib64/crt1.o: in function `_start': (.text+0x24): undefined reference to `main' collect2: error: ld returned 1 exit status The problem AFAICS is that if no -c or similar option is given, GCC decides what needs to be done based on the rest of the arguments. If the rest of the arguments include a -Wl,... option, it decides that linking needs to be done. If the rest of the arguments are just header files, it decides to create a precompiled header. I think a more appropriate solution is to patch domoticz to add the -c option for building precompiled headers, which AFAICS should fix the issue. If we want to fix it at the toolchain level, I think it would be more appropriate to handle it the way most distros do: by patching the spec file. Regards, Arnout [1] https://lists.fedoraproject.org/pipermail/devel/2011-July/153864.html
Arnout, On Wed, Nov 6, 2019 at 7:07 AM Arnout Vandecappelle <arnout@mind.be> wrote: > > > > On 05/11/2019 21:19, Matthew Weber wrote: > > Thomas, > > > > On Mon, Nov 4, 2019 at 4:12 PM Thomas Petazzoni > > <thomas.petazzoni@bootlin.com> wrote: > >> > >> Hello, > >> > >> I'm adding a few more people in Cc: Matt, Yann, Arnout, Peter. > >> > >> On Thu, 31 Oct 2019 14:01:33 +0100 > >> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote: > >> > >>>> What is the relationship between using precompiled headers and relro > >>>> build problems ? > >>> I took me some time to understand the issue. > >>> It seems that linker flag such as -Wl,-z,relro should not be used when > >>> building a precompiled header. > >>> I discovered this through this domoticz commit message: > >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_domoticz_domoticz_commit_68698e7a5dce80dea5a9b997d7e171b80bf566ac&d=DwICaQ&c=ilBQI1lupc9Y65XwNblLtw&r=y1sOV0GV8NZUkffv7oCRxs2Sd3nOBS-NxDM3NY8lOgs&m=DjIUeN2SXon2vr5dX4sl6rmhJvz5cmbRv5u_qS4doBw&s=ZPd1FlLwup8D1uVOQHfI-t-Whn2V55bYtrI9ZDYtrR4&e= > >>> > >>> However, this commit does not fix the issue on buildroot as > >>> -Wl,-z,relro is not passed through CXXFLAGS but through our > >>> toolchain-wrapper. > >>> Updating toolchain/toolchain-wrapper.c to add the logic to manage > >>> precompiled header seems more complicated than adding an option to > >>> disable this feature from domoticz. > >> > >> So, I looked into this, found > >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.fedoraproject.org_pipermail_devel_2011-2DJuly_153664.html&d=DwICaQ&c=ilBQI1lupc9Y65XwNblLtw&r=y1sOV0GV8NZUkffv7oCRxs2Sd3nOBS-NxDM3NY8lOgs&m=DjIUeN2SXon2vr5dX4sl6rmhJvz5cmbRv5u_qS4doBw&s=6ZTbPXMho6JGTlraCYmMM2nc_HlF9NsMkXyNc5GCgho&e= > >> and https://urldefense.proofpoint.com/v2/url?u=https-3A__bugzilla.redhat.com_show-5Fbug.cgi-3Fid-3D718719&d=DwICaQ&c=ilBQI1lupc9Y65XwNblLtw&r=y1sOV0GV8NZUkffv7oCRxs2Sd3nOBS-NxDM3NY8lOgs&m=DjIUeN2SXon2vr5dX4sl6rmhJvz5cmbRv5u_qS4doBw&s=e_knYSAlNZ7WMHm6gZnbJn7Kj5mhCVIe5zWhgxkb23o&e= . > >> > >> So it really seems like -Wl,-z,relzo should not be passed when > >> compiling. But our compiler wrapper passes it unconditionally when > >> RELRO is enabled. > >> > >> Do we have a way to detect that the wrapper is called to compile or to > >> link ? I don't think we do. > > > > Looks like we can just key on -xc-header or -xc++-header and > > selectively enable the RELRO flags. I'll do some builds and then send > > a patch if that looks ok. > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__pastebin.com_XGc8bFxh&d=DwICaQ&c=ilBQI1lupc9Y65XwNblLtw&r=y1sOV0GV8NZUkffv7oCRxs2Sd3nOBS-NxDM3NY8lOgs&m=DjIUeN2SXon2vr5dX4sl6rmhJvz5cmbRv5u_qS4doBw&s=5sBtnwkv13zwuv2FZm1hYBGap0wC14KeSXkljxL8JtQ&e= > > I'm not so sure if that is a generic solution... As explained in [1]: > > $ gcc -o /tmp/stdio.gch /usr/include/stdio.h > $ gcc -c -o /tmp/stdio.gch /usr/include/stdio.h -Wl,-z,relro > $ gcc -o /tmp/stdio.gch /usr/include/stdio.h -Wl,-z,relro > /usr/bin/ld: /usr/lib/gcc/x86_64-redhat-linux/9/../../../../lib64/crt1.o: in > function `_start': > (.text+0x24): undefined reference to `main' > collect2: error: ld returned 1 exit status > > The problem AFAICS is that if no -c or similar option is given, GCC decides > what needs to be done based on the rest of the arguments. If the rest of the > arguments include a -Wl,... option, it decides that linking needs to be done. If > the rest of the arguments are just header files, it decides to create a > precompiled header. > > I think a more appropriate solution is to patch domoticz to add the -c option > for building precompiled headers, which AFAICS should fix the issue. > > If we want to fix it at the toolchain level, I think it would be more > appropriate to handle it the way most distros do: by patching the spec file. Agree, the little bit of testing I did ended up at the conclusion of [1] where GCC infers a direction to take. I believe the last time we looked at doing a custom spec file we instead choose the wrapper path. However I'd be curious how often this will show up as it would be easier to patch domoticz and the couple other packages for now. > > Regards, > Arnout > > > [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.fedoraproject.org_pipermail_devel_2011-2DJuly_153864.html&d=DwICaQ&c=ilBQI1lupc9Y65XwNblLtw&r=y1sOV0GV8NZUkffv7oCRxs2Sd3nOBS-NxDM3NY8lOgs&m=DjIUeN2SXon2vr5dX4sl6rmhJvz5cmbRv5u_qS4doBw&s=h1AFIQjg3Krsi6JqWc3h0L83OT887ET8xVrRdcoBNqg&e=
diff --git a/package/domoticz/0003-CMakeLists.txt-Add-USE_PRECOMPILED_HEADER-option.patch b/package/domoticz/0003-CMakeLists.txt-Add-USE_PRECOMPILED_HEADER-option.patch new file mode 100644 index 0000000000..831e4816e1 --- /dev/null +++ b/package/domoticz/0003-CMakeLists.txt-Add-USE_PRECOMPILED_HEADER-option.patch @@ -0,0 +1,35 @@ +From e2dfb2ece19748ba99ec8199fc902c0c9daff325 Mon Sep 17 00:00:00 2001 +From: Fabrice Fontaine <fontaine.fabrice@gmail.com> +Date: Wed, 30 Oct 2019 18:55:48 +0100 +Subject: [PATCH] CMakeLists.txt: Add USE_PRECOMPILED_HEADER option + +Add USE_PRECOMPILED_HEADER to allow the user to disable precompiled +header feature. Thanks to this, domoticz will be able to be built with +RELRO on buildroot + +Fixes: + - http://autobuild.buildroot.org/results/5c1ca3083ad672401d1e050c6c3a07b8c33b851d + +Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> +[Retrieved from: +https://github.com/domoticz/domoticz/commit/e2dfb2ece19748ba99ec8199fc902c0c9daff325] +--- + CMakeLists.txt | 5 ++++- + 1 file changed, 4 insertions(+), 1 deletion(-) + +diff --git a/CMakeLists.txt b/CMakeLists.txt +index cb6150ce4c..bd48872214 100644 +--- a/CMakeLists.txt ++++ b/CMakeLists.txt +@@ -744,7 +744,10 @@ else() + target_link_libraries(domoticz -lrt -lresolv ${EXECINFO_LIBRARIES}) + ENDIF() + +-ADD_PRECOMPILED_HEADER(domoticz "main/stdafx.h") ++option(USE_PRECOMPILED_HEADER "Use precompiled header feature to speed up build time " YES) ++if(USE_PRECOMPILED_HEADER) ++ ADD_PRECOMPILED_HEADER(domoticz "main/stdafx.h") ++ENDIF(USE_PRECOMPILED_HEADER) + + IF(CMAKE_COMPILER_IS_GNUCXX) + option(USE_STATIC_LIBSTDCXX "Build with static libgcc/libstdc++ libraries" YES) diff --git a/package/domoticz/domoticz.mk b/package/domoticz/domoticz.mk index c0568c61c6..d8ccfeee5a 100644 --- a/package/domoticz/domoticz.mk +++ b/package/domoticz/domoticz.mk @@ -31,6 +31,9 @@ DOMOTICZ_CONF_OPTS += \ -DUSE_BUILTIN_SQLITE=OFF \ -DUSE_BUILTIN_MQTT=OFF +# Disable precompiled header feature to fix build with RELRO +DOMOTICZ_CONF_OPTS += -DUSE_PRECOMPILED_HEADER=OFF + ifeq ($(BR2_PACKAGE_LIBUSB),y) DOMOTICZ_DEPENDENCIES += libusb endif
Fixes: - http://autobuild.buildroot.org/results/5c1ca3083ad672401d1e050c6c3a07b8c33b851d Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> --- ...xt-Add-USE_PRECOMPILED_HEADER-option.patch | 35 +++++++++++++++++++ package/domoticz/domoticz.mk | 3 ++ 2 files changed, 38 insertions(+) create mode 100644 package/domoticz/0003-CMakeLists.txt-Add-USE_PRECOMPILED_HEADER-option.patch