* [PR100843] store by mult pieces: punt on max_len < min_len
@ 2021-12-09 22:16 Alexandre Oliva
2021-12-10 22:33 ` Jeff Law
0 siblings, 1 reply; 4+ messages in thread
From: Alexandre Oliva @ 2021-12-09 22:16 UTC (permalink / raw)
To: gcc-patches; +Cc: cnsun
The testcase confuses the code that detects min and max len for the
memset, so max_len ends up less than min_len. That shouldn't be
possible, but the testcase requires us to handle this case.
The store-by-mult-pieces algorithm actually relies on min and max
lengths, so if we find them to be inconsistent, the best we can do is
punting.
Regstrapped on x86_64-linux-gnu. Ok to install?
for gcc/ChangeLog
PR middle-end/100843
* builtins.c (try_store_by_multiple_pieces): Fail if min_len
is greater than max_len.
for gcc/testsuite/ChangeLog
PR middle-end/100843
* gcc.dg/pr100843.c: New.
---
gcc/builtins.c | 3 ++-
gcc/testsuite/gcc.dg/pr100843.c | 8 ++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/pr100843.c
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 03829c03a5a11..304d87dafb750 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -3963,7 +3963,8 @@ try_store_by_multiple_pieces (rtx to, rtx len, unsigned int ctz_len,
else if (max_len == min_len)
blksize = max_len;
else
- gcc_unreachable ();
+ /* Huh, max_len < min_len? Punt. See pr100843.c. */
+ return false;
if (min_len >= blksize)
{
min_len -= blksize;
diff --git a/gcc/testsuite/gcc.dg/pr100843.c b/gcc/testsuite/gcc.dg/pr100843.c
new file mode 100644
index 0000000000000..695a2ec3f6818
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr100843.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -w" } */
+
+char c;
+void *memset();
+void test_integer_conversion_memset(void *d) {
+ memset(d, '\0', c);
+}
--
Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts. Ask me about <https://stallmansupport.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PR100843] store by mult pieces: punt on max_len < min_len
2021-12-09 22:16 [PR100843] store by mult pieces: punt on max_len < min_len Alexandre Oliva
@ 2021-12-10 22:33 ` Jeff Law
2021-12-11 5:18 ` Alexandre Oliva
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Law @ 2021-12-10 22:33 UTC (permalink / raw)
To: Alexandre Oliva, gcc-patches; +Cc: cnsun
On 12/9/2021 3:16 PM, Alexandre Oliva via Gcc-patches wrote:
> The testcase confuses the code that detects min and max len for the
> memset, so max_len ends up less than min_len. That shouldn't be
> possible, but the testcase requires us to handle this case.
>
> The store-by-mult-pieces algorithm actually relies on min and max
> lengths, so if we find them to be inconsistent, the best we can do is
> punting.
>
> Regstrapped on x86_64-linux-gnu. Ok to install?
>
>
> for gcc/ChangeLog
>
> PR middle-end/100843
> * builtins.c (try_store_by_multiple_pieces): Fail if min_len
> is greater than max_len.
>
> for gcc/testsuite/ChangeLog
>
> PR middle-end/100843
> * gcc.dg/pr100843.c: New.
The patch is clearly safe. My question is should we have caught this
earlier in the call chain? If so, I'd prefer to fix it there and leave
the gcc_unreachable in place to catch other bogus cases that may get
into try_store_by_multiple_pieces.
Jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PR100843] store by mult pieces: punt on max_len < min_len
2021-12-10 22:33 ` Jeff Law
@ 2021-12-11 5:18 ` Alexandre Oliva
2021-12-14 21:58 ` Jeff Law
0 siblings, 1 reply; 4+ messages in thread
From: Alexandre Oliva @ 2021-12-11 5:18 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches, cnsun
On Dec 10, 2021, Jeff Law <jeffreyalaw@gmail.com> wrote:
> The patch is clearly safe. My question is should we have caught this
> earlier in the call chain?
Callers will call try_store_by_multiple_pieces if set_storage_via_setmem
fails. setmem doesn't necessarily need min and max len to do its job,
so if we were to modify callers, it would be just guarding the calls of
try_store_by_multiple_pieces with max_len >= min_len: 3 callers in 2
files, which didn't seem appealing to me.
--
Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts. Ask me about <https://stallmansupport.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PR100843] store by mult pieces: punt on max_len < min_len
2021-12-11 5:18 ` Alexandre Oliva
@ 2021-12-14 21:58 ` Jeff Law
0 siblings, 0 replies; 4+ messages in thread
From: Jeff Law @ 2021-12-14 21:58 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: gcc-patches, cnsun
On 12/10/2021 10:18 PM, Alexandre Oliva wrote:
> On Dec 10, 2021, Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>> The patch is clearly safe. My question is should we have caught this
>> earlier in the call chain?
> Callers will call try_store_by_multiple_pieces if set_storage_via_setmem
> fails. setmem doesn't necessarily need min and max len to do its job,
> so if we were to modify callers, it would be just guarding the calls of
> try_store_by_multiple_pieces with max_len >= min_len: 3 callers in 2
> files, which didn't seem appealing to me.
Thanks for the additional info. OK for the trunk.
Jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-12-14 21:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-09 22:16 [PR100843] store by mult pieces: punt on max_len < min_len Alexandre Oliva
2021-12-10 22:33 ` Jeff Law
2021-12-11 5:18 ` Alexandre Oliva
2021-12-14 21:58 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).