| Message ID | 1570036546-122464-1-git-send-email-u9012063@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] travis: Fix 32-bit libunwind system build. | expand |
On 02.10.2019 20:15, William Tu wrote: > 32-bit and 64-bit libunwind can not be installed at the same time. > For 32-bit build, this patch removes the 64-bit libunwind and install > 32-bit version. > > Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/592649311 > Fixes: e2ed6fbeb18c ("fatal-signal: Catch SIGSEGV and print backtrace.") > Signed-off-by: William Tu <u9012063@gmail.com> > --- > .travis/linux-prepare.sh | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh > index 70fd98f715ed..164adf7ec4f8 100755 > --- a/.travis/linux-prepare.sh > +++ b/.travis/linux-prepare.sh > @@ -14,3 +14,9 @@ cd .. > > pip install --disable-pip-version-check --user six flake8 hacking > pip install --user --upgrade docutils > + > +if [[ $BUILD_ENV =~ "-m32" ]]; then > + # 32-bit and 64-bit libunwind can not be installed at the same time. > + # This will remove the 64-bit libunwind and install 32-bit version. > + sudo apt-get install -y libunwind-dev:i386 > +fi > Thanks for the new version. One thing I noticed is that without additional changes[1] this patch just makes 'configure' script to fail the library check, because 32bit library can't be linked with 64bit binary it checks: checking for unw_backtrace in -lunwind... no But it should work as intended with the patch I posted: [1] https://patchwork.ozlabs.org/patch/1171259/ Note that I replaced $BUILD_ENV with $M32 variable, so one of the patches will need to be updated depending on the order of applying. i.e. following diff should be squashed to one of them: -if [[ $BUILD_ENV =~ "-m32" ]]; then +if [ "$M32" ]; then Other than that, Acked-by: Ilya Maximets <i.maximets@ovn.org>
On 03.10.2019 18:13, Ilya Maximets wrote: > On 02.10.2019 20:15, William Tu wrote: >> 32-bit and 64-bit libunwind can not be installed at the same time. >> For 32-bit build, this patch removes the 64-bit libunwind and install >> 32-bit version. >> >> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/592649311 >> Fixes: e2ed6fbeb18c ("fatal-signal: Catch SIGSEGV and print backtrace.") >> Signed-off-by: William Tu <u9012063@gmail.com> >> --- >> .travis/linux-prepare.sh | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh >> index 70fd98f715ed..164adf7ec4f8 100755 >> --- a/.travis/linux-prepare.sh >> +++ b/.travis/linux-prepare.sh >> @@ -14,3 +14,9 @@ cd .. >> pip install --disable-pip-version-check --user six flake8 hacking >> pip install --user --upgrade docutils >> + >> +if [[ $BUILD_ENV =~ "-m32" ]]; then >> + # 32-bit and 64-bit libunwind can not be installed at the same time. >> + # This will remove the 64-bit libunwind and install 32-bit version. >> + sudo apt-get install -y libunwind-dev:i386 >> +fi >> > > Thanks for the new version. > > One thing I noticed is that without additional changes[1] this > patch just makes 'configure' script to fail the library check, > because 32bit library can't be linked with 64bit binary it checks: > > checking for unw_backtrace in -lunwind... no > > But it should work as intended with the patch I posted: > > [1] https://patchwork.ozlabs.org/patch/1171259/ > > Note that I replaced $BUILD_ENV with $M32 variable, so one of the > patches will need to be updated depending on the order of applying. > i.e. following diff should be squashed to one of them: > > -if [[ $BUILD_ENV =~ "-m32" ]]; then > +if [ "$M32" ]; then > > Other than that, > Acked-by: Ilya Maximets <i.maximets@ovn.org> Hmm, I tried to apply this patch and check the build and it failed: lib/backtrace.c: In function ‘log_received_backtrace’: lib/backtrace.c:105:23: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘unw_word_t {aka unsigned int}’ [-Werror=format=] VLOG_WARN("0x%016lx <%s+0x%lx>\n", ^ ./include/openvswitch/vlog.h:277:41: note: in definition of macro ‘VLOG’ vlog(&this_module, level__, __VA_ARGS__); \ lib/backtrace.c:105:13: note: in expansion of macro ‘VLOG_WARN’ VLOG_WARN("0x%016lx <%s+0x%lx>\n", ^ lib/backtrace.c:105:23: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 6 has type ‘unw_word_t {aka unsigned int}’ [-Werror=format=] VLOG_WARN("0x%016lx <%s+0x%lx>\n", ^ ./include/openvswitch/vlog.h:277:41: note: in definition of macro ‘VLOG’ vlog(&this_module, level__, __VA_ARGS__); \ lib/backtrace.c:105:13: note: in expansion of macro ‘VLOG_WARN’ VLOG_WARN("0x%016lx <%s+0x%lx>\n", ^ Full log: https://travis-ci.org/igsilya/ovs/jobs/593132451 Best regards, Ilya Maximets.
On Thu, Oct 3, 2019 at 10:15 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 03.10.2019 18:13, Ilya Maximets wrote: > > On 02.10.2019 20:15, William Tu wrote: > >> 32-bit and 64-bit libunwind can not be installed at the same time. > >> For 32-bit build, this patch removes the 64-bit libunwind and install > >> 32-bit version. > >> > >> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/592649311 > >> Fixes: e2ed6fbeb18c ("fatal-signal: Catch SIGSEGV and print backtrace.") > >> Signed-off-by: William Tu <u9012063@gmail.com> > >> --- > >> .travis/linux-prepare.sh | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh > >> index 70fd98f715ed..164adf7ec4f8 100755 > >> --- a/.travis/linux-prepare.sh > >> +++ b/.travis/linux-prepare.sh > >> @@ -14,3 +14,9 @@ cd .. > >> pip install --disable-pip-version-check --user six flake8 hacking > >> pip install --user --upgrade docutils > >> + > >> +if [[ $BUILD_ENV =~ "-m32" ]]; then > >> + # 32-bit and 64-bit libunwind can not be installed at the same time. > >> + # This will remove the 64-bit libunwind and install 32-bit version. > >> + sudo apt-get install -y libunwind-dev:i386 > >> +fi > >> > > > > Thanks for the new version. > > > > One thing I noticed is that without additional changes[1] this > > patch just makes 'configure' script to fail the library check, > > because 32bit library can't be linked with 64bit binary it checks: > > > > checking for unw_backtrace in -lunwind... no > > > > But it should work as intended with the patch I posted: > > > > [1] https://patchwork.ozlabs.org/patch/1171259/ > > > > Note that I replaced $BUILD_ENV with $M32 variable, so one of the > > patches will need to be updated depending on the order of applying. > > i.e. following diff should be squashed to one of them: > > > > -if [[ $BUILD_ENV =~ "-m32" ]]; then > > +if [ "$M32" ]; then > > > > Other than that, > > Acked-by: Ilya Maximets <i.maximets@ovn.org> > > > Hmm, I tried to apply this patch and check the build and it > failed: > > lib/backtrace.c: In function ‘log_received_backtrace’: > lib/backtrace.c:105:23: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘unw_word_t {aka unsigned int}’ [-Werror=format=] > VLOG_WARN("0x%016lx <%s+0x%lx>\n", > ^ > ./include/openvswitch/vlog.h:277:41: note: in definition of macro ‘VLOG’ > vlog(&this_module, level__, __VA_ARGS__); \ > > lib/backtrace.c:105:13: note: in expansion of macro ‘VLOG_WARN’ > VLOG_WARN("0x%016lx <%s+0x%lx>\n", > ^ > lib/backtrace.c:105:23: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 6 has type ‘unw_word_t {aka unsigned int}’ [-Werror=format=] > VLOG_WARN("0x%016lx <%s+0x%lx>\n", > ^ > ./include/openvswitch/vlog.h:277:41: note: in definition of macro ‘VLOG’ > vlog(&this_module, level__, __VA_ARGS__); \ > > lib/backtrace.c:105:13: note: in expansion of macro ‘VLOG_WARN’ > > VLOG_WARN("0x%016lx <%s+0x%lx>\n", > ^ > > Full log: > https://travis-ci.org/igsilya/ovs/jobs/593132451 Thanks! For 32-bit libunwind, the unw_word_t is defined as uint32_t, and for 64-bit libunwind, it is defined as uint64_t. Here actually it is printing a pointer, so I will change it to use PRIxPTR diff --git a/lib/backtrace.c b/lib/backtrace.c index 9347634487c8..2853d5ff150d 100644 --- a/lib/backtrace.c +++ b/lib/backtrace.c @@ -102,7 +102,7 @@ log_received_backtrace(int fd) { if (backtrace[i].func[0] == 0) { break; } - VLOG_WARN("0x%016lx <%s+0x%lx>\n", + VLOG_WARN("0x%016"PRIxPTR" <%s+0x%"PRIxPTR">\n", backtrace[i].ip, backtrace[i].func, backtrace[i].offset); I clone your upstream_freebsd_11.3 branch, and tested-at https://travis-ci.org/williamtu/ovs-travis/builds/593284524 I will send a patch based on your patch. Regards, William
On 04.10.2019 2:34, William Tu wrote: > On Thu, Oct 3, 2019 at 10:15 AM Ilya Maximets <i.maximets@ovn.org> wrote: >> >> On 03.10.2019 18:13, Ilya Maximets wrote: >>> On 02.10.2019 20:15, William Tu wrote: >>>> 32-bit and 64-bit libunwind can not be installed at the same time. >>>> For 32-bit build, this patch removes the 64-bit libunwind and install >>>> 32-bit version. >>>> >>>> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/592649311 >>>> Fixes: e2ed6fbeb18c ("fatal-signal: Catch SIGSEGV and print backtrace.") >>>> Signed-off-by: William Tu <u9012063@gmail.com> >>>> --- >>>> .travis/linux-prepare.sh | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>>> diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh >>>> index 70fd98f715ed..164adf7ec4f8 100755 >>>> --- a/.travis/linux-prepare.sh >>>> +++ b/.travis/linux-prepare.sh >>>> @@ -14,3 +14,9 @@ cd .. >>>> pip install --disable-pip-version-check --user six flake8 hacking >>>> pip install --user --upgrade docutils >>>> + >>>> +if [[ $BUILD_ENV =~ "-m32" ]]; then >>>> + # 32-bit and 64-bit libunwind can not be installed at the same time. >>>> + # This will remove the 64-bit libunwind and install 32-bit version. >>>> + sudo apt-get install -y libunwind-dev:i386 >>>> +fi >>>> >>> >>> Thanks for the new version. >>> >>> One thing I noticed is that without additional changes[1] this >>> patch just makes 'configure' script to fail the library check, >>> because 32bit library can't be linked with 64bit binary it checks: >>> >>> checking for unw_backtrace in -lunwind... no >>> >>> But it should work as intended with the patch I posted: >>> >>> [1] https://patchwork.ozlabs.org/patch/1171259/ >>> >>> Note that I replaced $BUILD_ENV with $M32 variable, so one of the >>> patches will need to be updated depending on the order of applying. >>> i.e. following diff should be squashed to one of them: >>> >>> -if [[ $BUILD_ENV =~ "-m32" ]]; then >>> +if [ "$M32" ]; then >>> >>> Other than that, >>> Acked-by: Ilya Maximets <i.maximets@ovn.org> >> >> >> Hmm, I tried to apply this patch and check the build and it >> failed: >> >> lib/backtrace.c: In function ‘log_received_backtrace’: >> lib/backtrace.c:105:23: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘unw_word_t {aka unsigned int}’ [-Werror=format=] >> VLOG_WARN("0x%016lx <%s+0x%lx>\n", >> ^ >> ./include/openvswitch/vlog.h:277:41: note: in definition of macro ‘VLOG’ >> vlog(&this_module, level__, __VA_ARGS__); \ >> >> lib/backtrace.c:105:13: note: in expansion of macro ‘VLOG_WARN’ >> VLOG_WARN("0x%016lx <%s+0x%lx>\n", >> ^ >> lib/backtrace.c:105:23: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 6 has type ‘unw_word_t {aka unsigned int}’ [-Werror=format=] >> VLOG_WARN("0x%016lx <%s+0x%lx>\n", >> ^ >> ./include/openvswitch/vlog.h:277:41: note: in definition of macro ‘VLOG’ >> vlog(&this_module, level__, __VA_ARGS__); \ >> >> lib/backtrace.c:105:13: note: in expansion of macro ‘VLOG_WARN’ >> >> VLOG_WARN("0x%016lx <%s+0x%lx>\n", >> ^ >> >> Full log: >> https://travis-ci.org/igsilya/ovs/jobs/593132451 > > Thanks! > For 32-bit libunwind, the unw_word_t is defined as uint32_t, and > for 64-bit libunwind, it is defined as uint64_t. Here actually it is printing > a pointer, so I will change it to use PRIxPTR > > diff --git a/lib/backtrace.c b/lib/backtrace.c > index 9347634487c8..2853d5ff150d 100644 > --- a/lib/backtrace.c > +++ b/lib/backtrace.c > @@ -102,7 +102,7 @@ log_received_backtrace(int fd) { > if (backtrace[i].func[0] == 0) { > break; > } > - VLOG_WARN("0x%016lx <%s+0x%lx>\n", > + VLOG_WARN("0x%016"PRIxPTR" <%s+0x%"PRIxPTR">\n", > backtrace[i].ip, > backtrace[i].func, > backtrace[i].offset); > Above fix looks good to me. Now when OVS_CFLAGS fix is merged, you could send above two changes (for travis and for backtrace.c) based on current master branch. These could be 2 separate patches or a single squashed change. Best regards, Ilya Maximets.
On Fri, Oct 04, 2019 at 08:53:49PM +0200, Ilya Maximets wrote: > On 04.10.2019 2:34, William Tu wrote: > >On Thu, Oct 3, 2019 at 10:15 AM Ilya Maximets <i.maximets@ovn.org> wrote: > >> > >>On 03.10.2019 18:13, Ilya Maximets wrote: > >>>On 02.10.2019 20:15, William Tu wrote: > >>>>32-bit and 64-bit libunwind can not be installed at the same time. > >>>>For 32-bit build, this patch removes the 64-bit libunwind and install > >>>>32-bit version. > >>>> > >>>>Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/592649311 > >>>>Fixes: e2ed6fbeb18c ("fatal-signal: Catch SIGSEGV and print backtrace.") > >>>>Signed-off-by: William Tu <u9012063@gmail.com> > >>>>--- > >>>> .travis/linux-prepare.sh | 6 ++++++ > >>>> 1 file changed, 6 insertions(+) > >>>> > >>>>diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh > >>>>index 70fd98f715ed..164adf7ec4f8 100755 > >>>>--- a/.travis/linux-prepare.sh > >>>>+++ b/.travis/linux-prepare.sh > >>>>@@ -14,3 +14,9 @@ cd .. > >>>> pip install --disable-pip-version-check --user six flake8 hacking > >>>> pip install --user --upgrade docutils > >>>>+ > >>>>+if [[ $BUILD_ENV =~ "-m32" ]]; then > >>>>+ # 32-bit and 64-bit libunwind can not be installed at the same time. > >>>>+ # This will remove the 64-bit libunwind and install 32-bit version. > >>>>+ sudo apt-get install -y libunwind-dev:i386 > >>>>+fi > >>>> > >>> > >>>Thanks for the new version. > >>> > >>>One thing I noticed is that without additional changes[1] this > >>>patch just makes 'configure' script to fail the library check, > >>>because 32bit library can't be linked with 64bit binary it checks: > >>> > >>> checking for unw_backtrace in -lunwind... no > >>> > >>>But it should work as intended with the patch I posted: > >>> > >>>[1] https://patchwork.ozlabs.org/patch/1171259/ > >>> > >>>Note that I replaced $BUILD_ENV with $M32 variable, so one of the > >>>patches will need to be updated depending on the order of applying. > >>>i.e. following diff should be squashed to one of them: > >>> > >>>-if [[ $BUILD_ENV =~ "-m32" ]]; then > >>>+if [ "$M32" ]; then > >>> > >>>Other than that, > >>>Acked-by: Ilya Maximets <i.maximets@ovn.org> > >> > >> > >>Hmm, I tried to apply this patch and check the build and it > >>failed: > >> > >>lib/backtrace.c: In function ‘log_received_backtrace’: > >>lib/backtrace.c:105:23: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘unw_word_t {aka unsigned int}’ [-Werror=format=] > >> VLOG_WARN("0x%016lx <%s+0x%lx>\n", > >> ^ > >>./include/openvswitch/vlog.h:277:41: note: in definition of macro ‘VLOG’ > >> vlog(&this_module, level__, __VA_ARGS__); \ > >> > >>lib/backtrace.c:105:13: note: in expansion of macro ‘VLOG_WARN’ > >> VLOG_WARN("0x%016lx <%s+0x%lx>\n", > >> ^ > >>lib/backtrace.c:105:23: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 6 has type ‘unw_word_t {aka unsigned int}’ [-Werror=format=] > >> VLOG_WARN("0x%016lx <%s+0x%lx>\n", > >> ^ > >>./include/openvswitch/vlog.h:277:41: note: in definition of macro ‘VLOG’ > >> vlog(&this_module, level__, __VA_ARGS__); \ > >> > >>lib/backtrace.c:105:13: note: in expansion of macro ‘VLOG_WARN’ > >> > >> VLOG_WARN("0x%016lx <%s+0x%lx>\n", > >> ^ > >> > >>Full log: > >>https://travis-ci.org/igsilya/ovs/jobs/593132451 > > > >Thanks! > >For 32-bit libunwind, the unw_word_t is defined as uint32_t, and > >for 64-bit libunwind, it is defined as uint64_t. Here actually it is printing > >a pointer, so I will change it to use PRIxPTR > > > >diff --git a/lib/backtrace.c b/lib/backtrace.c > >index 9347634487c8..2853d5ff150d 100644 > >--- a/lib/backtrace.c > >+++ b/lib/backtrace.c > >@@ -102,7 +102,7 @@ log_received_backtrace(int fd) { > > if (backtrace[i].func[0] == 0) { > > break; > > } > >- VLOG_WARN("0x%016lx <%s+0x%lx>\n", > >+ VLOG_WARN("0x%016"PRIxPTR" <%s+0x%"PRIxPTR">\n", > > backtrace[i].ip, > > backtrace[i].func, > > backtrace[i].offset); > > > > Above fix looks good to me. Now when OVS_CFLAGS fix is merged, you > could send above two changes (for travis and for backtrace.c) based > on current master branch. These could be 2 separate patches or a > single squashed change. > > Best regards, Ilya Maximets. Thanks! I sent the two saparated patches. Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/593718821
diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh index 70fd98f715ed..164adf7ec4f8 100755 --- a/.travis/linux-prepare.sh +++ b/.travis/linux-prepare.sh @@ -14,3 +14,9 @@ cd .. pip install --disable-pip-version-check --user six flake8 hacking pip install --user --upgrade docutils + +if [[ $BUILD_ENV =~ "-m32" ]]; then + # 32-bit and 64-bit libunwind can not be installed at the same time. + # This will remove the 64-bit libunwind and install 32-bit version. + sudo apt-get install -y libunwind-dev:i386 +fi
32-bit and 64-bit libunwind can not be installed at the same time. For 32-bit build, this patch removes the 64-bit libunwind and install 32-bit version. Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/592649311 Fixes: e2ed6fbeb18c ("fatal-signal: Catch SIGSEGV and print backtrace.") Signed-off-by: William Tu <u9012063@gmail.com> --- .travis/linux-prepare.sh | 6 ++++++ 1 file changed, 6 insertions(+)