[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex
From: |
Danny Milosavljevic |
Subject: |
Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello' |
Date: |
Thu, 7 Jan 2021 23:52:41 +0100 |
Hi Janneke,
I just found the bug in tinycc that caused failed our ARM bootstrap to fail.
I use the following reproducer:
int main() {
double f = 1.0;
return 0;
}
and then invoke
tcc a.c
on ARM (32) using your patched tcc. (but it's also broken in the unpatched tcc)
(tcc cross compiler is not enough. tcc has to actually itself be an ARM
EABI executable)
I get a bus error here:
│ 0x24698 <init_putv+1688> vstr d0, [r0]
│
Debugging some more, I find:
tccgen.c:
/* store a value or an expression directly in global data or in local array */
static void init_putv(CType *type, Section *sec, unsigned long c)
{
[...]
size = type_size(type, &align);
section_reserve(sec, c + size); // c == 0, size == 8
ptr = sec->data + c; // sec->data == 0x24b01e, c == 0
switch(bt) {
/* XXX: when cross-compiling we assume that each type has the
same representation on host and target, which is likely to
be wrong in the case of long double */
case VT_BOOL:
vtop->c.i = vtop->c.i != 0;
case VT_BYTE:
*(char *)ptr = vtop->c.i;
break;
case VT_SHORT:
*(short *)ptr = vtop->c.i;
break;
case VT_FLOAT:
*(float*)ptr = vtop->c.f;
break;
case VT_DOUBLE:
*(double *)ptr = vtop->c.d;
break;
[... and so on]
tccelf.c:
/* reserve at least 'size' bytes from section start */
ST_FUNC void section_reserve(Section *sec, unsigned long size)
{
if (size > sec->data_allocated) // both 8
section_realloc(sec, size);
if (size > sec->data_offset) // both 8
sec->data_offset = size;
}
Nothing here make sure that the VFP double is aligned to 8 Byte.
And indeed, (0x24b01e % 8) == 6, not 0.
Alignment could be disabled on the CPU
https://developer.arm.com/documentation/ddi0464/f/system-control/register-descriptions/system-control-register
but I don't think EABI wants that.
tinycc does have:
/* reserve at least 'size' bytes aligned per 'align' in section
'sec' from current offset, and return the aligned offset */
ST_FUNC size_t section_add(Section *sec, addr_t size, int align)
{
size_t offset, offset1;
offset = (sec->data_offset + align - 1) & -align;
offset1 = offset + size;
if (sec->sh_type != SHT_NOBITS && offset1 > sec->data_allocated)
section_realloc(sec, offset1);
sec->data_offset = offset1;
if (align > sec->sh_addralign)
sec->sh_addralign = align;
return offset;
}
But that's not used for init_putv.
And section_reserve, which is used, doesn't care about alignment at all.
(it seems there's a reserved part and a data part in each section, and
it holds that the data part elements are aligned--but the reserved part
elements are NOT aligned. I don't see how sec->data would be aligned
by the dynamic memory allocator either)
Other notes:
tccgen.c even has this:
> /* XXX: when cross-compiling we assume that each type has the
> same representation on host and target, which is likely to
> be wrong in the case of long double */
Yeah, and even when NOT cross-compiling, the alignment is wrong--which means
it sometimes won't work at all on ARM, depending on luck.
As a workaround, we can patch tcc to instead do the assignments on elements
on the stack and then copy those over, instead of doing
*(double *)ptr = vtop->c.d
(the latter of which emits VFP instructions that expect double-aligned
pointers).
pgptOGcbf1_s5.pgp
Description: OpenPGP digital signature
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', (continued)
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Ludovic Courtès, 2021/01/14
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', jeremiah, 2021/01/14
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Ludovic Courtès, 2021/01/21
- RE: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Orians, Jeremiah (DTMB), 2021/01/21
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Ludovic Courtès, 2021/01/28
- RE: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Orians, Jeremiah (DTMB), 2021/01/28
Re: [bootstrappable] wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Paul Sherwood, 2021/01/06
- Re: [bootstrappable] wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Jan Nieuwenhuizen, 2021/01/07
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Danny Milosavljevic, 2021/01/07
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Danny Milosavljevic, 2021/01/07
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello',
Danny Milosavljevic <=
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Jan Nieuwenhuizen, 2021/01/08
- Re: [Tinycc-devel] [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', arnold, 2021/01/08
- Re: [Tinycc-devel] [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Jan Nieuwenhuizen, 2021/01/08
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Danny Milosavljevic, 2021/01/08
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Danny Milosavljevic, 2021/01/08
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Jan Nieuwenhuizen, 2021/01/08
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Danny Milosavljevic, 2021/01/08
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Danny Milosavljevic, 2021/01/08
- Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', Jan Nieuwenhuizen, 2021/01/08
Re: [Tinycc-devel] [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello', grischka, 2021/01/08