> I doubt this is correct either. What does the ILP32E abi say? Does it
> say `long long` is 4 byte aligned?

I believe so:

"The ILP32E calling convention is not compatible with ISAs that have
 registers that require load and store alignments of more than 32
 bits. In particular, this calling convention must not be used with the D
 ISA extension."

> RV32E is not a ratified base ISA and so we cannot guarantee the
> stability of ILP32E, in contrast with the rest of this document.

Yeah, which means we haven't any certainty about any of this. Perhaps
someone with more knowledge of the Risc-V process can provide a hint as
to whether this extension will ever be adopted formally.

> So there is no ABI document it seems.

Nothing certain, alas.

> > +      if (align > STACK_BOUNDARY / BITS_PER_UNIT)
> > +       align = STACK_BOUNDARY / BITS_PER_UNIT;
> 
> This is definitely not correct and will break other targets where the
> stack is realigned for that purpose.

This code isn't actually manipulating any visible program state, it's
checking pointer alignment before de-referencing to ensure it
follows the ISA requirements.

> The stack should be realigned if there is a mode that requires a
> higher alignment.

Which conflicts with the ABI specification that in ILP32E mode the stack
need only be 32-bit aligned.

I believe this condition should never be true as the stack should always
be aligned to at least the most strict alignment requirement in the ABI.
The only alignment adjustment that selecting ILP32E does in gcc is relax
the stack requirements without adjusting anything else, which I suspect
is a bug, both in the ISA document which fails to make that clear and in
GCC which doesn't adjust the alignment requirements for wider datatypes.

I suspect the correct answer to this problem is "don't use ILP32E ABI
yet". Or, in this case, at least don't use -fsanitize=undefined and
expect alignment checks to work correctly.

Thanks so much for taking a look at this; I think I may work-around this
by adjusting the pointer alignment UB sanitizer hook when running with
this ISA. I'll want to do that anyways; I'll need to make things work
with existing compilers.

-- 
-keith

Attachment: signature.asc
Description: PGP signature

Reply via email to