Re: gcc-patches From rewriting mailman settings

[Mark Wielaard]

Hi,

On Sun, Sep 17, 2023 at 10:04:37PM +0200, Mark Wielaard wrote:
> On Tue, Sep 12, 2023 at 05:00:07PM +0200, Mark Wielaard wrote:
> > We (Jeff or anyone else with mailman admin privs) could use the same
> > settings for gcc-patches. The settings that need to be set are in that
> > bug:
> > 
> > - subject_prefix (general): (empty)
> > - from_is_list (general): No
> > - anonymous_list (general): No
> > - first_strip_reply_to (general): No
> > - reply_goes_to_list (general): Poster
> > - reply_to_address (general): (empty)
> > - include_sender_header (general): No
> > - drop_cc (general): No
> > - msg_header (nondigest): (empty)
> > - msg_footer (nondigest): (empty)
> > - scrub_nondigest (nondigest): No
> > - dmarc_moderation_action (privacy): Accept
> > - filter_content (contentfilter): No
> > 
> > The only visible change (apart from no more From rewriting) is that
> > HTML multi-parts aren't scrubbed anymore (that would be a message
> > altering issue). The html part is still scrubbed from the
> > inbox.sourceware.org archive, so b4 works just fine. But I don't know
> > what patchwork.sourceware.org does with HTML attachements. Of course
> > people really shouldn't sent HTML attachments to gcc-patches, so maybe
> > this is no real problem.
> 
> Although there were some positive responses (on list and on irc) it is
> sometimes hard to know if there really is consensus for these kind of
> infrastructure tweaks. But I believe there is at least no sustained
> opposition to changing the gcc-patches mailman setting as proposed
> above.
> 
> So unless someone objects I like to make this change Tuesday September
> 19 around 08:00 UTC.
> 
> And if there are no complaints at Cauldron we could do the same for
> the other patch lists the week after.

Since there were no complaints (and some happy users) and we didn't
real issues (there was an issue when using the Errors-To header, which
might break your dkim signature, but the only user of Errors-To has
dropped it) the jit, libstdc++, fortran and gcc-rust lists now also
have the above mailman settings.

The admin email address for fortran was updated to Toon's new address
and the one for libstdc++ was changed from bkoz to jwakely.

If there are any issues with the list settings please discuss on the
overse...@sourceware.org mailinglist.

Cheers,

Mark

> > > [1] https://patchwork.sourceware.org/project/gcc/list/
> > > [2] https://sourceware.org/bugzilla/show_bug.cgi?id=29713

Re: gcc-patches From rewriting mailman settings (Was: [Linaro-TCWG-CI] gcc patch #75674: FAIL: 68 regressions)

[Mark Wielaard]

Hi Gerald,

On Wed, Oct 04, 2023 at 12:17:48AM +0200, Gerald Pfeifer wrote:
> On Tue, 19 Sep 2023, Mark Wielaard wrote:
> >> Although there were some positive responses (on list and on irc) it is
> >> sometimes hard to know if there really is consensus for these kind of
> >> infrastructure tweaks. But I believe there is at least no sustained
> >> opposition to changing the gcc-patches mailman setting as proposed
> >> above.
> > This change is now done for gcc-patches.
> 
> Yeah, yeah, yeah. Thank you!

Thanks to the fsf-tech team who explained the setup they are using for
lists.gnu.org and everybody helping to test in the bug:
https://sourceware.org/bugzilla/show_bug.cgi?id=29713

It seems to work well, but it does mean disabling most of the things
mailman can do, like filtering out HTML attachements, adjusting
headers, adding footers or subject prefixes, etc. And we did break at
least one workflow for people who were using DKIM signing and the
Errors-To header.

> >> And if there are no complaints at Cauldron we could do the same for
> >> the other patch lists the week after.
> 
> Sadly I missed Cauldron - have there been any complaints there?

No, the opposite. People seemed happy and there were some examples
shown where (on other lists, like binutils) From rewriting caused
issues for some tools relying on patchwork.sourceware.org.

> Can you adjust the g...@gcc.gnu.org list and others @gcc.gnu.org as well?
> I for one would love to see that.

Being somewhat conservative doing that in steps. Last week we switched
the other gcc lists that receive patches (gcc-rust, libstdc++, fortran
and jit). This week looking at non-gcc lists on sourceware that use
patchwork (newlib, binutils, elfutils-devel, gdb-patches and
libabigail). Then if nothing breaks horribly more general lists if
people want.

Cheers,

Mark

[PATCH] PR debug/86459 - Fix -gsplit-dwarf -g3 gcc_assert

[Mark Wielaard]

There was a typo in the output_macinfo_op gcc_assert.
The function is called dwarf_FORM, not dwarf_form.
Add the provided testcase from the bug to test -gsplit-dwarf -g3.

gcc/ChangeLog:

PR debug/86459
* dwarf2out.c (output_macinfo_op): Fix dwarf_FORM typo in gcc_assert.

gcc/testsuite/ChangeLog:

PR debug/86459
* gcc.dg/pr86459.c: New test.
---
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 9523217..4640a22 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -28066,7 +28066,7 @@ output_macinfo_op (macinfo_entry *ref)
   node = find_AT_string (ref->info);
   gcc_assert (node
  && (node->form == DW_FORM_strp
- || node->form == dwarf_form (DW_FORM_strx)));
+ || node->form == dwarf_FORM (DW_FORM_strx)));
   dw2_asm_output_data (1, ref->code,
   ref->code == DW_MACRO_define_strp
   ? "Define macro strp"
diff --git a/gcc/testsuite/gcc.dg/pr86459.c b/gcc/testsuite/gcc.dg/pr86459.c
new file mode 100644
index 000..7856a37
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr86459.c
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-g -O2 -fno-var-tracking-assignments -gsplit-dwarf -g3" } */
+
+/* Same as pr86064.c but compiled with -g3 it showed an issue in
+   output_macinfo_op because of a typo in an assert.  */
+
+int a;
+__attribute__((__cold__)) void b();
+
+void e(int *);
+int f();
+
+void c() {
+  int d;
+  e(&d);
+  a = d;
+  if (f())
+b();
+}
-- 
1.8.3.1

Re: [PATCH] RISC-V: Fix compiler warning of riscv_arg_has_vector

[Mark Wielaard]

Hi all,

On Tue, 2023-06-20 at 07:11 -0600, Jeff Law wrote:
> On 6/20/23 04:56, Robin Dapp wrote:
> > > Could you merge it ?
> > > By the way, could Lehua get the write access?
> > 
> > IMHO nothing stands in the way but I'll defer to Jeff to have
> > the "official seal" :)
> > Once he ACKs Lehua needs to go the usual way of requesting
> > sourceware access via https://sourceware.org/cgi-bin/pdw/ps_form.cgi.
> Lehua fills out that form.  List me as the approver and the process will 
> run from there.  Takes a day or two for everything to get into place.

All done. Welcome Lehua.

> ps.  If Lehua has already filled out the form with Robin as the > approver, 
>that's fine too.  Might take a bit longer as I suspect the
> IT folks may not recognize Robin. 

Also Robin is right, you are on the hook as approver for the "official
seal" :) Because the "IT folks" check that the approver is listed as a
gcc maintainer and not just has write after approval status.

Cheers,

Mark

Re: Default to DWARF5

[Mark Wielaard]

Hi Paul,

On Thu, Jan 28, 2021 at 08:25:20PM -0600, Paul A. Clarke wrote:
> The subject commit, 3804e937b0e252a7e42632fe6d9f898f1851a49c, causes a
> failure in the test suite for the IBM Advance Toolchain.  The test in
> question uses "perf probe" to set a tracepoint at "main" in a newly built
> (with GCC 11) binary of "/bin/ld".  With the patch applied, the command
> enters an infinte loop, calling libdw1 functions but making no progress.
> 
> The infinite loop can be found in the Linux kernel
> tools/perf/utils/probe-finder.c:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/util/probe-finder.c?h=v5.11-rc5#n1190
> 

The infinite loop is because off isn't updated to noff before calling
continue in the while loop. Moving the last line of the loop off =
noff; to the start of the loop should fix that.

Why the call to dwarf_offdie fails is unclear. Try printing the error
using dwarf_errmsg (-1) when it returns NULL. If you also print out
off (in hex) and the filename the dbg was created from you can inspect
the file using [eu-]readelf --debug-dump=info to see how the DIEs look
at that offset.

BTW. Make sure you have elfutils libdw version 0.172 or newer when
dealing with DWARF5. Latest elfutils release is 0.182.

Cheers,

Mark

Re: Default to DWARF5

[Mark Wielaard]

Hi Paul,

On Fri, Jan 29, 2021 at 02:04:02PM -0600, Paul A. Clarke wrote:
> On Fri, Jan 29, 2021 at 12:22:41PM +0100, Mark Wielaard wrote:
> > The infinite loop is because off isn't updated to noff before calling
> > continue in the while loop. Moving the last line of the loop off =
> > noff; to the start of the loop should fix that.
> 
> I noticed that, too.  I'll report that over on linux-perf-users.
> (That code is about 10 years old, surprisingly.)

Thanks. Note that newer elfutils libdw (since support for DWARF5 was
added) provide dwarf_get_units which makes iterating over CUs slightly
simpler because it gives you the CU and DIE directly instead of having
to deal with offsets.

> > Why the call to dwarf_offdie fails is unclear. Try printing the error
> > using dwarf_errmsg (-1) when it returns NULL.
> 
> The message is not terribly helpful, to me anyway:
> invalid DWARF

Yeah, sorry, that is because DWARF5 looks invalid to old versions.

> > If you also print out
> > off (in hex) and the filename the dbg was created from you can inspect
> > the file using [eu-]readelf --debug-dump=info to see how the DIEs look
> > at that offset.
> 
> I'm not a DWARF/DIE expert at this point to know what to look for.
> --
> Contents of the .debug_info section:
> 
>   Compilation Unit @ offset 0x0:
>Length:0x24 (32-bit)
>Version:   5
>Abbrev Offset: 0x0
>Pointer Size:  8
>  <0>: Abbrev Number: 1 (DW_TAG_compile_unit)
>DW_AT_stmt_list   : 0x0
> <11>   DW_AT_low_pc  : 0x100089d4
> <19>   DW_AT_high_pc : 68
> <1a>   DW_AT_name: (indirect string, offset: 0x0): start.S
> <1e>   DW_AT_comp_dir: (indirect string, offset: 0x8): 
> /path/to/AT.../sources/glibc/csu
> <22>   DW_AT_producer: (indirect string, offset: 0x5c): GNU AS 2.36.50
> <26>   DW_AT_language: 32769(MIPS assembler)
> ...
> 
> Is there an incompatibility introduced with DWARF5?

Yes, the unit header was extended in DWARF5, so the offset given
to the first DIE by the old code is invalid.

> > BTW. Make sure you have elfutils libdw version 0.172 or newer when
> > dealing with DWARF5. Latest elfutils release is 0.182.
> 
> SLES15SP2 has an older version:
> $ rpm -qi libdw1
> Name: libdw1
> Version : 0.168
> Release : 4.5.3

Yes, unfortunately 0.168 is too old. It was released in 2016 before
the DWARF 5 specification (which is from 2017).

Cheers,

Mark

[PATCH] Document the GCC11 change to DWARF5 default.

[Mark Wielaard]

* gcc-11/changes.html (General Improvements): Add a section on
the DWARF version 5 default.
---
 htdocs/gcc-11/changes.html | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/htdocs/gcc-11/changes.html b/htdocs/gcc-11/changes.html
index de75b8d6..04c3c449 100644
--- a/htdocs/gcc-11/changes.html
+++ b/htdocs/gcc-11/changes.html
@@ -139,6 +139,36 @@ a work-in-progress.
   -fsanitize=kernel-hwaddress to instrument kernel 
code.
 
   
+  
+
+  For targets that produce DWARF debugging information GCC now
+  defaults to http://dwarfstd.org/doc/DWARF5.pdf";>DWARF
+  version 5 (with the exception of VxWorks and Darwin/Mac OS X
+  which default to version 2 and AIX which defaults to version 4).
+  This can produce up to 25% more compact debug information
+  compared to earlier versions.
+
+  To take full advantage of DWARF version 5 GCC needs to be build
+  against binutils version 2.35.2 or higher.  When GCC is build
+  against earlier versions of binutils GCC will still emit DWARF
+  version 5 for most debuginfo data, but will generate version 4
+  debug line tables (even when explicitly given -gdwarf-5).
+
+  The following debug information consumers can process DWARF version 5:
+  
+   GDB 8.0, or higher
+   valgrind 3.17.0
+   elfutils 0.172, or higher (for use with systemtap,
+ dwarves/pahole, perf and libabigail).
+  
+
+  Programs embedding libbacktrace are urged to upgrade to the version
+  shipping with GCC 11.
+
+  To make GCC 11 generate an older DWARF version
+  use -g together with -gdwarf-2,
+  -gdwarf-3 or -gdwarf-4.
+  
 
 
 
-- 
2.18.4

Re: [PATCH] Document the GCC11 change to DWARF5 default.

[Mark Wielaard]

On Fri, 2021-02-19 at 18:04 +0100, Mark Wielaard wrote:
>   * gcc-11/changes.html (General Improvements): Add a section on
>   the DWARF version 5 default.

Ping. OK to commit?

> diff --git a/htdocs/gcc-11/changes.html b/htdocs/gcc-11/changes.html
> index 6a47b0b8..9734eee8 100644
> --- a/htdocs/gcc-11/changes.html
> +++ b/htdocs/gcc-11/changes.html
> @@ -139,6 +139,37 @@ a work-in-progress.
>-fsanitize=kernel-hwaddress to instrument kernel 
> code.
>  
>
> +  
> +
> +  For targets that produce DWARF debugging information GCC now
> +  defaults to http://dwarfstd.org/doc/DWARF5.pdf";>DWARF
> +  version 5 (with the exception of VxWorks and Darwin/Mac OS X
> +  which default to version 2 and AIX which defaults to version 4).
> +  This can produce up to 25% more compact debug information
> +  compared to earlier versions.
> +
> +  To take full advantage of DWARF version 5 GCC needs to be build
> +  against binutils version 2.35.2 or higher.  When GCC is build
> +  against earlier versions of binutils GCC will still emit DWARF
> +  version 5 for most debuginfo data, but will generate version 4
> +  debug line tables (even when explicitly given -gdwarf-5).
> +
> +  The following debug information consumers can process DWARF version 5:
> +  
> +   GDB 8.0, or higher
> +   valgrind 3.17.0
> +   elfutils 0.172, or higher (for use with systemtap,
> + dwarves/pahole, perf and libabigail)
> +   dwz 0.14
> +  
> +
> +  Programs embedding libbacktrace are urged to upgrade to the version
> +  shipping with GCC 11.
> +
> +  To make GCC 11 generate an older DWARF version
> +  use -g together with -gdwarf-2,
> +  -gdwarf-3 or -gdwarf-4.
> +  
>  
>  
>  
> 

Re: [PATCH] debug: Pass --gdwarf-N to assembler if fixed gas is detected during configure

[Mark Wielaard]

Hi,

On Tue, 2020-09-15 at 20:40 +0200, Jakub Jelinek wrote:
> Ok, here it is in patch form.
> I've briefly tested it, with the older binutils I have around (no --gdwarf-N
> support), with latest gas (--gdwarf-N that can be passed to as even when
> compiling C/C++ etc. code and emitting .debug_line) and latest gas with 
> Mark's fix
> reverted (--gdwarf-N support, but can only pass it to as when assembling
> user .s/.S files, not when compiling C/C++ etc.).
> Will bootstrap/regtest (with the older binutils) later tonight.
> 
> 2020-09-15  Jakub Jelinek  
> 
>   * configure.ac (HAVE_AS_GDWARF_5_DEBUG_FLAG,
>   HAVE_AS_WORKING_DWARF_4_FLAG): New tests.
>   * gcc.c (ASM_DEBUG_DWARF_OPTION): Define.
>   (ASM_DEBUG_SPEC): Use ASM_DEBUG_DWARF_OPTION instead of
>   "--gdwarf2".  Use %{cond:opt1;:opt2} style.
>   (ASM_DEBUG_OPTION_DWARF_OPT): Define.
>   (ASM_DEBUG_OPTION_SPEC): Define.
>   (asm_debug_option): New variable.
>   (asm_options): Add "%(asm_debug_option)".
>   (static_specs): Add asm_debug_option entry.
>   (static_spec_functions): Add dwarf-version-gt.
>   (debug_level_greater_than_spec_func): New function.
>   * config/darwin.h (ASM_DEBUG_OPTION_SPEC): Define.
>   * config/darwin9.h (ASM_DEBUG_OPTION_SPEC): Redefine.
>   * config.in: Regenerated.
>   * configure: Regenerated.

I tested against the binutils-2_35-branch which will become 2.35.1 next
week. The configure tests succeed there. So I think you can change the
[elf,2,36,0] to [elf,2,35,1].

Also tested that they fail with an older binutils (2.32).

Cheers,

Mark

Re: [PATCH 3/5] Add DWARF5 variants of assembly scan tests that use DW_FORM_implicit_const

[Mark Wielaard]

On Mon, 2020-08-24 at 22:26 +0200, Mark Wielaard wrote:
> On Mon, Aug 24, 2020 at 07:44:27PM +0200, Jakub Jelinek wrote:
> > On Mon, Aug 24, 2020 at 02:56:56PM +0200, Mark Wielaard wrote:
> > > Some DWARF tests scan the assembly output looking for constant values.
> > > When using DWARF5 those constants might use DW_FORM_implicit_const,
> > > which are output (in the comments) after the attribute instead of
> > > before. To make sure these tests work introduce a -gdwarf-5 variant
> > > of these tests and explicitly use -gdwarf-2 for the original.
> > 
> > I just wonder if we want to use -gdwarf-2 rather than -gdwarf-4 in the
> > original, -gdwarf-5 has been the default for a couple of years and thus
> > that is what those testshave been compiled with.
> 
> I used -gdwarf-2 because I thought that was still the default for some
> arches/platforms. And they pass with -gdwarf-2.
> 
> > Also not sure about the -dwarf5 suffixes, couldn't we say just use
> > pr41445-{7,8}.c, inline-var-2.C or inline3.c (or whatever next number
> > with the same prefix is still unused)?
> 
> Sure, if that is a better naming scheme I'll rename them.

Here is the adjusted patch.

gcc/testsuite/ChangeLog:

* gcc.dg/debug/dwarf2/inline2.c: Add -gdwarf-2.
* g++.dg/debug/dwarf2/inline-var-1.C: Likewise.
* gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c: Likewise.
* gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c: Likewise.
* gcc.dg/debug/dwarf2/inline6.c: New variant with -gdwarf-5.
* g++.dg/debug/dwarf2/inline-var-3.C: Likewise.
* gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-7.c: Likewise.
* gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-8.c: Likewise.

OK to commit?

From 99e4e5bca0d55971c515b98188ec6f206517267d Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Sun, 23 Aug 2020 16:21:08 +0200
Subject: [PATCH] Add DWARF5 variants of assembly scan tests that use
 DW_FORM_implicit_const

Some DWARF tests scan the assembly output looking for constant values.
When using DWARF5 those constants might use DW_FORM_implicit_const,
which are output (in the comments) after the attribute instead of
before. To make sure these tests work introduce a -gdwarf-5 variant
of these tests and explicitly use -gdwarf-2 for the original.

gcc/testsuite/ChangeLog:

	* gcc.dg/debug/dwarf2/inline2.c: Add -gdwarf-2.
	* g++.dg/debug/dwarf2/inline-var-1.C: Likewise.
	* gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c: Likewise.
	* gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c: Likewise.
	* gcc.dg/debug/dwarf2/inline6.c: New variant with -gdwarf-5.
	* g++.dg/debug/dwarf2/inline-var-3.C: Likewise.
	* gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-7.c: Likewise.
	* gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-8.c: Likewise.
---
 gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C  | 2 +-
 .../debug/dwarf2/{inline-var-1.C => inline-var-3.C}   | 6 --
 gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c   | 4 +++-
 .../gcc.dg/debug/dwarf2/{inline2.c => inline6.c}  | 7 ---
 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c | 2 +-
 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c | 2 +-
 .../gcc.dg/debug/dwarf2/{pr41445-5.c => pr41445-7.c}  | 8 
 .../gcc.dg/debug/dwarf2/{pr41445-6.c => pr41445-8.c}  | 8 
 8 files changed, 22 insertions(+), 17 deletions(-)
 copy gcc/testsuite/g++.dg/debug/dwarf2/{inline-var-1.C => inline-var-3.C} (76%)
 copy gcc/testsuite/gcc.dg/debug/dwarf2/{inline2.c => inline6.c} (88%)
 copy gcc/testsuite/gcc.dg/debug/dwarf2/{pr41445-5.c => pr41445-7.c} (73%)
 copy gcc/testsuite/gcc.dg/debug/dwarf2/{pr41445-6.c => pr41445-8.c} (68%)

diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
index 3b1c913edfca..9a88e28cbe0f 100644
--- a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
@@ -1,5 +1,5 @@
 // { dg-do compile { target c++17 } }
-// { dg-options "-O -g -dA -gno-strict-dwarf -fno-eliminate-unused-debug-symbols" }
+// { dg-options "-O -gdwarf-2 -dA -gno-strict-dwarf -fno-eliminate-unused-debug-symbols" }
 // { dg-require-weak "" }
 // { dg-final { scan-assembler-times "0x3\[^\n\r]* DW_AT_inline" 6 { xfail *-*-aix* } } }
 // { dg-final { scan-assembler-times "0x1\[^\n\r]* DW_AT_inline" 2 { xfail *-*-aix* } } }
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-3.C
similarity index 76%
copy from gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
copy to gcc/testsuite/g++.dg/debug/dwarf2/inline-var-3.C
index 3b1c913edfca..52ed5b6912fd 100644
--- a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-3.C
@@ -1,7 +1,9 @@
+// DWARF5 vari

Re: [PATCH] debug: Pass --gdwarf-N to assembler if fixed gas is detected during configure

[Mark Wielaard]

Hi,

On Tue, 2020-09-15 at 20:40 +0200, Jakub Jelinek wrote:
> Ok, here it is in patch form.
> I've briefly tested it, with the older binutils I have around (no --gdwarf-N
> support), with latest gas (--gdwarf-N that can be passed to as even when
> compiling C/C++ etc. code and emitting .debug_line) and latest gas with 
> Mark's fix
> reverted (--gdwarf-N support, but can only pass it to as when assembling
> user .s/.S files, not when compiling C/C++ etc.).
> Will bootstrap/regtest (with the older binutils) later tonight.
> 
> 2020-09-15  Jakub Jelinek  
> 
>   * configure.ac (HAVE_AS_GDWARF_5_DEBUG_FLAG,
>   HAVE_AS_WORKING_DWARF_4_FLAG): New tests.
>   * gcc.c (ASM_DEBUG_DWARF_OPTION): Define.
>   (ASM_DEBUG_SPEC): Use ASM_DEBUG_DWARF_OPTION instead of
>   "--gdwarf2".  Use %{cond:opt1;:opt2} style.
>   (ASM_DEBUG_OPTION_DWARF_OPT): Define.
>   (ASM_DEBUG_OPTION_SPEC): Define.
>   (asm_debug_option): New variable.
>   (asm_options): Add "%(asm_debug_option)".
>   (static_specs): Add asm_debug_option entry.
>   (static_spec_functions): Add dwarf-version-gt.
>   (debug_level_greater_than_spec_func): New function.
>   * config/darwin.h (ASM_DEBUG_OPTION_SPEC): Define.
>   * config/darwin9.h (ASM_DEBUG_OPTION_SPEC): Redefine.
>   * config.in: Regenerated.
>   * configure: Regenerated.

Once this is in we can more generally emit DW_FORM_line_str for
filepaths in CU DIEs for the name and comp_dir attribute. There
currently is a bit of a hack to do this in dwarf2out_early_finish, but
that only works when the assembler doesn't emit a DWARF5 .debug_line,
but gcc does it itself.

What do you think of the attached patch?
From c31667db57de62c3107a0b2a5e30fbd57a4708a3 Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Fri, 18 Sep 2020 17:07:03 +0200
Subject: [PATCH] Output filepath strings in .debug_line_str for DWARF5

DWARF5 has a new string table specially for file paths. .debug_line
file and dir tables reference strings in .debug_line_str.  If a
.debug_line_str section is emitted then also place CU DIE file
names and comp dirs there.

gcc/ChangeLog:

	* dwarf2out.c (add_filepath_AT_string): New function.
	(asm_outputs_debug_line_str): Likewise.
	(add_filename_attribute): Likewise.
	(add_comp_dir_attribute): Call add_filepath_AT_string.
	(gen_compile_unit_die): Call add_filename_attribute for name.
	(init_sections_and_labels): Init debug_line_str_section when
	asm_outputs_debug_line_str return true.
	(dwarf2out_early_finish): Remove DW_AT_name and DW_AT_comp_dir
	hack and call add_filename_attribute for the remap_debug_filename.
---
 gcc/dwarf2out.c | 96 -
 1 file changed, 64 insertions(+), 32 deletions(-)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 4096c0c0d69f..a43082864a75 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3347,6 +3347,8 @@ output_asm_line_debug_info (void)
 	  || !debug_variable_location_views));
 }
 
+static bool asm_outputs_debug_line_str (void);
+
 /* Minimum line offset in a special line info. opcode.
This value was chosen to give a reasonable range of values.  */
 #define DWARF_LINE_BASE  -10
@@ -4731,6 +4733,33 @@ reset_indirect_string (indirect_string_node **h, void *)
   return 1;
 }
 
+static inline void
+add_filepath_AT_string (dw_die_ref die, enum dwarf_attribute attr_kind,
+			const char *str)
+{
+  if (! asm_outputs_debug_line_str ())
+add_AT_string (die, attr_kind, str);
+  else
+{
+  dw_attr_node attr;
+  struct indirect_string_node *node;
+
+  if (!debug_line_str_hash)
+	debug_line_str_hash
+	  = hash_table::create_ggc (10);
+
+  node = find_AT_string_in_table (str, debug_line_str_hash);
+  set_indirect_string (node);
+  node->form = DW_FORM_line_strp;
+
+  attr.dw_attr = attr_kind;
+  attr.dw_attr_val.val_class = dw_val_class_str;
+  attr.dw_attr_val.val_entry = NULL;
+  attr.dw_attr_val.v.val_str = node;
+  add_dwarf_attr (die, &attr);
+}
+}
+
 /* Find out whether a string should be output inline in DIE
or out-of-line in .debug_str section.  */
 
@@ -11839,6 +11868,29 @@ output_ranges (void)
   for -gsplit-dwarf we should use DW_FORM_strx instead.  */	\
&& !dwarf_split_debug_info)
 
