search

ATLFlight / cmake_hexagon

CMake toolchain and rules for building apps for Hexagon DSP and apps processor on Qualcomm SoCs
29 stars 56 forks source link

qurt_lib.cmake: fix redundant linking #15

Closed julianoes closed 7 years ago

julianoes commented 7 years ago

This fixes the issue where all PX4 modules where linked twice, once into libpx4.so and once into libpx4muorb_skel.so. This lead to cases where data existed twice and lead to bugs through e.g. uninitialized memory.

This resolves https://github.com/PX4/Firmware/issues/5756#issuecomment-257964558.

Found with @CarloWood.

@mcharleb and @jywilson please review and comment, thanks.

jywilson commented 7 years ago

Mark,

Can you work on adjusting the change that Julian has made?

Julian,

Awesome work finding this issue! (just read this post from github)

Jim W.

From: Mark Charlebois notifications@github.com Sent: Monday, November 7, 2016 8:49:05 AM To: ATLFlight/cmake_hexagon Cc: jywilson; Mention Subject: Re: [ATLFlight/cmake_hexagon] qurt_lib.cmake: fix redundant linking (#15)

@mcharleb commented on this pull request.

In qurt_lib.cmakehttps://github.com/ATLFlight/cmake_hexagon/pull/15#pullrequestreview-7455339:

  endif()
    add_library(${QURT_LIB_IDL_NAME}_skel MODULE
            ${QURT_LIB_IDL_NAME}_skel.c
            )

This seems to completely eliminate the ability to add other library deps via QURT_LIB_LINK_LIBS. Something needs to change here. Either QURT_LIB_LINK_LIBS should be entirely removed, or it needs to use used somewhere.

You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/ATLFlight/cmakehexagon/pull/15#pullrequestreview-7455339, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ADhMdspZzHVFXaaii58wHoZq6DQ-o9o7ks5q71aBgaJpZM4Kqpf.

mcharleb commented 7 years ago

@jywilson The issues has been resolved in https://github.com/ATLFlight/cmake_hexagon/pull/16

julianoes commented 7 years ago

@mcharleb did you actually test #16? It crashes for me straightaway while my fix actually works!

mcharleb commented 7 years ago

@julianoes I did not have time to test with PX4 yesterday, but tested Hello World, dspal tester. I will try using your "lib${QURT_LIB_LIB_NAME}.so" change and test PX4

mcharleb commented 7 years ago

Hopefully resolved by: https://github.com/ATLFlight/cmake_hexagon/commit/69a7b313eb8963e2b8909dc8e2ce8e98ae4a99e8