Re: [PATCH] newlib: Remove debug flag for --enable-target-optspace

Hi Andrew and Jon,

> Actually you are wrong.  --enable-target-optspace changes the (normal)
CFLAGS to be "-g -O2 -g -Os".  The "-g" in the config/mt-ospace is
redundant now.

It does seem that way (very messy though ...).

> The reasoning Stephanos is giving of removing the "-g" there is not
correct though.

If you are overriding CFLAGS for other purposes and want to _only_ enable `-Os`, you cannot use `--enable-target-optspace` because it has an unwanted side effect of enabling debug information output alongside optimisation for size. After all, the option is called `--enable-target-optspace`, not `--enable-target-optspace-and-debug-info`.

What is more frustrating is that the documentation for `--enable-target-optspace` mentions nothing about enabling debug information output and this can be very confusing and misleading to users; I spent hours trying out many different build options in attempt to figure out why the compiled libraries included debug information in spite of never being instructed to do so, until I finally decided to have a look at the newlib build scripts.

*By removing `-g` in `mt-ospace`, we can support both cases:*

1. CFLAGS is not overridden: Since the default CFLAGS is `-g -O2`, `-Os` will override`-O2` to optimise for size. 
2. CFLAGS is overridden: It will simply add `-Os` to optimise for size. If user wanted debug information output, he/she will add "-g" in the CFLAGS separately.

One could possibly argue that user can simply put `-Os` in the overridden CFLAGS instead of using `--enable-target-optspace`, but that is not a valid excuse for the latter to misbehave.

> Because many people want to be able to debug their code!

Some might, some might not- especially if they are compiling newlib to be used by someone else, which would often be the case; the debug information is not very useful unless you have a local copy of the newlib source code.

For instance, GNU ARM Embedded build script is setting CFLAGS to `-Os` instead of using `--enable-target-optspace` because the latter is not achieving the intended behavior.

More importantly, I do not think any normal average person would somehow automatically assume `--enable-target-optspace` must mean enabling both optimisation for size and debugging information output!

Regards,

Stephanos
________________________________________
From: Andrew Pinski <[email protected]>
Sent: 30 November 2019 10:13
To: Jon Beniston <[email protected]>
Cc: Stephanos Ioannidis <[email protected]>; <[email protected]> <[email protected]>
Subject: Re: [PATCH] newlib: Remove debug flag for --enable-target-optspace 
 
On Fri, Nov 29, 2019 at 3:19 AM Jon Beniston <[email protected]> wrote:
>
> Hi Stephanos,
>
> >As you suggested, the debug information added by `--enable-target-optspace`
> can be stripped, but why add it in the first place if it is not necessary?
>
> Because many people want to be able to debug their code!
>
> >Unless there is a good reason to keep `-g` and `-Os` together in
> `--enable-target-optspace`,
> >I strongly believe `-g` should be removed and moved into a separate
> configuration option.
>
> Currently the default for CFLAGS_FOR_TARGET is "-g -O2".
> --enable-target-optspace Changes this to "-g -Os"

Actually you are wrong.  --enable-target-optspace changes the (normal)
CFLAGS to be "-g -O2 -g -Os".  The "-g" in the config/mt-ospace is
redundant now.
Please see https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02062.html as
that changed CFLAGS from being destroyed to be additional.

The reasoning Stephanos is giving of removing the "-g" there is not
correct though.  I am saying the patch is correct but the REASON
behind of why it is correct is NOT.  The "-g" is redudant and can be
removed from config/mt-ospace.

Thanks,
Andrew Pinski

>
> If -g is to be controlled by a separate option, then it should apply in both
> cases. And this new option should probably be enabled by default to maintain
> existing behaviour (so perhaps add a --disable-target-debug option that
> removes -g)
>
> Cheers,
> Jon
>
>