[Bug analyzer/95188] analyzer-unsafe-call-within-signal-handler shows wrong statement for signal registration event
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188 David Malcolm changed: What|Removed |Added Blocks||99390 --- Comment #13 from David Malcolm --- At least some of the state explosion appears to be due to malfunctioning call summaries, e.g.: function ‘fileExists’, with call string: [(SN: 422 -> SN: 163 in main), (SN: 599 -> SN: 334 in uncompress)] probably should have been summarized, but wasn't. Adding this to the call summaries tracker bug. Referenced Bugs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99390 [Bug 99390] [meta-bug] tracker bug for call summaries in -fanalyzer
[Bug analyzer/95188] analyzer-unsafe-call-within-signal-handler shows wrong statement for signal registration event
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188 --- Comment #12 from Mark Wielaard --- (In reply to David Malcolm from comment #11) > (In reply to Mark Wielaard from comment #10) > > Created attachment 49293 [details] > > supergraph > > Thanks. Compared to my testing, I'm seeing what appear to be differences in > the inputs to the analyzer at the gimple level, which are likely due to > differences in the rest of the compiler. > > Is this with the same version of gcc as in comment #4, where you said "gcc > (GCC) 11.0.0 20200916 (experimental)". > > You don't happen to know exactly which revision, do you? I am afraid I don't know exactly. I have been experimenting with having DWARF 5 as default in my builds, which is another difference. > [The md5sum of the bzip2.c I'm using is 23f66348f80345353d5b5b98e299ff15. > There could also be differences in the system headers, I suppose] The MD5 matches. But I am indeed using system headers from RHEL7 with DTS9 installed to bootstrap my GCC builds.
[Bug analyzer/95188] analyzer-unsafe-call-within-signal-handler shows wrong statement for signal registration event
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188 --- Comment #11 from David Malcolm --- (In reply to Mark Wielaard from comment #10) > Created attachment 49293 [details] > supergraph Thanks. Compared to my testing, I'm seeing what appear to be differences in the inputs to the analyzer at the gimple level, which are likely due to differences in the rest of the compiler. Is this with the same version of gcc as in comment #4, where you said "gcc (GCC) 11.0.0 20200916 (experimental)". You don't happen to know exactly which revision, do you? [The md5sum of the bzip2.c I'm using is 23f66348f80345353d5b5b98e299ff15. There could also be differences in the system headers, I suppose]
[Bug analyzer/95188] analyzer-unsafe-call-within-signal-handler shows wrong statement for signal registration event
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188 --- Comment #10 from Mark Wielaard --- Created attachment 49293 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49293=edit supergraph > Mark: please can you add -fdump-analyzer-supergraph to the arguments and > attach > the bzip2.c.supergraph.dot file to this bug. Doing so may help > track down (d). gcc -g -O2 -fanalyzer -fdump-analyzer-supergraph -c bzip2.c
[Bug analyzer/95188] analyzer-unsafe-call-within-signal-handler shows wrong statement for signal registration event
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188 --- Comment #9 from David Malcolm --- The above patch fixes (a) from comment #7 above, but (b), (c) and (d) still need fixing, so keeping this open for now.
[Bug analyzer/95188] analyzer-unsafe-call-within-signal-handler shows wrong statement for signal registration event
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188 --- Comment #8 from CVS Commits --- The master branch has been updated by David Malcolm : https://gcc.gnu.org/g:d60d63a00bb50ba6896939705c589578177b404d commit r11-3537-gd60d63a00bb50ba6896939705c589578177b404d Author: David Malcolm Date: Tue Sep 29 15:55:33 2020 -0400 analyzer: fix signal-handler registration location [PR95188] PR analyzer/95188 reports that diagnostics from -Wanalyzer-unsafe-call-within-signal-handler use the wrong source location when reporting the signal-handler registration event in the diagnostic_path. The diagnostics erroneously use the location of the first stmt in the basic block containing the call to "signal", rather than that of the call itself. Fixed thusly. gcc/analyzer/ChangeLog: PR analyzer/95188 * engine.cc (stmt_requires_new_enode_p): Split enodes before "signal" calls. gcc/testsuite/ChangeLog: PR analyzer/95188 * gcc.dg/analyzer/signal-registration-loc.c: New test.
[Bug analyzer/95188] analyzer-unsafe-call-within-signal-handler shows wrong statement for signal registration event
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188 --- Comment #7 from David Malcolm --- Thanks. I see a similar deluge of "terminating analysis for this program point" warnings, but at different locations. My warnings eventually terminate with: bzip2.c:1537:31: warning: analysis bailed out early (4561 'after-snode' enodes; 15071 enodes) [-Wanalyzer-too-complex] whereas yours don't - my machine is eventually hitting the overall complexity limit (as opposed to merely hitting the per-program-point limit). If I add -fparam-analyzer-bb-explosion-factor=50 to try to get past that limit (the default for that param is 5) then I see the -Wanalyzer-unsafe-call-within-signal-handler warnings you're seeing (it takes quite a while to run). As in comment #5, I think what's happening is some "random" aspect of the analysis affecting the order in which the worklist is processed, which leads to my machine terminating early and yours running to completion. So there are at least four issues here: (a) the reported bug: that -Wanalyzer-unsafe-call-within-signal-handler uses the wrong source location when reporting the signal registration event in the diagnostic_path (b) that -fanalyzer is hitting per-program-point limits (c) that -fanalyzer can hit the overall enode limit (d) that the behavior is sufficiently "random" that (c) can happen on one machine and not on another, and that the log for the (b) events shows the order of exploration varying between machines. Mark: please can you add -fdump-analyzer-supergraph to the arguments and attach the bzip2.c.supergraph.dot file to this bug. Doing so may help track down (d).
[Bug analyzer/95188] analyzer-unsafe-call-within-signal-handler shows wrong statement for signal registration event
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188 --- Comment #6 from Mark Wielaard --- Created attachment 49291 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49291=edit Output for gcc -Wanalyzer-too-complex -g -O2 -fanalyzer -c bzip2.c (In reply to David Malcolm from comment #5) > Thanks Mark. What architecture are you on? RHEL7 x86_64. > When I do those steps, there's a long wait and then in terminates with no > analyzer output. > > If I add -Wanalyzer-too-complex I see lots of warnings about "terminating > analysis for this program point". > > What do you see if you add -Wanalyzer-too-complex? Lots and lots of output saying "warning: terminating analysis for this program point". log attached.
[Bug analyzer/95188] analyzer-unsafe-call-within-signal-handler shows wrong statement for signal registration event
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188 --- Comment #5 from David Malcolm --- Thanks Mark. What architecture are you on? When I do those steps, there's a long wait and then in terminates with no analyzer output. If I add -Wanalyzer-too-complex I see lots of warnings about "terminating analysis for this program point". What do you see if you add -Wanalyzer-too-complex? If you do, my guess as to what's happening is that there's a "random" factor in how the worklist is being explored, and that on my machine it's hitting the complexity limits before finding the issue, and on your machine it's finding the issue first. Perhaps it relates to pointer addresses; PR 96608 notes some places where hash values could vary between runs, and that could be enough to throw out the worklist traversal order.
[Bug analyzer/95188] analyzer-unsafe-call-within-signal-handler shows wrong statement for signal registration event
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188
--- Comment #4 from Mark Wielaard ---
Note that I can replicate it with the instructions in the description and gcc
git: gcc (GCC) 11.0.0 20200916 (experimental)
$ /opt/local/install/gcc/bin/gcc -g -O2 -fanalyzer -c bzip2.c 2>&1 | head -25
bzip2.c: In function ‘showFileNames.part.0’:
bzip2.c:677:4: warning: call to ‘fprintf’ from within signal handler [CWE-479]
[-Wanalyzer-unsafe-call-within-signal-handler]
677 |fprintf (
|^
678 | stderr,
| ~~~
679 | "\tInput file = %s, output file = %s\n",
|
680 | inName, outName
| ~~~
681 |);
|~
‘main’: events 1-2
|
| 1776 | IntNative main ( IntNative argc, Char *argv[] )
| | ^~~~
| | |
| | (1) entry to ‘main’
| 1777 | {
| 1778 |Int32 i, j;
| |~
| ||
| |(2) registering ‘mySIGSEGVorSIGBUScatcher’ as signal handler
|
event 3
It doesn't point at smallMode anymore, but the Int32 type isn't the right place
either.
For reference this is the main method starting at line 1776:
IntNative main ( IntNative argc, Char *argv[] )
{
Int32 i, j;
Char *tmp;
Cell *argList;
Cell *aa;
Bool decode;
/*-- Be really really really paranoid :-) --*/
if (sizeof(Int32) != 4 || sizeof(UInt32) != 4 ||
sizeof(Int16) != 2 || sizeof(UInt16) != 2 ||
sizeof(Char) != 1 || sizeof(UChar) != 1)
configError();
/*-- Initialise --*/
outputHandleJustInCase = NULL;
smallMode = False;
keepInputFiles = False;
forceOverwrite = False;
noisy = True;
verbosity = 0;
blockSize100k = 9;
testFailsExist = False;
unzFailsExist = False;
numFileNames= 0;
numFilesProcessed = 0;
workFactor = 30;
deleteOutputOnInterrupt = False;
exitValue = 0;
i = j = 0; /* avoid bogus warning from egcs-1.1.X */
/*-- Set up signal handlers for mem access errors --*/
signal (SIGSEGV, mySIGSEGVorSIGBUScatcher);
[Bug analyzer/95188] analyzer-unsafe-call-within-signal-handler shows wrong statement for signal registration event
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188 --- Comment #3 from David Malcolm --- Also, I can probably cook up a more minimal reproducer - but it would be good to track down the state-explosion issues: scaling up -fanalyzer to deal effectively with real-world C code is a goal for me for GCC 11.
[Bug analyzer/95188] analyzer-unsafe-call-within-signal-handler shows wrong statement for signal registration event
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188 David Malcolm changed: What|Removed |Added Ever confirmed|0 |1 Last reconfirmed||2020-09-16 Status|UNCONFIRMED |ASSIGNED --- Comment #2 from David Malcolm --- (In reply to CVS Commits from comment #1) > The master branch has been updated by David Malcolm : > > https://gcc.gnu.org/g:b28491dc2d79763ecbff4f0b9f1f3e48a443be1d > > commit r11-3245-gb28491dc2d79763ecbff4f0b9f1f3e48a443be1d > Author: David Malcolm > Date: Tue Aug 18 18:52:17 2020 -0400 > > analyzer: bulk merger/processing of runs of nodes at CFG join points [...] > The patch fixes a state explosion seen in bzip2.c seen when attempting > to reproduce PR analyzer/95188, in a switch statement in a loop for > argument parsing. With this patch, the analyzer successfully > consolidates the state after the argument parsing to a single exploded > node. [...] As noted above, I'm currently not able to reproduce this bug. My guess is that there was a pre-existing failure to fully explore the program and we previously were lucky to explore enough to trigger the bug, but at some point (probably the reimplementation of state tracking of r11-2694-g808f4dfeb3a95f50f15e71148e5c1067f90a126d) the bug is now in the unexplored section. The above commit from comment #1 will help, but I'm still not able to reproduce the bug. Marking as ASSIGNED since the state-explosion issue ought to be fixed, and I can at least reproduce that.
[Bug analyzer/95188] analyzer-unsafe-call-within-signal-handler shows wrong statement for signal registration event
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188 --- Comment #1 from CVS Commits --- The master branch has been updated by David Malcolm : https://gcc.gnu.org/g:b28491dc2d79763ecbff4f0b9f1f3e48a443be1d commit r11-3245-gb28491dc2d79763ecbff4f0b9f1f3e48a443be1d Author: David Malcolm Date: Tue Aug 18 18:52:17 2020 -0400 analyzer: bulk merger/processing of runs of nodes at CFG join points Prior to this patch the analyzer worklist considered only one node or two nodes at a time, processing and/or merging state individually or pairwise. This could lead to explosions of merger nodes at CFG join points, especially after switch statements, which could have large numbers of in-edges, and thus large numbers of merger exploded_nodes could be created, exceeding the per-point limit and thus stopping analysis with -Wanalyzer-too-complex. This patch special-cases the handling for runs of consecutive nodes in the worklist at a CFG join point, processing and merging them all together. The patch fixes a state explosion seen in bzip2.c seen when attempting to reproduce PR analyzer/95188, in a switch statement in a loop for argument parsing. With this patch, the analyzer successfully consolidates the state after the argument parsing to a single exploded node. In gcc.dg/analyzer/pr96653.c there is a switch statement with over 300 cases which leads to hitting the per-point limit. With this patch the consolidation code doesn't manage to merge all of them due to other worklist-ordering bugs, and it still hits the per-point limits, but it does manage some very long consolidations: merged 2 in-enodes into 2 out-enode(s) at SN: 403 merged 2 in-enodes into 2 out-enode(s) at SN: 403 merged 2 in-enodes into 1 out-enode(s) at SN: 11 merged 29 in-enodes into 1 out-enode(s) at SN: 35 merged 6 in-enodes into 1 out-enode(s) at SN: 41 merged 31 in-enodes into 1 out-enode(s) at SN: 35 and with a followup patch to fix an SCC issue it manages: merged 358 in-enodes into 2 out-enode(s) at SN: 402 The patch appears to fix the failure on non-x86_64 of: gcc.dg/analyzer/pr93032-mztools.c (test for excess errors) which is PR analyzer/96616. Unfortunately, the patch introduces a memory leak false positive in gcc.dg/analyzer/pr94851-1.c, but this appears to be a pre-existing bug that was hidden by state-merging failures. gcc/analyzer/ChangeLog: * engine.cc (exploded_node::dump_dot): Show STATUS_BULK_MERGED. (exploded_graph::process_worklist): Call maybe_process_run_of_before_supernode_enodes. (exploded_graph::maybe_process_run_of_before_supernode_enodes): New. (exploded_graph_annotator::print_enode): Show STATUS_BULK_MERGED. * exploded-graph.h (enum exploded_node::status): Add STATUS_BULK_MERGED. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/bzip2-arg-parse-1.c: New test. * gcc.dg/analyzer/loop-n-down-to-1-by-1.c: Remove xfail. * gcc.dg/analyzer/pr94851-1.c: Add xfail.
