From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail1.windriver.com (mail1.windriver.com [147.11.146.13]) by mail.openembedded.org (Postfix) with ESMTP id 0078260761 for ; Wed, 23 Mar 2016 01:29:27 +0000 (UTC) Received: from ALA-HCB.corp.ad.wrs.com (ala-hcb.corp.ad.wrs.com [147.11.189.41]) by mail1.windriver.com (8.15.2/8.15.1) with ESMTPS id u2N1TQfa010345 (version=TLSv1 cipher=AES128-SHA bits=128 verify=FAIL); Tue, 22 Mar 2016 18:29:26 -0700 (PDT) Received: from [128.224.162.159] (128.224.162.159) by ALA-HCB.corp.ad.wrs.com (147.11.189.41) with Microsoft SMTP Server id 14.3.248.2; Tue, 22 Mar 2016 18:29:25 -0700 To: Bruce Ashfield , Mark Hatle References: <56F17064.6040507@windriver.com> From: Hongxu Jia Message-ID: <56F1F173.7070504@windriver.com> Date: Wed, 23 Mar 2016 09:29:23 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: Cc: Patches and discussions about the oe-core layer Subject: Re: [PATCH 02/16] kernel.bbclass: fix buildpath QA issue X-BeenThere: openembedded-core@lists.openembedded.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Patches and discussions about the oe-core layer List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Mar 2016 01:29:30 -0000 Content-Type: multipart/alternative; boundary="------------000909020302080901080309" --------------000909020302080901080309 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit On 03/23/2016 12:49 AM, Bruce Ashfield wrote: > > > On Tue, Mar 22, 2016 at 12:18 PM, Mark Hatle > wrote: > > On 3/22/16 7:21 AM, Bruce Ashfield wrote: > > > > > > On Tue, Mar 22, 2016 at 8:12 AM, Hongxu Jia > > > >> wrote: > > > > Since CFLAGS CPPFLAGS CXXFLAGS has been unset, variable > DEBUG_FLAGS could > > not been passed to compiler, so we explicitly add > DEBUG_FLAGS to CC to > > replace build path with target path. > > > > > > Can you be more explicit here ? What is typically contained in > DEBUG_FLAGS ? The > > kernel builds its own compiler line and flags, so anything you > are setting from this > > environment, should not be leaking into the kernel build. > > > > .. which leaves me wondering, what exactly is in DEBUG_FLAGS, > and how is it > > actually fixing the QA issue ? > > The specific debug flag in question is being added to remap the > on-disk path to > the path we want the items to look like from a debugging point of > view. > > The typical format of DEBUG_FLAGS is something like: > > DEBUG_FLAGS ?= "-g -feliminate-unused-debug-types" > > This was recently updated to include the remapping: > > DEBUG_FLAGS ?= "-g -feliminate-unused-debug-types \ > -fdebug-prefix-map=${WORKDIR}=/usr/src/debug \ > -fdebug-prefix-map=${STAGING_DIR_NATIVE}= \ > -fdebug-prefix-map=${STAGING_DIR_HOST}= \ > > The idea is the WORKDIR is replaced with '/usr/src/debug' inside > of the binary > and any debug symbol references. This makes it much easier for > the system to > work with supplied debug sources/symbols. > > I'm not sure you want to use DEBUG_FLAGS since anything can be put > in there.... > > Application space code should always use the DEBUG_FLAGS, or I > think it's likely > a bug. > > The kernel though should probably define it's own flag(s). Most > likely you'll > want something simple like: > > -fdebug-prefix-map=${WORKDIR}=/usr/src/debug > > > > I can see that. > > What outputs of the kernel build need this remapping ? The userspace > parts ? > Something else ? > While building kernel, there are many build paths which caused by macro __FILE__. Once you invoke marco WARN_ON in kernel/module, there will be a __FILE__ used. .... #define WARN_ON(condition) ({ \ int __ret_warn_on = !!(condition); \ if (unlikely(__ret_warn_on)) \ __WARN(); \ unlikely(__ret_warn_on); \ }) #define __WARN() warn_slowpath_null(__FILE__, __LINE__) .... That's why I add -fdebug-prefix-map to kernel build. I original added a new option -ffile-prefix-map for gcc to remap __FILE__, and enhance -fdebug-prefix-map to replace -ffile-prefix-map to compat with external toolchain. > The point is that we patch the kernel build, or make a specific > Kconfig option, or > something similar to that, versus leaking our build flags into the > kernel build or > "bad things can happen". > If the leaking idea is bad, would you like to accept '-fdebug-prefix-map' as kernel's private? Or suggestions else to fix the build path in kernel? //Hongxu > So I'm sure the solution in the build is the same, but passing flags > via a general > mechanism like I saw in the patch is generally not a good idea. > > Bruce > > > Then all of the internal references to WORKDIR will be replaced with > '/usr/src/debug' which cross gdb can 'map' or on-target will > actually look in > that location. (A corresponding '-dbg' package should be provided > with the > matching compilation sources at the local specified.) > > --Mark > > > Bruce > > > > > > > > [YOCTO #7058] > > > > Signed-off-by: Hongxu Jia > > >> > > --- > > meta/classes/kernel.bbclass | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/meta/classes/kernel.bbclass > b/meta/classes/kernel.bbclass > > index c3eab50..d357ccf 100644 > > --- a/meta/classes/kernel.bbclass > > +++ b/meta/classes/kernel.bbclass > > @@ -207,7 +207,7 @@ kernel_do_compile() { > > copy_initramfs > > > > > use_alternate_initrd=CONFIG_INITRAMFS_SOURCE=${B}/usr/${INITRAMFS_IMAGE}-${MACHINE}.cpio > > fi > > - oe_runmake ${KERNEL_IMAGETYPE_FOR_MAKE} > ${KERNEL_ALT_IMAGETYPE} > > CC="${KERNEL_CC}" LD="${KERNEL_LD}" ${KERNEL_EXTRA_ARGS} > $use_alternate_initrd > > + oe_runmake ${KERNEL_IMAGETYPE_FOR_MAKE} > ${KERNEL_ALT_IMAGETYPE} > > CC="${KERNEL_CC} ${DEBUG_FLAGS}" LD="${KERNEL_LD}" > ${KERNEL_EXTRA_ARGS} > > $use_alternate_initrd > > if test "${KERNEL_IMAGETYPE_FOR_MAKE}.gz" = > "${KERNEL_IMAGETYPE}"; then > > gzip -9c < "${KERNEL_IMAGETYPE_FOR_MAKE}" > > "${KERNEL_OUTPUT}" > > fi > > @@ -216,7 +216,7 @@ kernel_do_compile() { > > do_compile_kernelmodules() { > > unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE > > if (grep -q -i -e '^CONFIG_MODULES=y$' > ${B}/.config); then > > - oe_runmake -C ${B} ${PARALLEL_MAKE} modules > > CC="${KERNEL_CC}" LD="${KERNEL_LD}" ${KERNEL_EXTRA_ARGS} > > + oe_runmake -C ${B} ${PARALLEL_MAKE} modules > CC="${KERNEL_CC} > > ${DEBUG_FLAGS}" LD="${KERNEL_LD}" ${KERNEL_EXTRA_ARGS} > > > > # Module.symvers gets updated during the > > # building of the kernel modules. We need to > > -- > > 1.9.1 > > > > -- > > _______________________________________________ > > Openembedded-core mailing list > > Openembedded-core@lists.openembedded.org > > > > > > http://lists.openembedded.org/mailman/listinfo/openembedded-core > > > > > > > > > > -- > > "Thou shalt not follow the NULL pointer, for chaos and madness > await thee at its > > end" > > > > > -- > "Thou shalt not follow the NULL pointer, for chaos and madness await > thee at its end" --------------000909020302080901080309 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit
On 03/23/2016 12:49 AM, Bruce Ashfield wrote:


