From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 81648 invoked by alias); 2 May 2018 15:18:26 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 81638 invoked by uid 89); 2 May 2018 15:18:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=linkedin, shine, LinkedIn, julio X-HELO: a2-118.smtp-out.eu-west-1.amazonses.com Received: from a2-118.smtp-out.eu-west-1.amazonses.com (HELO a2-118.smtp-out.eu-west-1.amazonses.com) (54.240.2.118) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 02 May 2018 15:18:24 +0000 Subject: Re: [PATCH 3/4] Explicit fixed-width integral File I/O protocol types From: =?UTF-8?Q?Julio_Guerra?= To: =?UTF-8?Q?Pedro_Alves?= , =?UTF-8?Q?gdb-patches=40s?= =?UTF-8?Q?ourceware=2Eorg?= Cc: =?UTF-8?Q?Mike_Frysinger?= , =?UTF-8?Q?Joel_Brobeck?= =?UTF-8?Q?er?= Date: Wed, 02 May 2018 15:18:00 -0000 Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable In-Reply-To: References: <20180428011940.115515-1-julio@farjump.io> <20180428011940.115515-4-julio@farjump.io> <332abe88-4b88-3f06-7141-31a798f2b153@farjump.io> Message-ID: <01020163216ed537-dae77402-9d35-42f2-ad27-64c89137aa03-000000@eu-west-1.amazonses.com> X-SES-Outgoing: 2018.05.02-54.240.2.118 Feedback-ID: 1.eu-west-1.b24dn6frgCi6dh20skzbuMRr7UL8M6Soir/3ogtEjHQ=:AmazonSES X-IsSubscribed: yes X-SW-Source: 2018-05/txt/msg00014.txt.bz2 Hello,=0D =0D Since I am talking a lot of embedded software, I CC'd Joel Brobecker=0D from AdaCore to maybe give the final direction for a v2.=0D =0D On 05/02/2018 12:45, Pedro Alves wrote :=0D > I don't see how using GCC preprocessor builtins would be more=0D > portable than stdint.h. That doesn't make the file=0D > standalone -- it makes it dependent on gcc instead.=0D I was indeed writing a GCC-based solution. Isn't GDB only compiled with GCC= ?=0D =0D When compiling embedded programs doing File IOs, I have always been in=0D bare-metal cases, thus using CFLAGS "-ffreestanding -nostdinc=0D -nostdlib". So including "stdint.h" would fail because of -nostdinc. And=0D since my end goal here is to make fileio.h the single source of truth of=0D File IO protocol types, to then be able to include it in other C/C++=0D programs, I need it be compiled with these flags, i.e. without the=0D standard library. And in my opinion, using the File IO protocol in=0D bare-metal programs is where File IOs shine the most :)=0D =0D So, if other compilers than GCC are required, I can rewrite it using=0D standard unsigned int, etc.=0D > But regardless, the major problem here is that this approach=0D > does not work, because it ignores the issue of alignment and=0D > padding holes.=0D > With the current approach, all structure fields are aligned=0D > to 1 byte. With the explicit types approach, fields are=0D > aligned to their natural alignment. E.g., from a glance,=0D > it seems like we'd be creating a 4-byte padding hole after=0D > fst_rdev. Even if by chance the fields end up aligned,=0D > that's not something we should be relying on. Both host=0D > and client must agree on the layout of the data structures.=0D > fio_stat objects are copied directly into target memory,=0D > see tail end of remote-fileio.c:remote_fileio_func_stat,=0D > for example:=0D >=0D > if (statptr)=0D > {=0D > host_to_fileio_stat (&st, &fst);=0D > host_to_fileio_uint (0, fst.fst_dev);=0D >=0D > errno =3D target_write_memory (statptr, (gdb_byte *) &fst, sizeof f= st);=0D > if (errno !=3D 0)=0D =0D Yes, and this is why I rather prefer this fileio.h file to become the=0D single source of truth instead of redefining them and ending with=0D mistakes such as memory overflows. For example, note the "sizeof fst",=0D where we are assuming that the target program has correctly redefined=0D it. Sharing fileio.h with the target will most likely limit the problem.=0D =0D I indeed missed the alignment. Can't we simply pack the structure=0D "__attribute__ ((packed))"? I find these implicit "address-of" used=0D everywhere in GDB's code very opaque, and I am not sure how it would end=0D up when included in other C/C++ programs. It is anyway a source of=0D errors since no one expects integral types to be defined as an array, at=0D least not named like that (for example, I had problems because I wrote=0D fio_uint* to pass a File IO integer pointer, which ends up as a=0D "char(*)[32]"...).=0D > As for the "int"/"long"/etc naming vs explicitly-sized uint32_t etc.,=0D > note that the File I/O protocol's "int" "long" etc. types are defined her= e:=0D >=0D > https://sourceware.org/gdb/current/onlinedocs/gdb/Integral-Datatypes.htm= l#Integral-Datatypes=0D >=0D > I don't have a strong opinion, but I'm not sure it's worth it to=0D > deviate from the documentation.=0D Sorry, I missed this documentation entry. I'm totally fine in keeping=0D current type names since if fielio.h ends up installed, we would still=0D be able to avoid redefiniton mistakes and reuse them.=0D =0D -- =0D Julio Guerra=0D Co-founder & CTO of Farjump=0D Mobile: +33 618 644 164=0D LinkedIn: https://linkedin.com/in/guerrajulio=0D Slack: farjump.slack.com=0D =0D