All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: Ben Wolsieffer <ben.wolsieffer@hefring.com>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH] package/uclibc: fix usage of DODEBUG option
Date: Wed, 3 Aug 2022 00:29:33 +0200	[thread overview]
Message-ID: <20220803002933.0e959c3b@windsurf> (raw)
In-Reply-To: <20220802133625.1285283-1-Ben.Wolsieffer@hefring.com>

Hello Ben,

Thanks for this contribution, good change! Below some
comments/questions.

On Tue,  2 Aug 2022 09:36:25 -0400
Ben Wolsieffer <ben.wolsieffer@hefring.com> wrote:

> The DODEBUG option passes -O0 and -DDEBUG to the compiler, which has a
> significant impact on runtime behavior and performance. Currently,
> DODEBUG is enabled by BR2_ENABLE_DEBUG, but it makes more sense for it
> to be enabled by BR_ENABLE_RUNTIME_DEBUG.
> 
> This patch implements the above change, and also disables the DOSTRIP
> option when BR2_ENABLE_DEBUG is set, allowing debug info to be retained.

We want to disable stripping unconditionally. stripping is done by
Buildroot on the $(TARGET_DIR) when stripping is enabled. Ideally,
packages should not themselves strip binaries.

> Lastly, this patch passes all of $(TARGET_CFLAGS) to make, instead of
> just $(TARGET_ABI). This enables debug symbols when appropriate.

This has a bit more impact:

TARGET_CFLAGS = $(TARGET_CPPFLAGS) $(TARGET_ABI) $(TARGET_OPTIMIZATION) $(TARGET_DEBUGGING) $(TARGET_HARDENED)

TARGET_CPPFLAGS means we will now be passing -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64

TARGET_OPTIMIZATION means we will now be passing -Oxyz, perhaps
overriding the optimization level chosen by uClibc? We have seen that
causing problems with glibc for example, I don't know if we need to be
careful in a similar way with uClibc.

TARGET_HARDENED means we will be passing -D_FORTIFY_SOURCE=1/2 if
BR2_FORTIFY_SOURCE_1=y or BR2_FORTIFY_SOURCE_2=y.

I'm not saying no to this change, but again we need to consider the
impact.

One more careful option is perhaps to do:

UCLIBC_EXTRA_CFLAGS = $(TARGET_CFLAGS) $(TARGET_DEBUGGING)

This achieves the same goal of having the debugging symbols, without
risking of introducing extra flags.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  reply	other threads:[~2022-08-02 22:29 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-02 13:36 [Buildroot] [PATCH] package/uclibc: fix usage of DODEBUG option Ben Wolsieffer
2022-08-02 22:29 ` Thomas Petazzoni via buildroot [this message]
2022-08-03 20:05   ` Ben Wolsieffer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220803002933.0e959c3b@windsurf \
    --to=buildroot@buildroot.org \
    --cc=ben.wolsieffer@hefring.com \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.