[Bug other/116978] ICE with -fdiagnostics-format=sarif-file when main_input_filename is null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116978 --- Comment #4 from David Malcolm --- Note that if you're invoking cc1 or another compiler directly, rather than via the "gcc" or "xgcc" driver, add -dumpbase foo or similar before -fdiagnostics-format=sarif-file, so that it will write to foo.sarif: $ ./cc1 -dumpbase foo -fdiagnostics-format=sarif-file /dev/null -dumpbase $ ll foo.sarif -rw-rw-r--. 1 david david 2110 Oct 4 18:43 foo.sarif otherwise you'll get: $ ./cc1 -fdiagnostics-format=sarif-file /dev/null cc1: error: unable to determine filename for SARIF output
[Bug other/116978] ICE with -fdiagnostics-format=sarif-file when main_input_filename is null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116978 David Malcolm changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #3 from David Malcolm --- The ICE should be fixed by the above commit.
[Bug analyzer/116977] Analyzer: track OpenACC "host" vs. "device" pointers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116977 --- Comment #3 from David Malcolm --- (In reply to Thomas Schwinge from comment #1) > David, re (a) and (c), do we teach directly the Analyzer about specific > functions (like 'acc_malloc', 'acc_free' if '-fopenacc' is enabled), or do > we add GCC function attributes to designate return value and formal > parameters as "device" instead of "host" pointer, or something else? I've done both. Using the specific hardcoded functions approach (see known_function logic, and the various hacks in e.g. sm-malloc.cc), is easier to get started with, and allows for much more complicated semantics, such as "realloc", which has its own complicated special-case behaviors and is thus a pain to deal with. Using function attributes is much more flexible in that the end-user can use them, but perhaps isn't as expressive.
[Bug analyzer/116977] Analyzer: track OpenACC "host" vs. "device" pointers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116977 --- Comment #2 from David Malcolm --- Thanks for looking at this, and for filing the bug. There are two ways of tracking state in the analyzer: (a) using state machines, like in sm-malloc.cc, which let program_state instances track per-svalue states using sm_state_map. (b) by using the region_model, which tracks information on the state of memory. Each program_state has one of these. and sometimes the way these two systems interact is, alas, rather ugly/hackish. The region-model code has a concept of a region, and of memory spaces. See enum memory_space in gcc/analyzer/region.h. Currently there's a concept of a heap_allocated_region, which will be in memory space MEMSPACE_HEAP, and be a child region of the heap_region. It strikes me that "host" vs "device" pointers could be tracked as a new kind of memory space, perhaps with a second heap_region instance representing device memory. That way acc_malloc could return a pointer to a heap_allocated_region in that memory space, and "free" could complain if the underlying buffer of its argument isn't in MEMSPACE_HEAP but is instead in MEMSPACE_DEVICE_HEAP, or somesuch. Note that the analyzer assumes that pointers in different memory spaces can't alias. So that might be worth modeling, if I'm understanding OpenACC's pointer model correctly. That said, looking at the existing code, free_of_non_heap is implemented in sm-malloc.cc as part of the state machine logic, called from malloc_state_machine::on_deallocator_call. So maybe that's the simplest place to implement this?
[Bug other/116970] -ftime-report -fdiagnostics-format=sarif-file causes ICE
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116970 David Malcolm changed: What|Removed |Added Resolution|--- |DUPLICATE Status|UNCONFIRMED |RESOLVED Keywords||SARIF --- Comment #1 from David Malcolm --- Sorry, I didn't see this before filing bug 116978; marking this as a dup. *** This bug has been marked as a duplicate of bug 116978 ***
[Bug other/116978] ICE with -fdiagnostics-format=sarif-file when main_input_filename is null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116978 David Malcolm changed: What|Removed |Added CC||andi-gcc at firstfloor dot org --- Comment #1 from David Malcolm --- *** Bug 116970 has been marked as a duplicate of this bug. ***
[Bug other/116978] New: ICE with -fdiagnostics-format=sarif-file when main_input_filename is null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116978 Bug ID: 116978 Summary: ICE with -fdiagnostics-format=sarif-file when main_input_filename is null Product: gcc Version: unknown Status: UNCONFIRMED Keywords: SARIF Severity: normal Priority: P3 Component: other Assignee: unassigned at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org Target Milestone: --- Running cc1/cc1plus/etc directly like this ICEs: $ ./cc1 -fdiagnostics-format=sarif-file cc1: internal compiler error: Segmentation fault 0x2bb73ef internal_error(char const*, ...) ../../src/gcc/diagnostic-global-context.cc:517 0x15e35a1 crash_signal ../../src/gcc/toplev.cc:321 0x2c9d0a4 htab_hash_string ../../src/libiberty/hashtab.c:838 0xce6b71 string_hash::hash(char const*) ../../src/gcc/hash-traits.h:239 0x2bac2da simple_hashmap_traits, sarif_artifact*>::hash(char const* const&) ../../src/gcc/hash-map-traits.h:50 0x2ba7580 hash_map, sarif_artifact*> >::get(char const* const&) ../../src/gcc/hash-map.h:191 0x2ba1dc4 ordered_hash_map, sarif_artifact*> >::get(char const* const&) ../../src/gcc/ordered-hash-map.h:76 0x2b98ec3 sarif_builder::get_or_create_artifact(char const*, diagnostic_artifact_role, bool) ../../src/gcc/diagnostic-format-sarif.cc:2939 0x2b9506b sarif_builder::sarif_builder(diagnostic_context&, line_maps const*, char const*, bool, sarif_version) ../../src/gcc/diagnostic-format-sarif.cc:1515 0x2b9db12 sarif_output_format::sarif_output_format(diagnostic_context&, line_maps const*, char const*, bool, sarif_version) ../../src/gcc/diagnostic-format-sarif.cc:3202 0x2b9dc7b sarif_file_output_format::sarif_file_output_format(diagnostic_context&, line_maps const*, char const*, bool, sarif_version, char const*) ../../src/gcc/diagnostic-format-sarif.cc:3245 0x2b99e61 diagnostic_output_format_init_sarif_file(diagnostic_context&, line_maps const*, char const*, bool, sarif_version, char const*) ../../src/gcc/diagnostic-format-sarif.cc:3447 0x2b8bca0 diagnostic_output_format_init(diagnostic_context&, char const*, char const*, diagnostics_output_format, bool) ../../src/gcc/diagnostic.cc:1635 0x2b65d8a common_handle_option(gcc_options*, gcc_options*, cl_decoded_option const*, unsigned int, int, unsigned int, cl_option_handlers const*, diagnostic_context*, void (*)()) ../../src/gcc/opts.cc:2968 0x2b6ce10 handle_option ../../src/gcc/opts-common.cc:1316 0x2b6db90 read_cmdline_option(gcc_options*, gcc_options*, cl_decoded_option*, unsigned int, unsigned int, cl_option_handlers const*, diagnostic_context*) ../../src/gcc/opts-common.cc:1646 0x1495e0c read_cmdline_options ../../src/gcc/opts-global.cc:242 0x149604f decode_options(gcc_options*, gcc_options*, cl_decoded_option*, unsigned int, unsigned int, diagnostic_context*, void (*)()) ../../src/gcc/opts-global.cc:329 Please submit a full bug report, with preprocessed source (by using -freport-bug). Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions. #6 0x02b9506c in sarif_builder::sarif_builder (this=0x3d3d770, context=..., line_maps=0x77ffb000, main_input_filename_=0x0, formatted=true, version=sarif_version::v2_1_0) at ../../src/gcc/diagnostic-format-sarif.cc:1515 1515 false); (gdb) list 1510 since otherwise the "no diagnostics" case would quote the main input 1511 file, and doing so noticeably bloated the output seen in analyzer 1512 integration testing (build directory went from 20G -> 21G). */ 1513 get_or_create_artifact (main_input_filename_, 1514 diagnostic_artifact_role::analysis_target, 1515 false); 1516} (gdb) p main_input_filename_ $3 = 0x0
[Bug other/116301] RFE: provide support for future SARIF versions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116301 --- Comment #3 from David Malcolm --- Also, perhaps sarif-output.exp could have a loop to run all tests per-sarif-version, rather than just for 2.1.0
[Bug other/116301] RFE: provide support for future SARIF versions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116301 David Malcolm changed: What|Removed |Added Last reconfirmed||2024-10-03 Ever confirmed|0 |1 Status|UNCONFIRMED |ASSIGNED --- Comment #2 from David Malcolm --- The above patch adds: * internal support for SARIF 2.2 output * testsuite and selftest coverage, and * support for capturing #include information for ICEs that occur inside a header It doesn't add: * a documented/supported way for users to select SARIF 2.2 * support for the "scannedFile" role (see https://github.com/oasis-tcs/sarif-spec/issues/459), leaving it to followup work. Leaving open for those items.
[Bug other/116613] RFE: support outputting diagnostics in *multiple* formats
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116613
--- Comment #15 from David Malcolm ---
(In reply to Kamil Dudka from comment #14)
> I think that the above described interface looks reasonable. A few
> questions:
Thanks for the feedback.
> 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.
> 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.
Perhaps we need a specific exit code for the case of "can't open diagnostic
output stream"?
> 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.
> 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"},
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?
[Bug other/116613] RFE: support outputting diagnostics in *multiple* formats
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116613
--- Comment #13 from David Malcolm ---
I'm experimenting with a new
-fdiagnostics-add-output=
with this rough grammar:
:
":" key_value_pairs
: "text"
: "sarif"
(and others could be added by GCC plugins)
:
: ","
: "="
: string without a '='
: string without a ','
i.e. an output format name followed by zero or more comma-separated key=value
pairs.
Some examples:
-fdiagnostics-add-output=text:color=yes
-fdiagnostics-add-output=sarif:file={output-filename}.sarif
where e.g. {output-filename} would be substituted with the output filename from
the gcc invocation.
More complicated examples:
Specifiying the SARIF version (see bug 116301):
-fdiagnostics-add-output=sarif:version=2.1,file={output-filename}.2.1.sarif
-fdiagnostics-add-output=sarif:version=2.2-prerelease,file={output-filename}.2.2.sarif
Supporting experiments with hierarchical diagnostics (see bug 116253):
-fdiagnostics-add-output=text:experimental-hierarchical-diagnostics=yes
and so forth.
Thoughts?
[Bug c++/116866] RFE: warn about suspected missing "throw" when discarding temporaries that inherit from std::exception
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116866
--- Comment #5 from David Malcolm ---
(In reply to Jonathan Wakely from comment #3)
> If we add [[nodiscard]] to the constructors of the standard exception types
> then we get a warning with no compiler changes needed:
Is it possible to do this to the standard library? Even if it is, maybe a
warning for discarded temporary of a type that inherits from std::exception is
still useful.
A possible wording:
warning: discarding temporary exception object; did you mean to `throw' it?
[-Wmissing-throw]
std::invalid_argument ("division by zero");
^~
note: `std::invalid_argument' is derived from `std::exception'
Probably shouldn't issue this warning for e.g.: named local that you forgot to
throw:
#include
int divide(int num, int denom) {
std::invalid_argument exc ("division by zero");
return num / denom;
}
since the unused-variable warning should detect it.
[Bug c++/116866] RFE: warn about suspected missing "throw" when discarding temporaries that inherit from std::exception
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116866 --- Comment #4 from David Malcolm --- (In reply to Jonathan Wakely from comment #3) > If we add [[nodiscard]] to the constructors of the standard exception types > then we get a warning with no compiler changes needed: ...in Compiler Explorer as: https://godbolt.org/z/Po4oMPcox
[Bug c++/116866] RFE: warn about suspected missing "throw" when discarding temporaries that inherit from std::exception
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116866
--- Comment #2 from David Malcolm ---
A simple example:
#include
int divide(int num, int denom) {
if (denom == 0)
std::invalid_argument ("division by zero"); // forgot "throw"
return num / denom;
}
which compiles with -Wall with gcc 14.2 without warnings:
https://godbolt.org/z/c6r5PM97W
constructing and desctructing the exception object, and then falling through to
the division.
[Bug c++/116866] New: RFE: warn about suspected missing "throw" when discarding temporaries that inherit from std::exception
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116866
Bug ID: 116866
Summary: RFE: warn about suspected missing "throw" when
discarding temporaries that inherit from
std::exception
Product: gcc
Version: unknown
Status: UNCONFIRMED
Keywords: diagnostic
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: dmalcolm at gcc dot gnu.org
Target Milestone: ---
See
https://gitlab.com/antto/pimpmykicadbom/-/commit/c338027d737d02504fe92a6df00add2f59f462dd
which has:
>From c338027d737d02504fe92a6df00add2f59f462dd Mon Sep 17 00:00:00 2001
From: cad
Date: Fri, 27 Sep 2024 18:16:33 +0300
Subject: [PATCH] throwing an exception might work better with a throw
---
pimpmykicadbom.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pimpmykicadbom.cpp b/pimpmykicadbom.cpp
index 04bed95..a904086 100644
--- a/pimpmykicadbom.cpp
+++ b/pimpmykicadbom.cpp
@@ -2593,7 +2593,7 @@ tbl_group_t BOM::prep(GROUP &g)
// generate a hash/color
if (pcfg == nullptr)
{
-std::logic_error("pcfg is nullptr");
+throw std::logic_error("pcfg is nullptr");
}
const CFG &cfg = *pcfg;
const vector &v_cci = cfg.v_clrinfl;
@@ -2975,7 +2975,7 @@ vector gen_co(BOM &bom)
vector vec;
if (bom.pcfg == nullptr)
{
-std::logic_error("pcfg is nullptr");
+throw std::logic_error("pcfg is nullptr");
}
const CFG &cfg = *bom.pcfg;
const vector &v_co = cfg.v_colorder;
It would be nice if we warned about such cases (if we don't already), perhaps
something like:
warning: possible missing `throw' [-Wsomething-or-other]
std::logic_error("pcfg is nullptr");
throw
>From discussion on IRC:
feels like a potential new warning for g++ if we don't have one
already
I don't think they do, because it has a non-trivial dtor. We could
add [[nodiscard]] to the class declaration, but I think that doesn't actually
work with gcc right now (it doesn't warn when it should)
jwakely: is it an instance of a subclass derived from an "exception"
base?
or whatnot
yes
the non-trivial dtor makes the compiler think that creating a
temporary that immediately goes out of scope might be done for its side effects
(my knowledge of C++ exceptions is hazy, sorry)
but we could either add [[nodiscard]] to it, or add a heuristic in
the compiler that warns about any temporary of a type derived from
std::exception
as a heuristic: if the C++ FE sees a exception_subclass (args);
by itself, warn did you mean "throw " ?
since such types do not have useful side effects, and it's probably a
typo like this case
yup
for belt and braces, you could check if the statement that creates
that temporary exception object is the last statement of a block
because if there are more statements in that block, maybe the author
expected them to be reached. If they were intending to throw, they wouldn't
have written unreachable statements after the throw
probably overkill though
in case I'm not being clear: if (some_condition) {
std::range_error("blah"); func("this would be unreachable if the previous
statement was a throw"); }
but I can't think of a time when anybody would ever do that, so I
think your original heuristic is good enough. A discarded temporary of a type
that inherits from std::exception is probably a mistake
[Bug libstdc++/116847] [15 regression] r15-3859-g63a598deb0c9fc causes many excess errors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116847 --- Comment #15 from David Malcolm --- Ouch, sorry about this. The -3 patch looks reasonable to me.
[Bug other/116792] New: RFE: should we be able to generate diagnostics in HTML format?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116792 Bug ID: 116792 Summary: RFE: should we be able to generate diagnostics in HTML format? Product: gcc Version: unknown Status: UNCONFIRMED Keywords: diagnostic Severity: normal Priority: P3 Component: other Assignee: unassigned at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org Target Milestone: --- Currently we can emit diagnostics as: * text to stderr * SARIF to stderr or to a file * "json" to stderr or to a file I wonder if we should provide a way to emit diagnostics in HTML form? I'm not sure it's useful for the command-line tools, but might be helpful for libdiagnostics and sarif-replay. I have an experimental patch which adds an HTML output format, but for now I'm just going to add it as a plugin in the testsuite, rather than a user-facing feature. In particular, I'd prefer it to be in-tree, given the various refactorings I'm making to the diagnostics subsystem. Filing this so that we have a place to keep track of this idea.
[Bug c++/116771] New: Missing suggestion on mispelled class name
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116771
Bug ID: 116771
Summary: Missing suggestion on mispelled class name
Product: gcc
Version: unknown
Status: UNCONFIRMED
Keywords: easyhack
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: dmalcolm at gcc dot gnu.org
Target Milestone: ---
I made this typo in C++:
class layout_printer
{
void print_newline ();
};
void
layout_pirnter::print_newline ()
{
}
and noticed that we don't yet issue a suggestion for the missppelled class
name:
:7:1: error: 'layout_pirnter' has not been declared
7 | layout_pirnter::print_newline ()
| ^~
Compiler returned: 1
whereas clang does:
:7:1: error: use of undeclared identifier 'layout_pirnter'; did you
mean 'layout_printer'?
7 | layout_pirnter::print_newline ()
| ^~
| layout_printer
:1:7: note: 'layout_printer' declared here
1 | class layout_printer
| ^
:7:1: error: use of undeclared identifier 'layout_pirnter'; did you
mean 'layout_printer'?
7 | layout_pirnter::print_newline ()
| ^~
| layout_printer
:1:7: note: 'layout_printer' declared here
1 | class layout_printer
| ^
2 errors generated.
Compiler returned: 1
See https://godbolt.org/z/q968ePqq9
I'm not sure why clang emits two errors, but that's for them to fix.
In the meantime, we should provide a suggestion for the misspelling.
[Bug other/85716] No easy way for end-user to tell what GCC is doing when compilation is slow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85716 David Malcolm changed: What|Removed |Added Ever confirmed|1 |0 Status|ASSIGNED|UNCONFIRMED
[Bug other/85716] No easy way for end-user to tell what GCC is doing when compilation is slow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85716 --- Comment #15 from David Malcolm --- Brainstorming some ideas here: LSP has an interface for reporting progress notifications; see: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workDoneProgress The various programs launched by the driver could report back to it with progress notifications (maybe via a unix domain socket specified via an environment var?) Presentation: the driver could see if it's connected to a TTY and if things are taking above a threshold start displaying/refreshing a line showing the progress information, perhaps e.g.: [== ] 50% : cc1: expand: some_function_name showing estimated overall % complete; what current tool is, for cc1 what current pass and funtion is; or somesuch. Maybe show: - wallclock time elapsed - number of "items" done vs overall number of "items" to be done (hence able to show %), where what an "item" is would vary Or the driver could re-report the notifications back to another LSP consumer (e.g. an IDE, or to "make") Could have some kind of rate-limiting on the IPC so that we're sending IPC across the protocol at a time-based frequency, rather than doing it for every (pass,function) pair, since that would presumably introduce lots of overhead. Could have gcc driver, cc1 etc, gas, ld, LTO all co-operate in this so that e.g. the user gets meaningful progress information on a big LTO compile. Maybe event teach GNU "make" about this, so that the user can receive "integrated" progress reports from a parallel make. (If so, probably want the protocol to support parallel work) (this is just a brainstorm; no guarantees about implementing any of this)
[Bug other/116724] RFE: can generated SARIF diagnostics contain untranslated messages (plus translations?)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116724 --- Comment #7 from David Malcolm --- (In reply to Hans-Peter Nilsson from comment #4) > (In reply to David Malcolm from comment #1) > > > Perhaps we should try to capture both the untranslated text and the > > translated text? SARIF has various abilities for handling translations. > > Works for me! The use-case I was thinking of, is for the SARIF output to be > a nice containment of the non-source-code part of bug-reports FWIW that SARIF can capture embedded copies of source files of interest, and that the GCC SARIF generation code does this for any source file mentioned in a diagnostic. So perhaps the .sarif file could contain everything.
[Bug other/116724] RFE: can generated SARIF diagnostics contain untranslated messages (plus translations?)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116724 --- Comment #6 from David Malcolm --- (In reply to David Malcolm from comment #5) [...] > (the SARIF spec's tutorial doesn't seem to cover translations yet) FWIW I've requested this as https://github.com/microsoft/sarif-tutorials/issues/40 [...]
[Bug other/116724] RFE: can generated SARIF diagnostics contain untranslated messages (plus translations?)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116724
--- Comment #5 from David Malcolm ---
(In reply to Hans-Peter Nilsson from comment #4)
> (In reply to David Malcolm from comment #1)
>
> > Perhaps we should try to capture both the untranslated text and the
> > translated text? SARIF has various abilities for handling translations.
To clarify, consider this hypothethical diagnostic:
error_at (location,
"missing %qs after %qs",
"decl-name", "foo");
with a hypothetical "pig-latin" locale and translation (pig-latin.po); see
https://en.wikipedia.org/wiki/Pig_Latin
where
"missing %qs after %qs"
has this translation in the .po file:
"issingmay %qs afteray %qs"
The classic text output format might read:
foo.c:42:11: erroray: issingmay `decl-name' afteray `foo'
and currently GCC's SARIF output would presumably capture the text of the
message with:
message: {"text": {"issingmay `decl-name' afteray `foo'"}}
i.e. currently GCC's SARIF output for a formatted string "bakes in" both
localization of the format string *and* param substitution.
We could instead defer parameter substitution to the SARIF consumer via §3.11.5
"Messages with placeholders"
(https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141790716)
like this:
message: {"text": {"issingmay `{0}' afteray `{1}'",
"arguments: ["decl-name", "foo"]}
and, with that, potentially capture the pre-translated message string *and* its
translation in the currently after .po file e.g.:
message: {id: "missing %qs after %qs",
"arguments: ["decl-name", "foo"]}
with something like this: (see 3.11.7 "Message string lookup")
"translations": [
{ # A toolComponent object.
"language": "pig-latin",
"contents": ["localizedData"],
"globalMessageStrings": [
{"missing %qs after %qs": {"text": "issingmay {0} afteray {1}"}}]}]
where we'd list the subset of format strings that got used by diagnostics in
the particular log, and their translations, with the caveats that:
- I'm not sure that that's how translations of strings are meant to be stored
(the SARIF spec's tutorial doesn't seem to cover translations yet)
- I'm using (abusing?) the string as its own "id"
If gettext supported it, could even try to capture translations from *all* .po
files. But if needed that's probably much easier to handle via a
post-processing script.
> Works for me! The use-case I was thinking of, is for the SARIF output to be
> a nice containment of the non-source-code part of bug-reports: "instead of
> quoting stderr, use --diagnostics-format=sarif-file and send
> sourcename.sarif".
Sounds like an interesting idea; can you open this as a separate RFE please?
> But, to fulfill that, more is needed, including the gcc
> arguments. (Maybe that's all.)
I've added support for capturing the command-line arguments in GCC 15:
https://gcc.gnu.org/pipermail/gcc-patches/2024-July/658206.html
though note that it's capturing the arguments as supplied by the driver to e.g.
cc1, as opposed to those that the user supplied to the driver.
> I don't see that included, right?
> Sorry for the "creaturization request"!
Thanks for the feedback; hope the above makes sense.
[Bug other/116724] RFE: can generated SARIF diagnostics contain untranslated messages (plus translations?)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116724
--- Comment #3 from David Malcolm ---
Note to self:
499 diagnostic_set_info (diagnostic_info *diagnostic, const char *gmsgid,
500 va_list *args, rich_location *richloc,
501 diagnostic_t kind)
502 {
503 gcc_assert (richloc);
504 diagnostic_set_info_translated (diagnostic, _(gmsgid), args, richloc,
kind);
505 }
Perhaps the diagnostic_info should store both the untranslated *and* translated
strings. Could probably defer the _() lookup until later, e.g. if the warning
isn't actually emitted.
[Bug other/116724] RFE: can generated SARIF diagnostics contain untranslated messages (plus translations?)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116724 --- Comment #2 from David Malcolm --- See e.g. §3.19.4 "Translations" https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141790787
[Bug other/116724] RFE: can generated SARIF diagnostics contain untranslated messages (plus translations?)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116724
--- Comment #1 from David Malcolm ---
I tried this for some examples; consider:
$ LANG=C ./xgcc -B. ../../src/gcc/testsuite/gcc.dg/format/sentinel-1.c
-fdiagnostics-format=text -Wall -S -fdiagnostics-format=text
../../src/gcc/testsuite/gcc.dg/format/sentinel-1.c:15:1: warning: 'sentinel'
attribute only applies to function types [-Wattributes]
15 | extern int a ATTR; /* { dg-warning "applies to function types"
"sentinel" } */
| ^~
../../src/gcc/testsuite/gcc.dg/format/sentinel-1.c:19:1: warning: 'sentinel'
attribute requires prototypes with named arguments [-Wattributes]
19 | extern void foo3 () ATTR; /* { dg-warning "named arguments" "sentinel"
} */
| ^~
../../src/gcc/testsuite/gcc.dg/format/sentinel-1.c:20:1: warning: 'sentinel'
attribute only applies to variadic functions [-Wattributes]
20 | extern void foo4 (const char *, int) ATTR; /* { dg-warning "variadic
functions" "sentinel" } */
| ^~
[...snip...]
Now try running with LANG=ja_JP.UTF-8
$ LANG=ja_JP.UTF-8 ./xgcc -B.
../../src/gcc/testsuite/gcc.dg/format/sentinel-1.c -fdiagnostics-format=text
-Wall -S -fdiagnostics-format=text
../../src/gcc/testsuite/gcc.dg/format/sentinel-1.c:15:1: 警告: ‘sentinel’
属性は関数型にのみ適用できます [-Wattributes]
15 | extern int a ATTR; /* { dg-warning "applies to function types"
"sentinel" } */
| ^~
../../src/gcc/testsuite/gcc.dg/format/sentinel-1.c:19:1: 警告: ‘sentinel’
属性は名前付き引数があるプロトタイプが必要です [-Wattributes]
19 | extern void foo3 () ATTR; /* { dg-warning "named arguments" "sentinel"
} */
| ^~
../../src/gcc/testsuite/gcc.dg/format/sentinel-1.c:20:1: 警告: ‘sentinel’
attribute only applies to variadic functions [-Wattributes]
20 | extern void foo4 (const char *, int) ATTR; /* { dg-warning "variadic
functions" "sentinel" } */
| ^~
[...lots snipped...]
Note that some of the msg fmt strings have translations in ja.po; others don't.
Trying again with -fdiagnostics-format=sarif-file
"results": [{"ruleId": "-Wattributes",
"level": "warning",
"message": {"text":
"‘[sentinel](https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-sentinel-function-attribute)’
属性は関数型にのみ適用できます"},
[...snipped...]
Note how the message text in the SARIF output is translated via the ja.po,
which could be a problem if the maintainer doesn't speak the same language as
the user.
Perhaps we should try to capture both the untranslated text and the translated
text? SARIF has various abilities for handling translations.
[Bug other/116724] New: RFE: can generated SARIF diagnostics contain untranslated messages (plus translations?)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116724 Bug ID: 116724 Summary: RFE: can generated SARIF diagnostics contain untranslated messages (plus translations?) Product: gcc Version: unknown Status: UNCONFIRMED Keywords: SARIF Severity: normal Priority: P3 Component: other Assignee: unassigned at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org CC: hp at gcc dot gnu.org Target Milestone: --- We were chatting about this at Cauldron; filing this as a placeholder for now
[Bug c/87588] gcc does not warn about unused variable which references to itself
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87588 David Malcolm changed: What|Removed |Added CC||dmalcolm at gcc dot gnu.org --- Comment #3 from David Malcolm --- For reference, the warning about 'b' is emitted here: Breakpoint 6, check_global_declaration (snode=) at ../../src/gcc/cgraphunit.cc:1138 1138warning_at (DECL_SOURCE_LOCATION (decl), (gdb) bt #0 check_global_declaration (snode=) at ../../src/gcc/cgraphunit.cc:1138 #1 0x00f3013f in analyze_functions (first_time=true) at ../../src/gcc/cgraphunit.cc:1343 #2 0x00f335c8 in symbol_table::finalize_compilation_unit (this=0x7fffe9a06000) at ../../src/gcc/cgraphunit.cc:2560 #3 0x0157edb4 in compile_file () at ../../src/gcc/toplev.cc:478 #4 0x00c8ccbe in do_compile () at ../../src/gcc/toplev.cc:2209 #5 toplev::main (this=this@entry=0x7fffdb0e, argc=, argc@entry=19, argv=, argv@entry=0x7fffdc48) at ../../src/gcc/toplev.cc:2369 #6 0x00c8e0a2 in main (argc=19, argv=0x7fffdc48) at ../../src/gcc/main.cc:39
[Bug c/87588] gcc does not warn about unused variable which references to itself
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87588 --- Comment #2 from David Malcolm --- FWIW here's the reproducer in Compiler Explorer: https://godbolt.org/z/54KzMcasP
[Bug analyzer/111567] RFE: support __attribute__((counted_by)) in -fanalyzer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111567 --- Comment #4 from David Malcolm --- Perhaps a lot of this could be handled by teaching the analyzer about the .ACCESS_WITH_SIZE internal func (that wraps an access, annotating it with size info)
[Bug analyzer/111567] RFE: support __attribute__((counted_by)) in -fanalyzer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111567 --- Comment #3 from David Malcolm --- FWIW I posted some ideas about this here: https://gcc.gnu.org/pipermail/gcc-patches/2024-June/653719.html
[Bug analyzer/116060] -fanalyzer -fdiagnostics-text-art-charset=unicode replaces typedef'ed type with "int" in some cases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116060 --- Comment #1 from David Malcolm --- https://godbolt.org/z/jrqEKfsd1
[Bug other/116603] SARIF schema URL gives 404 not found
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116603 David Malcolm changed: What|Removed |Added Last reconfirmed||2024-09-10 Ever confirmed|0 |1 Status|UNCONFIRMED |ASSIGNED --- Comment #2 from David Malcolm --- Should be fixed on trunk for GCC 15 by the above patch. Keeping open to track backporting to GCC 14 and GCC 13.
[Bug analyzer/116173] [15 regression] analyzer/call-summary.cc:727:28: warning: unused variable ‘summary_cast_reg’ [-Wunused-variable] since r15-1108-g70f26314b62e2d
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116173 --- Comment #2 from David Malcolm --- Thanks
[Bug d/116632] New: d_diagnostic_report_diagnostic and non-textual diagnostic output formats (e.g. SARIF)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116632
Bug ID: 116632
Summary: d_diagnostic_report_diagnostic and non-textual
diagnostic output formats (e.g. SARIF)
Product: gcc
Version: unknown
Status: UNCONFIRMED
Keywords: SARIF
Severity: normal
Priority: P3
Component: d
Assignee: ibuclaw at gdcproject dot org
Reporter: dmalcolm at gcc dot gnu.org
Blocks: 116613
Target Milestone: ---
I notice that d_diagnostic_report_diagnostic has:
if (loc.filename () || !verbatim)
{
// Trimmed for quoting here, but uses diagnostic_report_diagnostic
}
else
{
/* Write verbatim messages with no location direct to stream. */
text_info text (expand_d_format (format), &argp, errno, nullptr);
pp_format_verbatim (global_dc->printer, &text);
pp_newline_and_flush (global_dc->printer);
}
What should that "else" clause do for non-textual output formats, such as
SARIF? The output format code assumes all diagnostics goes through
diagnostic_report_diagnostic.
I have a feeling that they're going to go straight into the SARIF output as
plain text, leading to malformed JSON.
How do I trigger these, and what's the motivation/intent of the "else" clause
here?
Thanks!
Referenced Bugs:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116613
[Bug 116613] RFE: support outputting diagnostics in *multiple* formats
[Bug other/116613] New: RFE: support outputting diagnostics in *multiple* formats
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116613 Bug ID: 116613 Summary: RFE: support outputting diagnostics in *multiple* formats Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: other Assignee: unassigned at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org Target Milestone: --- Currently there is a single diagnostic output format, specified via -fdiagnostics-format=. It would be nice if it were possible to generate both text *and* SARIF output from a single invocation of the compiler. This could be done be revamping -fdiagnostics-format= to support multiple "sinks", somehow, but see also bug 115970.
[Bug other/116603] New: SARIF schema URL gives 404 not found
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116603 Bug ID: 116603 Summary: SARIF schema URL gives 404 not found Product: gcc Version: unknown Status: UNCONFIRMED Keywords: SARIF Severity: normal Priority: P3 Component: other Assignee: unassigned at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org Target Milestone: --- Currently we're using: #define SARIF_SCHEMA "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json"; but that URL gives 404 not found. The spec gives this URL: https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/schemas/sarif-schema-2.1.0.json which works, so presumably we should use that instead. Should also doublecheck the schema copy that we check against in DejaGnu.
[Bug other/116602] New: RFE: capture GCC backtraces in SARIF output for ICEs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116602 Bug ID: 116602 Summary: RFE: capture GCC backtraces in SARIF output for ICEs Product: gcc Version: unknown Status: UNCONFIRMED Keywords: SARIF Severity: normal Priority: P3 Component: other Assignee: unassigned at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org Target Milestone: --- I have an experimental patch that uses libbacktrace to scrape a backtrace, turn it into JSON values, and add it to SARIF output on an ICE. Might be too ambitious for a crash handler, but filing this here as a reminder to myself to finish this.
[Bug other/116419] RFE: capture URLs in SARIF messages
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116419 David Malcolm changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #6 from David Malcolm --- Should be implemented by the above patch for GCC 15.
[Bug driver/110522] `-fdiagnostics-format=sarif-file`: file name conflicts / races
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110522 --- Comment #4 from David Malcolm --- I wonder if extending -fdiagnostics-format to support extra args would be a way out of this e.g. -fdiagnostics-format=sarif-file=path/to/foo.sarif But it would also be nice to support multiple output streams e.g. -fdiagnostics-format=text,sarif-file=path/to/foo.sarif Or a new param: -fdiagnostics-sarif-output-file=PATH/TO/foo.sarif or somesuch.
[Bug driver/110522] `-fdiagnostics-format=sarif-file`: file name conflicts / races
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110522 David Malcolm changed: What|Removed |Added See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=115970 Ever confirmed|0 |1 Last reconfirmed||2024-08-27 Status|UNCONFIRMED |NEW --- Comment #3 from David Malcolm --- Thanks for filing this bug; you make some very useful points. I had thought that the .sarif filename was respecting the dumpfile options (e.g. -dumpdir, see https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#index-dumpdir ) but it turns out that it doesn't even handle that. Bother. I like the idea of using the output file name/path as the basis on which to build the .sarif filename (e.g. outputdir/foo.o.sarif), but there may be projects that are using the existing behavior, so we'd probably need some way to ease transition. Some other possibly-awkward cases: what about e.g. -S, or gcc invocations that take multiple source files, and/or those that imply a.out as the output? It may make sense to support both text *and* sarif output of diagnostics, to avoid the "nothing on stderr" problem. See also bug 115970 for another approach to capturing sarif from a build.
[Bug other/116419] RFE: capture URLs in SARIF messages
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116419 --- Comment #4 from David Malcolm --- Filed https://github.com/oasis-tcs/sarif-spec/issues/658 about ambiguity with '[' literals in plaintext messages.
[Bug other/116419] RFE: capture URLs in SARIF messages
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116419 --- Comment #3 from David Malcolm --- Filed https://github.com/oasis-tcs/sarif-spec/issues/657 about possible need to escape URIs within embedded links.
[Bug other/116419] RFE: capture URLs in SARIF messages
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116419 --- Comment #2 from David Malcolm --- Filed https://github.com/oasis-tcs/sarif-spec/issues/656 with the spec about escaping of link text.
[Bug other/116419] RFE: capture URLs in SARIF messages
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116419 David Malcolm changed: What|Removed |Added Ever confirmed|0 |1 Last reconfirmed||2024-08-19 Status|UNCONFIRMED |ASSIGNED --- Comment #1 from David Malcolm --- Note that we could e.g. use the sarif URI scheme to handle event IDs in messages (e.g. "first `free' at (1)", so these could be marked as links to the corresponding events.
[Bug other/116419] New: RFE: capture URLs in SARIF messages
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116419 Bug ID: 116419 Summary: RFE: capture URLs in SARIF messages Product: gcc Version: unknown Status: UNCONFIRMED Keywords: SARIF Severity: normal Priority: P3 Component: other Assignee: unassigned at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org Target Milestone: --- GCC diagnostic messages can contain URLs, but I believe we're currently dropping them on the floor when emitting SARIF. See "3.11.6 Messages with embedded links": https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141790717 for how we could express these in SARIF. I'm working on implementing this.
[Bug other/116392] New: RFE: capture inlining information in SARIF output
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116392
Bug ID: 116392
Summary: RFE: capture inlining information in SARIF output
Product: gcc
Version: unknown
Status: UNCONFIRMED
Keywords: SARIF
Severity: normal
Priority: P3
Component: other
Assignee: unassigned at gcc dot gnu.org
Reporter: dmalcolm at gcc dot gnu.org
Target Milestone: ---
Similar to RFE 116300 (which is for macro expansions), our textual output can
report inlining information for middle-end warnings, but our SARIF output
doesn't yet capture that.
AFAIK SARIF doesn't yet have a way of expressing this, but it could probably be
done via a new kind of location relationship.
An example can be seen in gcc/testsuite/gcc.dg/Wnonnull.c in our testsuite:
https://godbolt.org/z/3o1ehYf7b
which has e.g.:
In function 'my_strcpy',
inlined from 'my_strcpy' at :20:1,
inlined from 'test' at :40:5:
:27:7: warning: argument 2 null where non-null expected [-Wnonnull]
27 | memcpy (dst, src, size - 1); /* { dg-warning "argument 2 null
where non-null expected" } */
| ^~~
: In function 'test':
:7:14: note: in a call to function 'memcpy' declared 'nonnull'
7 | extern void *memcpy (void *__restrict __dest, const void *__restrict
__src,
| ^~
SARIF output for the same is here:
https://godbolt.org/z/9W5M7Gd31
where the SARIF corresponding to the above currently has just:
{"ruleId": "-Wnonnull",
"level": "warning",
"message": {"text": "argument 2 null where non-null expected"},
"locations": [{"physicalLocation": {"artifactLocation": {"uri": ""},
"region": {"startLine": 27,
"startColumn": 7,
"endColumn": 34},
"contextRegion": {"startLine": 27,
"snippet": {"text": "
memcpy (dst, src, size - 1); /* { dg-warning \"argument 2 null where
non-null expected\" } */\n"}}},
"logicalLocations": [{"name": "test",
"fullyQualifiedName": "test",
"decoratedName": "test",
"kind": "function"}]}],
"relatedLocations": [{"physicalLocation": {"artifactLocation": {"uri":
""},
"region": {"startLine": 7,
"startColumn": 14,
"endColumn": 20},
"contextRegion": {"startLine": 7,
"snippet":
{"text": "extern void *memcpy (void *__restrict __dest, const void *__restrict
__src,\n"}}},
"message": {"text": "in a call to function 'memcpy'
declared 'nonnull'"}}]}]}]}
which is missing all that inlining info.
[Bug analyzer/116304] New: False negative from -fanalyzer on "return NULL"; due to NULL being in a system header
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116304 Bug ID: 116304 Summary: False negative from -fanalyzer on "return NULL"; due to NULL being in a system header Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: analyzer Assignee: dmalcolm at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org Target Milestone: --- gcc/testsuite/c-c++-common/analyzer/malloc-CWE-401-example.c has: return NULL; /* TODO: should complain that "buf" is leaked on this path. */ I'm filing this bug to track that issue. Debugging Arsen's patch [PATCH v2] c++: improve diagnostic of 'return's in coroutines https://gcc.gnu.org/pipermail/gcc-patches/2024-August/659933.html showed that the diagnostics subsystem seems to be rejecting the diagnostic here due to the "NULL" of return NULL being in a system header: Breakpoint 8, diagnostic_context::report_diagnostic (this=0x4b00e80 , diagnostic=0x7fff7d20) at ../../src/gcc/diagnostic.cc:1477 1477return false; (gdb) list 1472 && ((!m_warn_system_headers 1473 && diagnostic->m_iinfo.m_allsyslocs) 1474 || m_inhibit_warnings)) 1475/* Bail if the warning is not to be reported because all locations in the 1476 inlining stack (if there is one) are in system headers. */ 1477return false; 1478 1479 if (diagnostic->kind != DK_NOTE && diagnostic->kind != DK_ICE) 1480diagnostic_check_max_errors (this); 1481 (gdb) list 1470 1465 /* Check to see if the diagnostic is enabled at the location and 1466 not disabled by #pragma GCC diagnostic anywhere along the inlining 1467 stack. . */ 1468 if (!diagnostic_enabled (diagnostic)) 1469return false; 1470 1471 if ((was_warning || diagnostic->kind == DK_WARNING) 1472 && ((!m_warn_system_headers 1473 && diagnostic->m_iinfo.m_allsyslocs) 1474 || m_inhibit_warnings)) (gdb) 1475/* Bail if the warning is not to be reported because all locations in the 1476 inlining stack (if there is one) are in system headers. */ 1477return false; 1478 1479 if (diagnostic->kind != DK_NOTE && diagnostic->kind != DK_ICE) 1480diagnostic_check_max_errors (this); 1481 1482 m_lock++; 1483 1484 if (diagnostic->kind == DK_ICE || diagnostic->kind == DK_ICE_NOBT) (gdb) p was_warning $8 = true (gdb) p m_warn_system_headers $9 = false (gdb) p diagnostic->m_iinfo.m_allsyslocs $10 = true
[Bug other/116301] New: RFE: provide support for future SARIF versions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116301 Bug ID: 116301 Summary: RFE: provide support for future SARIF versions Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: other Assignee: unassigned at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org Target Milestone: --- Our SARIF output code is currently hardcoded to emit version 2.1.0 of SARIF. 2.2 isn't out yet, but has gained some features we might want to use, such as relatedLocations within notification objects. We probably want an option to specify which version of sarif we output e.g. -fsarif-version=2.1.0 or somesuch, and default to emitting the most recently approved version of the standard.
[Bug other/116300] New: RFE: capture macro expansion information in SARIF output
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116300 Bug ID: 116300 Summary: RFE: capture macro expansion information in SARIF output Product: gcc Version: unknown Status: UNCONFIRMED Keywords: SARIF Severity: normal Priority: P3 Component: other Assignee: unassigned at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org Target Milestone: --- Our machine-readable diagnostic output doesn't include information on macro expansions. It would be nice to capture it in the SARIF output - though SARIF doesn't yet specify a way to do so. See https://github.com/oasis-tcs/sarif-spec/issues/650 I'm experimenting with a way to capture these using relatedLocations and location relationships (similar to include chains; see bug 107941)
[Bug other/116177] SARIF output invocation's "executionSuccessful": true even when failing with an error (§3.20.14)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116177 David Malcolm changed: What|Removed |Added Ever confirmed|0 |1 Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2024-08-06 --- Comment #2 from David Malcolm --- Should be fixed on trunk for GCC 15 by the above trunk. Keeping open to track backporting the fix to GCC 13 and GCC 14.
[Bug other/115970] RFE: add support for sending SARIF output via IPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115970 David Malcolm changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |dmalcolm at gcc dot gnu.org Last reconfirmed||2024-08-06 Status|UNCONFIRMED |ASSIGNED Ever confirmed|0 |1
[Bug other/115970] RFE: add support for sending SARIF output via IPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115970 --- Comment #4 from David Malcolm --- Created attachment 58854 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58854&action=edit WIP patch to output diagnostics as SARIF notifications to a unix domain socket The attached patch is very much a prototype. If the environment has SARIF_SOCKET set to a path to a socket, then GCC diagnostics are emitted as JSON-RPC notifications to the socket in a manner akin to that for "SARIF_OUTPUT_PIPE" in P3358R0, albeit with Unix domain sockets. The patch also implements a trivial "sarif-listener" program that creates the socket and listens for notifications. The patch adds a -fdiagnostics-format=sarif-socket which currently is also required, but it might be good to simply have the environment variable. That way an IDE could make the socket and set the env var when invoking a build framework, and then all diagnostics would go to the IDE, rather than textually. Or we could do both text *and* socket output, but that would require more surgery to the diagnostics framework.
[Bug other/116253] RFE: support for nested diagnostics
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116253 --- Comment #2 from David Malcolm --- Created attachment 58852 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58852&action=edit Output from attachment 58851 with -fconcepts-diagnostics-depth=3 -std=c++20 -fdiagnostics-format=sarif-file on the testcase in P3358R0
[Bug other/116253] RFE: support for nested diagnostics
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116253 David Malcolm changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |dmalcolm at gcc dot gnu.org Last reconfirmed||2024-08-06 Ever confirmed|0 |1 Status|UNCONFIRMED |ASSIGNED --- Comment #1 from David Malcolm --- Created attachment 58851 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58851&action=edit Messy prototype of P3358R0 Here's the messy prototype of P3358R0 I implemented; see the screenshot in https://github.com/oasis-tcs/sarif-spec/issues/572#issuecomment-2243386699 Adds: * a simple way to nest diagnostics within GCC's internals (although the nesting level doesn't exactly match the existing -fconcepts-diagnostics-depth=; each depth of -fconcepts-diagnostics-depth= could correspond to several levels of nested messages). * support for this in SARIF output using the property bag property nestingLevel as per P3358R0 (though see my concerns about this in that sarif-spec issue) * a hacked-up way to visualize nesting levels in GCC's textual output Caveats: lots of FIXMEs, and the textual output is obviously excessively verbose, for clarity during debugging.
[Bug other/116253] New: RFE: support for nested diagnostics
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116253 Bug ID: 116253 Summary: RFE: support for nested diagnostics Product: gcc Version: unknown Status: UNCONFIRMED Keywords: SARIF Severity: normal Priority: P3 Component: other Assignee: unassigned at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org Target Milestone: --- There's an interesting proposal from Sy Brand here: P3358R0: SARIF for Structured Diagnostics https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p3358r0.html (and https://github.com/oasis-tcs/sarif-spec/issues/572) for better supporting C++ template diagnostics by introducing a hierarchical/nested structure. I have a prototype of this for GCC working, though I have some concerns about the proposal as it stands - I think it has a bad interaction with how SARIF expresses header inclusion chains - which I've mentioned in https://github.com/oasis-tcs/sarif-spec/issues/572#issuecomment-2243386699 Aspects of this: * extend g++ to add hierarchical structure to the messages * extend GCC's text-based diagnostic output to emit that structure, somehow * work with Sy, WG15 and the SARIF TC to improve the way it's expressed in SARIF * extend GCC's SARIF output to support the structure
[Bug analyzer/116228] [14/15 regression] -fanalyzer ICE in get_or_create_null_ptr, at analyzer/region-model-manager.cc:271
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116228 David Malcolm changed: What|Removed |Added Last reconfirmed||2024-08-05 Status|UNCONFIRMED |NEW Ever confirmed|0 |1 --- Comment #1 from David Malcolm --- Thanks for filing this bug. ICE confirmed with trunk: https://godbolt.org/z/37nhjEj4h ICE confirmed with GCC 14.1: https://godbolt.org/z/bdb1oKM4s ICE doesn't affect GCC 13.3: https://godbolt.org/z/KjMvvr456
[Bug other/116201] New: SARIF output doesn't properly reference the CWE taxonomy
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116201
Bug ID: 116201
Summary: SARIF output doesn't properly reference the CWE
taxonomy
Product: gcc
Version: unknown
Status: UNCONFIRMED
Keywords: SARIF
Severity: normal
Priority: P3
Component: other
Assignee: unassigned at gcc dot gnu.org
Reporter: dmalcolm at gcc dot gnu.org
Target Milestone: ---
We have:
std::unique_ptr
sarif_builder::
make_tool_component_reference_object_for_cwe () const
{
auto comp_ref_obj = ::make_unique ();
/* "name" property (SARIF v2.1.0 section 3.54.3). */
comp_ref_obj->set_string ("name", "cwe");
return comp_ref_obj;
}
where name is lowercase "cwe" whereas in:
std::unique_ptr
sarif_builder::maybe_make_cwe_taxonomy_object () const
{
if (m_cwe_id_set.is_empty ())
return nullptr;
auto taxonomy_obj = ::make_unique ();
/* "name" property (SARIF v2.1.0 section 3.19.8). */
taxonomy_obj->set_string ("name", "CWE");
the taxonomy object has uppercase name "CWE".
We should use uppercase for both for consistency.
That said, the spec says:
> 3.54.3 name property
>
> A toolComponentReference object MAY contain a property named name whose
> value is a string equal to theComponent.name (§3.19.8).
>
> NOTE: This property does not participate in the lookup, but its presence
> improves the readability of the log file at the expense of increased file
> size.
and so we should be using "index" or "guid" to establish the reference
(probably by adding an "index" property to the toolComponentReference).
[Bug other/116177] New: SARIF output invocation's "executionSuccessful": true even when failing with an error (3.20.14)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116177
Bug ID: 116177
Summary: SARIF output invocation's "executionSuccessful": true
even when failing with an error (3.20.14)
Product: gcc
Version: unknown
Status: UNCONFIRMED
Keywords: SARIF
Severity: normal
Priority: P3
Component: other
Assignee: unassigned at gcc dot gnu.org
Reporter: dmalcolm at gcc dot gnu.org
Target Milestone: ---
When the compilation fails with an error, the SARIF output invocation's
"executionSuccessful" is true. Presumably this should be false for such cases.
Quoting
https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141790831
> An invocation object SHALL contain a property named executionSuccessful whose
> value is a Boolean that is true if the engineering system that started the
> process
> knows that the analysis tool succeeded, and false if the engineering system
> knows
> that the tool failed.
>
> NOTE: This property is needed because not all programs exit with an exit code
> of 0 on success and non-0 on failure.
>
> EXAMPLE:
>
> {
> "exitCode": 1,
> "exitCodeDescription": "Scan successful; warnings detected.",
> "executionSuccessful": true
>
> }
[Bug other/116176] New: SARIF output doesn't yet capture nesting of logical locations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116176
Bug ID: 116176
Summary: SARIF output doesn't yet capture nesting of logical
locations
Product: gcc
Version: unknown
Status: UNCONFIRMED
Keywords: SARIF
Severity: normal
Priority: P3
Component: other
Assignee: unassigned at gcc dot gnu.org
Reporter: dmalcolm at gcc dot gnu.org
Target Milestone: ---
Consider:
namespace ns {
class foo
{
void bar ()
{
return 42;
}
};
}
for which we emit:
: In member function 'void ns::foo::bar()':
:6:20: error: return-statement with a value, in function returning
'void' [-fpermissive]
6 | return 42;
|^~
Compiler returned: 1
g++ trunk with -fdiagnostics-format=sarif-stderr:
https://godbolt.org/z/d6endYsr9
where the result has this logical location:
"logicalLocations": [{"name": "bar",
"fullyQualifiedName": "ns::foo::bar",
"decoratedName": "_ZN2ns3foo3barEv",
"kind": "function"}]}]}]}]}
SARIF supports capturing the "ancestry" of a logical location via §3.33.8
parentIndex property, and can consolidate logical locations into the run via
run.logicalLocations (§3.14.17), allowing a consumer to query e.g. "all results
affecting namespace 'ns'".
We should capture this ancestry in our SARIF output, and thus in this example
emit logical locations for "class ns::foo" and for "namespace ns".
[Bug other/116164] New: SARIF output doesn't yet capture fix-it hints on notes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116164 Bug ID: 116164 Summary: SARIF output doesn't yet capture fix-it hints on notes Product: gcc Version: unknown Status: UNCONFIRMED Keywords: SARIF Severity: normal Priority: P3 Component: other Assignee: unassigned at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org Target Milestone: --- Currently our SARIF output only captures fix-it hints on the top-level diagnostic in a group. Followup notes within a group are added as relatedLocations to the result, but we fail to capture any fix-it hints. I haven't yet found an existing reproducer for this in our testsuite, but there's a case of it in my forthcoming https://gcc.gnu.org/wiki/libdiagnostics testsuite (test-note-with-fix-it-hint.c) Presumably we should add fix-it hints for notes into the result["fixes"]
[Bug testsuite/116163] New: RFE: add a linting tool for DegaGnu tests
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116163 Bug ID: 116163 Summary: RFE: add a linting tool for DegaGnu tests Product: gcc Version: unknown Status: UNCONFIRMED Keywords: internal-improvement Severity: normal Priority: P3 Component: testsuite Assignee: unassigned at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org Target Milestone: --- Sam's been doing great work lately fixing malformed dg directives in our testsuite. We chatted briefly on IRC about maybe having a linting tool to automate detection of the common mistakes, so that we can prevent categories of mistakes going forward. Filing this RFE to capture this idea, and to give a place to record ideas for things it could check. Could probably be done as a simple Python 3 script under "contrib".
[Bug testsuite/116143] [15 regression] gcc.dg/plugin/diagnostic-* test fails intermittently
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116143 --- Comment #5 from David Malcolm --- (In reply to David Malcolm from comment #1) [...snip...] > My guess is that nothing is using class simple_diagnostic_path in cc1, and > thus the code for simple_diagnostic_path is being dropped by the linker, so > it's not available to the plugin. It's used by a selftest in simple-diagnostic-path.cc, but that's only enabled if #if CHECKING_P so maybe this is failing on release builds and passing on debug builds??? FWIW I've seen a couple of patches in progress that add "real" uses of the class to the middle-end (e.g. Qing's "[RFC][PATCH v1] Provide more contexts for -Warray-bounds warning messages " https://gcc.gnu.org/pipermail/gcc-patches/2024-July/656199.html )
[Bug other/115970] RFE: add support for sending SARIF output to a pipe
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115970 --- Comment #3 from David Malcolm --- >From what I can tell, in Microsoft's implementation the JSON-RPC messages are being "packaged" or "framed" via LSP's base protocol, as per: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#baseProtocol but the notifications are a subset of SARIF.
[Bug other/116143] [15 regression] gcc.dg/plugin/diagnostic-* test fails intermittently
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116143 --- Comment #1 from David Malcolm --- Sorry about this. Demangling, _ZNK22simple_diagnostic_path10num_eventsEv is simple_diagnostic_path::num_events() const which is a vfunc implemented in gcc/simple-diagnostic-path.o My guess is that nothing is using class simple_diagnostic_path in cc1, and thus the code for simple_diagnostic_path is being dropped by the linker, so it's not available to the plugin.
[Bug middle-end/107941] json diagnostics format does not include header stack info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107941 David Malcolm changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #7 from David Malcolm --- (In reply to David Malcolm from comment #5) > I plan to implement this in our SARIF output (adding "SARIF" keyword). I've implemented this via the above patch, for GCC 15. > FWIW I wasn't planning to add anything further to GCC's "json" output, > focusing instead on SARIF output. I don't plan to implement it for the "json" output. Closing this out as "resolved fixed".
[Bug translation/79423] Translation of warnings breaks IDE parsing of output
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79423
--- Comment #7 from David Malcolm ---
We've had -fdiagnostics-format=json since GCC 9, and
-fdiagnostics-format=sarif-{file,stderr} support since GCC 13, IIRC. Arguably
an IDE should be using the latter instead.
[Bug middle-end/107941] json diagnostics format does not include header stack info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107941 David Malcolm changed: What|Removed |Added Keywords||SARIF --- Comment #5 from David Malcolm --- The SARIF spec supports recording include stack info via the 3.34 "locationRelationship" object (in SARIF 2.1.0); we're just not yet generating it yet. I plan to implement this in our SARIF output (adding "SARIF" keyword). FWIW I wasn't planning to add anything further to GCC's "json" output, focusing instead on SARIF output.
[Bug analyzer/116006] New: RFE: detect usage of pointers to local variables that are out of scope
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116006
Bug ID: 116006
Summary: RFE: detect usage of pointers to local variables that
are out of scope
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: analyzer
Assignee: dmalcolm at gcc dot gnu.org
Reporter: dmalcolm at gcc dot gnu.org
Target Milestone: ---
Consider e.g.:
struct A *extref = something;
...
for (int i = 0; i < n; i++)
{
struct A intref = *extref;
intref.memb = i;
extref = &intref;
}
This is storing a pointer to local variable "intref" into extref.
I think we detect the case where someone tries to use *extref once the function
frame has been popped (via the poisoned_svalue machinery) but I don't think we
detect the case where the usage happens in the same function but outside the
scope of "intref".
Trying it out here: https://godbolt.org/z/6s9WMh53K but I can't seem to trigger
even the popped frame case
[Bug other/115970] RFE: add support for sending SARIF output to a pipe
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115970 --- Comment #2 from David Malcolm --- One difference here is that VS code is sending the diagnostics one at a time as JSON-RPC messages containing SARIF "result" objects as they occur, whereas currently we write all our SARIF output at once on exit as a single "sarifLog" object.
[Bug other/115970] New: RFE: add support for sending SARIF output to a pipe
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115970 Bug ID: 115970 Summary: RFE: add support for sending SARIF output to a pipe Product: gcc Version: unknown Status: UNCONFIRMED Keywords: SARIF Severity: normal Priority: P3 Component: other Assignee: unassigned at gcc dot gnu.org Reporter: dmalcolm at gcc dot gnu.org Target Milestone: --- https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p3358r0.html#msvc describes a feature of Visual Studio 2022 version 17.8. which can send its diagnostics in SARIF form to a pipe: https://learn.microsoft.com/en-us/cpp/build/reference/sarif-output?view=msvc-170#retrieving-sarif-through-a-pipe This looks useful for IDE consumption. Maybe GCC should implement a similar feature?
[Bug other/84889] Ideas on revamping how we format diagnostics
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84889 --- Comment #25 from David Malcolm --- (In reply to David Malcolm from comment #0) [...snip...] > We might want to revisit our current colorization choices. Done, in https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657201.html Screenshots can be see at: https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657203.html [...snip...]
[Bug analyzer/115784] -Wanalyzer-infinite-loop does not take into account noreturn attribute
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115784 David Malcolm changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |WONTFIX --- Comment #3 from David Malcolm --- Thanks Alexey. It seems reasonable to me for a do-nothing forever loop to have *some* attention drawn to it in the source code, so my feeling here is that it's best to manually suppress these warnings with diagnostic_push/pop. I wondered if the documentation could be improved here, but looking at https://gcc.gnu.org/onlinedocs/gcc/Static-Analyzer-Options.html#index-Wanalyzer-infinite-loop it seems reasonable to me. I'm going to close this ticket out (with the code "WONTFIX") - thanks again.
[Bug analyzer/115784] -Wanalyzer-infinite-loop does not take into account noreturn attribute
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115784 --- Comment #1 from David Malcolm --- Thanks for filing this bug report. The analyzer reports -Wanalyzer-infinite-loop on the example code you posted; trunk on Compiler Explorer: https://godbolt.org/z/WzPEc3hxh But it is indeed an infinite loop, so it's arguably a true positive. It sounds like you want the warning to be suppressed in "noreturn" functions (perhaps with some kind of heuristic???) Do you have an example of real-world code that runs into this?
[Bug analyzer/115724] analyzer does not recognise glibc (GNU) extension, non-returning error
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115724 --- Comment #9 from David Malcolm --- Fixed by the above patch for trunk for gcc 15. Handling of "error" was added in r11-7333-g5ee4ba031dd9fc to fix PR analyzer/99196 so keeping this open to track backporting to earlier branches.
[Bug analyzer/115735] Analyzer misses trivial syslog() call in signal handler for -Wanalyzer-unsafe-call-within-signal-handler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115735 --- Comment #11 from David Malcolm --- FWIW I just noticed that your reproducer is wrong (I think) whilst trying to implement sigaction: if you're going to use sa_sigaction for the function pointer (rather than sa_handler), you have to set the SA_SIGINFO bit within sa_flags; see: https://www.man7.org/linux/man-pages/man2/sigaction.2.html https://linux.die.net/man/2/sigaction
[Bug analyzer/115735] Analyzer misses trivial syslog() call in signal handler for -Wanalyzer-unsafe-call-within-signal-handler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115735 --- Comment #9 from David Malcolm --- (In reply to Sam James from comment #4) > Hi David, I started hacking up a list based on the man pages I had from > `man-pages` & removing the POSIX list of safe functions, but it's kind of > gross. It's not something we could put in the build system. > > A friend pointed out we might be able to get the list from glibc's docs > (texi) which have markers. See e.g. > https://www.gnu.org/software/libc/manual/html_node/syslog_003b-vsyslog.html > which has: > > Preliminary: | MT-Safe env locale | AS-Unsafe corrupt heap lock dlopen | > > AC-Unsafe corrupt lock mem fd | See POSIX Safety Concepts. > > We want to parse the "AS-Unsafe" part out. > > I don't know what's best here. We could perhaps have a Python script in the "contrib" to regenerate a .def file included by sm-signal.cc Could take the lists of "all functions" and "known safe functions" and generate an unsafe list by taking the difference. Note that the POSIX list might not be redistributable, and the stuff in glibc is probably under the GFDL rather than the GPL. Using glibc's is probably the better bet; maybe we can get blanket permissions from the FSF to regenerate this GCC file from glibc's docs?
[Bug analyzer/115735] Analyzer misses trivial syslog() call in signal handler for -Wanalyzer-unsafe-call-within-signal-handler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115735 --- Comment #8 from David Malcolm --- Not only are we not treating "syslog" as unsafe, we're also failing to handle sigaction as a way of setting a signal handler. I'm taking a look.
[Bug analyzer/115736] Analyzer is sensitive to printf argument (or puts does not warn) for -Wanalyzer-unsafe-call-within-signal-handler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115736 --- Comment #2 from David Malcolm --- Indeed. Is "puts" safe or unsafe to call from a signal handler?
[Bug analyzer/115735] Analyzer misses trivial syslog() call in signal handler for -Wanalyzer-unsafe-call-within-signal-handler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115735 --- Comment #3 from David Malcolm --- The analyzer's list of functions known to be signal unsafe was introduced in r10-5959-g4804c5fe965eef and has only gained "exit" since then, in r11-577-g2221fb6f668a7e. Is there a wider list of such functions somewhere?
[Bug analyzer/115724] analyzer does not recognise glibc (GNU) extension, non-returning error
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115724 David Malcolm changed: What|Removed |Added Status|NEW |ASSIGNED --- Comment #7 from David Malcolm --- As noted by Jakub, it works fine at -O1 and above: https://godbolt.org/z/GrPv3rjoj I'll fix it in the analyzer for the -O0 case.
[Bug analyzer/115724] analyzer does not recognise glibc (GNU) extension, non-returning error
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115724 --- Comment #6 from David Malcolm --- Posted to mailing lists: https://gcc.gnu.org/pipermail/gcc/2024-July/244257.html https://sourceware.org/pipermail/libc-alpha/2024-July/157942.html
[Bug analyzer/115724] analyzer does not recognise glibc (GNU) extension, non-returning error
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115724
--- Comment #5 from David Malcolm ---
glib'c bits/error.h has:
/* If we know the function will never return make sure the compiler
realizes that, too. */
__extern_always_inline void
error (int __status, int __errnum, const char *__format, ...)
{
if (__builtin_constant_p (__status) && __status != 0)
__error_noreturn (__status, __errnum, __format, __va_arg_pack ());
else
__error_alias (__status, __errnum, __format, __va_arg_pack ());
}
and similar for error_at_line.
However, looking at GENERIC (via -fdump-tree-original=stderr)
https://godbolt.org/z/dqWr1G97s
I see that this is coming out of the C front end as:
;; Function error (null)
;; enabled by -tree-original
{
if (0)
{
__error_noreturn (__status, __errnum, __format, __builtin_va_arg_pack
());
}
else
{
__error_alias (__status, __errnum, __format, __builtin_va_arg_pack ());
}
}
before any of GCC's inlining logic gets to work on it.
Hence "test" quickly gets optimized down to just:
__error_alias (__status_2(D), __errnum_3(D), __format_4(D),
__builtin_va_arg_pack ());
and this is what is inlined into the callsite of "error".
Hence by the time the analyzer sees it we have a call to "__error_alias" which
the analyzer/kf.cc doesn't recognize, and conservatively assumes could return.
So there are arguably three issues here:
(a) should gcc keep the __builtin_constant_p around longer so that it's still
around when the inliner gets a chance to work on it? (I don't know if this is
a good idea or not)
(b) should glibc's header be rewritten (perhaps losing the
__builtin_constant_p?)
(c) the analyzer's special-casing for error and error_at_line should also
recognize __error_alias and __error_at_line_alias (probably an easy workaround)
[Bug analyzer/115724] analyzer does not recognise glibc (GNU) extension, non-returning error
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115724 --- Comment #4 from David Malcolm --- The analyzer *does* try to handle error() and error_at_line() from GNU's non-standard ; see: https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/analyzer/kf.cc;h=4213b89ac9fb4ff11994cf2c35f15a281be3b024;hb=HEAD#l496 In particular it tries to "know" that arg0 == 0 means "doesn't return". So something's going wrong with that; I'll take a look.
[Bug analyzer/115680] ICE in on_ranges, at analyzer/constraint-manager.cc:3166
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115680 David Malcolm changed: What|Removed |Added Ever confirmed|0 |1 Last reconfirmed||2024-06-27 Status|UNCONFIRMED |NEW --- Comment #6 from David Malcolm --- Thanks for filing this bug report. Minimal args to reproduce the ICE with attachment 58531 seem to be: -fanalyzer -ftree-vrp -std=gnu23 -O1 (https://godbolt.org/z/P6T91PzWf)
[Bug c/97687] -Wfatal-errors prints some notes but not others
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97687 --- Comment #7 from David Malcolm --- (In reply to David Malcolm from comment #5) > scope of the auto_diagnostic_context. ^^^ auto_diagnostic_group
[Bug c/97687] -Wfatal-errors prints some notes but not others
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97687 David Malcolm changed: What|Removed |Added Keywords||patch --- Comment #6 from David Malcolm --- (In reply to David Malcolm from comment #5) [...] > Unfinished, as it would need: also: fixing the typo "m_handing_fatal_error" -> "m_handling_fatal_error"
[Bug c/97687] -Wfatal-errors prints some notes but not others
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97687 --- Comment #5 from David Malcolm --- Created attachment 58523 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58523&action=edit WIP patch to try to print all notes for an error with -Wfatal-errors Am attaching an experimental patch for this that keeps the compiler around on -Wfatal-errors if there's an active diagnostic_group until after the diagnostic_group ends, or a non-note follows. Unfinished, as it would need: - ChangeLog - docs update - test cases (for text, sarif and json output); possibly via a test plugin to synthesize the case of a followup error within the same diagnostic_group But the main thing is, are we comfortable not bailing out immediately on the error in -Wfatal-errors? There could be non-trivial work being done in the scope of the auto_diagnostic_context. Perhaps something to discuss on mailing list.
[Bug c/97687] -Wfatal-errors prints some notes but not others
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97687 --- Comment #4 from David Malcolm --- In particular, consider e.g. the cases of SARIF and json output.
[Bug c/97687] -Wfatal-errors prints some notes but not others
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97687 --- Comment #3 from David Malcolm --- The "in expansion of macro ‘FOO’" notes (messages 2 and 4) are coming from the diagnostic finalizer, and so they are effectively printed as part of the first "error" (message 1) and the "previous declaration of ‘foo’ was here" note (message 3). Messages 1 and 3 are both coming from within gcc/c/c-decl.cc: duplicate_decls. There is an auto_diagnostic_group. Perhaps -Wfatal-errors should delay bailing out if an auto_diagnostic_group is still "in flight", but immediately bail out if it's about to emit another non-"note" diagnostic? That might give less surprising behavior, but probably has some nasty interactions with existing code.
[Bug c/97687] -Wfatal-errors prints some notes but not others
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97687 --- Comment #2 from David Malcolm --- (In reply to Nicholas Fraser from comment #0) Behavior confirmed on Compiler Explorer with trunk (for GCC 15); URLs as follows: > > gcc a.c https://godbolt.org/z/rxbz5eWMd > gcc -Wfatal-errors a.c > https://godbolt.org/z/e86nPefdM
[Bug bootstrap/115635] [15 regression] Bootstrap fails with failed self-test (diagnostic-path.cc:1153: test_empty_path: FAIL: ASSERT_FALSE ((path.interprocedural_p ())))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115635
--- Comment #1 from David Malcolm ---
"make selftest-valgrind" is clean for me. Note that if you can reproducer this
standalone, "make selftest-gdb" is a handy way to run the selftests under the
debugger.
FWIW, the output suggests that the assertion at line 1153 is failing:
1147 /* Verify that empty paths are handled gracefully. */
1148
1149 static void
1150 test_empty_path (pretty_printer *event_pp)
1151 {
1152test_diagnostic_path path (event_pp);
->1153ASSERT_FALSE (path.interprocedural_p ());
1154
"path" is empty i.e. num_events ought to be returning 0.
Hence I'd expect diagnostic_path::interprocedural_p:
184 bool
185 diagnostic_path::interprocedural_p () const
186 {
187/* Ignore leading events that are outside of any function. */
188unsigned first_fn_event_idx;
189if (!get_first_event_in_a_function (&first_fn_event_idx))
190 return false;
to call get_first_event_in_a_function, and for that to get 0 for "num", and
thus bail out with num == 0 again, never entering the loop here:
164 bool
165 diagnostic_path::get_first_event_in_a_function (unsigned *out_idx)
const
166 {
167const unsigned num = num_events ();
168for (unsigned i = 0; i < num; i++)
169 {
170const diagnostic_event &event = get_event (i);
171if (const logical_location *logical_loc =
event.get_logical_location ())
172 if (logical_loc->function_p ())
173{
174 *out_idx = i;
175 return true;
176}
177 }
178return false;
179 }
thus returning false to the callsite at line 189, and thus having
diagnostic_path::interprocedural_p return false at line 190:
189if (!get_first_event_in_a_function (&first_fn_event_idx))
190 return false;
So I'm not sure what's happening here (or maybe I'm missing something silly?)
[Bug analyzer/115647] No warning when a loop is infinite due to type of operand in conditional
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115647 --- Comment #3 from David Malcolm --- (In reply to David Malcolm from comment #0) > In the below `for` statement, the `char` counter `i` has a smaller range > than the value of the `int` condition `n` (in this case 500) hence it will > overflow and wrap around when incremented beyond its max range value (in > this case 127) before reaching that value, resulting in an infinite loop. Note '127' assumes signed char by default, whereas on some targets the max range value is 255. But when comparing against 500 it doesn't affect the logic.
[Bug analyzer/115647] No warning when a loop is infinite due to type of operand in conditional
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115647
--- Comment #2 from David Malcolm ---
Without optimization
https://godbolt.org/z/15v7zMMo5
the analyzer "sees" this gimple IR:
int main ()
{
const int n;
char i;
int D.3184;
int _1;
unsigned char i.0_2;
unsigned char _3;
int _4;
int _10;
:
n_7 = 500;
i_8 = 0;
goto ; [INV]
:
_1 = (int) i_5;
printf ("%d\n", _1);
i.0_2 = (unsigned char) i_5;
_3 = i.0_2 + 1;
i_12 = (char) _3;
:
# i_5 = PHI
_4 = (int) i_5;
if (n_7 > _4)
goto ; [INV]
else
goto ; [INV]
:
_10 = 0;
:
:
return _10;
}
The analyzer could probably figure out from:
char i;
[...snip...]
n_7 = 500;
[...snip...]
_4 = (int) i_5;
if (n_7 > _4)
that the conditional is:
if (500 > (int)(char)X)
and thus always true.
But maybe this would be better placed elsewhere?
[Bug analyzer/115647] No warning when a loop is infinite due to type of operand in conditional
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115647
--- Comment #1 from David Malcolm ---
There's no warning from the analyzer below -O1:
https://godbolt.org/z/34js3xT8G
but there is at -O1 and above:
https://godbolt.org/z/5z9x1vP6G
which reports:
: In function 'main':
:10:5: warning: infinite loop [CWE-835] [-Wanalyzer-infinite-loop]
10 | printf ("%d\n", i);
| ^~
'main': event 1
10 | printf ("%d\n", i);
| ^~
| |
| (1) ⚠️ infinite loop here
'main': event 2
cc1:
(2): looping back...
'main': event 3
10 | printf ("%d\n", i);
| ^~
| |
|└───>(3) ...to here
This was surprising to me, as -Wanalyzer is meant to not report on infinite
loops with side-effects, and printf ought to be a side-effect.
That said, we probably ought to warn for the conditional 'i < n' given that it
will always be true given the value of n and the type of i. Such a warning
might be better implemented in the frontends; not sure.
[Bug analyzer/115647] New: No warning when a loop is infinite due to type of operand in conditional
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115647
Bug ID: 115647
Summary: No warning when a loop is infinite due to type of
operand in conditional
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: analyzer
Assignee: dmalcolm at gcc dot gnu.org
Reporter: dmalcolm at gcc dot gnu.org
Target Milestone: ---
A user sent in this report, noting that it might not be in scope for the
analyzer:
SUBJECT:
Wanalyzer-infinite-loop: Add warning for counter unable to ever not meet
condition in `for` loop
COMMENT:
In the below `for` statement, the `char` counter `i` has a smaller range
than the value of the `int` condition `n` (in this case 500) hence it will
overflow and wrap around when incremented beyond its max range value (in
this case 127) before reaching that value, resulting in an infinite loop.
#include
int
main (void)
{
char i;
const int n = 500;
for (i = 0; i < n; ++i)
printf ("%d\n", i);
return 0;
}
Using -Wall, -pedantic, -Wconversion and -fanalyzer on gcc 14.1 doesn't
warn about anything for this infinite loop. Maybe this is something
-Wanalyzer-infinite-loop should warn about.
[Bug analyzer/115564] -Wanalyzer-infinite-loop false negative when there are no source locations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115564 David Malcolm changed: What|Removed |Added URL||https://gcc.gnu.org/piperma ||il/gcc-patches/2024-June/65 ||5424.html Status|ASSIGNED|WAITING Keywords||patch --- Comment #2 from David Malcolm --- I've posted a patch for review here: https://gcc.gnu.org/pipermail/gcc-patches/2024-June/655424.html
[Bug testsuite/109360] RFE: check that generated .sarif files validate against the SARIF schema
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109360 David Malcolm changed: What|Removed |Added Last reconfirmed||2024-06-21 Status|UNCONFIRMED |ASSIGNED Ever confirmed|0 |1 --- Comment #3 from David Malcolm --- Should be fixed on trunk for GCC 15 by the above patche. Keeping open to track backporting to GCC 14 and GCC 13.
[Bug other/112425] Invalid SARIF output when column number is zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112425 David Malcolm changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=109360 --- Comment #1 from David Malcolm --- Should be fixed by commit r15-1540-g9f4fdc3acebcf6b045edea1361570658da4bc0ab.
[Bug analyzer/115564] -Wanalyzer-infinite-loop false negative when there are no source locations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115564 David Malcolm changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2024-06-20 Ever confirmed|0 |1 --- Comment #1 from David Malcolm --- Am testing a fix
