[PATCH v2 1/2] debug: Refactor and expand _FORTIFY_SOURCE tests

Adhemerval Zanella adhemerval.zanella@linaro.org
Thu Jan 6 20:51:48 GMT 2022



On 05/01/2022 01:45, Siddhesh Poyarekar via Libc-alpha wrote:
> Rename all debug/tst-chk* tests to reflect the fortification level
> they're testing and any additional macros so that rules for them can be
> autogenerated.  tst-chk0* are without fortification, tst-chk1 for
> _FORTIFY_SOURCE=1 and so on.  This allows easier replication of the
> tests to check additional macros.
> 
> The change also expands the -lfs to include _FORTIFY_SOURCE=3.
> 
> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

Looks good in general, some comments below.

> ---
>  debug/Makefile                               | 75 +++++++++-----------
>  debug/{tst-lfschk1.c => tst-chk-0-cc-lfs.cc} |  2 +-
>  debug/tst-chk-0-cc.cc                        |  1 +
>  debug/{tst-lfschk2.c => tst-chk-0-lfs.c}     |  2 +-
>  debug/{tst-chk1.c => tst-chk-0.c}            |  0
>  debug/{tst-lfschk3.c => tst-chk-1-cc-lfs.cc} |  2 +-
>  debug/{tst-chk2.c => tst-chk-1-cc.cc}        |  2 +-
>  debug/{tst-lfschk4.cc => tst-chk-1-lfs.c}    |  2 +-
>  debug/{tst-chk5.cc => tst-chk-1.c}           |  2 +-
>  debug/tst-chk-2-cc-lfs.cc                    |  2 +
>  debug/{tst-chk6.cc => tst-chk-2-cc.cc}       |  2 +-
>  debug/tst-chk-2-lfs.c                        |  2 +
>  debug/{tst-chk3.c => tst-chk-2.c}            |  2 +-
>  debug/tst-chk-3-cc-lfs.cc                    |  2 +
>  debug/{tst-chk7.c => tst-chk-3-cc.cc}        |  2 +-
>  debug/tst-chk-3-lfs.c                        |  2 +
>  debug/{tst-chk8.cc => tst-chk-3.c}           |  2 +-
>  debug/tst-chk4.cc                            |  1 -
>  debug/tst-lfschk5.cc                         |  2 -
>  debug/tst-lfschk6.cc                         |  2 -
>  20 files changed, 52 insertions(+), 57 deletions(-)
>  rename debug/{tst-lfschk1.c => tst-chk-0-cc-lfs.cc} (55%)
>  create mode 100644 debug/tst-chk-0-cc.cc
>  rename debug/{tst-lfschk2.c => tst-chk-0-lfs.c} (55%)
>  rename debug/{tst-chk1.c => tst-chk-0.c} (100%)
>  rename debug/{tst-lfschk3.c => tst-chk-1-cc-lfs.cc} (55%)
>  rename debug/{tst-chk2.c => tst-chk-1-cc.cc} (53%)
>  rename debug/{tst-lfschk4.cc => tst-chk-1-lfs.c} (55%)
>  rename debug/{tst-chk5.cc => tst-chk-1.c} (53%)
>  create mode 100644 debug/tst-chk-2-cc-lfs.cc
>  rename debug/{tst-chk6.cc => tst-chk-2-cc.cc} (53%)
>  create mode 100644 debug/tst-chk-2-lfs.c
>  rename debug/{tst-chk3.c => tst-chk-2.c} (53%)
>  create mode 100644 debug/tst-chk-3-cc-lfs.cc
>  rename debug/{tst-chk7.c => tst-chk-3-cc.cc} (53%)
>  create mode 100644 debug/tst-chk-3-lfs.c
>  rename debug/{tst-chk8.cc => tst-chk-3.c} (53%)
>  delete mode 100644 debug/tst-chk4.cc
>  delete mode 100644 debug/tst-lfschk5.cc
>  delete mode 100644 debug/tst-lfschk6.cc
> 
> diff --git a/debug/Makefile b/debug/Makefile
> index 9c2ce61a86..9aa27eb00c 100644
> --- a/debug/Makefile
> +++ b/debug/Makefile
> @@ -1,4 +1,5 @@
>  # Copyright (C) 1998-2022 Free Software Foundation, Inc.
> +# Copyright The GNU Toolchain Authors.
>  # This file is part of the GNU C Library.
>  
>  # The GNU C Library is free software; you can redistribute it and/or
> @@ -110,32 +111,31 @@ CFLAGS-tst-longjmp_chk3.c += -fexceptions -fasynchronous-unwind-tables
>  CPPFLAGS-tst-longjmp_chk3.c += -D_FORTIFY_SOURCE=1
>  CPPFLAGS-tst-realpath-chk.c += -D_FORTIFY_SOURCE=2
>  
> +# _FORTIFY_SOURCE tests.
> +tests-chk = $(addprefix tst-chk-, 0 1 2 3)
> +tests-chk-cc = $(addsuffix -cc, $(tests-chk))
> +tests-chk-lfs = $(addsuffix -lfs, $(tests-chk))
> +tests-chk-cc-lfs = $(addsuffix -lfs, $(tests-chk-cc))
> +
>  # We know these tests have problems with format strings, this is what
>  # we are testing.  Disable that warning.  They are also testing
>  # deprecated functions (notably gets) so disable that warning as well.
>  # And they also generate warnings from warning attributes, which
>  # cannot be disabled via pragmas, so require -Wno-error to be used.
> -CFLAGS-tst-chk1.c += -Wno-format -Wno-deprecated-declarations -Wno-error
> -CFLAGS-tst-chk2.c += -Wno-format -Wno-deprecated-declarations -Wno-error
> -CFLAGS-tst-chk3.c += -Wno-format -Wno-deprecated-declarations -Wno-error
> -CFLAGS-tst-chk4.cc += -Wno-format -Wno-deprecated-declarations -Wno-error
> -CFLAGS-tst-chk5.cc += -Wno-format -Wno-deprecated-declarations -Wno-error
> -CFLAGS-tst-chk6.cc += -Wno-format -Wno-deprecated-declarations -Wno-error
> -CFLAGS-tst-chk7.c += -Wno-format -Wno-deprecated-declarations -Wno-error
> -CFLAGS-tst-chk8.cc += -Wno-format -Wno-deprecated-declarations -Wno-error
> -CFLAGS-tst-lfschk1.c += -Wno-format -Wno-deprecated-declarations -Wno-error
> -CFLAGS-tst-lfschk2.c += -Wno-format -Wno-deprecated-declarations -Wno-error
> -CFLAGS-tst-lfschk3.c += -Wno-format -Wno-deprecated-declarations -Wno-error
> -CFLAGS-tst-lfschk4.cc += -Wno-format -Wno-deprecated-declarations -Wno-error
> -CFLAGS-tst-lfschk5.cc += -Wno-format -Wno-deprecated-declarations -Wno-error
> -CFLAGS-tst-lfschk6.cc += -Wno-format -Wno-deprecated-declarations -Wno-error
> -LDLIBS-tst-chk4 = -lstdc++
> -LDLIBS-tst-chk5 = -lstdc++
> -LDLIBS-tst-chk6 = -lstdc++
> -LDLIBS-tst-chk8 = -lstdc++
> -LDLIBS-tst-lfschk4 = -lstdc++
> -LDLIBS-tst-lfschk5 = -lstdc++
> -LDLIBS-tst-lfschk6 = -lstdc++
> +define disable-warnings
> +CFLAGS-$(1).$(2) += -Wno-format -Wno-deprecated-declarations -Wno-error
> +endef
> +
> +$(foreach t,$(tests-chk) $(tests-chk-lfs), \
> +	  $(eval $(call disable-warnings,$(t),c)))
> +
> +$(foreach t,$(tests-chk-cc) $(tests-chk-cc-lfs), \
> +	  $(eval $(call disable-warnings,$(t),cc)))
> +
> +define link-cc
> +LDLIBS-$(1) = -lstdc++
> +endef
> +$(foreach t,$(tests-chk-cc) $(tests-chk-cc-lfs), $(eval $(call link-cc,$(t))))
>  
>  # backtrace_symbols only works if we link with -rdynamic.  backtrace
>  # requires unwind tables on most architectures.
> @@ -152,19 +152,18 @@ LDFLAGS-tst-backtrace6 = -rdynamic
>  
>  CFLAGS-tst-ssp-1.c += -fstack-protector-all
>  