+
+/* Returns TRUE if we are outputting DWARF5 and the assembler supports
+   DWARF5 .debug_line tables using .debug_line_str or we generate
+   it ourselves, except for split-dwarf which doesn't have a
+   .debug_line_str.  */
+static bool
+asm_outputs_debug_line_str (void)
+{
+  if (dwarf_version >= 5
+  && ! output_asm_line_debug_info ()
+  && DWARF5_USE_DEBUG_LINE_STR)
+return true;
+  else
+{
+#if defined(HAVE_AS_GDWARF_5_DEBUG_FLAG) && defined(HAVE_AS_WORKING_DWARF_4_FLAG)
+  return !dwarf_spl

[PATCH] libiberty: Add get_DW_UT_name and update include/dwarf2.{def, h}

[Mark Wielaard]

This adds a get_DW_UT_name function to dwarfnames using dwarf2.def
for use in binutils readelf to show the unit types in a DWARF5 header.

Also remove DW_CIE_VERSION which was already removed in binutils/gdb
and is not used in gcc.

include/ChangeLog:

* dwarf2.def: Add DWARF5 Unit type header encoding macros
DW_UT_FIRST, DW_UT and DW_UT_END.
* dwarf2.h (enum dwarf_unit_type): Removed and define using
DW_UT_FIRST, DW_UT and DW_UT_END macros.
(DW_CIE_VERSION): Removed.
(get_DW_UT_name): New function declaration.

libiberty/ChangeLog:

* dwarfnames.c (get_DW_UT_name): Define using DW_UT_FIRST, DW_UT
and DW_UT_END.
---
 include/dwarf2.def | 11 +++
 include/dwarf2.h   | 25 +++--
 libiberty/dwarfnames.c |  7 +++
 3 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/include/dwarf2.def b/include/dwarf2.def
index d8a8cce79478..13825a3eef7e 100644
--- a/include/dwarf2.def
+++ b/include/dwarf2.def
@@ -805,3 +805,14 @@ DW_IDX (DW_IDX_hi_user, 0x3fff)
 DW_IDX (DW_IDX_GNU_internal, 0x2000)
 DW_IDX (DW_IDX_GNU_external, 0x2001)
 DW_END_IDX
+
+/* DWARF5 Unit type header encodings  */
+DW_FIRST_UT (DW_UT_compile, 0x01)
+DW_UT (DW_UT_type, 0x02)
+DW_UT (DW_UT_partial, 0x03)
+DW_UT (DW_UT_skeleton, 0x04)
+DW_UT (DW_UT_split_compile, 0x05)
+DW_UT (DW_UT_split_type, 0x06)
+DW_UT (DW_UT_lo_user, 0x80)
+DW_UT (DW_UT_hi_user, 0xff)
+DW_END_UT
diff --git a/include/dwarf2.h b/include/dwarf2.h
index 34273e84a5aa..83cf50d7bf53 100644
--- a/include/dwarf2.h
+++ b/include/dwarf2.h
@@ -55,6 +55,7 @@
 #define DW_CFA_DUP(name, value) , name = value
 #define DW_IDX(name, value) , name = value
 #define DW_IDX_DUP(name, value) , name = value
+#define DW_UT(name, value) , name = value
 
 #define DW_FIRST_TAG(name, value) enum dwarf_tag { \
   name = value
@@ -77,6 +78,9 @@
 #define DW_FIRST_IDX(name, value) enum dwarf_name_index_attribute { \
   name = value
 #define DW_END_IDX };
+#define DW_FIRST_UT(name, value) enum dwarf_unit_type { \
+  name = value
+#define DW_END_UT };
 
 #include "dwarf2.def"
 
@@ -94,6 +98,8 @@
 #undef DW_END_CFA
 #undef DW_FIRST_IDX
 #undef DW_END_IDX
+#undef DW_FIRST_UT
+#undef DW_END_UT
 
 #undef DW_TAG
 #undef DW_TAG_DUP
@@ -108,6 +114,7 @@
 #undef DW_CFA_DUP
 #undef DW_IDX
 #undef DW_IDX_DUP
+#undef DW_UT
 
 /* Flag that tells whether entry has a child or not.  */
 #define DW_children_no   0
@@ -316,7 +323,6 @@ enum dwarf_location_list_entry_type
 
 #define DW_CIE_ID0x
 #define DW64_CIE_ID  0xULL
-#define DW_CIE_VERSION   1
 
 #define DW_CFA_extended   0
 
@@ -451,19 +457,6 @@ enum dwarf_range_list_entry
 DW_RLE_start_end = 0x06,
 DW_RLE_start_length = 0x07
   };
-
-/* Unit types in unit_type unit header field.  */
-enum dwarf_unit_type
-  {
-DW_UT_compile = 0x01,
-DW_UT_type = 0x02,
-DW_UT_partial = 0x03,
-DW_UT_skeleton = 0x04,
-DW_UT_split_compile = 0x05,
-DW_UT_split_type = 0x06,
-DW_UT_lo_user = 0x80,
-DW_UT_hi_user = 0xff
-  };
 
 /* @@@ For use with GNU frame unwind information.  */
 
@@ -552,6 +545,10 @@ extern const char *get_DW_CFA_name (unsigned int opc);
recognized.  */
 extern const char *get_DW_IDX_name (unsigned int idx);
 
+/* Return the name of a DW_UT_ constant, or NULL if the value is not
+   recognized.  */
+extern const char *get_DW_UT_name (unsigned int ut);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
diff --git a/libiberty/dwarfnames.c b/libiberty/dwarfnames.c
index 968d19175328..af11668b431f 100644
--- a/libiberty/dwarfnames.c
+++ b/libiberty/dwarfnames.c
@@ -64,6 +64,11 @@ Boston, MA 02110-1301, USA.  */
   switch (idx) {   \
   DW_IDX (name, value)
 #define DW_END_IDX } return 0; }
+#define DW_FIRST_UT(name, value) \
+  const char *get_DW_UT_name (unsigned int ut) {   \
+  switch (ut) {\
+  DW_UT (name, value)
+#define DW_END_UT } return 0; }
 
 #define DW_TAG(name, value) case name: return # name ;
 #define DW_TAG_DUP(name, value)
@@ -78,6 +83,7 @@ Boston, MA 02110-1301, USA.  */
 #define DW_CFA_DUP(name, value)
 #define DW_IDX(name, value) case name: return # name ;
 #define DW_IDX_DUP(name, value)
+#define DW_UT(name, value) case name: return # name ;
 
 #include "dwarf2.def"
 
@@ -95,6 +101,7 @@ Boston, MA 02110-1301, USA.  */
 #undef DW_END_CFA
 #undef DW_FIRST_IDX
 #undef DW_END_IDX
+#undef DW_END_UT
 
 #undef DW_TAG
 #undef DW_TAG_DUP
-- 
2.18.4

Re: [PATCH 1/5] Don't enable -gvariable-location-views by default for DWARF5.

[Mark Wielaard]

Hi,

On Mon, 2020-08-24 at 19:38 +0200, Jakub Jelinek wrote:
> On Mon, Aug 24, 2020 at 02:56:54PM +0200, Mark Wielaard wrote:
> > DWARF5 makes it possible to read loclists tables without consulting
> > the debuginfo tree by introducing a table header. Adding location
> > views
> > breaks this (at least for binutils and elfutils). So don't enable
> > variable-location-views by default if DWARF5 or higher is selected.
> 
> This should be discussed with Alex, CCed.
> I'd say elfutils/binutils should just show .debug_loclists
> independent of
> .debug_info if there are no loc views and use .debug_info otherwise.

So it turned out that it was really bugs in elfutils and binutils.
For elfutils it now tracks locviews in .debug_loclists just like it did
for .debug_loc:
https://sourceware.org/pipermail/elfutils-devel/2020q3/002900.html

For binutils it actually tracked locviews correctly, but didn't handle
DW_LLE_start_end and DW_LLE_start_length. Patch submitted:
https://sourceware.org/pipermail/binutils/2020-September/113510.html

For tools that access the location lists (and locviews) through the DIE
attributes this never was an issue.

Patch retracted.

Cheers,

Mark

Re: BoF DWARF5 patches (25% .debug section size reduction)

[Mark Wielaard]

On Thu, 2020-09-10 at 13:16 +0200, Jakub Jelinek wrote:
> On Wed, Sep 09, 2020 at 09:57:54PM +0200, Mark Wielaard wrote:
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -9057,13 +9057,14 @@ possible.
> >  @opindex gdwarf
> >  Produce debugging information in DWARF format (if that is supported).
> >  The value of @var{version} may be either 2, 3, 4 or 5; the default version
> > -for most targets is 4.  DWARF Version 5 is only experimental.
> > +for most targets is 5 (with the exception of vxworks and darwin which
> > +default to version 2).
> 
> I think in documentation we should spell these VxWorks and Darwin/Mac OS X

OK. As attached.

Are we ready to flip the default to 5?

Thanks,

Mark
From 409bd1b2c60905b0f96c94fface12154d3be4d32 Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Tue, 29 Sep 2020 15:52:44 +0200
Subject: [PATCH] Default to DWARF5

gcc/ChangeLog:

	* common.opt (gdwarf-): Init(5).
	* doc/invoke.texi (-gdwarf): Document default to 5.
---
 gcc/common.opt  | 2 +-
 gcc/doc/invoke.texi | 7 ---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index 292c2de694ef..d1722de80bf0 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -3148,7 +3148,7 @@ Common Driver JoinedOrMissing Negative(gdwarf-)
 Generate debug information in default version of DWARF format.
 
 gdwarf-
-Common Driver Joined UInteger Var(dwarf_version) Init(4) Negative(gstabs)
+Common Driver Joined UInteger Var(dwarf_version) Init(5) Negative(gstabs)
 Generate debug information in DWARF v2 (or later) format.
 
 ggdb
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 2091e0cd23b9..e6453374bcd4 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9210,14 +9210,15 @@ possible.
 @itemx -gdwarf-@var{version}
 @opindex gdwarf
 Produce debugging information in DWARF format (if that is supported).
-The value of @var{version} may be either 2, 3, 4 or 5; the default version
-for most targets is 4.  DWARF Version 5 is only experimental.
+The value of @var{version} may be either 2, 3, 4 or 5; the default
+version for most targets is 5 (with the exception of VxWorks and
+Darwin/Mac OS X which default to version 2).
 
 Note that with DWARF Version 2, some ports require and always
 use some non-conflicting DWARF 3 extensions in the unwind tables.
 
 Version 4 may require GDB 7.0 and @option{-fvar-tracking-assignments}
-for maximum benefit.
+for maximum benefit. Version 5 requires GDB 8.0 or higher.
 
 GCC no longer supports DWARF Version 1, which is substantially
 different than Version 2 and later.  For historical reasons, some
-- 
2.18.4

Re: [PATCH] debug: Pass --gdwarf-N to assembler if fixed gas is detected during configure

[Mark Wielaard]

Hi,

On Fri, 2020-09-18 at 17:21 +0200, Mark Wielaard wrote:
> On Tue, 2020-09-15 at 20:40 +0200, Jakub Jelinek wrote:
> > Ok, here it is in patch form.
> > I've briefly tested it, with the older binutils I have around (no --gdwarf-N
> > support), with latest gas (--gdwarf-N that can be passed to as even when
> > compiling C/C++ etc. code and emitting .debug_line) and latest gas with 
> > Mark's fix
> > reverted (--gdwarf-N support, but can only pass it to as when assembling
> > user .s/.S files, not when compiling C/C++ etc.).
> > Will bootstrap/regtest (with the older binutils) later tonight.
> > 
> > 2020-09-15  Jakub Jelinek  
> > 
> > * configure.ac (HAVE_AS_GDWARF_5_DEBUG_FLAG,
> > HAVE_AS_WORKING_DWARF_4_FLAG): New tests.
> > * gcc.c (ASM_DEBUG_DWARF_OPTION): Define.
> > (ASM_DEBUG_SPEC): Use ASM_DEBUG_DWARF_OPTION instead of
> > "--gdwarf2".  Use %{cond:opt1;:opt2} style.
> > (ASM_DEBUG_OPTION_DWARF_OPT): Define.
> > (ASM_DEBUG_OPTION_SPEC): Define.
> > (asm_debug_option): New variable.
> > (asm_options): Add "%(asm_debug_option)".
> > (static_specs): Add asm_debug_option entry.
> > (static_spec_functions): Add dwarf-version-gt.
> > (debug_level_greater_than_spec_func): New function.
> > * config/darwin.h (ASM_DEBUG_OPTION_SPEC): Define.
> > * config/darwin9.h (ASM_DEBUG_OPTION_SPEC): Redefine.
> > * config.in: Regenerated.
> > * configure: Regenerated.
> 
> Once this is in we can more generally emit DW_FORM_line_str for
> filepaths in CU DIEs for the name and comp_dir attribute. There
> currently is a bit of a hack to do this in dwarf2out_early_finish, but
> that only works when the assembler doesn't emit a DWARF5 .debug_line,
> but gcc does it itself.
> 
> What do you think of the attached patch?
>
> DWARF5 has a new string table specially for file paths. .debug_line
> file and dir tables reference strings in .debug_line_str.  If a
> .debug_line_str section is emitted then also place CU DIE file
> names and comp dirs there.
> 
> gcc/ChangeLog:
> 
>   * dwarf2out.c (add_filepath_AT_string): New function.
>   (asm_outputs_debug_line_str): Likewise.
>   (add_filename_attribute): Likewise.
>   (add_comp_dir_attribute): Call add_filepath_AT_string.
>   (gen_compile_unit_die): Call add_filename_attribute for name.
>   (init_sections_and_labels): Init debug_line_str_section when
>   asm_outputs_debug_line_str return true.
>   (dwarf2out_early_finish): Remove DW_AT_name and DW_AT_comp_dir
>   hack and call add_filename_attribute for the remap_debug_filename.

On top of that, we also need the following, which makes sure the actual
compilation directory is used in a DWARF5 .debug_line directory table
(and not just a relative path).


From 66b25bc0a5df06e211b48a54e3b5d33999c24fb6 Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Tue, 6 Oct 2020 17:41:19 +0200
Subject: [PATCH] debug: Make sure to output .file 0 when generating DWARF5.

When gas outputs DWARF5 .debug_line[_str] then we have to tell it the
comp_dir and main file name for the zero entry line table. Otherwise
gas has to guess at the CU compilation directory and file.

Before a gcc -gdwarf-5 ../src/hello.c line table looked like:

Directory table:
 0 ../src (24)
 1 ../src (24)
 2 /usr/include (31)

File name table:
 0 hello.c (16),  0
 1 hello.c (16),  1
 2 stdio.h (44),  2

With this patch it looks like:

Directory table:
 0 /tmp/obj (0)
 1 ../src (24)
 2 /usr/include (31)

File name table:
 0 ../src/hello.c (9),  0
 1 hello.c (16),  1
 2 stdio.h (44),  2

gcc/ChangeLog:

	* dwarf2out.c (dwarf2out_finish): Emit .file 0 entry when
	generating DWARF5 .debug_line table through gas.
---
 gcc/dwarf2out.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index a43082864a75..399937a9f310 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -31764,6 +31764,27 @@ dwarf2out_finish (const char *filename)
   ASM_OUTPUT_LABEL (asm_out_file, debug_line_section_label);
   if (! output_asm_line_debug_info ())
 output_line_info (false);
+  else if (asm_outputs_debug_line_str ())
+{
+  /* When gas outputs DWARF5 .debug_line[_str] then we have to
+	 tell it the comp_dir and main file name for the zero entry
+	 line table.  */
+  const char *comp_dir, *filename0;
+
+  comp_dir = comp_dir_string ();
+  if (comp_dir == NULL)
+	comp_dir = "";
+
+  filename0 = get_AT_string (comp_unit_die (), DW_AT_name);
+  if (filename0 == NULL)
+	filename0 = "";
+
+  fprintf (asm_out_file, "\t.file 0 ");
+  output_quoted_string (asm_out_file, remap_debug_filename (comp_dir));
+  fputc (' ', asm_out_file);
+  output_quoted_string (asm_out_file, remap_debug_filename (filename0));
+  fputc ('\n', asm_out_file);
+}
 
   if (dwarf_split_debug_info && info_section_emitted)
 {
-- 
2.18.4

Re: [PATCH] debug: Pass --gdwarf-N to assembler if fixed gas is detected during configure

[Mark Wielaard]

Hi,

On Tue, 2020-10-06 at 16:57 -0400, Jason Merrill wrote:
> All three of these patches (Jakub's, and your two) look good to me, 
> except that your add_filepath_AT_string patch is missing comments on 
> some of the new functions.

I added documentation for the two new functions missing comments before
pushing.

Thanks,

Mark

Re: [PATCHv3] libiberty rust-demangle, ignore .suffix

[Mark Wielaard]

Ping. Is this OK to commit now?
I am not sure who can approve this.

On Sun, Jan 16, 2022 at 01:35:34AM +0100, Mark Wielaard wrote:
> Rust symbols can have a .suffix because of compiler transformations.
> These can be ignored in the demangled name. Which is what this patch
> implements. By stopping at the first dot for v0 symbols and searching
> backwards to the ending 'E' for legacy symbols.
> 
> An alternative implementation could be to follow what C++ does and
> represent these as [clone .suffix] tagged onto the demangled name.
> But this seems somewhat confusing since it results in a demangled
> name that cannot be mangled again. And it would mean trying to
> decode compiler internal naming.
> 
> https://bugs.kde.org/show_bug.cgi?id=445916
> https://github.com/rust-lang/rust/issues/60705
> 
> libiberty/Changelog
> 
>   * rust-demangle.c (rust_demangle_callback): Ignore everything
>   after '.' char in sym for v0. For legacy symbols search
>   backwards to find the last 'E' before any '.'.
>   * testsuite/rust-demangle-expected: Add new .suffix testcases.
> ---
>  libiberty/rust-demangle.c  | 21 ++---
>  libiberty/testsuite/rust-demangle-expected | 26 ++
>  2 files changed, 44 insertions(+), 3 deletions(-)
> 
> V3 - Add more testcases
>- Allow @ in legacy symbols (which can appear in the .suffix)
> 
> diff --git a/libiberty/rust-demangle.c b/libiberty/rust-demangle.c
> index 18c760491bdc..42c88161da30 100644
> --- a/libiberty/rust-demangle.c
> +++ b/libiberty/rust-demangle.c
> @@ -1340,13 +1340,19 @@ rust_demangle_callback (const char *mangled, int 
> options,
>/* Rust symbols (v0) use only [_0-9a-zA-Z] characters. */
>for (p = rdm.sym; *p; p++)
>  {
> +  /* Rust v0 symbols can have '.' suffixes, ignore those.  */
> +  if (rdm.version == 0 && *p == '.')
> +break;
> +
>rdm.sym_len++;
>  
>if (*p == '_' || ISALNUM (*p))
>  continue;
>  
> -  /* Legacy Rust symbols can also contain [.:$] characters. */
> -  if (rdm.version == -1 && (*p == '$' || *p == '.' || *p == ':'))
> +  /* Legacy Rust symbols can also contain [.:$] characters.
> + Or @ in the .suffix (which will be skipped, see below). */
> +  if (rdm.version == -1 && (*p == '$' || *p == '.' || *p == ':'
> +|| *p == '@'))
>  continue;
>  
>return 0;
> @@ -1355,7 +1361,16 @@ rust_demangle_callback (const char *mangled, int 
> options,
>/* Legacy Rust symbols need to be handled separately. */
>if (rdm.version == -1)
>  {
> -  /* Legacy Rust symbols always end with E. */
> +  /* Legacy Rust symbols always end with E.  But can be followed by a
> + .suffix (which we want to ignore).  */
> +  int dot_suffix = 1;
> +  while (rdm.sym_len > 0 &&
> + !(dot_suffix && rdm.sym[rdm.sym_len - 1] == 'E'))
> +{
> +  dot_suffix = rdm.sym[rdm.sym_len - 1] == '.';
> +  rdm.sym_len--;
> +}
> +
>if (!(rdm.sym_len > 0 && rdm.sym[rdm.sym_len - 1] == 'E'))
>  return 0;
>rdm.sym_len--;
> diff --git a/libiberty/testsuite/rust-demangle-expected 
> b/libiberty/testsuite/rust-demangle-expected
> index 7dca315d0054..b565084cfefa 100644
> --- a/libiberty/testsuite/rust-demangle-expected
> +++ b/libiberty/testsuite/rust-demangle-expected
> @@ -295,3 +295,29 @@ _RMCs4fqI2P2rA04_13const_genericINtB0_4CharKc2202_E
>  --format=auto
>  _RNvNvMCs4fqI2P2rA04_13const_genericINtB4_3FooKpE3foo3FOO
>  >::foo::FOO
> +#
> +# Suffixes
> +#
> +--format=rust
> +_RNvMs0_NtCs5l0EXMQXRMU_21rustc_data_structures17obligation_forestINtB5_16ObligationForestNtNtNtCsdozMG8X9FIu_21rustc_trait_selection6traits7fulfill26PendingPredicateObligationE22register_obligation_atB1v_.llvm.8517020237817239694
> +>::register_obligation_at
> +--format=rust
> +_ZN4core3ptr85drop_in_place$LT$std..rt..lang_start$LT$$LP$$RP$$GT$..$u7b$$u7b$closure$u7d$$u7d$$GT$17h27f14859c664490dE.llvm.8091179795805947855
> +core::ptr::drop_in_place::{{closure}}>
> +# old style rustc llvm thinlto
> +--format=rust
> +_ZN9backtrace3foo17hbb467fcdaea5d79bE.llvm.A5310EB9
> +backtrace::foo
> +--format=rust
> +_ZN9backtrace3foo17hbb467fcdaea5d79bE.llvm.A5310EB9@@16
> +backtrace::foo
> +# new style rustc llvm thinlto
> +--format=rust
> +_RC3foo.llvm.9D1C9369
> +foo
> +--format=rust
> +_RC3foo.llvm.9D1C9369@@16
> +foo
> +--format=rust
> +_RNvC9backtrace3foo.llvm.A5310EB9
> +backtrace::foo
> -- 
> 2.30.2
> 

[PATCH] libiberty rust-demangle, ignore .suffix

[Mark Wielaard]

Rust v0 symbols can have a .suffix because if compiler transformations.
These can be ignored it the demangled name. Which is what this patch
implements). But an alternative implementation could be to follow what
C++ does and represent these as [clone .suffix] tagged onto the
demangled name. But this seems somewhat confusing since it results in
a demangled name that cannot be mangled again. And it would mean
trying to decode compiler internal naming.

https://bugs.kde.org/show_bug.cgi?id=445916
https://github.com/rust-lang/rust/issues/60705

libiberty/Changelog

* rust-demangle.c (rust_demangle_callback): Ignore everything
after '.' char in sym for v0.
* testsuite/rust-demangle-expected: Add new .suffix testcase.
---
 libiberty/rust-demangle.c  | 4 
 libiberty/testsuite/rust-demangle-expected | 6 ++
 2 files changed, 10 insertions(+)

diff --git a/libiberty/rust-demangle.c b/libiberty/rust-demangle.c
index 449941b56dc1..dee9eb64df79 100644
--- a/libiberty/rust-demangle.c
+++ b/libiberty/rust-demangle.c
@@ -1340,6 +1340,10 @@ rust_demangle_callback (const char *mangled, int options,
   /* Rust symbols (v0) use only [_0-9a-zA-Z] characters. */
   for (p = rdm.sym; *p; p++)
 {
+  /* Rust v0 symbols can have '.' suffixes, ignore those.  */
+  if (rdm.version == 0 && *p == '.')
+   break;
+
   rdm.sym_len++;
 
   if (*p == '_' || ISALNUM (*p))
diff --git a/libiberty/testsuite/rust-demangle-expected 
b/libiberty/testsuite/rust-demangle-expected
index 7dca315d0054..bc32ed85f882 100644
--- a/libiberty/testsuite/rust-demangle-expected
+++ b/libiberty/testsuite/rust-demangle-expected
@@ -295,3 +295,9 @@ _RMCs4fqI2P2rA04_13const_genericINtB0_4CharKc2202_E
 --format=auto
 _RNvNvMCs4fqI2P2rA04_13const_genericINtB4_3FooKpE3foo3FOO
 >::foo::FOO
+#
+# Suffixes
+#
+--format=rust
+_RNvMs0_NtCs5l0EXMQXRMU_21rustc_data_structures17obligation_forestINtB5_16ObligationForestNtNtNtCsdozMG8X9FIu_21rustc_trait_selection6traits7fulfill26PendingPredicateObligationE22register_obligation_atB1v_.llvm.8517020237817239694
+>::register_obligation_at
-- 
2.18.4

Re: [PATCH] libiberty rust-demangle, ignore .suffix

[Mark Wielaard]

Hi Eddy,

On Thu, Dec 02, 2021 at 07:35:17PM +0200, Eduard-Mihai Burtescu wrote:
> On Thu, Dec 2, 2021, at 19:17, Mark Wielaard wrote:
> > Rust v0 symbols can have a .suffix because if compiler transformations.
> 
> For some context, the suffix comes from LLVM (I believe as part of
> its LTO).  If this were a semantic part of a Rust symbol, it would
> have an encoding within v0 (as we already do for e.g. shims).

The same is true for gccrs or the libgccjit backend, gcc might clone
the symbol name and make it unique by appending some .suffix name.

> That also means that for consistency, suffixes like these should be
> handled uniformly for both v0 and legacy (as rustc-demangle does),
> since LLVM doesn't distinguish.

The problem with the legacy mangling is that dot '.' is a valid
character. That is why the patch only handles the v0 mangling case
(where dot '.' isn't valid).

> You may even be able to get Clang to generate C++ mangled symbols
> with ".llvm." suffixes, with enough application of LTO.  This is not
> unlike GCC ".clone" suffixes, AFAIK.  Sadly I don't think there's a
> way to handle both as "outside the symbol", without hardcoding
> ".llvm." in the implementation.

We could use the scheme used by c++ where the .suffix is added as "
[clone .suffix]", it even handles multiple dots, where something like
_Z3fooi.part.9.165493.constprop.775.31805
demangles to
foo(int) [clone .part.9.165493] [clone .constprop.775.31805]

I just don't think that is very useful and a little confusing.

> I don't recall the libiberty demangling API having any provisions
> for the demangler deciding that a mangled symbol "stops early",
> which would maybe allow for a more general solution.

No, there indeed is no interface. We might introduce a new option flag
for treating '.' as end of symbol. But do we really need that
flexibility?

> Despite all that, if it helps in practice, I would still not mind
> this patch landing in its current form, I just wanted to share my
> perspective on the larger issue.

Thanks for that. Do you happen to know what other rust demanglers do?

Cheers,

Mark

Re: [PATCH] libiberty rust-demangle, ignore .suffix

[Mark Wielaard]

Hi,

On Fri, Dec 03, 2021 at 06:58:36AM +1100, Nicholas Nethercote wrote:
> On Fri, 3 Dec 2021 at 04:17, Mark Wielaard  wrote:
> >
> > * rust-demangle.c (rust_demangle_callback): Ignore everything
> > after '.' char in sym for v0.
> >
> 
> I just applied this change to Valgrind's copy of rust-demangle.c and I can
> confirm that it works -- the symbols that were failing to be demangled are
> now demangled.

Thanks for testing.

> > +  /* Rust v0 symbols can have '.' suffixes, ignore those.  */
> > +  if (rdm.version == 0 && *p == '.')
> > +   break;
> >
> 
> Note that the indent on the `break` is 1 char, it should be 2.

Oops. That is actually because I used a tab for indentation, but this
file doesn't use tabs. Fixed in my local copy
https://code.wildebeest.org/git/user/mjw/gcc/commit/?h=rust-demangle-suffix
Will push with that fix if approved.

Thanks,

Mark

Re: [PATCH] libiberty rust-demangle, ignore .suffix

[Mark Wielaard]

Hi Eddy,

On Fri, 2021-12-03 at 01:14 +0200, Eduard-Mihai Burtescu wrote:
> On Fri, Dec 3, 2021, at 00:07, Mark Wielaard wrote:
> > On Thu, Dec 02, 2021 at 07:35:17PM +0200, Eduard-Mihai Burtescu
> > wrote:
> > > That also means that for consistency, suffixes like these should
> > > be
> > > handled uniformly for both v0 and legacy (as rustc-demangle
> > > does),
> > > since LLVM doesn't distinguish.
> > 
> > The problem with the legacy mangling is that dot '.' is a valid
> > character. That is why the patch only handles the v0 mangling case
> > (where dot '.' isn't valid).
> 
> Thought so, that's an annoying complication - however, see later down
> why that's still not a blocker to the way rustc-demangle handles it.
> 
> > > You may even be able to get Clang to generate C++ mangled symbols
> > > with ".llvm." suffixes, with enough application of LTO.  This is
> > > not
> > > unlike GCC ".clone" suffixes, AFAIK.  Sadly I don't think there's
> > > a
> > > way to handle both as "outside the symbol", without hardcoding
> > > ".llvm." in the implementation.
> > 
> > We could use the scheme used by c++ where the .suffix is added as "
> > [clone .suffix]", it even handles multiple dots, where something
> > like
> > _Z3fooi.part.9.165493.constprop.775.31805
> > demangles to
> > foo(int) [clone .part.9.165493] [clone .constprop.775.31805]
> > 
> > I just don't think that is very useful and a little confusing.
> 
> Calling it "clone" is a bit weird, but I just checked what rustc-
> demangle
> does for printing suffixes back out and it's not great either:
> - ".llvm." (and everything after it) is completely removed
> - any left-over suffixes (after demangling), if they start with ".",
> are
>   not considered errors, but printed out verbatim after the
> demangling
> 
> > > I don't recall the libiberty demangling API having any provisions
> > > for the demangler deciding that a mangled symbol "stops early",
> > > which would maybe allow for a more general solution.
> > 
> > No, there indeed is no interface. We might introduce a new option
> > flag
> > for treating '.' as end of symbol. But do we really need that
> > flexibility?
> 
> That's not what I meant - a v0 or legacy symbol is self-terminating
> in
> its parsing (or at the very least there are not dots allowed outside
> of a length-prefixed identifier), so that when you see the start of
> a valid mangled symbol, you can always find its end in the string,
> even when that end is half-way through (and is followed by suffixes
> or any other unrelated noise).
> 
> What I was imagining is a way to return to the caller the number of
> chars from the start of the original string, that were demangled,
> letting the caller do something else with the rest of that string.
> (see below for how rustc-demangle already does something similar)
>
> > > Despite all that, if it helps in practice, I would still not mind
> > > this patch landing in its current form, I just wanted to share my
> > > perspective on the larger issue.
> > 
> > Thanks for that. Do you happen to know what other rust demanglers
> > do?
> 
> rustc-demangle's internal API returns a pair of the demangler and the
> "leftover" parts of the original string, after the end of the symbol.
> You can see here how that suffix is further checked, and kept:
> https://github.com/rust-lang/rustc-demangle/blob/2811a1ad6f7c8bead2ef3671e4fdc10de1553e96/src/lib.rs#L108-L138

Yes, returning a struct that returns the style detected, the demangled
string and the left over chars makes sense. But we don't have an
interface like that at the moment, and I am not sure we (currently)
have users who want this.

> As mentioned above, ".llvm." is handled differently, just above the snippet
> linked - perhaps it was deemed too common to let it pollute the output.

But that also makes it a slightly odd interface. I would imagine that
people would be interested in the .llvm. part. Now that just gets
dropped.

Since we don't have an interface to return the suffix (and I find the
choice of dropping .llvm. but not other suffixes odd), I think we
should just simply always drop the .suffix. I understand now how to do
that for legacy symbols too, thanks for the hints.

See the attached update to the patch. What do you think?

Thanks,

Mark
From b9ea4a1cc5d139a08fd04bd5c59dae1d1519f82b Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Th

Re: [PATCH Rust front-end v3 01/46] Use DW_ATE_UTF for the Rust 'char' type

[Mark Wielaard]

Hi,

On Wed, Oct 26, 2022 at 10:39:09AM +0200, Jakub Jelinek wrote:
> I must say I don't understand nor like this DW_LANG_Rust_old stuff at all.
> Other languages don't do similar dances.
> Look for D, or Go.  Neither of them has any non-standard lang code as
> fallback, they use the DWARF assigned DW_LANG_* code, and DW_LANG_C as
> fallback.  On most arches, DWARF 5 is the default anyway, or non-strict
> DWARF at least.  Where neither is enabled because of prehistoric or buggy
> DWARF consumers, it is unlikely they'd handle Rust sanely anyway.
> Just follow what Go does in the same function.

DW_LANG_Rust_old was used by old rustc compilers <= 2016 before DWARF5
assigned an official number. It might be recognized by some
debuggers. But I agree that these days it doesn't really make sense to
emit it. When producing strict DWARF it is also slightly odd to emit a
non-standard language code. So I agree that it makes sense to do what
Go does, always emit DW_LANG_Rust unless we emit strict DWARF for
versions before 5 (and then just fall back to DW_LANG_C).

The attached patch (against "upstream gccrs") does that. I kept the
oldlang.rs testcase just to see that the -gstrict-dwarf -gdwarf-3 case
does something sane.

The only "issue" is that is_rust () depends on the comp_unit_die
DW_AT_language being DW_LANG_Rust. But the only usage of is_rust
already depends on strict DWARF.

https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=no-Rust-old
if someone wants to push that, to merge for a v4.

Thanks,

Mark>From cdcfe27cfba23402f91200c64c1ef8e0bf3528a0 Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Sun, 30 Oct 2022 16:03:16 +0100
Subject: [PATCH] dwarf2out.c: Don't emit DW_LANG_Rust_old

DW_LANG_Rust_old is a non-standard DWARF language code used by old
rustc compilers before DWARF5 (released in 2017). Just always emit
DW_LANG_Rust unless producing strict DWARF for versions before 5.
And in that old strict DWARF case just emit DW_LANG_C instead of a
non-standard language code.
---
 gcc/dwarf2out.cc| 14 +-
 gcc/testsuite/rust/debug/oldlang.rs |  4 ++--
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc
index 7b9d5ae33fc..87c0d103a27 100644
--- a/gcc/dwarf2out.cc
+++ b/gcc/dwarf2out.cc
@@ -5600,14 +5600,15 @@ is_fortran (const_tree decl)
   return is_fortran ();
 }
 
-/* Return TRUE if the language is Rust.  */
+/* Return TRUE if the language is Rust.
+   Note, returns FALSE for dwarf_version < 5 && dwarf_strict. */
 
 static inline bool
 is_rust ()
 {
   unsigned int lang = get_AT_unsigned (comp_unit_die (), DW_AT_language);
 
-  return lang == DW_LANG_Rust || lang == DW_LANG_Rust_old;
+  return lang == DW_LANG_Rust;
 }
 
 /* Return TRUE if the language is Ada.  */
@@ -25216,13 +25217,6 @@ gen_compile_unit_die (const char *filename)
 }
   else if (strcmp (language_string, "GNU F77") == 0)
 language = DW_LANG_Fortran77;
-  else if (strcmp (language_string, "GNU Rust") == 0)
-{
-  if (dwarf_version >= 5 || !dwarf_strict)
-	language = DW_LANG_Rust;
-  else
-	language = DW_LANG_Rust_old;
-}
   else if (dwarf_version >= 3 || !dwarf_strict)
 {
   if (strcmp (language_string, "GNU Ada") == 0)
@@ -25248,6 +25242,8 @@ gen_compile_unit_die (const char *filename)
 	{
 	  if (strcmp (language_string, "GNU Go") == 0)
 	language = DW_LANG_Go;
+	  else if (strcmp (language_string, "GNU Rust") == 0)
+	language = DW_LANG_Rust;
 	}
 }
   /* Use a degraded Fortran setting in strict DWARF2 so is_fortran works.  */
diff --git a/gcc/testsuite/rust/debug/oldlang.rs b/gcc/testsuite/rust/debug/oldlang.rs
index ddacf0e4392..648d6b78f06 100644
--- a/gcc/testsuite/rust/debug/oldlang.rs
+++ b/gcc/testsuite/rust/debug/oldlang.rs
@@ -1,6 +1,6 @@
 fn main () {
 // { dg-do compile }
 // { dg-options "-gstrict-dwarf -gdwarf-3 -dA" }
-// DW_LANG_Rust_old is 0x9000
-// { dg-final { scan-assembler "0x9000\[ \t]\[^\n\r]* DW_AT_language" } } */
+// Strict DWARF < 5 uses DW_LANG_C = 0x0002
+// { dg-final { scan-assembler "0x2\[ \t]\[^\n\r]* DW_AT_language" } } */
 }
-- 
2.30.2

Re: Rust frontend patches v2

[Mark Wielaard]

Hi Martin,

On Thu, 2022-08-25 at 11:52 +0200, Martin Liška wrote:
> What about limit increase, how much space do we have on sourceware
> infrastructure?

Feel free to increase the limits, there is a couple of hundred GB free
on sourceware and we can add more.

The public-inbox instance at inbox.sourceware.org was setup to
workaround some of these mailman limits and make it easier to handle
patch emails. It was only finalized last week, but testing shows it
seems ready for more wide use. Let me write an announcement for gcc.

Cheers,

Mark

Re: BoF DWARF5 patches (25% .debug section size reduction)

[Mark Wielaard]

On Tue, 2020-09-29 at 15:56 +0200, Mark Wielaard wrote:
> On Thu, 2020-09-10 at 13:16 +0200, Jakub Jelinek wrote:
> > On Wed, Sep 09, 2020 at 09:57:54PM +0200, Mark Wielaard wrote:
> > > --- a/gcc/doc/invoke.texi
> > > +++ b/gcc/doc/invoke.texi
> > > @@ -9057,13 +9057,14 @@ possible.
> > >  @opindex gdwarf
> > >  Produce debugging information in DWARF format (if that is supported).
> > >  The value of @var{version} may be either 2, 3, 4 or 5; the default 
> > > version
> > > -for most targets is 4.  DWARF Version 5 is only experimental.
> > > +for most targets is 5 (with the exception of vxworks and darwin which
> > > +default to version 2).
> > 
> > I think in documentation we should spell these VxWorks and Darwin/Mac OS X
> 
> OK. As attached.
> 
> Are we ready to flip the default to 5?

Ping. It would be good to get this in now so that we can fix issues (if
any) with the DWARF5 support in the general bugfixing stage 3.

Thanks,

Mark
From c04727b6209ad4d52d1b9ba86873961bda0e1724 Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Tue, 29 Sep 2020 15:52:44 +0200
Subject: [PATCH] Default to DWARF5

gcc/ChangeLog:

	* common.opt (gdwarf-): Init(5).
	* doc/invoke.texi (-gdwarf): Document default to 5.
---
 gcc/common.opt  | 2 +-
 gcc/doc/invoke.texi | 7 ---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index 9552cebe0d6c..dd92ef1027c6 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -3164,7 +3164,7 @@ Common Driver JoinedOrMissing Negative(gdwarf-)
 Generate debug information in default version of DWARF format.
 
 gdwarf-
-Common Driver Joined UInteger Var(dwarf_version) Init(4) Negative(gstabs)
+Common Driver Joined UInteger Var(dwarf_version) Init(5) Negative(gstabs)
 Generate debug information in DWARF v2 (or later) format.
 
 ggdb
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index b3a2c7ce51d8..f31d6a46fff8 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9410,14 +9410,15 @@ possible.
 @itemx -gdwarf-@var{version}
 @opindex gdwarf
 Produce debugging information in DWARF format (if that is supported).
-The value of @var{version} may be either 2, 3, 4 or 5; the default version
-for most targets is 4.  DWARF Version 5 is only experimental.
+The value of @var{version} may be either 2, 3, 4 or 5; the default
+version for most targets is 5 (with the exception of VxWorks and
+Darwin/Mac OS X which default to version 2).
 
 Note that with DWARF Version 2, some ports require and always
 use some non-conflicting DWARF 3 extensions in the unwind tables.
 
 Version 4 may require GDB 7.0 and @option{-fvar-tracking-assignments}
-for maximum benefit.
+for maximum benefit. Version 5 requires GDB 8.0 or higher.
 
 GCC no longer supports DWARF Version 1, which is substantially
 different than Version 2 and later.  For historical reasons, some
-- 
2.18.4

[COMMITTED htdocs] robots.txt: Disallow various wiki actions

[Mark Wielaard]

It is fine for robots to crawl the wiki pages, but they should perform
actions, generate huge diffs, search/highlight pages or generate
calendars.
---
 htdocs/robots.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/htdocs/robots.txt b/htdocs/robots.txt
index 057c5899..36be4d13 100644
--- a/htdocs/robots.txt
+++ b/htdocs/robots.txt
@@ -14,4 +14,8 @@ Disallow: /bugzilla/show_bug.cgi*ctype=xml*
 Disallow: /bugzilla/attachment.cgi
 Disallow: /bugzilla/showdependencygraph.cgi
 Disallow: /bugzilla/showdependencytree.cgi
+Disallow: /wiki/*?action=*
+Disallow: /wiki/*?diffs=*
+Disallow: /wiki/*?highlight=*
+Disallow: /wiki/*?calparms=*
 Crawl-Delay: 60
-- 
2.43.2

Re: CI for "Option handling: add documentation URLs"

[Mark Wielaard]

Hi,

On Sat, Feb 24, 2024 at 06:42:58PM +0100, Mark Wielaard wrote:
> On Thu, Feb 22, 2024 at 11:57:50AM +0800, YunQiang Su wrote:
> > Mark Wielaard  于2024年2月19日周一 06:58写道：
> > > So, I did try the regenerate-opt-urls locally, and it did generate the
> > > attached diff. Which seems to show we really need this automated.
> > >
> > > Going over the diff. The -Winfinite-recursion in rust does indeed seem
> > > new.  As do the -mapx-inline-asm-use-gpr32 and mevex512 for i386.  And
> > > the avr options -mskip-bug, -mflmap and mrodata-in-ram.  The change in
> > > common.opt.urls for -Wuse-after-free comes from it being moved from
> > > c++ to the c-family. The changes in mips.opt.urls seem to come from
> > > commit 46df1369 "doc/invoke: Remove duplicate explicit-relocs entry of
> > > MIPS".
> > 
> > For MIPS, it's due to malformed patches to invoke.text.
> > I will fix them.
> 
> Thanks. So with your commit 00bc8c0998d8 ("invoke.texi: Fix some
> skipping UrlSuffix problem for MIPS") pushed now, the attached patch
> fixes the remaining issues.
> 
> Is this OK to push?

Ping.

I have now regenerated the patch to also include the new avr mfuse-add
change. It would be nice to get this committed so we can turn on the
automatic checker.

Thanks,

Mark
>From 84373cd8045e67f0d1716dad899c3463b823ea97 Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Sun, 3 Mar 2024 20:50:32 +0100
Subject: [PATCH] Regenerate opt.urls

There were several commits that didn't regenerate the opt.urls files.

Fixes: 438ef143679e ("rs6000: Neuter option -mpower{8,9}-vector")
Fixes: 50c549ef3db6 ("gccrs: enable -Winfinite-recursion warnings by default")
Fixes: 25bb8a40abd9 ("Move docs for -Wuse-after-free and -Wuseless-cast")
Fixes: 48448055fb70 ("AVR: Support .rodata in Flash for AVR64* and AVR128*")
Fixes: 42503cc257fb ("AVR: Document option -mskip-bug")
Fixes: 7de5bb642c12 ("i386: [APX] Document inline asm behavior and new switch")
Fixes: 49a14ee488b8 ("Add -mevex512 into invoke.texi")
Fixes: 4666cbde5e6d ("Sort warning options in c-family/c.opt.")
Fixes: cda383616183 ("AVR: target/114100 - Better indirect accesses for reduced 
Tiny")

gcc/config/
* rs6000/rs6000.opt.urls: Regenerate.
* avr/avr.opt.urls: Likewise.
* i386/i386.opt.urls: Likewise.
* pru/pru.opt.urls: Likewise.
* riscv/riscv.opt.urls: Likewise.

gcc/rust/
* lang.opt.urls: Regenerate.

gcc/
* common.opt.urls: Regenerate.

gcc/c-family/
* c.opt.urls: Regenerate.
---
 gcc/c-family/c.opt.urls   | 351 +++---
 gcc/common.opt.urls   |   4 +-
 gcc/config/avr/avr.opt.urls   |  15 ++
 gcc/config/i386/i386.opt.urls |   8 +-
 gcc/config/pru/pru.opt.urls   |   2 +-
 gcc/config/riscv/riscv.opt.urls   |   2 +-
 gcc/config/rs6000/rs6000.opt.urls |   3 -
 gcc/rust/lang.opt.urls|   3 +
 8 files changed, 206 insertions(+), 182 deletions(-)

diff --git a/gcc/c-family/c.opt.urls b/gcc/c-family/c.opt.urls
index 5365c8e2bc54..9f97dc61a778 100644
--- a/gcc/c-family/c.opt.urls
+++ b/gcc/c-family/c.opt.urls
@@ -88,6 +88,9 @@ UrlSuffix(gcc/Warning-Options.html#index-Wabsolute-value)
 Waddress
 UrlSuffix(gcc/Warning-Options.html#index-Waddress)
 
+Waddress-of-packed-member
+UrlSuffix(gcc/Warning-Options.html#index-Waddress-of-packed-member)
+
 Waligned-new
 UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Waligned-new)
 
@@ -115,6 +118,9 @@ UrlSuffix(gcc/Warning-Options.html#index-Walloc-zero)
 Walloca-larger-than=
 UrlSuffix(gcc/Warning-Options.html#index-Walloca-larger-than_003d) 
LangUrlSuffix_D(gdc/Warnings.html#index-Walloca-larger-than)
 
+Warith-conversion
+UrlSuffix(gcc/Warning-Options.html#index-Warith-conversion)
+
 Warray-bounds=
 UrlSuffix(gcc/Warning-Options.html#index-Warray-bounds)
 
@@ -122,13 +128,10 @@ Warray-compare
 UrlSuffix(gcc/Warning-Options.html#index-Warray-compare)
 
 Warray-parameter
-UrlSuffix(gcc/Warning-Options.html#index-Wno-array-parameter)
+UrlSuffix(gcc/Warning-Options.html#index-Warray-parameter)
 
 Warray-parameter=
-UrlSuffix(gcc/Warning-Options.html#index-Wno-array-parameter)
-
-Wzero-length-bounds
-UrlSuffix(gcc/Warning-Options.html#index-Wzero-length-bounds)
+UrlSuffix(gcc/Warning-Options.html#index-Warray-parameter)
 
 Wassign-intercept
 
UrlSuffix(gcc/Objective-C-and-Objective-C_002b_002b-Dialect-Options.html#index-Wassign-intercept)
@@ -148,9 +151,6 @@ UrlSuffix(gcc/Warning-Options.html#index-Wbool-compare)
 Wbool-operation
 UrlSuffix(gcc/Warning-Options.html#index-Wbool-operation)
 
-Wframe-address
-UrlSuffix(gcc/Warning-Options.html#index-Wframe-address)
-
 Wbuiltin-declaration-mismatch
 UrlSuffix(gcc/Warning-Options.html#index-Wbuiltin-declaration-mismatch) 
La

Re: CI for "Option handling: add documentation URLs"

[Mark Wielaard]

Hi,

On Mon, 2024-03-04 at 08:48 -0500, David Malcolm wrote:
> > I have now regenerated the patch to also include the new avr mfuse-
> > add change. It would be nice to get this committed so we can turn on the
> > automatic checker.
> 
> Please go ahead with that.

I committed that patch, but was not fast enough actually enabling the
buildbot and missed another fixlet needed first.

OK, to push the attached regeneration patch?

Thanks,

Mark
From e5c2b9983d7c09e5a21fa587dc9cd03d53d67a23 Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Tue, 5 Mar 2024 13:01:08 +0100
Subject: [PATCH] Regenerate c.opt.urls

Fixes: 08edf85f747b ("c++/modules: relax diagnostic about GMF contents")

gcc/c-family/ChangeLog:

	* c.opt.urls: Regenerate.
---
 gcc/c-family/c.opt.urls | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/c-family/c.opt.urls b/gcc/c-family/c.opt.urls
index 9f97dc61a778..631719863a5e 100644
--- a/gcc/c-family/c.opt.urls
+++ b/gcc/c-family/c.opt.urls
@@ -403,6 +403,9 @@ UrlSuffix(gcc/Warning-Options.html#index-Wformat)
 Wframe-address
 UrlSuffix(gcc/Warning-Options.html#index-Wframe-address)
 
+Wglobal-module
+UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Wglobal-module)
+
 Wif-not-aligned
 UrlSuffix(gcc/Warning-Options.html#index-Wif-not-aligned)
 
-- 
2.44.0

Re: CI for "Option handling: add documentation URLs"

[Mark Wielaard]

On Tue, Mar 05, 2024 at 08:34:31AM -0500, David Malcolm wrote:
> > I committed that patch, but was not fast enough actually enabling the
> > buildbot and missed another fixlet needed first.
> > 
> > OK, to push the attached regeneration patch?
> 
> Yes

Thanks, pushed. And now also pushed the builder patch (attached) to
enable it in the CI autoregen checker. It already ran without finding
any issues.

https://builder.sourceware.org/buildbot/#/builders/gcc-autoregen

Re: CI for "Option handling: add documentation URLs"

[Mark Wielaard]

Hi YunQiang Su,

On Fri, Mar 15, 2024 at 03:33:28PM +0800, YunQiang Su wrote:
> Great work. The CI works well now: it blames me ;)
> https://builder.sourceware.org/buildbot/#/builders/269/builds/3846
> 
> When I add '-mstrict-align' option to MIPS,
> the riscv.opt.urls, sysv4.opt.urls, xtensa.opt.urls are changed also.
> (why they are effected?

They are effected because they also have a '-mstrict-align' option and
each option with the same name gets an unique number.

> So what's the best practice for this cases?
> Should I push a new commit? Or in fact a single commit is preferred?

I don't know if there is a rule for this. But I hope this falls under
the obvious rule. What I would do is simply take that diff the CI
produced.
https://builder.sourceware.org/buildbot/api/v2/logs/7798308/raw

Apply it and commit that with:

Regenerate opt.urls

Fixes: acc38ff59976 ("MIPS: Add -m(no-)strict-align option")

gcc/ChangeLog:

* config/riscv/riscv.opt.urls: Regenerated.
* config/rs6000/sysv4.opt.urls: Likewise.
* config/xtensa/xtensa.opt.urls: Likewise.

Cheers,

Mark

Re: [PATCH] Regenerate loongarch.opt.urls.

[Mark Wielaard]

Hi,

On Mon, Apr 01, 2024 at 11:08:08AM +0800, Lulu Cheng wrote:
> Fixes: d28ea8e5a704 ("LoongArch: Split loongarch_option_override_internal
> into smaller procedures")
> 
> gcc/ChangeLog:
> 
>   * config/loongarch/loongarch.opt.urls: Regenerate.

This looks OK to me. I cannot officially approve patches, but I think
this falls under the Obvious fixes rule.

Cheers,

Mark

[COMMITTED] Regenerate i386.opt.urls

[Mark Wielaard]

LoongArch added an -mtls-dialect option, causing the similarly
named option for i386 get renumbered.

Fixes: b253b4695dda ("LoongArch: Add support for TLS descriptors.")

gcc/ChangeLog:

* config/i386/i386.opt.urls: Regenerate.
---

Pushed as obvious.

 gcc/config/i386/i386.opt.urls | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/i386/i386.opt.urls b/gcc/config/i386/i386.opt.urls
index fa821eba2006..81c5bb9a9270 100644
--- a/gcc/config/i386/i386.opt.urls
+++ b/gcc/config/i386/i386.opt.urls
@@ -128,7 +128,7 @@ mstackrealign
 UrlSuffix(gcc/x86-Options.html#index-mstackrealign)
 
 mtls-dialect=
-UrlSuffix(gcc/x86-Options.html#index-mtls-dialect-1)
+UrlSuffix(gcc/x86-Options.html#index-mtls-dialect-2)
 
 mtls-direct-seg-refs
 UrlSuffix(gcc/x86-Options.html#index-mtls-direct-seg-refs)
-- 
2.44.0

[COMMITTED] Regenerate common.opt.urls

[Mark Wielaard]

The new support for gcov modified condition/decision coverage
introduced two new flags for gcc, -Wcoverage-too-many-conditions and
-fcondition-coverage. But didn't regenerate the gcc/common.opt.urls.

Fixes: 08a52331803f ("Add condition coverage (MC/DC)")

gcc/ChangeLog:

* common.opt.urls: Regenerate.
---

Pushed as obvious.

 gcc/common.opt.urls | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/gcc/common.opt.urls b/gcc/common.opt.urls
index db4354989fcc..f71ed80a34b4 100644
--- a/gcc/common.opt.urls
+++ b/gcc/common.opt.urls
@@ -280,6 +280,9 @@ UrlSuffix(gcc/Warning-Options.html#index-Wcoverage-mismatch)
 Wcoverage-invalid-line-number
 UrlSuffix(gcc/Warning-Options.html#index-Wcoverage-invalid-line-number)
 
+Wcoverage-too-many-conditions
+UrlSuffix(gcc/Warning-Options.html#index-Wcoverage-too-many-conditions)
+
 Wmissing-profile
 UrlSuffix(gcc/Warning-Options.html#index-Wmissing-profile)
 
@@ -1057,6 +1060,9 @@ 
UrlSuffix(gcc/Instrumentation-Options.html#index-fprofile-abs-path)
 ;   duplicate: 'gcc/Instrumentation-Options.html#index-fprofile-arcs'
 ;   duplicate: 'gcc/Other-Builtins.html#index-fprofile-arcs-1'
 
+fcondition-coverage
+UrlSuffix(gcc/Instrumentation-Options.html#index-fcondition-coverage)
+
 fprofile-dir=
 UrlSuffix(gcc/Instrumentation-Options.html#index-fprofile-dir)
 
-- 
2.44.0

Re: CI for "Option handling: add documentation URLs"

[Mark Wielaard]

Hi David,

On Thu, Jan 04, 2024 at 09:57:09AM -0500, David Malcolm wrote:
> I've pushed the .opt.urls patch kit to gcc trunk [1], so hopefully the
> CI check you wrote can go live now.

And then I was on vacation myself and forgot. I am sorry.

So, I did try the regenerate-opt-urls locally, and it did generate the
attached diff. Which seems to show we really need this automated.

Going over the diff. The -Winfinite-recursion in rust does indeed seem
new.  As do the -mapx-inline-asm-use-gpr32 and mevex512 for i386.  And
the avr options -mskip-bug, -mflmap and mrodata-in-ram.  The change in
common.opt.urls for -Wuse-after-free comes from it being moved from
c++ to the c-family. The changes in mips.opt.urls seem to come from
commit 46df1369 "doc/invoke: Remove duplicate explicit-relocs entry of
MIPS".

The changes in c.opt.urls seem mostly reordering. The sorting makes
more sense after the diff imho. And must have come from commit
4666cbde5 "Sort warning options in c-family/c.opt".

Also the documentation for -Warray-parameter was fixed.

So I think the regenerate-opt-urls check does work as intended. So
lets automate it, because it looks like nobody regenerated the
url.opts after updating the documentation.

But we should first apply this diff. Could you double check it is
sane/correct?

Thanks,

Mark

Re: CI for "Option handling: add documentation URLs"

[Mark Wielaard]

On Sun, 2024-02-18 at 23:58 +0100, Mark Wielaard wrote:
> So I think the regenerate-opt-urls check does work as intended. So
> lets automate it, because it looks like nobody regenerated the
> url.opts after updating the documentation.
> 
> But we should first apply this diff. Could you double check it is
> sane/correct?

And then I forgot to attach the diff. Attached now.
Hopefully it is identical for you after doing
  make html && cd gcc && make regenerate-opt-urls
(It is for me having now done it on a debian and fedora x86_64 setup.)

Cheers,

Mark
diff --git a/gcc/c-family/c.opt.urls b/gcc/c-family/c.opt.urls
index 5365c8e2bc5..9f97dc61a77 100644
--- a/gcc/c-family/c.opt.urls
+++ b/gcc/c-family/c.opt.urls
@@ -88,6 +88,9 @@ UrlSuffix(gcc/Warning-Options.html#index-Wabsolute-value)
 Waddress
 UrlSuffix(gcc/Warning-Options.html#index-Waddress)
 
+Waddress-of-packed-member
+UrlSuffix(gcc/Warning-Options.html#index-Waddress-of-packed-member)
+
 Waligned-new
 UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Waligned-new)
 
@@ -115,6 +118,9 @@ UrlSuffix(gcc/Warning-Options.html#index-Walloc-zero)
 Walloca-larger-than=
 UrlSuffix(gcc/Warning-Options.html#index-Walloca-larger-than_003d) LangUrlSuffix_D(gdc/Warnings.html#index-Walloca-larger-than)
 
+Warith-conversion
+UrlSuffix(gcc/Warning-Options.html#index-Warith-conversion)
+
 Warray-bounds=
 UrlSuffix(gcc/Warning-Options.html#index-Warray-bounds)
 
@@ -122,13 +128,10 @@ Warray-compare
 UrlSuffix(gcc/Warning-Options.html#index-Warray-compare)
 
 Warray-parameter
-UrlSuffix(gcc/Warning-Options.html#index-Wno-array-parameter)
+UrlSuffix(gcc/Warning-Options.html#index-Warray-parameter)
 
 Warray-parameter=
-UrlSuffix(gcc/Warning-Options.html#index-Wno-array-parameter)
-
-Wzero-length-bounds
-UrlSuffix(gcc/Warning-Options.html#index-Wzero-length-bounds)
+UrlSuffix(gcc/Warning-Options.html#index-Warray-parameter)
 
 Wassign-intercept
 UrlSuffix(gcc/Objective-C-and-Objective-C_002b_002b-Dialect-Options.html#index-Wassign-intercept)
@@ -148,9 +151,6 @@ UrlSuffix(gcc/Warning-Options.html#index-Wbool-compare)
 Wbool-operation
 UrlSuffix(gcc/Warning-Options.html#index-Wbool-operation)
 
-Wframe-address
-UrlSuffix(gcc/Warning-Options.html#index-Wframe-address)
-
 Wbuiltin-declaration-mismatch
 UrlSuffix(gcc/Warning-Options.html#index-Wbuiltin-declaration-mismatch) LangUrlSuffix_D(gdc/Warnings.html#index-Wbuiltin-declaration-mismatch)
 
@@ -217,6 +217,12 @@ UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Wcatch-value)
 Wchar-subscripts
 UrlSuffix(gcc/Warning-Options.html#index-Wchar-subscripts)
 
+Wclass-conversion
+UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Wclass-conversion)
+
+Wclass-memaccess
+UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Wclass-memaccess)
+
 Wclobbered
 UrlSuffix(gcc/Warning-Options.html#index-Wclobbered)
 
@@ -298,6 +304,12 @@ UrlSuffix(gcc/Warning-Options.html#index-Wdiscarded-qualifiers)
 Wdiv-by-zero
 UrlSuffix(gcc/Warning-Options.html#index-Wdiv-by-zero)
 
+Wdouble-promotion
+UrlSuffix(gcc/Warning-Options.html#index-Wdouble-promotion)
+
+Wduplicate-decl-specifier
+UrlSuffix(gcc/Warning-Options.html#index-Wduplicate-decl-specifier)
+
 Wduplicated-branches
 UrlSuffix(gcc/Warning-Options.html#index-Wduplicated-branches)
 
@@ -307,6 +319,9 @@ UrlSuffix(gcc/Warning-Options.html#index-Wduplicated-cond)
 Weffc++
 UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Weffc_002b_002b)
 
+Welaborated-enum-base
+UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Welaborated-enum-base)
+
 Wempty-body
 UrlSuffix(gcc/Warning-Options.html#index-Wempty-body)
 
@@ -328,12 +343,18 @@ UrlSuffix(gcc/Warning-Options.html#index-Werror) LangUrlSuffix_D(gdc/Warnings.ht
 Wexceptions
 UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Wexceptions)
 
+Wexpansion-to-defined
+UrlSuffix(gcc/Warning-Options.html#index-Wexpansion-to-defined)
+
 Wextra
 UrlSuffix(gcc/Warning-Options.html#index-Wextra) LangUrlSuffix_D(gdc/Warnings.html#index-Wextra)
 
 Wextra-semi
 UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Wextra-semi)
 
+Wflex-array-member-not-at-end
+UrlSuffix(gcc/Warning-Options.html#index-Wflex-array-member-not-at-end)
+
 Wfloat-conversion
 UrlSuffix(gcc/Warning-Options.html#index-Wfloat-conversion)
 
@@ -355,6 +376,9 @@ UrlSuffix(gcc/Warning-Options.html#index-Wformat-nonliteral)
 Wformat-overflow
 UrlSuffix(gcc/Warning-Options.html#index-Wformat-overflow)
 
+Wformat-overflow=
+UrlSuffix(gcc/Warning-Options.html#index-Wformat-overflow)
+
 Wformat-security
 UrlSuffix(gcc/Warning-Options.html#index-Wformat-security)
 
@@ -364,6 +388,9 @@ UrlSuffix(gcc/Warning-Options.html#index-Wformat-signedness)
 Wformat-truncation
 UrlSuffix(gcc/Warning-Options.html#index-Wformat-truncation)
 
+Wformat-truncation=
+UrlSuffix(gcc/Warning-Options.html#index-Wformat-truncation)
+
 Wformat-y2k
 UrlSuffix(gcc/Warning-Options.html#index-Wformat-y2k)
 
@@ -373,14 +400,8 @@ UrlSuffix(gcc/Warning-Options.html#index-Wformat

Re: CI for "Option handling: add documentation URLs"

[Mark Wielaard]

Hi,

On Thu, Feb 22, 2024 at 11:57:50AM +0800, YunQiang Su wrote:
> Mark Wielaard  于2024年2月19日周一 06:58写道：
> > So, I did try the regenerate-opt-urls locally, and it did generate the
> > attached diff. Which seems to show we really need this automated.
> >
> > Going over the diff. The -Winfinite-recursion in rust does indeed seem
> > new.  As do the -mapx-inline-asm-use-gpr32 and mevex512 for i386.  And
> > the avr options -mskip-bug, -mflmap and mrodata-in-ram.  The change in
> > common.opt.urls for -Wuse-after-free comes from it being moved from
> > c++ to the c-family. The changes in mips.opt.urls seem to come from
> > commit 46df1369 "doc/invoke: Remove duplicate explicit-relocs entry of
> > MIPS".
> >
> 
> For MIPS, it's due to malformed patches to invoke.text.
> I will fix them.

Thanks. So with your commit 00bc8c0998d8 ("invoke.texi: Fix some
skipping UrlSuffix problem for MIPS") pushed now, the attached patch
fixes the remaining issues.

Is this OK to push?

> > The changes in c.opt.urls seem mostly reordering. The sorting makes
> > more sense after the diff imho. And must have come from commit
> > 4666cbde5 "Sort warning options in c-family/c.opt".
> >
> > Also the documentation for -Warray-parameter was fixed.
> >
> > So I think the regenerate-opt-urls check does work as intended. So
> > lets automate it, because it looks like nobody regenerated the
> > url.opts after updating the documentation.
> >
> > But we should first apply this diff. Could you double check it is
> > sane/correct?
> >
> > Thanks,
> >
> > Mark
> 
> 
> 
> -- 
> YunQiang Su
>From c019327e919fff87ffa94799e8f521bda707a883 Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Sat, 24 Feb 2024 17:34:05 +0100
Subject: [PATCH] Regenerate opt.urls

There were several commits that didn't regenerate the opt.urls files.

Fixes: 438ef143679e ("rs6000: Neuter option -mpower{8,9}-vector")
Fixes: 50c549ef3db6 ("gccrs: enable -Winfinite-recursion warnings by default")
Fixes: 25bb8a40abd9 ("Move docs for -Wuse-after-free and -Wuseless-cast")
Fixes: 48448055fb70 ("AVR: Support .rodata in Flash for AVR64* and AVR128*")
Fixes: 42503cc257fb ("AVR: Document option -mskip-bug")
Fixes: 7de5bb642c12 ("i386: [APX] Document inline asm behavior and new switch")
Fixes: 49a14ee488b8 ("Add -mevex512 into invoke.texi")
Fixes: 4666cbde5e6d ("Sort warning options in c-family/c.opt.")

gcc/config/
* rs6000/rs6000.opt.urls: Regenerate.
* avr/avr.opt.urls: Likewise.
* i386/i386.opt.urls: Likewise.
* pru/pru.opt.urls: Likewise.
* riscv/riscv.opt.urls: Likewise.

gcc/rust/
* lang.opt.urls: Regenerate.

gcc/
* common.opt.urls: Regenerate.

gcc/c-family/
* c.opt.urls: Regenerate.
---
 gcc/c-family/c.opt.urls   | 351 +++---
 gcc/common.opt.urls   |   4 +-
 gcc/config/avr/avr.opt.urls   |   9 +
 gcc/config/i386/i386.opt.urls |   8 +-
 gcc/config/pru/pru.opt.urls   |   2 +-
 gcc/config/riscv/riscv.opt.urls   |   2 +-
 gcc/config/rs6000/rs6000.opt.urls |   3 -
 gcc/rust/lang.opt.urls|   3 +
 8 files changed, 200 insertions(+), 182 deletions(-)

diff --git a/gcc/c-family/c.opt.urls b/gcc/c-family/c.opt.urls
index 5365c8e2bc54..9f97dc61a778 100644
--- a/gcc/c-family/c.opt.urls
+++ b/gcc/c-family/c.opt.urls
@@ -88,6 +88,9 @@ UrlSuffix(gcc/Warning-Options.html#index-Wabsolute-value)
 Waddress
 UrlSuffix(gcc/Warning-Options.html#index-Waddress)
 
+Waddress-of-packed-member
+UrlSuffix(gcc/Warning-Options.html#index-Waddress-of-packed-member)
+
 Waligned-new
 UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Waligned-new)
 
@@ -115,6 +118,9 @@ UrlSuffix(gcc/Warning-Options.html#index-Walloc-zero)
 Walloca-larger-than=
 UrlSuffix(gcc/Warning-Options.html#index-Walloca-larger-than_003d) 
LangUrlSuffix_D(gdc/Warnings.html#index-Walloca-larger-than)
 
+Warith-conversion
+UrlSuffix(gcc/Warning-Options.html#index-Warith-conversion)
+
 Warray-bounds=
 UrlSuffix(gcc/Warning-Options.html#index-Warray-bounds)
 
@@ -122,13 +128,10 @@ Warray-compare
 UrlSuffix(gcc/Warning-Options.html#index-Warray-compare)
 
 Warray-parameter
-UrlSuffix(gcc/Warning-Options.html#index-Wno-array-parameter)
+UrlSuffix(gcc/Warning-Options.html#index-Warray-parameter)
 
 Warray-parameter=
-UrlSuffix(gcc/Warning-Options.html#index-Wno-array-parameter)
-
-Wzero-length-bounds
-UrlSuffix(gcc/Warning-Options.html#index-Wzero-length-bounds)
+UrlSuffix(gcc/Warning-Options.html#index-Warray-parameter)
 
 Wassign-intercept
 
UrlSuffix(gcc/Objective-C-and-Objective-C_002b_002b-Dialect-Options.html#index-Wassign-intercept)
@@ -148,9 +151,6 @@ UrlSuffix(gcc/Warning-Options.html#index-W

Re: [PATCH 0/4] v3 of: Option handling: add documentation URLs

[Mark Wielaard]

Hi David,

On Thu, Dec 14, 2023 at 10:01:39AM -0500, David Malcolm wrote:
> > Once your patch is in please feel free to sent an email to
> > build...@sourceware.org
> > https://sourceware.org/mailman/listinfo/buildbot
> > And we'll add the above build steps and update the autotools
> > Containerfile to include the fortran (gfortran?) and d (gdc?) build
> > dependencies.
> 
> Joseph: it seems that we have a way to add CI for this.
> 
> I refreshed the patches and successfully bootstrapped & regrtested them
> on x86_64-pc-linux-gnu; here's the v3 version of them.
> 
> Are these OK for trunk, assuming I followup with adding CI for this?
> (that said, I disappear for the rest of 2023 at the end of this week, so
> I'd work on the CI in early January)

I will be around next week to fixup any CI issues.
But once you commit this we can immediate activate the check.

I have attached a patch for the gcc-autoregen builder to also do
regenerate-opt-urls. Since it is a --disable-bootstrap build and uses
ccache it should take just a few minutes. So can be done on every
commit.

Note that with you patch applied to master it does flag and generate
the attached diff (I assume that is expected).

Cheers,

Mark>From 83914698dfb77a85496e93e3faa5de9131347cb8 Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Fri, 15 Dec 2023 01:43:27 +0100
Subject: [PATCH] Add regenerate-opt-urls to gcc-autoregen

Add gcc build dependencies and ccacheto Containerfile-autotools.
Add regenerate_opt_urls steps to gcc_autoregen_factory.
---
 builder/containers/Containerfile-autotools |  2 ++
 builder/master.cfg | 34 ++
 2 files changed, 36 insertions(+)

diff --git a/builder/containers/Containerfile-autotools 
b/builder/containers/Containerfile-autotools
index 1099986..8cc8a22 100644
--- a/builder/containers/Containerfile-autotools
+++ b/builder/containers/Containerfile-autotools
@@ -7,6 +7,8 @@ RUN apt-get update && \
 apt-get upgrade -y && \
 apt-get install -y \
   build-essential libtool perl python3-minimal bash \
+  libmpc-dev libgmp-dev libmpfr-dev gdc texinfo flex \
+  ccache \
   make git \
   gettext \
   m4 pkg-config \
diff --git a/builder/master.cfg b/builder/master.cfg
index 04c54da..0e5ce62 100644
--- a/builder/master.cfg
+++ b/builder/master.cfg
@@ -1200,6 +1200,35 @@ git_diff_step = steps.ShellCommand(
 name="git diff",
 haltOnFailure=True)
 
+# Note that the name of the default workdir is 'build'
+# 'build' is the name of the source checkout (yes, confusing).
+# So this creates 'build/objdir'.
+gcc_build_mkdir_step = steps.ShellCommand(
+command=['mkdir', '-p', 'objdir'],
+name='mkdir objdir',
+haltOnFailure=True)
+
+gcc_configure_opt_urls_step = steps.Configure(
+workdir='build/objdir',
+command=["../configure",
+ "--disable-multilib",
+ "--disable-bootstrap",
+ "--enable-languages=c,d,fortran"],
+name="configure",
+haltOnFailure=True)
+
+gcc_make_html_step = steps.Compile(
+workdir='build/objdir',
+command=['make', util.Interpolate('-j%(prop:ncpus)s'), 'html'],
+name='make html',
+haltOnFailure=True)
+
+gcc_make_regenerate_opt_urls_step = steps.Compile(
+workdir='build/objdir/gcc',
+command=['make', 'regenerate-opt-urls'],
+name='make regenerate-opt-urls',
+haltOnFailure=True)
+
 # Generic make clean step to be run at the end of a build
 make_clean_step = steps.ShellCommand(
 command=["make", "clean"],
@@ -3454,6 +3483,11 @@ gcc_autoregen_factory = util.BuildFactory()
 gcc_autoregen_factory.addStep(gcc_git_step)
 gcc_autoregen_factory.addStep(autoregen_step)
 gcc_autoregen_factory.addStep(git_diff_step)
+gcc_autoregen_factory.addStep(gcc_build_mkdir_step)
+gcc_autoregen_factory.addStep(gcc_configure_opt_urls_step)
+gcc_autoregen_factory.addStep(gcc_make_html_step)
+gcc_autoregen_factory.addStep(gcc_make_regenerate_opt_urls_step)
+gcc_autoregen_factory.addStep(git_diff_step)
 
 gcc_autoregen_builder = util.BuilderConfig(
 name="gcc-autoregen",
-- 
2.39.3

diff --git a/gcc/analyzer/analyzer.opt.urls b/gcc/analyzer/analyzer.opt.urls
index 9f7d33ff434..5fcab720582 100644
--- a/gcc/analyzer/analyzer.opt.urls
+++ b/gcc/analyzer/analyzer.opt.urls
@@ -48,6 +48,9 @@ 
UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-free-of-non-heap)
 Wanalyzer-imprecise-fp-arithmetic
 
UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-imprecise-fp-arithmetic)
 
+Wanalyzer-infinite-loop
+UrlSuffix(gcc/Stati

[COMMITTED] robots.txt: Disallow a few more bugzilla queries

[Mark Wielaard]

Some spiders are hitting bugzilla hard generating dependency trees
or graphs, downloading large attachements or requesting all bugs
in xml format. Disallow all that.
---
 htdocs/robots.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/htdocs/robots.txt b/htdocs/robots.txt
index b9fc830d..057c5899 100644
--- a/htdocs/robots.txt
+++ b/htdocs/robots.txt
@@ -10,4 +10,8 @@ Disallow: /cgit/
 Disallow: /svn
 Disallow: /cgi-bin/
 Disallow: /bugzilla/buglist.cgi
+Disallow: /bugzilla/show_bug.cgi*ctype=xml*
+Disallow: /bugzilla/attachment.cgi
+Disallow: /bugzilla/showdependencygraph.cgi
+Disallow: /bugzilla/showdependencytree.cgi
 Crawl-Delay: 60
-- 
2.39.3

[COMMITTED] Regenerate libgomp/configure for copyright year update

[Mark Wielaard]

commit a945c346f57ba40fc80c14ac59be0d43624e559d updated
libgomp/plugin/configfrag.ac but didn't regenerate/update
libgomp/configure which includes that configfrag.

libgomp/Changelog:

* configure: Regenerate.
---
 libgomp/configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libgomp/configure b/libgomp/configure
index c69a13cfe14..b3646c9936d 100755
--- a/libgomp/configure
+++ b/libgomp/configure
@@ -15159,7 +15159,7 @@ _ACEOF
 
 # Plugins for offload execution, configure.ac fragment.  -*- mode: autoconf -*-
 #
-# Copyright (C) 2014-2023 Free Software Foundation, Inc.
+# Copyright (C) 2014-2024 Free Software Foundation, Inc.
 #
 # Contributed by Mentor Embedded.
 #
-- 
2.39.3

[gcc-wwwdocs COMMITTED] Disallow /cgit for web robots

[Mark Wielaard]

Although cgit is more efficient than gitweb it still is not great
for bots to go through it.
---
 htdocs/robots.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/htdocs/robots.txt b/htdocs/robots.txt
index c650057b..b9fc830d 100644
--- a/htdocs/robots.txt
+++ b/htdocs/robots.txt
@@ -6,6 +6,7 @@ User-agent: *
 Disallow: /viewvc/
 Disallow: /viewcvs
 Disallow: /git/
+Disallow: /cgit/
 Disallow: /svn
 Disallow: /cgi-bin/
 Disallow: /bugzilla/buglist.cgi
-- 
2.39.3

Re: [PATCH 0/4] v2 of Option handling: add documentation URLs

[Mark Wielaard]

Hi David,

On Fri, Dec 08, 2023 at 06:35:56PM -0500, David Malcolm wrote:
> On Tue, 2023-11-21 at 23:43 +, Joseph Myers wrote:
> > On Tue, 21 Nov 2023, Tobias Burnus wrote:
> > 
> > > On 21.11.23 14:57, David Malcolm wrote:
> > > > On Tue, 2023-11-21 at 02:09 +0100, Hans-Peter Nilsson wrote:
> > > > > Sorry for barging in though I did try finding the relevant
> > > > > discussion, but is committing this generated stuff necessary?
> > > > > Is it because we don't want to depend on Python being
> > > > > present at build time?
> > > > Partly, yes, [...]
> > > 
> > > I wonder how to ensure that this remains up to date. Should there
> > > be an item at
> > > 
> > > https://gcc.gnu.org/branching.html and/or
> > > https://gcc.gnu.org/releasing.html similar to the .pot generation?
> >
> > My suggestion earlier in the discussion was that it should be
> > added to the post-commit CI discussed starting at
> >  (I
> > think that CI is now in operation).  These are generated files
> > that ought to be kept up to date with each commit that affects
> > .opt files, unlike the .pot files where the expectation is that
> > they should be up to date for releases and updated from time to
> > time at other times for submission to the TP.
> > 
> I had a go at scripting the testing of this, but I am terrible at shell
> scripts (maybe I should use Python?).  Here's what I have so far:
> 
> $ cat contrib/regenerate-index-urls.sh
> 
> set -x
> 
> SRC_DIR=$1
> BUILD_DIR=$2
> NUM_JOBS=$3
> 
> # FIXME: error-checking!
> 
> mkdir -p $BUILD_DIR || exit 1
> cd $BUILD_DIR
> $SRC_DIR/configure --disable-bootstrap --enable-languages=c,d,fortran || exit 
> 2
> make html-gcc -j$NUM_JOBS || exit 3
> cd gcc || exit 4
> make regenerate-opt-urls || exit 5
> cd $SRC_DIR
> (git diff $1 > /dev/null ) && echo "regenerate-opt-urls needs to be run and 
> the results committed" || exit 6
> 
> # e.g.
> #  time bash contrib/regenerate-index-urls.sh $(pwd) $(pwd)/../build-ci 64
> 
> This takes about 100 seconds of wallclock on my 64-core box (mostly
> configuring gcc, which as well as the usual sequence of unparallelized
> tests seems to require building libiberty and lto-plugin).  Is that
> something we want to do on every commit?  Is implementing the CI a
> blocker for getting the patches in? (if so, I'll likely need some help)

The CI builers don't have 64-cores, but a couple of hundred seconds
shouldn't be an issue to do on each commit (OSUOSL just got us a
second x86_64 container builder for larger jobs). The above can easily
be added to the existing gcc-autoregen builder:
https://builder.sourceware.org/buildbot/#/builders/gcc-autoregen
https://sourceware.org/cgit/builder/tree/builder/master.cfg#n3453

Once your patch is in please feel free to sent an email to
build...@sourceware.org
https://sourceware.org/mailman/listinfo/buildbot
And we'll add the above build steps and update the autotools
Containerfile to include the fortran (gfortran?) and d (gdc?) build
dependencies.

Cheers,

Mark

[COMMITTED] Regenerate libiberty/aclocal.m4 with aclocal 1.15.1

[Mark Wielaard]

There is a new buildbot check that all autotool files are generated
with the correct versions (automake 1.15.1 and autoconf 2.69).
https://builder.sourceware.org/buildbot/#/builders/gcc-autoregen

Correct one file that was generated with the wrong version.

libiberty/
* aclocal.m4: Rebuild.
---
 libiberty/aclocal.m4 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libiberty/aclocal.m4 b/libiberty/aclocal.m4
index f327865aaf9..0757688d52a 100644
--- a/libiberty/aclocal.m4
+++ b/libiberty/aclocal.m4
@@ -1,6 +1,6 @@
-# generated automatically by aclocal 1.16.5 -*- Autoconf -*-
+# generated automatically by aclocal 1.15.1 -*- Autoconf -*-
 
-# Copyright (C) 1996-2021 Free Software Foundation, Inc.
+# Copyright (C) 1996-2017 Free Software Foundation, Inc.
 
 # This file is free software; the Free Software Foundation
 # gives unlimited permission to copy and/or distribute it,
-- 
2.39.3

gcc-patches From rewriting mailman settings (Was: [Linaro-TCWG-CI] gcc patch #75674: FAIL: 68 regressions)

[Mark Wielaard]

Hi Maxim,

Adding Jeff to CC who is the official gcc-patches mailinglist admin.

On Tue, 2023-09-12 at 11:08 +0400, Maxim Kuvyrkov wrote:
> Normally, notifications from Linaro TCWG precommit CI are sent only to
> patch author and patch submitter.  In this case the sender was rewritten
> to "Benjamin Priour via Gcc-patches ",
> which was detected by Patchwork [1] as patch submitter.

BTW. Really looking forward to your talk at Cauldron about this!

> Is "From:" re-write on gcc-patches@ mailing list a side-effect of [2]?
> I see that some, but not all messages to gcc-patches@ have their
> "From:" re-written.
> 
> Also, do you know if re-write of "From:" on gcc-patches@ is expected?

Yes, it is expected for emails that come from domains with a dmarc
policy. That is because the current settings of the gcc-patches
mailinglist might slightly alter the message or headers in a way that
invalidates the DKIM signature. Without From rewriting those messages
would be bounced by recipients that check the dmarc policy/dkim
signature.

As you noticed the glibc hackers have recently worked together with the
sourceware overseers to upgrade mailman and alter the postfix and the
libc-alpha mailinglist setting so it doesn't require From rewriting
anymore (the message and header aren't altered anymore to invalidate
the DKIM signatures).

We (Jeff or anyone else with mailman admin privs) could use the same
settings for gcc-patches. The settings that need to be set are in that
bug:

- subject_prefix (general): (empty)
- from_is_list (general): No
- anonymous_list (general): No
- first_strip_reply_to (general): No
- reply_goes_to_list (general): Poster
- reply_to_address (general): (empty)
- include_sender_header (general): No
- drop_cc (general): No
- msg_header (nondigest): (empty)
- msg_footer (nondigest): (empty)
- scrub_nondigest (nondigest): No
- dmarc_moderation_action (privacy): Accept
- filter_content (contentfilter): No

The only visible change (apart from no more From rewriting) is that
HTML multi-parts aren't scrubbed anymore (that would be a message
altering issue). The html part is still scrubbed from the
inbox.sourceware.org archive, so b4 works just fine. But I don't know
what patchwork.sourceware.org does with HTML attachements. Of course
people really shouldn't sent HTML attachments to gcc-patches, so maybe
this is no real problem.

Let me know if you want Jeff (or me or one of the other overseers) make
the above changes to the gcc-patches mailman settings.

Cheers,

Mark

> [1] https://patchwork.sourceware.org/project/gcc/list/
> [2] https://sourceware.org/bugzilla/show_bug.cgi?id=29713

Re: gcc-patches From rewriting mailman settings (Was: [Linaro-TCWG-CI] gcc patch #75674: FAIL: 68 regressions)

[Mark Wielaard]

Hi Thomas,

On Thu, Sep 14, 2023 at 09:00:39AM +0200, Thomas Schwinge wrote:
> >> Let me know if you want Jeff (or me or one of the other overseers) make
> >> the above changes to the gcc-patches mailman settings.
> >
> > yes, please!
> 
> Yes, please!  For all mailing lists, globally.

Call me a little conservative, but lets do this in stages. Although I
believe this works as intended, it is email and email is finicky.
Lets try out gcc-patches first. Then if that works and nobody
complains in say two week switch over other lists that receive patches
(gcc-rust, libstdc++, fortran and jit). But maybe just keep the
discussion lists as is for now.

Cheers,

Mark

Re: gcc-patches From rewriting mailman settings (Was: [Linaro-TCWG-CI] gcc patch #75674: FAIL: 68 regressions)

[Mark Wielaard]

Hi Richard,

On Thu, Sep 14, 2023 at 11:07:21AM +0200, Richard Biener wrote:
> On Tue, Sep 12, 2023 at 5:00 PM Mark Wielaard  wrote:
> > The only visible change (apart from no more From rewriting) is that
> > HTML multi-parts aren't scrubbed anymore (that would be a message
> > altering issue). The html part is still scrubbed from the
> > inbox.sourceware.org archive, so b4 works just fine. But I don't know
> > what patchwork.sourceware.org does with HTML attachements. Of course
> > people really shouldn't sent HTML attachments to gcc-patches, so maybe
> > this is no real problem.
> 
> Ick (to the HTML part).  I wonder if we can use From rewriting for those,
> still stripping the HTML part?

It is possible to mark some domains, like gmail.com which is used by a
lot of HTML posters, to always use From rewriting. But it isn't easy
to do it for emails containing HTML parts (and strip them).

>  Maybe we can also go back to rejecting
> mails that are not text/plain ...

Sadly people do seem to use these web email providers like gmail a
lot. Although not many people sent patches as email, reviews of those
patches do sometimes contain HTML parts.

Sorry,

Mark

Re: gcc-patches From rewriting mailman settings (Was: [Linaro-TCWG-CI] gcc patch #75674: FAIL: 68 regressions)

[Mark Wielaard]

Hi all,

On Tue, Sep 12, 2023 at 05:00:07PM +0200, Mark Wielaard wrote:
> Adding Jeff to CC who is the official gcc-patches mailinglist admin.
> [...] 
> Yes, it is expected for emails that come from domains with a dmarc
> policy. That is because the current settings of the gcc-patches
> mailinglist might slightly alter the message or headers in a way that
> invalidates the DKIM signature. Without From rewriting those messages
> would be bounced by recipients that check the dmarc policy/dkim
> signature.
> 
> As you noticed the glibc hackers have recently worked together with the
> sourceware overseers to upgrade mailman and alter the postfix and the
> libc-alpha mailinglist setting so it doesn't require From rewriting
> anymore (the message and header aren't altered anymore to invalidate
> the DKIM signatures).
> 
> We (Jeff or anyone else with mailman admin privs) could use the same
> settings for gcc-patches. The settings that need to be set are in that
> bug:
> 
> - subject_prefix (general): (empty)
> - from_is_list (general): No
> - anonymous_list (general): No
> - first_strip_reply_to (general): No
> - reply_goes_to_list (general): Poster
> - reply_to_address (general): (empty)
> - include_sender_header (general): No
> - drop_cc (general): No
> - msg_header (nondigest): (empty)
> - msg_footer (nondigest): (empty)
> - scrub_nondigest (nondigest): No
> - dmarc_moderation_action (privacy): Accept
> - filter_content (contentfilter): No
> 
> The only visible change (apart from no more From rewriting) is that
> HTML multi-parts aren't scrubbed anymore (that would be a message
> altering issue). The html part is still scrubbed from the
> inbox.sourceware.org archive, so b4 works just fine. But I don't know
> what patchwork.sourceware.org does with HTML attachements. Of course
> people really shouldn't sent HTML attachments to gcc-patches, so maybe
> this is no real problem.

Although there were some positive responses (on list and on irc) it is
sometimes hard to know if there really is consensus for these kind of
infrastructure tweaks. But I believe there is at least no sustained
opposition to changing the gcc-patches mailman setting as proposed
above.

So unless someone objects I like to make this change Tuesday September
19 around 08:00 UTC.

And if there are no complaints at Cauldron we could do the same for
the other patch lists the week after.

Cheers,

Mark

> > [1] https://patchwork.sourceware.org/project/gcc/list/
> > [2] https://sourceware.org/bugzilla/show_bug.cgi?id=29713

Re: gcc-patches From rewriting mailman settings (Was: [Linaro-TCWG-CI] gcc patch #75674: FAIL: 68 regressions)

[Mark Wielaard]

Hi all,

On Sun, Sep 17, 2023 at 10:04:37PM +0200, Mark Wielaard wrote:
> > We (Jeff or anyone else with mailman admin privs) could use the same
> > settings for gcc-patches. The settings that need to be set are in that
> > bug:
> > 
> > - subject_prefix (general): (empty)
> > - from_is_list (general): No
> > - anonymous_list (general): No
> > - first_strip_reply_to (general): No
> > - reply_goes_to_list (general): Poster
> > - reply_to_address (general): (empty)
> > - include_sender_header (general): No
> > - drop_cc (general): No
> > - msg_header (nondigest): (empty)
> > - msg_footer (nondigest): (empty)
> > - scrub_nondigest (nondigest): No
> > - dmarc_moderation_action (privacy): Accept
> > - filter_content (contentfilter): No
> > 
> > The only visible change (apart from no more From rewriting) is that
> > HTML multi-parts aren't scrubbed anymore (that would be a message
> > altering issue). The html part is still scrubbed from the
> > inbox.sourceware.org archive, so b4 works just fine. But I don't know
> > what patchwork.sourceware.org does with HTML attachements. Of course
> > people really shouldn't sent HTML attachments to gcc-patches, so maybe
> > this is no real problem.
> 
> Although there were some positive responses (on list and on irc) it is
> sometimes hard to know if there really is consensus for these kind of
> infrastructure tweaks. But I believe there is at least no sustained
> opposition to changing the gcc-patches mailman setting as proposed
> above.
> 
> So unless someone objects I like to make this change Tuesday September
> 19 around 08:00 UTC.

This change is now done for gcc-patches.

> And if there are no complaints at Cauldron we could do the same for
> the other patch lists the week after.
> 
> > > [1] https://patchwork.sourceware.org/project/gcc/list/
> > > [2] https://sourceware.org/bugzilla/show_bug.cgi?id=29713

[COMMITTED] Regenerate c.opt.urls

[Mark Wielaard]

Hi,

On Wed, 2024-07-17 at 13:55 +0200, Mark Wielaard wrote:
> On Sun, 2024-07-14 at 15:31 +0200, Alejandro Colomar wrote:
> > On Sun, Jul 14, 2024 at 01:37:02PM GMT, Jonathan Wakely wrote:
> > > On Sun, 14 Jul 2024, 12:30 Alejandro Colomar via Gcc-help, <
> > > gcc-h...@gcc.gnu.org> wrote:
> > > > Did I break anything?  I see the failure being `git diff --exit-code`,
> > > > which doesn't seem like anything broken, but I don't know what that test
> > > > is for, so I'll ask.
> > > > 
> > > 
> > > It checks that necessary auto-generated files have been regenerated and
> > > committed.
> > > 
> > > If you didn't do anything related to that warning option that would have
> > > affected the c.opts.urls file then it wasn't you (I think it was a change
> > > from Marek).
> > 
> > I did add that warning option:
> > 
> > commit 44c9403ed1833ae71a59e84f9e37af3182be0df5
> > Author: Alejandro Colomar 
> > AuthorDate: Sat Jun 29 15:10:43 2024 +0200
> > Commit: Martin Uecker 
> > CommitDate: Sun Jul 14 11:41:00 2024 +0200
> > 
> > c, objc: Add -Wunterminated-string-initialization
> > 
> > Warn about the following:
> > 
> > char  s[3] = "foo";
> > 
> > I guess I should have committed some re-generated files?  Is that
> > documented somewhere?
> 
> Adding David Malcolm to CC who might know where this is documented.
> 
> But yes, after adding a new warning option you should run:
> 
> make html && cd gcc && make regenerate-opt-urls
> 
> This will produce the needed opt.url changes you should commit together
> with your change. In this case:
> 
> diff --git a/gcc/c-family/c.opt.urls b/gcc/c-family/c.opt.urls
> index 1b60ae4847b..df5f58a1eee 100644
> --- a/gcc/c-family/c.opt.urls
> +++ b/gcc/c-family/c.opt.urls
> @@ -870,6 +870,9 @@ 
> UrlSuffix(gcc/Warning-Options.html#index-Wno-unknown-pragmas) 
> LangUrlSuffix_D(gd
>  Wunsuffixed-float-constants
>  UrlSuffix(gcc/Warning-Options.html#index-Wno-unsuffixed-float-constants)
>  
> +Wunterminated-string-initialization
> +UrlSuffix(gcc/Warning-Options.html#index-Wno-unterminated-string-initialization)
> +
>  Wunused
>  UrlSuffix(gcc/Warning-Options.html#index-Wno-unused)

I made sure that was also generated locally and pushed the attached to
make the autoregen buildbot happy.

Cheers,

Mark
From bdb4db1ec966c974b9b7bf5e3d2edda93d8635aa Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Wed, 17 Jul 2024 17:58:14 +0200
Subject: [PATCH] Regenerate c.opt.urls

The addition of -Wunterminated-string-initialization should have
regenerated the c.opt.urls file.

Fixes: 44c9403ed183 ("c, objc: Add -Wunterminated-string-initialization")

gcc/c-family/ChangeLog:

	* c.opt.urls: Regenerate.
---
 gcc/c-family/c.opt.urls | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/c-family/c.opt.urls b/gcc/c-family/c.opt.urls
index 1b60ae4847b1..df5f58a1eeed 100644
--- a/gcc/c-family/c.opt.urls
+++ b/gcc/c-family/c.opt.urls
@@ -870,6 +870,9 @@ UrlSuffix(gcc/Warning-Options.html#index-Wno-unknown-pragmas) LangUrlSuffix_D(gd
 Wunsuffixed-float-constants
 UrlSuffix(gcc/Warning-Options.html#index-Wno-unsuffixed-float-constants)
 
+Wunterminated-string-initialization
+UrlSuffix(gcc/Warning-Options.html#index-Wno-unterminated-string-initialization)
+
 Wunused
 UrlSuffix(gcc/Warning-Options.html#index-Wno-unused)
 
-- 
2.45.2

[COMMITTED] Regenerate riscv.opt.urls and i386.opt.urls

[Mark Wielaard]

risc-v added an -mfence-tso option. i386 removed Xeon Phi ISA support
options. But the opt.urls files weren't regenerated.

Fixes: a6114c2a6911 ("RISC-V: Implement -m{,no}fence-tso")
Fixes: e1a7e2c54d52 ("i386: Remove Xeon Phi ISA support")

gcc/ChangeLog:

* config/riscv/riscv.opt.urls: Regenerate.
* config/i386/i386.opt.urls: Likewise.
---
 gcc/config/i386/i386.opt.urls   | 15 ---
 gcc/config/riscv/riscv.opt.urls |  3 +++
 2 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/gcc/config/i386/i386.opt.urls b/gcc/config/i386/i386.opt.urls
index 81c5bb9a9270..40e8a8449367 100644
--- a/gcc/config/i386/i386.opt.urls
+++ b/gcc/config/i386/i386.opt.urls
@@ -238,12 +238,6 @@ UrlSuffix(gcc/x86-Options.html#index-mavx2)
 mavx512f
 UrlSuffix(gcc/x86-Options.html#index-mavx512f)
 
-mavx512pf
-UrlSuffix(gcc/x86-Options.html#index-mavx512pf)
-
-mavx512er
-UrlSuffix(gcc/x86-Options.html#index-mavx512er)
-
 mavx512cd
 UrlSuffix(gcc/x86-Options.html#index-mavx512cd)
 
@@ -262,12 +256,6 @@ UrlSuffix(gcc/x86-Options.html#index-mavx512ifma)
 mavx512vbmi
 UrlSuffix(gcc/x86-Options.html#index-mavx512vbmi)
 
-mavx5124fmaps
-UrlSuffix(gcc/x86-Options.html#index-mavx5124fmaps)
-
-mavx5124vnniw
-UrlSuffix(gcc/x86-Options.html#index-mavx5124vnniw)
-
 mavx512vpopcntdq
 UrlSuffix(gcc/x86-Options.html#index-mavx512vpopcntdq)
 
@@ -409,9 +397,6 @@ UrlSuffix(gcc/x86-Options.html#index-mrdrnd)
 mf16c
 UrlSuffix(gcc/x86-Options.html#index-mf16c)
 
-mprefetchwt1
-UrlSuffix(gcc/x86-Options.html#index-mprefetchwt1)
-
 mfentry
 UrlSuffix(gcc/x86-Options.html#index-mfentry)
 
diff --git a/gcc/config/riscv/riscv.opt.urls b/gcc/config/riscv/riscv.opt.urls
index 2f01ae5d6271..e02ef3ee3dd9 100644
--- a/gcc/config/riscv/riscv.opt.urls
+++ b/gcc/config/riscv/riscv.opt.urls
@@ -91,3 +91,6 @@ UrlSuffix(gcc/RISC-V-Options.html#index-minline-strlen)
 
 ; skipping UrlSuffix for 'mtls-dialect=' due to finding no URLs
 
+mfence-tso
+UrlSuffix(gcc/RISC-V-Options.html#index-mfence-tso)
+
-- 
2.45.1

Re: [COMMITTED] Regenerate riscv.opt.urls and i386.opt.urls

[Mark Wielaard]

Hi,

On Mon, 2024-05-20 at 12:37 -0400, David Malcolm wrote:
> On Mon, 2024-05-20 at 16:19 +, Jiang, Haochen wrote:
> > Thanks for your help! I haven't noticed this file is newly added to
> > GCC.
> > I suppose that is why the buildbot is reporting something the whole
> > afternoon for me.
> > 
> > So just for confirm, does that mean we will always need to run
> > gcc/regenerate-opt-urls.py after adding or removing options in GCC?
> > My current understanding is yes.
> 
> Yes please (and make sure you've got a clean build of the HTML docs
> with the new options added when you do)
> 
> Though if you forget, the only problem will be some missing URLs at the
> command line, and complaints from the CI.

Also note that the CI will provide a diff that is most likely the
correct patch you need to apply. e.g. for the last issue:
https://builder.sourceware.org/buildbot/#/builders/269/builds/5194/steps/8/logs/stdio

It is still recommended you run regenerate-opt-urls yourself. But if
you just quickly want to shut up the buildbot then you cannot really go
wrong with just applying the diff it generated for you.

Cheers,

Mark

Re: [committed v2 0/2] VAX: Fix issues with FP format option documentation

[Mark Wielaard]

Hi Maciej (Hi David, added to CC),

On Mon, 2024-05-27 at 05:19 +0100, Maciej W. Rozycki wrote:
>  As reported in PR target/79646 and fixed by a change proposed by Abe we 
> have a couple of issues with the descriptions of the VAX floating-point 
> format options in the option definition file.  Additionally most of these 
> options are not documented in the manual.
> 
>  This mini patch series addresses these issues, including Abe's change, 
> slightly updated, and my new change.  See individual change descriptions 
> for details.
> 
>  Verified by inspecting output produced by `vax-netbsdelf-gcc -v --help' 
> and by eyeballing `gcc.info' and `gcc.pdf' files produced.  Committed.

This broke the gcc-autoregen checker because the
gcc/config/vax/vax.opt.urls file wasn't regenerated:
https://builder.sourceware.org/buildbot/#/builders/269/builds/5347

Producing the following diff:

diff --git a/gcc/config/vax/vax.opt.urls b/gcc/config/vax/vax.opt.urls
index c6b1c418b61..ca78b31dd4c 100644
--- a/gcc/config/vax/vax.opt.urls
+++ b/gcc/config/vax/vax.opt.urls
@@ -1,7 +1,13 @@
 ; Autogenerated by regenerate-opt-urls.py from gcc/config/vax/vax.opt and 
generated HTML
 
+; skipping UrlSuffix for 'md' due to finding no URLs
+
+; skipping UrlSuffix for 'md-float' due to finding no URLs
+
 ; skipping UrlSuffix for 'mg' due to finding no URLs
 
+; skipping UrlSuffix for 'mg-float' due to finding no URLs
+
 ; skipping UrlSuffix for 'mgnu' due to finding no URLs
 
 ; skipping UrlSuffix for 'munix' due to finding no URLs

I am not completely clear on why though. Since it seems you actually
did add documentation for exactly these options.

David, should the above diff just be checked in, or do we need to
investigate why the URLs weren't found?

Cheers,

Mark

Re: [PATCH] lra: emit caller-save register spills before call insn [PR116028]

[Mark Wielaard]

Hi,

On Thu, Aug 08, 2024 at 02:07:50PM -0700, Andrew Pinski wrote:
> On Fri, Aug 2, 2024 at 7:30 AM Jeff Law  wrote:
> > > 2024-08-01  Surya Kumari Jangala  
> > >
> > > gcc/
> > >   PR rtl-optimization/PR116028
> > >   * lra-constraints.cc (split_reg): Spill register before call
> > >   insn.
> > >   (latest_call_insn): New variable.
> > >   (inherit_in_ebb): Track the latest call insn.
> > >
> > > gcc/testsuite/
> > >   PR rtl-optimization/PR116028
> > >   * gcc.dg/ira-shrinkwrap-prep-1.c: Remove xfail for powerpc.
> > >   * gcc.dg/pr10474.c: Remove xfail for powerpc.
> > Implementation looks fine.  I would suggest a comment indicating why
> > we're inserting before last_call_insn.  Otherwise someone in the future
> > would have to find the patch submission to know why we're handling that
> > case specially.
> >
> > OK with that additional comment.
> 
> This causes bootstrap failure on aarch64-linux-gnu; self-tests fail at
> stage 2. Looks to be wrong code is produced compiling stage 2
> compiler.
> I have not looked further than that right now.

There are also risc-v bootstrap comparison failures:

Comparing stages 2 and 3
Bootstrap comparison failure!
gcc/gimple-pretty-print.o differs
gcc/tree.o differs
gcc/fortran/trans.o differs
gcc/fortran/openmp.o differs
gcc/fortran/scanner.o differs
gcc/stor-layout.o differs
gcc/range-op-ptr.o differs
gcc/tree-vect-loop.o differs
make[2]: *** [Makefile:26283: compare] Error 1
make[1]: *** [Makefile:26263: stage3-bubble] Error 2
make: *** [Makefile:1102: all] Error 2

I haven't bisected it yet, but it started with a build that included
this patch:
https://builder.sourceware.org/buildbot/#/builders/310/builds/36

Cheers,

Mark

Re: [PATCH] lra: emit caller-save register spills before call insn [PR116028]

[Mark Wielaard]

Hi,

On Fri, Aug 09, 2024 at 08:15:31PM -0700, Andrew Pinski wrote:
> I had been wondering the same until I looked into it earlier today.
> Linaro CI's does `--disable-bootstrap` and there was no extra
> testsuite failures with the patch.
> So Linaro CI's is not catching all the bugs that a developer would
> catch in the end. Because bootstrap is one of the normal requirements;
> though usually only on one target.

Sam pointed out the same about the gcc arm64 build on
builder.sourceware.org. We enabled full bootrap and tests for that
architecture now.

fedora rawhide x86_64 (*), debian stable amd64, debian stable arm64,
fedora riscv and gentoo sparc.

https://builder.sourceware.org/buildbot/#/builders?tags=gcc-full

Note that these builds don't sent any email atm (they probably should
on build failure), so you would have to check by hand after commit
(after a a couple of hours). Test results are uploaded to bunsen.

Cheers,

Mark

(*) that one seems to loose a connection after a couple of ours
during the testsuite run though.

[PATCH] Restrict pr116202-run-1.c test to riscv_v target

[Mark Wielaard]

The testcase uses -march=rv64gcv and dg-do run, so should be
restricted to a riscv_v target.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/rvv/base/pr116202-run-1.c (dg-do run):
Add target riscv_v.
---
 gcc/testsuite/gcc.target/riscv/rvv/base/pr116202-run-1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr116202-run-1.c 
b/gcc/testsuite/gcc.target/riscv/rvv/base/pr116202-run-1.c
index 02814183dbb9..979989f8a857 100644
--- a/gcc/testsuite/gcc.target/riscv/rvv/base/pr116202-run-1.c
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr116202-run-1.c
@@ -1,4 +1,4 @@
-/* { dg-do run } */
+/* { dg-do run { target { riscv_v } } } */
 /* { dg-options "-O3 -march=rv64gcv_zvl256b -mabi=lp64d 
-fdump-rtl-expand-details" } */
 
 int b[24];
-- 
2.45.2

[COMMITTED] Regenerate avr.opt.urls

[Mark Wielaard]

avr added an -mlra option, but the avr.opt.url file wasn't
regenerated.

Note that commit 149a23ee2568 ("AVR: -mlra is not documeted in TEXI.")
did add the Undocumented flag, but that still needs the avr.op.urls
file to be updated.

Fixes: 09a87ea666b2 ("AVR: ad target/113934 - Add option -mlra to enable LRA.")

gcc/ChangeLog:

* config/avr/avr.opt.urls: Regenerate.
---
 gcc/config/avr/avr.opt.urls | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gcc/config/avr/avr.opt.urls b/gcc/config/avr/avr.opt.urls
index f38e67384ab1..6acc418b407d 100644
--- a/gcc/config/avr/avr.opt.urls
+++ b/gcc/config/avr/avr.opt.urls
@@ -1,5 +1,7 @@
 ; Autogenerated by regenerate-opt-urls.py from gcc/config/avr/avr.opt and 
generated HTML
 
+; skipping UrlSuffix for 'mlra' due to finding no URLs
+
 mcall-prologues
 UrlSuffix(gcc/AVR-Options.html#index-mcall-prologues)
 
-- 
2.45.2

[COMMITTED] Regenerate c.opt.urls

[Mark Wielaard]

Fixes: df7bfdb7dbf2 ("c++: reference cast, conversion fn [PR113141]")

A new warning option -Wcast-user-defined was added to c.opt and
documented in doc/invoke.texi. But c.opt.urls wasn't regenerate.

gcc/c-family/ChangeLog:

* c.opt.urls: Regenerate.
---
 gcc/c-family/c.opt.urls | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/c-family/c.opt.urls b/gcc/c-family/c.opt.urls
index 631719863a5e..dd455d7c0dc7 100644
--- a/gcc/c-family/c.opt.urls
+++ b/gcc/c-family/c.opt.urls
@@ -208,6 +208,9 @@ 
UrlSuffix(gcc/Warning-Options.html#index-Wcast-function-type)
 Wcast-qual
 UrlSuffix(gcc/Warning-Options.html#index-Wcast-qual)
 
+Wcast-user-defined
+UrlSuffix(gcc/Warning-Options.html#index-Wcast-user-defined)
+
 Wcatch-value
 UrlSuffix(gcc/C_002b_002b-Dialect-Options.html#index-Wcatch-value)
 
-- 
2.39.3

[COMMITTED] Regenerate cygming.opt.urls and mingw.opt.urls

[Mark Wielaard]

The new cygming.opt.urls and mingw.opt.urls in the
gcc/config/mingw/cygming.opt.urls directory need to generated by make
regenerate-opt-urls in the gcc subdirectory. They still contained
references to the gcc/config/i386 directory from which they were
copied.

Fixes: 1f05dfc131c7 ("Reuse MinGW from i386 for AArch64")
Fixes: e8d003736e6c ("Rename "x86 Windows Options" to "Cygwin and MinGW 
Options"")

gcc/ChangeLog:

* config/mingw/cygming.opt.urls: Regenerate.
* config/mingw/mingw.opt.urls: Likewise.
---
 gcc/config/mingw/cygming.opt.urls | 7 +++
 gcc/config/mingw/mingw.opt.urls   | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/gcc/config/mingw/cygming.opt.urls 
b/gcc/config/mingw/cygming.opt.urls
index c624e22e4427..af11c4997609 100644
--- a/gcc/config/mingw/cygming.opt.urls
+++ b/gcc/config/mingw/cygming.opt.urls
@@ -1,4 +1,4 @@
-; Autogenerated by regenerate-opt-urls.py from gcc/config/i386/cygming.opt and 
generated HTML
+; Autogenerated by regenerate-opt-urls.py from gcc/config/mingw/cygming.opt 
and generated HTML
 
 mconsole
 UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mconsole)
@@ -9,9 +9,8 @@ UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mdll)
 mnop-fun-dllimport
 UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mnop-fun-dllimport)
 
-; skipping UrlSuffix for 'mthreads' due to multiple URLs:
-;   duplicate: 'gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1'
-;   duplicate: 'gcc/x86-Options.html#index-mthreads'
+mthreads
+UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1)
 
 mwin32
 UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mwin32)
diff --git a/gcc/config/mingw/mingw.opt.urls b/gcc/config/mingw/mingw.opt.urls
index f8ee5be6a535..40fb086606b2 100644
--- a/gcc/config/mingw/mingw.opt.urls
+++ b/gcc/config/mingw/mingw.opt.urls
@@ -1,4 +1,4 @@
-; Autogenerated by regenerate-opt-urls.py from gcc/config/i386/mingw.opt and 
generated HTML
+; Autogenerated by regenerate-opt-urls.py from gcc/config/mingw/mingw.opt and 
generated HTML
 
 mcrtdll=
 UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mcrtdll)
-- 
2.39.3

Re: [EXTERNAL] [COMMITTED] Regenerate cygming.opt.urls and mingw.opt.urls

[Mark Wielaard]

Hi Evgeny,

Adding David to the CC, who might know the details.

On Mon, May 13, 2024 at 08:44:12AM +, Evgeny Karpov wrote:
> Sunday, May 12, 2024
>
> Thank you for reviewing our changes related to the refactoring of
> extracting the MinGW implementation from ix64.
>
> It was expected to move the MinGW-related files without changes in
> this commit ("Reuse MinGW from i386 for AArch64") and apply the
> renaming in a follow-up commit, which has been done in 'Rename "x86
> Windows Options" to "Cygwin and MinGW Options"'.
>
> The script to update opt.urls files has been used.
> 
> > diff --git a/gcc/config/mingw/cygming.opt.urls
> > b/gcc/config/mingw/cygming.opt.urls
> > index c624e22e4427..af11c4997609 100644
> > --- a/gcc/config/mingw/cygming.opt.urls
> > +++ b/gcc/config/mingw/cygming.opt.urls
> > @@ -1,4 +1,4 @@
> 
> > -; Autogenerated by regenerate-opt-urls.py from gcc/config/i386/cygming.opt
> > and generated HTML
> > +; Autogenerated by regenerate-opt-urls.py from
> > +gcc/config/mingw/cygming.opt and generated HTML
> 
> I am not sure why this comment has not been updated. Is it critical
> or it could be updated next time when it is needed?

Odd that the script didn't update this comment, it really should have.
It might be that running the script through make regenerate-opt-urls
inside the gcc build subdir invokes regenerate-opt-urls.py slightly
differently so that this line is updated.

> >  mconsole
> >  UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mconsole)
> > @@ -9,9 +9,8 @@ UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-
> > mdll)
> >  mnop-fun-dllimport
> >  UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mnop-fun-dllimport)
> > 
> > -; skipping UrlSuffix for 'mthreads' due to multiple URLs:
> > -;   duplicate: 'gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1'
> > -;   duplicate: 'gcc/x86-Options.html#index-mthreads'
> > +mthreads
> > +UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1)
> 
> mthreads has the same issue before applying changes. Has something been 
> changed recently?
> This is the change in patch series in 'Rename "x86 Windows Options" to 
> "Cygwin and MinGW Options"' commit.
> 
> ; skipping UrlSuffix for 'mthreads' due to multiple URLs:
> +;   duplicate: 'gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1'
>  ;   duplicate: 'gcc/x86-Options.html#index-mthreads'
> -;   duplicate: 'gcc/x86-Windows-Options.html#index-mthreads-1'

Again, it might be caused by invoking the script by hand vs with make
regenerate-opt-urls.py. I believe with the make option it will
renumber the suffixes making sure the urls are unique.

BTW. There is a CI buildbot that tries to regenerate all generated
files, which is how I spotted this:
https://builder.sourceware.org/buildbot/#/builders/gcc-autoregen
(It should also sent email to the author of the patch on failure.)

Cheers,

Mark

Re: [wwwdocs,Java] Replace sources.redhat.com by sourceware.org

[Mark Wielaard]

On Tue, Oct 23, 2012 at 10:52:41AM +0100, Andrew Haley wrote:
> On 10/23/2012 10:47 AM, Andrew Hughes wrote:
> > It's never been obvious to me how the web material gets updated.  GCJ
> > regularly misses out on being mentioned in changes too, despite fixes going 
> > in.
> 
> Web material gets updated with patches through the same process
> as the software.

Only thing to realize is that docs are still in CVS, not SVN:
http://gcc.gnu.org/cvs.html

For more background see also:
http://gcc.gnu.org/contribute.html#webchanges
http://gcc.gnu.org/projects/web.html

Cheers,

Mark

[PATCH] PR28901 Add two levels for -Wunused-const-variable.

[Mark Wielaard]

There is some controversy about enabling -Wunused-const-variable for all
unused static const variables because some feel there are too many errors
exposed in header files. Create two levels for -Wunused-const-variable.
One level to only check for unused static const variables in the main
compilation file. Which is enabled by -Wunused-variable. And a second
level that also checks for unused static const variables in included
header files. Which must be explicitly enabled.

gcc/ChangeLog

PR c/28901
* cgraphunit.c (check_global_declaration): Check level of
warn_unused_const_variable and main_input_filename.
* doc/invoke.texi (Warning Options): Add -Wunused-const-variable=.
(-Wunused-variable): For C implies -Wunused-const-variable=1.
(-Wunused-const-variable): Explain levels 1 and 2.

gcc/c-family/ChangeLog

PR c/28901
* c.opt (Wunused-const-variable): Turn into Alias for...
(Wunused-const-variable=): New option.

gcc/testsuite/ChangeLog

PR c/28901
* gcc.dg/unused-variable-3.c: New test.
---
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 638e9c2..7c5f6c7 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -949,7 +949,11 @@ C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wunused)
 ; documented in common.opt
 
 Wunused-const-variable
-C ObjC C++ ObjC++ Var(warn_unused_const_variable) Warning LangEnabledBy(C 
ObjC,Wunused-variable)
+C ObjC C++ ObjC++ Warning Alias(Wunused-const-variable=, 2, 0)
+Warn when a const variable is unused.
+
+Wunused-const-variable=
+C ObjC C++ ObjC++ Joined RejectNegative UInteger 
Var(warn_unused_const_variable) Warning LangEnabledBy(C ObjC,Wunused-variable, 
1, 0)
 Warn when a const variable is unused.
 
 Wvariadic-macros
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 0a745f0..27a073a 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -942,7 +942,10 @@ check_global_declaration (symtab_node *snode)
   /* Warn about static fns or vars defined but not used.  */
   if (((warn_unused_function && TREE_CODE (decl) == FUNCTION_DECL)
|| (((warn_unused_variable && ! TREE_READONLY (decl))
-   || (warn_unused_const_variable && TREE_READONLY (decl)))
+   || (warn_unused_const_variable > 0 && TREE_READONLY (decl)
+   && (warn_unused_const_variable == 2
+   || filename_cmp (main_input_filename,
+DECL_SOURCE_FILE (decl)) == 0)))
   && TREE_CODE (decl) == VAR_DECL))
   && ! DECL_IN_SYSTEM_HEADER (decl)
   && ! snode->referred_to_p (/*include_self=*/false)
@@ -971,7 +974,7 @@ check_global_declaration (symtab_node *snode)
(TREE_CODE (decl) == FUNCTION_DECL)
? OPT_Wunused_function
: (TREE_READONLY (decl)
-  ? OPT_Wunused_const_variable
+  ? OPT_Wunused_const_variable_
   : OPT_Wunused_variable),
"%qD defined but not used", decl);
 }
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index c1ab788..490df93 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -303,7 +303,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wunsuffixed-float-constants  -Wunused  -Wunused-function @gol
 -Wunused-label  -Wunused-local-typedefs -Wunused-parameter @gol
 -Wno-unused-result -Wunused-value @gol -Wunused-variable @gol
--Wunused-const-variable @gol
+-Wunused-const-variable -Wunused-const-variable=@var{n} @gol
 -Wunused-but-set-parameter -Wunused-but-set-variable @gol
 -Wuseless-cast -Wvariadic-macros -Wvector-operation-performance @gol
 -Wvla -Wvolatile-register-var  -Wwrite-strings @gol
@@ -4231,23 +4231,39 @@ its return value. The default is 
@option{-Wunused-result}.
 @opindex Wunused-variable
 @opindex Wno-unused-variable
 Warn whenever a local or static variable is unused aside from its
-declaration. This option implies @option{-Wunused-const-variable} for C,
+declaration. This option implies @option{-Wunused-const-variable=1} for C,
 but not for C++. This warning is enabled by @option{-Wall}.
 
 To suppress this warning use the @code{unused} attribute
 (@pxref{Variable Attributes}).
 
 @item -Wunused-const-variable
+@itemx -Wunused-const-variable=@var{n}
 @opindex Wunused-const-variable
 @opindex Wno-unused-const-variable
 Warn whenever a constant static variable is unused aside from its declaration.
-This warning is enabled by @option{-Wunused-variable} for C, but not for C++.
-In C++ this is normally not an error since const variables take the place of
-@code{#define}s in C++.
+@option{-Wunused-const-variable=1} is enabled by @option{-Wunused-variable}
+for C, but not for C++. In C this declares variable storage, but in C++ this
+is not an error since const variables take the place of @code{#define}s.
 
 To suppress this warning use the @code{unused} attribute
 (@pxref{Variable Attributes}).
 
+@table @gcctabopt
+@item -Wunused-const-variable=1
+This is the warnin

Re: [PATCH] PR28901 Add two levels for -Wunused-const-variable.

[Mark Wielaard]

On Mon, Feb 22, 2016 at 11:57:56AM -0700, Jeff Law wrote:
> On 02/20/2016 06:42 PM, Mark Wielaard wrote:
> Note that given the discussion in the BZ, I'm going to consider this a
> regression and thus eligible for the trunk.

Thanks. Unfortunately new warnings always seem to make some people
unhappy (even when others are happy and see them as useful). Hopefully
this compromise makes it so that nobody sees this warning as regression.

> >diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> >index 0a745f0..27a073a 100644
> >--- a/gcc/cgraphunit.c
> >+++ b/gcc/cgraphunit.c
> >@@ -971,7 +974,7 @@ check_global_declaration (symtab_node *snode)
> > (TREE_CODE (decl) == FUNCTION_DECL)
> > ? OPT_Wunused_function
> > : (TREE_READONLY (decl)
> >-   ? OPT_Wunused_const_variable
> >+   ? OPT_Wunused_const_variable_
> Typo here?
> 
> If that's not a typo, then just say so and this is approved.

As Jakub already explained that was deliberate. It is how a warning
option that can take a level is represented. Pushed.

Thanks,

Mark

Re: [PATCH] PR28901 Add two levels for -Wunused-const-variable.

[Mark Wielaard]

On Mon, 2016-02-22 at 19:20 -0800, H.J. Lu wrote:
> It caused:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69911

Apologies. Apparently main_input_filename can be NULL. I am not entirely
sure when that happens. Or how I failed to see that test failure. I
think I didn't have java enabled, causing libffi to be skipped.

I am testing this patch that skips the test in that case:

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 49f6c25..788f07e 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2016-02-23  Mark Wielaard  
+
+   PR c/69911
+   * cgraphunit.c (check_global_declaration): Check main_input_filename
+   is not NULL.
+
 2016-02-20  Mark Wielaard  
 
PR c/28901
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 27a073a..6c14be2 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -944,8 +944,9 @@ check_global_declaration (symtab_node *snode)
|| (((warn_unused_variable && ! TREE_READONLY (decl))
|| (warn_unused_const_variable > 0 && TREE_READONLY (decl)
&& (warn_unused_const_variable == 2
-   || filename_cmp (main_input_filename,
-DECL_SOURCE_FILE (decl)) == 0)))
+   || (main_input_filename != NULL
+   && filename_cmp (main_input_filename,
+DECL_SOURCE_FILE (decl)) == 0
   && TREE_CODE (decl) == VAR_DECL))
   && ! DECL_IN_SYSTEM_HEADER (decl)
   && ! snode->referred_to_p (/*include_self=*/false)

Re: [PATCH] PR28901 Add two levels for -Wunused-const-variable.

[Mark Wielaard]

On Tue, 2016-02-23 at 09:26 +0100, Jakub Jelinek wrote:
> On Tue, Feb 23, 2016 at 08:55:40AM +0100, Mark Wielaard wrote:
> > On Mon, 2016-02-22 at 19:20 -0800, H.J. Lu wrote:
> > > It caused:
> > > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69911
> > 
> > Apologies. Apparently main_input_filename can be NULL. I am not entirely
> > sure when that happens. Or how I failed to see that test failure. I
> > think I didn't have java enabled, causing libffi to be skipped.
> > 
> > I am testing this patch that skips the test in that case:
> 
> Are you sure that is the problem?

No :) I was still bootstrapping. I did see it didn't solve the issue but
hadn't understood why yet...

> I think it doesn't hurt to check for non-NULL main_input_filename, perhaps
> some non-c-family languages might not set it, and this is in generic code,
> but at least on the gcc.target/i386/iamcu/test_passing_structs.c testcase and 
> on one
> randomly selected libffi testcase I see the ICE from completely different
> reason - what is NULL is DECL_SOURCE_FILE (decl).

Thanks, that makes more sense than my first hypothesis.

> We are not really going to warn about this anyway, e.g. because
> it is DECL_ARTIFICIAL, but that is checked only in a much later
> condition.  So, I think you should check also for NULL DECL_SOURCE_FILE
> (and treat it as possibly in a header).  Unfortunately
> DECL_SOURCE_FILE involves a function call, so you might want to cache
> it in some automatic variable.
>   && (warn_unused_const_variable == 2
>   || (main_input_filename != NULL
>   && (decl_file = DECL_SOURCE_FILE (decl)) != NULL
>   && filename_cmp (main_input_filename,
>decl_file) == 0

That looks right. Now testing:

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 49f6c25..e9e1aab 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2016-02-23  Mark Wielaard  
+   Jakub Jelinek  
+
+   PR c/69911
+   * cgraphunit.c (check_global_declaration): Check main_input_filename
+   and DECL_SOURCE_FILE are not NULL.
+
 2016-02-20  Mark Wielaard  
 
PR c/28901
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 27a073a..8b3fddc 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -917,6 +917,7 @@ walk_polymorphic_call_targets (hash_set 
*reachable_call_targets,
 static void
 check_global_declaration (symtab_node *snode)
 {
+  const char *decl_file;
   tree decl = snode->decl;
 
   /* Warn about any function declared static but not defined.  We don't
@@ -944,8 +945,10 @@ check_global_declaration (symtab_node *snode)
|| (((warn_unused_variable && ! TREE_READONLY (decl))
|| (warn_unused_const_variable > 0 && TREE_READONLY (decl)
&& (warn_unused_const_variable == 2
-   || filename_cmp (main_input_filename,
-DECL_SOURCE_FILE (decl)) == 0)))
+   || (main_input_filename != NULL
+   && (decl_file = DECL_SOURCE_FILE (decl)) != NULL
+   && filename_cmp (main_input_filename,
+decl_file) == 0
   && TREE_CODE (decl) == VAR_DECL))
   && ! DECL_IN_SYSTEM_HEADER (decl)
   && ! snode->referred_to_p (/*include_self=*/false)

Re: [PATCH] PR28901 Add two levels for -Wunused-const-variable.

[Mark Wielaard]

On Tue, 2016-02-23 at 09:51 +, Manuel López-Ibáñez wrote:
> On 23/02/16 08:56, Jakub Jelinek wrote:
> > On Tue, Feb 23, 2016@09:53:57AM +0100, Mark Wielaard wrote:
> >> --- a/gcc/ChangeLog
> >> +++ b/gcc/ChangeLog
> >> @@ -1,3 +1,10 @@
> >> +2016-02-23  Mark Wielaard  
> >> +  Jakub Jelinek  
> >> +
> >> +  PR c/69911
> >> +  * cgraphunit.c (check_global_declaration): Check main_input_filename
> >> +  and DECL_SOURCE_FILE are not NULL.
> >> +
> >>   2016-02-20  Mark Wielaard  
> >
> > This is ok for trunk if it passes testing.  Thanks.
> >
> >> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> >> index 27a073a..8b3fddc 100644
> >> --- a/gcc/cgraphunit.c
> >> +++ b/gcc/cgraphunit.c
> >> @@ -917,6 +917,7 @@ walk_polymorphic_call_targets (hash_set 
> >> *reachable_call_targets,
> >>   static void
> >>   check_global_declaration (symtab_node *snode)
> >>   {
> >> +  const char *decl_file;
> >> tree decl = snode->decl;
> >>
> >> /* Warn about any function declared static but not defined.  We don't
> >> @@ -944,8 +945,10 @@ check_global_declaration (symtab_node *snode)
> >>  || (((warn_unused_variable && ! TREE_READONLY (decl))
> >>|| (warn_unused_const_variable > 0 && TREE_READONLY (decl)
> >>&& (warn_unused_const_variable == 2
> >> -  || filename_cmp (main_input_filename,
> >> -   DECL_SOURCE_FILE (decl)) == 0)))
> >> +  || (main_input_filename != NULL
> >> +  && (decl_file = DECL_SOURCE_FILE (decl)) != NULL
> >> +  && filename_cmp (main_input_filename,
> >> +   decl_file) == 0
> 
> 
> Can we please please please hide this ugliness behind an (inline?) function 
> such as bool in_main_file_at (location_t) in input.[ch]? The condition here 
> is 
> quickly becoming unreadable.
> 
> Also because in the future somebody would want to re-implement this using 
> MAIN_FILE_P() from line-map.h, which is faster.

Sorry, I pushed the minimum fix. I am too embarrassed already for
breaking stuff to risk refactoring anything and making another dumb
mistake. (My mistake was only checking the top-level compiler .sum
files, I had forgotten to check the library ones).

It might be a good idea to file a bug for this and write a patch to
submit during next stage one if you believe this is a useful cleanup.

Cheers,

Mark

Re: [patch] libstdc++/69945 Add __gnu_cxx::__freeres hook

[Mark Wielaard]

On Wed, 2016-02-24 at 18:35 +, Jonathan Wakely wrote:
> This adds a new function to libsupc++ which will free the memory still
> in use by the pool used for allocating exceptions when malloc fails.
> 
> This is similar to glibc's __libc_freeres, which valgrind (and other
> tools?) use to tell glibc to deallocate everything before exiting.
> 
> I initially called it __gnu_cxx::__free_eh_pool() but I figured we
> might have other memory in use at some later date, and we wouldn't
> want valgrind to have to start calling a second function, nor make a
> function called __free_eh_pool() actually free other things.

I tested this on x86_64-pc-linux-gnu with Ivo's valgrind patch from
https://bugs.kde.org/show_bug.cgi?id=345307 and it works pretty nicely.
No more spurious still reachable memory issues with memcheck.

Is there any possibility to get this backported for 5.4?

Thanks,

Mark

Re: [PATCH] Warn when comparing nonnull arguments to NULL in a function.

[Mark Wielaard]

On Tue, 2015-09-15 at 14:21 +0200, Manuel López-Ibáñez wrote:
> On 15/09/15 10:32, Mark Wielaard wrote:
> > On Mon, 2015-09-14 at 21:37 -0600, Martin Sebor wrote:
> > Although I now notice they differ on the placement of the carrot.
> > Maybe the location passed into the warning is not correct/ideal?
> 
> The caret is placed at the location given by expand_location(loc), with loc 
> being the location passed to warning_at(). As far as I am aware, there are no 
> bugs on that. If the caret is wrong, it is definitely because the location 
> passed is the wrong one. You need to find the correct one (which may appear 
> up 
> in the call stack and may need to be passed down) and pass that one instead.
> 
> You can test the location with { dg-warning "18:nonnull argument" } where 18 
> is 
> the column number expected. I wish it was used more frequently, in particular 
> in those cases where we have the perfect location and it would be pity to 
> regress.

As Marin pointed out, this is not a new issue. Since build_binary_op
effectively only has one (input) location (and possibly the
DECL_SOURCE_LOCATIONS of the expressions, which are irrelevant in this
case) it is hard to get the location for the diagnostic precise. We
really need the locations of the expressions, not just the input
location of the binary op (which already seems a little inconsistent).
As you say we would have to audit each call to build_binary_op and
somehow pass through the location of op1 and op2 (if we actually have
them in the first place). Given the large number of ways build_binary_op
is called this doesn't seem feasible.

The real solution is probably something like David's "Add source-ranges
for trees" https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00739.html
David mentions several alternatives. I am not sure which one would be
the most acceptable.

Cheers,

Mark

Re: [PATCH] PR28901 -Wunused-variable ignores unused const initialised variables

[Mark Wielaard]

On Tue, 2015-09-15 at 10:20 -0700, Steve Ellcey wrote:
> On Tue, 2015-09-15 at 19:10 +0200, Jakub Jelinek wrote:
> > On Tue, Sep 15, 2015 at 10:02:15AM -0700, Steve Ellcey wrote:
> > > I am not sure I like this change.  It broke the GLIBC build for me on
> > > MIPS.  Basically GLIBC has a header file with initialized static
> > > constant globals (sysdeps/ieee754/dbl-64/atnat2.h contains tqpi1 and
> > > qpi1) and that header file is included in multiple .c files like
> > 
> > Multiple?  All I can see is e_atan2.c including that header file, nothing
> > else.
> 
> Whoops, bad assumption on my part.  I thought it must be included
> somewhere else, otherwise why put it in a header.

Yeah, odd. But it seems the issues the warning pointed out are real. I
build glibc and found 9 unused variables. They all look like they are
really not used in the code, so they can all just be removed. Someone of
course should double check they aren't unused by accident before
committing upstream.

Cheers,

Mark
diff --git a/math/atest-exp2.c b/math/atest-exp2.c
index 307c741..ffa73b1 100644
--- a/math/atest-exp2.c
+++ b/math/atest-exp2.c
@@ -53,11 +53,6 @@ static const mp1 mp_exp1 = {
a784d904, 5190cfef, 324e7738, 926cfbe5, f4bf8d8d, 8c31d763)
 };
 
-static const mp1 mp_exp_m1 = {
-  CONSTSZ (0, 5e2d58d8, b3bcdf1a, badec782, 9054f90d, da9805aa, b56c7733,
-   3024b9d0, a507daed, b16400bf, 472b4215, b8245b66, 9d90d27a)
-};
-
 static const mp1 mp_log2 = {
   CONSTSZ (0, b17217f7, d1cf79ab, c9e3b398, 03f2f6af, 40f34326, 7298b62d,
8a0d175b, 8baafa2b, e7b87620, 6debac98, 559552fb, 4afa1b10)
diff --git a/resolv/base64.c b/resolv/base64.c
index ea584ed..519e5d2 100644
--- a/resolv/base64.c
+++ b/resolv/base64.c
@@ -40,10 +40,6 @@
  * IF IBM IS APPRISED OF THE POSSIBILITY OF SUCH DAMAGES.
  */
 
-#if !defined(LINT) && !defined(CODECENTER)
-static const char rcsid[] = "$BINDId: base64.c,v 8.7 1999/10/13 16:39:33 vixie Exp $";
-#endif /* not lint */
-
 #include 
 #include 
 #include 
diff --git a/sysdeps/ieee754/dbl-64/atnat2.h b/sysdeps/ieee754/dbl-64/atnat2.h
index e0d65af..82943f9 100644
--- a/sysdeps/ieee754/dbl-64/atnat2.h
+++ b/sysdeps/ieee754/dbl-64/atnat2.h
@@ -65,10 +65,8 @@
 /**/ hpi1   = {{0x3c91a626, 0x33145c07} }, /*  pi/2-hpi */
 /**/ mhpi   = {{0xbff921fb, 0x54442d18} }, /* -pi/2 */
 /**/ qpi= {{0x3fe921fb, 0x54442d18} }, /*  pi/4 */
-/**/ qpi1   = {{0x3c81a626, 0x33145c07} }, /*  pi/4-qpi */
 /**/ mqpi   = {{0xbfe921fb, 0x54442d18} }, /* -pi/4 */
 /**/ tqpi   = {{0x4002d97c, 0x7f3321d2} }, /*  3pi/4*/
-/**/ tqpi1  = {{0x3c9a7939, 0x4c9e8a0a} }, /*  3pi/4-tqpi   */
 /**/ mtqpi  = {{0xc002d97c, 0x7f3321d2} }, /* -3pi/4*/
 /**/ u1 = {{0x3c314c2a, 0x} }, /*  9.377e-19*/
 /**/ u2 = {{0x3bf955e4, 0x} }, /*  8.584e-20*/
@@ -129,10 +127,8 @@
 /**/ hpi1   = {{0x33145c07, 0x3c91a626} }, /*  pi/2-hpi */
 /**/ mhpi   = {{0x54442d18, 0xbff921fb} }, /* -pi/2 */
 /**/ qpi= {{0x54442d18, 0x3fe921fb} }, /*  pi/4 */
-/**/ qpi1   = {{0x33145c07, 0x3c81a626} }, /*  pi/4-qpi */
 /**/ mqpi   = {{0x54442d18, 0xbfe921fb} }, /* -pi/4 */
 /**/ tqpi   = {{0x7f3321d2, 0x4002d97c} }, /*  3pi/4*/
-/**/ tqpi1  = {{0x4c9e8a0a, 0x3c9a7939} }, /*  3pi/4-tqpi   */
 /**/ mtqpi  = {{0x7f3321d2, 0xc002d97c} }, /* -3pi/4*/
 /**/ u1 = {{0x, 0x3c314c2a} }, /*  9.377e-19*/
 /**/ u2 = {{0x, 0x3bf955e4} }, /*  8.584e-20*/
diff --git a/sysdeps/ieee754/dbl-64/uexp.h b/sysdeps/ieee754/dbl-64/uexp.h
index 6817eaf..42b21f2 100644
--- a/sysdeps/ieee754/dbl-64/uexp.h
+++ b/sysdeps/ieee754/dbl-64/uexp.h
@@ -29,7 +29,7 @@
 
 #include "mydefs.h"
 
-const static double one = 1.0, zero = 0.0, hhuge = 1.0e300, tiny = 1.0e-300,
+const static double zero = 0.0, hhuge = 1.0e300, tiny = 1.0e-300,
 err_0 = 1.14, err_1 = 0.16;
 const static int4 bigint = 0x40862002,
  badint = 0x40876000,smallint = 0x3C8f;
diff --git a/sysdeps/ieee754/dbl-64/upow.h b/sysdeps/ieee754/dbl-64/upow.h
index c8569a9..b4911e5 100644
--- a/sysdeps/ieee754/dbl-64/upow.h
+++ b/sysdeps/ieee754/dbl-64/upow.h
@@ -34,7 +34,6 @@
 /**/ nZERO	= {{0x8000, 0}},	  /* -0.0  */
 /**/ INF= {{0x7ff0, 0x}}, /* INF   */
 /**/ nINF   = {{0xfff0, 0x}}, /* -INF  */
-/**/ sqrt_2 = {{0x3ff6a09e, 0x667f3bcc}}, /* sqrt(2)   */
 /**/ ln2a   = {{0x3fe62e42, 0xfefa3800}}, /* ln(2) 43 bits */
 /**/ ln2b   = {{0x3d2ef357, 0x93c76730}}, /* ln(2)-ln2a*/
 /**/ bigu   = {{0x4297, 0xfd2c}}, /* 1.5*2**42 -724*2**-10  */
@@ -48,7 +47,6 @@
 /**/ nZERO	= {{0, 0x8000}},	  /* -0.0  */
 /**/ INF= {{0x, 0x7ff0}}, /

Re: [PATCH] PR28901 -Wunused-variable ignores unused const initialised variables

[Mark Wielaard]

On Tue, 2015-09-15 at 20:33 +0200, Mark Wielaard wrote:
> I build glibc and found 9 unused variables. They all look like they are
> really not used in the code, so they can all just be removed. Someone of
> course should double check they aren't unused by accident before
> committing upstream.

For the record there was one other issue with building glibc with
current GCC6 was:

 tst-endian.c: In function ‘do_test’:
 tst-endian.c:22:30: error: self-comparison always evaluates to false 
[-Werror=tautological-compare]
 if (htole16 (le16toh (i)) != i)
   ^
which is fixed by:

diff --git a/string/tst-endian.c b/string/tst-endian.c
index 8684bb2..9d64a87 100644
--- a/string/tst-endian.c
+++ b/string/tst-endian.c
@@ -9,7 +9,7 @@ do_test (void)
 {
   int result = 0;
 
-  for (uint64_t i = 0; i < (~UINT64_C (0)) >> 2; i = (i << 1) + 3)
+  for (volatile uint64_t i = 0; i < (~UINT64_C (0)) >> 2; i = (i << 1) + 3)
 {
   if (i < UINT64_C (65536))
{

Re: [PATCH] PR28901 -Wunused-variable ignores unused const initialised variables

[Mark Wielaard]

On Thu, 2015-09-17 at 18:12 +0200, Bernd Schmidt wrote:
> On 09/17/2015 02:01 PM, Gerald Pfeifer wrote:
> > On Sun, 13 Sep 2015, Mark Wielaard wrote:
> >> Slightly adjusted patch attached. Now it is explicit that the warning is
> >> enabled by -Wunused-variable for C, but not C++. There are testcases for
> >> both C and C++ to check the defaults. And the hardcoded override is
> >> removed for C++, so the user could enable it if they want.
> >
> > I believe making -Wunused-const-variable part of -Wall is not
> > a good idea.
> >
> > For example, I have a nightly build of Wine with a nightly build
> > of GCC.  And my notifaction mail went from twently lines of warning
> > to 6500 -- all coming from header files.
> 
> What does it warn about, do the warnings look valid?

I was just taking a look. They come from constructs like:

static const WCHAR INSTALLPROPERTY_VERSIONSTRINGW[] =
{'V','e','r','s','i','o','n','S','t','r','i','n','g',0};

Using the more idiomatic and a bit more readable:

#define INSTALLPROPERTY_VERSIONSTRINGW u"VersionString"

gets rid of most of the issues.

Cheers,

Mark

Re: [PATCH] PR28901 -Wunused-variable ignores unused const initialised variables

[Mark Wielaard]

On Wed, 2015-09-23 at 12:25 -0600, Jeff Law wrote:
> On 09/18/2015 08:29 PM, Martin Sebor wrote:
> >> I guess it is not the 'const' I think should be handled special but the
> >> 'static'.  Having unused static variables (const or not) declared in a
> >> header file but unused seems reasonable since the header file may be
> >> included in multiple .c files each of which uses a subset of the static
> >> variables.
> >
> > I tend to agree. I suppose diagnosing unused non-const static
> > definitions might be helpful but I can't think of a good reason
> > to diagnose unused initialized static consts in C. Especially
> > since they're not diagnosed in C++.
> >
> > Would diagnosing them in source files while avoiding the warning
> > for static const definitions in headers be an acceptable compromise?
> It's probably worth a try.

I am a little concerned that would hide some real issues. In the case of
glibc the header files were actually only used by one main file, so the
variables were indeed unused and needed to be investigated why they were
there in the first place. In the case of wine the issue was that the
header file contained non-idiomatic and somewhat unreadable C constructs
that could easily be replaced by more readable defines for 16bit char
string constants.

Even if there are such constructs in header files and they aren't
actually bugs or people are unwilling to fix the issue with something
that is more idiomatic C then there are various ways to suppress the
warning. Either just don't use -Wunused-variable or add
-Wno-unused-const-variable. Add an explicit __attribute__((used)) or
just add a #pragma GCC system_header to the .h file.

If we are concerned that this generates warnings that aren't easy to
avoid then we might want to add that particular check behind -Wextra.
But is that really necessary? I am not against implementing an extra
warning exception/flag if it really is necessary. But it does introduce
more complexity and makes the warning less consistent. So what would be
a good way to find out one way or another whether the extra complexity
is needed?

Cheers,

Mark

Re: [PATCH] PR28901 -Wunused-variable ignores unused const initialised variables

[Mark Wielaard]

On Sat, 2015-10-24 at 15:47 +0200, Gerald Pfeifer wrote:
> On Wed, 7 Oct 2015, Bernd Schmidt wrote:
> > I think not using -Wall -Werror is the right fix.
> 
> Of course there is a fair chance (and I'll likely end up proposing
> this for Wine) is that people will use -Wnounused-variable instead.
> 
> Which will disable _all_ such warnings, not only those coming from
>   const ... ... = ...;

Then use -Wno-unused-const-variable, not -Wno-unused-variable, if you
only want to suppress the new warning.

Cheers,

Mark

BoF DWARF5 patches

[Mark Wielaard]

Hi,

This afternoon there will be a BoF at the virtual Cauldron about
DWARF5 and beyond. https://linuxplumbersconf.org/event/7/contributions/746/

Here are some patches for GCC that I would like to discuss.
I'll reply to them after the BoF with any comments people made.

Cheers,

Mark

[PATCH 1/5] Don't enable -gvariable-location-views by default for DWARF5.

[Mark Wielaard]

DWARF5 makes it possible to read loclists tables without consulting
the debuginfo tree by introducing a table header. Adding location views
breaks this (at least for binutils and elfutils). So don't enable
variable-location-views by default if DWARF5 or higher is selected.
---
 gcc/doc/invoke.texi | 6 +++---
 gcc/toplev.c| 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 70dc1ab73a12..e5dddc236d7d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9301,9 +9301,9 @@ can be consumed by debug information consumers that are 
not aware of
 these augmentations, but they won't derive any benefit from them either.
 
 This is enabled by default when outputting DWARF 2 debug information at
-the normal level, as long as there is assembler support,
-@option{-fvar-tracking-assignments} is enabled and
-@option{-gstrict-dwarf} is not.  When assembler support is not
+the normal level for DWARF versions lower than 5, as long as there
+is assembler support, @option{-fvar-tracking-assignments} is enabled
+and @option{-gstrict-dwarf} is not.  When assembler support is not
 available, this may still be enabled, but it will force GCC to output
 internal line number tables, and if
 @option{-ginternal-reset-location-views} is not enabled, that will most
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 07457d08c3aa..34218c6b3349 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1672,6 +1672,7 @@ process_options (void)
   && (write_symbols == DWARF2_DEBUG
   || write_symbols == VMS_AND_DWARF2_DEBUG)
   && !dwarf_strict
+  && dwarf_version < 5
   && dwarf2out_as_loc_support
   && dwarf2out_as_locview_support);
 }
-- 
2.18.4

[PATCH 3/5] Add DWARF5 variants of assembly scan tests that use DW_FORM_implicit_const

[Mark Wielaard]

Some DWARF tests scan the assembly output looking for constant values.
When using DWARF5 those constants might use DW_FORM_implicit_const,
which are output (in the comments) after the attribute instead of
before. To make sure these tests work introduce a -gdwarf-5 variant
of these tests and explicitly use -gdwarf-2 for the original.
---
 .../dwarf2/{inline-var-1.C => inline-var-1-dwarf5.C}  | 6 --
 gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C  | 2 +-
 .../gcc.dg/debug/dwarf2/{inline2.c => inline2-dwarf5.c}   | 7 ---
 gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c   | 4 +++-
 .../debug/dwarf2/{pr41445-5.c => pr41445-5-dwarf5.c}  | 8 
 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-5.c | 2 +-
 .../debug/dwarf2/{pr41445-6.c => pr41445-6-dwarf5.c}  | 8 
 gcc/testsuite/gcc.dg/debug/dwarf2/pr41445-6.c | 2 +-
 8 files changed, 22 insertions(+), 17 deletions(-)
 copy gcc/testsuite/g++.dg/debug/dwarf2/{inline-var-1.C => 
inline-var-1-dwarf5.C} (76%)
 copy gcc/testsuite/gcc.dg/debug/dwarf2/{inline2.c => inline2-dwarf5.c} (88%)
 copy gcc/testsuite/gcc.dg/debug/dwarf2/{pr41445-5.c => pr41445-5-dwarf5.c} 
(73%)
 copy gcc/testsuite/gcc.dg/debug/dwarf2/{pr41445-6.c => pr41445-6-dwarf5.c} 
(68%)

diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C 
b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1-dwarf5.C
similarity index 76%
copy from gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
copy to gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1-dwarf5.C
index 3b1c913edfca..52ed5b6912fd 100644
--- a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1-dwarf5.C
@@ -1,7 +1,9 @@
+// DWARF5 variant of inline-var-1.C
 // { dg-do compile { target c++17 } }
-// { dg-options "-O -g -dA -gno-strict-dwarf 
-fno-eliminate-unused-debug-symbols" }
+// { dg-options "-O -gdwarf-5 -dA -gno-strict-dwarf 
-fno-eliminate-unused-debug-symbols" }
 // { dg-require-weak "" }
-// { dg-final { scan-assembler-times "0x3\[^\n\r]* DW_AT_inline" 6 { xfail 
*-*-aix* } } }
+// { dg-final { scan-assembler-times " DW_AT_inline \\(0x3\\)" 2 { xfail 
*-*-aix* } } }
+// { dg-final { scan-assembler-times "0x3\[^\n\r]* DW_AT_inline" 4 { xfail 
*-*-aix* } } }
 // { dg-final { scan-assembler-times "0x1\[^\n\r]* DW_AT_inline" 2 { xfail 
