[PATCH] MIPS: R6: load/store can process unaligned address
MIPS release 6 requires the lw/ld/sw/sd can work with
unaligned address, while it can be implemented by
full hardware or trap
Since it is may be fully done by hardware, we add an
option -m(no-)unaligned-access, the kernel may need it.
gcc/ChangeLog:
* config/mips/mips.h (ISA_HAS_UNALIGNED_ACCESS):
(STRICT_ALIGNMENT): R6 can unaligned access.
* config/mips/mips.md (movmisalign): Likewise.
* config/mips/mips.opt: add -m(no-)unaligned-access
* doc/invoke.texi: Likewise.
gcc/testsuite/ChangeLog:
* gcc.target/mips/mips.exp: add unaligned-access
* gcc.target/mips/unaligned-2.c: New test.
* gcc.target/mips/unaligned-3.c: New test.
---
gcc/config/mips/mips.h | 6 ++-
gcc/config/mips/mips.md | 10
gcc/config/mips/mips.opt| 4 ++
gcc/doc/invoke.texi | 10
gcc/testsuite/gcc.target/mips/mips.exp | 1 +
gcc/testsuite/gcc.target/mips/unaligned-2.c | 53 +
gcc/testsuite/gcc.target/mips/unaligned-3.c | 53 +
7 files changed, 136 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/mips/unaligned-2.c
create mode 100644 gcc/testsuite/gcc.target/mips/unaligned-3.c
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index b4a60a55d80..38c39f79ee2 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -226,6 +226,10 @@ struct mips_cpu_info {
&& (mips_isa_rev >= 6 \
|| ISA_HAS_MSA))
+/* ISA load/store instructions can handle unaligned address */
+#define ISA_HAS_UNALIGNED_ACCESS (TARGET_UNALIGNED_ACCESS \
+&& (mips_isa_rev >= 6))
+
/* The ISA compression flags that are currently in effect. */
#define TARGET_COMPRESSION (target_flags & (MASK_MIPS16 | MASK_MICROMIPS))
@@ -1665,7 +1669,7 @@ FP_ASM_SPEC "\
(ISA_HAS_MSA ? BITS_PER_MSA_REG : LONG_DOUBLE_TYPE_SIZE)
/* All accesses must be aligned. */
-#define STRICT_ALIGNMENT 1
+#define STRICT_ALIGNMENT (!ISA_HAS_UNALIGNED_ACCESS)
/* Define this if you wish to imitate the way many other C compilers
handle alignment of bitfields and the structures that contain
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index eef3cfd50a8..40e29c60432 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -4459,6 +4459,16 @@ (define_insn "mov_r"
[(set_attr "move_type" "store")
(set_attr "mode" "")])
+;; Unaligned direct access
+(define_expand "movmisalign"
+ [(set (match_operand:JOIN_MODE 0)
+ (match_operand:JOIN_MODE 1))]
+ "ISA_HAS_UNALIGNED_ACCESS"
+{
+ if (mips_legitimize_move (mode, operands[0], operands[1]))
+DONE;
+})
+
;; An instruction to calculate the high part of a 64-bit SYMBOL_ABSOLUTE.
;; The required value is:
;;
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index 6af8037e9bd..ebb4c616401 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -404,6 +404,10 @@ mtune=
Target RejectNegative Joined Var(mips_tune_option) ToLower
Enum(mips_arch_opt_value)
-mtune=PROCESSOR Optimize the output for PROCESSOR.
+munaligned-access
+Target Var(TARGET_UNALIGNED_ACCESS) Init(1)
+Generate code with unaligned load store, valid for MIPS R6.
+
muninit-const-in-rodata
Target Var(TARGET_UNINIT_CONST_IN_RODATA)
Put uninitialized constants in ROM (needs -membedded-data).
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 546e95453c1..27730d1a0de 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -1059,6 +1059,7 @@ Objective-C and Objective-C++ Dialects}.
-mcheck-zero-division -mno-check-zero-division @gol
-mdivide-traps -mdivide-breaks @gol
-mload-store-pairs -mno-load-store-pairs @gol
+-munaligned-access -mno-unaligned-access @gol
-mmemcpy -mno-memcpy -mlong-calls -mno-long-calls @gol
-mmad -mno-mad -mimadd -mno-imadd -mfused-madd -mno-fused-madd -nocpp
@gol
-mfix-24k -mno-fix-24k @gol
@@ -24967,6 +24968,15 @@ instructions to enable load/store bonding. This
option is enabled by
default but only takes effect when the selected architecture is known
to support bonding.
+@item -munaligned-access
+@itemx -mno-unaligned-access
+@opindex munaligned-access
+@opindex mno-unaligned-access
+Enable (disable) direct unaligned access for MIPS Release 6.
+MIPSr6 requires load/store unaligned-access support,
+by hardware or trap
+So @option{-mno-unaligned-access} may be needed by kernel.
+
@item -mmemcpy
@itemx -mno-memcpy
@opindex mmemcpy
diff --git a/gcc/testsuite/gcc.target/mips/mips.exp
b/gcc/testsuite/gcc.target/mips/mips.exp
index 01292316944..cb1ee7d71b5 100644
--- a/gcc/testsuite/gcc.target/mips/mips.exp
+++ b/gcc/testsuite/gcc.target/mips/mips.exp
@@ -264,6 +264,7 @@ set mips_option_groups {
frame-header "-mframe-header-opt|-mno-frame-header-opt"
stack-protector
Re: V3 [PATCH 3/5] Support the PGO build for binutils+gdb
On 19 Dec 2020 10:10, H.J. Lu via Gdb-patches wrote: > --- a/Makefile.in > +++ b/Makefile.in > > +PGO_BUILD_TRAINING_FLAGS_TO_PASS = \ > + PGO_BUILD_TRAINING=yes \ > + CFLAGS_FOR_TARGET="$(PGO_BUILD_TRAINING_CFLAGS)" \ > + CXXFLAGS_FOR_TARGET="$(PGO_BUILD_TRAINING_CXXFLAGS)" > + > +# Ignore "make check" errors in PGO training runs. > +PGO_BUILD_TRAINING_MFLAGS = -i these lines are in Makefile.in but not Makefile.tpl. so regenerating the file causes them to be removed. can you take a look please ? $ autogen --version autogen (GNU AutoGen) 5.18.16 $ autogen Makefile.def $ git diff diff --git a/Makefile.in b/Makefile.in index 0a64fc10e5b0..63565ad525b9 100644 --- a/Makefile.in +++ b/Makefile.in @@ -437,13 +437,9 @@ PGO_BUILD_TRAINING_CFLAGS:= \ PGO_BUILD_TRAINING_CXXFLAGS:= \ $(filter-out -specs=%,$(PGO_BUILD_TRAINING_CXXFLAGS)) PGO_BUILD_TRAINING_FLAGS_TO_PASS = \ - PGO_BUILD_TRAINING=yes \ CFLAGS_FOR_TARGET="$(PGO_BUILD_TRAINING_CFLAGS)" \ CXXFLAGS_FOR_TARGET="$(PGO_BUILD_TRAINING_CXXFLAGS)" -# Ignore "make check" errors in PGO training runs. -PGO_BUILD_TRAINING_MFLAGS = -i - # Additional PGO and LTO compiler options to use profiling data for the # PGO build. PGO_BUILD_USE_FLAGS_TO_PASS = \ @@ -1054,7 +1050,6 @@ all: $(PGO_BUILD_GEN_FLAGS_TO_PASS) all-host all-target \ @if pgo-build && $(MAKE) $(RECURSE_FLAGS_TO_PASS) \ - $(PGO_BUILD_TRAINING_MFLAGS) \ $(PGO_BUILD_TRAINING_FLAGS_TO_PASS) \ $(PGO_BUILD_TRAINING) \ && $(MAKE) $(RECURSE_FLAGS_TO_PASS) clean \ -mike
Re: [PATCH] coroutines : Remove throwing_cleanup marks from the ramp [PR95822].
On 2/26/21 4:36 PM, Iain Sandoe wrote:
Jason Merrill wrote:
On 2/24/21 3:06 PM, Iain Sandoe wrote:
The FE contains a mechanism for cleaning up return expressions if a
function throws during the execution of cleanups prior to the return.
If the original function has a return value with a non-trivial DTOR
and the body contains a var with a DTOR that might throw, the function
decl is marked "throwing_cleanup".
However, we do not [in the coroutine ramp function, which is
synthesised], use any body var types with DTORs that might throw.
The original body [which will then contain the type with the throwing
DTOR] is transformed into the actor function which only contains void
returns, and is also wrapped in a try-catch block.
So (a) the 'throwing_cleanup' is no longer correct for the ramp and
(b) we do not need to transfer it to the actor which only contains
void returns.
this is an ICE-on-valid,
tested on x86_64-darwin, x86_64-linux-gnu,
OK for master / 10.x ?
OK, but I wonder if there are other things that should also be reset.
That would be believable but, absent problem reports, have you any advice
on how/what to audit?
Looking at language_function, I guess it would make sense to clear
returns_abnormally and infinite_loop{,s}, maybe bindings. Since you're
completely replacing the function body, almost none of the information
we previously recorded about it in cfun is accurate. But it may be
harmless.
Jason
[pushed] c++: Allow GNU attributes before lambda -> [PR90333]
In my 9.3/10 patch for 90333 I allowed attributes between [] and (), and
after the trailing return type, but not in the place that GCC 8 expected
them, and we've gotten several bug reports about that. So let's allow them
there, as well.
Tested x86_64-pc-linux-gnu, applying to trunk.
gcc/cp/ChangeLog:
PR c++/90333
* parser.c (cp_parser_lambda_declarator_opt): Accept GNU attributes
between () and ->.
gcc/testsuite/ChangeLog:
PR c++/90333
* g++.dg/ext/attr-lambda3.C: New test.
---
gcc/cp/parser.c | 11 +--
gcc/testsuite/g++.dg/ext/attr-lambda3.C | 4
2 files changed, 13 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ext/attr-lambda3.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index c52f90c2715..bb1499ab741 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -11391,7 +11391,12 @@ cp_parser_lambda_declarator_opt (cp_parser* parser,
tree lambda_expr)
omitted_parms_loc = UNKNOWN_LOCATION;
}
- std_attrs = cp_parser_std_attribute_spec_seq (parser);
+ /* GCC 8 accepted attributes here, and this is the place for standard C++11
+ attributes that appertain to the function type. */
+ if (cp_next_tokens_can_be_gnu_attribute_p (parser))
+gnu_attrs = cp_parser_gnu_attributes_opt (parser);
+ else
+std_attrs = cp_parser_std_attribute_spec_seq (parser);
/* Parse optional trailing return type. */
if (cp_lexer_next_token_is (parser->lexer, CPP_DEREF))
@@ -11405,8 +11410,10 @@ cp_parser_lambda_declarator_opt (cp_parser* parser,
tree lambda_expr)
return_type = cp_parser_trailing_type_id (parser);
}
+ /* Also allow GNU attributes at the very end of the declaration, the usual
+ place for GNU attributes. */
if (cp_next_tokens_can_be_gnu_attribute_p (parser))
-gnu_attrs = cp_parser_gnu_attributes_opt (parser);
+gnu_attrs = chainon (gnu_attrs, cp_parser_gnu_attributes_opt (parser));
if (has_param_list)
{
diff --git a/gcc/testsuite/g++.dg/ext/attr-lambda3.C
b/gcc/testsuite/g++.dg/ext/attr-lambda3.C
new file mode 100644
index 000..f9c3ec11fe9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-lambda3.C
@@ -0,0 +1,4 @@
+// PR c++/90333
+// { dg-do compile { target c++11 } }
+
+auto x = []() __attribute__((always_inline)) -> int { return 0; }
base-commit: 652623f7c68594b1825a333bf8e83e87d1c3f523
--
2.27.0
[pushed] Darwin : Update a disgnostic message [NFC].
Hi
The avoids a contraction and a format diagnostic warning.
(it’s debatable whether this is really a useful user-facing error -
but leaving that to another day).
tested on x86_64-darwin,
pushed to master,
thanks
Iain
gcc/ChangeLog:
* config/host-darwin.c (darwin_gt_pch_use_address): Modify
diagnostic message to avoid use of a contraction and format
warning.
---
gcc/config/host-darwin.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gcc/config/host-darwin.c b/gcc/config/host-darwin.c
index 1816c619511..b101fca5c96 100644
--- a/gcc/config/host-darwin.c
+++ b/gcc/config/host-darwin.c
@@ -61,7 +61,8 @@ darwin_gt_pch_use_address (void *addr, size_t sz, int fd,
size_t off)
sz = (sz + pagesize - 1) / pagesize * pagesize;
if (munmap (pch_address_space + sz, sizeof (pch_address_space) - sz) != 0)
-fatal_error (input_location, "couldn%'t unmap pch_address_space: %m");
+fatal_error (input_location,
+"could not unmap % %m");
if (ret)
{
--
2.24.1
Re: [PATCH] libgomp, testsuite : Require alias support for PR96390 testcase.
On Sat, Feb 27, 2021 at 02:59:03PM +, Iain Sandoe wrote:
> This test fails everywhere on Darwin, which does not have support for symbol
> aliases. Add a dg-require-alias to UNSUPPORT it.
>
> Ordinarily, I would have considered adding the dg-require as ‘obvious’ but I
> see
> that you already handled one case specifically (so wonder if there is some
> reason
> not to use this).
The testcase needs alias support both on the host (for which
dg-require-alias is the right thing) and on the offloading target side
(nvptx in this case).
> OK for master?
Yes.
> libgomp/ChangeLog:
>
> * testsuite/libgomp.c-c++-common/pr96390.c: Require alias
> support from the target.
> ---
> libgomp/testsuite/libgomp.c-c++-common/pr96390.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/libgomp/testsuite/libgomp.c-c++-common/pr96390.c
> b/libgomp/testsuite/libgomp.c-c++-common/pr96390.c
> index 692bd730069..4fe09cebb5d 100644
> --- a/libgomp/testsuite/libgomp.c-c++-common/pr96390.c
> +++ b/libgomp/testsuite/libgomp.c-c++-common/pr96390.c
> @@ -1,4 +1,5 @@
> /* { dg-additional-options "-O0 -fdump-tree-omplower" } */
> +/* { dg-require-alias "" } */
> /* { dg-xfail-if "PR 97102/PR 97106 - .alias not (yet) supported for nvptx"
> { offload_target_nvptx } } */
>
> #ifdef __cplusplus
> --
> 2.24.1
>
Jakub
[PATCH] libgomp, testsuite : Require alias support for PR96390 testcase.
Hi,
This test fails everywhere on Darwin, which does not have support for symbol
aliases. Add a dg-require-alias to UNSUPPORT it.
Ordinarily, I would have considered adding the dg-require as ‘obvious’ but I see
that you already handled one case specifically (so wonder if there is some
reason
not to use this).
Note that this will be a compile-time fail on targets without alias support, so
the
alternative will be to skip it on Darwin (and other such targets).
tested on x86_64-darwin,
OK for master?
thanks
Iain
libgomp/ChangeLog:
* testsuite/libgomp.c-c++-common/pr96390.c: Require alias
support from the target.
---
libgomp/testsuite/libgomp.c-c++-common/pr96390.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libgomp/testsuite/libgomp.c-c++-common/pr96390.c
b/libgomp/testsuite/libgomp.c-c++-common/pr96390.c
index 692bd730069..4fe09cebb5d 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/pr96390.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/pr96390.c
@@ -1,4 +1,5 @@
/* { dg-additional-options "-O0 -fdump-tree-omplower" } */
+/* { dg-require-alias "" } */
/* { dg-xfail-if "PR 97102/PR 97106 - .alias not (yet) supported for nvptx" {
offload_target_nvptx } } */
#ifdef __cplusplus
--
2.24.1
Re: [PATCH/RFA] libstdc++: provide conversion from day, month to unsigned long, PR99301
On 27/02/21 12:42 +0100, Hans-Peter Nilsson via Libstdc++ wrote:
Since 97d6161f6a7fa712 / r11-7370 "libstdc++: More efficient
days from date" I see an additional 81 testsuite-errors for
cris-elf, with this in g++.log for one randomly picked
regressing test:
FAIL: g++.dg/cpp1y/pr57640.C -std=c++2a (test for excess errors)
Excess errors:
/x/gccobj/cris-elf/libstdc++-v3/include/chrono:2483:25: error: invalid
'static_cast' from type 'const std::chrono::month' to type 'uint32_t' {aka
'long unsigned int'}
/x/gccobj/cris-elf/libstdc++-v3/include/chrono:2484:25: error: invalid
'static_cast' from type 'const std::chrono::day' to type 'uint32_t' {aka 'long
unsigned int'}
/x/gccobj/cris-elf/libstdc++-v3/include/chrono:2496:69: error: no matching function for call to
'std::chrono::duration
>::duration()'
The commit shows conversions to uint32_t, which for
e.g. x86_64-linux is "unsigned int", and there are explicit
conversions to unsigned int for month and day (see patch
context).
But, "newlib ILP32 targets" have an uint32_t that is
effectively typedef'd "long unsigned int" (see
newlib-stdint.h UINT32_TYPE).
Better provide an unsigned long conversion aside the
unsigned ones.
No, the allowed conversions are specific by the standard. The right
fix is to convert to unsigned explicitly.
I've pushed the attached patch after testing on x86_64-linux. This
should work for newlib ILP32 targets too.
commit 699672d4dccfb5579dbe48977bda86f6836225a0
Author: Jonathan Wakely
Date: Sat Feb 27 12:50:53 2021
libstdc++: Fix conversions from date types to integers [PR 99301]
The conversions to integer types are explicit, so need to use the
correct type. Converting to uint32_t only works if that is the same type
as unsigned.
libstdc++-v3/ChangeLog:
PR libstdc++/99301
* include/std/chrono (year_month_day::_M_days_since_epoch()):
Convert chrono::month and chrono::day to unsigned before
converting to uint32_t.
diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index fcdaee7328e..11729aae708 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -2496,8 +2496,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
auto constexpr __r2_e3 = static_cast(536895458);
const auto __y1 = static_cast(static_cast(_M_y)) - __z2;
- const auto __m1 = static_cast(_M_m);
- const auto __d1 = static_cast(_M_d);
+ const auto __m1 = static_cast(static_cast(_M_m));
+ const auto __d1 = static_cast(static_cast(_M_d));
const auto __j = static_cast(__m1 < 3);
const auto __y0 = __y1 - __j;
New German PO file for 'gcc' (version 11.1-b20210207)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the German team of translators. The file is available at: https://translationproject.org/latest/gcc/de.po (This file, 'gcc-11.1-b20210207.de.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: https://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: https://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
[PATCH/RFA] libstdc++: provide conversion from day, month to unsigned long, PR99301
Since 97d6161f6a7fa712 / r11-7370 "libstdc++: More efficient
days from date" I see an additional 81 testsuite-errors for
cris-elf, with this in g++.log for one randomly picked
regressing test:
FAIL: g++.dg/cpp1y/pr57640.C -std=c++2a (test for excess errors)
Excess errors:
/x/gccobj/cris-elf/libstdc++-v3/include/chrono:2483:25: error: invalid
'static_cast' from type 'const std::chrono::month' to type 'uint32_t' {aka
'long unsigned int'}
/x/gccobj/cris-elf/libstdc++-v3/include/chrono:2484:25: error: invalid
'static_cast' from type 'const std::chrono::day' to type 'uint32_t' {aka 'long
unsigned int'}
/x/gccobj/cris-elf/libstdc++-v3/include/chrono:2496:69: error: no matching
function for call to 'std::chrono::duration
>::duration()'
The commit shows conversions to uint32_t, which for
e.g. x86_64-linux is "unsigned int", and there are explicit
conversions to unsigned int for month and day (see patch
context).
But, "newlib ILP32 targets" have an uint32_t that is
effectively typedef'd "long unsigned int" (see
newlib-stdint.h UINT32_TYPE).
Better provide an unsigned long conversion aside the
unsigned ones.
Tested cris-elf. Pending x86_64-linux regtest results, ok?
libstdc++-v3:
PR libstdc++/99301
* include/std/chrono (day, month): Provide
conversion to unsigned long.
---
libstdc++-v3/include/std/chrono | 8
1 file changed, 8 insertions(+)
diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index fcdaee7328ed..33fb0860b41a 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -1334,6 +1334,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
operator unsigned() const noexcept
{ return _M_d; }
+ constexpr explicit
+ operator unsigned long() const noexcept
+ { return _M_d; }
+
constexpr bool
ok() const noexcept
{ return 1 <= _M_d && _M_d <= 31; }
@@ -1443,6 +1447,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
operator unsigned() const noexcept
{ return _M_m; }
+ explicit constexpr
+ operator unsigned long() const noexcept
+ { return _M_m; }
+
constexpr bool
ok() const noexcept
{ return 1 <= _M_m && _M_m <= 12; }
--
2.11.0
[PATCH] dwarf2out: Fix -gsplit-dwarf on riscv or other non-.uleb128 targets [PR99090]
Hi!
As mentioned in the PR, riscv* only supports .uleb128 with constant
arguments, doesn't support difference of two labels because of aggressive
linker relaxations. But I bet various other targets, especially those not
using GNU assembler, might suffer from the same problem.
As the FIXME comment in output_loc_list indicates, we ICE on
-gsplit-dwarf on those targets whenever we need .debug_loclists, because
we only emit DW_LLE_startx_length which requires working .uleb128 delta
of 2 code section labels. We can't use DW_LLE_base_addressx
once followed by DW_LLE_offset_pair either because the latter suffers
from the same issue - need .uleb128 difference of code section labels
(and in that case not just for the second operand but also for the first).
So, this patch implements what the comment said and emits DW_LLE_startx_endx
instead, which wastes more space in .debug_addr, but will work.
Bootstrapped/regtested on x86_64-linux and i686-linux and as written in the
PR, Jim has tested it on riscv*linux. Ok for trunk?
BTW, for HAVE_AS_LEB128 -gdwarf-5 -gsplit-dwarf, maybe we should consider
instead of always emitting DW_LLE_startx_length do all the optimizations
that we do for HAVE_AS_LEB128 -gdwarf-5, or at least a subset of them.
For !have_multiple_function_sections, we in that case emit just
DW_LLE_offset_pair (that can certainly be a win for small TUs, we wouldn't
need any .debug_addr entry in that case; on the other side, just using
DW_LLE_offset_pair can be harmful for very large TUs especially if the
loclist has many entries, emitting in that case a single DW_LLE_base_address
or for -gsplit-dwarf DW_LLE_base_addressx followed by DW_LLE_offset_pair
might be much smaller), and for have_multiple_function_sections figuring
out if DW_LLE_base_address followed by DW_LLE_offset_pair entries
or DW_LLE_start_length is bettter. So perhaps a middle-ground for
-gsplit-dwarf would be to always do the have_multiple_function_sections
behavior, i.e. DW_LLE_base_addressx followed by DW_LLE_offset_pair vs.
DW_LLE_startx_length decisions based on the ranges and their counts.
And perhaps dwz could optimize afterwards, on linked binaries or shared
libraries it knows all the offsets and could figure out optimal DW_LLE_*.
2021-02-26 Jakub Jelinek
PR debug/99090
* dwarf2out.c (dw_loc_list_struct): Add end_entry member.
(new_loc_list): Clear end_entry.
(output_loc_list): Only use DW_LLE_startx_length for -gsplit-dwarf
if HAVE_AS_LEB128, otherwise use DW_LLE_startx_endx. Fix comment
typo.
(index_location_lists): For dwarf_version >= 5 without HAVE_AS_LEB128,
initialize also end_entry.
--- gcc/dwarf2out.c.jj 2021-02-10 07:54:25.210622383 +0100
+++ gcc/dwarf2out.c 2021-02-26 19:11:19.555409473 +0100
@@ -1317,6 +1317,7 @@ typedef struct GTY(()) dw_loc_list_struc
const char *begin; /* Label and addr_entry for start of range */
addr_table_entry *begin_entry;
const char *end; /* Label for end of range */
+ addr_table_entry *end_entry;
char *ll_symbol; /* Label for beginning of location list.
Only on head of list. */
char *vl_symbol; /* Label for beginning of view list. Ditto. */
@@ -10101,6 +10102,7 @@ new_loc_list (dw_loc_descr_ref expr, con
retlist->begin = begin;
retlist->begin_entry = NULL;
retlist->end = end;
+ retlist->end_entry = NULL;
retlist->expr = expr;
retlist->section = section;
retlist->vbegin = vbegin;
@@ -10327,10 +10329,10 @@ output_loc_list (dw_loc_list_ref list_he
if (dwarf_version >= 5)
{
- if (dwarf_split_debug_info)
+ if (dwarf_split_debug_info && HAVE_AS_LEB128)
{
dwarf2out_maybe_output_loclist_view_pair (curr);
- /* For -gsplit-dwarf, emit DW_LLE_starx_length, which has
+ /* For -gsplit-dwarf, emit DW_LLE_startx_length, which has
uleb128 index into .debug_addr and uleb128 length. */
dw2_asm_output_data (1, DW_LLE_startx_length,
"DW_LLE_startx_length (%s)",
@@ -10338,13 +10340,26 @@ output_loc_list (dw_loc_list_ref list_he
dw2_asm_output_data_uleb128 (curr->begin_entry->index,
"Location list range start index "
"(%s)", curr->begin);
- /* FIXME: This will ICE ifndef HAVE_AS_LEB128.
-For that case we probably need to emit DW_LLE_startx_endx,
-but we'd need 2 .debug_addr entries rather than just one. */
dw2_asm_output_delta_uleb128 (curr->end, curr->begin,
"Location list length (%s)",
list_head->ll_symbol);
}
+ else if (dwarf_split_debug_info)
+ {
+ dwarf2out_maybe_output_loclist_view_pair (curr);
+ /* For -gsplit-dwarf without
PR analyzer/94362 Partial Fix
Hi,
Please find a patch to fix part of the bug PR analyzer/94362. This bug is a
false positive for a null dereference found when compiling openssl. The cause
is the constraint_manager not knowing that i >= 0 within the for block:
for ( ; i-- > 0; )
The bug can be further reduced to the constraint manager not knowing that i >= 0
within the if block:
if (i-- > 0)
which is not replicated for other operators, such as prefix decrement. The
cause is that the constraint is applied to the initial_svalue of i, while it
is a binop_svalue of i that enters the block (with op PLUS and arg1 -1). The
constraint_manager does not have any constraints for this svalue and has no
handler. A handler has been added that essentially recurs on the remaining arg
if the other arg and other side of the condition are both constants and the op
is PLUS_EXPR or MINUS_EXPR.
This in essence fixed the problem, except an off by one error had been hiding
in range::eval_condition. This error is hard to notice, because, for example,
the code
if(idx > 0)
__analyzer_eval(idx >= 1);
will compile as (check -fdump-ipa-analyzer to see)
void test (int idx)
{
_Bool _1;
int _2;
:
if (idx_4(D) > 0)
goto ; [INV]
else
goto ; [INV]
:
_1 = idx_4(D) > 0;
_2 = (int) _1;
__analyzer_eval (_2);
:
return;
}
and will print "TRUE" to the screen, but as you can see, it is for the wrong
reason, because we are not really testing the condition we wanted to test.
You can force the failure (i.e. "UNKNOWN") for yourself with the following:
void test(int idx)
{
int check = 1;
if(idx > 0)
__analyzer_eval(idx >= check);
}
which the compiler will not "fix" on us. An examination of range::eval_condition
should convince you that there is an off by one error. Incidentally, I might
recommend doing away with "open intervals of integers" entirely.
When running the initial bug (the for loop), you will find that the analyzer
prints "UNKNOWN" twice for the postfix operator, and "TRUE" "UNKNOWN" for other
operators. This patch reduces postfix to the same state as the other operators.
The second "UNKNOWN" seems to be due to a second "iterated" pass through the
loop with a widening_svalue. A full fix of the bug will need a handler for the
widening svalue, and much more critically, a correct merge of the constraints
at the loop entrance. That, unfortunately, looks to be a hard problem.
This patch fixes a few xfails as noted in the commit message. These were tests
that were evidently devised to test whether the analyzer would understand
arithmetic being done on constrained values. Addition and subtraction is now
working as expected, a handler for multiplication and division can be added.
As was noted in those test files, consideration at some point should be given to
overflow.
Thank you,
Brian
Sent with ProtonMail Secure Email.commit d4052e8c273ca267f6dcf782084d60acfc50a609
Author: Brian Sobulefsky
Date: Sat Feb 27 00:36:40 2021 -0800
Changes to support eventual solution to bug PR analyzer/94362. This bug
originated with a false positive null dereference during compilation of
openssl. The bug is in effect caused by an error in constraint handling,
specifically that within the for block:
for ( ; i-- > 0; )
{
}
the constraint_manager should know i >= 0 but does not. A reduced form of
this bug was found where the constraint manager did not know within the if
block:
if (i-- > 0)
{
}
that i >= 0. This latter error was only present for the postfix
operators, and not for other forms, like --i > 0. It was due to the
constraint being set for the initial_svalue associated with i, but a
binop_svalue being what entered the if block for which no constraint
rules existed.
By adding handling logic for a binop_svalue that adds or
subtracts a constant, this problem was solved. This logic was added to
a new method, constraint_manager::maybe_fold_condition, with the
intent of eventually adding more cases there (unary_svalue and
widening_svalue for example). Additionally, an off by one error was
found in range::eval_condition that needed to be corrected to get
the expected result. Correction of this error was done in that
subroutine, resulting in no more calls to below_lower_bound and
above_upper_bound. As such, these functions were commented out and may
be removed if not needed for anything else.
This change does not entirely fix the initial bug pr94362, but it
reduces the postfix operator to the same state as other operators. In the
case of the for loop, there appears to be an "initial pass" through the
loop, which the analyzer will now understand for postfix, and then an
"iterated pass" with a widening_svalue that the analyzer does not
understand for any condition found. This seems to be due to the merging
of constraints and is under investigation.
[committed] gcse, ipa-devirt: Use %wd/%wu instead of HOST_WIDE_INT_PRINT* in diagnostics [PR99288]
Hi!
HOST_WIDE_INT_PRINT* in the string literals of warning/error/inform etc.
make those messages non-translatable, and we have a perfectly fine
alternative when not using system *printf - %w{d,u}.
Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk
as obvious.
2021-02-27 Jakub Jelinek
PR other/99288
* gcse.c (gcse_or_cprop_is_too_expensive): Use %wu instead of
HOST_WIDE_INT_PRINT_UNSIGNED in warning format string.
* ipa-devirt.c (ipa_odr_read_section): Use %wd instead of
HOST_WIDE_INT_PRINT_DEC in inform format string. Fix comment
typos.
--- gcc/gcse.c.jj 2021-01-29 15:18:07.997293117 +0100
+++ gcc/gcse.c 2021-02-26 17:03:23.174388798 +0100
@@ -4011,8 +4011,7 @@ gcse_or_cprop_is_too_expensive (const ch
{
warning (OPT_Wdisabled_optimization,
"%s: %d basic blocks and %d registers; "
- "increase %<--param max-gcse-memory%> above "
- HOST_WIDE_INT_PRINT_UNSIGNED,
+ "increase %<--param max-gcse-memory%> above %wu",
pass, n_basic_blocks_for_fn (cfun), max_reg_num (),
memory_request / 1024);
--- gcc/ipa-devirt.c.jj 2021-01-04 10:25:38.196241037 +0100
+++ gcc/ipa-devirt.c2021-02-26 17:05:11.904182848 +0100
@@ -4261,13 +4261,12 @@ ipa_odr_read_section (struct lto_file_de
" in another translation unit",
this_enum.vals[j].name, warn_name);
/* FIXME: In case there is easy way to print wide_ints,
-perhaps we could do it here instead of overlfow checpl. */
+perhaps we could do it here instead of overflow check. */
else if (wi::fits_shwi_p (this_enum.vals[j].val)
&& wi::fits_shwi_p (warn_value))
inform (this_enum.vals[j].locus,
- "name %qs is defined to " HOST_WIDE_INT_PRINT_DEC
- " while another translation unit defines "
- "it as " HOST_WIDE_INT_PRINT_DEC,
+ "name %qs is defined to %wd while another "
+ "translation unit defines it as %wd",
warn_name, this_enum.vals[j].val.to_shwi (),
warn_value.to_shwi ());
else
Jakub
