All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Wolsieffer <ben.wolsieffer@hefring.com>
To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH] package/uclibc: fix usage of DODEBUG option
Date: Wed, 3 Aug 2022 16:05:49 -0400	[thread overview]
Message-ID: <YurVHdu+AIRC3eoy@hefring> (raw)
In-Reply-To: <20220803002933.0e959c3b@windsurf>

Thanks for the review!

On Wed, Aug 03, 2022 at 12:29:33AM +0200, Thomas Petazzoni 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.
> 

Ok, I will make this unconditional in v2.

> > 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.
> 

I haven't experienced any problems due to these flags, but I can't prove
they won't break things for others, so I will use your suggestion for
v2.

Ben
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

      reply	other threads:[~2022-08-03 20:05 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
2022-08-03 20:05   ` Ben Wolsieffer [this message]

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=YurVHdu+AIRC3eoy@hefring \
    --to=ben.wolsieffer@hefring.com \
    --cc=buildroot@buildroot.org \
    --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.