On Tue, Mar 22, 2016 at 12:18 PM, Mark Hatle <mark.hatle@windriver.com> wrote:
On 3/22/16 7:21 AM, Bruce Ashfield wrote:
>
>
> On Tue, Mar 22, 2016 at 8:12 AM, Hongxu Jia <hongxu.jia@windriver.com
> <mailto:hongxu.jia@windriver.com>> wrote:
>
>     Since CFLAGS CPPFLAGS CXXFLAGS has been unset, variable DEBUG_FLAGS could
>     not been passed to compiler, so we explicitly add DEBUG_FLAGS to CC to
>     replace build path with target path.
>
>
> Can you be more explicit here ? What is typically contained in DEBUG_FLAGS ? The
> kernel builds its own compiler line and flags, so anything you are setting from this
> environment, should not be leaking into the kernel build.
>
> .. which leaves me wondering, what exactly is in DEBUG_FLAGS, and how is it
> actually fixing the QA issue ?

The specific debug flag in question is being added to remap the on-disk path to
the path we want the items to look like from a debugging point of view.

The typical format of DEBUG_FLAGS is something like:

DEBUG_FLAGS ?= "-g -feliminate-unused-debug-types"

This was recently updated to include the remapping:

 DEBUG_FLAGS ?= "-g -feliminate-unused-debug-types \
                 -fdebug-prefix-map=${WORKDIR}=/usr/src/debug \
                 -fdebug-prefix-map=${STAGING_DIR_NATIVE}= \
                 -fdebug-prefix-map=${STAGING_DIR_HOST}= \

The idea is the WORKDIR is replaced with '/usr/src/debug' inside of the binary
and any debug symbol references.  This makes it much easier for the system to
work with supplied debug sources/symbols.