*-*-aix* } } }
 // { dg-final { scan-assembler-times " DW_AT_declaration" 6 { xfail *-*-aix* } 
} }
 // { dg-final { scan-assembler-times " DW_AT_specification" 6 { xfail *-*-aix* 
} } }
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C 
b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
index 3b1c913edfca..9a88e28cbe0f 100644
--- a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
@@ -1,5 +1,5 @@
 // { dg-do compile { target c++17 } }
-// { dg-options "-O -g -dA -gno-strict-dwarf 
-fno-eliminate-unused-debug-symbols" }
+// { dg-options "-O -gdwarf-2 -dA -gno-strict-dwarf 
-fno-eliminate-unused-debug-symbols" }
 // { dg-require-weak "" }
 // { dg-final { scan-assembler-times "0x3\[^\n\r]* DW_AT_inline" 6 { xfail 
*-*-aix* } } }
 // { dg-final { scan-assembler-times "0x1\[^\n\r]* DW_AT_inline" 2 { xfail 
*-*-aix* } } }
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c 
b/gcc/testsuite/gcc.dg/debug/dwarf2/inline2-dwarf5.c
similarity index 88%
copy from gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c
copy to gcc/testsuite/gcc.dg/debug/dwarf2/inline2-dwarf5.c
index 7e019a6c06a0..03013f11bca8 100644
--- a/gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/inline2-dwarf5.c
@@ -1,4 +1,4 @@
-/* Contributed by Dodji Seketeli 
+/* DWARF5 variant of inline2.
Origin: PR debug/37801
 
   Abstract instances (DW_TAG_subroutines having the DW_AT_inline attribute)
@@ -14,7 +14,8 @@
   properly nested DW_TAG_inlined_subroutine DIEs for third, second and first.
 */
 
