public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] newlib: undefine _FORTIFY_SOURCE
@ 2021-11-09  2:56 Mike Frysinger
  2021-11-09  2:59 ` [PATCH alternative] newlib: ignore _FORTIFY_SOURCE when building newlib Mike Frysinger
  2021-11-09  3:15 ` [PATCH] newlib: undefine _FORTIFY_SOURCE Keith Packard
  0 siblings, 2 replies; 6+ messages in thread
From: Mike Frysinger @ 2021-11-09  2:56 UTC (permalink / raw)
  To: newlib

Some distros enable _FORTIFY_SOURCE by default which upsets building
newlib which itself implements the logic for this define.  For example,
building gets.c fails because the includes set up a gets() macro which
expands in the definition.

Since newlib isn't prepared to build itself with _FORTIFY_SOURCE, and
it's not clear if it's even useful, disable it when building the code.
This also matches what glibc is doing.
---
 newlib/configure.host | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/newlib/configure.host b/newlib/configure.host
index e737f73f5f75..cb0c5d01f874 100644
--- a/newlib/configure.host
+++ b/newlib/configure.host
@@ -54,7 +54,7 @@
 #   have_init_fini	have init/fini ("yes" or "no", set to "yes" by default)
 #   noinclude		list of include files to not install
 
-newlib_cflags="-D_COMPILING_NEWLIB"
+newlib_cflags="-D_COMPILING_NEWLIB -U_FORTIFY_SOURCE"
 libm_machine_dir=
 machine_dir=
 shared_machine_dir=
-- 
2.33.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH alternative] newlib: ignore _FORTIFY_SOURCE when building newlib
  2021-11-09  2:56 [PATCH] newlib: undefine _FORTIFY_SOURCE Mike Frysinger
@ 2021-11-09  2:59 ` Mike Frysinger
  2021-11-12 10:28   ` Corinna Vinschen
  2021-11-09  3:15 ` [PATCH] newlib: undefine _FORTIFY_SOURCE Keith Packard
  1 sibling, 1 reply; 6+ messages in thread
From: Mike Frysinger @ 2021-11-09  2:59 UTC (permalink / raw)
  To: newlib

Some distros enable _FORTIFY_SOURCE by default which upsets building
newlib which itself implements the logic for this define.  For example,
building gets.c fails because the includes set up a gets() macro which
expands in the definition.

Since newlib isn't prepared to build itself with _FORTIFY_SOURCE, and
it's not clear if it's even useful, ignore it when building the code.
This also matches what glibc is doing.
---
NB: This is an alternative approach instead of passing -U_FORTIFY_SOURCE.
I think the -U approach is better, but throwing this up for discussion.

 newlib/libc/include/sys/features.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/newlib/libc/include/sys/features.h b/newlib/libc/include/sys/features.h
index 2188071785f6..6b4999e83482 100644
--- a/newlib/libc/include/sys/features.h
+++ b/newlib/libc/include/sys/features.h
@@ -320,7 +320,8 @@ extern "C" {
 #endif
 
 #if _FORTIFY_SOURCE > 0 && !defined(__cplusplus) && !defined(__lint__) && \
-   (__OPTIMIZE__ > 0 || defined(__clang__)) && __GNUC_PREREQ__(4, 1)
+   (__OPTIMIZE__ > 0 || defined(__clang__)) && __GNUC_PREREQ__(4, 1) && \
+   !defined(_COMPILING_NEWLIB)
 #  if _FORTIFY_SOURCE > 1
 #    define __SSP_FORTIFY_LEVEL 2
 #  else
-- 
2.33.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] newlib: undefine _FORTIFY_SOURCE
  2021-11-09  2:56 [PATCH] newlib: undefine _FORTIFY_SOURCE Mike Frysinger
  2021-11-09  2:59 ` [PATCH alternative] newlib: ignore _FORTIFY_SOURCE when building newlib Mike Frysinger
@ 2021-11-09  3:15 ` Keith Packard
  2021-11-09 11:41   ` Corinna Vinschen
  2021-11-10  2:07   ` Mike Frysinger
  1 sibling, 2 replies; 6+ messages in thread
From: Keith Packard @ 2021-11-09  3:15 UTC (permalink / raw)
  To: Mike Frysinger, newlib

[-- Attachment #1: Type: text/plain, Size: 692 bytes --]

Mike Frysinger <vapier@gentoo.org> writes:

> Since newlib isn't prepared to build itself with _FORTIFY_SOURCE, and
> it's not clear if it's even useful, disable it when building the code.

It is useful; I've caught a few library bugs when I turned this on for
picolibc. Fixing newlib to handle -D_FORTIFY_SOURCE could probably start
with the patches I did in picolibc, which mostly just #undef the various
mem and str functions before defining them.

General code:

https://github.com/picolibc/picolibc/commit/c2c593afa60b64bff0a958ad930e0336463db5bf

Arm specific code:

https://github.com/picolibc/picolibc/commit/b3946840fb3a677125bc9232ca93ea7533375188


-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] newlib: undefine _FORTIFY_SOURCE
  2021-11-09  3:15 ` [PATCH] newlib: undefine _FORTIFY_SOURCE Keith Packard
