From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9E7EFC433B4 for ; Tue, 4 May 2021 21:30:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6BCD7613CD for ; Tue, 4 May 2021 21:30:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232841AbhEDVbs (ORCPT ); Tue, 4 May 2021 17:31:48 -0400 Received: from mail.kernel.org ([198.145.29.99]:49404 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232684AbhEDVbs (ORCPT ); Tue, 4 May 2021 17:31:48 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 7CBBB61106; Tue, 4 May 2021 21:30:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1620163852; bh=GzuA2VdQODBiMuxV+QB96CKaJY3mhfJcXcOM9EdM5d8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=qHvNFRceON0YxFtA7eoKZudyK4dJfdyFt/3c8s9TcmUEa0MK5MoR2WL+YUkjODQT9 v1Fx7ioB1MKuiZLLozbAZztBQFYHeW2G93RnzmX3gZbuTZ7qxvFpu8LC7mDZ6SGvqW 1qMHdwqT4+XzIupcJPpOHSEZ6R/M5O9kWURzSK0OksHBToo+qhPCpXE2Ba7gC1oeZj Ua995u2P3B5CwRe4ZS6mcZO/yFv+ZhTShSrxjLcoHEKcD6VsV5gx75Z6sQvXGMnnbi UgfCsVoC41nPbgivzyMSwCJYk7WH6Ij4nHBwXHNDMeUfI5dwBecnzYus8DF/YjDtwb UArJHF4XBosMA== Date: Tue, 4 May 2021 14:30:50 -0700 From: Eric Biggers To: Theodore Ts'o Cc: harshad shirwadkar , Andreas Dilger , Ext4 Developers List , Harshad Shirwadkar Subject: Re: [PATCH] e2fsck: fix portability problems caused by unaligned accesses Message-ID: References: <8E9C71E8-FE5F-4CB8-BA62-8D8895DCA92A@dilger.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On Tue, May 04, 2021 at 05:10:34PM -0400, Theodore Ts'o wrote: > > Basically, what gcc (and presumably clang) is doing is it is special > casing packed_ptr->field so that the compiled code will work > regardless of the alignment of packed_ptr. This isn't documented > anywhere, but it apparently is the case. (I had assumed that it would > only generate unaligned access for those fields that are not aligned > if the structure started on an aligned boundary.) I don't think it's related to the pointer dereference per se, but rather the compiler assigns an alignment of 1 to all fields in a packed struct (even the field at the beginning of the struct). If you had a packed struct as a global variable and did 'packed_struct.field', the behavior would be the same. > > If we really don't want to use __attribute__((packed)) that is fine, but then > > we'll need to remember to use an unaligned accessor *every* field access (except > > for bytes), which seems harder to me -- and the compiler won't warn when one of > > these is missing. (They can only be detected at runtime using UBSAN.) > > One reason not to use the __packed__ attribute is that there are cases > where people attempt to build e2fsprogs on non-gcc/non-clang binaries. > At one point FreeBSD was trying to use pcc to build e2fsprogs IIRC. > And certainly there are people who try to build e2fsprogs on MSVC on > Windows. Is that really true, given that e2fsprogs already uses a lot of gcc/clang extensions, including __attribute((packed))__ already? > So maybe the memcpy to a local copy is the better way to go, and > hopefully the C compiler will optimize away the local copy on > architectures where it is safe to do so. And in the unlikely case > that it is a performance bottleneck, we could add a -DUBSAN when > configure --enable-ubsan is in force, which switches in the memcpy > when only when ubsan is enabled. These days the memcpy() approach does get optimized properly. armv6 and armv7 with gcc used to be a notable exception, but it got fixed in gcc 6 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67366). - Eric