On 10/18/19 12:52 AM, Jakub Jelinek wrote:
On Thu, Oct 17, 2019 at 06:07:37PM -0600, Martin Sebor wrote:
On 10/17/19 1:00 AM, Jakub Jelinek wrote:
Hi!

The following bug has been introduced when cond_expr_object_size has been
added in 2007.  We want to treat a COND_EXPR like a PHI with 2 arguments,
and PHI is handled in a loop that breaks if the lhs value is unknown, and
then does the if (TREE_CODE (arg) == SSA_NAME) merge_object_sizes else
expr_object_size which is used even in places that handle just a single
operand (with the lhs value initialized to the opposite value of unknown
first).  At least expr_object_size asserts that the lhs value is not
unknown at the start.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

I'm not sure the other change (r277134) it the right way to fix
the problem with the missing initialization.  It was introduced
with the merger of the sprintf pass.  The latter still calls
init_object_sizes in get_destination_size.  I think the call
should be moved from there into the new combined sprintf/strlen
printf_strlen_execute function that also calls fini_object_sizes,
and the one from determine_min_objsize should be removed.  I can
take care of it unless you think it needs to stay the way it is
now for some reason.

Why?  As I said, init_object_sizes is designed to be called multiple times
and is cheap (load + comparison + early return) if it has been already
called, so is meant to be called only when needed, rather than at the
beginning of a pass just in case something appears.  The objsz pass does the
same.  No need to allocate bitmaps/vectors if nothing will need them.

Why?  Because as the bug you just fixed illustrates it's obviously
error prone to initialize the pass on demand in a utility function.
There are two now, and the next time someone adds a new one they
could easily reintroduce the same problem.

Martin

Reply via email to