-/* { dg-options "-O -g3 -gdwarf -dA -fgnu89-inline" } */
+/* Explicitly use dwarf-5 which uses DW_FORM_implicit_const.  */
+/* { dg-options "-O -g3 -gdwarf-5 -dA -fgnu89-inline" } */
 /* { dg-do compile } */
 
 /* There are 6 inlined subroutines:
@@ -32,7 +33,7 @@
 /* There are 3 DW_AT_inline attributes: one per abstract inline instance.
The value of the attribute must be 0x3, meaning the function was
actually inlined.  */
-/* { dg-final { scan-assembler-times  "(?:byte|data1)\[^\n\]*0x3\[^\n\]* 
DW_AT_inline" 3 } } */
+/* { dg-final { scan-assembler-times  " DW_AT_inline \\(0x3\\)" 3 } } */
 
 volatile int *a;
 
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c 
b/gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c
index 7e019a6c06a0..9c36450ae2de 100644
--- a/gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c
@@ -14,7 +14,9 @@
   properly nested DW_TAG_inlined_subroutine DIEs for third, second and first.
 */
 
-/* { dg-options "-O -g3 -gdwarf -dA -fgnu89-inline" } */
+/* Explicitly use dwarf-2 because dwa

[PATCH 5/5] Add --gdwarf-5 to ASM_SPEC

[Mark Wielaard]

This is needed to get DWARF version 5 .debug_line tables.
It is also obviously wrong. It needs a check for whether as supports
--gdwarf- for all versions we support and it should only
be added when generating DWARF debug information for the specific
DWARF version we are generating.

It also needs some fixes to binutils, to make sure the line table is
generated correctly:
https://sourceware.org/pipermail/binutils/2020-August/112685.html
And to make sure it can read the generated line table itself:
https://sourceware.org/pipermail/binutils/2020-August/112966.html
---
 gcc/gcc.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/gcc/gcc.c b/gcc/gcc.c
index 10bc9881aed3..98b10e7cd154 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -1882,6 +1882,11 @@ init_spec (void)
   }
 #endif
 
+  static const char dv[] = "--gdwarf-5 ";
+  obstack_grow (&obstack, dv, sizeof (dv) - 1);
+  obstack_grow0 (&obstack, asm_spec, strlen (asm_spec));
+  asm_spec = XOBFINISH (&obstack, const char *);
+
 #if defined LINK_EH_SPEC || defined LINK_BUILDID_SPEC || \
 defined LINKER_HASH_STYLE
 # ifdef LINK_BUILDID_SPEC
-- 
2.18.4

[PATCH 4/5] Default to DWARF5

[Mark Wielaard]

---
 gcc/common.opt  | 2 +-
 gcc/doc/invoke.texi | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index 513125f0c00b..05e4e0638ecb 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -3144,7 +3144,7 @@ Common Driver JoinedOrMissing Negative(gdwarf-)
 Generate debug information in default version of DWARF format.
 
 gdwarf-
-Common Driver Joined UInteger Var(dwarf_version) Init(4) Negative(gstabs)
+Common Driver Joined UInteger Var(dwarf_version) Init(5) Negative(gstabs)
 Generate debug information in DWARF v2 (or later) format.
 
 ggdb
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e5dddc236d7d..511fcd25 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9042,13 +9042,14 @@ possible.
 @opindex gdwarf
 Produce debugging information in DWARF format (if that is supported).
 The value of @var{version} may be either 2, 3, 4 or 5; the default version
-for most targets is 4.  DWARF Version 5 is only experimental.
+for most targets is 5 (with the exception of vxworks and darwin which
+default to version 2).
 
 Note that with DWARF Version 2, some ports require and always
 use some non-conflicting DWARF 3 extensions in the unwind tables.
 
 Version 4 may require GDB 7.0 and @option{-fvar-tracking-assignments}
-for maximum benefit.
+for maximum benefit. Version 5 requires GDB 8.0 or higher.
 
 GCC no longer supports DWARF Version 1, which is substantially
 different than Version 2 and later.  For historical reasons, some
-- 
2.18.4

[PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.

[Mark Wielaard]

In DWARF5 class variables (static data members) are represented with a
DW_TAG_variable instead of a DW_TAG_member. Make sure the variable isn't
optimized away in the constexpr-var-1.C testcase so we can still match (2)
const_expr in the the assembly output.

Note that the same issue causes some failures in the gdb testsuite
for static data members when we enable DWARF5 by default:
https://sourceware.org/bugzilla/show_bug.cgi?id=26525
---
 gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C 
b/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C
index 19062e29fd59..c6ad3f645379 100644
--- a/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C
@@ -7,3 +7,4 @@ struct S
 {
   static constexpr int b = 6;
 } s;
+const int &c = s.b;
-- 
2.18.4

Re: [PATCH 1/5] Don't enable -gvariable-location-views by default for DWARF5.

[Mark Wielaard]

Hi,

On Mon, Aug 24, 2020 at 07:38:10PM +0200, Jakub Jelinek wrote:
> On Mon, Aug 24, 2020 at 02:56:54PM +0200, Mark Wielaard wrote:
> > DWARF5 makes it possible to read loclists tables without consulting
> > the debuginfo tree by introducing a table header. Adding location views
> > breaks this (at least for binutils and elfutils). So don't enable
> > variable-location-views by default if DWARF5 or higher is selected.
> 
> This should be discussed with Alex, CCed.
> I'd say elfutils/binutils should just show .debug_loclists independent of
> .debug_info if there are no loc views and use .debug_info otherwise.

Yes, sorry, should have CCed Alex. I do agree this is less than
ideal. It was really just to make sure the elfutils testsuite was
clean.

But the issue is real. As is, when enabling DWARF5, it is impossible
for consumers to parse the .debug_loclists independently. The only way
to know whether (and where) the locviews are is by finding and then
parsing the corresponding CU DIE tree.

There is actually an alternative representation enabled by
-gvariable-location-views=incompat5 that might help, but as the name
implies it isn't standard DWARF5, and I believe neither elfutils nor
binutils parses it.

I am not sure what a good default is in case we enable -gdwarf-5.

Cheers,

Mark

Re: [PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.

[Mark Wielaard]

Hi Jakub,

On Mon, Aug 24, 2020 at 07:40:51PM +0200, Jakub Jelinek wrote:
> On Mon, Aug 24, 2020 at 02:56:55PM +0200, Mark Wielaard wrote:
> > In DWARF5 class variables (static data members) are represented with a
> > DW_TAG_variable instead of a DW_TAG_member. Make sure the variable isn't
> > optimized away in the constexpr-var-1.C testcase so we can still match (2)
> > const_expr in the the assembly output.
> > 
> > Note that the same issue causes some failures in the gdb testsuite
> > for static data members when we enable DWARF5 by default:
> > https://sourceware.org/bugzilla/show_bug.cgi?id=26525
> > ---
> >  gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C 
> > b/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C
> > index 19062e29fd59..c6ad3f645379 100644
> > --- a/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C
> > +++ b/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C
> > @@ -7,3 +7,4 @@ struct S
> >  {
> >static constexpr int b = 6;
> >  } s;
> > +const int &c = s.b;
> 
> This looks incorrect to me, that is a workaround for a real GCC bug.

I was discussing this after the BoF with Tom Tromey (CCed) and he also
thought gdb could/should actually support the DWARF5 representation,
but because the DW_TAG_variable was removed because the static data
member wasn't referenced in the gdb testcases.

> Shouldn't we instead do something like (untested) following patch?
> I mean, for DWARF < 5 the static data members were using DW_TAG_member,
> which has been always marked by the function, so IMHO we should also always
> mark the DW_TAG_variables at the class scope that replaced those.
> 
> 2020-08-24  Jakub Jelinek  
> 
>   * dwarf2out.c (prune_unused_types_walk): Mark DW_TAG_variable DIEs
>   at class scope for DWARF5+.

This looks really good, and it makes all the FAILs in the gdb bug
report PASS (when build with -gdwarf-5 as default).

> --- gcc/dwarf2out.c.jj2020-07-28 15:39:09.883757946 +0200
> +++ gcc/dwarf2out.c   2020-08-24 19:33:16.503961786 +0200
> @@ -29392,6 +29392,13 @@ prune_unused_types_walk (dw_die_ref die)
> if (die->die_perennial_p)
>   break;
>  
> +   /* For static data members, the declaration in the class is supposed
> +  to have DW_TAG_member tag in DWARF{3,4} but DW_TAG_variable in
> +  DWARF5.  DW_TAG_member will be marked, so mark even such
> +  DW_TAG_variables in DWARF5.  */
> +   if (dwarf_version >= 5 && class_scope_p (die->die_parent))
> + break;
> +
> /* premark_used_variables marks external variables --- don't mark
>them here.  But function-local externals are always considered
>used.  */

