Draft: Refactor testsuite#70
Open
ncopa wants to merge 169 commits into
Open
Conversation
GCC and Clang provide __builtin_dynamic_object_size (see documentation: https://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html), so we should make use of it when its available.
fortify/poll.h includes poll.h, which redirects the ppoll sys call to __ppoll_time64, if the _REDIR_TIME64 macro is 1. Then fortify/poll.h will #undef ppoll and use the 32 bit version, which is inconsistent. Taken from: openwrt/openwrt#12575
And make it available at https://jvoisin.github.io/fortify-headers/include/index.html
It seems useless and triggers 'error: expected external declaration'
- They're not used anywhere else in fortify-headers - It's breaking compilation on C++, because compatibility is hard It was initially reported on https://gitlab.alpinelinux.org/alpine/aports/-/issues/16200
This should fix the second part of jvoisin#59
just in case, and because 'PEDANTIC_CHECKS' is a really generic name
This fix the following issue:
```
In file included from exec-cmd.c:9:
In function 'vsnprintf',
inlined from 'report.constprop' at subcmd-util.h:13:2:
/usr/include/fortify/stdio.h:162:16: error: 'msg' may be used uninitialized [-Werror=maybe-uninitialized]
162 | return __orig_vsnprintf(__s, __n, __f, __v);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/fortify/strings.h:23,
from /usr/include/string.h:59,
from /usr/include/fortify/string.h:23,
from /home/ncopa/aports/main/linux-lts/src/linux-6.6/tools/include/linux/string.h:6,
from exec-cmd.c:3:
/usr/include/fortify/stdio.h: In function 'report.constprop':
/usr/include/fortify/stdio.h:152:1: note: in a call to '__orig_vsnprintf' declared with attribute 'access (read_write, 1, 2)' here
152 | _FORTIFY_FN(vsnprintf) int vsnprintf(char * _FORTIFY_POS0 __s, size_t __n,
| ^~~~~~~~~~~
In file included from exec-cmd.c:10:
subcmd-util.h:12:14: note: 'msg' declared here
12 | char msg[1024];
| ^~~
cc1: all warnings being treated as errors
make[5]: *** [/home/ncopa/aports/main/linux-lts/src/linux-6.6/tools/build/Makefile.build:98: /home/ncopa/aports/main/linux-lts/src/build-virt.x86_64/tools/objtool/libsubcmd/exec-cmd.o] Error 1
make[4]: *** [Makefile:80: /home/ncopa/aports/main/linux-lts/src/build-virt.x86_64/tools/objtool/libsubcmd/libsubcmd-in.o] Error 2
make[3]: *** [Makefile:78: /home/ncopa/aports/main/linux-lts/src/build-virt.x86_64/tools/objtool/libsubcmd/libsubcmd.a] Error 2
make[2]: *** [Makefile:73: objtool] Error 2
make[1]: *** [/home/ncopa/aports/main/linux-lts/src/linux-6.6/Makefile:1362: tools/objtool] Error 2
make: *** [/home/ncopa/aports/main/linux-lts/src/linux-6.6/Makefile:234: __sub-make] Error 2
```
Reported-by: ncopa
In the same spirit as the previous commit. Reported-by: ncopa
The dsize parameter is the length of the dst, not the length of the src. Reported-by: ncopa
As reported on irc: ``` 17:51 <q> jvoisin, fortify-headers seems to be broken (on Alpine at least) 17:52 <q> Repeating the message from over-there: 17:52 <q> /usr/include/fortify/string.h: In function 'strncat': 17:52 <q> /usr/include/fortify/string.h:297:36: error: implicit declaration of function 'strnlen'; did you mean 'strlen'? [-Wimplicit-function-declaration] 17:52 <q> This is with a simple file that includes string.h and call strncat, built with c99 -O1 f.c ```
It seems that annotating sprintf with `write` makes gcc unhappy, as its analyser is unable to understand that we're checking if `__b != -1` before calling `__orig_snprintf`, so let's comment this annotation for now.
Contributor
Author
|
we should also add |
Avoid installing *.orig or other files.
This fixes invalid conversion errors when the fd_set is defined as const. fixes jvoisin#66
As with previous commit, some strnlen calls where introduced in 22a8094, but not reverted. As strnlen isn't part of C standard, this was breaking C builds.
commit 459d202 (prefix special defines with FORTIFY_) changed define. Adjust the testsuite accordingly.
Use `gcc` as default and pass the CC compiler with the github workflow instead.
This makes it possible to do multiple runs without needing to clean in between.
This allows us to disable warnings on Alpine Linux by doing: make -C tests FORTIFY_CFLAGS="-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3" It also makes it possible to run the tests with -D_FORTIFY_SOURCE=2 without modifying the Makefile.
Set the system depended includes in github workflows, where we also define the syste (ubuntu, x86_64 etc). This makes it easier to run test suite on Alpine
- include a `config.mk` if it exists. This is useful to set local cflags
while debugging. For example:
echo "CFLAGS+=-U_FORTIFY_SOURCE" > config.mk
- Remove 'gcc' and 'clang' targets. Let user set that via CC.
- Add a `check` target, so user can run the test suite with
`make check`.
- set -nostdinc always to avoid pull in system headers
Contributor
|
Would it be possible to make a failing test result in a non-zero exit code with something like this? --- a/tests/Makefile
+++ b/tests/Makefile
@@ -185,9 +189,11 @@ cpp: test_compile.cc
timeout 1s ./test_compile_cc 1234567890 2 3 4 5 6 7 8 9 0 >/dev/null && echo "$(EXE) OK" || echo "$(EXE) FAIL"; \
run: $(RUNTIME_TARGETS)
+ r=0; \
$(foreach EXE, $(RUNTIME_TARGETS), \
- timeout 1s ./$(EXE) 1234567890 2 3 4 5 6 7 8 9 0 >/dev/null && echo "$(EXE) OK" || echo "$(EXE) FAIL"; \
- )
+ timeout 1s ./$(EXE) 1234567890 2 3 4 5 6 7 8 9 0 >/dev/null && echo "$(EXE) OK" || { r=1; echo "$(EXE) FAIL"; }; \
+ ) \
+ exit $$r
comptime: # only works on clang, as gcc doesn't have the diagnose_if attribute
$(foreach EXE, $(COMPTIME_TARGETS), \ |
jvoisin
force-pushed
the
master
branch
3 times, most recently
from
a6f4dca to
e2adb63
Compare
jvoisin
force-pushed
the
master
branch
3 times, most recently
from
acccd3d to
c938909
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor testsuite so the platform dependant (C compiler) and header loactions are more together.
Don't make assumptions on CC or platform to make it easier to run testsuite natively on Alpine, on any architecture.
fixes #69