[ovs-dev] [PATCH] travis: Fix 32-bit libunwind system build.

Ilya Maximets i.maximets at ovn.org
Fri Oct 4 18:53:49 UTC 2019


On 04.10.2019 2:34, William Tu wrote:
> On Thu, Oct 3, 2019 at 10:15 AM Ilya Maximets <i.maximets at 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 at 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 at 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.


More information about the dev mailing list