Re: This was fun to track down

["David S. Miller" <davem@jenolan.rutgers.edu>]

   Date: Tue, 23 Sep 1997 10:49:27 +0200
   From: Torbjorn Granlund <tege@nada.kth.se>

   Under what circumstances does this happen?  (A test case would be
   nice.)

I first saw the bug in a cross compiler from sparc-linux-gnulibc1 to
sparc64-linux (my patches for sparc64-linux support are in the process
of being merged in still).  Here is a small test case:

struct node {
	struct node *next;
	struct node *sibling;
	int hash;
};

struct node *htable[59];

extern void *alloc(int size);

void divmodbug(int hash, struct node *sib)
{
	struct node *p;

	p = alloc(sizeof(*p));
	p->hash = hash;
	p->sibling = sib;
	p->next = htable[hash % 59];
	htable[hash % 59] = p;
}

(this is just a simplified version of tree_add_hash() in gcc/tree.c as
that was what was originally miscompiled for me)

When compiled with no optimizations on, the following occurs in the
second call to expand_divmod() which is for the statement:

	htable[hash % 59] = p;

In expand_divmod() firstly:

	op1_is_constant is set to 1, since op1 is the '59' CONST_INT
	op1_is_pow2 is set to 0 for obvious reasons

Target is NULL_RTX, optab1 and optab2 are determined, the mode of 59
is smaller than Pmode so compute_mode ends up being a larger mode.

Since compute_mode != mode, convert_modes() is called on both op0 and
op1.  Since for sparc64 target code expanding from a signed 32-bit
mode to a 64-bit unsigned mode (SImode to DImode in this case)
requires an actual instruction, the 59 is thrown into a register and
op1 becomes this reg.

Now op1_is_constant is inaccurate.  Which is why I recompute it right
there, along with op1_is_pow2.  I also note that none of the decisions
already made about the old value of op1_is_constant and op1_is_pow2
need to be undone, so this is why I determined this fix to be
correct.

We then hit the switch statement coming up, specifically at cases
TRUNC_MOD_EXPR & TRUNC_DIV_EXPR.  Here op1_is_constant is tested, and
so is unsignedp, if these are both true (and they were before the fix
for this case) the variable 'd' is set to INTVAL(op1).  What was
amusing in my case was that op1 ended up in a pseudo reg with number
'128' so that is what ended up in 'd'.  Lo' and behold this is an
exact power or 2 and thus the next conditional passes, generating bad
code.

Later,
David "Sparc" Miller
davem@caip.rutgers.edu