[Bug c++/69534] [6 Regression] openjade is miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69534
Kamil Dudka changed:
What|Removed |Added
CC||kdudka at redhat dot com
--- Comment #6 from Kamil Dudka ---
(In reply to rguent...@suse.de from comment #5)
> Can you bisect to a file?
The cause seems to be incorrectly compiled code of the following method:
inline
void *Collector::allocateObject(bool hasFinalizer)
{
if (freePtr_ == &allObjectsList_)
makeSpace();
Object *tem = freePtr_;
freePtr_ = freePtr_->next();
tem->setColor(currentColor_);
tem->hasFinalizer_ = hasFinalizer;
if (hasFinalizer)
tem->moveAfter(&allObjectsList_);
return tem;
}
If I move the method from Collector.h to Collector.cxx and make it non-inline,
the problem goes away because the value of hasFinalizer is no longer known at
compile time.
Otherwise, from the modules where Collector::allocateObject() is called with
hasFinalizer equal to zero (I inspected the assembler code generated out of
primitive.cxx with/without -fno-tree-dse), the optimizer correctly drops the
code:
if (hasFinalizer)
tem->moveAfter(&allObjectsList_);
... but at the same time it incorrectly drops also the two assignments above
the condition:
tem->setColor(currentColor_);
tem->hasFinalizer_ = hasFinalizer;
Consequently one can see many uses of uninitialized values in valgrind's
output. If I qualify the 'tem' pointer as volatile in order to suppress the
optimization, the code of openjade starts to work reliably, even if compiled
with default compilation flags, and with completely clean valgrind's output:
--- a/style/Collector.h
+++ b/style/Collector.h
@@ -138,11 +138,11 @@ void Collector::Object::moveAfter(Object *tail)
inline
void *Collector::allocateObject(bool hasFinalizer)
{
if (freePtr_ == &allObjectsList_)
makeSpace();
- Object *tem = freePtr_;
+ Object *volatile tem = freePtr_;
freePtr_ = freePtr_->next();
tem->setColor(currentColor_);
tem->hasFinalizer_ = hasFinalizer;
if (hasFinalizer)
tem->moveAfter(&allObjectsList_);
Unless there is some undefined behavior in the code, this looks like a bug in
the optimizer.
[Bug c++/69534] [6 Regression] openjade is miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69534 --- Comment #10 from Kamil Dudka --- It makes sense to me. Thanks for the explanation!
[Bug analyzer/104308] no location info provided for [-Wanalyzer-use-of-uninitialized-value] warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104308
Kamil Dudka changed:
What|Removed |Added
Resolution|FIXED |---
Status|RESOLVED|ASSIGNED
--- Comment #8 from Kamil Dudka ---
As spotted by Vincent Mihalkovic, the fix seems to be incomplete. If we run
gcc-12.0.1-0.14.fc37.x86_64 on the following test-case, some diagnostic
messages are still printed without any location info:
$ cat test-memcpy.c
#include
int main(void)
{
char a1[5];
char a2[5];
return (memcpy(a1, a2, 5) == a1);
}
$ gcc -fanalyzer -fdiagnostics-path-format=separate-events -c test-memcpy.c
In function ‘main’:
cc1: warning: use of uninitialized value ‘*(unsigned char (*)[5])(&a2[0])’
[CWE-457] [-Wanalyzer-use-of-uninitialized-value]
test-memcpy.c:6:10: note: (1) region created on stack here
6 | char a2[5];
| ^~
cc1: note: (2) use of uninitialized value ‘*(unsigned char (*)[5])(&a2[0])’
here
[Bug analyzer/104308] New: no location info provided for [-Wanalyzer-use-of-uninitialized-value] warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104308
Bug ID: 104308
Summary: no location info provided for
[-Wanalyzer-use-of-uninitialized-value] warnings
Product: gcc
Version: 12.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: analyzer
Assignee: dmalcolm at gcc dot gnu.org
Reporter: kdudka at redhat dot com
Target Milestone: ---
I can see new (technically valid) analyzer warnings with gcc-12 on Fedora
rawhide without any location information being provided. This makes the output
difficult to use for csdiff utilities as well as human reviewers:
$ printf '#include \nint main() { char s[5]; memmove(s, s + 1, 2);
}\n' | gcc -fanalyzer -fdiagnostics-path-format=separate-events -c -xc -
In function ‘main’:
cc1: warning: use of uninitialized value ‘*(short unsigned int *)&s + 1’
[CWE-457] [-Wanalyzer-use-of-uninitialized-value]
cc1: note: (1) use of uninitialized value ‘*(short unsigned int *)&s + 1’ here
The same command produces no output with gcc-11.2.1 though.
[Bug analyzer/104308] no location info provided for [-Wanalyzer-use-of-uninitialized-value] warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104308 Kamil Dudka changed: What|Removed |Added CC||kdudka at redhat dot com --- Comment #3 from Kamil Dudka --- Thank you for having a look! I was curious where (short unsigned int *) came from but it seems to be a result of inlined memmove(). The report goes away when the code is compiled with -fno-builtin-memmove.
[Bug other/116613] RFE: support outputting diagnostics in *multiple* formats
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116613
--- Comment #24 from Kamil Dudka ---
(In reply to David Malcolm from comment #21)
> (In reply to David Malcolm from comment #20)
> > How about another option
> > -fdiagnostics-set-output=
> > that would work like
> > -fdiagnostics-add-output=
> > but rather than appending to the list of outputs, simply replaces anything
> > in the existing list?
I think this would solve the problem for OSH (and eliminate the need to create
unneeded JSON files).
> Now I think about it, that's only going to help for diagnostics that happen
> before the -fdiagnostics-{add,set}-output= param is processed.
I do not understand this part. Could you please provide an example where this
would not work as expected.
> FWIW SARIF has a distinction between a "result" and a "notification" where
> the former refer to the software under test, and the latter to the testing
> (e.g. a "tool configuration notification" could be "I can't read the config
> file to start this run"). Currently GCC doesn't make such a distinction;
> perhaps it should.
I did not know about this but it seems to work only partially. When gcc is
given a non-existing file name, it is recorded in the SARIF file as a result:
% gcc does-not-exist.c -fdiagnostics-add-output="sarif:file=msg.sarif"
cc1: fatal error: does-not-exist.c: No such file or directory
compilation terminated.
% csgrep msg.sarif
Error: COMPILER_WARNING:
: warning[fatal error]: does-not-exist.c: No such file or directory
However, when gcc is given an unsupported flag, the SARIF file is not created
at all:
% gcc xxx.c -fno-caret-diagnostics
-fdiagnostics-add-output="sarif:file=msg.sarif"
gcc: error: unrecognized command-line option ‘-fno-caret-diagnostics’
% ls msg.sarif
ls: cannot access 'msg.sarif': No such file or directory
[Bug other/116613] RFE: support outputting diagnostics in *multiple* formats
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116613 --- Comment #14 from Kamil Dudka --- I think that the above described interface looks reasonable. A few questions: 1. Will `file=` work with absolute paths? 2. If a file with the specified name exists, will it be overwritten? 3. Will the file always be created, even if no diagnostics is produced by gcc? 4. Will the SARIF data contain absolute paths to source code files? If not will the working directory be recorded in each SARIF file?
[Bug other/116613] RFE: support outputting diagnostics in *multiple* formats
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116613 --- Comment #19 from Kamil Dudka --- Thank you for working on this! I really like the gcc-latest COPR, which makes it easy to test the changes. I spotted that man pages installed by gcc-latest are broken but that is a cosmetic issue. I have hacked up a quick and dirty patch to make csmock-plugin-gcc use this, tested it on the units and curl RPM packages and it seemed to work fine: https://github.com/csutils/csmock/pull/187 I needed to suppress the diagnostics output printed by gcc to stderr so that csmock does not capture the same information twice using two different formats. I was able to get around with `-fdiagnostics-format=json-file` but it wastefully creates files that are not used for anything. Is there a better way to tell gcc to write all diagnostic messages only to the specified SARIF file while keeping messages about unsupported flags, non-existing input files and the like still printed to stderr?
[Bug other/116613] RFE: support outputting diagnostics in *multiple* formats
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116613 --- Comment #32 from Kamil Dudka --- Thanks for the update! I confirm that the man page is readable now and that the use of -fdiagnostics-set-output= eliminated the duplicated output without the ugly workaround. I have also tried sarif:version=2.2-prerelease and in the basic testing scenario everything worked as expected.
[Bug other/116613] RFE: support outputting diagnostics in *multiple* formats
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116613 --- Comment #34 from Kamil Dudka --- (In reply to David Malcolm from comment #33) > Note that the "2.2-prerelease" stuff does almost nothing at this stage and > is experimental; you should probably leave "version" unset so that it uses > the default (2.1, which is what every other SARIF-based project I've looked > at seems to be using). Will do, thanks!
[Bug other/116613] RFE: support outputting diagnostics in *multiple* formats
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116613
--- Comment #7 from Kamil Dudka ---
(In reply to Eric Gallager from comment #6)
> Yeah, in particular, I'd want to send text to stdout while also sending
> SARIF to a file that shares the input file's name, except with the .sarif
> extension. (i.e., compiling foo.c would create foo.sarif)
This does not work well for source code files that are generated during the
build. They can be created in temporary directories that are later removed
during the build. A source code file with the same path (such as conftest.c)
can be compiled repeatedly with different contents inside, etc.
I have been developing fully transparent capture and processing of GCC
diagnostic output since 2013: https://github.com/csutils/
Now in 2024 we are still using the plain-text diagnostic output of GCC because
the currently provided interface for configuring GCC diagnostic output does not
make it easy for us to use a JSON-based format. This was previously discussed
here: https://github.com/packit/dashboard/issues/441#issuecomment-2307711956
For fully automated capture of GCC diagnostic output that works regardless of
the build system, it would be much more useful if GCC was able to write a
separate SARIF file with a unique file name for each compilation unit to a
specified directory, something like `valgrind --xml-file=${dir}/%p-%n.xml ...`
does.
[Bug other/116613] RFE: support outputting diagnostics in *multiple* formats
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116613
--- Comment #16 from Kamil Dudka ---
(In reply to David Malcolm from comment #15)
> (In reply to Kamil Dudka from comment #14)
> > 1. Will `file=` work with absolute paths?
>
> Yes. There might be some issues with expressing paths/filenames containing
> whitespace, '=', or ',' due to the way the option-parsing works, but
> hopefully that's acceptable.
Yes, OSH does not need such chars in names of files with diagnostic output.
> > 2. If a file with the specified name exists, will it be overwritten?
>
> Short answer: yes
>
> Longer answer: My plan is to do:
> fopen (path, "w")
> on any specified outputs as cc1 starts up, and to fail with a hard error if
> any of the fopen fail, telling you why. Hence it could fail if a file with
> the specified name exists, but e.g. you don't have write permissions on it.
Sounds good.
> Perhaps we need a specific exit code for the case of "can't open diagnostic
> output stream"?
I do not think we need it but it may ease debugging in some corner cases.
> > 3. Will the file always be created, even if no diagnostics is produced by
> > gcc?
>
> Yes, with the caveat that if cc1 can't fopen a diagnostic output file it
> will fail immediately (as above), and obviously the file won't be created in
> such a case.
Although the SARIF file may contain useful info even for runs with no warnings,
OSH only collects the warnings. SARIF files that contain no warnings may
complicate the processing of results (e.g. by exceeding command line length
while expanding globs in our shell scripts). On the other hand, it is not that
difficult to remove the SARIF files with no warnings after the build has
finished. We already take a similar approach with ShellCheck:
https://github.com/csutils/csmock/blob/455de9cdadf5d828c3b380c87e3f6f24dddad6ba/scripts/run-shellcheck.sh#L95
> > 4. Will the SARIF data contain absolute paths to source code files? If not
> > will the working directory be recorded in each SARIF file?
>
> GCC's current behavior is (as of GCC 13):
>
> * for absolute paths, the GCC SARIF output for the "artifact" will have an
> absolute value in its "uri".
>
> "artifacts": [{"location": {"uri": "/tmp/test.c"},
>
> * for relative paths, the GCC SARIF output for the "artifact" will have a
> relative uri, and a "uriBaseId" of "PWD":
>
> "artifacts": [{"location": {"uri":
> "../../src/gcc/testsuite/gcc.dg/analyzer/malloc-1.c",
> "uriBaseId": "PWD"},
>
> and the "run" will have this giving the absolute path for "PWD":
>"originalUriBaseIds": {"PWD": {"uri":
> "file:///home/david/coding-3/gcc-newgit-path-revamp/build/gcc/"}},
>
> As of GCC 15 the "run"'s "invocation" also has the "workingDirectory"
> property:
>
> "workingDirectory": {"uri":
> "/home/david/coding-3/gcc-newgit-path-revamp/build/gcc"},
Thanks for explanation! So at least the full path can be reconstructed later
on. I have created a csdiff issue for this:
https://github.com/csutils/csdiff/issues/209
> Re my idea of:
>
> > -fdiagnostics-add-output=sarif:file={output-filename}.sarif
> >
> > where e.g. {output-filename} would be substituted with the output filename
> > from the gcc invocation.
>
> note that I don't have that working yet, or a precise set of "substitution"
> names and their semantics (I plan to work on it today). What
> "substitutions" might you need for your use-cases?
>
> Does the above support all your use-cases?
OSH does not care too much about the names of SARIF files because all the
important data are contained in the files inside. The ideal SARIF-based
workflow for OSH would be:
1. create an empty directory for scan results
2. run the (instrumented) build a of C/C++ project
3. each invocation of gcc (that produces warnings) during the build creates a
unique file with SARIF data in the pre-created directory with scan results
4. all the files created in the pre-created directory with scan results can be
processed after the build
Step 3. can be partially implemented in the compiler wrapper with flock/mktemp,
which can invoke gcc with an absolute path of an already created empty file to
write the SARIF data to. If we take this approach, OSH will not need any such
substitutions in gcc. If a substitution was provided by gcc to construct a
unique file name (such as %p and %n in valgrind), OSH would not need to
implement this part in the compiler wrapper.
[Bug other/116613] RFE: support outputting diagnostics in *multiple* formats
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116613 --- Comment #27 from Kamil Dudka --- Sounds good, looking forward to try out a new COPR build!
[Bug analyzer/117667] -flto=auto prevents -fanalyzer from reporting any warnings on a build of systemd
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117667 Kamil Dudka changed: What|Removed |Added CC||kdudka at redhat dot com See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=94976, ||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=102233, ||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=103602 --- Comment #4 from Kamil Dudka --- You are right, one needs to use -Dc_link_args=... to pass the same flags to the linker. Unfortunately, then the linker ran for minutes and ate all the memory on a 32 GiB machine, which I am afraid is bug #102233 and bug #103602. So it is way better to use csgcca, which runs the gcc analyzer in a separate process for each compilation unit with LTO disabled: https://github.com/csutils/cscppc/commit/d8d49b4cc92e0109b2654e9034c85e169bdc0566
[Bug sarif-replay/96032] RFE: add a way to use output from --fdiagnostics-format=json or sarif as input
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96032 --- Comment #8 from Kamil Dudka --- Where can one get the sarif-replay tool? It does not seem to be included in gcc-latest.
[Bug analyzer/117667] New: -flto=auto prevents -fanalyzer from reporting any warnings on a build of systemd
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117667 Bug ID: 117667 Summary: -flto=auto prevents -fanalyzer from reporting any warnings on a build of systemd Product: gcc Version: 15.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: analyzer Assignee: dmalcolm at gcc dot gnu.org Reporter: kdudka at redhat dot com Target Milestone: --- As originally reported by František Šumšal at https://lists.fedoraproject.org/archives/list/de...@lists.fedoraproject.org/message/2DXE6BMG3A3J72PDWUGZSEIKNHILUKR3/ -flto=auto prevents -fanalyzer from reporting any warnings on a build of systemd. Steps to reproduce: 1. compile/analyze without -flto=auto % git clone https://github.com/systemd/systemd.git % git reset --hard https://github.com/systemd/systemd.git % CC=/opt/gcc-latest/bin/gcc meson build -Dc_args="-fanalyzer -fdiagnostics-format=sarif-file" % ninja -C build systemd % find -name \*.sarif | xargs csgrep --mode=evtstat 1 COMPILER_WARNING warning[-Wunterminated-string-initialization] 1 GCC_ANALYZER_WARNING warning[-Wanalyzer-allocation-size] 34 GCC_ANALYZER_WARNING warning[-Wanalyzer-double-free] 12 GCC_ANALYZER_WARNING warning[-Wanalyzer-fd-leak] 1 GCC_ANALYZER_WARNING warning[-Wanalyzer-fd-phase-mismatch] 1 GCC_ANALYZER_WARNING warning[-Wanalyzer-file-leak] 1 GCC_ANALYZER_WARNING warning[-Wanalyzer-infinite-loop] 100 GCC_ANALYZER_WARNING warning[-Wanalyzer-malloc-leak] 1 GCC_ANALYZER_WARNING warning[-Wanalyzer-mismatching-deallocation] 7 GCC_ANALYZER_WARNING warning[-Wanalyzer-null-argument] 30 GCC_ANALYZER_WARNING warning[-Wanalyzer-null-dereference] 21 GCC_ANALYZER_WARNING warning[-Wanalyzer-out-of-bounds] 3 GCC_ANALYZER_WARNING warning[-Wanalyzer-use-after-free] 57 GCC_ANALYZER_WARNING warning[-Wanalyzer-use-of-uninitialized-value] 2 GCC_ANALYZER_WARNING warning[-Wanalyzer-va-list-exhausted] 2. compile/analyze with -flto=auto % git clean -dfx % CC=/opt/gcc-latest/bin/gcc meson build -Dc_args="-fanalyzer -fdiagnostics-format=sarif-file -flto=auto" % ninja -C build systemd % find -name \*.sarif | xargs csgrep --mode=evtstat 1 COMPILER_WARNING warning[-Wunterminated-string-initialization] That is, with -flto=auto, all the warnings from gcc analyzer silently disappear (and the compilation runs apparently faster). I am able to reproduce this with gcc-14.2.1-3.fc40.x86_64 as well as gcc-latest-15.0.0-4.20241020git01f50ebfd97a.pr116613.v0.155.fc40.x86_64 installed from https://copr.fedorainfracloud.org/coprs/dmalcolm/gcc-latest/
[Bug sarif-replay/96032] RFE: add a way to use output from --fdiagnostics-format=json or sarif as input
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96032 --- Comment #12 from Kamil Dudka --- I confirm that sarif-replay is available on f42+ and it seems to work as expected. Thanks!