Thanks,

Mark

Re: [PATCH 3/5] Add DWARF5 variants of assembly scan tests that use DW_FORM_implicit_const

[Mark Wielaard]

On Mon, Aug 24, 2020 at 07:44:27PM +0200, Jakub Jelinek wrote:
> On Mon, Aug 24, 2020 at 02:56:56PM +0200, Mark Wielaard wrote:
> > Some DWARF tests scan the assembly output looking for constant values.
> > When using DWARF5 those constants might use DW_FORM_implicit_const,
> > which are output (in the comments) after the attribute instead of
> > before. To make sure these tests work introduce a -gdwarf-5 variant
> > of these tests and explicitly use -gdwarf-2 for the original.
> 
> I just wonder if we want to use -gdwarf-2 rather than -gdwarf-4 in the
> original, -gdwarf-5 has been the default for a couple of years and thus
> that is what those testshave been compiled with.

I used -gdwarf-2 because I thought that was still the default for some
arches/platforms. And they pass with -gdwarf-2.

> Also not sure about the -dwarf5 suffixes, couldn't we say just use
> pr41445-{7,8}.c, inline-var-2.C or inline3.c (or whatever next number
> with the same prefix is still unused)?

Sure, if that is a better naming scheme I'll rename them.

Cheers,

Mark

