From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.44]) by sourceware.org (Postfix) with ESMTPS id 9A9C9384D161 for ; Wed, 29 Jun 2022 17:38:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9A9C9384D161 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f44.google.com with SMTP id v9so7955130wrp.7 for ; Wed, 29 Jun 2022 10:38:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=Z03XOsEmxDRjYtFuK9OvIJtKIvj0MDCv0UOn+A3mJu8=; b=0uDc517dj28tQ6JaFLUchhlHaJg1P1hhNOjjGraNKKLkRAvcmZKmcYSMDgGEeWNJ5B mOhhWN7QyhMuBku60MZvbCHoBIppP8OjZPQ0yf5g27CSDcZQcg0VIYQfI6YPTGr2IMYw nAP+0+BlZEnWu36ri/DsIHSBAbqT6tp5XXrGydSsTwLrVSfliqlAOqd9XsDJELwX4QSV AflCkxVGOgbnNvAEQOJO3O1s7PSR64o+Z0nC+QIAFWAgp7e3jXw9EmFrJOT+rLMGZGOs pMK2hRyoiu7iPClm4eZLaTL6awQFLak4h/mTw4m6hlHU7RWr/euPVEgkRCeHlFA11jHa D7/g== X-Gm-Message-State: AJIora/LwDZcUrlYHrCmUEPBwTUYL8qVhQlaGnt7uOL8X6cUmeVaP999 l9FuSiOXyV+Xj3fJ5ZWyERUmUcMCsiQ= X-Google-Smtp-Source: AGRyM1tdmHMrb+LR4DJhGcRVCTjBa4sKwGwyVGsq4hmiyN/XjSaExqLdlbYQooQNdQ413qqe+q9QDw== X-Received: by 2002:a05:6000:1445:b0:21b:a919:7d3 with SMTP id v5-20020a056000144500b0021ba91907d3mr4195484wrx.545.1656524318471; Wed, 29 Jun 2022 10:38:38 -0700 (PDT) Received: from ?IPV6:2001:8a0:f924:2600:209d:85e2:409e:8726? ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id t22-20020a05600c41d600b00397393419e3sm3893890wmh.28.2022.06.29.10.38.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 29 Jun 2022 10:38:37 -0700 (PDT) Message-ID: Date: Wed, 29 Jun 2022 18:38:36 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: Re: [PATCH 3/5] [gdb/symtab] Work around fsanitize=address false positive for per_cu->lang Content-Language: en-US To: Tom de Vries , gdb-patches@sourceware.org Cc: Tom Tromey References: <20220629152914.13149-1-tdevries@suse.de> <20220629152914.13149-3-tdevries@suse.de> From: Pedro Alves In-Reply-To: <20220629152914.13149-3-tdevries@suse.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Jun 2022 17:38:42 -0000 On 2022-06-29 16:29, Tom de Vries via Gdb-patches wrote: > When building gdb with -fsanitize=thread and gcc 12, and running test-case > gdb.dwarf2/dwz.exp, we run into a data race between: > ... > Read of size 1 at 0x7b200000300d by thread T2:^M > #0 cutu_reader::cutu_reader(dwarf2_per_cu_data*, dwarf2_per_objfile*, \ > abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) gdb/dwarf2/read.c:6164 \ > (gdb+0x82ec95)^M > ... > and: > ... > Previous write of size 1 at 0x7b200000300d by main thread:^M > #0 prepare_one_comp_unit gdb/dwarf2/read.c:23588 (gdb+0x86f973)^M > ... > > In other words, between: > ... > if (this_cu->reading_dwo_directly) > ... > and: > ... > cu->per_cu->lang = pretend_language; > ... > > Both fields are part of the same bitfield, and writing to one field while > reading from another is not a problem, so this is a false positive. I don't understand this sentence. Particularly "same bitfield", or really "Both fields are part of the same bitfield,". How can two bitfields be part of the same bitfield? Anyhow, both bitfields are part of a sequence of contiguous bitfields, here stripped of comments: unsigned int queued : 1; unsigned int is_debug_types : 1; unsigned int is_dwz : 1; unsigned int reading_dwo_directly : 1; unsigned int tu_read : 1; mutable bool m_header_read_in : 1; bool addresses_seen : 1; unsigned int mark : 1; bool files_read : 1; ENUM_BITFIELD (dwarf_unit_type) unit_type : 8; ENUM_BITFIELD (language) lang : LANGUAGE_BITS; Per C++11, they're all part of the same _memory location_. From N3253 (C++11), intro.memory: "A memory location is either an object of scalar type or a maximal sequence of adjacent bit-fields all having non-zero width. (...) Two threads of execution (1.10) can update and access separate memory locations without interfering with each other. (...) [ Note: Thus a bit-field and an adjacent non-bit-field are in separate memory locations, and therefore can be concurrently updated by two threads of execution without interference. The same applies to two bit-fields, if one is declared inside a nested struct declaration and the other is not, or if the two are separated by a zero-length bit-field declaration, or if they are separated by a non-bit-field declaration. It is not safe to concurrently update two bit-fields in the same struct if all fields between them are also bit-fields of non-zero width. — end note ]" And while it is true that in practice writing to one bit-field from one thread and reading from another, if they reside on the same location, is OK in practice, it is still undefined behavior. Note the escape hatch mentioned above: "if the two are separated by a zero-length bit-field declaration" Thus, a change like this: unsigned int queued : 1; unsigned int is_debug_types : 1; unsigned int is_dwz : 1; unsigned int reading_dwo_directly : 1; unsigned int tu_read : 1; mutable bool m_header_read_in : 1; bool addresses_seen : 1; unsigned int mark : 1; bool files_read : 1; ENUM_BITFIELD (dwarf_unit_type) unit_type : 8; + + /* Ensure lang is a separate memory location, so we can update + it concurrently with other bitfields. */ + char :0; + ENUM_BITFIELD (language) lang : LANGUAGE_BITS; ... should work.