@ 2021-11-09 11:41   ` Corinna Vinschen
  2021-11-10  2:07   ` Mike Frysinger
  1 sibling, 0 replies; 6+ messages in thread
From: Corinna Vinschen @ 2021-11-09 11:41 UTC (permalink / raw)
  To: newlib

On Nov  8 19:15, Keith Packard wrote:
> Mike Frysinger <vapier@gentoo.org> writes:
> 
> > Since newlib isn't prepared to build itself with _FORTIFY_SOURCE, and
> > it's not clear if it's even useful, disable it when building the code.
> 
> It is useful; I've caught a few library bugs when I turned this on for
> picolibc. Fixing newlib to handle -D_FORTIFY_SOURCE could probably start
> with the patches I did in picolibc, which mostly just #undef the various
> mem and str functions before defining them.

Please send patches.


Thanks,
Corinna



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] newlib: undefine _FORTIFY_SOURCE
  2021-11-09  3:15 ` [PATCH] newlib: undefine _FORTIFY_SOURCE Keith Packard
  2021-11-09 11:41   ` Corinna Vinschen
@ 2021-11-10  2:07   ` Mike Frysinger
  1 sibling, 0 replies; 6+ messages in thread
From: Mike Frysinger @ 2021-11-10  2:07 UTC (permalink / raw)
  To: Keith Packard; +Cc: newlib

[-- Attachment #1: Type: text/plain, Size: 649 bytes --]

On 08 Nov 2021 19:15, Keith Packard wrote:
> Mike Frysinger <vapier@gentoo.org> writes:
> > Since newlib isn't prepared to build itself with _FORTIFY_SOURCE, and
> > it's not clear if it's even useful, disable it when building the code.
> 
> It is useful; I've caught a few library bugs when I turned this on for
> picolibc. Fixing newlib to handle -D_FORTIFY_SOURCE could probably start
> with the patches I did in picolibc, which mostly just #undef the various
> mem and str functions before defining them.

i'm not against making it work, but it doesn't now, so we should disable the
logic if/until it's made to actually work.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH alternative] newlib: ignore _FORTIFY_SOURCE when building newlib
  2021-11-09  2:59 ` [PATCH alternative] newlib: ignore _FORTIFY_SOURCE when building newlib Mike Frysinger
@ 2021-11-12 10:28   ` Corinna Vinschen
  0 siblings, 0 replies; 6+ messages in thread
From: Corinna Vinschen @ 2021-11-12 10:28 UTC (permalink / raw)
  To: newlib

On Nov  8 21:59, Mike Frysinger wrote:
> Some distros enable _FORTIFY_SOURCE by default which upsets building
> newlib which itself implements the logic for this define.  For example,
> building gets.c fails because the includes set up a gets() macro which
> expands in the definition.
> 
> Since newlib isn't prepared to build itself with _FORTIFY_SOURCE, and
> it's not clear if it's even useful, ignore it when building the code.
> This also matches what glibc is doing.
> ---
> NB: This is an alternative approach instead of passing -U_FORTIFY_SOURCE.
> I think the -U approach is better, but throwing this up for discussion.
> 
>  newlib/libc/include/sys/features.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/newlib/libc/include/sys/features.h b/newlib/libc/include/sys/features.h
> index 2188071785f6..6b4999e83482 100644
> --- a/newlib/libc/include/sys/features.h
> +++ b/newlib/libc/include/sys/features.h
> @@ -320,7 +320,8 @@ extern "C" {
>  #endif
>  
>  #if _FORTIFY_SOURCE > 0 && !defined(__cplusplus) && !defined(__lint__) && \
> -   (__OPTIMIZE__ > 0 || defined(__clang__)) && __GNUC_PREREQ__(4, 1)
> +   (__OPTIMIZE__ > 0 || defined(__clang__)) && __GNUC_PREREQ__(4, 1) && \
> +   !defined(_COMPILING_NEWLIB)
>  #  if _FORTIFY_SOURCE > 1
>  #    define __SSP_FORTIFY_LEVEL 2
>  #  else
> -- 
> 2.33.0

I like this one more.  Most people have no problems to look into
source, but many shy away from the convoluted build system.


Thanks,
Corinna


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-12 10:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-09  2:56 [PATCH] newlib: undefine _FORTIFY_SOURCE Mike Frysinger
2021-11-09  2:59 ` [PATCH alternative] newlib: ignore _FORTIFY_SOURCE when building newlib Mike Frysinger
2021-11-12 10:28   ` Corinna Vinschen
2021-11-09  3:15 ` [PATCH] newlib: undefine _FORTIFY_SOURCE Keith Packard
2021-11-09 11:41   ` Corinna Vinschen
2021-11-10  2:07   ` Mike Frysinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).