Re: [PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.

[Mark Wielaard]

Hi,

On Mon, 2020-08-24 at 17:38 -0400, Jason Merrill wrote:
> > This looks incorrect to me, that is a workaround for a real GCC bug.
> > 
> > Shouldn't we instead do something like (untested) following patch?
> > I mean, for DWARF < 5 the static data members were using DW_TAG_member,
> > which has been always marked by the function, so IMHO we should also always
> > mark the DW_TAG_variables at the class scope that replaced those.
> 
> The earlier behavior seems like an accident, that happened because we 
> always need to emit information about non-static data members.  I don't 
> think we should take it as guidance.

Maybe the reason they got emitted this way was an accident on the GCC
side. But I don't think it is an accident on the GDB side. At least
looking at the various gdb testcases, the intention is to show a
struct/class type as defined to the user, which includes both the
static and non-static data members of a class.

> In this case one reason we don't emit debug info is because (before 
> C++17) there's no definition of 'b'.  After C++17 the in-class 
> declaration of 'b' is a definition, but we don't have to give it a 
> symbol, so there's still nothing for the debug info to describe.

But don't we describe other parts of a type that don't have a symbol as
part of the debug info?

> This issue doesn't seem specific to class members; it also affects 
> namespace-scope C++17 inline variables:
> 
> inline const int var = 42;
> int main() { return var; }
> 
> Compiling this testcase with -g doesn't emit any debug info for 'var' 
> even though it's used.

That is new in GCC11, don't have GCC10 here to compare against, but
GCC9 produces:

DWARF section [27] '.debug_info' at offset 0x30b5:
 [Offset]
 Compilation unit at offset 0:
 Version: 5, Abbreviation section offset: 0, Address size: 8, Offset size: 4
 Unit type: compile (1)
 [ c]  compile_unit abbrev: 1
   producer (strp) "GNU C++17 9.3.1 20200408 (Red Hat 
9.3.1-2) -mtune=generic -march=x86-64 -gdwarf-5 -O2 -std=gnu++17"
   language (data1) C_plus_plus_14 (33)
   name (strp) "p.cc"
   comp_dir (strp) "/tmp"
   ranges   (sec_offset) range list [ c]
   low_pc   (addr) 00
   stmt_list(sec_offset) 0
 [2a]variable abbrev: 2
 name (string) "var"
 decl_file(data1) p.cc (1)
 decl_line(data1) 1
 decl_column  (data1) 18
 type (ref4) [3f]
 external (flag_present) yes
 inline   (data1) declared_inlined (3)
 const_value  (data1) 42
 [38]base_typeabbrev: 3
 byte_size(data1) 4
 encoding (data1) signed (5)
 name (string) "int"
 [3f]const_type   abbrev: 4
 type (ref4) [38]
 [44]subprogram   abbrev: 5
 external (flag_present) yes
 name (strp) "main"
 decl_file(data1) p.cc (1)
 decl_line(data1) 2
 decl_column  (data1) 5
 type (ref4) [38]
 low_pc   (addr) 0x00401050 
 high_pc  (data8) 6 (0x00401056 <_start>)
 frame_base   (exprloc) 
  [ 0] call_frame_cfa
 call_all_calls   (flag_present) yes

> Should we assume that if a variable with DW_AT_const_value is TREE_USED, 
> we need to write out debug info for it?

I would say yes. If it is used a user might want to look up its value.

Cheers,

Mark

Re: [PATCH 1/5] Don't enable -gvariable-location-views by default for DWARF5.

[Mark Wielaard]

Hi Alex,

On Tue, 2020-08-25 at 01:05 -0300, Alexandre Oliva wrote:
> it would seem to
> make more sense to adopt and promote the proposed extension,
> implemented in =incompat5 in GCC, but it would need some
> implementation work in consumers to at least ignore the extension.

Is that what is described in:
http://www.fsfla.org/~lxoliva/papers/sfn/dwarf6-sfn-lvu.txt

Does it match the proposal in:
http://www.dwarfstd.org/ShowIssue.php?issue=170427.1

Should we try to introduce your extending loclists proposal:
http://www.dwarfstd.org/ShowIssue.php?issue=170427.2

The main issue is that there is no formal way of extending the
loclists.

Thanks,

Mark

Re: BoF DWARF5 patches

[Mark Wielaard]

Hi,

On Mon, 2020-08-24 at 14:56 +0200, Mark Wielaard wrote:
> This afternoon there will be a BoF at the virtual Cauldron about
> DWARF5 and beyond. 
> https://linuxplumbersconf.org/event/7/contributions/746/
> 
> Here are some patches for GCC that I would like to discuss.
> I'll reply to them after the BoF with any comments people made.

Jeremy Bennett was nice enough to add the BoF notes to the GCC wiki: 
https://gcc.gnu.org/wiki/linuxplumbers2020#DWARF5.2FDWARF64_BoF

Cheers,

Mark

Duplicate .debug_lines (Was: [PATCH 5/5] Add --gdwarf-5 to ASM_SPEC)

[Mark Wielaard]

Hi gcc hackers, binutils hackers,

Nick, Jakub and I were discussing the gcc patch below and all the ways
it is wrong. Most things can be fixed in the spec. Like only passing
-gdwarf if we are generating DWARF and passing the right DWARF version
as -gdwarf-N for the version that gcc itself creates. But whether or
not we want gas to generate .debug_line info is a bit tricky. But when
giving -gdwarf-N gas will always try to generate a .debug_line section
and error out when there is already one.

Would it be possible to have something like the following in gas, so
that it doesn't try generating a .debug_line section if there already
is one, even when -gdwarf-N is given (unless the assembly also
contains .loc directives because that shows the user is really
confused)?

diff --git a/gas/dwarf2dbg.c b/gas/dwarf2dbg.c
index e4ba56d82ba..c0c09f4e9d0 100644
--- a/gas/dwarf2dbg.c
+++ b/gas/dwarf2dbg.c
@@ -2626,7 +2626,7 @@ dwarf2_init (void)
 
 
 /* Finish the dwarf2 debug sections.  We emit .debug.line if there
-   were any .file/.loc directives, or --gdwarf2 was given, or if the
+   were any .file/.loc directives, or --gdwarf2 was given, and if the
file has a non-empty .debug_info section and an empty .debug_line
section.  If we emit .debug_line, and the .debug_info section is
empty, we also emit .debug_info, .debug_aranges and .debug_abbrev.
@@ -2650,9 +2650,16 @@ dwarf2_finish (void)
   empty_debug_line = line_seg == NULL || !seg_not_empty_p (line_seg);
 
   /* We can't construct a new debug_line section if we already have one.
- Give an error.  */
+ Give an error if we have seen any .loc, otherwise trust the user
+ knows what they are doing and want to generate the .debug_line
+ (and all other debug sections) themselves.  */
   if (all_segs && !empty_debug_line)