I'm not sure you want to use DEBUG_FLAGS since anything can be put in there....

Application space code should always use the DEBUG_FLAGS, or I think it's likely
a bug.

The kernel though should probably define it's own flag(s).  Most likely you'll
want something simple like:

-fdebug-prefix-map=${WORKDIR}=/usr/src/debug


I can see that.

What outputs of the kernel build need this remapping ? The userspace parts ?
Something else ?


While building kernel, there are many build paths which caused by macro __FILE__.
Once you invoke marco WARN_ON in kernel/module, there will be a __FILE__ used.
....
#define WARN_ON(condition) ({                                           \
        int __ret_warn_on = !!(condition);                              \
        if (unlikely(__ret_warn_on))                                    \
                __WARN();                                               \
        unlikely(__ret_warn_on);                                        \
})

#define __WARN()                warn_slowpath_null(__FILE__, __LINE__)
....

That's why I add -fdebug-prefix-map to kernel build. I original added a new option
-ffile-prefix-map for gcc to remap __FILE__, and enhance -fdebug-prefix-map
to replace -ffile-prefix-map to compat with external toolchain.

The point is that we patch the kernel build, or make a specific Kconfig option, or
something similar to that, versus leaking our build flags into the kernel build or
"bad things can happen".


If the leaking idea is bad, would you like to accept '-fdebug-prefix-map' as
kernel's private?

Or suggestions else to fix the build path in kernel?

//Hongxu

So I'm sure the solution in the build is the same, but passing flags via a general
mechanism like I saw in the patch is generally not a good idea.

Bruce
 

Then all of the internal references to WORKDIR will be replaced with
'/usr/src/debug' which cross gdb can 'map' or on-target will actually look in
that location.  (A corresponding '-dbg' package should be provided with the
matching compilation sources at the local specified.)

--Mark

> Bruce
>
>
>
>     [YOCTO #7058]
>
>     Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com
>     <mailto:hongxu.jia@windriver.com>>
>     ---
>      meta/classes/kernel.bbclass | 4 ++--
>      1 file changed, 2 insertions(+), 2 deletions(-)
>
>     diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
>     index c3eab50..d357ccf 100644
>     --- a/meta/classes/kernel.bbclass
>     +++ b/meta/classes/kernel.bbclass
>     @@ -207,7 +207,7 @@ kernel_do_compile() {
>                     copy_initramfs
>
>     use_alternate_initrd=CONFIG_INITRAMFS_SOURCE=${B}/usr/${INITRAMFS_IMAGE}-${MACHINE}.cpio
>             fi
>     -       oe_runmake ${KERNEL_IMAGETYPE_FOR_MAKE} ${KERNEL_ALT_IMAGETYPE}
>     CC="${KERNEL_CC}" LD="${KERNEL_LD}" ${KERNEL_EXTRA_ARGS} $use_alternate_initrd
>     +       oe_runmake ${KERNEL_IMAGETYPE_FOR_MAKE} ${KERNEL_ALT_IMAGETYPE}
>     CC="${KERNEL_CC} ${DEBUG_FLAGS}" LD="${KERNEL_LD}" ${KERNEL_EXTRA_ARGS}
>     $use_alternate_initrd
>             if test "${KERNEL_IMAGETYPE_FOR_MAKE}.gz" = "${KERNEL_IMAGETYPE}"; then
>                     gzip -9c < "${KERNEL_IMAGETYPE_FOR_MAKE}" > "${KERNEL_OUTPUT}"
>             fi
>     @@ -216,7 +216,7 @@ kernel_do_compile() {
>      do_compile_kernelmodules() {
>             unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE
>             if (grep -q -i -e '^CONFIG_MODULES=y$' ${B}/.config); then
>     -               oe_runmake -C ${B} ${PARALLEL_MAKE} modules
>     CC="${KERNEL_CC}" LD="${KERNEL_LD}" ${KERNEL_EXTRA_ARGS}
>     +               oe_runmake -C ${B} ${PARALLEL_MAKE} modules CC="${KERNEL_CC}
>     ${DEBUG_FLAGS}" LD="${KERNEL_LD}" ${KERNEL_EXTRA_ARGS}
>
>                     # Module.symvers gets updated during the
>                     # building of the kernel modules. We need to
>     --
>     1.9.1
>
>     --
>     _______________________________________________
>     Openembedded-core mailing list
>     Openembedded-core@lists.openembedded.org
>     <mailto:Openembedded-core@lists.openembedded.org>
>     http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
>
>
>
> --
> "Thou shalt not follow the NULL pointer, for chaos and madness await thee at its
> end"




--
"Thou shalt not follow the NULL pointer, for chaos and madness await thee at its end"

--------------000909020302080901080309--