[PATCH v2 1/2] debug: Refactor and expand _FORTIFY_SOURCE tests
Siddhesh Poyarekar
siddhesh@sourceware.org
Fri Jan 7 02:09:13 GMT 2022
On 07/01/2022 02:21, Adhemerval Zanella wrote:
>
>
> 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.
While it works for _FILE_OFFSET_BITS, I doubt if it'll work for
_GNU_SOURCE since it will get overridden by libc-symbols.h.
At some point I want to do a cleanup where libc-symbols.h undefines
_GNU_SOURCE for tests and only tests that require it, define it
explicitly. Then we can properly auto-generate _FORTIFY_SOURCE tests
out of tst-chk-0.c just like we do for mcheck and malloc-check tests.
May I keep it like this for now and do this additional cleanup later?
>
>> -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.
OK.
>
>>
>> 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.
OK.
>
>> 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"
>
Thanks,
Siddhesh
More information about the Libc-alpha
mailing list