-as_fatal ("duplicate .debug_line sections");
+{
+  if (dwarf2_loc_directive_seen)
+   as_fatal ("duplicate .debug_line sections");
+  else
+   return;
+}
 
   if ((!all_segs && emit_other_sections)
   || (!emit_other_sections && !empty_debug_line))

On Mon, Aug 24, 2020 at 02:56:58PM +0200, Mark Wielaard wrote:
> This is needed to get DWARF version 5 .debug_line tables.
> It is also obviously wrong. It needs a check for whether as supports
> --gdwarf- for all versions we support and it should only
> be added when generating DWARF debug information for the specific
> DWARF version we are generating.
> 
> It also needs some fixes to binutils, to make sure the line table is
> generated correctly:
> https://sourceware.org/pipermail/binutils/2020-August/112685.html
> And to make sure it can read the generated line table itself:
> https://sourceware.org/pipermail/binutils/2020-August/112966.html
> ---
>  gcc/gcc.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/gcc/gcc.c b/gcc/gcc.c
> index 10bc9881aed3..98b10e7cd154 100644
> --- a/gcc/gcc.c
> +++ b/gcc/gcc.c
> @@ -1882,6 +1882,11 @@ init_spec (void)
>}
>  #endif
>  
> +  static const char dv[] = "--gdwarf-5 ";
> +  obstack_grow (&obstack, dv, sizeof (dv) - 1);
> +  obstack_grow0 (&obstack, asm_spec, strlen (asm_spec));
> +  asm_spec = XOBFINISH (&obstack, const char *);
> +
>  #if defined LINK_EH_SPEC || defined LINK_BUILDID_SPEC || \
>  defined LINKER_HASH_STYLE
>  # ifdef LINK_BUILDID_SPEC
> -- 
> 2.18.4
> 

Re: Duplicate .debug_lines (Was: [PATCH 5/5] Add --gdwarf-5 to ASM_SPEC)

[Mark Wielaard]

Hi,

On Wed, Aug 26, 2020 at 04:38:21PM -0700, H.J. Lu wrote:
> On Wed, Aug 26, 2020 at 2:38 PM Mark Wielaard  wrote:
> > Would it be possible to have something like the following in gas, so
> > that it doesn't try generating a .debug_line section if there already
> > is one, even when -gdwarf-N is given (unless the assembly also
> > contains .loc directives because that shows the user is really
> > confused)?
> >
> > diff --git a/gas/dwarf2dbg.c b/gas/dwarf2dbg.c
> > index e4ba56d82ba..c0c09f4e9d0 100644
> > --- a/gas/dwarf2dbg.c
> > +++ b/gas/dwarf2dbg.c
> > @@ -2626,7 +2626,7 @@ dwarf2_init (void)
> >
> >
> >  /* Finish the dwarf2 debug sections.  We emit .debug.line if there
> > -   were any .file/.loc directives, or --gdwarf2 was given, or if the
> > +   were any .file/.loc directives, or --gdwarf2 was given, and if the
> > file has a non-empty .debug_info section and an empty .debug_line
> > section.  If we emit .debug_line, and the .debug_info section is
> > empty, we also emit .debug_info, .debug_aranges and .debug_abbrev.
> > @@ -2650,9 +2650,16 @@ dwarf2_finish (void)
> >empty_debug_line = line_seg == NULL || !seg_not_empty_p (line_seg);
> >
> >/* We can't construct a new debug_line section if we already have one.
> > - Give an error.  */
> > + Give an error if we have seen any .loc, otherwise trust the user
> > + knows what they are doing and want to generate the .debug_line
> > + (and all other debug sections) themselves.  */
> >if (all_segs && !empty_debug_line)
> > -as_fatal ("duplicate .debug_line sections");
> > +{
> > +  if (dwarf2_loc_directive_seen)
> > +   as_fatal ("duplicate .debug_line sections");
> > +  else
> > +   return;
> > +}
> >
> >if ((!all_segs && emit_other_sections)
> >|| (!emit_other_sections && !empty_debug_line))
> >
>
> I have run into this issue before.  "as -g" shouldn't silently
> generate incorrect debug info when input assembly codes already
> contain debug directives.  AS should either issue an error or
> ignore -g.

Right, that is what this patch does for .debug_line.  gas already
doesn't generate .debug_info, .debug_aranges and .debug_abbrev if
.debug_info is non-empty, even if -g is given.

> In either case, we need a testcase  to verify it.

Right, and the documentation needs to be update.  But first we have to
know whether the gas maintainers think this is the right approach.

Cheers,

Mark

Re: Duplicate .debug_lines (Was: [PATCH 5/5] Add --gdwarf-5 to ASM_SPEC)

[Mark Wielaard]

Hi,

On Sat, Aug 29, 2020 at 07:34:35AM -0700, H.J. Lu wrote:
> On Sat, Aug 29, 2020 at 5:24 AM Mark Wielaard  wrote:
> > On Wed, Aug 26, 2020 at 04:38:21PM -0700, H.J. Lu wrote:
> > > On Wed, Aug 26, 2020 at 2:38 PM Mark Wielaard  wrote:
> > > > Would it be possible to have something like the following in gas, so
> > > > that it doesn't try generating a .debug_line section if there already
> > > > is one, even when -gdwarf-N is given (unless the assembly also
> > > > contains .loc directives because that shows the user is really
> > > > confused)?
> > > >
> > > > diff --git a/gas/dwarf2dbg.c b/gas/dwarf2dbg.c
> > > > index e4ba56d82ba..c0c09f4e9d0 100644
> > > > --- a/gas/dwarf2dbg.c
> > > > +++ b/gas/dwarf2dbg.c
> > > > @@ -2626,7 +2626,7 @@ dwarf2_init (void)
> > > >
> > > >
> > > >  /* Finish the dwarf2 debug sections.  We emit .debug.line if there
> > > > -   were any .file/.loc directives, or --gdwarf2 was given, or if the
> > > > +   were any .file/.loc directives, or --gdwarf2 was given, and if the
> > > > file has a non-empty .debug_info section and an empty .debug_line
> > > > section.  If we emit .debug_line, and the .debug_info section is
> > > > empty, we also emit .debug_info, .debug_aranges and .debug_abbrev.
> > > > @@ -2650,9 +2650,16 @@ dwarf2_finish (void)
> > > >empty_debug_line = line_seg == NULL || !seg_not_empty_p (line_seg);
> > > >
> > > >/* We can't construct a new debug_line section if we already have 
> > > > one.
> > > > - Give an error.  */
> > > > + Give an error if we have seen any .loc, otherwise trust the user
> > > > + knows what they are doing and want to generate the .debug_line
> > > > + (and all other debug sections) themselves.  */
> > > >if (all_segs && !empty_debug_line)
> > > > -as_fatal ("duplicate .debug_line sections");
> > > > +{
> > > > +  if (dwarf2_loc_directive_seen)
> > > > +   as_fatal ("duplicate .debug_line sections");
> > > > +  else
> > > > +   return;
> > > > +}
> > > >
> > > >if ((!all_segs && emit_other_sections)
> > > >|| (!emit_other_sections && !empty_debug_line))
> > >
> > > I have run into this issue before.  "as -g" shouldn't silently
> > > generate incorrect debug info when input assembly codes already
> > > contain debug directives.  AS should either issue an error or
> > > ignore -g.
> >
> > Right, that is what this patch does for .debug_line.  gas already
> > doesn't generate .debug_info, .debug_aranges and .debug_abbrev if
> > .debug_info is non-empty, even if -g is given.
> >
> > > In either case, we need a testcase  to verify it.
> >
> > Right, and the documentation needs to be update.  But first we have to
> > know whether the gas maintainers think this is the right approach.
> 
> -g should be ignored in this case.

I am not sure what you mean by "in this case", or what precisely it
means to "ignore -g".

My proposal, and what my strawman patch implements, is that gas will
generate a .debug_line section when -g is given and the debug types is
DWARF (just as it does now). Unless there is a non-empty .debug_line
section already created by the input assembly and the input assembly
does not contain any .loc directive then gas will not try to generate
a .debug_line section itself but leaves the non-empty .debug_line as
is (currently gas will generate an error in this case). But if the
input assembly does contain both .loc directives and creates a
non-empty .debug line section gas will still generate an error (as it
does now, whether or not the input assembly contains any .loc
directives).

Does this sound sane?

Thanks,

Mark

Re: Duplicate .debug_lines (Was: [PATCH 5/5] Add --gdwarf-5 to ASM_SPEC)

[Mark Wielaard]

Hi,

On Sat, Aug 29, 2020 at 08:43:30AM -0700, H.J. Lu wrote:
> On Sat, Aug 29, 2020 at 8:23 AM Mark Wielaard  wrote:
> > My proposal, and what my strawman patch implements, is that gas will
> > generate a .debug_line section when -g is given and the debug types is
> > DWARF (just as it does now). Unless there is a non-empty .debug_line
> > section already created by the input assembly and the input assembly
> > does not contain any .loc directive then gas will not try to generate
> > a .debug_line section itself but leaves the non-empty .debug_line as
> > is (currently gas will generate an error in this case). But if the
> > input assembly does contain both .loc directives and creates a
> > non-empty .debug line section gas will still generate an error (as it
> > does now, whether or not the input assembly contains any .loc
> > directives).
> >
> > Does this sound sane?
> 
> What if there is a .file directive,  but without .loc directive, like
> 
> $ gcc -c x.c -Wa,-g

That situation does not change, since in that case no .debug_*
sections are generated in the assembly file, so gas will generate
everything it currently generates.

Cheers,

Mark

Re: Duplicate .debug_lines (Was: [PATCH 5/5] Add --gdwarf-5 to ASM_SPEC)

[Mark Wielaard]

H.J. Lu wrote:
> On Sat, Aug 29, 2020 at 9:33 AM Mark Wielaard  wrote:
>> Hi,
>>
>> On Sat, Aug 29, 2020 at 08:43:30AM -0700, H.J. Lu wrote:
>>> On Sat, Aug 29, 2020 at 8:23 AM Mark Wielaard  wrote:
>>>> My proposal, and what my strawman patch implements, is that gas will
>>>> generate a .debug_line section when -g is given and the debug types is
>>>> DWARF (just as it does now). Unless there is a non-empty .debug_line
>>>> section already created by the input assembly and the input assembly
>>>> does not contain any .loc directive then gas will not try to generate
>>>> a .debug_line section itself but leaves the non-empty .debug_line as
>>>> is (currently gas will generate an error in this case). But if the
>>>> input assembly does contain both .loc directives and creates a
>>>> non-empty .debug line section gas will still generate an error (as it
>>>> does now, whether or not the input assembly contains any .loc
>>>> directives).
>>>>
>>>> Does this sound sane?
>>>
>>> What if there is a .file directive,  but without .loc directive, like
>>>
>>> $ gcc -c x.c -Wa,-g
>>
>> That situation does not change, since in that case no .debug_*
>> sections are generated in the assembly file, so gas will generate
>> everything it currently generates.
>
> Will line info be correct in this case?

Nothing about how gas generates the line table will change with the above 
proposal. It only changes the behaviour when the assembly already contains a 
line table.

[PATCH] gas: Don't error when .debug_line already exists, unless .loc was used

[Mark Wielaard]

When -g was used to generate DWARF gas would error out when a .debug_line
already exists. But when a .debug_info section already exists it would
simply skip generating one without warning or error. Do the same for
.debug_line. It is only an error when the user explicitly uses .loc
directives and also generates the .debug_line table itself.

The tests are unfortunately arch specific because the line table is only
generated when actual instructions have been emitted. Use i386 because
that is probably the most used architecture. Before this patch the new
dwarf-line-2 testcase would fail, with this patch it succeeds (and doesn't
try to add its own line table).

gas/ChangeLog:

* as.texi (-g): Explicitly mention when .debug_info and .debug_line
are generated for the DWARF format.
(Loc): Add that it is an error to both use a .loc directive and
generate a .debug_line yourself.
* dwarf2dbg.c (dwarf2_any_loc_directive_seen): New static variable.
(dwarf2_directive_loc): Set dwarf2_any_loc_directive_seen to TRUE.
(dwarf2_finish): Check dwarf2_any_loc_directive_seen before emitting
an error. Only create .debug_line if it is empty (or doesn't exist).
* testsuite/gas/i386/i386.exp: Add dwarf2-line-{1,2,3,4} when testing
an elf target.
* testsuite/gas/i386/dwarf2-line-{1,2,3,4}.{s,d,l}: New test files.
---
 gas/ChangeLog  | 14 
 gas/doc/as.texi|  7 +-
 gas/dwarf2dbg.c| 29 +---
 gas/testsuite/gas/i386/dwarf2-line-1.d | 45 +
 gas/testsuite/gas/i386/dwarf2-line-1.s | 28 
 gas/testsuite/gas/i386/dwarf2-line-2.d | 48 ++
 gas/testsuite/gas/i386/dwarf2-line-2.s | 91 ++
 gas/testsuite/gas/i386/dwarf2-line-3.d |  3 +
 gas/testsuite/gas/i386/dwarf2-line-3.l |  2 +
 gas/testsuite/gas/i386/dwarf2-line-3.s | 32 +
 gas/testsuite/gas/i386/dwarf2-line-4.d | 46 +
 gas/testsuite/gas/i386/dwarf2-line-4.s | 29 
 gas/testsuite/gas/i386/i386.exp|  5 ++
 13 files changed, 369 insertions(+), 10 deletions(-)
 create mode 100644 gas/testsuite/gas/i386/dwarf2-line-1.d
 create mode 100644 gas/testsuite/gas/i386/dwarf2-line-1.s
 create mode 100644 gas/testsuite/gas/i386/dwarf2-line-2.d
 create mode 100644 gas/testsuite/gas/i386/dwarf2-line-2.s
 create mode 100644 gas/testsuite/gas/i386/dwarf2-line-3.d
 create mode 100644 gas/testsuite/gas/i386/dwarf2-line-3.l
 create mode 100644 gas/testsuite/gas/i386/dwarf2-line-3.s
 create mode 100644 gas/testsuite/gas/i386/dwarf2-line-4.d
 create mode 100644 gas/testsuite/gas/i386/dwarf2-line-4.s

diff --git a/gas/ChangeLog b/gas/ChangeLog
index d34c08e924c..70d09729443 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,17 @@
+2020-09-07  Mark Wielaard  
+
+   * as.texi (-g): Explicitly mention when .debug_info and .debug_line
+   are generated for the DWARF format.
+   (Loc): Add that it is an error to both use a .loc directive and
+   generate a .debug_line yourself.
+   * dwarf2dbg.c (dwarf2_any_loc_directive_seen): New static variable.
+   (dwarf2_directive_loc): Set dwarf2_any_loc_directive_seen to TRUE.
+   (dwarf2_finish): Check dwarf2_any_loc_directive_seen before emitting
+   an error. Only create .debug_line if it is empty (or doesn't exist).
+   * testsuite/gas/i386/i386.exp: Add dwarf2-line-{1,2,3,4} when testing
+   an elf target.
+   * testsuite/gas/i386/dwarf2-line-{1,2,3,4}.{s,d,l}: New test files.
+
 2020-09-04  Mark Wielaard  
 
* dwarf2dbg.c (add_line_strp): New function.
diff --git a/gas/doc/as.texi b/gas/doc/as.texi
index 112eaf810cd..f2a0314310d 100644
--- a/gas/doc/as.texi
+++ b/gas/doc/as.texi
@@ -745,7 +745,9 @@ compiler output).
 @itemx --gen-debug
 Generate debugging information for each assembler source line using whichever
 debug format is preferred by the target.  This currently means either STABS,
-ECOFF or DWARF2.
+ECOFF or DWARF2.  When the debug format is DWARF then a @code{.debug_info} and
+@code{.debug_line} section is only emitted when the assembly file doesn't
+generate one itself.
 
 @item --gstabs
 Generate stabs debugging information for each assembler line.  This
@@ -5857,7 +5859,8 @@ the @code{.loc} directive will add a row to the 
@code{.debug_line} line
 number matrix corresponding to the immediately following assembly
 instruction.  The @var{fileno}, @var{lineno}, and optional @var{column}
 arguments will be applied to the @code{.debug_line} state machine before
-the row is added.
+the row is added.  It is an error for the input assembly file to generate
+a non-empty @code{.debug_line} and also use @code{loc} directives.
 
 The @var{options} are a sequence of the following tokens in any order:
 
diff --git a/gas/dwarf2dbg.c b/gas/dwarf2dbg.c
index e0ea3991b45..1c21d58c591 100644
--- a/gas/dwarf2dbg.c
+++ b/gas/dwarf2dbg.c
@@ -226,9 +226,15 @@ static unsigned int  dirs_in_use =

Re: BoF DWARF5 patches (25% .debug section size reduction)

[Mark Wielaard]

Hi,

I added some fixes to binutils gas to make sure that it always
generates DWARF5 style .debug_rnglists and .debug_line with
.debug_line_str which are more efficient than the older .debug_ranges
and .debug_line data.

There is also a pending patch to make it possible to always pass
--gdwarf-N to gas even if gcc generates its own .debug_info and
.debug_line sections.
https://sourceware.org/pipermail/binutils/2020-September/113220.html

Ideally we have a configure check to make sure that as accepts
--gdwarf-N for N={2,3,4,5} and that when the assembly file already
contains a .debug_info or .debug_line section as doesn't error out. I
haven't written that yet. It requires some way to create target
specific assembler (could be NOPs) to check the .debug_line creation
by as.

Then we can add --gdwarf-N to ASM_SPEC when gcc generates debuginfo
for DWARF version N. The below has part of that, but always uses
--gdwarf-5, and adds it even for targets that don't use DWARF, which
is obviously wrong. But I don't fully understand if I should express
this in the spec or just depend on some configure check conditionals.

To compare the .debug section sizes generated between the current gcc
master default (DWARF4) on x86_64 and using DWARF5 by default I am
using binutils master plus the above unapproved patch plus the
attached patch to gcc to enable DWARF5 by default, pass --gdwarf-5 to
as and adding Jakub's patch to keep the static member variables in C++
classes. It keep locview enabled for now to make the comparison more
fair.

For libstdc++.so we get a 21M file with current master and a 17M when
making DWARF5 the default. The debug sections look as follows:

master lib64/libstdc++.so.6.0.29:
[31] .debug_aranges  PROGBITS  001da430 00015000 00 0  1
[32] .debug_info PROGBITS  001ef430 0079f7c3 00 0  1
[33] .debug_abbrev   PROGBITS  0098ebf3 00054e1a 00 0  1
[34] .debug_line PROGBITS  009e3a0d 001779c0 00 0  1
[35] .debug_str  PROGBITS  00b5b3cd 0012fbb6 1 MS 0 0  1
[36] .debug_loc  PROGBITS  00c8af83 005c05b0 00 0  1
[37] .debug_ranges   PROGBITS  0124b533 001b1140 00 0  1

dwarf5 lib64/libstdc++.so.6.0.29:
[32] .debug_aranges  PROGBITS  001d9350 00015000 00 0  1
[33] .debug_info PROGBITS  001ee350 0078b3d1 00 0  1
[34] .debug_abbrev   PROGBITS  00979721 00055972 00 0  1
[35] .debug_line PROGBITS  009cf093 0015c20b 00 0  1
[36] .debug_str  PROGBITS  00b2b29e 00130b55 1 MS 0 0  1
[37] .debug_loclists PROGBITS  00c5bdf3 00299d88 00 0  1
[38] .debug_rnglists PROGBITS  00ef5b7b 0009357e 00 0  1
[39] .debug_line_str PROGBITS  00f890f9 1685 1 MS 0 0  1

master:
.debug_aranges  00015000 0.08M
.debug_info 0079f7c3 7.62M
.debug_abbrev   00054e1a 0.33M
.debug_line 001779c0 1.47M
.debug_str  0012fbb6 1.19M
.debug_loc  005c05b0 5.75M
.debug_ranges   001b1140 1.69M
18.13M

dwarf5:
.debug_aranges  00015000 0.08M
.debug_info 0078b3d1 7.54M
.debug_abbrev   00055972 0.33M
.debug_line 0015c20b 1.36M
.debug_str  00130b55 1.19M
.debug_loclists 00299d88 2.60M
.debug_rnglists 0009357e 0.58M
.debug_line_str 1685 0.01M
13.69M

So the total size difference is 4.4MB with the DWARF5 loclists and
rngglists being much smaller than the DWARF5 locs and ranges, the
.debug_line section is slightly smaller because all directory/file
strings are now shared in the .debug_line_str. debug_info is also a
little bit smaller.

Cheers,

Markdiff --git a/gcc/common.opt b/gcc/common.opt
index dd68c61ae1d2..755df5445905 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -3144,7 +3144,7 @@ Common Driver JoinedOrMissing Negative(gdwarf-)
 Generate debug information in default version of DWARF format.
 
 gdwarf-
-Common Driver Joined UInteger Var(dwarf_version) Init(4) Negative(gstabs)
+Common Driver Joined UInteger Var(dwarf_version) Init(5) Negative(gstabs)
 Generate debug information in DWARF v2 (or later) format.
 
 ggdb
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index bca8c856dc82..d69aa253f72b 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9057,13 +9057,14 @@ possible.
 @opindex gdwarf
 Produce debugging information in DWARF format (if that is supported).
 The value of @var{version} may be either 2, 3, 4 or 5; the default version
-for most targets is 4.  DWARF Version 5 is only experimental.
+for most targets is 5 (with the exception of vxworks and darwin which
+default to version 2).
 
 Note that with DWARF Version 2, some ports require and always
 use some non-conflicting DWARF 3 extensions in the unwind tables.
 
 Version 4 may require GDB 7.0 and @option{-fvar-tracking-assignments}
-for maximum benefit.
+for maximum benefit. Version 5 

Re: V2 [PATCH] PKG_CHECK_MODULES: Check if $pkg_cv_[]$1[]_LIBS works

[Mark Wielaard]

Hi,

On Mon, 2020-07-27 at 12:32 -0700, H.J. Lu via Binutils wrote:
> diff --git a/config/pkg.m4 b/config/pkg.m4
> index 13a8890178..45587e97c8 100644
> --- a/config/pkg.m4
> +++ b/config/pkg.m4
> @@ -147,6 +147,12 @@ AC_MSG_CHECKING([for $2])
>  _PKG_CONFIG([$1][_CFLAGS], [cflags], [$2])
>  _PKG_CONFIG([$1][_LIBS], [libs], [$2])
>  
> +dnl Check whether $pkg_cv_[]$1[]_LIBS works.
> +pkg_save_LDFLAGS="$LDFLAGS"
> +LDFLAGS="$LDFLAGS $pkg_cv_[]$1[]_LIBS"
> +AC_TRY_LINK([],[return 0;], [pkg_failed=no], [pkg_failed=yes])
> +LDFLAGS=$pkg_save_LDFLAGS
> +
>  m4_define([_PKG_TEXT], [Alternatively, you may set the environment
> variables $1[]_CFLAGS
>  and $1[]_LIBS to avoid the need to call pkg-config.
>  See the pkg-config man page for more details.])

This hunk seems wrong. You are testing whether the $pkg_cv_[]$1[]_LIBS
flags work, but they might be empty if the library wasn't found
(pkg_failed=yes). Then the AC_TRY_LINK will obviously succeed, and then
you set pkg_failed=no. But that indicates that the library was found.

Cheers,

Mark

Re: GCC wwwdocs move to git done

[Mark Wielaard]

On Wed, 2019-10-09 at 10:44 +0100, Jonathan Wakely wrote:
> On Wed, 9 Oct 2019 at 01:28, Joseph Myers wrote:
> > 
> > I've done the move of GCC wwwdocs to git (using the previously
> > posted and
> > discussed scripts), including setting up the post-receive hook to
> > do the
> > same things previously covered by the old CVS hooks, and minimal
> > updates
> > to the web pages dealing with the CVS setup for wwwdocs.
> 
> Thanks, Joseph.
> 
> I just pushed a change and although it worked and appeared on
> gcc.gnu.org, I got an error message during the push:
> 
> remote: hooks/post-receive: line 14: /www/gcc/updatelog: Permission
> denied
> remote:   Updating /www/gcc/htdocs/gcc-10/changes.html

/www/gcc/updatelog is where the logs of the post-receive are written.
It was only readable by the gcc group, I did a chmod g+w.
Now things should work without warning.

Cheers,

Mark

[RFC] analyzer: Add exit, and _exit replacement, to sm-signal.

[Mark Wielaard]

Hi,

While trying out -fanalyzer on the bzip2 source code I noticed that it
did warn about some unsafe calls in the signal handler, but din't warn
about the exit call:
https://sourceware.org/pipermail/bzip2-devel/2020q2/000107.html

It was easy to add exit to the async_signal_unsafe_fns, but since
there is a signal safe _exit call (which is what you should use in a
signal handler, so no unsafe calls are made, like to the at_exit
handlers) I also wanted to add a fixit hint.

The fixit hint is emitted, but it is somewhat hard to see. Is there a
better way to do this for analyzer warnings so that it is a bit more
prominent?

This is how it currently looks:

/opt/local/gcc/bin/gcc -g -O2 -fanalyzer -c bzip2.c
bzip2.c: In function ‘mySIGSEGVorSIGBUScatcher’:
bzip2.c:874:4: warning: call to ‘exit’ from within signal handler [CWE-479] 
[-Wanalyzer-unsafe-call-within-signal-handler]
  874 |exit(exitValue);
  |^~~
  |_exit
  ‘main’: events 1-2
|
| 1784 | IntNative main ( IntNative argc, Char *argv[] )
|  |   ^~~~
|  |   |
|  |   (1) entry to ‘main’
|..
| 1800 |smallMode   = False;
|  |~
|  ||
|  |(2) registering ‘mySIGSEGVorSIGBUScatcher’ as signal handler
|
  event 3
|
|cc1:
| (3): later on, when the signal is delivered to the process
|
+--> ‘mySIGSEGVorSIGBUScatcher’: events 4-5
   |
   |  816 | void mySIGSEGVorSIGBUScatcher ( IntNative n )
   |  |  ^~~~
   |  |  |
   |  |  (4) entry to ‘mySIGSEGVorSIGBUScatcher’
   |..
   |  874 |exit(exitValue);
   |  |~~~
   |  ||
   |  |(5) call to ‘exit’ from within signal handler
   |

Thanks,

Mark

---
 gcc/analyzer/sm-signal.cc | 34 +-
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/gcc/analyzer/sm-signal.cc b/gcc/analyzer/sm-signal.cc
index 5935e229e77c..99e87706af63 100644
--- a/gcc/analyzer/sm-signal.cc
+++ b/gcc/analyzer/sm-signal.cc
@@ -108,8 +108,9 @@ class signal_unsafe_call
 {
 public:
   signal_unsafe_call (const signal_state_machine &sm, const gcall *unsafe_call,
- tree unsafe_fndecl)
-  : m_sm (sm), m_unsafe_call (unsafe_call), m_unsafe_fndecl (unsafe_fndecl)
+ tree unsafe_fndecl, const char *replacement)
+  : m_sm (sm), m_unsafe_call (unsafe_call), m_unsafe_fndecl (unsafe_fndecl),
+m_replacement (replacement)
   {
 gcc_assert (m_unsafe_fndecl);
   }
@@ -126,6 +127,8 @@ public:
 diagnostic_metadata m;
 /* CWE-479: Signal Handler Use of a Non-reentrant Function.  */
 m.add_cwe (479);
+if (m_replacement != NULL)
+  rich_loc->add_fixit_replace (m_replacement);
 return warning_meta (rich_loc, m,
 OPT_Wanalyzer_unsafe_call_within_signal_handler,
 "call to %qD from within signal handler",
@@ -156,6 +159,7 @@ private:
   const signal_state_machine &m_sm;
   const gcall *m_unsafe_call;
   tree m_unsafe_fndecl;
+  const char *m_replacement;
 };
 
 /* signal_state_machine's ctor.  */
@@ -259,6 +263,7 @@ get_async_signal_unsafe_fns ()
   // TODO: populate this list more fully
   static const char * const async_signal_unsafe_fns[] = {
 /* This array must be kept sorted.  */
+"exit",
 "fprintf",
 "free",
 "malloc",
@@ -286,6 +291,20 @@ signal_unsafe_p (tree fndecl)
   return fs.contains_decl_p (fndecl);
 }
 
+/* Returns a replacement function as text if it exists.  Currently
+   only "exit" has a signal-safe replacement "_exit", which does
+   slightly less, but can be used in a signal handler.  */
+const char *
+replacement_fn (tree fndecl)
+{
+  gcc_assert (fndecl && DECL_P (fndecl));
+
+  if (strcmp ("exit", IDENTIFIER_POINTER (DECL_NAME (fndecl))) == 0)
+return "_exit";
+
+  return NULL;
+}
+
 /* Implementation of state_machine::on_stmt vfunc for signal_state_machine.  */
 
 bool
@@ -315,9 +334,14 @@ signal_state_machine::on_stmt (sm_context *sm_ctxt,
   if (const gcall *call = dyn_cast  (stmt))
if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call))
  if (signal_unsafe_p (callee_fndecl))
