unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Danny Milosavljevic <dannym@scratchpost.org>
To: Jan Nieuwenhuizen <janneke@gnu.org>
Cc: guix-devel@gnu.org, tinycc-devel@nongnu.org,
	Michael Matz <matz.tcc@frakked.de>,
	Paul Sherwood <paul.sherwood@codethink.co.uk>,
	bootstrappable@freelists.org
Subject: Re: [bootstrappable] Re: wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello'
Date: Thu, 7 Jan 2021 23:52:41 +0100	[thread overview]
Message-ID: <20210107235208.04835b95@scratchpost.org> (raw)
In-Reply-To: <20210107212324.36bfd236@scratchpost.org>

[-- Attachment #1: Type: text/plain, Size: 3911 bytes --]

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).

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2021-01-07 23:14 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-04 17:01 wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello' Jan Nieuwenhuizen
2021-01-05  0:49 ` [bootstrappable] " jeremiah
2021-01-05 16:58 ` Pierre Neidhardt
2021-01-06 11:32 ` [bootstrappable] " Ludovic Courtès
2021-01-06 11:46   ` [bootstrappable] " Andrius Štikonas via Development of GNU Guix and the GNU System distribution.
2021-01-06 14:03     ` Orians, Jeremiah (DTMB)
2021-01-14 21:37     ` Ludovic Courtès
2021-01-15  1:27       ` jeremiah
2021-01-21 11:09         ` Ludovic Courtès
2021-01-21 17:52           ` Orians, Jeremiah (DTMB)
2021-01-28 13:40             ` Ludovic Courtès
2021-01-28 13:44               ` Orians, Jeremiah (DTMB)
2021-01-06 14:38 ` [bootstrappable] " Paul Sherwood
2021-01-07 10:43   ` Jan Nieuwenhuizen
2021-01-07 20:10     ` [bootstrappable] " Danny Milosavljevic
2021-01-07 20:23       ` Danny Milosavljevic
2021-01-07 22:52         ` Danny Milosavljevic [this message]
2021-01-08  6:25           ` Jan Nieuwenhuizen
2021-01-08  8:05             ` [Tinycc-devel] " arnold
2021-01-08 13:02               ` Jan Nieuwenhuizen
2021-01-08 13:43             ` Danny Milosavljevic
2021-01-08 14:07               ` Danny Milosavljevic
2021-01-08 16:15                 ` Jan Nieuwenhuizen
2021-01-08 18:56                   ` Danny Milosavljevic
2021-01-08 21:11                     ` Danny Milosavljevic
2021-01-08 22:13                       ` Jan Nieuwenhuizen
2021-01-08  7:16           ` [Tinycc-devel] " grischka
2021-01-08 13:25             ` Danny Milosavljevic
2021-01-08 13:36               ` [bootstrappable] Re: [Tinycc-devel] " Orians, Jeremiah (DTMB)
2021-01-08 15:16                 ` [Tinycc-devel] [bootstrappable] " Vincent Lefevre
2021-01-08 16:12               ` [Tinycc-devel] [bootstrappable] " Jan Nieuwenhuizen
2021-01-25 22:47       ` ARM Unified Assembly Language - GNU as does some weird stuff Danny Milosavljevic
2021-01-25 23:12         ` [bootstrappable] " Orians, Jeremiah (DTMB)
2021-01-26  1:14         ` Danny Milosavljevic
2021-01-08  0:29 ` wip-full-source-bootstrap: from a 357-byte `hex0' to 'hello' Jan Wielkiewicz
2021-01-20 20:19 ` Timothy Sample
2021-01-25 18:48   ` Efraim Flashner
2021-01-29  7:23     ` [bootstrappable] " Jan Nieuwenhuizen
2021-01-29 10:18       ` andrius--- via Development of GNU Guix and the GNU System distribution.
2021-01-29 16:02         ` Orians, Jeremiah (DTMB)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210107235208.04835b95@scratchpost.org \
    --to=dannym@scratchpost.org \
    --cc=bootstrappable@freelists.org \
    --cc=guix-devel@gnu.org \
    --cc=janneke@gnu.org \
    --cc=matz.tcc@frakked.de \
    --cc=paul.sherwood@codethink.co.uk \
    --cc=tinycc-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).