On 07/22/2018 03:03 AM, Bernd Edlinger wrote:
On 07/21/18 01:58, Martin Sebor wrote:
On 07/20/2018 03:11 PM, Bernd Edlinger wrote:
I think I have found a valid test case where the latest patch
does generate invalid code:
This is due to another bug in string_constant() that's latent
on trunk but that's exposed by the patch. Trunk only "works"
because of a bug/limitation in c_strlen() that the patch fixes
or removes.
I am not sure if that falls under the definition of "latent" bug.
A latent bug would be something unexpected in a completely different
area of the compiler that is triggered by an improved optimization
or a fixed bug elsewhere.
The underlying root cause is the handling of POINTER_PLUS
expressions in string_constant(). The original code (before
the handling of aggregates was added) just dealt with string
constants. The new code does much more but doesn't get it
quite right in these cases.
I think you should be much more careful with the expressions
you evaluate in string_constant. For instance when you look
at get_addr_base_and_unit_offset, you'll see it walks through
all kinds of type casts, VIEW_CONVERT_EXPR and MEM_REF with
nonzero displacement. When you see something like that do
not fold that on the assumption that the source code does not
have undefined behavior.
So I'd say you should add a check that there is no type
cast in the expression you parse. If that is the case,
your optimization will certainly be wrong.
There are no casts here. The pointer to array case is just
something I didn't think of, that's all. The bug is in not
rejecting those. As I explained, the original code handled
just string literals and constant character arrays. Current
trunk deals with all kinds of constants and doesn't get all
the constraints right.
Martin
