-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
reports: Fix a possible infinite loop in SourceManager #748
Conversation
fgetc returns an int, however char was specified. This fixes a case where the EOF is truncated to a char which may not be detected depending on the sign of a char. This issue is known to trigger on ppc64le.
@AlexDenisov regarding ppc64le: I have a couple of other changes, but I don't think they are issues only related to ppc64le. I also have a set of changes to get things working on LLVM 10.0.1, although I expect those break compat with older version of LLVM. Finally, on Fedora 33, I ran into a number of issues with functions not returning properly due to some return paths not being covered This is a quick change to get it to build, however I'm sure you might want something else.: Do note that I am still getting a crash when I try to use the elements reporter. I have not yet looked into this. (I'll probably test with llvm 9 first) |
@meklort Re: Crash in elements reporter |
This patch would be greatly appreciated! I think having it in
There is a compatibility layer for different versions of LLVM. There is a branch indeed, but I didn't finish the work back then and was planning to get it done soon. I'm happy to take care of it unless you want to bring it on yourself :)
I guess this is because of gcc? We don't explicitly support gcc assuming that if anyone wants to use an LLVM based tool, then they should have access to clang :)
That is certainly a bug. I'd suggest you give a try to @gebart's suggestion, but we should certainly do better and issue a warning and not crash in this case. |
Sure thing, I'll try send something your way this weekend.
Everything is working on my branch, so I'll also try to send it your way later.
Will do. It looks like /usr/bin/c++ defaults to GCC 10 on Fedora, and so cmake will use this by default. I was running the following; I'll do a pull request in a few minutes for it.
That certainly changes the behvaiour (no mutants remain - all are filtered out) (no more assertion), however all mutants are skipped. I'll take a look at it more when I get some free time. (Note that the IDE reporter works fine). Also note that I'm getting some intermittent behaviour where run-to-run I get different results (anywhere from 87% up to 100% killed). I personally would expect that to be consistent (test isn't randomized). So, I'll also need to look into that more (try LLVM9, x86, etc). |
Makes sense, I'll add it as a recommendation.
Hm-hm, how many mutants do you have before filtering?
Some little discrepancy is possible, but 87-100 looks like something is off. |
Assuming the number of mutants if big enough for this difference to be significant. |
Good point, this was a single test case w/ 33 mutants total, so 29/33 mutants on the low end, 33/33 on the high end. This is a small enough sample right now that it's probably not a problem. That said... the variability is a bit concerning, although I do understand the explanation. Note that I have implemented mutation testing in an instruction set simulator (assembly/opcode level mutations) and in that system no MMU exists, so crashing wasn't a valid kill case. In this case, I'm rebuilding firmware from an embedded system (with no MMU). For the code base I'm working on now, I'm planning on adding a gate based on the mutation score so that it never decreases before merge, however that will get a bit tricky with the variability. |
I'm filtering out mutants in the test code and system libraries with --exclude-path, and with that I get down to 33 from ~500. Note that as soon as I enable compile_commands.json (even without --exclude-path), none are tested. Note that it looks like compile_commands.json may be causing problems in my case as files are build multiple times to different targets (ARM, MIPS, and host) depending on where they are linking (different firmware binaries or host simulator). I did try manually removing the arm variant from the file, but that didn't seem to help. It may also be that I'm passing the files to clang and not clang++. { {
|
Is it a private or public project? I would be happy to take a closer look at the results if possible, it could still be that there is a bug somewhere.
But does the rest (33) looks like the correct mutants? The primary goal of this filtering is to remove the mutants that exist on the Bitcode level, but do not have a correspomding representation on the source code level. E.g., some branches and method calls generate from exceptions or macros/compiler builtins.
We certainly were not prepared for that. In this case, Mull will pick one of the commands for a file, but which one is undefined |
Public: https://github.com/meklort/bcm5719-fw/tree/mutation-testing
Yes, the rest look reasonable. The only ones that are potentially questionable are mutations on a switch statement, however they still look useful there. |
fgetc returns an int, however char was specified.
This fixes a case where the EOF is truncated to a char
which may not be detected depending on the sign of a char.
This issue is known to trigger on ppc64le.