-   sm_ctxt->warn_for_state (node, stmt, NULL_TREE, m_in_signal_handler,
-new signal_unsafe_call (*this, call,
-callee_fndecl));
+   {
+ const char *replace = replacement_fn (callee_fndecl);
+ sm_ctxt->warn_for_state (node, stmt, NULL_TREE,
+  m_in_signal_handler,
+  new signal_unsafe_call (*this, call,
+  callee_fndecl,
+ 

Re: [RFC] analyzer: Add exit, and _exit replacement, to sm-signal.

[Mark Wielaard]

Hi David,

On Sun, 2020-05-17 at 18:53 -0400, David Malcolm wrote:
> BTW, it looks like it's using the wrong location for event (2).  It
> ought to be showing a call to "signal", not an assignment.  Please can
> you file a bug about this, and attach the source in question so I can
> take a look at some point.

As briefly discussed on irc, this is an independent issue, it can be
shown with pristine gcc 10.1 and bzip2 1.0.8 source code:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95188

A workaround is to use -fanalyzer-fine-grained.

Cheers,

Mark

Re: [RFC] analyzer: Add exit, and _exit replacement, to sm-signal.

[Mark Wielaard]

Hi David,

On Mon, May 18, 2020 at 08:45:36AM -0400, David Malcolm wrote:
> Also, m_unsafe_fndecl is a field of signal_unsafe_call, so we can delay
> calling replacement_fn until inside signal_unsafe_call::emit, after the
> warning has been emitted.
> 
> It could even become a member function of signal_unsafe_call, giving
> something like this for signal_unsafe_call::emit:

Thanks for all the hints. I adopted all your suggestions and the
warning plus note now looks like:

/opt/local/gcc/bin/gcc -g -O2 -fanalyzer -c bzip2.c
bzip2.c: In function ‘mySIGSEGVorSIGBUScatcher’:
bzip2.c:874:4: warning: call to ‘exit’ from within signal handler [CWE-479] 
[-Wanalyzer-unsafe-call-within-signal-handler]
  874 |exit(exitValue);
  |^~~
  ‘main’: events 1-2
|
| 1784 | IntNative main ( IntNative argc, Char *argv[] )
|  |   ^~~~
|  |   |
|  |   (1) entry to ‘main’
|..
| 1800 |smallMode   = False;
|  |~
|  ||
|  |(2) registering ‘mySIGSEGVorSIGBUScatcher’ as signal handler
|
  event 3
|
|cc1:
| (3): later on, when the signal is delivered to the process
|
+--> ‘mySIGSEGVorSIGBUScatcher’: events 4-5
   |
   |  816 | void mySIGSEGVorSIGBUScatcher ( IntNative n )
   |  |  ^~~~
   |  |  |
   |  |  (4) entry to ‘mySIGSEGVorSIGBUScatcher’
   |..
   |  874 |exit(exitValue);
   |  |~~~
   |  ||
   |  |(5) call to ‘exit’ from within signal handler
   |
bzip2.c:874:4: note: ‘_exit’ is a possible signal-safe alternative for ‘exit’
  874 |exit(exitValue);
  |^~~
  |_exit

I also added a testcase. How about the attached?

Thanks,

Mark>From de08b1f0e1db85241bce22cdd920b351489af446 Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Sun, 17 May 2020 23:50:41 +0200
Subject: [PATCH] analyzer: Add exit, and _exit replacement, to sm-signal.

Warn about usage of exit in signal handler and offer _exit as alternative.

gcc/analyzer/ChangeLog:
	* sm-signal.cc(signal_unsafe_call::emit): Possibly add
	gcc_rich_location note for replacement.
	(signal_unsafe_call::get_replacement_fn): New private function.
	(get_async_signal_unsafe_fns): Add "exit".

gcc/testsuite/ChangeLog:
	* gcc.dg/analyzer/signal-exit.c: New testcase.
---
 gcc/analyzer/sm-signal.cc   | 40 ++---
 gcc/testsuite/gcc.dg/analyzer/signal-exit.c | 23 
 2 files changed, 59 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-exit.c

diff --git a/gcc/analyzer/sm-signal.cc b/gcc/analyzer/sm-signal.cc
index 5935e229e77c..d267548324e8 100644
--- a/gcc/analyzer/sm-signal.cc
+++ b/gcc/analyzer/sm-signal.cc
@@ -58,6 +58,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "analyzer/exploded-graph.h"
 #include "analyzer/function-set.h"
 #include "analyzer/analyzer-selftests.h"
+#include "gcc-rich-location.h"
 
 #if ENABLE_ANALYZER
 
@@ -123,13 +124,28 @@ public:
 
   bool emit (rich_location *rich_loc) FINAL OVERRIDE
   {
+auto_diagnostic_group d;
 diagnostic_metadata m;
 /* CWE-479: Signal Handler Use of a Non-reentrant Function.  */
 m.add_cwe (479);
-return warning_meta (rich_loc, m,
-			 OPT_Wanalyzer_unsafe_call_within_signal_handler,
-			 "call to %qD from within signal handler",
-			 m_unsafe_fndecl);
+if (warning_meta (rich_loc, m,
+		  OPT_Wanalyzer_unsafe_call_within_signal_handler,
+		  "call to %qD from within signal handler",
+		  m_unsafe_fndecl))
+  {
+	/* If we know a possible alternative function, add a note
+	   suggesting the replacement.  */
+	if (const char *replacement = get_replacement_fn ())
+	  {
+	gcc_rich_location note_rich_loc (gimple_location (m_unsafe_call));
+	note_rich_loc.add_fixit_replace (replacement);
+	inform (¬e_rich_loc,
+		"%qs is a possible signal-safe alternative for %qD",
+		replacement, m_unsafe_fndecl);
+	  }
+	return true;
+  }
+return false;
   }
 
   label_text describe_state_change (const evdesc::state_change &change)
@@ -156,6 +172,21 @@ private:
   const signal_state_machine &m_sm;
   const gcall *m_unsafe_call;
   tree m_unsafe_fndecl;
+
+  /* Returns a replacement function as text if it exists.  Currently
+ only "exit" has a signal-safe replacement "_exit", which does
+ slightly less, but can be used in a signal handler.  */
+  const char *
+  get_replacement_fn ()
+  {
+gcc_assert (m_unsafe_fndecl && DECL_P (m_unsafe_fndecl));
+
+if (strcmp ("exit",
+		IDENTIFIER_POINTER (DECL_NAME (m_unsafe_fndecl))) == 0)
+  return "_exit";

Re: [RFC] analyzer: Add exit, and _exit replacement, to sm-signal.

[Mark Wielaard]

Hi David,

On Mon, May 18, 2020 at 11:09:18AM -0400, David Malcolm wrote:
> On Mon, 2020-05-18 at 16:47 +0200, Mark Wielaard wrote:
> > On Mon, May 18, 2020 at 08:45:36AM -0400, David Malcolm wrote:
> > > Also, m_unsafe_fndecl is a field of signal_unsafe_call, so we can
> > > delay
> > > calling replacement_fn until inside signal_unsafe_call::emit, after
> > > the
> > > warning has been emitted.
> > > 
> > > It could even become a member function of signal_unsafe_call,
> > > giving
> > > something like this for signal_unsafe_call::emit:
> > 
> > Thanks for all the hints. I adopted all your suggestions and the
> > warning plus note now looks like:
> > [...] 
> > bzip2.c:874:4: note: ‘_exit’ is a possible signal-safe alternative
> > for ‘exit’
> >   874 |exit(exitValue);
> >   |^~~
> >   |_exit
> > 
> > I also added a testcase. How about the attached?
> 
> Overall, I like it, but it looks like there's a problem with the
> location of the fix-it hint: it looks like it might be replacing the
> whole of the underlined section of the call, argument, parentheses and
> all, with "_exit", rather than just the "exit" token.  I wonder if the
> location of that token is still available in the middle-end by the time
> the analyzer runs.
> 
> What does -fdiagnostics-generate-patch emit?

--- bzip2.c
+++ bzip2.c
@@ -871,7 +871,7 @@
   terribly wrong. Trying to clean up might fail spectacularly. */
 
if (opMode == OM_Z) setExit(3); else setExit(2);
-   exit(exitValue);
+   _exit;
 }

Hmmm, back to the drawing board it seems.

Thanks,

Mark

Re: [RFC] analyzer: Add exit, and _exit replacement, to sm-signal.

[Mark Wielaard]

On Mon, May 18, 2020 at 05:24:58PM +0200, Mark Wielaard wrote:
> On Mon, May 18, 2020 at 11:09:18AM -0400, David Malcolm wrote:
> > Overall, I like it, but it looks like there's a problem with the
> > location of the fix-it hint: it looks like it might be replacing the
> > whole of the underlined section of the call, argument, parentheses and
> > all, with "_exit", rather than just the "exit" token.  I wonder if the
> > location of that token is still available in the middle-end by the time
> > the analyzer runs.
> > 
> > What does -fdiagnostics-generate-patch emit?
> 
> --- bzip2.c
> +++ bzip2.c
> @@ -871,7 +871,7 @@
>terribly wrong. Trying to clean up might fail spectacularly. */
>  
> if (opMode == OM_Z) setExit(3); else setExit(2);
> -   exit(exitValue);
> +   _exit;
>  }
> 
> Hmmm, back to the drawing board it seems.

It seems it is impossible to untangle the gimple call. We do have the
function decl, but this is associated with the declaration of the
function. Since we know the call starts with a known symbol identifier
(that is all on one line), it seems we should be able to truncate the
source_range for the call location to just that prefix. But it is
incredibly hard to manipulate locations. Neither seems it simple to
get the actual text of the location or a range to sanity check what we
are doing.

So I am afraid we have to drop the actual fixit. But we can still add
the note itself. The diagnostic now looks as follows:

/opt/local/install/gcc/bin/gcc -g -O2 -fanalyzer -fanalyzer-fine-grained -c 
bzip2.c
bzip2.c: In function ‘mySIGSEGVorSIGBUScatcher’:
bzip2.c:874:4: warning: call to ‘exit’ from within signal handler [CWE-479] 
[-Wanalyzer-unsafe-call-within-signal-handler]
  874 |exit(exitValue);
  |^~~
  ‘main’: events 1-2
|
| 1784 | IntNative main ( IntNative argc, Char *argv[] )
|  |   ^~~~
|  |   |
|  |   (1) entry to ‘main’
|..
| 1816 |signal (SIGSEGV, mySIGSEGVorSIGBUScatcher);
|  |~~
|  ||
|  |(2) registering ‘mySIGSEGVorSIGBUScatcher’ as signal handler
|
  event 3
|
|cc1:
| (3): later on, when the signal is delivered to the process
|
+--> ‘mySIGSEGVorSIGBUScatcher’: events 4-5
   |
   |  816 | void mySIGSEGVorSIGBUScatcher ( IntNative n )
   |  |  ^~~~
   |  |  |
   |  |  (4) entry to ‘mySIGSEGVorSIGBUScatcher’
   |..
   |  874 |exit(exitValue);
   |  |~~~
   |  ||
   |  |(5) call to ‘exit’ from within signal handler
   |
bzip2.c:874:4: note: ‘_exit’ is a possible signal-safe alternative for ‘exit’
  874 |exit(exitValue);
  |^~~

What do you think of the attached patch?

Thanks,

Mark>From e47d9c6898b0db7f56cff03096b3fccaeb801efa Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Sun, 17 May 2020 23:50:41 +0200
Subject: [PATCH] analyzer: Add exit, and _exit replacement, to sm-signal.

Warn about using exit in signal handler and suggest _exit as alternative.

gcc/analyzer/ChangeLog:
	* sm-signal.cc(signal_unsafe_call::emit): Possibly add
	gcc_rich_location note for replacement.
	(signal_unsafe_call::get_replacement_fn): New private function.
	(get_async_signal_unsafe_fns): Add "exit".

gcc/testsuite/ChangeLog:
	* gcc.dg/analyzer/signal-exit.c: New testcase.
---
 gcc/analyzer/sm-signal.cc   | 43 +++--
 gcc/testsuite/gcc.dg/analyzer/signal-exit.c | 23 +++
 2 files changed, 62 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-exit.c

diff --git a/gcc/analyzer/sm-signal.cc b/gcc/analyzer/sm-signal.cc
index 5935e229e77c..379f58004219 100644
--- a/gcc/analyzer/sm-signal.cc
+++ b/gcc/analyzer/sm-signal.cc
@@ -123,13 +123,32 @@ public:
 
   bool emit (rich_location *rich_loc) FINAL OVERRIDE
   {
+auto_diagnostic_group d;
 diagnostic_metadata m;
 /* CWE-479: Signal Handler Use of a Non-reentrant Function.  */
 m.add_cwe (479);
-return warning_meta (rich_loc, m,
-			 OPT_Wanalyzer_unsafe_call_within_signal_handler,
-			 "call to %qD from within signal handler",
-			 m_unsafe_fndecl);
+if (warning_meta (rich_loc, m,
+		  OPT_Wanalyzer_unsafe_call_within_signal_handler,
+		  "call to %qD from within signal handler",
+		  m_unsafe_fndecl))
+  {
+	/* If we know a possible alternative function, add a note
+	   suggesting the replacement.  */
+	if (const char *replacement = get_replacement_fn ())
+	  {
+	location_t note_loc = gimple_location (m_unsafe_call);
+	/* It would be nice to add a fixit, but the gimple call
+	   location co

[PATCH] Suggest including for bool, true and false

[Mark Wielaard]

Currently gcc suggests to use _Bool instead of bool and doesn't give
any suggestions when true or false are used, but undefined. This patch
makes it so that (for C99 or higher) a fixit hint is emitted to include
.

gcc/c-family/ChangeLog:

* known-headers.cc (get_stdlib_header_for_name): Return
" for "bool", "true" or "false" when STDLIB_C and
flag_isoc99.

gcc/testsuite/ChangeLog:

* gcc.dg/spellcheck-stdbool.c: New test.
---
 gcc/c-family/known-headers.cc |  8 
 gcc/testsuite/gcc.dg/spellcheck-stdbool.c | 17 +
 2 files changed, 25 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-stdbool.c

diff --git a/gcc/c-family/known-headers.cc b/gcc/c-family/known-headers.cc
index a21166860c0f..183ce2834afd 100644
--- a/gcc/c-family/known-headers.cc
+++ b/gcc/c-family/known-headers.cc
@@ -158,6 +158,14 @@ get_stdlib_header_for_name (const char *name, enum stdlib 
lib)
   for (size_t i = 0; i < num_hints; i++)
 if (strcmp (name, hints[i].name) == 0)
   return hints[i].header[lib];
+
+  /* Only for C99 and higher.  */
+  if (lib == STDLIB_C && flag_isoc99)
+if (strcmp (name, "bool") == 0
+   || strcmp (name, "true") == 0
+   || strcmp (name, "false") == 0)
+  return "";
+
   return NULL;
 }
 
diff --git a/gcc/testsuite/gcc.dg/spellcheck-stdbool.c 
b/gcc/testsuite/gcc.dg/spellcheck-stdbool.c
new file mode 100644
index ..01f12da35cfe
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-stdbool.c
@@ -0,0 +1,17 @@
+/* { dg-options "-std=c99" } */
+/* Missing .  */
+
+bool b; /* { dg-error "unknown type name 'bool'" } */
+/* { dg-message "'bool' is defined in header ''; did you forget to 
'#include '?" "" { target *-*-* } .-1 } */
+
+int test_true (void)
+{
+  return true; /* { dg-error "'true' undeclared" } */
+  /* { dg-message "'true' is defined in header ''; did you forget 
to '#include '?" "" { target *-*-* } .-1 } */
+}
+
+int test_false (void)
+{
+  return false; /* { dg-error "'false' undeclared" } */
+  /* { dg-message "'false' is defined in header ''; did you forget 
to '#include '?" "" { target *-*-* } .-1 } */
+}
-- 
2.20.1

[PATCH] Suggest including or for [u]int[8|16|32|64]_t

[Mark Wielaard]

Plus [u]intptr_t and associated constants.

Refactor the bool, true, false,  code so it fits into the
new table based design.

gcc/c-family/ChangeLog:

* known-headers.cc (get_stdlib_header_for_name): Add a new

gcc/testsuite/ChangeLog:

* gcc.dg/spellcheck-stdint.c: New test.
* g++.dg/spellcheck-stdint.C: Likewise.
---
 gcc/c-family/known-headers.cc| 42 ---
 gcc/testsuite/g++.dg/spellcheck-stdint.C | 68 
 gcc/testsuite/gcc.dg/spellcheck-stdint.c | 62 +
 3 files changed, 166 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/spellcheck-stdint.C
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-stdint.c

diff --git a/gcc/c-family/known-headers.cc b/gcc/c-family/known-headers.cc
index 183ce2834afd..1e2bf49c439a 100644
--- a/gcc/c-family/known-headers.cc
+++ b/gcc/c-family/known-headers.cc
@@ -159,12 +159,42 @@ get_stdlib_header_for_name (const char *name, enum stdlib 
lib)
 if (strcmp (name, hints[i].name) == 0)
   return hints[i].header[lib];
 
-  /* Only for C99 and higher.  */
-  if (lib == STDLIB_C && flag_isoc99)
-if (strcmp (name, "bool") == 0
-   || strcmp (name, "true") == 0
-   || strcmp (name, "false") == 0)
-  return "";
+  static const stdlib_hint c99_cxx11_hints[] = {
+/* .  Defined natively in C++.  */
+{"bool", {"", NULL} },
+{"true", {"", NULL} },
+{"false", {"", NULL} },
+
+/*  and .  */
+{"int8_t", {"", ""} },
+{"uint8_t", {"", ""} },
+{"int16_t", {"", ""} },
+{"uint16_t", {"", ""} },
+{"int32_t", {"", ""} },
+{"uint32_t", {"", ""} },
+{"int64_t", {"", ""} },
+{"uint64_t", {"", ""} },
+{"intptr_t", {"", ""} },
+{"uintptr_t", {"", ""} },
+{"INT8_MAX", {"", ""} },
+{"INT16_MAX", {"", ""} },
+{"INT32_MAX", {"", ""} },
+{"INT64_MAX", {"", ""} },
+{"UINT8_MAX", {"", ""} },
+{"UINT16_MAX", {"", ""} },
+{"UINT32_MAX", {"", ""} },
+{"UINT64_MAX", {"", ""} },
+{"INTPTR_MAX", {"", ""} },
+{"UINTPTR_MAX", {"", ""} }
+  };
+
+  const size_t num_c99_cxx11_hints = sizeof (c99_cxx11_hints)
+/ sizeof (c99_cxx11_hints[0]);
+  if ((lib == STDLIB_C && flag_isoc99)
+  || (lib == STDLIB_CPLUSPLUS && cxx_dialect >= cxx11 ))
+for (size_t i = 0; i < num_c99_cxx11_hints; i++)
+  if (strcmp (name, c99_cxx11_hints[i].name) == 0)
+   return c99_cxx11_hints[i].header[lib];
 
   return NULL;
 }
diff --git a/gcc/testsuite/g++.dg/spellcheck-stdint.C 
b/gcc/testsuite/g++.dg/spellcheck-stdint.C
new file mode 100644
index ..b9ce3b7aed81
--- /dev/null
+++ b/gcc/testsuite/g++.dg/spellcheck-stdint.C
@@ -0,0 +1,68 @@
+/* { dg-options "-std=c++11" } */
+/* Missing .  */
+
+char c = INT8_MAX; // { dg-error "'INT8_MAX' was not declared" }
+// { dg-message "'INT8_MAX' is defined in header ''; did you forget 
to '#include '?" "" { target *-*-* } .-1 }
+
+short s = INT16_MAX; // { dg-error "'INT16_MAX' was not declared" }
+// { dg-message "'INT16_MAX' is defined in header ''; did you forget 
to '#include '?" "" { target *-*-* } .-1 }
+
+int i = INT32_MAX; // { dg-error "'INT32_MAX' was not declared" }
+// { dg-message "'INT32_MAX' is defined in header ''; did you forget 
to '#include '?" "" { target *-*-* } .-1 }
+
+long l = INT64_MAX; // { dg-error "'INT64_MAX' was not declared" }
+// { dg-message "'INT64_MAX' is defined in header ''; did you forget 
to '#include '?" "" { target *-*-* } .-1 }
+
+intptr_t test_intptr (void) // { dg-error "'intptr_t' does not name a type" }
+// { dg-message "'intptr_t' is defined in header ''; did you forget 
to '#include '?" "" { target *-*-* } .-1 }
+{
+  return 0;
+}
+
+int test_intptr_max (void)
+{
+  return (int) INTPTR_MAX; // { dg-error "'INTPTR_MAX' was not declared" }
+// { dg-message "'INTPTR_MAX' is defined in header ''; did you forget 
to '#include '?" "" { target *-*-* } .-1 }
+}
+
+uintptr_t test_uintptr (void) // { dg-error "'uintptr_t' does not name a type" 
}
+// { dg-message "'uintptr_t' is defined in header ''; did you forget 
to '#include '?" "" { target *-*-* } .-1 }
+{
+  return 0;
+}
+
+unsigned int test_uintptr_max (void)
+{
+  return (unsigned int) UINTPTR_MAX; // { dg-error "'UINTPTR_MAX' was not 
declared" }
+// { dg-message "'UINTPTR_MAX' is defined in header ''; did you 
forget to '#include '?" "" { target *-*-* } .-1 }
+}
+
+int8_t i8; // { dg-error "'int8_t' does not name a type" }
+// { dg-message "'int8_t' is defined in header ''; did you forget to 
'#include '?" "" { target *-*-* } .-1 }
+int16_t i16; // { dg-error "'int16_t' does not name a type" }
+// { dg-message "'int16_t' is defined in header ''; did you forget to 
'#include '?" "" { target *-*-* } .-1 }
+int32_t i32; // { dg-error "'int32_t' does not name a type" }
+// { dg-message "'int32_t' is defined in header ''; did you forget to 
'#include '?" "" { target *-*-* } .-1 }
+int64_t i64; // { dg-error "'int6

Re: [RFC] analyzer: Add exit, and _exit replacement, to sm-signal.

[Mark Wielaard]

Hi,

On Mon, May 18, 2020 at 07:30:58PM -0400, Marek Polacek wrote:
> > +  /* Returns a replacement function as text if it exists.  Currently
> > + only "exit" has a signal-safe replacement "_exit", which does
> > + slightly less, but can be used in a signal handler.  */
> > +  const char *
> > +  get_replacement_fn ()
> > +  {
> > +gcc_assert (m_unsafe_fndecl && DECL_P (m_unsafe_fndecl));
> > +
> > +if (strcmp ("exit",
> > +   IDENTIFIER_POINTER (DECL_NAME (m_unsafe_fndecl))) == 0)
> 
> Instead of strcmp, you should be able to use id_equal here, making this a bit
> simpler.

That does make it a little easier to read.
How about the attached?

Thanks,

Mark
>From 2406c5a70b407052bc4099a80ecddd4ed3d62385 Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Sun, 17 May 2020 23:50:41 +0200
Subject: [PATCH] analyzer: Add exit, and _exit replacement, to sm-signal.

Warn about using exit in signal handler and suggest _exit as alternative.

gcc/analyzer/ChangeLog:
	* sm-signal.cc(signal_unsafe_call::emit): Possibly add
	gcc_rich_location note for replacement.
	(signal_unsafe_call::get_replacement_fn): New private function.
	(get_async_signal_unsafe_fns): Add "exit".

gcc/testsuite/ChangeLog:
	* gcc.dg/analyzer/signal-exit.c: New testcase.
---
 gcc/analyzer/sm-signal.cc   | 42 +++--
 gcc/testsuite/gcc.dg/analyzer/signal-exit.c | 23 +++
 2 files changed, 61 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-exit.c

diff --git a/gcc/analyzer/sm-signal.cc b/gcc/analyzer/sm-signal.cc
index 5935e229e77c..c0020321071e 100644
--- a/gcc/analyzer/sm-signal.cc
+++ b/gcc/analyzer/sm-signal.cc
@@ -123,13 +123,32 @@ public:
 
   bool emit (rich_location *rich_loc) FINAL OVERRIDE
   {
+auto_diagnostic_group d;
 diagnostic_metadata m;
 /* CWE-479: Signal Handler Use of a Non-reentrant Function.  */
 m.add_cwe (479);
-return warning_meta (rich_loc, m,
-			 OPT_Wanalyzer_unsafe_call_within_signal_handler,
-			 "call to %qD from within signal handler",
-			 m_unsafe_fndecl);
+if (warning_meta (rich_loc, m,
+		  OPT_Wanalyzer_unsafe_call_within_signal_handler,
+		  "call to %qD from within signal handler",
+		  m_unsafe_fndecl))
+  {
+	/* If we know a possible alternative function, add a note
+	   suggesting the replacement.  */
+	if (const char *replacement = get_replacement_fn ())
+	  {
+	location_t note_loc = gimple_location (m_unsafe_call);
+	/* It would be nice to add a fixit, but the gimple call
+	   location covers the whole call expression.  It isn't
+	   currently possible to cut this down to just the call
+	   symbol.  So the fixit would replace too much.
+	   note_rich_loc.add_fixit_replace (replacement); */
+	inform (note_loc,
+		"%qs is a possible signal-safe alternative for %qD",
+		replacement, m_unsafe_fndecl);
+	  }
+	return true;
+  }
+return false;
   }
 
   label_text describe_state_change (const evdesc::state_change &change)
@@ -156,6 +175,20 @@ private:
   const signal_state_machine &m_sm;
   const gcall *m_unsafe_call;
   tree m_unsafe_fndecl;
+
+  /* Returns a replacement function as text if it exists.  Currently
+ only "exit" has a signal-safe replacement "_exit", which does
+ slightly less, but can be used in a signal handler.  */
+  const char *
+  get_replacement_fn ()
+  {
+gcc_assert (m_unsafe_fndecl && DECL_P (m_unsafe_fndecl));
+
+if (id_equal ("exit", DECL_NAME (m_unsafe_fndecl)))
+  return "_exit";
+
+return NULL;
+  }
 };
 
 /* signal_state_machine's ctor.  */
@@ -259,6 +292,7 @@ get_async_signal_unsafe_fns ()
   // TODO: populate this list more fully
   static const char * const async_signal_unsafe_fns[] = {
 /* This array must be kept sorted.  */
+"exit",
 "fprintf",
 "free",
 "malloc",
diff --git a/gcc/testsuite/gcc.dg/analyzer/signal-exit.c b/gcc/testsuite/gcc.dg/analyzer/signal-exit.c
new file mode 100644
index ..ed1583d4a920
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/signal-exit.c
@@ -0,0 +1,23 @@
+/* Example of a bad call within a signal handler with replacement
+   alternative.  'handler' calls 'exit', and 'exit' is not allowed
+   from a signal handler.  But '_exit' is allowed.  */
+
+#include 
+#include 
+
+extern void body_of_program(void);
+
+static void handler(int signum)
+{
+  exit(1); /* { dg-warning "call to 'exit' from within signal handler" } */
+  /* { dg-message "note: '_exit' is a possible signal-safe alternative for 'exit'" "" { target *-*-* } 12 } */
+}
+
+int main(int argc, const char *argv)
+{
+  signal(SIGINT, handler); /* { dg-message "registering 'handler' as signal handler" } */
+
+  body_of_program();
+
+  return 0;
+}
-- 
2.20.1

RFC: Provide diagnostic hints for missing inttypes.h string constants.

[Mark Wielaard]

This is on top of the stdbool.h and stdint.h patches.

This adds a flag to c_parser so we know when we were trying to
constract a string literal. If there is a parse error and we were
constructing a string literal, and the next token is an unknown
identifier name, and we know there is a standard header that defines
that name as a string literal, then add a missing header hint to
the error messsage.

I haven't checked yet if we can do something similar for the C++
parser. And the testcase needs to be extended a bit. But I hope the
direction is OK.

For the following source:

const char *hex64_fmt =  PRIx64;
const char *msg_fmt = "Provide %" PRIx64 "\n";

void foo (uint32_t v)
{
  printf ("%" PRIu32, v);
}

We will get the following:

$ /opt/local/gcc/bin/gcc -c t.c
t.c:4:26: error: ‘PRIx64’ undeclared here (not in a function)
4 | const char *hex64_fmt =  PRIx64;
  |  ^~
t.c:3:1: note: ‘PRIx64’ is defined in header ‘’; did you forget to 
‘#include ’?
2 | #include 
  +++ |+#include 
3 |
t.c:5:35: error: expected ‘,’ or ‘;’ before ‘PRIx64’
5 | const char *msg_fmt = "Provide %" PRIx64 "\n";
  |   ^~
t.c:5:35: note: ‘PRIx64’ is defined in header ‘’; did you forget to 
‘#include ’?
t.c: In function ‘foo’:
t.c:9:14: error: expected ‘)’ before ‘PRIu32’
9 |   printf ("%" PRIu32, v);
  |  ^~~
  |  )
t.c:9:15: note: ‘PRIu32’ is defined in header ‘’; did you forget to 
‘#include ’?
9 |   printf ("%" PRIu32, v);
  |   ^~
---
 gcc/c-family/known-headers.cc  | 88 ++
 gcc/c-family/known-headers.h   |  2 +
 gcc/c/c-parser.c   | 29 +++
 gcc/testsuite/gcc.dg/spellcheck-inttypes.c | 32 
 4 files changed, 151 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-inttypes.c

diff --git a/gcc/c-family/known-headers.cc b/gcc/c-family/known-headers.cc
index 1e2bf49c439a..b9772af21863 100644
--- a/gcc/c-family/known-headers.cc
+++ b/gcc/c-family/known-headers.cc
@@ -46,6 +46,71 @@ struct stdlib_hint
   const char *header[NUM_STDLIBS];
 };
 
+/* These can be used as string macros or as identifiers. Must all be
+   string-literals.  Used in get_stdlib_header_for_name and
+   get_c_stdlib_header_for_string_macro_name.  */
+static const stdlib_hint c99_cxx11_macro_hints[] = {
+/*  and .  */
+{"PRId8", {"", ""} },
+{"PRId16", {"", ""} },
+{"PRId32", {"", ""} },
+{"PRId64", {"", ""} },
+{"PRIi8", {"", ""} },
+{"PRIi16", {"", ""} },
+{"PRIi32", {"", ""} },
+{"PRIi64", {"", ""} },
+{"PRIo8", {"", ""} },
+{"PRIo16", {"", ""} },
+{"PRIo32", {"", ""} },
+{"PRIo64", {"", ""} },
+{"PRIu8", {"", ""} },
+{"PRIu16", {"", ""} },
+{"PRIu32", {"", ""} },
+{"PRIu64", {"", ""} },
+{"PRIx8", {"", ""} },
+{"PRIx16", {"", ""} },
+{"PRIx32", {"", ""} },
+{"PRIx64", {"", ""} },
+{"PRIX8", {"", ""} },
+{"PRIX16", {"", ""} },
+{"PRIX32", {"", ""} },
+{"PRIX64", {"", ""} },
+
+{"PRIdPTR", {"", ""} },
+{"PRIiPTR", {"", ""} },
+{"PRIoPTR", {"", ""} },
+{"PRIuPTR", {"", ""} },
+{"PRIxPTR", {"", ""} },
+{"PRIXPTR", {"", ""} },
+
+{"SCNd8", {"", ""} },
+{"SCNd16", {"", ""} },
+{"SCNd32", {"", ""} },
+{"SCNd64", {"", ""} },
+{"SCNi8", {"", ""} },
+{"SCNi16", {"", ""} },
+{"SCNi32", {"", ""} },
+{"SCNi64", {"", ""} },
+{"SCNo8", {"", ""} },
+{"SCNo16", {"", ""} },
+{"SCNo32", {"", ""} },
+{"SCNo64", {"", ""} },
+{"SCNu8", {"", ""} },
+{"SCNu16", {"", ""} },
+{"SCNu32", {"", ""} },
+{"SCNu64", {"", ""} },
+{"SCNx8", {"", ""} },
+{"SCNx16", {"", ""} },
+{"SCNx32", {"", ""} },
+{"SCNx64", {"", ""} },
+
+{"SCNdPTR", {"", ""} },
+{"SCNiPTR", {"", ""} },
+{"SCNoPTR", {"", ""} },
+{"SCNuPTR", {"", ""} },
+{"SCNxPTR", {"", ""} }
+};
+
 /* Given non-NULL NAME, return the header name defining it within either
the standard library (with '<' and '>'), or NULL.
Only handles a subset of the most common names within the stdlibs.  */
@@ -196,6 +261,14 @@ get_stdlib_header_for_name (const char *name, enum stdlib 
lib)
   if (strcmp (name, c99_cxx11_hints[i].name) == 0)
return c99_cxx11_hints[i].header[lib];
 
+  const size_t num_c99_cxx11_macro_hints
+= sizeof (c99_cxx11_macro_hints) / sizeof (c99_cxx11_macro_hints[0]);
+  if ((lib == STDLIB_C && flag_isoc99)
+  || (lib == STDLIB_CPLUSPLUS && cxx_dialect >= cxx11 ))
+for (size_t i = 0; i < num_c99_cxx11_macro_hints; i++)
+  if (strcmp (name, c99_cxx11_macro_hints[i].name) == 0)
+   return c99_cxx11_macro_hints[i].header[lib];
+
   return NULL;
 }
 
@@ -217,6 +290,21 @@ get_cp_stdlib_header_for_name (const char *name)
   return get_stdlib_header_for_name (name, STDLIB_CPLUSPLUS);
 }
 
+/* Given non-NULL NAME, return the header name defin