Nice, it does simplify the required rules.  Maybe you could also extend it to
LFS as well:

diff --git a/debug/Makefile b/debug/Makefile
index dc73600213..019046391b 100644
--- a/debug/Makefile
+++ b/debug/Makefile
@@ -127,6 +127,9 @@ tests-chk-cc-nongnu = $(addsuffix -nongnu, $(tests-chk-cc))
 define disable-warnings
 CFLAGS-$(1).$(2) += -Wno-format -Wno-deprecated-declarations -Wno-error
 endef
+define enable-lfs
+CFLAGS-$(1).$(2) += -D_FILE_OFFSET_BITS=64
+endef
 
 $(foreach t,$(tests-chk) $(tests-chk-lfs) $(tests-chk-nongnu), \
 	  $(eval $(call disable-warnings,$(t),c)))
@@ -134,6 +137,9 @@ $(foreach t,$(tests-chk) $(tests-chk-lfs) $(tests-chk-nongnu), \
 $(foreach t,$(tests-chk-cc) $(tests-chk-cc-lfs) $(tests-chk-cc-nongnu), \
 	  $(eval $(call disable-warnings,$(t),cc)))
 
+$(foreach t,$(tests-chk-lfs), $(eval $(call enable-lfs,$(t),c)))
+$(foreach t,$(tests-chk-cc-lfs), $(eval $(call enable-lfs,$(t),cc)))
+
 define link-cc
 LDLIBS-$(1) = -lstdc++
 endef

And then remove the '#define _FILE_OFFSET_BITS 64' so the rule is applied
regardless whether the file defines or not.

> -tests = backtrace-tst tst-longjmp_chk tst-chk1 tst-chk2 tst-chk3 \
> -	tst-lfschk1 tst-lfschk2 tst-lfschk3 test-strcpy_chk test-stpcpy_chk \
> -	tst-chk4 tst-chk5 tst-chk6 tst-chk7 tst-chk8 tst-lfschk4 tst-lfschk5 \
> -	tst-lfschk6 tst-longjmp_chk2 tst-backtrace2 tst-backtrace3 \
> -	tst-backtrace4 tst-backtrace5 tst-backtrace6 tst-realpath-chk
> +tests = backtrace-tst tst-longjmp_chk \
> +	test-strcpy_chk test-stpcpy_chk \
> +	tst-longjmp_chk2 tst-backtrace2 tst-backtrace3 \
> +	tst-backtrace4 tst-backtrace5 tst-backtrace6 tst-realpath-chk \
> +	$(tests-chk) $(tests-chk-cc) $(tests-chk-lfs) $(tests-chk-cc-lfs)

Since you are touching, put one file per line.

>  
>  ifeq ($(have-ssp),yes)
>  tests += tst-ssp-1
>  endif
>  
>  ifeq (,$(CXX))
> -tests-unsupported = tst-chk4 tst-chk5 tst-chk6 tst-chk8 \
> -		    tst-lfschk4 tst-lfschk5 tst-lfschk6
> +tests-unsupported = $(tests-chk-cc) $(tests-chk-cc-lfs)

Same.

>  endif
>  
>  extra-libs = libSegFault libpcprofile
> @@ -191,20 +190,12 @@ ifeq ($(run-built-tests),yes)
>  LOCALES := de_DE.UTF-8
>  include ../gen-locales.mk
>  
> -$(objpfx)tst-chk1.out: $(gen-locales)
> -$(objpfx)tst-chk2.out: $(gen-locales)
> -$(objpfx)tst-chk3.out: $(gen-locales)
> -$(objpfx)tst-chk4.out: $(gen-locales)
> -$(objpfx)tst-chk5.out: $(gen-locales)
> -$(objpfx)tst-chk6.out: $(gen-locales)
> -$(objpfx)tst-chk7.out: $(gen-locales)
> -$(objpfx)tst-chk8.out: $(gen-locales)
> -$(objpfx)tst-lfschk1.out: $(gen-locales)
> -$(objpfx)tst-lfschk2.out: $(gen-locales)
> -$(objpfx)tst-lfschk3.out: $(gen-locales)
> -$(objpfx)tst-lfschk4.out: $(gen-locales)
> -$(objpfx)tst-lfschk5.out: $(gen-locales)
> -$(objpfx)tst-lfschk6.out: $(gen-locales)
> +define chk-gen-locales
> +$(objpfx)$(1).out: $(gen-locales)
> +endef
> +$(foreach t, \
> +	  $(tests-chk) $(tests-chk-cc) $(tests-chk-lfs) $(tests-chk-cc-lfs), \
> +	  $(eval $(call link-cc,$(t))))
>  endif
>  
>  sLIBdir := $(shell echo $(slibdir) | sed 's,lib\(\|64\)$$,\\\\$$LIB,')

Same.

> diff --git a/debug/tst-lfschk1.c b/debug/tst-chk-0-cc-lfs.cc
> similarity index 55%
> rename from debug/tst-lfschk1.c
> rename to debug/tst-chk-0-cc-lfs.cc
> index f3e6d47d5e..db699b2391 100644
> --- a/debug/tst-lfschk1.c
> +++ b/debug/tst-chk-0-cc-lfs.cc
> @@ -1,2 +1,2 @@
>  #define _FILE_OFFSET_BITS 64
> -#include "tst-chk1.c"
> +#include "tst-chk-0.c"
> diff --git a/debug/tst-chk-0-cc.cc b/debug/tst-chk-0-cc.cc
> new file mode 100644
> index 0000000000..be76fc69d4
> --- /dev/null
> +++ b/debug/tst-chk-0-cc.cc
> @@ -0,0 +1 @@
> +#include "tst-chk-0.c"
> diff --git a/debug/tst-lfschk2.c b/debug/tst-chk-0-lfs.c
> similarity index 55%
> rename from debug/tst-lfschk2.c
> rename to debug/tst-chk-0-lfs.c
> index 95d4db1d32..db699b2391 100644
> --- a/debug/tst-lfschk2.c
> +++ b/debug/tst-chk-0-lfs.c
> @@ -1,2 +1,2 @@
>  #define _FILE_OFFSET_BITS 64
> -#include "tst-chk2.c"
> +#include "tst-chk-0.c"
> diff --git a/debug/tst-chk1.c b/debug/tst-chk-0.c
> similarity index 100%
> rename from debug/tst-chk1.c
> rename to debug/tst-chk-0.c
> diff --git a/debug/tst-lfschk3.c b/debug/tst-chk-1-cc-lfs.cc
> similarity index 55%
> rename from debug/tst-lfschk3.c
> rename to debug/tst-chk-1-cc-lfs.cc
> index 50a1ae1258..0d67643e9e 100644
> --- a/debug/tst-lfschk3.c
> +++ b/debug/tst-chk-1-cc-lfs.cc
> @@ -1,2 +1,2 @@
>  #define _FILE_OFFSET_BITS 64
> -#include "tst-chk3.c"
> +#include "tst-chk-1.c"
> diff --git a/debug/tst-chk2.c b/debug/tst-chk-1-cc.cc
> similarity index 53%
> rename from debug/tst-chk2.c
> rename to debug/tst-chk-1-cc.cc
> index be37ce2d22..945c1f8d5b 100644
> --- a/debug/tst-chk2.c
> +++ b/debug/tst-chk-1-cc.cc
> @@ -1,2 +1,2 @@
>  #define _FORTIFY_SOURCE 1
> -#include "tst-chk1.c"
> +#include "tst-chk-0.c"
> diff --git a/debug/tst-lfschk4.cc b/debug/tst-chk-1-lfs.c
> similarity index 55%
> rename from debug/tst-lfschk4.cc
> rename to debug/tst-chk-1-lfs.c
> index f3e6d47d5e..0d67643e9e 100644
> --- a/debug/tst-lfschk4.cc
> +++ b/debug/tst-chk-1-lfs.c
> @@ -1,2 +1,2 @@
>  #define _FILE_OFFSET_BITS 64
> -#include "tst-chk1.c"
> +#include "tst-chk-1.c"
> diff --git a/debug/tst-chk5.cc b/debug/tst-chk-1.c
> similarity index 53%
> rename from debug/tst-chk5.cc
> rename to debug/tst-chk-1.c
> index be37ce2d22..945c1f8d5b 100644
> --- a/debug/tst-chk5.cc
> +++ b/debug/tst-chk-1.c
> @@ -1,2 +1,2 @@
>  #define _FORTIFY_SOURCE 1
> -#include "tst-chk1.c"
> +#include "tst-chk-0.c"
> diff --git a/debug/tst-chk-2-cc-lfs.cc b/debug/tst-chk-2-cc-lfs.cc
> new file mode 100644
> index 0000000000..97c83def5c
> --- /dev/null
> +++ b/debug/tst-chk-2-cc-lfs.cc
> @@ -0,0 +1,2 @@
> +#define _FILE_OFFSET_BITS 64
> +#include "tst-chk-2.c"
> diff --git a/debug/tst-chk6.cc b/debug/tst-chk-2-cc.cc
> similarity index 53%
> rename from debug/tst-chk6.cc
> rename to debug/tst-chk-2-cc.cc
> index 38b8e4fb36..f178340bbd 100644
> --- a/debug/tst-chk6.cc
> +++ b/debug/tst-chk-2-cc.cc
> @@ -1,2 +1,2 @@
>  #define _FORTIFY_SOURCE 2
> -#include "tst-chk1.c"
> +#include "tst-chk-0.c"
> diff --git a/debug/tst-chk-2-lfs.c b/debug/tst-chk-2-lfs.c
> new file mode 100644
> index 0000000000..97c83def5c
> --- /dev/null
> +++ b/debug/tst-chk-2-lfs.c
> @@ -0,0 +1,2 @@
> +#define _FILE_OFFSET_BITS 64
> +#include "tst-chk-2.c"
> diff --git a/debug/tst-chk3.c b/debug/tst-chk-2.c
> similarity index 53%
> rename from debug/tst-chk3.c
> rename to debug/tst-chk-2.c
> index 38b8e4fb36..f178340bbd 100644
> --- a/debug/tst-chk3.c
> +++ b/debug/tst-chk-2.c
> @@ -1,2 +1,2 @@
>  #define _FORTIFY_SOURCE 2
> -#include "tst-chk1.c"
> +#include "tst-chk-0.c"
> diff --git a/debug/tst-chk-3-cc-lfs.cc b/debug/tst-chk-3-cc-lfs.cc
> new file mode 100644
> index 0000000000..d5d8c28bc7
> --- /dev/null
> +++ b/debug/tst-chk-3-cc-lfs.cc
> @@ -0,0 +1,2 @@
> +#define _FILE_OFFSET_BITS 64
> +#include "tst-chk-3.c"
> diff --git a/debug/tst-chk7.c b/debug/tst-chk-3-cc.cc
> similarity index 53%
> rename from debug/tst-chk7.c
> rename to debug/tst-chk-3-cc.cc
> index 2a7b323812..c1ddac1261 100644
> --- a/debug/tst-chk7.c
> +++ b/debug/tst-chk-3-cc.cc
> @@ -1,2 +1,2 @@
>  #define _FORTIFY_SOURCE 3
> -#include "tst-chk1.c"
> +#include "tst-chk-0.c"
> diff --git a/debug/tst-chk-3-lfs.c b/debug/tst-chk-3-lfs.c
> new file mode 100644
> index 0000000000..d5d8c28bc7
> --- /dev/null
> +++ b/debug/tst-chk-3-lfs.c
> @@ -0,0 +1,2 @@
> +#define _FILE_OFFSET_BITS 64
> +#include "tst-chk-3.c"
> diff --git a/debug/tst-chk8.cc b/debug/tst-chk-3.c
> similarity index 53%
> rename from debug/tst-chk8.cc
> rename to debug/tst-chk-3.c
> index 2a7b323812..c1ddac1261 100644
> --- a/debug/tst-chk8.cc
> +++ b/debug/tst-chk-3.c
> @@ -1,2 +1,2 @@
>  #define _FORTIFY_SOURCE 3
> -#include "tst-chk1.c"
> +#include "tst-chk-0.c"
> diff --git a/debug/tst-chk4.cc b/debug/tst-chk4.cc
> deleted file mode 100644
> index c82e6aac86..0000000000
> --- a/debug/tst-chk4.cc
> +++ /dev/null
> @@ -1 +0,0 @@
> -#include "tst-chk1.c"
> diff --git a/debug/tst-lfschk5.cc b/debug/tst-lfschk5.cc
> deleted file mode 100644
> index 95d4db1d32..0000000000
> --- a/debug/tst-lfschk5.cc
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -#define _FILE_OFFSET_BITS 64
> -#include "tst-chk2.c"
> diff --git a/debug/tst-lfschk6.cc b/debug/tst-lfschk6.cc
> deleted file mode 100644
> index 50a1ae1258..0000000000
> --- a/debug/tst-lfschk6.cc
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -#define _FILE_OFFSET_BITS 64
> -#include "tst-chk3.c"


More information about the Libc-alpha mailing list