From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 51814 invoked by alias); 12 Oct 2018 21:07:29 -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 51801 invoked by uid 89); 12 Oct 2018 21:07:29 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL,BAYES_00,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=guaranty, UD:the, html_node, longjmp X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 12 Oct 2018 21:07:27 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id CC63A560BE; Fri, 12 Oct 2018 17:07:25 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id S+Ew9izCIlbs; Fri, 12 Oct 2018 17:07:25 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id A4154560B7; Fri, 12 Oct 2018 17:07:25 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 0D9A082BB3; Fri, 12 Oct 2018 17:07:25 -0400 (EDT) Date: Fri, 12 Oct 2018 21:07:00 -0000 From: Joel Brobecker To: Pedro Alves Cc: John Baldwin , Tom Tromey , gdb-patches@sourceware.org Subject: Re: [PATCH v2 00/10] Undefined Behavior Sanitizer, this time with docs Message-ID: <20181012210724.GA6340@adacore.com> References: <20181002044420.17628-1-tom@tromey.com> <8df7a2b9-800a-5a70-1075-e687145c9394@FreeBSD.org> <20181008202245.GC2993@adacore.com> <4afd7c11-d43d-191a-201f-3ed3db7da38e@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4afd7c11-d43d-191a-201f-3ed3db7da38e@redhat.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-SW-Source: 2018-10/txt/msg00298.txt.bz2 > This also revealed PR 23743: > https://sourceware.org/bugzilla/show_bug.cgi?id=23743 For the record, I've also discovered something I didn't realize about struct memory allocations, which is a bit disturbing, because I think we have a lot of areas where we malloc struct objects... I reported it under https://sourceware.org/bugzilla/show_bug.cgi?id=23768 For now, it seems like the lessons learned is that it seems we should be very careful when using XNEW/XCNEW with struct types? ---------------------------------------------------------------------- on ppc-linux, a GDB built with --enable-ubsan=yes crashes immediately as soon as one tries to enter anything at the prompt: $ /path/to/gdb GNU gdb (GDB) 8.2.50.20181012-git (with AdaCore local changes) Copyright (C) 2018 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later This is free software: you are free to change and redistribute it. See your support agreement for details of warranty and support. If you do not have a current support agreement, then there is absolutely no warranty for this version of GDB. Type "show copying" and "show warranty" for details. This GDB was configured as "powerpc-generic-linux-gnu". Type "show configuration" for configuration details. For help, type "help". Type "apropos word" to search for commands related to "word". (gdb) /homes/brobecke/act/gdb/gdb-head/gdb/common/common-exceptions.c:84:26: runtime error: member access within misaligned address 0x124af518 for type 'struct catcher', which requires 16 byte alignment The code that triggers the error is fairly straightforward: | jmp_buf * | exceptions_state_mc_init (void) | { | struct catcher *new_catcher = XCNEW (struct catcher); | | /* Start with no exception. */ | new_catcher->exception = exception_none; The problem happens because "struct catcher" is a structure which requires a 16 bytes alignment, due to one of its members (the jmpbuf) having itself a 16bytes alignment: >From bits/setjmp.h on ppc-linux: | /* The current powerpc 32-bit Altivec ABI specifies for SVR4 ABI and EABI | the vrsave must be at byte 248 & v20 at byte 256. So we must pad this | correctly on 32 bit. It also insists that vecregs are only gauranteed | 4 byte alignment so we need to use vperm in the setjmp/longjmp routines. | We have to version the code because members like int __mask_was_saved | in the jmp_buf will move as jmp_buf is now larger than 248 bytes. We | cannot keep the altivec jmp_buf backward compatible with the jmp_buf. */ | #ifndef _ASM | # if __WORDSIZE == 64 | typedef long int __jmp_buf[64] __attribute__ ((__aligned__ (16))); | # else | /* The alignment is not essential, i.e.the buffer can be copied to a 4 byte | aligned buffer as per the ABI it is just added for performance reasons. */ | typedef long int __jmp_buf[64 + (12 * 4)] __attribute__ ((__aligned__ (16))); | # endif XCNEW performs calls to malloc without worrying about alignment, so there is indeed no guaranty that the returned address is aligned on 16 bytes. We looked at the GNU libc documentation, for instance, and they guaranty 8 bytes only on 32bit machines | https://www.gnu.org/software/libc/manual/html_node/Aligned-Memory-Blocks.html | | 3.2.3.6 Allocating Aligned Memory Blocks | | The address of a block returned by malloc or realloc in GNU systems is | always a multiple of eight (or sixteen on 64-bit systems). If you need a | block whose address is a multiple of a higher power of two than that, | use aligned_alloc or posix_memalign. aligned_alloc and posix_memalign | are declared in stdlib.h. We can certainly start by plugging the hole in this particular case, and swap the call to XCNEW with a call to an aligned malloc. But this made me realize we have a general issue, because potentially, all struct allocations are potentially problematic, if they happen to have alignment requirements that are stronger than what malloc itself follows. I hope that, in practice, aligment requirements beyond 4 or 8 bytes are rare. Perhaps one possible idea was to have XNEW/XCNEW et all take the alignment into account when performing the memory allocation (we pass the type, so it could use alignof); however, that is a major change affecting everyone. And looking closer at those macros, one notices the following important comment... /* Scalar allocators. */ ... which tells me the macros may not have been meant to be used in the context of allocating structures. It's unclear to me what we would want to do... -- Joel